zephyr: device: device_get_binding is broken for nodes with the same name

Describe the bug

With the removal of labels, DT devices get its name set to DT_NODE_FULL_NAME(node_id). The problem is that such name is not necessarily unique, e.g. this is valid in DT:

i2c@xy {
	...
	sensora: sensor@10 {  /* DT_NODE_FULL_NAME = sensor@10 */
		...
	};
}

i2c@yx {
	...
	sensorb: sensor@10 { /* DT_NODE_FULL_NAME = sensor@10 */
		...
	};
};

As a result, only one of them can be obtained at runtime using device_get_binding(). While not a frequent case, it is a limitation that should either be fixed or documented.

To Reproduce

Apply this patch:

diff --git a/tests/lib/devicetree/devices/app.overlay b/tests/lib/devicetree/devices/app.overlay
index 3a6212721c..20afde23b6 100644
--- a/tests/lib/devicetree/devices/app.overlay
+++ b/tests/lib/devicetree/devices/app.overlay
@@ -82,6 +82,22 @@
 			};
 		};
 
+		test_i2c2: i2c@11112223 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "vnd,i2c";
+			status = "okay";
+			reg = <0x11112223 0x1000>;
+			clock-frequency = <100000>;
+			test_gpioy: test-i2c-dev@11 {
+				gpio-controller;
+				#gpio-cells = <2>;
+				compatible = "vnd,gpio-expander";
+				status = "okay";
+				reg = <0x11>;
+			};
+		};
+
 		test_gpio_injected: gpio@1000 {
 			gpio-controller;
 			#gpio-cells = <0x2>;
diff --git a/tests/lib/devicetree/devices/src/main.c b/tests/lib/devicetree/devices/src/main.c
index a870253888..1affad6ec4 100644
--- a/tests/lib/devicetree/devices/src/main.c
+++ b/tests/lib/devicetree/devices/src/main.c
@@ -10,9 +10,11 @@
 #include <zephyr/drivers/gpio.h>
 
 #define TEST_GPIO DT_NODELABEL(test_gpio_0)
+#define TEST_I2C2 DT_NODELABEL(test_i2c2)
 #define TEST_I2C DT_NODELABEL(test_i2c)
 #define TEST_DEVA DT_NODELABEL(test_dev_a)
 #define TEST_GPIOX DT_NODELABEL(test_gpiox)
+#define TEST_GPIOY DT_NODELABEL(test_gpioy)
 #define TEST_DEVB DT_NODELABEL(test_dev_b)
 #define TEST_DEVC DT_NODELABEL(test_dev_c)
 #define TEST_PARTITION DT_NODELABEL(test_p0)
@@ -22,7 +24,7 @@
 static const struct device *devlist;
 static const struct device *devlist_end;
 
-static device_handle_t init_order[10];
+static device_handle_t init_order[12];
 
 static int dev_init(const struct device *dev)
 {
@@ -36,6 +38,8 @@ static int dev_init(const struct device *dev)
 
 DEVICE_DT_DEFINE(TEST_GPIO, dev_init, NULL,
 		 NULL, NULL, PRE_KERNEL_1, 90, NULL);
+DEVICE_DT_DEFINE(TEST_I2C2, dev_init, NULL,
+		 NULL, NULL, POST_KERNEL, 10, NULL);
 DEVICE_DT_DEFINE(TEST_I2C, dev_init, NULL,
 		 NULL, NULL, POST_KERNEL, 10, NULL);
 DEVICE_DT_DEFINE(TEST_DEVA, dev_init, NULL,
@@ -45,6 +49,8 @@ DEVICE_DT_DEFINE(TEST_DEVB, dev_init, NULL,
 		 NULL, NULL, POST_KERNEL, 30, NULL);
 DEVICE_DT_DEFINE(TEST_GPIOX, dev_init, NULL,
 		 NULL, NULL, POST_KERNEL, 40, NULL);
+DEVICE_DT_DEFINE(TEST_GPIOY, dev_init, NULL,
+		 NULL, NULL, POST_KERNEL, 40, NULL);
 DEVICE_DT_DEFINE(TEST_DEVC, dev_init, NULL,
 		 NULL, NULL, POST_KERNEL, 50, NULL);
 DEVICE_DT_DEFINE(TEST_PARTITION, dev_init, NULL,
@@ -73,8 +79,10 @@ ZTEST(devicetree_devices, test_init_get)
 		      DEVICE_DT_GET(TEST_DEVA), NULL);
 	zassert_equal(DEVICE_INIT_DT_GET(TEST_DEVB)->dev,
 		      DEVICE_DT_GET(TEST_DEVB), NULL);
-	zassert_equal(DEVICE_INIT_DT_GET(TEST_GPIOX)->dev,
+	zassert_equal(device_get_binding(DEVICE_DT_NAME(TEST_GPIOX)),
 		      DEVICE_DT_GET(TEST_GPIOX), NULL);
+	zassert_equal(device_get_binding(DEVICE_DT_NAME(TEST_GPIOY)),
+		      DEVICE_DT_GET(TEST_GPIOY), NULL);
 	zassert_equal(DEVICE_INIT_DT_GET(TEST_DEVC)->dev,
 		      DEVICE_DT_GET(TEST_DEVC), NULL);
 	zassert_equal(DEVICE_INIT_DT_GET(TEST_PARTITION)->dev,

and run west build -b native_posix tests/lib/devicetree/devices -p.

Expected behavior

I should be able to obtain all devices at runtime using device_get_binding(). If not, document the limitations.

Impact

N/A

Logs and console output

N/A

Environment (please complete the following information):

  • OS: Linux
  • Toolchain Zephyr SDK 0.15.0
  • Commit SHA or Version used f4d372bf7d41fc5ee4337733fc0ef562ece904f7

Additional context N/A

About this issue

  • Original URL
  • State: open
  • Created 2 years ago
  • Comments: 22 (6 by maintainers)

Most upvoted comments

@gmarull thoughts on how to proceed on this?

Not really. The thing is that we lost a way to reliably obtain a device reference at runtime. Before you could always set label, which was guaranteed to be unique. We probably need to rethink devices runtime facilities. label was used as a software configuration property, but now we have a similar pattern with node labels, sometimes used to refer to a node within DT but in many other cases to just take a device reference in the code. Maybe we could have an option that, when enabled, allows to have a device table that has human-friendly metadata to access the device at runtime, e.g. all sensors create a /dev/sensorN + compatible entry. This could be useful when debugging, or in shell-based apps.