All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] usb: gadget: udc: Fix duplicate uclass name
@ 2024-08-02  9:28 Zixun LI
  2024-08-02  9:28 ` [PATCH] dm: core: Show device sequence instead in dm_dump_tree() Zixun LI
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Zixun LI @ 2024-08-02  9:28 UTC (permalink / raw)
  To: Simon Glass, Tom Rini, Lukasz Majewski, Mattijs Korpershoek,
	Marek Vasut
  Cc: Zixun LI, u-boot

Currently both USB host uclass and USB gadget uclass are using the same
name "usb" which break uclass functions like uclass_get_by_name().

Rename the uclass to "usb_gadget" to fix, also makes bind/unbind by class
index (or sequence) working.

This breaks the capacity of using "usb" as DT alias sequence numbering
which needs a fix afterwards.

Signed-off-by: Zixun LI <admin@hifiphile.com>
---
 drivers/usb/gadget/udc/udc-uclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/udc-uclass.c b/drivers/usb/gadget/udc/udc-uclass.c
index fbe62bbce4..723d1cdfd7 100644
--- a/drivers/usb/gadget/udc/udc-uclass.c
+++ b/drivers/usb/gadget/udc/udc-uclass.c
@@ -83,7 +83,7 @@ __weak int dm_usb_gadget_handle_interrupts(struct udevice *dev)
 #if CONFIG_IS_ENABLED(DM)
 UCLASS_DRIVER(usb_gadget_generic) = {
 	.id		= UCLASS_USB_GADGET_GENERIC,
-	.name		= "usb",
+	.name		= "usb_gadget",
 	.flags		= DM_UC_FLAG_SEQ_ALIAS,
 };
 #endif
--
2.45.2


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

* [PATCH] dm: core: Show device sequence instead in dm_dump_tree()
  2024-08-02  9:28 [PATCH] usb: gadget: udc: Fix duplicate uclass name Zixun LI
@ 2024-08-02  9:28 ` Zixun LI
  2024-08-06 21:50   ` Simon Glass
  2024-08-27 23:22   ` Tom Rini
  2024-08-02  9:28 ` [PATCH] cmd: bind: Use device sequence instead for driver bind/unbind Zixun LI
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 15+ messages in thread
From: Zixun LI @ 2024-08-02  9:28 UTC (permalink / raw)
  To: Simon Glass, Tom Rini, Lukasz Majewski, Mattijs Korpershoek,
	Marek Vasut
  Cc: Zixun LI, u-boot

Currently uclass index is shown in DM tree dump which ignores alias
sequence numbering. The result could be confusing since these 2 numbers
could be different. Show device sequence number instead as it's more
meaningful.

Also update documentation to use sequence number.

Signed-off-by: Zixun LI <admin@hifiphile.com>
---
 doc/usage/cmd/dm.rst | 7 +++----
 drivers/core/dump.c  | 4 ++--
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/doc/usage/cmd/dm.rst b/doc/usage/cmd/dm.rst
index 7651507937..196b22d137 100644
--- a/doc/usage/cmd/dm.rst
+++ b/doc/usage/cmd/dm.rst
@@ -112,9 +112,8 @@ This shows the full tree of devices including the following fields:
 uclass
     Shows the name of the uclass for the device

-Index
-    Shows the index number of the device, within the uclass. This shows the
-    ordering within the uclass, but not the sequence number.
+Seq
+    Shows the sequence number of the device, within the uclass.

 Probed
     Shows `+` if the device is active
@@ -366,7 +365,7 @@ dm tree
 This example shows the abridged sandbox output::

     => dm tree
-    Class     Index  Probed  Driver                Name
+    Class     Seq    Probed  Driver                Name
     -----------------------------------------------------------
     root          0  [ + ]   root_driver           root_driver
     demo          0  [   ]   demo_shape_drv        |-- demo_shape_drv
diff --git a/drivers/core/dump.c b/drivers/core/dump.c
index 5ec30d5b3c..5cbaa97fa3 100644
--- a/drivers/core/dump.c
+++ b/drivers/core/dump.c
@@ -40,7 +40,7 @@ static void show_devices(struct udevice *dev, int depth, int last_flag,
 	/* print the first 20 characters to not break the tree-format. */
 	printf(CONFIG_IS_ENABLED(USE_TINY_PRINTF) ? " %s  %d  [ %c ]   %s  " :
 	       " %-10.10s  %3d  [ %c ]   %-20.20s  ", dev->uclass->uc_drv->name,
-	       dev_get_uclass_index(dev, NULL),
+	       dev->seq_,
 	       flags & DM_FLAG_ACTIVATED ? '+' : ' ', dev->driver->name);

 	for (i = depth; i >= 0; i--) {
@@ -129,7 +129,7 @@ void dm_dump_tree(char *dev_name, bool extended, bool sort)
 {
 	struct udevice *root;

-	printf(" Class     Index  Probed  Driver                Name\n");
+	printf(" Class     Seq    Probed  Driver                Name\n");
 	printf("-----------------------------------------------------------\n");

 	root = dm_root();
--
2.45.2


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

* [PATCH] cmd: bind: Use device sequence instead for driver bind/unbind
  2024-08-02  9:28 [PATCH] usb: gadget: udc: Fix duplicate uclass name Zixun LI
  2024-08-02  9:28 ` [PATCH] dm: core: Show device sequence instead in dm_dump_tree() Zixun LI
@ 2024-08-02  9:28 ` Zixun LI
  2024-08-06 21:50   ` Simon Glass
                     ` (2 more replies)
  2024-08-06 21:50 ` [PATCH] usb: gadget: udc: Fix duplicate uclass name Simon Glass
  2024-08-07  7:07 ` Mattijs Korpershoek
  3 siblings, 3 replies; 15+ messages in thread
From: Zixun LI @ 2024-08-02  9:28 UTC (permalink / raw)
  To: Simon Glass, Tom Rini, Lukasz Majewski, Mattijs Korpershoek,
	Marek Vasut
  Cc: Zixun LI, u-boot

Currently uclass index is used for bind/unbind which ignores alias
sequence numbering. Use device sequence number instead as it's
the number explicitly set in the DT.

Also update documentation to use sequence number.

Signed-off-by: Zixun LI <admin@hifiphile.com>
---
 cmd/bind.c               | 46 ++++++++++++++++++++--------------------
 doc/usage/cmd/bind.rst   | 12 +++++------
 doc/usage/cmd/unbind.rst | 14 ++++++------
 3 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/cmd/bind.c b/cmd/bind.c
index 3a59eefd5c..c0d31f5eb1 100644
--- a/cmd/bind.c
+++ b/cmd/bind.c
@@ -10,8 +10,8 @@
 #include <dm/root.h>
 #include <dm/uclass-internal.h>

-static int bind_by_class_index(const char *uclass, int index,
-			       const char *drv_name)
+static int bind_by_class_seq(const char *uclass, int seq,
+			     const char *drv_name)
 {
 	static enum uclass_id uclass_id;
 	struct udevice *dev;
@@ -31,9 +31,9 @@ static int bind_by_class_index(const char *uclass, int index,
 		return -EINVAL;
 	}

-	ret = uclass_find_device(uclass_id, index, &parent);
+	ret = uclass_find_device_by_seq(uclass_id, seq, &parent);
 	if (!parent || ret) {
-		printf("Cannot find device %d of class %s\n", index, uclass);
+		printf("Cannot find device %d of class %s\n", seq, uclass);
 		return ret;
 	}

@@ -47,7 +47,7 @@ static int bind_by_class_index(const char *uclass, int index,
 	return 0;
 }

-static int find_dev(const char *uclass, int index, struct udevice **devp)
+static int find_dev(const char *uclass, int seq, struct udevice **devp)
 {
 	static enum uclass_id uclass_id;
 	int rc;
@@ -58,21 +58,21 @@ static int find_dev(const char *uclass, int index, struct udevice **devp)
 		return -EINVAL;
 	}

-	rc = uclass_find_device(uclass_id, index, devp);
+	rc = uclass_find_device_by_seq(uclass_id, seq, devp);
 	if (!*devp || rc) {
-		printf("Cannot find device %d of class %s\n", index, uclass);
+		printf("Cannot find device %d of class %s\n", seq, uclass);
 		return rc;
 	}

 	return 0;
 }

-static int unbind_by_class_index(const char *uclass, int index)
+static int unbind_by_class_seq(const char *uclass, int seq)
 {
 	int ret;
 	struct udevice *dev;

-	ret = find_dev(uclass, index, &dev);
+	ret = find_dev(uclass, seq, &dev);
 	if (ret)
 		return ret;

@@ -91,8 +91,8 @@ static int unbind_by_class_index(const char *uclass, int index)
 	return 0;
 }

-static int unbind_child_by_class_index(const char *uclass, int index,
-				       const char *drv_name)
+static int unbind_child_by_class_seq(const char *uclass, int seq,
+				     const char *drv_name)
 {
 	struct udevice *parent;
 	int ret;
@@ -104,7 +104,7 @@ static int unbind_child_by_class_index(const char *uclass, int index,
 		return -ENOENT;
 	}

-	ret = find_dev(uclass, index, &parent);
+	ret = find_dev(uclass, seq, &parent);
 	if (ret)
 		return ret;

@@ -217,19 +217,19 @@ static int do_bind_unbind(struct cmd_tbl *cmdtp, int flag, int argc,
 			return CMD_RET_USAGE;
 		ret = unbind_by_node_path(argv[1]);
 	} else if (!by_node && bind) {
-		int index = (argc > 2) ? dectoul(argv[2], NULL) : 0;
+		int seq = (argc > 2) ? dectoul(argv[2], NULL) : 0;

 		if (argc != 4)
 			return CMD_RET_USAGE;
-		ret = bind_by_class_index(argv[1], index, argv[3]);
+		ret = bind_by_class_seq(argv[1], seq, argv[3]);
 	} else if (!by_node && !bind) {
-		int index = (argc > 2) ? dectoul(argv[2], NULL) : 0;
+		int seq = (argc > 2) ? dectoul(argv[2], NULL) : 0;

 		if (argc == 3)
-			ret = unbind_by_class_index(argv[1], index);
+			ret = unbind_by_class_seq(argv[1], seq);
 		else if (argc == 4)
-			ret = unbind_child_by_class_index(argv[1], index,
-							  argv[3]);
+			ret = unbind_child_by_class_seq(argv[1], seq,
+							argv[3]);
 		else
 			return CMD_RET_USAGE;
 	}
@@ -244,17 +244,17 @@ U_BOOT_CMD(
 	bind,	4,	0,	do_bind_unbind,
 	"Bind a device to a driver",
 	"<node path> <driver>\n"
-	"bind <class> <index> <driver>\n"
+	"bind <class> <seq> <driver>\n"
 	"Use 'dm tree' to list all devices registered in the driver model,\n"
-	"their path, class, index and current driver.\n"
+	"their path, class, sequence and current driver.\n"
 );

 U_BOOT_CMD(
 	unbind,	4,	0,	do_bind_unbind,
 	"Unbind a device from a driver",
 	"<node path>\n"
-	"unbind <class> <index>\n"
-	"unbind <class> <index> <driver>\n"
+	"unbind <class> <seq>\n"
+	"unbind <class> <seq> <driver>\n"
 	"Use 'dm tree' to list all devices registered in the driver model,\n"
-	"their path, class, index and current driver.\n"
+	"their path, class, sequence and current driver.\n"
 );
diff --git a/doc/usage/cmd/bind.rst b/doc/usage/cmd/bind.rst
index 2345778366..67a0405bab 100644
--- a/doc/usage/cmd/bind.rst
+++ b/doc/usage/cmd/bind.rst
@@ -12,7 +12,7 @@ Synopsis
 ::

     bind <node path> <driver>
-    bind <class> <index> <driver>
+    bind <class> <seq> <driver>

 Description
 -----------
@@ -30,8 +30,8 @@ node path
 class
     device class name

-index
-    index of the parent device in the device class
+seq
+    sequence number of the parent device in the device class

 driver
     device driver name
@@ -46,7 +46,7 @@ using the two alternative bind syntaxes.
 .. code-block::

     => dm tree
-     Class     Index  Probed  Driver                Name
+     Class     Seq    Probed  Driver                Name
     -----------------------------------------------------------
      root          0  [ + ]   root_driver           root_driver
     ...
@@ -75,13 +75,13 @@ using the two alternative bind syntaxes.
     => date
     Cannot find RTC: err=-19
     => dm tree
-     Class     Index  Probed  Driver                Name
+     Class     Seq    Probed  Driver                Name
     -----------------------------------------------------------
      root          0  [ + ]   root_driver           root_driver
     ...
     => bind /pl031@9010000 rtc-pl031
     => dm tree
-     Class     Index  Probed  Driver                Name
+     Class     Seq    Probed  Driver                Name
     -----------------------------------------------------------
      root          0  [ + ]   root_driver           root_driver
     ...
diff --git a/doc/usage/cmd/unbind.rst b/doc/usage/cmd/unbind.rst
index 0309e90f6e..1ae9c1b172 100644
--- a/doc/usage/cmd/unbind.rst
+++ b/doc/usage/cmd/unbind.rst
@@ -12,8 +12,8 @@ Synopsis
 ::

     unbind <node path>
-    unbind <class> <index>
-    unbind <class> <index> <driver>
+    unbind <class> <seq>
+    unbind <class> <seq> <driver>

 Description
 -----------
@@ -27,8 +27,8 @@ node path
 class
     device class name

-index
-    index of the device in the device class
+seq
+    sequence number of the device in the device class

 driver
     device driver name
@@ -43,7 +43,7 @@ using the three alternative unbind syntaxes.
 .. code-block::

     => dm tree
-     Class     Index  Probed  Driver                Name
+     Class     Seq    Probed  Driver                Name
     -----------------------------------------------------------
      root          0  [ + ]   root_driver           root_driver
     ...
@@ -70,7 +70,7 @@ using the three alternative unbind syntaxes.
     }
     => unbind /pl031@9010000
     => dm tree
-     Class     Index  Probed  Driver                Name
+     Class     Seq    Probed  Driver                Name
     -----------------------------------------------------------
      root          0  [ + ]   root_driver           root_driver
     ...
@@ -78,7 +78,7 @@ using the three alternative unbind syntaxes.
     Cannot find a device with path /pl031@9010000
     => bind /pl031@9010000 rtc-pl031
     => dm tree
-     Class     Index  Probed  Driver                Name
+     Class     Seq    Probed  Driver                Name
     -----------------------------------------------------------
      root          0  [ + ]   root_driver           root_driver
     ...
--
2.45.2


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

* Re: [PATCH] dm: core: Show device sequence instead in dm_dump_tree()
  2024-08-02  9:28 ` [PATCH] dm: core: Show device sequence instead in dm_dump_tree() Zixun LI
@ 2024-08-06 21:50   ` Simon Glass
  2024-08-27 23:22   ` Tom Rini
  1 sibling, 0 replies; 15+ messages in thread
From: Simon Glass @ 2024-08-06 21:50 UTC (permalink / raw)
  To: Zixun LI
  Cc: Tom Rini, Lukasz Majewski, Mattijs Korpershoek, Marek Vasut,
	u-boot

On Fri, 2 Aug 2024 at 03:31, Zixun LI <admin@hifiphile.com> wrote:
>
> Currently uclass index is shown in DM tree dump which ignores alias
> sequence numbering. The result could be confusing since these 2 numbers
> could be different. Show device sequence number instead as it's more
> meaningful.
>
> Also update documentation to use sequence number.
>
> Signed-off-by: Zixun LI <admin@hifiphile.com>
> ---
>  doc/usage/cmd/dm.rst | 7 +++----
>  drivers/core/dump.c  | 4 ++--
>  2 files changed, 5 insertions(+), 6 deletions(-)

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


>
> diff --git a/doc/usage/cmd/dm.rst b/doc/usage/cmd/dm.rst
> index 7651507937..196b22d137 100644
> --- a/doc/usage/cmd/dm.rst
> +++ b/doc/usage/cmd/dm.rst
> @@ -112,9 +112,8 @@ This shows the full tree of devices including the following fields:
>  uclass
>      Shows the name of the uclass for the device
>
> -Index
> -    Shows the index number of the device, within the uclass. This shows the
> -    ordering within the uclass, but not the sequence number.
> +Seq
> +    Shows the sequence number of the device, within the uclass.
>
>  Probed
>      Shows `+` if the device is active
> @@ -366,7 +365,7 @@ dm tree
>  This example shows the abridged sandbox output::
>
>      => dm tree
> -    Class     Index  Probed  Driver                Name
> +    Class     Seq    Probed  Driver                Name
>      -----------------------------------------------------------
>      root          0  [ + ]   root_driver           root_driver
>      demo          0  [   ]   demo_shape_drv        |-- demo_shape_drv
> diff --git a/drivers/core/dump.c b/drivers/core/dump.c
> index 5ec30d5b3c..5cbaa97fa3 100644
> --- a/drivers/core/dump.c
> +++ b/drivers/core/dump.c
> @@ -40,7 +40,7 @@ static void show_devices(struct udevice *dev, int depth, int last_flag,
>         /* print the first 20 characters to not break the tree-format. */
>         printf(CONFIG_IS_ENABLED(USE_TINY_PRINTF) ? " %s  %d  [ %c ]   %s  " :
>                " %-10.10s  %3d  [ %c ]   %-20.20s  ", dev->uclass->uc_drv->name,
> -              dev_get_uclass_index(dev, NULL),
> +              dev->seq_,
>                flags & DM_FLAG_ACTIVATED ? '+' : ' ', dev->driver->name);
>
>         for (i = depth; i >= 0; i--) {
> @@ -129,7 +129,7 @@ void dm_dump_tree(char *dev_name, bool extended, bool sort)
>  {
>         struct udevice *root;
>
> -       printf(" Class     Index  Probed  Driver                Name\n");
> +       printf(" Class     Seq    Probed  Driver                Name\n");
>         printf("-----------------------------------------------------------\n");
>
>         root = dm_root();
> --
> 2.45.2
>

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

* Re: [PATCH] usb: gadget: udc: Fix duplicate uclass name
  2024-08-02  9:28 [PATCH] usb: gadget: udc: Fix duplicate uclass name Zixun LI
  2024-08-02  9:28 ` [PATCH] dm: core: Show device sequence instead in dm_dump_tree() Zixun LI
  2024-08-02  9:28 ` [PATCH] cmd: bind: Use device sequence instead for driver bind/unbind Zixun LI
@ 2024-08-06 21:50 ` Simon Glass
  2024-08-07  7:07 ` Mattijs Korpershoek
  3 siblings, 0 replies; 15+ messages in thread
From: Simon Glass @ 2024-08-06 21:50 UTC (permalink / raw)
  To: Zixun LI
  Cc: Tom Rini, Lukasz Majewski, Mattijs Korpershoek, Marek Vasut,
	u-boot

On Fri, 2 Aug 2024 at 03:31, Zixun LI <admin@hifiphile.com> wrote:
>
> Currently both USB host uclass and USB gadget uclass are using the same
> name "usb" which break uclass functions like uclass_get_by_name().
>
> Rename the uclass to "usb_gadget" to fix, also makes bind/unbind by class
> index (or sequence) working.
>
> This breaks the capacity of using "usb" as DT alias sequence numbering
> which needs a fix afterwards.
>
> Signed-off-by: Zixun LI <admin@hifiphile.com>
> ---
>  drivers/usb/gadget/udc/udc-uclass.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

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

* Re: [PATCH] cmd: bind: Use device sequence instead for driver bind/unbind
  2024-08-02  9:28 ` [PATCH] cmd: bind: Use device sequence instead for driver bind/unbind Zixun LI
@ 2024-08-06 21:50   ` Simon Glass
  2024-08-20  6:23   ` Mattijs Korpershoek
  2024-08-27 23:22   ` Tom Rini
  2 siblings, 0 replies; 15+ messages in thread
From: Simon Glass @ 2024-08-06 21:50 UTC (permalink / raw)
  To: Zixun LI
  Cc: Tom Rini, Lukasz Majewski, Mattijs Korpershoek, Marek Vasut,
	u-boot

On Fri, 2 Aug 2024 at 03:31, Zixun LI <admin@hifiphile.com> wrote:
>
> Currently uclass index is used for bind/unbind which ignores alias
> sequence numbering. Use device sequence number instead as it's
> the number explicitly set in the DT.
>
> Also update documentation to use sequence number.
>
> Signed-off-by: Zixun LI <admin@hifiphile.com>
> ---
>  cmd/bind.c               | 46 ++++++++++++++++++++--------------------
>  doc/usage/cmd/bind.rst   | 12 +++++------
>  doc/usage/cmd/unbind.rst | 14 ++++++------
>  3 files changed, 36 insertions(+), 36 deletions(-)
>

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

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

* Re: [PATCH] usb: gadget: udc: Fix duplicate uclass name
  2024-08-02  9:28 [PATCH] usb: gadget: udc: Fix duplicate uclass name Zixun LI
                   ` (2 preceding siblings ...)
  2024-08-06 21:50 ` [PATCH] usb: gadget: udc: Fix duplicate uclass name Simon Glass
@ 2024-08-07  7:07 ` Mattijs Korpershoek
  2024-08-07 12:36   ` Zixun LI
  3 siblings, 1 reply; 15+ messages in thread
From: Mattijs Korpershoek @ 2024-08-07  7:07 UTC (permalink / raw)
  To: Zixun LI, Simon Glass, Tom Rini, Lukasz Majewski, Marek Vasut
  Cc: Zixun LI, u-boot

Hi Zixun,

Thank you for the patch.

On ven., août 02, 2024 at 11:28, Zixun LI <admin@hifiphile.com> wrote:

> Currently both USB host uclass and USB gadget uclass are using the same
> name "usb" which break uclass functions like uclass_get_by_name().
>
> Rename the uclass to "usb_gadget" to fix, also makes bind/unbind by class
> index (or sequence) working.
>
> This breaks the capacity of using "usb" as DT alias sequence numbering
> which needs a fix afterwards.

Have you identified boards which use the DT alias that will break
with this patch?

Maybe we can detail the required fix in the commit message a bit as well?
Or, if you know of a board that uses "usb" as DT alias sequence number,
we can submit a fix alongside with this one to document the fix.

>
> Signed-off-by: Zixun LI <admin@hifiphile.com>
> ---
>  drivers/usb/gadget/udc/udc-uclass.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/udc/udc-uclass.c b/drivers/usb/gadget/udc/udc-uclass.c
> index fbe62bbce4..723d1cdfd7 100644
> --- a/drivers/usb/gadget/udc/udc-uclass.c
> +++ b/drivers/usb/gadget/udc/udc-uclass.c
> @@ -83,7 +83,7 @@ __weak int dm_usb_gadget_handle_interrupts(struct udevice *dev)
>  #if CONFIG_IS_ENABLED(DM)
>  UCLASS_DRIVER(usb_gadget_generic) = {
>  	.id		= UCLASS_USB_GADGET_GENERIC,
> -	.name		= "usb",
> +	.name		= "usb_gadget",
>  	.flags		= DM_UC_FLAG_SEQ_ALIAS,
>  };
>  #endif
> --
> 2.45.2

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

* Re: [PATCH] usb: gadget: udc: Fix duplicate uclass name
  2024-08-07  7:07 ` Mattijs Korpershoek
@ 2024-08-07 12:36   ` Zixun LI
  2024-08-13  8:28     ` Mattijs Korpershoek
  0 siblings, 1 reply; 15+ messages in thread
From: Zixun LI @ 2024-08-07 12:36 UTC (permalink / raw)
  To: Mattijs Korpershoek
  Cc: Simon Glass, Tom Rini, Lukasz Majewski, Marek Vasut, u-boot

Hi Mattijs,

On Wed, Aug 7, 2024 at 9:07 AM Mattijs Korpershoek
<mkorpershoek@baylibre.com> wrote:
>
>
> Have you identified boards which use the DT alias that will break
> with this patch?
>
> Maybe we can detail the required fix in the commit message a bit as well?
> Or, if you know of a board that uses "usb" as DT alias sequence number,
> we can submit a fix alongside with this one to document the fix.

I did a search and it affects mostly i.MX devices and Nuvoton npcm845 where
host and device controller use the same alias.

like imx6qdl.dtsi:
aliases {
...
    usb0 = &usbotg;
    usb1 = &usbh1;
    usb2 = &usbh2;
    usb3 = &usbh3;
};

nuvoton-npcm845-evb.dts:
aliases {
...
    usb0 = &udc0;
    usb1 = &ehci1;
    usb2 = &ehci2;
};

But I got lost in EHCI's code, like in ehci_register() when ctrl->init ==
USB_INIT_DEVICE the function returns without the actual device init ?
I'm not sure how their gadget mode works and if it's really broken or not.

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

* Re: [PATCH] usb: gadget: udc: Fix duplicate uclass name
  2024-08-07 12:36   ` Zixun LI
@ 2024-08-13  8:28     ` Mattijs Korpershoek
  2024-08-13 13:39       ` Zixun LI
  0 siblings, 1 reply; 15+ messages in thread
From: Mattijs Korpershoek @ 2024-08-13  8:28 UTC (permalink / raw)
  To: Zixun LI
  Cc: Simon Glass, Tom Rini, Lukasz Majewski, Marek Vasut, u-boot,
	Jagan Teki, Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team

Hi Zixun,

On mer., août 07, 2024 at 14:36, Zixun LI <admin@hifiphile.com> wrote:

> Hi Mattijs,
>
> On Wed, Aug 7, 2024 at 9:07 AM Mattijs Korpershoek
> <mkorpershoek@baylibre.com> wrote:
>>
>>
>> Have you identified boards which use the DT alias that will break
>> with this patch?
>>
>> Maybe we can detail the required fix in the commit message a bit as well?
>> Or, if you know of a board that uses "usb" as DT alias sequence number,
>> we can submit a fix alongside with this one to document the fix.
>
> I did a search and it affects mostly i.MX devices and Nuvoton npcm845 where
> host and device controller use the same alias.
>
> like imx6qdl.dtsi:
> aliases {
> ...
>     usb0 = &usbotg;
>     usb1 = &usbh1;
>     usb2 = &usbh2;
>     usb3 = &usbh3;
> };
>
> nuvoton-npcm845-evb.dts:
> aliases {
> ...
>     usb0 = &udc0;
>     usb1 = &ehci1;
>     usb2 = &ehci2;
> };
>
> But I got lost in EHCI's code, like in ehci_register() when ctrl->init ==
> USB_INIT_DEVICE the function returns without the actual device init ?
> I'm not sure how their gadget mode works and if it's really broken or not.

Thank you for giving some board examples. I am still a bit unclear on
the meaning of:

"""
This breaks the capacity of using "usb" as DT alias sequence numbering
which needs a fix afterwards.
"""

I have added Jagan, Stefano, Fabio and the NXP team in CC. Does anyone
of you have any concerns with this patch ?

If someone could test it, that would be helpful.

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

* Re: [PATCH] usb: gadget: udc: Fix duplicate uclass name
  2024-08-13  8:28     ` Mattijs Korpershoek
@ 2024-08-13 13:39       ` Zixun LI
  2024-08-16 15:37         ` Mattijs Korpershoek
  0 siblings, 1 reply; 15+ messages in thread
From: Zixun LI @ 2024-08-13 13:39 UTC (permalink / raw)
  To: Mattijs Korpershoek
  Cc: Simon Glass, Tom Rini, Lukasz Majewski, Marek Vasut, u-boot,
	Jagan Teki, Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team

Hi Mattijs,

On Tue, Aug 13, 2024 at 10:28 AM Mattijs Korpershoek
<mkorpershoek@baylibre.com> wrote:
>
> Thank you for giving some board examples. I am still a bit unclear on
> the meaning of:
>
> """
> This breaks the capacity of using "usb" as DT alias sequence numbering
> which needs a fix afterwards.
> """
>
> I have added Jagan, Stefano, Fabio and the NXP team in CC. Does anyone
> of you have any concerns with this patch ?
>
> If someone could test it, that would be helpful.

The device sequence number is affected by uclass_find_next_free_seq() in
uclass.c, in this function uclass name is used to determine the number.

Since the gadget class's name changed to "usb_gadget" from "usb", alias
binding "usb1 = &usbotg;" is not effective anymore. As now it searches for
"usb_gadget1 = &usbotg;".

uclass_find_device_by_seq(UCLASS_USB_GADGET_GENERIC,) would fail as sequence
number is changed.

I made a tentative fix in
https://lore.kernel.org/u-boot/20240731134257.686017-2-admin@hifiphile.com/
But Simon doesn't want to modify uclass_driver structure only for gadget
class.

Regards,
Zixun

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

* Re: [PATCH] usb: gadget: udc: Fix duplicate uclass name
  2024-08-13 13:39       ` Zixun LI
@ 2024-08-16 15:37         ` Mattijs Korpershoek
  0 siblings, 0 replies; 15+ messages in thread
From: Mattijs Korpershoek @ 2024-08-16 15:37 UTC (permalink / raw)
  To: Zixun LI
  Cc: Simon Glass, Tom Rini, Lukasz Majewski, Marek Vasut, u-boot,
	Jagan Teki, Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team

Hi Zixun,

On mar., août 13, 2024 at 15:39, Zixun LI <admin@hifiphile.com> wrote:

> Hi Mattijs,
>
> On Tue, Aug 13, 2024 at 10:28 AM Mattijs Korpershoek
> <mkorpershoek@baylibre.com> wrote:
>>
>> Thank you for giving some board examples. I am still a bit unclear on
>> the meaning of:
>>
>> """
>> This breaks the capacity of using "usb" as DT alias sequence numbering
>> which needs a fix afterwards.
>> """
>>
>> I have added Jagan, Stefano, Fabio and the NXP team in CC. Does anyone
>> of you have any concerns with this patch ?
>>
>> If someone could test it, that would be helpful.
>
> The device sequence number is affected by uclass_find_next_free_seq() in
> uclass.c, in this function uclass name is used to determine the number.
>
> Since the gadget class's name changed to "usb_gadget" from "usb", alias
> binding "usb1 = &usbotg;" is not effective anymore. As now it searches for
> "usb_gadget1 = &usbotg;".
>
> uclass_find_device_by_seq(UCLASS_USB_GADGET_GENERIC,) would fail as sequence
> number is changed.

Thank you for the explanation.
Looking at the code, I don't see anyone calling:
- uclass_find_device_by_seq(UCLASS_USB_GADGET

And one occurrence of:
- uclass_get_device_by_seq(UCLASS_USB_GADGET_GENERIC, index, &dev);

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

>
> I made a tentative fix in
> https://lore.kernel.org/u-boot/20240731134257.686017-2-admin@hifiphile.com/
> But Simon doesn't want to modify uclass_driver structure only for gadget
> class.
>
> Regards,
> Zixun

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

* Re: [PATCH] cmd: bind: Use device sequence instead for driver bind/unbind
  2024-08-02  9:28 ` [PATCH] cmd: bind: Use device sequence instead for driver bind/unbind Zixun LI
  2024-08-06 21:50   ` Simon Glass
@ 2024-08-20  6:23   ` Mattijs Korpershoek
  2024-08-20  7:39     ` Mattijs Korpershoek
  2024-08-27 23:22   ` Tom Rini
  2 siblings, 1 reply; 15+ messages in thread
From: Mattijs Korpershoek @ 2024-08-20  6:23 UTC (permalink / raw)
  To: Simon Glass, Tom Rini, Lukasz Majewski, Marek Vasut, Zixun LI; +Cc: u-boot

Hi,

On Fri, 02 Aug 2024 11:28:13 +0200, Zixun LI wrote:
> Currently uclass index is used for bind/unbind which ignores alias
> sequence numbering. Use device sequence number instead as it's
> the number explicitly set in the DT.
> 
> Also update documentation to use sequence number.
> 
> 
> [...]

Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)

[1/1] cmd: bind: Use device sequence instead for driver bind/unbind
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/0c48507eee08b5e7d4e8484727efe47cfabff0ee

--
Mattijs

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

* Re: [PATCH] cmd: bind: Use device sequence instead for driver bind/unbind
  2024-08-20  6:23   ` Mattijs Korpershoek
@ 2024-08-20  7:39     ` Mattijs Korpershoek
  0 siblings, 0 replies; 15+ messages in thread
From: Mattijs Korpershoek @ 2024-08-20  7:39 UTC (permalink / raw)
  To: Simon Glass, Tom Rini, Lukasz Majewski, Marek Vasut, Zixun LI
  Cc: u-boot, tools

On mar., août 20, 2024 at 08:23, Mattijs Korpershoek <mkorpershoek@baylibre.com> wrote:

> Hi,
>
> On Fri, 02 Aug 2024 11:28:13 +0200, Zixun LI wrote:
>> Currently uclass index is used for bind/unbind which ignores alias
>> sequence numbering. Use device sequence number instead as it's
>> the number explicitly set in the DT.
>> 
>> Also update documentation to use sequence number.
>> 
>> 
>> [...]
>
> Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)
>
> [1/1] cmd: bind: Use device sequence instead for driver bind/unbind
>       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/0c48507eee08b5e7d4e8484727efe47cfabff0ee

Somehow, b4 did the wrong thing here when running:
$ b4 shazam -s -l --check 20240802092820.917450-1-admin@hifiphile.com

Grabbing thread from lore.kernel.org/all/20240802092820.917450-1-admin@hifiphile.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 12 messages in the thread
Assuming new revision: v2 ([PATCH] dm: core: Show device sequence instead in dm_dump_tree())
Assuming new revision: v3 ([PATCH] cmd: bind: Use device sequence instead for driver bind/unbind)
Analyzing 0 code-review messages
Will use the latest revision: v3
You can pick other revisions using the -vN flag
Checking attestation on all messages, may take a moment...
---
  ✗ [PATCH] cmd: bind: Use device sequence instead for driver bind/unbind
    + Reviewed-by: Simon Glass <sjg@chromium.org> (✗ DKIM/chromium.org)
    + Link: https://lore.kernel.org/r/20240802092820.917450-3-admin@hifiphile.com
    + Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
    ● checkpatch.pl: passed all checks
  ---
  ✗ BADSIG: DKIM/hifiphile-com.20230601.gappssmtp.com
---
Total patches: 1
---
Applying: cmd: bind: Use device sequence instead for driver bind/unbind

It applied:
https://patchwork.ozlabs.org/project/uboot/patch/20240802092820.917450-3-admin@hifiphile.com/

Instead of:
https://patchwork.ozlabs.org/project/uboot/patch/20240802092820.917450-1-admin@hifiphile.com/

I have fixed this up manually, see:
https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/73f5b54a589cb6ff97aaca55e7c97d68b43997ee

>
> --
> Mattijs

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

* Re: [PATCH] dm: core: Show device sequence instead in dm_dump_tree()
  2024-08-02  9:28 ` [PATCH] dm: core: Show device sequence instead in dm_dump_tree() Zixun LI
  2024-08-06 21:50   ` Simon Glass
@ 2024-08-27 23:22   ` Tom Rini
  1 sibling, 0 replies; 15+ messages in thread
From: Tom Rini @ 2024-08-27 23:22 UTC (permalink / raw)
  To: Simon Glass, Lukasz Majewski, Mattijs Korpershoek, Marek Vasut,
	Zixun LI
  Cc: u-boot

On Fri, 02 Aug 2024 11:28:12 +0200, Zixun LI wrote:

> Currently uclass index is shown in DM tree dump which ignores alias
> sequence numbering. The result could be confusing since these 2 numbers
> could be different. Show device sequence number instead as it's more
> meaningful.
> 
> Also update documentation to use sequence number.
> 
> [...]

Applied to u-boot/next, thanks!

-- 
Tom



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

* Re: [PATCH] cmd: bind: Use device sequence instead for driver bind/unbind
  2024-08-02  9:28 ` [PATCH] cmd: bind: Use device sequence instead for driver bind/unbind Zixun LI
  2024-08-06 21:50   ` Simon Glass
  2024-08-20  6:23   ` Mattijs Korpershoek
@ 2024-08-27 23:22   ` Tom Rini
  2 siblings, 0 replies; 15+ messages in thread
From: Tom Rini @ 2024-08-27 23:22 UTC (permalink / raw)
  To: Simon Glass, Lukasz Majewski, Mattijs Korpershoek, Marek Vasut,
	Zixun LI
  Cc: u-boot

On Fri, 02 Aug 2024 11:28:13 +0200, Zixun LI wrote:

> Currently uclass index is used for bind/unbind which ignores alias
> sequence numbering. Use device sequence number instead as it's
> the number explicitly set in the DT.
> 
> Also update documentation to use sequence number.
> 
> 
> [...]

Applied to u-boot/next, thanks!

-- 
Tom



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

end of thread, other threads:[~2024-08-27 23:23 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-02  9:28 [PATCH] usb: gadget: udc: Fix duplicate uclass name Zixun LI
2024-08-02  9:28 ` [PATCH] dm: core: Show device sequence instead in dm_dump_tree() Zixun LI
2024-08-06 21:50   ` Simon Glass
2024-08-27 23:22   ` Tom Rini
2024-08-02  9:28 ` [PATCH] cmd: bind: Use device sequence instead for driver bind/unbind Zixun LI
2024-08-06 21:50   ` Simon Glass
2024-08-20  6:23   ` Mattijs Korpershoek
2024-08-20  7:39     ` Mattijs Korpershoek
2024-08-27 23:22   ` Tom Rini
2024-08-06 21:50 ` [PATCH] usb: gadget: udc: Fix duplicate uclass name Simon Glass
2024-08-07  7:07 ` Mattijs Korpershoek
2024-08-07 12:36   ` Zixun LI
2024-08-13  8:28     ` Mattijs Korpershoek
2024-08-13 13:39       ` Zixun LI
2024-08-16 15:37         ` Mattijs Korpershoek

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.