Re: [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type

[Jerome Brunet]

On Wed 22 Nov 2023 at 16:46, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote:


>>>>>
>>>>> Again, where the "v2" is defined? Where is any document explaining the
>>>>> mapping between version blocks and SoC parts? Why do you list here only
>>>>> major version? Blocks almost always have also minor (e.g. v2.0).
>>>>
>>>> Again, v2 does has nothing to do with the HW. Never wrote it was.
>>>> The HW remains the same.
>>>
>>> Don't add compatibles which are not related to HW, but represent
>>> software versioning. Software does not matter for the bindings.
>> 
>> What I did I explicitly what is recommended in Grant's presentation from
>> 2013. 10y old, but I assume slide 10 "Making an incompatible update" is
>> still valid.
>> 
>> https://elinux.org/images/1/1e/DT_Binding_Process_glikely_ksummit_2013_10_28.pdf
>> 
>> Breaking the ABI of the old compatible would break all boards which use
>> u-boot DT and pass it to the kernel, because the meaning of the clock
>> property would change.
>
> You broke U-Boot now as well - it will get your new DTS from the kernel
> and stop working.

U-boot will continue to match the old compatible and work properly.
When the dts using the new compatible lands in u-boot, it won't
match until proper driver support is added. It is a lot better than
breaking the ABI, which would have silently broke u-boot.

I don't really see a way around that.

If you have better way to fix a bad interface, feel free to share it.

>
>> 
>> Doing things has suggested in this slide, and this patch, allows every
>> device to continue to work properly, whether the DT given is the one
>> shipped with u-boot (using the old compatible for now) or the kernel.
>
> OK, that explains the reasons. I read your commit msg and nothing like
> this was mentioned there. What's more, you did not deprecate the old
> binding, thus the confusion - it looked like you add entirely new
> hardware (although you put "deprecated" but in some unrelated place, not
> next to the compatibles).

The old interface being obsoleted by the new one is mentionned in the
commit description, the comments in the bindings and the bindings itself.
Thanks a lot for pointing out the placement mistake. I'll fix it.

The commit description says:
* What the patch does
* Why it does it:
  * Why the old bindings is bad/broken
  * How the new ones fixes the problem
* Why a single compatible properly describes, IMO, all the related HW.

This describes the entirety of what the change does.
That seemed clear enough for Rob. If that is not enough for you and you
would like it reworded, could please provide a few suggestions ?

>
> Anyway, the main point of Neil was that you started using generic
> compatible for all SoCs, which is wrong as well. I guess this was the
> original discussion.

The whole reason for this change is to properly describe the HW, which
is the 100% same on all the SoCs, or SoC families, concerned. The only
reason there was a lot of old compatibles is because it was used to match
data in the driver (this is clearly wrong). This data would now be
passed through DT.

I have been clear about this in the change description.

So why is it wrong to have single compatible for a type of device that
is 100% the same HW ?

It is lot a easier to apply a rule correctly when the intent is clear.

>
> Best regards,
> Krzysztof