Re: [PATCH v2 01/10] i2c-octeon: Cleanup i2c-octeon driver

From: Wolfram Sang
Date: Sun Mar 06 2016 - 09:36:21 EST



> Fixed. Strange that checkpatch.pl -f (use on file) does not report these
> errors though.

Huh, indeed.

> > > struct octeon_i2c {
> > > - wait_queue_head_t queue;
> > > - struct i2c_adapter adap;
> > > - int irq;
> > > - u32 twsi_freq;
> > > - int sys_freq;
> > > - resource_size_t twsi_phys;
> > > - void __iomem *twsi_base;
> > > - resource_size_t regsize;
> > > - struct device *dev;
> > > + wait_queue_head_t queue;
> > > + struct i2c_adapter adap;
> > > + int irq;
> > > + u32 twsi_freq;
> > > + int sys_freq;
> > > + void __iomem *twsi_base;
> > > + struct device *dev;
> >
> > NAK. structs change often, and then one needs to fix the whole
> > indentation. One space is enough here.
>
> Not sure I understand your argument. I find this form more readable
> but I can change that to one space.

a) it spoils git history and makes harder to find out which patch
added which member of the struct
b) patches get harder to read when the indentation of the whole
struct changes. New members are not always put at the end.

> > I dunno understand the change of the function name. However, this should
> > be refactored to use the i2c_bus_recovery infrastructure anyhow.
>
> I'll leave the function name as it is. Would it be possbile to address
> the refactoring in a follow-up patch after this series is finished?

Yes, that makes sense.

> I'll split it into several patches then.

Sounds good. Thanks!

Attachment: signature.asc
Description: PGP signature