Re: [PATCH] docs: dt: writing-bindings: Express better expectation of "specific"

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 02/07/2025 18:38, Conor Dooley wrote:
> On Wed, Jul 02, 2025 at 06:17:01PM +0200, Krzysztof Kozlowski wrote:
>> Devicetree bindings are supposed to be specific in terms of compatibles
>> and other properties.  Short "specific" has many implications, so extend
>> the description to cover them: no family names, avoid generic SoC
>> fallbacks, avoid versioned compatibles, avoid properties implied by
>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> IDK if it is intentional, but while you mention this here it doesn't
> appear in the text below. Probably as simple as "DON'T use versioned
> compatibles, unless documenting a HDL IP core"?

Indeed, it does not appear in my patch. I added it, but then I found it
is already in other point as you noticed. I'll rewrite commit msg.

> 
>> compatible.
>>
>> Also document desired lack of ABI impact and acceptable solution if such
>> needs arises: clearly marking it in commit msg.
>>
>> All above follows established Devicetree bindings maintainers review
>> practice, so no new rules are introduced here.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
>> ---
>>
>> I have vast (~10-page) document describing DT bindings and DTS rules,
>> based on Rob's and other people's reviews (with references...). Let me
>> try to funnel it here gradually. Why gradually? The intention of
>> writing-bindings document is to be concise, so rephrasing my 10 pages
>> covering each little detail into generic, concise rule is not that easy,
>> especially for non-native speaker.
>>
>> Optionally I could also post my 10-page guide somewhere, but then it
>> would be one more document to ignore. I think we have enough of such.
>> Ah, and I would have one less topic for conference. :)
>> ---
>>  .../devicetree/bindings/writing-bindings.rst  | 25 +++++++++++++++----
>>  1 file changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/writing-bindings.rst b/Documentation/devicetree/bindings/writing-bindings.rst
>> index 1ad081de2dd0..dc0e4b63984c 100644
>> --- a/Documentation/devicetree/bindings/writing-bindings.rst
>> +++ b/Documentation/devicetree/bindings/writing-bindings.rst
>> @@ -39,10 +39,15 @@ Overall design
>>  Properties
>>  ==========
>>  
>> -- 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.
>> +- DO make 'compatible' properties specific. DON'T use wildcards or
>> +  device-family names in compatible strings.
> 
>> DO use fallback compatibles when
>> +  devices are the same as or a subset of prior implementations.
> 
> Isn't this wrong? If you're a subset of a prior implementation, falling
> back to the prior implementation's compatible would enable features that
> are not present in the new device. That's not something you introduced,
> it's the existing wording, but I don't think that's correct. It would have
> to be a /superset/ of a prior implementation for this to be correct, no?

Indeed, this should be superset. I can fix it in a separate.

> 
>> DO add new
>> +  compatibles in case there are new features or bugs.
> 
> I think it might be best to break each "DO" or "DON'T" into a bullet
> point of its own. At least it'd make it more clear what the specific dos
> and don'ts are.

Ack.

> 
>> +
>> +   - Use SoC-specific compatible for all SoC devices, followed by a fallback if
> 
> "Use a".

Ack

> 
>> +     appropriate.
>> +
>> +   - Specific SoC compatible is also preferred for the fallbacks.
> 
> I hate to nitpick language to a non-native speaker, but when you have a
> plural "fallbacks", it would be "Specific SoC compatibles are also
> preferred". Probably also "SoC-specific" too, to match the wording used
> in the bullet prior?

AcK

> 
>>  
>>  - DO use a vendor prefix on device-specific property names. Consider if
>>    properties could be common among devices of the same class. Check other
>> @@ -51,12 +56,22 @@ Properties
>>  - DON'T redefine common properties. Just reference the definition and define
>>    constraints specific to the device.
>>  
>> +- DON'T add properties to avoid a specific compatible.  DON'T add properties if
>> +  they are implied by (deducible from) the compatible.
>> +
>>  - DO use common property unit suffixes for properties with scientific units.
>>    Recommended suffixes are listed at
>>    https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
>>  
>>  - DO define properties in terms of constraints. How many entries? What are
>> -  possible values? What is the order?
>> +  possible values? What is the order? All these constraints represent the ABI
>> +  as well.
>> +
>> +- DON't change the ABI, but if ever needed to change, then DO explicitly
> 
> "DON'T", not "DON't". But this immediately contradicts itself, so maybe
> a different wording could be used here. Maybe "DON'T make changes that
> break the ABI without explicit and detailed rationale for why the
> changes have to be made and their impact."?

Well, the goal is that ABI change is strongly discouraged but still
allowed. Your wording suggest that ABI break is okay with explicit and
detailed rationale, but I want to express allowing ABI break is exception.

> 
>> +  document that in the commit msg with rationale WHY ABI has to be broken and
>> +  what is the impact. ABI impact is beyond Linux kernel, because it covers also
> 
> I think this should be "ABI impact /goes/ beyond /the/ Linux kernel,
> because it /also/ covers other open-source upstream projects.".

Ack


Best regards,
Krzysztof