RE: [PATCH 0/7] net: stmmac: enable EHL SGMII

RE: [PATCH 0/7] net: stmmac: enable EHL SGMII

[Jose Abreu]

From: Weifeng Voon <weifeng.voon@intel.com>
Date: Wed, Apr 24, 2019 at 18:17:14

> From: "Voon, Weifeng" <weifeng.voon@intel.com@intel.com>
> 
> This patch-set is to enable Ethernet controller (DW Ethernet QoS
> and DW Ethernet PCS) with SGMII interface in Elkhart Lake. The
> DW Ethernet PCS is the Physical Coding Sublayer that is between
> Ethernet MAC and PHY and uses MDIO Clause-45 as Communication.
> 

Please re-send this series with the stmmac maintainers in CC. Also CC 
Andrew Lunn and Florian Fainelli for the C45 part.

Thanks,
Jose Miguel Abreu

Re: [PATCH 2/7] net: stmmac: enable clause 45 mdio support

[Andrew Lunn]

On Thu, Apr 25, 2019 at 01:17:16AM +0800, Weifeng Voon wrote:
> From: Kweh Hock Leong <hock.leong.kweh@intel.com>
> 
> DWMAC4 is capable to support clause 45 mdio communication.

Hi Weifeng

Is C45 a synthesis time option, or do all DWMAC4 have it?

Thanks
	Andrew

Re: [PATCH 4/7] net: stmmac: introducing support for DWC xPCS logics

[Andrew Lunn]

On Thu, Apr 25, 2019 at 01:17:18AM +0800, Weifeng Voon wrote:
> From: Ong Boon Leong <boon.leong.ong@intel.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->|xPCS|<-->|L1 | <-- SGMII --> | External GbE |
>   |   MAC    |         |    |    |PHY|               | 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.
> 
> Basic functionalities for initializing xPCS and configuring auto
> negotiation (AN), loopback, link status, AN advertisement and Link
> Partner ability are implemented.
> 
> xPCS interrupt handling for C37 AN complete is also implemented.
> 
> Tested-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
> Reviewed-by: Chuah Kim Tatt <kim.tatt.chuah@intel.com>
> Reviewed-by: Voon Weifeng <weifeng.voon@intel.com>
> Reviewed-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
> Reviewed-by: Baoli Zhang <baoli.zhang@intel.com>
> Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>

You need your own signed-off-by here, since you are submitting it.

> ---
>  drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h | 288 ++++++++++++++++++++++++++
>  drivers/net/ethernet/stmicro/stmmac/hwif.h    |  17 ++
>  include/linux/stmmac.h                        |   1 +
>  3 files changed, 306 insertions(+)
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h b/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
> new file mode 100644
> index 0000000..446b714
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
> @@ -0,0 +1,288 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* dw_xpcs.h: DWC Ethernet Physical Coding Sublayer Header
> + *
> + * Copyright (c) 2019, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + */

Since you have a SPDX line, you don't need the license boilerplate.

> +#ifndef __DW_XPCS_H__
> +#define __DW_XPCS_H__
> +
> +#include <linux/mdio.h>
> +#include <linux/bitops.h>
> +#include "stmmac.h"
> +
> +/* XPCS Control & MII MMD Device Addresses */
> +#define XPCS_MDIO_CONTROL_MMD	MDIO_MMD_VEND1
> +#define XPCS_MDIO_MII_MMD	MDIO_MMD_VEND2
> +
> +/* Control MMD register offsets */
> +#define MDIO_CONTROL_MMD_CTRL		0x9	/* Control */
> +
> +/* Control MMD Control defines */
> +#define MDIO_CONTROL_MMD_CTRL_MII_MMD_EN	1 /* MII MMD Enable */
> +
> +/* MII MMD registers offsets */
> +#define MDIO_MII_MMD_CTRL		0x0	/* Control */
> +#define MDIO_MII_MMD_ADV		0x4	/* AN Advertisement */
> +#define MDIO_MII_MMD_LPA		0x5	/* Link Partner Ability */

MII_BMCR, MII_ADVERTISE, MII_LPA.

> +#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 Control defines */
> +#define MDIO_MII_MMD_CTRL_ANE		BIT(12)	/* AN Enable */
> +#define MDIO_MII_MMD_CTRL_LBE		BIT(14)	/* Loopback Enable */
> +#define MDIO_MII_MMD_CTRL_RANE		BIT(9)	/* Restart AN */

BMCR_ANENABLE, BMCR_LOOPBACK, BMCR_ANENABLE

If you use the standard names, developers are going to find it easier
to understand the code.


> +/* MII MMD AN Advertisement & Link Partner Ability */
> +#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 */
> +
> +/* MII MMD Digital Control 1 defines */
> +#define MDIO_MII_MMD_DIGI_CTRL_1_SGMII_PHY_MD	BIT(0) /* SGMII PHY mode */
> +
> +/* 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 C37 AN SGMII Status */
> +#define AN_STAT_C37_AN_CMPLT		BIT(0)	/* AN Complete Intr */
> +#define AN_STAT_C37_AN_FD		BIT(1)	/* Full Duplex */
> +#define AN_STAT_C37_AN_SPEED_SHIFT	2	/* AN Speed shift */
> +#define AN_STAT_C37_AN_SPEED		GENMASK(3, 2)	/* AN Speed */
> +#define AN_STAT_C37_AN_10MBPS		0x0	/* 10 Mbps */
> +#define AN_STAT_C37_AN_100MBPS		0x1	/* 100 Mbps */
> +#define AN_STAT_C37_AN_1000MBPS		0x2	/* 1000 Mbps */
> +#define AN_STAT_C37_AN_LNKSTS		BIT(4)	/* Link Status */

Is these are standardized, not proprietary, consider adding them to
include/uapi/linux/mii.h so similar.

> +
> +/**
> + * dw_xpcs_init - To initialize xPCS
> + * @ndev: network device pointer
> + * @mode: PCS mode
> + * Description: this is to initialize xPCS
> + */
> +static inline void dw_xpcs_init(struct net_device *ndev, int pcs_mode)
> +{
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	int xpcs_phy_addr = priv->plat->xpcs_phy_addr;
> +
> +	/* Set SGMII PHY mode control */
> +	u16 phydata = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
> +				    (MII_ADDR_C45 |
> +				     (XPCS_MDIO_MII_MMD <<
> +				      MII_DEVADDR_C45_SHIFT) |
> +				     MDIO_MII_MMD_DIGITAL_CTRL_1));
> +

Why cast to u16? It return int for a reason.

> +	phydata |= MDIO_MII_MMD_DIGI_CTRL_1_SGMII_PHY_MD;
> +
> +	mdiobus_write(priv->mii, xpcs_phy_addr,
> +		      (MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
> +		       MII_DEVADDR_C45_SHIFT) | MDIO_MII_MMD_DIGITAL_CTRL_1),
> +		      (int)phydata);

Maybe add helpers xpcs_read() and xpcs_write()?

      Andrew

Re: [PATCH 0/7] net: stmmac: enable EHL SGMII

[Andrew Lunn]

On Thu, Apr 25, 2019 at 01:17:14AM +0800, Weifeng Voon wrote:
> From: "Voon, Weifeng" <weifeng.voon@intel.com@intel.com>
> 
> This patch-set is to enable Ethernet controller (DW Ethernet QoS
> and DW Ethernet PCS) with SGMII interface in Elkhart Lake.

Can the hardware also do 1000BaseX?

    Andrew

[PATCH 0/7] net: stmmac: enable EHL SGMII

[Weifeng Voon]

From: "Voon, Weifeng" <weifeng.voon@intel.com@intel.com>

This patch-set is to enable Ethernet controller (DW Ethernet QoS
and DW Ethernet PCS) with SGMII interface in Elkhart Lake. The
DW Ethernet PCS is the Physical Coding Sublayer that is between
Ethernet MAC and PHY and uses MDIO Clause-45 as Communication.

Kweh Hock Leong (1):
  net: stmmac: enable clause 45 mdio support

Ong Boon Leong (3):
  net: stmmac: introducing support for DWC xPCS logics
  net: stmmac: add xpcs function hooks into main driver and ethtool
  net: stmmac: add xPCS functions for device with DWMACv5.1

Weifeng Voon (3):
  net: stmmac: add EHL SGMII 1Gbps platform data and PCI ID
  net: stmmac: dma channel control register need to be init first
  net: stmmac: add xPCS platform data for EHL

 drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h      | 288 +++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c  |  67 +++++
 drivers/net/ethernet/stmicro/stmmac/hwif.c         |  31 ++-
 drivers/net/ethernet/stmicro/stmmac/hwif.h         |  18 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac.h       |   2 +
 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   |  14 +
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |  58 ++++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c  |  36 ++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c   | 108 ++++++++
 include/linux/phy.h                                |   2 +
 include/linux/stmmac.h                             |   3 +
 11 files changed, 616 insertions(+), 11 deletions(-)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h

--
1.9.1

[PATCH 1/7] net: stmmac: add EHL SGMII 1Gbps platform data and PCI ID

[Weifeng Voon]

Added EHL SGMII 1Gbps PCI ID. Different MII and speed will have
different PCI ID.
For EHL, default TX and RX FIFO size is set to 32KB. This is
because the FIFO size advertised in the HW features is not the
same as the HW implementation. The TX FIFO is shared among all
all the TX queues and the RX FIFO is also shared among all the
RX queues.

Signed-off-by: Weifeng Voon <weifeng.voon@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 103 +++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index d819e8e..b454a97 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -118,6 +118,107 @@ static int stmmac_default_data(struct pci_dev *pdev,
 	.setup = stmmac_default_data,
 };
 
+static int ehl_common_data(struct pci_dev *pdev,
+			   struct plat_stmmacenet_data *plat)
+{
+	int i;
+
+	plat->bus_id = 1;
+	plat->phy_addr = 0;
+	plat->clk_csr = 5;
+	plat->has_gmac = 0;
+	plat->has_gmac4 = 1;
+	plat->force_sf_dma_mode = 0;
+	plat->tso_en = 1;
+
+	plat->rx_queues_to_use = 8;
+	plat->tx_queues_to_use = 8;
+	plat->rx_sched_algorithm = MTL_RX_ALGORITHM_SP;
+
+	for (i = 0; i < plat->rx_queues_to_use; i++) {
+		plat->rx_queues_cfg[i].mode_to_use = MTL_QUEUE_DCB;
+		plat->rx_queues_cfg[i].chan = i;
+
+		/* Disable Priority config by default */
+		plat->tx_queues_cfg[i].use_prio = false;
+
+		/* Disable RX queues routing by default */
+		plat->rx_queues_cfg[i].pkt_route = 0x0;
+	}
+
+	for (i = 0; i < plat->tx_queues_to_use; i++) {
+		plat->tx_queues_cfg[i].mode_to_use = MTL_QUEUE_DCB;
+
+		/* Disable Priority config by default */
+		plat->tx_queues_cfg[i].use_prio = false;
+	}
+
+	plat->tx_sched_algorithm = MTL_TX_ALGORITHM_WRR;
+	plat->tx_queues_cfg[0].weight = 0x09;
+	plat->tx_queues_cfg[1].weight = 0x0A;
+	plat->tx_queues_cfg[2].weight = 0x0B;
+	plat->tx_queues_cfg[3].weight = 0x0C;
+	plat->tx_queues_cfg[4].weight = 0x0D;
+	plat->tx_queues_cfg[5].weight = 0x0E;
+	plat->tx_queues_cfg[6].weight = 0x0F;
+	plat->tx_queues_cfg[7].weight = 0x10;
+
+	plat->mdio_bus_data->phy_reset = NULL;
+	plat->mdio_bus_data->phy_mask = 0;
+
+	plat->dma_cfg->pbl = 32;
+	plat->dma_cfg->pblx8 = true;
+	plat->dma_cfg->fixed_burst = 0;
+	plat->dma_cfg->mixed_burst = 0;
+	plat->dma_cfg->aal = 0;
+
+	plat->axi = devm_kzalloc(&pdev->dev, sizeof(*plat->axi),
+				 GFP_KERNEL);
+	if (!plat->axi)
+		return -ENOMEM;
+	plat->axi->axi_lpi_en = 0;
+	plat->axi->axi_xit_frm = 0;
+	plat->axi->axi_wr_osr_lmt = 0;
+	plat->axi->axi_rd_osr_lmt = 2;
+	plat->axi->axi_blen[0] = 4;
+	plat->axi->axi_blen[1] = 8;
+	plat->axi->axi_blen[2] = 16;
+
+	/* Set default value for multicast hash bins */
+	plat->multicast_filter_bins = HASH_TABLE_SIZE;
+
+	/* Set default value for unicast filter entries */
+	plat->unicast_filter_entries = 1;
+
+	/* Set the maxmtu to a default of JUMBO_LEN */
+	plat->maxmtu = JUMBO_LEN;
+
+	plat->tx_fifo_size = 32768;
+	plat->rx_fifo_size = 32768;
+
+	return 0;
+}
+
+static int ehl_sgmii1g_data(struct pci_dev *pdev,
+			    struct plat_stmmacenet_data *plat)
+{
+	int ret;
+
+	/* Set common default data first */
+	ret = ehl_common_data(pdev, plat);
+
+	if (ret)
+		return ret;
+
+	plat->interface = PHY_INTERFACE_MODE_SGMII;
+
+	return 0;
+}
+
+static struct stmmac_pci_info ehl_sgmii1g_pci_info = {
+	.setup = ehl_sgmii1g_data,
+};
+
 static const struct stmmac_pci_func_data galileo_stmmac_func_data[] = {
 	{
 		.func = 6,
@@ -355,6 +456,7 @@ static int __maybe_unused stmmac_pci_resume(struct device *dev)
 
 #define STMMAC_QUARK_ID  0x0937
 #define STMMAC_DEVICE_ID 0x1108
+#define STMMAC_EHL_SGMII1G_ID   0x4b31
 
 #define STMMAC_DEVICE(vendor_id, dev_id, info)	{	\
 	PCI_VDEVICE(vendor_id, dev_id),			\
@@ -365,6 +467,7 @@ static int __maybe_unused stmmac_pci_resume(struct device *dev)
 	STMMAC_DEVICE(STMMAC, STMMAC_DEVICE_ID, stmmac_pci_info),
 	STMMAC_DEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_MAC, stmmac_pci_info),
 	STMMAC_DEVICE(INTEL, STMMAC_QUARK_ID, quark_pci_info),
+	STMMAC_DEVICE(INTEL, STMMAC_EHL_SGMII1G_ID, ehl_sgmii1g_pci_info),
 	{}
 };
 
-- 
1.9.1

[PATCH 2/7] net: stmmac: enable clause 45 mdio support

[Weifeng Voon]

From: Kweh Hock Leong <hock.leong.kweh@intel.com>

DWMAC4 is capable to support clause 45 mdio communication.
This patch enable the feature on stmmac_mdio_write() and
stmmac_mdio_read() by following phy_write_mmd() and
phy_read_mmd() mdiobus read write implementation format.

Reviewed-by: Li, Yifan <yifan2.li@intel.com>
Signed-off-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
Signed-off-by: Weifeng Voon <weifeng.voon@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 36 +++++++++++++++++++----
 include/linux/phy.h                               |  2 ++
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index bdd3515..149efa6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -34,11 +34,27 @@
 
 #define MII_BUSY 0x00000001
 #define MII_WRITE 0x00000002
+#define MII_DATA_MASK GENMASK(15, 0)
 
 /* GMAC4 defines */
 #define MII_GMAC4_GOC_SHIFT		2
+#define MII_GMAC4_REG_ADDR_SHIFT	16
 #define MII_GMAC4_WRITE			(1 << MII_GMAC4_GOC_SHIFT)
 #define MII_GMAC4_READ			(3 << MII_GMAC4_GOC_SHIFT)
+#define MII_GMAC4_C45E			BIT(1)
+
+static void stmmac_mdio_c45_setup(struct stmmac_priv *priv, int phyreg,
+				  u32 *val, u32 *data)
+{
+	unsigned int reg_mask = priv->hw->mii.reg_mask;
+	unsigned int reg_shift = priv->hw->mii.reg_shift;
+
+	*val |= MII_GMAC4_C45E;
+	*val &= ~reg_mask;
+	*val |= ((phyreg >> MII_DEVADDR_C45_SHIFT) << reg_shift) & reg_mask;
+
+	*data |= (phyreg & MII_REGADDR_C45_MASK) << MII_GMAC4_REG_ADDR_SHIFT;
+}
 
 /* XGMAC defines */
 #define MII_XGMAC_SADDR			BIT(18)
@@ -166,21 +182,25 @@ static int stmmac_mdio_read(struct mii_bus *bus, int phyaddr, int phyreg)
 	unsigned int mii_address = priv->hw->mii.addr;
 	unsigned int mii_data = priv->hw->mii.data;
 	u32 v;
-	int data;
 	u32 value = MII_BUSY;
+	int data = 0;
 
 	value |= (phyaddr << priv->hw->mii.addr_shift)
 		& priv->hw->mii.addr_mask;
 	value |= (phyreg << priv->hw->mii.reg_shift) & priv->hw->mii.reg_mask;
 	value |= (priv->clk_csr << priv->hw->mii.clk_csr_shift)
 		& priv->hw->mii.clk_csr_mask;
-	if (priv->plat->has_gmac4)
+	if (priv->plat->has_gmac4) {
 		value |= MII_GMAC4_READ;
+		if (phyreg & MII_ADDR_C45)
+			stmmac_mdio_c45_setup(priv, phyreg, &value, &data);
+	}
 
 	if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
 			       100, 10000))
 		return -EBUSY;
 
+	writel(data, priv->ioaddr + mii_data);
 	writel(value, priv->ioaddr + mii_address);
 
 	if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
@@ -188,7 +208,7 @@ static int stmmac_mdio_read(struct mii_bus *bus, int phyaddr, int phyreg)
 		return -EBUSY;
 
 	/* Read the data from the MII data register */
-	data = (int)readl(priv->ioaddr + mii_data);
+	data = (int)readl(priv->ioaddr + mii_data) & MII_DATA_MASK;
 
 	return data;
 }
@@ -210,6 +230,7 @@ static int stmmac_mdio_write(struct mii_bus *bus, int phyaddr, int phyreg,
 	unsigned int mii_data = priv->hw->mii.data;
 	u32 v;
 	u32 value = MII_BUSY;
+	int data = phydata;
 
 	value |= (phyaddr << priv->hw->mii.addr_shift)
 		& priv->hw->mii.addr_mask;
@@ -217,10 +238,13 @@ static int stmmac_mdio_write(struct mii_bus *bus, int phyaddr, int phyreg,
 
 	value |= (priv->clk_csr << priv->hw->mii.clk_csr_shift)
 		& priv->hw->mii.clk_csr_mask;
-	if (priv->plat->has_gmac4)
+	if (priv->plat->has_gmac4) {
 		value |= MII_GMAC4_WRITE;
-	else
+		if (phyreg & MII_ADDR_C45)
+			stmmac_mdio_c45_setup(priv, phyreg, &value, &data);
+	} else {
 		value |= MII_WRITE;
+	}
 
 	/* Wait until any existing MII operation is complete */
 	if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
@@ -228,7 +252,7 @@ static int stmmac_mdio_write(struct mii_bus *bus, int phyaddr, int phyreg,
 		return -EBUSY;
 
 	/* Set the MII address register to write */
-	writel(phydata, priv->ioaddr + mii_data);
+	writel(data, priv->ioaddr + mii_data);
 	writel(value, priv->ioaddr + mii_address);
 
 	/* Wait until any existing MII operation is complete */
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 0f9552b1..6710e66 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -198,6 +198,8 @@ static inline const char *phy_modes(phy_interface_t interface)
 /* Or MII_ADDR_C45 into regnum for read/write on mii_bus to enable the 21 bit
    IEEE 802.3ae clause 45 addressing mode used by 10GIGE phy chips. */
 #define MII_ADDR_C45 (1<<30)
+#define MII_DEVADDR_C45_SHIFT	16
+#define MII_REGADDR_C45_MASK	GENMASK(15, 0)
 
 struct device;
 struct phylink;
-- 
1.9.1

[PATCH 3/7] net: stmmac: dma channel control register need to be init first

[Weifeng Voon]

stmmac_init_chan() needs to be called before stmmac_init_rx_chan() and
stmmac_init_tx_chan(). This is because if PBLx8 is to be used,
"DMA_CH(#i)_Control.PBLx8" needs to be set before programming
"DMA_CH(#i)_TX_Control.TxPBL" and "DMA_CH(#i)_RX_Control.RxPBL".

Reviewed-by: Zhang, Baoli <baoli.zhang@intel.com>
Signed-off-by: Weifeng Voon <weifeng.voon@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index a26e36d..ec031e3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2208,6 +2208,10 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
 	if (priv->plat->axi)
 		stmmac_axi(priv, priv->ioaddr, priv->plat->axi);
 
+	/* DMA CSR Channel configuration */
+	for (chan = 0; chan < dma_csr_ch; chan++)
+		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
+
 	/* DMA RX Channel Configuration */
 	for (chan = 0; chan < rx_channels_count; chan++) {
 		rx_q = &priv->rx_queue[chan];
@@ -2233,10 +2237,6 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
 				       tx_q->tx_tail_addr, chan);
 	}
 
-	/* DMA CSR Channel configuration */
-	for (chan = 0; chan < dma_csr_ch; chan++)
-		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
-
 	return ret;
 }
 
-- 
1.9.1

[PATCH 4/7] net: stmmac: introducing support for DWC xPCS logics

[Weifeng Voon]

From: Ong Boon Leong <boon.leong.ong@intel.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->|xPCS|<-->|L1 | <-- SGMII --> | External GbE |
  |   MAC    |         |    |    |PHY|               | 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.

Basic functionalities for initializing xPCS and configuring auto
negotiation (AN), loopback, link status, AN advertisement and Link
Partner ability are implemented.

xPCS interrupt handling for C37 AN complete is also implemented.

Tested-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
Reviewed-by: Chuah Kim Tatt <kim.tatt.chuah@intel.com>
Reviewed-by: Voon Weifeng <weifeng.voon@intel.com>
Reviewed-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
Reviewed-by: Baoli Zhang <baoli.zhang@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h | 288 ++++++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  17 ++
 include/linux/stmmac.h                        |   1 +
 3 files changed, 306 insertions(+)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h

diff --git a/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h b/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
new file mode 100644
index 0000000..446b714
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
@@ -0,0 +1,288 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* dw_xpcs.h: DWC Ethernet Physical Coding Sublayer Header
+ *
+ * Copyright (c) 2019, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+#ifndef __DW_XPCS_H__
+#define __DW_XPCS_H__
+
+#include <linux/mdio.h>
+#include <linux/bitops.h>
+#include "stmmac.h"
+
+/* XPCS Control & MII MMD Device Addresses */
+#define XPCS_MDIO_CONTROL_MMD	MDIO_MMD_VEND1
+#define XPCS_MDIO_MII_MMD	MDIO_MMD_VEND2
+
+/* Control MMD register offsets */
+#define MDIO_CONTROL_MMD_CTRL		0x9	/* Control */
+
+/* Control MMD Control defines */
+#define MDIO_CONTROL_MMD_CTRL_MII_MMD_EN	1 /* MII MMD Enable */
+
+/* MII MMD registers offsets */
+#define MDIO_MII_MMD_CTRL		0x0	/* Control */
+#define MDIO_MII_MMD_ADV		0x4	/* AN Advertisement */
+#define MDIO_MII_MMD_LPA		0x5	/* Link Partner Ability */
+#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 Control defines */
+#define MDIO_MII_MMD_CTRL_ANE		BIT(12)	/* AN Enable */
+#define MDIO_MII_MMD_CTRL_LBE		BIT(14)	/* Loopback Enable */
+#define MDIO_MII_MMD_CTRL_RANE		BIT(9)	/* Restart AN */
+
+/* MII MMD AN Advertisement & Link Partner Ability */
+#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 */
+
+/* MII MMD Digital Control 1 defines */
+#define MDIO_MII_MMD_DIGI_CTRL_1_SGMII_PHY_MD	BIT(0) /* SGMII PHY mode */
+
+/* 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 C37 AN SGMII Status */
+#define AN_STAT_C37_AN_CMPLT		BIT(0)	/* AN Complete Intr */
+#define AN_STAT_C37_AN_FD		BIT(1)	/* Full Duplex */
+#define AN_STAT_C37_AN_SPEED_SHIFT	2	/* AN Speed shift */
+#define AN_STAT_C37_AN_SPEED		GENMASK(3, 2)	/* AN Speed */
+#define AN_STAT_C37_AN_10MBPS		0x0	/* 10 Mbps */
+#define AN_STAT_C37_AN_100MBPS		0x1	/* 100 Mbps */
+#define AN_STAT_C37_AN_1000MBPS		0x2	/* 1000 Mbps */
+#define AN_STAT_C37_AN_LNKSTS		BIT(4)	/* Link Status */
+
+/**
+ * dw_xpcs_init - To initialize xPCS
+ * @ndev: network device pointer
+ * @mode: PCS mode
+ * Description: this is to initialize xPCS
+ */
+static inline void dw_xpcs_init(struct net_device *ndev, int pcs_mode)
+{
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	int xpcs_phy_addr = priv->plat->xpcs_phy_addr;
+
+	/* Set SGMII PHY mode control */
+	u16 phydata = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
+				    (MII_ADDR_C45 |
+				     (XPCS_MDIO_MII_MMD <<
+				      MII_DEVADDR_C45_SHIFT) |
+				     MDIO_MII_MMD_DIGITAL_CTRL_1));
+
+	phydata |= MDIO_MII_MMD_DIGI_CTRL_1_SGMII_PHY_MD;
+
+	mdiobus_write(priv->mii, xpcs_phy_addr,
+		      (MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
+		       MII_DEVADDR_C45_SHIFT) | MDIO_MII_MMD_DIGITAL_CTRL_1),
+		      (int)phydata);
+
+	/* Set PHY side SGMII, PCS Mode & Enable C37 AN complete interrupt */
+	phydata = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
+				    (MII_ADDR_C45 |
+				     (XPCS_MDIO_MII_MMD <<
+				      MII_DEVADDR_C45_SHIFT) |
+				     MDIO_MII_MMD_AN_CTRL));
+
+	phydata &= ~MDIO_MII_MMD_AN_CTRL_PCS_MD;
+	phydata |= (((pcs_mode << MDIO_MII_MMD_AN_CTRL_PCS_MD_SHIFT) &
+		    MDIO_MII_MMD_AN_CTRL_PCS_MD) |
+		    (AN_CTRL_TX_CONF_PHY_SIDE_SGMII <<
+		    MDIO_MII_MMD_AN_CTRL_TX_CONFIG_SHIFT) |
+		    MDIO_MII_MMD_AN_CTRL_AN_INTR_EN);
+
+	mdiobus_write(priv->mii, xpcs_phy_addr,
+		      (MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
+		       MII_DEVADDR_C45_SHIFT) | MDIO_MII_MMD_AN_CTRL),
+		      (int)phydata);
+}
+
+/**
+ * dw_xpcs_rane - To restart Auto Negotiation (AN)
+ * @ndev: network device pointer
+ * @restart: to restart AN
+ * Description: this is to restart AN.
+ */
+static inline void dw_xpcs_rane(struct net_device *ndev, bool restart)
+{
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	int xpcs_phy_addr = priv->plat->xpcs_phy_addr;
+
+	u16 phydata = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
+				(MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
+				 MII_DEVADDR_C45_SHIFT) |
+				 MDIO_MII_MMD_CTRL));
+
+	if (restart)
+		phydata |= MDIO_MII_MMD_CTRL_RANE;
+
+	mdiobus_write(priv->mii, xpcs_phy_addr,
+		      (MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
+		       MII_DEVADDR_C45_SHIFT) | MDIO_MII_MMD_CTRL),
+		      (int)phydata);
+}
+
+/**
+ * dw_xpcs_ctrl_ane - To program the MII MMD Control Register.
+ * @ndev: network device pointer
+ * @ane: to enable the Auto Negotiation
+ * @loopback: to cause the PHY to loopback Tx data into Rx path.
+ * Description: this is the main function to configure the MII MMD
+ * control register and init the AN Enable and select loopback.
+ */
+static inline void dw_xpcs_ctrl_ane(struct net_device *ndev, bool ane,
+				    bool loopback)
+{
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	int xpcs_phy_addr = priv->plat->xpcs_phy_addr;
+
+	u16 phydata = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
+				(MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
+				 MII_DEVADDR_C45_SHIFT) |
+				 MDIO_MII_MMD_CTRL));
+
+	if (ane)
+		phydata |= (MDIO_MII_MMD_CTRL_ANE | MDIO_MII_MMD_CTRL_RANE);
+
+	if (loopback)
+		phydata |= MDIO_MII_MMD_CTRL_LBE;
+
+	mdiobus_write(priv->mii, xpcs_phy_addr,
+		      (MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
+		       MII_DEVADDR_C45_SHIFT) | MDIO_MII_MMD_CTRL),
+		      (int)phydata);
+}
+
+/**
+ * dw_xpcs_get_adv_lp - Get AN Advertisement and Link Partner Ability
+ * @ndev: network device pointer
+ * @adv_lp: structure to store the adv, lp status
+ * Description: this is to expose the Auto Negotiation Advertisement and
+ * Link partner ability status to ethtool support.
+ */
+static inline void dw_xpcs_get_adv_lp(struct net_device *ndev,
+				      struct rgmii_adv *adv_lp)
+{
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	int xpcs_phy_addr = priv->plat->xpcs_phy_addr;
+
+	/* AN Advertisement Ability */
+	u16 value = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
+				(MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
+				 MII_DEVADDR_C45_SHIFT) |
+				 MDIO_MII_MMD_ADV));
+
+	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 */
+	value = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
+				  (MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
+				   MII_DEVADDR_C45_SHIFT) |
+				   MDIO_MII_MMD_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);
+}
+
+/**
+ * dw_xpcs_get_linkstatus - Get Link Status
+ * @an_stat: C37 AN status value
+ * @x: stmmac extra status
+ * Description: this is to read the link extra status from <C37 AN SGMII
+ * status> field of MII MMD AN Status register.
+ */
+static inline void dw_xpcs_get_linkstatus(u16 an_stat,
+					  struct stmmac_extra_stats *x)
+{
+	/* Check the link status */
+	if (an_stat & AN_STAT_C37_AN_LNKSTS) {
+		int speed_value;
+
+		x->pcs_link = 1;
+
+		speed_value = ((an_stat & AN_STAT_C37_AN_SPEED) >>
+				AN_STAT_C37_AN_SPEED_SHIFT);
+		if (speed_value == AN_STAT_C37_AN_1000MBPS)
+			x->pcs_speed = SPEED_1000;
+		else if (speed_value == AN_STAT_C37_AN_100MBPS)
+			x->pcs_speed = SPEED_100;
+		else
+			x->pcs_speed = SPEED_10;
+
+		if (an_stat & AN_STAT_C37_AN_FD)
+			x->pcs_duplex = 1;
+		else
+			x->pcs_duplex = 0;
+
+		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");
+	}
+}
+
+/**
+ * dw_xpcs_irq_status - Get xPCS IRQ Status
+ * @ndev: network device pointer
+ * @x: stmmac extra status
+ * Description: this is to read the xPCS IRQ status.
+ */
+static inline int dw_xpcs_irq_status(struct net_device *ndev,
+				     struct stmmac_extra_stats *x)
+{
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	int xpcs_phy_addr = priv->plat->xpcs_phy_addr;
+	int ret = IRQ_NONE;
+
+	/* C37 AN status */
+	u16 an_stat = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
+					(MII_ADDR_C45 |
+					 (XPCS_MDIO_MII_MMD <<
+					  MII_DEVADDR_C45_SHIFT) |
+					 MDIO_MII_MMD_AN_STAT));
+
+	if (an_stat & AN_STAT_C37_AN_CMPLT) {
+		x->irq_pcs_ane_n++;
+		dw_xpcs_get_linkstatus(an_stat, x);
+
+		/* Clear C37 AN complete status by writing zero */
+		mdiobus_write(priv->mii, xpcs_phy_addr,
+			      (MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
+			       MII_DEVADDR_C45_SHIFT) |
+			       MDIO_MII_MMD_AN_STAT),
+			      0);
+		ret = IRQ_HANDLED;
+	}
+
+	return ret;
+}
+#endif /* __DW_XPCS_H__ */
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 5bb0023..cb7eb48 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -310,6 +310,13 @@ struct stmmac_ops {
 			     bool loopback);
 	void (*pcs_rane)(void __iomem *ioaddr, bool restart);
 	void (*pcs_get_adv_lp)(void __iomem *ioaddr, struct rgmii_adv *adv);
+	/* xPCS calls */
+	void (*xpcs_init)(struct net_device *ndev, int pcs_mode);
+	void (*xpcs_ctrl_ane)(struct net_device *ndev, bool ane, bool loopback);
+	void (*xpcs_rane)(struct net_device *ndev, bool restart);
+	void (*xpcs_get_adv_lp)(struct net_device *ndev, struct rgmii_adv *adv);
+	int (*xpcs_irq_status)(struct net_device *ndev,
+			       struct stmmac_extra_stats *x);
 	/* Safety Features */
 	int (*safety_feat_config)(void __iomem *ioaddr, unsigned int asp);
 	int (*safety_feat_irq_status)(struct net_device *ndev,
@@ -382,6 +389,16 @@ struct stmmac_ops {
 	stmmac_do_void_callback(__priv, mac, pcs_rane, __args)
 #define stmmac_pcs_get_adv_lp(__priv, __args...) \
 	stmmac_do_void_callback(__priv, mac, pcs_get_adv_lp, __args)
+#define stmmac_xpcs_init(__priv, __args...) \
+	stmmac_do_void_callback(__priv, mac, xpcs_init, __args)
+#define stmmac_xpcs_ctrl_ane(__priv, __args...) \
+	stmmac_do_void_callback(__priv, mac, xpcs_ctrl_ane, __args)
+#define stmmac_xpcs_rane(__priv, __args...) \
+	stmmac_do_void_callback(__priv, mac, xpcs_rane, __args)
+#define stmmac_xpcs_get_adv_lp(__priv, __args...) \
+	stmmac_do_void_callback(__priv, mac, xpcs_get_adv_lp, __args)
+#define stmmac_xpcs_irq_status(__priv, __args...) \
+	stmmac_do_callback(__priv, mac, xpcs_irq_status, __args)
 #define stmmac_safety_feat_config(__priv, __args...) \
 	stmmac_do_callback(__priv, mac, safety_feat_config, __args)
 #define stmmac_safety_feat_irq_status(__priv, __args...) \
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 4335bd7..b00e795 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;
-- 
1.9.1

[PATCH 5/7] net: stmmac: add xpcs function hooks into main driver and ethtool

[Weifeng Voon]

From: Ong Boon Leong <boon.leong.ong@intel.com>

With xPCS functions now ready, we add them into the main driver and
ethtool logics. To differentiate from EQoS MAC PCS and DWC Ethernet
xPCS, we introduce 'has_xpcs' in platform data as a mean to indicate
whether GBE controller includes xPCS or not.

To support platform-specific C37 AN PCS mode selection for MII MMD,
we introduce 'pcs_mode' in platform data.

The basic framework for xPCS interrupt handling is implemented too.

Reviewed-by: Chuah Kim Tatt <kim.tatt.chuah@intel.com>
Reviewed-by: Voon Weifeng <weifeng.voon@intel.com>
Reviewed-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
Reviewed-by: Baoli Zhang <baoli.zhang@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h       |  2 +
 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   | 14 ++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 50 ++++++++++++++++++++++
 include/linux/stmmac.h                             |  2 +
 4 files changed, 68 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index dd95d95..0b8460a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -36,6 +36,7 @@ struct stmmac_resources {
 	const char *mac;
 	int wol_irq;
 	int lpi_irq;
+	int xpcs_irq;
 	int irq;
 };
 
@@ -168,6 +169,7 @@ struct stmmac_priv {
 	int clk_csr;
 	struct timer_list eee_ctrl_timer;
 	int lpi_irq;
+	int xpcs_irq;
 	int eee_enabled;
 	int eee_active;
 	int tx_lpi_timer;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index 3c749c3..0c146b3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -392,6 +392,10 @@ static int stmmac_ethtool_get_link_ksettings(struct net_device *dev,
 
 		mutex_lock(&priv->lock);
 		stmmac_pcs_ctrl_ane(priv, priv->ioaddr, 1, priv->hw->ps, 0);
+
+		if (priv->plat->has_xpcs)
+			stmmac_xpcs_ctrl_ane(priv, dev, 1, 0);
+
 		mutex_unlock(&priv->lock);
 
 		return 0;
@@ -457,6 +461,11 @@ static void stmmac_ethtool_gregs(struct net_device *dev,
 		pause->autoneg = 1;
 		if (!adv_lp.pause)
 			return;
+	} else if (priv->plat->has_xpcs &&
+		   !stmmac_xpcs_get_adv_lp(priv, netdev, &adv_lp)) {
+		pause->autoneg = 1;
+		if (!adv_lp.pause)
+			return;
 	} else {
 		if (!linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
 				       netdev->phydev->supported) ||
@@ -488,6 +497,11 @@ static void stmmac_ethtool_gregs(struct net_device *dev,
 		pause->autoneg = 1;
 		if (!adv_lp.pause)
 			return -EOPNOTSUPP;
+	} else if (priv->plat->has_xpcs &&
+		   !stmmac_xpcs_get_adv_lp(priv, netdev, &adv_lp)) {
+		pause->autoneg = 1;
+		if (!adv_lp.pause)
+			return -EOPNOTSUPP;
 	} else {
 		if (!linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
 				       phy->supported) ||
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index ec031e3..d6e315f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -112,6 +112,7 @@
 MODULE_PARM_DESC(chain_mode, "To use chain instead of ring mode");
 
 static irqreturn_t stmmac_interrupt(int irq, void *dev_id);
+static irqreturn_t xpcs_interrupt(int irq, void *dev_id);
 
 #ifdef CONFIG_DEBUG_FS
 static int stmmac_init_fs(struct net_device *dev);
@@ -2538,6 +2539,10 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
 	/* Initialize MTL*/
 	stmmac_mtl_configuration(priv);
 
+	/* Initialize the xPCS PHY */
+	if (priv->plat->has_xpcs)
+		stmmac_xpcs_init(priv, dev, priv->plat->pcs_mode);
+
 	/* Initialize Safety Features */
 	stmmac_safety_feat_configuration(priv);
 
@@ -2579,6 +2584,9 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
 	if (priv->hw->pcs)
 		stmmac_pcs_ctrl_ane(priv, priv->hw, 1, priv->hw->ps, 0);
 
+	if (priv->plat->has_xpcs)
+		stmmac_xpcs_ctrl_ane(priv, dev, 1, 0);
+
 	/* set TX and RX rings length */
 	stmmac_set_rings_length(priv);
 
@@ -2696,11 +2704,27 @@ static int stmmac_open(struct net_device *dev)
 		}
 	}
 
+	/* xPCS IRQ line */
+	if (priv->xpcs_irq > 0) {
+		ret = request_irq(priv->xpcs_irq, xpcs_interrupt, IRQF_SHARED,
+				  dev->name, dev);
+		if (unlikely(ret < 0)) {
+			netdev_err(priv->dev,
+				   "%s: ERROR: allocating the xPCS IRQ %d (%d)\n",
+				   __func__, priv->xpcs_irq, ret);
+			goto xpcsirq_error;
+		}
+	}
+
 	stmmac_enable_all_queues(priv);
 	stmmac_start_all_queues(priv);
 
 	return 0;
 
+xpcsirq_error:
+	if (priv->lpi_irq > 0)
+		free_irq(priv->lpi_irq, dev);
+
 lpiirq_error:
 	if (priv->wol_irq != dev->irq)
 		free_irq(priv->wol_irq, dev);
@@ -3774,6 +3798,31 @@ static irqreturn_t stmmac_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+/**
+ *  xPCS_interrupt - xPCS ISR
+ *  @irq: interrupt number.
+ *  @dev_id: to pass the net device pointer.
+ *  Description: this is the xPCS interrupt service routine.
+ */
+static irqreturn_t xpcs_interrupt(int irq, void *dev_id)
+{
+	irqreturn_t ret = IRQ_NONE;
+	struct net_device *ndev = (struct net_device *)dev_id;
+	struct stmmac_priv *priv = netdev_priv(ndev);
+
+	if (unlikely(!ndev)) {
+		netdev_err(priv->dev, "%s: invalid dev pointer\n",
+			   __func__);
+		return ret;
+	}
+
+	/* To handle xPCS interrupts */
+	if (priv->plat->has_xpcs)
+		ret = stmmac_xpcs_irq_status(priv, ndev, &priv->xstats);
+
+	return ret;
+}
+
 #ifdef CONFIG_NET_POLL_CONTROLLER
 /* Polling receive - used by NETCONSOLE and other diagnostic tools
  * to allow network I/O with interrupts disabled.
@@ -4263,6 +4312,7 @@ int stmmac_dvr_probe(struct device *device,
 	priv->dev->irq = res->irq;
 	priv->wol_irq = res->wol_irq;
 	priv->lpi_irq = res->lpi_irq;
+	priv->xpcs_irq = res->xpcs_irq;
 
 	if (res->mac)
 		memcpy(priv->dev->dev_addr, res->mac, ETH_ALEN);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index b00e795..2adb78c 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -189,6 +189,8 @@ struct plat_stmmacenet_data {
 	struct reset_control *stmmac_rst;
 	struct stmmac_axi *axi;
 	int has_gmac4;
+	int has_xpcs;
+	int pcs_mode;
 	bool has_sun8i;
 	bool tso_en;
 	int mac_port_sel_speed;
-- 
1.9.1

[PATCH 6/7] net: stmmac: add xPCS platform data for EHL

[Weifeng Voon]

Enable xPCS capability inside EHL platform data.

Signed-off-by: Weifeng Voon <weifeng.voon@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index b454a97..daac2eb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -23,6 +23,7 @@
 #include <linux/dmi.h>
 
 #include "stmmac.h"
+#include "dw_xpcs.h"
 
 /*
  * This struct is used to associate PCI Function of MAC controller on a board,
@@ -128,6 +129,8 @@ static int ehl_common_data(struct pci_dev *pdev,
 	plat->clk_csr = 5;
 	plat->has_gmac = 0;
 	plat->has_gmac4 = 1;
+	plat->xpcs_phy_addr = 0x16;
+	plat->pcs_mode = AN_CTRL_PCS_MD_C37_SGMII;
 	plat->force_sf_dma_mode = 0;
 	plat->tso_en = 1;
 
@@ -211,6 +214,7 @@ static int ehl_sgmii1g_data(struct pci_dev *pdev,
 		return ret;
 
 	plat->interface = PHY_INTERFACE_MODE_SGMII;
+	plat->has_xpcs = 1;
 
 	return 0;
 }
@@ -387,6 +391,7 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
 	res.addr = pcim_iomap_table(pdev)[i];
 	res.wol_irq = pdev->irq;
 	res.irq = pdev->irq;
+	res.xpcs_irq = 0;
 
 	return stmmac_dvr_probe(&pdev->dev, plat, &res);
 }
-- 
1.9.1

[PATCH 7/7] net: stmmac: add xPCS functions for device with DWMACv5.1

[Weifeng Voon]

From: Ong Boon Leong <boon.leong.ong@intel.com>

We introduce support for driver that has v5.10 IP and is also using
xPCS as MMD. This can be easily enabled for other product that integrates
xPCS that is not using v5.00 IP.

Reviewed-by: Chuah Kim Tatt <kim.tatt.chuah@intel.com>
Reviewed-by: Voon Weifeng <weifeng.voon@intel.com>
Reviewed-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
Reviewed-by: Baoli Zhang <baoli.zhang@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 67 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/hwif.c        | 31 ++++++++++-
 drivers/net/ethernet/stmicro/stmmac/hwif.h        |  1 +
 3 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 7e5d5db..f36aa67 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -20,6 +20,7 @@
 #include <net/dsa.h>
 #include "stmmac.h"
 #include "stmmac_pcs.h"
+#include "dw_xpcs.h"
 #include "dwmac4.h"
 #include "dwmac5.h"
 
@@ -505,6 +506,34 @@ static void dwmac4_get_adv_lp(void __iomem *ioaddr, struct rgmii_adv *adv)
 	dwmac_get_adv_lp(ioaddr, GMAC_PCS_BASE, adv);
 }
 
+static void dwmac_xpcs_init(struct net_device *ndev, int pcs_mode)
+{
+	dw_xpcs_init(ndev, pcs_mode);
+}
+
+static void dwmac_xpcs_ctrl_ane(struct net_device *ndev, bool ane,
+				bool loopback)
+{
+	dw_xpcs_ctrl_ane(ndev, ane, loopback);
+}
+
+static void dwmac_xpcs_rane(struct net_device *ndev, bool restart)
+{
+	dw_xpcs_rane(ndev, restart);
+}
+
+static void dwmac_xpcs_get_adv_lp(struct net_device *ndev,
+				  struct rgmii_adv *adv)
+{
+	dw_xpcs_get_adv_lp(ndev, adv);
+}
+
+static int dwmac_xpcs_irq_status(struct net_device *ndev,
+				 struct stmmac_extra_stats *x)
+{
+	return dw_xpcs_irq_status(ndev, x);
+}
+
 /* RGMII or SMII interface */
 static void dwmac4_phystatus(void __iomem *ioaddr, struct stmmac_extra_stats *x)
 {
@@ -799,6 +828,44 @@ static void dwmac4_debug(void __iomem *ioaddr, struct stmmac_extra_stats *x,
 	.flex_pps_config = dwmac5_flex_pps_config,
 };
 
+const struct stmmac_ops dwmac510_xpcs_ops = {
+	.core_init = dwmac4_core_init,
+	.set_mac = stmmac_dwmac4_set_mac,
+	.rx_ipc = dwmac4_rx_ipc_enable,
+	.rx_queue_enable = dwmac4_rx_queue_enable,
+	.rx_queue_prio = dwmac4_rx_queue_priority,
+	.tx_queue_prio = dwmac4_tx_queue_priority,
+	.rx_queue_routing = dwmac4_rx_queue_routing,
+	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
+	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
+	.set_mtl_tx_queue_weight = dwmac4_set_mtl_tx_queue_weight,
+	.map_mtl_to_dma = dwmac4_map_mtl_dma,
+	.config_cbs = dwmac4_config_cbs,
+	.dump_regs = dwmac4_dump_regs,
+	.host_irq_status = dwmac4_irq_status,
+	.host_mtl_irq_status = dwmac4_irq_mtl_status,
+	.flow_ctrl = dwmac4_flow_ctrl,
+	.pmt = dwmac4_pmt,
+	.set_umac_addr = dwmac4_set_umac_addr,
+	.get_umac_addr = dwmac4_get_umac_addr,
+	.set_eee_mode = dwmac4_set_eee_mode,
+	.reset_eee_mode = dwmac4_reset_eee_mode,
+	.set_eee_timer = dwmac4_set_eee_timer,
+	.set_eee_pls = dwmac4_set_eee_pls,
+	.xpcs_init = dwmac_xpcs_init,
+	.xpcs_ctrl_ane = dwmac_xpcs_ctrl_ane,
+	.xpcs_rane = dwmac_xpcs_rane,
+	.xpcs_get_adv_lp = dwmac_xpcs_get_adv_lp,
+	.xpcs_irq_status = dwmac_xpcs_irq_status,
+	.debug = dwmac4_debug,
+	.set_filter = dwmac4_set_filter,
+	.safety_feat_config = dwmac5_safety_feat_config,
+	.safety_feat_irq_status = dwmac5_safety_feat_irq_status,
+	.safety_feat_dump = dwmac5_safety_feat_dump,
+	.rxp_config = dwmac5_rxp_config,
+	.flex_pps_config = dwmac5_flex_pps_config,
+};
+
 int dwmac4_setup(struct stmmac_priv *priv)
 {
 	struct mac_device_info *mac = priv->hw;
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c
index 81b966a..3725baa 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.c
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c
@@ -73,6 +73,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv *priv)
 	bool gmac;
 	bool gmac4;
 	bool xgmac;
+	bool xpcs;
 	u32 min_id;
 	const struct stmmac_regs_off regs;
 	const void *desc;
@@ -89,6 +90,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv *priv)
 		.gmac = false,
 		.gmac4 = false,
 		.xgmac = false,
+		.xpcs = false,
 		.min_id = 0,
 		.regs = {
 			.ptp_off = PTP_GMAC3_X_OFFSET,
@@ -106,6 +108,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv *priv)
 		.gmac = true,
 		.gmac4 = false,
 		.xgmac = false,
+		.xpcs = false,
 		.min_id = 0,
 		.regs = {
 			.ptp_off = PTP_GMAC3_X_OFFSET,
@@ -123,6 +126,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv *priv)
 		.gmac = false,
 		.gmac4 = true,
 		.xgmac = false,
+		.xpcs = false,
 		.min_id = 0,
 		.regs = {
 			.ptp_off = PTP_GMAC4_OFFSET,
@@ -140,6 +144,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv *priv)
 		.gmac = false,
 		.gmac4 = true,
 		.xgmac = false,
+		.xpcs = false,
 		.min_id = DWMAC_CORE_4_00,
 		.regs = {
 			.ptp_off = PTP_GMAC4_OFFSET,
@@ -157,6 +162,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv *priv)
 		.gmac = false,
 		.gmac4 = true,
 		.xgmac = false,
+		.xpcs = false,
 		.min_id = DWMAC_CORE_4_10,
 		.regs = {
 			.ptp_off = PTP_GMAC4_OFFSET,
@@ -174,6 +180,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv *priv)
 		.gmac = false,
 		.gmac4 = true,
 		.xgmac = false,
+		.xpcs = false,
 		.min_id = DWMAC_CORE_5_10,
 		.regs = {
 			.ptp_off = PTP_GMAC4_OFFSET,
@@ -191,6 +198,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv *priv)
 		.gmac = false,
 		.gmac4 = false,
 		.xgmac = true,
+		.xpcs = false,
 		.min_id = DWXGMAC_CORE_2_10,
 		.regs = {
 			.ptp_off = PTP_XGMAC_OFFSET,
@@ -204,7 +212,25 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv *priv)
 		.tc = &dwmac510_tc_ops,
 		.setup = dwxgmac2_setup,
 		.quirks = NULL,
-	},
+	}, {
+		.gmac = false,
+		.gmac4 = true,
+		.xgmac = false,
+		.xpcs = true,
+		.min_id = DWMAC_CORE_5_10,
+		.regs = {
+			.ptp_off = PTP_GMAC4_OFFSET,
+			.mmc_off = MMC_GMAC4_OFFSET,
+		},
+		.desc = &dwmac4_desc_ops,
+		.dma = &dwmac410_dma_ops,
+		.mac = &dwmac510_xpcs_ops,
+		.hwtimestamp = &stmmac_ptp,
+		.mode = &dwmac4_ring_mode_ops,
+		.tc = &dwmac510_tc_ops,
+		.setup = dwmac4_setup,
+		.quirks = NULL,
+	}
 };
 
 int stmmac_hwif_init(struct stmmac_priv *priv)
@@ -212,6 +238,7 @@ int stmmac_hwif_init(struct stmmac_priv *priv)
 	bool needs_xgmac = priv->plat->has_xgmac;
 	bool needs_gmac4 = priv->plat->has_gmac4;
 	bool needs_gmac = priv->plat->has_gmac;
+	bool needs_xpcs = priv->plat->has_xpcs;
 	const struct stmmac_hwif_entry *entry;
 	struct mac_device_info *mac;
 	bool needs_setup = true;
@@ -256,6 +283,8 @@ int stmmac_hwif_init(struct stmmac_priv *priv)
 			continue;
 		if (needs_xgmac ^ entry->xgmac)
 			continue;
+		if (needs_xpcs ^ entry->xpcs)
+			continue;
 		/* Use synopsys_id var because some setups can override this */
 		if (priv->synopsys_id < entry->min_id)
 			continue;
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index cb7eb48..485be41 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -495,6 +495,7 @@ struct stmmac_regs_off {
 extern const struct stmmac_ops dwmac410_ops;
 extern const struct stmmac_dma_ops dwmac410_dma_ops;
 extern const struct stmmac_ops dwmac510_ops;
+extern const struct stmmac_ops dwmac510_xpcs_ops;
 extern const struct stmmac_tc_ops dwmac510_tc_ops;
 extern const struct stmmac_ops dwxgmac210_ops;
 extern const struct stmmac_dma_ops dwxgmac210_dma_ops;
-- 
1.9.1

RE: [PATCH 4/7] net: stmmac: introducing support for DWC xPCS logics

[Ong, Boon Leong]

>-----Original Message-----
>From: Andrew Lunn [mailto:andrew@lunn.ch]
>Sent: Wednesday, April 24, 2019 9:42 PM
>To: Voon, Weifeng <weifeng.voon@intel.com>
>Cc: David S. Miller <davem@davemloft.net>; netdev@vger.kernel.org; linux-
>kernel@vger.kernel.org; Ong, Boon Leong <boon.leong.ong@intel.com>;
>Kweh, Hock Leong <hock.leong.kweh@intel.com>
>Subject: Re: [PATCH 4/7] net: stmmac: introducing support for DWC xPCS
>logics
>
>On Thu, Apr 25, 2019 at 01:17:18AM +0800, Weifeng Voon wrote:
>> From: Ong Boon Leong <boon.leong.ong@intel.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->|xPCS|<-->|L1 | <-- SGMII --> | External GbE |
>>   |   MAC    |         |    |    |PHY|               | 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.
>>
>> Basic functionalities for initializing xPCS and configuring auto
>> negotiation (AN), loopback, link status, AN advertisement and Link
>> Partner ability are implemented.
>>
>> xPCS interrupt handling for C37 AN complete is also implemented.
>>
>> Tested-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
>> Reviewed-by: Chuah Kim Tatt <kim.tatt.chuah@intel.com>
>> Reviewed-by: Voon Weifeng <weifeng.voon@intel.com>
>> Reviewed-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
>> Reviewed-by: Baoli Zhang <baoli.zhang@intel.com>
>> Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
>
>You need your own signed-off-by here, since you are submitting it.
>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h | 288
>++++++++++++++++++++++++++
>>  drivers/net/ethernet/stmicro/stmmac/hwif.h    |  17 ++
>>  include/linux/stmmac.h                        |   1 +
>>  3 files changed, 306 insertions(+)
>>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
>b/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
>> new file mode 100644
>> index 0000000..446b714
>> --- /dev/null
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
>> @@ -0,0 +1,288 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* dw_xpcs.h: DWC Ethernet Physical Coding Sublayer Header
>> + *
>> + * Copyright (c) 2019, Intel Corporation.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of
>MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
>License for
>> + * more details.
>> + */
>
>Since you have a SPDX line, you don't need the license boilerplate.
Thanks. Will edit. 

>
>> +#ifndef __DW_XPCS_H__
>> +#define __DW_XPCS_H__
>> +
>> +#include <linux/mdio.h>
>> +#include <linux/bitops.h>
>> +#include "stmmac.h"
>> +
>> +/* XPCS Control & MII MMD Device Addresses */
>> +#define XPCS_MDIO_CONTROL_MMD	MDIO_MMD_VEND1
>> +#define XPCS_MDIO_MII_MMD	MDIO_MMD_VEND2
>> +
>> +/* Control MMD register offsets */
>> +#define MDIO_CONTROL_MMD_CTRL		0x9	/* Control */
>> +
>> +/* Control MMD Control defines */
>> +#define MDIO_CONTROL_MMD_CTRL_MII_MMD_EN	1 /* MII MMD
>Enable */
>> +
>> +/* MII MMD registers offsets */
>> +#define MDIO_MII_MMD_CTRL		0x0	/* Control */
>> +#define MDIO_MII_MMD_ADV		0x4	/* AN Advertisement
>*/
>> +#define MDIO_MII_MMD_LPA		0x5	/* Link Partner Ability
>*/
>
>MII_BMCR, MII_ADVERTISE, MII_LPA.
Ok. Will change.

>
>> +#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 Control defines */
>> +#define MDIO_MII_MMD_CTRL_ANE		BIT(12)	/* AN Enable
>*/
>> +#define MDIO_MII_MMD_CTRL_LBE		BIT(14)	/* Loopback Enable */
>> +#define MDIO_MII_MMD_CTRL_RANE		BIT(9)	/* Restart AN
>*/
>
>BMCR_ANENABLE, BMCR_LOOPBACK, BMCR_ANENABLE
>
>If you use the standard names, developers are going to find it easier
>to understand the code.
Ok. Will change.

>
>
>> +/* MII MMD AN Advertisement & Link Partner Ability */
>> +#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 */
>> +
>> +/* MII MMD Digital Control 1 defines */
>> +#define MDIO_MII_MMD_DIGI_CTRL_1_SGMII_PHY_MD	BIT(0) /* SGMII
>PHY mode */
>> +
>> +/* 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 C37 AN SGMII Status */
>> +#define AN_STAT_C37_AN_CMPLT		BIT(0)	/* AN Complete Intr */
>> +#define AN_STAT_C37_AN_FD		BIT(1)	/* Full Duplex */
>> +#define AN_STAT_C37_AN_SPEED_SHIFT	2	/* AN Speed shift */
>> +#define AN_STAT_C37_AN_SPEED		GENMASK(3, 2)	/* AN Speed */
>> +#define AN_STAT_C37_AN_10MBPS		0x0	/* 10 Mbps */
>> +#define AN_STAT_C37_AN_100MBPS		0x1	/* 100 Mbps
>*/
>> +#define AN_STAT_C37_AN_1000MBPS		0x2	/* 1000 Mbps
>*/
>> +#define AN_STAT_C37_AN_LNKSTS		BIT(4)	/* Link Status */
>
>Is these are standardized, not proprietary, consider adding them to
>include/uapi/linux/mii.h so similar.

Yeah, it does look very standardized. However, per DW spec, they are
vendor-specific register set which uses MDIO_MMD_VEND2 to access.

>
>> +
>> +/**
>> + * dw_xpcs_init - To initialize xPCS
>> + * @ndev: network device pointer
>> + * @mode: PCS mode
>> + * Description: this is to initialize xPCS
>> + */
>> +static inline void dw_xpcs_init(struct net_device *ndev, int pcs_mode)
>> +{
>> +	struct stmmac_priv *priv = netdev_priv(ndev);
>> +	int xpcs_phy_addr = priv->plat->xpcs_phy_addr;
>> +
>> +	/* Set SGMII PHY mode control */
>> +	u16 phydata = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
>> +				    (MII_ADDR_C45 |
>> +				     (XPCS_MDIO_MII_MMD <<
>> +				      MII_DEVADDR_C45_SHIFT) |
>> +				     MDIO_MII_MMD_DIGITAL_CTRL_1));
>> +
>
>Why cast to u16? It return int for a reason.
Will change to u32 here.  

>
>> +	phydata |= MDIO_MII_MMD_DIGI_CTRL_1_SGMII_PHY_MD;
>> +
>> +	mdiobus_write(priv->mii, xpcs_phy_addr,
>> +		      (MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
>> +		       MII_DEVADDR_C45_SHIFT) |
>MDIO_MII_MMD_DIGITAL_CTRL_1),
>> +		      (int)phydata);
>
>Maybe add helpers xpcs_read() and xpcs_write()?
Good suggestion. Thanks for your review. 

>
>      Andrew

Re: [PATCH 4/7] net: stmmac: introducing support for DWC xPCS logics

[Andrew Lunn]

> >> +/* MII MMD AN Status defines for C37 AN SGMII Status */
> >> +#define AN_STAT_C37_AN_CMPLT		BIT(0)	/* AN Complete Intr */
> >> +#define AN_STAT_C37_AN_FD		BIT(1)	/* Full Duplex */
> >> +#define AN_STAT_C37_AN_SPEED_SHIFT	2	/* AN Speed shift */
> >> +#define AN_STAT_C37_AN_SPEED		GENMASK(3, 2)	/* AN Speed */
> >> +#define AN_STAT_C37_AN_10MBPS		0x0	/* 10 Mbps */
> >> +#define AN_STAT_C37_AN_100MBPS		0x1	/* 100 Mbps
> >*/
> >> +#define AN_STAT_C37_AN_1000MBPS		0x2	/* 1000 Mbps
> >*/
> >> +#define AN_STAT_C37_AN_LNKSTS		BIT(4)	/* Link Status */
> >
> >Is these are standardized, not proprietary, consider adding them to
> >include/uapi/linux/mii.h so similar.
> 
> Yeah, it does look very standardized. However, per DW spec, they are
> vendor-specific register set which uses MDIO_MMD_VEND2 to access.

Maybe you can go get the 802.3 standard and check section 37. If these
registers are the same, they could still be placed in mii.h. What MMD
you use is a different matter.

  Andrew

RE: [PATCH 4/7] net: stmmac: introducing support for DWC xPCS logics

[Ong, Boon Leong]

>-----Original Message-----
>From: Andrew Lunn [mailto:andrew@lunn.ch]
>Sent: Thursday, April 25, 2019 11:21 AM
>To: Ong, Boon Leong <boon.leong.ong@intel.com>
>Cc: David S. Miller <davem@davemloft.net>; netdev@vger.kernel.org; linux-
>kernel@vger.kernel.org; Kweh, Hock Leong <hock.leong.kweh@intel.com>;
>Voon, Weifeng <weifeng.voon@intel.com>
>Subject: Re: [PATCH 4/7] net: stmmac: introducing support for DWC xPCS
>logics
>
>> >> +/* MII MMD AN Status defines for C37 AN SGMII Status */
>> >> +#define AN_STAT_C37_AN_CMPLT		BIT(0)	/* AN
>Complete Intr */
>> >> +#define AN_STAT_C37_AN_FD		BIT(1)	/* Full Duplex */
>> >> +#define AN_STAT_C37_AN_SPEED_SHIFT	2	/* AN Speed shift */
>> >> +#define AN_STAT_C37_AN_SPEED		GENMASK(3, 2)	/* AN
>Speed */
>> >> +#define AN_STAT_C37_AN_10MBPS		0x0	/* 10 Mbps */
>> >> +#define AN_STAT_C37_AN_100MBPS		0x1	/* 100 Mbps
>> >*/
>> >> +#define AN_STAT_C37_AN_1000MBPS		0x2	/* 1000 Mbps
>> >*/
>> >> +#define AN_STAT_C37_AN_LNKSTS		BIT(4)	/* Link Status
>*/
>> >
>> >Is these are standardized, not proprietary, consider adding them to
>> >include/uapi/linux/mii.h so similar.
>>
>> Yeah, it does look very standardized. However, per DW spec, they are
>> vendor-specific register set which uses MDIO_MMD_VEND2 to access.
>
>Maybe you can go get the 802.3 standard and check section 37. If these
>registers are the same, they could still be placed in mii.h. What MMD
>you use is a different matter.
>
I cross-checked on 802.3 section 37.2.5.1 Management registers, the spec only
talks about BMSR and extended status. I also checked on 802.3 section 45.2
MDIO interface registers, section 45.2.7 AN registers, section 45.2.9 &
45.2.10 Vendor specific MMD1 & MMD2, there are no field format that
matches the above bit fields. So, it should be reasonable to park these #defines
in this local file for now.  

Thanks.
Boon Leong

RE: [PATCH 0/7] net: stmmac: enable EHL SGMII

[Voon, Weifeng]

> From: "Voon, Weifeng" <weifeng.voon@intel.com@intel.com>
> 
> This patch-set is to enable Ethernet controller (DW Ethernet QoS and DW
> Ethernet PCS) with SGMII interface in Elkhart Lake. The DW Ethernet PCS is
> the Physical Coding Sublayer that is between Ethernet MAC and PHY and
> uses MDIO Clause-45 as Communication.
> 
> Kweh Hock Leong (1):
>   net: stmmac: enable clause 45 mdio support
> 
> Ong Boon Leong (3):
>   net: stmmac: introducing support for DWC xPCS logics
>   net: stmmac: add xpcs function hooks into main driver and ethtool
>   net: stmmac: add xPCS functions for device with DWMACv5.1
> 
> Weifeng Voon (3):
>   net: stmmac: add EHL SGMII 1Gbps platform data and PCI ID
>   net: stmmac: dma channel control register need to be init first
>   net: stmmac: add xPCS platform data for EHL
> 
>  drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h      | 288
> +++++++++++++++++++++
>  drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c  |  67 +++++
>  drivers/net/ethernet/stmicro/stmmac/hwif.c         |  31 ++-
>  drivers/net/ethernet/stmicro/stmmac/hwif.h         |  18 ++
>  drivers/net/ethernet/stmicro/stmmac/stmmac.h       |   2 +
>  .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   |  14 +
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |  58 ++++-
> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c  |  36 ++-
>  drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c   | 108 ++++++++
>  include/linux/phy.h                                |   2 +
>  include/linux/stmmac.h                             |   3 +
>  11 files changed, 616 insertions(+), 11 deletions(-)  create mode 100644
> drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
> 
> --
> 1.9.1

++ stmmac maintainers and c45 experts

RE: [PATCH 1/7] net: stmmac: add EHL SGMII 1Gbps platform data and PCI ID

[Voon, Weifeng]

> Added EHL SGMII 1Gbps PCI ID. Different MII and speed will have different
> PCI ID.
> For EHL, default TX and RX FIFO size is set to 32KB. This is because the FIFO
> size advertised in the HW features is not the same as the HW
> implementation. The TX FIFO is shared among all all the TX queues and the
> RX FIFO is also shared among all the RX queues.
> 
> Signed-off-by: Weifeng Voon <weifeng.voon@intel.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 103
> +++++++++++++++++++++++
>  1 file changed, 103 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index d819e8e..b454a97 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -118,6 +118,107 @@ static int stmmac_default_data(struct pci_dev
> *pdev,
>  	.setup = stmmac_default_data,
>  };
> 
> +static int ehl_common_data(struct pci_dev *pdev,
> +			   struct plat_stmmacenet_data *plat) {
> +	int i;
> +
> +	plat->bus_id = 1;
> +	plat->phy_addr = 0;
> +	plat->clk_csr = 5;
> +	plat->has_gmac = 0;
> +	plat->has_gmac4 = 1;
> +	plat->force_sf_dma_mode = 0;
> +	plat->tso_en = 1;
> +
> +	plat->rx_queues_to_use = 8;
> +	plat->tx_queues_to_use = 8;
> +	plat->rx_sched_algorithm = MTL_RX_ALGORITHM_SP;
> +
> +	for (i = 0; i < plat->rx_queues_to_use; i++) {
> +		plat->rx_queues_cfg[i].mode_to_use = MTL_QUEUE_DCB;
> +		plat->rx_queues_cfg[i].chan = i;
> +
> +		/* Disable Priority config by default */
> +		plat->tx_queues_cfg[i].use_prio = false;
> +
> +		/* Disable RX queues routing by default */
> +		plat->rx_queues_cfg[i].pkt_route = 0x0;
> +	}
> +
> +	for (i = 0; i < plat->tx_queues_to_use; i++) {
> +		plat->tx_queues_cfg[i].mode_to_use = MTL_QUEUE_DCB;
> +
> +		/* Disable Priority config by default */
> +		plat->tx_queues_cfg[i].use_prio = false;
> +	}
> +
> +	plat->tx_sched_algorithm = MTL_TX_ALGORITHM_WRR;
> +	plat->tx_queues_cfg[0].weight = 0x09;
> +	plat->tx_queues_cfg[1].weight = 0x0A;
> +	plat->tx_queues_cfg[2].weight = 0x0B;
> +	plat->tx_queues_cfg[3].weight = 0x0C;
> +	plat->tx_queues_cfg[4].weight = 0x0D;
> +	plat->tx_queues_cfg[5].weight = 0x0E;
> +	plat->tx_queues_cfg[6].weight = 0x0F;
> +	plat->tx_queues_cfg[7].weight = 0x10;
> +
> +	plat->mdio_bus_data->phy_reset = NULL;
> +	plat->mdio_bus_data->phy_mask = 0;
> +
> +	plat->dma_cfg->pbl = 32;
> +	plat->dma_cfg->pblx8 = true;
> +	plat->dma_cfg->fixed_burst = 0;
> +	plat->dma_cfg->mixed_burst = 0;
> +	plat->dma_cfg->aal = 0;
> +
> +	plat->axi = devm_kzalloc(&pdev->dev, sizeof(*plat->axi),
> +				 GFP_KERNEL);
> +	if (!plat->axi)
> +		return -ENOMEM;
> +	plat->axi->axi_lpi_en = 0;
> +	plat->axi->axi_xit_frm = 0;
> +	plat->axi->axi_wr_osr_lmt = 0;
> +	plat->axi->axi_rd_osr_lmt = 2;
> +	plat->axi->axi_blen[0] = 4;
> +	plat->axi->axi_blen[1] = 8;
> +	plat->axi->axi_blen[2] = 16;
> +
> +	/* Set default value for multicast hash bins */
> +	plat->multicast_filter_bins = HASH_TABLE_SIZE;
> +
> +	/* Set default value for unicast filter entries */
> +	plat->unicast_filter_entries = 1;
> +
> +	/* Set the maxmtu to a default of JUMBO_LEN */
> +	plat->maxmtu = JUMBO_LEN;
> +
> +	plat->tx_fifo_size = 32768;
> +	plat->rx_fifo_size = 32768;
> +
> +	return 0;
> +}
> +
> +static int ehl_sgmii1g_data(struct pci_dev *pdev,
> +			    struct plat_stmmacenet_data *plat) {
> +	int ret;
> +
> +	/* Set common default data first */
> +	ret = ehl_common_data(pdev, plat);
> +
> +	if (ret)
> +		return ret;
> +
> +	plat->interface = PHY_INTERFACE_MODE_SGMII;
> +
> +	return 0;
> +}
> +
> +static struct stmmac_pci_info ehl_sgmii1g_pci_info = {
> +	.setup = ehl_sgmii1g_data,
> +};
> +
>  static const struct stmmac_pci_func_data galileo_stmmac_func_data[] = {
>  	{
>  		.func = 6,
> @@ -355,6 +456,7 @@ static int __maybe_unused
> stmmac_pci_resume(struct device *dev)
> 
>  #define STMMAC_QUARK_ID  0x0937
>  #define STMMAC_DEVICE_ID 0x1108
> +#define STMMAC_EHL_SGMII1G_ID   0x4b31
> 
>  #define STMMAC_DEVICE(vendor_id, dev_id, info)	{	\
>  	PCI_VDEVICE(vendor_id, dev_id),			\
> @@ -365,6 +467,7 @@ static int __maybe_unused
> stmmac_pci_resume(struct device *dev)
>  	STMMAC_DEVICE(STMMAC, STMMAC_DEVICE_ID,
> stmmac_pci_info),
>  	STMMAC_DEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_MAC,
> stmmac_pci_info),
>  	STMMAC_DEVICE(INTEL, STMMAC_QUARK_ID, quark_pci_info),
> +	STMMAC_DEVICE(INTEL, STMMAC_EHL_SGMII1G_ID,
> ehl_sgmii1g_pci_info),
>  	{}
>  };
> 
> --
> 1.9.1

++ stmmac maintainers and c45 experts

RE: [PATCH 3/7] net: stmmac: dma channel control register need to be init first

[Voon, Weifeng]

> stmmac_init_chan() needs to be called before stmmac_init_rx_chan() and
> stmmac_init_tx_chan(). This is because if PBLx8 is to be used,
> "DMA_CH(#i)_Control.PBLx8" needs to be set before programming
> "DMA_CH(#i)_TX_Control.TxPBL" and "DMA_CH(#i)_RX_Control.RxPBL".
> 
> Reviewed-by: Zhang, Baoli <baoli.zhang@intel.com>
> Signed-off-by: Weifeng Voon <weifeng.voon@intel.com>
> Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index a26e36d..ec031e3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2208,6 +2208,10 @@ static int stmmac_init_dma_engine(struct
> stmmac_priv *priv)
>  	if (priv->plat->axi)
>  		stmmac_axi(priv, priv->ioaddr, priv->plat->axi);
> 
> +	/* DMA CSR Channel configuration */
> +	for (chan = 0; chan < dma_csr_ch; chan++)
> +		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg,
> chan);
> +
>  	/* DMA RX Channel Configuration */
>  	for (chan = 0; chan < rx_channels_count; chan++) {
>  		rx_q = &priv->rx_queue[chan];
> @@ -2233,10 +2237,6 @@ static int stmmac_init_dma_engine(struct
> stmmac_priv *priv)
>  				       tx_q->tx_tail_addr, chan);
>  	}
> 
> -	/* DMA CSR Channel configuration */
> -	for (chan = 0; chan < dma_csr_ch; chan++)
> -		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg,
> chan);
> -
>  	return ret;
>  }
> 
> --
> 1.9.1

++ stmmac maintainers and c45 experts

RE: [PATCH 4/7] net: stmmac: introducing support for DWC xPCS logics

[Voon, Weifeng]

> From: Ong Boon Leong <boon.leong.ong@intel.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->|xPCS|<-->|L1 | <-- SGMII --> | External GbE |
>   |   MAC    |         |    |    |PHY|               | 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.
> 
> Basic functionalities for initializing xPCS and configuring auto negotiation (AN),
> loopback, link status, AN advertisement and Link Partner ability are
> implemented.
> 
> xPCS interrupt handling for C37 AN complete is also implemented.
> 
> Tested-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
> Reviewed-by: Chuah Kim Tatt <kim.tatt.chuah@intel.com>
> Reviewed-by: Voon Weifeng <weifeng.voon@intel.com>
> Reviewed-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
> Reviewed-by: Baoli Zhang <baoli.zhang@intel.com>
> Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h | 288
> ++++++++++++++++++++++++++
>  drivers/net/ethernet/stmicro/stmmac/hwif.h    |  17 ++
>  include/linux/stmmac.h                        |   1 +
>  3 files changed, 306 insertions(+)
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
> b/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
> new file mode 100644
> index 0000000..446b714
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
> @@ -0,0 +1,288 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* dw_xpcs.h: DWC Ethernet Physical Coding Sublayer Header
> + *
> + * Copyright (c) 2019, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> +it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but
> +WITHOUT
> + * ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY
> +or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> +License for
> + * more details.
> + */
> +#ifndef __DW_XPCS_H__
> +#define __DW_XPCS_H__
> +
> +#include <linux/mdio.h>
> +#include <linux/bitops.h>
> +#include "stmmac.h"
> +
> +/* XPCS Control & MII MMD Device Addresses */
> +#define XPCS_MDIO_CONTROL_MMD	MDIO_MMD_VEND1
> +#define XPCS_MDIO_MII_MMD	MDIO_MMD_VEND2
> +
> +/* Control MMD register offsets */
> +#define MDIO_CONTROL_MMD_CTRL		0x9	/* Control */
> +
> +/* Control MMD Control defines */
> +#define MDIO_CONTROL_MMD_CTRL_MII_MMD_EN	1 /* MII MMD
> Enable */
> +
> +/* MII MMD registers offsets */
> +#define MDIO_MII_MMD_CTRL		0x0	/* Control */
> +#define MDIO_MII_MMD_ADV		0x4	/* AN Advertisement
> */
> +#define MDIO_MII_MMD_LPA		0x5	/* Link Partner Ability
> */
> +#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 Control defines */
> +#define MDIO_MII_MMD_CTRL_ANE		BIT(12)	/* AN Enable */
> +#define MDIO_MII_MMD_CTRL_LBE		BIT(14)	/* Loopback Enable
> */
> +#define MDIO_MII_MMD_CTRL_RANE		BIT(9)	/* Restart AN
> */
> +
> +/* MII MMD AN Advertisement & Link Partner Ability */
> +#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 */
> +
> +/* MII MMD Digital Control 1 defines */
> +#define MDIO_MII_MMD_DIGI_CTRL_1_SGMII_PHY_MD	BIT(0) /*
> SGMII PHY mode */
> +
> +/* 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 C37 AN SGMII Status */
> +#define AN_STAT_C37_AN_CMPLT		BIT(0)	/* AN Complete Intr
> */
> +#define AN_STAT_C37_AN_FD		BIT(1)	/* Full Duplex */
> +#define AN_STAT_C37_AN_SPEED_SHIFT	2	/* AN Speed shift */
> +#define AN_STAT_C37_AN_SPEED		GENMASK(3, 2)	/* AN
> Speed */
> +#define AN_STAT_C37_AN_10MBPS		0x0	/* 10 Mbps */
> +#define AN_STAT_C37_AN_100MBPS		0x1	/* 100 Mbps */
> +#define AN_STAT_C37_AN_1000MBPS		0x2	/* 1000 Mbps
> */
> +#define AN_STAT_C37_AN_LNKSTS		BIT(4)	/* Link Status */
> +
> +/**
> + * dw_xpcs_init - To initialize xPCS
> + * @ndev: network device pointer
> + * @mode: PCS mode
> + * Description: this is to initialize xPCS  */ static inline void
> +dw_xpcs_init(struct net_device *ndev, int pcs_mode) {
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	int xpcs_phy_addr = priv->plat->xpcs_phy_addr;
> +
> +	/* Set SGMII PHY mode control */
> +	u16 phydata = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
> +				    (MII_ADDR_C45 |
> +				     (XPCS_MDIO_MII_MMD <<
> +				      MII_DEVADDR_C45_SHIFT) |
> +				     MDIO_MII_MMD_DIGITAL_CTRL_1));
> +
> +	phydata |= MDIO_MII_MMD_DIGI_CTRL_1_SGMII_PHY_MD;
> +
> +	mdiobus_write(priv->mii, xpcs_phy_addr,
> +		      (MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
> +		       MII_DEVADDR_C45_SHIFT) |
> MDIO_MII_MMD_DIGITAL_CTRL_1),
> +		      (int)phydata);
> +
> +	/* Set PHY side SGMII, PCS Mode & Enable C37 AN complete
> interrupt */
> +	phydata = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
> +				    (MII_ADDR_C45 |
> +				     (XPCS_MDIO_MII_MMD <<
> +				      MII_DEVADDR_C45_SHIFT) |
> +				     MDIO_MII_MMD_AN_CTRL));
> +
> +	phydata &= ~MDIO_MII_MMD_AN_CTRL_PCS_MD;
> +	phydata |= (((pcs_mode <<
> MDIO_MII_MMD_AN_CTRL_PCS_MD_SHIFT) &
> +		    MDIO_MII_MMD_AN_CTRL_PCS_MD) |
> +		    (AN_CTRL_TX_CONF_PHY_SIDE_SGMII <<
> +		    MDIO_MII_MMD_AN_CTRL_TX_CONFIG_SHIFT) |
> +		    MDIO_MII_MMD_AN_CTRL_AN_INTR_EN);
> +
> +	mdiobus_write(priv->mii, xpcs_phy_addr,
> +		      (MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
> +		       MII_DEVADDR_C45_SHIFT) |
> MDIO_MII_MMD_AN_CTRL),
> +		      (int)phydata);
> +}
> +
> +/**
> + * dw_xpcs_rane - To restart Auto Negotiation (AN)
> + * @ndev: network device pointer
> + * @restart: to restart AN
> + * Description: this is to restart AN.
> + */
> +static inline void dw_xpcs_rane(struct net_device *ndev, bool restart)
> +{
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	int xpcs_phy_addr = priv->plat->xpcs_phy_addr;
> +
> +	u16 phydata = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
> +				(MII_ADDR_C45 | (XPCS_MDIO_MII_MMD
> <<
> +				 MII_DEVADDR_C45_SHIFT) |
> +				 MDIO_MII_MMD_CTRL));
> +
> +	if (restart)
> +		phydata |= MDIO_MII_MMD_CTRL_RANE;
> +
> +	mdiobus_write(priv->mii, xpcs_phy_addr,
> +		      (MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
> +		       MII_DEVADDR_C45_SHIFT) | MDIO_MII_MMD_CTRL),
> +		      (int)phydata);
> +}
> +
> +/**
> + * dw_xpcs_ctrl_ane - To program the MII MMD Control Register.
> + * @ndev: network device pointer
> + * @ane: to enable the Auto Negotiation
> + * @loopback: to cause the PHY to loopback Tx data into Rx path.
> + * Description: this is the main function to configure the MII MMD
> + * control register and init the AN Enable and select loopback.
> + */
> +static inline void dw_xpcs_ctrl_ane(struct net_device *ndev, bool ane,
> +				    bool loopback)
> +{
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	int xpcs_phy_addr = priv->plat->xpcs_phy_addr;
> +
> +	u16 phydata = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
> +				(MII_ADDR_C45 | (XPCS_MDIO_MII_MMD
> <<
> +				 MII_DEVADDR_C45_SHIFT) |
> +				 MDIO_MII_MMD_CTRL));
> +
> +	if (ane)
> +		phydata |= (MDIO_MII_MMD_CTRL_ANE |
> MDIO_MII_MMD_CTRL_RANE);
> +
> +	if (loopback)
> +		phydata |= MDIO_MII_MMD_CTRL_LBE;
> +
> +	mdiobus_write(priv->mii, xpcs_phy_addr,
> +		      (MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
> +		       MII_DEVADDR_C45_SHIFT) | MDIO_MII_MMD_CTRL),
> +		      (int)phydata);
> +}
> +
> +/**
> + * dw_xpcs_get_adv_lp - Get AN Advertisement and Link Partner Ability
> + * @ndev: network device pointer
> + * @adv_lp: structure to store the adv, lp status
> + * Description: this is to expose the Auto Negotiation Advertisement
> +and
> + * Link partner ability status to ethtool support.
> + */
> +static inline void dw_xpcs_get_adv_lp(struct net_device *ndev,
> +				      struct rgmii_adv *adv_lp)
> +{
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	int xpcs_phy_addr = priv->plat->xpcs_phy_addr;
> +
> +	/* AN Advertisement Ability */
> +	u16 value = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
> +				(MII_ADDR_C45 | (XPCS_MDIO_MII_MMD
> <<
> +				 MII_DEVADDR_C45_SHIFT) |
> +				 MDIO_MII_MMD_ADV));
> +
> +	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 */
> +	value = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
> +				  (MII_ADDR_C45 | (XPCS_MDIO_MII_MMD
> <<
> +				   MII_DEVADDR_C45_SHIFT) |
> +				   MDIO_MII_MMD_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);
> +}
> +
> +/**
> + * dw_xpcs_get_linkstatus - Get Link Status
> + * @an_stat: C37 AN status value
> + * @x: stmmac extra status
> + * Description: this is to read the link extra status from <C37 AN
> +SGMII
> + * status> field of MII MMD AN Status register.
> + */
> +static inline void dw_xpcs_get_linkstatus(u16 an_stat,
> +					  struct stmmac_extra_stats *x)
> +{
> +	/* Check the link status */
> +	if (an_stat & AN_STAT_C37_AN_LNKSTS) {
> +		int speed_value;
> +
> +		x->pcs_link = 1;
> +
> +		speed_value = ((an_stat & AN_STAT_C37_AN_SPEED) >>
> +				AN_STAT_C37_AN_SPEED_SHIFT);
> +		if (speed_value == AN_STAT_C37_AN_1000MBPS)
> +			x->pcs_speed = SPEED_1000;
> +		else if (speed_value == AN_STAT_C37_AN_100MBPS)
> +			x->pcs_speed = SPEED_100;
> +		else
> +			x->pcs_speed = SPEED_10;
> +
> +		if (an_stat & AN_STAT_C37_AN_FD)
> +			x->pcs_duplex = 1;
> +		else
> +			x->pcs_duplex = 0;
> +
> +		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");
> +	}
> +}
> +
> +/**
> + * dw_xpcs_irq_status - Get xPCS IRQ Status
> + * @ndev: network device pointer
> + * @x: stmmac extra status
> + * Description: this is to read the xPCS IRQ status.
> + */
> +static inline int dw_xpcs_irq_status(struct net_device *ndev,
> +				     struct stmmac_extra_stats *x)
> +{
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	int xpcs_phy_addr = priv->plat->xpcs_phy_addr;
> +	int ret = IRQ_NONE;
> +
> +	/* C37 AN status */
> +	u16 an_stat = (u16)mdiobus_read(priv->mii, xpcs_phy_addr,
> +					(MII_ADDR_C45 |
> +					 (XPCS_MDIO_MII_MMD <<
> +					  MII_DEVADDR_C45_SHIFT) |
> +					 MDIO_MII_MMD_AN_STAT));
> +
> +	if (an_stat & AN_STAT_C37_AN_CMPLT) {
> +		x->irq_pcs_ane_n++;
> +		dw_xpcs_get_linkstatus(an_stat, x);
> +
> +		/* Clear C37 AN complete status by writing zero */
> +		mdiobus_write(priv->mii, xpcs_phy_addr,
> +			      (MII_ADDR_C45 | (XPCS_MDIO_MII_MMD <<
> +			       MII_DEVADDR_C45_SHIFT) |
> +			       MDIO_MII_MMD_AN_STAT),
> +			      0);
> +		ret = IRQ_HANDLED;
> +	}
> +
> +	return ret;
> +}
> +#endif /* __DW_XPCS_H__ */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index 5bb0023..cb7eb48 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> @@ -310,6 +310,13 @@ struct stmmac_ops {
>  			     bool loopback);
>  	void (*pcs_rane)(void __iomem *ioaddr, bool restart);
>  	void (*pcs_get_adv_lp)(void __iomem *ioaddr, struct rgmii_adv
> *adv);
> +	/* xPCS calls */
> +	void (*xpcs_init)(struct net_device *ndev, int pcs_mode);
> +	void (*xpcs_ctrl_ane)(struct net_device *ndev, bool ane, bool
> loopback);
> +	void (*xpcs_rane)(struct net_device *ndev, bool restart);
> +	void (*xpcs_get_adv_lp)(struct net_device *ndev, struct rgmii_adv
> *adv);
> +	int (*xpcs_irq_status)(struct net_device *ndev,
> +			       struct stmmac_extra_stats *x);
>  	/* Safety Features */
>  	int (*safety_feat_config)(void __iomem *ioaddr, unsigned int asp);
>  	int (*safety_feat_irq_status)(struct net_device *ndev, @@ -382,6
> +389,16 @@ struct stmmac_ops {
>  	stmmac_do_void_callback(__priv, mac, pcs_rane, __args)  #define
> stmmac_pcs_get_adv_lp(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, mac, pcs_get_adv_lp, __args)
> +#define stmmac_xpcs_init(__priv, __args...) \
> +	stmmac_do_void_callback(__priv, mac, xpcs_init, __args) #define
> +stmmac_xpcs_ctrl_ane(__priv, __args...) \
> +	stmmac_do_void_callback(__priv, mac, xpcs_ctrl_ane, __args)
> #define
> +stmmac_xpcs_rane(__priv, __args...) \
> +	stmmac_do_void_callback(__priv, mac, xpcs_rane, __args) #define
> +stmmac_xpcs_get_adv_lp(__priv, __args...) \
> +	stmmac_do_void_callback(__priv, mac, xpcs_get_adv_lp, __args)
> #define
> +stmmac_xpcs_irq_status(__priv, __args...) \
> +	stmmac_do_callback(__priv, mac, xpcs_irq_status, __args)
>  #define stmmac_safety_feat_config(__priv, __args...) \
>  	stmmac_do_callback(__priv, mac, safety_feat_config, __args)
> #define stmmac_safety_feat_irq_status(__priv, __args...) \ diff --git
> a/include/linux/stmmac.h b/include/linux/stmmac.h index 4335bd7..b00e795
> 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;
> --
> 1.9.1

++ stmmac maintainers and c45 experts

RE: [PATCH 5/7] net: stmmac: add xpcs function hooks into main driver and ethtool

[Voon, Weifeng]

> From: Ong Boon Leong <boon.leong.ong@intel.com>
> 
> With xPCS functions now ready, we add them into the main driver and
> ethtool logics. To differentiate from EQoS MAC PCS and DWC Ethernet xPCS,
> we introduce 'has_xpcs' in platform data as a mean to indicate whether GBE
> controller includes xPCS or not.
> 
> To support platform-specific C37 AN PCS mode selection for MII MMD, we
> introduce 'pcs_mode' in platform data.
> 
> The basic framework for xPCS interrupt handling is implemented too.
> 
> Reviewed-by: Chuah Kim Tatt <kim.tatt.chuah@intel.com>
> Reviewed-by: Voon Weifeng <weifeng.voon@intel.com>
> Reviewed-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
> Reviewed-by: Baoli Zhang <baoli.zhang@intel.com>
> Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac.h       |  2 +
>  .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   | 14 ++++++
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 50
> ++++++++++++++++++++++
>  include/linux/stmmac.h                             |  2 +
>  4 files changed, 68 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index dd95d95..0b8460a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -36,6 +36,7 @@ struct stmmac_resources {
>  	const char *mac;
>  	int wol_irq;
>  	int lpi_irq;
> +	int xpcs_irq;
>  	int irq;
>  };
> 
> @@ -168,6 +169,7 @@ struct stmmac_priv {
>  	int clk_csr;
>  	struct timer_list eee_ctrl_timer;
>  	int lpi_irq;
> +	int xpcs_irq;
>  	int eee_enabled;
>  	int eee_active;
>  	int tx_lpi_timer;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> index 3c749c3..0c146b3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> @@ -392,6 +392,10 @@ static int stmmac_ethtool_get_link_ksettings(struct
> net_device *dev,
> 
>  		mutex_lock(&priv->lock);
>  		stmmac_pcs_ctrl_ane(priv, priv->ioaddr, 1, priv->hw->ps, 0);
> +
> +		if (priv->plat->has_xpcs)
> +			stmmac_xpcs_ctrl_ane(priv, dev, 1, 0);
> +
>  		mutex_unlock(&priv->lock);
> 
>  		return 0;
> @@ -457,6 +461,11 @@ static void stmmac_ethtool_gregs(struct net_device
> *dev,
>  		pause->autoneg = 1;
>  		if (!adv_lp.pause)
>  			return;
> +	} else if (priv->plat->has_xpcs &&
> +		   !stmmac_xpcs_get_adv_lp(priv, netdev, &adv_lp)) {
> +		pause->autoneg = 1;
> +		if (!adv_lp.pause)
> +			return;
>  	} else {
>  		if (!linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
>  				       netdev->phydev->supported) || @@ -
> 488,6 +497,11 @@ static void stmmac_ethtool_gregs(struct net_device *dev,
>  		pause->autoneg = 1;
>  		if (!adv_lp.pause)
>  			return -EOPNOTSUPP;
> +	} else if (priv->plat->has_xpcs &&
> +		   !stmmac_xpcs_get_adv_lp(priv, netdev, &adv_lp)) {
> +		pause->autoneg = 1;
> +		if (!adv_lp.pause)
> +			return -EOPNOTSUPP;
>  	} else {
>  		if (!linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
>  				       phy->supported) ||
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index ec031e3..d6e315f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -112,6 +112,7 @@
>  MODULE_PARM_DESC(chain_mode, "To use chain instead of ring mode");
> 
>  static irqreturn_t stmmac_interrupt(int irq, void *dev_id);
> +static irqreturn_t xpcs_interrupt(int irq, void *dev_id);
> 
>  #ifdef CONFIG_DEBUG_FS
>  static int stmmac_init_fs(struct net_device *dev); @@ -2538,6 +2539,10 @@
> static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
>  	/* Initialize MTL*/
>  	stmmac_mtl_configuration(priv);
> 
> +	/* Initialize the xPCS PHY */
> +	if (priv->plat->has_xpcs)
> +		stmmac_xpcs_init(priv, dev, priv->plat->pcs_mode);
> +
>  	/* Initialize Safety Features */
>  	stmmac_safety_feat_configuration(priv);
> 
> @@ -2579,6 +2584,9 @@ static int stmmac_hw_setup(struct net_device
> *dev, bool init_ptp)
>  	if (priv->hw->pcs)
>  		stmmac_pcs_ctrl_ane(priv, priv->hw, 1, priv->hw->ps, 0);
> 
> +	if (priv->plat->has_xpcs)
> +		stmmac_xpcs_ctrl_ane(priv, dev, 1, 0);
> +
>  	/* set TX and RX rings length */
>  	stmmac_set_rings_length(priv);
> 
> @@ -2696,11 +2704,27 @@ static int stmmac_open(struct net_device *dev)
>  		}
>  	}
> 
> +	/* xPCS IRQ line */
> +	if (priv->xpcs_irq > 0) {
> +		ret = request_irq(priv->xpcs_irq, xpcs_interrupt,
> IRQF_SHARED,
> +				  dev->name, dev);
> +		if (unlikely(ret < 0)) {
> +			netdev_err(priv->dev,
> +				   "%s: ERROR: allocating the xPCS IRQ %d
> (%d)\n",
> +				   __func__, priv->xpcs_irq, ret);
> +			goto xpcsirq_error;
> +		}
> +	}
> +
>  	stmmac_enable_all_queues(priv);
>  	stmmac_start_all_queues(priv);
> 
>  	return 0;
> 
> +xpcsirq_error:
> +	if (priv->lpi_irq > 0)
> +		free_irq(priv->lpi_irq, dev);
> +
>  lpiirq_error:
>  	if (priv->wol_irq != dev->irq)
>  		free_irq(priv->wol_irq, dev);
> @@ -3774,6 +3798,31 @@ static irqreturn_t stmmac_interrupt(int irq, void
> *dev_id)
>  	return IRQ_HANDLED;
>  }
> 
> +/**
> + *  xPCS_interrupt - xPCS ISR
> + *  @irq: interrupt number.
> + *  @dev_id: to pass the net device pointer.
> + *  Description: this is the xPCS interrupt service routine.
> + */
> +static irqreturn_t xpcs_interrupt(int irq, void *dev_id) {
> +	irqreturn_t ret = IRQ_NONE;
> +	struct net_device *ndev = (struct net_device *)dev_id;
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +
> +	if (unlikely(!ndev)) {
> +		netdev_err(priv->dev, "%s: invalid dev pointer\n",
> +			   __func__);
> +		return ret;
> +	}
> +
> +	/* To handle xPCS interrupts */
> +	if (priv->plat->has_xpcs)
> +		ret = stmmac_xpcs_irq_status(priv, ndev, &priv->xstats);
> +
> +	return ret;
> +}
> +
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  /* Polling receive - used by NETCONSOLE and other diagnostic tools
>   * to allow network I/O with interrupts disabled.
> @@ -4263,6 +4312,7 @@ int stmmac_dvr_probe(struct device *device,
>  	priv->dev->irq = res->irq;
>  	priv->wol_irq = res->wol_irq;
>  	priv->lpi_irq = res->lpi_irq;
> +	priv->xpcs_irq = res->xpcs_irq;
> 
>  	if (res->mac)
>  		memcpy(priv->dev->dev_addr, res->mac, ETH_ALEN); diff --
> git a/include/linux/stmmac.h b/include/linux/stmmac.h index
> b00e795..2adb78c 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -189,6 +189,8 @@ struct plat_stmmacenet_data {
>  	struct reset_control *stmmac_rst;
>  	struct stmmac_axi *axi;
>  	int has_gmac4;
> +	int has_xpcs;
> +	int pcs_mode;
>  	bool has_sun8i;
>  	bool tso_en;
>  	int mac_port_sel_speed;
> --
> 1.9.1

++ stmmac maintainers and c45 experts

RE: [PATCH 6/7] net: stmmac: add xPCS platform data for EHL

[Voon, Weifeng]

> Enable xPCS capability inside EHL platform data.
> 
> Signed-off-by: Weifeng Voon <weifeng.voon@intel.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index b454a97..daac2eb 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -23,6 +23,7 @@
>  #include <linux/dmi.h>
> 
>  #include "stmmac.h"
> +#include "dw_xpcs.h"
> 
>  /*
>   * This struct is used to associate PCI Function of MAC controller on a board,
> @@ -128,6 +129,8 @@ static int ehl_common_data(struct pci_dev *pdev,
>  	plat->clk_csr = 5;
>  	plat->has_gmac = 0;
>  	plat->has_gmac4 = 1;
> +	plat->xpcs_phy_addr = 0x16;
> +	plat->pcs_mode = AN_CTRL_PCS_MD_C37_SGMII;
>  	plat->force_sf_dma_mode = 0;
>  	plat->tso_en = 1;
> 
> @@ -211,6 +214,7 @@ static int ehl_sgmii1g_data(struct pci_dev *pdev,
>  		return ret;
> 
>  	plat->interface = PHY_INTERFACE_MODE_SGMII;
> +	plat->has_xpcs = 1;
> 
>  	return 0;
>  }
> @@ -387,6 +391,7 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
>  	res.addr = pcim_iomap_table(pdev)[i];
>  	res.wol_irq = pdev->irq;
>  	res.irq = pdev->irq;
> +	res.xpcs_irq = 0;
> 
>  	return stmmac_dvr_probe(&pdev->dev, plat, &res);  }
> --
> 1.9.1

++ stmmac maintainers and c45 experts

RE: [PATCH 7/7] net: stmmac: add xPCS functions for device with DWMACv5.1

[Voon, Weifeng]

> From: Ong Boon Leong <boon.leong.ong@intel.com>
> 
> We introduce support for driver that has v5.10 IP and is also using xPCS as
> MMD. This can be easily enabled for other product that integrates xPCS that
> is not using v5.00 IP.
> 
> Reviewed-by: Chuah Kim Tatt <kim.tatt.chuah@intel.com>
> Reviewed-by: Voon Weifeng <weifeng.voon@intel.com>
> Reviewed-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
> Reviewed-by: Baoli Zhang <baoli.zhang@intel.com>
> Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 67
> +++++++++++++++++++++++
>  drivers/net/ethernet/stmicro/stmmac/hwif.c        | 31 ++++++++++-
>  drivers/net/ethernet/stmicro/stmmac/hwif.h        |  1 +
>  3 files changed, 98 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> index 7e5d5db..f36aa67 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> @@ -20,6 +20,7 @@
>  #include <net/dsa.h>
>  #include "stmmac.h"
>  #include "stmmac_pcs.h"
> +#include "dw_xpcs.h"
>  #include "dwmac4.h"
>  #include "dwmac5.h"
> 
> @@ -505,6 +506,34 @@ static void dwmac4_get_adv_lp(void __iomem
> *ioaddr, struct rgmii_adv *adv)
>  	dwmac_get_adv_lp(ioaddr, GMAC_PCS_BASE, adv);  }
> 
> +static void dwmac_xpcs_init(struct net_device *ndev, int pcs_mode) {
> +	dw_xpcs_init(ndev, pcs_mode);
> +}
> +
> +static void dwmac_xpcs_ctrl_ane(struct net_device *ndev, bool ane,
> +				bool loopback)
> +{
> +	dw_xpcs_ctrl_ane(ndev, ane, loopback); }
> +
> +static void dwmac_xpcs_rane(struct net_device *ndev, bool restart) {
> +	dw_xpcs_rane(ndev, restart);
> +}
> +
> +static void dwmac_xpcs_get_adv_lp(struct net_device *ndev,
> +				  struct rgmii_adv *adv)
> +{
> +	dw_xpcs_get_adv_lp(ndev, adv);
> +}
> +
> +static int dwmac_xpcs_irq_status(struct net_device *ndev,
> +				 struct stmmac_extra_stats *x)
> +{
> +	return dw_xpcs_irq_status(ndev, x);
> +}
> +
>  /* RGMII or SMII interface */
>  static void dwmac4_phystatus(void __iomem *ioaddr, struct
> stmmac_extra_stats *x)  { @@ -799,6 +828,44 @@ static void
> dwmac4_debug(void __iomem *ioaddr, struct stmmac_extra_stats *x,
>  	.flex_pps_config = dwmac5_flex_pps_config,  };
> 
> +const struct stmmac_ops dwmac510_xpcs_ops = {
> +	.core_init = dwmac4_core_init,
> +	.set_mac = stmmac_dwmac4_set_mac,
> +	.rx_ipc = dwmac4_rx_ipc_enable,
> +	.rx_queue_enable = dwmac4_rx_queue_enable,
> +	.rx_queue_prio = dwmac4_rx_queue_priority,
> +	.tx_queue_prio = dwmac4_tx_queue_priority,
> +	.rx_queue_routing = dwmac4_rx_queue_routing,
> +	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
> +	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
> +	.set_mtl_tx_queue_weight = dwmac4_set_mtl_tx_queue_weight,
> +	.map_mtl_to_dma = dwmac4_map_mtl_dma,
> +	.config_cbs = dwmac4_config_cbs,
> +	.dump_regs = dwmac4_dump_regs,
> +	.host_irq_status = dwmac4_irq_status,
> +	.host_mtl_irq_status = dwmac4_irq_mtl_status,
> +	.flow_ctrl = dwmac4_flow_ctrl,
> +	.pmt = dwmac4_pmt,
> +	.set_umac_addr = dwmac4_set_umac_addr,
> +	.get_umac_addr = dwmac4_get_umac_addr,
> +	.set_eee_mode = dwmac4_set_eee_mode,
> +	.reset_eee_mode = dwmac4_reset_eee_mode,
> +	.set_eee_timer = dwmac4_set_eee_timer,
> +	.set_eee_pls = dwmac4_set_eee_pls,
> +	.xpcs_init = dwmac_xpcs_init,
> +	.xpcs_ctrl_ane = dwmac_xpcs_ctrl_ane,
> +	.xpcs_rane = dwmac_xpcs_rane,
> +	.xpcs_get_adv_lp = dwmac_xpcs_get_adv_lp,
> +	.xpcs_irq_status = dwmac_xpcs_irq_status,
> +	.debug = dwmac4_debug,
> +	.set_filter = dwmac4_set_filter,
> +	.safety_feat_config = dwmac5_safety_feat_config,
> +	.safety_feat_irq_status = dwmac5_safety_feat_irq_status,
> +	.safety_feat_dump = dwmac5_safety_feat_dump,
> +	.rxp_config = dwmac5_rxp_config,
> +	.flex_pps_config = dwmac5_flex_pps_config, };
> +
>  int dwmac4_setup(struct stmmac_priv *priv)  {
>  	struct mac_device_info *mac = priv->hw; diff --git
> a/drivers/net/ethernet/stmicro/stmmac/hwif.c
> b/drivers/net/ethernet/stmicro/stmmac/hwif.c
> index 81b966a..3725baa 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c
> @@ -73,6 +73,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv
> *priv)
>  	bool gmac;
>  	bool gmac4;
>  	bool xgmac;
> +	bool xpcs;
>  	u32 min_id;
>  	const struct stmmac_regs_off regs;
>  	const void *desc;
> @@ -89,6 +90,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv
> *priv)
>  		.gmac = false,
>  		.gmac4 = false,
>  		.xgmac = false,
> +		.xpcs = false,
>  		.min_id = 0,
>  		.regs = {
>  			.ptp_off = PTP_GMAC3_X_OFFSET,
> @@ -106,6 +108,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv
> *priv)
>  		.gmac = true,
>  		.gmac4 = false,
>  		.xgmac = false,
> +		.xpcs = false,
>  		.min_id = 0,
>  		.regs = {
>  			.ptp_off = PTP_GMAC3_X_OFFSET,
> @@ -123,6 +126,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv
> *priv)
>  		.gmac = false,
>  		.gmac4 = true,
>  		.xgmac = false,
> +		.xpcs = false,
>  		.min_id = 0,
>  		.regs = {
>  			.ptp_off = PTP_GMAC4_OFFSET,
> @@ -140,6 +144,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv
> *priv)
>  		.gmac = false,
>  		.gmac4 = true,
>  		.xgmac = false,
> +		.xpcs = false,
>  		.min_id = DWMAC_CORE_4_00,
>  		.regs = {
>  			.ptp_off = PTP_GMAC4_OFFSET,
> @@ -157,6 +162,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv
> *priv)
>  		.gmac = false,
>  		.gmac4 = true,
>  		.xgmac = false,
> +		.xpcs = false,
>  		.min_id = DWMAC_CORE_4_10,
>  		.regs = {
>  			.ptp_off = PTP_GMAC4_OFFSET,
> @@ -174,6 +180,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv
> *priv)
>  		.gmac = false,
>  		.gmac4 = true,
>  		.xgmac = false,
> +		.xpcs = false,
>  		.min_id = DWMAC_CORE_5_10,
>  		.regs = {
>  			.ptp_off = PTP_GMAC4_OFFSET,
> @@ -191,6 +198,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv
> *priv)
>  		.gmac = false,
>  		.gmac4 = false,
>  		.xgmac = true,
> +		.xpcs = false,
>  		.min_id = DWXGMAC_CORE_2_10,
>  		.regs = {
>  			.ptp_off = PTP_XGMAC_OFFSET,
> @@ -204,7 +212,25 @@ static int stmmac_dwmac4_quirks(struct
> stmmac_priv *priv)
>  		.tc = &dwmac510_tc_ops,
>  		.setup = dwxgmac2_setup,
>  		.quirks = NULL,
> -	},
> +	}, {
> +		.gmac = false,
> +		.gmac4 = true,
> +		.xgmac = false,
> +		.xpcs = true,
> +		.min_id = DWMAC_CORE_5_10,
> +		.regs = {
> +			.ptp_off = PTP_GMAC4_OFFSET,
> +			.mmc_off = MMC_GMAC4_OFFSET,
> +		},
> +		.desc = &dwmac4_desc_ops,
> +		.dma = &dwmac410_dma_ops,
> +		.mac = &dwmac510_xpcs_ops,
> +		.hwtimestamp = &stmmac_ptp,
> +		.mode = &dwmac4_ring_mode_ops,
> +		.tc = &dwmac510_tc_ops,
> +		.setup = dwmac4_setup,
> +		.quirks = NULL,
> +	}
>  };
> 
>  int stmmac_hwif_init(struct stmmac_priv *priv) @@ -212,6 +238,7 @@ int
> stmmac_hwif_init(struct stmmac_priv *priv)
>  	bool needs_xgmac = priv->plat->has_xgmac;
>  	bool needs_gmac4 = priv->plat->has_gmac4;
>  	bool needs_gmac = priv->plat->has_gmac;
> +	bool needs_xpcs = priv->plat->has_xpcs;
>  	const struct stmmac_hwif_entry *entry;
>  	struct mac_device_info *mac;
>  	bool needs_setup = true;
> @@ -256,6 +283,8 @@ int stmmac_hwif_init(struct stmmac_priv *priv)
>  			continue;
>  		if (needs_xgmac ^ entry->xgmac)
>  			continue;
> +		if (needs_xpcs ^ entry->xpcs)
> +			continue;
>  		/* Use synopsys_id var because some setups can override
> this */
>  		if (priv->synopsys_id < entry->min_id)
>  			continue;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index cb7eb48..485be41 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> @@ -495,6 +495,7 @@ struct stmmac_regs_off {  extern const struct
> stmmac_ops dwmac410_ops;  extern const struct stmmac_dma_ops
> dwmac410_dma_ops;  extern const struct stmmac_ops dwmac510_ops;
> +extern const struct stmmac_ops dwmac510_xpcs_ops;
>  extern const struct stmmac_tc_ops dwmac510_tc_ops;  extern const struct
> stmmac_ops dwxgmac210_ops;  extern const struct stmmac_dma_ops
> dwxgmac210_dma_ops;
> --
> 1.9.1

++ stmmac maintainers and c45 experts

RE: [PATCH 2/7] net: stmmac: enable clause 45 mdio support

[Voon, Weifeng]

> > From: Kweh Hock Leong <hock.leong.kweh@intel.com>
> >
> > DWMAC4 is capable to support clause 45 mdio communication.
> 
> Hi Weifeng
> 
> Is C45 a synthesis time option, or do all DWMAC4 have it?

All DWMAC4 with MDIO enabled will have C45. It is not a
Synthesis time option. 

Regards,
Weifeng

> 
> Thanks
> 	Andrew

RE: [PATCH 0/7] net: stmmac: enable EHL SGMII

[Voon, Weifeng]

> > This patch-set is to enable Ethernet controller (DW Ethernet QoS and
> > DW Ethernet PCS) with SGMII interface in Elkhart Lake.
> 
> Can the hardware also do 1000BaseX?

Yes, it is able to do 1000BaseX.

Regards,
Weifeng
> 
>     Andrew

Re: [PATCH 4/7] net: stmmac: introducing support for DWC xPCS logics

[Andrew Lunn]

> >> >> +/* MII MMD AN Status defines for C37 AN SGMII Status */
> >> >> +#define AN_STAT_C37_AN_CMPLT		BIT(0)	/* AN
> >Complete Intr */
> >> >> +#define AN_STAT_C37_AN_FD		BIT(1)	/* Full Duplex */
> >> >> +#define AN_STAT_C37_AN_SPEED_SHIFT	2	/* AN Speed shift */
> >> >> +#define AN_STAT_C37_AN_SPEED		GENMASK(3, 2)	/* AN
> >Speed */
> >> >> +#define AN_STAT_C37_AN_10MBPS		0x0	/* 10 Mbps */
> >> >> +#define AN_STAT_C37_AN_100MBPS		0x1	/* 100 Mbps
> >> >*/
> >> >> +#define AN_STAT_C37_AN_1000MBPS		0x2	/* 1000 Mbps
> >> >*/
> >> >> +#define AN_STAT_C37_AN_LNKSTS		BIT(4)	/* Link Status
> >*/
> >> >
> >> >Is these are standardized, not proprietary, consider adding them to
> >> >include/uapi/linux/mii.h so similar.
> >>
> >> Yeah, it does look very standardized. However, per DW spec, they are
> >> vendor-specific register set which uses MDIO_MMD_VEND2 to access.
> >
> >Maybe you can go get the 802.3 standard and check section 37. If these
> >registers are the same, they could still be placed in mii.h. What MMD
> >you use is a different matter.
> >
> I cross-checked on 802.3 section 37.2.5.1 Management registers, the spec only
> talks about BMSR and extended status. I also checked on 802.3 section 45.2
> MDIO interface registers, section 45.2.7 AN registers, section 45.2.9 &
> 45.2.10 Vendor specific MMD1 & MMD2, there are no field format that
> matches the above bit fields. So, it should be reasonable to park these #defines
> in this local file for now.  

Thanks for checking the standards. If these don't actually represent
anything from C37, maybe you should change the name to avoid
confusion. s/C37/SGMII/g ?

	   Thanks
		Andrew

Re: [PATCH 1/7] net: stmmac: add EHL SGMII 1Gbps platform data and PCI ID

[Andrew Lunn]

> ++ stmmac maintainers and c45 experts

Normally you would repost the series as a v2, rather the additional
maintainers added. Please try to remember that for next time.

	   Andrew

Re: [PATCH 0/7] net: stmmac: enable EHL SGMII

[Andrew Lunn]

On Thu, Apr 25, 2019 at 07:27:51AM +0000, Voon, Weifeng wrote:
> > > This patch-set is to enable Ethernet controller (DW Ethernet QoS and
> > > DW Ethernet PCS) with SGMII interface in Elkhart Lake.
> > 
> > Can the hardware also do 1000BaseX?
> 
> Yes, it is able to do 1000BaseX.

I Voon

That means you should not really hard code it to SGMII. Somebody is
going to connect an SFP or an Ethernet switch and want to use
1000BaseX.

At minimum, please add support for phy-mode in the device tree.

Also, when the adjust_link callback passed to phy_connect() is called,
you can look at the interface type to know if you need to configure it
to SGMII or 1000BaseX. A copper PHY in an SFP module generally wants
SGMII, but an optical module wants 1000BaseX. But to properly support
SPFs the driver needs to swap to phylink, rather than phylib.

     Andrew

RE: [PATCH 0/7] net: stmmac: enable EHL SGMII

[Ong, Boon Leong]

>-----Original Message-----
>From: Andrew Lunn [mailto:andrew@lunn.ch]
>Sent: Thursday, April 25, 2019 8:38 PM
>To: Voon, Weifeng <weifeng.voon@intel.com>
>Cc: David S. Miller <davem@davemloft.net>; netdev@vger.kernel.org; linux-
>kernel@vger.kernel.org; Ong, Boon Leong <boon.leong.ong@intel.com>;
>Kweh, Hock Leong <hock.leong.kweh@intel.com>; Florian Fainelli
><f.fainelli@gmail.com>; Maxime Coquelin <mcoquelin.stm32@gmail.com>;
>Giuseppe Cavallaro <peppe.cavallaro@st.com>; Jose Abreu
><joabreu@synopsys.com>
>Subject: Re: [PATCH 0/7] net: stmmac: enable EHL SGMII
>
>On Thu, Apr 25, 2019 at 07:27:51AM +0000, Voon, Weifeng wrote:
>> > > This patch-set is to enable Ethernet controller (DW Ethernet QoS and
>> > > DW Ethernet PCS) with SGMII interface in Elkhart Lake.
>> >
>> > Can the hardware also do 1000BaseX?
>>
>> Yes, it is able to do 1000BaseX.
>
>I Voon
>
>That means you should not really hard code it to SGMII. Somebody is
>going to connect an SFP or an Ethernet switch and want to use
>1000BaseX.

Hi Andrew,

The Ethernet controller consists of two ways to connect to external PHY,
RGMII and SGMII. The selection is done through soft strap.
The patch-series is to enable SGMII interface. The DW xPCS IP is
configured to operate in 1000BASE-X mode. The xPCS IP is external
connected through internal PHY interface which presents externally
as SGMII interface. To help illustrate the connection:-

      <-----------------GBE Controller----------------->|<--External PHY chip-->

      +----------+                    +----+          +---+                          +-----------------+
      |   EQoS   | <-GMII->|xPCS|<--> | L1 | <-- SGMII --> | External GbE |
      |   MAC    |                 |         |       |PHY|                         | PHY Chip        |
      +----------+                    +----+          +---+                          +-----------------+
            
In future, we will submit the changes for the RGMII connection that
bypasses DW xPCS.  

Hope this help clarifies matter. 

>At minimum, please add support for phy-mode in the device tree.
>
>Also, when the adjust_link callback passed to phy_connect() is called,
>you can look at the interface type to know if you need to configure it
>to SGMII or 1000BaseX. A copper PHY in an SFP module generally wants
>SGMII, but an optical module wants 1000BaseX. But to properly support
>SPFs the driver needs to swap to phylink, rather than phylib.
>
>     Andrew

Re: [PATCH 0/7] net: stmmac: enable EHL SGMII

[Andrew Lunn]

> >> > > This patch-set is to enable Ethernet controller (DW Ethernet QoS and
> >> > > DW Ethernet PCS) with SGMII interface in Elkhart Lake.
> >> >
> >> > Can the hardware also do 1000BaseX?
> >>
> >> Yes, it is able to do 1000BaseX.
> >
> >I Voon
> >
> >That means you should not really hard code it to SGMII. Somebody is
> >going to connect an SFP or an Ethernet switch and want to use
> >1000BaseX.
> 
> Hi Andrew,
> 
> The Ethernet controller consists of two ways to connect to external PHY,
> RGMII and SGMII. The selection is done through soft strap.
> The patch-series is to enable SGMII interface. The DW xPCS IP is
> configured to operate in 1000BASE-X mode. The xPCS IP is external
> connected through internal PHY interface which presents externally
> as SGMII interface. To help illustrate the connection:-
> 
>       <-----------------GBE Controller----------------->|<--External PHY chip-->
> 
>       +----------+                    +----+          +---+                          +-----------------+
>       |   EQoS   | <-GMII->|xPCS|<--> | L1 | <-- SGMII --> | External GbE |
>       |   MAC    |                 |         |       |PHY|                         | PHY Chip        |
>       +----------+                    +----+          +---+                          +-----------------+
>             
> In future, we will submit the changes for the RGMII connection that
> bypasses DW xPCS.  

The ASCII art get messed up somewhere.

What you are implementing looks like:


       <-----------------GBE Controller------------>|<--External PHY chip-->
 
       +----------+                    +----+                 +---+
       |   EQoS   | <-GMII->|xPCS|<--> | L1 | <-- SGMII -->   |PHY|
       |   MAC    |                    |    |                 +---+
       +----------+                    +----+                      

With the Ethernet controller, the MAC connects to the xPCS. Out of the
xPCS you have a SERDES connection, running the SGMII protocol. That
connects to external pins of the SoC. These are then connected to a
copper PHY which also supports SGMII.

What i'm trying to understand is if the following is possible:

       <-----------------GBE Controller------------->|<--External SFP cage/module-->
 
       +----------+                    +----+                   +---+
       |   EQoS   | <-GMII->|xPCS|<--> | L1 | <-- 1000BaseX --> |SPF|
       |   MAC    |                    |    |                   +---+
       +----------+                    +----+                      

Rather than use a Copper PHY, an SFP cage+module is used. The same
SERDES interface is used, but 1000BaseX runs over it.

Generally, an xPCS which can run SGMII can also do 1000BaseX, because
SGMII is just some Cisco Proprietary extensions to 1000BaseX.

If the xPCS can do 1000BaseX over the SERDES, you should not hard code
it to SGMII, but allow it to be configured.

      Andrew

RE: [PATCH 1/7] net: stmmac: add EHL SGMII 1Gbps platform data and PCI ID

[Voon, Weifeng]

> > ++ stmmac maintainers and c45 experts
> 
> Normally you would repost the series as a v2, rather the additional
> maintainers added. Please try to remember that for next time.

Noted and I will remember that. The reason I did not repost the series
as v2 is because v1 is still in discussion and I am not changing any code
besides adding additional maintainers. Thanks. 

Weifeng
> 
> 	   Andrew

RE: [PATCH 0/7] net: stmmac: enable EHL SGMII

[Ong, Boon Leong]

>-----Original Message-----
>From: Andrew Lunn [mailto:andrew@lunn.ch]
>Sent: Thursday, April 25, 2019 11:24 PM
>To: Ong, Boon Leong <boon.leong.ong@intel.com>
>Cc: Voon, Weifeng <weifeng.voon@intel.com>; David S. Miller
><davem@davemloft.net>; netdev@vger.kernel.org; linux-
>kernel@vger.kernel.org; Kweh, Hock Leong <hock.leong.kweh@intel.com>;
>Florian Fainelli <f.fainelli@gmail.com>; Maxime Coquelin
><mcoquelin.stm32@gmail.com>; Giuseppe Cavallaro
><peppe.cavallaro@st.com>; Jose Abreu <joabreu@synopsys.com>
>Subject: Re: [PATCH 0/7] net: stmmac: enable EHL SGMII
>
>> >> > > This patch-set is to enable Ethernet controller (DW Ethernet QoS and
>> >> > > DW Ethernet PCS) with SGMII interface in Elkhart Lake.
>> >> >
>> >> > Can the hardware also do 1000BaseX?
>> >>
>> >> Yes, it is able to do 1000BaseX.
>> >
>> >I Voon
>> >
>> >That means you should not really hard code it to SGMII. Somebody is
>> >going to connect an SFP or an Ethernet switch and want to use
>> >1000BaseX.
>>
>> Hi Andrew,
>>
>> The Ethernet controller consists of two ways to connect to external PHY,
>> RGMII and SGMII. The selection is done through soft strap.
>> The patch-series is to enable SGMII interface. The DW xPCS IP is
>> configured to operate in 1000BASE-X mode. The xPCS IP is external
>> connected through internal PHY interface which presents externally
>> as SGMII interface. To help illustrate the connection:-
>>
>>       <-----------------GBE Controller----------------->|<--External PHY chip-->
>>
>>       +----------+                    +----+          +---+                          +-----------------+
>>       |   EQoS   | <-GMII->|xPCS|<--> | L1 | <-- SGMII --> | External GbE |
>>       |   MAC    |                 |         |       |PHY|                         | PHY Chip        |
>>       +----------+                    +----+          +---+                          +-----------------+
>>
>> In future, we will submit the changes for the RGMII connection that
>> bypasses DW xPCS.
>
>The ASCII art get messed up somewhere.
Sorry for that. 

>
>What you are implementing looks like:
>
>
>       <-----------------GBE Controller------------>|<--External PHY chip-->
>
>       +----------+                    +----+                 +---+
>       |   EQoS   | <-GMII->|xPCS|<--> | L1 | <-- SGMII -->   |PHY|
>       |   MAC    |                    |    |                 +---+
>       +----------+                    +----+
>
>With the Ethernet controller, the MAC connects to the xPCS. Out of the
>xPCS you have a SERDES connection, running the SGMII protocol. That
>connects to external pins of the SoC. These are then connected to a
>copper PHY which also supports SGMII.
>
>What i'm trying to understand is if the following is possible:
>
>       <-----------------GBE Controller------------->|<--External SFP cage/module-->
>
>       +----------+                    +----+                   +---+
>       |   EQoS   | <-GMII->|xPCS|<--> | L1 | <-- 1000BaseX --> |SPF|
>       |   MAC    |                    |    |                   +---+
>       +----------+                    +----+
>
>Rather than use a Copper PHY, an SFP cage+module is used. The same
>SERDES interface is used, but 1000BaseX runs over it.
>
>Generally, an xPCS which can run SGMII can also do 1000BaseX, because
>SGMII is just some Cisco Proprietary extensions to 1000BaseX.
>
>If the xPCS can do 1000BaseX over the SERDES, you should not hard code
>it to SGMII, but allow it to be configured.

Thanks for the review and checking above.

Sorry for the delay, we have checked with hardware/SoC HW architect
and gotten confirmation that the controller can only support SGMII inter-chip
connection. It does not support 1000Base-X. 

In this case, we believe that the current implementation of the DW xPCS
are sufficient. Ok?

RE: [PATCH 3/7] net: stmmac: dma channel control register need to be init first

[Jose Abreu]

From: Voon, Weifeng <weifeng.voon@intel.com>
Date: Thu, Apr 25, 2019 at 08:06:08

> > stmmac_init_chan() needs to be called before stmmac_init_rx_chan() and
> > stmmac_init_tx_chan(). This is because if PBLx8 is to be used,
> > "DMA_CH(#i)_Control.PBLx8" needs to be set before programming
> > "DMA_CH(#i)_TX_Control.TxPBL" and "DMA_CH(#i)_RX_Control.RxPBL".
> > 
> > Reviewed-by: Zhang, Baoli <baoli.zhang@intel.com>
> > Signed-off-by: Weifeng Voon <weifeng.voon@intel.com>
> > Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>

This is a fix so it should belong to -net tree and it should have the 
"Fixes: " tag.

Thanks,
Jose Miguel Abreu

Re: [PATCH 0/7] net: stmmac: enable EHL SGMII

[Andrew Lunn]

> Sorry for the delay, we have checked with hardware/SoC HW architect
> and gotten confirmation that the controller can only support SGMII inter-chip
> connection. It does not support 1000Base-X. 

O.K, i'm surprised about that, but if that is what the hardware
engineer says...

> In this case, we believe that the current implementation of the DW xPCS
> are sufficient. Ok?

Yes, if all i can do is SGMII, hard coding SGMII is fine. But you
should probably check phy-mode and return an error if it has a value
other than SGMII,

      Andrew

RE: [PATCH 4/7] net: stmmac: introducing support for DWC xPCS logics

[Jose Abreu]

From: Voon, Weifeng <weifeng.voon@intel.com>
Date: Thu, Apr 25, 2019 at 08:06:43

> > From: Ong Boon Leong <boon.leong.ong@intel.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->|xPCS|<-->|L1 | <-- SGMII --> | External GbE |
> >   |   MAC    |         |    |    |PHY|               | 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.
> > 
> > Basic functionalities for initializing xPCS and configuring auto negotiation (AN),
> > loopback, link status, AN advertisement and Link Partner ability are
> > implemented.
> > 
> > xPCS interrupt handling for C37 AN complete is also implemented.
> > 
> > Tested-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
> > Reviewed-by: Chuah Kim Tatt <kim.tatt.chuah@intel.com>
> > Reviewed-by: Voon Weifeng <weifeng.voon@intel.com>
> > Reviewed-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
> > Reviewed-by: Baoli Zhang <baoli.zhang@intel.com>
> > Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h | 288
> > ++++++++++++++++++++++++++
> >  drivers/net/ethernet/stmicro/stmmac/hwif.h    |  17 ++
> >  include/linux/stmmac.h                        |   1 +
> >  3 files changed, 306 insertions(+)
> >  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
> > b/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
> > new file mode 100644
> > index 0000000..446b714
> > --- /dev/null
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h

I would rather prefer see this as a .c file and then just export a new 
structure for HWIF because this does not belong to the MAC. Is there any 
specific reason why you added this as a .h file besides the reuse of 
callbacks across cores ?

And having inline functions everywhere seems overkill also.

Thanks,
Jose Miguel Abreu

RE: [PATCH 0/7] net: stmmac: enable EHL SGMII

[Jose Abreu]

From: Andrew Lunn <andrew@lunn.ch>
Date: Mon, Apr 29, 2019 at 14:10:16

> Yes, if all i can do is SGMII, hard coding SGMII is fine. But you
> should probably check phy-mode and return an error if it has a value
> other than SGMII,

+1 because XPCS supports 1000Base-X but it seems this SoC doesn't.

Thanks,
Jose Miguel Abreu

RE: [PATCH 3/7] net: stmmac: dma channel control register need to be init first

[Voon, Weifeng]

> From: Voon, Weifeng <weifeng.voon@intel.com>
> Date: Thu, Apr 25, 2019 at 08:06:08
> 
> > > stmmac_init_chan() needs to be called before stmmac_init_rx_chan()
> > > and stmmac_init_tx_chan(). This is because if PBLx8 is to be used,
> > > "DMA_CH(#i)_Control.PBLx8" needs to be set before programming
> > > "DMA_CH(#i)_TX_Control.TxPBL" and "DMA_CH(#i)_RX_Control.RxPBL".
> > >
> > > Reviewed-by: Zhang, Baoli <baoli.zhang@intel.com>
> > > Signed-off-by: Weifeng Voon <weifeng.voon@intel.com>
> > > Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
> 
> This is a fix so it should belong to -net tree and it should have the
> "Fixes: " tag.
> 
> Thanks,
> Jose Miguel Abreu

Noted. I will add the "Fixes" tag and re-submit to -net tree. 

Regards,
Weifeng