[PATCH] usb: ehci-platform: add reset controller number in struct ehci_platform_priv

[PATCH] usb: ehci-platform: add reset controller number in struct ehci_platform_priv

[Jiancheng Xue]

Some generic-ehci compatible controllers have more than one reset signal
lines, e.g., Synopsys DWC USB2.0 Host-AHB Controller has two resets bus_reset
and roothub_reset. Two more resets are added in this patch in order for this
kind of controller to use this driver directly.

Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com>
---
 drivers/usb/host/ehci-platform.c | 41 ++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index 1757ebb..a1358df 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -39,11 +39,12 @@
 
 #define DRIVER_DESC "EHCI generic platform driver"
 #define EHCI_MAX_CLKS 3
+#define EHCI_MAX_RSTS 3
 #define hcd_to_ehci_priv(h) ((struct ehci_platform_priv *)hcd_to_ehci(h)->priv)
 
 struct ehci_platform_priv {
 	struct clk *clks[EHCI_MAX_CLKS];
-	struct reset_control *rst;
+	struct reset_control *rsts[EHCI_MAX_RSTS];
 	struct phy **phys;
 	int num_phys;
 	bool reset_on_resume;
@@ -149,7 +150,7 @@ static int ehci_platform_probe(struct platform_device *dev)
 	struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
 	struct ehci_platform_priv *priv;
 	struct ehci_hcd *ehci;
-	int err, irq, phy_num, clk = 0;
+	int err, irq, phy_num, clk = 0, rst;
 
 	if (usb_disabled())
 		return -ENODEV;
@@ -234,16 +235,20 @@ static int ehci_platform_probe(struct platform_device *dev)
 		}
 	}
 
-	priv->rst = devm_reset_control_get_optional(&dev->dev, NULL);
-	if (IS_ERR(priv->rst)) {
-		err = PTR_ERR(priv->rst);
-		if (err == -EPROBE_DEFER)
-			goto err_put_clks;
-		priv->rst = NULL;
-	} else {
-		err = reset_control_deassert(priv->rst);
+	for (rst = 0; rst < EHCI_MAX_RSTS; rst++) {
+		priv->rsts[rst] = of_reset_control_get_by_index(
+					dev->dev.of_node, rst);
+		if (IS_ERR(priv->rsts[rst])) {
+			err = PTR_ERR(priv->rsts[rst]);
+			if (err == -EPROBE_DEFER)
+				goto err_reset;
+			priv->rsts[rst] = NULL;
+			break;
+		}
+
+		err = reset_control_deassert(priv->rsts[rst]);
 		if (err)
-			goto err_put_clks;
+			goto err_reset;
 	}
 
 	if (pdata->big_endian_desc)
@@ -300,8 +305,10 @@ err_power:
 	if (pdata->power_off)
 		pdata->power_off(dev);
 err_reset:
-	if (priv->rst)
-		reset_control_assert(priv->rst);
+	while (--rst >= 0) {
+		reset_control_assert(priv->rsts[rst]);
+		reset_control_put(priv->rsts[rst]);
+	}
 err_put_clks:
 	while (--clk >= 0)
 		clk_put(priv->clks[clk]);
@@ -319,15 +326,17 @@ static int ehci_platform_remove(struct platform_device *dev)
 	struct usb_hcd *hcd = platform_get_drvdata(dev);
 	struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
 	struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
-	int clk;
+	int clk, rst;
 
 	usb_remove_hcd(hcd);
 
 	if (pdata->power_off)
 		pdata->power_off(dev);
 
-	if (priv->rst)
-		reset_control_assert(priv->rst);
+	for (rst = 0; rst < EHCI_MAX_RSTS && priv->rst[rst]; rst++) {
+		reset_control_assert(priv->rsts[rst]);
+		reset_control_put(priv->rsts[rst]);
+	}
 
 	for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++)
 		clk_put(priv->clks[clk]);
-- 
1.9.1

Re: [PATCH] usb: ehci-platform: add reset controller number in struct ehci_platform_priv

[Alan Stern]

On Sat, 23 Apr 2016, Jiancheng Xue wrote:

> Some generic-ehci compatible controllers have more than one reset signal
> lines, e.g., Synopsys DWC USB2.0 Host-AHB Controller has two resets bus_reset
> and roothub_reset. Two more resets are added in this patch in order for this
> kind of controller to use this driver directly.
> 
> Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com>


> +	for (rst = 0; rst < EHCI_MAX_RSTS; rst++) {
> +		priv->rsts[rst] = of_reset_control_get_by_index(
> +					dev->dev.of_node, rst);
> +		if (IS_ERR(priv->rsts[rst])) {
> +			err = PTR_ERR(priv->rsts[rst]);
> +			if (err == -EPROBE_DEFER)
> +				goto err_reset;
> +			priv->rsts[rst] = NULL;
> +			break;
> +		}
> +
> +		err = reset_control_deassert(priv->rsts[rst]);
>  		if (err)
> -			goto err_put_clks;
> +			goto err_reset;

If an error occurs here...

>  	}
>  
>  	if (pdata->big_endian_desc)
> @@ -300,8 +305,10 @@ err_power:
>  	if (pdata->power_off)
>  		pdata->power_off(dev);
>  err_reset:
> -	if (priv->rst)
> -		reset_control_assert(priv->rst);
> +	while (--rst >= 0) {
> +		reset_control_assert(priv->rsts[rst]);
> +		reset_control_put(priv->rsts[rst]);
> +	}

You won't call reset_control_put() for the offending reset line.

Alan Stern

Re: [PATCH] usb: ehci-platform: add reset controller number in struct ehci_platform_priv

[Jiancheng Xue]

Hi Alan,

On 2016/4/25 22:43, Alan Stern wrote:
> On Sat, 23 Apr 2016, Jiancheng Xue wrote:
> 
>> Some generic-ehci compatible controllers have more than one reset signal
>> lines, e.g., Synopsys DWC USB2.0 Host-AHB Controller has two resets bus_reset
>> and roothub_reset. Two more resets are added in this patch in order for this
>> kind of controller to use this driver directly.
>>
>> Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com>
> 
> 
>> +	for (rst = 0; rst < EHCI_MAX_RSTS; rst++) {
>> +		priv->rsts[rst] = of_reset_control_get_by_index(
>> +					dev->dev.of_node, rst);
>> +		if (IS_ERR(priv->rsts[rst])) {
>> +			err = PTR_ERR(priv->rsts[rst]);
>> +			if (err == -EPROBE_DEFER)
>> +				goto err_reset;
>> +			priv->rsts[rst] = NULL;
>> +			break;
>> +		}
>> +
>> +		err = reset_control_deassert(priv->rsts[rst]);
>>  		if (err)
>> -			goto err_put_clks;
>> +			goto err_reset;
> 
> If an error occurs here...
> 
Sorry. It's really a problem. I'll modify it in v2 like below:

err = reset_control_deassert(priv->rsts[rst]);
if(err) {
	reset_control_put(priv->rsts[rst]);
	goto err_reset;
}

>>  	}
>>  
>>  	if (pdata->big_endian_desc)
>> @@ -300,8 +305,10 @@ err_power:
>>  	if (pdata->power_off)
>>  		pdata->power_off(dev);
>>  err_reset:
>> -	if (priv->rst)
>> -		reset_control_assert(priv->rst);
>> +	while (--rst >= 0) {
>> +		reset_control_assert(priv->rsts[rst]);
>> +		reset_control_put(priv->rsts[rst]);
>> +	}
> 
> You won't call reset_control_put() for the offending reset line.
> 
See above.

Is it OK except this bug? If so, I'll send out v2 patch.

Thank you very much!

Regards.
Jiancheng

Re: [PATCH] usb: ehci-platform: add reset controller number in struct ehci_platform_priv

[Alan Stern]

On Tue, 26 Apr 2016, Jiancheng Xue wrote:

> > If an error occurs here...
> > 
> Sorry. It's really a problem. I'll modify it in v2 like below:
> 
> err = reset_control_deassert(priv->rsts[rst]);
> if(err) {
> 	reset_control_put(priv->rsts[rst]);
> 	goto err_reset;
> }

Yes, that would be good.  (Except for the missing ' ' between "if" and 
"(".)

> Is it OK except this bug? If so, I'll send out v2 patch.

Everything else looked okay to me.

Alan Stern