2016-04-23 06:33:20

by Jiancheng Xue

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

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 <[email protected]>
---
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


2016-04-25 14:43:52

by Alan Stern

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

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 <[email protected]>


> + 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

2016-04-26 01:16:14

by Jiancheng Xue

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

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 <[email protected]>
>
>
>> + 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


2016-04-26 14:49:51

by Alan Stern

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

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