Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Date: 2021-12-20 13:01:04
Also in:
linux-arm-kernel, linux-i2c, linux-renesas-soc, lkml
Hi Geert, On Mon, Dec 20, 2021 at 12:54 PM Geert Uytterhoeven [off-list ref] wrote:
Hi Prabhakar, On Mon, Dec 20, 2021 at 12:56 PM Lad, Prabhakar [off-list ref] wrote:quoted
On Mon, Dec 20, 2021 at 10:18 AM Geert Uytterhoeven [off-list ref] wrote:quoted
On Sat, Dec 18, 2021 at 5:59 PM Lad Prabhakar [off-list ref] wrote:quoted
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!quoted
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.For the non DT users the IRQ resource is passed as a range [0] and not a single interrupt so I went with this approach. Is there a way I'm missing where we could still use platform_get_irq_xyz() variants for such cases?Oh, I didn't realize it used a single resource with a range. Is this common, i.e. would it make sense to add support for this to platform_get_irq_optional()?
No this isn't common even non dt users should ideally be passing a single IRQ resource. There are very few such platforms which do this so I don't see any point in adding this support to platform_get_irq_optional() unless the IRQ maintainers think otherwise.
quoted
quoted
quoted
--- a/drivers/i2c/busses/i2c-sh_mobile.c +++ b/drivers/i2c/busses/i2c-sh_mobile.cquoted
quoted
quoted
+ 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).I'll keep that in mind if the Ethernet driver falls in the convection patch changes.The Ethernet driver was converted 6 years ago, cfr. commit 965b2aa78fbcb831 ("net/smsc911x: fix irq resource allocation failure").
Thanks for the pointer. Cheers, Prabhakar
quoted
[0] https://elixir.bootlin.com/linux/v5.16-rc6/source/arch/sh/kernel/cpu/sh4a/setup-sh7724.c#L454Gr{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