[7/9] usb: dwc2/gadget: use soft disconnect mode for implementing pullup control

Message ID 1413464285-24172-8-git-send-email-m.szyprowski@samsung.com
State New
Headers show

Commit Message

Marek Szyprowski Oct. 16, 2014, 12:58 p.m.
This patch moves PHY enable and disable calls from pullup method to
udc_start/stop functions and adds calls to recently introduces soft
disconnect mode in pullup method. This improves dwc2 gadget driver
compatibility with gadget API requirements (now pullup method really
forces soft disconnect mode instead of shutting down the whole hardware)
and as a side effect also solves the issue related to limited caller
context for PHY related functions (they cannot be called from
non-sleeping context).

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/usb/dwc2/gadget.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Comments

Felipe Balbi Oct. 16, 2014, 1:41 p.m. | #1
Hi,

your subject line is wrong. ->pullup() is already implemented, you're
moving unnecessary code from ->pullup() to other places.

On Thu, Oct 16, 2014 at 02:58:03PM +0200, Marek Szyprowski wrote:
> This patch moves PHY enable and disable calls from pullup method to
> udc_start/stop functions and adds calls to recently introduces soft
> disconnect mode in pullup method. This improves dwc2 gadget driver
> compatibility with gadget API requirements (now pullup method really
> forces soft disconnect mode instead of shutting down the whole hardware)
> and as a side effect also solves the issue related to limited caller
> context for PHY related functions (they cannot be called from
> non-sleeping context).

you're doing two things in one patch. See below

> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/usb/dwc2/gadget.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index d039334967d7..cdf417a7ae63 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -2883,6 +2883,7 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
>  			   struct usb_gadget_driver *driver)
>  {
>  	struct s3c_hsotg *hsotg = to_hsotg(gadget);
> +	unsigned long flags;
>  	int ret;
>  
>  	if (!hsotg) {
> @@ -2919,7 +2920,15 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
>  		goto err;
>  	}
>  
> +	s3c_hsotg_phy_enable(hsotg);

you moved phy_enable to udc_start/udc_stop (one patch)

> +
> +	spin_lock_irqsave(&hsotg->lock, flags);
> +	s3c_hsotg_init(hsotg);
> +	s3c_hsotg_core_init_disconnected(hsotg);

you moved core init here (another patch).

> +	spin_unlock_irqrestore(&hsotg->lock, flags);
> +
>  	dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name);
> +
>  	return 0;
>  
>  err:
> @@ -2957,6 +2966,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,
>  
>  	spin_unlock_irqrestore(&hsotg->lock, flags);
>  
> +	s3c_hsotg_phy_disable(hsotg);
> +
>  	regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
>  
>  	clk_disable(hsotg->clk);
> @@ -2990,14 +3001,13 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)
>  	dev_dbg(hsotg->dev, "%s: is_on: %d\n", __func__, is_on);
>  
>  	spin_lock_irqsave(&hsotg->lock, flags);
> +
>  	if (is_on) {
> -		s3c_hsotg_phy_enable(hsotg);
>  		clk_enable(hsotg->clk);
> -		s3c_hsotg_core_init_disconnected(hsotg);
>  		s3c_hsotg_core_connect(hsotg);
>  	} else {
> +		s3c_hsotg_core_disconnect(hsotg);
>  		clk_disable(hsotg->clk);
> -		s3c_hsotg_phy_disable(hsotg);
>  	}
>  
>  	hsotg->gadget.speed = USB_SPEED_UNKNOWN;
> -- 
> 1.9.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index d039334967d7..cdf417a7ae63 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2883,6 +2883,7 @@  static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
 			   struct usb_gadget_driver *driver)
 {
 	struct s3c_hsotg *hsotg = to_hsotg(gadget);
+	unsigned long flags;
 	int ret;
 
 	if (!hsotg) {
@@ -2919,7 +2920,15 @@  static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
 		goto err;
 	}
 
+	s3c_hsotg_phy_enable(hsotg);
+
+	spin_lock_irqsave(&hsotg->lock, flags);
+	s3c_hsotg_init(hsotg);
+	s3c_hsotg_core_init_disconnected(hsotg);
+	spin_unlock_irqrestore(&hsotg->lock, flags);
+
 	dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name);
+
 	return 0;
 
 err:
@@ -2957,6 +2966,8 @@  static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,
 
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 
+	s3c_hsotg_phy_disable(hsotg);
+
 	regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
 
 	clk_disable(hsotg->clk);
@@ -2990,14 +3001,13 @@  static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)
 	dev_dbg(hsotg->dev, "%s: is_on: %d\n", __func__, is_on);
 
 	spin_lock_irqsave(&hsotg->lock, flags);
+
 	if (is_on) {
-		s3c_hsotg_phy_enable(hsotg);
 		clk_enable(hsotg->clk);
-		s3c_hsotg_core_init_disconnected(hsotg);
 		s3c_hsotg_core_connect(hsotg);
 	} else {
+		s3c_hsotg_core_disconnect(hsotg);
 		clk_disable(hsotg->clk);
-		s3c_hsotg_phy_disable(hsotg);
 	}
 
 	hsotg->gadget.speed = USB_SPEED_UNKNOWN;