Re: [PATCH 7/7] MIPS: GIC: Fix GICBIS macro

[Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>]

Hello.

On 07/18/2014 11:54 AM, Jeffrey Deans wrote:

>>> From: Jeffrey Deans <jeffrey.deans@imgtec.com>

>>> The GICBIS macro could update the GIC registers incorrectly, depending
>>> on the data value passed in:

>>> * Bits were only OR'd into the register data, so register fields could
>>>    not be cleared.

>>> * Bits were OR'd into the register data without masking the data to the
>>>    correct field width, corrupting adjacent bits.

>>> Signed-off-by: Jeffrey Deans <jeffrey.deans@imgtec.com>
>>> Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
>>> ---
>>>   arch/mips/include/asm/gic.h | 21 +++++++++++----------
>>>   1 file changed, 11 insertions(+), 10 deletions(-)

>>> diff --git a/arch/mips/include/asm/gic.h b/arch/mips/include/asm/gic.h
>>> index 8b30befd99d6..3f20b2111d56 100644
>>> --- a/arch/mips/include/asm/gic.h
>>> +++ b/arch/mips/include/asm/gic.h
>>> @@ -43,18 +43,17 @@
>>>   #ifdef GICISBYTELITTLEENDIAN
>>>   #define GICREAD(reg, data)    ((data) = (reg), (data) =
>>> le32_to_cpu(data))
>>>   #define GICWRITE(reg, data)    ((reg) = cpu_to_le32(data))
>>> -#define GICBIS(reg, bits)            \
>>> -    ({unsigned int data;            \
>>> -        GICREAD(reg, data);        \
>>> -        data |= bits;            \
>>> -        GICWRITE(reg, data);        \
>>> -    })
>>> -
>>>   #else
>>>   #define GICREAD(reg, data)    ((data) = (reg))
>>>   #define GICWRITE(reg, data)    ((reg) = (data))
>>> -#define GICBIS(reg, bits)    ((reg) |= (bits))
>>>   #endif
>>> +#define GICBIS(reg, mask, bits)            \
>>> +    do { u32 data;                \
>>> +        GICREAD((reg), data);        \

>>     Why () only around 'reg', not around 'data'?

> Brackets aren't necessary around "data" because it is declared at the start of
> the "do" code block, so it can't expand to anything else within that scope.

    Oh, I was not attentive enough, sorry about that... :-<
    However, it makes sense to at least put that declaration at a separate line.

WBR, Sergei