From: Shawn Lin <shawn.lin@rock-chips.com>
To: Vinod Koul <vkoul@kernel.org>, linux-phy@lists.infradead.org
Cc: shawn.lin@rock-chips.com, linux-rockchip@lists.infradead.org,
Heiko Stuebner <heiko@sntech.de>,
Neil Armstrong <neil.armstrong@linaro.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] phy: rockchip: naneng-combphy: Consolidate SSC configuration
Date: Wed, 8 Apr 2026 10:53:42 +0800 [thread overview]
Message-ID: <b6463bdf-43c3-98aa-e4d0-75c88812b674@rock-chips.com> (raw)
In-Reply-To: <6159826d-0f79-a204-a446-d863644abbb0@rock-chips.com>
Hi Vinod and linux-phy folks
在 2026/03/25 星期三 15:11, Shawn Lin 写道:
> Hi Vinod
>
> 在 2026/03/05 星期四 15:40, Shawn Lin 写道:
>> 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.
>>
>
> Gentle ping...
>
I saw there are lots of pending phy patches waiting in queue, pinging
for reviewing, but and the linux-phy [1] hasn't updated for quite a long
time. It's latc -RC7 now, I think this patch will miss this merge
windows without surprise. But what's the plan for this?
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/phy.git/log/?h=next
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> ---
>>
>> Changes in v2:
>> - rework to consolidate more configuration
>> - reword the commit message
>>
>> drivers/phy/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 b60d6bf..2b0f152 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;
>> }
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
prev parent reply other threads:[~2026-04-08 2:53 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-05 7:40 [PATCH v2] phy: rockchip: naneng-combphy: Consolidate SSC configuration Shawn Lin
2026-03-06 10:38 ` Neil Armstrong
2026-03-10 8:40 ` Heiko Stuebner
2026-03-25 7:11 ` Shawn Lin
2026-04-08 2:53 ` Shawn Lin [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=b6463bdf-43c3-98aa-e4d0-75c88812b674@rock-chips.com \
--to=shawn.lin@rock-chips.com \
--cc=heiko@sntech.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-phy@lists.infradead.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=neil.armstrong@linaro.org \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox