[U-Boot] [PATCH] Blackfin: TWI/I2C: implement multibus support

[Heiko Schocher <hs@denx.de>]

Hello Mike,

Sorry for the late reply ...

Mike Frysinger wrote:
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  drivers/i2c/bfin-twi_i2c.c |  169 ++++++++++++++++++++++++++++----------------
>  1 files changed, 108 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/i2c/bfin-twi_i2c.c b/drivers/i2c/bfin-twi_i2c.c
> index 73a78d2..b3a04d3 100644
> --- a/drivers/i2c/bfin-twi_i2c.c
> +++ b/drivers/i2c/bfin-twi_i2c.c
> @@ -1,7 +1,7 @@
>  /*
>   * i2c.c - driver for Blackfin on-chip TWI/I2C
>   *
> - * Copyright (c) 2006-2008 Analog Devices Inc.
> + * Copyright (c) 2006-2010 Analog Devices Inc.
>   *
>   * Licensed under the GPL-2 or later.
>   */
> @@ -12,6 +12,35 @@
>  #include <asm/blackfin.h>
>  #include <asm/mach-common/bits/twi.h>
>  
> +/* Every register is 32bit aligned, but only 16bits in size */
> +#define ureg(name) u16 name; u16 __pad_##name;
> +struct twi_regs {
> +	ureg(clkdiv);
> +	ureg(control);
> +	ureg(slave_ctl);
> +	ureg(slave_stat);
> +	ureg(slave_addr);
> +	ureg(master_ctl);
> +	ureg(master_stat);
> +	ureg(master_addr);
> +	ureg(int_stat);
> +	ureg(int_mask);
> +	ureg(fifo_ctl);
> +	ureg(fifo_stat);
> +	char __pad[0x50];
> +	ureg(xmt_data8);
> +	ureg(xmt_data16);
> +	ureg(rcv_data8);
> +	ureg(rcv_data16);
> +};
> +#undef ureg

Your commit message says only "implement multibus support"
Please add a comment, that you also introduce the "twi_regs"
struct, thanks.

> +
> +/* U-Boot I2C framework allows only one active device at a time.  */
> +#ifdef TWI_CLKDIV
> +#define TWI0_CLKDIV TWI_CLKDIV
> +#endif
> +static volatile struct twi_regs *twi = (void *)TWI0_CLKDIV;
> +
>  #ifdef DEBUG
>  # define dmemset(s, c, n) memset(s, c, n)
>  #else
> @@ -19,29 +48,10 @@
>  #endif
>  #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(), \
> +		"MSTAT:0x%03x FSTAT:0x%x ISTAT:0x%02x\t%-20s:%-3i: " fmt "\n", \
> +		twi->master_stat, twi->fifo_stat, twi->int_stat, \
>  		__func__, __LINE__, ## args)
>  
> -#ifdef TWI0_CLKDIV
> -#define bfin_write_TWI_CLKDIV(val)           bfin_write_TWI0_CLKDIV(val)
> -#define bfin_read_TWI_CLKDIV(val)            bfin_read_TWI0_CLKDIV(val)
> -#define bfin_write_TWI_CONTROL(val)          bfin_write_TWI0_CONTROL(val)
> -#define bfin_read_TWI_CONTROL(val)           bfin_read_TWI0_CONTROL(val)
> -#define bfin_write_TWI_MASTER_ADDR(val)      bfin_write_TWI0_MASTER_ADDR(val)
> -#define bfin_write_TWI_XMT_DATA8(val)        bfin_write_TWI0_XMT_DATA8(val)
> -#define bfin_read_TWI_RCV_DATA8()            bfin_read_TWI0_RCV_DATA8()
> -#define bfin_read_TWI_INT_STAT()             bfin_read_TWI0_INT_STAT()
> -#define bfin_write_TWI_INT_STAT(val)         bfin_write_TWI0_INT_STAT(val)
> -#define bfin_read_TWI_MASTER_STAT()          bfin_read_TWI0_MASTER_STAT()
> -#define bfin_write_TWI_MASTER_STAT(val)      bfin_write_TWI0_MASTER_STAT(val)
> -#define bfin_read_TWI_MASTER_CTL()           bfin_read_TWI0_MASTER_CTL()
> -#define bfin_write_TWI_MASTER_CTL(val)       bfin_write_TWI0_MASTER_CTL(val)
> -#define bfin_write_TWI_INT_MASK(val)         bfin_write_TWI0_INT_MASK(val)
> -#define bfin_write_TWI_FIFO_CTL(val)         bfin_write_TWI0_FIFO_CTL(val)
> -#endif
> -
>  #ifdef CONFIG_TWICLK_KHZ
>  # error do not define CONFIG_TWICLK_KHZ ... use CONFIG_SYS_I2C_SPEED
>  #endif
> @@ -87,49 +97,48 @@ static int wait_for_completion(struct i2c_msg *msg)
>  	ulong timebase = get_timer(0);
>  
>  	do {
> -		int_stat = bfin_read_TWI_INT_STAT();
> +		int_stat = twi->int_stat;

bfin_read/write_TWI_* macros uses bfin_read/write16 ... your patch
don;t longer use this I/O accessors. Is this OK?

Otherwise, your patch looks good.

bye
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany