diff mbox series

[net-next,v4,RESEND] stmmac: tegra: Add MGBE support

Message ID 20220923114922.864552-1-thierry.reding@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net-next,v4,RESEND] stmmac: tegra: Add MGBE support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 7 maintainers not CCed: peppe.cavallaro@st.com p.zabel@pengutronix.de joabreu@synopsys.com linux-stm32@st-md-mailman.stormreply.com alexandre.torgue@foss.st.com mcoquelin.stm32@gmail.com linux-arm-kernel@lists.infradead.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: DT compatible string "nvidia,tegra234-mgbe" appears un-documented -- check ./Documentation/devicetree/bindings/ WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 81 exceeds 80 columns WARNING: line length of 82 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns WARNING: line length of 84 exceeds 80 columns WARNING: line length of 92 exceeds 80 columns WARNING: please write a help paragraph that fully describes the config symbol
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Thierry Reding Sept. 23, 2022, 11:49 a.m. UTC
From: Bhadram Varka <vbhadram@nvidia.com>

Add support for the Multi-Gigabit Ethernet (MGBE/XPCS) IP found on
NVIDIA Tegra234 SoCs.

Signed-off-by: Bhadram Varka <vbhadram@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |   6 +
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../net/ethernet/stmicro/stmmac/dwmac-tegra.c | 290 ++++++++++++++++++
 3 files changed, 297 insertions(+)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c

Comments

Paolo Abeni Sept. 27, 2022, 9:29 a.m. UTC | #1
On Fri, 2022-09-23 at 13:49 +0200, Thierry Reding wrote:
> From: Bhadram Varka <vbhadram@nvidia.com>
> 
> Add support for the Multi-Gigabit Ethernet (MGBE/XPCS) IP found on
> NVIDIA Tegra234 SoCs.
> 
> Signed-off-by: Bhadram Varka <vbhadram@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/Kconfig   |   6 +
>  drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
>  .../net/ethernet/stmicro/stmmac/dwmac-tegra.c | 290 ++++++++++++++++++
>  3 files changed, 297 insertions(+)
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index 31ff35174034..e9f61bdaf7c4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -235,6 +235,12 @@ config DWMAC_INTEL_PLAT
>  	  the stmmac device driver. This driver is used for the Intel Keem Bay
>  	  SoC.
>  
> +config DWMAC_TEGRA
> +	tristate "NVIDIA Tegra MGBE support"
> +	depends on ARCH_TEGRA || COMPILE_TEST
> +	help
> +	  Support for the MGBE controller found on Tegra SoCs.
> +
>  config DWMAC_VISCONTI
>  	tristate "Toshiba Visconti DWMAC support"
>  	default ARCH_VISCONTI
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index d4e12e9ace4f..057e4bab5c08 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_DWMAC_DWC_QOS_ETH)	+= dwmac-dwc-qos-eth.o
>  obj-$(CONFIG_DWMAC_INTEL_PLAT)	+= dwmac-intel-plat.o
>  obj-$(CONFIG_DWMAC_GENERIC)	+= dwmac-generic.o
>  obj-$(CONFIG_DWMAC_IMX8)	+= dwmac-imx.o
> +obj-$(CONFIG_DWMAC_TEGRA)	+= dwmac-tegra.o
>  obj-$(CONFIG_DWMAC_VISCONTI)	+= dwmac-visconti.o
>  stmmac-platform-objs:= stmmac_platform.o
>  dwmac-altr-socfpga-objs := altr_tse_pcs.o dwmac-socfpga.o
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c
> new file mode 100644
> index 000000000000..bb4b540820fa
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c
> @@ -0,0 +1,290 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/platform_device.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
> +#include <linux/stmmac.h>
> +#include <linux/clk.h>
> +
> +#include "stmmac_platform.h"
> +
> +static const char *const mgbe_clks[] = {
> +	"rx-pcs", "tx", "tx-pcs", "mac-divider", "mac", "mgbe", "ptp-ref", "mac"
> +};
> +
> +struct tegra_mgbe {
> +	struct device *dev;
> +
> +	struct clk_bulk_data *clks;
> +
> +	struct reset_control *rst_mac;
> +	struct reset_control *rst_pcs;
> +
> +	void __iomem *hv;
> +	void __iomem *regs;
> +	void __iomem *xpcs;
> +
> +	struct mii_bus *mii;
> +};
> +
> +#define XPCS_WRAP_UPHY_RX_CONTROL 0x801c
> +#define XPCS_WRAP_UPHY_RX_CONTROL_RX_SW_OVRD BIT(31)
> +#define XPCS_WRAP_UPHY_RX_CONTROL_RX_PCS_PHY_RDY BIT(10)
> +#define XPCS_WRAP_UPHY_RX_CONTROL_RX_CDR_RESET BIT(9)
> +#define XPCS_WRAP_UPHY_RX_CONTROL_RX_CAL_EN BIT(8)
> +#define XPCS_WRAP_UPHY_RX_CONTROL_RX_SLEEP (BIT(7) | BIT(6))
> +#define XPCS_WRAP_UPHY_RX_CONTROL_AUX_RX_IDDQ BIT(5)
> +#define XPCS_WRAP_UPHY_RX_CONTROL_RX_IDDQ BIT(4)
> +#define XPCS_WRAP_UPHY_RX_CONTROL_RX_DATA_EN BIT(0)
> +#define XPCS_WRAP_UPHY_HW_INIT_CTRL 0x8020
> +#define XPCS_WRAP_UPHY_HW_INIT_CTRL_TX_EN BIT(0)
> +#define XPCS_WRAP_UPHY_HW_INIT_CTRL_RX_EN BIT(2)
> +#define XPCS_WRAP_UPHY_STATUS 0x8044
> +#define XPCS_WRAP_UPHY_STATUS_TX_P_UP BIT(0)
> +#define XPCS_WRAP_IRQ_STATUS 0x8050
> +#define XPCS_WRAP_IRQ_STATUS_PCS_LINK_STS BIT(6)
> +
> +#define XPCS_REG_ADDR_SHIFT 10
> +#define XPCS_REG_ADDR_MASK 0x1fff
> +#define XPCS_ADDR 0x3fc
> +
> +#define MGBE_WRAP_COMMON_INTR_ENABLE	0x8704
> +#define MAC_SBD_INTR			BIT(2)
> +#define MGBE_WRAP_AXI_ASID0_CTRL	0x8400
> +#define MGBE_SID			0x6
> +
> +static void mgbe_uphy_lane_bringup(struct tegra_mgbe *mgbe)
> +{
> +	unsigned int retry = 300;
> +	u32 value;
> +	int err;
> +
> +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_STATUS);
> +	if ((value & XPCS_WRAP_UPHY_STATUS_TX_P_UP) == 0) {
> +		value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_HW_INIT_CTRL);
> +		value |= XPCS_WRAP_UPHY_HW_INIT_CTRL_TX_EN;
> +		writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_HW_INIT_CTRL);
> +	}
> +
> +	err = readl_poll_timeout(mgbe->xpcs + XPCS_WRAP_UPHY_HW_INIT_CTRL, value,
> +				 (value & XPCS_WRAP_UPHY_HW_INIT_CTRL_TX_EN) == 0,
> +				 500, 500 * 2000);
> +	if (err < 0)
> +		dev_err(mgbe->dev, "timeout waiting for TX lane to become enabled\n");

Why you don't need to propagate this error to the caller?

Same question for more error cases below.

> +
> +	usleep_range(10000, 20000);
> +
> +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_SW_OVRD;
> +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +
> +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +	value &= ~XPCS_WRAP_UPHY_RX_CONTROL_RX_IDDQ;
> +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +
> +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +	value &= ~XPCS_WRAP_UPHY_RX_CONTROL_AUX_RX_IDDQ;
> +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +
> +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +	value &= ~XPCS_WRAP_UPHY_RX_CONTROL_RX_SLEEP;
> +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +
> +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_CAL_EN;
> +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +
> +	err = readl_poll_timeout(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL, value,
> +				 (value & XPCS_WRAP_UPHY_RX_CONTROL_RX_CAL_EN) == 0,
> +				 1000, 1000 * 2000);
> +	if (err < 0)
> +		dev_err(mgbe->dev, "timeout waiting for RX calibration to become enabled\n");
> +
> +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_DATA_EN;
> +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +
> +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_CDR_RESET;
> +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +
> +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +	value &= ~XPCS_WRAP_UPHY_RX_CONTROL_RX_CDR_RESET;
> +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +
> +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_PCS_PHY_RDY;
> +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> +
> +	while (--retry) {
> +		err = readl_poll_timeout(mgbe->xpcs + XPCS_WRAP_IRQ_STATUS, value,
> +					 value & XPCS_WRAP_IRQ_STATUS_PCS_LINK_STS,
> +					 500, 500 * 2000);
> +		if (err < 0) {
> +			dev_err(mgbe->dev, "timeout waiting for link to become ready\n");
> +			usleep_range(10000, 20000);
> +			continue;
> +		}
> +		break;
> +	}

It looks like the above loop can take up to 150 seconds (300
iterations, 500000usec each), can it be shortned?

> +
> +	/* clear status */
> +	writel(value, mgbe->xpcs + XPCS_WRAP_IRQ_STATUS);
> +}
> +
> +static int tegra_mgbe_probe(struct platform_device *pdev)
> +{
> +	struct plat_stmmacenet_data *plat;
> +	struct stmmac_resources res;
> +	struct tegra_mgbe *mgbe;
> +	int irq, err, i;
> +
> +	mgbe = devm_kzalloc(&pdev->dev, sizeof(*mgbe), GFP_KERNEL);
> +	if (!mgbe)
> +		return -ENOMEM;
> +
> +	mgbe->dev = &pdev->dev;
> +
> +	memset(&res, 0, sizeof(res));
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return irq;
> +
> +	mgbe->hv = devm_platform_ioremap_resource_byname(pdev, "hypervisor");
> +	if (IS_ERR(mgbe->hv))
> +		return PTR_ERR(mgbe->hv);
> +
> +	mgbe->regs = devm_platform_ioremap_resource_byname(pdev, "mac");
> +	if (IS_ERR(mgbe->regs))
> +		return PTR_ERR(mgbe->regs);
> +
> +	mgbe->xpcs = devm_platform_ioremap_resource_byname(pdev, "xpcs");
> +	if (IS_ERR(mgbe->xpcs))
> +		return PTR_ERR(mgbe->xpcs);
> +
> +	res.addr = mgbe->regs;
> +	res.irq = irq;
> +
> +	mgbe->clks = devm_kzalloc(&pdev->dev, sizeof(*mgbe->clks), GFP_KERNEL);
> +	if (!mgbe->clks)
> +		return -ENOMEM;
> +
> +	for (i = 0; i <  ARRAY_SIZE(mgbe_clks); i++)
> +		mgbe->clks[i].id = mgbe_clks[i];
> +
> +	err = devm_clk_bulk_get(mgbe->dev, ARRAY_SIZE(mgbe_clks), mgbe->clks);
> +	if (err < 0)
> +		return err;
> +
> +	err = clk_bulk_prepare_enable(ARRAY_SIZE(mgbe_clks), mgbe->clks);
> +	if (err < 0)
> +		return err;
> +
> +	/* Perform MAC reset */
> +	mgbe->rst_mac = devm_reset_control_get(&pdev->dev, "mac");
> +	if (IS_ERR(mgbe->rst_mac))
> +		return PTR_ERR(mgbe->rst_mac);
> +
> +	err = reset_control_assert(mgbe->rst_mac);
> +	if (err < 0)
> +		return err;
> +
> +	usleep_range(2000, 4000);
> +
> +	err = reset_control_deassert(mgbe->rst_mac);
> +	if (err < 0)
> +		return err;
> +
> +	/* Perform PCS reset */
> +	mgbe->rst_pcs = devm_reset_control_get(&pdev->dev, "pcs");
> +	if (IS_ERR(mgbe->rst_pcs))
> +		return PTR_ERR(mgbe->rst_pcs);
> +
> +	err = reset_control_assert(mgbe->rst_pcs);
> +	if (err < 0)
> +		return err;
> +
> +	usleep_range(2000, 4000);
> +
> +	err = reset_control_deassert(mgbe->rst_pcs);
> +	if (err < 0)
> +		return err;
> +
> +	plat = stmmac_probe_config_dt(pdev, res.mac);
> +	if (IS_ERR(plat))
> +		return PTR_ERR(plat);
> +
> +	plat->has_xgmac = 1;
> +	plat->tso_en = 1;
> +	plat->pmt = 1;
> +	plat->bsp_priv = mgbe;
> +
> +	if (!plat->mdio_node)
> +		plat->mdio_node = of_get_child_by_name(pdev->dev.of_node, "mdio");
> +
> +	if (!plat->mdio_bus_data) {
> +		plat->mdio_bus_data = devm_kzalloc(&pdev->dev, sizeof(*plat->mdio_bus_data),
> +						   GFP_KERNEL);
> +		if (!plat->mdio_bus_data) {
> +			err = -ENOMEM;
> +			goto remove;
> +		}
> +	}
> +
> +	plat->mdio_bus_data->needs_reset = true;
> +
> +	mgbe_uphy_lane_bringup(mgbe);
> +
> +	/* Tx FIFO Size - 128KB */
> +	plat->tx_fifo_size = 131072;
> +	/* Rx FIFO Size - 192KB */
> +	plat->rx_fifo_size = 196608;
> +
> +	/* Enable common interrupt at wrapper level */
> +	writel(MAC_SBD_INTR, mgbe->regs + MGBE_WRAP_COMMON_INTR_ENABLE);
> +
> +	/* Program SID */
> +	writel(MGBE_SID, mgbe->hv + MGBE_WRAP_AXI_ASID0_CTRL);
> +
> +	err = stmmac_dvr_probe(&pdev->dev, plat, &res);
> +	if (err < 0)
> +		goto remove;
> +
> +	return 0;
> +
> +remove:
> +	stmmac_remove_config_dt(pdev, plat);
> +	return err;
> +}
> +
> +static int tegra_mgbe_remove(struct platform_device *pdev)
> +{
> +	struct tegra_mgbe *mgbe = get_stmmac_bsp_priv(&pdev->dev);
> +
> +	clk_bulk_disable_unprepare(ARRAY_SIZE(mgbe_clks), mgbe->clks);
> +
> +	stmmac_pltfr_remove(pdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id tegra_mgbe_match[] = {
> +	{ .compatible = "nvidia,tegra234-mgbe", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, tegra_mgbe_match);

The DT bindings will land in 6.1, right?

Thanks!

Paolo
Thierry Reding Sept. 27, 2022, 3:33 p.m. UTC | #2
On Tue, Sep 27, 2022 at 11:29:32AM +0200, Paolo Abeni wrote:
> On Fri, 2022-09-23 at 13:49 +0200, Thierry Reding wrote:
> > From: Bhadram Varka <vbhadram@nvidia.com>
> > 
> > Add support for the Multi-Gigabit Ethernet (MGBE/XPCS) IP found on
> > NVIDIA Tegra234 SoCs.
> > 
> > Signed-off-by: Bhadram Varka <vbhadram@nvidia.com>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/Kconfig   |   6 +
> >  drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
> >  .../net/ethernet/stmicro/stmmac/dwmac-tegra.c | 290 ++++++++++++++++++
> >  3 files changed, 297 insertions(+)
> >  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > index 31ff35174034..e9f61bdaf7c4 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > @@ -235,6 +235,12 @@ config DWMAC_INTEL_PLAT
> >  	  the stmmac device driver. This driver is used for the Intel Keem Bay
> >  	  SoC.
> >  
> > +config DWMAC_TEGRA
> > +	tristate "NVIDIA Tegra MGBE support"
> > +	depends on ARCH_TEGRA || COMPILE_TEST
> > +	help
> > +	  Support for the MGBE controller found on Tegra SoCs.
> > +
> >  config DWMAC_VISCONTI
> >  	tristate "Toshiba Visconti DWMAC support"
> >  	default ARCH_VISCONTI
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > index d4e12e9ace4f..057e4bab5c08 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > @@ -31,6 +31,7 @@ obj-$(CONFIG_DWMAC_DWC_QOS_ETH)	+= dwmac-dwc-qos-eth.o
> >  obj-$(CONFIG_DWMAC_INTEL_PLAT)	+= dwmac-intel-plat.o
> >  obj-$(CONFIG_DWMAC_GENERIC)	+= dwmac-generic.o
> >  obj-$(CONFIG_DWMAC_IMX8)	+= dwmac-imx.o
> > +obj-$(CONFIG_DWMAC_TEGRA)	+= dwmac-tegra.o
> >  obj-$(CONFIG_DWMAC_VISCONTI)	+= dwmac-visconti.o
> >  stmmac-platform-objs:= stmmac_platform.o
> >  dwmac-altr-socfpga-objs := altr_tse_pcs.o dwmac-socfpga.o
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c
> > new file mode 100644
> > index 000000000000..bb4b540820fa
> > --- /dev/null
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c
> > @@ -0,0 +1,290 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +#include <linux/platform_device.h>
> > +#include <linux/of_device.h>
> > +#include <linux/module.h>
> > +#include <linux/stmmac.h>
> > +#include <linux/clk.h>
> > +
> > +#include "stmmac_platform.h"
> > +
> > +static const char *const mgbe_clks[] = {
> > +	"rx-pcs", "tx", "tx-pcs", "mac-divider", "mac", "mgbe", "ptp-ref", "mac"
> > +};
> > +
> > +struct tegra_mgbe {
> > +	struct device *dev;
> > +
> > +	struct clk_bulk_data *clks;
> > +
> > +	struct reset_control *rst_mac;
> > +	struct reset_control *rst_pcs;
> > +
> > +	void __iomem *hv;
> > +	void __iomem *regs;
> > +	void __iomem *xpcs;
> > +
> > +	struct mii_bus *mii;
> > +};
> > +
> > +#define XPCS_WRAP_UPHY_RX_CONTROL 0x801c
> > +#define XPCS_WRAP_UPHY_RX_CONTROL_RX_SW_OVRD BIT(31)
> > +#define XPCS_WRAP_UPHY_RX_CONTROL_RX_PCS_PHY_RDY BIT(10)
> > +#define XPCS_WRAP_UPHY_RX_CONTROL_RX_CDR_RESET BIT(9)
> > +#define XPCS_WRAP_UPHY_RX_CONTROL_RX_CAL_EN BIT(8)
> > +#define XPCS_WRAP_UPHY_RX_CONTROL_RX_SLEEP (BIT(7) | BIT(6))
> > +#define XPCS_WRAP_UPHY_RX_CONTROL_AUX_RX_IDDQ BIT(5)
> > +#define XPCS_WRAP_UPHY_RX_CONTROL_RX_IDDQ BIT(4)
> > +#define XPCS_WRAP_UPHY_RX_CONTROL_RX_DATA_EN BIT(0)
> > +#define XPCS_WRAP_UPHY_HW_INIT_CTRL 0x8020
> > +#define XPCS_WRAP_UPHY_HW_INIT_CTRL_TX_EN BIT(0)
> > +#define XPCS_WRAP_UPHY_HW_INIT_CTRL_RX_EN BIT(2)
> > +#define XPCS_WRAP_UPHY_STATUS 0x8044
> > +#define XPCS_WRAP_UPHY_STATUS_TX_P_UP BIT(0)
> > +#define XPCS_WRAP_IRQ_STATUS 0x8050
> > +#define XPCS_WRAP_IRQ_STATUS_PCS_LINK_STS BIT(6)
> > +
> > +#define XPCS_REG_ADDR_SHIFT 10
> > +#define XPCS_REG_ADDR_MASK 0x1fff
> > +#define XPCS_ADDR 0x3fc
> > +
> > +#define MGBE_WRAP_COMMON_INTR_ENABLE	0x8704
> > +#define MAC_SBD_INTR			BIT(2)
> > +#define MGBE_WRAP_AXI_ASID0_CTRL	0x8400
> > +#define MGBE_SID			0x6
> > +
> > +static void mgbe_uphy_lane_bringup(struct tegra_mgbe *mgbe)
> > +{
> > +	unsigned int retry = 300;
> > +	u32 value;
> > +	int err;
> > +
> > +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_STATUS);
> > +	if ((value & XPCS_WRAP_UPHY_STATUS_TX_P_UP) == 0) {
> > +		value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_HW_INIT_CTRL);
> > +		value |= XPCS_WRAP_UPHY_HW_INIT_CTRL_TX_EN;
> > +		writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_HW_INIT_CTRL);
> > +	}
> > +
> > +	err = readl_poll_timeout(mgbe->xpcs + XPCS_WRAP_UPHY_HW_INIT_CTRL, value,
> > +				 (value & XPCS_WRAP_UPHY_HW_INIT_CTRL_TX_EN) == 0,
> > +				 500, 500 * 2000);
> > +	if (err < 0)
> > +		dev_err(mgbe->dev, "timeout waiting for TX lane to become enabled\n");
> 
> Why you don't need to propagate this error to the caller?
> 
> Same question for more error cases below.

I suspect that we can simply propagate the error in these cases. We
never run into these issues in practice, so it went unnoticed.

> 
> > +
> > +	usleep_range(10000, 20000);
> > +
> > +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_SW_OVRD;
> > +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +
> > +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +	value &= ~XPCS_WRAP_UPHY_RX_CONTROL_RX_IDDQ;
> > +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +
> > +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +	value &= ~XPCS_WRAP_UPHY_RX_CONTROL_AUX_RX_IDDQ;
> > +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +
> > +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +	value &= ~XPCS_WRAP_UPHY_RX_CONTROL_RX_SLEEP;
> > +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +
> > +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_CAL_EN;
> > +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +
> > +	err = readl_poll_timeout(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL, value,
> > +				 (value & XPCS_WRAP_UPHY_RX_CONTROL_RX_CAL_EN) == 0,
> > +				 1000, 1000 * 2000);
> > +	if (err < 0)
> > +		dev_err(mgbe->dev, "timeout waiting for RX calibration to become enabled\n");
> > +
> > +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_DATA_EN;
> > +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +
> > +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_CDR_RESET;
> > +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +
> > +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +	value &= ~XPCS_WRAP_UPHY_RX_CONTROL_RX_CDR_RESET;
> > +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +
> > +	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_PCS_PHY_RDY;
> > +	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
> > +
> > +	while (--retry) {
> > +		err = readl_poll_timeout(mgbe->xpcs + XPCS_WRAP_IRQ_STATUS, value,
> > +					 value & XPCS_WRAP_IRQ_STATUS_PCS_LINK_STS,
> > +					 500, 500 * 2000);
> > +		if (err < 0) {
> > +			dev_err(mgbe->dev, "timeout waiting for link to become ready\n");
> > +			usleep_range(10000, 20000);
> > +			continue;
> > +		}
> > +		break;
> > +	}
> 
> It looks like the above loop can take up to 150 seconds (300
> iterations, 500000usec each), can it be shortned?

This is likely left-over from debugging. It might be possible to get rid
of the loop altogether and just use the built-in retry mechanism from
readl_poll_timeout().

Bhadram, do you have any concerns with removing the outer while loop
here? In your experience, if the link doesn't become ready within the 1
second timeout of the readl_poll_timeout() call, is it at all likely to
succeed subsequently or can we safely assume that something has gone
wrong?

> 
> > +
> > +	/* clear status */
> > +	writel(value, mgbe->xpcs + XPCS_WRAP_IRQ_STATUS);
> > +}
> > +
> > +static int tegra_mgbe_probe(struct platform_device *pdev)
> > +{
> > +	struct plat_stmmacenet_data *plat;
> > +	struct stmmac_resources res;
> > +	struct tegra_mgbe *mgbe;
> > +	int irq, err, i;
> > +
> > +	mgbe = devm_kzalloc(&pdev->dev, sizeof(*mgbe), GFP_KERNEL);
> > +	if (!mgbe)
> > +		return -ENOMEM;
> > +
> > +	mgbe->dev = &pdev->dev;
> > +
> > +	memset(&res, 0, sizeof(res));
> > +
> > +	irq = platform_get_irq(pdev, 0);
> > +	if (irq < 0)
> > +		return irq;
> > +
> > +	mgbe->hv = devm_platform_ioremap_resource_byname(pdev, "hypervisor");
> > +	if (IS_ERR(mgbe->hv))
> > +		return PTR_ERR(mgbe->hv);
> > +
> > +	mgbe->regs = devm_platform_ioremap_resource_byname(pdev, "mac");
> > +	if (IS_ERR(mgbe->regs))
> > +		return PTR_ERR(mgbe->regs);
> > +
> > +	mgbe->xpcs = devm_platform_ioremap_resource_byname(pdev, "xpcs");
> > +	if (IS_ERR(mgbe->xpcs))
> > +		return PTR_ERR(mgbe->xpcs);
> > +
> > +	res.addr = mgbe->regs;
> > +	res.irq = irq;
> > +
> > +	mgbe->clks = devm_kzalloc(&pdev->dev, sizeof(*mgbe->clks), GFP_KERNEL);
> > +	if (!mgbe->clks)
> > +		return -ENOMEM;
> > +
> > +	for (i = 0; i <  ARRAY_SIZE(mgbe_clks); i++)
> > +		mgbe->clks[i].id = mgbe_clks[i];
> > +
> > +	err = devm_clk_bulk_get(mgbe->dev, ARRAY_SIZE(mgbe_clks), mgbe->clks);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	err = clk_bulk_prepare_enable(ARRAY_SIZE(mgbe_clks), mgbe->clks);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	/* Perform MAC reset */
> > +	mgbe->rst_mac = devm_reset_control_get(&pdev->dev, "mac");
> > +	if (IS_ERR(mgbe->rst_mac))
> > +		return PTR_ERR(mgbe->rst_mac);
> > +
> > +	err = reset_control_assert(mgbe->rst_mac);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	usleep_range(2000, 4000);
> > +
> > +	err = reset_control_deassert(mgbe->rst_mac);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	/* Perform PCS reset */
> > +	mgbe->rst_pcs = devm_reset_control_get(&pdev->dev, "pcs");
> > +	if (IS_ERR(mgbe->rst_pcs))
> > +		return PTR_ERR(mgbe->rst_pcs);
> > +
> > +	err = reset_control_assert(mgbe->rst_pcs);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	usleep_range(2000, 4000);
> > +
> > +	err = reset_control_deassert(mgbe->rst_pcs);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	plat = stmmac_probe_config_dt(pdev, res.mac);
> > +	if (IS_ERR(plat))
> > +		return PTR_ERR(plat);
> > +
> > +	plat->has_xgmac = 1;
> > +	plat->tso_en = 1;
> > +	plat->pmt = 1;
> > +	plat->bsp_priv = mgbe;
> > +
> > +	if (!plat->mdio_node)
> > +		plat->mdio_node = of_get_child_by_name(pdev->dev.of_node, "mdio");
> > +
> > +	if (!plat->mdio_bus_data) {
> > +		plat->mdio_bus_data = devm_kzalloc(&pdev->dev, sizeof(*plat->mdio_bus_data),
> > +						   GFP_KERNEL);
> > +		if (!plat->mdio_bus_data) {
> > +			err = -ENOMEM;
> > +			goto remove;
> > +		}
> > +	}
> > +
> > +	plat->mdio_bus_data->needs_reset = true;
> > +
> > +	mgbe_uphy_lane_bringup(mgbe);
> > +
> > +	/* Tx FIFO Size - 128KB */
> > +	plat->tx_fifo_size = 131072;
> > +	/* Rx FIFO Size - 192KB */
> > +	plat->rx_fifo_size = 196608;
> > +
> > +	/* Enable common interrupt at wrapper level */
> > +	writel(MAC_SBD_INTR, mgbe->regs + MGBE_WRAP_COMMON_INTR_ENABLE);
> > +
> > +	/* Program SID */
> > +	writel(MGBE_SID, mgbe->hv + MGBE_WRAP_AXI_ASID0_CTRL);
> > +
> > +	err = stmmac_dvr_probe(&pdev->dev, plat, &res);
> > +	if (err < 0)
> > +		goto remove;
> > +
> > +	return 0;
> > +
> > +remove:
> > +	stmmac_remove_config_dt(pdev, plat);
> > +	return err;
> > +}
> > +
> > +static int tegra_mgbe_remove(struct platform_device *pdev)
> > +{
> > +	struct tegra_mgbe *mgbe = get_stmmac_bsp_priv(&pdev->dev);
> > +
> > +	clk_bulk_disable_unprepare(ARRAY_SIZE(mgbe_clks), mgbe->clks);
> > +
> > +	stmmac_pltfr_remove(pdev);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id tegra_mgbe_match[] = {
> > +	{ .compatible = "nvidia,tegra234-mgbe", },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, tegra_mgbe_match);
> 
> The DT bindings will land in 6.1, right?

Yes, the DT bindings and the device tree changes for Jetson AGX Orin
are currently targetted for 6.1.

Thierry
Florian Fainelli Sept. 27, 2022, 5:23 p.m. UTC | #3
+Russell, Andrew, Vladimir,

On 9/23/22 04:49, Thierry Reding wrote:
> From: Bhadram Varka <vbhadram@nvidia.com>
> 
> Add support for the Multi-Gigabit Ethernet (MGBE/XPCS) IP found on
> NVIDIA Tegra234 SoCs.
> 
> Signed-off-by: Bhadram Varka <vbhadram@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>   drivers/net/ethernet/stmicro/stmmac/Kconfig   |   6 +
>   drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
>   .../net/ethernet/stmicro/stmmac/dwmac-tegra.c | 290 ++++++++++++++++++

You should be modeling this as a proper PCS driver and have a 
'pcs-handle' property pointing to it in your Device Tree.

The configuration you are doing here is probably working the first time 
you bring-up the network device but I doubt it works across system 
suspend/resume states where power to the GMAC and PCS is lost, it also 
begs the question of which mediums this was tested with and whether 
dynamic switching of speeds and so on is working?
--
Florian
Bhadram Varka Oct. 12, 2022, 4:56 a.m. UTC | #4
Hi @Florian Fainelli,
Thanks for the review.

> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: 27 September 2022 10:53 PM
> To: Thierry Reding <thierry.reding@gmail.com>; David S . Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Russell
> King <linux@armlinux.org.uk>; Andrew Lunn <andrew@lunn.ch>; Vladimir
> Oltean <olteanv@gmail.com>
> Cc: Jonathan Hunter <jonathanh@nvidia.com>; Bhadram Varka
> <vbhadram@nvidia.com>; linux-tegra@vger.kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH net-next v4 RESEND] stmmac: tegra: Add MGBE support
> 
> External email: Use caution opening links or attachments
> 
> 
> +Russell, Andrew, Vladimir,
> 
> On 9/23/22 04:49, Thierry Reding wrote:
> > From: Bhadram Varka <vbhadram@nvidia.com>
> >
> > Add support for the Multi-Gigabit Ethernet (MGBE/XPCS) IP found on
> > NVIDIA Tegra234 SoCs.
> >
> > Signed-off-by: Bhadram Varka <vbhadram@nvidia.com>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >   drivers/net/ethernet/stmicro/stmmac/Kconfig   |   6 +
> >   drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
> >   .../net/ethernet/stmicro/stmmac/dwmac-tegra.c | 290
> > ++++++++++++++++++
> 
> You should be modeling this as a proper PCS driver and have a 'pcs-handle'
> property pointing to it in your Device Tree.
> 
> The configuration you are doing here is probably working the first time you
> bring-up the network device but I doubt it works across system
> suspend/resume states where power to the GMAC and PCS is lost, it also
> begs the question of which mediums this was tested with and whether
> dynamic switching of speeds and so on is working?
> --

For Tegra234, there is UPHY lanes control logic inside XPCS IP which is memory-mapped IP (not part of the MAC IP).
mgbe_uphy_lane_bringup performs UPHY lane bring up here. Here MGBE/XPCS works in XFI mode.

Agree that lane bring down logic is not present interface down/suspend paths. Will update the changes accordingly.
One more thing is that UPHY lane bring should happen only after the line side link is up. This also will make the changes.
Please let me know if I miss anything here.

Thanks,
Bhadram.
Jon Hunter Nov. 18, 2022, 9:23 a.m. UTC | #5
Hi Florian,

On 12/10/2022 05:56, Bhadram Varka wrote:

...

>> You should be modeling this as a proper PCS driver and have a 'pcs-handle'
>> property pointing to it in your Device Tree.
>>
>> The configuration you are doing here is probably working the first time you
>> bring-up the network device but I doubt it works across system
>> suspend/resume states where power to the GMAC and PCS is lost, it also
>> begs the question of which mediums this was tested with and whether
>> dynamic switching of speeds and so on is working?
>> --
> 
> For Tegra234, there is UPHY lanes control logic inside XPCS IP which is memory-mapped IP (not part of the MAC IP).
> mgbe_uphy_lane_bringup performs UPHY lane bring up here. Here MGBE/XPCS works in XFI mode.
> 
> Agree that lane bring down logic is not present interface down/suspend paths. Will update the changes accordingly.
> One more thing is that UPHY lane bring should happen only after the line side link is up. This also will make the changes.
> Please let me know if I miss anything here.


An updated version of this has now been posted [0]. It should have been 
marked as 'V5'. We have tested suspend/resume and verified that it is 
working. We are hoping to get this into Linux v6.2 if not too late. Let 
me know if you have any concerns.

Thanks
Jon

[0] 
https://lore.kernel.org/linux-tegra/20221118075744.49442-1-ruppala@nvidia.com/T/#t
Vladimir Oltean Nov. 18, 2022, 1:02 p.m. UTC | #6
Hi Bhadram,

On Wed, Oct 12, 2022 at 04:56:52AM +0000, Bhadram Varka wrote:
> > You should be modeling this as a proper PCS driver and have a 'pcs-handle'
> > property pointing to it in your Device Tree.
> > 
> > The configuration you are doing here is probably working the first time you
> > bring-up the network device but I doubt it works across system
> > suspend/resume states where power to the GMAC and PCS is lost, it also
> > begs the question of which mediums this was tested with and whether
> > dynamic switching of speeds and so on is working?
> > --
> 
> For Tegra234, there is UPHY lanes control logic inside XPCS IP which is memory-mapped IP (not part of the MAC IP).
> mgbe_uphy_lane_bringup performs UPHY lane bring up here. Here MGBE/XPCS works in XFI mode.
> 
> Agree that lane bring down logic is not present interface down/suspend paths. Will update the changes accordingly.
> One more thing is that UPHY lane bring should happen only after the line side link is up. This also will make the changes.
> Please let me know if I miss anything here.

What about the non-UPHY part of the XPCS IP, how does the dwmac-tegra.c
driver control it/see the status it reports?
Bhadram Varka Nov. 22, 2022, 7:05 a.m. UTC | #7
Hi Vladimir,

> -----Original Message-----
> From: Vladimir Oltean <olteanv@gmail.com>
> Sent: 18 November 2022 06:32 PM
> To: Bhadram Varka <vbhadram@nvidia.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>; Thierry Reding
> <thierry.reding@gmail.com>; David S . Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>; Russell King <linux@armlinux.org.uk>;
> Andrew Lunn <andrew@lunn.ch>; Revanth Kumar Uppala
> <ruppala@nvidia.com>; Jonathan Hunter <jonathanh@nvidia.com>; linux-
> tegra@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH net-next v4 RESEND] stmmac: tegra: Add MGBE support
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Bhadram,
> 
> On Wed, Oct 12, 2022 at 04:56:52AM +0000, Bhadram Varka wrote:
> > > You should be modeling this as a proper PCS driver and have a 'pcs-
> handle'
> > > property pointing to it in your Device Tree.
> > >
> > > The configuration you are doing here is probably working the first
> > > time you bring-up the network device but I doubt it works across
> > > system suspend/resume states where power to the GMAC and PCS is
> > > lost, it also begs the question of which mediums this was tested
> > > with and whether dynamic switching of speeds and so on is working?
> > > --
> >
> > For Tegra234, there is UPHY lanes control logic inside XPCS IP which is
> memory-mapped IP (not part of the MAC IP).
> > mgbe_uphy_lane_bringup performs UPHY lane bring up here. Here
> MGBE/XPCS works in XFI mode.
> >
> > Agree that lane bring down logic is not present interface down/suspend
> paths. Will update the changes accordingly.
> > One more thing is that UPHY lane bring should happen only after the line
> side link is up. This also will make the changes.
> > Please let me know if I miss anything here.
> 
> What about the non-UPHY part of the XPCS IP, how does the dwmac-tegra.c
> driver control it/see the status it reports?

Reset values of XPCS IP take care of configuring the IP in 10G mode. No need for extra register programming is required from the driver side. The only status that the driver expects from XPCS IP is RLU to be up which will be done by serdes_up in recent posted changes. Please let me know if any other queries on recent changes [0]

Thank You!

[0]: https://patchwork.ozlabs.org/project/linux-tegra/patch/20221118075744.49442-2-ruppala@nvidia.com/
Vladimir Oltean Nov. 22, 2022, 1:26 p.m. UTC | #8
On Tue, Nov 22, 2022 at 07:05:22AM +0000, Bhadram Varka wrote:
> Reset values of XPCS IP take care of configuring the IP in 10G mode.
> No need for extra register programming is required from the driver
> side. The only status that the driver expects from XPCS IP is RLU to
> be up which will be done by serdes_up in recent posted changes. Please
> let me know if any other queries on recent changes [0]
> 
> Thank You!
> 
> [0]: https://patchwork.ozlabs.org/project/linux-tegra/patch/20221118075744.49442-2-ruppala@nvidia.com/

What about link status reporting, if the XPCS is connected to an SFP cage?

What I'm trying to get at is that maybe it would be useful to consider
the pcs-xpcs.c phylink pcs driver, even if your XPCS IP is memory mapped,
that is not a problem. Using mdiobus_register(), you can create your own
"MDIO" controller with custom bus read() and write() operations which
translate C45 accesses as seen by the xpcs driver into proper MMIO
accesses at the right address.

If I understand the hardware model right, the XPCS MDIO bus could be
exported by a common, top-level SERDES driver. In addition to the XPCS
MDIO bus, it would also model the lanes as generic PHY devices, on which
you could call phy_set_mode_ext(serdes, PHY_MODE_ETHERNET, phy_mode),
and phy_power_on()/phy_power_off().

Can your SERDES lanes also operate in PCIe mode? If yes, how is the
selection between PCIe and Ethernet/XPCS done?
Bhadram Varka Nov. 25, 2022, 4:14 a.m. UTC | #9
Hi Vladimir,

> -----Original Message-----
> From: Vladimir Oltean <olteanv@gmail.com>
> Sent: 22 November 2022 06:56 PM
> To: Bhadram Varka <vbhadram@nvidia.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>; Thierry Reding
> <thierry.reding@gmail.com>; David S . Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; Russell King <linux@armlinux.org.uk>; Andrew
> Lunn <andrew@lunn.ch>; Revanth Kumar Uppala <ruppala@nvidia.com>;
> Jonathan Hunter <jonathanh@nvidia.com>; linux-tegra@vger.kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH net-next v4 RESEND] stmmac: tegra: Add MGBE support
> 
> External email: Use caution opening links or attachments
> 
> 
> On Tue, Nov 22, 2022 at 07:05:22AM +0000, Bhadram Varka wrote:
> > Reset values of XPCS IP take care of configuring the IP in 10G mode.
> > No need for extra register programming is required from the driver
> > side. The only status that the driver expects from XPCS IP is RLU to
> > be up which will be done by serdes_up in recent posted changes. Please
> > let me know if any other queries on recent changes [0]
> >
> > Thank You!
> >
> > [0]:
> > https://patchwork.ozlabs.org/project/linux-tegra/patch/20221118075744.
> > 49442-2-ruppala@nvidia.com/
> 
> What about link status reporting, if the XPCS is connected to an SFP cage?
> 
> What I'm trying to get at is that maybe it would be useful to consider the pcs-
> xpcs.c phylink pcs driver, even if your XPCS IP is memory mapped, that is not a
> problem. Using mdiobus_register(), you can create your own "MDIO"
> controller with custom bus read() and write() operations which translate C45
> accesses as seen by the xpcs driver into proper MMIO accesses at the right
> address.
> 
Except UPHY lane bring up through XPCS IP wrapper, nothing extra done from driver.
I think serdes_up/down function pointers gave the feasibility to do the same.

> If I understand the hardware model right, the XPCS MDIO bus could be
> exported by a common, top-level SERDES driver. In addition to the XPCS MDIO
> bus, it would also model the lanes as generic PHY devices, on which you could
> call phy_set_mode_ext(serdes, PHY_MODE_ETHERNET, phy_mode), and
> phy_power_on()/phy_power_off().

There is no MDIO bus in XPCS IP.

> Can your SERDES lanes also operate in PCIe mode? If yes, how is the selection
> between PCIe and Ethernet/XPCS done?
No. It only operates in XFI.

Please let me know if there are any comments.

Thanks!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 31ff35174034..e9f61bdaf7c4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -235,6 +235,12 @@  config DWMAC_INTEL_PLAT
 	  the stmmac device driver. This driver is used for the Intel Keem Bay
 	  SoC.
 
+config DWMAC_TEGRA
+	tristate "NVIDIA Tegra MGBE support"
+	depends on ARCH_TEGRA || COMPILE_TEST
+	help
+	  Support for the MGBE controller found on Tegra SoCs.
+
 config DWMAC_VISCONTI
 	tristate "Toshiba Visconti DWMAC support"
 	default ARCH_VISCONTI
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index d4e12e9ace4f..057e4bab5c08 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -31,6 +31,7 @@  obj-$(CONFIG_DWMAC_DWC_QOS_ETH)	+= dwmac-dwc-qos-eth.o
 obj-$(CONFIG_DWMAC_INTEL_PLAT)	+= dwmac-intel-plat.o
 obj-$(CONFIG_DWMAC_GENERIC)	+= dwmac-generic.o
 obj-$(CONFIG_DWMAC_IMX8)	+= dwmac-imx.o
+obj-$(CONFIG_DWMAC_TEGRA)	+= dwmac-tegra.o
 obj-$(CONFIG_DWMAC_VISCONTI)	+= dwmac-visconti.o
 stmmac-platform-objs:= stmmac_platform.o
 dwmac-altr-socfpga-objs := altr_tse_pcs.o dwmac-socfpga.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c
new file mode 100644
index 000000000000..bb4b540820fa
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c
@@ -0,0 +1,290 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/platform_device.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+#include <linux/stmmac.h>
+#include <linux/clk.h>
+
+#include "stmmac_platform.h"
+
+static const char *const mgbe_clks[] = {
+	"rx-pcs", "tx", "tx-pcs", "mac-divider", "mac", "mgbe", "ptp-ref", "mac"
+};
+
+struct tegra_mgbe {
+	struct device *dev;
+
+	struct clk_bulk_data *clks;
+
+	struct reset_control *rst_mac;
+	struct reset_control *rst_pcs;
+
+	void __iomem *hv;
+	void __iomem *regs;
+	void __iomem *xpcs;
+
+	struct mii_bus *mii;
+};
+
+#define XPCS_WRAP_UPHY_RX_CONTROL 0x801c
+#define XPCS_WRAP_UPHY_RX_CONTROL_RX_SW_OVRD BIT(31)
+#define XPCS_WRAP_UPHY_RX_CONTROL_RX_PCS_PHY_RDY BIT(10)
+#define XPCS_WRAP_UPHY_RX_CONTROL_RX_CDR_RESET BIT(9)
+#define XPCS_WRAP_UPHY_RX_CONTROL_RX_CAL_EN BIT(8)
+#define XPCS_WRAP_UPHY_RX_CONTROL_RX_SLEEP (BIT(7) | BIT(6))
+#define XPCS_WRAP_UPHY_RX_CONTROL_AUX_RX_IDDQ BIT(5)
+#define XPCS_WRAP_UPHY_RX_CONTROL_RX_IDDQ BIT(4)
+#define XPCS_WRAP_UPHY_RX_CONTROL_RX_DATA_EN BIT(0)
+#define XPCS_WRAP_UPHY_HW_INIT_CTRL 0x8020
+#define XPCS_WRAP_UPHY_HW_INIT_CTRL_TX_EN BIT(0)
+#define XPCS_WRAP_UPHY_HW_INIT_CTRL_RX_EN BIT(2)
+#define XPCS_WRAP_UPHY_STATUS 0x8044
+#define XPCS_WRAP_UPHY_STATUS_TX_P_UP BIT(0)
+#define XPCS_WRAP_IRQ_STATUS 0x8050
+#define XPCS_WRAP_IRQ_STATUS_PCS_LINK_STS BIT(6)
+
+#define XPCS_REG_ADDR_SHIFT 10
+#define XPCS_REG_ADDR_MASK 0x1fff
+#define XPCS_ADDR 0x3fc
+
+#define MGBE_WRAP_COMMON_INTR_ENABLE	0x8704
+#define MAC_SBD_INTR			BIT(2)
+#define MGBE_WRAP_AXI_ASID0_CTRL	0x8400
+#define MGBE_SID			0x6
+
+static void mgbe_uphy_lane_bringup(struct tegra_mgbe *mgbe)
+{
+	unsigned int retry = 300;
+	u32 value;
+	int err;
+
+	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_STATUS);
+	if ((value & XPCS_WRAP_UPHY_STATUS_TX_P_UP) == 0) {
+		value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_HW_INIT_CTRL);
+		value |= XPCS_WRAP_UPHY_HW_INIT_CTRL_TX_EN;
+		writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_HW_INIT_CTRL);
+	}
+
+	err = readl_poll_timeout(mgbe->xpcs + XPCS_WRAP_UPHY_HW_INIT_CTRL, value,
+				 (value & XPCS_WRAP_UPHY_HW_INIT_CTRL_TX_EN) == 0,
+				 500, 500 * 2000);
+	if (err < 0)
+		dev_err(mgbe->dev, "timeout waiting for TX lane to become enabled\n");
+
+	usleep_range(10000, 20000);
+
+	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_SW_OVRD;
+	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+
+	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+	value &= ~XPCS_WRAP_UPHY_RX_CONTROL_RX_IDDQ;
+	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+
+	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+	value &= ~XPCS_WRAP_UPHY_RX_CONTROL_AUX_RX_IDDQ;
+	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+
+	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+	value &= ~XPCS_WRAP_UPHY_RX_CONTROL_RX_SLEEP;
+	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+
+	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_CAL_EN;
+	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+
+	err = readl_poll_timeout(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL, value,
+				 (value & XPCS_WRAP_UPHY_RX_CONTROL_RX_CAL_EN) == 0,
+				 1000, 1000 * 2000);
+	if (err < 0)
+		dev_err(mgbe->dev, "timeout waiting for RX calibration to become enabled\n");
+
+	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_DATA_EN;
+	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+
+	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_CDR_RESET;
+	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+
+	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+	value &= ~XPCS_WRAP_UPHY_RX_CONTROL_RX_CDR_RESET;
+	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+
+	value = readl(mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+	value |= XPCS_WRAP_UPHY_RX_CONTROL_RX_PCS_PHY_RDY;
+	writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL);
+
+	while (--retry) {
+		err = readl_poll_timeout(mgbe->xpcs + XPCS_WRAP_IRQ_STATUS, value,
+					 value & XPCS_WRAP_IRQ_STATUS_PCS_LINK_STS,
+					 500, 500 * 2000);
+		if (err < 0) {
+			dev_err(mgbe->dev, "timeout waiting for link to become ready\n");
+			usleep_range(10000, 20000);
+			continue;
+		}
+		break;
+	}
+
+	/* clear status */
+	writel(value, mgbe->xpcs + XPCS_WRAP_IRQ_STATUS);
+}
+
+static int tegra_mgbe_probe(struct platform_device *pdev)
+{
+	struct plat_stmmacenet_data *plat;
+	struct stmmac_resources res;
+	struct tegra_mgbe *mgbe;
+	int irq, err, i;
+
+	mgbe = devm_kzalloc(&pdev->dev, sizeof(*mgbe), GFP_KERNEL);
+	if (!mgbe)
+		return -ENOMEM;
+
+	mgbe->dev = &pdev->dev;
+
+	memset(&res, 0, sizeof(res));
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	mgbe->hv = devm_platform_ioremap_resource_byname(pdev, "hypervisor");
+	if (IS_ERR(mgbe->hv))
+		return PTR_ERR(mgbe->hv);
+
+	mgbe->regs = devm_platform_ioremap_resource_byname(pdev, "mac");
+	if (IS_ERR(mgbe->regs))
+		return PTR_ERR(mgbe->regs);
+
+	mgbe->xpcs = devm_platform_ioremap_resource_byname(pdev, "xpcs");
+	if (IS_ERR(mgbe->xpcs))
+		return PTR_ERR(mgbe->xpcs);
+
+	res.addr = mgbe->regs;
+	res.irq = irq;
+
+	mgbe->clks = devm_kzalloc(&pdev->dev, sizeof(*mgbe->clks), GFP_KERNEL);
+	if (!mgbe->clks)
+		return -ENOMEM;
+
+	for (i = 0; i <  ARRAY_SIZE(mgbe_clks); i++)
+		mgbe->clks[i].id = mgbe_clks[i];
+
+	err = devm_clk_bulk_get(mgbe->dev, ARRAY_SIZE(mgbe_clks), mgbe->clks);
+	if (err < 0)
+		return err;
+
+	err = clk_bulk_prepare_enable(ARRAY_SIZE(mgbe_clks), mgbe->clks);
+	if (err < 0)
+		return err;
+
+	/* Perform MAC reset */
+	mgbe->rst_mac = devm_reset_control_get(&pdev->dev, "mac");
+	if (IS_ERR(mgbe->rst_mac))
+		return PTR_ERR(mgbe->rst_mac);
+
+	err = reset_control_assert(mgbe->rst_mac);
+	if (err < 0)
+		return err;
+
+	usleep_range(2000, 4000);
+
+	err = reset_control_deassert(mgbe->rst_mac);
+	if (err < 0)
+		return err;
+
+	/* Perform PCS reset */
+	mgbe->rst_pcs = devm_reset_control_get(&pdev->dev, "pcs");
+	if (IS_ERR(mgbe->rst_pcs))
+		return PTR_ERR(mgbe->rst_pcs);
+
+	err = reset_control_assert(mgbe->rst_pcs);
+	if (err < 0)
+		return err;
+
+	usleep_range(2000, 4000);
+
+	err = reset_control_deassert(mgbe->rst_pcs);
+	if (err < 0)
+		return err;
+
+	plat = stmmac_probe_config_dt(pdev, res.mac);
+	if (IS_ERR(plat))
+		return PTR_ERR(plat);
+
+	plat->has_xgmac = 1;
+	plat->tso_en = 1;
+	plat->pmt = 1;
+	plat->bsp_priv = mgbe;
+
+	if (!plat->mdio_node)
+		plat->mdio_node = of_get_child_by_name(pdev->dev.of_node, "mdio");
+
+	if (!plat->mdio_bus_data) {
+		plat->mdio_bus_data = devm_kzalloc(&pdev->dev, sizeof(*plat->mdio_bus_data),
+						   GFP_KERNEL);
+		if (!plat->mdio_bus_data) {
+			err = -ENOMEM;
+			goto remove;
+		}
+	}
+
+	plat->mdio_bus_data->needs_reset = true;
+
+	mgbe_uphy_lane_bringup(mgbe);
+
+	/* Tx FIFO Size - 128KB */
+	plat->tx_fifo_size = 131072;
+	/* Rx FIFO Size - 192KB */
+	plat->rx_fifo_size = 196608;
+
+	/* Enable common interrupt at wrapper level */
+	writel(MAC_SBD_INTR, mgbe->regs + MGBE_WRAP_COMMON_INTR_ENABLE);
+
+	/* Program SID */
+	writel(MGBE_SID, mgbe->hv + MGBE_WRAP_AXI_ASID0_CTRL);
+
+	err = stmmac_dvr_probe(&pdev->dev, plat, &res);
+	if (err < 0)
+		goto remove;
+
+	return 0;
+
+remove:
+	stmmac_remove_config_dt(pdev, plat);
+	return err;
+}
+
+static int tegra_mgbe_remove(struct platform_device *pdev)
+{
+	struct tegra_mgbe *mgbe = get_stmmac_bsp_priv(&pdev->dev);
+
+	clk_bulk_disable_unprepare(ARRAY_SIZE(mgbe_clks), mgbe->clks);
+
+	stmmac_pltfr_remove(pdev);
+
+	return 0;
+}
+
+static const struct of_device_id tegra_mgbe_match[] = {
+	{ .compatible = "nvidia,tegra234-mgbe", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tegra_mgbe_match);
+
+static struct platform_driver tegra_mgbe_driver = {
+	.probe = tegra_mgbe_probe,
+	.remove = tegra_mgbe_remove,
+	.driver = {
+		.name = "tegra-mgbe",
+		.pm		= &stmmac_pltfr_pm_ops,
+		.of_match_table = tegra_mgbe_match,
+	},
+};
+module_platform_driver(tegra_mgbe_driver);
+
+MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>");
+MODULE_DESCRIPTION("NVIDIA Tegra MGBE driver");
+MODULE_LICENSE("GPL");