Hi Prabhakar,
> + if (!np) {
Very minor nit: Maybe 'if (np)' and switch the blocks? Positive logic is
a tad easier to read.
> + struct resource *res;
> + resource_size_t n;
> +
> + while ((res = platform_get_resource(dev, IORESOURCE_IRQ, k))) {
> + for (n = res->start; n <= res->end; n++) {
> + ret = devm_request_irq(&dev->dev, n, sh_mobile_i2c_isr,
> + 0, dev_name(&dev->dev), pd);
> + if (ret) {
> + dev_err(&dev->dev, "cannot request IRQ %pa\n", &n);
> + return ret;
> + }
> + }
> + k++;
> + }
Yeah, it is good to keep the legacy block as is.
> + do {
> + irq = platform_get_irq_optional(dev, k);
> + if (irq <= 0 && irq != -ENXIO)
> + return irq ? irq : -ENXIO;
> + if (irq == -ENXIO)
> + break;
> + ret = devm_request_irq(&dev->dev, irq, sh_mobile_i2c_isr,
> + 0, dev_name(&dev->dev), pd);
> if (ret) {
> - dev_err(&dev->dev, "cannot request IRQ %pa\n", &n);
> + dev_err(&dev->dev, "cannot request IRQ %d\n", irq);
> return ret;
> }
> - }
> - k++;
> + k++;
> + } while (irq);
In addition to the 'irq == 0' case from patch 1, I tried to shorten the
block for the np-case. I only came up with this. The assigntment and
comparison of the while-argument is not exactly pretty, but the block
itself is easier to read. I'll let you decide.
while (irq = platform_get_irq_optional(dev, k) != -ENXIO) {
if (irq < 0)
return irq;
ret = devm_request_irq(&dev->dev, irq, sh_mobile_i2c_isr,
0, dev_name(&dev->dev), pd);
if (ret) {
dev_err(&dev->dev, "cannot request IRQ %d\n", irq);
return ret;
}
k++;
}
Only brainstorming, not even build tested.
All the best,
Wolfram
Attachment:
signature.asc
Description: PGP signature
