[PATCH 20/33] net: Convert to devm_ioremap_resource()

[PATCH 20/33] net: Convert to devm_ioremap_resource()

[Thierry Reding]

Convert all uses of devm_request_and_ioremap() to the newly introduced
devm_ioremap_resource() which provides more consistent error handling.

devm_ioremap_resource() provides its own error messages so all explicit
error messages can be removed from the failure code paths.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
---
 drivers/net/can/grcan.c                               | 8 ++++----
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 9 ++++-----
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index 17fbc7a..4c3a7dd 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -26,6 +26,7 @@
  * Contributors: Andreas Larsson <andreas@gaisler.com>
  */
 
+#include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
@@ -1683,10 +1684,9 @@ static int grcan_probe(struct platform_device *ofdev)
 	}
 
 	res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
-	base = devm_request_and_ioremap(&ofdev->dev, res);
-	if (!base) {
-		dev_err(&ofdev->dev, "couldn't map IO resource\n");
-		err = -EADDRNOTAVAIL;
+	base = devm_ioremap_resource(&ofdev->dev, res);
+	if (IS_ERR(base)) {
+		err = PTR_ERR(base);
 		goto exit_error;
 	}
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index b43d68b..a3431aa 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -22,6 +22,7 @@
   Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
 *******************************************************************************/
 
+#include <linux/err.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/of.h>
@@ -88,11 +89,9 @@ static int stmmac_pltfr_probe(struct platform_device *pdev)
 	if (!res)
 		return -ENODEV;
 
-	addr = devm_request_and_ioremap(dev, res);
-	if (!addr) {
-		pr_err("%s: ERROR: memory mapping failed", __func__);
-		return -ENOMEM;
-	}
+	addr = devm_ioremap_resource(dev, res);
+	if (IS_ERR(addr))
+		return PTR_ERR(addr);
 
 	if (pdev->dev.of_node) {
 		plat_dat = devm_kzalloc(&pdev->dev,
-- 
1.8.1.1

Re: [PATCH 20/33] net: Convert to devm_ioremap_resource()

[David Miller]

From: Thierry Reding <thierry.reding@avionic-design.de>
Date: Mon, 21 Jan 2013 11:09:13 +0100

> Convert all uses of devm_request_and_ioremap() to the newly introduced
> devm_ioremap_resource() which provides more consistent error handling.
> 
> devm_ioremap_resource() provides its own error messages so all explicit
> error messages can be removed from the failure code paths.
> 
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>

This won't compile if I apply it.

You really have to be clear when you submit patches like this.

Since you only CC:'d the networking developers for this one
patch, there is _ZERO_ context for us to work with to understand
what's going on.

You have to also CC: us on the other relevant changes and your
"[PATCH 00/33]" posting that explains what is happening.

Re: [PATCH 20/33] net: Convert to devm_ioremap_resource()

[Thierry Reding]

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

On Mon, Jan 21, 2013 at 03:29:13PM -0500, David Miller wrote:
> From: Thierry Reding <thierry.reding@avionic-design.de>
> Date: Mon, 21 Jan 2013 11:09:13 +0100
> 
> > Convert all uses of devm_request_and_ioremap() to the newly introduced
> > devm_ioremap_resource() which provides more consistent error handling.
> > 
> > devm_ioremap_resource() provides its own error messages so all explicit
> > error messages can be removed from the failure code paths.
> > 
> > Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> 
> This won't compile if I apply it.
> 
> You really have to be clear when you submit patches like this.
> 
> Since you only CC:'d the networking developers for this one
> patch, there is _ZERO_ context for us to work with to understand
> what's going on.
> 
> You have to also CC: us on the other relevant changes and your
> "[PATCH 00/33]" posting that explains what is happening.

I planned to do so initially, but that yielded a Cc list of 156 people
and mailing lists, which I thought wasn't going to go down so well
either. In general I like Cc'ing everyone concerned on all patches of
the series, specifically for reasons of context. Some people have been
annoyed when I did so. Still, for small series where only a few dozen
people are concerned that seems to me to be the best way. But 156 email
addresses is a different story.

Either you add to many people or you don't add enough. Where do we draw
the line?

Thierry

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

Re: [PATCH 20/33] net: Convert to devm_ioremap_resource()

[Arnd Bergmann]

On Tuesday 22 January 2013, Thierry Reding wrote:
> I planned to do so initially, but that yielded a Cc list of 156 people
> and mailing lists, which I thought wasn't going to go down so well
> either. In general I like Cc'ing everyone concerned on all patches of
> the series, specifically for reasons of context. Some people have been
> annoyed when I did so. Still, for small series where only a few dozen
> people are concerned that seems to me to be the best way. But 156 email
> addresses is a different story.
> 
> Either you add to many people or you don't add enough. Where do we draw
> the line?

I've had the same problem a couple of times. The best compromise seems
to be to Cc only the top-level subsystem maintainers and mailing lists
on the first email.

	Arnd

Re: [PATCH 20/33] net: Convert to devm_ioremap_resource()

[Thierry Reding]

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

On Tue, Jan 22, 2013 at 01:03:06PM +0000, Arnd Bergmann wrote:
> On Tuesday 22 January 2013, Thierry Reding wrote:
> > I planned to do so initially, but that yielded a Cc list of 156 people
> > and mailing lists, which I thought wasn't going to go down so well
> > either. In general I like Cc'ing everyone concerned on all patches of
> > the series, specifically for reasons of context. Some people have been
> > annoyed when I did so. Still, for small series where only a few dozen
> > people are concerned that seems to me to be the best way. But 156 email
> > addresses is a different story.
> > 
> > Either you add to many people or you don't add enough. Where do we draw
> > the line?
> 
> I've had the same problem a couple of times. The best compromise seems
> to be to Cc only the top-level subsystem maintainers and mailing lists
> on the first email.

Even that would have been about 50 addresses IIRC. But perhaps that's
still the best compromise to avoid any confusion.

Thierry

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

Re: [PATCH 20/33] net: Convert to devm_ioremap_resource()

[Arnd Bergmann]

On Tuesday 22 January 2013, Thierry Reding wrote:
> > I've had the same problem a couple of times. The best compromise seems
> > to be to Cc only the top-level subsystem maintainers and mailing lists
> > on the first email.
> 
> Even that would have been about 50 addresses IIRC. But perhaps that's
> still the best compromise to avoid any confusion.

Be careful though that the Cc list must not exceed 1024 characters or
it will get rejected by some of the mailing list servers.

	Arnd

Re: [PATCH 20/33] net: Convert to devm_ioremap_resource()

[David Miller]

From: Thierry Reding <thierry.reding@avionic-design.de>
Date: Tue, 22 Jan 2013 07:56:29 +0100

> I planned to do so initially, but that yielded a Cc list of 156 people
> and mailing lists,

Just use mailing lists, the individuals are subscribed to the subsystem
lists that cover what they maintain.

Yes, CC:'ing 156 "people" is stupid.

Re: [PATCH 20/33] net: Convert to devm_ioremap_resource()

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 22 Jan 2013 13:03:06 +0000

> On Tuesday 22 January 2013, Thierry Reding wrote:
>> I planned to do so initially, but that yielded a Cc list of 156 people
>> and mailing lists, which I thought wasn't going to go down so well
>> either. In general I like Cc'ing everyone concerned on all patches of
>> the series, specifically for reasons of context. Some people have been
>> annoyed when I did so. Still, for small series where only a few dozen
>> people are concerned that seems to me to be the best way. But 156 email
>> addresses is a different story.
>> 
>> Either you add to many people or you don't add enough. Where do we draw
>> the line?
> 
> I've had the same problem a couple of times. The best compromise seems
> to be to Cc only the top-level subsystem maintainers and mailing lists
> on the first email.

CC:'ing individuals is pointless, CC: only the subsystem lists in
question, the maintainers had better be reading those lists.