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

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

[Paul Cercueil]

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

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>;
+        };
+      };
+    };
-- 
2.24.0

[PATCH v2 2/3] usb: roles: Add API to register notifiers

[Paul Cercueil]

Add usb_role_switch_notifier_register() and
usb_role_switch_notifier_unregister().

The registered notifiers will be called when the USB role is changed.

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

Notes:
    v2: New patch

 drivers/usb/roles/class.c | 24 +++++++++++++++++++++++-
 include/linux/usb/role.h  | 20 ++++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index 8273126ffdf4..9b122b504b98 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -10,6 +10,7 @@
 #include <linux/usb/role.h>
 #include <linux/property.h>
 #include <linux/device.h>
+#include <linux/notifier.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
@@ -19,6 +20,7 @@ static struct class *role_class;
 struct usb_role_switch {
 	struct device dev;
 	struct mutex lock; /* device lock*/
+	struct blocking_notifier_head notifier_chain;
 	enum usb_role role;
 
 	/* From descriptor */
@@ -49,8 +51,11 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
 	mutex_lock(&sw->lock);
 
 	ret = sw->set(sw->dev.parent, role);
-	if (!ret)
+	if (!ret) {
 		sw->role = role;
+		ret = blocking_notifier_call_chain(&sw->notifier_chain,
+						   (long)role, sw);
+	}
 
 	mutex_unlock(&sw->lock);
 
@@ -85,6 +90,22 @@ enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_get_role);
 
+int
+usb_role_switch_register_notifier(struct usb_role_switch *sw,
+				  struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&sw->notifier_chain, nb);
+}
+EXPORT_SYMBOL_GPL(usb_role_switch_register_notifier);
+
+int
+usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
+				    struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&sw->notifier_chain, nb);
+}
+EXPORT_SYMBOL_GPL(usb_role_switch_unregister_notifier);
+
 static void *usb_role_switch_match(struct device_connection *con, int ep,
 				   void *data)
 {
@@ -317,6 +338,7 @@ usb_role_switch_register(struct device *parent,
 		return ERR_PTR(-ENOMEM);
 
 	mutex_init(&sw->lock);
+	BLOCKING_INIT_NOTIFIER_HEAD(&sw->notifier_chain);
 
 	sw->allow_userspace_control = desc->allow_userspace_control;
 	sw->usb2_port = desc->usb2_port;
diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
index efac3af83d6b..5f67d42cd28d 100644
--- a/include/linux/usb/role.h
+++ b/include/linux/usb/role.h
@@ -6,6 +6,7 @@
 #include <linux/device.h>
 
 struct usb_role_switch;
+struct notifier_block;
 
 enum usb_role {
 	USB_ROLE_NONE,
@@ -50,6 +51,11 @@ struct usb_role_switch *usb_role_switch_get(struct device *dev);
 struct usb_role_switch *fwnode_usb_role_switch_get(struct fwnode_handle *node);
 void usb_role_switch_put(struct usb_role_switch *sw);
 
+int usb_role_switch_register_notifier(struct usb_role_switch *sw,
+				      struct notifier_block *nb);
+int usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
+					struct notifier_block *nb);
+
 struct usb_role_switch *
 usb_role_switch_find_by_fwnode(const struct fwnode_handle *fwnode);
 
@@ -82,6 +88,20 @@ fwnode_usb_role_switch_get(struct fwnode_handle *node)
 
 static inline void usb_role_switch_put(struct usb_role_switch *sw) { }
 
+static inline int
+usb_role_switch_register_notifier(struct usb_role_switch *sw,
+				  struct notifier_block *nb)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline int
+usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
+				    struct notifier_block *nb)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 static inline struct usb_role_switch *
 usb_role_switch_register(struct device *parent,
 			 const struct usb_role_switch_desc *desc)
-- 
2.24.0

[PATCH v2 3/3] power/supply: Add generic USB charger driver

[Paul Cercueil]

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;
+
+	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

Re: [PATCH v2 3/3] power/supply: Add generic USB charger driver

[Peter Chen]

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
> 

-- 

Thanks,
Peter Chen

Re: [PATCH v2 3/3] power/supply: Add generic USB charger driver

[Paul Cercueil]

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

RE: [PATCH v2 3/3] power/supply: Add generic USB charger driver

[Peter Chen]

 
> >>  +
> >>  +	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
> 

RE: [PATCH v2 3/3] power/supply: Add generic USB charger driver

[Paul Cercueil]

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
>> 
> 

RE: [PATCH v2 3/3] power/supply: Add generic USB charger driver

[Peter Chen]

 
> 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
> >>
> >
> 

RE: [PATCH v2 3/3] power/supply: Add generic USB charger driver

[Paul Cercueil]

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
>>  >>
>>  >
>> 
> 

RE: [PATCH v2 3/3] power/supply: Add generic USB charger driver

[Peter Chen]

 
> >>  >
> >>  >>  >>  +
> >>  >>  >>  +	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

RE: [PATCH v2 3/3] power/supply: Add generic USB charger driver

[Paul Cercueil]

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

Re: [PATCH v2 3/3] power/supply: Add generic USB charger driver

[Rob Herring]

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

Re: [PATCH v2 3/3] power/supply: Add generic USB charger driver

[Paul Cercueil]

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