Re: [PATCH] i2c: rk3x: adjust the LOW divison based on characteristics of SCL
Addy, On Wed, Sep 24, 2014 at 9:36 PM, Doug Anderson diand...@chromium.org wrote: Addy, On Wed, Sep 24, 2014 at 6:56 PM, addy ke addy...@rock-chips.com wrote: In my measurement,all paramter but Data hold time are match the characteristics of SCL bus line. the measured value is 0.928us(data hold time on RK3X ~= the low period / 2) but the maximum value described in table is 0.9us About Data hold time, there are described in I2C specification: - for CBUS compatible masters for I2C-bus deivices - the maximum data hold time has only be met if the device does not stretch the LOW period of the SCL signal. I have tested on RK3288-Pinky board, there are no error. But I don't known whether this paramter will affect i2c communications. I'll have to spend more time tomorrow to really understand this, but if changing the code to bias towards slightly longer high times instead of low times helps fix it then that's fine with me. So what you're saying is that you're seeing a case where the clock goes low and the data is not valid until .928us. Is this data that is being driven by the master or data that is being driven by the slave? Do you know why the data takes so long to be valid? Maybe you can email me some of the waveforms and I can try to help you debug it. In any case it sounds like the the data hold time problem is unrelated to the clock ratio problem (right?), so maybe you could send out patch v2? -Doug P.S. I checked the Rockchip TRM and it claims 400kHz maximum i2c. I think that means you can just remove all of the fast mode plus and high speed mode clock rates from your table. -- 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: rk3x: adjust the LOW divison based on characteristics of SCL
Addy, On Thu, Sep 25, 2014 at 6:40 PM, addy ke addy...@rock-chips.com wrote: Hi, Doug On 2014/9/26 5:52, Doug Anderson wrote: Addy, On Wed, Sep 24, 2014 at 9:36 PM, Doug Anderson diand...@chromium.org wrote: Addy, On Wed, Sep 24, 2014 at 6:56 PM, addy ke addy...@rock-chips.com wrote: In my measurement,all paramter but Data hold time are match the characteristics of SCL bus line. the measured value is 0.928us(data hold time on RK3X ~= the low period / 2) but the maximum value described in table is 0.9us About Data hold time, there are described in I2C specification: - for CBUS compatible masters for I2C-bus deivices - the maximum data hold time has only be met if the device does not stretch the LOW period of the SCL signal. I have tested on RK3288-Pinky board, there are no error. But I don't known whether this paramter will affect i2c communications. I'll have to spend more time tomorrow to really understand this, but if changing the code to bias towards slightly longer high times instead of low times helps fix it then that's fine with me. So what you're saying is that you're seeing a case where the clock goes low and the data is not valid until .928us. Is this data that is being driven by the master or data that is being driven by the slave? It is driven by the master and will be release at half of LOW period in our IC design. Ah, I didn't know this. Is that in the TRM somewhere? ...so does it work to just replace: ideal_low_div = DIV_ROUND_UP(clk_rate * min_low_ns, scl_rate * 8 * min_total_ns) ...with: ideal_low_div = min_low_div That will assign all extra time to the high part. -Doug -- 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: rk3x: adjust the LOW divison based on characteristics of SCL
On 2014/9/26 10:08, Doug Anderson wrote: Addy, On Thu, Sep 25, 2014 at 6:40 PM, addy ke addy...@rock-chips.com wrote: Hi, Doug On 2014/9/26 5:52, Doug Anderson wrote: Addy, On Wed, Sep 24, 2014 at 9:36 PM, Doug Anderson diand...@chromium.org wrote: Addy, On Wed, Sep 24, 2014 at 6:56 PM, addy ke addy...@rock-chips.com wrote: In my measurement,all paramter but Data hold time are match the characteristics of SCL bus line. the measured value is 0.928us(data hold time on RK3X ~= the low period / 2) but the maximum value described in table is 0.9us About Data hold time, there are described in I2C specification: - for CBUS compatible masters for I2C-bus deivices - the maximum data hold time has only be met if the device does not stretch the LOW period of the SCL signal. I have tested on RK3288-Pinky board, there are no error. But I don't known whether this paramter will affect i2c communications. I'll have to spend more time tomorrow to really understand this, but if changing the code to bias towards slightly longer high times instead of low times helps fix it then that's fine with me. So what you're saying is that you're seeing a case where the clock goes low and the data is not valid until .928us. Is this data that is being driven by the master or data that is being driven by the slave? It is driven by the master and will be release at half of LOW period in our IC design. Ah, I didn't know this. Is that in the TRM somewhere? No, it was my test and confirmed by IC engineer. You can find that there is a pulse at half of LOW period of the ninth SCL clock each bytes. ...so does it work to just replace: ideal_low_div = DIV_ROUND_UP(clk_rate * min_low_ns, scl_rate * 8 * min_total_ns) ...with: ideal_low_div = min_low_div That will assign all extra time to the high part. I Think it is more reasonable. -Doug -- 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: rk3x: adjust the LOW divison based on characteristics of SCL
On 2014/9/24 12:10, Doug Anderson wrote:
Addy,
On Tue, Sep 23, 2014 at 6:55 PM, Addy Ke addy...@rock-chips.com wrote:
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
- Fast-mode plus:
the minimum HIGH period of the scl clock is 0.26us
the minimum LOW period of the scl clock is 0.5us
- HS-mode(1.7MHz):
the minimum HIGH period of the scl clock is 0.12us
the minimum LOW period of the scl clock is 0.32us
- HS-mode(3.4MHz):
the minimum HIGH period of the scl clock is 0.06us
the minimum LOW period of the scl clock is 0.16us
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.
Thanks Doug for the suggestion about division formula.
Signed-off-by: Addy Ke addy...@rock-chips.com
---
drivers/i2c/busses/i2c-rk3x.c | 79
+++
1 file changed, 72 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 93cfc83..49d67b7 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -428,18 +428,83 @@ out:
return IRQ_HANDLED;
}
+static void rk3x_i2c_get_ratios(unsigned long scl_rate,
+ unsigned long *high_ratio,
+ unsigned long *low_ratio)
+{
+ /* 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
+* - Fast-mode plus:
+* the minimum HIGH period of the scl clock is 0.26us
+* the minimum LOW period of the scl clock is 0.5us
+* - HS-mode(1.7MHz):
+* the minimum HIGH period of the scl clock is 0.12us
+* the minimum LOW period of the scl clock is 0.32us
+* - HS-mode(3.4MHz):
+* the minimum HIGH period of the scl clock is 0.06us
+* the minimum LOW period of the scl clock is 0.16us
Is the rest of the driver ready for Fast-mode plus or HS mode? If not
then maybe leave those off? If nothing else the commit message should
indicate that this is just being forward thinking.
+*/
+ if (scl_rate = 10) {
+ *high_ratio = 40;
+ *low_ratio = 47;
+ } else if (scl_rate = 40) {
+ *high_ratio = 6;
+ *low_ratio = 13;
+ } else if (scl_rate = 100) {
+ *high_ratio = 26;
+ *low_ratio = 50;
+ } else if (scl_rate = 170) {
+ *high_ratio = 12;
+ *low_ratio = 32;
+ } else {
+ *high_ratio = 6;
+ *low_ratio = 16;
Since it's only the ratio of high to low that matters, you can combine
the last two. 12 : 32 == 6 : 16
+ }
+}
+
+static void rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long
scl_rate,
+ unsigned long *divh, unsigned long *divl)
+{
+ unsigned long high_ratio, low_ratio;
+ unsigned long ratio_sum;
+
+ rk3x_i2c_get_ratios(scl_rate, high_ratio, low_ratio);
+ ratio_sum = high_ratio + low_ratio;
+
+ /* T_high = T_clk * (divh + 1) * 8
+* T_low = T_clk * (divl + 1) * 8
+* T_scl = T_high + T_low
+* T_scl = 1 / scl_rate
+* T_clk = 1 / i2c_rate
+* T_high : T_low = high_ratio : low_ratio
+* ratio_sum = high_ratio + low_ratio
+*
+* so:
+* divh = (i2c_rate * high_ratio) / (scl_rate * ratio_sum * 8) - 1
+* divl = (i2c_rate * low_ratio) / (scl_rate * ratio_sum * 8) - 1
+*/
+ *divh = DIV_ROUND_UP(i2c_rate * high_ratio, scl_rate * ratio_sum *
8);
+ if (*divh)
+ *divh = *divh - 1;
+
+ *divl = DIV_ROUND_UP(i2c_rate * low_ratio, scl_rate * ratio_sum * 8);
+ if (*divl)
+ *divl = *divl - 1;
When I sent you the sample formulas I purposely did it differently
than this. Any reason you changed from my formulas?
div_low = DIV_ROUND_UP(clk_rate * low_ratio, scl_rate * 8 * ratio_sum)
div_high = DIV_ROUND_UP(clk_rate, scl_rate * 8) - div_low
div_low -= 1
if div_high:
div_high -= 1
Why did I do it that way?
* Assuming i2c_rate and the ratio is non-zero then you can assume that
DIV_ROUND_UP gives a value that is = 1. No need to test the result
against 0.
* (I think) you'll get a
Re: [PATCH] i2c: rk3x: adjust the LOW divison based on characteristics of SCL
Addy,
On Wed, Sep 24, 2014 at 1:23 AM, addy ke addy...@rock-chips.com wrote:
On 2014/9/24 12:10, Doug Anderson wrote:
Addy,
On Tue, Sep 23, 2014 at 6:55 PM, Addy Ke addy...@rock-chips.com wrote:
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
- Fast-mode plus:
the minimum HIGH period of the scl clock is 0.26us
the minimum LOW period of the scl clock is 0.5us
- HS-mode(1.7MHz):
the minimum HIGH period of the scl clock is 0.12us
the minimum LOW period of the scl clock is 0.32us
- HS-mode(3.4MHz):
the minimum HIGH period of the scl clock is 0.06us
the minimum LOW period of the scl clock is 0.16us
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.
Thanks Doug for the suggestion about division formula.
Signed-off-by: Addy Ke addy...@rock-chips.com
---
drivers/i2c/busses/i2c-rk3x.c | 79
+++
1 file changed, 72 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 93cfc83..49d67b7 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -428,18 +428,83 @@ out:
return IRQ_HANDLED;
}
+static void rk3x_i2c_get_ratios(unsigned long scl_rate,
+ unsigned long *high_ratio,
+ unsigned long *low_ratio)
+{
+ /* 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
+* - Fast-mode plus:
+* the minimum HIGH period of the scl clock is 0.26us
+* the minimum LOW period of the scl clock is 0.5us
+* - HS-mode(1.7MHz):
+* the minimum HIGH period of the scl clock is 0.12us
+* the minimum LOW period of the scl clock is 0.32us
+* - HS-mode(3.4MHz):
+* the minimum HIGH period of the scl clock is 0.06us
+* the minimum LOW period of the scl clock is 0.16us
Is the rest of the driver ready for Fast-mode plus or HS mode? If not
then maybe leave those off? If nothing else the commit message should
indicate that this is just being forward thinking.
+*/
+ if (scl_rate = 10) {
+ *high_ratio = 40;
+ *low_ratio = 47;
+ } else if (scl_rate = 40) {
+ *high_ratio = 6;
+ *low_ratio = 13;
+ } else if (scl_rate = 100) {
+ *high_ratio = 26;
+ *low_ratio = 50;
+ } else if (scl_rate = 170) {
+ *high_ratio = 12;
+ *low_ratio = 32;
+ } else {
+ *high_ratio = 6;
+ *low_ratio = 16;
Since it's only the ratio of high to low that matters, you can combine
the last two. 12 : 32 == 6 : 16
+ }
+}
+
+static void rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long
scl_rate,
+ unsigned long *divh, unsigned long *divl)
+{
+ unsigned long high_ratio, low_ratio;
+ unsigned long ratio_sum;
+
+ rk3x_i2c_get_ratios(scl_rate, high_ratio, low_ratio);
+ ratio_sum = high_ratio + low_ratio;
+
+ /* T_high = T_clk * (divh + 1) * 8
+* T_low = T_clk * (divl + 1) * 8
+* T_scl = T_high + T_low
+* T_scl = 1 / scl_rate
+* T_clk = 1 / i2c_rate
+* T_high : T_low = high_ratio : low_ratio
+* ratio_sum = high_ratio + low_ratio
+*
+* so:
+* divh = (i2c_rate * high_ratio) / (scl_rate * ratio_sum * 8) - 1
+* divl = (i2c_rate * low_ratio) / (scl_rate * ratio_sum * 8) - 1
+*/
+ *divh = DIV_ROUND_UP(i2c_rate * high_ratio, scl_rate * ratio_sum *
8);
+ if (*divh)
+ *divh = *divh - 1;
+
+ *divl = DIV_ROUND_UP(i2c_rate * low_ratio, scl_rate * ratio_sum *
8);
+ if (*divl)
+ *divl = *divl - 1;
When I sent you the sample formulas I purposely did it differently
than this. Any reason you changed from my formulas?
div_low = DIV_ROUND_UP(clk_rate * low_ratio, scl_rate * 8 * ratio_sum)
div_high = DIV_ROUND_UP(clk_rate, scl_rate * 8) - div_low
div_low -= 1
if div_high:
div_high -= 1
Why did I do it that way?
* Assuming i2c_rate and the ratio is non-zero then you can assume that
DIV_ROUND_UP gives a value that is =
Re: [PATCH] i2c: rk3x: adjust the LOW divison based on characteristics of SCL
Hi, Doug
On 2014/9/25 1:13, Doug Anderson wrote:
Addy,
On Wed, Sep 24, 2014 at 1:23 AM, addy ke addy...@rock-chips.com wrote:
On 2014/9/24 12:10, Doug Anderson wrote:
Addy,
On Tue, Sep 23, 2014 at 6:55 PM, Addy Ke addy...@rock-chips.com wrote:
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
- Fast-mode plus:
the minimum HIGH period of the scl clock is 0.26us
the minimum LOW period of the scl clock is 0.5us
- HS-mode(1.7MHz):
the minimum HIGH period of the scl clock is 0.12us
the minimum LOW period of the scl clock is 0.32us
- HS-mode(3.4MHz):
the minimum HIGH period of the scl clock is 0.06us
the minimum LOW period of the scl clock is 0.16us
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.
Thanks Doug for the suggestion about division formula.
Signed-off-by: Addy Ke addy...@rock-chips.com
---
drivers/i2c/busses/i2c-rk3x.c | 79
+++
1 file changed, 72 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 93cfc83..49d67b7 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -428,18 +428,83 @@ out:
return IRQ_HANDLED;
}
+static void rk3x_i2c_get_ratios(unsigned long scl_rate,
+ unsigned long *high_ratio,
+ unsigned long *low_ratio)
+{
+ /* 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
+* - Fast-mode plus:
+* the minimum HIGH period of the scl clock is 0.26us
+* the minimum LOW period of the scl clock is 0.5us
+* - HS-mode(1.7MHz):
+* the minimum HIGH period of the scl clock is 0.12us
+* the minimum LOW period of the scl clock is 0.32us
+* - HS-mode(3.4MHz):
+* the minimum HIGH period of the scl clock is 0.06us
+* the minimum LOW period of the scl clock is 0.16us
Is the rest of the driver ready for Fast-mode plus or HS mode? If not
then maybe leave those off? If nothing else the commit message should
indicate that this is just being forward thinking.
+*/
+ if (scl_rate = 10) {
+ *high_ratio = 40;
+ *low_ratio = 47;
+ } else if (scl_rate = 40) {
+ *high_ratio = 6;
+ *low_ratio = 13;
+ } else if (scl_rate = 100) {
+ *high_ratio = 26;
+ *low_ratio = 50;
+ } else if (scl_rate = 170) {
+ *high_ratio = 12;
+ *low_ratio = 32;
+ } else {
+ *high_ratio = 6;
+ *low_ratio = 16;
Since it's only the ratio of high to low that matters, you can combine
the last two. 12 : 32 == 6 : 16
+ }
+}
+
+static void rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long
scl_rate,
+ unsigned long *divh, unsigned long *divl)
+{
+ unsigned long high_ratio, low_ratio;
+ unsigned long ratio_sum;
+
+ rk3x_i2c_get_ratios(scl_rate, high_ratio, low_ratio);
+ ratio_sum = high_ratio + low_ratio;
+
+ /* T_high = T_clk * (divh + 1) * 8
+* T_low = T_clk * (divl + 1) * 8
+* T_scl = T_high + T_low
+* T_scl = 1 / scl_rate
+* T_clk = 1 / i2c_rate
+* T_high : T_low = high_ratio : low_ratio
+* ratio_sum = high_ratio + low_ratio
+*
+* so:
+* divh = (i2c_rate * high_ratio) / (scl_rate * ratio_sum * 8) - 1
+* divl = (i2c_rate * low_ratio) / (scl_rate * ratio_sum * 8) - 1
+*/
+ *divh = DIV_ROUND_UP(i2c_rate * high_ratio, scl_rate * ratio_sum *
8);
+ if (*divh)
+ *divh = *divh - 1;
+
+ *divl = DIV_ROUND_UP(i2c_rate * low_ratio, scl_rate * ratio_sum *
8);
+ if (*divl)
+ *divl = *divl - 1;
When I sent you the sample formulas I purposely did it differently
than this. Any reason you changed from my formulas?
div_low = DIV_ROUND_UP(clk_rate * low_ratio, scl_rate * 8 * ratio_sum)
div_high = DIV_ROUND_UP(clk_rate, scl_rate * 8) - div_low
div_low -= 1
if div_high:
div_high -= 1
Why did I do it that way?
* Assuming i2c_rate and the ratio is non-zero then you
[PATCH] i2c: rk3x: adjust the LOW divison based on characteristics of SCL
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
- Fast-mode plus:
the minimum HIGH period of the scl clock is 0.26us
the minimum LOW period of the scl clock is 0.5us
- HS-mode(1.7MHz):
the minimum HIGH period of the scl clock is 0.12us
the minimum LOW period of the scl clock is 0.32us
- HS-mode(3.4MHz):
the minimum HIGH period of the scl clock is 0.06us
the minimum LOW period of the scl clock is 0.16us
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.
Thanks Doug for the suggestion about division formula.
Signed-off-by: Addy Ke addy...@rock-chips.com
---
drivers/i2c/busses/i2c-rk3x.c | 79 +++
1 file changed, 72 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 93cfc83..49d67b7 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -428,18 +428,83 @@ out:
return IRQ_HANDLED;
}
+static void rk3x_i2c_get_ratios(unsigned long scl_rate,
+ unsigned long *high_ratio,
+ unsigned long *low_ratio)
+{
+ /* 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
+* - Fast-mode plus:
+* the minimum HIGH period of the scl clock is 0.26us
+* the minimum LOW period of the scl clock is 0.5us
+* - HS-mode(1.7MHz):
+* the minimum HIGH period of the scl clock is 0.12us
+* the minimum LOW period of the scl clock is 0.32us
+* - HS-mode(3.4MHz):
+* the minimum HIGH period of the scl clock is 0.06us
+* the minimum LOW period of the scl clock is 0.16us
+*/
+ if (scl_rate = 10) {
+ *high_ratio = 40;
+ *low_ratio = 47;
+ } else if (scl_rate = 40) {
+ *high_ratio = 6;
+ *low_ratio = 13;
+ } else if (scl_rate = 100) {
+ *high_ratio = 26;
+ *low_ratio = 50;
+ } else if (scl_rate = 170) {
+ *high_ratio = 12;
+ *low_ratio = 32;
+ } else {
+ *high_ratio = 6;
+ *low_ratio = 16;
+ }
+}
+
+static void rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long scl_rate,
+ unsigned long *divh, unsigned long *divl)
+{
+ unsigned long high_ratio, low_ratio;
+ unsigned long ratio_sum;
+
+ rk3x_i2c_get_ratios(scl_rate, high_ratio, low_ratio);
+ ratio_sum = high_ratio + low_ratio;
+
+ /* T_high = T_clk * (divh + 1) * 8
+* T_low = T_clk * (divl + 1) * 8
+* T_scl = T_high + T_low
+* T_scl = 1 / scl_rate
+* T_clk = 1 / i2c_rate
+* T_high : T_low = high_ratio : low_ratio
+* ratio_sum = high_ratio + low_ratio
+*
+* so:
+* divh = (i2c_rate * high_ratio) / (scl_rate * ratio_sum * 8) - 1
+* divl = (i2c_rate * low_ratio) / (scl_rate * ratio_sum * 8) - 1
+*/
+ *divh = DIV_ROUND_UP(i2c_rate * high_ratio, scl_rate * ratio_sum * 8);
+ if (*divh)
+ *divh = *divh - 1;
+
+ *divl = DIV_ROUND_UP(i2c_rate * low_ratio, scl_rate * ratio_sum * 8);
+ if (*divl)
+ *divl = *divl - 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 divh, divl;
- /* set DIV = DIVH = DIVL
-* SCL rate = (clk rate) / (8 * (DIVH + 1 + DIVL + 1))
-* = (clk rate) / (16 * (DIV + 1))
-*/
- div = DIV_ROUND_UP(i2c_rate, scl_rate * 16) - 1;
+ rk3x_i2c_calc_divs(i2c_rate, scl_rate, divh, divl);
- i2c_writel(i2c, (div 16) | (div 0x), REG_CLKDIV);
+ i2c_writel(i2c, (divh 16) | (divl 0x), REG_CLKDIV);
}
/**
--
1.8.3.2
--
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: rk3x: adjust the LOW divison based on characteristics of SCL
Addy,
On Tue, Sep 23, 2014 at 6:55 PM, Addy Ke addy...@rock-chips.com wrote:
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
- Fast-mode plus:
the minimum HIGH period of the scl clock is 0.26us
the minimum LOW period of the scl clock is 0.5us
- HS-mode(1.7MHz):
the minimum HIGH period of the scl clock is 0.12us
the minimum LOW period of the scl clock is 0.32us
- HS-mode(3.4MHz):
the minimum HIGH period of the scl clock is 0.06us
the minimum LOW period of the scl clock is 0.16us
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.
Thanks Doug for the suggestion about division formula.
Signed-off-by: Addy Ke addy...@rock-chips.com
---
drivers/i2c/busses/i2c-rk3x.c | 79
+++
1 file changed, 72 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 93cfc83..49d67b7 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -428,18 +428,83 @@ out:
return IRQ_HANDLED;
}
+static void rk3x_i2c_get_ratios(unsigned long scl_rate,
+ unsigned long *high_ratio,
+ unsigned long *low_ratio)
+{
+ /* 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
+* - Fast-mode plus:
+* the minimum HIGH period of the scl clock is 0.26us
+* the minimum LOW period of the scl clock is 0.5us
+* - HS-mode(1.7MHz):
+* the minimum HIGH period of the scl clock is 0.12us
+* the minimum LOW period of the scl clock is 0.32us
+* - HS-mode(3.4MHz):
+* the minimum HIGH period of the scl clock is 0.06us
+* the minimum LOW period of the scl clock is 0.16us
Is the rest of the driver ready for Fast-mode plus or HS mode? If not
then maybe leave those off? If nothing else the commit message should
indicate that this is just being forward thinking.
+*/
+ if (scl_rate = 10) {
+ *high_ratio = 40;
+ *low_ratio = 47;
+ } else if (scl_rate = 40) {
+ *high_ratio = 6;
+ *low_ratio = 13;
+ } else if (scl_rate = 100) {
+ *high_ratio = 26;
+ *low_ratio = 50;
+ } else if (scl_rate = 170) {
+ *high_ratio = 12;
+ *low_ratio = 32;
+ } else {
+ *high_ratio = 6;
+ *low_ratio = 16;
Since it's only the ratio of high to low that matters, you can combine
the last two. 12 : 32 == 6 : 16
+ }
+}
+
+static void rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long
scl_rate,
+ unsigned long *divh, unsigned long *divl)
+{
+ unsigned long high_ratio, low_ratio;
+ unsigned long ratio_sum;
+
+ rk3x_i2c_get_ratios(scl_rate, high_ratio, low_ratio);
+ ratio_sum = high_ratio + low_ratio;
+
+ /* T_high = T_clk * (divh + 1) * 8
+* T_low = T_clk * (divl + 1) * 8
+* T_scl = T_high + T_low
+* T_scl = 1 / scl_rate
+* T_clk = 1 / i2c_rate
+* T_high : T_low = high_ratio : low_ratio
+* ratio_sum = high_ratio + low_ratio
+*
+* so:
+* divh = (i2c_rate * high_ratio) / (scl_rate * ratio_sum * 8) - 1
+* divl = (i2c_rate * low_ratio) / (scl_rate * ratio_sum * 8) - 1
+*/
+ *divh = DIV_ROUND_UP(i2c_rate * high_ratio, scl_rate * ratio_sum * 8);
+ if (*divh)
+ *divh = *divh - 1;
+
+ *divl = DIV_ROUND_UP(i2c_rate * low_ratio, scl_rate * ratio_sum * 8);
+ if (*divl)
+ *divl = *divl - 1;
When I sent you the sample formulas I purposely did it differently
than this. Any reason you changed from my formulas?
div_low = DIV_ROUND_UP(clk_rate * low_ratio, scl_rate * 8 * ratio_sum)
div_high = DIV_ROUND_UP(clk_rate, scl_rate * 8) - div_low
div_low -= 1
if div_high:
div_high -= 1
Why did I do it that way?
* Assuming i2c_rate and the ratio is non-zero then you can assume that
DIV_ROUND_UP gives a value that is = 1. No need to test the result
against 0.
* (I think) you'll get a more accurate clock rate by subtracting.
Try running your formula vs. my
