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

[Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3062 bytes --]

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-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]