Re: [PATCH v9 03/23] dt-bindings: ufs: mediatek,ufs: Add mt8196 variant

[Nicolas Frattaroli <nicolas.frattaroli@collabora.com>]

On Friday, 6 March 2026 17:33:05 Central European Standard Time Rob Herring wrote:
> On Fri, Mar 06, 2026 at 02:24:44PM +0100, Nicolas Frattaroli wrote:
> > The MediaTek MT8196 SoC's UFS controller uses three additional clocks
> > compared to the MT8195, and a different set of supplies. It is therefore
> > not compatible with the MT8195.
> > 
> > While it does have a AVDD09_UFS_1 pin in addition to the AVDD09_UFS pin,
> > it appears that these two pins are commoned together, as the board
> > schematic I have access to uses the same supply for both, and the
> > downstream driver does not distinguish between the two supplies either.
> > 
> > Add a compatible for it, and modify the binding correspondingly.
> > 
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > Acked-by: Vinod Koul <vkoul@kernel.org>
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >  .../devicetree/bindings/ufs/mediatek,ufs.yaml      | 58 +++++++++++++++++++++-
> >  1 file changed, 57 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> > index e0aef3e5f56b..a82119ecbfe8 100644
> > --- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> > +++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> > @@ -16,10 +16,11 @@ properties:
> >        - mediatek,mt8183-ufshci
> >        - mediatek,mt8192-ufshci
> >        - mediatek,mt8195-ufshci
> > +      - mediatek,mt8196-ufshci
> >  
> >    clocks:
> >      minItems: 1
> > -    maxItems: 13
> > +    maxItems: 16
> >  
> >    clock-names:
> >      minItems: 1
> > @@ -37,6 +38,9 @@ properties:
> >        - const: crypt_perf
> >        - const: ufs_rx_symbol0
> >        - const: ufs_rx_symbol1
> > +      - const: ufs_sel
> 
> "ufs" is redundant as all the clocks are for UFS. Same comment on prior 
> patch.

Is this naming a big enough concern to block this series with two
explicit acks on this patch that fixes a wholly broken and useless
binding?

> 
> > +      - const: ufs_sel_min_src
> > +      - const: ufs_sel_max_src
> 
> "src" sounds like a parent clock? If so, probably shouldn't be in the 
> clocks list. 'assigned-clocks' is for dealing with parent clocks.
> 

I don't know what it is, and I have no way to consult any documentation
that would tell me what it is. I am trying to put out this dumpster fire
of a downstream turd that made its way into mainline as the review process
has been completely subverted, and is only getting worse with each passing
month that MediaTek is allowed to block this series from progressing while
sneaking further changes through.

> Rob
>