Re: [PATCH v3] dt-binding: ipmi: add fallback to npcm845 compatible

[Krzysztof Kozlowski]

On 08/08/2022 15:26, Corey Minyard wrote:
> On Mon, Aug 08, 2022 at 11:11:16AM +0300, Krzysztof Kozlowski wrote:
>> On 08/08/2022 09:54, Tomer Maimon wrote:
>>> Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible
>>> string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.
>>>
>>> Fixes: 84261749e58a ("dt-bindings: ipmi: Add npcm845 compatible")
>>> Signed-off-by: Tomer Maimon <tmaimon77@xxxxxxxxx>
>>
>>
>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> 
> Ok, I think I understand how this is supposed to work.  It's not
> altogether clear from the device tree documentation.  It says in
> Documentation/devicetree/bindings/writing-bindings.rst:
> 
> - DO make 'compatible' properties specific. DON'T use wildcards in compatible
>   strings. DO use fallback compatibles when devices are the same as or a subset
>   of prior implementations. DO add new compatibles in case there are new
>   features or bugs.

This documentation is short, so it explains what should be done, not
exactly why it should be done. If we wanted "why" this would not be set
of 4 sentences but twice more...

> 
> AFAICT, there are no new features or bugs, just a new SOC with the same
> device.  In general usage I have seen, you would just use the same
> compatible.  

Most of the usages are like shown here. There are several other cases.
It's the same with poor or good code - you will always find both patterns.

> However, if I understand this, that last sentence should say:
> 
>   DO add new compatibles in case there is a new version of hardware with
>   the possibility of new features and/or bugs.
> 
> Also, the term "specific" is, ironically, vague.  Specific to what?

To me it is rather clear. Specific as in first meanings of the word (1,
3, 4 and 5):
https://en.wiktionary.org/wiki/specific

nuvoton,npcm7xx-kcs-bmc would not be definite (allows more meanings),
unique (in terms of devices it expresses), distinctive (as two different
devices use the same) or serving to identify one thing (again - two SoCs).

What other meaning do you think of?

> 
> It would be nice to have something added to "Typical cases and caveats"
> that says:
> 
> - If you are writing a binding for a new device that is the same as, or
>   a superset of another existing device, add a new specific compatible
>   for the new device followed by a compatible for the existing device.
>   That way, if the device has new bugs or new specific features are
>   added, you can add workarounds without modifying the device tree.
> 
> Anyway, I have added this to my tree with your ack.

Fantastic, thanks!


Best regards,
Krzysztof