public inbox for linux-i2c@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Hector Martin <marcan@marcan.st>, Neal Gompa <neal@gompa.dev>,
	Alyssa Rosenzweig <alyssa@rosenzweig.io>,
	Sven Peter <sven@svenpeter.dev>,
	Andi Shyti <andi.shyti@kernel.org>,
	Sasha Levin <sashal@kernel.org>,
	maddy@linux.ibm.com, mpe@ellerman.id.au, sven@kernel.org,
	j@jannau.net, linuxppc-dev@lists.ozlabs.org,
	asahi@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	linux-i2c@vger.kernel.org
Subject: [PATCH AUTOSEL 6.15 024/118] i2c: pasemi: Enable the unjam machine
Date: Tue,  3 Jun 2025 20:49:15 -0400	[thread overview]
Message-ID: <20250604005049.4147522-24-sashal@kernel.org> (raw)
In-Reply-To: <20250604005049.4147522-1-sashal@kernel.org>

From: Hector Martin <marcan@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 <marcan@marcan.st>
Reviewed-by: Neal Gompa <neal@gompa.dev>
Reviewed-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Signed-off-by: Sven Peter <sven@svenpeter.dev>
Link: https://lore.kernel.org/r/20250427-pasemi-fixes-v3-1-af28568296c0@svenpeter.dev
Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
Signed-off-by: Sasha Levin <sashal@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


  parent reply	other threads:[~2025-06-04  0:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20250604005049.4147522-1-sashal@kernel.org>
2025-06-04  0:48 ` [PATCH AUTOSEL 6.15 007/118] i2c: designware: Invoke runtime suspend on quick slave re-registration Sasha Levin
2025-06-04  0:49 ` [PATCH AUTOSEL 6.15 023/118] i2c: tegra: check msg length in SMBUS block read Sasha Levin
2025-06-04  0:49 ` Sasha Levin [this message]
2025-06-04  0:49 ` [PATCH AUTOSEL 6.15 025/118] i2c: npcm: Add clock toggle recovery Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250604005049.4147522-24-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=alyssa@rosenzweig.io \
    --cc=andi.shyti@kernel.org \
    --cc=asahi@lists.linux.dev \
    --cc=j@jannau.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=marcan@marcan.st \
    --cc=mpe@ellerman.id.au \
    --cc=neal@gompa.dev \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=sven@kernel.org \
    --cc=sven@svenpeter.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox