[PATCH v2] usb: common: usb-conn-gpio: use a unique name for usb connector device

[PATCH v2] usb: common: usb-conn-gpio: use a unique name for usb connector device

[Chance Yang]

The current implementation of the usb-conn-gpio driver uses a fixed
"usb-charger" name for all USB connector devices. This causes conflicts
in the power supply subsystem when multiple USB connectors are present,
as duplicate names are not allowed.

Use IDA to manage unique IDs for naming usb connectors (e.g.,
usb-charger-0, usb-charger-1).

Fixes: 880287910b189 ("usb: common: usb-conn-gpio: fix NULL pointer dereference of charger")
Signed-off-by: Chance Yang <chance.yang@kneron.us>
---
Changes in v2:
- Replaced atomic_t with IDA for unique ID management
- Link to v1: https://lore.kernel.org/r/20250411-work-next-v1-1-93c4b95ee6c1@kneron.us
---
 drivers/usb/common/usb-conn-gpio.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/common/usb-conn-gpio.c b/drivers/usb/common/usb-conn-gpio.c
index 1e36be2a28fd5ca5e1495b7923e4d3e25d7cedef..421c3af38e06975259f4a1792aa3b3708a192d59 100644
--- a/drivers/usb/common/usb-conn-gpio.c
+++ b/drivers/usb/common/usb-conn-gpio.c
@@ -21,6 +21,9 @@
 #include <linux/regulator/consumer.h>
 #include <linux/string_choices.h>
 #include <linux/usb/role.h>
+#include <linux/idr.h>
+
+static DEFINE_IDA(usb_conn_ida);
 
 #define USB_GPIO_DEB_MS		20	/* ms */
 #define USB_GPIO_DEB_US		((USB_GPIO_DEB_MS) * 1000)	/* us */
@@ -30,6 +33,7 @@
 
 struct usb_conn_info {
 	struct device *dev;
+	int conn_id; /* store the IDA-allocated ID */
 	struct usb_role_switch *role_sw;
 	enum usb_role last_role;
 	struct regulator *vbus;
@@ -161,7 +165,17 @@ static int usb_conn_psy_register(struct usb_conn_info *info)
 		.fwnode = dev_fwnode(dev),
 	};
 
-	desc->name = "usb-charger";
+	info->conn_id = ida_alloc(&usb_conn_ida, GFP_KERNEL);
+	if (info->conn_id < 0)
+		return info->conn_id;
+
+	desc->name = devm_kasprintf(dev, GFP_KERNEL, "usb-charger-%d",
+				    info->conn_id);
+	if (!desc->name) {
+		ida_free(&usb_conn_ida, info->conn_id);
+		return -ENOMEM;
+	}
+
 	desc->properties = usb_charger_properties;
 	desc->num_properties = ARRAY_SIZE(usb_charger_properties);
 	desc->get_property = usb_charger_get_property;
@@ -169,8 +183,10 @@ static int usb_conn_psy_register(struct usb_conn_info *info)
 	cfg.drv_data = info;
 
 	info->charger = devm_power_supply_register(dev, desc, &cfg);
-	if (IS_ERR(info->charger))
-		dev_err(dev, "Unable to register charger\n");
+	if (IS_ERR(info->charger)) {
+		dev_err(dev, "Unable to register charger %d\n", info->conn_id);
+		ida_free(&usb_conn_ida, info->conn_id);
+	}
 
 	return PTR_ERR_OR_ZERO(info->charger);
 }
@@ -278,6 +294,9 @@ static void usb_conn_remove(struct platform_device *pdev)
 
 	cancel_delayed_work_sync(&info->dw_det);
 
+	if (info->charger)
+		ida_free(&usb_conn_ida, info->conn_id);
+
 	if (info->last_role == USB_ROLE_HOST && info->vbus)
 		regulator_disable(info->vbus);
 

---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250411-work-next-d817787d63f2

Best regards,
-- 
Chance Yang <chance.yang@kneron.us>

Re: [PATCH v2] usb: common: usb-conn-gpio: use a unique name for usb connector device

[Greg Kroah-Hartman]

On Fri, Apr 11, 2025 at 03:47:08PM +0800, Chance Yang wrote:
> The current implementation of the usb-conn-gpio driver uses a fixed
> "usb-charger" name for all USB connector devices. This causes conflicts
> in the power supply subsystem when multiple USB connectors are present,
> as duplicate names are not allowed.
> 
> Use IDA to manage unique IDs for naming usb connectors (e.g.,
> usb-charger-0, usb-charger-1).
> 
> Fixes: 880287910b189 ("usb: common: usb-conn-gpio: fix NULL pointer dereference of charger")
> Signed-off-by: Chance Yang <chance.yang@kneron.us>
> ---
> Changes in v2:
> - Replaced atomic_t with IDA for unique ID management
> - Link to v1: https://lore.kernel.org/r/20250411-work-next-v1-1-93c4b95ee6c1@kneron.us
> ---
>  drivers/usb/common/usb-conn-gpio.c | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/common/usb-conn-gpio.c b/drivers/usb/common/usb-conn-gpio.c
> index 1e36be2a28fd5ca5e1495b7923e4d3e25d7cedef..421c3af38e06975259f4a1792aa3b3708a192d59 100644
> --- a/drivers/usb/common/usb-conn-gpio.c
> +++ b/drivers/usb/common/usb-conn-gpio.c
> @@ -21,6 +21,9 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/string_choices.h>
>  #include <linux/usb/role.h>
> +#include <linux/idr.h>
> +
> +static DEFINE_IDA(usb_conn_ida);
>  
>  #define USB_GPIO_DEB_MS		20	/* ms */
>  #define USB_GPIO_DEB_US		((USB_GPIO_DEB_MS) * 1000)	/* us */
> @@ -30,6 +33,7 @@
>  
>  struct usb_conn_info {
>  	struct device *dev;
> +	int conn_id; /* store the IDA-allocated ID */
>  	struct usb_role_switch *role_sw;
>  	enum usb_role last_role;
>  	struct regulator *vbus;
> @@ -161,7 +165,17 @@ static int usb_conn_psy_register(struct usb_conn_info *info)
>  		.fwnode = dev_fwnode(dev),
>  	};
>  
> -	desc->name = "usb-charger";
> +	info->conn_id = ida_alloc(&usb_conn_ida, GFP_KERNEL);
> +	if (info->conn_id < 0)
> +		return info->conn_id;
> +
> +	desc->name = devm_kasprintf(dev, GFP_KERNEL, "usb-charger-%d",
> +				    info->conn_id);
> +	if (!desc->name) {
> +		ida_free(&usb_conn_ida, info->conn_id);
> +		return -ENOMEM;
> +	}
> +
>  	desc->properties = usb_charger_properties;
>  	desc->num_properties = ARRAY_SIZE(usb_charger_properties);
>  	desc->get_property = usb_charger_get_property;
> @@ -169,8 +183,10 @@ static int usb_conn_psy_register(struct usb_conn_info *info)
>  	cfg.drv_data = info;
>  
>  	info->charger = devm_power_supply_register(dev, desc, &cfg);
> -	if (IS_ERR(info->charger))
> -		dev_err(dev, "Unable to register charger\n");
> +	if (IS_ERR(info->charger)) {
> +		dev_err(dev, "Unable to register charger %d\n", info->conn_id);
> +		ida_free(&usb_conn_ida, info->conn_id);
> +	}
>  
>  	return PTR_ERR_OR_ZERO(info->charger);
>  }
> @@ -278,6 +294,9 @@ static void usb_conn_remove(struct platform_device *pdev)
>  
>  	cancel_delayed_work_sync(&info->dw_det);
>  
> +	if (info->charger)
> +		ida_free(&usb_conn_ida, info->conn_id);
> +
>  	if (info->last_role == USB_ROLE_HOST && info->vbus)
>  		regulator_disable(info->vbus);
>  
> 
> ---
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> change-id: 20250411-work-next-d817787d63f2
> 
> Best regards,
> -- 
> Chance Yang <chance.yang@kneron.us>
> 
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You have marked a patch with a "Fixes:" tag for a commit that is in an
  older released kernel, yet you do not have a cc: stable line in the
  signed-off-by area at all, which means that the patch will not be
  applied to any older kernel releases.  To properly fix this, please
  follow the documented rules in the
  Documentation/process/stable-kernel-rules.rst file for how to resolve
  this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot