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

[Liu Kimriver/刘金河]

Hi Andy


>-----Original Message-----
>From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> 
>Sent: 2024年9月11日 22:53
>To: Liu Kimriver/刘金河 <kimriver.liu@xxxxxxxxxxxx>
>Cc: jarkko.nikula@xxxxxxxxxxxxxxx; mika.westerberg@xxxxxxxxxxxxxxx; jsd@xxxxxxxxxxxx; andi.shyti@xxxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
>Subject: Re: [PATCH v9] i2c: designware: fix controller is holding SCL low while ENABLE bit is disabled

>On Wed, Sep 11, 2024 at 05:45:05PM +0300, Andy Shevchenko wrote:
>> On Wed, Sep 11, 2024 at 04:39:45PM +0800, Kimriver Liu wrote:

>FWIW, below are the fixes
>
>...
>
>> > +/*
>> > + * This functions waits controller idling before disabling I2C
>> > + * When the controller is not in the IDLE state,
>> > + * MST_ACTIVITY bit (IC_STATUS[5]) is set.
>> > + * Values:
>> > + * 0x1 (ACTIVE): Controller not idle
>> > + * 0x0 (IDLE): Controller is idle
>> > + * The function is called after returning the end of the current transfer
>> > + * Returns:
>> 
>> > + * Return 0 as controller IDLE
>> > + * Return a negative errno as controller ACTIVE
>
> * False when controller is in IDLE state.
> * True when controller is in ACTIVE state.
>
>> But why non-boolean again?
>> 
>> > + */
>> > +static int i2c_dw_is_controller_active(struct dw_i2c_dev *dev)
>
>static bool i2c_dw_is_controller_active(struct dw_i2c_dev *dev)
>
>> > +{
>> > +	u32 status;
>> > +
>> > +	regmap_read(dev->map, DW_IC_STATUS, &status);
>> > +	if (!(status & DW_IC_STATUS_MASTER_ACTIVITY))
>> > +		return 0;
>
>		return false;
>
>> > +	return regmap_read_poll_timeout(dev->map, DW_IC_STATUS, status,
>> > +			!(status & DW_IC_STATUS_MASTER_ACTIVITY),
>> > +			1100, 20000);
>
>			1100, 20000) != 0;
>
>Alternatively
>
>	int ret;
>	...
>
>	ret = regmap_read_poll_timeout(dev->map, DW_IC_STATUS, status,
>				       !(status & DW_IC_STATUS_MASTER_ACTIVITY),
>				       1100, 20000);
>	if (ret)
>		return true;
>
>	return false;
>
> (Also mind indentation in _read_poll_timeout() lines.)
>
>> > +}

Sorry.
Thanks for your suggestions and scheme, I will update in patch v10 today.

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