[v2,06/10] usb: dwc2/gadget: decouple setting soft-disconnect from s3c_hsotg_core_init

Message ID 1413801940-31086-7-git-send-email-m.szyprowski@samsung.com
State Accepted
Commit ad38dc5dac5d8df80c0c394459f950eed1dccbcd
Headers show

Commit Message

Marek Szyprowski Oct. 20, 2014, 10:45 a.m.
This patch changes s3c_hsotg_core_init function to leave hardware in
soft disconnect mode, so the moment of coupling the hardware to the usb
bus can be later controlled by the separate functions for enabling and
disabling soft disconnect mode. This patch is a preparation to rework
pullup() method.

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

Comments

Paul Zimmerman Oct. 25, 2014, 12:35 a.m. | #1
> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com]
> Sent: Monday, October 20, 2014 3:46 AM
> 
> This patch changes s3c_hsotg_core_init function to leave hardware in
> soft disconnect mode, so the moment of coupling the hardware to the usb
> bus can be later controlled by the separate functions for enabling and
> disabling soft disconnect mode. This patch is a preparation to rework
> pullup() method.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/usb/dwc2/gadget.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index c1dad46bbbdd..5eb2473031c4 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -2124,7 +2124,7 @@ static int s3c_hsotg_corereset(struct s3c_hsotg *hsotg)
>   *
>   * Issue a soft reset to the core, and await the core finishing it.
>   */
> -static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
> +static void s3c_hsotg_core_init_disconnected(struct s3c_hsotg *hsotg)
>  {
>  	s3c_hsotg_corereset(hsotg);
> 
> @@ -2241,14 +2241,23 @@ static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
>  		readl(hsotg->regs + DOEPCTL0));
> 
>  	/* clear global NAKs */
> -	writel(DCTL_CGOUTNAK | DCTL_CGNPINNAK,
> +	writel(DCTL_CGOUTNAK | DCTL_CGNPINNAK | DCTL_SFTDISCON,
>  	       hsotg->regs + DCTL);
> 
>  	/* must be at-least 3ms to allow bus to see disconnect */
>  	mdelay(3);
> 
>  	hsotg->last_rst = jiffies;
> +}
> +
> +static void s3c_hsotg_core_disconnect(struct s3c_hsotg *hsotg)
> +{
> +	/* set the soft-disconnect bit */
> +	__orr32(hsotg->regs + DCTL, DCTL_SFTDISCON);

I'm not really happy with adding more uses of these __orr32, __bic32
functions. When I first saw those, I went 'wtf?", because with the
assembly-sounding names, they look like some special ARM instruction
or something.

But I guess cleaning that up can wait for a future patch series, so OK.

> +}
> 
> +static void s3c_hsotg_core_connect(struct s3c_hsotg *hsotg)
> +{
>  	/* remove the soft-disconnect and let's go */
>  	__bic32(hsotg->regs + DCTL, DCTL_SFTDISCON);
>  }
> @@ -2348,7 +2357,8 @@ irq_retry:
>  				kill_all_requests(hsotg, &hsotg->eps[0],
>  							  -ECONNRESET, true);
> 
> -				s3c_hsotg_core_init(hsotg);
> +				s3c_hsotg_core_init_disconnected(hsotg);
> +				s3c_hsotg_core_connect(hsotg);
>  			}
>  		}
>  	}
> @@ -2981,7 +2991,8 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)
>  	if (is_on) {
>  		s3c_hsotg_phy_enable(hsotg);
>  		clk_enable(hsotg->clk);
> -		s3c_hsotg_core_init(hsotg);
> +		s3c_hsotg_core_init_disconnected(hsotg);
> +		s3c_hsotg_core_connect(hsotg);
>  	} else {
>  		clk_disable(hsotg->clk);
>  		s3c_hsotg_phy_disable(hsotg);
> @@ -3668,7 +3679,8 @@ static int s3c_hsotg_resume(struct platform_device *pdev)
> 
>  	spin_lock_irqsave(&hsotg->lock, flags);
>  	s3c_hsotg_phy_enable(hsotg);
> -	s3c_hsotg_core_init(hsotg);
> +	s3c_hsotg_core_init_disconnected(hsotg);
> +	s3c_hsotg_core_connect(hsotg);
>  	spin_unlock_irqrestore(&hsotg->lock, flags);
> 
>  	return ret;

Acked-by: Paul Zimmerman <paulz@synopsys.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Szyprowski Oct. 27, 2014, 1:52 p.m. | #2
Hello,

On 2014-10-25 02:35, Paul Zimmerman wrote:
>> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com]
>> Sent: Monday, October 20, 2014 3:46 AM
>>
>> This patch changes s3c_hsotg_core_init function to leave hardware in
>> soft disconnect mode, so the moment of coupling the hardware to the usb
>> bus can be later controlled by the separate functions for enabling and
>> disabling soft disconnect mode. This patch is a preparation to rework
>> pullup() method.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   drivers/usb/dwc2/gadget.c | 22 +++++++++++++++++-----
>>   1 file changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index c1dad46bbbdd..5eb2473031c4 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -2124,7 +2124,7 @@ static int s3c_hsotg_corereset(struct s3c_hsotg *hsotg)
>>    *
>>    * Issue a soft reset to the core, and await the core finishing it.
>>    */
>> -static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
>> +static void s3c_hsotg_core_init_disconnected(struct s3c_hsotg *hsotg)
>>   {
>>   	s3c_hsotg_corereset(hsotg);
>>
>> @@ -2241,14 +2241,23 @@ static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
>>   		readl(hsotg->regs + DOEPCTL0));
>>
>>   	/* clear global NAKs */
>> -	writel(DCTL_CGOUTNAK | DCTL_CGNPINNAK,
>> +	writel(DCTL_CGOUTNAK | DCTL_CGNPINNAK | DCTL_SFTDISCON,
>>   	       hsotg->regs + DCTL);
>>
>>   	/* must be at-least 3ms to allow bus to see disconnect */
>>   	mdelay(3);
>>
>>   	hsotg->last_rst = jiffies;
>> +}
>> +
>> +static void s3c_hsotg_core_disconnect(struct s3c_hsotg *hsotg)
>> +{
>> +	/* set the soft-disconnect bit */
>> +	__orr32(hsotg->regs + DCTL, DCTL_SFTDISCON);
> I'm not really happy with adding more uses of these __orr32, __bic32
> functions. When I first saw those, I went 'wtf?", because with the
> assembly-sounding names, they look like some special ARM instruction
> or something.
>
> But I guess cleaning that up can wait for a future patch series, so OK.

Those functions are really convenient, but I agree we need a better 
names for
them.

>> +}
>>
>> +static void s3c_hsotg_core_connect(struct s3c_hsotg *hsotg)
>> +{
>>   	/* remove the soft-disconnect and let's go */
>>   	__bic32(hsotg->regs + DCTL, DCTL_SFTDISCON);
>>   }
>> @@ -2348,7 +2357,8 @@ irq_retry:
>>   				kill_all_requests(hsotg, &hsotg->eps[0],
>>   							  -ECONNRESET, true);
>>
>> -				s3c_hsotg_core_init(hsotg);
>> +				s3c_hsotg_core_init_disconnected(hsotg);
>> +				s3c_hsotg_core_connect(hsotg);
>>   			}
>>   		}
>>   	}
>> @@ -2981,7 +2991,8 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)
>>   	if (is_on) {
>>   		s3c_hsotg_phy_enable(hsotg);
>>   		clk_enable(hsotg->clk);
>> -		s3c_hsotg_core_init(hsotg);
>> +		s3c_hsotg_core_init_disconnected(hsotg);
>> +		s3c_hsotg_core_connect(hsotg);
>>   	} else {
>>   		clk_disable(hsotg->clk);
>>   		s3c_hsotg_phy_disable(hsotg);
>> @@ -3668,7 +3679,8 @@ static int s3c_hsotg_resume(struct platform_device *pdev)
>>
>>   	spin_lock_irqsave(&hsotg->lock, flags);
>>   	s3c_hsotg_phy_enable(hsotg);
>> -	s3c_hsotg_core_init(hsotg);
>> +	s3c_hsotg_core_init_disconnected(hsotg);
>> +	s3c_hsotg_core_connect(hsotg);
>>   	spin_unlock_irqrestore(&hsotg->lock, flags);
>>
>>   	return ret;
> Acked-by: Paul Zimmerman <paulz@synopsys.com>

Best regards

Patch

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index c1dad46bbbdd..5eb2473031c4 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2124,7 +2124,7 @@  static int s3c_hsotg_corereset(struct s3c_hsotg *hsotg)
  *
  * Issue a soft reset to the core, and await the core finishing it.
  */
-static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_core_init_disconnected(struct s3c_hsotg *hsotg)
 {
 	s3c_hsotg_corereset(hsotg);
 
@@ -2241,14 +2241,23 @@  static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
 		readl(hsotg->regs + DOEPCTL0));
 
 	/* clear global NAKs */
-	writel(DCTL_CGOUTNAK | DCTL_CGNPINNAK,
+	writel(DCTL_CGOUTNAK | DCTL_CGNPINNAK | DCTL_SFTDISCON,
 	       hsotg->regs + DCTL);
 
 	/* must be at-least 3ms to allow bus to see disconnect */
 	mdelay(3);
 
 	hsotg->last_rst = jiffies;
+}
+
+static void s3c_hsotg_core_disconnect(struct s3c_hsotg *hsotg)
+{
+	/* set the soft-disconnect bit */
+	__orr32(hsotg->regs + DCTL, DCTL_SFTDISCON);
+}
 
+static void s3c_hsotg_core_connect(struct s3c_hsotg *hsotg)
+{
 	/* remove the soft-disconnect and let's go */
 	__bic32(hsotg->regs + DCTL, DCTL_SFTDISCON);
 }
@@ -2348,7 +2357,8 @@  irq_retry:
 				kill_all_requests(hsotg, &hsotg->eps[0],
 							  -ECONNRESET, true);
 
-				s3c_hsotg_core_init(hsotg);
+				s3c_hsotg_core_init_disconnected(hsotg);
+				s3c_hsotg_core_connect(hsotg);
 			}
 		}
 	}
@@ -2981,7 +2991,8 @@  static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)
 	if (is_on) {
 		s3c_hsotg_phy_enable(hsotg);
 		clk_enable(hsotg->clk);
-		s3c_hsotg_core_init(hsotg);
+		s3c_hsotg_core_init_disconnected(hsotg);
+		s3c_hsotg_core_connect(hsotg);
 	} else {
 		clk_disable(hsotg->clk);
 		s3c_hsotg_phy_disable(hsotg);
@@ -3668,7 +3679,8 @@  static int s3c_hsotg_resume(struct platform_device *pdev)
 
 	spin_lock_irqsave(&hsotg->lock, flags);
 	s3c_hsotg_phy_enable(hsotg);
-	s3c_hsotg_core_init(hsotg);
+	s3c_hsotg_core_init_disconnected(hsotg);
+	s3c_hsotg_core_connect(hsotg);
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 
 	return ret;