[PATCH] usb: common: usb-conn-gpio: Register charger

[PATCH] usb: common: usb-conn-gpio: Register charger

[Paul Cercueil]

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

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/usb/common/usb-conn-gpio.c | 45 ++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/usb/common/usb-conn-gpio.c b/drivers/usb/common/usb-conn-gpio.c
index ed204cbb63ea..08a411388d3c 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 {
+		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,20 @@ static int usb_conn_probe(struct platform_device *pdev)
 		}
 	}
 
+	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");
+		return PTR_ERR(info->charger);
+	}
+
 	platform_set_drvdata(pdev, info);
 
 	/* Perform initial detection */
-- 
2.24.1

Re: [PATCH] usb: common: usb-conn-gpio: Register charger

[Greg Kroah-Hartman]

On Tue, Jan 07, 2020 at 01:29:01AM +0100, Paul Cercueil wrote:
> Register a power supply charger, whose online state depends on whether
> the USB role is set to device or not.

That says _what_ you are doing, but I have no idea _why_ you want to do
this.

What is this going to cause to have happen?  What new userspace api is
going to result?  What will a user do with this?

Is this going to always show up, no matter if the role is not even
relevant for a power supply?

You need a lot more text here to make this mergable.

thanks,

greg k-h

Re: [PATCH] usb: common: usb-conn-gpio: Register charger

[Paul Cercueil]

Hi Greg,


Le mer., janv. 8, 2020 at 17:53, Greg Kroah-Hartman 
<gregkh@linuxfoundation.org> a écrit :
> On Tue, Jan 07, 2020 at 01:29:01AM +0100, Paul Cercueil wrote:
>>  Register a power supply charger, whose online state depends on 
>> whether
>>  the USB role is set to device or not.
> 
> That says _what_ you are doing, but I have no idea _why_ you want to 
> do
> this.
> 
> What is this going to cause to have happen?  What new userspace api is
> going to result?  What will a user do with this?

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.

> Is this going to always show up, no matter if the role is not even
> relevant for a power supply?

I guess it'd always show up, yes. In which case would the role not be 
relevant for a power supply? Is gpio-b-connector not always used for 
OTG connectors?

Cheers,
-Paul


> You need a lot more text here to make this mergable.
> 
> thanks,
> 
> greg k-h

Re: [PATCH] usb: common: usb-conn-gpio: Register charger

[Greg Kroah-Hartman]

On Wed, Jan 08, 2020 at 02:26:10PM -0300, Paul Cercueil wrote:
> Hi Greg,
> 
> 
> Le mer., janv. 8, 2020 at 17:53, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> a écrit :
> > On Tue, Jan 07, 2020 at 01:29:01AM +0100, Paul Cercueil wrote:
> > >  Register a power supply charger, whose online state depends on
> > > whether
> > >  the USB role is set to device or not.
> > 
> > That says _what_ you are doing, but I have no idea _why_ you want to do
> > this.
> > 
> > What is this going to cause to have happen?  What new userspace api is
> > going to result?  What will a user do with this?
> 
> 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.

Ok, this is all good info that should go into the changelog text for
when you resend this.

> > Is this going to always show up, no matter if the role is not even
> > relevant for a power supply?
> 
> I guess it'd always show up, yes. In which case would the role not be
> relevant for a power supply? Is gpio-b-connector not always used for OTG
> connectors?

I do not know, I thought OTG was dead :)

thanks,

greg k-h

Re: [PATCH] usb: common: usb-conn-gpio: Register charger

[Chunfeng Yun]

Hi Paul,

On Wed, 2020-01-08 at 14:26 -0300, Paul Cercueil wrote:
> Hi Greg,
> 
> 
> Le mer., janv. 8, 2020 at 17:53, Greg Kroah-Hartman 
> <gregkh@linuxfoundation.org> a écrit :
> > On Tue, Jan 07, 2020 at 01:29:01AM +0100, Paul Cercueil wrote:
> >>  Register a power supply charger, whose online state depends on 
> >> whether
> >>  the USB role is set to device or not.
> > 
> > That says _what_ you are doing, but I have no idea _why_ you want to 
> > do
> > this.
> > 
> > What is this going to cause to have happen?  What new userspace api is
> > going to result?  What will a user do with this?
> 
> This is useful when the USB role is the only way to know if the device 
> is charging from USB.
There is no charger IC on your platform?

> 
> 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.
> 
> > Is this going to always show up, no matter if the role is not even
> > relevant for a power supply?
> 
> I guess it'd always show up, yes. In which case would the role not be 
> relevant for a power supply? 
Do you also need it if the platform don't have battery?


> Is gpio-b-connector not always used for 
> OTG connectors?
It also supports device only mode, usually uses vBus pin to
enable/disable controller

> 
> Cheers,
> -Paul
> 
> 
> > You need a lot more text here to make this mergable.
> > 
> > thanks,
> > 
> > greg k-h
> 
> 

Re: [PATCH] usb: common: usb-conn-gpio: Register charger

[Paul Cercueil]

Hi Chunfeng,

Le jeu., janv. 9, 2020 at 10:27, Chunfeng Yun 
<chunfeng.yun@mediatek.com> a écrit :
> Hi Paul,
> 
> On Wed, 2020-01-08 at 14:26 -0300, Paul Cercueil wrote:
>>  Hi Greg,
>> 
>> 
>>  Le mer., janv. 8, 2020 at 17:53, Greg Kroah-Hartman
>>  <gregkh@linuxfoundation.org> a écrit :
>>  > On Tue, Jan 07, 2020 at 01:29:01AM +0100, Paul Cercueil wrote:
>>  >>  Register a power supply charger, whose online state depends on
>>  >> whether
>>  >>  the USB role is set to device or not.
>>  >
>>  > That says _what_ you are doing, but I have no idea _why_ you want 
>> to
>>  > do
>>  > this.
>>  >
>>  > What is this going to cause to have happen?  What new userspace 
>> api is
>>  > going to result?  What will a user do with this?
>> 
>>  This is useful when the USB role is the only way to know if the 
>> device
>>  is charging from USB.
> There is no charger IC on your platform?

Correct.

> 
>> 
>>  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.
>> 
>>  > Is this going to always show up, no matter if the role is not even
>>  > relevant for a power supply?
>> 
>>  I guess it'd always show up, yes. In which case would the role not 
>> be
>>  relevant for a power supply?
> Do you also need it if the platform don't have battery?

I guess not. Would you prefer it to be disabled by default? If so, what 
would be the way to have it enabled? A flag in devicetree?

> 
>>  Is gpio-b-connector not always used for
>>  OTG connectors?
> It also supports device only mode, usually uses vBus pin to
> enable/disable controller

Yes, that would work too. The problem would be a host-only 
gpio-b-connector.

-Paul