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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux