From: Simon Horman <simon.horman@corigine•com>
To: Mengyuan Lou <mengyuanlou@net-swift•com>
Cc: netdev@vger•kernel.org, Jakub Kicinski <kuba@kernel•org>,
"Russell King (Oracle)" <linux@armlinux•org.uk>,
"David S. Miller" <davem@davemloft•net>,
Paolo Abeni <pabeni@redhat•com>,
Eric Dumazet <edumazet@google•com>,
Heiner Kallweit <hkallweit1@gmail•com>,
Andrew Lunn <andrew@lunn•ch>
Subject: Re: [PATCH net-next 2/2] net: phy: add keep_data_connection to struct phydev
Date: Tue, 25 Jul 2023 14:05:36 +0200 [thread overview]
Message-ID: <ZL+6kMqETdYL7QNF@corigine.com> (raw)
In-Reply-To: <20207E0578DCE44C+20230724092544.73531-3-mengyuanlou@net-swift.com>
+ Jakub Kicinski, "Russell King (Oracle)", "David S. Miller", Paolo Abeni,
Eric Dumazet, Heiner Kallweit, Andrew Lunn
On Mon, Jul 24, 2023 at 05:24:59PM +0800, Mengyuan Lou wrote:
> Add flag keep_data_connection to struct phydev indicating whether
> phy need to keep data connection.
> Phy_suspend() will use it to decide whether PHY can be suspended
> or not.
This feels like a bug fix.
What is the behaviour of the system without this change?
> Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift•com>
> ---
> drivers/net/phy/phy_device.c | 6 ++++--
> include/linux/phy.h | 3 +++
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 0c2014accba7..4fe26660458e 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1860,8 +1860,10 @@ int phy_suspend(struct phy_device *phydev)
>
> phy_ethtool_get_wol(phydev, &wol);
> phydev->wol_enabled = wol.wolopts || (netdev && netdev->wol_enabled);
> - /* If the device has WOL enabled, we cannot suspend the PHY */
> - if (phydev->wol_enabled && !(phydrv->flags & PHY_ALWAYS_CALL_SUSPEND))
> + phydev->keep_data_connection = phydev->wol_enabled ||
> + (netdev && netdev->ncsi_enabled);
> + /* We cannot suspend the PHY, when phy and mac need to receive packets. */
> + if (phydev->keep_data_connection && !(phydrv->flags & PHY_ALWAYS_CALL_SUSPEND))
As it stands, it seems that keep_data_connection is only used in this
function. Could it be a local variable rather than field of struct
phy_device.
That said, I think Russell and Andrew will likely have a deeper insight here.
> return -EBUSY;
>
> if (!phydrv || !phydrv->suspend)
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 11c1e91563d4..bda646e7cc23 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -554,6 +554,8 @@ struct macsec_ops;
> * @mac_managed_pm: Set true if MAC driver takes of suspending/resuming PHY
> * @wol_enabled: Set to true if the PHY or the attached MAC have Wake-on-LAN
> * enabled.
> + * @keep_data_connection: Set to true if the PHY or the attached MAC need
> + * physical connection to receive packets.
> * @state: State of the PHY for management purposes
> * @dev_flags: Device-specific flags used by the PHY driver.
> *
> @@ -651,6 +653,7 @@ struct phy_device {
> unsigned is_on_sfp_module:1;
> unsigned mac_managed_pm:1;
> unsigned wol_enabled:1;
> + unsigned keep_data_connection:1;
>
> unsigned autoneg:1;
> /* The most recently read link state */
> --
> 2.41.0
>
>
next prev parent reply other threads:[~2023-07-25 12:05 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20230724092544.73531-1-mengyuanlou@net-swift.com>
2023-07-24 9:24 ` [PATCH net-next 1/2] net: ngbe: add ncsi_enable flag for wangxun nics Mengyuan Lou
2023-07-25 23:22 ` Jakub Kicinski
2023-07-26 1:59 ` mengyuanlou
2023-07-26 2:44 ` Jakub Kicinski
2023-07-26 3:12 ` mengyuanlou
2023-07-26 3:23 ` Jakub Kicinski
2023-07-24 9:24 ` [PATCH net-next 2/2] net: phy: add keep_data_connection to struct phydev Mengyuan Lou
2023-07-25 12:05 ` Simon Horman [this message]
2023-07-25 13:12 ` Russell King (Oracle)
2023-07-26 2:35 ` mengyuanlou
2023-07-26 8:10 ` Russell King (Oracle)
2023-07-26 8:54 ` Andrew Lunn
2023-07-26 16:08 ` Jakub Kicinski
2023-07-26 16:43 ` Andrew Lunn
2023-07-26 18:29 ` Jakub Kicinski
2023-07-28 9:27 ` mengyuanlou
2023-07-28 9:48 ` Andrew Lunn
2023-07-28 15:11 ` Jakub Kicinski
2023-07-25 12:13 ` [PATCH net-next 1/2] net: ngbe: add ncsi_enable flag for wangxun nics Simon Horman
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=ZL+6kMqETdYL7QNF@corigine.com \
--to=simon.horman@corigine$(echo .)com \
--cc=andrew@lunn$(echo .)ch \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=hkallweit1@gmail$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=linux@armlinux$(echo .)org.uk \
--cc=mengyuanlou@net-swift$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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