[PATCH v2 1/4] iio: magnetometer: add clarifying comment

[PATCH v2 1/4] iio: magnetometer: add clarifying comment

[Martin Kelly]

From: Martin Kelly <martin@martingkelly.com>

Add a comment clarifying better when to use <device_name> vs
<device_name>-magn in compatibility strings.

Signed-off-by: Martin Kelly <martin@martingkelly.com>
---
 drivers/iio/magnetometer/st_magn_spi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
index 7b7cd08fcc32..15bb09267098 100644
--- a/drivers/iio/magnetometer/st_magn_spi.c
+++ b/drivers/iio/magnetometer/st_magn_spi.c
@@ -23,6 +23,8 @@
  * For new single-chip sensors use <device_name> as compatible string.
  * For old single-chip devices keep <device_name>-magn to maintain
  * compatibility
+ * For multi-chip devices, use <device_name>-magn to distinguish which
+ * capability is being used
  */
 static const struct of_device_id st_magn_of_match[] = {
 	{
-- 
2.11.0

[PATCH v2 2/4] iio:magnetometer: st_magn: add LSM9DS1 support

[Martin Kelly]

From: Martin Kelly <martin@martingkelly.com>

Update the sensor settings to support the LSM9DS1 sensor. Although the
LSM9DS1 accelerometer and gyroscope are coupled together to use the same
FIFO, the magnetometer is separate and can be cleanly supported without
refactoring the existing driver.

Signed-off-by: Martin Kelly <martin@martingkelly.com>
---
v2:
- Rename lsm9ds1 --> lsm9ds1-magn and lsm9ds1_magn, as this is a multi-device
  chip.
- Use the register layout of LIS3MDL, as the two chips match.
- Add I2C compatibility strings.

 drivers/iio/magnetometer/st_magn.h      | 1 +
 drivers/iio/magnetometer/st_magn_core.c | 1 +
 drivers/iio/magnetometer/st_magn_i2c.c  | 5 +++++
 drivers/iio/magnetometer/st_magn_spi.c  | 5 +++++
 4 files changed, 12 insertions(+)

diff --git a/drivers/iio/magnetometer/st_magn.h b/drivers/iio/magnetometer/st_magn.h
index 8fe51ce427bd..bc14ad4f1b26 100644
--- a/drivers/iio/magnetometer/st_magn.h
+++ b/drivers/iio/magnetometer/st_magn.h
@@ -20,6 +20,7 @@
 #define LIS3MDL_MAGN_DEV_NAME		"lis3mdl"
 #define LSM303AGR_MAGN_DEV_NAME		"lsm303agr_magn"
 #define LIS2MDL_MAGN_DEV_NAME		"lis2mdl"
+#define LSM9DS1_MAGN_DEV_NAME		"lsm9ds1_magn"

 int st_magn_common_probe(struct iio_dev *indio_dev);
 void st_magn_common_remove(struct iio_dev *indio_dev);
diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
index 72f6d1335a04..a186b844d664 100644
--- a/drivers/iio/magnetometer/st_magn_core.c
+++ b/drivers/iio/magnetometer/st_magn_core.c
@@ -267,6 +267,7 @@ static const struct st_sensor_settings st_magn_sensors_settings[] = {
 		.wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS,
 		.sensors_supported = {
 			[0] = LIS3MDL_MAGN_DEV_NAME,
+			[1] = LSM9DS1_MAGN_DEV_NAME,
 		},
 		.ch = (struct iio_chan_spec *)st_magn_2_16bit_channels,
 		.odr = {
diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c
index feaa28cf6a77..68650f5f5c19 100644
--- a/drivers/iio/magnetometer/st_magn_i2c.c
+++ b/drivers/iio/magnetometer/st_magn_i2c.c
@@ -44,6 +44,10 @@ static const struct of_device_id st_magn_of_match[] = {
 		.compatible = "st,lis2mdl",
 		.data = LIS2MDL_MAGN_DEV_NAME,
 	},
+	{
+		.compatible = "st,lsm9ds1-magn",
+		.data = LSM9DS1_MAGN_DEV_NAME,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, st_magn_of_match);
@@ -90,6 +94,7 @@ static const struct i2c_device_id st_magn_id_table[] = {
 	{ LIS3MDL_MAGN_DEV_NAME },
 	{ LSM303AGR_MAGN_DEV_NAME },
 	{ LIS2MDL_MAGN_DEV_NAME },
+	{ LSM9DS1_MAGN_DEV_NAME },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, st_magn_id_table);
diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
index 15bb09267098..cb05fcd9ddfe 100644
--- a/drivers/iio/magnetometer/st_magn_spi.c
+++ b/drivers/iio/magnetometer/st_magn_spi.c
@@ -39,6 +39,10 @@ static const struct of_device_id st_magn_of_match[] = {
 		.compatible = "st,lis2mdl",
 		.data = LIS2MDL_MAGN_DEV_NAME,
 	},
+	{
+		.compatible = "st,lsm9ds1-magn",
+		.data = LSM9DS1_MAGN_DEV_NAME,
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, st_magn_of_match);
@@ -81,6 +85,7 @@ static const struct spi_device_id st_magn_id_table[] = {
 	{ LIS3MDL_MAGN_DEV_NAME },
 	{ LSM303AGR_MAGN_DEV_NAME },
 	{ LIS2MDL_MAGN_DEV_NAME },
+	{ LSM9DS1_MAGN_DEV_NAME },
 	{},
 };
 MODULE_DEVICE_TABLE(spi, st_magn_id_table);
--
2.11.0

[PATCH v2 3/4] iio:magnetometer: st_magn: add BDU settings

[Martin Kelly]

From: Martin Kelly <martin@martingkelly.com>

LIS3MDL and LSM9DS1 are missing BDU settings in their register maps, so add
them. I don't have a LIS3MDL sensor to test, but this works correctly on
the LSM9DS1, which has the same register map.

Signed-off-by: Martin Kelly <martin@martingkelly.com>
---
 drivers/iio/magnetometer/st_magn_core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
index a186b844d664..370b93db528d 100644
--- a/drivers/iio/magnetometer/st_magn_core.c
+++ b/drivers/iio/magnetometer/st_magn_core.c
@@ -316,6 +316,10 @@ static const struct st_sensor_settings st_magn_sensors_settings[] = {
 				},
 			},
 		},
+		.bdu = {
+			.addr = 0x24,
+			.mask = 0x40,
+		},
 		.drdy_irq = {
 			/* drdy line is routed drdy pin */
 			.stat_drdy = {
-- 
2.11.0

[PATCH v2 4/4] dt-bindings: iio: magn: add LSM9DS1 bindings

[Martin Kelly]

From: Martin Kelly <martin@martingkelly.com>

Add DT binding documentation for the LSM9DS1 magnetometer driver.

Signed-off-by: Martin Kelly <martin@martingkelly.com>
---
 Documentation/devicetree/bindings/iio/st-sensors.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/st-sensors.txt b/Documentation/devicetree/bindings/iio/st-sensors.txt
index 6f626f73417e..07f1767c7ee6 100644
--- a/Documentation/devicetree/bindings/iio/st-sensors.txt
+++ b/Documentation/devicetree/bindings/iio/st-sensors.txt
@@ -67,6 +67,7 @@ Magnetometers:
 - st,lsm303dlm-magn
 - st,lis3mdl-magn
 - st,lis2mdl
+- st,lsm9ds1-magn
 
 Pressure sensors:
 - st,lps001wp-press
-- 
2.11.0

RE: [PATCH v2 1/4] iio: magnetometer: add clarifying comment

[Denis CIOCCA]

Hi Martin,

All the series [v2 / 4 patches] looks good to me. Thanks

Denis


-----Original Message-----
From: Martin Kelly <martin@martingkelly.com> 
Sent: Thursday, October 25, 2018 7:38 PM
To: linux-iio@vger.kernel.org; devicetree@vger.kernel.org
Cc: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>; Denis CIOCCA <denis.ciocca@st.com>; Jonathan Cameron <jic23@kernel.org>; Rob Herring <robh+dt@kernel.org>; Martin Kelly <martin@martingkelly.com>
Subject: [PATCH v2 1/4] iio: magnetometer: add clarifying comment

From: Martin Kelly <martin@martingkelly.com>

Add a comment clarifying better when to use <device_name> vs <device_name>-magn in compatibility strings.

Signed-off-by: Martin Kelly <martin@martingkelly.com>
---
 drivers/iio/magnetometer/st_magn_spi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
index 7b7cd08fcc32..15bb09267098 100644
--- a/drivers/iio/magnetometer/st_magn_spi.c
+++ b/drivers/iio/magnetometer/st_magn_spi.c
@@ -23,6 +23,8 @@
  * For new single-chip sensors use <device_name> as compatible string.
  * For old single-chip devices keep <device_name>-magn to maintain
  * compatibility
+ * For multi-chip devices, use <device_name>-magn to distinguish which
+ * capability is being used
  */
 static const struct of_device_id st_magn_of_match[] = {
 	{
--
2.11.0

Re: [PATCH v2 1/4] iio: magnetometer: add clarifying comment

[Jonathan Cameron]

On Fri, 26 Oct 2018 18:24:41 +0000
Denis CIOCCA <denis.ciocca@st.com> wrote:

> Hi Martin,
> 
> All the series [v2 / 4 patches] looks good to me. Thanks
> 

Formal Acked-by / Reviewed-by?

Thanks,

Jonathan

> Denis
> 
> 
> -----Original Message-----
> From: Martin Kelly <martin@martingkelly.com> 
> Sent: Thursday, October 25, 2018 7:38 PM
> To: linux-iio@vger.kernel.org; devicetree@vger.kernel.org
> Cc: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>; Denis CIOCCA <denis.ciocca@st.com>; Jonathan Cameron <jic23@kernel.org>; Rob Herring <robh+dt@kernel.org>; Martin Kelly <martin@martingkelly.com>
> Subject: [PATCH v2 1/4] iio: magnetometer: add clarifying comment
> 
> From: Martin Kelly <martin@martingkelly.com>
> 
> Add a comment clarifying better when to use <device_name> vs <device_name>-magn in compatibility strings.
> 
> Signed-off-by: Martin Kelly <martin@martingkelly.com>
> ---
>  drivers/iio/magnetometer/st_magn_spi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
> index 7b7cd08fcc32..15bb09267098 100644
> --- a/drivers/iio/magnetometer/st_magn_spi.c
> +++ b/drivers/iio/magnetometer/st_magn_spi.c
> @@ -23,6 +23,8 @@
>   * For new single-chip sensors use <device_name> as compatible string.
>   * For old single-chip devices keep <device_name>-magn to maintain
>   * compatibility
> + * For multi-chip devices, use <device_name>-magn to distinguish which
> + * capability is being used
>   */
>  static const struct of_device_id st_magn_of_match[] = {
>  	{
> --
> 2.11.0
> 

Re: [PATCH v2 1/4] iio: magnetometer: add clarifying comment

[Jonathan Cameron]

On Thu, 25 Oct 2018 19:38:09 -0700
Martin Kelly <martin@martingkelly.com> wrote:

> From: Martin Kelly <martin@martingkelly.com>
> 
> Add a comment clarifying better when to use <device_name> vs
> <device_name>-magn in compatibility strings.
> 
> Signed-off-by: Martin Kelly <martin@martingkelly.com>
Applied to the togreg branch of iio.git and pushed out as testing.
I'll ideally added acks from Dennis and Rob for relevant patches
before I push it out as non rebasing which will probably be next
weekend given my current habits.

Thanks,

Jonathan

> ---
>  drivers/iio/magnetometer/st_magn_spi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
> index 7b7cd08fcc32..15bb09267098 100644
> --- a/drivers/iio/magnetometer/st_magn_spi.c
> +++ b/drivers/iio/magnetometer/st_magn_spi.c
> @@ -23,6 +23,8 @@
>   * For new single-chip sensors use <device_name> as compatible string.
>   * For old single-chip devices keep <device_name>-magn to maintain
>   * compatibility
> + * For multi-chip devices, use <device_name>-magn to distinguish which
> + * capability is being used
>   */
>  static const struct of_device_id st_magn_of_match[] = {
>  	{

Re: [PATCH v2 2/4] iio:magnetometer: st_magn: add LSM9DS1 support

[Jonathan Cameron]

On Thu, 25 Oct 2018 19:38:10 -0700
Martin Kelly <martin@martingkelly.com> wrote:

> From: Martin Kelly <martin@martingkelly.com>
> 
> Update the sensor settings to support the LSM9DS1 sensor. Although the
> LSM9DS1 accelerometer and gyroscope are coupled together to use the same
> FIFO, the magnetometer is separate and can be cleanly supported without
> refactoring the existing driver.
> 
> Signed-off-by: Martin Kelly <martin@martingkelly.com>
Applied.

Thanks,

Jonathan

> ---
> v2:
> - Rename lsm9ds1 --> lsm9ds1-magn and lsm9ds1_magn, as this is a multi-device
>   chip.
> - Use the register layout of LIS3MDL, as the two chips match.
> - Add I2C compatibility strings.
> 
>  drivers/iio/magnetometer/st_magn.h      | 1 +
>  drivers/iio/magnetometer/st_magn_core.c | 1 +
>  drivers/iio/magnetometer/st_magn_i2c.c  | 5 +++++
>  drivers/iio/magnetometer/st_magn_spi.c  | 5 +++++
>  4 files changed, 12 insertions(+)
> 
> diff --git a/drivers/iio/magnetometer/st_magn.h b/drivers/iio/magnetometer/st_magn.h
> index 8fe51ce427bd..bc14ad4f1b26 100644
> --- a/drivers/iio/magnetometer/st_magn.h
> +++ b/drivers/iio/magnetometer/st_magn.h
> @@ -20,6 +20,7 @@
>  #define LIS3MDL_MAGN_DEV_NAME		"lis3mdl"
>  #define LSM303AGR_MAGN_DEV_NAME		"lsm303agr_magn"
>  #define LIS2MDL_MAGN_DEV_NAME		"lis2mdl"
> +#define LSM9DS1_MAGN_DEV_NAME		"lsm9ds1_magn"
> 
>  int st_magn_common_probe(struct iio_dev *indio_dev);
>  void st_magn_common_remove(struct iio_dev *indio_dev);
> diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
> index 72f6d1335a04..a186b844d664 100644
> --- a/drivers/iio/magnetometer/st_magn_core.c
> +++ b/drivers/iio/magnetometer/st_magn_core.c
> @@ -267,6 +267,7 @@ static const struct st_sensor_settings st_magn_sensors_settings[] = {
>  		.wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS,
>  		.sensors_supported = {
>  			[0] = LIS3MDL_MAGN_DEV_NAME,
> +			[1] = LSM9DS1_MAGN_DEV_NAME,
>  		},
>  		.ch = (struct iio_chan_spec *)st_magn_2_16bit_channels,
>  		.odr = {
> diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c
> index feaa28cf6a77..68650f5f5c19 100644
> --- a/drivers/iio/magnetometer/st_magn_i2c.c
> +++ b/drivers/iio/magnetometer/st_magn_i2c.c
> @@ -44,6 +44,10 @@ static const struct of_device_id st_magn_of_match[] = {
>  		.compatible = "st,lis2mdl",
>  		.data = LIS2MDL_MAGN_DEV_NAME,
>  	},
> +	{
> +		.compatible = "st,lsm9ds1-magn",
> +		.data = LSM9DS1_MAGN_DEV_NAME,
> +	},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, st_magn_of_match);
> @@ -90,6 +94,7 @@ static const struct i2c_device_id st_magn_id_table[] = {
>  	{ LIS3MDL_MAGN_DEV_NAME },
>  	{ LSM303AGR_MAGN_DEV_NAME },
>  	{ LIS2MDL_MAGN_DEV_NAME },
> +	{ LSM9DS1_MAGN_DEV_NAME },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(i2c, st_magn_id_table);
> diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
> index 15bb09267098..cb05fcd9ddfe 100644
> --- a/drivers/iio/magnetometer/st_magn_spi.c
> +++ b/drivers/iio/magnetometer/st_magn_spi.c
> @@ -39,6 +39,10 @@ static const struct of_device_id st_magn_of_match[] = {
>  		.compatible = "st,lis2mdl",
>  		.data = LIS2MDL_MAGN_DEV_NAME,
>  	},
> +	{
> +		.compatible = "st,lsm9ds1-magn",
> +		.data = LSM9DS1_MAGN_DEV_NAME,
> +	},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, st_magn_of_match);
> @@ -81,6 +85,7 @@ static const struct spi_device_id st_magn_id_table[] = {
>  	{ LIS3MDL_MAGN_DEV_NAME },
>  	{ LSM303AGR_MAGN_DEV_NAME },
>  	{ LIS2MDL_MAGN_DEV_NAME },
> +	{ LSM9DS1_MAGN_DEV_NAME },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(spi, st_magn_id_table);
> --
> 2.11.0
> 

Re: [PATCH v2 3/4] iio:magnetometer: st_magn: add BDU settings

[Jonathan Cameron]

On Thu, 25 Oct 2018 19:38:11 -0700
Martin Kelly <martin@martingkelly.com> wrote:

> From: Martin Kelly <martin@martingkelly.com>
> 
> LIS3MDL and LSM9DS1 are missing BDU settings in their register maps, so add
> them. I don't have a LIS3MDL sensor to test, but this works correctly on
> the LSM9DS1, which has the same register map.
> 
> Signed-off-by: Martin Kelly <martin@martingkelly.com>
Applied thanks,

Jonathan

> ---
>  drivers/iio/magnetometer/st_magn_core.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
> index a186b844d664..370b93db528d 100644
> --- a/drivers/iio/magnetometer/st_magn_core.c
> +++ b/drivers/iio/magnetometer/st_magn_core.c
> @@ -316,6 +316,10 @@ static const struct st_sensor_settings st_magn_sensors_settings[] = {
>  				},
>  			},
>  		},
> +		.bdu = {
> +			.addr = 0x24,
> +			.mask = 0x40,
> +		},
>  		.drdy_irq = {
>  			/* drdy line is routed drdy pin */
>  			.stat_drdy = {

Re: [PATCH v2 4/4] dt-bindings: iio: magn: add LSM9DS1 bindings

[Jonathan Cameron]

On Thu, 25 Oct 2018 19:38:12 -0700
Martin Kelly <martin@martingkelly.com> wrote:

> From: Martin Kelly <martin@martingkelly.com>
> 
> Add DT binding documentation for the LSM9DS1 magnetometer driver.
> 
> Signed-off-by: Martin Kelly <martin@martingkelly.com>
Given here isn't really an alternative way of doing this (or we
haven't come up with one yet) in the sensors that are basically multiple
chips in one package, I'll hope that Rob is happy with the new name
and apply it.   As I'm only pushing out for build tests today I can
revert or add his Reviewed-by as needed.

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan
> ---
>  Documentation/devicetree/bindings/iio/st-sensors.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/st-sensors.txt b/Documentation/devicetree/bindings/iio/st-sensors.txt
> index 6f626f73417e..07f1767c7ee6 100644
> --- a/Documentation/devicetree/bindings/iio/st-sensors.txt
> +++ b/Documentation/devicetree/bindings/iio/st-sensors.txt
> @@ -67,6 +67,7 @@ Magnetometers:
>  - st,lsm303dlm-magn
>  - st,lis3mdl-magn
>  - st,lis2mdl
> +- st,lsm9ds1-magn
>  
>  Pressure sensors:
>  - st,lps001wp-press

Re: [PATCH v2 4/4] dt-bindings: iio: magn: add LSM9DS1 bindings

[Rob Herring]

On Sun, Oct 28, 2018 at 06:53:08PM +0000, Jonathan Cameron wrote:
> On Thu, 25 Oct 2018 19:38:12 -0700
> Martin Kelly <martin@martingkelly.com> wrote:
> 
> > From: Martin Kelly <martin@martingkelly.com>
> > 
> > Add DT binding documentation for the LSM9DS1 magnetometer driver.

Bindings are for h/w, not drivers.

> > 
> > Signed-off-by: Martin Kelly <martin@martingkelly.com>
> Given here isn't really an alternative way of doing this (or we
> haven't come up with one yet) in the sensors that are basically multiple
> chips in one package, I'll hope that Rob is happy with the new name
> and apply it.   As I'm only pushing out for build tests today I can
> revert or add his Reviewed-by as needed.

Given only the supplies seem to be shared and those can be handled 
correctly with ref counting, separate devices is fine. We do the same 
thing for BT/WiFi combo chips with separate host interfaces.

Reviewed-by: Rob Herring <robh@kernel.org>

Rob

Re: [PATCH v2 4/4] dt-bindings: iio: magn: add LSM9DS1 bindings

[Jonathan Cameron]

On Tue, 30 Oct 2018 16:44:41 -0500
Rob Herring <robh@kernel.org> wrote:

> On Sun, Oct 28, 2018 at 06:53:08PM +0000, Jonathan Cameron wrote:
> > On Thu, 25 Oct 2018 19:38:12 -0700
> > Martin Kelly <martin@martingkelly.com> wrote:
> >   
> > > From: Martin Kelly <martin@martingkelly.com>
> > > 
> > > Add DT binding documentation for the LSM9DS1 magnetometer driver.  
> 
> Bindings are for h/w, not drivers.
> 
> > > 
> > > Signed-off-by: Martin Kelly <martin@martingkelly.com>  
> > Given here isn't really an alternative way of doing this (or we
> > haven't come up with one yet) in the sensors that are basically multiple
> > chips in one package, I'll hope that Rob is happy with the new name
> > and apply it.   As I'm only pushing out for build tests today I can
> > revert or add his Reviewed-by as needed.  
> 
> Given only the supplies seem to be shared and those can be handled 
> correctly with ref counting, separate devices is fine. We do the same 
> thing for BT/WiFi combo chips with separate host interfaces.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
Seems I'd actually failed to apply this last week, so now applied
with the message tweaked and Rob's tag.

Thanks to both of you,

Jonathan
> 
> Rob