On 07:53 Mon 30 Mar , Heiko Schocher wrote:
> Hello Jean,
>
> Jean-Christophe PLAGNIOL-VILLARD wrote:
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagn...@jcrosoft.com>
> > ---
> > cpu/arm926ejs/at91/Makefile | 6 +
> > .../arm926ejs/at91/at91cap9_i2c.c | 31 ++--
> > .../arm926ejs/at91/at91sam9260_i2c.c | 31 ++--
> > .../arm926ejs/at91/at91sam9261_i2c.c | 31 ++--
> > .../arm926ejs/at91/at91sam9263_i2c.c | 31 ++--
> > .../clk.h => cpu/arm926ejs/at91/at91sam9rl_i2c.c | 37 ++--
> > drivers/i2c/Makefile | 1 +
> > drivers/i2c/atmel_i2c.c | 234
> > ++++++++++++++++++++
> > include/asm-arm/arch-at91/at91_common.h | 2 +
> > include/asm-arm/arch-at91/at91_twi.h | 68 ++++++
> > include/asm-arm/arch-at91/clk.h | 4 +
> > include/asm-arm/arch-at91/hardware.h | 5 +
> > 12 files changed, 405 insertions(+), 76 deletions(-)
> > copy include/asm-arm/arch-at91/at91_common.h =>
> > cpu/arm926ejs/at91/at91cap9_i2c.c (61%)
> > copy include/asm-arm/arch-at91/at91_common.h =>
> > cpu/arm926ejs/at91/at91sam9260_i2c.c (61%)
> > copy include/asm-arm/arch-at91/at91_common.h =>
> > cpu/arm926ejs/at91/at91sam9261_i2c.c (61%)
> > copy include/asm-arm/arch-at91/at91_common.h =>
> > cpu/arm926ejs/at91/at91sam9263_i2c.c (61%)
> > copy include/asm-arm/arch-at91/clk.h =>
> > cpu/arm926ejs/at91/at91sam9rl_i2c.c (52%)
> > create mode 100644 drivers/i2c/atmel_i2c.c
> > create mode 100644 include/asm-arm/arch-at91/at91_twi.h
> >
> >
> [...]
> > diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> > index 9c74657..27a5251 100644
> > --- a/drivers/i2c/Makefile
> > +++ b/drivers/i2c/Makefile
> > @@ -25,6 +25,7 @@ include $(TOPDIR)/config.mk
> >
> > LIB := $(obj)libi2c.a
> >
> > +COBJS-$(CONFIG_I2C_ATMEL) += atmel_i2c.o
> > COBJS-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o
> > COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
> > COBJS-$(CONFIG_I2C_MXC) += mxc_i2c.o
> >
>
> Independent from your patch, I think we need here a common
> rule. Either we name in future this define "CONFIG_I2C_XXX"
> or "CONFIG_XXX_I2C. I vote for using "CONFIG_I2C_XXX"
it's plan to clean up the driver CONFIG_ just before send the Kconfig patch
series
> > + *
> > +*/
> > +#include <common.h>
> > +
> > +#ifdef CONFIG_HARD_I2C
> >
>
> Why not "#if defined(CONFIG_I2C_ATMEL)" as in the Makefile?
until the Kconfig I'll prefer to keep the CONFIG_HARD_I2C
>
> > +
> > +#include <i2c.h>
> > +#include <asm/io.h>
> >
> [...]
> > + if (alen > 1)
> > + return 1;
> > + /* XXX assume an ATMEL AT24C16 */
> > + if (alen == 1) {
> > +#if 0 /* EEPROM code already sets this correctly */
> > + chip |= (addr >> 8) & 0xff;
> > +#endif
> >
>
> please remove dead code.
>
> > + addr = addr & 0xff;
> > + }
> > +#endif
> > + return at91_xfer(chip, addr, alen, buffer, len, 1);
> > +}
> > +
> > +int
> > +i2c_write(unsigned char chip, unsigned int addr, int alen,
> > + unsigned char *buffer, int len)
> > +{
> > +#ifdef CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW
> > + int i;
> > + unsigned char *buf;
> > +
> > + /* we only allow one address byte */
> > + if (alen > 1)
> > + return 1;
> > + /* XXX assume an ATMEL AT24C16 */
> > + if (alen == 1) {
> > + buf = buffer;
> > + /* do single byte writes */
> > + for (i = 0; i < len; i++) {
> > +#if 0 /* EEPROM code already sets this correctly */
> > + chip |= (addr >> 8) & 0xff;
> > +#endif
> >
>
> here too, please remove dead code.
>
> > + addr = addr & 0xff;
> > + if (at91_xfer(chip, addr, alen, buf++, 1, 0))
> > + return 1;
> > + addr++;
> > + }
> > + return 0;
> >
> [...]
> > diff --git a/include/asm-arm/arch-at91/at91_twi.h
> > b/include/asm-arm/arch-at91/at91_twi.h
> > new file mode 100644
> > index 0000000..bb2880f
> > --- /dev/null
> > +++ b/include/asm-arm/arch-at91/at91_twi.h
> > @@ -0,0 +1,68 @@
> > +/*
> > + * arch/arm/mach-at91/include/mach/at91_twi.h
> > + *
> > + * Copyright (C) 2005 Ivan Kokshaysky
> > + * Copyright (C) SAN People
> > + *
> > + * Two-wire Interface (TWI) registers.
> > + * Based on AT91RM9200 datasheet revision E.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#ifndef AT91_TWI_H
> > +#define AT91_TWI_H
> > +
> > +#define AT91_TWI_CR 0x00 /* Control Register */
> > +#define AT91_TWI_START (1 << 0) /* Send a Start
> > Condition */
> > +#define AT91_TWI_STOP (1 << 1) /* Send a Stop
> > Condition */
> > +#define AT91_TWI_MSEN (1 << 2) /* Master
> > Transfer Enable */
> > +#define AT91_TWI_MSDIS (1 << 3) /* Master
> > Transfer Disable */
> > +#define AT91_TWI_SVEN (1 << 4) /* Slave
> > Transfer Enable [SAM9260 only] */
> > +#define AT91_TWI_SVDIS (1 << 5) /* Slave
> > Transfer Disable [SAM9260 only] */
> > +#define AT91_TWI_SWRST (1 << 7) /* Software
> > Reset */
> >
> ^^^^^^^^^^^^^^^
> doule tabs breaks line length (this affects for the
> whole file)
>
I'll prefer to keep it as it because it's an import from Linux
Best Regards,
J.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
