RE: [PATCH 1/8] i2c: img-scb: enable fencing for all versions of the ip

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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�����������ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]