Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1351125

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

From Wolfram Sang <wsa@the-dreams.de>
Newsgroups linux.kernel
Subject Re: [PATCH v2 01/10] i2c-octeon: Cleanup i2c-octeon driver
Date 2016-03-06 15:40 +0100
Message-ID <r9I0F-6gq-1@gated-at.bofh.it> (permalink)
References <r92b7-1V6-3@gated-at.bofh.it> <r92b8-1V6-21@gated-at.bofh.it> <r9pr4-1LI-3@gated-at.bofh.it> <r9GBA-5rX-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


[Multipart message — attachments visible in raw view] - view raw

> 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!

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v2 00/10] i2c-octeon and i2c-thunderx drivers  Jan Glauber <jglauber@cavium.com> - 2016-03-04 19:00 +0100
  [PATCH v2 10/10] i2c: thunderx: add smbus support Jan Glauber <jglauber@cavium.com> - 2016-03-04 19:00 +0100
  [PATCH v2 01/10] i2c-octeon: Cleanup i2c-octeon driver Jan Glauber <jglauber@cavium.com> - 2016-03-04 19:00 +0100
    Re: [PATCH v2 01/10] i2c-octeon: Cleanup i2c-octeon driver Wolfram Sang <wsa@the-dreams.de> - 2016-03-05 19:50 +0100
      Re: [PATCH v2 01/10] i2c-octeon: Cleanup i2c-octeon driver Jan Glauber <jan.glauber@caviumnetworks.com> - 2016-03-06 14:10 +0100
        Re: [PATCH v2 01/10] i2c-octeon: Cleanup i2c-octeon driver Wolfram Sang <wsa@the-dreams.de> - 2016-03-06 15:40 +0100
  [PATCH v2 07/10] i2c-octeon: Faster operation when IFLG signals late Jan Glauber <jglauber@cavium.com> - 2016-03-04 19:00 +0100
  [PATCH v2 05/10] i2c-octeon: Add support for cn78XX chips Jan Glauber <jglauber@cavium.com> - 2016-03-04 19:10 +0100
  [PATCH v2 04/10] dt-bindings: i2c: add Octeon cn78xx TWSI Jan Glauber <jglauber@cavium.com> - 2016-03-04 19:10 +0100
  [PATCH v2 06/10] i2c-octeon: Flush TWSI writes with readback Jan Glauber <jglauber@cavium.com> - 2016-03-04 19:10 +0100

csiph-web