[PATCH v2 0/8] net/macb: fixes after big driver update

[PATCH v2 0/8] net/macb: fixes after big driver update

[Nicolas Ferre]

The recent modifications to the macb driver lead to issues with the probe
function code flow. Here are some attempt to fix them.
This time, some more issues are fixed related to the clock as reported by Boris
Brezillon.

The series is written on top of net-next.

Changes in v2:
- address Cyrille comment about exit condition of queue configuration loop
- add fixes for probe sequence related to clocks
- add ethtool register dump
- fix peripheral version test

Nicolas Ferre (8):
  net/macb: only probe queues once and use stored values
  net/macb: add comment in macb_probe_queues
  net/macb: fix capabilities configuration
  net/macb: trivial: correct wording for caps
  net/macb: fix probe sequence to setup clocks earlier
  net/macb: add the user i/o to ethtool register dump
  net/macb: fix the peripheral version test
  net/macb: unify peripheral version testing

 drivers/net/ethernet/cadence/macb.c | 207 +++++++++++++++++++++---------------
 drivers/net/ethernet/cadence/macb.h |  10 +-
 2 files changed, 130 insertions(+), 87 deletions(-)

-- 
2.1.3

[PATCH v2 1/8] net/macb: only probe queues once and use stored values

[Nicolas Ferre]

When merging at91_ether and macb driver during 421d9df0628b (net/macb: merge
at91_ether driver into macb driver) the probe function has been split. The code
dealing with initialization of queues is now moved in macb_init() which needs
information computed in the parent macb_probe() function.
So, add the queue_mask information to the private structure and use it when
needed in macb_init().

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---

Changes in v2:
- address Cyrille comment about exit condition of queue configuration loop

 drivers/net/ethernet/cadence/macb.c | 7 +++----
 drivers/net/ethernet/cadence/macb.h | 1 +
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index a0a04b3638e6..ac1f18142f7e 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2180,7 +2180,7 @@ static void macb_probe_queues(void __iomem *mem,
 static int macb_init(struct platform_device *pdev)
 {
 	struct net_device *dev = platform_get_drvdata(pdev);
-	unsigned int hw_q, queue_mask, q, num_queues;
+	unsigned int hw_q, q;
 	struct macb *bp = netdev_priv(dev);
 	struct macb_queue *queue;
 	int err;
@@ -2226,10 +2226,8 @@ static int macb_init(struct platform_device *pdev)
 	 * register mapping but we don't want to test the queue index then
 	 * compute the corresponding register offset at run time.
 	 */
-	macb_probe_queues(bp->regs, &queue_mask, &num_queues);
-
 	for (hw_q = 0, q = 0; hw_q < MACB_MAX_QUEUES; ++hw_q) {
-		if (!(queue_mask & (1 << hw_q)))
+		if (!(bp->queue_mask & (1 << hw_q)))
 			continue;
 
 		queue = &bp->queues[q];
@@ -2715,6 +2713,7 @@ static int macb_probe(struct platform_device *pdev)
 	bp->dev = dev;
 	bp->regs = mem;
 	bp->num_queues = num_queues;
+	bp->queue_mask = queue_mask;
 	spin_lock_init(&bp->lock);
 
 	platform_set_drvdata(pdev, dev);
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index bc6e35c40822..0b6afac91bfe 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -785,6 +785,7 @@ struct macb {
 	size_t			rx_buffer_size;
 
 	unsigned int		num_queues;
+	unsigned int		queue_mask;
 	struct macb_queue	queues[MACB_MAX_QUEUES];
 
 	spinlock_t		lock;
-- 
2.1.3

[PATCH v2 2/8] net/macb: add comment in macb_probe_queues

[Nicolas Ferre]

As we access the MID register directly, we need to tell why
we don't use the macb_is_gem() dedicated function.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---

Changes in v2: None

 drivers/net/ethernet/cadence/macb.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index ac1f18142f7e..68d59b3900b1 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2161,9 +2161,13 @@ static void macb_probe_queues(void __iomem *mem,
 	*queue_mask = 0x1;
 	*num_queues = 1;
 
-	/* is it macb or gem ? */
+	/* is it macb or gem ?
+	 *
+	 * We need to read directly from the hardware here because
+	 * we are early in the probe process and don't have the
+	 * MACB_CAPS_MACB_IS_GEM flag positioned
+	 */
 	mid = readl_relaxed(mem + MACB_MID);
-
 	if (MACB_BFEXT(IDNUM, mid) < 0x2)
 		return;
 
-- 
2.1.3

[PATCH v2 3/8] net/macb: fix capabilities configuration

[Nicolas Ferre]

Capabilities configuration by macb_configure_caps() was moved far too late by
421d9df0628b (net/macb: merge at91_ether driver into macb driver) which would
lead to badly configured hardware.
So, move this function to early probe and modify its prototype to re-gain its
original behavior.
DT data retrieval is also moved to simplify the probe code flow.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
---

Changes in v2: None

 drivers/net/ethernet/cadence/macb.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 68d59b3900b1..680e1eebb7ea 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2132,10 +2132,13 @@ static const struct net_device_ops macb_netdev_ops = {
  * Configure peripheral capacities according to device tree
  * and integration options used
  */
-static void macb_configure_caps(struct macb *bp)
+static void macb_configure_caps(struct macb *bp, const struct macb_config *dt_conf)
 {
 	u32 dcfg;
 
+	if (dt_conf)
+		bp->caps = dt_conf->caps;
+
 	if (MACB_BFEXT(IDNUM, macb_readl(bp, MID)) == 0x2)
 		bp->caps |= MACB_CAPS_MACB_IS_GEM;
 
@@ -2313,9 +2316,6 @@ static int macb_init(struct platform_device *pdev)
 
 	macb_or_gem_writel(bp, USRIO, val);
 
-	/* setup capacities */
-	macb_configure_caps(bp);
-
 	/* Set MII management clock divider */
 	val = macb_mdc_clk_div(bp);
 	val |= macb_dbw(bp);
@@ -2720,6 +2720,20 @@ static int macb_probe(struct platform_device *pdev)
 	bp->queue_mask = queue_mask;
 	spin_lock_init(&bp->lock);
 
+	if (np) {
+		const struct of_device_id *match;
+
+		match = of_match_node(macb_dt_ids, np);
+		if (match && match->data) {
+			macb_config = match->data;
+			bp->dma_burst_length = macb_config->dma_burst_length;
+			init = macb_config->init;
+		}
+	}
+
+	/* setup capacities */
+	macb_configure_caps(bp, macb_config);
+
 	platform_set_drvdata(pdev, dev);
 
 	dev->irq = platform_get_irq(pdev, 0);
@@ -2743,20 +2757,6 @@ static int macb_probe(struct platform_device *pdev)
 		bp->phy_interface = err;
 	}
 
-	if (np) {
-		const struct of_device_id *match;
-
-		match = of_match_node(macb_dt_ids, np);
-		if (match)
-			macb_config = match->data;
-	}
-
-	if (macb_config) {
-		bp->caps = macb_config->caps;
-		bp->dma_burst_length = macb_config->dma_burst_length;
-		init = macb_config->init;
-	}
-
 	/* IP specific init */
 	err = init(pdev);
 	if (err)
-- 
2.1.3

[PATCH v2 4/8] net/macb: trivial: correct wording for caps

[Nicolas Ferre]

As a non-native English speaker, I would correct "capacities" of the macb
peripheral to "capabilities": correct me if I'm wrong!

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---

Changes in v2: None

 drivers/net/ethernet/cadence/macb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 680e1eebb7ea..e27bc6964402 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2129,7 +2129,7 @@ static const struct net_device_ops macb_netdev_ops = {
 };
 
 /*
- * Configure peripheral capacities according to device tree
+ * Configure peripheral capabilities according to device tree
  * and integration options used
  */
 static void macb_configure_caps(struct macb *bp, const struct macb_config *dt_conf)
@@ -2731,7 +2731,7 @@ static int macb_probe(struct platform_device *pdev)
 		}
 	}
 
-	/* setup capacities */
+	/* setup capabilities */
 	macb_configure_caps(bp, macb_config);
 
 	platform_set_drvdata(pdev, dev);
-- 
2.1.3

[PATCH v2 5/8] net/macb: fix probe sequence to setup clocks earlier

[Nicolas Ferre]

As accessing the peripheral registers need the clocks to be set, we have to
enable them as soon as possible. Their configuration depend on the type of
device used and determined by the DT compatible string. That lead to add
another initialization function in the DT configuration structure.
As the device private structure length depend on an information read in the
registers, we have to store the clock pointers in temporary variables before
feeding the structure fields.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---

Changes in v2:
- add fixes for probe sequence related to clocks

 drivers/net/ethernet/cadence/macb.c | 169 ++++++++++++++++++++++--------------
 drivers/net/ethernet/cadence/macb.h |   2 +
 2 files changed, 104 insertions(+), 67 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index e27bc6964402..205f4b86d9da 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2184,51 +2184,67 @@ static void macb_probe_queues(void __iomem *mem,
 			(*num_queues)++;
 }
 
-static int macb_init(struct platform_device *pdev)
+static int macb_clk_init(struct platform_device *pdev, struct clk **pclk,
+			 struct clk **hclk, struct clk **tx_clk)
 {
-	struct net_device *dev = platform_get_drvdata(pdev);
-	unsigned int hw_q, q;
-	struct macb *bp = netdev_priv(dev);
-	struct macb_queue *queue;
 	int err;
-	u32 val;
 
-	bp->pclk = devm_clk_get(&pdev->dev, "pclk");
-	if (IS_ERR(bp->pclk)) {
-		err = PTR_ERR(bp->pclk);
+	*pclk = devm_clk_get(&pdev->dev, "pclk");
+	if (IS_ERR(*pclk)) {
+		err = PTR_ERR(*pclk);
 		dev_err(&pdev->dev, "failed to get macb_clk (%u)\n", err);
 		return err;
 	}
 
-	bp->hclk = devm_clk_get(&pdev->dev, "hclk");
-	if (IS_ERR(bp->hclk)) {
-		err = PTR_ERR(bp->hclk);
+	*hclk = devm_clk_get(&pdev->dev, "hclk");
+	if (IS_ERR(*hclk)) {
+		err = PTR_ERR(*hclk);
 		dev_err(&pdev->dev, "failed to get hclk (%u)\n", err);
 		return err;
 	}
 
-	bp->tx_clk = devm_clk_get(&pdev->dev, "tx_clk");
-	if (IS_ERR(bp->tx_clk))
-		bp->tx_clk = NULL;
+	*tx_clk = devm_clk_get(&pdev->dev, "tx_clk");
+	if (IS_ERR(*tx_clk))
+		*tx_clk = NULL;
 
-	err = clk_prepare_enable(bp->pclk);
+	err = clk_prepare_enable(*pclk);
 	if (err) {
 		dev_err(&pdev->dev, "failed to enable pclk (%u)\n", err);
 		return err;
 	}
 
-	err = clk_prepare_enable(bp->hclk);
+	err = clk_prepare_enable(*hclk);
 	if (err) {
 		dev_err(&pdev->dev, "failed to enable hclk (%u)\n", err);
 		goto err_disable_pclk;
 	}
 
-	err = clk_prepare_enable(bp->tx_clk);
+	err = clk_prepare_enable(*tx_clk);
 	if (err) {
 		dev_err(&pdev->dev, "failed to enable tx_clk (%u)\n", err);
 		goto err_disable_hclk;
 	}
 
+	return 0;
+
+err_disable_hclk:
+	clk_disable_unprepare(*hclk);
+
+err_disable_pclk:
+	clk_disable_unprepare(*pclk);
+
+	return err;
+}
+
+static int macb_init(struct platform_device *pdev)
+{
+	struct net_device *dev = platform_get_drvdata(pdev);
+	unsigned int hw_q, q;
+	struct macb *bp = netdev_priv(dev);
+	struct macb_queue *queue;
+	int err;
+	u32 val;
+
 	/* set the queue register mapping once for all: queue0 has a special
 	 * register mapping but we don't want to test the queue index then
 	 * compute the corresponding register offset at run time.
@@ -2266,7 +2282,7 @@ static int macb_init(struct platform_device *pdev)
 			dev_err(&pdev->dev,
 				"Unable to request IRQ %d (error %d)\n",
 				queue->irq, err);
-			goto err_disable_tx_clk;
+			return err;
 		}
 
 		INIT_WORK(&queue->tx_error_task, macb_tx_error_task);
@@ -2322,17 +2338,6 @@ static int macb_init(struct platform_device *pdev)
 	macb_writel(bp, NCFGR, val);
 
 	return 0;
-
-err_disable_tx_clk:
-	clk_disable_unprepare(bp->tx_clk);
-
-err_disable_hclk:
-	clk_disable_unprepare(bp->hclk);
-
-err_disable_pclk:
-	clk_disable_unprepare(bp->pclk);
-
-	return err;
 }
 
 #if defined(CONFIG_OF)
@@ -2600,30 +2605,41 @@ static const struct net_device_ops at91ether_netdev_ops = {
 #endif
 };
 
-static int at91ether_init(struct platform_device *pdev)
+static int at91ether_clk_init(struct platform_device *pdev, struct clk **pclk,
+			      struct clk **hclk, struct clk **tx_clk)
 {
-	struct net_device *dev = platform_get_drvdata(pdev);
-	struct macb *bp = netdev_priv(dev);
 	int err;
-	u32 reg;
 
-	bp->pclk = devm_clk_get(&pdev->dev, "ether_clk");
-	if (IS_ERR(bp->pclk))
-		return PTR_ERR(bp->pclk);
+	*hclk = NULL;
+	*tx_clk = NULL;
+
+	*pclk = devm_clk_get(&pdev->dev, "ether_clk");
+	if (IS_ERR(*pclk))
+		return PTR_ERR(*pclk);
 
-	err = clk_prepare_enable(bp->pclk);
+	err = clk_prepare_enable(*pclk);
 	if (err) {
 		dev_err(&pdev->dev, "failed to enable pclk (%u)\n", err);
 		return err;
 	}
 
+	return 0;
+}
+
+static int at91ether_init(struct platform_device *pdev)
+{
+	struct net_device *dev = platform_get_drvdata(pdev);
+	struct macb *bp = netdev_priv(dev);
+	int err;
+	u32 reg;
+
 	dev->netdev_ops = &at91ether_netdev_ops;
 	dev->ethtool_ops = &macb_ethtool_ops;
 
 	err = devm_request_irq(&pdev->dev, dev->irq, at91ether_interrupt,
 			       0, dev->name, dev);
 	if (err)
-		goto err_disable_clk;
+		return err;
 
 	macb_writel(bp, NCR, 0);
 
@@ -2634,37 +2650,37 @@ static int at91ether_init(struct platform_device *pdev)
 	macb_writel(bp, NCFGR, reg);
 
 	return 0;
-
-err_disable_clk:
-	clk_disable_unprepare(bp->pclk);
-
-	return err;
 }
 
 static const struct macb_config at91sam9260_config = {
 	.caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII,
+	.clk_init = macb_clk_init,
 	.init = macb_init,
 };
 
 static const struct macb_config pc302gem_config = {
 	.caps = MACB_CAPS_SG_DISABLED | MACB_CAPS_GIGABIT_MODE_AVAILABLE,
 	.dma_burst_length = 16,
+	.clk_init = macb_clk_init,
 	.init = macb_init,
 };
 
 static const struct macb_config sama5d3_config = {
 	.caps = MACB_CAPS_SG_DISABLED | MACB_CAPS_GIGABIT_MODE_AVAILABLE,
 	.dma_burst_length = 16,
+	.clk_init = macb_clk_init,
 	.init = macb_init,
 };
 
 static const struct macb_config sama5d4_config = {
 	.caps = 0,
 	.dma_burst_length = 4,
+	.clk_init = macb_clk_init,
 	.init = macb_init,
 };
 
 static const struct macb_config emac_config = {
+	.clk_init = at91ether_clk_init,
 	.init = at91ether_init,
 };
 
@@ -2685,9 +2701,13 @@ MODULE_DEVICE_TABLE(of, macb_dt_ids);
 
 static int macb_probe(struct platform_device *pdev)
 {
+	int (*clk_init)(struct platform_device *, struct clk **,
+			struct clk **, struct clk **)
+					      = macb_clk_init;
 	int (*init)(struct platform_device *) = macb_init;
 	struct device_node *np = pdev->dev.of_node;
 	const struct macb_config *macb_config = NULL;
+	struct clk *pclk, *hclk, *tx_clk;
 	unsigned int queue_mask, num_queues;
 	struct macb_platform_data *pdata;
 	struct phy_device *phydev;
@@ -2698,15 +2718,34 @@ static int macb_probe(struct platform_device *pdev)
 	struct macb *bp;
 	int err;
 
+	if (np) {
+		const struct of_device_id *match;
+
+		match = of_match_node(macb_dt_ids, np);
+		if (match && match->data) {
+			macb_config = match->data;
+			clk_init = macb_config->clk_init;
+			init = macb_config->init;
+		}
+	}
+
+	err = clk_init(pdev, &pclk, &hclk, &tx_clk);
+	if (err)
+		return err;
+
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	mem = devm_ioremap_resource(&pdev->dev, regs);
-	if (IS_ERR(mem))
-		return PTR_ERR(mem);
+	if (IS_ERR(mem)) {
+		err = PTR_ERR(mem);
+		goto err_disable_clocks;
+	}
 
 	macb_probe_queues(mem, &queue_mask, &num_queues);
 	dev = alloc_etherdev_mq(sizeof(*bp), num_queues);
-	if (!dev)
-		return -ENOMEM;
+	if (!dev) {
+		err = -ENOMEM;
+		goto err_disable_clocks;
+	}
 
 	dev->base_addr = regs->start;
 
@@ -2718,27 +2757,23 @@ static int macb_probe(struct platform_device *pdev)
 	bp->regs = mem;
 	bp->num_queues = num_queues;
 	bp->queue_mask = queue_mask;
+	if (macb_config)
+		bp->dma_burst_length = macb_config->dma_burst_length;
+	bp->pclk = pclk;
+	bp->hclk = hclk;
+	bp->tx_clk = tx_clk;
 	spin_lock_init(&bp->lock);
 
-	if (np) {
-		const struct of_device_id *match;
-
-		match = of_match_node(macb_dt_ids, np);
-		if (match && match->data) {
-			macb_config = match->data;
-			bp->dma_burst_length = macb_config->dma_burst_length;
-			init = macb_config->init;
-		}
-	}
-
 	/* setup capabilities */
 	macb_configure_caps(bp, macb_config);
 
 	platform_set_drvdata(pdev, dev);
 
 	dev->irq = platform_get_irq(pdev, 0);
-	if (dev->irq < 0)
-		return dev->irq;
+	if (dev->irq < 0) {
+		err = dev->irq;
+		goto err_disable_clocks;
+	}
 
 	mac = of_get_mac_address(np);
 	if (mac)
@@ -2765,7 +2800,7 @@ static int macb_probe(struct platform_device *pdev)
 	err = register_netdev(dev);
 	if (err) {
 		dev_err(&pdev->dev, "Cannot register net device, aborting.\n");
-		goto err_disable_clocks;
+		goto err_out_unregister_netdev;
 	}
 
 	err = macb_mii_init(bp);
@@ -2787,14 +2822,14 @@ static int macb_probe(struct platform_device *pdev)
 err_out_unregister_netdev:
 	unregister_netdev(dev);
 
-err_disable_clocks:
-	clk_disable_unprepare(bp->tx_clk);
-	clk_disable_unprepare(bp->hclk);
-	clk_disable_unprepare(bp->pclk);
-
 err_out_free_netdev:
 	free_netdev(dev);
 
+err_disable_clocks:
+	clk_disable_unprepare(tx_clk);
+	clk_disable_unprepare(hclk);
+	clk_disable_unprepare(pclk);
+
 	return err;
 }
 
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 0b6afac91bfe..5f9950e84c5e 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -754,6 +754,8 @@ struct macb_or_gem_ops {
 struct macb_config {
 	u32			caps;
 	unsigned int		dma_burst_length;
+	int	(*clk_init)(struct platform_device *pdev, struct clk **pclk,
+			    struct clk **hclk, struct clk **tx_clk);
 	int	(*init)(struct platform_device *pdev);
 };
 
-- 
2.1.3

[PATCH v2 6/8] net/macb: add the user i/o to ethtool register dump

[Nicolas Ferre]

User i/o register EMAC_USRIO or GMAC_UR can be found on both macb and gem
flavors of the peripheral. By using the proper accessor, we can add it to the
register dump feature of ethtool.
Increment the version of this API so it can be noticed from user space.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---

Changes in v2:
- add ethtool register dump

 drivers/net/ethernet/cadence/macb.c | 2 +-
 drivers/net/ethernet/cadence/macb.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 205f4b86d9da..babe972a7c32 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2037,8 +2037,8 @@ static void macb_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 	regs_buff[10] = macb_tx_dma(&bp->queues[0], tail);
 	regs_buff[11] = macb_tx_dma(&bp->queues[0], head);
 
+	regs_buff[12] = macb_or_gem_readl(bp, USRIO);
 	if (macb_is_gem(bp)) {
-		regs_buff[12] = gem_readl(bp, USRIO);
 		regs_buff[13] = gem_readl(bp, DMACFG);
 	}
 }
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 5f9950e84c5e..fd0a22157a88 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -11,7 +11,7 @@
 #define _MACB_H
 
 #define MACB_GREGS_NBR 16
-#define MACB_GREGS_VERSION 1
+#define MACB_GREGS_VERSION 2
 #define MACB_MAX_QUEUES 8
 
 /* MACB register offsets */
-- 
2.1.3

[PATCH v2 7/8] net/macb: fix the peripheral version test

[Nicolas Ferre]

We currently need two checks of the peripheral version in MACB_MID register.
One of them got out of sync after modification by 8a013a9c71b2 (net: macb:
Include multi queue support for xilinx ZynqMP ethernet version).
Fix this in macb_configure_caps() so that xilinx ZynqMP will be considered
as a GEM flavor.

Fixes: 8a013a9c71b2 ("net: macb: Include multi queue support for xilinx ZynqMP
ethernet version")

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
Cc: <stable@vger.kernel.org> #4.0 (if it doesn't make it for -final)
---

Changes in v2:
- fix peripheral version test

 drivers/net/ethernet/cadence/macb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index babe972a7c32..4412895cf4a8 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2139,7 +2139,7 @@ static void macb_configure_caps(struct macb *bp, const struct macb_config *dt_co
 	if (dt_conf)
 		bp->caps = dt_conf->caps;
 
-	if (MACB_BFEXT(IDNUM, macb_readl(bp, MID)) == 0x2)
+	if (MACB_BFEXT(IDNUM, macb_readl(bp, MID)) >= 0x2)
 		bp->caps |= MACB_CAPS_MACB_IS_GEM;
 
 	if (macb_is_gem(bp)) {
-- 
2.1.3

RE: [PATCH v2 7/8] net/macb: fix the peripheral version test

[Punnaiah Choudary Kalluri]

> -----Original Message-----
> From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]
> Sent: Tuesday, March 31, 2015 6:32 PM
> To: linux-arm-kernel@lists.infradead.org; netdev@vger.kernel.org; David S.
> Miller
> Cc: linux-kernel@vger.kernel.org; Boris BREZILLON; Cyrille Pitchen; Alexandre
> Belloni; Michal Simek; Punnaiah Choudary Kalluri; Nicolas Ferre;
> stable@vger.kernel.org
> Subject: [PATCH v2 7/8] net/macb: fix the peripheral version test
>
> We currently need two checks of the peripheral version in MACB_MID
> register.
> One of them got out of sync after modification by 8a013a9c71b2 (net: macb:
> Include multi queue support for xilinx ZynqMP ethernet version).
> Fix this in macb_configure_caps() so that xilinx ZynqMP will be considered
> as a GEM flavor.
>
> Fixes: 8a013a9c71b2 ("net: macb: Include multi queue support for xilinx
> ZynqMP
> ethernet version")
>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
   Reviewed-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>

Thanks for the patch. We have this change in our tree but somehow missed while sending
this patch to mainline.

Regards,
Punnaiah
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
> Cc: <stable@vger.kernel.org> #4.0 (if it doesn't make it for -final)
> ---
>
> Changes in v2:
> - fix peripheral version test
>
>  drivers/net/ethernet/cadence/macb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c
> b/drivers/net/ethernet/cadence/macb.c
> index babe972a7c32..4412895cf4a8 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2139,7 +2139,7 @@ static void macb_configure_caps(struct macb *bp,
> const struct macb_config *dt_co
>       if (dt_conf)
>               bp->caps = dt_conf->caps;
>
> -     if (MACB_BFEXT(IDNUM, macb_readl(bp, MID)) == 0x2)
> +     if (MACB_BFEXT(IDNUM, macb_readl(bp, MID)) >= 0x2)
>               bp->caps |= MACB_CAPS_MACB_IS_GEM;
>
>       if (macb_is_gem(bp)) {
> --
> 2.1.3



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[PATCH v2 8/8] net/macb: unify peripheral version testing

[Nicolas Ferre]

As we need to check peripheral version from the hardware during probe, I
introduce a little helper to unify these tests. It would prevent to
de-synchronize the test like previously observed.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---

Changes in v2: None

 drivers/net/ethernet/cadence/macb.c | 7 ++-----
 drivers/net/ethernet/cadence/macb.h | 5 +++++
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 4412895cf4a8..448a32309dd0 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2139,10 +2139,9 @@ static void macb_configure_caps(struct macb *bp, const struct macb_config *dt_co
 	if (dt_conf)
 		bp->caps = dt_conf->caps;
 
-	if (MACB_BFEXT(IDNUM, macb_readl(bp, MID)) >= 0x2)
+	if (macb_is_gem_hw(bp->regs)) {
 		bp->caps |= MACB_CAPS_MACB_IS_GEM;
 
-	if (macb_is_gem(bp)) {
 		dcfg = gem_readl(bp, DCFG1);
 		if (GEM_BFEXT(IRQCOR, dcfg) == 0)
 			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
@@ -2159,7 +2158,6 @@ static void macb_probe_queues(void __iomem *mem,
 			      unsigned int *num_queues)
 {
 	unsigned int hw_q;
-	u32 mid;
 
 	*queue_mask = 0x1;
 	*num_queues = 1;
@@ -2170,8 +2168,7 @@ static void macb_probe_queues(void __iomem *mem,
 	 * we are early in the probe process and don't have the
 	 * MACB_CAPS_MACB_IS_GEM flag positioned
 	 */
-	mid = readl_relaxed(mem + MACB_MID);
-	if (MACB_BFEXT(IDNUM, mid) < 0x2)
+	if (!macb_is_gem_hw(mem))
 		return;
 
 	/* bit 0 is never set but queue 0 always exists */
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index fd0a22157a88..eb7d76f7bf6a 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -833,4 +833,9 @@ static inline bool macb_is_gem(struct macb *bp)
 	return !!(bp->caps & MACB_CAPS_MACB_IS_GEM);
 }
 
+static inline bool macb_is_gem_hw(void __iomem *addr)
+{
+	return !!(MACB_BFEXT(IDNUM, readl_relaxed(addr + MACB_MID)) >= 0x2);
+}
+
 #endif /* _MACB_H */
-- 
2.1.3

RE: [PATCH v2 8/8] net/macb: unify peripheral version testing

[Punnaiah Choudary Kalluri]

> -----Original Message-----
> From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]
> Sent: Tuesday, March 31, 2015 6:32 PM
> To: linux-arm-kernel@lists.infradead.org; netdev@vger.kernel.org; David S.
> Miller
> Cc: linux-kernel@vger.kernel.org; Boris BREZILLON; Cyrille Pitchen; Alexandre
> Belloni; Michal Simek; Punnaiah Choudary Kalluri; Nicolas Ferre
> Subject: [PATCH v2 8/8] net/macb: unify peripheral version testing
> 
> As we need to check peripheral version from the hardware during probe, I
> introduce a little helper to unify these tests. It would prevent to
> de-synchronize the test like previously observed.
> 
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
    Reviewed-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>

> ---
> 
> Changes in v2: None
> 
>  drivers/net/ethernet/cadence/macb.c | 7 ++-----
>  drivers/net/ethernet/cadence/macb.h | 5 +++++
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.c
> b/drivers/net/ethernet/cadence/macb.c
> index 4412895cf4a8..448a32309dd0 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2139,10 +2139,9 @@ static void macb_configure_caps(struct macb *bp,
> const struct macb_config *dt_co
>  	if (dt_conf)
>  		bp->caps = dt_conf->caps;
> 
> -	if (MACB_BFEXT(IDNUM, macb_readl(bp, MID)) >= 0x2)
> +	if (macb_is_gem_hw(bp->regs)) {
>  		bp->caps |= MACB_CAPS_MACB_IS_GEM;
> 
> -	if (macb_is_gem(bp)) {
>  		dcfg = gem_readl(bp, DCFG1);
>  		if (GEM_BFEXT(IRQCOR, dcfg) == 0)
>  			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
> @@ -2159,7 +2158,6 @@ static void macb_probe_queues(void __iomem
> *mem,
>  			      unsigned int *num_queues)
>  {
>  	unsigned int hw_q;
> -	u32 mid;
> 
>  	*queue_mask = 0x1;
>  	*num_queues = 1;
> @@ -2170,8 +2168,7 @@ static void macb_probe_queues(void __iomem
> *mem,
>  	 * we are early in the probe process and don't have the
>  	 * MACB_CAPS_MACB_IS_GEM flag positioned
>  	 */
> -	mid = readl_relaxed(mem + MACB_MID);
> -	if (MACB_BFEXT(IDNUM, mid) < 0x2)
> +	if (!macb_is_gem_hw(mem))
>  		return;
> 
>  	/* bit 0 is never set but queue 0 always exists */
> diff --git a/drivers/net/ethernet/cadence/macb.h
> b/drivers/net/ethernet/cadence/macb.h
> index fd0a22157a88..eb7d76f7bf6a 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -833,4 +833,9 @@ static inline bool macb_is_gem(struct macb *bp)
>  	return !!(bp->caps & MACB_CAPS_MACB_IS_GEM);
>  }
> 
> +static inline bool macb_is_gem_hw(void __iomem *addr)
> +{
> +	return !!(MACB_BFEXT(IDNUM, readl_relaxed(addr + MACB_MID))
> >= 0x2);
> +}
> +
>  #endif /* _MACB_H */
> --
> 2.1.3

Re: [PATCH v2 0/8] net/macb: fixes after big driver update

[David Miller]

From: Nicolas Ferre <nicolas.ferre@atmel.com>
Date: Tue, 31 Mar 2015 15:01:58 +0200

> The recent modifications to the macb driver lead to issues with the probe
> function code flow. Here are some attempt to fix them.
> This time, some more issues are fixed related to the clock as reported by Boris
> Brezillon.
> 
> The series is written on top of net-next.
> 
> Changes in v2:
> - address Cyrille comment about exit condition of queue configuration loop
> - add fixes for probe sequence related to clocks
> - add ethtool register dump
> - fix peripheral version test

Series applied, thanks.