All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: linux-mmc@vger.kernel.org, linux-sh@vger.kernel.org,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Chris Ball <chris@printf.net>,
	Guennadi Liakhovetski <g.liakhovetski@gmx.de>,
	magnus.damm@opensource.se, linux-kernel@lists.codethink.co.uk
Subject: Re: [PATCH 2/6] mmc: sh-mmcif: use devm_ for ioremap
Date: Tue, 01 Apr 2014 13:08:59 +0200	[thread overview]
Message-ID: <4491980.dtDUU42Y5f@avalon> (raw)
In-Reply-To: <1396347954-13740-3-git-send-email-ben.dooks@codethink.co.uk>

Hi Ben,

Thank you for the patch.

On Tuesday 01 April 2014 11:25:50 Ben Dooks wrote:
> Start tidying the probe/release code by using devm_ioremap_resource() to
> map the IO registers.
> 
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
>  drivers/mmc/host/sh_mmcif.c | 20 +++++---------------
>  1 file changed, 5 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index c48df98..be6be2b 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -1379,22 +1379,17 @@ static int sh_mmcif_probe(struct platform_device
> *pdev) dev_err(&pdev->dev, "Get irq error\n");
>  		return -ENXIO;
>  	}
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!res) {
> -		dev_err(&pdev->dev, "platform_get_resource error.\n");
> -		return -ENXIO;
> -	}
> -	reg = ioremap(res->start, resource_size(res));
> -	if (!reg) {
> +	reg = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(reg)) {
>  		dev_err(&pdev->dev, "ioremap error.\n");

devm_ioremap_resource already prints an error message on failure, you can this 
remove this one.

>  		return -ENOMEM;

What about returning PTR_ERR(reg) instead ?

>  	}
> 
>  	mmc = mmc_alloc_host(sizeof(struct sh_mmcif_host), &pdev->dev);
> -	if (!mmc) {
> -		ret = -ENOMEM;
> -		goto ealloch;
> -	}
> +	if (!mmc)
> +		return -ENOMEM;
> 
>  	ret = mmc_of_parse(mmc);
>  	if (ret < 0)
> @@ -1499,8 +1494,6 @@ eclkget:
>  	pm_runtime_disable(&pdev->dev);
>  eofparse:
>  	mmc_free_host(mmc);
> -ealloch:
> -	iounmap(reg);
>  	return ret;
>  }
> 
> @@ -1525,9 +1518,6 @@ static int sh_mmcif_remove(struct platform_device
> *pdev) */
>  	cancel_delayed_work_sync(&host->timeout_work);
> 
> -	if (host->addr)
> -		iounmap(host->addr);
> -
>  	irq[0] = platform_get_irq(pdev, 0);
>  	irq[1] = platform_get_irq(pdev, 1);

-- 
Regards,

Laurent Pinchart


WARNING: multiple messages have this Message-ID (diff)
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: linux-mmc@vger.kernel.org, linux-sh@vger.kernel.org,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Chris Ball <chris@printf.net>,
	Guennadi Liakhovetski <g.liakhovetski@gmx.de>,
	magnus.damm@opensource.se, linux-kernel@lists.codethink.co.uk
Subject: Re: [PATCH 2/6] mmc: sh-mmcif: use devm_ for ioremap
Date: Tue, 01 Apr 2014 11:08:59 +0000	[thread overview]
Message-ID: <4491980.dtDUU42Y5f@avalon> (raw)
In-Reply-To: <1396347954-13740-3-git-send-email-ben.dooks@codethink.co.uk>

Hi Ben,

Thank you for the patch.

On Tuesday 01 April 2014 11:25:50 Ben Dooks wrote:
> Start tidying the probe/release code by using devm_ioremap_resource() to
> map the IO registers.
> 
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
>  drivers/mmc/host/sh_mmcif.c | 20 +++++---------------
>  1 file changed, 5 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index c48df98..be6be2b 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -1379,22 +1379,17 @@ static int sh_mmcif_probe(struct platform_device
> *pdev) dev_err(&pdev->dev, "Get irq error\n");
>  		return -ENXIO;
>  	}
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!res) {
> -		dev_err(&pdev->dev, "platform_get_resource error.\n");
> -		return -ENXIO;
> -	}
> -	reg = ioremap(res->start, resource_size(res));
> -	if (!reg) {
> +	reg = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(reg)) {
>  		dev_err(&pdev->dev, "ioremap error.\n");

devm_ioremap_resource already prints an error message on failure, you can this 
remove this one.

>  		return -ENOMEM;

What about returning PTR_ERR(reg) instead ?

>  	}
> 
>  	mmc = mmc_alloc_host(sizeof(struct sh_mmcif_host), &pdev->dev);
> -	if (!mmc) {
> -		ret = -ENOMEM;
> -		goto ealloch;
> -	}
> +	if (!mmc)
> +		return -ENOMEM;
> 
>  	ret = mmc_of_parse(mmc);
>  	if (ret < 0)
> @@ -1499,8 +1494,6 @@ eclkget:
>  	pm_runtime_disable(&pdev->dev);
>  eofparse:
>  	mmc_free_host(mmc);
> -ealloch:
> -	iounmap(reg);
>  	return ret;
>  }
> 
> @@ -1525,9 +1518,6 @@ static int sh_mmcif_remove(struct platform_device
> *pdev) */
>  	cancel_delayed_work_sync(&host->timeout_work);
> 
> -	if (host->addr)
> -		iounmap(host->addr);
> -
>  	irq[0] = platform_get_irq(pdev, 0);
>  	irq[1] = platform_get_irq(pdev, 1);

-- 
Regards,

Laurent Pinchart


  parent reply	other threads:[~2014-04-01 11:08 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-01 10:25 cleanups for sh-mmcif driver Ben Dooks
2014-04-01 10:25 ` Ben Dooks
2014-04-01 10:25 ` [PATCH 1/6] mmc: sh-mmcif: print bus clock rate on probe Ben Dooks
2014-04-01 10:25   ` Ben Dooks
2014-04-01 10:45   ` Geert Uytterhoeven
2014-04-01 10:45     ` Geert Uytterhoeven
2014-04-01 11:08   ` Laurent Pinchart
2014-04-01 11:08     ` Laurent Pinchart
2014-04-01 11:37     ` Ben Dooks
2014-04-01 11:37       ` Ben Dooks
2014-04-01 10:25 ` [PATCH 2/6] mmc: sh-mmcif: use devm_ for ioremap Ben Dooks
2014-04-01 10:25   ` Ben Dooks
2014-04-01 10:55   ` Geert Uytterhoeven
2014-04-01 10:55     ` Geert Uytterhoeven
2014-04-01 10:59     ` Ben Dooks
2014-04-01 10:59       ` Ben Dooks
2014-04-01 11:08   ` Laurent Pinchart [this message]
2014-04-01 11:08     ` Laurent Pinchart
2014-04-01 11:26   ` Sergei Shtylyov
2014-04-01 11:26     ` Sergei Shtylyov
2014-04-01 10:25 ` [PATCH 3/6] mmc: sh-mmcif: use devm_ for clock management Ben Dooks
2014-04-01 10:25   ` Ben Dooks
2014-04-01 10:59   ` Geert Uytterhoeven
2014-04-01 10:59     ` Geert Uytterhoeven
2014-04-01 11:25     ` Ben Dooks
2014-04-01 11:25       ` Ben Dooks
2014-04-01 10:25 ` [PATCH 4/6] mmc: sh-mmcif: use devm_ for irq management Ben Dooks
2014-04-01 10:25   ` Ben Dooks
2014-04-01 10:25 ` [PATCH 5/6] mmc: sh-mmcif: no need to call pm_runtime_suspend on error Ben Dooks
2014-04-01 10:25   ` Ben Dooks
2014-04-01 10:25 ` [PATCH 6/6] mmc: sh-mmcif: final error path cleanup Ben Dooks
2014-04-01 10:25   ` Ben Dooks
  -- strict thread matches above, loose matches on Subject: below --
2014-06-04 11:42 resend - sh mmcif driver cleanup patch series Ben Dooks
2014-06-04 11:42 ` [PATCH 2/6] mmc: sh-mmcif: use devm_ for ioremap Ben Dooks
2014-06-04 12:58   ` Ulf Hansson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4491980.dtDUU42Y5f@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=ben.dooks@codethink.co.uk \
    --cc=chris@printf.net \
    --cc=g.liakhovetski@gmx.de \
    --cc=linux-kernel@lists.codethink.co.uk \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=magnus.damm@opensource.se \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.