devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/7] of: overlay: Add support for export-symbols node feature
@ 2025-04-30 12:51 Herve Codina
  2025-04-30 12:51 ` [PATCH v2 1/7] dt-bindings: Add support for export-symbols node Herve Codina
                   ` (6 more replies)
  0 siblings, 7 replies; 28+ messages in thread
From: Herve Codina @ 2025-04-30 12:51 UTC (permalink / raw)
  To: David Gibson, Andrew Davis, Ayush Singh, Geert Uytterhoeven,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Herve Codina,
	Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan
  Cc: devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-04-30 12:51 [PATCH v2 0/7] of: overlay: Add support for export-symbols node feature Herve Codina
@ 2025-04-30 12:51 ` Herve Codina
  2025-05-02 14:33   ` Luca Ceresoli
  2025-05-27 18:31   ` Krzysztof Kozlowski
  2025-04-30 12:51 ` [PATCH v2 2/7] of: resolver: Introduce get_phandle_from_symbols_node() Herve Codina
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 28+ messages in thread
From: Herve Codina @ 2025-04-30 12:51 UTC (permalink / raw)
  To: David Gibson, Andrew Davis, Ayush Singh, Geert Uytterhoeven,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Herve Codina,
	Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan
  Cc: devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

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


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 2/7] of: resolver: Introduce get_phandle_from_symbols_node()
  2025-04-30 12:51 [PATCH v2 0/7] of: overlay: Add support for export-symbols node feature Herve Codina
  2025-04-30 12:51 ` [PATCH v2 1/7] dt-bindings: Add support for export-symbols node Herve Codina
@ 2025-04-30 12:51 ` Herve Codina
  2025-04-30 12:51 ` [PATCH v2 3/7] of: resolver: Add export_symbols in of_resolve_phandles() parameters Herve Codina
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 28+ messages in thread
From: Herve Codina @ 2025-04-30 12:51 UTC (permalink / raw)
  To: David Gibson, Andrew Davis, Ayush Singh, Geert Uytterhoeven,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Herve Codina,
	Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan
  Cc: devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

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


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 3/7] of: resolver: Add export_symbols in of_resolve_phandles() parameters
  2025-04-30 12:51 [PATCH v2 0/7] of: overlay: Add support for export-symbols node feature Herve Codina
  2025-04-30 12:51 ` [PATCH v2 1/7] dt-bindings: Add support for export-symbols node Herve Codina
  2025-04-30 12:51 ` [PATCH v2 2/7] of: resolver: Introduce get_phandle_from_symbols_node() Herve Codina
@ 2025-04-30 12:51 ` Herve Codina
  2025-05-02 14:35   ` Luca Ceresoli
  2025-04-30 12:51 ` [PATCH v2 4/7] of: resolver: Add support for the export symbols node Herve Codina
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 28+ messages in thread
From: Herve Codina @ 2025-04-30 12:51 UTC (permalink / raw)
  To: David Gibson, Andrew Davis, Ayush Singh, Geert Uytterhoeven,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Herve Codina,
	Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan
  Cc: devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

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


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 4/7] of: resolver: Add support for the export symbols node
  2025-04-30 12:51 [PATCH v2 0/7] of: overlay: Add support for export-symbols node feature Herve Codina
                   ` (2 preceding siblings ...)
  2025-04-30 12:51 ` [PATCH v2 3/7] of: resolver: Add export_symbols in of_resolve_phandles() parameters Herve Codina
@ 2025-04-30 12:51 ` Herve Codina
  2025-04-30 12:51 ` [PATCH v2 5/7] of: overlay: Add export_symbols_name in of_overlay_fdt_apply() parameters Herve Codina
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 28+ messages in thread
From: Herve Codina @ 2025-04-30 12:51 UTC (permalink / raw)
  To: David Gibson, Andrew Davis, Ayush Singh, Geert Uytterhoeven,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Herve Codina,
	Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan
  Cc: devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

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


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 5/7] of: overlay: Add export_symbols_name in of_overlay_fdt_apply() parameters
  2025-04-30 12:51 [PATCH v2 0/7] of: overlay: Add support for export-symbols node feature Herve Codina
                   ` (3 preceding siblings ...)
  2025-04-30 12:51 ` [PATCH v2 4/7] of: resolver: Add support for the export symbols node Herve Codina
@ 2025-04-30 12:51 ` Herve Codina
  2025-05-02 14:40   ` Ayush Singh
  2025-04-30 12:51 ` [PATCH v2 6/7] of: overlay: Add support for the export symbols node Herve Codina
  2025-04-30 12:51 ` [PATCH v2 7/7] of: unittest: Add tests for export symbols Herve Codina
  6 siblings, 1 reply; 28+ messages in thread
From: Herve Codina @ 2025-04-30 12:51 UTC (permalink / raw)
  To: David Gibson, Andrew Davis, Ayush Singh, Geert Uytterhoeven,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Herve Codina,
	Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan
  Cc: devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

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


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 6/7] of: overlay: Add support for the export symbols node
  2025-04-30 12:51 [PATCH v2 0/7] of: overlay: Add support for export-symbols node feature Herve Codina
                   ` (4 preceding siblings ...)
  2025-04-30 12:51 ` [PATCH v2 5/7] of: overlay: Add export_symbols_name in of_overlay_fdt_apply() parameters Herve Codina
@ 2025-04-30 12:51 ` Herve Codina
  2025-04-30 12:51 ` [PATCH v2 7/7] of: unittest: Add tests for export symbols Herve Codina
  6 siblings, 0 replies; 28+ messages in thread
From: Herve Codina @ 2025-04-30 12:51 UTC (permalink / raw)
  To: David Gibson, Andrew Davis, Ayush Singh, Geert Uytterhoeven,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Herve Codina,
	Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan
  Cc: devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

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


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 7/7] of: unittest: Add tests for export symbols
  2025-04-30 12:51 [PATCH v2 0/7] of: overlay: Add support for export-symbols node feature Herve Codina
                   ` (5 preceding siblings ...)
  2025-04-30 12:51 ` [PATCH v2 6/7] of: overlay: Add support for the export symbols node Herve Codina
@ 2025-04-30 12:51 ` Herve Codina
  6 siblings, 0 replies; 28+ messages in thread
From: Herve Codina @ 2025-04-30 12:51 UTC (permalink / raw)
  To: David Gibson, Andrew Davis, Ayush Singh, Geert Uytterhoeven,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Herve Codina,
	Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan
  Cc: devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

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


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-04-30 12:51 ` [PATCH v2 1/7] dt-bindings: Add support for export-symbols node Herve Codina
@ 2025-05-02 14:33   ` Luca Ceresoli
  2025-05-27 18:31   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 28+ messages in thread
From: Luca Ceresoli @ 2025-05-02 14:33 UTC (permalink / raw)
  To: Herve Codina
  Cc: David Gibson, Andrew Davis, Ayush Singh, Geert Uytterhoeven,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Arnd Bergmann,
	Greg Kroah-Hartman, Saravana Kannan, devicetree,
	devicetree-compiler, linux-kernel, Thomas Petazzoni

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

> 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>

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 3/7] of: resolver: Add export_symbols in of_resolve_phandles() parameters
  2025-04-30 12:51 ` [PATCH v2 3/7] of: resolver: Add export_symbols in of_resolve_phandles() parameters Herve Codina
@ 2025-05-02 14:35   ` Luca Ceresoli
  2025-05-05  8:10     ` Herve Codina
  0 siblings, 1 reply; 28+ messages in thread
From: Luca Ceresoli @ 2025-05-02 14:35 UTC (permalink / raw)
  To: Herve Codina
  Cc: David Gibson, Andrew Davis, Ayush Singh, Geert Uytterhoeven,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Arnd Bergmann,
	Greg Kroah-Hartman, Saravana Kannan, devicetree,
	devicetree-compiler, linux-kernel, Thomas Petazzoni

Hello Hervé,

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

> 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>

[...]

> --- 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.

The rationale behind this logic is not clear to me. I'd have expected
instead this logic:

  if (export-symbols != NULL):
      match only in export-symbols
  else
      match only in __symbols__

following the idea that it's better to be strict when introducing
something, and possibly relax it later on.

As I see it, with the current logic if you use export-symbols but you
build dtbs with -@, you can still match a global label. export-symbols
should avoid that instead.

Let me know whether I'm missing something here (which is surely
possible).

Other than this, the series looks good to me. I'm not adding my R-by on
the implementation patches though, waiting for the above question to be
clarified.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 5/7] of: overlay: Add export_symbols_name in of_overlay_fdt_apply() parameters
  2025-04-30 12:51 ` [PATCH v2 5/7] of: overlay: Add export_symbols_name in of_overlay_fdt_apply() parameters Herve Codina
@ 2025-05-02 14:40   ` Ayush Singh
  2025-05-05  8:17     ` Herve Codina
  0 siblings, 1 reply; 28+ messages in thread
From: Ayush Singh @ 2025-05-02 14:40 UTC (permalink / raw)
  To: Herve Codina, David Gibson, Andrew Davis, Geert Uytterhoeven,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Arnd Bergmann,
	Greg Kroah-Hartman, Saravana Kannan
  Cc: devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

On 4/30/25 18:21, Herve Codina wrote:

> 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)

Do we really need the export-symbols node name to be configurable?


>   {
>   	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;
>   }


Ayush Singh


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 3/7] of: resolver: Add export_symbols in of_resolve_phandles() parameters
  2025-05-02 14:35   ` Luca Ceresoli
@ 2025-05-05  8:10     ` Herve Codina
  0 siblings, 0 replies; 28+ messages in thread
From: Herve Codina @ 2025-05-05  8:10 UTC (permalink / raw)
  To: Luca Ceresoli, Ayush Singh
  Cc: David Gibson, Andrew Davis, Geert Uytterhoeven, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Arnd Bergmann,
	Greg Kroah-Hartman, Saravana Kannan, devicetree,
	devicetree-compiler, linux-kernel, Thomas Petazzoni

Hi Luca,

On Fri, 2 May 2025 16:35:59 +0200
Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:

> Hello Hervé,
> 
> On Wed, 30 Apr 2025 14:51:47 +0200
> Herve Codina <herve.codina@bootlin.com> wrote:
> 
> > 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>  
> 
> [...]
> 
> > --- 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.  
> 
> The rationale behind this logic is not clear to me. I'd have expected
> instead this logic:
> 
>   if (export-symbols != NULL):
>       match only in export-symbols
>   else
>       match only in __symbols__
> 
> following the idea that it's better to be strict when introducing
> something, and possibly relax it later on.
> 
> As I see it, with the current logic if you use export-symbols but you
> build dtbs with -@, you can still match a global label. export-symbols
> should avoid that instead.
> 
> Let me know whether I'm missing something here (which is surely
> possible).

No, you don't miss anything, it was just a choice and I have chosen to be
not exclusive between export-symbols and __symbols__.

export-symbols is taken in priority and if the symbol is not found in
export-symbols, we try with __symbols__.

Maybe I should be stricter. I don't have any strong opinion about that.

Ayush, on fdtoverlay what is the choice done ?

If an export-symbols is present and a symbol needs to be resolved but this
symbol is not found in export-symbols, do you try to fing it in __symbols__
or do you simply abort the resolution with an error ?

Best regards,
Hervé

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 5/7] of: overlay: Add export_symbols_name in of_overlay_fdt_apply() parameters
  2025-05-02 14:40   ` Ayush Singh
@ 2025-05-05  8:17     ` Herve Codina
  0 siblings, 0 replies; 28+ messages in thread
From: Herve Codina @ 2025-05-05  8:17 UTC (permalink / raw)
  To: Ayush Singh
  Cc: David Gibson, Andrew Davis, Geert Uytterhoeven, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Arnd Bergmann,
	Greg Kroah-Hartman, Saravana Kannan, devicetree,
	devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

Hi Ayush,

On Fri, 2 May 2025 20:10:41 +0530
Ayush Singh <ayush@beagleboard.org> wrote:

> On 4/30/25 18:21, Herve Codina wrote:
> 
> > 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)  
> 
> Do we really need the export-symbols node name to be configurable?

Well, it depends on the export-symbols acceptance in device-tree spec or some
other global device-tree bindings.

If this export-symbols node is accepted globally, the name is not needed and
shouldn't be configurable.

If this node name can be changed from one node binding to an other, having it
configurable is interesting.

That said, according to your work done at higher level (device-tree spec), this
name tends to be global. If confirmed, I will remove the export_symbols_name
parameter in the next iteration and use 'export-symbols' for all cases.

Best regards,
Hervé

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-04-30 12:51 ` [PATCH v2 1/7] dt-bindings: Add support for export-symbols node Herve Codina
  2025-05-02 14:33   ` Luca Ceresoli
@ 2025-05-27 18:31   ` Krzysztof Kozlowski
  2025-05-28  7:59     ` Ayush Singh
  2025-05-28 16:57     ` Herve Codina
  1 sibling, 2 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-27 18:31 UTC (permalink / raw)
  To: Herve Codina, David Gibson, Andrew Davis, Ayush Singh,
	Geert Uytterhoeven, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan
  Cc: devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

On 30/04/2025 14:51, Herve Codina wrote:
> 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.


I have impression that this is being discussed in three places
simultaneously - here, DT spec and DT schema. I don't know how to solve
the multiplication, but I will keep answering here, because that's my part.

> 
> 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>;
>         };
>     };
>     ...

And what would this mean? Which symbol is exported - foo or bar?

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

<from my other reply in private>
Another problem is that the board DTS should not care about overlays. It
feels like breaking encapsulation and I cannot imagine now adding 1000
export-symbols, because every i2c, spi, mikrobus or PCI slot could have
an overlay applied.

You could argue that only few nodes will be exported like this, so only
real mikrobus connectors. Then I will argue: look at aliases. People
alias everything everywhere, not following the guidelines.

If we assume that such overlays are designed for specific boards, thus
there will be only one or two exported symbols not 1000, then what is
the benefit of export symbols comparing to referencing by full path?
</end from my other reply>

And with above assumption - such overlays designed per board - plus my
first point about duplicated exports:
connector = <&foo_connector>;
connector = <&bar_connector>;

why not exporting the symbol with the same name? E.g.:

     foo_connector: connector1 {
         export-symbols {
            whatever-property-style = <&foo_connector>;
         };
     };

and overlay:

     node {
         ...
         connector = <&foo_connector>;
         ...
     };

Or even annotation?

     foo_connector: connector1 __exported_symbol__ {
         ....
     };


     node {
         ...
         connector = <&foo_connector>;
         ...
     };

? This also answers my further question about DTS style (see at the bottom)

>     ...
> 
>  - 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>

I would argue you cannot test a binding, because testing here is part of
a process, but what do I know...


> ---
>  .../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_]*$":

This messes up with coding style which I would prefer keep intact.
Basically these properties will be using label style.

> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      A symbol exported in the form <symbol_name>=<phandle>.
> +
> +additionalProperties: false
> +
Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-05-27 18:31   ` Krzysztof Kozlowski
@ 2025-05-28  7:59     ` Ayush Singh
  2025-05-28  8:06       ` Krzysztof Kozlowski
  2025-05-28 16:57     ` Herve Codina
  1 sibling, 1 reply; 28+ messages in thread
From: Ayush Singh @ 2025-05-28  7:59 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Herve Codina, David Gibson, Andrew Davis,
	Geert Uytterhoeven, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan
  Cc: devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

On 5/28/25 00:01, Krzysztof Kozlowski wrote:

> On 30/04/2025 14:51, Herve Codina wrote:
>> 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.
>
> I have impression that this is being discussed in three places
> simultaneously - here, DT spec and DT schema. I don't know how to solve
> the multiplication, but I will keep answering here, because that's my part.
>
>> 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>;
>>          };
>>      };
>>      ...
> And what would this mean? Which symbol is exported - foo or bar?
>
>>   - Base device tree board B:
>>      ...
>>      front_connector: addon-connector {
>>          export-symbols {
>>             connector = <&front_connector>;
>>          };
>>      };
> <from my other reply in private>
> Another problem is that the board DTS should not care about overlays. It
> feels like breaking encapsulation and I cannot imagine now adding 1000
> export-symbols, because every i2c, spi, mikrobus or PCI slot could have
> an overlay applied.
>
> You could argue that only few nodes will be exported like this, so only
> real mikrobus connectors. Then I will argue: look at aliases. People
> alias everything everywhere, not following the guidelines.
>
> If we assume that such overlays are designed for specific boards, thus
> there will be only one or two exported symbols not 1000, then what is
> the benefit of export symbols comparing to referencing by full path?
> </end from my other reply>

Can you explain how referencing by full path will work in connector + 
addon board setups?

The full path will be dependent on the connector, which means the same 
addon  board overlay cannot work for different connectors.


>
> And with above assumption - such overlays designed per board - plus my
> first point about duplicated exports:
> connector = <&foo_connector>;
> connector = <&bar_connector>;
>
> why not exporting the symbol with the same name? E.g.:
>
>       foo_connector: connector1 {
>           export-symbols {
>              whatever-property-style = <&foo_connector>;
>           };
>       };
>
> and overlay:
>
>       node {
>           ...
>           connector = <&foo_connector>;
>           ...
>       };


Isn't this overlay tied to `foo_connector`, i.e. it cannot be used with 
`bar_connector`?


>
> Or even annotation?
>
>       foo_connector: connector1 __exported_symbol__ {
>           ....
>       };
>
>
>       node {
>           ...
>           connector = <&foo_connector>;
>           ...
>       };
>
> ? This also answers my further question about DTS style (see at the bottom)
>
>>      ...
>>
>>   - 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>
> I would argue you cannot test a binding, because testing here is part of
> a process, but what do I know...

Ahh, I added tested by for the whole patch series to check that the 
phandle resolution is working. But yes, I have not tested the bindings.


>
>
>> ---
>>   .../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_]*$":
> This messes up with coding style which I would prefer keep intact.
> Basically these properties will be using label style.
>
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description:
>> +      A symbol exported in the form <symbol_name>=<phandle>.
>> +
>> +additionalProperties: false
>> +
> Best regards,
> Krzysztof


Maybe the dt-schema discussion will give a better description of why I 
need export-symbols or something similar [0].

I have also been trying to move the discussion regarding global vs local 
scope overlay application for connectors here [1].


[0]: 
https://lore.kernel.org/devicetree-spec/20250411-export-symbols-v3-1-f59368d97063@beagleboard.org/T/#u

[1]: 
https://lore.kernel.org/linux-devicetree/9c326bb7-e09a-4c21-944f-006b3fad1870@beagleboard.org/


Best Regards,

Ayush Sigh


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-05-28  7:59     ` Ayush Singh
@ 2025-05-28  8:06       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-28  8:06 UTC (permalink / raw)
  To: Ayush Singh, Herve Codina, David Gibson, Andrew Davis,
	Geert Uytterhoeven, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan
  Cc: devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

On 28/05/2025 09:59, Ayush Singh wrote:
> On 5/28/25 00:01, Krzysztof Kozlowski wrote:
> 
>> On 30/04/2025 14:51, Herve Codina wrote:
>>> 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.
>>
>> I have impression that this is being discussed in three places
>> simultaneously - here, DT spec and DT schema. I don't know how to solve
>> the multiplication, but I will keep answering here, because that's my part.
>>
>>> 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>;
>>>          };
>>>      };
>>>      ...
>> And what would this mean? Which symbol is exported - foo or bar?
>>
>>>   - Base device tree board B:
>>>      ...
>>>      front_connector: addon-connector {
>>>          export-symbols {
>>>             connector = <&front_connector>;
>>>          };
>>>      };
>> <from my other reply in private>
>> Another problem is that the board DTS should not care about overlays. It
>> feels like breaking encapsulation and I cannot imagine now adding 1000
>> export-symbols, because every i2c, spi, mikrobus or PCI slot could have
>> an overlay applied.
>>
>> You could argue that only few nodes will be exported like this, so only
>> real mikrobus connectors. Then I will argue: look at aliases. People
>> alias everything everywhere, not following the guidelines.
>>
>> If we assume that such overlays are designed for specific boards, thus
>> there will be only one or two exported symbols not 1000, then what is
>> the benefit of export symbols comparing to referencing by full path?
>> </end from my other reply>
> 
> Can you explain how referencing by full path will work in connector + 
> addon board setups?
> 
> The full path will be dependent on the connector, which means the same 
> addon  board overlay cannot work for different connectors.

That was the assumption.

> 
> 
>>
>> And with above assumption - such overlays designed per board - plus my
>> first point about duplicated exports:
>> connector = <&foo_connector>;
>> connector = <&bar_connector>;
>>
>> why not exporting the symbol with the same name? E.g.:
>>
>>       foo_connector: connector1 {
>>           export-symbols {
>>              whatever-property-style = <&foo_connector>;
>>           };
>>       };
>>
>> and overlay:
>>
>>       node {
>>           ...
>>           connector = <&foo_connector>;
>>           ...
>>       };
> 
> 
> Isn't this overlay tied to `foo_connector`, i.e. it cannot be used with 
> `bar_connector`?

I don't know what the example tried to say. I asked the question, so
don't ask question to my question. We both apparently do not know. What
is the meaning of that example I questioned? To which connector this is
an overlay? Which symbol is exported for that example?

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-05-27 18:31   ` Krzysztof Kozlowski
  2025-05-28  7:59     ` Ayush Singh
@ 2025-05-28 16:57     ` Herve Codina
  2025-06-04 18:35       ` Krzysztof Kozlowski
  1 sibling, 1 reply; 28+ messages in thread
From: Herve Codina @ 2025-05-28 16:57 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: David Gibson, Andrew Davis, Ayush Singh, Geert Uytterhoeven,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Arnd Bergmann,
	Greg Kroah-Hartman, Saravana Kannan, devicetree,
	devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

Hi Krzysztof,

Thanks a lot for your feedback!

On Tue, 27 May 2025 20:31:14 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 30/04/2025 14:51, Herve Codina wrote:
> > 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.  
> 
> 
> I have impression that this is being discussed in three places
> simultaneously - here, DT spec and DT schema. I don't know how to solve
> the multiplication, but I will keep answering here, because that's my part.
> 
> > 
> > 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>;
> >         };
> >     };
> >     ...  
> 
> And what would this mean? Which symbol is exported - foo or bar?

Symbols are exported only when an overlay is applied on the node where the
export-symbols node is available. Those symbols are visible only from the
overlay applied. Symbols exported thanks to export-symbols are not global
to the all device-tree (it is not __symbols__) but local to a node.

If an overlay is applied at connector1 node, it can use the 'connector'
symbols and thanks to export-symbols, the 'connector' symbol will be
resolved to foo_connector.

If the overlay is applied at connector2 node, the 'connector' symbol is then
resolved to bar_connector.

> 
> > 
> >  - Base device tree board B:
> >     ...
> >     front_connector: addon-connector {
> >         export-symbols {
> >            connector = <&front_connector>;
> >         };
> >     };  
> 
> <from my other reply in private>
> Another problem is that the board DTS should not care about overlays. It
> feels like breaking encapsulation and I cannot imagine now adding 1000
> export-symbols, because every i2c, spi, mikrobus or PCI slot could have
> an overlay applied.

The board DTS don't care about overlay itself.
It describes a connector where an overlay can be applied.

At this connector, several hardware resources are wired and need to be
referenced. The baord DTS offer a way for something else to reference
those resources.

The goal is to have a connector where it is allowed to apply an overlay
instead of exporting all possible symbols and allowing overlays to add
nodes anywhere.

> 
> You could argue that only few nodes will be exported like this, so only
> real mikrobus connectors. Then I will argue: look at aliases. People
> alias everything everywhere, not following the guidelines.

Those connector nodes have a compatible string and a binding.
The export-symbols will be part of the binding.

> 
> If we assume that such overlays are designed for specific boards, thus
> there will be only one or two exported symbols not 1000, then what is
> the benefit of export symbols comparing to referencing by full path?

The overlay is not "designed" for a specific board but for a "specific"
connector.
He described an add-on board connected to a specific connector, specific
in terms of pinout. I mean an I2C bus is available at connector pin 1 and 2,
a GPIO, is available at connector pin 3, an irq line is available at
connector pin 4, ...

Every board that offer this connector (pinout compatible) can have the
overlay applied to the connector.

Also a board can offer several connectors with same pinout and then an
add-on board compatible with this connector pinout can be plugged on each
connector. The overlay described the add-on board and should be the same
whatever if is is applied at connector A or connector B.
The exact same dtbo should work.

> </end from my other reply>
> 
> And with above assumption - such overlays designed per board - plus my
> first point about duplicated exports:
> connector = <&foo_connector>;
> connector = <&bar_connector>;
> 
> why not exporting the symbol with the same name? E.g.:
> 
>      foo_connector: connector1 {
>          export-symbols {
>             whatever-property-style = <&foo_connector>;
>          };
>      };
> 
> and overlay:
> 
>      node {
>          ...
>          connector = <&foo_connector>;
>          ...
>      };

With that, the overlay is designed for the board and not the connector.
The exact same overlay cannot be use on different connectors (pinout
compatible) available on one board. It cannot also be used on different
board which provide the exact same connector (pinout compatible).

> 
> Or even annotation?
> 
>      foo_connector: connector1 __exported_symbol__ {
>          ....
>      };
> 
> 
>      node {
>          ...
>          connector = <&foo_connector>;
>          ...
>      };

If you board has 2 connectors:

 foo_connector1: connector1 __exported_symbol__ {...};
 foo_connector2: connector2 __exported_symbol__ {...};

your overlay need to be specific. The overlay shouldn't depend on the board
is going to be applied. The only knowledge from the overlay point of view is
the connector.

> 
> ? This also answers my further question about DTS style (see at the bottom)
> 
> >     ...
> > 
> >  - 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>  
> 
> I would argue you cannot test a binding, because testing here is part of
> a process, but what do I know...
> 
> 
> > ---
> >  .../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_]*$":  
> 
> This messes up with coding style which I would prefer keep intact.
> Basically these properties will be using label style.

Yes, those properties remap phandles.

Their names are the name of the label used from the overlay and their
values are the phandle mapped.

You already have this kind properties using label style in __symbols__,
__fixups__, __local_fixups__ nodes.

Those node are globals to the dtb or the dtbo.

export-symbols node has a finer grain. It is a child of a specific node
describing a hardware connector and should be described in the binding.

> 
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      A symbol exported in the form <symbol_name>=<phandle>.
> > +
> > +additionalProperties: false
> > +  
> Best regards,
> Krzysztof

Again, thanks for your feedback.
I hope we could move forward on this export-symbols topic.

Best regards,
Hervé

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-05-28 16:57     ` Herve Codina
@ 2025-06-04 18:35       ` Krzysztof Kozlowski
  2025-06-18  9:32         ` Herve Codina
  0 siblings, 1 reply; 28+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-04 18:35 UTC (permalink / raw)
  To: Herve Codina
  Cc: David Gibson, Andrew Davis, Ayush Singh, Geert Uytterhoeven,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Arnd Bergmann,
	Greg Kroah-Hartman, Saravana Kannan, devicetree,
	devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

On 28/05/2025 18:57, Herve Codina wrote:
> Hi Krzysztof,
> 
> Thanks a lot for your feedback!
> 
> On Tue, 27 May 2025 20:31:14 +0200
> Krzysztof Kozlowski <krzk@kernel.org> wrote:
> 
>> On 30/04/2025 14:51, Herve Codina wrote:
>>> 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.  
>>
>>
>> I have impression that this is being discussed in three places
>> simultaneously - here, DT spec and DT schema. I don't know how to solve
>> the multiplication, but I will keep answering here, because that's my part.
>>
>>>
>>> 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>;
>>>         };
>>>     };
>>>     ...  
>>
>> And what would this mean? Which symbol is exported - foo or bar?
> 
> Symbols are exported only when an overlay is applied on the node where the
> export-symbols node is available. Those symbols are visible only from the
> overlay applied. Symbols exported thanks to export-symbols are not global
> to the all device-tree (it is not __symbols__) but local to a node.
> 
> If an overlay is applied at connector1 node, it can use the 'connector'
> symbols and thanks to export-symbols, the 'connector' symbol will be
> resolved to foo_connector.
> 
> If the overlay is applied at connector2 node, the 'connector' symbol is then
> resolved to bar_connector.

OK, this explains a lot. Unless I missed it, would be nice to include it
in binding description.


...



...

>>> +patternProperties:
>>> +  "^[a-zA-Z_]?[a-zA-Z0-9_]*$":  
>>
>> This messes up with coding style which I would prefer keep intact.
>> Basically these properties will be using label style.
> 
> Yes, those properties remap phandles.
> 
> Their names are the name of the label used from the overlay and their
> values are the phandle mapped.
> 
> You already have this kind properties using label style in __symbols__,
> __fixups__, __local_fixups__ nodes.

I have them in DTB, but I don't have these in DTS. The exported-symbols
would be in the DTS and that is what coding style is about.



Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-06-04 18:35       ` Krzysztof Kozlowski
@ 2025-06-18  9:32         ` Herve Codina
  2025-06-18  9:54           ` Ayush Singh
  2025-08-17  7:38           ` Krzysztof Kozlowski
  0 siblings, 2 replies; 28+ messages in thread
From: Herve Codina @ 2025-06-18  9:32 UTC (permalink / raw)
  To: Krzysztof Kozlowski, David Gibson, Ayush Singh, Rob Herring
  Cc: Andrew Davis, Geert Uytterhoeven, Krzysztof Kozlowski,
	Conor Dooley, Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan,
	devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

Hi Krzysztof,

On Wed, 4 Jun 2025 20:35:51 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:

...

> > 
> > Symbols are exported only when an overlay is applied on the node where the
> > export-symbols node is available. Those symbols are visible only from the
> > overlay applied. Symbols exported thanks to export-symbols are not global
> > to the all device-tree (it is not __symbols__) but local to a node.
> > 
> > If an overlay is applied at connector1 node, it can use the 'connector'
> > symbols and thanks to export-symbols, the 'connector' symbol will be
> > resolved to foo_connector.
> > 
> > If the overlay is applied at connector2 node, the 'connector' symbol is then
> > resolved to bar_connector.  
> 
> OK, this explains a lot. Unless I missed it, would be nice to include it
> in binding description.

Sure, I will add something in the next iteration.

...

> >>> +patternProperties:
> >>> +  "^[a-zA-Z_]?[a-zA-Z0-9_]*$":    
> >>
> >> This messes up with coding style which I would prefer keep intact.
> >> Basically these properties will be using label style.  
> > 
> > Yes, those properties remap phandles.
> > 
> > Their names are the name of the label used from the overlay and their
> > values are the phandle mapped.
> > 
> > You already have this kind properties using label style in __symbols__,
> > __fixups__, __local_fixups__ nodes.  
> 
> I have them in DTB, but I don't have these in DTS. The exported-symbols
> would be in the DTS and that is what coding style is about.
> 

I think export-symbols has to be in DTS.
Maybe it could be described in an other way in order to avoid the coding style
issue you reported.

Hardware:
  i2c0 from SoC --------- connector 1, I2C A signals
  i2c1 from SoC --------- connector 1, I2C B signals

  connector1 {
      export-symbols {
	  i2c_a = <&i2c0>;
	  i2c_b = <&i2c1>;
      };
  };

In order to avoid the coding style issue, this could be replace
with:
 connector1 {
      export-symbols {
	  symbol-names = "i2c_a", "i2c_b";
	  symbols = <&i2c0>, <&i2c1>;
      };
  };

Krzysztof, Rob, do you think this could be accepted ?

Ayush, David, do you thing this could be easily implemented in fdtoverlay ?

Best regards,
Hervé


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-06-18  9:32         ` Herve Codina
@ 2025-06-18  9:54           ` Ayush Singh
  2025-07-04  9:10             ` Herve Codina
  2025-08-17  7:41             ` Krzysztof Kozlowski
  2025-08-17  7:38           ` Krzysztof Kozlowski
  1 sibling, 2 replies; 28+ messages in thread
From: Ayush Singh @ 2025-06-18  9:54 UTC (permalink / raw)
  To: Herve Codina, Krzysztof Kozlowski, David Gibson, Rob Herring
  Cc: Andrew Davis, Geert Uytterhoeven, Krzysztof Kozlowski,
	Conor Dooley, Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan,
	devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni


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

Well, it is possible.

However, on connectors like pb2 header, there will be 50-100 export 
symbols. So it will start becoming difficult to maintain.

Additionally, the further away we move from __symbols__ style, the more 
difficult the implementation will become since we can currently very 
easily piggy-back on __symbols__ resolution implementation.


Best Regards,

Ayush Singh


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-06-18  9:54           ` Ayush Singh
@ 2025-07-04  9:10             ` Herve Codina
  2025-08-17  7:41             ` Krzysztof Kozlowski
  1 sibling, 0 replies; 28+ messages in thread
From: Herve Codina @ 2025-07-04  9:10 UTC (permalink / raw)
  To: Ayush Singh
  Cc: Krzysztof Kozlowski, David Gibson, Rob Herring, Andrew Davis,
	Geert Uytterhoeven, Krzysztof Kozlowski, Conor Dooley,
	Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan, devicetree,
	devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

Hi Krzysztof, David, Rob,

Any opinion?

Best regards,
Hervé

On Wed, 18 Jun 2025 15:24:07 +0530
Ayush Singh <ayush@beagleboard.org> wrote:

> On 6/18/25 15:02, Herve Codina wrote:
...

> >  
> >>>>> +patternProperties:
> >>>>> +  "^[a-zA-Z_]?[a-zA-Z0-9_]*$":  
> >>>> This messes up with coding style which I would prefer keep intact.
> >>>> Basically these properties will be using label style.  
> >>> Yes, those properties remap phandles.
> >>>
> >>> Their names are the name of the label used from the overlay and their
> >>> values are the phandle mapped.
> >>>
> >>> You already have this kind properties using label style in __symbols__,
> >>> __fixups__, __local_fixups__ nodes.  
> >> I have them in DTB, but I don't have these in DTS. The exported-symbols
> >> would be in the DTS and that is what coding style is about.
> >>  
> > I think export-symbols has to be in DTS.
> > Maybe it could be described in an other way in order to avoid the coding style
> > issue you reported.
> >
> > Hardware:
> >    i2c0 from SoC --------- connector 1, I2C A signals
> >    i2c1 from SoC --------- connector 1, I2C B signals
> >
> >    connector1 {
> >        export-symbols {
> > 	  i2c_a = <&i2c0>;
> > 	  i2c_b = <&i2c1>;
> >        };
> >    };
> >
> > In order to avoid the coding style issue, this could be replace
> > with:
> >   connector1 {
> >        export-symbols {
> > 	  symbol-names = "i2c_a", "i2c_b";
> > 	  symbols = <&i2c0>, <&i2c1>;
> >        };
> >    };
> >
> > Krzysztof, Rob, do you think this could be accepted ?
> >
> > Ayush, David, do you thing this could be easily implemented in fdtoverlay ?
> >
> > Best regards,
> > Hervé
> >  
> 
> Well, it is possible.
> 
> However, on connectors like pb2 header, there will be 50-100 export 
> symbols. So it will start becoming difficult to maintain.
> 
> Additionally, the further away we move from __symbols__ style, the more 
> difficult the implementation will become since we can currently very 
> easily piggy-back on __symbols__ resolution implementation.
> 
> 
> Best Regards,
> 
> Ayush Singh
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-06-18  9:32         ` Herve Codina
  2025-06-18  9:54           ` Ayush Singh
@ 2025-08-17  7:38           ` Krzysztof Kozlowski
  1 sibling, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-17  7:38 UTC (permalink / raw)
  To: Herve Codina, David Gibson, Ayush Singh, Rob Herring
  Cc: Andrew Davis, Geert Uytterhoeven, Krzysztof Kozlowski,
	Conor Dooley, Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan,
	devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

On 18/06/2025 11:32, Herve Codina wrote:
> 
> In order to avoid the coding style issue, this could be replace
> with:
>  connector1 {
>       export-symbols {
> 	  symbol-names = "i2c_a", "i2c_b";
> 	  symbols = <&i2c0>, <&i2c1>;
>       };
>   };
> 
> Krzysztof, Rob, do you think this could be accepted ?
> 
> Ayush, David, do you thing this could be easily implemented in fdtoverlay ?


This is fine with me.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-06-18  9:54           ` Ayush Singh
  2025-07-04  9:10             ` Herve Codina
@ 2025-08-17  7:41             ` Krzysztof Kozlowski
  2025-08-17  8:18               ` Ayush Singh
  1 sibling, 1 reply; 28+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-17  7:41 UTC (permalink / raw)
  To: Ayush Singh, Herve Codina, David Gibson, Rob Herring
  Cc: Andrew Davis, Geert Uytterhoeven, Krzysztof Kozlowski,
	Conor Dooley, Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan,
	devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

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


And the first syntax solves this how? I don't see the practical difference.

> 
> Additionally, the further away we move from __symbols__ style, the more 
> difficult the implementation will become since we can currently very 
> easily piggy-back on __symbols__ resolution implementation.


I care more how I read DTS, so complexity in dtc is less important to me
than consistent DTS style.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-08-17  7:41             ` Krzysztof Kozlowski
@ 2025-08-17  8:18               ` Ayush Singh
  2025-08-17  8:22                 ` Krzysztof Kozlowski
  0 siblings, 1 reply; 28+ messages in thread
From: Ayush Singh @ 2025-08-17  8:18 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Herve Codina, David Gibson, Rob Herring
  Cc: Andrew Davis, Geert Uytterhoeven, Krzysztof Kozlowski,
	Conor Dooley, Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan,
	devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

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

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


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

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

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-08-17  8:18               ` Ayush Singh
@ 2025-08-17  8:22                 ` Krzysztof Kozlowski
  2025-08-17  8:42                   ` Ayush Singh
  0 siblings, 1 reply; 28+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-17  8:22 UTC (permalink / raw)
  To: Ayush Singh, Herve Codina, David Gibson, Rob Herring
  Cc: Andrew Davis, Geert Uytterhoeven, Krzysztof Kozlowski,
	Conor Dooley, Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan,
	devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

On 17/08/2025 10:18, Ayush Singh wrote:
>>>>
>>>> Hardware:
>>>>     i2c0 from SoC --------- connector 1, I2C A signals
>>>>     i2c1 from SoC --------- connector 1, I2C B signals
>>>>
>>>>     connector1 {
>>>>         export-symbols {
>>>> 	  i2c_a = <&i2c0>;
>>>> 	  i2c_b = <&i2c1>;
>>>>         };
>>>>     };
>>>>
>>>> In order to avoid the coding style issue, this could be replace
>>>> with:
>>>>    connector1 {
>>>>         export-symbols {
>>>> 	  symbol-names = "i2c_a", "i2c_b";
>>>> 	  symbols = <&i2c0>, <&i2c1>;
>>>>         };
>>>>     };
>>>>
>>>> Krzysztof, Rob, do you think this could be accepted ?
>>>>
>>>> Ayush, David, do you thing this could be easily implemented in fdtoverlay ?
>>>>
>>>> Best regards,
>>>> Hervé
>>>>
>>> Well, it is possible.
>>>
>>> However, on connectors like pb2 header, there will be 50-100 export
>>> symbols. So it will start becoming difficult to maintain.
>>
>> And the first syntax solves this how? I don't see the practical difference.
> 
> 
> Well, I was more worried about matching which phandle belongs to which 
> symbol easily. Let us assume that 2 symbols will be in each line (after 
> accounting for the indention and 80 char limit) and we have 70 symbols, 
> so 35 lines. To check which phandle belongs to the 2nd symbol on line 
> 25th line of  symbol-names, well, you would at the best case need to 
> have something like relative line numbers in your editor. Then you know 
> that the 35th line from the current one is where you need to look.
> 
> In the current syntax, the symbol name and phandle are on the same line. 
> So well, easy to see which symbols refers to which phandle.

OK, that's valid point. Any ideas how to solve it without introducing
underscores for properties?

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-08-17  8:22                 ` Krzysztof Kozlowski
@ 2025-08-17  8:42                   ` Ayush Singh
  2025-08-18 17:05                     ` Rob Herring
  0 siblings, 1 reply; 28+ messages in thread
From: Ayush Singh @ 2025-08-17  8:42 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Herve Codina, David Gibson, Rob Herring
  Cc: Andrew Davis, Geert Uytterhoeven, Krzysztof Kozlowski,
	Conor Dooley, Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan,
	devicetree, devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

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

> On 17/08/2025 10:18, Ayush Singh wrote:
>>>>> Hardware:
>>>>>      i2c0 from SoC --------- connector 1, I2C A signals
>>>>>      i2c1 from SoC --------- connector 1, I2C B signals
>>>>>
>>>>>      connector1 {
>>>>>          export-symbols {
>>>>> 	  i2c_a = <&i2c0>;
>>>>> 	  i2c_b = <&i2c1>;
>>>>>          };
>>>>>      };
>>>>>
>>>>> In order to avoid the coding style issue, this could be replace
>>>>> with:
>>>>>     connector1 {
>>>>>          export-symbols {
>>>>> 	  symbol-names = "i2c_a", "i2c_b";
>>>>> 	  symbols = <&i2c0>, <&i2c1>;
>>>>>          };
>>>>>      };
>>>>>
>>>>> Krzysztof, Rob, do you think this could be accepted ?
>>>>>
>>>>> Ayush, David, do you thing this could be easily implemented in fdtoverlay ?
>>>>>
>>>>> Best regards,
>>>>> Hervé
>>>>>
>>>> Well, it is possible.
>>>>
>>>> However, on connectors like pb2 header, there will be 50-100 export
>>>> symbols. So it will start becoming difficult to maintain.
>>> And the first syntax solves this how? I don't see the practical difference.
>>
>> Well, I was more worried about matching which phandle belongs to which
>> symbol easily. Let us assume that 2 symbols will be in each line (after
>> accounting for the indention and 80 char limit) and we have 70 symbols,
>> so 35 lines. To check which phandle belongs to the 2nd symbol on line
>> 25th line of  symbol-names, well, you would at the best case need to
>> have something like relative line numbers in your editor. Then you know
>> that the 35th line from the current one is where you need to look.
>>
>> In the current syntax, the symbol name and phandle are on the same line.
>> So well, easy to see which symbols refers to which phandle.
> OK, that's valid point. Any ideas how to solve it without introducing
> underscores for properties?
>
> Best regards,
> Krzysztof


Well, we can modify `get_phandle_from_symbols_node` to allow matching 
`*_*` to `*-*`. And we can do the same in devicetree easily enough. Not 
sure if implicit loose matching like that are the best idea.

Zephyr does something similar for compatible strings. It pretty much 
replaces the all non alphanumeric characters with `_` in compatible 
string match. Although that is more to do with the limitation they are 
working with, i.e. the devicetree being converted to static headers 
instead of being runtime thing.

Best Regards,

Ayush Singh


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-08-17  8:42                   ` Ayush Singh
@ 2025-08-18 17:05                     ` Rob Herring
  2025-08-18 17:37                       ` Ayush Singh
  0 siblings, 1 reply; 28+ messages in thread
From: Rob Herring @ 2025-08-18 17:05 UTC (permalink / raw)
  To: Ayush Singh
  Cc: Krzysztof Kozlowski, Herve Codina, David Gibson, Andrew Davis,
	Geert Uytterhoeven, Krzysztof Kozlowski, Conor Dooley,
	Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan, devicetree,
	devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni

On Sun, Aug 17, 2025 at 3:42 AM Ayush Singh <ayush@beagleboard.org> wrote:
>
> On 8/17/25 13:52, Krzysztof Kozlowski wrote:
>
> > On 17/08/2025 10:18, Ayush Singh wrote:
> >>>>> Hardware:
> >>>>>      i2c0 from SoC --------- connector 1, I2C A signals
> >>>>>      i2c1 from SoC --------- connector 1, I2C B signals
> >>>>>
> >>>>>      connector1 {
> >>>>>          export-symbols {
> >>>>>     i2c_a = <&i2c0>;
> >>>>>     i2c_b = <&i2c1>;
> >>>>>          };
> >>>>>      };
> >>>>>
> >>>>> In order to avoid the coding style issue, this could be replace
> >>>>> with:
> >>>>>     connector1 {
> >>>>>          export-symbols {
> >>>>>     symbol-names = "i2c_a", "i2c_b";
> >>>>>     symbols = <&i2c0>, <&i2c1>;
> >>>>>          };
> >>>>>      };
> >>>>>
> >>>>> Krzysztof, Rob, do you think this could be accepted ?
> >>>>>
> >>>>> Ayush, David, do you thing this could be easily implemented in fdtoverlay ?
> >>>>>
> >>>>> Best regards,
> >>>>> Hervé
> >>>>>
> >>>> Well, it is possible.
> >>>>
> >>>> However, on connectors like pb2 header, there will be 50-100 export
> >>>> symbols. So it will start becoming difficult to maintain.
> >>> And the first syntax solves this how? I don't see the practical difference.
> >>
> >> Well, I was more worried about matching which phandle belongs to which
> >> symbol easily. Let us assume that 2 symbols will be in each line (after
> >> accounting for the indention and 80 char limit) and we have 70 symbols,
> >> so 35 lines. To check which phandle belongs to the 2nd symbol on line
> >> 25th line of  symbol-names, well, you would at the best case need to
> >> have something like relative line numbers in your editor. Then you know
> >> that the 35th line from the current one is where you need to look.
> >>
> >> In the current syntax, the symbol name and phandle are on the same line.
> >> So well, easy to see which symbols refers to which phandle.
> > OK, that's valid point. Any ideas how to solve it without introducing
> > underscores for properties?
> >
> > Best regards,
> > Krzysztof
>
>
> Well, we can modify `get_phandle_from_symbols_node` to allow matching
> `*_*` to `*-*`. And we can do the same in devicetree easily enough. Not
> sure if implicit loose matching like that are the best idea.
>
> Zephyr does something similar for compatible strings. It pretty much
> replaces the all non alphanumeric characters with `_` in compatible
> string match. Although that is more to do with the limitation they are
> working with, i.e. the devicetree being converted to static headers
> instead of being runtime thing.

This is just going from bad to worse... If there's a real need to use
underscores, then use underscores. But that's all beside the point. I
didn't like v1 and nothing has changed in v2 to change that.

This looks like continuing down the path of working around DTB format
limitations like DT overlays originally did (which both David (IIRC)
and I think was a mistake). But now instead of somewhat hidden,
generated data, you're adding manually written/maintained data. I
don't have any suggestion currently how to avoid that other than we
need to rev the DTB format which no one really wants to hear. Maybe
there's some other solution, but I don't have one ATM.

Rob

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/7] dt-bindings: Add support for export-symbols node
  2025-08-18 17:05                     ` Rob Herring
@ 2025-08-18 17:37                       ` Ayush Singh
  0 siblings, 0 replies; 28+ messages in thread
From: Ayush Singh @ 2025-08-18 17:37 UTC (permalink / raw)
  To: Rob Herring
  Cc: Krzysztof Kozlowski, Herve Codina, David Gibson, Andrew Davis,
	Geert Uytterhoeven, Krzysztof Kozlowski, Conor Dooley,
	Arnd Bergmann, Greg Kroah-Hartman, Saravana Kannan, devicetree,
	devicetree-compiler, linux-kernel, Luca Ceresoli,
	Thomas Petazzoni


On 8/18/25 22:35, Rob Herring wrote:
> On Sun, Aug 17, 2025 at 3:42 AM Ayush Singh <ayush@beagleboard.org> wrote:
>> On 8/17/25 13:52, Krzysztof Kozlowski wrote:
>>
>>> On 17/08/2025 10:18, Ayush Singh wrote:
>>>>>>> Hardware:
>>>>>>>       i2c0 from SoC --------- connector 1, I2C A signals
>>>>>>>       i2c1 from SoC --------- connector 1, I2C B signals
>>>>>>>
>>>>>>>       connector1 {
>>>>>>>           export-symbols {
>>>>>>>      i2c_a = <&i2c0>;
>>>>>>>      i2c_b = <&i2c1>;
>>>>>>>           };
>>>>>>>       };
>>>>>>>
>>>>>>> In order to avoid the coding style issue, this could be replace
>>>>>>> with:
>>>>>>>      connector1 {
>>>>>>>           export-symbols {
>>>>>>>      symbol-names = "i2c_a", "i2c_b";
>>>>>>>      symbols = <&i2c0>, <&i2c1>;
>>>>>>>           };
>>>>>>>       };
>>>>>>>
>>>>>>> Krzysztof, Rob, do you think this could be accepted ?
>>>>>>>
>>>>>>> Ayush, David, do you thing this could be easily implemented in fdtoverlay ?
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Hervé
>>>>>>>
>>>>>> Well, it is possible.
>>>>>>
>>>>>> However, on connectors like pb2 header, there will be 50-100 export
>>>>>> symbols. So it will start becoming difficult to maintain.
>>>>> And the first syntax solves this how? I don't see the practical difference.
>>>> Well, I was more worried about matching which phandle belongs to which
>>>> symbol easily. Let us assume that 2 symbols will be in each line (after
>>>> accounting for the indention and 80 char limit) and we have 70 symbols,
>>>> so 35 lines. To check which phandle belongs to the 2nd symbol on line
>>>> 25th line of  symbol-names, well, you would at the best case need to
>>>> have something like relative line numbers in your editor. Then you know
>>>> that the 35th line from the current one is where you need to look.
>>>>
>>>> In the current syntax, the symbol name and phandle are on the same line.
>>>> So well, easy to see which symbols refers to which phandle.
>>> OK, that's valid point. Any ideas how to solve it without introducing
>>> underscores for properties?
>>>
>>> Best regards,
>>> Krzysztof
>>
>> Well, we can modify `get_phandle_from_symbols_node` to allow matching
>> `*_*` to `*-*`. And we can do the same in devicetree easily enough. Not
>> sure if implicit loose matching like that are the best idea.
>>
>> Zephyr does something similar for compatible strings. It pretty much
>> replaces the all non alphanumeric characters with `_` in compatible
>> string match. Although that is more to do with the limitation they are
>> working with, i.e. the devicetree being converted to static headers
>> instead of being runtime thing.
> This is just going from bad to worse... If there's a real need to use
> underscores, then use underscores. But that's all beside the point. I
> didn't like v1 and nothing has changed in v2 to change that.
>
> This looks like continuing down the path of working around DTB format
> limitations like DT overlays originally did (which both David (IIRC)
> and I think was a mistake). But now instead of somewhat hidden,
> generated data, you're adding manually written/maintained data. I
> don't have any suggestion currently how to avoid that other than we
> need to rev the DTB format which no one really wants to hear. Maybe
> there's some other solution, but I don't have one ATM.
>
> Rob

Well, if anyone decides to do a v2 of DTB, I would love to help in any 
way I can.


Best Regards,

Ayush Singh


^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2025-08-18 17:38 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-30 12:51 [PATCH v2 0/7] of: overlay: Add support for export-symbols node feature Herve Codina
2025-04-30 12:51 ` [PATCH v2 1/7] dt-bindings: Add support for export-symbols node Herve Codina
2025-05-02 14:33   ` Luca Ceresoli
2025-05-27 18:31   ` Krzysztof Kozlowski
2025-05-28  7:59     ` Ayush Singh
2025-05-28  8:06       ` Krzysztof Kozlowski
2025-05-28 16:57     ` Herve Codina
2025-06-04 18:35       ` Krzysztof Kozlowski
2025-06-18  9:32         ` Herve Codina
2025-06-18  9:54           ` Ayush Singh
2025-07-04  9:10             ` Herve Codina
2025-08-17  7:41             ` Krzysztof Kozlowski
2025-08-17  8:18               ` Ayush Singh
2025-08-17  8:22                 ` Krzysztof Kozlowski
2025-08-17  8:42                   ` Ayush Singh
2025-08-18 17:05                     ` Rob Herring
2025-08-18 17:37                       ` Ayush Singh
2025-08-17  7:38           ` Krzysztof Kozlowski
2025-04-30 12:51 ` [PATCH v2 2/7] of: resolver: Introduce get_phandle_from_symbols_node() Herve Codina
2025-04-30 12:51 ` [PATCH v2 3/7] of: resolver: Add export_symbols in of_resolve_phandles() parameters Herve Codina
2025-05-02 14:35   ` Luca Ceresoli
2025-05-05  8:10     ` Herve Codina
2025-04-30 12:51 ` [PATCH v2 4/7] of: resolver: Add support for the export symbols node Herve Codina
2025-04-30 12:51 ` [PATCH v2 5/7] of: overlay: Add export_symbols_name in of_overlay_fdt_apply() parameters Herve Codina
2025-05-02 14:40   ` Ayush Singh
2025-05-05  8:17     ` Herve Codina
2025-04-30 12:51 ` [PATCH v2 6/7] of: overlay: Add support for the export symbols node Herve Codina
2025-04-30 12:51 ` [PATCH v2 7/7] of: unittest: Add tests for export symbols Herve Codina

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).