[PATCH V4 1/9] i2c: xiic: Add standard mode support for > 255 byte

Manikanta Guntupalli
2023-02-02T09:43:03Z

read transfers

Added standard mode for AXI I2C controller to enable read transfers
of size more than 255 bytes. The driver selects standard mode in the
following scenarios.

1. If a single message request comes from user space, requesting a
read of more than 255 bytes

2. If a message set request comes from user space consisting of many
messages and if any one of them is a read operation, irrespective
of the size of transfer. (This is done because it is observed that
repeated start operation is not happening in dynamic mode read as
expected in a message set request from user space.)

Signed-off-by: Raviteja Narayanam <raviteja.narayanam@xilinx.com>
Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
Acked-by: Michal Simek <michal.simek@amd.com>
---
Changes for v2:
Merged the logic of xiic_std_fill_tx_fifo into xiic_fill_tx_fifo to remove
duplicate code.
Handled i2c msg->len == 0 case.
Squashed "Use 'nmsgs' variable instead of repeated_start" patch.
Squashed "Correct the BNB interrupt enable sequence" patch.
Squashed "remove unsupported flag I2C_M_NOSTART code" patch.
---
drivers/i2c/busses/i2c-xiic.c | 266 ++++++++++++++++++++++++++++------
1 file changed, 225 insertions(+), 41 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index bee5a2ef1f22..839564054501 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -60,6 +60,7 @@ enum xiic_endian {
* @clk: Pointer to AXI4-lite input clock
* @state: See STATE_
* @singlemaster: Indicates bus is single master
+ * @dynamic: Mode of controller
*/
struct xiic_i2c {
struct device *dev;
@@ -76,6 +77,7 @@ struct xiic_i2c {
struct clk *clk;
enum xilinx_i2c_state state;
bool singlemaster;
+ bool dynamic;
};

#define XIIC_MSB_OFFSET 0
@@ -143,6 +145,9 @@ struct xiic_i2c {
#define XIIC_TX_DYN_START_MASK 0x0100 /* 1 = Set dynamic start */
#define XIIC_TX_DYN_STOP_MASK 0x0200 /* 1 = Set dynamic stop */

+/* Dynamic mode constants */
+#define MAX_READ_LENGTH_DYNAMIC 255 /* Max length for dynamic read */
+
/*
* The following constants define the register offsets for the Interrupt
* registers. There are some holes in the memory map for reserved addresses
@@ -316,13 +321,14 @@ static void xiic_deinit(struct xiic_i2c *i2c)

static void xiic_read_rx(struct xiic_i2c *i2c)
{
- u8 bytes_in_fifo;
+ u8 bytes_in_fifo, cr = 0, bytes_to_read = 0;
+ u32 bytes_rem = 0;
int i;

bytes_in_fifo = xiic_getreg8(i2c, XIIC_RFO_REG_OFFSET) + 1;

dev_dbg(i2c->adap.dev.parent,
- "%s entry, bytes in fifo: %d, msg: %d, SR: 0x%x, CR: 0x%x\n",
+ "%s entry, bytes in fifo: %d, rem: %d, SR: 0x%x, CR: 0x%x\n",
__func__, bytes_in_fifo, xiic_rx_space(i2c),
xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
@@ -330,13 +336,52 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
if (bytes_in_fifo > xiic_rx_space(i2c))
bytes_in_fifo = xiic_rx_space(i2c);

- for (i = 0; i < bytes_in_fifo; i++)
+ bytes_to_read = bytes_in_fifo;
+
+ if (!i2c->dynamic) {
+ bytes_rem = xiic_rx_space(i2c) - bytes_in_fifo;
+
+ if (bytes_rem > IIC_RX_FIFO_DEPTH) {
+ bytes_to_read = bytes_in_fifo;
+ } else if (bytes_rem > 1) {
+ bytes_to_read = bytes_rem - 1;
+ } else if (bytes_rem == 1) {
+ bytes_to_read = 1;
+ /* Set NACK in CR to indicate slave transmitter */
+ cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
+ xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr |
+ XIIC_CR_NO_ACK_MASK);
+ } else if (bytes_rem == 0) {
+ bytes_to_read = bytes_in_fifo;
+
+ /* Generate stop on the bus if it is last message */
+ if (i2c->nmsgs == 1) {
+ cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
+ xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr &
+ ~XIIC_CR_MSMS_MASK);
+ }
+
+ /* Make TXACK=0, clean up for next transaction */
+ cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
+ xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr &
+ ~XIIC_CR_NO_ACK_MASK);
+ }
+ }
+
+ /* Read the fifo */
+ for (i = 0; i < bytes_to_read; i++) {
i2c->rx_msg->buf[i2c->rx_pos++] =
xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
+ }

- xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET,
- (xiic_rx_space(i2c) > IIC_RX_FIFO_DEPTH) ?
- IIC_RX_FIFO_DEPTH - 1 : xiic_rx_space(i2c) - 1);
+ if (i2c->dynamic) {
+ u8 bytes;
+
+ /* Receive remaining bytes if less than fifo depth */
+ bytes = min_t(u8, xiic_rx_space(i2c), IIC_RX_FIFO_DEPTH);
+ bytes--;
+ xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, bytes);
+ }
}

static int xiic_tx_fifo_space(struct xiic_i2c *i2c)
@@ -360,7 +405,15 @@ static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)

if (!xiic_tx_space(i2c) && i2c->nmsgs == 1) {
/* last message in transfer -> STOP */
- data |= XIIC_TX_DYN_STOP_MASK;
+ if (i2c->dynamic) {
+ data |= XIIC_TX_DYN_STOP_MASK;
+ } else {
+ u8 cr;
+ /* Write to CR to stop */
+ cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
+ xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr &
+ ~XIIC_CR_MSMS_MASK);
+ }
dev_dbg(i2c->adap.dev.parent, "%s TX STOP\n", __func__);
}
xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
@@ -401,7 +454,9 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
dev_dbg(i2c->adap.dev.parent, "%s: SR: 0x%x, msg: %p, nmsgs: %d\n",
__func__, xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
i2c->tx_msg, i2c->nmsgs);
-
+ dev_dbg(i2c->adap.dev.parent, "%s, ISR: 0x%x, CR: 0x%x\n",
+ __func__, xiic_getreg32(i2c, XIIC_IISR_OFFSET),
+ xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));

/* Service requesting interrupt */
if ((pend & XIIC_INTR_ARB_LOST_MASK) ||
@@ -579,31 +634,101 @@ static int xiic_busy(struct xiic_i2c *i2c)
static void xiic_start_recv(struct xiic_i2c *i2c)
{
u16 rx_watermark;
+ u8 cr = 0, rfd_set = 0;
struct i2c_msg *msg = i2c->rx_msg = i2c->tx_msg;
+ unsigned long flags;

- /* Clear and enable Rx full interrupt. */
- xiic_irq_clr_en(i2c, XIIC_INTR_RX_FULL_MASK | XIIC_INTR_TX_ERROR_MASK);
+ dev_dbg(i2c->adap.dev.parent, "%s entry, ISR: 0x%x, CR: 0x%x\n",
+ __func__, xiic_getreg32(i2c, XIIC_IISR_OFFSET),
+ xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));

- /* we want to get all but last byte, because the TX_ERROR IRQ is used
- * to inidicate error ACK on the address, and negative ack on the last
- * received byte, so to not mix them receive all but last.
- * In the case where there is only one byte to receive
- * we can check if ERROR and RX full is set at the same time
- */
- rx_watermark = msg->len;
- if (rx_watermark > IIC_RX_FIFO_DEPTH)
- rx_watermark = IIC_RX_FIFO_DEPTH;
- xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, (u8)(rx_watermark - 1));
+ /* Disable Tx interrupts */
+ xiic_irq_dis(i2c, XIIC_INTR_TX_HALF_MASK | XIIC_INTR_TX_EMPTY_MASK);
+
+ if (i2c->dynamic) {
+ u8 bytes;
+ u16 val;
+
+ /* Clear and enable Rx full interrupt. */
+ xiic_irq_clr_en(i2c, XIIC_INTR_RX_FULL_MASK |
+ XIIC_INTR_TX_ERROR_MASK);
+
+ /*
+ * We want to get all but last byte, because the TX_ERROR IRQ
+ * is used to indicate error ACK on the address, and
+ * negative ack on the last received byte, so to not mix
+ * them receive all but last.
+ * In the case where there is only one byte to receive
+ * we can check if ERROR and RX full is set at the same time
+ */
+ rx_watermark = msg->len;
+ bytes = min_t(u8, rx_watermark, IIC_RX_FIFO_DEPTH);
+
+ if (rx_watermark > 0)
+ bytes--;
+ xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, bytes);
+
+ local_irq_save(flags);

- if (!(msg->flags & I2C_M_NOSTART))
/* write the address */
xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
- i2c_8bit_addr_from_msg(msg) | XIIC_TX_DYN_START_MASK);
+ i2c_8bit_addr_from_msg(msg) |
+ XIIC_TX_DYN_START_MASK);
+
+ /* If last message, include dynamic stop bit with length */
+ val = (i2c->nmsgs == 1) ? XIIC_TX_DYN_STOP_MASK : 0;
+ val |= msg->len;
+
+ xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, val);
+
+ xiic_irq_clr_en(i2c, XIIC_INTR_BNB_MASK);
+
+ local_irq_restore(flags);
+ } else {
+ cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
+
+ /* Set Receive fifo depth */
+ rx_watermark = msg->len;
+ if (rx_watermark > IIC_RX_FIFO_DEPTH) {
+ rfd_set = IIC_RX_FIFO_DEPTH - 1;
+ } else if (rx_watermark == 1) {
+ rfd_set = rx_watermark - 1;
+ /* Handle single byte transfer separately */
+ cr |= XIIC_CR_NO_ACK_MASK;
+ } else if (rx_watermark == 0) {
+ rfd_set = rx_watermark;
+ cr |= XIIC_CR_NO_ACK_MASK;
+ } else {
+ rfd_set = rx_watermark - 2;
+ }
+ /* Check if RSTA should be set */
+ if (cr & XIIC_CR_MSMS_MASK) {
+ /* Already a master, RSTA should be set */
+ xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, (cr |
+ XIIC_CR_REPEATED_START_MASK) &
+ ~(XIIC_CR_DIR_IS_TX_MASK));
+ }

- xiic_irq_clr_en(i2c, XIIC_INTR_BNB_MASK);
+ xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, rfd_set);

- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
- msg->len | ((i2c->nmsgs == 1) ? XIIC_TX_DYN_STOP_MASK : 0));
+ /* Clear and enable Rx full and transmit complete interrupts */
+ xiic_irq_clr_en(i2c, XIIC_INTR_RX_FULL_MASK |
+ XIIC_INTR_TX_ERROR_MASK);
+
+ /* Write the address */
+ xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
+ i2c_8bit_addr_from_msg(msg));
+
+ /* Write to Control Register,to start transaction in Rx mode */
+ if ((cr & XIIC_CR_MSMS_MASK) == 0) {
+ xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, (cr |
+ XIIC_CR_MSMS_MASK)
+ & ~(XIIC_CR_DIR_IS_TX_MASK));
+ }
+ dev_dbg(i2c->adap.dev.parent, "%s end, ISR: 0x%x, CR: 0x%x\n",
+ __func__, xiic_getreg32(i2c, XIIC_IISR_OFFSET),
+ xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
+ }

if (i2c->nmsgs == 1)
/* very last, enable bus not busy as well */
@@ -611,10 +736,15 @@ static void xiic_start_recv(struct xiic_i2c *i2c)

/* the message is tx:ed */
i2c->tx_pos = msg->len;
+
+ /* Enable interrupts */
+ xiic_setreg32(i2c, XIIC_DGIER_OFFSET, XIIC_GINTR_ENABLE_MASK);
}

static void xiic_start_send(struct xiic_i2c *i2c)
{
+ u8 cr = 0;
+ u16 data;
struct i2c_msg *msg = i2c->tx_msg;

dev_dbg(i2c->adap.dev.parent, "%s entry, msg: %p, len: %d",
@@ -623,24 +753,56 @@ static void xiic_start_send(struct xiic_i2c *i2c)
__func__, xiic_getreg32(i2c, XIIC_IISR_OFFSET),
xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));

- if (!(msg->flags & I2C_M_NOSTART)) {
+ if (i2c->dynamic) {
/* write the address */
- u16 data = i2c_8bit_addr_from_msg(msg) |
- XIIC_TX_DYN_START_MASK;
- if ((i2c->nmsgs == 1) && msg->len == 0)
+ data = i2c_8bit_addr_from_msg(msg) |
+ XIIC_TX_DYN_START_MASK;
+
+ if (i2c->nmsgs == 1 && msg->len == 0)
/* no data and last message -> add STOP */
data |= XIIC_TX_DYN_STOP_MASK;

xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
- }

- /* Clear any pending Tx empty, Tx Error and then enable them. */
- xiic_irq_clr_en(i2c, XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_ERROR_MASK |
- XIIC_INTR_BNB_MASK |
- ((i2c->nmsgs > 1 || xiic_tx_space(i2c)) ?
- XIIC_INTR_TX_HALF_MASK : 0));
+ /* Clear any pending Tx empty, Tx Error and then enable them */
+ xiic_irq_clr_en(i2c, XIIC_INTR_TX_EMPTY_MASK |
+ XIIC_INTR_TX_ERROR_MASK |
+ XIIC_INTR_BNB_MASK |
+ ((i2c->nmsgs > 1 || xiic_tx_space(i2c)) ?
+ XIIC_INTR_TX_HALF_MASK : 0));
+
+ xiic_fill_tx_fifo(i2c);
+ } else {
+ /* Check if RSTA should be set */
+ cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
+ if (cr & XIIC_CR_MSMS_MASK) {
+ /* Already a master, RSTA should be set */
+ xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, (cr |
+ XIIC_CR_REPEATED_START_MASK |
+ XIIC_CR_DIR_IS_TX_MASK) &
+ ~(XIIC_CR_NO_ACK_MASK));
+ }
+
+ /* Write address to FIFO */
+ data = i2c_8bit_addr_from_msg(msg);
+ xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);

- xiic_fill_tx_fifo(i2c);
+ /* Fill fifo */
+ xiic_fill_tx_fifo(i2c);
+
+ if ((cr & XIIC_CR_MSMS_MASK) == 0) {
+ /* Start Tx by writing to CR */
+ cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
+ xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr |
+ XIIC_CR_MSMS_MASK |
+ XIIC_CR_DIR_IS_TX_MASK);
+ }
+
+ /* Clear any pending Tx empty, Tx Error and then enable them */
+ xiic_irq_clr_en(i2c, XIIC_INTR_TX_EMPTY_MASK |
+ XIIC_INTR_TX_ERROR_MASK |
+ XIIC_INTR_BNB_MASK);
+ }
}

static void __xiic_start_xfer(struct xiic_i2c *i2c)
@@ -701,6 +863,33 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
if (err < 0)
return err;

+ /* Decide standard mode or Dynamic mode */
+ i2c->dynamic = true;
+
+ /*
+ * If number of messages is 1 and read length is > 255 bytes,
+ * enter standard mode
+ */
+
+ if (i2c->nmsgs == 1 && (i2c->tx_msg->flags & I2C_M_RD) &&
+ i2c->tx_msg->len > MAX_READ_LENGTH_DYNAMIC) {
+ i2c->dynamic = false;
+ } else if (i2c->nmsgs > 1) {
+ int count;
+
+ /*
+ * If number of messages is more than 1 and one of them is
+ * a read message, enter standard mode. Since repeated start
+ * operation in dynamic mode read is not happenning
+ */
+ for (count = 0; count < i2c->nmsgs; count++) {
+ if (i2c->tx_msg[count].flags & I2C_M_RD) {
+ i2c->dynamic = false;
+ break;
+ }
+ }
+ }
+
err = xiic_start_xfer(i2c, msgs, num);
if (err < 0) {
dev_err(adap->dev.parent, "Error xiic_start_xfer\n");
@@ -737,15 +926,10 @@ static const struct i2c_algorithm xiic_algorithm = {
.functionality = xiic_func,
};

-static const struct i2c_adapter_quirks xiic_quirks = {
- .max_read_len = 255,
-};
-
static const struct i2c_adapter xiic_adapter = {
.owner = THIS_MODULE,
.class = I2C_CLASS_DEPRECATED,
.algo = &xiic_algorithm,
- .quirks = &xiic_quirks,
};

static int xiic_i2c_probe(struct platform_device *pdev)
--
2.25.1