[PATCH v5 0/2] net: phylink: add phylink_set_mac_pm() helper

[PATCH v5 0/2] net: phylink: add phylink_set_mac_pm() helper

[Shenwei Wang]

Per Russell's suggestion, the implementation is changed from the helper
function to add an extra property in phylink_config structure because this
change can easily cover SFP usecase too.

Changes in v5:
 - Add fix tag in the commit message

Changes in v4:
 - Clean up the codes in phylink.c
 - Continue the version number

Shenwei Wang (2):
  net: phylink: add mac_managed_pm in phylink_config structure
  net: stmmac: Enable mac_managed_pm phylink config

 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 1 +
 drivers/net/phy/phylink.c                         | 3 +++
 include/linux/phylink.h                           | 2 ++
 3 files changed, 6 insertions(+)

--
2.34.1

[PATCH v5 1/2] net: phylink: add mac_managed_pm in phylink_config structure

[Shenwei Wang]

The recent commit

'commit 47ac7b2f6a1f ("net: phy: Warn about incorrect
mdio_bus_phy_resume() state")'

requires the MAC driver explicitly tell the phy driver who is
managing the PM, otherwise you will see warning during resume
stage.

Add a boolean property in the phylink_config structure so that
the MAC driver can use it to tell the PHY driver if it wants to
manage the PM.

'Fixes: 47ac7b2f6a1f ("net: phy: Warn about incorrect
mdio_bus_phy_resume() state")'

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 3 +++
 include/linux/phylink.h   | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 75464df191ef..6547b6cc6cbe 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1661,6 +1661,9 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy,
 	if (phy_interrupt_is_valid(phy))
 		phy_request_interrupt(phy);

+	if (pl->config->mac_managed_pm)
+		phy->mac_managed_pm = true;
+
 	return 0;
 }

diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 664dd409feb9..3f01ac8017e0 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -122,6 +122,7 @@ enum phylink_op_type {
  *	(See commit 7cceb599d15d ("net: phylink: avoid mac_config calls")
  * @poll_fixed_state: if true, starts link_poll,
  *		      if MAC link is at %MLO_AN_FIXED mode.
+ * @mac_managed_pm: if true, indicate the MAC driver is responsible for PHY PM.
  * @ovr_an_inband: if true, override PCS to MLO_AN_INBAND
  * @get_fixed_state: callback to execute to determine the fixed link state,
  *		     if MAC link is at %MLO_AN_FIXED mode.
@@ -134,6 +135,7 @@ struct phylink_config {
 	enum phylink_op_type type;
 	bool legacy_pre_march2020;
 	bool poll_fixed_state;
+	bool mac_managed_pm;
 	bool ovr_an_inband;
 	void (*get_fixed_state)(struct phylink_config *config,
 				struct phylink_link_state *state);
--
2.34.1

[PATCH v5 2/2] net: stmmac: Enable mac_managed_pm phylink config

[Shenwei Wang]

Enable the mac_managed_pm configuration in the phylink_config
structure to avoid the kernel warning during system resume.

'Fixes: 47ac7b2f6a1f ("net: phy: Warn about incorrect
mdio_bus_phy_resume() state")'

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 65c96773c6d2..8273e6a175c8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1214,6 +1214,7 @@ static int stmmac_phy_setup(struct stmmac_priv *priv)
 	if (priv->plat->tx_queues_to_use > 1)
 		priv->phylink_config.mac_capabilities &=
 			~(MAC_10HD | MAC_100HD | MAC_1000HD);
+	priv->phylink_config.mac_managed_pm = true;

 	phylink = phylink_create(&priv->phylink_config, fwnode,
 				 mode, &stmmac_phylink_mac_ops);
--
2.34.1

Re: [PATCH v5 0/2] net: phylink: add phylink_set_mac_pm() helper

[Russell King (Oracle)]

On Thu, Oct 13, 2022 at 08:39:02AM -0500, Shenwei Wang wrote:
> Per Russell's suggestion, the implementation is changed from the helper
> function to add an extra property in phylink_config structure because this
> change can easily cover SFP usecase too.

Which tree are you aiming this for - net-next or net?

You should use [PATCH net ...] or [PATCH net-next ...] to indicate which
tree you're aiming these patches for.

Please don't repost due to this unless you want it to go into the net
tree, as net-next is currently closed.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v5 1/2] net: phylink: add mac_managed_pm in phylink_config structure

[Florian Fainelli]

On 10/13/22 06:39, Shenwei Wang wrote:
> The recent commit
> 
> 'commit 47ac7b2f6a1f ("net: phy: Warn about incorrect
> mdio_bus_phy_resume() state")'
> 
> requires the MAC driver explicitly tell the phy driver who is
> managing the PM, otherwise you will see warning during resume
> stage.
> 
> Add a boolean property in the phylink_config structure so that
> the MAC driver can use it to tell the PHY driver if it wants to
> manage the PM.
> 
> 'Fixes: 47ac7b2f6a1f ("net: phy: Warn about incorrect
> mdio_bus_phy_resume() state")'

This is not the way to provide a Fixse tag, it should simply be:

Fixes: 47ac7b2f6a1f ("net: phy: Warn about incorrect 
mdio_bus_phy_resume() state"

With that fixed:

Acked-by: Florian Fainelli <f.fainelli@gmail.com>

as a courtesy, you could CC the author of the patch you are fixing BTW
-- 
Florian

Re: [PATCH v5 2/2] net: stmmac: Enable mac_managed_pm phylink config

[Florian Fainelli]

On 10/13/22 06:39, Shenwei Wang wrote:
> Enable the mac_managed_pm configuration in the phylink_config
> structure to avoid the kernel warning during system resume.
> 
> 'Fixes: 47ac7b2f6a1f ("net: phy: Warn about incorrect
> mdio_bus_phy_resume() state")'
> 
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>

Same comment as patch #1, the Fixes tag is improperly formatted and does 
not require the use of ' before and after.

With that fixed:

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

RE: [EXT] Re: [PATCH v5 0/2] net: phylink: add phylink_set_mac_pm() helper

[Shenwei Wang]

> -----Original Message-----
> From: Russell King <linux@armlinux.org.uk>
> Sent: Thursday, October 13, 2022 11:07 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Maxime Coquelin <mcoquelin.stm32@gmail.com>;
> Andrew Lunn <andrew@lunn.ch>; Heiner Kallweit <hkallweit1@gmail.com>;
> Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue
> <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>;
> netdev@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com; linux-
> arm-kernel@lists.infradead.org; imx@lists.linux.dev
> Subject: [EXT] Re: [PATCH v5 0/2] net: phylink: add phylink_set_mac_pm() helper
> 
> Caution: EXT Email
> 
> On Thu, Oct 13, 2022 at 08:39:02AM -0500, Shenwei Wang wrote:
> > Per Russell's suggestion, the implementation is changed from the
> > helper function to add an extra property in phylink_config structure
> > because this change can easily cover SFP usecase too.
> 
> Which tree are you aiming this for - net-next or net?
> 

The patch can be applied to both trees.  You can select the one which is easy to
go ahead.

Thanks,
Shenwei

> You should use [PATCH net ...] or [PATCH net-next ...] to indicate which tree
> you're aiming these patches for.
> 
> Please don't repost due to this unless you want it to go into the net tree, as net-
> next is currently closed.
> 
> --
> RMK's Patch system:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ar
> mlinux.org.uk%2Fdeveloper%2Fpatches%2F&amp;data=05%7C01%7Cshenwei.
> wang%40nxp.com%7C9eb73c5218ab4ca2e25908daad350702%7C686ea1d3bc2
> b4c6fa92cd99c5c301635%7C0%7C0%7C638012740506537030%7CUnknown%7
> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJ
> XVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=ZUtXNJ8wXqhi9Tvcg51uJJvcS
> 9CbJt6yxF3zjuoEfuQ%3D&amp;reserved=0
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

RE: [EXT] Re: [PATCH v5 1/2] net: phylink: add mac_managed_pm in phylink_config structure

[Shenwei Wang]

> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: Thursday, October 13, 2022 1:48 PM
> On 10/13/22 06:39, Shenwei Wang wrote:
> > The recent commit
> >
> > 'commit 47ac7b2f6a1f ("net: phy: Warn about incorrect
> > mdio_bus_phy_resume() state")'
> >
> > requires the MAC driver explicitly tell the phy driver who is managing
> > the PM, otherwise you will see warning during resume stage.
> >
> > Add a boolean property in the phylink_config structure so that the MAC
> > driver can use it to tell the PHY driver if it wants to manage the PM.
> >
> > 'Fixes: 47ac7b2f6a1f ("net: phy: Warn about incorrect
> > mdio_bus_phy_resume() state")'
> 
> This is not the way to provide a Fixse tag, it should simply be:
> 
> Fixes: 47ac7b2f6a1f ("net: phy: Warn about incorrect
> mdio_bus_phy_resume() state"
> 

That was my original format. But it met the following warning when ran checkpatch.pl script:

./scripts/checkpatch.pl 0001-net-phylink-add-mac_managed_pm-in-phylink_config-str.patch 
WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: e6a39ffcfe22 ("net: stmmac: Enable mac_managed_pm phylink config")'
#20: 
Fixes: 47ac7b2f6a1f ("net: phy: Warn about incorrect

That's why I changed to the current tag format.

Regards,
Shenwei

> With that fixed:
> 
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> as a courtesy, you could CC the author of the patch you are fixing BTW
> --
> Florian

Re: [EXT] Re: [PATCH v5 0/2] net: phylink: add phylink_set_mac_pm() helper

[Russell King (Oracle)]

On Thu, Oct 13, 2022 at 07:24:02PM +0000, Shenwei Wang wrote:
> > -----Original Message-----
> > From: Russell King <linux@armlinux.org.uk>
> > Sent: Thursday, October 13, 2022 11:07 AM
> > On Thu, Oct 13, 2022 at 08:39:02AM -0500, Shenwei Wang wrote:
> > > Per Russell's suggestion, the implementation is changed from the
> > > helper function to add an extra property in phylink_config structure
> > > because this change can easily cover SFP usecase too.
> > 
> > Which tree are you aiming this for - net-next or net?
> 
> The patch can be applied to both trees.  You can select the one which is easy to
> go ahead.

That may be the case at the moment, because the net-next tree has been
merged into mainline and the net tree recently updated to mainline, but
that is not always the case.

The purpose of the tag in the subject line is to tell the various
maintainers on the netdev mailing list what _your_ expectation is for
the patch and where _you_ intend it to be applied.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

RE: [EXT] Re: [PATCH v5 1/2] net: phylink: add mac_managed_pm in phylink_config structure

[Shenwei Wang]

RESEND because some recipients' mail address were somehow corrupted.

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Thursday, October 13, 2022 6:12 PM
> To: Shenwei Wang <shenwei.wang@nxp.com> On Thu, 13 Oct 2022 19:28:45 
> +0000 Shenwei Wang wrote:
> > > Fixes: 47ac7b2f6a1f ("net: phy: Warn about incorrect
> > > mdio_bus_phy_resume() state"
> > >
> >
> > That was my original format. But it met the following warning when 
> > ran
> checkpatch.pl script:
> >
> > ./scripts/checkpatch.pl
> > 0001-net-phylink-add-mac_managed_pm-in-phylink_config-str.patch
> > WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> 
> > ("<title
> line>")' - ie: 'Fixes: e6a39ffcfe22 ("net: stmmac: Enable 
> line>mac_managed_pm
> phylink config")'
> > #20:
> > Fixes: 47ac7b2f6a1f ("net: phy: Warn about incorrect
> >
> > That's why I changed to the current tag format.
> 
> It needs to be all on one line:
> 
> Fixes: 47ac7b2f6a1f ("net: phy: Warn about incorrect 
> mdio_bus_phy_resume()
> state")
> 

En, that's the tricky.  Seems the rule of size limit is not applied for this use case.

> But I don't see 47ac7b2f6a1f in my trees, are you sure that's the 
> upstream commit id?

Just had a check again, and commit id on the upstream changed to 

commit 744d23c71af39c7dc77ac7c3cac87ae86a181a85 upstream

Will post a new patches to fix the tag.

Thanks,
Shenwei