[PATCH RFC/RFT 1/3] i2c: omap24xx_i2c: Use new function __omap24_i2c_xfer_msg()
Heiko Schocher
hs at denx.de
Thu Mar 6 06:51:35 CET 2025
Hello Aniket,
On 04.03.25 23:04, Aniket Limaye wrote:
> Remove __omap24_i2c_read/write() usage from omap_i2c_xfer() in favour of
> the more flexible __omap24_i2c_xfer_msg().
> Consequently, these are also no longer needed when DM_I2C is enabled.
>
> New function __omap24_i2c_xfer_msg() will take care of individual read
> OR write transfers with a target device. It goes through below sequence:
> - Program the provided Target Chip address (OMAP_I2C_SA_REG)
> - Program the provided Data len (OMAP_I2C_CNT_REG)
> - Program the provided Control register flags (OMAP_I2C_CON_REG)
> - Read from or Write to the provided Data buffer (OMAP_I2C_DATA_REG)
>
> For a detailed programming guide, refer to the TRM[0] (12.1.3.4 I2C
> Programming Guide).
>
> This patch by itself should be a transparent change. However this is
> needed for implementing a proper Repeated Start (Sr) functionality for
> i2c_msgs.
>
> Previous implementation for omap_i2c_xfer called __omap24_i2c_read/write
> functions, with hardcoded addr=0 and alen=0 for each i2c_msg. Each of
> these calls would program the registers always with a Stop bit set, not
> allowing for a repeated start between i2c_msgs in the same xfer().
>
> [0]: https://www.ti.com/lit/zip/spruj28 (TRM)
>
> Signed-off-by: Aniket Limaye <a-limaye at ti.com>
> ---
> drivers/i2c/omap24xx_i2c.c | 139 ++++++++++++++++++++++++++++++++-----
> 1 file changed, 121 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
> index ebe472e20cd..0d044121d27 100644
> --- a/drivers/i2c/omap24xx_i2c.c
> +++ b/drivers/i2c/omap24xx_i2c.c
> @@ -535,6 +535,107 @@ pr_exit:
> return res;
> }
>
> +static int __omap24_i2c_xfer_msg(void __iomem *i2c_base, int ip_rev, int waitdelay,
> + uchar chip, uchar *buffer, int len, u16 i2c_con_reg)
> +{
> + int i;
> + u16 status;
> + int i2c_error = 0;
> + int timeout = I2C_TIMEOUT;
> +
> + if (len < 0) {
> + puts("I2C xfer: data len < 0\n");
> + return 1;
> + }
> +
> + if (!buffer) {
> + puts("I2C xfer: NULL pointer passed\n");
> + return 1;
> + }
> +
> + if (!(i2c_con_reg & I2C_CON_EN)) {
> + puts("I2C xfer: I2C_CON_EN not set\n");
> + return 1;
> + }
Can we use here (and above) a negative error code? Later you only
check in omap_i2c_xfer() if returncode is ! 0 and return "-EREMOTEIO"
may, you can return here better codes and simply return than this
error codes from omap_i2c_xfer() ?
And may you change the strings from the puts call to a more driver specifc
string ... so its clear from which i2c driver the error message comes...
please fix this globally in your patchset.
> +
> + /* Set slave address */
> + omap_i2c_write_reg(i2c_base, ip_rev, chip, OMAP_I2C_SA_REG);
> + /* Read/Write len bytes data */
> + omap_i2c_write_reg(i2c_base, ip_rev, len, OMAP_I2C_CNT_REG);
> + /* Configure the I2C_CON register */
> + omap_i2c_write_reg(i2c_base, ip_rev, i2c_con_reg, OMAP_I2C_CON_REG);
> +
> + /* read/write data bytewise */
> + for (i = 0; i < len; i++) {
> + status = wait_for_event(i2c_base, ip_rev, waitdelay);
> + /* Ignore I2C_STAT_RRDY in transmitter mode */
> + if (i2c_con_reg & I2C_CON_TRX)
> + status &= ~I2C_STAT_RRDY;
> + else
> + status &= ~I2C_STAT_XRDY;
> +
> + /* Try to identify bus that is not padconf'd for I2C */
> + if (status == I2C_STAT_XRDY) {
> + i2c_error = 2;
> + printf("I2C xfer: pads on bus probably not configured (status=0x%x)\n",
> + status);
> + goto xfer_exit;
> + }
> + if (status == 0 || (status & I2C_STAT_NACK)) {
> + i2c_error = 1;
> + printf("I2C xfer: error waiting for ACK (status=0x%x)\n",
> + status);
> + goto xfer_exit;
> + }
> + if (status & I2C_STAT_XRDY) {
> + /* Transmit data */
> + omap_i2c_write_reg(i2c_base, ip_rev,
> + buffer[i], OMAP_I2C_DATA_REG);
> + omap_i2c_write_reg(i2c_base, ip_rev,
> + I2C_STAT_XRDY, OMAP_I2C_STAT_REG);
> + }
> + if (status & I2C_STAT_RRDY) {
> + /* Receive data */
> + *buffer++ = omap_i2c_read_reg(i2c_base, ip_rev,
> + OMAP_I2C_DATA_REG);
> + omap_i2c_write_reg(i2c_base, ip_rev,
> + I2C_STAT_RRDY, OMAP_I2C_STAT_REG);
> + }
> + }
> +
> + /*
> + * poll ARDY bit for making sure that last byte really has been
> + * transferred on the bus.
> + */
> + do {
> + status = wait_for_event(i2c_base, ip_rev, waitdelay);
> + } while (!(status & I2C_STAT_ARDY) && timeout--);
> + if (timeout <= 0) {
> + puts("I2C xfer: timed out on last byte!\n");
> + i2c_error = 1;
> + goto xfer_exit;
> + } else {
> + omap_i2c_write_reg(i2c_base, ip_rev, I2C_STAT_ARDY, OMAP_I2C_STAT_REG);
> + }
> +
> + /* If Stop bit set, flush FIFO. */
> + if (i2c_con_reg & I2C_CON_STP)
> + goto xfer_exit;
> +
> + return 0;
> +
> +xfer_exit:
> + flush_fifo(i2c_base, ip_rev);
> + omap_i2c_write_reg(i2c_base, ip_rev, 0xFFFF, OMAP_I2C_STAT_REG);
> + return i2c_error;
> +}
> +
> +#if !CONFIG_IS_ENABLED(DM_I2C)
> +/*
> + * The legacy I2C functions. These need to get removed once
> + * all users of this driver are converted to DM.
> + */
> +
> /*
> * i2c_read: Function now uses a single I2C read transaction with bulk transfer
> * of the requested number of bytes (note that the 'i2c md' command
> @@ -836,11 +937,6 @@ wr_exit:
> return i2c_error;
> }
>
> -#if !CONFIG_IS_ENABLED(DM_I2C)
> -/*
> - * The legacy I2C functions. These need to get removed once
> - * all users of this driver are converted to DM.
> - */
> static void __iomem *omap24_get_base(struct i2c_adapter *adap)
> {
> switch (adap->hwadapnr) {
> @@ -975,23 +1071,30 @@ static int omap_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, int nmsgs)
> {
> struct omap_i2c *priv = dev_get_priv(bus);
> int ret;
> + u16 i2c_con_reg = 0;
>
> debug("i2c_xfer: %d messages\n", nmsgs);
> for (; nmsgs > 0; nmsgs--, msg++) {
> - debug("i2c_xfer: chip=0x%x, len=0x%x\n", msg->addr, msg->len);
> - if (msg->flags & I2C_M_RD) {
> - ret = __omap24_i2c_read(priv->regs, priv->ip_rev,
> - priv->waitdelay,
> - msg->addr, 0, 0, msg->buf,
> - msg->len);
> - } else {
> - ret = __omap24_i2c_write(priv->regs, priv->ip_rev,
> - priv->waitdelay,
> - msg->addr, 0, 0, msg->buf,
> - msg->len);
> - }
> + debug("i2c_xfer: chip=0x%x, len=0x%x, read=0x%x\n",
> + msg->addr, msg->len, (msg->flags & I2C_M_RD));
> +
> + /* Wait until bus not busy */
> + if (wait_for_bb(priv->regs, priv->ip_rev, priv->waitdelay))
> + return -EREMOTEIO;
> +
> + /* Set Controller mode with Start and Stop bit */
> + i2c_con_reg = I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP;
> + /* Set Transmitter/Receiver mode if it is a write/read msg */
> + if (msg->flags & I2C_M_RD)
> + i2c_con_reg &= ~I2C_CON_TRX;
> + else
> + i2c_con_reg |= I2C_CON_TRX;
> +
> + ret = __omap24_i2c_xfer_msg(priv->regs, priv->ip_rev, priv->waitdelay,
> + msg->addr, msg->buf, msg->len,
> + i2c_con_reg);
> if (ret) {
> - debug("i2c_write: error sending\n");
> + debug("i2c_xfer: error sending\n");
> return -EREMOTEIO;
> }
> }
>
Thanks!
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de
More information about the U-Boot
mailing list