lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ