From: Hector Martin <mar...@marcan.st> [ Upstream commit 88fe3078b54c9efaea7d1adfcf295e37dfb0274f ]
The I2C bus can get stuck under some conditions (desync between controller and device). The pasemi controllers include an unjam feature that is enabled on reset, but was being disabled by the driver. Keep it enabled by explicitly setting the UJM bit in the CTL register. This should help recover the bus from certain conditions, which would otherwise remain stuck forever. Signed-off-by: Hector Martin <mar...@marcan.st> Reviewed-by: Neal Gompa <n...@gompa.dev> Reviewed-by: Alyssa Rosenzweig <aly...@rosenzweig.io> Signed-off-by: Sven Peter <s...@svenpeter.dev> Link: https://lore.kernel.org/r/20250427-pasemi-fixes-v3-1-af2856829...@svenpeter.dev Signed-off-by: Andi Shyti <andi.sh...@kernel.org> Signed-off-by: Sasha Levin <sas...@kernel.org> --- Based on my thorough analysis of the commit and the surrounding codebase, I can now provide a comprehensive assessment. **YES** ## Extensive Analysis ### Nature of the Fix This commit addresses a **real hardware bug** where I2C buses can become permanently stuck under certain conditions. The analysis shows: 1. **Hardware Feature**: CTL_UJM is defined as `BIT(8)` in the control register and stands for "UnJaM mode" - a hardware feature specifically designed to automatically detect and recover from bus jam conditions. 2. **Real Problem**: The codebase reveals extensive jam detection mechanisms: - `SMSTA_JAM` (BIT(24)) and `SMSTA_JMD` (BIT(25)) status bits for detecting jams - Bus status polling that specifically waits for `!(status & SMSTA_JAM)` - Automatic resets triggered when `SMSTA_JMD` is detected 3. **User Impact**: The commit message states "The I2C bus can get stuck under some conditions (desync between controller and device)" and "would otherwise remain stuck forever" - indicating this causes **complete system functionality loss** for affected I2C peripherals. ### Code Change Analysis The fix is **minimal and surgical**: ```c -u32 val = (CTL_MTR | CTL_MRR | (smbus->clk_div & CTL_CLK_M)); +u32 val = (CTL_MTR | CTL_MRR | CTL_UJM | (smbus->clk_div & CTL_CLK_M)); ``` This single-line change adds `CTL_UJM` to the control register value during reset, enabling the hardware unjam feature that was previously being disabled. ### Stability Criteria Assessment ✅ **Fixes Important Bug**: Resolves permanent I2C bus lockups affecting system functionality ✅ **Small and Contained**: Single bit flip in one register during reset operation ✅ **Minimal Risk**: - Uses existing hardware feature (CTL_UJM already defined) - Only affects reset behavior - Hardware designed to handle this bit safely - No architectural changes ✅ **Clear Side Effects**: Only enables a hardware recovery mechanism - no negative side effects expected ✅ **Critical Subsystem**: I2C is fundamental infrastructure used by many system components ### Platform Context This affects **production hardware**: - Apple Silicon (M1, M2, etc.) devices - Maintained by active Asahi Linux team - Real users depend on functional I2C for USB- PD, audio, and other critical peripherals ### Comparison to Similar Commits This aligns closely with **Similar Commit #2** (marked YES) which also addressed I2C bus recovery by improving lockup detection and triggering recovery mechanisms. Both commits: - Fix real I2C bus lockup issues - Make minimal, targeted changes - Improve hardware recovery capabilities - Have low regression risk ### Conclusion This commit represents an ideal stable backport candidate: it fixes a serious hardware issue that completely breaks I2C functionality with a minimal, low-risk change that enables an existing hardware recovery feature. The fix prevents permanent system failures on affected hardware platforms. drivers/i2c/busses/i2c-pasemi-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-pasemi-core.c b/drivers/i2c/busses/i2c-pasemi-core.c index bd128ab2e2ebb..27ab09854c927 100644 --- a/drivers/i2c/busses/i2c-pasemi-core.c +++ b/drivers/i2c/busses/i2c-pasemi-core.c @@ -71,7 +71,7 @@ static inline int reg_read(struct pasemi_smbus *smbus, int reg) static void pasemi_reset(struct pasemi_smbus *smbus) { - u32 val = (CTL_MTR | CTL_MRR | (smbus->clk_div & CTL_CLK_M)); + u32 val = (CTL_MTR | CTL_MRR | CTL_UJM | (smbus->clk_div & CTL_CLK_M)); if (smbus->hw_rev >= 6) val |= CTL_EN; -- 2.39.5
