Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1398669 > unrolled thread

[PATCH v8 0/8] add i2c driver supported for rk3399

Back to article view | Back to linux.kernel

Contents

  [PATCH v8 0/8] add i2c driver supported for rk3399 David Wu <david.wu@rock-chips.com> - 2016-05-11 06:10 +0200
    [PATCH v8 2/8] i2c: rk3x: use struct "rk3x_i2c_calced_timings" David Wu <david.wu@rock-chips.com> - 2016-05-11 06:10 +0200
    [PATCH v8 1/8] i2c: rk3x: add documentation to fields in "struct rk3x_i2c" David Wu <david.wu@rock-chips.com> - 2016-05-11 06:10 +0200
      Re: [PATCH v8 1/8] i2c: rk3x: add documentation to fields in "struct rk3x_i2c" Heiko Stuebner <heiko@sntech.de> - 2016-05-11 17:10 +0200
    [PATCH v8 3/8] i2c: rk3x: Remove redundant rk3x_i2c_clean_ipd() David Wu <david.wu@rock-chips.com> - 2016-05-11 06:10 +0200
      Re: [PATCH v8 3/8] i2c: rk3x: Remove redundant rk3x_i2c_clean_ipd() Heiko Stuebner <heiko@sntech.de> - 2016-05-11 20:30 +0200
        Re: [PATCH v8 3/8] i2c: rk3x: Remove redundant rk3x_i2c_clean_ipd() "David.Wu" <david.wu@rock-chips.com> - 2016-05-12 03:20 +0200
    [PATCH v8 4/8] i2c: rk3x: Change SoC data to not use array David Wu <david.wu@rock-chips.com> - 2016-05-11 06:10 +0200
    [PATCH v8 5/8] i2c: rk3x: Move spec timing data to "static const" structs David Wu <david.wu@rock-chips.com> - 2016-05-11 06:10 +0200
    [PATCH v8 8/8] i2c: rk3x: support fast-mode plus for rk3399 David Wu <david.wu@rock-chips.com> - 2016-05-11 06:20 +0200
      Re: [PATCH v8 8/8] i2c: rk3x: support fast-mode plus for rk3399 Caesar Wang <caesar.upstream@gmail.com> - 2016-05-11 13:50 +0200
      Re: [PATCH v8 8/8] i2c: rk3x: support fast-mode plus for rk3399 Heiko Stuebner <heiko@sntech.de> - 2016-05-11 23:20 +0200
        Re: [PATCH v8 8/8] i2c: rk3x: support fast-mode plus for rk3399 Doug Anderson <dianders@chromium.org> - 2016-05-12 01:50 +0200
          Re: [PATCH v8 8/8] i2c: rk3x: support fast-mode plus for rk3399 Heiko Stuebner <heiko@sntech.de> - 2016-05-12 10:40 +0200

#1398669 — [PATCH v8 0/8] add i2c driver supported for rk3399

There are three points differert from others:
 - new method to caculate i2c timings for rk3399
 - pclk and function clk are separated at rk3399
 - add fast-plus mode supported for rk3399

David Wu (8):
  i2c: rk3x: add documentation to fields in "struct rk3x_i2c"
  i2c: rk3x: use struct "rk3x_i2c_calced_timings"
  i2c: rk3x: Remove redundant rk3x_i2c_clean_ipd()
  i2c: rk3x: Change SoC data to not use array
  i2c: rk3x: Move spec timing data to "static const" structs
  dt-bindings: i2c: rk3x: add support for rk3399
  i2c: rk3x: add i2c support for rk3399 soc
  i2c: rk3x: support fast-mode plus for rk3399

 Documentation/devicetree/bindings/i2c/i2c-rk3x.txt |  16 +-
 drivers/i2c/busses/i2c-rk3x.c                      | 493 +++++++++++++++++----
 2 files changed, 430 insertions(+), 79 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1398672 — [PATCH v8 2/8] i2c: rk3x: use struct "rk3x_i2c_calced_timings"

The "div_high" and "div_low" values are always used together.  Group them
into a structure to make it easier to pass them both around.  This
structure also provides a place for future calculated timings.

Signed-off-by: David Wu <david.wu@rock-chips.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
Change in v8:
- add commit description.

 drivers/i2c/busses/i2c-rk3x.c | 55 +++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 7e45d51..1e2677a 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -75,6 +75,16 @@ enum {
 #define WAIT_TIMEOUT      1000 /* ms */
 #define DEFAULT_SCL_RATE  (100 * 1000) /* Hz */
 
+/**
+ * struct rk3x_i2c_calced_timings:
+ * @div_low: Divider output for low
+ * @div_high: Divider output for high
+ */
+struct rk3x_i2c_calced_timings {
+	unsigned long div_low;
+	unsigned long div_high;
+};
+
 enum rk3x_i2c_state {
 	STATE_IDLE,
 	STATE_START,
@@ -454,9 +464,8 @@ out:
  * Calculate divider values for desired SCL frequency
  *
  * @clk_rate: I2C input clock rate
- * @t: Known I2C timing information.
- * @div_low: Divider output for low
- * @div_high: Divider output for high
+ * @t: Known I2C timing information
+ * @t_calc: Caculated rk3x private timings 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
@@ -464,8 +473,7 @@ out:
  */
 static int rk3x_i2c_calc_divs(unsigned long clk_rate,
 			      struct i2c_timings *t,
-			      unsigned long *div_low,
-			      unsigned long *div_high)
+			      struct rk3x_i2c_calced_timings *t_calc)
 {
 	unsigned long spec_min_low_ns, spec_min_high_ns;
 	unsigned long spec_setup_start, spec_max_data_hold_ns;
@@ -572,8 +580,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_calc->div_low = min_low_div;
+		t_calc->div_high = min_high_div;
 	} else {
 		/*
 		 * We've got to distribute some time among the low and high
@@ -602,25 +610,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_calc->div_low = ideal_low_div;
+		t_calc->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_calc->div_low--;
+	t_calc->div_high--;
 
 	/* Maximum divider supported by hw is 0xffff */
-	if (*div_low > 0xffff) {
-		*div_low = 0xffff;
+	if (t_calc->div_low > 0xffff) {
+		t_calc->div_low = 0xffff;
 		ret = -EINVAL;
 	}
 
-	if (*div_high > 0xffff) {
-		*div_high = 0xffff;
+	if (t_calc->div_high > 0xffff) {
+		t_calc->div_high = 0xffff;
 		ret = -EINVAL;
 	}
 
@@ -630,19 +638,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)
 {
 	struct i2c_timings *t = &i2c->t;
-	unsigned long div_low, div_high;
+	struct rk3x_i2c_calced_timings calc;
 	u64 t_low_ns, t_high_ns;
 	int ret;
 
-	ret = rk3x_i2c_calc_divs(clk_rate, t, &div_low, &div_high);
+	ret = rk3x_i2c_calc_divs(clk_rate, t, &calc);
 	WARN_ONCE(ret != 0, "Could not reach SCL freq %u", t->bus_freq_hz);
 
 	clk_enable(i2c->clk);
-	i2c_writel(i2c, (div_high << 16) | (div_low & 0xffff), REG_CLKDIV);
+	i2c_writel(i2c, (calc.div_high << 16) | (calc.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)calc.div_low + 1) * 8 * 1000000000, clk_rate);
+	t_high_ns = div_u64(((u64)calc.div_high + 1) * 8 * 1000000000,
+			    clk_rate);
 	dev_dbg(i2c->dev,
 		"CLK %lukhz, Req %uns, Act low %lluns high %lluns\n",
 		clk_rate / 1000,
@@ -672,12 +682,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;
+	struct rk3x_i2c_calced_timings calc;
 
 	switch (event) {
 	case PRE_RATE_CHANGE:
-		if (rk3x_i2c_calc_divs(ndata->new_rate, &i2c->t,
-				       &div_low, &div_high) != 0)
+		if (rk3x_i2c_calc_divs(ndata->new_rate, &i2c->t, &calc) != 0)
 			return NOTIFY_STOP;
 
 		/* scale up */
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1398673 — [PATCH v8 1/8] i2c: rk3x: add documentation to fields in "struct rk3x_i2c"

Signed-off-by: David Wu <david.wu@rock-chips.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
Change in v8:
- none

 drivers/i2c/busses/i2c-rk3x.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 80bed02..7e45d51 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -90,6 +90,26 @@ struct rk3x_i2c_soc_data {
 	int grf_offset;
 };
 
+/**
+ * struct rk3x_i2c - private data of the controller
+ * @adap: corresponding I2C adapter
+ * @dev: device for this controller
+ * @soc_data: related soc data struct
+ * @regs: virtual memory area
+ * @clk: clock of i2c bus
+ * @clk_rate_nb: i2c clk rate change notify
+ * @t: I2C known timing information
+ * @lock: spinlock for the i2c bus
+ * @wait: the waitqueue to wait for i2c transfer
+ * @busy: the condition for the event to wait for
+ * @msg: current i2c message
+ * @addr: addr of i2c slave device
+ * @mode: mode of i2c transfer
+ * @is_last_msg: flag determines whether it is the last msg in this transfer
+ * @state: state of i2c transfer
+ * @processed: byte length which has been send or received
+ * @error: error code for i2c transfer
+ */
 struct rk3x_i2c {
 	struct i2c_adapter adap;
 	struct device *dev;
@@ -116,7 +136,7 @@ struct rk3x_i2c {
 
 	/* I2C state machine */
 	enum rk3x_i2c_state state;
-	unsigned int processed; /* sent/received bytes */
+	unsigned int processed;
 	int error;
 };
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1399230 — Re: [PATCH v8 1/8] i2c: rk3x: add documentation to fields in "struct rk3x_i2c"

Am Mittwoch, 11. Mai 2016, 03:24:05 schrieb David Wu:

In general, all patches should have a commit message (even if it's obvious) 
and some maintainers even enforce this in a hard way.

How about something like:

----
Add kernel-doc documentation for the elements of the previously
undocumented struct rk3x_i2c.
----

Some minor nitpicks below, otherwise
Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> Signed-off-by: David Wu <david.wu@rock-chips.com>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
> Change in v8:
> - none
> 
>  drivers/i2c/busses/i2c-rk3x.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> index 80bed02..7e45d51 100644
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -90,6 +90,26 @@ struct rk3x_i2c_soc_data {
>  	int grf_offset;
>  };
> 
> +/**
> + * struct rk3x_i2c - private data of the controller
> + * @adap: corresponding I2C adapter
> + * @dev: device for this controller
> + * @soc_data: related soc data struct
> + * @regs: virtual memory area
> + * @clk: clock of i2c bus
> + * @clk_rate_nb: i2c clk rate change notify
		^ i2c clk rate change notifier block

> + * @t: I2C known timing information
> + * @lock: spinlock for the i2c bus
> + * @wait: the waitqueue to wait for i2c transfer
> + * @busy: the condition for the event to wait for
> + * @msg: current i2c message
> + * @addr: addr of i2c slave device
> + * @mode: mode of i2c transfer
> + * @is_last_msg: flag determines whether it is the last msg in this
> transfer + * @state: state of i2c transfer
> + * @processed: byte length which has been send or received
										^ sent (with a "t")

> + * @error: error code for i2c transfer
> + */
>  struct rk3x_i2c {
>  	struct i2c_adapter adap;
>  	struct device *dev;
> @@ -116,7 +136,7 @@ struct rk3x_i2c {
> 
>  	/* I2C state machine */
>  	enum rk3x_i2c_state state;
> -	unsigned int processed; /* sent/received bytes */
> +	unsigned int processed;
>  	int error;
>  };

[toc] | [prev] | [next] | [standalone]

#1398674 — [PATCH v8 3/8] i2c: rk3x: Remove redundant rk3x_i2c_clean_ipd()

Call rk3x_i2c_setup() before rk3x_i2c_start()
and the last thing in setup was to clean the IPD,
so no reason to do it at the beginning of start.

Signed-off-by: David Wu <david.wu@rock-chips.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
Change in v8:
- none

 drivers/i2c/busses/i2c-rk3x.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 1e2677a..9eeb4e5 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -174,7 +174,6 @@ static void rk3x_i2c_start(struct rk3x_i2c *i2c)
 {
 	u32 val;
 
-	rk3x_i2c_clean_ipd(i2c);
 	i2c_writel(i2c, REG_INT_START, REG_IEN);
 
 	/* enable adapter with correct mode, send START condition */
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1399404 — Re: [PATCH v8 3/8] i2c: rk3x: Remove redundant rk3x_i2c_clean_ipd()

Am Mittwoch, 11. Mai 2016, 03:24:07 schrieb David Wu:
> Call rk3x_i2c_setup() before rk3x_i2c_start()

That beginning of the sentence could use a tiny improvement, like

----
rk3x_i2c_setup() gets called directly before rk3x_i2c_start()
----

to make it obvious what happens in the code.

But a nice small improvement
Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> and the last thing in setup was to clean the IPD,
> so no reason to do it at the beginning of start.
> 
> Signed-off-by: David Wu <david.wu@rock-chips.com>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
> Change in v8:
> - none
> 
>  drivers/i2c/busses/i2c-rk3x.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> index 1e2677a..9eeb4e5 100644
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -174,7 +174,6 @@ static void rk3x_i2c_start(struct rk3x_i2c *i2c)
>  {
>  	u32 val;
> 
> -	rk3x_i2c_clean_ipd(i2c);
>  	i2c_writel(i2c, REG_INT_START, REG_IEN);
> 
>  	/* enable adapter with correct mode, send START condition */

[toc] | [prev] | [next] | [standalone]

#1399629 — Re: [PATCH v8 3/8] i2c: rk3x: Remove redundant rk3x_i2c_clean_ipd()

Hi Heiko

在 2016/5/12 2:26, Heiko Stuebner 写道:
> Am Mittwoch, 11. Mai 2016, 03:24:07 schrieb David Wu:
>> Call rk3x_i2c_setup() before rk3x_i2c_start()
>
> That beginning of the sentence could use a tiny improvement, like
>
> ----
> rk3x_i2c_setup() gets called directly before rk3x_i2c_start()
> ----
>

Okay, i will add it in next version, Thanks.

> to make it obvious what happens in the code.
>
> But a nice small improvement
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>
>> and the last thing in setup was to clean the IPD,
>> so no reason to do it at the beginning of start.
>>
>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>> ---
>> Change in v8:
>> - none
>>
>>   drivers/i2c/busses/i2c-rk3x.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
>> index 1e2677a..9eeb4e5 100644
>> --- a/drivers/i2c/busses/i2c-rk3x.c
>> +++ b/drivers/i2c/busses/i2c-rk3x.c
>> @@ -174,7 +174,6 @@ static void rk3x_i2c_start(struct rk3x_i2c *i2c)
>>   {
>>   	u32 val;
>>
>> -	rk3x_i2c_clean_ipd(i2c);
>>   	i2c_writel(i2c, REG_INT_START, REG_IEN);
>>
>>   	/* enable adapter with correct mode, send START condition */
>
>
>
>

[toc] | [prev] | [next] | [standalone]

#1398675 — [PATCH v8 4/8] i2c: rk3x: Change SoC data to not use array

Specifying the i2c SoC data in an array provides very little benefit and
gets unwieldly / confusing as the array grows since the next bit of code
needs to refer to elements in the array by their raw integral index.

Let's just create a single 'static const' structure for each SoC so that
we can refer to these structures by ID.

Signed-off-by: David Wu <david.wu@rock-chips.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Suggested-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
Change in v8:
- add commit description.

 drivers/i2c/busses/i2c-rk3x.c | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 9eeb4e5..d7a871f 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -864,17 +864,39 @@ static const struct i2c_algorithm rk3x_i2c_algorithm = {
 	.functionality		= rk3x_i2c_func,
 };
 
-static struct rk3x_i2c_soc_data soc_data[3] = {
-	{ .grf_offset = 0x154 }, /* rk3066 */
-	{ .grf_offset = 0x0a4 }, /* rk3188 */
-	{ .grf_offset = -1 },    /* no I2C switching needed */
+static const struct rk3x_i2c_soc_data rk3066_soc_data = {
+	.grf_offset = 0x154,
+};
+
+static const struct rk3x_i2c_soc_data rk3188_soc_data = {
+	.grf_offset = 0x0a4,
+};
+
+static const struct rk3x_i2c_soc_data rk3228_soc_data = {
+	.grf_offset = -1,
+};
+
+static const struct rk3x_i2c_soc_data rk3288_soc_data = {
+	.grf_offset = -1,
 };
 
 static const struct of_device_id rk3x_i2c_match[] = {
-	{ .compatible = "rockchip,rk3066-i2c", .data = (void *)&soc_data[0] },
-	{ .compatible = "rockchip,rk3188-i2c", .data = (void *)&soc_data[1] },
-	{ .compatible = "rockchip,rk3228-i2c", .data = (void *)&soc_data[2] },
-	{ .compatible = "rockchip,rk3288-i2c", .data = (void *)&soc_data[2] },
+	{
+		.compatible = "rockchip,rk3066-i2c",
+		.data = (void *)&rk3066_soc_data
+	},
+	{
+		.compatible = "rockchip,rk3188-i2c",
+		.data = (void *)&rk3188_soc_data
+	},
+	{
+		.compatible = "rockchip,rk3228-i2c",
+		.data = (void *)&rk3228_soc_data
+	},
+	{
+		.compatible = "rockchip,rk3288-i2c",
+		.data = (void *)&rk3288_soc_data
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, rk3x_i2c_match);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1398678 — [PATCH v8 5/8] i2c: rk3x: Move spec timing data to "static const" structs

The i2c timing specs are really just constant data.  There's no reason
to write code to init them, so move them out to structures.  This not
only is a cleaner solution but it will reduce code duplication when we
introduce a new variant of rk3x_i2c_calc_divs() in a future patch.

Signed-off-by: David Wu <david.wu@rock-chips.com>
Suggested-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v8:
- add commit description.
- remove the spec values that were not needed, then
  introduce the additional values in the rk3399 patch. 

 drivers/i2c/busses/i2c-rk3x.c | 83 ++++++++++++++++++++++++++++---------------
 1 file changed, 55 insertions(+), 28 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index d7a871f..9791797 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -76,6 +76,34 @@ enum {
 #define DEFAULT_SCL_RATE  (100 * 1000) /* Hz */
 
 /**
+ * struct i2c_spec_values:
+ * @min_low_ns: min LOW period of the SCL clock
+ * @min_high_ns: min HIGH period of the SCL cloc
+ * @min_setup_start_ns: min set-up time for a repeated START conditio
+ * @max_data_hold_ns: max data hold time
+ */
+struct i2c_spec_values {
+	unsigned long min_low_ns;
+	unsigned long min_high_ns;
+	unsigned long min_setup_start_ns;
+	unsigned long max_data_hold_ns;
+};
+
+static const struct i2c_spec_values standard_mode_spec = {
+	.min_low_ns = 4700,
+	.min_high_ns = 4000,
+	.min_setup_start_ns = 4700,
+	.max_data_hold_ns = 3450,
+};
+
+static const struct i2c_spec_values fast_mode_spec = {
+	.min_low_ns = 1300,
+	.min_high_ns = 600,
+	.min_setup_start_ns = 600,
+	.max_data_hold_ns = 900,
+};
+
+/**
  * struct rk3x_i2c_calced_timings:
  * @div_low: Divider output for low
  * @div_high: Divider output for high
@@ -460,6 +488,21 @@ out:
 }
 
 /**
+ * Get timing values of I2C specification
+ *
+ * @speed: Desired SCL frequency
+ *
+ * Returns: Matched i2c spec values.
+ */
+static const struct i2c_spec_values *rk3x_i2c_get_spec(unsigned int speed)
+{
+	if (speed <= 100000)
+		return &standard_mode_spec;
+	else
+		return &fast_mode_spec;
+}
+
+/**
  * Calculate divider values for desired SCL frequency
  *
  * @clk_rate: I2C input clock rate
@@ -474,10 +517,6 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
 			      struct i2c_timings *t,
 			      struct rk3x_i2c_calced_timings *t_calc)
 {
-	unsigned long spec_min_low_ns, spec_min_high_ns;
-	unsigned long spec_setup_start, spec_max_data_hold_ns;
-	unsigned long data_hold_buffer_ns;
-
 	unsigned long min_low_ns, min_high_ns;
 	unsigned long max_low_ns, min_total_ns;
 
@@ -489,6 +528,8 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
 	unsigned long min_div_for_hold, min_total_div;
 	unsigned long extra_div, extra_low_div, ideal_low_div;
 
+	unsigned long data_hold_buffer_ns = 50;
+	const struct i2c_spec_values *spec;
 	int ret = 0;
 
 	/* Only support standard-mode and fast-mode */
@@ -511,22 +552,8 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
 	 *	 This is because the i2c host on Rockchip holds the data line
 	 *	 for half the low time.
 	 */
-	if (t->bus_freq_hz <= 100000) {
-		/* Standard-mode */
-		spec_min_low_ns = 4700;
-		spec_setup_start = 4700;
-		spec_min_high_ns = 4000;
-		spec_max_data_hold_ns = 3450;
-		data_hold_buffer_ns = 50;
-	} else {
-		/* Fast-mode */
-		spec_min_low_ns = 1300;
-		spec_setup_start = 600;
-		spec_min_high_ns = 600;
-		spec_max_data_hold_ns = 900;
-		data_hold_buffer_ns = 50;
-	}
-	min_high_ns = t->scl_rise_ns + spec_min_high_ns;
+	spec = rk3x_i2c_get_spec(t->bus_freq_hz);
+	min_high_ns = t->scl_rise_ns + spec->min_high_ns;
 
 	/*
 	 * Timings for repeated start:
@@ -536,14 +563,14 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
 	 * We need to account for those rules in picking our "high" time so
 	 * we meet tSU;STA and tHD;STA times.
 	 */
-	min_high_ns = max(min_high_ns,
-		DIV_ROUND_UP((t->scl_rise_ns + spec_setup_start) * 1000, 875));
-	min_high_ns = max(min_high_ns,
-		DIV_ROUND_UP((t->scl_rise_ns + spec_setup_start +
-			      t->sda_fall_ns + spec_min_high_ns), 2));
-
-	min_low_ns = t->scl_fall_ns + spec_min_low_ns;
-	max_low_ns = spec_max_data_hold_ns * 2 - data_hold_buffer_ns;
+	min_high_ns = max(min_high_ns, DIV_ROUND_UP(
+		(t->scl_rise_ns + spec->min_setup_start_ns) * 1000, 875));
+	min_high_ns = max(min_high_ns, DIV_ROUND_UP(
+		(t->scl_rise_ns + spec->min_setup_start_ns + t->sda_fall_ns +
+		spec->min_high_ns), 2));
+
+	min_low_ns = t->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 */
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1398683 — [PATCH v8 8/8] i2c: rk3x: support fast-mode plus for rk3399

Signed-off-by: David Wu <david.wu@rock-chips.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
Change in v8:
- None

 drivers/i2c/busses/i2c-rk3x.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 25ed1ad..0ba25ee 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -126,6 +126,17 @@ static const struct i2c_spec_values fast_mode_spec = {
 	.min_hold_buffer_ns = 1300,
 };
 
+static const struct i2c_spec_values fast_mode_plus_spec = {
+	.min_hold_start_ns = 260,
+	.min_low_ns = 500,
+	.min_high_ns = 260,
+	.min_setup_start_ns = 260,
+	.max_data_hold_ns = 400,
+	.min_data_setup_ns = 50,
+	.min_setup_stop_ns = 260,
+	.min_hold_buffer_ns = 500,
+};
+
 /**
  * struct rk3x_i2c_calced_timings:
  * @div_low: Divider output for low
@@ -531,8 +542,10 @@ static const struct i2c_spec_values *rk3x_i2c_get_spec(unsigned int speed)
 {
 	if (speed <= 100000)
 		return &standard_mode_spec;
-	else
+	else if (speed <= 400000)
 		return &fast_mode_spec;
+	else
+		return &fast_mode_plus_spec;
 }
 
 /**
@@ -743,9 +756,9 @@ static int rk3x_i2c_v1_calc_timings(unsigned long clk_rate,
 	const struct i2c_spec_values *spec;
 	int ret = 0;
 
-	/* Support standard-mode and fast-mode */
-	if (WARN_ON(t->bus_freq_hz > 400000))
-		t->bus_freq_hz = 400000;
+	/* Support standard-mode, fast-mode and fast-mode plus */
+	if (WARN_ON(t->bus_freq_hz > 1000000))
+		t->bus_freq_hz = 1000000;
 
 	/* prevent scl_rate_khz from becoming 0 */
 	if (WARN_ON(t->bus_freq_hz < 1000))
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1398973 — Re: [PATCH v8 8/8] i2c: rk3x: support fast-mode plus for rk3399

在 2016年05月11日 03:33, David Wu 写道:
> Signed-off-by: David Wu <david.wu@rock-chips.com>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Tested-by: Caesar Wang <wxt@rock-chips.com>
> ---
> Change in v8:
> - None
>
>   drivers/i2c/busses/i2c-rk3x.c | 21 +++++++++++++++++----
>   1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> index 25ed1ad..0ba25ee 100644
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -126,6 +126,17 @@ static const struct i2c_spec_values fast_mode_spec = {
>   	.min_hold_buffer_ns = 1300,
>   };
>   
> +static const struct i2c_spec_values fast_mode_plus_spec = {
> +	.min_hold_start_ns = 260,
> +	.min_low_ns = 500,
> +	.min_high_ns = 260,
> +	.min_setup_start_ns = 260,
> +	.max_data_hold_ns = 400,
> +	.min_data_setup_ns = 50,
> +	.min_setup_stop_ns = 260,
> +	.min_hold_buffer_ns = 500,
> +};
> +
>   /**
>    * struct rk3x_i2c_calced_timings:
>    * @div_low: Divider output for low
> @@ -531,8 +542,10 @@ static const struct i2c_spec_values *rk3x_i2c_get_spec(unsigned int speed)
>   {
>   	if (speed <= 100000)
>   		return &standard_mode_spec;
> -	else
> +	else if (speed <= 400000)
>   		return &fast_mode_spec;
> +	else
> +		return &fast_mode_plus_spec;
>   }
>   
>   /**
> @@ -743,9 +756,9 @@ static int rk3x_i2c_v1_calc_timings(unsigned long clk_rate,
>   	const struct i2c_spec_values *spec;
>   	int ret = 0;
>   
> -	/* Support standard-mode and fast-mode */
> -	if (WARN_ON(t->bus_freq_hz > 400000))
> -		t->bus_freq_hz = 400000;
> +	/* Support standard-mode, fast-mode and fast-mode plus */
> +	if (WARN_ON(t->bus_freq_hz > 1000000))
> +		t->bus_freq_hz = 1000000;
>   
>   	/* prevent scl_rate_khz from becoming 0 */
>   	if (WARN_ON(t->bus_freq_hz < 1000))


-- 
Thanks,
Caesar

[toc] | [prev] | [next] | [standalone]

#1399521 — Re: [PATCH v8 8/8] i2c: rk3x: support fast-mode plus for rk3399

Am Mittwoch, 11. Mai 2016, 03:33:14 schrieb David Wu:

please always try to also provide some sort of commit message.

---
Implement fast mode plus that allows bus speeds of up to 1MHz.
.....

---

Additionally, the i2c-noob in me would wish for a short sentence on where 
the timing values come from (measured, somewhere in the TRM I didn't find or 
something completely different).


Patch content itself looks good, so with a suitable commit message,
Reviewed-by: Heiko Stuebner <heiko@sntech.de>


> Signed-off-by: David Wu <david.wu@rock-chips.com>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
> Change in v8:
> - None
> 
>  drivers/i2c/busses/i2c-rk3x.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> index 25ed1ad..0ba25ee 100644
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -126,6 +126,17 @@ static const struct i2c_spec_values fast_mode_spec =
> { .min_hold_buffer_ns = 1300,
>  };
> 
> +static const struct i2c_spec_values fast_mode_plus_spec = {
> +	.min_hold_start_ns = 260,
> +	.min_low_ns = 500,
> +	.min_high_ns = 260,
> +	.min_setup_start_ns = 260,
> +	.max_data_hold_ns = 400,
> +	.min_data_setup_ns = 50,
> +	.min_setup_stop_ns = 260,
> +	.min_hold_buffer_ns = 500,
> +};
> +
>  /**
>   * struct rk3x_i2c_calced_timings:
>   * @div_low: Divider output for low
> @@ -531,8 +542,10 @@ static const struct i2c_spec_values
> *rk3x_i2c_get_spec(unsigned int speed) {
>  	if (speed <= 100000)
>  		return &standard_mode_spec;
> -	else
> +	else if (speed <= 400000)
>  		return &fast_mode_spec;
> +	else
> +		return &fast_mode_plus_spec;
>  }
> 
>  /**
> @@ -743,9 +756,9 @@ static int rk3x_i2c_v1_calc_timings(unsigned long
> clk_rate, const struct i2c_spec_values *spec;
>  	int ret = 0;
> 
> -	/* Support standard-mode and fast-mode */
> -	if (WARN_ON(t->bus_freq_hz > 400000))
> -		t->bus_freq_hz = 400000;
> +	/* Support standard-mode, fast-mode and fast-mode plus */
> +	if (WARN_ON(t->bus_freq_hz > 1000000))
> +		t->bus_freq_hz = 1000000;
> 
>  	/* prevent scl_rate_khz from becoming 0 */
>  	if (WARN_ON(t->bus_freq_hz < 1000))

[toc] | [prev] | [next] | [standalone]

#1399609 — Re: [PATCH v8 8/8] i2c: rk3x: support fast-mode plus for rk3399

Heiko,

On Wed, May 11, 2016 at 2:09 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> Additionally, the i2c-noob in me would wish for a short sentence on where
> the timing values come from (measured, somewhere in the TRM I didn't find or
> something completely different).

If you search for "UM10204" you'll find the first link is
<http://www.nxp.com/documents/user_manual/UM10204.pdf>.  That appears
to be the official standard.  At least that's what
<http://www.i2c-bus.org/fast-mode-plus/> says (though they have an old
link).

Agree that it wouldn't hurt to mention that in the commit message.

-Doug

[toc] | [prev] | [next] | [standalone]

#1399751 — Re: [PATCH v8 8/8] i2c: rk3x: support fast-mode plus for rk3399

Am Mittwoch, 11. Mai 2016, 16:41:54 schrieb Doug Anderson:
> Heiko,
> 
> On Wed, May 11, 2016 at 2:09 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> > Additionally, the i2c-noob in me would wish for a short sentence on
> > where
> > the timing values come from (measured, somewhere in the TRM I didn't
> > find or something completely different).
> 
> If you search for "UM10204" you'll find the first link is
> <http://www.nxp.com/documents/user_manual/UM10204.pdf>.  That appears
> to be the official standard.  At least that's what
> <http://www.i2c-bus.org/fast-mode-plus/> says (though they have an old
> link).

I did find that document yesterday, but was to blind and somehow only saw the 
separate table for highspeed-timings. On a closer look, I now also found the 
timing-table for the other modes [due to the bigger font-size used there? 
;-) ]

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web