| Message ID | 20240602143636.5839-1-fancer.lancer@gmail.com |
|---|---|
| Headers | show |
| Series | net: pcs: xpcs: Add memory-mapped device support | expand |
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 >
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.
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!
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.
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!
The main goal of this series is to extend the DW XPCS device support in the kernel. Particularly the patchset adds a support of the DW XPCS device with the MCI/APB3 IO interface registered as a platform device. In order to have them utilized by the DW XPCS core the fwnode-based DW XPCS descriptor creation procedure has been introduced. Finally the STMMAC driver has been altered to support the DW XPCS passed via the 'pcs-handle' property. Note the series has been significantly re-developed since v1. So I even had to change the subject. Anyway I've done my best to take all the noted into account. The series traditionally starts with a set of the preparation patches. First one just moves the generic DW XPCS IDs macros from the internal header file to the external one where some other IDs also reside. Second patch splits up the xpcs_create() method to a set of the coherent sub-functions for the sake of the further easier updates and to have it looking less complicated. The goal of the next three patches is to extend the DW XPCS ID management code by defining a new dw_xpcs_info structure with both PCS and PMA IDs. The next two patches provide the DW XPCS device DT-bindings and the respective platform-device driver for the memory-mapped DW XPCS devices. Besides the later patch makes use of the introduced dw_xpcs_info structure to pre-define the DW XPCS IDs based on the platform-device compatible string. Thus if there is no way to auto-identify the XPCS device capabilities it can be done based on the custom device IDs passed via the MDIO-device platform data. Final DW XPCS driver change is about adding a new method of the DW XPCS descriptor creation. The xpcs_create_fwnode() function has been introduced with the same semantics as a similar method recently added to the Lynx PCS driver. It's supposed to be called with the fwnode pointing to the DW XPCS device node, for which the XPCS descriptor will be created. The series is terminated with two STMMAC driver patches. The former one simplifies the DW XPCS descriptor registration procedure by dropping the MDIO-bus scanning and creating the descriptor for the particular device address. The later patch alters the STMMAC PCS setup method so one would support the DW XPCS specified via the "pcs-handle" property. That's it for now. Thanks for review in advance. Any tests are very welcome. After this series is merged in, I'll submit another one which adds the generic 10GBase-R and 10GBase-X interfaces support to the STMMAC and DW XPCS driver with the proper CSRs re-initialization, PMA initialization and reference clock selection as it's described in the Synopsys DW XPCS HW manual. Link: https://lore.kernel.org/netdev/20231205103559.9605-1-fancer.lancer@gmail.com Changelog v2: - Drop the patches: [PATCH net-next 01/16] net: pcs: xpcs: Drop sentinel entry from 2500basex ifaces list [PATCH net-next 02/16] net: pcs: xpcs: Drop redundant workqueue.h include directive [PATCH net-next 03/16] net: pcs: xpcs: Return EINVAL in the internal methods [PATCH net-next 04/16] net: pcs: xpcs: Explicitly return error on caps validation as ones have already been merged into the kernel repo: Link: https://lore.kernel.org/netdev/20240222175843.26919-1-fancer.lancer@gmail.com/ - Drop the patches: [PATCH net-next 14/16] net: stmmac: Pass netdev to XPCS setup function [PATCH net-next 15/16] net: stmmac: Add dedicated XPCS cleanup method as ones have already been merged into the kernel repo: Link: https://lore.kernel.org/netdev/20240513-rzn1-gmac1-v7-0-6acf58b5440d@bootlin.com/ - Drop the patch: [PATCH net-next 06/16] net: pcs: xpcs: Avoid creating dummy XPCS MDIO device [PATCH net-next 09/16] net: mdio: Add Synopsys DW XPCS management interface support [PATCH net-next 11/16] net: pcs: xpcs: Change xpcs_create_mdiodev() suffix to "byaddr" [PATCH net-next 13/16] net: stmmac: intel: Register generic MDIO device as no longer relevant. - Add new patches: [PATCH net-next v2 03/10] net: pcs: xpcs: Convert xpcs_id to dw_xpcs_desc [PATCH net-next v2 04/10] net: pcs: xpcs: Convert xpcs_compat to dw_xpcs_compat [PATCH net-next v2 05/10] net: pcs: xpcs: Introduce DW XPCS info structure [PATCH net-next v2 09/10] net: stmmac: Create DW XPCS device with particular address - Use the xpcs_create_fwnode() function name and semantics similar to the Lynx PCS driver. - Add kdoc describing the DW XPCS registration functions. - Convert the memory-mapped DW XPCS device driver to being the platform-device driver. - Convert the DW XPCS DT-bindings to defining both memory-mapped and MDIO devices. - Drop inline'es from the methods statically defined in *.c. (@Maxime) - Preserve the strict refcount-ing pattern. (@Russell) Signed-off-by: Serge Semin <fancer.lancer@gmail.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: Eric Dumazet <edumazet@google.com> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Paolo Abeni <pabeni@redhat.com> Cc: Sagar Cheluvegowda <quic_scheluve@quicinc.com> Cc: Abhishek Chauhan <quic_abchauha@quicinc.com> Cc: Andrew Halaney <ahalaney@redhat.com> Cc: Jiawen Wu <jiawenwu@trustnetic.com> Cc: Mengyuan Lou <mengyuanlou@net-swift.com> Cc: Tomer Maimon <tmaimon77@gmail.com> Cc: openbmc@lists.ozlabs.org Cc: netdev@vger.kernel.org Cc: devicetree@vger.kernel.org Cc: linux-kernel@vger.kernel.org Serge Semin (10): net: pcs: xpcs: Move native device ID macro to linux/pcs/pcs-xpcs.h net: pcs: xpcs: Split up xpcs_create() body to sub-functions net: pcs: xpcs: Convert xpcs_id to dw_xpcs_desc net: pcs: xpcs: Convert xpcs_compat to dw_xpcs_compat net: pcs: xpcs: Introduce DW XPCS info structure dt-bindings: net: Add Synopsys DW xPCS bindings net: pcs: xpcs: Add Synopsys DW xPCS platform device driver net: pcs: xpcs: Add fwnode-based descriptor creation method net: stmmac: Create DW XPCS device with particular address net: stmmac: Add DW XPCS specified via "pcs-handle" support .../bindings/net/pcs/snps,dw-xpcs.yaml | 133 +++++ .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 1 + .../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 28 +- drivers/net/pcs/Kconfig | 6 +- drivers/net/pcs/Makefile | 3 +- drivers/net/pcs/pcs-xpcs-plat.c | 460 ++++++++++++++++++ drivers/net/pcs/pcs-xpcs.c | 361 +++++++++----- drivers/net/pcs/pcs-xpcs.h | 7 +- include/linux/pcs/pcs-xpcs.h | 49 +- include/linux/stmmac.h | 1 + 10 files changed, 905 insertions(+), 144 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml create mode 100644 drivers/net/pcs/pcs-xpcs-plat.c