Re: [net-next,PATCH v2 3/3] net: phy: realtek: Add property to enable SSC

[Ivan Galkin <Ivan.Galkin@axis.com>]

On Wed, 2025-12-03 at 22:08 +0100, Marek Vasut wrote:
> Add support for spread spectrum clocking (SSC) on RTL8211F(D)(I)-CG,
> RTL8211FS(I)(-VS)-CG, RTL8211FG(I)(-VS)-CG PHYs. The implementation
> follows EMI improvement application note Rev. 1.2 for these PHYs.
> 
> The current implementation enables SSC for both RXC and SYSCLK clock
> signals. Introduce DT properties 'realtek,clkout-ssc-enable',
> 'realtek,rxc-ssc-enable' and 'realtek,sysclk-ssc-enable' which
> control
> CLKOUT, RXC and SYSCLK SSC spread spectrum clocking enablement on
> these
> signals.
> 
> Signed-off-by: Marek Vasut <marek.vasut@mailbox.org>
> ---
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Aleksander Jan Bajkowski <olek2@wp.pl>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Heiner Kallweit <hkallweit1@gmail.com>
> Cc: Ivan Galkin <ivan.galkin@axis.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Michael Klein <michael@fossekall.de>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>
> Cc: devicetree@vger.kernel.org
> Cc: netdev@vger.kernel.org
> ---
> V2: Split SSC clock control for each CLKOUT, RXC, SYSCLK signal
> ---
>  drivers/net/phy/realtek/realtek_main.c | 124
> +++++++++++++++++++++++++
>  1 file changed, 124 insertions(+)
> 
> diff --git a/drivers/net/phy/realtek/realtek_main.c
> b/drivers/net/phy/realtek/realtek_main.c
> index 67ecf3d4af2b1..ac80653cdbe28 100644
> --- a/drivers/net/phy/realtek/realtek_main.c
> +++ b/drivers/net/phy/realtek/realtek_main.c
> @@ -74,11 +74,19 @@
>  
>  #define RTL8211F_PHYCR2                                0x19
>  #define RTL8211F_CLKOUT_EN                     BIT(0)
> +#define RTL8211F_SYSCLK_SSC_EN                 BIT(3)
>  #define RTL8211F_PHYCR2_PHY_EEE_ENABLE         BIT(5)
> +#define RTL8211F_CLKOUT_SSC_EN                 BIT(7)
>  
>  #define RTL8211F_INSR_PAGE                     0xa43
>  #define RTL8211F_INSR                          0x1d
>  
> +/* RTL8211F SSC settings */
> +#define RTL8211F_SSC_PAGE                      0xc44
> +#define RTL8211F_SSC_RXC                       0x13
> +#define RTL8211F_SSC_SYSCLK                    0x17
> +#define RTL8211F_SSC_CLKOUT                    0x19
> +
>  /* RTL8211F LED configuration */
>  #define RTL8211F_LEDCR_PAGE                    0xd04
>  #define RTL8211F_LEDCR                         0x10
> @@ -203,6 +211,9 @@ MODULE_LICENSE("GPL");
>  struct rtl821x_priv {
>         bool enable_aldps;
>         bool disable_clk_out;
> +       bool enable_clkout_ssc;
> +       bool enable_rxc_ssc;
> +       bool enable_sysclk_ssc;
>         struct clk *clk;
>         /* rtl8211f */
>         u16 iner;
> @@ -266,6 +277,12 @@ static int rtl821x_probe(struct phy_device
> *phydev)
>                                                    "realtek,aldps-
> enable");
>         priv->disable_clk_out = of_property_read_bool(dev->of_node,
>                                                      
> "realtek,clkout-disable");
> +       priv->enable_clkout_ssc = of_property_read_bool(dev->of_node,
> +                                                       "realtek,clko
> ut-ssc-enable");
> +       priv->enable_rxc_ssc = of_property_read_bool(dev->of_node,
> +                                                    "realtek,rxc-
> ssc-enable");
> +       priv->enable_sysclk_ssc = of_property_read_bool(dev->of_node,
> +                                                       "realtek,sysc
> lk-ssc-enable");
>  
>         phydev->priv = priv;
>  
> @@ -700,6 +717,101 @@ static int rtl8211f_config_phy_eee(struct
> phy_device *phydev)
>                                 RTL8211F_PHYCR2_PHY_EEE_ENABLE, 0);
>  }
>  
> +static int rtl8211f_config_clkout_ssc(struct phy_device *phydev)
> +{
> +       struct rtl821x_priv *priv = phydev->priv;
> +       struct device *dev = &phydev->mdio.dev;
> +       int ret;
> +
> +       /* The value is preserved if the device tree property is
> absent */
> +       if (!priv->enable_clkout_ssc)
> +               return 0;
> +
> +       /* RTL8211FVD has no PHYCR2 register */
> +       if (phydev->drv->phy_id == RTL_8211FVD_PHYID)
> +               return 0;

I recommend rewording this comment, otherwise any manipulation of
PHYCR2 on RTL8211FVD will be considered an error. For example, PHY-mode
EEE on this PHY is still set via PHYCR2.

IMHO the technically correct comment should state that the
configuration of CLKOUT SSC is not currently supported by this driver.