[PATCH net-next 0/2] net: stmmac: Add 2500BASEX support for integrated PCS

[PATCH net-next 0/2] net: stmmac: Add 2500BASEX support for integrated PCS

[Sneh Shah]

Qcom mac supports both SGMII and 2500BASEX with integrated PCS.
Add changes to enable 2500BASEX along woth SGMII.

Sneh Shah (2):
  net: stmmac: Add support for multiple phy interface for integrated PCS
  net: stmmac: dwmac-qcom-ethqos: Enable support for 2500BASEX

 .../net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c   | 11 +++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     |  3 +++
 include/linux/stmmac.h                                |  1 +
 3 files changed, 15 insertions(+)

-- 
2.17.1


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

[PATCH net-next 1/2] net: stmmac: Add support for multiple phy interface for integrated PCS

[Sneh Shah]

In case of integrated PCS ethernet does't have external PCS driver.
Currently stmmac supports multiple phy interfaces if there is external PCS.
Add a function to support mupliple phy interfaces when PCS is integrated.

Signed-off-by: Sneh Shah <quic_snehshah@quicinc.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
 include/linux/stmmac.h                            | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index fe3498e86de9..765332627ad0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1231,6 +1231,9 @@ static int stmmac_phy_setup(struct stmmac_priv *priv)
 	 */
 	__set_bit(mode, priv->phylink_config.supported_interfaces);
 
+	if (priv->plat->get_interfaces)
+		priv->plat->get_interfaces(priv);
+
 	/* If we have an xpcs, it defines which PHY interfaces are supported. */
 	if (priv->hw->xpcs)
 		xpcs_get_interfaces(priv->hw->xpcs,
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index dfa1828cd756..66f8205b331e 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -278,6 +278,7 @@ struct plat_stmmacenet_data {
 	void (*serdes_powerdown)(struct net_device *ndev, void *priv);
 	void (*speed_mode_2500)(struct net_device *ndev, void *priv);
 	void (*ptp_clk_freq_config)(struct stmmac_priv *priv);
+	void (*get_interfaces)(struct stmmac_priv *priv);
 	int (*init)(struct platform_device *pdev, void *priv);
 	void (*exit)(struct platform_device *pdev, void *priv);
 	struct mac_device_info *(*setup)(void *priv);
-- 
2.17.1


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

[PATCH net-next 2/2] net: stmmac: dwmac-qcom-ethqos: Enable support for 2500BASEX

[Sneh Shah]

With integrated PCS qcom mac supports both SGMII and 2500BASEX mode.
Implement get_interfaces to add support for 2500BASEX.

Signed-off-by: Sneh Shah <quic_snehshah@quicinc.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c   | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index e254b21fdb59..dad6e2448475 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -731,6 +731,13 @@ static void ethqos_clks_disable(void *data)
 	ethqos_clks_config(data, false);
 }
 
+static void ethqos_get_interfaces(struct stmmac_priv *priv)
+{
+	if (priv->plat->phy_interface == PHY_INTERFACE_MODE_SGMII)
+		__set_bit(PHY_INTERFACE_MODE_2500BASEX,
+			  priv->phylink_config.supported_interfaces);
+};
+
 static void ethqos_ptp_clk_freq_config(struct stmmac_priv *priv)
 {
 	struct plat_stmmacenet_data *plat_dat = priv->plat;
@@ -786,6 +793,7 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 		ethqos->configure_func = ethqos_configure_rgmii;
 		break;
 	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_2500BASEX:
 		ethqos->configure_func = ethqos_configure_sgmii;
 		break;
 	default:
@@ -851,6 +859,9 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 		plat_dat->serdes_powerdown  = qcom_ethqos_serdes_powerdown;
 	}
 
+	if (plat_dat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS)
+		plat_dat->get_interfaces = ethqos_get_interfaces;
+
 	/* Enable TSO on queue0 and enable TBS on rest of the queues */
 	for (i = 1; i < plat_dat->tx_queues_to_use; i++)
 		plat_dat->tx_queues_cfg[i].tbs_en = 1;
-- 
2.17.1


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

Re: [PATCH net-next 1/2] net: stmmac: Add support for multiple phy interface for integrated PCS

[Andrew Lunn]

On Fri, May 24, 2024 at 06:36:52PM +0530, Sneh Shah wrote:
> In case of integrated PCS ethernet does't have external PCS driver.
> Currently stmmac supports multiple phy interfaces if there is external PCS.
> Add a function to support mupliple phy interfaces when PCS is integrated.

s/mupliple/multiple.

	Andrew

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

Re: [PATCH net-next 2/2] net: stmmac: dwmac-qcom-ethqos: Enable support for 2500BASEX

[Andrew Lunn]

On Fri, May 24, 2024 at 06:36:53PM +0530, Sneh Shah wrote:
> With integrated PCS qcom mac supports both SGMII and 2500BASEX mode.
> Implement get_interfaces to add support for 2500BASEX.

I don't know this driver very well..... but

/* PCS defines */
#define STMMAC_PCS_RGMII        (1 << 0)
#define STMMAC_PCS_SGMII        (1 << 1)
#define STMMAC_PCS_TBI          (1 << 2)
#define STMMAC_PCS_RTBI         (1 << 3)


static int stmmac_ethtool_get_link_ksettings(struct net_device *dev,
                                             struct ethtool_link_ksettings *cmd)
{
        struct stmmac_priv *priv = netdev_priv(dev);

        if (!(priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS) &&
            (priv->hw->pcs & STMMAC_PCS_RGMII ||
             priv->hw->pcs & STMMAC_PCS_SGMII)) {
                struct rgmii_adv adv;
                u32 supported, advertising, lp_advertising;

                if (!priv->xstats.pcs_link) {
                        cmd->base.speed = SPEED_UNKNOWN;
                        cmd->base.duplex = DUPLEX_UNKNOWN;
                        return 0;
                }

/**
 * stmmac_check_pcs_mode - verify if RGMII/SGMII is supported
 * @priv: driver private structure
 * Description: this is to verify if the HW supports the PCS.
 * Physical Coding Sublayer (PCS) interface that can be used when the MAC is
 * configured for the TBI, RTBI, or SGMII PHY interface.
 */
static void stmmac_check_pcs_mode(struct stmmac_priv *priv)
{
        int interface = priv->plat->mac_interface;

        if (priv->dma_cap.pcs) {
                if ((interface == PHY_INTERFACE_MODE_RGMII) ||
                    (interface == PHY_INTERFACE_MODE_RGMII_ID) ||
                    (interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
                    (interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
                        netdev_dbg(priv->dev, "PCS RGMII support enabled\n");
                        priv->hw->pcs = STMMAC_PCS_RGMII;
                } else if (interface == PHY_INTERFACE_MODE_SGMII) {
                        netdev_dbg(priv->dev, "PCS SGMII support enabled\n");
                        priv->hw->pcs = STMMAC_PCS_SGMII;
                }
        }
}

I get the feeling this is a minimal hack, rather than a proper
solution.

    Andrew

---
pw-bot: cr

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

Re: [PATCH net-next 2/2] net: stmmac: dwmac-qcom-ethqos: Enable support for 2500BASEX

[Russell King (Oracle)]

On Fri, May 24, 2024 at 06:07:15PM +0200, Andrew Lunn wrote:
> On Fri, May 24, 2024 at 06:36:53PM +0530, Sneh Shah wrote:
> > With integrated PCS qcom mac supports both SGMII and 2500BASEX mode.
> > Implement get_interfaces to add support for 2500BASEX.
> 
> I don't know this driver very well..... but
> 
> /* PCS defines */
> #define STMMAC_PCS_RGMII        (1 << 0)
> #define STMMAC_PCS_SGMII        (1 << 1)
> #define STMMAC_PCS_TBI          (1 << 2)
> #define STMMAC_PCS_RTBI         (1 << 3)
> 
> 
> static int stmmac_ethtool_get_link_ksettings(struct net_device *dev,
>                                              struct ethtool_link_ksettings *cmd)
> {
>         struct stmmac_priv *priv = netdev_priv(dev);
> 
>         if (!(priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS) &&
>             (priv->hw->pcs & STMMAC_PCS_RGMII ||
>              priv->hw->pcs & STMMAC_PCS_SGMII)) {
>                 struct rgmii_adv adv;
>                 u32 supported, advertising, lp_advertising;
> 
>                 if (!priv->xstats.pcs_link) {
>                         cmd->base.speed = SPEED_UNKNOWN;
>                         cmd->base.duplex = DUPLEX_UNKNOWN;
>                         return 0;
>                 }

Note that this checks for !STMMAC_FLAG_HAS_INTEGRATED_PCS, so this isn't
going to be used by this code which is conditional on this flag being
set.

In any case, I posted a patch set 12 days ago, which has remained
unreviewed and untested for 10 days from a promise to do so converting
this ugly hack to a phylink PCS driver (not that I would have had time
to deal with any feedback due to an urgent work issue, but that's sort
of beside the point.) There's some vague handwaving by Serge that there
are some issues in this series, but there hasn't been any feedback yet
on what these issues may be.

Also, from what I can tell, neither STMMAC_PCS_TBI nor STMMAC_PCS_RTBI
are ever assigned to hw->pcs. So, in stmmac_eee_init():

        if (priv->hw->pcs == STMMAC_PCS_TBI ||
            priv->hw->pcs == STMMAC_PCS_RTBI)
                return false;

is always false, and can be removed.

In __stmmac_open():

        if (priv->hw->pcs != STMMAC_PCS_TBI &&
            priv->hw->pcs != STMMAC_PCS_RTBI &&
            (!priv->hw->xpcs ||
             xpcs_get_an_mode(priv->hw->xpcs, mode) != DW_AN_C73)) {

can become:
	if (!priv->hw->xpcs ||
	    xpcs_get_an_mode(priv->hw->xpcs, mode) != DW_AN_C73)) {

In stmmac_dvr_probe():

        if (priv->hw->pcs != STMMAC_PCS_TBI &&
            priv->hw->pcs != STMMAC_PCS_RTBI) {
                /* MDIO bus Registration */
                ret = stmmac_mdio_register(ndev);

This if() condition can be eliminated, and we always register the
MDIO, and similarly in the cleanup and stmmac_dvr_remove() paths.

> /**
>  * stmmac_check_pcs_mode - verify if RGMII/SGMII is supported
>  * @priv: driver private structure
>  * Description: this is to verify if the HW supports the PCS.
>  * Physical Coding Sublayer (PCS) interface that can be used when the MAC is
>  * configured for the TBI, RTBI, or SGMII PHY interface.
>  */
> static void stmmac_check_pcs_mode(struct stmmac_priv *priv)
> {
>         int interface = priv->plat->mac_interface;
> 
>         if (priv->dma_cap.pcs) {
>                 if ((interface == PHY_INTERFACE_MODE_RGMII) ||
>                     (interface == PHY_INTERFACE_MODE_RGMII_ID) ||
>                     (interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
>                     (interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
>                         netdev_dbg(priv->dev, "PCS RGMII support enabled\n");
>                         priv->hw->pcs = STMMAC_PCS_RGMII;
>                 } else if (interface == PHY_INTERFACE_MODE_SGMII) {
>                         netdev_dbg(priv->dev, "PCS SGMII support enabled\n");
>                         priv->hw->pcs = STMMAC_PCS_SGMII;
>                 }
>         }
> }
> 
> I get the feeling this is a minimal hack, rather than a proper
> solution.

I didn't remove that in my patch set because I don't understand fully
the logic here - and I didn't want to add further dubious complication
to my six patch series. I actually kept the logic and continued to
use it explicitly to avoid changing the decision making:

+static struct phylink_pcs *
+dwmac4_phylink_select_pcs(struct stmmac_priv *priv, phy_interface_t interface)
+{
+       if (priv->hw->pcs & STMMAC_PCS_RGMII ||
+           priv->hw->pcs & STMMAC_PCS_SGMII)
+               return &priv->hw->mac_pcs;
+
+       return NULL;
+}

Ultimately, this _should_ check the "interface" here, but bear in
mind that stmmac_check_pcs_mode() checks plat->mac_interface
(which is the interface between the MAC and PCS) whereas the
"interface" passed here is the interface between the PCS and PHY.
This is why removing stmmac_check_pcs_mode() isn't a sane change
to make until we have worked through the issues with removing the
its-a-PCS-but-not-phylink_pcs hack.

I _think_ a sensible next step would be to eliminate priv->hw->pcs,
instead testing priv->plat->mac_interface in
dwmac4_phylink_select_pcs() and dwmac1000_phylink_select_pcs():

	phy_interface_t mac_interface = priv->plat->mac_interface;

	if (phy_interface_is_rgmii(mac_interface) ||
	    mac_interface == PHY_INTERFACE_MODE_SGMII)
		return &priv->hw->mac_pcs;

and _then_ maybe as a separate patch switch this to test the
PHY-side interface (because that would make more sense... but
that would be a behavioural change to the driver.)

-- 
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 2/2] net: stmmac: dwmac-qcom-ethqos: Enable support for 2500BASEX

[Russell King (Oracle)]

On Fri, May 24, 2024 at 06:13:17PM +0100, Russell King (Oracle) wrote:
> Note that this checks for !STMMAC_FLAG_HAS_INTEGRATED_PCS, so this isn't
> going to be used by this code which is conditional on this flag being
> set.
> 
> In any case, I posted a patch set 12 days ago, which has remained

Oops, forgot the link to the patch series:

https://lore.kernel.org/r/ZkDuJAx7atDXjf5m@shell.armlinux.org.uk

-- 
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