Re: [PATCH v6 5/6] i2c: Add Actions Semiconductor Owl family S900 I2C driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2018-07-23 09:24, Andy Shevchenko wrote:
> On Mon, Jul 23, 2018 at 9:12 AM, Peter Rosin <peda@xxxxxxxxxx> wrote:
>> On 2018-07-23 05:40, Manivannan Sadhasivam wrote:
>>> +             /* Handle the remaining bytes which were not sent */
>>> +             while (!(readl(i2c_dev->base + OWL_I2C_REG_FIFOSTAT) &
>>> +                    OWL_I2C_FIFOSTAT_TFF) && i2c_dev->msg_ptr < msg->len) {
>>
>> You moved the OWL_ line to the left instead of right, so this is still
>> misaligned and thus not helping the reader.
> 
> Guys, wouldn't be better to
> 
> 
> while (i2c_dev->msg_ptr < msg->len) {
>  u32 fifostat = readl();
> 
>  if (!(fifostat & ...))
>  break;
> ...
> }
> 
> ?
> 
> Same for the other branch.
> 
> Yes, it's more LOCs, but less bikeshedding,

Says the guy arriving late to the table with an incredibly valuable
suggestion... Please explain why my suggestion is bikeshedding and
yours is not?

Aligning with the opening parenthesis is right. Aligning with the wrong
opening parenthesis is of course going to risk confusing the reader.

Cheers,
Peter



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux