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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20160306143522.GA5376@katana>
Date:	Sun, 6 Mar 2016 15:35:22 +0100
From:	Wolfram Sang <wsa@...-dreams.de>
To:	Jan Glauber <jan.glauber@...iumnetworks.com>
Cc:	linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
	David Daney <ddaney@...iumnetworks.com>
Subject: Re: [PATCH v2 01/10] i2c-octeon: Cleanup i2c-octeon driver


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


Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)