[v7,net,4/4] net: axiemac: use a phandle to reference pcs_phy

Commit Message

Andy Chiu March 29, 2022, 2:49 a.m. UTC

In some SGMII use cases where both a fixed link external PHY and the
internal PCS/PMA PHY need to be configured, we should explicitly use a
phandle "pcs-phy" to get the reference to the PCS/PMA PHY. Otherwise, the
driver would use "phy-handle" in the DT as the reference to both the
external and the internal PCS/PMA PHY.

In other cases where the core is connected to a SFP cage, we could still
point phy-handle to the intenal PCS/PMA PHY, and let the driver connect
to the SFP module, if exist, via phylink.

Fixes: 1a02556086fc (net: axienet: Properly handle PCS/PMA PHY for 1000BaseX mode)
Signed-off-by: Andy Chiu <[e-mail]>
Reviewed-by: Greentime Hu <[e-mail]>
Reviewed-by: Robert Hancock <[e-mail]>
---
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Jakub Kicinski March 29, 2022, 10:56 p.m. UTC | #1

On Tue, 29 Mar 2022 10:49:21 +0800 Andy Chiu wrote:
> In some SGMII use cases where both a fixed link external PHY and the
> internal PCS/PMA PHY need to be configured, we should explicitly use a
> phandle "pcs-phy" to get the reference to the PCS/PMA PHY. Otherwise, the
> driver would use "phy-handle" in the DT as the reference to both the
> external and the internal PCS/PMA PHY.
> 
> In other cases where the core is connected to a SFP cage, we could still
> point phy-handle to the intenal PCS/PMA PHY, and let the driver connect
> to the SFP module, if exist, via phylink.
> 
> Fixes: 1a02556086fc (net: axienet: Properly handle PCS/PMA PHY for 1000BaseX mode)
> Signed-off-by: Andy Chiu <[e-mail]>
> Reviewed-by: Greentime Hu <[e-mail]>
> Reviewed-by: Robert Hancock <[e-mail]>

I'm not sure if this is a fix or adding support for a new configuration.
Andrew, WDYT?

If it really is a fix and needs to be backported we should take patch 2
out of this series, and post it separately later. Refactoring does not
belong in stable trees.

Andrew Lunn March 30, 2022, 1:25 a.m. UTC | #2

On Tue, Mar 29, 2022 at 03:56:09PM -0700, Jakub Kicinski wrote:
> On Tue, 29 Mar 2022 10:49:21 +0800 Andy Chiu wrote:
> > In some SGMII use cases where both a fixed link external PHY and the
> > internal PCS/PMA PHY need to be configured, we should explicitly use a
> > phandle "pcs-phy" to get the reference to the PCS/PMA PHY. Otherwise, the
> > driver would use "phy-handle" in the DT as the reference to both the
> > external and the internal PCS/PMA PHY.
> > 
> > In other cases where the core is connected to a SFP cage, we could still
> > point phy-handle to the intenal PCS/PMA PHY, and let the driver connect
> > to the SFP module, if exist, via phylink.
> > 
> > Fixes: 1a02556086fc (net: axienet: Properly handle PCS/PMA PHY for 1000BaseX mode)
> > Signed-off-by: Andy Chiu <[e-mail]>
> > Reviewed-by: Greentime Hu <[e-mail]>
> > Reviewed-by: Robert Hancock <[e-mail]>
> 
> I'm not sure if this is a fix or adding support for a new configuration.
> Andrew, WDYT?

I guess it fails this stable rule:

It must fix a problem that causes a build error (but not for things
marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
security issue, or some “oh, that’s not good” issue. In short,
something critical.

So this probably should be for net-next.

   Andrew

Jakub Kicinski March 30, 2022, 1:48 a.m. UTC | #3

On Wed, 30 Mar 2022 03:25:15 +0200 Andrew Lunn wrote:
> > I'm not sure if this is a fix or adding support for a new configuration.
> > Andrew, WDYT?  
> 
> I guess it fails this stable rule:
> 
> It must fix a problem that causes a build error (but not for things
> marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
> security issue, or some “oh, that’s not good” issue. In short,
> something critical.
> 
> So this probably should be for net-next.

Okay, thanks! Just to spell it out this means you'll need to hold off
with reposting, Andy, until net-next re-opens (which means until
5.18-rc1 kernel version is tagged by Linus, see the netdev-FAQ).

You should also drop the Fixes tag from patch 4. The change will make
it to Linux 5.19 and won't be backported to stable.

Andy Chiu March 30, 2022, 8:50 a.m. UTC | #4

thanks for explaining this,

I will make the change accordingly and post it to net-next when available.

Andy

Patch

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 3daef64a85bd..d6fc3f7acdf0 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -2071,9 +2071,16 @@  static int axienet_probe(struct platform_device *pdev)
 
 	if (lp->phy_mode == PHY_INTERFACE_MODE_SGMII ||
 	    lp->phy_mode == PHY_INTERFACE_MODE_1000BASEX) {
-		np = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
+		np = of_parse_phandle(pdev->dev.of_node, "pcs-handle", 0);
 		if (!np) {
-			dev_err(&pdev->dev, "phy-handle required for 1000BaseX/SGMII\n");
+			/* Deprecated: Always use "pcs-handle" for pcs_phy.
+			 * Falling back to "phy-handle" here is only for
+			 * backward compatibility with old device trees.
+			 */
+			np = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
+		}
+		if (!np) {
+			dev_err(&pdev->dev, "pcs-handle (preferred) or phy-handle required for 1000BaseX/SGMII\n");
 			ret = -EINVAL;
 			goto cleanup_mdio;
 		}