All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 1/2] usb: provide a device tree node to USB devices
@ 2020-05-22 16:38 Michael Walle
  2020-05-22 16:38 ` [PATCH v6 2/2] dm: uclass: don't assign aliased seq numbers Michael Walle
  2020-05-22 23:13 ` [PATCH v6 1/2] usb: provide a device tree node to USB devices Simon Glass
  0 siblings, 2 replies; 3+ messages in thread
From: Michael Walle @ 2020-05-22 16:38 UTC (permalink / raw)
  To: u-boot

It is possible to specify a device tree node for an USB device. This is
useful if you have a static USB setup and want to use aliases which
point to these nodes, like on the Raspberry Pi.
The nodes are matched against their hub port number, the compatible
strings are not matched for now.

Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Marek Vasut <marex@denx.de>
---
Changes since v5:
  - added test cases

This is a new patch in v5:
  Fixes the ethernet0 alias on Raspberry Pis. This has never been
  working, but wasn't a problem until recently. Patch 2/2 changes
  the allocation of the numbers and reserves possible aliases.

 arch/sandbox/dts/test.dts     |  9 ++++++++
 drivers/usb/host/usb-uclass.c | 41 ++++++++++++++++++++++++++++++-----
 test/dm/usb.c                 | 22 +++++++++++++++++++
 3 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 5ce5e28476..b9255e4593 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -810,6 +810,8 @@
 		hub {
 			compatible = "usb-hub";
 			usb,device-class = <9>;
+			#address-cells = <1>;
+			#size-cells = <0>;
 			hub-emul {
 				compatible = "sandbox,usb-hub";
 				#address-cells = <1>;
@@ -838,6 +840,13 @@
 				};
 
 			};
+
+			usbstor at 1 {
+				reg = <1>;
+			};
+			usbstor at 3 {
+				reg = <3>;
+			};
 		};
 	};
 
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index cb79dfbbd5..f42c0625cb 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -494,6 +494,35 @@ static int usb_match_one_id(struct usb_device_descriptor *desc,
 	return usb_match_one_id_intf(desc, int_desc, id);
 }
 
+static ofnode usb_get_ofnode(struct udevice *hub, int port)
+{
+	ofnode node;
+	u32 reg;
+
+	if (!dev_has_of_node(hub))
+		return ofnode_null();
+
+	/*
+	 * The USB controller and its USB hub are two different udevices,
+	 * but the device tree has only one node for both. Thus we are
+	 * assigning this node to both udevices.
+	 * If port is zero, the controller scans its root hub, thus we
+	 * are using the same ofnode as the controller here.
+	 */
+	if (!port)
+		return dev_ofnode(hub);
+
+	ofnode_for_each_subnode(node, dev_ofnode(hub)) {
+		if (ofnode_read_u32(node, "reg", &reg))
+			continue;
+
+		if (reg == port)
+			return node;
+	}
+
+	return ofnode_null();
+}
+
 /**
  * usb_find_and_bind_driver() - Find and bind the right USB driver
  *
@@ -502,13 +531,14 @@ static int usb_match_one_id(struct usb_device_descriptor *desc,
 static int usb_find_and_bind_driver(struct udevice *parent,
 				    struct usb_device_descriptor *desc,
 				    struct usb_interface_descriptor *iface,
-				    int bus_seq, int devnum,
+				    int bus_seq, int devnum, int port,
 				    struct udevice **devp)
 {
 	struct usb_driver_entry *start, *entry;
 	int n_ents;
 	int ret;
 	char name[30], *str;
+	ofnode node = usb_get_ofnode(parent, port);
 
 	*devp = NULL;
 	debug("%s: Searching for driver\n", __func__);
@@ -533,8 +563,8 @@ static int usb_find_and_bind_driver(struct udevice *parent,
 			 * find another driver. For now this doesn't seem
 			 * necesssary, so just bind the first match.
 			 */
-			ret = device_bind(parent, drv, drv->name, NULL, -1,
-					  &dev);
+			ret = device_bind_ofnode(parent, drv, drv->name, NULL,
+						 node, &dev);
 			if (ret)
 				goto error;
 			debug("%s: Match found: %s\n", __func__, drv->name);
@@ -651,9 +681,10 @@ int usb_scan_device(struct udevice *parent, int port,
 	if (ret) {
 		if (ret != -ENOENT)
 			return ret;
-		ret = usb_find_and_bind_driver(parent, &udev->descriptor, iface,
+		ret = usb_find_and_bind_driver(parent, &udev->descriptor,
+					       iface,
 					       udev->controller_dev->seq,
-					       udev->devnum, &dev);
+					       udev->devnum, port, &dev);
 		if (ret)
 			return ret;
 		created = true;
diff --git a/test/dm/usb.c b/test/dm/usb.c
index a25c2c1482..b273a515ef 100644
--- a/test/dm/usb.c
+++ b/test/dm/usb.c
@@ -78,6 +78,28 @@ static int dm_test_usb_multi(struct unit_test_state *uts)
 }
 DM_TEST(dm_test_usb_multi, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
 
+/* test that we have an associated ofnode with the usb device */
+static int dm_test_usb_fdt_node(struct unit_test_state *uts)
+{
+	struct udevice *dev;
+	ofnode node;
+
+	state_set_skip_delays(true);
+	ut_assertok(usb_init());
+	ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 0, &dev));
+	node = ofnode_path("/usb at 1/hub/usbstor at 1");
+	ut_asserteq(1, ofnode_equal(node, dev_ofnode(dev)));
+	ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 1, &dev));
+	ut_asserteq(1, ofnode_equal(ofnode_null(), dev_ofnode(dev)));
+	ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 2, &dev));
+	node = ofnode_path("/usb at 1/hub/usbstor at 3");
+	ut_asserteq(1, ofnode_equal(node, dev_ofnode(dev)));
+	ut_assertok(usb_stop());
+
+	return 0;
+}
+DM_TEST(dm_test_usb_fdt_node, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
 static int count_usb_devices(void)
 {
 	struct udevice *hub;
-- 
2.20.1

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

* [PATCH v6 2/2] dm: uclass: don't assign aliased seq numbers
  2020-05-22 16:38 [PATCH v6 1/2] usb: provide a device tree node to USB devices Michael Walle
@ 2020-05-22 16:38 ` Michael Walle
  2020-05-22 23:13 ` [PATCH v6 1/2] usb: provide a device tree node to USB devices Simon Glass
  1 sibling, 0 replies; 3+ messages in thread
From: Michael Walle @ 2020-05-22 16:38 UTC (permalink / raw)
  To: u-boot

If there are aliases for an uclass, set the base for the "dynamically"
allocated numbers next to the highest alias.

Please note, that this might lead to holes in the sequences, depending
on the device tree. For example if there is only an alias "ethernet1",
the next device seq number would be 2.

In particular this fixes a problem with boards which are using ethernet
aliases but also might have network add-in cards like the E1000. If the
board is started with the add-in card and depending on the order of the
drivers, the E1000 might occupy the first ethernet device and mess up
all the hardware addresses, because the devices are now shifted by one.

Also adapt the test cases to the new handling and add test cases
checking the holes in the seq numbers.

Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Alex Marginean <alexandru.marginean@nxp.com>
Tested-by: Alex Marginean <alexandru.marginean@nxp.com>
Acked-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Michal Simek <michal.simek@xilinx.com> [on zcu102-revA]
---
changes since v5:
 none

changes since v4:
 none

changes since v3:
 - dev_read_alias_highest_id() is only available if CONFIG_OF_CONTROL is
   set. Thus added an additional condition "CONFIG_IS_ENABLED(OF_CONTROL)",
   thanks Simon.

changes since v2:
 - adapt/new test cases, thanks Simon

changes since v1:
 - move notice about superfluous commits from commit message to this
   section.
 - fix the comment style

 arch/sandbox/dts/test.dts |  4 ++--
 drivers/core/uclass.c     | 21 +++++++++++++++------
 include/configs/sandbox.h |  6 +++---
 test/dm/eth.c             | 14 +++++++-------
 test/dm/test-fdt.c        | 22 +++++++++++++++++-----
 5 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index b9255e4593..a6e2bfd082 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -23,8 +23,8 @@
 		pci0 = &pci0;
 		pci1 = &pci1;
 		pci2 = &pci2;
-		remoteproc1 = &rproc_1;
-		remoteproc2 = &rproc_2;
+		remoteproc0 = &rproc_1;
+		remoteproc1 = &rproc_2;
 		rtc0 = &rtc_0;
 		rtc1 = &rtc_1;
 		spi0 = "/spi at 0";
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index 2ab419cfe4..c3f1b73cd6 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -689,13 +689,14 @@ int uclass_unbind_device(struct udevice *dev)
 
 int uclass_resolve_seq(struct udevice *dev)
 {
+	struct uclass *uc = dev->uclass;
+	struct uclass_driver *uc_drv = uc->uc_drv;
 	struct udevice *dup;
-	int seq;
+	int seq = 0;
 	int ret;
 
 	assert(dev->seq == -1);
-	ret = uclass_find_device_by_seq(dev->uclass->uc_drv->id, dev->req_seq,
-					false, &dup);
+	ret = uclass_find_device_by_seq(uc_drv->id, dev->req_seq, false, &dup);
 	if (!ret) {
 		dm_warn("Device '%s': seq %d is in use by '%s'\n",
 			dev->name, dev->req_seq, dup->name);
@@ -707,9 +708,17 @@ int uclass_resolve_seq(struct udevice *dev)
 		return ret;
 	}
 
-	for (seq = 0; seq < DM_MAX_SEQ; seq++) {
-		ret = uclass_find_device_by_seq(dev->uclass->uc_drv->id, seq,
-						false, &dup);
+	if (CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(DM_SEQ_ALIAS) &&
+	    (uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS)) {
+		/*
+		 * dev_read_alias_highest_id() will return -1 if there no
+		 * alias. Thus we can always add one.
+		 */
+		seq = dev_read_alias_highest_id(uc_drv->name) + 1;
+	}
+
+	for (; seq < DM_MAX_SEQ; seq++) {
+		ret = uclass_find_device_by_seq(uc_drv->id, seq, false, &dup);
 		if (ret == -ENODEV)
 			break;
 		if (ret)
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 7fda63f71a..4549c81169 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -95,9 +95,9 @@
 #endif
 
 #define SANDBOX_ETH_SETTINGS		"ethaddr=00:00:11:22:33:44\0" \
-					"eth1addr=00:00:11:22:33:45\0" \
-					"eth3addr=00:00:11:22:33:46\0" \
-					"eth5addr=00:00:11:22:33:47\0" \
+					"eth3addr=00:00:11:22:33:45\0" \
+					"eth5addr=00:00:11:22:33:46\0" \
+					"eth6addr=00:00:11:22:33:47\0" \
 					"ipaddr=1.2.3.4\0"
 
 #define MEM_LAYOUT_ENV_SETTINGS \
diff --git a/test/dm/eth.c b/test/dm/eth.c
index 1fddcaabb0..b58c9640a2 100644
--- a/test/dm/eth.c
+++ b/test/dm/eth.c
@@ -48,7 +48,7 @@ static int dm_test_eth_alias(struct unit_test_state *uts)
 	ut_assertok(net_loop(PING));
 	ut_asserteq_str("eth at 10002000", env_get("ethact"));
 
-	env_set("ethact", "eth1");
+	env_set("ethact", "eth6");
 	ut_assertok(net_loop(PING));
 	ut_asserteq_str("eth at 10004000", env_get("ethact"));
 
@@ -105,7 +105,7 @@ static int dm_test_eth_act(struct unit_test_state *uts)
 	const char *ethname[DM_TEST_ETH_NUM] = {"eth at 10002000", "eth at 10003000",
 						"sbe5", "eth at 10004000"};
 	const char *addrname[DM_TEST_ETH_NUM] = {"ethaddr", "eth5addr",
-						 "eth3addr", "eth1addr"};
+						 "eth3addr", "eth6addr"};
 	char ethaddr[DM_TEST_ETH_NUM][18];
 	int i;
 
@@ -188,15 +188,15 @@ static int dm_test_eth_rotate(struct unit_test_state *uts)
 
 	/* Invalidate eth1's MAC address */
 	memset(ethaddr, '\0', sizeof(ethaddr));
-	strncpy(ethaddr, env_get("eth1addr"), 17);
-	/* Must disable access protection for eth1addr before clearing */
-	env_set(".flags", "eth1addr");
-	env_set("eth1addr", NULL);
+	strncpy(ethaddr, env_get("eth6addr"), 17);
+	/* Must disable access protection for eth6addr before clearing */
+	env_set(".flags", "eth6addr");
+	env_set("eth6addr", NULL);
 
 	retval = _dm_test_eth_rotate1(uts);
 
 	/* Restore the env */
-	env_set("eth1addr", ethaddr);
+	env_set("eth6addr", ethaddr);
 	env_set("ethrotate", NULL);
 
 	if (!retval) {
diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c
index 4fcae03dc5..51f2547409 100644
--- a/test/dm/test-fdt.c
+++ b/test/dm/test-fdt.c
@@ -361,20 +361,32 @@ static int dm_test_fdt_uclass_seq(struct unit_test_state *uts)
 	ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 2, &dev));
 	ut_asserteq_str("d-test", dev->name);
 
-	/* d-test actually gets 0 */
-	ut_assertok(uclass_get_device_by_seq(UCLASS_TEST_FDT, 0, &dev));
+	/*
+	 * d-test actually gets 9, because thats the next free one after the
+	 * aliases.
+	 */
+	ut_assertok(uclass_get_device_by_seq(UCLASS_TEST_FDT, 9, &dev));
 	ut_asserteq_str("d-test", dev->name);
 
-	/* initially no one wants seq 1 */
-	ut_asserteq(-ENODEV, uclass_get_device_by_seq(UCLASS_TEST_FDT, 1,
+	/* initially no one wants seq 10 */
+	ut_asserteq(-ENODEV, uclass_get_device_by_seq(UCLASS_TEST_FDT, 10,
 						      &dev));
 	ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));
 	ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 4, &dev));
 
 	/* But now that it is probed, we can find it */
-	ut_assertok(uclass_get_device_by_seq(UCLASS_TEST_FDT, 1, &dev));
+	ut_assertok(uclass_get_device_by_seq(UCLASS_TEST_FDT, 10, &dev));
 	ut_asserteq_str("f-test", dev->name);
 
+	/*
+	 * And we should still have holes in our sequence numbers, that is 2
+	 * and 4 should not be used.
+	 */
+	ut_asserteq(-ENODEV, uclass_find_device_by_seq(UCLASS_TEST_FDT, 2,
+						       true, &dev));
+	ut_asserteq(-ENODEV, uclass_find_device_by_seq(UCLASS_TEST_FDT, 4,
+						       true, &dev));
+
 	return 0;
 }
 DM_TEST(dm_test_fdt_uclass_seq, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
-- 
2.20.1

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

* [PATCH v6 1/2] usb: provide a device tree node to USB devices
  2020-05-22 16:38 [PATCH v6 1/2] usb: provide a device tree node to USB devices Michael Walle
  2020-05-22 16:38 ` [PATCH v6 2/2] dm: uclass: don't assign aliased seq numbers Michael Walle
@ 2020-05-22 23:13 ` Simon Glass
  1 sibling, 0 replies; 3+ messages in thread
From: Simon Glass @ 2020-05-22 23:13 UTC (permalink / raw)
  To: u-boot

On Fri, 22 May 2020 at 10:38, Michael Walle <michael@walle.cc> wrote:
>
> It is possible to specify a device tree node for an USB device. This is
> useful if you have a static USB setup and want to use aliases which
> point to these nodes, like on the Raspberry Pi.
> The nodes are matched against their hub port number, the compatible
> strings are not matched for now.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> Reviewed-by: Marek Vasut <marex@denx.de>
> ---
> Changes since v5:
>   - added test cases
>
> This is a new patch in v5:
>   Fixes the ethernet0 alias on Raspberry Pis. This has never been
>   working, but wasn't a problem until recently. Patch 2/2 changes
>   the allocation of the numbers and reserves possible aliases.
>
>  arch/sandbox/dts/test.dts     |  9 ++++++++
>  drivers/usb/host/usb-uclass.c | 41 ++++++++++++++++++++++++++++++-----
>  test/dm/usb.c                 | 22 +++++++++++++++++++
>  3 files changed, 67 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

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

end of thread, other threads:[~2020-05-22 23:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-22 16:38 [PATCH v6 1/2] usb: provide a device tree node to USB devices Michael Walle
2020-05-22 16:38 ` [PATCH v6 2/2] dm: uclass: don't assign aliased seq numbers Michael Walle
2020-05-22 23:13 ` [PATCH v6 1/2] usb: provide a device tree node to USB devices Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.