[U-Boot] [PATCH 1/2] I2C: add Marvell TWSI simple driver

[Heiko Schocher]

Hello Albert,

Albert Aribaud wrote:
> This driver is for the Marvell TWSI/I2C module found in
> the orion and kirkwood families among others.
> 
> Signed-off-by: Albert Aribaud <albert.aribaud at free.fr>
> ---
> While the 'kirkwood_i2c' driver for the Marvell TWSI module
> is already available in u-boot, this one is 25% smaller, less
> complex (no state machine) and much faster (i2c probe on an
> ED Mini V2 takes no noticeable time vs. half a second).

Great!

As the kirkwood_i2c driver is actually not used in any board,
we should remove it completely, can you add this for v2?

Beside of that, I have just some minor codstyling comments:

>  drivers/i2c/Makefile |    1 +
>  drivers/i2c/mvtwsi.c |  419 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 420 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/i2c/mvtwsi.c
> 
> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> index d2c2515..73c415d 100644
> --- a/drivers/i2c/Makefile
> +++ b/drivers/i2c/Makefile
> @@ -30,6 +30,7 @@ COBJS-$(CONFIG_DRIVER_DAVINCI_I2C) += davinci_i2c.o
>  COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
>  COBJS-$(CONFIG_I2C_KIRKWOOD) += kirkwood_i2c.o
>  COBJS-$(CONFIG_I2C_MXC) += mxc_i2c.o
> +COBJS-$(CONFIG_I2C_DRIVER_MVTWSI) += mvtwsi.o
>  COBJS-$(CONFIG_DRIVER_OMAP1510_I2C) += omap1510_i2c.o
>  COBJS-$(CONFIG_DRIVER_OMAP24XX_I2C) += omap24xx_i2c.o
>  COBJS-$(CONFIG_DRIVER_OMAP34XX_I2C) += omap24xx_i2c.o
> diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
> new file mode 100644
> index 0000000..b591328
> --- /dev/null
> +++ b/drivers/i2c/mvtwsi.c
> @@ -0,0 +1,419 @@
> +/*
> + * Driver for the TWSI (i2c) controller on the Marvell orion5x
> + *
> + * Author: Albert Aribaud <albert.aribaud at free.fr>
> + * 2005 (c) MontaVista, Software, Inc.
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> + 
> +#include <common.h>
> +#include <i2c.h>
> +#include <asm/errno.h>
> +#include <asm/io.h>
> +
> +/*
> + * include a file that will provide CONFIG_I2C_MVTWSI_BASE
> + * and possibly other settings
> + */
> +
> +#if defined(CONFIG_ORION5X)
> +#include <asm/arch/orion5x.h>
> +#else
> +#error Driver mvtwsi not supported by SoC or board
> +#endif
> +
> +/*
> + * TWSI register structure
> + */
> +
> +struct  mvtwsi_registers {
> +	u32 slave_address;
> +	u32 data;
> +	u32 control;
> +	union {
> +		u32 status;	/* when reading */
> +		u32 baudrate;	/* when writing */
> +	};
> +	u32 extended_slave_address;
> +	u32 reserved[2];
> +	u32 soft_reset;
> +};
> +
> +/*
> + * Control register fields
> + */
> +
> +#define	MVTWSI_CONTROL_ACK	0x00000004
> +#define	MVTWSI_CONTROL_IFLG	0x00000008
> +#define	MVTWSI_CONTROL_STOP	0x00000010
> +#define	MVTWSI_CONTROL_START	0x00000020
> +#define	MVTWSI_CONTROL_TWSIEN	0x00000040
> +#define	MVTWSI_CONTROL_INTEN	0x00000080
> +
> +/*
> + * Status register values -- only those expected in normal master
> + * operation on non-10-bit-address devices; whatever status we don't
> + * expect in nominal conditions (bus errors, arbitration losses,
> + * missing ACKs...) we just pass back to the caller as an error
> + * code.
> + */
> +
> +#define	MVTWSI_STATUS_START		0x08
> +#define	MVTWSI_STATUS_REPEATED_START	0x10
> +#define	MVTWSI_STATUS_ADDR_W_ACK	0x18
> +#define	MVTWSI_STATUS_DATA_W_ACK	0x28
> +#define	MVTWSI_STATUS_ADDR_R_ACK	0x40
> +#define	MVTWSI_STATUS_ADDR_R_NAK	0x48
> +#define	MVTWSI_STATUS_DATA_R_ACK	0x50
> +#define	MVTWSI_STATUS_DATA_R_NAK	0x58 /* our NAK, not the slave's */

line too long.

> +#define	MVTWSI_STATUS_IDLE		0xF8
> +
> +/*
> + * The single instance of the controller we'll be dealing with
> + */
> +
> +static struct  mvtwsi_registers *twsi =
> +	(struct  mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE;
> +
> +/*
> + * Returned statuses are 0 for success and nonzero otherwise.
> + * Currently, cmd_i2c and cmd_eeprom do not interpret an error status.
> + * Thus to ease debugging, the return status contains some debug info:
> + * - bits 31..24 are an error class: 0x01 is timeout, 0x02 is 'status mismatch'.
> + * - bits 23..16 are the last value of the control register.
> + * - bits 15..8 are the last value of the status register.
> + * - bits 7..0 are the expected value of the status register.
> + */
> +
> +#define MVTWSI_ERROR_WRONG_STATUS	0x01
> +#define MVTWSI_ERROR_TIMEOUT		0x02
> +
> +#define MVTWSI_ERROR(ec,lc,ls,es) ( ( (ec << 24) & 0xFF000000) | \
> +	( (lc << 16) & 0x00FF0000) | ( (ls<<8) & 0x0000FF00) | (es & 0xFF) )
> +
> +/*
> + * Wait for IFLG to raise, or return 'timeout'; then if status is as expected,
> + * return 0 (ok) or return 'wrong status'.
> + */
> +

blank line not necessary.

> +static int twsi_wait(int expected_status)
> +{
> +	int control,status;
> +	int timeout = 1000;

please insert a blank line, check this on other places too.

> +	do {
> +		control = readl(&twsi->control);
> +		if (control & MVTWSI_CONTROL_IFLG) {
> +			status = readl(&twsi->status);
> +			if (status==expected_status)

please insert a space between "=="

> +				return 0;
> +			else
> +				return MVTWSI_ERROR(MVTWSI_ERROR_WRONG_STATUS,control, status,

line too long, also please insert a space after a ","
Please check this for the whole file.

> +					expected_status);
> +		}
> +		udelay(10);
> +	} while (timeout--);
> +	status = readl(&twsi->status);
> +	return MVTWSI_ERROR(MVTWSI_ERROR_TIMEOUT,control,status,expected_status);
> +}
> +
> +/*
> + * These flags are ORed to any write to the control register
> + * They allow global setting of TWSIEN and ACK.
> + * By default none are set.
> + * twsi_start() sets TWSIEN (in case the controller was disabled)
> + * twsi_recv() sets ACK or resets it depending on expected status.
> + */
> +

blank line not necessary. Please check the whole file

> +static u8 twsi_control_flags = MVTWSI_CONTROL_TWSIEN;
> +
> +/*
> + * Assert the START condition, either in a single I2C transaction
> + * or inside back-to-back ones (repeated starts).
> + */
> +
> +static int twsi_start(int expected_status)
> +{
> +	/* globally set TWSIEN in case it was not */
> +	twsi_control_flags |= MVTWSI_CONTROL_TWSIEN;
> +	/* assert START */
> +	writel(twsi_control_flags | MVTWSI_CONTROL_START, &twsi->control);
> +	/* wait for controller to process START */
> +	return twsi_wait(expected_status);
> +}
> +
> +/*
> + * Send a byte (i2c address or data).
> + */
> +
> +static int twsi_send(u8 byte, int expected_status)
> +{
> +	/* put byte in data register for sending */
> +	writel(byte, &twsi->data);
> +	/* clear any pending interrupt -- that'll cause sending */
> +	writel(twsi_control_flags, &twsi->control);
> +	/* wait for controller to receive byte and check ACK */
> +	return twsi_wait(expected_status);
> +}
> +
> +/*
> + * Receive a byte.
> + * Global mvtwsi_control_flags variable says if we should ack or nak.
> + */
> +
> +static int twsi_recv(u8 *byte)
> +{
> +	int expected_status, status;
> +	/* compute expected status based on ACK bit in global control flags */
> +	if (twsi_control_flags & MVTWSI_CONTROL_ACK)
> +		expected_status = MVTWSI_STATUS_DATA_R_ACK;
> +	else
> +		expected_status = MVTWSI_STATUS_DATA_R_NAK;
> +	/* acknowledge *previous state* and launch receive */
> +	writel(twsi_control_flags, &twsi->control);
> +	/* wait for controller to receive byte and assert ACK or NAK */
> +	status = twsi_wait(expected_status);
> +	/* if we did receive expected byte then store it */
> +	if (status==0)
> +		*byte = readl(&twsi->data);
> +	/* return status */

comment not necessary.

> +	return status;
> +}
> +
> +/*
> + * Assert the STOP condition.
> + * This is also used to force the bus back in idle (SDA=SCL=1).
> + */
> +
> +static int twsi_stop(void)
> +{
> +	int control,status;
> +	int timeout = 1000;
> +	/* assert STOP */
> +	control = MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_STOP;
> +	writel(control, &twsi->control);
> +	/* wait for idle condition. There won't be an IFLG so we cannot use twsi_wait(). */
> +	do {
> +		status = readl(&twsi->status);
> +		if (status==MVTWSI_STATUS_IDLE)
> +			return 0;
> +		udelay(10);
> +	} while (timeout--);
> +	control = readl(&twsi->control);
> +	return MVTWSI_ERROR(MVTWSI_ERROR_TIMEOUT,control,status,MVTWSI_STATUS_IDLE);
> +}
> +
> +/*
> + * Ugly formula to convert m and n values to a frequency comes from
> + * TWSI specifications
> + */
> +
> +#define TWSI_FREQUENCY(m,n) \
> +	( (u8) (CONFIG_SYS_TCLK / (10 * (m + 1) * 2 * (1 << n))) )
> +
> +/*
> + * These are required to be reprogrammed before enabling the controller
> + * because a reset loses them.
> + * Default values come from the spec, but a twsi_reset will change them.
> + */
> +
> +static u8 twsi_baud_rate = 0x44;
> +static u8 twsi_actual_speed = TWSI_FREQUENCY(4,4);
> +static u8 twsi_slave_address = 0;
> +
> +/*
> + * Reset controller.
> + * Called at end of i2c_init unsuccessful i2c transactions.
> + * Controller reset also resets the baud rate and slave address, so
> + * re-establish them.
> + */
> +
> +static void twsi_reset(void)
> +{
> +	/* ensure controller will be enabled by any twsi*() function */
> +	twsi_control_flags = MVTWSI_CONTROL_TWSIEN;
> +	/* reset controller */
> +	writel(0, &twsi->soft_reset);
> +	/* wait 2 ms -- this is what the Marvell LSP does */
> +	udelay(20000);
> +	/* set baud rate */
> +	writel(twsi_baud_rate, &twsi->baudrate);
> +	/* set slave address even though we don't use it */
> +	writel(twsi_slave_address, &twsi->slave_address);
> +	writel(0, &twsi->extended_slave_address);
> +	/* assert STOP but don't care for the result */
> +	(void) twsi_stop();
> +}
> +
> +/*
> + * I2C init called by cmd_i2c when doing 'i2c reset'.
> + * Sets baud to the highest possible value not exceeding requested one.
> + */
> +
> +void i2c_init(int requested_speed, int slaveadd)
> +{
> +	int	tmp_speed, highest_speed, n, m;
> +	int	baud = 0x44; /* value at controller reset */
> +	/* use actual speed to collect progressively higher values */
> +	highest_speed = 0;
> +	/* compute m,n setting for highest speed not above requested speed */
> +	for (n = 0; n < 8; n++) {
> +		for (m = 0; m < 16; m++) {
> +			tmp_speed = TWSI_FREQUENCY(m,n);
> +			if (tmp_speed <= requested_speed)
> +			if (tmp_speed > highest_speed) {
> +				highest_speed = tmp_speed;
> +				baud = (m << 3) | n;
> +			}
> +		}
> +	}
> +	/* save baud rate and slave for later calls to twsi_reset */
> +	twsi_baud_rate = baud;
> +	twsi_actual_speed = highest_speed;
> +	twsi_slave_address = slaveadd;
> +	/* reset controller */
> +	twsi_reset();
> +}
> +
> +/*
> + * Begin I2C transaction with expected start status, at given address.
> + * Common to i2c_probe, i2c_read and i2c_write.
> + * Expected address status will derive from direction bit (bit 0) in addr.
> + */
> +
> +static int i2c_begin(int expected_start_status, u8 addr)
> +{
> +	int status, expected_addr_status;
> +	/* compute expected address status from direction bit in addr */
> +	if (addr & 1) /* reading */
> +		expected_addr_status = MVTWSI_STATUS_ADDR_R_ACK;
> +	else /* writing */
> +		expected_addr_status = MVTWSI_STATUS_ADDR_W_ACK;
> +	/* assert START */
> +	status = twsi_start(expected_start_status);
> +	/* send out the address if the start went well */
> +	if (status == 0)
> +		status = twsi_send(addr, expected_addr_status);
> +	/* return ok or status of first failure to caller */
> +	return status;
> +}
> +
> +/*
> + * End I2C transaction, of which the current status is given.
> + * Common to i2c_probe, i2c_read and i2c_write.
> + */
> +
> +static int i2c_end(int status)
> +{
> +	/* if transaction went well so far, try to assert a STOP */
> +	/*if (status == 0)*/

Don;t add dead code.

> +		twsi_stop();
> +	/* if anything went wrong (including the STOP we tried), do a reset */
> +	/*else
> +		twsi_reset();*/

here too.

> +	/* return ok or status of first failure to caller */
> +	return status;
> +}
> +
> +/*
> + * I2C probe called by cmd_i2c when doing 'i2c probe'.
> + * Begin read, nak data byte, end.
> + */
> +
> +int i2c_probe(uchar chip)
> +{
> +	u8 dummy_byte;
> +	int status;
> +	/* begin i2c read */
> +	status = i2c_begin(MVTWSI_STATUS_START, (chip << 1) | 1 );
> +	/* dummy read was accepted: receive byte but NAK it. */
> +	if (status==0)
> +		status = twsi_recv( &dummy_byte);
> +	/* We expected either 0 (ok) or MVTWSI_STATUS_ADDR_R_NAK */
> +	return i2c_end( (status==MVTWSI_STATUS_ADDR_R_NAK) ? 0: status);
> +}
> +
> +/*
> + * I2C read called by cmd_i2c when doing 'i2c read' and by cmd_eeprom.c
> + * Begin write, send address byte(s), begin read, receive data bytes, end.
> + */
> +
> +int i2c_read(u8 dev, uint addr, int alen, u8 *data, int length)
> +{
> +	int status;
> +	/* begin i2c write to send the address bytes */
> +	status = i2c_begin(MVTWSI_STATUS_START, (dev << 1) );
> +	/* send addr bytes */
> +	while ( (status==0) && alen--)
> +		status = twsi_send(addr >> (8*alen), MVTWSI_STATUS_DATA_W_ACK);
> +	/* NOTE: should we send a stop before a start? That's eeprom-level stuff */
> +	/* begin i2c read to receive eeprom data bytes */
> +	if (status == 0)
> +		status = i2c_begin(MVTWSI_STATUS_REPEATED_START, (dev << 1) | 1);
> +	/* prepare ACK if at least one byte must be received */
> +	if (length > 0)
> +		twsi_control_flags |= MVTWSI_CONTROL_ACK;
> +	/* now receive actual bytes */
> +	while ( (status==0) && length-- ) {
> +		/* reset NAK if we if no more to read now */
> +		if (length == 0)
> +			twsi_control_flags &= ~MVTWSI_CONTROL_ACK;
> +		/* read current byte */
> +		status = twsi_recv(data++);
> +	}
> +	/* end i2c transaction */
> +	return i2c_end(status);
> +}
> +
> +/*
> + * I2C write called by cmd_i2c when doing 'i2c write' and by cmd_eeprom.c
> + * Begin write, send address byte(s), send data bytes, end.
> + */
> +
> +int i2c_write(u8 dev, uint addr, int alen, u8 *data, int length)
> +{
> +	int status;
> +	/* begin i2c write to send the eeprom adress bytes then data bytes */
> +	status = i2c_begin(MVTWSI_STATUS_START, (dev << 1) );
> +	/* send addr bytes */
> +	while ( (status==0) && alen--)
> +		status = twsi_send(addr >> (8*alen), MVTWSI_STATUS_DATA_W_ACK);
> +	/* send data bytes */
> +	while ( (status==0) && (length-->0) )
> +		status = twsi_send(*(data++), MVTWSI_STATUS_DATA_W_ACK);
> +	/* end i2c transaction */
> +	return i2c_end(status);
> +}
> +
> +/*
> + * Bus set/get routines: we only support bus 0.
> + */
> +
> +int i2c_set_bus_num(unsigned int bus)
> +{
> +	if (bus > 0) {
> +		return -1;
> +	}
> +	return 0;
> +}
> +
> +unsigned int i2c_get_bus_num(void)
> +{
> +	return 0;
> +}