Re: [Cocci] [PATCH v5 0/3] Add error message to platform_get_irq*()
Hi Wolfram,
On Thu, Aug 1, 2019 at 3:13 PM Wolfram Sang wrote:
> > these drivers pop up, I think we can have another function like
> > platform_get_irq_probe() or platform_get_irq_nowarn() that doesn't print
> > an error message. Then we can convert the drivers that are poking around
> > for interrupts to use this new function instead. It isn't the same as a
> > platform_get_optional_irq() API because it returns an error when the irq
> > isn't there or we fail to parse something, but at least the error
> > message is gone.
>
> True.
>
> I still feel uneasy about pushing false positive error messages to
> users. Do you think your cocci-script could be updated to modify drivers
> which do not bail out when platform_get_irq() fails to use
> platform_get_irq_nowarn()? I'd think this would catch most of them?
>
> Or maybe the other way around? platform_get_irq_warn() and only convert
> those which print something?
Following clk, gpio, regulator, and reset, the functions should be called
platform_get_irq() and platform_get_irq_optional().
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [Cocci] [PATCH v5 0/3] Add error message to platform_get_irq*()
> these drivers pop up, I think we can have another function like > platform_get_irq_probe() or platform_get_irq_nowarn() that doesn't print > an error message. Then we can convert the drivers that are poking around > for interrupts to use this new function instead. It isn't the same as a > platform_get_optional_irq() API because it returns an error when the irq > isn't there or we fail to parse something, but at least the error > message is gone. True. I still feel uneasy about pushing false positive error messages to users. Do you think your cocci-script could be updated to modify drivers which do not bail out when platform_get_irq() fails to use platform_get_irq_nowarn()? I'd think this would catch most of them? Or maybe the other way around? platform_get_irq_warn() and only convert those which print something? signature.asc Description: PGP signature ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v5 0/3] Add error message to platform_get_irq*()
Quoting Wolfram Sang (2019-07-31 07:26:46) > Hi Stephen, > > > There were some comments about adding an 'optional' platform_get_irq() > > API in v4. This series doesn't include that, but I can add such an API > > if it's required. I started to look into how it might work and got hung > > up on what an optional IRQ means. I suppose it means that in DT there > > isn't an 'interrupts' property in the device node, but in ACPI based > > firmware I'm not sure what that would correspond to. Furthermore, the > > return value is hard to comprehend. Do we return an error when an > > optional irq can't be found? It doesn't seem safe to return 0 because > > sometimes 0 is a valid IRQ. Do other errors in parsing the IRQ > > constitute a failure when the IRQ is optional? > > Some time ago, I tried a series like yours and got stuck at this very > point. I found drivers where using an interrupt was optional and > platform_get_irq() returning a failure wasn't fatal. The drivers used > PIO then or dropped some additional functionality. Some of them were > very old. > > I didn't like the idea that platform_get_irq() will spit out errors for > those drivers, yet I couldn't create a suitable cocci-script to convert > drivers to use the *_optional callback where possible. So, I neither > created the optional callback. > > I still have doubts of unneeded error messages popping up. Has this been > discussed already? (Sorry, I missed the first iterations of this series) Not heavily discussed, but it was mentioned in an earlier round. If these drivers pop up, I think we can have another function like platform_get_irq_probe() or platform_get_irq_nowarn() that doesn't print an error message. Then we can convert the drivers that are poking around for interrupts to use this new function instead. It isn't the same as a platform_get_optional_irq() API because it returns an error when the irq isn't there or we fail to parse something, but at least the error message is gone.
Re: [Cocci] [PATCH v5 0/3] Add error message to platform_get_irq*()
Hi Stephen, > There were some comments about adding an 'optional' platform_get_irq() > API in v4. This series doesn't include that, but I can add such an API > if it's required. I started to look into how it might work and got hung > up on what an optional IRQ means. I suppose it means that in DT there > isn't an 'interrupts' property in the device node, but in ACPI based > firmware I'm not sure what that would correspond to. Furthermore, the > return value is hard to comprehend. Do we return an error when an > optional irq can't be found? It doesn't seem safe to return 0 because > sometimes 0 is a valid IRQ. Do other errors in parsing the IRQ > constitute a failure when the IRQ is optional? Some time ago, I tried a series like yours and got stuck at this very point. I found drivers where using an interrupt was optional and platform_get_irq() returning a failure wasn't fatal. The drivers used PIO then or dropped some additional functionality. Some of them were very old. I didn't like the idea that platform_get_irq() will spit out errors for those drivers, yet I couldn't create a suitable cocci-script to convert drivers to use the *_optional callback where possible. So, I neither created the optional callback. I still have doubts of unneeded error messages popping up. Has this been discussed already? (Sorry, I missed the first iterations of this series) Thanks, Wolfram signature.asc Description: PGP signature
