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
+ - const: ufs_sel_min_src
+ - const: ufs_sel_max_src
operating-points-v2: true
@@ -131,9 +135,27 @@ allOf:
properties:
clocks:
minItems: 13
+ maxItems: 13
clock-names:
minItems: 13
+ maxItems: 13
avdd09-supply: false
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: mediatek,mt8196-ufshci
+ then:
+ properties:
+ clocks:
+ minItems: 16
+ maxItems: 16
+ clock-names:
+ minItems: 16
+ maxItems: 16
+ avdd18-supply: false
+ required:
+ - operating-points-v2
examples:
- |
@@ -183,3 +205,37 @@ examples:
mediatek,ufs-disable-mcq;
};
+ - |
+ #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>;
+ 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>;
+ vccq-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;
+ };
--
2.53.0On 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. > + - 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. Rob
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 >
On Fri, Mar 6, 2026 at 12:37 PM Nicolas Frattaroli <nicolas.frattaroli@collabora.com> wrote: > > 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? Shrug... Is changing it really that hard? > > > + - 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. It's good Mediatek is active, then they can tell us what the clocks are for. I would think the driver would give some clue. I don't see how accepting sub-par bindings or not fixes the issues here. Rob
Nicolas, >> "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? It is if it comes from one of the DT maintainers. > 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 This has to stop. Please read Documentation/process/code-of-conduct.rst. -- Martin K. Petersen
On Saturday, 7 March 2026 19:01:17 Central European Standard Time Martin K. Petersen wrote: > > Nicolas, > > >> "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? > > It is if it comes from one of the DT maintainers. > > > 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 > > This has to stop. Please read Documentation/process/code-of-conduct.rst. > > I apologise for my tone, it's my frustration getting the better of me. I'll be handing off this series to someone else, so you won't have to deal with me anymore. I do ask however that you don't apply patches from MediaTek blindly; if there's code to read an OF property, and that OF property is not in the binding, then the patch should be rejected, even if there's an Ack from the MediaTek maintainer.
On Mon, Mar 9, 2026 at 5:04 AM Nicolas Frattaroli <nicolas.frattaroli@collabora.com> wrote: > > On Saturday, 7 March 2026 19:01:17 Central European Standard Time Martin K. Petersen wrote: > > > > Nicolas, > > > > >> "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? > > > > It is if it comes from one of the DT maintainers. > > > > > 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 > > > > This has to stop. Please read Documentation/process/code-of-conduct.rst. I have little doubt that that is an accurate description of downstream. And if properties are getting added without bindings, then that is certainly a problem that should be complained about. > > > > > > I apologise for my tone, it's my frustration getting the better of me. > > I'll be handing off this series to someone else, so you won't have to > deal with me anymore. > > I do ask however that you don't apply patches from MediaTek blindly; > if there's code to read an OF property, and that OF property is not > in the binding, then the patch should be rejected, even if there's an > Ack from the MediaTek maintainer. There's functionality to find undocumented compatibles in kernel code (make dt_compatible_check), but not properties. Sounds like I need to add that. Rob
Il 10/03/26 19:21, Rob Herring ha scritto: > On Mon, Mar 9, 2026 at 5:04 AM Nicolas Frattaroli > <nicolas.frattaroli@collabora.com> wrote: >> >> On Saturday, 7 March 2026 19:01:17 Central European Standard Time Martin K. Petersen wrote: >>> >>> Nicolas, >>> >>>>> "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? >>> >>> It is if it comes from one of the DT maintainers. >>> >>>> 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 >>> >>> This has to stop. Please read Documentation/process/code-of-conduct.rst. > > I have little doubt that that is an accurate description of > downstream. And if properties are getting added without bindings, then > that is certainly a problem that should be complained about. > >>> >>> >> >> I apologise for my tone, it's my frustration getting the better of me. >> >> I'll be handing off this series to someone else, so you won't have to >> deal with me anymore. >> >> I do ask however that you don't apply patches from MediaTek blindly; >> if there's code to read an OF property, and that OF property is not >> in the binding, then the patch should be rejected, even if there's an >> Ack from the MediaTek maintainer. > > There's functionality to find undocumented compatibles in kernel code > (make dt_compatible_check), but not properties. Sounds like I need to > add that. Rob: yes please, that would help a lot in general, not just with this UFS driver. Cheers, Angelo > > Rob
© 2016 - 2026 Red Hat, Inc.