* [PATCH net-next] net: phy: micrel: Add callback for restoring context
@ 2025-07-07 14:29 Biju Das
2025-07-10 13:38 ` Paolo Abeni
0 siblings, 1 reply; 3+ messages in thread
From: Biju Das @ 2025-07-07 14:29 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Biju Das, Russell King, netdev, linux-kernel, Geert Uytterhoeven,
Prabhakar Mahadev Lad, Biju Das, linux-renesas-soc
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] net: phy: micrel: Add callback for restoring context
2025-07-07 14:29 [PATCH net-next] net: phy: micrel: Add callback for restoring context Biju Das
@ 2025-07-10 13:38 ` Paolo Abeni
2025-07-10 17:43 ` Biju Das
0 siblings, 1 reply; 3+ messages in thread
From: Paolo Abeni @ 2025-07-10 13:38 UTC (permalink / raw)
To: Biju Das, Andrew Lunn, Heiner Kallweit, David S. Miller,
Eric Dumazet, Jakub Kicinski
Cc: Russell King, netdev, linux-kernel, Geert Uytterhoeven,
Prabhakar Mahadev Lad, Biju Das, linux-renesas-soc
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH net-next] net: phy: micrel: Add callback for restoring context
2025-07-10 13:38 ` Paolo Abeni
@ 2025-07-10 17:43 ` Biju Das
0 siblings, 0 replies; 3+ messages in thread
From: Biju Das @ 2025-07-10 17:43 UTC (permalink / raw)
To: Paolo Abeni, Andrew Lunn, Heiner Kallweit, David S. Miller,
Eric Dumazet, Jakub Kicinski
Cc: Russell King, netdev, linux-kernel, Geert Uytterhoeven,
Prabhakar Mahadev Lad, biju.das.au, linux-renesas-soc
Hi Paolo Abeni,
Thanks for the feedback.
> -----Original Message-----
> From: Paolo Abeni <pabeni@redhat.com>
> Sent: 10 July 2025 14:38
> Subject: Re: [PATCH net-next] net: phy: micrel: Add callback for restoring context
>
> 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.
It is being called from open() as well see [1]
[1]
[ 1.934177] kszphy_resume+0x18/0xc4
[ 1.934193] phy_resume+0x3c/0x74
[ 1.934207] phy_attach_direct+0x1bc/0x398
[ 1.934220] phylink_fwnode_phy_connect+0xb0/0x130
[ 1.934237] __stmmac_open+0xe4/0x50c
[ 1.934251] stmmac_open+0x44/0xd4
>
> > @@ -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.
On the kszphy_config_init(), there is a check for NULL. That is the reason for adding it here.
Yor right, priv cannot be 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.
Yes, I can use phydev->suspended as phylink_prepare_resume() calls phy_resume()
with phydev->suspended set.
>
> Also, can resume be invoked without the phydev being suspended?
Yes, phy_attach_direct() during open() invokes resume.
Cheers,
Biju
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-07-10 17:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-07 14:29 [PATCH net-next] net: phy: micrel: Add callback for restoring context Biju Das
2025-07-10 13:38 ` Paolo Abeni
2025-07-10 17:43 ` Biju Das
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®