From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Geraldo Nascimento <geraldogabriel@gmail.com>,
Neil Armstrong <neil.armstrong@linaro.org>,
Vinod Koul <vkoul@kernel.org>, Sasha Levin <sashal@kernel.org>,
heiko@sntech.de, linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
Subject: [PATCH AUTOSEL 6.16-6.6] phy: rockchip-pcie: Properly disable TEST_WRITE strobe signal
Date: Tue, 5 Aug 2025 09:08:42 -0400 [thread overview]
Message-ID: <20250805130945.471732-7-sashal@kernel.org> (raw)
In-Reply-To: <20250805130945.471732-1-sashal@kernel.org>
From: Geraldo Nascimento <geraldogabriel@gmail.com>
[ Upstream commit 25facbabc3fc33c794ad09d73f73268c0f8cbc7d ]
pcie_conf is used to touch TEST_WRITE strobe signal. This signal should
be enabled, a little time waited, and then disabled. Current code clearly
was copy-pasted and never disables the strobe signal. Adjust the define.
While at it, remove PHY_CFG_RD_MASK which has been unused since
64cdc0360811 ("phy: rockchip-pcie: remove unused phy_rd_cfg function").
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
Link: https://lore.kernel.org/r/d514d5d5627680caafa8b7548cbdfee4307f5440.1751322015.git.geraldogabriel@gmail.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Backport Status: YES
This commit should be backported to stable kernel trees for the
following reasons:
### Bug Analysis
The commit fixes a clear programming bug where `PHY_CFG_WR_DISABLE` was
incorrectly defined as `1` instead of `0`. Looking at the code in
`phy_wr_cfg()` function (lines 100-120), the sequence is:
1. Write data and address to the configuration register
2. Wait 1 microsecond
3. Enable the TEST_WRITE strobe signal by writing `PHY_CFG_WR_ENABLE`
(value 1)
4. Wait 1 microsecond
5. Intended to disable the strobe signal with `PHY_CFG_WR_DISABLE`
However, since both `PHY_CFG_WR_ENABLE` and `PHY_CFG_WR_DISABLE` were
defined as `1`, the strobe signal was never actually disabled. This
means the PHY configuration writes were leaving the strobe signal
permanently enabled, which is incorrect hardware programming.
### Impact Assessment
1. **Real bug affecting users**: This is a functional bug that affects
all Rockchip PCIe PHY operations. The strobe signal being left
enabled could cause:
- Incorrect PHY configuration behavior
- Potential power consumption issues
- Possible hardware state corruption
- Unpredictable PCIe link behavior
2. **Small and contained fix**: The change is minimal - just changing
one define from `1` to `0`. This is exactly the type of targeted fix
suitable for stable.
3. **Clear correctness**: The fix is obviously correct - a disable
operation should use value `0`, not `1`. The bug appears to be a
copy-paste error from the initial driver introduction in 2016 (commit
fcffee3d54fca).
4. **Long-standing issue**: This bug has existed since the driver was
first introduced in 2016, affecting all kernel versions with this
driver.
5. **No architectural changes**: The fix doesn't introduce new features
or change the driver architecture - it simply corrects an incorrect
constant value.
6. **Low regression risk**: Changing the disable value from 1 to 0 is
the correct behavior according to typical hardware programming
patterns. The risk of regression is minimal since this fixes
incorrect behavior rather than changing working functionality.
### Additional Context
The commit also removes the unused `PHY_CFG_RD_MASK` define as cleanup,
which was left over from commit 64cdc0360811. This is harmless cleanup
that doesn't affect the backport decision.
The commit message clearly describes the issue: the TEST_WRITE strobe
signal should follow a pattern of enable→wait→disable, but the current
code never actually disables it due to the incorrect define value.
This meets all the criteria for stable backports: it fixes a real bug,
is small and self-contained, has minimal risk, and corrects clearly
incorrect behavior that has been present since driver introduction.
drivers/phy/rockchip/phy-rockchip-pcie.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-pcie.c b/drivers/phy/rockchip/phy-rockchip-pcie.c
index bd44af36c67a..63e88abc66c6 100644
--- a/drivers/phy/rockchip/phy-rockchip-pcie.c
+++ b/drivers/phy/rockchip/phy-rockchip-pcie.c
@@ -30,9 +30,8 @@
#define PHY_CFG_ADDR_SHIFT 1
#define PHY_CFG_DATA_MASK 0xf
#define PHY_CFG_ADDR_MASK 0x3f
-#define PHY_CFG_RD_MASK 0x3ff
#define PHY_CFG_WR_ENABLE 1
-#define PHY_CFG_WR_DISABLE 1
+#define PHY_CFG_WR_DISABLE 0
#define PHY_CFG_WR_SHIFT 0
#define PHY_CFG_WR_MASK 1
#define PHY_CFG_PLL_LOCK 0x10
--
2.39.5
WARNING: multiple messages have this Message-ID (diff)
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Geraldo Nascimento <geraldogabriel@gmail.com>,
Neil Armstrong <neil.armstrong@linaro.org>,
Vinod Koul <vkoul@kernel.org>, Sasha Levin <sashal@kernel.org>,
heiko@sntech.de, linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
Subject: [PATCH AUTOSEL 6.16-6.6] phy: rockchip-pcie: Properly disable TEST_WRITE strobe signal
Date: Tue, 5 Aug 2025 09:08:42 -0400 [thread overview]
Message-ID: <20250805130945.471732-7-sashal@kernel.org> (raw)
In-Reply-To: <20250805130945.471732-1-sashal@kernel.org>
From: Geraldo Nascimento <geraldogabriel@gmail.com>
[ Upstream commit 25facbabc3fc33c794ad09d73f73268c0f8cbc7d ]
pcie_conf is used to touch TEST_WRITE strobe signal. This signal should
be enabled, a little time waited, and then disabled. Current code clearly
was copy-pasted and never disables the strobe signal. Adjust the define.
While at it, remove PHY_CFG_RD_MASK which has been unused since
64cdc0360811 ("phy: rockchip-pcie: remove unused phy_rd_cfg function").
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
Link: https://lore.kernel.org/r/d514d5d5627680caafa8b7548cbdfee4307f5440.1751322015.git.geraldogabriel@gmail.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Backport Status: YES
This commit should be backported to stable kernel trees for the
following reasons:
### Bug Analysis
The commit fixes a clear programming bug where `PHY_CFG_WR_DISABLE` was
incorrectly defined as `1` instead of `0`. Looking at the code in
`phy_wr_cfg()` function (lines 100-120), the sequence is:
1. Write data and address to the configuration register
2. Wait 1 microsecond
3. Enable the TEST_WRITE strobe signal by writing `PHY_CFG_WR_ENABLE`
(value 1)
4. Wait 1 microsecond
5. Intended to disable the strobe signal with `PHY_CFG_WR_DISABLE`
However, since both `PHY_CFG_WR_ENABLE` and `PHY_CFG_WR_DISABLE` were
defined as `1`, the strobe signal was never actually disabled. This
means the PHY configuration writes were leaving the strobe signal
permanently enabled, which is incorrect hardware programming.
### Impact Assessment
1. **Real bug affecting users**: This is a functional bug that affects
all Rockchip PCIe PHY operations. The strobe signal being left
enabled could cause:
- Incorrect PHY configuration behavior
- Potential power consumption issues
- Possible hardware state corruption
- Unpredictable PCIe link behavior
2. **Small and contained fix**: The change is minimal - just changing
one define from `1` to `0`. This is exactly the type of targeted fix
suitable for stable.
3. **Clear correctness**: The fix is obviously correct - a disable
operation should use value `0`, not `1`. The bug appears to be a
copy-paste error from the initial driver introduction in 2016 (commit
fcffee3d54fca).
4. **Long-standing issue**: This bug has existed since the driver was
first introduced in 2016, affecting all kernel versions with this
driver.
5. **No architectural changes**: The fix doesn't introduce new features
or change the driver architecture - it simply corrects an incorrect
constant value.
6. **Low regression risk**: Changing the disable value from 1 to 0 is
the correct behavior according to typical hardware programming
patterns. The risk of regression is minimal since this fixes
incorrect behavior rather than changing working functionality.
### Additional Context
The commit also removes the unused `PHY_CFG_RD_MASK` define as cleanup,
which was left over from commit 64cdc0360811. This is harmless cleanup
that doesn't affect the backport decision.
The commit message clearly describes the issue: the TEST_WRITE strobe
signal should follow a pattern of enable→wait→disable, but the current
code never actually disables it due to the incorrect define value.
This meets all the criteria for stable backports: it fixes a real bug,
is small and self-contained, has minimal risk, and corrects clearly
incorrect behavior that has been present since driver introduction.
drivers/phy/rockchip/phy-rockchip-pcie.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-pcie.c b/drivers/phy/rockchip/phy-rockchip-pcie.c
index bd44af36c67a..63e88abc66c6 100644
--- a/drivers/phy/rockchip/phy-rockchip-pcie.c
+++ b/drivers/phy/rockchip/phy-rockchip-pcie.c
@@ -30,9 +30,8 @@
#define PHY_CFG_ADDR_SHIFT 1
#define PHY_CFG_DATA_MASK 0xf
#define PHY_CFG_ADDR_MASK 0x3f
-#define PHY_CFG_RD_MASK 0x3ff
#define PHY_CFG_WR_ENABLE 1
-#define PHY_CFG_WR_DISABLE 1
+#define PHY_CFG_WR_DISABLE 0
#define PHY_CFG_WR_SHIFT 0
#define PHY_CFG_WR_MASK 1
#define PHY_CFG_PLL_LOCK 0x10
--
2.39.5
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
next prev parent reply other threads:[~2025-08-05 13:20 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-05 13:08 [PATCH AUTOSEL 6.16-6.6] mfd: axp20x: Set explicit ID for AXP313 regulator Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-5.4] MIPS: vpe-mt: add missing prototypes for vpe_{alloc,start,stop,free} Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-5.10] leds: leds-lp50xx: Handle reg to get correct multi_index Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-5.4] scsi: bfa: Double-free fix Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-5.4] pinctrl: stm32: Manage irq affinity settings Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16] PCI: dw-rockchip: Delay link training after hot reset in EP mode Sasha Levin
2025-08-05 13:08 ` Sasha Levin
2025-08-05 13:08 ` Sasha Levin [this message]
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-6.6] phy: rockchip-pcie: Properly disable TEST_WRITE strobe signal Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-6.6] soundwire: Move handle_nested_irq outside of sdw_dev_lock Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-5.4] media: uvcvideo: Fix bandwidth issue for Alcor camera Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-5.15] crypto: hisilicon/hpre - fix dma unmap sequence Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-6.6] soundwire: amd: serialize amd manager resume sequence during pm_prepare Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-5.15] watchdog: sbsa: Adjust keepalive timeout to avoid MediaTek WS0 race condition Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-6.6] clk: qcom: ipq5018: keep XO clock always on Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16] media: i2c: vd55g1: Fix RATE macros not being expressed in bps Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-5.4] media: usb: hdpvr: disable zero-length read messages Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-6.15] media: raspberrypi: cfe: Fix min_reqbufs_allocation Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-6.1] hwmon: (emc2305) Set initial PWM minimum value during probe based on thermal state Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-6.12] media: uvcvideo: Add quirk for HP Webcam HD 2300 Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-6.1] drm/amd/display: Only finalize atomic_obj if it was initialized Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-5.4] vhost: fail early when __vhost_add_used() fails Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-6.12] scsi: lpfc: Ensure HBA_SETUP flag is used only for SLI4 in dev_loss_tmo_callbk Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16] ext4: limit the maximum folio order Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-5.4] fs/orangefs: use snprintf() instead of sprintf() Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16] crypto: caam - Support iMX8QXP and variants thereof Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.12] crypto: ccp - Add missing bootloader info reg for pspv6 Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] scsi: lpfc: Check for hdwq null ptr when cleaning up lpfc_vport structure Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] media: dvb-frontends: dib7090p: fix null-ptr-deref in dib7090p_rw_on_apb() Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.15] scsi: pm80xx: Free allocated tags after failure Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.15] HID: rate-limit hid_warn to prevent log flooding Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16] media: i2c: vd55g1: Setup sensor external clock before patching Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.15] watchdog: iTCO_wdt: Report error if timeout configuration fails Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.15] media: iris: Add handling for corrupt and drop frames Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.15] phy: rockchip-pcie: Enable all four lanes if required Sasha Levin
2025-08-05 13:09 ` Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] watchdog: dw_wdt: Fix default timeout Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] MIPS: Don't crash in stack_top() for tasks without ABI or vDSO Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.6] crypto: jitter - fix intermediary handling Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.1] vsock/virtio: Resize receive buffers so that each SKB fits in a 4K page Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.1] MIPS: lantiq: falcon: sysctrl: fix request memory check logic Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] media: tc358743: Check I2C succeeded during probe Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.1] scsi: mpi3mr: Correctly handle ATA device errors Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.12] clk: renesas: rzg2l: Postpone updating priv->clks[] Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] scsi: mpt3sas: Correctly handle ATA device errors Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.15] smb: client: fix session setup against servers that require SPN Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] ext4: do not BUG when INLINE_DATA_FL lacks system.data xattr Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.1] fbdev: fix potential buffer overflow in do_register_framebuffer() Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.15] sphinx: kernel_abi: fix performance regression with O=<dir> Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] media: tc358743: Return an appropriate colorspace from tc358743_set_fmt Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.6] drm/amd/display: Avoid configuring PSR granularity if PSR-SU not supported Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] media: tc358743: Increase FIFO trigger level to 374 Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] jfs: truncate good inode pages when hard link is 0 Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.15] media: v4l2-common: Reduce warnings about missing V4L2_CID_LINK_FREQ control Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.1] dmaengine: stm32-dma: configure next sg only if there are more than 2 sgs Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.12] RDMA/bnxt_re: Fix size of uverbs_copy_to() in BNXT_RE_METHOD_GET_TOGGLE_MEM Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] cifs: Fix calling CIFSFindFirst() for root path without msearch Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.10] RDMA/core: reduce stack using in nldev_stat_get_doit() Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] scsi: libiscsi: Initialize iscsi_conn->dd_data only if memory is allocated Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] media: dvb-frontends: w7090p: fix null-ptr-deref in w7090p_tuner_write_serpar and w7090p_tuner_read_serpar Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.12] soundwire: amd: cancel pending slave status handling workqueue during remove sequence Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.6] PCI: xgene-msi: Resend an MSI racing with itself on a different CPU Sasha Levin
2025-08-05 13:20 ` Marc Zyngier
2025-08-05 13:59 ` Sasha Levin
2025-08-05 18:09 ` Marc Zyngier
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.6] clk: tegra: periph: Fix error handling and resolve unsigned compare warning Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.12] drm/amd/display: Disable dsc_power_gate for dcn314 by default Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] RDMA: hfi1: fix possible divide-by-zero in find_hw_thread_mask() Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.15] crypto: octeontx2 - add timeout for load_fvc completion poll Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.6] power: supply: qcom_battmgr: Add lithium-polymer entry Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.12] media: ipu-bridge: Add _HID for OV5670 Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.12] media: hi556: Fix reset GPIO timings Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.12] clk: thead: Mark essential bus clocks as CLK_IGNORE_UNUSED Sasha Levin
2025-08-05 13:09 ` Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.15] media: uvcvideo: Set V4L2_CTRL_FLAG_DISABLED during queryctrl errors Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] jfs: Regular file corruption check Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] jfs: upper bound check of tree index in dbAllocAG 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=20250805130945.471732-7-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=geraldogabriel@gmail.com \
--cc=heiko@sntech.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=neil.armstrong@linaro.org \
--cc=patches@lists.linux.dev \
--cc=stable@vger.kernel.org \
--cc=vkoul@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.