From: Russell King - ARM Linux <linux@armlinux.org.uk> To: Arnd Bergmann <arnd@arndb.de> Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/4] ARM: common/locomo: remove NO_IRQ check Date: Tue, 6 Sep 2016 15:21:44 +0100 [thread overview] Message-ID: <20160906142144.GX1041@n2100.armlinux.org.uk> (raw) In-Reply-To: <20160906135637.2622666-2-arnd@arndb.de> 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.
WARNING: multiple messages have this Message-ID (diff)
From: linux@armlinux.org.uk (Russell King - ARM Linux) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 2/4] ARM: common/locomo: remove NO_IRQ check Date: Tue, 6 Sep 2016 15:21:44 +0100 [thread overview] Message-ID: <20160906142144.GX1041@n2100.armlinux.org.uk> (raw) In-Reply-To: <20160906135637.2622666-2-arnd@arndb.de> 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.
next prev parent reply other threads:[~2016-09-06 14:21 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 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 13:53 ` [PATCH 2/4] ARM: common/locomo: " Arnd Bergmann 2016-09-06 13:53 ` Arnd Bergmann 2016-09-06 14:21 ` Russell King - ARM Linux [this message] 2016-09-06 14:21 ` Russell King - ARM Linux 2016-09-06 14:50 ` Arnd Bergmann 2016-09-06 14:50 ` Arnd Bergmann 2016-09-06 15:20 ` [PATCH v2] " Arnd Bergmann 2016-09-06 15:20 ` Arnd Bergmann 2016-09-06 13:53 ` [PATCH 3/4] mfd: ucb1x00: " Arnd Bergmann 2016-09-07 11:24 ` Lee Jones 2016-09-06 13:53 ` [PATCH 4/4] pcmcia: soc-common: remove incorrect NO_IRQ use Arnd Bergmann 2016-09-06 14:14 ` [PATCH 1/4] ARM: common/sa1111: remove NO_IRQ check Russell King - ARM Linux 2016-09-06 14:14 ` Russell King - ARM Linux
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=20160906142144.GX1041@n2100.armlinux.org.uk \ --to=linux@armlinux.org.uk \ --cc=arnd@arndb.de \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.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: linkBe 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.