[U-Boot] [PATCH 00/13] i2c: mvtwsi: DM conversion and improvements

This patch series converts the MVTWSI I2C driver to DM, fixes style violations, improves and cleans up the code, and adds lots of documentation.
Mario Six (13): i2c: mvtwsi: Fix style violations i2c: mvtwsi: Streamline code and add documentation i2c: mvtwsi: Improve and fix comments i2c: mvtwsi: Eliminate flags parameter i2c: mvtwsi: Get rid of status parameter i2c: mvtwsi: Use 'uint' instead of 'unsigned int' i2c: mvtwsi: Add compatibility functions i2c: mvtwsi: Factor out adap parameter i2c: mvtwsi: Make address length variable i2c: mvtwsi: Add compatibility to DM i2c: mvtwsi: Handle zero-length offsets properly i2c: mvtwsi: Make delay times frequency-dependent i2c: mvtwsi: Add documentation
drivers/i2c/Kconfig | 7 + drivers/i2c/mvtwsi.c | 781 +++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 574 insertions(+), 214 deletions(-)
-- 2.9.0

This patch fixes seven style violations: Six superfluous spaces after casts, and one logical continuation violation.
Signed-off-by: Mario Six mario.six@gdsys.cc --- drivers/i2c/mvtwsi.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index bf44432..e9e7c47 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -110,27 +110,27 @@ static struct mvtwsi_registers *twsi_get_base(struct i2c_adapter *adap) switch (adap->hwadapnr) { #ifdef CONFIG_I2C_MVTWSI_BASE0 case 0: - return (struct mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE0; + return (struct mvtwsi_registers *)CONFIG_I2C_MVTWSI_BASE0; #endif #ifdef CONFIG_I2C_MVTWSI_BASE1 case 1: - return (struct mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE1; + return (struct mvtwsi_registers *)CONFIG_I2C_MVTWSI_BASE1; #endif #ifdef CONFIG_I2C_MVTWSI_BASE2 case 2: - return (struct mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE2; + return (struct mvtwsi_registers *)CONFIG_I2C_MVTWSI_BASE2; #endif #ifdef CONFIG_I2C_MVTWSI_BASE3 case 3: - return (struct mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE3; + return (struct mvtwsi_registers *)CONFIG_I2C_MVTWSI_BASE3; #endif #ifdef CONFIG_I2C_MVTWSI_BASE4 case 4: - return (struct mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE4; + return (struct mvtwsi_registers *)CONFIG_I2C_MVTWSI_BASE4; #endif #ifdef CONFIG_I2C_MVTWSI_BASE5 case 5: - return (struct mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE5; + return (struct mvtwsi_registers *)CONFIG_I2C_MVTWSI_BASE5; #endif default: printf("Missing mvtwsi controller %d base\n", adap->hwadapnr); @@ -312,8 +312,8 @@ static unsigned int twsi_i2c_set_bus_speed(struct i2c_adapter *adap, for (n = 0; n < 8; n++) { for (m = 0; m < 16; m++) { tmp_speed = twsi_calc_freq(n, m); - if ((tmp_speed <= requested_speed) - && (tmp_speed > highest_speed)) { + if ((tmp_speed <= requested_speed) && + (tmp_speed > highest_speed)) { highest_speed = tmp_speed; baud = (m << 3) | n; }

Convert groups of logically connected preprocessor defines into proper enums, one macro into an inline function, and add documentation to/extend existing documentation of these items.
Signed-off-by: Mario Six mario.six@gdsys.cc --- drivers/i2c/mvtwsi.c | 113 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 75 insertions(+), 38 deletions(-)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index e9e7c47..93bb88b 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -62,15 +62,23 @@ struct mvtwsi_registers { #endif
/* - * Control register fields + * enum mvtwsi_ctrl_register_fields - Bit masks for flags in the control + * register */ - -#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 +enum mvtwsi_ctrl_register_fields { + /* Acknowledge bit */ + MVTWSI_CONTROL_ACK = 0x00000004, + /* Interrupt flag */ + MVTWSI_CONTROL_IFLG = 0x00000008, + /* Stop bit */ + MVTWSI_CONTROL_STOP = 0x00000010, + /* Start bit */ + MVTWSI_CONTROL_START = 0x00000020, + /* I2C enable */ + MVTWSI_CONTROL_TWSIEN = 0x00000040, + /* Interrupt enable */ + MVTWSI_CONTROL_INTEN = 0x00000080, +};
/* * On sun6i and newer IFLG is a write-clear bit which is cleared by writing 1, @@ -84,22 +92,36 @@ struct mvtwsi_registers { #endif
/* - * 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 + * enum mvstwsi_status_values - Possible values of I2C controller's status + * register + * + * Only those statuses expected in normal master operation on + * non-10-bit-address devices are specified. + * + * Every status that's unexpected during normal operation (bus errors, + * arbitration losses, missing ACKs...) is passed 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 -#define MVTWSI_STATUS_IDLE 0xF8 +enum mvstwsi_status_values { + /* START condition transmitted */ + MVTWSI_STATUS_START = 0x08, + /* Repeated START condition transmitted */ + MVTWSI_STATUS_REPEATED_START = 0x10, + /* Address + write bit transmitted, ACK received */ + MVTWSI_STATUS_ADDR_W_ACK = 0x18, + /* Data transmitted, ACK received */ + MVTWSI_STATUS_DATA_W_ACK = 0x28, + /* Address + read bit transmitted, ACK received */ + MVTWSI_STATUS_ADDR_R_ACK = 0x40, + /* Address + read bit transmitted, ACK not received */ + MVTWSI_STATUS_ADDR_R_NAK = 0x48, + /* Data received, ACK transmitted */ + MVTWSI_STATUS_DATA_R_ACK = 0x50, + /* Data received, ACK not transmitted */ + MVTWSI_STATUS_DATA_R_NAK = 0x58, + /* No relevant status */ + MVTWSI_STATUS_IDLE = 0xF8, +};
/* * MVTWSI controller base @@ -141,20 +163,35 @@ static struct mvtwsi_registers *twsi_get_base(struct i2c_adapter *adap) }
/* - * 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 error class: 1 is timeout, 2 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. + * enum mvtwsi_error_class - types of I2C errors */ +enum mvtwsi_error_class { + /* The controller returned a different status than expected */ + MVTWSI_ERROR_WRONG_STATUS = 0x01, + /* The controller timed out */ + MVTWSI_ERROR_TIMEOUT = 0x02, +};
-#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)) +/* + * mvtwsi_error() - Build I2C return code from error information + * + * For debugging purposes, this function packs some information of an occurred + * error into a return code. These error codes are returned from I2C API + * functions (i2c_{read,write}, dm_i2c_{read,write}, etc.). + * + * @ec: The error class of the error (enum mvtwsi_error_class). + * @lc: The last value of the control register. + * @ls: The last value of the status register. + * @es: The expected value of the status register. + * @return The generated error code. + */ +inline uint mvtwsi_error(uint ec, uint lc, uint ls, uint es) +{ + return ((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, @@ -173,15 +210,15 @@ static int twsi_wait(struct i2c_adapter *adap, int expected_status) if (status == expected_status) return 0; else - return MVTWSI_ERROR( + return mvtwsi_error( MVTWSI_ERROR_WRONG_STATUS, control, status, expected_status); } udelay(10); /* one clock cycle at 100 kHz */ } while (timeout--); status = readl(&twsi->status); - return MVTWSI_ERROR( - MVTWSI_ERROR_TIMEOUT, control, status, expected_status); + return mvtwsi_error(MVTWSI_ERROR_TIMEOUT, control, status, + expected_status); }
/* @@ -265,7 +302,7 @@ static int twsi_stop(struct i2c_adapter *adap, int status) control = readl(&twsi->control); if (stop_status != MVTWSI_STATUS_IDLE) if (status == 0) - status = MVTWSI_ERROR( + status = mvtwsi_error( MVTWSI_ERROR_TIMEOUT, control, status, MVTWSI_STATUS_IDLE); return status;

This patch fixes only comments/documentation: Streamline capitalization and improve grammar/punctuation.
Signed-off-by: Mario Six mario.six@gdsys.cc --- drivers/i2c/mvtwsi.c | 129 +++++++++++++++++++++++++-------------------------- 1 file changed, 62 insertions(+), 67 deletions(-)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index 93bb88b..7b2c611 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -14,8 +14,8 @@ #include <asm/io.h>
/* - * include a file that will provide CONFIG_I2C_MVTWSI_BASE* - * and possibly other settings + * Include a file that will provide CONFIG_I2C_MVTWSI_BASE*, and possibly other + * settings */
#if defined(CONFIG_ORION5X) @@ -51,8 +51,8 @@ struct mvtwsi_registers { u32 data; u32 control; union { - u32 status; /* when reading */ - u32 baudrate; /* when writing */ + u32 status; /* When reading */ + u32 baudrate; /* When writing */ }; u32 xtnd_slave_addr; u32 reserved[2]; @@ -81,8 +81,8 @@ enum mvtwsi_ctrl_register_fields { };
/* - * On sun6i and newer IFLG is a write-clear bit which is cleared by writing 1, - * on other platforms it is a normal r/w bit which is cleared by writing 0. + * On sun6i and newer, IFLG is a write-clear bit, which is cleared by writing 1; + * on other platforms, it is a normal r/w bit, which is cleared by writing 0. */
#ifdef CONFIG_SUNXI_GEN_SUN6I @@ -194,8 +194,8 @@ inline uint mvtwsi_error(uint ec, uint lc, uint ls, uint es) }
/* - * Wait for IFLG to raise, or return 'timeout'; then if status is as expected, - * return 0 (ok) or return 'wrong status'. + * Wait for IFLG to raise, or return 'timeout.' Then, if the status is as + * expected, return 0 (ok) or 'wrong status' otherwise. */ static int twsi_wait(struct i2c_adapter *adap, int expected_status) { @@ -214,7 +214,7 @@ static int twsi_wait(struct i2c_adapter *adap, int expected_status) MVTWSI_ERROR_WRONG_STATUS, control, status, expected_status); } - udelay(10); /* one clock cycle at 100 kHz */ + udelay(10); /* One clock cycle at 100 kHz */ } while (timeout--); status = readl(&twsi->status); return mvtwsi_error(MVTWSI_ERROR_TIMEOUT, control, status, @@ -229,12 +229,12 @@ static int twsi_start(struct i2c_adapter *adap, int expected_status, u8 *flags) { struct mvtwsi_registers *twsi = twsi_get_base(adap);
- /* globally set TWSIEN in case it was not */ + /* Set TWSIEN */ *flags |= MVTWSI_CONTROL_TWSIEN; - /* assert START */ + /* Assert START */ writel(*flags | MVTWSI_CONTROL_START | - MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); - /* wait for controller to process START */ + MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); + /* Wait for controller to process START */ return twsi_wait(adap, expected_status); }
@@ -246,42 +246,40 @@ static int twsi_send(struct i2c_adapter *adap, u8 byte, int expected_status, { struct mvtwsi_registers *twsi = twsi_get_base(adap);
- /* put byte in data register for sending */ + /* Write byte to data register for sending */ writel(byte, &twsi->data); - /* clear any pending interrupt -- that'll cause sending */ + /* Clear any pending interrupt -- that will cause sending */ writel(*flags | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); - /* wait for controller to receive byte and check ACK */ + /* Wait for controller to receive byte, and check ACK */ return twsi_wait(adap, expected_status); }
/* * Receive a byte. - * Global mvtwsi_control_flags variable says if we should ack or nak. */ static int twsi_recv(struct i2c_adapter *adap, u8 *byte, u8 *flags) { struct mvtwsi_registers *twsi = twsi_get_base(adap); int expected_status, status;
- /* compute expected status based on ACK bit in global control flags */ + /* Compute expected status based on ACK bit in passed control flags */ if (*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 */ + /* Acknowledge *previous state*, and launch receive */ writel(*flags | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); - /* wait for controller to receive byte and assert ACK or NAK */ + /* Wait for controller to receive byte, and assert ACK or NAK */ status = twsi_wait(adap, expected_status); - /* if we did receive expected byte then store it */ + /* If we did receive the expected byte, 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). + * This is also used to force the bus back to idle (SDA = SCL = 1). */ static int twsi_stop(struct i2c_adapter *adap, int status) { @@ -289,15 +287,15 @@ static int twsi_stop(struct i2c_adapter *adap, int status) int control, stop_status; int timeout = 1000;
- /* assert STOP */ + /* Assert STOP */ control = MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_STOP; writel(control | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); - /* wait for IDLE; IFLG won't rise so twsi_wait() is no use. */ + /* Wait for IDLE; IFLG won't rise, so we can't use twsi_wait() */ do { stop_status = readl(&twsi->status); if (stop_status == MVTWSI_STATUS_IDLE) break; - udelay(10); /* one clock cycle at 100 kHz */ + udelay(10); /* One clock cycle at 100 kHz */ } while (timeout--); control = readl(&twsi->control); if (stop_status != MVTWSI_STATUS_IDLE) @@ -326,26 +324,26 @@ static void twsi_reset(struct i2c_adapter *adap) { struct mvtwsi_registers *twsi = twsi_get_base(adap);
- /* reset controller */ + /* Reset controller */ writel(0, &twsi->soft_reset); - /* wait 2 ms -- this is what the Marvell LSP does */ + /* Wait 2 ms -- this is what the Marvell LSP does */ udelay(20000); }
/* - * I2C init called by cmd_i2c when doing 'i2c reset'. - * Sets baud to the highest possible value not exceeding requested one. + * Sets baud to the highest possible value not exceeding the requested one. */ static unsigned int twsi_i2c_set_bus_speed(struct i2c_adapter *adap, unsigned int requested_speed) { struct mvtwsi_registers *twsi = twsi_get_base(adap); unsigned int tmp_speed, highest_speed, n, m; - unsigned int baud = 0x44; /* baudrate at controller reset */ + unsigned int baud = 0x44; /* Baud rate after controller reset */
- /* use actual speed to collect progressively higher values */ highest_speed = 0; - /* compute m, n setting for highest speed not above requested speed */ + /* Successively try m, n combinations, and use the combination + * resulting in the largest speed that's not above the requested + * speed */ for (n = 0; n < 8; n++) { for (m = 0; m < 16; m++) { tmp_speed = twsi_calc_freq(n, m); @@ -364,20 +362,19 @@ static void twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd) { struct mvtwsi_registers *twsi = twsi_get_base(adap);
- /* reset controller */ + /* Reset controller */ twsi_reset(adap); - /* set speed */ + /* Set speed */ twsi_i2c_set_bus_speed(adap, speed); - /* set slave address even though we don't use it */ + /* Set slave address; even though we don't use it */ writel(slaveadd, &twsi->slave_address); writel(0, &twsi->xtnd_slave_addr); - /* assert STOP but don't care for the result */ + /* Assert STOP, but don't care for the result */ (void) twsi_stop(adap, 0); }
/* * 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(struct i2c_adapter *adap, int expected_start_status, @@ -385,23 +382,23 @@ static int i2c_begin(struct i2c_adapter *adap, int expected_start_status, { int status, expected_addr_status;
- /* compute expected address status from direction bit in addr */ - if (addr & 1) /* reading */ + /* Compute the expected address status from the direction bit in + * the address byte */ + if (addr & 1) /* Reading */ expected_addr_status = MVTWSI_STATUS_ADDR_R_ACK; - else /* writing */ + else /* Writing */ expected_addr_status = MVTWSI_STATUS_ADDR_W_ACK; - /* assert START */ + /* Assert START */ status = twsi_start(adap, expected_start_status, flags); - /* send out the address if the start went well */ + /* Send out the address if the start went well */ if (status == 0) status = twsi_send(adap, addr, expected_addr_status, flags); - /* return ok or status of first failure to caller */ + /* Return 0, or the status of the first failure */ return status; }
/* - * I2C probe called by cmd_i2c when doing 'i2c probe'. * Begin read, nak data byte, end. */ static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip) @@ -410,26 +407,24 @@ static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip) u8 flags = 0; int status;
- /* begin i2c read */ + /* Begin i2c read */ status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1) | 1, &flags); - /* dummy read was accepted: receive byte but NAK it. */ + /* Dummy read was accepted: receive byte, but NAK it. */ if (status == 0) status = twsi_recv(adap, &dummy_byte, &flags); /* Stop transaction */ twsi_stop(adap, 0); - /* return 0 or status of first failure */ + /* Return 0, or the status of the first failure */ return 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. * - * NOTE: some EEPROMS want a stop right before the second start, while - * some will choke if it is there. Deciding which we should do is eeprom - * stuff, not i2c, but at the moment the APIs won't let us put it in - * cmd_eeprom, so we have to choose here, and for the moment that'll be - * a repeated start without a preceding stop. + * NOTE: Some devices want a stop right before the second start, while some + * will choke if it is there. Since deciding this is not yet supported in + * higher level APIs, we need to make a decision here, and for the moment that + * will be a repeated start without a preceding stop. */ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, int alen, uchar *data, int length) @@ -437,35 +432,34 @@ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, int status; u8 flags = 0;
- /* begin i2c write to send the address bytes */ + /* Begin i2c write to send the address bytes */ status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1), &flags); - /* send addr bytes */ + /* Send address bytes */ while ((status == 0) && alen--) status = twsi_send(adap, addr >> (8*alen), MVTWSI_STATUS_DATA_W_ACK, &flags); - /* begin i2c read to receive eeprom data bytes */ + /* Begin i2c read to receive data bytes */ if (status == 0) status = i2c_begin(adap, MVTWSI_STATUS_REPEATED_START, (chip << 1) | 1, &flags); - /* prepare ACK if at least one byte must be received */ + /* Prepare ACK if at least one byte must be received */ if (length > 0) flags |= MVTWSI_CONTROL_ACK; - /* now receive actual bytes */ + /* Receive actual data bytes */ while ((status == 0) && length--) { - /* reset NAK if we if no more to read now */ + /* Set NAK if we if we have nothing more to read */ if (length == 0) flags &= ~MVTWSI_CONTROL_ACK; - /* read current byte */ + /* Read current byte */ status = twsi_recv(adap, data++, &flags); } /* Stop transaction */ status = twsi_stop(adap, status); - /* return 0 or status of first failure */ + /* Return 0, or the status of the first failure */ return 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. */ static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, @@ -474,19 +468,20 @@ static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, int status; u8 flags = 0;
- /* begin i2c write to send the eeprom adress bytes then data bytes */ + /* Begin i2c write to send first the address bytes, then the + * data bytes */ status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1), &flags); - /* send addr bytes */ + /* Send address bytes */ while ((status == 0) && alen--) status = twsi_send(adap, addr >> (8*alen), MVTWSI_STATUS_DATA_W_ACK, &flags); - /* send data bytes */ + /* Send data bytes */ while ((status == 0) && (length-- > 0)) status = twsi_send(adap, *(data++), MVTWSI_STATUS_DATA_W_ACK, &flags); /* Stop transaction */ status = twsi_stop(adap, status); - /* return 0 or status of first failure */ + /* Return 0, or the status of the first failure */ return status; }

Due to breaking boots from NOR flashes, commit d6b7757 ("i2c: mvtwsi: Eliminate twsi_control_flags") removed the static global twsi_control_flags variable, which kept a set of default flags that were always or'd to the control register when writing. It was replaced with a flags parameter, which was passed around between the functions that needed it.
Since the twsi_control_flags variable was used just for the purposes of a) setting the MVTWSI_CONTROL_TWSIEN on every control register write, and b) setting the MVTWSI_CONTROL_ACK from twsi_i2c_read if needed, anyway, the added overhead of another variable being passed around is no longer justified, and we are better off implementing this flag setting logic locally in the functions that actually write to the control register.
Therefore, this patch sets MVTWSI_CONTROL_TWSIEN on every control register write, replaces the twsi_i2c_read's flags parameter with a ack_flag parameter, which tells the function whether to acknowledge the read or not, and removes every other instance of the flags variable. This has the added benefit that now every notion of "global default flags" is gone, and it's much easier to see which control flags are actually set at which point in time.
Signed-off-by: Mario Six mario.six@gdsys.cc --- drivers/i2c/mvtwsi.c | 83 ++++++++++++++++++++++++++-------------------------- 1 file changed, 41 insertions(+), 42 deletions(-)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index 7b2c611..4a34eab 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -124,6 +124,17 @@ enum mvstwsi_status_values { };
/* + * enum mvstwsi_ack_flags - Determine whether a read byte should be + * acknowledged or not. + */ +enum mvtwsi_ack_flags { + /* Send NAK after received byte */ + MVTWSI_READ_NAK = 0, + /* Send ACK after received byte */ + MVTWSI_READ_ACK = 1, +}; + +/* * MVTWSI controller base */
@@ -225,14 +236,12 @@ static int twsi_wait(struct i2c_adapter *adap, int expected_status) * Assert the START condition, either in a single I2C transaction * or inside back-to-back ones (repeated starts). */ -static int twsi_start(struct i2c_adapter *adap, int expected_status, u8 *flags) +static int twsi_start(struct i2c_adapter *adap, int expected_status) { struct mvtwsi_registers *twsi = twsi_get_base(adap);
- /* Set TWSIEN */ - *flags |= MVTWSI_CONTROL_TWSIEN; /* Assert START */ - writel(*flags | MVTWSI_CONTROL_START | + writel(MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_START | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); /* Wait for controller to process START */ return twsi_wait(adap, expected_status); @@ -241,15 +250,15 @@ static int twsi_start(struct i2c_adapter *adap, int expected_status, u8 *flags) /* * Send a byte (i2c address or data). */ -static int twsi_send(struct i2c_adapter *adap, u8 byte, int expected_status, - u8 *flags) +static int twsi_send(struct i2c_adapter *adap, u8 byte, int expected_status) { struct mvtwsi_registers *twsi = twsi_get_base(adap);
/* Write byte to data register for sending */ writel(byte, &twsi->data); /* Clear any pending interrupt -- that will cause sending */ - writel(*flags | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); + writel(MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_CLEAR_IFLG, + &twsi->control); /* Wait for controller to receive byte, and check ACK */ return twsi_wait(adap, expected_status); } @@ -257,18 +266,18 @@ static int twsi_send(struct i2c_adapter *adap, u8 byte, int expected_status, /* * Receive a byte. */ -static int twsi_recv(struct i2c_adapter *adap, u8 *byte, u8 *flags) +static int twsi_recv(struct i2c_adapter *adap, u8 *byte, int ack_flag) { struct mvtwsi_registers *twsi = twsi_get_base(adap); - int expected_status, status; + int expected_status, status, control;
- /* Compute expected status based on ACK bit in passed control flags */ - if (*flags & MVTWSI_CONTROL_ACK) - expected_status = MVTWSI_STATUS_DATA_R_ACK; - else - expected_status = MVTWSI_STATUS_DATA_R_NAK; + /* Compute expected status based on passed ACK flag */ + expected_status = ack_flag ? MVTWSI_STATUS_DATA_R_ACK : + MVTWSI_STATUS_DATA_R_NAK; /* Acknowledge *previous state*, and launch receive */ - writel(*flags | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); + control = MVTWSI_CONTROL_TWSIEN; + control |= ack_flag == MVTWSI_READ_ACK ? MVTWSI_CONTROL_ACK : 0; + writel(control | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); /* Wait for controller to receive byte, and assert ACK or NAK */ status = twsi_wait(adap, expected_status); /* If we did receive the expected byte, store it */ @@ -378,7 +387,7 @@ static void twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd) * Expected address status will derive from direction bit (bit 0) in addr. */ static int i2c_begin(struct i2c_adapter *adap, int expected_start_status, - u8 addr, u8 *flags) + u8 addr) { int status, expected_addr_status;
@@ -389,11 +398,10 @@ static int i2c_begin(struct i2c_adapter *adap, int expected_start_status, else /* Writing */ expected_addr_status = MVTWSI_STATUS_ADDR_W_ACK; /* Assert START */ - status = twsi_start(adap, expected_start_status, flags); + status = twsi_start(adap, expected_start_status); /* Send out the address if the start went well */ if (status == 0) - status = twsi_send(adap, addr, expected_addr_status, - flags); + status = twsi_send(adap, addr, expected_addr_status); /* Return 0, or the status of the first failure */ return status; } @@ -404,14 +412,13 @@ static int i2c_begin(struct i2c_adapter *adap, int expected_start_status, static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip) { u8 dummy_byte; - u8 flags = 0; int status;
/* Begin i2c read */ - status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1) | 1, &flags); + status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1) | 1); /* Dummy read was accepted: receive byte, but NAK it. */ if (status == 0) - status = twsi_recv(adap, &dummy_byte, &flags); + status = twsi_recv(adap, &dummy_byte, MVTWSI_READ_NAK); /* Stop transaction */ twsi_stop(adap, 0); /* Return 0, or the status of the first failure */ @@ -430,29 +437,23 @@ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, int alen, uchar *data, int length) { int status; - u8 flags = 0;
/* Begin i2c write to send the address bytes */ - status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1), &flags); + status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1)); /* Send address bytes */ while ((status == 0) && alen--) status = twsi_send(adap, addr >> (8*alen), - MVTWSI_STATUS_DATA_W_ACK, &flags); + MVTWSI_STATUS_DATA_W_ACK); /* Begin i2c read to receive data bytes */ if (status == 0) status = i2c_begin(adap, MVTWSI_STATUS_REPEATED_START, - (chip << 1) | 1, &flags); - /* Prepare ACK if at least one byte must be received */ - if (length > 0) - flags |= MVTWSI_CONTROL_ACK; - /* Receive actual data bytes */ - while ((status == 0) && length--) { - /* Set NAK if we if we have nothing more to read */ - if (length == 0) - flags &= ~MVTWSI_CONTROL_ACK; - /* Read current byte */ - status = twsi_recv(adap, data++, &flags); - } + (chip << 1) | 1); + /* Receive actual data bytes; set NAK if we if we have nothing more to + * read */ + while ((status == 0) && length--) + status = twsi_recv(adap, data++, + length > 0 ? + MVTWSI_READ_ACK : MVTWSI_READ_NAK); /* Stop transaction */ status = twsi_stop(adap, status); /* Return 0, or the status of the first failure */ @@ -466,19 +467,17 @@ static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, int alen, uchar *data, int length) { int status; - u8 flags = 0;
/* Begin i2c write to send first the address bytes, then the * data bytes */ - status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1), &flags); + status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1)); /* Send address bytes */ while ((status == 0) && alen--) status = twsi_send(adap, addr >> (8*alen), - MVTWSI_STATUS_DATA_W_ACK, &flags); + MVTWSI_STATUS_DATA_W_ACK); /* Send data bytes */ while ((status == 0) && (length-- > 0)) - status = twsi_send(adap, *(data++), MVTWSI_STATUS_DATA_W_ACK, - &flags); + status = twsi_send(adap, *(data++), MVTWSI_STATUS_DATA_W_ACK); /* Stop transaction */ status = twsi_stop(adap, status); /* Return 0, or the status of the first failure */

The twsi_stop function contains a parameter "status," which is used to pass in the current exit status of the function calling twsi_stop, and either return this status unchanged if it indicates an error, or return twsi_stop's exit status if it does not indicate an error.
While not massively complicated, this adds another purpose to the twsi_stop function, which should have the sole purpose of asserting a STOP condition on the bus (and not manage the exit status of its caller).
Therefore, we move the exit status management into the caller functions by introducing a "stop_status" variable and returning either the status before the twsi_stop call (kept in the "status" variable), or the status from the twsi_stop call, depending on which indicates an error.
Signed-off-by: Mario Six mario.six@gdsys.cc --- drivers/i2c/mvtwsi.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index 4a34eab..d0e3c3f 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -290,10 +290,11 @@ static int twsi_recv(struct i2c_adapter *adap, u8 *byte, int ack_flag) * Assert the STOP condition. * This is also used to force the bus back to idle (SDA = SCL = 1). */ -static int twsi_stop(struct i2c_adapter *adap, int status) +static int twsi_stop(struct i2c_adapter *adap) { struct mvtwsi_registers *twsi = twsi_get_base(adap); int control, stop_status; + int status = 0; int timeout = 1000;
/* Assert STOP */ @@ -308,10 +309,8 @@ static int twsi_stop(struct i2c_adapter *adap, int status) } while (timeout--); control = readl(&twsi->control); if (stop_status != MVTWSI_STATUS_IDLE) - if (status == 0) - status = mvtwsi_error( - MVTWSI_ERROR_TIMEOUT, - control, status, MVTWSI_STATUS_IDLE); + status = mvtwsi_error(MVTWSI_ERROR_TIMEOUT, + control, status, MVTWSI_STATUS_IDLE); return status; }
@@ -379,7 +378,7 @@ static void twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd) writel(slaveadd, &twsi->slave_address); writel(0, &twsi->xtnd_slave_addr); /* Assert STOP, but don't care for the result */ - (void) twsi_stop(adap, 0); + (void) twsi_stop(adap); }
/* @@ -420,7 +419,7 @@ static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip) if (status == 0) status = twsi_recv(adap, &dummy_byte, MVTWSI_READ_NAK); /* Stop transaction */ - twsi_stop(adap, 0); + twsi_stop(adap); /* Return 0, or the status of the first failure */ return status; } @@ -436,7 +435,8 @@ static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip) static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, int alen, uchar *data, int length) { - int status; + int status = 0; + int stop_status;
/* Begin i2c write to send the address bytes */ status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1)); @@ -455,9 +455,9 @@ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, length > 0 ? MVTWSI_READ_ACK : MVTWSI_READ_NAK); /* Stop transaction */ - status = twsi_stop(adap, status); + stop_status = twsi_stop(adap); /* Return 0, or the status of the first failure */ - return status; + return status != 0 ? status : stop_status; }
/* @@ -466,7 +466,7 @@ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, int alen, uchar *data, int length) { - int status; + int status, stop_status;
/* Begin i2c write to send first the address bytes, then the * data bytes */ @@ -479,9 +479,9 @@ static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, while ((status == 0) && (length-- > 0)) status = twsi_send(adap, *(data++), MVTWSI_STATUS_DATA_W_ACK); /* Stop transaction */ - status = twsi_stop(adap, status); + stop_status = twsi_stop(adap); /* Return 0, or the status of the first failure */ - return status; + return status != 0 ? status : stop_status; }
#ifdef CONFIG_I2C_MVTWSI_BASE0

Since some additional parameters will be added in the course of this patch series (especially with the addition of DM support), we replace the longer "unsigned int" declarations with "uint" declarations to keep the parameter lists more readable.
Signed-off-by: Mario Six mario.six@gdsys.cc --- drivers/i2c/mvtwsi.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index d0e3c3f..9ddd3ee 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -314,7 +314,7 @@ static int twsi_stop(struct i2c_adapter *adap) return status; }
-static unsigned int twsi_calc_freq(const int n, const int m) +static uint twsi_calc_freq(const int n, const int m) { #ifdef CONFIG_SUNXI return CONFIG_SYS_TCLK / (10 * (m + 1) * (1 << n)); @@ -341,12 +341,12 @@ static void twsi_reset(struct i2c_adapter *adap) /* * Sets baud to the highest possible value not exceeding the requested one. */ -static unsigned int twsi_i2c_set_bus_speed(struct i2c_adapter *adap, - unsigned int requested_speed) +static uint twsi_i2c_set_bus_speed(struct i2c_adapter *adap, + uint requested_speed) { struct mvtwsi_registers *twsi = twsi_get_base(adap); - unsigned int tmp_speed, highest_speed, n, m; - unsigned int baud = 0x44; /* Baud rate after controller reset */ + uint tmp_speed, highest_speed, n, m; + uint baud = 0x44; /* Baud rate after controller reset */
highest_speed = 0; /* Successively try m, n combinations, and use the combination

To prepare for the DM conversion, we add a layer of compatibility functions to be used by both the legacy and the DM functions.
Signed-off-by: Mario Six mario.six@gdsys.cc --- drivers/i2c/mvtwsi.c | 47 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index 9ddd3ee..f1bfd5d 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -341,8 +341,8 @@ static void twsi_reset(struct i2c_adapter *adap) /* * Sets baud to the highest possible value not exceeding the requested one. */ -static uint twsi_i2c_set_bus_speed(struct i2c_adapter *adap, - uint requested_speed) +static uint __twsi_i2c_set_bus_speed(struct i2c_adapter *adap, + uint requested_speed) { struct mvtwsi_registers *twsi = twsi_get_base(adap); uint tmp_speed, highest_speed, n, m; @@ -366,14 +366,14 @@ static uint twsi_i2c_set_bus_speed(struct i2c_adapter *adap, return 0; }
-static void twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd) +static void __twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd) { struct mvtwsi_registers *twsi = twsi_get_base(adap);
/* Reset controller */ twsi_reset(adap); /* Set speed */ - twsi_i2c_set_bus_speed(adap, speed); + __twsi_i2c_set_bus_speed(adap, speed); /* Set slave address; even though we don't use it */ writel(slaveadd, &twsi->slave_address); writel(0, &twsi->xtnd_slave_addr); @@ -408,7 +408,7 @@ static int i2c_begin(struct i2c_adapter *adap, int expected_start_status, /* * Begin read, nak data byte, end. */ -static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip) +static int __twsi_i2c_probe_chip(struct i2c_adapter *adap, uchar chip) { u8 dummy_byte; int status; @@ -432,8 +432,8 @@ static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip) * higher level APIs, we need to make a decision here, and for the moment that * will be a repeated start without a preceding stop. */ -static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, - int alen, uchar *data, int length) +static int __twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, + int alen, uchar *data, int length) { int status = 0; int stop_status; @@ -463,8 +463,8 @@ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, /* * Begin write, send address byte(s), send data bytes, end. */ -static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, - int alen, uchar *data, int length) +static int __twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, + int alen, uchar *data, int length) { int status, stop_status;
@@ -484,6 +484,35 @@ static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, return status != 0 ? status : stop_status; }
+static void twsi_i2c_init(struct i2c_adapter *adap, int speed, + int slaveadd) +{ + __twsi_i2c_init(adap, speed, slaveadd); +} + +static uint twsi_i2c_set_bus_speed(struct i2c_adapter *adap, + uint requested_speed) +{ + return __twsi_i2c_set_bus_speed(adap, requested_speed); +} + +static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip) +{ + return __twsi_i2c_probe_chip(adap, chip); +} + +static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, + int alen, uchar *data, int length) +{ + return __twsi_i2c_read(adap, chip, addr, alen, data, length); +} + +static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, + int alen, uchar *data, int length) +{ + return __twsi_i2c_write(adap, chip, addr, alen, data, length); +} + #ifdef CONFIG_I2C_MVTWSI_BASE0 U_BOOT_I2C_ADAP_COMPLETE(twsi0, twsi_i2c_init, twsi_i2c_probe, twsi_i2c_read, twsi_i2c_write,

To be able to use the compatibility layer from the DM functions, we factor the adap parameter out of all functions, and pass the actual register base instead.
Signed-off-by: Mario Six mario.six@gdsys.cc --- drivers/i2c/mvtwsi.c | 97 +++++++++++++++++++++++++--------------------------- 1 file changed, 46 insertions(+), 51 deletions(-)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index f1bfd5d..688efc2 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -208,9 +208,8 @@ inline uint mvtwsi_error(uint ec, uint lc, uint ls, uint es) * Wait for IFLG to raise, or return 'timeout.' Then, if the status is as * expected, return 0 (ok) or 'wrong status' otherwise. */ -static int twsi_wait(struct i2c_adapter *adap, int expected_status) +static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status) { - struct mvtwsi_registers *twsi = twsi_get_base(adap); int control, status; int timeout = 1000;
@@ -236,39 +235,35 @@ static int twsi_wait(struct i2c_adapter *adap, int expected_status) * Assert the START condition, either in a single I2C transaction * or inside back-to-back ones (repeated starts). */ -static int twsi_start(struct i2c_adapter *adap, int expected_status) +static int twsi_start(struct mvtwsi_registers *twsi, int expected_status) { - struct mvtwsi_registers *twsi = twsi_get_base(adap); - /* Assert START */ writel(MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_START | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); /* Wait for controller to process START */ - return twsi_wait(adap, expected_status); + return twsi_wait(twsi, expected_status); }
/* * Send a byte (i2c address or data). */ -static int twsi_send(struct i2c_adapter *adap, u8 byte, int expected_status) +static int twsi_send(struct mvtwsi_registers *twsi, u8 byte, + int expected_status) { - struct mvtwsi_registers *twsi = twsi_get_base(adap); - /* Write byte to data register for sending */ writel(byte, &twsi->data); /* Clear any pending interrupt -- that will cause sending */ writel(MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); /* Wait for controller to receive byte, and check ACK */ - return twsi_wait(adap, expected_status); + return twsi_wait(twsi, expected_status); }
/* * Receive a byte. */ -static int twsi_recv(struct i2c_adapter *adap, u8 *byte, int ack_flag) +static int twsi_recv(struct mvtwsi_registers *twsi, u8 *byte, int ack_flag) { - struct mvtwsi_registers *twsi = twsi_get_base(adap); int expected_status, status, control;
/* Compute expected status based on passed ACK flag */ @@ -279,7 +274,7 @@ static int twsi_recv(struct i2c_adapter *adap, u8 *byte, int ack_flag) control |= ack_flag == MVTWSI_READ_ACK ? MVTWSI_CONTROL_ACK : 0; writel(control | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); /* Wait for controller to receive byte, and assert ACK or NAK */ - status = twsi_wait(adap, expected_status); + status = twsi_wait(twsi, expected_status); /* If we did receive the expected byte, store it */ if (status == 0) *byte = readl(&twsi->data); @@ -290,9 +285,8 @@ static int twsi_recv(struct i2c_adapter *adap, u8 *byte, int ack_flag) * Assert the STOP condition. * This is also used to force the bus back to idle (SDA = SCL = 1). */ -static int twsi_stop(struct i2c_adapter *adap) +static int twsi_stop(struct mvtwsi_registers *twsi) { - struct mvtwsi_registers *twsi = twsi_get_base(adap); int control, stop_status; int status = 0; int timeout = 1000; @@ -328,10 +322,8 @@ static uint twsi_calc_freq(const int n, const int m) * Controller reset also resets the baud rate and slave address, so * they must be re-established afterwards. */ -static void twsi_reset(struct i2c_adapter *adap) +static void twsi_reset(struct mvtwsi_registers *twsi) { - struct mvtwsi_registers *twsi = twsi_get_base(adap); - /* Reset controller */ writel(0, &twsi->soft_reset); /* Wait 2 ms -- this is what the Marvell LSP does */ @@ -341,10 +333,9 @@ static void twsi_reset(struct i2c_adapter *adap) /* * Sets baud to the highest possible value not exceeding the requested one. */ -static uint __twsi_i2c_set_bus_speed(struct i2c_adapter *adap, +static uint __twsi_i2c_set_bus_speed(struct mvtwsi_registers *twsi, uint requested_speed) { - struct mvtwsi_registers *twsi = twsi_get_base(adap); uint tmp_speed, highest_speed, n, m; uint baud = 0x44; /* Baud rate after controller reset */
@@ -366,26 +357,25 @@ static uint __twsi_i2c_set_bus_speed(struct i2c_adapter *adap, return 0; }
-static void __twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd) +static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed, + int slaveadd) { - struct mvtwsi_registers *twsi = twsi_get_base(adap); - /* Reset controller */ - twsi_reset(adap); + twsi_reset(twsi); /* Set speed */ - __twsi_i2c_set_bus_speed(adap, speed); + __twsi_i2c_set_bus_speed(twsi, speed); /* Set slave address; even though we don't use it */ writel(slaveadd, &twsi->slave_address); writel(0, &twsi->xtnd_slave_addr); /* Assert STOP, but don't care for the result */ - (void) twsi_stop(adap); + (void) twsi_stop(twsi); }
/* * Begin I2C transaction with expected start status, at given address. * Expected address status will derive from direction bit (bit 0) in addr. */ -static int i2c_begin(struct i2c_adapter *adap, int expected_start_status, +static int i2c_begin(struct mvtwsi_registers *twsi, int expected_start_status, u8 addr) { int status, expected_addr_status; @@ -397,10 +387,10 @@ static int i2c_begin(struct i2c_adapter *adap, int expected_start_status, else /* Writing */ expected_addr_status = MVTWSI_STATUS_ADDR_W_ACK; /* Assert START */ - status = twsi_start(adap, expected_start_status); + status = twsi_start(twsi, expected_start_status); /* Send out the address if the start went well */ if (status == 0) - status = twsi_send(adap, addr, expected_addr_status); + status = twsi_send(twsi, addr, expected_addr_status); /* Return 0, or the status of the first failure */ return status; } @@ -408,18 +398,18 @@ static int i2c_begin(struct i2c_adapter *adap, int expected_start_status, /* * Begin read, nak data byte, end. */ -static int __twsi_i2c_probe_chip(struct i2c_adapter *adap, uchar chip) +static int __twsi_i2c_probe_chip(struct mvtwsi_registers *twsi, uchar chip) { u8 dummy_byte; int status;
/* Begin i2c read */ - status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1) | 1); + status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1) | 1); /* Dummy read was accepted: receive byte, but NAK it. */ if (status == 0) - status = twsi_recv(adap, &dummy_byte, MVTWSI_READ_NAK); + status = twsi_recv(twsi, &dummy_byte, MVTWSI_READ_NAK); /* Stop transaction */ - twsi_stop(adap); + twsi_stop(twsi); /* Return 0, or the status of the first failure */ return status; } @@ -432,30 +422,30 @@ static int __twsi_i2c_probe_chip(struct i2c_adapter *adap, uchar chip) * higher level APIs, we need to make a decision here, and for the moment that * will be a repeated start without a preceding stop. */ -static int __twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, - int alen, uchar *data, int length) +static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip, + uint addr, int alen, uchar *data, int length) { int status = 0; int stop_status;
/* Begin i2c write to send the address bytes */ - status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1)); + status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1)); /* Send address bytes */ while ((status == 0) && alen--) - status = twsi_send(adap, addr >> (8*alen), + status = twsi_send(twsi, addr >> (8*alen), MVTWSI_STATUS_DATA_W_ACK); /* Begin i2c read to receive data bytes */ if (status == 0) - status = i2c_begin(adap, MVTWSI_STATUS_REPEATED_START, + status = i2c_begin(twsi, MVTWSI_STATUS_REPEATED_START, (chip << 1) | 1); /* Receive actual data bytes; set NAK if we if we have nothing more to * read */ while ((status == 0) && length--) - status = twsi_recv(adap, data++, + status = twsi_recv(twsi, data++, length > 0 ? MVTWSI_READ_ACK : MVTWSI_READ_NAK); /* Stop transaction */ - stop_status = twsi_stop(adap); + stop_status = twsi_stop(twsi); /* Return 0, or the status of the first failure */ return status != 0 ? status : stop_status; } @@ -463,23 +453,23 @@ static int __twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, /* * Begin write, send address byte(s), send data bytes, end. */ -static int __twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, - int alen, uchar *data, int length) +static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip, + uint addr, int alen, uchar *data, int length) { int status, stop_status;
/* Begin i2c write to send first the address bytes, then the * data bytes */ - status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1)); + status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1)); /* Send address bytes */ while ((status == 0) && alen--) - status = twsi_send(adap, addr >> (8*alen), + status = twsi_send(twsi, addr >> (8*alen), MVTWSI_STATUS_DATA_W_ACK); /* Send data bytes */ while ((status == 0) && (length-- > 0)) - status = twsi_send(adap, *(data++), MVTWSI_STATUS_DATA_W_ACK); + status = twsi_send(twsi, *(data++), MVTWSI_STATUS_DATA_W_ACK); /* Stop transaction */ - stop_status = twsi_stop(adap); + stop_status = twsi_stop(twsi); /* Return 0, or the status of the first failure */ return status != 0 ? status : stop_status; } @@ -487,30 +477,35 @@ static int __twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, static void twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd) { - __twsi_i2c_init(adap, speed, slaveadd); + struct mvtwsi_registers *twsi = twsi_get_base(adap); + __twsi_i2c_init(twsi, speed, slaveadd); }
static uint twsi_i2c_set_bus_speed(struct i2c_adapter *adap, uint requested_speed) { - return __twsi_i2c_set_bus_speed(adap, requested_speed); + struct mvtwsi_registers *twsi = twsi_get_base(adap); + return __twsi_i2c_set_bus_speed(twsi, requested_speed); }
static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip) { - return __twsi_i2c_probe_chip(adap, chip); + struct mvtwsi_registers *twsi = twsi_get_base(adap); + return __twsi_i2c_probe_chip(twsi, chip); }
static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, int alen, uchar *data, int length) { - return __twsi_i2c_read(adap, chip, addr, alen, data, length); + struct mvtwsi_registers *twsi = twsi_get_base(adap); + return __twsi_i2c_read(twsi, chip, addr, alen, data, length); }
static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, int alen, uchar *data, int length) { - return __twsi_i2c_write(adap, chip, addr, alen, data, length); + struct mvtwsi_registers *twsi = twsi_get_base(adap); + return __twsi_i2c_write(twsi, chip, addr, alen, data, length); }
#ifdef CONFIG_I2C_MVTWSI_BASE0

The length of the address parameter of the __twsi_i2c_read and __twsi_i2c_write functions is fixed to four bytes.
As a final step in the preparation of the DM conversion, we make the length of this parameter variable by turning it into an array of bytes, and convert the 32 byte value that's passed to the legacy functions into a four-byte-array on the fly.
Signed-off-by: Mario Six mario.six@gdsys.cc --- drivers/i2c/mvtwsi.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index 688efc2..3325c4b 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -423,7 +423,7 @@ static int __twsi_i2c_probe_chip(struct mvtwsi_registers *twsi, uchar chip) * will be a repeated start without a preceding stop. */ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip, - uint addr, int alen, uchar *data, int length) + u8 *addr, int alen, uchar *data, int length) { int status = 0; int stop_status; @@ -432,8 +432,7 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip, status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1)); /* Send address bytes */ while ((status == 0) && alen--) - status = twsi_send(twsi, addr >> (8*alen), - MVTWSI_STATUS_DATA_W_ACK); + status = twsi_send(twsi, *(addr++), MVTWSI_STATUS_DATA_W_ACK); /* Begin i2c read to receive data bytes */ if (status == 0) status = i2c_begin(twsi, MVTWSI_STATUS_REPEATED_START, @@ -454,7 +453,7 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip, * Begin write, send address byte(s), send data bytes, end. */ static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip, - uint addr, int alen, uchar *data, int length) + u8 *addr, int alen, uchar *data, int length) { int status, stop_status;
@@ -462,9 +461,8 @@ static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip, * data bytes */ status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1)); /* Send address bytes */ - while ((status == 0) && alen--) - status = twsi_send(twsi, addr >> (8*alen), - MVTWSI_STATUS_DATA_W_ACK); + while ((status == 0) && (alen-- > 0)) + status = twsi_send(twsi, *(addr++), MVTWSI_STATUS_DATA_W_ACK); /* Send data bytes */ while ((status == 0) && (length-- > 0)) status = twsi_send(twsi, *(data++), MVTWSI_STATUS_DATA_W_ACK); @@ -498,14 +496,28 @@ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, int alen, uchar *data, int length) { struct mvtwsi_registers *twsi = twsi_get_base(adap); - return __twsi_i2c_read(twsi, chip, addr, alen, data, length); + u8 addr_bytes[4]; + + addr_bytes[0] = (addr >> 0) & 0xFF; + addr_bytes[1] = (addr >> 8) & 0xFF; + addr_bytes[2] = (addr >> 16) & 0xFF; + addr_bytes[3] = (addr >> 24) & 0xFF; + + return __twsi_i2c_read(twsi, chip, addr_bytes, alen, data, length); }
static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, int alen, uchar *data, int length) { struct mvtwsi_registers *twsi = twsi_get_base(adap); - return __twsi_i2c_write(twsi, chip, addr, alen, data, length); + u8 addr_bytes[4]; + + addr_bytes[0] = (addr >> 0) & 0xFF; + addr_bytes[1] = (addr >> 8) & 0xFF; + addr_bytes[2] = (addr >> 16) & 0xFF; + addr_bytes[3] = (addr >> 24) & 0xFF; + + return __twsi_i2c_write(twsi, chip, addr_bytes, alen, data, length); }
#ifdef CONFIG_I2C_MVTWSI_BASE0

This patch adds the necessary functions and Kconfig entry to make the MVTWSI I2C driver compatible with the driver model.
A possible device tree entry might look like this:
i2c@11100 { compatible = "marvell,mv64xxx-i2c"; reg = <0x11000 0x20>; clock-frequency = <100000>; u-boot,i2c-slave-addr = <0x0>; };
Signed-off-by: Mario Six mario.six@gdsys.cc --- drivers/i2c/Kconfig | 7 +++ drivers/i2c/mvtwsi.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 123 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index 6e22bba..b3e8405 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -154,6 +154,13 @@ config SYS_I2C_UNIPHIER_F Support for UniPhier FIFO-builtin I2C controller driver. This I2C controller is used on PH1-Pro4 or newer UniPhier SoCs.
+config SYS_I2C_MVTWSI + bool "Marvell I2C driver" + depends on DM_I2C + help + Support for Marvell I2C controllers as used on the orion5x and + kirkwood SoC families. + source "drivers/i2c/muxes/Kconfig"
endmenu diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index 3325c4b..c81e9d4 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -12,12 +12,19 @@ #include <i2c.h> #include <asm/errno.h> #include <asm/io.h> +#ifdef CONFIG_DM_I2C +#include <dm.h> +#include <mapmem.h> +#endif + +DECLARE_GLOBAL_DATA_PTR;
/* * Include a file that will provide CONFIG_I2C_MVTWSI_BASE*, and possibly other * settings */
+#ifndef CONFIG_DM_I2C #if defined(CONFIG_ORION5X) #include <asm/arch/orion5x.h> #elif (defined(CONFIG_KIRKWOOD) || defined(CONFIG_ARCH_MVEBU)) @@ -27,6 +34,7 @@ #else #error Driver mvtwsi not supported by SoC or board #endif +#endif /* CONFIG_DM_I2C */
/* * TWSI register structure @@ -61,6 +69,19 @@ struct mvtwsi_registers {
#endif
+#ifdef CONFIG_DM_I2C +struct mvtwsi_i2c_dev { + /* TWSI Register base for the device */ + struct mvtwsi_registers *base; + /* Number of the device (determined from cell-index property) */ + int index; + /* The I2C slave address for the device */ + u8 slaveadd; + /* The configured I2C speed in Hz */ + uint speed; +}; +#endif /* CONFIG_DM_I2C */ + /* * enum mvtwsi_ctrl_register_fields - Bit masks for flags in the control * register @@ -134,6 +155,7 @@ enum mvtwsi_ack_flags { MVTWSI_READ_ACK = 1, };
+#ifndef CONFIG_DM_I2C /* * MVTWSI controller base */ @@ -172,6 +194,7 @@ static struct mvtwsi_registers *twsi_get_base(struct i2c_adapter *adap)
return NULL; } +#endif
/* * enum mvtwsi_error_class - types of I2C errors @@ -358,7 +381,7 @@ static uint __twsi_i2c_set_bus_speed(struct mvtwsi_registers *twsi, }
static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed, - int slaveadd) + int slaveadd) { /* Reset controller */ twsi_reset(twsi); @@ -472,6 +495,7 @@ static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip, return status != 0 ? status : stop_status; }
+#ifndef CONFIG_DM_I2C static void twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd) { @@ -561,3 +585,94 @@ U_BOOT_I2C_ADAP_COMPLETE(twsi5, twsi_i2c_init, twsi_i2c_probe, CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE, 5)
#endif +#else /* CONFIG_DM_I2C */ + +static int mvtwsi_i2c_probe_chip(struct udevice *bus, u32 chip_addr, + u32 chip_flags) +{ + struct mvtwsi_i2c_dev *dev = dev_get_priv(bus); + return __twsi_i2c_probe_chip(dev->base, chip_addr); +} + +static int mvtwsi_i2c_set_bus_speed(struct udevice *bus, uint speed) +{ + struct mvtwsi_i2c_dev *dev = dev_get_priv(bus); + return __twsi_i2c_set_bus_speed(dev->base, speed); +} + +static int mvtwsi_i2c_ofdata_to_platdata(struct udevice *bus) +{ + struct mvtwsi_i2c_dev *dev = dev_get_priv(bus); + fdt_addr_t addr; + fdt_size_t size; + + addr = fdtdec_get_addr_size_auto_noparent(gd->fdt_blob, bus->of_offset, + "reg", 0, &size); + + dev->base = map_sysmem(SOC_REGS_PHY_BASE + addr, size); + + if (!dev->base) + return -ENOMEM; + + dev->index = fdtdec_get_int(gd->fdt_blob, bus->of_offset, + "cell-index", -1); + dev->slaveadd = fdtdec_get_int(gd->fdt_blob, bus->of_offset, + "u-boot,i2c-slave-addr", 0x0); + dev->speed = fdtdec_get_int(gd->fdt_blob, bus->of_offset, + "clock-frequency", 100000); + return 0; +} + +static int mvtwsi_i2c_probe(struct udevice *bus) +{ + struct mvtwsi_i2c_dev *dev = dev_get_priv(bus); + __twsi_i2c_init(dev->base, dev->speed, dev->slaveadd); + return 0; +} + +static int mvtwsi_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, int nmsgs) +{ + struct mvtwsi_i2c_dev *dev = dev_get_priv(bus); + struct i2c_msg *dmsg, *omsg, dummy; + + memset(&dummy, 0, sizeof(struct i2c_msg)); + + /* We expect either two messages (one with an offset and one with the + * actual data) or one message (just data or offset/data combined) */ + if (nmsgs > 2 || nmsgs == 0) { + debug("%s: Only one or two messages are supported.", __func__); + return -1; + } + + omsg = nmsgs == 1 ? &dummy : msg; + dmsg = nmsgs == 1 ? msg : msg + 1; + + if (dmsg->flags & I2C_M_RD) + return __twsi_i2c_read(dev->base, dmsg->addr, omsg->buf, + omsg->len, dmsg->buf, dmsg->len); + else + return __twsi_i2c_write(dev->base, dmsg->addr, omsg->buf, + omsg->len, dmsg->buf, dmsg->len); +} + +static const struct dm_i2c_ops mvtwsi_i2c_ops = { + .xfer = mvtwsi_i2c_xfer, + .probe_chip = mvtwsi_i2c_probe_chip, + .set_bus_speed = mvtwsi_i2c_set_bus_speed, +}; + +static const struct udevice_id mvtwsi_i2c_ids[] = { + { .compatible = "marvell,mv64xxx-i2c", }, + { /* sentinel */ } +}; + +U_BOOT_DRIVER(i2c_mvtwsi) = { + .name = "i2c_mvtwsi", + .id = UCLASS_I2C, + .of_match = mvtwsi_i2c_ids, + .probe = mvtwsi_i2c_probe, + .ofdata_to_platdata = mvtwsi_i2c_ofdata_to_platdata, + .priv_auto_alloc_size = sizeof(struct mvtwsi_i2c_dev), + .ops = &mvtwsi_i2c_ops, +}; +#endif /* CONFIG_DM_I2C */

Hi Mario,
first, thanks for this very nice patch series. Really appreciated. One comment below...
On 18.07.2016 10:27, Mario Six wrote:
This patch adds the necessary functions and Kconfig entry to make the MVTWSI I2C driver compatible with the driver model.
A possible device tree entry might look like this:
i2c@11100 { compatible = "marvell,mv64xxx-i2c"; reg = <0x11000 0x20>; clock-frequency = <100000>; u-boot,i2c-slave-addr = <0x0>; };
Signed-off-by: Mario Six mario.six@gdsys.cc
drivers/i2c/Kconfig | 7 +++ drivers/i2c/mvtwsi.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 123 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index 6e22bba..b3e8405 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -154,6 +154,13 @@ config SYS_I2C_UNIPHIER_F Support for UniPhier FIFO-builtin I2C controller driver. This I2C controller is used on PH1-Pro4 or newer UniPhier SoCs.
+config SYS_I2C_MVTWSI
- bool "Marvell I2C driver"
- depends on DM_I2C
- help
Support for Marvell I2C controllers as used on the orion5x and
kirkwood SoC families.
source "drivers/i2c/muxes/Kconfig"
endmenu diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index 3325c4b..c81e9d4 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -12,12 +12,19 @@ #include <i2c.h> #include <asm/errno.h> #include <asm/io.h> +#ifdef CONFIG_DM_I2C +#include <dm.h> +#include <mapmem.h> +#endif
+DECLARE_GLOBAL_DATA_PTR;
/*
- Include a file that will provide CONFIG_I2C_MVTWSI_BASE*, and possibly other
- settings
*/
+#ifndef CONFIG_DM_I2C #if defined(CONFIG_ORION5X) #include <asm/arch/orion5x.h> #elif (defined(CONFIG_KIRKWOOD) || defined(CONFIG_ARCH_MVEBU)) @@ -27,6 +34,7 @@ #else #error Driver mvtwsi not supported by SoC or board #endif +#endif /* CONFIG_DM_I2C */
/*
- TWSI register structure
@@ -61,6 +69,19 @@ struct mvtwsi_registers {
#endif
+#ifdef CONFIG_DM_I2C +struct mvtwsi_i2c_dev {
- /* TWSI Register base for the device */
- struct mvtwsi_registers *base;
- /* Number of the device (determined from cell-index property) */
- int index;
- /* The I2C slave address for the device */
- u8 slaveadd;
- /* The configured I2C speed in Hz */
- uint speed;
+}; +#endif /* CONFIG_DM_I2C */
/*
- enum mvtwsi_ctrl_register_fields - Bit masks for flags in the control
- register
@@ -134,6 +155,7 @@ enum mvtwsi_ack_flags { MVTWSI_READ_ACK = 1, };
+#ifndef CONFIG_DM_I2C /*
- MVTWSI controller base
*/ @@ -172,6 +194,7 @@ static struct mvtwsi_registers *twsi_get_base(struct i2c_adapter *adap)
return NULL; } +#endif
/*
- enum mvtwsi_error_class - types of I2C errors
@@ -358,7 +381,7 @@ static uint __twsi_i2c_set_bus_speed(struct mvtwsi_registers *twsi, }
static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed,
int slaveadd)
int slaveadd)
{ /* Reset controller */ twsi_reset(twsi); @@ -472,6 +495,7 @@ static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip, return status != 0 ? status : stop_status; }
+#ifndef CONFIG_DM_I2C static void twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd) { @@ -561,3 +585,94 @@ U_BOOT_I2C_ADAP_COMPLETE(twsi5, twsi_i2c_init, twsi_i2c_probe, CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE, 5)
#endif +#else /* CONFIG_DM_I2C */
+static int mvtwsi_i2c_probe_chip(struct udevice *bus, u32 chip_addr,
u32 chip_flags)
+{
- struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
- return __twsi_i2c_probe_chip(dev->base, chip_addr);
+}
+static int mvtwsi_i2c_set_bus_speed(struct udevice *bus, uint speed) +{
- struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
- return __twsi_i2c_set_bus_speed(dev->base, speed);
+}
+static int mvtwsi_i2c_ofdata_to_platdata(struct udevice *bus) +{
- struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
- fdt_addr_t addr;
- fdt_size_t size;
- addr = fdtdec_get_addr_size_auto_noparent(gd->fdt_blob, bus->of_offset,
"reg", 0, &size);
- dev->base = map_sysmem(SOC_REGS_PHY_BASE + addr, size);
- if (!dev->base)
return -ENOMEM;
SOC_REGS_PHY_BASE is only defined for MVEBU / Armada XP / 38x. You should use dev_get_addr() instead here. This will do all the address translation you need:
addr = dev_get_addr(bus);
Or if you need a ptr:
dev->base = dev_get_addr_ptr(bus);
Otherwise your patch serial look good, so please add my:
Reviewed-by: Stefan Roese sr@denx.de
to all the patches.
Thanks, Stefan

Hi Stefan,
On Thu, Jul 21, 2016 at 9:05 AM, Stefan Roese sr@denx.de wrote:
Hi Mario,
first, thanks for this very nice patch series. Really appreciated. One comment below...
On 18.07.2016 10:27, Mario Six wrote:
This patch adds the necessary functions and Kconfig entry to make the MVTWSI I2C driver compatible with the driver model.
A possible device tree entry might look like this:
i2c@11100 { compatible = "marvell,mv64xxx-i2c"; reg = <0x11000 0x20>; clock-frequency = <100000>; u-boot,i2c-slave-addr = <0x0>; };
Signed-off-by: Mario Six mario.six@gdsys.cc
drivers/i2c/Kconfig | 7 +++ drivers/i2c/mvtwsi.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 123 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index 6e22bba..b3e8405 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -154,6 +154,13 @@ config SYS_I2C_UNIPHIER_F Support for UniPhier FIFO-builtin I2C controller driver. This I2C controller is used on PH1-Pro4 or newer UniPhier SoCs.
+config SYS_I2C_MVTWSI
bool "Marvell I2C driver"
depends on DM_I2C
help
Support for Marvell I2C controllers as used on the orion5x and
kirkwood SoC families.
source "drivers/i2c/muxes/Kconfig"
endmenu diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index 3325c4b..c81e9d4 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -12,12 +12,19 @@ #include <i2c.h> #include <asm/errno.h> #include <asm/io.h> +#ifdef CONFIG_DM_I2C +#include <dm.h> +#include <mapmem.h> +#endif
+DECLARE_GLOBAL_DATA_PTR;
/*
- Include a file that will provide CONFIG_I2C_MVTWSI_BASE*, and possibly
other
- settings
*/
+#ifndef CONFIG_DM_I2C #if defined(CONFIG_ORION5X) #include <asm/arch/orion5x.h> #elif (defined(CONFIG_KIRKWOOD) || defined(CONFIG_ARCH_MVEBU)) @@ -27,6 +34,7 @@ #else #error Driver mvtwsi not supported by SoC or board #endif +#endif /* CONFIG_DM_I2C */
/*
- TWSI register structure
@@ -61,6 +69,19 @@ struct mvtwsi_registers {
#endif
+#ifdef CONFIG_DM_I2C +struct mvtwsi_i2c_dev {
/* TWSI Register base for the device */
struct mvtwsi_registers *base;
/* Number of the device (determined from cell-index property) */
int index;
/* The I2C slave address for the device */
u8 slaveadd;
/* The configured I2C speed in Hz */
uint speed;
+}; +#endif /* CONFIG_DM_I2C */
/*
- enum mvtwsi_ctrl_register_fields - Bit masks for flags in the control
- register
@@ -134,6 +155,7 @@ enum mvtwsi_ack_flags { MVTWSI_READ_ACK = 1, };
+#ifndef CONFIG_DM_I2C /*
- MVTWSI controller base
*/ @@ -172,6 +194,7 @@ static struct mvtwsi_registers *twsi_get_base(struct i2c_adapter *adap)
return NULL;
} +#endif
/*
- enum mvtwsi_error_class - types of I2C errors
@@ -358,7 +381,7 @@ static uint __twsi_i2c_set_bus_speed(struct mvtwsi_registers *twsi, }
static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed,
int slaveadd)
int slaveadd)
{ /* Reset controller */ twsi_reset(twsi); @@ -472,6 +495,7 @@ static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip, return status != 0 ? status : stop_status; }
+#ifndef CONFIG_DM_I2C static void twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd) { @@ -561,3 +585,94 @@ U_BOOT_I2C_ADAP_COMPLETE(twsi5, twsi_i2c_init, twsi_i2c_probe, CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE, 5)
#endif +#else /* CONFIG_DM_I2C */
+static int mvtwsi_i2c_probe_chip(struct udevice *bus, u32 chip_addr,
u32 chip_flags)
+{
struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
return __twsi_i2c_probe_chip(dev->base, chip_addr);
+}
+static int mvtwsi_i2c_set_bus_speed(struct udevice *bus, uint speed) +{
struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
return __twsi_i2c_set_bus_speed(dev->base, speed);
+}
+static int mvtwsi_i2c_ofdata_to_platdata(struct udevice *bus) +{
struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
fdt_addr_t addr;
fdt_size_t size;
addr = fdtdec_get_addr_size_auto_noparent(gd->fdt_blob,
bus->of_offset,
"reg", 0, &size);
dev->base = map_sysmem(SOC_REGS_PHY_BASE + addr, size);
if (!dev->base)
return -ENOMEM;
SOC_REGS_PHY_BASE is only defined for MVEBU / Armada XP / 38x. You should use dev_get_addr() instead here. This will do all the address translation you need:
addr = dev_get_addr(bus);
Or if you need a ptr:
dev->base = dev_get_addr_ptr(bus);
Ah, yes, I just copied that from the MVEBU_REGISTER macro to get it working, and never went back to clean it up properly.
Will fix in v2.
Otherwise your patch serial look good, so please add my:
Reviewed-by: Stefan Roese sr@denx.de
to all the patches.
Thanks for reviewing!
Thanks, Stefan
Best regards,
Mario

Zero-length offsets are not properly handled by the driver. When a read operation with a zero-length offset is started, a START condition is asserted, and since no offset bytes are transferred, a repeated START is issued immediately after, which confuses the controller.
To fix this, we send the first START only if any address bytes need to be sent, and keep track of the expected start status accordingly.
Signed-off-by: Mario Six mario.six@gdsys.cc --- drivers/i2c/mvtwsi.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index c81e9d4..11b5a5c 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -450,16 +450,21 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip, { int status = 0; int stop_status; - - /* Begin i2c write to send the address bytes */ - status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1)); - /* Send address bytes */ - while ((status == 0) && alen--) - status = twsi_send(twsi, *(addr++), MVTWSI_STATUS_DATA_W_ACK); + int expected_start = MVTWSI_STATUS_START; + + if (alen > 0) { + /* Begin i2c write to send the address bytes */ + status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1)); + /* Send address bytes */ + while ((status == 0) && alen--) + status = twsi_send(twsi, *(addr++), + MVTWSI_STATUS_DATA_W_ACK); + /* Send repeated STARTs after the initial START */ + expected_start = MVTWSI_STATUS_REPEATED_START; + } /* Begin i2c read to receive data bytes */ if (status == 0) - status = i2c_begin(twsi, MVTWSI_STATUS_REPEATED_START, - (chip << 1) | 1); + status = i2c_begin(twsi, expected_start, (chip << 1) | 1, tick); /* Receive actual data bytes; set NAK if we if we have nothing more to * read */ while ((status == 0) && length--)

Some devices using the MVTWSI driver have the option to run at speeds faster than Standard Mode (100kHZ). On the Armada 38x controllers, this is actually necessary, since due to erratum FE-8471889, a timing violation concerning repeated starts prevents the controller from working correctly in Standard Mode. One of the workarounds recommended in the erratum is to set the bus to Fast Mode (400kHZ) operation and ensure all connected devices are set to Fast Mode.
In the current version of the driver, however, the delay times are hard-coded to 10ms, corresponding to Standard Mode operation. To take full advantage of the faster modes, we would need to either keep the currently configured I2C speed in a globally accessible variable, or pass it to the necessary functions as a parameter. For DM, the first option is not a problem, and we can simply keep the speed in the private data of the driver. For the legacy interface, however, we would need to introduce a static variable, which would cause problems with boots from NOR flashes; see commit d6b7757 "i2c: mvtwsi: Eliminate twsi_control_flags."
As to not clutter the interface with yet another parameter, we therefore keep the default 10ms delays for the legacy functions.
In DM mode, we make the delay time dependant on the frequency to allow taking full advantage of faster modes of operation (tested with up to 1MHZ frequency on Armada MV88F6820).
Signed-off-by: Mario Six mario.six@gdsys.cc --- drivers/i2c/mvtwsi.c | 125 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 83 insertions(+), 42 deletions(-)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index 11b5a5c..dd42a3c 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -79,6 +79,8 @@ struct mvtwsi_i2c_dev { u8 slaveadd; /* The configured I2C speed in Hz */ uint speed; + /* The current length of a clock period (depending on speed) */ + uint tick; }; #endif /* CONFIG_DM_I2C */
@@ -155,7 +157,15 @@ enum mvtwsi_ack_flags { MVTWSI_READ_ACK = 1, };
+inline uint calc_tick(uint speed) +{ + /* One tick = the duration of a period at the specified speed in ns (we + * add 100 ns to be on the safe side) */ + return (1000000000u / speed) + 100; +} + #ifndef CONFIG_DM_I2C + /* * MVTWSI controller base */ @@ -231,7 +241,8 @@ inline uint mvtwsi_error(uint ec, uint lc, uint ls, uint es) * Wait for IFLG to raise, or return 'timeout.' Then, if the status is as * expected, return 0 (ok) or 'wrong status' otherwise. */ -static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status) +static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status, + uint tick) { int control, status; int timeout = 1000; @@ -247,7 +258,7 @@ static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status) MVTWSI_ERROR_WRONG_STATUS, control, status, expected_status); } - udelay(10); /* One clock cycle at 100 kHz */ + ndelay(tick); /* One clock cycle */ } while (timeout--); status = readl(&twsi->status); return mvtwsi_error(MVTWSI_ERROR_TIMEOUT, control, status, @@ -258,20 +269,21 @@ static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status) * Assert the START condition, either in a single I2C transaction * or inside back-to-back ones (repeated starts). */ -static int twsi_start(struct mvtwsi_registers *twsi, int expected_status) +static int twsi_start(struct mvtwsi_registers *twsi, int expected_status, + uint tick) { /* Assert START */ writel(MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_START | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); /* Wait for controller to process START */ - return twsi_wait(twsi, expected_status); + return twsi_wait(twsi, expected_status, tick); }
/* * Send a byte (i2c address or data). */ static int twsi_send(struct mvtwsi_registers *twsi, u8 byte, - int expected_status) + int expected_status, uint tick) { /* Write byte to data register for sending */ writel(byte, &twsi->data); @@ -279,13 +291,14 @@ static int twsi_send(struct mvtwsi_registers *twsi, u8 byte, writel(MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); /* Wait for controller to receive byte, and check ACK */ - return twsi_wait(twsi, expected_status); + return twsi_wait(twsi, expected_status, tick); }
/* * Receive a byte. */ -static int twsi_recv(struct mvtwsi_registers *twsi, u8 *byte, int ack_flag) +static int twsi_recv(struct mvtwsi_registers *twsi, u8 *byte, int ack_flag, + uint tick) { int expected_status, status, control;
@@ -297,7 +310,7 @@ static int twsi_recv(struct mvtwsi_registers *twsi, u8 *byte, int ack_flag) control |= ack_flag == MVTWSI_READ_ACK ? MVTWSI_CONTROL_ACK : 0; writel(control | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control); /* Wait for controller to receive byte, and assert ACK or NAK */ - status = twsi_wait(twsi, expected_status); + status = twsi_wait(twsi, expected_status, tick); /* If we did receive the expected byte, store it */ if (status == 0) *byte = readl(&twsi->data); @@ -308,7 +321,7 @@ static int twsi_recv(struct mvtwsi_registers *twsi, u8 *byte, int ack_flag) * Assert the STOP condition. * This is also used to force the bus back to idle (SDA = SCL = 1). */ -static int twsi_stop(struct mvtwsi_registers *twsi) +static int twsi_stop(struct mvtwsi_registers *twsi, uint tick) { int control, stop_status; int status = 0; @@ -322,7 +335,7 @@ static int twsi_stop(struct mvtwsi_registers *twsi) stop_status = readl(&twsi->status); if (stop_status == MVTWSI_STATUS_IDLE) break; - udelay(10); /* One clock cycle at 100 kHz */ + ndelay(tick); /* One clock cycle */ } while (timeout--); control = readl(&twsi->control); if (stop_status != MVTWSI_STATUS_IDLE) @@ -377,21 +390,32 @@ static uint __twsi_i2c_set_bus_speed(struct mvtwsi_registers *twsi, } } writel(baud, &twsi->baudrate); - return 0; + + /* Wait for controller for one tick */ +#ifdef CONFIG_DM_I2C + ndelay(calc_tick(highest_speed)); +#else + ndelay(10000); +#endif + return highest_speed; }
static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed, - int slaveadd) + int slaveadd, uint *actual_speed) { /* Reset controller */ twsi_reset(twsi); /* Set speed */ - __twsi_i2c_set_bus_speed(twsi, speed); + *actual_speed = __twsi_i2c_set_bus_speed(twsi, speed); /* Set slave address; even though we don't use it */ writel(slaveadd, &twsi->slave_address); writel(0, &twsi->xtnd_slave_addr); /* Assert STOP, but don't care for the result */ - (void) twsi_stop(twsi); +#ifdef CONFIG_DM_I2C + (void) twsi_stop(twsi, calc_tick(*actual_speed)); +#else + (void) twsi_stop(twsi, 10000); +#endif }
/* @@ -399,7 +423,7 @@ static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed, * Expected address status will derive from direction bit (bit 0) in addr. */ static int i2c_begin(struct mvtwsi_registers *twsi, int expected_start_status, - u8 addr) + u8 addr, uint tick) { int status, expected_addr_status;
@@ -410,10 +434,10 @@ static int i2c_begin(struct mvtwsi_registers *twsi, int expected_start_status, else /* Writing */ expected_addr_status = MVTWSI_STATUS_ADDR_W_ACK; /* Assert START */ - status = twsi_start(twsi, expected_start_status); + status = twsi_start(twsi, expected_start_status, tick); /* Send out the address if the start went well */ if (status == 0) - status = twsi_send(twsi, addr, expected_addr_status); + status = twsi_send(twsi, addr, expected_addr_status, tick); /* Return 0, or the status of the first failure */ return status; } @@ -421,18 +445,19 @@ static int i2c_begin(struct mvtwsi_registers *twsi, int expected_start_status, /* * Begin read, nak data byte, end. */ -static int __twsi_i2c_probe_chip(struct mvtwsi_registers *twsi, uchar chip) +static int __twsi_i2c_probe_chip(struct mvtwsi_registers *twsi, uchar chip, + uint tick) { u8 dummy_byte; int status;
/* Begin i2c read */ - status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1) | 1); + status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1) | 1, tick); /* Dummy read was accepted: receive byte, but NAK it. */ if (status == 0) - status = twsi_recv(twsi, &dummy_byte, MVTWSI_READ_NAK); + status = twsi_recv(twsi, &dummy_byte, MVTWSI_READ_NAK, tick); /* Stop transaction */ - twsi_stop(twsi); + twsi_stop(twsi, tick); /* Return 0, or the status of the first failure */ return status; } @@ -446,7 +471,8 @@ static int __twsi_i2c_probe_chip(struct mvtwsi_registers *twsi, uchar chip) * will be a repeated start without a preceding stop. */ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip, - u8 *addr, int alen, uchar *data, int length) + u8 *addr, int alen, uchar *data, int length, + uint tick) { int status = 0; int stop_status; @@ -454,11 +480,11 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip,
if (alen > 0) { /* Begin i2c write to send the address bytes */ - status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1)); + status = i2c_begin(twsi, expected_start, (chip << 1), tick); /* Send address bytes */ while ((status == 0) && alen--) status = twsi_send(twsi, *(addr++), - MVTWSI_STATUS_DATA_W_ACK); + MVTWSI_STATUS_DATA_W_ACK, tick); /* Send repeated STARTs after the initial START */ expected_start = MVTWSI_STATUS_REPEATED_START; } @@ -470,9 +496,9 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip, while ((status == 0) && length--) status = twsi_recv(twsi, data++, length > 0 ? - MVTWSI_READ_ACK : MVTWSI_READ_NAK); + MVTWSI_READ_ACK : MVTWSI_READ_NAK, tick); /* Stop transaction */ - stop_status = twsi_stop(twsi); + stop_status = twsi_stop(twsi, tick); /* Return 0, or the status of the first failure */ return status != 0 ? status : stop_status; } @@ -481,21 +507,23 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip, * Begin write, send address byte(s), send data bytes, end. */ static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip, - u8 *addr, int alen, uchar *data, int length) + u8 *addr, int alen, uchar *data, int length, + uint tick) { int status, stop_status;
/* Begin i2c write to send first the address bytes, then the * data bytes */ - status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1)); + status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1), tick); /* Send address bytes */ while ((status == 0) && (alen-- > 0)) - status = twsi_send(twsi, *(addr++), MVTWSI_STATUS_DATA_W_ACK); - /* Send data bytes */ + status = twsi_send(twsi, *(addr++), MVTWSI_STATUS_DATA_W_ACK, + tick); while ((status == 0) && (length-- > 0)) - status = twsi_send(twsi, *(data++), MVTWSI_STATUS_DATA_W_ACK); + status = twsi_send(twsi, *(data++), MVTWSI_STATUS_DATA_W_ACK, + tick); /* Stop transaction */ - stop_status = twsi_stop(twsi); + stop_status = twsi_stop(twsi, tick); /* Return 0, or the status of the first failure */ return status != 0 ? status : stop_status; } @@ -505,20 +533,21 @@ static void twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd) { struct mvtwsi_registers *twsi = twsi_get_base(adap); - __twsi_i2c_init(twsi, speed, slaveadd); + __twsi_i2c_init(twsi, speed, slaveadd, NULL); }
static uint twsi_i2c_set_bus_speed(struct i2c_adapter *adap, uint requested_speed) { struct mvtwsi_registers *twsi = twsi_get_base(adap); - return __twsi_i2c_set_bus_speed(twsi, requested_speed); + __twsi_i2c_set_bus_speed(twsi, requested_speed); + return 0; }
static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip) { struct mvtwsi_registers *twsi = twsi_get_base(adap); - return __twsi_i2c_probe_chip(twsi, chip); + return __twsi_i2c_probe_chip(twsi, chip, 10000); }
static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, @@ -532,7 +561,8 @@ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, addr_bytes[2] = (addr >> 16) & 0xFF; addr_bytes[3] = (addr >> 24) & 0xFF;
- return __twsi_i2c_read(twsi, chip, addr_bytes, alen, data, length); + return __twsi_i2c_read(twsi, chip, addr_bytes, alen, data, length, + 10000); }
static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, @@ -546,7 +576,8 @@ static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, addr_bytes[2] = (addr >> 16) & 0xFF; addr_bytes[3] = (addr >> 24) & 0xFF;
- return __twsi_i2c_write(twsi, chip, addr_bytes, alen, data, length); + return __twsi_i2c_write(twsi, chip, addr_bytes, alen, data, length, + 10000); }
#ifdef CONFIG_I2C_MVTWSI_BASE0 @@ -596,13 +627,17 @@ static int mvtwsi_i2c_probe_chip(struct udevice *bus, u32 chip_addr, u32 chip_flags) { struct mvtwsi_i2c_dev *dev = dev_get_priv(bus); - return __twsi_i2c_probe_chip(dev->base, chip_addr); + return __twsi_i2c_probe_chip(dev->base, chip_addr, dev->tick); }
static int mvtwsi_i2c_set_bus_speed(struct udevice *bus, uint speed) { struct mvtwsi_i2c_dev *dev = dev_get_priv(bus); - return __twsi_i2c_set_bus_speed(dev->base, speed); + + dev->speed = __twsi_i2c_set_bus_speed(dev->base, speed); + dev->tick = calc_tick(dev->speed); + + return 0; }
static int mvtwsi_i2c_ofdata_to_platdata(struct udevice *bus) @@ -631,7 +666,11 @@ static int mvtwsi_i2c_ofdata_to_platdata(struct udevice *bus) static int mvtwsi_i2c_probe(struct udevice *bus) { struct mvtwsi_i2c_dev *dev = dev_get_priv(bus); - __twsi_i2c_init(dev->base, dev->speed, dev->slaveadd); + uint actual_speed; + + __twsi_i2c_init(dev->base, dev->speed, dev->slaveadd, &actual_speed); + dev->speed = actual_speed; + dev->tick = calc_tick(dev->speed); return 0; }
@@ -654,10 +693,12 @@ static int mvtwsi_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, int nmsgs)
if (dmsg->flags & I2C_M_RD) return __twsi_i2c_read(dev->base, dmsg->addr, omsg->buf, - omsg->len, dmsg->buf, dmsg->len); + omsg->len, dmsg->buf, dmsg->len, + dev->tick); else return __twsi_i2c_write(dev->base, dmsg->addr, omsg->buf, - omsg->len, dmsg->buf, dmsg->len); + omsg->len, dmsg->buf, dmsg->len, + dev->tick); }
static const struct dm_i2c_ops mvtwsi_i2c_ops = {

Add full documentation to all driver functions.
Signed-off-by: Mario Six mario.six@gdsys.cc --- drivers/i2c/mvtwsi.c | 163 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 144 insertions(+), 19 deletions(-)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index dd42a3c..abbca3a 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -157,6 +157,12 @@ enum mvtwsi_ack_flags { MVTWSI_READ_ACK = 1, };
+/* + * calc_tick() - Calculate the duration of a clock cycle from the I2C speed + * + * @speed: The speed in Hz to calculate the clock cycle duration for. + * @return The duration of a clock cycle in ns. + */ inline uint calc_tick(uint speed) { /* One tick = the duration of a period at the specified speed in ns (we @@ -167,9 +173,11 @@ inline uint calc_tick(uint speed) #ifndef CONFIG_DM_I2C
/* - * MVTWSI controller base + * twsi_get_base() - Get controller register base for specified adapter + * + * @adap: Adapter to get the register base for. + * @return Register base for the specified adapter. */ - static struct mvtwsi_registers *twsi_get_base(struct i2c_adapter *adap) { switch (adap->hwadapnr) { @@ -238,8 +246,10 @@ inline uint mvtwsi_error(uint ec, uint lc, uint ls, uint es) }
/* - * Wait for IFLG to raise, or return 'timeout.' Then, if the status is as - * expected, return 0 (ok) or 'wrong status' otherwise. + * twsi_wait() - Wait for I2C bus interrupt flag and check status, or time out. + * + * @return Zero if status is as expected, or a non-zero code if either a time + * out occurred, or the status was not the expected one. */ static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status, uint tick) @@ -266,8 +276,17 @@ static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status, }
/* - * Assert the START condition, either in a single I2C transaction - * or inside back-to-back ones (repeated starts). + * twsi_start() - Assert a START condition on the bus. + * + * This function is used in both single I2C transactions and inside + * back-to-back transactions (repeated starts). + * + * @twsi: The MVTWSI register structure to use. + * @expected_status: The I2C bus status expected to be asserted after the + * operation completion. + * @tick: The duration of a clock cycle at the current I2C speed. + * @return Zero if status is as expected, or a non-zero code if either a time + * out occurred or the status was not the expected one. */ static int twsi_start(struct mvtwsi_registers *twsi, int expected_status, uint tick) @@ -280,7 +299,17 @@ static int twsi_start(struct mvtwsi_registers *twsi, int expected_status, }
/* - * Send a byte (i2c address or data). + * twsi_send() - Send a byte on the I2C bus. + * + * The byte may be part of an address byte or data. + * + * @twsi: The MVTWSI register structure to use. + * @byte: The byte to send. + * @expected_status: The I2C bus status expected to be asserted after the + * operation completion. + * @tick: The duration of a clock cycle at the current I2C speed. + * @return Zero if status is as expected, or a non-zero code if either a time + * out occurred or the status was not the expected one. */ static int twsi_send(struct mvtwsi_registers *twsi, u8 byte, int expected_status, uint tick) @@ -295,7 +324,17 @@ static int twsi_send(struct mvtwsi_registers *twsi, u8 byte, }
/* - * Receive a byte. + * twsi_recv() - Receive a byte on the I2C bus. + * + * The static variable mvtwsi_control_flags controls whether we ack or nak. + * + * @twsi: The MVTWSI register structure to use. + * @byte: The byte to send. + * @ack_flag: Flag that determines whether the received byte should + * be acknowledged by the controller or not (sent ACK/NAK). + * @tick: The duration of a clock cycle at the current I2C speed. + * @return Zero if status is as expected, or a non-zero code if either a time + * out occurred or the status was not the expected one. */ static int twsi_recv(struct mvtwsi_registers *twsi, u8 *byte, int ack_flag, uint tick) @@ -318,8 +357,15 @@ static int twsi_recv(struct mvtwsi_registers *twsi, u8 *byte, int ack_flag, }
/* - * Assert the STOP condition. - * This is also used to force the bus back to idle (SDA = SCL = 1). + * twsi_stop() - Assert a STOP condition on the bus. + * + * This function is also used to force the bus back to idle state (SDA = + * SCL = 1). + * + * @twsi: The MVTWSI register structure to use. + * @tick: The duration of a clock cycle at the current I2C speed. + * @return Zero if the operation succeeded, or a non-zero code if a time out + * occurred. */ static int twsi_stop(struct mvtwsi_registers *twsi, uint tick) { @@ -344,6 +390,13 @@ static int twsi_stop(struct mvtwsi_registers *twsi, uint tick) return status; }
+/* + * twsi_calc_freq() - Compute I2C frequency depending on m and n parameters. + * + * @n: Parameter 'n' for the frequency calculation algorithm. + * @m: Parameter 'm' for the frequency calculation algorithm. + * @return The I2C frequency corresponding to the passed m and n parameters. + */ static uint twsi_calc_freq(const int n, const int m) { #ifdef CONFIG_SUNXI @@ -354,9 +407,12 @@ static uint twsi_calc_freq(const int n, const int m) }
/* - * Reset controller. - * Controller reset also resets the baud rate and slave address, so - * they must be re-established afterwards. + * twsi_reset() - Reset the I2C controller. + * + * Resetting the controller also resets the baud rate and slave address, hence + * they must be re-established after the reset. + * + * @twsi: The MVTWSI register structure to use. */ static void twsi_reset(struct mvtwsi_registers *twsi) { @@ -367,7 +423,15 @@ static void twsi_reset(struct mvtwsi_registers *twsi) }
/* - * Sets baud to the highest possible value not exceeding the requested one. + * __twsi_i2c_set_bus_speed() - Set the speed of the I2C controller. + * + * This function sets baud rate to the highest possible value that does not + * exceed the requested rate. + * + * @twsi: The MVTWSI register structure to use. + * @requested_speed: The desired frequency the controller should run at + * in Hz. + * @return The actual frequency the controller was configured to. */ static uint __twsi_i2c_set_bus_speed(struct mvtwsi_registers *twsi, uint requested_speed) @@ -400,6 +464,18 @@ static uint __twsi_i2c_set_bus_speed(struct mvtwsi_registers *twsi, return highest_speed; }
+/* + * __twsi_i2c_init() - Initialize the I2C controller. + * + * @twsi: The MVTWSI register structure to use. + * @speed: The initial frequency the controller should run at + * in Hz. + * @slaveadd: The I2C address to be set for the I2C master. + * @actual_speed: A output parameter that receives the actual frequency + * in Hz the controller was set to by the function. + * @return Zero if the operation succeeded, or a non-zero code if a time out + * occurred. + */ static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed, int slaveadd, uint *actual_speed) { @@ -419,8 +495,21 @@ static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed, }
/* - * Begin I2C transaction with expected start status, at given address. - * Expected address status will derive from direction bit (bit 0) in addr. + * i2c_begin() - Start a I2C transaction. + * + * Begin a I2C transaction with a given expected start status and chip address. + * A START is asserted, and the address byte is sent to the I2C controller. The + * expected address status will be derived from the direction bit (bit 0) of + * the address byte. + * + * @twsi: The MVTWSI register structure to use. + * @expected_start_status: The I2C status the controller is expected to + * assert after the address byte was sent. + * @addr: The address byte to be sent. + * @tick: The duration of a clock cycle at the current + * I2C speed. + * @return Zero if the operation succeeded, or a non-zero code if a time out or + * unexpected I2C status occurred. */ static int i2c_begin(struct mvtwsi_registers *twsi, int expected_start_status, u8 addr, uint tick) @@ -443,7 +532,16 @@ static int i2c_begin(struct mvtwsi_registers *twsi, int expected_start_status, }
/* - * Begin read, nak data byte, end. + * __twsi_i2c_probe_chip() - Probe the given I2C chip address. + * + * This function begins a I2C read transaction, does a dummy read and NAKs; if + * the procedure succeeds, the chip is considered to be present. + * + * @twsi: The MVTWSI register structure to use. + * @chip: The chip address to probe. + * @tick: The duration of a clock cycle at the current I2C speed. + * @return Zero if the operation succeeded, or a non-zero code if a time out or + * unexpected I2C status occurred. */ static int __twsi_i2c_probe_chip(struct mvtwsi_registers *twsi, uchar chip, uint tick) @@ -463,12 +561,26 @@ static int __twsi_i2c_probe_chip(struct mvtwsi_registers *twsi, uchar chip, }
/* - * Begin write, send address byte(s), begin read, receive data bytes, end. + * __twsi_i2c_read() - Read data from a I2C chip. + * + * This function begins a I2C write transaction, and transmits the address + * bytes; then begins a I2C read transaction, and receives the data bytes. * * NOTE: Some devices want a stop right before the second start, while some * will choke if it is there. Since deciding this is not yet supported in * higher level APIs, we need to make a decision here, and for the moment that * will be a repeated start without a preceding stop. + * + * @twsi: The MVTWSI register structure to use. + * @chip: The chip address to read from. + * @addr: The address bytes to send. + * @alen: The length of the address bytes in bytes. + * @data: The buffer to receive the data read from the chip (has to have + * a size of at least 'length' bytes). + * @length: The amount of data to be read from the chip in bytes. + * @tick: The duration of a clock cycle at the current I2C speed. + * @return Zero if the operation succeeded, or a non-zero code if a time out or + * unexpected I2C status occurred. */ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip, u8 *addr, int alen, uchar *data, int length, @@ -504,7 +616,20 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip, }
/* - * Begin write, send address byte(s), send data bytes, end. + * __twsi_i2c_write() - Send data to a I2C chip. + * + * This function begins a I2C write transaction, and transmits the address + * bytes; then begins a new I2C write transaction, and sends the data bytes. + * + * @twsi: The MVTWSI register structure to use. + * @chip: The chip address to read from. + * @addr: The address bytes to send. + * @alen: The length of the address bytes in bytes. + * @data: The buffer containing the data to be sent to the chip. + * @length: The length of data to be sent to the chip in bytes. + * @tick: The duration of a clock cycle at the current I2C speed. + * @return Zero if the operation succeeded, or a non-zero code if a time out or + * unexpected I2C status occurred. */ static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip, u8 *addr, int alen, uchar *data, int length,
participants (2)
-
Mario Six
-
Stefan Roese