lkml.org 
[lkml]   [2021]   [Dec]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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

\
 
 \ /
  Last update: 2021-12-20 11:18    [W:0.253 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site