| Message ID | 54C8D2F4.9050404@ti.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi Roger, We need to discuss one point about 'id_irqwake'. I don't recommend to use 'id_irqwake' field. And I catch build warning by using "select" keywork in Kconfig. It is my wrong guide of "select" keyword. So, I'll change it as 'depends on' keyword. Looks good to me except for 'id_irqwake'. I'll apply this patch on 3.21 queue after completing this discussion. On 01/28/2015 09:15 PM, Roger Quadros wrote: > This driver observes the USB ID pin connected over a GPIO and > updates the USB cable extcon states accordingly. > > The existing GPIO extcon driver is not suitable for this purpose > as it needs to be taught to understand USB cable states and it > can't handle more than one cable per instance. > > For the USB case we need to handle 2 cable states. > 1) USB (attach/detach) > 2) USB-Host (attach/detach) > > This driver can be easily updated in the future to handle VBUS > events in case it happens to be available on GPIO for any platform. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > v3: > - removed IRQF_NO_SUSPEND flag. Added IRQF_TRIGGER_RISING and > IRQF_TRIGGER_FALLING > - Added disable_irq() to suspend() and enable_irq() to resume() > > .../devicetree/bindings/extcon/extcon-usb-gpio.txt | 18 ++ > drivers/extcon/Kconfig | 7 + > drivers/extcon/Makefile | 1 + > drivers/extcon/extcon-usb-gpio.c | 233 +++++++++++++++++++++ > 4 files changed, 259 insertions(+) > create mode 100644 Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt > create mode 100644 drivers/extcon/extcon-usb-gpio.c > > diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt > new file mode 100644 > index 0000000..85fe6b0 > --- /dev/null > +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt > @@ -0,0 +1,18 @@ > +USB GPIO Extcon device > + > +This is a virtual device used to generate USB cable states from the USB ID pin > +connected to a GPIO pin. > + > +Required properties: > +- compatible: Should be "linux,extcon-usb-gpio" > +- id-gpio: gpio for USB ID pin. See gpio binding. > + > +Example: > + extcon_usb1 { > + compatible = "linux,extcon-usb-gpio"; > + id-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>; > + } > + > + &omap_dwc3_1 { > + extcon = <&extcon_usb1>; > + }; > diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig > index 6a1f7de..fd11536 100644 > --- a/drivers/extcon/Kconfig > +++ b/drivers/extcon/Kconfig > @@ -93,4 +93,11 @@ config EXTCON_SM5502 > Silicon Mitus SM5502. The SM5502 is a USB port accessory > detector and switch. > > +config EXTCON_USB_GPIO > + tristate "USB GPIO extcon support" > + select GPIOLIB I catch the build warning if using 'select' instead of 'depends on' as following: It is my wrong guide to you. So, I'll modify it by using "depends on" as your original patch. make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- zImage -j 8 scripts/kconfig/conf --silentoldconfig Kconfig drivers/gpio/Kconfig:34:error: recursive dependency detected! drivers/gpio/Kconfig:34: symbol GPIOLIB is selected by EXTCON_USB_GPIO drivers/extcon/Kconfig:96: symbol EXTCON_USB_GPIO depends on EXTCON drivers/extcon/Kconfig:1: symbol EXTCON is selected by CHARGER_MANAGER drivers/power/Kconfig:316: symbol CHARGER_MANAGER depends on POWER_SUPPLY drivers/power/Kconfig:1: symbol POWER_SUPPLY is selected by HID_SONY drivers/hid/Kconfig:670: symbol HID_SONY depends on NEW_LEDS drivers/leds/Kconfig:8: symbol NEW_LEDS is selected by BCMA_DRIVER_GPIO drivers/bcma/Kconfig:75: symbol BCMA_DRIVER_GPIO depends on GPIOLIB > + help > + Say Y here to enable GPIO based USB cable detection extcon support. > + Used typically if GPIO is used for USB ID pin detection. > + > endif # MULTISTATE_SWITCH > diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile > index 0370b42..6a08a98 100644 > --- a/drivers/extcon/Makefile > +++ b/drivers/extcon/Makefile > @@ -12,3 +12,4 @@ obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o > obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o > obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o > obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o > +obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o > diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c > new file mode 100644 > index 0000000..99a58b2 > --- /dev/null > +++ b/drivers/extcon/extcon-usb-gpio.c > @@ -0,0 +1,233 @@ > +/** > + * drivers/extcon/extcon-usb-gpio.c - USB GPIO extcon driver > + * > + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com > + * Author: Roger Quadros <rogerq@ti.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <linux/extcon.h> > +#include <linux/init.h> > +#include <linux/interrupt.h> > +#include <linux/irq.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of_gpio.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > +#include <linux/workqueue.h> > + > +#define USB_GPIO_DEBOUNCE_MS 20 /* ms */ > + > +struct usb_extcon_info { > + struct device *dev; > + struct extcon_dev *edev; > + > + struct gpio_desc *id_gpiod; > + int id_irq; > + bool id_irqwake; /* ID wakeup enabled flag */ Do you really think id_irqwake is necessary? I think it is not necessary. > + > + unsigned long debounce_jiffies; > + struct delayed_work wq_detcable; > +}; > + > +/* List of detectable cables */ > +enum { > + EXTCON_CABLE_USB = 0, > + EXTCON_CABLE_USB_HOST, > + > + EXTCON_CABLE_END, > +}; > + > +static const char *usb_extcon_cable[] = { > + [EXTCON_CABLE_USB] = "USB", > + [EXTCON_CABLE_USB_HOST] = "USB-Host", > + NULL, > +}; > + [snip] > + > +static int usb_extcon_remove(struct platform_device *pdev) > +{ > + struct usb_extcon_info *info = platform_get_drvdata(pdev); > + > + cancel_delayed_work_sync(&info->wq_detcable); > + > + return 0; > +} > + > +#ifdef CONFIG_PM_SLEEP > +static int usb_extcon_suspend(struct device *dev) > +{ > + struct usb_extcon_info *info = dev_get_drvdata(dev); > + > + if (device_may_wakeup(dev)) > + if (!enable_irq_wake(info->id_irq)) > + info->id_irqwake = true; > + You can simplify this code as following without 'id_irqwake': if (device_may_wakeup(dev)) enable_irq_wake(info->id_irq); > + /* > + * We don't want to process any IRQs after this point > + * as GPIOs used behind I2C subsystem might not be > + * accessible until resume completes. So disable IRQ. > + */ > + disable_irq(info->id_irq); > + > + return 0; > +} > + > +static int usb_extcon_resume(struct device *dev) > +{ > + struct usb_extcon_info *info = dev_get_drvdata(dev); > + > + if (info->id_irqwake) { > + disable_irq_wake(info->id_irq); > + info->id_irqwake = false; > + } ditto. if (device_may_wakeup(dev)) disable_irq_wake(info->id_irq); > + > + enable_irq(info->id_irq); > + > + return 0; > +} > +#endif > + > +static SIMPLE_DEV_PM_OPS(usb_extcon_pm_ops, > + usb_extcon_suspend, usb_extcon_resume); > + > +static struct of_device_id usb_extcon_dt_match[] = { > + { .compatible = "linux,extcon-usb-gpio", }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, usb_extcon_dt_match); > + > +static struct platform_driver usb_extcon_driver = { > + .probe = usb_extcon_probe, > + .remove = usb_extcon_remove, > + .driver = { > + .name = "extcon-usb-gpio", > + .pm = &usb_extcon_pm_ops, > + .of_match_table = usb_extcon_dt_match, > + }, > +}; > + > +module_platform_driver(usb_extcon_driver); > + > +MODULE_AUTHOR("Roger Quadros <rogerq@ti.com>"); > +MODULE_DESCRIPTION("USB GPIO extcon driver"); > +MODULE_LICENSE("GPL v2"); > Thanks, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Chanwoo, On 29/01/15 03:49, Chanwoo Choi wrote: > Hi Roger, > > We need to discuss one point about 'id_irqwake'. > I don't recommend to use 'id_irqwake' field. > > And I catch build warning by using "select" keywork in Kconfig. > It is my wrong guide of "select" keyword. So, I'll change it > as 'depends on' keyword. > > Looks good to me except for 'id_irqwake'. > I'll apply this patch on 3.21 queue after completing this discussion. > > On 01/28/2015 09:15 PM, Roger Quadros wrote: >> This driver observes the USB ID pin connected over a GPIO and >> updates the USB cable extcon states accordingly. >> >> The existing GPIO extcon driver is not suitable for this purpose >> as it needs to be taught to understand USB cable states and it >> can't handle more than one cable per instance. >> >> For the USB case we need to handle 2 cable states. >> 1) USB (attach/detach) >> 2) USB-Host (attach/detach) >> >> This driver can be easily updated in the future to handle VBUS >> events in case it happens to be available on GPIO for any platform. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> v3: >> - removed IRQF_NO_SUSPEND flag. Added IRQF_TRIGGER_RISING and >> IRQF_TRIGGER_FALLING >> - Added disable_irq() to suspend() and enable_irq() to resume() >> >> .../devicetree/bindings/extcon/extcon-usb-gpio.txt | 18 ++ >> drivers/extcon/Kconfig | 7 + >> drivers/extcon/Makefile | 1 + >> drivers/extcon/extcon-usb-gpio.c | 233 +++++++++++++++++++++ >> 4 files changed, 259 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >> create mode 100644 drivers/extcon/extcon-usb-gpio.c >> >> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >> new file mode 100644 >> index 0000000..85fe6b0 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >> @@ -0,0 +1,18 @@ >> +USB GPIO Extcon device >> + >> +This is a virtual device used to generate USB cable states from the USB ID pin >> +connected to a GPIO pin. >> + >> +Required properties: >> +- compatible: Should be "linux,extcon-usb-gpio" >> +- id-gpio: gpio for USB ID pin. See gpio binding. >> + >> +Example: >> + extcon_usb1 { >> + compatible = "linux,extcon-usb-gpio"; >> + id-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>; >> + } >> + >> + &omap_dwc3_1 { >> + extcon = <&extcon_usb1>; >> + }; >> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >> index 6a1f7de..fd11536 100644 >> --- a/drivers/extcon/Kconfig >> +++ b/drivers/extcon/Kconfig >> @@ -93,4 +93,11 @@ config EXTCON_SM5502 >> Silicon Mitus SM5502. The SM5502 is a USB port accessory >> detector and switch. >> >> +config EXTCON_USB_GPIO >> + tristate "USB GPIO extcon support" >> + select GPIOLIB > > I catch the build warning if using 'select' instead of 'depends on' as following: > It is my wrong guide to you. So, I'll modify it by using "depends on" as your original patch. OK. Thanks. > > make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- zImage -j 8 > scripts/kconfig/conf --silentoldconfig Kconfig > drivers/gpio/Kconfig:34:error: recursive dependency detected! > drivers/gpio/Kconfig:34: symbol GPIOLIB is selected by EXTCON_USB_GPIO > drivers/extcon/Kconfig:96: symbol EXTCON_USB_GPIO depends on EXTCON > drivers/extcon/Kconfig:1: symbol EXTCON is selected by CHARGER_MANAGER > drivers/power/Kconfig:316: symbol CHARGER_MANAGER depends on POWER_SUPPLY > drivers/power/Kconfig:1: symbol POWER_SUPPLY is selected by HID_SONY > drivers/hid/Kconfig:670: symbol HID_SONY depends on NEW_LEDS > drivers/leds/Kconfig:8: symbol NEW_LEDS is selected by BCMA_DRIVER_GPIO > drivers/bcma/Kconfig:75: symbol BCMA_DRIVER_GPIO depends on GPIOLIB > >> + help >> + Say Y here to enable GPIO based USB cable detection extcon support. >> + Used typically if GPIO is used for USB ID pin detection. >> + >> endif # MULTISTATE_SWITCH >> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile >> index 0370b42..6a08a98 100644 >> --- a/drivers/extcon/Makefile >> +++ b/drivers/extcon/Makefile >> @@ -12,3 +12,4 @@ obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o >> obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o >> obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o >> obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o >> +obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o >> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c >> new file mode 100644 >> index 0000000..99a58b2 >> --- /dev/null >> +++ b/drivers/extcon/extcon-usb-gpio.c >> @@ -0,0 +1,233 @@ >> +/** >> + * drivers/extcon/extcon-usb-gpio.c - USB GPIO extcon driver >> + * >> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com >> + * Author: Roger Quadros <rogerq@ti.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> + >> +#include <linux/extcon.h> >> +#include <linux/init.h> >> +#include <linux/interrupt.h> >> +#include <linux/irq.h> >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/of_gpio.h> >> +#include <linux/platform_device.h> >> +#include <linux/slab.h> >> +#include <linux/workqueue.h> >> + >> +#define USB_GPIO_DEBOUNCE_MS 20 /* ms */ >> + >> +struct usb_extcon_info { >> + struct device *dev; >> + struct extcon_dev *edev; >> + >> + struct gpio_desc *id_gpiod; >> + int id_irq; >> + bool id_irqwake; /* ID wakeup enabled flag */ > > Do you really think id_irqwake is necessary? > I think it is not necessary. I will explain below why it is necessary. > >> + >> + unsigned long debounce_jiffies; >> + struct delayed_work wq_detcable; >> +}; >> + >> +/* List of detectable cables */ >> +enum { >> + EXTCON_CABLE_USB = 0, >> + EXTCON_CABLE_USB_HOST, >> + >> + EXTCON_CABLE_END, >> +}; >> + >> +static const char *usb_extcon_cable[] = { >> + [EXTCON_CABLE_USB] = "USB", >> + [EXTCON_CABLE_USB_HOST] = "USB-Host", >> + NULL, >> +}; >> + > > [snip] > >> + >> +static int usb_extcon_remove(struct platform_device *pdev) >> +{ >> + struct usb_extcon_info *info = platform_get_drvdata(pdev); >> + >> + cancel_delayed_work_sync(&info->wq_detcable); >> + >> + return 0; >> +} >> + >> +#ifdef CONFIG_PM_SLEEP >> +static int usb_extcon_suspend(struct device *dev) >> +{ >> + struct usb_extcon_info *info = dev_get_drvdata(dev); >> + >> + if (device_may_wakeup(dev)) >> + if (!enable_irq_wake(info->id_irq)) >> + info->id_irqwake = true; >> + > > You can simplify this code as following without 'id_irqwake': > > if (device_may_wakeup(dev)) > enable_irq_wake(info->id_irq); enable_irq_wake() can fail. And if it does we need to keep track of it to prevent unbalanced disable_irq_wake() call in resume(). That's the reason I've added the id_irqwake flag in struct usb_extcon_info. > >> + /* >> + * We don't want to process any IRQs after this point >> + * as GPIOs used behind I2C subsystem might not be >> + * accessible until resume completes. So disable IRQ. >> + */ >> + disable_irq(info->id_irq); >> + >> + return 0; >> +} >> + >> +static int usb_extcon_resume(struct device *dev) >> +{ >> + struct usb_extcon_info *info = dev_get_drvdata(dev); >> + >> + if (info->id_irqwake) { >> + disable_irq_wake(info->id_irq); >> + info->id_irqwake = false; >> + } > > ditto. > > if (device_may_wakeup(dev)) > disable_irq_wake(info->id_irq); > >> + >> + enable_irq(info->id_irq); >> + >> + return 0; >> +} >> +#endif >> + >> +static SIMPLE_DEV_PM_OPS(usb_extcon_pm_ops, >> + usb_extcon_suspend, usb_extcon_resume); >> + >> +static struct of_device_id usb_extcon_dt_match[] = { >> + { .compatible = "linux,extcon-usb-gpio", }, >> + { /* sentinel */ } >> +}; >> +MODULE_DEVICE_TABLE(of, usb_extcon_dt_match); >> + >> +static struct platform_driver usb_extcon_driver = { >> + .probe = usb_extcon_probe, >> + .remove = usb_extcon_remove, >> + .driver = { >> + .name = "extcon-usb-gpio", >> + .pm = &usb_extcon_pm_ops, >> + .of_match_table = usb_extcon_dt_match, >> + }, >> +}; >> + >> +module_platform_driver(usb_extcon_driver); >> + >> +MODULE_AUTHOR("Roger Quadros <rogerq@ti.com>"); >> +MODULE_DESCRIPTION("USB GPIO extcon driver"); >> +MODULE_LICENSE("GPL v2"); >> > cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Roger, On 01/29/2015 08:26 PM, Roger Quadros wrote: > Chanwoo, > > On 29/01/15 03:49, Chanwoo Choi wrote: >> Hi Roger, >> >> We need to discuss one point about 'id_irqwake'. >> I don't recommend to use 'id_irqwake' field. >> >> And I catch build warning by using "select" keywork in Kconfig. >> It is my wrong guide of "select" keyword. So, I'll change it >> as 'depends on' keyword. >> >> Looks good to me except for 'id_irqwake'. >> I'll apply this patch on 3.21 queue after completing this discussion. >> >> On 01/28/2015 09:15 PM, Roger Quadros wrote: >>> This driver observes the USB ID pin connected over a GPIO and >>> updates the USB cable extcon states accordingly. >>> >>> The existing GPIO extcon driver is not suitable for this purpose >>> as it needs to be taught to understand USB cable states and it >>> can't handle more than one cable per instance. >>> >>> For the USB case we need to handle 2 cable states. >>> 1) USB (attach/detach) >>> 2) USB-Host (attach/detach) >>> >>> This driver can be easily updated in the future to handle VBUS >>> events in case it happens to be available on GPIO for any platform. >>> >>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>> --- >>> v3: >>> - removed IRQF_NO_SUSPEND flag. Added IRQF_TRIGGER_RISING and >>> IRQF_TRIGGER_FALLING >>> - Added disable_irq() to suspend() and enable_irq() to resume() >>> >>> .../devicetree/bindings/extcon/extcon-usb-gpio.txt | 18 ++ >>> drivers/extcon/Kconfig | 7 + >>> drivers/extcon/Makefile | 1 + >>> drivers/extcon/extcon-usb-gpio.c | 233 +++++++++++++++++++++ >>> 4 files changed, 259 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >>> create mode 100644 drivers/extcon/extcon-usb-gpio.c >>> >>> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >>> new file mode 100644 >>> index 0000000..85fe6b0 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >>> @@ -0,0 +1,18 @@ >>> +USB GPIO Extcon device >>> + >>> +This is a virtual device used to generate USB cable states from the USB ID pin >>> +connected to a GPIO pin. >>> + >>> +Required properties: >>> +- compatible: Should be "linux,extcon-usb-gpio" >>> +- id-gpio: gpio for USB ID pin. See gpio binding. >>> + >>> +Example: >>> + extcon_usb1 { >>> + compatible = "linux,extcon-usb-gpio"; >>> + id-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>; >>> + } >>> + >>> + &omap_dwc3_1 { >>> + extcon = <&extcon_usb1>; >>> + }; >>> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >>> index 6a1f7de..fd11536 100644 >>> --- a/drivers/extcon/Kconfig >>> +++ b/drivers/extcon/Kconfig >>> @@ -93,4 +93,11 @@ config EXTCON_SM5502 >>> Silicon Mitus SM5502. The SM5502 is a USB port accessory >>> detector and switch. >>> >>> +config EXTCON_USB_GPIO >>> + tristate "USB GPIO extcon support" >>> + select GPIOLIB >> >> I catch the build warning if using 'select' instead of 'depends on' as following: >> It is my wrong guide to you. So, I'll modify it by using "depends on" as your original patch. > > OK. Thanks. > >> >> make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- zImage -j 8 >> scripts/kconfig/conf --silentoldconfig Kconfig >> drivers/gpio/Kconfig:34:error: recursive dependency detected! >> drivers/gpio/Kconfig:34: symbol GPIOLIB is selected by EXTCON_USB_GPIO >> drivers/extcon/Kconfig:96: symbol EXTCON_USB_GPIO depends on EXTCON >> drivers/extcon/Kconfig:1: symbol EXTCON is selected by CHARGER_MANAGER >> drivers/power/Kconfig:316: symbol CHARGER_MANAGER depends on POWER_SUPPLY >> drivers/power/Kconfig:1: symbol POWER_SUPPLY is selected by HID_SONY >> drivers/hid/Kconfig:670: symbol HID_SONY depends on NEW_LEDS >> drivers/leds/Kconfig:8: symbol NEW_LEDS is selected by BCMA_DRIVER_GPIO >> drivers/bcma/Kconfig:75: symbol BCMA_DRIVER_GPIO depends on GPIOLIB >> >>> + help >>> + Say Y here to enable GPIO based USB cable detection extcon support. >>> + Used typically if GPIO is used for USB ID pin detection. >>> + >>> endif # MULTISTATE_SWITCH >>> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile >>> index 0370b42..6a08a98 100644 >>> --- a/drivers/extcon/Makefile >>> +++ b/drivers/extcon/Makefile >>> @@ -12,3 +12,4 @@ obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o >>> obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o >>> obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o >>> obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o >>> +obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o >>> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c >>> new file mode 100644 >>> index 0000000..99a58b2 >>> --- /dev/null >>> +++ b/drivers/extcon/extcon-usb-gpio.c >>> @@ -0,0 +1,233 @@ >>> +/** >>> + * drivers/extcon/extcon-usb-gpio.c - USB GPIO extcon driver >>> + * >>> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com >>> + * Author: Roger Quadros <rogerq@ti.com> >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License version 2 as >>> + * published by the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> + * GNU General Public License for more details. >>> + */ >>> + >>> +#include <linux/extcon.h> >>> +#include <linux/init.h> >>> +#include <linux/interrupt.h> >>> +#include <linux/irq.h> >>> +#include <linux/kernel.h> >>> +#include <linux/module.h> >>> +#include <linux/of_gpio.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/slab.h> >>> +#include <linux/workqueue.h> >>> + >>> +#define USB_GPIO_DEBOUNCE_MS 20 /* ms */ >>> + >>> +struct usb_extcon_info { >>> + struct device *dev; >>> + struct extcon_dev *edev; >>> + >>> + struct gpio_desc *id_gpiod; >>> + int id_irq; >>> + bool id_irqwake; /* ID wakeup enabled flag */ >> >> Do you really think id_irqwake is necessary? >> I think it is not necessary. > > I will explain below why it is necessary. > >> >>> + >>> + unsigned long debounce_jiffies; >>> + struct delayed_work wq_detcable; >>> +}; >>> + >>> +/* List of detectable cables */ >>> +enum { >>> + EXTCON_CABLE_USB = 0, >>> + EXTCON_CABLE_USB_HOST, >>> + >>> + EXTCON_CABLE_END, >>> +}; >>> + >>> +static const char *usb_extcon_cable[] = { >>> + [EXTCON_CABLE_USB] = "USB", >>> + [EXTCON_CABLE_USB_HOST] = "USB-Host", >>> + NULL, >>> +}; >>> + >> >> [snip] >> >>> + >>> +static int usb_extcon_remove(struct platform_device *pdev) >>> +{ >>> + struct usb_extcon_info *info = platform_get_drvdata(pdev); >>> + >>> + cancel_delayed_work_sync(&info->wq_detcable); >>> + >>> + return 0; >>> +} >>> + >>> +#ifdef CONFIG_PM_SLEEP >>> +static int usb_extcon_suspend(struct device *dev) >>> +{ >>> + struct usb_extcon_info *info = dev_get_drvdata(dev); >>> + >>> + if (device_may_wakeup(dev)) >>> + if (!enable_irq_wake(info->id_irq)) >>> + info->id_irqwake = true; >>> + >> >> You can simplify this code as following without 'id_irqwake': >> >> if (device_may_wakeup(dev)) >> enable_irq_wake(info->id_irq); > > enable_irq_wake() can fail. And if it does we need to keep track of it > to prevent unbalanced disable_irq_wake() call in resume(). > That's the reason I've added the id_irqwake flag in struct usb_extcon_info. It is not proper solution using id_irqwake. If fail to execute enable_irq_wake(), usb_extcon_suspend() function have to return error immediately. > >> >>> + /* >>> + * We don't want to process any IRQs after this point >>> + * as GPIOs used behind I2C subsystem might not be >>> + * accessible until resume completes. So disable IRQ. >>> + */ >>> + disable_irq(info->id_irq); >>> + >>> + return 0; >>> +} >>> + >>> +static int usb_extcon_resume(struct device *dev) >>> +{ >>> + struct usb_extcon_info *info = dev_get_drvdata(dev); >>> + >>> + if (info->id_irqwake) { >>> + disable_irq_wake(info->id_irq); >>> + info->id_irqwake = false; >>> + } >> >> ditto. >> >> if (device_may_wakeup(dev)) >> disable_irq_wake(info->id_irq); >> >>> + >>> + enable_irq(info->id_irq); >>> + >>> + return 0; >>> +} >>> +#endif >>> + >>> +static SIMPLE_DEV_PM_OPS(usb_extcon_pm_ops, >>> + usb_extcon_suspend, usb_extcon_resume); >>> + >>> +static struct of_device_id usb_extcon_dt_match[] = { >>> + { .compatible = "linux,extcon-usb-gpio", }, >>> + { /* sentinel */ } >>> +}; >>> +MODULE_DEVICE_TABLE(of, usb_extcon_dt_match); >>> + >>> +static struct platform_driver usb_extcon_driver = { >>> + .probe = usb_extcon_probe, >>> + .remove = usb_extcon_remove, >>> + .driver = { >>> + .name = "extcon-usb-gpio", >>> + .pm = &usb_extcon_pm_ops, >>> + .of_match_table = usb_extcon_dt_match, >>> + }, >>> +}; >>> + >>> +module_platform_driver(usb_extcon_driver); >>> + >>> +MODULE_AUTHOR("Roger Quadros <rogerq@ti.com>"); >>> +MODULE_DESCRIPTION("USB GPIO extcon driver"); >>> +MODULE_LICENSE("GPL v2"); Thanks, Chanwoo -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Roger, On 01/28/2015 09:15 PM, Roger Quadros wrote: > This driver observes the USB ID pin connected over a GPIO and > updates the USB cable extcon states accordingly. > > The existing GPIO extcon driver is not suitable for this purpose > as it needs to be taught to understand USB cable states and it > can't handle more than one cable per instance. > > For the USB case we need to handle 2 cable states. > 1) USB (attach/detach) > 2) USB-Host (attach/detach) > > This driver can be easily updated in the future to handle VBUS > events in case it happens to be available on GPIO for any platform. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > v3: > - removed IRQF_NO_SUSPEND flag. Added IRQF_TRIGGER_RISING and > IRQF_TRIGGER_FALLING > - Added disable_irq() to suspend() and enable_irq() to resume() > > .../devicetree/bindings/extcon/extcon-usb-gpio.txt | 18 ++ > drivers/extcon/Kconfig | 7 + > drivers/extcon/Makefile | 1 + > drivers/extcon/extcon-usb-gpio.c | 233 +++++++++++++++++++++ > 4 files changed, 259 insertions(+) > create mode 100644 Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt > create mode 100644 drivers/extcon/extcon-usb-gpio.c > > diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt > new file mode 100644 > index 0000000..85fe6b0 > --- /dev/null > +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt > @@ -0,0 +1,18 @@ > +USB GPIO Extcon device > + > +This is a virtual device used to generate USB cable states from the USB ID pin > +connected to a GPIO pin. > + > +Required properties: > +- compatible: Should be "linux,extcon-usb-gpio" > +- id-gpio: gpio for USB ID pin. See gpio binding. > + > +Example: > + extcon_usb1 { > + compatible = "linux,extcon-usb-gpio"; > + id-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>; > + } > + > + &omap_dwc3_1 { > + extcon = <&extcon_usb1>; > + }; > diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig > index 6a1f7de..fd11536 100644 > --- a/drivers/extcon/Kconfig > +++ b/drivers/extcon/Kconfig > @@ -93,4 +93,11 @@ config EXTCON_SM5502 > Silicon Mitus SM5502. The SM5502 is a USB port accessory > detector and switch. > > +config EXTCON_USB_GPIO > + tristate "USB GPIO extcon support" > + select GPIOLIB > + help > + Say Y here to enable GPIO based USB cable detection extcon support. > + Used typically if GPIO is used for USB ID pin detection. > + > endif # MULTISTATE_SWITCH > diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile > index 0370b42..6a08a98 100644 > --- a/drivers/extcon/Makefile > +++ b/drivers/extcon/Makefile > @@ -12,3 +12,4 @@ obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o > obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o > obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o > obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o > +obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o > diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c > new file mode 100644 > index 0000000..99a58b2 > --- /dev/null > +++ b/drivers/extcon/extcon-usb-gpio.c > @@ -0,0 +1,233 @@ > +/** > + * drivers/extcon/extcon-usb-gpio.c - USB GPIO extcon driver > + * > + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com > + * Author: Roger Quadros <rogerq@ti.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <linux/extcon.h> > +#include <linux/init.h> > +#include <linux/interrupt.h> > +#include <linux/irq.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of_gpio.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > +#include <linux/workqueue.h> > + > +#define USB_GPIO_DEBOUNCE_MS 20 /* ms */ > + > +struct usb_extcon_info { > + struct device *dev; > + struct extcon_dev *edev; > + > + struct gpio_desc *id_gpiod; > + int id_irq; > + bool id_irqwake; /* ID wakeup enabled flag */ > + > + unsigned long debounce_jiffies; > + struct delayed_work wq_detcable; > +}; > + > +/* List of detectable cables */ > +enum { > + EXTCON_CABLE_USB = 0, > + EXTCON_CABLE_USB_HOST, > + > + EXTCON_CABLE_END, > +}; > + > +static const char *usb_extcon_cable[] = { > + [EXTCON_CABLE_USB] = "USB", > + [EXTCON_CABLE_USB_HOST] = "USB-Host", I'll use the defined name for extcon cable name as soon because it has potential isseu about the conflict of extcon cable name between subsystems. So, I recommend to use a captical letter as "USB-HOST" instead of "USB-Host". If other extcon driver don't use the captical letter, I'll fix it. [snip] Thanks, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Chanwoo, On 30/01/15 02:06, Chanwoo Choi wrote: > Hi Roger, > > On 01/29/2015 08:26 PM, Roger Quadros wrote: >> Chanwoo, >> >> On 29/01/15 03:49, Chanwoo Choi wrote: >>> Hi Roger, >>> >>> We need to discuss one point about 'id_irqwake'. >>> I don't recommend to use 'id_irqwake' field. >>> >>> And I catch build warning by using "select" keywork in Kconfig. >>> It is my wrong guide of "select" keyword. So, I'll change it >>> as 'depends on' keyword. >>> >>> Looks good to me except for 'id_irqwake'. >>> I'll apply this patch on 3.21 queue after completing this discussion. >>> >>> On 01/28/2015 09:15 PM, Roger Quadros wrote: >>>> This driver observes the USB ID pin connected over a GPIO and >>>> updates the USB cable extcon states accordingly. >>>> >>>> The existing GPIO extcon driver is not suitable for this purpose >>>> as it needs to be taught to understand USB cable states and it >>>> can't handle more than one cable per instance. >>>> >>>> For the USB case we need to handle 2 cable states. >>>> 1) USB (attach/detach) >>>> 2) USB-Host (attach/detach) >>>> >>>> This driver can be easily updated in the future to handle VBUS >>>> events in case it happens to be available on GPIO for any platform. >>>> >>>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>>> --- >>>> v3: >>>> - removed IRQF_NO_SUSPEND flag. Added IRQF_TRIGGER_RISING and >>>> IRQF_TRIGGER_FALLING >>>> - Added disable_irq() to suspend() and enable_irq() to resume() >>>> >>>> .../devicetree/bindings/extcon/extcon-usb-gpio.txt | 18 ++ >>>> drivers/extcon/Kconfig | 7 + >>>> drivers/extcon/Makefile | 1 + >>>> drivers/extcon/extcon-usb-gpio.c | 233 +++++++++++++++++++++ >>>> 4 files changed, 259 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >>>> create mode 100644 drivers/extcon/extcon-usb-gpio.c >>>> >>>> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >>>> new file mode 100644 >>>> index 0000000..85fe6b0 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >>>> @@ -0,0 +1,18 @@ >>>> +USB GPIO Extcon device >>>> + >>>> +This is a virtual device used to generate USB cable states from the USB ID pin >>>> +connected to a GPIO pin. >>>> + >>>> +Required properties: >>>> +- compatible: Should be "linux,extcon-usb-gpio" >>>> +- id-gpio: gpio for USB ID pin. See gpio binding. >>>> + >>>> +Example: >>>> + extcon_usb1 { >>>> + compatible = "linux,extcon-usb-gpio"; >>>> + id-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>; >>>> + } >>>> + >>>> + &omap_dwc3_1 { >>>> + extcon = <&extcon_usb1>; >>>> + }; >>>> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >>>> index 6a1f7de..fd11536 100644 >>>> --- a/drivers/extcon/Kconfig >>>> +++ b/drivers/extcon/Kconfig >>>> @@ -93,4 +93,11 @@ config EXTCON_SM5502 >>>> Silicon Mitus SM5502. The SM5502 is a USB port accessory >>>> detector and switch. >>>> >>>> +config EXTCON_USB_GPIO >>>> + tristate "USB GPIO extcon support" >>>> + select GPIOLIB >>> >>> I catch the build warning if using 'select' instead of 'depends on' as following: >>> It is my wrong guide to you. So, I'll modify it by using "depends on" as your original patch. >> >> OK. Thanks. >> >>> >>> make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- zImage -j 8 >>> scripts/kconfig/conf --silentoldconfig Kconfig >>> drivers/gpio/Kconfig:34:error: recursive dependency detected! >>> drivers/gpio/Kconfig:34: symbol GPIOLIB is selected by EXTCON_USB_GPIO >>> drivers/extcon/Kconfig:96: symbol EXTCON_USB_GPIO depends on EXTCON >>> drivers/extcon/Kconfig:1: symbol EXTCON is selected by CHARGER_MANAGER >>> drivers/power/Kconfig:316: symbol CHARGER_MANAGER depends on POWER_SUPPLY >>> drivers/power/Kconfig:1: symbol POWER_SUPPLY is selected by HID_SONY >>> drivers/hid/Kconfig:670: symbol HID_SONY depends on NEW_LEDS >>> drivers/leds/Kconfig:8: symbol NEW_LEDS is selected by BCMA_DRIVER_GPIO >>> drivers/bcma/Kconfig:75: symbol BCMA_DRIVER_GPIO depends on GPIOLIB >>> >>>> + help >>>> + Say Y here to enable GPIO based USB cable detection extcon support. >>>> + Used typically if GPIO is used for USB ID pin detection. >>>> + >>>> endif # MULTISTATE_SWITCH >>>> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile >>>> index 0370b42..6a08a98 100644 >>>> --- a/drivers/extcon/Makefile >>>> +++ b/drivers/extcon/Makefile >>>> @@ -12,3 +12,4 @@ obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o >>>> obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o >>>> obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o >>>> obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o >>>> +obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o >>>> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c >>>> new file mode 100644 >>>> index 0000000..99a58b2 >>>> --- /dev/null >>>> +++ b/drivers/extcon/extcon-usb-gpio.c >>>> @@ -0,0 +1,233 @@ >>>> +/** >>>> + * drivers/extcon/extcon-usb-gpio.c - USB GPIO extcon driver >>>> + * >>>> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com >>>> + * Author: Roger Quadros <rogerq@ti.com> >>>> + * >>>> + * This program is free software; you can redistribute it and/or modify >>>> + * it under the terms of the GNU General Public License version 2 as >>>> + * published by the Free Software Foundation. >>>> + * >>>> + * This program is distributed in the hope that it will be useful, >>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>> + * GNU General Public License for more details. >>>> + */ >>>> + >>>> +#include <linux/extcon.h> >>>> +#include <linux/init.h> >>>> +#include <linux/interrupt.h> >>>> +#include <linux/irq.h> >>>> +#include <linux/kernel.h> >>>> +#include <linux/module.h> >>>> +#include <linux/of_gpio.h> >>>> +#include <linux/platform_device.h> >>>> +#include <linux/slab.h> >>>> +#include <linux/workqueue.h> >>>> + >>>> +#define USB_GPIO_DEBOUNCE_MS 20 /* ms */ >>>> + >>>> +struct usb_extcon_info { >>>> + struct device *dev; >>>> + struct extcon_dev *edev; >>>> + >>>> + struct gpio_desc *id_gpiod; >>>> + int id_irq; >>>> + bool id_irqwake; /* ID wakeup enabled flag */ >>> >>> Do you really think id_irqwake is necessary? >>> I think it is not necessary. >> >> I will explain below why it is necessary. >> >>> >>>> + >>>> + unsigned long debounce_jiffies; >>>> + struct delayed_work wq_detcable; >>>> +}; >>>> + >>>> +/* List of detectable cables */ >>>> +enum { >>>> + EXTCON_CABLE_USB = 0, >>>> + EXTCON_CABLE_USB_HOST, >>>> + >>>> + EXTCON_CABLE_END, >>>> +}; >>>> + >>>> +static const char *usb_extcon_cable[] = { >>>> + [EXTCON_CABLE_USB] = "USB", >>>> + [EXTCON_CABLE_USB_HOST] = "USB-Host", >>>> + NULL, >>>> +}; >>>> + >>> >>> [snip] >>> >>>> + >>>> +static int usb_extcon_remove(struct platform_device *pdev) >>>> +{ >>>> + struct usb_extcon_info *info = platform_get_drvdata(pdev); >>>> + >>>> + cancel_delayed_work_sync(&info->wq_detcable); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +#ifdef CONFIG_PM_SLEEP >>>> +static int usb_extcon_suspend(struct device *dev) >>>> +{ >>>> + struct usb_extcon_info *info = dev_get_drvdata(dev); >>>> + >>>> + if (device_may_wakeup(dev)) >>>> + if (!enable_irq_wake(info->id_irq)) >>>> + info->id_irqwake = true; >>>> + >>> >>> You can simplify this code as following without 'id_irqwake': >>> >>> if (device_may_wakeup(dev)) >>> enable_irq_wake(info->id_irq); >> >> enable_irq_wake() can fail. And if it does we need to keep track of it >> to prevent unbalanced disable_irq_wake() call in resume(). >> That's the reason I've added the id_irqwake flag in struct usb_extcon_info. > > It is not proper solution using id_irqwake. If fail to execute enable_irq_wake(), > usb_extcon_suspend() function have to return error immediately. > Your point is valid. I will now need to investigate why enable_irq_wake() is failing when used for GPIO of PCA857x on DRA7-evm. cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
+Thomas (for irq/dummychip.c question) Hi, On 30/01/15 13:09, Roger Quadros wrote: > Chanwoo, > > On 30/01/15 02:06, Chanwoo Choi wrote: >> Hi Roger, >> >> On 01/29/2015 08:26 PM, Roger Quadros wrote: >>> Chanwoo, >>> >>> On 29/01/15 03:49, Chanwoo Choi wrote: >>>> Hi Roger, >>>> >>>> We need to discuss one point about 'id_irqwake'. >>>> I don't recommend to use 'id_irqwake' field. >>>> >>>> And I catch build warning by using "select" keywork in Kconfig. >>>> It is my wrong guide of "select" keyword. So, I'll change it >>>> as 'depends on' keyword. >>>> >>>> Looks good to me except for 'id_irqwake'. >>>> I'll apply this patch on 3.21 queue after completing this discussion. >>>> >>>> On 01/28/2015 09:15 PM, Roger Quadros wrote: >>>>> This driver observes the USB ID pin connected over a GPIO and >>>>> updates the USB cable extcon states accordingly. >>>>> >>>>> The existing GPIO extcon driver is not suitable for this purpose >>>>> as it needs to be taught to understand USB cable states and it >>>>> can't handle more than one cable per instance. >>>>> >>>>> For the USB case we need to handle 2 cable states. >>>>> 1) USB (attach/detach) >>>>> 2) USB-Host (attach/detach) >>>>> >>>>> This driver can be easily updated in the future to handle VBUS >>>>> events in case it happens to be available on GPIO for any platform. >>>>> >>>>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>>>> --- >>>>> v3: >>>>> - removed IRQF_NO_SUSPEND flag. Added IRQF_TRIGGER_RISING and >>>>> IRQF_TRIGGER_FALLING >>>>> - Added disable_irq() to suspend() and enable_irq() to resume() >>>>> >>>>> .../devicetree/bindings/extcon/extcon-usb-gpio.txt | 18 ++ >>>>> drivers/extcon/Kconfig | 7 + >>>>> drivers/extcon/Makefile | 1 + >>>>> drivers/extcon/extcon-usb-gpio.c | 233 +++++++++++++++++++++ >>>>> 4 files changed, 259 insertions(+) >>>>> create mode 100644 Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >>>>> create mode 100644 drivers/extcon/extcon-usb-gpio.c >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >>>>> new file mode 100644 >>>>> index 0000000..85fe6b0 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >>>>> @@ -0,0 +1,18 @@ >>>>> +USB GPIO Extcon device >>>>> + >>>>> +This is a virtual device used to generate USB cable states from the USB ID pin >>>>> +connected to a GPIO pin. >>>>> + >>>>> +Required properties: >>>>> +- compatible: Should be "linux,extcon-usb-gpio" >>>>> +- id-gpio: gpio for USB ID pin. See gpio binding. >>>>> + >>>>> +Example: >>>>> + extcon_usb1 { >>>>> + compatible = "linux,extcon-usb-gpio"; >>>>> + id-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>; >>>>> + } >>>>> + >>>>> + &omap_dwc3_1 { >>>>> + extcon = <&extcon_usb1>; >>>>> + }; >>>>> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >>>>> index 6a1f7de..fd11536 100644 >>>>> --- a/drivers/extcon/Kconfig >>>>> +++ b/drivers/extcon/Kconfig >>>>> @@ -93,4 +93,11 @@ config EXTCON_SM5502 >>>>> Silicon Mitus SM5502. The SM5502 is a USB port accessory >>>>> detector and switch. >>>>> >>>>> +config EXTCON_USB_GPIO >>>>> + tristate "USB GPIO extcon support" >>>>> + select GPIOLIB >>>> >>>> I catch the build warning if using 'select' instead of 'depends on' as following: >>>> It is my wrong guide to you. So, I'll modify it by using "depends on" as your original patch. >>> >>> OK. Thanks. >>> >>>> >>>> make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- zImage -j 8 >>>> scripts/kconfig/conf --silentoldconfig Kconfig >>>> drivers/gpio/Kconfig:34:error: recursive dependency detected! >>>> drivers/gpio/Kconfig:34: symbol GPIOLIB is selected by EXTCON_USB_GPIO >>>> drivers/extcon/Kconfig:96: symbol EXTCON_USB_GPIO depends on EXTCON >>>> drivers/extcon/Kconfig:1: symbol EXTCON is selected by CHARGER_MANAGER >>>> drivers/power/Kconfig:316: symbol CHARGER_MANAGER depends on POWER_SUPPLY >>>> drivers/power/Kconfig:1: symbol POWER_SUPPLY is selected by HID_SONY >>>> drivers/hid/Kconfig:670: symbol HID_SONY depends on NEW_LEDS >>>> drivers/leds/Kconfig:8: symbol NEW_LEDS is selected by BCMA_DRIVER_GPIO >>>> drivers/bcma/Kconfig:75: symbol BCMA_DRIVER_GPIO depends on GPIOLIB >>>> >>>>> + help >>>>> + Say Y here to enable GPIO based USB cable detection extcon support. >>>>> + Used typically if GPIO is used for USB ID pin detection. >>>>> + >>>>> endif # MULTISTATE_SWITCH >>>>> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile >>>>> index 0370b42..6a08a98 100644 >>>>> --- a/drivers/extcon/Makefile >>>>> +++ b/drivers/extcon/Makefile >>>>> @@ -12,3 +12,4 @@ obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o >>>>> obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o >>>>> obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o >>>>> obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o >>>>> +obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o >>>>> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c >>>>> new file mode 100644 >>>>> index 0000000..99a58b2 >>>>> --- /dev/null >>>>> +++ b/drivers/extcon/extcon-usb-gpio.c >>>>> @@ -0,0 +1,233 @@ >>>>> +/** >>>>> + * drivers/extcon/extcon-usb-gpio.c - USB GPIO extcon driver >>>>> + * >>>>> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com >>>>> + * Author: Roger Quadros <rogerq@ti.com> >>>>> + * >>>>> + * This program is free software; you can redistribute it and/or modify >>>>> + * it under the terms of the GNU General Public License version 2 as >>>>> + * published by the Free Software Foundation. >>>>> + * >>>>> + * This program is distributed in the hope that it will be useful, >>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>>> + * GNU General Public License for more details. >>>>> + */ >>>>> + >>>>> +#include <linux/extcon.h> >>>>> +#include <linux/init.h> >>>>> +#include <linux/interrupt.h> >>>>> +#include <linux/irq.h> >>>>> +#include <linux/kernel.h> >>>>> +#include <linux/module.h> >>>>> +#include <linux/of_gpio.h> >>>>> +#include <linux/platform_device.h> >>>>> +#include <linux/slab.h> >>>>> +#include <linux/workqueue.h> >>>>> + >>>>> +#define USB_GPIO_DEBOUNCE_MS 20 /* ms */ >>>>> + >>>>> +struct usb_extcon_info { >>>>> + struct device *dev; >>>>> + struct extcon_dev *edev; >>>>> + >>>>> + struct gpio_desc *id_gpiod; >>>>> + int id_irq; >>>>> + bool id_irqwake; /* ID wakeup enabled flag */ >>>> >>>> Do you really think id_irqwake is necessary? >>>> I think it is not necessary. >>> >>> I will explain below why it is necessary. >>> >>>> >>>>> + >>>>> + unsigned long debounce_jiffies; >>>>> + struct delayed_work wq_detcable; >>>>> +}; >>>>> + >>>>> +/* List of detectable cables */ >>>>> +enum { >>>>> + EXTCON_CABLE_USB = 0, >>>>> + EXTCON_CABLE_USB_HOST, >>>>> + >>>>> + EXTCON_CABLE_END, >>>>> +}; >>>>> + >>>>> +static const char *usb_extcon_cable[] = { >>>>> + [EXTCON_CABLE_USB] = "USB", >>>>> + [EXTCON_CABLE_USB_HOST] = "USB-Host", >>>>> + NULL, >>>>> +}; >>>>> + >>>> >>>> [snip] >>>> >>>>> + >>>>> +static int usb_extcon_remove(struct platform_device *pdev) >>>>> +{ >>>>> + struct usb_extcon_info *info = platform_get_drvdata(pdev); >>>>> + >>>>> + cancel_delayed_work_sync(&info->wq_detcable); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +#ifdef CONFIG_PM_SLEEP >>>>> +static int usb_extcon_suspend(struct device *dev) >>>>> +{ >>>>> + struct usb_extcon_info *info = dev_get_drvdata(dev); >>>>> + >>>>> + if (device_may_wakeup(dev)) >>>>> + if (!enable_irq_wake(info->id_irq)) >>>>> + info->id_irqwake = true; >>>>> + >>>> >>>> You can simplify this code as following without 'id_irqwake': >>>> >>>> if (device_may_wakeup(dev)) >>>> enable_irq_wake(info->id_irq); >>> >>> enable_irq_wake() can fail. And if it does we need to keep track of it >>> to prevent unbalanced disable_irq_wake() call in resume(). >>> That's the reason I've added the id_irqwake flag in struct usb_extcon_info. >> >> It is not proper solution using id_irqwake. If fail to execute enable_irq_wake(), >> usb_extcon_suspend() function have to return error immediately. >> > > Your point is valid. I will now need to investigate why enable_irq_wake() is failing > when used for GPIO of PCA857x on DRA7-evm. > The reason of enable_irq_wake() failure with PCA857x is that it uses dummy_irq_chip which doesn't have irq_set_wake() or IRQCHIP_SKIP_SET_WAKE flag set. I can send out another patch to add IRQCHIP_SKIP_SET_WAKE to dummy_irq_chip. Thomas, is this a reasonable option? The problem at hand is that enable_irq_wake() on a IRQ line based off PCA857x GPIO chip fails. PCA857x uses dummy_irq_chip to implement the IRQ feature. http://lxr.free-electrons.com/source/drivers/gpio/gpio-pcf857x.c#L227 cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30/01/15 02:11, Chanwoo Choi wrote: > Hi Roger, > > On 01/28/2015 09:15 PM, Roger Quadros wrote: >> This driver observes the USB ID pin connected over a GPIO and >> updates the USB cable extcon states accordingly. >> >> The existing GPIO extcon driver is not suitable for this purpose >> as it needs to be taught to understand USB cable states and it >> can't handle more than one cable per instance. >> >> For the USB case we need to handle 2 cable states. >> 1) USB (attach/detach) >> 2) USB-Host (attach/detach) >> >> This driver can be easily updated in the future to handle VBUS >> events in case it happens to be available on GPIO for any platform. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> v3: >> - removed IRQF_NO_SUSPEND flag. Added IRQF_TRIGGER_RISING and >> IRQF_TRIGGER_FALLING >> - Added disable_irq() to suspend() and enable_irq() to resume() >> >> .../devicetree/bindings/extcon/extcon-usb-gpio.txt | 18 ++ >> drivers/extcon/Kconfig | 7 + >> drivers/extcon/Makefile | 1 + >> drivers/extcon/extcon-usb-gpio.c | 233 +++++++++++++++++++++ >> 4 files changed, 259 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >> create mode 100644 drivers/extcon/extcon-usb-gpio.c >> >> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >> new file mode 100644 >> index 0000000..85fe6b0 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >> @@ -0,0 +1,18 @@ >> +USB GPIO Extcon device >> + >> +This is a virtual device used to generate USB cable states from the USB ID pin >> +connected to a GPIO pin. >> + >> +Required properties: >> +- compatible: Should be "linux,extcon-usb-gpio" >> +- id-gpio: gpio for USB ID pin. See gpio binding. >> + >> +Example: >> + extcon_usb1 { >> + compatible = "linux,extcon-usb-gpio"; >> + id-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>; >> + } >> + >> + &omap_dwc3_1 { >> + extcon = <&extcon_usb1>; >> + }; >> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >> index 6a1f7de..fd11536 100644 >> --- a/drivers/extcon/Kconfig >> +++ b/drivers/extcon/Kconfig >> @@ -93,4 +93,11 @@ config EXTCON_SM5502 >> Silicon Mitus SM5502. The SM5502 is a USB port accessory >> detector and switch. >> >> +config EXTCON_USB_GPIO >> + tristate "USB GPIO extcon support" >> + select GPIOLIB >> + help >> + Say Y here to enable GPIO based USB cable detection extcon support. >> + Used typically if GPIO is used for USB ID pin detection. >> + >> endif # MULTISTATE_SWITCH >> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile >> index 0370b42..6a08a98 100644 >> --- a/drivers/extcon/Makefile >> +++ b/drivers/extcon/Makefile >> @@ -12,3 +12,4 @@ obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o >> obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o >> obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o >> obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o >> +obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o >> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c >> new file mode 100644 >> index 0000000..99a58b2 >> --- /dev/null >> +++ b/drivers/extcon/extcon-usb-gpio.c >> @@ -0,0 +1,233 @@ >> +/** >> + * drivers/extcon/extcon-usb-gpio.c - USB GPIO extcon driver >> + * >> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com >> + * Author: Roger Quadros <rogerq@ti.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> + >> +#include <linux/extcon.h> >> +#include <linux/init.h> >> +#include <linux/interrupt.h> >> +#include <linux/irq.h> >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/of_gpio.h> >> +#include <linux/platform_device.h> >> +#include <linux/slab.h> >> +#include <linux/workqueue.h> >> + >> +#define USB_GPIO_DEBOUNCE_MS 20 /* ms */ >> + >> +struct usb_extcon_info { >> + struct device *dev; >> + struct extcon_dev *edev; >> + >> + struct gpio_desc *id_gpiod; >> + int id_irq; >> + bool id_irqwake; /* ID wakeup enabled flag */ >> + >> + unsigned long debounce_jiffies; >> + struct delayed_work wq_detcable; >> +}; >> + >> +/* List of detectable cables */ >> +enum { >> + EXTCON_CABLE_USB = 0, >> + EXTCON_CABLE_USB_HOST, >> + >> + EXTCON_CABLE_END, >> +}; >> + >> +static const char *usb_extcon_cable[] = { >> + [EXTCON_CABLE_USB] = "USB", >> + [EXTCON_CABLE_USB_HOST] = "USB-Host", > > I'll use the defined name for extcon cable name as soon because > it has potential isseu about the conflict of extcon cable name between subsystems. > So, I recommend to use a captical letter as "USB-HOST" instead of "USB-Host". > If other extcon driver don't use the captical letter, I'll fix it. Did you see patch 2 in this series? In that I had fixed all instances to use "USB-Host" based on http://lxr.free-electrons.com/source/drivers/extcon/extcon-class.c#L45 What do you suggest? Skip patch 2 and convert all "USB-Host" to "USB-HOST"? cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Roger, On 01/30/2015 11:03 PM, Roger Quadros wrote: > On 30/01/15 02:11, Chanwoo Choi wrote: >> Hi Roger, >> >> On 01/28/2015 09:15 PM, Roger Quadros wrote: >>> This driver observes the USB ID pin connected over a GPIO and >>> updates the USB cable extcon states accordingly. >>> >>> The existing GPIO extcon driver is not suitable for this purpose >>> as it needs to be taught to understand USB cable states and it >>> can't handle more than one cable per instance. >>> >>> For the USB case we need to handle 2 cable states. >>> 1) USB (attach/detach) >>> 2) USB-Host (attach/detach) >>> >>> This driver can be easily updated in the future to handle VBUS >>> events in case it happens to be available on GPIO for any platform. >>> >>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>> --- >>> v3: >>> - removed IRQF_NO_SUSPEND flag. Added IRQF_TRIGGER_RISING and >>> IRQF_TRIGGER_FALLING >>> - Added disable_irq() to suspend() and enable_irq() to resume() >>> >>> .../devicetree/bindings/extcon/extcon-usb-gpio.txt | 18 ++ >>> drivers/extcon/Kconfig | 7 + >>> drivers/extcon/Makefile | 1 + >>> drivers/extcon/extcon-usb-gpio.c | 233 +++++++++++++++++++++ >>> 4 files changed, 259 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >>> create mode 100644 drivers/extcon/extcon-usb-gpio.c >>> >>> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >>> new file mode 100644 >>> index 0000000..85fe6b0 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt >>> @@ -0,0 +1,18 @@ >>> +USB GPIO Extcon device >>> + >>> +This is a virtual device used to generate USB cable states from the USB ID pin >>> +connected to a GPIO pin. >>> + >>> +Required properties: >>> +- compatible: Should be "linux,extcon-usb-gpio" >>> +- id-gpio: gpio for USB ID pin. See gpio binding. >>> + >>> +Example: >>> + extcon_usb1 { >>> + compatible = "linux,extcon-usb-gpio"; >>> + id-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>; >>> + } >>> + >>> + &omap_dwc3_1 { >>> + extcon = <&extcon_usb1>; >>> + }; >>> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >>> index 6a1f7de..fd11536 100644 >>> --- a/drivers/extcon/Kconfig >>> +++ b/drivers/extcon/Kconfig >>> @@ -93,4 +93,11 @@ config EXTCON_SM5502 >>> Silicon Mitus SM5502. The SM5502 is a USB port accessory >>> detector and switch. >>> >>> +config EXTCON_USB_GPIO >>> + tristate "USB GPIO extcon support" >>> + select GPIOLIB >>> + help >>> + Say Y here to enable GPIO based USB cable detection extcon support. >>> + Used typically if GPIO is used for USB ID pin detection. >>> + >>> endif # MULTISTATE_SWITCH >>> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile >>> index 0370b42..6a08a98 100644 >>> --- a/drivers/extcon/Makefile >>> +++ b/drivers/extcon/Makefile >>> @@ -12,3 +12,4 @@ obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o >>> obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o >>> obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o >>> obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o >>> +obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o >>> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c >>> new file mode 100644 >>> index 0000000..99a58b2 >>> --- /dev/null >>> +++ b/drivers/extcon/extcon-usb-gpio.c >>> @@ -0,0 +1,233 @@ >>> +/** >>> + * drivers/extcon/extcon-usb-gpio.c - USB GPIO extcon driver >>> + * >>> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com >>> + * Author: Roger Quadros <rogerq@ti.com> >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License version 2 as >>> + * published by the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> + * GNU General Public License for more details. >>> + */ >>> + >>> +#include <linux/extcon.h> >>> +#include <linux/init.h> >>> +#include <linux/interrupt.h> >>> +#include <linux/irq.h> >>> +#include <linux/kernel.h> >>> +#include <linux/module.h> >>> +#include <linux/of_gpio.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/slab.h> >>> +#include <linux/workqueue.h> >>> + >>> +#define USB_GPIO_DEBOUNCE_MS 20 /* ms */ >>> + >>> +struct usb_extcon_info { >>> + struct device *dev; >>> + struct extcon_dev *edev; >>> + >>> + struct gpio_desc *id_gpiod; >>> + int id_irq; >>> + bool id_irqwake; /* ID wakeup enabled flag */ >>> + >>> + unsigned long debounce_jiffies; >>> + struct delayed_work wq_detcable; >>> +}; >>> + >>> +/* List of detectable cables */ >>> +enum { >>> + EXTCON_CABLE_USB = 0, >>> + EXTCON_CABLE_USB_HOST, >>> + >>> + EXTCON_CABLE_END, >>> +}; >>> + >>> +static const char *usb_extcon_cable[] = { >>> + [EXTCON_CABLE_USB] = "USB", >>> + [EXTCON_CABLE_USB_HOST] = "USB-Host", >> >> I'll use the defined name for extcon cable name as soon because >> it has potential isseu about the conflict of extcon cable name between subsystems. >> So, I recommend to use a captical letter as "USB-HOST" instead of "USB-Host". >> If other extcon driver don't use the captical letter, I'll fix it. > > Did you see patch 2 in this series? > In that I had fixed all instances to use "USB-Host" based on > http://lxr.free-electrons.com/source/drivers/extcon/extcon-class.c#L45 > > What do you suggest? > Skip patch 2 and convert all "USB-Host" to "USB-HOST"? I replied my opinion about this on following patch[1]. [1] Re: [PATCH v2 2/7] usb: extcon: Fix USB-Host cable name Thanks, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt new file mode 100644 index 0000000..85fe6b0 --- /dev/null +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt @@ -0,0 +1,18 @@ +USB GPIO Extcon device + +This is a virtual device used to generate USB cable states from the USB ID pin +connected to a GPIO pin. + +Required properties: +- compatible: Should be "linux,extcon-usb-gpio" +- id-gpio: gpio for USB ID pin. See gpio binding. + +Example: + extcon_usb1 { + compatible = "linux,extcon-usb-gpio"; + id-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>; + } + + &omap_dwc3_1 { + extcon = <&extcon_usb1>; + }; diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig index 6a1f7de..fd11536 100644 --- a/drivers/extcon/Kconfig +++ b/drivers/extcon/Kconfig @@ -93,4 +93,11 @@ config EXTCON_SM5502 Silicon Mitus SM5502. The SM5502 is a USB port accessory detector and switch. +config EXTCON_USB_GPIO + tristate "USB GPIO extcon support" + select GPIOLIB + help + Say Y here to enable GPIO based USB cable detection extcon support. + Used typically if GPIO is used for USB ID pin detection. + endif # MULTISTATE_SWITCH diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile index 0370b42..6a08a98 100644 --- a/drivers/extcon/Makefile +++ b/drivers/extcon/Makefile @@ -12,3 +12,4 @@ obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o +obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c new file mode 100644 index 0000000..99a58b2 --- /dev/null +++ b/drivers/extcon/extcon-usb-gpio.c @@ -0,0 +1,233 @@ +/** + * drivers/extcon/extcon-usb-gpio.c - USB GPIO extcon driver + * + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com + * Author: Roger Quadros <rogerq@ti.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <linux/extcon.h> +#include <linux/init.h> +#include <linux/interrupt.h> +#include <linux/irq.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_gpio.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/workqueue.h> + +#define USB_GPIO_DEBOUNCE_MS 20 /* ms */ + +struct usb_extcon_info { + struct device *dev; + struct extcon_dev *edev; + + struct gpio_desc *id_gpiod; + int id_irq; + bool id_irqwake; /* ID wakeup enabled flag */ + + unsigned long debounce_jiffies; + struct delayed_work wq_detcable; +}; + +/* List of detectable cables */ +enum { + EXTCON_CABLE_USB = 0, + EXTCON_CABLE_USB_HOST, + + EXTCON_CABLE_END, +}; + +static const char *usb_extcon_cable[] = { + [EXTCON_CABLE_USB] = "USB", + [EXTCON_CABLE_USB_HOST] = "USB-Host", + NULL, +}; + +static void usb_extcon_detect_cable(struct work_struct *work) +{ + int id; + struct usb_extcon_info *info = container_of(to_delayed_work(work), + struct usb_extcon_info, + wq_detcable); + + /* check ID and update cable state */ + id = gpiod_get_value_cansleep(info->id_gpiod); + if (id) { + /* + * ID = 1 means USB HOST cable detached. + * As we don't have event for USB peripheral cable attached, + * we simulate USB peripheral attach here. + */ + extcon_set_cable_state(info->edev, + usb_extcon_cable[EXTCON_CABLE_USB_HOST], + false); + extcon_set_cable_state(info->edev, + usb_extcon_cable[EXTCON_CABLE_USB], + true); + } else { + /* + * ID = 0 means USB HOST cable attached. + * As we don't have event for USB peripheral cable detached, + * we simulate USB peripheral detach here. + */ + extcon_set_cable_state(info->edev, + usb_extcon_cable[EXTCON_CABLE_USB], + false); + extcon_set_cable_state(info->edev, + usb_extcon_cable[EXTCON_CABLE_USB_HOST], + true); + } +} + +static irqreturn_t usb_irq_handler(int irq, void *dev_id) +{ + struct usb_extcon_info *info = dev_id; + + queue_delayed_work(system_power_efficient_wq, &info->wq_detcable, + info->debounce_jiffies); + + return IRQ_HANDLED; +} + +static int usb_extcon_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; + struct usb_extcon_info *info; + int ret; + + if (!np) + return -EINVAL; + + info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL); + if (!info) + return -ENOMEM; + + info->dev = dev; + info->id_gpiod = devm_gpiod_get(&pdev->dev, "id"); + if (IS_ERR(info->id_gpiod)) { + dev_err(dev, "failed to get ID GPIO\n"); + return PTR_ERR(info->id_gpiod); + } + + ret = gpiod_set_debounce(info->id_gpiod, + USB_GPIO_DEBOUNCE_MS * 1000); + if (ret < 0) + info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEBOUNCE_MS); + + INIT_DELAYED_WORK(&info->wq_detcable, usb_extcon_detect_cable); + + info->id_irq = gpiod_to_irq(info->id_gpiod); + if (info->id_irq < 0) { + dev_err(dev, "failed to get ID IRQ\n"); + return info->id_irq; + } + + ret = devm_request_threaded_irq(dev, info->id_irq, NULL, + usb_irq_handler, + IRQF_TRIGGER_RISING | + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, + pdev->name, info); + if (ret < 0) { + dev_err(dev, "failed to request handler for ID IRQ\n"); + return ret; + } + + info->edev = devm_extcon_dev_allocate(dev, usb_extcon_cable); + if (IS_ERR(info->edev)) { + dev_err(dev, "failed to allocate extcon device\n"); + return -ENOMEM; + } + + ret = devm_extcon_dev_register(dev, info->edev); + if (ret < 0) { + dev_err(dev, "failed to register extcon device\n"); + return ret; + } + + platform_set_drvdata(pdev, info); + device_init_wakeup(dev, 1); + + /* Perform initial detection */ + usb_extcon_detect_cable(&info->wq_detcable.work); + + return 0; +} + +static int usb_extcon_remove(struct platform_device *pdev) +{ + struct usb_extcon_info *info = platform_get_drvdata(pdev); + + cancel_delayed_work_sync(&info->wq_detcable); + + return 0; +} + +#ifdef CONFIG_PM_SLEEP +static int usb_extcon_suspend(struct device *dev) +{ + struct usb_extcon_info *info = dev_get_drvdata(dev); + + if (device_may_wakeup(dev)) + if (!enable_irq_wake(info->id_irq)) + info->id_irqwake = true; + + /* + * We don't want to process any IRQs after this point + * as GPIOs used behind I2C subsystem might not be + * accessible until resume completes. So disable IRQ. + */ + disable_irq(info->id_irq); + + return 0; +} + +static int usb_extcon_resume(struct device *dev) +{ + struct usb_extcon_info *info = dev_get_drvdata(dev); + + if (info->id_irqwake) { + disable_irq_wake(info->id_irq); + info->id_irqwake = false; + } + + enable_irq(info->id_irq); + + return 0; +} +#endif + +static SIMPLE_DEV_PM_OPS(usb_extcon_pm_ops, + usb_extcon_suspend, usb_extcon_resume); + +static struct of_device_id usb_extcon_dt_match[] = { + { .compatible = "linux,extcon-usb-gpio", }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, usb_extcon_dt_match); + +static struct platform_driver usb_extcon_driver = { + .probe = usb_extcon_probe, + .remove = usb_extcon_remove, + .driver = { + .name = "extcon-usb-gpio", + .pm = &usb_extcon_pm_ops, + .of_match_table = usb_extcon_dt_match, + }, +}; + +module_platform_driver(usb_extcon_driver); + +MODULE_AUTHOR("Roger Quadros <rogerq@ti.com>"); +MODULE_DESCRIPTION("USB GPIO extcon driver"); +MODULE_LICENSE("GPL v2");
This driver observes the USB ID pin connected over a GPIO and updates the USB cable extcon states accordingly. The existing GPIO extcon driver is not suitable for this purpose as it needs to be taught to understand USB cable states and it can't handle more than one cable per instance. For the USB case we need to handle 2 cable states. 1) USB (attach/detach) 2) USB-Host (attach/detach) This driver can be easily updated in the future to handle VBUS events in case it happens to be available on GPIO for any platform. Signed-off-by: Roger Quadros <rogerq@ti.com> --- v3: - removed IRQF_NO_SUSPEND flag. Added IRQF_TRIGGER_RISING and IRQF_TRIGGER_FALLING - Added disable_irq() to suspend() and enable_irq() to resume() .../devicetree/bindings/extcon/extcon-usb-gpio.txt | 18 ++ drivers/extcon/Kconfig | 7 + drivers/extcon/Makefile | 1 + drivers/extcon/extcon-usb-gpio.c | 233 +++++++++++++++++++++ 4 files changed, 259 insertions(+) create mode 100644 Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt create mode 100644 drivers/extcon/extcon-usb-gpio.c