On Sun, Jan 12, 2014 at 11:27:39AM +0100, Geert Uytterhoeven wrote:

> +     for (i = 0; i < MAX_NUM_IRQ; i++) {
> +             irq = platform_get_irq(pdev, i);
> +             if (irq < 0) {
> +                     if (rspi->num_irqs)
> +                             break;
> +                     dev_err(&pdev->dev, "platform_get_irq error\n");
> +                     ret = -ENODEV;
> +                     goto error1;
> +             }
> +             ret = devm_request_irq(&pdev->dev, irq, rspi_irq, 0,
> +                                    dev_name(&pdev->dev), rspi);
> +             if (ret < 0) {
> +                     dev_err(&pdev->dev, "request_irq %d error\n", irq);
> +                     goto error1;
> +             }
> +             rspi->irqs[i] = irq;
> +             rspi->num_irqs++;
>       }

This should really have some way of identifying which interrupt is which
either through defines in the header or by using _get_irq_byname() (the
latter is a bit neater) - even if the driver can get away with treating
all interrupts interchangably now it would seem very surprising if they
were totally interchangible so the driver might get support for treating
them separately in the future (or some new IP revision might add another
interrupt type for that matter).

Some blank lines (for example between get and request) would be good
too.

Attachment: signature.asc
Description: Digital signature

Reply via email to