Re: [RFC][PATCH 1/3 v2] usb: dwc2: Add extcon support to dwc2 driver

From: Chanwoo Choi
Date: Tue Dec 06 2016 - 22:55:26 EST


Hi John,

I give a some guide for extcon API.
This patch uses the deprecated extcon API (extcon_get_cable_state_).
So, I recommend that you better to use following extcon API:
- extcon_get_cable_state_() -> extcon_get_state()
- extcon_register_notifier() -> devm_extcon_register_notifier()

Best Regards,
Chanwoo Choi

On 2016ë 12ì 06ì 17:06, John Stultz wrote:
> This patch wires up extcon support to the dwc2 driver
> so that devices that use a modern generic phy driver
> and don't use the usb-phy infrastructure can still
> signal connect/disconnect events.
>
> Cc: Wei Xu <xuwei5@xxxxxxxxxxxxx>
> Cc: Guodong Xu <guodong.xu@xxxxxxxxxx>
> Cc: Amit Pundir <amit.pundir@xxxxxxxxxx>
> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> Cc: John Youn <johnyoun@xxxxxxxxxxxx>
> Cc: Douglas Anderson <dianders@xxxxxxxxxxxx>
> Cc: Chen Yu <chenyu56@xxxxxxxxxx>
> Cc: Kishon Vijay Abraham I <kishon@xxxxxx>
> Cc: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: linux-usb@xxxxxxxxxxxxxxx
> Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx>
> ---
> v2:
> * Move extcon logic from generic phy to dwc2 driver, as
> suggested by Kishon.
> ---
> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 11 +++++++++
> drivers/usb/dwc2/core.h | 16 ++++++++++++
> drivers/usb/dwc2/core_intr.c | 25 +++++++++++++++++++
> drivers/usb/dwc2/hcd.c | 23 +++++++++++++++++
> drivers/usb/dwc2/platform.c | 41 +++++++++++++++++++++++++++++++
> 5 files changed, 116 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> index 17839db..8a86a11 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> @@ -732,6 +732,16 @@
> regulator-always-on;
> };
>
> + usb_vbus: usb-vbus {
> + compatible = "linux,extcon-usb-gpio";
> + id-gpio = <&gpio2 6 1>;
> + };
> +
> + usb_id: usb-id {
> + compatible = "linux,extcon-usb-gpio";
> + id-gpio = <&gpio2 5 1>;
> + };
> +
> usb_phy: usbphy {
> compatible = "hisilicon,hi6220-usb-phy";
> #phy-cells = <0>;
> @@ -745,6 +755,7 @@
> phys = <&usb_phy>;
> phy-names = "usb2-phy";
> clocks = <&sys_ctrl HI6220_USBOTG_HCLK>;
> + extcon = <&usb_vbus>, <&usb_id>;
> clock-names = "otg";
> dr_mode = "otg";
> g-use-dma;
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 2a21a04..4cfce62 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -623,6 +623,13 @@ struct dwc2_hregs_backup {
> bool valid;
> };
>
> +struct dwc2_extcon {
> + struct notifier_block nb;
> + struct extcon_dev *extcon;
> + int state;
> +};
> +
> +
> /*
> * Constants related to high speed periodic scheduling
> *
> @@ -996,6 +1003,10 @@ struct dwc2_hsotg {
> u32 g_np_g_tx_fifo_sz;
> u32 g_tx_fifo_sz[MAX_EPS_CHANNELS];
> #endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */
> +
> + struct dwc2_extcon extcon_vbus;
> + struct dwc2_extcon extcon_id;
> + struct delayed_work extcon_work;
> };
>
> /* Reasons for halting a host channel */
> @@ -1041,6 +1052,11 @@ extern void dwc2_flush_rx_fifo(struct dwc2_hsotg *hsotg);
> extern void dwc2_enable_global_interrupts(struct dwc2_hsotg *hcd);
> extern void dwc2_disable_global_interrupts(struct dwc2_hsotg *hcd);
>
> +extern int dwc2_extcon_vbus_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr);
> +extern int dwc2_extcon_id_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr);
> +
> /* This function should be called on every hardware interrupt. */
> extern irqreturn_t dwc2_handle_common_intr(int irq, void *dev);
>
> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
> index d85c5c9..d4fa938 100644
> --- a/drivers/usb/dwc2/core_intr.c
> +++ b/drivers/usb/dwc2/core_intr.c
> @@ -479,6 +479,31 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg *hsotg)
> }
> }
>
> +
> +
> +int dwc2_extcon_vbus_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct dwc2_extcon *vbus = container_of(nb, struct dwc2_extcon, nb);
> + struct dwc2_hsotg *hsotg = container_of(vbus, struct dwc2_hsotg,
> + extcon_vbus);
> +
> + schedule_delayed_work(&hsotg->extcon_work, msecs_to_jiffies(100));
> + return NOTIFY_DONE;
> +}
> +
> +int dwc2_extcon_id_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct dwc2_extcon *id = container_of(nb, struct dwc2_extcon, nb);
> + struct dwc2_hsotg *hsotg = container_of(id, struct dwc2_hsotg,
> + extcon_id);
> + schedule_delayed_work(&hsotg->extcon_work, msecs_to_jiffies(100));
> +
> + return NOTIFY_DONE;
> +}
> +
> +
> #define GINTMSK_COMMON (GINTSTS_WKUPINT | GINTSTS_SESSREQINT | \
> GINTSTS_CONIDSTSCHNG | GINTSTS_OTGINT | \
> GINTSTS_MODEMIS | GINTSTS_DISCONNINT | \
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index df5a065..61eea70 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -47,6 +47,7 @@
> #include <linux/io.h>
> #include <linux/slab.h>
> #include <linux/usb.h>
> +#include <linux/extcon.h>
>
> #include <linux/usb/hcd.h>
> #include <linux/usb/ch11.h>
> @@ -3246,6 +3247,26 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
> }
> }
>
> +static void dwc2_hcd_extcon_func(struct work_struct *work)
> +{
> + struct dwc2_hsotg *hsotg = container_of(work, struct dwc2_hsotg,
> + extcon_work.work);
> +
> + if (!IS_ERR(hsotg->extcon_vbus.extcon))
> + hsotg->extcon_vbus.state =
> + extcon_get_cable_state_(hsotg->extcon_vbus.extcon,
> + EXTCON_USB);
> + if (!IS_ERR(hsotg->extcon_id.extcon))
> + hsotg->extcon_id.state =
> + extcon_get_cable_state_(hsotg->extcon_id.extcon,
> + EXTCON_USB_HOST);
> +
> + if (hsotg->extcon_id.state)
> + usb_gadget_vbus_connect(&hsotg->gadget);
> + else
> + usb_gadget_vbus_disconnect(&hsotg->gadget);
> +}
> +
> static void dwc2_wakeup_detected(unsigned long data)
> {
> struct dwc2_hsotg *hsotg = (struct dwc2_hsotg *)data;
> @@ -5085,6 +5106,8 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
> /* Initialize port reset work */
> INIT_DELAYED_WORK(&hsotg->reset_work, dwc2_hcd_reset_func);
>
> + INIT_DELAYED_WORK(&hsotg->extcon_work, dwc2_hcd_extcon_func);
> +
> /*
> * Allocate space for storing data on status transactions. Normally no
> * data is sent, but this space acts as a bit bucket. This must be
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index 8e1728b..2e4545f 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -46,6 +46,7 @@
> #include <linux/phy/phy.h>
> #include <linux/platform_data/s3c-hsotg.h>
> #include <linux/reset.h>
> +#include <linux/extcon.h>
>
> #include <linux/usb/of.h>
>
> @@ -543,6 +544,7 @@ static int dwc2_driver_probe(struct platform_device *dev)
> struct dwc2_core_params defparams;
> struct dwc2_hsotg *hsotg;
> struct resource *res;
> + struct extcon_dev *ext_id, *ext_vbus;
> int retval;
>
> match = of_match_device(dwc2_of_match_table, &dev->dev);
> @@ -620,6 +622,45 @@ static int dwc2_driver_probe(struct platform_device *dev)
> if (retval)
> goto error;
>
> + ext_id = ERR_PTR(-ENODEV);
> + ext_vbus = ERR_PTR(-ENODEV);
> + if (of_property_read_bool(dev->dev.of_node, "extcon")) {
> + /* Each one of them is not mandatory */
> + ext_vbus = extcon_get_edev_by_phandle(&dev->dev, 0);
> + if (IS_ERR(ext_vbus) && PTR_ERR(ext_vbus) != -ENODEV)
> + return PTR_ERR(ext_vbus);
> +
> + ext_id = extcon_get_edev_by_phandle(&dev->dev, 1);
> + if (IS_ERR(ext_id) && PTR_ERR(ext_id) != -ENODEV)
> + return PTR_ERR(ext_id);
> + }
> +
> + hsotg->extcon_vbus.extcon = ext_vbus;
> + if (!IS_ERR(ext_vbus)) {
> + hsotg->extcon_vbus.nb.notifier_call = dwc2_extcon_vbus_notifier;
> + retval = extcon_register_notifier(ext_vbus, EXTCON_USB,
> + &hsotg->extcon_vbus.nb);
> + if (retval < 0) {
> + dev_err(&dev->dev, "register VBUS notifier failed\n");
> + return retval;
> + }
> + hsotg->extcon_vbus.state = extcon_get_cable_state_(ext_vbus,
> + EXTCON_USB);
> + }
> +
> + hsotg->extcon_id.extcon = ext_id;
> + if (!IS_ERR(ext_id)) {
> + hsotg->extcon_id.nb.notifier_call = dwc2_extcon_id_notifier;
> + retval = extcon_register_notifier(ext_id, EXTCON_USB_HOST,
> + &hsotg->extcon_id.nb);
> + if (retval < 0) {
> + dev_err(&dev->dev, "register ID notifier failed\n");
> + return retval;
> + }
> + hsotg->extcon_id.state = extcon_get_cable_state_(ext_id,
> + EXTCON_USB_HOST);
> + }
> +
> /*
> * Reset before dwc2_get_hwparams() then it could get power-on real
> * reset value form registers.
>