2023-03-14 07:02:41

by Sit, Michael Wei Hong

[permalink] [raw]
Subject: [PATCH net v2 0/2] Fix PHY handle no longer parsing

After the fixed link support was introduced, it is observed that PHY
no longer attach to the MAC properly.

This patch series fixes the issue and maintains fixed-link capability.

Michael Sit Wei Hong (2):
net: stmmac: fix PHY handle parsing
net: stmmac: move fixed-link support fixup code

.../net/ethernet/stmicro/stmmac/dwmac-intel.c | 11 ---------
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 24 +++++++++++++++++--
2 files changed, 22 insertions(+), 13 deletions(-)

v2: Initialize fwnode before using the variable
--
2.34.1



2023-03-14 07:03:12

by Sit, Michael Wei Hong

[permalink] [raw]
Subject: [PATCH net v2 1/2] net: stmmac: fix PHY handle parsing

phylink_fwnode_phy_connect returns 0 when set to MLO_AN_INBAND.
This causes the PHY handle parsing to skip and the PHY will not be attached
to the MAC.

Add additional check for PHY handle parsing when set to MLO_AN_INBAND.

Fixes: ab21cf920928 ("net: stmmac: make mdio register skips PHY scanning for fixed-link")
Signed-off-by: Michael Sit Wei Hong <[email protected]>
Signed-off-by: Lai Peter Jun Ann <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 8f543c3ab5c5..398adcd68ee8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1134,6 +1134,7 @@ static void stmmac_check_pcs_mode(struct stmmac_priv *priv)
static int stmmac_init_phy(struct net_device *dev)
{
struct stmmac_priv *priv = netdev_priv(dev);
+ struct fwnode_handle *fixed_node;
struct fwnode_handle *fwnode;
int ret;

@@ -1141,13 +1142,16 @@ static int stmmac_init_phy(struct net_device *dev)
if (!fwnode)
fwnode = dev_fwnode(priv->device);

- if (fwnode)
+ if (fwnode) {
+ fixed_node = fwnode_get_named_child_node(fwnode, "fixed-link");
+ fwnode_handle_put(fixed_node);
ret = phylink_fwnode_phy_connect(priv->phylink, fwnode, 0);
+ }

/* Some DT bindings do not set-up the PHY handle. Let's try to
* manually parse it
*/
- if (!fwnode || ret) {
+ if (!fwnode || ret || !fixed_node) {
int addr = priv->plat->phy_addr;
struct phy_device *phydev;

--
2.34.1


2023-03-14 07:03:21

by Sit, Michael Wei Hong

[permalink] [raw]
Subject: [PATCH net v2 2/2] net: stmmac: move fixed-link support fixup code

xpcs_an_inband value is updated in the speed_mode_2500 function
which turns on the xpcs_an_inband mode.

Moving the fixed-link fixup code to right before phylink setup to
ensure no more fixup will affect the fixed-link mode configurations.

Fixes: 72edaf39fc65 ("stmmac: intel: add phy-mode and fixed-link ACPI _DSD setting support")
Signed-off-by: Michael Sit Wei Hong <[email protected]>
---
.../net/ethernet/stmicro/stmmac/dwmac-intel.c | 11 -----------
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 16 ++++++++++++++++
2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
index 7deb1f817dac..d02db2b529b9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
@@ -592,17 +592,6 @@ static int intel_mgbe_common_data(struct pci_dev *pdev,
plat->mdio_bus_data->xpcs_an_inband = true;
}

- /* For fixed-link setup, we clear xpcs_an_inband */
- if (fwnode) {
- struct fwnode_handle *fixed_node;
-
- fixed_node = fwnode_get_named_child_node(fwnode, "fixed-link");
- if (fixed_node)
- plat->mdio_bus_data->xpcs_an_inband = false;
-
- fwnode_handle_put(fixed_node);
- }
-
/* Ensure mdio bus scan skips intel serdes and pcs-xpcs */
plat->mdio_bus_data->phy_mask = 1 << INTEL_MGBE_ADHOC_ADDR;
plat->mdio_bus_data->phy_mask |= 1 << INTEL_MGBE_XPCS_ADDR;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 398adcd68ee8..6560aed95036 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7064,6 +7064,7 @@ int stmmac_dvr_probe(struct device *device,
struct stmmac_resources *res)
{
struct net_device *ndev = NULL;
+ struct fwnode_handle *fwnode;
struct stmmac_priv *priv;
u32 rxq;
int i, ret = 0;
@@ -7306,6 +7307,21 @@ int stmmac_dvr_probe(struct device *device,
goto error_xpcs_setup;
}

+ /* For fixed-link setup, we clear xpcs_an_inband */
+ fwnode = of_fwnode_handle(priv->plat->phylink_node);
+ if (!fwnode)
+ fwnode = dev_fwnode(priv->device);
+
+ if (fwnode) {
+ struct fwnode_handle *fixed_node;
+
+ fixed_node = fwnode_get_named_child_node(fwnode, "fixed-link");
+ if (fixed_node)
+ priv->plat->mdio_bus_data->xpcs_an_inband = false;
+
+ fwnode_handle_put(fixed_node);
+ }
+
ret = stmmac_phy_setup(priv);
if (ret) {
netdev_err(ndev, "failed to setup phy (%d)\n", ret);
--
2.34.1


2023-03-14 10:03:39

by Fabian Bläse

[permalink] [raw]
Subject: Re: [PATCH net v2 1/2] net: stmmac: fix PHY handle parsing

On 14.03.23 08:02, Michael Sit Wei Hong wrote:
> phylink_fwnode_phy_connect returns 0 when set to MLO_AN_INBAND.
> This causes the PHY handle parsing to skip and the PHY will not be attached
> to the MAC.
>
> Add additional check for PHY handle parsing when set to MLO_AN_INBAND.
>
> Fixes: ab21cf920928 ("net: stmmac: make mdio register skips PHY scanning for fixed-link")
> Signed-off-by: Michael Sit Wei Hong <[email protected]>
> Signed-off-by: Lai Peter Jun Ann <[email protected]>

Tested-by: Fabian Bläse <[email protected]>

2023-03-14 10:03:42

by Fabian Bläse

[permalink] [raw]
Subject: Re: [PATCH net v2 2/2] net: stmmac: move fixed-link support fixup code

On 14.03.23 08:02, Michael Sit Wei Hong wrote:
> xpcs_an_inband value is updated in the speed_mode_2500 function
> which turns on the xpcs_an_inband mode.
>
> Moving the fixed-link fixup code to right before phylink setup to
> ensure no more fixup will affect the fixed-link mode configurations.
>
> Fixes: 72edaf39fc65 ("stmmac: intel: add phy-mode and fixed-link ACPI _DSD setting support")
> Signed-off-by: Michael Sit Wei Hong <[email protected]>

Tested-by: Fabian Bläse <[email protected]>

2023-03-17 00:00:21

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net v2 1/2] net: stmmac: fix PHY handle parsing

On Tue, 14 Mar 2023 15:02:07 +0800 Michael Sit Wei Hong wrote:
> + fixed_node = fwnode_get_named_child_node(fwnode, "fixed-link");
> + fwnode_handle_put(fixed_node);

fwnode_property_present() ?


2023-03-17 00:01:35

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net v2 2/2] net: stmmac: move fixed-link support fixup code

On Tue, 14 Mar 2023 15:02:08 +0800 Michael Sit Wei Hong wrote:
> xpcs_an_inband value is updated in the speed_mode_2500 function
> which turns on the xpcs_an_inband mode.
>
> Moving the fixed-link fixup code to right before phylink setup to
> ensure no more fixup will affect the fixed-link mode configurations.
>
> Fixes: 72edaf39fc65 ("stmmac: intel: add phy-mode and fixed-link ACPI _DSD setting support")
> Signed-off-by: Michael Sit Wei Hong <[email protected]>

Ong Boon, since you added the code which gets moved - could we get
an ack/review tag from you? Helps increase the confidence in the change.

2023-03-17 02:11:11

by Sit, Michael Wei Hong

[permalink] [raw]
Subject: RE: [PATCH net v2 1/2] net: stmmac: fix PHY handle parsing



> -----Original Message-----
> From: Jakub Kicinski <[email protected]>
> Sent: Friday, March 17, 2023 8:00 AM
> To: Sit, Michael Wei Hong <[email protected]>
> Cc: Giuseppe Cavallaro <[email protected]>; Alexandre
> Torgue <[email protected]>; Jose Abreu
> <[email protected]>; David S . Miller
> <[email protected]>; Eric Dumazet
> <[email protected]>; Paolo Abeni <[email protected]>;
> Maxime Coquelin <[email protected]>; Ong, Boon
> Leong <[email protected]>; [email protected];
> [email protected]; linux-arm-
> [email protected]; [email protected]; Looi,
> Hong Aun <[email protected]>; Voon, Weifeng
> <[email protected]>; Lai, Peter Jun Ann
> <[email protected]>
> Subject: Re: [PATCH net v2 1/2] net: stmmac: fix PHY handle
> parsing
>
> On Tue, 14 Mar 2023 15:02:07 +0800 Michael Sit Wei Hong wrote:
> > + fixed_node =
> fwnode_get_named_child_node(fwnode, "fixed-link");
> > + fwnode_handle_put(fixed_node);
>
> fwnode_property_present() ?
Good suggestion, will modify and submit in next revision.

2023-03-17 19:56:48

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net v2 1/2] net: stmmac: fix PHY handle parsing

On Tue, Mar 14, 2023 at 03:02:07PM +0800, Michael Sit Wei Hong wrote:
> phylink_fwnode_phy_connect returns 0 when set to MLO_AN_INBAND.
> This causes the PHY handle parsing to skip and the PHY will not be attached
> to the MAC.

Please could you expand the commit message because i'm having trouble
following this.

phylink_fwnode_phy_connect() says:

/* Fixed links and 802.3z are handled without needing a PHY */
if (pl->cfg_link_an_mode == MLO_AN_FIXED ||
(pl->cfg_link_an_mode == MLO_AN_INBAND &&
phy_interface_mode_is_8023z(pl->link_interface)))
return 0;

So your first statement is not true. It should be MLO_AN_INBAND
and phy_interface_mode_is_8023z.

> Add additional check for PHY handle parsing when set to MLO_AN_INBAND.

Looking at the patch, there is no reference to MLO_AN_INBAND, or
managed = "in-band-status";

Andrew

2023-03-17 20:02:58

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net v2 2/2] net: stmmac: move fixed-link support fixup code

On Tue, Mar 14, 2023 at 03:02:08PM +0800, Michael Sit Wei Hong wrote:
> xpcs_an_inband value is updated in the speed_mode_2500 function
> which turns on the xpcs_an_inband mode.
>
> Moving the fixed-link fixup code to right before phylink setup to
> ensure no more fixup will affect the fixed-link mode configurations.

Please could you explain why this is correct, when you could simple
not set priv->plat->mdio_bus_data->xpcs_an_inband = true;
in intel_speed_mode_2500()?

This all seems like hacks, rather than a clean design.

Andrew

2023-03-17 20:55:00

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH net v2 0/2] Fix PHY handle no longer parsing

On Tue, Mar 14, 2023 at 03:02:06PM +0800, Michael Sit Wei Hong wrote:
> After the fixed link support was introduced, it is observed that PHY
> no longer attach to the MAC properly.

You are aware, I hope, that fixed-link and having a PHY are mutually
exclusive?

In other words, you can't use fixed-link to specify parameters for a
PHY.

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

2023-03-17 20:58:18

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH net v2 1/2] net: stmmac: fix PHY handle parsing

On Fri, Mar 17, 2023 at 08:56:19PM +0100, Andrew Lunn wrote:
> On Tue, Mar 14, 2023 at 03:02:07PM +0800, Michael Sit Wei Hong wrote:
> > phylink_fwnode_phy_connect returns 0 when set to MLO_AN_INBAND.
> > This causes the PHY handle parsing to skip and the PHY will not be attached
> > to the MAC.
>
> Please could you expand the commit message because i'm having trouble
> following this.
>
> phylink_fwnode_phy_connect() says:
>
> /* Fixed links and 802.3z are handled without needing a PHY */
> if (pl->cfg_link_an_mode == MLO_AN_FIXED ||
> (pl->cfg_link_an_mode == MLO_AN_INBAND &&
> phy_interface_mode_is_8023z(pl->link_interface)))
> return 0;
>
> So your first statement is not true. It should be MLO_AN_INBAND
> and phy_interface_mode_is_8023z.
>
> > Add additional check for PHY handle parsing when set to MLO_AN_INBAND.
>
> Looking at the patch, there is no reference to MLO_AN_INBAND, or
> managed = "in-band-status";

That's the pesky "xpcs_an_inband" which ends up as phylink's
"ovr_an_inband"... I'm sure these are random renames of stuff to make
sure that people struggle to follow the code.

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

2023-03-21 08:36:17

by Sit, Michael Wei Hong

[permalink] [raw]
Subject: RE: [PATCH net v2 1/2] net: stmmac: fix PHY handle parsing



> -----Original Message-----
> From: Russell King <[email protected]>
> Sent: Saturday, March 18, 2023 4:58 AM
> To: Andrew Lunn <[email protected]>
> Cc: Sit, Michael Wei Hong <[email protected]>;
> Giuseppe Cavallaro <[email protected]>; Alexandre
> Torgue <[email protected]>; Jose Abreu
> <[email protected]>; David S . Miller
> <[email protected]>; Eric Dumazet
> <[email protected]>; Jakub Kicinski <[email protected]>;
> Paolo Abeni <[email protected]>; Maxime Coquelin
> <[email protected]>; Ong, Boon Leong
> <[email protected]>; [email protected]; linux-
> [email protected]; linux-arm-
> [email protected]; [email protected]; Looi,
> Hong Aun <[email protected]>; Voon, Weifeng
> <[email protected]>; Lai, Peter Jun Ann
> <[email protected]>
> Subject: Re: [PATCH net v2 1/2] net: stmmac: fix PHY handle
> parsing
>
> On Fri, Mar 17, 2023 at 08:56:19PM +0100, Andrew Lunn wrote:
> > On Tue, Mar 14, 2023 at 03:02:07PM +0800, Michael Sit Wei
> Hong wrote:
> > > phylink_fwnode_phy_connect returns 0 when set to
> MLO_AN_INBAND.
> > > This causes the PHY handle parsing to skip and the PHY will not
> be
> > > attached to the MAC.
> >
> > Please could you expand the commit message because i'm
> having trouble
> > following this.
> >
> > phylink_fwnode_phy_connect() says:
> >
> > /* Fixed links and 802.3z are handled without needing a
> PHY */
> > if (pl->cfg_link_an_mode == MLO_AN_FIXED ||
> > (pl->cfg_link_an_mode == MLO_AN_INBAND &&
> > phy_interface_mode_is_8023z(pl->link_interface)))
> > return 0;
> >
> > So your first statement is not true. It should be
> MLO_AN_INBAND and
> > phy_interface_mode_is_8023z.
> >
> > > Add additional check for PHY handle parsing when set to
> MLO_AN_INBAND.
> >
> > Looking at the patch, there is no reference to
> MLO_AN_INBAND, or
> > managed = "in-band-status";
>
> That's the pesky "xpcs_an_inband" which ends up as phylink's
> "ovr_an_inband"... I'm sure these are random renames of stuff
> to make sure that people struggle to follow the code.
>
It is as mentioned above, the "xpcs_an_inband" will end up as
"ovr_an_inband" which will then
set pl->cfg_link_an_mode = MLO_AN_INBAND in the
phylink_parse_mode() in phylink.c

The phylink_fwnode_phy_connect() checks if both
MLO_AN_INBAND && phy_interface_mode_is_8023z() true
before returning 0.

But in our case, we only have MLO_AN_INBAND is true, which
then goes to the next part of the code.

phy_fwnode = fwnode_get_phy_node(fwnode);
if (IS_ERR(phy_fwnode)) {
if (pl->cfg_link_an_mode == MLO_AN_PHY)
return -ENODEV;
return 0;
}

Where here the IS_ERR(phy_fwnode) returns true, then it
Checks for MLO_AN_PHY, which in our case is not, so it returns
a 0.

When returned 0, our driver will then skip the manual phy parsing
due to if (!fwnode || ret)
> --
> RMK's Patch system:
> https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at
> last!

2023-03-21 08:39:30

by Sit, Michael Wei Hong

[permalink] [raw]
Subject: RE: [PATCH net v2 0/2] Fix PHY handle no longer parsing



> -----Original Message-----
> From: Russell King <[email protected]>
> Sent: Saturday, March 18, 2023 4:54 AM
> To: Sit, Michael Wei Hong <[email protected]>
> Cc: Giuseppe Cavallaro <[email protected]>; Alexandre
> Torgue <[email protected]>; Jose Abreu
> <[email protected]>; David S . Miller
> <[email protected]>; Eric Dumazet
> <[email protected]>; Jakub Kicinski <[email protected]>;
> Paolo Abeni <[email protected]>; Maxime Coquelin
> <[email protected]>; Ong, Boon Leong
> <[email protected]>; [email protected]; linux-
> [email protected]; linux-arm-
> [email protected]; [email protected]; Looi,
> Hong Aun <[email protected]>; Voon, Weifeng
> <[email protected]>; Lai, Peter Jun Ann
> <[email protected]>
> Subject: Re: [PATCH net v2 0/2] Fix PHY handle no longer parsing
>
> On Tue, Mar 14, 2023 at 03:02:06PM +0800, Michael Sit Wei Hong
> wrote:
> > After the fixed link support was introduced, it is observed that
> PHY
> > no longer attach to the MAC properly.
>
> You are aware, I hope, that fixed-link and having a PHY are
> mutually exclusive?
>
> In other words, you can't use fixed-link to specify parameters for
> a PHY.
>
Yes but when the fixed-link support code was added, all our non
fixed-link devices are not attaching the PHYs to the MACs, hence
the reason for this patch series, to ensure both fixed-link and
non fixed-link scenarios work properly.
> --
> RMK's Patch system:
> https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at
> last!

2023-03-21 10:17:58

by Sit, Michael Wei Hong

[permalink] [raw]
Subject: RE: [PATCH net v2 2/2] net: stmmac: move fixed-link support fixup code



> -----Original Message-----
> From: Andrew Lunn <[email protected]>
> Sent: Saturday, March 18, 2023 4:03 AM
> To: Sit, Michael Wei Hong <[email protected]>
> Cc: Giuseppe Cavallaro <[email protected]>; Alexandre
> Torgue <[email protected]>; Jose Abreu
> <[email protected]>; David S . Miller
> <[email protected]>; Eric Dumazet
> <[email protected]>; Jakub Kicinski <[email protected]>;
> Paolo Abeni <[email protected]>; Maxime Coquelin
> <[email protected]>; Ong, Boon Leong
> <[email protected]>; [email protected]; linux-
> [email protected]; linux-arm-
> [email protected]; [email protected]; Looi,
> Hong Aun <[email protected]>; Voon, Weifeng
> <[email protected]>; Lai, Peter Jun Ann
> <[email protected]>
> Subject: Re: [PATCH net v2 2/2] net: stmmac: move fixed-link
> support fixup code
>
> On Tue, Mar 14, 2023 at 03:02:08PM +0800, Michael Sit Wei Hong
> wrote:
> > xpcs_an_inband value is updated in the speed_mode_2500
> function which
> > turns on the xpcs_an_inband mode.
> >
> > Moving the fixed-link fixup code to right before phylink setup
> to
> > ensure no more fixup will affect the fixed-link mode
> configurations.
>
> Please could you explain why this is correct, when you could
> simple not set priv->plat->mdio_bus_data->xpcs_an_inband =
> true; in intel_speed_mode_2500()?
>
> This all seems like hacks, rather than a clean design.
>
> Andrew
Makes sense, will test out with your suggestion and submit
the changes in next version, will feedback if there is any
findings.

2023-03-21 10:22:42

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH net v2 0/2] Fix PHY handle no longer parsing

On Tue, Mar 21, 2023 at 08:37:48AM +0000, Sit, Michael Wei Hong wrote:
>
>
> > -----Original Message-----
> > From: Russell King <[email protected]>
> > Sent: Saturday, March 18, 2023 4:54 AM
> > To: Sit, Michael Wei Hong <[email protected]>
> > Cc: Giuseppe Cavallaro <[email protected]>; Alexandre
> > Torgue <[email protected]>; Jose Abreu
> > <[email protected]>; David S . Miller
> > <[email protected]>; Eric Dumazet
> > <[email protected]>; Jakub Kicinski <[email protected]>;
> > Paolo Abeni <[email protected]>; Maxime Coquelin
> > <[email protected]>; Ong, Boon Leong
> > <[email protected]>; [email protected]; linux-
> > [email protected]; linux-arm-
> > [email protected]; [email protected]; Looi,
> > Hong Aun <[email protected]>; Voon, Weifeng
> > <[email protected]>; Lai, Peter Jun Ann
> > <[email protected]>
> > Subject: Re: [PATCH net v2 0/2] Fix PHY handle no longer parsing
> >
> > On Tue, Mar 14, 2023 at 03:02:06PM +0800, Michael Sit Wei Hong
> > wrote:
> > > After the fixed link support was introduced, it is observed that
> > PHY
> > > no longer attach to the MAC properly.
> >
> > You are aware, I hope, that fixed-link and having a PHY are
> > mutually exclusive?
> >
> > In other words, you can't use fixed-link to specify parameters for
> > a PHY.
> >
> Yes but when the fixed-link support code was added, all our non
> fixed-link devices are not attaching the PHYs to the MACs, hence
> the reason for this patch series, to ensure both fixed-link and
> non fixed-link scenarios work properly.

Ah, it's the "there should be a PHY here" case you're trying to cater
for, rather than actually a fixed-link.

So, how about adding a helper that is basically:

bool phylink_expects_phy(struct phylink *phylink)
{
return pl->cfg_link_an_mode == MLO_AN_PHY;
}
EXPORT_SYMBOL_GPL(phylink_expects_phy);

and use that to test whether you need to manually find a PHY to attach,
rather than trying to detect whether a fixed-link stanza appears in
firmware?

Thanks.

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

2023-03-21 10:25:45

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH net v2 1/2] net: stmmac: fix PHY handle parsing

On Tue, Mar 21, 2023 at 08:34:49AM +0000, Sit, Michael Wei Hong wrote:
>
>
> > -----Original Message-----
> > From: Russell King <[email protected]>
> > Sent: Saturday, March 18, 2023 4:58 AM
> > To: Andrew Lunn <[email protected]>
> > Cc: Sit, Michael Wei Hong <[email protected]>;
> > Giuseppe Cavallaro <[email protected]>; Alexandre
> > Torgue <[email protected]>; Jose Abreu
> > <[email protected]>; David S . Miller
> > <[email protected]>; Eric Dumazet
> > <[email protected]>; Jakub Kicinski <[email protected]>;
> > Paolo Abeni <[email protected]>; Maxime Coquelin
> > <[email protected]>; Ong, Boon Leong
> > <[email protected]>; [email protected]; linux-
> > [email protected]; linux-arm-
> > [email protected]; [email protected]; Looi,
> > Hong Aun <[email protected]>; Voon, Weifeng
> > <[email protected]>; Lai, Peter Jun Ann
> > <[email protected]>
> > Subject: Re: [PATCH net v2 1/2] net: stmmac: fix PHY handle
> > parsing
> >
> > On Fri, Mar 17, 2023 at 08:56:19PM +0100, Andrew Lunn wrote:
> > > On Tue, Mar 14, 2023 at 03:02:07PM +0800, Michael Sit Wei
> > Hong wrote:
> > > > phylink_fwnode_phy_connect returns 0 when set to
> > MLO_AN_INBAND.
> > > > This causes the PHY handle parsing to skip and the PHY will not
> > be
> > > > attached to the MAC.
> > >
> > > Please could you expand the commit message because i'm
> > having trouble
> > > following this.
> > >
> > > phylink_fwnode_phy_connect() says:
> > >
> > > /* Fixed links and 802.3z are handled without needing a
> > PHY */
> > > if (pl->cfg_link_an_mode == MLO_AN_FIXED ||
> > > (pl->cfg_link_an_mode == MLO_AN_INBAND &&
> > > phy_interface_mode_is_8023z(pl->link_interface)))
> > > return 0;
> > >
> > > So your first statement is not true. It should be
> > MLO_AN_INBAND and
> > > phy_interface_mode_is_8023z.
> > >
> > > > Add additional check for PHY handle parsing when set to
> > MLO_AN_INBAND.
> > >
> > > Looking at the patch, there is no reference to
> > MLO_AN_INBAND, or
> > > managed = "in-band-status";
> >
> > That's the pesky "xpcs_an_inband" which ends up as phylink's
> > "ovr_an_inband"... I'm sure these are random renames of stuff
> > to make sure that people struggle to follow the code.
> >
> It is as mentioned above, the "xpcs_an_inband" will end up as
> "ovr_an_inband" which will then
> set pl->cfg_link_an_mode = MLO_AN_INBAND in the
> phylink_parse_mode() in phylink.c

Let me make my comment more clear, because I don't think you understood
it correctly.

Please rename "xpcs_an_inband" to "ovr_an_inband" or
"phylink_ovr_an_inband" so it's obvious what it is and where it ends up.

Thanks.

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