dt-binding: ipmi: add fallback to npcm845 compatible
Subsystems (3)
Tomer Maimon
maintainerPATCH v3 1/1 dt-binding: ipmi: add fallback to npcm845 compatible
Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.
--- Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +1 -1
@@ -7,7 +7,7 @@ used to perform in-band IPMI communication with their host. Required properties: - compatible : should be one of "nuvoton,npcm750-kcs-bmc" - "nuvoton,npcm845-kcs-bmc" + "nuvoton,npcm845-kcs-bmc", "nuvoton,npcm750-kcs-bmc" - interrupts : interrupt generated by the controller - kcs_chan : The KCS channel number in the controller
Signature · 1 line
2.33.0
Quoted 5 lines · Tomer Maimon
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 <[email protected]>
Best regards,
Krzysztof
Quoted 9 lines · Krzysztof Kozlowski
Quoted 5 lines · Tomer Maimon
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 <[email protected]>
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.
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. 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?
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.
-corey
Quoted 4 lines
Best regards,
Krzysztof
Quoted 11 lines · Corey Minyard
Quoted 9 lines · Krzysztof Kozlowski
Quoted 5 lines · Tomer Maimon
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 <[email protected]>
Quoted 8 lines
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...
Quoted 4 lines
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.
Quoted 6 lines
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?
Quoted 11 lines
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
Quoted 26 lines · Krzysztof Kozlowski
Quoted 11 lines · Corey Minyard
Quoted 9 lines · Krzysztof Kozlowski
Quoted 5 lines · Tomer Maimon
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 <[email protected]>Quoted 8 lines
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...
Quoted 1 line
Quoted 6 lines
Quoted 3 lines
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.
It is true, but lack of specified good examples makes it hard for people like me to know what is right and wrong.
Quoted 11 lines
Quoted 7 lines
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
Everything is always clear when you understand something :). The really hard part about technical documentation is forgetting what you know and approaching it from a newbie's context.
Quoted 6 lines
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 is not the definition of specific that is vague, it is what "specific" applies to. Is it specific to a SOC? Specific to a board? Specific to a particular device implementation? Specific to a rev of the silicon?
I will say that when I read that sentence, it means nothing to me. If it said "DO make compatible properties as specific as possible to the particular hardware implementation of the device" that would have more meaning, but still leaves the reader wondering exactly how to do this.
For instance, should I put board/rev specific compatibles in? Would it be:
"mycompany,myboard-rev1-npcm845-kcs-bmc", "mycompany,myboard-npcm845-kcs-bmc", "nuvoton,npcm845-revb-kcs-bmc", "nuvoton,npcm845-kcs-bmc", "nuvoton,npcm750-kcs-bmc"
That's about as specific as you can get with fallbacks for everything, but it is too specific? How far do you go? There might be wiring differences on specific board, maybe that makes a difference.
That's where good (and identified bad) examples and rationale come in. Tell the user why something is being done and it's easier to understand what to do in different situations. It's not a matter of number of sentences. Just like code, shorter is not always better.
Anyway, I have ranted for too long. Thank you for clearing this up for me.
-corey
Quoted 18 lines
Quoted 5 lines
It would be nice to have something added to "Typical cases and caveats" that says:
Quoted 7 lines
- 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