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

Nicolas Frattaroli
2025-10-24T17:52:03Z

On Friday, 24 October 2025 19:13:36 Central European Summer Time Conor Dooley wrote:
> On Thu, Oct 23, 2025 at 09:49:21PM +0200, Nicolas Frattaroli wrote:
>
> > };
> > + - |
> > + #include <dt-bindings/reset/mediatek,mt8196-resets.h>
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > + ufshci@16810000 {
> > + compatible = "mediatek,mt8196-ufshci";
> > + reg = <0x16810000 0x2a00>;
> > + interrupts = <GIC_SPI 320 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > + clocks = <&ufs_ao_clk 6>,
> > + <&ufs_ao_clk 7>,
> > + <&clk26m>,
> > + <&ufs_ao_clk 3>,
> > + <&clk26m>,
> > + <&ufs_ao_clk 4>,
> > + <&ufs_ao_clk 0>,
> > + <&topckgen 7>,
> > + <&topckgen 41>,
> > + <&topckgen 105>,
> > + <&topckgen 83>,
> > + <&ufs_ao_clk 1>,
> > + <&ufs_ao_clk 2>,
> > + <&topckgen 42>,
> > + <&topckgen 84>,
> > + <&topckgen 102>;
>
> This is absolutely a nitpick thing, but if you end up resubmitting, can
> you pick a consistent format between the two examples your series adds
> for the clocks/clock names?

No problem, will do. IIRC I kept them as a list like this so I could
easily reorder things, but now that I'm fairly sure this order is the
correct one, it's probably best to make this more compact.

Also sorry for the numbers as clock IDs, but MediaTek clock headers
have conflicting symbols and the dt schema example extractor dumps
all examples into one dts file. :(

Since this has bugged me in the past, and many schemas may rely on
the concat behaviour now: would a patch in the distant future that
prefixes all MediaTek clock binding headers with the SoC name be
acceptable if it keeps the old names intact as aliases to them with
a #ifndef guard?

I should also think about some way we can avoid similar bindings
symbol naming mishaps in the future.

Thank you for pointing me in the right direction with regards to
the binding!

Kind regards,
Nicolas Frattaroli

> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> pw-bot: not-applicable
>
> > + clock-names = "ufs",
> > + "ufs_aes",
> > + "ufs_tick",
> > + "unipro_sysclk",
> > + "unipro_tick",
> > + "unipro_mp_bclk",
> > + "ufs_tx_symbol",
> > + "ufs_mem_sub",
> > + "crypt_mux",
> > + "crypt_lp",
> > + "crypt_perf",
> > + "ufs_rx_symbol0",
> > + "ufs_rx_symbol1",
> > + "ufs_sel",
> > + "ufs_sel_min_src",
> > + "ufs_sel_max_src";
> > +
> > + operating-points-v2 = <&ufs_opp_table>;
> > +
> > + phys = <&ufsphy>;
> > +
> > + avdd09-supply = <&mt6363_vsram_modem>;
> > + vcc-supply = <&mt6363_vemc>;
> > + vcc-supply-1p8;
> > + vccq-supply = <&mt6363_va12_2>;
> > + vccq2-supply = <&mt6363_vufs12>;
> > +
> > + resets = <&ufs_ao_clk MT8196_UFSAO_RST1_UFS_UNIPRO>,
> > + <&ufs_ao_clk MT8196_UFSAO_RST1_UFS_CRYPTO>,
> > + <&ufs_ao_clk MT8196_UFSAO_RST1_UFSHCI>;
> > + reset-names = "unipro", "crypto", "hci";
> > + mediatek,ufs-disable-mcq;
> > + };
> >
>