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
Re: [Cocci] How to match all conditions in if-else if blocks?
On Tue, 30 Jul 2019, Chuhong Yuan wrote: > Hi all, > I want to write a script to match a pattern of conditional statement. > However, I find that I can only match the first "if" in the block. > The script is like this: > fn(...) { > ... > - f(arg1, arg2, arg3) > ... > } > where f returns the condition check result. ... means that what is before and after should not appear in the code matched by ... This is useful for patterns like A ... B when you want the closest A and B. You can remove this restriction using ... when any However that will not completely solve your problem, because when transformation is used with ..., the patterns have to appear on every control-flow path. To eliminate this constraint, you can use fn(...) { <... - f(...) ...> } The <... ...> allows the pattern to appear 0 or more times in the matched region. If you want to be sure that it appears one or more times, you can use <+... ...+>. But the latter is more expensive and may not be necessary. On the other hand, if the name of the function is not very important, it would be much more efficient to just match the function call that you are interested in directly, without putting the function definition pattern around it. julia > > But when I apply to source code like this: > if (f(a1, a2, a3)) { > ... > } else if (f(a4, a5, a6)) { > ... > } > I find it can only match f(a1, a2, a3). > So how to fix this problem? > > Thanks, > Chuhong > ___ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] How to match all conditions in if-else if blocks?
Hi all, I want to write a script to match a pattern of conditional statement. However, I find that I can only match the first "if" in the block. The script is like this: fn(...) { ... - f(arg1, arg2, arg3) ... } where f returns the condition check result. But when I apply to source code like this: if (f(a1, a2, a3)) { ... } else if (f(a4, a5, a6)) { ... } I find it can only match f(a1, a2, a3). So how to fix this problem? Thanks, Chuhong ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v6 00/57] Add error message to platform_get_irq*()
> This is a resend of patch 2/3 from the v5[1] series, but split > up to be per-subsystem. Please apply patches directly to > subsystem trees if possible. > > [1] https://lkml.kernel.org/r/20190730053845.126834-1-swb...@chromium.org Would it have been nicer to fix also the commit descriptions before such a resend? How will the the corresponding software development attention evolve further? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: patch "coccinelle: Add script to check for platform_get_irq() excessive" added to driver-core-next
> This is a note to let you know that I've just added the patch titled > > coccinelle: Add script to check for platform_get_irq() excessive > > to my driver-core git tree which can be found at > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git > in the driver-core-next branch. I find it interesting somehow that this version of a script for the semantic patch language can be integrated already in this development branch. > The patch will also be merged in the next major kernel release > during the merge window. How much will questionable implementation details influence this process? Regards, Markus