From: Tali Perry tali.perry1@gmail.com
[ Upstream commit 38010591a0fc3203f1cee45b01ab358b72dd9ab2 ]
During init of the bus, the module checks that the bus is idle. If one of the lines are stuck try to recover them first before failing. Sometimes SDA and SCL are low if improper reset occurs (e.g., reboot).
Signed-off-by: Tali Perry tali.perry1@gmail.com Signed-off-by: Mohammed Elbadry mohammed.0.elbadry@gmail.com Reviewed-by: Mukesh Kumar Savaliya quic_msavaliy@quicinc.com Link: https://lore.kernel.org/r/20250328193252.1570811-1-mohammed.0.elbadry@gmail.... Signed-off-by: Andi Shyti andi.shyti@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
**YES** This commit should be backported to stable kernel trees. Here's my detailed analysis: ## **Primary Reasons for Backporting:** ### **1. Fixes a Real Hardware Bug** The commit addresses a legitimate hardware initialization issue where the NPCM I2C controller can fail to initialize if SDA or SCL lines are stuck low after system reset. This is not a theoretical problem but a real-world scenario that occurs when: - External I2C devices (sensors, EEPROMs) hold SDA low after improper reset - Power cycling leaves slave devices in an inconsistent state - BMC systems reboot while I2C transactions are in progress ### **2. Small, Contained Fix** The code change is minimal and well-contained: - **Before**: Hard failure with `dev_err()` and `return -ENXIO` when lines are stuck - **After**: Attempts recovery first, only fails if recovery doesn't work - Uses existing `npcm_i2c_recovery_tgclk()` function that's already proven and in use for runtime recovery ### **3. Prevents System Boot Failures** Without this fix, systems can fail to boot completely when I2C controllers can't initialize due to stuck bus lines. The commit message specifically mentions "Sometimes SDA and SCL are low if improper reset occurs (e.g., reboot)" - this is a boot-critical issue. ### **4. Conservative Error Handling** The fix uses defensive programming: - First attempts recovery using hardware-specific TGCLK mechanism - Only fails initialization if recovery is unsuccessful - Downgrades the initial error from `dev_err` to `dev_warn` with recovery attempt - Maintains the same failure path if recovery doesn't work ### **5. Alignment with Similar Successful Backports** Looking at the reference commits, this follows the pattern of similar commit #4 (npcm timeout calculation fix) which was marked "YES" for backporting. Both: - Fix NPCM I2C driver issues - Address real hardware problems - Make small, targeted changes - Don't introduce new features ### **6. Hardware- Specific, Low Risk** The change only affects the NPCM I2C controller initialization path and uses existing recovery mechanisms. The risk of regression is minimal since: - It only adds a recovery attempt before an existing failure case - Uses proven recovery logic already in the driver - Specific to Nuvoton BMC hardware ## **Code Analysis:** The key change replaces immediate failure: ```c // OLD: Immediate failure dev_err(bus->dev, "I2C%d init fail: lines are low\n", bus->num); return -ENXIO; ``` With recovery attempt: ```c // NEW: Try recovery first dev_warn(bus->dev, " I2C%d SDA=%d SCL=%d, attempting to recover\n", ...); if (npcm_i2c_recovery_tgclk(&bus->adap)) { dev_err(bus->dev, "I2C%d init fail: SDA=%d SCL=%d\n", ...); return -ENXIO; } ``` This is a textbook example of a good stable backport candidate: it fixes a real bug that prevents system functionality, uses minimal changes, and has low regression risk.
drivers/i2c/busses/i2c-npcm7xx.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c index d97694ac29ca9..3f30c3cff7201 100644 --- a/drivers/i2c/busses/i2c-npcm7xx.c +++ b/drivers/i2c/busses/i2c-npcm7xx.c @@ -1950,10 +1950,14 @@ static int npcm_i2c_init_module(struct npcm_i2c *bus, enum i2c_mode mode,
/* check HW is OK: SDA and SCL should be high at this point. */ if ((npcm_i2c_get_SDA(&bus->adap) == 0) || (npcm_i2c_get_SCL(&bus->adap) == 0)) { - dev_err(bus->dev, "I2C%d init fail: lines are low\n", bus->num); - dev_err(bus->dev, "SDA=%d SCL=%d\n", npcm_i2c_get_SDA(&bus->adap), - npcm_i2c_get_SCL(&bus->adap)); - return -ENXIO; + dev_warn(bus->dev, " I2C%d SDA=%d SCL=%d, attempting to recover\n", bus->num, + npcm_i2c_get_SDA(&bus->adap), npcm_i2c_get_SCL(&bus->adap)); + if (npcm_i2c_recovery_tgclk(&bus->adap)) { + dev_err(bus->dev, "I2C%d init fail: SDA=%d SCL=%d\n", + bus->num, npcm_i2c_get_SDA(&bus->adap), + npcm_i2c_get_SCL(&bus->adap)); + return -ENXIO; + } }
npcm_i2c_int_enable(bus, true);