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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d10e541a-cdfd-08af-6a2c-eb7edcdb486a@linaro.org>
Date:   Mon, 13 Feb 2023 09:38:05 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Sergio Paracuellos <sergio.paracuellos@...il.com>
Cc:     Arınç ÜNAL <arinc.unal@...nc9.com>,
        linux-watchdog@...r.kernel.org, wim@...ux-watchdog.org,
        linux@...ck-us.net, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, matthias.bgg@...il.com,
        tsbogend@...ha.franken.de, p.zabel@...gutronix.de,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        linux-mips@...r.kernel.org
Subject: 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
> <krzysztof.kozlowski@...aro.org> wrote:
>>
>> On 11/02/2023 12:01, Sergio Paracuellos wrote:
>>> On Sat, Feb 11, 2023 at 11:47 AM Arınç ÜNAL <arinc.unal@...nc9.com> wrote:
>>>>
>>>> On 11.02.2023 13:41, Sergio Paracuellos wrote:
>>>>> On Sat, Feb 11, 2023 at 10:10 AM Arınç ÜNAL <arinc.unal@...nc9.com> wrote:
>>>>>>
>>>>>> 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