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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <c4f85357180e4f9c92169930d2cc5ada@siengine.com>
Date: Tue, 10 Sep 2024 01:44:43 +0000
From: Liu Kimriver/刘金河 <kimriver.liu@...ngine.com>
To: Andi Shyti <andi.shyti@...nel.org>
CC: "andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>,
        "mika.westerberg@...ux.intel.com" <mika.westerberg@...ux.intel.com>,
        "jsd@...ihalf.com" <jsd@...ihalf.com>,
        "linux-i2c@...r.kernel.org"
	<linux-i2c@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "jarkko.nikula@...ux.intel.com"
	<jarkko.nikula@...ux.intel.com>,
        Andy Shevchenko
	<andriy.shevchenko@...ux.intel.com>
Subject: RE: [PATCH v7] i2c: designware: fix master is holding SCL low while
 ENABLE bit is disabled

Hi Andi


> -----Original Message-----
> From: Andi Shyti <andi.shyti@...nel.org> 
> Sent: 2024年9月10日 1:37
> To: Liu Kimriver/刘金河 <kimriver.liu@...ngine.com>
> Cc: andriy.shevchenko@...ux.intel.com; mika.westerberg@...ux.intel.com; jsd@...ihalf.com; linux-i2c@...r.kernel.org; linux-kernel@...r.kernel.org; jarkko.nikula@...ux.intel.com
> Subject: Re: [PATCH v7] i2c: designware: fix master is holding SCL low while ENABLE bit is disabled

> Hi Kimriver,

> On Mon, Sep 09, 2024 at 02:26:30PM GMT, Liu Kimriver/刘金河 wrote:
> > 
> > HI andi
> > 
> > Due to a 12 time difference，I had been off work.
> > I am very sorry that I can't reply email in time. I will reply to your email  immediately after going to work tomorrow.

> It's not a problem, take your time! :-)

> > >> --- 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;
> > >> +			/*
> > >> +			 * Need two ic_clk delay when enabling the I2C to ensure ENABLE bit
> > >> +			 * is already set. Wait 10 times the signaling period of the highest
> > >> +			 * I2C transfer supported by the driver(for 400KHz this is 25us)
> > >> +			 * as described in the DesignWare I2C databook.
> > >> +			 */
> > >> +			usleep_range(25, 250);
> > 
> > >I think there is a misunderstanding here. Andy asked you to use 
> > >flseep and improve the calculation: "Please, calculate this delay 
> > >based on the actual speed in use (or about to be in use)."[*]
> > 
> > >Andy can you please clarify with Kimriver here?
> > 
>>   if we use 400kHz ,need setting flseep(25);  if we use 100kHz ,need 
> > setting flseep(100);  Overall, take the maximum value:flseep(100);

> Thanks for clarifying, then this is what Andy asked to do, instead of waiting an average random amount from 25 to 250us.
> Does it make sense to you?

 as described in the DesignWare I2C databook:
 Define a timer interval (ti2c_poll) equal to the 10 times the signaling period for the highest I2C transfer
 speed used in the system and supported by DW_apb_i2c. For example, if the highest I2C transfer mode
 is 400 kb/s, then this ti2c_poll is 25μs.

 Now I understand how to calculate this delay based on actual usage speed:
 fsleep(DIV_ROUND_CLOSEST_ULL(10 * MICRO, t->bus_freq_hz));
 I will update code and re-send it as V8.

Thanks.


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