RE: [PATCH] i2c: designware: fix master is holding SCL low while ENABLE bit is disabled

["Liu Kimriver/刘金河" <kimriver.liu@siengine.com>]

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