[U-Boot] [PATCH 1/2] I2C: add Marvell TWSI simple driver
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@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).
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@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 */ +#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'. + */ + +static int twsi_wait(int expected_status) +{ + int control,status; + int timeout = 1000; + do { + control = readl(&twsi->control); + if (control & MVTWSI_CONTROL_IFLG) { + status = readl(&twsi->status); + if (status==expected_status) + return 0; + else + return MVTWSI_ERROR(MVTWSI_ERROR_WRONG_STATUS,control, status, + 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. + */ + +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 */ + 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)*/ + twsi_stop(); + /* if anything went wrong (including the STOP we tried), do a reset */ + /*else + twsi_reset();*/ + /* 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; +}
Signed-off-by: Albert Aribaud albert.aribaud@free.fr --- include/configs/edminiv2.h | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/include/configs/edminiv2.h b/include/configs/edminiv2.h index 57dd165..36ed392 100644 --- a/include/configs/edminiv2.h +++ b/include/configs/edminiv2.h @@ -182,6 +182,15 @@ #endif /* CMD_IDE */
/* + * I2C related stuff + */ +#define CONFIG_I2C_DRIVER_MVTWSI +#define CONFIG_I2C_MVTWSI_BASE ORION5X_TWSI_BASE +#define CONFIG_SYS_I2C_SLAVE 0x0 +#define CONFIG_SYS_I2C_SPEED 100000 +#define CONFIG_CMD_I2C + +/* * Environment variables configurations */ #define CONFIG_ENV_IS_IN_FLASH 1
Reviewing myself, as a blatant copy-paste mistake only hit my eyes right after the patch was posted. :(
Le 25/08/2010 16:23, Albert Aribaud a écrit :
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
@@ -0,0 +1,419 @@ +/*
- Driver for the TWSI (i2c) controller on the Marvell orion5x
- Author: Albert Aribaudalbert.aribaud@free.fr
- 2005 (c) MontaVista, Software, Inc.
This code is not (c) Montavista originally; it is a completely new implementation, with the license notice copied from the kirkwood file. I'll remove this line in patch V2.
Amicalement,
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Albert Aribaud Sent: Wednesday, August 25, 2010 7:54 PM To: u-boot@lists.denx.de Subject: [U-Boot] [PATCH 1/2] I2C: add Marvell TWSI simple driver
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@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).
Hi Albert This will be very good enhancement indeed.
drivers/i2c/Makefile | 1 + drivers/i2c/mvtwsi.c | 419
Can you pls follow the same strategy as we followed for mvgbe, mvsata? Please rename and enhance current kirkwood_i2c driver support, and then add support for Orion followed by board support for edminiv2
Regards.. Prafulla ..
(adding Heiko, custodian of I2C/EEPROM and committer of kirkwood_i2c)
Le 26/08/2010 06:33, Prafulla Wadaskar a écrit :
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Albert Aribaud Sent: Wednesday, August 25, 2010 7:54 PM To: u-boot@lists.denx.de Subject: [U-Boot] [PATCH 1/2] I2C: add Marvell TWSI simple driver
This driver is for the Marvell TWSI/I2C module found in the orion and kirkwood families among others.
Signed-off-by: Albert Aribaudalbert.aribaud@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).
Hi Albert This will be very good enhancement indeed.
drivers/i2c/Makefile | 1 + drivers/i2c/mvtwsi.c | 419
Can you pls follow the same strategy as we followed for mvgbe, mvsata? Please rename and enhance current kirkwood_i2c driver support, and then add support for Orion followed by board support for edminiv2
Regards.. Prafulla ..
I can do this of course; however I felt that I was not fixing an existing driver (as I did with mvgbe) or adding support (as I did with mvsata where there was no existing driver) but introducing competition (as kirkwood_i2c exists and is functional) and I did not want to rudely stomp the existing driver.
Besides, as mvtwsi is new code, and even though I tested it (probe, read, write) with the ED Mini V2 EEPROM and RTC, until we are sure that it works we might want to keep the older kirkwood_i2c code around and be able to switch from one to the other -- having two different drivers for the same HW IP and selecting at config time is done in include/configs/km_arm.h where an option can be set to use either the soft I2C driver or the kirkwood one.
Finally, we can always remove the kirkwood_i2c driver later on if we want, in a separate patch (which will also switch km_arm to using mvtwsi).
Anyway, this mvtwsi patch will require Heiko's ACK as well as yours; let's hear from him (when he is back) on whether I should add mvtwsi or replace kirkwood_i2c.
Amicalement,
Hello Albert,
Albert ARIBAUD wrote:
(adding Heiko, custodian of I2C/EEPROM and committer of kirkwood_i2c)
Le 26/08/2010 06:33, Prafulla Wadaskar a écrit :
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Albert Aribaud Sent: Wednesday, August 25, 2010 7:54 PM To: u-boot@lists.denx.de Subject: [U-Boot] [PATCH 1/2] I2C: add Marvell TWSI simple driver
This driver is for the Marvell TWSI/I2C module found in the orion and kirkwood families among others.
Signed-off-by: Albert Aribaudalbert.aribaud@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).
Hi Albert This will be very good enhancement indeed.
drivers/i2c/Makefile | 1 + drivers/i2c/mvtwsi.c | 419
Can you pls follow the same strategy as we followed for mvgbe, mvsata? Please rename and enhance current kirkwood_i2c driver support, and then add support for Orion followed by board support for edminiv2
Regards.. Prafulla ..
I can do this of course; however I felt that I was not fixing an existing driver (as I did with mvgbe) or adding support (as I did with mvsata where there was no existing driver) but introducing competition (as kirkwood_i2c exists and is functional) and I did not want to rudely stomp the existing driver.
Besides, as mvtwsi is new code, and even though I tested it (probe, read, write) with the ED Mini V2 EEPROM and RTC, until we are sure that it works we might want to keep the older kirkwood_i2c code around and be able to switch from one to the other -- having two different drivers for the same HW IP and selecting at config time is done in include/configs/km_arm.h where an option can be set to use either the soft I2C driver or the kirkwood one.
We use only soft i2c on this board, so please remove the kirkwood_i2c.c driver completely. So we have only your driver in tree, which is used and working.
bye Heiko
-----Original Message----- From: Heiko Schocher [mailto:hs@denx.de] Sent: Thursday, August 26, 2010 12:17 PM To: Albert ARIBAUD Cc: Prafulla Wadaskar; u-boot@lists.denx.de; Heiko Schosher; Ashish Karkare; Prabhanjan Sarnaik Subject: Re: [U-Boot] [PATCH 1/2] I2C: add Marvell TWSI simple driver
Hello Albert,
Albert ARIBAUD wrote:
(adding Heiko, custodian of I2C/EEPROM and committer of
kirkwood_i2c)
Le 26/08/2010 06:33, Prafulla Wadaskar a écrit :
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Albert Aribaud Sent: Wednesday, August 25, 2010 7:54 PM To: u-boot@lists.denx.de Subject: [U-Boot] [PATCH 1/2] I2C: add Marvell TWSI simple driver
This driver is for the Marvell TWSI/I2C module found in the orion and kirkwood families among others.
Signed-off-by: Albert Aribaudalbert.aribaud@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).
Hi Albert This will be very good enhancement indeed.
drivers/i2c/Makefile | 1 + drivers/i2c/mvtwsi.c | 419
Can you pls follow the same strategy as we followed for
mvgbe, mvsata?
Please rename and enhance current kirkwood_i2c driver support, and then add support for Orion followed by board support
for edminiv2
Regards.. Prafulla ..
I can do this of course; however I felt that I was not fixing an existing driver (as I did with mvgbe) or adding support (as
I did with
mvsata where there was no existing driver) but introducing
competition
(as kirkwood_i2c exists and is functional) and I did not
want to rudely
stomp the existing driver.
Besides, as mvtwsi is new code, and even though I tested it (probe, read, write) with the ED Mini V2 EEPROM and RTC, until we
are sure that
it works we might want to keep the older kirkwood_i2c code
around and be
able to switch from one to the other -- having two
different drivers for
the same HW IP and selecting at config time is done in include/configs/km_arm.h where an option can be set to use
either the
soft I2C driver or the kirkwood one.
We use only soft i2c on this board, so please remove the kirkwood_i2c.c driver completely. So we have only your driver in tree, which is used and working.
Ack. Regards.. Prafulla . .
bye Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Le 26/08/2010 10:23, Prafulla Wadaskar a écrit :
We use only soft i2c on this board, so please remove the kirkwood_i2c.c driver completely. So we have only your driver in tree, which is used and working.
Ack. Regards.. Prafulla . .
Wilco. As a safety and cleanliness measure, I'll prepend a patch to the set, which will remove the 'hard i2c' related options from km_arm since they aren't used, and won't build any more once the switch is done.
Amicalement,
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@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@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;elsereturn 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;
+}
Le 26/08/2010 08:30, Heiko Schocher a écrit :
As the kirkwood_i2c driver is actually not used in any board, we should remove it completely, can you add this for v2?
That answers the questions I *just* asked on the list :) -- Wilco.
Beside of that, I have just some minor codstyling comments:
[...]
Will apply these. Thanks!
Amicalement,
Le 26/08/2010 08:30, Heiko Schocher a écrit :
Beside of that, I have just some minor codstyling comments:
Meanwhile, I've run checkpatch on this file and fixed every warning and error, thus some of your comments were taken care of in the process. That left:
+#define MVTWSI_STATUS_DATA_R_NAK 0x58 /* our NAK, not the slave's */
line too long.
Checkpatch did not complain on this one. Is there a specific line length contraint beside passing checkpatch?
+/*
- 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.
Which blank line exactly? Between the comment block and code? If so, does this apply everywhere, i.e. after the topmost license comment block, before blocks of #define, etc?
As a side note, if V2 gets Acked by Prafulla and you, can you add the commit to your i2c tree and ask for a new pull? Anyway, Wolfgang said he won't resume activity on u-boot before sep 5th.
Amicalement,
Hello Albert,
Albert ARIBAUD wrote:
Le 26/08/2010 08:30, Heiko Schocher a écrit :
Beside of that, I have just some minor codstyling comments:
Meanwhile, I've run checkpatch on this file and fixed every warning and error, thus some of your comments were taken care of in the process. That left:
+#define MVTWSI_STATUS_DATA_R_NAK 0x58 /* our NAK, not the slave's */
line too long.
Checkpatch did not complain on this one. Is there a specific line length contraint beside passing checkpatch?
Yep, 78 characters.
+/*
- 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.
Which blank line exactly? Between the comment block and code? If so,
Yes.
does this apply everywhere, i.e. after the topmost license comment block, before blocks of #define, etc?
No, there, it is Ok.
As a side note, if V2 gets Acked by Prafulla and you, can you add the commit to your i2c tree and ask for a new pull? Anyway, Wolfgang said he won't resume activity on u-boot before sep 5th.
Yes, if I and Prafulla accept it, this is the usual way.
bye, Heiko
Le 27/08/2010 07:25, Heiko Schocher a écrit :
+#define MVTWSI_STATUS_DATA_R_NAK 0x58 /* our NAK, not the slave's */
line too long.
Checkpatch did not complain on this one. Is there a specific line length contraint beside passing checkpatch?
Yep, 78 characters.
Ok. I added a possibility for checkpatch to accept a custom line length; I'll try and push that to the lkml. Meanwhile, it showed me three lines beyond 78 characters, fixed.
Which blank line exactly? Between the comment block and code? If so,
Yes.
does this apply everywhere, i.e. after the topmost license comment block, before blocks of #define, etc?
No, there, it is Ok.
Fixed, thanks.
As a side note, if V2 gets Acked by Prafulla and you, can you add the commit to your i2c tree and ask for a new pull? Anyway, Wolfgang said he won't resume activity on u-boot before sep 5th.
Yes, if I and Prafulla accept it, this is the usual way.
Here you go, then. :)
Thanks for your review.
Amicalement,
participants (5)
-
Albert ARIBAUD -
Albert Aribaud
-
Heiko Schocher -
Heiko Schocher -
Prafulla Wadaskar