[PATCH 1/4] ARM: common/sa1111: remove NO_IRQ check

[PATCH 1/4] ARM: common/sa1111: remove NO_IRQ check

[Arnd Bergmann]

Since commit 489447380a29 ("[PATCH] handle errors returned by
platform_get_irq*()") ten years ago, the sa1111 driver refuses to
work without an interrupt line passed in its resources, so the
check for NO_IRQ is unnecessary.

I have also checked that all four machines files that register
an sa1111 device (lubbock, badge4, journada720, and neponset)
do set an interrupt line.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/common/sa1111.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/arch/arm/common/sa1111.c b/arch/arm/common/sa1111.c
index fb0a0a4dfea4..64d8cf08b7d0 100644
--- a/arch/arm/common/sa1111.c
+++ b/arch/arm/common/sa1111.c
@@ -751,11 +751,9 @@ static int __sa1111_probe(struct device *me, struct resource *mem, int irq)
 	 * The interrupt controller must be initialised before any
 	 * other device to ensure that the interrupts are available.
 	 */
-	if (sachip->irq != NO_IRQ) {
-		ret = sa1111_setup_irq(sachip, pd->irq_base);
-		if (ret)
-			goto err_unmap;
-	}
+	ret = sa1111_setup_irq(sachip, pd->irq_base);
+	if (ret)
+		goto err_unmap;
 
 #ifdef CONFIG_ARCH_SA1100
 	{
@@ -834,12 +832,10 @@ static void __sa1111_remove(struct sa1111 *sachip)
 	clk_disable(sachip->clk);
 	clk_unprepare(sachip->clk);
 
-	if (sachip->irq != NO_IRQ) {
-		irq_set_chained_handler_and_data(sachip->irq, NULL, NULL);
-		irq_free_descs(sachip->irq_base, SA1111_IRQ_NR);
+	irq_set_chained_handler_and_data(sachip->irq, NULL, NULL);
+	irq_free_descs(sachip->irq_base, SA1111_IRQ_NR);
 
-		release_mem_region(sachip->phys + SA1111_INTC, 512);
-	}
+	release_mem_region(sachip->phys + SA1111_INTC, 512);
 
 	iounmap(sachip->base);
 	clk_put(sachip->clk);
-- 
2.9.0

[PATCH 1/4] ARM: common/sa1111: remove NO_IRQ check

[Arnd Bergmann]

Since commit 489447380a29 ("[PATCH] handle errors returned by
platform_get_irq*()") ten years ago, the sa1111 driver refuses to
work without an interrupt line passed in its resources, so the
check for NO_IRQ is unnecessary.

I have also checked that all four machines files that register
an sa1111 device (lubbock, badge4, journada720, and neponset)
do set an interrupt line.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/common/sa1111.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/arch/arm/common/sa1111.c b/arch/arm/common/sa1111.c
index fb0a0a4dfea4..64d8cf08b7d0 100644
--- a/arch/arm/common/sa1111.c
+++ b/arch/arm/common/sa1111.c
@@ -751,11 +751,9 @@ static int __sa1111_probe(struct device *me, struct resource *mem, int irq)
 	 * The interrupt controller must be initialised before any
 	 * other device to ensure that the interrupts are available.
 	 */
-	if (sachip->irq != NO_IRQ) {
-		ret = sa1111_setup_irq(sachip, pd->irq_base);
-		if (ret)
-			goto err_unmap;
-	}
+	ret = sa1111_setup_irq(sachip, pd->irq_base);
+	if (ret)
+		goto err_unmap;
 
 #ifdef CONFIG_ARCH_SA1100
 	{
@@ -834,12 +832,10 @@ static void __sa1111_remove(struct sa1111 *sachip)
 	clk_disable(sachip->clk);
 	clk_unprepare(sachip->clk);
 
-	if (sachip->irq != NO_IRQ) {
-		irq_set_chained_handler_and_data(sachip->irq, NULL, NULL);
-		irq_free_descs(sachip->irq_base, SA1111_IRQ_NR);
+	irq_set_chained_handler_and_data(sachip->irq, NULL, NULL);
+	irq_free_descs(sachip->irq_base, SA1111_IRQ_NR);
 
-		release_mem_region(sachip->phys + SA1111_INTC, 512);
-	}
+	release_mem_region(sachip->phys + SA1111_INTC, 512);
 
 	iounmap(sachip->base);
 	clk_put(sachip->clk);
-- 
2.9.0

[PATCH 2/4] ARM: common/locomo: remove NO_IRQ check

[Arnd Bergmann]

Since commit 489447380a29 ("[PATCH] handle errors returned by
platform_get_irq*()") ten years ago, the locomo driver refuses to
work without an interrupt line passed in its resources, so the
check for NO_IRQ is unnecessary.

We still check the irq_base argument for NO_IRQ, but as both
platforms that use locomo (poodle and collie) provide both
'irq' and 'irq_base', this can be done more consistently
by just checking that both are valid in the probe function
and otherwise returning an error.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/common/locomo.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c
index 0e97b4b871f9..81abb04e5254 100644
--- a/arch/arm/common/locomo.c
+++ b/arch/arm/common/locomo.c
@@ -253,8 +253,7 @@ locomo_init_one_child(struct locomo *lchip, struct locomo_dev_info *info)
 		dev->mapbase = 0;
 	dev->length = info->length;
 
-	dev->irq[0] = (lchip->irq_base == NO_IRQ) ?
-			NO_IRQ : lchip->irq_base + info->irq[0];
+	dev->irq[0] = lchip->irq_base + info->irq[0];
 
 	ret = device_register(&dev->dev);
 	if (ret) {
@@ -376,6 +375,9 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
 	unsigned long r;
 	int i, ret = -ENODEV;
 
+	if (!pdata->irq_base)
+		return ret;
+
 	lchip = kzalloc(sizeof(struct locomo), GFP_KERNEL);
 	if (!lchip)
 		return -ENOMEM;
@@ -387,7 +389,7 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
 
 	lchip->phys = mem->start;
 	lchip->irq = irq;
-	lchip->irq_base = (pdata) ? pdata->irq_base : NO_IRQ;
+	lchip->irq_base = pdata->irq_base;
 
 	/*
 	 * Map the whole region.  This also maps the
@@ -454,8 +456,7 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
 	 * The interrupt controller must be initialised before any
 	 * other device to ensure that the interrupts are available.
 	 */
-	if (lchip->irq != NO_IRQ && lchip->irq_base != NO_IRQ)
-		locomo_setup_irq(lchip);
+	locomo_setup_irq(lchip);
 
 	for (i = 0; i < ARRAY_SIZE(locomo_devices); i++)
 		locomo_init_one_child(lchip, &locomo_devices[i]);
@@ -476,9 +477,7 @@ static void __locomo_remove(struct locomo *lchip)
 {
 	device_for_each_child(lchip->dev, NULL, locomo_remove_child);
 
-	if (lchip->irq != NO_IRQ) {
-		irq_set_chained_handler_and_data(lchip->irq, NULL, NULL);
-	}
+	irq_set_chained_handler_and_data(lchip->irq, NULL, NULL);
 
 	iounmap(lchip->base);
 	kfree(lchip);
-- 
2.9.0

[PATCH 2/4] ARM: common/locomo: remove NO_IRQ check

[Arnd Bergmann]

Since commit 489447380a29 ("[PATCH] handle errors returned by
platform_get_irq*()") ten years ago, the locomo driver refuses to
work without an interrupt line passed in its resources, so the
check for NO_IRQ is unnecessary.

We still check the irq_base argument for NO_IRQ, but as both
platforms that use locomo (poodle and collie) provide both
'irq' and 'irq_base', this can be done more consistently
by just checking that both are valid in the probe function
and otherwise returning an error.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/common/locomo.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c
index 0e97b4b871f9..81abb04e5254 100644
--- a/arch/arm/common/locomo.c
+++ b/arch/arm/common/locomo.c
@@ -253,8 +253,7 @@ locomo_init_one_child(struct locomo *lchip, struct locomo_dev_info *info)
 		dev->mapbase = 0;
 	dev->length = info->length;
 
-	dev->irq[0] = (lchip->irq_base == NO_IRQ) ?
-			NO_IRQ : lchip->irq_base + info->irq[0];
+	dev->irq[0] = lchip->irq_base + info->irq[0];
 
 	ret = device_register(&dev->dev);
 	if (ret) {
@@ -376,6 +375,9 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
 	unsigned long r;
 	int i, ret = -ENODEV;
 
+	if (!pdata->irq_base)
+		return ret;
+
 	lchip = kzalloc(sizeof(struct locomo), GFP_KERNEL);
 	if (!lchip)
 		return -ENOMEM;
@@ -387,7 +389,7 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
 
 	lchip->phys = mem->start;
 	lchip->irq = irq;
-	lchip->irq_base = (pdata) ? pdata->irq_base : NO_IRQ;
+	lchip->irq_base = pdata->irq_base;
 
 	/*
 	 * Map the whole region.  This also maps the
@@ -454,8 +456,7 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
 	 * The interrupt controller must be initialised before any
 	 * other device to ensure that the interrupts are available.
 	 */
-	if (lchip->irq != NO_IRQ && lchip->irq_base != NO_IRQ)
-		locomo_setup_irq(lchip);
+	locomo_setup_irq(lchip);
 
 	for (i = 0; i < ARRAY_SIZE(locomo_devices); i++)
 		locomo_init_one_child(lchip, &locomo_devices[i]);
@@ -476,9 +477,7 @@ static void __locomo_remove(struct locomo *lchip)
 {
 	device_for_each_child(lchip->dev, NULL, locomo_remove_child);
 
-	if (lchip->irq != NO_IRQ) {
-		irq_set_chained_handler_and_data(lchip->irq, NULL, NULL);
-	}
+	irq_set_chained_handler_and_data(lchip->irq, NULL, NULL);
 
 	iounmap(lchip->base);
 	kfree(lchip);
-- 
2.9.0

[PATCH 3/4] mfd: ucb1x00: remove NO_IRQ check

[Arnd Bergmann]

probe_irq_off() returns '0' on failure, not NO_IRQ, so the check
in this driver is clearly wrong. This replaces it with the
regular '!irq' check used in other drivers.

The sa1100 platform that this driver is used on originally numbered
all its interrupts starting at '0', which would have conflicted with
this change, but as of commit 18f3aec ("ARM: 8230/1: sa1100: shift
IRQs by one"), this is not a problem any more.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/mfd/ucb1x00-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/ucb1x00-core.c b/drivers/mfd/ucb1x00-core.c
index 48bea5038654..d6fb2e1a759a 100644
--- a/drivers/mfd/ucb1x00-core.c
+++ b/drivers/mfd/ucb1x00-core.c
@@ -537,7 +537,7 @@ static int ucb1x00_probe(struct mcp *mcp)
 	ucb1x00_enable(ucb);
 	ucb->irq = ucb1x00_detect_irq(ucb);
 	ucb1x00_disable(ucb);
-	if (ucb->irq == NO_IRQ) {
+	if (!ucb->irq) {
 		dev_err(&ucb->dev, "IRQ probe failed\n");
 		ret = -ENODEV;
 		goto err_no_irq;
-- 
2.9.0

[PATCH 4/4] pcmcia: soc-common: remove incorrect NO_IRQ use

[Arnd Bergmann]

The soc_common driver (used on ARM sa1100 and pxa) initializes the
socket->pci_irq member to NO_IRQ by default to signify an invalid
interrupt, and normally overrides this with a proper interrupt later.

However, the code that checks socked->pci_irq for validity compares
it to zero instead of NO_IRQ, as most drivers do, so this cannot
work right. While zero is a valid interrupt number on PXA (and
in the past also on sa1100), it is the interrupt line for the 'ssp'
serial port, so there is no possible conflict in practice and
we can simply change the default to zero.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/pcmcia/soc_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pcmcia/soc_common.c b/drivers/pcmcia/soc_common.c
index eed5e9c05353..339ce29fa97b 100644
--- a/drivers/pcmcia/soc_common.c
+++ b/drivers/pcmcia/soc_common.c
@@ -691,7 +691,7 @@ void soc_pcmcia_init_one(struct soc_pcmcia_socket *skt,
 	skt->ops = ops;
 	skt->socket.owner = ops->owner;
 	skt->socket.dev.parent = dev;
-	skt->socket.pci_irq = NO_IRQ;
+	skt->socket.pci_irq = 0;
 
 	for (i = 0; i < ARRAY_SIZE(skt->stat); i++)
 		skt->stat[i].gpio = -EINVAL;
-- 
2.9.0

Re: [PATCH 1/4] ARM: common/sa1111: remove NO_IRQ check

[Russell King - ARM Linux]

Please check other patches previously sent - these conflict with the
patch series I sent last week.  This use is already gone.

On Tue, Sep 06, 2016 at 03:53:27PM +0200, Arnd Bergmann wrote:
> Since commit 489447380a29 ("[PATCH] handle errors returned by
> platform_get_irq*()") ten years ago, the sa1111 driver refuses to
> work without an interrupt line passed in its resources, so the
> check for NO_IRQ is unnecessary.
> 
> I have also checked that all four machines files that register
> an sa1111 device (lubbock, badge4, journada720, and neponset)
> do set an interrupt line.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/common/sa1111.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/common/sa1111.c b/arch/arm/common/sa1111.c
> index fb0a0a4dfea4..64d8cf08b7d0 100644
> --- a/arch/arm/common/sa1111.c
> +++ b/arch/arm/common/sa1111.c
> @@ -751,11 +751,9 @@ static int __sa1111_probe(struct device *me, struct resource *mem, int irq)
>  	 * The interrupt controller must be initialised before any
>  	 * other device to ensure that the interrupts are available.
>  	 */
> -	if (sachip->irq != NO_IRQ) {
> -		ret = sa1111_setup_irq(sachip, pd->irq_base);
> -		if (ret)
> -			goto err_unmap;
> -	}
> +	ret = sa1111_setup_irq(sachip, pd->irq_base);
> +	if (ret)
> +		goto err_unmap;
>  
>  #ifdef CONFIG_ARCH_SA1100
>  	{
> @@ -834,12 +832,10 @@ static void __sa1111_remove(struct sa1111 *sachip)
>  	clk_disable(sachip->clk);
>  	clk_unprepare(sachip->clk);
>  
> -	if (sachip->irq != NO_IRQ) {
> -		irq_set_chained_handler_and_data(sachip->irq, NULL, NULL);
> -		irq_free_descs(sachip->irq_base, SA1111_IRQ_NR);
> +	irq_set_chained_handler_and_data(sachip->irq, NULL, NULL);
> +	irq_free_descs(sachip->irq_base, SA1111_IRQ_NR);
>  
> -		release_mem_region(sachip->phys + SA1111_INTC, 512);
> -	}
> +	release_mem_region(sachip->phys + SA1111_INTC, 512);
>  
>  	iounmap(sachip->base);
>  	clk_put(sachip->clk);
> -- 
> 2.9.0
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 1/4] ARM: common/sa1111: remove NO_IRQ check

[Russell King - ARM Linux]

Please check other patches previously sent - these conflict with the
patch series I sent last week.  This use is already gone.

On Tue, Sep 06, 2016 at 03:53:27PM +0200, Arnd Bergmann wrote:
> Since commit 489447380a29 ("[PATCH] handle errors returned by
> platform_get_irq*()") ten years ago, the sa1111 driver refuses to
> work without an interrupt line passed in its resources, so the
> check for NO_IRQ is unnecessary.
> 
> I have also checked that all four machines files that register
> an sa1111 device (lubbock, badge4, journada720, and neponset)
> do set an interrupt line.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/common/sa1111.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/common/sa1111.c b/arch/arm/common/sa1111.c
> index fb0a0a4dfea4..64d8cf08b7d0 100644
> --- a/arch/arm/common/sa1111.c
> +++ b/arch/arm/common/sa1111.c
> @@ -751,11 +751,9 @@ static int __sa1111_probe(struct device *me, struct resource *mem, int irq)
>  	 * The interrupt controller must be initialised before any
>  	 * other device to ensure that the interrupts are available.
>  	 */
> -	if (sachip->irq != NO_IRQ) {
> -		ret = sa1111_setup_irq(sachip, pd->irq_base);
> -		if (ret)
> -			goto err_unmap;
> -	}
> +	ret = sa1111_setup_irq(sachip, pd->irq_base);
> +	if (ret)
> +		goto err_unmap;
>  
>  #ifdef CONFIG_ARCH_SA1100
>  	{
> @@ -834,12 +832,10 @@ static void __sa1111_remove(struct sa1111 *sachip)
>  	clk_disable(sachip->clk);
>  	clk_unprepare(sachip->clk);
>  
> -	if (sachip->irq != NO_IRQ) {
> -		irq_set_chained_handler_and_data(sachip->irq, NULL, NULL);
> -		irq_free_descs(sachip->irq_base, SA1111_IRQ_NR);
> +	irq_set_chained_handler_and_data(sachip->irq, NULL, NULL);
> +	irq_free_descs(sachip->irq_base, SA1111_IRQ_NR);
>  
> -		release_mem_region(sachip->phys + SA1111_INTC, 512);
> -	}
> +	release_mem_region(sachip->phys + SA1111_INTC, 512);
>  
>  	iounmap(sachip->base);
>  	clk_put(sachip->clk);
> -- 
> 2.9.0
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/4] ARM: common/locomo: remove NO_IRQ check

[Russell King - ARM Linux]

On Tue, Sep 06, 2016 at 03:53:28PM +0200, Arnd Bergmann wrote:
> Since commit 489447380a29 ("[PATCH] handle errors returned by
> platform_get_irq*()") ten years ago, the locomo driver refuses to
> work without an interrupt line passed in its resources, so the
> check for NO_IRQ is unnecessary.

This description is inaccurate and misleading (it looks like it was
cut'n'pasted from patch 1.)

platform_get_irq() has nothing to do with your change, as your change
is more about the irq_base value passed through platform data, and
not through IRQ resources.

> We still check the irq_base argument for NO_IRQ, but as both
> platforms that use locomo (poodle and collie) provide both
> 'irq' and 'irq_base', this can be done more consistently
> by just checking that both are valid in the probe function
> and otherwise returning an error.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/common/locomo.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c
> index 0e97b4b871f9..81abb04e5254 100644
> --- a/arch/arm/common/locomo.c
> +++ b/arch/arm/common/locomo.c
> @@ -253,8 +253,7 @@ locomo_init_one_child(struct locomo *lchip, struct locomo_dev_info *info)
>  		dev->mapbase = 0;
>  	dev->length = info->length;
>  
> -	dev->irq[0] = (lchip->irq_base == NO_IRQ) ?
> -			NO_IRQ : lchip->irq_base + info->irq[0];
> +	dev->irq[0] = lchip->irq_base + info->irq[0];
>  
>  	ret = device_register(&dev->dev);
>  	if (ret) {
> @@ -376,6 +375,9 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
>  	unsigned long r;
>  	int i, ret = -ENODEV;
>  
> +	if (!pdata->irq_base)
> +		return ret;
> +
>  	lchip = kzalloc(sizeof(struct locomo), GFP_KERNEL);
>  	if (!lchip)
>  		return -ENOMEM;
> @@ -387,7 +389,7 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
>  
>  	lchip->phys = mem->start;
>  	lchip->irq = irq;
> -	lchip->irq_base = (pdata) ? pdata->irq_base : NO_IRQ;
> +	lchip->irq_base = pdata->irq_base;

This removes a NULL pointer check.  Before this change, a NULL pdata
would be accepted and would lead to the interrupts not being setup.
After this change, it results in a NULL pointer deference.

Thankfully, both collie and poodle supply platform data, and are the
only providers of the locomo device.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 2/4] ARM: common/locomo: remove NO_IRQ check

[Russell King - ARM Linux]

On Tue, Sep 06, 2016 at 03:53:28PM +0200, Arnd Bergmann wrote:
> Since commit 489447380a29 ("[PATCH] handle errors returned by
> platform_get_irq*()") ten years ago, the locomo driver refuses to
> work without an interrupt line passed in its resources, so the
> check for NO_IRQ is unnecessary.

This description is inaccurate and misleading (it looks like it was
cut'n'pasted from patch 1.)

platform_get_irq() has nothing to do with your change, as your change
is more about the irq_base value passed through platform data, and
not through IRQ resources.

> We still check the irq_base argument for NO_IRQ, but as both
> platforms that use locomo (poodle and collie) provide both
> 'irq' and 'irq_base', this can be done more consistently
> by just checking that both are valid in the probe function
> and otherwise returning an error.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/common/locomo.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c
> index 0e97b4b871f9..81abb04e5254 100644
> --- a/arch/arm/common/locomo.c
> +++ b/arch/arm/common/locomo.c
> @@ -253,8 +253,7 @@ locomo_init_one_child(struct locomo *lchip, struct locomo_dev_info *info)
>  		dev->mapbase = 0;
>  	dev->length = info->length;
>  
> -	dev->irq[0] = (lchip->irq_base == NO_IRQ) ?
> -			NO_IRQ : lchip->irq_base + info->irq[0];
> +	dev->irq[0] = lchip->irq_base + info->irq[0];
>  
>  	ret = device_register(&dev->dev);
>  	if (ret) {
> @@ -376,6 +375,9 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
>  	unsigned long r;
>  	int i, ret = -ENODEV;
>  
> +	if (!pdata->irq_base)
> +		return ret;
> +
>  	lchip = kzalloc(sizeof(struct locomo), GFP_KERNEL);
>  	if (!lchip)
>  		return -ENOMEM;
> @@ -387,7 +389,7 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
>  
>  	lchip->phys = mem->start;
>  	lchip->irq = irq;
> -	lchip->irq_base = (pdata) ? pdata->irq_base : NO_IRQ;
> +	lchip->irq_base = pdata->irq_base;

This removes a NULL pointer check.  Before this change, a NULL pdata
would be accepted and would lead to the interrupts not being setup.
After this change, it results in a NULL pointer deference.

Thankfully, both collie and poodle supply platform data, and are the
only providers of the locomo device.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/4] ARM: common/locomo: remove NO_IRQ check

[Arnd Bergmann]

On Tuesday, September 6, 2016 3:21:44 PM CEST Russell King - ARM Linux wrote:
> On Tue, Sep 06, 2016 at 03:53:28PM +0200, Arnd Bergmann wrote:
> > Since commit 489447380a29 ("[PATCH] handle errors returned by
> > platform_get_irq*()") ten years ago, the locomo driver refuses to
> > work without an interrupt line passed in its resources, so the
> > check for NO_IRQ is unnecessary.
> 
> This description is inaccurate and misleading (it looks like it was
> cut'n'pasted from patch 1.)
> 
> platform_get_irq() has nothing to do with your change, as your change
> is more about the irq_base value passed through platform data, and
> not through IRQ resources.

It was copied, but this part refers to this hunk

        irq = platform_get_irq(dev, 0);
        if (irq < 0)
                return -ENXIO;

from locomo_probe that was changed in the same patch as
the on in sa1111.c

> > We still check the irq_base argument for NO_IRQ, but as both

where the irq_base comes in.

I'll try to reword this to make it clearer.

> > @@ -387,7 +389,7 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
> >  
> >  	lchip->phys = mem->start;
> >  	lchip->irq = irq;
> > -	lchip->irq_base = (pdata) ? pdata->irq_base : NO_IRQ;
> > +	lchip->irq_base = pdata->irq_base;
> 
> This removes a NULL pointer check.  Before this change, a NULL pdata
> would be accepted and would lead to the interrupts not being setup.
> After this change, it results in a NULL pointer deference.
> 
> Thankfully, both collie and poodle supply platform data, and are the
> only providers of the locomo device.

Right, that is what I tried to say above. With the check I've added
in __locomo_probe, it would actually get the NULL pointer dereference
earlier than this line. I'll add back that check earlier in the function
and return an error in that case.

	Arnd

[PATCH 2/4] ARM: common/locomo: remove NO_IRQ check

[Arnd Bergmann]

On Tuesday, September 6, 2016 3:21:44 PM CEST Russell King - ARM Linux wrote:
> On Tue, Sep 06, 2016 at 03:53:28PM +0200, Arnd Bergmann wrote:
> > Since commit 489447380a29 ("[PATCH] handle errors returned by
> > platform_get_irq*()") ten years ago, the locomo driver refuses to
> > work without an interrupt line passed in its resources, so the
> > check for NO_IRQ is unnecessary.
> 
> This description is inaccurate and misleading (it looks like it was
> cut'n'pasted from patch 1.)
> 
> platform_get_irq() has nothing to do with your change, as your change
> is more about the irq_base value passed through platform data, and
> not through IRQ resources.

It was copied, but this part refers to this hunk

        irq = platform_get_irq(dev, 0);
        if (irq < 0)
                return -ENXIO;

from locomo_probe that was changed in the same patch as
the on in sa1111.c

> > We still check the irq_base argument for NO_IRQ, but as both

where the irq_base comes in.

I'll try to reword this to make it clearer.

> > @@ -387,7 +389,7 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
> >  
> >  	lchip->phys = mem->start;
> >  	lchip->irq = irq;
> > -	lchip->irq_base = (pdata) ? pdata->irq_base : NO_IRQ;
> > +	lchip->irq_base = pdata->irq_base;
> 
> This removes a NULL pointer check.  Before this change, a NULL pdata
> would be accepted and would lead to the interrupts not being setup.
> After this change, it results in a NULL pointer deference.
> 
> Thankfully, both collie and poodle supply platform data, and are the
> only providers of the locomo device.

Right, that is what I tried to say above. With the check I've added
in __locomo_probe, it would actually get the NULL pointer dereference
earlier than this line. I'll add back that check earlier in the function
and return an error in that case.

	Arnd

[PATCH v2] ARM: common/locomo: remove NO_IRQ check

[Arnd Bergmann]

The locomo driver uses two irq numbers, its own IRQ as passed
in the platform resources, and the base number for the interupts
of its child devices, passed in platform_data.

Since commit 489447380a29 ("[PATCH] handle errors returned by
platform_get_irq*()") ten years ago, the locomo driver refuses to
work without an interrupt line passed in its resources, so the
check comparing lchip->irq to NO_IRQ is unnecessary.

We still check the irq_base provided in the platform_data for
NO_IRQ, but as both platforms that use locomo (poodle and collie)
provide an irq_base, this can be done more consistently
by just checking that both are valid in the probe function
and otherwise returning an error.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: add back NULL pointer check, clarify changelog

diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c
index 0e97b4b871f9..38ca2db0cf12 100644
--- a/arch/arm/common/locomo.c
+++ b/arch/arm/common/locomo.c
@@ -253,8 +253,7 @@ locomo_init_one_child(struct locomo *lchip, struct locomo_dev_info *info)
 		dev->mapbase = 0;
 	dev->length = info->length;
 
-	dev->irq[0] = (lchip->irq_base == NO_IRQ) ?
-			NO_IRQ : lchip->irq_base + info->irq[0];
+	dev->irq[0] = lchip->irq_base + info->irq[0];
 
 	ret = device_register(&dev->dev);
 	if (ret) {
@@ -376,6 +375,9 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
 	unsigned long r;
 	int i, ret = -ENODEV;
 
+	if (!pdata || !pdata->irq_base)
+		return ret;
+
 	lchip = kzalloc(sizeof(struct locomo), GFP_KERNEL);
 	if (!lchip)
 		return -ENOMEM;
@@ -387,7 +389,7 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
 
 	lchip->phys = mem->start;
 	lchip->irq = irq;
-	lchip->irq_base = (pdata) ? pdata->irq_base : NO_IRQ;
+	lchip->irq_base = pdata->irq_base;
 
 	/*
 	 * Map the whole region.  This also maps the
@@ -454,8 +456,7 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
 	 * The interrupt controller must be initialised before any
 	 * other device to ensure that the interrupts are available.
 	 */
-	if (lchip->irq != NO_IRQ && lchip->irq_base != NO_IRQ)
-		locomo_setup_irq(lchip);
+	locomo_setup_irq(lchip);
 
 	for (i = 0; i < ARRAY_SIZE(locomo_devices); i++)
 		locomo_init_one_child(lchip, &locomo_devices[i]);
@@ -476,9 +477,7 @@ static void __locomo_remove(struct locomo *lchip)
 {
 	device_for_each_child(lchip->dev, NULL, locomo_remove_child);
 
-	if (lchip->irq != NO_IRQ) {
-		irq_set_chained_handler_and_data(lchip->irq, NULL, NULL);
-	}
+	irq_set_chained_handler_and_data(lchip->irq, NULL, NULL);
 
 	iounmap(lchip->base);
 	kfree(lchip);

[PATCH v2] ARM: common/locomo: remove NO_IRQ check

[Arnd Bergmann]

The locomo driver uses two irq numbers, its own IRQ as passed
in the platform resources, and the base number for the interupts
of its child devices, passed in platform_data.

Since commit 489447380a29 ("[PATCH] handle errors returned by
platform_get_irq*()") ten years ago, the locomo driver refuses to
work without an interrupt line passed in its resources, so the
check comparing lchip->irq to NO_IRQ is unnecessary.

We still check the irq_base provided in the platform_data for
NO_IRQ, but as both platforms that use locomo (poodle and collie)
provide an irq_base, this can be done more consistently
by just checking that both are valid in the probe function
and otherwise returning an error.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: add back NULL pointer check, clarify changelog

diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c
index 0e97b4b871f9..38ca2db0cf12 100644
--- a/arch/arm/common/locomo.c
+++ b/arch/arm/common/locomo.c
@@ -253,8 +253,7 @@ locomo_init_one_child(struct locomo *lchip, struct locomo_dev_info *info)
 		dev->mapbase = 0;
 	dev->length = info->length;
 
-	dev->irq[0] = (lchip->irq_base == NO_IRQ) ?
-			NO_IRQ : lchip->irq_base + info->irq[0];
+	dev->irq[0] = lchip->irq_base + info->irq[0];
 
 	ret = device_register(&dev->dev);
 	if (ret) {
@@ -376,6 +375,9 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
 	unsigned long r;
 	int i, ret = -ENODEV;
 
+	if (!pdata || !pdata->irq_base)
+		return ret;
+
 	lchip = kzalloc(sizeof(struct locomo), GFP_KERNEL);
 	if (!lchip)
 		return -ENOMEM;
@@ -387,7 +389,7 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
 
 	lchip->phys = mem->start;
 	lchip->irq = irq;
-	lchip->irq_base = (pdata) ? pdata->irq_base : NO_IRQ;
+	lchip->irq_base = pdata->irq_base;
 
 	/*
 	 * Map the whole region.  This also maps the
@@ -454,8 +456,7 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
 	 * The interrupt controller must be initialised before any
 	 * other device to ensure that the interrupts are available.
 	 */
-	if (lchip->irq != NO_IRQ && lchip->irq_base != NO_IRQ)
-		locomo_setup_irq(lchip);
+	locomo_setup_irq(lchip);
 
 	for (i = 0; i < ARRAY_SIZE(locomo_devices); i++)
 		locomo_init_one_child(lchip, &locomo_devices[i]);
@@ -476,9 +477,7 @@ static void __locomo_remove(struct locomo *lchip)
 {
 	device_for_each_child(lchip->dev, NULL, locomo_remove_child);
 
-	if (lchip->irq != NO_IRQ) {
-		irq_set_chained_handler_and_data(lchip->irq, NULL, NULL);
-	}
+	irq_set_chained_handler_and_data(lchip->irq, NULL, NULL);
 
 	iounmap(lchip->base);
 	kfree(lchip);

Re: [PATCH 3/4] mfd: ucb1x00: remove NO_IRQ check

[Lee Jones]

On Tue, 06 Sep 2016, Arnd Bergmann wrote:

> probe_irq_off() returns '0' on failure, not NO_IRQ, so the check
> in this driver is clearly wrong. This replaces it with the
> regular '!irq' check used in other drivers.
> 
> The sa1100 platform that this driver is used on originally numbered
> all its interrupts starting at '0', which would have conflicted with
> this change, but as of commit 18f3aec ("ARM: 8230/1: sa1100: shift
> IRQs by one"), this is not a problem any more.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/mfd/ucb1x00-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

I'm going to leave this in MFD 'as is' at them moment.  This will
break bisectability of course, but it's better we get this in -next
now in order to match it up with the other half of the fix.

The plan is probably to squash the two patches together, keeping
Russell's authorship (first come, first served) and give creds to Arnd
for this part of the patch.

Unless anyone has any complaints, I also plan to keep both of your
SoBs.

> diff --git a/drivers/mfd/ucb1x00-core.c b/drivers/mfd/ucb1x00-core.c
> index 48bea5038654..d6fb2e1a759a 100644
> --- a/drivers/mfd/ucb1x00-core.c
> +++ b/drivers/mfd/ucb1x00-core.c
> @@ -537,7 +537,7 @@ static int ucb1x00_probe(struct mcp *mcp)
>  	ucb1x00_enable(ucb);
>  	ucb->irq = ucb1x00_detect_irq(ucb);
>  	ucb1x00_disable(ucb);
> -	if (ucb->irq == NO_IRQ) {
> +	if (!ucb->irq) {
>  		dev_err(&ucb->dev, "IRQ probe failed\n");
>  		ret = -ENODEV;
>  		goto err_no_irq;

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog