On 28/07/15 10:26, Sifan Naeem wrote:
> 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.
Okay, no problem then.
>
>>>
>>> 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).
>
>>> 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
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@xxxxxxxxxxxxxxx.
Cheers
James
>
>> 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
>>>
Attachment:
signature.asc
Description: OpenPGP digital signature