From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-m1973181.qiye.163.com (mail-m1973181.qiye.163.com [220.197.31.81]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 64D732EBBA9 for ; Wed, 8 Apr 2026 02:53:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=220.197.31.81 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775616836; cv=none; b=B7a7YoF0PZc3+hYvk5DJmly58N5ocahEzPRAcPMSwLesDaCtp1R4AmXNQvKwcpa8+kaqxkeVk1LzV6/J4q98COPVOTkuf2VI8KsoQp2xv2sw5sFedSM49pwt/rB1W3SuFsfz6EvNr+hKXEQRUy3GteZerPGJauHs/7rYSEaNYsg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775616836; c=relaxed/simple; bh=iV0fQr+GS8PbLptxQS636bh0sel8bcjEJqie4Lppa/Y=; h=Message-ID:Date:MIME-Version:Cc:Subject:To:References:From: In-Reply-To:Content-Type; b=bk9WD0kZcLm7dSnz29IZTso5RbFVJpqc5RdS+RdyzutQ5NLvh3OmKSO1DFlkVJsKE6o+neeZk8dk7S/WQ3UmxpPYFoqDwzMp/0fcmj5t9/0ALDDeZNfA8N1ssPKCLIllPmRlrlfsZkbI29El8b2Yn5pPcUcyAQkXrIDmip14g1g= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=rock-chips.com; spf=pass smtp.mailfrom=rock-chips.com; dkim=pass (1024-bit key) header.d=rock-chips.com header.i=@rock-chips.com header.b=C9rlPLt5; arc=none smtp.client-ip=220.197.31.81 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=rock-chips.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=rock-chips.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=rock-chips.com header.i=@rock-chips.com header.b="C9rlPLt5" Received: from [172.16.12.17] (unknown [58.22.7.114]) by smtp.qiye.163.com (Hmail) with ESMTP id 39e20ca6b; Wed, 8 Apr 2026 10:53:43 +0800 (GMT+08:00) Message-ID: Date: Wed, 8 Apr 2026 10:53:42 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.13.1 Cc: shawn.lin@rock-chips.com, linux-rockchip@lists.infradead.org, Heiko Stuebner , Neil Armstrong , linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] phy: rockchip: naneng-combphy: Consolidate SSC configuration To: Vinod Koul , linux-phy@lists.infradead.org References: <1772696450-139583-1-git-send-email-shawn.lin@rock-chips.com> <6159826d-0f79-a204-a446-d863644abbb0@rock-chips.com> From: Shawn Lin In-Reply-To: <6159826d-0f79-a204-a446-d863644abbb0@rock-chips.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-HM-Tid: 0a9d6b02908309cckunm3d36b83c38ea46 X-HM-MType: 1 X-HM-Spam-Status: e1kfGhgUHx5ZQUpXWQgPGg8OCBgUHx5ZQUlOS1dZFg8aDwILHllBWSg2Ly tZV1koWUFDSUNOT01LS0k3V1ktWUFJV1kPCRoVCBIfWUFZQk1NGVZMTkhIQx9JSE1MSEJWFRQJFh oXVRMBExYaEhckFA4PWVdZGBILWUFZTkNVSUlVTFVKSk9ZV1kWGg8SFR0UWUFZT0tIVUpLSU9PT0 hVSktLVUpCS0tZBg++ DKIM-Signature: a=rsa-sha256; b=C9rlPLt548aiFYZ6T4KTTt59Ip2LD/hAXdZgTYfhiP3gv+f7//bLLoGI+eVzP3JWhIiVUR6iMHF22V1UjnXLAwbaTxTbyKV/dzdLdW7em1Wfbk/tXBpKHP3DE1N0fPo7uNVJpnYbdsrk6DlLKDlEdk6Db0m3ZjFjzeJm4SeajJg=; s=default; c=relaxed/relaxed; d=rock-chips.com; v=1; bh=iUVNdXVYzKmZYVQFKhSD6IlO5Ku6zzmSXDxmhEKEoBU=; h=date:mime-version:subject:message-id:from; 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 >> >> --- >> >> 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; >>   } >