diff mbox

[v4,1/4] i2c: designware: wait for disable/enable only if necessary

Message ID 1471990736-30190-2-git-send-email-lucas.demarchi@intel.com
State Accepted
Headers show

Commit Message

Lucas De Marchi Aug. 23, 2016, 10:18 p.m. UTC
From: José Roberto de Souza <jose.souza@intel.com>

If we aren't going to continue using the controller we can just disable
it instead of waiting for it to complete. The biggest improvement here
is when a I2C transaction is completed and it doesn't block until
the adapter is disabled. When a new transfer is needed we will disable
and wait for its completion.

This way the adapter will continue changing its state in parallel to the
execution of the thread that requested the I2C transaction saving most
of the time 25~250 usec per I2C transaction.

A simple program doing a register read (1 byte write, 1 byte read)
alternating on 2 different slaves repeated 25k times for each and
measurements taken 4 times we get:

perf stat -r4 chrt -f 10 ./i2c-test /dev/i2c-1 25000 0x40 0x6 0x1e 0x00

Before:
	30.879317977 seconds time elapsed                 ( +- 14.83% )
After:
	8.638705161 seconds time elapsed                  ( +-  5.90% )

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/i2c/busses/i2c-designware-core.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

Andrey Utkin March 26, 2017, 7:44 p.m. UTC | #1
On Tue, Aug 23, 2016 at 07:18:53PM -0300, Lucas De Marchi wrote:
> From: José Roberto de Souza <jose.souza@intel.com>
> 
> If we aren't going to continue using the controller we can just disable
> it instead of waiting for it to complete. The biggest improvement here
> is when a I2C transaction is completed and it doesn't block until
> the adapter is disabled. When a new transfer is needed we will disable
> and wait for its completion.
> 
> This way the adapter will continue changing its state in parallel to the
> execution of the thread that requested the I2C transaction saving most
> of the time 25~250 usec per I2C transaction.
> 
> A simple program doing a register read (1 byte write, 1 byte read)
> alternating on 2 different slaves repeated 25k times for each and
> measurements taken 4 times we get:
> 
> perf stat -r4 chrt -f 10 ./i2c-test /dev/i2c-1 25000 0x40 0x6 0x1e 0x00
> 
> Before:
> 	30.879317977 seconds time elapsed                 ( +- 14.83% )
> After:
> 	8.638705161 seconds time elapsed                  ( +-  5.90% )
> 
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/i2c/busses/i2c-designware-core.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
> index 99b54be..2c61585 100644
> --- a/drivers/i2c/busses/i2c-designware-core.c
> +++ b/drivers/i2c/busses/i2c-designware-core.c
> @@ -252,10 +252,15 @@ static u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
>  
>  static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable)
>  {
> +	dw_writel(dev, enable, DW_IC_ENABLE);
> +}
> +
> +static void __i2c_dw_enable_and_wait(struct dw_i2c_dev *dev, bool enable)
> +{
>  	int timeout = 100;
>  
>  	do {
> -		dw_writel(dev, enable, DW_IC_ENABLE);
> +		__i2c_dw_enable(dev, enable);
>  		if ((dw_readl(dev, DW_IC_ENABLE_STATUS) & 1) == enable)
>  			return;
>  
> @@ -321,7 +326,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
>  	}
>  
>  	/* Disable the adapter */
> -	__i2c_dw_enable(dev, false);
> +	__i2c_dw_enable_and_wait(dev, false);
>  
>  	/* set standard and fast speed deviders for high/low periods */
>  
> @@ -414,7 +419,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
>  	u32 ic_con, ic_tar = 0;
>  
>  	/* Disable the adapter */
> -	__i2c_dw_enable(dev, false);
> +	__i2c_dw_enable_and_wait(dev, false);
>  
>  	/* if the slave address is ten bit address, enable 10BITADDR */
>  	ic_con = dw_readl(dev, DW_IC_CON);
> @@ -833,7 +838,7 @@ tx_aborted:
>  void i2c_dw_disable(struct dw_i2c_dev *dev)
>  {
>  	/* Disable controller */
> -	__i2c_dw_enable(dev, false);
> +	__i2c_dw_enable_and_wait(dev, false);
>  
>  	/* Disable all interupts */
>  	dw_writel(dev, 0, DW_IC_INTR_MASK);
> -- 
> 2.7.4
> 

Regression reported: https://bugzilla.kernel.org/show_bug.cgi?id=194969
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jarkko Nikula March 27, 2017, 8:29 a.m. UTC | #2
On 03/26/2017 10:44 PM, Andrey Utkin wrote:
> Regression reported: https://bugzilla.kernel.org/show_bug.cgi?id=194969
>
Thanks, I commented the bug.
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 99b54be..2c61585 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -252,10 +252,15 @@  static u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
 
 static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable)
 {
+	dw_writel(dev, enable, DW_IC_ENABLE);
+}
+
+static void __i2c_dw_enable_and_wait(struct dw_i2c_dev *dev, bool enable)
+{
 	int timeout = 100;
 
 	do {
-		dw_writel(dev, enable, DW_IC_ENABLE);
+		__i2c_dw_enable(dev, enable);
 		if ((dw_readl(dev, DW_IC_ENABLE_STATUS) & 1) == enable)
 			return;
 
@@ -321,7 +326,7 @@  int i2c_dw_init(struct dw_i2c_dev *dev)
 	}
 
 	/* Disable the adapter */
-	__i2c_dw_enable(dev, false);
+	__i2c_dw_enable_and_wait(dev, false);
 
 	/* set standard and fast speed deviders for high/low periods */
 
@@ -414,7 +419,7 @@  static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
 	u32 ic_con, ic_tar = 0;
 
 	/* Disable the adapter */
-	__i2c_dw_enable(dev, false);
+	__i2c_dw_enable_and_wait(dev, false);
 
 	/* if the slave address is ten bit address, enable 10BITADDR */
 	ic_con = dw_readl(dev, DW_IC_CON);
@@ -833,7 +838,7 @@  tx_aborted:
 void i2c_dw_disable(struct dw_i2c_dev *dev)
 {
 	/* Disable controller */
-	__i2c_dw_enable(dev, false);
+	__i2c_dw_enable_and_wait(dev, false);
 
 	/* Disable all interupts */
 	dw_writel(dev, 0, DW_IC_INTR_MASK);