[PATCH 00/27] devm cleanup, part #1, take #3

[PATCH 00/27] devm cleanup, part #1, take #3

[Wolfram Sang]

Here is another bit of cleaning up the devm usage. It is again removing the
resource check with devm_ioremap_resource, because a) new drivers came in and
b) coccinelle had a bug and missed to find a couple of occasions. Unlike last
time, I think it is better if these patches go in via the subsystem trees to
reduce merge conflicts. And there is one driver which I fixed manually because
the original code needed some bigger update. All is based on v3.11-rc2 and the
branch can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git devm_no_resource_check

Other things which happened: I wanted to get rid of devm_request_and_ioremap,
luckily other people are already working on it. Hooray! I already sent a patch
series picking another low hanging fruit, i.e. drivers can skip devm_pinctrl
handling if they are using only the default pin setup. And not devm related,
there is still my proposal to rename INIT_COMPLETION to reinit_completion, that
probably needs some more persistence... Yay, so much to clean up \o/

Regards,

   Wolfram


Wolfram Sang (27):
  arch/mips/lantiq/xway: don't check resource with
    devm_ioremap_resource
  drivers/amba: don't check resource with devm_ioremap_resource
  drivers/cpuidle: don't check resource with devm_ioremap_resource
  drivers/dma: don't check resource with devm_ioremap_resource
  drivers/gpu/host1x/drm: don't check resource with
    devm_ioremap_resource
  drivers/i2c/busses: don't check resource with devm_ioremap_resource
  drivers/input/serio: don't check resource with devm_ioremap_resource
  drivers/iommu: don't check resource with devm_ioremap_resource
  drivers/media/platform: don't check resource with
    devm_ioremap_resource
  drivers/memory: don't check resource with devm_ioremap_resource
  drivers/mtd/nand: don't check resource with devm_ioremap_resource
  drivers/net/ethernet/stmicro/stmmac: don't check resource with
    devm_ioremap_resource
  drivers/pci/host: don't check resource with devm_ioremap_resource
  drivers/pinctrl: don't check resource with devm_ioremap_resource
  drivers/pwm: don't check resource with devm_ioremap_resource
  drivers/scsi/ufs: don't check resource with devm_ioremap_resource
  drivers/spi: don't check resource with devm_ioremap_resource
  drivers/staging/imx-drm: don't check resource with
    devm_ioremap_resource
  drivers/usb/phy: don't check resource with devm_ioremap_resource
  drivers/watchdog: don't check resource with devm_ioremap_resource
  sound/soc/au1x: don't check resource with devm_ioremap_resource
  sound/soc/cirrus: don't check resource with devm_ioremap_resource
  sound/soc/nuc900: don't check resource with devm_ioremap_resource
  sound/soc/pxa: don't check resource with devm_ioremap_resource
  sound/soc/tegra: don't check resource with devm_ioremap_resource
  sound/soc/txx9: don't check resource with devm_ioremap_resource
  thermal: ti-bandgap: cleanup resource allocation

 arch/mips/lantiq/xway/dma.c                        |    4 ----
 drivers/amba/tegra-ahb.c                           |    2 --
 drivers/cpuidle/cpuidle-kirkwood.c                 |    3 ---
 drivers/dma/mmp_pdma.c                             |    3 ---
 drivers/dma/mmp_tdma.c                             |    3 ---
 drivers/gpu/host1x/drm/hdmi.c                      |    3 ---
 drivers/i2c/busses/i2c-stu300.c                    |    3 ---
 drivers/input/serio/olpc_apsp.c                    |    3 ---
 drivers/iommu/tegra-smmu.c                         |    2 --
 drivers/media/platform/coda.c                      |    5 -----
 drivers/memory/tegra20-mc.c                        |    2 --
 drivers/memory/tegra30-mc.c                        |    2 --
 drivers/mtd/nand/mxc_nand.c                        |    5 -----
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |    3 ---
 drivers/pci/host/pcie-designware.c                 |   12 ------------
 drivers/pinctrl/pinctrl-imx.c                      |    3 ---
 drivers/pinctrl/pinctrl-rockchip.c                 |    5 -----
 drivers/pinctrl/pinctrl-u300.c                     |    3 ---
 drivers/pwm/pwm-lpc32xx.c                          |    3 ---
 drivers/pwm/pwm-renesas-tpu.c                      |    5 -----
 drivers/scsi/ufs/ufshcd-pltfrm.c                   |    6 ------
 drivers/spi/spi-bcm2835.c                          |    6 ------
 drivers/staging/imx-drm/imx-tve.c                  |    5 -----
 drivers/thermal/ti-soc-thermal/ti-bandgap.c        |   20 ++++----------------
 drivers/usb/phy/phy-rcar-usb.c                     |    5 -----
 drivers/watchdog/nuc900_wdt.c                      |    5 -----
 drivers/watchdog/ts72xx_wdt.c                      |   10 ----------
 sound/soc/au1x/psc-ac97.c                          |    3 ---
 sound/soc/cirrus/ep93xx-ac97.c                     |    3 ---
 sound/soc/cirrus/ep93xx-i2s.c                      |    3 ---
 sound/soc/nuc900/nuc900-ac97.c                     |    3 ---
 sound/soc/pxa/mmp-sspa.c                           |    3 ---
 sound/soc/tegra/tegra20_ac97.c                     |    7 -------
 sound/soc/txx9/txx9aclc-ac97.c                     |    3 ---
 34 files changed, 4 insertions(+), 152 deletions(-)

-- 
1.7.10.4

[PATCH 02/27] drivers/amba: don't check resource with devm_ioremap_resource

[Wolfram Sang]

devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.

Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
Please apply via the subsystem-tree.

 drivers/amba/tegra-ahb.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/amba/tegra-ahb.c b/drivers/amba/tegra-ahb.c
index 1f44e56c..558a239 100644
--- a/drivers/amba/tegra-ahb.c
+++ b/drivers/amba/tegra-ahb.c
@@ -256,8 +256,6 @@ static int tegra_ahb_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -ENODEV;
 	ahb->regs = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(ahb->regs))
 		return PTR_ERR(ahb->regs);
-- 
1.7.10.4

[PATCH 05/27] drivers/gpu/host1x/drm: don't check resource with devm_ioremap_resource

[Wolfram Sang]

devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.

Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
Please apply via the subsystem-tree.

 drivers/gpu/host1x/drm/hdmi.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/host1x/drm/hdmi.c b/drivers/gpu/host1x/drm/hdmi.c
index 01097da..9ffece6 100644
--- a/drivers/gpu/host1x/drm/hdmi.c
+++ b/drivers/gpu/host1x/drm/hdmi.c
@@ -1248,9 +1248,6 @@ static int tegra_hdmi_probe(struct platform_device *pdev)
 		return err;
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!regs)
-		return -ENXIO;
-
 	hdmi->regs = devm_ioremap_resource(&pdev->dev, regs);
 	if (IS_ERR(hdmi->regs))
 		return PTR_ERR(hdmi->regs);
-- 
1.7.10.4

[PATCH 08/27] drivers/iommu: don't check resource with devm_ioremap_resource

[Wolfram Sang]

devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.

Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
Please apply via the subsystem-tree.

 drivers/iommu/tegra-smmu.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index f6f120e..e066560 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -1177,8 +1177,6 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 		struct resource *res;
 
 		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
-		if (!res)
-			return -ENODEV;
 		smmu->regs[i] = devm_ioremap_resource(&pdev->dev, res);
 		if (IS_ERR(smmu->regs[i]))
 			return PTR_ERR(smmu->regs[i]);
-- 
1.7.10.4

[PATCH 10/27] drivers/memory: don't check resource with devm_ioremap_resource

[Wolfram Sang]

devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.

Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
Please apply via the subsystem-tree.

 drivers/memory/tegra20-mc.c |    2 --
 drivers/memory/tegra30-mc.c |    2 --
 2 files changed, 4 deletions(-)

diff --git a/drivers/memory/tegra20-mc.c b/drivers/memory/tegra20-mc.c
index 0548eea..7cd82b8 100644
--- a/drivers/memory/tegra20-mc.c
+++ b/drivers/memory/tegra20-mc.c
@@ -218,8 +218,6 @@ static int tegra20_mc_probe(struct platform_device *pdev)
 		struct resource *res;
 
 		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
-		if (!res)
-			return -ENODEV;
 		mc->regs[i] = devm_ioremap_resource(&pdev->dev, res);
 		if (IS_ERR(mc->regs[i]))
 			return PTR_ERR(mc->regs[i]);
diff --git a/drivers/memory/tegra30-mc.c b/drivers/memory/tegra30-mc.c
index 58d2979..ef79345 100644
--- a/drivers/memory/tegra30-mc.c
+++ b/drivers/memory/tegra30-mc.c
@@ -340,8 +340,6 @@ static int tegra30_mc_probe(struct platform_device *pdev)
 		struct resource *res;
 
 		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
-		if (!res)
-			return -ENODEV;
 		mc->regs[i] = devm_ioremap_resource(&pdev->dev, res);
 		if (IS_ERR(mc->regs[i]))
 			return PTR_ERR(mc->regs[i]);
-- 
1.7.10.4

[PATCH 25/27] sound/soc/tegra: don't check resource with devm_ioremap_resource

[Wolfram Sang]

devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.

Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
Please apply via the subsystem-tree.

 sound/soc/tegra/tegra20_ac97.c |    7 -------
 1 file changed, 7 deletions(-)

diff --git a/sound/soc/tegra/tegra20_ac97.c b/sound/soc/tegra/tegra20_ac97.c
index e58233f..ef3d0e0 100644
--- a/sound/soc/tegra/tegra20_ac97.c
+++ b/sound/soc/tegra/tegra20_ac97.c
@@ -334,12 +334,6 @@ static int tegra20_ac97_platform_probe(struct platform_device *pdev)
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!mem) {
-		dev_err(&pdev->dev, "No memory resource\n");
-		ret = -ENODEV;
-		goto err_clk_put;
-	}
-
 	regs = devm_ioremap_resource(&pdev->dev, mem);
 	if (IS_ERR(regs)) {
 		ret = PTR_ERR(regs);
@@ -432,7 +426,6 @@ static int tegra20_ac97_platform_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_unregister_pcm:
 	tegra_pcm_platform_unregister(&pdev->dev);
 err_unregister_component:
 	snd_soc_unregister_component(&pdev->dev);
-- 
1.7.10.4

Re: [PATCH 10/27] drivers/memory: don't check resource with devm_ioremap_resource

[Joe Perches]

On Tue, 2013-07-23 at 20:01 +0200, Wolfram Sang wrote:
> devm_ioremap_resource does sanity checks on the given resource. No need to
> duplicate this in the driver.

Hi Wolfram:

This is the first and only one of the patch series I looked at.

> diff --git a/drivers/memory/tegra20-mc.c b/drivers/memory/tegra20-mc.c
[]
> @@ -218,8 +218,6 @@ static int tegra20_mc_probe(struct platform_device *pdev)
>  		struct resource *res;
>  
>  		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> -		if (!res)
> -			return -ENODEV;
>  		mc->regs[i] = devm_ioremap_resource(&pdev->dev, res);

I'm not so sure this is appropriate.

devm_ioremap_resource returns ERR_PTR(-EINVAL) for
null resource so this changes the return.

devm_ioremap_resource also emits a noisy dev_err
message when resource is NULL.

It's a probe and before the message log would be silent
but now there's a new dmesg.

Perhaps that should be mentioned in the changelog here
and elsewhere in the series.

Re: [PATCH 10/27] drivers/memory: don't check resource with devm_ioremap_resource

[Stephen Warren]

On 07/23/2013 11:25 AM, Joe Perches wrote:
> On Tue, 2013-07-23 at 20:01 +0200, Wolfram Sang wrote:
>> devm_ioremap_resource does sanity checks on the given resource. No need to
>> duplicate this in the driver.
> 
> Hi Wolfram:
> 
> This is the first and only one of the patch series I looked at.
> 
>> diff --git a/drivers/memory/tegra20-mc.c b/drivers/memory/tegra20-mc.c
> []
>> @@ -218,8 +218,6 @@ static int tegra20_mc_probe(struct platform_device *pdev)
>>  		struct resource *res;
>>  
>>  		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
>> -		if (!res)
>> -			return -ENODEV;
>>  		mc->regs[i] = devm_ioremap_resource(&pdev->dev, res);
> 
> I'm not so sure this is appropriate.
> 
> devm_ioremap_resource returns ERR_PTR(-EINVAL) for
> null resource so this changes the return.

I think the exact return value is probably pretty arbitrary here.

> devm_ioremap_resource also emits a noisy dev_err
> message when resource is NULL.
> 
> It's a probe and before the message log would be silent
> but now there's a new dmesg.

I think those changes are fine, at least for this driver. It's a bug if
the required resources are missing, and having probe() actively point
out why it's failing can only be a good thing in my book.

Re: [PATCH 08/27] drivers/iommu: don't check resource with devm_ioremap_resource

[Stephen Warren]

On 07/23/2013 11:01 AM, Wolfram Sang wrote:
> devm_ioremap_resource does sanity checks on the given resource. No need to
> duplicate this in the driver.
> 
> Signed-off-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> ---
> Please apply via the subsystem-tree.

You probably want to Cc the usual commiter (Joerg Roedel
<joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>). I've done so here.

Re: [PATCH 10/27] drivers/memory: don't check resource with devm_ioremap_resource

[Stephen Warren]

On 07/23/2013 11:01 AM, Wolfram Sang wrote:
> devm_ioremap_resource does sanity checks on the given resource. No need to
> duplicate this in the driver.
> 
> Signed-off-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> ---
> Please apply via the subsystem-tree.

Greg KH usually commits patches to thus tree. I Cc'd him here.

Re: [PATCH 02/27] drivers/amba: don't check resource with devm_ioremap_resource

[Stephen Warren]

On 07/23/2013 11:01 AM, Wolfram Sang wrote:
> devm_ioremap_resource does sanity checks on the given resource. No need to
> duplicate this in the driver.
> 
> Signed-off-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> ---
> Please apply via the subsystem-tree.

Russell, I assume you'll take this patch?

Re: [PATCH 10/27] drivers/memory: don't check resource with devm_ioremap_resource

[Joe Perches]

On Tue, 2013-07-23 at 18:27 -0700, Stephen Warren wrote:
> On 07/23/2013 11:25 AM, Joe Perches wrote:
> > On Tue, 2013-07-23 at 20:01 +0200, Wolfram Sang wrote:
> >> devm_ioremap_resource does sanity checks on the given resource. No need to
> >> duplicate this in the driver.
[]
> > This is the first and only one of the patch series I looked at.
> > 
> >> diff --git a/drivers/memory/tegra20-mc.c b/drivers/memory/tegra20-mc.c
> > []
> >> @@ -218,8 +218,6 @@ static int tegra20_mc_probe(struct platform_device *pdev)
> >>  		struct resource *res;
> >>  
> >>  		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> >> -		if (!res)
> >> -			return -ENODEV;
> >>  		mc->regs[i] = devm_ioremap_resource(&pdev->dev, res);
> > 
> > I'm not so sure this is appropriate.
> > 
> > devm_ioremap_resource returns ERR_PTR(-EINVAL) for
> > null resource so this changes the return.
> 
> I think the exact return value is probably pretty arbitrary here.

I think so as well, but it takes code inspection to
determine whether or not there's any code impact.

I want to make sure Wolfram has done that inspection.

> > devm_ioremap_resource also emits a noisy dev_err
> > message when resource is NULL.
> > 
> > It's a probe and before the message log would be silent
> > but now there's a new dmesg.
> 
> I think those changes are fine, at least for this driver. It's a bug if
> the required resources are missing, and having probe() actively point
> out why it's failing can only be a good thing in my book.

Again, I haven't looked at _all_ the paths for all
of these patches, I just picked one at random.

Extra dmesg output with some device probes that are
expected to fail is not good.

Re: [PATCH 25/27] sound/soc/tegra: don't check resource with devm_ioremap_resource

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 259 bytes --]

On Tue, Jul 23, 2013 at 08:01:58PM +0200, Wolfram Sang wrote:
> devm_ioremap_resource does sanity checks on the given resource. No need to
> duplicate this in the driver.

This doesn't seem to apply against my current branch, can you please
check and resend?

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 05/27] drivers/gpu/host1x/drm: don't check resource with devm_ioremap_resource

[Terje Bergström]

On 23.07.2013 21:01, Wolfram Sang wrote:
> diff --git a/drivers/gpu/host1x/drm/hdmi.c b/drivers/gpu/host1x/drm/hdmi.c
> index 01097da..9ffece6 100644
> --- a/drivers/gpu/host1x/drm/hdmi.c
> +++ b/drivers/gpu/host1x/drm/hdmi.c
> @@ -1248,9 +1248,6 @@ static int tegra_hdmi_probe(struct platform_device *pdev)
>  		return err;
>  
>  	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!regs)
> -		return -ENXIO;
> -
>  	hdmi->regs = devm_ioremap_resource(&pdev->dev, regs);
>  	if (IS_ERR(hdmi->regs))
>  		return PTR_ERR(hdmi->regs);
> 

Looks good to me.

Reviewed-By: Terje Bergstrom <tbergstrom-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Terje