devm_ioremap_resource often uses the result of a call to
platform_get_resource_byname as its last argument. devm_ioremap_resource
does appropriate error handling on this argument, so error handling can be
removed from the call to platform_get_resource_byname.
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression pdev,res,n,e,e1;
expression ret != 0;
identifier l,f;
expression list es;
@@
(
res = f(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
e = devm_ioremap_resource(e1, res);
|
- res = f(es);
... when != res
- if (res == NULL) { ... \(goto l;\|return ret;\) }
... when != res
+ res = f(es);
e = devm_ioremap_resource(e1, res);
)
// </smpl>
In practice, f is always platform_get_resource_byname (or
platform_get_resource, which was handled by a previous patch series). And
the call to platform_get_resource_byname always immediately precedes the
call to devm_ioremap_resource.
From: Julia Lawall <[email protected]>
Remove unneeded error handling on the result of a call to
platform_get_resource_byname when the value is passed to devm_ioremap_resource.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression pdev,res,e,e1;
expression ret != 0;
identifier l;
@@
res = platform_get_resource_byname(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
e = devm_ioremap_resource(e1, res);
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
drivers/mtd/nand/fsmc_nand.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 2a3b1b9..3dc1a75 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -958,9 +958,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
}
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nand_data");
- if (!res)
- return -EINVAL;
-
host->data_va = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(host->data_va))
return PTR_ERR(host->data_va);
@@ -968,25 +965,16 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
host->data_pa = (dma_addr_t)res->start;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nand_addr");
- if (!res)
- return -EINVAL;
-
host->addr_va = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(host->addr_va))
return PTR_ERR(host->addr_va);
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nand_cmd");
- if (!res)
- return -EINVAL;
-
host->cmd_va = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(host->cmd_va))
return PTR_ERR(host->cmd_va);
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fsmc_regs");
- if (!res)
- return -EINVAL;
-
host->regs_va = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(host->regs_va))
return PTR_ERR(host->regs_va);
From: Julia Lawall <[email protected]>
Remove unneeded error handling on the result of a call to
platform_get_resource_byname when the value is passed to devm_ioremap_resource.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression pdev,res,e,e1;
expression ret != 0;
identifier l;
@@
res = platform_get_resource_byname(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
e = devm_ioremap_resource(e1, res);
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
drivers/regulator/ti-abb-regulator.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c
index 3753ed0..d8e3e12 100644
--- a/drivers/regulator/ti-abb-regulator.c
+++ b/drivers/regulator/ti-abb-regulator.c
@@ -717,11 +717,6 @@ static int ti_abb_probe(struct platform_device *pdev)
/* Map ABB resources */
pname = "base-address";
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
- if (!res) {
- dev_err(dev, "Missing '%s' IO resource\n", pname);
- ret = -ENODEV;
- goto err;
- }
abb->base = devm_ioremap_resource(dev, res);
if (IS_ERR(abb->base)) {
ret = PTR_ERR(abb->base);
@@ -770,11 +765,6 @@ static int ti_abb_probe(struct platform_device *pdev)
pname = "ldo-address";
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
- if (!res) {
- dev_dbg(dev, "Missing '%s' IO resource\n", pname);
- ret = -ENODEV;
- goto skip_opt;
- }
abb->ldo_base = devm_ioremap_resource(dev, res);
if (IS_ERR(abb->ldo_base)) {
ret = PTR_ERR(abb->ldo_base);
From: Julia Lawall <[email protected]>
Remove unneeded error handling on the result of a call to
platform_get_resource_byname when the value is passed to devm_ioremap_resource.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression pdev,res,e,e1;
expression ret != 0;
identifier l;
@@
res = platform_get_resource_byname(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
e = devm_ioremap_resource(e1, res);
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
arch/mips/pci/pci-ar71xx.c | 3 ---
arch/mips/pci/pci-ar724x.c | 9 ---------
2 files changed, 12 deletions(-)
diff --git a/arch/mips/pci/pci-ar71xx.c b/arch/mips/pci/pci-ar71xx.c
index 18517dd..d471a26 100644
--- a/arch/mips/pci/pci-ar71xx.c
+++ b/arch/mips/pci/pci-ar71xx.c
@@ -363,9 +363,6 @@ static int ar71xx_pci_probe(struct platform_device *pdev)
spin_lock_init(&apc->lock);
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg_base");
- if (!res)
- return -EINVAL;
-
apc->cfg_base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(apc->cfg_base))
return PTR_ERR(apc->cfg_base);
diff --git a/arch/mips/pci/pci-ar724x.c b/arch/mips/pci/pci-ar724x.c
index 65ec032..785b265 100644
--- a/arch/mips/pci/pci-ar724x.c
+++ b/arch/mips/pci/pci-ar724x.c
@@ -362,25 +362,16 @@ static int ar724x_pci_probe(struct platform_device *pdev)
return -ENOMEM;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctrl_base");
- if (!res)
- return -EINVAL;
-
apc->ctrl_base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(apc->ctrl_base))
return PTR_ERR(apc->ctrl_base);
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg_base");
- if (!res)
- return -EINVAL;
-
apc->devcfg_base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(apc->devcfg_base))
return PTR_ERR(apc->devcfg_base);
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "crp_base");
- if (!res)
- return -EINVAL;
-
apc->crp_base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(apc->crp_base))
return PTR_ERR(apc->crp_base);
From: Julia Lawall <[email protected]>
Remove unneeded error handling on the result of a call to
platform_get_resource_byname when the value is passed to devm_ioremap_resource.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression pdev,res,e,e1;
expression ret != 0;
identifier l;
@@
res = platform_get_resource_byname(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
e = devm_ioremap_resource(e1, res);
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
drivers/usb/musb/musb_dsps.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 4ffbaac..f2f9710 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -357,9 +357,6 @@ static int dsps_musb_init(struct musb *musb)
u32 rev, val;
r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
- if (!r)
- return -EINVAL;
-
reg_base = devm_ioremap_resource(dev, r);
if (!musb->ctrl_base)
return -EINVAL;
From: Julia Lawall <[email protected]>
Remove unneeded error handling on the result of a call to
platform_get_resource_byname when the value is passed to devm_ioremap_resource.
In the case of omap-dmic.c, the error-handling code of
devm_ioremap_resource is also corrected to include releasing the clock.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression pdev,res,e,e1;
expression ret != 0;
identifier l;
@@
res = platform_get_resource_byname(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
e = devm_ioremap_resource(e1, res);
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
sound/soc/omap/omap-dmic.c | 9 +++------
sound/soc/omap/omap-mcpdm.c | 3 ---
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/sound/soc/omap/omap-dmic.c b/sound/soc/omap/omap-dmic.c
index 4db1f8e..12e566b 100644
--- a/sound/soc/omap/omap-dmic.c
+++ b/sound/soc/omap/omap-dmic.c
@@ -480,15 +480,12 @@ static int asoc_dmic_probe(struct platform_device *pdev)
dmic->dma_data.filter_data = "up_link";
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mpu");
- if (!res) {
- dev_err(dmic->dev, "invalid memory resource\n");
- ret = -ENODEV;
+ dmic->io_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(dmic->io_base)) {
+ ret = PTR_ERR(dmic->io_base);
goto err_put_clk;
}
- dmic->io_base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(dmic->io_base))
- return PTR_ERR(dmic->io_base);
ret = snd_soc_register_component(&pdev->dev, &omap_dmic_component,
&omap_dmic_dai, 1);
diff --git a/sound/soc/omap/omap-mcpdm.c b/sound/soc/omap/omap-mcpdm.c
index a49dc52..90d2a7c 100644
--- a/sound/soc/omap/omap-mcpdm.c
+++ b/sound/soc/omap/omap-mcpdm.c
@@ -480,9 +480,6 @@ static int asoc_mcpdm_probe(struct platform_device *pdev)
mcpdm->dma_data[1].filter_data = "up_link";
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mpu");
- if (res == NULL)
- return -ENOMEM;
-
mcpdm->io_base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(mcpdm->io_base))
return PTR_ERR(mcpdm->io_base);
From: Julia Lawall <[email protected]>
Remove unneeded error handling on the result of a call to
platform_get_resource_byname when the value is passed to devm_ioremap_resource.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression pdev,res,e,e1;
expression ret != 0;
identifier l;
@@
res = platform_get_resource_byname(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
e = devm_ioremap_resource(e1, res);
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
drivers/watchdog/ar7_wdt.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/watchdog/ar7_wdt.c b/drivers/watchdog/ar7_wdt.c
index 2f3cc8f..b3709f9 100644
--- a/drivers/watchdog/ar7_wdt.c
+++ b/drivers/watchdog/ar7_wdt.c
@@ -280,11 +280,6 @@ static int ar7_wdt_probe(struct platform_device *pdev)
ar7_regs_wdt =
platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
- if (!ar7_regs_wdt) {
- pr_err("could not get registers resource\n");
- return -ENODEV;
- }
-
ar7_wdt = devm_ioremap_resource(&pdev->dev, ar7_regs_wdt);
if (IS_ERR(ar7_wdt))
return PTR_ERR(ar7_wdt);
Am 19.08.2013 10:51, schrieb Julia Lawall:
> From: Julia Lawall <[email protected]>
>
> Remove unneeded error handling on the result of a call to
> platform_get_resource_byname when the value is passed to devm_ioremap_resource.
>
> A simplified version of the semantic patch that makes this change is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression pdev,res,e,e1;
> expression ret != 0;
> identifier l;
> @@
>
> res = platform_get_resource_byname(...);
> - if (res == NULL) { ... \(goto l;\|return ret;\) }
> e = devm_ioremap_resource(e1, res);
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>
>
> ---
> drivers/regulator/ti-abb-regulator.c | 10 ----------
> 1 file changed, 10 deletions(-)
>
> diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c
> index 3753ed0..d8e3e12 100644
> --- a/drivers/regulator/ti-abb-regulator.c
> +++ b/drivers/regulator/ti-abb-regulator.c
> @@ -717,11 +717,6 @@ static int ti_abb_probe(struct platform_device *pdev)
> /* Map ABB resources */
> pname = "base-address";
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
> - if (!res) {
> - dev_err(dev, "Missing '%s' IO resource\n", pname);
> - ret = -ENODEV;
> - goto err;
> - }
> abb->base = devm_ioremap_resource(dev, res);
> if (IS_ERR(abb->base)) {
> ret = PTR_ERR(abb->base);
is pname used by anything else ? (also below)
re,
wh
> @@ -770,11 +765,6 @@ static int ti_abb_probe(struct platform_device *pdev)
>
> pname = "ldo-address";
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
> - if (!res) {
> - dev_dbg(dev, "Missing '%s' IO resource\n", pname);
> - ret = -ENODEV;
> - goto skip_opt;
> - }
> abb->ldo_base = devm_ioremap_resource(dev, res);
> if (IS_ERR(abb->ldo_base)) {
> ret = PTR_ERR(abb->ldo_base);
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Mon, 19 Aug 2013, walter harms wrote:
>
>
> Am 19.08.2013 10:51, schrieb Julia Lawall:
> > From: Julia Lawall <[email protected]>
> >
> > Remove unneeded error handling on the result of a call to
> > platform_get_resource_byname when the value is passed to devm_ioremap_resource.
> >
> > A simplified version of the semantic patch that makes this change is as
> > follows: (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression pdev,res,e,e1;
> > expression ret != 0;
> > identifier l;
> > @@
> >
> > res = platform_get_resource_byname(...);
> > - if (res == NULL) { ... \(goto l;\|return ret;\) }
> > e = devm_ioremap_resource(e1, res);
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <[email protected]>
> >
> > ---
> > drivers/regulator/ti-abb-regulator.c | 10 ----------
> > 1 file changed, 10 deletions(-)
> >
> > diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c
> > index 3753ed0..d8e3e12 100644
> > --- a/drivers/regulator/ti-abb-regulator.c
> > +++ b/drivers/regulator/ti-abb-regulator.c
> > @@ -717,11 +717,6 @@ static int ti_abb_probe(struct platform_device *pdev)
> > /* Map ABB resources */
> > pname = "base-address";
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
> > - if (!res) {
> > - dev_err(dev, "Missing '%s' IO resource\n", pname);
> > - ret = -ENODEV;
> > - goto err;
> > - }
> > abb->base = devm_ioremap_resource(dev, res);
> > if (IS_ERR(abb->base)) {
> > ret = PTR_ERR(abb->base);
>
>
> is pname used by anything else ? (also below)
I'll check. Thanks for the feedback.
julia
>
> re,
> wh
>
> > @@ -770,11 +765,6 @@ static int ti_abb_probe(struct platform_device *pdev)
> >
> > pname = "ldo-address";
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
> > - if (!res) {
> > - dev_dbg(dev, "Missing '%s' IO resource\n", pname);
> > - ret = -ENODEV;
> > - goto skip_opt;
> > - }
> > abb->ldo_base = devm_ioremap_resource(dev, res);
> > if (IS_ERR(abb->ldo_base)) {
> > ret = PTR_ERR(abb->ldo_base);
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Mon, 19 Aug 2013, walter harms wrote:
>
>
> Am 19.08.2013 10:51, schrieb Julia Lawall:
> > From: Julia Lawall <[email protected]>
> >
> > Remove unneeded error handling on the result of a call to
> > platform_get_resource_byname when the value is passed to devm_ioremap_resource.
> >
> > A simplified version of the semantic patch that makes this change is as
> > follows: (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression pdev,res,e,e1;
> > expression ret != 0;
> > identifier l;
> > @@
> >
> > res = platform_get_resource_byname(...);
> > - if (res == NULL) { ... \(goto l;\|return ret;\) }
> > e = devm_ioremap_resource(e1, res);
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <[email protected]>
> >
> > ---
> > drivers/regulator/ti-abb-regulator.c | 10 ----------
> > 1 file changed, 10 deletions(-)
> >
> > diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c
> > index 3753ed0..d8e3e12 100644
> > --- a/drivers/regulator/ti-abb-regulator.c
> > +++ b/drivers/regulator/ti-abb-regulator.c
> > @@ -717,11 +717,6 @@ static int ti_abb_probe(struct platform_device *pdev)
> > /* Map ABB resources */
> > pname = "base-address";
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
> > - if (!res) {
> > - dev_err(dev, "Missing '%s' IO resource\n", pname);
> > - ret = -ENODEV;
> > - goto err;
> > - }
> > abb->base = devm_ioremap_resource(dev, res);
> > if (IS_ERR(abb->base)) {
> > ret = PTR_ERR(abb->base);
>
>
> is pname used by anything else ? (also below)
I'm not sure to understand the sense of the question. My patch does
remove a use of pname, but there is another one on the line above, in the
call to platform_get_resource_byname. Perhaps the definition of pname
could be inlined at this use, but it is a common pattern throughout the
function.
julia
The function affected by this patch also calls devm_ioremap_nocache twice,
with no form of request_mem_region. I wonder if these calls should also
use devm_ioremap_resource?
julia
On Mon, 19 Aug 2013, walter harms wrote:
>
>
> Am 19.08.2013 10:51, schrieb Julia Lawall:
> > From: Julia Lawall <[email protected]>
> >
> > Remove unneeded error handling on the result of a call to
> > platform_get_resource_byname when the value is passed to devm_ioremap_resource.
> >
> > A simplified version of the semantic patch that makes this change is as
> > follows: (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression pdev,res,e,e1;
> > expression ret != 0;
> > identifier l;
> > @@
> >
> > res = platform_get_resource_byname(...);
> > - if (res == NULL) { ... \(goto l;\|return ret;\) }
> > e = devm_ioremap_resource(e1, res);
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <[email protected]>
> >
> > ---
> > drivers/regulator/ti-abb-regulator.c | 10 ----------
> > 1 file changed, 10 deletions(-)
> >
> > diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c
> > index 3753ed0..d8e3e12 100644
> > --- a/drivers/regulator/ti-abb-regulator.c
> > +++ b/drivers/regulator/ti-abb-regulator.c
> > @@ -717,11 +717,6 @@ static int ti_abb_probe(struct platform_device *pdev)
> > /* Map ABB resources */
> > pname = "base-address";
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
> > - if (!res) {
> > - dev_err(dev, "Missing '%s' IO resource\n", pname);
> > - ret = -ENODEV;
> > - goto err;
> > - }
> > abb->base = devm_ioremap_resource(dev, res);
> > if (IS_ERR(abb->base)) {
> > ret = PTR_ERR(abb->base);
>
>
> is pname used by anything else ? (also below)
>
> re,
> wh
>
> > @@ -770,11 +765,6 @@ static int ti_abb_probe(struct platform_device *pdev)
> >
> > pname = "ldo-address";
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
> > - if (!res) {
> > - dev_dbg(dev, "Missing '%s' IO resource\n", pname);
> > - ret = -ENODEV;
> > - goto skip_opt;
> > - }
> > abb->ldo_base = devm_ioremap_resource(dev, res);
> > if (IS_ERR(abb->ldo_base)) {
> > ret = PTR_ERR(abb->ldo_base);
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Am 19.08.2013 12:12, schrieb Julia Lawall:
> On Mon, 19 Aug 2013, walter harms wrote:
>
>>
>>
>> Am 19.08.2013 10:51, schrieb Julia Lawall:
>>> From: Julia Lawall <[email protected]>
>>>
>>> Remove unneeded error handling on the result of a call to
>>> platform_get_resource_byname when the value is passed to devm_ioremap_resource.
>>>
>>> A simplified version of the semantic patch that makes this change is as
>>> follows: (http://coccinelle.lip6.fr/)
>>>
>>> // <smpl>
>>> @@
>>> expression pdev,res,e,e1;
>>> expression ret != 0;
>>> identifier l;
>>> @@
>>>
>>> res = platform_get_resource_byname(...);
>>> - if (res == NULL) { ... \(goto l;\|return ret;\) }
>>> e = devm_ioremap_resource(e1, res);
>>> // </smpl>
>>>
>>> Signed-off-by: Julia Lawall <[email protected]>
>>>
>>> ---
>>> drivers/regulator/ti-abb-regulator.c | 10 ----------
>>> 1 file changed, 10 deletions(-)
>>>
>>> diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c
>>> index 3753ed0..d8e3e12 100644
>>> --- a/drivers/regulator/ti-abb-regulator.c
>>> +++ b/drivers/regulator/ti-abb-regulator.c
>>> @@ -717,11 +717,6 @@ static int ti_abb_probe(struct platform_device *pdev)
>>> /* Map ABB resources */
>>> pname = "base-address";
>>> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
>>> - if (!res) {
>>> - dev_err(dev, "Missing '%s' IO resource\n", pname);
>>> - ret = -ENODEV;
>>> - goto err;
>>> - }
>>> abb->base = devm_ioremap_resource(dev, res);
>>> if (IS_ERR(abb->base)) {
>>> ret = PTR_ERR(abb->base);
>>
>>
>> is pname used by anything else ? (also below)
>
> I'm not sure to understand the sense of the question. My patch does
> remove a use of pname, but there is another one on the line above, in the
> call to platform_get_resource_byname. Perhaps the definition of pname
> could be inlined at this use, but it is a common pattern throughout the
> function.
>
in other functions this looks like that:
r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
i did not dive into the driver but from the part i see makeing this into:
es = platform_get_resource_byname(pdev, IORESOURCE_MEM,"base-address");
would render pname obsolete.
re,
wh
On Mon, 19 Aug 2013, walter harms wrote:
>
>
> Am 19.08.2013 12:12, schrieb Julia Lawall:
> > On Mon, 19 Aug 2013, walter harms wrote:
> >
> >>
> >>
> >> Am 19.08.2013 10:51, schrieb Julia Lawall:
> >>> From: Julia Lawall <[email protected]>
> >>>
> >>> Remove unneeded error handling on the result of a call to
> >>> platform_get_resource_byname when the value is passed to devm_ioremap_resource.
> >>>
> >>> A simplified version of the semantic patch that makes this change is as
> >>> follows: (http://coccinelle.lip6.fr/)
> >>>
> >>> // <smpl>
> >>> @@
> >>> expression pdev,res,e,e1;
> >>> expression ret != 0;
> >>> identifier l;
> >>> @@
> >>>
> >>> res = platform_get_resource_byname(...);
> >>> - if (res == NULL) { ... \(goto l;\|return ret;\) }
> >>> e = devm_ioremap_resource(e1, res);
> >>> // </smpl>
> >>>
> >>> Signed-off-by: Julia Lawall <[email protected]>
> >>>
> >>> ---
> >>> drivers/regulator/ti-abb-regulator.c | 10 ----------
> >>> 1 file changed, 10 deletions(-)
> >>>
> >>> diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c
> >>> index 3753ed0..d8e3e12 100644
> >>> --- a/drivers/regulator/ti-abb-regulator.c
> >>> +++ b/drivers/regulator/ti-abb-regulator.c
> >>> @@ -717,11 +717,6 @@ static int ti_abb_probe(struct platform_device *pdev)
> >>> /* Map ABB resources */
> >>> pname = "base-address";
> >>> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
> >>> - if (!res) {
> >>> - dev_err(dev, "Missing '%s' IO resource\n", pname);
> >>> - ret = -ENODEV;
> >>> - goto err;
> >>> - }
> >>> abb->base = devm_ioremap_resource(dev, res);
> >>> if (IS_ERR(abb->base)) {
> >>> ret = PTR_ERR(abb->base);
> >>
> >>
> >> is pname used by anything else ? (also below)
> >
> > I'm not sure to understand the sense of the question. My patch does
> > remove a use of pname, but there is another one on the line above, in the
> > call to platform_get_resource_byname. Perhaps the definition of pname
> > could be inlined at this use, but it is a common pattern throughout the
> > function.
> >
> in other functions this looks like that:
> r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
>
> i did not dive into the driver but from the part i see makeing this into:
>
> es = platform_get_resource_byname(pdev, IORESOURCE_MEM,"base-address");
>
> would render pname obsolete.
This function always uses pname for platform_get_resource_byname (4 calls)
and of_property_read_u32 (2 calls). So perhaps it is better to leave it
as is.
julia
On 19-08-2013 10:51, Julia Lawall wrote:
> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> index 4ffbaac..f2f9710 100644
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -357,9 +357,6 @@ static int dsps_musb_init(struct musb *musb)
> u32 rev, val;
>
> r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
> - if (!r)
> - return -EINVAL;
> -
> reg_base = devm_ioremap_resource(dev, r);
> if (!musb->ctrl_base)
> return -EINVAL;
Not really related to Julia's patch, apart from making it more obvious
that there's a bug here.
I believe it is reg_base that needs to be checked, right?
Svenning
On Mon, 19 Aug 2013, Svenning S?rensen wrote:
> On 19-08-2013 10:51, Julia Lawall wrote:
> > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> > index 4ffbaac..f2f9710 100644
> > --- a/drivers/usb/musb/musb_dsps.c
> > +++ b/drivers/usb/musb/musb_dsps.c
> > @@ -357,9 +357,6 @@ static int dsps_musb_init(struct musb *musb)
> > u32 rev, val;
> > r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
> > - if (!r)
> > - return -EINVAL;
> > -
> > reg_base = devm_ioremap_resource(dev, r);
> > if (!musb->ctrl_base)
> > return -EINVAL;
> Not really related to Julia's patch, apart from making it more obvious that
> there's a bug here.
> I believe it is reg_base that needs to be checked, right?
Indeed, it is all backwards. I could extend the patch, if you want.
julia
From: Julia Lawall <[email protected]>
Remove unneeded error handling on the result of a call to
platform_get_resource_byname when the value is passed to
devm_ioremap_resource.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression pdev,res,e,e1;
expression ret != 0;
identifier l;
@@
res = platform_get_resource_byname(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
e = devm_ioremap_resource(e1, res);
// </smpl>
This patch also adjusts the error-handling code on the call to
devm_ioremap_resource to check the right value, noticed by Svenning S?rensen.
Signed-off-by: Julia Lawall <[email protected]>
---
v2: Fixed the bug on the test of the result of devm_ioremap_resource
drivers/usb/musb/musb_dsps.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 4ffbaac..e60be6f 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -357,12 +357,9 @@ static int dsps_musb_init(struct musb *musb)
u32 rev, val;
r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
- if (!r)
- return -EINVAL;
-
reg_base = devm_ioremap_resource(dev, r);
- if (!musb->ctrl_base)
- return -EINVAL;
+ if (IS_ERR(reg_base))
+ return PTR_ERR(reg_base);
musb->ctrl_base = reg_base;
/* NOP driver needs change if supporting dual instance */
On 19-08-2013 13:35, Julia Lawall wrote:
> reg_base = devm_ioremap_resource(dev, r);
> if (!musb->ctrl_base)
> return -EINVAL;
>> Not really related to Julia's patch, apart from making it more obvious that
>> there's a bug here.
>> I believe it is reg_base that needs to be checked, right?
> Indeed, it is all backwards. I could extend the patch, if you want.
>
I'll let Felipe decide on that, but I can't imagine any objections.
IS_ERR() is the proper test, of course :)
Svenning
On Mon, 19 Aug 2013, Svenning S?rensen wrote:
>
> On 19-08-2013 13:35, Julia Lawall wrote:
> > reg_base = devm_ioremap_resource(dev, r);
> > if (!musb->ctrl_base)
> > return -EINVAL;
> > > Not really related to Julia's patch, apart from making it more obvious
> > > that
> > > there's a bug here.
> > > I believe it is reg_base that needs to be checked, right?
> > Indeed, it is all backwards. I could extend the patch, if you want.
> >
> I'll let Felipe decide on that, but I can't imagine any objections.
> IS_ERR() is the proper test, of course :)
I've already sent a patch, in case it is useful.
julia
On 08/19/2013 01:51 AM, Julia Lawall wrote:
> From: Julia Lawall <[email protected]>
>
> Remove unneeded error handling on the result of a call to
> platform_get_resource_byname when the value is passed to devm_ioremap_resource.
>
> A simplified version of the semantic patch that makes this change is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression pdev,res,e,e1;
> expression ret != 0;
> identifier l;
> @@
>
> res = platform_get_resource_byname(...);
> - if (res == NULL) { ... \(goto l;\|return ret;\) }
> e = devm_ioremap_resource(e1, res);
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>
>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/ar7_wdt.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/watchdog/ar7_wdt.c b/drivers/watchdog/ar7_wdt.c
> index 2f3cc8f..b3709f9 100644
> --- a/drivers/watchdog/ar7_wdt.c
> +++ b/drivers/watchdog/ar7_wdt.c
> @@ -280,11 +280,6 @@ static int ar7_wdt_probe(struct platform_device *pdev)
>
> ar7_regs_wdt =
> platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> - if (!ar7_regs_wdt) {
> - pr_err("could not get registers resource\n");
> - return -ENODEV;
> - }
> -
> ar7_wdt = devm_ioremap_resource(&pdev->dev, ar7_regs_wdt);
> if (IS_ERR(ar7_wdt))
> return PTR_ERR(ar7_wdt);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
On Mon, 2013-08-19 at 10:51 +0200, Julia Lawall wrote:
> From: Julia Lawall <[email protected]>
>
> Remove unneeded error handling on the result of a call to
> platform_get_resource_byname when the value is passed to devm_ioremap_resource.
>
> A simplified version of the semantic patch that makes this change is as
> follows: (http://coccinelle.lip6.fr/)
Pushed to l2-mtd.git, thanks.
--
Best Regards,
Artem Bityutskiy
Hello.
On 08/19/2013 03:47 PM, Julia Lawall wrote:
> From: Julia Lawall <[email protected]>
> Remove unneeded error handling on the result of a call to
> platform_get_resource_byname when the value is passed to
> devm_ioremap_resource.
> A simplified version of the semantic patch that makes this change is as
> follows: (http://coccinelle.lip6.fr/)
> // <smpl>
> @@
> expression pdev,res,e,e1;
> expression ret != 0;
> identifier l;
> @@
>
> res = platform_get_resource_byname(...);
> - if (res == NULL) { ... \(goto l;\|return ret;\) }
> e = devm_ioremap_resource(e1, res);
> // </smpl>
> This patch also adjusts the error-handling code on the call to
> devm_ioremap_resource to check the right value, noticed by Svenning S?rensen.
Saying "also" in the changelog is usuallly a good sign the patch should be
split.
> Signed-off-by: Julia Lawall <[email protected]>
> ---
> v2: Fixed the bug on the test of the result of devm_ioremap_resource
> drivers/usb/musb/musb_dsps.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> index 4ffbaac..e60be6f 100644
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -357,12 +357,9 @@ static int dsps_musb_init(struct musb *musb)
> u32 rev, val;
>
> r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
> - if (!r)
> - return -EINVAL;
> -
> reg_base = devm_ioremap_resource(dev, r);
> - if (!musb->ctrl_base)
> - return -EINVAL;
> + if (IS_ERR(reg_base))
> + return PTR_ERR(reg_base);
This is indented with space, not tabs. And of course, this was a matter of
a separate *fix* patch, while the original patch was a *cleanup*.
WBR, Sergei
On Mon, 19 Aug 2013, Sergei Shtylyov wrote:
> Hello.
>
> On 08/19/2013 03:47 PM, Julia Lawall wrote:
>
> > From: Julia Lawall <[email protected]>
>
> > Remove unneeded error handling on the result of a call to
> > platform_get_resource_byname when the value is passed to
> > devm_ioremap_resource.
>
> > A simplified version of the semantic patch that makes this change is as
> > follows: (http://coccinelle.lip6.fr/)
>
> > // <smpl>
> > @@
> > expression pdev,res,e,e1;
> > expression ret != 0;
> > identifier l;
> > @@
> >
> > res = platform_get_resource_byname(...);
> > - if (res == NULL) { ... \(goto l;\|return ret;\) }
> > e = devm_ioremap_resource(e1, res);
> > // </smpl>
>
> > This patch also adjusts the error-handling code on the call to
> > devm_ioremap_resource to check the right value, noticed by Svenning
> > S?rensen.
>
> Saying "also" in the changelog is usuallly a good sign the patch should be
> split.
OK, the original patch already does the first part. I will send a new
patch for the bug fix.
julia
>
> > Signed-off-by: Julia Lawall <[email protected]>
>
> > ---
> > v2: Fixed the bug on the test of the result of devm_ioremap_resource
>
> > drivers/usb/musb/musb_dsps.c | 7 ++-----
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> > index 4ffbaac..e60be6f 100644
> > --- a/drivers/usb/musb/musb_dsps.c
> > +++ b/drivers/usb/musb/musb_dsps.c
> > @@ -357,12 +357,9 @@ static int dsps_musb_init(struct musb *musb)
> > u32 rev, val;
> >
> > r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
> > - if (!r)
> > - return -EINVAL;
> > -
> > reg_base = devm_ioremap_resource(dev, r);
> > - if (!musb->ctrl_base)
> > - return -EINVAL;
> > + if (IS_ERR(reg_base))
> > + return PTR_ERR(reg_base);
>
> This is indented with space, not tabs. And of course, this was a matter of
> a separate *fix* patch, while the original patch was a *cleanup*.
>
> WBR, Sergei
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Mon, Aug 19, 2013 at 06:29:37PM +0400, Sergei Shtylyov wrote:
> Hello.
>
> On 08/19/2013 03:47 PM, Julia Lawall wrote:
>
> >From: Julia Lawall <[email protected]>
>
> >Remove unneeded error handling on the result of a call to
> >platform_get_resource_byname when the value is passed to
> >devm_ioremap_resource.
>
> >A simplified version of the semantic patch that makes this change is as
> >follows: (http://coccinelle.lip6.fr/)
>
> >// <smpl>
> >@@
> >expression pdev,res,e,e1;
> >expression ret != 0;
> >identifier l;
> >@@
> >
> > res = platform_get_resource_byname(...);
> >- if (res == NULL) { ... \(goto l;\|return ret;\) }
> > e = devm_ioremap_resource(e1, res);
> >// </smpl>
>
> >This patch also adjusts the error-handling code on the call to
> >devm_ioremap_resource to check the right value, noticed by Svenning S?rensen.
>
> Saying "also" in the changelog is usuallly a good sign the patch
> should be split.
>
> >Signed-off-by: Julia Lawall <[email protected]>
>
> >---
> >v2: Fixed the bug on the test of the result of devm_ioremap_resource
>
> > drivers/usb/musb/musb_dsps.c | 7 ++-----
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> >diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> >index 4ffbaac..e60be6f 100644
> >--- a/drivers/usb/musb/musb_dsps.c
> >+++ b/drivers/usb/musb/musb_dsps.c
> >@@ -357,12 +357,9 @@ static int dsps_musb_init(struct musb *musb)
> > u32 rev, val;
> >
> > r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
> >- if (!r)
> >- return -EINVAL;
> >-
> > reg_base = devm_ioremap_resource(dev, r);
> >- if (!musb->ctrl_base)
> >- return -EINVAL;
> >+ if (IS_ERR(reg_base))
> >+ return PTR_ERR(reg_base);
>
> This is indented with space, not tabs. And of course, this was a
> matter of a separate *fix* patch, while the original patch was a
> *cleanup*.
>
I would say this falls under the "trivial and closely related"
banner. But I would prefer if the changelog said "Fixed a bug,
also made some cleanups." Especially I wish the subject mentioned
the bugfix.
regards,
dan carpenter
Hi
On 08/19/2013 11:51 AM, Julia Lawall wrote:
> From: Julia Lawall <[email protected]>
>
> Remove unneeded error handling on the result of a call to
> platform_get_resource_byname when the value is passed to devm_ioremap_resource.
>
> In the case of omap-dmic.c, the error-handling code of
> devm_ioremap_resource is also corrected to include releasing the clock.
>
> A simplified version of the semantic patch that makes this change is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression pdev,res,e,e1;
> expression ret != 0;
> identifier l;
> @@
>
> res = platform_get_resource_byname(...);
> - if (res == NULL) { ... \(goto l;\|return ret;\) }
> e = devm_ioremap_resource(e1, res);
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>
>
> ---
> sound/soc/omap/omap-dmic.c | 9 +++------
> sound/soc/omap/omap-mcpdm.c | 3 ---
> 2 files changed, 3 insertions(+), 9 deletions(-)
>
To the patch and catch of missing clock release in omap-dmic.c in case
of failing devm_ioremap_resource:
Acked-by: Jarkko Nikula <[email protected]>
On Mon, Aug 19, 2013 at 10:51:51AM +0200, Julia Lawall wrote:
> From: Julia Lawall <[email protected]>
>
> Remove unneeded error handling on the result of a call to
> platform_get_resource_byname when the value is passed to devm_ioremap_resource.
Applied, thanks.
On Mon, Aug 19, 2013 at 10:51:55AM +0200, Julia Lawall wrote:
> From: Julia Lawall <[email protected]>
>
> Remove unneeded error handling on the result of a call to
> platform_get_resource_byname when the value is passed to devm_ioremap_resource.
Applied, thanks.
2013.08.19. 10:51 keltez?ssel, Julia Lawall ?rta:
> From: Julia Lawall <[email protected]>
>
> Remove unneeded error handling on the result of a call to
> platform_get_resource_byname when the value is passed to devm_ioremap_resource.
>
> A simplified version of the semantic patch that makes this change is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression pdev,res,e,e1;
> expression ret != 0;
> identifier l;
> @@
>
> res = platform_get_resource_byname(...);
> - if (res == NULL) { ... \(goto l;\|return ret;\) }
> e = devm_ioremap_resource(e1, res);
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>
Acked-by: Gabor Juhos <[email protected]>
Thanks!
-Gabor