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 <[email protected]>
---
  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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to