[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6n4xvu6b43aptstdevdkzx2uqblwabaqndle2omqx5tcxk4lnz@wm3zqdrcr6m5>
Date: Wed, 5 Jun 2024 15:05:43 -0500
From: Andrew Halaney <ahalaney@...hat.com>
To: "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>
Cc: Serge Semin <fancer.lancer@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>, Jose Abreu <joabreu@...opsys.com>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>, netdev@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH RFC net-next v2 3/8] net: stmmac: dwmac1000: convert
sgmii/rgmii "pcs" to phylink
On Fri, May 31, 2024 at 12:26:25PM GMT, Russell King (Oracle) wrote:
> Convert dwmac1000 sgmii/rgmii "pcs" implementation to use a phylink_pcs
> so we can eventually get rid of the exceptional paths that conflict
> with phylink.
>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> ---
> .../ethernet/stmicro/stmmac/dwmac1000_core.c | 113 ++++++++++++------
> 1 file changed, 75 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> index d413d76a8936..4a0572d5f865 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> @@ -16,6 +16,7 @@
> #include <linux/slab.h>
> #include <linux/ethtool.h>
> #include <linux/io.h>
> +#include <linux/phylink.h>
> #include "stmmac.h"
> #include "stmmac_pcs.h"
> #include "dwmac1000.h"
> @@ -261,39 +262,6 @@ static void dwmac1000_pmt(struct mac_device_info *hw, unsigned long mode)
> writel(pmt, ioaddr + GMAC_PMT);
> }
>
> -/* RGMII or SMII interface */
> -static void dwmac1000_rgsmii(void __iomem *ioaddr, struct stmmac_extra_stats *x)
> -{
> - u32 status;
> -
> - status = readl(ioaddr + GMAC_RGSMIIIS);
> - x->irq_rgmii_n++;
> -
> - /* Check the link status */
> - if (status & GMAC_RGSMIIIS_LNKSTS) {
> - int speed_value;
> -
> - x->pcs_link = 1;
> -
> - speed_value = ((status & GMAC_RGSMIIIS_SPEED) >>
> - GMAC_RGSMIIIS_SPEED_SHIFT);
> - if (speed_value == GMAC_RGSMIIIS_SPEED_125)
> - x->pcs_speed = SPEED_1000;
> - else if (speed_value == GMAC_RGSMIIIS_SPEED_25)
> - x->pcs_speed = SPEED_100;
> - else
> - x->pcs_speed = SPEED_10;
> -
> - x->pcs_duplex = (status & GMAC_RGSMIIIS_LNKMOD_MASK);
> -
> - pr_info("Link is Up - %d/%s\n", (int)x->pcs_speed,
> - x->pcs_duplex ? "Full" : "Half");
> - } else {
> - x->pcs_link = 0;
> - pr_info("Link is Down\n");
> - }
> -}
> -
> static int dwmac1000_irq_status(struct mac_device_info *hw,
> struct stmmac_extra_stats *x)
> {
> @@ -335,8 +303,12 @@ static int dwmac1000_irq_status(struct mac_device_info *hw,
>
> dwmac_pcs_isr(ioaddr, GMAC_PCS_BASE, intr_status, x);
>
> - if (intr_status & PCS_RGSMIIIS_IRQ)
> - dwmac1000_rgsmii(ioaddr, x);
> + if (intr_status & PCS_RGSMIIIS_IRQ) {
> + /* TODO Dummy-read to clear the IRQ status */
> + readl(ioaddr + GMAC_RGSMIIIS);
This seems to me that you're doing the TODO here? Maybe I'm
misunderstanding... maybe not :)
> + phylink_pcs_change(&hw->mac_pcs, false);
> + x->irq_rgmii_n++;
> + }
>
> return ret;
> }
> @@ -404,9 +376,71 @@ static void dwmac1000_ctrl_ane(void __iomem *ioaddr, bool ane, bool srgmi_ral,
> dwmac_ctrl_ane(ioaddr, GMAC_PCS_BASE, ane, srgmi_ral, loopback);
> }
>
> -static void dwmac1000_get_adv_lp(void __iomem *ioaddr, struct rgmii_adv *adv)
> +static int dwmac1000_mii_pcs_validate(struct phylink_pcs *pcs,
> + unsigned long *supported,
> + const struct phylink_link_state *state)
> +{
> + /* Only support in-band */
> + if (!test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, state->advertising))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int dwmac1000_mii_pcs_config(struct phylink_pcs *pcs,
> + unsigned int neg_mode,
> + phy_interface_t interface,
> + const unsigned long *advertising,
> + bool permit_pause_to_mac)
> {
> - dwmac_get_adv_lp(ioaddr, GMAC_PCS_BASE, adv);
> + struct mac_device_info *hw = phylink_pcs_to_mac_dev_info(pcs);
> +
> + return dwmac_pcs_config(hw, neg_mode, interface, advertising,
> + GMAC_PCS_BASE);
> +}
> +
> +static void dwmac1000_mii_pcs_get_state(struct phylink_pcs *pcs,
> + struct phylink_link_state *state)
> +{
> + struct mac_device_info *hw = phylink_pcs_to_mac_dev_info(pcs);
> + unsigned int spd_clk;
> + u32 status;
> +
> + status = readl(hw->pcsr + GMAC_RGSMIIIS);
> +
> + state->link = status & GMAC_RGSMIIIS_LNKSTS;
> + if (!state->link)
> + return;
> +
> + spd_clk = FIELD_GET(GMAC_RGSMIIIS_SPEED, status);
> + if (spd_clk == GMAC_RGSMIIIS_SPEED_125)
> + state->speed = SPEED_1000;
> + else if (spd_clk == GMAC_RGSMIIIS_SPEED_25)
> + state->speed = SPEED_100;
> + else if (spd_clk == GMAC_RGSMIIIS_SPEED_2_5)
> + state->speed = SPEED_10;
> +
> + state->duplex = status & GMAC_RGSMIIIS_LNKMOD_MASK ?
> + DUPLEX_FULL : DUPLEX_HALF;
> +
> + dwmac_pcs_get_state(hw, state, GMAC_PCS_BASE);
> +}
> +
> +static const struct phylink_pcs_ops dwmac1000_mii_pcs_ops = {
> + .pcs_validate = dwmac1000_mii_pcs_validate,
> + .pcs_config = dwmac1000_mii_pcs_config,
> + .pcs_get_state = dwmac1000_mii_pcs_get_state,
> +};
> +
> +static struct phylink_pcs *
> +dwmac1000_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;
> }
>
> static void dwmac1000_debug(struct stmmac_priv *priv, void __iomem *ioaddr,
> @@ -499,6 +533,7 @@ static void dwmac1000_set_mac_loopback(void __iomem *ioaddr, bool enable)
>
> const struct stmmac_ops dwmac1000_ops = {
> .core_init = dwmac1000_core_init,
> + .phylink_select_pcs = dwmac1000_phylink_select_pcs,
> .set_mac = stmmac_set_mac,
> .rx_ipc = dwmac1000_rx_ipc_enable,
> .dump_regs = dwmac1000_dump_regs,
> @@ -514,7 +549,6 @@ const struct stmmac_ops dwmac1000_ops = {
> .set_eee_pls = dwmac1000_set_eee_pls,
> .debug = dwmac1000_debug,
> .pcs_ctrl_ane = dwmac1000_ctrl_ane,
> - .pcs_get_adv_lp = dwmac1000_get_adv_lp,
> .set_mac_loopback = dwmac1000_set_mac_loopback,
> };
>
> @@ -549,5 +583,8 @@ int dwmac1000_setup(struct stmmac_priv *priv)
> mac->mii.clk_csr_shift = 2;
> mac->mii.clk_csr_mask = GENMASK(5, 2);
>
> + mac->mac_pcs.ops = &dwmac1000_mii_pcs_ops;
> + mac->mac_pcs.neg_mode = true;
> +
> return 0;
> }
> --
> 2.30.2
>
Powered by blists - more mailing lists