From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Takeshi Yoshimura <yos@sslab.ics.keio.ac.jp>,
Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] clocksource: sh_mtu2: Fix irq leaks when sh_mtu2_setup() fails
Date: Mon, 29 Jun 2015 10:43:00 +0200 [thread overview]
Message-ID: <55910514.4000809@linaro.org> (raw)
In-Reply-To: <1434279427-6775-1-git-send-email-yos@sslab.ics.keio.ac.jp>
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
prev parent reply other threads:[~2015-06-29 8:43 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-14 10:57 [PATCH 1/1] clocksource: sh_mtu2: Fix irq leaks when sh_mtu2_setup() fails Takeshi Yoshimura
2015-06-29 8:43 ` Daniel Lezcano [this message]
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=55910514.4000809@linaro.org \
--to=daniel.lezcano@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=yos@sslab.ics.keio.ac.jp \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).