Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Mon, 20 Dec 2021 11:17:53 +0100 | Subject | Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt |
| |
Hi Prabhakar,
On Sat, Dec 18, 2021 at 5:59 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static > allocation of IRQ resources in DT core code, this causes an issue > when using hierarchical interrupt domains using "interrupts" property > in the node as this bypasses the hierarchical setup and messes up the > irq chaining.
Thanks for your patch!
> In preparation for removal of static setup of IRQ resource from DT core > code use platform_get_irq_optional() for DT users only.
Why only for DT users? Plenty of driver code shared by Renesas ARM (DT-based) on SuperH (non-DT) SoCs already uses platform_get_irq_optional(), so I expect that to work for both.
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> --- a/drivers/i2c/busses/i2c-sh_mobile.c > +++ b/drivers/i2c/busses/i2c-sh_mobile.c > @@ -830,20 +830,41 @@ static void sh_mobile_i2c_release_dma(struct sh_mobile_i2c_data *pd) > > static int sh_mobile_i2c_hook_irqs(struct platform_device *dev, struct sh_mobile_i2c_data *pd) > { > - struct resource *res; > - resource_size_t n; > + struct device_node *np = dev_of_node(&dev->dev); > int k = 0, ret; > > - 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 (!np) { > + 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++; > + } > + } else { > + int irq; > + > + do { > + irq = platform_get_irq_optional(dev, k);
Check for irq == -ENXIO first, to simplify the checks below?
> + if (irq <= 0 && irq != -ENXIO) > + return irq ? irq : -ENXIO;
Can irq == 0 really happen?
All SuperH users of the "i2c-sh_mobile" platform device use an evt2irq() value that is non-zero.
I might have missed something, but it seems the only user of IRQ 0 on SuperH is smsc911x Ethernet in arch/sh/boards/board-apsh4a3a.c and arch/sh/boards/board-apsh4ad0a.c, which use evt2irq(0x200). These should have been seeing the "0 is an invalid IRQ number" warning splat since it was introduced in commit a85a6c86c25be2d2 ("driver core: platform: Clarify that IRQ 0 is invalid"). Or not: the rare users may not have upgraded their kernels beyond v5.8 yet...
> + 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); > } > > return k > 0 ? 0 : -ENOENT;
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
| |