2014-07-07 14:05:45

by Himangi Saraogi

[permalink] [raw]
Subject: [PATCH] drivers: CCI: Correct use of ! and &

In commit ae91d60ba88ef0bdb1b5e9b2363bd52fc45d2af7, a bug was fixed that
involved converting !x & y to !(x & y). The code below shows the same
pattern, and thus should perhaps be fixed in the same way.

The Coccinelle semantic patch that makes this change is as follows:

// <smpl>
@@ expression E1,E2; @@
(
!E1 & !E2
|
- !E1 & E2
+ !(E1 & E2)
)
// </smpl>

Signed-off-by: Himangi Saraogi <[email protected]>
Acked-by: Julia Lawall <[email protected]>
---
This is not tested and clearly changes the semantics, so it is only
something to consider.
drivers/bus/arm-cci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 5a86da9..7af78df 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -397,7 +397,8 @@ static irqreturn_t pmu_handle_irq(int irq_num, void *dev)
hw_counter = &event->hw;

/* Did this counter overflow? */
- if (!pmu_read_register(idx, CCI_PMU_OVRFLW) & CCI_PMU_OVRFLW_FLAG)
+ if (!(pmu_read_register(idx, CCI_PMU_OVRFLW) &
+ CCI_PMU_OVRFLW_FLAG))
continue;

pmu_write_register(CCI_PMU_OVRFLW_FLAG, idx, CCI_PMU_OVRFLW);
--
1.9.1


2014-07-23 15:03:05

by Punit Agrawal

[permalink] [raw]
Subject: Re: [PATCH] drivers: CCI: Correct use of ! and &

Adding alkml and Will

Hi Himangi,

In future it would help if you send the patch to the the maintainers by
running the get_maintainers.pl script on the patch.

Himangi Saraogi <[email protected]> writes:

> In commit ae91d60ba88ef0bdb1b5e9b2363bd52fc45d2af7, a bug was fixed that
> involved converting !x & y to !(x & y). The code below shows the same
> pattern, and thus should perhaps be fixed in the same way.
>
> The Coccinelle semantic patch that makes this change is as follows:
>
> // <smpl>
> @@ expression E1,E2; @@
> (
> !E1 & !E2
> |
> - !E1 & E2
> + !(E1 & E2)
> )
> // </smpl>
>
> Signed-off-by: Himangi Saraogi <[email protected]>
> Acked-by: Julia Lawall <[email protected]>
> ---
> This is not tested and clearly changes the semantics, so it is only
> something to consider.
> drivers/bus/arm-cci.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> index 5a86da9..7af78df 100644
> --- a/drivers/bus/arm-cci.c
> +++ b/drivers/bus/arm-cci.c
> @@ -397,7 +397,8 @@ static irqreturn_t pmu_handle_irq(int irq_num, void *dev)
> hw_counter = &event->hw;
>
> /* Did this counter overflow? */
> - if (!pmu_read_register(idx, CCI_PMU_OVRFLW) & CCI_PMU_OVRFLW_FLAG)
> + if (!(pmu_read_register(idx, CCI_PMU_OVRFLW) &
> + CCI_PMU_OVRFLW_FLAG))
> continue;


Going back to the manual, this fix looks correct.

Acked-by: Punit Agrawal <[email protected]>

Will, would this go via your tree?

>
> pmu_write_register(CCI_PMU_OVRFLW_FLAG, idx, CCI_PMU_OVRFLW);

2014-07-23 15:05:52

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH] drivers: CCI: Correct use of ! and &

On Wed, Jul 23, 2014 at 04:01:56PM +0100, Punit Agrawal wrote:
> > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> > index 5a86da9..7af78df 100644
> > --- a/drivers/bus/arm-cci.c
> > +++ b/drivers/bus/arm-cci.c
> > @@ -397,7 +397,8 @@ static irqreturn_t pmu_handle_irq(int irq_num, void *dev)
> > hw_counter = &event->hw;
> >
> > /* Did this counter overflow? */
> > - if (!pmu_read_register(idx, CCI_PMU_OVRFLW) & CCI_PMU_OVRFLW_FLAG)
> > + if (!(pmu_read_register(idx, CCI_PMU_OVRFLW) &
> > + CCI_PMU_OVRFLW_FLAG))
> > continue;
>
>
> Going back to the manual, this fix looks correct.
>
> Acked-by: Punit Agrawal <[email protected]>
>
> Will, would this go via your tree?

Given that you're happy with it, I don't mind which tree it goes in.
Probably deserves a CC stable on it too.

If you get stuck, put it in rmk's patch system.

Will

2014-07-29 11:35:54

by Punit Agrawal

[permalink] [raw]
Subject: Re: [PATCH] drivers: CCI: Correct use of ! and &

Hi Arnd,

Will Deacon <[email protected]> writes:

> On Wed, Jul 23, 2014 at 04:01:56PM +0100, Punit Agrawal wrote:
>> > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
>> > index 5a86da9..7af78df 100644
>> > --- a/drivers/bus/arm-cci.c
>> > +++ b/drivers/bus/arm-cci.c
>> > @@ -397,7 +397,8 @@ static irqreturn_t pmu_handle_irq(int irq_num, void *dev)
>> > hw_counter = &event->hw;
>> >
>> > /* Did this counter overflow? */
>> > - if (!pmu_read_register(idx, CCI_PMU_OVRFLW) & CCI_PMU_OVRFLW_FLAG)
>> > + if (!(pmu_read_register(idx, CCI_PMU_OVRFLW) &
>> > + CCI_PMU_OVRFLW_FLAG))
>> > continue;
>>
>>
>> Going back to the manual, this fix looks correct.
>>
>> Acked-by: Punit Agrawal <[email protected]>
>>
>> Will, would this go via your tree?
>
> Given that you're happy with it, I don't mind which tree it goes in.
> Probably deserves a CC stable on it too.

The CCI PMU patches went via arm-soc. Are you happy to pick this fix with
the Ack and a Cc to stable.

Cheers,
Punit

>
> If you get stuck, put it in rmk's patch system.
>
> Will
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2014-07-29 16:41:38

by Olof Johansson

[permalink] [raw]
Subject: Re: [PATCH] drivers: CCI: Correct use of ! and &

On Tue, Jul 29, 2014 at 4:34 AM, Punit Agrawal <[email protected]> wrote:
> Hi Arnd,
>
> Will Deacon <[email protected]> writes:
>
>> On Wed, Jul 23, 2014 at 04:01:56PM +0100, Punit Agrawal wrote:
>>> > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
>>> > index 5a86da9..7af78df 100644
>>> > --- a/drivers/bus/arm-cci.c
>>> > +++ b/drivers/bus/arm-cci.c
>>> > @@ -397,7 +397,8 @@ static irqreturn_t pmu_handle_irq(int irq_num, void *dev)
>>> > hw_counter = &event->hw;
>>> >
>>> > /* Did this counter overflow? */
>>> > - if (!pmu_read_register(idx, CCI_PMU_OVRFLW) & CCI_PMU_OVRFLW_FLAG)
>>> > + if (!(pmu_read_register(idx, CCI_PMU_OVRFLW) &
>>> > + CCI_PMU_OVRFLW_FLAG))
>>> > continue;
>>>
>>>
>>> Going back to the manual, this fix looks correct.
>>>
>>> Acked-by: Punit Agrawal <[email protected]>
>>>
>>> Will, would this go via your tree?
>>
>> Given that you're happy with it, I don't mind which tree it goes in.
>> Probably deserves a CC stable on it too.
>
> The CCI PMU patches went via arm-soc. Are you happy to pick this fix with
> the Ack and a Cc to stable.

Yeah, we've been merging most of the CCI patches.

Please resend the patch to [email protected] if you want us to apply it.


Thanks,

-Olof

2014-07-30 10:33:42

by Punit Agrawal

[permalink] [raw]
Subject: Re: [PATCH] drivers: CCI: Correct use of ! and &

Olof Johansson <[email protected]> writes:

> On Tue, Jul 29, 2014 at 4:34 AM, Punit Agrawal <[email protected]> wrote:
>> Hi Arnd,
>>
>> Will Deacon <[email protected]> writes:
>>
>>> On Wed, Jul 23, 2014 at 04:01:56PM +0100, Punit Agrawal wrote:
>>>> > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
>>>> > index 5a86da9..7af78df 100644
>>>> > --- a/drivers/bus/arm-cci.c
>>>> > +++ b/drivers/bus/arm-cci.c
>>>> > @@ -397,7 +397,8 @@ static irqreturn_t pmu_handle_irq(int irq_num, void *dev)
>>>> > hw_counter = &event->hw;
>>>> >
>>>> > /* Did this counter overflow? */
>>>> > - if (!pmu_read_register(idx, CCI_PMU_OVRFLW) & CCI_PMU_OVRFLW_FLAG)
>>>> > + if (!(pmu_read_register(idx, CCI_PMU_OVRFLW) &
>>>> > + CCI_PMU_OVRFLW_FLAG))
>>>> > continue;
>>>>
>>>>
>>>> Going back to the manual, this fix looks correct.
>>>>
>>>> Acked-by: Punit Agrawal <[email protected]>
>>>>
>>>> Will, would this go via your tree?
>>>
>>> Given that you're happy with it, I don't mind which tree it goes in.
>>> Probably deserves a CC stable on it too.
>>
>> The CCI PMU patches went via arm-soc. Are you happy to pick this fix with
>> the Ack and a Cc to stable.
>
> Yeah, we've been merging most of the CCI patches.
>
> Please resend the patch to [email protected] if you want us to apply it.

Ok, thanks! I'll send the patch with the tags applied shortly.

>
>
> Thanks,
>
> -Olof
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/