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] [day] [month] [year] [list]

Message-ID: <8115db78c21b4fb587be1dcfcbfd8d4d@siengine.com>
Date: Thu, 12 Sep 2024 01:17:08 +0000
From: Liu Kimriver/刘金河 <kimriver.liu@...ngine.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
CC: "jarkko.nikula@...ux.intel.com" <jarkko.nikula@...ux.intel.com>,
        "mika.westerberg@...ux.intel.com" <mika.westerberg@...ux.intel.com>,
        "jsd@...ihalf.com" <jsd@...ihalf.com>,
        "andi.shyti@...nel.org"
	<andi.shyti@...nel.org>,
        "linux-i2c@...r.kernel.org"
	<linux-i2c@...r.kernel.org>,
        "linux-kernel@...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

Hi Andy


>-----Original Message-----
>From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com> 
>Sent: 2024年9月11日 22:53
>To: Liu Kimriver/刘金河 <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.)
>
>> > +}

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

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