[PATCH v3 2/5] i2c: img-scb: remove fifo EMPTYING interrupts handle
Now that we are using the transaction halt interrupt to safely control
repeated start transfers, we no longer need to handle the fifo
emptying interrupts.
Handling this interrupt along with Transaction Halt interrupt can
cause erratic behaviour.
Signed-off-by: Sifan Naeem
---
drivers/i2c/busses/i2c-img-scb.c | 16 +++-
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index c92bcf7c204f..0af554a16ae5 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -154,7 +154,6 @@
#define INT_TIMING BIT(18)
#define INT_FIFO_FULL_FILLING (INT_FIFO_FULL | INT_FIFO_FILLING)
-#define INT_FIFO_EMPTY_EMPTYING(INT_FIFO_EMPTY | INT_FIFO_EMPTYING)
/* Level interrupts need clearing after handling instead of before */
#define INT_LEVEL 0x01e00
@@ -176,8 +175,7 @@
INT_WRITE_ACK_ERR| \
INT_FIFO_FULL| \
INT_FIFO_FILLING | \
-INT_FIFO_EMPTY | \
-INT_FIFO_EMPTYING)
+INT_FIFO_EMPTY)
#define INT_ENABLE_MASK_WAITSTOP (INT_SLAVE_EVENT | \
INT_ADDR_ACK_ERR | \
@@ -874,16 +872,8 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
return ISR_WAITSTOP;
}
} else {
- if (int_status & INT_FIFO_EMPTY_EMPTYING) {
- /*
-* The write fifo empty indicates that we're in the
-* last byte so it's safe to start a new write
-* transaction without losing any bytes from the
-* previous one.
-* see 2.3.7 Repeated Start Transactions.
-*/
- if ((int_status & INT_FIFO_EMPTY) &&
- i2c->msg.len == 0)
+ if (int_status & INT_FIFO_EMPTY) {
+ if (i2c->msg.len == 0)
return ISR_WAITSTOP;
img_i2c_write_fifo(i2c);
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/5] i2c: img-scb: enchancements to support i2c on pistachio
The following patches are required to enchance the existing driver to support i2c on pistachio. This patch series depends on the series of fixes posted earlier[1]. The features added in this series were tested with the earlier fixes in place. Tested on Pistachio bub and on tz1090 using an Adafruit I2C Non-Volatile FRAM Breakout (256Kbit / 32KByte) eeprom. Used i2c buildroot tools to test the eeprom and the other i2c blocks. Also used dd commands to copy data to and then to dump data from the eeprom. i2ctransfer was used to test repeated starts and verified using a scope. [1]: http://marc.info/?l=linux-i2c&m=144189643306030&w=2 Changes from v1: removed v1 3/7: "mark transaction as complete when all data is read" removed v1 4/7: "mark transaction as complete when no more data to write" 1/5: Acked-by added. 2/5 reworked with J Hogan suggestions. 3/5 reworked with J Hogan suggestions. 4/5 reworked with J Hogan suggestions. 5/5 moved v1 2/7: "support repeated starts on IP v3.3 to" the end Sifan Naeem (5): i2c: img-scb: support I2C_M_IGNORE_NAK i2c: img-scb: remove fifo EMPTYING interrupts handle i2c: img-scb: add handle for stop detected interrupt i2c: img-scb: add handle for Master halt interrupt i2c: img-scb: support repeated starts on IP v3.3 drivers/i2c/busses/i2c-img-scb.c | 92 +++--- 1 file changed, 67 insertions(+), 25 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/5] i2c: img-scb: add handle for stop detected interrupt
Stop Detected interrupt is triggered when a Stop bit is detected on
the bus, which indicates the end of the current transfer.
When the end of a transfer is indicated by the Stop Detected interrupt,
drain the FIFO and signal completion for the transaction.
Signed-off-by: Sifan Naeem
---
drivers/i2c/busses/i2c-img-scb.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 0af554a16ae5..f416010a0b49 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -152,6 +152,7 @@
#define INT_TRANSACTION_DONE BIT(15)
#define INT_SLAVE_EVENTBIT(16)
#define INT_TIMING BIT(18)
+#define INT_STOP_DETECTED BIT(19)
#define INT_FIFO_FULL_FILLING (INT_FIFO_FULL | INT_FIFO_FILLING)
@@ -175,7 +176,8 @@
INT_WRITE_ACK_ERR| \
INT_FIFO_FULL| \
INT_FIFO_FILLING | \
-INT_FIFO_EMPTY)
+INT_FIFO_EMPTY | \
+INT_STOP_DETECTED)
#define INT_ENABLE_MASK_WAITSTOP (INT_SLAVE_EVENT | \
INT_ADDR_ACK_ERR | \
@@ -865,6 +867,13 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
+ if (int_status & INT_STOP_DETECTED) {
+ /* Drain remaining data in FIFO and complete transaction */
+ if (i2c->msg.flags & I2C_M_RD)
+ img_i2c_read_fifo(i2c);
+ return ISR_COMPLETE(0);
+ }
+
if (i2c->msg.flags & I2C_M_RD) {
if (int_status & INT_FIFO_FULL_FILLING) {
img_i2c_read_fifo(i2c);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 5/5] i2c: img-scb: support repeated starts on IP v3.3
In version 3.3 of the IP when transaction halt is set, an interrupt
will be generated after each byte of a transfer instead of after
every transfer but before the stop bit.
Due to this behaviour we have to be careful that every time we
release the transaction halt we have to re-enable it straight away
so that we only process a single byte, not doing so will result in
all remaining bytes been processed and a stop bit being issued,
which will prevent us having a repeated start.
This change will have no effect on earlier versions of the IP.
Signed-off-by: Sifan Naeem
Acked-by: James Hogan
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c | 45 ++
1 file changed, 36 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 991118f837be..379ef9c31664 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -513,7 +513,17 @@ static void img_i2c_soft_reset(struct img_i2c *i2c)
SCB_CONTROL_CLK_ENABLE | SCB_CONTROL_SOFT_RESET);
}
-/* enable or release transaction halt for control of repeated starts */
+/*
+ * Enable or release transaction halt for control of repeated starts.
+ * In version 3.3 of the IP when transaction halt is set, an interrupt
+ * will be generated after each byte of a transfer instead of after
+ * every transfer but before the stop bit.
+ * Due to this behaviour we have to be careful that every time we
+ * release the transaction halt we have to re-enable it straight away
+ * so that we only process a single byte, not doing so will result in
+ * all remaining bytes been processed and a stop bit being issued,
+ * which will prevent us having a repeated start.
+ */
static void img_i2c_transaction_halt(struct img_i2c *i2c, bool t_halt)
{
u32 val;
@@ -582,7 +592,6 @@ static void img_i2c_read(struct img_i2c *i2c)
img_i2c_writel(i2c, SCB_READ_ADDR_REG, i2c->msg.addr);
img_i2c_writel(i2c, SCB_READ_COUNT_REG, i2c->msg.len);
- img_i2c_transaction_halt(i2c, false);
mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
}
@@ -596,7 +605,6 @@ static void img_i2c_write(struct img_i2c *i2c)
img_i2c_writel(i2c, SCB_WRITE_ADDR_REG, i2c->msg.addr);
img_i2c_writel(i2c, SCB_WRITE_COUNT_REG, i2c->msg.len);
- img_i2c_transaction_halt(i2c, false);
mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
img_i2c_write_fifo(i2c);
@@ -862,7 +870,7 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
/* Enable transaction halt on start bit */
if (!i2c->last_msg && line_status & LINESTAT_START_BIT_DET) {
- img_i2c_transaction_halt(i2c, true);
+ img_i2c_transaction_halt(i2c, !i2c->last_msg);
/* we're no longer interested in the slave event */
i2c->int_enable &= ~INT_SLAVE_EVENT;
}
@@ -1084,12 +1092,31 @@ static int img_i2c_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs,
img_i2c_writel(i2c, SCB_INT_CLEAR_REG, ~0);
img_i2c_writel(i2c, SCB_CLEAR_REG, ~0);
- if (atomic)
+ if (atomic) {
img_i2c_atomic_start(i2c);
- else if (msg->flags & I2C_M_RD)
- img_i2c_read(i2c);
- else
- img_i2c_write(i2c);
+ } else {
+ /*
+* Enable transaction halt if not the last message in
+* the queue so that we can control repeated starts.
+*/
+ img_i2c_transaction_halt(i2c, !i2c->last_msg);
+
+ if (msg->flags & I2C_M_RD)
+ img_i2c_read(i2c);
+ else
+ img_i2c_write(i2c);
+
+ /*
+* Release and then enable transaction halt, to
+* allow only a single byte to proceed.
+* This doesn't have an effect on the initial transfer
+* but will allow the following transfers to start
+* processing if the previous transfer was marked as
+* complete while the i2c block was halted.
+*/
+ img_i2c_transaction_halt(i2c, false);
+ img_i2c_transaction_halt(i2c, !i2c->last_msg);
+ }
spin_unlock_irqrestore(&i2c->lock, flags);
time_left = wait_for_completion_timeout(&i2c->msg_complete,
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/5] i2c: img-scb: add handle for Master halt interrupt
Master halt is issued after each byte of a transaction is processed in
IP version 3.3.
Master halt will stall the bus by holding the SCK line low until the
halt bit in the scb_general_control is cleared.
After the last byte of a transfer is processed we can use the Master
Halt interrupt to facilitate a repeated start transfer without
issuing a stop bit.
Signed-off-by: Sifan Naeem
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index f416010a0b49..991118f837be 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -151,6 +151,7 @@
#define INT_FIFO_EMPTYING BIT(12)
#define INT_TRANSACTION_DONE BIT(15)
#define INT_SLAVE_EVENTBIT(16)
+#define INT_MASTER_HALTED BIT(17)
#define INT_TIMING BIT(18)
#define INT_STOP_DETECTED BIT(19)
@@ -177,6 +178,7 @@
INT_FIFO_FULL| \
INT_FIFO_FILLING | \
INT_FIFO_EMPTY | \
+INT_MASTER_HALTED| \
INT_STOP_DETECTED)
#define INT_ENABLE_MASK_WAITSTOP (INT_SLAVE_EVENT | \
@@ -875,18 +877,27 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
}
if (i2c->msg.flags & I2C_M_RD) {
- if (int_status & INT_FIFO_FULL_FILLING) {
+ if (int_status & (INT_FIFO_FULL_FILLING | INT_MASTER_HALTED)) {
img_i2c_read_fifo(i2c);
if (i2c->msg.len == 0)
return ISR_WAITSTOP;
}
} else {
- if (int_status & INT_FIFO_EMPTY) {
- if (i2c->msg.len == 0)
+ if (int_status & (INT_FIFO_EMPTY | INT_MASTER_HALTED)) {
+ if ((int_status & INT_FIFO_EMPTY) &&
+ i2c->msg.len == 0)
return ISR_WAITSTOP;
img_i2c_write_fifo(i2c);
}
}
+ if (int_status & INT_MASTER_HALTED) {
+ /*
+* Release and then enable transaction halt, to
+* allow only a single byte to proceed.
+*/
+ img_i2c_transaction_halt(i2c, false);
+ img_i2c_transaction_halt(i2c, !i2c->last_msg);
+ }
return 0;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/5] i2c: img-scb: support I2C_M_IGNORE_NAK
This commit adds support for the I2C_M_IGNORE_NAK protocol
modification.
Such behaviour can only be implemented in atomic mode. So, if a
transaction contains a message with such flag the drivers
switches to atomic mode. The implementation consists simply in
treating NAKs as ACKs.
Signed-off-by: Sifan Naeem
Acked-by: James Hogan
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 3795fe130ef2..c92bcf7c204f 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -750,7 +750,9 @@ static unsigned int img_i2c_atomic(struct img_i2c *i2c,
next_cmd = CMD_RET_ACK;
break;
case CMD_RET_ACK:
- if (i2c->line_status & LINESTAT_ACK_DET) {
+ if (i2c->line_status & LINESTAT_ACK_DET ||
+ (i2c->line_status & LINESTAT_NACK_DET &&
+ i2c->msg.flags & I2C_M_IGNORE_NAK)) {
if (i2c->msg.len == 0) {
next_cmd = CMD_GEN_STOP;
} else if (i2c->msg.flags & I2C_M_RD) {
@@ -1017,20 +1019,23 @@ static int img_i2c_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs,
return -EIO;
for (i = 0; i < num; i++) {
- if (likely(msgs[i].len))
- continue;
/*
* 0 byte reads are not possible because the slave could try
* and pull the data line low, preventing a stop bit.
*/
- if (unlikely(msgs[i].flags & I2C_M_RD))
+ if (!msgs[i].len && msgs[i].flags & I2C_M_RD)
return -EIO;
/*
* 0 byte writes are possible and used for probing, but we
* cannot do them in automatic mode, so use atomic mode
* instead.
+*
+* Also, the I2C_M_IGNORE_NAK mode can only be implemented
+* in atomic mode.
*/
- atomic = true;
+ if (!msgs[i].len ||
+ (msgs[i].flags & I2C_M_IGNORE_NAK))
+ atomic = true;
}
ret = clk_prepare_enable(i2c->scb_clk);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 2/5] i2c: img-scb: remove fifo EMPTYING interrupts handle
Hi James,
> -Original Message-
> From: James Hogan
> Sent: 03 September 2015 09:54
> To: Sifan Naeem; Wolfram Sang; linux-i2c@vger.kernel.org; Ezequiel Garcia
> Cc: Ionela Voinescu
> Subject: Re: [PATCH v2 2/5] i2c: img-scb: remove fifo EMPTYING interrupts
> handle
>
> On 14/08/15 16:50, Sifan Naeem wrote:
> > Now that we are using the transaction halt interrupt to safely control
>
> Is that referring to patch 4, which comes after this one?
Yes, sorry will update the commit message.
>
> > repeated start transfers, we no longer need to handle the fifo
> > emptying interrupts.
> >
> > Handling this interrupt along with Transaction Halt interrupt can
> > cause erratic behaviour.
>
> You said in response to my question before:
> > EMPTYING interrupt indicates that the transfer is in its last byte,
> > and in old ip versions it was safe to start a new transfer at this
> > point. The erratic behaviour I saw was due to how the latest IP
> > handles Master Halt. In this IP a transaction is halted after each
> > byte of a transfer. Having to halt the transfer after the last byte
> > means we can no longer service the EMPTYING interrupt, doing so may
> > cause repeated start transfers to fails.
>
> That doesn't look like its what the code did though. If emptying and not
> empty, it only wrote to fifo, but didn't start the next transaction.
>
The issue might be caused by the data being written to the fifo, hence
i2c->msg.len = 0, but the transfer is actually still halted. So it will need
the T_halt being lifted.
And as I saw this issue intermittently, it might be a case of the order
interrupts are serviced.
Where as in the old IP, after data is written it was safe to return
ISR_WAITSTOP.
Thanks,
Sifan
> >
> > Signed-off-by: Sifan Naeem
> > ---
> > drivers/i2c/busses/i2c-img-scb.c | 16 +++-
> > 1 file changed, 3 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c
> > b/drivers/i2c/busses/i2c-img-scb.c
> > index ad1d1df943db..75a44e794d75 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -154,7 +154,6 @@
> > #define INT_TIMING BIT(18)
> >
> > #define INT_FIFO_FULL_FILLING (INT_FIFO_FULL |
> INT_FIFO_FILLING)
> > -#define INT_FIFO_EMPTY_EMPTYING(INT_FIFO_EMPTY |
> INT_FIFO_EMPTYING)
> >
> > /* Level interrupts need clearing after handling instead of before */
> > #define INT_LEVEL 0x01e00
> > @@ -176,8 +175,7 @@
> > INT_WRITE_ACK_ERR| \
> > INT_FIFO_FULL| \
> > INT_FIFO_FILLING | \
> > -INT_FIFO_EMPTY | \
> > -INT_FIFO_EMPTYING)
> > +INT_FIFO_EMPTY)
>
> img_i2c_write_fifo also clears INT_FIFO_EMPTYING from int_enable if
> nothing left to write. That would seem redundant after this change.
>
> Cheers
> James
>
> >
> > #define INT_ENABLE_MASK_WAITSTOP (INT_SLAVE_EVENT | \
> > INT_ADDR_ACK_ERR | \
> > @@ -882,16 +880,8 @@ static unsigned int img_i2c_auto(struct img_i2c
> *i2c,
> > return ISR_WAITSTOP;
> > }
> > } else {
> > - if (int_status & INT_FIFO_EMPTY_EMPTYING) {
> > - /*
> > -* The write fifo empty indicates that we're in the
> > -* last byte so it's safe to start a new write
> > -* transaction without losing any bytes from the
> > -* previous one.
> > -* see 2.3.7 Repeated Start Transactions.
> > -*/
> > - if ((int_status & INT_FIFO_EMPTY) &&
> > - i2c->msg.len == 0)
> > + if (int_status & INT_FIFO_EMPTY) {
> > + if (i2c->msg.len == 0)
> > return ISR_WAITSTOP;
> > img_i2c_write_fifo(i2c);
> > }
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/7] i2c: img-scb: enable fencing for all versions of the ip
The code to read from the master read fifo, and write to the master
write fifo, checks a bit in an SCB register before every byte to
ensure that the fifo is not full (write fifo) or empty (read fifo).
Due to clock domain crossing inside the SCB block the updated value
of this bit is only visible after 2 cycles.
The scb_wr_rd_fence() function does 2 dummy writes (to the read-only
revision register), and it's called before reading from or writing to the
fifos to ensure that subsequent reads of the fifo status bits do not read
stale values.
As the 2 dummy writes are required in all versions of the ip, the version
check is dropped.
Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB
driver")
Signed-off-by: Sifan Naeem
Acked-by: James Hogan
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c |8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 00ffd6613680..5c3c61586d4a 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -278,8 +278,6 @@
#define ISR_COMPLETE(err) (ISR_COMPLETE_M | (ISR_STATUS_M & (err)))
#define ISR_FATAL(err) (ISR_COMPLETE(err) | ISR_FATAL_M)
-#define REL_SOC_IP_SCB_2_2_1 0x00020201
-
enum img_i2c_mode {
MODE_INACTIVE,
MODE_RAW,
@@ -1120,10 +1118,8 @@ static int img_i2c_init(struct img_i2c *i2c)
return -EINVAL;
}
- if (rev == REL_SOC_IP_SCB_2_2_1) {
- i2c->need_wr_rd_fence = true;
- dev_info(i2c->adap.dev.parent, "fence quirk enabled");
- }
+ /* Fencing enabled by default. */
+ i2c->need_wr_rd_fence = true;
bitrate_khz = i2c->bitrate / 1000;
clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 5/7] i2c: img-scb: use line_status instead of i2c->line_status
i2c->line_status accumulates the line status bits that have been seen
with each interrupt. As we're only interested in that bit from the
current interrupt, refer to line_status (the argument to img_i2c_auto)
instead of i2c->line_status.
Signed-off-by: Sifan Naeem
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index e4daebcdf824..049d193d01ac 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -857,7 +857,7 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
}
/* Enable transaction halt on start bit */
- if (!i2c->last_msg && i2c->line_status & LINESTAT_START_BIT_DET) {
+ if (!i2c->last_msg && line_status & LINESTAT_START_BIT_DET) {
img_i2c_transaction_halt(i2c, true);
/* we're no longer interested in the slave event */
i2c->int_enable &= ~INT_SLAVE_EVENT;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/7] i2c: img-scb: do dummy writes before fifo access
Move scb_wr_rd_fence to before reading from fifo and writing to
fifo to make sure the the first read/write is done after the required
number of cycles.
Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB
driver")
Signed-off-by: Sifan Naeem
Acked-by: James Hogan
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 5c3c61586d4a..0368d91b6805 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -534,6 +534,7 @@ static void img_i2c_read_fifo(struct img_i2c *i2c)
u32 fifo_status;
u8 data;
+ img_i2c_wr_rd_fence(i2c);
fifo_status = img_i2c_readl(i2c, SCB_FIFO_STATUS_REG);
if (fifo_status & FIFO_READ_EMPTY)
break;
@@ -542,7 +543,6 @@ static void img_i2c_read_fifo(struct img_i2c *i2c)
*i2c->msg.buf = data;
img_i2c_writel(i2c, SCB_READ_FIFO_REG, 0xff);
- img_i2c_wr_rd_fence(i2c);
i2c->msg.len--;
i2c->msg.buf++;
}
@@ -554,12 +554,12 @@ static void img_i2c_write_fifo(struct img_i2c *i2c)
while (i2c->msg.len) {
u32 fifo_status;
+ img_i2c_wr_rd_fence(i2c);
fifo_status = img_i2c_readl(i2c, SCB_FIFO_STATUS_REG);
if (fifo_status & FIFO_WRITE_FULL)
break;
img_i2c_writel(i2c, SCB_WRITE_DATA_REG, *i2c->msg.buf);
- img_i2c_wr_rd_fence(i2c);
i2c->msg.len--;
i2c->msg.buf++;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/7] i2c: img-scb: fix LOW and HIGH period values for the SCL clock
Currently, after determining the minimum value for the High period
(TCKH) the remainder of the internal clock pulses is set as the Low
period (TCKL). This causes the i2c clock duty cycle to be much less
than 50%.
Modify the starting position to TCKH and TCKL at 50% of the internal
clock, and adjusts the TCKH and TCKL values from there should the
minimum value for TCKL not be met. This results in duty cycles closer
to 50%.
Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB
driver")
Signed-off-by: Sifan Naeem
Acked-by: James Hogan
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index b4f59e1a5cac..e4daebcdf824 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -1178,25 +1178,29 @@ static int img_i2c_init(struct img_i2c *i2c)
((bitrate_khz * clk_period) / 2))
int_bitrate++;
- /* Setup TCKH value */
- tckh = DIV_ROUND_UP(timing.tckh, clk_period);
+ /*
+* Setup clock duty cycle, start with 50% and adjust TCKH and TCKL
+* values from there if they don't meet minimum timing requirements
+*/
+ tckh = int_bitrate / 2;
+ tckl = int_bitrate - tckh;
- if (tckh > 0)
- data = tckh - 1;
- else
- data = 0;
+ /* Adjust TCKH and TCKL values */
+ data = DIV_ROUND_UP(timing.tckl, clk_period);
- img_i2c_writel(i2c, SCB_TIME_TCKH_REG, data);
+ if (tckl < data) {
+ tckl = data;
+ tckh = int_bitrate - tckl;
+ }
- /* Setup TCKL value */
- tckl = int_bitrate - tckh;
+ if (tckh > 0)
+ --tckh;
if (tckl > 0)
- data = tckl - 1;
- else
- data = 0;
+ --tckl;
- img_i2c_writel(i2c, SCB_TIME_TCKL_REG, data);
+ img_i2c_writel(i2c, SCB_TIME_TCKH_REG, tckh);
+ img_i2c_writel(i2c, SCB_TIME_TCKL_REG, tckl);
/* Setup TSDH value */
tsdh = DIV_ROUND_UP(timing.tsdh, clk_period);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/7] i2c: img-scb: fixes to support i2c on pistachio
The following patches are required to fix the existing driver to support i2c on pistachio. Tested on Pistachio bub using an Adafruit I2C Non-Volatile FRAM Breakout (256Kbit / 32KByte) eeprom. Used i2c buildroot tools to test the eeprom and the other i2c blocks. Also used dd commands to copy data to and then to dump data from the eeprom. i2ctransfer was used to test repeated starts and verified using a scope. Odeally these fixes should go in to Stable kernel (v3.19+). Changes from v2: 5/7: Remove start bit detected not required, use line_status instead of i2c->line_status 7/7: use %u instead if %d for unsigned Sifan Naeem (7): i2c: img-scb: enable fencing for all versions of the ip i2c: img-scb: do dummy writes before fifo access i2c: img-scb: use DIV_ROUND_UP to round divisor values i2c: img-scb: fix LOW and HIGH period values for the SCL clock i2c: img-scb: use line_status instead of i2c->line_status i2c: img-scb: Clear line and interrupt status before starting a transfer i2c: img-scb: verify support for requested bit rate drivers/i2c/busses/i2c-img-scb.c | 73 ++ 1 file changed, 43 insertions(+), 30 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 6/7] i2c: img-scb: Clear line and interrupt status before starting a transfer
Clear line status and all generated interrupts from the interrupt
status register before starting a transfer, as we may have
unserviced interrupts from previous transfers that might be
handled in the context of the new transfer.
Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB
driver")
Signed-off-by: Sifan Naeem
Acked-by: James Hogan
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c |9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 049d193d01ac..0fa47158dbe4 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -1060,6 +1060,15 @@ static int img_i2c_xfer(struct i2c_adapter *adap, struct
i2c_msg *msgs,
i2c->last_msg = (i == num - 1);
reinit_completion(&i2c->msg_complete);
+ /*
+* Clear line status and all interrupts before starting a
+* transfer, as we may have unserviced interrupts from
+* previous transfers that might be handled in the context
+* of the new transfer.
+*/
+ img_i2c_writel(i2c, SCB_INT_CLEAR_REG, ~0);
+ img_i2c_writel(i2c, SCB_CLEAR_REG, ~0);
+
if (atomic)
img_i2c_atomic_start(i2c);
else if (msg->flags & I2C_M_RD)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/7] i2c: img-scb: use DIV_ROUND_UP to round divisor values
Using % can be slow depending on the architecture.
Using DIV_ROUND_UP is nicer and more efficient way to do it.
Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB
driver")
Signed-off-by: Sifan Naeem
Acked-by: James Hogan
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c |8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 0368d91b6805..b4f59e1a5cac 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -1179,9 +1179,7 @@ static int img_i2c_init(struct img_i2c *i2c)
int_bitrate++;
/* Setup TCKH value */
- tckh = timing.tckh / clk_period;
- if (timing.tckh % clk_period)
- tckh++;
+ tckh = DIV_ROUND_UP(timing.tckh, clk_period);
if (tckh > 0)
data = tckh - 1;
@@ -1201,9 +1199,7 @@ static int img_i2c_init(struct img_i2c *i2c)
img_i2c_writel(i2c, SCB_TIME_TCKL_REG, data);
/* Setup TSDH value */
- tsdh = timing.tsdh / clk_period;
- if (timing.tsdh % clk_period)
- tsdh++;
+ tsdh = DIV_ROUND_UP(timing.tsdh, clk_period);
if (tsdh > 1)
data = tsdh - 1;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 7/7] i2c: img-scb: verify support for requested bit rate
The requested bit rate can be outside the range supported by the driver.
The maximum bit rate this driver supports at the moment is 400Khz.
If the requested bit rate is larger than the maximum supported by the
driver, set the bitrate to the maximum supported before bitrate_khz is
calculated.
Maximum speed supported by the driver can be increased to 1Mhz by
adding support for "fast plus mode" in the future.
Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB
driver")
Signed-off-by: Sifan Naeem
Acked-by: James Hogan
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 0fa47158dbe4..3795fe130ef2 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -1130,9 +1130,6 @@ static int img_i2c_init(struct img_i2c *i2c)
/* Fencing enabled by default. */
i2c->need_wr_rd_fence = true;
- bitrate_khz = i2c->bitrate / 1000;
- clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
-
/* Determine what mode we're in from the bitrate */
timing = timings[0];
for (i = 0; i < ARRAY_SIZE(timings); i++) {
@@ -1141,6 +1138,17 @@ static int img_i2c_init(struct img_i2c *i2c)
break;
}
}
+ if (i2c->bitrate > timings[ARRAY_SIZE(timings) - 1].max_bitrate) {
+ dev_warn(i2c->adap.dev.parent,
+"requested bitrate (%u) is higher than the max bitrate
supported (%u)\n",
+i2c->bitrate,
+timings[ARRAY_SIZE(timings) - 1].max_bitrate);
+ timing = timings[ARRAY_SIZE(timings) - 1];
+ i2c->bitrate = timing.max_bitrate;
+ }
+
+ bitrate_khz = i2c->bitrate / 1000;
+ clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
/* Find the prescale that would give us that inc (approx delay = 0) */
prescale = SCB_OPT_INC * clk_khz / (256 * 16 * bitrate_khz);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/5] i2c: img-scb: support repeated starts on IP v3.3
In version 3.3 of the IP when transaction halt is set, an interrupt
will be generated after each byte of a transfer instead of after
every transfer but before the stop bit.
Due to this behaviour we have to be careful that every time we
release the transaction halt we have to re-enable it straight away
so that we only process a single byte, not doing so will result in
all remaining bytes been processed and a stop bit being issued,
which will prevent us having a repeated start.
This change will have no effect on earlier versions of the IP.
Signed-off-by: Sifan Naeem
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c | 45 ++
1 file changed, 36 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 837a73a43a6d..7a51f39528d7 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -513,7 +513,17 @@ static void img_i2c_soft_reset(struct img_i2c *i2c)
SCB_CONTROL_CLK_ENABLE | SCB_CONTROL_SOFT_RESET);
}
-/* enable or release transaction halt for control of repeated starts */
+/*
+ * Enable or release transaction halt for control of repeated starts.
+ * In version 3.3 of the IP when transaction halt is set, an interrupt
+ * will be generated after each byte of a transfer instead of after
+ * every transfer but before the stop bit.
+ * Due to this behaviour we have to be careful that every time we
+ * release the transaction halt we have to re-enable it straight away
+ * so that we only process a single byte, not doing so will result in
+ * all remaining bytes been processed and a stop bit being issued,
+ * which will prevent us having a repeated start.
+ */
static void img_i2c_transaction_halt(struct img_i2c *i2c, bool t_halt)
{
u32 val;
@@ -582,7 +592,6 @@ static void img_i2c_read(struct img_i2c *i2c)
img_i2c_writel(i2c, SCB_READ_ADDR_REG, i2c->msg.addr);
img_i2c_writel(i2c, SCB_READ_COUNT_REG, i2c->msg.len);
- img_i2c_transaction_halt(i2c, false);
mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
}
@@ -596,7 +605,6 @@ static void img_i2c_write(struct img_i2c *i2c)
img_i2c_writel(i2c, SCB_WRITE_ADDR_REG, i2c->msg.addr);
img_i2c_writel(i2c, SCB_WRITE_COUNT_REG, i2c->msg.len);
- img_i2c_transaction_halt(i2c, false);
mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
img_i2c_write_fifo(i2c);
@@ -863,7 +871,7 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
/* Enable transaction halt on start bit */
if (line_status & LINESTAT_START_BIT_DET) {
if (!i2c->last_msg) {
- img_i2c_transaction_halt(i2c, true);
+ img_i2c_transaction_halt(i2c, !i2c->last_msg);
/* we're no longer interested in the slave event */
i2c->int_enable &= ~INT_SLAVE_EVENT;
}
@@ -1093,12 +1101,31 @@ static int img_i2c_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs,
img_i2c_writel(i2c, SCB_INT_CLEAR_REG, ~0);
img_i2c_writel(i2c, SCB_CLEAR_REG, ~0);
- if (atomic)
+ if (atomic) {
img_i2c_atomic_start(i2c);
- else if (msg->flags & I2C_M_RD)
- img_i2c_read(i2c);
- else
- img_i2c_write(i2c);
+ } else {
+ /*
+* Enable transaction halt if not the last message in
+* the queue so that we can control repeated starts.
+*/
+ img_i2c_transaction_halt(i2c, !i2c->last_msg);
+
+ if (msg->flags & I2C_M_RD)
+ img_i2c_read(i2c);
+ else
+ img_i2c_write(i2c);
+
+ /*
+* Release and then enable transaction halt, to
+* allow only a single byte to proceed.
+* This doesn't have an effect on the initial transfer
+* but will allow the following transfers to start
+* processing if the previous transfer was marked as
+* complete while the i2c block was halted.
+*/
+ img_i2c_transaction_halt(i2c, false);
+ img_i2c_transaction_halt(i2c, !i2c->last_msg);
+ }
spin_unlock_irqrestore(&i2c->lock, flags);
time_left = wait_for_completion_timeout(&i2c->msg_complete,
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/5] i2c: img-scb: remove fifo EMPTYING interrupts handle
Now that we are using the transaction halt interrupt to safely control
repeated start transfers, we no longer need to handle the fifo
emptying interrupts.
Handling this interrupt along with Transaction Halt interrupt can
cause erratic behaviour.
Signed-off-by: Sifan Naeem
---
drivers/i2c/busses/i2c-img-scb.c | 16 +++-
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index ad1d1df943db..75a44e794d75 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -154,7 +154,6 @@
#define INT_TIMING BIT(18)
#define INT_FIFO_FULL_FILLING (INT_FIFO_FULL | INT_FIFO_FILLING)
-#define INT_FIFO_EMPTY_EMPTYING(INT_FIFO_EMPTY | INT_FIFO_EMPTYING)
/* Level interrupts need clearing after handling instead of before */
#define INT_LEVEL 0x01e00
@@ -176,8 +175,7 @@
INT_WRITE_ACK_ERR| \
INT_FIFO_FULL| \
INT_FIFO_FILLING | \
-INT_FIFO_EMPTY | \
-INT_FIFO_EMPTYING)
+INT_FIFO_EMPTY)
#define INT_ENABLE_MASK_WAITSTOP (INT_SLAVE_EVENT | \
INT_ADDR_ACK_ERR | \
@@ -882,16 +880,8 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
return ISR_WAITSTOP;
}
} else {
- if (int_status & INT_FIFO_EMPTY_EMPTYING) {
- /*
-* The write fifo empty indicates that we're in the
-* last byte so it's safe to start a new write
-* transaction without losing any bytes from the
-* previous one.
-* see 2.3.7 Repeated Start Transactions.
-*/
- if ((int_status & INT_FIFO_EMPTY) &&
- i2c->msg.len == 0)
+ if (int_status & INT_FIFO_EMPTY) {
+ if (i2c->msg.len == 0)
return ISR_WAITSTOP;
img_i2c_write_fifo(i2c);
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/7] i2c: img-scb: do dummy writes before fifo access
Move scb_wr_rd_fence to before reading from fifo and writing to
fifo to make sure the the first read/write is done after the required
number of cycles.
Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB
driver")
Signed-off-by: Sifan Naeem
Acked-by: James Hogan
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 5c3c61586d4a..0368d91b6805 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -534,6 +534,7 @@ static void img_i2c_read_fifo(struct img_i2c *i2c)
u32 fifo_status;
u8 data;
+ img_i2c_wr_rd_fence(i2c);
fifo_status = img_i2c_readl(i2c, SCB_FIFO_STATUS_REG);
if (fifo_status & FIFO_READ_EMPTY)
break;
@@ -542,7 +543,6 @@ static void img_i2c_read_fifo(struct img_i2c *i2c)
*i2c->msg.buf = data;
img_i2c_writel(i2c, SCB_READ_FIFO_REG, 0xff);
- img_i2c_wr_rd_fence(i2c);
i2c->msg.len--;
i2c->msg.buf++;
}
@@ -554,12 +554,12 @@ static void img_i2c_write_fifo(struct img_i2c *i2c)
while (i2c->msg.len) {
u32 fifo_status;
+ img_i2c_wr_rd_fence(i2c);
fifo_status = img_i2c_readl(i2c, SCB_FIFO_STATUS_REG);
if (fifo_status & FIFO_WRITE_FULL)
break;
img_i2c_writel(i2c, SCB_WRITE_DATA_REG, *i2c->msg.buf);
- img_i2c_wr_rd_fence(i2c);
i2c->msg.len--;
i2c->msg.buf++;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/7] i2c: img-scb: remove start bit detected status after handling
Remove start bit detected status after it is handled,
doing so will prevent this condition being hit for
every interrupt on a particular transfer.
Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB
driver")
Signed-off-by: Sifan Naeem
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index e4daebcdf824..200108dbd194 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -857,10 +857,18 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
}
/* Enable transaction halt on start bit */
- if (!i2c->last_msg && i2c->line_status & LINESTAT_START_BIT_DET) {
- img_i2c_transaction_halt(i2c, true);
- /* we're no longer interested in the slave event */
- i2c->int_enable &= ~INT_SLAVE_EVENT;
+ if (line_status & LINESTAT_START_BIT_DET) {
+ if (!i2c->last_msg) {
+ img_i2c_transaction_halt(i2c, true);
+ /* we're no longer interested in the slave event */
+ i2c->int_enable &= ~INT_SLAVE_EVENT;
+ }
+ /*
+* Remove start bit detected status after it is handled,
+* doing so will prevent this condition being hit for
+* every interrupt on a particular transfer.
+*/
+ i2c->line_status &= ~LINESTAT_START_BIT_DET;
}
mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/5] i2c: img-scb: enchancements to support i2c on pistachio
The following patches are required to enchance the existing driver to support i2c on pistachio. This patch series depends on the series of fixes posted earlier[1]. The features added in this series were tested with the earlier fixes in place. Tested on Pistachio bub and on tz1090 using an Adafruit I2C Non-Volatile FRAM Breakout (256Kbit / 32KByte) eeprom. Used i2c buildroot tools to test the eeprom and the other i2c blocks. Also used dd commands to copy data to and then to dump data from the eeprom. i2ctransfer was used to test repeated starts and verified using a scope. [1]: http://marc.info/?l=linux-i2c&m=143799753022541&w=2 Changes from v1: removed v1 3/7: "mark transaction as complete when all data is read" removed v1 4/7: "mark transaction as complete when no more data to write" 1/5: Acked-by added. 2/5 reworked with J Hogan suggestions. 3/5 reworked with J Hogan suggestions. 4/5 reworked with J Hogan suggestions. 5/5 moved v1 2/7: "support repeated starts on IP v3.3 to" the end Sifan Naeem (5): i2c: img-scb: support I2C_M_IGNORE_NAK i2c: img-scb: remove fifo EMPTYING interrupts handle i2c: img-scb: add handle for stop detected interrupt i2c: img-scb: add handle for Master halt interrupt i2c: img-scb: support repeated starts on IP v3.3 drivers/i2c/busses/i2c-img-scb.c | 93 -- 1 file changed, 68 insertions(+), 25 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/7] i2c: img-scb: fix LOW and HIGH period values for the SCL clock
Currently, after determining the minimum value for the High period
(TCKH) the remainder of the internal clock pulses is set as the Low
period (TCKL). This causes the i2c clock duty cycle to be much less
than 50%.
Modify the starting position to TCKH and TCKL at 50% of the internal
clock, and adjusts the TCKH and TCKL values from there should the
minimum value for TCKL not be met. This results in duty cycles closer
to 50%.
Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB
driver")
Signed-off-by: Sifan Naeem
Acked-by: James Hogan
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index b4f59e1a5cac..e4daebcdf824 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -1178,25 +1178,29 @@ static int img_i2c_init(struct img_i2c *i2c)
((bitrate_khz * clk_period) / 2))
int_bitrate++;
- /* Setup TCKH value */
- tckh = DIV_ROUND_UP(timing.tckh, clk_period);
+ /*
+* Setup clock duty cycle, start with 50% and adjust TCKH and TCKL
+* values from there if they don't meet minimum timing requirements
+*/
+ tckh = int_bitrate / 2;
+ tckl = int_bitrate - tckh;
- if (tckh > 0)
- data = tckh - 1;
- else
- data = 0;
+ /* Adjust TCKH and TCKL values */
+ data = DIV_ROUND_UP(timing.tckl, clk_period);
- img_i2c_writel(i2c, SCB_TIME_TCKH_REG, data);
+ if (tckl < data) {
+ tckl = data;
+ tckh = int_bitrate - tckl;
+ }
- /* Setup TCKL value */
- tckl = int_bitrate - tckh;
+ if (tckh > 0)
+ --tckh;
if (tckl > 0)
- data = tckl - 1;
- else
- data = 0;
+ --tckl;
- img_i2c_writel(i2c, SCB_TIME_TCKL_REG, data);
+ img_i2c_writel(i2c, SCB_TIME_TCKH_REG, tckh);
+ img_i2c_writel(i2c, SCB_TIME_TCKL_REG, tckl);
/* Setup TSDH value */
tsdh = DIV_ROUND_UP(timing.tsdh, clk_period);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/7] i2c: img-scb: enable fencing for all versions of the ip
The code to read from the master read fifo, and write to the master
write fifo, checks a bit in an SCB register before every byte to
ensure that the fifo is not full (write fifo) or empty (read fifo).
Due to clock domain crossing inside the SCB block the updated value
of this bit is only visible after 2 cycles.
The scb_wr_rd_fence() function does 2 dummy writes (to the read-only
revision register), and it's called before reading from or writing to the
fifos to ensure that subsequent reads of the fifo status bits do not read
stale values.
As the 2 dummy writes are required in all versions of the ip, the version
check is dropped.
Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB
driver")
Signed-off-by: Sifan Naeem
Acked-by: James Hogan
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c |8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 00ffd6613680..5c3c61586d4a 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -278,8 +278,6 @@
#define ISR_COMPLETE(err) (ISR_COMPLETE_M | (ISR_STATUS_M & (err)))
#define ISR_FATAL(err) (ISR_COMPLETE(err) | ISR_FATAL_M)
-#define REL_SOC_IP_SCB_2_2_1 0x00020201
-
enum img_i2c_mode {
MODE_INACTIVE,
MODE_RAW,
@@ -1120,10 +1118,8 @@ static int img_i2c_init(struct img_i2c *i2c)
return -EINVAL;
}
- if (rev == REL_SOC_IP_SCB_2_2_1) {
- i2c->need_wr_rd_fence = true;
- dev_info(i2c->adap.dev.parent, "fence quirk enabled");
- }
+ /* Fencing enabled by default. */
+ i2c->need_wr_rd_fence = true;
bitrate_khz = i2c->bitrate / 1000;
clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/5] i2c: img-scb: support I2C_M_IGNORE_NAK
This commit adds support for the I2C_M_IGNORE_NAK protocol
modification.
Such behaviour can only be implemented in atomic mode. So, if a
transaction contains a message with such flag the drivers
switches to atomic mode. The implementation consists simply in
treating NAKs as ACKs.
Signed-off-by: Sifan Naeem
Acked-by: James Hogan
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index c16caaccc363..ad1d1df943db 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -750,7 +750,9 @@ static unsigned int img_i2c_atomic(struct img_i2c *i2c,
next_cmd = CMD_RET_ACK;
break;
case CMD_RET_ACK:
- if (i2c->line_status & LINESTAT_ACK_DET) {
+ if (i2c->line_status & LINESTAT_ACK_DET ||
+ (i2c->line_status & LINESTAT_NACK_DET &&
+ i2c->msg.flags & I2C_M_IGNORE_NAK)) {
if (i2c->msg.len == 0) {
next_cmd = CMD_GEN_STOP;
} else if (i2c->msg.flags & I2C_M_RD) {
@@ -1025,20 +1027,23 @@ static int img_i2c_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs,
return -EIO;
for (i = 0; i < num; i++) {
- if (likely(msgs[i].len))
- continue;
/*
* 0 byte reads are not possible because the slave could try
* and pull the data line low, preventing a stop bit.
*/
- if (unlikely(msgs[i].flags & I2C_M_RD))
+ if (!msgs[i].len && msgs[i].flags & I2C_M_RD)
return -EIO;
/*
* 0 byte writes are possible and used for probing, but we
* cannot do them in automatic mode, so use atomic mode
* instead.
+*
+* Also, the I2C_M_IGNORE_NAK mode can only be implemented
+* in atomic mode.
*/
- atomic = true;
+ if (!msgs[i].len ||
+ (msgs[i].flags & I2C_M_IGNORE_NAK))
+ atomic = true;
}
ret = clk_prepare_enable(i2c->scb_clk);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/7] i2c: img-scb: use DIV_ROUND_UP to round divisor values
Using % can be slow depending on the architecture.
Using DIV_ROUND_UP is nicer and more efficient way to do it.
Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB
driver")
Signed-off-by: Sifan Naeem
Acked-by: James Hogan
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c |8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 0368d91b6805..b4f59e1a5cac 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -1179,9 +1179,7 @@ static int img_i2c_init(struct img_i2c *i2c)
int_bitrate++;
/* Setup TCKH value */
- tckh = timing.tckh / clk_period;
- if (timing.tckh % clk_period)
- tckh++;
+ tckh = DIV_ROUND_UP(timing.tckh, clk_period);
if (tckh > 0)
data = tckh - 1;
@@ -1201,9 +1199,7 @@ static int img_i2c_init(struct img_i2c *i2c)
img_i2c_writel(i2c, SCB_TIME_TCKL_REG, data);
/* Setup TSDH value */
- tsdh = timing.tsdh / clk_period;
- if (timing.tsdh % clk_period)
- tsdh++;
+ tsdh = DIV_ROUND_UP(timing.tsdh, clk_period);
if (tsdh > 1)
data = tsdh - 1;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/7] i2c: img-scb: Clear line and interrupt status before starting a transfer
Clear line status and all generated interrupts from the interrupt
status register before starting a transfer, as we may have
unserviced interrupts from previous transfers that might be
handled in the context of the new transfer.
Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB
driver")
Signed-off-by: Sifan Naeem
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c |9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 200108dbd194..6c21a7bd9a66 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -1068,6 +1068,15 @@ static int img_i2c_xfer(struct i2c_adapter *adap, struct
i2c_msg *msgs,
i2c->last_msg = (i == num - 1);
reinit_completion(&i2c->msg_complete);
+ /*
+* Clear line status and all interrupts before starting a
+* transfer, as we may have unserviced interrupts from
+* previous transfers that might be handled in the context
+* of the new transfer.
+*/
+ img_i2c_writel(i2c, SCB_INT_CLEAR_REG, ~0);
+ img_i2c_writel(i2c, SCB_CLEAR_REG, ~0);
+
if (atomic)
img_i2c_atomic_start(i2c);
else if (msg->flags & I2C_M_RD)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/5] i2c: img-scb: add handle for Master halt interrupt
Master halt is issued after each byte of a transaction is processed in
IP version 3.3.
Master halt will stall the bus by holding the SCK line low until the
halt bit in the scb_general_control is cleared.
After the last byte of a transfer is processed we can use the Master
Halt interrupt to facilitate a repeated start transfer without
issuing a stop bit.
Signed-off-by: Sifan Naeem
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 17e13ff475bb..837a73a43a6d 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -151,6 +151,7 @@
#define INT_FIFO_EMPTYING BIT(12)
#define INT_TRANSACTION_DONE BIT(15)
#define INT_SLAVE_EVENTBIT(16)
+#define INT_MASTER_HALTED BIT(17)
#define INT_TIMING BIT(18)
#define INT_STOP_DETECTED BIT(19)
@@ -177,6 +178,7 @@
INT_FIFO_FULL| \
INT_FIFO_FILLING | \
INT_FIFO_EMPTY | \
+INT_MASTER_HALTED| \
INT_STOP_DETECTED)
#define INT_ENABLE_MASK_WAITSTOP (INT_SLAVE_EVENT | \
@@ -883,17 +885,26 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
}
if (i2c->msg.flags & I2C_M_RD) {
- if (int_status & INT_FIFO_FULL_FILLING) {
+ if (int_status & (INT_FIFO_FULL_FILLING | INT_MASTER_HALTED)) {
img_i2c_read_fifo(i2c);
if (i2c->msg.len == 0)
return ISR_WAITSTOP;
}
} else {
- if (int_status & INT_FIFO_EMPTY) {
- if (i2c->msg.len == 0)
+ if (int_status & (INT_FIFO_EMPTY | INT_MASTER_HALTED)) {
+ if ((int_status & INT_FIFO_EMPTY) &&
+ i2c->msg.len == 0)
return ISR_WAITSTOP;
img_i2c_write_fifo(i2c);
}
+ }
+ if (int_status & INT_MASTER_HALTED) {
+ /*
+* Release and then enable transaction halt, to
+* allow only a single byte to proceed.
+*/
+ img_i2c_transaction_halt(i2c, false);
+ img_i2c_transaction_halt(i2c, !i2c->last_msg);
}
return 0;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/5] i2c: img-scb: add handle for stop detected interrupt
Stop Detected interrupt is triggered when a Stop bit is detected on
the bus, which indicates the end of the current transfer.
When the end of a transfer is indicated by the Stop Detected interrupt,
drain the FIFO and signal completion for the transaction. But if the
interrupt was triggered before all data is written to the fifo or with
more data expected return error with transfer complete signal.
Halting the bus is no longer necessary after a stop bit is detected
on the bus, as there cannot be a repeated start transfer when the stop
bit has been issued, hence remove the transaction halt bit.
Signed-off-by: Sifan Naeem
---
drivers/i2c/busses/i2c-img-scb.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 75a44e794d75..17e13ff475bb 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -152,6 +152,7 @@
#define INT_TRANSACTION_DONE BIT(15)
#define INT_SLAVE_EVENTBIT(16)
#define INT_TIMING BIT(18)
+#define INT_STOP_DETECTED BIT(19)
#define INT_FIFO_FULL_FILLING (INT_FIFO_FULL | INT_FIFO_FILLING)
@@ -175,7 +176,8 @@
INT_WRITE_ACK_ERR| \
INT_FIFO_FULL| \
INT_FIFO_FILLING | \
-INT_FIFO_EMPTY)
+INT_FIFO_EMPTY | \
+INT_STOP_DETECTED)
#define INT_ENABLE_MASK_WAITSTOP (INT_SLAVE_EVENT | \
INT_ADDR_ACK_ERR | \
@@ -873,6 +875,13 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
+ if (int_status & INT_STOP_DETECTED) {
+ /* Drain remaining data in FIFO and complete transaction */
+ if (i2c->msg.flags & I2C_M_RD)
+ img_i2c_read_fifo(i2c);
+ return ISR_COMPLETE(0);
+ }
+
if (i2c->msg.flags & I2C_M_RD) {
if (int_status & INT_FIFO_FULL_FILLING) {
img_i2c_read_fifo(i2c);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/7] i2c: img-scb: fixes to support i2c on pistachio
The following patches are required to fix the existing driver to support i2c on pistachio. Tested on Pistachio bub using an Adafruit I2C Non-Volatile FRAM Breakout (256Kbit / 32KByte) eeprom. Used i2c buildroot tools to test the eeprom and the other i2c blocks. Also used dd commands to copy data to and then to dump data from the eeprom. i2ctransfer was used to test repeated starts and verified using a scope. Changes from v1: removed v1 5/8: i2c-img-scb-reset-interrupts-in-img_i2c_soft_reset removed v1 7/8: i2c-img-scb-improve-transaction-complete-handle 4/7: extra space removed 5/7: line_status used instead of i2c->line_status 6/7: reworked v1 7/8 and added as new patch 7/7: reworked, limit bitrate to maximum supported 12 digit hash id used. Sifan Naeem (7): i2c: img-scb: enable fencing for all versions of the ip i2c: img-scb: do dummy writes before fifo access i2c: img-scb: use DIV_ROUND_UP to round divisor values i2c: img-scb: fix LOW and HIGH period values for the SCL clock i2c: img-scb: remove start bit detected status after handling i2c: img-scb: Clear line and interrupt status before starting a transfer i2c: img-scb: verify support for requested bit rate drivers/i2c/busses/i2c-img-scb.c | 87 +++--- 1 file changed, 54 insertions(+), 33 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 7/7] i2c: img-scb: verify support for requested bit rate
The requested bit rate can be outside the range supported by the driver.
The maximum bit rate this driver supports at the moment is 400Khz.
If the requested bit rate is larger than the maximum supported by the
driver, set the bitrate to the maximum supported before bitrate_khz is
calculated.
Maximum speed supported by the driver can be increased to 1Mhz by
adding support for "fast plus mode" in the future.
Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB
driver")
Signed-off-by: Sifan Naeem
Reviewed-by: James Hartley
---
drivers/i2c/busses/i2c-img-scb.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 6c21a7bd9a66..c16caaccc363 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -1138,9 +1138,6 @@ static int img_i2c_init(struct img_i2c *i2c)
/* Fencing enabled by default. */
i2c->need_wr_rd_fence = true;
- bitrate_khz = i2c->bitrate / 1000;
- clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
-
/* Determine what mode we're in from the bitrate */
timing = timings[0];
for (i = 0; i < ARRAY_SIZE(timings); i++) {
@@ -1149,6 +1146,17 @@ static int img_i2c_init(struct img_i2c *i2c)
break;
}
}
+ if (i2c->bitrate > timings[ARRAY_SIZE(timings) - 1].max_bitrate) {
+ dev_warn(i2c->adap.dev.parent,
+"requested bitrate (%d) is higher than the max bitrate
supported (%d)\n",
+i2c->bitrate,
+timings[ARRAY_SIZE(timings) - 1].max_bitrate);
+ timing = timings[ARRAY_SIZE(timings) - 1];
+ i2c->bitrate = timing.max_bitrate;
+ }
+
+ bitrate_khz = i2c->bitrate / 1000;
+ clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
/* Find the prescale that would give us that inc (approx delay = 0) */
prescale = SCB_OPT_INC * clk_khz / (256 * 16 * bitrate_khz);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/7] i2c: img-scb: add handle for Master halt interrupt
Hi James,
> -Original Message-
> From: James Hogan
> Sent: 29 July 2015 16:59
> To: Sifan Naeem; Wolfram Sang; linux-i2c@vger.kernel.org
> Subject: Re: [PATCH 7/7] i2c: img-scb: add handle for Master halt interrupt
>
> On 27/07/15 12:56, Sifan Naeem wrote:
> > Master halt is issued after each byte of a transaction is processed in
> > IP version 3.3.
> > Master halt will stall the bus by holding the SCK line low until the
> > halt bit in the scb_general_control is cleared.
> >
> > After the last byte of a transfer is processed we can use the Master
> > Halt interrupt to facilitate a repeated start transfer without issuing
> > a stop bit.
> >
> > Signed-off-by: Sifan Naeem
> > ---
> > drivers/i2c/busses/i2c-img-scb.c | 25 +
> > 1 file changed, 25 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c
> > b/drivers/i2c/busses/i2c-img-scb.c
> > index 90faf48..df3d25a 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -151,6 +151,7 @@
> > #define INT_FIFO_EMPTYING BIT(12)
> > #define INT_TRANSACTION_DONE BIT(15)
> > #define INT_SLAVE_EVENTBIT(16)
> > +#define INT_MASTER_HALTED BIT(17)
> > #define INT_TIMING BIT(18)
> > #define INT_STOP_DETECTED BIT(19)
> >
> > @@ -177,6 +178,7 @@
> > INT_FIFO_FULL| \
> > INT_FIFO_FILLING | \
> > INT_FIFO_EMPTY | \
> > +INT_MASTER_HALTED| \
> > INT_STOP_DETECTED)
> >
> > #define INT_ENABLE_MASK_WAITSTOP (INT_SLAVE_EVENT | \
> > @@ -901,6 +903,17 @@ static unsigned int img_i2c_auto(struct img_i2c
> *i2c,
> > mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
> >
> > if (i2c->msg.flags & I2C_M_RD) {
> > + if (int_status & INT_MASTER_HALTED) {
> > + img_i2c_read_fifo(i2c);
> > + if (i2c->msg.len == 0)
> > + return ISR_COMPLETE(0);
>
> don't you still need to wait for stop bit on last message?
>
> I suspect you could have a bit less duplication with something like this
> (again
> untested):
>
> diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-
> scb.c
> index f694b47dcf74..2de2d63083e5 100644
> --- a/drivers/i2c/busses/i2c-img-scb.c
> +++ b/drivers/i2c/busses/i2c-img-scb.c
> @@ -875,13 +875,14 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
> }
>
> if (i2c->msg.flags & I2C_M_RD) {
> - if (int_status & INT_FIFO_FULL_FILLING) {
> + if (int_status & (INT_FIFO_FULL_FILLING |
> INT_MASTER_HALTED)) {
> img_i2c_read_fifo(i2c);
> if (i2c->msg.len == 0)
> return ISR_WAITSTOP;
> }
> } else {
> - if (int_status & INT_FIFO_EMPTY_EMPTYING) {
> + if (int_status & (INT_FIFO_EMPTY_EMPTYING |
> + INT_MASTER_HALTED)) {
> /*
>* The write fifo empty indicates that we're in the
>* last byte so it's safe to start a new write @@ -895,6
> +896,14 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
> img_i2c_write_fifo(i2c);
> }
> }
> + if (int_status & INT_MASTER_HALTED) {
> + /*
> + * Release and then enable transaction halt, to allow only a
> + * single byte to proceed.
> + */
> + img_i2c_transaction_halt(i2c, false);
> + img_i2c_transaction_halt(i2c, !i2c->last_msg);
> + }
>
> return 0;
> }
>
> would that do the trick?
>
Yes, I'll try this. Getting rid of patches 3/7 and 4/7 makes it easier to
squash these together.
Sifan
> Cheers
> James
>
> > + /*
> > +* Release and then enable transaction halt, to
> > +* allow only a single byte to proceed.
> > +*/
> > + img_i2c_transaction_halt(i2c, false);
> > + img_i2c_transaction_halt(i2c, !i2c->last_msg);
> > + }
> > if (int_s
RE: [PATCH 6/7] i2c: img-scb: add handle for stop detected interrupt
Hi James,
> -Original Message-
> From: James Hogan
> Sent: 29 July 2015 16:34
> To: Sifan Naeem; Wolfram Sang; linux-i2c@vger.kernel.org
> Subject: Re: [PATCH 6/7] i2c: img-scb: add handle for stop detected interrupt
>
> On 27/07/15 12:56, Sifan Naeem wrote:
> > Stop Detected interrupt is triggered when a Stop bit is detected on
> > the bus, which indicates the end of the current transfer.
> >
> > When the end of a transfer is indicated by the Stop bit interrupt,
> > drain the FIFO and signal completion for the transaction. But if the
> > interrupt was triggered before all data is written to the fifo or with
> > more data expected return error with transfer complete signal.
> >
> > Halting the bus is no longer necessary after a stop bit is detected on
> > the bus, as there cannot be a repeated start transfer when the stop
> > bit has been issued, hence remove the transaction halt bit.
> >
> > Signed-off-by: Sifan Naeem
> > ---
> > drivers/i2c/busses/i2c-img-scb.c | 28 +++-
> > 1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c
> > b/drivers/i2c/busses/i2c-img-scb.c
> > index 10141a9..90faf48 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -152,6 +152,7 @@
> > #define INT_TRANSACTION_DONE BIT(15)
> > #define INT_SLAVE_EVENTBIT(16)
> > #define INT_TIMING BIT(18)
> > +#define INT_STOP_DETECTED BIT(19)
> >
> > #define INT_FIFO_FULL_FILLING (INT_FIFO_FULL |
> INT_FIFO_FILLING)
> >
> > @@ -175,7 +176,8 @@
> > INT_WRITE_ACK_ERR| \
> > INT_FIFO_FULL| \
> > INT_FIFO_FILLING | \
> > -INT_FIFO_EMPTY)
> > +INT_FIFO_EMPTY | \
> > +INT_STOP_DETECTED)
> >
> > #define INT_ENABLE_MASK_WAITSTOP (INT_SLAVE_EVENT | \
> > INT_ADDR_ACK_ERR | \
> > @@ -907,6 +909,18 @@ static unsigned int img_i2c_auto(struct img_i2c
> *i2c,
> > return ISR_COMPLETE(0);
> > }
> > }
> > + if (int_status & INT_STOP_DETECTED) {
> > + int ret;
> > + /*
> > +* Stop bit indicates the end of the transfer, it means
> > +* we should read all the data (or drain the FIFO). We
> > +* must signal completion for this transaction.
> > +*/
> > + img_i2c_transaction_halt(i2c, false);
>
> to get a stop bit detected, wouldn't transaction halt already have to be off?
Yes, but in case we were too slow re-enabling the master halt and a stop was
detected simultaneously to when master halt was set.
>
> > + img_i2c_read_fifo(i2c);
> > + ret = (i2c->msg.len == 0) ? 0 : EIO;
>
> If it wasn't fully transferred, wouldn't that already imply an
> INT_WRITE_ACK_ERR or INT_ADDR_ACK_ERR, which should've already been
> handled?
>
This was added as a safety check, yes I guess it's impossible to get a stop bit
before all data is transferred without an error condition.
>
> Could it then be as simple as this? (untested):
>
Yes, this would do.
Thanks,
Sifan
> diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-
> scb.c
> index 00ffd6613680..f694b47dcf74 100644
> --- a/drivers/i2c/busses/i2c-img-scb.c
> +++ b/drivers/i2c/busses/i2c-img-scb.c
> @@ -867,6 +867,13 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
>
> mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
>
> + if (int_status & INT_STOP_DETECTED) {
> + /* Drain remaining data in FIFO and complete transaction */
> + if (i2c->msg.flags & I2C_M_RD)
> + img_i2c_read_fifo(i2c);
> + return ISR_COMPLETE(0);
> + }
> +
> if (i2c->msg.flags & I2C_M_RD) {
> if (int_status & INT_FIFO_FULL_FILLING) {
> img_i2c_read_fifo(i2c);
>
> Cheers
> James
>
> > + return ISR_COMPLETE(ret);
> > + }
> > } else {
> > if (int_status & INT_FIFO_EMPTY) {
> >
RE: [PATCH 5/7] i2c: img-scb: remove fifo EMPTYING interrupts handle
Hi James,
> -Original Message-
> From: James Hogan
> Sent: 29 July 2015 17:01
> To: Sifan Naeem; Wolfram Sang; linux-i2c@vger.kernel.org
> Subject: Re: [PATCH 5/7] i2c: img-scb: remove fifo EMPTYING interrupts
> handle
>
> On 27/07/15 12:56, Sifan Naeem wrote:
> > This interrupt could have been useful for repeated start transfers as
> > the current transfer could be marked as complete while it's processing
> > the final byte of the transfer.
> > But having to use the transaction halt interrupt to safely control
> > repeated start transfers, means handling of the fifo EMPTYING
> > interrupts is no longer necessary.
> >
> > Handling this interrupt along with Transaction Halt interrupt can
> > cause erratic behaviour.
>
> I'd be interested to know the cause of the erratic behaviour. It feels a
> little
> like we're just masking a real bug somewhere.
>
EMPTYING interrupt indicates that the transfer is in its last byte, and in old
ip versions it was safe to start a new transfer at this point.
The erratic behaviour I saw was due to how the latest IP handles Master Halt.
In this IP a transaction is halted after each byte of a transfer.
Having to halt the transfer after the last byte means we can no longer service
the EMPTYING interrupt, doing so may cause repeated start transfers to fails.
Thanks,
Sifan
> Cheers
> James
>
> >
> > Signed-off-by: Sifan Naeem
> > ---
> > drivers/i2c/busses/i2c-img-scb.c |6 ++
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c
> > b/drivers/i2c/busses/i2c-img-scb.c
> > index efad4d7..10141a9 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -154,7 +154,6 @@
> > #define INT_TIMING BIT(18)
> >
> > #define INT_FIFO_FULL_FILLING (INT_FIFO_FULL |
> INT_FIFO_FILLING)
> > -#define INT_FIFO_EMPTY_EMPTYING(INT_FIFO_EMPTY |
> INT_FIFO_EMPTYING)
> >
> > /* Level interrupts need clearing after handling instead of before */
> > #define INT_LEVEL 0x01e00
> > @@ -176,8 +175,7 @@
> > INT_WRITE_ACK_ERR| \
> > INT_FIFO_FULL| \
> > INT_FIFO_FILLING | \
> > -INT_FIFO_EMPTY | \
> > -INT_FIFO_EMPTYING)
> > +INT_FIFO_EMPTY)
> >
> > #define INT_ENABLE_MASK_WAITSTOP (INT_SLAVE_EVENT | \
> > INT_ADDR_ACK_ERR | \
> > @@ -910,7 +908,7 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
> > }
> > }
> > } else {
> > - if (int_status & INT_FIFO_EMPTY_EMPTYING) {
> > + if (int_status & INT_FIFO_EMPTY) {
> > if (i2c->msg.len == 0) {
> > if (i2c->last_msg)
> > return ISR_WAITSTOP;
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/7] i2c: img-scb: mark transaction as complete when no more data to write
Hi James,
> On 27/07/15 12:55, Sifan Naeem wrote:
> > We can mark the transfer as complete without waiting for the stop bit.
> > This is important when handling repeated start transfers as we have to
> > start the next transfer without the stop bit being issued.
> >
> > This doesn't affect the older versions of the IP.
> >
> > Signed-off-by: Sifan Naeem
> > ---
> > drivers/i2c/busses/i2c-img-scb.c | 15 +--
> > 1 file changed, 5 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c
> > b/drivers/i2c/busses/i2c-img-scb.c
> > index e27c3e0..efad4d7 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -911,16 +911,11 @@ static unsigned int img_i2c_auto(struct img_i2c
> *i2c,
> > }
> > } else {
> > if (int_status & INT_FIFO_EMPTY_EMPTYING) {
> > - /*
> > -* The write fifo empty indicates that we're in the
> > -* last byte so it's safe to start a new write
> > -* transaction without losing any bytes from the
> > -* previous one.
> > -* see 2.3.7 Repeated Start Transactions.
> > -*/
> > - if ((int_status & INT_FIFO_EMPTY) &&
> > - i2c->msg.len == 0)
> > - return ISR_WAITSTOP;
> > + if (i2c->msg.len == 0) {
> > + if (i2c->last_msg)
> > + return ISR_WAITSTOP;
> > + return ISR_COMPLETE(0);
> > + }
>
> Again, already happens in img_i2c_isr().
>
Will drop this patch from v2.
Sifan
> Cheers
> James
>
> > img_i2c_write_fifo(i2c);
> > }
> > }
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/7] i2c: img-scb: mark transaction as complete when all data is read
Hi James,
>
> On 27/07/15 12:55, Sifan Naeem wrote:
> > We can mark the transfer as complete without waiting for the stop bit.
> > This is important when handling repeated start transfers as we have to
> > start the next transfer without the stop bit being issued.
> >
> > Signed-off-by: Sifan Naeem
> > ---
> > drivers/i2c/busses/i2c-img-scb.c |7 +--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c
> > b/drivers/i2c/busses/i2c-img-scb.c
> > index 24b09fe..e27c3e0 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -903,8 +903,11 @@ static unsigned int img_i2c_auto(struct img_i2c
> *i2c,
> > if (i2c->msg.flags & I2C_M_RD) {
> > if (int_status & INT_FIFO_FULL_FILLING) {
> > img_i2c_read_fifo(i2c);
> > - if (i2c->msg.len == 0)
> > - return ISR_WAITSTOP;
> > + if (i2c->msg.len == 0) {
> > + if (i2c->last_msg)
> > + return ISR_WAITSTOP;
> > + return ISR_COMPLETE(0);
>
> That already happens in img_i2c_isr().
>
Will drop this patch from v2.
Sifan
> Cheers
> James
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/8] i2c: img-scb: improve transaction complete handle
Hi James,
> -Original Message-
> From: James Hogan
> Sent: 29 July 2015 13:22
> To: Sifan Naeem; Wolfram Sang; linux-i2c@vger.kernel.org
> Cc: Stable kernel (v3.19+)
> Subject: Re: [PATCH 7/8] i2c: img-scb: improve transaction complete handle
>
> Hi Sifan,
>
> On 27/07/15 12:47, Sifan Naeem wrote:
> > Clear line status and all interrupts when transaction is complete, as
> > not doing so might leave unserviced interrupts that might be
>
> Do you have a specific example of when this might happen, and whether it
> could occur after img_i2c_complete_transaction()?
>
> I'm just wondering if it would be better to do this before starting every new
> message instead of after handling the last irq that is of interest (maybe
> somewhere in img_i2c_xfer).
>
Moved to happen before each transfer.
> > handled in the context of a new transfer. Soft reset if the the
> > transfer failed to bring back the i2c block to a reset state.
> >
> > Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB
> > driver")
> > Signed-off-by: Sifan Naeem
> > Cc: Stable kernel (v3.19+)
> > ---
> > drivers/i2c/busses/i2c-img-scb.c |5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c
> > b/drivers/i2c/busses/i2c-img-scb.c
> > index 341130e..bbfee33 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -626,7 +626,10 @@ static void img_i2c_complete_transaction(struct
> img_i2c *i2c, int status)
> > img_i2c_switch_mode(i2c, MODE_INACTIVE);
> > if (status) {
> > i2c->msg_status = status;
> > - img_i2c_transaction_halt(i2c, false);
> > + img_i2c_soft_reset(i2c);
>
> This seems like overkill. This will only happen in a couple of cases:
> (1) an automatic mode ack error, which is completely recoverable.
> (2) a fatal error (clock low timeout), which switches mode to MODE_FATAL
> anyway, preventing further transactions.
>
Removed.
Thanks,
Sifan
> Cheers
> James
>
> > + } else {
> > + img_i2c_writel(i2c, SCB_INT_CLEAR_REG, ~0);
> > + img_i2c_writel(i2c, SCB_CLEAR_REG, ~0);
> > }
> > complete(&i2c->msg_complete);
> > }
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 8/8] i2c: img-scb: verify support for requested bit rate
Hi James,
> On 27/07/15 12:47, Sifan Naeem wrote:
> > The requested bit rate can be outside the range supported by the driver.
> > The maximum bit rate this driver supports at the moment is 400Khz.
> >
> > Return -EINVAL if the bit rate is larger than 400khz.
> >
> > Maximum speed supported by the driver can be increased to 1Mhz by
> > adding support for "fast plus mode" in the future.
> >
> > Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB
> > driver")
> > Signed-off-by: Sifan Naeem
> > Cc: Stable kernel (v3.19+)
> > ---
> > drivers/i2c/busses/i2c-img-scb.c |6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c
> > b/drivers/i2c/busses/i2c-img-scb.c
> > index bbfee33..07a039c 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -1157,6 +1157,12 @@ static int img_i2c_init(struct img_i2c *i2c)
> > break;
> > }
> > }
> > + if (i2c->bitrate > timing.max_bitrate) {
> > + dev_err(i2c->adap.dev.parent,
> > + "requested bitrate (%d) is higher than the max
> bitrate supported (%d)\n",
> > +i2c->bitrate, timing.max_bitrate);
> > + return -EINVAL;
> > + }
>
> The timing is only chosen if i2c->bitrate <= timing.max_bitrate, so you'd only
> hit this case if none of the timings were valid, in which case timing ==
> timings[0], so when you print timing.max_bitrate it won't be the max bitrate
> supported, it'll be the max bitrate of the first timing in the array.
>
> Anyway, I think the original intent of the DT provided clock-frequency was
> more as a maximum bitrate. This was used with TZ1090 as a way to limit the
> bitrate of the bus if some devices on the bus don't support the full speed,
> e.g. we had an HDMI chip that would get confused at 400khz.
>
> So would it be acceptable to change it to just clamp the bitrate to the
> maximum rate supported, before the bitrate_khz calculation, such that if you
> specified 1MHz in DT, it could safely fall back to 400KHz until the driver
> supports the faster mode?
>
I'll rework this, when the requested bit rate is larger than the maximum
supported, set the bitrate down to the maximum supported before bitrate_khz is
calculated.
Thanks,
Sifan
> Cheers
> James
>
> >
> > /* Find the prescale that would give us that inc (approx delay = 0) */
> > prescale = SCB_OPT_INC * clk_khz / (256 * 16 * bitrate_khz);
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 6/8] i2c: img-scb: remove start bit detected status after handling
> -Original Message-
> From: James Hogan
> Sent: 28 July 2015 14:53
> To: Sifan Naeem; Wolfram Sang; linux-i2c@vger.kernel.org
> Cc: Stable kernel (v3.19+)
> Subject: Re: [PATCH 6/8] i2c: img-scb: remove start bit detected status after
> handling
>
> On 27/07/15 12:47, Sifan Naeem wrote:
> > Remove start bit detected status after it is handled, doing so will
> > prevent this condition being hit for every interrupt on a particular
> > transfer.
> >
> > Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB
> > driver")
> > Signed-off-by: Sifan Naeem
> > Cc: Stable kernel (v3.19+)
> > ---
> > drivers/i2c/busses/i2c-img-scb.c | 16
> > 1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c
> > b/drivers/i2c/busses/i2c-img-scb.c
> > index 653f9bd..341130e 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -871,10 +871,18 @@ static unsigned int img_i2c_auto(struct img_i2c
> *i2c,
> > }
> >
> > /* Enable transaction halt on start bit */
> > - if (!i2c->last_msg && i2c->line_status & LINESTAT_START_BIT_DET) {
>
> i2c->line_status accumulates the line status bits that have been seen
> with each interrupt. If we're only interested in that bit from the current
> interrupt, should it just be referring to line_status (the argument to
> img_i2c_auto) instead of i2c->line_status?
>
Yes, I can't think of why we cannot use line_status from the argument.
Thanks,
Sifan
> Cheers
> James
>
> > - img_i2c_transaction_halt(i2c, true);
> > - /* we're no longer interested in the slave event */
> > - i2c->int_enable &= ~INT_SLAVE_EVENT;
> > + if (i2c->line_status & LINESTAT_START_BIT_DET) {
> > + if (!i2c->last_msg) {
> > + img_i2c_transaction_halt(i2c, true);
> > + /* we're no longer interested in the slave event */
> > + i2c->int_enable &= ~INT_SLAVE_EVENT;
> > + }
> > + /*
> > +* Remove start bit detected status after it is handled,
> > +* doing so will prevent this condition being hit for
> > +* every interrupt on a particular transfer.
> > +*/
> > + i2c->line_status &= ~LINESTAT_START_BIT_DET;
> > }
> >
> > mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 5/8] i2c: img-scb: reset interrupts in img_i2c_soft_reset
Hi James,
> -Original Message-
> From: James Hogan
> Sent: 28 July 2015 12:36
> To: Sifan Naeem; Wolfram Sang; linux-i2c@vger.kernel.org
> Cc: Stable kernel (v3.19+)
> Subject: Re: [PATCH 5/8] i2c: img-scb: reset interrupts in img_i2c_soft_reset
>
> On 27/07/15 12:47, Sifan Naeem wrote:
> > Reset interrupt enable register and clear any generated interrupts to
> > make sure of a clean slate after a soft reset. Not doing so might
> > leave unhandle line status or generated interrupts which can cause
> > issues when handling new transfers.
>
> That already happens after the call to img_i2c_soft_reset, you've just moved
> it to while the clock is off and the block is in soft reset. So are you
> saying its
> important for interrupts and the various event bits to be clear before taking
> it
> out of reset and enabling the clock? TBH I'm confused how they could do any
> harm.
>
In " [PATCH 7/8] i2c: img-scb: improve transaction complete handle "
img_i2c_soft_reset is added to img_i2c_complete_transaction, this will be
useful in this scenario, not necessarily for when initialising the block.
Thanks
Sifan
> Cheers
> James
>
> >
> > Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB
> > driver")
> > Signed-off-by: Sifan Naeem
> > Cc: Stable kernel (v3.19+)
> > ---
> > drivers/i2c/busses/i2c-img-scb.c | 26 ++
> > 1 file changed, 14 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c
> > b/drivers/i2c/busses/i2c-img-scb.c
> > index 51a5be8..653f9bd 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -507,8 +507,22 @@ static void img_i2c_soft_reset(struct img_i2c
> > *i2c) {
> > i2c->t_halt = false;
> > img_i2c_writel(i2c, SCB_CONTROL_REG, 0);
> > +
> > + /* Disable all interrupts */
> > + img_i2c_writel(i2c, SCB_INT_MASK_REG, 0);
> > +
> > + /* Clear all interrupts */
> > + img_i2c_writel(i2c, SCB_INT_CLEAR_REG, ~0);
> > +
> > + /* Clear the scb_line_status events */
> > + img_i2c_writel(i2c, SCB_CLEAR_REG, ~0);
> > +
> > img_i2c_writel(i2c, SCB_CONTROL_REG,
> >SCB_CONTROL_CLK_ENABLE |
> SCB_CONTROL_SOFT_RESET);
> > +
> > + /* Enable interrupts */
> > + img_i2c_switch_mode(i2c, MODE_INACTIVE);
> > + img_i2c_writel(i2c, SCB_INT_MASK_REG, i2c->int_enable);
> > }
> >
> > /* enable or release transaction halt for control of repeated starts
> > */ @@ -1242,18 +1256,6 @@ static int img_i2c_init(struct img_i2c *i2c)
> > /* Take module out of soft reset and enable clocks */
> > img_i2c_soft_reset(i2c);
> >
> > - /* Disable all interrupts */
> > - img_i2c_writel(i2c, SCB_INT_MASK_REG, 0);
> > -
> > - /* Clear all interrupts */
> > - img_i2c_writel(i2c, SCB_INT_CLEAR_REG, ~0);
> > -
> > - /* Clear the scb_line_status events */
> > - img_i2c_writel(i2c, SCB_CLEAR_REG, ~0);
> > -
> > - /* Enable interrupts */
> > - img_i2c_writel(i2c, SCB_INT_MASK_REG, i2c->int_enable);
> > -
> > /* Perform a synchronous sequence to reset the bus */
> > ret = img_i2c_reset_bus(i2c);
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/8] i2c: img-scb: enable fencing for all versions of the ip
Hi James,
> -Original Message-
> From: James Hogan
> Sent: 27 July 2015 21:21
> To: Sifan Naeem
> Cc: Wolfram Sang; linux-i2c@vger.kernel.org; Stable kernel (v3.19+)
> Subject: Re: [PATCH 1/8] i2c: img-scb: enable fencing for all versions of the
> ip
>
> Hi Sifan,
>
> On Mon, Jul 27, 2015 at 12:47:14PM +0100, Sifan Naeem wrote:
> > The code to read from the master read fifo, and write to the master
> > write fifo, checks a bit in an SCB register before every byte to
> > ensure that the fifo is not full (write fifo) or empty (read fifo).
> > Due to clock domain crossing inside the SCB block the updated value of
> > this bit is only visible after 2 cycles.
> >
> > The scb_wr_rd_fence() function does 2 dummy writes (to the read-only
> > revision register), and it's called before reading from or writing to
> > the fifos to ensure that subsequent reads of the fifo status bits do
> > not read stale values.
> >
> > As the 2 dummy writes are required in all versions of the ip, the
> > version check is dropped.
>
> Is it anticipated that a future version of the hardware will probably resolve
> the clock domain crossing issue? If so fine, but if not its probably worth
> removing need_wr_rd_fence.
>
Yes, it's expected to be fixed in the future, albeit not in the near future.
> >
> > Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB
> > driver")
>
> I believe 12 digits of SHA1 is recommended now, to avoid collisions. I suggest
> doing this:
> $ git config --global core.abbrev 12
>
Should I send a new patch with 12 digit SHA1?
> > Signed-off-by: Sifan Naeem
> > Cc: Stable kernel (v3.19+)
>
> That's a fairly non-conventional way to specify stable versions. The
> recommended way to Cc stable according to
> Documentation/stable_kernel_rules.txt is more like this:
> Cc: # 3.19.x-
>
I go this error when doing that way:
(body) Adding cc: Sifan Naeem from line
'Signed-off-by: Sifan Naeem '
(body) Adding cc: # 4.1 from line 'Cc:
# 4.1'
Use of uninitialized value $cc in string eq at /usr/lib/git-core/git-send-email
line 983.
Use of uninitialized value $cc in quotemeta at /usr/lib/git-core/git-send-email
line 983.
W: unable to extract a valid address from: # 4.1
W: unable to extract a valid address from: # 4.1
> Patch looks fine though
>
> Acked-by: James Hogan
>
Thanks,
Sifan
> Thanks
> James
>
> > ---
> > drivers/i2c/busses/i2c-img-scb.c |8 ++--
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c
> > b/drivers/i2c/busses/i2c-img-scb.c
> > index 00ffd66..5c3c615 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -278,8 +278,6 @@
> > #define ISR_COMPLETE(err) (ISR_COMPLETE_M | (ISR_STATUS_M &
> (err)))
> > #define ISR_FATAL(err) (ISR_COMPLETE(err) | ISR_FATAL_M)
> >
> > -#define REL_SOC_IP_SCB_2_2_1 0x00020201
> > -
> > enum img_i2c_mode {
> > MODE_INACTIVE,
> > MODE_RAW,
> > @@ -1120,10 +1118,8 @@ static int img_i2c_init(struct img_i2c *i2c)
> > return -EINVAL;
> > }
> >
> > - if (rev == REL_SOC_IP_SCB_2_2_1) {
> > - i2c->need_wr_rd_fence = true;
> > - dev_info(i2c->adap.dev.parent, "fence quirk enabled");
> > - }
> > + /* Fencing enabled by default. */
> > + i2c->need_wr_rd_fence = true;
> >
> > bitrate_khz = i2c->bitrate / 1000;
> > clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
> > --
> > 1.7.9.5
> >
[PATCH 4/7] i2c: img-scb: mark transaction as complete when no more data to write
We can mark the transfer as complete without waiting for the stop
bit. This is important when handling repeated start transfers as
we have to start the next transfer without the stop bit being issued.
This doesn't affect the older versions of the IP.
Signed-off-by: Sifan Naeem
---
drivers/i2c/busses/i2c-img-scb.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index e27c3e0..efad4d7 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -911,16 +911,11 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
}
} else {
if (int_status & INT_FIFO_EMPTY_EMPTYING) {
- /*
-* The write fifo empty indicates that we're in the
-* last byte so it's safe to start a new write
-* transaction without losing any bytes from the
-* previous one.
-* see 2.3.7 Repeated Start Transactions.
-*/
- if ((int_status & INT_FIFO_EMPTY) &&
- i2c->msg.len == 0)
- return ISR_WAITSTOP;
+ if (i2c->msg.len == 0) {
+ if (i2c->last_msg)
+ return ISR_WAITSTOP;
+ return ISR_COMPLETE(0);
+ }
img_i2c_write_fifo(i2c);
}
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/7] i2c: img-scb: mark transaction as complete when all data is read
We can mark the transfer as complete without waiting for the stop
bit. This is important when handling repeated start transfers as
we have to start the next transfer without the stop bit being issued.
Signed-off-by: Sifan Naeem
---
drivers/i2c/busses/i2c-img-scb.c |7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 24b09fe..e27c3e0 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -903,8 +903,11 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
if (i2c->msg.flags & I2C_M_RD) {
if (int_status & INT_FIFO_FULL_FILLING) {
img_i2c_read_fifo(i2c);
- if (i2c->msg.len == 0)
- return ISR_WAITSTOP;
+ if (i2c->msg.len == 0) {
+ if (i2c->last_msg)
+ return ISR_WAITSTOP;
+ return ISR_COMPLETE(0);
+ }
}
} else {
if (int_status & INT_FIFO_EMPTY_EMPTYING) {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/7] i2c: img-scb: add handle for Master halt interrupt
Master halt is issued after each byte of a transaction is processed in
IP version 3.3.
Master halt will stall the bus by holding the SCK line low until the
halt bit in the scb_general_control is cleared.
After the last byte of a transfer is processed we can use the Master
Halt interrupt to facilitate a repeated start transfer without
issuing a stop bit.
Signed-off-by: Sifan Naeem
---
drivers/i2c/busses/i2c-img-scb.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 90faf48..df3d25a 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -151,6 +151,7 @@
#define INT_FIFO_EMPTYING BIT(12)
#define INT_TRANSACTION_DONE BIT(15)
#define INT_SLAVE_EVENTBIT(16)
+#define INT_MASTER_HALTED BIT(17)
#define INT_TIMING BIT(18)
#define INT_STOP_DETECTED BIT(19)
@@ -177,6 +178,7 @@
INT_FIFO_FULL| \
INT_FIFO_FILLING | \
INT_FIFO_EMPTY | \
+INT_MASTER_HALTED| \
INT_STOP_DETECTED)
#define INT_ENABLE_MASK_WAITSTOP (INT_SLAVE_EVENT | \
@@ -901,6 +903,17 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
if (i2c->msg.flags & I2C_M_RD) {
+ if (int_status & INT_MASTER_HALTED) {
+ img_i2c_read_fifo(i2c);
+ if (i2c->msg.len == 0)
+ return ISR_COMPLETE(0);
+ /*
+* Release and then enable transaction halt, to
+* allow only a single byte to proceed.
+*/
+ img_i2c_transaction_halt(i2c, false);
+ img_i2c_transaction_halt(i2c, !i2c->last_msg);
+ }
if (int_status & INT_FIFO_FULL_FILLING) {
img_i2c_read_fifo(i2c);
if (i2c->msg.len == 0) {
@@ -922,6 +935,18 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
return ISR_COMPLETE(ret);
}
} else {
+ if (int_status & INT_MASTER_HALTED) {
+ if ((int_status & INT_FIFO_EMPTY) &&
+ i2c->msg.len == 0)
+ return ISR_COMPLETE(0);
+ img_i2c_write_fifo(i2c);
+ /*
+* Release and then enable transaction halt, to
+* allow only a single byte to proceed.
+*/
+ img_i2c_transaction_halt(i2c, false);
+ img_i2c_transaction_halt(i2c, !i2c->last_msg);
+ }
if (int_status & INT_FIFO_EMPTY) {
if (i2c->msg.len == 0) {
if (i2c->last_msg)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/7] i2c: img-scb: support I2C_M_IGNORE_NAK
This commit adds support for the I2C_M_IGNORE_NAK protocol
modification.
Such behaviour can only be implemented in atomic mode. So, if a
transaction contains a message with such flag the drivers
switches to atomic mode. The implementation consists simply in
treating NAKs as ACKs.
Signed-off-by: Sifan Naeem
---
drivers/i2c/busses/i2c-img-scb.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 07a039c..31cd8c3 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -767,7 +767,9 @@ static unsigned int img_i2c_atomic(struct img_i2c *i2c,
next_cmd = CMD_RET_ACK;
break;
case CMD_RET_ACK:
- if (i2c->line_status & LINESTAT_ACK_DET) {
+ if (i2c->line_status & LINESTAT_ACK_DET ||
+ (i2c->line_status & LINESTAT_NACK_DET &&
+ i2c->msg.flags & I2C_M_IGNORE_NAK)) {
if (i2c->msg.len == 0) {
next_cmd = CMD_GEN_STOP;
} else if (i2c->msg.flags & I2C_M_RD) {
@@ -1042,20 +1044,23 @@ static int img_i2c_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs,
return -EIO;
for (i = 0; i < num; i++) {
- if (likely(msgs[i].len))
- continue;
/*
* 0 byte reads are not possible because the slave could try
* and pull the data line low, preventing a stop bit.
*/
- if (unlikely(msgs[i].flags & I2C_M_RD))
+ if (!msgs[i].len && msgs[i].flags & I2C_M_RD)
return -EIO;
/*
* 0 byte writes are possible and used for probing, but we
* cannot do them in automatic mode, so use atomic mode
* instead.
+*
+* Also, the I2C_M_IGNORE_NAK mode can only be implemented
+* in atomic mode.
*/
- atomic = true;
+ if (!msgs[i].len ||
+ (msgs[i].flags & I2C_M_IGNORE_NAK))
+ atomic = true;
}
ret = clk_prepare_enable(i2c->scb_clk);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/7] i2c: img-scb: remove fifo EMPTYING interrupts handle
This interrupt could have been useful for repeated start transfers
as the current transfer could be marked as complete while it's
processing the final byte of the transfer.
But having to use the transaction halt interrupt to safely control
repeated start transfers, means handling of the fifo EMPTYING
interrupts is no longer necessary.
Handling this interrupt along with Transaction Halt interrupt can
cause erratic behaviour.
Signed-off-by: Sifan Naeem
---
drivers/i2c/busses/i2c-img-scb.c |6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index efad4d7..10141a9 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -154,7 +154,6 @@
#define INT_TIMING BIT(18)
#define INT_FIFO_FULL_FILLING (INT_FIFO_FULL | INT_FIFO_FILLING)
-#define INT_FIFO_EMPTY_EMPTYING(INT_FIFO_EMPTY | INT_FIFO_EMPTYING)
/* Level interrupts need clearing after handling instead of before */
#define INT_LEVEL 0x01e00
@@ -176,8 +175,7 @@
INT_WRITE_ACK_ERR| \
INT_FIFO_FULL| \
INT_FIFO_FILLING | \
-INT_FIFO_EMPTY | \
-INT_FIFO_EMPTYING)
+INT_FIFO_EMPTY)
#define INT_ENABLE_MASK_WAITSTOP (INT_SLAVE_EVENT | \
INT_ADDR_ACK_ERR | \
@@ -910,7 +908,7 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
}
}
} else {
- if (int_status & INT_FIFO_EMPTY_EMPTYING) {
+ if (int_status & INT_FIFO_EMPTY) {
if (i2c->msg.len == 0) {
if (i2c->last_msg)
return ISR_WAITSTOP;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/7] i2c: img-scb: enchancements to support i2c on pistachio
Following patches are required to enchance the existing driver to support i2c on pistachio. This patch series depends on the series of fixes posted earlier[1]. The features added in this series were tested with the earlier fixes series. Tested on Pistachio bub and on tz1090 using an Adafruit I2C Non-Volatile FRAM Breakout (256Kbit / 32KByte) eeprom. Used i2c buildroot tools to test the eeprom and the other i2c blocks. Also used dd commands to copy data to and then to dump data from the eeprom. i2ctransfer was used to test repeated starts and verified using a scope. [1]: http://marc.info/?l=linux-i2c&m=143799753022541&w=2 Sifan Naeem (7): i2c: img-scb: support I2C_M_IGNORE_NAK i2c: img-scb: support repeated starts on IP v3.3 i2c: img-scb: mark transaction as complete when all data is read i2c: img-scb: mark transaction as complete when no more data to write i2c: img-scb: remove fifo EMPTYING interrupts handle i2c: img-scb: add handle for stop detected interrupt i2c: img-scb: add handle for Master halt interrupt drivers/i2c/busses/i2c-img-scb.c | 129 ++ 1 file changed, 104 insertions(+), 25 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/7] i2c: img-scb: add handle for stop detected interrupt
Stop Detected interrupt is triggered when a Stop bit is detected on
the bus, which indicates the end of the current transfer.
When the end of a transfer is indicated by the Stop bit interrupt,
drain the FIFO and signal completion for the transaction. But if the
interrupt was triggered before all data is written to the fifo or with
more data expected return error with transfer complete signal.
Halting the bus is no longer necessary after a stop bit is detected
on the bus, as there cannot be a repeated start transfer when the stop
bit has been issued, hence remove the transaction halt bit.
Signed-off-by: Sifan Naeem
---
drivers/i2c/busses/i2c-img-scb.c | 28 +++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 10141a9..90faf48 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -152,6 +152,7 @@
#define INT_TRANSACTION_DONE BIT(15)
#define INT_SLAVE_EVENTBIT(16)
#define INT_TIMING BIT(18)
+#define INT_STOP_DETECTED BIT(19)
#define INT_FIFO_FULL_FILLING (INT_FIFO_FULL | INT_FIFO_FILLING)
@@ -175,7 +176,8 @@
INT_WRITE_ACK_ERR| \
INT_FIFO_FULL| \
INT_FIFO_FILLING | \
-INT_FIFO_EMPTY)
+INT_FIFO_EMPTY | \
+INT_STOP_DETECTED)
#define INT_ENABLE_MASK_WAITSTOP (INT_SLAVE_EVENT | \
INT_ADDR_ACK_ERR | \
@@ -907,6 +909,18 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
return ISR_COMPLETE(0);
}
}
+ if (int_status & INT_STOP_DETECTED) {
+ int ret;
+ /*
+* Stop bit indicates the end of the transfer, it means
+* we should read all the data (or drain the FIFO). We
+* must signal completion for this transaction.
+*/
+ img_i2c_transaction_halt(i2c, false);
+ img_i2c_read_fifo(i2c);
+ ret = (i2c->msg.len == 0) ? 0 : EIO;
+ return ISR_COMPLETE(ret);
+ }
} else {
if (int_status & INT_FIFO_EMPTY) {
if (i2c->msg.len == 0) {
@@ -916,6 +930,18 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
}
img_i2c_write_fifo(i2c);
}
+ if (int_status & INT_STOP_DETECTED) {
+ int ret;
+
+ img_i2c_transaction_halt(i2c, false);
+ /*
+* Stop bit indicates the end of a transfer and if the
+* transfer has ended before all data is written to the
+* fifo, return an error with transfer complete signal.
+*/
+ ret = (i2c->msg.len == 0) ? 0 : EIO;
+ return ISR_COMPLETE(ret);
+ }
}
return 0;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/7] i2c: img-scb: support repeated starts on IP v3.3
In version 3.3 of the IP when transaction halt is set, an interrupt
will be generated after each byte of a transfer instead of after
every transfer but before the stop bit.
Due to this behaviour we have to be careful that every time we
release the transaction halt we have to re-enable it straight away
so that we only process a single byte, not doing so will result in
all remaining bytes been processed and a stop bit being issued,
which will prevent us having a repeated start.
This change will have no effect on earlier versions of the IP.
Signed-off-by: Sifan Naeem
---
drivers/i2c/busses/i2c-img-scb.c | 43 +++---
1 file changed, 35 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 31cd8c3..24b09fe 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -525,7 +525,17 @@ static void img_i2c_soft_reset(struct img_i2c *i2c)
img_i2c_writel(i2c, SCB_INT_MASK_REG, i2c->int_enable);
}
-/* enable or release transaction halt for control of repeated starts */
+/*
+ * Enable or release transaction halt for control of repeated starts.
+ * In version 3.3 of the IP when transaction halt is set, an interrupt
+ * will be generated after each byte of a transfer instead of after
+ * every transfer but before the stop bit.
+ * Due to this behaviour we have to be careful that every time we
+ * release the transaction halt we have to re-enable it straight away
+ * so that we only process a single byte, not doing so will result in
+ * all remaining bytes been processed and a stop bit being issued,
+ * which will prevent us having a repeated start.
+ */
static void img_i2c_transaction_halt(struct img_i2c *i2c, bool t_halt)
{
u32 val;
@@ -594,7 +604,6 @@ static void img_i2c_read(struct img_i2c *i2c)
img_i2c_writel(i2c, SCB_READ_ADDR_REG, i2c->msg.addr);
img_i2c_writel(i2c, SCB_READ_COUNT_REG, i2c->msg.len);
- img_i2c_transaction_halt(i2c, false);
mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
}
@@ -608,7 +617,6 @@ static void img_i2c_write(struct img_i2c *i2c)
img_i2c_writel(i2c, SCB_WRITE_ADDR_REG, i2c->msg.addr);
img_i2c_writel(i2c, SCB_WRITE_COUNT_REG, i2c->msg.len);
- img_i2c_transaction_halt(i2c, false);
mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
img_i2c_write_fifo(i2c);
@@ -1090,12 +1098,31 @@ static int img_i2c_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs,
i2c->last_msg = (i == num - 1);
reinit_completion(&i2c->msg_complete);
- if (atomic)
+ if (atomic) {
img_i2c_atomic_start(i2c);
- else if (msg->flags & I2C_M_RD)
- img_i2c_read(i2c);
- else
- img_i2c_write(i2c);
+ } else {
+ /*
+* Enable transaction halt if not the last message in
+* the queue so that we can control repeated starts.
+*/
+ img_i2c_transaction_halt(i2c, !i2c->last_msg);
+
+ if (msg->flags & I2C_M_RD)
+ img_i2c_read(i2c);
+ else
+ img_i2c_write(i2c);
+
+ /*
+* Release and then enable transaction halt, to
+* allow only a single byte to proceed.
+* This doesn't have an effect on the initial transfer
+* but will allow the following transfers to start
+* processing if the previous transfer was marked as
+* complete while the i2c block was halted.
+*/
+ img_i2c_transaction_halt(i2c, false);
+ img_i2c_transaction_halt(i2c, !i2c->last_msg);
+ }
spin_unlock_irqrestore(&i2c->lock, flags);
time_left = wait_for_completion_timeout(&i2c->msg_complete,
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/8] i2c: img-scb: use DIV_ROUND_UP to round divisor values
Using % can be slow depending on the architecture.
Using DIV_ROUND_UP is nicer and more efficient way to do it.
Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB driver")
Signed-off-by: Sifan Naeem
Cc: Stable kernel (v3.19+)
---
drivers/i2c/busses/i2c-img-scb.c |8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 0368d91..b4f59e1 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -1179,9 +1179,7 @@ static int img_i2c_init(struct img_i2c *i2c)
int_bitrate++;
/* Setup TCKH value */
- tckh = timing.tckh / clk_period;
- if (timing.tckh % clk_period)
- tckh++;
+ tckh = DIV_ROUND_UP(timing.tckh, clk_period);
if (tckh > 0)
data = tckh - 1;
@@ -1201,9 +1199,7 @@ static int img_i2c_init(struct img_i2c *i2c)
img_i2c_writel(i2c, SCB_TIME_TCKL_REG, data);
/* Setup TSDH value */
- tsdh = timing.tsdh / clk_period;
- if (timing.tsdh % clk_period)
- tsdh++;
+ tsdh = DIV_ROUND_UP(timing.tsdh, clk_period);
if (tsdh > 1)
data = tsdh - 1;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/8] i2c: img-scb: fixes to support i2c on pistachio
Following patches are required to fix the existing driver to support i2c on pistachio. Tested on Pistachio bub using an Adafruit I2C Non-Volatile FRAM Breakout (256Kbit / 32KByte) eeprom. Used i2c buildroot tools to test the eeprom and the other i2c blocks. Also used dd commands to copy data to and then to dump data from the eeprom. i2ctransfer was used to test repeated starts and verified using a scope. Cc: Stable kernel (v3.19+) Sifan Naeem (8): i2c: img-scb: enable fencing for all versions of the ip i2c: img-scb: do dummy writes before fifo access i2c: img-scb: use DIV_ROUND_UP to round divisor values i2c: img-scb: fix LOW and HIGH period values for the SCL clock i2c: img-scb: reset interrupts in img_i2c_soft_reset i2c: img-scb: remove start bit detected status after handling i2c: img-scb: improve transaction complete handle i2c: img-scb: verify support for requested bit rate drivers/i2c/busses/i2c-img-scb.c | 101 ++ 1 file changed, 58 insertions(+), 43 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8] i2c: img-scb: remove start bit detected status after handling
Remove start bit detected status after it is handled,
doing so will prevent this condition being hit for
every interrupt on a particular transfer.
Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB driver")
Signed-off-by: Sifan Naeem
Cc: Stable kernel (v3.19+)
---
drivers/i2c/busses/i2c-img-scb.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 653f9bd..341130e 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -871,10 +871,18 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
}
/* Enable transaction halt on start bit */
- if (!i2c->last_msg && i2c->line_status & LINESTAT_START_BIT_DET) {
- img_i2c_transaction_halt(i2c, true);
- /* we're no longer interested in the slave event */
- i2c->int_enable &= ~INT_SLAVE_EVENT;
+ if (i2c->line_status & LINESTAT_START_BIT_DET) {
+ if (!i2c->last_msg) {
+ img_i2c_transaction_halt(i2c, true);
+ /* we're no longer interested in the slave event */
+ i2c->int_enable &= ~INT_SLAVE_EVENT;
+ }
+ /*
+* Remove start bit detected status after it is handled,
+* doing so will prevent this condition being hit for
+* every interrupt on a particular transfer.
+*/
+ i2c->line_status &= ~LINESTAT_START_BIT_DET;
}
mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/8] i2c: img-scb: verify support for requested bit rate
The requested bit rate can be outside the range supported by the driver.
The maximum bit rate this driver supports at the moment is 400Khz.
Return -EINVAL if the bit rate is larger than 400khz.
Maximum speed supported by the driver can be increased to 1Mhz by
adding support for "fast plus mode" in the future.
Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB driver")
Signed-off-by: Sifan Naeem
Cc: Stable kernel (v3.19+)
---
drivers/i2c/busses/i2c-img-scb.c |6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index bbfee33..07a039c 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -1157,6 +1157,12 @@ static int img_i2c_init(struct img_i2c *i2c)
break;
}
}
+ if (i2c->bitrate > timing.max_bitrate) {
+ dev_err(i2c->adap.dev.parent,
+ "requested bitrate (%d) is higher than the max bitrate
supported (%d)\n",
+i2c->bitrate, timing.max_bitrate);
+ return -EINVAL;
+ }
/* Find the prescale that would give us that inc (approx delay = 0) */
prescale = SCB_OPT_INC * clk_khz / (256 * 16 * bitrate_khz);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/8] i2c: img-scb: improve transaction complete handle
Clear line status and all interrupts when transaction is complete,
as not doing so might leave unserviced interrupts that might be
handled in the context of a new transfer. Soft reset if the the
transfer failed to bring back the i2c block to a reset state.
Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB driver")
Signed-off-by: Sifan Naeem
Cc: Stable kernel (v3.19+)
---
drivers/i2c/busses/i2c-img-scb.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 341130e..bbfee33 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -626,7 +626,10 @@ static void img_i2c_complete_transaction(struct img_i2c
*i2c, int status)
img_i2c_switch_mode(i2c, MODE_INACTIVE);
if (status) {
i2c->msg_status = status;
- img_i2c_transaction_halt(i2c, false);
+ img_i2c_soft_reset(i2c);
+ } else {
+ img_i2c_writel(i2c, SCB_INT_CLEAR_REG, ~0);
+ img_i2c_writel(i2c, SCB_CLEAR_REG, ~0);
}
complete(&i2c->msg_complete);
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/8] i2c: img-scb: enable fencing for all versions of the ip
The code to read from the master read fifo, and write to the master
write fifo, checks a bit in an SCB register before every byte to
ensure that the fifo is not full (write fifo) or empty (read fifo).
Due to clock domain crossing inside the SCB block the updated value
of this bit is only visible after 2 cycles.
The scb_wr_rd_fence() function does 2 dummy writes (to the read-only
revision register), and it's called before reading from or writing to the
fifos to ensure that subsequent reads of the fifo status bits do not read
stale values.
As the 2 dummy writes are required in all versions of the ip, the version
check is dropped.
Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB driver")
Signed-off-by: Sifan Naeem
Cc: Stable kernel (v3.19+)
---
drivers/i2c/busses/i2c-img-scb.c |8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 00ffd66..5c3c615 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -278,8 +278,6 @@
#define ISR_COMPLETE(err) (ISR_COMPLETE_M | (ISR_STATUS_M & (err)))
#define ISR_FATAL(err) (ISR_COMPLETE(err) | ISR_FATAL_M)
-#define REL_SOC_IP_SCB_2_2_1 0x00020201
-
enum img_i2c_mode {
MODE_INACTIVE,
MODE_RAW,
@@ -1120,10 +1118,8 @@ static int img_i2c_init(struct img_i2c *i2c)
return -EINVAL;
}
- if (rev == REL_SOC_IP_SCB_2_2_1) {
- i2c->need_wr_rd_fence = true;
- dev_info(i2c->adap.dev.parent, "fence quirk enabled");
- }
+ /* Fencing enabled by default. */
+ i2c->need_wr_rd_fence = true;
bitrate_khz = i2c->bitrate / 1000;
clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/8] i2c: img-scb: reset interrupts in img_i2c_soft_reset
Reset interrupt enable register and clear any generated interrupts
to make sure of a clean slate after a soft reset. Not doing so might
leave unhandle line status or generated interrupts which can cause
issues when handling new transfers.
Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB driver")
Signed-off-by: Sifan Naeem
Cc: Stable kernel (v3.19+)
---
drivers/i2c/busses/i2c-img-scb.c | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 51a5be8..653f9bd 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -507,8 +507,22 @@ static void img_i2c_soft_reset(struct img_i2c *i2c)
{
i2c->t_halt = false;
img_i2c_writel(i2c, SCB_CONTROL_REG, 0);
+
+ /* Disable all interrupts */
+ img_i2c_writel(i2c, SCB_INT_MASK_REG, 0);
+
+ /* Clear all interrupts */
+ img_i2c_writel(i2c, SCB_INT_CLEAR_REG, ~0);
+
+ /* Clear the scb_line_status events */
+ img_i2c_writel(i2c, SCB_CLEAR_REG, ~0);
+
img_i2c_writel(i2c, SCB_CONTROL_REG,
SCB_CONTROL_CLK_ENABLE | SCB_CONTROL_SOFT_RESET);
+
+ /* Enable interrupts */
+ img_i2c_switch_mode(i2c, MODE_INACTIVE);
+ img_i2c_writel(i2c, SCB_INT_MASK_REG, i2c->int_enable);
}
/* enable or release transaction halt for control of repeated starts */
@@ -1242,18 +1256,6 @@ static int img_i2c_init(struct img_i2c *i2c)
/* Take module out of soft reset and enable clocks */
img_i2c_soft_reset(i2c);
- /* Disable all interrupts */
- img_i2c_writel(i2c, SCB_INT_MASK_REG, 0);
-
- /* Clear all interrupts */
- img_i2c_writel(i2c, SCB_INT_CLEAR_REG, ~0);
-
- /* Clear the scb_line_status events */
- img_i2c_writel(i2c, SCB_CLEAR_REG, ~0);
-
- /* Enable interrupts */
- img_i2c_writel(i2c, SCB_INT_MASK_REG, i2c->int_enable);
-
/* Perform a synchronous sequence to reset the bus */
ret = img_i2c_reset_bus(i2c);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/8] i2c: img-scb: do dummy writes before fifo access
Move scb_wr_rd_fence to before reading from fifo and writing to
fifo to make sure the the first read/write is done after the required
number of cycles.
Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB driver")
Signed-off-by: Sifan Naeem
Acked-by: James Hogan
Cc: Stable kernel (v3.19+)
---
drivers/i2c/busses/i2c-img-scb.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 5c3c615..0368d91 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -534,6 +534,7 @@ static void img_i2c_read_fifo(struct img_i2c *i2c)
u32 fifo_status;
u8 data;
+ img_i2c_wr_rd_fence(i2c);
fifo_status = img_i2c_readl(i2c, SCB_FIFO_STATUS_REG);
if (fifo_status & FIFO_READ_EMPTY)
break;
@@ -542,7 +543,6 @@ static void img_i2c_read_fifo(struct img_i2c *i2c)
*i2c->msg.buf = data;
img_i2c_writel(i2c, SCB_READ_FIFO_REG, 0xff);
- img_i2c_wr_rd_fence(i2c);
i2c->msg.len--;
i2c->msg.buf++;
}
@@ -554,12 +554,12 @@ static void img_i2c_write_fifo(struct img_i2c *i2c)
while (i2c->msg.len) {
u32 fifo_status;
+ img_i2c_wr_rd_fence(i2c);
fifo_status = img_i2c_readl(i2c, SCB_FIFO_STATUS_REG);
if (fifo_status & FIFO_WRITE_FULL)
break;
img_i2c_writel(i2c, SCB_WRITE_DATA_REG, *i2c->msg.buf);
- img_i2c_wr_rd_fence(i2c);
i2c->msg.len--;
i2c->msg.buf++;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/8] i2c: img-scb: fix LOW and HIGH period values for the SCL clock
After determining the minimum value for the High period (TCKH) the
remainder of the internal clock pulses is set as the Low period (TCKL).
This causes the i2c clock duty cycle to be much less than 50%.
The fix suggested here, start with TCKH and TCKL at 50% of the internal
clock pulses and adjusts the TCKH and TCKL values from there if the
minimum value for TCKL is not met. This will make sure the i2c clock
duty cycle is at 50% or close 50% whenever possible.
Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB driver")
Signed-off-by: Sifan Naeem
Cc: Stable kernel (v3.19+)
---
drivers/i2c/busses/i2c-img-scb.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index b4f59e1..51a5be8 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -1178,25 +1178,29 @@ static int img_i2c_init(struct img_i2c *i2c)
((bitrate_khz * clk_period) / 2))
int_bitrate++;
- /* Setup TCKH value */
- tckh = DIV_ROUND_UP(timing.tckh, clk_period);
+ /*
+* Setup clock duty cycle, start with 50% and adjust TCKH and TCKL
+* values from there if they don't meet minimum timing requirements
+*/
+tckh = int_bitrate / 2;
+tckl = int_bitrate - tckh;
- if (tckh > 0)
- data = tckh - 1;
- else
- data = 0;
+ /* Adjust TCKH and TCKL values */
+ data = DIV_ROUND_UP(timing.tckl, clk_period);
- img_i2c_writel(i2c, SCB_TIME_TCKH_REG, data);
+ if (tckl < data) {
+ tckl = data;
+ tckh = int_bitrate - tckl;
+ }
- /* Setup TCKL value */
- tckl = int_bitrate - tckh;
+ if (tckh > 0)
+ --tckh;
if (tckl > 0)
- data = tckl - 1;
- else
- data = 0;
+ --tckl;
- img_i2c_writel(i2c, SCB_TIME_TCKL_REG, data);
+ img_i2c_writel(i2c, SCB_TIME_TCKH_REG, tckh);
+ img_i2c_writel(i2c, SCB_TIME_TCKL_REG, tckl);
/* Setup TSDH value */
tsdh = DIV_ROUND_UP(timing.tsdh, clk_period);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
