Re: [Qemu-devel] [PATCH 02/11] pxa2xx: QOM'ify I2C slave

[Peter Maydell]

On 9 February 2014 12:24, Andreas Färber <address@hidden> wrote:
> Am 09.02.2014 02:35, schrieb Peter Crosthwaite:
>> On Sat, Feb 1, 2014 at 12:34 AM, Andreas Färber <address@hidden> wrote:
>>> @@ -1292,10 +1298,12 @@ static void pxa2xx_i2c_event(I2CSlave *i2c, enum 
>>> i2c_event event)
>>>
>>>  static int pxa2xx_i2c_rx(I2CSlave *i2c)
>>>  {
>>> -    PXA2xxI2CSlaveState *slave = FROM_I2C_SLAVE(PXA2xxI2CSlaveState, i2c);
>>> +    PXA2xxI2CSlaveState *slave = PXA2XX_I2C_SLAVE(i2c);
>>>      PXA2xxI2CState *s = slave->host;
>>> -    if ((s->control & (1 << 14)) || !(s->control & (1 << 6)))
>>> +
>>> +    if ((s->control & (1 << 14)) || !(s->control & (1 << 6))) {
>>>          return 0;
>>> +    }
>>
>> This will look funny when git-blamed. Should out-of-scope trivials be
>> separate patch so anyone git-blame will at least see "Fix coding
>> style" rather than then misleading "QOM'ify I2C"?.
>
> We've had the policy of fixing Coding Style on lines touched or adjacent
> to those touched, to gradually get rid of them - this one was within 3
> lines of context. I'm sure PXA code will have many more Coding Style
> faults, so placing 2 out of X in their own patch seems silly. Should I
> rather drop them?

I'll weigh in here since in some sense pxa2xx is my department,
I guess. I would probably not have personally bothered with this
style fix (my usual personal approach is "if checkpatch complains"),
but as Andreas says it's within our usual practice, so I don't have
a problem with it.

thanks
-- PMM