Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node

[Ayush Singh <ayush@beagleboard.org>]

On 8/17/25 13:11, Krzysztof Kozlowski wrote:

> On 18/06/2025 11:54, Ayush Singh wrote:
>> On 6/18/25 15:02, Herve Codina wrote:
>>> Hi Krzysztof,
>>>
>>> On Wed, 4 Jun 2025 20:35:51 +0200
>>> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>
>>> ...
>>>
>>>>> Symbols are exported only when an overlay is applied on the node where the
>>>>> export-symbols node is available. Those symbols are visible only from the
>>>>> overlay applied. Symbols exported thanks to export-symbols are not global
>>>>> to the all device-tree (it is not __symbols__) but local to a node.
>>>>>
>>>>> If an overlay is applied at connector1 node, it can use the 'connector'
>>>>> symbols and thanks to export-symbols, the 'connector' symbol will be
>>>>> resolved to foo_connector.
>>>>>
>>>>> If the overlay is applied at connector2 node, the 'connector' symbol is then
>>>>> resolved to bar_connector.
>>>> OK, this explains a lot. Unless I missed it, would be nice to include it
>>>> in binding description.
>>> Sure, I will add something in the next iteration.
>>>
>>> ...
>>>
>>>>>>> +patternProperties:
>>>>>>> +  "^[a-zA-Z_]?[a-zA-Z0-9_]*$":
>>>>>> This messes up with coding style which I would prefer keep intact.
>>>>>> Basically these properties will be using label style.
>>>>> Yes, those properties remap phandles.
>>>>>
>>>>> Their names are the name of the label used from the overlay and their
>>>>> values are the phandle mapped.
>>>>>
>>>>> You already have this kind properties using label style in __symbols__,
>>>>> __fixups__, __local_fixups__ nodes.
>>>> I have them in DTB, but I don't have these in DTS. The exported-symbols
>>>> would be in the DTS and that is what coding style is about.
>>>>
>>> I think export-symbols has to be in DTS.
>>> Maybe it could be described in an other way in order to avoid the coding style
>>> issue you reported.
>>>
>>> Hardware:
>>>     i2c0 from SoC --------- connector 1, I2C A signals
>>>     i2c1 from SoC --------- connector 1, I2C B signals
>>>
>>>     connector1 {
>>>         export-symbols {
>>> 	  i2c_a = <&i2c0>;
>>> 	  i2c_b = <&i2c1>;
>>>         };
>>>     };
>>>
>>> In order to avoid the coding style issue, this could be replace
>>> with:
>>>    connector1 {
>>>         export-symbols {
>>> 	  symbol-names = "i2c_a", "i2c_b";
>>> 	  symbols = <&i2c0>, <&i2c1>;
>>>         };
>>>     };
>>>
>>> Krzysztof, Rob, do you think this could be accepted ?
>>>
>>> Ayush, David, do you thing this could be easily implemented in fdtoverlay ?
>>>
>>> Best regards,
>>> Hervé
>>>
>> Well, it is possible.
>>
>> However, on connectors like pb2 header, there will be 50-100 export
>> symbols. So it will start becoming difficult to maintain.
>
> And the first syntax solves this how? I don't see the practical difference.


Well, I was more worried about matching which phandle belongs to which 
symbol easily. Let us assume that 2 symbols will be in each line (after 
accounting for the indention and 80 char limit) and we have 70 symbols, 
so 35 lines. To check which phandle belongs to the 2nd symbol on line 
25th line of  symbol-names, well, you would at the best case need to 
have something like relative line numbers in your editor. Then you know 
that the 35th line from the current one is where you need to look.

In the current syntax, the symbol name and phandle are on the same line. 
So well, easy to see which symbols refers to which phandle.

>> Additionally, the further away we move from __symbols__ style, the more
>> difficult the implementation will become since we can currently very
>> easily piggy-back on __symbols__ resolution implementation.
>
> I care more how I read DTS, so complexity in dtc is less important to me
> than consistent DTS style.
>
> Best regards,
> Krzysztof