Re: [PATCH] phy: rockchip-snps-pcie3: add support for rockchip,phy-ref-use-pad

[Damien Le Moal <dlemoal@kernel.org>]

On 7/15/25 19:58, Rick Wertenbroek wrote:
>>From the RK3588 Technical Reference Manual, Part1,
> section 6.19 PCIe3PHY_GRF Register Description: "ref_use_pad"
> 
> "Select reference clock connected to ref_pad_clk_p/ref_pad_clk_m.
> Selects the external ref_pad_clk_p and ref_pad_clk_m inputs as the
> reference clock source when asserted. When de-asserted, ref_alt_clk_p
> and ref_alt_clk_m are the sources of the reference clock."
> 
> The hardware reset value for this field is 0x1 (enabled).
> Note that this register field is only available on RK3588, not on RK3568.
> 
> Add support for the device tree property rockchip,phy-ref-use-pad,
> such that the PCIe PHY can be used on boards where there is no PCIe
> reference clock generated or connected to the external pad, by setting
> this property to 0 so that the internal clock is used.
> 
> DT bindings for internal clocks are CLK_PHY0_REF_ALT_P/M and
> CLK_PHY1_REF_ALT_P/M and clock rate should be set to 100MHz in
> the RK3588 cru clock controller (PLL_PPLL).
> 
> Example DT overlay where PHY0 uses internal clock (the first clock of
> the cru (PLL_PPLL) must be set to 100MHz, other values are copied from
> rk3588-base.dtsi) and PHY1 uses the external pad (the default):
> 
> ---
> &cru {
>         assigned-clock-rates =
>                 <100000000>, <786432000>,
>                 <850000000>, <1188000000>,
>                 <702000000>,
>                 <400000000>, <500000000>,
>                 <800000000>, <100000000>,
>                 <400000000>, <100000000>,
>                 <200000000>, <500000000>,
>                 <375000000>, <150000000>,
>                 <200000000>;
> };
> 
> &pcie30phy {
>         rockchip,rx-common-refclk-mode = <0 0 1 1>;
>         rockchip,phy-ref-use-pad = <0 1>;
>         clocks = <&cru PCLK_PCIE_COMBO_PIPE_PHY>, <&cru CLK_PHY0_REF_ALT_P>,
>                          <&cru CLK_PHY0_REF_ALT_M>, <&cru CLK_PHY1_REF_ALT_P>,
>                          <&cru CLK_PHY1_REF_ALT_M>;
>         clock-names = "pclk", "phy0_ref_alt_p",
>                               "phy0_ref_alt_m", "phy1_ref_alt_p",
>                               "phy1_ref_alt_m";
> };
> ---
> 

This looks OK to me, but don't you need to also update the dt-bindings yaml to
document this new "phy-ref-use-pad" property ?


> Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
> ---
>  .../phy/rockchip/phy-rockchip-snps-pcie3.c    | 32 +++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> index 4e8ffd173096..0859c7960167 100644
> --- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> +++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> @@ -33,6 +33,8 @@
>  /* Register for RK3588 */
>  #define PHP_GRF_PCIESEL_CON			0x100
>  #define RK3588_PCIE3PHY_GRF_CMN_CON0		0x0
> +#define RK3588_PCIE3PHY_GRF_PHY0_CONTROL6	0x118
> +#define RK3588_PCIE3PHY_GRF_PHY1_CONTROL6	0x218
>  #define RK3588_PCIE3PHY_GRF_PHY0_STATUS1	0x904
>  #define RK3588_PCIE3PHY_GRF_PHY1_STATUS1	0xa04
>  #define RK3588_PCIE3PHY_GRF_PHY0_LN0_CON1	0x1004
> @@ -44,6 +46,8 @@
>  #define RK3588_BIFURCATION_LANE_0_1		BIT(0)
>  #define RK3588_BIFURCATION_LANE_2_3		BIT(1)
>  #define RK3588_LANE_AGGREGATION		BIT(2)
> +#define RK3588_PHY_REF_USE_PAD_EN		((BIT(2) << 16 | BIT(2)))
> +#define RK3588_PHY_REF_USE_PAD_DIS		((BIT(2) << 16))
>  #define RK3588_RX_CMN_REFCLK_MODE_EN		((BIT(7) << 16) |  BIT(7))
>  #define RK3588_RX_CMN_REFCLK_MODE_DIS		(BIT(7) << 16)
>  #define RK3588_PCIE1LN_SEL_EN			(GENMASK(1, 0) << 16)
> @@ -67,6 +71,7 @@ struct rockchip_p3phy_priv {
>  	int num_lanes;
>  	u32 lanes[4];
>  	u32 rx_cmn_refclk_mode[4];
> +	u32 phy_ref_use_pad[2];
>  };
>  
>  struct rockchip_p3phy_ops {
> @@ -157,6 +162,14 @@ static int rockchip_p3phy_rk3588_init(struct rockchip_p3phy_priv *priv)
>  		     priv->rx_cmn_refclk_mode[3] ? RK3588_RX_CMN_REFCLK_MODE_EN :
>  		     RK3588_RX_CMN_REFCLK_MODE_DIS);
>  
> +	/* Select PHY reference clock, external pad or internal clock */
> +	regmap_write(priv->phy_grf, RK3588_PCIE3PHY_GRF_PHY0_CONTROL6,
> +		     priv->phy_ref_use_pad[0] ? RK3588_PHY_REF_USE_PAD_EN :
> +		     RK3588_PHY_REF_USE_PAD_DIS);
> +	regmap_write(priv->phy_grf, RK3588_PCIE3PHY_GRF_PHY1_CONTROL6,
> +		     priv->phy_ref_use_pad[1] ? RK3588_PHY_REF_USE_PAD_EN :
> +		     RK3588_PHY_REF_USE_PAD_DIS);
> +
>  	/* Deassert PCIe PMA output clamp mode */
>  	regmap_write(priv->phy_grf, RK3588_PCIE3PHY_GRF_CMN_CON0, BIT(8) | BIT(24));
>  
> @@ -312,6 +325,25 @@ static int rockchip_p3phy_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	ret = of_property_read_variable_u32_array(dev->of_node,
> +						  "rockchip,phy-ref-use-pad",
> +						  priv->phy_ref_use_pad, 1,
> +						  ARRAY_SIZE(priv->phy_ref_use_pad));
> +
> +	/*
> +	 * if no rockhip,phy-use-internal-clk, assume PHY uses pad for the
> +	 * reference clock in order to be DT backwards compatible. (Since HW
> +	 * reset val is enabled.)
> +	 */
> +	if (ret == -EINVAL) {
> +		for (int i = 0; i < ARRAY_SIZE(priv->phy_ref_use_pad); i++)
> +			priv->phy_ref_use_pad[i] = 1;
> +	} else if (ret < 0) {
> +		dev_err(dev, "failed to read rockchip,phy-ref-use-pad property %d\n",
> +			ret);
> +		return ret;
> +	}
> +
>  	priv->phy = devm_phy_create(dev, NULL, &rockchip_p3phy_ops);
>  	if (IS_ERR(priv->phy)) {
>  		dev_err(dev, "failed to create combphy\n");


-- 
Damien Le Moal
Western Digital Research