* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2021-12-20 10:17 ` Geert Uytterhoeven
0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2021-12-20 10:17 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Rob Herring, Nicolas Saenz Julienne, Florian Fainelli, Ray Jui,
Scott Branden, bcm-kernel-feedback-list, Chris Brandt,
Wolfram Sang, Linux I2C, linux-rpi-kernel, Linux ARM,
Linux Kernel Mailing List, Linux-Renesas, Prabhakar,
Linux-sh list
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
2021-12-20 10:17 ` Geert Uytterhoeven
@ 2021-12-20 11:53 ` Sergei Shtylyov
-1 siblings, 0 replies; 54+ messages in thread
From: Sergei Shtylyov @ 2021-12-20 11:53 UTC (permalink / raw)
To: Geert Uytterhoeven, Lad Prabhakar
Cc: Rob Herring, Nicolas Saenz Julienne, Florian Fainelli, Ray Jui,
Scott Branden, bcm-kernel-feedback-list, Chris Brandt,
Wolfram Sang, Linux I2C, linux-rpi-kernel, Linux ARM,
Linux Kernel Mailing List, Linux-Renesas, Prabhakar,
Linux-sh list
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@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?
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@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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2021-12-20 11:53 ` Sergei Shtylyov
0 siblings, 0 replies; 54+ messages in thread
From: Sergei Shtylyov @ 2021-12-20 11:53 UTC (permalink / raw)
To: Geert Uytterhoeven, Lad Prabhakar
Cc: Rob Herring, Nicolas Saenz Julienne, Florian Fainelli, Ray Jui,
Scott Branden, bcm-kernel-feedback-list, Chris Brandt,
Wolfram Sang, Linux I2C, linux-rpi-kernel, Linux ARM,
Linux Kernel Mailing List, Linux-Renesas, Prabhakar,
Linux-sh list
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@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?
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@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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
2021-12-20 11:53 ` Sergei Shtylyov
@ 2022-02-08 12:31 ` Arnd Bergmann
-1 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2022-02-08 12:31 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Geert Uytterhoeven, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On Mon, Dec 20, 2021 at 12:53 PM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
> On 20.12.2021 13:17, Geert Uytterhoeven wrote:
>
> > 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...
I had another look at this after you asked about it on IRC. I don't
know much SH assembly, but I suspect IRQ 0 has not been delivered
since 2009 after 1e1030dccb10 ("sh: nmi_debug support."). On a
related note, CONFIG_INTC_BALANCING was broken in 2be6bb0c79c7
("sh: intc: Split up the INTC code.") by inadvertently removing the Kconfig
symbol.
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2022-02-08 12:31 ` Arnd Bergmann
0 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2022-02-08 12:31 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Geert Uytterhoeven, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On Mon, Dec 20, 2021 at 12:53 PM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
> On 20.12.2021 13:17, Geert Uytterhoeven wrote:
>
> > 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...
I had another look at this after you asked about it on IRC. I don't
know much SH assembly, but I suspect IRQ 0 has not been delivered
since 2009 after 1e1030dccb10 ("sh: nmi_debug support."). On a
related note, CONFIG_INTC_BALANCING was broken in 2be6bb0c79c7
("sh: intc: Split up the INTC code.") by inadvertently removing the Kconfig
symbol.
Arnd
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
2022-02-08 12:31 ` Arnd Bergmann
@ 2022-02-09 15:11 ` Sergei Shtylyov
-1 siblings, 0 replies; 54+ messages in thread
From: Sergei Shtylyov @ 2022-02-09 15:11 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Geert Uytterhoeven, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On 2/8/22 3:31 PM, Arnd Bergmann wrote:
[...]
>>> 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...
>
> I had another look at this after you asked about it on IRC. I don't
> know much SH assembly, but I suspect IRQ 0 has not been delivered
> since 2009 after 1e1030dccb10 ("sh: nmi_debug support."). On a
Mhm... this commit changes the SH3 code while SH778x are SH4A, no?
[...]
>
> Arnd
MBR, Sergey
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2022-02-09 15:11 ` Sergei Shtylyov
0 siblings, 0 replies; 54+ messages in thread
From: Sergei Shtylyov @ 2022-02-09 15:11 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Geert Uytterhoeven, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On 2/8/22 3:31 PM, Arnd Bergmann wrote:
[...]
>>> 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...
>
> I had another look at this after you asked about it on IRC. I don't
> know much SH assembly, but I suspect IRQ 0 has not been delivered
> since 2009 after 1e1030dccb10 ("sh: nmi_debug support."). On a
Mhm... this commit changes the SH3 code while SH778x are SH4A, no?
[...]
>
> Arnd
MBR, Sergey
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
2022-02-09 15:11 ` Sergei Shtylyov
@ 2022-02-09 15:18 ` Arnd Bergmann
-1 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2022-02-09 15:18 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Arnd Bergmann, Geert Uytterhoeven, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On Wed, Feb 9, 2022 at 4:11 PM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
>
> On 2/8/22 3:31 PM, Arnd Bergmann wrote:
>
> [...]
> >>> 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...
> >
> > I had another look at this after you asked about it on IRC. I don't
> > know much SH assembly, but I suspect IRQ 0 has not been delivered
> > since 2009 after 1e1030dccb10 ("sh: nmi_debug support."). On a
>
> Mhm... this commit changes the SH3 code while SH778x are SH4A, no?
This code is shared between both:
arch/sh/kernel/cpu/sh4/Makefile:common-y += $(addprefix
../sh3/, entry.o ex.o)
Arnd
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2022-02-09 15:18 ` Arnd Bergmann
0 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2022-02-09 15:18 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Arnd Bergmann, Geert Uytterhoeven, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On Wed, Feb 9, 2022 at 4:11 PM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
>
> On 2/8/22 3:31 PM, Arnd Bergmann wrote:
>
> [...]
> >>> 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...
> >
> > I had another look at this after you asked about it on IRC. I don't
> > know much SH assembly, but I suspect IRQ 0 has not been delivered
> > since 2009 after 1e1030dccb10 ("sh: nmi_debug support."). On a
>
> Mhm... this commit changes the SH3 code while SH778x are SH4A, no?
This code is shared between both:
arch/sh/kernel/cpu/sh4/Makefile:common-y += $(addprefix
../sh3/, entry.o ex.o)
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
2022-02-09 15:18 ` Arnd Bergmann
@ 2022-02-09 15:48 ` Sergei Shtylyov
-1 siblings, 0 replies; 54+ messages in thread
From: Sergei Shtylyov @ 2022-02-09 15:48 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Geert Uytterhoeven, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On 2/9/22 6:18 PM, Arnd Bergmann wrote:
> On Wed, Feb 9, 2022 at 4:11 PM Sergei Shtylyov
> <sergei.shtylyov@gmail.com> wrote:
>>
>> On 2/8/22 3:31 PM, Arnd Bergmann wrote:
>>
>> [...]
>>>>> 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...
>>>
>>> I had another look at this after you asked about it on IRC. I don't
>>> know much SH assembly, but I suspect IRQ 0 has not been delivered
Neither do I, sigh...
I do know the instuctions are 16-bit and so there are no immediate
opperands... :-)
>>> since 2009 after 1e1030dccb10 ("sh: nmi_debug support."). On a
>>
>> Mhm... this commit changes the SH3 code while SH778x are SH4A, no?
>
> This code is shared between both:
>
> arch/sh/kernel/cpu/sh4/Makefile:common-y += $(addprefix
> ../sh3/, entry.o ex.o)
Ah, quite convoluted! :-)
So you mean thet broke the delivery of EVT 0x200 when mucking with NMI?
> Arnd
MBR, Sergey
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2022-02-09 15:48 ` Sergei Shtylyov
0 siblings, 0 replies; 54+ messages in thread
From: Sergei Shtylyov @ 2022-02-09 15:48 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Geert Uytterhoeven, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On 2/9/22 6:18 PM, Arnd Bergmann wrote:
> On Wed, Feb 9, 2022 at 4:11 PM Sergei Shtylyov
> <sergei.shtylyov@gmail.com> wrote:
>>
>> On 2/8/22 3:31 PM, Arnd Bergmann wrote:
>>
>> [...]
>>>>> 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...
>>>
>>> I had another look at this after you asked about it on IRC. I don't
>>> know much SH assembly, but I suspect IRQ 0 has not been delivered
Neither do I, sigh...
I do know the instuctions are 16-bit and so there are no immediate
opperands... :-)
>>> since 2009 after 1e1030dccb10 ("sh: nmi_debug support."). On a
>>
>> Mhm... this commit changes the SH3 code while SH778x are SH4A, no?
>
> This code is shared between both:
>
> arch/sh/kernel/cpu/sh4/Makefile:common-y += $(addprefix
> ../sh3/, entry.o ex.o)
Ah, quite convoluted! :-)
So you mean thet broke the delivery of EVT 0x200 when mucking with NMI?
> Arnd
MBR, Sergey
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
2022-02-09 15:48 ` Sergei Shtylyov
@ 2022-02-09 16:02 ` Arnd Bergmann
-1 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2022-02-09 16:02 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Arnd Bergmann, Geert Uytterhoeven, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On Wed, Feb 9, 2022 at 4:48 PM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
> On 2/9/22 6:18 PM, Arnd Bergmann wrote:
> >>> since 2009 after 1e1030dccb10 ("sh: nmi_debug support."). On a
> >>
> >> Mhm... this commit changes the SH3 code while SH778x are SH4A, no?
> >
> > This code is shared between both:
> >
> > arch/sh/kernel/cpu/sh4/Makefile:common-y += $(addprefix
> > ../sh3/, entry.o ex.o)
>
> Ah, quite convoluted! :-)
> So you mean thet broke the delivery of EVT 0x200 when mucking with NMI?
Yes, exactly: If I read this right, the added code:
+ shlr2 r4
+ shlr r4
+ mov r4, r0 ! save vector->jmp table offset for later
+
+ shlr2 r4 ! vector to IRQ# conversion
+ add #-0x10, r4
+
+ cmp/pz r4 ! is it a valid IRQ?
+ bt 10f
gets the vector (0x200 for this device), shifts it five bits to 0x10,
and subtracts 0x10,
then branches to do_IRQ if the interrupt number is non-zero, otherwise it goes
through the exception_handling_table.
Arnd
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2022-02-09 16:02 ` Arnd Bergmann
0 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2022-02-09 16:02 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Arnd Bergmann, Geert Uytterhoeven, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On Wed, Feb 9, 2022 at 4:48 PM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
> On 2/9/22 6:18 PM, Arnd Bergmann wrote:
> >>> since 2009 after 1e1030dccb10 ("sh: nmi_debug support."). On a
> >>
> >> Mhm... this commit changes the SH3 code while SH778x are SH4A, no?
> >
> > This code is shared between both:
> >
> > arch/sh/kernel/cpu/sh4/Makefile:common-y += $(addprefix
> > ../sh3/, entry.o ex.o)
>
> Ah, quite convoluted! :-)
> So you mean thet broke the delivery of EVT 0x200 when mucking with NMI?
Yes, exactly: If I read this right, the added code:
+ shlr2 r4
+ shlr r4
+ mov r4, r0 ! save vector->jmp table offset for later
+
+ shlr2 r4 ! vector to IRQ# conversion
+ add #-0x10, r4
+
+ cmp/pz r4 ! is it a valid IRQ?
+ bt 10f
gets the vector (0x200 for this device), shifts it five bits to 0x10,
and subtracts 0x10,
then branches to do_IRQ if the interrupt number is non-zero, otherwise it goes
through the exception_handling_table.
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
2022-02-09 16:02 ` Arnd Bergmann
@ 2022-02-09 16:08 ` Sergei Shtylyov
-1 siblings, 0 replies; 54+ messages in thread
From: Sergei Shtylyov @ 2022-02-09 16:08 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Geert Uytterhoeven, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On 2/9/22 7:02 PM, Arnd Bergmann wrote:
>>>>> since 2009 after 1e1030dccb10 ("sh: nmi_debug support."). On a
>>>>
>>>> Mhm... this commit changes the SH3 code while SH778x are SH4A, no?
>>>
>>> This code is shared between both:
>>>
>>> arch/sh/kernel/cpu/sh4/Makefile:common-y += $(addprefix
>>> ../sh3/, entry.o ex.o)
>>
>> Ah, quite convoluted! :-)
>> So you mean thet broke the delivery of EVT 0x200 when mucking with NMI?
>
> Yes, exactly: If I read this right, the added code:
>
> + shlr2 r4
> + shlr r4
> + mov r4, r0 ! save vector->jmp table offset for later
> +
> + shlr2 r4 ! vector to IRQ# conversion
> + add #-0x10, r4
> +
> + cmp/pz r4 ! is it a valid IRQ?
> + bt 10f
>
> gets the vector (0x200 for this device), shifts it five bits to 0x10,
> and subtracts 0x10,
> then branches to do_IRQ if the interrupt number is non-zero, otherwise it goes
> through the exception_handling_table.
The SH4 manual I found on my disk (have it from MontaVista times) tells me cmp/pz
sets T if Rn is >= 0, then bt branches if T = 1. So I do think the code is correct.
One more thing: the board code for those boards was added in 2011, we can assume
it was working back then, right? :-_
> Arnd
MBR, Sergey
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2022-02-09 16:08 ` Sergei Shtylyov
0 siblings, 0 replies; 54+ messages in thread
From: Sergei Shtylyov @ 2022-02-09 16:08 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Geert Uytterhoeven, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On 2/9/22 7:02 PM, Arnd Bergmann wrote:
>>>>> since 2009 after 1e1030dccb10 ("sh: nmi_debug support."). On a
>>>>
>>>> Mhm... this commit changes the SH3 code while SH778x are SH4A, no?
>>>
>>> This code is shared between both:
>>>
>>> arch/sh/kernel/cpu/sh4/Makefile:common-y += $(addprefix
>>> ../sh3/, entry.o ex.o)
>>
>> Ah, quite convoluted! :-)
>> So you mean thet broke the delivery of EVT 0x200 when mucking with NMI?
>
> Yes, exactly: If I read this right, the added code:
>
> + shlr2 r4
> + shlr r4
> + mov r4, r0 ! save vector->jmp table offset for later
> +
> + shlr2 r4 ! vector to IRQ# conversion
> + add #-0x10, r4
> +
> + cmp/pz r4 ! is it a valid IRQ?
> + bt 10f
>
> gets the vector (0x200 for this device), shifts it five bits to 0x10,
> and subtracts 0x10,
> then branches to do_IRQ if the interrupt number is non-zero, otherwise it goes
> through the exception_handling_table.
The SH4 manual I found on my disk (have it from MontaVista times) tells me cmp/pz
sets T if Rn is >= 0, then bt branches if T = 1. So I do think the code is correct.
One more thing: the board code for those boards was added in 2011, we can assume
it was working back then, right? :-_
> Arnd
MBR, Sergey
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
2022-02-09 16:08 ` Sergei Shtylyov
@ 2022-02-09 22:56 ` Arnd Bergmann
-1 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2022-02-09 22:56 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Arnd Bergmann, Geert Uytterhoeven, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On Wed, Feb 9, 2022 at 5:08 PM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
> On 2/9/22 7:02 PM, Arnd Bergmann wrote:
> >
> > + shlr2 r4
> > + shlr r4
> > + mov r4, r0 ! save vector->jmp table offset for later
> > +
> > + shlr2 r4 ! vector to IRQ# conversion
> > + add #-0x10, r4
> > +
> > + cmp/pz r4 ! is it a valid IRQ?
> > + bt 10f
> >
> > gets the vector (0x200 for this device), shifts it five bits to 0x10,
> > and subtracts 0x10,
> > then branches to do_IRQ if the interrupt number is non-zero, otherwise it goes
> > through the exception_handling_table.
>
> The SH4 manual I found on my disk (have it from MontaVista times) tells me cmp/pz
> sets T if Rn is >= 0, then bt branches if T = 1. So I do think the code is correct.
> One more thing: the board code for those boards was added in 2011, we can assume
> it was working back then, right? :-_
Indeed, this does make more sense, I had not realized that the numbers could get
negative here.
Arnd
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2022-02-09 22:56 ` Arnd Bergmann
0 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2022-02-09 22:56 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Arnd Bergmann, Geert Uytterhoeven, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On Wed, Feb 9, 2022 at 5:08 PM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
> On 2/9/22 7:02 PM, Arnd Bergmann wrote:
> >
> > + shlr2 r4
> > + shlr r4
> > + mov r4, r0 ! save vector->jmp table offset for later
> > +
> > + shlr2 r4 ! vector to IRQ# conversion
> > + add #-0x10, r4
> > +
> > + cmp/pz r4 ! is it a valid IRQ?
> > + bt 10f
> >
> > gets the vector (0x200 for this device), shifts it five bits to 0x10,
> > and subtracts 0x10,
> > then branches to do_IRQ if the interrupt number is non-zero, otherwise it goes
> > through the exception_handling_table.
>
> The SH4 manual I found on my disk (have it from MontaVista times) tells me cmp/pz
> sets T if Rn is >= 0, then bt branches if T = 1. So I do think the code is correct.
> One more thing: the board code for those boards was added in 2011, we can assume
> it was working back then, right? :-_
Indeed, this does make more sense, I had not realized that the numbers could get
negative here.
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
2022-02-09 16:08 ` Sergei Shtylyov
@ 2022-02-10 8:54 ` Geert Uytterhoeven
-1 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2022-02-10 8:54 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Arnd Bergmann, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
Hi Sergei,
On Wed, Feb 9, 2022 at 5:08 PM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
> One more thing: the board code for those boards was added in 2011, we can assume
> it was working back then, right? :-_
This assumption may not be true: there is plenty of driver/board
support that was only upstreamed partially.
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2022-02-10 8:54 ` Geert Uytterhoeven
0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2022-02-10 8:54 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Arnd Bergmann, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
Hi Sergei,
On Wed, Feb 9, 2022 at 5:08 PM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
> One more thing: the board code for those boards was added in 2011, we can assume
> it was working back then, right? :-_
This assumption may not be true: there is plenty of driver/board
support that was only upstreamed partially.
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
2022-02-09 15:48 ` Sergei Shtylyov
@ 2022-02-10 9:32 ` Geert Uytterhoeven
-1 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2022-02-10 9:32 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Arnd Bergmann, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
Hi Sergei,
On Wed, Feb 9, 2022 at 4:48 PM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
> On 2/9/22 6:18 PM, Arnd Bergmann wrote:
> >>> I had another look at this after you asked about it on IRC. I don't
> >>> know much SH assembly, but I suspect IRQ 0 has not been delivered
>
> Neither do I, sigh...
> I do know the instuctions are 16-bit and so there are no immediate
> opperands... :-)
There is byte immediate data (TIL).
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2022-02-10 9:32 ` Geert Uytterhoeven
0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2022-02-10 9:32 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Arnd Bergmann, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
Hi Sergei,
On Wed, Feb 9, 2022 at 4:48 PM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
> On 2/9/22 6:18 PM, Arnd Bergmann wrote:
> >>> I had another look at this after you asked about it on IRC. I don't
> >>> know much SH assembly, but I suspect IRQ 0 has not been delivered
>
> Neither do I, sigh...
> I do know the instuctions are 16-bit and so there are no immediate
> opperands... :-)
There is byte immediate data (TIL).
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
2022-02-10 9:32 ` Geert Uytterhoeven
@ 2022-02-10 9:46 ` Sergei Shtylyov
-1 siblings, 0 replies; 54+ messages in thread
From: Sergei Shtylyov @ 2022-02-10 9:46 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Arnd Bergmann, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On 2/10/22 12:32 PM, Geert Uytterhoeven wrote:
[...]
>>>>> I had another look at this after you asked about it on IRC. I don't
>>>>> know much SH assembly, but I suspect IRQ 0 has not been delivered
>>
>> Neither do I, sigh...
>> I do know the instuctions are 16-bit and so there are no immediate
>> opperands... :-)
>
> There is byte immediate data (TIL).
Yeah, I figured. :-)
> Gr{oetje,eeting}s,
>
> Geert
MBR, Sergey
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2022-02-10 9:46 ` Sergei Shtylyov
0 siblings, 0 replies; 54+ messages in thread
From: Sergei Shtylyov @ 2022-02-10 9:46 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Arnd Bergmann, Lad Prabhakar, Rob Herring,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Prabhakar, Linux-sh list
On 2/10/22 12:32 PM, Geert Uytterhoeven wrote:
[...]
>>>>> I had another look at this after you asked about it on IRC. I don't
>>>>> know much SH assembly, but I suspect IRQ 0 has not been delivered
>>
>> Neither do I, sigh...
>> I do know the instuctions are 16-bit and so there are no immediate
>> opperands... :-)
>
> There is byte immediate data (TIL).
Yeah, I figured. :-)
> Gr{oetje,eeting}s,
>
> Geert
MBR, Sergey
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
2021-12-20 10:17 ` Geert Uytterhoeven
@ 2021-12-20 11:55 ` Lad, Prabhakar
-1 siblings, 0 replies; 54+ messages in thread
From: Lad, Prabhakar @ 2021-12-20 11:55 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Lad Prabhakar, Rob Herring, Nicolas Saenz Julienne,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Linux-sh list
Hi Geert,
Thank you for the review.
On Mon, Dec 20, 2021 at 10:18 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> 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.
>
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?
> > 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?
>
OK.
> > + 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.
> 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...
>
Might be users have not updated their kernels.
[0] https://elixir.bootlin.com/linux/v5.16-rc6/source/arch/sh/kernel/cpu/sh4a/setup-sh7724.c#L454
Cheers,
Prabhakar
> > + 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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2021-12-20 11:55 ` Lad, Prabhakar
0 siblings, 0 replies; 54+ messages in thread
From: Lad, Prabhakar @ 2021-12-20 11:55 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Lad Prabhakar, Rob Herring, Nicolas Saenz Julienne,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Linux-sh list
Hi Geert,
Thank you for the review.
On Mon, Dec 20, 2021 at 10:18 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> 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.
>
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?
> > 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?
>
OK.
> > + 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.
> 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...
>
Might be users have not updated their kernels.
[0] https://elixir.bootlin.com/linux/v5.16-rc6/source/arch/sh/kernel/cpu/sh4a/setup-sh7724.c#L454
Cheers,
Prabhakar
> > + 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
2021-12-20 11:55 ` Lad, Prabhakar
@ 2021-12-20 12:54 ` Geert Uytterhoeven
-1 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2021-12-20 12:54 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: Lad Prabhakar, Rob Herring, Nicolas Saenz Julienne,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Linux-sh list
Hi Prabhakar,
On Mon, Dec 20, 2021 at 12:56 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Mon, Dec 20, 2021 at 10:18 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > 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.
> >
> 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()?
> > > --- a/drivers/i2c/busses/i2c-sh_mobile.c
> > > +++ b/drivers/i2c/busses/i2c-sh_mobile.c
> > > + 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").
> [0] https://elixir.bootlin.com/linux/v5.16-rc6/source/arch/sh/kernel/cpu/sh4a/setup-sh7724.c#L454
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2021-12-20 12:54 ` Geert Uytterhoeven
0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2021-12-20 12:54 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: Lad Prabhakar, Rob Herring, Nicolas Saenz Julienne,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Linux-sh list
Hi Prabhakar,
On Mon, Dec 20, 2021 at 12:56 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Mon, Dec 20, 2021 at 10:18 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > 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.
> >
> 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()?
> > > --- a/drivers/i2c/busses/i2c-sh_mobile.c
> > > +++ b/drivers/i2c/busses/i2c-sh_mobile.c
> > > + 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").
> [0] https://elixir.bootlin.com/linux/v5.16-rc6/source/arch/sh/kernel/cpu/sh4a/setup-sh7724.c#L454
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
2021-12-20 12:54 ` Geert Uytterhoeven
@ 2021-12-20 13:00 ` Lad, Prabhakar
-1 siblings, 0 replies; 54+ messages in thread
From: Lad, Prabhakar @ 2021-12-20 13:00 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Marc Zyngier, Lad Prabhakar, Rob Herring, Nicolas Saenz Julienne,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Linux-sh list
Hi Geert,
On Mon, Dec 20, 2021 at 12:54 PM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, Dec 20, 2021 at 12:56 PM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Mon, Dec 20, 2021 at 10:18 AM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > 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.
> > >
> > 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.
> > > > --- a/drivers/i2c/busses/i2c-sh_mobile.c
> > > > +++ b/drivers/i2c/busses/i2c-sh_mobile.c
>
> > > > + 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
> > [0] https://elixir.bootlin.com/linux/v5.16-rc6/source/arch/sh/kernel/cpu/sh4a/setup-sh7724.c#L454
>
> 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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to get the interrupt
@ 2021-12-20 13:00 ` Lad, Prabhakar
0 siblings, 0 replies; 54+ messages in thread
From: Lad, Prabhakar @ 2021-12-20 13:00 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Marc Zyngier, Lad Prabhakar, Rob Herring, Nicolas Saenz Julienne,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Chris Brandt, Wolfram Sang, Linux I2C,
linux-rpi-kernel, Linux ARM, Linux Kernel Mailing List,
Linux-Renesas, Linux-sh list
Hi Geert,
On Mon, Dec 20, 2021 at 12:54 PM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, Dec 20, 2021 at 12:56 PM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Mon, Dec 20, 2021 at 10:18 AM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > 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.
> > >
> > 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.
> > > > --- a/drivers/i2c/busses/i2c-sh_mobile.c
> > > > +++ b/drivers/i2c/busses/i2c-sh_mobile.c
>
> > > > + 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
> > [0] https://elixir.bootlin.com/linux/v5.16-rc6/source/arch/sh/kernel/cpu/sh4a/setup-sh7724.c#L454
>
> 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 54+ messages in thread