Re: [PATCH 2/6] phy: qcom: qmp-pcie: Add pcs_lane1 offset to V5 offsets

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing `.pcs_lane1` offset in `qmp_pcie_offsets_v6_20` causes silent out-of-bounds register overwrite of the `tx` block.
--

--- Patch [2]: [PATCH 2/6] phy: qcom: qmp-pcie: Add pcs_lane1 offset to V5 offsets ---
commit 796aa959cc0a08be913f9eb871fe4312d98e16fc
Author: Esteban Urrutia <esteuwu@proton.me>

phy: qcom: qmp-pcie: Add pcs_lane1 offset to V5 offsets

Some SoCs such as SM8475 write data to registers using this offset,
specifically SW_CTRL2 and MX_CTRL2. Add pcs_lane1 offset to V5 offsets using
the proper offset, otherwise these registers would be part of RX and would not
be defined.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index d3effad7a074b..3618812e84d5f 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> @@ -3554,6 +3554,7 @@ static const struct qmp_pcie_offsets qmp_pcie_offsets_v5 = {
>  	.pcs_misc	= 0x0600,
>  	.tx		= 0x0e00,
>  	.rx		= 0x1000,
> +	.pcs_lane1	= 0x1400,
>  	.tx2		= 0x1600,
>  	.rx2		= 0x1800,
>  };

[Severity: High]
This is a pre-existing issue and wasn't introduced by this patch, but while
you are adding the pcs_lane1 offset here, is it also missing from
qmp_pcie_offsets_v6_20?

Looking at drivers/phy/qualcomm/phy-qcom-qmp-pcie.c, qmp_pcie_offsets_v6_20
omits the .pcs_lane1 mapping:

static const struct qmp_pcie_offsets qmp_pcie_offsets_v6_20 = {
	.serdes		= 0x1000,
	.pcs		= 0x1200,
	.pcs_misc	= 0x1400,
	.tx		= 0x0000,
	.rx		= 0x0200,
	.tx2		= 0x0800,
	.rx2		= 0x0a00,
	.ln_shrd	= 0x0e00,
};

This omission causes .pcs_lane1 to implicitly default to 0x0000, which is the
same offset as the .tx block.

However, sdx65_qmp_pciephy_cfg pairs this offset structure with a valid
pcs_lane1 table:

static const struct qmp_phy_cfg sdx65_qmp_pciephy_cfg = {
	.lanes                  = 2,

	.offsets		= &qmp_pcie_offsets_v6_20,

	.tbls = {
// ...
			.pcs_lane1       = sdx65_qmp_pcie_pcs_lane1_tbl,
			.pcs_lane1_num   = ARRAY_SIZE(sdx65_qmp_pcie_pcs_lane1_tbl),
		},

Will this cause the pcs_lane1 table to blindly overwrite the PHY transmitter
registers in the tx block during initialization?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-sm8475-bup-v1-0-b2871be2b4ec@proton.me?part=2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy