Hi James, > -----Original Message----- > From: James Hogan > Sent: 27 July 2015 21:21 > To: Sifan Naeem > Cc: Wolfram Sang; linux-i2c@xxxxxxxxxxxxxxx; 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 <sifan.naeem@xxxxxxxxxx> > > Cc: Stable kernel (v3.19+) <stable@xxxxxxxxxxxxxxx> > > 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: <stable@xxxxxxxxxxxxxxx> # 3.19.x- > I go this error when doing that way: (body) Adding cc: Sifan Naeem <sifan.naeem@xxxxxxxxxx> from line 'Signed-off-by: Sifan Naeem <sifan.naeem@xxxxxxxxxx>' (body) Adding cc: <stable@xxxxxxxxxxxxxxx> # 4.1 from line 'Cc: <stable@xxxxxxxxxxxxxxx> # 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: <stable@xxxxxxxxxxxxxxx> # 4.1 W: unable to extract a valid address from: <stable@xxxxxxxxxxxxxxx> # 4.1 > Patch looks fine though > > Acked-by: James Hogan <james.hogan@xxxxxxxxxx> > 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 > > ��.n��������+%������w��{.n�����{��-��)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥