Linux-Rockchip Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Vinod Koul <vkoul@kernel.org>
To: Shawn Lin <shawn.lin@rock-chips.com>
Cc: linux-rockchip@lists.infradead.org,
	linux-phy@lists.infradead.org, Heiko Stuebner <heiko@sntech.de>,
	Neil Armstrong <neil.armstrong@linaro.org>
Subject: Re: [PATCH v3] phy: rockchip: naneng-combphy: Consolidate SSC configuration
Date: Sun, 10 May 2026 16:31:39 +0530	[thread overview]
Message-ID: <agBlk6-vV3o2mPkf@vaman> (raw)
In-Reply-To: <1777251433-110466-1-git-send-email-shawn.lin@rock-chips.com>

On 27-04-26, 08:57, Shawn Lin wrote:
> The PCIe SSC configuration for the RK3588 and RK3576 SoCs required
> additional tuning which is missing. When adding these same SSC
> configurations for both of these two SoCs, as well as upcoming
> platforms, it's obvious the SSC setup code was largely duplicated
> across the platform-specific configuration functions. This becomes
> harder to maintain as more platforms are added.
> 
> So extract the common SSC logic into a shared helper function,
> rk_combphy_common_cfg_ssc(). This cleans up the per-platform drivers
> and centralizes the standard configuration as possible.

Please check

https://sashiko.dev/#/patchset/1777251433-110466-1-git-send-email-shawn.lin%40rock-chips.com

> 
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
>  .../rockchip/phy-rockchip-naneng-combphy.c    | 173 ++++++++----------
>  1 file changed, 73 insertions(+), 100 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> index b60d6bf3f33c..2b0f152f5470 100644
> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> @@ -121,6 +121,7 @@
>  #define RK3568_PHYREG32_SSC_OFFSET_500PPM	1
>  
>  #define RK3568_PHYREG33				0x80
> +#define RK3568_PHYREG33_PLL_SSC_CTRL		BIT(5)
>  #define RK3568_PHYREG33_PLL_KVCO_MASK		GENMASK(4, 2)
>  #define RK3568_PHYREG33_PLL_KVCO_SHIFT		2
>  #define RK3568_PHYREG33_PLL_KVCO_VALUE		2
> @@ -446,6 +447,74 @@ static int rockchip_combphy_probe(struct platform_device *pdev)
>  	return PTR_ERR_OR_ZERO(phy_provider);
>  }
>  
> +static void rk_combphy_common_cfg_ssc(struct rockchip_combphy_priv *priv, unsigned long rate)
> +{
> +	struct device_node *np = priv->dev->of_node;
> +	u32 val;
> +
> +	if (!priv->enable_ssc)
> +		return;
> +
> +	/* Set SSC downward spread spectrum for PCIe and USB3 */
> +	if (priv->type == PHY_TYPE_PCIE || priv->type == PHY_TYPE_USB3) {
> +		val = FIELD_PREP(RK3568_PHYREG32_SSC_MASK, RK3568_PHYREG32_SSC_DOWNWARD);
> +		rockchip_combphy_updatel(priv, RK3568_PHYREG32_SSC_MASK, val, RK3568_PHYREG32);
> +	}
> +
> +	/* Set SSC downward spread spectrum +500ppm for SATA in 100MHz */
> +	if (priv->type == PHY_TYPE_SATA && rate == REF_CLOCK_100MHz) {
> +		val = FIELD_PREP(RK3568_PHYREG32_SSC_DIR_MASK,
> +				 RK3568_PHYREG32_SSC_DOWNWARD);
> +		val |= FIELD_PREP(RK3568_PHYREG32_SSC_OFFSET_MASK,
> +				  RK3568_PHYREG32_SSC_OFFSET_500PPM);
> +		rockchip_combphy_updatel(priv, RK3568_PHYREG32_SSC_MASK, val,
> +					 RK3568_PHYREG32);
> +	}
> +
> +	/* Enable SSC */
> +	val = readl(priv->mmio + RK3568_PHYREG8);
> +	val |= RK3568_PHYREG8_SSC_EN;
> +	writel(val, priv->mmio + RK3568_PHYREG8);
> +
> +	/* Some SoCs need tuning PCIe SSC instead of default configuration in 24MHz */
> +	if (!of_device_is_compatible(np, "rockchip,rk3588-naneng-combphy") &&
> +	    !of_device_is_compatible(np, "rockchip,rk3576-naneng-combphy"))
> +		return;
> +
> +	/* PLL control SSC module period should be set if need tuning */
> +	val = readl(priv->mmio + RK3568_PHYREG33);
> +	val |= RK3568_PHYREG33_PLL_SSC_CTRL;
> +	writel(val, priv->mmio + RK3568_PHYREG33);
> +
> +	if (priv->type == PHY_TYPE_PCIE && rate == REF_CLOCK_24MHz) {
> +		/* Set PLL loop divider */
> +		writel(0x00, priv->mmio + RK3576_PHYREG17);
> +		writel(RK3568_PHYREG18_PLL_LOOP, priv->mmio + RK3568_PHYREG18);
> +
> +		/* Set up rx_pck invert and rx msb to disable */
> +		writel(0x00, priv->mmio + RK3588_PHYREG27);
> +
> +		/*
> +		 * Set up SU adjust signal:
> +		 * su_trim[7:0],   PLL KVCO adjust bits[2:0] to min
> +		 * su_trim[15:8],  PLL LPF R1 adujst bits[9:7]=3'b101
> +		 * su_trim[23:16], CKRCV adjust
> +		 * su_trim[31:24], CKDRV adjust
> +		 */
> +		writel(0x90, priv->mmio + RK3568_PHYREG11);
> +		writel(0x02, priv->mmio + RK3568_PHYREG12);
> +		writel(0x08, priv->mmio + RK3568_PHYREG13);
> +		writel(0x57, priv->mmio + RK3568_PHYREG14);
> +		writel(0x40, priv->mmio + RK3568_PHYREG15);
> +
> +		writel(RK3568_PHYREG16_SSC_CNT_VALUE, priv->mmio + RK3568_PHYREG16);
> +
> +		val = FIELD_PREP(RK3568_PHYREG33_PLL_KVCO_MASK,
> +				 RK3576_PHYREG33_PLL_KVCO_VALUE);
> +		writel(val, priv->mmio + RK3568_PHYREG33);
> +	}
> +}
> +
>  static int rk3528_combphy_cfg(struct rockchip_combphy_priv *priv)
>  {
>  	const struct rockchip_combphy_grfcfg *cfg = priv->cfg->grfcfg;
> @@ -600,21 +669,12 @@ static int rk3562_combphy_cfg(struct rockchip_combphy_priv *priv)
>  
>  	switch (priv->type) {
>  	case PHY_TYPE_PCIE:
> -		/* Set SSC downward spread spectrum */
> -		val = RK3568_PHYREG32_SSC_DOWNWARD << RK3568_PHYREG32_SSC_DIR_SHIFT;
> -		rockchip_combphy_updatel(priv, RK3568_PHYREG32_SSC_MASK, val, RK3568_PHYREG32);
> -
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con0_for_pcie, true);
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con1_for_pcie, true);
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con2_for_pcie, true);
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con3_for_pcie, true);
>  		break;
>  	case PHY_TYPE_USB3:
> -		/* Set SSC downward spread spectrum */
> -		val = RK3568_PHYREG32_SSC_DOWNWARD << RK3568_PHYREG32_SSC_DIR_SHIFT;
> -		rockchip_combphy_updatel(priv, RK3568_PHYREG32_SSC_MASK, val,
> -					 RK3568_PHYREG32);
> -
>  		/* Enable adaptive CTLE for USB3.0 Rx */
>  		rockchip_combphy_updatel(priv, RK3568_PHYREG15_CTLE_EN,
>  					 RK3568_PHYREG15_CTLE_EN, RK3568_PHYREG15);
> @@ -706,11 +766,7 @@ static int rk3562_combphy_cfg(struct rockchip_combphy_priv *priv)
>  		}
>  	}
>  
> -	if (priv->enable_ssc) {
> -		val = readl(priv->mmio + RK3568_PHYREG8);
> -		val |= RK3568_PHYREG8_SSC_EN;
> -		writel(val, priv->mmio + RK3568_PHYREG8);
> -	}
> +	rk_combphy_common_cfg_ssc(priv, rate);
>  
>  	return 0;
>  }
> @@ -755,11 +811,6 @@ static int rk3568_combphy_cfg(struct rockchip_combphy_priv *priv)
>  
>  	switch (priv->type) {
>  	case PHY_TYPE_PCIE:
> -		/* Set SSC downward spread spectrum. */
> -		val = RK3568_PHYREG32_SSC_DOWNWARD << RK3568_PHYREG32_SSC_DIR_SHIFT;
> -
> -		rockchip_combphy_updatel(priv, RK3568_PHYREG32_SSC_MASK, val, RK3568_PHYREG32);
> -
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con0_for_pcie, true);
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con1_for_pcie, true);
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con2_for_pcie, true);
> @@ -767,10 +818,6 @@ static int rk3568_combphy_cfg(struct rockchip_combphy_priv *priv)
>  		break;
>  
>  	case PHY_TYPE_USB3:
> -		/* Set SSC downward spread spectrum. */
> -		val = RK3568_PHYREG32_SSC_DOWNWARD << RK3568_PHYREG32_SSC_DIR_SHIFT,
> -		rockchip_combphy_updatel(priv, RK3568_PHYREG32_SSC_MASK, val, RK3568_PHYREG32);
> -
>  		/* Enable adaptive CTLE for USB3.0 Rx. */
>  		val = readl(priv->mmio + RK3568_PHYREG15);
>  		val |= RK3568_PHYREG15_CTLE_EN;
> @@ -880,13 +927,6 @@ static int rk3568_combphy_cfg(struct rockchip_combphy_priv *priv)
>  
>  			writel(RK3568_PHYREG18_PLL_LOOP, priv->mmio + RK3568_PHYREG18);
>  			writel(RK3568_PHYREG11_SU_TRIM_0_7, priv->mmio + RK3568_PHYREG11);
> -		} else if (priv->type == PHY_TYPE_SATA) {
> -			/* downward spread spectrum +500ppm */
> -			val = RK3568_PHYREG32_SSC_DOWNWARD << RK3568_PHYREG32_SSC_DIR_SHIFT;
> -			val |= RK3568_PHYREG32_SSC_OFFSET_500PPM <<
> -			       RK3568_PHYREG32_SSC_OFFSET_SHIFT;
> -			rockchip_combphy_updatel(priv, RK3568_PHYREG32_SSC_MASK, val,
> -						 RK3568_PHYREG32);
>  		}
>  		break;
>  
> @@ -909,11 +949,7 @@ static int rk3568_combphy_cfg(struct rockchip_combphy_priv *priv)
>  		}
>  	}
>  
> -	if (priv->enable_ssc) {
> -		val = readl(priv->mmio + RK3568_PHYREG8);
> -		val |= RK3568_PHYREG8_SSC_EN;
> -		writel(val, priv->mmio + RK3568_PHYREG8);
> -	}
> +	rk_combphy_common_cfg_ssc(priv, rate);
>  
>  	return 0;
>  }
> @@ -972,10 +1008,6 @@ static int rk3576_combphy_cfg(struct rockchip_combphy_priv *priv)
>  
>  	switch (priv->type) {
>  	case PHY_TYPE_PCIE:
> -		/* Set SSC downward spread spectrum */
> -		val = FIELD_PREP(RK3568_PHYREG32_SSC_MASK, RK3568_PHYREG32_SSC_DOWNWARD);
> -		rockchip_combphy_updatel(priv, RK3568_PHYREG32_SSC_MASK, val, RK3568_PHYREG32);
> -
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con0_for_pcie, true);
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con1_for_pcie, true);
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con2_for_pcie, true);
> @@ -983,10 +1015,6 @@ static int rk3576_combphy_cfg(struct rockchip_combphy_priv *priv)
>  		break;
>  
>  	case PHY_TYPE_USB3:
> -		/* Set SSC downward spread spectrum */
> -		val = FIELD_PREP(RK3568_PHYREG32_SSC_MASK, RK3568_PHYREG32_SSC_DOWNWARD);
> -		rockchip_combphy_updatel(priv, RK3568_PHYREG32_SSC_MASK, val, RK3568_PHYREG32);
> -
>  		/* Enable adaptive CTLE for USB3.0 Rx */
>  		val = readl(priv->mmio + RK3568_PHYREG15);
>  		val |= RK3568_PHYREG15_CTLE_EN;
> @@ -1110,14 +1138,6 @@ static int rk3576_combphy_cfg(struct rockchip_combphy_priv *priv)
>  			writel(0x88, priv->mmio + RK3568_PHYREG13);
>  			writel(0x56, priv->mmio + RK3568_PHYREG14);
>  		} else if (priv->type == PHY_TYPE_SATA) {
> -			/* downward spread spectrum +500ppm */
> -			val = FIELD_PREP(RK3568_PHYREG32_SSC_DIR_MASK,
> -					 RK3568_PHYREG32_SSC_DOWNWARD);
> -			val |= FIELD_PREP(RK3568_PHYREG32_SSC_OFFSET_MASK,
> -					  RK3568_PHYREG32_SSC_OFFSET_500PPM);
> -			rockchip_combphy_updatel(priv, RK3568_PHYREG32_SSC_MASK, val,
> -						 RK3568_PHYREG32);
> -
>  			/* ssc ppm adjust to 3500ppm */
>  			rockchip_combphy_updatel(priv, RK3576_PHYREG10_SSC_PCM_MASK,
>  						 RK3576_PHYREG10_SSC_PCM_3500PPM,
> @@ -1156,39 +1176,7 @@ static int rk3576_combphy_cfg(struct rockchip_combphy_priv *priv)
>  		}
>  	}
>  
> -	if (priv->enable_ssc) {
> -		val = readl(priv->mmio + RK3568_PHYREG8);
> -		val |= RK3568_PHYREG8_SSC_EN;
> -		writel(val, priv->mmio + RK3568_PHYREG8);
> -
> -		if (priv->type == PHY_TYPE_PCIE && rate == REF_CLOCK_24MHz) {
> -			/* Set PLL loop divider */
> -			writel(0x00, priv->mmio + RK3576_PHYREG17);
> -			writel(RK3568_PHYREG18_PLL_LOOP, priv->mmio + RK3568_PHYREG18);
> -
> -			/* Set up rx_pck invert and rx msb to disable */
> -			writel(0x00, priv->mmio + RK3588_PHYREG27);
> -
> -			/*
> -			 * Set up SU adjust signal:
> -			 * su_trim[7:0],   PLL KVCO adjust bits[2:0] to min
> -			 * su_trim[15:8],  PLL LPF R1 adujst bits[9:7]=3'b101
> -			 * su_trim[23:16], CKRCV adjust
> -			 * su_trim[31:24], CKDRV adjust
> -			 */
> -			writel(0x90, priv->mmio + RK3568_PHYREG11);
> -			writel(0x02, priv->mmio + RK3568_PHYREG12);
> -			writel(0x08, priv->mmio + RK3568_PHYREG13);
> -			writel(0x57, priv->mmio + RK3568_PHYREG14);
> -			writel(0x40, priv->mmio + RK3568_PHYREG15);
> -
> -			writel(RK3568_PHYREG16_SSC_CNT_VALUE, priv->mmio + RK3568_PHYREG16);
> -
> -			val = FIELD_PREP(RK3568_PHYREG33_PLL_KVCO_MASK,
> -					 RK3576_PHYREG33_PLL_KVCO_VALUE);
> -			writel(val, priv->mmio + RK3568_PHYREG33);
> -		}
> -	}
> +	rk_combphy_common_cfg_ssc(priv, rate);
>  
>  	return 0;
>  }
> @@ -1255,10 +1243,6 @@ static int rk3588_combphy_cfg(struct rockchip_combphy_priv *priv)
>  		}
>  		break;
>  	case PHY_TYPE_USB3:
> -		/* Set SSC downward spread spectrum */
> -		val = RK3568_PHYREG32_SSC_DOWNWARD << RK3568_PHYREG32_SSC_DIR_SHIFT;
> -		rockchip_combphy_updatel(priv, RK3568_PHYREG32_SSC_MASK, val, RK3568_PHYREG32);
> -
>  		/* Enable adaptive CTLE for USB3.0 Rx. */
>  		val = readl(priv->mmio + RK3568_PHYREG15);
>  		val |= RK3568_PHYREG15_CTLE_EN;
> @@ -1343,13 +1327,6 @@ static int rk3588_combphy_cfg(struct rockchip_combphy_priv *priv)
>  
>  			/* Set up su_trim:  */
>  			writel(RK3568_PHYREG11_SU_TRIM_0_7, priv->mmio + RK3568_PHYREG11);
> -		} else if (priv->type == PHY_TYPE_SATA) {
> -			/* downward spread spectrum +500ppm */
> -			val = RK3568_PHYREG32_SSC_DOWNWARD << RK3568_PHYREG32_SSC_DIR_SHIFT;
> -			val |= RK3568_PHYREG32_SSC_OFFSET_500PPM <<
> -			       RK3568_PHYREG32_SSC_OFFSET_SHIFT;
> -			rockchip_combphy_updatel(priv, RK3568_PHYREG32_SSC_MASK, val,
> -						 RK3568_PHYREG32);
>  		}
>  		break;
>  	default:
> @@ -1371,11 +1348,7 @@ static int rk3588_combphy_cfg(struct rockchip_combphy_priv *priv)
>  		}
>  	}
>  
> -	if (priv->enable_ssc) {
> -		val = readl(priv->mmio + RK3568_PHYREG8);
> -		val |= RK3568_PHYREG8_SSC_EN;
> -		writel(val, priv->mmio + RK3568_PHYREG8);
> -	}
> +	rk_combphy_common_cfg_ssc(priv, rate);
>  
>  	return 0;
>  }
> -- 
> 2.43.0
> 
> 
> -- 
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
~Vinod

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

      reply	other threads:[~2026-05-10 11:01 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-27  0:57 [PATCH v3] phy: rockchip: naneng-combphy: Consolidate SSC configuration Shawn Lin
2026-05-10 11:01 ` Vinod Koul [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=agBlk6-vV3o2mPkf@vaman \
    --to=vkoul@kernel.org \
    --cc=heiko@sntech.de \
    --cc=linux-phy@lists.infradead.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=neil.armstrong@linaro.org \
    --cc=shawn.lin@rock-chips.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox