[PATCH v2 0/9] i2c: add generic support for timing parameters in DT

[PATCH v2 0/9] i2c: add generic support for timing parameters in DT

[Wolfram Sang]

Here is a patch series adding better DT support for timing parameters like the
raise time or the fall time which are generic for the I2C subsystem. There is a
core function for parsing and an implementation for the RCar driver how to use
it.

I added people of the designware and rk3x driver to CC because they might be
interested in this new function. They provided the basis for this series, so it
should be easy to convert them.

Please have a look, test, comment...

Thanks,

    Wolfram

Changes since V1:
* switched from DT properties to device properties
* that means no dependency on OF and rename of the parsing function
* removed a superfluous comment
* adapted the RCar driver to the changes
* added Rob's ack

Changes since RFC:
* better tested
* added documentation for the new function and the new struct
* reworded some commit messages slightly
* moved the new property in the dtsi above the "status" property

Wolfram Sang (9):
  i2c: document generic DT bindings for timing parameters
  i2c: add generic routine to parse DT for timing information
  i2c: rcar: refactor probe function a little
  i2c: rcar: switch to i2c generic dt parsing
  i2c: rcar: honor additional i2c timings from DT
  ARM: shmobile: r8a7790: dtsi: add internal delay for i2c IPs
  ARM: shmobile: r8a7791: dtsi: add internal delay for i2c IPs
  ARM: shmobile: r8a7794: dtsi: add internal delay for i2c IPs
  arm64: renesas: r8a7795: add internal delay for i2c IPs

 Documentation/devicetree/bindings/i2c/i2c-rcar.txt |  4 ++
 Documentation/devicetree/bindings/i2c/i2c.txt      | 31 ++++++++--
 arch/arm/boot/dts/r8a7790.dtsi                     |  4 ++
 arch/arm/boot/dts/r8a7791.dtsi                     |  6 ++
 arch/arm/boot/dts/r8a7794.dtsi                     |  6 ++
 arch/arm64/boot/dts/renesas/r8a7795.dtsi           |  7 +++
 drivers/i2c/busses/i2c-rcar.c                      | 67 +++++++++++-----------
 drivers/i2c/i2c-core.c                             | 47 +++++++++++++++
 include/linux/i2c.h                                | 18 ++++++
 9 files changed, 151 insertions(+), 39 deletions(-)

-- 
2.1.4

[PATCH v2 1/9] i2c: document generic DT bindings for timing parameters

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Also, sort the properties alphabetically and make indentation
consistent. Wording largely taken from i2c-rk3x.txt, thanks guys!

Only "i2c-scl-internal-delay-ns" is new, the rest is used by two drivers
already and was documented in their driver binding documentation.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/i2c/i2c.txt | 31 ++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index 8a99150ac3a7fd..a00219f5ee0733 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -29,12 +29,33 @@ Optional properties
 These properties may not be supported by all drivers. However, if a driver
 wants to support one of the below features, it should adapt the bindings below.
 
-- clock-frequency	- frequency of bus clock in Hz.
-- wakeup-source		- device can be used as a wakeup source.
+- clock-frequency
+	frequency of bus clock in Hz.
 
-- interrupts		- interrupts used by the device.
-- interrupt-names	- "irq" and "wakeup" names are recognized by I2C core,
-			  other names are left to individual drivers.
+- i2c-scl-falling-time-ns
+	Number of nanoseconds the SCL signal takes to fall; t(f) in the I2C
+	specification.
+
+- i2c-scl-internal-delay-ns
+	Number of nanoseconds the IP core additionally needs to setup SCL.
+
+- i2c-scl-rising-time-ns
+	Number of nanoseconds the SCL signal takes to rise; t(r) in the I2C
+	specification.
+
+- i2c-sda-falling-time-ns
+	Number of nanoseconds the SDA signal takes to fall; t(f) in the I2C
+	specification.
+
+- interrupts
+	interrupts used by the device.
+
+- interrupt-names
+	"irq" and "wakeup" names are recognized by I2C core, other names are
+	left to individual drivers.
+
+- wakeup-source
+	device can be used as a wakeup source.
 
 Binding may contain optional "interrupts" property, describing interrupts
 used by the device. I2C core will assign "irq" interrupt (or the very first
-- 
2.1.4

[PATCH v2 2/9] i2c: add generic routine to parse DT for timing information

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Inspired from the i2c-rk3x driver (thanks guys!) but refactored and
extended. See built-in docs for further information.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/i2c-core.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c.h    | 18 ++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index ba8eb087f22465..e94d2ca2aab4aa 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -53,6 +53,7 @@
 #include <linux/jump_label.h>
 #include <asm/uaccess.h>
 #include <linux/err.h>
+#include <linux/property.h>
 
 #include "i2c-core.h"
 
@@ -1438,6 +1439,52 @@ static void of_i2c_register_devices(struct i2c_adapter *adap)
 	}
 }
 
+/**
+ * i2c_parse_fw_timings - get I2C related timing parameters from firmware
+ * @dev: The device to scan for I2C timing properties
+ * @t: the i2c_timings struct to be filled with values
+ * @use_defaults: bool to use sane defaults derived from the I2C specification
+ * 		  when properties are not found, otherwise use 0
+ *
+ * Scan the device for the generic I2C properties describing timing parameters
+ * for the signal and fill the given struct with the results. If a property was
+ * not found and use_defaults was true, then maximum timings are assumed which
+ * are derived from the I2C specification. If use_defaults is not used, the
+ * results will be 0, so drivers can apply their own defaults later. The latter
+ * is mainly intended for avoiding regressions of existing drivers which want
+ * to switch to this function. New drivers almost always should use the defaults.
+ */
+
+void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool use_defaults)
+{
+	memset(t, 0, sizeof(*t));
+
+	if (device_property_read_u32(dev, "clock-frequency", &t->bus_freq_hz) && use_defaults)
+		t->bus_freq_hz = 100000;
+
+	if (device_property_read_u32(dev, "i2c-scl-rising-time-ns", &t->scl_rise_ns) && use_defaults) {
+		if (t->bus_freq_hz <= 100000)
+			t->scl_rise_ns = 1000;
+		else if (t->bus_freq_hz <= 400000)
+			t->scl_rise_ns = 300;
+		else
+			t->scl_rise_ns = 120;
+	}
+
+	if (device_property_read_u32(dev, "i2c-scl-falling-time-ns", &t->scl_fall_ns) && use_defaults) {
+		if (t->bus_freq_hz <= 400000)
+			t->scl_fall_ns = 300;
+		else
+			t->scl_fall_ns = 120;
+	}
+
+	device_property_read_u32(dev, "i2c-scl-internal-delay-ns", &t->scl_int_delay_ns);
+
+	if (device_property_read_u32(dev, "i2c-sda-falling-time-ns", &t->sda_fall_ns) && use_defaults)
+		t->sda_fall_ns = t->scl_fall_ns;
+}
+EXPORT_SYMBOL_GPL(i2c_parse_fw_timings);
+
 static int of_dev_node_match(struct device *dev, void *data)
 {
 	return dev->of_node == data;
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 768063baafbf5e..7c45181f41ab7d 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -414,6 +414,22 @@ struct i2c_algorithm {
 };
 
 /**
+ * struct i2c_timings - I2C timing information
+ * @bus_freq_hz: the bus frequency in Hz
+ * @scl_rise_ns: time SCL signal takes to rise in ns; t(r) in the I2C specification
+ * @scl_fall_ns: time SCL signal takes to fall in ns; t(f) in the I2C specification
+ * @scl_int_delay_ns: time IP core additionally needs to setup SCL in ns
+ * @sda_fall_ns: time SDA signal takes to fall in ns; t(f) in the I2C specification
+ */
+struct i2c_timings {
+	u32 bus_freq_hz;
+	u32 scl_rise_ns;
+	u32 scl_fall_ns;
+	u32 scl_int_delay_ns;
+	u32 sda_fall_ns;
+};
+
+/**
  * struct i2c_bus_recovery_info - I2C bus recovery information
  * @recover_bus: Recover routine. Either pass driver's recover_bus() routine, or
  *	i2c_generic_scl_recovery() or i2c_generic_gpio_recovery().
@@ -602,6 +618,7 @@ extern void i2c_clients_command(struct i2c_adapter *adap,
 extern struct i2c_adapter *i2c_get_adapter(int nr);
 extern void i2c_put_adapter(struct i2c_adapter *adap);
 
+void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool use_defaults);
 
 /* Return the functionality mask */
 static inline u32 i2c_get_functionality(struct i2c_adapter *adap)
@@ -644,6 +661,7 @@ extern struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
 
 /* must call i2c_put_adapter() when done with returned i2c_adapter device */
 struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node);
+
 #else
 
 static inline struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
-- 
2.1.4

Re: [PATCH v2 2/9] i2c: add generic routine to parse DT for timing information

[Andy Shevchenko]

On Tue, 2015-12-08 at 10:37 +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Inspired from the i2c-rk3x driver (thanks guys!) but refactored and
> extended. See built-in docs for further information.

One style comment.

> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/i2c/i2c-core.c | 47
> +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/i2c.h    | 18 ++++++++++++++++++
>  2 files changed, 65 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index ba8eb087f22465..e94d2ca2aab4aa 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -53,6 +53,7 @@
>  #include <linux/jump_label.h>
>  #include <asm/uaccess.h>
>  #include <linux/err.h>
> +#include <linux/property.h>
>  
>  #include "i2c-core.h"
>  
> @@ -1438,6 +1439,52 @@ static void of_i2c_register_devices(struct
> i2c_adapter *adap)
>  	}
>  }
>  
> +/**
> + * i2c_parse_fw_timings - get I2C related timing parameters from
> firmware
> + * @dev: The device to scan for I2C timing properties
> + * @t: the i2c_timings struct to be filled with values
> + * @use_defaults: bool to use sane defaults derived from the I2C
> specification
> + * 		  when properties are not found, otherwise use 0
> + *
> + * Scan the device for the generic I2C properties describing timing
> parameters
> + * for the signal and fill the given struct with the results. If a
> property was
> + * not found and use_defaults was true, then maximum timings are
> assumed which
> + * are derived from the I2C specification. If use_defaults is not
> used, the
> + * results will be 0, so drivers can apply their own defaults later.
> The latter
> + * is mainly intended for avoiding regressions of existing drivers
> which want
> + * to switch to this function. New drivers almost always should use
> the defaults.
> + */
> +
> +void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t,
> bool use_defaults)
> +{
> +	memset(t, 0, sizeof(*t));
> +
> +	if (device_property_read_u32(dev, "clock-frequency", &t-
> >bus_freq_hz) && use_defaults)
> +		t->bus_freq_hz = 100000;
> +
> +	if (device_property_read_u32(dev, "i2c-scl-rising-time-ns",
> &t->scl_rise_ns) && use_defaults) {
> +		if (t->bus_freq_hz <= 100000)
> +			t->scl_rise_ns = 1000;
> +		else if (t->bus_freq_hz <= 400000)
> +			t->scl_rise_ns = 300;
> +		else
> +			t->scl_rise_ns = 120;
> +	}
> +
> +	if (device_property_read_u32(dev, "i2c-scl-falling-time-ns", 
> &t->scl_fall_ns) && use_defaults) {
> +		if (t->bus_freq_hz <= 400000)
> +			t->scl_fall_ns = 300;
> +		else
> +			t->scl_fall_ns = 120;
> +	}
> +
> +	device_property_read_u32(dev, "i2c-scl-internal-delay-ns",
> &t->scl_int_delay_ns);
> +
> +	if (device_property_read_u32(dev, "i2c-sda-falling-time-ns", 
> &t->sda_fall_ns) && use_defaults)
> +		t->sda_fall_ns = t->scl_fall_ns;

Too many && use_defaults. What about

memset(t, 0, sizeof(*t));

device_property_read_u32(dev, "i2c-scl-internal-delay-ns", &t-
>scl_int_delay_ns);

if (!use_defaults)
 return;

...


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 2/9] i2c: add generic routine to parse DT for timing information

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 247 bytes --]


> Too many && use_defaults. What about
> 
> memset(t, 0, sizeof(*t));
> 
> device_property_read_u32(dev, "i2c-scl-internal-delay-ns", &t-
> >scl_int_delay_ns);
> 
> if (!use_defaults)
>  return;

I like this! Thanks for the input.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/9] i2c: add generic routine to parse DT for timing information

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 478 bytes --]

On Tue, Dec 08, 2015 at 02:03:23PM +0100, Wolfram Sang wrote:
> 
> > Too many && use_defaults. What about
> > 
> > memset(t, 0, sizeof(*t));
> > 
> > device_property_read_u32(dev, "i2c-scl-internal-delay-ns", &t-
> > >scl_int_delay_ns);
> > 
> > if (!use_defaults)
> >  return;
> 
> I like this! Thanks for the input.

Oops, too enthusiastic. This skips parsing all the other properties...
The defaults should only be applied if reading the properties fails.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/9] i2c: add generic routine to parse DT for timing information

[Andy Shevchenko]

On Tue, 2015-12-08 at 22:51 +0100, Wolfram Sang wrote:
> On Tue, Dec 08, 2015 at 02:03:23PM +0100, Wolfram Sang wrote:
> > 
> > > Too many && use_defaults. What about
> > > 
> > > memset(t, 0, sizeof(*t));
> > > 
> > > device_property_read_u32(dev, "i2c-scl-internal-delay-ns", &t-
> > > > scl_int_delay_ns);
> > > 
> > > if (!use_defaults)
> > >  return;
> > 
> > I like this! Thanks for the input.
> 
> Oops, too enthusiastic. This skips parsing all the other
> properties...
> The defaults should only be applied if reading the properties fails.

Looks like property stuff is not destructive in case of error, so, what
about

paramX = use_defaults ? defaultX : 0;
device_property_read…(…, &paramX);

Or maybe just be a bit verbose like the original variant
int ret;

ret = device_property_read…(…);
if (ret && use_defaults) {
…
}

Among all variants I like the latter one here. What do you think?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 2/9] i2c: add generic routine to parse DT for timing information

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 267 bytes --]

> Or maybe just be a bit verbose like the original variant
> int ret;
> 
> ret = device_property_read…(…);
> if (ret && use_defaults) {
> …
> }
> 
> Among all variants I like the latter one here. What do you think?

I agree and fixed it like this.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/9] i2c: add generic routine to parse DT for timing information

[Mika Westerberg]

On Tue, Dec 08, 2015 at 10:37:46AM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Inspired from the i2c-rk3x driver (thanks guys!) but refactored and
> extended. See built-in docs for further information.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Looks good. I think we can take advantage of this in the designware
driver as well.

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

I wonder if it makes sense to add "i2c-sda-hold-time-ns" (taken from the
designware driver DT binding) to the timings structure? It is tHD;DAT
parameter in the I2C bus specification.

Re: [PATCH v2 2/9] i2c: add generic routine to parse DT for timing information

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 445 bytes --]


> I wonder if it makes sense to add "i2c-sda-hold-time-ns" (taken from the
> designware driver DT binding) to the timings structure? It is tHD;DAT
> parameter in the I2C bus specification.

It totally makes sense. I just didn't need it for the RCar driver and
didn't want to implement something which isn't actually used. So feel
free to add it, or ask me to do it, if you promise to use it in the
designware driver :)

Thanks for the review!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/9] i2c: add generic routine to parse DT for timing information

[Mika Westerberg]

On Tue, Dec 08, 2015 at 01:53:07PM +0100, Wolfram Sang wrote:
> 
> > I wonder if it makes sense to add "i2c-sda-hold-time-ns" (taken from the
> > designware driver DT binding) to the timings structure? It is tHD;DAT
> > parameter in the I2C bus specification.
> 
> It totally makes sense. I just didn't need it for the RCar driver and
> didn't want to implement something which isn't actually used. So feel
> free to add it, or ask me to do it, if you promise to use it in the
> designware driver :)

I can add it in a separate patch later on. Thanks!

[PATCH v2 3/9] i2c: rcar: refactor probe function a little

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The probe function is a little bit messy, something here, something
there. Rework it so that there is some order: first init the private
structure, then the adapter, then do HW init. This also allows us to
remove the device argument of the clock calculation function, because it
now can be deduced from the private structure. Also, shorten some lines
where possible. This is a preparation for further refactoring.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 40 +++++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 23 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 3ed1f0aa5eeb16..d4322a9096786f 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -162,15 +162,11 @@ static int rcar_i2c_bus_barrier(struct rcar_i2c_priv *priv)
 	return -EBUSY;
 }
 
-static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv,
-				    u32 bus_speed,
-				    struct device *dev)
+static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv, u32 bus_speed)
 {
-	u32 scgd, cdf;
-	u32 round, ick;
-	u32 scl;
-	u32 cdf_width;
+	u32 scgd, cdf, round, ick, scl, cdf_width;
 	unsigned long rate;
+	struct device *dev = rcar_i2c_priv_to_dev(priv);
 
 	switch (priv->devtype) {
 	case I2C_RCAR_GEN1:
@@ -610,21 +606,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->io))
 		return PTR_ERR(priv->io);
 
-	bus_speed = 100000; /* default 100 kHz */
-	of_property_read_u32(dev->of_node, "clock-frequency", &bus_speed);
-
 	priv->devtype = (enum rcar_i2c_type)of_match_device(rcar_i2c_dt_ids, dev)->data;
-
-	pm_runtime_enable(dev);
-	pm_runtime_get_sync(dev);
-	ret = rcar_i2c_clock_calculate(priv, bus_speed, dev);
-	if (ret < 0)
-		goto out_pm_put;
-
-	rcar_i2c_init(priv);
-	pm_runtime_put(dev);
-
-	irq = platform_get_irq(pdev, 0);
 	init_waitqueue_head(&priv->wait);
 
 	adap = &priv->adap;
@@ -637,8 +619,20 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 	i2c_set_adapdata(adap, priv);
 	strlcpy(adap->name, pdev->name, sizeof(adap->name));
 
-	ret = devm_request_irq(dev, irq, rcar_i2c_irq, 0,
-			       dev_name(dev), priv);
+	bus_speed = 100000; /* default 100 kHz */
+	of_property_read_u32(dev->of_node, "clock-frequency", &bus_speed);
+
+	pm_runtime_enable(dev);
+	pm_runtime_get_sync(dev);
+	ret = rcar_i2c_clock_calculate(priv, bus_speed);
+	if (ret < 0)
+		goto out_pm_put;
+
+	rcar_i2c_init(priv);
+	pm_runtime_put(dev);
+
+	irq = platform_get_irq(pdev, 0);
+	ret = devm_request_irq(dev, irq, rcar_i2c_irq, 0, dev_name(dev), priv);
 	if (ret < 0) {
 		dev_err(dev, "cannot get irq %d\n", irq);
 		goto out_pm_disable;
-- 
2.1.4

[PATCH v2 4/9] i2c: rcar: switch to i2c generic dt parsing

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Switch to the new generic functions. Plain convert, no functionality
added yet.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index d4322a9096786f..c663f4389bf898 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -162,12 +162,15 @@ static int rcar_i2c_bus_barrier(struct rcar_i2c_priv *priv)
 	return -EBUSY;
 }
 
-static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv, u32 bus_speed)
+static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv, struct i2c_timings *t)
 {
 	u32 scgd, cdf, round, ick, scl, cdf_width;
 	unsigned long rate;
 	struct device *dev = rcar_i2c_priv_to_dev(priv);
 
+	/* Fall back to previously used values if not supplied */
+	t->bus_freq_hz = t->bus_freq_hz ?: 100000;
+
 	switch (priv->devtype) {
 	case I2C_RCAR_GEN1:
 		cdf_width = 2;
@@ -230,7 +233,7 @@ static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv, u32 bus_speed)
 	 */
 	for (scgd = 0; scgd < 0x40; scgd++) {
 		scl = ick / (20 + (scgd * 8) + round);
-		if (scl <= bus_speed)
+		if (scl <= t->bus_freq_hz)
 			goto scgd_find;
 	}
 	dev_err(dev, "it is impossible to calculate best SCL\n");
@@ -238,7 +241,7 @@ static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv, u32 bus_speed)
 
 scgd_find:
 	dev_dbg(dev, "clk %d/%d(%lu), round %u, CDF:0x%x, SCGD: 0x%x\n",
-		scl, bus_speed, clk_get_rate(priv->clk), round, cdf, scgd);
+		scl, t->bus_freq_hz, clk_get_rate(priv->clk), round, cdf, scgd);
 
 	/* keep icccr value */
 	priv->icccr = scgd << cdf_width | cdf;
@@ -588,7 +591,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 	struct i2c_adapter *adap;
 	struct resource *res;
 	struct device *dev = &pdev->dev;
-	u32 bus_speed;
+	struct i2c_timings i2c_t;
 	int irq, ret;
 
 	priv = devm_kzalloc(dev, sizeof(struct rcar_i2c_priv), GFP_KERNEL);
@@ -619,12 +622,11 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 	i2c_set_adapdata(adap, priv);
 	strlcpy(adap->name, pdev->name, sizeof(adap->name));
 
-	bus_speed = 100000; /* default 100 kHz */
-	of_property_read_u32(dev->of_node, "clock-frequency", &bus_speed);
+	i2c_parse_fw_timings(dev, &i2c_t, false);
 
 	pm_runtime_enable(dev);
 	pm_runtime_get_sync(dev);
-	ret = rcar_i2c_clock_calculate(priv, bus_speed);
+	ret = rcar_i2c_clock_calculate(priv, &i2c_t);
 	if (ret < 0)
 		goto out_pm_put;
 
-- 
2.1.4

Re: [PATCH v2 4/9] i2c: rcar: switch to i2c generic dt parsing

[Andy Shevchenko]

On Tue, 2015-12-08 at 10:37 +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Switch to the new generic functions. Plain convert, no functionality
> added yet.

One style nitpick.

> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/i2c/busses/i2c-rcar.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-
> rcar.c
> index d4322a9096786f..c663f4389bf898 100644
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
> @@ -162,12 +162,15 @@ static int rcar_i2c_bus_barrier(struct
> rcar_i2c_priv *priv)
>  	return -EBUSY;
>  }
>  
> -static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv, u32
> bus_speed)
> +static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv,
> struct i2c_timings *t)
>  {
>  	u32 scgd, cdf, round, ick, scl, cdf_width;
>  	unsigned long rate;
>  	struct device *dev = rcar_i2c_priv_to_dev(priv);
>  
> +	/* Fall back to previously used values if not supplied */
> +	t->bus_freq_hz = t->bus_freq_hz ?: 100000;

On one hand it seems enough space to put one more t->bus_freq_hz, on
the other why not

if (!t->bus_freq_hz)
  = 100000;

I think a bit better to maintain latter.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 4/9] i2c: rcar: switch to i2c generic dt parsing

[Shevchenko, Andriy]

On Tue, 2015-12-08 at 12:59 +0200, Andy Shevchenko wrote:
> On Tue, 2015-12-08 at 10:37 +0100, Wolfram Sang wrote:
> > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > 
> > Switch to the new generic functions. Plain convert, no
> > functionality
> > added yet.
> 
> One style nitpick.

Ah, just seen next patch. Yeah, current make sense.
Never mind below comment.

> 
> > 
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> >  drivers/i2c/busses/i2c-rcar.c | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-rcar.c
> > b/drivers/i2c/busses/i2c-
> > rcar.c
> > index d4322a9096786f..c663f4389bf898 100644
> > --- a/drivers/i2c/busses/i2c-rcar.c
> > +++ b/drivers/i2c/busses/i2c-rcar.c
> > @@ -162,12 +162,15 @@ static int rcar_i2c_bus_barrier(struct
> > rcar_i2c_priv *priv)
> >  	return -EBUSY;
> >  }
> >  
> > -static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv,
> > u32
> > bus_speed)
> > +static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv,
> > struct i2c_timings *t)
> >  {
> >  	u32 scgd, cdf, round, ick, scl, cdf_width;
> >  	unsigned long rate;
> >  	struct device *dev = rcar_i2c_priv_to_dev(priv);
> >  
> > +	/* Fall back to previously used values if not supplied */
> > +	t->bus_freq_hz = t->bus_freq_hz ?: 100000;
> 
> On one hand it seems enough space to put one more t->bus_freq_hz, on
> the other why not
> 
> if (!t->bus_freq_hz)
>   = 100000;
> 
> I think a bit better to maintain latter.
> 

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[PATCH v2 5/9] i2c: rcar: honor additional i2c timings from DT

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 Documentation/devicetree/bindings/i2c/i2c-rcar.txt |  4 ++++
 drivers/i2c/busses/i2c-rcar.c                      | 21 ++++++++++++---------
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
index ea406eb20fa5ad..95e97223a71c83 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
@@ -20,6 +20,10 @@ Optional properties:
   propoerty indicates the default frequency 100 kHz.
 - clocks: clock specifier.
 
+- i2c-scl-falling-time-ns: see i2c.txt
+- i2c-scl-internal-delay-ns: see i2c.txt
+- i2c-scl-rising-time-ns: see i2c.txt
+
 Examples :
 
 i2c0: i2c@e6508000 {
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index c663f4389bf898..b2389c492579cf 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -164,12 +164,15 @@ static int rcar_i2c_bus_barrier(struct rcar_i2c_priv *priv)
 
 static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv, struct i2c_timings *t)
 {
-	u32 scgd, cdf, round, ick, scl, cdf_width;
+	u32 scgd, cdf, round, ick, sum, scl, cdf_width;
 	unsigned long rate;
 	struct device *dev = rcar_i2c_priv_to_dev(priv);
 
 	/* Fall back to previously used values if not supplied */
 	t->bus_freq_hz = t->bus_freq_hz ?: 100000;
+	t->scl_fall_ns = t->scl_fall_ns ?: 35;
+	t->scl_rise_ns = t->scl_rise_ns ?: 200;
+	t->scl_int_delay_ns = t->scl_int_delay_ns ?: 50;
 
 	switch (priv->devtype) {
 	case I2C_RCAR_GEN1:
@@ -193,9 +196,9 @@ static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv, struct i2c_timin
 	 * SCL	= ick / (20 + SCGD * 8 + F[(ticf + tr + intd) * ick])
 	 *
 	 * ick  : I2C internal clock < 20 MHz
-	 * ticf : I2C SCL falling time  =  35 ns here
-	 * tr   : I2C SCL rising  time  = 200 ns here
-	 * intd : LSI internal delay    =  50 ns here
+	 * ticf : I2C SCL falling time
+	 * tr   : I2C SCL rising  time
+	 * intd : LSI internal delay
 	 * clkp : peripheral_clk
 	 * F[]  : integer up-valuation
 	 */
@@ -211,12 +214,12 @@ static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv, struct i2c_timin
 	 * it is impossible to calculate large scale
 	 * number on u32. separate it
 	 *
-	 * F[(ticf + tr + intd) * ick]
-	 *  = F[(35 + 200 + 50)ns * ick]
-	 *  = F[285 * ick / 1000000000]
-	 *  = F[(ick / 1000000) * 285 / 1000]
+	 * F[(ticf + tr + intd) * ick] with sum = (ticf + tr + intd)
+	 *  = F[sum * ick / 1000000000]
+	 *  = F[(ick / 1000000) * sum / 1000]
 	 */
-	round = (ick + 500000) / 1000000 * 285;
+	sum = t->scl_fall_ns + t->scl_rise_ns + t->scl_int_delay_ns;
+	round = (ick + 500000) / 1000000 * sum;
 	round = (round + 500) / 1000;
 
 	/*
-- 
2.1.4

[PATCH v2 6/9] ARM: shmobile: r8a7790: dtsi: add internal delay for i2c IPs

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm/boot/dts/r8a7790.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 6cfd0dc79bbec0..64a3f4a05f36d0 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -417,6 +417,7 @@
 		interrupts = <0 287 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7790_CLK_I2C0>;
 		power-domains = <&cpg_clocks>;
+		i2c-scl-internal-delay-ns = <110>;
 		status = "disabled";
 	};
 
@@ -428,6 +429,7 @@
 		interrupts = <0 288 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7790_CLK_I2C1>;
 		power-domains = <&cpg_clocks>;
+		i2c-scl-internal-delay-ns = <6>;
 		status = "disabled";
 	};
 
@@ -439,6 +441,7 @@
 		interrupts = <0 286 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7790_CLK_I2C2>;
 		power-domains = <&cpg_clocks>;
+		i2c-scl-internal-delay-ns = <6>;
 		status = "disabled";
 	};
 
@@ -450,6 +453,7 @@
 		interrupts = <0 290 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7790_CLK_I2C3>;
 		power-domains = <&cpg_clocks>;
+		i2c-scl-internal-delay-ns = <110>;
 		status = "disabled";
 	};
 
-- 
2.1.4

[PATCH v2 7/9] ARM: shmobile: r8a7791: dtsi: add internal delay for i2c IPs

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm/boot/dts/r8a7791.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 76b33e895513c1..3161dfcdd002dd 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -407,6 +407,7 @@
 		interrupts = <0 287 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7791_CLK_I2C0>;
 		power-domains = <&cpg_clocks>;
+		i2c-scl-internal-delay-ns = <6>;
 		status = "disabled";
 	};
 
@@ -418,6 +419,7 @@
 		interrupts = <0 288 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7791_CLK_I2C1>;
 		power-domains = <&cpg_clocks>;
+		i2c-scl-internal-delay-ns = <6>;
 		status = "disabled";
 	};
 
@@ -429,6 +431,7 @@
 		interrupts = <0 286 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7791_CLK_I2C2>;
 		power-domains = <&cpg_clocks>;
+		i2c-scl-internal-delay-ns = <6>;
 		status = "disabled";
 	};
 
@@ -440,6 +443,7 @@
 		interrupts = <0 290 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7791_CLK_I2C3>;
 		power-domains = <&cpg_clocks>;
+		i2c-scl-internal-delay-ns = <6>;
 		status = "disabled";
 	};
 
@@ -451,6 +455,7 @@
 		interrupts = <0 19 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7791_CLK_I2C4>;
 		power-domains = <&cpg_clocks>;
+		i2c-scl-internal-delay-ns = <6>;
 		status = "disabled";
 	};
 
@@ -463,6 +468,7 @@
 		interrupts = <0 20 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7791_CLK_I2C5>;
 		power-domains = <&cpg_clocks>;
+		i2c-scl-internal-delay-ns = <110>;
 		status = "disabled";
 	};
 
-- 
2.1.4

[PATCH v2 8/9] ARM: shmobile: r8a7794: dtsi: add internal delay for i2c IPs

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm/boot/dts/r8a7794.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7794.dtsi b/arch/arm/boot/dts/r8a7794.dtsi
index 6f2f01914db295..37809b8aae65f9 100644
--- a/arch/arm/boot/dts/r8a7794.dtsi
+++ b/arch/arm/boot/dts/r8a7794.dtsi
@@ -519,6 +519,7 @@
 		power-domains = <&cpg_clocks>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+		i2c-scl-internal-delay-ns = <6>;
 		status = "disabled";
 	};
 
@@ -530,6 +531,7 @@
 		power-domains = <&cpg_clocks>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+		i2c-scl-internal-delay-ns = <6>;
 		status = "disabled";
 	};
 
@@ -541,6 +543,7 @@
 		power-domains = <&cpg_clocks>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+		i2c-scl-internal-delay-ns = <6>;
 		status = "disabled";
 	};
 
@@ -552,6 +555,7 @@
 		power-domains = <&cpg_clocks>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+		i2c-scl-internal-delay-ns = <6>;
 		status = "disabled";
 	};
 
@@ -563,6 +567,7 @@
 		power-domains = <&cpg_clocks>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+		i2c-scl-internal-delay-ns = <6>;
 		status = "disabled";
 	};
 
@@ -574,6 +579,7 @@
 		power-domains = <&cpg_clocks>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+		i2c-scl-internal-delay-ns = <6>;
 		status = "disabled";
 	};
 
-- 
2.1.4

[PATCH v2 9/9] arm64: renesas: r8a7795: add internal delay for i2c IPs

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index 56d6a5b23a4e38..b02a7ad1583933 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -519,6 +519,7 @@
 			interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 931>;
 			power-domains = <&cpg>;
+			i2c-scl-internal-delay-ns = <110>;
 			status = "disabled";
 		};
 
@@ -530,6 +531,7 @@
 			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 930>;
 			power-domains = <&cpg>;
+			i2c-scl-internal-delay-ns = <6>;
 			status = "disabled";
 		};
 
@@ -541,6 +543,7 @@
 			interrupts = <GIC_SPI 286 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 929>;
 			power-domains = <&cpg>;
+			i2c-scl-internal-delay-ns = <6>;
 			status = "disabled";
 		};
 
@@ -552,6 +555,7 @@
 			interrupts = <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 928>;
 			power-domains = <&cpg>;
+			i2c-scl-internal-delay-ns = <110>;
 			status = "disabled";
 		};
 
@@ -563,6 +567,7 @@
 			interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 927>;
 			power-domains = <&cpg>;
+			i2c-scl-internal-delay-ns = <110>;
 			status = "disabled";
 		};
 
@@ -574,6 +579,7 @@
 			interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 919>;
 			power-domains = <&cpg>;
+			i2c-scl-internal-delay-ns = <110>;
 			status = "disabled";
 		};
 
@@ -585,6 +591,7 @@
 			interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 918>;
 			power-domains = <&cpg>;
+			i2c-scl-internal-delay-ns = <6>;
 			status = "disabled";
 		};
 
-- 
2.1.4

Re: [PATCH v2 0/9] i2c: add generic support for timing parameters in DT

[Simon Horman]

On Tue, Dec 08, 2015 at 10:37:44AM +0100, Wolfram Sang wrote:
> Here is a patch series adding better DT support for timing parameters like the
> raise time or the fall time which are generic for the I2C subsystem. There is a
> core function for parsing and an implementation for the RCar driver how to use
> it.
> 
> I added people of the designware and rk3x driver to CC because they might be
> interested in this new function. They provided the basis for this series, so it
> should be easy to convert them.
> 
> Please have a look, test, comment...
> 
> Thanks,
> 
>     Wolfram
> 
> Changes since V1:
> * switched from DT properties to device properties
> * that means no dependency on OF and rename of the parsing function
> * removed a superfluous comment
> * adapted the RCar driver to the changes
> * added Rob's ack
> 
> Changes since RFC:
> * better tested
> * added documentation for the new function and the new struct
> * reworded some commit messages slightly
> * moved the new property in the dtsi above the "status" property
> 
> Wolfram Sang (9):
>   i2c: document generic DT bindings for timing parameters
>   i2c: add generic routine to parse DT for timing information
>   i2c: rcar: refactor probe function a little
>   i2c: rcar: switch to i2c generic dt parsing
>   i2c: rcar: honor additional i2c timings from DT
>   ARM: shmobile: r8a7790: dtsi: add internal delay for i2c IPs
>   ARM: shmobile: r8a7791: dtsi: add internal delay for i2c IPs
>   ARM: shmobile: r8a7794: dtsi: add internal delay for i2c IPs
>   arm64: renesas: r8a7795: add internal delay for i2c IPs

I have marked the ARM and arm64 patches as deferred pending
acceptance of the driver patches.

Re: [PATCH v2 0/9] i2c: add generic support for timing parameters in DT

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

On Tue, Dec 08, 2015 at 10:37:44AM +0100, Wolfram Sang wrote:
> Here is a patch series adding better DT support for timing parameters like the
> raise time or the fall time which are generic for the I2C subsystem. There is a
> core function for parsing and an implementation for the RCar driver how to use
> it.
> 
> I added people of the designware and rk3x driver to CC because they might be
> interested in this new function. They provided the basis for this series, so it
> should be easy to convert them.
> 
> Please have a look, test, comment...
> 
> Thanks,
> 
>     Wolfram

Applied patches 1-5 to for-next with the change Andy suggested, thanks!

Simon, you can pick up patches 6-9 now, I'd think.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 0/9] i2c: add generic support for timing parameters in DT

[Simon Horman]

On Mon, Dec 14, 2015 at 11:00:26AM +0100, Wolfram Sang wrote:
> On Tue, Dec 08, 2015 at 10:37:44AM +0100, Wolfram Sang wrote:
> > Here is a patch series adding better DT support for timing parameters like the
> > raise time or the fall time which are generic for the I2C subsystem. There is a
> > core function for parsing and an implementation for the RCar driver how to use
> > it.
> > 
> > I added people of the designware and rk3x driver to CC because they might be
> > interested in this new function. They provided the basis for this series, so it
> > should be easy to convert them.
> > 
> > Please have a look, test, comment...
> > 
> > Thanks,
> > 
> >     Wolfram
> 
> Applied patches 1-5 to for-next with the change Andy suggested, thanks!
> 
> Simon, you can pick up patches 6-9 now, I'd think.

Thanks for letting me know, I have done so.