2023-07-06 12:45:08

by 李扬韬

[permalink] [raw]
Subject: [PATCH 5/8] ata: sata_rcar: Convert to devm_platform_ioremap_resource()

Use devm_platform_ioremap_resource() to simplify code.

Signed-off-by: Yangtao Li <[email protected]>
---
drivers/ata/sata_rcar.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index 34790f15c1b8..63f8337c2a98 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -861,15 +861,11 @@ static int sata_rcar_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct ata_host *host;
struct sata_rcar_priv *priv;
- struct resource *mem;
- int irq;
- int ret = 0;
+ int irq, ret;

irq = platform_get_irq(pdev, 0);
if (irq < 0)
return irq;
- if (!irq)
- return -EINVAL;

priv = devm_kzalloc(dev, sizeof(struct sata_rcar_priv), GFP_KERNEL);
if (!priv)
@@ -890,8 +886,7 @@ static int sata_rcar_probe(struct platform_device *pdev)

host->private_data = priv;

- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- priv->base = devm_ioremap_resource(dev, mem);
+ priv->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(priv->base)) {
ret = PTR_ERR(priv->base);
goto err_pm_put;
--
2.39.0



2023-07-06 13:24:23

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 5/8] ata: sata_rcar: Convert to devm_platform_ioremap_resource()

Hi Yangtao,

On Thu, Jul 6, 2023 at 2:44 PM Yangtao Li <[email protected]> wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>

Thanks for your patch!

> --- a/drivers/ata/sata_rcar.c
> +++ b/drivers/ata/sata_rcar.c
> @@ -861,15 +861,11 @@ static int sata_rcar_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct ata_host *host;
> struct sata_rcar_priv *priv;
> - struct resource *mem;
> - int irq;
> - int ret = 0;
> + int irq, ret;
>
> irq = platform_get_irq(pdev, 0);
> if (irq < 0)
> return irq;
> - if (!irq)
> - return -EINVAL;

Why this change?
It is not documented in the patch description.

>
> priv = devm_kzalloc(dev, sizeof(struct sata_rcar_priv), GFP_KERNEL);
> if (!priv)
> @@ -890,8 +886,7 @@ static int sata_rcar_probe(struct platform_device *pdev)
>
> host->private_data = priv;
>
> - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - priv->base = devm_ioremap_resource(dev, mem);
> + priv->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(priv->base)) {
> ret = PTR_ERR(priv->base);
> goto err_pm_put;

The rest LGTM.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2023-07-06 21:14:51

by Sergey Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 5/8] ata: sata_rcar: Convert to devm_platform_ioremap_resource()

On 7/6/23 3:42 PM, Yangtao Li wrote:

> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
> ---
> drivers/ata/sata_rcar.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
> index 34790f15c1b8..63f8337c2a98 100644
> --- a/drivers/ata/sata_rcar.c
> +++ b/drivers/ata/sata_rcar.c
> @@ -861,15 +861,11 @@ static int sata_rcar_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct ata_host *host;
> struct sata_rcar_priv *priv;
> - struct resource *mem;
> - int irq;
> - int ret = 0;
> + int irq, ret;

This unneeded init of ret should be in a separate patch.

>
> irq = platform_get_irq(pdev, 0);
> if (irq < 0)
> return irq;
> - if (!irq)
> - return -EINVAL;

This is a separate issue too. I thought I've removed this myself
but apparently not, the commit [1] took shamefully long time to merge... :-(

[...]
> @@ -890,8 +886,7 @@ static int sata_rcar_probe(struct platform_device *pdev)
>
> host->private_data = priv;
>
> - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - priv->base = devm_ioremap_resource(dev, mem);
> + priv->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(priv->base)) {
> ret = PTR_ERR(priv->base);
> goto err_pm_put;

The (documented) change looks OK.


[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ce753ad1549cbe9ccaea4c06a1f5fa47432c8289

MBR, Seregy

2023-07-06 23:50:21

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH 5/8] ata: sata_rcar: Convert to devm_platform_ioremap_resource()

On 7/6/23 21:42, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
> ---
> drivers/ata/sata_rcar.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
> index 34790f15c1b8..63f8337c2a98 100644
> --- a/drivers/ata/sata_rcar.c
> +++ b/drivers/ata/sata_rcar.c
> @@ -861,15 +861,11 @@ static int sata_rcar_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct ata_host *host;
> struct sata_rcar_priv *priv;
> - struct resource *mem;
> - int irq;
> - int ret = 0;
> + int irq, ret;
>
> irq = platform_get_irq(pdev, 0);
> if (irq < 0)
> return irq;
> - if (!irq)
> - return -EINVAL;

Same comment as Geert: this is OK to remove, but make that a different
patch.

>
> priv = devm_kzalloc(dev, sizeof(struct sata_rcar_priv), GFP_KERNEL);
> if (!priv)
> @@ -890,8 +886,7 @@ static int sata_rcar_probe(struct platform_device *pdev)
>
> host->private_data = priv;
>
> - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - priv->base = devm_ioremap_resource(dev, mem);
> + priv->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(priv->base)) {
> ret = PTR_ERR(priv->base);
> goto err_pm_put;

--
Damien Le Moal
Western Digital Research