MIME-version: 1.0
Content-type: text/plain; charset=UTF-8
Content-transfer-encoding: 8BIT
Message-id: <584787FF.6030002@samsung.com>
Date: Wed, 07 Dec 2016 12:54:39 +0900
From: Chanwoo Choi <cw00.choi@samsung.com>
Organization: Samsung Electronics
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101
 Thunderbird/31.6.0
To: John Stultz <john.stultz@linaro.org>,
        lkml <linux-kernel@vger.kernel.org>
Cc: Wei Xu <xuwei5@hisilicon.com>, Guodong Xu <guodong.xu@linaro.org>,
        Amit Pundir <amit.pundir@linaro.org>, Rob Herring <robh+dt@kernel.org>,
        John Youn <johnyoun@synopsys.com>,
        Douglas Anderson <dianders@chromium.org>,
        Chen Yu <chenyu56@huawei.com>, Kishon Vijay Abraham I <kishon@ti.com>,
        Felipe Balbi <felipe.balbi@linux.intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-usb@vger.kernel.org
Subject: Re: [RFC][PATCH 1/3 v2] usb: dwc2: Add extcon support to dwc2 driver
References: <1481011582-7162-1-git-send-email-john.stultz@linaro.org>
 <CGME20161206080833epcas4p1cb6159fa2eb82cddda8266c84b5bb0d5@epcas4p1.samsung.com>
 <1481011582-7162-2-git-send-email-john.stultz@linaro.org>
In-reply-to: <1481011582-7162-2-git-send-email-john.stultz@linaro.org>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 8851
Lines: 262

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@hisilicon.com>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Amit Pundir <amit.pundir@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: John Youn <johnyoun@synopsys.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Chen Yu <chenyu56@huawei.com>
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> 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.
>