Re: [PATCH] i2c: davinci: Fix bus rate calculation on Keystone SoC
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
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 > > --- > > > > 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
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 > --- > > 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
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 --- 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
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
> >>---
> >>
> >>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?
>
> 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
signature.asc
Description: Digital signature
Re: [PATCH] i2c: davinci: Fix bus rate calculation on Keystone SoC
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
---
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
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
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
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
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
Re: [PATCH] i2c: davinci: Fix bus rate calculation on Keystone SoC
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
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
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
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
> ---
>
> 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
[PATCH] i2c: davinci: Fix bus rate calculation on Keystone SoC
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
---
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
