diff mbox series

dm: core: Fix address translation in devfdt_get_addr_index()

Message ID 20240217131943.2472659-1-jonas@kwiboo.se
State Accepted
Commit d07d868be38f2316e4e8818d4d8c30442b8a4fb0
Delegated to: Tom Rini
Headers show
Series dm: core: Fix address translation in devfdt_get_addr_index() | expand

Commit Message

Jonas Karlman Feb. 17, 2024, 1:19 p.m. UTC
During address translation #address/size-cells props are expected to
apply to child nodes. However, devfdt_get_addr_index() incorrectly use
the parent offset of the udevice parent instead of getting the offset of
the node parent. This will work in most cases when there is only one
udevice tied to a node.

On e.g. an Orange Pi R1 Plus TLS board the dwc3-generic (parent) and
dwc3-generic-host (child) udevice is tied to the same node.

In that case both the offset and parent values end up being the same.
As a result, the #address/size-cells props intended for child nodes
incorrectly gets applied to the node itself resulting in wrong addr
being returned, 0x0 instead of 0xff600000.

The following can be seen on console:

  dwc3-generic-host usb@ff600000: this is not a DesignWare USB3 DRD Core
  dwc3-generic-host usb@ff600000: failed to initialize core

Fix this by using the offset of the parent node and not the offset to
the node the parent udevice is tied to.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
I think the following unit test dts and code should be able to detect
this issue. Not sure on how to add a test with a different dts, most
existing ones seemed to use #address/size-cells = <1>.

Please advide on how following could be turned into a proper unit test.

/dts-v1/;

/ {
	#address-cells = <2>;
	#size-cells = <2>;

	some-bus {
		compatible = "denx,u-boot-test-bus";
		reg = <0x0 0xffff0000 0x0 0x1000>;
		#address-cells = <1>;
		#size-cells = <0>;

		device@0 {
			compatible = "denx,u-boot-fdt-test";
			reg = <0>;
		};
	};
};

struct udevice *bus, *dev;

ut_assertok(uclass_get_device(UCLASS_TEST_BUS, 0, &bus));
ut_asserteq(0xffff0000, devfdt_get_addr(bus));
ut_assertok(device_bind(bus, DM_DRIVER_GET(denx_u_boot_fdt_test),
			"test", NULL, dev_ofnode(bus), &dev));
ut_asserteq(0xffff0000, devfdt_get_addr(dev));
---
 drivers/core/fdtaddr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tom Rini March 4, 2024, 3:28 p.m. UTC | #1
On Sat, Feb 17, 2024 at 01:19:41PM +0000, Jonas Karlman wrote:

> During address translation #address/size-cells props are expected to
> apply to child nodes. However, devfdt_get_addr_index() incorrectly use
> the parent offset of the udevice parent instead of getting the offset of
> the node parent. This will work in most cases when there is only one
> udevice tied to a node.
> 
> On e.g. an Orange Pi R1 Plus TLS board the dwc3-generic (parent) and
> dwc3-generic-host (child) udevice is tied to the same node.
> 
> In that case both the offset and parent values end up being the same.
> As a result, the #address/size-cells props intended for child nodes
> incorrectly gets applied to the node itself resulting in wrong addr
> being returned, 0x0 instead of 0xff600000.
> 
> The following can be seen on console:
> 
>   dwc3-generic-host usb@ff600000: this is not a DesignWare USB3 DRD Core
>   dwc3-generic-host usb@ff600000: failed to initialize core
> 
> Fix this by using the offset of the parent node and not the offset to
> the node the parent udevice is tied to.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

Applied to u-boot/next, thanks!
diff mbox series

Patch

diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c
index 8e774d49ce6e..5f27d2511485 100644
--- a/drivers/core/fdtaddr.c
+++ b/drivers/core/fdtaddr.c
@@ -23,7 +23,7 @@  fdt_addr_t devfdt_get_addr_index(const struct udevice *dev, int index)
 {
 #if CONFIG_IS_ENABLED(OF_REAL)
 	int offset = dev_of_offset(dev);
-	int parent = dev_of_offset(dev->parent);
+	int parent = fdt_parent_offset(gd->fdt_blob, offset);
 	fdt_addr_t addr;
 
 	if (CONFIG_IS_ENABLED(OF_TRANSLATE)) {