| Message ID | 1359372631-8180-10-git-send-email-rogerq@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 28 Jan 2013, Roger Quadros wrote:
> Make use of devm_request_and_ioremap() and correct comment.
Didn't a big patch come through recently converting all usages of
devm_request_and_ioremap() to another function (I forget the name) that
does its own error reporting and returns an ERR_PTR value?
(Checks the mailing list archive...) Ah, here it is:
http://marc.info/?l=linux-usb&m=135876311801537&w=2
And the new function is devm_ioremap_resource(). We should avoid
adding new usages of an old interface.
Alan Stern
On Mon, Jan 28, 2013 at 10:17:33AM -0500, Alan Stern wrote: > On Mon, 28 Jan 2013, Roger Quadros wrote: > > > Make use of devm_request_and_ioremap() and correct comment. > > Didn't a big patch come through recently converting all usages of > devm_request_and_ioremap() to another function (I forget the name) that > does its own error reporting and returns an ERR_PTR value? > > (Checks the mailing list archive...) Ah, here it is: > > http://marc.info/?l=linux-usb&m=135876311801537&w=2 > > And the new function is devm_ioremap_resource(). We should avoid > adding new usages of an old interface. Maybe... if devm_ioremap_resource() was already in the kernel. The problem at the moment is it isn't, and it'll probably be rather horrid for everyone to deal with especially when it comes to testing.
On Mon, 28 Jan 2013, Russell King - ARM Linux wrote: > On Mon, Jan 28, 2013 at 10:17:33AM -0500, Alan Stern wrote: > > On Mon, 28 Jan 2013, Roger Quadros wrote: > > > > > Make use of devm_request_and_ioremap() and correct comment. > > > > Didn't a big patch come through recently converting all usages of > > devm_request_and_ioremap() to another function (I forget the name) that > > does its own error reporting and returns an ERR_PTR value? > > > > (Checks the mailing list archive...) Ah, here it is: > > > > http://marc.info/?l=linux-usb&m=135876311801537&w=2 > > > > And the new function is devm_ioremap_resource(). We should avoid > > adding new usages of an old interface. > > Maybe... if devm_ioremap_resource() was already in the kernel. The > problem at the moment is it isn't, and it'll probably be rather > horrid for everyone to deal with especially when it comes to testing. Not in the kernel yet? I didn't realize that. Looks like Thierry Reding will have some clean-up work to do when it does get in. Okay, then Acked-by: Alan Stern <stern@rowland.harvard.edu>
On Tue, Jan 29, 2013 at 10:34:53AM -0500, Alan Stern wrote: > On Mon, 28 Jan 2013, Russell King - ARM Linux wrote: > > > On Mon, Jan 28, 2013 at 10:17:33AM -0500, Alan Stern wrote: > > > On Mon, 28 Jan 2013, Roger Quadros wrote: > > > > > > > Make use of devm_request_and_ioremap() and correct comment. > > > > > > Didn't a big patch come through recently converting all usages of > > > devm_request_and_ioremap() to another function (I forget the name) that > > > does its own error reporting and returns an ERR_PTR value? > > > > > > (Checks the mailing list archive...) Ah, here it is: > > > > > > http://marc.info/?l=linux-usb&m=135876311801537&w=2 > > > > > > And the new function is devm_ioremap_resource(). We should avoid > > > adding new usages of an old interface. > > > > Maybe... if devm_ioremap_resource() was already in the kernel. The > > problem at the moment is it isn't, and it'll probably be rather > > horrid for everyone to deal with especially when it comes to testing. > > Not in the kernel yet? I didn't realize that. Looks like Thierry > Reding will have some clean-up work to do when it does get in. It may be in linux-next, but it isn't in Linus' yet... (of course, it's probably scheduled for the upcoming merge window). Certainly: $ grep -r devm_ioremap_resource include against Linus' tree (6abb7c2) still turns up nothing.
On Tue, Jan 29, 2013 at 04:01:34PM +0000, Russell King - ARM Linux wrote: > On Tue, Jan 29, 2013 at 10:34:53AM -0500, Alan Stern wrote: > > On Mon, 28 Jan 2013, Russell King - ARM Linux wrote: > > > > > On Mon, Jan 28, 2013 at 10:17:33AM -0500, Alan Stern wrote: > > > > On Mon, 28 Jan 2013, Roger Quadros wrote: > > > > > > > > > Make use of devm_request_and_ioremap() and correct comment. > > > > > > > > Didn't a big patch come through recently converting all usages of > > > > devm_request_and_ioremap() to another function (I forget the name) that > > > > does its own error reporting and returns an ERR_PTR value? > > > > > > > > (Checks the mailing list archive...) Ah, here it is: > > > > > > > > http://marc.info/?l=linux-usb&m=135876311801537&w=2 > > > > > > > > And the new function is devm_ioremap_resource(). We should avoid > > > > adding new usages of an old interface. > > > > > > Maybe... if devm_ioremap_resource() was already in the kernel. The > > > problem at the moment is it isn't, and it'll probably be rather > > > horrid for everyone to deal with especially when it comes to testing. > > > > Not in the kernel yet? I didn't realize that. Looks like Thierry > > Reding will have some clean-up work to do when it does get in. > > It may be in linux-next, but it isn't in Linus' yet... (of course, it's > probably scheduled for the upcoming merge window). Certainly: > > $ grep -r devm_ioremap_resource include > > against Linus' tree (6abb7c2) still turns up nothing. It's in my driver-core-next tree and will be going to Linus for 3.9-rc1. thanks, greg k-h
On Mon, Jan 28, 2013 at 01:30:10PM +0200, Roger Quadros wrote: > Make use of devm_request_and_ioremap() and correct comment. > > Signed-off-by: Roger Quadros <rogerq@ti.com> Acked-by: Felipe Balbi <balbi@ti.com> > --- > drivers/usb/host/ehci-omap.c | 19 +++++-------------- > 1 files changed, 5 insertions(+), 14 deletions(-) > > diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c > index 30fc482..fd2f5450 100644 > --- a/drivers/usb/host/ehci-omap.c > +++ b/drivers/usb/host/ehci-omap.c > @@ -216,23 +216,17 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) > > res = platform_get_resource_byname(pdev, > IORESOURCE_MEM, "ehci"); > - if (!res) { > - dev_err(dev, "UHH EHCI get resource failed\n"); > - return -ENODEV; > - } > - > - regs = ioremap(res->start, resource_size(res)); > + regs = devm_request_and_ioremap(dev, res); > if (!regs) { > - dev_err(dev, "UHH EHCI ioremap failed\n"); > - return -ENOMEM; > + dev_err(dev, "Resource request/ioremap failed\n"); > + return -EADDRNOTAVAIL; > } > > hcd = usb_create_hcd(&ehci_omap_hc_driver, dev, > dev_name(dev)); > if (!hcd) { > - dev_err(dev, "failed to create hcd with err %d\n", ret); > - ret = -ENOMEM; > - goto err_io; > + dev_err(dev, "Failed to create HCD\n"); > + return -ENOMEM; > } > > hcd->rsrc_start = res->start; > @@ -285,8 +279,6 @@ err_pm_runtime: > pm_runtime_put_sync(dev); > usb_put_hcd(hcd); > > -err_io: > - iounmap(regs); > return ret; > } > > @@ -306,7 +298,6 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev) > > usb_remove_hcd(hcd); > disable_put_regulator(dev->platform_data); > - iounmap(hcd->regs); > usb_put_hcd(hcd); > > pm_runtime_put_sync(dev); > -- > 1.7.4.1 >
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c index 30fc482..fd2f5450 100644 --- a/drivers/usb/host/ehci-omap.c +++ b/drivers/usb/host/ehci-omap.c @@ -216,23 +216,17 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ehci"); - if (!res) { - dev_err(dev, "UHH EHCI get resource failed\n"); - return -ENODEV; - } - - regs = ioremap(res->start, resource_size(res)); + regs = devm_request_and_ioremap(dev, res); if (!regs) { - dev_err(dev, "UHH EHCI ioremap failed\n"); - return -ENOMEM; + dev_err(dev, "Resource request/ioremap failed\n"); + return -EADDRNOTAVAIL; } hcd = usb_create_hcd(&ehci_omap_hc_driver, dev, dev_name(dev)); if (!hcd) { - dev_err(dev, "failed to create hcd with err %d\n", ret); - ret = -ENOMEM; - goto err_io; + dev_err(dev, "Failed to create HCD\n"); + return -ENOMEM; } hcd->rsrc_start = res->start; @@ -285,8 +279,6 @@ err_pm_runtime: pm_runtime_put_sync(dev); usb_put_hcd(hcd); -err_io: - iounmap(regs); return ret; } @@ -306,7 +298,6 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev) usb_remove_hcd(hcd); disable_put_regulator(dev->platform_data); - iounmap(hcd->regs); usb_put_hcd(hcd); pm_runtime_put_sync(dev);
Make use of devm_request_and_ioremap() and correct comment. Signed-off-by: Roger Quadros <rogerq@ti.com> --- drivers/usb/host/ehci-omap.c | 19 +++++-------------- 1 files changed, 5 insertions(+), 14 deletions(-)