Linux-PHY Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/5] dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4
From: Konrad Dybcio @ 2026-07-20 14:41 UTC (permalink / raw)
  To: Konrad Dybcio, Vinod Koul, Neil Armstrong, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson
  Cc: linux-kernel, linux-phy, linux-arm-msm, devicetree, usb4-upstream,
	Raghavendra Thoorpu, Mika Westerberg, Sven Peter
In-Reply-To: <20260518-topic-usb4phy-v1-1-71d827c49dca@oss.qualcomm.com>

On 5/18/26 12:29 PM, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> Some instances of the QMP combo PHY (called USB43DP) feature a third
> functional sub-block, responsible for USB4/Thunderbolt 3 communication.
> 
> Compared to the today's state of the binding, one more clock (P2RR2P -
> PHY-to-Router, Router-to-PHY) needs to be enabled for the PHY to be
> able to switch to USB4 mode. Allow that for X1E.
> 
> Also, add a bindings define to let consumers access it.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---

It's been a while and I see Vinod is picking up patches.
Krzysztof, could you please take a look at this binding change?

Konrad

>  .../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml         | 3 ++-
>  include/dt-bindings/phy/phy-qcom-qmp.h                                 | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> index 3d537b7f9985..eba4bee474fb 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> @@ -52,7 +52,7 @@ properties:
>        - const: ref
>        - const: com_aux
>        - const: usb3_pipe
> -      - const: cfg_ahb
> +      - enum: [ p2rr2p_pipe, cfg_ahb ]
>  
>    power-domains:
>      maxItems: 1
> @@ -186,6 +186,7 @@ allOf:
>            enum:
>              - qcom,sc7180-qmp-usb3-dp-phy
>              - qcom,sdm845-qmp-usb3-dp-phy
> +            - qcom,x1e80100-qmp-usb3-dp-phy
>      then:
>        properties:
>          clocks:
> diff --git a/include/dt-bindings/phy/phy-qcom-qmp.h b/include/dt-bindings/phy/phy-qcom-qmp.h
> index 6b43ea9e0051..1c3ce0c02b0c 100644
> --- a/include/dt-bindings/phy/phy-qcom-qmp.h
> +++ b/include/dt-bindings/phy/phy-qcom-qmp.h
> @@ -16,6 +16,7 @@
>  /* QMP USB4-USB3-DP PHYs */
>  #define QMP_USB43DP_USB3_PHY		0
>  #define QMP_USB43DP_DP_PHY		1
> +#define QMP_USB43DP_USB4_PHY		2
>  
>  /* QMP PCIE PHYs */
>  #define QMP_PCIE_PIPE_CLK		0
> 

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

^ permalink raw reply

* Re: [PATCH v4 8/8] phy: rockchip: samsung-hdptx: Consistently use bitfield macros
From: Dmitry Baryshkov @ 2026-07-20 14:31 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
	Dmitry Baryshkov, kernel, linux-phy, linux-arm-kernel,
	linux-rockchip, linux-kernel, Thomas Niederprüm,
	Simon Wright
In-Reply-To: <20260612-hdptx-clk-fixes-v4-8-ce5e1d456cda@collabora.com>

On Fri, Jun 12, 2026 at 02:45:27AM +0300, Cristian Ciocaltea wrote:
> Make the code more robust and improve readability by using the available
> bitfield macros (e.g. FIELD_PREP, FIELD_GET) whenever possible, instead
> of open coding the related bit operations.
> 
> Tested-by: Thomas Niederprüm <dubito@online.de>
> Tested-by: Simon Wright <simon@symple.nz>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 30 +++++++++++++++--------
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

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

^ permalink raw reply

* Re: [PATCH v4 7/8] phy: rockchip: samsung-hdptx: Simplify GRF access with FIELD_PREP_WM16()
From: Dmitry Baryshkov @ 2026-07-20 14:30 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
	Dmitry Baryshkov, kernel, linux-phy, linux-arm-kernel,
	linux-rockchip, linux-kernel, Thomas Niederprüm,
	Simon Wright
In-Reply-To: <20260612-hdptx-clk-fixes-v4-7-ce5e1d456cda@collabora.com>

On Fri, Jun 12, 2026 at 02:45:26AM +0300, Cristian Ciocaltea wrote:
> The 16 most significant bits of the general-purpose register (GRF) are
> used as a write-enable mask for the remaining 16 bits.
> 
> Make use of the recently introduced FIELD_PREP_WM16() macro to avoid
> open-coding the bit shift operations and improve code readability.
> 
> Tested-by: Thomas Niederprüm <dubito@online.de>
> Tested-by: Simon Wright <simon@symple.nz>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 52 +++++++++++------------
>  1 file changed, 25 insertions(+), 27 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

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

^ permalink raw reply

* Re: [PATCH v4 6/8] phy: rockchip: samsung-hdptx: Drop restrict_rate_change handling
From: Dmitry Baryshkov @ 2026-07-20 14:26 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
	Dmitry Baryshkov, kernel, linux-phy, linux-arm-kernel,
	linux-rockchip, linux-kernel, Thomas Niederprüm,
	Simon Wright
In-Reply-To: <20260612-hdptx-clk-fixes-v4-6-ce5e1d456cda@collabora.com>

On Fri, Jun 12, 2026 at 02:45:25AM +0300, Cristian Ciocaltea wrote:
> Since commit 6efbd0f46dd8 ("phy: rockchip: samsung-hdptx: Restrict
> altering TMDS char rate via CCF"), adjusting the rate via the Common
> Clock Framework API has been disallowed.
> 
> To avoid breaking existing users until switching to the PHY config API,
> it introduced a temporary exception to the rule, controlled via the
> 'restrict_rate_change' flag.
> 
> As the API transition completed, remove the now deprecated exception
> logic.
> 
> Tested-by: Thomas Niederprüm <dubito@online.de>
> Tested-by: Simon Wright <simon@symple.nz>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 42 +++++------------------
>  1 file changed, 8 insertions(+), 34 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

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

^ permalink raw reply

* Re: [PATCH v4 5/8] phy: rockchip: samsung-hdptx: Drop TMDS rate setup workaround
From: Dmitry Baryshkov @ 2026-07-20 14:19 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
	Dmitry Baryshkov, kernel, linux-phy, linux-arm-kernel,
	linux-rockchip, linux-kernel, Thomas Niederprüm,
	Simon Wright
In-Reply-To: <20260612-hdptx-clk-fixes-v4-5-ce5e1d456cda@collabora.com>

On Fri, Jun 12, 2026 at 02:45:24AM +0300, Cristian Ciocaltea wrote:
> Since commit ba9c2fe18c17 ("drm/rockchip: dw_hdmi_qp: Switch to
> phy_configure()") the TMDS rate setup doesn't rely anymore on the
> unconventional usage of the bus width, instead it is managed exclusively
> through the HDMI PHY configuration API.
> 
> Drop the now obsolete workaround to retrieve the TMDS character rate via
> phy_get_bus_width() during power_on().
> 
> While at it, get rid of the extra call to rk_hdptx_phy_consumer_put() by
> moving the statement at the end of the function.
> 
> Tested-by: Thomas Niederprüm <dubito@online.de>
> Tested-by: Simon Wright <simon@symple.nz>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 27 +++++------------------
>  1 file changed, 6 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> index 25bd821cd039..35997087d61c 100644
> --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> @@ -1660,22 +1660,6 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
>  	enum phy_mode mode = phy_get_mode(phy);
>  	int ret, lane;
>  
> -	if (mode != PHY_MODE_DP) {
> -		if (!hdptx->hdmi_cfg.rate && hdptx->hdmi_cfg.mode != PHY_HDMI_MODE_FRL) {
> -			/*
> -			 * FIXME: Temporary workaround to setup TMDS char rate
> -			 * from the RK DW HDMI QP bridge driver.
> -			 * Will be removed as soon the switch to the HDMI PHY
> -			 * configuration API has been completed on both ends.
> -			 */
> -			hdptx->hdmi_cfg.rate = phy_get_bus_width(hdptx->phy) & 0xfffffff;
> -			hdptx->hdmi_cfg.rate *= 100;
> -		}
> -
> -		dev_dbg(hdptx->dev, "%s rate=%llu bpc=%u\n", __func__,
> -			hdptx->hdmi_cfg.rate, hdptx->hdmi_cfg.bpc);
> -	}
> -
>  	ret = rk_hdptx_phy_consumer_get(hdptx);
>  	if (ret)
>  		return ret;
> @@ -1701,9 +1685,10 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
>  		rk_hdptx_dp_pll_init(hdptx);
>  
>  		ret = rk_hdptx_dp_aux_init(hdptx);
> -		if (ret)
> -			rk_hdptx_phy_consumer_put(hdptx, true);
>  	} else {
> +		dev_dbg(hdptx->dev, "%s rate=%llu bpc=%u\n", __func__,
> +			hdptx->hdmi_cfg.rate, hdptx->hdmi_cfg.bpc);
> +
>  		if (hdptx->pll_config_dirty)
>  			ret = rk_hdptx_pll_cmn_config(hdptx);
>  
> @@ -1716,11 +1701,11 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
>  			else
>  				ret = rk_hdptx_tmds_ropll_mode_config(hdptx);
>  		}
> -
> -		if (ret)
> -			rk_hdptx_phy_consumer_put(hdptx, true);
>  	}
>  
> +	if (ret)
> +		rk_hdptx_phy_consumer_put(hdptx, true);

This looks likes an unrelated change. Could you please split it to a
separate commit?

> +
>  	return ret;
>  }
>  
> 
> -- 
> 2.54.0
> 

-- 
With best wishes
Dmitry

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

^ permalink raw reply

* Re: [PATCH v4 4/8] phy: rockchip: samsung-hdptx: Handle uncommitted PHY config changes
From: Dmitry Baryshkov @ 2026-07-20 14:17 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
	Dmitry Baryshkov, kernel, linux-phy, linux-arm-kernel,
	linux-rockchip, linux-kernel, Thomas Niederprüm,
	Simon Wright
In-Reply-To: <20260612-hdptx-clk-fixes-v4-4-ce5e1d456cda@collabora.com>

On Fri, Jun 12, 2026 at 02:45:23AM +0300, Cristian Ciocaltea wrote:
> Any changes to the PHY link rate and/or color depth done via the HDMI
> PHY configuration API are not immediately programmed into the hardware,
> but are delayed until the PHY usage count gets incremented from 0 to 1,
> that is when it is powered on or when the PLL clock exposed through
> the CCF API is prepared, whichever comes first.

Why do you need either-of here? Can it be just one path?

> 
> Since the clock might remain in prepared state after subsequent PHY
> config changes, the programming can also be triggered via
> clk_ops.set_rate().  However, from the clock consumer perspective (i.e.
> VOP2 display controller), the (pixel) clock rate doesn't vary with bpc,
> as that is handled internally by the PHY and reflected in the TDMS
> character rate only.
> 
> As a consequence, changing the bpc while preserving the modeline may
> lead to out-of-sync issues between CCF and HDMI PHY config state,
> because the .set_rate() callback is not invoked when clock rate remains
> constant.  This may also happen when the PHY PLL has been pre-programmed
> by an external entity, e.g. the bootloader, which is actually a
> regression introduced by the recent FRL patches.
> 
> Introduce a pll_config_dirty flag to keep track of uncommitted PHY
> config changes and use it in clk_ops.determine_rate() to invalidate the
> current clock rate (as known by CCF) and, consequently, ensure those
> changes are programmed into hardware via clk_ops.set_rate().
> 
> Moreover, proceed with a similar fix in phy_ops.power_on() callback, to
> handle the scenario where the CCF API is not used due to operating in
> FRL mode, while the clock is still in a prepared state and thus
> preventing rk_hdptx_phy_consumer_get() to apply the updated PHY
> configuration.
> 
> Fixes: de5dba833118 ("phy: rockchip: samsung-hdptx: Add HDMI 2.1 FRL support")
> Fixes: 9d0ec51d7c22 ("phy: rockchip: samsung-hdptx: Add high color depth management")
> Tested-by: Thomas Niederprüm <dubito@online.de>
> Tested-by: Simon Wright <simon@symple.nz>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 84 +++++++++++++----------
>  1 file changed, 48 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> index b210c1a88b25..25bd821cd039 100644
> --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> @@ -413,6 +413,7 @@ struct rk_hdptx_phy {
>  
>  	/* clk provider */
>  	struct clk_hw hw;
> +	bool pll_config_dirty;

Which lock protects it?

>  	bool restrict_rate_change;
>  
>  	atomic_t usage_count;

-- 
With best wishes
Dmitry

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

^ permalink raw reply

* Re: [PATCH v3 phy-next 8/8] phy: lynx-10g: use RCW override procedure for dynamic protocol change
From: sashiko-bot @ 2026-07-20 14:13 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: robh, linux-phy, conor+dt, vkoul, devicetree, neil.armstrong,
	olteanv
In-Reply-To: <20260720133642.136324-9-vladimir.oltean@nxp.com>

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

Pre-existing issues:
- [High] An off-by-one error in the bounds check for `serdes_idx` within `fsl_guts_serdes_get_rcw_override` incorrectly rejects the final SerDes block on the SoC.
- [High] Uninitialized lanes on `ls2088a` SerDes1 have their XGMII clock selection bits blindly overwritten to GMII during RCW initialization.
--

--- Patch [8]: [PATCH v3 phy-next 8/8] phy: lynx-10g: use RCW override procedure for dynamic protocol change ---
commit 7e0651dd0ae8f0fcd028d4877a54ccdb1dcfef2d
Author: Vladimir Oltean <vladimir.oltean@nxp.com>

phy: lynx-10g: use RCW override procedure for dynamic protocol change

Up until this patch, the only protocol change supported was between
1000Base-X/SGMII and 2500Base-X. The others require an RCW override
procedure which was lacking.

Since now the guts driver provides the means of applying this procedure,
make use of it and remove any comment which mentioned the limitation.

> diff --git a/drivers/phy/freescale/phy-fsl-lynx-10g.c b/drivers/phy/freescale/phy-fsl-lynx-10g.c
> index 38def160ef1aa..5ece7889aed7b 100644
> --- a/drivers/phy/freescale/phy-fsl-lynx-10g.c
> +++ b/drivers/phy/freescale/phy-fsl-lynx-10g.c
[ ... ]
> @@ -1225,6 +1223,13 @@ static int lynx_10g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
>  	if (powered_up)
>  		lynx_10g_lane_halt(phy);
>  
> +	if (lynx_10g_lane_mode_needs_rcw_override(lane, lane_mode)) {
> +		err = fsl_guts_lane_set_mode(priv->info->index, lane->id,
> +					     lane_mode);

[Severity: High]
This isn't a bug introduced by this patch, but does the underlying RCW override
procedure contain an off-by-one error that rejects the final SerDes block?

When this calls into fsl_guts_lane_set_mode(), it eventually hits a bounds
check in fsl_guts_serdes_get_rcw_override():

drivers/soc/fsl/guts.c:fsl_guts_serdes_get_rcw_override() {
    ...
    if (serdes_idx >= soc_data->num_serdes || serdes_idx <= 0)
        return -ERANGE;
    ...
}

Since serdes_idx is 1-based, and this uses >= instead of >, won't this
completely fail to apply the dynamic protocol change for any interface on
the last SerDes block (e.g. index 2 on SoCs with 2 SerDes blocks)?

[Severity: High]
This is a pre-existing issue, but does initializing the SerDes RCW
inadvertently corrupt unmanaged lanes on LS2088A?

When fsl_guts_lane_set_mode() is called for the first time on an LS2088A SoC,
it triggers ls2088a_serdes_init_rcwcr() to build a new state for the RCW
utilizing soc.lanes_initialized:

drivers/soc/fsl/guts.c:ls2088a_serdes_init_rcwcr() {
    ...
    for_each_set_bit(i, &soc.lanes_initialized[serdes_idx - 1], MAX_NUM_LANES)
        if (lynx_lane_mode_uses_xgmii_mac(soc.lane_mode[serdes_idx - 1][i]))
            reg |= LS2088A_RCWSR30_SRDS_CLK_SEL_XGMII_Ln_S1(i);

    err = fsl_guts_rcw_rmw(29, reg,
                   LS2088A_RCWSR30_SRDS_CLK_EN_SEL_XGMII_S1 |
                   LS2088A_RCWSR30_SRDS_CLK_SEL_MSK);
    ...
}

Because the blanket mask LS2088A_RCWSR30_SRDS_CLK_SEL_MSK clears the bits
for all 8 lanes but only restores them for explicitly managed lanes, won't
unmanaged lanes (such as those used by DPDK or alternative firmware) have
their hardware protocol selection forced to 0 (GMII), potentially breaking
network connectivity?

> +		if (err)
> +			goto out;
> +	}
> +
>  	err = lynx_10g_lane_disable_pcvt(lane, lane->mode);
>  	if (err)
>  		goto out;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133642.136324-1-vladimir.oltean@nxp.com?part=8

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

^ permalink raw reply

* Re: [PATCH v2 2/2] phy: mediatek: Add support for PCI-Express Gen4 S-PHY
From: sashiko-bot @ 2026-07-20 14:13 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: neil.armstrong, olteanv, devicetree, vkoul, linux-phy, conor+dt,
	robh
In-Reply-To: <20260720135344.84183-3-angelogioacchino.delregno@collabora.com>

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Improper handling of missing or unsupported NVMEM calibration data causes spurious error logs and hard probe failures.
- [Medium] A valid calibration value of 0 is incorrectly treated as missing data, discarding all calibrations.
- [Low] The TX_IMPSEL registers are updated twice with identical values during initialization.
- [Low] Macros for AEQ_EGEQ_RATIO are defined but unused in favor of literal magic numbers.
--

--- Patch [2]: [PATCH v2 2/2] phy: mediatek: Add support for PCI-Express Gen4 S-PHY ---
commit d91c6c0718440c44adc7e6a83f5aeca706a6c3d5
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Subject: phy: mediatek: Add support for PCI-Express Gen4 S-PHY

This patch adds support for the PCI-Express Gen4 S-PHY found in the latest
MediaTek SoCs, including MT8196 and MT8894.

> --- a/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
> +++ b/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
[ ... ]
> +#define PEXTP_DIG_LN_RX2_AEQ_EDGE_0			0x6004
> +#  define RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN3		GENMASK(21, 16)
> +#  define RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN4		GENMASK(29, 24)
> +#  define AEQ_EGEQ_RATIO_GEN3_TO_22			0x16
> +#  define AEQ_EGEQ_RATIO_GEN4_TO_22			0x16
[ ... ]
> +		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX2_AEQ_EDGE_0,
> +				     RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN3, 22);
> +
> +		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX2_AEQ_EDGE_0,
> +				     RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN4, 22);

[Severity: Low]
Is there a reason the literal 22 is used here instead of the
AEQ_EGEQ_RATIO_GEN3_TO_22 and AEQ_EGEQ_RATIO_GEN4_TO_22 macros defined
earlier?

[ ... ]
> +static void mtk_pcie_sphy_apply_calibration(struct mtk_pcie_sphy *pcie_sphy)
> +{
[ ... ]
> +		mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
> +				     RG_XTP_LN_TX_IMPSEL_PMOS, cal->tx_impsel.pmos);
> +
> +		mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
> +				     RG_XTP_LN_TX_IMPSEL_NMOS, cal->tx_impsel.nmos);
> +	}
> +}
> +
[ ... ]
> +static int mtk_pcie_sphy_init(struct phy *phy)
> +{
[ ... ]
> +	if (pcie_sphy->calibration.supported) {
> +		mtk_pcie_sphy_apply_calibration(pcie_sphy);
> +
> +		tx_impsel.pmos = pcie_sphy->calibration.tx_impsel.pmos;
> +		tx_impsel.nmos = pcie_sphy->calibration.tx_impsel.nmos;
> +	} else {
> +		/* Set P=10, N=9 to prevent EMI if no calibration present */
> +		tx_impsel.pmos = 10;
> +		tx_impsel.nmos = 9;
> +	}
> +
> +	/* Select TX Impedance on N and P MOSFETs */
> +	for (i = 0; i < pcie_sphy->num_lanes; i++) {
> +		void __iomem *sif_lane_base = pcie_sphy->sif_base + PEXTP_REG_LANE(i);
> +
> +		mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
> +				     RG_XTP_LN_TX_IMPSEL_PMOS, tx_impsel.pmos);
> +
> +		mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
> +				     RG_XTP_LN_TX_IMPSEL_NMOS, tx_impsel.nmos);
> +	}

[Severity: Low]
Are these register updates redundant?

When calibration is supported, mtk_pcie_sphy_apply_calibration() already
writes cal->tx_impsel.pmos and cal->tx_impsel.nmos to PEXTP_ANA_LN_TRX_A0.

Then this loop immediately overwrites the same registers with the exact same
values.

[ ... ]
> +static int mtk_pcie_sphy_get_one_cal_para(struct device *dev, const char *name, u8 max_val)
> +{
[ ... ]
> +	ret = nvmem_cell_read_u8(dev, name, &tmp);
> +	if (ret == 0)
> +		buf = tmp;
> +	else
> +		ret = nvmem_cell_read_u16(dev, name, &buf);
> +
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Cannot get calibration data for %s\n", name);

[Severity: High]
Will this cause spurious error logs or fail the probe when calibration data is
missing or unsupported?

If the system does not have CONFIG_NVMEM enabled, nvmem_cell_read_u8() returns
-EOPNOTSUPP, which gets logged as an error here.

This error is then passed back up to mtk_pcie_sphy_get_calibration_data(),
where it is not caught. This will cause the entire driver probe to fail.

Similarly, if the cell is simply missing from the Device Tree, it returns
-ENOENT. This also triggers an error log via dev_err_probe(), even though
missing calibration data is explicitly documented as non-fatal and optional.

> +
> +	if (buf == 0) {
> +		dev_info(dev, "No calibration for %s. Using defaults\n", name);
> +		return -ENOENT;
> +	}

[Severity: Medium]
Is a value of 0 always indicative of missing calibration?

If a valid tuned calibration value is 0, this check will return -ENOENT and
cause mtk_pcie_sphy_get_calibration_data() to discard all calibrations entirely.

This also seems to prevent the quirk for cal-version == 0 from ever being
applied:

[ ... ]
> +static int mtk_pcie_sphy_get_calibration_data(struct mtk_pcie_sphy *pcie_sphy)
> +{
[ ... ]
> +	ret = mtk_pcie_sphy_get_one_cal_para(dev, "cal-version", 15);
> +	if (ret < 0)
> +		goto end;
> +	version = ret;
> +
> +	/* Quirk for eFuse calibration table versions 0 to 4 */
> +	if ((version <= MTK_PCIE_SPHY_CALIBRATION_LAST_QUIRK_VER) &&
> +	    cal->rx_leq_rl_ctle == 10) {

Because mtk_pcie_sphy_get_one_cal_para() returns -ENOENT when it reads a 0,
the version == 0 condition will never be reached.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720135344.84183-1-angelogioacchino.delregno@collabora.com?part=2

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

^ permalink raw reply

* Re: [PATCH v4 3/8] phy: rockchip: samsung-hdptx: Fix rate recalculation for 3.2GHz FRL
From: Dmitry Baryshkov @ 2026-07-20 14:12 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
	Dmitry Baryshkov, kernel, linux-phy, linux-arm-kernel,
	linux-rockchip, linux-kernel, Sashiko
In-Reply-To: <20260612-hdptx-clk-fixes-v4-3-ce5e1d456cda@collabora.com>

On Fri, Jun 12, 2026 at 02:45:22AM +0300, Cristian Ciocaltea wrote:
> rk_hdptx_phy_clk_calc_rate_from_pll_cfg() is currently unable to handle
> cascade mode for the 3.2GHz FRL operating mode, as it relies solely on
> LCPLL_LCVCO_MODE_EN_MASK to determinate the rate from the
> rk_hdptx_frl_lcpll_cfg array.  Since there is no entry for this
> particular rate, the function returns 0.
> 
> This is the only rate which requires LC_REF_CLK_SEL to be set in
> GRF_HDPTX_CON0, hence extend the FRL matching accordingly.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260611-hdptx-clk-fixes-v3-0-67b1b0c00e16@collabora.com?part=1
> Fixes: de5dba833118 ("phy: rockchip: samsung-hdptx: Add HDMI 2.1 FRL support")
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 33 ++++++++++++++++-------
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

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

^ permalink raw reply

* Re: [PATCH v4 2/8] phy: rockchip: samsung-hdptx: Prevent divide-by-zero when computing clk rate
From: Dmitry Baryshkov @ 2026-07-20 14:10 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: sashiko-reviews, neil.armstrong, olteanv, linux-phy, vkoul
In-Reply-To: <90ac7bbc-977b-432a-9f17-b7280e19d819@collabora.com>

On Fri, Jun 12, 2026 at 03:18:52AM +0300, Cristian Ciocaltea wrote:
> On 6/12/26 2:57 AM, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > 
> > Pre-existing issues:
> > - [High] Potential integer underflow in `fout` calculation when hardware state is misconfigured, leading to a massive bogus clock rate that could hang downstream components.
> > --
> 
> If the hardware reached such a damaged state, the kernel is likely already encountering more serious issues.
> 
> Regardless, I will address this in the "FRL TxFFE level control" series, which is currently blocked by this one, to avoid any further delays.
> 

With the overflow handled:


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>



-- 
With best wishes
Dmitry

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

^ permalink raw reply

* Re: [PATCH v3 phy-next 7/8] soc: fsl: guts: implement the RCW override procedure
From: sashiko-bot @ 2026-07-20 14:03 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: neil.armstrong, olteanv, linux-phy, vkoul, conor+dt, devicetree,
	robh
In-Reply-To: <20260720133642.136324-8-vladimir.oltean@nxp.com>

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

New issues:
- [High] SerDes block index validation incorrectly uses `>=` instead of `>`, rejecting the highest valid SerDes index and breaking configuration.
- [High] SoC-specific GENMASK macros execute undefined behavior via negative shifts when `lane >= 4`.
- [High] Bulk RCW override initialization clobbers active configurations on uninitialized lanes.
- [Medium] Non-atomic read-modify-write on shared state causes lost lane initializations.
- [Medium] Extremely short 1 microsecond timeout for hardware register state reflection causes spurious failures.

Pre-existing issues:
- [High] Dereferencing `soc_dev_attr` on the error path causes a kernel panic.
--

--- Patch [7]: [PATCH v3 phy-next 7/8] soc: fsl: guts: implement the RCW override procedure ---
commit a1f1c62bf1e452bc97a7f348215d8401bd095833
Author: Ioana Ciornei <ioana.ciornei@nxp.com>
soc: fsl: guts: implement the RCW override procedure

This commit adds support for the RCW override procedure to enable runtime
reconfiguration of SerDes lane protocols through the DCFG DCSR space.
It exports functions to notify boot time modes, validate protocol changes,
and execute the RCW procedure on specific SoCs.

> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> index b9973ca3a440d..4d49d1b953007 100644
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
[ ... ]
> @@ -210,6 +210,13 @@ static int fsl_guts_rcw_rmw(int offset, u32 val, u32 mask)
>  {
>  	u32 rcwcr, rcwsr = fsl_guts_read(&soc.dcfg_ccsr->rcwsr[offset]);
>  
>  	rcwcr = rcwsr & ~mask;
>  	rcwcr |= val;
>  	fsl_guts_write(&soc.dcfg_dcsr->rcwcr[offset], rcwcr);
>  
>  	/* Updates to RCWCR should be visible back in RCWSR */
>  	return read_poll_timeout_atomic(fsl_guts_read, rcwsr, rcwsr == rcwcr,
>  					0, RCW_TIMEOUT_US, false,
>  					&soc.dcfg_ccsr->rcwsr[offset]);
>  }

[Severity: Medium]
Is a 1 microsecond timeout sufficient for cross-domain hardware reflections?

The atomic polling loop strictly uses RCW_TIMEOUT_US set to 1, which might be
too brief for the hardware to propagate the state across clock domains. This
could lead to spurious -ETIMEDOUT failures when configuring SerDes lane modes.

[ ... ]
> +static int
> +fsl_guts_serdes_get_rcw_override(int serdes_idx, int lane,
> +				 enum lynx_lane_mode lane_mode,
> +				 struct fsl_soc_serdes_rcw_override *override)
> +{
> +	const struct fsl_soc_data *soc_data = soc.data;
> +
> +	if (!soc_data)
> +		return -ENODEV;
> +
> +	if (serdes_idx >= soc_data->num_serdes || serdes_idx <= 0)
> +		return -ERANGE;

[Severity: High]
Does this bounds check incorrectly reject the highest valid SerDes index?

If soc_data->num_serdes is 2, a client driver attempting to initialize
serdes_idx 2 will trigger this return path because 2 >= 2 evaluates to true.
Should this use > instead of >= to allow configuring the last SerDes block?

> +
> +	if (lane >= MAX_NUM_LANES || lane < 0)
> +		return -ERANGE;

[Severity: High]
Can this loose bounds check lead to undefined behavior in the SoC-specific
macros?

MAX_NUM_LANES is 8, but macros like LS1088A_RCWSR30_SRDS_PRTCL_S2_LNn
assume a maximum of 4 lanes:

GENMASK(3 + 4 * (3 - (lane)), 4 * (3 - (lane)))

If a PHY driver requests a lane index of 4 or higher on LS1088A, this macro
evaluates to GENMASK(-1, -4), which causes negative bitwise shifts.

[ ... ]
> +int fsl_guts_lane_init(int serdes_idx, int lane, enum lynx_lane_mode lane_mode)
> +{
> +	int err;
> +
> +	err = fsl_guts_lane_validate(serdes_idx, lane, lane_mode);
> +	if (err)
> +		return err;
> +
> +	soc.lane_mode[serdes_idx - 1][lane] = lane_mode;
> +	soc.lanes_initialized[serdes_idx - 1] |= BIT(lane);

[Severity: Medium]
Could concurrent PHY probes cause lost lane initialization status here?

This performs a non-atomic read-modify-write on the shared global state array
without holding a lock. If two PHY instances on the same SerDes controller
probe asynchronously, one update to soc.lanes_initialized might overwrite the
other.

[ ... ]
> +static int ls2088a_serdes_init_rcwcr(int serdes_idx)
> +{
> +	int i, err;
> +	u32 reg;
[ ... ]
> +	/* We need to configure the initial state of all lanes for
> +	 * the SerDes block #1
> +	 */
> +	for_each_set_bit(i, &soc.lanes_initialized[serdes_idx - 1], MAX_NUM_LANES)
> +		if (lynx_lane_mode_uses_xgmii_mac(soc.lane_mode[serdes_idx - 1][i]))
> +			reg |= LS2088A_RCWSR30_SRDS_CLK_SEL_XGMII_Ln_S1(i);
> +
> +	err = fsl_guts_rcw_rmw(29, reg,
> +			       LS2088A_RCWSR30_SRDS_CLK_EN_SEL_XGMII_S1 |
> +			       LS2088A_RCWSR30_SRDS_CLK_SEL_MSK);

[Severity: High]
Does this bulk RCW override initialization clobber active configurations on
uninitialized lanes?

If a client driver for lane A calls fsl_guts_lane_set_mode() before sibling
lane B has initialized, this read-modify-write uses a mask spanning all
lanes (LS2088A_RCWSR30_SRDS_CLK_SEL_MSK). This forcefully writes zeroes
to lane B's configuration because it relies purely on the software state
in soc.lanes_initialized, potentially discarding its bootloader-configured
active state.

[ ... ]
>  static int __init fsl_guts_init(void)
>  {
>  	struct soc_device_attribute *soc_dev_attr = NULL;
[ ... ]
>  err_nomem:
>  	ret = -ENOMEM;
>  err:
>  	kfree(soc_dev_attr->family);
>  	kfree(soc_dev_attr->soc_id);
>  	kfree(soc_dev_attr->revision);
>  	kfree(soc_dev_attr->serial_number);
>  	kfree(soc_dev_attr);

[Severity: High]
This is a pre-existing issue, but can dereferencing soc_dev_attr here cause
a kernel panic on the error path?

If earlier initialization steps fail, such as of_iomap() failing and jumping
to err_nomem before soc_dev_attr is allocated, soc_dev_attr will remain NULL.
The unconditional access to member variables like soc_dev_attr->family will
result in a NULL pointer dereference.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133642.136324-1-vladimir.oltean@nxp.com?part=7

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

^ permalink raw reply

* Re: [PATCH v2 1/2] dt-bindings: phy: Document MT8196 MediaTek PCI-Express Gen4 S-PHY
From: sashiko-bot @ 2026-07-20 14:01 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: olteanv, linux-phy, conor+dt, neil.armstrong, vkoul, devicetree,
	robh
In-Reply-To: <20260720135344.84183-2-angelogioacchino.delregno@collabora.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Typo in the device tree binding description listing an invalid PCIe link rate (6.0 GT/s instead of 8.0 GT/s).
--

--- Patch [1]: [PATCH v2 1/2] dt-bindings: phy: Document MT8196 MediaTek PCI-Express Gen4 S-PHY ---
commit 44ee552d23d168aeb37986df1a94ba1101ff1252
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

dt-bindings: phy: Document MT8196 MediaTek PCI-Express Gen4 S-PHY

This commit adds device tree bindings for the MT8196 MediaTek PCI-Express
Gen4 S-PHY. It documents the capabilities, register spaces, and required
properties for this PHY.

> diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
> new file mode 100644
> index 0000000000000..c24871f7e0e5a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
> @@ -0,0 +1,93 @@

[ ... ]

> +description:
> +  The MediaTek PCI-Express Gen4 S-PHY is capable of data serialization
> +  and de-serialization with 8, 10, 128 or 130 bits (16GT/s) encoding
> +  and decoding, has analog buffers, elastic buffers and mechanisms for
> +  receiver detection.
> +  This PHY supports link rates of 2.5, 5.0, 6.0 and 16.0 GT/s per-lane
                                              ^^^

[Severity: Low]
This isn't a bug, but is 6.0 GT/s a typo for 8.0 GT/s here? 

Standard PCIe link rates are typically 2.5, 5.0, 8.0 (Gen3), and 16.0 GT/s,
while 6.0 GT/s is a rate usually associated with SATA rather than PCIe.

> +  in both Root Complex (RC) and End Point (EP) modes, depending on the
> +  chosen port.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720135344.84183-1-angelogioacchino.delregno@collabora.com?part=1

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

^ permalink raw reply

* Re: [PATCH v3 phy-next 2/8] soc: fsl: guts: add a global structure to hold state
From: Vladimir Oltean @ 2026-07-20 14:01 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: devicetree, robh, conor+dt, vkoul, neil.armstrong, olteanv,
	linux-phy
In-Reply-To: <20260720135049.8AC231F00A3D@smtp.kernel.org>

On Mon, Jul 20, 2026 at 01:50:49PM +0000, sashiko-bot@kernel.org wrote:
> [Severity: High]
> Will jumping to err_nomem cause a NULL pointer dereference?
> 
> If the allocation of soc_dev_attr fails, the code now jumps to the error
> handling path which dereferences soc_dev_attr to free its members:
> 
> > @@ -269,6 +271,8 @@ static int __init fsl_guts_init(void)
> >  	kfree(soc_dev_attr->revision);
> >  	kfree(soc_dev_attr->serial_number);
> 
> Since soc_dev_attr is NULL when this allocation fails, won't evaluating
> soc_dev_attr->revision and other members result in a kernel panic?

yeah.

I tried to be nice and play with the existing "err_nomem" and "err"
error handling labels, but the truth is that a single error cleanup path
is just an antipattern that doesn't scale as code becomes more complex.

Instead of having a single error handling procedure which needs to
carefully fend off from various invalid contexts from all the goto
sites, the standard convention is to have an incremental set of labels
which undo just the setup that was done up to the goto jump site.

I'll refactor fsl_guts_init() to use that convention for v4.

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

^ permalink raw reply

* Re: [PATCH v3 phy-next 5/8] soc: fsl: guts: make fsl_soc_data available after fsl_guts_init()
From: sashiko-bot @ 2026-07-20 13:58 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: robh, conor+dt, devicetree, linux-phy, olteanv, vkoul,
	neil.armstrong
In-Reply-To: <20260720133642.136324-6-vladimir.oltean@nxp.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] NULL pointer dereference in the error handling path of `fsl_guts_init()`.
--

--- Patch [5]: [PATCH v3 phy-next 5/8] soc: fsl: guts: make fsl_soc_data available after fsl_guts_init() ---
commit d316b69e1c587b56ad6f914ddbd2e412490c8594
Author: Vladimir Oltean <vladimir.oltean@nxp.com>
soc: fsl: guts: make fsl_soc_data available after fsl_guts_init()

This commit preserves the soc_data determined during fsl_guts_init() so it
can be referenced later for RCW override methods. It updates the error
handling paths to properly clean up the saved state if initialization fails.

> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
> @@ -231,10 +232,9 @@ static int __init fsl_guts_init(void)
>  {
> -	struct soc_device_attribute *soc_dev_attr;
> +	struct soc_device_attribute *soc_dev_attr = NULL;
>  	static struct soc_device *soc_dev;

[ ... ]

> @@ -243,12 +243,12 @@ static int __init fsl_guts_init(void)
>  	np = of_find_matching_node_and_match(NULL, fsl_guts_of_match, &match);
>  	if (!np)
>  		return 0;
> -	soc_data = match->data;
> +	soc.data = match->data;
>  
>  	soc.dcfg_ccsr = of_iomap(np, DCFG_CCSR);
>  	if (!soc.dcfg_ccsr) {
>  		of_node_put(np);
> -		return -ENOMEM;
> +		goto err_nomem;
>  	}

[Severity: High]
If of_iomap() fails here, the code executes a goto to the error path while
soc_dev_attr is still explicitly NULL. 

Down in the error cleanup path, the code unconditionally dereferences 
soc_dev_attr to retrieve its members:

> @@ -311,8 +311,11 @@ static int __init fsl_guts_init(void)
>  	kfree(soc_dev_attr->revision);
>  	kfree(soc_dev_attr->serial_number);
>  	kfree(soc_dev_attr);

Will this trigger a NULL pointer dereference?

While kfree() itself handles NULL pointers gracefully, evaluating
soc_dev_attr->revision and soc_dev_attr->serial_number requires reading
memory from the NULL soc_dev_attr pointer before passing the argument to
kfree().

> -	iounmap(soc.dcfg_ccsr);
> -	soc.dcfg_ccsr = NULL;
> +	if (soc.dcfg_ccsr) {
> +		iounmap(soc.dcfg_ccsr);
> +		soc.dcfg_ccsr = NULL;
> +	}
> +	soc.data = NULL;
>  
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133642.136324-1-vladimir.oltean@nxp.com?part=5

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

^ permalink raw reply

* Re: [PATCH v3 phy-next 6/8] dt-bindings: fsl: layerscape-dcfg: define DCFG_DCSR region
From: sashiko-bot @ 2026-07-20 13:57 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: neil.armstrong, linux-phy, robh, devicetree, olteanv, conor+dt,
	vkoul
In-Reply-To: <20260720133642.136324-7-vladimir.oltean@nxp.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Dereferencing NULL pointer `soc_dev_attr` on the error path.
--

--- Patch [6]: [PATCH v3 phy-next 6/8] dt-bindings: fsl: layerscape-dcfg: define DCFG_DCSR region ---
commit 87aa71bb5ea85cc0388e2ad87f720397de246b0d
Author: Vladimir Oltean <vladimir.oltean@nxp.com>

dt-bindings: fsl: layerscape-dcfg: define DCFG_DCSR region

This commit defines a secondary "reg" array element in the DCFG binding
for the hidden DCSR register space. Exposing this area permits operating
systems to perform Reset Configuration Word (RCW) overrides, enabling
runtime reconfiguration of networking SerDes lanes.

> diff --git a/Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml b/Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
> index 3fb0534ea597b..fc14fd0bf84b1 100644
> --- a/Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
> +++ b/Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
> @@ -36,7 +36,20 @@ properties:
>            - const: simple-mfd
>  
>    reg:
> -    maxItems: 1
> +    minItems: 1

[ ... ]

[Severity: High]
Does the error path in fsl_guts_init() cause a NULL pointer dereference?

drivers/soc/fsl/guts.c:fsl_guts_init() {
    ...
	soc.dcfg_ccsr = of_iomap(np, DCFG_CCSR);
	if (!soc.dcfg_ccsr) {
		of_node_put(np);
		goto err_nomem;
	}
    ...
err_nomem:
	ret = -ENOMEM;
err:
	kfree(soc_dev_attr->family);
	kfree(soc_dev_attr->soc_id);
    ...
}

If of_iomap() or the allocation for soc_dev_attr fails in fsl_guts_init(),
the error handling path jumps to err_nomem while soc_dev_attr is still NULL.

The cleanup code at the err label unconditionally evaluates
soc_dev_attr->family and soc_dev_attr->soc_id before passing them to
kfree(). Reading these fields dereferences the NULL soc_dev_attr pointer,
which can cause an immediate page fault during initialization.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133642.136324-1-vladimir.oltean@nxp.com?part=6

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

^ permalink raw reply

* [PATCH v2 2/2] phy: mediatek: Add support for PCI-Express Gen4 S-PHY
From: AngeloGioacchino Del Regno @ 2026-07-20 13:53 UTC (permalink / raw)
  To: chunfeng.yun
  Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
	angelogioacchino.delregno, linux-arm-kernel, linux-mediatek,
	linux-phy, devicetree, linux-kernel, kernel
In-Reply-To: <20260720135344.84183-1-angelogioacchino.delregno@collabora.com>

Add support for the PCI-Express Gen4 S-PHY found in the latest
MediaTek SoCs, including MT8196, MT8894 and similar.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/phy/mediatek/Kconfig             |   9 +
 drivers/phy/mediatek/Makefile            |   1 +
 drivers/phy/mediatek/phy-mtk-pcie-sphy.c | 561 +++++++++++++++++++++++
 3 files changed, 571 insertions(+)
 create mode 100644 drivers/phy/mediatek/phy-mtk-pcie-sphy.c

diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
index ba6461350951..77236f3084c8 100644
--- a/drivers/phy/mediatek/Kconfig
+++ b/drivers/phy/mediatek/Kconfig
@@ -13,6 +13,15 @@ config PHY_MTK_PCIE
 	  callback for PCIe GEN3 port, it supports software efuse
 	  initialization.
 
+config PHY_MTK_PCIE_SPHY
+	tristate "MediaTek PCIe S-PHY Driver"
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	depends on OF
+	select GENERIC_PHY
+	help
+	  Say 'Y' here to add support for MediaTek PCIe S-PHY driver for
+	  PCI-Express Gen4 controllers as found in MT6991, MT8196 and others.
+
 config PHY_MTK_XFI_TPHY
 	tristate "MediaTek 10GE SerDes XFI T-PHY driver"
 	depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
index ed0da708759b..7e984c7cfea3 100644
--- a/drivers/phy/mediatek/Makefile
+++ b/drivers/phy/mediatek/Makefile
@@ -5,6 +5,7 @@
 
 obj-$(CONFIG_PHY_MTK_DP)		+= phy-mtk-dp.o
 obj-$(CONFIG_PHY_MTK_PCIE)		+= phy-mtk-pcie.o
+obj-$(CONFIG_PHY_MTK_PCIE_SPHY)		+= phy-mtk-pcie-sphy.o
 obj-$(CONFIG_PHY_MTK_TPHY)		+= phy-mtk-tphy.o
 obj-$(CONFIG_PHY_MTK_UFS)		+= phy-mtk-ufs.o
 obj-$(CONFIG_PHY_MTK_XSPHY)		+= phy-mtk-xsphy.o
diff --git a/drivers/phy/mediatek/phy-mtk-pcie-sphy.c b/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
new file mode 100644
index 000000000000..6c1362b70fab
--- /dev/null
+++ b/drivers/phy/mediatek/phy-mtk-pcie-sphy.c
@@ -0,0 +1,561 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 MediaTek Inc.
+ * Copyright (c) 2026 Collabora Ltd.
+ *                    AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+
+#include "phy-mtk-io.h"
+
+/* PHY System Interface (SIF) registers */
+#define PEXTP_DIG_GLB_TOP				0x20
+#  define RG_XTP_BYPASS_PIPE_RST_RC			BIT(17)
+#define PEXTP_DIG_GLB_CKBG0				0x30
+#  define RG_XTP_CKBG_XTAL_STABLE_TIME_SEL		GENMASK(25, 16)
+#define PEXTP_DIG_GLB_TPLL_CTL0				0x38
+#  define RG_XTP_TPLL_SET_STABLE_TIME_SEL		GENMASK(7, 2)
+#  define RG_XTP_TPLL_PWE_ON_STABLE_TIME_SEL		GENMASK(9, 8)
+#define PEXTP_DIG_GLB_CLKREQ_CTL			0x50
+#  define RG_XTP_CKM_EN_L1S0				BIT(13)
+#  define RG_XTP_CKM_EN_L1S1				BIT(14)
+#define PEXTP_DIG_GLB_TPLL_CTL2				0xf4
+#  define RG_XTP_TPLL_ISO_EN_STABLE_TIME_SEL		GENMASK(13, 12)
+
+/* PHY System Interface Digital registers */
+#define PEXTP_DIG_LN_TRX_PIPE_IF_17			0x30e8
+#  define RG_XTP_LN_RX_LF_CTLE_CSEL_GEN4		GENMASK(14, 12)
+#define PEXTP_DIG_LN_RX_F0				0x50f0
+#  define RG_XTP_LN_RX_GEN1_CTLE1_CSEL			GENMASK(3, 0)
+#  define RG_XTP_LN_RX_GEN2_CTLE1_CSEL			GENMASK(7, 4)
+#  define RG_XTP_LN_RX_GEN3_CTLE1_CSEL			GENMASK(11, 8)
+#  define RG_XTP_LN_RX_GEN4_CTLE1_CSEL			GENMASK(15, 12)
+#define PEXTP_DIG_LN_RX2_AEQ_EDGE_0			0x6004
+#  define RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN3		GENMASK(21, 16)
+#  define RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN4		GENMASK(29, 24)
+#  define AEQ_EGEQ_RATIO_GEN3_TO_22			0x16
+#  define AEQ_EGEQ_RATIO_GEN4_TO_22			0x16
+
+/* PHY System Interface Analog registers */
+#define PEXTP_ANA_GLB_TPLL1_RSVD			0x902c
+#  define RG_XTP_GLB_TPLL1_P_PATH_GAIN			GENMASK(2, 0)
+#define PEXTP_ANA_GLB_BIAS_0				0x9060
+#  define RG_XTP_GLB_BIAS_INTR_CTRL			GENMASK(5, 0)
+#define PEXTP_ANA_GLB_BIAS_1				0x90c0
+#  define RG_XTP_GLB_BIAS_V2V_VTRIM			GENMASK(9, 6)
+#define PEXTP_ANA_LN_TRX_0C				0xa00c
+#  define RG_XTP_LN_TX_RSWN_IMPSEL			GENMASK(20, 16)
+#define PEXTP_ANA_LN_TRX_34				0xa034
+#  define RG_XTP_LN_RX_FE				BIT(15)
+#define PEXTP_ANA_LN_TRX_6C				0xa06c
+#  define RG_XTP_LN_RX_AEQ_CTLE_ERR_TYPE		GENMASK(14, 13)
+#   define AEQ_CTLE_SEARCH_ERR_TYPE_H1P5		0
+#   define AEQ_CTLE_SEARCH_ERR_TYPE_H1P5_H2P5		1
+#   define AEQ_CTLE_SEARCH_ERR_TYPE_P1P5_H2P5_H3P5	2
+#define PEXTP_ANA_LN_TRX_A0				0xa0a0
+#  define RG_XTP_LN_TX_IMPSEL_PMOS			GENMASK(4, 0)
+#  define RG_XTP_LN_TX_IMPSEL_NMOS			GENMASK(11, 7)
+#  define RG_XTP_LN_RX_IMPSEL				GENMASK(15, 12)
+#define PEXTP_ANA_LN_TRX_A8				0xa0a8
+#  define RG_XTP_LN_RX_LEQ_RL_CTLE_CAL			GENMASK(6, 2)
+#  define RG_XTP_LN_RX_LEQ_RL_VGA_CAL			GENMASK(11, 7)
+#  define RG_XTP_LN_RX_LEQ_RL_DFE_CAL			GENMASK(23, 19)
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_4			0xb004
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_8			0xb008
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_C			0xb00c
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_10		0xb010
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_14		0xb014
+#define PEXTP_DIG_LN_TX_LC_TABLE_RSWN_18		0xb018
+#  define RG_XTP_LN_TX_LC_PRESET_MGx_Px_CM1		GENMASK(5, 0)
+#  define RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0		GENMASK(13, 8)
+#  define RG_XTP_LN_TX_LC_PRESET_MGx_Px_CP1		GENMASK(21, 16)
+#define PEXTP_REG_LANE(x)				((x) * 0x100)
+
+/* PHY Clock Management (CKM) registers */
+#define XTP_CKM_FORCE_6					0x38
+#  define RG_CKM_BIAS_WAIT_PRD_US			GENMASK(21, 16)
+#define XTP_CKM_REG_SPLL_FBKDIV_5			0xd4
+#  define RG_CKM_CKTX_IMPSEL_PMOS			GENMASK(19, 16)
+#  define RG_CKM_CKTX_IMPSEL_NMOS			GENMASK(23, 20)
+#  define RG_CKM_CKTX_IMPSEL_SW				GENMASK(27, 24)
+
+/* Calibration data from eFuses */
+#define MTK_PCIE_SPHY_CALIBRATION_MAX_DATA_LANES	2
+#define MTK_PCIE_SPHY_CALIBRATION_LAST_QUIRK_VER	4
+
+/**
+ * struct mtk_pcie_sphy_imp_sel - Impedance Selection parameters
+ * @pmos: Impedance selection for P-Channel MOSFET
+ * @nmos: Impedance selection for N-Channel MOSFET
+ */
+struct mtk_pcie_sphy_imp_sel {
+	u8 pmos;
+	u8 nmos;
+};
+
+/**
+ * struct mtk_pcie_sphy_efuse - eFuse calibration data for S-PHY
+ * @int_r_ctrl:     Internal resistor selection of TX Bias Current
+ * @xtp_vtrim:      XTP Bias V2V voltage calibration
+ * @cktx_impsel:    SPLL CKTX Impedance Selection (P and N MOSFET)
+ * @cktx_r_mid:     SPLL CKTX Intermediate Transition Impedance (Rmid)
+ * @rx_leq_rl_ctle: RX Front-End Return Loss Continuous Time Linear Equalization value
+ * @rx_leq_rl_vga:  RX Front-End Return Loss Variable Gain Amplifier value
+ * @rx_leq_rl_dfe:  RX Front-End Return Loss Decision Feedback Equalization value
+ * @rx_impsel:      RX Impedance Selection
+ * @tx_impsel:      TX Impedance Selection (P and N MOSFET)
+ * @tx_rswn_impsel: TX RSWn (Switch Resistance) impedance selection
+ * @supported:      eFuse calibration data is supported
+ */
+struct mtk_pcie_sphy_efuse {
+	u8 int_r_ctrl;
+	u8 xtp_vtrim;
+	struct mtk_pcie_sphy_imp_sel cktx_impsel;
+	u8 cktx_r_mid;
+	u8 rx_leq_rl_ctle;
+	u8 rx_leq_rl_vga;
+	u8 rx_leq_rl_dfe;
+	u8 rx_impsel;
+	struct mtk_pcie_sphy_imp_sel tx_impsel;
+	u8 tx_rswn_impsel[MTK_PCIE_SPHY_CALIBRATION_MAX_DATA_LANES];
+	bool supported;
+};
+
+/**
+ * struct mtk_pcie_sphy - PCI-Express S-PHY driver main structure
+ * @dev:         Pointer to device structure
+ * @phy:         Pointer to generic phy structure
+ * @sif_base:    IO mapped register base address of system interface
+ * @ckm_base:    IO mapped register base address of clock management interface
+ * @num_lanes:   Number of lanes
+ * @calibration: eFuse calibration data for S-PHY
+ */
+struct mtk_pcie_sphy {
+	struct device *dev;
+	struct phy *phy;
+	void __iomem *sif_base;
+	void __iomem *ckm_base;
+	u8 num_lanes;
+	struct mtk_pcie_sphy_efuse calibration;
+};
+
+static void mtk_pcie_sphy_apply_calibration(struct mtk_pcie_sphy *pcie_sphy)
+{
+	struct mtk_pcie_sphy_efuse *cal = &pcie_sphy->calibration;
+	int i;
+
+	mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_ANA_GLB_BIAS_0,
+			     RG_XTP_GLB_BIAS_INTR_CTRL, cal->int_r_ctrl);
+
+	mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_ANA_GLB_BIAS_1,
+			     RG_XTP_GLB_BIAS_V2V_VTRIM, cal->xtp_vtrim);
+
+	mtk_phy_update_field(pcie_sphy->ckm_base + XTP_CKM_REG_SPLL_FBKDIV_5,
+			     RG_CKM_CKTX_IMPSEL_PMOS, cal->cktx_impsel.pmos);
+
+	mtk_phy_update_field(pcie_sphy->ckm_base + XTP_CKM_REG_SPLL_FBKDIV_5,
+			     RG_CKM_CKTX_IMPSEL_NMOS, cal->cktx_impsel.nmos);
+
+	mtk_phy_update_field(pcie_sphy->ckm_base + XTP_CKM_REG_SPLL_FBKDIV_5,
+			     RG_CKM_CKTX_IMPSEL_SW, cal->cktx_r_mid);
+
+	for (i = 0; i < pcie_sphy->num_lanes; i++) {
+		void __iomem *sif_lane_base = pcie_sphy->sif_base + PEXTP_REG_LANE(i);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_0C,
+				     RG_XTP_LN_TX_RSWN_IMPSEL, cal->tx_rswn_impsel[i]);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A8,
+				     RG_XTP_LN_RX_LEQ_RL_CTLE_CAL, cal->rx_leq_rl_ctle);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A8,
+				     RG_XTP_LN_RX_LEQ_RL_VGA_CAL, cal->rx_leq_rl_vga);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A8,
+				     RG_XTP_LN_RX_LEQ_RL_DFE_CAL, cal->rx_leq_rl_dfe);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
+				     RG_XTP_LN_RX_IMPSEL, cal->rx_impsel);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
+				     RG_XTP_LN_TX_IMPSEL_PMOS, cal->tx_impsel.pmos);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
+				     RG_XTP_LN_TX_IMPSEL_NMOS, cal->tx_impsel.nmos);
+	}
+}
+
+/**
+ * mtk_pcie_sphy_init() - Initialize the PCI-Express S-PHY
+ * @phy: the phy to be initialized
+ *
+ * The hardware settings will be reset during suspend, it should be
+ * reinitialized when the consumer calls phy_init() again on resume.
+ */
+static int mtk_pcie_sphy_init(struct phy *phy)
+{
+	struct mtk_pcie_sphy *pcie_sphy = phy_get_drvdata(phy);
+	struct mtk_pcie_sphy_imp_sel tx_impsel;
+	int i;
+
+	/* Set CKM Bias wait time to 4 microseconds */
+	mtk_phy_update_field(pcie_sphy->ckm_base + XTP_CKM_FORCE_6,
+			     RG_CKM_BIAS_WAIT_PRD_US, 4);
+
+	/* TPLL needs 63 ref_ck ticks to stabilize when setting frequency */
+	mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_DIG_GLB_TPLL_CTL0,
+			     RG_XTP_TPLL_SET_STABLE_TIME_SEL, 63);
+
+	/* TPLL needs 3 ref_ck ticks to stabilize when powering on... */
+	mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_DIG_GLB_TPLL_CTL0,
+			     RG_XTP_TPLL_PWE_ON_STABLE_TIME_SEL, 3);
+
+	/* ...and the same goes for setting isolation */
+	mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_DIG_GLB_TPLL_CTL2,
+			     RG_XTP_TPLL_ISO_EN_STABLE_TIME_SEL, 3);
+
+	/* XTAL doesn't need any stabilization time */
+	mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_DIG_GLB_CKBG0,
+			     RG_XTP_CKBG_XTAL_STABLE_TIME_SEL, 0);
+
+	/* Keep pextp_ckm enabled when in L1SS_L1S1 state */
+	mtk_phy_clear_bits(pcie_sphy->sif_base + PEXTP_DIG_GLB_CLKREQ_CTL, RG_XTP_CKM_EN_L1S1);
+
+	/* Set PIPE to reset TPLL */
+	mtk_phy_clear_bits(pcie_sphy->sif_base + PEXTP_DIG_GLB_TOP, RG_XTP_BYPASS_PIPE_RST_RC);
+
+	/* Set TPLL P-Path gain compensation to 1 */
+	mtk_phy_update_field(pcie_sphy->sif_base + PEXTP_ANA_GLB_TPLL1_RSVD,
+			     RG_XTP_GLB_TPLL1_P_PATH_GAIN, 1);
+
+	for (i = 0; i < pcie_sphy->num_lanes; i++) {
+		void __iomem *sif_lane_base = pcie_sphy->sif_base + PEXTP_REG_LANE(i);
+
+		/* Set RX Lane AEQ CTRL-E Search Error type to h1.5 + h2.5 */
+		mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_6C,
+				     RG_XTP_LN_RX_AEQ_CTLE_ERR_TYPE,
+				     AEQ_CTLE_SEARCH_ERR_TYPE_H1P5_H2P5);
+
+		mtk_phy_set_bits(sif_lane_base + PEXTP_ANA_LN_TRX_34, RG_XTP_LN_RX_FE);
+
+		/* TRX: Select CTLE1 for RX Lane AutoEQ CTRL-E Setting on Gen4 */
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TRX_PIPE_IF_17,
+				     RG_XTP_LN_RX_LF_CTLE_CSEL_GEN4, 1);
+
+		/* Set RX Lane AutoEQ CTRL-E for PCI-Express Gen1 to Gen 4 */
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX_F0,
+				     RG_XTP_LN_RX_GEN1_CTLE1_CSEL, 13);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX_F0,
+				     RG_XTP_LN_RX_GEN2_CTLE1_CSEL, 13);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX_F0,
+				     RG_XTP_LN_RX_GEN3_CTLE1_CSEL, 13);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX_F0,
+				     RG_XTP_LN_RX_GEN4_CTLE1_CSEL, 0);
+
+		/* Set RX Lane AutoEQ's Edge EQ Ratio to 22 * 0.0625 = 1.375 */
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX2_AEQ_EDGE_0,
+				     RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN3, 22);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_RX2_AEQ_EDGE_0,
+				     RG_XTP_LN_RX_AEQ_EGEQ_RATIO_GEN4, 22);
+
+		/* Setup Digital lane TX Link Characteristics Table */
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_4,
+				     RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 10);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_4,
+				     RG_XTP_LN_TX_LC_PRESET_MGx_Px_CP1, 2);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_8,
+				     RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 11);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_8,
+				     RG_XTP_LN_TX_LC_PRESET_MGx_Px_CP1, 1);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_C,
+				     RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 12);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_10,
+				     RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 13);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_10,
+				     RG_XTP_LN_TX_LC_PRESET_MGx_Px_CM1, 1);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_14,
+				     RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 11);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_14,
+				     RG_XTP_LN_TX_LC_PRESET_MGx_Px_CM1, 1);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_18,
+				     RG_XTP_LN_TX_LC_PRESET_MGx_Px_C0, 10);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_DIG_LN_TX_LC_TABLE_RSWN_18,
+				     RG_XTP_LN_TX_LC_PRESET_MGx_Px_CM1, 2);
+	}
+
+	if (pcie_sphy->calibration.supported) {
+		mtk_pcie_sphy_apply_calibration(pcie_sphy);
+
+		tx_impsel.pmos = pcie_sphy->calibration.tx_impsel.pmos;
+		tx_impsel.nmos = pcie_sphy->calibration.tx_impsel.nmos;
+	} else {
+		/* Set P=10, N=9 to prevent EMI if no calibration present */
+		tx_impsel.pmos = 10;
+		tx_impsel.nmos = 9;
+	}
+
+	/* Select TX Impedance on N and P MOSFETs */
+	for (i = 0; i < pcie_sphy->num_lanes; i++) {
+		void __iomem *sif_lane_base = pcie_sphy->sif_base + PEXTP_REG_LANE(i);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
+				     RG_XTP_LN_TX_IMPSEL_PMOS, tx_impsel.pmos);
+
+		mtk_phy_update_field(sif_lane_base + PEXTP_ANA_LN_TRX_A0,
+				     RG_XTP_LN_TX_IMPSEL_NMOS, tx_impsel.nmos);
+	}
+
+	return 0;
+}
+
+static const struct phy_ops mtk_pcie_sphy_ops = {
+	.init	= mtk_pcie_sphy_init,
+	.owner	= THIS_MODULE,
+};
+
+static int mtk_pcie_sphy_get_one_cal_para(struct device *dev, const char *name, u8 max_val)
+{
+	u16 buf;
+	u8 tmp;
+	int ret;
+
+	/*
+	 * All of the calibrations are always max 8 bits long, but some may
+	 * be split between two different 8-bits cells: handle this corner
+	 * case by retrying reading as u16.
+	 */
+	ret = nvmem_cell_read_u8(dev, name, &tmp);
+	if (ret == 0)
+		buf = tmp;
+	else
+		ret = nvmem_cell_read_u16(dev, name, &buf);
+
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Cannot get calibration data for %s\n", name);
+
+	if (buf == 0) {
+		dev_info(dev, "No calibration for %s. Using defaults\n", name);
+		return -ENOENT;
+	}
+
+	if (buf > max_val)
+		return dev_err_probe(dev, -ERANGE,
+				     "Bad value %u retrieved for %s.\n", buf, name);
+
+	return buf;
+}
+
+static int mtk_pcie_sphy_get_calibration_data(struct mtk_pcie_sphy *pcie_sphy)
+{
+	struct mtk_pcie_sphy_efuse *cal = &pcie_sphy->calibration;
+	struct device *dev = pcie_sphy->dev;
+	u8 version;
+	int ret;
+
+	ret = mtk_pcie_sphy_get_one_cal_para(dev, "int-r",
+					     FIELD_MAX(RG_XTP_GLB_BIAS_INTR_CTRL));
+	if (ret < 0)
+		goto end;
+	cal->int_r_ctrl = ret;
+
+	ret = mtk_pcie_sphy_get_one_cal_para(dev, "xtp-vtrim",
+					     FIELD_MAX(RG_XTP_GLB_BIAS_V2V_VTRIM));
+	if (ret < 0)
+		goto end;
+	cal->xtp_vtrim = ret;
+
+	ret = mtk_pcie_sphy_get_one_cal_para(dev, "cktx-pmos",
+					     FIELD_MAX(RG_CKM_CKTX_IMPSEL_PMOS));
+	if (ret < 0)
+		goto end;
+	cal->cktx_impsel.pmos = ret;
+
+	ret = mtk_pcie_sphy_get_one_cal_para(dev, "cktx-nmos",
+					     FIELD_MAX(RG_CKM_CKTX_IMPSEL_NMOS));
+	if (ret < 0)
+		goto end;
+	cal->cktx_impsel.nmos = ret;
+
+	ret = mtk_pcie_sphy_get_one_cal_para(dev, "cktx-r-mid",
+					     FIELD_MAX(RG_CKM_CKTX_IMPSEL_SW));
+	if (ret < 0)
+		goto end;
+	cal->cktx_r_mid = ret;
+
+	ret = mtk_pcie_sphy_get_one_cal_para(dev, "rxfe-lanes-rl-ctle",
+					     FIELD_MAX(RG_XTP_LN_RX_LEQ_RL_CTLE_CAL));
+	if (ret < 0)
+		goto end;
+	cal->rx_leq_rl_ctle = ret;
+
+	ret = mtk_pcie_sphy_get_one_cal_para(dev, "rxfe-lanes-rl-vga",
+					     FIELD_MAX(RG_XTP_LN_RX_LEQ_RL_VGA_CAL));
+	if (ret < 0)
+		goto end;
+	cal->rx_leq_rl_vga = ret;
+
+	ret = mtk_pcie_sphy_get_one_cal_para(dev, "rxfe-lanes-rl-dfe",
+					     FIELD_MAX(RG_XTP_LN_RX_LEQ_RL_DFE_CAL));
+	if (ret < 0)
+		goto end;
+	cal->rx_leq_rl_dfe = ret;
+
+	ret = mtk_pcie_sphy_get_one_cal_para(dev, "rx-lanes-imp",
+					     FIELD_MAX(RG_XTP_LN_RX_IMPSEL));
+	if (ret < 0)
+		goto end;
+	cal->rx_impsel = ret;
+
+	ret = mtk_pcie_sphy_get_one_cal_para(dev, "tx-lanes-pmos",
+					     FIELD_MAX(RG_XTP_LN_TX_IMPSEL_PMOS));
+	if (ret < 0)
+		goto end;
+	cal->tx_impsel.pmos = ret;
+
+	ret = mtk_pcie_sphy_get_one_cal_para(dev, "tx-lanes-nmos",
+					     FIELD_MAX(RG_XTP_LN_TX_IMPSEL_NMOS));
+	if (ret < 0)
+		goto end;
+	cal->tx_impsel.nmos = ret;
+
+	ret = mtk_pcie_sphy_get_one_cal_para(dev, "tx-ln0-rswn",
+					     FIELD_MAX(RG_XTP_LN_TX_RSWN_IMPSEL));
+	if (ret < 0)
+		goto end;
+	cal->tx_rswn_impsel[0] = ret;
+
+	if (pcie_sphy->num_lanes == 2) {
+		ret = mtk_pcie_sphy_get_one_cal_para(dev, "tx-ln1-rswn",
+						     FIELD_MAX(RG_XTP_LN_TX_RSWN_IMPSEL));
+		if (ret < 0)
+			goto end;
+		cal->tx_rswn_impsel[1] = ret;
+	}
+
+	ret = mtk_pcie_sphy_get_one_cal_para(dev, "cal-version", 15);
+	if (ret < 0)
+		goto end;
+	version = ret;
+
+	/* Quirk for eFuse calibration table versions 0 to 4 */
+	if ((version <= MTK_PCIE_SPHY_CALIBRATION_LAST_QUIRK_VER) &&
+	    cal->rx_leq_rl_ctle == 10) {
+		cal->rx_leq_rl_vga = cal->rx_leq_rl_ctle;
+		cal->rx_leq_rl_dfe = cal->rx_leq_rl_ctle;
+	}
+
+end:
+	if (ret < 0) {
+		/*
+		 * If any of the calibration values is missing, or if there is
+		 * no calibration at all in the eFuses, this is not a problem,
+		 * as the PHY doesn't require one to actually work.
+		 */
+		if (ret == -ENOENT) {
+			cal->supported = false;
+			return 0;
+		}
+		return ret;
+	};
+	cal->supported = true;
+
+	return 0;
+}
+
+static int mtk_pcie_sphy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct phy_provider *provider;
+	struct mtk_pcie_sphy *pcie_sphy;
+	u32 num_lanes;
+	int ret;
+
+	pcie_sphy = devm_kzalloc(dev, sizeof(*pcie_sphy), GFP_KERNEL);
+	if (!pcie_sphy)
+		return -ENOMEM;
+
+	pcie_sphy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
+	if (IS_ERR(pcie_sphy->sif_base))
+		return dev_err_probe(dev, PTR_ERR(pcie_sphy->sif_base),
+				     "Failed to map phy-sif base\n");
+
+	pcie_sphy->ckm_base = devm_platform_ioremap_resource_byname(pdev, "ckm");
+	if (IS_ERR(pcie_sphy->ckm_base))
+		return dev_err_probe(dev, PTR_ERR(pcie_sphy->ckm_base),
+				     "Failed to map phy-ckm base\n");
+
+	pcie_sphy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_sphy_ops);
+	if (IS_ERR(pcie_sphy->phy))
+		return dev_err_probe(dev, PTR_ERR(pcie_sphy->phy),
+				     "Failed to create PCIe phy\n");
+
+	ret = of_property_read_u32(dev->of_node, "num-lanes", &num_lanes);
+	if (ret)
+		num_lanes = 1;
+	else if (num_lanes > 4)
+		return dev_err_probe(dev, -EINVAL, "Invalid number of lanes.\n");
+
+	pcie_sphy->num_lanes = num_lanes;
+	pcie_sphy->dev = dev;
+
+	if (pcie_sphy->num_lanes <= MTK_PCIE_SPHY_CALIBRATION_MAX_DATA_LANES) {
+		ret = mtk_pcie_sphy_get_calibration_data(pcie_sphy);
+		if (ret)
+			return ret;
+	}
+
+	phy_set_drvdata(pcie_sphy->phy, pcie_sphy);
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
+	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(provider))
+		return dev_err_probe(dev, PTR_ERR(provider),
+				     "Could not register PCI-Express S-PHY\n");
+
+	return 0;
+}
+
+static const struct of_device_id mtk_pcie_sphy_of_match[] = {
+	{ .compatible = "mediatek,mt8196-pcie-sphy" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mtk_pcie_sphy_of_match);
+
+static struct platform_driver mtk_pcie_sphy_driver = {
+	.probe	= mtk_pcie_sphy_probe,
+	.driver	= {
+		.name = "mtk-pcie-sphy",
+		.of_match_table = mtk_pcie_sphy_of_match,
+	},
+};
+module_platform_driver(mtk_pcie_sphy_driver);
+
+MODULE_DESCRIPTION("MediaTek PCIe SPHY driver");
+MODULE_AUTHOR("AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>");
+MODULE_LICENSE("GPL");
-- 
2.55.0


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

^ permalink raw reply related

* [PATCH v2 0/2] PHY: Add MediaTek PCI-Express Gen4 S-PHY Driver
From: AngeloGioacchino Del Regno @ 2026-07-20 13:53 UTC (permalink / raw)
  To: chunfeng.yun
  Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
	angelogioacchino.delregno, linux-arm-kernel, linux-mediatek,
	linux-phy, devicetree, linux-kernel, kernel

Changes in v2:
 - Added items/description to reg in binding
 - Added missing pm_runtime.h inclusion
 - Moved pm_runtime_enable call to before registering PHY
 - Fixed pmos/nmos variable size for 5 bits calibration values
 - Fixed calibration for single-lane PCIe, as in, the ln1 rswn
   calibration nvmem value is ignored in code if it's single
   lane and will not return an error; this is due to the fact
   that, effectively, single-lane may have a zero calibration
   in tx-ln1-rswn which is fine, as that'd be anyway unused
 - Changed calibration data errors to dev_err_probe and changed
   the "no calibration for ..." message to dev_info instead

This adds a driver for the PCI-Express Gen4 "S-PHY" found in the
Genio MT8894, Kompanio MT8196, Dimensity MT6991 SoCs (which are
all variants of the same chip).

This was successfully tested on MT8894 and MT8196.

AngeloGioacchino Del Regno (2):
  dt-bindings: phy: Document MT8196 MediaTek PCI-Express Gen4 S-PHY
  phy: mediatek: Add support for PCI-Express Gen4 S-PHY

 .../phy/mediatek,mt8196-pcie-sphy.yaml        |  93 +++
 drivers/phy/mediatek/Kconfig                  |   9 +
 drivers/phy/mediatek/Makefile                 |   1 +
 drivers/phy/mediatek/phy-mtk-pcie-sphy.c      | 561 ++++++++++++++++++
 4 files changed, 664 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
 create mode 100644 drivers/phy/mediatek/phy-mtk-pcie-sphy.c

-- 
2.55.0


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

^ permalink raw reply

* [PATCH v2 1/2] dt-bindings: phy: Document MT8196 MediaTek PCI-Express Gen4 S-PHY
From: AngeloGioacchino Del Regno @ 2026-07-20 13:53 UTC (permalink / raw)
  To: chunfeng.yun
  Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
	angelogioacchino.delregno, linux-arm-kernel, linux-mediatek,
	linux-phy, devicetree, linux-kernel, kernel
In-Reply-To: <20260720135344.84183-1-angelogioacchino.delregno@collabora.com>

This adds bindings for the PCI-Express Gen4 S-PHY found in newer
MediaTek SoCs, such as MT8196 and its variants.

In the current "revision 3", depending on the specific port, this
S-PHY supports up to two lanes of PCI-Express Gen 4 and both EP
and RC modes.
It is not clear whether revisions/versions earlier than 3 have
ever been shipped in any other MediaTek SoC.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../phy/mediatek,mt8196-pcie-sphy.yaml        | 93 +++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml

diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
new file mode 100644
index 000000000000..c24871f7e0e5
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/mediatek,mt8196-pcie-sphy.yaml
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/mediatek,mt8196-pcie-sphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek PCI-Express Gen4 S-PHY
+
+maintainers:
+  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
+
+description:
+  The MediaTek PCI-Express Gen4 S-PHY is capable of data serialization
+  and de-serialization with 8, 10, 128 or 130 bits (16GT/s) encoding
+  and decoding, has analog buffers, elastic buffers and mechanisms for
+  receiver detection.
+  This PHY supports link rates of 2.5, 5.0, 6.0 and 16.0 GT/s per-lane
+  in both Root Complex (RC) and End Point (EP) modes, depending on the
+  chosen port.
+  The Digital PHY (PHYD) part adheres to the Intel PIPE (PHY Interface
+  for the PCIe) specification.
+
+properties:
+  compatible:
+    const: mediatek,mt8196-pcie-sphy
+
+  reg:
+    items:
+      - description: System Interface (SIF) registers
+      - description: Clock Management (CKM) registers
+
+  reg-names:
+    items:
+      - const: sif
+      - const: ckm
+
+  "#phy-cells":
+    const: 0
+
+  nvmem-cells:
+    items:
+      - description: Internal resistor selection of TX Bias Current
+      - description: XTP Bias V2V voltage calibration
+      - description: S-PLL CKTX Impedance Selection (P-MOSFET side)
+      - description: S-PLL CKTX Impedance Selection (N-MOSFET side)
+      - description: S-PLL CKTX Intermediate Transition Impedance
+      - description: RX Front-End Return Loss Continuous Time Linear EQ
+      - description: RX Front-End Return Loss Variable Gain Amplifier
+      - description: RX Front-End Return Loss Decision Feedback EQ
+      - description: RX Impedance Selection
+      - description: TX Impedance Selection (P-MOSFET side)
+      - description: TX Impedance Selection (N-MOSFET side)
+      - description: TX Lane 0 Switch Resistance (RSWn)
+      - description: TX Lane 1 Switch Resistance (RSWn)
+      - description: eFuse calibration table version
+
+  nvmem-cell-names:
+    items:
+      - const: int-r
+      - const: xtp-vtrim
+      - const: cktx-pmos
+      - const: cktx-nmos
+      - const: cktx-r-mid
+      - const: rxfe-lanes-rl-ctle
+      - const: rxfe-lanes-rl-vga
+      - const: rxfe-lanes-rl-dfe
+      - const: rx-lanes-imp
+      - const: tx-lanes-pmos
+      - const: tx-lanes-nmos
+      - const: tx-ln0-rswn
+      - const: tx-ln1-rswn
+      - const: cal-version
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    phy@16900000 {
+        compatible = "mediatek,mt8196-pcie-sphy";
+        reg = <0x16900000 0x10000>, <0x16920000 0x10000>;
+        reg-names = "sif", "ckm";
+        power-domains = <&spm 10>;
+        #phy-cells = <0>;
+    };
-- 
2.55.0


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

^ permalink raw reply related

* Re: [PATCH v3 phy-next 4/8] soc: fsl: guts: make it easier to determine on which SoC we are running
From: sashiko-bot @ 2026-07-20 13:53 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: olteanv, linux-phy, devicetree, robh, conor+dt, vkoul,
	neil.armstrong
In-Reply-To: <20260720133642.136324-5-vladimir.oltean@nxp.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Dereferencing a NULL pointer on the `soc_dev_attr` allocation error path in `fsl_guts_init()`.
--

--- Patch [4]: [PATCH v3 phy-next 4/8] soc: fsl: guts: make it easier to determine on which SoC we are running ---
commit 27fff30cda7c763b92c9fd890450416b6a304140
Author: Ioana Ciornei <ioana.ciornei@nxp.com>

soc: fsl: guts: make it easier to determine on which SoC we are running

The guts driver will need to easily determine on which SoC it's running
when it will need to perform RCW override at runtime. The guts driver
knows this already because fsl_guts_init() reads the QorIQ/Layerscape
architectural System Version Register (SVR), but it doesn't save this
for later lookups.

> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> index e1b3a054e681b..922560d987824 100644
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
[ ... ]
> @@ -239,12 +274,12 @@ static int __init fsl_guts_init(void)
>  	if (!soc_dev_attr->family)
>  		goto err_nomem;
>  
> -	soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "svr:0x%08x", svr);
> +	soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "svr:0x%08x", soc.svr);
>  	if (!soc_dev_attr->soc_id)
>  		goto err_nomem;

[Severity: High]
Does jumping to err_nomem when soc_dev_attr allocation fails cause a NULL
pointer dereference?

In an earlier commit in this series ("soc: fsl: guts: add a global structure
to hold state"), the error path in fsl_guts_init() was modified to unmap
soc.dcfg_ccsr, but it seems the cleanup assumes soc_dev_attr is valid:

fsl_guts_init()
	soc_dev_attr = kzalloc_obj(*soc_dev_attr);
	if (!soc_dev_attr)
		goto err_nomem;
    ...
err_nomem:
	ret = -ENOMEM;
err:
	kfree(soc_dev_attr->family);

If the dynamic allocation for soc_dev_attr fails, wouldn't we dereference
the NULL pointer when freeing its members in the err block?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133642.136324-1-vladimir.oltean@nxp.com?part=4

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

^ permalink raw reply

* Re: [PATCH v3 phy-next 3/8] soc: fsl: guts: add a central fsl_guts_read() function
From: sashiko-bot @ 2026-07-20 13:51 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: robh, conor+dt, devicetree, olteanv, linux-phy, neil.armstrong,
	vkoul
In-Reply-To: <20260720133642.136324-4-vladimir.oltean@nxp.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Dereferencing NULL pointer 'soc_dev_attr' on the error path if its memory allocation fails.
--

--- Patch [3]: [PATCH v3 phy-next 3/8] soc: fsl: guts: add a central fsl_guts_read() function ---
commit cc1312c02ab5c2cd3fed5e2907608e44453c7816
Author: Ioana Ciornei <ioana.ciornei@nxp.com>

soc: fsl: guts: add a central fsl_guts_read() function

This commit adds a central fsl_guts_read() function that handles reading
DCFG registers based on the already determined endianness. This avoids
duplicating the if-else endianness checks across the driver.

> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> index a2b4c477b064b..e1b3a054e681b 100644
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
> @@ -111,6 +111,14 @@ static struct fsl_soc_guts {
>  	bool little_endian;
>  } soc;
>  
> +static unsigned int fsl_guts_read(const void __iomem *reg)
> +{
> +	if (soc.little_endian)
> +		return ioread32(reg);
> +
> +	return ioread32be(reg);
> +}
> +
>  static const struct fsl_soc_die_attr *fsl_soc_die_match(
>  	u32 svr, const struct fsl_soc_die_attr *matches)
>  {

[ ... ]

> @@ -209,10 +217,7 @@ static int __init fsl_guts_init(void)
>  	}
>  
>  	soc.little_endian = of_property_read_bool(np, "little-endian");
> -	if (soc.little_endian)
> -		svr = ioread32(&soc.dcfg_ccsr->svr);
> -	else
> -		svr = ioread32be(&soc.dcfg_ccsr->svr);
> +	svr = fsl_guts_read(&soc.dcfg_ccsr->svr);
>  	of_node_put(np);
>  
>  	/* Register soc device */

[Severity: High]
Does this code trigger a NULL pointer dereference on the error path if memory
allocation for soc_dev_attr fails?

In fsl_guts_init(), if the kzalloc_obj() allocation for soc_dev_attr fails
just below this block, the code jumps to err_nomem and falls through to the
err label:

fsl_guts_init() {
    ...
	soc_dev_attr = kzalloc_obj(*soc_dev_attr);
	if (!soc_dev_attr)
		goto err_nomem;
    ...
err_nomem:
	ret = -ENOMEM;
err:
	kfree(soc_dev_attr->family);
	kfree(soc_dev_attr->soc_id);
    ...
}

Since soc_dev_attr is NULL when taking this path, won't calling
kfree(soc_dev_attr->family) dereference the NULL pointer during device
initialization?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133642.136324-1-vladimir.oltean@nxp.com?part=3

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

^ permalink raw reply

* Re: [PATCH v3 phy-next 2/8] soc: fsl: guts: add a global structure to hold state
From: sashiko-bot @ 2026-07-20 13:50 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: devicetree, robh, conor+dt, vkoul, neil.armstrong, olteanv,
	linux-phy
In-Reply-To: <20260720133642.136324-3-vladimir.oltean@nxp.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Dereferencing a NULL pointer during error cleanup if the initial allocation of `soc_dev_attr` fails.
--

--- Patch [2]: [PATCH v3 phy-next 2/8] soc: fsl: guts: add a global structure to hold state ---
commit dce21fc9192bcf2ec7c5c2e9639e43b7a6c53c57
Author: Ioana Ciornei <ioana.ciornei@nxp.com>

soc: fsl: guts: add a global structure to hold state

This commit introduces the fsl_soc_guts global structure to hold the DCFG CCSR
mapping and endianness state. It maps the memory once at initialization so it
can be reused during runtime operations instead of repeatedly mapping and
unmapping the space.

> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> index f87ee47c15035..a2b4c477b064b 100644
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
[ ... ]
> @@ -199,24 +202,23 @@ static int __init fsl_guts_init(void)
[ ... ]
>  	/* Register soc device */
>  	soc_dev_attr = kzalloc_obj(*soc_dev_attr);
>  	if (!soc_dev_attr)
> -		return -ENOMEM;
> +		goto err_nomem;

[Severity: High]
Will jumping to err_nomem cause a NULL pointer dereference?

If the allocation of soc_dev_attr fails, the code now jumps to the error
handling path which dereferences soc_dev_attr to free its members:

> @@ -269,6 +271,8 @@ static int __init fsl_guts_init(void)
>  	kfree(soc_dev_attr->revision);
>  	kfree(soc_dev_attr->serial_number);

Since soc_dev_attr is NULL when this allocation fails, won't evaluating
soc_dev_attr->revision and other members result in a kernel panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133642.136324-1-vladimir.oltean@nxp.com?part=2

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

^ permalink raw reply

* [PATCH v3 phy-next 8/8] phy: lynx-10g: use RCW override procedure for dynamic protocol change
From: Vladimir Oltean @ 2026-07-20 13:36 UTC (permalink / raw)
  To: linux-phy
  Cc: devicetree, linuxppc-dev, linux-arm-kernel, Ioana Ciornei,
	Vinod Koul, Neil Armstrong, Tanjeff Moos,
	Christophe Leroy (CS GROUP), Michael Walle, Shawn Guo, Frank Li,
	linux-kernel
In-Reply-To: <20260720133642.136324-1-vladimir.oltean@nxp.com>

Up until this patch, the only protocol change supported was between
1000Base-X/SGMII and 2500Base-X. The others require an RCW override
procedure which was lacking.

Since now the guts driver provides the means of applying this procedure,
make use of it and remove any comment which mentioned the limitation.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v3: none
---
 drivers/phy/freescale/Kconfig            |  1 +
 drivers/phy/freescale/phy-fsl-lynx-10g.c | 24 +++++++++++++++---------
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
index 5bf3864fbe64..d4e189fffbf8 100644
--- a/drivers/phy/freescale/Kconfig
+++ b/drivers/phy/freescale/Kconfig
@@ -58,6 +58,7 @@ config PHY_FSL_LYNX_10G
 	tristate "Freescale Layerscape Lynx 10G SerDes PHY support"
 	depends on OF
 	depends on ARCH_LAYERSCAPE || COMPILE_TEST
+	select FSL_GUTS
 	select GENERIC_PHY
 	select PHY_FSL_LYNX_CORE
 	help
diff --git a/drivers/phy/freescale/phy-fsl-lynx-10g.c b/drivers/phy/freescale/phy-fsl-lynx-10g.c
index 38def160ef1a..5ece7889aed7 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-10g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-10g.c
@@ -8,6 +8,7 @@
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/workqueue.h>
+#include <linux/fsl/guts.h>
 
 #include "phy-fsl-lynx-core.h"
 
@@ -446,6 +447,7 @@ static void lynx_10g_lane_read_configuration(struct lynx_lane *lane)
 	}
 
 	lynx_10g_backup_pccr_val(lane);
+	fsl_guts_lane_init(priv->info->index, lane->id, lane->mode);
 }
 
 static int ls1028a_get_pccr(enum lynx_lane_mode lane_mode, int lane,
@@ -1167,14 +1169,7 @@ static bool lynx_10g_lane_mode_needs_rcw_override(struct lynx_lane *lane,
 
 	/* Major protocol changes, which involve changing the PCS connection to
 	 * the GMII MAC with the one to the XGMII MAC, require an RCW override
-	 * procedure to reconfigure an internal mux, as documented here:
-	 * https://lore.kernel.org/linux-phy/20230810102631.bvozjer3t67r67iy@skbuf/
-	 * This is SoC-specific, and not yet implemented in drivers/soc/fsl/guts.c.
-	 *
-	 * So the supported set of protocols depends on the initial lane mode.
-	 *
-	 * Minor protocol changes (SGMII <-> 1000Base-X <-> 2500Base-X or
-	 * 10GBase-R <-> USXGMII) are supported.
+	 * procedure to reconfigure an internal mux.
 	 */
 	if ((lynx_lane_mode_uses_gmii_mac(curr) &&
 	     lynx_lane_mode_uses_xgmii_mac(new)) ||
@@ -1189,6 +1184,7 @@ static int lynx_10g_validate(struct phy *phy, enum phy_mode mode, int submode,
 			     union phy_configure_opts *opts)
 {
 	struct lynx_lane *lane = phy_get_drvdata(phy);
+	struct lynx_priv *priv = lane->priv;
 	enum lynx_lane_mode lane_mode;
 	int err;
 
@@ -1197,7 +1193,8 @@ static int lynx_10g_validate(struct phy *phy, enum phy_mode mode, int submode,
 		return err;
 
 	if (lynx_10g_lane_mode_needs_rcw_override(lane, lane_mode))
-		return -EINVAL;
+		return fsl_guts_lane_validate(priv->info->index, lane->id,
+					      lane_mode);
 
 	return 0;
 }
@@ -1205,6 +1202,7 @@ static int lynx_10g_validate(struct phy *phy, enum phy_mode mode, int submode,
 static int lynx_10g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 {
 	struct lynx_lane *lane = phy_get_drvdata(phy);
+	struct lynx_priv *priv = lane->priv;
 	bool powered_up = lane->powered_up;
 	enum lynx_lane_mode lane_mode;
 	int err;
@@ -1225,6 +1223,13 @@ static int lynx_10g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 	if (powered_up)
 		lynx_10g_lane_halt(phy);
 
+	if (lynx_10g_lane_mode_needs_rcw_override(lane, lane_mode)) {
+		err = fsl_guts_lane_set_mode(priv->info->index, lane->id,
+					     lane_mode);
+		if (err)
+			goto out;
+	}
+
 	err = lynx_10g_lane_disable_pcvt(lane, lane->mode);
 	if (err)
 		goto out;
@@ -1314,6 +1319,7 @@ static struct platform_driver lynx_10g_driver = {
 };
 module_platform_driver(lynx_10g_driver);
 
+MODULE_IMPORT_NS("FSL_GUTS");
 MODULE_IMPORT_NS("PHY_FSL_LYNX");
 MODULE_AUTHOR("Ioana Ciornei <ioana.ciornei@nxp.com>");
 MODULE_AUTHOR("Vladimir Oltean <vladimir.oltean@nxp.com>");
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 7/8] soc: fsl: guts: implement the RCW override procedure
From: Vladimir Oltean @ 2026-07-20 13:36 UTC (permalink / raw)
  To: linux-phy
  Cc: devicetree, linuxppc-dev, linux-arm-kernel, Ioana Ciornei,
	Vinod Koul, Neil Armstrong, Tanjeff Moos,
	Christophe Leroy (CS GROUP), Michael Walle, Shawn Guo, Frank Li,
	linux-kernel
In-Reply-To: <20260720133642.136324-1-vladimir.oltean@nxp.com>

From: Ioana Ciornei <ioana.ciornei@nxp.com>

Add support for the RCW override procedure which enables runtime
reconfiguration of the protocol running on a SerDes lane. The procedure
is done through the DCFG DCSR space which now can be defined as the
second memory region of the guts DT node.
Support is added on the following SoCs: LS1046A, LS1088A, LS2088A.

The procedure is exported to the "client" driver - the Lynx10G SerDes
PHY driver - through the following functions:
- fsl_guts_lane_init() used to notify the initial / boot time lane mode
  running on a SerDes lane.
- fsl_guts_lane_validate() used to validate that changing the protocol
  on a specific lane is supported.
- fsl_guts_lane_set_mode() which can be used to request the RCW
  procedure be executed for a specific lane.

Since the RCW override procedure is different depending on the SoC, the
private fsl_soc_data structure is updated with two new per SoC callbacks
(.serdes_get_rcw_override() and .serdes_init_rcwcr()) which get used
from the generic fsl_guts_lane_set_mode() function. These two callbacks
hide all the SoC specific register offsets, masks and values so that the
_set_mode() procedure is straightforward.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v2->v3:
- move iounmap() of dcfg_dcsr area on the 'err' label
- use __ffs() (standard API which also works on armv7) instead of
  __bf_shf() (an internal helper meant for compile-time constants in
  bitfield.h)
- only operate on lanes on which fsl_guts_lane_init() was called in
  ls2088a_serdes_init_rcwcr()
- use read_poll_timeout_atomic() in fsl_guts_rcw_rmw() to clarify that
  DCFG_DCSR writes are supposed to reflect back in DCFG_CCSR
- check soc.data in fsl_guts_serdes_get_rcw_override() to ensure
  fsl_guts_init() succeeded
- put parentheses around (lane) in LS1088A_RCWSR29_SRDS_PRTCL_S1_LNn()
  and LS1088A_RCWSR30_SRDS_PRTCL_S2_LNn() macro expressions

v1->v2:
- drop DT maintainers from explicit CC
- keep devicetree@vger.kernel.org CCed on entire series
- include missing <linux/bitfield.h>
- namespace SRDS_PRTCL values for LS1046A and LS1088A, even if they are
  the same. For LS1028A (not covered here) they are not.
- prefix SRDS_CLK_SEL_{GMII,XGMII} with LS2088A_
- reorder alphanumerically (LS1046A should come before LS1088A)
---
 drivers/soc/fsl/guts.c   | 343 ++++++++++++++++++++++++++++++++++++++-
 include/linux/fsl/guts.h |  20 ++-
 2 files changed, 357 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index b9973ca3a440..4d49d1b95300 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -5,7 +5,10 @@
  * Copyright (C) 2016 Freescale Semiconductor, Inc.
  */
 
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/of_fdt.h>
@@ -15,6 +18,30 @@
 #include <linux/fsl/guts.h>
 
 #define DCFG_CCSR	0
+#define DCFG_DCSR	1
+
+#define MAX_NUM_LANES	8
+#define MAX_NUM_SERDES	2
+
+#define RCW_TIMEOUT_US	1
+
+#define LS1046A_RCWSR5_SRDS_PRTCL_S1(lane)	\
+	GENMASK(19 + 4 * (lane), 16 + 4 * (lane))
+#define LS1046A_SRDS_PRTCL_XFI				1
+#define LS1046A_SRDS_PRTCL_100BASEX_SGMII		3
+
+#define LS1088A_RCWSR29_SRDS_PRTCL_S1_LNn(lane)	\
+	GENMASK(19 + 4 * (3 - (lane)), 16 + 4 * (3 - (lane)))
+#define LS1088A_RCWSR30_SRDS_PRTCL_S2_LNn(lane)	\
+	GENMASK(3 + 4 * (3 - (lane)), 4 * (3 - (lane)))
+#define LS1088A_SRDS_PRTCL_XFI				1
+#define LS1088A_SRDS_PRTCL_100BASEX_SGMII		3
+
+#define LS2088A_RCWSR30_SRDS_CLK_EN_SEL_XGMII_S1	BIT(14)
+#define LS2088A_RCWSR30_SRDS_CLK_SEL_XGMII_Ln_S1(lane)	BIT(6 + (7 - (lane)))
+#define LS2088A_RCWSR30_SRDS_CLK_SEL_MSK		GENMASK(13, 6)
+#define LS2088A_SRDS_CLK_SEL_XGMII			1
+#define LS2088A_SRDS_CLK_SEL_GMII			0
 
 struct fsl_soc_die_attr {
 	char	*die;
@@ -22,9 +49,20 @@ struct fsl_soc_die_attr {
 	u32	mask;
 };
 
+struct fsl_soc_serdes_rcw_override {
+	int offset;
+	int mask;
+	int val;
+};
+
 struct fsl_soc_data {
 	const char *sfp_compat;
 	u32 uid_offset;
+	int num_serdes;
+	int (*serdes_get_rcw_override)(int index, int lane,
+				       enum lynx_lane_mode lane_mode,
+				       struct fsl_soc_serdes_rcw_override *override);
+	int (*serdes_init_rcwcr)(int index);
 };
 
 enum qoriq_die {
@@ -138,9 +176,14 @@ static const struct fsl_soc_die_attr fsl_soc_die[] = {
 
 static struct fsl_soc_guts {
 	struct ccsr_guts __iomem *dcfg_ccsr;
+	struct ccsr_guts __iomem *dcfg_dcsr;
 	const struct fsl_soc_data *data;
 	bool little_endian;
 	u32 svr;
+	enum lynx_lane_mode lane_mode[MAX_NUM_SERDES][MAX_NUM_LANES];
+	unsigned long lanes_initialized[MAX_NUM_SERDES];
+	bool rcwcr_init_done;
+	spinlock_t rcwcr_lock; /* serializes concurrent writes to the RCWCR */
 } soc;
 
 static unsigned int fsl_guts_read(const void __iomem *reg)
@@ -151,6 +194,33 @@ static unsigned int fsl_guts_read(const void __iomem *reg)
 	return ioread32be(reg);
 }
 
+static void fsl_guts_write(void __iomem *reg, u32 val)
+{
+	if (soc.little_endian)
+		iowrite32(val, reg);
+	else
+		iowrite32be(val, reg);
+}
+
+/* Some fields of the Reset Configuration Word (RCW) can be overridden at
+ * runtime by writing to the RCWCRn registers contained within the DCSR space
+ * of the Device Configuration (DCFG) block. The layout of the RCWCRn registers
+ * is identical with the read-only RCWSRn from the CCSR space.
+ */
+static int fsl_guts_rcw_rmw(int offset, u32 val, u32 mask)
+{
+	u32 rcwcr, rcwsr = fsl_guts_read(&soc.dcfg_ccsr->rcwsr[offset]);
+
+	rcwcr = rcwsr & ~mask;
+	rcwcr |= val;
+	fsl_guts_write(&soc.dcfg_dcsr->rcwcr[offset], rcwcr);
+
+	/* Updates to RCWCR should be visible back in RCWSR */
+	return read_poll_timeout_atomic(fsl_guts_read, rcwsr, rcwsr == rcwcr,
+					0, RCW_TIMEOUT_US, false,
+					&soc.dcfg_ccsr->rcwsr[offset]);
+}
+
 static bool fsl_soc_die_match_one(u32 svr, const struct fsl_soc_die_attr *match)
 {
 	return match->svr == (svr & match->mask);
@@ -167,6 +237,131 @@ static const struct fsl_soc_die_attr *fsl_soc_die_match(
 	return NULL;
 }
 
+static int
+fsl_guts_serdes_get_rcw_override(int serdes_idx, int lane,
+				 enum lynx_lane_mode lane_mode,
+				 struct fsl_soc_serdes_rcw_override *override)
+{
+	const struct fsl_soc_data *soc_data = soc.data;
+
+	if (!soc_data)
+		return -ENODEV;
+
+	if (serdes_idx >= soc_data->num_serdes || serdes_idx <= 0)
+		return -ERANGE;
+
+	if (lane >= MAX_NUM_LANES || lane < 0)
+		return -ERANGE;
+
+	if ((!fsl_soc_die_match_one(soc.svr, &fsl_soc_die[DIE_LS1088A]) &&
+	     !fsl_soc_die_match_one(soc.svr, &fsl_soc_die[DIE_LS2088A]) &&
+	     !fsl_soc_die_match_one(soc.svr, &fsl_soc_die[DIE_LS1046A])) ||
+	    !soc_data->serdes_get_rcw_override) {
+		pr_debug("RCW override not implemented for SoC\n");
+		return -EINVAL;
+	}
+
+	if (!soc.dcfg_dcsr) {
+		pr_debug("Device tree does not define DCFG_DCSR region necessary for RCW override\n");
+		return -EINVAL;
+	}
+
+	return soc_data->serdes_get_rcw_override(serdes_idx, lane, lane_mode,
+						 override);
+}
+
+/**
+ * fsl_guts_lane_validate() - Validate that SerDes protocol is implemented and
+ *	supported on current SoC
+ * @serdes_idx: one-based SerDes block index
+ * @lane: zero-based lane index within SerDes
+ * @lane_mode: requested SerDes protocol
+ *
+ * Should be called before actually requesting the RCW override procedure to be
+ * applied using %fsl_guts_lane_set_mode()
+ *
+ * Return: 0 if RCW override to protocol is possible, negative error otherwise
+ */
+int fsl_guts_lane_validate(int serdes_idx, int lane, enum lynx_lane_mode lane_mode)
+{
+	struct fsl_soc_serdes_rcw_override override;
+
+	return fsl_guts_serdes_get_rcw_override(serdes_idx, lane, lane_mode,
+						&override);
+}
+EXPORT_SYMBOL_NS_GPL(fsl_guts_lane_validate, "FSL_GUTS");
+
+/**
+ * fsl_guts_lane_init() - Notify guts module of current SerDes lane configuration
+ * @serdes_idx: one-based SerDes block index
+ * @lane: zero-based lane index within SerDes
+ * @lane_mode: initial / boot time SerDes protocol for lane
+ *
+ * On the LS208xA SoC, the RCW override procedure needs to be aware of all link
+ * modes which are configured on a SerDes block.
+ *
+ * Return: 0 if passed parameters are known to driver, negative error otherwise
+ */
+int fsl_guts_lane_init(int serdes_idx, int lane, enum lynx_lane_mode lane_mode)
+{
+	int err;
+
+	err = fsl_guts_lane_validate(serdes_idx, lane, lane_mode);
+	if (err)
+		return err;
+
+	soc.lane_mode[serdes_idx - 1][lane] = lane_mode;
+	soc.lanes_initialized[serdes_idx - 1] |= BIT(lane);
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(fsl_guts_lane_init, "FSL_GUTS");
+
+/**
+ * fsl_guts_lane_set_mode() - apply RCW override procedure for SerDes lane
+ * @serdes_idx: one-based SerDes block index
+ * @lane: zero-based lane index within SerDes
+ * @lane_mode: requested SerDes protocol
+ *
+ * Return: 0 on success, negative error otherwise
+ */
+int fsl_guts_lane_set_mode(int serdes_idx, int lane, enum lynx_lane_mode lane_mode)
+{
+	struct fsl_soc_serdes_rcw_override override;
+	int err;
+
+	err = fsl_guts_serdes_get_rcw_override(serdes_idx, lane, lane_mode,
+					       &override);
+	if (err)
+		return err;
+
+	/* Ensure fsl_guts_lane_init() was previously called */
+	if (!(soc.lanes_initialized[serdes_idx - 1] & BIT(lane)))
+		return -EINVAL;
+
+	spin_lock(&soc.rcwcr_lock);
+
+	if (soc.data->serdes_init_rcwcr) {
+		err = soc.data->serdes_init_rcwcr(serdes_idx);
+		if (err)
+			goto out_unlock;
+	}
+
+	err = fsl_guts_rcw_rmw(override.offset,
+			       override.val << __ffs(override.mask),
+			       override.mask);
+	if (err)
+		pr_err("RCW override failed: %pe\n", ERR_PTR(err));
+	else
+		soc.lane_mode[serdes_idx - 1][lane] = lane_mode;
+
+out_unlock:
+	spin_unlock(&soc.rcwcr_lock);
+
+	return err;
+}
+EXPORT_SYMBOL_NS_GPL(fsl_guts_lane_set_mode, "FSL_GUTS");
+
 static u64 fsl_guts_get_soc_uid(const char *compat, unsigned int offset)
 {
 	struct device_node *np;
@@ -193,9 +388,143 @@ static u64 fsl_guts_get_soc_uid(const char *compat, unsigned int offset)
 	return uid;
 }
 
+static int ls1046a_serdes_get_rcw_override(int index, int lane,
+					   enum lynx_lane_mode lane_mode,
+					   struct fsl_soc_serdes_rcw_override *override)
+{
+	/* The RCW override procedure has to write to different registers
+	 * depending on the SerDes block index.
+	 */
+	switch (index) {
+	case 1:
+		override->offset = 4;
+		override->mask = LS1046A_RCWSR5_SRDS_PRTCL_S1(lane);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (lynx_lane_mode_uses_xgmii_mac(lane_mode))
+		override->val = LS1046A_SRDS_PRTCL_XFI;
+	else if (lynx_lane_mode_uses_gmii_mac(lane_mode))
+		override->val = LS1046A_SRDS_PRTCL_100BASEX_SGMII;
+	else
+		return -EINVAL;
+
+	return 0;
+}
+
+static int ls1088a_serdes_get_rcw_override(int index, int lane,
+					   enum lynx_lane_mode lane_mode,
+					   struct fsl_soc_serdes_rcw_override *override)
+{
+	/* The RCW override procedure has to write to different registers
+	 * depending on the SerDes block index.
+	 */
+	switch (index) {
+	case 1:
+		override->offset = 28;
+		override->mask = LS1088A_RCWSR29_SRDS_PRTCL_S1_LNn(lane);
+		break;
+	case 2:
+		override->offset = 29;
+		override->mask = LS1088A_RCWSR30_SRDS_PRTCL_S2_LNn(lane);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (lynx_lane_mode_uses_xgmii_mac(lane_mode))
+		override->val = LS1088A_SRDS_PRTCL_XFI;
+	else if (lynx_lane_mode_uses_gmii_mac(lane_mode))
+		override->val = LS1088A_SRDS_PRTCL_100BASEX_SGMII;
+	else
+		return -EINVAL;
+
+	return 0;
+}
+
+static int ls2088a_serdes_get_rcw_override(int index, int lane,
+					   enum lynx_lane_mode lane_mode,
+					   struct fsl_soc_serdes_rcw_override *override)
+{
+	switch (index) {
+	case 1:
+		override->offset = 29;
+		override->mask = LS2088A_RCWSR30_SRDS_CLK_SEL_XGMII_Ln_S1(lane);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (lynx_lane_mode_uses_xgmii_mac(lane_mode))
+		override->val = LS2088A_SRDS_CLK_SEL_XGMII;
+	else if (lynx_lane_mode_uses_gmii_mac(lane_mode))
+		override->val = LS2088A_SRDS_CLK_SEL_GMII;
+	else
+		return -EINVAL;
+
+	return 0;
+}
+
+static int ls2088a_serdes_init_rcwcr(int serdes_idx)
+{
+	int i, err;
+	u32 reg;
+
+	/* SerDes 2 supports only SGMII for networking. There should be
+	 * no need for RCW override
+	 */
+	if (serdes_idx != 1)
+		return -EINVAL;
+
+	if (soc.rcwcr_init_done)
+		return 0;
+
+	/* SRDS_CLK_EN_SEL_XGMII_S1: SerDes Clock Enable Select XGMII Serdes 1:
+	 * Enables to select GMII/XGMII clock according to
+	 * SRDS_CLK_SEL_XGMII_Ln_S1
+	 */
+	reg = LS2088A_RCWSR30_SRDS_CLK_EN_SEL_XGMII_S1;
+
+	/* We need to configure the initial state of all lanes for
+	 * the SerDes block #1
+	 */
+	for_each_set_bit(i, &soc.lanes_initialized[serdes_idx - 1], MAX_NUM_LANES)
+		if (lynx_lane_mode_uses_xgmii_mac(soc.lane_mode[serdes_idx - 1][i]))
+			reg |= LS2088A_RCWSR30_SRDS_CLK_SEL_XGMII_Ln_S1(i);
+
+	err = fsl_guts_rcw_rmw(29, reg,
+			       LS2088A_RCWSR30_SRDS_CLK_EN_SEL_XGMII_S1 |
+			       LS2088A_RCWSR30_SRDS_CLK_SEL_MSK);
+	if (err)
+		return err;
+
+	soc.rcwcr_init_done = true;
+
+	return 0;
+}
+
+static const struct fsl_soc_data ls1088a_data = {
+	.serdes_get_rcw_override = ls1088a_serdes_get_rcw_override,
+	.num_serdes = 2,
+};
+
+static const struct fsl_soc_data ls1046a_data = {
+	.serdes_get_rcw_override = ls1046a_serdes_get_rcw_override,
+	.num_serdes = 2,
+};
+
+static const struct fsl_soc_data ls2088a_data = {
+	.serdes_get_rcw_override = ls2088a_serdes_get_rcw_override,
+	.serdes_init_rcwcr = ls2088a_serdes_init_rcwcr,
+	.num_serdes = 2,
+};
+
 static const struct fsl_soc_data ls1028a_data = {
 	.sfp_compat = "fsl,ls1028a-sfp",
 	.uid_offset = 0x21c,
+	.num_serdes = 1,
 };
 
 /*
@@ -221,10 +550,10 @@ static const struct of_device_id fsl_guts_of_match[] = {
 	{ .compatible = "fsl,mpc8572-guts", },
 	{ .compatible = "fsl,ls1021a-dcfg", },
 	{ .compatible = "fsl,ls1043a-dcfg", },
-	{ .compatible = "fsl,ls2080a-dcfg", },
-	{ .compatible = "fsl,ls1088a-dcfg", },
+	{ .compatible = "fsl,ls2080a-dcfg", .data = &ls2088a_data},
+	{ .compatible = "fsl,ls1088a-dcfg", .data = &ls1088a_data},
 	{ .compatible = "fsl,ls1012a-dcfg", },
-	{ .compatible = "fsl,ls1046a-dcfg", },
+	{ .compatible = "fsl,ls1046a-dcfg", .data = &ls1046a_data},
 	{ .compatible = "fsl,lx2160a-dcfg", },
 	{ .compatible = "fsl,ls1028a-dcfg", .data = &ls1028a_data},
 	{}
@@ -250,6 +579,8 @@ static int __init fsl_guts_init(void)
 		of_node_put(np);
 		goto err_nomem;
 	}
+	/* DCFG_DCSR is optional */
+	soc.dcfg_dcsr = of_iomap(np, DCFG_DCSR);
 
 	soc.little_endian = of_property_read_bool(np, "little-endian");
 	soc.svr = fsl_guts_read(&soc.dcfg_ccsr->svr);
@@ -296,6 +627,8 @@ static int __init fsl_guts_init(void)
 		goto err;
 	}
 
+	spin_lock_init(&soc.rcwcr_lock);
+
 	pr_info("Machine: %s\n", soc_dev_attr->machine);
 	pr_info("SoC family: %s\n", soc_dev_attr->family);
 	pr_info("SoC ID: %s, Revision: %s\n",
@@ -311,6 +644,10 @@ static int __init fsl_guts_init(void)
 	kfree(soc_dev_attr->revision);
 	kfree(soc_dev_attr->serial_number);
 	kfree(soc_dev_attr);
+	if (soc.dcfg_dcsr) {
+		iounmap(soc.dcfg_dcsr);
+		soc.dcfg_dcsr = NULL;
+	}
 	if (soc.dcfg_ccsr) {
 		iounmap(soc.dcfg_ccsr);
 		soc.dcfg_ccsr = NULL;
diff --git a/include/linux/fsl/guts.h b/include/linux/fsl/guts.h
index fdb55ca47a4f..605300908a35 100644
--- a/include/linux/fsl/guts.h
+++ b/include/linux/fsl/guts.h
@@ -13,6 +13,7 @@
 
 #include <linux/types.h>
 #include <linux/io.h>
+#include <soc/fsl/phy-fsl-lynx.h>
 
 /*
  * Global Utility Registers.
@@ -91,9 +92,15 @@ struct ccsr_guts {
 	u32	iovselsr;	/* 0x.00c0 - I/O voltage select status register
 					     Called 'elbcvselcr' on 86xx SOCs */
 	u8	res0c4[0x100 - 0xc4];
-	u32	rcwsr[16];	/* 0x.0100 - Reset Control Word Status registers
-					     There are 16 registers */
-	u8	res140[0x224 - 0x140];
+	/* 0x.0100 - read-only Reset Configuration Word Status registers in
+	 * CCSR, or write-only Reset Configuration Word Control registers in
+	 * DCSR. In both cases there are 32 registers.
+	 */
+	union {
+		u32	rcwsr[32];
+		u32	rcwcr[32];
+	};
+	u8	res180[0x224 - 0x180];
 	u32	iodelay1;	/* 0x.0224 - IO delay control register 1 */
 	u32	iodelay2;	/* 0x.0228 - IO delay control register 2 */
 	u8	res22c[0x604 - 0x22c];
@@ -131,6 +138,13 @@ struct ccsr_guts {
 	u32	srds2cr1;	/* 0x.0f44 - SerDes2 Control Register 0 */
 } __attribute__ ((packed));
 
+int fsl_guts_lane_init(int serdes_idx, int lane,
+		       enum lynx_lane_mode lane_mode);
+int fsl_guts_lane_validate(int serdes_idx, int lane,
+			   enum lynx_lane_mode lane_mode);
+int fsl_guts_lane_set_mode(int serdes_idx, int lane,
+			   enum lynx_lane_mode lane_mode);
+
 /* Alternate function signal multiplex control */
 #define MPC85xx_PMUXCR_QE(x) (0x8000 >> (x))
 
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 6/8] dt-bindings: fsl: layerscape-dcfg: define DCFG_DCSR region
From: Vladimir Oltean @ 2026-07-20 13:36 UTC (permalink / raw)
  To: linux-phy
  Cc: devicetree, linuxppc-dev, linux-arm-kernel, Ioana Ciornei,
	Vinod Koul, Neil Armstrong, Tanjeff Moos,
	Christophe Leroy (CS GROUP), Michael Walle, Shawn Guo, Frank Li,
	linux-kernel, Conor Dooley, Conor Dooley, Krzysztof Kozlowski,
	Rob Herring
In-Reply-To: <20260720133642.136324-1-vladimir.oltean@nxp.com>

In Layerscape (Arm) and QorIQ (PowerPC) devices, hardware peripherals
are accessed by the CPU through a portion of the SoC address space
called CCSR ("Configuration, Control, and Status Registers"). All
hardware IP blocks have their registers mapped here, and the Device
Configuration block makes no exception.

However, there exists a secondary range of the address space named DCSR
("Debug Control and Status Registers") which, like CCSR, also holds
registers of hardware IP blocks, except the DCSR contents is hidden in
all public reference manuals.

The intention of the CCSR/DCSR split, to the best of my knowledge, was
to place the functionality that is too low level for normal use, and
which is necessary only for debug, in a completely separate address
space which can be hidden.

A use case has appeared where networking SerDes lanes need to be
reconfigured at runtime for a different protocol (example: 10GBase-R to
SGMII), and the architecture of the SoCs does not normally permit that.
The Reset Configuration Word (RCW) is a data structure read by the SoC
preboot loader (PBL) which contains stuff like pinmuxing and SerDes
protocol mapping for each lane.

The RCW that the PBL has loaded is visible in the DCFG block's normal
status registers (from CCSR), as read only. Turns out, the RCW is also
mapped in the DCFG's shadow register map (in DCSR), in a write-only
form. Writing to the RCW registers from the DCFG's DCSR space to change
what the PBL has loaded is called "RCW override".

It has been validated that the RCW override procedure is necessary to
reconfigure the networking data path when a SerDes lane performs a major
protocol change. It changes some internal muxes which connect the PCS to
either the 10G MAC or to the 1G MAC.

Defining the DCSR area of the DCFG as a secondary 'reg' array element
allows operating systems to perform RCW overrides. Since it is
introduced late in the binding's lifetime, it is optional. It can be
identified by name, but also by index (first 'reg' is CCSR).

Note that while all SoCs should have a DCFG register block in DCSR, we
only need to expose it for the SoCs where the RCW override procedure is
known to be needed and has been validated.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Rob Herring <robh@kernel.org>

v2->v3: none
v1->v2:
- add Conor's review tag
- update email addresses of DT maintainers
---
 .../bindings/soc/fsl/fsl,layerscape-dcfg.yaml     | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml b/Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
index 3fb0534ea597..fc14fd0bf84b 100644
--- a/Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
+++ b/Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
@@ -36,7 +36,20 @@ properties:
           - const: simple-mfd
 
   reg:
-    maxItems: 1
+    minItems: 1
+    items:
+      - description:
+          Customer-visible DCFG register map from CCSR address space
+          (Configuration, Control and Status Registers)
+      - description:
+          Customer-hidden DCFG register map from DCSR address space
+          (Debug Control and Status Registers)
+
+  reg-names:
+    minItems: 1
+    items:
+      - const: dcfg_ccsr
+      - const: dcfg_dcsr
 
   little-endian: true
   big-endian: true
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 5/8] soc: fsl: guts: make fsl_soc_data available after fsl_guts_init()
From: Vladimir Oltean @ 2026-07-20 13:36 UTC (permalink / raw)
  To: linux-phy
  Cc: devicetree, linuxppc-dev, linux-arm-kernel, Ioana Ciornei,
	Vinod Koul, Neil Armstrong, Tanjeff Moos,
	Christophe Leroy (CS GROUP), Michael Walle, Shawn Guo, Frank Li,
	linux-kernel
In-Reply-To: <20260720133642.136324-1-vladimir.oltean@nxp.com>

In a future change, struct fsl_soc_data will be extended with methods
for performing RCW override.

Since this will be performed from a calling context outside
fsl_guts_init(), we need to keep track of the soc_data that we determine
at fsl_guts_init() time, so we can reference it later.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v2->v3: don't leave soc.data a valid pointer if fsl_guts_init() fails
v1->v2: none
---
 drivers/soc/fsl/guts.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 922560d98782..b9973ca3a440 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -138,6 +138,7 @@ static const struct fsl_soc_die_attr fsl_soc_die[] = {
 
 static struct fsl_soc_guts {
 	struct ccsr_guts __iomem *dcfg_ccsr;
+	const struct fsl_soc_data *data;
 	bool little_endian;
 	u32 svr;
 } soc;
@@ -231,10 +232,9 @@ static const struct of_device_id fsl_guts_of_match[] = {
 
 static int __init fsl_guts_init(void)
 {
-	struct soc_device_attribute *soc_dev_attr;
+	struct soc_device_attribute *soc_dev_attr = NULL;
 	static struct soc_device *soc_dev;
 	const struct fsl_soc_die_attr *soc_die;
-	const struct fsl_soc_data *soc_data;
 	const struct of_device_id *match;
 	struct device_node *np;
 	u64 soc_uid = 0;
@@ -243,12 +243,12 @@ static int __init fsl_guts_init(void)
 	np = of_find_matching_node_and_match(NULL, fsl_guts_of_match, &match);
 	if (!np)
 		return 0;
-	soc_data = match->data;
+	soc.data = match->data;
 
 	soc.dcfg_ccsr = of_iomap(np, DCFG_CCSR);
 	if (!soc.dcfg_ccsr) {
 		of_node_put(np);
-		return -ENOMEM;
+		goto err_nomem;
 	}
 
 	soc.little_endian = of_property_read_bool(np, "little-endian");
@@ -283,9 +283,9 @@ static int __init fsl_guts_init(void)
 	if (!soc_dev_attr->revision)
 		goto err_nomem;
 
-	if (soc_data)
-		soc_uid = fsl_guts_get_soc_uid(soc_data->sfp_compat,
-					       soc_data->uid_offset);
+	if (soc.data)
+		soc_uid = fsl_guts_get_soc_uid(soc.data->sfp_compat,
+					       soc.data->uid_offset);
 	if (soc_uid)
 		soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX",
 							soc_uid);
@@ -311,8 +311,11 @@ static int __init fsl_guts_init(void)
 	kfree(soc_dev_attr->revision);
 	kfree(soc_dev_attr->serial_number);
 	kfree(soc_dev_attr);
-	iounmap(soc.dcfg_ccsr);
-	soc.dcfg_ccsr = NULL;
+	if (soc.dcfg_ccsr) {
+		iounmap(soc.dcfg_ccsr);
+		soc.dcfg_ccsr = NULL;
+	}
+	soc.data = NULL;
 
 	return ret;
 }
-- 
2.34.1


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

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox