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  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]
Date:   Tue, 4 Jun 2019 13:46:00 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Voon Weifeng <weifeng.voon@...el.com>,
        "David S. Miller" <davem@...emloft.net>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Russell King <linux@...linux.org.uk>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jose Abreu <joabreu@...opsys.com>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Andrew Lunn <andrew@...n.ch>,
        Alexandre Torgue <alexandre.torgue@...com>,
        biao huang <biao.huang@...iatek.com>,
        Ong Boon Leong <boon.leong.ong@...el.com>,
        Kweh Hock Leong <hock.leong.kweh@...el.com>
Subject: Re: [PATCH net-next v6 2/5] net: stmmac: introducing support for DWC
 xPCS logics

+Russell,

On 6/4/2019 11:58 AM, Voon Weifeng wrote:
> From: Ong Boon Leong <boon.leong.ong@...el.com>
> 
> xPCS is DWC Ethernet Physical Coding Sublayer that may be integrated
> into a GbE controller that uses DWC EQoS MAC controller. An example of
> HW configuration is shown below:-
> 
>   <-----------------GBE Controller---------->|<--External PHY chip-->
> 
>   +----------+         +----+    +---+               +--------------+
>   |   EQoS   | <-GMII->| DW |<-->|PHY| <-- SGMII --> | External GbE |
>   |   MAC    |         |xPCS|    |IF |               | PHY Chip     |
>   +----------+         +----+    +---+               +--------------+
>          ^               ^                                  ^
>          |               |                                  |
>          +---------------------MDIO-------------------------+
> 
> xPCS is a Clause-45 MDIO Manageable Device (MMD) and we need a way to
> differentiate it from external PHY chip that is discovered over MDIO.
> Therefore, xpcs_phy_addr is introduced in stmmac platform data
> (plat_stmmacenet_data) for differentiating xPCS from 'phy_addr' that
> belongs to external PHY.

Assuming this DW xPCS can be found with designs other than STMMAC would
not it make sense to model this as some kind of PHY/MDIO bridge? A
little bit like what drivers/net/phy/xilinx_gmii2rgmii.c tries to do?

> 
> Basic functionalities for initializing xPCS and configuring auto
> negotiation (AN), loopback, link status, AN advertisement and Link
> Partner ability are implemented. The implementation supports the C37
> AN for 1000BASE-X and SGMII (MAC side SGMII only).
> 
> Tested-by: Tan, Tee Min <tee.min.tan@...el.com>
> Reviewed-by: Voon Weifeng <weifeng.voon@...el.com>
> Reviewed-by: Kweh Hock Leong <hock.leong.kweh@...el.com>
> Signed-off-by: Ong Boon Leong <boon.leong.ong@...el.com>
> Signed-off-by: Voon Weifeng <weifeng.voon@...el.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/Makefile |   2 +-
>  drivers/net/ethernet/stmicro/stmmac/common.h |   1 +
>  drivers/net/ethernet/stmicro/stmmac/dwxpcs.c | 208 +++++++++++++++++++++++++++
>  drivers/net/ethernet/stmicro/stmmac/dwxpcs.h |  51 +++++++
>  drivers/net/ethernet/stmicro/stmmac/hwif.h   |  19 +++
>  include/linux/stmmac.h                       |   1 +
>  6 files changed, 281 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwxpcs.c
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwxpcs.h
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index c59926d96bcc..f007fb873455 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -6,7 +6,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o	\
>  	      mmc_core.o stmmac_hwtstamp.o stmmac_ptp.o dwmac4_descs.o	\
>  	      dwmac4_dma.o dwmac4_lib.o dwmac4_core.o dwmac5.o hwif.o \
>  	      stmmac_tc.o dwxgmac2_core.o dwxgmac2_dma.o dwxgmac2_descs.o \
> -	      $(stmmac-y)
> +	      dwxpcs.o $(stmmac-y)
>  
>  stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o
>  
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index 1961fe9144ca..83df093c4636 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -419,6 +419,7 @@ struct mii_regs {
>  
>  struct mac_device_info {
>  	const struct stmmac_ops *mac;
> +	const struct stmmac_xpcs_ops *xpcs;
>  	const struct stmmac_desc_ops *desc;
>  	const struct stmmac_dma_ops *dma;
>  	const struct stmmac_mode_ops *mode;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxpcs.c b/drivers/net/ethernet/stmicro/stmmac/dwxpcs.c
> new file mode 100644
> index 000000000000..7e850b9dd7b7
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxpcs.c
> @@ -0,0 +1,208 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2019, Intel Corporation.
> + * DWC Ethernet Physical Coding Sublayer
> + */
> +#include <linux/bitops.h>
> +#include <linux/mdio.h>
> +#include "dwxpcs.h"
> +#include "stmmac.h"
> +
> +/* DW xPCS mdiobus_read and mdiobus_write helper functions */
> +#define xpcs_read(dev, reg) \
> +	mdiobus_read(priv->mii, xpcs_phy_addr, \
> +		     MII_ADDR_C45 | (reg) | \
> +		     ((dev) << MII_DEVADDR_C45_SHIFT))
> +#define xpcs_write(dev, reg, val) \
> +	mdiobus_write(priv->mii, xpcs_phy_addr, \
> +		      MII_ADDR_C45 | (reg) | \
> +		      ((dev) << MII_DEVADDR_C45_SHIFT), val)
> +
> +static void dw_xpcs_init(struct net_device *ndev, int pcs_mode)
> +{
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	int xpcs_phy_addr;
> +	int phydata;
> +
> +	xpcs_phy_addr = priv->plat->xpcs_phy_addr;
> +
> +	if (pcs_mode == AN_CTRL_PCS_MD_C37_SGMII) {
> +		/* For AN for SGMII mode, the settings are :-
> +		 * 1) VR_MII_AN_CTRL Bit(2:1)[PCS_MODE] = 10b (SGMII AN)
> +		 * 2) VR_MII_AN_CTRL Bit(3) [TX_CONFIG] = 0b (MAC side SGMII)
> +		 *    DW xPCS used with DW EQoS MAC is always MAC
> +		 *    side SGMII.
> +		 * 3) VR_MII_AN_CTRL Bit(0) [AN_INTR_EN] = 1b (AN Interrupt
> +		 *    enabled)
> +		 * 4) VR_MII_DIG_CTRL1 Bit(9) [MAC_AUTO_SW] = 1b (Automatic
> +		 *    speed mode change after SGMII AN complete)
> +		 * Note: Since it is MAC side SGMII, there is no need to set
> +		 *	 SR_MII_AN_ADV. MAC side SGMII receives AN Tx Config
> +		 *	 from PHY about the link state change after C28 AN
> +		 *	 is completed between PHY and Link Partner.
> +		 */
> +		phydata = xpcs_read(XPCS_MDIO_MII_MMD, MDIO_MII_MMD_AN_CTRL);
> +		phydata &= ~MDIO_MII_MMD_AN_CTRL_PCS_MD;
> +		phydata |= MDIO_MII_MMD_AN_CTRL_AN_INTR_EN |
> +			   (AN_CTRL_PCS_MD_C37_SGMII <<
> +			    MDIO_MII_MMD_AN_CTRL_PCS_MD_SHIFT &
> +			    MDIO_MII_MMD_AN_CTRL_PCS_MD) |
> +			   (AN_CTRL_TX_CONF_MAC_SIDE_SGMII <<
> +			    MDIO_MII_MMD_AN_CTRL_TX_CONFIG_SHIFT);
> +		xpcs_write(XPCS_MDIO_MII_MMD, MDIO_MII_MMD_AN_CTRL, phydata);
> +
> +		phydata = xpcs_read(XPCS_MDIO_MII_MMD,
> +				    MDIO_MII_MMD_DIGITAL_CTRL_1);
> +		phydata |= MDIO_MII_MMD_DIGI_CTRL_1_MAC_AUTO_SW;
> +		xpcs_write(XPCS_MDIO_MII_MMD, MDIO_MII_MMD_DIGITAL_CTRL_1,
> +			   phydata);
> +	} else {
> +		/* For AN for 1000BASE-X mode, the settings are :-
> +		 * 1) VR_MII_AN_CTRL Bit(2:1)[PCS_MODE] = 00b (1000BASE-X C37)
> +		 * 2) VR_MII_AN_CTRL Bit(0) [AN_INTR_EN] = 1b (AN Interrupt
> +		 *    enabled)
> +		 * 3) SR_MII_AN_ADV Bit(6)[FD] = 1b (Full Duplex)
> +		 *    Note: Half Duplex is rarely used, so don't advertise.
> +		 * 4) SR_MII_AN_ADV Bit(8:7)[PSE] = 11b (Sym & Asym Pause)
> +		 */
> +		phydata = xpcs_read(XPCS_MDIO_MII_MMD, MDIO_MII_MMD_AN_CTRL);
> +		phydata &= ~MDIO_MII_MMD_AN_CTRL_PCS_MD;
> +		phydata |= MDIO_MII_MMD_AN_CTRL_AN_INTR_EN |
> +			   (AN_CTRL_PCS_MD_C37_1000BASEX <<
> +			    MDIO_MII_MMD_AN_CTRL_PCS_MD_SHIFT &
> +			    MDIO_MII_MMD_AN_CTRL_PCS_MD);
> +		xpcs_write(XPCS_MDIO_MII_MMD, MDIO_MII_MMD_AN_CTRL, phydata);
> +
> +		phydata = xpcs_read(XPCS_MDIO_MII_MMD, MII_ADVERTISE);
> +		phydata |= MDIO_MII_MMD_FD |
> +			   (MDIO_MII_MMD_PSE_BOTH << MDIO_MII_MMD_PSE_SHIFT);
> +		xpcs_write(XPCS_MDIO_MII_MMD, MII_ADVERTISE, phydata);
> +	}
> +}
> +
> +static void dw_xpcs_ctrl_ane(struct net_device *ndev, bool ane,
> +			     bool loopback)
> +{
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	int xpcs_phy_addr;
> +	int phydata;
> +
> +	xpcs_phy_addr = priv->plat->xpcs_phy_addr;
> +	phydata = xpcs_read(XPCS_MDIO_MII_MMD, MII_BMCR);
> +
> +	if (ane)
> +		phydata |= (BMCR_ANENABLE | BMCR_ANRESTART);
> +
> +	if (loopback)
> +		phydata |= BMCR_LOOPBACK;
> +
> +	xpcs_write(XPCS_MDIO_MII_MMD, MII_BMCR, phydata);
> +}
> +
> +static void dw_xpcs_get_adv_lp(struct net_device *ndev,
> +			       struct rgmii_adv *adv_lp,
> +			       int pcs_mode)
> +{
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	int xpcs_phy_addr;
> +	int value;
> +
> +	xpcs_phy_addr = priv->plat->xpcs_phy_addr;
> +
> +	/* AN Advertisement Ability */
> +	value = xpcs_read(XPCS_MDIO_MII_MMD, MII_ADVERTISE);
> +
> +	if (value & MDIO_MII_MMD_FD)
> +		adv_lp->duplex = DUPLEX_FULL;
> +	if (value & MDIO_MII_MMD_HD)
> +		adv_lp->duplex = DUPLEX_HALF;
> +	adv_lp->pause = (u32)((value & MDIO_MII_MMD_PSE) >>
> +			      MDIO_MII_MMD_PSE_SHIFT);
> +
> +	/* Link Partner Ability - 1000BASE-X only*/
> +	if (pcs_mode == AN_CTRL_PCS_MD_C37_1000BASEX) {
> +		value = xpcs_read(XPCS_MDIO_MII_MMD, MII_LPA);
> +		if (value & MDIO_MII_MMD_FD)
> +			adv_lp->lp_duplex = DUPLEX_FULL;
> +		if (value & MDIO_MII_MMD_HD)
> +			adv_lp->lp_duplex = DUPLEX_HALF;
> +		adv_lp->lp_pause = (u32)((value & MDIO_MII_MMD_PSE) >>
> +					 MDIO_MII_MMD_PSE_SHIFT);
> +	}
> +}
> +
> +static void dw_xpcs_get_linkstatus(struct net_device *ndev,
> +				   u16 an_stat,
> +				   struct stmmac_extra_stats *x)
> +{
> +	/* Check the SGMII AN link status */
> +	if (an_stat & AN_STAT_SGMII_AN_LNKSTS) {
> +		int speed_value;
> +
> +		x->pcs_link = 1;
> +
> +		speed_value = ((an_stat & AN_STAT_SGMII_AN_SPEED) >>
> +				AN_STAT_SGMII_AN_SPEED_SHIFT);
> +		if (speed_value == AN_STAT_SGMII_AN_1000MBPS)
> +			x->pcs_speed = SPEED_1000;
> +		else if (speed_value == AN_STAT_SGMII_AN_100MBPS)
> +			x->pcs_speed = SPEED_100;
> +		else
> +			x->pcs_speed = SPEED_10;
> +
> +		if (an_stat & AN_STAT_SGMII_AN_FD)
> +			x->pcs_duplex = 1;
> +		else
> +			x->pcs_duplex = 0;
> +
> +		netdev_info(ndev, "Link is Up - %d/%s\n", (int)x->pcs_speed,
> +			    x->pcs_duplex ? "Full" : "Half");
> +	} else {
> +		x->pcs_link = 0;
> +		netdev_info(ndev, "Link is Down\n");
> +	}
> +}
> +
> +static int dw_xpcs_irq_status(struct net_device *ndev,
> +			      struct stmmac_extra_stats *x,
> +			      int pcs_mode)
> +{
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	int ret = IRQ_NONE;
> +	int xpcs_phy_addr;
> +	int an_stat;
> +
> +	xpcs_phy_addr = priv->plat->xpcs_phy_addr;
> +
> +	/* AN status */
> +	an_stat = xpcs_read(XPCS_MDIO_MII_MMD, MDIO_MII_MMD_AN_STAT);
> +
> +	if (an_stat & AN_STAT_SGMII_AN_CMPLT) {
> +		x->irq_pcs_ane_n++;
> +
> +		if (pcs_mode == AN_CTRL_PCS_MD_C37_SGMII) {
> +			dw_xpcs_get_linkstatus(ndev, an_stat, x);
> +		} else {
> +			/* For 1000BASE-X AN, DW xPCS does not have register
> +			 * to read the link state of 1000BASE-X C37 AN and
> +			 * since 1000BASE-X is always 1000Mbps and FD, we
> +			 * just set the default link here.
> +			 */
> +			x->pcs_link = 1;
> +			x->pcs_duplex = 1;
> +			x->pcs_speed = SPEED_1000;
> +		}
> +
> +		/* Clear C37 AN complete status by writing zero */
> +		xpcs_write(XPCS_MDIO_MII_MMD, MDIO_MII_MMD_AN_STAT, 0);
> +		ret = IRQ_HANDLED;
> +	}
> +
> +	return ret;
> +}
> +
> +const struct stmmac_xpcs_ops xpcs_ops = {
> +	.xpcs_init = dw_xpcs_init,
> +	.xpcs_ctrl_ane = dw_xpcs_ctrl_ane,
> +	.xpcs_get_adv_lp = dw_xpcs_get_adv_lp,
> +	.xpcs_irq_status = dw_xpcs_irq_status,
> +};
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxpcs.h b/drivers/net/ethernet/stmicro/stmmac/dwxpcs.h
> new file mode 100644
> index 000000000000..bd52ce80bf4e
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxpcs.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2019, Intel Corporation.
> + * DWC Ethernet Physical Coding Sublayer
> + */
> +#ifndef __DW_XPCS_H__
> +#define __DW_XPCS_H__
> +
> +/* XPCS MII MMD Device Addresses */
> +#define XPCS_MDIO_MII_MMD	MDIO_MMD_VEND2
> +
> +/* MII MMD registers offsets */
> +#define MDIO_MII_MMD_DIGITAL_CTRL_1	0x8000	/* Digital Control 1 */
> +#define MDIO_MII_MMD_AN_CTRL		0x8001	/* AN Control */
> +#define MDIO_MII_MMD_AN_STAT		0x8002	/* AN Status */
> +
> +/* MII MMD SR AN Advertisement & Link Partner Ability are slightly
> + * different from MII_ADVERTISEMENT & MII_LPA in below fields:
> + */
> +#define MDIO_MII_MMD_HD			BIT(6)	/* Half duplex */
> +#define MDIO_MII_MMD_FD			BIT(5)	/* Full duplex */
> +#define MDIO_MII_MMD_PSE_SHIFT		7	/* Pause Ability shift */
> +#define MDIO_MII_MMD_PSE		GENMASK(8, 7)	/* Pause Ability */
> +#define MDIO_MII_MMD_PSE_NO		0x0
> +#define MDIO_MII_MMD_PSE_ASYM		0x1
> +#define MDIO_MII_MMD_PSE_SYM		0x2
> +#define MDIO_MII_MMD_PSE_BOTH		0x3
> +
> +/* Automatic Speed Mode Change for MAC side SGMII AN */
> +#define MDIO_MII_MMD_DIGI_CTRL_1_MAC_AUTO_SW	BIT(9)
> +
> +/* MII MMD AN Control defines */
> +#define MDIO_MII_MMD_AN_CTRL_TX_CONFIG_SHIFT	3 /* TX Config shift */
> +#define AN_CTRL_TX_CONF_PHY_SIDE_SGMII		0x1 /* PHY side SGMII mode */
> +#define AN_CTRL_TX_CONF_MAC_SIDE_SGMII		0x0 /* MAC side SGMII mode */
> +#define MDIO_MII_MMD_AN_CTRL_PCS_MD_SHIFT	1  /* PCS Mode shift */
> +#define MDIO_MII_MMD_AN_CTRL_PCS_MD	GENMASK(2, 1) /* PCS Mode */
> +#define AN_CTRL_PCS_MD_C37_1000BASEX	0x0	/* C37 AN for 1000BASE-X */
> +#define AN_CTRL_PCS_MD_C37_SGMII	0x2	/* C37 AN for SGMII */
> +#define MDIO_MII_MMD_AN_CTRL_AN_INTR_EN	BIT(0)	/* AN Complete Intr Enable */
> +
> +/* MII MMD AN Status defines for SGMII AN Status */
> +#define AN_STAT_SGMII_AN_CMPLT		BIT(0)	/* AN Complete Intr */
> +#define AN_STAT_SGMII_AN_FD		BIT(1)	/* Full Duplex */
> +#define AN_STAT_SGMII_AN_SPEED_SHIFT	2	/* AN Speed shift */
> +#define AN_STAT_SGMII_AN_SPEED		GENMASK(3, 2)	/* AN Speed */
> +#define AN_STAT_SGMII_AN_10MBPS		0x0	/* 10 Mbps */
> +#define AN_STAT_SGMII_AN_100MBPS	0x1	/* 100 Mbps */
> +#define AN_STAT_SGMII_AN_1000MBPS	0x2	/* 1000 Mbps */
> +#define AN_STAT_SGMII_AN_LNKSTS		BIT(4)	/* Link Status */
> +
> +#endif /* __DW_XPCS_H__ */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index 2acfbc70e3c8..431cf4261264 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> @@ -398,6 +398,25 @@ struct stmmac_ops {
>  #define stmmac_set_mac_loopback(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, mac, set_mac_loopback, __args)
>  
> +/* Helpers for DW xPCS */
> +struct stmmac_xpcs_ops {
> +	void (*xpcs_init)(struct net_device *ndev, int pcs_mode);
> +	void (*xpcs_ctrl_ane)(struct net_device *ndev, bool ane, bool loopback);
> +	void (*xpcs_get_adv_lp)(struct net_device *ndev, struct rgmii_adv *adv,
> +				int pcs_mode);
> +	int (*xpcs_irq_status)(struct net_device *ndev,
> +			       struct stmmac_extra_stats *x, int pcs_mode);
> +};
> +
> +#define stmmac_xpcs_init(__priv, __args...) \
> +	stmmac_do_void_callback(__priv, xpcs, xpcs_init, __args)
> +#define stmmac_xpcs_ctrl_ane(__priv, __args...) \
> +	stmmac_do_void_callback(__priv, xpcs, xpcs_ctrl_ane, __args)
> +#define stmmac_xpcs_get_adv_lp(__priv, __args...) \
> +	stmmac_do_void_callback(__priv, xpcs, xpcs_get_adv_lp, __args)
> +#define stmmac_xpcs_irq_status(__priv, __args...) \
> +	stmmac_do_callback(__priv, xpcs, xpcs_irq_status, __args)
> +
>  /* PTP and HW Timer helpers */
>  struct stmmac_hwtimestamp {
>  	void (*config_hw_tstamping) (void __iomem *ioaddr, u32 data);
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 4335bd771ce5..b00e7951a66d 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -148,6 +148,7 @@ struct stmmac_txq_cfg {
>  struct plat_stmmacenet_data {
>  	int bus_id;
>  	int phy_addr;
> +	int xpcs_phy_addr;
>  	int interface;
>  	struct stmmac_mdio_bus_data *mdio_bus_data;
>  	struct device_node *phy_node;
> 

-- 
Florian

Powered by blists - more mailing lists