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

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

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@collabora.com/

...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