Re: [PATCH v4 1/5] dt-bindings: watchdog: mt7621-wdt: add phandle to access system controller registers

On 12/02/2023 09:13, Sergio Paracuellos wrote:

On Sat, Feb 11, 2023 at 12:42 PM Krzysztof Kozlowski
[off-list ref] wrote:

quoted

On 11/02/2023 12:01, Sergio Paracuellos wrote:

quoted

On Sat, Feb 11, 2023 at 11:47 AM Arınç ÜNAL [off-list ref] wrote:

quoted

On 11.02.2023 13:41, Sergio Paracuellos wrote:

quoted

On Sat, Feb 11, 2023 at 10:10 AM Arınç ÜNAL [off-list ref] wrote:

quoted

Is this mediatek,sysctl property required after your changes on the
watchdog code?

I don't really understand the question :-) Yes, it is. Since we have
introduced a new phandle in the watchdog node to be able to access the
reset status register through the 'sysc' syscon node.
We need the bindings to be aligned with the mt7621.dtsi file and we
are getting the syscon regmap handler via
'syscon_regmap_lookup_by_phandle()'. See PATCH 5 of the series, Arınç.

I believe you need to put mediatek,sysctl under "required:".

Ah, I understood your question now :-). You meant 'required' property.
I need more coffee, I guess :-). I am not sure if you can add
properties as required after bindings are already mainlined for
compatibility issues. The problem with this SoC is that drivers become
mainlined before the device tree was so if things are properly fixed
now this kind of issues appear.  Let's see Krzysztof and Rob comments
for this.

If your driver fails to probe without mediatek,sysctl, you already made
it required (thus broke the ABI) regardless what dt-binding is saying.
In such case you should update dt-binding to reflect reality.

Now ABI break is different case. Usually you should not break it without
valid reasons (e.g. it was never working before). Your commit msg
suggests that you only improve the code, thus ABI break is not really
justified. In such case - binding is correct, driver should be reworked
to accept DTS without the new property.

Thanks for clarification, Krzysztof. Ok, so if this is the case I need
to add this property required (as Arinc was properly pointing out in
previous mail) since without it the driver is going to fail on probe
(PATCH 5 of the series). I understand the "it was never working
before" argument reason for ABI breaks. What happens if the old driver
code was not ideal and totally dependent on architecture specific
operations when this could be totally avoided and properly make arch
independent agnostic drivers?

It's just an improvement and improvements should be incremental and not
break ABI.

This driver was added in 2016 [0]. There
was not a device tree file in the kernel for this SoC mainlined until
2022 [1]. 

2022 march was almost a year ago, so there were kernel releases with
this ABI.

Also, what about all out of tree DTS? What about other operating
systems, bootloaders, firmwares etc?


Best regards,
Krzysztof