Re: [PATCH 6/8] platform: arm64: qcom-hamoa-ec: Retry I2C transfers on NACK

Anvesh Jain P <[email protected]> Thu, 30 Jul 2026 12:10:20 +0530
Newsgroups org.kernel.vger.platform-driver-x86,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel
Message-ID <[email protected]>

On 7/29/2026 4:41 PM, Konrad Dybcio wrote:
> On 7/28/26 7:44 PM, Anvesh Jain P wrote:
>> The EC firmware NACKs the bus instead of clock-stretching while it is
>> busy servicing internal work (e.g. right after boot, when it is also
>> emitting a burst of SCI events). A transfer issued in that window fails
>> with -ENXIO even though the EC is otherwise healthy.
>>
>> This isn't visible with the smbus-only command set used previously; it
>> surfaced while stress-testing the new fan LUT command on Hamoa, which
>> issues raw i2c_transfer() calls more aggressively and is thus more
>> likely to land in the NACK window right after boot or during an SCI
>> event burst.
>>
>> Retry qcom_ec_write() and qcom_ec_read() a bounded number of times with
>> a short delay when the EC NACKs the transfer, before giving up.
>>
>> Signed-off-by: Anvesh Jain P <[email protected]>
>> ---
> [...]
> 
>>  enum qcom_ec_sci_events {
>>  	EC_FAN1_STATUS_CHANGE_EVT = 0x30,
>>  	EC_FAN2_STATUS_CHANGE_EVT,
>> @@ -233,11 +245,20 @@ static int qcom_ec_write(struct qcom_ec *ec, u8 cmd, const u8 *data, size_t len)
>>  	msg.buf[0] = cmd;
>>  	memcpy(&msg.buf[1], data, len);
>>  
>> -	ret = i2c_transfer(ec->client->adapter, &msg, 1);
>> +	for (int i = 0; i < EC_I2C_NACK_MAX_ATTEMPTS; i++) {
>> +		ret = i2c_transfer(ec->client->adapter, &msg, 1);
>> +		ret = ret == 1 ? 0 : (ret < 0 ? ret : -EIO);
> Please break this monster up. I know it already exists in-tree, but
> since you're moving it around, we might as well.

Agreed, will break that up into an explicit if/else instead of the
nested ternary in both qcom_ec_write() and qcom_ec_read().

-- 
Best Regards,
Anvesh