[PATCH 0/6] simplify platform_get_resource_byname/devm_ioremap_resource

[PATCH 0/6] simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/6] simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

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.

[PATCH 0/6] simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

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.

[PATCH 0/6] simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

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.

[PATCH 1/6] ASoC: omap: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

---
 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);

[PATCH 1/6] ASoC: omap: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

---
 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);

[PATCH 1/6] ASoC: omap: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

---
 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);

Re: [PATCH 1/6] ASoC: omap: simplify platform_get_resource_byname/devm_ioremap_resource

[Jarkko Nikula]

Hi

On 08/19/2013 11:51 AM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> 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 <Julia.Lawall@lip6.fr>
> 
> ---
>  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 <jarkko.nikula@bitmer.com>

Re: [PATCH 1/6] ASoC: omap: simplify platform_get_resource_byname/devm_ioremap_resource

[Jarkko Nikula]

Hi

On 08/19/2013 11:51 AM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> 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 <Julia.Lawall@lip6.fr>
> 
> ---
>  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 <jarkko.nikula@bitmer.com>

Re: [PATCH 1/6] ASoC: omap: simplify platform_get_resource_byname/devm_ioremap_resource

[Jarkko Nikula]

Hi

On 08/19/2013 11:51 AM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> 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 <Julia.Lawall@lip6.fr>
> 
> ---
>  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 <jarkko.nikula@bitmer.com>

Re: [PATCH 1/6] ASoC: omap: simplify platform_get_resource_byname/devm_ioremap_resource

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 276 bytes --]

On Mon, Aug 19, 2013 at 10:51:51AM +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> 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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/6] ASoC: omap: simplify platform_get_resource_byname/devm_ioremap_resource

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 276 bytes --]

On Mon, Aug 19, 2013 at 10:51:51AM +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> 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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 2/6] watchdog: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

---
 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);

[PATCH 2/6] watchdog: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

---
 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);

Re: [PATCH 2/6] watchdog: simplify platform_get_resource_byname/devm_ioremap_resource

[Guenter Roeck]

On 08/19/2013 01:51 AM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> 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 <Julia.Lawall@lip6.fr>
>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

Re: [PATCH 2/6] watchdog: simplify platform_get_resource_byname/devm_ioremap_resource

[Guenter Roeck]

On 08/19/2013 01:51 AM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> 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 <Julia.Lawall@lip6.fr>
>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

[PATCH 3/6] mtd: fsmc_nand: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

---
 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);

[PATCH 3/6] mtd: fsmc_nand: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

---
 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);

[PATCH 3/6] mtd: fsmc_nand: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

---
 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);

Re: [PATCH 3/6] mtd: fsmc_nand: simplify platform_get_resource_byname/devm_ioremap_resource

[Artem Bityutskiy]

On Mon, 2013-08-19 at 10:51 +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> 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

Re: [PATCH 3/6] mtd: fsmc_nand: simplify platform_get_resource_byname/devm_ioremap_resource

[Artem Bityutskiy]

On Mon, 2013-08-19 at 10:51 +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> 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

Re: [PATCH 3/6] mtd: fsmc_nand: simplify platform_get_resource_byname/devm_ioremap_resource

[Artem Bityutskiy]

On Mon, 2013-08-19 at 10:51 +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> 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

[PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

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

[PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

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

Re: [PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Svenning Sørensen]

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

Re: [PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Svenning Sørensen]

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

Re: [PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 854 bytes --]



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

Re: [PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 830 bytes --]



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

Re: [PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Svenning Sørensen]

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

Re: [PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Svenning Sørensen]

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

Re: [PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 640 bytes --]

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

Re: [PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 622 bytes --]

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

[PATCH 4/6 v2] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1524 bytes --]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

---
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 */

[PATCH 4/6 v2] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1474 bytes --]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

---
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 */

Re: [PATCH 4/6 v2] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Sergei Shtylyov]

Hello.

On 08/19/2013 03:47 PM, Julia Lawall wrote:

> From: Julia Lawall <Julia.Lawall@lip6.fr>

> 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 <Julia.Lawall@lip6.fr>

> ---
> 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6 v2] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Sergei Shtylyov]

Hello.

On 08/19/2013 03:47 PM, Julia Lawall wrote:

> From: Julia Lawall <Julia.Lawall@lip6.fr>

> 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 <Julia.Lawall@lip6.fr>

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

Re: [PATCH 4/6 v2] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2315 bytes --]



On Mon, 19 Aug 2013, Sergei Shtylyov wrote:

> Hello.
>
> On 08/19/2013 03:47 PM, Julia Lawall wrote:
>
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> > 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 <Julia.Lawall@lip6.fr>
>
> > ---
> > 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 4/6 v2] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2239 bytes --]



On Mon, 19 Aug 2013, Sergei Shtylyov wrote:

> Hello.
>
> On 08/19/2013 03:47 PM, Julia Lawall wrote:
>
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> > 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 <Julia.Lawall@lip6.fr>
>
> > ---
> > 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 4/6 v2] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Dan Carpenter]

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 <Julia.Lawall@lip6.fr>
> 
> >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 <Julia.Lawall@lip6.fr>
> 
> >---
> >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

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6 v2] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

[Dan Carpenter]

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 <Julia.Lawall@lip6.fr>
> 
> >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 <Julia.Lawall@lip6.fr>
> 
> >---
> >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

[PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

---
 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);

[PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

---
 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);

Re: [PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[walter harms]

Am 19.08.2013 10:51, schrieb Julia Lawall:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> 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 <Julia.Lawall@lip6.fr>
> 
> ---
>  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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[walter harms]

Am 19.08.2013 10:51, schrieb Julia Lawall:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> 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 <Julia.Lawall@lip6.fr>
> 
> ---
>  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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

On Mon, 19 Aug 2013, walter harms wrote:

>
>
> Am 19.08.2013 10:51, schrieb Julia Lawall:
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > 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 <Julia.Lawall@lip6.fr>
> >
> > ---
> >  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 majordomo@vger.kernel.org
> > 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

On Mon, 19 Aug 2013, walter harms wrote:

>
>
> Am 19.08.2013 10:51, schrieb Julia Lawall:
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > 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 <Julia.Lawall@lip6.fr>
> >
> > ---
> >  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 majordomo@vger.kernel.org
> > 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

On Mon, 19 Aug 2013, walter harms wrote:

>
>
> Am 19.08.2013 10:51, schrieb Julia Lawall:
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > 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 <Julia.Lawall@lip6.fr>
> >
> > ---
> >  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

Re: [PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

On Mon, 19 Aug 2013, walter harms wrote:

>
>
> Am 19.08.2013 10:51, schrieb Julia Lawall:
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > 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 <Julia.Lawall@lip6.fr>
> >
> > ---
> >  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

Re: [PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[walter harms]

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 <Julia.Lawall@lip6.fr>
>>>
>>> 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 <Julia.Lawall@lip6.fr>
>>>
>>> ---
>>>  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

Re: [PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[walter harms]

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 <Julia.Lawall@lip6.fr>
>>>
>>> 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 <Julia.Lawall@lip6.fr>
>>>
>>> ---
>>>  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

Re: [PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

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 <Julia.Lawall@lip6.fr>
> >>>
> >>> 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 <Julia.Lawall@lip6.fr>
> >>>
> >>> ---
> >>>  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

Re: [PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

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 <Julia.Lawall@lip6.fr>
> >>>
> >>> 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 <Julia.Lawall@lip6.fr>
> >>>
> >>> ---
> >>>  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

Re: [PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

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 <Julia.Lawall@lip6.fr>
> >
> > 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 <Julia.Lawall@lip6.fr>
> >
> > ---
> >  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 majordomo@vger.kernel.org
> > 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

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 <Julia.Lawall@lip6.fr>
> >
> > 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 <Julia.Lawall@lip6.fr>
> >
> > ---
> >  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 majordomo@vger.kernel.org
> > 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 276 bytes --]

On Mon, Aug 19, 2013 at 10:51:55AM +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> 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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 5/6] regulator: ti-abb: simplify platform_get_resource_byname/devm_ioremap_resource

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 276 bytes --]

On Mon, Aug 19, 2013 at 10:51:55AM +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> 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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 6/6] MIPS: ath79: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

---
 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);

[PATCH 6/6] MIPS: ath79: simplify platform_get_resource_byname/devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

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 <Julia.Lawall@lip6.fr>

---
 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);

Re: [PATCH 6/6] MIPS: ath79: simplify platform_get_resource_byname/devm_ioremap_resource

[Gabor Juhos]

2013.08.19. 10:51 keltezéssel, Julia Lawall írta:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> 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 <Julia.Lawall@lip6.fr>

Acked-by: Gabor Juhos <juhosg@openwrt.org>

Thanks!

-Gabor
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] MIPS: ath79: simplify platform_get_resource_byname/devm_ioremap_resource

[Gabor Juhos]

2013.08.19. 10:51 keltezéssel, Julia Lawall írta:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> 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 <Julia.Lawall@lip6.fr>

Acked-by: Gabor Juhos <juhosg@openwrt.org>

Thanks!

-Gabor

Re: [PATCH 6/6] MIPS: ath79: simplify platform_get_resource_byname/devm_ioremap_resource

[John Crispin]

On 22/08/13 19:50, Gabor Juhos wrote:
> 2013.08.19. 10:51 keltezéssel, Julia Lawall írta:
>> From: Julia Lawall<Julia.Lawall@lip6.fr>
>>
>> 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<Julia.Lawall@lip6.fr>
>
> Acked-by: Gabor Juhos<juhosg@openwrt.org>
>
> Thanks!
>
> -Gabor

Hi,

should this patch go upstream via the lmo tree ?

	John

Re: [PATCH 6/6] MIPS: ath79: simplify platform_get_resource_byname/devm_ioremap_resource

[Gabor Juhos]

2013.08.31. 10:09 keltezéssel, John Crispin írta:

<snip>

> should this patch go upstream via the lmo tree ?

Yes, I think.

-Gabor