i2c: designware: Don't set SCL timings and speed mode when in slave mode

Message ID 20170823134612.26117-1-jarkko.nikula@linux.intel.com
State Needs Review / ACK
Headers show

Commit Message

Jarkko Nikula Aug. 23, 2017, 1:46 p.m.
According to data sheet SCL timing parameters and DW_IC_CON SPEED mode
bits are not used when operating in slave mode.

Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
I've done quick testing with i2c-slave-eeprom and i2cdump over together
hooked busses without issues. Would like to hear tested by or comment
from Luis though. Patch is not urgent so can wait after vacation etc.
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 11 -----
 drivers/i2c/busses/i2c-designware-slave.c   | 64 -----------------------------
 2 files changed, 75 deletions(-)

Comments

Wolfram Sang Oct. 5, 2017, 11:18 a.m. | #1
On Wed, Aug 23, 2017 at 04:46:12PM +0300, Jarkko Nikula wrote:
> According to data sheet SCL timing parameters and DW_IC_CON SPEED mode
> bits are not used when operating in slave mode.
> 
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> ---
> I've done quick testing with i2c-slave-eeprom and i2cdump over together
> hooked busses without issues. Would like to hear tested by or comment
> from Luis though. Patch is not urgent so can wait after vacation etc.

Luis?

> ---
>  drivers/i2c/busses/i2c-designware-platdrv.c | 11 -----
>  drivers/i2c/busses/i2c-designware-slave.c   | 64 -----------------------------
>  2 files changed, 75 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 56a17fdc2270..962c7ab25b93 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -201,17 +201,6 @@ static void i2c_dw_configure_slave(struct dw_i2c_dev *dev)
>  			 DW_IC_CON_RESTART_EN | DW_IC_CON_STOP_DET_IFADDRESSED;
>  
>  	dev->mode = DW_IC_SLAVE;
> -
> -	switch (dev->clk_freq) {
> -	case 100000:
> -		dev->slave_cfg |= DW_IC_CON_SPEED_STD;
> -		break;
> -	case 3400000:
> -		dev->slave_cfg |= DW_IC_CON_SPEED_HIGH;
> -		break;
> -	default:
> -		dev->slave_cfg |= DW_IC_CON_SPEED_FAST;
> -	}
>  }
>  
>  static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare)
> diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
> index ea9578ab19a1..d42558d1b002 100644
> --- a/drivers/i2c/busses/i2c-designware-slave.c
> +++ b/drivers/i2c/busses/i2c-designware-slave.c
> @@ -51,9 +51,7 @@ static void i2c_dw_configure_fifo_slave(struct dw_i2c_dev *dev)
>   */
>  static int i2c_dw_init_slave(struct dw_i2c_dev *dev)
>  {
> -	u32 sda_falling_time, scl_falling_time;
>  	u32 reg, comp_param1;
> -	u32 hcnt, lcnt;
>  	int ret;
>  
>  	ret = i2c_dw_acquire_lock(dev);
> @@ -79,68 +77,6 @@ static int i2c_dw_init_slave(struct dw_i2c_dev *dev)
>  	/* Disable the adapter. */
>  	__i2c_dw_enable_and_wait(dev, false);
>  
> -	/* Set standard and fast speed deviders for high/low periods. */
> -	sda_falling_time = dev->sda_falling_time ?: 300; /* ns */
> -	scl_falling_time = dev->scl_falling_time ?: 300; /* ns */
> -
> -	/* Set SCL timing parameters for standard-mode. */
> -	if (dev->ss_hcnt && dev->ss_lcnt) {
> -		hcnt = dev->ss_hcnt;
> -		lcnt = dev->ss_lcnt;
> -	} else {
> -		hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev),
> -				       4000,	/* tHD;STA = tHIGH = 4.0 us */
> -				       sda_falling_time,
> -				       0,	/* 0: DW default, 1: Ideal */
> -				       0);	/* No offset */
> -		lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev),
> -				       4700,	/* tLOW = 4.7 us */
> -				       scl_falling_time,
> -				       0);	/* No offset */
> -	}
> -	dw_writel(dev, hcnt, DW_IC_SS_SCL_HCNT);
> -	dw_writel(dev, lcnt, DW_IC_SS_SCL_LCNT);
> -	dev_dbg(dev->dev, "Standard-mode HCNT:LCNT = %d:%d\n", hcnt, lcnt);
> -
> -	/* Set SCL timing parameters for fast-mode or fast-mode plus. */
> -	if ((dev->clk_freq == 1000000) && dev->fp_hcnt && dev->fp_lcnt) {
> -		hcnt = dev->fp_hcnt;
> -		lcnt = dev->fp_lcnt;
> -	} else if (dev->fs_hcnt && dev->fs_lcnt) {
> -		hcnt = dev->fs_hcnt;
> -		lcnt = dev->fs_lcnt;
> -	} else {
> -		hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev),
> -				       600,	/* tHD;STA = tHIGH = 0.6 us */
> -				       sda_falling_time,
> -				       0,	/* 0: DW default, 1: Ideal */
> -				       0);	/* No offset */
> -		lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev),
> -				       1300,	/* tLOW = 1.3 us */
> -				       scl_falling_time,
> -				       0);	/* No offset */
> -	}
> -	dw_writel(dev, hcnt, DW_IC_FS_SCL_HCNT);
> -	dw_writel(dev, lcnt, DW_IC_FS_SCL_LCNT);
> -	dev_dbg(dev->dev, "Fast-mode HCNT:LCNT = %d:%d\n", hcnt, lcnt);
> -
> -	if ((dev->slave_cfg & DW_IC_CON_SPEED_MASK) ==
> -		DW_IC_CON_SPEED_HIGH) {
> -		if ((comp_param1 & DW_IC_COMP_PARAM_1_SPEED_MODE_MASK)
> -			!= DW_IC_COMP_PARAM_1_SPEED_MODE_HIGH) {
> -			dev_err(dev->dev, "High Speed not supported!\n");
> -			dev->slave_cfg &= ~DW_IC_CON_SPEED_MASK;
> -			dev->slave_cfg |= DW_IC_CON_SPEED_FAST;
> -		} else if (dev->hs_hcnt && dev->hs_lcnt) {
> -			hcnt = dev->hs_hcnt;
> -			lcnt = dev->hs_lcnt;
> -			dw_writel(dev, hcnt, DW_IC_HS_SCL_HCNT);
> -			dw_writel(dev, lcnt, DW_IC_HS_SCL_LCNT);
> -			dev_dbg(dev->dev, "HighSpeed-mode HCNT:LCNT = %d:%d\n",
> -				hcnt, lcnt);
> -		}
> -	}
> -
>  	/* Configure SDA Hold Time if required. */
>  	reg = dw_readl(dev, DW_IC_COMP_VERSION);
>  	if (reg >= DW_IC_SDA_HOLD_MIN_VERS) {
> -- 
> 2.14.1
>
Jarkko Nikula Oct. 19, 2017, 9:32 a.m. | #2
Hi Luis

On 10/05/2017 02:18 PM, Wolfram Sang wrote:
> On Wed, Aug 23, 2017 at 04:46:12PM +0300, Jarkko Nikula wrote:
>> According to data sheet SCL timing parameters and DW_IC_CON SPEED mode
>> bits are not used when operating in slave mode.
>>
>> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
>> ---
>> I've done quick testing with i2c-slave-eeprom and i2cdump over together
>> hooked busses without issues. Would like to hear tested by or comment
>> from Luis though. Patch is not urgent so can wait after vacation etc.
> 
> Luis?
> 
Do you have possibility test my patch it doesn't break anything?
Luis Oliveira Oct. 19, 2017, 11:52 a.m. | #3
Hi Jarkko

I will try to have my hardware available asap. But for what I saw it should run
smoothly.

On 19-Oct-17 10:32, Jarkko Nikula wrote:
> Hi Luis
> 
> On 10/05/2017 02:18 PM, Wolfram Sang wrote:
>> On Wed, Aug 23, 2017 at 04:46:12PM +0300, Jarkko Nikula wrote:
>>> According to data sheet SCL timing parameters and DW_IC_CON SPEED mode
>>> bits are not used when operating in slave mode.
>>>
>>> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
>>> ---
>>> I've done quick testing with i2c-slave-eeprom and i2cdump over together
>>> hooked busses without issues. Would like to hear tested by or comment
>>> from Luis though. Patch is not urgent so can wait after vacation etc.
>>
>> Luis?
>>
> Do you have possibility test my patch it doesn't break anything?
> 

Regards,
Luis
Jarkko Nikula Nov. 14, 2017, 11:17 a.m. | #4
Hi Luis

On 10/19/2017 02:52 PM, Luis Oliveira wrote:
> Hi Jarkko
> 
> I will try to have my hardware available asap. But for what I saw it should run
> smoothly.
> 
Were you able to test my patch ?

Patch

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 56a17fdc2270..962c7ab25b93 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -201,17 +201,6 @@  static void i2c_dw_configure_slave(struct dw_i2c_dev *dev)
 			 DW_IC_CON_RESTART_EN | DW_IC_CON_STOP_DET_IFADDRESSED;
 
 	dev->mode = DW_IC_SLAVE;
-
-	switch (dev->clk_freq) {
-	case 100000:
-		dev->slave_cfg |= DW_IC_CON_SPEED_STD;
-		break;
-	case 3400000:
-		dev->slave_cfg |= DW_IC_CON_SPEED_HIGH;
-		break;
-	default:
-		dev->slave_cfg |= DW_IC_CON_SPEED_FAST;
-	}
 }
 
 static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare)
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index ea9578ab19a1..d42558d1b002 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -51,9 +51,7 @@  static void i2c_dw_configure_fifo_slave(struct dw_i2c_dev *dev)
  */
 static int i2c_dw_init_slave(struct dw_i2c_dev *dev)
 {
-	u32 sda_falling_time, scl_falling_time;
 	u32 reg, comp_param1;
-	u32 hcnt, lcnt;
 	int ret;
 
 	ret = i2c_dw_acquire_lock(dev);
@@ -79,68 +77,6 @@  static int i2c_dw_init_slave(struct dw_i2c_dev *dev)
 	/* Disable the adapter. */
 	__i2c_dw_enable_and_wait(dev, false);
 
-	/* Set standard and fast speed deviders for high/low periods. */
-	sda_falling_time = dev->sda_falling_time ?: 300; /* ns */
-	scl_falling_time = dev->scl_falling_time ?: 300; /* ns */
-
-	/* Set SCL timing parameters for standard-mode. */
-	if (dev->ss_hcnt && dev->ss_lcnt) {
-		hcnt = dev->ss_hcnt;
-		lcnt = dev->ss_lcnt;
-	} else {
-		hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev),
-				       4000,	/* tHD;STA = tHIGH = 4.0 us */
-				       sda_falling_time,
-				       0,	/* 0: DW default, 1: Ideal */
-				       0);	/* No offset */
-		lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev),
-				       4700,	/* tLOW = 4.7 us */
-				       scl_falling_time,
-				       0);	/* No offset */
-	}
-	dw_writel(dev, hcnt, DW_IC_SS_SCL_HCNT);
-	dw_writel(dev, lcnt, DW_IC_SS_SCL_LCNT);
-	dev_dbg(dev->dev, "Standard-mode HCNT:LCNT = %d:%d\n", hcnt, lcnt);
-
-	/* Set SCL timing parameters for fast-mode or fast-mode plus. */
-	if ((dev->clk_freq == 1000000) && dev->fp_hcnt && dev->fp_lcnt) {
-		hcnt = dev->fp_hcnt;
-		lcnt = dev->fp_lcnt;
-	} else if (dev->fs_hcnt && dev->fs_lcnt) {
-		hcnt = dev->fs_hcnt;
-		lcnt = dev->fs_lcnt;
-	} else {
-		hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev),
-				       600,	/* tHD;STA = tHIGH = 0.6 us */
-				       sda_falling_time,
-				       0,	/* 0: DW default, 1: Ideal */
-				       0);	/* No offset */
-		lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev),
-				       1300,	/* tLOW = 1.3 us */
-				       scl_falling_time,
-				       0);	/* No offset */
-	}
-	dw_writel(dev, hcnt, DW_IC_FS_SCL_HCNT);
-	dw_writel(dev, lcnt, DW_IC_FS_SCL_LCNT);
-	dev_dbg(dev->dev, "Fast-mode HCNT:LCNT = %d:%d\n", hcnt, lcnt);
-
-	if ((dev->slave_cfg & DW_IC_CON_SPEED_MASK) ==
-		DW_IC_CON_SPEED_HIGH) {
-		if ((comp_param1 & DW_IC_COMP_PARAM_1_SPEED_MODE_MASK)
-			!= DW_IC_COMP_PARAM_1_SPEED_MODE_HIGH) {
-			dev_err(dev->dev, "High Speed not supported!\n");
-			dev->slave_cfg &= ~DW_IC_CON_SPEED_MASK;
-			dev->slave_cfg |= DW_IC_CON_SPEED_FAST;
-		} else if (dev->hs_hcnt && dev->hs_lcnt) {
-			hcnt = dev->hs_hcnt;
-			lcnt = dev->hs_lcnt;
-			dw_writel(dev, hcnt, DW_IC_HS_SCL_HCNT);
-			dw_writel(dev, lcnt, DW_IC_HS_SCL_LCNT);
-			dev_dbg(dev->dev, "HighSpeed-mode HCNT:LCNT = %d:%d\n",
-				hcnt, lcnt);
-		}
-	}
-
 	/* Configure SDA Hold Time if required. */
 	reg = dw_readl(dev, DW_IC_COMP_VERSION);
 	if (reg >= DW_IC_SDA_HOLD_MIN_VERS) {