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 <sifan.na...@imgtec.com>
> > Cc: Stable kernel (v3.19+) <sta...@vger.kernel.org>
>
> 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: <sta...@vger.kernel.org> # 3.19.x-
>
I go this error when doing that way:
(body) Adding cc: Sifan Naeem <sifan.na...@imgtec.com> from line
'Signed-off-by: Sifan Naeem <sifan.na...@imgtec.com>'
(body) Adding cc: <sta...@vger.kernel.org> # 4.1 from line 'Cc:
<sta...@vger.kernel.org> # 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: <sta...@vger.kernel.org> # 4.1
W: unable to extract a valid address from: <sta...@vger.kernel.org> # 4.1
> Patch looks fine though
>
> Acked-by: James Hogan <james.ho...@imgtec.com>
>
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
> >
