| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 20 Dec 2021 14:53:27 +0300
From: Sergei Shtylyov <sergei.shtylyov@...il.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>,
Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Cc: Rob Herring <robh+dt@...nel.org>,
Nicolas Saenz Julienne <nsaenz@...nel.org>,
Florian Fainelli <f.fainelli@...il.com>,
Ray Jui <rjui@...adcom.com>,
Scott Branden <sbranden@...adcom.com>,
bcm-kernel-feedback-list <bcm-kernel-feedback-list@...adcom.com>,
Chris Brandt <chris.brandt@...esas.com>,
Wolfram Sang <wsa+renesas@...g-engineering.com>,
Linux I2C <linux-i2c@...r.kernel.org>,
linux-rpi-kernel <linux-rpi-kernel@...ts.infradead.org>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
Prabhakar <prabhakar.csengg@...il.com>,
Linux-sh list <linux-sh@...r.kernel.org>
Subject: Re: [PATCH 2/3] i2c: sh_mobile: Use platform_get_irq_optional() to
get the interrupt
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@...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@...ux-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
Powered by blists - more mailing lists