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