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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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.)

> > +}


-- 
With Best Regards,
Andy Shevchenko