| Message ID | 20191103220801.10666-1-paul@crapouillou.net |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/2] dt-bindings: power/supply: Document generic USB charger | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Sun, Nov 3, 2019 at 4:08 PM Paul Cercueil <paul@crapouillou.net> wrote: > > Add documentation about the devicetree bindings for the generic USB > charger. What makes it generic? > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > --- > .../bindings/power/supply/usb-charger.txt | 24 +++++++++++++++++++ > 1 file changed, 24 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/supply/usb-charger.txt > > diff --git a/Documentation/devicetree/bindings/power/supply/usb-charger.txt b/Documentation/devicetree/bindings/power/supply/usb-charger.txt > new file mode 100644 > index 000000000000..fd46734cb0e5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/supply/usb-charger.txt > @@ -0,0 +1,24 @@ > +Generic USB charger bindings > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + > +Required properties : > + - compatible : should be "usb-charger" > + - phys: phandle to the USB PHY > + > +Example: > + > +usb_con: extcon { > + compatible = "linux,extcon-usb-gpio"; > + vbus-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>; > +}; > + > +usb_phy: usb-phy@0 { > + compatible = "usb-nop-xceiv"; > + #phy-cells = <0>; > + extcon = <&usb_con>; extcon is deprecated in favor of usb-connector binding. See .../bindings/connector/usb-connector.txt. There's also some pending patches for adding GPIO based connector controls including Vbus sense (GPIO input) and control (regulator via a GPIO). Rob
Hi Rob, Le lun., nov. 4, 2019 at 07:52, Rob Herring <robh+dt@kernel.org> a écrit : > On Sun, Nov 3, 2019 at 4:08 PM Paul Cercueil <paul@crapouillou.net> > wrote: >> >> Add documentation about the devicetree bindings for the generic USB >> charger. > > What makes it generic? It only uses the USB PHY subsystem, which already has some half-baked support for chargers but not bound to the power-supply subsystem. >> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >> --- >> .../bindings/power/supply/usb-charger.txt | 24 >> +++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/power/supply/usb-charger.txt >> >> diff --git >> a/Documentation/devicetree/bindings/power/supply/usb-charger.txt >> b/Documentation/devicetree/bindings/power/supply/usb-charger.txt >> new file mode 100644 >> index 000000000000..fd46734cb0e5 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power/supply/usb-charger.txt >> @@ -0,0 +1,24 @@ >> +Generic USB charger bindings >> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> + >> +Required properties : >> + - compatible : should be "usb-charger" >> + - phys: phandle to the USB PHY >> + >> +Example: >> + >> +usb_con: extcon { >> + compatible = "linux,extcon-usb-gpio"; >> + vbus-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>; >> +}; >> + >> +usb_phy: usb-phy@0 { >> + compatible = "usb-nop-xceiv"; >> + #phy-cells = <0>; >> + extcon = <&usb_con>; > > extcon is deprecated in favor of usb-connector binding. See > .../bindings/connector/usb-connector.txt. There's also some pending > patches for adding GPIO based connector controls including Vbus sense > (GPIO input) and control (regulator via a GPIO). > > Rob I understand that the usb-connector binding is better, but the current code doesn't integrate at all with the USB PHY subsystem, which has its own code to handle ID and VBUS GPIOs and supports notifiers. Is that deprecated then? What's the big picture here? Thanks, -Paul
On Tue, Nov 5, 2019 at 3:16 AM Paul Cercueil <paul@crapouillou.net> wrote: > > Hi Rob, > > > Le lun., nov. 4, 2019 at 07:52, Rob Herring <robh+dt@kernel.org> a > écrit : > > On Sun, Nov 3, 2019 at 4:08 PM Paul Cercueil <paul@crapouillou.net> > > wrote: > >> > >> Add documentation about the devicetree bindings for the generic USB > >> charger. > > > > What makes it generic? > > It only uses the USB PHY subsystem, which already has some half-baked > support for chargers but not bound to the power-supply subsystem. > > > >> > >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> > >> --- > >> .../bindings/power/supply/usb-charger.txt | 24 > >> +++++++++++++++++++ > >> 1 file changed, 24 insertions(+) > >> create mode 100644 > >> Documentation/devicetree/bindings/power/supply/usb-charger.txt > >> > >> diff --git > >> a/Documentation/devicetree/bindings/power/supply/usb-charger.txt > >> b/Documentation/devicetree/bindings/power/supply/usb-charger.txt > >> new file mode 100644 > >> index 000000000000..fd46734cb0e5 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/power/supply/usb-charger.txt > >> @@ -0,0 +1,24 @@ > >> +Generic USB charger bindings > >> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > >> + > >> +Required properties : > >> + - compatible : should be "usb-charger" > >> + - phys: phandle to the USB PHY > >> + > >> +Example: > >> + > >> +usb_con: extcon { > >> + compatible = "linux,extcon-usb-gpio"; > >> + vbus-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>; > >> +}; > >> + > >> +usb_phy: usb-phy@0 { > >> + compatible = "usb-nop-xceiv"; > >> + #phy-cells = <0>; > >> + extcon = <&usb_con>; > > > > extcon is deprecated in favor of usb-connector binding. See > > .../bindings/connector/usb-connector.txt. There's also some pending > > patches for adding GPIO based connector controls including Vbus sense > > (GPIO input) and control (regulator via a GPIO). > > > > Rob > > I understand that the usb-connector binding is better, but the current > code doesn't integrate at all with the USB PHY subsystem, which has its > own code to handle ID and VBUS GPIOs and supports notifiers. Is that > deprecated then? > > What's the big picture here? Does this series work for you?: https://patchwork.kernel.org/cover/11120707/
Le mar., nov. 5, 2019 at 07:48, Rob Herring <robh+dt@kernel.org> a écrit : > On Tue, Nov 5, 2019 at 3:16 AM Paul Cercueil <paul@crapouillou.net> > wrote: >> >> Hi Rob, >> >> >> Le lun., nov. 4, 2019 at 07:52, Rob Herring <robh+dt@kernel.org> a >> écrit : >> > On Sun, Nov 3, 2019 at 4:08 PM Paul Cercueil >> <paul@crapouillou.net> >> > wrote: >> >> >> >> Add documentation about the devicetree bindings for the generic >> USB >> >> charger. >> > >> > What makes it generic? >> >> It only uses the USB PHY subsystem, which already has some >> half-baked >> support for chargers but not bound to the power-supply subsystem. >> >> >> >> >> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >> >> --- >> >> .../bindings/power/supply/usb-charger.txt | 24 >> >> +++++++++++++++++++ >> >> 1 file changed, 24 insertions(+) >> >> create mode 100644 >> >> Documentation/devicetree/bindings/power/supply/usb-charger.txt >> >> >> >> diff --git >> >> a/Documentation/devicetree/bindings/power/supply/usb-charger.txt >> >> b/Documentation/devicetree/bindings/power/supply/usb-charger.txt >> >> new file mode 100644 >> >> index 000000000000..fd46734cb0e5 >> >> --- /dev/null >> >> +++ >> b/Documentation/devicetree/bindings/power/supply/usb-charger.txt >> >> @@ -0,0 +1,24 @@ >> >> +Generic USB charger bindings >> >> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> + >> >> +Required properties : >> >> + - compatible : should be "usb-charger" >> >> + - phys: phandle to the USB PHY >> >> + >> >> +Example: >> >> + >> >> +usb_con: extcon { >> >> + compatible = "linux,extcon-usb-gpio"; >> >> + vbus-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>; >> >> +}; >> >> + >> >> +usb_phy: usb-phy@0 { >> >> + compatible = "usb-nop-xceiv"; >> >> + #phy-cells = <0>; >> >> + extcon = <&usb_con>; >> > >> > extcon is deprecated in favor of usb-connector binding. See >> > .../bindings/connector/usb-connector.txt. There's also some >> pending >> > patches for adding GPIO based connector controls including Vbus >> sense >> > (GPIO input) and control (regulator via a GPIO). >> > >> > Rob >> >> I understand that the usb-connector binding is better, but the >> current >> code doesn't integrate at all with the USB PHY subsystem, which has >> its >> own code to handle ID and VBUS GPIOs and supports notifiers. Is that >> deprecated then? >> >> What's the big picture here? > > Does this series work for you?: > > https://patchwork.kernel.org/cover/11120707/ I had to do some changes to my musb and PHY drivers but it works, yes. The good thing is that I didn't have to change this driver, so I'll wait for feedback on patch 2/2 then post a v2 with a fixed devicetree example. Thanks, -Paul
Hi Paul, On Sun, Nov 03, 2019 at 11:08:01PM +0100, Paul Cercueil wrote: > This simple charger driver uses the USB PHY framework to detect the > presence of a charger. > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > --- > drivers/power/supply/Kconfig | 7 ++ > drivers/power/supply/Makefile | 1 + > drivers/power/supply/generic-usb-charger.c | 140 +++++++++++++++++++++ > 3 files changed, 148 insertions(+) > create mode 100644 drivers/power/supply/generic-usb-charger.c > > diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig > index c84a7b1caeb6..069a91d89a42 100644 > --- a/drivers/power/supply/Kconfig > +++ b/drivers/power/supply/Kconfig > @@ -51,6 +51,13 @@ 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_PHY > + help > + Say Y here to enable a generic USB charger driver which uses > + the USB PHY 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..d005acfc33c7 > --- /dev/null > +++ b/drivers/power/supply/generic-usb-charger.c > @@ -0,0 +1,140 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Simple USB charger driver > + * Copyright (c) 2019 Paul Cercueil <paul@crapouillou.net> > + */ > + > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/power_supply.h> > +#include <linux/usb/phy.h> Missing <linux/of.h> for of_match_ptr() > + > +struct usb_charger { > + struct usb_phy *phy; > + struct notifier_block nb; > + 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); > + > + switch (psp) { > + case POWER_SUPPLY_PROP_ONLINE: > + val->intval = charger->phy->chg_state == USB_CHARGER_PRESENT; > + 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 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; > + > + platform_set_drvdata(pdev, charger); > + charger->nb.notifier_call = usb_charger_event; > + cfg.drv_data = charger; > + > + if (dev->of_node) > + charger->phy = devm_usb_get_phy_by_phandle(dev, "phys", 0); > + else > + charger->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); > + if (IS_ERR(charger->phy)) { > + err = PTR_ERR(charger->phy); > + if (err != -EPROBE_DEFER) > + dev_err(dev, "No transceiver configured"); > + return err; > + } > + > + 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; > + > + switch (charger->phy->chg_type) { > + case SDP_TYPE: > + desc->type = POWER_SUPPLY_TYPE_USB; > + break; > + case DCP_TYPE: > + desc->type = POWER_SUPPLY_TYPE_USB_DCP; > + break; > + case CDP_TYPE: > + desc->type = POWER_SUPPLY_TYPE_USB_CDP; > + break; > + case ACA_TYPE: > + desc->type = POWER_SUPPLY_TYPE_USB_ACA; > + break; > + default: > + desc->type = POWER_SUPPLY_TYPE_UNKNOWN; > + } This is deprecated in favour of the POWER_SUPPLY_PROP_USB_TYPE property. Set desc->type to POWER_SUPPLY_TYPE_USB and have a look at e.g. drivers/power/supply/cros_usbpd-charger.c to see how POWER_SUPPLY_PROP_USB_TYPE is supposed to work. > + 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); > + } > + > + return usb_register_notifier(charger->phy, &charger->nb); Please register with devm_add_action_or_reset() or (better) create a devm_usb_register_notifier(). > +} > + > +static int usb_charger_remove(struct platform_device *pdev) > +{ > + struct usb_charger *charger = platform_get_drvdata(pdev); > + > + usb_unregister_notifier(charger->phy, &charger->nb); > + > + return 0; > +} > + > +#ifdef CONFIG_OF > +static const struct of_device_id usb_charger_of_match[] = { > + { .compatible = "usb-charger" }, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, usb_charger_of_match); > +#endif > + > +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, > + .remove = usb_charger_remove, > +}; > +module_platform_driver(usb_charger_driver); > + > +MODULE_DESCRIPTION("Simple USB charger driver"); > +MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>"); > +MODULE_LICENSE("GPL"); -- Sebastian
diff --git a/Documentation/devicetree/bindings/power/supply/usb-charger.txt b/Documentation/devicetree/bindings/power/supply/usb-charger.txt new file mode 100644 index 000000000000..fd46734cb0e5 --- /dev/null +++ b/Documentation/devicetree/bindings/power/supply/usb-charger.txt @@ -0,0 +1,24 @@ +Generic USB charger bindings +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Required properties : + - compatible : should be "usb-charger" + - phys: phandle to the USB PHY + +Example: + +usb_con: extcon { + compatible = "linux,extcon-usb-gpio"; + vbus-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>; +}; + +usb_phy: usb-phy@0 { + compatible = "usb-nop-xceiv"; + #phy-cells = <0>; + extcon = <&usb_con>; +}; + +usb_charger: usb-charger { + compatible = "usb-charger"; + phys = <&usb_phy>; +};
Add documentation about the devicetree bindings for the generic USB charger. Signed-off-by: Paul Cercueil <paul@crapouillou.net> --- .../bindings/power/supply/usb-charger.txt | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/usb-charger.txt