[U-Boot-Users] [PATCH/review] Blackfin: overhaul i2c driver

[Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>]

On 20:37 Fri 18 Apr     , Mike Frysinger wrote:
> The current Blackfin i2c driver does not work properly with certain devices
> due to it breaking up transfers incorrectly.  This is a rewrite of the
> driver and relocates it to the newer place in the source tree.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> I couldn't get git-format-patch to detect the rename (tried -B, -M, and -C),
> but the resulting diff between the files isn't terribly readable in the
> first place, so it shouldn't be that big of a deal.
> 
>  cpu/blackfin/Makefile         |    2 +-
>  cpu/blackfin/i2c.c            |  444 -----------------------------------------
>  drivers/i2c/Makefile          |    1 +
>  drivers/i2c/bfin-twi_i2c.c    |  300 +++++++++++++++++++++++++++
>  include/configs/bf533-ezkit.h |    2 +-
>  include/configs/bf533-stamp.h |    2 +-
>  include/configs/bf537-stamp.h |   43 +----
>  7 files changed, 308 insertions(+), 486 deletions(-)
>  delete mode 100644 cpu/blackfin/i2c.c
>  create mode 100644 drivers/i2c/bfin-twi_i2c.c
> 
> new file mode 100644
> index 0000000..9aceb0a
> --- /dev/null
> +++ b/drivers/i2c/bfin-twi_i2c.c
> @@ -0,0 +1,300 @@
> +/*
> + * i2c.c - driver for Blackfin on-chip TWI/I2C
> + *
> + * Copyright (c) 2006-2008 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include <common.h>
> +#include <i2c.h>
> +
> +#include <asm/blackfin.h>
> +#include <asm/mach-common/bits/twi.h>
> +
> +#define debugi(fmt, args...) \
> +	debug( \
> +		"MSTAT:0x%03x FSTAT:0x%x ISTAT:0x%02x\t" \
> +		"%-20s:%-3i: " fmt "\n", \
> +		bfin_read_TWI_MASTER_STAT(), bfin_read_TWI_FIFO_STAT(), bfin_read_TWI_INT_STAT(), \
could you split it
> +		__func__, __LINE__, ## args)
> +
<snip>
> +#if CFG_I2C_SLAVE
> +# error I2C slave support not tested/supported
> +	/* If they want us as a slave, do it */
> +	if (slaveaddr) {
> +		bfin_write_TWI_SLAVE_ADDR(slaveaddr);
> +		bfin_write_TWI_SLAVE_CTL(SEN);
> +	}
> +#endif
> +}
> +
> +/**
> + * i2c_probe: - Test if a chip answers for a given i2c address
> + *
> + * @chip:	address of the chip which is searched for
> + * @return: 	0 if a chip was found, -1 otherwhise
              ^
whitespace
> + */
> +int i2c_probe(uchar chip)
> +{
> +	u8 byte;
add an empty line
> +	return i2c_read(chip, 0, 0, &byte, 1);
> +}
> +
> +/**
> + *   i2c_read: - Read multiple bytes from an i2c device
> + *
> + *   chip:    I2C chip address, range 0..127
> + *   addr:    Memory (register) address within the chip
> + *   alen:    Number of bytes to use for addr (typically 1, 2 for larger
> + *              memories, 0 for register type devices with only one
> + *              register)
> + *   buffer:  Where to read/write the data
> + *   len:     How many bytes to read/write
> + *
> + *   Returns: 0 on success, not 0 on failure
> + */
> +int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
> +{
> +	return i2c_transfer(chip, addr, alen, buffer, len, (alen ? I2C_M_COMBO : I2C_M_READ));
> +}
> +
> +/**
> + *   i2c_write: -  Write multiple bytes to an i2c device
> + *
> + *   chip:    I2C chip address, range 0..127
> + *   addr:    Memory (register) address within the chip
> + *   alen:    Number of bytes to use for addr (typically 1, 2 for larger
> + *              memories, 0 for register type devices with only one
> + *              register)
> + *   buffer:  Where to read/write the data
> + *   len:     How many bytes to read/write
> + *
> + *   Returns: 0 on success, not 0 on failure
> + */
> +int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
> +{
> +	return i2c_transfer(chip, addr, alen, buffer, len, 0);
> +}
> +
> +/*
> + * Utility routines to read/write registers.
> + */
> +uchar i2c_reg_read(uchar chip, uchar reg)
> +{
> +	uchar buf;
add an empty line
> +	i2c_read(chip, reg, 1, &buf, 1);
> +	return buf;
> +}
add an empty line
> +void i2c_reg_write(uchar chip, uchar reg, uchar val)
> +{
> +	i2c_write(chip, reg, 1, &val, 1);
> +}
> diff --git a/include/configs/bf533-ezkit.h b/include/configs/bf533-ezkit.h
> index 2f551ad..f267301 100644
> --- a/include/configs/bf533-ezkit.h
> +++ b/include/configs/bf533-ezkit.h
> @@ -198,7 +198,7 @@
>  #define I2C_DELAY	udelay(5)	/* 1/4 I2C clock duration */
>  
>  #define CFG_I2C_SPEED		50000
> -#define CFG_I2C_SLAVE		0xFE
> +#define CFG_I2C_SLAVE		0
>  
>  #define CFG_BOOTM_LEN		0x4000000	/* Large Image Length, set to 64 Meg */
>  
> diff --git a/include/configs/bf533-stamp.h b/include/configs/bf533-stamp.h
> index 66a0af6..feadf86 100644
> --- a/include/configs/bf533-stamp.h
> +++ b/include/configs/bf533-stamp.h
> @@ -300,7 +300,7 @@
>  #define I2C_DELAY		udelay(5)	/* 1/4 I2C clock duration */
>  
>  #define CFG_I2C_SPEED		50000
> -#define CFG_I2C_SLAVE		0xFE
> +#define CFG_I2C_SLAVE		0
Could you comment this in the commit please.

Best Regards,
J.