Re: [PATCH] i2c: davinci: Fix bus rate calculation on Keystone SoC

[Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2867 bytes --]

On Fri, Jul 10, 2015 at 09:26:30PM +0300, Grygorii Strashko wrote:
> Hi,
> On 07/10/2015 07:02 PM, Sekhar Nori wrote:
> > On Friday 10 July 2015 01:23 AM, Wolfram Sang wrote:
> >> On Thu, Jun 18, 2015 at 12:22:33PM -0400, Murali Karicheri wrote:
> >>> On 06/18/2015 05:00 AM, Sekhar Nori wrote:
> >>>> On Thursday 18 June 2015 02:23 PM, Alexander Sverdlin wrote:
> >>>>> According to KeyStone Architecture I2C User Guide,
> >>>>>
> >>>>>                           module clock frequency
> >>>>> master clock frequency = ----------------------
> >>>>>                           (ICCL + 6) + (ICCH + 6)
> >>>>>
> >>>>> i.e. "d" in i2c_davinci_calc_clk_dividers() should be fixed and
> >>>>> not dependent from module clock prescaler PSC on these SoCs.
> >>>>>
> >>>>> Signed-off-by: Alexander Sverdlin <alexander.sverdlin-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> >>>>> ---
> >>>>>
> >>>>> RFC: If someone from TI has an idea how to improve the coverage of future Keystone
> >>>>> revisions -- hints/patches are welcome. The current ID check is based on
> >>>>> Davinci/Keystone datasheets and is at least working on real Keystone II.
> >>>>
> >>>> + Murali who works on Keystone devices in TI.
> >>>
> >>> + Grygorii
> >>>
> >>> + Grygorii has been involved in the Keystone related enhancement and
> >>> reviewing prior patches. Need to have his ack for this change
> >>
> >> Any news?
> > 
> > Fixing Grygorii's e-mail id.
> > 
> > Grygorii, let me know if you don't have the thread. I can forward.
> 
> Thanks Sekhar.
> 
> My opinion - it's time for compatible string :) "ti,keystone-i2c". 
> Especially taking int account two things:
> 1) In Datasheet SPRS893B TCI6630K2L Multicore DSP+ARM KeyStone II System-on-Chip (SoC) (Rev. E) 
>    values for those registers specified as:
>    0x0034 ICPID1 I2C Peripheral Identification Register 1 [value: 0x0000 0105]
>    0x0038 ICPID2 I2C Peripheral Identification Register 2 [value: 0x0000 0005]
>    (actually the same is in k2h, k2e Datasheets).
> 
> 2) This is not the first time such discussion has been raised.
> 
> 
> >> > This is not really critical fix. Currently bus rate is lower than expected because of these
> >> > calculation errors. The fix maximizes the bus rate. So newer SoCs will run little bit slower
> >> > until support is added to this part of the code. Not really critical. 
> 
> Regarding the patch itself:
> - Seems the "d" value is fixed to 6 as per User Guide SPRUGV3
>   "KeyStone Architecture 2 Inter-IC Control Bus (I2C)" and this change is correct. 
>   It would be nice to have ref on document in commit message as above.
> 
> - I think, it will be very useful to have same real digits/calculation mentioned in
>   commit message which can show how valuable is the improvement. 

Alexander, any comments to this feedback?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]