| Message ID | 1386079669-3995-4-git-send-email-b.brezillon@overkiz.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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; > } > > /*-------------------------------------------------------------------------*/ >
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
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 > >
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
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 >
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
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
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
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; } /*-------------------------------------------------------------------------*/