From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Tali Perry <tali.perry1@gmail.com>,
Mohammed Elbadry <mohammed.0.elbadry@gmail.com>,
Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>,
Andi Shyti <andi.shyti@kernel.org>,
Sasha Levin <sashal@kernel.org>,
avifishman70@gmail.com, tmaimon77@gmail.com,
openbmc@lists.ozlabs.org, linux-i2c@vger.kernel.org
Subject: [PATCH AUTOSEL 6.6 14/62] i2c: npcm: Add clock toggle recovery
Date: Tue, 3 Jun 2025 21:01:25 -0400 [thread overview]
Message-ID: <20250604010213.3462-14-sashal@kernel.org> (raw)
In-Reply-To: <20250604010213.3462-1-sashal@kernel.org>
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.com
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 91f508d50e7ab..5b3987460976e 100644
--- a/drivers/i2c/busses/i2c-npcm7xx.c
+++ b/drivers/i2c/busses/i2c-npcm7xx.c
@@ -1971,10 +1971,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);
--
2.39.5
next prev parent reply other threads:[~2025-06-04 1:02 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-04 1:01 [PATCH AUTOSEL 6.6 01/62] net: macb: Check return value of dma_set_mask_and_coherent() Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 02/62] net: lan743x: Modify the EEPROM and OTP size for PCI1xxxx devices Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 03/62] tipc: use kfree_sensitive() for aead cleanup Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 04/62] f2fs: use vmalloc instead of kvmalloc in .init_{,de}compress_ctx Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 05/62] bpf: Check rcu_read_lock_trace_held() in bpf_map_lookup_percpu_elem() Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 06/62] i2c: designware: Invoke runtime suspend on quick slave re-registration Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 07/62] wifi: mt76: mt7996: drop fragments with multicast or broadcast RA Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 08/62] emulex/benet: correct command version selection in be_cmd_get_stats() Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 09/62] wifi: mt76: mt76x2: Add support for LiteOn WN4516R,WN4519R Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 10/62] wifi: mt76: mt7921: add 160 MHz AP for mt7922 device Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 11/62] sctp: Do not wake readers in __sctp_write_space() Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 12/62] cpufreq: scmi: Skip SCMI devices that aren't used by the CPUs Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 13/62] i2c: tegra: check msg length in SMBUS block read Sasha Levin
2025-06-04 1:01 ` Sasha Levin [this message]
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 15/62] net: dlink: add synchronization for stats update Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 16/62] wifi: ath12k: fix macro definition HAL_RX_MSDU_PKT_LENGTH_GET Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 17/62] wifi: ath12k: fix a possible dead lock caused by ab->base_lock Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 18/62] wifi: ath11k: Fix QMI memory reuse logic Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 19/62] wifi: rtw89: leave idle mode when setting WEP encryption for AP mode Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 20/62] tcp: always seek for minimal rtt in tcp_rcv_rtt_update() Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 21/62] tcp: fix initial tp->rcvq_space.space value for passive TS enabled flows Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 22/62] x86/sgx: Prevent attempts to reclaim poisoned pages Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 23/62] ipv4/route: Use this_cpu_inc() for stats on PREEMPT_RT Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 24/62] openvswitch: Stricter validation for the userspace action Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 25/62] net: atlantic: generate software timestamp just before the doorbell Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 26/62] pinctrl: armada-37xx: propagate error from armada_37xx_pmx_set_by_name() Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 27/62] pinctrl: armada-37xx: propagate error from armada_37xx_gpio_get_direction() Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 28/62] pinctrl: armada-37xx: propagate error from armada_37xx_pmx_gpio_set_direction() Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 29/62] pinctrl: armada-37xx: propagate error from armada_37xx_gpio_get() Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 30/62] net: mlx4: add SOF_TIMESTAMPING_TX_SOFTWARE flag when getting ts info Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 31/62] net: vertexcom: mse102x: Return code for mse102x_rx_pkt_spi Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 32/62] wireless: purelifi: plfxlc: fix memory leak in plfxlc_usb_wreq_asyn() Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 33/62] wifi: mac80211: do not offer a mesh path if forwarding is disabled Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 34/62] bpftool: Fix cgroup command to only show cgroup bpf programs Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 35/62] clk: rockchip: rk3036: mark ddrphy as critical Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 36/62] libbpf: Add identical pointer detection to btf_dedup_is_equiv() Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 37/62] scsi: lpfc: Fix lpfc_check_sli_ndlp() handling for GEN_REQUEST64 commands Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 38/62] iommu/amd: Ensure GA log notifier callbacks finish running before module unload Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 39/62] wifi: iwlwifi: pcie: make sure to lock rxq->read Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 40/62] wifi: mac80211_hwsim: Prevent tsf from setting if beacon is disabled Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 41/62] wifi: mac80211: VLAN traffic in multicast path Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 42/62] wifi: iwlwifi: Add missing MODULE_FIRMWARE for Qu-c0-jf-b0 Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 43/62] net: bridge: mcast: update multicast contex when vlan state is changed Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 44/62] net: bridge: mcast: re-implement br_multicast_{enable, disable}_port functions Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 45/62] vxlan: Do not treat dst cache initialization errors as fatal Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 46/62] net: ethernet: ti: am65-cpsw: handle -EPROBE_DEFER Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 47/62] software node: Correct a OOB check in software_node_get_reference_args() Sasha Levin
2025-06-04 1:01 ` [PATCH AUTOSEL 6.6 48/62] pinctrl: mcp23s08: Reset all pins to input at probe Sasha Levin
2025-06-04 1:02 ` [PATCH AUTOSEL 6.6 49/62] wifi: ath12k: fix failed to set mhi state error during reboot with hardware grouping Sasha Levin
2025-06-04 1:02 ` [PATCH AUTOSEL 6.6 50/62] scsi: lpfc: Use memcpy() for BIOS version Sasha Levin
2025-06-04 1:02 ` [PATCH AUTOSEL 6.6 51/62] sock: Correct error checking condition for (assign|release)_proto_idx() Sasha Levin
2025-06-04 1:02 ` [PATCH AUTOSEL 6.6 52/62] i40e: fix MMIO write access to an invalid page in i40e_clear_hw Sasha Levin
2025-06-04 1:02 ` [PATCH AUTOSEL 6.6 53/62] ice: fix check for existing switch rule Sasha Levin
2025-06-04 1:02 ` [PATCH AUTOSEL 6.6 54/62] usbnet: asix AX88772: leave the carrier control to phylink Sasha Levin
2025-06-04 1:02 ` [PATCH AUTOSEL 6.6 55/62] f2fs: fix to set atomic write status more clear Sasha Levin
2025-06-04 1:02 ` [PATCH AUTOSEL 6.6 56/62] bpf, sockmap: Fix data lost during EAGAIN retries Sasha Levin
2025-06-04 1:02 ` [PATCH AUTOSEL 6.6 57/62] net: ethernet: cortina: Use TOE/TSO on all TCP Sasha Levin
2025-06-04 1:02 ` [PATCH AUTOSEL 6.6 58/62] octeontx2-pf: Add error log forcn10k_map_unmap_rq_policer() Sasha Levin
2025-06-04 1:02 ` [PATCH AUTOSEL 6.6 59/62] wifi: ath11k: determine PM policy based on machine model Sasha Levin
2025-06-04 1:02 ` [PATCH AUTOSEL 6.6 60/62] wifi: ath12k: fix link valid field initialization in the monitor Rx Sasha Levin
2025-06-04 1:02 ` [PATCH AUTOSEL 6.6 61/62] wifi: ath12k: fix incorrect CE addresses Sasha Levin
2025-06-04 1:02 ` [PATCH AUTOSEL 6.6 62/62] wifi: ath12k: Pass correct values of center freq1 and center freq2 for 160 MHz 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=20250604010213.3462-14-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=andi.shyti@kernel.org \
--cc=avifishman70@gmail.com \
--cc=linux-i2c@vger.kernel.org \
--cc=mohammed.0.elbadry@gmail.com \
--cc=openbmc@lists.ozlabs.org \
--cc=patches@lists.linux.dev \
--cc=quic_msavaliy@quicinc.com \
--cc=stable@vger.kernel.org \
--cc=tali.perry1@gmail.com \
--cc=tmaimon77@gmail.com \
/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