* [PATCH v2 0/3] Cadence I2C driver fixes
@ 2014-12-03 12:35 ` Harini Katakam
0 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-03 12:35 UTC (permalink / raw)
To: wsa, mark.rutland
Cc: michal.simek, soren.brinkmann, linux-arm-kernel, linux-i2c,
linux-kernel, vishnum, harinik, harinikatakamlinux
This series implements workarounds for some bugs in Cadence I2C controller.
- The I2C controller when configured in Master Mode generates invalid read transactions.
When HOLD bit is set and HW timeout occurs, invalid read transactions are
generated on the bus. This will affect repeated start conditions and large
data transfer condtions where transfer_size_register has to be updated again.
The transfer size register rolls over erroneously under these conditions.
Note that this HW timeout cannot be disabled even if the interrupt is unused.
Errata link: http://www.xilinx.com/support/answers/61664.html
-The I2C controller when configured in Master Mode is the missing master completion interrupt.
During repeated start condition, the driver has to set the HOLD bit for
the set of transfer being done together and clear the HOLD bit just before
the last transfer. But the controller does not indicate completion when
a read/receive transfer is done if the HOLD bit is set. This affects
all repeated start operation where a read/receive transfer is not
the last transfer.
Errata link: http://www.xilinx.com/support/answers/61665.html
To address the above,
- >252 byte transfer are done such that transfer_size never becomes zero.
- timeout register value is increased (even though the driver doesn't use this).
- A check is performed to see if there any transfer following a
read/receive transfer in the set of messages using repeated start.
An error is returned in such cases because if we proceed, completion interrupt
is never obtained and a SW timeout will occur.
Harini Katakam (2):
i2c: cadence: Handle > 252 byte transfers
i2c: cadence: Check for errata condition involving master receive
Vishnu Motghare (1):
i2c: cadence: Set the hardware time-out register to maximum value
drivers/i2c/busses/i2c-cadence.c | 178 ++++++++++++++++++++++----------------
1 file changed, 103 insertions(+), 75 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 0/3] Cadence I2C driver fixes
@ 2014-12-03 12:35 ` Harini Katakam
0 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-03 12:35 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, mark.rutland-5wv7dgnIgG8
Cc: michal.simek-gjFFaj9aHVfQT0dZR+AlfA,
soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
vishnum-gjFFaj9aHVfQT0dZR+AlfA, harinik-gjFFaj9aHVfQT0dZR+AlfA,
harinikatakamlinux-Re5JQEeQqe8AvxtiuMwx3w
This series implements workarounds for some bugs in Cadence I2C controller.
- The I2C controller when configured in Master Mode generates invalid read transactions.
When HOLD bit is set and HW timeout occurs, invalid read transactions are
generated on the bus. This will affect repeated start conditions and large
data transfer condtions where transfer_size_register has to be updated again.
The transfer size register rolls over erroneously under these conditions.
Note that this HW timeout cannot be disabled even if the interrupt is unused.
Errata link: http://www.xilinx.com/support/answers/61664.html
-The I2C controller when configured in Master Mode is the missing master completion interrupt.
During repeated start condition, the driver has to set the HOLD bit for
the set of transfer being done together and clear the HOLD bit just before
the last transfer. But the controller does not indicate completion when
a read/receive transfer is done if the HOLD bit is set. This affects
all repeated start operation where a read/receive transfer is not
the last transfer.
Errata link: http://www.xilinx.com/support/answers/61665.html
To address the above,
- >252 byte transfer are done such that transfer_size never becomes zero.
- timeout register value is increased (even though the driver doesn't use this).
- A check is performed to see if there any transfer following a
read/receive transfer in the set of messages using repeated start.
An error is returned in such cases because if we proceed, completion interrupt
is never obtained and a SW timeout will occur.
Harini Katakam (2):
i2c: cadence: Handle > 252 byte transfers
i2c: cadence: Check for errata condition involving master receive
Vishnu Motghare (1):
i2c: cadence: Set the hardware time-out register to maximum value
drivers/i2c/busses/i2c-cadence.c | 178 ++++++++++++++++++++++----------------
1 file changed, 103 insertions(+), 75 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 0/3] Cadence I2C driver fixes
@ 2014-12-03 12:35 ` Harini Katakam
0 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-03 12:35 UTC (permalink / raw)
To: linux-arm-kernel
This series implements workarounds for some bugs in Cadence I2C controller.
- The I2C controller when configured in Master Mode generates invalid read transactions.
When HOLD bit is set and HW timeout occurs, invalid read transactions are
generated on the bus. This will affect repeated start conditions and large
data transfer condtions where transfer_size_register has to be updated again.
The transfer size register rolls over erroneously under these conditions.
Note that this HW timeout cannot be disabled even if the interrupt is unused.
Errata link: http://www.xilinx.com/support/answers/61664.html
-The I2C controller when configured in Master Mode is the missing master completion interrupt.
During repeated start condition, the driver has to set the HOLD bit for
the set of transfer being done together and clear the HOLD bit just before
the last transfer. But the controller does not indicate completion when
a read/receive transfer is done if the HOLD bit is set. This affects
all repeated start operation where a read/receive transfer is not
the last transfer.
Errata link: http://www.xilinx.com/support/answers/61665.html
To address the above,
- >252 byte transfer are done such that transfer_size never becomes zero.
- timeout register value is increased (even though the driver doesn't use this).
- A check is performed to see if there any transfer following a
read/receive transfer in the set of messages using repeated start.
An error is returned in such cases because if we proceed, completion interrupt
is never obtained and a SW timeout will occur.
Harini Katakam (2):
i2c: cadence: Handle > 252 byte transfers
i2c: cadence: Check for errata condition involving master receive
Vishnu Motghare (1):
i2c: cadence: Set the hardware time-out register to maximum value
drivers/i2c/busses/i2c-cadence.c | 178 ++++++++++++++++++++++----------------
1 file changed, 103 insertions(+), 75 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
@ 2014-12-03 12:35 ` Harini Katakam
0 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-03 12:35 UTC (permalink / raw)
To: wsa, mark.rutland
Cc: michal.simek, soren.brinkmann, linux-arm-kernel, linux-i2c,
linux-kernel, vishnum, harinik, harinikatakamlinux
The I2C controller sends a NACK to the slave when transfer size register
reaches zero, irrespective of the hold bit. So, in order to handle transfers
greater than 252 bytes, the transfer size register has to be maintained at a
value >= 1. This patch implements the same.
The interrupt status is cleared at the beginning of the isr instead of
the end, to avoid missing any interrupts - this is in sync with the new
transfer handling.
Signed-off-by: Harini Katakam <harinik@xilinx.com>
---
v2:
No changes
---
drivers/i2c/busses/i2c-cadence.c | 156 ++++++++++++++++++++------------------
1 file changed, 81 insertions(+), 75 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 63f3f03..e54899e 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -126,6 +126,7 @@
* @suspended: Flag holding the device's PM status
* @send_count: Number of bytes still expected to send
* @recv_count: Number of bytes still expected to receive
+ * @curr_recv_count: Number of bytes to be received in current transfer
* @irq: IRQ number
* @input_clk: Input clock to I2C controller
* @i2c_clk: Maximum I2C clock speed
@@ -144,6 +145,7 @@ struct cdns_i2c {
u8 suspended;
unsigned int send_count;
unsigned int recv_count;
+ unsigned int curr_recv_count;
int irq;
unsigned long input_clk;
unsigned int i2c_clk;
@@ -180,14 +182,15 @@ static void cdns_i2c_clear_bus_hold(struct cdns_i2c *id)
*/
static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
{
- unsigned int isr_status, avail_bytes;
- unsigned int bytes_to_recv, bytes_to_send;
+ unsigned int isr_status, avail_bytes, updatetx;
+ unsigned int bytes_to_send;
struct cdns_i2c *id = ptr;
/* Signal completion only after everything is updated */
int done_flag = 0;
irqreturn_t status = IRQ_NONE;
isr_status = cdns_i2c_readreg(CDNS_I2C_ISR_OFFSET);
+ cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
/* Handling nack and arbitration lost interrupt */
if (isr_status & (CDNS_I2C_IXR_NACK | CDNS_I2C_IXR_ARB_LOST)) {
@@ -195,89 +198,91 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
status = IRQ_HANDLED;
}
- /* Handling Data interrupt */
- if ((isr_status & CDNS_I2C_IXR_DATA) &&
- (id->recv_count >= CDNS_I2C_DATA_INTR_DEPTH)) {
- /* Always read data interrupt threshold bytes */
- bytes_to_recv = CDNS_I2C_DATA_INTR_DEPTH;
- id->recv_count -= CDNS_I2C_DATA_INTR_DEPTH;
- avail_bytes = cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
-
- /*
- * if the tranfer size register value is zero, then
- * check for the remaining bytes and update the
- * transfer size register.
- */
- if (!avail_bytes) {
- if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
- cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
- CDNS_I2C_XFER_SIZE_OFFSET);
- else
- cdns_i2c_writereg(id->recv_count,
- CDNS_I2C_XFER_SIZE_OFFSET);
- }
+ updatetx = 0;
+ if (id->recv_count > id->curr_recv_count)
+ updatetx = 1;
+
+ /* When receiving, handle data and transfer complete interrupts */
+ if (id->p_recv_buf &&
+ ((isr_status & CDNS_I2C_IXR_COMP) ||
+ (isr_status & CDNS_I2C_IXR_DATA))) {
+ while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
+ CDNS_I2C_SR_RXDV) {
+ if ((id->recv_count < CDNS_I2C_FIFO_DEPTH) &&
+ !id->bus_hold_flag)
+ cdns_i2c_clear_bus_hold(id);
- /* Process the data received */
- while (bytes_to_recv--)
*(id->p_recv_buf)++ =
cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
+ id->recv_count--;
+ id->curr_recv_count--;
- if (!id->bus_hold_flag &&
- (id->recv_count <= CDNS_I2C_FIFO_DEPTH))
- cdns_i2c_clear_bus_hold(id);
+ if (updatetx &&
+ (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1))
+ break;
+ }
- status = IRQ_HANDLED;
- }
+ if (updatetx &&
+ (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1)) {
+ /* wait while fifo is full */
+ while (cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET) !=
+ (id->curr_recv_count - CDNS_I2C_FIFO_DEPTH))
+ ;
- /* Handling Transfer Complete interrupt */
- if (isr_status & CDNS_I2C_IXR_COMP) {
- if (!id->p_recv_buf) {
- /*
- * If the device is sending data If there is further
- * data to be sent. Calculate the available space
- * in FIFO and fill the FIFO with that many bytes.
- */
- if (id->send_count) {
- avail_bytes = CDNS_I2C_FIFO_DEPTH -
- cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
- if (id->send_count > avail_bytes)
- bytes_to_send = avail_bytes;
- else
- bytes_to_send = id->send_count;
-
- while (bytes_to_send--) {
- cdns_i2c_writereg(
- (*(id->p_send_buf)++),
- CDNS_I2C_DATA_OFFSET);
- id->send_count--;
- }
+ if (((int)(id->recv_count) - CDNS_I2C_FIFO_DEPTH) >
+ CDNS_I2C_TRANSFER_SIZE) {
+ cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
+ CDNS_I2C_XFER_SIZE_OFFSET);
+ id->curr_recv_count = CDNS_I2C_TRANSFER_SIZE +
+ CDNS_I2C_FIFO_DEPTH;
} else {
- /*
- * Signal the completion of transaction and
- * clear the hold bus bit if there are no
- * further messages to be processed.
- */
- done_flag = 1;
+ cdns_i2c_writereg(id->recv_count -
+ CDNS_I2C_FIFO_DEPTH,
+ CDNS_I2C_XFER_SIZE_OFFSET);
+ id->curr_recv_count = id->recv_count;
}
- if (!id->send_count && !id->bus_hold_flag)
- cdns_i2c_clear_bus_hold(id);
- } else {
+ }
+
+ if ((isr_status & CDNS_I2C_IXR_COMP) && !id->recv_count) {
if (!id->bus_hold_flag)
cdns_i2c_clear_bus_hold(id);
+ done_flag = 1;
+ }
+
+ status = IRQ_HANDLED;
+ }
+
+ /* When sending, handle transfer complete interrupt */
+ if ((isr_status & CDNS_I2C_IXR_COMP) && !id->p_recv_buf) {
+ /*
+ * If the device is sending data If there is further
+ * data to be sent. Calculate the available space
+ * in FIFO and fill the FIFO with that many bytes.
+ */
+ if (id->send_count) {
+ avail_bytes = CDNS_I2C_FIFO_DEPTH -
+ cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
+ if (id->send_count > avail_bytes)
+ bytes_to_send = avail_bytes;
+ else
+ bytes_to_send = id->send_count;
+
+ while (bytes_to_send--) {
+ cdns_i2c_writereg(
+ (*(id->p_send_buf)++),
+ CDNS_I2C_DATA_OFFSET);
+ id->send_count--;
+ }
+ } else {
/*
- * If the device is receiving data, then signal
- * the completion of transaction and read the data
- * present in the FIFO. Signal the completion of
- * transaction.
+ * Signal the completion of transaction and
+ * clear the hold bus bit if there are no
+ * further messages to be processed.
*/
- while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
- CDNS_I2C_SR_RXDV) {
- *(id->p_recv_buf)++ =
- cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
- id->recv_count--;
- }
done_flag = 1;
}
+ if (!id->send_count && !id->bus_hold_flag)
+ cdns_i2c_clear_bus_hold(id);
status = IRQ_HANDLED;
}
@@ -287,8 +292,6 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
if (id->err_status)
status = IRQ_HANDLED;
- cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
-
if (done_flag)
complete(&id->xfer_done);
@@ -314,6 +317,8 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
if (id->p_msg->flags & I2C_M_RECV_LEN)
id->recv_count = I2C_SMBUS_BLOCK_MAX + 1;
+ id->curr_recv_count = id->recv_count;
+
/*
* Check for the message size against FIFO depth and set the
* 'hold bus' bit if it is greater than FIFO depth.
@@ -333,10 +338,11 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
* receive if it is less than transfer size and transfer size if
* it is more. Enable the interrupts.
*/
- if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
+ if (id->recv_count > CDNS_I2C_TRANSFER_SIZE) {
cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
CDNS_I2C_XFER_SIZE_OFFSET);
- else
+ id->curr_recv_count = CDNS_I2C_TRANSFER_SIZE;
+ } else
cdns_i2c_writereg(id->recv_count, CDNS_I2C_XFER_SIZE_OFFSET);
/* Clear the bus hold flag if bytes to receive is less than FIFO size */
if (!id->bus_hold_flag &&
--
1.7.9.5
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
@ 2014-12-03 12:35 ` Harini Katakam
0 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-03 12:35 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, mark.rutland-5wv7dgnIgG8
Cc: michal.simek-gjFFaj9aHVfQT0dZR+AlfA,
soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
vishnum-gjFFaj9aHVfQT0dZR+AlfA, harinik-gjFFaj9aHVfQT0dZR+AlfA,
harinikatakamlinux-Re5JQEeQqe8AvxtiuMwx3w
The I2C controller sends a NACK to the slave when transfer size register
reaches zero, irrespective of the hold bit. So, in order to handle transfers
greater than 252 bytes, the transfer size register has to be maintained at a
value >= 1. This patch implements the same.
The interrupt status is cleared at the beginning of the isr instead of
the end, to avoid missing any interrupts - this is in sync with the new
transfer handling.
Signed-off-by: Harini Katakam <harinik-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
v2:
No changes
---
drivers/i2c/busses/i2c-cadence.c | 156 ++++++++++++++++++++------------------
1 file changed, 81 insertions(+), 75 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 63f3f03..e54899e 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -126,6 +126,7 @@
* @suspended: Flag holding the device's PM status
* @send_count: Number of bytes still expected to send
* @recv_count: Number of bytes still expected to receive
+ * @curr_recv_count: Number of bytes to be received in current transfer
* @irq: IRQ number
* @input_clk: Input clock to I2C controller
* @i2c_clk: Maximum I2C clock speed
@@ -144,6 +145,7 @@ struct cdns_i2c {
u8 suspended;
unsigned int send_count;
unsigned int recv_count;
+ unsigned int curr_recv_count;
int irq;
unsigned long input_clk;
unsigned int i2c_clk;
@@ -180,14 +182,15 @@ static void cdns_i2c_clear_bus_hold(struct cdns_i2c *id)
*/
static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
{
- unsigned int isr_status, avail_bytes;
- unsigned int bytes_to_recv, bytes_to_send;
+ unsigned int isr_status, avail_bytes, updatetx;
+ unsigned int bytes_to_send;
struct cdns_i2c *id = ptr;
/* Signal completion only after everything is updated */
int done_flag = 0;
irqreturn_t status = IRQ_NONE;
isr_status = cdns_i2c_readreg(CDNS_I2C_ISR_OFFSET);
+ cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
/* Handling nack and arbitration lost interrupt */
if (isr_status & (CDNS_I2C_IXR_NACK | CDNS_I2C_IXR_ARB_LOST)) {
@@ -195,89 +198,91 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
status = IRQ_HANDLED;
}
- /* Handling Data interrupt */
- if ((isr_status & CDNS_I2C_IXR_DATA) &&
- (id->recv_count >= CDNS_I2C_DATA_INTR_DEPTH)) {
- /* Always read data interrupt threshold bytes */
- bytes_to_recv = CDNS_I2C_DATA_INTR_DEPTH;
- id->recv_count -= CDNS_I2C_DATA_INTR_DEPTH;
- avail_bytes = cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
-
- /*
- * if the tranfer size register value is zero, then
- * check for the remaining bytes and update the
- * transfer size register.
- */
- if (!avail_bytes) {
- if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
- cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
- CDNS_I2C_XFER_SIZE_OFFSET);
- else
- cdns_i2c_writereg(id->recv_count,
- CDNS_I2C_XFER_SIZE_OFFSET);
- }
+ updatetx = 0;
+ if (id->recv_count > id->curr_recv_count)
+ updatetx = 1;
+
+ /* When receiving, handle data and transfer complete interrupts */
+ if (id->p_recv_buf &&
+ ((isr_status & CDNS_I2C_IXR_COMP) ||
+ (isr_status & CDNS_I2C_IXR_DATA))) {
+ while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
+ CDNS_I2C_SR_RXDV) {
+ if ((id->recv_count < CDNS_I2C_FIFO_DEPTH) &&
+ !id->bus_hold_flag)
+ cdns_i2c_clear_bus_hold(id);
- /* Process the data received */
- while (bytes_to_recv--)
*(id->p_recv_buf)++ =
cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
+ id->recv_count--;
+ id->curr_recv_count--;
- if (!id->bus_hold_flag &&
- (id->recv_count <= CDNS_I2C_FIFO_DEPTH))
- cdns_i2c_clear_bus_hold(id);
+ if (updatetx &&
+ (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1))
+ break;
+ }
- status = IRQ_HANDLED;
- }
+ if (updatetx &&
+ (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1)) {
+ /* wait while fifo is full */
+ while (cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET) !=
+ (id->curr_recv_count - CDNS_I2C_FIFO_DEPTH))
+ ;
- /* Handling Transfer Complete interrupt */
- if (isr_status & CDNS_I2C_IXR_COMP) {
- if (!id->p_recv_buf) {
- /*
- * If the device is sending data If there is further
- * data to be sent. Calculate the available space
- * in FIFO and fill the FIFO with that many bytes.
- */
- if (id->send_count) {
- avail_bytes = CDNS_I2C_FIFO_DEPTH -
- cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
- if (id->send_count > avail_bytes)
- bytes_to_send = avail_bytes;
- else
- bytes_to_send = id->send_count;
-
- while (bytes_to_send--) {
- cdns_i2c_writereg(
- (*(id->p_send_buf)++),
- CDNS_I2C_DATA_OFFSET);
- id->send_count--;
- }
+ if (((int)(id->recv_count) - CDNS_I2C_FIFO_DEPTH) >
+ CDNS_I2C_TRANSFER_SIZE) {
+ cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
+ CDNS_I2C_XFER_SIZE_OFFSET);
+ id->curr_recv_count = CDNS_I2C_TRANSFER_SIZE +
+ CDNS_I2C_FIFO_DEPTH;
} else {
- /*
- * Signal the completion of transaction and
- * clear the hold bus bit if there are no
- * further messages to be processed.
- */
- done_flag = 1;
+ cdns_i2c_writereg(id->recv_count -
+ CDNS_I2C_FIFO_DEPTH,
+ CDNS_I2C_XFER_SIZE_OFFSET);
+ id->curr_recv_count = id->recv_count;
}
- if (!id->send_count && !id->bus_hold_flag)
- cdns_i2c_clear_bus_hold(id);
- } else {
+ }
+
+ if ((isr_status & CDNS_I2C_IXR_COMP) && !id->recv_count) {
if (!id->bus_hold_flag)
cdns_i2c_clear_bus_hold(id);
+ done_flag = 1;
+ }
+
+ status = IRQ_HANDLED;
+ }
+
+ /* When sending, handle transfer complete interrupt */
+ if ((isr_status & CDNS_I2C_IXR_COMP) && !id->p_recv_buf) {
+ /*
+ * If the device is sending data If there is further
+ * data to be sent. Calculate the available space
+ * in FIFO and fill the FIFO with that many bytes.
+ */
+ if (id->send_count) {
+ avail_bytes = CDNS_I2C_FIFO_DEPTH -
+ cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
+ if (id->send_count > avail_bytes)
+ bytes_to_send = avail_bytes;
+ else
+ bytes_to_send = id->send_count;
+
+ while (bytes_to_send--) {
+ cdns_i2c_writereg(
+ (*(id->p_send_buf)++),
+ CDNS_I2C_DATA_OFFSET);
+ id->send_count--;
+ }
+ } else {
/*
- * If the device is receiving data, then signal
- * the completion of transaction and read the data
- * present in the FIFO. Signal the completion of
- * transaction.
+ * Signal the completion of transaction and
+ * clear the hold bus bit if there are no
+ * further messages to be processed.
*/
- while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
- CDNS_I2C_SR_RXDV) {
- *(id->p_recv_buf)++ =
- cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
- id->recv_count--;
- }
done_flag = 1;
}
+ if (!id->send_count && !id->bus_hold_flag)
+ cdns_i2c_clear_bus_hold(id);
status = IRQ_HANDLED;
}
@@ -287,8 +292,6 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
if (id->err_status)
status = IRQ_HANDLED;
- cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
-
if (done_flag)
complete(&id->xfer_done);
@@ -314,6 +317,8 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
if (id->p_msg->flags & I2C_M_RECV_LEN)
id->recv_count = I2C_SMBUS_BLOCK_MAX + 1;
+ id->curr_recv_count = id->recv_count;
+
/*
* Check for the message size against FIFO depth and set the
* 'hold bus' bit if it is greater than FIFO depth.
@@ -333,10 +338,11 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
* receive if it is less than transfer size and transfer size if
* it is more. Enable the interrupts.
*/
- if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
+ if (id->recv_count > CDNS_I2C_TRANSFER_SIZE) {
cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
CDNS_I2C_XFER_SIZE_OFFSET);
- else
+ id->curr_recv_count = CDNS_I2C_TRANSFER_SIZE;
+ } else
cdns_i2c_writereg(id->recv_count, CDNS_I2C_XFER_SIZE_OFFSET);
/* Clear the bus hold flag if bytes to receive is less than FIFO size */
if (!id->bus_hold_flag &&
--
1.7.9.5
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
@ 2014-12-03 12:35 ` Harini Katakam
0 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-03 12:35 UTC (permalink / raw)
To: linux-arm-kernel
The I2C controller sends a NACK to the slave when transfer size register
reaches zero, irrespective of the hold bit. So, in order to handle transfers
greater than 252 bytes, the transfer size register has to be maintained at a
value >= 1. This patch implements the same.
The interrupt status is cleared at the beginning of the isr instead of
the end, to avoid missing any interrupts - this is in sync with the new
transfer handling.
Signed-off-by: Harini Katakam <harinik@xilinx.com>
---
v2:
No changes
---
drivers/i2c/busses/i2c-cadence.c | 156 ++++++++++++++++++++------------------
1 file changed, 81 insertions(+), 75 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 63f3f03..e54899e 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -126,6 +126,7 @@
* @suspended: Flag holding the device's PM status
* @send_count: Number of bytes still expected to send
* @recv_count: Number of bytes still expected to receive
+ * @curr_recv_count: Number of bytes to be received in current transfer
* @irq: IRQ number
* @input_clk: Input clock to I2C controller
* @i2c_clk: Maximum I2C clock speed
@@ -144,6 +145,7 @@ struct cdns_i2c {
u8 suspended;
unsigned int send_count;
unsigned int recv_count;
+ unsigned int curr_recv_count;
int irq;
unsigned long input_clk;
unsigned int i2c_clk;
@@ -180,14 +182,15 @@ static void cdns_i2c_clear_bus_hold(struct cdns_i2c *id)
*/
static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
{
- unsigned int isr_status, avail_bytes;
- unsigned int bytes_to_recv, bytes_to_send;
+ unsigned int isr_status, avail_bytes, updatetx;
+ unsigned int bytes_to_send;
struct cdns_i2c *id = ptr;
/* Signal completion only after everything is updated */
int done_flag = 0;
irqreturn_t status = IRQ_NONE;
isr_status = cdns_i2c_readreg(CDNS_I2C_ISR_OFFSET);
+ cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
/* Handling nack and arbitration lost interrupt */
if (isr_status & (CDNS_I2C_IXR_NACK | CDNS_I2C_IXR_ARB_LOST)) {
@@ -195,89 +198,91 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
status = IRQ_HANDLED;
}
- /* Handling Data interrupt */
- if ((isr_status & CDNS_I2C_IXR_DATA) &&
- (id->recv_count >= CDNS_I2C_DATA_INTR_DEPTH)) {
- /* Always read data interrupt threshold bytes */
- bytes_to_recv = CDNS_I2C_DATA_INTR_DEPTH;
- id->recv_count -= CDNS_I2C_DATA_INTR_DEPTH;
- avail_bytes = cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
-
- /*
- * if the tranfer size register value is zero, then
- * check for the remaining bytes and update the
- * transfer size register.
- */
- if (!avail_bytes) {
- if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
- cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
- CDNS_I2C_XFER_SIZE_OFFSET);
- else
- cdns_i2c_writereg(id->recv_count,
- CDNS_I2C_XFER_SIZE_OFFSET);
- }
+ updatetx = 0;
+ if (id->recv_count > id->curr_recv_count)
+ updatetx = 1;
+
+ /* When receiving, handle data and transfer complete interrupts */
+ if (id->p_recv_buf &&
+ ((isr_status & CDNS_I2C_IXR_COMP) ||
+ (isr_status & CDNS_I2C_IXR_DATA))) {
+ while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
+ CDNS_I2C_SR_RXDV) {
+ if ((id->recv_count < CDNS_I2C_FIFO_DEPTH) &&
+ !id->bus_hold_flag)
+ cdns_i2c_clear_bus_hold(id);
- /* Process the data received */
- while (bytes_to_recv--)
*(id->p_recv_buf)++ =
cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
+ id->recv_count--;
+ id->curr_recv_count--;
- if (!id->bus_hold_flag &&
- (id->recv_count <= CDNS_I2C_FIFO_DEPTH))
- cdns_i2c_clear_bus_hold(id);
+ if (updatetx &&
+ (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1))
+ break;
+ }
- status = IRQ_HANDLED;
- }
+ if (updatetx &&
+ (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1)) {
+ /* wait while fifo is full */
+ while (cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET) !=
+ (id->curr_recv_count - CDNS_I2C_FIFO_DEPTH))
+ ;
- /* Handling Transfer Complete interrupt */
- if (isr_status & CDNS_I2C_IXR_COMP) {
- if (!id->p_recv_buf) {
- /*
- * If the device is sending data If there is further
- * data to be sent. Calculate the available space
- * in FIFO and fill the FIFO with that many bytes.
- */
- if (id->send_count) {
- avail_bytes = CDNS_I2C_FIFO_DEPTH -
- cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
- if (id->send_count > avail_bytes)
- bytes_to_send = avail_bytes;
- else
- bytes_to_send = id->send_count;
-
- while (bytes_to_send--) {
- cdns_i2c_writereg(
- (*(id->p_send_buf)++),
- CDNS_I2C_DATA_OFFSET);
- id->send_count--;
- }
+ if (((int)(id->recv_count) - CDNS_I2C_FIFO_DEPTH) >
+ CDNS_I2C_TRANSFER_SIZE) {
+ cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
+ CDNS_I2C_XFER_SIZE_OFFSET);
+ id->curr_recv_count = CDNS_I2C_TRANSFER_SIZE +
+ CDNS_I2C_FIFO_DEPTH;
} else {
- /*
- * Signal the completion of transaction and
- * clear the hold bus bit if there are no
- * further messages to be processed.
- */
- done_flag = 1;
+ cdns_i2c_writereg(id->recv_count -
+ CDNS_I2C_FIFO_DEPTH,
+ CDNS_I2C_XFER_SIZE_OFFSET);
+ id->curr_recv_count = id->recv_count;
}
- if (!id->send_count && !id->bus_hold_flag)
- cdns_i2c_clear_bus_hold(id);
- } else {
+ }
+
+ if ((isr_status & CDNS_I2C_IXR_COMP) && !id->recv_count) {
if (!id->bus_hold_flag)
cdns_i2c_clear_bus_hold(id);
+ done_flag = 1;
+ }
+
+ status = IRQ_HANDLED;
+ }
+
+ /* When sending, handle transfer complete interrupt */
+ if ((isr_status & CDNS_I2C_IXR_COMP) && !id->p_recv_buf) {
+ /*
+ * If the device is sending data If there is further
+ * data to be sent. Calculate the available space
+ * in FIFO and fill the FIFO with that many bytes.
+ */
+ if (id->send_count) {
+ avail_bytes = CDNS_I2C_FIFO_DEPTH -
+ cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
+ if (id->send_count > avail_bytes)
+ bytes_to_send = avail_bytes;
+ else
+ bytes_to_send = id->send_count;
+
+ while (bytes_to_send--) {
+ cdns_i2c_writereg(
+ (*(id->p_send_buf)++),
+ CDNS_I2C_DATA_OFFSET);
+ id->send_count--;
+ }
+ } else {
/*
- * If the device is receiving data, then signal
- * the completion of transaction and read the data
- * present in the FIFO. Signal the completion of
- * transaction.
+ * Signal the completion of transaction and
+ * clear the hold bus bit if there are no
+ * further messages to be processed.
*/
- while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
- CDNS_I2C_SR_RXDV) {
- *(id->p_recv_buf)++ =
- cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
- id->recv_count--;
- }
done_flag = 1;
}
+ if (!id->send_count && !id->bus_hold_flag)
+ cdns_i2c_clear_bus_hold(id);
status = IRQ_HANDLED;
}
@@ -287,8 +292,6 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
if (id->err_status)
status = IRQ_HANDLED;
- cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
-
if (done_flag)
complete(&id->xfer_done);
@@ -314,6 +317,8 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
if (id->p_msg->flags & I2C_M_RECV_LEN)
id->recv_count = I2C_SMBUS_BLOCK_MAX + 1;
+ id->curr_recv_count = id->recv_count;
+
/*
* Check for the message size against FIFO depth and set the
* 'hold bus' bit if it is greater than FIFO depth.
@@ -333,10 +338,11 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
* receive if it is less than transfer size and transfer size if
* it is more. Enable the interrupts.
*/
- if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
+ if (id->recv_count > CDNS_I2C_TRANSFER_SIZE) {
cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
CDNS_I2C_XFER_SIZE_OFFSET);
- else
+ id->curr_recv_count = CDNS_I2C_TRANSFER_SIZE;
+ } else
cdns_i2c_writereg(id->recv_count, CDNS_I2C_XFER_SIZE_OFFSET);
/* Clear the bus hold flag if bytes to receive is less than FIFO size */
if (!id->bus_hold_flag &&
--
1.7.9.5
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 2/3] i2c: cadence: Set the hardware time-out register to maximum value
2014-12-03 12:35 ` Harini Katakam
@ 2014-12-03 12:35 ` Harini Katakam
-1 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-03 12:35 UTC (permalink / raw)
To: wsa, mark.rutland
Cc: michal.simek, soren.brinkmann, linux-arm-kernel, linux-i2c,
linux-kernel, vishnum, harinik, harinikatakamlinux
From: Vishnu Motghare <vishnum@xilinx.com>
Cadence I2C controller has bug wherein it generates invalid read transactions
after timeout in master receiver mode. This driver does not use the HW
timeout and this interrupt is disabled but the feature itself cannot be
disabled. Hence, this patch writes the maximum value (0xFF) to this register.
This is one of the workarounds to this bug and it will not avoid the issue
completely but reduces the chances of error.
Signed-off-by: Vishnu Motghare <vishnum@xilinx.com>
Signed-off-by: Harini Katakam <harinik@xilinx.com>
---
v2:
Added comments in driver.
---
drivers/i2c/busses/i2c-cadence.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index e54899e..67077c2 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -111,6 +111,8 @@
#define CDNS_I2C_DIVA_MAX 4
#define CDNS_I2C_DIVB_MAX 64
+#define CDNS_I2C_TIMEOUT_MAX 0xFF
+
#define cdns_i2c_readreg(offset) readl_relaxed(id->membase + offset)
#define cdns_i2c_writereg(val, offset) writel_relaxed(val, id->membase + offset)
@@ -858,6 +860,15 @@ static int cdns_i2c_probe(struct platform_device *pdev)
goto err_clk_dis;
}
+ /*
+ * Cadence I2C controller has a bug wherein it generates
+ * invalid read transaction after HW timeout in master receiver mode.
+ * HW timeout is not used by this driver and the interrupt is disabled.
+ * But the feature itself cannot be disabled. Hence maximum value
+ * is written to this register to reduce the chances of error.
+ */
+ cdns_i2c_writereg(CDNS_I2C_TIMEOUT_MAX, CDNS_I2C_TIME_OUT_OFFSET);
+
dev_info(&pdev->dev, "%u kHz mmio %08lx irq %d\n",
id->i2c_clk / 1000, (unsigned long)r_mem->start, id->irq);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 2/3] i2c: cadence: Set the hardware time-out register to maximum value
@ 2014-12-03 12:35 ` Harini Katakam
0 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-03 12:35 UTC (permalink / raw)
To: linux-arm-kernel
From: Vishnu Motghare <vishnum@xilinx.com>
Cadence I2C controller has bug wherein it generates invalid read transactions
after timeout in master receiver mode. This driver does not use the HW
timeout and this interrupt is disabled but the feature itself cannot be
disabled. Hence, this patch writes the maximum value (0xFF) to this register.
This is one of the workarounds to this bug and it will not avoid the issue
completely but reduces the chances of error.
Signed-off-by: Vishnu Motghare <vishnum@xilinx.com>
Signed-off-by: Harini Katakam <harinik@xilinx.com>
---
v2:
Added comments in driver.
---
drivers/i2c/busses/i2c-cadence.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index e54899e..67077c2 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -111,6 +111,8 @@
#define CDNS_I2C_DIVA_MAX 4
#define CDNS_I2C_DIVB_MAX 64
+#define CDNS_I2C_TIMEOUT_MAX 0xFF
+
#define cdns_i2c_readreg(offset) readl_relaxed(id->membase + offset)
#define cdns_i2c_writereg(val, offset) writel_relaxed(val, id->membase + offset)
@@ -858,6 +860,15 @@ static int cdns_i2c_probe(struct platform_device *pdev)
goto err_clk_dis;
}
+ /*
+ * Cadence I2C controller has a bug wherein it generates
+ * invalid read transaction after HW timeout in master receiver mode.
+ * HW timeout is not used by this driver and the interrupt is disabled.
+ * But the feature itself cannot be disabled. Hence maximum value
+ * is written to this register to reduce the chances of error.
+ */
+ cdns_i2c_writereg(CDNS_I2C_TIMEOUT_MAX, CDNS_I2C_TIME_OUT_OFFSET);
+
dev_info(&pdev->dev, "%u kHz mmio %08lx irq %d\n",
id->i2c_clk / 1000, (unsigned long)r_mem->start, id->irq);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 3/3] i2c: cadence: Check for errata condition involving master receive
2014-12-03 12:35 ` Harini Katakam
@ 2014-12-03 12:35 ` Harini Katakam
-1 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-03 12:35 UTC (permalink / raw)
To: wsa, mark.rutland
Cc: michal.simek, soren.brinkmann, linux-arm-kernel, linux-i2c,
linux-kernel, vishnum, harinik, harinikatakamlinux
Cadence I2C controller has the following bugs:
- completion indication is not given to the driver at the end of
a read/receive transfer with HOLD bit set.
- Invalid read transaction are generated on the bus when HW timeout
condition occurs with HOLD bit set.
As a result of the above, if a set of messages to be transferred with
repeated start includes any transfer following a read transfer,
completion is never indicated and timeout occurs.
Hence a check is implemented to return -EOPNOTSUPP for such sequences.
Signed-off-by: Harini Katakam <harinik@xilinx.com>
Signed-off-by: Vishnu Motghare <vishnum@xilinx.com>
---
v2:
Dont defeteature repeated start. Just check for unsupported conditions in the
driver and return error.
---
drivers/i2c/busses/i2c-cadence.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 67077c2..f5437e2 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -522,6 +522,17 @@ static int cdns_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
* processed with a repeated start.
*/
if (num > 1) {
+ /*
+ * This controller does not give completion interrupt after a
+ * master receive transfer if HOLD bit is set (repeated start),
+ * resulting in SW timeout. Hence, if a receive transfer is
+ * followed by any other transfer, an error is returned
+ * indicating that this sequence is not supported.
+ */
+ for (count = 0; count < num-1; count++) {
+ if (msgs[count].flags & I2C_M_RD)
+ return -EOPNOTSUPP;
+ }
id->bus_hold_flag = 1;
reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET);
reg |= CDNS_I2C_CR_HOLD;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 3/3] i2c: cadence: Check for errata condition involving master receive
@ 2014-12-03 12:35 ` Harini Katakam
0 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-03 12:35 UTC (permalink / raw)
To: linux-arm-kernel
Cadence I2C controller has the following bugs:
- completion indication is not given to the driver at the end of
a read/receive transfer with HOLD bit set.
- Invalid read transaction are generated on the bus when HW timeout
condition occurs with HOLD bit set.
As a result of the above, if a set of messages to be transferred with
repeated start includes any transfer following a read transfer,
completion is never indicated and timeout occurs.
Hence a check is implemented to return -EOPNOTSUPP for such sequences.
Signed-off-by: Harini Katakam <harinik@xilinx.com>
Signed-off-by: Vishnu Motghare <vishnum@xilinx.com>
---
v2:
Dont defeteature repeated start. Just check for unsupported conditions in the
driver and return error.
---
drivers/i2c/busses/i2c-cadence.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 67077c2..f5437e2 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -522,6 +522,17 @@ static int cdns_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
* processed with a repeated start.
*/
if (num > 1) {
+ /*
+ * This controller does not give completion interrupt after a
+ * master receive transfer if HOLD bit is set (repeated start),
+ * resulting in SW timeout. Hence, if a receive transfer is
+ * followed by any other transfer, an error is returned
+ * indicating that this sequence is not supported.
+ */
+ for (count = 0; count < num-1; count++) {
+ if (msgs[count].flags & I2C_M_RD)
+ return -EOPNOTSUPP;
+ }
id->bus_hold_flag = 1;
reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET);
reg |= CDNS_I2C_CR_HOLD;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/3] i2c: cadence: Set the hardware time-out register to maximum value
2014-12-03 12:35 ` Harini Katakam
@ 2014-12-04 18:30 ` Wolfram Sang
-1 siblings, 0 replies; 32+ messages in thread
From: Wolfram Sang @ 2014-12-04 18:30 UTC (permalink / raw)
To: Harini Katakam
Cc: mark.rutland, michal.simek, soren.brinkmann, linux-arm-kernel,
linux-i2c, linux-kernel, vishnum, harinikatakamlinux
[-- Attachment #1: Type: text/plain, Size: 702 bytes --]
On Wed, Dec 03, 2014 at 06:05:25PM +0530, Harini Katakam wrote:
> From: Vishnu Motghare <vishnum@xilinx.com>
>
> Cadence I2C controller has bug wherein it generates invalid read transactions
> after timeout in master receiver mode. This driver does not use the HW
> timeout and this interrupt is disabled but the feature itself cannot be
> disabled. Hence, this patch writes the maximum value (0xFF) to this register.
> This is one of the workarounds to this bug and it will not avoid the issue
> completely but reduces the chances of error.
>
> Signed-off-by: Vishnu Motghare <vishnum@xilinx.com>
> Signed-off-by: Harini Katakam <harinik@xilinx.com>
Applied to for-current, thanks!
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 2/3] i2c: cadence: Set the hardware time-out register to maximum value
@ 2014-12-04 18:30 ` Wolfram Sang
0 siblings, 0 replies; 32+ messages in thread
From: Wolfram Sang @ 2014-12-04 18:30 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Dec 03, 2014 at 06:05:25PM +0530, Harini Katakam wrote:
> From: Vishnu Motghare <vishnum@xilinx.com>
>
> Cadence I2C controller has bug wherein it generates invalid read transactions
> after timeout in master receiver mode. This driver does not use the HW
> timeout and this interrupt is disabled but the feature itself cannot be
> disabled. Hence, this patch writes the maximum value (0xFF) to this register.
> This is one of the workarounds to this bug and it will not avoid the issue
> completely but reduces the chances of error.
>
> Signed-off-by: Vishnu Motghare <vishnum@xilinx.com>
> Signed-off-by: Harini Katakam <harinik@xilinx.com>
Applied to for-current, thanks!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141204/7509448e/attachment.sig>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
@ 2014-12-04 18:32 ` Wolfram Sang
0 siblings, 0 replies; 32+ messages in thread
From: Wolfram Sang @ 2014-12-04 18:32 UTC (permalink / raw)
To: Harini Katakam
Cc: mark.rutland, michal.simek, soren.brinkmann, linux-arm-kernel,
linux-i2c, linux-kernel, vishnum, harinikatakamlinux
[-- Attachment #1: Type: text/plain, Size: 314 bytes --]
> + /*
> + * If the device is sending data If there is further
> + * data to be sent. Calculate the available space
> + * in FIFO and fill the FIFO with that many bytes.
> + */
This comment looks broken. In general, I think there should be more
comments explaining why things have to be done this way.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
@ 2014-12-04 18:32 ` Wolfram Sang
0 siblings, 0 replies; 32+ messages in thread
From: Wolfram Sang @ 2014-12-04 18:32 UTC (permalink / raw)
To: Harini Katakam
Cc: mark.rutland-5wv7dgnIgG8, michal.simek-gjFFaj9aHVfQT0dZR+AlfA,
soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
vishnum-gjFFaj9aHVfQT0dZR+AlfA,
harinikatakamlinux-Re5JQEeQqe8AvxtiuMwx3w
[-- Attachment #1: Type: text/plain, Size: 314 bytes --]
> + /*
> + * If the device is sending data If there is further
> + * data to be sent. Calculate the available space
> + * in FIFO and fill the FIFO with that many bytes.
> + */
This comment looks broken. In general, I think there should be more
comments explaining why things have to be done this way.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
@ 2014-12-04 18:32 ` Wolfram Sang
0 siblings, 0 replies; 32+ messages in thread
From: Wolfram Sang @ 2014-12-04 18:32 UTC (permalink / raw)
To: linux-arm-kernel
> + /*
> + * If the device is sending data If there is further
> + * data to be sent. Calculate the available space
> + * in FIFO and fill the FIFO with that many bytes.
> + */
This comment looks broken. In general, I think there should be more
comments explaining why things have to be done this way.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141204/f1e23a22/attachment.sig>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/3] i2c: cadence: Check for errata condition involving master receive
@ 2014-12-04 18:34 ` Wolfram Sang
0 siblings, 0 replies; 32+ messages in thread
From: Wolfram Sang @ 2014-12-04 18:34 UTC (permalink / raw)
To: Harini Katakam
Cc: mark.rutland, michal.simek, soren.brinkmann, linux-arm-kernel,
linux-i2c, linux-kernel, vishnum, harinikatakamlinux
[-- Attachment #1: Type: text/plain, Size: 556 bytes --]
> + /*
> + * This controller does not give completion interrupt after a
> + * master receive transfer if HOLD bit is set (repeated start),
> + * resulting in SW timeout. Hence, if a receive transfer is
> + * followed by any other transfer, an error is returned
> + * indicating that this sequence is not supported.
> + */
> + for (count = 0; count < num-1; count++) {
> + if (msgs[count].flags & I2C_M_RD)
> + return -EOPNOTSUPP;
> + }
Yeah, a lot better. Probably it would be good to inform the user with a
warning what went wrong?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/3] i2c: cadence: Check for errata condition involving master receive
@ 2014-12-04 18:34 ` Wolfram Sang
0 siblings, 0 replies; 32+ messages in thread
From: Wolfram Sang @ 2014-12-04 18:34 UTC (permalink / raw)
To: Harini Katakam
Cc: mark.rutland-5wv7dgnIgG8, michal.simek-gjFFaj9aHVfQT0dZR+AlfA,
soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
vishnum-gjFFaj9aHVfQT0dZR+AlfA,
harinikatakamlinux-Re5JQEeQqe8AvxtiuMwx3w
[-- Attachment #1: Type: text/plain, Size: 556 bytes --]
> + /*
> + * This controller does not give completion interrupt after a
> + * master receive transfer if HOLD bit is set (repeated start),
> + * resulting in SW timeout. Hence, if a receive transfer is
> + * followed by any other transfer, an error is returned
> + * indicating that this sequence is not supported.
> + */
> + for (count = 0; count < num-1; count++) {
> + if (msgs[count].flags & I2C_M_RD)
> + return -EOPNOTSUPP;
> + }
Yeah, a lot better. Probably it would be good to inform the user with a
warning what went wrong?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 3/3] i2c: cadence: Check for errata condition involving master receive
@ 2014-12-04 18:34 ` Wolfram Sang
0 siblings, 0 replies; 32+ messages in thread
From: Wolfram Sang @ 2014-12-04 18:34 UTC (permalink / raw)
To: linux-arm-kernel
> + /*
> + * This controller does not give completion interrupt after a
> + * master receive transfer if HOLD bit is set (repeated start),
> + * resulting in SW timeout. Hence, if a receive transfer is
> + * followed by any other transfer, an error is returned
> + * indicating that this sequence is not supported.
> + */
> + for (count = 0; count < num-1; count++) {
> + if (msgs[count].flags & I2C_M_RD)
> + return -EOPNOTSUPP;
> + }
Yeah, a lot better. Probably it would be good to inform the user with a
warning what went wrong?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141204/0accd3da/attachment.sig>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/3] i2c: cadence: Check for errata condition involving master receive
2014-12-04 18:34 ` Wolfram Sang
@ 2014-12-05 4:12 ` Harini Katakam
-1 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-05 4:12 UTC (permalink / raw)
To: Wolfram Sang
Cc: Mark Rutland, Michal Simek, Sören Brinkmann,
linux-arm-kernel, linux-i2c, linux-kernel, vishnum
Hi,
On Fri, Dec 5, 2014 at 12:04 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> + /*
>> + * This controller does not give completion interrupt after a
>> + * master receive transfer if HOLD bit is set (repeated start),
>> + * resulting in SW timeout. Hence, if a receive transfer is
>> + * followed by any other transfer, an error is returned
>> + * indicating that this sequence is not supported.
>> + */
>> + for (count = 0; count < num-1; count++) {
>> + if (msgs[count].flags & I2C_M_RD)
>> + return -EOPNOTSUPP;
>> + }
>
> Yeah, a lot better. Probably it would be good to inform the user with a
> warning what went wrong?
>
Sure. I'll add that.
Regards,
Harini
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 3/3] i2c: cadence: Check for errata condition involving master receive
@ 2014-12-05 4:12 ` Harini Katakam
0 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-05 4:12 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Fri, Dec 5, 2014 at 12:04 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> + /*
>> + * This controller does not give completion interrupt after a
>> + * master receive transfer if HOLD bit is set (repeated start),
>> + * resulting in SW timeout. Hence, if a receive transfer is
>> + * followed by any other transfer, an error is returned
>> + * indicating that this sequence is not supported.
>> + */
>> + for (count = 0; count < num-1; count++) {
>> + if (msgs[count].flags & I2C_M_RD)
>> + return -EOPNOTSUPP;
>> + }
>
> Yeah, a lot better. Probably it would be good to inform the user with a
> warning what went wrong?
>
Sure. I'll add that.
Regards,
Harini
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
2014-12-04 18:32 ` Wolfram Sang
(?)
@ 2014-12-05 4:20 ` Harini Katakam
-1 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-05 4:20 UTC (permalink / raw)
To: Wolfram Sang
Cc: Mark Rutland, Michal Simek, Sören Brinkmann,
linux-arm-kernel, linux-i2c, linux-kernel, vishnum
Hi,
On Fri, Dec 5, 2014 at 12:02 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> + /*
>> + * If the device is sending data If there is further
>> + * data to be sent. Calculate the available space
>> + * in FIFO and fill the FIFO with that many bytes.
>> + */
>
> This comment looks broken. In general, I think there should be more
> comments explaining why things have to be done this way.
>
There's some grammatical errors here. Let me correct it and add more
comments.
Regards,
Harini
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
@ 2014-12-05 4:20 ` Harini Katakam
0 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-05 4:20 UTC (permalink / raw)
To: Wolfram Sang
Cc: Mark Rutland, Michal Simek, Sören Brinkmann,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
vishnum-gjFFaj9aHVfQT0dZR+AlfA
Hi,
On Fri, Dec 5, 2014 at 12:02 AM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>
>> + /*
>> + * If the device is sending data If there is further
>> + * data to be sent. Calculate the available space
>> + * in FIFO and fill the FIFO with that many bytes.
>> + */
>
> This comment looks broken. In general, I think there should be more
> comments explaining why things have to be done this way.
>
There's some grammatical errors here. Let me correct it and add more
comments.
Regards,
Harini
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
@ 2014-12-05 4:20 ` Harini Katakam
0 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-05 4:20 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Fri, Dec 5, 2014 at 12:02 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> + /*
>> + * If the device is sending data If there is further
>> + * data to be sent. Calculate the available space
>> + * in FIFO and fill the FIFO with that many bytes.
>> + */
>
> This comment looks broken. In general, I think there should be more
> comments explaining why things have to be done this way.
>
There's some grammatical errors here. Let me correct it and add more
comments.
Regards,
Harini
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
@ 2014-12-05 5:41 ` rajeev kumar
0 siblings, 0 replies; 32+ messages in thread
From: rajeev kumar @ 2014-12-05 5:41 UTC (permalink / raw)
To: Harini Katakam
Cc: wsa, mark.rutland, michal.simek, soren.brinkmann,
linux-arm-kernel, linux-i2c, linux-kernel, vishnum,
harinikatakamlinux
On Wed, Dec 3, 2014 at 6:05 PM, Harini Katakam <harinik@xilinx.com> wrote:
> The I2C controller sends a NACK to the slave when transfer size register
> reaches zero, irrespective of the hold bit. So, in order to handle transfers
> greater than 252 bytes, the transfer size register has to be maintained at a
> value >= 1. This patch implements the same.
Why 252 Bytes ? Is it word allign or what ?
> The interrupt status is cleared at the beginning of the isr instead of
> the end, to avoid missing any interrupts - this is in sync with the new
> transfer handling.
>
No need to write this, actually this is the correct way of handling interrupt.
> Signed-off-by: Harini Katakam <harinik@xilinx.com>
> ---
>
> v2:
> No changes
>
> ---
> drivers/i2c/busses/i2c-cadence.c | 156 ++++++++++++++++++++------------------
> 1 file changed, 81 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
> index 63f3f03..e54899e 100644
> --- a/drivers/i2c/busses/i2c-cadence.c
> +++ b/drivers/i2c/busses/i2c-cadence.c
> @@ -126,6 +126,7 @@
> * @suspended: Flag holding the device's PM status
> * @send_count: Number of bytes still expected to send
> * @recv_count: Number of bytes still expected to receive
> + * @curr_recv_count: Number of bytes to be received in current transfer
Please do the alignment properly
> * @irq: IRQ number
> * @input_clk: Input clock to I2C controller
> * @i2c_clk: Maximum I2C clock speed
> @@ -144,6 +145,7 @@ struct cdns_i2c {
> u8 suspended;
> unsigned int send_count;
> unsigned int recv_count;
> + unsigned int curr_recv_count;
same here..
> int irq;
> unsigned long input_clk;
> unsigned int i2c_clk;
> @@ -180,14 +182,15 @@ static void cdns_i2c_clear_bus_hold(struct cdns_i2c *id)
> */
> static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
> {
> - unsigned int isr_status, avail_bytes;
> - unsigned int bytes_to_recv, bytes_to_send;
> + unsigned int isr_status, avail_bytes, updatetx;
> + unsigned int bytes_to_send;
why you are mixing tab and space..
> struct cdns_i2c *id = ptr;
> /* Signal completion only after everything is updated */
> int done_flag = 0;
> irqreturn_t status = IRQ_NONE;
>
> isr_status = cdns_i2c_readreg(CDNS_I2C_ISR_OFFSET);
> + cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
>
> /* Handling nack and arbitration lost interrupt */
> if (isr_status & (CDNS_I2C_IXR_NACK | CDNS_I2C_IXR_ARB_LOST)) {
> @@ -195,89 +198,91 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
> status = IRQ_HANDLED;
> }
>
> - /* Handling Data interrupt */
> - if ((isr_status & CDNS_I2C_IXR_DATA) &&
> - (id->recv_count >= CDNS_I2C_DATA_INTR_DEPTH)) {
> - /* Always read data interrupt threshold bytes */
> - bytes_to_recv = CDNS_I2C_DATA_INTR_DEPTH;
> - id->recv_count -= CDNS_I2C_DATA_INTR_DEPTH;
> - avail_bytes = cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
> -
> - /*
> - * if the tranfer size register value is zero, then
> - * check for the remaining bytes and update the
> - * transfer size register.
> - */
> - if (!avail_bytes) {
> - if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
> - cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
> - CDNS_I2C_XFER_SIZE_OFFSET);
> - else
> - cdns_i2c_writereg(id->recv_count,
> - CDNS_I2C_XFER_SIZE_OFFSET);
> - }
> + updatetx = 0;
> + if (id->recv_count > id->curr_recv_count)
> + updatetx = 1;
> +
> + /* When receiving, handle data and transfer complete interrupts */
Breaking statement
> + if (id->p_recv_buf &&
> + ((isr_status & CDNS_I2C_IXR_COMP) ||
> + (isr_status & CDNS_I2C_IXR_DATA))) {
> + while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
> + CDNS_I2C_SR_RXDV) {
> + if ((id->recv_count < CDNS_I2C_FIFO_DEPTH) &&
> + !id->bus_hold_flag)
> + cdns_i2c_clear_bus_hold(id);
make it simple. you can use extra variables also.
>
> - /* Process the data received */
> - while (bytes_to_recv--)
> *(id->p_recv_buf)++ =
> cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
> + id->recv_count--;
> + id->curr_recv_count--;
>
> - if (!id->bus_hold_flag &&
> - (id->recv_count <= CDNS_I2C_FIFO_DEPTH))
> - cdns_i2c_clear_bus_hold(id);
> + if (updatetx &&
> + (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1))
> + break;
> + }
>
> - status = IRQ_HANDLED;
> - }
> + if (updatetx &&
> + (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1)) {
> + /* wait while fifo is full */
Not convinced with the implementation . you are waiting in ISR.. You
can move this part in transfer function,
> + while (cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET) !=
> + (id->curr_recv_count - CDNS_I2C_FIFO_DEPTH))
> + ;
>
> - /* Handling Transfer Complete interrupt */
> - if (isr_status & CDNS_I2C_IXR_COMP) {
> - if (!id->p_recv_buf) {
> - /*
> - * If the device is sending data If there is further
> - * data to be sent. Calculate the available space
> - * in FIFO and fill the FIFO with that many bytes.
> - */
> - if (id->send_count) {
> - avail_bytes = CDNS_I2C_FIFO_DEPTH -
> - cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
> - if (id->send_count > avail_bytes)
> - bytes_to_send = avail_bytes;
> - else
> - bytes_to_send = id->send_count;
> -
> - while (bytes_to_send--) {
> - cdns_i2c_writereg(
> - (*(id->p_send_buf)++),
> - CDNS_I2C_DATA_OFFSET);
> - id->send_count--;
> - }
> + if (((int)(id->recv_count) - CDNS_I2C_FIFO_DEPTH) >
> + CDNS_I2C_TRANSFER_SIZE) {
> + cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
> + CDNS_I2C_XFER_SIZE_OFFSET);
> + id->curr_recv_count = CDNS_I2C_TRANSFER_SIZE +
> + CDNS_I2C_FIFO_DEPTH;
> } else {
> - /*
> - * Signal the completion of transaction and
> - * clear the hold bus bit if there are no
> - * further messages to be processed.
> - */
> - done_flag = 1;
> + cdns_i2c_writereg(id->recv_count -
> + CDNS_I2C_FIFO_DEPTH,
> + CDNS_I2C_XFER_SIZE_OFFSET);
> + id->curr_recv_count = id->recv_count;
> }
> - if (!id->send_count && !id->bus_hold_flag)
> - cdns_i2c_clear_bus_hold(id);
> - } else {
> + }
> +
> + if ((isr_status & CDNS_I2C_IXR_COMP) && !id->recv_count) {
> if (!id->bus_hold_flag)
> cdns_i2c_clear_bus_hold(id);
> + done_flag = 1;
> + }
> +
> + status = IRQ_HANDLED;
> + }
> +
> + /* When sending, handle transfer complete interrupt */
> + if ((isr_status & CDNS_I2C_IXR_COMP) && !id->p_recv_buf) {
> + /*
> + * If the device is sending data If there is further
> + * data to be sent. Calculate the available space
> + * in FIFO and fill the FIFO with that many bytes.
> + */
> + if (id->send_count) {
> + avail_bytes = CDNS_I2C_FIFO_DEPTH -
> + cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
> + if (id->send_count > avail_bytes)
> + bytes_to_send = avail_bytes;
> + else
> + bytes_to_send = id->send_count;
> +
> + while (bytes_to_send--) {
> + cdns_i2c_writereg(
> + (*(id->p_send_buf)++),
> + CDNS_I2C_DATA_OFFSET);
> + id->send_count--;
> + }
> + } else {
> /*
> - * If the device is receiving data, then signal
> - * the completion of transaction and read the data
> - * present in the FIFO. Signal the completion of
> - * transaction.
> + * Signal the completion of transaction and
> + * clear the hold bus bit if there are no
> + * further messages to be processed.
> */
> - while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
> - CDNS_I2C_SR_RXDV) {
> - *(id->p_recv_buf)++ =
> - cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
> - id->recv_count--;
> - }
> done_flag = 1;
> }
> + if (!id->send_count && !id->bus_hold_flag)
> + cdns_i2c_clear_bus_hold(id);
>
> status = IRQ_HANDLED;
> }
> @@ -287,8 +292,6 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
> if (id->err_status)
> status = IRQ_HANDLED;
>
> - cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
> -
> if (done_flag)
> complete(&id->xfer_done);
>
> @@ -314,6 +317,8 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
> if (id->p_msg->flags & I2C_M_RECV_LEN)
> id->recv_count = I2C_SMBUS_BLOCK_MAX + 1;
>
> + id->curr_recv_count = id->recv_count;
> +
> /*
> * Check for the message size against FIFO depth and set the
> * 'hold bus' bit if it is greater than FIFO depth.
> @@ -333,10 +338,11 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
> * receive if it is less than transfer size and transfer size if
> * it is more. Enable the interrupts.
> */
> - if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
> + if (id->recv_count > CDNS_I2C_TRANSFER_SIZE) {
> cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
> CDNS_I2C_XFER_SIZE_OFFSET);
> - else
> + id->curr_recv_count = CDNS_I2C_TRANSFER_SIZE;
> + } else
> cdns_i2c_writereg(id->recv_count, CDNS_I2C_XFER_SIZE_OFFSET);
> /* Clear the bus hold flag if bytes to receive is less than FIFO size */
> if (!id->bus_hold_flag &&
> --
> 1.7.9.5
Overall I think it is required to re-write isr.
~Rajeev
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
@ 2014-12-05 5:41 ` rajeev kumar
0 siblings, 0 replies; 32+ messages in thread
From: rajeev kumar @ 2014-12-05 5:41 UTC (permalink / raw)
To: Harini Katakam
Cc: wsa-z923LK4zBo2bacvFa/9K2g, mark.rutland-5wv7dgnIgG8,
michal.simek-gjFFaj9aHVfQT0dZR+AlfA,
soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
vishnum-gjFFaj9aHVfQT0dZR+AlfA,
harinikatakamlinux-Re5JQEeQqe8AvxtiuMwx3w
On Wed, Dec 3, 2014 at 6:05 PM, Harini Katakam <harinik-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> The I2C controller sends a NACK to the slave when transfer size register
> reaches zero, irrespective of the hold bit. So, in order to handle transfers
> greater than 252 bytes, the transfer size register has to be maintained at a
> value >= 1. This patch implements the same.
Why 252 Bytes ? Is it word allign or what ?
> The interrupt status is cleared at the beginning of the isr instead of
> the end, to avoid missing any interrupts - this is in sync with the new
> transfer handling.
>
No need to write this, actually this is the correct way of handling interrupt.
> Signed-off-by: Harini Katakam <harinik-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
>
> v2:
> No changes
>
> ---
> drivers/i2c/busses/i2c-cadence.c | 156 ++++++++++++++++++++------------------
> 1 file changed, 81 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
> index 63f3f03..e54899e 100644
> --- a/drivers/i2c/busses/i2c-cadence.c
> +++ b/drivers/i2c/busses/i2c-cadence.c
> @@ -126,6 +126,7 @@
> * @suspended: Flag holding the device's PM status
> * @send_count: Number of bytes still expected to send
> * @recv_count: Number of bytes still expected to receive
> + * @curr_recv_count: Number of bytes to be received in current transfer
Please do the alignment properly
> * @irq: IRQ number
> * @input_clk: Input clock to I2C controller
> * @i2c_clk: Maximum I2C clock speed
> @@ -144,6 +145,7 @@ struct cdns_i2c {
> u8 suspended;
> unsigned int send_count;
> unsigned int recv_count;
> + unsigned int curr_recv_count;
same here..
> int irq;
> unsigned long input_clk;
> unsigned int i2c_clk;
> @@ -180,14 +182,15 @@ static void cdns_i2c_clear_bus_hold(struct cdns_i2c *id)
> */
> static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
> {
> - unsigned int isr_status, avail_bytes;
> - unsigned int bytes_to_recv, bytes_to_send;
> + unsigned int isr_status, avail_bytes, updatetx;
> + unsigned int bytes_to_send;
why you are mixing tab and space..
> struct cdns_i2c *id = ptr;
> /* Signal completion only after everything is updated */
> int done_flag = 0;
> irqreturn_t status = IRQ_NONE;
>
> isr_status = cdns_i2c_readreg(CDNS_I2C_ISR_OFFSET);
> + cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
>
> /* Handling nack and arbitration lost interrupt */
> if (isr_status & (CDNS_I2C_IXR_NACK | CDNS_I2C_IXR_ARB_LOST)) {
> @@ -195,89 +198,91 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
> status = IRQ_HANDLED;
> }
>
> - /* Handling Data interrupt */
> - if ((isr_status & CDNS_I2C_IXR_DATA) &&
> - (id->recv_count >= CDNS_I2C_DATA_INTR_DEPTH)) {
> - /* Always read data interrupt threshold bytes */
> - bytes_to_recv = CDNS_I2C_DATA_INTR_DEPTH;
> - id->recv_count -= CDNS_I2C_DATA_INTR_DEPTH;
> - avail_bytes = cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
> -
> - /*
> - * if the tranfer size register value is zero, then
> - * check for the remaining bytes and update the
> - * transfer size register.
> - */
> - if (!avail_bytes) {
> - if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
> - cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
> - CDNS_I2C_XFER_SIZE_OFFSET);
> - else
> - cdns_i2c_writereg(id->recv_count,
> - CDNS_I2C_XFER_SIZE_OFFSET);
> - }
> + updatetx = 0;
> + if (id->recv_count > id->curr_recv_count)
> + updatetx = 1;
> +
> + /* When receiving, handle data and transfer complete interrupts */
Breaking statement
> + if (id->p_recv_buf &&
> + ((isr_status & CDNS_I2C_IXR_COMP) ||
> + (isr_status & CDNS_I2C_IXR_DATA))) {
> + while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
> + CDNS_I2C_SR_RXDV) {
> + if ((id->recv_count < CDNS_I2C_FIFO_DEPTH) &&
> + !id->bus_hold_flag)
> + cdns_i2c_clear_bus_hold(id);
make it simple. you can use extra variables also.
>
> - /* Process the data received */
> - while (bytes_to_recv--)
> *(id->p_recv_buf)++ =
> cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
> + id->recv_count--;
> + id->curr_recv_count--;
>
> - if (!id->bus_hold_flag &&
> - (id->recv_count <= CDNS_I2C_FIFO_DEPTH))
> - cdns_i2c_clear_bus_hold(id);
> + if (updatetx &&
> + (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1))
> + break;
> + }
>
> - status = IRQ_HANDLED;
> - }
> + if (updatetx &&
> + (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1)) {
> + /* wait while fifo is full */
Not convinced with the implementation . you are waiting in ISR.. You
can move this part in transfer function,
> + while (cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET) !=
> + (id->curr_recv_count - CDNS_I2C_FIFO_DEPTH))
> + ;
>
> - /* Handling Transfer Complete interrupt */
> - if (isr_status & CDNS_I2C_IXR_COMP) {
> - if (!id->p_recv_buf) {
> - /*
> - * If the device is sending data If there is further
> - * data to be sent. Calculate the available space
> - * in FIFO and fill the FIFO with that many bytes.
> - */
> - if (id->send_count) {
> - avail_bytes = CDNS_I2C_FIFO_DEPTH -
> - cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
> - if (id->send_count > avail_bytes)
> - bytes_to_send = avail_bytes;
> - else
> - bytes_to_send = id->send_count;
> -
> - while (bytes_to_send--) {
> - cdns_i2c_writereg(
> - (*(id->p_send_buf)++),
> - CDNS_I2C_DATA_OFFSET);
> - id->send_count--;
> - }
> + if (((int)(id->recv_count) - CDNS_I2C_FIFO_DEPTH) >
> + CDNS_I2C_TRANSFER_SIZE) {
> + cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
> + CDNS_I2C_XFER_SIZE_OFFSET);
> + id->curr_recv_count = CDNS_I2C_TRANSFER_SIZE +
> + CDNS_I2C_FIFO_DEPTH;
> } else {
> - /*
> - * Signal the completion of transaction and
> - * clear the hold bus bit if there are no
> - * further messages to be processed.
> - */
> - done_flag = 1;
> + cdns_i2c_writereg(id->recv_count -
> + CDNS_I2C_FIFO_DEPTH,
> + CDNS_I2C_XFER_SIZE_OFFSET);
> + id->curr_recv_count = id->recv_count;
> }
> - if (!id->send_count && !id->bus_hold_flag)
> - cdns_i2c_clear_bus_hold(id);
> - } else {
> + }
> +
> + if ((isr_status & CDNS_I2C_IXR_COMP) && !id->recv_count) {
> if (!id->bus_hold_flag)
> cdns_i2c_clear_bus_hold(id);
> + done_flag = 1;
> + }
> +
> + status = IRQ_HANDLED;
> + }
> +
> + /* When sending, handle transfer complete interrupt */
> + if ((isr_status & CDNS_I2C_IXR_COMP) && !id->p_recv_buf) {
> + /*
> + * If the device is sending data If there is further
> + * data to be sent. Calculate the available space
> + * in FIFO and fill the FIFO with that many bytes.
> + */
> + if (id->send_count) {
> + avail_bytes = CDNS_I2C_FIFO_DEPTH -
> + cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
> + if (id->send_count > avail_bytes)
> + bytes_to_send = avail_bytes;
> + else
> + bytes_to_send = id->send_count;
> +
> + while (bytes_to_send--) {
> + cdns_i2c_writereg(
> + (*(id->p_send_buf)++),
> + CDNS_I2C_DATA_OFFSET);
> + id->send_count--;
> + }
> + } else {
> /*
> - * If the device is receiving data, then signal
> - * the completion of transaction and read the data
> - * present in the FIFO. Signal the completion of
> - * transaction.
> + * Signal the completion of transaction and
> + * clear the hold bus bit if there are no
> + * further messages to be processed.
> */
> - while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
> - CDNS_I2C_SR_RXDV) {
> - *(id->p_recv_buf)++ =
> - cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
> - id->recv_count--;
> - }
> done_flag = 1;
> }
> + if (!id->send_count && !id->bus_hold_flag)
> + cdns_i2c_clear_bus_hold(id);
>
> status = IRQ_HANDLED;
> }
> @@ -287,8 +292,6 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
> if (id->err_status)
> status = IRQ_HANDLED;
>
> - cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
> -
> if (done_flag)
> complete(&id->xfer_done);
>
> @@ -314,6 +317,8 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
> if (id->p_msg->flags & I2C_M_RECV_LEN)
> id->recv_count = I2C_SMBUS_BLOCK_MAX + 1;
>
> + id->curr_recv_count = id->recv_count;
> +
> /*
> * Check for the message size against FIFO depth and set the
> * 'hold bus' bit if it is greater than FIFO depth.
> @@ -333,10 +338,11 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
> * receive if it is less than transfer size and transfer size if
> * it is more. Enable the interrupts.
> */
> - if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
> + if (id->recv_count > CDNS_I2C_TRANSFER_SIZE) {
> cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
> CDNS_I2C_XFER_SIZE_OFFSET);
> - else
> + id->curr_recv_count = CDNS_I2C_TRANSFER_SIZE;
> + } else
> cdns_i2c_writereg(id->recv_count, CDNS_I2C_XFER_SIZE_OFFSET);
> /* Clear the bus hold flag if bytes to receive is less than FIFO size */
> if (!id->bus_hold_flag &&
> --
> 1.7.9.5
Overall I think it is required to re-write isr.
~Rajeev
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
@ 2014-12-05 5:41 ` rajeev kumar
0 siblings, 0 replies; 32+ messages in thread
From: rajeev kumar @ 2014-12-05 5:41 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Dec 3, 2014 at 6:05 PM, Harini Katakam <harinik@xilinx.com> wrote:
> The I2C controller sends a NACK to the slave when transfer size register
> reaches zero, irrespective of the hold bit. So, in order to handle transfers
> greater than 252 bytes, the transfer size register has to be maintained at a
> value >= 1. This patch implements the same.
Why 252 Bytes ? Is it word allign or what ?
> The interrupt status is cleared at the beginning of the isr instead of
> the end, to avoid missing any interrupts - this is in sync with the new
> transfer handling.
>
No need to write this, actually this is the correct way of handling interrupt.
> Signed-off-by: Harini Katakam <harinik@xilinx.com>
> ---
>
> v2:
> No changes
>
> ---
> drivers/i2c/busses/i2c-cadence.c | 156 ++++++++++++++++++++------------------
> 1 file changed, 81 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
> index 63f3f03..e54899e 100644
> --- a/drivers/i2c/busses/i2c-cadence.c
> +++ b/drivers/i2c/busses/i2c-cadence.c
> @@ -126,6 +126,7 @@
> * @suspended: Flag holding the device's PM status
> * @send_count: Number of bytes still expected to send
> * @recv_count: Number of bytes still expected to receive
> + * @curr_recv_count: Number of bytes to be received in current transfer
Please do the alignment properly
> * @irq: IRQ number
> * @input_clk: Input clock to I2C controller
> * @i2c_clk: Maximum I2C clock speed
> @@ -144,6 +145,7 @@ struct cdns_i2c {
> u8 suspended;
> unsigned int send_count;
> unsigned int recv_count;
> + unsigned int curr_recv_count;
same here..
> int irq;
> unsigned long input_clk;
> unsigned int i2c_clk;
> @@ -180,14 +182,15 @@ static void cdns_i2c_clear_bus_hold(struct cdns_i2c *id)
> */
> static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
> {
> - unsigned int isr_status, avail_bytes;
> - unsigned int bytes_to_recv, bytes_to_send;
> + unsigned int isr_status, avail_bytes, updatetx;
> + unsigned int bytes_to_send;
why you are mixing tab and space..
> struct cdns_i2c *id = ptr;
> /* Signal completion only after everything is updated */
> int done_flag = 0;
> irqreturn_t status = IRQ_NONE;
>
> isr_status = cdns_i2c_readreg(CDNS_I2C_ISR_OFFSET);
> + cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
>
> /* Handling nack and arbitration lost interrupt */
> if (isr_status & (CDNS_I2C_IXR_NACK | CDNS_I2C_IXR_ARB_LOST)) {
> @@ -195,89 +198,91 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
> status = IRQ_HANDLED;
> }
>
> - /* Handling Data interrupt */
> - if ((isr_status & CDNS_I2C_IXR_DATA) &&
> - (id->recv_count >= CDNS_I2C_DATA_INTR_DEPTH)) {
> - /* Always read data interrupt threshold bytes */
> - bytes_to_recv = CDNS_I2C_DATA_INTR_DEPTH;
> - id->recv_count -= CDNS_I2C_DATA_INTR_DEPTH;
> - avail_bytes = cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
> -
> - /*
> - * if the tranfer size register value is zero, then
> - * check for the remaining bytes and update the
> - * transfer size register.
> - */
> - if (!avail_bytes) {
> - if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
> - cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
> - CDNS_I2C_XFER_SIZE_OFFSET);
> - else
> - cdns_i2c_writereg(id->recv_count,
> - CDNS_I2C_XFER_SIZE_OFFSET);
> - }
> + updatetx = 0;
> + if (id->recv_count > id->curr_recv_count)
> + updatetx = 1;
> +
> + /* When receiving, handle data and transfer complete interrupts */
Breaking statement
> + if (id->p_recv_buf &&
> + ((isr_status & CDNS_I2C_IXR_COMP) ||
> + (isr_status & CDNS_I2C_IXR_DATA))) {
> + while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
> + CDNS_I2C_SR_RXDV) {
> + if ((id->recv_count < CDNS_I2C_FIFO_DEPTH) &&
> + !id->bus_hold_flag)
> + cdns_i2c_clear_bus_hold(id);
make it simple. you can use extra variables also.
>
> - /* Process the data received */
> - while (bytes_to_recv--)
> *(id->p_recv_buf)++ =
> cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
> + id->recv_count--;
> + id->curr_recv_count--;
>
> - if (!id->bus_hold_flag &&
> - (id->recv_count <= CDNS_I2C_FIFO_DEPTH))
> - cdns_i2c_clear_bus_hold(id);
> + if (updatetx &&
> + (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1))
> + break;
> + }
>
> - status = IRQ_HANDLED;
> - }
> + if (updatetx &&
> + (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1)) {
> + /* wait while fifo is full */
Not convinced with the implementation . you are waiting in ISR.. You
can move this part in transfer function,
> + while (cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET) !=
> + (id->curr_recv_count - CDNS_I2C_FIFO_DEPTH))
> + ;
>
> - /* Handling Transfer Complete interrupt */
> - if (isr_status & CDNS_I2C_IXR_COMP) {
> - if (!id->p_recv_buf) {
> - /*
> - * If the device is sending data If there is further
> - * data to be sent. Calculate the available space
> - * in FIFO and fill the FIFO with that many bytes.
> - */
> - if (id->send_count) {
> - avail_bytes = CDNS_I2C_FIFO_DEPTH -
> - cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
> - if (id->send_count > avail_bytes)
> - bytes_to_send = avail_bytes;
> - else
> - bytes_to_send = id->send_count;
> -
> - while (bytes_to_send--) {
> - cdns_i2c_writereg(
> - (*(id->p_send_buf)++),
> - CDNS_I2C_DATA_OFFSET);
> - id->send_count--;
> - }
> + if (((int)(id->recv_count) - CDNS_I2C_FIFO_DEPTH) >
> + CDNS_I2C_TRANSFER_SIZE) {
> + cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
> + CDNS_I2C_XFER_SIZE_OFFSET);
> + id->curr_recv_count = CDNS_I2C_TRANSFER_SIZE +
> + CDNS_I2C_FIFO_DEPTH;
> } else {
> - /*
> - * Signal the completion of transaction and
> - * clear the hold bus bit if there are no
> - * further messages to be processed.
> - */
> - done_flag = 1;
> + cdns_i2c_writereg(id->recv_count -
> + CDNS_I2C_FIFO_DEPTH,
> + CDNS_I2C_XFER_SIZE_OFFSET);
> + id->curr_recv_count = id->recv_count;
> }
> - if (!id->send_count && !id->bus_hold_flag)
> - cdns_i2c_clear_bus_hold(id);
> - } else {
> + }
> +
> + if ((isr_status & CDNS_I2C_IXR_COMP) && !id->recv_count) {
> if (!id->bus_hold_flag)
> cdns_i2c_clear_bus_hold(id);
> + done_flag = 1;
> + }
> +
> + status = IRQ_HANDLED;
> + }
> +
> + /* When sending, handle transfer complete interrupt */
> + if ((isr_status & CDNS_I2C_IXR_COMP) && !id->p_recv_buf) {
> + /*
> + * If the device is sending data If there is further
> + * data to be sent. Calculate the available space
> + * in FIFO and fill the FIFO with that many bytes.
> + */
> + if (id->send_count) {
> + avail_bytes = CDNS_I2C_FIFO_DEPTH -
> + cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
> + if (id->send_count > avail_bytes)
> + bytes_to_send = avail_bytes;
> + else
> + bytes_to_send = id->send_count;
> +
> + while (bytes_to_send--) {
> + cdns_i2c_writereg(
> + (*(id->p_send_buf)++),
> + CDNS_I2C_DATA_OFFSET);
> + id->send_count--;
> + }
> + } else {
> /*
> - * If the device is receiving data, then signal
> - * the completion of transaction and read the data
> - * present in the FIFO. Signal the completion of
> - * transaction.
> + * Signal the completion of transaction and
> + * clear the hold bus bit if there are no
> + * further messages to be processed.
> */
> - while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
> - CDNS_I2C_SR_RXDV) {
> - *(id->p_recv_buf)++ =
> - cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
> - id->recv_count--;
> - }
> done_flag = 1;
> }
> + if (!id->send_count && !id->bus_hold_flag)
> + cdns_i2c_clear_bus_hold(id);
>
> status = IRQ_HANDLED;
> }
> @@ -287,8 +292,6 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
> if (id->err_status)
> status = IRQ_HANDLED;
>
> - cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
> -
> if (done_flag)
> complete(&id->xfer_done);
>
> @@ -314,6 +317,8 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
> if (id->p_msg->flags & I2C_M_RECV_LEN)
> id->recv_count = I2C_SMBUS_BLOCK_MAX + 1;
>
> + id->curr_recv_count = id->recv_count;
> +
> /*
> * Check for the message size against FIFO depth and set the
> * 'hold bus' bit if it is greater than FIFO depth.
> @@ -333,10 +338,11 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
> * receive if it is less than transfer size and transfer size if
> * it is more. Enable the interrupts.
> */
> - if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
> + if (id->recv_count > CDNS_I2C_TRANSFER_SIZE) {
> cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
> CDNS_I2C_XFER_SIZE_OFFSET);
> - else
> + id->curr_recv_count = CDNS_I2C_TRANSFER_SIZE;
> + } else
> cdns_i2c_writereg(id->recv_count, CDNS_I2C_XFER_SIZE_OFFSET);
> /* Clear the bus hold flag if bytes to receive is less than FIFO size */
> if (!id->bus_hold_flag &&
> --
> 1.7.9.5
Overall I think it is required to re-write isr.
~Rajeev
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
2014-12-05 5:41 ` rajeev kumar
@ 2014-12-05 5:57 ` Harini Katakam
-1 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-05 5:57 UTC (permalink / raw)
To: rajeev kumar
Cc: wsa, Mark Rutland, Michal Simek, Sören Brinkmann,
linux-arm-kernel, linux-i2c, linux-kernel, vishnum
Hi,
On Fri, Dec 5, 2014 at 11:11 AM, rajeev kumar
<rajeevkumar.linux@gmail.com> wrote:
> On Wed, Dec 3, 2014 at 6:05 PM, Harini Katakam <harinik@xilinx.com> wrote:
>> The I2C controller sends a NACK to the slave when transfer size register
>> reaches zero, irrespective of the hold bit. So, in order to handle transfers
>> greater than 252 bytes, the transfer size register has to be maintained at a
>> value >= 1. This patch implements the same.
>
> Why 252 Bytes ? Is it word allign or what ?
>
It is the maximum transfer size that can be written that is a multiple of
the data interrupt (this occurs when the fifo has 14 bytes).
I will include an explanation in driver as well.
Regards,
Harini
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
@ 2014-12-05 5:57 ` Harini Katakam
0 siblings, 0 replies; 32+ messages in thread
From: Harini Katakam @ 2014-12-05 5:57 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Fri, Dec 5, 2014 at 11:11 AM, rajeev kumar
<rajeevkumar.linux@gmail.com> wrote:
> On Wed, Dec 3, 2014 at 6:05 PM, Harini Katakam <harinik@xilinx.com> wrote:
>> The I2C controller sends a NACK to the slave when transfer size register
>> reaches zero, irrespective of the hold bit. So, in order to handle transfers
>> greater than 252 bytes, the transfer size register has to be maintained at a
>> value >= 1. This patch implements the same.
>
> Why 252 Bytes ? Is it word allign or what ?
>
It is the maximum transfer size that can be written that is a multiple of
the data interrupt (this occurs when the fifo has 14 bytes).
I will include an explanation in driver as well.
Regards,
Harini
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
2014-12-05 5:41 ` rajeev kumar
(?)
@ 2014-12-05 8:15 ` Michal Simek
-1 siblings, 0 replies; 32+ messages in thread
From: Michal Simek @ 2014-12-05 8:15 UTC (permalink / raw)
To: rajeev kumar, Harini Katakam
Cc: wsa, mark.rutland, michal.simek, soren.brinkmann,
linux-arm-kernel, linux-i2c, linux-kernel, vishnum,
harinikatakamlinux
On 12/05/2014 06:41 AM, rajeev kumar wrote:
> On Wed, Dec 3, 2014 at 6:05 PM, Harini Katakam <harinik@xilinx.com> wrote:
>> The I2C controller sends a NACK to the slave when transfer size register
>> reaches zero, irrespective of the hold bit. So, in order to handle transfers
>> greater than 252 bytes, the transfer size register has to be maintained at a
>> value >= 1. This patch implements the same.
>
> Why 252 Bytes ? Is it word allign or what ?
>
>> The interrupt status is cleared at the beginning of the isr instead of
>> the end, to avoid missing any interrupts - this is in sync with the new
>> transfer handling.
>>
>
> No need to write this, actually this is the correct way of handling interrupt.
>
>> Signed-off-by: Harini Katakam <harinik@xilinx.com>
>> ---
>>
>> v2:
>> No changes
>>
>> ---
>> drivers/i2c/busses/i2c-cadence.c | 156 ++++++++++++++++++++------------------
>> 1 file changed, 81 insertions(+), 75 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
>> index 63f3f03..e54899e 100644
>> --- a/drivers/i2c/busses/i2c-cadence.c
>> +++ b/drivers/i2c/busses/i2c-cadence.c
>> @@ -126,6 +126,7 @@
>> * @suspended: Flag holding the device's PM status
>> * @send_count: Number of bytes still expected to send
>> * @recv_count: Number of bytes still expected to receive
>> + * @curr_recv_count: Number of bytes to be received in current transfer
>
> Please do the alignment properly
Alignments are correct when you apply this patch.
Please let us know if you see any problem.
Thanks,
Michal
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
@ 2014-12-05 8:15 ` Michal Simek
0 siblings, 0 replies; 32+ messages in thread
From: Michal Simek @ 2014-12-05 8:15 UTC (permalink / raw)
To: rajeev kumar, Harini Katakam
Cc: wsa, mark.rutland, michal.simek, soren.brinkmann,
linux-arm-kernel, linux-i2c, linux-kernel, vishnum,
harinikatakamlinux
On 12/05/2014 06:41 AM, rajeev kumar wrote:
> On Wed, Dec 3, 2014 at 6:05 PM, Harini Katakam <harinik@xilinx.com> wrote:
>> The I2C controller sends a NACK to the slave when transfer size register
>> reaches zero, irrespective of the hold bit. So, in order to handle transfers
>> greater than 252 bytes, the transfer size register has to be maintained at a
>> value >= 1. This patch implements the same.
>
> Why 252 Bytes ? Is it word allign or what ?
>
>> The interrupt status is cleared at the beginning of the isr instead of
>> the end, to avoid missing any interrupts - this is in sync with the new
>> transfer handling.
>>
>
> No need to write this, actually this is the correct way of handling interrupt.
>
>> Signed-off-by: Harini Katakam <harinik@xilinx.com>
>> ---
>>
>> v2:
>> No changes
>>
>> ---
>> drivers/i2c/busses/i2c-cadence.c | 156 ++++++++++++++++++++------------------
>> 1 file changed, 81 insertions(+), 75 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
>> index 63f3f03..e54899e 100644
>> --- a/drivers/i2c/busses/i2c-cadence.c
>> +++ b/drivers/i2c/busses/i2c-cadence.c
>> @@ -126,6 +126,7 @@
>> * @suspended: Flag holding the device's PM status
>> * @send_count: Number of bytes still expected to send
>> * @recv_count: Number of bytes still expected to receive
>> + * @curr_recv_count: Number of bytes to be received in current transfer
>
> Please do the alignment properly
Alignments are correct when you apply this patch.
Please let us know if you see any problem.
Thanks,
Michal
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
@ 2014-12-05 8:15 ` Michal Simek
0 siblings, 0 replies; 32+ messages in thread
From: Michal Simek @ 2014-12-05 8:15 UTC (permalink / raw)
To: linux-arm-kernel
On 12/05/2014 06:41 AM, rajeev kumar wrote:
> On Wed, Dec 3, 2014 at 6:05 PM, Harini Katakam <harinik@xilinx.com> wrote:
>> The I2C controller sends a NACK to the slave when transfer size register
>> reaches zero, irrespective of the hold bit. So, in order to handle transfers
>> greater than 252 bytes, the transfer size register has to be maintained at a
>> value >= 1. This patch implements the same.
>
> Why 252 Bytes ? Is it word allign or what ?
>
>> The interrupt status is cleared at the beginning of the isr instead of
>> the end, to avoid missing any interrupts - this is in sync with the new
>> transfer handling.
>>
>
> No need to write this, actually this is the correct way of handling interrupt.
>
>> Signed-off-by: Harini Katakam <harinik@xilinx.com>
>> ---
>>
>> v2:
>> No changes
>>
>> ---
>> drivers/i2c/busses/i2c-cadence.c | 156 ++++++++++++++++++++------------------
>> 1 file changed, 81 insertions(+), 75 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
>> index 63f3f03..e54899e 100644
>> --- a/drivers/i2c/busses/i2c-cadence.c
>> +++ b/drivers/i2c/busses/i2c-cadence.c
>> @@ -126,6 +126,7 @@
>> * @suspended: Flag holding the device's PM status
>> * @send_count: Number of bytes still expected to send
>> * @recv_count: Number of bytes still expected to receive
>> + * @curr_recv_count: Number of bytes to be received in current transfer
>
> Please do the alignment properly
Alignments are correct when you apply this patch.
Please let us know if you see any problem.
Thanks,
Michal
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
2014-12-03 12:35 ` Harini Katakam
` (3 preceding siblings ...)
(?)
@ 2015-01-27 15:14 ` Martin Hisch
-1 siblings, 0 replies; 32+ messages in thread
From: Martin Hisch @ 2015-01-27 15:14 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA
Harini Katakam <harinik@...> writes:
>
> The I2C controller sends a NACK to the slave when transfer size register
> reaches zero, irrespective of the hold bit. So, in order to handle transfers
> greater than 252 bytes, the transfer size register has to be maintained at a
> value >= 1. This patch implements the same.
> The interrupt status is cleared at the beginning of the isr instead of
> the end, to avoid missing any interrupts - this is in sync with the new
> transfer handling.
>
> Signed-off-by: Harini Katakam <harinik@...>
> ---
>
> v2:
> No changes
>
Hi,
just had a look to the ISR code and found:
/* wait while fifo is full */
while (cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET)
(id->curr_recv_count - CDNS_I2C_FIFO_DEPTH))
this means, that (16 x 9 bit / SCL freq) is being waited in the ISR.
At 400 kHz SCL freq, this is 360 us. Is that a valid
workaround in respect to realtime behaviour?
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2015-01-27 15:14 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-03 12:35 [PATCH v2 0/3] Cadence I2C driver fixes Harini Katakam
2014-12-03 12:35 ` Harini Katakam
2014-12-03 12:35 ` Harini Katakam
2014-12-03 12:35 ` [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers Harini Katakam
2014-12-03 12:35 ` Harini Katakam
2014-12-03 12:35 ` Harini Katakam
2014-12-04 18:32 ` Wolfram Sang
2014-12-04 18:32 ` Wolfram Sang
2014-12-04 18:32 ` Wolfram Sang
2014-12-05 4:20 ` Harini Katakam
2014-12-05 4:20 ` Harini Katakam
2014-12-05 4:20 ` Harini Katakam
2014-12-05 5:41 ` rajeev kumar
2014-12-05 5:41 ` rajeev kumar
2014-12-05 5:41 ` rajeev kumar
2014-12-05 5:57 ` Harini Katakam
2014-12-05 5:57 ` Harini Katakam
2014-12-05 8:15 ` Michal Simek
2014-12-05 8:15 ` Michal Simek
2014-12-05 8:15 ` Michal Simek
2015-01-27 15:14 ` Martin Hisch
2014-12-03 12:35 ` [PATCH v2 2/3] i2c: cadence: Set the hardware time-out register to maximum value Harini Katakam
2014-12-03 12:35 ` Harini Katakam
2014-12-04 18:30 ` Wolfram Sang
2014-12-04 18:30 ` Wolfram Sang
2014-12-03 12:35 ` [PATCH v2 3/3] i2c: cadence: Check for errata condition involving master receive Harini Katakam
2014-12-03 12:35 ` Harini Katakam
2014-12-04 18:34 ` Wolfram Sang
2014-12-04 18:34 ` Wolfram Sang
2014-12-04 18:34 ` Wolfram Sang
2014-12-05 4:12 ` Harini Katakam
2014-12-05 4:12 ` Harini Katakam
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.