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

2015-08-13 Thread Alexander Sverdlin
Hello Wolfram,

On 12/08/15 10:43, ext Wolfram Sang wrote:
  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: 0x 
  0105]
 0x0038 ICPID2 I2C Peripheral Identification Register 2 [value: 0x 
  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?

I would need to rework the patch to distinguish Keystones basing on DT 
compatible property.
I will re-send another version along with actual bofore-after real world 
numbers.
Thanks for feedback!

-- 
Best regards,
Alexander Sverdlin.
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-08-12 Thread Wolfram Sang
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.sverd...@nokia.com
  ---
 
  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: 0x 0105]
0x0038 ICPID2 I2C Peripheral Identification Register 2 [value: 0x 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?



signature.asc
Description: Digital signature


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

2015-07-10 Thread Grygorii Strashko
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.sverd...@nokia.com
 ---

 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: 0x 0105]
   0x0038 ICPID2 I2C Peripheral Identification Register 2 [value: 0x 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. 

-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-07-10 Thread Sekhar Nori
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.sverd...@nokia.com
 ---

 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
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-18 Thread Alexander Sverdlin
Hello!

On 18/06/15 13:12, ext Sekhar Nori wrote:
 Ah, beyond the evalboards, there are device-trees not linked into the 
 kernel,
  but flashed into the boards, as originally in OF. They are part of 
  the HW, its
  description. Not part or description of the Kernel. And you have no 
  way to
  introduce this fix any more without updating this OF part if you go 
  with
  new compatible property.
  I see. So how critical is this fix? That should be described in the
  commit description. And if its really critical, stable kernel should be
  CCed too.
  
  Now we got to the point, see below...
  
  And from the other PoV, device-trees are for something 
  one cannot probe. We
  can probe for Keystone revisions and can free the 
  end-user from this headache
  completely.
  Keep in mind that this can invite driver patching whenever 
  version
  number is tinkered with in hardware - even for otherwise
  software-invsible changes.
 
  That's true. But I do not have an overview, how many IP versions do 
  you actually have?
  I've found one revision in Davinci manual, one revision in Keystone 
  manual, even
  including minor revision. Checking only major revision now can 
  survive couple of minor
  changes in IP.
  Yeah, sticking to major version should help. What I am worried about are
  versions coming in future, not those existing. And development on
  keystone architecture is ongoing in TI.
  
  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. So 
  no point in CCing
  stable maintainers also.
 If its not a critical fix, do we really need to care about older DTBs
 which have been ROM'ed into production?

I tend not to change the DT binding, but if majority will decide it's the way 
to go,
I'll prepare another patch. Let's wait for other opinions...

-- 
Best regards,
Alexander Sverdlin.
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-18 Thread Sekhar Nori
On Thursday 18 June 2015 03:34 PM, Alexander Sverdlin wrote:
 Hi!
 
 On 18/06/15 11:47, ext Sekhar Nori wrote:
 Ah, beyond the evalboards, there are device-trees not linked into the 
 kernel,
 but flashed into the boards, as originally in OF. They are part of the HW, 
 its
 description. Not part or description of the Kernel. And you have no way to
 introduce this fix any more without updating this OF part if you go with
 new compatible property.
 I see. So how critical is this fix? That should be described in the
 commit description. And if its really critical, stable kernel should be
 CCed too.
 
 Now we got to the point, see below...
 
 And from the other PoV, device-trees are for something one cannot 
 probe. We
 can probe for Keystone revisions and can free the end-user from this 
 headache
 completely.
 Keep in mind that this can invite driver patching whenever version
 number is tinkered with in hardware - even for otherwise
 software-invsible changes.

 That's true. But I do not have an overview, how many IP versions do you 
 actually have?
 I've found one revision in Davinci manual, one revision in Keystone 
 manual, even
 including minor revision. Checking only major revision now can survive 
 couple of minor
 changes in IP.
 Yeah, sticking to major version should help. What I am worried about are
 versions coming in future, not those existing. And development on
 keystone architecture is ongoing in TI.
 
 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. So no 
 point in CCing
 stable maintainers also.

If its not a critical fix, do we really need to care about older DTBs
which have been ROM'ed into production?

Thanks,
Sekhar
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-18 Thread Murali Karicheri

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.sverd...@nokia.com
---

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


Murali




  drivers/i2c/busses/i2c-davinci.c |   10 ++
  1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 4a110af..3d78f6a 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -60,6 +60,8 @@
  #define DAVINCI_I2C_IVR_REG   0x28
  #define DAVINCI_I2C_EMDR_REG  0x2c
  #define DAVINCI_I2C_PSC_REG   0x30
+#define DAVINCI_I2C_ICPID1_REG 0x34
+#define DAVINCI_I2C_ICPID2_REG 0x38
  #define DAVINCI_I2C_FUNC_REG  0x48
  #define DAVINCI_I2C_DIR_REG   0x4c
  #define DAVINCI_I2C_DIN_REG   0x50
@@ -203,6 +205,9 @@ static void i2c_davinci_calc_clk_dividers(struct 
davinci_i2c_dev *dev)
 * where if PSC == 0, d = 7,
 *   if PSC == 1, d = 6
 *   if PSC  1 , d = 5
+*
+* Note:
+* d is always 6 on Keystone I2C controller
 */

/* get minimum of 7 MHz clock, but max of 12 MHz */
@@ -211,6 +216,11 @@ static void i2c_davinci_calc_clk_dividers(struct 
davinci_i2c_dev *dev)
psc++;  /* better to run under spec than over */
d = (psc = 2) ? 5 : 7 - psc;

+   if (davinci_i2c_read_reg(dev, DAVINCI_I2C_ICPID2_REG) == 0x2206) {
+   dev_dbg(dev-dev, Keystone SoC detected\n);
+   d = 6;
+   }


I think its better to use a different compatible string for i2c on
keystone devices rather than using a fixed hardcoded IP version.

Thanks,
Sekhar




--
Murali Karicheri
Linux Kernel, Keystone
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-18 Thread Alexander Sverdlin
Hi!

On 18/06/15 11:47, ext Sekhar Nori wrote:
 Ah, beyond the evalboards, there are device-trees not linked into the kernel,
  but flashed into the boards, as originally in OF. They are part of the HW, 
  its
  description. Not part or description of the Kernel. And you have no way to
  introduce this fix any more without updating this OF part if you go with
  new compatible property.
 I see. So how critical is this fix? That should be described in the
 commit description. And if its really critical, stable kernel should be
 CCed too.

Now we got to the point, see below...

  And from the other PoV, device-trees are for something one cannot 
  probe. We
  can probe for Keystone revisions and can free the end-user from this 
  headache
  completely.
  Keep in mind that this can invite driver patching whenever version
  number is tinkered with in hardware - even for otherwise
  software-invsible changes.
  
  That's true. But I do not have an overview, how many IP versions do you 
  actually have?
  I've found one revision in Davinci manual, one revision in Keystone 
  manual, even
  including minor revision. Checking only major revision now can survive 
  couple of minor
  changes in IP.
 Yeah, sticking to major version should help. What I am worried about are
 versions coming in future, not those existing. And development on
 keystone architecture is ongoing in TI.

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. So no 
point in CCing
stable maintainers also.

-- 
Best regards,
Alexander Sverdlin.
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-18 Thread Alexander Sverdlin
Hi!

On 18/06/15 11:00, ext Sekhar Nori wrote:
 +if (davinci_i2c_read_reg(dev, DAVINCI_I2C_ICPID2_REG) == 0x2206) {
  +  dev_dbg(dev-dev, Keystone SoC detected\n);
  +  d = 6;
  +  }
 I think its better to use a different compatible string for i2c on
 keystone devices rather than using a fixed hardcoded IP version.

Yeah, this should have been done from the beginning, when the driver has been
re-used for Keystone, but this time is already missed, so I don't want
to introduce huge incompatibility with the existing device-trees.

And from the other PoV, device-trees are for something one cannot probe. We
can probe for Keystone revisions and can free the end-user from this headache
completely.

-- 
Best regards,
Alexander Sverdlin.
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-18 Thread Sekhar Nori
Hi Alexander,

On Thursday 18 June 2015 02:39 PM, Alexander Sverdlin wrote:
 Hi!
 
 On 18/06/15 11:00, ext Sekhar Nori wrote:
 +   if (davinci_i2c_read_reg(dev, DAVINCI_I2C_ICPID2_REG) == 0x2206) {
 +  dev_dbg(dev-dev, Keystone SoC detected\n);
 +  d = 6;
 +  }
 I think its better to use a different compatible string for i2c on
 keystone devices rather than using a fixed hardcoded IP version.
 
 Yeah, this should have been done from the beginning, when the driver has been
 re-used for Keystone, but this time is already missed, so I don't want
 to introduce huge incompatibility with the existing device-trees.

How is backward compatibility broken? compatible property is a list, so
you would do something like:

compatible = ti,keystone-i2c, ti,davinci-i2c;

Newer kernels would keep working with older device tree blobs. Older
kernels wont have the fix, but thats true even now.

 And from the other PoV, device-trees are for something one cannot probe. We
 can probe for Keystone revisions and can free the end-user from this headache
 completely.

Keep in mind that this can invite driver patching whenever version
number is tinkered with in hardware - even for otherwise
software-invsible changes.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-18 Thread Alexander Sverdlin
Hello!

On 18/06/15 11:25, ext Sekhar Nori wrote:
 +  if (davinci_i2c_read_reg(dev, DAVINCI_I2C_ICPID2_REG) == 0x2206) {
  +dev_dbg(dev-dev, Keystone SoC detected\n);
  +d = 6;
  +}
  I think its better to use a different compatible string for i2c on
  keystone devices rather than using a fixed hardcoded IP version.
  
  Yeah, this should have been done from the beginning, when the driver has 
  been
  re-used for Keystone, but this time is already missed, so I don't want
  to introduce huge incompatibility with the existing device-trees.
 How is backward compatibility broken? compatible property is a list, so
 you would do something like:
 
   compatible = ti,keystone-i2c, ti,davinci-i2c;
 
 Newer kernels would keep working with older device tree blobs. Older
 kernels wont have the fix, but thats true even now.

Ah, beyond the evalboards, there are device-trees not linked into the kernel,
but flashed into the boards, as originally in OF. They are part of the HW, its
description. Not part or description of the Kernel. And you have no way to
introduce this fix any more without updating this OF part if you go with
new compatible property.

  And from the other PoV, device-trees are for something one cannot probe. We
  can probe for Keystone revisions and can free the end-user from this 
  headache
  completely.
 Keep in mind that this can invite driver patching whenever version
 number is tinkered with in hardware - even for otherwise
 software-invsible changes.

That's true. But I do not have an overview, how many IP versions do you 
actually have?
I've found one revision in Davinci manual, one revision in Keystone manual, even
including minor revision. Checking only major revision now can survive couple 
of minor
changes in IP.

-- 
Best regards,
Alexander Sverdlin.
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-18 Thread Sekhar Nori
On Thursday 18 June 2015 03:07 PM, Alexander Sverdlin wrote:
 Hello!
 
 On 18/06/15 11:25, ext Sekhar Nori wrote:
 + if (davinci_i2c_read_reg(dev, DAVINCI_I2C_ICPID2_REG) == 0x2206) {
 +dev_dbg(dev-dev, Keystone SoC detected\n);
 +d = 6;
 +}
 I think its better to use a different compatible string for i2c on
 keystone devices rather than using a fixed hardcoded IP version.

 Yeah, this should have been done from the beginning, when the driver has 
 been
 re-used for Keystone, but this time is already missed, so I don't want
 to introduce huge incompatibility with the existing device-trees.
 How is backward compatibility broken? compatible property is a list, so
 you would do something like:

  compatible = ti,keystone-i2c, ti,davinci-i2c;

 Newer kernels would keep working with older device tree blobs. Older
 kernels wont have the fix, but thats true even now.
 
 Ah, beyond the evalboards, there are device-trees not linked into the kernel,
 but flashed into the boards, as originally in OF. They are part of the HW, its
 description. Not part or description of the Kernel. And you have no way to
 introduce this fix any more without updating this OF part if you go with
 new compatible property.

I see. So how critical is this fix? That should be described in the
commit description. And if its really critical, stable kernel should be
CCed too.

 
 And from the other PoV, device-trees are for something one cannot probe. We
 can probe for Keystone revisions and can free the end-user from this 
 headache
 completely.
 Keep in mind that this can invite driver patching whenever version
 number is tinkered with in hardware - even for otherwise
 software-invsible changes.
 
 That's true. But I do not have an overview, how many IP versions do you 
 actually have?
 I've found one revision in Davinci manual, one revision in Keystone manual, 
 even
 including minor revision. Checking only major revision now can survive couple 
 of minor
 changes in IP.

Yeah, sticking to major version should help. What I am worried about are
versions coming in future, not those existing. And development on
keystone architecture is ongoing in TI.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-18 Thread Alexander Sverdlin
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.sverd...@nokia.com
---

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.

 drivers/i2c/busses/i2c-davinci.c |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 4a110af..3d78f6a 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -60,6 +60,8 @@
 #define DAVINCI_I2C_IVR_REG0x28
 #define DAVINCI_I2C_EMDR_REG   0x2c
 #define DAVINCI_I2C_PSC_REG0x30
+#define DAVINCI_I2C_ICPID1_REG 0x34
+#define DAVINCI_I2C_ICPID2_REG 0x38
 #define DAVINCI_I2C_FUNC_REG   0x48
 #define DAVINCI_I2C_DIR_REG0x4c
 #define DAVINCI_I2C_DIN_REG0x50
@@ -203,6 +205,9 @@ static void i2c_davinci_calc_clk_dividers(struct 
davinci_i2c_dev *dev)
 * where if PSC == 0, d = 7,
 *   if PSC == 1, d = 6
 *   if PSC  1 , d = 5
+*
+* Note:
+* d is always 6 on Keystone I2C controller
 */

/* get minimum of 7 MHz clock, but max of 12 MHz */
@@ -211,6 +216,11 @@ static void i2c_davinci_calc_clk_dividers(struct 
davinci_i2c_dev *dev)
psc++;  /* better to run under spec than over */
d = (psc = 2) ? 5 : 7 - psc;

+   if (davinci_i2c_read_reg(dev, DAVINCI_I2C_ICPID2_REG) == 0x2206) {
+   dev_dbg(dev-dev, Keystone SoC detected\n);
+   d = 6;
+   }
+
clk = ((input_clock / (psc + 1)) / (pdata-bus_freq * 1000));
/* Avoid driving the bus too fast because of rounding errors above */
if (input_clock / (psc + 1) / clk  pdata-bus_freq * 1000)
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-18 Thread Sekhar Nori
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.sverd...@nokia.com
 ---
 
 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.

 
  drivers/i2c/busses/i2c-davinci.c |   10 ++
  1 files changed, 10 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-davinci.c 
 b/drivers/i2c/busses/i2c-davinci.c
 index 4a110af..3d78f6a 100644
 --- a/drivers/i2c/busses/i2c-davinci.c
 +++ b/drivers/i2c/busses/i2c-davinci.c
 @@ -60,6 +60,8 @@
  #define DAVINCI_I2C_IVR_REG  0x28
  #define DAVINCI_I2C_EMDR_REG 0x2c
  #define DAVINCI_I2C_PSC_REG  0x30
 +#define DAVINCI_I2C_ICPID1_REG   0x34
 +#define DAVINCI_I2C_ICPID2_REG   0x38
  #define DAVINCI_I2C_FUNC_REG 0x48
  #define DAVINCI_I2C_DIR_REG  0x4c
  #define DAVINCI_I2C_DIN_REG  0x50
 @@ -203,6 +205,9 @@ static void i2c_davinci_calc_clk_dividers(struct 
 davinci_i2c_dev *dev)
* where if PSC == 0, d = 7,
*   if PSC == 1, d = 6
*   if PSC  1 , d = 5
 +  *
 +  * Note:
 +  * d is always 6 on Keystone I2C controller
*/
 
   /* get minimum of 7 MHz clock, but max of 12 MHz */
 @@ -211,6 +216,11 @@ static void i2c_davinci_calc_clk_dividers(struct 
 davinci_i2c_dev *dev)
   psc++;  /* better to run under spec than over */
   d = (psc = 2) ? 5 : 7 - psc;
 
 + if (davinci_i2c_read_reg(dev, DAVINCI_I2C_ICPID2_REG) == 0x2206) {
 + dev_dbg(dev-dev, Keystone SoC detected\n);
 + d = 6;
 + }

I think its better to use a different compatible string for i2c on
keystone devices rather than using a fixed hardcoded IP version.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html