Messages in this thread | | | Date | Mon, 29 Jun 2015 10:43:00 +0200 | From | Daniel Lezcano <> | Subject | Re: [PATCH 1/1] clocksource: sh_mtu2: Fix irq leaks when sh_mtu2_setup() fails |
| |
On 06/14/2015 12:57 PM, Takeshi Yoshimura wrote: > My static checker detected irq leaks in sh_mtu2_setup(). The problem > happens when the first request_irq() succeeds but the later ones fail. > This situation also results that the clockevent for the first channel > is registered although the driver fails. So I introduce > sh_mtu2_setup_channels() to ensure the consistency of all the clockevent > and irqs even if the error occurs. > > Signed-off-by: Takeshi Yoshimura <yos@sslab.ics.keio.ac.jp> > --- > drivers/clocksource/sh_mtu2.c | 65 ++++++++++++++++++++++++++----------------- > 1 file changed, 39 insertions(+), 26 deletions(-) > > diff --git a/drivers/clocksource/sh_mtu2.c b/drivers/clocksource/sh_mtu2.c > index 3d88698..5ddc433 100644 > --- a/drivers/clocksource/sh_mtu2.c > +++ b/drivers/clocksource/sh_mtu2.c > @@ -344,38 +344,54 @@ static int sh_mtu2_register(struct sh_mtu2_channel *ch, const char *name) > return 0; > } > > -static int sh_mtu2_setup_channel(struct sh_mtu2_channel *ch, unsigned int index, > - struct sh_mtu2_device *mtu) > +static int sh_mtu2_setup_channels(struct sh_mtu2_device *mtu) > { > static const unsigned int channel_offsets[] = { > 0x300, 0x380, 0x000, > }; > char name[6]; > - int irq; > + int irq[3];
int irq[mtu->channels] ?
> int ret; > + int i; > > - ch->mtu = mtu; > - > - sprintf(name, "tgi%ua", index); > - irq = platform_get_irq_byname(mtu->pdev, name); > - if (irq < 0) { > - /* Skip channels with no declared interrupt. */ > - return 0; > + for (i = 0; i < mtu->num_channels; ++i) {
For the sake of consistency, perhaps take the opportunity to replace '++i' by 'i++'.
> + struct sh_mtu2_channel *ch = &mtu->channels[i]; > + > + ch->mtu = mtu; > + sprintf(name, "tgi%ua", i); > + irq[i] = platform_get_irq_byname(mtu->pdev, name); > + if (irq[i] < 0) { > + /* Skip channels with no declared interrupt. */ > + continue; > + } > + > + ret = request_irq(irq[i], sh_mtu2_interrupt, > + IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING, > + dev_name(&ch->mtu->pdev->dev), ch); > + if (ret) { > + dev_err(&ch->mtu->pdev->dev, "ch%u: failed to request irq %d\n", > + i, irq[i]);i
As 'i' is a type of 'int', 'ch%u' must be 'ch%d'
> + goto err; > + } > } > > - ret = request_irq(irq, sh_mtu2_interrupt, > - IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING, > - dev_name(&ch->mtu->pdev->dev), ch); > - if (ret) { > - dev_err(&ch->mtu->pdev->dev, "ch%u: failed to request irq %d\n", > - index, irq); > - return ret; > + for (i = 0; i < mtu->num_channels; ++i) { > + struct sh_mtu2_channel *ch = &mtu->channels[i]; > + > + ch->base = mtu->mapbase + channel_offsets[i]; > + ch->index = i;
'ch->index' is 'unsigned int' but 'i' is 'int'
> + sh_mtu2_register(ch, dev_name(&mtu->pdev->dev));
sh_mtu2_register return value was checked before this patch.
> } > > - ch->base = mtu->mapbase + channel_offsets[index]; > - ch->index = index; > + return 0; > > - return sh_mtu2_register(ch, dev_name(&mtu->pdev->dev)); > +err: > + --i; > + for (; i >= 0; --i) {
for (i--; i >= 0; i--)
> + if (irq[i] >= 0) > + free_irq(irq[i], &mtu->channels[i]); > + } > + return ret; > }
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
| |