Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [RESEND PATCH] usb: common: usb-conn-gpio: Register optional charger
@ 2020-06-21 22:48 Paul Cercueil
  2020-07-21 11:41 ` Greg Kroah-Hartman
  2020-07-26 10:14 ` Andy Shevchenko
  0 siblings, 2 replies; 10+ messages in thread
From: Paul Cercueil @ 2020-06-21 22:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Chunfeng Yun, od, linux-usb, linux-kernel, Paul Cercueil

Register a power supply charger, if the Kconfig option
USB_CONN_GPIO_CHARGER is set, whose online state depends on whether
the USB role is set to device or not.

This is useful when the USB role is the only way to know if the device
is charging from USB. The API is the standard power supply charger API,
you get a /sys/class/power_supply/xxx/online node which tells you the
state of the charger.

The sole purpose of this is to give userspace applications a way to
know whether or not the charger is plugged.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/usb/common/Kconfig         | 11 +++++++
 drivers/usb/common/usb-conn-gpio.c | 47 ++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/drivers/usb/common/Kconfig b/drivers/usb/common/Kconfig
index d611477aae41..5405ae96c68f 100644
--- a/drivers/usb/common/Kconfig
+++ b/drivers/usb/common/Kconfig
@@ -49,3 +49,14 @@ config USB_CONN_GPIO
 
 	  To compile the driver as a module, choose M here: the module will
 	  be called usb-conn-gpio.ko
+
+if USB_CONN_GPIO
+
+config USB_CONN_GPIO_CHARGER
+	bool "USB charger support"
+	select POWER_SUPPLY
+	help
+	  Register a charger with the power supply subsystem. This will allow
+	  userspace to know whether or not the device is charging from USB.
+
+endif
diff --git a/drivers/usb/common/usb-conn-gpio.c b/drivers/usb/common/usb-conn-gpio.c
index ed204cbb63ea..129d48db280b 100644
--- a/drivers/usb/common/usb-conn-gpio.c
+++ b/drivers/usb/common/usb-conn-gpio.c
@@ -17,6 +17,7 @@
 #include <linux/of.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
+#include <linux/power_supply.h>
 #include <linux/regulator/consumer.h>
 #include <linux/usb/role.h>
 
@@ -38,6 +39,9 @@ struct usb_conn_info {
 	struct gpio_desc *vbus_gpiod;
 	int id_irq;
 	int vbus_irq;
+
+	struct power_supply_desc desc;
+	struct power_supply *charger;
 };
 
 /**
@@ -98,6 +102,8 @@ static void usb_conn_detect_cable(struct work_struct *work)
 		ret = regulator_enable(info->vbus);
 		if (ret)
 			dev_err(info->dev, "enable vbus regulator failed\n");
+	} else if (IS_ENABLED(CONFIG_USB_CONN_GPIO_CHARGER)) {
+		power_supply_changed(info->charger);
 	}
 
 	info->last_role = role;
@@ -121,10 +127,35 @@ static irqreturn_t usb_conn_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static enum power_supply_property usb_charger_properties[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+};
+
+static int usb_charger_get_property(struct power_supply *psy,
+				    enum power_supply_property psp,
+				    union power_supply_propval *val)
+{
+	struct usb_conn_info *info = power_supply_get_drvdata(psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval = info->last_role == USB_ROLE_DEVICE;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int usb_conn_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct power_supply_desc *desc;
 	struct usb_conn_info *info;
+	struct power_supply_config cfg = {
+		.of_node = dev->of_node,
+	};
 	int ret = 0;
 
 	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
@@ -203,6 +234,22 @@ static int usb_conn_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (IS_ENABLED(CONFIG_USB_CONN_GPIO_CHARGER)) {
+		desc = &info->desc;
+		desc->name = "usb-charger";
+		desc->properties = usb_charger_properties;
+		desc->num_properties = ARRAY_SIZE(usb_charger_properties);
+		desc->get_property = usb_charger_get_property;
+		desc->type = POWER_SUPPLY_TYPE_USB;
+		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");
+			return PTR_ERR(info->charger);
+		}
+	}
+
 	platform_set_drvdata(pdev, info);
 
 	/* Perform initial detection */
-- 
2.27.0


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

* Re: [RESEND PATCH] usb: common: usb-conn-gpio: Register optional charger
  2020-06-21 22:48 [RESEND PATCH] usb: common: usb-conn-gpio: Register optional charger Paul Cercueil
@ 2020-07-21 11:41 ` Greg Kroah-Hartman
  2020-07-25 17:51   ` Paul Cercueil
  2020-07-26 10:14 ` Andy Shevchenko
  1 sibling, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2020-07-21 11:41 UTC (permalink / raw)
  To: Paul Cercueil; +Cc: Chunfeng Yun, od, linux-usb, linux-kernel

On Mon, Jun 22, 2020 at 12:48:07AM +0200, Paul Cercueil wrote:
> Register a power supply charger, if the Kconfig option
> USB_CONN_GPIO_CHARGER is set, whose online state depends on whether
> the USB role is set to device or not.
> 
> This is useful when the USB role is the only way to know if the device
> is charging from USB. The API is the standard power supply charger API,
> you get a /sys/class/power_supply/xxx/online node which tells you the
> state of the charger.
> 
> The sole purpose of this is to give userspace applications a way to
> know whether or not the charger is plugged.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  drivers/usb/common/Kconfig         | 11 +++++++
>  drivers/usb/common/usb-conn-gpio.c | 47 ++++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/drivers/usb/common/Kconfig b/drivers/usb/common/Kconfig
> index d611477aae41..5405ae96c68f 100644
> --- a/drivers/usb/common/Kconfig
> +++ b/drivers/usb/common/Kconfig
> @@ -49,3 +49,14 @@ config USB_CONN_GPIO
>  
>  	  To compile the driver as a module, choose M here: the module will
>  	  be called usb-conn-gpio.ko
> +
> +if USB_CONN_GPIO
> +
> +config USB_CONN_GPIO_CHARGER
> +	bool "USB charger support"
> +	select POWER_SUPPLY
> +	help
> +	  Register a charger with the power supply subsystem. This will allow
> +	  userspace to know whether or not the device is charging from USB.

Why make this an option at all?  Why wouldn't we always want this here?

As this is a charger, exporting that information to userspace should
probably always happen, right?

thanks,

greg k-h

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

* Re: [RESEND PATCH] usb: common: usb-conn-gpio: Register optional charger
  2020-07-21 11:41 ` Greg Kroah-Hartman
@ 2020-07-25 17:51   ` Paul Cercueil
  2020-07-26  8:39     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 10+ messages in thread
From: Paul Cercueil @ 2020-07-25 17:51 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Chunfeng Yun, od, linux-usb, linux-kernel

Hi Greg,

Le mar. 21 juil. 2020 à 13:41, Greg Kroah-Hartman 
<gregkh@linuxfoundation.org> a écrit :
> On Mon, Jun 22, 2020 at 12:48:07AM +0200, Paul Cercueil wrote:
>>  Register a power supply charger, if the Kconfig option
>>  USB_CONN_GPIO_CHARGER is set, whose online state depends on whether
>>  the USB role is set to device or not.
>> 
>>  This is useful when the USB role is the only way to know if the 
>> device
>>  is charging from USB. The API is the standard power supply charger 
>> API,
>>  you get a /sys/class/power_supply/xxx/online node which tells you 
>> the
>>  state of the charger.
>> 
>>  The sole purpose of this is to give userspace applications a way to
>>  know whether or not the charger is plugged.
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>>   drivers/usb/common/Kconfig         | 11 +++++++
>>   drivers/usb/common/usb-conn-gpio.c | 47 
>> ++++++++++++++++++++++++++++++
>>   2 files changed, 58 insertions(+)
>> 
>>  diff --git a/drivers/usb/common/Kconfig b/drivers/usb/common/Kconfig
>>  index d611477aae41..5405ae96c68f 100644
>>  --- a/drivers/usb/common/Kconfig
>>  +++ b/drivers/usb/common/Kconfig
>>  @@ -49,3 +49,14 @@ config USB_CONN_GPIO
>> 
>>   	  To compile the driver as a module, choose M here: the module 
>> will
>>   	  be called usb-conn-gpio.ko
>>  +
>>  +if USB_CONN_GPIO
>>  +
>>  +config USB_CONN_GPIO_CHARGER
>>  +	bool "USB charger support"
>>  +	select POWER_SUPPLY
>>  +	help
>>  +	  Register a charger with the power supply subsystem. This will 
>> allow
>>  +	  userspace to know whether or not the device is charging from 
>> USB.
> 
> Why make this an option at all?  Why wouldn't we always want this 
> here?
> 
> As this is a charger, exporting that information to userspace should
> probably always happen, right?

I wanted to avoid the hardcoded dependency on CONFIG_POWER_SUPPLY.

I can very well make that non-optional.

Cheers,
-Paul

> thanks,
> 
> greg k-h



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

* Re: [RESEND PATCH] usb: common: usb-conn-gpio: Register optional charger
  2020-07-25 17:51   ` Paul Cercueil
@ 2020-07-26  8:39     ` Greg Kroah-Hartman
  2020-07-26 10:07       ` Paul Cercueil
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2020-07-26  8:39 UTC (permalink / raw)
  To: Paul Cercueil; +Cc: Chunfeng Yun, od, linux-usb, linux-kernel

On Sat, Jul 25, 2020 at 07:51:14PM +0200, Paul Cercueil wrote:
> Hi Greg,
> 
> Le mar. 21 juil. 2020 à 13:41, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> a écrit :
> > On Mon, Jun 22, 2020 at 12:48:07AM +0200, Paul Cercueil wrote:
> > >  Register a power supply charger, if the Kconfig option
> > >  USB_CONN_GPIO_CHARGER is set, whose online state depends on whether
> > >  the USB role is set to device or not.
> > > 
> > >  This is useful when the USB role is the only way to know if the
> > > device
> > >  is charging from USB. The API is the standard power supply charger
> > > API,
> > >  you get a /sys/class/power_supply/xxx/online node which tells you
> > > the
> > >  state of the charger.
> > > 
> > >  The sole purpose of this is to give userspace applications a way to
> > >  know whether or not the charger is plugged.
> > > 
> > >  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > >  ---
> > >   drivers/usb/common/Kconfig         | 11 +++++++
> > >   drivers/usb/common/usb-conn-gpio.c | 47
> > > ++++++++++++++++++++++++++++++
> > >   2 files changed, 58 insertions(+)
> > > 
> > >  diff --git a/drivers/usb/common/Kconfig b/drivers/usb/common/Kconfig
> > >  index d611477aae41..5405ae96c68f 100644
> > >  --- a/drivers/usb/common/Kconfig
> > >  +++ b/drivers/usb/common/Kconfig
> > >  @@ -49,3 +49,14 @@ config USB_CONN_GPIO
> > > 
> > >   	  To compile the driver as a module, choose M here: the module
> > > will
> > >   	  be called usb-conn-gpio.ko
> > >  +
> > >  +if USB_CONN_GPIO
> > >  +
> > >  +config USB_CONN_GPIO_CHARGER
> > >  +	bool "USB charger support"
> > >  +	select POWER_SUPPLY
> > >  +	help
> > >  +	  Register a charger with the power supply subsystem. This will
> > > allow
> > >  +	  userspace to know whether or not the device is charging from
> > > USB.
> > 
> > Why make this an option at all?  Why wouldn't we always want this here?
> > 
> > As this is a charger, exporting that information to userspace should
> > probably always happen, right?
> 
> I wanted to avoid the hardcoded dependency on CONFIG_POWER_SUPPLY.
> 
> I can very well make that non-optional.

As the whole reason for this driver is to be a power supply, make it
depend on the power supply core please.

thanks,

greg k-h

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

* Re: [RESEND PATCH] usb: common: usb-conn-gpio: Register optional charger
  2020-07-26  8:39     ` Greg Kroah-Hartman
@ 2020-07-26 10:07       ` Paul Cercueil
  0 siblings, 0 replies; 10+ messages in thread
From: Paul Cercueil @ 2020-07-26 10:07 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Chunfeng Yun, od, linux-usb, linux-kernel



Le dim. 26 juil. 2020 à 10:39, Greg Kroah-Hartman 
<gregkh@linuxfoundation.org> a écrit :
> On Sat, Jul 25, 2020 at 07:51:14PM +0200, Paul Cercueil wrote:
>>  Hi Greg,
>> 
>>  Le mar. 21 juil. 2020 à 13:41, Greg Kroah-Hartman
>>  <gregkh@linuxfoundation.org> a écrit :
>>  > On Mon, Jun 22, 2020 at 12:48:07AM +0200, Paul Cercueil wrote:
>>  > >  Register a power supply charger, if the Kconfig option
>>  > >  USB_CONN_GPIO_CHARGER is set, whose online state depends on 
>> whether
>>  > >  the USB role is set to device or not.
>>  > >
>>  > >  This is useful when the USB role is the only way to know if the
>>  > > device
>>  > >  is charging from USB. The API is the standard power supply 
>> charger
>>  > > API,
>>  > >  you get a /sys/class/power_supply/xxx/online node which tells 
>> you
>>  > > the
>>  > >  state of the charger.
>>  > >
>>  > >  The sole purpose of this is to give userspace applications a 
>> way to
>>  > >  know whether or not the charger is plugged.
>>  > >
>>  > >  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  > >  ---
>>  > >   drivers/usb/common/Kconfig         | 11 +++++++
>>  > >   drivers/usb/common/usb-conn-gpio.c | 47
>>  > > ++++++++++++++++++++++++++++++
>>  > >   2 files changed, 58 insertions(+)
>>  > >
>>  > >  diff --git a/drivers/usb/common/Kconfig 
>> b/drivers/usb/common/Kconfig
>>  > >  index d611477aae41..5405ae96c68f 100644
>>  > >  --- a/drivers/usb/common/Kconfig
>>  > >  +++ b/drivers/usb/common/Kconfig
>>  > >  @@ -49,3 +49,14 @@ config USB_CONN_GPIO
>>  > >
>>  > >   	  To compile the driver as a module, choose M here: the 
>> module
>>  > > will
>>  > >   	  be called usb-conn-gpio.ko
>>  > >  +
>>  > >  +if USB_CONN_GPIO
>>  > >  +
>>  > >  +config USB_CONN_GPIO_CHARGER
>>  > >  +	bool "USB charger support"
>>  > >  +	select POWER_SUPPLY
>>  > >  +	help
>>  > >  +	  Register a charger with the power supply subsystem. This 
>> will
>>  > > allow
>>  > >  +	  userspace to know whether or not the device is charging 
>> from
>>  > > USB.
>>  >
>>  > Why make this an option at all?  Why wouldn't we always want this 
>> here?
>>  >
>>  > As this is a charger, exporting that information to userspace 
>> should
>>  > probably always happen, right?
>> 
>>  I wanted to avoid the hardcoded dependency on CONFIG_POWER_SUPPLY.
>> 
>>  I can very well make that non-optional.
> 
> As the whole reason for this driver is to be a power supply, make it
> depend on the power supply core please.

It's the whole reason of my patch, not of the driver itself, whose 
purpose is to report the USB role (device, host, unconnected).

-Paul



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

* Re: [RESEND PATCH] usb: common: usb-conn-gpio: Register optional charger
  2020-06-21 22:48 [RESEND PATCH] usb: common: usb-conn-gpio: Register optional charger Paul Cercueil
  2020-07-21 11:41 ` Greg Kroah-Hartman
@ 2020-07-26 10:14 ` Andy Shevchenko
  2020-07-26 10:27   ` Paul Cercueil
  1 sibling, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2020-07-26 10:14 UTC (permalink / raw)
  To: Paul Cercueil, Chanwoo Choi, MyungJoo Ham
  Cc: Greg Kroah-Hartman, Chunfeng Yun, od, USB, Linux Kernel Mailing List

On Mon, Jun 22, 2020 at 1:51 AM Paul Cercueil <paul@crapouillou.net> wrote:
>
> Register a power supply charger, if the Kconfig option
> USB_CONN_GPIO_CHARGER is set, whose online state depends on whether
> the USB role is set to device or not.
>
> This is useful when the USB role is the only way to know if the device
> is charging from USB. The API is the standard power supply charger API,
> you get a /sys/class/power_supply/xxx/online node which tells you the
> state of the charger.
>
> The sole purpose of this is to give userspace applications a way to
> know whether or not the charger is plugged.

I'm not sure I understand the purpose of this (third?) way to detect
USB charger and notify user space about.
Why is extcon not good enough?

> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  drivers/usb/common/Kconfig         | 11 +++++++
>  drivers/usb/common/usb-conn-gpio.c | 47 ++++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
>
> diff --git a/drivers/usb/common/Kconfig b/drivers/usb/common/Kconfig
> index d611477aae41..5405ae96c68f 100644
> --- a/drivers/usb/common/Kconfig
> +++ b/drivers/usb/common/Kconfig
> @@ -49,3 +49,14 @@ config USB_CONN_GPIO
>
>           To compile the driver as a module, choose M here: the module will
>           be called usb-conn-gpio.ko
> +
> +if USB_CONN_GPIO
> +
> +config USB_CONN_GPIO_CHARGER
> +       bool "USB charger support"
> +       select POWER_SUPPLY
> +       help
> +         Register a charger with the power supply subsystem. This will allow
> +         userspace to know whether or not the device is charging from USB.
> +
> +endif
> diff --git a/drivers/usb/common/usb-conn-gpio.c b/drivers/usb/common/usb-conn-gpio.c
> index ed204cbb63ea..129d48db280b 100644
> --- a/drivers/usb/common/usb-conn-gpio.c
> +++ b/drivers/usb/common/usb-conn-gpio.c
> @@ -17,6 +17,7 @@
>  #include <linux/of.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/platform_device.h>
> +#include <linux/power_supply.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/usb/role.h>
>
> @@ -38,6 +39,9 @@ struct usb_conn_info {
>         struct gpio_desc *vbus_gpiod;
>         int id_irq;
>         int vbus_irq;
> +
> +       struct power_supply_desc desc;
> +       struct power_supply *charger;
>  };
>
>  /**
> @@ -98,6 +102,8 @@ static void usb_conn_detect_cable(struct work_struct *work)
>                 ret = regulator_enable(info->vbus);
>                 if (ret)
>                         dev_err(info->dev, "enable vbus regulator failed\n");
> +       } else if (IS_ENABLED(CONFIG_USB_CONN_GPIO_CHARGER)) {
> +               power_supply_changed(info->charger);
>         }
>
>         info->last_role = role;
> @@ -121,10 +127,35 @@ static irqreturn_t usb_conn_isr(int irq, void *dev_id)
>         return IRQ_HANDLED;
>  }
>
> +static enum power_supply_property usb_charger_properties[] = {
> +       POWER_SUPPLY_PROP_ONLINE,
> +};
> +
> +static int usb_charger_get_property(struct power_supply *psy,
> +                                   enum power_supply_property psp,
> +                                   union power_supply_propval *val)
> +{
> +       struct usb_conn_info *info = power_supply_get_drvdata(psy);
> +
> +       switch (psp) {
> +       case POWER_SUPPLY_PROP_ONLINE:
> +               val->intval = info->last_role == USB_ROLE_DEVICE;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
>  static int usb_conn_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
> +       struct power_supply_desc *desc;
>         struct usb_conn_info *info;
> +       struct power_supply_config cfg = {
> +               .of_node = dev->of_node,
> +       };
>         int ret = 0;
>
>         info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
> @@ -203,6 +234,22 @@ static int usb_conn_probe(struct platform_device *pdev)
>                 }
>         }
>
> +       if (IS_ENABLED(CONFIG_USB_CONN_GPIO_CHARGER)) {
> +               desc = &info->desc;
> +               desc->name = "usb-charger";
> +               desc->properties = usb_charger_properties;
> +               desc->num_properties = ARRAY_SIZE(usb_charger_properties);
> +               desc->get_property = usb_charger_get_property;
> +               desc->type = POWER_SUPPLY_TYPE_USB;
> +               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");
> +                       return PTR_ERR(info->charger);
> +               }
> +       }
> +
>         platform_set_drvdata(pdev, info);
>
>         /* Perform initial detection */
> --
> 2.27.0
>


-- 
With Best Regards,
Andy Shevchenko

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

* Re: [RESEND PATCH] usb: common: usb-conn-gpio: Register optional charger
  2020-07-26 10:14 ` Andy Shevchenko
@ 2020-07-26 10:27   ` Paul Cercueil
  2020-07-27  5:42     ` Chunfeng Yun
  0 siblings, 1 reply; 10+ messages in thread
From: Paul Cercueil @ 2020-07-26 10:27 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Chanwoo Choi, MyungJoo Ham, Greg Kroah-Hartman, Chunfeng Yun, od,
	USB, Linux Kernel Mailing List



Le dim. 26 juil. 2020 à 13:14, Andy Shevchenko 
<andy.shevchenko@gmail.com> a écrit :
> On Mon, Jun 22, 2020 at 1:51 AM Paul Cercueil <paul@crapouillou.net> 
> wrote:
>> 
>>  Register a power supply charger, if the Kconfig option
>>  USB_CONN_GPIO_CHARGER is set, whose online state depends on whether
>>  the USB role is set to device or not.
>> 
>>  This is useful when the USB role is the only way to know if the 
>> device
>>  is charging from USB. The API is the standard power supply charger 
>> API,
>>  you get a /sys/class/power_supply/xxx/online node which tells you 
>> the
>>  state of the charger.
>> 
>>  The sole purpose of this is to give userspace applications a way to
>>  know whether or not the charger is plugged.
> 
> I'm not sure I understand the purpose of this (third?) way to detect
> USB charger and notify user space about.
> Why is extcon not good enough?

We can't have extcon and USB role detection at the same time.

-Paul

>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>>   drivers/usb/common/Kconfig         | 11 +++++++
>>   drivers/usb/common/usb-conn-gpio.c | 47 
>> ++++++++++++++++++++++++++++++
>>   2 files changed, 58 insertions(+)
>> 
>>  diff --git a/drivers/usb/common/Kconfig b/drivers/usb/common/Kconfig
>>  index d611477aae41..5405ae96c68f 100644
>>  --- a/drivers/usb/common/Kconfig
>>  +++ b/drivers/usb/common/Kconfig
>>  @@ -49,3 +49,14 @@ config USB_CONN_GPIO
>> 
>>            To compile the driver as a module, choose M here: the 
>> module will
>>            be called usb-conn-gpio.ko
>>  +
>>  +if USB_CONN_GPIO
>>  +
>>  +config USB_CONN_GPIO_CHARGER
>>  +       bool "USB charger support"
>>  +       select POWER_SUPPLY
>>  +       help
>>  +         Register a charger with the power supply subsystem. This 
>> will allow
>>  +         userspace to know whether or not the device is charging 
>> from USB.
>>  +
>>  +endif
>>  diff --git a/drivers/usb/common/usb-conn-gpio.c 
>> b/drivers/usb/common/usb-conn-gpio.c
>>  index ed204cbb63ea..129d48db280b 100644
>>  --- a/drivers/usb/common/usb-conn-gpio.c
>>  +++ b/drivers/usb/common/usb-conn-gpio.c
>>  @@ -17,6 +17,7 @@
>>   #include <linux/of.h>
>>   #include <linux/pinctrl/consumer.h>
>>   #include <linux/platform_device.h>
>>  +#include <linux/power_supply.h>
>>   #include <linux/regulator/consumer.h>
>>   #include <linux/usb/role.h>
>> 
>>  @@ -38,6 +39,9 @@ struct usb_conn_info {
>>          struct gpio_desc *vbus_gpiod;
>>          int id_irq;
>>          int vbus_irq;
>>  +
>>  +       struct power_supply_desc desc;
>>  +       struct power_supply *charger;
>>   };
>> 
>>   /**
>>  @@ -98,6 +102,8 @@ static void usb_conn_detect_cable(struct 
>> work_struct *work)
>>                  ret = regulator_enable(info->vbus);
>>                  if (ret)
>>                          dev_err(info->dev, "enable vbus regulator 
>> failed\n");
>>  +       } else if (IS_ENABLED(CONFIG_USB_CONN_GPIO_CHARGER)) {
>>  +               power_supply_changed(info->charger);
>>          }
>> 
>>          info->last_role = role;
>>  @@ -121,10 +127,35 @@ static irqreturn_t usb_conn_isr(int irq, void 
>> *dev_id)
>>          return IRQ_HANDLED;
>>   }
>> 
>>  +static enum power_supply_property usb_charger_properties[] = {
>>  +       POWER_SUPPLY_PROP_ONLINE,
>>  +};
>>  +
>>  +static int usb_charger_get_property(struct power_supply *psy,
>>  +                                   enum power_supply_property psp,
>>  +                                   union power_supply_propval *val)
>>  +{
>>  +       struct usb_conn_info *info = power_supply_get_drvdata(psy);
>>  +
>>  +       switch (psp) {
>>  +       case POWER_SUPPLY_PROP_ONLINE:
>>  +               val->intval = info->last_role == USB_ROLE_DEVICE;
>>  +               break;
>>  +       default:
>>  +               return -EINVAL;
>>  +       }
>>  +
>>  +       return 0;
>>  +}
>>  +
>>   static int usb_conn_probe(struct platform_device *pdev)
>>   {
>>          struct device *dev = &pdev->dev;
>>  +       struct power_supply_desc *desc;
>>          struct usb_conn_info *info;
>>  +       struct power_supply_config cfg = {
>>  +               .of_node = dev->of_node,
>>  +       };
>>          int ret = 0;
>> 
>>          info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
>>  @@ -203,6 +234,22 @@ static int usb_conn_probe(struct 
>> platform_device *pdev)
>>                  }
>>          }
>> 
>>  +       if (IS_ENABLED(CONFIG_USB_CONN_GPIO_CHARGER)) {
>>  +               desc = &info->desc;
>>  +               desc->name = "usb-charger";
>>  +               desc->properties = usb_charger_properties;
>>  +               desc->num_properties = 
>> ARRAY_SIZE(usb_charger_properties);
>>  +               desc->get_property = usb_charger_get_property;
>>  +               desc->type = POWER_SUPPLY_TYPE_USB;
>>  +               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");
>>  +                       return PTR_ERR(info->charger);
>>  +               }
>>  +       }
>>  +
>>          platform_set_drvdata(pdev, info);
>> 
>>          /* Perform initial detection */
>>  --
>>  2.27.0
>> 
> 
> 
> --
> With Best Regards,
> Andy Shevchenko



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

* Re: [RESEND PATCH] usb: common: usb-conn-gpio: Register optional charger
  2020-07-26 10:27   ` Paul Cercueil
@ 2020-07-27  5:42     ` Chunfeng Yun
  2020-07-27 15:02       ` Paul Cercueil
  0 siblings, 1 reply; 10+ messages in thread
From: Chunfeng Yun @ 2020-07-27  5:42 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Andy Shevchenko, Chanwoo Choi, MyungJoo Ham, Greg Kroah-Hartman,
	od, USB, Linux Kernel Mailing List

On Sun, 2020-07-26 at 12:27 +0200, Paul Cercueil wrote:
> 
> Le dim. 26 juil. 2020 à 13:14, Andy Shevchenko 
> <andy.shevchenko@gmail.com> a écrit :
> > On Mon, Jun 22, 2020 at 1:51 AM Paul Cercueil <paul@crapouillou.net> 
> > wrote:
> >> 
> >>  Register a power supply charger, if the Kconfig option
> >>  USB_CONN_GPIO_CHARGER is set, whose online state depends on whether
> >>  the USB role is set to device or not.
> >> 
> >>  This is useful when the USB role is the only way to know if the 
> >> device
> >>  is charging from USB. The API is the standard power supply charger 
> >> API,
> >>  you get a /sys/class/power_supply/xxx/online node which tells you 
> >> the
> >>  state of the charger.
> >> 
> >>  The sole purpose of this is to give userspace applications a way to
> >>  know whether or not the charger is plugged.
> > 
> > I'm not sure I understand the purpose of this (third?) way to detect
> > USB charger and notify user space about.
> > Why is extcon not good enough?
> 
> We can't have extcon and USB role detection at the same time.
> 
> -Paul
> 
> >>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> >>  ---
> >>   drivers/usb/common/Kconfig         | 11 +++++++
> >>   drivers/usb/common/usb-conn-gpio.c | 47 
> >> ++++++++++++++++++++++++++++++
> >>   2 files changed, 58 insertions(+)
> >> 
> >>  diff --git a/drivers/usb/common/Kconfig b/drivers/usb/common/Kconfig
> >>  index d611477aae41..5405ae96c68f 100644
> >>  --- a/drivers/usb/common/Kconfig
> >>  +++ b/drivers/usb/common/Kconfig
> >>  @@ -49,3 +49,14 @@ config USB_CONN_GPIO
> >> 
> >>            To compile the driver as a module, choose M here: the 
> >> module will
> >>            be called usb-conn-gpio.ko
> >>  +
> >>  +if USB_CONN_GPIO
> >>  +
> >>  +config USB_CONN_GPIO_CHARGER
> >>  +       bool "USB charger support"
> >>  +       select POWER_SUPPLY
> >>  +       help
> >>  +         Register a charger with the power supply subsystem. This 
> >> will allow
> >>  +         userspace to know whether or not the device is charging 
> >> from USB.
> >>  +
> >>  +endif
> >>  diff --git a/drivers/usb/common/usb-conn-gpio.c 
> >> b/drivers/usb/common/usb-conn-gpio.c
> >>  index ed204cbb63ea..129d48db280b 100644
> >>  --- a/drivers/usb/common/usb-conn-gpio.c
> >>  +++ b/drivers/usb/common/usb-conn-gpio.c
> >>  @@ -17,6 +17,7 @@
> >>   #include <linux/of.h>
> >>   #include <linux/pinctrl/consumer.h>
> >>   #include <linux/platform_device.h>
> >>  +#include <linux/power_supply.h>
> >>   #include <linux/regulator/consumer.h>
> >>   #include <linux/usb/role.h>
> >> 
> >>  @@ -38,6 +39,9 @@ struct usb_conn_info {
> >>          struct gpio_desc *vbus_gpiod;
> >>          int id_irq;
> >>          int vbus_irq;
> >>  +
> >>  +       struct power_supply_desc desc;
> >>  +       struct power_supply *charger;
> >>   };
> >> 
> >>   /**
> >>  @@ -98,6 +102,8 @@ static void usb_conn_detect_cable(struct 
> >> work_struct *work)
> >>                  ret = regulator_enable(info->vbus);
> >>                  if (ret)
> >>                          dev_err(info->dev, "enable vbus regulator 
> >> failed\n");
> >>  +       } else if (IS_ENABLED(CONFIG_USB_CONN_GPIO_CHARGER)) {
> >>  +               power_supply_changed(info->charger);
> >>          }
> >> 
> >>          info->last_role = role;
> >>  @@ -121,10 +127,35 @@ static irqreturn_t usb_conn_isr(int irq, void 
> >> *dev_id)
> >>          return IRQ_HANDLED;
> >>   }
> >> 
> >>  +static enum power_supply_property usb_charger_properties[] = {
> >>  +       POWER_SUPPLY_PROP_ONLINE,
> >>  +};
> >>  +
> >>  +static int usb_charger_get_property(struct power_supply *psy,
> >>  +                                   enum power_supply_property psp,
> >>  +                                   union power_supply_propval *val)
> >>  +{
> >>  +       struct usb_conn_info *info = power_supply_get_drvdata(psy);
> >>  +
> >>  +       switch (psp) {
> >>  +       case POWER_SUPPLY_PROP_ONLINE:
> >>  +               val->intval = info->last_role == USB_ROLE_DEVICE;
What will happen if you not change info->last_role here?
I prefer it's only changed by usb_conn_isr(), if it's changed by other
drivers, for example, through power_supply_get_property(), may skip role
switch.

> >>  +               break;
> >>  +       default:
> >>  +               return -EINVAL;
> >>  +       }
> >>  +
> >>  +       return 0;
> >>  +}
> >>  +
> >>   static int usb_conn_probe(struct platform_device *pdev)
> >>   {
> >>          struct device *dev = &pdev->dev;
> >>  +       struct power_supply_desc *desc;
> >>          struct usb_conn_info *info;
> >>  +       struct power_supply_config cfg = {
> >>  +               .of_node = dev->of_node,
> >>  +       };
> >>          int ret = 0;
> >> 
> >>          info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
> >>  @@ -203,6 +234,22 @@ static int usb_conn_probe(struct 
> >> platform_device *pdev)
> >>                  }
> >>          }
> >> 
> >>  +       if (IS_ENABLED(CONFIG_USB_CONN_GPIO_CHARGER)) {
> >>  +               desc = &info->desc;
> >>  +               desc->name = "usb-charger";
> >>  +               desc->properties = usb_charger_properties;
> >>  +               desc->num_properties = 
> >> ARRAY_SIZE(usb_charger_properties);
> >>  +               desc->get_property = usb_charger_get_property;
> >>  +               desc->type = POWER_SUPPLY_TYPE_USB;
> >>  +               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");
> >>  +                       return PTR_ERR(info->charger);
> >>  +               }
> >>  +       }
> >>  +
> >>          platform_set_drvdata(pdev, info);
> >> 
> >>          /* Perform initial detection */
> >>  --
> >>  2.27.0
> >> 
> > 
> > 
> > --
> > With Best Regards,
> > Andy Shevchenko
> 
> 


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

* Re: [RESEND PATCH] usb: common: usb-conn-gpio: Register optional charger
  2020-07-27  5:42     ` Chunfeng Yun
@ 2020-07-27 15:02       ` Paul Cercueil
  2020-07-28  6:04         ` Chunfeng Yun
  0 siblings, 1 reply; 10+ messages in thread
From: Paul Cercueil @ 2020-07-27 15:02 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Andy Shevchenko, Chanwoo Choi, MyungJoo Ham, Greg Kroah-Hartman,
	od, USB, Linux Kernel Mailing List



Le lun. 27 juil. 2020 à 13:42, Chunfeng Yun 
<chunfeng.yun@mediatek.com> a écrit :
> On Sun, 2020-07-26 at 12:27 +0200, Paul Cercueil wrote:
>> 
>>  Le dim. 26 juil. 2020 à 13:14, Andy Shevchenko
>>  <andy.shevchenko@gmail.com> a écrit :
>>  > On Mon, Jun 22, 2020 at 1:51 AM Paul Cercueil 
>> <paul@crapouillou.net>
>>  > wrote:
>>  >>
>>  >>  Register a power supply charger, if the Kconfig option
>>  >>  USB_CONN_GPIO_CHARGER is set, whose online state depends on 
>> whether
>>  >>  the USB role is set to device or not.
>>  >>
>>  >>  This is useful when the USB role is the only way to know if the
>>  >> device
>>  >>  is charging from USB. The API is the standard power supply 
>> charger
>>  >> API,
>>  >>  you get a /sys/class/power_supply/xxx/online node which tells 
>> you
>>  >> the
>>  >>  state of the charger.
>>  >>
>>  >>  The sole purpose of this is to give userspace applications a 
>> way to
>>  >>  know whether or not the charger is plugged.
>>  >
>>  > I'm not sure I understand the purpose of this (third?) way to 
>> detect
>>  > USB charger and notify user space about.
>>  > Why is extcon not good enough?
>> 
>>  We can't have extcon and USB role detection at the same time.
>> 
>>  -Paul
>> 
>>  >>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  >>  ---
>>  >>   drivers/usb/common/Kconfig         | 11 +++++++
>>  >>   drivers/usb/common/usb-conn-gpio.c | 47
>>  >> ++++++++++++++++++++++++++++++
>>  >>   2 files changed, 58 insertions(+)
>>  >>
>>  >>  diff --git a/drivers/usb/common/Kconfig 
>> b/drivers/usb/common/Kconfig
>>  >>  index d611477aae41..5405ae96c68f 100644
>>  >>  --- a/drivers/usb/common/Kconfig
>>  >>  +++ b/drivers/usb/common/Kconfig
>>  >>  @@ -49,3 +49,14 @@ config USB_CONN_GPIO
>>  >>
>>  >>            To compile the driver as a module, choose M here: the
>>  >> module will
>>  >>            be called usb-conn-gpio.ko
>>  >>  +
>>  >>  +if USB_CONN_GPIO
>>  >>  +
>>  >>  +config USB_CONN_GPIO_CHARGER
>>  >>  +       bool "USB charger support"
>>  >>  +       select POWER_SUPPLY
>>  >>  +       help
>>  >>  +         Register a charger with the power supply subsystem. 
>> This
>>  >> will allow
>>  >>  +         userspace to know whether or not the device is 
>> charging
>>  >> from USB.
>>  >>  +
>>  >>  +endif
>>  >>  diff --git a/drivers/usb/common/usb-conn-gpio.c
>>  >> b/drivers/usb/common/usb-conn-gpio.c
>>  >>  index ed204cbb63ea..129d48db280b 100644
>>  >>  --- a/drivers/usb/common/usb-conn-gpio.c
>>  >>  +++ b/drivers/usb/common/usb-conn-gpio.c
>>  >>  @@ -17,6 +17,7 @@
>>  >>   #include <linux/of.h>
>>  >>   #include <linux/pinctrl/consumer.h>
>>  >>   #include <linux/platform_device.h>
>>  >>  +#include <linux/power_supply.h>
>>  >>   #include <linux/regulator/consumer.h>
>>  >>   #include <linux/usb/role.h>
>>  >>
>>  >>  @@ -38,6 +39,9 @@ struct usb_conn_info {
>>  >>          struct gpio_desc *vbus_gpiod;
>>  >>          int id_irq;
>>  >>          int vbus_irq;
>>  >>  +
>>  >>  +       struct power_supply_desc desc;
>>  >>  +       struct power_supply *charger;
>>  >>   };
>>  >>
>>  >>   /**
>>  >>  @@ -98,6 +102,8 @@ static void usb_conn_detect_cable(struct
>>  >> work_struct *work)
>>  >>                  ret = regulator_enable(info->vbus);
>>  >>                  if (ret)
>>  >>                          dev_err(info->dev, "enable vbus 
>> regulator
>>  >> failed\n");
>>  >>  +       } else if (IS_ENABLED(CONFIG_USB_CONN_GPIO_CHARGER)) {
>>  >>  +               power_supply_changed(info->charger);
>>  >>          }
>>  >>
>>  >>          info->last_role = role;
>>  >>  @@ -121,10 +127,35 @@ static irqreturn_t usb_conn_isr(int irq, 
>> void
>>  >> *dev_id)
>>  >>          return IRQ_HANDLED;
>>  >>   }
>>  >>
>>  >>  +static enum power_supply_property usb_charger_properties[] = {
>>  >>  +       POWER_SUPPLY_PROP_ONLINE,
>>  >>  +};
>>  >>  +
>>  >>  +static int usb_charger_get_property(struct power_supply *psy,
>>  >>  +                                   enum power_supply_property 
>> psp,
>>  >>  +                                   union power_supply_propval 
>> *val)
>>  >>  +{
>>  >>  +       struct usb_conn_info *info = 
>> power_supply_get_drvdata(psy);
>>  >>  +
>>  >>  +       switch (psp) {
>>  >>  +       case POWER_SUPPLY_PROP_ONLINE:
>>  >>  +               val->intval = info->last_role == 
>> USB_ROLE_DEVICE;
> What will happen if you not change info->last_role here?
> I prefer it's only changed by usb_conn_isr(), if it's changed by other
> drivers, for example, through power_supply_get_property(), may skip 
> role
> switch.

If you read carefully, info->last_role is not modified here :)

-Paul

> 
>>  >>  +               break;
>>  >>  +       default:
>>  >>  +               return -EINVAL;
>>  >>  +       }
>>  >>  +
>>  >>  +       return 0;
>>  >>  +}
>>  >>  +
>>  >>   static int usb_conn_probe(struct platform_device *pdev)
>>  >>   {
>>  >>          struct device *dev = &pdev->dev;
>>  >>  +       struct power_supply_desc *desc;
>>  >>          struct usb_conn_info *info;
>>  >>  +       struct power_supply_config cfg = {
>>  >>  +               .of_node = dev->of_node,
>>  >>  +       };
>>  >>          int ret = 0;
>>  >>
>>  >>          info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
>>  >>  @@ -203,6 +234,22 @@ static int usb_conn_probe(struct
>>  >> platform_device *pdev)
>>  >>                  }
>>  >>          }
>>  >>
>>  >>  +       if (IS_ENABLED(CONFIG_USB_CONN_GPIO_CHARGER)) {
>>  >>  +               desc = &info->desc;
>>  >>  +               desc->name = "usb-charger";
>>  >>  +               desc->properties = usb_charger_properties;
>>  >>  +               desc->num_properties =
>>  >> ARRAY_SIZE(usb_charger_properties);
>>  >>  +               desc->get_property = usb_charger_get_property;
>>  >>  +               desc->type = POWER_SUPPLY_TYPE_USB;
>>  >>  +               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");
>>  >>  +                       return PTR_ERR(info->charger);
>>  >>  +               }
>>  >>  +       }
>>  >>  +
>>  >>          platform_set_drvdata(pdev, info);
>>  >>
>>  >>          /* Perform initial detection */
>>  >>  --
>>  >>  2.27.0
>>  >>
>>  >
>>  >
>>  > --
>>  > With Best Regards,
>>  > Andy Shevchenko
>> 
>> 
> 



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

* Re: [RESEND PATCH] usb: common: usb-conn-gpio: Register optional charger
  2020-07-27 15:02       ` Paul Cercueil
@ 2020-07-28  6:04         ` Chunfeng Yun
  0 siblings, 0 replies; 10+ messages in thread
From: Chunfeng Yun @ 2020-07-28  6:04 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Andy Shevchenko, Chanwoo Choi, MyungJoo Ham, Greg Kroah-Hartman,
	od, USB, Linux Kernel Mailing List

On Mon, 2020-07-27 at 17:02 +0200, Paul Cercueil wrote:
> 
> Le lun. 27 juil. 2020 à 13:42, Chunfeng Yun 
> <chunfeng.yun@mediatek.com> a écrit :
> > On Sun, 2020-07-26 at 12:27 +0200, Paul Cercueil wrote:
> >> 
> >>  Le dim. 26 juil. 2020 à 13:14, Andy Shevchenko
> >>  <andy.shevchenko@gmail.com> a écrit :
> >>  > On Mon, Jun 22, 2020 at 1:51 AM Paul Cercueil 
> >> <paul@crapouillou.net>
> >>  > wrote:
> >>  >>
> >>  >>  Register a power supply charger, if the Kconfig option
> >>  >>  USB_CONN_GPIO_CHARGER is set, whose online state depends on 
> >> whether
> >>  >>  the USB role is set to device or not.
> >>  >>
> >>  >>  This is useful when the USB role is the only way to know if the
> >>  >> device
> >>  >>  is charging from USB. The API is the standard power supply 
> >> charger
> >>  >> API,
> >>  >>  you get a /sys/class/power_supply/xxx/online node which tells 
> >> you
> >>  >> the
> >>  >>  state of the charger.
> >>  >>
> >>  >>  The sole purpose of this is to give userspace applications a 
> >> way to
> >>  >>  know whether or not the charger is plugged.
[...]
> >>  >>  +       switch (psp) {
> >>  >>  +       case POWER_SUPPLY_PROP_ONLINE:
> >>  >>  +               val->intval = info->last_role == 
> >> USB_ROLE_DEVICE;
> > What will happen if you not change info->last_role here?
> > I prefer it's only changed by usb_conn_isr(), if it's changed by other
> > drivers, for example, through power_supply_get_property(), may skip 
> > role
> > switch.
> 
> If you read carefully, info->last_role is not modified here :)
Sorry, my bad

> 
> -Paul
> 
> > 
> >>  >>  +               break;
> >>  >>  +       default:
> >>  >>  +               return -EINVAL;
> >>  >>  +       }
> >>  >>  +
> >>  >>  +       return 0;
> >>  >>  +}
> >>  >>  +
> >>  >>   static int usb_conn_probe(struct platform_device *pdev)
> >>  >>   {
> >>  >>          struct device *dev = &pdev->dev;
> >>  >>  +       struct power_supply_desc *desc;
> >>  >>          struct usb_conn_info *info;
> >>  >>  +       struct power_supply_config cfg = {
> >>  >>  +               .of_node = dev->of_node,
> >>  >>  +       };
> >>  >>          int ret = 0;
> >>  >>
> >>  >>          info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
> >>  >>  @@ -203,6 +234,22 @@ static int usb_conn_probe(struct
> >>  >> platform_device *pdev)
> >>  >>                  }
> >>  >>          }
> >>  >>
> >>  >>  +       if (IS_ENABLED(CONFIG_USB_CONN_GPIO_CHARGER)) {
> >>  >>  +               desc = &info->desc;
> >>  >>  +               desc->name = "usb-charger";
> >>  >>  +               desc->properties = usb_charger_properties;
> >>  >>  +               desc->num_properties =
> >>  >> ARRAY_SIZE(usb_charger_properties);
> >>  >>  +               desc->get_property = usb_charger_get_property;
> >>  >>  +               desc->type = POWER_SUPPLY_TYPE_USB;
> >>  >>  +               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");
> >>  >>  +                       return PTR_ERR(info->charger);
> >>  >>  +               }
> >>  >>  +       }
> >>  >>  +
> >>  >>          platform_set_drvdata(pdev, info);
> >>  >>
> >>  >>          /* Perform initial detection */
> >>  >>  --
> >>  >>  2.27.0
> >>  >>
> >>  >
> >>  >
> >>  > --
> >>  > With Best Regards,
> >>  > Andy Shevchenko
> >> 
> >> 
> > 
> 
> 


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

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-21 22:48 [RESEND PATCH] usb: common: usb-conn-gpio: Register optional charger Paul Cercueil
2020-07-21 11:41 ` Greg Kroah-Hartman
2020-07-25 17:51   ` Paul Cercueil
2020-07-26  8:39     ` Greg Kroah-Hartman
2020-07-26 10:07       ` Paul Cercueil
2020-07-26 10:14 ` Andy Shevchenko
2020-07-26 10:27   ` Paul Cercueil
2020-07-27  5:42     ` Chunfeng Yun
2020-07-27 15:02       ` Paul Cercueil
2020-07-28  6:04         ` Chunfeng Yun

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git