[PATCH 0/3] usb: ohci-at91: various fixes and improvements

[PATCH 0/3] usb: ohci-at91: various fixes and improvements

[Boris BREZILLON]

Hello,

This patch series fixes a bug detected in 3.13-rc1 caused by a wrong
assumption on platform device resources order in the platform device
resource table.

It also move the different driver resources (clks and iomem) retrieval
to the device managed versions (devm_ functions).

Best Regards,

Boris

Boris BREZILLON (3):
  usb: ohci-at91: fix irq and iomem resource retrieval
  usb: ohci-at91: replace request_mem_region + ioremap by
    devm_request_and_ioremap
  usb: ohci-at91: use device managed clk retrieval

 drivers/usb/host/ohci-at91.c |   75 ++++++++++++++----------------------------
 1 file changed, 24 insertions(+), 51 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] usb: ohci-at91: fix irq and iomem resource retrieval

[Boris BREZILLON]

When using dt resources retrieval (interrupts and reg properties) there is
no predefined order for these resources in the platform dev resource
table.

Retrieve resources using the platform_get_resource function instead of
direct resource table entries to avoid resource type mismatch.

Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
Tested-by: Robert Nelson <robertcnelson@gmail.com>
---
 drivers/usb/host/ohci-at91.c |   19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 418444e..7aec6ca 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -136,23 +136,26 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	struct ohci_hcd *ohci;
 	int retval;
 	struct usb_hcd *hcd = NULL;
+	struct device *dev = &pdev->dev;
+	struct resource *mem_r, *irq_r;
 
-	if (pdev->num_resources != 2) {
-		pr_debug("hcd probe: invalid num_resources");
+	mem_r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem_r) {
+		dev_dbg(dev, "hcd probe: missing memory resource\n");
 		return -ENODEV;
 	}
 
-	if ((pdev->resource[0].flags != IORESOURCE_MEM)
-			|| (pdev->resource[1].flags != IORESOURCE_IRQ)) {
-		pr_debug("hcd probe: invalid resource type\n");
+	irq_r = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!irq_r) {
+		dev_dbg(dev, "hcd probe: missing irq resource\n");
 		return -ENODEV;
 	}
 
 	hcd = usb_create_hcd(driver, &pdev->dev, "at91");
 	if (!hcd)
 		return -ENOMEM;
-	hcd->rsrc_start = pdev->resource[0].start;
-	hcd->rsrc_len = resource_size(&pdev->resource[0]);
+	hcd->rsrc_start = mem_r->start;
+	hcd->rsrc_len = resource_size(mem_r);
 
 	if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
 		pr_debug("request_mem_region failed\n");
@@ -199,7 +202,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	ohci->num_ports = board->ports;
 	at91_start_hc(pdev);
 
-	retval = usb_add_hcd(hcd, pdev->resource[1].start, IRQF_SHARED);
+	retval = usb_add_hcd(hcd, irq_r->start, IRQF_SHARED);
 	if (retval == 0)
 		return retval;
 
-- 
1.7.9.5

[PATCH 2/3] usb: ohci-at91: replace request_mem_region + ioremap by devm_request_and_ioremap

[Boris BREZILLON]

Replace the request_mem_region + ioremap calls by the
devm_request_and_ioremap call which does the same things but with device
managed resources.

Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
Tested-by: Robert Nelson <robertcnelson@gmail.com>
---
 drivers/usb/host/ohci-at91.c |   24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 7aec6ca..c406f1e 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -157,24 +157,18 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	hcd->rsrc_start = mem_r->start;
 	hcd->rsrc_len = resource_size(mem_r);
 
-	if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
-		pr_debug("request_mem_region failed\n");
-		retval = -EBUSY;
-		goto err1;
-	}
-
-	hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+	hcd->regs = devm_request_and_ioremap(&pdev->dev, mem_r);
 	if (!hcd->regs) {
-		pr_debug("ioremap failed\n");
+		dev_dbg(dev, "devm_request_and_ioremap failed\n");
 		retval = -EIO;
-		goto err2;
+		goto err;
 	}
 
 	iclk = clk_get(&pdev->dev, "ohci_clk");
 	if (IS_ERR(iclk)) {
 		dev_err(&pdev->dev, "failed to get ohci_clk\n");
 		retval = PTR_ERR(iclk);
-		goto err3;
+		goto err;
 	}
 	fclk = clk_get(&pdev->dev, "uhpck");
 	if (IS_ERR(fclk)) {
@@ -218,13 +212,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
  err4:
 	clk_put(iclk);
 
- err3:
-	iounmap(hcd->regs);
-
- err2:
-	release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
-
- err1:
+ err:
 	usb_put_hcd(hcd);
 	return retval;
 }
@@ -247,8 +235,6 @@ static void usb_hcd_at91_remove(struct usb_hcd *hcd,
 {
 	usb_remove_hcd(hcd);
 	at91_stop_hc(pdev);
-	iounmap(hcd->regs);
-	release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 	usb_put_hcd(hcd);
 
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
-- 
1.7.9.5

[PATCH 3/3] usb: ohci-at91: use device managed clk retrieval

[Boris BREZILLON]

Replace clk_get calls by devm_clk_get calls.

Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
Tested-by: Robert Nelson <robertcnelson@gmail.com>
---
 drivers/usb/host/ohci-at91.c |   32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index c406f1e..3652962 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -164,30 +164,30 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 		goto err;
 	}
 
-	iclk = clk_get(&pdev->dev, "ohci_clk");
+	iclk = devm_clk_get(dev, "ohci_clk");
 	if (IS_ERR(iclk)) {
-		dev_err(&pdev->dev, "failed to get ohci_clk\n");
+		dev_err(dev, "failed to get ohci_clk\n");
 		retval = PTR_ERR(iclk);
 		goto err;
 	}
-	fclk = clk_get(&pdev->dev, "uhpck");
+	fclk = devm_clk_get(dev, "uhpck");
 	if (IS_ERR(fclk)) {
 		dev_err(&pdev->dev, "failed to get uhpck\n");
 		retval = PTR_ERR(fclk);
-		goto err4;
+		goto err;
 	}
-	hclk = clk_get(&pdev->dev, "hclk");
+	hclk = devm_clk_get(dev, "hclk");
 	if (IS_ERR(hclk)) {
 		dev_err(&pdev->dev, "failed to get hclk\n");
 		retval = PTR_ERR(hclk);
-		goto err5;
+		goto err;
 	}
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		uclk = clk_get(&pdev->dev, "usb_clk");
+		uclk = devm_clk_get(&pdev->dev, "usb_clk");
 		if (IS_ERR(uclk)) {
 			dev_err(&pdev->dev, "failed to get uclk\n");
 			retval = PTR_ERR(uclk);
-			goto err6;
+			goto err;
 		}
 	}
 
@@ -203,15 +203,6 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	/* Error handling */
 	at91_stop_hc(pdev);
 
-	if (IS_ENABLED(CONFIG_COMMON_CLK))
-		clk_put(uclk);
- err6:
-	clk_put(hclk);
- err5:
-	clk_put(fclk);
- err4:
-	clk_put(iclk);
-
  err:
 	usb_put_hcd(hcd);
 	return retval;
@@ -236,13 +227,6 @@ static void usb_hcd_at91_remove(struct usb_hcd *hcd,
 	usb_remove_hcd(hcd);
 	at91_stop_hc(pdev);
 	usb_put_hcd(hcd);
-
-	if (IS_ENABLED(CONFIG_COMMON_CLK))
-		clk_put(uclk);
-	clk_put(hclk);
-	clk_put(fclk);
-	clk_put(iclk);
-	fclk = iclk = hclk = NULL;
 }
 
 /*-------------------------------------------------------------------------*/
-- 
1.7.9.5

[PATCH 1/3] usb: ohci-at91: fix irq and iomem resource retrieval

[Nicolas Ferre]

On 03/12/2013 15:07, Boris BREZILLON :
> When using dt resources retrieval (interrupts and reg properties) there is
> no predefined order for these resources in the platform dev resource
> table.
>
> Retrieve resources using the platform_get_resource function instead of
> direct resource table entries to avoid resource type mismatch.
>
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
> Tested-by: Robert Nelson <robertcnelson@gmail.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
>   drivers/usb/host/ohci-at91.c |   19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index 418444e..7aec6ca 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -136,23 +136,26 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
>   	struct ohci_hcd *ohci;
>   	int retval;
>   	struct usb_hcd *hcd = NULL;
> +	struct device *dev = &pdev->dev;
> +	struct resource *mem_r, *irq_r;
>
> -	if (pdev->num_resources != 2) {
> -		pr_debug("hcd probe: invalid num_resources");
> +	mem_r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!mem_r) {
> +		dev_dbg(dev, "hcd probe: missing memory resource\n");
>   		return -ENODEV;
>   	}
>
> -	if ((pdev->resource[0].flags != IORESOURCE_MEM)
> -			|| (pdev->resource[1].flags != IORESOURCE_IRQ)) {
> -		pr_debug("hcd probe: invalid resource type\n");
> +	irq_r = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +	if (!irq_r) {
> +		dev_dbg(dev, "hcd probe: missing irq resource\n");
>   		return -ENODEV;
>   	}
>
>   	hcd = usb_create_hcd(driver, &pdev->dev, "at91");
>   	if (!hcd)
>   		return -ENOMEM;
> -	hcd->rsrc_start = pdev->resource[0].start;
> -	hcd->rsrc_len = resource_size(&pdev->resource[0]);
> +	hcd->rsrc_start = mem_r->start;
> +	hcd->rsrc_len = resource_size(mem_r);
>
>   	if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
>   		pr_debug("request_mem_region failed\n");
> @@ -199,7 +202,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
>   	ohci->num_ports = board->ports;
>   	at91_start_hc(pdev);
>
> -	retval = usb_add_hcd(hcd, pdev->resource[1].start, IRQF_SHARED);
> +	retval = usb_add_hcd(hcd, irq_r->start, IRQF_SHARED);
>   	if (retval == 0)
>   		return retval;
>
>


-- 
Nicolas Ferre

[PATCH 2/3] usb: ohci-at91: replace request_mem_region + ioremap by devm_request_and_ioremap

[Nicolas Ferre]

On 03/12/2013 15:07, Boris BREZILLON :
> Replace the request_mem_region + ioremap calls by the
> devm_request_and_ioremap call which does the same things but with device
> managed resources.
>
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
> Tested-by: Robert Nelson <robertcnelson@gmail.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
>   drivers/usb/host/ohci-at91.c |   24 +++++-------------------
>   1 file changed, 5 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index 7aec6ca..c406f1e 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -157,24 +157,18 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
>   	hcd->rsrc_start = mem_r->start;
>   	hcd->rsrc_len = resource_size(mem_r);
>
> -	if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
> -		pr_debug("request_mem_region failed\n");
> -		retval = -EBUSY;
> -		goto err1;
> -	}
> -
> -	hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
> +	hcd->regs = devm_request_and_ioremap(&pdev->dev, mem_r);
>   	if (!hcd->regs) {
> -		pr_debug("ioremap failed\n");
> +		dev_dbg(dev, "devm_request_and_ioremap failed\n");
>   		retval = -EIO;
> -		goto err2;
> +		goto err;
>   	}
>
>   	iclk = clk_get(&pdev->dev, "ohci_clk");
>   	if (IS_ERR(iclk)) {
>   		dev_err(&pdev->dev, "failed to get ohci_clk\n");
>   		retval = PTR_ERR(iclk);
> -		goto err3;
> +		goto err;
>   	}
>   	fclk = clk_get(&pdev->dev, "uhpck");
>   	if (IS_ERR(fclk)) {
> @@ -218,13 +212,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
>    err4:
>   	clk_put(iclk);
>
> - err3:
> -	iounmap(hcd->regs);
> -
> - err2:
> -	release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
> -
> - err1:
> + err:
>   	usb_put_hcd(hcd);
>   	return retval;
>   }
> @@ -247,8 +235,6 @@ static void usb_hcd_at91_remove(struct usb_hcd *hcd,
>   {
>   	usb_remove_hcd(hcd);
>   	at91_stop_hc(pdev);
> -	iounmap(hcd->regs);
> -	release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
>   	usb_put_hcd(hcd);
>
>   	if (IS_ENABLED(CONFIG_COMMON_CLK))
>


-- 
Nicolas Ferre

[PATCH 3/3] usb: ohci-at91: use device managed clk retrieval

[Nicolas Ferre]

On 03/12/2013 15:07, Boris BREZILLON :
> Replace clk_get calls by devm_clk_get calls.
>
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
> Tested-by: Robert Nelson <robertcnelson@gmail.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Thanks Boris for these fixes.

Alan, Greg, can you take the whole series as fixes for 3.13?

Thanks, best regards,

> ---
>   drivers/usb/host/ohci-at91.c |   32 ++++++++------------------------
>   1 file changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index c406f1e..3652962 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -164,30 +164,30 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
>   		goto err;
>   	}
>
> -	iclk = clk_get(&pdev->dev, "ohci_clk");
> +	iclk = devm_clk_get(dev, "ohci_clk");
>   	if (IS_ERR(iclk)) {
> -		dev_err(&pdev->dev, "failed to get ohci_clk\n");
> +		dev_err(dev, "failed to get ohci_clk\n");
>   		retval = PTR_ERR(iclk);
>   		goto err;
>   	}
> -	fclk = clk_get(&pdev->dev, "uhpck");
> +	fclk = devm_clk_get(dev, "uhpck");
>   	if (IS_ERR(fclk)) {
>   		dev_err(&pdev->dev, "failed to get uhpck\n");
>   		retval = PTR_ERR(fclk);
> -		goto err4;
> +		goto err;
>   	}
> -	hclk = clk_get(&pdev->dev, "hclk");
> +	hclk = devm_clk_get(dev, "hclk");
>   	if (IS_ERR(hclk)) {
>   		dev_err(&pdev->dev, "failed to get hclk\n");
>   		retval = PTR_ERR(hclk);
> -		goto err5;
> +		goto err;
>   	}
>   	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> -		uclk = clk_get(&pdev->dev, "usb_clk");
> +		uclk = devm_clk_get(&pdev->dev, "usb_clk");
>   		if (IS_ERR(uclk)) {
>   			dev_err(&pdev->dev, "failed to get uclk\n");
>   			retval = PTR_ERR(uclk);
> -			goto err6;
> +			goto err;
>   		}
>   	}
>
> @@ -203,15 +203,6 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
>   	/* Error handling */
>   	at91_stop_hc(pdev);
>
> -	if (IS_ENABLED(CONFIG_COMMON_CLK))
> -		clk_put(uclk);
> - err6:
> -	clk_put(hclk);
> - err5:
> -	clk_put(fclk);
> - err4:
> -	clk_put(iclk);
> -
>    err:
>   	usb_put_hcd(hcd);
>   	return retval;
> @@ -236,13 +227,6 @@ static void usb_hcd_at91_remove(struct usb_hcd *hcd,
>   	usb_remove_hcd(hcd);
>   	at91_stop_hc(pdev);
>   	usb_put_hcd(hcd);
> -
> -	if (IS_ENABLED(CONFIG_COMMON_CLK))
> -		clk_put(uclk);
> -	clk_put(hclk);
> -	clk_put(fclk);
> -	clk_put(iclk);
> -	fclk = iclk = hclk = NULL;
>   }
>
>   /*-------------------------------------------------------------------------*/
>


-- 
Nicolas Ferre

[PATCH 3/3] usb: ohci-at91: use device managed clk retrieval

[Alan Stern]

On Tue, 3 Dec 2013, Nicolas Ferre wrote:

> On 03/12/2013 15:07, Boris BREZILLON :
> > Replace clk_get calls by devm_clk_get calls.
> >
> > Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
> > Tested-by: Robert Nelson <robertcnelson@gmail.com>
> 
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> 
> Thanks Boris for these fixes.
> 
> Alan, Greg, can you take the whole series as fixes for 3.13?

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>

The patches look fine to me.  But only the 1/3 patch fixes a bug; the 
others merely change the resource management.

Alan Stern

[PATCH 3/3] usb: ohci-at91: use device managed clk retrieval

[boris brezillon]

Hello Sergei,

On 03/12/2013 19:01, Sergei Shtylyov wrote:
> Hello.
>
> On 12/03/2013 05:07 PM, Boris BREZILLON wrote:
>
>> Replace clk_get calls by devm_clk_get calls.
>
>> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
>> Tested-by: Robert Nelson <robertcnelson@gmail.com>
>> ---
>>   drivers/usb/host/ohci-at91.c |   32 ++++++++------------------------
>>   1 file changed, 8 insertions(+), 24 deletions(-)
>
>> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
>> index c406f1e..3652962 100644
>> --- a/drivers/usb/host/ohci-at91.c
>> +++ b/drivers/usb/host/ohci-at91.c
>> @@ -164,30 +164,30 @@ static int usb_hcd_at91_probe(const struct 
>> hc_driver *driver,
>>           goto err;
>>       }
>>
>> -    iclk = clk_get(&pdev->dev, "ohci_clk");
>> +    iclk = devm_clk_get(dev, "ohci_clk");
>>       if (IS_ERR(iclk)) {
>> -        dev_err(&pdev->dev, "failed to get ohci_clk\n");
>> +        dev_err(dev, "failed to get ohci_clk\n");
>
>    You changed this call...
>
>>           retval = PTR_ERR(iclk);
>>           goto err;
>>       }
>> -    fclk = clk_get(&pdev->dev, "uhpck");
>> +    fclk = devm_clk_get(dev, "uhpck");
>>       if (IS_ERR(fclk)) {
>>           dev_err(&pdev->dev, "failed to get uhpck\n");
>
>    ... but not this one.

This is an oversight.

>
>>           retval = PTR_ERR(fclk);
>> -        goto err4;
>> +        goto err;
>>       }
>> -    hclk = clk_get(&pdev->dev, "hclk");
>> +    hclk = devm_clk_get(dev, "hclk");
>>       if (IS_ERR(hclk)) {
>>           dev_err(&pdev->dev, "failed to get hclk\n");
>
>    ... or this one. Actually, I think cganging these calls would be 
> out of scope for this patch.

Ditto.

I'll split the patch to in 2 patches:
  1) replace &pdev->dev by dev
  2) use devm_clk_get instead of clk_get.

>
>>           retval = PTR_ERR(hclk);
>> -        goto err5;
>> +        goto err;
>>       }
>>       if (IS_ENABLED(CONFIG_COMMON_CLK)) {
>> -        uclk = clk_get(&pdev->dev, "usb_clk");
>> +        uclk = devm_clk_get(&pdev->dev, "usb_clk");
>
>    Hm, why not 'dev' like in all of the above?
Ditto.

Thanks for your review.

Best Regards,

Boris
>
>>           if (IS_ERR(uclk)) {
>>               dev_err(&pdev->dev, "failed to get uclk\n");
>>               retval = PTR_ERR(uclk);
>> -            goto err6;
>> +            goto err;
>>           }
>>       }
>>
>
> WBR, Sergei
>
>

[PATCH 3/3] usb: ohci-at91: use device managed clk retrieval

[Sergei Shtylyov]

Hello.

On 12/03/2013 05:07 PM, Boris BREZILLON wrote:

> Replace clk_get calls by devm_clk_get calls.

> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
> Tested-by: Robert Nelson <robertcnelson@gmail.com>
> ---
>   drivers/usb/host/ohci-at91.c |   32 ++++++++------------------------
>   1 file changed, 8 insertions(+), 24 deletions(-)

> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index c406f1e..3652962 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -164,30 +164,30 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
>   		goto err;
>   	}
>
> -	iclk = clk_get(&pdev->dev, "ohci_clk");
> +	iclk = devm_clk_get(dev, "ohci_clk");
>   	if (IS_ERR(iclk)) {
> -		dev_err(&pdev->dev, "failed to get ohci_clk\n");
> +		dev_err(dev, "failed to get ohci_clk\n");

    You changed this call...

>   		retval = PTR_ERR(iclk);
>   		goto err;
>   	}
> -	fclk = clk_get(&pdev->dev, "uhpck");
> +	fclk = devm_clk_get(dev, "uhpck");
>   	if (IS_ERR(fclk)) {
>   		dev_err(&pdev->dev, "failed to get uhpck\n");

    ... but not this one.

>   		retval = PTR_ERR(fclk);
> -		goto err4;
> +		goto err;
>   	}
> -	hclk = clk_get(&pdev->dev, "hclk");
> +	hclk = devm_clk_get(dev, "hclk");
>   	if (IS_ERR(hclk)) {
>   		dev_err(&pdev->dev, "failed to get hclk\n");

    ... or this one. Actually, I think cganging these calls would be out of 
scope for this patch.

>   		retval = PTR_ERR(hclk);
> -		goto err5;
> +		goto err;
>   	}
>   	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> -		uclk = clk_get(&pdev->dev, "usb_clk");
> +		uclk = devm_clk_get(&pdev->dev, "usb_clk");

    Hm, why not 'dev' like in all of the above?

>   		if (IS_ERR(uclk)) {
>   			dev_err(&pdev->dev, "failed to get uclk\n");
>   			retval = PTR_ERR(uclk);
> -			goto err6;
> +			goto err;
>   		}
>   	}
>

WBR, Sergei

[PATCH 3/3] usb: ohci-at91: use device managed clk retrieval

[boris brezillon]

Hello Alan,

On 03/12/2013 16:32, Alan Stern wrote:
> On Tue, 3 Dec 2013, Nicolas Ferre wrote:
>
>> On 03/12/2013 15:07, Boris BREZILLON :
>>> Replace clk_get calls by devm_clk_get calls.
>>>
>>> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
>>> Tested-by: Robert Nelson <robertcnelson@gmail.com>
>> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>
>> Thanks Boris for these fixes.
>>
>> Alan, Greg, can you take the whole series as fixes for 3.13?
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Sorry, but I forgot to add your Signed-off-by in the 2nd version of this
series.
>
> The patches look fine to me.  But only the 1/3 patch fixes a bug; the
> others merely change the resource management.

Do you want me to split this series ?
  1) the 1st patch that should be merged in 3.13
  2) patches 2 to 4 that might be applied later

Best Regards,

Boris

>
> Alan Stern
>

[PATCH 3/3] usb: ohci-at91: use device managed clk retrieval

[Alan Stern]

On Wed, 4 Dec 2013, boris brezillon wrote:

> > The patches look fine to me.  But only the 1/3 patch fixes a bug; the
> > others merely change the resource management.
> 
> Do you want me to split this series ?
>   1) the 1st patch that should be merged in 3.13
>   2) patches 2 to 4 that might be applied later

That probably would make Greg happier.

Alan Stern

[PATCH 3/3] usb: ohci-at91: use device managed clk retrieval

[Douglas Gilbert]

On 13-12-04 04:21 PM, Alan Stern wrote:
> On Wed, 4 Dec 2013, boris brezillon wrote:
>
>>> The patches look fine to me.  But only the 1/3 patch fixes a bug; the
>>> others merely change the resource management.
>>
>> Do you want me to split this series ?
>>    1) the 1st patch that should be merged in 3.13
>>    2) patches 2 to 4 that might be applied later
>
> That probably would make Greg happier.

Putting my initial reporter hat on, USB OHCI is
completely broken in lk 3.13.0 rc1 and rc2 for AT91
family members. So it is difficult to make the
situation worse. IMO the whole 4 patches should go
in, since it only impacts that family. Also the kernel
is more than a month away from release. Perhaps the
naysayers should be looking around for whatever else
the "of/irq: Pass trigger type in IRQ resource flags"
patch has broken.

Doug Gilbert

[PATCH 3/3] usb: ohci-at91: use device managed clk retrieval

[Tomasz Figa]

2013/12/4 Douglas Gilbert <dgilbert@interlog.com>:
> On 13-12-04 04:21 PM, Alan Stern wrote:
>>
>> On Wed, 4 Dec 2013, boris brezillon wrote:
>>
>>>> The patches look fine to me.  But only the 1/3 patch fixes a bug; the
>>>> others merely change the resource management.
>>>
>>>
>>> Do you want me to split this series ?
>>>    1) the 1st patch that should be merged in 3.13
>>>    2) patches 2 to 4 that might be applied later
>>
>>
>> That probably would make Greg happier.
>
>
> Putting my initial reporter hat on, USB OHCI is
> completely broken in lk 3.13.0 rc1 and rc2 for AT91
> family members. So it is difficult to make the
> situation worse. IMO the whole 4 patches should go
> in, since it only impacts that family. Also the kernel
> is more than a month away from release. Perhaps the
> naysayers should be looking around for whatever else
> the "of/irq: Pass trigger type in IRQ resource flags"
> patch has broken.

...or rather whatever brokenness it has uncovered.

Best regards,
Tomasz