[PATCH net-next] net: phy: micrel: Add callback for restoring context

Biju Das posted 1 patch 3 months ago
drivers/net/phy/micrel.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
[PATCH net-next] net: phy: micrel: Add callback for restoring context
Posted by Biju Das 3 months ago
The Renesas RZ/G3E SMARC EVK uses KSZ9131RNXC phy. On deep power state,
PHY loses the power and on wakeup the rgmii delays are not reconfigured
causing it to fail.

Add a generic resume callback for restoring the context on exit from
the PM suspend state. KSZ9131 is the first user of this callback and
reconfigure the rgmii_delay when it exit from PM suspend state.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
Ref:
 https://patchwork.kernel.org/project/linux-renesas-soc/patch/20250705170326.106073-1-biju.das.jz@bp.renesas.com/#26459627
---
 drivers/net/phy/micrel.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 74fd6ff32c6c..cea6082ac45c 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -362,6 +362,8 @@
 /* Delay used to get the second part from the LTC */
 #define LAN8841_GET_SEC_LTC_DELAY		(500 * NSEC_PER_MSEC)
 
+static void ksz9131_restore_rgmii_delay(struct phy_device *phydev);
+
 struct kszphy_hw_stat {
 	const char *string;
 	u8 reg;
@@ -374,6 +376,7 @@ static struct kszphy_hw_stat kszphy_hw_stats[] = {
 };
 
 struct kszphy_type {
+	void (*resume)(struct phy_device *phydev);
 	u32 led_mode_reg;
 	u16 interrupt_level_mask;
 	u16 cable_diag_reg;
@@ -444,6 +447,7 @@ struct kszphy_priv {
 	bool rmii_ref_clk_sel;
 	bool rmii_ref_clk_sel_val;
 	bool clk_enable;
+	bool is_suspended;
 	u64 stats[ARRAY_SIZE(kszphy_hw_stats)];
 	struct kszphy_phy_stats phy_stats;
 };
@@ -491,6 +495,7 @@ static const struct kszphy_type ksz9021_type = {
 };
 
 static const struct kszphy_type ksz9131_type = {
+	.resume = ksz9131_restore_rgmii_delay,
 	.interrupt_level_mask	= BIT(14),
 	.disable_dll_tx_bit	= BIT(12),
 	.disable_dll_rx_bit	= BIT(12),
@@ -1387,6 +1392,12 @@ static int ksz9131_config_rgmii_delay(struct phy_device *phydev)
 			      txcdll_val);
 }
 
+static void ksz9131_restore_rgmii_delay(struct phy_device *phydev)
+{
+	if (phy_interface_is_rgmii(phydev))
+		ksz9131_config_rgmii_delay(phydev);
+}
+
 /* Silicon Errata DS80000693B
  *
  * When LEDs are configured in Individual Mode, LED1 is ON in a no-link
@@ -2345,6 +2356,11 @@ static int kszphy_generic_suspend(struct phy_device *phydev)
 
 static int kszphy_suspend(struct phy_device *phydev)
 {
+	struct kszphy_priv *priv = phydev->priv;
+
+	if (priv)
+		priv->is_suspended = true;
+
 	/* Disable PHY Interrupts */
 	if (phy_interrupt_is_valid(phydev)) {
 		phydev->interrupts = PHY_INTERRUPT_DISABLED;
@@ -2381,8 +2397,17 @@ static void kszphy_parse_led_mode(struct phy_device *phydev)
 
 static int kszphy_resume(struct phy_device *phydev)
 {
+	struct kszphy_priv *priv = phydev->priv;
 	int ret;
 
+	if (priv && priv->is_suspended) {
+		const struct kszphy_type *type = priv->type;
+
+		priv->is_suspended = false;
+		if (type->resume)
+			type->resume(phydev);
+	}
+
 	ret = kszphy_generic_resume(phydev);
 	if (ret)
 		return ret;
-- 
2.43.0
Re: [PATCH net-next] net: phy: micrel: Add callback for restoring context
Posted by Paolo Abeni 2 months, 4 weeks ago
On 7/7/25 4:29 PM, Biju Das wrote:
> @@ -374,6 +376,7 @@ static struct kszphy_hw_stat kszphy_hw_stats[] = {
>  };
>  
>  struct kszphy_type {
> +	void (*resume)(struct phy_device *phydev);
>  	u32 led_mode_reg;
>  	u16 interrupt_level_mask;
>  	u16 cable_diag_reg;

Why adding another callback? I think you could avoid it using a
ksz9131-specific phy_driver->resume.

> @@ -444,6 +447,7 @@ struct kszphy_priv {
>  	bool rmii_ref_clk_sel;
>  	bool rmii_ref_clk_sel_val;
>  	bool clk_enable;
> +	bool is_suspended;
>  	u64 stats[ARRAY_SIZE(kszphy_hw_stats)];
>  	struct kszphy_phy_stats phy_stats;
>  };
> @@ -491,6 +495,7 @@ static const struct kszphy_type ksz9021_type = {
>  };
>  
>  static const struct kszphy_type ksz9131_type = {
> +	.resume = ksz9131_restore_rgmii_delay,
>  	.interrupt_level_mask	= BIT(14),
>  	.disable_dll_tx_bit	= BIT(12),
>  	.disable_dll_rx_bit	= BIT(12),
> @@ -1387,6 +1392,12 @@ static int ksz9131_config_rgmii_delay(struct phy_device *phydev)
>  			      txcdll_val);
>  }
>  
> +static void ksz9131_restore_rgmii_delay(struct phy_device *phydev)
> +{
> +	if (phy_interface_is_rgmii(phydev))
> +		ksz9131_config_rgmii_delay(phydev);
> +}
> +
>  /* Silicon Errata DS80000693B
>   *
>   * When LEDs are configured in Individual Mode, LED1 is ON in a no-link
> @@ -2345,6 +2356,11 @@ static int kszphy_generic_suspend(struct phy_device *phydev)
>  
>  static int kszphy_suspend(struct phy_device *phydev)
>  {
> +	struct kszphy_priv *priv = phydev->priv;
> +
> +	if (priv)
> +		priv->is_suspended = true;

Under which circumstances `priv` could be NULL? AFAICS it should always
not NULL after probe.

> +
>  	/* Disable PHY Interrupts */
>  	if (phy_interrupt_is_valid(phydev)) {
>  		phydev->interrupts = PHY_INTERRUPT_DISABLED;
> @@ -2381,8 +2397,17 @@ static void kszphy_parse_led_mode(struct phy_device *phydev)
>  
>  static int kszphy_resume(struct phy_device *phydev)
>  {
> +	struct kszphy_priv *priv = phydev->priv;
>  	int ret;
>  
> +	if (priv && priv->is_suspended) {

I think you can use phydev->suspended instead of adding another flag.

Also, can resume be invoked without the phydev being suspended?

/P