| Message ID | 1411801888-3152-1-git-send-email-addy.ke@rock-chips.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Addy, On Sat, Sep 27, 2014 at 12:11 AM, Addy Ke <addy.ke@rock-chips.com> wrote: > From: Addy <addy.ke@rock-chips.com> > > As show in I2C specification: > - Standard-mode: the minimum HIGH period of the scl clock is 4.0us > the minimum LOW period of the scl clock is 4.7us > - Fast-mode: the minimum HIGH period of the scl clock is 0.6us > the minimum LOW period of the scl clock is 1.3us > > I have measured i2c SCL waveforms in fast-mode by oscilloscope > on rk3288-pinky board. the LOW period of the scl clock is 1.3us. > It is so critical that we must adjust LOW division to increase > the LOW period of the scl clock. > > After put this patch, we can find that min_low_ns is about > 5.4us in Standard-mode and 1.6us in Fast-mode. > > Thanks Doug for the suggestion about division formulas. > > Signed-off-by: Addy <addy.ke@rock-chips.com> > --- > Changes in v2: > - remove Fast-mode plus and HS-mode > - use new formulas suggested by Doug > > drivers/i2c/busses/i2c-rk3x.c | 104 +++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 97 insertions(+), 7 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c > index e637c32..81672a8 100644 > --- a/drivers/i2c/busses/i2c-rk3x.c > +++ b/drivers/i2c/busses/i2c-rk3x.c > @@ -428,19 +428,109 @@ out: > return IRQ_HANDLED; > } > > +static void rk3x_i2c_get_min_ns(unsigned int scl_rate, > + unsigned long *min_low_ns, > + unsigned long *min_high_ns) > +{ > + WARN_ON(scl_rate > 400000); > + > + /* As show in I2C specification: > + * - Standard-mode(100KHz): > + * min_low_ns is 4700ns > + * min_high_ns is 4000ns > + * - Fast-mode(400KHz): > + * min_low_ns is 1300ns > + * min_high_ns is 600ns > + * > + * (min_low_ns + min_high_ns) up to 10000ns in Standard-mode > + * and 2500ns in Fast-mode > + */ > + if (scl_rate <= 100000) { > + *min_low_ns = 4700 + 650; > + *min_high_ns = 4000 + 650; > + } else { > + *min_low_ns = 1300 + 300; > + *min_high_ns = 600 + 300; Wait, where did the extra 650 and 300 come from here? Are you just trying to balance things out? That's not the right way to do things. Please explain what you were trying to accomplish and we can figure out a better way. ...maybe this helped make thins nicer in the clock rates you tried, but I'd bet that I can find clock rates that this is broken for... > + } > +} > + > +static void rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long scl_rate, > + unsigned long *div_low, unsigned long *div_high) > +{ > + unsigned long min_low_ns, min_high_ns, min_total_ns; > + unsigned long min_low_div, min_high_div; > + unsigned long min_total_div, min_div_for_hold; > + unsigned long extra_div, extra_low_div, ideal_low_div; > + unsigned long i2c_rate_khz, scl_rate_khz; > + > + rk3x_i2c_get_min_ns(scl_rate, &min_low_ns, &min_high_ns); > + min_total_ns = min_low_ns + min_high_ns; > + > + /* To avoid from overflow warning */ > + i2c_rate_khz = i2c_rate / 1000; > + scl_rate_khz = scl_rate / 1000; DIV_ROUND_UP? > + > + /*We need the total div to be >= this number > + * so we don't clock too fast. > + */ Please match the commenting style in the rest of the file. That's: /* * A * B * C */ Not: /* A * B * C */ > + min_total_div = DIV_ROUND_UP(i2c_rate_khz, scl_rate_khz * 8); > + > + /* These are the min dividers needed for hold times */ > + min_low_div = DIV_ROUND_UP(i2c_rate_khz * min_low_ns, 8 * 1000000); > + min_high_div = DIV_ROUND_UP(i2c_rate_khz * min_high_ns, 8 * 1000000); > + min_div_for_hold = (min_low_div + min_high_div); > + > + if (min_div_for_hold > min_total_div) { > + /* Time needed to meet hold requirements is important. > + * Just use that > + * */ Extra "*" in "* */" > + *div_low = min_low_div; > + *div_high = min_high_div; > + } else { > + /* We've got to distribute some time among the low and high > + * so we don't run too fast. > + */ > + extra_div = min_total_div - min_div_for_hold; > + /* We'll try to split things up perfectly evenly, > + * biasing slightly towards having a higher div > + * for low (spend more time low). > + */ Last I remember you said that this wasn't going to work quite right and you were just going to assign all of the "extra" to the high part. What changed? > + ideal_low_div = DIV_ROUND_UP(i2c_rate_khz * min_low_ns, > + scl_rate_khz * 8 * min_total_ns); > + /* Handle when the ideal low div is going to take up > + * more than we have. > + */ > + if (ideal_low_div > min_low_div + extra_div) > + ideal_low_div = min_low_div + extra_div; > + /* Give low the "ideal" and give high whatever extra is left.*/ > + extra_low_div = ideal_low_div - min_low_div; > + *div_low = ideal_low_div; > + *div_high > + = min_high_div + (extra_div - extra_low_div); > + } > + > + /* Adjust to the fact that the hardware has an implicit "+1".*/ > + *div_low = *div_low - 1; > + *div_high = *div_high - 1; > +} > + > static void rk3x_i2c_set_scl_rate(struct rk3x_i2c *i2c, unsigned long scl_rate) > { > unsigned long i2c_rate = clk_get_rate(i2c->clk); > - unsigned int div; > + unsigned long div_low, div_high; > > - /* SCL rate = (clk rate) / (8 * DIV) */ > - div = DIV_ROUND_UP(i2c_rate, scl_rate * 8); Strangely this now seems to be based upon an older version of the file. I had to revert (b4a7bd7 i2c: rk3x: fix divisor calculation for SCL frequency) in order to apply v2. Please submit your code against the latest accepted code. > + /* The formulas in rk3x TRM: > + * - T_scl_high = T_clk * (divh + 1) * 8 > + * - T_scl_low = T_clk * (divl + 1) * 8 > + * */ > + rk3x_i2c_calc_divs(i2c_rate, scl_rate, &div_low, &div_high); > > - /* The lower and upper half of the CLKDIV reg describe the length of > - * SCL low & high periods. */ > - div = DIV_ROUND_UP(div, 2); > + i2c_writel(i2c, (div_high << 16) | (div_low & 0xffff), REG_CLKDIV); > > - i2c_writel(i2c, (div << 16) | (div & 0xffff), REG_CLKDIV); > + dev_dbg(i2c->dev, "scl_ns: %luns, scl_low_ns: %luns, scl_high_ns: %luns\n", > + 1000000000/scl_rate, > + (1000000000 / i2c_rate) * (div_low + 1) * 8, > + (1000000000 / i2c_rate) * (div_high + 1) * 8); > } > > /** > -- > 1.8.3.2 > > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Doug On 2014/9/29 12:39, Doug Anderson wrote: > Addy, > > On Sat, Sep 27, 2014 at 12:11 AM, Addy Ke <addy.ke@rock-chips.com> wrote: >> From: Addy <addy.ke@rock-chips.com> >> >> As show in I2C specification: >> - Standard-mode: the minimum HIGH period of the scl clock is 4.0us >> the minimum LOW period of the scl clock is 4.7us >> - Fast-mode: the minimum HIGH period of the scl clock is 0.6us >> the minimum LOW period of the scl clock is 1.3us >> >> I have measured i2c SCL waveforms in fast-mode by oscilloscope >> on rk3288-pinky board. the LOW period of the scl clock is 1.3us. >> It is so critical that we must adjust LOW division to increase >> the LOW period of the scl clock. >> >> After put this patch, we can find that min_low_ns is about >> 5.4us in Standard-mode and 1.6us in Fast-mode. >> >> Thanks Doug for the suggestion about division formulas. >> >> Signed-off-by: Addy <addy.ke@rock-chips.com> >> --- >> Changes in v2: >> - remove Fast-mode plus and HS-mode >> - use new formulas suggested by Doug >> >> drivers/i2c/busses/i2c-rk3x.c | 104 +++++++++++++++++++++++++++++++++++++++--- >> 1 file changed, 97 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c >> index e637c32..81672a8 100644 >> --- a/drivers/i2c/busses/i2c-rk3x.c >> +++ b/drivers/i2c/busses/i2c-rk3x.c >> @@ -428,19 +428,109 @@ out: >> return IRQ_HANDLED; >> } >> >> +static void rk3x_i2c_get_min_ns(unsigned int scl_rate, >> + unsigned long *min_low_ns, >> + unsigned long *min_high_ns) >> +{ >> + WARN_ON(scl_rate > 400000); >> + >> + /* As show in I2C specification: >> + * - Standard-mode(100KHz): >> + * min_low_ns is 4700ns >> + * min_high_ns is 4000ns >> + * - Fast-mode(400KHz): >> + * min_low_ns is 1300ns >> + * min_high_ns is 600ns >> + * >> + * (min_low_ns + min_high_ns) up to 10000ns in Standard-mode >> + * and 2500ns in Fast-mode >> + */ >> + if (scl_rate <= 100000) { >> + *min_low_ns = 4700 + 650; >> + *min_high_ns = 4000 + 650; >> + } else { >> + *min_low_ns = 1300 + 300; >> + *min_high_ns = 600 + 300; > > Wait, where did the extra 650 and 300 come from here? Are you just > trying to balance things out? That's not the right way to do things. > Please explain what you were trying to accomplish and we can figure > out a better way. > > ...maybe this helped make thins nicer in the clock rates you tried, > but I'd bet that I can find clock rates that this is broken for... > > 650 = [(1000000000/100K) ns - min_low_ns_100k - min_high_ns_100k] / 2 300 = [(1000000000/300K) ns - min_low_ns_400k - min_high_ns_400k] / 2 if there is no extra 650 and 300: extra_div is 3 in fast-mode and 11 in standard-mode. 1)if all of the "extra" is asssigned to the high part, the LOW period is too close to min_low. In my test: 400k, scl_low_ns: 1440ns, scl_high_ns: 1120ns 100k, scl_low_ns: 4800ns, scl_high_ns: 5120ns 2)if not, dato hold time is too close to min_hold_time(400khz, 900ns) In my test: 400k, scl_low_ns: 1760ns, scl_high_ns: 800ns 100k, scl_low_ns: 5304ns, scl_high_ns: 4368ns hold_time_ns ~= scl_low_ns / 2 = 880ns So I think we must decrease min_low_ns:min_high_ns to decrease scl_low in case 2 So I set the extra 650 and 300. After this, in my test: 400k, scl_low_ns: 1600ns, scl_high_ns: 960ns 100k, scl_low_ns: 5200ns, scl_hign_ns: 4576ns I think this value is more reasonable. ps: min_low_ns/min_high_ns > (min_low_ns + extra) / (min_high_ns + extra) (if min_low_ns > min_high_ns) >> + } >> +} >> + >> +static void rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long scl_rate, >> + unsigned long *div_low, unsigned long *div_high) >> +{ >> + unsigned long min_low_ns, min_high_ns, min_total_ns; >> + unsigned long min_low_div, min_high_div; >> + unsigned long min_total_div, min_div_for_hold; >> + unsigned long extra_div, extra_low_div, ideal_low_div; >> + unsigned long i2c_rate_khz, scl_rate_khz; >> + >> + rk3x_i2c_get_min_ns(scl_rate, &min_low_ns, &min_high_ns); >> + min_total_ns = min_low_ns + min_high_ns; >> + >> + /* To avoid from overflow warning */ >> + i2c_rate_khz = i2c_rate / 1000; >> + scl_rate_khz = scl_rate / 1000; > > DIV_ROUND_UP? > >> + >> + /*We need the total div to be >= this number >> + * so we don't clock too fast. >> + */ > > Please match the commenting style in the rest of the file. That's: > > /* > * A > * B > * C > */ > > Not: > > /* A > * B > * C > */ > >> + min_total_div = DIV_ROUND_UP(i2c_rate_khz, scl_rate_khz * 8); >> + >> + /* These are the min dividers needed for hold times */ >> + min_low_div = DIV_ROUND_UP(i2c_rate_khz * min_low_ns, 8 * 1000000); >> + min_high_div = DIV_ROUND_UP(i2c_rate_khz * min_high_ns, 8 * 1000000); >> + min_div_for_hold = (min_low_div + min_high_div); >> + >> + if (min_div_for_hold > min_total_div) { >> + /* Time needed to meet hold requirements is important. >> + * Just use that >> + * */ > > Extra "*" in "* */" > > >> + *div_low = min_low_div; >> + *div_high = min_high_div; >> + } else { >> + /* We've got to distribute some time among the low and high >> + * so we don't run too fast. >> + */ >> + extra_div = min_total_div - min_div_for_hold; >> + /* We'll try to split things up perfectly evenly, >> + * biasing slightly towards having a higher div >> + * for low (spend more time low). >> + */ > > Last I remember you said that this wasn't going to work quite right > and you were just going to assign all of the "extra" to the high part. > What changed? if all of the "extra" is asssigned to the high part, the LOW period is too close to min_low. As shown in the above. > > >> + ideal_low_div = DIV_ROUND_UP(i2c_rate_khz * min_low_ns, >> + scl_rate_khz * 8 * min_total_ns); >> + /* Handle when the ideal low div is going to take up >> + * more than we have. >> + */ >> + if (ideal_low_div > min_low_div + extra_div) >> + ideal_low_div = min_low_div + extra_div; >> + /* Give low the "ideal" and give high whatever extra is left.*/ >> + extra_low_div = ideal_low_div - min_low_div; >> + *div_low = ideal_low_div; >> + *div_high >> + = min_high_div + (extra_div - extra_low_div); >> + } >> + >> + /* Adjust to the fact that the hardware has an implicit "+1".*/ >> + *div_low = *div_low - 1; >> + *div_high = *div_high - 1; >> +} >> + >> static void rk3x_i2c_set_scl_rate(struct rk3x_i2c *i2c, unsigned long scl_rate) >> { >> unsigned long i2c_rate = clk_get_rate(i2c->clk); >> - unsigned int div; >> + unsigned long div_low, div_high; >> >> - /* SCL rate = (clk rate) / (8 * DIV) */ >> - div = DIV_ROUND_UP(i2c_rate, scl_rate * 8); > > Strangely this now seems to be based upon an older version of the > file. I had to revert (b4a7bd7 i2c: rk3x: fix divisor calculation for > SCL frequency) in order to apply v2. Please submit your code against > the latest accepted code. > > This patch is based on ../kernel/git/wsa/linux.git, master branch. It is my mistake. I will checkout for_next branch. Thank you. >> + /* The formulas in rk3x TRM: >> + * - T_scl_high = T_clk * (divh + 1) * 8 >> + * - T_scl_low = T_clk * (divl + 1) * 8 >> + * */ >> + rk3x_i2c_calc_divs(i2c_rate, scl_rate, &div_low, &div_high); >> >> - /* The lower and upper half of the CLKDIV reg describe the length of >> - * SCL low & high periods. */ >> - div = DIV_ROUND_UP(div, 2); >> + i2c_writel(i2c, (div_high << 16) | (div_low & 0xffff), REG_CLKDIV); >> >> - i2c_writel(i2c, (div << 16) | (div & 0xffff), REG_CLKDIV); >> + dev_dbg(i2c->dev, "scl_ns: %luns, scl_low_ns: %luns, scl_high_ns: %luns\n", >> + 1000000000/scl_rate, >> + (1000000000 / i2c_rate) * (div_low + 1) * 8, >> + (1000000000 / i2c_rate) * (div_high + 1) * 8); >> } >> >> /** >> -- >> 1.8.3.2 >> >> > > > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Addy, On Mon, Sep 29, 2014 at 12:45 AM, addy ke <addy.ke@rock-chips.com> wrote: > Hi Doug > > On 2014/9/29 12:39, Doug Anderson wrote: >> Addy, >> >> On Sat, Sep 27, 2014 at 12:11 AM, Addy Ke <addy.ke@rock-chips.com> wrote: >>> From: Addy <addy.ke@rock-chips.com> >>> >>> As show in I2C specification: >>> - Standard-mode: the minimum HIGH period of the scl clock is 4.0us >>> the minimum LOW period of the scl clock is 4.7us >>> - Fast-mode: the minimum HIGH period of the scl clock is 0.6us >>> the minimum LOW period of the scl clock is 1.3us >>> >>> I have measured i2c SCL waveforms in fast-mode by oscilloscope >>> on rk3288-pinky board. the LOW period of the scl clock is 1.3us. >>> It is so critical that we must adjust LOW division to increase >>> the LOW period of the scl clock. >>> >>> After put this patch, we can find that min_low_ns is about >>> 5.4us in Standard-mode and 1.6us in Fast-mode. >>> >>> Thanks Doug for the suggestion about division formulas. >>> >>> Signed-off-by: Addy <addy.ke@rock-chips.com> >>> --- >>> Changes in v2: >>> - remove Fast-mode plus and HS-mode >>> - use new formulas suggested by Doug >>> >>> drivers/i2c/busses/i2c-rk3x.c | 104 +++++++++++++++++++++++++++++++++++++++--- >>> 1 file changed, 97 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c >>> index e637c32..81672a8 100644 >>> --- a/drivers/i2c/busses/i2c-rk3x.c >>> +++ b/drivers/i2c/busses/i2c-rk3x.c >>> @@ -428,19 +428,109 @@ out: >>> return IRQ_HANDLED; >>> } >>> >>> +static void rk3x_i2c_get_min_ns(unsigned int scl_rate, >>> + unsigned long *min_low_ns, >>> + unsigned long *min_high_ns) >>> +{ >>> + WARN_ON(scl_rate > 400000); >>> + >>> + /* As show in I2C specification: >>> + * - Standard-mode(100KHz): >>> + * min_low_ns is 4700ns >>> + * min_high_ns is 4000ns >>> + * - Fast-mode(400KHz): >>> + * min_low_ns is 1300ns >>> + * min_high_ns is 600ns >>> + * >>> + * (min_low_ns + min_high_ns) up to 10000ns in Standard-mode >>> + * and 2500ns in Fast-mode >>> + */ >>> + if (scl_rate <= 100000) { >>> + *min_low_ns = 4700 + 650; >>> + *min_high_ns = 4000 + 650; >>> + } else { >>> + *min_low_ns = 1300 + 300; >>> + *min_high_ns = 600 + 300; >> >> Wait, where did the extra 650 and 300 come from here? Are you just >> trying to balance things out? That's not the right way to do things. >> Please explain what you were trying to accomplish and we can figure >> out a better way. >> >> ...maybe this helped make thins nicer in the clock rates you tried, >> but I'd bet that I can find clock rates that this is broken for... >> >> > 650 = [(1000000000/100K) ns - min_low_ns_100k - min_high_ns_100k] / 2 > 300 = [(1000000000/300K) ns - min_low_ns_400k - min_high_ns_400k] / 2 > > if there is no extra 650 and 300: > extra_div is 3 in fast-mode and 11 in standard-mode. Umm, OK. Except that "min_low_ns" is no longer the actual minimum "ns" we need according to the spec. That will mean that you're sometimes going to end up with a clock rate that's slower than you want. > 1)if all of the "extra" is asssigned to the high part, the LOW period is too close to min_low. > In my test: > 400k, scl_low_ns: 1440ns, scl_high_ns: 1120ns > 100k, scl_low_ns: 4800ns, scl_high_ns: 5120ns Why does it matter if it's close to "min_low"? It's above the minimum so it's fine, right? If you need a buffer, add an explicit buffer to the calculations. > 2)if not, dato hold time is too close to min_hold_time(400khz, 900ns) > In my test: > 400k, scl_low_ns: 1760ns, scl_high_ns: 800ns > 100k, scl_low_ns: 5304ns, scl_high_ns: 4368ns > > hold_time_ns ~= scl_low_ns / 2 = 880ns It's still under, though. Why does this matter? Again, if you need a buffer then add a buffer. > So I think we must decrease min_low_ns:min_high_ns to decrease scl_low in case 2 > > So I set the extra 650 and 300. > > After this, in my test: > 400k, scl_low_ns: 1600ns, scl_high_ns: 960ns > 100k, scl_low_ns: 5200ns, scl_hign_ns: 4576ns > > I think this value is more reasonable. You're testing with a very particular clock input rate. The function needs to be general and work across a lot of different input clock rates. That's why my python code has loops in it to test the results over a huge variety of input clock rates... --- Here are a few examples of differences (SEP 30 is my new proposal below): With this example your code produces a slightly slower clock rate than desirable: SEP 30: CLK = 74250000, Req = 100000, act = 99798.39, 5.49 us low, 4.53 us high, low/high = 1.21 (50 41) AddyV2: CLK = 74250000, Req = 100000, act = 98736.70, 5.39 us low, 4.74 us high, low/high = 1.14 (49 43) Here's another example where your patch doesn't produce ideal timings (in this case the difference is more pronounced) SEP 30: CLK = 66380000, Req = 400000, act = 395119.05, 1.69 us low, 0.84 us high, low/high = 2.00 (13 6) AddyV2: CLK = 66380000, Req = 400000, act = 377159.09, 1.69 us low, 0.96 us high, low/high = 1.75 (13 7) Here's an example your code violates timings (true, a 1.61MHz clock isn't super realistic in rk3288, but it does show a non-ideal case): SEP 30: CLK = 1610000, Req = 100000, act = 67083.33, 4.97 us low, 9.94 us high, low/high = 0.50 (0 1) AddyV2: CLK = 1610000, Req = 100000, act = 67083.33, 9.94 us low, 4.97 us high, low/high = 2.00 (1 0) ...ERROR: low too long 9.94 us > 6.85 us (/2 4.97 us > 3.42 us) (conflicting=0) --- I'll paste new code here with a proposal that adds a buffer (could be 0) and also send you an attachment in a separate email (I can't quite believe that the lists will handle attachments). One thing you'll notice is that with sufficiently slow input clocks it's possible to get into a state where we can't meet both the minimum and maximum times for SCL being low. This probably isn't super realistic, but the code could certainly have a warning. --- def DIV_ROUND_UP(a, b): return (a + b - 1) // b # This is bassed on v2 sent by Addy (fixed DIV_ROUND_UP) def test_it_addy_v2(min_low_ns, min_high_ns, max_low_ns, i2c_rate, scl_rate): # Add the buffer in suggested by addy if min_low_ns == 4700: min_low_ns += 650 min_high_ns += 650 else: min_low_ns += 300 min_high_ns += 300 min_total_ns = min_low_ns + min_high_ns # Adjust to avoid overflow i2c_rate_khz = DIV_ROUND_UP(i2c_rate, 1000) scl_rate_khz = DIV_ROUND_UP(scl_rate, 1000) # We need the total div to be >= this number so we don't clock too fast. min_total_div = DIV_ROUND_UP(i2c_rate_khz, scl_rate_khz * 8); # These are the min dividers needed for min hold times. min_low_div = DIV_ROUND_UP(i2c_rate_khz * min_low_ns, 8 * 1000000) min_high_div = DIV_ROUND_UP(i2c_rate_khz * min_high_ns, 8 * 1000000) min_div_for_hold = (min_low_div + min_high_div) if min_div_for_hold > min_total_div: # Time needed to meet hold requirements is important. Just use that div_low = min_low_div div_high = min_high_div else: # We've got to distribute some time among the low and high so we # don't run too fast. extra_div = min_total_div - min_div_for_hold # We'll try to split things up perfectly evenly, biasing slightly # towards having a higher div for low (spend more time low). ideal_low_div = DIV_ROUND_UP(i2c_rate_khz * min_low_ns, scl_rate_khz * 8 * min_total_ns) # Handle when the ideal low div is going to take up more than we have if ideal_low_div > min_low_div + extra_div: assert ideal_low_div == min_low_div + extra_div + 1 ideal_low_div = min_low_div + extra_div # Give low the "ideal" and give high whatever extra is left. div_low = ideal_low_div div_high = min_high_div + (extra_div - (ideal_low_div - min_low_div)) # Adjust to the fact that the hardware has an implicit "+1". # NOTE: Above calculations always produce div_low > 0 and div_high > 0. div_low -= 1 div_high -= 1 return div_low, div_high, False # Note: we don't calculate "conflicting" # test_it_sep30 - Sept 30th proposal for i2c stuff # # min_low_ns: The minimum number of ns we need to hold low to meet i2c spec # min_high_ns: The minimum number of ns we need to hold high to meet i2c spec # max_low_ns: The maximum number of ns we can hold low to meet i2c spec # # Note: max_low_ns should be (max data hold time * 2 - buffer) # This is because the i2c host on Rockchip holds the data line for # half the low time. def test_it_sep30(min_low_ns, min_high_ns, max_low_ns, i2c_rate, scl_rate): min_total_ns = min_low_ns + min_high_ns # Adjust to avoid overflow i2c_rate_khz = DIV_ROUND_UP(i2c_rate, 1000) scl_rate_khz = DIV_ROUND_UP(scl_rate, 1000) # We need the total div to be >= this number so we don't clock too fast. min_total_div = DIV_ROUND_UP(i2c_rate_khz, scl_rate_khz * 8); # These are the min dividers needed for min hold times. min_low_div = DIV_ROUND_UP(i2c_rate_khz * min_low_ns, 8 * 1000000) min_high_div = DIV_ROUND_UP(i2c_rate_khz * min_high_ns, 8 * 1000000) min_div_for_hold = (min_low_div + min_high_div) # This is the maximum divider so we don't go over the max. We don't round up # here (we round down) since this is a max. max_low_div = i2c_rate_khz * max_low_ns / (8 * 1000000) # Giving different rounding, it's possible that min > max (!?!) We're # totally out of luck in that case, so let's go with getting clocks # right I guess... if min_low_div > max_low_div: # print "WARNING: conflicting restrictions" conflicting = True max_low_div = min_low_div else: conflicting = False if min_div_for_hold > min_total_div: # Time needed to meet hold requirements is important. Just use that div_low = min_low_div div_high = min_high_div else: # We've got to distribute some time among the low and high so we # don't run too fast. extra_div = min_total_div - min_div_for_hold # We'll try to split things up perfectly evenly, biasing slightly # towards having a higher div for low (spend more time low). ideal_low_div = DIV_ROUND_UP(i2c_rate_khz * min_low_ns, scl_rate_khz * 8 * min_total_ns) # Don't allow it to go over the max if ideal_low_div > max_low_div: ideal_low_div = max_low_div # Handle when the ideal low div is going to take up more than we have if ideal_low_div > min_low_div + extra_div: assert ideal_low_div == min_low_div + extra_div + 1 ideal_low_div = min_low_div + extra_div # Give low the "ideal" and give high whatever extra is left. div_low = ideal_low_div div_high = min_high_div + (extra_div - (ideal_low_div - min_low_div)) # Adjust to the fact that the hardware has an implicit "+1". # NOTE: Above calculations always produce div_low > 0 and div_high > 0. div_low -= 1 div_high -= 1 return div_low, div_high, conflicting def print_it(label, min_low_ns, min_high_ns, max_low_ns, i2c_rate, scl_rate, div_low, div_high, conflicting): T_pclk_us = 1000000. / i2c_rate T_sclk_us = 1000000. / scl_rate T_low_us = T_pclk_us * (div_low + 1) * 8 T_high_us = T_pclk_us * (div_high + 1) * 8 T_tot_us = (T_high_us + T_low_us) freq = 1000000. / T_tot_us print "%s: CLK = %d, Req = %d, act = %.2f, %.2f us low, " \ "%.2f us high, low/high = %.2f (%d %d)" % ( label, i2c_rate, scl_rate, freq, T_low_us, T_high_us, T_low_us / T_high_us, div_low, div_high) if T_low_us * 1000 < min_low_ns: print "...ERROR: not low long enough" if T_high_us * 1000 < min_high_ns: print "...ERROR: not high long enough" if T_low_us * 1000 > max_low_ns: print "...ERROR: low too long %.2f us > %.2f us " \ "(/2 %.2f us > %.2f us) (conflicting=%d)" % ( T_low_us, max_low_ns / 1000., T_low_us / 2., max_low_ns / 2000., conflicting) elif conflicting: print "...Conflicting, but not low too long?" return (i2c_rate, scl_rate, freq, T_low_us, T_high_us) def test_it(min_low_ns, min_high_ns, max_low_ns, i2c_rate, scl_rate): sep30 = test_it_sep30(min_low_ns, min_high_ns, max_low_ns, i2c_rate, scl_rate) addy_v2 = test_it_addy_v2(min_low_ns, min_high_ns, max_low_ns, i2c_rate, scl_rate) print_it("SEP 30", min_low_ns, min_high_ns, max_low_ns, i2c_rate, scl_rate, sep30[0], sep30[1], sep30[2]) print_it("AddyV2", min_low_ns, min_high_ns, max_low_ns, i2c_rate, scl_rate, addy_v2[0], addy_v2[1], addy_v2[2]) # Test to see where results are differet #if (sep30[0], sep30[1]) != (addy_v2[0], addy_v2[1]): # print_it("SEP 30", min_low_ns, min_high_ns, max_low_ns, # i2c_rate, scl_rate, sep30[0], sep30[1], sep30[2]) # print_it("AddyV2", min_low_ns, min_high_ns, max_low_ns, # i2c_rate, scl_rate, addy_v2[0], addy_v2[1], addy_v2[2]) # Test to see where clock rates are different. #if (sep30[0] + sep30[1]) != (addy_v2[0] + addy_v2[1]): # print_it("SEP 30", min_low_ns, min_high_ns, max_low_ns, # i2c_rate, scl_rate, sep30[0], sep30[1], sep30[2]) # print_it("AddyV2", min_low_ns, min_high_ns, max_low_ns, # i2c_rate, scl_rate, addy_v2[0], addy_v2[1], addy_v2[2]) min_low_std_ns = 4700 min_high_std_ns = 4000 max_data_hold_std_ns = 3450 data_hold_buffer_std_ns = 50 min_low_fast_ns = 1300 min_high_fast_ns = 600 max_data_hold_fast_ns = 900 data_hold_buffer_fast_ns = 50 test_it(min_low_std_ns, min_high_std_ns, max_data_hold_std_ns * 2 - data_hold_buffer_std_ns, 74250000, 100000) test_it(min_low_std_ns, min_high_std_ns, max_data_hold_std_ns * 2 - data_hold_buffer_std_ns, 2000001, 100000) test_it(min_low_std_ns, min_high_std_ns, max_data_hold_std_ns * 2 - data_hold_buffer_std_ns, 1610000, 100000) test_it(min_low_std_ns, min_high_std_ns, max_data_hold_std_ns * 2 - data_hold_buffer_std_ns, 1484000, 100000) test_it(min_low_fast_ns, min_high_fast_ns, max_data_hold_fast_ns * 2 - data_hold_buffer_fast_ns, 66380000, 400000) test_it(min_low_fast_ns, min_high_fast_ns, max_data_hold_fast_ns * 2 - data_hold_buffer_fast_ns, 74250000, 400000) test_it(min_low_fast_ns, min_high_fast_ns, max_data_hold_fast_ns * 2 - data_hold_buffer_fast_ns, 12800000, 400000) test_it(min_low_fast_ns, min_high_fast_ns, max_data_hold_fast_ns * 2 - data_hold_buffer_fast_ns, 9400000, 400000) test_it(min_low_fast_ns, min_high_fast_ns, max_data_hold_fast_ns * 2 - data_hold_buffer_fast_ns, 6400000, 400000) test_it(min_low_fast_ns, min_high_fast_ns, max_data_hold_fast_ns * 2 - data_hold_buffer_fast_ns, 5000000, 400000) test_it(min_low_fast_ns, min_high_fast_ns, max_data_hold_fast_ns * 2 - data_hold_buffer_fast_ns, 3200000, 400000) test_it(min_low_fast_ns, min_high_fast_ns, max_data_hold_fast_ns * 2 - data_hold_buffer_fast_ns, 1600000, 400000) test_it(min_low_fast_ns, min_high_fast_ns, max_data_hold_fast_ns * 2 - data_hold_buffer_fast_ns, 800000, 400000) #for i in xrange(74250000, 800000, -100): # test_it(min_low_std_ns, min_high_std_ns, # max_data_hold_std_ns * 2 - data_hold_buffer_std_ns, i, 100000) # #for i in xrange(74250000, 800000, -100): # test_it(min_low_fast_ns, min_high_fast_ns, # max_data_hold_fast_ns * 2 - data_hold_buffer_fast_ns, i, 400000) --- -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c index e637c32..81672a8 100644 --- a/drivers/i2c/busses/i2c-rk3x.c +++ b/drivers/i2c/busses/i2c-rk3x.c @@ -428,19 +428,109 @@ out: return IRQ_HANDLED; } +static void rk3x_i2c_get_min_ns(unsigned int scl_rate, + unsigned long *min_low_ns, + unsigned long *min_high_ns) +{ + WARN_ON(scl_rate > 400000); + + /* As show in I2C specification: + * - Standard-mode(100KHz): + * min_low_ns is 4700ns + * min_high_ns is 4000ns + * - Fast-mode(400KHz): + * min_low_ns is 1300ns + * min_high_ns is 600ns + * + * (min_low_ns + min_high_ns) up to 10000ns in Standard-mode + * and 2500ns in Fast-mode + */ + if (scl_rate <= 100000) { + *min_low_ns = 4700 + 650; + *min_high_ns = 4000 + 650; + } else { + *min_low_ns = 1300 + 300; + *min_high_ns = 600 + 300; + } +} + +static void rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long scl_rate, + unsigned long *div_low, unsigned long *div_high) +{ + unsigned long min_low_ns, min_high_ns, min_total_ns; + unsigned long min_low_div, min_high_div; + unsigned long min_total_div, min_div_for_hold; + unsigned long extra_div, extra_low_div, ideal_low_div; + unsigned long i2c_rate_khz, scl_rate_khz; + + rk3x_i2c_get_min_ns(scl_rate, &min_low_ns, &min_high_ns); + min_total_ns = min_low_ns + min_high_ns; + + /* To avoid from overflow warning */ + i2c_rate_khz = i2c_rate / 1000; + scl_rate_khz = scl_rate / 1000; + + /*We need the total div to be >= this number + * so we don't clock too fast. + */ + min_total_div = DIV_ROUND_UP(i2c_rate_khz, scl_rate_khz * 8); + + /* These are the min dividers needed for hold times */ + min_low_div = DIV_ROUND_UP(i2c_rate_khz * min_low_ns, 8 * 1000000); + min_high_div = DIV_ROUND_UP(i2c_rate_khz * min_high_ns, 8 * 1000000); + min_div_for_hold = (min_low_div + min_high_div); + + if (min_div_for_hold > min_total_div) { + /* Time needed to meet hold requirements is important. + * Just use that + * */ + *div_low = min_low_div; + *div_high = min_high_div; + } else { + /* We've got to distribute some time among the low and high + * so we don't run too fast. + */ + extra_div = min_total_div - min_div_for_hold; + /* We'll try to split things up perfectly evenly, + * biasing slightly towards having a higher div + * for low (spend more time low). + */ + ideal_low_div = DIV_ROUND_UP(i2c_rate_khz * min_low_ns, + scl_rate_khz * 8 * min_total_ns); + /* Handle when the ideal low div is going to take up + * more than we have. + */ + if (ideal_low_div > min_low_div + extra_div) + ideal_low_div = min_low_div + extra_div; + /* Give low the "ideal" and give high whatever extra is left.*/ + extra_low_div = ideal_low_div - min_low_div; + *div_low = ideal_low_div; + *div_high + = min_high_div + (extra_div - extra_low_div); + } + + /* Adjust to the fact that the hardware has an implicit "+1".*/ + *div_low = *div_low - 1; + *div_high = *div_high - 1; +} + static void rk3x_i2c_set_scl_rate(struct rk3x_i2c *i2c, unsigned long scl_rate) { unsigned long i2c_rate = clk_get_rate(i2c->clk); - unsigned int div; + unsigned long div_low, div_high; - /* SCL rate = (clk rate) / (8 * DIV) */ - div = DIV_ROUND_UP(i2c_rate, scl_rate * 8); + /* The formulas in rk3x TRM: + * - T_scl_high = T_clk * (divh + 1) * 8 + * - T_scl_low = T_clk * (divl + 1) * 8 + * */ + rk3x_i2c_calc_divs(i2c_rate, scl_rate, &div_low, &div_high); - /* The lower and upper half of the CLKDIV reg describe the length of - * SCL low & high periods. */ - div = DIV_ROUND_UP(div, 2); + i2c_writel(i2c, (div_high << 16) | (div_low & 0xffff), REG_CLKDIV); - i2c_writel(i2c, (div << 16) | (div & 0xffff), REG_CLKDIV); + dev_dbg(i2c->dev, "scl_ns: %luns, scl_low_ns: %luns, scl_high_ns: %luns\n", + 1000000000/scl_rate, + (1000000000 / i2c_rate) * (div_low + 1) * 8, + (1000000000 / i2c_rate) * (div_high + 1) * 8); } /**