On 28/07/15 10:26, Sifan Naeem wrote:

Hi James,

quoted

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

quoted

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.

Okay, no problem then.

quoted

quoted

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?

I need to review the other patches anyway (sorry i've been slow to look
through them properly).

quoted

quoted

Signed-off-by: Sifan Naeem <redacted>
Cc: Stable kernel (v3.19+) <redacted>

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: <redacted> # 3.19.x-

I go this error when doing that way:

(body) Adding cc: Sifan Naeem [off-list ref] from line 'Signed-off-by: Sifan Naeem [off-list ref]'
(body) Adding cc: [off-list ref] # 4.1 from line 'Cc: [off-list ref] # 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: [off-list ref] # 4.1
W: unable to extract a valid address from: [off-list ref] # 4.1

Sounds like you're using an old version of git. Something similar is
described here:
http://comments.gmane.org/gmane.comp.version-control.git/210042

If the warning can be ignored, you can always add stable back to cc list
with --cc=stable@vger.kernel.org.

Cheers
James

quoted

Patch looks fine though

Acked-by: James Hogan <redacted>

Thanks,
Sifan

quoted

Thanks
James

quoted

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

quoted

 #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