Hi Ayush, On Wed, 8 Jan 2025 13:06:03 +0530 Ayush Singh <ayush@xxxxxxxxxxxxxxx> wrote: > On 10/12/24 16:25, Herve Codina wrote: > > Hi Ayush, > > > > On Tue, 10 Dec 2024 15:26:44 +0530 > > Ayush Singh <ayush@xxxxxxxxxxxxxxx> wrote: > > > >> On 10/12/24 15:11, Herve Codina wrote: > >>> Hi Ayush, > >>> > >>> On Tue, 10 Dec 2024 14:52:22 +0530 > >>> Ayush Singh <ayush@xxxxxxxxxxxxxxx> wrote: > >>> > >>> ... > >>>> > >>>> What is the reason for not using symbols directly as described here [3]? > >>>> > >>>> I do like this approach since it does not pollute the global symbols. > >>>> Just want to know if there are any other reasons for it. > >>>> > >>> > >>> Modifying the __symbols__ node at runtime (adding / removing properties in > >>> it) exposes memory leaks if __symbols__ already exist in the live DT. > >>> This __symbols__ node exist if the dtb was compiled with '-@' or if you > >>> chain the overlay (i.e. __symbols__ node created by the first overlay). > >> > >> Yeah, that is a problem, specially in a setup which might involve > >> hot-plugging. > >> > >>> > >>> I think also that some conflicts can appears. What happens if you want to > >>> add a new label but this label is already present for some other purpose? > >> > >> I do not think that actually is a problem. As described in the original > >> patch [0], the symbol and connector overlay is supposed to be applied as > >> a group (overwriting any conflicting symbols in the process). > >> > >> The reason why this is not a problem is that `__symbols__` are only used > >> to resolve the phandles (overlays do not support path references yet), > >> but do not really have a purpose in the livetree (at least far as I > >> know, but I can be wrong). > >> > >>> > >>> Best regards, > >>> Hervé > >> > >> [0]: https://lore.kernel.org/lkml/20240702164403.29067-1-afd@xxxxxx/ > > > > > > Also, in your first overlay (adding symbols in __sympbols__ node), you have > > something like: > > GROVE_PIN1_MUX_I2C_SCL = "/bus@f0000/pinctrl@f4000/grove-i2c-pins"; > > > > If I understood correctly, other overlays will have GROVE_PIN1_MUX_I2C_SCL > > as unresolved symbols and will use GROVE_PIN1_MUX_I2C_SCL to reference the > > grove-i2c-pins node. > > This unresolved symbol from the overlay is resolved thanks to the __symbols__ > > table where you added GROVE_PIN1_MUX_I2C_SCL (first overlay operation). > > > > In order to work, you need to have a phandle property set in the > > grove-i2c-pins node. > > > > This is done by dtc when you compile the dtb containing the grove-i2c-pins > > node (i.e. k3-am625-beagleplay.dts) > > > > The phandle property will be set only if: > > - a label for grove-i2c-pins already exist and -@ option is used > > or > > - a label for grove-i2c-pins already exist and it is referenced as a phandle > > in the dts (k3-am625-beagleplay.dts). > > > > Otherwise, dtc will not create the phandle property and without this > > property, the symbol resolution will not be correct. > > > > Best regards, > > Hervé > > > > Hello Hervé > > Thanks for the clarification. things have changed a bit since the last > message and it seems like trying to add path reference support to > overlays is not the best way forward [0]. So I would love to help move > this approach forward. > > I do have a question regarding this approach, so here I go: > > Can the `export-symbols` node be added to devicetree spec and be > resolved by the devicetree compiler (and fdtoverlay) instead of being > runtime resolution. Of course, a solution with fdtoverlay is welcome but it should not fully replace the runtime resolution. In our case, we need runtime resolution because the overlay is loaded by a driver. Both resolutions (fdtoverlay and runtime) should work. > > To get some context, I would like to share the addon-board overlays > between ZephyrRTOS and Linux kernel. I would be happy to try adding > support to dtc compiler for it. I am also tagging David Gibson (dtc > maintainer) in this discussion since he also had some ideas regarding > the feasibility and pitfalls of adding it to devicetree compiler (and spec). > > > [0]: > https://lore.kernel.org/devicetree-compiler/6b2dba90-3c52-4933-88f3-b47f96dc7710@xxxxxxxxxxxxxxx/T/#m900b5ca13cfc28396d4d46d9c3130a7070fa8c90 > > Best regards, > Ayush Singh > Thanks for your help proposal! Best regards, Hervé
