| Message ID | 1341493826-13861-1-git-send-email-s.hauer@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sascha, On Thu, Jul 05, 2012 at 03:10:26PM +0200, Sascha Hauer wrote: > + * These values (mostly) do not match the ones in the datasheet because > + * measurements have shown that the values are wrong. This was tested > + * on i.MX1 and i.MX53, so it shoud be safe to assume that the SoCs in s/shoud/should/ > + * between behave the same. > */ baruch
On Thu, Jul 05, 2012 at 03:10:26PM +0200, Sascha Hauer wrote: > Measurements on i.MX1 and i.MX53 have shown that the divider values > in the datasheets are wrong. the values from first, third and fourth > column were all measured to be 8 higher than in the datasheet. It > should be safe to assume that the SoCs between i.MX1 and i.MX53 behave > the same as the i2c unit is unchanged since the i.MX1. We may need to check with IC guys? Or you've done? Thanks Richard > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/i2c/busses/i2c-imx.c | 31 ++++++++++++++++++------------- > 1 file changed, 18 insertions(+), 13 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c > index 8d6b504..aabbe31 100644 > --- a/drivers/i2c/busses/i2c-imx.c > +++ b/drivers/i2c/busses/i2c-imx.c > @@ -98,22 +98,27 @@ > * Document Number: MC9328MXLRM, Rev. 5.1, 06/2007 > * > * Duplicated divider values removed from list > + * > + * These values (mostly) do not match the ones in the datasheet because > + * measurements have shown that the values are wrong. This was tested > + * on i.MX1 and i.MX53, so it shoud be safe to assume that the SoCs in > + * between behave the same. > */ > > static u16 __initdata i2c_clk_div[50][2] = { > - { 22, 0x20 }, { 24, 0x21 }, { 26, 0x22 }, { 28, 0x23 }, > - { 30, 0x00 }, { 32, 0x24 }, { 36, 0x25 }, { 40, 0x26 }, > - { 42, 0x03 }, { 44, 0x27 }, { 48, 0x28 }, { 52, 0x05 }, > - { 56, 0x29 }, { 60, 0x06 }, { 64, 0x2A }, { 72, 0x2B }, > - { 80, 0x2C }, { 88, 0x09 }, { 96, 0x2D }, { 104, 0x0A }, > - { 112, 0x2E }, { 128, 0x2F }, { 144, 0x0C }, { 160, 0x30 }, > - { 192, 0x31 }, { 224, 0x32 }, { 240, 0x0F }, { 256, 0x33 }, > - { 288, 0x10 }, { 320, 0x34 }, { 384, 0x35 }, { 448, 0x36 }, > - { 480, 0x13 }, { 512, 0x37 }, { 576, 0x14 }, { 640, 0x38 }, > - { 768, 0x39 }, { 896, 0x3A }, { 960, 0x17 }, { 1024, 0x3B }, > - { 1152, 0x18 }, { 1280, 0x3C }, { 1536, 0x3D }, { 1792, 0x3E }, > - { 1920, 0x1B }, { 2048, 0x3F }, { 2304, 0x1C }, { 2560, 0x1D }, > - { 3072, 0x1E }, { 3840, 0x1F } > + { 30, 0x20 }, { 32, 0x21 }, { 34, 0x22 }, { 36, 0x23 }, > + { 38, 0x00 }, { 40, 0x24 }, { 44, 0x25 }, { 48, 0x26 }, > + { 50, 0x03 }, { 52, 0x27 }, { 56, 0x28 }, { 60, 0x05 }, > + { 64, 0x29 }, { 68, 0x06 }, { 72, 0x2A }, { 80, 0x2B }, > + { 88, 0x2C }, { 96, 0x09 }, { 104, 0x2D }, { 112, 0x0A }, > + { 120, 0x2E }, { 136, 0x2F }, { 152, 0x0C }, { 168, 0x30 }, > + { 200, 0x31 }, { 232, 0x32 }, { 248, 0x0F }, { 264, 0x33 }, > + { 288, 0x10 }, { 328, 0x34 }, { 392, 0x35 }, { 456, 0x36 }, > + { 480, 0x13 }, { 520, 0x37 }, { 576, 0x14 }, { 648, 0x38 }, > + { 776, 0x39 }, { 904, 0x3A }, { 960, 0x17 }, { 1032, 0x3B }, > + { 1152, 0x18 }, { 1288, 0x3C }, { 1544, 0x3D }, { 1800, 0x3E }, > + { 1920, 0x1B }, { 2048, 0x3F }, { 2304, 0x1C }, { 2560, 0x1D }, > + { 3072, 0x1E }, { 3840, 0x1F } > }; > > struct imx_i2c_struct { > -- > 1.7.10 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Jul 05, 2012 at 10:52:39PM +0800, Richard Zhao wrote: > On Thu, Jul 05, 2012 at 03:10:26PM +0200, Sascha Hauer wrote: > > Measurements on i.MX1 and i.MX53 have shown that the divider values > > in the datasheets are wrong. the values from first, third and fourth > > column were all measured to be 8 higher than in the datasheet. It > > should be safe to assume that the SoCs between i.MX1 and i.MX53 behave > > the same as the i2c unit is unchanged since the i.MX1. > We may need to check with IC guys? Or you've done? I don't have contact to the IC guys. Could you do this? I'd be interested in the results. Sascha
On Thu, Jul 5, 2012 at 6:40 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote: > Measurements on i.MX1 and i.MX53 have shown that the divider values > in the datasheets are wrong. How were the measurements made? > the values from first, third and fourth > column were all measured to be 8 higher than in the datasheet. It > should be safe to assume that the SoCs between i.MX1 and i.MX53 behave > the same as the i2c unit is unchanged since the i.MX1. Also does it vary board to board or is fixed by the ip? What I mean is that the external cap etc. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > ---
On Thu, Jul 05, 2012 at 11:36:27PM +0530, Shubhrajyoti Datta wrote: > On Thu, Jul 5, 2012 at 6:40 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote: > > Measurements on i.MX1 and i.MX53 have shown that the divider values > > in the datasheets are wrong. > How were the measurements made? With an oscilloscope measuring the period length. This is probably not very exact, but the current values are way off. We had 350KHz instead of 380KHz (best possible divider) on an i.MX53. Funny enough some divider values really match the ones in the datasheet. BTW I'm pretty sure the input frequency to the core is calculated correctly as it's the same clock that also drives the timer. > > > the values from first, third and fourth > > column were all measured to be 8 higher than in the datasheet. It > > should be safe to assume that the SoCs between i.MX1 and i.MX53 behave > > the same as the i2c unit is unchanged since the i.MX1. > > Also does it vary board to board or is fixed by the ip? > What I mean is that the external cap etc. The clock is derived from an internal SoC clock, I don't think this can be influenced that much by external components. I did not test different boards, but two board with different SoCs. Let's see what the IC guys tell us. Sascha
On Thu, Jul 05, 2012 at 06:01:53PM +0200, Sascha Hauer wrote: > On Thu, Jul 05, 2012 at 10:52:39PM +0800, Richard Zhao wrote: > > On Thu, Jul 05, 2012 at 03:10:26PM +0200, Sascha Hauer wrote: > > > Measurements on i.MX1 and i.MX53 have shown that the divider values > > > in the datasheets are wrong. the values from first, third and fourth > > > column were all measured to be 8 higher than in the datasheet. It > > > should be safe to assume that the SoCs between i.MX1 and i.MX53 behave > > > the same as the i2c unit is unchanged since the i.MX1. > > We may need to check with IC guys? Or you've done? > > I don't have contact to the IC guys. Could you do this? I'd be > interested in the results. Sure, but it may be slow, as you know, many people are in summer break. Thanks Richard > > Sascha > > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Fri, Jul 06, 2012 at 08:52:50AM +0800, Richard Zhao wrote: > On Thu, Jul 05, 2012 at 06:01:53PM +0200, Sascha Hauer wrote: > > On Thu, Jul 05, 2012 at 10:52:39PM +0800, Richard Zhao wrote: > > > On Thu, Jul 05, 2012 at 03:10:26PM +0200, Sascha Hauer wrote: > > > > Measurements on i.MX1 and i.MX53 have shown that the divider values > > > > in the datasheets are wrong. the values from first, third and fourth > > > > column were all measured to be 8 higher than in the datasheet. It > > > > should be safe to assume that the SoCs between i.MX1 and i.MX53 behave > > > > the same as the i2c unit is unchanged since the i.MX1. > > > We may need to check with IC guys? Or you've done? > > > > I don't have contact to the IC guys. Could you do this? I'd be > > interested in the results. > Sure, but it may be slow, as you know, many people are in summer break. Thanks. It can take longer, the resulting frequencies are all too low, so we are on the safe side. Sascha
On 7/5/2012 11:28 PM, Sascha Hauer wrote: > On Fri, Jul 06, 2012 at 08:52:50AM +0800, Richard Zhao wrote: >> On Thu, Jul 05, 2012 at 06:01:53PM +0200, Sascha Hauer wrote: >>> On Thu, Jul 05, 2012 at 10:52:39PM +0800, Richard Zhao wrote: >>>> On Thu, Jul 05, 2012 at 03:10:26PM +0200, Sascha Hauer wrote: >>>>> Measurements on i.MX1 and i.MX53 have shown that the divider values >>>>> in the datasheets are wrong. the values from first, third and fourth >>>>> column were all measured to be 8 higher than in the datasheet. It >>>>> should be safe to assume that the SoCs between i.MX1 and i.MX53 behave >>>>> the same as the i2c unit is unchanged since the i.MX1. >>>> We may need to check with IC guys? Or you've done? >>> I don't have contact to the IC guys. Could you do this? I'd be >>> interested in the results. >> Sure, but it may be slow, as you know, many people are in summer break. > Thanks. It can take longer, the resulting frequencies are all too low, > so we are on the safe side. > > Sascha > You mean safe before the patch is applied, not after. Right? Troy
On Fri, Jul 6, 2012 at 12:17 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote: > On Thu, Jul 05, 2012 at 11:36:27PM +0530, Shubhrajyoti Datta wrote: >> On Thu, Jul 5, 2012 at 6:40 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote: >> > Measurements on i.MX1 and i.MX53 have shown that the divider values >> > in the datasheets are wrong. >> How were the measurements made? > > With an oscilloscope measuring the period length. This is probably not > very exact, but the current values are way off. We had 350KHz instead > of 380KHz (best possible divider) on an i.MX53. Funny enough some > divider values really match the ones in the datasheet. > > BTW I'm pretty sure the input frequency to the core is calculated > correctly as it's the same clock that also drives the timer. OK thanks for the explanation. > >> >> > the values from first, third and fourth >> > column were all measured to be 8 higher than in the datasheet. It >> > should be safe to assume that the SoCs between i.MX1 and i.MX53 behave >> > the same as the i2c unit is unchanged since the i.MX1. >> >> Also does it vary board to board or is fixed by the ip? >> What I mean is that the external cap etc. > > The clock is derived from an internal SoC clock, I don't think this can > be influenced that much by external components. I did not test different > boards, but two board with different SoCs. OK thanks. > > Let's see what the IC guys tell us. Sure. > > Sascha > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Fri, Jul 06, 2012 at 08:28:52AM +0200, Sascha Hauer wrote: > On Fri, Jul 06, 2012 at 08:52:50AM +0800, Richard Zhao wrote: > > On Thu, Jul 05, 2012 at 06:01:53PM +0200, Sascha Hauer wrote: > > > On Thu, Jul 05, 2012 at 10:52:39PM +0800, Richard Zhao wrote: > > > > On Thu, Jul 05, 2012 at 03:10:26PM +0200, Sascha Hauer wrote: > > > > > Measurements on i.MX1 and i.MX53 have shown that the divider values > > > > > in the datasheets are wrong. the values from first, third and fourth > > > > > column were all measured to be 8 higher than in the datasheet. It > > > > > should be safe to assume that the SoCs between i.MX1 and i.MX53 behave > > > > > the same as the i2c unit is unchanged since the i.MX1. > > > > We may need to check with IC guys? Or you've done? > > > > > > I don't have contact to the IC guys. Could you do this? I'd be > > > interested in the results. > > Sure, but it may be slow, as you know, many people are in summer break. > > Thanks. It can take longer, the resulting frequencies are all too low, > so we are on the safe side. Sascha, IC guys confirmed that the spec is right: This an adaptive feature of our I2C module may apply to all IMX chips. No mistake in the table of RMs. The divider is designed to guarantee SCL high level and low level last time. Divider will hold when SCL transition from 1 to 0 or 0 to 1, if the transition time is longer than 1 internal pre-divided clock cycle. The pre-divided clock is divided from I2C module clock, used for generating SCL. So you will see SCL clock cycle is some way longer than calculated value using IFDR. Transition time will different from rising or falling edge, different pull-up resistors, and different SCL loading. This feature make sure transition time won’t eat both level time of SCL. Thanks Richard > > Sascha > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | >
Hi Richard, On Wed, Jul 11, 2012 at 02:01:21PM +0800, Richard Zhao wrote: > > IC guys confirmed that the spec is right: > > This an adaptive feature of our I2C module may apply to all IMX chips. > No mistake in the table of RMs. > > The divider is designed to guarantee SCL high level and low level last > time. Divider will hold when SCL transition from 1 to 0 or 0 to 1, if > the transition time is longer than 1 internal pre-divided clock cycle. > The pre-divided clock is divided from I2C module clock, used for > generating SCL. So you will see SCL clock cycle is some way longer than > calculated value using IFDR. > > Transition time will different from rising or falling edge, different > pull-up resistors, and different SCL loading. > > This feature make sure transition time won’t eat both level time of SCL. Thanks for clarification. Does this mean that this feature is used to synchronize between the bus clock and and bitclock? I'll send a documentation patch for this next week to make this clear. Sascha
On 7/11/2012 11:38 AM, Sascha Hauer wrote: > Hi Richard, > > On Wed, Jul 11, 2012 at 02:01:21PM +0800, Richard Zhao wrote: >> IC guys confirmed that the spec is right: >> >> This an adaptive feature of our I2C module may apply to all IMX chips. >> No mistake in the table of RMs. >> >> The divider is designed to guarantee SCL high level and low level last >> time. Divider will hold when SCL transition from 1 to 0 or 0 to 1, if >> the transition time is longer than 1 internal pre-divided clock cycle. >> The pre-divided clock is divided from I2C module clock, used for >> generating SCL. So you will see SCL clock cycle is some way longer than >> calculated value using IFDR. >> >> Transition time will different from rising or falling edge, different >> pull-up resistors, and different SCL loading. >> >> This feature make sure transition time won’t eat both level time of SCL. > Thanks for clarification. Does this mean that this feature is used to > synchronize between the bus clock and and bitclock? > > I'll send a documentation patch for this next week to make this clear. > > Sascha > How does this explain why column 2 matched your measurements, but 1, 3, and 4 didn't. And you tested on 2 different boards. Something doesn't smell right. Just my 2cents Troy
On Wed, Jul 11, 2012 at 12:11:23PM -0700, Troy Kisky wrote: > On 7/11/2012 11:38 AM, Sascha Hauer wrote: > >Hi Richard, > > > >On Wed, Jul 11, 2012 at 02:01:21PM +0800, Richard Zhao wrote: > >>IC guys confirmed that the spec is right: > >> > >>This an adaptive feature of our I2C module may apply to all IMX chips. > >>No mistake in the table of RMs. > >> > >>The divider is designed to guarantee SCL high level and low level last > >>time. Divider will hold when SCL transition from 1 to 0 or 0 to 1, if > >>the transition time is longer than 1 internal pre-divided clock cycle. > >>The pre-divided clock is divided from I2C module clock, used for > >>generating SCL. So you will see SCL clock cycle is some way longer than > >>calculated value using IFDR. > >> > >>Transition time will different from rising or falling edge, different > >>pull-up resistors, and different SCL loading. > >> > >>This feature make sure transition time won’t eat both level time of SCL. > >Thanks for clarification. Does this mean that this feature is used to > >synchronize between the bus clock and and bitclock? > > > >I'll send a documentation patch for this next week to make this clear. > > > >Sascha > > > How does this explain why column 2 matched your measurements, but 1, > 3, and 4 didn't. > And you tested on 2 different boards. > Something doesn't smell right. This reminds me that one divider value on i.MX53 crashed the whole I2C module (it was unusable until reset). The same value worked on i.MX1. Some divider values produced a terrible jitter on the clock output. So it really seems that there is something more going on than just a simple divider. Sascha
On Wed, Jul 11, 2012 at 08:38:38PM +0200, Sascha Hauer wrote: > Hi Richard, > > On Wed, Jul 11, 2012 at 02:01:21PM +0800, Richard Zhao wrote: > > > > IC guys confirmed that the spec is right: > > > > This an adaptive feature of our I2C module may apply to all IMX chips. > > No mistake in the table of RMs. > > > > The divider is designed to guarantee SCL high level and low level last > > time. Divider will hold when SCL transition from 1 to 0 or 0 to 1, if > > the transition time is longer than 1 internal pre-divided clock cycle. > > The pre-divided clock is divided from I2C module clock, used for > > generating SCL. So you will see SCL clock cycle is some way longer than > > calculated value using IFDR. > > > > Transition time will different from rising or falling edge, different > > pull-up resistors, and different SCL loading. > > > > This feature make sure transition time won’t eat both level time of SCL. > > Thanks for clarification. Does this mean that this feature is used to > synchronize between the bus clock and and bitclock? Per my understanding, Not exactly. Divided clock is only used for count time of SCL level hold. This means, even if the i2c bus loading is high (long level setup time), the i2c may still work. Thanks Richard > > I'll send a documentation patch for this next week to make this clear. > > Sascha > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | >
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index 8d6b504..aabbe31 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -98,22 +98,27 @@ * Document Number: MC9328MXLRM, Rev. 5.1, 06/2007 * * Duplicated divider values removed from list + * + * These values (mostly) do not match the ones in the datasheet because + * measurements have shown that the values are wrong. This was tested + * on i.MX1 and i.MX53, so it shoud be safe to assume that the SoCs in + * between behave the same. */ static u16 __initdata i2c_clk_div[50][2] = { - { 22, 0x20 }, { 24, 0x21 }, { 26, 0x22 }, { 28, 0x23 }, - { 30, 0x00 }, { 32, 0x24 }, { 36, 0x25 }, { 40, 0x26 }, - { 42, 0x03 }, { 44, 0x27 }, { 48, 0x28 }, { 52, 0x05 }, - { 56, 0x29 }, { 60, 0x06 }, { 64, 0x2A }, { 72, 0x2B }, - { 80, 0x2C }, { 88, 0x09 }, { 96, 0x2D }, { 104, 0x0A }, - { 112, 0x2E }, { 128, 0x2F }, { 144, 0x0C }, { 160, 0x30 }, - { 192, 0x31 }, { 224, 0x32 }, { 240, 0x0F }, { 256, 0x33 }, - { 288, 0x10 }, { 320, 0x34 }, { 384, 0x35 }, { 448, 0x36 }, - { 480, 0x13 }, { 512, 0x37 }, { 576, 0x14 }, { 640, 0x38 }, - { 768, 0x39 }, { 896, 0x3A }, { 960, 0x17 }, { 1024, 0x3B }, - { 1152, 0x18 }, { 1280, 0x3C }, { 1536, 0x3D }, { 1792, 0x3E }, - { 1920, 0x1B }, { 2048, 0x3F }, { 2304, 0x1C }, { 2560, 0x1D }, - { 3072, 0x1E }, { 3840, 0x1F } + { 30, 0x20 }, { 32, 0x21 }, { 34, 0x22 }, { 36, 0x23 }, + { 38, 0x00 }, { 40, 0x24 }, { 44, 0x25 }, { 48, 0x26 }, + { 50, 0x03 }, { 52, 0x27 }, { 56, 0x28 }, { 60, 0x05 }, + { 64, 0x29 }, { 68, 0x06 }, { 72, 0x2A }, { 80, 0x2B }, + { 88, 0x2C }, { 96, 0x09 }, { 104, 0x2D }, { 112, 0x0A }, + { 120, 0x2E }, { 136, 0x2F }, { 152, 0x0C }, { 168, 0x30 }, + { 200, 0x31 }, { 232, 0x32 }, { 248, 0x0F }, { 264, 0x33 }, + { 288, 0x10 }, { 328, 0x34 }, { 392, 0x35 }, { 456, 0x36 }, + { 480, 0x13 }, { 520, 0x37 }, { 576, 0x14 }, { 648, 0x38 }, + { 776, 0x39 }, { 904, 0x3A }, { 960, 0x17 }, { 1032, 0x3B }, + { 1152, 0x18 }, { 1288, 0x3C }, { 1544, 0x3D }, { 1800, 0x3E }, + { 1920, 0x1B }, { 2048, 0x3F }, { 2304, 0x1C }, { 2560, 0x1D }, + { 3072, 0x1E }, { 3840, 0x1F } }; struct imx_i2c_struct {
Measurements on i.MX1 and i.MX53 have shown that the divider values in the datasheets are wrong. the values from first, third and fourth column were all measured to be 8 higher than in the datasheet. It should be safe to assume that the SoCs between i.MX1 and i.MX53 behave the same as the i2c unit is unchanged since the i.MX1. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/i2c/busses/i2c-imx.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-)