[PATCH 0/7] net: stmmac: Fixes and Tegra186 support

[PATCH 0/7] net: stmmac: Fixes and Tegra186 support

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Hi everyone,

This series of patches start with a few cleanups that I ran across while
adding Tegra186 support to the stmmac driver. It then adds code for FIFO
size parsing from feature registers and finally enables support for the
incarnation of the Synopsys DWC QOS IP found on NVIDIA Tegra186 SoCs.

This is based on next-20170223.

Thanks,
Thierry

Thierry Reding (7):
  net: stmmac: Rename clk_ptp_ref clock to ptp_ref
  net: stmmac: Balance PTP reference clock enable/disable
  net: stmmac: Check for DMA mapping errors
  net: stmmac: Parse FIFO sizes from feature registers
  net: stmmac: Program RX queue size and flow control
  net: stmmac: dwc-qos: Split out ->probe() and ->remove()
  net: stmmac: dwc-qos: Add Tegra186 support

 Documentation/devicetree/bindings/net/stmmac.txt   |   6 +-
 drivers/net/ethernet/stmicro/stmmac/common.h       |   3 +
 .../ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c    | 366 +++++++++++++++++++--
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h       |  12 +
 drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c   |  45 ++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |   9 +
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |   3 +-
 7 files changed, 411 insertions(+), 33 deletions(-)

-- 
2.11.1

[PATCH 1/7] net: stmmac: Rename clk_ptp_ref clock to ptp_ref

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

There aren't currently any users of the "clk_ptp_ref", but there are
other references to "ptp_ref", so I'm leaning towards considering that a
typo. Fix it.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 Documentation/devicetree/bindings/net/stmmac.txt      | 6 +++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
index d3bfc2b30fb5..11b27dfd1627 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -28,9 +28,9 @@ Optional properties:
   clocks may be specified in derived bindings.
 - clock-names: One name for each entry in the clocks property, the
   first one should be "stmmaceth" and the second one should be "pclk".
-- clk_ptp_ref: this is the PTP reference clock; in case of the PTP is
-  available this clock is used for programming the Timestamp Addend Register.
-  If not passed then the system clock will be used and this is fine on some
+- ptp_ref: this is the PTP reference clock; in case of the PTP is available
+  this clock is used for programming the Timestamp Addend Register. If not
+  passed then the system clock will be used and this is fine on some
   platforms.
 - tx-fifo-depth: See ethernet.txt file in the same directory
 - rx-fifo-depth: See ethernet.txt file in the same directory
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 433a84239a68..5b18355c0d2b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -359,7 +359,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	clk_prepare_enable(plat->pclk);
 
 	/* Fall-back to main clock in case of no PTP ref is passed */
-	plat->clk_ptp_ref = devm_clk_get(&pdev->dev, "clk_ptp_ref");
+	plat->clk_ptp_ref = devm_clk_get(&pdev->dev, "ptp_ref");
 	if (IS_ERR(plat->clk_ptp_ref)) {
 		plat->clk_ptp_rate = clk_get_rate(plat->stmmac_clk);
 		plat->clk_ptp_ref = NULL;
-- 
2.11.1

[PATCH 2/7] net: stmmac: Balance PTP reference clock enable/disable

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

clk_prepare_enable() and clk_disable_unprepare() for this clock aren't
properly balanced, which can trigger a WARN_ON() in the common clock
framework.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 4 ++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 -
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3cbe09682afe..6b7a5ce19589 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1711,6 +1711,10 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
 	stmmac_mmc_setup(priv);
 
 	if (init_ptp) {
+		ret = clk_prepare_enable(priv->plat->clk_ptp_ref);
+		if (ret < 0)
+			netdev_warn(priv->dev, "failed to enable PTP reference clock: %d\n", ret);
+
 		ret = stmmac_init_ptp(priv);
 		if (ret == -EOPNOTSUPP)
 			netdev_warn(priv->dev, "PTP not supported by HW\n");
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 5b18355c0d2b..d285d6cfbd0d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -365,7 +365,6 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 		plat->clk_ptp_ref = NULL;
 		dev_warn(&pdev->dev, "PTP uses main clock\n");
 	} else {
-		clk_prepare_enable(plat->clk_ptp_ref);
 		plat->clk_ptp_rate = clk_get_rate(plat->clk_ptp_ref);
 		dev_dbg(&pdev->dev, "PTP rate %d\n", plat->clk_ptp_rate);
 	}
-- 
2.11.1

[PATCH 3/7] net: stmmac: Check for DMA mapping errors

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

When DMA mapping an SKB fragment, the mapping must be checked for
errors, otherwise the DMA debug code will complain upon unmap.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6b7a5ce19589..d7387919bdb6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2072,6 +2072,8 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
 		des = skb_frag_dma_map(priv->device, frag, 0,
 				       skb_frag_size(frag),
 				       DMA_TO_DEVICE);
+		if (dma_mapping_error(priv->device, des))
+			goto dma_map_err;
 
 		stmmac_tso_allocator(priv, des, skb_frag_size(frag),
 				     (i == nfrags - 1));
-- 
2.11.1

[PATCH 4/7] net: stmmac: Parse FIFO sizes from feature registers

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

New version of this core encode the FIFO sizes in one of the feature
registers. Use these sizes as default, but still allow device tree to
override them for backwards compatibility.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/net/ethernet/stmicro/stmmac/common.h      | 3 +++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c  | 2 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
 3 files changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 144fe84e8a53..6ac653845d82 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -324,6 +324,9 @@ struct dma_features {
 	unsigned int number_tx_queues;
 	/* Alternate (enhanced) DESC mode */
 	unsigned int enh_desc;
+	/* TX and RX FIFO sizes */
+	unsigned int tx_fifo_size;
+	unsigned int rx_fifo_size;
 };
 
 /* GMAC TX FIFO is 8K, Rx FIFO is 16K */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index 377d1b44d4f2..8d249f3b34c8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -296,6 +296,8 @@ static void dwmac4_get_hw_feature(void __iomem *ioaddr,
 	hw_cap = readl(ioaddr + GMAC_HW_FEATURE1);
 	dma_cap->av = (hw_cap & GMAC_HW_FEAT_AVSEL) >> 20;
 	dma_cap->tsoen = (hw_cap & GMAC_HW_TSOEN) >> 18;
+	dma_cap->tx_fifo_size = 128 << ((hw_cap >> 6) & 0x1f);
+	dma_cap->rx_fifo_size = 128 << ((hw_cap >> 0) & 0x1f);
 	/* MAC HW feature2 */
 	hw_cap = readl(ioaddr + GMAC_HW_FEATURE2);
 	/* TX and RX number of channels */
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d7387919bdb6..291e34f0ca94 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1281,6 +1281,9 @@ static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
 {
 	int rxfifosz = priv->plat->rx_fifo_size;
 
+	if (rxfifosz == 0)
+		rxfifosz = priv->dma_cap.rx_fifo_size;
+
 	if (priv->plat->force_thresh_dma_mode)
 		priv->hw->dma->dma_mode(priv->ioaddr, tc, tc, rxfifosz);
 	else if (priv->plat->force_sf_dma_mode || priv->plat->tx_coe) {
-- 
2.11.1

[PATCH 5/7] net: stmmac: Program RX queue size and flow control

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Program the receive queue size based on the RX FIFO size and enable
hardware flow control for large FIFOs.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h     | 12 +++++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 43 ++++++++++++++++++++++--
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index db45134fddf0..9acc1f1252b3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -180,6 +180,7 @@ enum power_event {
 #define MTL_OP_MODE_TSF			BIT(1)
 
 #define MTL_OP_MODE_TQS_MASK		GENMASK(24, 16)
+#define MTL_OP_MODE_TQS_SHIFT		16
 
 #define MTL_OP_MODE_TTC_MASK		0x70
 #define MTL_OP_MODE_TTC_SHIFT		4
@@ -193,6 +194,17 @@ enum power_event {
 #define MTL_OP_MODE_TTC_384		(6 << MTL_OP_MODE_TTC_SHIFT)
 #define MTL_OP_MODE_TTC_512		(7 << MTL_OP_MODE_TTC_SHIFT)
 
+#define MTL_OP_MODE_RQS_MASK		GENMASK(29, 20)
+#define MTL_OP_MODE_RQS_SHIFT		20
+
+#define MTL_OP_MODE_RFD_MASK		GENMASK(19, 14)
+#define MTL_OP_MODE_RFD_SHIFT		14
+
+#define MTL_OP_MODE_RFA_MASK		GENMASK(13, 8)
+#define MTL_OP_MODE_RFA_SHIFT		8
+
+#define MTL_OP_MODE_EHFC		BIT(7)
+
 #define MTL_OP_MODE_RTC_MASK		0x18
 #define MTL_OP_MODE_RTC_SHIFT		3
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index 8d249f3b34c8..03d230201960 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -185,8 +185,9 @@ static void dwmac4_rx_watchdog(void __iomem *ioaddr, u32 riwt)
 }
 
 static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
-				    int rxmode, u32 channel)
+				    int rxmode, u32 channel, int rxfifosz)
 {
+	unsigned int rqs = rxfifosz / 256 - 1;
 	u32 mtl_tx_op, mtl_rx_op, mtl_rx_int;
 
 	/* Following code only done for channel 0, other channels not yet
@@ -252,6 +253,44 @@ static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
 			mtl_rx_op |= MTL_OP_MODE_RTC_128;
 	}
 
+	mtl_rx_op &= ~MTL_OP_MODE_RQS_MASK;
+	mtl_rx_op |= rqs << MTL_OP_MODE_RQS_SHIFT;
+
+	/* enable flow control only if each channel gets 4 KiB or more FIFO */
+	if (rxfifosz >= 4096) {
+		unsigned int rfd, rfa;
+
+		mtl_rx_op |= MTL_OP_MODE_EHFC;
+
+		switch (rxfifosz) {
+		case 4096:
+			rfd = 0x03;
+			rfa = 0x01;
+			break;
+
+		case 8192:
+			rfd = 0x06;
+			rfa = 0x0a;
+			break;
+
+		case 16384:
+			rfd = 0x06;
+			rfa = 0x12;
+			break;
+
+		default:
+			rfd = 0x06;
+			rfa = 0x1e;
+			break;
+		}
+
+		mtl_rx_op &= ~MTL_OP_MODE_RFD_MASK;
+		mtl_rx_op |= rfd << MTL_OP_MODE_RFD_SHIFT;
+
+		mtl_rx_op &= ~MTL_OP_MODE_RFA_MASK;
+		mtl_rx_op |= rfa << MTL_OP_MODE_RFA_SHIFT;
+	}
+
 	writel(mtl_rx_op, ioaddr + MTL_CHAN_RX_OP_MODE(channel));
 
 	/* Enable MTL RX overflow */
@@ -264,7 +303,7 @@ static void dwmac4_dma_operation_mode(void __iomem *ioaddr, int txmode,
 				      int rxmode, int rxfifosz)
 {
 	/* Only Channel 0 is actually configured and used */
-	dwmac4_dma_chan_op_mode(ioaddr, txmode, rxmode, 0);
+	dwmac4_dma_chan_op_mode(ioaddr, txmode, rxmode, 0, rxfifosz);
 }
 
 static void dwmac4_get_hw_feature(void __iomem *ioaddr,
-- 
2.11.1

[PATCH 6/7] net: stmmac: dwc-qos: Split out ->probe() and ->remove()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Split out the binding specific parts of ->probe() and ->remove() to
enable the driver to support variants of the binding. This is useful in
order to keep backwards-compatibility while making it easy for a sub-
driver to deal only with the updated bindings rather than having to add
compatibility quirks all over the place.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 .../ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c    | 114 ++++++++++++++++-----
 1 file changed, 88 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
index 1a3fa3d9f855..5071d3c15adc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
@@ -18,6 +18,7 @@
 #include <linux/io.h>
 #include <linux/ioport.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/of_net.h>
 #include <linux/mfd/syscon.h>
 #include <linux/platform_device.h>
@@ -106,13 +107,70 @@ static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
 	return 0;
 }
 
+static void *dwc_qos_probe(struct platform_device *pdev,
+			   struct plat_stmmacenet_data *plat_dat,
+			   struct stmmac_resources *stmmac_res)
+{
+	int err;
+
+	plat_dat->stmmac_clk = devm_clk_get(&pdev->dev, "apb_pclk");
+	if (IS_ERR(plat_dat->stmmac_clk)) {
+		dev_err(&pdev->dev, "apb_pclk clock not found.\n");
+		return ERR_CAST(plat_dat->stmmac_clk);
+	}
+
+	clk_prepare_enable(plat_dat->stmmac_clk);
+
+	plat_dat->pclk = devm_clk_get(&pdev->dev, "phy_ref_clk");
+	if (IS_ERR(plat_dat->pclk)) {
+		dev_err(&pdev->dev, "phy_ref_clk clock not found.\n");
+		err = PTR_ERR(plat_dat->pclk);
+		goto disable;
+	}
+
+	clk_prepare_enable(plat_dat->pclk);
+
+	return NULL;
+
+disable:
+	clk_disable_unprepare(plat_dat->stmmac_clk);
+	return ERR_PTR(err);
+}
+
+static int dwc_qos_remove(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
+
+	clk_disable_unprepare(priv->plat->pclk);
+	clk_disable_unprepare(priv->plat->stmmac_clk);
+
+	return 0;
+}
+
+struct dwc_eth_dwmac_data {
+	void *(*probe)(struct platform_device *pdev,
+		       struct plat_stmmacenet_data *data,
+		       struct stmmac_resources *res);
+	int (*remove)(struct platform_device *pdev);
+};
+
+static const struct dwc_eth_dwmac_data dwc_qos_data = {
+	.probe = dwc_qos_probe,
+	.remove = dwc_qos_remove,
+};
+
 static int dwc_eth_dwmac_probe(struct platform_device *pdev)
 {
+	const struct dwc_eth_dwmac_data *data;
 	struct plat_stmmacenet_data *plat_dat;
 	struct stmmac_resources stmmac_res;
 	struct resource *res;
+	void *priv;
 	int ret;
 
+	data = of_device_get_match_data(&pdev->dev);
+
 	memset(&stmmac_res, 0, sizeof(struct stmmac_resources));
 
 	/**
@@ -138,39 +196,26 @@ static int dwc_eth_dwmac_probe(struct platform_device *pdev)
 	if (IS_ERR(plat_dat))
 		return PTR_ERR(plat_dat);
 
-	plat_dat->stmmac_clk = devm_clk_get(&pdev->dev, "apb_pclk");
-	if (IS_ERR(plat_dat->stmmac_clk)) {
-		dev_err(&pdev->dev, "apb_pclk clock not found.\n");
-		ret = PTR_ERR(plat_dat->stmmac_clk);
-		plat_dat->stmmac_clk = NULL;
-		goto err_remove_config_dt;
-	}
-	clk_prepare_enable(plat_dat->stmmac_clk);
-
-	plat_dat->pclk = devm_clk_get(&pdev->dev, "phy_ref_clk");
-	if (IS_ERR(plat_dat->pclk)) {
-		dev_err(&pdev->dev, "phy_ref_clk clock not found.\n");
-		ret = PTR_ERR(plat_dat->pclk);
-		plat_dat->pclk = NULL;
-		goto err_out_clk_dis_phy;
+	priv = data->probe(pdev, plat_dat, &stmmac_res);
+	if (IS_ERR(priv)) {
+		ret = PTR_ERR(priv);
+		dev_err(&pdev->dev, "failed to probe subdriver: %d\n", ret);
+		goto remove_config;
 	}
-	clk_prepare_enable(plat_dat->pclk);
 
 	ret = dwc_eth_dwmac_config_dt(pdev, plat_dat);
 	if (ret)
-		goto err_out_clk_dis_aper;
+		goto remove;
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
 	if (ret)
-		goto err_out_clk_dis_aper;
+		goto remove;
 
-	return 0;
+	return ret;
 
-err_out_clk_dis_aper:
-	clk_disable_unprepare(plat_dat->pclk);
-err_out_clk_dis_phy:
-	clk_disable_unprepare(plat_dat->stmmac_clk);
-err_remove_config_dt:
+remove:
+	data->remove(pdev);
+remove_config:
 	stmmac_remove_config_dt(pdev, plat_dat);
 
 	return ret;
@@ -178,11 +223,28 @@ static int dwc_eth_dwmac_probe(struct platform_device *pdev)
 
 static int dwc_eth_dwmac_remove(struct platform_device *pdev)
 {
-	return stmmac_pltfr_remove(pdev);
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	const struct dwc_eth_dwmac_data *data;
+	int err;
+
+	data = of_device_get_match_data(&pdev->dev);
+
+	err = stmmac_dvr_remove(&pdev->dev);
+	if (err < 0)
+		dev_err(&pdev->dev, "failed to remove platform: %d\n", err);
+
+	err = data->remove(pdev);
+	if (err < 0)
+		dev_err(&pdev->dev, "failed to remove subdriver: %d\n", err);
+
+	stmmac_remove_config_dt(pdev, priv->plat);
+
+	return err;
 }
 
 static const struct of_device_id dwc_eth_dwmac_match[] = {
-	{ .compatible = "snps,dwc-qos-ethernet-4.10", },
+	{ .compatible = "snps,dwc-qos-ethernet-4.10", .data = &dwc_qos_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, dwc_eth_dwmac_match);
-- 
2.11.1

[PATCH 7/7] net: stmmac: dwc-qos: Add Tegra186 support

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The NVIDIA Tegra186 SoC contains an instance of the Synopsys DWC
ethernet QOS IP core. The binding that it uses is slightly different
from existing ones because of the integration (clocks, resets, ...).

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 .../ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c    | 252 +++++++++++++++++++++
 1 file changed, 252 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
index 5071d3c15adc..54dfbdc48f6d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
@@ -14,6 +14,7 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/device.h>
+#include <linux/gpio/consumer.h>
 #include <linux/ethtool.h>
 #include <linux/io.h>
 #include <linux/ioport.h>
@@ -22,10 +23,24 @@
 #include <linux/of_net.h>
 #include <linux/mfd/syscon.h>
 #include <linux/platform_device.h>
+#include <linux/reset.h>
 #include <linux/stmmac.h>
 
 #include "stmmac_platform.h"
 
+struct tegra_eqos {
+	struct device *dev;
+	void __iomem *regs;
+
+	struct reset_control *rst;
+	struct clk *clk_master;
+	struct clk *clk_slave;
+	struct clk *clk_tx;
+	struct clk *clk_rx;
+
+	struct gpio_desc *reset;
+};
+
 static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
 				   struct plat_stmmacenet_data *plat_dat)
 {
@@ -148,6 +163,237 @@ static int dwc_qos_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#define SDMEMCOMPPADCTRL 0x8800
+#define  SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD BIT(31)
+
+#define AUTO_CAL_CONFIG 0x8804
+#define  AUTO_CAL_CONFIG_START BIT(31)
+#define  AUTO_CAL_CONFIG_ENABLE BIT(29)
+
+#define AUTO_CAL_STATUS 0x880c
+#define  AUTO_CAL_STATUS_ACTIVE BIT(31)
+
+static void tegra_eqos_fix_speed(void *priv, unsigned int speed)
+{
+	struct tegra_eqos *eqos = priv;
+	unsigned long rate = 125000000;
+	bool needs_calibration = false;
+	unsigned int i;
+	u32 value;
+
+	switch (speed) {
+	case SPEED_1000:
+		needs_calibration = true;
+		rate = 125000000;
+		break;
+
+	case SPEED_100:
+		needs_calibration = true;
+		rate = 25000000;
+		break;
+
+	case SPEED_10:
+		rate = 2500000;
+		break;
+
+	default:
+		dev_err(eqos->dev, "invalid speed %u\n", speed);
+		break;
+	}
+
+	if (needs_calibration) {
+		/* calibrate */
+		value = readl(eqos->regs + SDMEMCOMPPADCTRL);
+		value |= SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD;
+		writel(value, eqos->regs + SDMEMCOMPPADCTRL);
+
+		udelay(1);
+
+		value = readl(eqos->regs + AUTO_CAL_CONFIG);
+		value |= AUTO_CAL_CONFIG_START | AUTO_CAL_CONFIG_ENABLE;
+		writel(value, eqos->regs + AUTO_CAL_CONFIG);
+
+		for (i = 0; i <= 10; i++) {
+			value = readl(eqos->regs + AUTO_CAL_STATUS);
+			if (value & AUTO_CAL_STATUS_ACTIVE)
+				break;
+
+			udelay(1);
+		}
+
+		if ((value & AUTO_CAL_STATUS_ACTIVE) == 0) {
+			dev_err(eqos->dev, "calibration did not start\n");
+			goto failed;
+		}
+
+		for (i = 0; i <= 10; i++) {
+			value = readl(eqos->regs + AUTO_CAL_STATUS);
+			if ((value & AUTO_CAL_STATUS_ACTIVE) == 0)
+				break;
+
+			udelay(20);
+		}
+
+		if (value & AUTO_CAL_STATUS_ACTIVE) {
+			dev_err(eqos->dev, "calibration didn't finish\n");
+			goto failed;
+		}
+
+	failed:
+		value = readl(eqos->regs + SDMEMCOMPPADCTRL);
+		value &= ~SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD;
+		writel(value, eqos->regs + SDMEMCOMPPADCTRL);
+	} else {
+		value = readl(eqos->regs + AUTO_CAL_CONFIG);
+		value &= ~AUTO_CAL_CONFIG_ENABLE;
+		writel(value, eqos->regs + AUTO_CAL_CONFIG);
+	}
+
+	clk_set_rate(eqos->clk_tx, rate);
+}
+
+static int tegra_eqos_init(struct platform_device *pdev, void *priv)
+{
+	struct tegra_eqos *eqos = priv;
+	unsigned long rate;
+	u32 value;
+
+	rate = clk_get_rate(eqos->clk_slave);
+
+	value = readl(eqos->regs + 0xdc);
+	value = (rate / 1000000) - 1;
+	writel(value, eqos->regs + 0xdc);
+
+	return 0;
+}
+
+static void *tegra_eqos_probe(struct platform_device *pdev,
+			      struct plat_stmmacenet_data *data,
+			      struct stmmac_resources *res)
+{
+	struct tegra_eqos *eqos;
+	int err;
+
+	eqos = devm_kzalloc(&pdev->dev, sizeof(*eqos), GFP_KERNEL);
+	if (!eqos) {
+		err = -ENOMEM;
+		goto error;
+	}
+
+	eqos->dev = &pdev->dev;
+	eqos->regs = res->addr;
+
+	eqos->clk_master = devm_clk_get(&pdev->dev, "master_bus");
+	if (IS_ERR(eqos->clk_master)) {
+		err = PTR_ERR(eqos->clk_master);
+		goto error;
+	}
+
+	err = clk_prepare_enable(eqos->clk_master);
+	if (err < 0)
+		goto error;
+
+	eqos->clk_slave = devm_clk_get(&pdev->dev, "slave_bus");
+	if (IS_ERR(eqos->clk_slave)) {
+		err = PTR_ERR(eqos->clk_slave);
+		goto disable_master;
+	}
+
+	data->stmmac_clk = eqos->clk_slave;
+
+	err = clk_prepare_enable(eqos->clk_slave);
+	if (err < 0)
+		goto disable_master;
+
+	eqos->clk_rx = devm_clk_get(&pdev->dev, "rx");
+	if (IS_ERR(eqos->clk_rx)) {
+		err = PTR_ERR(eqos->clk_rx);
+		goto disable_slave;
+	}
+
+	err = clk_prepare_enable(eqos->clk_rx);
+	if (err < 0)
+		goto disable_slave;
+
+	eqos->clk_tx = devm_clk_get(&pdev->dev, "tx");
+	if (IS_ERR(eqos->clk_tx)) {
+		err = PTR_ERR(eqos->clk_tx);
+		goto disable_rx;
+	}
+
+	err = clk_prepare_enable(eqos->clk_tx);
+	if (err < 0)
+		goto disable_rx;
+
+	eqos->reset = devm_gpiod_get(&pdev->dev, "phy-reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(eqos->reset)) {
+		err = PTR_ERR(eqos->reset);
+		goto disable_tx;
+	}
+
+	usleep_range(2000, 4000);
+	gpiod_set_value(eqos->reset, 0);
+
+	eqos->rst = devm_reset_control_get(&pdev->dev, "eqos");
+	if (IS_ERR(eqos->rst)) {
+		err = PTR_ERR(eqos->rst);
+		goto reset_phy;
+	}
+
+	err = reset_control_assert(eqos->rst);
+	if (err < 0)
+		goto reset_phy;
+
+	usleep_range(2000, 4000);
+
+	err = reset_control_deassert(eqos->rst);
+	if (err < 0)
+		goto reset_phy;
+
+	usleep_range(2000, 4000);
+
+	data->fix_mac_speed = tegra_eqos_fix_speed;
+	data->init = tegra_eqos_init;
+	data->bsp_priv = eqos;
+
+	err = tegra_eqos_init(pdev, eqos);
+	if (err < 0)
+		goto reset;
+
+out:
+	return eqos;
+
+reset:
+	reset_control_assert(eqos->rst);
+reset_phy:
+	gpiod_set_value(eqos->reset, 1);
+disable_tx:
+	clk_disable_unprepare(eqos->clk_tx);
+disable_rx:
+	clk_disable_unprepare(eqos->clk_rx);
+disable_slave:
+	clk_disable_unprepare(eqos->clk_slave);
+disable_master:
+	clk_disable_unprepare(eqos->clk_master);
+error:
+	eqos = ERR_PTR(err);
+	goto out;
+}
+
+static int tegra_eqos_remove(struct platform_device *pdev)
+{
+	struct tegra_eqos *eqos = get_stmmac_bsp_priv(&pdev->dev);
+
+	reset_control_assert(eqos->rst);
+	gpiod_set_value(eqos->reset, 1);
+	clk_disable_unprepare(eqos->clk_tx);
+	clk_disable_unprepare(eqos->clk_rx);
+	clk_disable_unprepare(eqos->clk_slave);
+	clk_disable_unprepare(eqos->clk_master);
+
+	return 0;
+}
+
 struct dwc_eth_dwmac_data {
 	void *(*probe)(struct platform_device *pdev,
 		       struct plat_stmmacenet_data *data,
@@ -160,6 +406,11 @@ static const struct dwc_eth_dwmac_data dwc_qos_data = {
 	.remove = dwc_qos_remove,
 };
 
+static const struct dwc_eth_dwmac_data tegra_eqos_data = {
+	.probe = tegra_eqos_probe,
+	.remove = tegra_eqos_remove,
+};
+
 static int dwc_eth_dwmac_probe(struct platform_device *pdev)
 {
 	const struct dwc_eth_dwmac_data *data;
@@ -245,6 +496,7 @@ static int dwc_eth_dwmac_remove(struct platform_device *pdev)
 
 static const struct of_device_id dwc_eth_dwmac_match[] = {
 	{ .compatible = "snps,dwc-qos-ethernet-4.10", .data = &dwc_qos_data },
+	{ .compatible = "nvidia,tegra186-eqos", .data = &tegra_eqos_data},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, dwc_eth_dwmac_match);
-- 
2.11.1

Re: [PATCH 0/7] net: stmmac: Fixes and Tegra186 support

[David Miller]

The net-next tree is closed, therefore it is not appropriate to submit
feature patches or cleanups at this time.

Please wait for the merge window to be finished and the net-next tree
to open back up before resubmitting this patch series.

Thanks.

Re: [PATCH 0/7] net: stmmac: Fixes and Tegra186 support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 447 bytes --]

On Thu, Feb 23, 2017 at 12:57:05PM -0500, David Miller wrote:
> 
> The net-next tree is closed, therefore it is not appropriate to submit
> feature patches or cleanups at this time.
> 
> Please wait for the merge window to be finished and the net-next tree
> to open back up before resubmitting this patch series.

Okay, I'll resend this after the merge window. In the meantime, surely
it's okay for others to review patches?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/7] net: stmmac: Balance PTP reference clock enable/disable

[Mikko Perttunen]

On 23.02.2017 19:24, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> clk_prepare_enable() and clk_disable_unprepare() for this clock aren't
> properly balanced, which can trigger a WARN_ON() in the common clock
> framework.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 4 ++++
>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 -
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 3cbe09682afe..6b7a5ce19589 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1711,6 +1711,10 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
>  	stmmac_mmc_setup(priv);
>
>  	if (init_ptp) {
> +		ret = clk_prepare_enable(priv->plat->clk_ptp_ref);
> +		if (ret < 0)
> +			netdev_warn(priv->dev, "failed to enable PTP reference clock: %d\n", ret);
> +

Should we return an error code if the clock enable fails?

>  		ret = stmmac_init_ptp(priv);
>  		if (ret == -EOPNOTSUPP)
>  			netdev_warn(priv->dev, "PTP not supported by HW\n");
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 5b18355c0d2b..d285d6cfbd0d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -365,7 +365,6 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
>  		plat->clk_ptp_ref = NULL;
>  		dev_warn(&pdev->dev, "PTP uses main clock\n");
>  	} else {
> -		clk_prepare_enable(plat->clk_ptp_ref);
>  		plat->clk_ptp_rate = clk_get_rate(plat->clk_ptp_ref);
>  		dev_dbg(&pdev->dev, "PTP rate %d\n", plat->clk_ptp_rate);
>  	}
>

It seems like there will still be a refcount mismatch for the clock if 
any of the request_irqs that are after stmmac_hw_setup in stmmac_open fail.

Cheers,
Mikko.

Re: [PATCH 3/7] net: stmmac: Check for DMA mapping errors

[Mikko Perttunen]

On 23.02.2017 19:24, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> When DMA mapping an SKB fragment, the mapping must be checked for
> errors, otherwise the DMA debug code will complain upon unmap.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 6b7a5ce19589..d7387919bdb6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2072,6 +2072,8 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
>  		des = skb_frag_dma_map(priv->device, frag, 0,
>  				       skb_frag_size(frag),
>  				       DMA_TO_DEVICE);
> +		if (dma_mapping_error(priv->device, des))
> +			goto dma_map_err;

If this map fails, we should also unmap the previously mapped fragments 
and the separate mapping made using dma_map_single.

>
>  		stmmac_tso_allocator(priv, des, skb_frag_size(frag),
>  				     (i == nfrags - 1));
>

Re: [PATCH 4/7] net: stmmac: Parse FIFO sizes from feature registers

[Mikko Perttunen]

On 23.02.2017 19:24, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> New version of this core encode the FIFO sizes in one of the feature
> registers. Use these sizes as default, but still allow device tree to
> override them for backwards compatibility.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/common.h      | 3 +++
>  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c  | 2 ++
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
>  3 files changed, 8 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index 144fe84e8a53..6ac653845d82 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -324,6 +324,9 @@ struct dma_features {
>  	unsigned int number_tx_queues;
>  	/* Alternate (enhanced) DESC mode */
>  	unsigned int enh_desc;
> +	/* TX and RX FIFO sizes */
> +	unsigned int tx_fifo_size;
> +	unsigned int rx_fifo_size;
>  };
>
>  /* GMAC TX FIFO is 8K, Rx FIFO is 16K */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> index 377d1b44d4f2..8d249f3b34c8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> @@ -296,6 +296,8 @@ static void dwmac4_get_hw_feature(void __iomem *ioaddr,
>  	hw_cap = readl(ioaddr + GMAC_HW_FEATURE1);
>  	dma_cap->av = (hw_cap & GMAC_HW_FEAT_AVSEL) >> 20;
>  	dma_cap->tsoen = (hw_cap & GMAC_HW_TSOEN) >> 18;
> +	dma_cap->tx_fifo_size = 128 << ((hw_cap >> 6) & 0x1f);
> +	dma_cap->rx_fifo_size = 128 << ((hw_cap >> 0) & 0x1f);
>  	/* MAC HW feature2 */
>  	hw_cap = readl(ioaddr + GMAC_HW_FEATURE2);
>  	/* TX and RX number of channels */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index d7387919bdb6..291e34f0ca94 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1281,6 +1281,9 @@ static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
>  {
>  	int rxfifosz = priv->plat->rx_fifo_size;
>
> +	if (rxfifosz == 0)
> +		rxfifosz = priv->dma_cap.rx_fifo_size;
> +
>  	if (priv->plat->force_thresh_dma_mode)
>  		priv->hw->dma->dma_mode(priv->ioaddr, tc, tc, rxfifosz);
>  	else if (priv->plat->force_sf_dma_mode || priv->plat->tx_coe) {
>

Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>

Re: [PATCH 5/7] net: stmmac: Program RX queue size and flow control

[Mikko Perttunen]

On 23.02.2017 19:24, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Program the receive queue size based on the RX FIFO size and enable
> hardware flow control for large FIFOs.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac4.h     | 12 +++++++
>  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 43 ++++++++++++++++++++++--
>  2 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> index db45134fddf0..9acc1f1252b3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> @@ -180,6 +180,7 @@ enum power_event {
>  #define MTL_OP_MODE_TSF			BIT(1)
>
>  #define MTL_OP_MODE_TQS_MASK		GENMASK(24, 16)
> +#define MTL_OP_MODE_TQS_SHIFT		16
>
>  #define MTL_OP_MODE_TTC_MASK		0x70
>  #define MTL_OP_MODE_TTC_SHIFT		4
> @@ -193,6 +194,17 @@ enum power_event {
>  #define MTL_OP_MODE_TTC_384		(6 << MTL_OP_MODE_TTC_SHIFT)
>  #define MTL_OP_MODE_TTC_512		(7 << MTL_OP_MODE_TTC_SHIFT)
>
> +#define MTL_OP_MODE_RQS_MASK		GENMASK(29, 20)
> +#define MTL_OP_MODE_RQS_SHIFT		20
> +
> +#define MTL_OP_MODE_RFD_MASK		GENMASK(19, 14)
> +#define MTL_OP_MODE_RFD_SHIFT		14
> +
> +#define MTL_OP_MODE_RFA_MASK		GENMASK(13, 8)
> +#define MTL_OP_MODE_RFA_SHIFT		8
> +
> +#define MTL_OP_MODE_EHFC		BIT(7)
> +
>  #define MTL_OP_MODE_RTC_MASK		0x18
>  #define MTL_OP_MODE_RTC_SHIFT		3
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> index 8d249f3b34c8..03d230201960 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> @@ -185,8 +185,9 @@ static void dwmac4_rx_watchdog(void __iomem *ioaddr, u32 riwt)
>  }
>
>  static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
> -				    int rxmode, u32 channel)
> +				    int rxmode, u32 channel, int rxfifosz)
>  {
> +	unsigned int rqs = rxfifosz / 256 - 1;
>  	u32 mtl_tx_op, mtl_rx_op, mtl_rx_int;
>
>  	/* Following code only done for channel 0, other channels not yet
> @@ -252,6 +253,44 @@ static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
>  			mtl_rx_op |= MTL_OP_MODE_RTC_128;
>  	}
>
> +	mtl_rx_op &= ~MTL_OP_MODE_RQS_MASK;
> +	mtl_rx_op |= rqs << MTL_OP_MODE_RQS_SHIFT;
> +
> +	/* enable flow control only if each channel gets 4 KiB or more FIFO */
> +	if (rxfifosz >= 4096) {
> +		unsigned int rfd, rfa;
> +
> +		mtl_rx_op |= MTL_OP_MODE_EHFC;
> +
> +		switch (rxfifosz) {
> +		case 4096:
> +			rfd = 0x03;
> +			rfa = 0x01;
> +			break;
> +
> +		case 8192:
> +			rfd = 0x06;
> +			rfa = 0x0a;
> +			break;
> +
> +		case 16384:
> +			rfd = 0x06;
> +			rfa = 0x12;
> +			break;
> +
> +		default:
> +			rfd = 0x06;
> +			rfa = 0x1e;
> +			break;
> +		}

Are these values correct? In the 4096 case, rfd > rfa, in all other 
cases the other way around. In any case it would be useful to have a 
comment clarifying the thresholds in bytes.

> +
> +		mtl_rx_op &= ~MTL_OP_MODE_RFD_MASK;
> +		mtl_rx_op |= rfd << MTL_OP_MODE_RFD_SHIFT;
> +
> +		mtl_rx_op &= ~MTL_OP_MODE_RFA_MASK;
> +		mtl_rx_op |= rfa << MTL_OP_MODE_RFA_SHIFT;
> +	}
> +
>  	writel(mtl_rx_op, ioaddr + MTL_CHAN_RX_OP_MODE(channel));
>
>  	/* Enable MTL RX overflow */
> @@ -264,7 +303,7 @@ static void dwmac4_dma_operation_mode(void __iomem *ioaddr, int txmode,
>  				      int rxmode, int rxfifosz)
>  {
>  	/* Only Channel 0 is actually configured and used */
> -	dwmac4_dma_chan_op_mode(ioaddr, txmode, rxmode, 0);
> +	dwmac4_dma_chan_op_mode(ioaddr, txmode, rxmode, 0, rxfifosz);
>  }
>
>  static void dwmac4_get_hw_feature(void __iomem *ioaddr,
>

Re: [PATCH 6/7] net: stmmac: dwc-qos: Split out ->probe() and ->remove()

[Mikko Perttunen]

On 23.02.2017 19:24, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Split out the binding specific parts of ->probe() and ->remove() to
> enable the driver to support variants of the binding. This is useful in
> order to keep backwards-compatibility while making it easy for a sub-
> driver to deal only with the updated bindings rather than having to add
> compatibility quirks all over the place.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  .../ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c    | 114 ++++++++++++++++-----
>  1 file changed, 88 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> index 1a3fa3d9f855..5071d3c15adc 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> @@ -18,6 +18,7 @@
>  #include <linux/io.h>
>  #include <linux/ioport.h>
>  #include <linux/module.h>
> +#include <linux/of_device.h>
>  #include <linux/of_net.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/platform_device.h>
> @@ -106,13 +107,70 @@ static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
>  	return 0;
>  }
>
> +static void *dwc_qos_probe(struct platform_device *pdev,
> +			   struct plat_stmmacenet_data *plat_dat,
> +			   struct stmmac_resources *stmmac_res)
> +{
> +	int err;
> +
> +	plat_dat->stmmac_clk = devm_clk_get(&pdev->dev, "apb_pclk");
> +	if (IS_ERR(plat_dat->stmmac_clk)) {
> +		dev_err(&pdev->dev, "apb_pclk clock not found.\n");
> +		return ERR_CAST(plat_dat->stmmac_clk);
> +	}
> +
> +	clk_prepare_enable(plat_dat->stmmac_clk);
> +
> +	plat_dat->pclk = devm_clk_get(&pdev->dev, "phy_ref_clk");
> +	if (IS_ERR(plat_dat->pclk)) {
> +		dev_err(&pdev->dev, "phy_ref_clk clock not found.\n");
> +		err = PTR_ERR(plat_dat->pclk);
> +		goto disable;
> +	}
> +
> +	clk_prepare_enable(plat_dat->pclk);
> +
> +	return NULL;
> +
> +disable:
> +	clk_disable_unprepare(plat_dat->stmmac_clk);
> +	return ERR_PTR(err);
> +}
> +
> +static int dwc_qos_remove(struct platform_device *pdev)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +
> +	clk_disable_unprepare(priv->plat->pclk);
> +	clk_disable_unprepare(priv->plat->stmmac_clk);
> +
> +	return 0;
> +}
> +
> +struct dwc_eth_dwmac_data {
> +	void *(*probe)(struct platform_device *pdev,
> +		       struct plat_stmmacenet_data *data,
> +		       struct stmmac_resources *res);
> +	int (*remove)(struct platform_device *pdev);
> +};
> +
> +static const struct dwc_eth_dwmac_data dwc_qos_data = {
> +	.probe = dwc_qos_probe,
> +	.remove = dwc_qos_remove,
> +};
> +
>  static int dwc_eth_dwmac_probe(struct platform_device *pdev)
>  {
> +	const struct dwc_eth_dwmac_data *data;
>  	struct plat_stmmacenet_data *plat_dat;
>  	struct stmmac_resources stmmac_res;
>  	struct resource *res;
> +	void *priv;
>  	int ret;
>
> +	data = of_device_get_match_data(&pdev->dev);
> +
>  	memset(&stmmac_res, 0, sizeof(struct stmmac_resources));
>
>  	/**
> @@ -138,39 +196,26 @@ static int dwc_eth_dwmac_probe(struct platform_device *pdev)
>  	if (IS_ERR(plat_dat))
>  		return PTR_ERR(plat_dat);
>
> -	plat_dat->stmmac_clk = devm_clk_get(&pdev->dev, "apb_pclk");
> -	if (IS_ERR(plat_dat->stmmac_clk)) {
> -		dev_err(&pdev->dev, "apb_pclk clock not found.\n");
> -		ret = PTR_ERR(plat_dat->stmmac_clk);
> -		plat_dat->stmmac_clk = NULL;
> -		goto err_remove_config_dt;
> -	}
> -	clk_prepare_enable(plat_dat->stmmac_clk);
> -
> -	plat_dat->pclk = devm_clk_get(&pdev->dev, "phy_ref_clk");
> -	if (IS_ERR(plat_dat->pclk)) {
> -		dev_err(&pdev->dev, "phy_ref_clk clock not found.\n");
> -		ret = PTR_ERR(plat_dat->pclk);
> -		plat_dat->pclk = NULL;
> -		goto err_out_clk_dis_phy;
> +	priv = data->probe(pdev, plat_dat, &stmmac_res);
> +	if (IS_ERR(priv)) {
> +		ret = PTR_ERR(priv);
> +		dev_err(&pdev->dev, "failed to probe subdriver: %d\n", ret);
> +		goto remove_config;
>  	}
> -	clk_prepare_enable(plat_dat->pclk);
>
>  	ret = dwc_eth_dwmac_config_dt(pdev, plat_dat);
>  	if (ret)
> -		goto err_out_clk_dis_aper;
> +		goto remove;
>
>  	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>  	if (ret)
> -		goto err_out_clk_dis_aper;
> +		goto remove;
>
> -	return 0;
> +	return ret;
>
> -err_out_clk_dis_aper:
> -	clk_disable_unprepare(plat_dat->pclk);
> -err_out_clk_dis_phy:
> -	clk_disable_unprepare(plat_dat->stmmac_clk);
> -err_remove_config_dt:
> +remove:
> +	data->remove(pdev);
> +remove_config:
>  	stmmac_remove_config_dt(pdev, plat_dat);
>
>  	return ret;
> @@ -178,11 +223,28 @@ static int dwc_eth_dwmac_probe(struct platform_device *pdev)
>
>  static int dwc_eth_dwmac_remove(struct platform_device *pdev)
>  {
> -	return stmmac_pltfr_remove(pdev);
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	const struct dwc_eth_dwmac_data *data;
> +	int err;
> +
> +	data = of_device_get_match_data(&pdev->dev);
> +
> +	err = stmmac_dvr_remove(&pdev->dev);
> +	if (err < 0)
> +		dev_err(&pdev->dev, "failed to remove platform: %d\n", err);
> +
> +	err = data->remove(pdev);
> +	if (err < 0)
> +		dev_err(&pdev->dev, "failed to remove subdriver: %d\n", err);
> +
> +	stmmac_remove_config_dt(pdev, priv->plat);
> +
> +	return err;
>  }
>
>  static const struct of_device_id dwc_eth_dwmac_match[] = {
> -	{ .compatible = "snps,dwc-qos-ethernet-4.10", },
> +	{ .compatible = "snps,dwc-qos-ethernet-4.10", .data = &dwc_qos_data },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, dwc_eth_dwmac_match);
>

Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>

Re: [PATCH 7/7] net: stmmac: dwc-qos: Add Tegra186 support

[Mikko Perttunen]

On 23.02.2017 19:24, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> The NVIDIA Tegra186 SoC contains an instance of the Synopsys DWC
> ethernet QOS IP core. The binding that it uses is slightly different
> from existing ones because of the integration (clocks, resets, ...).
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  .../ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c    | 252 +++++++++++++++++++++
>  1 file changed, 252 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> index 5071d3c15adc..54dfbdc48f6d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> @@ -14,6 +14,7 @@
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/device.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/ethtool.h>
>  #include <linux/io.h>
>  #include <linux/ioport.h>
> @@ -22,10 +23,24 @@
>  #include <linux/of_net.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/platform_device.h>
> +#include <linux/reset.h>
>  #include <linux/stmmac.h>
>
>  #include "stmmac_platform.h"
>
> +struct tegra_eqos {
> +	struct device *dev;
> +	void __iomem *regs;
> +
> +	struct reset_control *rst;
> +	struct clk *clk_master;
> +	struct clk *clk_slave;
> +	struct clk *clk_tx;
> +	struct clk *clk_rx;
> +
> +	struct gpio_desc *reset;
> +};
> +
>  static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
>  				   struct plat_stmmacenet_data *plat_dat)
>  {
> @@ -148,6 +163,237 @@ static int dwc_qos_remove(struct platform_device *pdev)
>  	return 0;
>  }
>
> +#define SDMEMCOMPPADCTRL 0x8800
> +#define  SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD BIT(31)
> +
> +#define AUTO_CAL_CONFIG 0x8804
> +#define  AUTO_CAL_CONFIG_START BIT(31)
> +#define  AUTO_CAL_CONFIG_ENABLE BIT(29)
> +
> +#define AUTO_CAL_STATUS 0x880c
> +#define  AUTO_CAL_STATUS_ACTIVE BIT(31)
> +
> +static void tegra_eqos_fix_speed(void *priv, unsigned int speed)
> +{
> +	struct tegra_eqos *eqos = priv;
> +	unsigned long rate = 125000000;
> +	bool needs_calibration = false;
> +	unsigned int i;
> +	u32 value;
> +
> +	switch (speed) {
> +	case SPEED_1000:
> +		needs_calibration = true;
> +		rate = 125000000;
> +		break;
> +
> +	case SPEED_100:
> +		needs_calibration = true;
> +		rate = 25000000;
> +		break;
> +
> +	case SPEED_10:
> +		rate = 2500000;
> +		break;
> +
> +	default:
> +		dev_err(eqos->dev, "invalid speed %u\n", speed);
> +		break;
> +	}
> +
> +	if (needs_calibration) {
> +		/* calibrate */
> +		value = readl(eqos->regs + SDMEMCOMPPADCTRL);
> +		value |= SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD;
> +		writel(value, eqos->regs + SDMEMCOMPPADCTRL);
> +
> +		udelay(1);
> +
> +		value = readl(eqos->regs + AUTO_CAL_CONFIG);
> +		value |= AUTO_CAL_CONFIG_START | AUTO_CAL_CONFIG_ENABLE;
> +		writel(value, eqos->regs + AUTO_CAL_CONFIG);
> +
> +		for (i = 0; i <= 10; i++) {
> +			value = readl(eqos->regs + AUTO_CAL_STATUS);
> +			if (value & AUTO_CAL_STATUS_ACTIVE)
> +				break;
> +
> +			udelay(1);
> +		}
> +
> +		if ((value & AUTO_CAL_STATUS_ACTIVE) == 0) {
> +			dev_err(eqos->dev, "calibration did not start\n");
> +			goto failed;
> +		}
> +
> +		for (i = 0; i <= 10; i++) {
> +			value = readl(eqos->regs + AUTO_CAL_STATUS);
> +			if ((value & AUTO_CAL_STATUS_ACTIVE) == 0)
> +				break;
> +
> +			udelay(20);
> +		}
> +
> +		if (value & AUTO_CAL_STATUS_ACTIVE) {
> +			dev_err(eqos->dev, "calibration didn't finish\n");
> +			goto failed;
> +		}

Could use readl_poll_timeout/readl_poll_timeout_atomic for these loops 
instead.

> +
> +	failed:
> +		value = readl(eqos->regs + SDMEMCOMPPADCTRL);
> +		value &= ~SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD;
> +		writel(value, eqos->regs + SDMEMCOMPPADCTRL);
> +	} else {
> +		value = readl(eqos->regs + AUTO_CAL_CONFIG);
> +		value &= ~AUTO_CAL_CONFIG_ENABLE;
> +		writel(value, eqos->regs + AUTO_CAL_CONFIG);
> +	}
> +
> +	clk_set_rate(eqos->clk_tx, rate);

Could check error code here, and for other clock ops too.

> +}
> +
> +static int tegra_eqos_init(struct platform_device *pdev, void *priv)
> +{
> +	struct tegra_eqos *eqos = priv;
> +	unsigned long rate;
> +	u32 value;
> +
> +	rate = clk_get_rate(eqos->clk_slave);
> +
> +	value = readl(eqos->regs + 0xdc);

No point in reading the value when it is fully overwritten.

> +	value = (rate / 1000000) - 1;
> +	writel(value, eqos->regs + 0xdc);

Please add a define for 0xdc.

> +
> +	return 0;
> +}
> +
> +static void *tegra_eqos_probe(struct platform_device *pdev,
> +			      struct plat_stmmacenet_data *data,
> +			      struct stmmac_resources *res)
> +{
> +	struct tegra_eqos *eqos;
> +	int err;
> +
> +	eqos = devm_kzalloc(&pdev->dev, sizeof(*eqos), GFP_KERNEL);
> +	if (!eqos) {
> +		err = -ENOMEM;
> +		goto error;
> +	}
> +
> +	eqos->dev = &pdev->dev;
> +	eqos->regs = res->addr;
> +
> +	eqos->clk_master = devm_clk_get(&pdev->dev, "master_bus");
> +	if (IS_ERR(eqos->clk_master)) {
> +		err = PTR_ERR(eqos->clk_master);
> +		goto error;
> +	}
> +
> +	err = clk_prepare_enable(eqos->clk_master);
> +	if (err < 0)
> +		goto error;
> +
> +	eqos->clk_slave = devm_clk_get(&pdev->dev, "slave_bus");
> +	if (IS_ERR(eqos->clk_slave)) {
> +		err = PTR_ERR(eqos->clk_slave);
> +		goto disable_master;
> +	}
> +
> +	data->stmmac_clk = eqos->clk_slave;
> +
> +	err = clk_prepare_enable(eqos->clk_slave);
> +	if (err < 0)
> +		goto disable_master;
> +
> +	eqos->clk_rx = devm_clk_get(&pdev->dev, "rx");
> +	if (IS_ERR(eqos->clk_rx)) {
> +		err = PTR_ERR(eqos->clk_rx);
> +		goto disable_slave;
> +	}
> +
> +	err = clk_prepare_enable(eqos->clk_rx);
> +	if (err < 0)
> +		goto disable_slave;
> +
> +	eqos->clk_tx = devm_clk_get(&pdev->dev, "tx");
> +	if (IS_ERR(eqos->clk_tx)) {
> +		err = PTR_ERR(eqos->clk_tx);
> +		goto disable_rx;
> +	}
> +
> +	err = clk_prepare_enable(eqos->clk_tx);
> +	if (err < 0)
> +		goto disable_rx;
> +
> +	eqos->reset = devm_gpiod_get(&pdev->dev, "phy-reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(eqos->reset)) {
> +		err = PTR_ERR(eqos->reset);
> +		goto disable_tx;
> +	}
> +
> +	usleep_range(2000, 4000);
> +	gpiod_set_value(eqos->reset, 0);
> +
> +	eqos->rst = devm_reset_control_get(&pdev->dev, "eqos");
> +	if (IS_ERR(eqos->rst)) {
> +		err = PTR_ERR(eqos->rst);
> +		goto reset_phy;
> +	}
> +
> +	err = reset_control_assert(eqos->rst);
> +	if (err < 0)
> +		goto reset_phy;
> +
> +	usleep_range(2000, 4000);
> +
> +	err = reset_control_deassert(eqos->rst);
> +	if (err < 0)
> +		goto reset_phy;
> +
> +	usleep_range(2000, 4000);
> +
> +	data->fix_mac_speed = tegra_eqos_fix_speed;
> +	data->init = tegra_eqos_init;
> +	data->bsp_priv = eqos;
> +
> +	err = tegra_eqos_init(pdev, eqos);
> +	if (err < 0)
> +		goto reset;
> +
> +out:
> +	return eqos;
> +
> +reset:
> +	reset_control_assert(eqos->rst);
> +reset_phy:
> +	gpiod_set_value(eqos->reset, 1);
> +disable_tx:
> +	clk_disable_unprepare(eqos->clk_tx);
> +disable_rx:
> +	clk_disable_unprepare(eqos->clk_rx);
> +disable_slave:
> +	clk_disable_unprepare(eqos->clk_slave);
> +disable_master:
> +	clk_disable_unprepare(eqos->clk_master);
> +error:
> +	eqos = ERR_PTR(err);
> +	goto out;
> +}
> +
> +static int tegra_eqos_remove(struct platform_device *pdev)
> +{
> +	struct tegra_eqos *eqos = get_stmmac_bsp_priv(&pdev->dev);
> +
> +	reset_control_assert(eqos->rst);
> +	gpiod_set_value(eqos->reset, 1);
> +	clk_disable_unprepare(eqos->clk_tx);
> +	clk_disable_unprepare(eqos->clk_rx);
> +	clk_disable_unprepare(eqos->clk_slave);
> +	clk_disable_unprepare(eqos->clk_master);
> +
> +	return 0;
> +}
> +
>  struct dwc_eth_dwmac_data {
>  	void *(*probe)(struct platform_device *pdev,
>  		       struct plat_stmmacenet_data *data,
> @@ -160,6 +406,11 @@ static const struct dwc_eth_dwmac_data dwc_qos_data = {
>  	.remove = dwc_qos_remove,
>  };
>
> +static const struct dwc_eth_dwmac_data tegra_eqos_data = {
> +	.probe = tegra_eqos_probe,
> +	.remove = tegra_eqos_remove,
> +};
> +
>  static int dwc_eth_dwmac_probe(struct platform_device *pdev)
>  {
>  	const struct dwc_eth_dwmac_data *data;
> @@ -245,6 +496,7 @@ static int dwc_eth_dwmac_remove(struct platform_device *pdev)
>
>  static const struct of_device_id dwc_eth_dwmac_match[] = {
>  	{ .compatible = "snps,dwc-qos-ethernet-4.10", .data = &dwc_qos_data },
> +	{ .compatible = "nvidia,tegra186-eqos", .data = &tegra_eqos_data},

Missing space before '}'.

>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, dwc_eth_dwmac_match);
>

Re: [PATCH 2/7] net: stmmac: Balance PTP reference clock enable/disable

[Joao Pinto]

Às 5:24 PM de 2/23/2017, Thierry Reding escreveu:
> From: Thierry Reding <treding@nvidia.com>
> 
> clk_prepare_enable() and clk_disable_unprepare() for this clock aren't
> properly balanced, which can trigger a WARN_ON() in the common clock
> framework.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 4 ++++
>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 -
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 3cbe09682afe..6b7a5ce19589 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1711,6 +1711,10 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
>  	stmmac_mmc_setup(priv);
>  
>  	if (init_ptp) {
> +		ret = clk_prepare_enable(priv->plat->clk_ptp_ref);
> +		if (ret < 0)
> +			netdev_warn(priv->dev, "failed to enable PTP reference clock: %d\n", ret);
> +
>  		ret = stmmac_init_ptp(priv);
>  		if (ret == -EOPNOTSUPP)
>  			netdev_warn(priv->dev, "PTP not supported by HW\n");
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 5b18355c0d2b..d285d6cfbd0d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -365,7 +365,6 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
>  		plat->clk_ptp_ref = NULL;
>  		dev_warn(&pdev->dev, "PTP uses main clock\n");
>  	} else {
> -		clk_prepare_enable(plat->clk_ptp_ref);
>  		plat->clk_ptp_rate = clk_get_rate(plat->clk_ptp_ref);
>  		dev_dbg(&pdev->dev, "PTP rate %d\n", plat->clk_ptp_rate);
>  	}
> 

Reviewed-By: Joao Pinto <jpinto@synopsys.com>

Re: [PATCH 4/7] net: stmmac: Parse FIFO sizes from feature registers

[Joao Pinto]

Hi Thierry,

Às 5:24 PM de 2/23/2017, Thierry Reding escreveu:
> From: Thierry Reding <treding@nvidia.com>
> 
> New version of this core encode the FIFO sizes in one of the feature
> registers. Use these sizes as default, but still allow device tree to
> override them for backwards compatibility.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/common.h      | 3 +++
>  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c  | 2 ++
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
>  3 files changed, 8 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index 144fe84e8a53..6ac653845d82 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -324,6 +324,9 @@ struct dma_features {
>  	unsigned int number_tx_queues;
>  	/* Alternate (enhanced) DESC mode */
>  	unsigned int enh_desc;
> +	/* TX and RX FIFO sizes */
> +	unsigned int tx_fifo_size;
> +	unsigned int rx_fifo_size;
>  };
>  
>  /* GMAC TX FIFO is 8K, Rx FIFO is 16K */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> index 377d1b44d4f2..8d249f3b34c8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> @@ -296,6 +296,8 @@ static void dwmac4_get_hw_feature(void __iomem *ioaddr,
>  	hw_cap = readl(ioaddr + GMAC_HW_FEATURE1);
>  	dma_cap->av = (hw_cap & GMAC_HW_FEAT_AVSEL) >> 20;
>  	dma_cap->tsoen = (hw_cap & GMAC_HW_TSOEN) >> 18;
> +	dma_cap->tx_fifo_size = 128 << ((hw_cap >> 6) & 0x1f);
> +	dma_cap->rx_fifo_size = 128 << ((hw_cap >> 0) & 0x1f);

I suggest you follow the way that has been done for other parameters:

dma_cap->rx_fifo_size = (hw_cap & GMAC_HW_RXFIFOSIZE) >> 0; where
GMAC_HW_RXFIFOSIZE is GENMASK(4, 0)
dma_cap->tx_fifo_size = (hw_cap & GMAC_HW_TXFIFOSIZE) >> 6; where
GMAC_HW_TXFIFOSIZE is GENMASK(10, 6)

>  	/* MAC HW feature2 */
>  	hw_cap = readl(ioaddr + GMAC_HW_FEATURE2);
>  	/* TX and RX number of channels */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index d7387919bdb6..291e34f0ca94 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1281,6 +1281,9 @@ static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
>  {
>  	int rxfifosz = priv->plat->rx_fifo_size;
>  
> +	if (rxfifosz == 0)
> +		rxfifosz = priv->dma_cap.rx_fifo_size;
> +
>  	if (priv->plat->force_thresh_dma_mode)
>  		priv->hw->dma->dma_mode(priv->ioaddr, tc, tc, rxfifosz);
>  	else if (priv->plat->force_sf_dma_mode || priv->plat->tx_coe) {
> 

In my current patch for adding support for MTL in stmmac, I also had this
approach (sizes comming from feature register and platform data) so nice to see
it here, because I will be able to reutilize it in my future versions.

There is only a slight twist that we have to pay attention to it. The RX and TX
queue size configuration needs to be one the following values:

FIFO_256 = 0x0,
FIFO_512 = 0x1,
FIFO_1k = 0x3,
FIFO_2k = 0x7,
FIFO_4k = 0xf,
FIFO_8k = 0x1f,
FIFO_16k = 0x3f,
FIFO_32k = 0x7f

These are the values you get from the features register, but are these the
values that users will configure in "plat->rx_fifo_size"? From a quick grep you
can see that users use real units:

arch/arm/boot/dts/socfpga.dtsi:			rx-fifo-depth = <4096>;
arch/arm/boot/dts/socfpga.dtsi:			rx-fifo-depth = <4096>;
arch/arm/boot/dts/socfpga_arria10.dtsi:			rx-fifo-depth = <16384>;
arch/arm/boot/dts/socfpga_arria10.dtsi:			rx-fifo-depth = <16384>;
arch/arm/boot/dts/socfpga_arria10.dtsi:			rx-fifo-depth = <16384>;
arch/arm/boot/dts/lpc18xx.dtsi:			rx-fifo-depth = <256>;
arch/nios2/boot/dts/3c120_devboard.dts:				rx-fifo-depth = <8192>;
arch/nios2/boot/dts/10m50_devboard.dts:			rx-fifo-depth = <8192>;

So in order to have it configurable from platform and features register you need
to convert the features values to "real" units and then in the end when you
write to the DMA Op Mode you need to convert it back to the required values.

You can check my patch here that already has this done:
http://patchwork.ozlabs.org/patch/728566/

Thanks.

Re: [PATCH 5/7] net: stmmac: Program RX queue size and flow control

[Joao Pinto]

Às 5:24 PM de 2/23/2017, Thierry Reding escreveu:
> From: Thierry Reding <treding@nvidia.com>
> 
> Program the receive queue size based on the RX FIFO size and enable
> hardware flow control for large FIFOs.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac4.h     | 12 +++++++
>  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 43 ++++++++++++++++++++++--
>  2 files changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> index db45134fddf0..9acc1f1252b3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> @@ -180,6 +180,7 @@ enum power_event {
>  #define MTL_OP_MODE_TSF			BIT(1)
>  
>  #define MTL_OP_MODE_TQS_MASK		GENMASK(24, 16)
> +#define MTL_OP_MODE_TQS_SHIFT		16
>  
>  #define MTL_OP_MODE_TTC_MASK		0x70
>  #define MTL_OP_MODE_TTC_SHIFT		4
> @@ -193,6 +194,17 @@ enum power_event {
>  #define MTL_OP_MODE_TTC_384		(6 << MTL_OP_MODE_TTC_SHIFT)
>  #define MTL_OP_MODE_TTC_512		(7 << MTL_OP_MODE_TTC_SHIFT)
>  
> +#define MTL_OP_MODE_RQS_MASK		GENMASK(29, 20)
> +#define MTL_OP_MODE_RQS_SHIFT		20
> +
> +#define MTL_OP_MODE_RFD_MASK		GENMASK(19, 14)
> +#define MTL_OP_MODE_RFD_SHIFT		14
> +
> +#define MTL_OP_MODE_RFA_MASK		GENMASK(13, 8)
> +#define MTL_OP_MODE_RFA_SHIFT		8
> +
> +#define MTL_OP_MODE_EHFC		BIT(7)
> +
>  #define MTL_OP_MODE_RTC_MASK		0x18
>  #define MTL_OP_MODE_RTC_SHIFT		3
>  
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> index 8d249f3b34c8..03d230201960 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> @@ -185,8 +185,9 @@ static void dwmac4_rx_watchdog(void __iomem *ioaddr, u32 riwt)
>  }
>  
>  static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
> -				    int rxmode, u32 channel)
> +				    int rxmode, u32 channel, int rxfifosz)
>  {
> +	unsigned int rqs = rxfifosz / 256 - 1;
>  	u32 mtl_tx_op, mtl_rx_op, mtl_rx_int;
>  
>  	/* Following code only done for channel 0, other channels not yet
> @@ -252,6 +253,44 @@ static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
>  			mtl_rx_op |= MTL_OP_MODE_RTC_128;
>  	}
>  
> +	mtl_rx_op &= ~MTL_OP_MODE_RQS_MASK;
> +	mtl_rx_op |= rqs << MTL_OP_MODE_RQS_SHIFT;
> +
> +	/* enable flow control only if each channel gets 4 KiB or more FIFO */
> +	if (rxfifosz >= 4096) {
> +		unsigned int rfd, rfa;
> +
> +		mtl_rx_op |= MTL_OP_MODE_EHFC;
> +
> +		switch (rxfifosz) {
> +		case 4096:
> +			rfd = 0x03;
> +			rfa = 0x01;
> +			break;
> +
> +		case 8192:
> +			rfd = 0x06;
> +			rfa = 0x0a;
> +			break;
> +
> +		case 16384:
> +			rfd = 0x06;
> +			rfa = 0x12;
> +			break;
> +
> +		default:
> +			rfd = 0x06;
> +			rfa = 0x1e;
> +			break;
> +		}

Are these RFA and RFD values suitable to any setup using thresholds? Please
justify their values in oerder for other developers to understand them.

> +
> +		mtl_rx_op &= ~MTL_OP_MODE_RFD_MASK;
> +		mtl_rx_op |= rfd << MTL_OP_MODE_RFD_SHIFT;
> +
> +		mtl_rx_op &= ~MTL_OP_MODE_RFA_MASK;
> +		mtl_rx_op |= rfa << MTL_OP_MODE_RFA_SHIFT;
> +	}
> +
>  	writel(mtl_rx_op, ioaddr + MTL_CHAN_RX_OP_MODE(channel));
>  
>  	/* Enable MTL RX overflow */
> @@ -264,7 +303,7 @@ static void dwmac4_dma_operation_mode(void __iomem *ioaddr, int txmode,
>  				      int rxmode, int rxfifosz)
>  {
>  	/* Only Channel 0 is actually configured and used */
> -	dwmac4_dma_chan_op_mode(ioaddr, txmode, rxmode, 0);
> +	dwmac4_dma_chan_op_mode(ioaddr, txmode, rxmode, 0, rxfifosz);

Why not configure tx fifo as well, you already getting it from the features
register.

>  }
>  
>  static void dwmac4_get_hw_feature(void __iomem *ioaddr,
> 

Thanks.

Re: [PATCH 6/7] net: stmmac: dwc-qos: Split out ->probe() and ->remove()

[Joao Pinto]

Às 5:24 PM de 2/23/2017, Thierry Reding escreveu:
> From: Thierry Reding <treding@nvidia.com>
> 
> Split out the binding specific parts of ->probe() and ->remove() to
> enable the driver to support variants of the binding. This is useful in
> order to keep backwards-compatibility while making it easy for a sub-
> driver to deal only with the updated bindings rather than having to add
> compatibility quirks all over the place.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  .../ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c    | 114 ++++++++++++++++-----
>  1 file changed, 88 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> index 1a3fa3d9f855..5071d3c15adc 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> @@ -18,6 +18,7 @@
>  #include <linux/io.h>
>  #include <linux/ioport.h>
>  #include <linux/module.h>
> +#include <linux/of_device.h>
>  #include <linux/of_net.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/platform_device.h>
> @@ -106,13 +107,70 @@ static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
>  	return 0;
>  }
>  
> +static void *dwc_qos_probe(struct platform_device *pdev,
> +			   struct plat_stmmacenet_data *plat_dat,
> +			   struct stmmac_resources *stmmac_res)
> +{
> +	int err;
> +
> +	plat_dat->stmmac_clk = devm_clk_get(&pdev->dev, "apb_pclk");
> +	if (IS_ERR(plat_dat->stmmac_clk)) {
> +		dev_err(&pdev->dev, "apb_pclk clock not found.\n");
> +		return ERR_CAST(plat_dat->stmmac_clk);
> +	}
> +
> +	clk_prepare_enable(plat_dat->stmmac_clk);
> +
> +	plat_dat->pclk = devm_clk_get(&pdev->dev, "phy_ref_clk");
> +	if (IS_ERR(plat_dat->pclk)) {
> +		dev_err(&pdev->dev, "phy_ref_clk clock not found.\n");
> +		err = PTR_ERR(plat_dat->pclk);
> +		goto disable;
> +	}
> +
> +	clk_prepare_enable(plat_dat->pclk);
> +
> +	return NULL;
> +
> +disable:
> +	clk_disable_unprepare(plat_dat->stmmac_clk);
> +	return ERR_PTR(err);
> +}
> +
> +static int dwc_qos_remove(struct platform_device *pdev)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +
> +	clk_disable_unprepare(priv->plat->pclk);
> +	clk_disable_unprepare(priv->plat->stmmac_clk);
> +
> +	return 0;
> +}
> +
> +struct dwc_eth_dwmac_data {
> +	void *(*probe)(struct platform_device *pdev,
> +		       struct plat_stmmacenet_data *data,
> +		       struct stmmac_resources *res);
> +	int (*remove)(struct platform_device *pdev);
> +};
> +
> +static const struct dwc_eth_dwmac_data dwc_qos_data = {
> +	.probe = dwc_qos_probe,
> +	.remove = dwc_qos_remove,
> +};
> +
>  static int dwc_eth_dwmac_probe(struct platform_device *pdev)
>  {
> +	const struct dwc_eth_dwmac_data *data;
>  	struct plat_stmmacenet_data *plat_dat;
>  	struct stmmac_resources stmmac_res;
>  	struct resource *res;
> +	void *priv;
>  	int ret;
>  
> +	data = of_device_get_match_data(&pdev->dev);
> +
>  	memset(&stmmac_res, 0, sizeof(struct stmmac_resources));
>  
>  	/**
> @@ -138,39 +196,26 @@ static int dwc_eth_dwmac_probe(struct platform_device *pdev)
>  	if (IS_ERR(plat_dat))
>  		return PTR_ERR(plat_dat);
>  
> -	plat_dat->stmmac_clk = devm_clk_get(&pdev->dev, "apb_pclk");
> -	if (IS_ERR(plat_dat->stmmac_clk)) {
> -		dev_err(&pdev->dev, "apb_pclk clock not found.\n");
> -		ret = PTR_ERR(plat_dat->stmmac_clk);
> -		plat_dat->stmmac_clk = NULL;
> -		goto err_remove_config_dt;
> -	}
> -	clk_prepare_enable(plat_dat->stmmac_clk);
> -
> -	plat_dat->pclk = devm_clk_get(&pdev->dev, "phy_ref_clk");
> -	if (IS_ERR(plat_dat->pclk)) {
> -		dev_err(&pdev->dev, "phy_ref_clk clock not found.\n");
> -		ret = PTR_ERR(plat_dat->pclk);
> -		plat_dat->pclk = NULL;
> -		goto err_out_clk_dis_phy;
> +	priv = data->probe(pdev, plat_dat, &stmmac_res);
> +	if (IS_ERR(priv)) {
> +		ret = PTR_ERR(priv);
> +		dev_err(&pdev->dev, "failed to probe subdriver: %d\n", ret);
> +		goto remove_config;
>  	}
> -	clk_prepare_enable(plat_dat->pclk);
>  
>  	ret = dwc_eth_dwmac_config_dt(pdev, plat_dat);
>  	if (ret)
> -		goto err_out_clk_dis_aper;
> +		goto remove;
>  
>  	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>  	if (ret)
> -		goto err_out_clk_dis_aper;
> +		goto remove;
>  
> -	return 0;
> +	return ret;
>  
> -err_out_clk_dis_aper:
> -	clk_disable_unprepare(plat_dat->pclk);
> -err_out_clk_dis_phy:
> -	clk_disable_unprepare(plat_dat->stmmac_clk);
> -err_remove_config_dt:
> +remove:
> +	data->remove(pdev);
> +remove_config:
>  	stmmac_remove_config_dt(pdev, plat_dat);
>  
>  	return ret;
> @@ -178,11 +223,28 @@ static int dwc_eth_dwmac_probe(struct platform_device *pdev)
>  
>  static int dwc_eth_dwmac_remove(struct platform_device *pdev)
>  {
> -	return stmmac_pltfr_remove(pdev);
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	const struct dwc_eth_dwmac_data *data;
> +	int err;
> +
> +	data = of_device_get_match_data(&pdev->dev);
> +
> +	err = stmmac_dvr_remove(&pdev->dev);
> +	if (err < 0)
> +		dev_err(&pdev->dev, "failed to remove platform: %d\n", err);
> +
> +	err = data->remove(pdev);
> +	if (err < 0)
> +		dev_err(&pdev->dev, "failed to remove subdriver: %d\n", err);
> +
> +	stmmac_remove_config_dt(pdev, priv->plat);
> +
> +	return err;
>  }
>  
>  static const struct of_device_id dwc_eth_dwmac_match[] = {
> -	{ .compatible = "snps,dwc-qos-ethernet-4.10", },
> +	{ .compatible = "snps,dwc-qos-ethernet-4.10", .data = &dwc_qos_data },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, dwc_eth_dwmac_match);
> 

Reviewed-By: Joao Pinto <jpinto@synopsys.com>

Re: [PATCH 7/7] net: stmmac: dwc-qos: Add Tegra186 support

[Joao Pinto]

Às 5:24 PM de 2/23/2017, Thierry Reding escreveu:
> From: Thierry Reding <treding@nvidia.com>
> 
> The NVIDIA Tegra186 SoC contains an instance of the Synopsys DWC
> ethernet QOS IP core. The binding that it uses is slightly different
> from existing ones because of the integration (clocks, resets, ...).
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  .../ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c    | 252 +++++++++++++++++++++
>  1 file changed, 252 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> index 5071d3c15adc..54dfbdc48f6d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> @@ -14,6 +14,7 @@
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/device.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/ethtool.h>
>  #include <linux/io.h>
>  #include <linux/ioport.h>
> @@ -22,10 +23,24 @@
>  #include <linux/of_net.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/platform_device.h>
> +#include <linux/reset.h>
>  #include <linux/stmmac.h>
>  
>  #include "stmmac_platform.h"
>  
> +struct tegra_eqos {
> +	struct device *dev;
> +	void __iomem *regs;
> +
> +	struct reset_control *rst;
> +	struct clk *clk_master;
> +	struct clk *clk_slave;
> +	struct clk *clk_tx;
> +	struct clk *clk_rx;
> +
> +	struct gpio_desc *reset;
> +};
> +
>  static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
>  				   struct plat_stmmacenet_data *plat_dat)
>  {
> @@ -148,6 +163,237 @@ static int dwc_qos_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#define SDMEMCOMPPADCTRL 0x8800
> +#define  SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD BIT(31)
> +
> +#define AUTO_CAL_CONFIG 0x8804
> +#define  AUTO_CAL_CONFIG_START BIT(31)
> +#define  AUTO_CAL_CONFIG_ENABLE BIT(29)
> +
> +#define AUTO_CAL_STATUS 0x880c
> +#define  AUTO_CAL_STATUS_ACTIVE BIT(31)
> +
> +static void tegra_eqos_fix_speed(void *priv, unsigned int speed)
> +{
> +	struct tegra_eqos *eqos = priv;
> +	unsigned long rate = 125000000;
> +	bool needs_calibration = false;
> +	unsigned int i;
> +	u32 value;
> +
> +	switch (speed) {
> +	case SPEED_1000:
> +		needs_calibration = true;
> +		rate = 125000000;
> +		break;
> +
> +	case SPEED_100:
> +		needs_calibration = true;
> +		rate = 25000000;
> +		break;
> +
> +	case SPEED_10:
> +		rate = 2500000;
> +		break;
> +
> +	default:
> +		dev_err(eqos->dev, "invalid speed %u\n", speed);
> +		break;
> +	}
> +
> +	if (needs_calibration) {
> +		/* calibrate */
> +		value = readl(eqos->regs + SDMEMCOMPPADCTRL);
> +		value |= SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD;
> +		writel(value, eqos->regs + SDMEMCOMPPADCTRL);
> +
> +		udelay(1);
> +
> +		value = readl(eqos->regs + AUTO_CAL_CONFIG);
> +		value |= AUTO_CAL_CONFIG_START | AUTO_CAL_CONFIG_ENABLE;
> +		writel(value, eqos->regs + AUTO_CAL_CONFIG);
> +
> +		for (i = 0; i <= 10; i++) {
> +			value = readl(eqos->regs + AUTO_CAL_STATUS);
> +			if (value & AUTO_CAL_STATUS_ACTIVE)
> +				break;
> +
> +			udelay(1);
> +		}
> +
> +		if ((value & AUTO_CAL_STATUS_ACTIVE) == 0) {
> +			dev_err(eqos->dev, "calibration did not start\n");
> +			goto failed;
> +		}
> +
> +		for (i = 0; i <= 10; i++) {
> +			value = readl(eqos->regs + AUTO_CAL_STATUS);
> +			if ((value & AUTO_CAL_STATUS_ACTIVE) == 0)
> +				break;
> +
> +			udelay(20);
> +		}
> +
> +		if (value & AUTO_CAL_STATUS_ACTIVE) {
> +			dev_err(eqos->dev, "calibration didn't finish\n");
> +			goto failed;
> +		}
> +
> +	failed:
> +		value = readl(eqos->regs + SDMEMCOMPPADCTRL);
> +		value &= ~SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD;
> +		writel(value, eqos->regs + SDMEMCOMPPADCTRL);
> +	} else {
> +		value = readl(eqos->regs + AUTO_CAL_CONFIG);
> +		value &= ~AUTO_CAL_CONFIG_ENABLE;
> +		writel(value, eqos->regs + AUTO_CAL_CONFIG);
> +	}
> +
> +	clk_set_rate(eqos->clk_tx, rate);
> +}
> +
> +static int tegra_eqos_init(struct platform_device *pdev, void *priv)
> +{
> +	struct tegra_eqos *eqos = priv;
> +	unsigned long rate;
> +	u32 value;
> +
> +	rate = clk_get_rate(eqos->clk_slave);
> +
> +	value = readl(eqos->regs + 0xdc);
> +	value = (rate / 1000000) - 1;
> +	writel(value, eqos->regs + 0xdc);
> +
> +	return 0;
> +}
> +
> +static void *tegra_eqos_probe(struct platform_device *pdev,
> +			      struct plat_stmmacenet_data *data,
> +			      struct stmmac_resources *res)
> +{
> +	struct tegra_eqos *eqos;
> +	int err;
> +
> +	eqos = devm_kzalloc(&pdev->dev, sizeof(*eqos), GFP_KERNEL);
> +	if (!eqos) {
> +		err = -ENOMEM;
> +		goto error;
> +	}
> +
> +	eqos->dev = &pdev->dev;
> +	eqos->regs = res->addr;
> +
> +	eqos->clk_master = devm_clk_get(&pdev->dev, "master_bus");
> +	if (IS_ERR(eqos->clk_master)) {
> +		err = PTR_ERR(eqos->clk_master);
> +		goto error;
> +	}
> +
> +	err = clk_prepare_enable(eqos->clk_master);
> +	if (err < 0)
> +		goto error;
> +
> +	eqos->clk_slave = devm_clk_get(&pdev->dev, "slave_bus");
> +	if (IS_ERR(eqos->clk_slave)) {
> +		err = PTR_ERR(eqos->clk_slave);
> +		goto disable_master;
> +	}
> +
> +	data->stmmac_clk = eqos->clk_slave;
> +
> +	err = clk_prepare_enable(eqos->clk_slave);
> +	if (err < 0)
> +		goto disable_master;
> +
> +	eqos->clk_rx = devm_clk_get(&pdev->dev, "rx");
> +	if (IS_ERR(eqos->clk_rx)) {
> +		err = PTR_ERR(eqos->clk_rx);
> +		goto disable_slave;
> +	}
> +
> +	err = clk_prepare_enable(eqos->clk_rx);
> +	if (err < 0)
> +		goto disable_slave;
> +
> +	eqos->clk_tx = devm_clk_get(&pdev->dev, "tx");
> +	if (IS_ERR(eqos->clk_tx)) {
> +		err = PTR_ERR(eqos->clk_tx);
> +		goto disable_rx;
> +	}
> +
> +	err = clk_prepare_enable(eqos->clk_tx);
> +	if (err < 0)
> +		goto disable_rx;
> +
> +	eqos->reset = devm_gpiod_get(&pdev->dev, "phy-reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(eqos->reset)) {
> +		err = PTR_ERR(eqos->reset);
> +		goto disable_tx;
> +	}
> +
> +	usleep_range(2000, 4000);
> +	gpiod_set_value(eqos->reset, 0);
> +
> +	eqos->rst = devm_reset_control_get(&pdev->dev, "eqos");
> +	if (IS_ERR(eqos->rst)) {
> +		err = PTR_ERR(eqos->rst);
> +		goto reset_phy;
> +	}
> +
> +	err = reset_control_assert(eqos->rst);
> +	if (err < 0)
> +		goto reset_phy;
> +
> +	usleep_range(2000, 4000);
> +
> +	err = reset_control_deassert(eqos->rst);
> +	if (err < 0)
> +		goto reset_phy;
> +
> +	usleep_range(2000, 4000);
> +
> +	data->fix_mac_speed = tegra_eqos_fix_speed;
> +	data->init = tegra_eqos_init;
> +	data->bsp_priv = eqos;
> +
> +	err = tegra_eqos_init(pdev, eqos);
> +	if (err < 0)
> +		goto reset;
> +
> +out:
> +	return eqos;
> +
> +reset:
> +	reset_control_assert(eqos->rst);
> +reset_phy:
> +	gpiod_set_value(eqos->reset, 1);
> +disable_tx:
> +	clk_disable_unprepare(eqos->clk_tx);
> +disable_rx:
> +	clk_disable_unprepare(eqos->clk_rx);
> +disable_slave:
> +	clk_disable_unprepare(eqos->clk_slave);
> +disable_master:
> +	clk_disable_unprepare(eqos->clk_master);
> +error:
> +	eqos = ERR_PTR(err);
> +	goto out;
> +}
> +
> +static int tegra_eqos_remove(struct platform_device *pdev)
> +{
> +	struct tegra_eqos *eqos = get_stmmac_bsp_priv(&pdev->dev);
> +
> +	reset_control_assert(eqos->rst);
> +	gpiod_set_value(eqos->reset, 1);
> +	clk_disable_unprepare(eqos->clk_tx);
> +	clk_disable_unprepare(eqos->clk_rx);
> +	clk_disable_unprepare(eqos->clk_slave);
> +	clk_disable_unprepare(eqos->clk_master);
> +
> +	return 0;
> +}
> +
>  struct dwc_eth_dwmac_data {
>  	void *(*probe)(struct platform_device *pdev,
>  		       struct plat_stmmacenet_data *data,
> @@ -160,6 +406,11 @@ static const struct dwc_eth_dwmac_data dwc_qos_data = {
>  	.remove = dwc_qos_remove,
>  };
>  
> +static const struct dwc_eth_dwmac_data tegra_eqos_data = {
> +	.probe = tegra_eqos_probe,
> +	.remove = tegra_eqos_remove,
> +};
> +
>  static int dwc_eth_dwmac_probe(struct platform_device *pdev)
>  {
>  	const struct dwc_eth_dwmac_data *data;
> @@ -245,6 +496,7 @@ static int dwc_eth_dwmac_remove(struct platform_device *pdev)
>  
>  static const struct of_device_id dwc_eth_dwmac_match[] = {
>  	{ .compatible = "snps,dwc-qos-ethernet-4.10", .data = &dwc_qos_data },
> +	{ .compatible = "nvidia,tegra186-eqos", .data = &tegra_eqos_data},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, dwc_eth_dwmac_match);
> 

Please check comments from Mikko Perttunen. Overall is ok.

Thanks.

Re: [PATCH 3/7] net: stmmac: Check for DMA mapping errors

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]

On Mon, Feb 27, 2017 at 11:37:24AM +0200, Mikko Perttunen wrote:
> On 23.02.2017 19:24, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > When DMA mapping an SKB fragment, the mapping must be checked for
> > errors, otherwise the DMA debug code will complain upon unmap.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 6b7a5ce19589..d7387919bdb6 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -2072,6 +2072,8 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
> >  		des = skb_frag_dma_map(priv->device, frag, 0,
> >  				       skb_frag_size(frag),
> >  				       DMA_TO_DEVICE);
> > +		if (dma_mapping_error(priv->device, des))
> > +			goto dma_map_err;
> 
> If this map fails, we should also unmap the previously mapped fragments and
> the separate mapping made using dma_map_single.

That's already taken care of via the dma_map_err label:

	dev_kfree_skb()
	  consume_skb()
	    __kfree_skb()
	      skb_release_all()
	        skb_release_data()

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/7] net: stmmac: Balance PTP reference clock enable/disable

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2483 bytes --]

On Mon, Feb 27, 2017 at 11:31:39AM +0200, Mikko Perttunen wrote:
> On 23.02.2017 19:24, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > clk_prepare_enable() and clk_disable_unprepare() for this clock aren't
> > properly balanced, which can trigger a WARN_ON() in the common clock
> > framework.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 4 ++++
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 -
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 3cbe09682afe..6b7a5ce19589 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -1711,6 +1711,10 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
> >  	stmmac_mmc_setup(priv);
> > 
> >  	if (init_ptp) {
> > +		ret = clk_prepare_enable(priv->plat->clk_ptp_ref);
> > +		if (ret < 0)
> > +			netdev_warn(priv->dev, "failed to enable PTP reference clock: %d\n", ret);
> > +
> 
> Should we return an error code if the clock enable fails?

Yeah, that's probably a good idea.

> >  		ret = stmmac_init_ptp(priv);
> >  		if (ret == -EOPNOTSUPP)
> >  			netdev_warn(priv->dev, "PTP not supported by HW\n");
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > index 5b18355c0d2b..d285d6cfbd0d 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > @@ -365,7 +365,6 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
> >  		plat->clk_ptp_ref = NULL;
> >  		dev_warn(&pdev->dev, "PTP uses main clock\n");
> >  	} else {
> > -		clk_prepare_enable(plat->clk_ptp_ref);
> >  		plat->clk_ptp_rate = clk_get_rate(plat->clk_ptp_ref);
> >  		dev_dbg(&pdev->dev, "PTP rate %d\n", plat->clk_ptp_rate);
> >  	}
> > 
> 
> It seems like there will still be a refcount mismatch for the clock if any
> of the request_irqs that are after stmmac_hw_setup in stmmac_open fail.

Looks like there's a few more things that could be cleaned up on failure
to request those interrupts. I've added another patch to the series that
will attempt to do this.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/7] net: stmmac: Parse FIFO sizes from feature registers

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 5169 bytes --]

On Thu, Mar 02, 2017 at 03:09:11PM +0000, Joao Pinto wrote:
> Hi Thierry,
> 
> Às 5:24 PM de 2/23/2017, Thierry Reding escreveu:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > New version of this core encode the FIFO sizes in one of the feature
> > registers. Use these sizes as default, but still allow device tree to
> > override them for backwards compatibility.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/common.h      | 3 +++
> >  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c  | 2 ++
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
> >  3 files changed, 8 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> > index 144fe84e8a53..6ac653845d82 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> > +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> > @@ -324,6 +324,9 @@ struct dma_features {
> >  	unsigned int number_tx_queues;
> >  	/* Alternate (enhanced) DESC mode */
> >  	unsigned int enh_desc;
> > +	/* TX and RX FIFO sizes */
> > +	unsigned int tx_fifo_size;
> > +	unsigned int rx_fifo_size;
> >  };
> >  
> >  /* GMAC TX FIFO is 8K, Rx FIFO is 16K */
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > index 377d1b44d4f2..8d249f3b34c8 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > @@ -296,6 +296,8 @@ static void dwmac4_get_hw_feature(void __iomem *ioaddr,
> >  	hw_cap = readl(ioaddr + GMAC_HW_FEATURE1);
> >  	dma_cap->av = (hw_cap & GMAC_HW_FEAT_AVSEL) >> 20;
> >  	dma_cap->tsoen = (hw_cap & GMAC_HW_TSOEN) >> 18;
> > +	dma_cap->tx_fifo_size = 128 << ((hw_cap >> 6) & 0x1f);
> > +	dma_cap->rx_fifo_size = 128 << ((hw_cap >> 0) & 0x1f);
> 
> I suggest you follow the way that has been done for other parameters:
> 
> dma_cap->rx_fifo_size = (hw_cap & GMAC_HW_RXFIFOSIZE) >> 0; where
> GMAC_HW_RXFIFOSIZE is GENMASK(4, 0)
> dma_cap->tx_fifo_size = (hw_cap & GMAC_HW_TXFIFOSIZE) >> 6; where
> GMAC_HW_TXFIFOSIZE is GENMASK(10, 6)

Okay, can do.

> >  	/* MAC HW feature2 */
> >  	hw_cap = readl(ioaddr + GMAC_HW_FEATURE2);
> >  	/* TX and RX number of channels */
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index d7387919bdb6..291e34f0ca94 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -1281,6 +1281,9 @@ static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
> >  {
> >  	int rxfifosz = priv->plat->rx_fifo_size;
> >  
> > +	if (rxfifosz == 0)
> > +		rxfifosz = priv->dma_cap.rx_fifo_size;
> > +
> >  	if (priv->plat->force_thresh_dma_mode)
> >  		priv->hw->dma->dma_mode(priv->ioaddr, tc, tc, rxfifosz);
> >  	else if (priv->plat->force_sf_dma_mode || priv->plat->tx_coe) {
> > 
> 
> In my current patch for adding support for MTL in stmmac, I also had this
> approach (sizes comming from feature register and platform data) so nice to see
> it here, because I will be able to reutilize it in my future versions.
> 
> There is only a slight twist that we have to pay attention to it. The RX and TX
> queue size configuration needs to be one the following values:
> 
> FIFO_256 = 0x0,
> FIFO_512 = 0x1,
> FIFO_1k = 0x3,
> FIFO_2k = 0x7,
> FIFO_4k = 0xf,
> FIFO_8k = 0x1f,
> FIFO_16k = 0x3f,
> FIFO_32k = 0x7f
> 
> These are the values you get from the features register, but are these the
> values that users will configure in "plat->rx_fifo_size"? From a quick grep you
> can see that users use real units:
> 
> arch/arm/boot/dts/socfpga.dtsi:			rx-fifo-depth = <4096>;
> arch/arm/boot/dts/socfpga.dtsi:			rx-fifo-depth = <4096>;
> arch/arm/boot/dts/socfpga_arria10.dtsi:			rx-fifo-depth = <16384>;
> arch/arm/boot/dts/socfpga_arria10.dtsi:			rx-fifo-depth = <16384>;
> arch/arm/boot/dts/socfpga_arria10.dtsi:			rx-fifo-depth = <16384>;
> arch/arm/boot/dts/lpc18xx.dtsi:			rx-fifo-depth = <256>;
> arch/nios2/boot/dts/3c120_devboard.dts:				rx-fifo-depth = <8192>;
> arch/nios2/boot/dts/10m50_devboard.dts:			rx-fifo-depth = <8192>;
> 
> So in order to have it configurable from platform and features register you need
> to convert the features values to "real" units and then in the end when you
> write to the DMA Op Mode you need to convert it back to the required values.

That's what the "128 << " part in dwmac4_get_hw_feature() does. As far
as I can tell, that's equivalent to dwmac4_get_real_fifo_sz() function
from your patch, but it's more compact.

> You can check my patch here that already has this done:
> http://patchwork.ozlabs.org/patch/728566/

I see that there's a bit of overlap between that patch and this series.
How do you want to handle this? Do you want me to rebase on top of your
patch, or would you prefer this series to get merged first and rework
your patch on top of it?

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 5/7] net: stmmac: Program RX queue size and flow control

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3623 bytes --]

On Mon, Feb 27, 2017 at 12:09:02PM +0200, Mikko Perttunen wrote:
> On 23.02.2017 19:24, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Program the receive queue size based on the RX FIFO size and enable
> > hardware flow control for large FIFOs.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/dwmac4.h     | 12 +++++++
> >  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 43 ++++++++++++++++++++++--
> >  2 files changed, 53 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> > index db45134fddf0..9acc1f1252b3 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> > @@ -180,6 +180,7 @@ enum power_event {
> >  #define MTL_OP_MODE_TSF			BIT(1)
> > 
> >  #define MTL_OP_MODE_TQS_MASK		GENMASK(24, 16)
> > +#define MTL_OP_MODE_TQS_SHIFT		16
> > 
> >  #define MTL_OP_MODE_TTC_MASK		0x70
> >  #define MTL_OP_MODE_TTC_SHIFT		4
> > @@ -193,6 +194,17 @@ enum power_event {
> >  #define MTL_OP_MODE_TTC_384		(6 << MTL_OP_MODE_TTC_SHIFT)
> >  #define MTL_OP_MODE_TTC_512		(7 << MTL_OP_MODE_TTC_SHIFT)
> > 
> > +#define MTL_OP_MODE_RQS_MASK		GENMASK(29, 20)
> > +#define MTL_OP_MODE_RQS_SHIFT		20
> > +
> > +#define MTL_OP_MODE_RFD_MASK		GENMASK(19, 14)
> > +#define MTL_OP_MODE_RFD_SHIFT		14
> > +
> > +#define MTL_OP_MODE_RFA_MASK		GENMASK(13, 8)
> > +#define MTL_OP_MODE_RFA_SHIFT		8
> > +
> > +#define MTL_OP_MODE_EHFC		BIT(7)
> > +
> >  #define MTL_OP_MODE_RTC_MASK		0x18
> >  #define MTL_OP_MODE_RTC_SHIFT		3
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > index 8d249f3b34c8..03d230201960 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > @@ -185,8 +185,9 @@ static void dwmac4_rx_watchdog(void __iomem *ioaddr, u32 riwt)
> >  }
> > 
> >  static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
> > -				    int rxmode, u32 channel)
> > +				    int rxmode, u32 channel, int rxfifosz)
> >  {
> > +	unsigned int rqs = rxfifosz / 256 - 1;
> >  	u32 mtl_tx_op, mtl_rx_op, mtl_rx_int;
> > 
> >  	/* Following code only done for channel 0, other channels not yet
> > @@ -252,6 +253,44 @@ static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
> >  			mtl_rx_op |= MTL_OP_MODE_RTC_128;
> >  	}
> > 
> > +	mtl_rx_op &= ~MTL_OP_MODE_RQS_MASK;
> > +	mtl_rx_op |= rqs << MTL_OP_MODE_RQS_SHIFT;
> > +
> > +	/* enable flow control only if each channel gets 4 KiB or more FIFO */
> > +	if (rxfifosz >= 4096) {
> > +		unsigned int rfd, rfa;
> > +
> > +		mtl_rx_op |= MTL_OP_MODE_EHFC;
> > +
> > +		switch (rxfifosz) {
> > +		case 4096:
> > +			rfd = 0x03;
> > +			rfa = 0x01;
> > +			break;
> > +
> > +		case 8192:
> > +			rfd = 0x06;
> > +			rfa = 0x0a;
> > +			break;
> > +
> > +		case 16384:
> > +			rfd = 0x06;
> > +			rfa = 0x12;
> > +			break;
> > +
> > +		default:
> > +			rfd = 0x06;
> > +			rfa = 0x1e;
> > +			break;
> > +		}
> 
> Are these values correct? In the 4096 case, rfd > rfa, in all other cases
> the other way around. In any case it would be useful to have a comment
> clarifying the thresholds in bytes.

I'll investigate. To be honest I simply took this from Stephen's U-Boot
driver since that's already tested. I trust Stephen, so I didn't bother
double-checking.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 5/7] net: stmmac: Program RX queue size and flow control

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4387 bytes --]

On Thu, Mar 02, 2017 at 03:15:12PM +0000, Joao Pinto wrote:
> Às 5:24 PM de 2/23/2017, Thierry Reding escreveu:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Program the receive queue size based on the RX FIFO size and enable
> > hardware flow control for large FIFOs.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/dwmac4.h     | 12 +++++++
> >  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 43 ++++++++++++++++++++++--
> >  2 files changed, 53 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> > index db45134fddf0..9acc1f1252b3 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> > @@ -180,6 +180,7 @@ enum power_event {
> >  #define MTL_OP_MODE_TSF			BIT(1)
> >  
> >  #define MTL_OP_MODE_TQS_MASK		GENMASK(24, 16)
> > +#define MTL_OP_MODE_TQS_SHIFT		16
> >  
> >  #define MTL_OP_MODE_TTC_MASK		0x70
> >  #define MTL_OP_MODE_TTC_SHIFT		4
> > @@ -193,6 +194,17 @@ enum power_event {
> >  #define MTL_OP_MODE_TTC_384		(6 << MTL_OP_MODE_TTC_SHIFT)
> >  #define MTL_OP_MODE_TTC_512		(7 << MTL_OP_MODE_TTC_SHIFT)
> >  
> > +#define MTL_OP_MODE_RQS_MASK		GENMASK(29, 20)
> > +#define MTL_OP_MODE_RQS_SHIFT		20
> > +
> > +#define MTL_OP_MODE_RFD_MASK		GENMASK(19, 14)
> > +#define MTL_OP_MODE_RFD_SHIFT		14
> > +
> > +#define MTL_OP_MODE_RFA_MASK		GENMASK(13, 8)
> > +#define MTL_OP_MODE_RFA_SHIFT		8
> > +
> > +#define MTL_OP_MODE_EHFC		BIT(7)
> > +
> >  #define MTL_OP_MODE_RTC_MASK		0x18
> >  #define MTL_OP_MODE_RTC_SHIFT		3
> >  
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > index 8d249f3b34c8..03d230201960 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > @@ -185,8 +185,9 @@ static void dwmac4_rx_watchdog(void __iomem *ioaddr, u32 riwt)
> >  }
> >  
> >  static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
> > -				    int rxmode, u32 channel)
> > +				    int rxmode, u32 channel, int rxfifosz)
> >  {
> > +	unsigned int rqs = rxfifosz / 256 - 1;
> >  	u32 mtl_tx_op, mtl_rx_op, mtl_rx_int;
> >  
> >  	/* Following code only done for channel 0, other channels not yet
> > @@ -252,6 +253,44 @@ static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
> >  			mtl_rx_op |= MTL_OP_MODE_RTC_128;
> >  	}
> >  
> > +	mtl_rx_op &= ~MTL_OP_MODE_RQS_MASK;
> > +	mtl_rx_op |= rqs << MTL_OP_MODE_RQS_SHIFT;
> > +
> > +	/* enable flow control only if each channel gets 4 KiB or more FIFO */
> > +	if (rxfifosz >= 4096) {
> > +		unsigned int rfd, rfa;
> > +
> > +		mtl_rx_op |= MTL_OP_MODE_EHFC;
> > +
> > +		switch (rxfifosz) {
> > +		case 4096:
> > +			rfd = 0x03;
> > +			rfa = 0x01;
> > +			break;
> > +
> > +		case 8192:
> > +			rfd = 0x06;
> > +			rfa = 0x0a;
> > +			break;
> > +
> > +		case 16384:
> > +			rfd = 0x06;
> > +			rfa = 0x12;
> > +			break;
> > +
> > +		default:
> > +			rfd = 0x06;
> > +			rfa = 0x1e;
> > +			break;
> > +		}
> 
> Are these RFA and RFD values suitable to any setup using thresholds? Please
> justify their values in oerder for other developers to understand them.
> 
> > +
> > +		mtl_rx_op &= ~MTL_OP_MODE_RFD_MASK;
> > +		mtl_rx_op |= rfd << MTL_OP_MODE_RFD_SHIFT;
> > +
> > +		mtl_rx_op &= ~MTL_OP_MODE_RFA_MASK;
> > +		mtl_rx_op |= rfa << MTL_OP_MODE_RFA_SHIFT;
> > +	}
> > +
> >  	writel(mtl_rx_op, ioaddr + MTL_CHAN_RX_OP_MODE(channel));
> >  
> >  	/* Enable MTL RX overflow */
> > @@ -264,7 +303,7 @@ static void dwmac4_dma_operation_mode(void __iomem *ioaddr, int txmode,
> >  				      int rxmode, int rxfifosz)
> >  {
> >  	/* Only Channel 0 is actually configured and used */
> > -	dwmac4_dma_chan_op_mode(ioaddr, txmode, rxmode, 0);
> > +	dwmac4_dma_chan_op_mode(ioaddr, txmode, rxmode, 0, rxfifosz);
> 
> Why not configure tx fifo as well, you already getting it from the features
> register.

Primarily I'm not initializing TX FIFO here because we don't pass the
size into this function. Secondarily, the default seems to work fine,
and therefore changing it didn't seem relevant.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 7/7] net: stmmac: dwc-qos: Add Tegra186 support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 5767 bytes --]

On Mon, Feb 27, 2017 at 01:46:01PM +0200, Mikko Perttunen wrote:
> On 23.02.2017 19:24, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > The NVIDIA Tegra186 SoC contains an instance of the Synopsys DWC
> > ethernet QOS IP core. The binding that it uses is slightly different
> > from existing ones because of the integration (clocks, resets, ...).
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  .../ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c    | 252 +++++++++++++++++++++
> >  1 file changed, 252 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> > index 5071d3c15adc..54dfbdc48f6d 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/clk.h>
> >  #include <linux/clk-provider.h>
> >  #include <linux/device.h>
> > +#include <linux/gpio/consumer.h>
> >  #include <linux/ethtool.h>
> >  #include <linux/io.h>
> >  #include <linux/ioport.h>
> > @@ -22,10 +23,24 @@
> >  #include <linux/of_net.h>
> >  #include <linux/mfd/syscon.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/reset.h>
> >  #include <linux/stmmac.h>
> > 
> >  #include "stmmac_platform.h"
> > 
> > +struct tegra_eqos {
> > +	struct device *dev;
> > +	void __iomem *regs;
> > +
> > +	struct reset_control *rst;
> > +	struct clk *clk_master;
> > +	struct clk *clk_slave;
> > +	struct clk *clk_tx;
> > +	struct clk *clk_rx;
> > +
> > +	struct gpio_desc *reset;
> > +};
> > +
> >  static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
> >  				   struct plat_stmmacenet_data *plat_dat)
> >  {
> > @@ -148,6 +163,237 @@ static int dwc_qos_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> > 
> > +#define SDMEMCOMPPADCTRL 0x8800
> > +#define  SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD BIT(31)
> > +
> > +#define AUTO_CAL_CONFIG 0x8804
> > +#define  AUTO_CAL_CONFIG_START BIT(31)
> > +#define  AUTO_CAL_CONFIG_ENABLE BIT(29)
> > +
> > +#define AUTO_CAL_STATUS 0x880c
> > +#define  AUTO_CAL_STATUS_ACTIVE BIT(31)
> > +
> > +static void tegra_eqos_fix_speed(void *priv, unsigned int speed)
> > +{
> > +	struct tegra_eqos *eqos = priv;
> > +	unsigned long rate = 125000000;
> > +	bool needs_calibration = false;
> > +	unsigned int i;
> > +	u32 value;
> > +
> > +	switch (speed) {
> > +	case SPEED_1000:
> > +		needs_calibration = true;
> > +		rate = 125000000;
> > +		break;
> > +
> > +	case SPEED_100:
> > +		needs_calibration = true;
> > +		rate = 25000000;
> > +		break;
> > +
> > +	case SPEED_10:
> > +		rate = 2500000;
> > +		break;
> > +
> > +	default:
> > +		dev_err(eqos->dev, "invalid speed %u\n", speed);
> > +		break;
> > +	}
> > +
> > +	if (needs_calibration) {
> > +		/* calibrate */
> > +		value = readl(eqos->regs + SDMEMCOMPPADCTRL);
> > +		value |= SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD;
> > +		writel(value, eqos->regs + SDMEMCOMPPADCTRL);
> > +
> > +		udelay(1);
> > +
> > +		value = readl(eqos->regs + AUTO_CAL_CONFIG);
> > +		value |= AUTO_CAL_CONFIG_START | AUTO_CAL_CONFIG_ENABLE;
> > +		writel(value, eqos->regs + AUTO_CAL_CONFIG);
> > +
> > +		for (i = 0; i <= 10; i++) {
> > +			value = readl(eqos->regs + AUTO_CAL_STATUS);
> > +			if (value & AUTO_CAL_STATUS_ACTIVE)
> > +				break;
> > +
> > +			udelay(1);
> > +		}
> > +
> > +		if ((value & AUTO_CAL_STATUS_ACTIVE) == 0) {
> > +			dev_err(eqos->dev, "calibration did not start\n");
> > +			goto failed;
> > +		}
> > +
> > +		for (i = 0; i <= 10; i++) {
> > +			value = readl(eqos->regs + AUTO_CAL_STATUS);
> > +			if ((value & AUTO_CAL_STATUS_ACTIVE) == 0)
> > +				break;
> > +
> > +			udelay(20);
> > +		}
> > +
> > +		if (value & AUTO_CAL_STATUS_ACTIVE) {
> > +			dev_err(eqos->dev, "calibration didn't finish\n");
> > +			goto failed;
> > +		}
> 
> Could use readl_poll_timeout/readl_poll_timeout_atomic for these loops
> instead.

I had considered that, but the code ends up looking a lot uglier with
that. But since you and Joao both prefer it, I'll switch over to the I/O
poll helpers anyway.

> 
> > +
> > +	failed:
> > +		value = readl(eqos->regs + SDMEMCOMPPADCTRL);
> > +		value &= ~SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD;
> > +		writel(value, eqos->regs + SDMEMCOMPPADCTRL);
> > +	} else {
> > +		value = readl(eqos->regs + AUTO_CAL_CONFIG);
> > +		value &= ~AUTO_CAL_CONFIG_ENABLE;
> > +		writel(value, eqos->regs + AUTO_CAL_CONFIG);
> > +	}
> > +
> > +	clk_set_rate(eqos->clk_tx, rate);
> 
> Could check error code here, and for other clock ops too.

Done.

> > +}
> > +
> > +static int tegra_eqos_init(struct platform_device *pdev, void *priv)
> > +{
> > +	struct tegra_eqos *eqos = priv;
> > +	unsigned long rate;
> > +	u32 value;
> > +
> > +	rate = clk_get_rate(eqos->clk_slave);
> > +
> > +	value = readl(eqos->regs + 0xdc);
> 
> No point in reading the value when it is fully overwritten.

Good catch.

> > +	value = (rate / 1000000) - 1;
> > +	writel(value, eqos->regs + 0xdc);
> 
> Please add a define for 0xdc.

Right, I must've overlooked that while cleaning up the patch. I've added
a GMAC_1US_TIC_COUNTER define for it.

> > @@ -245,6 +496,7 @@ static int dwc_eth_dwmac_remove(struct platform_device *pdev)
> > 
> >  static const struct of_device_id dwc_eth_dwmac_match[] = {
> >  	{ .compatible = "snps,dwc-qos-ethernet-4.10", .data = &dwc_qos_data },
> > +	{ .compatible = "nvidia,tegra186-eqos", .data = &tegra_eqos_data},
> 
> Missing space before '}'.

Good catch! Fixed.

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 5/7] net: stmmac: Program RX queue size and flow control

[Stephen Warren]

On 03/09/2017 12:42 PM, Thierry Reding wrote:
> On Mon, Feb 27, 2017 at 12:09:02PM +0200, Mikko Perttunen wrote:
>> On 23.02.2017 19:24, Thierry Reding wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> Program the receive queue size based on the RX FIFO size and enable
>>> hardware flow control for large FIFOs.

>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c

>>> @@ -252,6 +253,44 @@ static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
>>>  			mtl_rx_op |= MTL_OP_MODE_RTC_128;
>>>  	}
>>>
>>> +	mtl_rx_op &= ~MTL_OP_MODE_RQS_MASK;
>>> +	mtl_rx_op |= rqs << MTL_OP_MODE_RQS_SHIFT;
>>> +
>>> +	/* enable flow control only if each channel gets 4 KiB or more FIFO */
>>> +	if (rxfifosz >= 4096) {
>>> +		unsigned int rfd, rfa;
>>> +
>>> +		mtl_rx_op |= MTL_OP_MODE_EHFC;
>>> +
>>> +		switch (rxfifosz) {
>>> +		case 4096:
>>> +			rfd = 0x03;
>>> +			rfa = 0x01;
>>> +			break;
>>> +
>>> +		case 8192:
>>> +			rfd = 0x06;
>>> +			rfa = 0x0a;
>>> +			break;
>>> +
>>> +		case 16384:
>>> +			rfd = 0x06;
>>> +			rfa = 0x12;
>>> +			break;
>>> +
>>> +		default:
>>> +			rfd = 0x06;
>>> +			rfa = 0x1e;
>>> +			break;
>>> +		}
>>
>> Are these values correct? In the 4096 case, rfd > rfa, in all other cases
>> the other way around. In any case it would be useful to have a comment
>> clarifying the thresholds in bytes.
>
> I'll investigate. To be honest I simply took this from Stephen's U-Boot
> driver since that's already tested. I trust Stephen, so I didn't bother
> double-checking.

I don't recall for sure, but I think these values came directly from 
either the upstream kernel (the non-stmmac driver) or NV downstream 
kernel EQoS driver, and I re-used them without investigating. I'm not 
even sure if the outer if() expression is true; these numbers might not 
even end up being used?

Re: [PATCH 5/7] net: stmmac: Program RX queue size and flow control

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4632 bytes --]

On Thu, Mar 09, 2017 at 01:18:11PM -0700, Stephen Warren wrote:
> On 03/09/2017 12:42 PM, Thierry Reding wrote:
> > On Mon, Feb 27, 2017 at 12:09:02PM +0200, Mikko Perttunen wrote:
> > > On 23.02.2017 19:24, Thierry Reding wrote:
> > > > From: Thierry Reding <treding@nvidia.com>
> > > > 
> > > > Program the receive queue size based on the RX FIFO size and enable
> > > > hardware flow control for large FIFOs.
> 
> > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> 
> > > > @@ -252,6 +253,44 @@ static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
> > > >  			mtl_rx_op |= MTL_OP_MODE_RTC_128;
> > > >  	}
> > > > 
> > > > +	mtl_rx_op &= ~MTL_OP_MODE_RQS_MASK;
> > > > +	mtl_rx_op |= rqs << MTL_OP_MODE_RQS_SHIFT;
> > > > +
> > > > +	/* enable flow control only if each channel gets 4 KiB or more FIFO */
> > > > +	if (rxfifosz >= 4096) {
> > > > +		unsigned int rfd, rfa;
> > > > +
> > > > +		mtl_rx_op |= MTL_OP_MODE_EHFC;
> > > > +
> > > > +		switch (rxfifosz) {
> > > > +		case 4096:
> > > > +			rfd = 0x03;
> > > > +			rfa = 0x01;
> > > > +			break;
> > > > +
> > > > +		case 8192:
> > > > +			rfd = 0x06;
> > > > +			rfa = 0x0a;
> > > > +			break;
> > > > +
> > > > +		case 16384:
> > > > +			rfd = 0x06;
> > > > +			rfa = 0x12;
> > > > +			break;
> > > > +
> > > > +		default:
> > > > +			rfd = 0x06;
> > > > +			rfa = 0x1e;
> > > > +			break;
> > > > +		}
> > > 
> > > Are these values correct? In the 4096 case, rfd > rfa, in all other cases
> > > the other way around. In any case it would be useful to have a comment
> > > clarifying the thresholds in bytes.
> > 
> > I'll investigate. To be honest I simply took this from Stephen's U-Boot
> > driver since that's already tested. I trust Stephen, so I didn't bother
> > double-checking.
> 
> I don't recall for sure, but I think these values came directly from either
> the upstream kernel (the non-stmmac driver) or NV downstream kernel EQoS
> driver, and I re-used them without investigating. I'm not even sure if the
> outer if() expression is true; these numbers might not even end up being
> used?

Yes they are, and they were even the key to making the STMMAC driver
work on Tegra186. Without programming these fields the driver would fail
to receive any packets.

I noticed that you had comments in the U-Boot driver that I had left out
(most likely because I forgot to add them after cleaning up after the
hacking session). Here's the original extract from U-Boot:

		/*
		 * Set Threshold for Activating Flow Contol space for min 2
		 * frames ie, (1500 * 1) = 1500 bytes.
		 *
		 * Set Threshold for Deactivating Flow Contol for space of
		 * min 1 frame (frame size 1500bytes) in receive fifo
		 */
		if (rqs == ((4096 / 256) - 1)) {
			/*
			 * This violates the above formula because of FIFO size
			 * limit therefore overflow may occur inspite of this.
			 */
			rfd = 0x3;	/* Full-3K */
			rfa = 0x1;	/* Full-1.5K */
		} else if (rqs == ((8192 / 256) - 1)) {
			rfd = 0x6;	/* Full-4K */
			rfa = 0xa;	/* Full-6K */
		} else if (rqs == ((16384 / 256) - 1)) {
			rfd = 0x6;	/* Full-4K */
			rfa = 0x12;	/* Full-10K */
		} else {
			rfd = 0x6;	/* Full-4K */
			rfa = 0x1E;	/* Full-16K */
		}

Two things are strange about this:

	1) the first comment says "2 frames", but the threshold value is
	   clearly just one frame

	2) the first set of rfd/rfa values has a wrong comment, by my
	   understanding: Full-3K should really be Full-2.5K

The encoding of these values is essentially:

	threshold = full - (1K + value * 0.5K)

Here's my updated version from the kernel driver:

		/*
		 * Set Threshold for Activating Flow Control to min 2 frames,
		 * i.e. 1500 * 2 = 3000 bytes.
		 *
		 * Set Threshold for Deactivating Flow Control to min 1 frame,
		 * i.e. 1500 bytes.
		 */
		switch (rxfifosz) {
		case 4096:
			/*
			 * This violates the above formula because of FIFO size
			 * limit therefore overflow may occur in spite of this.
			 */
			rfd = 0x03; /* Full-2.5K */
			rfa = 0x01; /* Full-1.5K */
			break;

		case 8192:
			rfd = 0x06; /* Full-4K */
			rfa = 0x0a; /* Full-6K */
			break;

		case 16384:
			rfd = 0x06; /* Full-4K */
			rfa = 0x12; /* Full-10K */
			break;

		default:
			rfd = 0x06; /* Full-4K */
			rfa = 0x1e; /* Full-16K */
			break;
		}

As best as I can tell these values are within the constraints given in
the TRM and they also make sense to me for the purposes they're used
for.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/7] net: stmmac: Parse FIFO sizes from feature registers

[Joao Pinto]

Às 7:41 PM de 3/9/2017, Thierry Reding escreveu:
> On Thu, Mar 02, 2017 at 03:09:11PM +0000, Joao Pinto wrote:
>> Hi Thierry,
>>
>> Às 5:24 PM de 2/23/2017, Thierry Reding escreveu:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> New version of this core encode the FIFO sizes in one of the feature
>>> registers. Use these sizes as default, but still allow device tree to
>>> override them for backwards compatibility.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>>  drivers/net/ethernet/stmicro/stmmac/common.h      | 3 +++
>>>  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c  | 2 ++
>>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
>>>  3 files changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
>>> index 144fe84e8a53..6ac653845d82 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
>>> @@ -324,6 +324,9 @@ struct dma_features {
>>>  	unsigned int number_tx_queues;
>>>  	/* Alternate (enhanced) DESC mode */
>>>  	unsigned int enh_desc;
>>> +	/* TX and RX FIFO sizes */
>>> +	unsigned int tx_fifo_size;
>>> +	unsigned int rx_fifo_size;
>>>  };
>>>  
>>>  /* GMAC TX FIFO is 8K, Rx FIFO is 16K */
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>> index 377d1b44d4f2..8d249f3b34c8 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>> @@ -296,6 +296,8 @@ static void dwmac4_get_hw_feature(void __iomem *ioaddr,
>>>  	hw_cap = readl(ioaddr + GMAC_HW_FEATURE1);
>>>  	dma_cap->av = (hw_cap & GMAC_HW_FEAT_AVSEL) >> 20;
>>>  	dma_cap->tsoen = (hw_cap & GMAC_HW_TSOEN) >> 18;
>>> +	dma_cap->tx_fifo_size = 128 << ((hw_cap >> 6) & 0x1f);
>>> +	dma_cap->rx_fifo_size = 128 << ((hw_cap >> 0) & 0x1f);
>>
>> I suggest you follow the way that has been done for other parameters:
>>
>> dma_cap->rx_fifo_size = (hw_cap & GMAC_HW_RXFIFOSIZE) >> 0; where
>> GMAC_HW_RXFIFOSIZE is GENMASK(4, 0)
>> dma_cap->tx_fifo_size = (hw_cap & GMAC_HW_TXFIFOSIZE) >> 6; where
>> GMAC_HW_TXFIFOSIZE is GENMASK(10, 6)
> 
> Okay, can do.
> 
>>>  	/* MAC HW feature2 */
>>>  	hw_cap = readl(ioaddr + GMAC_HW_FEATURE2);
>>>  	/* TX and RX number of channels */
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> index d7387919bdb6..291e34f0ca94 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> @@ -1281,6 +1281,9 @@ static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
>>>  {
>>>  	int rxfifosz = priv->plat->rx_fifo_size;
>>>  
>>> +	if (rxfifosz == 0)
>>> +		rxfifosz = priv->dma_cap.rx_fifo_size;
>>> +
>>>  	if (priv->plat->force_thresh_dma_mode)
>>>  		priv->hw->dma->dma_mode(priv->ioaddr, tc, tc, rxfifosz);
>>>  	else if (priv->plat->force_sf_dma_mode || priv->plat->tx_coe) {
>>>
>>
>> In my current patch for adding support for MTL in stmmac, I also had this
>> approach (sizes comming from feature register and platform data) so nice to see
>> it here, because I will be able to reutilize it in my future versions.
>>
>> There is only a slight twist that we have to pay attention to it. The RX and TX
>> queue size configuration needs to be one the following values:
>>
>> FIFO_256 = 0x0,
>> FIFO_512 = 0x1,
>> FIFO_1k = 0x3,
>> FIFO_2k = 0x7,
>> FIFO_4k = 0xf,
>> FIFO_8k = 0x1f,
>> FIFO_16k = 0x3f,
>> FIFO_32k = 0x7f
>>
>> These are the values you get from the features register, but are these the
>> values that users will configure in "plat->rx_fifo_size"? From a quick grep you
>> can see that users use real units:
>>
>> arch/arm/boot/dts/socfpga.dtsi:			rx-fifo-depth = <4096>;
>> arch/arm/boot/dts/socfpga.dtsi:			rx-fifo-depth = <4096>;
>> arch/arm/boot/dts/socfpga_arria10.dtsi:			rx-fifo-depth = <16384>;
>> arch/arm/boot/dts/socfpga_arria10.dtsi:			rx-fifo-depth = <16384>;
>> arch/arm/boot/dts/socfpga_arria10.dtsi:			rx-fifo-depth = <16384>;
>> arch/arm/boot/dts/lpc18xx.dtsi:			rx-fifo-depth = <256>;
>> arch/nios2/boot/dts/3c120_devboard.dts:				rx-fifo-depth = <8192>;
>> arch/nios2/boot/dts/10m50_devboard.dts:			rx-fifo-depth = <8192>;
>>
>> So in order to have it configurable from platform and features register you need
>> to convert the features values to "real" units and then in the end when you
>> write to the DMA Op Mode you need to convert it back to the required values.
> 
> That's what the "128 << " part in dwmac4_get_hw_feature() does. As far
> as I can tell, that's equivalent to dwmac4_get_real_fifo_sz() function
> from your patch, but it's more compact.

Ok, great, but I suggest you put the operation clearer then (a convert macro
maybe), because this way will be more obvious.

> 
>> You can check my patch here that already has this done:
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.ozlabs.org_patch_728566_&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=vfSNvldRkxJ8_e-lvP03KElsSdtNkACrt5RZt47TVZE&s=KByW4fH0QMLR16vmW6ekKqBC-cvqe0nBeOWpXB6uG_4&e= 
> 
> I see that there's a bit of overlap between that patch and this series.
> How do you want to handle this? Do you want me to rebase on top of your
> patch, or would you prefer this series to get merged first and rework
> your patch on top of it?

I suggest you go first, and then I will adapt my development since I still
submiting the multiple queues MAC related operations.

Thanks.

> 
> Thanks,
> Thierry
>