[PATCH] i2c: designware: deduce speed mode from device tree setting

[PATCH] i2c: designware: deduce speed mode from device tree setting

[atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

From: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

Use the documented, but unimplemented "clock-frequency"
Device Tree setting as a guide on whether to set the speed
mode bits in DW_IC_CON to standard or fast i2c mode.

Previously, the driver was hardwired to fast mode.  Default
to fast mode if the "clock-frequency" property is not present
for backwards compatiblity.

Signed-off-by: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-platdrv.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index bc87733..18cd3d9 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -122,7 +122,8 @@ static int dw_i2c_probe(struct platform_device *pdev)
 	struct dw_i2c_dev *dev;
 	struct i2c_adapter *adap;
 	struct resource *mem;
-	int irq, r;
+	int irq, r, ret, speed = DW_IC_CON_SPEED_FAST;
+	u32 bus_rate;
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
@@ -167,6 +168,11 @@ static int dw_i2c_probe(struct platform_device *pdev)
 		of_property_read_u32(pdev->dev.of_node,
 				     "i2c-scl-falling-time-ns",
 				     &dev->scl_falling_time);
+
+		ret = of_property_read_u32(pdev->dev.of_node,
+					   "clock-frequency", &bus_rate);
+		if (!ret && (bus_rate <= 100000))
+			speed = DW_IC_CON_SPEED_STD;
 	}
 
 	dev->functionality =
@@ -177,7 +183,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
 		I2C_FUNC_SMBUS_WORD_DATA |
 		I2C_FUNC_SMBUS_I2C_BLOCK;
 	dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
-		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
+		DW_IC_CON_RESTART_EN | speed;
 
 	/* Try first if we can configure the device from ACPI */
 	r = dw_i2c_acpi_configure(pdev);
-- 
1.7.9.5

Re: [PATCH] i2c: designware: deduce speed mode from device tree setting

[Mika Westerberg]

On Tue, Aug 19, 2014 at 03:18:49PM -0500, atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:
> From: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> 
> Use the documented, but unimplemented "clock-frequency"
> Device Tree setting as a guide on whether to set the speed
> mode bits in DW_IC_CON to standard or fast i2c mode.
> 
> Previously, the driver was hardwired to fast mode.  Default
> to fast mode if the "clock-frequency" property is not present
> for backwards compatiblity.
> 
> Signed-off-by: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

I don't know much about the Device Tree bindings but the patch looks
good to me,

Acked-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Re: [PATCH] i2c: designware: deduce speed mode from device tree setting

[Mark Rutland]

On Tue, Aug 19, 2014 at 09:18:49PM +0100, atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:
> From: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> 
> Use the documented, but unimplemented "clock-frequency"
> Device Tree setting as a guide on whether to set the speed
> mode bits in DW_IC_CON to standard or fast i2c mode.
> 
> Previously, the driver was hardwired to fast mode.  Default
> to fast mode if the "clock-frequency" property is not present
> for backwards compatiblity.
> 
> Signed-off-by: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-designware-platdrv.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index bc87733..18cd3d9 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -122,7 +122,8 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  	struct dw_i2c_dev *dev;
>  	struct i2c_adapter *adap;
>  	struct resource *mem;
> -	int irq, r;
> +	int irq, r, ret, speed = DW_IC_CON_SPEED_FAST;
> +	u32 bus_rate;
>  
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
> @@ -167,6 +168,11 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  		of_property_read_u32(pdev->dev.of_node,
>  				     "i2c-scl-falling-time-ns",
>  				     &dev->scl_falling_time);
> +
> +		ret = of_property_read_u32(pdev->dev.of_node,
> +					   "clock-frequency", &bus_rate);
> +		if (!ret && (bus_rate <= 100000))
> +			speed = DW_IC_CON_SPEED_STD;

This looks a bit odd.

If the device only supports two particular speeds why do we accept any
other speed in the clock-frequency property? Surely we should at least
warn that something was off?

Thanks,
Mark

>  	}
>  
>  	dev->functionality =
> @@ -177,7 +183,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  		I2C_FUNC_SMBUS_WORD_DATA |
>  		I2C_FUNC_SMBUS_I2C_BLOCK;
>  	dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> -		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> +		DW_IC_CON_RESTART_EN | speed;
>  
>  	/* Try first if we can configure the device from ACPI */
>  	r = dw_i2c_acpi_configure(pdev);
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] i2c: designware: deduce speed mode from device tree setting

[Romain Baeriswyl]

Hi,

With the patch "i2c designware add support of I2C standard mode" I already proposed:
- I2C standard mode is selected with 100kHz clock frequency.
- I2C fast mode is selected with 400kHy clock frequency.
- EINVAL error is returned if clock frequency is not 100000 and not 400000.

but this patch seems not available yet.
What about the other patch "i2c designware make SCL and SDA falling time configurable" ?

In i2c-designware_platdrv.c the flag DW_IC_CON_SPEED_xxx is well set
depending on the mode:

  if (clk_freq == 100000)
     dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
       DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
   else
     dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
       DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;


So for me everything should be fine if there patches are applied.

Regards,

Romain

----- Original Message -----
From: "Mark Rutland" <mark.rutland-5wv7dgnIgG8@public.gmane.org>
To: atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org
Cc: wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org, baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org, "mika westerberg" <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>, "grant likely" <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, skuribay-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org, "Romain Baeriswyl" <Romain.Baeriswyl-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>, "rafael j wysocki" <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, alan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "delicious quinoa" <delicious.quinoa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, dinguyen-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org, yvanderv-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org
Sent: Wednesday, August 20, 2014 11:22:57 AM
Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree setting

On Tue, Aug 19, 2014 at 09:18:49PM +0100, atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:
> From: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> 
> Use the documented, but unimplemented "clock-frequency"
> Device Tree setting as a guide on whether to set the speed
> mode bits in DW_IC_CON to standard or fast i2c mode.
> 
> Previously, the driver was hardwired to fast mode.  Default
> to fast mode if the "clock-frequency" property is not present
> for backwards compatiblity.
> 
> Signed-off-by: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-designware-platdrv.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index bc87733..18cd3d9 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -122,7 +122,8 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  	struct dw_i2c_dev *dev;
>  	struct i2c_adapter *adap;
>  	struct resource *mem;
> -	int irq, r;
> +	int irq, r, ret, speed = DW_IC_CON_SPEED_FAST;
> +	u32 bus_rate;
>  
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
> @@ -167,6 +168,11 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  		of_property_read_u32(pdev->dev.of_node,
>  				     "i2c-scl-falling-time-ns",
>  				     &dev->scl_falling_time);
> +
> +		ret = of_property_read_u32(pdev->dev.of_node,
> +					   "clock-frequency", &bus_rate);
> +		if (!ret && (bus_rate <= 100000))
> +			speed = DW_IC_CON_SPEED_STD;

This looks a bit odd.

If the device only supports two particular speeds why do we accept any
other speed in the clock-frequency property? Surely we should at least
warn that something was off?

Thanks,
Mark

>  	}
>  
>  	dev->functionality =
> @@ -177,7 +183,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  		I2C_FUNC_SMBUS_WORD_DATA |
>  		I2C_FUNC_SMBUS_I2C_BLOCK;
>  	dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> -		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> +		DW_IC_CON_RESTART_EN | speed;
>  
>  	/* Try first if we can configure the device from ACPI */
>  	r = dw_i2c_acpi_configure(pdev);
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] i2c: designware: deduce speed mode from device tree setting

[Romain Baeriswyl]

Please find below link to these patches:

[PATCH 1/2] i2c designware make SCL and SDA falling time configurable
https://lkml.org/lkml/2013/10/8/443

[PATCH V3 2/2] i2c designware add support of I2C standard mode
https://lkml.org/lkml/2014/3/25/135

Are they fine to be included in the mainline ?

Romain

----- Original Message -----
From: "Romain Baeriswyl" <Romain.Baeriswyl@abilis.com>
To: "Mark Rutland" <mark.rutland@arm.com>, atull@opensource.altera.com
Cc: wsa@the-dreams.de, baruch@tkos.co.il, "mika westerberg" <mika.westerberg@linux.intel.com>, "grant likely" <grant.likely@linaro.org>, robh+dt@kernel.org, skuribay@pobox.com, "rafael j wysocki" <rafael.j.wysocki@intel.com>, alan@linux.intel.com, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, "delicious quinoa" <delicious.quinoa@gmail.com>, dinguyen@opensource.altera.com, yvanderv@opensource.altera.com
Sent: Wednesday, August 20, 2014 11:49:37 AM
Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree setting

Hi,

With the patch "i2c designware add support of I2C standard mode" I already proposed:
- I2C standard mode is selected with 100kHz clock frequency.
- I2C fast mode is selected with 400kHy clock frequency.
- EINVAL error is returned if clock frequency is not 100000 and not 400000.

but this patch seems not available yet.
What about the other patch "i2c designware make SCL and SDA falling time configurable" ?

In i2c-designware_platdrv.c the flag DW_IC_CON_SPEED_xxx is well set
depending on the mode:

  if (clk_freq == 100000)
     dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
       DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
   else
     dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
       DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;


So for me everything should be fine if there patches are applied.

Regards,

Romain

----- Original Message -----
From: "Mark Rutland" <mark.rutland@arm.com>
To: atull@opensource.altera.com
Cc: wsa@the-dreams.de, baruch@tkos.co.il, "mika westerberg" <mika.westerberg@linux.intel.com>, "grant likely" <grant.likely@linaro.org>, robh+dt@kernel.org, skuribay@pobox.com, "Romain Baeriswyl" <Romain.Baeriswyl@abilis.com>, "rafael j wysocki" <rafael.j.wysocki@intel.com>, alan@linux.intel.com, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, "delicious quinoa" <delicious.quinoa@gmail.com>, dinguyen@opensource.altera.com, yvanderv@opensource.altera.com
Sent: Wednesday, August 20, 2014 11:22:57 AM
Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree setting

On Tue, Aug 19, 2014 at 09:18:49PM +0100, atull@opensource.altera.com wrote:
> From: Alan Tull <atull@opensource.altera.com>
> 
> Use the documented, but unimplemented "clock-frequency"
> Device Tree setting as a guide on whether to set the speed
> mode bits in DW_IC_CON to standard or fast i2c mode.
> 
> Previously, the driver was hardwired to fast mode.  Default
> to fast mode if the "clock-frequency" property is not present
> for backwards compatiblity.
> 
> Signed-off-by: Alan Tull <atull@opensource.altera.com>
> ---
>  drivers/i2c/busses/i2c-designware-platdrv.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index bc87733..18cd3d9 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -122,7 +122,8 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  	struct dw_i2c_dev *dev;
>  	struct i2c_adapter *adap;
>  	struct resource *mem;
> -	int irq, r;
> +	int irq, r, ret, speed = DW_IC_CON_SPEED_FAST;
> +	u32 bus_rate;
>  
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
> @@ -167,6 +168,11 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  		of_property_read_u32(pdev->dev.of_node,
>  				     "i2c-scl-falling-time-ns",
>  				     &dev->scl_falling_time);
> +
> +		ret = of_property_read_u32(pdev->dev.of_node,
> +					   "clock-frequency", &bus_rate);
> +		if (!ret && (bus_rate <= 100000))
> +			speed = DW_IC_CON_SPEED_STD;

This looks a bit odd.

If the device only supports two particular speeds why do we accept any
other speed in the clock-frequency property? Surely we should at least
warn that something was off?

Thanks,
Mark

>  	}
>  
>  	dev->functionality =
> @@ -177,7 +183,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  		I2C_FUNC_SMBUS_WORD_DATA |
>  		I2C_FUNC_SMBUS_I2C_BLOCK;
>  	dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> -		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> +		DW_IC_CON_RESTART_EN | speed;
>  
>  	/* Try first if we can configure the device from ACPI */
>  	r = dw_i2c_acpi_configure(pdev);
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] i2c: designware: deduce speed mode from device tree setting

[Wolfram Sang]

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


> With the patch "i2c designware add support of I2C standard mode" I already proposed:
> - I2C standard mode is selected with 100kHz clock frequency.
> - I2C fast mode is selected with 400kHy clock frequency.
> - EINVAL error is returned if clock frequency is not 100000 and not 400000.

You forgot to send this patch to the i2c list. So, it didn't get into
patchwork which I rely on for tracking patches.

Please resend an updated version to the latest codebase. I like that it
has the EINVAL error checking.

> What about the other patch "i2c designware make SCL and SDA falling time configurable" ?

6468276b22069d4442aafcd8c59e5d8ccae23f5f upstream.


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

Re: [PATCH] i2c: designware: deduce speed mode from device tree setting

[atull]

On Wed, 20 Aug 2014, Romain Baeriswyl wrote:

> Hi,
> 
> With the patch "i2c designware add support of I2C standard mode" I already proposed:
> - I2C standard mode is selected with 100kHz clock frequency.
> - I2C fast mode is selected with 400kHy clock frequency.
> - EINVAL error is returned if clock frequency is not 100000 and not 400000.
> 
> but this patch seems not available yet.
> What about the other patch "i2c designware make SCL and SDA falling time configurable" ?
> 
> In i2c-designware_platdrv.c the flag DW_IC_CON_SPEED_xxx is well set
> depending on the mode:
> 
>   if (clk_freq == 100000)

Romain,

I'm really happy if your patches get accepted.  Can this be <= 100000?
It is really common to run I2C at a lower speed if you have some
board issues with the i2c bus.

Alan

>      dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
>        DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
>    else
>      dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
>        DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> 
> 
> So for me everything should be fine if there patches are applied.
> 
> Regards,
> 
> Romain
> 
> ----- Original Message -----
> From: "Mark Rutland" <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> To: atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org
> Cc: wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org, baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org, "mika westerberg" <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>, "grant likely" <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, skuribay-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org, "Romain Baeriswyl" <Romain.Baeriswyl-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>, "rafael j wysocki" <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, alan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "delicious quinoa" <delicious.quinoa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, dinguyen-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org, yvanderv-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org
> Sent: Wednesday, August 20, 2014 11:22:57 AM
> Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
> 
> On Tue, Aug 19, 2014 at 09:18:49PM +0100, atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:
> > From: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> > 
> > Use the documented, but unimplemented "clock-frequency"
> > Device Tree setting as a guide on whether to set the speed
> > mode bits in DW_IC_CON to standard or fast i2c mode.
> > 
> > Previously, the driver was hardwired to fast mode.  Default
> > to fast mode if the "clock-frequency" property is not present
> > for backwards compatiblity.
> > 
> > Signed-off-by: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> > ---
> >  drivers/i2c/busses/i2c-designware-platdrv.c |   10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> > index bc87733..18cd3d9 100644
> > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > @@ -122,7 +122,8 @@ static int dw_i2c_probe(struct platform_device *pdev)
> >  	struct dw_i2c_dev *dev;
> >  	struct i2c_adapter *adap;
> >  	struct resource *mem;
> > -	int irq, r;
> > +	int irq, r, ret, speed = DW_IC_CON_SPEED_FAST;
> > +	u32 bus_rate;
> >  
> >  	irq = platform_get_irq(pdev, 0);
> >  	if (irq < 0) {
> > @@ -167,6 +168,11 @@ static int dw_i2c_probe(struct platform_device *pdev)
> >  		of_property_read_u32(pdev->dev.of_node,
> >  				     "i2c-scl-falling-time-ns",
> >  				     &dev->scl_falling_time);
> > +
> > +		ret = of_property_read_u32(pdev->dev.of_node,
> > +					   "clock-frequency", &bus_rate);
> > +		if (!ret && (bus_rate <= 100000))
> > +			speed = DW_IC_CON_SPEED_STD;
> 
> This looks a bit odd.
> 
> If the device only supports two particular speeds why do we accept any
> other speed in the clock-frequency property? Surely we should at least
> warn that something was off?
> 
> Thanks,
> Mark
> 
> >  	}
> >  
> >  	dev->functionality =
> > @@ -177,7 +183,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
> >  		I2C_FUNC_SMBUS_WORD_DATA |
> >  		I2C_FUNC_SMBUS_I2C_BLOCK;
> >  	dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> > -		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> > +		DW_IC_CON_RESTART_EN | speed;
> >  
> >  	/* Try first if we can configure the device from ACPI */
> >  	r = dw_i2c_acpi_configure(pdev);
> > -- 
> > 1.7.9.5
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 

Re: [PATCH] i2c: designware: deduce speed mode from device tree setting

[Romain Baeriswyl]

Hi Alan,

We got board issue using I2C and they were solved by changing the i2c timing.

It is possible to change the tLOW and tHIGH period with the following parameters:
  i2c-sda-hold-time-ns
  i2c-sda-falling-time-ns
  i2c-scl-falling-time-ns

Romain

----- Original Message -----
From: "atull" <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
To: "Romain Baeriswyl" <Romain.Baeriswyl-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
Cc: "Mark Rutland" <mark.rutland-5wv7dgnIgG8@public.gmane.org>, wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org, baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org, "mika westerberg" <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>, "grant likely" <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, skuribay-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org, "rafael j wysocki" <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, alan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "delicious quinoa" <delicious.quinoa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, dinguyen-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org, yvanderv-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org
Sent: Wednesday, August 20, 2014 4:24:45 PM
Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree setting



On Wed, 20 Aug 2014, Romain Baeriswyl wrote:

> Hi,
> 
> With the patch "i2c designware add support of I2C standard mode" I already proposed:
> - I2C standard mode is selected with 100kHz clock frequency.
> - I2C fast mode is selected with 400kHy clock frequency.
> - EINVAL error is returned if clock frequency is not 100000 and not 400000.
> 
> but this patch seems not available yet.
> What about the other patch "i2c designware make SCL and SDA falling time configurable" ?
> 
> In i2c-designware_platdrv.c the flag DW_IC_CON_SPEED_xxx is well set
> depending on the mode:
> 
>   if (clk_freq == 100000)

Romain,

I'm really happy if your patches get accepted.  Can this be <= 100000?
It is really common to run I2C at a lower speed if you have some
board issues with the i2c bus.

Alan

>      dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
>        DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
>    else
>      dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
>        DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> 
> 
> So for me everything should be fine if there patches are applied.
> 
> Regards,
> 
> Romain
> 
> ----- Original Message -----
> From: "Mark Rutland" <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> To: atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org
> Cc: wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org, baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org, "mika westerberg" <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>, "grant likely" <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, skuribay-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org, "Romain Baeriswyl" <Romain.Baeriswyl-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>, "rafael j wysocki" <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, alan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "delicious quinoa" <delicious.quinoa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, dinguyen-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org, yvanderv-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org
> Sent: Wednesday, August 20, 2014 11:22:57 AM
> Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
> 
> On Tue, Aug 19, 2014 at 09:18:49PM +0100, atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:
> > From: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> > 
> > Use the documented, but unimplemented "clock-frequency"
> > Device Tree setting as a guide on whether to set the speed
> > mode bits in DW_IC_CON to standard or fast i2c mode.
> > 
> > Previously, the driver was hardwired to fast mode.  Default
> > to fast mode if the "clock-frequency" property is not present
> > for backwards compatiblity.
> > 
> > Signed-off-by: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> > ---
> >  drivers/i2c/busses/i2c-designware-platdrv.c |   10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> > index bc87733..18cd3d9 100644
> > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > @@ -122,7 +122,8 @@ static int dw_i2c_probe(struct platform_device *pdev)
> >  	struct dw_i2c_dev *dev;
> >  	struct i2c_adapter *adap;
> >  	struct resource *mem;
> > -	int irq, r;
> > +	int irq, r, ret, speed = DW_IC_CON_SPEED_FAST;
> > +	u32 bus_rate;
> >  
> >  	irq = platform_get_irq(pdev, 0);
> >  	if (irq < 0) {
> > @@ -167,6 +168,11 @@ static int dw_i2c_probe(struct platform_device *pdev)
> >  		of_property_read_u32(pdev->dev.of_node,
> >  				     "i2c-scl-falling-time-ns",
> >  				     &dev->scl_falling_time);
> > +
> > +		ret = of_property_read_u32(pdev->dev.of_node,
> > +					   "clock-frequency", &bus_rate);
> > +		if (!ret && (bus_rate <= 100000))
> > +			speed = DW_IC_CON_SPEED_STD;
> 
> This looks a bit odd.
> 
> If the device only supports two particular speeds why do we accept any
> other speed in the clock-frequency property? Surely we should at least
> warn that something was off?
> 
> Thanks,
> Mark
> 
> >  	}
> >  
> >  	dev->functionality =
> > @@ -177,7 +183,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
> >  		I2C_FUNC_SMBUS_WORD_DATA |
> >  		I2C_FUNC_SMBUS_I2C_BLOCK;
> >  	dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> > -		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> > +		DW_IC_CON_RESTART_EN | speed;
> >  
> >  	/* Try first if we can configure the device from ACPI */
> >  	r = dw_i2c_acpi_configure(pdev);
> > -- 
> > 1.7.9.5
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: designware: deduce speed mode from device tree setting

[atull]

On Wed, 20 Aug 2014, Romain Baeriswyl wrote:

> Hi Alan,
> 
> We got board issue using I2C and they were solved by changing the i2c timing.
> 
> It is possible to change the tLOW and tHIGH period with the following parameters:
>   i2c-sda-hold-time-ns
>   i2c-sda-falling-time-ns
>   i2c-scl-falling-time-ns
> 
> Romain

Hi Romain,

This thread is getting kind of messed up.  Yep, that's how I am slowing 
down the bus, using i2c-sda-falling-time-ns and i2c-scl-falling-time-ns.
It is fine with me if you want to test for == 100KHz, just not my 
(probably hair-splitting) preference. :)

Alan

> 
> ----- Original Message -----
> From: "atull" <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> To: "Romain Baeriswyl" <Romain.Baeriswyl-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
> Cc: "Mark Rutland" <mark.rutland-5wv7dgnIgG8@public.gmane.org>, wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org, baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org, "mika westerberg" <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>, "grant likely" <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, skuribay-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org, "rafael j wysocki" <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, alan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "delicious quinoa" <delicious.quinoa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, dinguyen-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org, yvanderv-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org
> Sent: Wednesday, August 20, 2014 4:24:45 PM
> Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
> 
> 
> 
> On Wed, 20 Aug 2014, Romain Baeriswyl wrote:
> 
> > Hi,
> > 
> > With the patch "i2c designware add support of I2C standard mode" I already proposed:
> > - I2C standard mode is selected with 100kHz clock frequency.
> > - I2C fast mode is selected with 400kHy clock frequency.
> > - EINVAL error is returned if clock frequency is not 100000 and not 400000.
> > 
> > but this patch seems not available yet.
> > What about the other patch "i2c designware make SCL and SDA falling time configurable" ?
> > 
> > In i2c-designware_platdrv.c the flag DW_IC_CON_SPEED_xxx is well set
> > depending on the mode:
> > 
> >   if (clk_freq == 100000)
> 
> Romain,
> 
> I'm really happy if your patches get accepted.  Can this be <= 100000?
> It is really common to run I2C at a lower speed if you have some
> board issues with the i2c bus.
> 
> Alan
> 
> >      dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> >        DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
> >    else
> >      dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> >        DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> > 
> > 
> > So for me everything should be fine if there patches are applied.
> > 
> > Regards,
> > 
> > Romain
> > 
> > ----- Original Message -----
> > From: "Mark Rutland" <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> > To: atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org
> > Cc: wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org, baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org, "mika westerberg" <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>, "grant likely" <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, skuribay-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org, "Romain Baeriswyl" <Romain.Baeriswyl-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>, "rafael j wysocki" <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, alan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "delicious quinoa" <delicious.quinoa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, dinguyen-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org, yvanderv-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org
> > Sent: Wednesday, August 20, 2014 11:22:57 AM
> > Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
> > 
> > On Tue, Aug 19, 2014 at 09:18:49PM +0100, atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:
> > > From: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> > > 
> > > Use the documented, but unimplemented "clock-frequency"
> > > Device Tree setting as a guide on whether to set the speed
> > > mode bits in DW_IC_CON to standard or fast i2c mode.
> > > 
> > > Previously, the driver was hardwired to fast mode.  Default
> > > to fast mode if the "clock-frequency" property is not present
> > > for backwards compatiblity.
> > > 
> > > Signed-off-by: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> > > ---
> > >  drivers/i2c/busses/i2c-designware-platdrv.c |   10 ++++++++--
> > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> > > index bc87733..18cd3d9 100644
> > > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > > @@ -122,7 +122,8 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > >  	struct dw_i2c_dev *dev;
> > >  	struct i2c_adapter *adap;
> > >  	struct resource *mem;
> > > -	int irq, r;
> > > +	int irq, r, ret, speed = DW_IC_CON_SPEED_FAST;
> > > +	u32 bus_rate;
> > >  
> > >  	irq = platform_get_irq(pdev, 0);
> > >  	if (irq < 0) {
> > > @@ -167,6 +168,11 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > >  		of_property_read_u32(pdev->dev.of_node,
> > >  				     "i2c-scl-falling-time-ns",
> > >  				     &dev->scl_falling_time);
> > > +
> > > +		ret = of_property_read_u32(pdev->dev.of_node,
> > > +					   "clock-frequency", &bus_rate);
> > > +		if (!ret && (bus_rate <= 100000))
> > > +			speed = DW_IC_CON_SPEED_STD;
> > 
> > This looks a bit odd.
> > 
> > If the device only supports two particular speeds why do we accept any
> > other speed in the clock-frequency property? Surely we should at least
> > warn that something was off?
> > 
> > Thanks,
> > Mark
> > 
> > >  	}
> > >  
> > >  	dev->functionality =
> > > @@ -177,7 +183,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > >  		I2C_FUNC_SMBUS_WORD_DATA |
> > >  		I2C_FUNC_SMBUS_I2C_BLOCK;
> > >  	dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> > > -		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> > > +		DW_IC_CON_RESTART_EN | speed;
> > >  
> > >  	/* Try first if we can configure the device from ACPI */
> > >  	r = dw_i2c_acpi_configure(pdev);
> > > -- 
> > > 1.7.9.5
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > 
> > 
> 

Re: [PATCH] i2c: designware: deduce speed mode from device tree setting

[Wolfram Sang]

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

> > +
> > +		ret = of_property_read_u32(pdev->dev.of_node,
> > +					   "clock-frequency", &bus_rate);
> > +		if (!ret && (bus_rate <= 100000))
> > +			speed = DW_IC_CON_SPEED_STD;
> 
> This looks a bit odd.
> 
> If the device only supports two particular speeds why do we accept any
> other speed in the clock-frequency property?

"clock-frequency" is the default binding for specifying i2c bus speeds
today. Some controllers can be programmed to do various speeds, some can
only do a set of fixed values.

> Surely we should at least warn that something was off?

Yes, I was going to say the same until Romain's old patch showed up
which does that.

Thanks,

   Wolfram


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

Re: [PATCH] i2c: designware: deduce speed mode from device tree setting

[Mark Rutland]

On Wed, Aug 20, 2014 at 01:36:18PM +0100, Wolfram Sang wrote:
> > > +
> > > +		ret = of_property_read_u32(pdev->dev.of_node,
> > > +					   "clock-frequency", &bus_rate);
> > > +		if (!ret && (bus_rate <= 100000))
> > > +			speed = DW_IC_CON_SPEED_STD;
> > 
> > This looks a bit odd.
> > 
> > If the device only supports two particular speeds why do we accept any
> > other speed in the clock-frequency property?
> 
> "clock-frequency" is the default binding for specifying i2c bus speeds
> today. Some controllers can be programmed to do various speeds, some can
> only do a set of fixed values.

Sure. My complaint was that the driver would accept invalid values.
That wasn't meant to be suggestion to use a property other than the
standard clock-freqeuncy.

> > Surely we should at least warn that something was off?
> 
> Yes, I was going to say the same until Romain's old patch showed up
> which does that.

Cool. Sounds like we can use Romain's patch to handle this then.

Cheers,
Mark.

[PATCH] i2c designware add support of I2C standard mode

[Romain Baeriswyl]

From: Romain Baeriswyl <Romain.Baeriswyl-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>

Some legacy devices support ony I2C standard mode at 100kHz.
This patch allows to select the standard mode through the DTS
with the use of the existing clock-frequency parameter.

When clock-frequency parameter is not set, the fast mode is selected.
Only when the parameter is set at 100000, the standard mode is selected.

Signed-off-by: Romain Baeriswyl <romainba-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Christian Ruppert <christian.ruppert-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-platdrv.c |   23 +++++++++++++++++++++--
 1 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 402ec39..b543fe1 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -122,6 +122,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
 	struct i2c_adapter *adap;
 	struct resource *mem;
 	int irq, r;
+	u32 clk_freq;
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
@@ -151,6 +152,9 @@ static int dw_i2c_probe(struct platform_device *pdev)
 		return PTR_ERR(dev->clk);
 	clk_prepare_enable(dev->clk);
 
+	/* fast mode by default because of legacy reasons */
+	clk_freq = 400000;
+
 	if (pdev->dev.of_node) {
 		u32 ht = 0;
 		u32 ic_clk = dev->get_clk_rate_khz(dev);
@@ -166,6 +170,17 @@ static int dw_i2c_probe(struct platform_device *pdev)
 		of_property_read_u32(pdev->dev.of_node,
 				     "i2c-scl-falling-time-ns",
 				     &dev->scl_falling_time);
+
+		of_property_read_u32(pdev->dev.of_node, "clock-frequency",
+				     &clk_freq);
+
+		/* Only standard mode at 100kHz and fast mode at 400kHz
+		 * are supported.
+		 */
+		if (clk_freq != 100000 && clk_freq != 400000) {
+			dev_err(&pdev->dev, "Only 100kHz and 400kHz supported");
+			return -EINVAL;
+		}
 	}
 
 	dev->functionality =
@@ -175,8 +190,12 @@ static int dw_i2c_probe(struct platform_device *pdev)
 		I2C_FUNC_SMBUS_BYTE_DATA |
 		I2C_FUNC_SMBUS_WORD_DATA |
 		I2C_FUNC_SMBUS_I2C_BLOCK;
-	dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
-		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
+	if (clk_freq == 100000)
+		dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
+			DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
+	else
+		dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
+			DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
 
 	/* Try first if we can configure the device from ACPI */
 	r = dw_i2c_acpi_configure(pdev);
-- 
1.7.1

Re: [PATCH] i2c designware add support of I2C standard mode

[atull]

On Wed, 20 Aug 2014, Romain Baeriswyl wrote:

> From: Romain Baeriswyl <Romain.Baeriswyl-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
> 
> Some legacy devices support ony I2C standard mode at 100kHz.
> This patch allows to select the standard mode through the DTS
> with the use of the existing clock-frequency parameter.
> 
> When clock-frequency parameter is not set, the fast mode is selected.
> Only when the parameter is set at 100000, the standard mode is selected.
> 
> Signed-off-by: Romain Baeriswyl <romainba-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Christian Ruppert <christian.ruppert-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-designware-platdrv.c |   23 +++++++++++++++++++++--
>  1 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 402ec39..b543fe1 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -122,6 +122,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  	struct i2c_adapter *adap;
>  	struct resource *mem;
>  	int irq, r;
> +	u32 clk_freq;
>  
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
> @@ -151,6 +152,9 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  		return PTR_ERR(dev->clk);
>  	clk_prepare_enable(dev->clk);
>  
> +	/* fast mode by default because of legacy reasons */
> +	clk_freq = 400000;
> +
>  	if (pdev->dev.of_node) {
>  		u32 ht = 0;
>  		u32 ic_clk = dev->get_clk_rate_khz(dev);
> @@ -166,6 +170,17 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  		of_property_read_u32(pdev->dev.of_node,
>  				     "i2c-scl-falling-time-ns",
>  				     &dev->scl_falling_time);
> +
> +		of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> +				     &clk_freq);
> +
> +		/* Only standard mode at 100kHz and fast mode at 400kHz
> +		 * are supported.
> +		 */
> +		if (clk_freq != 100000 && clk_freq != 400000) {
> +			dev_err(&pdev->dev, "Only 100kHz and 400kHz supported");
> +			return -EINVAL;
> +		}

Hi Romain,

It is common to operate i2c at <100KHz on boards that have i2c 
issues.  I am using this driver at 50KHz because I have a LCD module 
that just doesn't work at a full 100KHz.  Please remove the check for 
these two frequency points.

>  	}
>  
>  	dev->functionality =
> @@ -175,8 +190,12 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  		I2C_FUNC_SMBUS_BYTE_DATA |
>  		I2C_FUNC_SMBUS_WORD_DATA |
>  		I2C_FUNC_SMBUS_I2C_BLOCK;
> -	dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> -		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> +	if (clk_freq == 100000)

Please change to <=

> +		dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> +			DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
> +	else
> +		dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> +			DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
>  
>  	/* Try first if we can configure the device from ACPI */
>  	r = dw_i2c_acpi_configure(pdev);
> -- 
> 1.7.1
> 
> 

Re: [PATCH] i2c designware add support of I2C standard mode

[atull]

On Wed, 20 Aug 2014, atull wrote:

> 
> 
> On Wed, 20 Aug 2014, Romain Baeriswyl wrote:
> 
> > From: Romain Baeriswyl <Romain.Baeriswyl-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
> > 
> > Some legacy devices support ony I2C standard mode at 100kHz.
> > This patch allows to select the standard mode through the DTS
> > with the use of the existing clock-frequency parameter.
> > 
> > When clock-frequency parameter is not set, the fast mode is selected.
> > Only when the parameter is set at 100000, the standard mode is selected.
> > 
> > Signed-off-by: Romain Baeriswyl <romainba-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: Christian Ruppert <christian.ruppert-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
> > ---
> >  drivers/i2c/busses/i2c-designware-platdrv.c |   23 +++++++++++++++++++++--
> >  1 files changed, 21 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> > index 402ec39..b543fe1 100644
> > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > @@ -122,6 +122,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
> >  	struct i2c_adapter *adap;
> >  	struct resource *mem;
> >  	int irq, r;
> > +	u32 clk_freq;
> >  
> >  	irq = platform_get_irq(pdev, 0);
> >  	if (irq < 0) {
> > @@ -151,6 +152,9 @@ static int dw_i2c_probe(struct platform_device *pdev)
> >  		return PTR_ERR(dev->clk);
> >  	clk_prepare_enable(dev->clk);
> >  
> > +	/* fast mode by default because of legacy reasons */
> > +	clk_freq = 400000;
> > +
> >  	if (pdev->dev.of_node) {
> >  		u32 ht = 0;
> >  		u32 ic_clk = dev->get_clk_rate_khz(dev);
> > @@ -166,6 +170,17 @@ static int dw_i2c_probe(struct platform_device *pdev)
> >  		of_property_read_u32(pdev->dev.of_node,
> >  				     "i2c-scl-falling-time-ns",
> >  				     &dev->scl_falling_time);
> > +
> > +		of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> > +				     &clk_freq);
> > +
> > +		/* Only standard mode at 100kHz and fast mode at 400kHz
> > +		 * are supported.
> > +		 */
> > +		if (clk_freq != 100000 && clk_freq != 400000) {
> > +			dev_err(&pdev->dev, "Only 100kHz and 400kHz supported");
> > +			return -EINVAL;
> > +		}
> 
> Hi Romain,
> 
> It is common to operate i2c at <100KHz on boards that have i2c 
> issues.  I am using this driver at 50KHz because I have a LCD module 
> that just doesn't work at a full 100KHz.  Please remove the check for 
> these two frequency points.
> 
> >  	}
> >  
> >  	dev->functionality =
> > @@ -175,8 +190,12 @@ static int dw_i2c_probe(struct platform_device *pdev)
> >  		I2C_FUNC_SMBUS_BYTE_DATA |
> >  		I2C_FUNC_SMBUS_WORD_DATA |
> >  		I2C_FUNC_SMBUS_I2C_BLOCK;
> > -	dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> > -		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> > +	if (clk_freq == 100000)
> 
> Please change to <=
> 

Hi Romain,

This looks fine to me, my "<= issues" are not important as this isn't 
really setting a frequency, but just setting a bit that controls speed.

Acked-by: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>


> > +		dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> > +			DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
> > +	else
> > +		dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> > +			DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> >  
> >  	/* Try first if we can configure the device from ACPI */
> >  	r = dw_i2c_acpi_configure(pdev);
> > -- 
> > 1.7.1
> > 
> > 
> 

Re: [PATCH] i2c designware add support of I2C standard mode

[Wolfram Sang]

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

On Wed, Aug 20, 2014 at 04:29:08PM +0200, Romain Baeriswyl wrote:
> From: Romain Baeriswyl <Romain.Baeriswyl-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
> 
> Some legacy devices support ony I2C standard mode at 100kHz.
> This patch allows to select the standard mode through the DTS
> with the use of the existing clock-frequency parameter.
> 
> When clock-frequency parameter is not set, the fast mode is selected.
> Only when the parameter is set at 100000, the standard mode is selected.
> 
> Signed-off-by: Romain Baeriswyl <romainba-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Christian Ruppert <christian.ruppert-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>

Applied to for-next, thanks to all involved!


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

Re: [PATCH] i2c designware add support of I2C standard mode

[atull]

On Sat, 20 Sep 2014, Wolfram Sang wrote:

> On Wed, Aug 20, 2014 at 04:29:08PM +0200, Romain Baeriswyl wrote:
> > From: Romain Baeriswyl <Romain.Baeriswyl-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
> > 
> > Some legacy devices support ony I2C standard mode at 100kHz.
> > This patch allows to select the standard mode through the DTS
> > with the use of the existing clock-frequency parameter.
> > 
> > When clock-frequency parameter is not set, the fast mode is selected.
> > Only when the parameter is set at 100000, the standard mode is selected.
> > 
> > Signed-off-by: Romain Baeriswyl <romainba-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: Christian Ruppert <christian.ruppert-ux6zf3SgZrrQT0dZR+AlfA@public.gmane.org>
> 
> Applied to for-next, thanks to all involved!
> 
> 

I don't see this in 
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git
Is this in i2c/i2c/for-next?

Alan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c designware add support of I2C standard mode

[atull]

On Wed, 20 Aug 2014, Romain Baeriswyl wrote:

> From: Romain Baeriswyl <Romain.Baeriswyl@abilis.com>
> 
> Some legacy devices support ony I2C standard mode at 100kHz.
> This patch allows to select the standard mode through the DTS
> with the use of the existing clock-frequency parameter.
> 
> When clock-frequency parameter is not set, the fast mode is selected.
> Only when the parameter is set at 100000, the standard mode is selected.
> 
> Signed-off-by: Romain Baeriswyl <romainba@abilis.com>
> Reviewed-by: Christian Ruppert <christian.ruppert@abilis.com>
> ---
>  drivers/i2c/busses/i2c-designware-platdrv.c |   23 +++++++++++++++++++++--
>  1 files changed, 21 insertions(+), 2 deletions(-)
> 

Hi Wolfram,

This patch is really useful for us.  Is it going to be picked up?

Alan