diff mbox series

[v2,1/3] dt-bindings: power/supply: Document generic USB charger

Message ID 20191211155032.167032-1-paul@crapouillou.net
State Changes Requested, archived
Headers show
Series [v2,1/3] dt-bindings: power/supply: Document generic USB charger | expand

Checks

Context Check Description
robh/checkpatch success
robh/dt-meta-schema success

Commit Message

Paul Cercueil Dec. 11, 2019, 3:50 p.m. UTC 
Add documentation about the devicetree bindings for the generic USB
charger.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    v2: Convert to YAML

 .../bindings/power/supply/usb-charger.yaml    | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/usb-charger.yaml

Comments

Peter Chen Dec. 12, 2019, 9:18 a.m. UTC | #1 
On 19-12-11 16:50:32, Paul Cercueil wrote:
> This simple charger driver uses the USB role switch framework to detect
> the presence of a charger. The USB charger will report as online when
> the USB role is set to device, and offline otherwise.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> 
> Notes:
>     v2: Instead of detecting charger state from USB PHY, we detect it from the
>         USB role in use.
> 
>  drivers/power/supply/Kconfig               |   8 ++
>  drivers/power/supply/Makefile              |   1 +
>  drivers/power/supply/generic-usb-charger.c | 124 +++++++++++++++++++++
>  3 files changed, 133 insertions(+)
>  create mode 100644 drivers/power/supply/generic-usb-charger.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 27164a1d3c7c..c4221bcabee4 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -51,6 +51,14 @@ config GENERIC_ADC_BATTERY
>  	  Say Y here to enable support for the generic battery driver
>  	  which uses IIO framework to read adc.
>  
> +config GENERIC_USB_CHARGER
> +	tristate "Generic USB charger"
> +	depends on USB_SUPPORT
> +	select USB_ROLE_SWITCH
> +	help
> +	  Say Y here to enable a generic USB charger driver which uses
> +	  the USB role switch framework to detect the presence of the charger.
> +
>  config MAX8925_POWER
>  	tristate "MAX8925 battery charger support"
>  	depends on MFD_MAX8925
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 6c7da920ea83..03f9b553bdfc 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -8,6 +8,7 @@ power_supply-$(CONFIG_LEDS_TRIGGERS)	+= power_supply_leds.o
>  obj-$(CONFIG_POWER_SUPPLY)	+= power_supply.o
>  obj-$(CONFIG_POWER_SUPPLY_HWMON) += power_supply_hwmon.o
>  obj-$(CONFIG_GENERIC_ADC_BATTERY)	+= generic-adc-battery.o
> +obj-$(CONFIG_GENERIC_USB_CHARGER)	+= generic-usb-charger.o
>  
>  obj-$(CONFIG_PDA_POWER)		+= pda_power.o
>  obj-$(CONFIG_APM_POWER)		+= apm_power.o
> diff --git a/drivers/power/supply/generic-usb-charger.c b/drivers/power/supply/generic-usb-charger.c
> new file mode 100644
> index 000000000000..0493fafbd4c0
> --- /dev/null
> +++ b/drivers/power/supply/generic-usb-charger.c
> @@ -0,0 +1,124 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Simple USB charger driver
> + * Copyright (c) 2019 Paul Cercueil <paul@crapouillou.net>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/usb/role.h>
> +
> +struct usb_charger {
> +	struct notifier_block nb;
> +	struct usb_role_switch *role;
> +	struct power_supply_desc desc;
> +	struct power_supply *charger;
> +};
> +
> +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_charger *charger = power_supply_get_drvdata(psy);
> +	enum usb_role role;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_ONLINE:
> +		role = usb_role_switch_get_role(charger->role);
> +		val->intval = role == USB_ROLE_DEVICE;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int usb_charger_event(struct notifier_block *nb,
> +			     unsigned long event, void *d)
> +{
> +	struct usb_charger *charger = container_of(nb, struct usb_charger, nb);
> +
> +	power_supply_changed(charger->charger);
> +
> +	return 0;
> +}
> +
> +static void usb_charger_unregister(void *data)
> +{
> +	struct usb_charger *charger = data;
> +
> +	usb_role_switch_unregister_notifier(charger->role, &charger->nb);
> +}
> +
> +static int usb_charger_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct power_supply_desc *desc;
> +	struct usb_charger *charger;
> +	struct power_supply_config cfg = {
> +		.of_node = dev->of_node,
> +	};
> +	int err;
> +
> +	charger = devm_kzalloc(dev, sizeof(*charger), GFP_KERNEL);
> +	if (!charger)
> +		return -ENOMEM;
> +
> +	cfg.drv_data = charger;
> +	charger->nb.notifier_call = usb_charger_event;
> +
> +	charger->role = usb_role_switch_get(dev);
> +	if (IS_ERR(charger->role)) {
> +		if (PTR_ERR(charger->role) != -EPROBE_DEFER)
> +			dev_err(dev, "Unable to get USB role");
> +		return PTR_ERR(charger->role);
> +	}
> +
> +	desc = &charger->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;

What's your further plan for this generic USB charger?
To support BC1.2, we need to know charger type, and how
we could get it?

Peter

> +
> +	charger->charger = devm_power_supply_register(dev, desc, &cfg);
> +	if (IS_ERR(charger->charger)) {
> +		dev_err(dev, "Unable to register charger");
> +		return PTR_ERR(charger->charger);
> +	}
> +
> +	err = usb_role_switch_register_notifier(charger->role, &charger->nb);
> +	if (err) {
> +		dev_err(dev, "Unable to register USB role switch notifier");
> +		return err;
> +	}
> +
> +	return devm_add_action_or_reset(dev, usb_charger_unregister, charger);
> +}
> +
> +static const struct of_device_id usb_charger_of_match[] = {
> +	{ .compatible = "usb-charger" },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, usb_charger_of_match);
> +
> +static struct platform_driver usb_charger_driver = {
> +	.driver = {
> +		.name = "usb-charger",
> +		.of_match_table = of_match_ptr(usb_charger_of_match),
> +	},
> +	.probe = usb_charger_probe,
> +};
> +module_platform_driver(usb_charger_driver);
> +
> +MODULE_DESCRIPTION("Simple USB charger driver");
> +MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.24.0
>
Paul Cercueil Dec. 13, 2019, 8:49 p.m. UTC | #2 
Hi Peter,


Le jeu., déc. 12, 2019 at 09:18, Peter Chen <peter.chen@nxp.com> a 
écrit :
> On 19-12-11 16:50:32, Paul Cercueil wrote:
>>  This simple charger driver uses the USB role switch framework to 
>> detect
>>  the presence of a charger. The USB charger will report as online 
>> when
>>  the USB role is set to device, and offline otherwise.
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>> 
>>  Notes:
>>      v2: Instead of detecting charger state from USB PHY, we detect 
>> it from the
>>          USB role in use.
>> 
>>   drivers/power/supply/Kconfig               |   8 ++
>>   drivers/power/supply/Makefile              |   1 +
>>   drivers/power/supply/generic-usb-charger.c | 124 
>> +++++++++++++++++++++
>>   3 files changed, 133 insertions(+)
>>   create mode 100644 drivers/power/supply/generic-usb-charger.c
>> 
>>  diff --git a/drivers/power/supply/Kconfig 
>> b/drivers/power/supply/Kconfig
>>  index 27164a1d3c7c..c4221bcabee4 100644
>>  --- a/drivers/power/supply/Kconfig
>>  +++ b/drivers/power/supply/Kconfig
>>  @@ -51,6 +51,14 @@ config GENERIC_ADC_BATTERY
>>   	  Say Y here to enable support for the generic battery driver
>>   	  which uses IIO framework to read adc.
>> 
>>  +config GENERIC_USB_CHARGER
>>  +	tristate "Generic USB charger"
>>  +	depends on USB_SUPPORT
>>  +	select USB_ROLE_SWITCH
>>  +	help
>>  +	  Say Y here to enable a generic USB charger driver which uses
>>  +	  the USB role switch framework to detect the presence of the 
>> charger.
>>  +
>>   config MAX8925_POWER
>>   	tristate "MAX8925 battery charger support"
>>   	depends on MFD_MAX8925
>>  diff --git a/drivers/power/supply/Makefile 
>> b/drivers/power/supply/Makefile
>>  index 6c7da920ea83..03f9b553bdfc 100644
>>  --- a/drivers/power/supply/Makefile
>>  +++ b/drivers/power/supply/Makefile
>>  @@ -8,6 +8,7 @@ power_supply-$(CONFIG_LEDS_TRIGGERS)	+= 
>> power_supply_leds.o
>>   obj-$(CONFIG_POWER_SUPPLY)	+= power_supply.o
>>   obj-$(CONFIG_POWER_SUPPLY_HWMON) += power_supply_hwmon.o
>>   obj-$(CONFIG_GENERIC_ADC_BATTERY)	+= generic-adc-battery.o
>>  +obj-$(CONFIG_GENERIC_USB_CHARGER)	+= generic-usb-charger.o
>> 
>>   obj-$(CONFIG_PDA_POWER)		+= pda_power.o
>>   obj-$(CONFIG_APM_POWER)		+= apm_power.o
>>  diff --git a/drivers/power/supply/generic-usb-charger.c 
>> b/drivers/power/supply/generic-usb-charger.c
>>  new file mode 100644
>>  index 000000000000..0493fafbd4c0
>>  --- /dev/null
>>  +++ b/drivers/power/supply/generic-usb-charger.c
>>  @@ -0,0 +1,124 @@
>>  +// SPDX-License-Identifier: GPL-2.0
>>  +/*
>>  + * Simple USB charger driver
>>  + * Copyright (c) 2019 Paul Cercueil <paul@crapouillou.net>
>>  + */
>>  +
>>  +#include <linux/device.h>
>>  +#include <linux/module.h>
>>  +#include <linux/of.h>
>>  +#include <linux/platform_device.h>
>>  +#include <linux/power_supply.h>
>>  +#include <linux/usb/role.h>
>>  +
>>  +struct usb_charger {
>>  +	struct notifier_block nb;
>>  +	struct usb_role_switch *role;
>>  +	struct power_supply_desc desc;
>>  +	struct power_supply *charger;
>>  +};
>>  +
>>  +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_charger *charger = power_supply_get_drvdata(psy);
>>  +	enum usb_role role;
>>  +
>>  +	switch (psp) {
>>  +	case POWER_SUPPLY_PROP_ONLINE:
>>  +		role = usb_role_switch_get_role(charger->role);
>>  +		val->intval = role == USB_ROLE_DEVICE;
>>  +		break;
>>  +	default:
>>  +		return -EINVAL;
>>  +	}
>>  +
>>  +	return 0;
>>  +}
>>  +
>>  +static int usb_charger_event(struct notifier_block *nb,
>>  +			     unsigned long event, void *d)
>>  +{
>>  +	struct usb_charger *charger = container_of(nb, struct 
>> usb_charger, nb);
>>  +
>>  +	power_supply_changed(charger->charger);
>>  +
>>  +	return 0;
>>  +}
>>  +
>>  +static void usb_charger_unregister(void *data)
>>  +{
>>  +	struct usb_charger *charger = data;
>>  +
>>  +	usb_role_switch_unregister_notifier(charger->role, &charger->nb);
>>  +}
>>  +
>>  +static int usb_charger_probe(struct platform_device *pdev)
>>  +{
>>  +	struct device *dev = &pdev->dev;
>>  +	struct power_supply_desc *desc;
>>  +	struct usb_charger *charger;
>>  +	struct power_supply_config cfg = {
>>  +		.of_node = dev->of_node,
>>  +	};
>>  +	int err;
>>  +
>>  +	charger = devm_kzalloc(dev, sizeof(*charger), GFP_KERNEL);
>>  +	if (!charger)
>>  +		return -ENOMEM;
>>  +
>>  +	cfg.drv_data = charger;
>>  +	charger->nb.notifier_call = usb_charger_event;
>>  +
>>  +	charger->role = usb_role_switch_get(dev);
>>  +	if (IS_ERR(charger->role)) {
>>  +		if (PTR_ERR(charger->role) != -EPROBE_DEFER)
>>  +			dev_err(dev, "Unable to get USB role");
>>  +		return PTR_ERR(charger->role);
>>  +	}
>>  +
>>  +	desc = &charger->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;
> 
> What's your further plan for this generic USB charger?
> To support BC1.2, we need to know charger type, and how
> we could get it?
> 
> Peter

Well I don't really know. The USB role framework does not give any info 
about what's plugged.

-Paul


> 
>>  +
>>  +	charger->charger = devm_power_supply_register(dev, desc, &cfg);
>>  +	if (IS_ERR(charger->charger)) {
>>  +		dev_err(dev, "Unable to register charger");
>>  +		return PTR_ERR(charger->charger);
>>  +	}
>>  +
>>  +	err = usb_role_switch_register_notifier(charger->role, 
>> &charger->nb);
>>  +	if (err) {
>>  +		dev_err(dev, "Unable to register USB role switch notifier");
>>  +		return err;
>>  +	}
>>  +
>>  +	return devm_add_action_or_reset(dev, usb_charger_unregister, 
>> charger);
>>  +}
>>  +
>>  +static const struct of_device_id usb_charger_of_match[] = {
>>  +	{ .compatible = "usb-charger" },
>>  +	{ /* sentinel */ },
>>  +};
>>  +MODULE_DEVICE_TABLE(of, usb_charger_of_match);
>>  +
>>  +static struct platform_driver usb_charger_driver = {
>>  +	.driver = {
>>  +		.name = "usb-charger",
>>  +		.of_match_table = of_match_ptr(usb_charger_of_match),
>>  +	},
>>  +	.probe = usb_charger_probe,
>>  +};
>>  +module_platform_driver(usb_charger_driver);
>>  +
>>  +MODULE_DESCRIPTION("Simple USB charger driver");
>>  +MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
>>  +MODULE_LICENSE("GPL");
>>  --
>>  2.24.0
>> 
> 
> --
> 
> Thanks,
> Peter Chen
Peter Chen Dec. 16, 2019, 1:24 a.m. UTC | #3 
> >>  +
> >>  +	desc = &charger->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;
> >
> > What's your further plan for this generic USB charger?
> > To support BC1.2, we need to know charger type, and how we could get
> > it?
> >
> > Peter
> 
> Well I don't really know. The USB role framework does not give any info about
> what's plugged.
> 

What's the use case for this patch set? How it be used?

Thanks,
Peter

> -Paul
> 
> 
> >
> >>  +
> >>  +	charger->charger = devm_power_supply_register(dev, desc, &cfg);
> >>  +	if (IS_ERR(charger->charger)) {
> >>  +		dev_err(dev, "Unable to register charger");
> >>  +		return PTR_ERR(charger->charger);
> >>  +	}
> >>  +
> >>  +	err = usb_role_switch_register_notifier(charger->role,
> >> &charger->nb);
> >>  +	if (err) {
> >>  +		dev_err(dev, "Unable to register USB role switch notifier");
> >>  +		return err;
> >>  +	}
> >>  +
> >>  +	return devm_add_action_or_reset(dev, usb_charger_unregister,
> >> charger);
> >>  +}
> >>  +
> >>  +static const struct of_device_id usb_charger_of_match[] = {
> >>  +	{ .compatible = "usb-charger" },
> >>  +	{ /* sentinel */ },
> >>  +};
> >>  +MODULE_DEVICE_TABLE(of, usb_charger_of_match);  +  +static struct
> >> platform_driver usb_charger_driver = {
> >>  +	.driver = {
> >>  +		.name = "usb-charger",
> >>  +		.of_match_table = of_match_ptr(usb_charger_of_match),
> >>  +	},
> >>  +	.probe = usb_charger_probe,
> >>  +};
> >>  +module_platform_driver(usb_charger_driver);
> >>  +
> >>  +MODULE_DESCRIPTION("Simple USB charger driver");
> >> +MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
> >> +MODULE_LICENSE("GPL");
> >>  --
> >>  2.24.0
> >>
> >
> > --
> >
> > Thanks,
> > Peter Chen
>
Paul Cercueil Dec. 16, 2019, 10:52 a.m. UTC | #4 
Hi Peter,


Le lun., déc. 16, 2019 at 01:24, Peter Chen <peter.chen@nxp.com> a 
écrit :
> 
>>  >>  +
>>  >>  +	desc = &charger->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;
>>  >
>>  > What's your further plan for this generic USB charger?
>>  > To support BC1.2, we need to know charger type, and how we could 
>> get
>>  > it?
>>  >
>>  > Peter
>> 
>>  Well I don't really know. The USB role framework does not give any 
>> info about
>>  what's plugged.
>> 
> 
> What's the use case for this patch set? How it be used?

My devicetree:

usb_otg: usb@13440000 {
	compatible = "ingenic,jz4770-musb", "simple-mfd";
	reg = <0x13440000 0x10000>;
	[...]

	usb-role-switch;

	connector {
		compatible = "gpio-usb-b-connector", "usb-b-connector";
		label = "mini-USB";
		type = "mini";

		id-gpios = <&gpf 18 GPIO_ACTIVE_HIGH>;
		vbus-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>;
		[...]
	};

	usb_charger: usb-charger {
		compatible = "usb-charger";
	};
};

The new gpio-usb-connector driver uses the ID/VBUS GPIOs to detect in 
which state (device, host, unconnected) a OTG connector is. However, 
that means I cannot use the standard gpio-charger driver to detect the 
presence of a charger based on the state of the VBUS gpio, since it's 
already requested here. So the point of this patchset is to provide an 
alternative to gpio-charger that works with OTG controllers compatible 
with 'usb-role-switch'.

Cheers,
-Paul


>>  >
>>  >>  +
>>  >>  +	charger->charger = devm_power_supply_register(dev, desc, 
>> &cfg);
>>  >>  +	if (IS_ERR(charger->charger)) {
>>  >>  +		dev_err(dev, "Unable to register charger");
>>  >>  +		return PTR_ERR(charger->charger);
>>  >>  +	}
>>  >>  +
>>  >>  +	err = usb_role_switch_register_notifier(charger->role,
>>  >> &charger->nb);
>>  >>  +	if (err) {
>>  >>  +		dev_err(dev, "Unable to register USB role switch notifier");
>>  >>  +		return err;
>>  >>  +	}
>>  >>  +
>>  >>  +	return devm_add_action_or_reset(dev, usb_charger_unregister,
>>  >> charger);
>>  >>  +}
>>  >>  +
>>  >>  +static const struct of_device_id usb_charger_of_match[] = {
>>  >>  +	{ .compatible = "usb-charger" },
>>  >>  +	{ /* sentinel */ },
>>  >>  +};
>>  >>  +MODULE_DEVICE_TABLE(of, usb_charger_of_match);  +  +static 
>> struct
>>  >> platform_driver usb_charger_driver = {
>>  >>  +	.driver = {
>>  >>  +		.name = "usb-charger",
>>  >>  +		.of_match_table = of_match_ptr(usb_charger_of_match),
>>  >>  +	},
>>  >>  +	.probe = usb_charger_probe,
>>  >>  +};
>>  >>  +module_platform_driver(usb_charger_driver);
>>  >>  +
>>  >>  +MODULE_DESCRIPTION("Simple USB charger driver");
>>  >> +MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
>>  >> +MODULE_LICENSE("GPL");
>>  >>  --
>>  >>  2.24.0
>>  >>
>>  >
>>  > --
>>  >
>>  > Thanks,
>>  > Peter Chen
>> 
>
Peter Chen Dec. 17, 2019, 1:32 a.m. UTC | #5 
> Le lun., déc. 16, 2019 at 01:24, Peter Chen <peter.chen@nxp.com> a écrit :
> >
> >>  >>  +
> >>  >>  +	desc = &charger->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;
> >>  >
> >>  > What's your further plan for this generic USB charger?
> >>  > To support BC1.2, we need to know charger type, and how we could
> >> get  > it?
> >>  >
> >>  > Peter
> >>
> >>  Well I don't really know. The USB role framework does not give any
> >> info about  what's plugged.
> >>
> >
> > What's the use case for this patch set? How it be used?
> 
> My devicetree:
> 
> usb_otg: usb@13440000 {
> 	compatible = "ingenic,jz4770-musb", "simple-mfd";
> 	reg = <0x13440000 0x10000>;
> 	[...]
> 
> 	usb-role-switch;
> 
> 	connector {
> 		compatible = "gpio-usb-b-connector", "usb-b-connector";
> 		label = "mini-USB";
> 		type = "mini";
> 
> 		id-gpios = <&gpf 18 GPIO_ACTIVE_HIGH>;
> 		vbus-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>;
> 		[...]
> 	};
> 
> 	usb_charger: usb-charger {
> 		compatible = "usb-charger";
> 	};
> };
> 
> The new gpio-usb-connector driver uses the ID/VBUS GPIOs to detect in
> which state (device, host, unconnected) a OTG connector is. However,
> that means I cannot use the standard gpio-charger driver to detect the
> presence of a charger based on the state of the VBUS gpio, since it's
> already requested here. So the point of this patchset is to provide an
> alternative to gpio-charger that works with OTG controllers compatible
> with 'usb-role-switch'.
> 

Thanks for explaining it.

What's the user for this USB charger,  PMIC or what else? How the user uses
this USB charger interface?

Peter

> 
> 
> >>  >
> >>  >>  +
> >>  >>  +	charger->charger = devm_power_supply_register(dev, desc,
> >> &cfg);
> >>  >>  +	if (IS_ERR(charger->charger)) {
> >>  >>  +		dev_err(dev, "Unable to register charger");
> >>  >>  +		return PTR_ERR(charger->charger);
> >>  >>  +	}
> >>  >>  +
> >>  >>  +	err = usb_role_switch_register_notifier(charger->role,
> >>  >> &charger->nb);
> >>  >>  +	if (err) {
> >>  >>  +		dev_err(dev, "Unable to register USB role switch notifier");
> >>  >>  +		return err;
> >>  >>  +	}
> >>  >>  +
> >>  >>  +	return devm_add_action_or_reset(dev, usb_charger_unregister,
> >>  >> charger);
> >>  >>  +}
> >>  >>  +
> >>  >>  +static const struct of_device_id usb_charger_of_match[] = {
> >>  >>  +	{ .compatible = "usb-charger" },
> >>  >>  +	{ /* sentinel */ },
> >>  >>  +};
> >>  >>  +MODULE_DEVICE_TABLE(of, usb_charger_of_match);  +  +static
> >> struct
> >>  >> platform_driver usb_charger_driver = {
> >>  >>  +	.driver = {
> >>  >>  +		.name = "usb-charger",
> >>  >>  +		.of_match_table = of_match_ptr(usb_charger_of_match),
> >>  >>  +	},
> >>  >>  +	.probe = usb_charger_probe,
> >>  >>  +};
> >>  >>  +module_platform_driver(usb_charger_driver);
> >>  >>  +
> >>  >>  +MODULE_DESCRIPTION("Simple USB charger driver");
> >>  >> +MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
> >>  >> +MODULE_LICENSE("GPL");
> >>  >>  --
> >>  >>  2.24.0
> >>  >>
> >>  >
> >>  > --
> >>  >
> >>  > Thanks,
> >>  > Peter Chen
> >>
> >
>
Paul Cercueil Dec. 17, 2019, 9:24 p.m. UTC | #6 
Hi Peter,

Le mar., déc. 17, 2019 at 01:32, Peter Chen <peter.chen@nxp.com> a 
écrit :
> 
>>  Le lun., déc. 16, 2019 at 01:24, Peter Chen <peter.chen@nxp.com> a 
>> écrit :
>>  >
>>  >>  >>  +
>>  >>  >>  +	desc = &charger->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;
>>  >>  >
>>  >>  > What's your further plan for this generic USB charger?
>>  >>  > To support BC1.2, we need to know charger type, and how we 
>> could
>>  >> get  > it?
>>  >>  >
>>  >>  > Peter
>>  >>
>>  >>  Well I don't really know. The USB role framework does not give 
>> any
>>  >> info about  what's plugged.
>>  >>
>>  >
>>  > What's the use case for this patch set? How it be used?
>> 
>>  My devicetree:
>> 
>>  usb_otg: usb@13440000 {
>>  	compatible = "ingenic,jz4770-musb", "simple-mfd";
>>  	reg = <0x13440000 0x10000>;
>>  	[...]
>> 
>>  	usb-role-switch;
>> 
>>  	connector {
>>  		compatible = "gpio-usb-b-connector", "usb-b-connector";
>>  		label = "mini-USB";
>>  		type = "mini";
>> 
>>  		id-gpios = <&gpf 18 GPIO_ACTIVE_HIGH>;
>>  		vbus-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>;
>>  		[...]
>>  	};
>> 
>>  	usb_charger: usb-charger {
>>  		compatible = "usb-charger";
>>  	};
>>  };
>> 
>>  The new gpio-usb-connector driver uses the ID/VBUS GPIOs to detect 
>> in
>>  which state (device, host, unconnected) a OTG connector is. However,
>>  that means I cannot use the standard gpio-charger driver to detect 
>> the
>>  presence of a charger based on the state of the VBUS gpio, since 
>> it's
>>  already requested here. So the point of this patchset is to provide 
>> an
>>  alternative to gpio-charger that works with OTG controllers 
>> compatible
>>  with 'usb-role-switch'.
>> 
> 
> Thanks for explaining it.
> 
> What's the user for this USB charger,  PMIC or what else? How the 
> user uses
> this USB charger interface?

It's exported as a standard charger, so it can be passed to client 
drivers through devicetree, and its online status can be retrieved from 
sysfs.

-Paul

> 
>> 
>> 
>>  >>  >
>>  >>  >>  +
>>  >>  >>  +	charger->charger = devm_power_supply_register(dev, desc,
>>  >> &cfg);
>>  >>  >>  +	if (IS_ERR(charger->charger)) {
>>  >>  >>  +		dev_err(dev, "Unable to register charger");
>>  >>  >>  +		return PTR_ERR(charger->charger);
>>  >>  >>  +	}
>>  >>  >>  +
>>  >>  >>  +	err = usb_role_switch_register_notifier(charger->role,
>>  >>  >> &charger->nb);
>>  >>  >>  +	if (err) {
>>  >>  >>  +		dev_err(dev, "Unable to register USB role switch 
>> notifier");
>>  >>  >>  +		return err;
>>  >>  >>  +	}
>>  >>  >>  +
>>  >>  >>  +	return devm_add_action_or_reset(dev, 
>> usb_charger_unregister,
>>  >>  >> charger);
>>  >>  >>  +}
>>  >>  >>  +
>>  >>  >>  +static const struct of_device_id usb_charger_of_match[] = {
>>  >>  >>  +	{ .compatible = "usb-charger" },
>>  >>  >>  +	{ /* sentinel */ },
>>  >>  >>  +};
>>  >>  >>  +MODULE_DEVICE_TABLE(of, usb_charger_of_match);  +  +static
>>  >> struct
>>  >>  >> platform_driver usb_charger_driver = {
>>  >>  >>  +	.driver = {
>>  >>  >>  +		.name = "usb-charger",
>>  >>  >>  +		.of_match_table = of_match_ptr(usb_charger_of_match),
>>  >>  >>  +	},
>>  >>  >>  +	.probe = usb_charger_probe,
>>  >>  >>  +};
>>  >>  >>  +module_platform_driver(usb_charger_driver);
>>  >>  >>  +
>>  >>  >>  +MODULE_DESCRIPTION("Simple USB charger driver");
>>  >>  >> +MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
>>  >>  >> +MODULE_LICENSE("GPL");
>>  >>  >>  --
>>  >>  >>  2.24.0
>>  >>  >>
>>  >>  >
>>  >>  > --
>>  >>  >
>>  >>  > Thanks,
>>  >>  > Peter Chen
>>  >>
>>  >
>> 
>
Peter Chen Dec. 18, 2019, 2:46 a.m. UTC | #7 
> >>  >
> >>  >>  >>  +
> >>  >>  >>  +	desc = &charger->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;
> >>  >>  >
> >>  >>  > What's your further plan for this generic USB charger?
> >>  >>  > To support BC1.2, we need to know charger type, and how we
> >> could  >> get  > it?
> >>  >>  >
> >>  >>  > Peter
> >>  >>
> >>  >>  Well I don't really know. The USB role framework does not give
> >> any  >> info about  what's plugged.
> >>  >>
> >>  >
> >>  > What's the use case for this patch set? How it be used?
> >>
> >>  My devicetree:
> >>
> >>  usb_otg: usb@13440000 {
> >>  	compatible = "ingenic,jz4770-musb", "simple-mfd";
> >>  	reg = <0x13440000 0x10000>;
> >>  	[...]
> >>
> >>  	usb-role-switch;
> >>
> >>  	connector {
> >>  		compatible = "gpio-usb-b-connector", "usb-b-connector";
> >>  		label = "mini-USB";
> >>  		type = "mini";
> >>
> >>  		id-gpios = <&gpf 18 GPIO_ACTIVE_HIGH>;
> >>  		vbus-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>;
> >>  		[...]
> >>  	};
> >>
> >>  	usb_charger: usb-charger {
> >>  		compatible = "usb-charger";
> >>  	};
> >>  };
> >>
> >>  The new gpio-usb-connector driver uses the ID/VBUS GPIOs to detect
> >> in  which state (device, host, unconnected) a OTG connector is.
> >> However,  that means I cannot use the standard gpio-charger driver to
> >> detect the  presence of a charger based on the state of the VBUS
> >> gpio, since it's  already requested here. So the point of this
> >> patchset is to provide an  alternative to gpio-charger that works
> >> with OTG controllers compatible  with 'usb-role-switch'.
> >>
> >
> > Thanks for explaining it.
> >
> > What's the user for this USB charger,  PMIC or what else? How the user
> > uses this USB charger interface?
> 
> It's exported as a standard charger, so it can be passed to client drivers through
> devicetree, and its online status can be retrieved from sysfs.
> 
 
Hi Paul,

If you would like to get role from usb-role-switch, the udc driver may probably have already worked.
There is a 'state' entry under the udc device to indicate USB Ch9 state. Try to see if it could
satisfy your requirement.

Peter
Paul Cercueil Dec. 19, 2019, 11:35 a.m. UTC | #8 
Hi Peter,


Le mer., déc. 18, 2019 at 02:46, Peter Chen <peter.chen@nxp.com> a 
écrit :
> 
>>  >>  >
>>  >>  >>  >>  +
>>  >>  >>  >>  +	desc = &charger->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;
>>  >>  >>  >
>>  >>  >>  > What's your further plan for this generic USB charger?
>>  >>  >>  > To support BC1.2, we need to know charger type, and how we
>>  >> could  >> get  > it?
>>  >>  >>  >
>>  >>  >>  > Peter
>>  >>  >>
>>  >>  >>  Well I don't really know. The USB role framework does not 
>> give
>>  >> any  >> info about  what's plugged.
>>  >>  >>
>>  >>  >
>>  >>  > What's the use case for this patch set? How it be used?
>>  >>
>>  >>  My devicetree:
>>  >>
>>  >>  usb_otg: usb@13440000 {
>>  >>  	compatible = "ingenic,jz4770-musb", "simple-mfd";
>>  >>  	reg = <0x13440000 0x10000>;
>>  >>  	[...]
>>  >>
>>  >>  	usb-role-switch;
>>  >>
>>  >>  	connector {
>>  >>  		compatible = "gpio-usb-b-connector", "usb-b-connector";
>>  >>  		label = "mini-USB";
>>  >>  		type = "mini";
>>  >>
>>  >>  		id-gpios = <&gpf 18 GPIO_ACTIVE_HIGH>;
>>  >>  		vbus-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>;
>>  >>  		[...]
>>  >>  	};
>>  >>
>>  >>  	usb_charger: usb-charger {
>>  >>  		compatible = "usb-charger";
>>  >>  	};
>>  >>  };
>>  >>
>>  >>  The new gpio-usb-connector driver uses the ID/VBUS GPIOs to 
>> detect
>>  >> in  which state (device, host, unconnected) a OTG connector is.
>>  >> However,  that means I cannot use the standard gpio-charger 
>> driver to
>>  >> detect the  presence of a charger based on the state of the VBUS
>>  >> gpio, since it's  already requested here. So the point of this
>>  >> patchset is to provide an  alternative to gpio-charger that works
>>  >> with OTG controllers compatible  with 'usb-role-switch'.
>>  >>
>>  >
>>  > Thanks for explaining it.
>>  >
>>  > What's the user for this USB charger,  PMIC or what else? How the 
>> user
>>  > uses this USB charger interface?
>> 
>>  It's exported as a standard charger, so it can be passed to client 
>> drivers through
>>  devicetree, and its online status can be retrieved from sysfs.
>> 
> 
> Hi Paul,
> 
> If you would like to get role from usb-role-switch, the udc driver 
> may probably have already worked.
> There is a 'state' entry under the udc device to indicate USB Ch9 
> state. Try to see if it could
> satisfy your requirement.

This is not the proper way to retrieve charger status.
Linux supports chargers through the power supply subsystem, that's 
where it should be exported.

Cheers,
-Paul
Rob Herring (Arm) Dec. 19, 2019, 9:38 p.m. UTC | #9 
On Mon, Dec 16, 2019 at 11:52:05AM +0100, Paul Cercueil wrote:
> Hi Peter,
> 
> 
> Le lun., déc. 16, 2019 at 01:24, Peter Chen <peter.chen@nxp.com> a écrit :
> > 
> > >  >>  +
> > >  >>  +	desc = &charger->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;
> > >  >
> > >  > What's your further plan for this generic USB charger?
> > >  > To support BC1.2, we need to know charger type, and how we could
> > > get
> > >  > it?
> > >  >
> > >  > Peter
> > > 
> > >  Well I don't really know. The USB role framework does not give any
> > > info about
> > >  what's plugged.
> > > 
> > 
> > What's the use case for this patch set? How it be used?
> 
> My devicetree:
> 
> usb_otg: usb@13440000 {
> 	compatible = "ingenic,jz4770-musb", "simple-mfd";
> 	reg = <0x13440000 0x10000>;
> 	[...]
> 
> 	usb-role-switch;
> 
> 	connector {
> 		compatible = "gpio-usb-b-connector", "usb-b-connector";
> 		label = "mini-USB";
> 		type = "mini";
> 
> 		id-gpios = <&gpf 18 GPIO_ACTIVE_HIGH>;
> 		vbus-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>;
> 		[...]
> 	};
> 
> 	usb_charger: usb-charger {
> 		compatible = "usb-charger";

What h/w device is this?

> 	};
> };
> 
> The new gpio-usb-connector driver uses the ID/VBUS GPIOs to detect in which
> state (device, host, unconnected) a OTG connector is. However, that means I
> cannot use the standard gpio-charger driver to detect the presence of a
> charger based on the state of the VBUS gpio, since it's already requested
> here. So the point of this patchset is to provide an alternative to
> gpio-charger that works with OTG controllers compatible with
> 'usb-role-switch'.

Why not fix gpio-charger or make whatever 'owns' vbus-gpios to register 
a charger device?

I guess you could have Vbus sensing with no charging capability, but 
that sounds like a new property, not a node to me.

Rob
Paul Cercueil Dec. 21, 2019, 1:55 p.m. UTC | #10 
Hi Rob,


Le jeu., déc. 19, 2019 at 13:38, Rob Herring <robh@kernel.org> a 
écrit :
> On Mon, Dec 16, 2019 at 11:52:05AM +0100, Paul Cercueil wrote:
>>  Hi Peter,
>> 
>> 
>>  Le lun., déc. 16, 2019 at 01:24, Peter Chen <peter.chen@nxp.com> a 
>> écrit :
>>  >
>>  > >  >>  +
>>  > >  >>  +	desc = &charger->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;
>>  > >  >
>>  > >  > What's your further plan for this generic USB charger?
>>  > >  > To support BC1.2, we need to know charger type, and how we 
>> could
>>  > > get
>>  > >  > it?
>>  > >  >
>>  > >  > Peter
>>  > >
>>  > >  Well I don't really know. The USB role framework does not give 
>> any
>>  > > info about
>>  > >  what's plugged.
>>  > >
>>  >
>>  > What's the use case for this patch set? How it be used?
>> 
>>  My devicetree:
>> 
>>  usb_otg: usb@13440000 {
>>  	compatible = "ingenic,jz4770-musb", "simple-mfd";
>>  	reg = <0x13440000 0x10000>;
>>  	[...]
>> 
>>  	usb-role-switch;
>> 
>>  	connector {
>>  		compatible = "gpio-usb-b-connector", "usb-b-connector";
>>  		label = "mini-USB";
>>  		type = "mini";
>> 
>>  		id-gpios = <&gpf 18 GPIO_ACTIVE_HIGH>;
>>  		vbus-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>;
>>  		[...]
>>  	};
>> 
>>  	usb_charger: usb-charger {
>>  		compatible = "usb-charger";
> 
> What h/w device is this?

GCW Zero: arch/mips/boot/dts/ingenic/gcw0.dts

Most of it isn't upstream, so I can still experiment things to get the 
perfect devicetree.

> 
>>  	};
>>  };
>> 
>>  The new gpio-usb-connector driver uses the ID/VBUS GPIOs to detect 
>> in which
>>  state (device, host, unconnected) a OTG connector is. However, that 
>> means I
>>  cannot use the standard gpio-charger driver to detect the presence 
>> of a
>>  charger based on the state of the VBUS gpio, since it's already 
>> requested
>>  here. So the point of this patchset is to provide an alternative to
>>  gpio-charger that works with OTG controllers compatible with
>>  'usb-role-switch'.
> 
> Why not fix gpio-charger or make whatever 'owns' vbus-gpios to 
> register
> a charger device?

I see there is a GPIOD_FLAGS_BIT_NONEXCLUSIVE bit that could be used, 
I'll have a look.

Thanks,
-Paul

> 
> I guess you could have Vbus sensing with no charging capability, but
> that sounds like a new property, not a node to me.
> 
> Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/power/supply/usb-charger.yaml b/Documentation/devicetree/bindings/power/supply/usb-charger.yaml
new file mode 100644
index 000000000000..dcd705df6e73
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/usb-charger.yaml
@@ -0,0 +1,41 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/usb-charger.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic USB charger driver bindings
+
+maintainers:
+  - Paul Cercueil <paul@crapouillou.net>
+
+description: |
+  Devicetree bindings for a generic USB charger.
+
+  The node should be either a child of a USB node, or connect to the USB
+  node through the graph. The USB node must have the usb-role-switch property
+  set. The USB charger will report as online when the USB role is set to device,
+  and offline otherwise.
+
+properties:
+  compatible:
+    const: usb-charger
+
+  port: true
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |+
+    usb_charger: usb-charger {
+      compatible = "usb-charger";
+
+      port {
+        usb_charger_ep: endpoint {
+          remote-endpoint = <&usb_otg>;
+        };
+      };
+    };