[PATCH net-next v2 09/10] net: stmmac: Create DW XPCS device with particular address

[PATCH net-next v2 09/10] net: stmmac: Create DW XPCS device with particular address

[Serge Semin]

Currently the only STMMAC platform driver using the DW XPCS code is the
Intel mGBE device driver. (It can be determined by finding all the drivers
having the stmmac_mdio_bus_data::has_xpcs flag set.) At the same time the
low-level platform driver masks out the DW XPCS MDIO-address from being
auto-detected as PHY by the MDIO subsystem core. Seeing the PCS MDIO ID is
known the procedure of the DW XPCS device creation can be simplified by
dropping the loop over all the MDIO IDs. From now the DW XPCS device
descriptor will be created for the pre-defined MDIO-bus address.

Note besides this shall speed up a bit the Intel mGBE probing.

Signed-off-by: Serge Semin <fancer.lancer@gmail.com>

---

Changelog v2:
- This is a new patch introduced on v2 stage of the review.
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c |  1 +
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 15 ++++-----------
 include/linux/stmmac.h                            |  1 +
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
index 56649edb18cd..e60b7e955c35 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
@@ -586,6 +586,7 @@ static int intel_mgbe_common_data(struct pci_dev *pdev,
 	if (plat->phy_interface == PHY_INTERFACE_MODE_SGMII ||
 	    plat->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
 		plat->mdio_bus_data->has_xpcs = true;
+		plat->mdio_bus_data->xpcs_addr = INTEL_MGBE_XPCS_ADDR;
 		plat->mdio_bus_data->default_an_inband = true;
 	}
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index aa43117134d3..807789d7309a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -499,8 +499,7 @@ int stmmac_pcs_setup(struct net_device *ndev)
 {
 	struct dw_xpcs *xpcs = NULL;
 	struct stmmac_priv *priv;
-	int ret = -ENODEV;
-	int mode, addr;
+	int addr, mode, ret;
 
 	priv = netdev_priv(ndev);
 	mode = priv->plat->phy_interface;
@@ -509,15 +508,9 @@ int stmmac_pcs_setup(struct net_device *ndev)
 		ret = priv->plat->pcs_init(priv);
 	} else if (priv->plat->mdio_bus_data &&
 		   priv->plat->mdio_bus_data->has_xpcs) {
-		/* Try to probe the XPCS by scanning all addresses */
-		for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
-			xpcs = xpcs_create_mdiodev(priv->mii, addr, mode);
-			if (IS_ERR(xpcs))
-				continue;
-
-			ret = 0;
-			break;
-		}
+		addr = priv->plat->mdio_bus_data->xpcs_addr;
+		xpcs = xpcs_create_mdiodev(priv->mii, addr, mode);
+		ret = PTR_ERR_OR_ZERO(xpcs);
 	} else {
 		return 0;
 	}
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index f337286623bb..a11b850d3672 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -83,6 +83,7 @@ struct stmmac_priv;
 struct stmmac_mdio_bus_data {
 	unsigned int phy_mask;
 	unsigned int has_xpcs;
+	unsigned int xpcs_addr;
 	unsigned int default_an_inband;
 	int *irqs;
 	int probed_phy_irq;
-- 
2.43.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next v2 10/10] net: stmmac: Add DW XPCS specified via "pcs-handle" support

[Serge Semin]

Recently the DW XPCS DT-bindings have been introduced and the DW XPCS
driver has been altered to support the DW XPCS registered as a platform
device. In order to have the DW XPCS DT-device accessed from the STMMAC
driver let's alter the STMMAC PCS-setup procedure to support the
"pcs-handle" property containing the phandle reference to the DW XPCS
device DT-node. The respective fwnode will be then passed to the
xpcs_create_fwnode() function which in its turn will create the DW XPCS
descriptor utilized in the main driver for the PCS-related setups.

Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index 807789d7309a..dc040051aa53 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -497,15 +497,22 @@ int stmmac_mdio_reset(struct mii_bus *bus)
 
 int stmmac_pcs_setup(struct net_device *ndev)
 {
+	struct fwnode_handle *devnode, *pcsnode;
 	struct dw_xpcs *xpcs = NULL;
 	struct stmmac_priv *priv;
 	int addr, mode, ret;
 
 	priv = netdev_priv(ndev);
 	mode = priv->plat->phy_interface;
+	devnode = priv->plat->port_node;
 
 	if (priv->plat->pcs_init) {
 		ret = priv->plat->pcs_init(priv);
+	} else if (fwnode_property_present(devnode, "pcs-handle")) {
+		pcsnode = fwnode_find_reference(devnode, "pcs-handle", 0);
+		xpcs = xpcs_create_fwnode(pcsnode, mode);
+		fwnode_handle_put(pcsnode);
+		ret = PTR_ERR_OR_ZERO(xpcs);
 	} else if (priv->plat->mdio_bus_data &&
 		   priv->plat->mdio_bus_data->has_xpcs) {
 		addr = priv->plat->mdio_bus_data->xpcs_addr;
@@ -515,10 +522,8 @@ int stmmac_pcs_setup(struct net_device *ndev)
 		return 0;
 	}
 
-	if (ret) {
-		dev_warn(priv->device, "No xPCS found\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(priv->device, ret, "No xPCS found\n");
 
 	priv->hw->xpcs = xpcs;
 
-- 
2.43.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 10/10] net: stmmac: Add DW XPCS specified via "pcs-handle" support

[Serge Semin]

On Sun, Jun 02, 2024 at 05:36:24PM +0300, Serge Semin wrote:
> Recently the DW XPCS DT-bindings have been introduced and the DW XPCS
> driver has been altered to support the DW XPCS registered as a platform
> device. In order to have the DW XPCS DT-device accessed from the STMMAC
> driver let's alter the STMMAC PCS-setup procedure to support the
> "pcs-handle" property containing the phandle reference to the DW XPCS
> device DT-node. The respective fwnode will be then passed to the
> xpcs_create_fwnode() function which in its turn will create the DW XPCS
> descriptor utilized in the main driver for the PCS-related setups.
> 
> Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> index 807789d7309a..dc040051aa53 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> @@ -497,15 +497,22 @@ int stmmac_mdio_reset(struct mii_bus *bus)
>  
>  int stmmac_pcs_setup(struct net_device *ndev)
>  {
> +	struct fwnode_handle *devnode, *pcsnode;
>  	struct dw_xpcs *xpcs = NULL;
>  	struct stmmac_priv *priv;
>  	int addr, mode, ret;
>  
>  	priv = netdev_priv(ndev);
>  	mode = priv->plat->phy_interface;
> +	devnode = priv->plat->port_node;
>  
>  	if (priv->plat->pcs_init) {
>  		ret = priv->plat->pcs_init(priv);

> +	} else if (fwnode_property_present(devnode, "pcs-handle")) {
> +		pcsnode = fwnode_find_reference(devnode, "pcs-handle", 0);
> +		xpcs = xpcs_create_fwnode(pcsnode, mode);
> +		fwnode_handle_put(pcsnode);
> +		ret = PTR_ERR_OR_ZERO(xpcs);

Just figured, we might wish to be a bit more portable in the
"pcs-handle" property semantics implementation seeing there can be at
least three different PCS attached:
DW XPCS
Lynx PCS
Renesas RZ/N1 MII

Any suggestion of how to distinguish the passed handle? Perhaps
named-property, phandle argument, by the compatible string or the
node-name?

-Serge(y)

>  	} else if (priv->plat->mdio_bus_data &&
>  		   priv->plat->mdio_bus_data->has_xpcs) {
>  		addr = priv->plat->mdio_bus_data->xpcs_addr;
> @@ -515,10 +522,8 @@ int stmmac_pcs_setup(struct net_device *ndev)
>  		return 0;
>  	}
>  
> -	if (ret) {
> -		dev_warn(priv->device, "No xPCS found\n");
> -		return ret;
> -	}
> +	if (ret)
> +		return dev_err_probe(priv->device, ret, "No xPCS found\n");
>  
>  	priv->hw->xpcs = xpcs;
>  
> -- 
> 2.43.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 10/10] net: stmmac: Add DW XPCS specified via "pcs-handle" support

[Russell King (Oracle)]

On Mon, Jun 03, 2024 at 11:54:22AM +0300, Serge Semin wrote:
> >  	if (priv->plat->pcs_init) {
> >  		ret = priv->plat->pcs_init(priv);
> 
> > +	} else if (fwnode_property_present(devnode, "pcs-handle")) {
> > +		pcsnode = fwnode_find_reference(devnode, "pcs-handle", 0);
> > +		xpcs = xpcs_create_fwnode(pcsnode, mode);
> > +		fwnode_handle_put(pcsnode);
> > +		ret = PTR_ERR_OR_ZERO(xpcs);
> 
> Just figured, we might wish to be a bit more portable in the
> "pcs-handle" property semantics implementation seeing there can be at
> least three different PCS attached:
> DW XPCS
> Lynx PCS
> Renesas RZ/N1 MII
> 
> Any suggestion of how to distinguish the passed handle? Perhaps
> named-property, phandle argument, by the compatible string or the
> node-name?

I can't think of a reasonable solution to this at the moment. One
solution could be pushing this down into the platform code to deal
with as an interim solution, via the new .pcs_init() method.

We could also do that with the current XPCS code, since we know that
only Intel mGBE uses xpcs. This would probably allow us to get rid
of the has_xpcs flag.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 10/10] net: stmmac: Add DW XPCS specified via "pcs-handle" support

[Serge Semin]

On Mon, Jun 03, 2024 at 10:03:54AM +0100, Russell King (Oracle) wrote:
> On Mon, Jun 03, 2024 at 11:54:22AM +0300, Serge Semin wrote:
> > >  	if (priv->plat->pcs_init) {
> > >  		ret = priv->plat->pcs_init(priv);
> > 
> > > +	} else if (fwnode_property_present(devnode, "pcs-handle")) {
> > > +		pcsnode = fwnode_find_reference(devnode, "pcs-handle", 0);
> > > +		xpcs = xpcs_create_fwnode(pcsnode, mode);
> > > +		fwnode_handle_put(pcsnode);
> > > +		ret = PTR_ERR_OR_ZERO(xpcs);
> > 
> > Just figured, we might wish to be a bit more portable in the
> > "pcs-handle" property semantics implementation seeing there can be at
> > least three different PCS attached:
> > DW XPCS
> > Lynx PCS
> > Renesas RZ/N1 MII
> > 
> > Any suggestion of how to distinguish the passed handle? Perhaps
> > named-property, phandle argument, by the compatible string or the
> > node-name?
> 

> I can't think of a reasonable solution to this at the moment. One
> solution could be pushing this down into the platform code to deal
> with as an interim solution, via the new .pcs_init() method.
> 
> We could also do that with the current XPCS code, since we know that
> only Intel mGBE uses xpcs. This would probably allow us to get rid
> of the has_xpcs flag.

Basically you suggest to move the entire stmmac_pcs_setup() to the
platforms, don't you? The patch 9 of this series indeed could have
been converted to just moving the entire PCS-detection loop from
stmmac_pcs_setup() to the Intel-specific pcs_init.

But IMO some default/generic code would be still useful to preserve in
the stmmac_pcs_setup() method. When it comes to the fwnode-based
platform we at least could be falling back to the default DW XPCS
device registration if no plat_stmmacenet_data::pcs_init() callback
was specified and there was the "pcs-handle" property found,
especially seeing DW *MAC and DW XPCS are of the same vendor.

Based on that I can convert patch 9 of this series to introducing the
pcs_init() callback in the Intel mGBE driver, but preserve the
semantics of the rest of the series changes.

-Serge(y)

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 10/10] net: stmmac: Add DW XPCS specified via "pcs-handle" support

[Russell King (Oracle)]

On Tue, Jun 04, 2024 at 12:04:57PM +0300, Serge Semin wrote:
> On Mon, Jun 03, 2024 at 10:03:54AM +0100, Russell King (Oracle) wrote:
> > I can't think of a reasonable solution to this at the moment. One
> > solution could be pushing this down into the platform code to deal
> > with as an interim solution, via the new .pcs_init() method.
> > 
> > We could also do that with the current XPCS code, since we know that
> > only Intel mGBE uses xpcs. This would probably allow us to get rid
> > of the has_xpcs flag.
> 
> Basically you suggest to move the entire stmmac_pcs_setup() to the
> platforms, don't you? The patch 9 of this series indeed could have
> been converted to just moving the entire PCS-detection loop from
> stmmac_pcs_setup() to the Intel-specific pcs_init.

Yes, it's not like XPCS is used by more than one platform, it's only
Intel mGBE. So I don't see why it should have a privileged position
over any other PCS implementation that stmmac supports (there's now
three different PCS.)

If you don't want the code in the Intel driver, then what could be
done is provide a core implementation that gets hooked into the
.pcs_init() method.

The same is probably true of other PCSes if they end up being shared
across several different platforms.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 10/10] net: stmmac: Add DW XPCS specified via "pcs-handle" support

[Serge Semin]

On Tue, Jun 04, 2024 at 10:29:40AM +0100, Russell King (Oracle) wrote:
> On Tue, Jun 04, 2024 at 12:04:57PM +0300, Serge Semin wrote:
> > On Mon, Jun 03, 2024 at 10:03:54AM +0100, Russell King (Oracle) wrote:
> > > I can't think of a reasonable solution to this at the moment. One
> > > solution could be pushing this down into the platform code to deal
> > > with as an interim solution, via the new .pcs_init() method.
> > > 
> > > We could also do that with the current XPCS code, since we know that
> > > only Intel mGBE uses xpcs. This would probably allow us to get rid
> > > of the has_xpcs flag.
> > 
> > Basically you suggest to move the entire stmmac_pcs_setup() to the
> > platforms, don't you? The patch 9 of this series indeed could have
> > been converted to just moving the entire PCS-detection loop from
> > stmmac_pcs_setup() to the Intel-specific pcs_init.
> 

> Yes, it's not like XPCS is used by more than one platform, it's only
> Intel mGBE. So I don't see why it should have a privileged position
> over any other PCS implementation that stmmac supports (there's now
> three different PCS.)
> 

Alas DW XPCS has already got a more privileged position. The STMMAC
driver calls the XPCS driver methods here and there (supported ifaces,
EEE or PHY setup). Unless these calls are converted to some
standard/new phylink_pcs calls IMO it would be better to preserve the
default DW XPCS init at least for the "pcs-handle" property to
motivate the platform drivers developers to follow some pre-defined
device description pattern (e.g. defining DW XPCS devices in device
tree), but leave the .pcs_init() for some platform-specific PCS inits
(including which have already been implemented).

As I already mentioned DW XPCS is of Synopsys vendor. The IP-core has
been invented to provide a bridge between the Synopsys MAC IP-cores
and PMA (mainly Synopsys PMAs) for the 1G/10G links like 1000Base-X,
and 10GBase-X/-R/-KX4/-KR. The reason we see just a single use-case
of the XPCS in the driver is that even though the STMMAC driver has DW
XGMAC support the driver is mainly utilized for the 1G MACs (I don't
see any platform currently having DW XGMAC defined). Since DW GMAC/QoS
Eth can be configured to have the standard PHY interfaces available
there is no need in XPCS in these cases (except a weird Intel mGBE).

But when it comes to DW XGMAC it can be synthesized with GMII and XGMII
interfaces only. These're exactly interfaces which DW XPCS supports on
upstream. Thus basically the DW XPCS IP-core has been mainly produced
for been utilized in a couple with DW XGMAC providing a ready-to-use
solution for the XFP/SFP(+) ports or backplane-based applications. So
should we have more DW XGMACs supported in the kernel we would have met
more DW XPCS defined in there too.

> If you don't want the code in the Intel driver, then what could be
> done is provide a core implementation that gets hooked into the
> .pcs_init() method.

I don't mind converting patch 9 to moving the XPCS registration in the
Intel-specific .pcs_init() (especially seeing it's just a single
xpcs_create_mdiodev() call), but having the "pcs-handle" property
handled generically in the STMMAC core would be a useful thing to have
(see my reasoning above).

-Serge(y)

> 
> The same is probably true of other PCSes if they end up being shared
> across several different platforms.
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel