Re: [Cocci] [PATCH v5 0/3] Add error message to platform_get_irq*()

2019-07-31 Thread Stephen Boyd
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*()

2019-07-31 Thread Wolfram Sang
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?

2019-07-31 Thread Julia Lawall



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?

2019-07-31 Thread Chuhong Yuan
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*()

2019-07-31 Thread Markus Elfring
> 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

2019-07-31 Thread Markus Elfring
> 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