linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/7] i2c: img-scb: fixes to support i2c on pistachio
@ 2015-08-14 15:50 Sifan Naeem
       [not found] ` <1439567424-8094-1-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 12+ messages in thread
From: Sifan Naeem @ 2015-08-14 15:50 UTC (permalink / raw)
  To: Wolfram Sang, James Hogan, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	Ezequiel Garcia
  Cc: Ionela Voinescu, Sifan Naeem

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/7] i2c: img-scb: enable fencing for all versions of the ip
       [not found] ` <1439567424-8094-1-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
@ 2015-08-14 15:50   ` Sifan Naeem
  2015-08-14 15:50   ` [PATCH v2 2/7] i2c: img-scb: do dummy writes before fifo access Sifan Naeem
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Sifan Naeem @ 2015-08-14 15:50 UTC (permalink / raw)
  To: Wolfram Sang, James Hogan, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	Ezequiel Garcia
  Cc: Ionela Voinescu, Sifan Naeem

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 <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Acked-by: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Reviewed-by: James Hartley <james.hartley-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/7] i2c: img-scb: do dummy writes before fifo access
       [not found] ` <1439567424-8094-1-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
  2015-08-14 15:50   ` [PATCH v2 1/7] i2c: img-scb: enable fencing for all versions of the ip Sifan Naeem
@ 2015-08-14 15:50   ` Sifan Naeem
  2015-08-14 15:50   ` [PATCH v2 3/7] i2c: img-scb: use DIV_ROUND_UP to round divisor values Sifan Naeem
                     ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Sifan Naeem @ 2015-08-14 15:50 UTC (permalink / raw)
  To: Wolfram Sang, James Hogan, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	Ezequiel Garcia
  Cc: Ionela Voinescu, Sifan Naeem

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 <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Acked-by: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Reviewed-by: James Hartley <james.hartley-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/7] i2c: img-scb: use DIV_ROUND_UP to round divisor values
       [not found] ` <1439567424-8094-1-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
  2015-08-14 15:50   ` [PATCH v2 1/7] i2c: img-scb: enable fencing for all versions of the ip Sifan Naeem
  2015-08-14 15:50   ` [PATCH v2 2/7] i2c: img-scb: do dummy writes before fifo access Sifan Naeem
@ 2015-08-14 15:50   ` Sifan Naeem
  2015-08-14 15:50   ` [PATCH v2 4/7] i2c: img-scb: fix LOW and HIGH period values for the SCL clock Sifan Naeem
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Sifan Naeem @ 2015-08-14 15:50 UTC (permalink / raw)
  To: Wolfram Sang, James Hogan, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	Ezequiel Garcia
  Cc: Ionela Voinescu, Sifan Naeem

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 <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Acked-by: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Reviewed-by: James Hartley <james.hartley-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/7] i2c: img-scb: fix LOW and HIGH period values for the SCL clock
       [not found] ` <1439567424-8094-1-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
                     ` (2 preceding siblings ...)
  2015-08-14 15:50   ` [PATCH v2 3/7] i2c: img-scb: use DIV_ROUND_UP to round divisor values Sifan Naeem
@ 2015-08-14 15:50   ` Sifan Naeem
  2015-08-14 15:50   ` [PATCH v2 5/7] i2c: img-scb: remove start bit detected status after handling Sifan Naeem
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Sifan Naeem @ 2015-08-14 15:50 UTC (permalink / raw)
  To: Wolfram Sang, James Hogan, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	Ezequiel Garcia
  Cc: Ionela Voinescu, Sifan Naeem

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 <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Acked-by: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Reviewed-by: James Hartley <james.hartley-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 5/7] i2c: img-scb: remove start bit detected status after handling
       [not found] ` <1439567424-8094-1-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
                     ` (3 preceding siblings ...)
  2015-08-14 15:50   ` [PATCH v2 4/7] i2c: img-scb: fix LOW and HIGH period values for the SCL clock Sifan Naeem
@ 2015-08-14 15:50   ` Sifan Naeem
       [not found]     ` <1439567424-8094-6-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
  2015-08-14 15:50   ` [PATCH v2 6/7] i2c: img-scb: Clear line and interrupt status before starting a transfer Sifan Naeem
  2015-08-14 15:50   ` [PATCH v2 7/7] i2c: img-scb: verify support for requested bit rate Sifan Naeem
  6 siblings, 1 reply; 12+ messages in thread
From: Sifan Naeem @ 2015-08-14 15:50 UTC (permalink / raw)
  To: Wolfram Sang, James Hogan, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	Ezequiel Garcia
  Cc: Ionela Voinescu, Sifan Naeem

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 <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Reviewed-by: James Hartley <james.hartley-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 6/7] i2c: img-scb: Clear line and interrupt status before starting a transfer
       [not found] ` <1439567424-8094-1-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
                     ` (4 preceding siblings ...)
  2015-08-14 15:50   ` [PATCH v2 5/7] i2c: img-scb: remove start bit detected status after handling Sifan Naeem
@ 2015-08-14 15:50   ` Sifan Naeem
       [not found]     ` <1439567424-8094-7-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
  2015-08-14 15:50   ` [PATCH v2 7/7] i2c: img-scb: verify support for requested bit rate Sifan Naeem
  6 siblings, 1 reply; 12+ messages in thread
From: Sifan Naeem @ 2015-08-14 15:50 UTC (permalink / raw)
  To: Wolfram Sang, James Hogan, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	Ezequiel Garcia
  Cc: Ionela Voinescu, Sifan Naeem

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 <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Reviewed-by: James Hartley <james.hartley-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 7/7] i2c: img-scb: verify support for requested bit rate
       [not found] ` <1439567424-8094-1-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
                     ` (5 preceding siblings ...)
  2015-08-14 15:50   ` [PATCH v2 6/7] i2c: img-scb: Clear line and interrupt status before starting a transfer Sifan Naeem
@ 2015-08-14 15:50   ` Sifan Naeem
       [not found]     ` <1439567424-8094-8-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
  6 siblings, 1 reply; 12+ messages in thread
From: Sifan Naeem @ 2015-08-14 15:50 UTC (permalink / raw)
  To: Wolfram Sang, James Hogan, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	Ezequiel Garcia
  Cc: Ionela Voinescu, Sifan Naeem

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 <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Reviewed-by: James Hartley <james.hartley-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 7/7] i2c: img-scb: verify support for requested bit rate
       [not found]     ` <1439567424-8094-8-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
@ 2015-08-19 21:17       ` James Hogan
       [not found]         ` <20150819211750.GD8443-4bYivNCBEGTR3KXKvIWQxtm+Uo4AYnCiHZ5vskTnxNA@public.gmane.org>
  0 siblings, 1 reply; 12+ messages in thread
From: James Hogan @ 2015-08-19 21:17 UTC (permalink / raw)
  To: Sifan Naeem
  Cc: Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Ezequiel Garcia,
	Ionela Voinescu

[-- Attachment #1: Type: text/plain, Size: 2631 bytes --]

On Fri, Aug 14, 2015 at 04:50:24PM +0100, 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.
> 
> 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 <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: James Hartley <james.hartley-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> ---
>  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",

Technically both i2c->bitrate and timings[].max_bitrate are unsigned, so
%u would be more correct for both.

Otherwise:
Acked-by: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

Wolfram: are you happy to fix the %u thing when applying it, or would
you prefer me to submit a fixed patch (Sifan is off for a few weeks I
believe).

(Still need to look at the other changes...)

Cheers
James

> +			 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
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 5/7] i2c: img-scb: remove start bit detected status after handling
       [not found]     ` <1439567424-8094-6-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
@ 2015-08-19 21:46       ` James Hogan
  0 siblings, 0 replies; 12+ messages in thread
From: James Hogan @ 2015-08-19 21:46 UTC (permalink / raw)
  To: Sifan Naeem
  Cc: Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Ezequiel Garcia,
	Ionela Voinescu

[-- Attachment #1: Type: text/plain, Size: 2232 bytes --]

On Fri, Aug 14, 2015 at 04:50:22PM +0100, 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: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB driver")
> Signed-off-by: Sifan Naeem <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: James Hartley <james.hartley-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> ---
>  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;

If we start checking line_status instead of i2c->line_status, this
masking off of START_BIT_DET from i2c->line_status seems redundant as it
won't have any effect on whether we hit this condition again on the next
interrupt.

I think the one line patch with just s/i2c->line_status/line_status/ on
the condition should be sufficient to prevent it handling the situation
repeatedly (that line status bit is latched, and should have been
already acked).

Cheers
James

>  	}
>  
>  	mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
> -- 
> 1.7.9.5
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 6/7] i2c: img-scb: Clear line and interrupt status before starting a transfer
       [not found]     ` <1439567424-8094-7-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
@ 2015-08-19 21:58       ` James Hogan
  0 siblings, 0 replies; 12+ messages in thread
From: James Hogan @ 2015-08-19 21:58 UTC (permalink / raw)
  To: Sifan Naeem
  Cc: Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Ezequiel Garcia,
	Ionela Voinescu

[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]

On Fri, Aug 14, 2015 at 04:50:23PM +0100, Sifan Naeem wrote:
> 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 <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: James Hartley <james.hartley-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

I think it could have done with some more explanation about when this
happens and whether between messages in the transfer or between
transfers, but it does look safe and reasonable, so:

Acked-by: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

Cheers
James

> ---
>  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
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 7/7] i2c: img-scb: verify support for requested bit rate
       [not found]         ` <20150819211750.GD8443-4bYivNCBEGTR3KXKvIWQxtm+Uo4AYnCiHZ5vskTnxNA@public.gmane.org>
@ 2015-08-20 20:28           ` Wolfram Sang
  0 siblings, 0 replies; 12+ messages in thread
From: Wolfram Sang @ 2015-08-20 20:28 UTC (permalink / raw)
  To: James Hogan
  Cc: Sifan Naeem, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Ezequiel Garcia,
	Ionela Voinescu

[-- Attachment #1: Type: text/plain, Size: 285 bytes --]


> Wolfram: are you happy to fix the %u thing when applying it, or would
> you prefer me to submit a fixed patch (Sifan is off for a few weeks I
> believe).

If this is the only thing to be fixed, I can do it. If more things
accumulate, I'd prefer a new version.

Thanks,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-08-20 20:28 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-14 15:50 [PATCH v2 0/7] i2c: img-scb: fixes to support i2c on pistachio Sifan Naeem
     [not found] ` <1439567424-8094-1-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
2015-08-14 15:50   ` [PATCH v2 1/7] i2c: img-scb: enable fencing for all versions of the ip Sifan Naeem
2015-08-14 15:50   ` [PATCH v2 2/7] i2c: img-scb: do dummy writes before fifo access Sifan Naeem
2015-08-14 15:50   ` [PATCH v2 3/7] i2c: img-scb: use DIV_ROUND_UP to round divisor values Sifan Naeem
2015-08-14 15:50   ` [PATCH v2 4/7] i2c: img-scb: fix LOW and HIGH period values for the SCL clock Sifan Naeem
2015-08-14 15:50   ` [PATCH v2 5/7] i2c: img-scb: remove start bit detected status after handling Sifan Naeem
     [not found]     ` <1439567424-8094-6-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
2015-08-19 21:46       ` James Hogan
2015-08-14 15:50   ` [PATCH v2 6/7] i2c: img-scb: Clear line and interrupt status before starting a transfer Sifan Naeem
     [not found]     ` <1439567424-8094-7-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
2015-08-19 21:58       ` James Hogan
2015-08-14 15:50   ` [PATCH v2 7/7] i2c: img-scb: verify support for requested bit rate Sifan Naeem
     [not found]     ` <1439567424-8094-8-git-send-email-sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
2015-08-19 21:17       ` James Hogan
     [not found]         ` <20150819211750.GD8443-4bYivNCBEGTR3KXKvIWQxtm+Uo4AYnCiHZ5vskTnxNA@public.gmane.org>
2015-08-20 20:28           ` Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).