| Message ID | 20240411114300.169055-1-angelogioacchino.delregno@collabora.com |
|---|---|
| Headers | show |
| Series | MediaTek UFS fixes and cleanups - Part 1 | expand |
On Thu, Apr 11, 2024 at 01:42:57PM +0200, AngeloGioacchino Del Regno wrote: > The MT8192 UFS controller is compatible with the MT8183 one: > document this by allowing to assign both compatible strings > "mediatek,mt8192-ufshci", "mediatek,mt8183-ufshci" to the UFSHCI node. > > In preparation for adding MT8195 to the mix, the MT8192 compatible > was added as enum instead of const. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > .../devicetree/bindings/ufs/mediatek,ufs.yaml | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml > index 32fd535a514a..adcd13023866 100644 > --- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml > +++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml > @@ -14,9 +14,15 @@ allOf: > > properties: > compatible: > - enum: > - - mediatek,mt8183-ufshci > - - mediatek,mt8192-ufshci > + oneOf: > + - items: > + - enum: > + - mediatek,mt8183-ufshci > + - mediatek,mt8192-ufshci > + - items: > + - enum: > + - mediatek,mt8192-ufshci > + - const: mediatek,mt8183-ufshci It's a bit more distruptive since you'll have to modify a dts, but why permit both of these ways of describing the mt8192? Could we drop it from the original enum and no longer allow it in isolation? There shouldn't be any compatibility concerns with doing so.
On Thu, Apr 11, 2024 at 01:42:59PM +0200, AngeloGioacchino Del Regno wrote: > Add additional clocks, used on all MediaTek SoCs' UFSHCI controllers: I appreciate being told they're on all, rather than it being unsaid and having to ask. > some of these clocks are optional and used only for scaling purposes > to save power, or to improve performance in the case of the crypt > clocks. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > .../devicetree/bindings/ufs/mediatek,ufs.yaml | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml > index e2c276da3f2c..21b038db100c 100644 > --- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml > +++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml > @@ -26,11 +26,23 @@ properties: > - const: mediatek,mt8183-ufshci > > clocks: > - maxItems: 1 > + minItems: 1 Could you add an itemised list to the clocks property please? > > clock-names: > + minItems: 1 > items: > - const: ufs > + - const: ufs-aes > + - const: ufs-tick > + - const: unipro-sys > + - const: unipro-tick > + - const: ufs-sap > + - const: ufs-tx-symbol > + - const: ufs-rx-symbol > + - const: ufs-mem > + - const: crypt-mux > + - const: crypt-lp > + - const: crypt-perf > > phys: > maxItems: 1 > -- > 2.44.0 >
Il 11/04/24 17:10, Conor Dooley ha scritto: > On Thu, Apr 11, 2024 at 01:42:59PM +0200, AngeloGioacchino Del Regno wrote: >> Add additional clocks, used on all MediaTek SoCs' UFSHCI controllers: > > I appreciate being told they're on all, rather than it being unsaid and > having to ask. > You're welcome :-) >> some of these clocks are optional and used only for scaling purposes >> to save power, or to improve performance in the case of the crypt >> clocks. >> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >> --- >> .../devicetree/bindings/ufs/mediatek,ufs.yaml | 14 +++++++++++++- >> 1 file changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml >> index e2c276da3f2c..21b038db100c 100644 >> --- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml >> +++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml >> @@ -26,11 +26,23 @@ properties: >> - const: mediatek,mt8183-ufshci >> >> clocks: >> - maxItems: 1 >> + minItems: 1 > > Could you add an itemised list to the clocks property please? > Not really... Honestly, I'm not confident that the description will be 100% correct for all of them... can we do that at a later time, when I will be really that much confident in writing down a proper description for each of them? The only thing that I'm really sure of is exactly what I wrote in this commit, nothing less, nothing more... for now :') Cheers, Angelo >> >> clock-names: >> + minItems: 1 >> items: >> - const: ufs >> + - const: ufs-aes > > >> + - const: ufs-tick >> + - const: unipro-sys >> + - const: unipro-tick >> + - const: ufs-sap >> + - const: ufs-tx-symbol >> + - const: ufs-rx-symbol >> + - const: ufs-mem >> + - const: crypt-mux >> + - const: crypt-lp >> + - const: crypt-perf >> >> phys: >> maxItems: 1 >> -- >> 2.44.0 >>
On Thu, Apr 11, 2024 at 05:14:34PM +0200, AngeloGioacchino Del Regno wrote: > Il 11/04/24 17:10, Conor Dooley ha scritto: > > On Thu, Apr 11, 2024 at 01:42:59PM +0200, AngeloGioacchino Del Regno wrote: > > > Add additional clocks, used on all MediaTek SoCs' UFSHCI controllers: > > > > I appreciate being told they're on all, rather than it being unsaid and > > having to ask. > > > > You're welcome :-) > > > > some of these clocks are optional and used only for scaling purposes > > > to save power, or to improve performance in the case of the crypt > > > clocks. > > > > > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > > > --- > > > .../devicetree/bindings/ufs/mediatek,ufs.yaml | 14 +++++++++++++- > > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml > > > index e2c276da3f2c..21b038db100c 100644 > > > --- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml > > > +++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml > > > @@ -26,11 +26,23 @@ properties: > > > - const: mediatek,mt8183-ufshci > > > clocks: > > > - maxItems: 1 > > > + minItems: 1 > > > > Could you add an itemised list to the clocks property please? > > > > Not really... Honestly, I'm not confident that the description will be 100% > correct for all of them... can we do that at a later time, when I will be > really that much confident in writing down a proper description for each > of them? > > The only thing that I'm really sure of is exactly what I wrote in this commit, > nothing less, nothing more... for now :') fwiw, my motivation here was a better explanation for what "ufs" means as a clock. When the block has some "ufs" clock and a "axi" clock it's kinda clear what they do, but with 7 ufs clocks now, it's not really clear what the bare "ufs" one actually does. If you can't provide an itemised list, please set maxitems. > > > clock-names: > > > + minItems: 1 > > > items: > > > - const: ufs > > > + - const: ufs-aes > > > > > > > + - const: ufs-tick > > > + - const: unipro-sys > > > + - const: unipro-tick > > > + - const: ufs-sap > > > + - const: ufs-tx-symbol > > > + - const: ufs-rx-symbol > > > + - const: ufs-mem > > > + - const: crypt-mux > > > + - const: crypt-lp > > > + - const: crypt-perf > > > phys: > > > maxItems: 1 > > > -- > > > 2.44.0 > > > >
Changes in v2: - Rebased over next-20240409 (because of merge issue for patch 1) - Added ufs: prefix to patch 1 - Added forgotten ufs-rx-symbol clock to the binding This series performs some fixes and cleanups for the MediaTek UFSHCI controller driver. In particular, while adding the MT8195 compatible to the mediatek,ufs binding, I noticed that it was allowing just one clock, completely ignoring the optional ones, including the crypt-xxx clocks, all of the optional regulators, and other properties. Between all the other properties, two are completely useless, as they are there just to activate features that, on SoCs that don't support these, won't anyway be activated because of missing clocks or missing regulators, or missing other properties; as for the other vendor-specific properties, like ufs-disable-ah8, ufs-broken-vcc, ufs-pmc-via-fastauto, since the current merge window is closing, I didn't do extensive research so I've left them in place but didn't add them to the devicetree binding yet. The plan is to check those later and eventually give them a removal treatment, or add them to the bindings in a part two series. For now, at least, this is already a big improvement. P.S.: The only SoC having UFSHCI upstream is MT8183, which only has just one clock, and *nothing else* uses properties, clocks, etc that were renamed in this cleanup. Cheers! AngeloGioacchino Del Regno (8): scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-support-va09 property scsi: ufs: ufs-mediatek: Fix property name for crypt boost voltage scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-boost-crypt property scsi: ufs: ufs-mediatek: Avoid underscores in crypt clock names dt-bindings: ufs: mediatek,ufs: Document MT8192 compatible with MT8183 dt-bindings: ufs: mediatek,ufs: Document MT8195 compatible dt-bindings: ufs: mediatek,ufs: Document additional clocks dt-bindings: ufs: mediatek,ufs: Document optional dvfsrc/va09 regulators .../devicetree/bindings/ufs/mediatek,ufs.yaml | 29 +++++- drivers/ufs/host/ufs-mediatek.c | 91 +++++++++++-------- 2 files changed, 80 insertions(+), 40 deletions(-)