linux-kernel - Re: [PATCH v9] i2c: designware: fix controller 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZuGuwhPvxbY5OqNg@smile.fi.intel.com>
Date: Wed, 11 Sep 2024 17:52:50 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Kimriver Liu <kimriver.liu@...ngine.com>
Cc: jarkko.nikula@...ux.intel.com, mika.westerberg@...ux.intel.com,
	jsd@...ihalf.com, andi.shyti@...nel.org, linux-i2c@...r.kernel.org,
	linux-kernel@...r.kernel.org
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.)

> > +}


-- 
With Best Regards,
Andy Shevchenko