[PATCH 0/7] of: overlay: Add support for export-symbols node feature

[PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Herve Codina]

Hi,

At Linux Plumbers Conference 2024, we (me and Luca Ceresolli) talked
about issues we have with runtime hotplug on non-discoverable busses
with device tree overlays [1].

On our system, a base board has a connector and addon boards can be
connected to this connector. Both boards are described using device
tree. The base board is described by a base device tree and addon boards
are describe by overlays device tree. More details can be found at [2].

This kind of use case can be found also on:
  - Grove Sunlight Sensor [3]
  - mikroBUS [4]

One of the issue we were facing on was referencing resources available
on the base board device tree from the addon overlay device tree.

Using a nexus node [5] helps decoupling resources and avoid the
knowledge of the full base board from the overlay. Indeed, with nexus
node, the overlay need to know only about the nexus node itself.

For instance, suppose a connector where a GPIO is connected at PinA. On
the base board this GPIO is connected to the GPIO 12 of the SoC GPIO
controller.

The base board can describe this GPIO using a nexus node:
    soc_gpio: gpio-controller {
      #gpio-cells = <2>;
    };

    connector1: connector1 {
        /*
         * Nexus node for the GPIO available on the connector.
         * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
         * controller
         */
        #gpio-cells = <2>;
        gpio-map = <0 0 &soc_gpio 12 0>;
        gpio-map-mask = <0xf 0x0>;
        gpio-map-pass-thru = <0x0 0xf>;
    };

The connector pin A GPIO can be referenced using:
  <&connector1 0 GPIO_ACTIVE_HIGH>

This implies that the overlay needs to know about exact label that
references the connector. This label can be different on a different
board and so applying the overlay could failed even if it is used to
describe the exact same addon board. Further more, a given base board
can have several connectors where the exact same addon board can be
connected. In that case, the same overlay cannot be used on both
connector. Indeed, the connector labels have to be different.

The export-symbols node introduced by this current series solves this
issue.

The idea of export-symbols is to have something similar to the global
__symbols__ node but local to a specific node. Symbols listed in this
export-symbols are local and visible only when an overlay is applied on
a node having an export-symbols subnode.

Using export-symbols, our example becomes:
    soc_gpio: gpio-controller {
      #gpio-cells = <2>;
    };

    connector1: connector1 {
        /*
         * Nexus node for the GPIO available on the connector.
         * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
         * controller
         */
        #gpio-cells = <2>;
        gpio-map = <0 0 &soc_gpio 12 0>;
        gpio-map-mask = <0xf 0x0>;
        gpio-map-pass-thru = <0x0 0xf>;

        export-symbols {
          connector = <&connector1>;
        };
    };

With that export-symbols node, an overlay applied on connector1 node can
have the symbol named 'connector' resolved to connector1. Indeed, the
export-symbols node available at connector1 node is used when the
overlay is applied. If the overlay has an unresolved 'connector' symbol,
it will be resolved to connector1 thanks to export-symbols.

Our overlay using the nexus node can contains:
   node {
      foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
   };
It used the GPIO 0 from the connector it is applied on.

A board with two connectors can be described with:
    connector1: connector1 {
        ...
        export-symbols {
          connector = <&connector1>;
        };
    };

    connector2: connector2 {
        ...
        export-symbols {
          connector = <&connector2>;
        };
    };

In that case, the same overlay with unresolved 'connector' symbol can be
applied on both connectors and the correct symbol resolution (connector1
or connector2) will be done.

This current series add support for the export-symbols node feature:
  - Patch 1 describes the export-symbols binding
  - Patches 2 to 6 prepare and add the support for the export-symbols
    feature
  - Patch 7 adds an unittest for the export-symbols feature

Best regards,
Hervé

[1] https://lpc.events/event/18/contributions/1696/
[2] https://lore.kernel.org/lkml/20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com/
[3] https://lore.kernel.org/lkml/20240702164403.29067-1-afd@ti.com/
[4] https://lore.kernel.org/lkml/20240627-mikrobus-scratch-spi-v5-0-9e6c148bf5f0@beagleboard.org/
[5] https://github.com/devicetree-org/devicetree-specification/blob/v0.4/source/chapter2-devicetree-basics.rst#nexus-nodes-and-specifier-mapping

Herve Codina (7):
  dt-bindings: Add support for export-symbols node
  of: resolver: Introduce get_phandle_from_symbols_node()
  of: resolver: Add export_symbols in of_resolve_phandles() parameters
  of: resolver: Add support for the export symbols node
  of: overlay: Add export_symbols_name in of_overlay_fdt_apply()
    parameters
  of: overlay: Add support for the export symbols node
  of: unittest: Add tests for export symbols

 .../devicetree/bindings/export-symbols.yaml   | 43 ++++++++++
 drivers/misc/lan966x_pci.c                    |  3 +-
 drivers/of/of_kunit_helpers.c                 |  2 +-
 drivers/of/of_private.h                       |  2 +-
 drivers/of/overlay.c                          | 30 ++++++-
 drivers/of/resolver.c                         | 80 ++++++++++++++-----
 drivers/of/unittest-data/Makefile             |  5 ++
 .../unittest-data/overlay_export_symbols.dtso | 15 ++++
 .../of/unittest-data/testcases_common.dtsi    |  1 +
 .../unittest-data/tests-export-symbols.dtsi   | 30 +++++++
 drivers/of/unittest.c                         | 76 ++++++++++++++++--
 include/linux/of.h                            |  6 +-
 12 files changed, 259 insertions(+), 34 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/export-symbols.yaml
 create mode 100644 drivers/of/unittest-data/overlay_export_symbols.dtso
 create mode 100644 drivers/of/unittest-data/tests-export-symbols.dtsi

-- 
2.47.0

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Andrew Davis]

On 12/9/24 9:18 AM, Herve Codina wrote:
> Hi,
> 
> At Linux Plumbers Conference 2024, we (me and Luca Ceresolli) talked
> about issues we have with runtime hotplug on non-discoverable busses
> with device tree overlays [1].
> 
> On our system, a base board has a connector and addon boards can be
> connected to this connector. Both boards are described using device
> tree. The base board is described by a base device tree and addon boards
> are describe by overlays device tree. More details can be found at [2].
> 
> This kind of use case can be found also on:
>    - Grove Sunlight Sensor [3]
>    - mikroBUS [4]
> 
> One of the issue we were facing on was referencing resources available
> on the base board device tree from the addon overlay device tree.
> 
> Using a nexus node [5] helps decoupling resources and avoid the
> knowledge of the full base board from the overlay. Indeed, with nexus
> node, the overlay need to know only about the nexus node itself.
> 
> For instance, suppose a connector where a GPIO is connected at PinA. On
> the base board this GPIO is connected to the GPIO 12 of the SoC GPIO
> controller.
> 
> The base board can describe this GPIO using a nexus node:
>      soc_gpio: gpio-controller {
>        #gpio-cells = <2>;
>      };
> 
>      connector1: connector1 {
>          /*
>           * Nexus node for the GPIO available on the connector.
>           * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
>           * controller
>           */
>          #gpio-cells = <2>;
>          gpio-map = <0 0 &soc_gpio 12 0>;
>          gpio-map-mask = <0xf 0x0>;
>          gpio-map-pass-thru = <0x0 0xf>;
>      };
> 
> The connector pin A GPIO can be referenced using:
>    <&connector1 0 GPIO_ACTIVE_HIGH>
> 
> This implies that the overlay needs to know about exact label that
> references the connector. This label can be different on a different
> board and so applying the overlay could failed even if it is used to
> describe the exact same addon board. Further more, a given base board
> can have several connectors where the exact same addon board can be
> connected. In that case, the same overlay cannot be used on both
> connector. Indeed, the connector labels have to be different.
> 
> The export-symbols node introduced by this current series solves this
> issue.
> 
> The idea of export-symbols is to have something similar to the global
> __symbols__ node but local to a specific node. Symbols listed in this
> export-symbols are local and visible only when an overlay is applied on
> a node having an export-symbols subnode.
> 
> Using export-symbols, our example becomes:
>      soc_gpio: gpio-controller {
>        #gpio-cells = <2>;
>      };
> 
>      connector1: connector1 {
>          /*
>           * Nexus node for the GPIO available on the connector.
>           * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
>           * controller
>           */
>          #gpio-cells = <2>;
>          gpio-map = <0 0 &soc_gpio 12 0>;
>          gpio-map-mask = <0xf 0x0>;
>          gpio-map-pass-thru = <0x0 0xf>;
> 
>          export-symbols {
>            connector = <&connector1>;
>          };
>      };
> 
> With that export-symbols node, an overlay applied on connector1 node can
> have the symbol named 'connector' resolved to connector1. Indeed, the
> export-symbols node available at connector1 node is used when the
> overlay is applied. If the overlay has an unresolved 'connector' symbol,
> it will be resolved to connector1 thanks to export-symbols.
> 
> Our overlay using the nexus node can contains:
>     node {
>        foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
>     };
> It used the GPIO 0 from the connector it is applied on.
> 
> A board with two connectors can be described with:
>      connector1: connector1 {
>          ...
>          export-symbols {
>            connector = <&connector1>;
>          };
>      };
> 
>      connector2: connector2 {
>          ...
>          export-symbols {
>            connector = <&connector2>;
>          };
>      };
> 
> In that case, the same overlay with unresolved 'connector' symbol can be
> applied on both connectors and the correct symbol resolution (connector1
> or connector2) will be done.
> 

I might be missing something, but how is the correct connector (connector1
or connector2) selected? Let's say I connect my addon board to connector2,
then I apply the addon board's overlay to the base DTB. What connector
just got referenced?

Andrew

> This current series add support for the export-symbols node feature:
>    - Patch 1 describes the export-symbols binding
>    - Patches 2 to 6 prepare and add the support for the export-symbols
>      feature
>    - Patch 7 adds an unittest for the export-symbols feature
> 
> Best regards,
> Hervé
> 
> [1] https://lpc.events/event/18/contributions/1696/
> [2] https://lore.kernel.org/lkml/20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com/
> [3] https://lore.kernel.org/lkml/20240702164403.29067-1-afd@ti.com/
> [4] https://lore.kernel.org/lkml/20240627-mikrobus-scratch-spi-v5-0-9e6c148bf5f0@beagleboard.org/
> [5] https://github.com/devicetree-org/devicetree-specification/blob/v0.4/source/chapter2-devicetree-basics.rst#nexus-nodes-and-specifier-mapping
> 
> Herve Codina (7):
>    dt-bindings: Add support for export-symbols node
>    of: resolver: Introduce get_phandle_from_symbols_node()
>    of: resolver: Add export_symbols in of_resolve_phandles() parameters
>    of: resolver: Add support for the export symbols node
>    of: overlay: Add export_symbols_name in of_overlay_fdt_apply()
>      parameters
>    of: overlay: Add support for the export symbols node
>    of: unittest: Add tests for export symbols
> 
>   .../devicetree/bindings/export-symbols.yaml   | 43 ++++++++++
>   drivers/misc/lan966x_pci.c                    |  3 +-
>   drivers/of/of_kunit_helpers.c                 |  2 +-
>   drivers/of/of_private.h                       |  2 +-
>   drivers/of/overlay.c                          | 30 ++++++-
>   drivers/of/resolver.c                         | 80 ++++++++++++++-----
>   drivers/of/unittest-data/Makefile             |  5 ++
>   .../unittest-data/overlay_export_symbols.dtso | 15 ++++
>   .../of/unittest-data/testcases_common.dtsi    |  1 +
>   .../unittest-data/tests-export-symbols.dtsi   | 30 +++++++
>   drivers/of/unittest.c                         | 76 ++++++++++++++++--
>   include/linux/of.h                            |  6 +-
>   12 files changed, 259 insertions(+), 34 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/export-symbols.yaml
>   create mode 100644 drivers/of/unittest-data/overlay_export_symbols.dtso
>   create mode 100644 drivers/of/unittest-data/tests-export-symbols.dtsi
> 

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Herve Codina]

On Mon, 9 Dec 2024 10:47:50 -0600
Andrew Davis <afd@ti.com> wrote:

> On 12/9/24 9:18 AM, Herve Codina wrote:
> > Hi,
> > 
> > At Linux Plumbers Conference 2024, we (me and Luca Ceresolli) talked
> > about issues we have with runtime hotplug on non-discoverable busses
> > with device tree overlays [1].
> > 
> > On our system, a base board has a connector and addon boards can be
> > connected to this connector. Both boards are described using device
> > tree. The base board is described by a base device tree and addon boards
> > are describe by overlays device tree. More details can be found at [2].
> > 
> > This kind of use case can be found also on:
> >    - Grove Sunlight Sensor [3]
> >    - mikroBUS [4]
> > 
> > One of the issue we were facing on was referencing resources available
> > on the base board device tree from the addon overlay device tree.
> > 
> > Using a nexus node [5] helps decoupling resources and avoid the
> > knowledge of the full base board from the overlay. Indeed, with nexus
> > node, the overlay need to know only about the nexus node itself.
> > 
> > For instance, suppose a connector where a GPIO is connected at PinA. On
> > the base board this GPIO is connected to the GPIO 12 of the SoC GPIO
> > controller.
> > 
> > The base board can describe this GPIO using a nexus node:
> >      soc_gpio: gpio-controller {
> >        #gpio-cells = <2>;
> >      };
> > 
> >      connector1: connector1 {
> >          /*
> >           * Nexus node for the GPIO available on the connector.
> >           * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
> >           * controller
> >           */
> >          #gpio-cells = <2>;
> >          gpio-map = <0 0 &soc_gpio 12 0>;
> >          gpio-map-mask = <0xf 0x0>;
> >          gpio-map-pass-thru = <0x0 0xf>;
> >      };
> > 
> > The connector pin A GPIO can be referenced using:
> >    <&connector1 0 GPIO_ACTIVE_HIGH>
> > 
> > This implies that the overlay needs to know about exact label that
> > references the connector. This label can be different on a different
> > board and so applying the overlay could failed even if it is used to
> > describe the exact same addon board. Further more, a given base board
> > can have several connectors where the exact same addon board can be
> > connected. In that case, the same overlay cannot be used on both
> > connector. Indeed, the connector labels have to be different.
> > 
> > The export-symbols node introduced by this current series solves this
> > issue.
> > 
> > The idea of export-symbols is to have something similar to the global
> > __symbols__ node but local to a specific node. Symbols listed in this
> > export-symbols are local and visible only when an overlay is applied on
> > a node having an export-symbols subnode.
> > 
> > Using export-symbols, our example becomes:
> >      soc_gpio: gpio-controller {
> >        #gpio-cells = <2>;
> >      };
> > 
> >      connector1: connector1 {
> >          /*
> >           * Nexus node for the GPIO available on the connector.
> >           * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
> >           * controller
> >           */
> >          #gpio-cells = <2>;
> >          gpio-map = <0 0 &soc_gpio 12 0>;
> >          gpio-map-mask = <0xf 0x0>;
> >          gpio-map-pass-thru = <0x0 0xf>;
> > 
> >          export-symbols {
> >            connector = <&connector1>;
> >          };
> >      };
> > 
> > With that export-symbols node, an overlay applied on connector1 node can
> > have the symbol named 'connector' resolved to connector1. Indeed, the
> > export-symbols node available at connector1 node is used when the
> > overlay is applied. If the overlay has an unresolved 'connector' symbol,
> > it will be resolved to connector1 thanks to export-symbols.
> > 
> > Our overlay using the nexus node can contains:
> >     node {
> >        foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
> >     };
> > It used the GPIO 0 from the connector it is applied on.
> > 
> > A board with two connectors can be described with:
> >      connector1: connector1 {
> >          ...
> >          export-symbols {
> >            connector = <&connector1>;
> >          };
> >      };
> > 
> >      connector2: connector2 {
> >          ...
> >          export-symbols {
> >            connector = <&connector2>;
> >          };
> >      };
> > 
> > In that case, the same overlay with unresolved 'connector' symbol can be
> > applied on both connectors and the correct symbol resolution (connector1
> > or connector2) will be done.
> >   
> 
> I might be missing something, but how is the correct connector (connector1
> or connector2) selected? Let's say I connect my addon board to connector2,
> then I apply the addon board's overlay to the base DTB. What connector
> just got referenced?
> 

A driver for the connector is needed.
The driver applies the overlay using of_overlay_fdt_apply().
The node the overlay has to be applied to is passed by the driver to
of_overlay_fdt_apply().

Even if obsolete because I added one more parameter (export_symbols_name)
in of_overlay_fdt_apply() in this current series, you can have a look at the
following patch to see the connector driver:
  https://lore.kernel.org/lkml/20240917-hotplug-drm-bridge-v4-8-bc4dfee61be6@bootlin.com/

Best regards,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Andrew Davis]

On 12/9/24 11:03 AM, Herve Codina wrote:
> On Mon, 9 Dec 2024 10:47:50 -0600
> Andrew Davis <afd@ti.com> wrote:
> 
>> On 12/9/24 9:18 AM, Herve Codina wrote:
>>> Hi,
>>>
>>> At Linux Plumbers Conference 2024, we (me and Luca Ceresolli) talked
>>> about issues we have with runtime hotplug on non-discoverable busses
>>> with device tree overlays [1].
>>>
>>> On our system, a base board has a connector and addon boards can be
>>> connected to this connector. Both boards are described using device
>>> tree. The base board is described by a base device tree and addon boards
>>> are describe by overlays device tree. More details can be found at [2].
>>>
>>> This kind of use case can be found also on:
>>>     - Grove Sunlight Sensor [3]
>>>     - mikroBUS [4]
>>>
>>> One of the issue we were facing on was referencing resources available
>>> on the base board device tree from the addon overlay device tree.
>>>
>>> Using a nexus node [5] helps decoupling resources and avoid the
>>> knowledge of the full base board from the overlay. Indeed, with nexus
>>> node, the overlay need to know only about the nexus node itself.
>>>
>>> For instance, suppose a connector where a GPIO is connected at PinA. On
>>> the base board this GPIO is connected to the GPIO 12 of the SoC GPIO
>>> controller.
>>>
>>> The base board can describe this GPIO using a nexus node:
>>>       soc_gpio: gpio-controller {
>>>         #gpio-cells = <2>;
>>>       };
>>>
>>>       connector1: connector1 {
>>>           /*
>>>            * Nexus node for the GPIO available on the connector.
>>>            * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
>>>            * controller
>>>            */
>>>           #gpio-cells = <2>;
>>>           gpio-map = <0 0 &soc_gpio 12 0>;
>>>           gpio-map-mask = <0xf 0x0>;
>>>           gpio-map-pass-thru = <0x0 0xf>;
>>>       };
>>>
>>> The connector pin A GPIO can be referenced using:
>>>     <&connector1 0 GPIO_ACTIVE_HIGH>
>>>
>>> This implies that the overlay needs to know about exact label that
>>> references the connector. This label can be different on a different
>>> board and so applying the overlay could failed even if it is used to
>>> describe the exact same addon board. Further more, a given base board
>>> can have several connectors where the exact same addon board can be
>>> connected. In that case, the same overlay cannot be used on both
>>> connector. Indeed, the connector labels have to be different.
>>>
>>> The export-symbols node introduced by this current series solves this
>>> issue.
>>>
>>> The idea of export-symbols is to have something similar to the global
>>> __symbols__ node but local to a specific node. Symbols listed in this
>>> export-symbols are local and visible only when an overlay is applied on
>>> a node having an export-symbols subnode.
>>>
>>> Using export-symbols, our example becomes:
>>>       soc_gpio: gpio-controller {
>>>         #gpio-cells = <2>;
>>>       };
>>>
>>>       connector1: connector1 {
>>>           /*
>>>            * Nexus node for the GPIO available on the connector.
>>>            * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
>>>            * controller
>>>            */
>>>           #gpio-cells = <2>;
>>>           gpio-map = <0 0 &soc_gpio 12 0>;
>>>           gpio-map-mask = <0xf 0x0>;
>>>           gpio-map-pass-thru = <0x0 0xf>;
>>>
>>>           export-symbols {
>>>             connector = <&connector1>;
>>>           };
>>>       };
>>>
>>> With that export-symbols node, an overlay applied on connector1 node can
>>> have the symbol named 'connector' resolved to connector1. Indeed, the
>>> export-symbols node available at connector1 node is used when the
>>> overlay is applied. If the overlay has an unresolved 'connector' symbol,
>>> it will be resolved to connector1 thanks to export-symbols.
>>>
>>> Our overlay using the nexus node can contains:
>>>      node {
>>>         foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
>>>      };
>>> It used the GPIO 0 from the connector it is applied on.
>>>
>>> A board with two connectors can be described with:
>>>       connector1: connector1 {
>>>           ...
>>>           export-symbols {
>>>             connector = <&connector1>;
>>>           };
>>>       };
>>>
>>>       connector2: connector2 {
>>>           ...
>>>           export-symbols {
>>>             connector = <&connector2>;
>>>           };
>>>       };
>>>
>>> In that case, the same overlay with unresolved 'connector' symbol can be
>>> applied on both connectors and the correct symbol resolution (connector1
>>> or connector2) will be done.
>>>    
>>
>> I might be missing something, but how is the correct connector (connector1
>> or connector2) selected? Let's say I connect my addon board to connector2,
>> then I apply the addon board's overlay to the base DTB. What connector
>> just got referenced?
>>
> 
> A driver for the connector is needed.
> The driver applies the overlay using of_overlay_fdt_apply().
> The node the overlay has to be applied to is passed by the driver to
> of_overlay_fdt_apply().
> 

So every connector needs a driver? Most connectors are dumb connectors,
just a bunch of wires broken out to a header.

What if an addon board overlay uses multiple connectors?

If you need a connector-specific driver, and that driver needs to know
which node this overlay will be applied to, then why not just do a
fixup directly to the overlay in the driver?

Andrew

> Even if obsolete because I added one more parameter (export_symbols_name)
> in of_overlay_fdt_apply() in this current series, you can have a look at the
> following patch to see the connector driver:
>    https://lore.kernel.org/lkml/20240917-hotplug-drm-bridge-v4-8-bc4dfee61be6@bootlin.com/
> 
> Best regards,
> Hervé
> 

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Rob Herring]

On Mon, Dec 9, 2024 at 11:47 AM Andrew Davis <afd@ti.com> wrote:
>
> On 12/9/24 11:03 AM, Herve Codina wrote:
> > On Mon, 9 Dec 2024 10:47:50 -0600
> > Andrew Davis <afd@ti.com> wrote:
> >
> >> On 12/9/24 9:18 AM, Herve Codina wrote:
> >>> Hi,
> >>>
> >>> At Linux Plumbers Conference 2024, we (me and Luca Ceresolli) talked
> >>> about issues we have with runtime hotplug on non-discoverable busses
> >>> with device tree overlays [1].
> >>>
> >>> On our system, a base board has a connector and addon boards can be
> >>> connected to this connector. Both boards are described using device
> >>> tree. The base board is described by a base device tree and addon boards
> >>> are describe by overlays device tree. More details can be found at [2].
> >>>
> >>> This kind of use case can be found also on:
> >>>     - Grove Sunlight Sensor [3]
> >>>     - mikroBUS [4]
> >>>
> >>> One of the issue we were facing on was referencing resources available
> >>> on the base board device tree from the addon overlay device tree.
> >>>
> >>> Using a nexus node [5] helps decoupling resources and avoid the
> >>> knowledge of the full base board from the overlay. Indeed, with nexus
> >>> node, the overlay need to know only about the nexus node itself.
> >>>
> >>> For instance, suppose a connector where a GPIO is connected at PinA. On
> >>> the base board this GPIO is connected to the GPIO 12 of the SoC GPIO
> >>> controller.
> >>>
> >>> The base board can describe this GPIO using a nexus node:
> >>>       soc_gpio: gpio-controller {
> >>>         #gpio-cells = <2>;
> >>>       };
> >>>
> >>>       connector1: connector1 {
> >>>           /*
> >>>            * Nexus node for the GPIO available on the connector.
> >>>            * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
> >>>            * controller
> >>>            */
> >>>           #gpio-cells = <2>;
> >>>           gpio-map = <0 0 &soc_gpio 12 0>;
> >>>           gpio-map-mask = <0xf 0x0>;
> >>>           gpio-map-pass-thru = <0x0 0xf>;
> >>>       };
> >>>
> >>> The connector pin A GPIO can be referenced using:
> >>>     <&connector1 0 GPIO_ACTIVE_HIGH>
> >>>
> >>> This implies that the overlay needs to know about exact label that
> >>> references the connector. This label can be different on a different
> >>> board and so applying the overlay could failed even if it is used to
> >>> describe the exact same addon board. Further more, a given base board
> >>> can have several connectors where the exact same addon board can be
> >>> connected. In that case, the same overlay cannot be used on both
> >>> connector. Indeed, the connector labels have to be different.
> >>>
> >>> The export-symbols node introduced by this current series solves this
> >>> issue.
> >>>
> >>> The idea of export-symbols is to have something similar to the global
> >>> __symbols__ node but local to a specific node. Symbols listed in this
> >>> export-symbols are local and visible only when an overlay is applied on
> >>> a node having an export-symbols subnode.
> >>>
> >>> Using export-symbols, our example becomes:
> >>>       soc_gpio: gpio-controller {
> >>>         #gpio-cells = <2>;
> >>>       };
> >>>
> >>>       connector1: connector1 {
> >>>           /*
> >>>            * Nexus node for the GPIO available on the connector.
> >>>            * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
> >>>            * controller
> >>>            */
> >>>           #gpio-cells = <2>;
> >>>           gpio-map = <0 0 &soc_gpio 12 0>;
> >>>           gpio-map-mask = <0xf 0x0>;
> >>>           gpio-map-pass-thru = <0x0 0xf>;
> >>>
> >>>           export-symbols {
> >>>             connector = <&connector1>;
> >>>           };
> >>>       };
> >>>
> >>> With that export-symbols node, an overlay applied on connector1 node can
> >>> have the symbol named 'connector' resolved to connector1. Indeed, the
> >>> export-symbols node available at connector1 node is used when the
> >>> overlay is applied. If the overlay has an unresolved 'connector' symbol,
> >>> it will be resolved to connector1 thanks to export-symbols.
> >>>
> >>> Our overlay using the nexus node can contains:
> >>>      node {
> >>>         foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
> >>>      };
> >>> It used the GPIO 0 from the connector it is applied on.
> >>>
> >>> A board with two connectors can be described with:
> >>>       connector1: connector1 {
> >>>           ...
> >>>           export-symbols {
> >>>             connector = <&connector1>;
> >>>           };
> >>>       };
> >>>
> >>>       connector2: connector2 {
> >>>           ...
> >>>           export-symbols {
> >>>             connector = <&connector2>;
> >>>           };
> >>>       };
> >>>
> >>> In that case, the same overlay with unresolved 'connector' symbol can be
> >>> applied on both connectors and the correct symbol resolution (connector1
> >>> or connector2) will be done.
> >>>
> >>
> >> I might be missing something, but how is the correct connector (connector1
> >> or connector2) selected? Let's say I connect my addon board to connector2,
> >> then I apply the addon board's overlay to the base DTB. What connector
> >> just got referenced?
> >>
> >
> > A driver for the connector is needed.
> > The driver applies the overlay using of_overlay_fdt_apply().
> > The node the overlay has to be applied to is passed by the driver to
> > of_overlay_fdt_apply().
> >
>
> So every connector needs a driver? Most connectors are dumb connectors,
> just a bunch of wires broken out to a header.

Yes. So write a dumb-connector driver for all the dumb/simple/generic
connectors.

Though once pinmuxing comes into play, I'm not sure that anything will
be dumb or generic.

Rob

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Rob Herring]

On Mon, Dec 9, 2024 at 9:18 AM Herve Codina <herve.codina@bootlin.com> wrote:
>
> Hi,
>
> At Linux Plumbers Conference 2024, we (me and Luca Ceresolli) talked
> about issues we have with runtime hotplug on non-discoverable busses
> with device tree overlays [1].
>
> On our system, a base board has a connector and addon boards can be
> connected to this connector. Both boards are described using device
> tree. The base board is described by a base device tree and addon boards
> are describe by overlays device tree. More details can be found at [2].
>
> This kind of use case can be found also on:
>   - Grove Sunlight Sensor [3]
>   - mikroBUS [4]
>
> One of the issue we were facing on was referencing resources available
> on the base board device tree from the addon overlay device tree.
>
> Using a nexus node [5] helps decoupling resources and avoid the
> knowledge of the full base board from the overlay. Indeed, with nexus
> node, the overlay need to know only about the nexus node itself.
>
> For instance, suppose a connector where a GPIO is connected at PinA. On
> the base board this GPIO is connected to the GPIO 12 of the SoC GPIO
> controller.
>
> The base board can describe this GPIO using a nexus node:
>     soc_gpio: gpio-controller {
>       #gpio-cells = <2>;
>     };
>
>     connector1: connector1 {
>         /*
>          * Nexus node for the GPIO available on the connector.
>          * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
>          * controller
>          */
>         #gpio-cells = <2>;
>         gpio-map = <0 0 &soc_gpio 12 0>;
>         gpio-map-mask = <0xf 0x0>;
>         gpio-map-pass-thru = <0x0 0xf>;
>     };
>
> The connector pin A GPIO can be referenced using:
>   <&connector1 0 GPIO_ACTIVE_HIGH>
>
> This implies that the overlay needs to know about exact label that
> references the connector. This label can be different on a different
> board and so applying the overlay could failed even if it is used to
> describe the exact same addon board. Further more, a given base board
> can have several connectors where the exact same addon board can be
> connected. In that case, the same overlay cannot be used on both
> connector. Indeed, the connector labels have to be different.
>
> The export-symbols node introduced by this current series solves this
> issue.
>
> The idea of export-symbols is to have something similar to the global
> __symbols__ node but local to a specific node. Symbols listed in this
> export-symbols are local and visible only when an overlay is applied on
> a node having an export-symbols subnode.
>
> Using export-symbols, our example becomes:
>     soc_gpio: gpio-controller {
>       #gpio-cells = <2>;
>     };
>
>     connector1: connector1 {
>         /*
>          * Nexus node for the GPIO available on the connector.
>          * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
>          * controller
>          */
>         #gpio-cells = <2>;
>         gpio-map = <0 0 &soc_gpio 12 0>;
>         gpio-map-mask = <0xf 0x0>;
>         gpio-map-pass-thru = <0x0 0xf>;
>
>         export-symbols {
>           connector = <&connector1>;
>         };
>     };
>
> With that export-symbols node, an overlay applied on connector1 node can
> have the symbol named 'connector' resolved to connector1. Indeed, the
> export-symbols node available at connector1 node is used when the
> overlay is applied. If the overlay has an unresolved 'connector' symbol,
> it will be resolved to connector1 thanks to export-symbols.
>
> Our overlay using the nexus node can contains:
>    node {
>       foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
>    };

Couldn't we make something like this work:

connector: __overlay__ {

   node {
      foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
   };
};

We already have to process all the phandles in the overlay. So this
just changes handling of 'connector' from being a local phandle which
we just renumber to an unresolved phandle which we have to lookup and
replace the phandle uses with.

Rob

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Herve Codina]

Hi Rob,

On Mon, 9 Dec 2024 14:11:09 -0600
Rob Herring <robh@kernel.org> wrote:

...
> >
> > Our overlay using the nexus node can contains:
> >    node {
> >       foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
> >    };  
> 
> Couldn't we make something like this work:
> 
> connector: __overlay__ {
> 
>    node {
>       foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
>    };
> };
> 
> We already have to process all the phandles in the overlay. So this
> just changes handling of 'connector' from being a local phandle which
> we just renumber to an unresolved phandle which we have to lookup and
> replace the phandle uses with.
> 

I have tried what you suggested but I've got some issues with dtc.

If a label is not used as a phandle in a dts, dtc doesn't create the phandle
property in the pointed node (except if we use '-@' option but I don't want
to add all symbols in my dtb just for one or two connector symbols).

The way to make sure that the phandle property will be created in the base
DT node by dtc is to reference the label as a phandle in the base DT.
The export-symbols node references this label as a phandle in the base DT
and so, with that, dtc creates the phandle property.

Also, using 'connector: __overlay__' allows to have only one label from
the base DT to be referenced by the overlay.

I don't know if use cases exist where more than one label need to be
referenced but this 'one label' constraint is not present with the
export-symbols node.

The use case where more than one label would be needed is the need for a
phandle from the overlay that couldn't be translated by the connector nexus
node. Maybe pinctrl because it uses of_find_node_by_phandle().

Last point, having export-symbols node makes some nodes explicitly
candidates for an overlay and defines the label to be used on the base DT
node side. This specific label can be described in the node binding as well
as the nexus node properties.

With 'connector: __overlay__', the overlay can be applied on any nodes, at
least from the needed label point of view without any restrictions.

With all of that, I discarded the 'connector: __overlay__' description.

Best regards,
Hervé

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Ayush Singh]

On 09/12/24 20:48, Herve Codina wrote:
> Hi,
> 
> At Linux Plumbers Conference 2024, we (me and Luca Ceresolli) talked
> about issues we have with runtime hotplug on non-discoverable busses
> with device tree overlays [1].
> 
> On our system, a base board has a connector and addon boards can be
> connected to this connector. Both boards are described using device
> tree. The base board is described by a base device tree and addon boards
> are describe by overlays device tree. More details can be found at [2].
> 
> This kind of use case can be found also on:
>    - Grove Sunlight Sensor [3]
>    - mikroBUS [4]
> 
> One of the issue we were facing on was referencing resources available
> on the base board device tree from the addon overlay device tree.
> 
> Using a nexus node [5] helps decoupling resources and avoid the
> knowledge of the full base board from the overlay. Indeed, with nexus
> node, the overlay need to know only about the nexus node itself.
> 
> For instance, suppose a connector where a GPIO is connected at PinA. On
> the base board this GPIO is connected to the GPIO 12 of the SoC GPIO
> controller.
> 
> The base board can describe this GPIO using a nexus node:
>      soc_gpio: gpio-controller {
>        #gpio-cells = <2>;
>      };
> 
>      connector1: connector1 {
>          /*
>           * Nexus node for the GPIO available on the connector.
>           * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
>           * controller
>           */
>          #gpio-cells = <2>;
>          gpio-map = <0 0 &soc_gpio 12 0>;
>          gpio-map-mask = <0xf 0x0>;
>          gpio-map-pass-thru = <0x0 0xf>;
>      };
> 
> The connector pin A GPIO can be referenced using:
>    <&connector1 0 GPIO_ACTIVE_HIGH>
> 
> This implies that the overlay needs to know about exact label that
> references the connector. This label can be different on a different
> board and so applying the overlay could failed even if it is used to
> describe the exact same addon board. Further more, a given base board
> can have several connectors where the exact same addon board can be
> connected. In that case, the same overlay cannot be used on both
> connector. Indeed, the connector labels have to be different.
> 
> The export-symbols node introduced by this current series solves this
> issue.
> 
> The idea of export-symbols is to have something similar to the global
> __symbols__ node but local to a specific node. Symbols listed in this
> export-symbols are local and visible only when an overlay is applied on
> a node having an export-symbols subnode.
> 
> Using export-symbols, our example becomes:
>      soc_gpio: gpio-controller {
>        #gpio-cells = <2>;
>      };
> 
>      connector1: connector1 {
>          /*
>           * Nexus node for the GPIO available on the connector.
>           * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
>           * controller
>           */
>          #gpio-cells = <2>;
>          gpio-map = <0 0 &soc_gpio 12 0>;
>          gpio-map-mask = <0xf 0x0>;
>          gpio-map-pass-thru = <0x0 0xf>;
> 
>          export-symbols {
>            connector = <&connector1>;
>          };
>      };
> 
> With that export-symbols node, an overlay applied on connector1 node can
> have the symbol named 'connector' resolved to connector1. Indeed, the
> export-symbols node available at connector1 node is used when the
> overlay is applied. If the overlay has an unresolved 'connector' symbol,
> it will be resolved to connector1 thanks to export-symbols.
> 
> Our overlay using the nexus node can contains:
>     node {
>        foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
>     };
> It used the GPIO 0 from the connector it is applied on.
> 
> A board with two connectors can be described with:
>      connector1: connector1 {
>          ...
>          export-symbols {
>            connector = <&connector1>;
>          };
>      };
> 
>      connector2: connector2 {
>          ...
>          export-symbols {
>            connector = <&connector2>;
>          };
>      };
> 
> In that case, the same overlay with unresolved 'connector' symbol can be
> applied on both connectors and the correct symbol resolution (connector1
> or connector2) will be done.

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.

> 
> This current series add support for the export-symbols node feature:
>    - Patch 1 describes the export-symbols binding
>    - Patches 2 to 6 prepare and add the support for the export-symbols
>      feature
>    - Patch 7 adds an unittest for the export-symbols feature
> 
> Best regards,
> Hervé
> 
> [1] https://lpc.events/event/18/contributions/1696/
> [2] https://lore.kernel.org/lkml/20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com/
> [3] https://lore.kernel.org/lkml/20240702164403.29067-1-afd@ti.com/
> [4] https://lore.kernel.org/lkml/20240627-mikrobus-scratch-spi-v5-0-9e6c148bf5f0@beagleboard.org/
> [5] https://github.com/devicetree-org/devicetree-specification/blob/v0.4/source/chapter2-devicetree-basics.rst#nexus-nodes-and-specifier-mapping
> 
> Herve Codina (7):
>    dt-bindings: Add support for export-symbols node
>    of: resolver: Introduce get_phandle_from_symbols_node()
>    of: resolver: Add export_symbols in of_resolve_phandles() parameters
>    of: resolver: Add support for the export symbols node
>    of: overlay: Add export_symbols_name in of_overlay_fdt_apply()
>      parameters
>    of: overlay: Add support for the export symbols node
>    of: unittest: Add tests for export symbols
> 
>   .../devicetree/bindings/export-symbols.yaml   | 43 ++++++++++
>   drivers/misc/lan966x_pci.c                    |  3 +-
>   drivers/of/of_kunit_helpers.c                 |  2 +-
>   drivers/of/of_private.h                       |  2 +-
>   drivers/of/overlay.c                          | 30 ++++++-
>   drivers/of/resolver.c                         | 80 ++++++++++++++-----
>   drivers/of/unittest-data/Makefile             |  5 ++
>   .../unittest-data/overlay_export_symbols.dtso | 15 ++++
>   .../of/unittest-data/testcases_common.dtsi    |  1 +
>   .../unittest-data/tests-export-symbols.dtsi   | 30 +++++++
>   drivers/of/unittest.c                         | 76 ++++++++++++++++--
>   include/linux/of.h                            |  6 +-
>   12 files changed, 259 insertions(+), 34 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/export-symbols.yaml
>   create mode 100644 drivers/of/unittest-data/overlay_export_symbols.dtso
>   create mode 100644 drivers/of/unittest-data/tests-export-symbols.dtsi
> 


Ayush Singh

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Ayush Singh]

On 09/12/24 23:17, Andrew Davis wrote:
> On 12/9/24 11:03 AM, Herve Codina wrote:
>> On Mon, 9 Dec 2024 10:47:50 -0600
>> Andrew Davis <afd@ti.com> wrote:
>>
>>> On 12/9/24 9:18 AM, Herve Codina wrote:
>>>> Hi,
>>>>
>>>> At Linux Plumbers Conference 2024, we (me and Luca Ceresolli) talked
>>>> about issues we have with runtime hotplug on non-discoverable busses
>>>> with device tree overlays [1].
>>>>
>>>> On our system, a base board has a connector and addon boards can be
>>>> connected to this connector. Both boards are described using device
>>>> tree. The base board is described by a base device tree and addon 
>>>> boards
>>>> are describe by overlays device tree. More details can be found at [2].
>>>>
>>>> This kind of use case can be found also on:
>>>>     - Grove Sunlight Sensor [3]
>>>>     - mikroBUS [4]
>>>>
>>>> One of the issue we were facing on was referencing resources available
>>>> on the base board device tree from the addon overlay device tree.
>>>>
>>>> Using a nexus node [5] helps decoupling resources and avoid the
>>>> knowledge of the full base board from the overlay. Indeed, with nexus
>>>> node, the overlay need to know only about the nexus node itself.
>>>>
>>>> For instance, suppose a connector where a GPIO is connected at PinA. On
>>>> the base board this GPIO is connected to the GPIO 12 of the SoC GPIO
>>>> controller.
>>>>
>>>> The base board can describe this GPIO using a nexus node:
>>>>       soc_gpio: gpio-controller {
>>>>         #gpio-cells = <2>;
>>>>       };
>>>>
>>>>       connector1: connector1 {
>>>>           /*
>>>>            * Nexus node for the GPIO available on the connector.
>>>>            * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC 
>>>> gpio
>>>>            * controller
>>>>            */
>>>>           #gpio-cells = <2>;
>>>>           gpio-map = <0 0 &soc_gpio 12 0>;
>>>>           gpio-map-mask = <0xf 0x0>;
>>>>           gpio-map-pass-thru = <0x0 0xf>;
>>>>       };
>>>>
>>>> The connector pin A GPIO can be referenced using:
>>>>     <&connector1 0 GPIO_ACTIVE_HIGH>
>>>>
>>>> This implies that the overlay needs to know about exact label that
>>>> references the connector. This label can be different on a different
>>>> board and so applying the overlay could failed even if it is used to
>>>> describe the exact same addon board. Further more, a given base board
>>>> can have several connectors where the exact same addon board can be
>>>> connected. In that case, the same overlay cannot be used on both
>>>> connector. Indeed, the connector labels have to be different.
>>>>
>>>> The export-symbols node introduced by this current series solves this
>>>> issue.
>>>>
>>>> The idea of export-symbols is to have something similar to the global
>>>> __symbols__ node but local to a specific node. Symbols listed in this
>>>> export-symbols are local and visible only when an overlay is applied on
>>>> a node having an export-symbols subnode.
>>>>
>>>> Using export-symbols, our example becomes:
>>>>       soc_gpio: gpio-controller {
>>>>         #gpio-cells = <2>;
>>>>       };
>>>>
>>>>       connector1: connector1 {
>>>>           /*
>>>>            * Nexus node for the GPIO available on the connector.
>>>>            * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC 
>>>> gpio
>>>>            * controller
>>>>            */
>>>>           #gpio-cells = <2>;
>>>>           gpio-map = <0 0 &soc_gpio 12 0>;
>>>>           gpio-map-mask = <0xf 0x0>;
>>>>           gpio-map-pass-thru = <0x0 0xf>;
>>>>
>>>>           export-symbols {
>>>>             connector = <&connector1>;
>>>>           };
>>>>       };
>>>>
>>>> With that export-symbols node, an overlay applied on connector1 node 
>>>> can
>>>> have the symbol named 'connector' resolved to connector1. Indeed, the
>>>> export-symbols node available at connector1 node is used when the
>>>> overlay is applied. If the overlay has an unresolved 'connector' 
>>>> symbol,
>>>> it will be resolved to connector1 thanks to export-symbols.
>>>>
>>>> Our overlay using the nexus node can contains:
>>>>      node {
>>>>         foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
>>>>      };
>>>> It used the GPIO 0 from the connector it is applied on.
>>>>
>>>> A board with two connectors can be described with:
>>>>       connector1: connector1 {
>>>>           ...
>>>>           export-symbols {
>>>>             connector = <&connector1>;
>>>>           };
>>>>       };
>>>>
>>>>       connector2: connector2 {
>>>>           ...
>>>>           export-symbols {
>>>>             connector = <&connector2>;
>>>>           };
>>>>       };
>>>>
>>>> In that case, the same overlay with unresolved 'connector' symbol 
>>>> can be
>>>> applied on both connectors and the correct symbol resolution 
>>>> (connector1
>>>> or connector2) will be done.
>>>
>>> I might be missing something, but how is the correct connector 
>>> (connector1
>>> or connector2) selected? Let's say I connect my addon board to 
>>> connector2,
>>> then I apply the addon board's overlay to the base DTB. What connector
>>> just got referenced?
>>>
>>
>> A driver for the connector is needed.
>> The driver applies the overlay using of_overlay_fdt_apply().
>> The node the overlay has to be applied to is passed by the driver to
>> of_overlay_fdt_apply().
>>
> 
> So every connector needs a driver? Most connectors are dumb connectors,
> just a bunch of wires broken out to a header.
> 
> What if an addon board overlay uses multiple connectors?
> 
> If you need a connector-specific driver, and that driver needs to know
> which node this overlay will be applied to, then why not just do a
> fixup directly to the overlay in the driver?
> 
> Andrew

Well, even in the symbols based approach, a driver is needed since stuff 
like nexus nodes does need a dummy driver (else the device enters 
deferred probing). So it is not a big deal.

> 
>> Even if obsolete because I added one more parameter (export_symbols_name)
>> in of_overlay_fdt_apply() in this current series, you can have a look 
>> at the
>> following patch to see the connector driver:
>>    https://lore.kernel.org/lkml/20240917-hotplug-drm-bridge-v4-8- 
>> bc4dfee61be6@bootlin.com/
>>
>> Best regards,
>> Hervé
>>

Ayush Singh

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Herve Codina]

Hi Ayush,

On Tue, 10 Dec 2024 14:52:22 +0530
Ayush Singh <ayush@beagleboard.org> 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).

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?

Best regards,
Hervé

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Ayush Singh]

On 10/12/24 15:11, Herve Codina wrote:
> Hi Ayush,
> 
> On Tue, 10 Dec 2024 14:52:22 +0530
> Ayush Singh <ayush@beagleboard.org> 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@ti.com/

Ayush Singh

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Herve Codina]

Hi Ayush,

On Tue, 10 Dec 2024 15:26:44 +0530
Ayush Singh <ayush@beagleboard.org> wrote:

> On 10/12/24 15:11, Herve Codina wrote:
> > Hi Ayush,
> > 
> > On Tue, 10 Dec 2024 14:52:22 +0530
> > Ayush Singh <ayush@beagleboard.org> 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@ti.com/


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é

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Rob Herring]

+dtc list and David G.

On Tue, Dec 10, 2024 at 2:16 AM Herve Codina <herve.codina@bootlin.com> wrote:
>
> Hi Rob,
>
> On Mon, 9 Dec 2024 14:11:09 -0600
> Rob Herring <robh@kernel.org> wrote:
>
> ...
> > >
> > > Our overlay using the nexus node can contains:
> > >    node {
> > >       foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
> > >    };
> >
> > Couldn't we make something like this work:
> >
> > connector: __overlay__ {
> >
> >    node {
> >       foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
> >    };
> > };
> >
> > We already have to process all the phandles in the overlay. So this
> > just changes handling of 'connector' from being a local phandle which
> > we just renumber to an unresolved phandle which we have to lookup and
> > replace the phandle uses with.
> >
>
> I have tried what you suggested but I've got some issues with dtc.
>
> If a label is not used as a phandle in a dts, dtc doesn't create the phandle
> property in the pointed node (except if we use '-@' option but I don't want
> to add all symbols in my dtb just for one or two connector symbols).

Sorry, but that's the cost of using overlays, and that's pretty
orthogonal to the issue of how the overlay references the connector
node.

However, I agree '-@' is a pretty big switch and an issue that's been
discussed before. I also don't like that all labels become part of the
ABI nor the fact that overlays can make any random modification
anywhere in the DT. I would rather see some sort of explicit opt-in
mechanism of nodes we can apply overlays to. Perhaps we could do
something like this:

/export/ label: node {
};

And then __symbols__ can be only those exported labels (unless -@ is used).

> The way to make sure that the phandle property will be created in the base
> DT node by dtc is to reference the label as a phandle in the base DT.
> The export-symbols node references this label as a phandle in the base DT
> and so, with that, dtc creates the phandle property.
>
> Also, using 'connector: __overlay__' allows to have only one label from
> the base DT to be referenced by the overlay.
>
> I don't know if use cases exist where more than one label need to be
> referenced but this 'one label' constraint is not present with the
> export-symbols node.
>
> The use case where more than one label would be needed is the need for a
> phandle from the overlay that couldn't be translated by the connector nexus
> node. Maybe pinctrl because it uses of_find_node_by_phandle().

Labels are an ABI. I can't see that we need to remap them when we can
just say the name must be X. We can have multiple labels on a node as
well. So I think the problem space is purely mapping 1 name to
multiple possible names.

The connector handling has to be addressed binding by binding at least
for each pattern of binding. Pinctrl binding is pretty unique, so we
should make sure we can handle it in this case.

> Last point, having export-symbols node makes some nodes explicitly
> candidates for an overlay and defines the label to be used on the base DT
> node side. This specific label can be described in the node binding as well
> as the nexus node properties.

Both David (IIRC) and I feel that putting the overlay info
(__symbols__, __fixups__, etc.) within the DT data rather than in the
DTB format was a mistake. The export-symbols node expands on that, so
I'm not sure that's the right direction.

(We should have rev'ed the DTB format to store type information for
(at a minimum) phandles.)

> With 'connector: __overlay__', the overlay can be applied on any nodes, at
> least from the needed label point of view without any restrictions.

Certainly that is something I'd like to have some control over. An
/export/ tag would accomplish that.

One idea I have there is that the overlay could have the compatible of
the connector and we use that for matching. That would give us a way
to know what base DTs overlays apply to. Then you could load an
overlay and dispatch it to the correct driver to handle. It would have
to be handled as a special case as the compatible may match, but
wouldn't necessarily be equal values.


I'll throw out another idea. What if we make resolving phandle errors
something that can be handled by the connector driver? The driver
knows 'connector' resolves to the connector node it is applying the
overlay to.

Rob

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Herve Codina]

Hi Rob,

On Tue, 10 Dec 2024 07:46:02 -0600
Rob Herring <robh@kernel.org> wrote:

> +dtc list and David G.
> 
> On Tue, Dec 10, 2024 at 2:16 AM Herve Codina <herve.codina@bootlin.com> wrote:
> >
> > Hi Rob,
> >
> > On Mon, 9 Dec 2024 14:11:09 -0600
> > Rob Herring <robh@kernel.org> wrote:
> >
> > ...  
> > > >
> > > > Our overlay using the nexus node can contains:
> > > >    node {
> > > >       foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
> > > >    };  
> > >
> > > Couldn't we make something like this work:
> > >
> > > connector: __overlay__ {
> > >
> > >    node {
> > >       foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
> > >    };
> > > };
> > >
> > > We already have to process all the phandles in the overlay. So this
> > > just changes handling of 'connector' from being a local phandle which
> > > we just renumber to an unresolved phandle which we have to lookup and
> > > replace the phandle uses with.
> > >  
> >
> > I have tried what you suggested but I've got some issues with dtc.
> >
> > If a label is not used as a phandle in a dts, dtc doesn't create the phandle
> > property in the pointed node (except if we use '-@' option but I don't want
> > to add all symbols in my dtb just for one or two connector symbols).  
> 
> Sorry, but that's the cost of using overlays, and that's pretty
> orthogonal to the issue of how the overlay references the connector
> node.
> 
> However, I agree '-@' is a pretty big switch and an issue that's been
> discussed before. I also don't like that all labels become part of the
> ABI nor the fact that overlays can make any random modification
> anywhere in the DT. I would rather see some sort of explicit opt-in
> mechanism of nodes we can apply overlays to. Perhaps we could do
> something like this:
> 
> /export/ label: node {
> };
> 
> And then __symbols__ can be only those exported labels (unless -@ is used).
> 
> > The way to make sure that the phandle property will be created in the base
> > DT node by dtc is to reference the label as a phandle in the base DT.
> > The export-symbols node references this label as a phandle in the base DT
> > and so, with that, dtc creates the phandle property.
> >
> > Also, using 'connector: __overlay__' allows to have only one label from
> > the base DT to be referenced by the overlay.
> >
> > I don't know if use cases exist where more than one label need to be
> > referenced but this 'one label' constraint is not present with the
> > export-symbols node.
> >
> > The use case where more than one label would be needed is the need for a
> > phandle from the overlay that couldn't be translated by the connector nexus
> > node. Maybe pinctrl because it uses of_find_node_by_phandle().  
> 
> Labels are an ABI. I can't see that we need to remap them when we can
> just say the name must be X. We can have multiple labels on a node as
> well. So I think the problem space is purely mapping 1 name to
> multiple possible names.

So, with a base DT having:
  /export/ label0: node@0 {
  }

  /export/ label1: node@1 {
  }

the __symbols__ node will contains:
  __symbols__ {
	label0 = ...;
	label1 = ...;
  }

without export-symbols, the overlay will look like this:
  connector: __overlay__ {
	...
	ref = <&connector>;
  }

The "connector" label is the only one we can use from the overlay to
reference the base DT (special label defined on the __overlay__ node).
As it is defined to point to __overlay__, it has to be resolved to the
exported symbol that point to the node where the overlay is applied.

If the overlay is applied on node@0, 'connector' is resolved to node@0.

This case cannot be handled:
   connector: __overlay__ {
	...
	ref1 = <&connector>;
        ref2 = <&other-label-from-base-dt>;
   }

Indeed, only one 'connector' label can be resolved to node@0 or node@1.
other-label-from-base-dt cannot be resolved based on the node the overlay
is applied to.

Again, I am not sure on my side that we have to handle this case of multiple
labels in the overlay that point to the base DT dependent on node@0 or node@1.

On my use case, I considered the node@0 or node@1 as nexus nodes and so, all
GPIOs (soon PWM, I hope, and probably other ressources in the future) can be
referenced using nexus nodes.

> 
> The connector handling has to be addressed binding by binding at least
> for each pattern of binding. Pinctrl binding is pretty unique, so we
> should make sure we can handle it in this case.

If pinctrl can be handled using nexus node, it should be ok. Otherwise
things are going to be complicated. Again, with your proposal, we can
reference only one label from the overlay, the node where the overlay
is applied to.

> 
> > Last point, having export-symbols node makes some nodes explicitly
> > candidates for an overlay and defines the label to be used on the base DT
> > node side. This specific label can be described in the node binding as well
> > as the nexus node properties.  
> 
> Both David (IIRC) and I feel that putting the overlay info
> (__symbols__, __fixups__, etc.) within the DT data rather than in the
> DTB format was a mistake. The export-symbols node expands on that, so
> I'm not sure that's the right direction.
> 
> (We should have rev'ed the DTB format to store type information for
> (at a minimum) phandles.)
> 
> > With 'connector: __overlay__', the overlay can be applied on any nodes, at
> > least from the needed label point of view without any restrictions.  
> 
> Certainly that is something I'd like to have some control over. An
> /export/ tag would accomplish that.
> 
> One idea I have there is that the overlay could have the compatible of
> the connector and we use that for matching. That would give us a way
> to know what base DTs overlays apply to. Then you could load an
> overlay and dispatch it to the correct driver to handle. It would have
> to be handled as a special case as the compatible may match, but
> wouldn't necessarily be equal values.

compatible property will not be enough. We can have two exact same
connectors on the same base board:
  /export/ label0: node@0 {
     compatible = 'foo,addon-connector';
  }

  /export/ label1: node@1 {
     compatible = 'foo,addon-connector';
  }

To load the overlay, we can match compatible for sure but we need to node
the overlay is supposed to be applied to.

Also, it you want to check bindings for node@0 available in the base DT
and bindings in the overlay, having one compatible string with different
meaning will introduce some complexity.
A compatible string can now define the "provider" part (the base DT) and
the "consumer" part (the overlay).

> 
> 
> I'll throw out another idea. What if we make resolving phandle errors
> something that can be handled by the connector driver? The driver
> knows 'connector' resolves to the connector node it is applying the
> overlay to.

Well, my proposal was to give enough information to the resolver instead of
handling errors.

I probably miss something but I don't see what could be the benefit to do
that in the other way. Can you give me more details about your idea?

Best regards,
Hervé

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Herve Codina]

Hi Rob, David,

On Tue, 10 Dec 2024 15:58:33 +0100
Herve Codina <herve.codina@bootlin.com> wrote:

> Hi Rob,
> 
> On Tue, 10 Dec 2024 07:46:02 -0600
> Rob Herring <robh@kernel.org> wrote:
> 
> > +dtc list and David G.
> > 
> > On Tue, Dec 10, 2024 at 2:16 AM Herve Codina <herve.codina@bootlin.com> wrote:  
> > >
> > > Hi Rob,
> > >
> > > On Mon, 9 Dec 2024 14:11:09 -0600
> > > Rob Herring <robh@kernel.org> wrote:
> > >
> > > ...    
> > > > >
> > > > > Our overlay using the nexus node can contains:
> > > > >    node {
> > > > >       foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
> > > > >    };    
> > > >
> > > > Couldn't we make something like this work:
> > > >
> > > > connector: __overlay__ {
> > > >
> > > >    node {
> > > >       foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
> > > >    };
> > > > };
> > > >
> > > > We already have to process all the phandles in the overlay. So this
> > > > just changes handling of 'connector' from being a local phandle which
> > > > we just renumber to an unresolved phandle which we have to lookup and
> > > > replace the phandle uses with.
> > > >    
> > >
> > > I have tried what you suggested but I've got some issues with dtc.
> > >
> > > If a label is not used as a phandle in a dts, dtc doesn't create the phandle
> > > property in the pointed node (except if we use '-@' option but I don't want
> > > to add all symbols in my dtb just for one or two connector symbols).    
> > 
> > Sorry, but that's the cost of using overlays, and that's pretty
> > orthogonal to the issue of how the overlay references the connector
> > node.
> > 
> > However, I agree '-@' is a pretty big switch and an issue that's been
> > discussed before. I also don't like that all labels become part of the
> > ABI nor the fact that overlays can make any random modification
> > anywhere in the DT. I would rather see some sort of explicit opt-in
> > mechanism of nodes we can apply overlays to. Perhaps we could do
> > something like this:
> > 
> > /export/ label: node {
> > };
> > 
> > And then __symbols__ can be only those exported labels (unless -@ is used).
> >   
> > > The way to make sure that the phandle property will be created in the base
> > > DT node by dtc is to reference the label as a phandle in the base DT.
> > > The export-symbols node references this label as a phandle in the base DT
> > > and so, with that, dtc creates the phandle property.
> > >
> > > Also, using 'connector: __overlay__' allows to have only one label from
> > > the base DT to be referenced by the overlay.
> > >
> > > I don't know if use cases exist where more than one label need to be
> > > referenced but this 'one label' constraint is not present with the
> > > export-symbols node.
> > >
> > > The use case where more than one label would be needed is the need for a
> > > phandle from the overlay that couldn't be translated by the connector nexus
> > > node. Maybe pinctrl because it uses of_find_node_by_phandle().    
> > 
> > Labels are an ABI. I can't see that we need to remap them when we can
> > just say the name must be X. We can have multiple labels on a node as
> > well. So I think the problem space is purely mapping 1 name to
> > multiple possible names.  
> 
> So, with a base DT having:
>   /export/ label0: node@0 {
>   }
> 
>   /export/ label1: node@1 {
>   }
> 
> the __symbols__ node will contains:
>   __symbols__ {
> 	label0 = ...;
> 	label1 = ...;
>   }
> 
> without export-symbols, the overlay will look like this:
>   connector: __overlay__ {
> 	...
> 	ref = <&connector>;
>   }
> 
> The "connector" label is the only one we can use from the overlay to
> reference the base DT (special label defined on the __overlay__ node).
> As it is defined to point to __overlay__, it has to be resolved to the
> exported symbol that point to the node where the overlay is applied.
> 
> If the overlay is applied on node@0, 'connector' is resolved to node@0.
> 
> This case cannot be handled:
>    connector: __overlay__ {
> 	...
> 	ref1 = <&connector>;
>         ref2 = <&other-label-from-base-dt>;
>    }
> 
> Indeed, only one 'connector' label can be resolved to node@0 or node@1.
> other-label-from-base-dt cannot be resolved based on the node the overlay
> is applied to.
> 
> Again, I am not sure on my side that we have to handle this case of multiple
> labels in the overlay that point to the base DT dependent on node@0 or node@1.
> 
> On my use case, I considered the node@0 or node@1 as nexus nodes and so, all
> GPIOs (soon PWM, I hope, and probably other ressources in the future) can be
> referenced using nexus nodes.
> 
> > 
> > The connector handling has to be addressed binding by binding at least
> > for each pattern of binding. Pinctrl binding is pretty unique, so we
> > should make sure we can handle it in this case.  
> 
> If pinctrl can be handled using nexus node, it should be ok. Otherwise
> things are going to be complicated. Again, with your proposal, we can
> reference only one label from the overlay, the node where the overlay
> is applied to.
> 
> >   
> > > Last point, having export-symbols node makes some nodes explicitly
> > > candidates for an overlay and defines the label to be used on the base DT
> > > node side. This specific label can be described in the node binding as well
> > > as the nexus node properties.    
> > 
> > Both David (IIRC) and I feel that putting the overlay info
> > (__symbols__, __fixups__, etc.) within the DT data rather than in the
> > DTB format was a mistake. The export-symbols node expands on that, so
> > I'm not sure that's the right direction.
> > 
> > (We should have rev'ed the DTB format to store type information for
> > (at a minimum) phandles.)
> >   
> > > With 'connector: __overlay__', the overlay can be applied on any nodes, at
> > > least from the needed label point of view without any restrictions.    
> > 
> > Certainly that is something I'd like to have some control over. An
> > /export/ tag would accomplish that.
> > 
> > One idea I have there is that the overlay could have the compatible of
> > the connector and we use that for matching. That would give us a way
> > to know what base DTs overlays apply to. Then you could load an
> > overlay and dispatch it to the correct driver to handle. It would have
> > to be handled as a special case as the compatible may match, but
> > wouldn't necessarily be equal values.  
> 
> compatible property will not be enough. We can have two exact same
> connectors on the same base board:
>   /export/ label0: node@0 {
>      compatible = 'foo,addon-connector';
>   }
> 
>   /export/ label1: node@1 {
>      compatible = 'foo,addon-connector';
>   }
> 
> To load the overlay, we can match compatible for sure but we need to node
> the overlay is supposed to be applied to.
> 
> Also, it you want to check bindings for node@0 available in the base DT
> and bindings in the overlay, having one compatible string with different
> meaning will introduce some complexity.
> A compatible string can now define the "provider" part (the base DT) and
> the "consumer" part (the overlay).
> 
> > 
> > 
> > I'll throw out another idea. What if we make resolving phandle errors
> > something that can be handled by the connector driver? The driver
> > knows 'connector' resolves to the connector node it is applying the
> > overlay to.  
> 
> Well, my proposal was to give enough information to the resolver instead of
> handling errors.
> 
> I probably miss something but I don't see what could be the benefit to do
> that in the other way. Can you give me more details about your idea?
> 

If I understand correctly, to move forward on the topic, we are waiting from
feedback from David.

Is that correct or do you have in mind an other plan I could explore?

Best regards,
Hervé

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Ayush Singh]

On 10/12/24 16:25, Herve Codina wrote:
> Hi Ayush,
> 
> On Tue, 10 Dec 2024 15:26:44 +0530
> Ayush Singh <ayush@beagleboard.org> wrote:
> 
>> On 10/12/24 15:11, Herve Codina wrote:
>>> Hi Ayush,
>>>
>>> On Tue, 10 Dec 2024 14:52:22 +0530
>>> Ayush Singh <ayush@beagleboard.org> 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@ti.com/
> 
> 
> 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.

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@beagleboard.org/T/#m900b5ca13cfc28396d4d46d9c3130a7070fa8c90

Best regards,
Ayush Singh

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Herve Codina]

Hi Ayush,

On Wed, 8 Jan 2025 13:06:03 +0530
Ayush Singh <ayush@beagleboard.org> wrote:

> On 10/12/24 16:25, Herve Codina wrote:
> > Hi Ayush,
> > 
> > On Tue, 10 Dec 2024 15:26:44 +0530
> > Ayush Singh <ayush@beagleboard.org> wrote:
> >   
> >> On 10/12/24 15:11, Herve Codina wrote:  
> >>> Hi Ayush,
> >>>
> >>> On Tue, 10 Dec 2024 14:52:22 +0530
> >>> Ayush Singh <ayush@beagleboard.org> 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@ti.com/  
> > 
> > 
> > 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@beagleboard.org/T/#m900b5ca13cfc28396d4d46d9c3130a7070fa8c90
> 
> Best regards,
> Ayush Singh
> 

Thanks for your help proposal!

Best regards,
Hervé

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Ayush Singh]

On 08/01/25 13:37, Herve Codina wrote:
> Hi Ayush,
> 
> On Wed, 8 Jan 2025 13:06:03 +0530
> Ayush Singh <ayush@beagleboard.org> wrote:
> 
>> On 10/12/24 16:25, Herve Codina wrote:
>>> Hi Ayush,
>>>
>>> On Tue, 10 Dec 2024 15:26:44 +0530
>>> Ayush Singh <ayush@beagleboard.org> wrote:
>>>    
>>>> On 10/12/24 15:11, Herve Codina wrote:
>>>>> Hi Ayush,
>>>>>
>>>>> On Tue, 10 Dec 2024 14:52:22 +0530
>>>>> Ayush Singh <ayush@beagleboard.org> 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@ti.com/
>>>
>>>
>>> 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.

I see, it seems linux does not use libfdt for applying overlays internally.

> 
>>
>> 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@beagleboard.org/T/#m900b5ca13cfc28396d4d46d9c3130a7070fa8c90
>>
>> Best regards,
>> Ayush Singh
>>
> 
> Thanks for your help proposal!
> 
> Best regards,
> Hervé

I will experiment with adding support to dtc and see how things look. 
Hopefully, 2025 is the year of addon board support.

Best regards,
Ayush Singh

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Herve Codina]

Hi Ayush,

On Wed, 8 Jan 2025 13:58:04 +0530
Ayush Singh <ayush@beagleboard.org> wrote:

...
> 
> I will experiment with adding support to dtc and see how things look. 
> Hopefully, 2025 is the year of addon board support.
> 

Also one point different between fdtoverlay an runtime loading is
that runtime loading allows to set the target node of the overlay
at runtime.

For instance, on LAN966X PCI driver, an overlay is loaded and
applied on a PCI device node.
The overlay loading is done by the PCI driver device:
  https://elixir.bootlin.com/linux/v6.13-rc1/source/drivers/misc/lan966x_pci.c#L131
The overlay loaded is the following one:
  https://elixir.bootlin.com/linux/v6.13-rc1/source/drivers/misc/lan966x_pci.dtso

For addon boards, this feature is also useful because without any
modification in the overlay itself, it can be applied on the correct
connector. This allows to support, without any overlay modification the
following cases:
 - A base board with multiple connectors where an addon board can be
   connected.
 - An addon board with its own DT overlay used on different base board

This feature is not supported by fdtoverlay. Maybe something like
  fdtoverlay --base=/somewhere/my_connector
could be implemented in fdtoverlay in order to choose the node where the
overlay has to be applied.

Best regards,
Hervé

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1252 bytes --]

On Wed, Jan 08, 2025 at 10:47:19AM +0100, Herve Codina wrote:
> Hi Ayush,
> 
> On Wed, 8 Jan 2025 13:58:04 +0530
> Ayush Singh <ayush@beagleboard.org> wrote:
> 
> ...
> > 
> > I will experiment with adding support to dtc and see how things look. 
> > Hopefully, 2025 is the year of addon board support.
> > 
> 
> Also one point different between fdtoverlay an runtime loading is
> that runtime loading allows to set the target node of the overlay
> at runtime.

I'm not really sure what you mean by "runtime loading".  Do you mean
the kernel's implementation of loading dtbo overlays?

While that is a different implementation from the one in fdtoverlay
(AIUI), they're both working from the same dtb format.  As we
discovered attempting Ayush's proposal, it turns out that the dtbo
simply doesn't have the information we need to correctly path
substitutions; and it's not at all easy to add it.

So, the problem of the encoding format needs to be solved regardless
of which implementation is actually processing the overlays.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Herve Codina]

Hi David,

On Fri, 10 Jan 2025 15:26:23 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, Jan 08, 2025 at 10:47:19AM +0100, Herve Codina wrote:
> > Hi Ayush,
> > 
> > On Wed, 8 Jan 2025 13:58:04 +0530
> > Ayush Singh <ayush@beagleboard.org> wrote:
> > 
> > ...  
> > > 
> > > I will experiment with adding support to dtc and see how things look. 
> > > Hopefully, 2025 is the year of addon board support.
> > >   
> > 
> > Also one point different between fdtoverlay an runtime loading is
> > that runtime loading allows to set the target node of the overlay
> > at runtime.  
> 
> I'm not really sure what you mean by "runtime loading".  Do you mean
> the kernel's implementation of loading dtbo overlays?

Yes, exactly.

> 
> While that is a different implementation from the one in fdtoverlay
> (AIUI), they're both working from the same dtb format.  As we
> discovered attempting Ayush's proposal, it turns out that the dtbo
> simply doesn't have the information we need to correctly path
> substitutions; and it's not at all easy to add it.
> 
> So, the problem of the encoding format needs to be solved regardless
> of which implementation is actually processing the overlays.
> 

What is the issue with the encoding format?

It works on my side (kernel loading) and I didn't see anything problematic.

Best regards
Hervé

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Ayush Singh]

On 10/01/25 09:56, David Gibson wrote:
> On Wed, Jan 08, 2025 at 10:47:19AM +0100, Herve Codina wrote:
>> Hi Ayush,
>>
>> On Wed, 8 Jan 2025 13:58:04 +0530
>> Ayush Singh <ayush@beagleboard.org> wrote:
>>
>> ...
>>>
>>> I will experiment with adding support to dtc and see how things look.
>>> Hopefully, 2025 is the year of addon board support.
>>>
>>
>> Also one point different between fdtoverlay an runtime loading is
>> that runtime loading allows to set the target node of the overlay
>> at runtime.
> 
> I'm not really sure what you mean by "runtime loading".  Do you mean
> the kernel's implementation of loading dtbo overlays?
> 
> While that is a different implementation from the one in fdtoverlay
> (AIUI), they're both working from the same dtb format.  As we
> discovered attempting Ayush's proposal, it turns out that the dtbo
> simply doesn't have the information we need to correctly path
> substitutions; and it's not at all easy to add it.

Ahh, I think there is a misunderstanding. `export-symbols` only seems to 
support phandles, not paths. So no resizing involved.

It's closer to the phandle support in `__symbols__`, just local in scope.

> 
> So, the problem of the encoding format needs to be solved regardless
> of which implementation is actually processing the overlays.
> 

So there is no problem with the encoding format.

The questions I have are more regarding weather `export-symbols` or 
something close to it can be made part of spec instead of the custom 
linux thing.


Ayush Singh

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2026 bytes --]

On Fri, Jan 10, 2025 at 01:25:43PM +0530, Ayush Singh wrote:
> 
> 
> On 10/01/25 09:56, David Gibson wrote:
> > On Wed, Jan 08, 2025 at 10:47:19AM +0100, Herve Codina wrote:
> > > Hi Ayush,
> > > 
> > > On Wed, 8 Jan 2025 13:58:04 +0530
> > > Ayush Singh <ayush@beagleboard.org> wrote:
> > > 
> > > ...
> > > > 
> > > > I will experiment with adding support to dtc and see how things look.
> > > > Hopefully, 2025 is the year of addon board support.
> > > > 
> > > 
> > > Also one point different between fdtoverlay an runtime loading is
> > > that runtime loading allows to set the target node of the overlay
> > > at runtime.
> > 
> > I'm not really sure what you mean by "runtime loading".  Do you mean
> > the kernel's implementation of loading dtbo overlays?
> > 
> > While that is a different implementation from the one in fdtoverlay
> > (AIUI), they're both working from the same dtb format.  As we
> > discovered attempting Ayush's proposal, it turns out that the dtbo
> > simply doesn't have the information we need to correctly path
> > substitutions; and it's not at all easy to add it.
> 
> Ahh, I think there is a misunderstanding. `export-symbols` only seems to
> support phandles, not paths. So no resizing involved.

Oh, sorry, I was mixing this up with a different feature Ayush was
working on.

> It's closer to the phandle support in `__symbols__`, just local in scope.
> 
> > 
> > So, the problem of the encoding format needs to be solved regardless
> > of which implementation is actually processing the overlays.
> > 
> 
> So there is no problem with the encoding format.
> 
> The questions I have are more regarding weather `export-symbols` or
> something close to it can be made part of spec instead of the custom linux
> thing.
> 
> 
> Ayush Singh
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Ayush Singh]

On 12/9/24 20:48, Herve Codina wrote:

> Hi,
>
> At Linux Plumbers Conference 2024, we (me and Luca Ceresolli) talked
> about issues we have with runtime hotplug on non-discoverable busses
> with device tree overlays [1].
>
> On our system, a base board has a connector and addon boards can be
> connected to this connector. Both boards are described using device
> tree. The base board is described by a base device tree and addon boards
> are describe by overlays device tree. More details can be found at [2].
>
> This kind of use case can be found also on:
>    - Grove Sunlight Sensor [3]
>    - mikroBUS [4]
>
> One of the issue we were facing on was referencing resources available
> on the base board device tree from the addon overlay device tree.
>
> Using a nexus node [5] helps decoupling resources and avoid the
> knowledge of the full base board from the overlay. Indeed, with nexus
> node, the overlay need to know only about the nexus node itself.
>
> For instance, suppose a connector where a GPIO is connected at PinA. On
> the base board this GPIO is connected to the GPIO 12 of the SoC GPIO
> controller.
>
> The base board can describe this GPIO using a nexus node:
>      soc_gpio: gpio-controller {
>        #gpio-cells = <2>;
>      };
>
>      connector1: connector1 {
>          /*
>           * Nexus node for the GPIO available on the connector.
>           * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
>           * controller
>           */
>          #gpio-cells = <2>;
>          gpio-map = <0 0 &soc_gpio 12 0>;
>          gpio-map-mask = <0xf 0x0>;
>          gpio-map-pass-thru = <0x0 0xf>;
>      };
>
> The connector pin A GPIO can be referenced using:
>    <&connector1 0 GPIO_ACTIVE_HIGH>
>
> This implies that the overlay needs to know about exact label that
> references the connector. This label can be different on a different
> board and so applying the overlay could failed even if it is used to
> describe the exact same addon board. Further more, a given base board
> can have several connectors where the exact same addon board can be
> connected. In that case, the same overlay cannot be used on both
> connector. Indeed, the connector labels have to be different.
>
> The export-symbols node introduced by this current series solves this
> issue.
>
> The idea of export-symbols is to have something similar to the global
> __symbols__ node but local to a specific node. Symbols listed in this
> export-symbols are local and visible only when an overlay is applied on
> a node having an export-symbols subnode.
>
> Using export-symbols, our example becomes:
>      soc_gpio: gpio-controller {
>        #gpio-cells = <2>;
>      };
>
>      connector1: connector1 {
>          /*
>           * Nexus node for the GPIO available on the connector.
>           * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
>           * controller
>           */
>          #gpio-cells = <2>;
>          gpio-map = <0 0 &soc_gpio 12 0>;
>          gpio-map-mask = <0xf 0x0>;
>          gpio-map-pass-thru = <0x0 0xf>;
>
>          export-symbols {
>            connector = <&connector1>;
>          };
>      };
>
> With that export-symbols node, an overlay applied on connector1 node can
> have the symbol named 'connector' resolved to connector1. Indeed, the
> export-symbols node available at connector1 node is used when the
> overlay is applied. If the overlay has an unresolved 'connector' symbol,
> it will be resolved to connector1 thanks to export-symbols.
>
> Our overlay using the nexus node can contains:
>     node {
>        foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
>     };
> It used the GPIO 0 from the connector it is applied on.
>
> A board with two connectors can be described with:
>      connector1: connector1 {
>          ...
>          export-symbols {
>            connector = <&connector1>;
>          };
>      };
>
>      connector2: connector2 {
>          ...
>          export-symbols {
>            connector = <&connector2>;
>          };
>      };
>
> In that case, the same overlay with unresolved 'connector' symbol can be
> applied on both connectors and the correct symbol resolution (connector1
> or connector2) will be done.
>
> This current series add support for the export-symbols node feature:
>    - Patch 1 describes the export-symbols binding
>    - Patches 2 to 6 prepare and add the support for the export-symbols
>      feature
>    - Patch 7 adds an unittest for the export-symbols feature
>
> Best regards,
> Hervé
>
> [1] https://lpc.events/event/18/contributions/1696/
> [2] https://lore.kernel.org/lkml/20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com/
> [3] https://lore.kernel.org/lkml/20240702164403.29067-1-afd@ti.com/
> [4] https://lore.kernel.org/lkml/20240627-mikrobus-scratch-spi-v5-0-9e6c148bf5f0@beagleboard.org/
> [5] https://github.com/devicetree-org/devicetree-specification/blob/v0.4/source/chapter2-devicetree-basics.rst#nexus-nodes-and-specifier-mapping
>
> Herve Codina (7):
>    dt-bindings: Add support for export-symbols node
>    of: resolver: Introduce get_phandle_from_symbols_node()
>    of: resolver: Add export_symbols in of_resolve_phandles() parameters
>    of: resolver: Add support for the export symbols node
>    of: overlay: Add export_symbols_name in of_overlay_fdt_apply()
>      parameters
>    of: overlay: Add support for the export symbols node
>    of: unittest: Add tests for export symbols
>
>   .../devicetree/bindings/export-symbols.yaml   | 43 ++++++++++
>   drivers/misc/lan966x_pci.c                    |  3 +-
>   drivers/of/of_kunit_helpers.c                 |  2 +-
>   drivers/of/of_private.h                       |  2 +-
>   drivers/of/overlay.c                          | 30 ++++++-
>   drivers/of/resolver.c                         | 80 ++++++++++++++-----
>   drivers/of/unittest-data/Makefile             |  5 ++
>   .../unittest-data/overlay_export_symbols.dtso | 15 ++++
>   .../of/unittest-data/testcases_common.dtsi    |  1 +
>   .../unittest-data/tests-export-symbols.dtsi   | 30 +++++++
>   drivers/of/unittest.c                         | 76 ++++++++++++++++--
>   include/linux/of.h                            |  6 +-
>   12 files changed, 259 insertions(+), 34 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/export-symbols.yaml
>   create mode 100644 drivers/of/unittest-data/overlay_export_symbols.dtso
>   create mode 100644 drivers/of/unittest-data/tests-export-symbols.dtsi
>
Tested for pocketbeagle2 connector [0]


[0]: https://github.com/Ayush1325/linux/tree/b4/beagle-cape


Tested-by: Ayush Singh <ayush@beagleboard.org>

[PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Herve Codina]

Hi,

At Linux Plumbers Conference 2024, we (me and Luca Ceresolli) talked
about issues we have with runtime hotplug on non-discoverable busses
with device tree overlays [1].

On our system, a base board has a connector and addon boards can be
connected to this connector. Both boards are described using device
tree. The base board is described by a base device tree and addon boards
are describe by overlays device tree. More details can be found at [2].

This kind of use case can be found also on:
  - Grove Sunlight Sensor [3]
  - mikroBUS [4]

One of the issue we were facing on was referencing resources available
on the base board device tree from the addon overlay device tree.

Using a nexus node [5] helps decoupling resources and avoid the
knowledge of the full base board from the overlay. Indeed, with nexus
node, the overlay need to know only about the nexus node itself.

For instance, suppose a connector where a GPIO is connected at PinA. On
the base board this GPIO is connected to the GPIO 12 of the SoC GPIO
controller.

The base board can describe this GPIO using a nexus node:
    soc_gpio: gpio-controller {
      #gpio-cells = <2>;
    };

    connector1: connector1 {
        /*
         * Nexus node for the GPIO available on the connector.
         * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
         * controller
         */
        #gpio-cells = <2>;
        gpio-map = <0 0 &soc_gpio 12 0>;
        gpio-map-mask = <0xf 0x0>;
        gpio-map-pass-thru = <0x0 0xf>;
    };

The connector pin A GPIO can be referenced using:
  <&connector1 0 GPIO_ACTIVE_HIGH>

This implies that the overlay needs to know about exact label that
references the connector. This label can be different on a different
board and so applying the overlay could failed even if it is used to
describe the exact same addon board. Further more, a given base board
can have several connectors where the exact same addon board can be
connected. In that case, the same overlay cannot be used on both
connector. Indeed, the connector labels have to be different.

The export-symbols node introduced by this current series solves this
issue.

The idea of export-symbols is to have something similar to the global
__symbols__ node but local to a specific node. Symbols listed in this
export-symbols are local and visible only when an overlay is applied on
a node having an export-symbols subnode.

Using export-symbols, our example becomes:
    soc_gpio: gpio-controller {
      #gpio-cells = <2>;
    };

    connector1: connector1 {
        /*
         * Nexus node for the GPIO available on the connector.
         * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
         * controller
         */
        #gpio-cells = <2>;
        gpio-map = <0 0 &soc_gpio 12 0>;
        gpio-map-mask = <0xf 0x0>;
        gpio-map-pass-thru = <0x0 0xf>;

        export-symbols {
          connector = <&connector1>;
        };
    };

With that export-symbols node, an overlay applied on connector1 node can
have the symbol named 'connector' resolved to connector1. Indeed, the
export-symbols node available at connector1 node is used when the
overlay is applied. If the overlay has an unresolved 'connector' symbol,
it will be resolved to connector1 thanks to export-symbols.

Our overlay using the nexus node can contains:
   node {
      foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
   };
It used the GPIO 0 from the connector it is applied on.

A board with two connectors can be described with:
    connector1: connector1 {
        ...
        export-symbols {
          connector = <&connector1>;
        };
    };

    connector2: connector2 {
        ...
        export-symbols {
          connector = <&connector2>;
        };
    };

In that case, the same overlay with unresolved 'connector' symbol can be
applied on both connectors and the correct symbol resolution (connector1
or connector2) will be done.

This current series add support for the export-symbols node feature:
  - Patch 1 describes the export-symbols binding
  - Patches 2 to 6 prepare and add the support for the export-symbols
    feature
  - Patch 7 adds an unittest for the export-symbols feature

Compare to the previous iteration, the series has been rebased on
top of v6.15-rc1 and an compilation issue in unittest has been fixed.

Also it is worth noting the work already done by Ayush Singh related to
this topic on other repositories:
  - Add export-symbols in device-tree specification
      [PATCH v3] Add new `export-symbols` node [6]

  - Support for export-symbols in the device tree compiler
      [PATCH 0/3] Allow specifying target node in fdtoverlay [7]
      [PATCH] checks: Add support for export-symbols [8]

Best regards,
Hervé

[1] https://lpc.events/event/18/contributions/1696/
[2] https://lore.kernel.org/lkml/20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com/
[3] https://lore.kernel.org/lkml/20240702164403.29067-1-afd@ti.com/
[4] https://lore.kernel.org/lkml/20240627-mikrobus-scratch-spi-v5-0-9e6c148bf5f0@beagleboard.org/
[5] https://github.com/devicetree-org/devicetree-specification/blob/v0.4/source/chapter2-devicetree-basics.rst#nexus-nodes-and-specifier-mapping
[6] https://lore.kernel.org/all/20250411-export-symbols-v3-1-f59368d97063@beagleboard.org/
[7] https://lore.kernel.org/all/20250313-fdtoverlay-target-v1-0-dd5924e12bd3@beagleboard.org/
[8] https://lore.kernel.org/all/20250110-export-symbols-v1-1-b6213fcd6c82@beagleboard.org/

Changes v1 -> v2
  v1: https://lore.kernel.org/all/20241209151830.95723-1-herve.codina@bootlin.com/

  - All patches
    Add 'Tested-by: Ayush Singh <ayush@beagleboard.org>'

  - Patch 1
    Update the 'patternProperties' regexp

  - Patch 2 and 4
    Fix conflicts due to the rebase on top of v6.15-rc1

  - Patch 5
    Fix a typo in commit log

  - Patch 7
    Fix a compilation issue detected by a kernel test robot

Herve Codina (7):
  dt-bindings: Add support for export-symbols node
  of: resolver: Introduce get_phandle_from_symbols_node()
  of: resolver: Add export_symbols in of_resolve_phandles() parameters
  of: resolver: Add support for the export symbols node
  of: overlay: Add export_symbols_name in of_overlay_fdt_apply()
    parameters
  of: overlay: Add support for the export symbols node
  of: unittest: Add tests for export symbols

 .../devicetree/bindings/export-symbols.yaml   | 43 +++++++++++
 drivers/misc/lan966x_pci.c                    |  3 +-
 drivers/of/of_kunit_helpers.c                 |  2 +-
 drivers/of/of_private.h                       |  2 +-
 drivers/of/overlay.c                          | 30 +++++++-
 drivers/of/resolver.c                         | 75 ++++++++++++++----
 drivers/of/unittest-data/Makefile             |  5 ++
 .../unittest-data/overlay_export_symbols.dtso | 15 ++++
 .../of/unittest-data/testcases_common.dtsi    |  1 +
 .../unittest-data/tests-export-symbols.dtsi   | 30 ++++++++
 drivers/of/unittest.c                         | 77 +++++++++++++++++--
 include/linux/of.h                            |  6 +-
 12 files changed, 258 insertions(+), 31 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/export-symbols.yaml
 create mode 100644 drivers/of/unittest-data/overlay_export_symbols.dtso
 create mode 100644 drivers/of/unittest-data/tests-export-symbols.dtsi

-- 
2.49.0

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

[Herve Codina]

An export-symbols node allows to export symbols for symbols resolution
performed when applying a device tree overlay.

When a device tree overlay is applied on a node having an export-symbols
node, symbols listed in the export-symbols node are used to resolve
undefined symbols referenced from the overlay.

This allows:
  - Referencing symbols from an device tree overlay without the need to
    know the full base board. Only the connector definition is needed.

  - Using the exact same overlay on several connectors available on a given
    board.

For instance, the following description is supported with the
export-symbols node:
 - Base device tree board A:
    ...
    foo_connector: connector1 {
        export-symbols {
           connector = <&foo_connector>;
        };
    };

    bar_connector: connector2 {
        export-symbols {
           connector = <&bar_connector>;
        };
    };
    ...

 - Base device tree board B:
    ...
    front_connector: addon-connector {
        export-symbols {
           connector = <&front_connector>;
        };
    };
    ...

 - Overlay describing an addon board the can be connected on connectors:
    ...
    node {
        ...
        connector = <&connector>;
        ...
    };
    ...

Thanks to the export-symbols node, the overlay can be applied on
connector1 or connector2 available on board A but also on
addon-connector available on board B.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Tested-by: Ayush Singh <ayush@beagleboard.org>
---
 .../devicetree/bindings/export-symbols.yaml   | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/export-symbols.yaml

diff --git a/Documentation/devicetree/bindings/export-symbols.yaml b/Documentation/devicetree/bindings/export-symbols.yaml
new file mode 100644
index 000000000000..0e404eff8937
--- /dev/null
+++ b/Documentation/devicetree/bindings/export-symbols.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/export-symbols.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Export symbols
+
+maintainers:
+  - Herve Codina <herve.codina@bootlin.com>
+
+description: |
+  An export-symbols node allows to export symbols for symbols resolution
+  performed when applying a device tree overlay.
+
+  When a device tree overlay is applied on a node having an export-symbols
+  node, symbols listed in the export-symbols node are used to resolve undefined
+  symbols referenced from the overlay.
+
+properties:
+  $nodename:
+    const: export-symbols
+
+patternProperties:
+  "^[a-zA-Z_]?[a-zA-Z0-9_]*$":
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      A symbol exported in the form <symbol_name>=<phandle>.
+
+additionalProperties: false
+
+examples:
+  - |
+    /*
+     * Allows 'connector' symbol used in a device-tree overlay to be resolved to
+     * connector0 when the device-tree overlay is applied on connector0 node.
+     */
+    connector0: connector0 {
+      export-symbols {
+        connector = <&connector0>;
+      };
+    };
+...
-- 
2.49.0

[PATCH 2/7] of: resolver: Introduce get_phandle_from_symbols_node()

[Herve Codina]

In order to simplify the introduction of the export symbols node
handling, group necessary operations done to get a phandle from the
__symbols__ node in this dedicated get_phandle_from_symbols_node()
function.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Tested-by: Ayush Singh <ayush@beagleboard.org>
---
 drivers/of/resolver.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 86424e145919..2c7a3b479b15 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -212,6 +212,28 @@ static int adjust_local_phandle_references(const struct device_node *local_fixup
 	return 0;
 }
 
+static int get_phandle_from_symbols_node(const struct device_node *tree_symbols,
+					 const char *symbol_name,
+					 phandle *phandle)
+{
+	struct device_node *refnode;
+	const char *refpath;
+	int err;
+
+	err = of_property_read_string(tree_symbols, symbol_name, &refpath);
+	if (err)
+		return err;
+
+	refnode = of_find_node_by_path(refpath);
+	if (!refnode)
+		return -ENOENT;
+
+	*phandle = refnode->phandle;
+	of_node_put(refnode);
+
+	return 0;
+}
+
 /**
  * of_resolve_phandles - Relocate and resolve overlay against live tree
  *
@@ -247,11 +269,9 @@ static int adjust_local_phandle_references(const struct device_node *local_fixup
  */
 int of_resolve_phandles(struct device_node *overlay)
 {
-	struct device_node *child, *refnode;
-	struct device_node *overlay_fixups;
+	struct device_node *child, *overlay_fixups;
 	struct device_node __free(device_node) *local_fixups = NULL;
 	struct property *prop;
-	const char *refpath;
 	phandle phandle, phandle_delta;
 	int err;
 
@@ -298,21 +318,14 @@ int of_resolve_phandles(struct device_node *overlay)
 		if (!of_prop_cmp(prop->name, "name"))
 			continue;
 
-		err = of_property_read_string(tree_symbols,
-				prop->name, &refpath);
+		err = get_phandle_from_symbols_node(tree_symbols, prop->name,
+						    &phandle);
 		if (err) {
-			pr_err("node label '%s' not found in live devicetree symbols table\n",
+			pr_err("node label '%s' not found or invalid in live devicetree symbols table\n",
 			       prop->name);
 			return err;
 		}
 
-		refnode = of_find_node_by_path(refpath);
-		if (!refnode)
-			return -ENOENT;
-
-		phandle = refnode->phandle;
-		of_node_put(refnode);
-
 		err = update_usages_of_a_phandle_reference(overlay, prop, phandle);
 		if (err)
 			break;
-- 
2.49.0

[PATCH 3/7] of: resolver: Add export_symbols in of_resolve_phandles() parameters

[Herve Codina]

In order to prepare the introduction of the export symbols node
handling, add a export_symbols parameter in of_resolve_phandles().

The export_symbols is the export symbols device tree node the resolver
will use for the overlay symbols resolution.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Tested-by: Ayush Singh <ayush@beagleboard.org>
---
 drivers/of/of_private.h | 2 +-
 drivers/of/overlay.c    | 2 +-
 drivers/of/resolver.c   | 9 +++++++--
 drivers/of/unittest.c   | 4 ++--
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index df0bb00349e0..cf68edb873ab 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -94,7 +94,7 @@ static inline void __of_detach_node_sysfs(struct device_node *np) {}
 #endif
 
 #if defined(CONFIG_OF_RESOLVE)
-int of_resolve_phandles(struct device_node *tree);
+int of_resolve_phandles(struct device_node *tree, const struct device_node *export_symbols);
 #endif
 
 void __of_phandle_cache_inv_entry(phandle handle);
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 1af6f52d0708..aa1b97e634aa 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -919,7 +919,7 @@ static int of_overlay_apply(struct overlay_changeset *ovcs,
 {
 	int ret = 0, ret_revert, ret_tmp;
 
-	ret = of_resolve_phandles(ovcs->overlay_root);
+	ret = of_resolve_phandles(ovcs->overlay_root, NULL);
 	if (ret)
 		goto out;
 
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 2c7a3b479b15..5c492711b21f 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -237,7 +237,8 @@ static int get_phandle_from_symbols_node(const struct device_node *tree_symbols,
 /**
  * of_resolve_phandles - Relocate and resolve overlay against live tree
  *
- * @overlay:	Pointer to devicetree overlay to relocate and resolve
+ * @overlay:		Pointer to devicetree overlay to relocate and resolve
+ * @export_symbols:	Pointer to devicetree export symbols node.
  *
  * Modify (relocate) values of local phandles in @overlay to a range that
  * does not conflict with the live expanded devicetree.  Update references
@@ -257,6 +258,10 @@ static int get_phandle_from_symbols_node(const struct device_node *tree_symbols,
  * corresponding to that symbol in the live tree.  Update the references in
  * the overlay with the phandle values in the live tree.
  *
+ * @export_symbols can be use in this references update. The resolver tries
+ * first to find a match in the @export_symbols. If not found, it uses the
+ * "__symbol__" node in the live tree.
+ *
  * @overlay must be detached.
  *
  * Resolving and applying @overlay to the live expanded devicetree must be
@@ -267,7 +272,7 @@ static int get_phandle_from_symbols_node(const struct device_node *tree_symbols,
  *
  * Return: %0 on success or a negative error value on error.
  */
-int of_resolve_phandles(struct device_node *overlay)
+int of_resolve_phandles(struct device_node *overlay, const struct device_node *export_symbols)
 {
 	struct device_node *child, *overlay_fixups;
 	struct device_node __free(device_node) *local_fixups = NULL;
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 64d301893af7..620237365566 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -2026,7 +2026,7 @@ static int __init unittest_data_add(void)
 	 */
 	of_overlay_mutex_lock();
 
-	rc = of_resolve_phandles(unittest_data_node);
+	rc = of_resolve_phandles(unittest_data_node, NULL);
 	if (rc) {
 		pr_err("%s: Failed to resolve phandles (rc=%i)\n", __func__, rc);
 		of_overlay_mutex_unlock();
@@ -3915,7 +3915,7 @@ static __init void of_unittest_overlay_high_level(void)
 	 * because kmalloc() was not yet available.
 	 */
 	of_overlay_mutex_lock();
-	of_resolve_phandles(overlay_base_root);
+	of_resolve_phandles(overlay_base_root, NULL);
 	of_overlay_mutex_unlock();
 
 
-- 
2.49.0

[PATCH 4/7] of: resolver: Add support for the export symbols node

[Herve Codina]

Symbols resolution done when an overlay is applied looks only at the
global __symbol__ node to resolve unknown symbols from the overlay (i.e
symbols listed in the overlay __fixups__ node).

In order to provide flexibilities and allow to define some additional
symbols visible only when an overlay is applied to a specific node,
introduce the export symbols node.

The export symbols node adds some additional symbols that can be used
in the symbols resolution. The resolver tries to match unresolved
symbols first using the export symbols node and, if a match is not
found, it tries to match using the global __symbol__ node.

Contrary to symbols available in the global __symbols__ node, symbols
listed in the export symbols node can be considered as local symbols.
Indeed, they can be changed depending on the node the overlay is going
to be applied to and are only visibible from the current recolver call.

Handle those additional symbols given by the export symbols node in the
symbols resolution.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Tested-by: Ayush Singh <ayush@beagleboard.org>
---
 drivers/of/resolver.c | 33 +++++++++++++++++++++++++++++----
 drivers/of/unittest.c |  4 ++--
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 5c492711b21f..c2b63cec1865 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -220,6 +220,9 @@ static int get_phandle_from_symbols_node(const struct device_node *tree_symbols,
 	const char *refpath;
 	int err;
 
+	if (!tree_symbols)
+		return -ENOENT;
+
 	err = of_property_read_string(tree_symbols, symbol_name, &refpath);
 	if (err)
 		return err;
@@ -234,6 +237,25 @@ static int get_phandle_from_symbols_node(const struct device_node *tree_symbols,
 	return 0;
 }
 
+static int get_phandle_from_export_node(const struct device_node *export_symbols,
+					const char *symbol_name,
+					phandle *phandle)
+{
+	struct device_node *refnode;
+
+	if (!export_symbols)
+		return -ENOENT;
+
+	refnode = of_parse_phandle(export_symbols, symbol_name, 0);
+	if (!refnode)
+		return -ENOENT;
+
+	*phandle = refnode->phandle;
+	of_node_put(refnode);
+
+	return 0;
+}
+
 /**
  * of_resolve_phandles - Relocate and resolve overlay against live tree
  *
@@ -312,7 +334,7 @@ int of_resolve_phandles(struct device_node *overlay, const struct device_node *e
 		return 0;
 
 	struct device_node __free(device_node) *tree_symbols = of_find_node_by_path("/__symbols__");
-	if (!tree_symbols) {
+	if (!tree_symbols && !export_symbols) {
 		pr_err("no symbols in root of device tree.\n");
 		return -EINVAL;
 	}
@@ -323,10 +345,13 @@ int of_resolve_phandles(struct device_node *overlay, const struct device_node *e
 		if (!of_prop_cmp(prop->name, "name"))
 			continue;
 
-		err = get_phandle_from_symbols_node(tree_symbols, prop->name,
-						    &phandle);
+		err = get_phandle_from_export_node(export_symbols, prop->name,
+						   &phandle);
+		if (err)
+			err = get_phandle_from_symbols_node(tree_symbols, prop->name,
+							    &phandle);
 		if (err) {
-			pr_err("node label '%s' not found or invalid in live devicetree symbols table\n",
+			pr_err("node label '%s' not found or invalid in live devicetree symbols or export tables\n",
 			       prop->name);
 			return err;
 		}
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 620237365566..658690fd6980 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -4146,7 +4146,7 @@ static __init void of_unittest_overlay_high_level(void)
 	/* ---  overlay_bad_unresolved  --- */
 
 	EXPECT_BEGIN(KERN_ERR,
-		     "OF: resolver: node label 'this_label_does_not_exist' not found in live devicetree symbols table");
+		     "OF: resolver: node label 'this_label_does_not_exist' not found or invalid in live devicetree symbols or export tables");
 	EXPECT_BEGIN(KERN_ERR,
 		     "OF: resolver: overlay phandle fixup failed: -22");
 
@@ -4156,7 +4156,7 @@ static __init void of_unittest_overlay_high_level(void)
 	EXPECT_END(KERN_ERR,
 		   "OF: resolver: overlay phandle fixup failed: -22");
 	EXPECT_END(KERN_ERR,
-		   "OF: resolver: node label 'this_label_does_not_exist' not found in live devicetree symbols table");
+		   "OF: resolver: node label 'this_label_does_not_exist' not found or invalid in live devicetree symbols or export tables");
 
 	return;
 
-- 
2.49.0

[PATCH 5/7] of: overlay: Add export_symbols_name in of_overlay_fdt_apply() parameters

[Herve Codina]

In order to prepare the introduction of the export symbols node
handling, add a export_symbols_name parameter in of_overlay_fdt_apply().

The export_symbols_name is the name of the export symbols subnode
available in the base node that will be used by the resolver to handle
export symbols resolution.

Having the name of the subnode in parameters instead of the subnode
itself avoids the use of an export symbol node that is not directly
related to the base node.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Tested-by: Ayush Singh <ayush@beagleboard.org>
---
 drivers/misc/lan966x_pci.c    | 3 ++-
 drivers/of/of_kunit_helpers.c | 2 +-
 drivers/of/overlay.c          | 7 ++++++-
 drivers/of/unittest.c         | 4 ++--
 include/linux/of.h            | 6 ++++--
 5 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/misc/lan966x_pci.c b/drivers/misc/lan966x_pci.c
index 9c79b58137e5..f05cb040ec69 100644
--- a/drivers/misc/lan966x_pci.c
+++ b/drivers/misc/lan966x_pci.c
@@ -128,7 +128,8 @@ static int lan966x_pci_load_overlay(struct lan966x_pci *data)
 	u32 dtbo_size = __dtbo_lan966x_pci_end - __dtbo_lan966x_pci_begin;
 	void *dtbo_start = __dtbo_lan966x_pci_begin;
 
-	return of_overlay_fdt_apply(dtbo_start, dtbo_size, &data->ovcs_id, dev_of_node(data->dev));
+	return of_overlay_fdt_apply(dtbo_start, dtbo_size, &data->ovcs_id,
+				    dev_of_node(data->dev), NULL);
 }
 
 static void lan966x_pci_unload_overlay(struct lan966x_pci *data)
diff --git a/drivers/of/of_kunit_helpers.c b/drivers/of/of_kunit_helpers.c
index 7b3ed5a382aa..476b43474168 100644
--- a/drivers/of/of_kunit_helpers.c
+++ b/drivers/of/of_kunit_helpers.c
@@ -56,7 +56,7 @@ int of_overlay_fdt_apply_kunit(struct kunit *test, void *overlay_fdt,
 		return -ENOMEM;
 
 	ret = of_overlay_fdt_apply(overlay_fdt, overlay_fdt_size,
-				   ovcs_id, NULL);
+				   ovcs_id, NULL, NULL);
 	if (ret)
 		return ret;
 
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index aa1b97e634aa..73ff38c41de2 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -968,6 +968,10 @@ static int of_overlay_apply(struct overlay_changeset *ovcs,
  * @overlay_fdt_size:	number of bytes in @overlay_fdt
  * @ret_ovcs_id:	pointer for returning created changeset id
  * @base:		pointer for the target node to apply overlay
+ * @export_symbols_name:
+ *			Name of the export symbol subnode of the @base node to
+ *			provide extra symbols. Those extra symbols are used in
+ *			the overlay symbols resolution.
  *
  * Creates and applies an overlay changeset.
  *
@@ -983,7 +987,8 @@ static int of_overlay_apply(struct overlay_changeset *ovcs,
  */
 
 int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
-			 int *ret_ovcs_id, const struct device_node *base)
+			 int *ret_ovcs_id, const struct device_node *base,
+			 const char *export_symbols_name)
 {
 	void *new_fdt;
 	void *new_fdt_align;
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 658690fd6980..11091b0176e0 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -3858,7 +3858,7 @@ static int __init overlay_data_apply(const char *overlay_name, int *ovcs_id)
 		pr_err("no overlay data for %s\n", overlay_name);
 
 	ret = of_overlay_fdt_apply(info->dtbo_begin, size, &info->ovcs_id,
-				   NULL);
+				   NULL, NULL);
 	if (ovcs_id)
 		*ovcs_id = info->ovcs_id;
 	if (ret < 0)
@@ -4198,7 +4198,7 @@ static int testdrv_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	}
 
 	size = info->dtbo_end - info->dtbo_begin;
-	ret = of_overlay_fdt_apply(info->dtbo_begin, size, &ovcs_id, dn);
+	ret = of_overlay_fdt_apply(info->dtbo_begin, size, &ovcs_id, dn, NULL);
 	of_node_put(dn);
 	if (ret)
 		return ret;
diff --git a/include/linux/of.h b/include/linux/of.h
index a62154aeda1b..d8e0dd210e09 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1749,7 +1749,8 @@ struct of_overlay_notify_data {
 #ifdef CONFIG_OF_OVERLAY
 
 int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
-			 int *ovcs_id, const struct device_node *target_base);
+			 int *ovcs_id, const struct device_node *target_base,
+			 const char *export_symbols_name);
 int of_overlay_remove(int *ovcs_id);
 int of_overlay_remove_all(void);
 
@@ -1759,7 +1760,8 @@ int of_overlay_notifier_unregister(struct notifier_block *nb);
 #else
 
 static inline int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
-				       int *ovcs_id, const struct device_node *target_base)
+				       int *ovcs_id, const struct device_node *target_base,
+				       const char *export_symbols_name)
 {
 	return -ENOTSUPP;
 }
-- 
2.49.0

[PATCH 6/7] of: overlay: Add support for the export symbols node

[Herve Codina]

The resolver already supports the use of the export symbol node.

Add support of the export symbol node feature in of_overlay_fdt_apply()
simply getting the export node from its name and passing it to the
resolver.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Tested-by: Ayush Singh <ayush@beagleboard.org>
---
 drivers/of/overlay.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 73ff38c41de2..7c55d39f89af 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -891,6 +891,10 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
  * of_overlay_apply() - Create and apply an overlay changeset
  * @ovcs:	overlay changeset
  * @base:	point to the target node to apply overlay
+ * @export_symbols_name:
+ *		Name of the export symbol subnode of the @base node to
+ *		provide extra symbols. Those extra symbols are used in
+ *		the overlay symbols resolution.
  *
  * Creates and applies an overlay changeset.
  *
@@ -915,11 +919,24 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
  */
 
 static int of_overlay_apply(struct overlay_changeset *ovcs,
-			    const struct device_node *base)
+			    const struct device_node *base,
+			    const char *export_symbols_name)
 {
+	struct device_node *export_symbols = NULL;
 	int ret = 0, ret_revert, ret_tmp;
 
-	ret = of_resolve_phandles(ovcs->overlay_root, NULL);
+	if (base && export_symbols_name) {
+		export_symbols = of_get_child_by_name(base, export_symbols_name);
+		if (!export_symbols) {
+			pr_err("overlay get export symbols '%s' from %pOF failed\n",
+			       export_symbols_name, base);
+			ret = -EINVAL;
+			goto out;
+		}
+	}
+
+	ret = of_resolve_phandles(ovcs->overlay_root, export_symbols);
+	of_node_put(export_symbols);
 	if (ret)
 		goto out;
 
@@ -1059,7 +1076,7 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
 	}
 	ovcs->overlay_mem = overlay_mem;
 
-	ret = of_overlay_apply(ovcs, base);
+	ret = of_overlay_apply(ovcs, base, export_symbols_name);
 	/*
 	 * If of_overlay_apply() error, calling free_overlay_changeset() may
 	 * result in a memory leak if the apply partly succeeded, so do NOT
-- 
2.49.0

[PATCH 7/7] of: unittest: Add tests for export symbols

[Herve Codina]

The export symbols feature allows to use some additional symbols
provided in an export symbols node to resolve overlay symbols.

Add tests to exercise the export symbols feature.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Tested-by: Ayush Singh <ayush@beagleboard.org>
---
 drivers/of/unittest-data/Makefile             |  5 ++
 .../unittest-data/overlay_export_symbols.dtso | 15 +++++
 .../of/unittest-data/testcases_common.dtsi    |  1 +
 .../unittest-data/tests-export-symbols.dtsi   | 30 +++++++++
 drivers/of/unittest.c                         | 65 +++++++++++++++++++
 5 files changed, 116 insertions(+)
 create mode 100644 drivers/of/unittest-data/overlay_export_symbols.dtso
 create mode 100644 drivers/of/unittest-data/tests-export-symbols.dtsi

diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index 01a966e39f23..b51be046749a 100644
--- a/drivers/of/unittest-data/Makefile
+++ b/drivers/of/unittest-data/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_OF_OVERLAY) += overlay.dtbo.o \
 			    overlay_gpio_04a.dtbo.o \
 			    overlay_gpio_04b.dtbo.o \
 			    overlay_pci_node.dtbo.o \
+			    overlay_export_symbols.dtbo.o \
 			    overlay_bad_unresolved.dtbo.o
 
 # enable creation of __symbols__ node
@@ -66,6 +67,10 @@ DTC_FLAGS_testcases += -Wno-interrupts_property \
 #			  overlay_bad_add_dup_prop.dtbo \
 #			  overlay_bad_phandle.dtbo \
 #			  overlay_bad_symbol.dtbo \
+#
+# Also overlay_export_symbols_ovl.dtbo is designed to be applied to a specific
+# node and cannot be applied statically with fdtoverlay
+
 
 apply_static_overlay_1 := overlay_0.dtbo \
 			  overlay_1.dtbo \
diff --git a/drivers/of/unittest-data/overlay_export_symbols.dtso b/drivers/of/unittest-data/overlay_export_symbols.dtso
new file mode 100644
index 000000000000..89c9df4ef89b
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_export_symbols.dtso
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+/ {
+	fragment@0 {
+		target-path="";
+		__overlay__ {
+			ovl_node {
+				ref-base = <&test_export_base>;
+				ref-node = <&test_export_node>;
+			};
+		};
+	};
+};
diff --git a/drivers/of/unittest-data/testcases_common.dtsi b/drivers/of/unittest-data/testcases_common.dtsi
index 1c2cdf353ae3..21ffe0fb03ef 100644
--- a/drivers/of/unittest-data/testcases_common.dtsi
+++ b/drivers/of/unittest-data/testcases_common.dtsi
@@ -18,4 +18,5 @@ node-remove {
 #include "tests-address.dtsi"
 #include "tests-platform.dtsi"
 #include "tests-overlay.dtsi"
+#include "tests-export-symbols.dtsi"
 #include "tests-lifecycle.dtsi"
diff --git a/drivers/of/unittest-data/tests-export-symbols.dtsi b/drivers/of/unittest-data/tests-export-symbols.dtsi
new file mode 100644
index 000000000000..1650289b34cd
--- /dev/null
+++ b/drivers/of/unittest-data/tests-export-symbols.dtsi
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/ {
+	testcase-data {
+		test-export-symbols {
+			test_export_symbols_b0: base0 {
+				test_export_symbols_n0: node {
+					dummy;
+				};
+
+				export-symbols {
+					test_export_base = <&test_export_symbols_b0>;
+					test_export_node = <&test_export_symbols_n0>;
+				};
+			};
+
+			test_export_symbols_b1: base1 {
+
+				test_export_symbols_n1: node {
+					dummy;
+				};
+
+				export-symbols {
+					test_export_base = <&test_export_symbols_b1>;
+					test_export_node = <&test_export_symbols_n1>;
+				};
+			};
+		};
+	};
+};
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 11091b0176e0..bf286902c65b 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -4164,6 +4164,69 @@ static __init void of_unittest_overlay_high_level(void)
 	mutex_unlock(&of_mutex);
 }
 
+OVERLAY_INFO_EXTERN(overlay_export_symbols);
+
+static __init void of_unittest_export_symbols(const char *prefix,
+					      const char *base_full_path)
+{
+	const struct overlay_info ovl = OVERLAY_INFO(overlay_export_symbols, 0, 0);
+	struct device_node *ovl_node;
+	struct device_node *base;
+	struct device_node *node;
+	struct device_node *ref;
+	int ovcs_id;
+	u32 size;
+	int ret;
+
+	base = of_find_node_by_path(base_full_path);
+	if (unittest(base, "%s: Get base (%s) failed\n", prefix, base_full_path))
+		return;
+
+	node = of_get_child_by_name(base, "node");
+	if (unittest(base, "%s: Get node from %pOF failed\n", prefix, base))
+		goto end_put_base;
+
+	size = ovl.dtbo_end - ovl.dtbo_begin;
+	ret = of_overlay_fdt_apply(ovl.dtbo_begin, size, &ovcs_id, base, "export-symbols");
+	if (unittest(!ret, "%s: Apply '%s' failed (%d)\n", prefix, ovl.name, ret))
+		goto end_put_node;
+
+	ovl_node = of_get_child_by_name(base, "ovl_node");
+	if (unittest(ovl_node, "%s: Get ovl_node from %pOF failed\n", prefix, base))
+		goto end_remove_overlay;
+
+	ref = of_parse_phandle(ovl_node, "ref-base", 0);
+	if (unittest(ref, "%s: Parse 'ref-base' from %pOF failed\n", prefix, ovl_node))
+		goto end_put_ovl_node;
+	unittest(ref == base,
+		 "%s: Node from 'ref-base' phandle mismatches (got %pOF, expected %pOF)\n",
+		 prefix, ref, base);
+	of_node_put(ref);
+
+	ref = of_parse_phandle(ovl_node, "ref-node", 0);
+	if (unittest(ref, "%s: Parse 'ref-node' from %pOF failed\n", prefix, ovl_node))
+		goto end_put_ovl_node;
+	unittest(ref == node,
+		 "%s: Node from 'ref-node' phandle mismatches (got %pOF, expected %pOF)\n",
+		 prefix, ref, node);
+	of_node_put(ref);
+
+end_put_ovl_node:
+	of_node_put(ovl_node);
+end_remove_overlay:
+	of_overlay_remove(&ovcs_id);
+end_put_node:
+	of_node_put(node);
+end_put_base:
+	of_node_put(base);
+}
+
+static __init void of_unittest_overlay_export_symbols(void)
+{
+	of_unittest_export_symbols("base0", "/testcase-data/test-export-symbols/base0");
+	of_unittest_export_symbols("base1", "/testcase-data/test-export-symbols/base1");
+}
+
 static int of_unittest_pci_dev_num;
 static int of_unittest_pci_child_num;
 
@@ -4349,6 +4412,7 @@ static void __init of_unittest_pci_node(void)
 #else
 
 static inline __init void of_unittest_overlay_high_level(void) {}
+static inline __init void of_unittest_overlay_export_symbols(void) {}
 static inline __init void of_unittest_pci_node(void) { }
 
 #endif
@@ -4404,6 +4468,7 @@ static int __init of_unittest(void)
 	of_unittest_overlay();
 	of_unittest_lifecycle();
 	of_unittest_pci_node();
+	of_unittest_overlay_export_symbols();
 
 	/* Double check linkage after removing testcase data */
 	of_unittest_check_tree_linkage();
-- 
2.49.0

Re: [PATCH 0/7] of: overlay: Add support for export-symbols node feature

[Herve Codina]

Hi all,

Sorry, this should be a v2 series.

I resend as v2.

Please ighnore this one.

Best regards,
Hervé


On Wed, 30 Apr 2025 14:48:59 +0200
Herve Codina <herve.codina@bootlin.com> wrote:

> Hi,
> 
> At Linux Plumbers Conference 2024, we (me and Luca Ceresolli) talked
> about issues we have with runtime hotplug on non-discoverable busses
> with device tree overlays [1].
> 
> On our system, a base board has a connector and addon boards can be
> connected to this connector. Both boards are described using device
> tree. The base board is described by a base device tree and addon boards
> are describe by overlays device tree. More details can be found at [2].
> 
> This kind of use case can be found also on:
>   - Grove Sunlight Sensor [3]
>   - mikroBUS [4]
> 
> One of the issue we were facing on was referencing resources available
> on the base board device tree from the addon overlay device tree.
> 
> Using a nexus node [5] helps decoupling resources and avoid the
> knowledge of the full base board from the overlay. Indeed, with nexus
> node, the overlay need to know only about the nexus node itself.
> 
> For instance, suppose a connector where a GPIO is connected at PinA. On
> the base board this GPIO is connected to the GPIO 12 of the SoC GPIO
> controller.
> 
> The base board can describe this GPIO using a nexus node:
>     soc_gpio: gpio-controller {
>       #gpio-cells = <2>;
>     };
> 
>     connector1: connector1 {
>         /*
>          * Nexus node for the GPIO available on the connector.
>          * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
>          * controller
>          */
>         #gpio-cells = <2>;
>         gpio-map = <0 0 &soc_gpio 12 0>;
>         gpio-map-mask = <0xf 0x0>;
>         gpio-map-pass-thru = <0x0 0xf>;
>     };
> 
> The connector pin A GPIO can be referenced using:
>   <&connector1 0 GPIO_ACTIVE_HIGH>
> 
> This implies that the overlay needs to know about exact label that
> references the connector. This label can be different on a different
> board and so applying the overlay could failed even if it is used to
> describe the exact same addon board. Further more, a given base board
> can have several connectors where the exact same addon board can be
> connected. In that case, the same overlay cannot be used on both
> connector. Indeed, the connector labels have to be different.
> 
> The export-symbols node introduced by this current series solves this
> issue.
> 
> The idea of export-symbols is to have something similar to the global
> __symbols__ node but local to a specific node. Symbols listed in this
> export-symbols are local and visible only when an overlay is applied on
> a node having an export-symbols subnode.
> 
> Using export-symbols, our example becomes:
>     soc_gpio: gpio-controller {
>       #gpio-cells = <2>;
>     };
> 
>     connector1: connector1 {
>         /*
>          * Nexus node for the GPIO available on the connector.
>          * GPIO 0 (Pin A GPIO) is connected to GPIO 12 of the SoC gpio
>          * controller
>          */
>         #gpio-cells = <2>;
>         gpio-map = <0 0 &soc_gpio 12 0>;
>         gpio-map-mask = <0xf 0x0>;
>         gpio-map-pass-thru = <0x0 0xf>;
> 
>         export-symbols {
>           connector = <&connector1>;
>         };
>     };
> 
> With that export-symbols node, an overlay applied on connector1 node can
> have the symbol named 'connector' resolved to connector1. Indeed, the
> export-symbols node available at connector1 node is used when the
> overlay is applied. If the overlay has an unresolved 'connector' symbol,
> it will be resolved to connector1 thanks to export-symbols.
> 
> Our overlay using the nexus node can contains:
>    node {
>       foo-gpio = <&connector 0 GPIO_ACTIVE_HIGH>;
>    };
> It used the GPIO 0 from the connector it is applied on.
> 
> A board with two connectors can be described with:
>     connector1: connector1 {
>         ...
>         export-symbols {
>           connector = <&connector1>;
>         };
>     };
> 
>     connector2: connector2 {
>         ...
>         export-symbols {
>           connector = <&connector2>;
>         };
>     };
> 
> In that case, the same overlay with unresolved 'connector' symbol can be
> applied on both connectors and the correct symbol resolution (connector1
> or connector2) will be done.
> 
> This current series add support for the export-symbols node feature:
>   - Patch 1 describes the export-symbols binding
>   - Patches 2 to 6 prepare and add the support for the export-symbols
>     feature
>   - Patch 7 adds an unittest for the export-symbols feature
> 
> Compare to the previous iteration, the series has been rebased on
> top of v6.15-rc1 and an compilation issue in unittest has been fixed.
> 
> Also it is worth noting the work already done by Ayush Singh related to
> this topic on other repositories:
>   - Add export-symbols in device-tree specification
>       [PATCH v3] Add new `export-symbols` node [6]
> 
>   - Support for export-symbols in the device tree compiler
>       [PATCH 0/3] Allow specifying target node in fdtoverlay [7]
>       [PATCH] checks: Add support for export-symbols [8]
> 
> Best regards,
> Hervé
> 
> [1] https://lpc.events/event/18/contributions/1696/
> [2] https://lore.kernel.org/lkml/20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com/
> [3] https://lore.kernel.org/lkml/20240702164403.29067-1-afd@ti.com/
> [4] https://lore.kernel.org/lkml/20240627-mikrobus-scratch-spi-v5-0-9e6c148bf5f0@beagleboard.org/
> [5] https://github.com/devicetree-org/devicetree-specification/blob/v0.4/source/chapter2-devicetree-basics.rst#nexus-nodes-and-specifier-mapping
> [6] https://lore.kernel.org/all/20250411-export-symbols-v3-1-f59368d97063@beagleboard.org/
> [7] https://lore.kernel.org/all/20250313-fdtoverlay-target-v1-0-dd5924e12bd3@beagleboard.org/
> [8] https://lore.kernel.org/all/20250110-export-symbols-v1-1-b6213fcd6c82@beagleboard.org/
> 
> Changes v1 -> v2
>   v1: https://lore.kernel.org/all/20241209151830.95723-1-herve.codina@bootlin.com/
> 
>   - All patches
>     Add 'Tested-by: Ayush Singh <ayush@beagleboard.org>'
> 
>   - Patch 1
>     Update the 'patternProperties' regexp
> 
>   - Patch 2 and 4
>     Fix conflicts due to the rebase on top of v6.15-rc1
> 
>   - Patch 5
>     Fix a typo in commit log
> 
>   - Patch 7
>     Fix a compilation issue detected by a kernel test robot
> 
> Herve Codina (7):
>   dt-bindings: Add support for export-symbols node
>   of: resolver: Introduce get_phandle_from_symbols_node()
>   of: resolver: Add export_symbols in of_resolve_phandles() parameters
>   of: resolver: Add support for the export symbols node
>   of: overlay: Add export_symbols_name in of_overlay_fdt_apply()
>     parameters
>   of: overlay: Add support for the export symbols node
>   of: unittest: Add tests for export symbols
> 
>  .../devicetree/bindings/export-symbols.yaml   | 43 +++++++++++
>  drivers/misc/lan966x_pci.c                    |  3 +-
>  drivers/of/of_kunit_helpers.c                 |  2 +-
>  drivers/of/of_private.h                       |  2 +-
>  drivers/of/overlay.c                          | 30 +++++++-
>  drivers/of/resolver.c                         | 75 ++++++++++++++----
>  drivers/of/unittest-data/Makefile             |  5 ++
>  .../unittest-data/overlay_export_symbols.dtso | 15 ++++
>  .../of/unittest-data/testcases_common.dtsi    |  1 +
>  .../unittest-data/tests-export-symbols.dtsi   | 30 ++++++++
>  drivers/of/unittest.c                         | 77 +++++++++++++++++--
>  include/linux/of.h                            |  6 +-
>  12 files changed, 258 insertions(+), 31 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/export-symbols.yaml
>  create mode 100644 drivers/of/unittest-data/overlay_export_symbols.dtso
>  create mode 100644 drivers/of/unittest-data/tests-export-symbols.dtsi
> 



-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com