* [PATCH 1/4] ARM: common/sa1111: remove NO_IRQ check
@ 2016-09-06 13:53 Arnd Bergmann
2016-09-06 13:53 ` [PATCH 2/4] ARM: common/locomo: " Arnd Bergmann
2016-09-06 14:14 ` [PATCH 1/4] ARM: common/sa1111: " Russell King - ARM Linux
0 siblings, 2 replies; 6+ messages in thread
From: Arnd Bergmann @ 2016-09-06 13:53 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/4] ARM: common/locomo: remove NO_IRQ check
2016-09-06 13:53 [PATCH 1/4] ARM: common/sa1111: remove NO_IRQ check Arnd Bergmann
@ 2016-09-06 13:53 ` Arnd Bergmann
2016-09-06 14:21 ` Russell King - ARM Linux
2016-09-06 14:14 ` [PATCH 1/4] ARM: common/sa1111: " Russell King - ARM Linux
1 sibling, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2016-09-06 13:53 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/4] ARM: common/sa1111: remove NO_IRQ check
2016-09-06 13:53 [PATCH 1/4] ARM: common/sa1111: remove NO_IRQ check Arnd Bergmann
2016-09-06 13:53 ` [PATCH 2/4] ARM: common/locomo: " Arnd Bergmann
@ 2016-09-06 14:14 ` Russell King - ARM Linux
1 sibling, 0 replies; 6+ messages in thread
From: Russell King - ARM Linux @ 2016-09-06 14:14 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/4] ARM: common/locomo: remove NO_IRQ check
2016-09-06 13:53 ` [PATCH 2/4] ARM: common/locomo: " Arnd Bergmann
@ 2016-09-06 14:21 ` Russell King - ARM Linux
2016-09-06 14:50 ` Arnd Bergmann
0 siblings, 1 reply; 6+ messages in thread
From: Russell King - ARM Linux @ 2016-09-06 14:21 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/4] ARM: common/locomo: remove NO_IRQ check
2016-09-06 14:21 ` Russell King - ARM Linux
@ 2016-09-06 14:50 ` Arnd Bergmann
2016-09-06 15:20 ` [PATCH v2] " Arnd Bergmann
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2016-09-06 14:50 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] ARM: common/locomo: remove NO_IRQ check
2016-09-06 14:50 ` Arnd Bergmann
@ 2016-09-06 15:20 ` Arnd Bergmann
0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2016-09-06 15:20 UTC (permalink / raw)
To: linux-arm-kernel
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);
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-06 15:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-06 13:53 [PATCH 1/4] ARM: common/sa1111: remove NO_IRQ check Arnd Bergmann
2016-09-06 13:53 ` [PATCH 2/4] ARM: common/locomo: " Arnd Bergmann
2016-09-06 14:21 ` Russell King - ARM Linux
2016-09-06 14:50 ` Arnd Bergmann
2016-09-06 15:20 ` [PATCH v2] " Arnd Bergmann
2016-09-06 14:14 ` [PATCH 1/4] ARM: common/sa1111: " Russell King - ARM Linux
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).