Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 20.12.2021 13:17, Geert Uytterhoeven 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@xxxxxxxxxxxxxx>

--- 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?

   Doesn't matter much in this case -- devm_request_irq() happily takes IRQ0. :-)

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:

   Warning or no warning, 0 is still returned. :-/
My attempt to put an end to this has stuck waiting a review from the IRQ people...

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@xxxxxxxxxxxxxx

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




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux