[U-Boot] [PATCH] blackfin: rename blackfin i2c driver to adi

Heiko Schocher hs at denx.de
Fri Oct 31 07:29:41 CET 2014


Hello Scott,

Am 01.11.2014 07:08, schrieb Scott Jiang:

something is wrong with your clock ... patch is not yet
in patchwork ...

> This driver is not only used on blackfin. So it's better to
> rename it and access i2c registers by standard io functions.
> Fix coding style as well.
>
> Signed-off-by: Scott Jiang<scott.jiang.linux at gmail.com>
> ---
>   drivers/i2c/Makefile                      |    2 +-
>   drivers/i2c/{bfin-twi_i2c.c =>  adi_i2c.c} |  154 +++++++++++++++--------------
>   include/configs/bct-brettl2.h             |    2 +-
>   include/configs/bf518f-ezbrd.h            |    2 +-
>   include/configs/bf526-ezbrd.h             |    2 +-
>   include/configs/bf527-ad7160-eval.h       |    2 +-
>   include/configs/bf527-ezkit.h             |    2 +-
>   include/configs/bf527-sdp.h               |    2 +-
>   include/configs/bf537-minotaur.h          |    2 +-
>   include/configs/bf537-pnav.h              |    2 +-
>   include/configs/bf537-srv1.h              |    2 +-
>   include/configs/bf537-stamp.h             |    2 +-
>   include/configs/bf538f-ezkit.h            |    2 +-
>   include/configs/bf548-ezkit.h             |    2 +-
>   include/configs/bf609-ezkit.h             |    2 +-
>   include/configs/br4.h                     |    2 +-
>   include/configs/cm-bf527.h                |    2 +-
>   include/configs/cm-bf537e.h               |    2 +-
>   include/configs/cm-bf537u.h               |    2 +-
>   include/configs/cm-bf548.h                |    2 +-
>   include/configs/pr1.h                     |    2 +-
>   include/configs/tcm-bf518.h               |    2 +-
>   include/configs/tcm-bf537.h               |    2 +-
>   23 files changed, 103 insertions(+), 95 deletions(-)
>   rename drivers/i2c/{bfin-twi_i2c.c =>  adi_i2c.c} (72%)

Thanks for this work ... some "nitpicks" ...

> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> index d067897..b7f0098 100644
> --- a/drivers/i2c/Makefile
> +++ b/drivers/i2c/Makefile
> @@ -5,7 +5,7 @@
>   # SPDX-License-Identifier:	GPL-2.0+
>   #
>
> -obj-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o
> +obj-$(CONFIG_ADI_I2C) += adi_i2c.o

please use something like "CONFIG_SYS_I2C_ADI" and please
convert this driver to CONFIG_SYS_I2C framework ...

>   obj-$(CONFIG_I2C_MV) += mv_i2c.o
>   obj-$(CONFIG_PCA9564_I2C) += pca9564_i2c.o
>   obj-$(CONFIG_TSI108_I2C) += tsi108_i2c.o
> diff --git a/drivers/i2c/bfin-twi_i2c.c b/drivers/i2c/adi_i2c.c
> similarity index 72%
> rename from drivers/i2c/bfin-twi_i2c.c
> rename to drivers/i2c/adi_i2c.c
> index cfab064..c971f03 100644
> --- a/drivers/i2c/bfin-twi_i2c.c
> +++ b/drivers/i2c/adi_i2c.c
> @@ -1,7 +1,7 @@
>   /*
> - * i2c.c - driver for Blackfin on-chip TWI/I2C
> + * i2c.c - driver for ADI TWI/I2C
>    *
> - * Copyright (c) 2006-2010 Analog Devices Inc.
> + * Copyright (c) 2006-2014 Analog Devices Inc.
>    *
>    * Licensed under the GPL-2 or later.
>    */
> @@ -9,9 +9,9 @@
>   #include<common.h>
>   #include<i2c.h>
>
> -#include<asm/blackfin.h>
>   #include<asm/clock.h>
> -#include<asm/mach-common/bits/twi.h>
> +#include<asm/twi.h>
> +#include<asm/io.h>
>
>   /* Every register is 32bit aligned, but only 16bits in size */
>   #define ureg(name) u16 name; u16 __pad_##name;
> @@ -40,7 +40,7 @@ struct twi_regs {
>   #ifdef TWI_CLKDIV
>   #define TWI0_CLKDIV TWI_CLKDIV
>   #endif
> -static volatile struct twi_regs *twi = (void *)TWI0_CLKDIV;
> +static struct twi_regs *twi = (void *)TWI0_CLKDIV;
>
>   #ifdef DEBUG
>   # define dmemset(s, c, n) memset(s, c, n)
> @@ -94,53 +94,54 @@ struct i2c_msg {
>    */
>   static int wait_for_completion(struct i2c_msg *msg)
>   {
> -	uint16_t int_stat;
> +	u16 int_stat, ctl;

This change is not related to the subject of the patch,
please split this into seperate patches ...

>   	ulong timebase = get_timer(0);
>
>   	do {
> -		int_stat = twi->int_stat;
> +		int_stat = readw(&twi->int_stat);

Here too... please fix globally.

>
>   		if (int_stat&  XMTSERV) {
>   			debugi("processing XMTSERV");
> -			twi->int_stat = XMTSERV;
> -			SSYNC();
> +			writew(XMTSERV,&twi->int_stat);

Here too...

>   			if (msg->alen) {
> -				twi->xmt_data8 = *(msg->abuf++);
> +				writew(*(msg->abuf++),&twi->xmt_data8);
>   				--msg->alen;
>   			} else if (!(msg->flags&  I2C_M_COMBO)&&  msg->len) {
> -				twi->xmt_data8 = *(msg->buf++);
> +				writew(*(msg->buf++),&twi->xmt_data8);
>   				--msg->len;
>   			} else {
> -				twi->master_ctl |= (msg->flags&  I2C_M_COMBO) ? RSTART | MDIR : STOP;
> -				SSYNC();
> +				ctl = readw(&twi->master_ctl);
> +				if (msg->flags&  I2C_M_COMBO)
> +					writew(ctl | RSTART | MDIR,
> +							&twi->master_ctl);
> +				else
> +					writew(ctl | STOP,&twi->master_ctl);
>   			}
>   		}
>   		if (int_stat&  RCVSERV) {
>   			debugi("processing RCVSERV");
> -			twi->int_stat = RCVSERV;
> -			SSYNC();
> +			writew(RCVSERV,&twi->int_stat);
>   			if (msg->len) {
> -				*(msg->buf++) = twi->rcv_data8;
> +				*(msg->buf++) = readw(&twi->rcv_data8);
>   				--msg->len;
>   			} else if (msg->flags&  I2C_M_STOP) {
> -				twi->master_ctl |= STOP;
> -				SSYNC();
> +				ctl = readw(&twi->master_ctl);
> +				writew(ctl | STOP,&twi->master_ctl);
>   			}
>   		}
>   		if (int_stat&  MERR) {
>   			debugi("processing MERR");
> -			twi->int_stat = MERR;
> -			SSYNC();
> +			writew(MERR,&twi->int_stat);
>   			return msg->len;
>   		}
>   		if (int_stat&  MCOMP) {
>   			debugi("processing MCOMP");
> -			twi->int_stat = MCOMP;
> -			SSYNC();
> +			writew(MCOMP,&twi->int_stat);
>   			if (msg->flags&  I2C_M_COMBO&&  msg->len) {
> -				twi->master_ctl = (twi->master_ctl&  ~RSTART) |
> +				ctl = readw(&twi->master_ctl);
> +				ctl = (ctl&  ~RSTART) |
>   					(min(msg->len, 0xff)<<  6) | MEN | MDIR;
> -				SSYNC();
> +				writew(ctl,&twi->master_ctl);
>   			} else
>   				break;
>   		}
> @@ -161,8 +162,11 @@ static int wait_for_completion(struct i2c_msg *msg)
>    *	Here we just get the i2c stuff all prepped and ready, and then tail off
>    *	into wait_for_completion() for all the bits to go.
>    */
> -static int i2c_transfer(uchar chip, uint addr, int alen, uchar *buffer, int len, u8 flags)
> +static int i2c_transfer(uchar chip, uint addr, int alen, uchar *buffer,
> +			int len, u8 flags)
>   {
> +	int ret;
> +	u16 ctl;
>   	uchar addr_buffer[] = {
>   		(addr>>   0),
>   		(addr>>   8),
> @@ -175,62 +179,59 @@ static int i2c_transfer(uchar chip, uint addr, int alen, uchar *buffer, int len,
>   		.abuf  = addr_buffer,
>   		.alen  = alen,
>   	};
> -	int ret;
>
>   	dmemset(buffer, 0xff, len);
> -	debugi("chip=0x%x addr=0x%02x alen=%i buf[0]=0x%02x len=%i flags=0x%02x[%s] ",
> -		chip, addr, alen, buffer[0], len, flags, (flags&  I2C_M_READ ? "rd" : "wr"));
> +	debugi("chip=0x%x addr=0x%02x alen=%i buf[0]=0x%02x len=%i ",
> +		chip, addr, alen, buffer[0], len);
> +	debugi("flags=0x%02x[%s] ", flags,
> +		(flags&  I2C_M_READ ? "rd" : "wr"));
>
>   	/* wait for things to settle */
> -	while (twi->master_stat&  BUSBUSY)
> +	while (readw(&twi->master_stat)&  BUSBUSY)
>   		if (ctrlc())
>   			return 1;
>
>   	/* Set Transmit device address */
> -	twi->master_addr = chip;
> +	writew(chip,&twi->master_addr);
>
>   	/* Clear the FIFO before starting things */
> -	twi->fifo_ctl = XMTFLUSH | RCVFLUSH;
> -	SSYNC();
> -	twi->fifo_ctl = 0;
> -	SSYNC();
> +	writew(XMTFLUSH | RCVFLUSH,&twi->fifo_ctl);
> +	writew(0,&twi->fifo_ctl);
>
>   	/* prime the pump */
>   	if (msg.alen) {
>   		len = (msg.flags&  I2C_M_COMBO) ? msg.alen : msg.alen + len;
>   		debugi("first byte=0x%02x", *msg.abuf);
> -		twi->xmt_data8 = *(msg.abuf++);
> +		writew(*(msg.abuf++),&twi->xmt_data8);
>   		--msg.alen;
>   	} else if (!(msg.flags&  I2C_M_READ)&&  msg.len) {
>   		debugi("first byte=0x%02x", *msg.buf);
> -		twi->xmt_data8 = *(msg.buf++);
> +		writew(*(msg.buf++),&twi->xmt_data8);
>   		--msg.len;
>   	}
>
>   	/* clear int stat */
> -	twi->master_stat = -1;
> -	twi->int_stat = -1;
> -	twi->int_mask = 0;
> -	SSYNC();
> +	writew(-1,&twi->master_stat);
> +	writew(-1,&twi->int_stat);
> +	writew(0,&twi->int_mask);
>
>   	/* Master enable */
> -	twi->master_ctl =
> -			(twi->master_ctl&  FAST) |
> -			(min(len, 0xff)<<  6) | MEN |
> -			((msg.flags&  I2C_M_READ) ? MDIR : 0);
> -	SSYNC();
> -	debugi("CTL=0x%04x", twi->master_ctl);
> +	ctl = readw(&twi->master_ctl);
> +	ctl = (ctl&  FAST) | (min(len, 0xff)<<  6) | MEN |
> +		((msg.flags&  I2C_M_READ) ? MDIR : 0);
> +	writew(ctl,&twi->master_ctl);
>
>   	/* process the rest */
>   	ret = wait_for_completion(&msg);
>   	debugi("ret=%d", ret);
>
>   	if (ret) {
> -		twi->master_ctl&= ~MEN;
> -		twi->control&= ~TWI_ENA;
> -		SSYNC();
> -		twi->control |= TWI_ENA;
> -		SSYNC();
> +		ctl = readw(&twi->master_ctl)&  ~MEN;
> +		writew(ctl,&twi->master_ctl);
> +		ctl = readw(&twi->control)&  ~TWI_ENA;
> +		writew(ctl,&twi->control);
> +		ctl = readw(&twi->control) | TWI_ENA;
> +		writew(ctl,&twi->control);
>   	}
>
>   	return ret;
> @@ -247,10 +248,11 @@ int i2c_set_bus_speed(unsigned int speed)
>   	/* Set TWI interface clock */
>   	if (clkdiv<  I2C_DUTY_MAX || clkdiv>  I2C_DUTY_MIN)
>   		return -1;
> -	twi->clkdiv = (clkdiv<<  8) | (clkdiv&  0xff);
> +	clkdiv = (clkdiv<<  8) | (clkdiv&  0xff);
> +	writew(clkdiv,&twi->clkdiv);
>
>   	/* Don't turn it on */
> -	twi->master_ctl = (speed>  100000 ? FAST : 0);
> +	writew(speed>  100000 ? FAST : 0,&twi->master_ctl);
>
>   	return 0;
>   }
> @@ -261,8 +263,9 @@ int i2c_set_bus_speed(unsigned int speed)
>    */
>   unsigned int i2c_get_bus_speed(void)
>   {
> +	u16 clkdiv = readw(&twi->clkdiv)&  0xff;
>   	/* 10 MHz / (2 * CLKDIV) ->  5 MHz / CLKDIV */
> -	return 5000000 / (twi->clkdiv&  0xff);
> +	return 5000000 / clkdiv;
>   }
>
>   /**
> @@ -275,27 +278,22 @@ unsigned int i2c_get_bus_speed(void)
>    */
>   void i2c_init(int speed, int slaveaddr)
>   {
> -	uint8_t prescale = ((get_i2c_clk() / 1000 / 1000 + 5) / 10)&  0x7F;
> +	u16 prescale = ((get_i2c_clk() / 1000 / 1000 + 5) / 10)&  0x7F;
>
>   	/* Set TWI internal clock as 10MHz */
> -	twi->control = prescale;
> +	writew(prescale,&twi->control);
>
>   	/* Set TWI interface clock as specified */
>   	i2c_set_bus_speed(speed);
>
>   	/* Enable it */
> -	twi->control = TWI_ENA | prescale;
> -	SSYNC();
> +	writew(TWI_ENA | prescale,&twi->control);
>
> -	debugi("CONTROL:0x%04x CLKDIV:0x%04x", twi->control, twi->clkdiv);
> +	debugi("CONTROL:0x%04x CLKDIV:0x%04x", readw(&twi->control),
> +		readw(&twi->clkdiv));
>
>   #if CONFIG_SYS_I2C_SLAVE
>   # error I2C slave support not tested/supported
> -	/* If they want us as a slave, do it */
> -	if (slaveaddr) {
> -		twi->slave_addr = slaveaddr;
> -		twi->slave_ctl = SEN;
> -	}
>   #endif
>   }
>
> @@ -321,7 +319,8 @@ int i2c_probe(uchar chip)
>    */
>   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));
> +	return i2c_transfer(chip, addr, alen, buffer,
> +			len, (alen ? I2C_M_COMBO : I2C_M_READ));
>   }
>
>   /**
> @@ -347,15 +346,21 @@ int i2c_set_bus_num(unsigned int bus)
>   {
>   	switch (bus) {
>   #if CONFIG_SYS_MAX_I2C_BUS>  0
> -		case 0: twi = (void *)TWI0_CLKDIV; return 0;
> +	case 0:
> +		twi = (void *)TWI0_CLKDIV;
> +		return 0;
>   #endif
>   #if CONFIG_SYS_MAX_I2C_BUS>  1
> -		case 1: twi = (void *)TWI1_CLKDIV; return 0;
> +	case 1:
> +		twi = (void *)TWI1_CLKDIV;
> +		return 0;
>   #endif
>   #if CONFIG_SYS_MAX_I2C_BUS>  2
> -		case 2: twi = (void *)TWI2_CLKDIV; return 0;
> +	case 2:
> +		twi = (void *)TWI2_CLKDIV;
> +		return 0;
>   #endif
> -		default: return -1;
> +	default: return -1;
>   	}
>   }
>
> @@ -366,14 +371,17 @@ unsigned int i2c_get_bus_num(void)
>   {
>   	switch ((unsigned long)twi) {
>   #if CONFIG_SYS_MAX_I2C_BUS>  0
> -		case TWI0_CLKDIV: return 0;
> +	case TWI0_CLKDIV:
> +		return 0;
>   #endif
>   #if CONFIG_SYS_MAX_I2C_BUS>  1
> -		case TWI1_CLKDIV: return 1;
> +	case TWI1_CLKDIV:
> +		return 1;
>   #endif
>   #if CONFIG_SYS_MAX_I2C_BUS>  2
> -		case TWI2_CLKDIV: return 2;
> +	case TWI2_CLKDIV:
> +		return 2;
>   #endif
> -		default: return -1;
> +	default: return -1;
>   	}
>   }
> diff --git a/include/configs/bct-brettl2.h b/include/configs/bct-brettl2.h
> index d0828d5..25f3e11 100644
> --- a/include/configs/bct-brettl2.h
> +++ b/include/configs/bct-brettl2.h
> @@ -122,7 +122,7 @@
>   /*
>    * I2C Settings
>    */
> -#define CONFIG_BFIN_TWI_I2C	1
> +#define CONFIG_ADI_I2C		1
>   #define CONFIG_HARD_I2C		1

Please get rid of HARD_I2C ... thanks!

[...]

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


More information about the U-Boot mailing list