RE: [PATCH] i2c: designware: fix master is holding SCL low while ENABLE bit is disabled
From: Liu Kimriver/刘金河
Date: Mon Sep 09 2024 - 02:53:06 EST
Hi jarkko
-----Original Message-----
From: kernel test robot <lkp@xxxxxxxxx>
Sent: 2024年9月8日 21:32
To: Liu Kimriver/刘金河 <kimriver.liu@xxxxxxxxxxxx>; jarkko.nikula@xxxxxxxxxxxxxxx
Cc: oe-kbuild-all@xxxxxxxxxxxxxxx; andriy.shevchenko@xxxxxxxxxxxxxxx; mika.westerberg@xxxxxxxxxxxxxxx; jsd@xxxxxxxxxxxx; andi.shyti@xxxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Liu Kimriver/刘金河 <kimriver.liu@xxxxxxxxxxxx>
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@xxxxxxxxxxxx/
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@xxxxxxxxx/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@xxxxxxxxx/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@xxxxxxxxx>
>>| Closes: https://lore.kernel.org/oe-kbuild-all/202409082011.9JF6aYsk-lkp@xxxxxxxxx/
>>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@xxxxxxxxxxxx/
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