Re: [PATCH v1 1/2] Documentation: dt: reset: add mediatek,syscon-reset binding

[AngeloGioacchino Del Regno]

On 7/1/26 18:57, Conor Dooley wrote:
> On Wed, Jul 01, 2026 at 06:35:20AM +0000, Peter Wang (王信友) wrote:
> > On Fri, 2026-06-26 at 10:33 +0200, Philipp Zabel wrote
> > > Where is the binding doc for mediatek,mt8183-ufs0cfg_ao? Is this
> > > simple-mfd just to load the reset driver?
> >
> > Hi Philipp,
> >
> > Thanks for the review, and sorry for the late reply.
> > Yes, "mediatek,mt8183-ufs0cfg_ao" should be removed.
> > I will remove it in the next version.
> >
> > > > +        reg = <0x16840000 0x1000>;
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <1>;
> > > > +
> > > > +        ufs0cfgao_rst: reset-controller {
> > > > +            compatible = "mediatek,syscon-reset";
> > >
> > > It looks to me like this is just two registers inside ufs0cfg_ao, not
> > > a
> > > separate device. Why don't you just add #reset-cells to the parent
> > > node?
> > >
> > > > +            #reset-cells = <1>;
> > > > +            mediatek,reset-bits =
> > > > +                <0x48  3  0x4c  3  100>,
> > > > +                <0x148 0  0x14c 0  100>,
> > > > +                <0x148 1  0x14c 1  100>,
> > > > +                <0x148 2  0x14c 2  0>;
> > > > +        };
> > >
> > > Why is this in DT? This should be a table in the reset driver.
> > >
> > > regards
> > > Philipp
> >
> > Regarding the child node and reset-bits in DT,
> > We chose the child node approach with 'mediatek,reset-bits' defined
> > in DT to keep the reset line descriptions self-contained and reusable
> > across different SoC variants. MediaTek has many SoCs (mt8183, mt6985,
> > mt6989, ...) where the same UFS subsystem may have different register
> > offsets for reset lines. By describing them in DT, we can support new
> > SoC variants by updating the DT alone, without requiring a new driver
> > patch for every new SoC.
>
>  From what I recall, mediatek ufs is a mess with lots of vendor kernel
> type things slipping into mainline without proper review on the DT
> front.
> Because of that, I at least am going to require that everything is done
> completely (and perhaps excessively) by the book here, including
> introducing complete bindings for syscon regions rather than partial
> bits for components like this one.

Conor, thanks for chiming in.

I would've done that earlier but I'm going through a bit of busy period here, so
thanks x2.

Yes the UFS driver is quite a bit messy.
The latest version of our series that cleans it up is version 9, sent quite a while
ago at this point:

https://patchwork.kernel.org/project/linux-scsi/cover/20260306-mt8196-ufs-v9-0-55b073f7a830@xxxxxxxxxxxxx/

...but anyway, during these months we kept going on with it and we do have a newer
version of this series in our tree, which we're planning to send in a week or two.

That'll start fixing stuff around, at least, in both bindings and code.

> > This approach is also consistent with the existing 'ti,syscon-reset'
> > binding, which uses a similar per-entry table property 'ti,reset-bits'
> > to describe reset lines within a syscon block.
>
> This was done about 10 years ago, I would not consider it a guide for
> what's acceptable today.

I completely agree about that. Honestly, I'm not even sure why MediaTek needs the
resets in the ufscfg0-ao space (because I didn't do extensive research, but that
might be rightful), but this is getting a bit annoying as they keep pushing for
adding something similar to the TI syscon reset at least at every new chip that
comes out (or every 6-8 months if I recall correctly), and we keep doing the same
review over and over.

Please MediaTek, stop trying to add syscon-reset. Please!

I know that you're trying to do that because in your downstream you never stopped
using ti,syscon-reset: if that works better for you in your downstream, that's ok
as it's purely yours but, as a matter of fact, in this form, it's not upstreamable.

If you have to upstream a *pure* reset controller, make a reset controller driver
and put it in the appropriate kernel subsystem - but I also want to remind you
that I know MediaTek SoCs, and I know that up until MT8196 there shouldn't be any
hardware that is purely a reset controller (as in, there's no IP that manages only
resets).

Thanks everyone!
Angelo