Re: [PATCH v3] i2c: rk3x: fix bug that cause measured high_ns doesn't meet I2C spec
On 2014/12/8 10:59, Addy Ke wrote:
high_ns calculated from the low division of CLKDIV register is the sum
of actual measured high_ns and rise_ns. The rise time which related to
external pull-up resistor can be up to the maximum rise time in I2C spec.
In my test, if external pull-up resistor is 4.7K, rise_ns is about
700ns. So the actual measured high_ns is about 3900ns, which is less
than 4000ns(the minimum high_ns in I2C spec).
To fix this bug, min_low_ns should include fall time and min_high_ns
should include rise time too.
This patch merged the patch that Doug submitted to chromium, which
can get the rise and fall times for signals from the device tree.
This allows us to more accurately calculate timings. see:
https://chromium-review.googlesource.com/#/c/232774/
Signed-off-by: Addy Ke addy...@rock-chips.com
---
Changes in v2:
- merged the patch that Doug submitted to chromium
Changes in v3:
- merged the patch that Doug submitted to chromium to change bindins
see: https://chromium-review.googlesource.com/#/c/232775/
Sorry, see https://chromium-review.googlesource.com/#/c/232774/ not 232775
Documentation/devicetree/bindings/i2c/i2c-rk3x.txt | 11 +
drivers/i2c/busses/i2c-rk3x.c | 57
+++---
2 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-rk3x.txt
b/Documentation/devicetree/bindings/i2c/i2c-rk3x.txt
index dde6c22..925d6eb 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-rk3x.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-rk3x.txt
@@ -21,6 +21,14 @@ Required on RK3066, RK3188 :
Optional properties :
- clock-frequency : SCL frequency to use (in Hz). If omitted, 100kHz is
used.
+ - i2c-scl-rising-time-ns : Number of nanoseconds the signal takes to rise
+ (t(r) in i2c spec). If not specified this is assumed to be the max the
+ spec allows(1000 ns for standard mode, 300 ns for fast mode) which
+ might cause slightly slower communication.
+ - i2c-scl-falling-time-ns : Number of nanoseconds the signal takes to fall
+ (t(f) in the i2c0 spec). If not specified this is assumed to be the
+ max the spec allows (300 ns) which might cause slightly slower
+ communication.
Example:
@@ -39,4 +47,7 @@ i2c0: i2c@2002d000 {
clock-names = i2c;
clocks = cru PCLK_I2C0;
+
+ i2c-scl-rising-time-ns = 800;
+ i2c-scl-falling-time-ns = 100;
};
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 0ee5802..50c1678 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -102,6 +102,8 @@ struct rk3x_i2c {
/* Settings */
unsigned int scl_frequency;
+ unsigned int rise_ns;
+ unsigned int fall_ns;
/* Synchronization notification */
spinlock_t lock;
@@ -435,6 +437,8 @@ out:
*
* @clk_rate: I2C input clock rate
* @scl_rate: Desired SCL rate
+ * @rise_ns: How many ns it takes for signals to rise.
+ * @fall_ns: How many ns it takes for signals to fall.
* @div_low: Divider output for low
* @div_high: Divider output for high
*
@@ -443,11 +447,14 @@ out:
* too high, we silently use the highest possible rate.
*/
static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long scl_rate,
+ unsigned long rise_ns, unsigned long fall_ns,
unsigned long *div_low, unsigned long *div_high)
{
+ unsigned long spec_min_low_ns, spec_min_high_ns;
+ unsigned long spec_max_data_hold_ns;
+ unsigned long spec_data_hold_buffer_ns;
+
unsigned long min_low_ns, min_high_ns;
- unsigned long max_data_hold_ns;
- unsigned long data_hold_buffer_ns;
unsigned long max_low_ns, min_total_ns;
unsigned long clk_rate_khz, scl_rate_khz;
@@ -470,28 +477,30 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
unsigned long scl_rate,
/*
* min_low_ns: The minimum number of ns we need to hold low
- * to meet i2c spec
+ * to meet i2c spec, should include fall time.
* min_high_ns: The minimum number of ns we need to hold high
- * to meet i2c spec
+ * to meet i2c spec, should include rise time.
* max_low_ns: The maximum number of ns we can hold low
- * to meet i2c spec
+ * 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.
*/
if (scl_rate = 10) {
- min_low_ns = 4700;
- min_high_ns = 4000;
- max_data_hold_ns = 3450;
- data_hold_buffer_ns = 50;
+ spec_min_low_ns = 4700;
+ spec_min_high_ns = 4000
[PATCH] i2c: rk3x: fix bug that cause measured high_ns doesn't meet I2C spec
high_ns calculated from the low division of CLKDIV register is the sum of
actual measured high_ns and rise_ns. The rise time which related to
external pull-up resistor can be up to the maximum rise time in I2C spec.
In my test, if external pull-up resistor is 4.7K, rise_ns is about 700ns.
So the actual measured high_ns is about 3900ns, which is less than 4000ns
(the minimum high_ns in I2C spec).
Signed-off-by: Addy Ke addy...@rock-chips.com
---
drivers/i2c/busses/i2c-rk3x.c | 58 +++
1 file changed, 37 insertions(+), 21 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index e276ffb..8e1cc2b 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -432,9 +432,12 @@ out:
static int rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long scl_rate,
unsigned long *div_low, unsigned long *div_high)
{
+ unsigned long spec_min_low_ns, spec_min_high_ns;
+ unsigned long spec_max_data_hold_ns;
+ unsigned long spec_data_hold_buffer_ns;
+ unsigned long spec_max_rise_ns;
+
unsigned long min_low_ns, min_high_ns;
- unsigned long max_data_hold_ns;
- unsigned long data_hold_buffer_ns;
unsigned long max_low_ns, min_total_ns;
unsigned long i2c_rate_khz, scl_rate_khz;
@@ -453,30 +456,43 @@ static int rk3x_i2c_calc_divs(unsigned long i2c_rate,
unsigned long scl_rate,
if (WARN_ON(scl_rate 1000))
scl_rate = 1000;
+ if (scl_rate = 10) {
+ spec_min_low_ns = 4700;
+ spec_min_high_ns = 4000;
+ spec_max_rise_ns = 1000;
+ spec_max_data_hold_ns = 3450;
+ spec_data_hold_buffer_ns = 50;
+ } else {
+ spec_min_low_ns = 1300;
+ spec_min_high_ns = 600;
+ spec_max_rise_ns = 300;
+ spec_max_data_hold_ns = 900;
+ spec_data_hold_buffer_ns = 50;
+ }
+
/*
-* 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
+* min_low_ns: The minimum number of ns we need to hold low.
+* The fall time in RK3X's I2C controller is approximately
+* equal to 0. So min_low_ns = spec_min_low_ns.
+* Note: low_ns should be (measured_low_ns + measured_fall_time)
+* and measured_low_ns must meet I2C spec.
*
-* Note: max_low_ns should be (max data hold time * 2 - buffer)
+* min_high_ns: The minimum number of ns we need to hold high.
+* The rise time which related to external pull-up resistor
+* can be up to spec_max_rise_ns.
+* So min_high_ns = spec_min_high_ns + spec_max_rise_ns
+* Note: high_ns should be (measured_high_ns + measured_rise_time)
+* and measured_high_ns must meet I2C spec.
+*
+* max_low_ns: The maximum number of ns we can hold low.
+* 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.
*/
- if (scl_rate = 10) {
- min_low_ns = 4700;
- min_high_ns = 4000;
- max_data_hold_ns = 3450;
- data_hold_buffer_ns = 50;
- } else {
- min_low_ns = 1300;
- min_high_ns = 600;
- max_data_hold_ns = 900;
- data_hold_buffer_ns = 50;
- }
- max_low_ns = max_data_hold_ns * 2 - data_hold_buffer_ns;
+ min_low_ns = spec_min_low_ns;
+ min_high_ns = spec_min_high_ns + spec_max_rise_ns;
+ max_low_ns = spec_max_data_hold_ns * 2 - spec_data_hold_buffer_ns;
+
min_total_ns = min_low_ns + min_high_ns;
/* Adjust to avoid overflow */
--
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
[PATCH v6] 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
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 formulas.
Tested-by: Heiko Stuebner he...@sntech.de
Reviewed-by: Doug Anderson diand...@chromium.org
Tested-by: Doug Anderson diand...@chromium.org
Signed-off-by: Addy Ke addy...@rock-chips.com
---
Changes in v2:
- remove Fast-mode plus and HS-mode
- use new formulas suggested by Doug
Changes in V3:
- use new formulas (sep 30) suggested by Doug
Changes in V4:
- fix some wrong style
- WARN_ONCE if min_low_div max_low_div
Changes in V5:
- round up for i2c_rate and round down for scl_rate, suggested by Max
- place a WARN_ON if scl_rate 1000, suggested by Max
- add div_high and div_low overflow protection, suggested by Max
Changes in V6:
- goto to jump to clk_disable if set_scl_rate return error
drivers/i2c/busses/i2c-rk3x.c | 162 +++---
1 file changed, 153 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index b41d979..47e85dc 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -24,6 +24,7 @@
#include linux/wait.h
#include linux/mfd/syscon.h
#include linux/regmap.h
+#include linux/math64.h
/* Register Map */
@@ -428,18 +429,158 @@ out:
return IRQ_HANDLED;
}
-static void rk3x_i2c_set_scl_rate(struct rk3x_i2c *i2c, unsigned long scl_rate)
+static int rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long scl_rate,
+ unsigned long *div_low, unsigned long *div_high)
{
- unsigned long i2c_rate = clk_get_rate(i2c-clk);
- unsigned int div;
+ unsigned long min_low_ns, min_high_ns;
+ unsigned long max_data_hold_ns;
+ unsigned long data_hold_buffer_ns;
+ unsigned long max_low_ns, min_total_ns;
+
+ unsigned long i2c_rate_khz, scl_rate_khz;
+
+ unsigned long min_low_div, min_high_div;
+ unsigned long max_low_div;
+
+ unsigned long min_div_for_hold, min_total_div;
+ unsigned long extra_div, extra_low_div, ideal_low_div;
+
+ /* Only support standard-mode and fast-mode */
+ if (WARN_ON(scl_rate 40))
+ scl_rate = 40;
+
+ /* prevent scl_rate_khz from becoming 0 */
+ if (WARN_ON(scl_rate 1000))
+ scl_rate = 1000;
+
+ /*
+* 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.
+*/
+ if (scl_rate = 10) {
+ min_low_ns = 4700;
+ min_high_ns = 4000;
+ max_data_hold_ns = 3450;
+ data_hold_buffer_ns = 50;
+ } else {
+ min_low_ns = 1300;
+ min_high_ns = 600;
+ max_data_hold_ns = 900;
+ data_hold_buffer_ns = 50;
+ }
+ max_low_ns = max_data_hold_ns * 2 - data_hold_buffer_ns;
+ 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 = scl_rate / 1000;
- /* set DIV = DIVH = DIVL
-* SCL rate = (clk rate) / (8 * (DIVH + 1 + DIVL + 1))
-* = (clk rate) / (16 * (DIV + 1))
+ /*
+* 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 * 100);
+ min_high_div = DIV_ROUND_UP(i2c_rate_khz * min_high_ns, 8 * 100);
+ 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.
*/
- div = DIV_ROUND_UP(i2c_rate, scl_rate * 16) - 1;
+ max_low_div = i2c_rate_khz * max_low_ns / (8 * 100);
+
+ if (min_low_div max_low_div) {
+ WARN_ONCE(true,
+ Conflicting, min_low_div %lu
[PATCH v5] 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
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 formulas.
Tested-by: Heiko Stuebner he...@sntech.de
Reviewed-by: Doug Anderson diand...@chromium.org
Tested-by: Doug Anderson diand...@chromium.org
Signed-off-by: Addy Ke addy...@rock-chips.com
---
Changes in v2:
- remove Fast-mode plus and HS-mode
- use new formulas suggested by Doug
Changes in V3:
- use new formulas (sep 30) suggested by Doug
Changes in V4:
- fix some wrong style
- WARN_ONCE if min_low_div max_low_div
Changes in V5:
- round up for i2c_rate and round down for scl_rate, suggested by Max
- place a WARN_ON if scl_rate 1000, suggested by Max
- add div_high and div_low overflow protection, suggested by Max
drivers/i2c/busses/i2c-rk3x.c | 161 +++---
1 file changed, 152 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index b41d979..a30a4fb 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -24,6 +24,7 @@
#include linux/wait.h
#include linux/mfd/syscon.h
#include linux/regmap.h
+#include linux/math64.h
/* Register Map */
@@ -428,18 +429,158 @@ out:
return IRQ_HANDLED;
}
-static void rk3x_i2c_set_scl_rate(struct rk3x_i2c *i2c, unsigned long scl_rate)
+static int rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long scl_rate,
+ unsigned long *div_low, unsigned long *div_high)
{
- unsigned long i2c_rate = clk_get_rate(i2c-clk);
- unsigned int div;
+ unsigned long min_low_ns, min_high_ns;
+ unsigned long max_data_hold_ns;
+ unsigned long data_hold_buffer_ns;
+ unsigned long max_low_ns, min_total_ns;
+
+ unsigned long i2c_rate_khz, scl_rate_khz;
+
+ unsigned long min_low_div, min_high_div;
+ unsigned long max_low_div;
+
+ unsigned long min_div_for_hold, min_total_div;
+ unsigned long extra_div, extra_low_div, ideal_low_div;
+
+ /* Only support standard-mode and fast-mode */
+ if (WARN_ON(scl_rate 40))
+ scl_rate = 40;
+
+ /* prevent scl_rate_khz from becoming 0 */
+ if (WARN_ON(scl_rate 1000))
+ scl_rate = 1000;
+
+ /*
+* 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.
+*/
+ if (scl_rate = 10) {
+ min_low_ns = 4700;
+ min_high_ns = 4000;
+ max_data_hold_ns = 3450;
+ data_hold_buffer_ns = 50;
+ } else {
+ min_low_ns = 1300;
+ min_high_ns = 600;
+ max_data_hold_ns = 900;
+ data_hold_buffer_ns = 50;
+ }
+ max_low_ns = max_data_hold_ns * 2 - data_hold_buffer_ns;
+ 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 = scl_rate / 1000;
- /* set DIV = DIVH = DIVL
-* SCL rate = (clk rate) / (8 * (DIVH + 1 + DIVL + 1))
-* = (clk rate) / (16 * (DIV + 1))
+ /*
+* 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 * 100);
+ min_high_div = DIV_ROUND_UP(i2c_rate_khz * min_high_ns, 8 * 100);
+ 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.
*/
- div = DIV_ROUND_UP(i2c_rate, scl_rate * 16) - 1;
+ max_low_div = i2c_rate_khz * max_low_ns / (8 * 100);
+
+ if (min_low_div max_low_div) {
+ WARN_ONCE(true,
+ Conflicting, min_low_div %lu, max_low_div %lu\n,
+ min_low_div, max_low_div
[PATCH v4] 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
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 formulas.
Reviewed-by: Doug Anderson diand...@chromium.org
Tested-by: Doug Anderson diand...@chromium.org
Signed-off-by: Addy Ke addy...@rock-chips.com
---
Changes in v2:
- remove Fast-mode plus and HS-mode
- use new formulas suggested by Doug
Changes in V3:
- use new formulas (sep 30) suggested by Doug
Changes in V3:
- fix some wrong style
- WARN_ONCE if min_low_div max_low_div
drivers/i2c/busses/i2c-rk3x.c | 140 +++---
1 file changed, 133 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index b41d979..f17f7a2 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -24,6 +24,7 @@
#include linux/wait.h
#include linux/mfd/syscon.h
#include linux/regmap.h
+#include linux/math64.h
/* Register Map */
@@ -428,18 +429,143 @@ out:
return IRQ_HANDLED;
}
+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;
+ unsigned long max_data_hold_ns;
+ unsigned long data_hold_buffer_ns;
+ unsigned long max_low_ns, min_total_ns;
+
+ unsigned long i2c_rate_khz, scl_rate_khz;
+
+ unsigned long min_low_div, min_high_div;
+ unsigned long max_low_div;
+
+ unsigned long min_div_for_hold, min_total_div;
+ unsigned long extra_div, extra_low_div, ideal_low_div;
+
+ /* Only support standard-mode and fast-mode */
+ WARN_ON(scl_rate 40);
+
+ /*
+* 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.
+*/
+ if (scl_rate = 10) {
+ min_low_ns = 4700;
+ min_high_ns = 4000;
+ max_data_hold_ns = 3450;
+ data_hold_buffer_ns = 50;
+ } else {
+ min_low_ns = 1300;
+ min_high_ns = 600;
+ max_data_hold_ns = 900;
+ data_hold_buffer_ns = 50;
+ }
+ max_low_ns = max_data_hold_ns * 2 - data_hold_buffer_ns;
+ 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 * 100);
+ min_high_div = DIV_ROUND_UP(i2c_rate_khz * min_high_ns, 8 * 100);
+ 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 * 100);
+
+ if (min_low_div max_low_div) {
+ WARN_ONCE(true,
+ Conflicting, min_low_div %lu, max_low_div %lu\n,
+ min_low_div, max_low_div);
+ max_low_div = min_low_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
[PATCH v3] 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
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 formulas.
Signed-off-by: Addy Ke addy...@rock-chips.com
---
Changes in v2:
- remove Fast-mode plus and HS-mode
- use new formulas suggested by Doug
Changes in V3:
- use new formulas (sep 30) suggested by Doug
drivers/i2c/busses/i2c-rk3x.c | 135 +++---
1 file changed, 128 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index b41d979..9612eba 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -24,6 +24,7 @@
#include linux/wait.h
#include linux/mfd/syscon.h
#include linux/regmap.h
+#include linux/math64.h
/* Register Map */
@@ -428,18 +429,138 @@ out:
return IRQ_HANDLED;
}
+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;
+ unsigned long max_data_hold_ns;
+ unsigned long data_hold_buffer_ns;
+ unsigned long max_low_ns, min_total_ns;
+
+ unsigned long i2c_rate_khz, scl_rate_khz;
+
+ unsigned long min_low_div, min_high_div;
+ unsigned long max_low_div;
+
+ unsigned long min_div_for_hold, min_total_div;
+ unsigned long extra_div, extra_low_div, ideal_low_div;
+
+ /* Only support standard-mode and fast-mode */
+ WARN_ON(scl_rate 40);
+
+ /*
+* 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.
+*/
+ if (scl_rate = 10) {
+ min_low_ns = 4700;
+ min_high_ns = 4000;
+ max_data_hold_ns = 3450;
+ data_hold_buffer_ns = 50;
+ } else {
+ min_low_ns = 1300;
+ min_high_ns = 600;
+ max_data_hold_ns = 900;
+ data_hold_buffer_ns = 50;
+ }
+ max_low_ns = max_data_hold_ns * 2 - data_hold_buffer_ns;
+ 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 * 100);
+ min_high_div = DIV_ROUND_UP(i2c_rate_khz * min_high_ns, 8 * 100);
+ 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 * 100);
+
+ if (min_low_div max_low_div)
+ max_low_div = min_low_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);
+
+ /* Don't allow it to go over the max */
+ if (ideal_low_div max_low_div)
+ ideal_low_div = max_low_div
Re: [PATCH v2] i2c: rk3x: adjust the LOW divison based on characteristics of SCL
Hi Doug
On 2014/9/29 12:39, Doug Anderson wrote:
Addy,
On Sat, Sep 27, 2014 at 12:11 AM, Addy Ke addy...@rock-chips.com wrote:
From: Addy addy...@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...@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 40);
+
+ /* 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 1ns in Standard-mode
+* and 2500ns in Fast-mode
+*/
+ if (scl_rate = 10) {
+ *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 = [(10/100K) ns - min_low_ns_100k - min_high_ns_100k] / 2
300 = [(10/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 * 100);
+ min_high_div = DIV_ROUND_UP(i2c_rate_khz * min_high_ns, 8 * 100);
+ min_div_for_hold = (min_low_div + min_high_div
[PATCH v2] i2c: rk3x: adjust the LOW divison based on characteristics of SCL
From: Addy addy...@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...@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 40);
+
+ /* 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 1ns in Standard-mode
+* and 2500ns in Fast-mode
+*/
+ if (scl_rate = 10) {
+ *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 * 100);
+ min_high_div = DIV_ROUND_UP(i2c_rate_khz * min_high_ns, 8 * 100);
+ 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);
+
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
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: About RK3288 i2c scl duty cycle
Add public list On 2014/9/17 23:17, Doug Anderson wrote: Addy, On Tue, Sep 16, 2014 at 6:30 PM, addy...@rock-chips.com addy...@rock-chips.com wrote: hi, all Any reason why you didn't add some public lists? It seems like this is a perfect discussion for linux-i2c. According to i2c-bus specification(version2.1, page 32, Table5, FAST-MODE): The minimum LOW period of the scl clock is 1.3us, and the minimum HIGH period of the scl clock is 0.6us. T(min_low) : T(min_high) ~= 2 : 1 If DIVH = DIVL in fast mode(scl rate = 400Khz) 1）Under ideal conditions, T(scl_low) = T(scl_high) = 1.25us 2）Our measurement, T(scl_low) = 1.3us, T(scl_high) = 1.25us The low period of the scl clock is critical. Do we need set DIVL:DIVH = 1 : 2 to increase T(scl_low)? // T(scl_low ) : T(scl_High) = 2 : 1 I can't say I've ever looked at that pat of the i2c spec before, but what you say seems reasonable to me. ...well for 400kHz, at least. At 100kHz you shouldn't use the same 2:1 ratio. Yes, in normal-mode(100K) we can be only used 1:1 ratio. But in FAST-MODE maybe we must use 2:1 ratio. In Table 5(Characteristics of the SDA and SCL bus lines for F/S-mode I2C-bus devices) 1)FAST-MODE(400K): The minimum LOW period of the scl clock1.3us the minimum HIGH period of the scl clock 0.6us T(min_low) : T(min_high) ~= 2 : 1 But I can't see any ratio about In FAST-mode(400k) and Normal-mode(100k). 2)Normal-MODE(100K): The minimum LOW period of the scl clock4.7us the minimum HIGH period of the scl clock 4.0us T(min_low) : T(min_high) ~= 1 : 1 3) HS-mode(3.4M) ratio of 1 to 2 is required, decribed as follows: Hs-mode master devices generate a serial clock signal with a HIGH to LOW ratio of 1 to 2 I'm sure other drivers have solved this problem too, so maybe you can copy some code. In i2c-designware-core.c you can see them doing all the calculations you need, I think. -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
[PATCH v2] i2c: rk3x: fix divisor calculation for SCL frequency
I2C_CLKDIV register descripted in the previous version of RK3x chip manual is incorrect. Plus 1 is required. The correct formula: - T(SCL_HIGH) = T(PCLK) * (CLKDIVH + 1) * 8 - T(SCL_LOW) = T(PCLK) * (CLKDIVL + 1) * 8 - (SCL Divsor) = 8 * ((CLKDIVL + 1) + (CLKDIVH + 1)) - SCL = PCLK / (CLK Divsor) It will be updated to the latest version of chip manual. Signed-off-by: Addy Ke addy...@rock-chips.com --- changes since v1: - make it more cleaner, suggested by Doug Anderson drivers/i2c/busses/i2c-rk3x.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c index e637c32..93cfc83 100644 --- a/drivers/i2c/busses/i2c-rk3x.c +++ b/drivers/i2c/busses/i2c-rk3x.c @@ -433,12 +433,11 @@ 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; - /* SCL rate = (clk rate) / (8 * DIV) */ - div = DIV_ROUND_UP(i2c_rate, scl_rate * 8); - - /* The lower and upper half of the CLKDIV reg describe the length of -* SCL low high periods. */ - div = DIV_ROUND_UP(div, 2); + /* 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; i2c_writel(i2c, (div 16) | (div 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: fix divisor calculation for SCL frequency
Addy,
On Thu, Sep 4, 2014 at 7:32 PM, Addy Ke addy...@rock-chips.com wrote:
I2C_CLKDIV register descripted in the previous version of
RK3x chip manual is incorrect. Plus 1 is required.
The correct formula:
- T(SCL_HIGH) = T(PCLK) * (CLKDIVH + 1) * 8
- T(SCL_LOW) = T(PCLK) * (CLKDIVL + 1) * 8
- (SCL Divsor) = 8 * ((CLKDIVL + 1) + (CLKDIVH + 1))
- SCL = PCLK / (CLK Divsor)
I'll trust that you tested this with a scope
Yes ,I have tested on RK3188 and RK3288, and confirmed by oscilloscope.
It will be updated to the latest version of chip manual.
Signed-off-by: Addy Ke addy...@rock-chips.com
---
drivers/i2c/busses/i2c-rk3x.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index e637c32..76b6604 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -433,8 +433,8 @@ 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;
- /* SCL rate = (clk rate) / (8 * DIV) */
- div = DIV_ROUND_UP(i2c_rate, scl_rate * 8);
+ /* SCL rate = (clk rate) / (8 * (DIV + 2)) */
+ div = DIV_ROUND_UP(i2c_rate, scl_rate * 8) - 2;
Given the bug I just fixed in the Rockchip SPI driver, I was a little
worried about div becoming -1 (and thus being a really large positive
number since div is unsigned).
However, it seems that you get saved by the next statement:
div = DIV_ROUND_UP(div, 2);
In the testing I did with the Linux macros, that magically transformed
a div of 0x (-1) to 0, so it's not technically a bug. ...but
it's very non-obvious. Can you do something a little cleaner?
The following modifications is reasonable?
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;
/* 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;
i2c_writel(i2c, (div 16) | (div 0x), REG_CLKDIV);
}
-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
[PATCH] i2c: rk3x: fix divisor calculation for SCL frequency
I2C_CLKDIV register descripted in the previous version of RK3x chip manual is incorrect. Plus 1 is required. The correct formula: - T(SCL_HIGH) = T(PCLK) * (CLKDIVH + 1) * 8 - T(SCL_LOW) = T(PCLK) * (CLKDIVL + 1) * 8 - (SCL Divsor) = 8 * ((CLKDIVL + 1) + (CLKDIVH + 1)) - SCL = PCLK / (CLK Divsor) It will be updated to the latest version of chip manual. Signed-off-by: Addy Ke addy...@rock-chips.com --- drivers/i2c/busses/i2c-rk3x.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c index e637c32..76b6604 100644 --- a/drivers/i2c/busses/i2c-rk3x.c +++ b/drivers/i2c/busses/i2c-rk3x.c @@ -433,8 +433,8 @@ 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; - /* SCL rate = (clk rate) / (8 * DIV) */ - div = DIV_ROUND_UP(i2c_rate, scl_rate * 8); + /* SCL rate = (clk rate) / (8 * (DIV + 2)) */ + div = DIV_ROUND_UP(i2c_rate, scl_rate * 8) - 2; /* The lower and upper half of the CLKDIV reg describe the length of * SCL low high periods. */ -- 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
[PATCH v4] i2c: rk3x: fix bug that cause transfer fails in master receive mode
In rk3x SOC, the I2C controller can receive/transmit up to 32 bytes data
in one chunk, so the size of data to be write/read to/from TXDATAx/RXDATAx
must be less than or equal 32 bytes at a time.
Tested on rk3288-pinky board, elan receive 158 bytes data.
Suggested-by: Dmitry Torokhov dmitry.torok...@gmail.com
Signed-off-by: Addy Ke addy...@rock-chips.com
Acked-by: Max Schwarz max.schw...@online.de
---
Changes in v2:
- Use cleaner syntax as suggested by Sergei.
- Update commit message as suggested by Wolfram.
Changes in v3:
- fix typo: maste -- master and double spaces after 'len'
drivers/i2c/busses/i2c-rk3x.c | 4
Changes in v4:
- remove unlikely() annotations, suggested by Dmitry Torokhov
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index b8b2b89..31de730 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -323,6 +323,10 @@ static void rk3x_i2c_handle_read(struct rk3x_i2c *i2c,
unsigned int ipd)
/* ack interrupt */
i2c_writel(i2c, REG_INT_MBRF, REG_IPD);
+ /* Can only handle a maximum of 32 bytes at a time */
+ if (len 32)
+ len = 32;
+
/* read the data from receive buffer */
for (i = 0; i len; ++i) {
if (i % 4 == 0)
--
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 v4] i2c: rk3x: fix bug that cause transfer fails in master receive mode
Addy, On Fri, Aug 22, 2014 at 11:00 AM, Addy Ke addy...@rock-chips.com wrote: In rk3x SOC, the I2C controller can receive/transmit up to 32 bytes data in one chunk, so the size of data to be write/read to/from TXDATAx/RXDATAx must be less than or equal 32 bytes at a time. Tested on rk3288-pinky board, elan receive 158 bytes data. Suggested-by: Dmitry Torokhov dmitry.torok...@gmail.com You only need a Suggested-by if the entire patch was suggested by someone. If someone provides you review feedback you don't need it. Said another way: in this case Dmitry didn't suggest that you need to fix the i2c controller to transmit 32 byte chunks (he only provided review feedback), so you shouldn't say this was Suggested-by him. You also had my reviewed-by on a previous version so you could keep it. Reviewed-by: Doug Anderson diand...@chromium.org so, Do I need repost this patch or post a new one as patch v5? -- 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: rk3x: fix bug that cause transfer fails in master receive mode
In rk3x SOC, the I2C controller can receive/transmit up to 32 bytes data
in one transaction, so the size of data to be write/read to/from
TXDATAx/RXDATAx must be less than or equal 32 bytes at a time.
Test on pinky board, elan receive 158 bytes data.
Signed-off-by: Addy Ke addy...@rock-chips.com
---
drivers/i2c/busses/i2c-rk3x.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 69e1185..dc0aa64 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -323,6 +323,9 @@ static void rk3x_i2c_handle_read(struct rk3x_i2c *i2c,
unsigned int ipd)
/* ack interrupt */
i2c_writel(i2c, REG_INT_MBRF, REG_IPD);
+ /* Can only handle a maximum of 32 bytes at a time */
+ len = (len 32) ? 32 : len;
+
/* read the data from receive buffer */
for (i = 0; i len; ++i) {
if (i % 4 == 0)
--
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
[PATCH v2] i2c: rk3x: fix bug that cause transfer fails in maste receive mode
In rk3x SOC, the I2C controller can receive/transmit up to 32 bytes data
in one chunk, so the size of data to be write/read to/from TXDATAx/RXDATAx
must be less than or equal 32 bytes at a time.
Tested on rk3288-pinky board, elan receive 158 bytes data.
Acked-by: Max Schwarz max.schw...@online.de
Signed-off-by: Addy Ke addy...@rock-chips.com
---
Changes in v2:
- Use cleaner syntax as suggested by Sergei.
- Update commit message as suggested by Wolfram.
drivers/i2c/busses/i2c-rk3x.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 69e1185..806724a 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -323,6 +323,10 @@ static void rk3x_i2c_handle_read(struct rk3x_i2c *i2c,
unsigned int ipd)
/* ack interrupt */
i2c_writel(i2c, REG_INT_MBRF, REG_IPD);
+ /* Can only handle a maximum of 32 bytes at a time */
+ if (unlikely(len 32))
+ len = 32;
+
/* read the data from receive buffer */
for (i = 0; i len; ++i) {
if (i % 4 == 0)
--
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
[PATCH v3] i2c: rk3x: fix bug that cause transfer fails in master receive mode
In rk3x SOC, the I2C controller can receive/transmit up to 32 bytes data
in one chunk, so the size of data to be write/read to/from TXDATAx/RXDATAx
must be less than or equal 32 bytes at a time.
Tested on rk3288-pinky board, elan receive 158 bytes data.
Acked-by: Max Schwarz max.schw...@online.de
Signed-off-by: Addy Ke addy...@rock-chips.com
---
Changes in v2:
- Use cleaner syntax as suggested by Sergei.
- Update commit message as suggested by Wolfram.
Changes in v3:
- fix typo: maste -- master and double spaces after 'len'
drivers/i2c/busses/i2c-rk3x.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 69e1185..806724a 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -323,6 +323,10 @@ static void rk3x_i2c_handle_read(struct rk3x_i2c *i2c,
unsigned int ipd)
/* ack interrupt */
i2c_writel(i2c, REG_INT_MBRF, REG_IPD);
+ /* Can only handle a maximum of 32 bytes at a time */
+ if (unlikely(len 32))
+ len = 32;
+
/* read the data from receive buffer */
for (i = 0; i len; ++i) {
if (i % 4 == 0)
--
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
[PATCH] i2c: rk3x: fix interrupt handling issue
If slave holds scl, I2C_IPD[7] will be set 1 by controller
for debugging. Driver must ignore it.
[5.752391] rk3x-i2c ff16.i2c: unexpected irq in WRITE: 0x80
[5.939027] rk3x-i2c ff16.i2c: timeout, ipd: 0x80, state: 4
Signed-off-by: Addy Ke addy...@rock-chips.com
---
drivers/i2c/busses/i2c-rk3x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 9e3084c..eab49e6 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -399,7 +399,7 @@ static irqreturn_t rk3x_i2c_irq(int irqno, void *dev_id)
}
/* is there anything left to handle? */
- if (unlikely(ipd == 0))
+ if (unlikely((ipd REG_INT_ALL) == 0))
goto out;
switch (i2c-state) {
--
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
