[PATCH 0/5] irqchip/renesas-irqc: Miscellaneous cleanups and improvements

[PATCH 0/5] irqchip/renesas-irqc: Miscellaneous cleanups and improvements

[Geert Uytterhoeven]

	Hi all,

This is a set of miscellaneous cleanups and improvements for the Renesas
R-Mobile APE6 and R-Car IRQC driver.

Thanks!

Geert Uytterhoeven (5):
  irqchip/renesas-irqc: Remove unneeded inclusion of <linux/spinlock.h>
  irqchip/renesas-irqc: Remove devm_kzalloc()/ioremap_nocache() error
    printing
  irqchip/renesas-irqc: Add helper variable dev = &pdev->dev
  irqchip/renesas-irqc: Replace irqc_priv.pdev by irqc_priv.dev
  irqchip/renesas-irqc: Convert to managed initializations

 drivers/irqchip/irq-renesas-irqc.c | 88 ++++++++++--------------------
 1 file changed, 30 insertions(+), 58 deletions(-)

-- 
2.17.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

[PATCH 1/5] irqchip/renesas-irqc: Remove unneeded inclusion of <linux/spinlock.h>

[Geert Uytterhoeven]

The driver never used spinlocks, and thus does not need to include
<linux/spinlock.h>.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/irqchip/irq-renesas-irqc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/irqchip/irq-renesas-irqc.c b/drivers/irqchip/irq-renesas-irqc.c
index a449a7c839b3ec08..438a063c76156d98 100644
--- a/drivers/irqchip/irq-renesas-irqc.c
+++ b/drivers/irqchip/irq-renesas-irqc.c
@@ -7,7 +7,6 @@
 
 #include <linux/init.h>
 #include <linux/platform_device.h>
-#include <linux/spinlock.h>
 #include <linux/interrupt.h>
 #include <linux/ioport.h>
 #include <linux/io.h>
-- 
2.17.1

[PATCH 2/5] irqchip/renesas-irqc: Remove devm_kzalloc()/ioremap_nocache() error printing

[Geert Uytterhoeven]

There is no need to print a message if devm_kzalloc() or
ioremap_nocache() fails, as the memory allocation core already takes
care of that.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/irqchip/irq-renesas-irqc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-irqc.c b/drivers/irqchip/irq-renesas-irqc.c
index 438a063c76156d98..0955ffe12b32eb36 100644
--- a/drivers/irqchip/irq-renesas-irqc.c
+++ b/drivers/irqchip/irq-renesas-irqc.c
@@ -133,7 +133,6 @@ static int irqc_probe(struct platform_device *pdev)
 
 	p = kzalloc(sizeof(*p), GFP_KERNEL);
 	if (!p) {
-		dev_err(&pdev->dev, "failed to allocate driver data\n");
 		ret = -ENOMEM;
 		goto err0;
 	}
@@ -173,7 +172,6 @@ static int irqc_probe(struct platform_device *pdev)
 	/* ioremap IOMEM and setup read/write callbacks */
 	p->iomem = ioremap_nocache(io->start, resource_size(io));
 	if (!p->iomem) {
-		dev_err(&pdev->dev, "failed to remap IOMEM\n");
 		ret = -ENXIO;
 		goto err2;
 	}
-- 
2.17.1

[PATCH 3/5] irqchip/renesas-irqc: Add helper variable dev = &pdev->dev

[Geert Uytterhoeven]

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/irqchip/irq-renesas-irqc.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-irqc.c b/drivers/irqchip/irq-renesas-irqc.c
index 0955ffe12b32eb36..3cc428ba495c3793 100644
--- a/drivers/irqchip/irq-renesas-irqc.c
+++ b/drivers/irqchip/irq-renesas-irqc.c
@@ -124,10 +124,11 @@ static irqreturn_t irqc_irq_handler(int irq, void *dev_id)
 
 static int irqc_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
+	const char *name = dev_name(dev);
 	struct irqc_priv *p;
 	struct resource *io;
 	struct resource *irq;
-	const char *name = dev_name(&pdev->dev);
 	int ret;
 	int k;
 
@@ -140,13 +141,13 @@ static int irqc_probe(struct platform_device *pdev)
 	p->pdev = pdev;
 	platform_set_drvdata(pdev, p);
 
-	pm_runtime_enable(&pdev->dev);
-	pm_runtime_get_sync(&pdev->dev);
+	pm_runtime_enable(dev);
+	pm_runtime_get_sync(dev);
 
 	/* get hold of manadatory IOMEM */
 	io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!io) {
-		dev_err(&pdev->dev, "not enough IOMEM resources\n");
+		dev_err(dev, "not enough IOMEM resources\n");
 		ret = -EINVAL;
 		goto err1;
 	}
@@ -164,7 +165,7 @@ static int irqc_probe(struct platform_device *pdev)
 
 	p->number_of_irqs = k;
 	if (p->number_of_irqs < 1) {
-		dev_err(&pdev->dev, "not enough IRQ resources\n");
+		dev_err(dev, "not enough IRQ resources\n");
 		ret = -EINVAL;
 		goto err1;
 	}
@@ -178,12 +179,11 @@ static int irqc_probe(struct platform_device *pdev)
 
 	p->cpu_int_base = p->iomem + IRQC_INT_CPU_BASE(0); /* SYS-SPI */
 
-	p->irq_domain = irq_domain_add_linear(pdev->dev.of_node,
-					      p->number_of_irqs,
+	p->irq_domain = irq_domain_add_linear(dev->of_node, p->number_of_irqs,
 					      &irq_generic_chip_ops, p);
 	if (!p->irq_domain) {
 		ret = -ENXIO;
-		dev_err(&pdev->dev, "cannot initialize irq domain\n");
+		dev_err(dev, "cannot initialize irq domain\n");
 		goto err2;
 	}
 
@@ -191,7 +191,7 @@ static int irqc_probe(struct platform_device *pdev)
 					     1, name, handle_level_irq,
 					     0, 0, IRQ_GC_INIT_NESTED_LOCK);
 	if (ret) {
-		dev_err(&pdev->dev, "cannot allocate generic chip\n");
+		dev_err(dev, "cannot allocate generic chip\n");
 		goto err3;
 	}
 
@@ -209,13 +209,13 @@ static int irqc_probe(struct platform_device *pdev)
 	for (k = 0; k < p->number_of_irqs; k++) {
 		if (request_irq(p->irq[k].requested_irq, irqc_irq_handler,
 				0, name, &p->irq[k])) {
-			dev_err(&pdev->dev, "failed to request IRQ\n");
+			dev_err(dev, "failed to request IRQ\n");
 			ret = -ENOENT;
 			goto err4;
 		}
 	}
 
-	dev_info(&pdev->dev, "driving %d irqs\n", p->number_of_irqs);
+	dev_info(dev, "driving %d irqs\n", p->number_of_irqs);
 
 	return 0;
 err4:
@@ -227,8 +227,8 @@ static int irqc_probe(struct platform_device *pdev)
 err2:
 	iounmap(p->iomem);
 err1:
-	pm_runtime_put(&pdev->dev);
-	pm_runtime_disable(&pdev->dev);
+	pm_runtime_put(dev);
+	pm_runtime_disable(dev);
 	kfree(p);
 err0:
 	return ret;
-- 
2.17.1

[PATCH 4/5] irqchip/renesas-irqc: Replace irqc_priv.pdev by irqc_priv.dev

[Geert Uytterhoeven]

Nothing really uses irqc_priv.pdev, all users need irqc_priv.pdev->dev.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/irqchip/irq-renesas-irqc.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-irqc.c b/drivers/irqchip/irq-renesas-irqc.c
index 3cc428ba495c3793..af03ee31a87bb11d 100644
--- a/drivers/irqchip/irq-renesas-irqc.c
+++ b/drivers/irqchip/irq-renesas-irqc.c
@@ -47,7 +47,7 @@ struct irqc_priv {
 	void __iomem *cpu_int_base;
 	struct irqc_irq irq[IRQC_IRQ_MAX];
 	unsigned int number_of_irqs;
-	struct platform_device *pdev;
+	struct device *dev;
 	struct irq_chip_generic *gc;
 	struct irq_domain *irq_domain;
 	atomic_t wakeup_path;
@@ -60,8 +60,7 @@ static struct irqc_priv *irq_data_to_priv(struct irq_data *data)
 
 static void irqc_dbg(struct irqc_irq *i, char *str)
 {
-	dev_dbg(&i->p->pdev->dev, "%s (%d:%d)\n",
-		str, i->requested_irq, i->hw_irq);
+	dev_dbg(i->p->dev, "%s (%d:%d)\n", str, i->requested_irq, i->hw_irq);
 }
 
 static unsigned char irqc_sense[IRQ_TYPE_SENSE_MASK + 1] = {
@@ -138,7 +137,7 @@ static int irqc_probe(struct platform_device *pdev)
 		goto err0;
 	}
 
-	p->pdev = pdev;
+	p->dev = dev;
 	platform_set_drvdata(pdev, p);
 
 	pm_runtime_enable(dev);
-- 
2.17.1

[PATCH 5/5] irqchip/renesas-irqc: Convert to managed initializations

[Geert Uytterhoeven]

Simplify error handling by converting the driver to use managed
allocations and initializations.

Note that platform_get_resource() and ioremap_nocache() are combined in
devm_platform_ioremap_resource().

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/irqchip/irq-renesas-irqc.c | 54 +++++++++---------------------
 1 file changed, 15 insertions(+), 39 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-irqc.c b/drivers/irqchip/irq-renesas-irqc.c
index af03ee31a87bb11d..cde9f9c0687e94a4 100644
--- a/drivers/irqchip/irq-renesas-irqc.c
+++ b/drivers/irqchip/irq-renesas-irqc.c
@@ -126,16 +126,13 @@ static int irqc_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	const char *name = dev_name(dev);
 	struct irqc_priv *p;
-	struct resource *io;
 	struct resource *irq;
 	int ret;
 	int k;
 
-	p = kzalloc(sizeof(*p), GFP_KERNEL);
-	if (!p) {
-		ret = -ENOMEM;
-		goto err0;
-	}
+	p = devm_kzalloc(dev, sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
 
 	p->dev = dev;
 	platform_set_drvdata(pdev, p);
@@ -143,14 +140,6 @@ static int irqc_probe(struct platform_device *pdev)
 	pm_runtime_enable(dev);
 	pm_runtime_get_sync(dev);
 
-	/* get hold of manadatory IOMEM */
-	io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!io) {
-		dev_err(dev, "not enough IOMEM resources\n");
-		ret = -EINVAL;
-		goto err1;
-	}
-
 	/* allow any number of IRQs between 1 and IRQC_IRQ_MAX */
 	for (k = 0; k < IRQC_IRQ_MAX; k++) {
 		irq = platform_get_resource(pdev, IORESOURCE_IRQ, k);
@@ -166,14 +155,14 @@ static int irqc_probe(struct platform_device *pdev)
 	if (p->number_of_irqs < 1) {
 		dev_err(dev, "not enough IRQ resources\n");
 		ret = -EINVAL;
-		goto err1;
+		goto err_runtime_pm_disable;
 	}
 
 	/* ioremap IOMEM and setup read/write callbacks */
-	p->iomem = ioremap_nocache(io->start, resource_size(io));
-	if (!p->iomem) {
-		ret = -ENXIO;
-		goto err2;
+	p->iomem = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(p->iomem)) {
+		ret = PTR_ERR(p->iomem);
+		goto err_runtime_pm_disable;
 	}
 
 	p->cpu_int_base = p->iomem + IRQC_INT_CPU_BASE(0); /* SYS-SPI */
@@ -183,7 +172,7 @@ static int irqc_probe(struct platform_device *pdev)
 	if (!p->irq_domain) {
 		ret = -ENXIO;
 		dev_err(dev, "cannot initialize irq domain\n");
-		goto err2;
+		goto err_runtime_pm_disable;
 	}
 
 	ret = irq_alloc_domain_generic_chips(p->irq_domain, p->number_of_irqs,
@@ -191,7 +180,7 @@ static int irqc_probe(struct platform_device *pdev)
 					     0, 0, IRQ_GC_INIT_NESTED_LOCK);
 	if (ret) {
 		dev_err(dev, "cannot allocate generic chip\n");
-		goto err3;
+		goto err_remove_domain;
 	}
 
 	p->gc = irq_get_domain_generic_chip(p->irq_domain, 0);
@@ -206,46 +195,33 @@ static int irqc_probe(struct platform_device *pdev)
 
 	/* request interrupts one by one */
 	for (k = 0; k < p->number_of_irqs; k++) {
-		if (request_irq(p->irq[k].requested_irq, irqc_irq_handler,
-				0, name, &p->irq[k])) {
+		if (devm_request_irq(dev, p->irq[k].requested_irq,
+				     irqc_irq_handler, 0, name, &p->irq[k])) {
 			dev_err(dev, "failed to request IRQ\n");
 			ret = -ENOENT;
-			goto err4;
+			goto err_remove_domain;
 		}
 	}
 
 	dev_info(dev, "driving %d irqs\n", p->number_of_irqs);
 
 	return 0;
-err4:
-	while (--k >= 0)
-		free_irq(p->irq[k].requested_irq, &p->irq[k]);
 
-err3:
+err_remove_domain:
 	irq_domain_remove(p->irq_domain);
-err2:
-	iounmap(p->iomem);
-err1:
+err_runtime_pm_disable:
 	pm_runtime_put(dev);
 	pm_runtime_disable(dev);
-	kfree(p);
-err0:
 	return ret;
 }
 
 static int irqc_remove(struct platform_device *pdev)
 {
 	struct irqc_priv *p = platform_get_drvdata(pdev);
-	int k;
-
-	for (k = 0; k < p->number_of_irqs; k++)
-		free_irq(p->irq[k].requested_irq, &p->irq[k]);
 
 	irq_domain_remove(p->irq_domain);
-	iounmap(p->iomem);
 	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-	kfree(p);
 	return 0;
 }
 
-- 
2.17.1

Re: [PATCH 2/5] irqchip/renesas-irqc: Remove devm_kzalloc()/ioremap_nocache() error printing

[Sergei Shtylyov]

Hello!

On 29.04.2019 18:20, Geert Uytterhoeven wrote:

> There is no need to print a message if devm_kzalloc() or

    Just kzalloc() in this case.

> ioremap_nocache() fails, as the memory allocation core already takes
> care of that.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>   drivers/irqchip/irq-renesas-irqc.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-renesas-irqc.c b/drivers/irqchip/irq-renesas-irqc.c
> index 438a063c76156d98..0955ffe12b32eb36 100644
> --- a/drivers/irqchip/irq-renesas-irqc.c
> +++ b/drivers/irqchip/irq-renesas-irqc.c
> @@ -133,7 +133,6 @@ static int irqc_probe(struct platform_device *pdev)
>   
>   	p = kzalloc(sizeof(*p), GFP_KERNEL);
>   	if (!p) {
> -		dev_err(&pdev->dev, "failed to allocate driver data\n");
>   		ret = -ENOMEM;
>   		goto err0;
>   	}
> @@ -173,7 +172,6 @@ static int irqc_probe(struct platform_device *pdev)
>   	/* ioremap IOMEM and setup read/write callbacks */
>   	p->iomem = ioremap_nocache(io->start, resource_size(io));
>   	if (!p->iomem) {
> -		dev_err(&pdev->dev, "failed to remap IOMEM\n");
>   		ret = -ENXIO;

    -ENOMEM?

[...]

MBR, Sergei

Re: [PATCH 2/5] irqchip/renesas-irqc: Remove devm_kzalloc()/ioremap_nocache() error printing

[Geert Uytterhoeven]

Hi Sergei,

On Tue, Apr 30, 2019 at 10:12 AM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 29.04.2019 18:20, Geert Uytterhoeven wrote:
> > There is no need to print a message if devm_kzalloc() or
>
>     Just kzalloc() in this case.

Thanks, silly copy-and-paste error.

> > --- a/drivers/irqchip/irq-renesas-irqc.c
> > +++ b/drivers/irqchip/irq-renesas-irqc.c
> > @@ -173,7 +172,6 @@ static int irqc_probe(struct platform_device *pdev)
> >       /* ioremap IOMEM and setup read/write callbacks */
> >       p->iomem = ioremap_nocache(io->start, resource_size(io));
> >       if (!p->iomem) {
> > -             dev_err(&pdev->dev, "failed to remap IOMEM\n");
> >               ret = -ENXIO;
>
>     -ENOMEM?

I'd say yes, except that this error code is removed in patch [5/5], so I don't
think it's worth changing it in this patch.

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH 1/5] irqchip/renesas-irqc: Remove unneeded inclusion of <linux/spinlock.h>

[Simon Horman]

On Mon, Apr 29, 2019 at 05:20:02PM +0200, Geert Uytterhoeven wrote:
> The driver never used spinlocks, and thus does not need to include
> <linux/spinlock.h>.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> ---
>  drivers/irqchip/irq-renesas-irqc.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-renesas-irqc.c b/drivers/irqchip/irq-renesas-irqc.c
> index a449a7c839b3ec08..438a063c76156d98 100644
> --- a/drivers/irqchip/irq-renesas-irqc.c
> +++ b/drivers/irqchip/irq-renesas-irqc.c
> @@ -7,7 +7,6 @@
>  
>  #include <linux/init.h>
>  #include <linux/platform_device.h>
> -#include <linux/spinlock.h>
>  #include <linux/interrupt.h>
>  #include <linux/ioport.h>
>  #include <linux/io.h>
> -- 
> 2.17.1
> 

Re: [PATCH 2/5] irqchip/renesas-irqc: Remove devm_kzalloc()/ioremap_nocache() error printing

[Simon Horman]

On Mon, Apr 29, 2019 at 05:20:03PM +0200, Geert Uytterhoeven wrote:
> There is no need to print a message if devm_kzalloc() or
> ioremap_nocache() fails, as the memory allocation core already takes
> care of that.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> ---
>  drivers/irqchip/irq-renesas-irqc.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-renesas-irqc.c b/drivers/irqchip/irq-renesas-irqc.c
> index 438a063c76156d98..0955ffe12b32eb36 100644
> --- a/drivers/irqchip/irq-renesas-irqc.c
> +++ b/drivers/irqchip/irq-renesas-irqc.c
> @@ -133,7 +133,6 @@ static int irqc_probe(struct platform_device *pdev)
>  
>  	p = kzalloc(sizeof(*p), GFP_KERNEL);
>  	if (!p) {
> -		dev_err(&pdev->dev, "failed to allocate driver data\n");
>  		ret = -ENOMEM;
>  		goto err0;
>  	}
> @@ -173,7 +172,6 @@ static int irqc_probe(struct platform_device *pdev)
>  	/* ioremap IOMEM and setup read/write callbacks */
>  	p->iomem = ioremap_nocache(io->start, resource_size(io));
>  	if (!p->iomem) {
> -		dev_err(&pdev->dev, "failed to remap IOMEM\n");
>  		ret = -ENXIO;
>  		goto err2;
>  	}
> -- 
> 2.17.1
> 

Re: [PATCH 3/5] irqchip/renesas-irqc: Add helper variable dev = &pdev->dev

[Simon Horman]

On Mon, Apr 29, 2019 at 05:20:04PM +0200, Geert Uytterhoeven wrote:

I think some text needs to go here.

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

With an (updated) changelog feel free to add:

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 4/5] irqchip/renesas-irqc: Replace irqc_priv.pdev by irqc_priv.dev

[Simon Horman]

On Mon, Apr 29, 2019 at 05:20:05PM +0200, Geert Uytterhoeven wrote:
> Nothing really uses irqc_priv.pdev, all users need irqc_priv.pdev->dev.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 5/5] irqchip/renesas-irqc: Convert to managed initializations

[Simon Horman]

On Mon, Apr 29, 2019 at 05:20:06PM +0200, Geert Uytterhoeven wrote:
> Simplify error handling by converting the driver to use managed
> allocations and initializations.
> 
> Note that platform_get_resource() and ioremap_nocache() are combined in
> devm_platform_ioremap_resource().
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>