[RFC PATCH 0/4] dm: Duplicate uclass name fix and alias improvements

[RFC PATCH 0/4] dm: Duplicate uclass name fix and alias improvements

[Zixun LI]

Patch 1 is a tentative fix for duplicate uclass name issue met in
https://lists.denx.de/pipermail/u-boot/2024-July/560189.html

The idea is to use orignal class name only for sequence alias to keep this
alias function working and rename the class something else.

Patch 2 and 3 make "dm tree", "bind" and "unbind" commands to take care
of alias sequence numbering. As the alias sequence numbering is more
meanful than uclass index.

Patch 4 is the actual fix for usb gadet class.

There are some situations where uclass index and device sequence number
are misused which is not convered by this patch set, for example in
drivers/net/sandbox.c, uclass_get_device() is used which is based on
uclass index, while in the comments it says "index - The alias index
(also DM seq number)"

Zixun LI (4):
  dm: core: Add a way to specify an alt name for alias sequence
    numbering
  dm: core: Show device sequence instead in dm_dump_tree()
  cmd: bind: Use device sequence instead for driver bind/unbind
  usb: gadget: udc: Fix duplicate uclass name

 cmd/bind.c                          |  4 ++--
 drivers/core/device.c               |  3 ++-
 drivers/core/dump.c                 |  2 +-
 drivers/core/read.c                 |  7 ++++++-
 drivers/core/uclass.c               | 11 +++++++++--
 drivers/usb/gadget/udc/udc-uclass.c |  3 ++-
 include/dm/read.h                   |  9 ++++++++-
 include/dm/uclass.h                 |  2 ++
 8 files changed, 32 insertions(+), 9 deletions(-)

--
2.45.2

[RFC PATCH 1/4] dm: core: Add a way to specify an alt name for alias sequence numbering

[Zixun LI]

A new field name_seq_alias is added to uclass_driver structure, which
allows an uclass driver to use an alternate name for alias sequence
numbering.

For example an uclass named "usb_gadget" can share alias with
"usb" uclass :
    UCLASS_DRIVER(usb_gadget_generic) = {
	    .id = UCLASS_USB_GADGET_GENERIC,
	    .name = "usb_gadget",
	    .name_seq_alias = "usb",
    	.flags = DM_UC_FLAG_SEQ_ALIAS,
    };

Currently there are some uclasses with duplicate name which break uclass
functions like uclass_get_by_name(), with this patch it's now possible
to rename these classes without break the existing alias function.

Signed-off-by: Zixun LI <admin@hifiphile.com>
---
 drivers/core/device.c |  3 ++-
 drivers/core/read.c   |  7 ++++++-
 drivers/core/uclass.c | 11 +++++++++--
 include/dm/read.h     |  9 ++++++++-
 include/dm/uclass.h   |  2 ++
 5 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/core/device.c b/drivers/core/device.c
index 779f371b9d..a98e75b0b8 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -89,7 +89,8 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv,
 		 */
 		if (CONFIG_IS_ENABLED(OF_CONTROL) &&
 		    !CONFIG_IS_ENABLED(OF_PLATDATA)) {
-			if (uc->uc_drv->name && ofnode_valid(node)) {
+			if ((uc->uc_drv->name || uc->uc_drv->name_seq_alias) &&
+			    ofnode_valid(node)) {
 				if (!dev_read_alias_seq(dev, &dev->seq_)) {
 					auto_seq = false;
 					log_debug("   - seq=%d\n", dev->seq_);
diff --git a/drivers/core/read.c b/drivers/core/read.c
index 55c19f335a..97b12fb7a9 100644
--- a/drivers/core/read.c
+++ b/drivers/core/read.c
@@ -347,9 +347,14 @@ const void *dev_read_prop_by_prop(struct ofprop *prop,
 int dev_read_alias_seq(const struct udevice *dev, int *devnump)
 {
 	ofnode node = dev_ofnode(dev);
-	const char *uc_name = dev->uclass->uc_drv->name;
+	const char *uc_name;
 	int ret = -ENOTSUPP;
 
+	if (dev->uclass->uc_drv->name_seq_alias)
+		uc_name = dev->uclass->uc_drv->name_seq_alias;
+	else
+		uc_name = dev->uclass->uc_drv->name;
+
 	if (ofnode_is_np(node)) {
 		ret = of_alias_get_id(ofnode_to_np(node), uc_name);
 		if (ret >= 0) {
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index 7ae0884a75..6dfbee2e78 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -308,11 +308,18 @@ int uclass_find_next_free_seq(struct uclass *uc)
 {
 	struct udevice *dev;
 	int max = -1;
+	const char *uc_name;
 
 	/* If using aliases, start with the highest alias value */
 	if (CONFIG_IS_ENABLED(DM_SEQ_ALIAS) &&
-	    (uc->uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS))
-		max = dev_read_alias_highest_id(uc->uc_drv->name);
+	    (uc->uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS)) {
+		if (uc->uc_drv->name_seq_alias)
+			uc_name = uc->uc_drv->name_seq_alias;
+		else
+			uc_name = uc->uc_drv->name;
+
+		max = dev_read_alias_highest_id(uc_name);
+	}
 
 	/* Avoid conflict with existing devices */
 	list_for_each_entry(dev, &uc->dev_head, uclass_node) {
diff --git a/include/dm/read.h b/include/dm/read.h
index 894bc698bb..5a86e43d86 100644
--- a/include/dm/read.h
+++ b/include/dm/read.h
@@ -1169,7 +1169,14 @@ static inline const void *dev_read_prop_by_prop(struct ofprop *prop,
 static inline int dev_read_alias_seq(const struct udevice *dev, int *devnump)
 {
 #if CONFIG_IS_ENABLED(OF_CONTROL)
-	return fdtdec_get_alias_seq(gd->fdt_blob, dev->uclass->uc_drv->name,
+	const char *uc_name;
+
+	if (dev->uclass->uc_drv->name_seq_alias)
+		uc_name = dev->uclass->uc_drv->name_seq_alias;
+	else
+		uc_name = dev->uclass->uc_drv->name;
+
+	return fdtdec_get_alias_seq(gd->fdt_blob, uc_name,
 				    dev_of_offset(dev), devnump);
 #else
 	return -ENOTSUPP;
diff --git a/include/dm/uclass.h b/include/dm/uclass.h
index 456eef7f2f..8339e0d88b 100644
--- a/include/dm/uclass.h
+++ b/include/dm/uclass.h
@@ -57,6 +57,7 @@ struct udevice;
  * drivers.
  *
  * @name: Name of uclass driver
+ * @name_seq_alias: Name used for alias sequence numbering
  * @id: ID number of this uclass
  * @post_bind: Called after a new device is bound to this uclass
  * @pre_unbind: Called before a device is unbound from this uclass
@@ -88,6 +89,7 @@ struct udevice;
  */
 struct uclass_driver {
 	const char *name;
+	const char *name_seq_alias;
 	enum uclass_id id;
 	int (*post_bind)(struct udevice *dev);
 	int (*pre_unbind)(struct udevice *dev);
-- 
2.45.2

[RFC PATCH 2/4] dm: core: Show device sequence instead in dm_dump_tree()

[Zixun LI]

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.

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

diff --git a/drivers/core/dump.c b/drivers/core/dump.c
index 5ec30d5b3c..c812d87fa4 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--) {
-- 
2.45.2

Re: [RFC PATCH 2/4] dm: core: Show device sequence instead in dm_dump_tree()

[Simon Glass]

Hi Zixun,

On Wed, 31 Jul 2024 at 07:43, 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.

I think it makes sense to show the seq instead of the index, but
please update the header for the table to say 'Seq'

>
> Signed-off-by: Zixun LI <admin@hifiphile.com>
> ---
>  drivers/core/dump.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/core/dump.c b/drivers/core/dump.c
> index 5ec30d5b3c..c812d87fa4 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--) {
> --
> 2.45.2
>

Regards,
Simon

[RFC PATCH 3/4] cmd: bind: Use device sequence instead for driver bind/unbind

[Zixun LI]

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.

Signed-off-by: Zixun LI <admin@hifiphile.com>
---
 cmd/bind.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cmd/bind.c b/cmd/bind.c
index 3a59eefd5c..30f1163e17 100644
--- a/cmd/bind.c
+++ b/cmd/bind.c
@@ -31,7 +31,7 @@ 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, index, &parent);
 	if (!parent || ret) {
 		printf("Cannot find device %d of class %s\n", index, uclass);
 		return ret;
@@ -58,7 +58,7 @@ 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, index, devp);
 	if (!*devp || rc) {
 		printf("Cannot find device %d of class %s\n", index, uclass);
 		return rc;
-- 
2.45.2

Re: [RFC PATCH 3/4] cmd: bind: Use device sequence instead for driver bind/unbind

[Simon Glass]

Hi Zixun,

On Wed, 31 Jul 2024 at 07:43, 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.
>
> Signed-off-by: Zixun LI <admin@hifiphile.com>
> ---
>  cmd/bind.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

I assume no tests need updating, but please do add a note to
doc/usage/cmd/bind.rst

> diff --git a/cmd/bind.c b/cmd/bind.c
> index 3a59eefd5c..30f1163e17 100644
> --- a/cmd/bind.c
> +++ b/cmd/bind.c
> @@ -31,7 +31,7 @@ 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, index, &parent);
>         if (!parent || ret) {
>                 printf("Cannot find device %d of class %s\n", index, uclass);
>                 return ret;
> @@ -58,7 +58,7 @@ 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, index, devp);
>         if (!*devp || rc) {
>                 printf("Cannot find device %d of class %s\n", index, uclass);
>                 return rc;
> --
> 2.45.2
>

Regards,
Simon

[RFC PATCH 4/4] usb: gadget: udc: Fix duplicate uclass name

[Zixun LI]

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" while using "usb" as sequence alias
naming to keep the alias function working.

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

diff --git a/drivers/usb/gadget/udc/udc-uclass.c b/drivers/usb/gadget/udc/udc-uclass.c
index fbe62bbce4..b14f84e818 100644
--- a/drivers/usb/gadget/udc/udc-uclass.c
+++ b/drivers/usb/gadget/udc/udc-uclass.c
@@ -83,7 +83,8 @@ __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",
+	.name_seq_alias = "usb",
 	.flags		= DM_UC_FLAG_SEQ_ALIAS,
 };
 #endif
-- 
2.45.2

Re: [RFC PATCH 0/4] dm: Duplicate uclass name fix and alias improvements

[Simon Glass]

Hi Zixun,

On Wed, 31 Jul 2024 at 07:43, Zixun LI <admin@hifiphile.com> wrote:
>
> Patch 1 is a tentative fix for duplicate uclass name issue met in
> https://lists.denx.de/pipermail/u-boot/2024-July/560189.html
>
> The idea is to use orignal class name only for sequence alias to keep this
> alias function working and rename the class something else.

Can we rename the gadget uclass to usb_gadget or similar, then update
the aliases?

>
> Patch 2 and 3 make "dm tree", "bind" and "unbind" commands to take care
> of alias sequence numbering. As the alias sequence numbering is more
> meanful than uclass index.
>
> Patch 4 is the actual fix for usb gadet class.
>
> There are some situations where uclass index and device sequence number
> are misused which is not convered by this patch set, for example in
> drivers/net/sandbox.c, uclass_get_device() is used which is based on
> uclass index, while in the comments it says "index - The alias index
> (also DM seq number)"
>
> Zixun LI (4):
>   dm: core: Add a way to specify an alt name for alias sequence
>     numbering
>   dm: core: Show device sequence instead in dm_dump_tree()
>   cmd: bind: Use device sequence instead for driver bind/unbind
>   usb: gadget: udc: Fix duplicate uclass name
>
>  cmd/bind.c                          |  4 ++--
>  drivers/core/device.c               |  3 ++-
>  drivers/core/dump.c                 |  2 +-
>  drivers/core/read.c                 |  7 ++++++-
>  drivers/core/uclass.c               | 11 +++++++++--
>  drivers/usb/gadget/udc/udc-uclass.c |  3 ++-
>  include/dm/read.h                   |  9 ++++++++-
>  include/dm/uclass.h                 |  2 ++
>  8 files changed, 32 insertions(+), 9 deletions(-)
>
> --
> 2.45.2
>

Regards,
Simon

Re: [RFC PATCH 0/4] dm: Duplicate uclass name fix and alias improvements

[Zixun LI]

Hi Simon,

On Thu, Aug 1, 2024 at 4:42 PM Simon Glass <sjg@chromium.org> wrote:
>
> Can we rename the gadget uclass to usb_gadget or similar, then update
> the aliases?
>

You mean split patch no.4 into 2 parts, first rename the class to usb_gadget
before doing the alias job ?

> I think it makes sense to show the seq instead of the index, but
> please update the header for the table to say 'Seq'

Will do.

> I assume no tests need updating, but please do add a note to
> doc/usage/cmd/bind.rst

Will do.

Regards,
Zixun LI

Re: [RFC PATCH 0/4] dm: Duplicate uclass name fix and alias improvements

[Simon Glass]

Hi Zixun,

On Thu, 1 Aug 2024 at 12:37, Zixun LI <admin@hifiphile.com> wrote:
>
> Hi Simon,
>
> On Thu, Aug 1, 2024 at 4:42 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Can we rename the gadget uclass to usb_gadget or similar, then update
> > the aliases?
> >
>
> You mean split patch no.4 into 2 parts, first rename the class to usb_gadget
> before doing the alias job ?

Yes I suppose so. I am trying to avoid adding the extra field to the
uclass, just for the use of one uclass.

>
> > I think it makes sense to show the seq instead of the index, but
> > please update the header for the table to say 'Seq'
>
> Will do.
>
> > I assume no tests need updating, but please do add a note to
> > doc/usage/cmd/bind.rst
>
> Will do.
>
> Regards,
> Zixun LI

Regards,
Simon