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

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

Hi Andi


> -----Original Message-----
> From: Andi Shyti <andi.shyti@kernel.org> 
> Sent: 2024年9月10日 1:37
> To: Liu Kimriver/刘金河 <kimriver.liu@siengine.com>
> Cc: andriy.shevchenko@linux.intel.com; mika.westerberg@linux.intel.com; jsd@semihalf.com; linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; jarkko.nikula@linux.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