-Add 'standard mode' feature for reads > 255 bytes.
-Add 'smbus block read' functionality.
-Add 'xlnx,axi-iic-2.1' new IP version support.
-Switch to 'AXI I2C standard mode' for i2c reads in affected IP versions.
-Remove 'local_irq_save/restore' calls as discussed here: https://www.spinics.net/lists/linux-i2c/msg46483.html.
-Some trivial fixes.
Michal Simek (1):
i2c: xiic: Fix coding style issues
Raviteja Narayanam (7):
i2c: xiic: Fix Tx Interrupt path for grouped messages
i2c: xiic: Add standard mode support for > 255 byte read transfers
i2c: xiic: Add smbus_block_read functionality
i2c: xiic: Switch to Xiic standard mode for i2c-read
i2c: xiic: Remove interrupt enable/disable in Rx path
dt-bindings: i2c: xiic: Add 'xlnx,axi-iic-2.1' to compatible
i2c: xiic: Update compatible with new IP version
Shubhrajyoti Datta (2):
i2c: xiic: Return value of xiic_reinit
i2c: xiic: Fix the type check for xiic_wakeup
.../bindings/i2c/xlnx,xps-iic-2.00.a.yaml | 4 +-
drivers/i2c/busses/i2c-xiic.c | 593 ++++++++++++++----
2 files changed, 487 insertions(+), 110 deletions(-)
--
2.17.1
smbus_block_read is added to xiic driver to read from few sensors
which support this command. Since the number of bytes to read is not
known prior to transfer, we are using xiic standard mode for low level
control of IP.
Signed-off-by: Raviteja Narayanam <[email protected]>
---
drivers/i2c/busses/i2c-xiic.c | 82 ++++++++++++++++++++++++++++++++---
1 file changed, 76 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index fa2eef9e622f..51b5fd5768db 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -62,6 +62,7 @@ enum xiic_endian {
* @singlemaster: Indicates bus is single master
* @dynamic: Mode of controller
* @prev_msg_tx: Previous message is Tx
+ * @smbus_block_read: Flag to handle block read
*/
struct xiic_i2c {
struct device *dev;
@@ -80,6 +81,7 @@ struct xiic_i2c {
bool singlemaster;
bool dynamic;
bool prev_msg_tx;
+ bool smbus_block_read;
};
#define XIIC_MSB_OFFSET 0
@@ -336,6 +338,54 @@ static void xiic_deinit(struct xiic_i2c *i2c)
xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
}
+static void xiic_smbus_block_read_setup(struct xiic_i2c *i2c)
+{
+ u8 rxmsg_len;
+ u8 rfd_set = 0;
+
+ /*
+ * Clear the I2C_M_RECV_LEN flag to avoid setting
+ * message length again
+ */
+ i2c->rx_msg->flags &= ~I2C_M_RECV_LEN;
+
+ /* Set smbus_block_read flag to identify in isr */
+ i2c->smbus_block_read = true;
+
+ /* Read byte from rx fifo and set message length */
+ rxmsg_len = xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
+
+ i2c->rx_msg->buf[i2c->rx_pos++] = rxmsg_len;
+
+ /* Check if received length is valid */
+ if (rxmsg_len <= I2C_SMBUS_BLOCK_MAX) {
+ /* Set Receive fifo depth */
+ if (rxmsg_len > IIC_RX_FIFO_DEPTH) {
+ rfd_set = IIC_RX_FIFO_DEPTH - 1;
+ i2c->rx_msg->len = rxmsg_len + 1;
+ } else if ((rxmsg_len == 1) ||
+ (rxmsg_len == 0)) {
+ /*
+ * Minimum of 3 bytes required to exit cleanly. 1 byte
+ * already received, Second byte is being received. Have
+ * to set NACK in read_rx before receiving the last byte
+ */
+ i2c->rx_msg->len = 3;
+ } else {
+ rfd_set = rxmsg_len - 2;
+ i2c->rx_msg->len = rxmsg_len + 1;
+ }
+ xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, rfd_set);
+
+ return;
+ }
+
+ /* Invalid message length, trigger STATE_ERROR with tx_msg_len in ISR */
+ i2c->tx_msg->len = 3;
+ i2c->smbus_block_read = false;
+ dev_err(i2c->adap.dev.parent, "smbus_block_read Invalid msg length\n");
+}
+
static void xiic_read_rx(struct xiic_i2c *i2c)
{
u8 bytes_in_fifo, cr = 0, bytes_to_read = 0;
@@ -358,6 +408,12 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
if (!i2c->dynamic) {
bytes_rem = xiic_rx_space(i2c) - bytes_in_fifo;
+ /* Set msg length if smbus_block_read */
+ if (i2c->rx_msg->flags & I2C_M_RECV_LEN) {
+ xiic_smbus_block_read_setup(i2c);
+ return;
+ }
+
if (bytes_rem > IIC_RX_FIFO_DEPTH) {
bytes_to_read = bytes_in_fifo;
} else if (bytes_rem > 1) {
@@ -627,6 +683,12 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
/* The bus is not busy, disable BusNotBusy interrupt */
xiic_irq_dis(i2c, XIIC_INTR_BNB_MASK);
+ if (i2c->tx_msg && i2c->smbus_block_read) {
+ i2c->smbus_block_read = false;
+ /* Set requested message len=1 to indicate STATE_DONE */
+ i2c->tx_msg->len = 1;
+ }
+
if (!i2c->tx_msg)
goto out;
@@ -755,8 +817,12 @@ static void xiic_start_recv(struct xiic_i2c *i2c)
rfd_set = IIC_RX_FIFO_DEPTH - 1;
} else if ((rx_watermark == 1) || (rx_watermark == 0)) {
rfd_set = rx_watermark - 1;
- /* Handle single byte transfer separately */
- cr |= XIIC_CR_NO_ACK_MASK;
+
+ /* Set No_ACK, except for smbus_block_read */
+ if (!(i2c->rx_msg->flags & I2C_M_RECV_LEN)) {
+ /* Handle single byte transfer separately */
+ cr |= XIIC_CR_NO_ACK_MASK;
+ }
} else {
rfd_set = rx_watermark - 2;
}
@@ -986,10 +1052,14 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
/* Initialize prev message type */
i2c->prev_msg_tx = false;
- /* Enter standard mode only when read length is > 255 bytes */
+ /*
+ * Enter standard mode only when read length is > 255 bytes or
+ * for smbus_block_read transaction
+ */
for (count = 0; count < i2c->nmsgs; count++) {
- if ((i2c->tx_msg[count].flags & I2C_M_RD) &&
- i2c->tx_msg[count].len > MAX_READ_LENGTH_DYNAMIC) {
+ if (((i2c->tx_msg[count].flags & I2C_M_RD) &&
+ i2c->tx_msg[count].len > MAX_READ_LENGTH_DYNAMIC) ||
+ (i2c->tx_msg[count].flags & I2C_M_RECV_LEN)) {
i2c->dynamic = false;
break;
}
@@ -1020,7 +1090,7 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
static u32 xiic_func(struct i2c_adapter *adap)
{
- return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
}
static const struct i2c_algorithm xiic_algorithm = {
--
2.17.1
From: Michal Simek <[email protected]>
Most of these stuff are reported by checkpatch.
But fixes are:
- Incorrect indetation
- Missing blank line after variable declaration
- Additional ()
- Missing spaces around +
- Else after if with return
- Missing parenthesis when if has them
- Newlines
- Remove MODULE_ALIAS - none is really using it
- Changing msleep to usleep_range
- Other trivial fixes
Signed-off-by: Michal Simek <[email protected]>
Signed-off-by: Raviteja Narayanam <[email protected]>
---
drivers/i2c/busses/i2c-xiic.c | 69 +++++++++++++++++------------------
1 file changed, 34 insertions(+), 35 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 004103267e9c..fa2eef9e622f 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -69,7 +69,7 @@ struct xiic_i2c {
wait_queue_head_t wait;
struct i2c_adapter adap;
struct i2c_msg *tx_msg;
- struct mutex lock;
+ struct mutex lock; /* Locking between isr and new xfer */
unsigned int tx_pos;
unsigned int nmsgs;
struct i2c_msg *rx_msg;
@@ -82,24 +82,23 @@ struct xiic_i2c {
bool prev_msg_tx;
};
-
#define XIIC_MSB_OFFSET 0
-#define XIIC_REG_OFFSET (0x100+XIIC_MSB_OFFSET)
+#define XIIC_REG_OFFSET (0x100 + XIIC_MSB_OFFSET)
/*
* Register offsets in bytes from RegisterBase. Three is added to the
* base offset to access LSB (IBM style) of the word
*/
-#define XIIC_CR_REG_OFFSET (0x00+XIIC_REG_OFFSET) /* Control Register */
-#define XIIC_SR_REG_OFFSET (0x04+XIIC_REG_OFFSET) /* Status Register */
-#define XIIC_DTR_REG_OFFSET (0x08+XIIC_REG_OFFSET) /* Data Tx Register */
-#define XIIC_DRR_REG_OFFSET (0x0C+XIIC_REG_OFFSET) /* Data Rx Register */
-#define XIIC_ADR_REG_OFFSET (0x10+XIIC_REG_OFFSET) /* Address Register */
-#define XIIC_TFO_REG_OFFSET (0x14+XIIC_REG_OFFSET) /* Tx FIFO Occupancy */
-#define XIIC_RFO_REG_OFFSET (0x18+XIIC_REG_OFFSET) /* Rx FIFO Occupancy */
-#define XIIC_TBA_REG_OFFSET (0x1C+XIIC_REG_OFFSET) /* 10 Bit Address reg */
-#define XIIC_RFD_REG_OFFSET (0x20+XIIC_REG_OFFSET) /* Rx FIFO Depth reg */
-#define XIIC_GPO_REG_OFFSET (0x24+XIIC_REG_OFFSET) /* Output Register */
+#define XIIC_CR_REG_OFFSET (0x00 + XIIC_REG_OFFSET) /* Control Register */
+#define XIIC_SR_REG_OFFSET (0x04 + XIIC_REG_OFFSET) /* Status Register */
+#define XIIC_DTR_REG_OFFSET (0x08 + XIIC_REG_OFFSET) /* Data Tx Register */
+#define XIIC_DRR_REG_OFFSET (0x0C + XIIC_REG_OFFSET) /* Data Rx Register */
+#define XIIC_ADR_REG_OFFSET (0x10 + XIIC_REG_OFFSET) /* Address Register */
+#define XIIC_TFO_REG_OFFSET (0x14 + XIIC_REG_OFFSET) /* Tx FIFO Occupancy */
+#define XIIC_RFO_REG_OFFSET (0x18 + XIIC_REG_OFFSET) /* Rx FIFO Occupancy */
+#define XIIC_TBA_REG_OFFSET (0x1C + XIIC_REG_OFFSET) /* 10 Bit Address reg */
+#define XIIC_RFD_REG_OFFSET (0x20 + XIIC_REG_OFFSET) /* Rx FIFO Depth reg */
+#define XIIC_GPO_REG_OFFSET (0x24 + XIIC_REG_OFFSET) /* Output Register */
/* Control Register masks */
#define XIIC_CR_ENABLE_DEVICE_MASK 0x01 /* Device enable = 1 */
@@ -237,18 +236,21 @@ static inline int xiic_getreg32(struct xiic_i2c *i2c, int reg)
static inline void xiic_irq_dis(struct xiic_i2c *i2c, u32 mask)
{
u32 ier = xiic_getreg32(i2c, XIIC_IIER_OFFSET);
+
xiic_setreg32(i2c, XIIC_IIER_OFFSET, ier & ~mask);
}
static inline void xiic_irq_en(struct xiic_i2c *i2c, u32 mask)
{
u32 ier = xiic_getreg32(i2c, XIIC_IIER_OFFSET);
+
xiic_setreg32(i2c, XIIC_IIER_OFFSET, ier | mask);
}
static inline void xiic_irq_clr(struct xiic_i2c *i2c, u32 mask)
{
u32 isr = xiic_getreg32(i2c, XIIC_IISR_OFFSET);
+
xiic_setreg32(i2c, XIIC_IISR_OFFSET, isr & mask);
}
@@ -418,7 +420,8 @@ static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
while (len--) {
u16 data = i2c->tx_msg->buf[i2c->tx_pos++];
- if ((xiic_tx_space(i2c) == 0) && (i2c->nmsgs == 1)) {
+
+ if (!xiic_tx_space(i2c) && i2c->nmsgs == 1) {
/* last message in transfer -> STOP */
data |= XIIC_TX_DYN_STOP_MASK;
dev_dbg(i2c->adap.dev.parent, "%s TX STOP\n", __func__);
@@ -519,8 +522,8 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
/* Service requesting interrupt */
if ((pend & XIIC_INTR_ARB_LOST_MASK) ||
- ((pend & XIIC_INTR_TX_ERROR_MASK) &&
- !(pend & XIIC_INTR_RX_FULL_MASK))) {
+ ((pend & XIIC_INTR_TX_ERROR_MASK) &&
+ !(pend & XIIC_INTR_RX_FULL_MASK))) {
/* bus arbritration lost, or...
* Transmit error _OR_ RX completed
* if this happens when RX_FULL is not set
@@ -662,9 +665,8 @@ static int xiic_busy(struct xiic_i2c *i2c)
* should ignore it, since bus will never be released and i2c will be
* stuck forever.
*/
- if (i2c->singlemaster) {
+ if (i2c->singlemaster)
return 0;
- }
/* for instance if previous transfer was terminated due to TX error
* it might be that the bus is on it's way to become available
@@ -672,7 +674,7 @@ static int xiic_busy(struct xiic_i2c *i2c)
*/
err = xiic_bus_busy(i2c);
while (err && tries--) {
- msleep(1);
+ usleep_range(1000, 2000);
err = xiic_bus_busy(i2c);
}
@@ -867,7 +869,6 @@ static void xiic_start_send(struct xiic_i2c *i2c)
xiic_std_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 |
@@ -891,9 +892,6 @@ static irqreturn_t xiic_isr(int irq, void *dev_id)
/* Do not processes a devices interrupts if the device has no
* interrupts pending
*/
-
- dev_dbg(i2c->adap.dev.parent, "%s entry\n", __func__);
-
isr = xiic_getreg32(i2c, XIIC_IISR_OFFSET);
ier = xiic_getreg32(i2c, XIIC_IIER_OFFSET);
pend = isr & ier;
@@ -907,6 +905,7 @@ static void __xiic_start_xfer(struct xiic_i2c *i2c)
{
int first = 1;
int fifo_space = xiic_tx_fifo_space(i2c);
+
dev_dbg(i2c->adap.dev.parent, "%s entry, msg: %p, fifos space: %d\n",
__func__, i2c->tx_msg, fifo_space);
@@ -921,19 +920,20 @@ static void __xiic_start_xfer(struct xiic_i2c *i2c)
i2c->nmsgs--;
i2c->tx_msg++;
i2c->tx_pos = 0;
- } else
+ } else {
first = 0;
+ }
if (i2c->tx_msg->flags & I2C_M_RD) {
/* we dont date putting several reads in the FIFO */
xiic_start_recv(i2c);
return;
- } else {
- xiic_start_send(i2c);
- if (xiic_tx_space(i2c) != 0) {
- /* the message could not be completely sent */
- break;
- }
+ }
+
+ xiic_start_send(i2c);
+ if (xiic_tx_space(i2c) != 0) {
+ /* the message could not be completely sent */
+ break;
}
fifo_space = xiic_tx_fifo_space(i2c);
@@ -944,12 +944,12 @@ static void __xiic_start_xfer(struct xiic_i2c *i2c)
*/
if (i2c->nmsgs > 1 || xiic_tx_space(i2c))
xiic_irq_clr_en(i2c, XIIC_INTR_TX_HALF_MASK);
-
}
static int xiic_start_xfer(struct xiic_i2c *i2c)
{
int ret;
+
mutex_lock(&i2c->lock);
ret = xiic_reinit(i2c);
@@ -1001,8 +1001,8 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
goto out;
}
- if (wait_event_timeout(i2c->wait, (i2c->state == STATE_ERROR) ||
- (i2c->state == STATE_DONE), HZ)) {
+ if (wait_event_timeout(i2c->wait, i2c->state == STATE_ERROR ||
+ i2c->state == STATE_DONE, HZ)) {
err = (i2c->state == STATE_DONE) ? num : -EIO;
goto out;
} else {
@@ -1035,7 +1035,6 @@ static const struct i2c_adapter xiic_adapter = {
.algo = &xiic_algorithm,
};
-
static int xiic_i2c_probe(struct platform_device *pdev)
{
struct xiic_i2c *i2c;
@@ -1195,6 +1194,7 @@ static const struct dev_pm_ops xiic_dev_pm_ops = {
SET_RUNTIME_PM_OPS(xiic_i2c_runtime_suspend,
xiic_i2c_runtime_resume, NULL)
};
+
static struct platform_driver xiic_i2c_driver = {
.probe = xiic_i2c_probe,
.remove = xiic_i2c_remove,
@@ -1210,4 +1210,3 @@ module_platform_driver(xiic_i2c_driver);
MODULE_AUTHOR("[email protected]");
MODULE_DESCRIPTION("Xilinx I2C bus driver");
MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:"DRIVER_NAME);
--
2.17.1
From: Shubhrajyoti Datta <[email protected]>
Check the return value of xiic_reinit.
Signed-off-by: Shubhrajyoti Datta <[email protected]>
Signed-off-by: Raviteja Narayanam <[email protected]>
---
drivers/i2c/busses/i2c-xiic.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 51b5fd5768db..b2ac76d94212 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -556,6 +556,7 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
struct xiic_i2c *i2c = dev_id;
u32 pend, isr, ier;
u32 clr = 0;
+ int ret;
/* Get the interrupt Status from the IPIF. There is no clearing of
* interrupts in the IPIF. Interrupts must be cleared at the source.
@@ -592,7 +593,9 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
* fifos and the next message is a TX with len 0 (only addr)
* reset the IP instead of just flush fifos
*/
- xiic_reinit(i2c);
+ ret = xiic_reinit(i2c);
+ if (!ret)
+ dev_dbg(i2c->adap.dev.parent, "reinit failed\n");
if (i2c->rx_msg)
xiic_wakeup(i2c, STATE_ERROR);
--
2.17.1
When a group of messages are sent from user space as a set, if
the last message has less than Tx FIFO DEPTH number of bytes
to transfer, Tx half empty interrupt is triggered continuously
from the hardware. It is due to Bus not busy interrupt coming
along with Tx half empty and tx empty.
Hence, service the Tx interrupts before Bus not busy interrupt
to update the i2c message status correctly.
Signed-off-by: Raviteja Narayanam <[email protected]>
---
drivers/i2c/busses/i2c-xiic.c | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 2a8568b97c14..b0cfd9d15467 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -453,22 +453,6 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
}
}
}
- if (pend & XIIC_INTR_BNB_MASK) {
- /* IIC bus has transitioned to not busy */
- clr |= XIIC_INTR_BNB_MASK;
-
- /* The bus is not busy, disable BusNotBusy interrupt */
- xiic_irq_dis(i2c, XIIC_INTR_BNB_MASK);
-
- if (!i2c->tx_msg)
- goto out;
-
- if ((i2c->nmsgs == 1) && !i2c->rx_msg &&
- xiic_tx_space(i2c) == 0)
- xiic_wakeup(i2c, STATE_DONE);
- else
- xiic_wakeup(i2c, STATE_ERROR);
- }
if (pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK)) {
/* Transmit register/FIFO is empty or ½ empty */
@@ -505,6 +489,24 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
*/
xiic_irq_dis(i2c, XIIC_INTR_TX_HALF_MASK);
}
+
+ if (pend & XIIC_INTR_BNB_MASK) {
+ /* IIC bus has transitioned to not busy */
+ clr |= XIIC_INTR_BNB_MASK;
+
+ /* The bus is not busy, disable BusNotBusy interrupt */
+ xiic_irq_dis(i2c, XIIC_INTR_BNB_MASK);
+
+ if (!i2c->tx_msg)
+ goto out;
+
+ if (i2c->nmsgs == 1 && !i2c->rx_msg &&
+ xiic_tx_space(i2c) == 0)
+ xiic_wakeup(i2c, STATE_DONE);
+ else
+ xiic_wakeup(i2c, STATE_ERROR);
+ }
+
out:
dev_dbg(i2c->adap.dev.parent, "%s clr: 0x%x\n", __func__, clr);
--
2.17.1
From: Shubhrajyoti Datta <[email protected]>
Fix the coverity warning
mixed_enum_type: enumerated type mixed with another type
We are passing an enum in the xiic_wakeup lets change
the function parameters to reflect that.
Signed-off-by: Shubhrajyoti Datta <[email protected]>
Signed-off-by: Raviteja Narayanam <[email protected]>
---
drivers/i2c/busses/i2c-xiic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index b2ac76d94212..1a26e5ebfc6c 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -542,7 +542,7 @@ static void xiic_send_tx(struct xiic_i2c *i2c)
}
}
-static void xiic_wakeup(struct xiic_i2c *i2c, int code)
+static void xiic_wakeup(struct xiic_i2c *i2c, enum xilinx_i2c_state code)
{
i2c->tx_msg = NULL;
i2c->rx_msg = NULL;
--
2.17.1
Xilinx I2C IP has two modes of operation, both of which implement
I2C transactions. The only difference from sw perspective is the
programming sequence for these modes.
Dynamic mode -> Simple to program, less number of steps in sequence.
Standard mode -> Gives flexibility, more number of steps in sequence.
In dynamic mode, during the i2c-read transactions, if there is a
delay(> 200us) between the register writes (address & byte count),
read transaction fails. On a system with load, this scenario is
occurring frequently.
To avoid this, switch to standard mode if there is a read request.
Added a quirk to identify the IP version effected by this and follow
the standard mode.
Signed-off-by: Raviteja Narayanam <[email protected]>
---
drivers/i2c/busses/i2c-xiic.c | 58 +++++++++++++++++++++++++++--------
1 file changed, 45 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 1a26e5ebfc6c..2f0808249ceb 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -33,6 +33,8 @@
#define DRIVER_NAME "xiic-i2c"
+#define DYNAMIC_MODE_READ_BROKEN_BIT BIT(0)
+
enum xilinx_i2c_state {
STATE_DONE,
STATE_ERROR,
@@ -63,6 +65,7 @@ enum xiic_endian {
* @dynamic: Mode of controller
* @prev_msg_tx: Previous message is Tx
* @smbus_block_read: Flag to handle block read
+ * @quirks: To hold platform specific bug info
*/
struct xiic_i2c {
struct device *dev;
@@ -82,6 +85,11 @@ struct xiic_i2c {
bool dynamic;
bool prev_msg_tx;
bool smbus_block_read;
+ u32 quirks;
+};
+
+struct xiic_version_data {
+ u32 quirks;
};
#define XIIC_MSB_OFFSET 0
@@ -1032,6 +1040,7 @@ static int xiic_start_xfer(struct xiic_i2c *i2c)
static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
{
+ bool broken_read, max_read_len, smbus_blk_read;
struct xiic_i2c *i2c = i2c_get_adapdata(adap);
int err, count;
@@ -1056,13 +1065,22 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
i2c->prev_msg_tx = false;
/*
- * Enter standard mode only when read length is > 255 bytes or
- * for smbus_block_read transaction
+ * Scan through nmsgs, use dynamic mode when none of the below three
+ * conditions occur. We need standard mode even if one condition holds
+ * true in the entire array of messages in a single transfer.
+ * If read transaction as dynamic mode is broken for delayed reads
+ * in xlnx,axi-iic-2.0 / xlnx,xps-iic-2.00.a IP versions.
+ * If read length is > 255 bytes.
+ * If smbus_block_read transaction.
*/
for (count = 0; count < i2c->nmsgs; count++) {
- if (((i2c->tx_msg[count].flags & I2C_M_RD) &&
- i2c->tx_msg[count].len > MAX_READ_LENGTH_DYNAMIC) ||
- (i2c->tx_msg[count].flags & I2C_M_RECV_LEN)) {
+ broken_read = (i2c->quirks & DYNAMIC_MODE_READ_BROKEN_BIT) &&
+ (i2c->tx_msg[count].flags & I2C_M_RD);
+ max_read_len = (i2c->tx_msg[count].flags & I2C_M_RD) &&
+ (i2c->tx_msg[count].len > MAX_READ_LENGTH_DYNAMIC);
+ smbus_blk_read = (i2c->tx_msg[count].flags & I2C_M_RECV_LEN);
+
+ if (broken_read || max_read_len || smbus_blk_read) {
i2c->dynamic = false;
break;
}
@@ -1108,10 +1126,23 @@ static const struct i2c_adapter xiic_adapter = {
.algo = &xiic_algorithm,
};
+static const struct xiic_version_data xiic_2_00 = {
+ .quirks = DYNAMIC_MODE_READ_BROKEN_BIT,
+};
+
+#if defined(CONFIG_OF)
+static const struct of_device_id xiic_of_match[] = {
+ { .compatible = "xlnx,xps-iic-2.00.a", .data = &xiic_2_00 },
+ {},
+};
+MODULE_DEVICE_TABLE(of, xiic_of_match);
+#endif
+
static int xiic_i2c_probe(struct platform_device *pdev)
{
struct xiic_i2c *i2c;
struct xiic_i2c_platform_data *pdata;
+ const struct of_device_id *match;
struct resource *res;
int ret, irq;
u8 i;
@@ -1121,6 +1152,13 @@ static int xiic_i2c_probe(struct platform_device *pdev)
if (!i2c)
return -ENOMEM;
+ match = of_match_node(xiic_of_match, pdev->dev.of_node);
+ if (match && match->data) {
+ const struct xiic_version_data *data = match->data;
+
+ i2c->quirks = data->quirks;
+ }
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
i2c->base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(i2c->base))
@@ -1201,6 +1239,8 @@ static int xiic_i2c_probe(struct platform_device *pdev)
i2c_new_client_device(&i2c->adap, pdata->devices + i);
}
+ dev_info(&pdev->dev, "mmio %08lx irq %d\n", (unsigned long)res->start, irq);
+
return 0;
err_clk_dis:
@@ -1232,14 +1272,6 @@ static int xiic_i2c_remove(struct platform_device *pdev)
return 0;
}
-#if defined(CONFIG_OF)
-static const struct of_device_id xiic_of_match[] = {
- { .compatible = "xlnx,xps-iic-2.00.a", },
- {},
-};
-MODULE_DEVICE_TABLE(of, xiic_of_match);
-#endif
-
static int __maybe_unused xiic_i2c_runtime_suspend(struct device *dev)
{
struct xiic_i2c *i2c = dev_get_drvdata(dev);
--
2.17.1
As the 'DYNAMIC_MODE_READ_BROKEN_BIT' quirk is added in the driver,
we no longer enter dynamic mode for the effected IP versions.
So, remove local_irq_save/local_irq_restore APIs from driver.
Signed-off-by: Raviteja Narayanam <[email protected]>
---
drivers/i2c/busses/i2c-xiic.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 2f0808249ceb..f022a1885b6a 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -759,7 +759,6 @@ 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;
dev_dbg(i2c->adap.dev.parent, "%s entry, ISR: 0x%x, CR: 0x%x\n",
__func__, xiic_getreg32(i2c, XIIC_IISR_OFFSET),
@@ -790,7 +789,6 @@ static void xiic_start_recv(struct xiic_i2c *i2c)
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,
@@ -804,7 +802,6 @@ static void xiic_start_recv(struct xiic_i2c *i2c)
val |= msg->len;
xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, val);
- local_irq_restore(flags);
} else {
/*
* If previous message is Tx, make sure that Tx FIFO is empty
--
2.17.1
Xilinx AXI I2C IP is updated with a bug fix for dynamic mode reads.
Older IPs are handled with a workaround in which they are using
xiic standard mode for all these effected use cases.
Added the new IP version to compatible.
Signed-off-by: Raviteja Narayanam <[email protected]>
---
drivers/i2c/busses/i2c-xiic.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index f022a1885b6a..45bd3b1a4c22 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -1130,6 +1130,7 @@ static const struct xiic_version_data xiic_2_00 = {
#if defined(CONFIG_OF)
static const struct of_device_id xiic_of_match[] = {
{ .compatible = "xlnx,xps-iic-2.00.a", .data = &xiic_2_00 },
+ { .compatible = "xlnx,axi-iic-2.1", },
{},
};
MODULE_DEVICE_TABLE(of, xiic_of_match);
--
2.17.1
Added the xilinx I2C new version 'xlnx,axi-iic-2.1' string to compatible
Signed-off-by: Raviteja Narayanam <[email protected]>
---
.../devicetree/bindings/i2c/xlnx,xps-iic-2.00.a.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/i2c/xlnx,xps-iic-2.00.a.yaml b/Documentation/devicetree/bindings/i2c/xlnx,xps-iic-2.00.a.yaml
index 715dcfa5a922..e516c1ed358c 100644
--- a/Documentation/devicetree/bindings/i2c/xlnx,xps-iic-2.00.a.yaml
+++ b/Documentation/devicetree/bindings/i2c/xlnx,xps-iic-2.00.a.yaml
@@ -14,7 +14,9 @@ allOf:
properties:
compatible:
- const: xlnx,xps-iic-2.00.a
+ enum:
+ - xlnx,xps-iic-2.00.a
+ - xlnx,axi-iic-2.1
reg:
maxItems: 1
--
2.17.1
On Mon, 2021-05-31 at 07:19 -0600, Raviteja Narayanam wrote:
> From: Michal Simek <[email protected]>
>
> Most of these stuff are reported by checkpatch.
trivia:
> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
[]
> @@ -519,8 +522,8 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
> ?
>
> ? /* Service requesting interrupt */
> ? if ((pend & XIIC_INTR_ARB_LOST_MASK) ||
> - ((pend & XIIC_INTR_TX_ERROR_MASK) &&
> - !(pend & XIIC_INTR_RX_FULL_MASK))) {
> + ((pend & XIIC_INTR_TX_ERROR_MASK) &&
> + !(pend & XIIC_INTR_RX_FULL_MASK))) {
This last line would more commonly be indented one more space to
align to the appropriate open parenthesis depth.
tab then 4 spaces for
((pend & XIIC_INTR_TX_ERROR_MASK) &&
tab then 5 spaces for
!(pend & XIIC_INTR_RX_FULL_MASK))) {