Re: [RFC] Adding export-symbols to specification

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



Hi Ayush,

On Wed, 5 Feb 2025 01:53:54 +0530
Ayush Singh <ayush@xxxxxxxxxxxxxxx> wrote:

> On 2/5/25 00:51, Herve Codina wrote:
> 
> > On Tue, 4 Feb 2025 22:54:52 +0530
> > Ayush Singh <ayush@xxxxxxxxxxxxxxx> wrote:
> >  
> >>> You have perfectly summarized the export-symbols goal and the benefit of
> >>> this new feature.
> >>>
> >>> I am waiting for feedback from other people. I hope we will move forward
> >>> on this topic and unblock several users (me included) stuck on this real
> >>> issue.
> >>>
> >>> Thanks a lot!
> >>>
> >>> Best regards,
> >>> Hervé  
> >> While working on a Patch series to the specification itself, I realized
> >> that I was missing some edge cases, so wanted to discuss those:
> >>
> >> # Scope
> >>
> >> Should export-symbols only be used to resolve the properties in parent,
> >> or should all other children (and their decedents) use the
> >> `export-symbols` for resolving phandles and path references?
> >>
> >> For example, should the following work:
> >>
> >>
> >> parent {
> >>
> >>     sibling {
> >>
> >>         led = <&gpio 0 GPIO_ACTIVE_HIGH>
> >>
> >>     };
> >>
> >>
> >>     export-symbols {
> >>
> >>         gpio = <&my_gpio1>;
> >>
> >>     };
> >>
> >> };
> >>
> >>
> >> This would also mean that bottoms up lookup needs to take place for all
> >> `export-symbols` that might be present in path to root, before using top
> >> level `__symbols__` or `/aliases`.  
> > I restricted the use of export-symbols node when an overlay is applied 
> > at a node
> > which contains an export-symbols sub-node.
> >
> > In your example, If you apply an overlay at parent node, your 
> > export-symbols
> > is used but if you apply an overlay at sibling node, your 
> > export-symbols is
> > not used.
> >
> > Of course if your overlay applied at parent node looks like the following:
> >
> > __overlay__ {
> > sibling {
> > prop = <&gpio>;
> > };
> > };
> >
> > &gpio will be resolved using your export-symbols. __overlay__ is applied
> > at parent -> visible in the overlay.
> >
> > If the base device-tree looks like this:
> > parent {
> > sibling {
> > export-symbols {
> > gpio = <&my_gpio1>;
> > };
> > };
> > };
> >
> > The same overlay applied at parent will failed.
> > Indeed, no export-symbols is available at parent node and so the
> > gpio symbols used by the overlay will not be resolved even
> > if an export-symbols exists in the sibling node.
> > To see this export-symbols from the overlay, the overlay has to be applied
> > at sibling node and not parent node.  
> 
> Yes, but adding export-symbols to specification would mean defining the 
> behavior in base devicetree as well.
> 
> 
> ```
> 
> \ {
> 
>      parent {
> 
>          export-symbols {
> 
>              gpio = <&main_gpio0>;
> 
>          };
> 
>      };
> 
> };
> 
> 
> &parent {
> 
>      led = <&gpio 0 GPIO_ACTIVE_HIGH>
> 
> };
> 
> ```
> 

In this example, we can consider that 'led = <&gpio 0 GPIO_ACTIVE_HIGH>' is
applied to the parent node (&parent).

Suppose:
 \ {
      node-one {
	  export-symbols {
		gpio = <&main_gpio0>;
          };
 
	  node-two {
          	export-symbols {
              		gpio = <&main_gpio1>;
          	};
	  };
      };
 };

&node-one {  <--- Applied to node-one
	my-gpio = <&gpio>;   <--- &main_gpio0 from node-one.export-symbols
};

&node-one { <--- Applied to node-one
	node-two {
		my-gpio = <&gpio>;   <--- &main_gpio0 from node-one.export-symbols
	};
};

&node-two { <--- Applied to node-two
	my-gpio = <&gpio>;   <--- &main_gpio1 from node-two.export-symbols
};

Suppose this other base DT
\ {
	node-one {
		export-symbols {
			gpio = <&main_gpio0>;
		};
		my-gpio = <&gpio>;  <--- Not allowed as there is no notion of
				         fragment applied. my-gpio was not
					 "applied" but already present in the
					 node description
	};
 };

Two questions:
- Does it make sense ?
- Is it easily implementable ?

> 
> It might not make sense to add it to devicetree specification if the 
> node does not function in normal devicetree context.
> 
> >>
> >> # Export symbols phandles
> >>
> >> Can export symbols reference each other? For example is the following 
> >> valid:
> >>
> >>
> >> parent {
> >>
> >>     export-symbols {
> >>
> >>         shadow_gpio = <&my_gpio1>;
> >>
> >>         gpio = <&shadow_gpio>;
> >>
> >>     };
> >>
> >> };  
> > For sure, not planned.
> >
> > Also, maybe I missed something but I am not sure you can have a
> > phandle referencing a property.
> >
> > In your example, shadow_gpio is a property, even if we add a label
> > to this property i.e.
> > label_shadow_gpio: shadow_gpio = <&my_gpio1>;
> >
> > How can we set:
> > gpio = <&label_shadow_gpio>;  
> 
> Well, technically, all phandles actually just reference a property since 
> they are resolved using the properties defined in `__symbols__` node or 
> `/aliases`. So it is pretty simple to add the functionality described above.

I think in __symbols__ we have labels not phandles.
At runtime, in the end, a phandles are numbers.

  some-node {
	phandle = <0x12>;
  };

  other-node {
	ref-to-some-node = <0x12>; <--- It references a node which has phandle == 0x12
  };

> 
> > Best regards,
> > Hervé
> >  
> 
> 
> I have been thinking about some alternative approaches as well, and 
> something that sounds nice is `/ephimeral-symbols`. Let me explain what 
> I mean:
> 
> `/ephemeral-symbols` can be a global node whose properties have a single 
> phandle as value. For example:
> 
> 
> / {
>      ephemeral-symbols {
>        connector1 = <&connector1>;
>        gpio = <&my_gpio1>;
>      };
> };
> 
> 
> This node will take precedence over `__symbols__` and will be used for 
> symbol resolution. However, it will never be present in the compiled 
> devicetree blob.
> 
> In case of base devicetree, it can be used to generate symbols for 
> specific nodes:
> 
> 
> / {
>      connector1 {
>          prop = "1";
>      };
> 
>      ephemeral-symbols {
>          connector1 = <&connector1>;
>      };
> };
> 
> 
> Will compile to:
> 
> 
> / {
> 
>      connector1 {
>          prop = "1";
>      };
> 
>      __symbols__ {
>          connector1 = "/connector1";
>      };
> 
> };
> 
> 
> In case of addon board overlay setup, it can be used in a similar way to 
> the __symbols__ proposal by Andrew [0].
> 
> 
> So you have the base devicetree:
> 
> 
> / {
> 
>      connector1 {
>      };
> 
>      connector2 {
>      };
> 
>      ephemeral-symbols {
>          connector1 = <&connector1>;
>      };
> };
> 
> 
> adapter overlay:
> 
> 
> /dts-v1/;
> /plugin/;
> 
> &{/} {
>      ephemeral-symbols {
>          ABC_CONNECTOR = <&connector1>;
>      };
> };
> 
> 
> addon board overlay:
> 
> 
> /dts-v1/;
> /plugin/;
> 
> 
> &ABC_CONNECTOR {
> 
>      prop = "val";
> 
> };
> 
> 
> Which will end up resolving to:
> 
> 
> / {
> 
>      connector1 {
>          prop = "val";
>      };
> 
>      connector2 {
>      };
> 
>      __symbols__ {
>          connector1 = "/connector1";
>          connector2 = "/connector2";
>      };
> 
> };

You need 2 overlays and you need to have both overlay applied in an atomic
way.

adapter-connector1.dtso then addon.dtso

You should avoid this kind of non atomic flow:
  adapter-connector1.dtso
  adapter-connector2.dtso  <--- ABC_CONNECTOR changed
  addon.dtso <--- ABC_CONNECTOR from adapter-connector2.dtso: Ok
  addon.dtso <-- ABC_CONNECTOR is no more from adapter-connector1.dtso

I really wanted to avoid this extra complexity (runtime and maintenance)
involving two overlays.

With epheral-symbols, you need to use ABC_CONNECTOR as a reference and not
a phandle. I mean:
in the overlay:
  &ABC_CONNECTOR {   <--- Use a reference: ok
      some-thing;
  };

  node {
    phandle-to-connector = <&ABC_CONNECTOR>; <--- Use as phandle: ko
  };

To use a phandle, you need to have a phandle = <0x??> property set in the node
the phandle is pointing to. In your base dt:
> / {
> 
>      connector1 {
>      };
> 
>      connector2 {
>      };
> 
>      ephemeral-symbols {
>          connector1 = <&connector1>;
>      };
> };

connector2 will not have its phandle property set unless you
  - set a label and use that label in a phandle reference in your base DT.
or
  - set a label and compile with -@ flag

On my use case, my connector node is not referenced by a phandle in my
base DT, except export-symbols.
I really don't need and don't want to export all possible symbols of my
base DT (I don't use -@ option).

An other rule we try to follow on our side (me and Luca) is:
Overlays are supposed to add or remove descriptions of new or removed
hardwares and so overlays add or remove nodes and not properties in
existing node (which means modify existing hardware).

> 
> 
> It does seem like it avoids the pitfalls of [0] (no global __symbols__ 
> polluting and chaining is fine since we do not need to resize devicetree 
> to resolve path references). But it might have some other flaws I am 
> missing right now.

I don't think we pollute the global __symbols__ with export-symbols
solution :)

Related to implementation details (resizing devicetree), my turn :)

At runtime the implementation constraint is that adding a property in an
already existing node leads to a memory leak and so adding connector2 in
__symbols__ node at runtime is an issue.

Best regards,
Hervé




[Index of Archives]     [Device Tree]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Photos]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]

  Powered by Linux