* [PATCH v3 0/4] Add rk3399 i2c clocks calculated method
@ 2016-01-14 12:31 ` David Wu
0 siblings, 0 replies; 25+ messages in thread
From: David Wu @ 2016-01-14 12:31 UTC (permalink / raw)
To: heiko
Cc: wsa, andy.shevchenko, dianders, huangtao, zyw, cf, xjq, hl,
linux-arm-kernel, linux-rockchip, linux-gpio, linux-kernel,
David Wu
RK3399 have updated a new way to calculate i2c timing information to slove
"repeat start" timing issue. So it needs to integrate existing method, use
different ops to seperate them.
After picking this series, pmic-rk818 and touchscreen-ts could work well
on the rk3368 sdk board. 100k, 400k and 1.7M i2c clk rates were tested on
the rk3399 fpga board, where i2c0 connected to pmic-ti65910. But 3.4M clk
rate was not tested, because of the scl rise time is 60ns, it could not
meet the i2c spec, the scl rise time is hard to reduce on fpga board.
David Wu (4):
i2c: rk3x: switch to i2c generic dt parsing
i2c: rk3x: add ops to caculate i2c clocks
i2c: rk3x: new method to caculate i2c clocks
i2c: rk3x: support I2C Highspeed Mode
drivers/i2c/busses/i2c-rk3x.c | 405 +++++++++++++++++++++++++++++++++---------
1 file changed, 319 insertions(+), 86 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 0/4] Add rk3399 i2c clocks calculated method
@ 2016-01-14 12:31 ` David Wu
0 siblings, 0 replies; 25+ messages in thread
From: David Wu @ 2016-01-14 12:31 UTC (permalink / raw)
To: linux-arm-kernel
RK3399 have updated a new way to calculate i2c timing information to slove
"repeat start" timing issue. So it needs to integrate existing method, use
different ops to seperate them.
After picking this series, pmic-rk818 and touchscreen-ts could work well
on the rk3368 sdk board. 100k, 400k and 1.7M i2c clk rates were tested on
the rk3399 fpga board, where i2c0 connected to pmic-ti65910. But 3.4M clk
rate was not tested, because of the scl rise time is 60ns, it could not
meet the i2c spec, the scl rise time is hard to reduce on fpga board.
David Wu (4):
i2c: rk3x: switch to i2c generic dt parsing
i2c: rk3x: add ops to caculate i2c clocks
i2c: rk3x: new method to caculate i2c clocks
i2c: rk3x: support I2C Highspeed Mode
drivers/i2c/busses/i2c-rk3x.c | 405 +++++++++++++++++++++++++++++++++---------
1 file changed, 319 insertions(+), 86 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 1/4] i2c: rk3x: switch to i2c generic dt parsing
2016-01-14 12:31 ` David Wu
@ 2016-01-14 12:31 ` David Wu
-1 siblings, 0 replies; 25+ messages in thread
From: David Wu @ 2016-01-14 12:31 UTC (permalink / raw)
To: heiko
Cc: wsa, andy.shevchenko, dianders, huangtao, zyw, cf, xjq, hl,
linux-arm-kernel, linux-rockchip, linux-gpio, linux-kernel,
David Wu
Switch to the new generic functions: i2c_parse_fw_timings().
Signed-off-by: David Wu <david.wu@rock-chips.com>
---
changes in v3:
- parse timing information with common interface(Wolfram, Andy)
drivers/i2c/busses/i2c-rk3x.c | 87 ++++++++++++-------------------------------
1 file changed, 24 insertions(+), 63 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 9096d17..1d86e8c 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -101,10 +101,7 @@ struct rk3x_i2c {
struct notifier_block clk_rate_nb;
/* Settings */
- unsigned int scl_frequency;
- unsigned int scl_rise_ns;
- unsigned int scl_fall_ns;
- unsigned int sda_fall_ns;
+ struct i2c_timings t;
/* Synchronization & notification */
spinlock_t lock;
@@ -437,10 +434,7 @@ out:
* Calculate divider values for desired SCL frequency
*
* @clk_rate: I2C input clock rate
- * @scl_rate: Desired SCL rate
- * @scl_rise_ns: How many ns it takes for SCL to rise.
- * @scl_fall_ns: How many ns it takes for SCL to fall.
- * @sda_fall_ns: How many ns it takes for SDA to fall.
+ * @t_input: Known I2C timing information.
* @div_low: Divider output for low
* @div_high: Divider output for high
*
@@ -448,11 +442,10 @@ out:
* a best-effort divider value is returned in divs. If the target rate is
* 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 scl_rise_ns,
- unsigned long scl_fall_ns,
- unsigned long sda_fall_ns,
- unsigned long *div_low, unsigned long *div_high)
+static int rk3x_i2c_calc_divs(unsigned long clk_rate,
+ struct i2c_timings *t_input,
+ unsigned long *div_low,
+ unsigned long *div_high)
{
unsigned long spec_min_low_ns, spec_min_high_ns;
unsigned long spec_setup_start, spec_max_data_hold_ns;
@@ -472,12 +465,12 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long scl_rate,
int ret = 0;
/* Only support standard-mode and fast-mode */
- if (WARN_ON(scl_rate > 400000))
- scl_rate = 400000;
+ if (WARN_ON(t_input->bus_freq_hz > 400000))
+ t_input->bus_freq_hz = 400000;
/* prevent scl_rate_khz from becoming 0 */
- if (WARN_ON(scl_rate < 1000))
- scl_rate = 1000;
+ if (WARN_ON(t_input->bus_freq_hz < 1000))
+ t_input->bus_freq_hz = 1000;
/*
* min_low_ns: The minimum number of ns we need to hold low to
@@ -491,7 +484,7 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long scl_rate,
* This is because the i2c host on Rockchip holds the data line
* for half the low time.
*/
- if (scl_rate <= 100000) {
+ if (t_input->bus_freq_hz <= 100000) {
/* Standard-mode */
spec_min_low_ns = 4700;
spec_setup_start = 4700;
@@ -506,7 +499,7 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long scl_rate,
spec_max_data_hold_ns = 900;
data_hold_buffer_ns = 50;
}
- min_high_ns = scl_rise_ns + spec_min_high_ns;
+ min_high_ns = t_input->scl_rise_ns + spec_min_high_ns;
/*
* Timings for repeated start:
@@ -517,18 +510,19 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long scl_rate,
* we meet tSU;STA and tHD;STA times.
*/
min_high_ns = max(min_high_ns,
- DIV_ROUND_UP((scl_rise_ns + spec_setup_start) * 1000, 875));
+ DIV_ROUND_UP((t_input->scl_rise_ns + spec_setup_start) * 1000,
+ 875));
min_high_ns = max(min_high_ns,
- DIV_ROUND_UP((scl_rise_ns + spec_setup_start +
- sda_fall_ns + spec_min_high_ns), 2));
+ DIV_ROUND_UP((t_input->scl_rise_ns + spec_setup_start +
+ t_input->sda_fall_ns + spec_min_high_ns), 2));
- min_low_ns = scl_fall_ns + spec_min_low_ns;
+ min_low_ns = t_input->scl_fall_ns + spec_min_low_ns;
max_low_ns = spec_max_data_hold_ns * 2 - data_hold_buffer_ns;
min_total_ns = min_low_ns + min_high_ns;
/* Adjust to avoid overflow */
clk_rate_khz = DIV_ROUND_UP(clk_rate, 1000);
- scl_rate_khz = scl_rate / 1000;
+ scl_rate_khz = t_input->bus_freq_hz / 1000;
/*
* We need the total div to be >= this number
@@ -620,10 +614,8 @@ static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
u64 t_low_ns, t_high_ns;
int ret;
- ret = rk3x_i2c_calc_divs(clk_rate, i2c->scl_frequency, i2c->scl_rise_ns,
- i2c->scl_fall_ns, i2c->sda_fall_ns,
- &div_low, &div_high);
- WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->scl_frequency);
+ ret = rk3x_i2c_calc_divs(clk_rate, &i2c->t, &div_low, &div_high);
+ WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->t.bus_freq_hz);
clk_enable(i2c->clk);
i2c_writel(i2c, (div_high << 16) | (div_low & 0xffff), REG_CLKDIV);
@@ -634,7 +626,7 @@ static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
dev_dbg(i2c->dev,
"CLK %lukhz, Req %uns, Act low %lluns high %lluns\n",
clk_rate / 1000,
- 1000000000 / i2c->scl_frequency,
+ 1000000000 / i2c->t.bus_freq_hz,
t_low_ns, t_high_ns);
}
@@ -664,9 +656,7 @@ static int rk3x_i2c_clk_notifier_cb(struct notifier_block *nb, unsigned long
switch (event) {
case PRE_RATE_CHANGE:
- if (rk3x_i2c_calc_divs(ndata->new_rate, i2c->scl_frequency,
- i2c->scl_rise_ns, i2c->scl_fall_ns,
- i2c->sda_fall_ns,
+ if (rk3x_i2c_calc_divs(ndata->new_rate, &i2c->t,
&div_low, &div_high) != 0)
return NOTIFY_STOP;
@@ -879,37 +869,8 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
match = of_match_node(rk3x_i2c_match, np);
i2c->soc_data = (struct rk3x_i2c_soc_data *)match->data;
- if (of_property_read_u32(pdev->dev.of_node, "clock-frequency",
- &i2c->scl_frequency)) {
- dev_info(&pdev->dev, "using default SCL frequency: %d\n",
- DEFAULT_SCL_RATE);
- i2c->scl_frequency = DEFAULT_SCL_RATE;
- }
-
- if (i2c->scl_frequency == 0 || i2c->scl_frequency > 400 * 1000) {
- dev_warn(&pdev->dev, "invalid SCL frequency specified.\n");
- dev_warn(&pdev->dev, "using default SCL frequency: %d\n",
- DEFAULT_SCL_RATE);
- i2c->scl_frequency = DEFAULT_SCL_RATE;
- }
-
- /*
- * Read rise and fall time from device tree. If not available use
- * the default maximum timing from the specification.
- */
- if (of_property_read_u32(pdev->dev.of_node, "i2c-scl-rising-time-ns",
- &i2c->scl_rise_ns)) {
- if (i2c->scl_frequency <= 100000)
- i2c->scl_rise_ns = 1000;
- else
- i2c->scl_rise_ns = 300;
- }
- if (of_property_read_u32(pdev->dev.of_node, "i2c-scl-falling-time-ns",
- &i2c->scl_fall_ns))
- i2c->scl_fall_ns = 300;
- if (of_property_read_u32(pdev->dev.of_node, "i2c-sda-falling-time-ns",
- &i2c->sda_fall_ns))
- i2c->sda_fall_ns = i2c->scl_fall_ns;
+ /* use common interface to get I2C timing properties */
+ i2c_parse_fw_timings(&pdev->dev, &i2c->t, true);
strlcpy(i2c->adap.name, "rk3x-i2c", sizeof(i2c->adap.name));
i2c->adap.owner = THIS_MODULE;
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 1/4] i2c: rk3x: switch to i2c generic dt parsing
@ 2016-01-14 12:31 ` David Wu
0 siblings, 0 replies; 25+ messages in thread
From: David Wu @ 2016-01-14 12:31 UTC (permalink / raw)
To: linux-arm-kernel
Switch to the new generic functions: i2c_parse_fw_timings().
Signed-off-by: David Wu <david.wu@rock-chips.com>
---
changes in v3:
- parse timing information with common interface(Wolfram, Andy)
drivers/i2c/busses/i2c-rk3x.c | 87 ++++++++++++-------------------------------
1 file changed, 24 insertions(+), 63 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 9096d17..1d86e8c 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -101,10 +101,7 @@ struct rk3x_i2c {
struct notifier_block clk_rate_nb;
/* Settings */
- unsigned int scl_frequency;
- unsigned int scl_rise_ns;
- unsigned int scl_fall_ns;
- unsigned int sda_fall_ns;
+ struct i2c_timings t;
/* Synchronization & notification */
spinlock_t lock;
@@ -437,10 +434,7 @@ out:
* Calculate divider values for desired SCL frequency
*
* @clk_rate: I2C input clock rate
- * @scl_rate: Desired SCL rate
- * @scl_rise_ns: How many ns it takes for SCL to rise.
- * @scl_fall_ns: How many ns it takes for SCL to fall.
- * @sda_fall_ns: How many ns it takes for SDA to fall.
+ * @t_input: Known I2C timing information.
* @div_low: Divider output for low
* @div_high: Divider output for high
*
@@ -448,11 +442,10 @@ out:
* a best-effort divider value is returned in divs. If the target rate is
* 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 scl_rise_ns,
- unsigned long scl_fall_ns,
- unsigned long sda_fall_ns,
- unsigned long *div_low, unsigned long *div_high)
+static int rk3x_i2c_calc_divs(unsigned long clk_rate,
+ struct i2c_timings *t_input,
+ unsigned long *div_low,
+ unsigned long *div_high)
{
unsigned long spec_min_low_ns, spec_min_high_ns;
unsigned long spec_setup_start, spec_max_data_hold_ns;
@@ -472,12 +465,12 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long scl_rate,
int ret = 0;
/* Only support standard-mode and fast-mode */
- if (WARN_ON(scl_rate > 400000))
- scl_rate = 400000;
+ if (WARN_ON(t_input->bus_freq_hz > 400000))
+ t_input->bus_freq_hz = 400000;
/* prevent scl_rate_khz from becoming 0 */
- if (WARN_ON(scl_rate < 1000))
- scl_rate = 1000;
+ if (WARN_ON(t_input->bus_freq_hz < 1000))
+ t_input->bus_freq_hz = 1000;
/*
* min_low_ns: The minimum number of ns we need to hold low to
@@ -491,7 +484,7 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long scl_rate,
* This is because the i2c host on Rockchip holds the data line
* for half the low time.
*/
- if (scl_rate <= 100000) {
+ if (t_input->bus_freq_hz <= 100000) {
/* Standard-mode */
spec_min_low_ns = 4700;
spec_setup_start = 4700;
@@ -506,7 +499,7 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long scl_rate,
spec_max_data_hold_ns = 900;
data_hold_buffer_ns = 50;
}
- min_high_ns = scl_rise_ns + spec_min_high_ns;
+ min_high_ns = t_input->scl_rise_ns + spec_min_high_ns;
/*
* Timings for repeated start:
@@ -517,18 +510,19 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long scl_rate,
* we meet tSU;STA and tHD;STA times.
*/
min_high_ns = max(min_high_ns,
- DIV_ROUND_UP((scl_rise_ns + spec_setup_start) * 1000, 875));
+ DIV_ROUND_UP((t_input->scl_rise_ns + spec_setup_start) * 1000,
+ 875));
min_high_ns = max(min_high_ns,
- DIV_ROUND_UP((scl_rise_ns + spec_setup_start +
- sda_fall_ns + spec_min_high_ns), 2));
+ DIV_ROUND_UP((t_input->scl_rise_ns + spec_setup_start +
+ t_input->sda_fall_ns + spec_min_high_ns), 2));
- min_low_ns = scl_fall_ns + spec_min_low_ns;
+ min_low_ns = t_input->scl_fall_ns + spec_min_low_ns;
max_low_ns = spec_max_data_hold_ns * 2 - data_hold_buffer_ns;
min_total_ns = min_low_ns + min_high_ns;
/* Adjust to avoid overflow */
clk_rate_khz = DIV_ROUND_UP(clk_rate, 1000);
- scl_rate_khz = scl_rate / 1000;
+ scl_rate_khz = t_input->bus_freq_hz / 1000;
/*
* We need the total div to be >= this number
@@ -620,10 +614,8 @@ static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
u64 t_low_ns, t_high_ns;
int ret;
- ret = rk3x_i2c_calc_divs(clk_rate, i2c->scl_frequency, i2c->scl_rise_ns,
- i2c->scl_fall_ns, i2c->sda_fall_ns,
- &div_low, &div_high);
- WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->scl_frequency);
+ ret = rk3x_i2c_calc_divs(clk_rate, &i2c->t, &div_low, &div_high);
+ WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->t.bus_freq_hz);
clk_enable(i2c->clk);
i2c_writel(i2c, (div_high << 16) | (div_low & 0xffff), REG_CLKDIV);
@@ -634,7 +626,7 @@ static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
dev_dbg(i2c->dev,
"CLK %lukhz, Req %uns, Act low %lluns high %lluns\n",
clk_rate / 1000,
- 1000000000 / i2c->scl_frequency,
+ 1000000000 / i2c->t.bus_freq_hz,
t_low_ns, t_high_ns);
}
@@ -664,9 +656,7 @@ static int rk3x_i2c_clk_notifier_cb(struct notifier_block *nb, unsigned long
switch (event) {
case PRE_RATE_CHANGE:
- if (rk3x_i2c_calc_divs(ndata->new_rate, i2c->scl_frequency,
- i2c->scl_rise_ns, i2c->scl_fall_ns,
- i2c->sda_fall_ns,
+ if (rk3x_i2c_calc_divs(ndata->new_rate, &i2c->t,
&div_low, &div_high) != 0)
return NOTIFY_STOP;
@@ -879,37 +869,8 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
match = of_match_node(rk3x_i2c_match, np);
i2c->soc_data = (struct rk3x_i2c_soc_data *)match->data;
- if (of_property_read_u32(pdev->dev.of_node, "clock-frequency",
- &i2c->scl_frequency)) {
- dev_info(&pdev->dev, "using default SCL frequency: %d\n",
- DEFAULT_SCL_RATE);
- i2c->scl_frequency = DEFAULT_SCL_RATE;
- }
-
- if (i2c->scl_frequency == 0 || i2c->scl_frequency > 400 * 1000) {
- dev_warn(&pdev->dev, "invalid SCL frequency specified.\n");
- dev_warn(&pdev->dev, "using default SCL frequency: %d\n",
- DEFAULT_SCL_RATE);
- i2c->scl_frequency = DEFAULT_SCL_RATE;
- }
-
- /*
- * Read rise and fall time from device tree. If not available use
- * the default maximum timing from the specification.
- */
- if (of_property_read_u32(pdev->dev.of_node, "i2c-scl-rising-time-ns",
- &i2c->scl_rise_ns)) {
- if (i2c->scl_frequency <= 100000)
- i2c->scl_rise_ns = 1000;
- else
- i2c->scl_rise_ns = 300;
- }
- if (of_property_read_u32(pdev->dev.of_node, "i2c-scl-falling-time-ns",
- &i2c->scl_fall_ns))
- i2c->scl_fall_ns = 300;
- if (of_property_read_u32(pdev->dev.of_node, "i2c-sda-falling-time-ns",
- &i2c->sda_fall_ns))
- i2c->sda_fall_ns = i2c->scl_fall_ns;
+ /* use common interface to get I2C timing properties */
+ i2c_parse_fw_timings(&pdev->dev, &i2c->t, true);
strlcpy(i2c->adap.name, "rk3x-i2c", sizeof(i2c->adap.name));
i2c->adap.owner = THIS_MODULE;
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 2/4] i2c: rk3x: add ops to caculate i2c clocks
2016-01-14 12:31 ` David Wu
@ 2016-01-14 12:31 ` David Wu
-1 siblings, 0 replies; 25+ messages in thread
From: David Wu @ 2016-01-14 12:31 UTC (permalink / raw)
To: heiko
Cc: wsa, andy.shevchenko, dianders, huangtao, zyw, cf, xjq, hl,
linux-arm-kernel, linux-rockchip, linux-gpio, linux-kernel,
David Wu, David Wu
From: David Wu <wdc@rock-chips.com>
I2c Controller of rk3x is updated for the rules to caculate clocks.
So it need a new method to caculate i2c clock timing information
for new version. The current method is defined as v0, and new is
v1, next maybe v2......
Signed-off-by: David Wu <david.wu@rock-chips.com>
---
Changes in v3:
- use GENMASK(Andy)
- Too many arguments for ops func, use struct for them(Andy)
changes in v2:
- split patch to three patches(Heiko)
drivers/i2c/busses/i2c-rk3x.c | 80 +++++++++++++++++++++++++++++--------------
1 file changed, 54 insertions(+), 26 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 1d86e8c..185e0f9 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -58,6 +58,11 @@ enum {
#define REG_CON_LASTACK BIT(5) /* 1: send NACK after last received byte */
#define REG_CON_ACTACK BIT(6) /* 1: stop if NACK is received */
+#define VERSION_MASK GENMASK(31, 16)
+#define VERSION_SHIFT 16
+
+#define RK3X_I2C_V0 0x0
+
/* REG_MRXADDR bits */
#define REG_MRXADDR_VALID(x) BIT(24 + (x)) /* [x*8+7:x*8] of MRX[R]ADDR valid */
@@ -90,10 +95,27 @@ struct rk3x_i2c_soc_data {
int grf_offset;
};
+/**
+ * struct rk3x_priv_i2c_timings - rk3x I2C timing information
+ * @div_low: Divider output for low
+ * @div_high: Divider output for high
+ */
+struct rk3x_priv_i2c_timings {
+ unsigned long div_low;
+ unsigned long div_high;
+};
+
+struct rk3x_i2c_ops {
+ int (*calc_clock)(unsigned long,
+ struct i2c_timings *,
+ struct rk3x_priv_i2c_timings *);
+};
+
struct rk3x_i2c {
struct i2c_adapter adap;
struct device *dev;
struct rk3x_i2c_soc_data *soc_data;
+ struct rk3x_i2c_ops ops;
/* Hardware resources */
void __iomem *regs;
@@ -102,6 +124,7 @@ struct rk3x_i2c {
/* Settings */
struct i2c_timings t;
+ struct rk3x_priv_i2c_timings t_priv;
/* Synchronization & notification */
spinlock_t lock;
@@ -431,21 +454,20 @@ out:
}
/**
- * Calculate divider values for desired SCL frequency
+ * Calculate timing clock info values for desired SCL frequency
*
* @clk_rate: I2C input clock rate
- * @t_input: Known I2C timing information.
- * @div_low: Divider output for low
- * @div_high: Divider output for high
+ * @t_input: Known I2C timing information
+ * @t_output: Caculated rk3x private timing information that would
+ * be written into regs
*
* Returns: 0 on success, -EINVAL if the goal SCL rate is too slow. In that case
* a best-effort divider value is returned in divs. If the target rate is
* too high, we silently use the highest possible rate.
*/
-static int rk3x_i2c_calc_divs(unsigned long clk_rate,
- struct i2c_timings *t_input,
- unsigned long *div_low,
- unsigned long *div_high)
+static int rk3x_i2c_v0_calc_clock(unsigned long clk_rate,
+ struct i2c_timings *t_input,
+ struct rk3x_priv_i2c_timings *t_output)
{
unsigned long spec_min_low_ns, spec_min_high_ns;
unsigned long spec_setup_start, spec_max_data_hold_ns;
@@ -553,8 +575,8 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
* Time needed to meet hold requirements is important.
* Just use that.
*/
- *div_low = min_low_div;
- *div_high = min_high_div;
+ t_output->div_low = min_low_div;
+ t_output->div_high = min_high_div;
} else {
/*
* We've got to distribute some time among the low and high
@@ -583,25 +605,25 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
/* 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);
+ t_output->div_low = ideal_low_div;
+ t_output->div_high = min_high_div + (extra_div - extra_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 = *div_low - 1;
- *div_high = *div_high - 1;
+ t_output->div_low = t_output->div_low - 1;
+ t_output->div_high = t_output->div_high - 1;
/* Maximum divider supported by hw is 0xffff */
- if (*div_low > 0xffff) {
- *div_low = 0xffff;
+ if (t_output->div_low > 0xffff) {
+ t_output->div_low = 0xffff;
ret = -EINVAL;
}
- if (*div_high > 0xffff) {
- *div_high = 0xffff;
+ if (t_output->div_high > 0xffff) {
+ t_output->div_high = 0xffff;
ret = -EINVAL;
}
@@ -610,19 +632,21 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
{
- unsigned long div_low, div_high;
u64 t_low_ns, t_high_ns;
int ret;
- ret = rk3x_i2c_calc_divs(clk_rate, &i2c->t, &div_low, &div_high);
+ ret = i2c->ops.calc_clock(clk_rate, &i2c->t, &i2c->t_priv);
WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->t.bus_freq_hz);
clk_enable(i2c->clk);
- i2c_writel(i2c, (div_high << 16) | (div_low & 0xffff), REG_CLKDIV);
+ i2c_writel(i2c, (i2c->t_priv.div_high << 16) |
+ (i2c->t_priv.div_low & 0xffff), REG_CLKDIV);
clk_disable(i2c->clk);
- t_low_ns = div_u64(((u64)div_low + 1) * 8 * 1000000000, clk_rate);
- t_high_ns = div_u64(((u64)div_high + 1) * 8 * 1000000000, clk_rate);
+ t_low_ns = div_u64(((u64)i2c->t_priv.div_low + 1) * 8 * 1000000000,
+ clk_rate);
+ t_high_ns = div_u64(((u64)i2c->t_priv.div_high + 1) * 8 * 1000000000,
+ clk_rate);
dev_dbg(i2c->dev,
"CLK %lukhz, Req %uns, Act low %lluns high %lluns\n",
clk_rate / 1000,
@@ -652,12 +676,11 @@ static int rk3x_i2c_clk_notifier_cb(struct notifier_block *nb, unsigned long
{
struct clk_notifier_data *ndata = data;
struct rk3x_i2c *i2c = container_of(nb, struct rk3x_i2c, clk_rate_nb);
- unsigned long div_low, div_high;
switch (event) {
case PRE_RATE_CHANGE:
- if (rk3x_i2c_calc_divs(ndata->new_rate, &i2c->t,
- &div_low, &div_high) != 0)
+ if (i2c->ops.calc_clock(ndata->new_rate, &i2c->t,
+ &i2c->t_priv) != 0)
return NOTIFY_STOP;
/* scale up */
@@ -861,6 +884,7 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
u32 value;
int irq;
unsigned long clk_rate;
+ unsigned int version;
i2c = devm_kzalloc(&pdev->dev, sizeof(struct rk3x_i2c), GFP_KERNEL);
if (!i2c)
@@ -944,6 +968,10 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, i2c);
+ version = (readl(i2c->regs + REG_CON) & VERSION_MASK) >> VERSION_SHIFT;
+ if (version == RK3X_I2C_V0)
+ i2c->ops.calc_clock = rk3x_i2c_v0_calc_clock;
+
ret = clk_prepare(i2c->clk);
if (ret < 0) {
dev_err(&pdev->dev, "Could not prepare clock\n");
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 2/4] i2c: rk3x: add ops to caculate i2c clocks
@ 2016-01-14 12:31 ` David Wu
0 siblings, 0 replies; 25+ messages in thread
From: David Wu @ 2016-01-14 12:31 UTC (permalink / raw)
To: linux-arm-kernel
From: David Wu <wdc@rock-chips.com>
I2c Controller of rk3x is updated for the rules to caculate clocks.
So it need a new method to caculate i2c clock timing information
for new version. The current method is defined as v0, and new is
v1, next maybe v2......
Signed-off-by: David Wu <david.wu@rock-chips.com>
---
Changes in v3:
- use GENMASK(Andy)
- Too many arguments for ops func, use struct for them(Andy)
changes in v2:
- split patch to three patches(Heiko)
drivers/i2c/busses/i2c-rk3x.c | 80 +++++++++++++++++++++++++++++--------------
1 file changed, 54 insertions(+), 26 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 1d86e8c..185e0f9 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -58,6 +58,11 @@ enum {
#define REG_CON_LASTACK BIT(5) /* 1: send NACK after last received byte */
#define REG_CON_ACTACK BIT(6) /* 1: stop if NACK is received */
+#define VERSION_MASK GENMASK(31, 16)
+#define VERSION_SHIFT 16
+
+#define RK3X_I2C_V0 0x0
+
/* REG_MRXADDR bits */
#define REG_MRXADDR_VALID(x) BIT(24 + (x)) /* [x*8+7:x*8] of MRX[R]ADDR valid */
@@ -90,10 +95,27 @@ struct rk3x_i2c_soc_data {
int grf_offset;
};
+/**
+ * struct rk3x_priv_i2c_timings - rk3x I2C timing information
+ * @div_low: Divider output for low
+ * @div_high: Divider output for high
+ */
+struct rk3x_priv_i2c_timings {
+ unsigned long div_low;
+ unsigned long div_high;
+};
+
+struct rk3x_i2c_ops {
+ int (*calc_clock)(unsigned long,
+ struct i2c_timings *,
+ struct rk3x_priv_i2c_timings *);
+};
+
struct rk3x_i2c {
struct i2c_adapter adap;
struct device *dev;
struct rk3x_i2c_soc_data *soc_data;
+ struct rk3x_i2c_ops ops;
/* Hardware resources */
void __iomem *regs;
@@ -102,6 +124,7 @@ struct rk3x_i2c {
/* Settings */
struct i2c_timings t;
+ struct rk3x_priv_i2c_timings t_priv;
/* Synchronization & notification */
spinlock_t lock;
@@ -431,21 +454,20 @@ out:
}
/**
- * Calculate divider values for desired SCL frequency
+ * Calculate timing clock info values for desired SCL frequency
*
* @clk_rate: I2C input clock rate
- * @t_input: Known I2C timing information.
- * @div_low: Divider output for low
- * @div_high: Divider output for high
+ * @t_input: Known I2C timing information
+ * @t_output: Caculated rk3x private timing information that would
+ * be written into regs
*
* Returns: 0 on success, -EINVAL if the goal SCL rate is too slow. In that case
* a best-effort divider value is returned in divs. If the target rate is
* too high, we silently use the highest possible rate.
*/
-static int rk3x_i2c_calc_divs(unsigned long clk_rate,
- struct i2c_timings *t_input,
- unsigned long *div_low,
- unsigned long *div_high)
+static int rk3x_i2c_v0_calc_clock(unsigned long clk_rate,
+ struct i2c_timings *t_input,
+ struct rk3x_priv_i2c_timings *t_output)
{
unsigned long spec_min_low_ns, spec_min_high_ns;
unsigned long spec_setup_start, spec_max_data_hold_ns;
@@ -553,8 +575,8 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
* Time needed to meet hold requirements is important.
* Just use that.
*/
- *div_low = min_low_div;
- *div_high = min_high_div;
+ t_output->div_low = min_low_div;
+ t_output->div_high = min_high_div;
} else {
/*
* We've got to distribute some time among the low and high
@@ -583,25 +605,25 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
/* 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);
+ t_output->div_low = ideal_low_div;
+ t_output->div_high = min_high_div + (extra_div - extra_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 = *div_low - 1;
- *div_high = *div_high - 1;
+ t_output->div_low = t_output->div_low - 1;
+ t_output->div_high = t_output->div_high - 1;
/* Maximum divider supported by hw is 0xffff */
- if (*div_low > 0xffff) {
- *div_low = 0xffff;
+ if (t_output->div_low > 0xffff) {
+ t_output->div_low = 0xffff;
ret = -EINVAL;
}
- if (*div_high > 0xffff) {
- *div_high = 0xffff;
+ if (t_output->div_high > 0xffff) {
+ t_output->div_high = 0xffff;
ret = -EINVAL;
}
@@ -610,19 +632,21 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
{
- unsigned long div_low, div_high;
u64 t_low_ns, t_high_ns;
int ret;
- ret = rk3x_i2c_calc_divs(clk_rate, &i2c->t, &div_low, &div_high);
+ ret = i2c->ops.calc_clock(clk_rate, &i2c->t, &i2c->t_priv);
WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->t.bus_freq_hz);
clk_enable(i2c->clk);
- i2c_writel(i2c, (div_high << 16) | (div_low & 0xffff), REG_CLKDIV);
+ i2c_writel(i2c, (i2c->t_priv.div_high << 16) |
+ (i2c->t_priv.div_low & 0xffff), REG_CLKDIV);
clk_disable(i2c->clk);
- t_low_ns = div_u64(((u64)div_low + 1) * 8 * 1000000000, clk_rate);
- t_high_ns = div_u64(((u64)div_high + 1) * 8 * 1000000000, clk_rate);
+ t_low_ns = div_u64(((u64)i2c->t_priv.div_low + 1) * 8 * 1000000000,
+ clk_rate);
+ t_high_ns = div_u64(((u64)i2c->t_priv.div_high + 1) * 8 * 1000000000,
+ clk_rate);
dev_dbg(i2c->dev,
"CLK %lukhz, Req %uns, Act low %lluns high %lluns\n",
clk_rate / 1000,
@@ -652,12 +676,11 @@ static int rk3x_i2c_clk_notifier_cb(struct notifier_block *nb, unsigned long
{
struct clk_notifier_data *ndata = data;
struct rk3x_i2c *i2c = container_of(nb, struct rk3x_i2c, clk_rate_nb);
- unsigned long div_low, div_high;
switch (event) {
case PRE_RATE_CHANGE:
- if (rk3x_i2c_calc_divs(ndata->new_rate, &i2c->t,
- &div_low, &div_high) != 0)
+ if (i2c->ops.calc_clock(ndata->new_rate, &i2c->t,
+ &i2c->t_priv) != 0)
return NOTIFY_STOP;
/* scale up */
@@ -861,6 +884,7 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
u32 value;
int irq;
unsigned long clk_rate;
+ unsigned int version;
i2c = devm_kzalloc(&pdev->dev, sizeof(struct rk3x_i2c), GFP_KERNEL);
if (!i2c)
@@ -944,6 +968,10 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, i2c);
+ version = (readl(i2c->regs + REG_CON) & VERSION_MASK) >> VERSION_SHIFT;
+ if (version == RK3X_I2C_V0)
+ i2c->ops.calc_clock = rk3x_i2c_v0_calc_clock;
+
ret = clk_prepare(i2c->clk);
if (ret < 0) {
dev_err(&pdev->dev, "Could not prepare clock\n");
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 3/4] i2c: rk3x: new method to caculate i2c clocks
2016-01-14 12:31 ` David Wu
@ 2016-01-14 12:31 ` David Wu
-1 siblings, 0 replies; 25+ messages in thread
From: David Wu @ 2016-01-14 12:31 UTC (permalink / raw)
To: heiko
Cc: wsa, andy.shevchenko, dianders, huangtao, zyw, cf, xjq, hl,
linux-arm-kernel, linux-rockchip, linux-gpio, linux-kernel,
David Wu
There was an timing issue about "repeated start" time at the I2C
controller of version0, controller appears to drop SDA at .875x (7/8)
programmed clk high. The rule(.875x) isn't enough to meet tSU;STA
requirements on 100k's Standard-mode. To resolve this issue,
data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
timing information are added, new rules are designed to calculate
the timing information at new v1.
There are two examples for clocks calculated by the rules, not contain
hardware elements like scl_rise time, scl_fall time and sda_rise time:
1. Standard-mode:
Source Pclk: 80M, Target scl: 100K, Final scl: 100K;
Tpclk = 12.5ns;
divl = 53, divh = 45;
l = 54, h = 46;
tLow = 5.400us, tHigh = 4.600us;
start_setup_cnt = 1, stop_setup_cnt = 0;
u = 2, p = 1;
tSU;sta = (8h * u + 1) * T = 9.2125us;
tHD;sta = [8h * (u + 1) - 1] * T = 13.7875us;
tSU;sto = (8h * p + 1) * T = 4.6125;
data_upd_st = 2;
s = data_upd_st + 1 = 3;
tHD;sda = (l * s + 1) * T = 2.038us;
tSU;sda = [(8 - s) * l + 1] * T = 3.388us;
2. Fast-mode:
Source Pclk: 80M, Target scl: 400K, Final scl: 400K;
Tpclk = 12.5ns;
divl = 16, divh = 7;
l = 17, h = 8;
tLow = 1.7us, tHigh = 0.8us;
start_setup_cnt = stop_setup_cnt = 0;
u = p = 1;
tSU;sta = (8h * u + 1) * T = 0.8125us;
tHD;sta = [8h * (u + 1) - 1] * T = 1.5875us;
tSU;sto = (8h * p + 1) * T = 0.8125us;
data_upd_st = 2;
s = data_upd_st + 1 = 3;
tHD;sda = (l * s + 1) * T = 650ns;
tSU;sda = [(8 - l) * s + 1] * T = 1075ns;
It seemed the rules make the timing meet the I2C spec requirements
both Standard-mode and Fast-mode.
Signed-off-by: David Wu <david.wu@rock-chips.com>
---
changes in v3:
- Add more comments
- Calculate counts behind divh and divl
changes in v2:
- split patch to three patches(Heiko)
drivers/i2c/busses/i2c-rk3x.c | 236 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 231 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 185e0f9..b7229a7 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -58,10 +58,15 @@ enum {
#define REG_CON_LASTACK BIT(5) /* 1: send NACK after last received byte */
#define REG_CON_ACTACK BIT(6) /* 1: stop if NACK is received */
+#define REG_CON_SDA_CNT(cnt) ((cnt) << 8)
+#define REG_CON_STA_CNT(cnt) ((cnt) << 12)
+#define REG_CON_STO_CNT(cnt) ((cnt) << 14)
+
#define VERSION_MASK GENMASK(31, 16)
#define VERSION_SHIFT 16
#define RK3X_I2C_V0 0x0
+#define RK3X_I2C_V1 0x1
/* REG_MRXADDR bits */
#define REG_MRXADDR_VALID(x) BIT(24 + (x)) /* [x*8+7:x*8] of MRX[R]ADDR valid */
@@ -99,10 +104,16 @@ struct rk3x_i2c_soc_data {
* struct rk3x_priv_i2c_timings - rk3x I2C timing information
* @div_low: Divider output for low
* @div_high: Divider output for high
+ * @thd_sda_count: SDA update point config used to adjust sda setup/hold time
+ * @tsu_sta_count: Start setup config for setup start time and hold start time
+ * @tsu_sto_count: Stop setup config for setup stop time
*/
struct rk3x_priv_i2c_timings {
unsigned long div_low;
unsigned long div_high;
+ unsigned int thd_sda_count;
+ unsigned int tsu_sta_count;
+ unsigned int tsu_sto_count;
};
struct rk3x_i2c_ops {
@@ -154,6 +165,13 @@ static inline u32 i2c_readl(struct rk3x_i2c *i2c, unsigned int offset)
return readl(i2c->regs + offset);
}
+static inline u32 rk3x_i2c_get_con_count(struct rk3x_i2c *i2c)
+{
+ return REG_CON_SDA_CNT(i2c->t_priv.thd_sda_count) |
+ REG_CON_STA_CNT(i2c->t_priv.tsu_sta_count) |
+ REG_CON_STO_CNT(i2c->t_priv.tsu_sto_count);
+}
+
/* Reset all interrupt pending bits */
static inline void rk3x_i2c_clean_ipd(struct rk3x_i2c *i2c)
{
@@ -165,13 +183,13 @@ static inline void rk3x_i2c_clean_ipd(struct rk3x_i2c *i2c)
*/
static void rk3x_i2c_start(struct rk3x_i2c *i2c)
{
- u32 val;
+ u32 val = rk3x_i2c_get_con_count(i2c);
rk3x_i2c_clean_ipd(i2c);
i2c_writel(i2c, REG_INT_START, REG_IEN);
/* enable adapter with correct mode, send START condition */
- val = REG_CON_EN | REG_CON_MOD(i2c->mode) | REG_CON_START;
+ val = val | REG_CON_EN | REG_CON_MOD(i2c->mode) | REG_CON_START;
/* if we want to react to NACK, set ACTACK bit */
if (!(i2c->msg->flags & I2C_M_IGNORE_NAK))
@@ -212,7 +230,7 @@ static void rk3x_i2c_stop(struct rk3x_i2c *i2c, int error)
* get the intended effect by resetting its internal state
* and issuing an ordinary START.
*/
- i2c_writel(i2c, 0, REG_CON);
+ i2c_writel(i2c, rk3x_i2c_get_con_count(i2c), REG_CON);
/* signal that we are finished with the current msg */
wake_up(&i2c->wait);
@@ -630,6 +648,211 @@ static int rk3x_i2c_v0_calc_clock(unsigned long clk_rate,
return ret;
}
+/**
+ * Calculate timing clock info values for desired SCL frequency
+ *
+ * @clk_rate: I2C input clock rate
+ * @t_input: Known I2C timing information
+ * @t_output: Caculated rk3x private timing information that would
+ * be written into regs
+ * Returns: 0 on success, -EINVAL if the goal SCL rate is too slow. In that case
+ * a best-effort divider value is returned in divs. If the target rate is
+ * too high, we silently use the highest possible rate.
+ * The following formulas are v1's method to calculate clock.
+ *
+ * l = divl + 1;
+ * h = divh + 1;
+ * s = data_upd_st + 1;
+ * u = start_setup_cnt + 1;
+ * p = stop_setup_cnt + 1;
+ * T = Tclk_i2c;
+
+ * tHigh = 8 * h * T;
+ * tLow = 8 * l * T;
+
+ * tHD;sda = (l * s + 1) * T;
+ * tSU;sda = [(8 - s) * l + 1] * T;
+ * tI2C = 8 * (l + h) * T;
+
+ * tSU;sta = (8h * u + 1) * T;
+ * tHD;sta = [8h * (u + 1) - 1] * T;
+ * tSU;sto = (8h * p + 1) * T;
+ */
+static int rk3x_i2c_v1_calc_clock(unsigned long clk_rate,
+ struct i2c_timings *t_input,
+ struct rk3x_priv_i2c_timings *t_output)
+{
+ unsigned long spec_min_low_ns, spec_min_high_ns;
+ unsigned long spec_min_setup_start_ns, spec_min_stop_setup_ns;
+ unsigned long spec_min_data_setup_ns, spec_max_data_hold_ns;
+
+ unsigned long min_low_ns, min_high_ns, min_total_ns;
+ unsigned long min_setup_start_ns, min_setup_data_ns;
+ unsigned long min_stop_setup_ns, max_hold_data_ns;
+
+ unsigned long clk_rate_khz, scl_rate_khz;
+
+ unsigned long min_low_div, min_high_div;
+
+ unsigned long min_div_for_hold, min_total_div;
+ unsigned long extra_div, extra_low_div;
+ unsigned long data_hd_cnt;
+
+ int ret = 0;
+
+ /* Support standard-mode and fast-mode */
+ if (WARN_ON(t_input->bus_freq_hz > 400000))
+ t_input->bus_freq_hz = 400000;
+
+ /* prevent scl_rate_khz from becoming 0 */
+ if (WARN_ON(t_input->bus_freq_hz < 1000))
+ t_input->bus_freq_hz = 1000;
+
+ /*
+ * min_low_ns: The minimum number of ns we need to hold low to
+ * meet I2C specification, should include fall time.
+ * min_high_ns: The minimum number of ns we need to hold high to
+ * meet I2C specification, should include rise time.
+ */
+ if (t_input->bus_freq_hz <= 100000) {
+ spec_min_low_ns = 4700;
+ spec_min_high_ns = 4000;
+
+ spec_min_setup_start_ns = 4700;
+ spec_min_stop_setup_ns = 4000;
+
+ spec_min_data_setup_ns = 250;
+ spec_max_data_hold_ns = 3450;
+ } else if (t_input->bus_freq_hz <= 400000) {
+ spec_min_low_ns = 1300;
+ spec_min_high_ns = 600;
+
+ spec_min_setup_start_ns = 600;
+ spec_min_stop_setup_ns = 600;
+
+ spec_min_data_setup_ns = 100;
+ spec_max_data_hold_ns = 900;
+ }
+
+ /* caculate min-divh and min-divl */
+ clk_rate_khz = DIV_ROUND_UP(clk_rate, 1000);
+ scl_rate_khz = t_input->bus_freq_hz / 1000;
+ min_total_div = DIV_ROUND_UP(clk_rate_khz, scl_rate_khz * 8);
+
+ min_high_ns = t_input->scl_rise_ns + spec_min_high_ns;
+ min_high_div = DIV_ROUND_UP(clk_rate_khz * min_high_ns, 8 * 1000000);
+
+ min_low_ns = t_input->scl_fall_ns + spec_min_low_ns;
+ min_low_div = DIV_ROUND_UP(clk_rate_khz * min_low_ns, 8 * 1000000);
+
+ /* Final divh and divl must be greater than 0, otherwise the
+ * hardware would not output the i2c clk.
+ */
+ if (min_high_div <= 1)
+ min_high_div = 2;
+ if (min_low_div <= 1)
+ min_low_div = 2;
+
+ /* These are the min dividers needed for min hold times. */
+ min_div_for_hold = (min_low_div + min_high_div);
+ min_total_ns = min_low_ns + min_high_ns;
+
+ /*
+ * This is the maximum divider so we don't go over the maximum.
+ * We don't round up here (we round down) since this is a maximum.
+ */
+ if (min_div_for_hold >= min_total_div) {
+ /*
+ * Time needed to meet hold requirements is important.
+ * Just use that.
+ */
+ t_output->div_low = min_low_div;
+ t_output->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.
+ * We'll try to split things up by the scale of min_low_div and
+ * min_high_div, biasing slightly towards having a higher div
+ * for low (spend more time low).
+ */
+ extra_div = min_total_div - min_div_for_hold;
+ extra_low_div = DIV_ROUND_UP(min_low_div * extra_div,
+ min_div_for_hold);
+
+ t_output->div_low = min_low_div + extra_low_div;
+ t_output->div_high = min_high_div + (extra_div - extra_low_div);
+ }
+
+ /*
+ * calculate sda data hold count by the rules, thd_sda_count:3
+ * is a appropriate value to reduce calculated times.
+ * tHD;sda = (l * s + 1) * T
+ * tSU;sda = ((8 - s) * l + 1) * T
+ */
+ for (data_hd_cnt = 3; data_hd_cnt >= 0; data_hd_cnt--) {
+ max_hold_data_ns = DIV_ROUND_UP((data_hd_cnt
+ * (t_output->div_low) + 1)
+ * 1000000, clk_rate_khz);
+ min_setup_data_ns = DIV_ROUND_UP(((8 - data_hd_cnt)
+ * (t_output->div_low) + 1)
+ * 1000000, clk_rate_khz);
+ if ((max_hold_data_ns < spec_max_data_hold_ns) &&
+ (min_setup_data_ns > spec_min_data_setup_ns)) {
+ t_output->thd_sda_count = data_hd_cnt;
+ break;
+ }
+ }
+
+ /*
+ * calculate start setup count, and we aren't care tHD;STA.
+ * If the start setup count meets the rule of tSU;sta, it also
+ * meets the rule of tHD;STA.
+ * tSU;sta = (8h * u + 1) * T
+ * tHD;sta = [8h * (u + 1) - 1] * T
+ */
+ min_setup_start_ns = t_input->scl_rise_ns + spec_min_setup_start_ns;
+ t_output->tsu_sta_count = DIV_ROUND_UP(clk_rate_khz * min_setup_start_ns
+ - 1000000, 8 * 1000000 * (t_output->div_high));
+
+ /*
+ * calculate start setup count by the rule:
+ * tSU;sto =(8h * p + 1) * T
+ */
+ min_stop_setup_ns = t_input->scl_rise_ns + spec_min_stop_setup_ns;
+ t_output->tsu_sto_count = DIV_ROUND_UP(clk_rate_khz * min_stop_setup_ns
+ - 1000000, 8 * 1000000 * (t_output->div_high));
+
+ /*
+ * Adjust to the fact that the hardware has an implicit "+1".
+ * NOTE: Above calculations always produce div_low > 0 and div_high > 0.
+ */
+ t_output->div_low -= 1;
+ t_output->div_high -= 1;
+
+ /* Maximum divider supported by hw is 0xffff */
+ if (t_output->div_low > 0xffff) {
+ t_output->div_low = 0xffff;
+ ret = -EINVAL;
+ }
+
+ if (t_output->div_high > 0xffff) {
+ t_output->div_high = 0xffff;
+ ret = -EINVAL;
+ }
+
+ /*
+ * Adjust to the fact that the hardware has an implicit "+1".
+ * NOTE: Above calculations always produce thd_sda_count > 0,
+ * tsu_sta_count > 0 and tsu_sta_count > 0.
+ */
+ t_output->thd_sda_count -= 1;
+ t_output->tsu_sta_count -= 1;
+ t_output->tsu_sto_count -= 1;
+
+ return ret;
+}
+
static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
{
u64 t_low_ns, t_high_ns;
@@ -829,7 +1052,8 @@ static int rk3x_i2c_xfer(struct i2c_adapter *adap,
/* Force a STOP condition without interrupt */
i2c_writel(i2c, 0, REG_IEN);
- i2c_writel(i2c, REG_CON_EN | REG_CON_STOP, REG_CON);
+ i2c_writel(i2c, rk3x_i2c_get_con_count(i2c) |
+ REG_CON_EN | REG_CON_STOP, REG_CON);
i2c->state = STATE_IDLE;
@@ -969,7 +1193,9 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, i2c);
version = (readl(i2c->regs + REG_CON) & VERSION_MASK) >> VERSION_SHIFT;
- if (version == RK3X_I2C_V0)
+ if (version == RK3X_I2C_V1)
+ i2c->ops.calc_clock = rk3x_i2c_v1_calc_clock;
+ else
i2c->ops.calc_clock = rk3x_i2c_v0_calc_clock;
ret = clk_prepare(i2c->clk);
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 3/4] i2c: rk3x: new method to caculate i2c clocks
@ 2016-01-14 12:31 ` David Wu
0 siblings, 0 replies; 25+ messages in thread
From: David Wu @ 2016-01-14 12:31 UTC (permalink / raw)
To: linux-arm-kernel
There was an timing issue about "repeated start" time at the I2C
controller of version0, controller appears to drop SDA at .875x (7/8)
programmed clk high. The rule(.875x) isn't enough to meet tSU;STA
requirements on 100k's Standard-mode. To resolve this issue,
data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
timing information are added, new rules are designed to calculate
the timing information at new v1.
There are two examples for clocks calculated by the rules, not contain
hardware elements like scl_rise time, scl_fall time and sda_rise time:
1. Standard-mode:
Source Pclk: 80M, Target scl: 100K, Final scl: 100K;
Tpclk = 12.5ns;
divl = 53, divh = 45;
l = 54, h = 46;
tLow = 5.400us, tHigh = 4.600us;
start_setup_cnt = 1, stop_setup_cnt = 0;
u = 2, p = 1;
tSU;sta = (8h * u + 1) * T = 9.2125us;
tHD;sta = [8h * (u + 1) - 1] * T = 13.7875us;
tSU;sto = (8h * p + 1) * T = 4.6125;
data_upd_st = 2;
s = data_upd_st + 1 = 3;
tHD;sda = (l * s + 1) * T = 2.038us;
tSU;sda = [(8 - s) * l + 1] * T = 3.388us;
2. Fast-mode:
Source Pclk: 80M, Target scl: 400K, Final scl: 400K;
Tpclk = 12.5ns;
divl = 16, divh = 7;
l = 17, h = 8;
tLow = 1.7us, tHigh = 0.8us;
start_setup_cnt = stop_setup_cnt = 0;
u = p = 1;
tSU;sta = (8h * u + 1) * T = 0.8125us;
tHD;sta = [8h * (u + 1) - 1] * T = 1.5875us;
tSU;sto = (8h * p + 1) * T = 0.8125us;
data_upd_st = 2;
s = data_upd_st + 1 = 3;
tHD;sda = (l * s + 1) * T = 650ns;
tSU;sda = [(8 - l) * s + 1] * T = 1075ns;
It seemed the rules make the timing meet the I2C spec requirements
both Standard-mode and Fast-mode.
Signed-off-by: David Wu <david.wu@rock-chips.com>
---
changes in v3:
- Add more comments
- Calculate counts behind divh and divl
changes in v2:
- split patch to three patches(Heiko)
drivers/i2c/busses/i2c-rk3x.c | 236 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 231 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 185e0f9..b7229a7 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -58,10 +58,15 @@ enum {
#define REG_CON_LASTACK BIT(5) /* 1: send NACK after last received byte */
#define REG_CON_ACTACK BIT(6) /* 1: stop if NACK is received */
+#define REG_CON_SDA_CNT(cnt) ((cnt) << 8)
+#define REG_CON_STA_CNT(cnt) ((cnt) << 12)
+#define REG_CON_STO_CNT(cnt) ((cnt) << 14)
+
#define VERSION_MASK GENMASK(31, 16)
#define VERSION_SHIFT 16
#define RK3X_I2C_V0 0x0
+#define RK3X_I2C_V1 0x1
/* REG_MRXADDR bits */
#define REG_MRXADDR_VALID(x) BIT(24 + (x)) /* [x*8+7:x*8] of MRX[R]ADDR valid */
@@ -99,10 +104,16 @@ struct rk3x_i2c_soc_data {
* struct rk3x_priv_i2c_timings - rk3x I2C timing information
* @div_low: Divider output for low
* @div_high: Divider output for high
+ * @thd_sda_count: SDA update point config used to adjust sda setup/hold time
+ * @tsu_sta_count: Start setup config for setup start time and hold start time
+ * @tsu_sto_count: Stop setup config for setup stop time
*/
struct rk3x_priv_i2c_timings {
unsigned long div_low;
unsigned long div_high;
+ unsigned int thd_sda_count;
+ unsigned int tsu_sta_count;
+ unsigned int tsu_sto_count;
};
struct rk3x_i2c_ops {
@@ -154,6 +165,13 @@ static inline u32 i2c_readl(struct rk3x_i2c *i2c, unsigned int offset)
return readl(i2c->regs + offset);
}
+static inline u32 rk3x_i2c_get_con_count(struct rk3x_i2c *i2c)
+{
+ return REG_CON_SDA_CNT(i2c->t_priv.thd_sda_count) |
+ REG_CON_STA_CNT(i2c->t_priv.tsu_sta_count) |
+ REG_CON_STO_CNT(i2c->t_priv.tsu_sto_count);
+}
+
/* Reset all interrupt pending bits */
static inline void rk3x_i2c_clean_ipd(struct rk3x_i2c *i2c)
{
@@ -165,13 +183,13 @@ static inline void rk3x_i2c_clean_ipd(struct rk3x_i2c *i2c)
*/
static void rk3x_i2c_start(struct rk3x_i2c *i2c)
{
- u32 val;
+ u32 val = rk3x_i2c_get_con_count(i2c);
rk3x_i2c_clean_ipd(i2c);
i2c_writel(i2c, REG_INT_START, REG_IEN);
/* enable adapter with correct mode, send START condition */
- val = REG_CON_EN | REG_CON_MOD(i2c->mode) | REG_CON_START;
+ val = val | REG_CON_EN | REG_CON_MOD(i2c->mode) | REG_CON_START;
/* if we want to react to NACK, set ACTACK bit */
if (!(i2c->msg->flags & I2C_M_IGNORE_NAK))
@@ -212,7 +230,7 @@ static void rk3x_i2c_stop(struct rk3x_i2c *i2c, int error)
* get the intended effect by resetting its internal state
* and issuing an ordinary START.
*/
- i2c_writel(i2c, 0, REG_CON);
+ i2c_writel(i2c, rk3x_i2c_get_con_count(i2c), REG_CON);
/* signal that we are finished with the current msg */
wake_up(&i2c->wait);
@@ -630,6 +648,211 @@ static int rk3x_i2c_v0_calc_clock(unsigned long clk_rate,
return ret;
}
+/**
+ * Calculate timing clock info values for desired SCL frequency
+ *
+ * @clk_rate: I2C input clock rate
+ * @t_input: Known I2C timing information
+ * @t_output: Caculated rk3x private timing information that would
+ * be written into regs
+ * Returns: 0 on success, -EINVAL if the goal SCL rate is too slow. In that case
+ * a best-effort divider value is returned in divs. If the target rate is
+ * too high, we silently use the highest possible rate.
+ * The following formulas are v1's method to calculate clock.
+ *
+ * l = divl + 1;
+ * h = divh + 1;
+ * s = data_upd_st + 1;
+ * u = start_setup_cnt + 1;
+ * p = stop_setup_cnt + 1;
+ * T = Tclk_i2c;
+
+ * tHigh = 8 * h * T;
+ * tLow = 8 * l * T;
+
+ * tHD;sda = (l * s + 1) * T;
+ * tSU;sda = [(8 - s) * l + 1] * T;
+ * tI2C = 8 * (l + h) * T;
+
+ * tSU;sta = (8h * u + 1) * T;
+ * tHD;sta = [8h * (u + 1) - 1] * T;
+ * tSU;sto = (8h * p + 1) * T;
+ */
+static int rk3x_i2c_v1_calc_clock(unsigned long clk_rate,
+ struct i2c_timings *t_input,
+ struct rk3x_priv_i2c_timings *t_output)
+{
+ unsigned long spec_min_low_ns, spec_min_high_ns;
+ unsigned long spec_min_setup_start_ns, spec_min_stop_setup_ns;
+ unsigned long spec_min_data_setup_ns, spec_max_data_hold_ns;
+
+ unsigned long min_low_ns, min_high_ns, min_total_ns;
+ unsigned long min_setup_start_ns, min_setup_data_ns;
+ unsigned long min_stop_setup_ns, max_hold_data_ns;
+
+ unsigned long clk_rate_khz, scl_rate_khz;
+
+ unsigned long min_low_div, min_high_div;
+
+ unsigned long min_div_for_hold, min_total_div;
+ unsigned long extra_div, extra_low_div;
+ unsigned long data_hd_cnt;
+
+ int ret = 0;
+
+ /* Support standard-mode and fast-mode */
+ if (WARN_ON(t_input->bus_freq_hz > 400000))
+ t_input->bus_freq_hz = 400000;
+
+ /* prevent scl_rate_khz from becoming 0 */
+ if (WARN_ON(t_input->bus_freq_hz < 1000))
+ t_input->bus_freq_hz = 1000;
+
+ /*
+ * min_low_ns: The minimum number of ns we need to hold low to
+ * meet I2C specification, should include fall time.
+ * min_high_ns: The minimum number of ns we need to hold high to
+ * meet I2C specification, should include rise time.
+ */
+ if (t_input->bus_freq_hz <= 100000) {
+ spec_min_low_ns = 4700;
+ spec_min_high_ns = 4000;
+
+ spec_min_setup_start_ns = 4700;
+ spec_min_stop_setup_ns = 4000;
+
+ spec_min_data_setup_ns = 250;
+ spec_max_data_hold_ns = 3450;
+ } else if (t_input->bus_freq_hz <= 400000) {
+ spec_min_low_ns = 1300;
+ spec_min_high_ns = 600;
+
+ spec_min_setup_start_ns = 600;
+ spec_min_stop_setup_ns = 600;
+
+ spec_min_data_setup_ns = 100;
+ spec_max_data_hold_ns = 900;
+ }
+
+ /* caculate min-divh and min-divl */
+ clk_rate_khz = DIV_ROUND_UP(clk_rate, 1000);
+ scl_rate_khz = t_input->bus_freq_hz / 1000;
+ min_total_div = DIV_ROUND_UP(clk_rate_khz, scl_rate_khz * 8);
+
+ min_high_ns = t_input->scl_rise_ns + spec_min_high_ns;
+ min_high_div = DIV_ROUND_UP(clk_rate_khz * min_high_ns, 8 * 1000000);
+
+ min_low_ns = t_input->scl_fall_ns + spec_min_low_ns;
+ min_low_div = DIV_ROUND_UP(clk_rate_khz * min_low_ns, 8 * 1000000);
+
+ /* Final divh and divl must be greater than 0, otherwise the
+ * hardware would not output the i2c clk.
+ */
+ if (min_high_div <= 1)
+ min_high_div = 2;
+ if (min_low_div <= 1)
+ min_low_div = 2;
+
+ /* These are the min dividers needed for min hold times. */
+ min_div_for_hold = (min_low_div + min_high_div);
+ min_total_ns = min_low_ns + min_high_ns;
+
+ /*
+ * This is the maximum divider so we don't go over the maximum.
+ * We don't round up here (we round down) since this is a maximum.
+ */
+ if (min_div_for_hold >= min_total_div) {
+ /*
+ * Time needed to meet hold requirements is important.
+ * Just use that.
+ */
+ t_output->div_low = min_low_div;
+ t_output->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.
+ * We'll try to split things up by the scale of min_low_div and
+ * min_high_div, biasing slightly towards having a higher div
+ * for low (spend more time low).
+ */
+ extra_div = min_total_div - min_div_for_hold;
+ extra_low_div = DIV_ROUND_UP(min_low_div * extra_div,
+ min_div_for_hold);
+
+ t_output->div_low = min_low_div + extra_low_div;
+ t_output->div_high = min_high_div + (extra_div - extra_low_div);
+ }
+
+ /*
+ * calculate sda data hold count by the rules, thd_sda_count:3
+ * is a appropriate value to reduce calculated times.
+ * tHD;sda = (l * s + 1) * T
+ * tSU;sda = ((8 - s) * l + 1) * T
+ */
+ for (data_hd_cnt = 3; data_hd_cnt >= 0; data_hd_cnt--) {
+ max_hold_data_ns = DIV_ROUND_UP((data_hd_cnt
+ * (t_output->div_low) + 1)
+ * 1000000, clk_rate_khz);
+ min_setup_data_ns = DIV_ROUND_UP(((8 - data_hd_cnt)
+ * (t_output->div_low) + 1)
+ * 1000000, clk_rate_khz);
+ if ((max_hold_data_ns < spec_max_data_hold_ns) &&
+ (min_setup_data_ns > spec_min_data_setup_ns)) {
+ t_output->thd_sda_count = data_hd_cnt;
+ break;
+ }
+ }
+
+ /*
+ * calculate start setup count, and we aren't care tHD;STA.
+ * If the start setup count meets the rule of tSU;sta, it also
+ * meets the rule of tHD;STA.
+ * tSU;sta = (8h * u + 1) * T
+ * tHD;sta = [8h * (u + 1) - 1] * T
+ */
+ min_setup_start_ns = t_input->scl_rise_ns + spec_min_setup_start_ns;
+ t_output->tsu_sta_count = DIV_ROUND_UP(clk_rate_khz * min_setup_start_ns
+ - 1000000, 8 * 1000000 * (t_output->div_high));
+
+ /*
+ * calculate start setup count by the rule:
+ * tSU;sto =(8h * p + 1) * T
+ */
+ min_stop_setup_ns = t_input->scl_rise_ns + spec_min_stop_setup_ns;
+ t_output->tsu_sto_count = DIV_ROUND_UP(clk_rate_khz * min_stop_setup_ns
+ - 1000000, 8 * 1000000 * (t_output->div_high));
+
+ /*
+ * Adjust to the fact that the hardware has an implicit "+1".
+ * NOTE: Above calculations always produce div_low > 0 and div_high > 0.
+ */
+ t_output->div_low -= 1;
+ t_output->div_high -= 1;
+
+ /* Maximum divider supported by hw is 0xffff */
+ if (t_output->div_low > 0xffff) {
+ t_output->div_low = 0xffff;
+ ret = -EINVAL;
+ }
+
+ if (t_output->div_high > 0xffff) {
+ t_output->div_high = 0xffff;
+ ret = -EINVAL;
+ }
+
+ /*
+ * Adjust to the fact that the hardware has an implicit "+1".
+ * NOTE: Above calculations always produce thd_sda_count > 0,
+ * tsu_sta_count > 0 and tsu_sta_count > 0.
+ */
+ t_output->thd_sda_count -= 1;
+ t_output->tsu_sta_count -= 1;
+ t_output->tsu_sto_count -= 1;
+
+ return ret;
+}
+
static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
{
u64 t_low_ns, t_high_ns;
@@ -829,7 +1052,8 @@ static int rk3x_i2c_xfer(struct i2c_adapter *adap,
/* Force a STOP condition without interrupt */
i2c_writel(i2c, 0, REG_IEN);
- i2c_writel(i2c, REG_CON_EN | REG_CON_STOP, REG_CON);
+ i2c_writel(i2c, rk3x_i2c_get_con_count(i2c) |
+ REG_CON_EN | REG_CON_STOP, REG_CON);
i2c->state = STATE_IDLE;
@@ -969,7 +1193,9 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, i2c);
version = (readl(i2c->regs + REG_CON) & VERSION_MASK) >> VERSION_SHIFT;
- if (version == RK3X_I2C_V0)
+ if (version == RK3X_I2C_V1)
+ i2c->ops.calc_clock = rk3x_i2c_v1_calc_clock;
+ else
i2c->ops.calc_clock = rk3x_i2c_v0_calc_clock;
ret = clk_prepare(i2c->clk);
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 4/4] i2c: rk3x: support I2C Highspeed Mode
2016-01-14 12:31 ` David Wu
@ 2016-01-14 12:31 ` David Wu
-1 siblings, 0 replies; 25+ messages in thread
From: David Wu @ 2016-01-14 12:31 UTC (permalink / raw)
To: heiko
Cc: wsa, andy.shevchenko, dianders, huangtao, zyw, cf, xjq, hl,
linux-arm-kernel, linux-rockchip, linux-gpio, linux-kernel,
David Wu
The i2c controller of new version1 supports highspeed mode,
1.7M and 3.4M rate. It also could be calculated clocks by the rules.
Note: The final divs would be effected a lot by hardware elements like
scl_rise_ns, scl_fall_ns and sda_rise_ns.
There are two examples of div calculated by the rules, not contain
hardware elements like scl_rise time, scl_fall time and sda_rise time:
1. scl: 1.7M:
Source Pclk: 200M, Target scl: 1700K, Final scl: 1.667K;
Tpclk = 5ns;
divl = 9, divh = 4;
l = 10, h = 5;
tLow = 400ns, tHigh = 200ns;
start_setup_cnt = stop_setup_cnt = 0;
u = p = 1;
tSU;sta = (8h * u + 1) * T = 205ns;
tHD;sta = [8h * (u + 1) - 1] * T = 395ns;
tSU;sto = (8h * p + 1) * T = 205ns;
data_upd_st = 1;
s = data_upd_st + 1 = 2;
tHD;sda = (l * s + 1) * T = 105;
tSU;sda = [(8 - s) * l + 1] * T = 295;
2. scl: 3.4M
Source Pclk: 200M, Target scl: 3400K, Final scl: 3125K;
Tpclk = 5ns;
divl = 4, divh = 2;
l = 5, h = 3;
tLow = 200ns, tHigh = 120ns;
start_setup_cnt = stop_setup_cnt = 1;
u = p = 2;
tSU;sta = (8h * u + 1) * T = 245ns;
tHD;sta = [8h * (u + 1) - 1] * T = 355ns;
tSU;sto = (8h * p + 1) * T = 245ns;
data_upd_st = 1;
s = data_upd_st + 1 = 2;
tHD;sda = (l * s + 1) * T = 55ns;
tSU;sda = [(8 - l) * s + 1] * T = 145ns;
Signed-off-by: David Wu <david.wu@rock-chips.com>
---
Changes in v3: None
changes in v2:
- split patch to three patches(Heiko)
drivers/i2c/busses/i2c-rk3x.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index b7229a7..bb041aa 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -700,9 +700,9 @@ static int rk3x_i2c_v1_calc_clock(unsigned long clk_rate,
int ret = 0;
- /* Support standard-mode and fast-mode */
- if (WARN_ON(t_input->bus_freq_hz > 400000))
- t_input->bus_freq_hz = 400000;
+ /* Support standard-mode, fast-mode and highspeed-mode */
+ if (WARN_ON(t_input->bus_freq_hz > 3400000))
+ t_input->bus_freq_hz = 3400000;
/* prevent scl_rate_khz from becoming 0 */
if (WARN_ON(t_input->bus_freq_hz < 1000))
@@ -732,6 +732,24 @@ static int rk3x_i2c_v1_calc_clock(unsigned long clk_rate,
spec_min_data_setup_ns = 100;
spec_max_data_hold_ns = 900;
+ } else if (t_input->bus_freq_hz <= 1700000) {
+ spec_min_low_ns = 320;
+ spec_min_high_ns = 120;
+
+ spec_min_setup_start_ns = 160;
+ spec_min_stop_setup_ns = 160;
+
+ spec_min_data_setup_ns = 10;
+ spec_max_data_hold_ns = 150;
+ } else {
+ spec_min_low_ns = 160;
+ spec_min_high_ns = 60;
+
+ spec_min_setup_start_ns = 160;
+ spec_min_stop_setup_ns = 160;
+
+ spec_min_data_setup_ns = 10;
+ spec_max_data_hold_ns = 70;
}
/* caculate min-divh and min-divl */
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 4/4] i2c: rk3x: support I2C Highspeed Mode
@ 2016-01-14 12:31 ` David Wu
0 siblings, 0 replies; 25+ messages in thread
From: David Wu @ 2016-01-14 12:31 UTC (permalink / raw)
To: linux-arm-kernel
The i2c controller of new version1 supports highspeed mode,
1.7M and 3.4M rate. It also could be calculated clocks by the rules.
Note: The final divs would be effected a lot by hardware elements like
scl_rise_ns, scl_fall_ns and sda_rise_ns.
There are two examples of div calculated by the rules, not contain
hardware elements like scl_rise time, scl_fall time and sda_rise time:
1. scl: 1.7M:
Source Pclk: 200M, Target scl: 1700K, Final scl: 1.667K;
Tpclk = 5ns;
divl = 9, divh = 4;
l = 10, h = 5;
tLow = 400ns, tHigh = 200ns;
start_setup_cnt = stop_setup_cnt = 0;
u = p = 1;
tSU;sta = (8h * u + 1) * T = 205ns;
tHD;sta = [8h * (u + 1) - 1] * T = 395ns;
tSU;sto = (8h * p + 1) * T = 205ns;
data_upd_st = 1;
s = data_upd_st + 1 = 2;
tHD;sda = (l * s + 1) * T = 105;
tSU;sda = [(8 - s) * l + 1] * T = 295;
2. scl: 3.4M
Source Pclk: 200M, Target scl: 3400K, Final scl: 3125K;
Tpclk = 5ns;
divl = 4, divh = 2;
l = 5, h = 3;
tLow = 200ns, tHigh = 120ns;
start_setup_cnt = stop_setup_cnt = 1;
u = p = 2;
tSU;sta = (8h * u + 1) * T = 245ns;
tHD;sta = [8h * (u + 1) - 1] * T = 355ns;
tSU;sto = (8h * p + 1) * T = 245ns;
data_upd_st = 1;
s = data_upd_st + 1 = 2;
tHD;sda = (l * s + 1) * T = 55ns;
tSU;sda = [(8 - l) * s + 1] * T = 145ns;
Signed-off-by: David Wu <david.wu@rock-chips.com>
---
Changes in v3: None
changes in v2:
- split patch to three patches(Heiko)
drivers/i2c/busses/i2c-rk3x.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index b7229a7..bb041aa 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -700,9 +700,9 @@ static int rk3x_i2c_v1_calc_clock(unsigned long clk_rate,
int ret = 0;
- /* Support standard-mode and fast-mode */
- if (WARN_ON(t_input->bus_freq_hz > 400000))
- t_input->bus_freq_hz = 400000;
+ /* Support standard-mode, fast-mode and highspeed-mode */
+ if (WARN_ON(t_input->bus_freq_hz > 3400000))
+ t_input->bus_freq_hz = 3400000;
/* prevent scl_rate_khz from becoming 0 */
if (WARN_ON(t_input->bus_freq_hz < 1000))
@@ -732,6 +732,24 @@ static int rk3x_i2c_v1_calc_clock(unsigned long clk_rate,
spec_min_data_setup_ns = 100;
spec_max_data_hold_ns = 900;
+ } else if (t_input->bus_freq_hz <= 1700000) {
+ spec_min_low_ns = 320;
+ spec_min_high_ns = 120;
+
+ spec_min_setup_start_ns = 160;
+ spec_min_stop_setup_ns = 160;
+
+ spec_min_data_setup_ns = 10;
+ spec_max_data_hold_ns = 150;
+ } else {
+ spec_min_low_ns = 160;
+ spec_min_high_ns = 60;
+
+ spec_min_setup_start_ns = 160;
+ spec_min_stop_setup_ns = 160;
+
+ spec_min_data_setup_ns = 10;
+ spec_max_data_hold_ns = 70;
}
/* caculate min-divh and min-divl */
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/4] i2c: rk3x: switch to i2c generic dt parsing
2016-01-14 12:31 ` David Wu
@ 2016-01-14 13:05 ` Andy Shevchenko
-1 siblings, 0 replies; 25+ messages in thread
From: Andy Shevchenko @ 2016-01-14 13:05 UTC (permalink / raw)
To: David Wu
Cc: Heiko Stübner, Wolfram Sang, Douglas Anderson, Tao Huang,
Chris Zhong, cf, Jianqun Xu, Lin Huang, linux-arm Mailing List,
linux-rockchip, linux-gpio, linux-kernel
On Thu, Jan 14, 2016 at 2:31 PM, David Wu <david.wu@rock-chips.com> wrote:
> Switch to the new generic functions: i2c_parse_fw_timings().
Nice one!
Reviwed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Minor comments below.
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -437,10 +434,7 @@ out:
> * Calculate divider values for desired SCL frequency
> *
> * @clk_rate: I2C input clock rate
> - * @scl_rate: Desired SCL rate
> - * @scl_rise_ns: How many ns it takes for SCL to rise.
> - * @scl_fall_ns: How many ns it takes for SCL to fall.
> - * @sda_fall_ns: How many ns it takes for SDA to fall.
> + * @t_input: Known I2C timing information.
Perhaps t_input -> t.
> * @div_low: Divider output for low
> * @div_high: Divider output for high
> *
> @@ -448,11 +442,10 @@ out:
> * a best-effort divider value is returned in divs. If the target rate is
> * 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 scl_rise_ns,
> - unsigned long scl_fall_ns,
> - unsigned long sda_fall_ns,
> - unsigned long *div_low, unsigned long *div_high)
> +static int rk3x_i2c_calc_divs(unsigned long clk_rate,
> + struct i2c_timings *t_input,
Ditto.
> + unsigned long *div_low,
> + unsigned long *div_high)
> {
> unsigned long spec_min_low_ns, spec_min_high_ns;
> unsigned long spec_setup_start, spec_max_data_hold_ns;
> @@ -517,18 +510,19 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long scl_rate,
> * we meet tSU;STA and tHD;STA times.
> */
> min_high_ns = max(min_high_ns,
> - DIV_ROUND_UP((scl_rise_ns + spec_setup_start) * 1000, 875));
> + DIV_ROUND_UP((t_input->scl_rise_ns + spec_setup_start) * 1000,
> + 875));
To one line (after above change).
> min_high_ns = max(min_high_ns,
> - DIV_ROUND_UP((scl_rise_ns + spec_setup_start +
> - sda_fall_ns + spec_min_high_ns), 2));
> + DIV_ROUND_UP((t_input->scl_rise_ns + spec_setup_start +
> + t_input->sda_fall_ns + spec_min_high_ns), 2));
Ditto.
> @@ -620,10 +614,8 @@ static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
> u64 t_low_ns, t_high_ns;
> int ret;
>
> - ret = rk3x_i2c_calc_divs(clk_rate, i2c->scl_frequency, i2c->scl_rise_ns,
> - i2c->scl_fall_ns, i2c->sda_fall_ns,
> - &div_low, &div_high);
> - WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->scl_frequency);
> + ret = rk3x_i2c_calc_divs(clk_rate, &i2c->t, &div_low, &div_high);
> + WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->t.bus_freq_hz);
I would recommend to
struct i2c_timings *t = &i2c->t;
+ ret = rk3x_i2c_calc_divs(clk_rate, t, &div_low, &div_high);
+ WARN_ONCE(ret != 0, "Could not reach SCL freq %u", t->bus_freq_hz);
> @@ -634,7 +626,7 @@ static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
> dev_dbg(i2c->dev,
> "CLK %lukhz, Req %uns, Act low %lluns high %lluns\n",
> clk_rate / 1000,
> - 1000000000 / i2c->scl_frequency,
> + 1000000000 / i2c->t.bus_freq_hz,
Ditto.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 1/4] i2c: rk3x: switch to i2c generic dt parsing
@ 2016-01-14 13:05 ` Andy Shevchenko
0 siblings, 0 replies; 25+ messages in thread
From: Andy Shevchenko @ 2016-01-14 13:05 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jan 14, 2016 at 2:31 PM, David Wu <david.wu@rock-chips.com> wrote:
> Switch to the new generic functions: i2c_parse_fw_timings().
Nice one!
Reviwed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Minor comments below.
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -437,10 +434,7 @@ out:
> * Calculate divider values for desired SCL frequency
> *
> * @clk_rate: I2C input clock rate
> - * @scl_rate: Desired SCL rate
> - * @scl_rise_ns: How many ns it takes for SCL to rise.
> - * @scl_fall_ns: How many ns it takes for SCL to fall.
> - * @sda_fall_ns: How many ns it takes for SDA to fall.
> + * @t_input: Known I2C timing information.
Perhaps t_input -> t.
> * @div_low: Divider output for low
> * @div_high: Divider output for high
> *
> @@ -448,11 +442,10 @@ out:
> * a best-effort divider value is returned in divs. If the target rate is
> * 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 scl_rise_ns,
> - unsigned long scl_fall_ns,
> - unsigned long sda_fall_ns,
> - unsigned long *div_low, unsigned long *div_high)
> +static int rk3x_i2c_calc_divs(unsigned long clk_rate,
> + struct i2c_timings *t_input,
Ditto.
> + unsigned long *div_low,
> + unsigned long *div_high)
> {
> unsigned long spec_min_low_ns, spec_min_high_ns;
> unsigned long spec_setup_start, spec_max_data_hold_ns;
> @@ -517,18 +510,19 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long scl_rate,
> * we meet tSU;STA and tHD;STA times.
> */
> min_high_ns = max(min_high_ns,
> - DIV_ROUND_UP((scl_rise_ns + spec_setup_start) * 1000, 875));
> + DIV_ROUND_UP((t_input->scl_rise_ns + spec_setup_start) * 1000,
> + 875));
To one line (after above change).
> min_high_ns = max(min_high_ns,
> - DIV_ROUND_UP((scl_rise_ns + spec_setup_start +
> - sda_fall_ns + spec_min_high_ns), 2));
> + DIV_ROUND_UP((t_input->scl_rise_ns + spec_setup_start +
> + t_input->sda_fall_ns + spec_min_high_ns), 2));
Ditto.
> @@ -620,10 +614,8 @@ static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
> u64 t_low_ns, t_high_ns;
> int ret;
>
> - ret = rk3x_i2c_calc_divs(clk_rate, i2c->scl_frequency, i2c->scl_rise_ns,
> - i2c->scl_fall_ns, i2c->sda_fall_ns,
> - &div_low, &div_high);
> - WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->scl_frequency);
> + ret = rk3x_i2c_calc_divs(clk_rate, &i2c->t, &div_low, &div_high);
> + WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->t.bus_freq_hz);
I would recommend to
struct i2c_timings *t = &i2c->t;
+ ret = rk3x_i2c_calc_divs(clk_rate, t, &div_low, &div_high);
+ WARN_ONCE(ret != 0, "Could not reach SCL freq %u", t->bus_freq_hz);
> @@ -634,7 +626,7 @@ static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
> dev_dbg(i2c->dev,
> "CLK %lukhz, Req %uns, Act low %lluns high %lluns\n",
> clk_rate / 1000,
> - 1000000000 / i2c->scl_frequency,
> + 1000000000 / i2c->t.bus_freq_hz,
Ditto.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 2/4] i2c: rk3x: add ops to caculate i2c clocks
2016-01-14 12:31 ` David Wu
@ 2016-01-14 13:19 ` Andy Shevchenko
-1 siblings, 0 replies; 25+ messages in thread
From: Andy Shevchenko @ 2016-01-14 13:19 UTC (permalink / raw)
To: David Wu
Cc: Heiko Stübner, Wolfram Sang, Douglas Anderson, Tao Huang,
Chris Zhong, cf, Jianqun Xu, Lin Huang, linux-arm Mailing List,
linux-rockchip, linux-gpio, linux-kernel, David Wu
On Thu, Jan 14, 2016 at 2:31 PM, David Wu <david.wu@rock-chips.com> wrote:
> From: David Wu <wdc@rock-chips.com>
>
> I2c Controller of rk3x is updated for the rules to caculate clocks.
> So it need a new method to caculate i2c clock timing information
> for new version. The current method is defined as v0, and new is
> v1, next maybe v2......
Thanks for an update. My comments below.
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -90,10 +95,27 @@ struct rk3x_i2c_soc_data {
> int grf_offset;
> };
>
> +/**
> + * struct rk3x_priv_i2c_timings - rk3x I2C timing information
> + * @div_low: Divider output for low
> + * @div_high: Divider output for high
> + */
> +struct rk3x_priv_i2c_timings {
> + unsigned long div_low;
> + unsigned long div_high;
> +};
> +
> +struct rk3x_i2c_ops {
> + int (*calc_clock)(unsigned long,
> + struct i2c_timings *,
> + struct rk3x_priv_i2c_timings *);
> +};
> +
> struct rk3x_i2c {
> struct i2c_adapter adap;
> struct device *dev;
> struct rk3x_i2c_soc_data *soc_data;
> + struct rk3x_i2c_ops ops;
Not much sense for now to have a struct for one member.
>
> /* Hardware resources */
> void __iomem *regs;
> @@ -102,6 +124,7 @@ struct rk3x_i2c {
>
What about
/* I2C timing settings */
struct i2c_timings t;
/* Divider settings */
int (*calc_divs)(unsigned long,
struct i2c_timings *,
unsigned long *div_low,
unsigned long *div_high);
unsigned long div_low;
unsigned long div_high;
?
As far as I understand you still calculate divider values.
>
> /* Synchronization & notification */
> spinlock_t lock;
> @@ -431,21 +454,20 @@ out:
> }
>
> /**
> - * Calculate divider values for desired SCL frequency
> + * Calculate timing clock info values for desired SCL frequency
> *
> * @clk_rate: I2C input clock rate
> - * @t_input: Known I2C timing information.
> - * @div_low: Divider output for low
> - * @div_high: Divider output for high
For now seems better to leave this prototype.
> + * @t_input: Known I2C timing information
> + * @t_output: Caculated rk3x private timing information that would
> + * be written into regs
> *
> * Returns: 0 on success, -EINVAL if the goal SCL rate is too slow. In that case
> * a best-effort divider value is returned in divs. If the target rate is
> * too high, we silently use the highest possible rate.
> */
> -static int rk3x_i2c_calc_divs(unsigned long clk_rate,
> - struct i2c_timings *t_input,
> - unsigned long *div_low,
> - unsigned long *div_high)
> +static int rk3x_i2c_v0_calc_clock(unsigned long clk_rate,
> + struct i2c_timings *t_input,
> + struct rk3x_priv_i2c_timings *t_output)
> {
> unsigned long spec_min_low_ns, spec_min_high_ns;
> unsigned long spec_setup_start, spec_max_data_hold_ns;
> @@ -583,25 +605,25 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
>
> /* 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);
> + t_output->div_low = ideal_low_div;
> + t_output->div_high = min_high_div + (extra_div - extra_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 = *div_low - 1;
> - *div_high = *div_high - 1;
> + t_output->div_low = t_output->div_low - 1;
-= 1
> + t_output->div_high = t_output->div_high - 1;
Ditto.
But without change of prototype those no needed.
>
> /* Maximum divider supported by hw is 0xffff */
> - if (*div_low > 0xffff) {
> - *div_low = 0xffff;
> + if (t_output->div_low > 0xffff) {
> + t_output->div_low = 0xffff;
> ret = -EINVAL;
> }
>
> - if (*div_high > 0xffff) {
> - *div_high = 0xffff;
> + if (t_output->div_high > 0xffff) {
> + t_output->div_high = 0xffff;
> ret = -EINVAL;
> }
>
> @@ -610,19 +632,21 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
>
> static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
> {
> - unsigned long div_low, div_high;
> u64 t_low_ns, t_high_ns;
> int ret;
>
> - ret = rk3x_i2c_calc_divs(clk_rate, &i2c->t, &div_low, &div_high);
> + ret = i2c->ops.calc_clock(clk_rate, &i2c->t, &i2c->t_priv);
> WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->t.bus_freq_hz);
>
> clk_enable(i2c->clk);
> - i2c_writel(i2c, (div_high << 16) | (div_low & 0xffff), REG_CLKDIV);
> + i2c_writel(i2c, (i2c->t_priv.div_high << 16) |
> + (i2c->t_priv.div_low & 0xffff), REG_CLKDIV);
> clk_disable(i2c->clk);
>
> - t_low_ns = div_u64(((u64)div_low + 1) * 8 * 1000000000, clk_rate);
> - t_high_ns = div_u64(((u64)div_high + 1) * 8 * 1000000000, clk_rate);
> + t_low_ns = div_u64(((u64)i2c->t_priv.div_low + 1) * 8 * 1000000000,
> + clk_rate);
> + t_high_ns = div_u64(((u64)i2c->t_priv.div_high + 1) * 8 * 1000000000,
> + clk_rate);
> dev_dbg(i2c->dev,
> "CLK %lukhz, Req %uns, Act low %lluns high %lluns\n",
> clk_rate / 1000,
> @@ -652,12 +676,11 @@ static int rk3x_i2c_clk_notifier_cb(struct notifier_block *nb, unsigned long
> {
> struct clk_notifier_data *ndata = data;
> struct rk3x_i2c *i2c = container_of(nb, struct rk3x_i2c, clk_rate_nb);
> - unsigned long div_low, div_high;
>
> switch (event) {
> case PRE_RATE_CHANGE:
> - if (rk3x_i2c_calc_divs(ndata->new_rate, &i2c->t,
> - &div_low, &div_high) != 0)
> + if (i2c->ops.calc_clock(ndata->new_rate, &i2c->t,
> + &i2c->t_priv) != 0)
> return NOTIFY_STOP;
>
> /* scale up */
> @@ -861,6 +884,7 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
> u32 value;
> int irq;
> unsigned long clk_rate;
> + unsigned int version;
No need to have a separate variable…
> + version = (readl(i2c->regs + REG_CON) & VERSION_MASK) >> VERSION_SHIFT;
> + if (version == RK3X_I2C_V0)
> + i2c->ops.calc_clock = rk3x_i2c_v0_calc_clock;
value = readl(i2c->regs + REG_CON);
if ((value & VERSION_MASK) >> VERSION_SHIFT) == RK3X_I2C_V0)
i2c->ops.calc_clock = rk3x_i2c_v0_calc_clock;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 2/4] i2c: rk3x: add ops to caculate i2c clocks
@ 2016-01-14 13:19 ` Andy Shevchenko
0 siblings, 0 replies; 25+ messages in thread
From: Andy Shevchenko @ 2016-01-14 13:19 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jan 14, 2016 at 2:31 PM, David Wu <david.wu@rock-chips.com> wrote:
> From: David Wu <wdc@rock-chips.com>
>
> I2c Controller of rk3x is updated for the rules to caculate clocks.
> So it need a new method to caculate i2c clock timing information
> for new version. The current method is defined as v0, and new is
> v1, next maybe v2......
Thanks for an update. My comments below.
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -90,10 +95,27 @@ struct rk3x_i2c_soc_data {
> int grf_offset;
> };
>
> +/**
> + * struct rk3x_priv_i2c_timings - rk3x I2C timing information
> + * @div_low: Divider output for low
> + * @div_high: Divider output for high
> + */
> +struct rk3x_priv_i2c_timings {
> + unsigned long div_low;
> + unsigned long div_high;
> +};
> +
> +struct rk3x_i2c_ops {
> + int (*calc_clock)(unsigned long,
> + struct i2c_timings *,
> + struct rk3x_priv_i2c_timings *);
> +};
> +
> struct rk3x_i2c {
> struct i2c_adapter adap;
> struct device *dev;
> struct rk3x_i2c_soc_data *soc_data;
> + struct rk3x_i2c_ops ops;
Not much sense for now to have a struct for one member.
>
> /* Hardware resources */
> void __iomem *regs;
> @@ -102,6 +124,7 @@ struct rk3x_i2c {
>
What about
/* I2C timing settings */
struct i2c_timings t;
/* Divider settings */
int (*calc_divs)(unsigned long,
struct i2c_timings *,
unsigned long *div_low,
unsigned long *div_high);
unsigned long div_low;
unsigned long div_high;
?
As far as I understand you still calculate divider values.
>
> /* Synchronization & notification */
> spinlock_t lock;
> @@ -431,21 +454,20 @@ out:
> }
>
> /**
> - * Calculate divider values for desired SCL frequency
> + * Calculate timing clock info values for desired SCL frequency
> *
> * @clk_rate: I2C input clock rate
> - * @t_input: Known I2C timing information.
> - * @div_low: Divider output for low
> - * @div_high: Divider output for high
For now seems better to leave this prototype.
> + * @t_input: Known I2C timing information
> + * @t_output: Caculated rk3x private timing information that would
> + * be written into regs
> *
> * Returns: 0 on success, -EINVAL if the goal SCL rate is too slow. In that case
> * a best-effort divider value is returned in divs. If the target rate is
> * too high, we silently use the highest possible rate.
> */
> -static int rk3x_i2c_calc_divs(unsigned long clk_rate,
> - struct i2c_timings *t_input,
> - unsigned long *div_low,
> - unsigned long *div_high)
> +static int rk3x_i2c_v0_calc_clock(unsigned long clk_rate,
> + struct i2c_timings *t_input,
> + struct rk3x_priv_i2c_timings *t_output)
> {
> unsigned long spec_min_low_ns, spec_min_high_ns;
> unsigned long spec_setup_start, spec_max_data_hold_ns;
> @@ -583,25 +605,25 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
>
> /* 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);
> + t_output->div_low = ideal_low_div;
> + t_output->div_high = min_high_div + (extra_div - extra_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 = *div_low - 1;
> - *div_high = *div_high - 1;
> + t_output->div_low = t_output->div_low - 1;
-= 1
> + t_output->div_high = t_output->div_high - 1;
Ditto.
But without change of prototype those no needed.
>
> /* Maximum divider supported by hw is 0xffff */
> - if (*div_low > 0xffff) {
> - *div_low = 0xffff;
> + if (t_output->div_low > 0xffff) {
> + t_output->div_low = 0xffff;
> ret = -EINVAL;
> }
>
> - if (*div_high > 0xffff) {
> - *div_high = 0xffff;
> + if (t_output->div_high > 0xffff) {
> + t_output->div_high = 0xffff;
> ret = -EINVAL;
> }
>
> @@ -610,19 +632,21 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
>
> static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
> {
> - unsigned long div_low, div_high;
> u64 t_low_ns, t_high_ns;
> int ret;
>
> - ret = rk3x_i2c_calc_divs(clk_rate, &i2c->t, &div_low, &div_high);
> + ret = i2c->ops.calc_clock(clk_rate, &i2c->t, &i2c->t_priv);
> WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->t.bus_freq_hz);
>
> clk_enable(i2c->clk);
> - i2c_writel(i2c, (div_high << 16) | (div_low & 0xffff), REG_CLKDIV);
> + i2c_writel(i2c, (i2c->t_priv.div_high << 16) |
> + (i2c->t_priv.div_low & 0xffff), REG_CLKDIV);
> clk_disable(i2c->clk);
>
> - t_low_ns = div_u64(((u64)div_low + 1) * 8 * 1000000000, clk_rate);
> - t_high_ns = div_u64(((u64)div_high + 1) * 8 * 1000000000, clk_rate);
> + t_low_ns = div_u64(((u64)i2c->t_priv.div_low + 1) * 8 * 1000000000,
> + clk_rate);
> + t_high_ns = div_u64(((u64)i2c->t_priv.div_high + 1) * 8 * 1000000000,
> + clk_rate);
> dev_dbg(i2c->dev,
> "CLK %lukhz, Req %uns, Act low %lluns high %lluns\n",
> clk_rate / 1000,
> @@ -652,12 +676,11 @@ static int rk3x_i2c_clk_notifier_cb(struct notifier_block *nb, unsigned long
> {
> struct clk_notifier_data *ndata = data;
> struct rk3x_i2c *i2c = container_of(nb, struct rk3x_i2c, clk_rate_nb);
> - unsigned long div_low, div_high;
>
> switch (event) {
> case PRE_RATE_CHANGE:
> - if (rk3x_i2c_calc_divs(ndata->new_rate, &i2c->t,
> - &div_low, &div_high) != 0)
> + if (i2c->ops.calc_clock(ndata->new_rate, &i2c->t,
> + &i2c->t_priv) != 0)
> return NOTIFY_STOP;
>
> /* scale up */
> @@ -861,6 +884,7 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
> u32 value;
> int irq;
> unsigned long clk_rate;
> + unsigned int version;
No need to have a separate variable?
> + version = (readl(i2c->regs + REG_CON) & VERSION_MASK) >> VERSION_SHIFT;
> + if (version == RK3X_I2C_V0)
> + i2c->ops.calc_clock = rk3x_i2c_v0_calc_clock;
value = readl(i2c->regs + REG_CON);
if ((value & VERSION_MASK) >> VERSION_SHIFT) == RK3X_I2C_V0)
i2c->ops.calc_clock = rk3x_i2c_v0_calc_clock;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/4] i2c: rk3x: new method to caculate i2c clocks
2016-01-14 12:31 ` David Wu
@ 2016-01-14 13:29 ` Andy Shevchenko
-1 siblings, 0 replies; 25+ messages in thread
From: Andy Shevchenko @ 2016-01-14 13:29 UTC (permalink / raw)
To: David Wu
Cc: Heiko Stübner, Wolfram Sang, Douglas Anderson, Tao Huang,
Chris Zhong, cf, Jianqun Xu, Lin Huang, linux-arm Mailing List,
linux-rockchip, linux-gpio, linux-kernel
On Thu, Jan 14, 2016 at 2:31 PM, David Wu <david.wu@rock-chips.com> wrote:
> There was an timing issue about "repeated start" time at the I2C
> controller of version0, controller appears to drop SDA at .875x (7/8)
> programmed clk high. The rule(.875x) isn't enough to meet tSU;STA
> requirements on 100k's Standard-mode. To resolve this issue,
> data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
> timing information are added, new rules are designed to calculate
> the timing information at new v1.
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -58,10 +58,15 @@ enum {
> #define REG_CON_LASTACK BIT(5) /* 1: send NACK after last received byte */
> #define REG_CON_ACTACK BIT(6) /* 1: stop if NACK is received */
>
> +#define REG_CON_SDA_CNT(cnt) ((cnt) << 8)
> +#define REG_CON_STA_CNT(cnt) ((cnt) << 12)
> +#define REG_CON_STO_CNT(cnt) ((cnt) << 14)
> +
> #define VERSION_MASK GENMASK(31, 16)
> #define VERSION_SHIFT 16
>
> #define RK3X_I2C_V0 0x0
> +#define RK3X_I2C_V1 0x1
>
> /* REG_MRXADDR bits */
> #define REG_MRXADDR_VALID(x) BIT(24 + (x)) /* [x*8+7:x*8] of MRX[R]ADDR valid */
> @@ -99,10 +104,16 @@ struct rk3x_i2c_soc_data {
> * struct rk3x_priv_i2c_timings - rk3x I2C timing information
> * @div_low: Divider output for low
> * @div_high: Divider output for high
> + * @thd_sda_count: SDA update point config used to adjust sda setup/hold time
> + * @tsu_sta_count: Start setup config for setup start time and hold start time
> + * @tsu_sto_count: Stop setup config for setup stop time
> */
> struct rk3x_priv_i2c_timings {
> unsigned long div_low;
> unsigned long div_high;
> + unsigned int thd_sda_count;
> + unsigned int tsu_sta_count;
> + unsigned int tsu_sto_count;
> };
And in this (or even separate) patch makes sense to introduce
extension structure, which is struct rk3x_priv_i2c_timings.
> struct rk3x_i2c_ops {
> @@ -154,6 +165,13 @@ static inline u32 i2c_readl(struct rk3x_i2c *i2c, unsigned int offset)
> return readl(i2c->regs + offset);
> }
>
> +static inline u32 rk3x_i2c_get_con_count(struct rk3x_i2c *i2c)
> +{
> + return REG_CON_SDA_CNT(i2c->t_priv.thd_sda_count) |
> + REG_CON_STA_CNT(i2c->t_priv.tsu_sta_count) |
> + REG_CON_STO_CNT(i2c->t_priv.tsu_sto_count);
> +}
> +
> /* Reset all interrupt pending bits */
> static inline void rk3x_i2c_clean_ipd(struct rk3x_i2c *i2c)
> {
> @@ -165,13 +183,13 @@ static inline void rk3x_i2c_clean_ipd(struct rk3x_i2c *i2c)
> */
> static void rk3x_i2c_start(struct rk3x_i2c *i2c)
> {
> - u32 val;
> + u32 val = rk3x_i2c_get_con_count(i2c);
>
> rk3x_i2c_clean_ipd(i2c);
> i2c_writel(i2c, REG_INT_START, REG_IEN);
>
> /* enable adapter with correct mode, send START condition */
> - val = REG_CON_EN | REG_CON_MOD(i2c->mode) | REG_CON_START;
> + val = val | REG_CON_EN | REG_CON_MOD(i2c->mode) | REG_CON_START;
>
> /* if we want to react to NACK, set ACTACK bit */
> if (!(i2c->msg->flags & I2C_M_IGNORE_NAK))
> @@ -212,7 +230,7 @@ static void rk3x_i2c_stop(struct rk3x_i2c *i2c, int error)
> * get the intended effect by resetting its internal state
> * and issuing an ordinary START.
> */
> - i2c_writel(i2c, 0, REG_CON);
> + i2c_writel(i2c, rk3x_i2c_get_con_count(i2c), REG_CON);
>
> /* signal that we are finished with the current msg */
> wake_up(&i2c->wait);
> @@ -630,6 +648,211 @@ static int rk3x_i2c_v0_calc_clock(unsigned long clk_rate,
> return ret;
> }
>
> +/**
> + * Calculate timing clock info values for desired SCL frequency
> + *
> + * @clk_rate: I2C input clock rate
> + * @t_input: Known I2C timing information
> + * @t_output: Caculated rk3x private timing information that would
> + * be written into regs
> + * Returns: 0 on success, -EINVAL if the goal SCL rate is too slow. In that case
> + * a best-effort divider value is returned in divs. If the target rate is
> + * too high, we silently use the highest possible rate.
> + * The following formulas are v1's method to calculate clock.
> + *
> + * l = divl + 1;
> + * h = divh + 1;
> + * s = data_upd_st + 1;
> + * u = start_setup_cnt + 1;
> + * p = stop_setup_cnt + 1;
> + * T = Tclk_i2c;
> +
> + * tHigh = 8 * h * T;
> + * tLow = 8 * l * T;
> +
> + * tHD;sda = (l * s + 1) * T;
> + * tSU;sda = [(8 - s) * l + 1] * T;
> + * tI2C = 8 * (l + h) * T;
> +
> + * tSU;sta = (8h * u + 1) * T;
> + * tHD;sta = [8h * (u + 1) - 1] * T;
> + * tSU;sto = (8h * p + 1) * T;
> + */
> +static int rk3x_i2c_v1_calc_clock(unsigned long clk_rate,
> + struct i2c_timings *t_input,
> + struct rk3x_priv_i2c_timings *t_output)
> +{
I see some similarities with existing code for v0. Can be refactored?
> + unsigned long spec_min_low_ns, spec_min_high_ns;
> + unsigned long spec_min_setup_start_ns, spec_min_stop_setup_ns;
> + unsigned long spec_min_data_setup_ns, spec_max_data_hold_ns;
> +
> + unsigned long min_low_ns, min_high_ns, min_total_ns;
> + unsigned long min_setup_start_ns, min_setup_data_ns;
> + unsigned long min_stop_setup_ns, max_hold_data_ns;
> +
> + unsigned long clk_rate_khz, scl_rate_khz;
> +
> + unsigned long min_low_div, min_high_div;
> +
> + unsigned long min_div_for_hold, min_total_div;
> + unsigned long extra_div, extra_low_div;
> + unsigned long data_hd_cnt;
> +
> + int ret = 0;
> +
> + /* Support standard-mode and fast-mode */
> + if (WARN_ON(t_input->bus_freq_hz > 400000))
> + t_input->bus_freq_hz = 400000;
> +
> + /* prevent scl_rate_khz from becoming 0 */
> + if (WARN_ON(t_input->bus_freq_hz < 1000))
> + t_input->bus_freq_hz = 1000;
> +
> + /*
> + * min_low_ns: The minimum number of ns we need to hold low to
> + * meet I2C specification, should include fall time.
> + * min_high_ns: The minimum number of ns we need to hold high to
> + * meet I2C specification, should include rise time.
> + */
> + if (t_input->bus_freq_hz <= 100000) {
> + spec_min_low_ns = 4700;
> + spec_min_high_ns = 4000;
> +
> + spec_min_setup_start_ns = 4700;
> + spec_min_stop_setup_ns = 4000;
> +
> + spec_min_data_setup_ns = 250;
> + spec_max_data_hold_ns = 3450;
> + } else if (t_input->bus_freq_hz <= 400000) {
> + spec_min_low_ns = 1300;
> + spec_min_high_ns = 600;
> +
> + spec_min_setup_start_ns = 600;
> + spec_min_stop_setup_ns = 600;
> +
> + spec_min_data_setup_ns = 100;
> + spec_max_data_hold_ns = 900;
> + }
> +
> + /* caculate min-divh and min-divl */
> + clk_rate_khz = DIV_ROUND_UP(clk_rate, 1000);
> + scl_rate_khz = t_input->bus_freq_hz / 1000;
> + min_total_div = DIV_ROUND_UP(clk_rate_khz, scl_rate_khz * 8);
> +
> + min_high_ns = t_input->scl_rise_ns + spec_min_high_ns;
> + min_high_div = DIV_ROUND_UP(clk_rate_khz * min_high_ns, 8 * 1000000);
> +
> + min_low_ns = t_input->scl_fall_ns + spec_min_low_ns;
> + min_low_div = DIV_ROUND_UP(clk_rate_khz * min_low_ns, 8 * 1000000);
> +
> + /* Final divh and divl must be greater than 0, otherwise the
> + * hardware would not output the i2c clk.
> + */
> + if (min_high_div <= 1)
> + min_high_div = 2;
> + if (min_low_div <= 1)
> + min_low_div = 2;
> +
> + /* These are the min dividers needed for min hold times. */
> + min_div_for_hold = (min_low_div + min_high_div);
> + min_total_ns = min_low_ns + min_high_ns;
> +
> + /*
> + * This is the maximum divider so we don't go over the maximum.
> + * We don't round up here (we round down) since this is a maximum.
> + */
> + if (min_div_for_hold >= min_total_div) {
> + /*
> + * Time needed to meet hold requirements is important.
> + * Just use that.
> + */
> + t_output->div_low = min_low_div;
> + t_output->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.
> + * We'll try to split things up by the scale of min_low_div and
> + * min_high_div, biasing slightly towards having a higher div
> + * for low (spend more time low).
> + */
> + extra_div = min_total_div - min_div_for_hold;
> + extra_low_div = DIV_ROUND_UP(min_low_div * extra_div,
> + min_div_for_hold);
> +
> + t_output->div_low = min_low_div + extra_low_div;
> + t_output->div_high = min_high_div + (extra_div - extra_low_div);
> + }
> +
> + /*
> + * calculate sda data hold count by the rules, thd_sda_count:3
> + * is a appropriate value to reduce calculated times.
> + * tHD;sda = (l * s + 1) * T
> + * tSU;sda = ((8 - s) * l + 1) * T
> + */
> + for (data_hd_cnt = 3; data_hd_cnt >= 0; data_hd_cnt--) {
> + max_hold_data_ns = DIV_ROUND_UP((data_hd_cnt
> + * (t_output->div_low) + 1)
> + * 1000000, clk_rate_khz);
> + min_setup_data_ns = DIV_ROUND_UP(((8 - data_hd_cnt)
> + * (t_output->div_low) + 1)
> + * 1000000, clk_rate_khz);
> + if ((max_hold_data_ns < spec_max_data_hold_ns) &&
> + (min_setup_data_ns > spec_min_data_setup_ns)) {
> + t_output->thd_sda_count = data_hd_cnt;
> + break;
> + }
> + }
> +
> + /*
> + * calculate start setup count, and we aren't care tHD;STA.
> + * If the start setup count meets the rule of tSU;sta, it also
> + * meets the rule of tHD;STA.
> + * tSU;sta = (8h * u + 1) * T
> + * tHD;sta = [8h * (u + 1) - 1] * T
> + */
> + min_setup_start_ns = t_input->scl_rise_ns + spec_min_setup_start_ns;
> + t_output->tsu_sta_count = DIV_ROUND_UP(clk_rate_khz * min_setup_start_ns
> + - 1000000, 8 * 1000000 * (t_output->div_high));
> +
> + /*
> + * calculate start setup count by the rule:
> + * tSU;sto =(8h * p + 1) * T
> + */
> + min_stop_setup_ns = t_input->scl_rise_ns + spec_min_stop_setup_ns;
> + t_output->tsu_sto_count = DIV_ROUND_UP(clk_rate_khz * min_stop_setup_ns
> + - 1000000, 8 * 1000000 * (t_output->div_high));
> +
> + /*
> + * Adjust to the fact that the hardware has an implicit "+1".
> + * NOTE: Above calculations always produce div_low > 0 and div_high > 0.
> + */
> + t_output->div_low -= 1;
> + t_output->div_high -= 1;
> +
> + /* Maximum divider supported by hw is 0xffff */
> + if (t_output->div_low > 0xffff) {
> + t_output->div_low = 0xffff;
> + ret = -EINVAL;
> + }
> +
> + if (t_output->div_high > 0xffff) {
> + t_output->div_high = 0xffff;
> + ret = -EINVAL;
> + }
> +
> + /*
> + * Adjust to the fact that the hardware has an implicit "+1".
> + * NOTE: Above calculations always produce thd_sda_count > 0,
> + * tsu_sta_count > 0 and tsu_sta_count > 0.
> + */
> + t_output->thd_sda_count -= 1;
> + t_output->tsu_sta_count -= 1;
> + t_output->tsu_sto_count -= 1;
> +
> + return ret;
> +}
> +
> static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
> {
> u64 t_low_ns, t_high_ns;
> @@ -829,7 +1052,8 @@ static int rk3x_i2c_xfer(struct i2c_adapter *adap,
>
> /* Force a STOP condition without interrupt */
> i2c_writel(i2c, 0, REG_IEN);
> - i2c_writel(i2c, REG_CON_EN | REG_CON_STOP, REG_CON);
> + i2c_writel(i2c, rk3x_i2c_get_con_count(i2c) |
> + REG_CON_EN | REG_CON_STOP, REG_CON);
>
> i2c->state = STATE_IDLE;
>
> @@ -969,7 +1193,9 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, i2c);
>
> version = (readl(i2c->regs + REG_CON) & VERSION_MASK) >> VERSION_SHIFT;
> - if (version == RK3X_I2C_V0)
> + if (version == RK3X_I2C_V1)
> + i2c->ops.calc_clock = rk3x_i2c_v1_calc_clock;
> + else
> i2c->ops.calc_clock = rk3x_i2c_v0_calc_clock;
Perhaps time to use switch-case:
switch ((value & MASK) >> SHIFT) {
case V1:
v1();
break;
case V0:
default:
v0();
break;
}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 3/4] i2c: rk3x: new method to caculate i2c clocks
@ 2016-01-14 13:29 ` Andy Shevchenko
0 siblings, 0 replies; 25+ messages in thread
From: Andy Shevchenko @ 2016-01-14 13:29 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jan 14, 2016 at 2:31 PM, David Wu <david.wu@rock-chips.com> wrote:
> There was an timing issue about "repeated start" time at the I2C
> controller of version0, controller appears to drop SDA at .875x (7/8)
> programmed clk high. The rule(.875x) isn't enough to meet tSU;STA
> requirements on 100k's Standard-mode. To resolve this issue,
> data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
> timing information are added, new rules are designed to calculate
> the timing information at new v1.
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -58,10 +58,15 @@ enum {
> #define REG_CON_LASTACK BIT(5) /* 1: send NACK after last received byte */
> #define REG_CON_ACTACK BIT(6) /* 1: stop if NACK is received */
>
> +#define REG_CON_SDA_CNT(cnt) ((cnt) << 8)
> +#define REG_CON_STA_CNT(cnt) ((cnt) << 12)
> +#define REG_CON_STO_CNT(cnt) ((cnt) << 14)
> +
> #define VERSION_MASK GENMASK(31, 16)
> #define VERSION_SHIFT 16
>
> #define RK3X_I2C_V0 0x0
> +#define RK3X_I2C_V1 0x1
>
> /* REG_MRXADDR bits */
> #define REG_MRXADDR_VALID(x) BIT(24 + (x)) /* [x*8+7:x*8] of MRX[R]ADDR valid */
> @@ -99,10 +104,16 @@ struct rk3x_i2c_soc_data {
> * struct rk3x_priv_i2c_timings - rk3x I2C timing information
> * @div_low: Divider output for low
> * @div_high: Divider output for high
> + * @thd_sda_count: SDA update point config used to adjust sda setup/hold time
> + * @tsu_sta_count: Start setup config for setup start time and hold start time
> + * @tsu_sto_count: Stop setup config for setup stop time
> */
> struct rk3x_priv_i2c_timings {
> unsigned long div_low;
> unsigned long div_high;
> + unsigned int thd_sda_count;
> + unsigned int tsu_sta_count;
> + unsigned int tsu_sto_count;
> };
And in this (or even separate) patch makes sense to introduce
extension structure, which is struct rk3x_priv_i2c_timings.
> struct rk3x_i2c_ops {
> @@ -154,6 +165,13 @@ static inline u32 i2c_readl(struct rk3x_i2c *i2c, unsigned int offset)
> return readl(i2c->regs + offset);
> }
>
> +static inline u32 rk3x_i2c_get_con_count(struct rk3x_i2c *i2c)
> +{
> + return REG_CON_SDA_CNT(i2c->t_priv.thd_sda_count) |
> + REG_CON_STA_CNT(i2c->t_priv.tsu_sta_count) |
> + REG_CON_STO_CNT(i2c->t_priv.tsu_sto_count);
> +}
> +
> /* Reset all interrupt pending bits */
> static inline void rk3x_i2c_clean_ipd(struct rk3x_i2c *i2c)
> {
> @@ -165,13 +183,13 @@ static inline void rk3x_i2c_clean_ipd(struct rk3x_i2c *i2c)
> */
> static void rk3x_i2c_start(struct rk3x_i2c *i2c)
> {
> - u32 val;
> + u32 val = rk3x_i2c_get_con_count(i2c);
>
> rk3x_i2c_clean_ipd(i2c);
> i2c_writel(i2c, REG_INT_START, REG_IEN);
>
> /* enable adapter with correct mode, send START condition */
> - val = REG_CON_EN | REG_CON_MOD(i2c->mode) | REG_CON_START;
> + val = val | REG_CON_EN | REG_CON_MOD(i2c->mode) | REG_CON_START;
>
> /* if we want to react to NACK, set ACTACK bit */
> if (!(i2c->msg->flags & I2C_M_IGNORE_NAK))
> @@ -212,7 +230,7 @@ static void rk3x_i2c_stop(struct rk3x_i2c *i2c, int error)
> * get the intended effect by resetting its internal state
> * and issuing an ordinary START.
> */
> - i2c_writel(i2c, 0, REG_CON);
> + i2c_writel(i2c, rk3x_i2c_get_con_count(i2c), REG_CON);
>
> /* signal that we are finished with the current msg */
> wake_up(&i2c->wait);
> @@ -630,6 +648,211 @@ static int rk3x_i2c_v0_calc_clock(unsigned long clk_rate,
> return ret;
> }
>
> +/**
> + * Calculate timing clock info values for desired SCL frequency
> + *
> + * @clk_rate: I2C input clock rate
> + * @t_input: Known I2C timing information
> + * @t_output: Caculated rk3x private timing information that would
> + * be written into regs
> + * Returns: 0 on success, -EINVAL if the goal SCL rate is too slow. In that case
> + * a best-effort divider value is returned in divs. If the target rate is
> + * too high, we silently use the highest possible rate.
> + * The following formulas are v1's method to calculate clock.
> + *
> + * l = divl + 1;
> + * h = divh + 1;
> + * s = data_upd_st + 1;
> + * u = start_setup_cnt + 1;
> + * p = stop_setup_cnt + 1;
> + * T = Tclk_i2c;
> +
> + * tHigh = 8 * h * T;
> + * tLow = 8 * l * T;
> +
> + * tHD;sda = (l * s + 1) * T;
> + * tSU;sda = [(8 - s) * l + 1] * T;
> + * tI2C = 8 * (l + h) * T;
> +
> + * tSU;sta = (8h * u + 1) * T;
> + * tHD;sta = [8h * (u + 1) - 1] * T;
> + * tSU;sto = (8h * p + 1) * T;
> + */
> +static int rk3x_i2c_v1_calc_clock(unsigned long clk_rate,
> + struct i2c_timings *t_input,
> + struct rk3x_priv_i2c_timings *t_output)
> +{
I see some similarities with existing code for v0. Can be refactored?
> + unsigned long spec_min_low_ns, spec_min_high_ns;
> + unsigned long spec_min_setup_start_ns, spec_min_stop_setup_ns;
> + unsigned long spec_min_data_setup_ns, spec_max_data_hold_ns;
> +
> + unsigned long min_low_ns, min_high_ns, min_total_ns;
> + unsigned long min_setup_start_ns, min_setup_data_ns;
> + unsigned long min_stop_setup_ns, max_hold_data_ns;
> +
> + unsigned long clk_rate_khz, scl_rate_khz;
> +
> + unsigned long min_low_div, min_high_div;
> +
> + unsigned long min_div_for_hold, min_total_div;
> + unsigned long extra_div, extra_low_div;
> + unsigned long data_hd_cnt;
> +
> + int ret = 0;
> +
> + /* Support standard-mode and fast-mode */
> + if (WARN_ON(t_input->bus_freq_hz > 400000))
> + t_input->bus_freq_hz = 400000;
> +
> + /* prevent scl_rate_khz from becoming 0 */
> + if (WARN_ON(t_input->bus_freq_hz < 1000))
> + t_input->bus_freq_hz = 1000;
> +
> + /*
> + * min_low_ns: The minimum number of ns we need to hold low to
> + * meet I2C specification, should include fall time.
> + * min_high_ns: The minimum number of ns we need to hold high to
> + * meet I2C specification, should include rise time.
> + */
> + if (t_input->bus_freq_hz <= 100000) {
> + spec_min_low_ns = 4700;
> + spec_min_high_ns = 4000;
> +
> + spec_min_setup_start_ns = 4700;
> + spec_min_stop_setup_ns = 4000;
> +
> + spec_min_data_setup_ns = 250;
> + spec_max_data_hold_ns = 3450;
> + } else if (t_input->bus_freq_hz <= 400000) {
> + spec_min_low_ns = 1300;
> + spec_min_high_ns = 600;
> +
> + spec_min_setup_start_ns = 600;
> + spec_min_stop_setup_ns = 600;
> +
> + spec_min_data_setup_ns = 100;
> + spec_max_data_hold_ns = 900;
> + }
> +
> + /* caculate min-divh and min-divl */
> + clk_rate_khz = DIV_ROUND_UP(clk_rate, 1000);
> + scl_rate_khz = t_input->bus_freq_hz / 1000;
> + min_total_div = DIV_ROUND_UP(clk_rate_khz, scl_rate_khz * 8);
> +
> + min_high_ns = t_input->scl_rise_ns + spec_min_high_ns;
> + min_high_div = DIV_ROUND_UP(clk_rate_khz * min_high_ns, 8 * 1000000);
> +
> + min_low_ns = t_input->scl_fall_ns + spec_min_low_ns;
> + min_low_div = DIV_ROUND_UP(clk_rate_khz * min_low_ns, 8 * 1000000);
> +
> + /* Final divh and divl must be greater than 0, otherwise the
> + * hardware would not output the i2c clk.
> + */
> + if (min_high_div <= 1)
> + min_high_div = 2;
> + if (min_low_div <= 1)
> + min_low_div = 2;
> +
> + /* These are the min dividers needed for min hold times. */
> + min_div_for_hold = (min_low_div + min_high_div);
> + min_total_ns = min_low_ns + min_high_ns;
> +
> + /*
> + * This is the maximum divider so we don't go over the maximum.
> + * We don't round up here (we round down) since this is a maximum.
> + */
> + if (min_div_for_hold >= min_total_div) {
> + /*
> + * Time needed to meet hold requirements is important.
> + * Just use that.
> + */
> + t_output->div_low = min_low_div;
> + t_output->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.
> + * We'll try to split things up by the scale of min_low_div and
> + * min_high_div, biasing slightly towards having a higher div
> + * for low (spend more time low).
> + */
> + extra_div = min_total_div - min_div_for_hold;
> + extra_low_div = DIV_ROUND_UP(min_low_div * extra_div,
> + min_div_for_hold);
> +
> + t_output->div_low = min_low_div + extra_low_div;
> + t_output->div_high = min_high_div + (extra_div - extra_low_div);
> + }
> +
> + /*
> + * calculate sda data hold count by the rules, thd_sda_count:3
> + * is a appropriate value to reduce calculated times.
> + * tHD;sda = (l * s + 1) * T
> + * tSU;sda = ((8 - s) * l + 1) * T
> + */
> + for (data_hd_cnt = 3; data_hd_cnt >= 0; data_hd_cnt--) {
> + max_hold_data_ns = DIV_ROUND_UP((data_hd_cnt
> + * (t_output->div_low) + 1)
> + * 1000000, clk_rate_khz);
> + min_setup_data_ns = DIV_ROUND_UP(((8 - data_hd_cnt)
> + * (t_output->div_low) + 1)
> + * 1000000, clk_rate_khz);
> + if ((max_hold_data_ns < spec_max_data_hold_ns) &&
> + (min_setup_data_ns > spec_min_data_setup_ns)) {
> + t_output->thd_sda_count = data_hd_cnt;
> + break;
> + }
> + }
> +
> + /*
> + * calculate start setup count, and we aren't care tHD;STA.
> + * If the start setup count meets the rule of tSU;sta, it also
> + * meets the rule of tHD;STA.
> + * tSU;sta = (8h * u + 1) * T
> + * tHD;sta = [8h * (u + 1) - 1] * T
> + */
> + min_setup_start_ns = t_input->scl_rise_ns + spec_min_setup_start_ns;
> + t_output->tsu_sta_count = DIV_ROUND_UP(clk_rate_khz * min_setup_start_ns
> + - 1000000, 8 * 1000000 * (t_output->div_high));
> +
> + /*
> + * calculate start setup count by the rule:
> + * tSU;sto =(8h * p + 1) * T
> + */
> + min_stop_setup_ns = t_input->scl_rise_ns + spec_min_stop_setup_ns;
> + t_output->tsu_sto_count = DIV_ROUND_UP(clk_rate_khz * min_stop_setup_ns
> + - 1000000, 8 * 1000000 * (t_output->div_high));
> +
> + /*
> + * Adjust to the fact that the hardware has an implicit "+1".
> + * NOTE: Above calculations always produce div_low > 0 and div_high > 0.
> + */
> + t_output->div_low -= 1;
> + t_output->div_high -= 1;
> +
> + /* Maximum divider supported by hw is 0xffff */
> + if (t_output->div_low > 0xffff) {
> + t_output->div_low = 0xffff;
> + ret = -EINVAL;
> + }
> +
> + if (t_output->div_high > 0xffff) {
> + t_output->div_high = 0xffff;
> + ret = -EINVAL;
> + }
> +
> + /*
> + * Adjust to the fact that the hardware has an implicit "+1".
> + * NOTE: Above calculations always produce thd_sda_count > 0,
> + * tsu_sta_count > 0 and tsu_sta_count > 0.
> + */
> + t_output->thd_sda_count -= 1;
> + t_output->tsu_sta_count -= 1;
> + t_output->tsu_sto_count -= 1;
> +
> + return ret;
> +}
> +
> static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
> {
> u64 t_low_ns, t_high_ns;
> @@ -829,7 +1052,8 @@ static int rk3x_i2c_xfer(struct i2c_adapter *adap,
>
> /* Force a STOP condition without interrupt */
> i2c_writel(i2c, 0, REG_IEN);
> - i2c_writel(i2c, REG_CON_EN | REG_CON_STOP, REG_CON);
> + i2c_writel(i2c, rk3x_i2c_get_con_count(i2c) |
> + REG_CON_EN | REG_CON_STOP, REG_CON);
>
> i2c->state = STATE_IDLE;
>
> @@ -969,7 +1193,9 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, i2c);
>
> version = (readl(i2c->regs + REG_CON) & VERSION_MASK) >> VERSION_SHIFT;
> - if (version == RK3X_I2C_V0)
> + if (version == RK3X_I2C_V1)
> + i2c->ops.calc_clock = rk3x_i2c_v1_calc_clock;
> + else
> i2c->ops.calc_clock = rk3x_i2c_v0_calc_clock;
Perhaps time to use switch-case:
switch ((value & MASK) >> SHIFT) {
case V1:
v1();
break;
case V0:
default:
v0();
break;
}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/4] i2c: rk3x: new method to caculate i2c clocks
2016-01-14 12:31 ` David Wu
(?)
@ 2016-01-14 16:12 ` Doug Anderson
-1 siblings, 0 replies; 25+ messages in thread
From: Doug Anderson @ 2016-01-14 16:12 UTC (permalink / raw)
To: David Wu
Cc: Heiko Stübner, Wolfram Sang, andy.shevchenko, Tao Huang,
Chris, Eddie Cai, Jianqun Xu, Lin Huang, linux-arm-kernel,
open list:ARM/Rockchip SoC...,
linux-gpio, linux-kernel
Hi,
On Thu, Jan 14, 2016 at 4:31 AM, David Wu <david.wu@rock-chips.com> wrote:
> There was an timing issue about "repeated start" time at the I2C
> controller of version0, controller appears to drop SDA at .875x (7/8)
> programmed clk high. The rule(.875x) isn't enough to meet tSU;STA
> requirements on 100k's Standard-mode. To resolve this issue,
> data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
> timing information are added, new rules are designed to calculate
> the timing information at new v1.
I'm curious: does new hardware behave differently and that's why you
need v1? ...or does old and new hardware behave the same and you're
just introducing v1 so you don't mess with how old boards are working?
>From the description it sounds as if the old code had problems as 100k too...
If the new controller is different, I'd probably reword like the
following (you'd have to re-wordwrap):
There was an timing issue about "repeated start" time at the I2C
controller of version0, controller appears to drop SDA at .875x (7/8)
programmed clk high. On version 1 of the controller, the rule(.875x)
isn't enough to meet tSU;STA
requirements on 100k's Standard-mode. To resolve this issue,
data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
timing information are added, new rules are designed to calculate
the timing information at new v1.
-Doug
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/4] i2c: rk3x: new method to caculate i2c clocks
@ 2016-01-14 16:12 ` Doug Anderson
0 siblings, 0 replies; 25+ messages in thread
From: Doug Anderson @ 2016-01-14 16:12 UTC (permalink / raw)
To: David Wu
Cc: Heiko Stübner, Wolfram Sang, andy.shevchenko, Tao Huang,
Chris, Eddie Cai, Jianqun Xu, Lin Huang, linux-arm-kernel,
open list:ARM/Rockchip SoC...,
linux-gpio, linux-kernel
Hi,
On Thu, Jan 14, 2016 at 4:31 AM, David Wu <david.wu@rock-chips.com> wrote:
> There was an timing issue about "repeated start" time at the I2C
> controller of version0, controller appears to drop SDA at .875x (7/8)
> programmed clk high. The rule(.875x) isn't enough to meet tSU;STA
> requirements on 100k's Standard-mode. To resolve this issue,
> data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
> timing information are added, new rules are designed to calculate
> the timing information at new v1.
I'm curious: does new hardware behave differently and that's why you
need v1? ...or does old and new hardware behave the same and you're
just introducing v1 so you don't mess with how old boards are working?
From the description it sounds as if the old code had problems as 100k too...
If the new controller is different, I'd probably reword like the
following (you'd have to re-wordwrap):
There was an timing issue about "repeated start" time at the I2C
controller of version0, controller appears to drop SDA at .875x (7/8)
programmed clk high. On version 1 of the controller, the rule(.875x)
isn't enough to meet tSU;STA
requirements on 100k's Standard-mode. To resolve this issue,
data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
timing information are added, new rules are designed to calculate
the timing information at new v1.
-Doug
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 3/4] i2c: rk3x: new method to caculate i2c clocks
@ 2016-01-14 16:12 ` Doug Anderson
0 siblings, 0 replies; 25+ messages in thread
From: Doug Anderson @ 2016-01-14 16:12 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Thu, Jan 14, 2016 at 4:31 AM, David Wu <david.wu@rock-chips.com> wrote:
> There was an timing issue about "repeated start" time at the I2C
> controller of version0, controller appears to drop SDA at .875x (7/8)
> programmed clk high. The rule(.875x) isn't enough to meet tSU;STA
> requirements on 100k's Standard-mode. To resolve this issue,
> data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
> timing information are added, new rules are designed to calculate
> the timing information at new v1.
I'm curious: does new hardware behave differently and that's why you
need v1? ...or does old and new hardware behave the same and you're
just introducing v1 so you don't mess with how old boards are working?
>From the description it sounds as if the old code had problems as 100k too...
If the new controller is different, I'd probably reword like the
following (you'd have to re-wordwrap):
There was an timing issue about "repeated start" time at the I2C
controller of version0, controller appears to drop SDA at .875x (7/8)
programmed clk high. On version 1 of the controller, the rule(.875x)
isn't enough to meet tSU;STA
requirements on 100k's Standard-mode. To resolve this issue,
data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
timing information are added, new rules are designed to calculate
the timing information at new v1.
-Doug
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 2/4] i2c: rk3x: add ops to caculate i2c clocks
2016-01-14 13:19 ` Andy Shevchenko
@ 2016-01-15 8:34 ` David.Wu
-1 siblings, 0 replies; 25+ messages in thread
From: David.Wu @ 2016-01-15 8:34 UTC (permalink / raw)
To: Andy Shevchenko, David Wu
Cc: Heiko Stübner, Wolfram Sang, Douglas Anderson, Tao Huang,
Chris Zhong, cf, Jianqun Xu, Lin Huang, linux-arm Mailing List,
linux-rockchip, linux-i2c, linux-kernel
Hi Andy,
在 2016/1/14 21:19, Andy Shevchenko 写道:
> On Thu, Jan 14, 2016 at 2:31 PM, David Wu <david.wu@rock-chips.com> wrote:
>> From: David Wu <wdc@rock-chips.com>
>>
>> I2c Controller of rk3x is updated for the rules to caculate clocks.
>> So it need a new method to caculate i2c clock timing information
>> for new version. The current method is defined as v0, and new is
>> v1, next maybe v2......
> Thanks for an update. My comments below.
>
>> --- a/drivers/i2c/busses/i2c-rk3x.c
>> +++ b/drivers/i2c/busses/i2c-rk3x.c
>> @@ -90,10 +95,27 @@ struct rk3x_i2c_soc_data {
>> int grf_offset;
>> };
>>
>> +/**
>> + * struct rk3x_priv_i2c_timings - rk3x I2C timing information
>> + * @div_low: Divider output for low
>> + * @div_high: Divider output for high
>> + */
>> +struct rk3x_priv_i2c_timings {
>> + unsigned long div_low;
>> + unsigned long div_high;
>> +};
>> +
>> +struct rk3x_i2c_ops {
>> + int (*calc_clock)(unsigned long,
>> + struct i2c_timings *,
>> + struct rk3x_priv_i2c_timings *);
>> +};
>> +
>> struct rk3x_i2c {
>> struct i2c_adapter adap;
>> struct device *dev;
>> struct rk3x_i2c_soc_data *soc_data;
>> + struct rk3x_i2c_ops ops;
> Not much sense for now to have a struct for one member.
Yes, it looks ok not to do change here.
>> /* Hardware resources */
>> void __iomem *regs;
>> @@ -102,6 +124,7 @@ struct rk3x_i2c {
>>
> What about
> /* I2C timing settings */
> struct i2c_timings t;
>
> /* Divider settings */
> int (*calc_divs)(unsigned long,
> struct i2c_timings *,
> unsigned long *div_low,
> unsigned long *div_high);
> unsigned long div_low;
> unsigned long div_high;
> ?
>
> As far as I understand you still calculate divider values.
>
>> /* Synchronization & notification */
>> spinlock_t lock;
>> @@ -431,21 +454,20 @@ out:
>> }
>>
>> /**
>> - * Calculate divider values for desired SCL frequency
>> + * Calculate timing clock info values for desired SCL frequency
>> *
>> * @clk_rate: I2C input clock rate
>> - * @t_input: Known I2C timing information.
>> - * @div_low: Divider output for low
>> - * @div_high: Divider output for high
> For now seems better to leave this prototype.
>
>> + * @t_input: Known I2C timing information
>> + * @t_output: Caculated rk3x private timing information that would
>> + * be written into regs
>> *
>> * Returns: 0 on success, -EINVAL if the goal SCL rate is too slow. In that case
>> * a best-effort divider value is returned in divs. If the target rate is
>> * too high, we silently use the highest possible rate.
>> */
>> -static int rk3x_i2c_calc_divs(unsigned long clk_rate,
>> - struct i2c_timings *t_input,
>> - unsigned long *div_low,
>> - unsigned long *div_high)
>> +static int rk3x_i2c_v0_calc_clock(unsigned long clk_rate,
>> + struct i2c_timings *t_input,
>> + struct rk3x_priv_i2c_timings *t_output)
>> {
>> unsigned long spec_min_low_ns, spec_min_high_ns;
>> unsigned long spec_setup_start, spec_max_data_hold_ns;
>
>> @@ -583,25 +605,25 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
>>
>> /* 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);
>> + t_output->div_low = ideal_low_div;
>> + t_output->div_high = min_high_div + (extra_div - extra_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 = *div_low - 1;
>> - *div_high = *div_high - 1;
>> + t_output->div_low = t_output->div_low - 1;
> -= 1
>
>> + t_output->div_high = t_output->div_high - 1;
> Ditto.
>
> But without change of prototype those no needed.
>
>> /* Maximum divider supported by hw is 0xffff */
>> - if (*div_low > 0xffff) {
>> - *div_low = 0xffff;
>> + if (t_output->div_low > 0xffff) {
>> + t_output->div_low = 0xffff;
>> ret = -EINVAL;
>> }
>>
>> - if (*div_high > 0xffff) {
>> - *div_high = 0xffff;
>> + if (t_output->div_high > 0xffff) {
>> + t_output->div_high = 0xffff;
>> ret = -EINVAL;
>> }
>>
>> @@ -610,19 +632,21 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
>>
>> static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
>> {
>> - unsigned long div_low, div_high;
>> u64 t_low_ns, t_high_ns;
>> int ret;
>>
>> - ret = rk3x_i2c_calc_divs(clk_rate, &i2c->t, &div_low, &div_high);
>> + ret = i2c->ops.calc_clock(clk_rate, &i2c->t, &i2c->t_priv);
>> WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->t.bus_freq_hz);
>>
>> clk_enable(i2c->clk);
>> - i2c_writel(i2c, (div_high << 16) | (div_low & 0xffff), REG_CLKDIV);
>> + i2c_writel(i2c, (i2c->t_priv.div_high << 16) |
>> + (i2c->t_priv.div_low & 0xffff), REG_CLKDIV);
>> clk_disable(i2c->clk);
>>
>> - t_low_ns = div_u64(((u64)div_low + 1) * 8 * 1000000000, clk_rate);
>> - t_high_ns = div_u64(((u64)div_high + 1) * 8 * 1000000000, clk_rate);
>> + t_low_ns = div_u64(((u64)i2c->t_priv.div_low + 1) * 8 * 1000000000,
>> + clk_rate);
>> + t_high_ns = div_u64(((u64)i2c->t_priv.div_high + 1) * 8 * 1000000000,
>> + clk_rate);
>> dev_dbg(i2c->dev,
>> "CLK %lukhz, Req %uns, Act low %lluns high %lluns\n",
>> clk_rate / 1000,
>> @@ -652,12 +676,11 @@ static int rk3x_i2c_clk_notifier_cb(struct notifier_block *nb, unsigned long
>> {
>> struct clk_notifier_data *ndata = data;
>> struct rk3x_i2c *i2c = container_of(nb, struct rk3x_i2c, clk_rate_nb);
>> - unsigned long div_low, div_high;
>>
>> switch (event) {
>> case PRE_RATE_CHANGE:
>> - if (rk3x_i2c_calc_divs(ndata->new_rate, &i2c->t,
>> - &div_low, &div_high) != 0)
>> + if (i2c->ops.calc_clock(ndata->new_rate, &i2c->t,
>> + &i2c->t_priv) != 0)
>> return NOTIFY_STOP;
>>
>> /* scale up */
>> @@ -861,6 +884,7 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
>> u32 value;
>> int irq;
>> unsigned long clk_rate;
>> + unsigned int version;
> No need to have a separate variable…
>
>> + version = (readl(i2c->regs + REG_CON) & VERSION_MASK) >> VERSION_SHIFT;
>> + if (version == RK3X_I2C_V0)
>> + i2c->ops.calc_clock = rk3x_i2c_v0_calc_clock;
> value = readl(i2c->regs + REG_CON);
> if ((value & VERSION_MASK) >> VERSION_SHIFT) == RK3X_I2C_V0)
> i2c->ops.calc_clock = rk3x_i2c_v0_calc_clock;
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 2/4] i2c: rk3x: add ops to caculate i2c clocks
@ 2016-01-15 8:34 ` David.Wu
0 siblings, 0 replies; 25+ messages in thread
From: David.Wu @ 2016-01-15 8:34 UTC (permalink / raw)
To: linux-arm-kernel
Hi Andy,
? 2016/1/14 21:19, Andy Shevchenko ??:
> On Thu, Jan 14, 2016 at 2:31 PM, David Wu <david.wu@rock-chips.com> wrote:
>> From: David Wu <wdc@rock-chips.com>
>>
>> I2c Controller of rk3x is updated for the rules to caculate clocks.
>> So it need a new method to caculate i2c clock timing information
>> for new version. The current method is defined as v0, and new is
>> v1, next maybe v2......
> Thanks for an update. My comments below.
>
>> --- a/drivers/i2c/busses/i2c-rk3x.c
>> +++ b/drivers/i2c/busses/i2c-rk3x.c
>> @@ -90,10 +95,27 @@ struct rk3x_i2c_soc_data {
>> int grf_offset;
>> };
>>
>> +/**
>> + * struct rk3x_priv_i2c_timings - rk3x I2C timing information
>> + * @div_low: Divider output for low
>> + * @div_high: Divider output for high
>> + */
>> +struct rk3x_priv_i2c_timings {
>> + unsigned long div_low;
>> + unsigned long div_high;
>> +};
>> +
>> +struct rk3x_i2c_ops {
>> + int (*calc_clock)(unsigned long,
>> + struct i2c_timings *,
>> + struct rk3x_priv_i2c_timings *);
>> +};
>> +
>> struct rk3x_i2c {
>> struct i2c_adapter adap;
>> struct device *dev;
>> struct rk3x_i2c_soc_data *soc_data;
>> + struct rk3x_i2c_ops ops;
> Not much sense for now to have a struct for one member.
Yes, it looks ok not to do change here.
>> /* Hardware resources */
>> void __iomem *regs;
>> @@ -102,6 +124,7 @@ struct rk3x_i2c {
>>
> What about
> /* I2C timing settings */
> struct i2c_timings t;
>
> /* Divider settings */
> int (*calc_divs)(unsigned long,
> struct i2c_timings *,
> unsigned long *div_low,
> unsigned long *div_high);
> unsigned long div_low;
> unsigned long div_high;
> ?
>
> As far as I understand you still calculate divider values.
>
>> /* Synchronization & notification */
>> spinlock_t lock;
>> @@ -431,21 +454,20 @@ out:
>> }
>>
>> /**
>> - * Calculate divider values for desired SCL frequency
>> + * Calculate timing clock info values for desired SCL frequency
>> *
>> * @clk_rate: I2C input clock rate
>> - * @t_input: Known I2C timing information.
>> - * @div_low: Divider output for low
>> - * @div_high: Divider output for high
> For now seems better to leave this prototype.
>
>> + * @t_input: Known I2C timing information
>> + * @t_output: Caculated rk3x private timing information that would
>> + * be written into regs
>> *
>> * Returns: 0 on success, -EINVAL if the goal SCL rate is too slow. In that case
>> * a best-effort divider value is returned in divs. If the target rate is
>> * too high, we silently use the highest possible rate.
>> */
>> -static int rk3x_i2c_calc_divs(unsigned long clk_rate,
>> - struct i2c_timings *t_input,
>> - unsigned long *div_low,
>> - unsigned long *div_high)
>> +static int rk3x_i2c_v0_calc_clock(unsigned long clk_rate,
>> + struct i2c_timings *t_input,
>> + struct rk3x_priv_i2c_timings *t_output)
>> {
>> unsigned long spec_min_low_ns, spec_min_high_ns;
>> unsigned long spec_setup_start, spec_max_data_hold_ns;
>
>> @@ -583,25 +605,25 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
>>
>> /* 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);
>> + t_output->div_low = ideal_low_div;
>> + t_output->div_high = min_high_div + (extra_div - extra_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 = *div_low - 1;
>> - *div_high = *div_high - 1;
>> + t_output->div_low = t_output->div_low - 1;
> -= 1
>
>> + t_output->div_high = t_output->div_high - 1;
> Ditto.
>
> But without change of prototype those no needed.
>
>> /* Maximum divider supported by hw is 0xffff */
>> - if (*div_low > 0xffff) {
>> - *div_low = 0xffff;
>> + if (t_output->div_low > 0xffff) {
>> + t_output->div_low = 0xffff;
>> ret = -EINVAL;
>> }
>>
>> - if (*div_high > 0xffff) {
>> - *div_high = 0xffff;
>> + if (t_output->div_high > 0xffff) {
>> + t_output->div_high = 0xffff;
>> ret = -EINVAL;
>> }
>>
>> @@ -610,19 +632,21 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
>>
>> static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate)
>> {
>> - unsigned long div_low, div_high;
>> u64 t_low_ns, t_high_ns;
>> int ret;
>>
>> - ret = rk3x_i2c_calc_divs(clk_rate, &i2c->t, &div_low, &div_high);
>> + ret = i2c->ops.calc_clock(clk_rate, &i2c->t, &i2c->t_priv);
>> WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->t.bus_freq_hz);
>>
>> clk_enable(i2c->clk);
>> - i2c_writel(i2c, (div_high << 16) | (div_low & 0xffff), REG_CLKDIV);
>> + i2c_writel(i2c, (i2c->t_priv.div_high << 16) |
>> + (i2c->t_priv.div_low & 0xffff), REG_CLKDIV);
>> clk_disable(i2c->clk);
>>
>> - t_low_ns = div_u64(((u64)div_low + 1) * 8 * 1000000000, clk_rate);
>> - t_high_ns = div_u64(((u64)div_high + 1) * 8 * 1000000000, clk_rate);
>> + t_low_ns = div_u64(((u64)i2c->t_priv.div_low + 1) * 8 * 1000000000,
>> + clk_rate);
>> + t_high_ns = div_u64(((u64)i2c->t_priv.div_high + 1) * 8 * 1000000000,
>> + clk_rate);
>> dev_dbg(i2c->dev,
>> "CLK %lukhz, Req %uns, Act low %lluns high %lluns\n",
>> clk_rate / 1000,
>> @@ -652,12 +676,11 @@ static int rk3x_i2c_clk_notifier_cb(struct notifier_block *nb, unsigned long
>> {
>> struct clk_notifier_data *ndata = data;
>> struct rk3x_i2c *i2c = container_of(nb, struct rk3x_i2c, clk_rate_nb);
>> - unsigned long div_low, div_high;
>>
>> switch (event) {
>> case PRE_RATE_CHANGE:
>> - if (rk3x_i2c_calc_divs(ndata->new_rate, &i2c->t,
>> - &div_low, &div_high) != 0)
>> + if (i2c->ops.calc_clock(ndata->new_rate, &i2c->t,
>> + &i2c->t_priv) != 0)
>> return NOTIFY_STOP;
>>
>> /* scale up */
>> @@ -861,6 +884,7 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
>> u32 value;
>> int irq;
>> unsigned long clk_rate;
>> + unsigned int version;
> No need to have a separate variable?
>
>> + version = (readl(i2c->regs + REG_CON) & VERSION_MASK) >> VERSION_SHIFT;
>> + if (version == RK3X_I2C_V0)
>> + i2c->ops.calc_clock = rk3x_i2c_v0_calc_clock;
> value = readl(i2c->regs + REG_CON);
> if ((value & VERSION_MASK) >> VERSION_SHIFT) == RK3X_I2C_V0)
> i2c->ops.calc_clock = rk3x_i2c_v0_calc_clock;
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/4] i2c: rk3x: new method to calculate i2c clocks
2016-01-14 16:12 ` Doug Anderson
@ 2016-01-15 9:39 ` David.Wu
-1 siblings, 0 replies; 25+ messages in thread
From: David.Wu @ 2016-01-15 9:39 UTC (permalink / raw)
To: Doug Anderson, David Wu
Cc: Heiko Stübner, Wolfram Sang, andy.shevchenko, Tao Huang,
Chris, Eddie Cai, Jianqun Xu, Lin Huang, linux-arm-kernel,
open list:ARM/Rockchip SoC...,
linux-i2c, linux-kernel
Hi Doug，
在 2016/1/15 0:12, Doug Anderson 写道:
> Hi,
>
> On Thu, Jan 14, 2016 at 4:31 AM, David Wu <david.wu@rock-chips.com> wrote:
>> There was an timing issue about "repeated start" time at the I2C
>> controller of version0, controller appears to drop SDA at .875x (7/8)
>> programmed clk high. The rule(.875x) isn't enough to meet tSU;STA
>> requirements on 100k's Standard-mode. To resolve this issue,
>> data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
>> timing information are added, new rules are designed to calculate
>> the timing information at new v1.
> I'm curious: does new hardware behave differently and that's why you
> need v1?
Yes , i didn't describe clearly about difference between old and new.
Old and new hardware behave the same except some timing rules.
v0 is the same with the v1 for tHigh and tLow :
tHigh = 8 * divh * pclk_cycle;
tLow = 8 * divl * pclk_cycle;
v0 rules' difference:
start setup： 7/8 * tHigh + 1 pclk cycle
start hold : 2 * （7/8 * tHigh） - 1 pclk cycle
stop setup : 7/8 * tHigh + 1 pclk cycle
data setup: 1/2 tLow - 1 pclk cycle
data hold : 1/2 tLow + 1 pclk cycle
For 100k's example:
spec_min_low_ns = 4700;
spec_min_high_ns = 4000;
spec_min_setup_start_ns = 4700;
We could calculate the timing info by the rules of v0, and ignore effect
of the pclk cycle here.
tSU;sta >=4700;
tHigh_min = tSU;sta * 8/7 >= 5372ns;
tHigh_min = max(tHigh_min, spec_min_high_ns) = 5372ns;
We get the final scl clk rate is 99k(1000000000 / (5372ns + 4700ns)),
it looks ok for the 100k
Standard mode. But the timing point of repeat start and low time is very
critical, it is dangerous
to some slave devices which are perhaps not so specified.
So need to give some time margin for them, that would increase the cycle
time and reduce
the clk rate, the final rate we get may be 97k or 96k.
In fact, we must think about scl rise time and scl fall time, and get
final clk may be 93k or less.
After that, we get about 7 percent lost at clk rate, it would reduce the
efficiency of i2c transfer.
In other words, we could say there is a timing issue about "repeat
start" time when we want
accurate 100k's rate, it is short to meet I2C spec requirements.
3.4M clk rate also has the same issue.
The start_setup count is added to fix this issue, tHigh is not need to
considered of "repeat start" time.
After divs calculated, the count would be calculated to meet i2c spec.
v1 rule:
start setup： tHigh + 1 pclk cycle
start hold: [8h * (u + 1) - 1] * T;
tSU;sto = (8h * p + 1) * T;
---------------------------------------------------------------------------------------------------------------
The data_upd_st is added for the Data set-up time and Data hold time on
Highspeed mode.
For 1.7M's example on v0:
tHD;DAT = 1/2 tLow；
tHD;DAT <= spec_max_data_hold_ns = 150ns
tLow <= 2 * tHD;DAT <= 300ns;
tLow >= spec_min_low_ns = 320ns;
According to these, we could not get a value for tLow, need changes for
the rule: tHD;DAT = 1/2 tLow.
Cut the tLow into eight euqal parts，the range of data_upd_st is 1 ~ 7.
It seems that v1 rule could resolve the issue.
v1 rule:
tHD;sda = n/8 * tLow;
tSU;sda = [(8 - n)/8 * tLow;
3.4M clk rate also has the same issue.
> ...or does old and new hardware behave the same and you're
> just introducing v1 so you don't mess with how old boards are working?
The registers of counts added would not effect old boards.
No matter what values of count was written in regs, that old i2c
controller didn't care,
it worked by original timing rule.
After picking this patch, pmic-rk818 and touchscreen-gt911 worked well
on the rk3368 sdk
board which use old method.
>
> >From the description it sounds as if the old code had problems as 100k too...
>
> If the new controller is different, I'd probably reword like the
> following (you'd have to re-wordwrap):
>
> There was an timing issue about "repeated start" time at the I2C
> controller of version0, controller appears to drop SDA at .875x (7/8)
> programmed clk high. On version 1 of the controller, the rule(.875x)
> isn't enough to meet tSU;STA
> requirements on 100k's Standard-mode. To resolve this issue,
> data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
> timing information are added, new rules are designed to calculate
> the timing information at new v1.
There was an timing issue about "repeated start" time at the I2C
controller of version0, controller appears to drop SDA at .875x (7/8)
programmed clk high. On version 1 of the controller, the rule(.875x)
isn't enough to meet tSU;STA requirements on 100k's Standard-mode.
To resolve this issue,data_upd_st, start_setup_cnt and stop_setup_cnt
configs for I2C timing information are added, new rules are designed
to calculate the timing information at new v1.
> -Doug
>
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 3/4] i2c: rk3x: new method to calculate i2c clocks
@ 2016-01-15 9:39 ` David.Wu
0 siblings, 0 replies; 25+ messages in thread
From: David.Wu @ 2016-01-15 9:39 UTC (permalink / raw)
To: linux-arm-kernel
Hi Doug?
? 2016/1/15 0:12, Doug Anderson ??:
> Hi,
>
> On Thu, Jan 14, 2016 at 4:31 AM, David Wu <david.wu@rock-chips.com> wrote:
>> There was an timing issue about "repeated start" time at the I2C
>> controller of version0, controller appears to drop SDA at .875x (7/8)
>> programmed clk high. The rule(.875x) isn't enough to meet tSU;STA
>> requirements on 100k's Standard-mode. To resolve this issue,
>> data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
>> timing information are added, new rules are designed to calculate
>> the timing information at new v1.
> I'm curious: does new hardware behave differently and that's why you
> need v1?
Yes , i didn't describe clearly about difference between old and new.
Old and new hardware behave the same except some timing rules.
v0 is the same with the v1 for tHigh and tLow :
tHigh = 8 * divh * pclk_cycle;
tLow = 8 * divl * pclk_cycle;
v0 rules' difference:
start setup? 7/8 * tHigh + 1 pclk cycle
start hold : 2 * ?7/8 * tHigh? - 1 pclk cycle
stop setup : 7/8 * tHigh + 1 pclk cycle
data setup: 1/2 tLow - 1 pclk cycle
data hold : 1/2 tLow + 1 pclk cycle
For 100k's example:
spec_min_low_ns = 4700;
spec_min_high_ns = 4000;
spec_min_setup_start_ns = 4700;
We could calculate the timing info by the rules of v0, and ignore effect
of the pclk cycle here.
tSU;sta >=4700;
tHigh_min = tSU;sta * 8/7 >= 5372ns;
tHigh_min = max(tHigh_min, spec_min_high_ns) = 5372ns;
We get the final scl clk rate is 99k(1000000000 / (5372ns + 4700ns)),
it looks ok for the 100k
Standard mode. But the timing point of repeat start and low time is very
critical, it is dangerous
to some slave devices which are perhaps not so specified.
So need to give some time margin for them, that would increase the cycle
time and reduce
the clk rate, the final rate we get may be 97k or 96k.
In fact, we must think about scl rise time and scl fall time, and get
final clk may be 93k or less.
After that, we get about 7 percent lost at clk rate, it would reduce the
efficiency of i2c transfer.
In other words, we could say there is a timing issue about "repeat
start" time when we want
accurate 100k's rate, it is short to meet I2C spec requirements.
3.4M clk rate also has the same issue.
The start_setup count is added to fix this issue, tHigh is not need to
considered of "repeat start" time.
After divs calculated, the count would be calculated to meet i2c spec.
v1 rule:
start setup? tHigh + 1 pclk cycle
start hold: [8h * (u + 1) - 1] * T;
tSU;sto = (8h * p + 1) * T;
---------------------------------------------------------------------------------------------------------------
The data_upd_st is added for the Data set-up time and Data hold time on
Highspeed mode.
For 1.7M's example on v0:
tHD;DAT = 1/2 tLow?
tHD;DAT <= spec_max_data_hold_ns = 150ns
tLow <= 2 * tHD;DAT <= 300ns;
tLow >= spec_min_low_ns = 320ns;
According to these, we could not get a value for tLow, need changes for
the rule: tHD;DAT = 1/2 tLow.
Cut the tLow into eight euqal parts?the range of data_upd_st is 1 ~ 7.
It seems that v1 rule could resolve the issue.
v1 rule:
tHD;sda = n/8 * tLow;
tSU;sda = [(8 - n)/8 * tLow;
3.4M clk rate also has the same issue.
> ...or does old and new hardware behave the same and you're
> just introducing v1 so you don't mess with how old boards are working?
The registers of counts added would not effect old boards.
No matter what values of count was written in regs, that old i2c
controller didn't care,
it worked by original timing rule.
After picking this patch, pmic-rk818 and touchscreen-gt911 worked well
on the rk3368 sdk
board which use old method.
>
> >From the description it sounds as if the old code had problems as 100k too...
>
> If the new controller is different, I'd probably reword like the
> following (you'd have to re-wordwrap):
>
> There was an timing issue about "repeated start" time at the I2C
> controller of version0, controller appears to drop SDA at .875x (7/8)
> programmed clk high. On version 1 of the controller, the rule(.875x)
> isn't enough to meet tSU;STA
> requirements on 100k's Standard-mode. To resolve this issue,
> data_upd_st, start_setup_cnt and stop_setup_cnt configs for I2C
> timing information are added, new rules are designed to calculate
> the timing information at new v1.
There was an timing issue about "repeated start" time at the I2C
controller of version0, controller appears to drop SDA at .875x (7/8)
programmed clk high. On version 1 of the controller, the rule(.875x)
isn't enough to meet tSU;STA requirements on 100k's Standard-mode.
To resolve this issue,data_upd_st, start_setup_cnt and stop_setup_cnt
configs for I2C timing information are added, new rules are designed
to calculate the timing information at new v1.
> -Doug
>
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/4] i2c: rk3x: new method to caculate i2c clocks
@ 2016-01-14 14:50 ` Julia Lawall
0 siblings, 0 replies; 25+ messages in thread
From: Julia Lawall @ 2016-01-14 14:50 UTC (permalink / raw)
Cc: heiko, wsa, andy.shevchenko, dianders, huangtao, zyw, cf, xjq,
hl, linux-arm-kernel, linux-rockchip, linux-gpio, linux-kernel,
David Wu
It looks like it would be an infinite loop?
julia
---------- Forwarded message ----------
Date: Thu, 14 Jan 2016 22:42:19 +0800
From: kbuild test robot <fengguang.wu@intel.com>
To: kbuild@01.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Subject: Re: [PATCH v3 3/4] i2c: rk3x: new method to caculate i2c clocks
CC: kbuild-all@01.org
In-Reply-To: <1452774699-57455-4-git-send-email-david.wu@rock-chips.com>
TO: David Wu <david.wu@rock-chips.com>
CC: heiko@sntech.de
CC: wsa@the-dreams.de, andy.shevchenko@gmail.com, dianders@chromium.org, huangtao@rock-chips.com, zyw@rock-chips.com, cf@rock-chips.com, xjq@rock-chips.com, hl@rock-chips.com, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, David Wu <david.wu@rock-chips.com>
Hi David,
[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.4 next-20160114]
[cannot apply to rockchip/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/David-Wu/i2c-rk3x-switch-to-i2c-generic-dt-parsing/20160114-203309
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux i2c/for-next
:::::: branch date: 2 hours ago
:::::: commit date: 2 hours ago
>> drivers/i2c/busses/i2c-rk3x.c:793:23-34: WARNING: Unsigned expression compared with zero: data_hd_cnt >= 0
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 5fed4c320427dd34a909f9b408fadf297121924c
vim +793 drivers/i2c/busses/i2c-rk3x.c
5fed4c32 David Wu 2016-01-14 777 * for low (spend more time low).
5fed4c32 David Wu 2016-01-14 778 */
5fed4c32 David Wu 2016-01-14 779 extra_div = min_total_div - min_div_for_hold;
5fed4c32 David Wu 2016-01-14 780 extra_low_div = DIV_ROUND_UP(min_low_div * extra_div,
5fed4c32 David Wu 2016-01-14 781 min_div_for_hold);
5fed4c32 David Wu 2016-01-14 782
5fed4c32 David Wu 2016-01-14 783 t_output->div_low = min_low_div + extra_low_div;
5fed4c32 David Wu 2016-01-14 784 t_output->div_high = min_high_div + (extra_div - extra_low_div);
5fed4c32 David Wu 2016-01-14 785 }
5fed4c32 David Wu 2016-01-14 786
5fed4c32 David Wu 2016-01-14 787 /*
5fed4c32 David Wu 2016-01-14 788 * calculate sda data hold count by the rules, thd_sda_count:3
5fed4c32 David Wu 2016-01-14 789 * is a appropriate value to reduce calculated times.
5fed4c32 David Wu 2016-01-14 790 * tHD;sda = (l * s + 1) * T
5fed4c32 David Wu 2016-01-14 791 * tSU;sda = ((8 - s) * l + 1) * T
5fed4c32 David Wu 2016-01-14 792 */
5fed4c32 David Wu 2016-01-14 @793 for (data_hd_cnt = 3; data_hd_cnt >= 0; data_hd_cnt--) {
5fed4c32 David Wu 2016-01-14 794 max_hold_data_ns = DIV_ROUND_UP((data_hd_cnt
5fed4c32 David Wu 2016-01-14 795 * (t_output->div_low) + 1)
5fed4c32 David Wu 2016-01-14 796 * 1000000, clk_rate_khz);
5fed4c32 David Wu 2016-01-14 797 min_setup_data_ns = DIV_ROUND_UP(((8 - data_hd_cnt)
5fed4c32 David Wu 2016-01-14 798 * (t_output->div_low) + 1)
5fed4c32 David Wu 2016-01-14 799 * 1000000, clk_rate_khz);
5fed4c32 David Wu 2016-01-14 800 if ((max_hold_data_ns < spec_max_data_hold_ns) &&
5fed4c32 David Wu 2016-01-14 801 (min_setup_data_ns > spec_min_data_setup_ns)) {
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/4] i2c: rk3x: new method to caculate i2c clocks
@ 2016-01-14 14:50 ` Julia Lawall
0 siblings, 0 replies; 25+ messages in thread
From: Julia Lawall @ 2016-01-14 14:50 UTC (permalink / raw)
To: David Wu
Cc: heiko, wsa, andy.shevchenko, dianders, huangtao, zyw, cf, xjq,
hl, linux-arm-kernel, linux-rockchip, linux-gpio, linux-kernel,
David Wu
It looks like it would be an infinite loop?
julia
---------- Forwarded message ----------
Date: Thu, 14 Jan 2016 22:42:19 +0800
From: kbuild test robot <fengguang.wu@intel.com>
To: kbuild@01.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Subject: Re: [PATCH v3 3/4] i2c: rk3x: new method to caculate i2c clocks
CC: kbuild-all@01.org
In-Reply-To: <1452774699-57455-4-git-send-email-david.wu@rock-chips.com>
TO: David Wu <david.wu@rock-chips.com>
CC: heiko@sntech.de
CC: wsa@the-dreams.de, andy.shevchenko@gmail.com, dianders@chromium.org, huangtao@rock-chips.com, zyw@rock-chips.com, cf@rock-chips.com, xjq@rock-chips.com, hl@rock-chips.com, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, David Wu <david.wu@rock-chips.com>
Hi David,
[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.4 next-20160114]
[cannot apply to rockchip/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/David-Wu/i2c-rk3x-switch-to-i2c-generic-dt-parsing/20160114-203309
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux i2c/for-next
:::::: branch date: 2 hours ago
:::::: commit date: 2 hours ago
>> drivers/i2c/busses/i2c-rk3x.c:793:23-34: WARNING: Unsigned expression compared with zero: data_hd_cnt >= 0
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 5fed4c320427dd34a909f9b408fadf297121924c
vim +793 drivers/i2c/busses/i2c-rk3x.c
5fed4c32 David Wu 2016-01-14 777 * for low (spend more time low).
5fed4c32 David Wu 2016-01-14 778 */
5fed4c32 David Wu 2016-01-14 779 extra_div = min_total_div - min_div_for_hold;
5fed4c32 David Wu 2016-01-14 780 extra_low_div = DIV_ROUND_UP(min_low_div * extra_div,
5fed4c32 David Wu 2016-01-14 781 min_div_for_hold);
5fed4c32 David Wu 2016-01-14 782
5fed4c32 David Wu 2016-01-14 783 t_output->div_low = min_low_div + extra_low_div;
5fed4c32 David Wu 2016-01-14 784 t_output->div_high = min_high_div + (extra_div - extra_low_div);
5fed4c32 David Wu 2016-01-14 785 }
5fed4c32 David Wu 2016-01-14 786
5fed4c32 David Wu 2016-01-14 787 /*
5fed4c32 David Wu 2016-01-14 788 * calculate sda data hold count by the rules, thd_sda_count:3
5fed4c32 David Wu 2016-01-14 789 * is a appropriate value to reduce calculated times.
5fed4c32 David Wu 2016-01-14 790 * tHD;sda = (l * s + 1) * T
5fed4c32 David Wu 2016-01-14 791 * tSU;sda = ((8 - s) * l + 1) * T
5fed4c32 David Wu 2016-01-14 792 */
5fed4c32 David Wu 2016-01-14 @793 for (data_hd_cnt = 3; data_hd_cnt >= 0; data_hd_cnt--) {
5fed4c32 David Wu 2016-01-14 794 max_hold_data_ns = DIV_ROUND_UP((data_hd_cnt
5fed4c32 David Wu 2016-01-14 795 * (t_output->div_low) + 1)
5fed4c32 David Wu 2016-01-14 796 * 1000000, clk_rate_khz);
5fed4c32 David Wu 2016-01-14 797 min_setup_data_ns = DIV_ROUND_UP(((8 - data_hd_cnt)
5fed4c32 David Wu 2016-01-14 798 * (t_output->div_low) + 1)
5fed4c32 David Wu 2016-01-14 799 * 1000000, clk_rate_khz);
5fed4c32 David Wu 2016-01-14 800 if ((max_hold_data_ns < spec_max_data_hold_ns) &&
5fed4c32 David Wu 2016-01-14 801 (min_setup_data_ns > spec_min_data_setup_ns)) {
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2016-01-15 9:39 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-14 12:31 [PATCH v3 0/4] Add rk3399 i2c clocks calculated method David Wu
2016-01-14 12:31 ` David Wu
2016-01-14 12:31 ` [PATCH v3 1/4] i2c: rk3x: switch to i2c generic dt parsing David Wu
2016-01-14 12:31 ` David Wu
2016-01-14 13:05 ` Andy Shevchenko
2016-01-14 13:05 ` Andy Shevchenko
2016-01-14 12:31 ` [PATCH v3 2/4] i2c: rk3x: add ops to caculate i2c clocks David Wu
2016-01-14 12:31 ` David Wu
2016-01-14 13:19 ` Andy Shevchenko
2016-01-14 13:19 ` Andy Shevchenko
2016-01-15 8:34 ` David.Wu
2016-01-15 8:34 ` David.Wu
2016-01-14 12:31 ` [PATCH v3 3/4] i2c: rk3x: new method " David Wu
2016-01-14 12:31 ` David Wu
2016-01-14 13:29 ` Andy Shevchenko
2016-01-14 13:29 ` Andy Shevchenko
2016-01-14 16:12 ` Doug Anderson
2016-01-14 16:12 ` Doug Anderson
2016-01-14 16:12 ` Doug Anderson
2016-01-15 9:39 ` [PATCH v3 3/4] i2c: rk3x: new method to calculate " David.Wu
2016-01-15 9:39 ` David.Wu
2016-01-14 12:31 ` [PATCH v3 4/4] i2c: rk3x: support I2C Highspeed Mode David Wu
2016-01-14 12:31 ` David Wu
2016-01-14 14:50 [PATCH v3 3/4] i2c: rk3x: new method to caculate i2c clocks Julia Lawall
2016-01-14 14:50 ` Julia Lawall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.