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

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 1475 bytes --]


> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]