diff mbox series

i2c: designware: fix master is holding SCL low while ENABLE bit is disabled

Message ID 20240905074211.2278-1-kimriver.liu@siengine.com
State Changes Requested
Delegated to: Andi Shyti
Headers show
Series i2c: designware: fix master is holding SCL low while ENABLE bit is disabled | expand

Commit Message

Liu Kimriver/刘金河 Sept. 5, 2024, 7:42 a.m. UTC 
From: "kimriver.liu" <kimriver.liu@siengine.com>

Failure in normal Stop operational path

This failure happens rarely and is hard to reproduce. Debug trace
showed that IC_STATUS had value of 0x23 when STOP_DET occurred,
immediately disable ENABLE bit that can result in
IC_RAW_INTR_STAT.MASTER_ON_HOLD holding SCL low.

Failure in ENABLE bit is disabled path

It was observed that master is holding SCL low and the IC_ENABLE is
already disabled, Enable ABORT bit and ENABLE bit simultaneously
cannot take effect.

Check if the master is holding SCL low after ENABLE bit is already
disabled. If SCL is held low, The software can set this ABORT bit only
when ENABLE is already set，otherwise,
the controller ignores any write to ABORT bit. When the abort is done,
then proceed with disabling the controller.

These kernel logs show up whenever an I2C transaction is attempted
after this failure.
i2c_designware e95e0000.i2c: timeout in disabling adapter
i2c_designware e95e0000.i2c: timeout waiting for bus ready

The patch can be fix the controller cannot be disabled while SCL is
held low in ENABLE bit is already disabled.

Signed-off-by: kimriver.liu <kimriver.liu@siengine.com>
---
 drivers/i2c/busses/i2c-designware-common.c | 12 +++++++++++
 drivers/i2c/busses/i2c-designware-master.c | 23 +++++++++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

Comments

Andy Shevchenko Sept. 5, 2024, 11:03 a.m. UTC | #1 
On Thu, Sep 05, 2024 at 03:42:11PM +0800, kimriver liu wrote:
> From: "kimriver.liu" <kimriver.liu@siengine.com>

You forgot bumping patch version in the Subject and now it's quite confusing.

> Failure in normal Stop operational path

Is this a subsection?
Make it more clear, by using additional formatting, like

Failure in normal Stop operational path
---------------------------------------

> This failure happens rarely and is hard to reproduce. Debug trace
> showed that IC_STATUS had value of 0x23 when STOP_DET occurred,
> immediately disable ENABLE bit that can result in
> IC_RAW_INTR_STAT.MASTER_ON_HOLD holding SCL low.

> Failure in ENABLE bit is disabled path

Ditto.

> It was observed that master is holding SCL low and the IC_ENABLE is
> already disabled, Enable ABORT bit and ENABLE bit simultaneously
> cannot take effect.
> 
> Check if the master is holding SCL low after ENABLE bit is already
> disabled. If SCL is held low, The software can set this ABORT bit only
> when ENABLE is already set，otherwise,
> the controller ignores any write to ABORT bit. When the abort is done,
> then proceed with disabling the controller.
> 
> These kernel logs show up whenever an I2C transaction is attempted
> after this failure.
> i2c_designware e95e0000.i2c: timeout in disabling adapter
> i2c_designware e95e0000.i2c: timeout waiting for bus ready
> 
> The patch can be fix the controller cannot be disabled while SCL is
> held low in ENABLE bit is already disabled.
> 
> Signed-off-by: kimriver.liu <kimriver.liu@siengine.com>
> ---

Here is the place for comments and changelog.
Since it's not the first version of the patch, changelog is a must.
Andi Shyti Sept. 5, 2024, 7:49 p.m. UTC | #2 
Hi Kimriver,

On Thu, Sep 05, 2024 at 03:42:11PM GMT, kimriver liu wrote:
> From: "kimriver.liu" <kimriver.liu@siengine.com>

Is there any reason to have "kimriver.liu" instead of "Kimriver
Liu"?

> Failure in normal Stop operational path
> 
> This failure happens rarely and is hard to reproduce. Debug trace
> showed that IC_STATUS had value of 0x23 when STOP_DET occurred,
> immediately disable ENABLE bit that can result in
> IC_RAW_INTR_STAT.MASTER_ON_HOLD holding SCL low.
> 
> Failure in ENABLE bit is disabled path
> 
> It was observed that master is holding SCL low and the IC_ENABLE is
> already disabled, Enable ABORT bit and ENABLE bit simultaneously
> cannot take effect.
> 
> Check if the master is holding SCL low after ENABLE bit is already
> disabled. If SCL is held low, The software can set this ABORT bit only
> when ENABLE is already set，otherwise,
> the controller ignores any write to ABORT bit. When the abort is done,
> then proceed with disabling the controller.
> 
> These kernel logs show up whenever an I2C transaction is attempted
> after this failure.
> i2c_designware e95e0000.i2c: timeout in disabling adapter
> i2c_designware e95e0000.i2c: timeout waiting for bus ready
> 
> The patch can be fix the controller cannot be disabled while SCL is
> held low in ENABLE bit is already disabled.

I'm sorry, but this commit log is difficult to understand. Could
you please polish it, fix the grammar and the punctuation and
make it more understandable?

> Signed-off-by: kimriver.liu <kimriver.liu@siengine.com>

As I said above, would be nicer to have "Kimriver Liu" rather
than "kimriver.liu".

I'm wondering if we need the Fixes: tag here. If this is not a
frequent issue then we can probably omit it.

> ---

Andy already pointed out that you are missing versioning (is this
v2? v3?) and changelog.

>  drivers/i2c/busses/i2c-designware-common.c | 12 +++++++++++
>  drivers/i2c/busses/i2c-designware-master.c | 23 +++++++++++++++++++++-
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
> index e8a688d04aee..54acf8554582 100644
> --- a/drivers/i2c/busses/i2c-designware-common.c
> +++ b/drivers/i2c/busses/i2c-designware-common.c
> @@ -453,6 +453,18 @@ void __i2c_dw_disable(struct dw_i2c_dev *dev)
>  
>  	abort_needed = raw_intr_stats & DW_IC_INTR_MST_ON_HOLD;
>  	if (abort_needed) {
> +		if (!(enable & DW_IC_ENABLE_ENABLE)) {
> +			regmap_write(dev->map, DW_IC_ENABLE, DW_IC_ENABLE_ENABLE);
> +			enable |= DW_IC_ENABLE_ENABLE;
> +
> +			/*
> +			 * Wait two ic_clk delay when enabling the i2c to ensure ENABLE bit
> +			 * is already set by the driver (for 400KHz this is 25us)
> +			 * as described in the DesignWare I2C databook.
> +			 */
> +			fsleep(25);
> +		}
> +
>  		regmap_write(dev->map, DW_IC_ENABLE, enable | DW_IC_ENABLE_ABORT);
>  		ret = regmap_read_poll_timeout(dev->map, DW_IC_ENABLE, enable,
>  					       !(enable & DW_IC_ENABLE_ABORT), 10,
> diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
> index c7e56002809a..aba0b8fdfe9a 100644
> --- a/drivers/i2c/busses/i2c-designware-master.c
> +++ b/drivers/i2c/busses/i2c-designware-master.c
> @@ -253,6 +253,26 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
>  	__i2c_dw_write_intr_mask(dev, DW_IC_INTR_MASTER_MASK);
>  }
>  
> +static bool i2c_dw_is_master_idling(struct dw_i2c_dev *dev)
> +{
> +	u32 status;
> +	int ret;
> +
> +	regmap_read(dev->map, DW_IC_STATUS, &status);
> +	if (!(status & DW_IC_STATUS_MASTER_ACTIVITY))
> +		return true;
> +
> +	ret = regmap_read_poll_timeout(dev->map, DW_IC_STATUS, status,
> +			!(status & DW_IC_STATUS_MASTER_ACTIVITY),
> +			1100, 20000);
> +	if (ret) {
> +		dev_err(dev->dev, "i2c master controller not idle %d\n", ret);

Please be a bit more descriptive. It's not an error for the i2c
master not to be idle, it is if you try to disable the adapter.

Besides, it makes sense to me that this print is done in the
i2c_dw_xfer function rather than here. The task of this function
is only to check whether the controller is idling or active.

> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  static int i2c_dw_check_stopbit(struct dw_i2c_dev *dev)
>  {
>  	u32 val;
> @@ -796,7 +816,8 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
>  	 * additional interrupts are a hardware bug or this driver doesn't
>  	 * handle them correctly yet.
>  	 */

Please update the comment according to your change.

> -	__i2c_dw_disable_nowait(dev);
> +	if (i2c_dw_is_master_idling(dev))

Please print the error message here.

Thanks for your patch,
Andi

> +		__i2c_dw_disable_nowait(dev);
>  
>  	if (dev->msg_err) {
>  		ret = dev->msg_err;
> -- 
> 2.17.1
>
kernel test robot Sept. 8, 2024, 1:31 p.m. UTC | #3 
Hi kimriver,

kernel test robot noticed the following build errors:

[auto build test ERROR on andi-shyti/i2c/i2c-host]
[also build test ERROR on linus/master v6.11-rc6 next-20240906]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/kimriver-liu/i2c-designware-fix-master-is-holding-SCL-low-while-ENABLE-bit-is-disabled/20240905-154711
base:   https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host
patch link:    https://lore.kernel.org/r/20240905074211.2278-1-kimriver.liu%40siengine.com
patch subject: [PATCH] i2c: designware: fix master is holding SCL low while ENABLE bit is disabled
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20240908/202409082011.9JF6aYsk-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240908/202409082011.9JF6aYsk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409082011.9JF6aYsk-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/i2c/busses/i2c-designware-common.c: In function '__i2c_dw_disable':
>> drivers/i2c/busses/i2c-designware-common.c:538:32: error: 'DW_IC_ENABLE_ENABLE' undeclared (first use in this function); did you mean 'DW_IC_ENABLE_STATUS'?
     538 |                 if (!(enable & DW_IC_ENABLE_ENABLE)) {
         |                                ^~~~~~~~~~~~~~~~~~~
         |                                DW_IC_ENABLE_STATUS
   drivers/i2c/busses/i2c-designware-common.c:538:32: note: each undeclared identifier is reported only once for each function it appears in


vim +538 drivers/i2c/busses/i2c-designware-common.c

   523	
   524	void __i2c_dw_disable(struct dw_i2c_dev *dev)
   525	{
   526		unsigned int raw_intr_stats;
   527		unsigned int enable;
   528		int timeout = 100;
   529		bool abort_needed;
   530		unsigned int status;
   531		int ret;
   532	
   533		regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_intr_stats);
   534		regmap_read(dev->map, DW_IC_ENABLE, &enable);
   535	
   536		abort_needed = raw_intr_stats & DW_IC_INTR_MST_ON_HOLD;
   537		if (abort_needed) {
 > 538			if (!(enable & DW_IC_ENABLE_ENABLE)) {
   539				regmap_write(dev->map, DW_IC_ENABLE, DW_IC_ENABLE_ENABLE);
   540				enable |= DW_IC_ENABLE_ENABLE;
   541	
   542				/*
   543				 * Wait two ic_clk delay when enabling the i2c to ensure ENABLE bit
   544				 * is already set by the driver (for 400KHz this is 25us)
   545				 * as described in the DesignWare I2C databook.
   546				 */
   547				fsleep(25);
   548			}
   549	
   550			regmap_write(dev->map, DW_IC_ENABLE, enable | DW_IC_ENABLE_ABORT);
   551			ret = regmap_read_poll_timeout(dev->map, DW_IC_ENABLE, enable,
   552						       !(enable & DW_IC_ENABLE_ABORT), 10,
   553						       100);
   554			if (ret)
   555				dev_err(dev->dev, "timeout while trying to abort current transfer\n");
   556		}
   557	
   558		do {
   559			__i2c_dw_disable_nowait(dev);
   560			/*
   561			 * The enable status register may be unimplemented, but
   562			 * in that case this test reads zero and exits the loop.
   563			 */
   564			regmap_read(dev->map, DW_IC_ENABLE_STATUS, &status);
   565			if ((status & 1) == 0)
   566				return;
   567	
   568			/*
   569			 * Wait 10 times the signaling period of the highest I2C
   570			 * transfer supported by the driver (for 400KHz this is
   571			 * 25us) as described in the DesignWare I2C databook.
   572			 */
   573			usleep_range(25, 250);
   574		} while (timeout--);
   575	
   576		dev_warn(dev->dev, "timeout in disabling adapter\n");
   577	}
   578
Liu Kimriver/刘金河 Sept. 9, 2024, 1:31 a.m. UTC | #4 
HI，robot,
 Today I will resend a version v7 patch([PATCH V7] i2c: designware: fix master is holding SCL low while ENABLE bit is disabled)
 based on the latest Linux master branch to resolve compile errors.

-------------
Best Regards,
Kimriver Liu

-----Original Message-----
From: kernel test robot <lkp@intel.com> 
Sent: 2024年9月8日 21:32
To: Liu Kimriver/刘金河 <kimriver.liu@siengine.com>; jarkko.nikula@linux.intel.com
Cc: oe-kbuild-all@lists.linux.dev; andriy.shevchenko@linux.intel.com; mika.westerberg@linux.intel.com; jsd@semihalf.com; andi.shyti@kernel.org; linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; Liu Kimriver/刘金河 <kimriver.liu@siengine.com>
Subject: Re: [PATCH] i2c: designware: fix master is holding SCL low while ENABLE bit is disabled

Hi kimriver,

kernel test robot noticed the following build errors:

[auto build test ERROR on andi-shyti/i2c/i2c-host]
[also build test ERROR on linus/master v6.11-rc6 next-20240906]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/kimriver-liu/i2c-designware-fix-master-is-holding-SCL-low-while-ENABLE-bit-is-disabled/20240905-154711
base:   https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host
patch link:    https://lore.kernel.org/r/20240905074211.2278-1-kimriver.liu%40siengine.com
patch subject: [PATCH] i2c: designware: fix master is holding SCL low while ENABLE bit is disabled
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20240908/202409082011.9JF6aYsk-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240908/202409082011.9JF6aYsk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409082011.9JF6aYsk-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/i2c/busses/i2c-designware-common.c: In function '__i2c_dw_disable':
>> drivers/i2c/busses/i2c-designware-common.c:538:32: error: 'DW_IC_ENABLE_ENABLE' undeclared (first use in this function); did you mean 'DW_IC_ENABLE_STATUS'?
     538 |                 if (!(enable & DW_IC_ENABLE_ENABLE)) {
         |                                ^~~~~~~~~~~~~~~~~~~
         |                                DW_IC_ENABLE_STATUS
   drivers/i2c/busses/i2c-designware-common.c:538:32: note: each undeclared identifier is reported only once for each function it appears in


vim +538 drivers/i2c/busses/i2c-designware-common.c

   523	
   524	void __i2c_dw_disable(struct dw_i2c_dev *dev)
   525	{
   526		unsigned int raw_intr_stats;
   527		unsigned int enable;
   528		int timeout = 100;
   529		bool abort_needed;
   530		unsigned int status;
   531		int ret;
   532	
   533		regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_intr_stats);
   534		regmap_read(dev->map, DW_IC_ENABLE, &enable);
   535	
   536		abort_needed = raw_intr_stats & DW_IC_INTR_MST_ON_HOLD;
   537		if (abort_needed) {
 > 538			if (!(enable & DW_IC_ENABLE_ENABLE)) {
   539				regmap_write(dev->map, DW_IC_ENABLE, DW_IC_ENABLE_ENABLE);
   540				enable |= DW_IC_ENABLE_ENABLE;
   541	
   542				/*
   543				 * Wait two ic_clk delay when enabling the i2c to ensure ENABLE bit
   544				 * is already set by the driver (for 400KHz this is 25us)
   545				 * as described in the DesignWare I2C databook.
   546				 */
   547				fsleep(25);
   548			}
   549	
   550			regmap_write(dev->map, DW_IC_ENABLE, enable | DW_IC_ENABLE_ABORT);
   551			ret = regmap_read_poll_timeout(dev->map, DW_IC_ENABLE, enable,
   552						       !(enable & DW_IC_ENABLE_ABORT), 10,
   553						       100);
   554			if (ret)
   555				dev_err(dev->dev, "timeout while trying to abort current transfer\n");
   556		}
   557	
   558		do {
   559			__i2c_dw_disable_nowait(dev);
   560			/*
   561			 * The enable status register may be unimplemented, but
   562			 * in that case this test reads zero and exits the loop.
   563			 */
   564			regmap_read(dev->map, DW_IC_ENABLE_STATUS, &status);
   565			if ((status & 1) == 0)
   566				return;
   567	
   568			/*
   569			 * Wait 10 times the signaling period of the highest I2C
   570			 * transfer supported by the driver (for 400KHz this is
   571			 * 25us) as described in the DesignWare I2C databook.
   572			 */
   573			usleep_range(25, 250);
   574		} while (timeout--);
   575	
   576		dev_warn(dev->dev, "timeout in disabling adapter\n");
   577	}
   578
Liu Kimriver/刘金河 Sept. 9, 2024, 6:50 a.m. UTC | #5 
Hi jarkko 

-----Original Message-----
From: kernel test robot <lkp@intel.com> 
Sent: 2024年9月8日 21:32
To: Liu Kimriver/刘金河 <kimriver.liu@siengine.com>; jarkko.nikula@linux.intel.com
Cc: oe-kbuild-all@lists.linux.dev; andriy.shevchenko@linux.intel.com; mika.westerberg@linux.intel.com; jsd@semihalf.com; andi.shyti@kernel.org; linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; Liu Kimriver/刘金河 <kimriver.liu@siengine.com>
Subject: Re: [PATCH] i2c: designware: fix master is holding SCL low while ENABLE bit is disabled

>>Hi kimriver,

>>kernel test robot noticed the following build errors:

>> [auto build test ERROR on andi-shyti/i2c/i2c-host]
>> [also build test ERROR on linus/master v6.11-rc6 next-20240906]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>>And when submitting patch, we suggest to use '--base' as documented in
>>https://git-scm.com/docs/git-format-patch#_base_tree_information]


 I applied patch to the wrong git tree last week , I had resent V7 patch To resolve build test ERROR.
 patch Subject: [PATCH v7] i2c: designware: fix master is holding SCL low while ENABLE bit is disabled
 New patch link: 
  https://lore.kernel.org/all/20240909015646.2285-1-kimriver.liu@siengine.com/
The patch rebase on the latest Linux v6.11.0-rc6 (89f5e14d05b) branch (the latest linux master branch)


>>url:    https://github.com/intel-lab-lkp/linux/commits/kimriver-liu/i2c-designware-fix-master-is-holding-SCL-low-while-ENABLE-bit-is-disabled/20240905-154711
>>base:   https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host
>>patch link:    https://lore.kernel.org/r/20240905074211.2278-1-kimriver.liu%40siengine.com
>>patch subject: [PATCH] i2c: designware: fix master is holding SCL low while ENABLE bit is disabled
>>config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20240908/202409082011.9JF6aYsk-lkp@intel.com/config)
>>compiler: sh4-linux-gcc (GCC) 14.1.0
>>reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240908/202409082011.9JF6aYsk-lkp@intel.com/reproduce)

>>If you fix the issue in a separate patch/commit (i.e. not just a new version of
>>the same patch/commit), kindly add following tags
>>| Reported-by: kernel test robot <lkp@intel.com>
>>| Closes: https://lore.kernel.org/oe-kbuild-all/202409082011.9JF6aYsk-lkp@intel.com/

>>All errors (new ones prefixed by >>):

>>   drivers/i2c/busses/i2c-designware-common.c: In function '__i2c_dw_disable':
>>>> drivers/i2c/busses/i2c-designware-common.c:538:32: error: 'DW_IC_ENABLE_ENABLE' undeclared (first use in this function); did you mean 'DW_IC_ENABLE_STATUS'?
>>     538 |                 if (!(enable & DW_IC_ENABLE_ENABLE)) {
>>         |                                ^~~~~~~~~~~~~~~~~~~
>>         |                                DW_IC_ENABLE_STATUS
>>   drivers/i2c/busses/i2c-designware-common.c:538:32: note: each undeclared identifier is reported only once for each function it appears in


I fixexd the issue at the same commit as In V7 version patch link:  
   https://lore.kernel.org/all/20240909015646.2285-1-kimriver.liu@siengine.com/ 
patch Subject: [PATCH v7] i2c: designware: fix master is holding SCL low while ENABLE bit is disabled
Declaration DW_IC_ENABLE_ENABLE as follow:
 
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index e9606c00b8d1..e45daedad967 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -109,6 +109,7 @@
 						 DW_IC_INTR_RX_UNDER | \
 						 DW_IC_INTR_RD_REQ)
 
+#define DW_IC_ENABLE_ENABLE			BIT(0)
 #define DW_IC_ENABLE_ABORT			BIT(1)
 
 #define DW_IC_STATUS_ACTIVITY			BIT(0)


>>vim +538 drivers/i2c/busses/i2c-designware-common.c

>>   523	
>>   524	void __i2c_dw_disable(struct dw_i2c_dev *dev)
>>   525	{
>>   526		unsigned int raw_intr_stats;
>>   527		unsigned int enable;
>>   528		int timeout = 100;
>>   529		bool abort_needed;
>>   530		unsigned int status;
>>   531		int ret;
>>   532	
>>   533		regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_intr_stats);
>>   534		regmap_read(dev->map, DW_IC_ENABLE, &enable);
>>   535	
>>   536		abort_needed = raw_intr_stats & DW_IC_INTR_MST_ON_HOLD;
>>   537		if (abort_needed) {
>> > 538			if (!(enable & DW_IC_ENABLE_ENABLE)) {
>>   539				regmap_write(dev->map, DW_IC_ENABLE, DW_IC_ENABLE_ENABLE);
>>   540				enable |= DW_IC_ENABLE_ENABLE;
>>   541	
>>   542				/*
>>   543				 * Wait two ic_clk delay when enabling the i2c to ensure ENABLE bit
>>   544				 * is already set by the driver (for 400KHz this is 25us)
>>   545				 * as described in the DesignWare I2C databook.
>>   546				 */
>>   547				fsleep(25);
>>   548			}
>>   549	
>>   550			regmap_write(dev->map, DW_IC_ENABLE, enable | DW_IC_ENABLE_ABORT);
>>   551			ret = regmap_read_poll_timeout(dev->map, DW_IC_ENABLE, enable,
>>   552						       !(enable & DW_IC_ENABLE_ABORT), 10,
>>   553						       100);
>>   554			if (ret)
>>   555				dev_err(dev->dev, "timeout while trying to abort current transfer\n");
>>   556		}
>>   557	
>>   558		do {
>>   559			__i2c_dw_disable_nowait(dev);
>>   560			/*
>>   561			 * The enable status register may be unimplemented, but
>>   562			 * in that case this test reads zero and exits the loop.
>>   563			 */
>>   564			regmap_read(dev->map, DW_IC_ENABLE_STATUS, &status);
>>   565			if ((status & 1) == 0)
>>   566				return;
>>   567	
>>   568			/*
>>   569			 * Wait 10 times the signaling period of the highest I2C
>>   570			 * transfer supported by the driver (for 400KHz this is
>>   571			 * 25us) as described in the DesignWare I2C databook.
>>   572			 */
>>   573			usleep_range(25, 250);
>>   574		} while (timeout--);
>>   575	
>>   576		dev_warn(dev->dev, "timeout in disabling adapter\n");
>>   577	}
>>   578	
------------------------------------------
Best Regards
Kimriver Liu
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index e8a688d04aee..54acf8554582 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -453,6 +453,18 @@  void __i2c_dw_disable(struct dw_i2c_dev *dev)
 
 	abort_needed = raw_intr_stats & DW_IC_INTR_MST_ON_HOLD;
 	if (abort_needed) {
+		if (!(enable & DW_IC_ENABLE_ENABLE)) {
+			regmap_write(dev->map, DW_IC_ENABLE, DW_IC_ENABLE_ENABLE);
+			enable |= DW_IC_ENABLE_ENABLE;
+
+			/*
+			 * Wait two ic_clk delay when enabling the i2c to ensure ENABLE bit
+			 * is already set by the driver (for 400KHz this is 25us)
+			 * as described in the DesignWare I2C databook.
+			 */
+			fsleep(25);
+		}
+
 		regmap_write(dev->map, DW_IC_ENABLE, enable | DW_IC_ENABLE_ABORT);
 		ret = regmap_read_poll_timeout(dev->map, DW_IC_ENABLE, enable,
 					       !(enable & DW_IC_ENABLE_ABORT), 10,
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index c7e56002809a..aba0b8fdfe9a 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -253,6 +253,26 @@  static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
 	__i2c_dw_write_intr_mask(dev, DW_IC_INTR_MASTER_MASK);
 }
 
+static bool i2c_dw_is_master_idling(struct dw_i2c_dev *dev)
+{
+	u32 status;
+	int ret;
+
+	regmap_read(dev->map, DW_IC_STATUS, &status);
+	if (!(status & DW_IC_STATUS_MASTER_ACTIVITY))
+		return true;
+
+	ret = regmap_read_poll_timeout(dev->map, DW_IC_STATUS, status,
+			!(status & DW_IC_STATUS_MASTER_ACTIVITY),
+			1100, 20000);
+	if (ret) {
+		dev_err(dev->dev, "i2c master controller not idle %d\n", ret);
+		return false;
+	}
+
+	return true;
+}
+
 static int i2c_dw_check_stopbit(struct dw_i2c_dev *dev)
 {
 	u32 val;
@@ -796,7 +816,8 @@  i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	 * additional interrupts are a hardware bug or this driver doesn't
 	 * handle them correctly yet.
 	 */
-	__i2c_dw_disable_nowait(dev);
+	if (i2c_dw_is_master_idling(dev))
+		__i2c_dw_disable_nowait(dev);
 
 	if (dev->msg_err) {
 		ret = dev->msg_err;