Re: [PATCH v2] i2c: add Marvell 64xxx driver

[Antony Pavlov <antonynpavlov@gmail.com>]

On Thu, 17 Jul 2014 11:33:29 +0200
Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote:

> On 07/16/2014 11:25 PM, Antony Pavlov wrote:
> > This driver is also used for Allwinner SoCs I2C controllers.
> >
> > Ported from linux-3.15.
> >
> > The most notable barebox driver version changes:
> >
> >    * drop message offloading support;
> >    * add reg-io-width parameter to use driver with byte-oriented
> >      controller versions.
> >
> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > ---
> [...]
> > diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
> > new file mode 100644
> > index 0000000..324796a
> > --- /dev/null
> > +++ b/drivers/i2c/busses/i2c-mv64xxx.c
> > @@ -0,0 +1,687 @@
> > +/*
> > + * Driver for the i2c controller on the Marvell line of host bridges
> > + * (e.g, gt642[46]0, mv643[46]0, mv644[46]0, and Orion SoC family).
> > + *
> > + * This code was ported from linux-3.15 kernel by Antony Pavlov.
> > + *
> > + * Author: Mark A. Greer <mgreer@mvista.com>
> > + *
> > + * 2005 (c) MontaVista, Software, Inc.  This file is licensed under
> > + * the terms of the GNU General Public License version 2.  This program
> > + * is licensed "as is" without any warranty of any kind, whether express
> > + * or implied.
> > + */
> > +#include <common.h>
> > +#include <driver.h>
> > +#include <init.h>
> > +#include <of.h>
> > +#include <malloc.h>
> > +#include <types.h>
> > +#include <xfuncs.h>
> > +#include <clock.h>
> > +#include <linux/clk.h>
> > +#include <linux/err.h>
> > +
> > +#include <io.h>
> > +#include <i2c/i2c.h>
> > +#include <printk.h>
> > +
> > +#define MV64XXX_I2C_ADDR_ADDR(val)			((val & 0x7f) << 1)
> > +#define MV64XXX_I2C_BAUD_DIV_N(val)			(val & 0x7)
> > +#define MV64XXX_I2C_BAUD_DIV_M(val)			((val & 0xf) << 3)
> > +
> > +#define	MV64XXX_I2C_REG_CONTROL_ACK			0x00000004
> > +#define	MV64XXX_I2C_REG_CONTROL_IFLG			0x00000008
> > +#define	MV64XXX_I2C_REG_CONTROL_STOP			0x00000010
> > +#define	MV64XXX_I2C_REG_CONTROL_START			0x00000020
> > +#define	MV64XXX_I2C_REG_CONTROL_TWSIEN			0x00000040
> > +#define	MV64XXX_I2C_REG_CONTROL_INTEN			0x00000080
> 
> Antony,
> 
> I'll have a look at the driver itself later, but your patch is full
> of superfluous tabs like above between '#define' and 'MV64XXX_foo'.

I have kept original linux driver formatting.

Please see it with any interactive two panels diff tool (e.g. vimdiff):

   vimdiff barebox.git/drivers/i2c/busses/i2c-mv64xxx.c linux.git/drivers/i2c/busses/i2c-mv64xxx.c

> 
> Isn't checkpatch.pl complaining about it?

On the one hand I need to change all writel/readl to mv64xxx_write/mv64xxx_read, so
some lines become 'over 80 characters'.

But on the other hand I'm trying to keep formatting compatibility with original linux driver;
I decided to keep original formatting even if lines become 'over 80 characters'.

Moreover Sascha said that there is no problem with 'line over 80 characters' warnings :)
see http://lists.infradead.org/pipermail/barebox/2014-June/019484.html


> Anyway, no need to resend before I have tested the driver itself.

 Great! The driver needs addition tests.

-- 
Best regards,
  Antony Pavlov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox