On 12/02/2023 09:13, Sergio Paracuellos wrote: > On Sat, Feb 11, 2023 at 12:42 PM Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> >> On 11/02/2023 12:01, Sergio Paracuellos wrote: >>> On Sat, Feb 11, 2023 at 11:47 AM Arınç ÜNAL <arinc.unal@xxxxxxxxxx> wrote: >>>> >>>> On 11.02.2023 13:41, Sergio Paracuellos wrote: >>>>> On Sat, Feb 11, 2023 at 10:10 AM Arınç ÜNAL <arinc.unal@xxxxxxxxxx> 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
