linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/7] net: ethernet: bgmac: Add platform device support
@ 2016-06-28 19:34 Jon Mason
  2016-06-28 19:34 ` [RFC 1/7] net: ethernet: bgmac: change bgmac_* prints to dev_* prints Jon Mason
                   ` (7 more replies)
  0 siblings, 8 replies; 25+ messages in thread
From: Jon Mason @ 2016-06-28 19:34 UTC (permalink / raw)
  To: zajec5
  Cc: davem, f.fainelli, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

I'm sending out this RFC to see if this is the direction the maintainers
would like to go to add support for other, non-bcma iProc SoC's to the
bgmac driver.  Specifically, we are interested in adding support for the
NSP, Cygnus, and NS2 families (with more possible down the road).

To support non-bcma enabled SoCs, we need to add the standard device
tree "platform device" support.  Unfortunately, this driver is very
tighly coupled with the bcma bus and much unwinding is needed.  I tried
to break this up into a number of patches to make it more obvious what
was being done to add platform device support.  I was able to verify
that the bcma code still works using a 53012K board (NS SoC), and that
the platform code works using a 58625K board (NSP SoC).

It is worth noting that the phy logic present in the driver needs to be
moved to drivers/phy.  However, I was not able to fully decouple that
code from the bgmac driver.  I was able to move it into a separate C
file, with only 2 function calls needed to create and destroy the mii
bus.  Someone with more knowledge of this and HW to test it needs to do
it properly.  This would natually dovetail into creating an interface
which the NSP bgmac can use for the external MDIO Phy to properly
connect (instead of using the fixed phy).

Thanks,
Jon


Jon Mason (7):
  net: ethernet: bgmac: change bgmac_* prints to dev_* prints
  net: ethernet: bgmac: add dma_dev pointer
  net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file
  net: ethernet: bgmac: convert to feature flags
  net: ethernet: bgmac: Add platform device support
  dt-bindings: net: bgmac: add bindings documentation for bgmac
  ARM: dts: NSP: Add bgmac entries

 .../devicetree/bindings/net/brcm,bgmac-enet.txt    |  21 +
 arch/arm/boot/dts/bcm-nsp.dtsi                     |  16 +
 arch/arm/boot/dts/bcm958625k.dts                   |   8 +
 drivers/net/ethernet/broadcom/Kconfig              |  23 +-
 drivers/net/ethernet/broadcom/Makefile             |   2 +
 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c    | 264 +++++++++
 drivers/net/ethernet/broadcom/bgmac-bcma.c         | 315 ++++++++++
 drivers/net/ethernet/broadcom/bgmac-platform.c     | 208 +++++++
 drivers/net/ethernet/broadcom/bgmac.c              | 658 +++++----------------
 drivers/net/ethernet/broadcom/bgmac.h              | 112 +++-
 10 files changed, 1111 insertions(+), 516 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma.c
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-platform.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC 1/7] net: ethernet: bgmac: change bgmac_* prints to dev_* prints
  2016-06-28 19:34 [RFC 0/7] net: ethernet: bgmac: Add platform device support Jon Mason
@ 2016-06-28 19:34 ` Jon Mason
  2016-06-28 19:43   ` Joe Perches
  2016-06-28 19:34 ` [RFC 2/7] net: ethernet: bgmac: add dma_dev pointer Jon Mason
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: Jon Mason @ 2016-06-28 19:34 UTC (permalink / raw)
  To: zajec5
  Cc: davem, f.fainelli, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

The bgmac_* print wrappers call dev_* prints with the dev pointer from
the bcma core.  In anticipation of removing the bcma requirement for
this driver, these must be changed to not reference that struct.  So,
simply change all of the bgmac_* prints to their dev_* counterparts.  In
some cases netdev_* prints are more appropriate, so change those as
well.

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac.c | 105 +++++++++++++++++-----------------
 drivers/net/ethernet/broadcom/bgmac.h |  14 +----
 2 files changed, 56 insertions(+), 63 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index e6e74ca..2ab0887 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -50,7 +50,7 @@ static bool bgmac_wait_value(struct bcma_device *core, u16 reg, u32 mask,
 			return true;
 		udelay(10);
 	}
-	pr_err("Timeout waiting for reg 0x%X\n", reg);
+	dev_err(&core->dev, "Timeout waiting for reg 0x%X\n", reg);
 	return false;
 }
 
@@ -84,8 +84,8 @@ static void bgmac_dma_tx_reset(struct bgmac *bgmac, struct bgmac_dma_ring *ring)
 		udelay(10);
 	}
 	if (i)
-		bgmac_err(bgmac, "Timeout suspending DMA TX ring 0x%X (BGMAC_DMA_TX_STAT: 0x%08X)\n",
-			  ring->mmio_base, val);
+		dev_err(bgmac->dev, "Timeout suspending DMA TX ring 0x%X (BGMAC_DMA_TX_STAT: 0x%08X)\n",
+			ring->mmio_base, val);
 
 	/* Remove SUSPEND bit */
 	bgmac_write(bgmac, ring->mmio_base + BGMAC_DMA_TX_CTL, 0);
@@ -93,13 +93,13 @@ static void bgmac_dma_tx_reset(struct bgmac *bgmac, struct bgmac_dma_ring *ring)
 			      ring->mmio_base + BGMAC_DMA_TX_STATUS,
 			      BGMAC_DMA_TX_STAT, BGMAC_DMA_TX_STAT_DISABLED,
 			      10000)) {
-		bgmac_warn(bgmac, "DMA TX ring 0x%X wasn't disabled on time, waiting additional 300us\n",
-			   ring->mmio_base);
+		dev_warn(bgmac->dev, "DMA TX ring 0x%X wasn't disabled on time, waiting additional 300us\n",
+			 ring->mmio_base);
 		udelay(300);
 		val = bgmac_read(bgmac, ring->mmio_base + BGMAC_DMA_TX_STATUS);
 		if ((val & BGMAC_DMA_TX_STAT) != BGMAC_DMA_TX_STAT_DISABLED)
-			bgmac_err(bgmac, "Reset of DMA TX ring 0x%X failed\n",
-				  ring->mmio_base);
+			dev_err(bgmac->dev, "Reset of DMA TX ring 0x%X failed\n",
+				ring->mmio_base);
 	}
 }
 
@@ -161,7 +161,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 	int i;
 
 	if (skb->len > BGMAC_DESC_CTL1_LEN) {
-		bgmac_err(bgmac, "Too long skb (%d)\n", skb->len);
+		netdev_err(bgmac->net_dev, "Too long skb (%d)\n", skb->len);
 		goto err_drop;
 	}
 
@@ -174,7 +174,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 	 * even when ring->end overflows
 	 */
 	if (ring->end - ring->start + nr_frags + 1 >= BGMAC_TX_RING_SLOTS) {
-		bgmac_err(bgmac, "TX ring is full, queue should be stopped!\n");
+		netdev_err(bgmac->net_dev, "TX ring is full, queue should be stopped!\n");
 		netif_stop_queue(net_dev);
 		return NETDEV_TX_BUSY;
 	}
@@ -241,8 +241,8 @@ err_dma:
 	}
 
 err_dma_head:
-	bgmac_err(bgmac, "Mapping error of skb on ring 0x%X\n",
-		  ring->mmio_base);
+	netdev_err(bgmac->net_dev, "Mapping error of skb on ring 0x%X\n",
+		   ring->mmio_base);
 
 err_drop:
 	dev_kfree_skb(skb);
@@ -320,8 +320,8 @@ static void bgmac_dma_rx_reset(struct bgmac *bgmac, struct bgmac_dma_ring *ring)
 			      ring->mmio_base + BGMAC_DMA_RX_STATUS,
 			      BGMAC_DMA_RX_STAT, BGMAC_DMA_RX_STAT_DISABLED,
 			      10000))
-		bgmac_err(bgmac, "Reset of ring 0x%X RX failed\n",
-			  ring->mmio_base);
+		dev_err(bgmac->dev, "Reset of ring 0x%X RX failed\n",
+			ring->mmio_base);
 }
 
 static void bgmac_dma_rx_enable(struct bgmac *bgmac,
@@ -370,7 +370,7 @@ static int bgmac_dma_rx_skb_for_slot(struct bgmac *bgmac,
 	dma_addr = dma_map_single(dma_dev, buf + BGMAC_RX_BUF_OFFSET,
 				  BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
 	if (dma_mapping_error(dma_dev, dma_addr)) {
-		bgmac_err(bgmac, "DMA mapping error\n");
+		netdev_err(bgmac->net_dev, "DMA mapping error\n");
 		put_page(virt_to_head_page(buf));
 		return -ENOMEM;
 	}
@@ -465,16 +465,16 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 
 			/* Check for poison and drop or pass the packet */
 			if (len == 0xdead && flags == 0xbeef) {
-				bgmac_err(bgmac, "Found poisoned packet at slot %d, DMA issue!\n",
-					  ring->start);
+				netdev_err(bgmac->net_dev, "Found poisoned packet at slot %d, DMA issue!\n",
+					   ring->start);
 				put_page(virt_to_head_page(buf));
 				bgmac->net_dev->stats.rx_errors++;
 				break;
 			}
 
 			if (len > BGMAC_RX_ALLOC_SIZE) {
-				bgmac_err(bgmac, "Found oversized packet at slot %d, DMA issue!\n",
-					  ring->start);
+				netdev_err(bgmac->net_dev, "Found oversized packet at slot %d, DMA issue!\n",
+					   ring->start);
 				put_page(virt_to_head_page(buf));
 				bgmac->net_dev->stats.rx_length_errors++;
 				bgmac->net_dev->stats.rx_errors++;
@@ -486,7 +486,7 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 
 			skb = build_skb(buf, BGMAC_RX_ALLOC_SIZE);
 			if (unlikely(!skb)) {
-				bgmac_err(bgmac, "build_skb failed\n");
+				netdev_err(bgmac->net_dev, "build_skb failed\n");
 				put_page(virt_to_head_page(buf));
 				bgmac->net_dev->stats.rx_errors++;
 				break;
@@ -640,7 +640,7 @@ static int bgmac_dma_alloc(struct bgmac *bgmac)
 	BUILD_BUG_ON(BGMAC_MAX_RX_RINGS > ARRAY_SIZE(ring_base));
 
 	if (!(bcma_aread32(bgmac->core, BCMA_IOST) & BCMA_IOST_DMA64)) {
-		bgmac_err(bgmac, "Core does not report 64-bit DMA\n");
+		dev_err(bgmac->dev, "Core does not report 64-bit DMA\n");
 		return -ENOTSUPP;
 	}
 
@@ -654,8 +654,8 @@ static int bgmac_dma_alloc(struct bgmac *bgmac)
 						     &ring->dma_base,
 						     GFP_KERNEL);
 		if (!ring->cpu_base) {
-			bgmac_err(bgmac, "Allocation of TX ring 0x%X failed\n",
-				  ring->mmio_base);
+			dev_err(bgmac->dev, "Allocation of TX ring 0x%X failed\n",
+				ring->mmio_base);
 			goto err_dma_free;
 		}
 
@@ -679,8 +679,8 @@ static int bgmac_dma_alloc(struct bgmac *bgmac)
 						     &ring->dma_base,
 						     GFP_KERNEL);
 		if (!ring->cpu_base) {
-			bgmac_err(bgmac, "Allocation of RX ring 0x%X failed\n",
-				  ring->mmio_base);
+			dev_err(bgmac->dev, "Allocation of RX ring 0x%X failed\n",
+				ring->mmio_base);
 			err = -ENOMEM;
 			goto err_dma_free;
 		}
@@ -799,8 +799,8 @@ static u16 bgmac_phy_read(struct bgmac *bgmac, u8 phyaddr, u8 reg)
 	bcma_write32(core, phy_access_addr, tmp);
 
 	if (!bgmac_wait_value(core, phy_access_addr, BGMAC_PA_START, 0, 1000)) {
-		bgmac_err(bgmac, "Reading PHY %d register 0x%X failed\n",
-			  phyaddr, reg);
+		dev_err(bgmac->dev, "Reading PHY %d register 0x%X failed\n",
+			phyaddr, reg);
 		return 0xffff;
 	}
 
@@ -832,7 +832,7 @@ static int bgmac_phy_write(struct bgmac *bgmac, u8 phyaddr, u8 reg, u16 value)
 
 	bgmac_write(bgmac, BGMAC_INT_STATUS, BGMAC_IS_MDIO);
 	if (bgmac_read(bgmac, BGMAC_INT_STATUS) & BGMAC_IS_MDIO)
-		bgmac_warn(bgmac, "Error setting MDIO int\n");
+		dev_warn(bgmac->dev, "Error setting MDIO int\n");
 
 	tmp = BGMAC_PA_START;
 	tmp |= BGMAC_PA_WRITE;
@@ -842,8 +842,8 @@ static int bgmac_phy_write(struct bgmac *bgmac, u8 phyaddr, u8 reg, u16 value)
 	bcma_write32(core, phy_access_addr, tmp);
 
 	if (!bgmac_wait_value(core, phy_access_addr, BGMAC_PA_START, 0, 1000)) {
-		bgmac_err(bgmac, "Writing to PHY %d register 0x%X failed\n",
-			  phyaddr, reg);
+		dev_err(bgmac->dev, "Writing to PHY %d register 0x%X failed\n",
+			phyaddr, reg);
 		return -ETIMEDOUT;
 	}
 
@@ -896,7 +896,7 @@ static void bgmac_phy_reset(struct bgmac *bgmac)
 	bgmac_phy_write(bgmac, bgmac->phyaddr, MII_BMCR, BMCR_RESET);
 	udelay(100);
 	if (bgmac_phy_read(bgmac, bgmac->phyaddr, MII_BMCR) & BMCR_RESET)
-		bgmac_err(bgmac, "PHY reset failed\n");
+		dev_err(bgmac->dev, "PHY reset failed\n");
 	bgmac_phy_init(bgmac);
 }
 
@@ -997,7 +997,8 @@ static void bgmac_mac_speed(struct bgmac *bgmac)
 		set |= BGMAC_CMDCFG_ES_2500;
 		break;
 	default:
-		bgmac_err(bgmac, "Unsupported speed: %d\n", bgmac->mac_speed);
+		dev_err(bgmac->dev, "Unsupported speed: %d\n",
+			bgmac->mac_speed);
 	}
 
 	if (bgmac->mac_duplex == DUPLEX_HALF)
@@ -1096,8 +1097,8 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 
 		if (bcm47xx_nvram_getenv("et_swtype", buf, sizeof(buf)) > 0) {
 			if (kstrtou8(buf, 0, &et_swtype))
-				bgmac_err(bgmac, "Failed to parse et_swtype (%s)\n",
-					  buf);
+				dev_err(bgmac->dev, "Failed to parse et_swtype (%s)\n",
+					buf);
 			et_swtype &= 0x0f;
 			et_swtype <<= 4;
 			sw_type = et_swtype;
@@ -1260,7 +1261,7 @@ static irqreturn_t bgmac_interrupt(int irq, void *dev_id)
 
 	int_status &= ~(BGMAC_IS_TX0 | BGMAC_IS_RX);
 	if (int_status)
-		bgmac_err(bgmac, "Unknown IRQs: 0x%08X\n", int_status);
+		dev_err(bgmac->dev, "Unknown IRQs: 0x%08X\n", int_status);
 
 	/* Disable new interrupts until handling existing ones */
 	bgmac_chip_intrs_off(bgmac);
@@ -1314,7 +1315,7 @@ static int bgmac_open(struct net_device *net_dev)
 	err = request_irq(bgmac->core->irq, bgmac_interrupt, IRQF_SHARED,
 			  KBUILD_MODNAME, net_dev);
 	if (err < 0) {
-		bgmac_err(bgmac, "IRQ request error: %d!\n", err);
+		dev_err(bgmac->dev, "IRQ request error: %d!\n", err);
 		bgmac_dma_cleanup(bgmac);
 		return err;
 	}
@@ -1515,7 +1516,7 @@ static void bgmac_get_drvinfo(struct net_device *net_dev,
 			      struct ethtool_drvinfo *info)
 {
 	strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
-	strlcpy(info->bus_info, "BCMA", sizeof(info->bus_info));
+	strlcpy(info->bus_info, "AXI", sizeof(info->bus_info));
 }
 
 static const struct ethtool_ops bgmac_ethtool_ops = {
@@ -1578,14 +1579,14 @@ static int bgmac_fixed_phy_register(struct bgmac *bgmac)
 
 	phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, -1, NULL);
 	if (!phy_dev || IS_ERR(phy_dev)) {
-		bgmac_err(bgmac, "Failed to register fixed PHY device\n");
+		dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
 		return -ENODEV;
 	}
 
 	err = phy_connect_direct(bgmac->net_dev, phy_dev, bgmac_adjust_link,
 				 PHY_INTERFACE_MODE_MII);
 	if (err) {
-		bgmac_err(bgmac, "Connecting PHY failed\n");
+		dev_err(bgmac->dev, "Connecting PHY failed\n");
 		return err;
 	}
 
@@ -1617,7 +1618,7 @@ static int bgmac_mii_register(struct bgmac *bgmac)
 
 	err = mdiobus_register(mii_bus);
 	if (err) {
-		bgmac_err(bgmac, "Registration of mii bus failed\n");
+		dev_err(bgmac->dev, "Registration of mii bus failed\n");
 		goto err_free_bus;
 	}
 
@@ -1629,7 +1630,7 @@ static int bgmac_mii_register(struct bgmac *bgmac)
 	phy_dev = phy_connect(bgmac->net_dev, bus_id, &bgmac_adjust_link,
 			      PHY_INTERFACE_MODE_MII);
 	if (IS_ERR(phy_dev)) {
-		bgmac_err(bgmac, "PHY connection failed\n");
+		dev_err(bgmac->dev, "PHY connecton failed\n");
 		err = PTR_ERR(phy_dev);
 		goto err_unregister_bus;
 	}
@@ -1675,7 +1676,8 @@ static int bgmac_probe(struct bcma_device *core)
 		mac = sprom->et2mac;
 		break;
 	default:
-		pr_err("Unsupported core_unit %d\n", core->core_unit);
+		dev_err(&core->dev, "Unsupported core_unit %d\n",
+			core->core_unit);
 		return -ENOTSUPP;
 	}
 
@@ -1698,6 +1700,7 @@ static int bgmac_probe(struct bcma_device *core)
 	net_dev->irq = core->irq;
 	net_dev->ethtool_ops = &bgmac_ethtool_ops;
 	bgmac = netdev_priv(net_dev);
+	bgmac->dev = &core->dev;
 	bgmac->net_dev = net_dev;
 	bgmac->core = core;
 	bcma_set_drvdata(core, bgmac);
@@ -1709,7 +1712,7 @@ static int bgmac_probe(struct bcma_device *core)
 	/* On BCM4706 we need common core to access PHY */
 	if (core->id.id == BCMA_CORE_4706_MAC_GBIT &&
 	    !core->bus->drv_gmac_cmn.core) {
-		bgmac_err(bgmac, "GMAC CMN core not found (required for BCM4706)\n");
+		dev_err(bgmac->dev, "GMAC CMN core not found (required for BCM4706)\n");
 		err = -ENODEV;
 		goto err_netdev_free;
 	}
@@ -1728,15 +1731,15 @@ static int bgmac_probe(struct bcma_device *core)
 	}
 	bgmac->phyaddr &= BGMAC_PHY_MASK;
 	if (bgmac->phyaddr == BGMAC_PHY_MASK) {
-		bgmac_err(bgmac, "No PHY found\n");
+		dev_err(bgmac->dev, "No PHY found\n");
 		err = -ENODEV;
 		goto err_netdev_free;
 	}
-	bgmac_info(bgmac, "Found PHY addr: %d%s\n", bgmac->phyaddr,
-		   bgmac->phyaddr == BGMAC_PHY_NOREGS ? " (NOREGS)" : "");
+	dev_info(bgmac->dev, "Found PHY addr: %d%s\n", bgmac->phyaddr,
+		 bgmac->phyaddr == BGMAC_PHY_NOREGS ? " (NOREGS)" : "");
 
 	if (core->bus->hosttype == BCMA_HOSTTYPE_PCI) {
-		bgmac_err(bgmac, "PCI setup not implemented\n");
+		dev_err(bgmac->dev, "PCI setup not implemented\n");
 		err = -ENOTSUPP;
 		goto err_netdev_free;
 	}
@@ -1765,7 +1768,7 @@ static int bgmac_probe(struct bcma_device *core)
 
 	err = bgmac_dma_alloc(bgmac);
 	if (err) {
-		bgmac_err(bgmac, "Unable to alloc memory for DMA\n");
+		dev_err(bgmac->dev, "Unable to alloc memory for DMA\n");
 		goto err_netdev_free;
 	}
 
@@ -1779,16 +1782,16 @@ static int bgmac_probe(struct bcma_device *core)
 	bgmac->has_robosw = !!(core->bus->sprom.boardflags_lo &
 			       BGMAC_BFL_ENETROBO);
 	if (bgmac->has_robosw)
-		bgmac_warn(bgmac, "Support for Roboswitch not implemented\n");
+		dev_warn(bgmac->dev, "Support for Roboswitch not implemented\n");
 
 	if (core->bus->sprom.boardflags_lo & BGMAC_BFL_ENETADM)
-		bgmac_warn(bgmac, "Support for ADMtek ethernet switch not implemented\n");
+		dev_warn(bgmac->dev, "Support for ADMtek ethernet switch not implemented\n");
 
 	netif_napi_add(net_dev, &bgmac->napi, bgmac_poll, BGMAC_WEIGHT);
 
 	err = bgmac_mii_register(bgmac);
 	if (err) {
-		bgmac_err(bgmac, "Cannot register MDIO\n");
+		dev_err(bgmac->dev, "Cannot connect to phy\n");
 		goto err_dma_free;
 	}
 
@@ -1798,7 +1801,7 @@ static int bgmac_probe(struct bcma_device *core)
 
 	err = register_netdev(bgmac->net_dev);
 	if (err) {
-		bgmac_err(bgmac, "Cannot register net device\n");
+		dev_err(bgmac->dev, "Cannot register net device\n");
 		goto err_mii_unregister;
 	}
 
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 99beb18..abb9dd8 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -1,17 +1,6 @@
 #ifndef _BGMAC_H
 #define _BGMAC_H
 
-#define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
-
-#define bgmac_err(bgmac, fmt, ...) \
-	dev_err(&(bgmac)->core->dev, fmt, ##__VA_ARGS__)
-#define bgmac_warn(bgmac, fmt, ...) \
-	dev_warn(&(bgmac)->core->dev, fmt,  ##__VA_ARGS__)
-#define bgmac_info(bgmac, fmt, ...) \
-	dev_info(&(bgmac)->core->dev, fmt,  ##__VA_ARGS__)
-#define bgmac_dbg(bgmac, fmt, ...) \
-	dev_dbg(&(bgmac)->core->dev, fmt, ##__VA_ARGS__)
-
 #include <linux/bcma/bcma.h>
 #include <linux/brcmphy.h>
 #include <linux/netdevice.h>
@@ -438,6 +427,8 @@ struct bgmac_rx_header {
 struct bgmac {
 	struct bcma_device *core;
 	struct bcma_device *cmn; /* Reference to CMN core for BCM4706 */
+
+	struct device *dev;
 	struct net_device *net_dev;
 	struct napi_struct napi;
 	struct mii_bus *mii_bus;
@@ -489,5 +480,4 @@ static inline void bgmac_set(struct bgmac *bgmac, u16 offset, u32 set)
 {
 	bgmac_maskset(bgmac, offset, ~0, set);
 }
-
 #endif /* _BGMAC_H */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC 2/7] net: ethernet: bgmac: add dma_dev pointer
  2016-06-28 19:34 [RFC 0/7] net: ethernet: bgmac: Add platform device support Jon Mason
  2016-06-28 19:34 ` [RFC 1/7] net: ethernet: bgmac: change bgmac_* prints to dev_* prints Jon Mason
@ 2016-06-28 19:34 ` Jon Mason
  2016-06-28 19:34 ` [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file Jon Mason
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 25+ messages in thread
From: Jon Mason @ 2016-06-28 19:34 UTC (permalink / raw)
  To: zajec5
  Cc: davem, f.fainelli, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

The dma buffer allocation, etc references a dma_dev device pointer from
the bcma core.  In anticipation of removing the bcma requirement for
this driver, these must be changed to not reference that struct.  Add a
dma_dev device pointer to the bgmac stuct and reference that instead.

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac.c | 17 +++++++++--------
 drivers/net/ethernet/broadcom/bgmac.h |  1 +
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 2ab0887..b5c35bc 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -152,7 +152,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 				    struct bgmac_dma_ring *ring,
 				    struct sk_buff *skb)
 {
-	struct device *dma_dev = bgmac->core->dma_dev;
+	struct device *dma_dev = bgmac->dma_dev;
 	struct net_device *net_dev = bgmac->net_dev;
 	int index = ring->end % BGMAC_TX_RING_SLOTS;
 	struct bgmac_slot_info *slot = &ring->slots[index];
@@ -254,7 +254,7 @@ err_drop:
 /* Free transmitted packets */
 static void bgmac_dma_tx_free(struct bgmac *bgmac, struct bgmac_dma_ring *ring)
 {
-	struct device *dma_dev = bgmac->core->dma_dev;
+	struct device *dma_dev = bgmac->dma_dev;
 	int empty_slot;
 	bool freed = false;
 	unsigned bytes_compl = 0, pkts_compl = 0;
@@ -351,7 +351,7 @@ static void bgmac_dma_rx_enable(struct bgmac *bgmac,
 static int bgmac_dma_rx_skb_for_slot(struct bgmac *bgmac,
 				     struct bgmac_slot_info *slot)
 {
-	struct device *dma_dev = bgmac->core->dma_dev;
+	struct device *dma_dev = bgmac->dma_dev;
 	dma_addr_t dma_addr;
 	struct bgmac_rx_header *rx;
 	void *buf;
@@ -440,7 +440,7 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 	end_slot /= sizeof(struct bgmac_dma_desc);
 
 	while (ring->start != end_slot) {
-		struct device *dma_dev = bgmac->core->dma_dev;
+		struct device *dma_dev = bgmac->dma_dev;
 		struct bgmac_slot_info *slot = &ring->slots[ring->start];
 		struct bgmac_rx_header *rx = slot->buf + BGMAC_RX_BUF_OFFSET;
 		struct sk_buff *skb;
@@ -543,7 +543,7 @@ static bool bgmac_dma_unaligned(struct bgmac *bgmac,
 static void bgmac_dma_tx_ring_free(struct bgmac *bgmac,
 				   struct bgmac_dma_ring *ring)
 {
-	struct device *dma_dev = bgmac->core->dma_dev;
+	struct device *dma_dev = bgmac->dma_dev;
 	struct bgmac_dma_desc *dma_desc = ring->cpu_base;
 	struct bgmac_slot_info *slot;
 	int i;
@@ -569,7 +569,7 @@ static void bgmac_dma_tx_ring_free(struct bgmac *bgmac,
 static void bgmac_dma_rx_ring_free(struct bgmac *bgmac,
 				   struct bgmac_dma_ring *ring)
 {
-	struct device *dma_dev = bgmac->core->dma_dev;
+	struct device *dma_dev = bgmac->dma_dev;
 	struct bgmac_slot_info *slot;
 	int i;
 
@@ -590,7 +590,7 @@ static void bgmac_dma_ring_desc_free(struct bgmac *bgmac,
 				     struct bgmac_dma_ring *ring,
 				     int num_slots)
 {
-	struct device *dma_dev = bgmac->core->dma_dev;
+	struct device *dma_dev = bgmac->dma_dev;
 	int size;
 
 	if (!ring->cpu_base)
@@ -628,7 +628,7 @@ static void bgmac_dma_free(struct bgmac *bgmac)
 
 static int bgmac_dma_alloc(struct bgmac *bgmac)
 {
-	struct device *dma_dev = bgmac->core->dma_dev;
+	struct device *dma_dev = bgmac->dma_dev;
 	struct bgmac_dma_ring *ring;
 	static const u16 ring_base[] = { BGMAC_DMA_BASE0, BGMAC_DMA_BASE1,
 					 BGMAC_DMA_BASE2, BGMAC_DMA_BASE3, };
@@ -1701,6 +1701,7 @@ static int bgmac_probe(struct bcma_device *core)
 	net_dev->ethtool_ops = &bgmac_ethtool_ops;
 	bgmac = netdev_priv(net_dev);
 	bgmac->dev = &core->dev;
+	bgmac->dma_dev = core->dma_dev;
 	bgmac->net_dev = net_dev;
 	bgmac->core = core;
 	bcma_set_drvdata(core, bgmac);
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index abb9dd8..fd20018 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -429,6 +429,7 @@ struct bgmac {
 	struct bcma_device *cmn; /* Reference to CMN core for BCM4706 */
 
 	struct device *dev;
+	struct device *dma_dev;
 	struct net_device *net_dev;
 	struct napi_struct napi;
 	struct mii_bus *mii_bus;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file
  2016-06-28 19:34 [RFC 0/7] net: ethernet: bgmac: Add platform device support Jon Mason
  2016-06-28 19:34 ` [RFC 1/7] net: ethernet: bgmac: change bgmac_* prints to dev_* prints Jon Mason
  2016-06-28 19:34 ` [RFC 2/7] net: ethernet: bgmac: add dma_dev pointer Jon Mason
@ 2016-06-28 19:34 ` Jon Mason
  2016-06-28 20:02   ` Andrew Lunn
  2016-06-29 14:13   ` Andrew Lunn
  2016-06-28 19:34 ` [RFC 4/7] net: ethernet: bgmac: convert to feature flags Jon Mason
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 25+ messages in thread
From: Jon Mason @ 2016-06-28 19:34 UTC (permalink / raw)
  To: zajec5
  Cc: davem, f.fainelli, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

Move the BCMA MDIO phy into a separate file, as it is very tightly
coupled with the BCMA bus.  This will help with the upcoming BCMA
removal from the bgmac driver.  Optimally, this should be moved into
phy drivers, but it is too tightly coupled with the bgmac driver to
effectively move it without more changes to the driver.

Note: the phy_reset was intentionally removed, as the mdio phy subsystem
automatically resets the phy if a reset function pointer is present.  In
addition to the moving of the driver, this reset function is added.

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/ethernet/broadcom/Makefile          |   2 +-
 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c | 264 ++++++++++++++++++++++++
 drivers/net/ethernet/broadcom/bgmac.c           | 246 +++-------------------
 drivers/net/ethernet/broadcom/bgmac.h           |   3 +
 4 files changed, 298 insertions(+), 217 deletions(-)
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c

diff --git a/drivers/net/ethernet/broadcom/Makefile b/drivers/net/ethernet/broadcom/Makefile
index 00584d7..f559794 100644
--- a/drivers/net/ethernet/broadcom/Makefile
+++ b/drivers/net/ethernet/broadcom/Makefile
@@ -10,6 +10,6 @@ obj-$(CONFIG_CNIC) += cnic.o
 obj-$(CONFIG_BNX2X) += bnx2x/
 obj-$(CONFIG_SB1250_MAC) += sb1250-mac.o
 obj-$(CONFIG_TIGON3) += tg3.o
-obj-$(CONFIG_BGMAC) += bgmac.o
+obj-$(CONFIG_BGMAC) += bgmac.o bgmac-bcma-mdio.o
 obj-$(CONFIG_SYSTEMPORT) += bcmsysport.o
 obj-$(CONFIG_BNXT) += bnxt/
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
new file mode 100644
index 0000000..1e65349
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
@@ -0,0 +1,264 @@
+/*
+ * Driver for (BCM4706)? GBit MAC core on BCMA bus.
+ *
+ * Copyright (C) 2012 Rafał Miłecki <zajec5@gmail.com>
+ *
+ * Licensed under the GNU/GPL. See COPYING for details.
+ */
+
+#define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
+
+#include <linux/bcma/bcma.h>
+#include <linux/brcmphy.h>
+#include "bgmac.h"
+
+struct bcma_mdio {
+	struct bcma_device *core;
+	u8 phyaddr;
+};
+
+static bool bcma_mdio_wait_value(struct bcma_device *core, u16 reg, u32 mask,
+				 u32 value, int timeout)
+{
+	u32 val;
+	int i;
+
+	for (i = 0; i < timeout / 10; i++) {
+		val = bcma_read32(core, reg);
+		if ((val & mask) == value)
+			return true;
+		udelay(10);
+	}
+	dev_err(&core->dev, "Timeout waiting for reg 0x%X\n", reg);
+	return false;
+}
+
+/**************************************************
+ * PHY ops
+ **************************************************/
+
+static u16 bcma_mdio_phy_read(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg)
+{
+	struct bcma_device *core;
+	u16 phy_access_addr;
+	u16 phy_ctl_addr;
+	u32 tmp;
+
+	BUILD_BUG_ON(BGMAC_PA_DATA_MASK != BCMA_GMAC_CMN_PA_DATA_MASK);
+	BUILD_BUG_ON(BGMAC_PA_ADDR_MASK != BCMA_GMAC_CMN_PA_ADDR_MASK);
+	BUILD_BUG_ON(BGMAC_PA_ADDR_SHIFT != BCMA_GMAC_CMN_PA_ADDR_SHIFT);
+	BUILD_BUG_ON(BGMAC_PA_REG_MASK != BCMA_GMAC_CMN_PA_REG_MASK);
+	BUILD_BUG_ON(BGMAC_PA_REG_SHIFT != BCMA_GMAC_CMN_PA_REG_SHIFT);
+	BUILD_BUG_ON(BGMAC_PA_WRITE != BCMA_GMAC_CMN_PA_WRITE);
+	BUILD_BUG_ON(BGMAC_PA_START != BCMA_GMAC_CMN_PA_START);
+	BUILD_BUG_ON(BGMAC_PC_EPA_MASK != BCMA_GMAC_CMN_PC_EPA_MASK);
+	BUILD_BUG_ON(BGMAC_PC_MCT_MASK != BCMA_GMAC_CMN_PC_MCT_MASK);
+	BUILD_BUG_ON(BGMAC_PC_MCT_SHIFT != BCMA_GMAC_CMN_PC_MCT_SHIFT);
+	BUILD_BUG_ON(BGMAC_PC_MTE != BCMA_GMAC_CMN_PC_MTE);
+
+	if (bcma_mdio->core->id.id == BCMA_CORE_4706_MAC_GBIT) {
+		core = bcma_mdio->core->bus->drv_gmac_cmn.core;
+		phy_access_addr = BCMA_GMAC_CMN_PHY_ACCESS;
+		phy_ctl_addr = BCMA_GMAC_CMN_PHY_CTL;
+	} else {
+		core = bcma_mdio->core;
+		phy_access_addr = BGMAC_PHY_ACCESS;
+		phy_ctl_addr = BGMAC_PHY_CNTL;
+	}
+
+	tmp = bcma_read32(core, phy_ctl_addr);
+	tmp &= ~BGMAC_PC_EPA_MASK;
+	tmp |= phyaddr;
+	bcma_write32(core, phy_ctl_addr, tmp);
+
+	tmp = BGMAC_PA_START;
+	tmp |= phyaddr << BGMAC_PA_ADDR_SHIFT;
+	tmp |= reg << BGMAC_PA_REG_SHIFT;
+	bcma_write32(core, phy_access_addr, tmp);
+
+	if (!bcma_mdio_wait_value(core, phy_access_addr, BGMAC_PA_START, 0,
+				  1000)) {
+		dev_err(&core->dev, "Reading PHY %d register 0x%X failed\n",
+			phyaddr, reg);
+		return 0xffff;
+	}
+
+	return bcma_read32(core, phy_access_addr) & BGMAC_PA_DATA_MASK;
+}
+
+/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphywr */
+static int bcma_mdio_phy_write(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg,
+			       u16 value)
+{
+	struct bcma_device *core;
+	u16 phy_access_addr;
+	u16 phy_ctl_addr;
+	u32 tmp;
+
+	if (bcma_mdio->core->id.id == BCMA_CORE_4706_MAC_GBIT) {
+		core = bcma_mdio->core->bus->drv_gmac_cmn.core;
+		phy_access_addr = BCMA_GMAC_CMN_PHY_ACCESS;
+		phy_ctl_addr = BCMA_GMAC_CMN_PHY_CTL;
+	} else {
+		core = bcma_mdio->core;
+		phy_access_addr = BGMAC_PHY_ACCESS;
+		phy_ctl_addr = BGMAC_PHY_CNTL;
+	}
+
+	tmp = bcma_read32(core, phy_ctl_addr);
+	tmp &= ~BGMAC_PC_EPA_MASK;
+	tmp |= phyaddr;
+	bcma_write32(core, phy_ctl_addr, tmp);
+
+	bcma_write32(bcma_mdio->core, BGMAC_INT_STATUS, BGMAC_IS_MDIO);
+	if (bcma_read32(bcma_mdio->core, BGMAC_INT_STATUS) & BGMAC_IS_MDIO)
+		dev_warn(&core->dev, "Error setting MDIO int\n");
+
+	tmp = BGMAC_PA_START;
+	tmp |= BGMAC_PA_WRITE;
+	tmp |= phyaddr << BGMAC_PA_ADDR_SHIFT;
+	tmp |= reg << BGMAC_PA_REG_SHIFT;
+	tmp |= value;
+	bcma_write32(core, phy_access_addr, tmp);
+
+	if (!bcma_mdio_wait_value(core, phy_access_addr, BGMAC_PA_START, 0,
+				  1000)) {
+		dev_err(&core->dev, "Writing to PHY %d register 0x%X failed\n",
+			phyaddr, reg);
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphyinit */
+static void bcma_mdio_phy_init(struct bcma_mdio *bcma_mdio)
+{
+	struct bcma_chipinfo *ci = &bcma_mdio->core->bus->chipinfo;
+	u8 i;
+
+	if (ci->id == BCMA_CHIP_ID_BCM5356) {
+		for (i = 0; i < 5; i++) {
+			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x008b);
+			bcma_mdio_phy_write(bcma_mdio, i, 0x15, 0x0100);
+			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000f);
+			bcma_mdio_phy_write(bcma_mdio, i, 0x12, 0x2aaa);
+			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000b);
+		}
+	}
+	if ((ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg != 10) ||
+	    (ci->id == BCMA_CHIP_ID_BCM4749 && ci->pkg != 10) ||
+	    (ci->id == BCMA_CHIP_ID_BCM53572 && ci->pkg != 9)) {
+		struct bcma_drv_cc *cc = &bcma_mdio->core->bus->drv_cc;
+
+		bcma_chipco_chipctl_maskset(cc, 2, ~0xc0000000, 0);
+		bcma_chipco_chipctl_maskset(cc, 4, ~0x80000000, 0);
+		for (i = 0; i < 5; i++) {
+			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000f);
+			bcma_mdio_phy_write(bcma_mdio, i, 0x16, 0x5284);
+			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000b);
+			bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x0010);
+			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000f);
+			bcma_mdio_phy_write(bcma_mdio, i, 0x16, 0x5296);
+			bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x1073);
+			bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x9073);
+			bcma_mdio_phy_write(bcma_mdio, i, 0x16, 0x52b6);
+			bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x9273);
+			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000b);
+		}
+	}
+}
+
+/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphyreset */
+static int bcma_mdio_phy_reset(struct mii_bus *bus)
+{
+	struct bcma_mdio *bcma_mdio = bus->priv;
+	u8 phyaddr = bcma_mdio->phyaddr;
+
+	if (bcma_mdio->phyaddr == BGMAC_PHY_NOREGS)
+		return 0;
+
+	bcma_mdio_phy_write(bcma_mdio, phyaddr, MII_BMCR, BMCR_RESET);
+	udelay(100);
+	if (bcma_mdio_phy_read(bcma_mdio, phyaddr, MII_BMCR) & BMCR_RESET)
+		dev_err(&bcma_mdio->core->dev, "PHY reset failed\n");
+	bcma_mdio_phy_init(bcma_mdio);
+
+	return 0;
+}
+
+/**************************************************
+ * MII
+ **************************************************/
+
+static int bcma_mdio_mii_read(struct mii_bus *bus, int mii_id, int regnum)
+{
+	return bcma_mdio_phy_read(bus->priv, mii_id, regnum);
+}
+
+static int bcma_mdio_mii_write(struct mii_bus *bus, int mii_id, int regnum,
+			       u16 value)
+{
+	return bcma_mdio_phy_write(bus->priv, mii_id, regnum, value);
+}
+
+struct mii_bus *bcma_mdio_mii_register(struct bcma_device *core, u8 phyaddr)
+{
+	struct bcma_mdio *bcma_mdio;
+	struct mii_bus *mii_bus;
+	int err;
+
+	bcma_mdio = kzalloc(sizeof(*bcma_mdio), GFP_KERNEL);
+	if (!bcma_mdio)
+		return ERR_PTR(-ENOMEM);
+
+	mii_bus = mdiobus_alloc();
+	if (!mii_bus) {
+		err = -ENOMEM;
+		goto err;
+	}
+
+	mii_bus->name = "bcma_mdio mii bus";
+	sprintf(mii_bus->id, "%s-%d-%d", "bcma_mdio", core->bus->num,
+		core->core_unit);
+	mii_bus->priv = bcma_mdio;
+	mii_bus->read = bcma_mdio_mii_read;
+	mii_bus->write = bcma_mdio_mii_write;
+	mii_bus->reset = bcma_mdio_phy_reset;
+	mii_bus->parent = &core->dev;
+	mii_bus->phy_mask = ~(1 << phyaddr);
+
+	bcma_mdio->core = core;
+	bcma_mdio->phyaddr = phyaddr;
+
+	err = mdiobus_register(mii_bus);
+	if (err) {
+		dev_err(&core->dev, "Registration of mii bus failed\n");
+		goto err_free_bus;
+	}
+
+	return mii_bus;
+
+err_free_bus:
+	mdiobus_free(mii_bus);
+err:
+	kfree(bcma_mdio);
+	return ERR_PTR(err);
+}
+
+void bcma_mdio_mii_unregister(struct mii_bus *mii_bus)
+{
+	struct bcma_mdio *bcma_mdio;
+
+	if (!mii_bus)
+		return;
+
+	bcma_mdio = mii_bus->priv;
+
+	mdiobus_unregister(mii_bus);
+	mdiobus_free(mii_bus);
+	kfree(bcma_mdio);
+}
+
+MODULE_AUTHOR("Rafał Miłecki");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index b5c35bc..88b9513 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -755,150 +755,6 @@ error:
 	return err;
 }
 
-/**************************************************
- * PHY ops
- **************************************************/
-
-static u16 bgmac_phy_read(struct bgmac *bgmac, u8 phyaddr, u8 reg)
-{
-	struct bcma_device *core;
-	u16 phy_access_addr;
-	u16 phy_ctl_addr;
-	u32 tmp;
-
-	BUILD_BUG_ON(BGMAC_PA_DATA_MASK != BCMA_GMAC_CMN_PA_DATA_MASK);
-	BUILD_BUG_ON(BGMAC_PA_ADDR_MASK != BCMA_GMAC_CMN_PA_ADDR_MASK);
-	BUILD_BUG_ON(BGMAC_PA_ADDR_SHIFT != BCMA_GMAC_CMN_PA_ADDR_SHIFT);
-	BUILD_BUG_ON(BGMAC_PA_REG_MASK != BCMA_GMAC_CMN_PA_REG_MASK);
-	BUILD_BUG_ON(BGMAC_PA_REG_SHIFT != BCMA_GMAC_CMN_PA_REG_SHIFT);
-	BUILD_BUG_ON(BGMAC_PA_WRITE != BCMA_GMAC_CMN_PA_WRITE);
-	BUILD_BUG_ON(BGMAC_PA_START != BCMA_GMAC_CMN_PA_START);
-	BUILD_BUG_ON(BGMAC_PC_EPA_MASK != BCMA_GMAC_CMN_PC_EPA_MASK);
-	BUILD_BUG_ON(BGMAC_PC_MCT_MASK != BCMA_GMAC_CMN_PC_MCT_MASK);
-	BUILD_BUG_ON(BGMAC_PC_MCT_SHIFT != BCMA_GMAC_CMN_PC_MCT_SHIFT);
-	BUILD_BUG_ON(BGMAC_PC_MTE != BCMA_GMAC_CMN_PC_MTE);
-
-	if (bgmac->core->id.id == BCMA_CORE_4706_MAC_GBIT) {
-		core = bgmac->core->bus->drv_gmac_cmn.core;
-		phy_access_addr = BCMA_GMAC_CMN_PHY_ACCESS;
-		phy_ctl_addr = BCMA_GMAC_CMN_PHY_CTL;
-	} else {
-		core = bgmac->core;
-		phy_access_addr = BGMAC_PHY_ACCESS;
-		phy_ctl_addr = BGMAC_PHY_CNTL;
-	}
-
-	tmp = bcma_read32(core, phy_ctl_addr);
-	tmp &= ~BGMAC_PC_EPA_MASK;
-	tmp |= phyaddr;
-	bcma_write32(core, phy_ctl_addr, tmp);
-
-	tmp = BGMAC_PA_START;
-	tmp |= phyaddr << BGMAC_PA_ADDR_SHIFT;
-	tmp |= reg << BGMAC_PA_REG_SHIFT;
-	bcma_write32(core, phy_access_addr, tmp);
-
-	if (!bgmac_wait_value(core, phy_access_addr, BGMAC_PA_START, 0, 1000)) {
-		dev_err(bgmac->dev, "Reading PHY %d register 0x%X failed\n",
-			phyaddr, reg);
-		return 0xffff;
-	}
-
-	return bcma_read32(core, phy_access_addr) & BGMAC_PA_DATA_MASK;
-}
-
-/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphywr */
-static int bgmac_phy_write(struct bgmac *bgmac, u8 phyaddr, u8 reg, u16 value)
-{
-	struct bcma_device *core;
-	u16 phy_access_addr;
-	u16 phy_ctl_addr;
-	u32 tmp;
-
-	if (bgmac->core->id.id == BCMA_CORE_4706_MAC_GBIT) {
-		core = bgmac->core->bus->drv_gmac_cmn.core;
-		phy_access_addr = BCMA_GMAC_CMN_PHY_ACCESS;
-		phy_ctl_addr = BCMA_GMAC_CMN_PHY_CTL;
-	} else {
-		core = bgmac->core;
-		phy_access_addr = BGMAC_PHY_ACCESS;
-		phy_ctl_addr = BGMAC_PHY_CNTL;
-	}
-
-	tmp = bcma_read32(core, phy_ctl_addr);
-	tmp &= ~BGMAC_PC_EPA_MASK;
-	tmp |= phyaddr;
-	bcma_write32(core, phy_ctl_addr, tmp);
-
-	bgmac_write(bgmac, BGMAC_INT_STATUS, BGMAC_IS_MDIO);
-	if (bgmac_read(bgmac, BGMAC_INT_STATUS) & BGMAC_IS_MDIO)
-		dev_warn(bgmac->dev, "Error setting MDIO int\n");
-
-	tmp = BGMAC_PA_START;
-	tmp |= BGMAC_PA_WRITE;
-	tmp |= phyaddr << BGMAC_PA_ADDR_SHIFT;
-	tmp |= reg << BGMAC_PA_REG_SHIFT;
-	tmp |= value;
-	bcma_write32(core, phy_access_addr, tmp);
-
-	if (!bgmac_wait_value(core, phy_access_addr, BGMAC_PA_START, 0, 1000)) {
-		dev_err(bgmac->dev, "Writing to PHY %d register 0x%X failed\n",
-			phyaddr, reg);
-		return -ETIMEDOUT;
-	}
-
-	return 0;
-}
-
-/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphyinit */
-static void bgmac_phy_init(struct bgmac *bgmac)
-{
-	struct bcma_chipinfo *ci = &bgmac->core->bus->chipinfo;
-	struct bcma_drv_cc *cc = &bgmac->core->bus->drv_cc;
-	u8 i;
-
-	if (ci->id == BCMA_CHIP_ID_BCM5356) {
-		for (i = 0; i < 5; i++) {
-			bgmac_phy_write(bgmac, i, 0x1f, 0x008b);
-			bgmac_phy_write(bgmac, i, 0x15, 0x0100);
-			bgmac_phy_write(bgmac, i, 0x1f, 0x000f);
-			bgmac_phy_write(bgmac, i, 0x12, 0x2aaa);
-			bgmac_phy_write(bgmac, i, 0x1f, 0x000b);
-		}
-	}
-	if ((ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg != 10) ||
-	    (ci->id == BCMA_CHIP_ID_BCM4749 && ci->pkg != 10) ||
-	    (ci->id == BCMA_CHIP_ID_BCM53572 && ci->pkg != 9)) {
-		bcma_chipco_chipctl_maskset(cc, 2, ~0xc0000000, 0);
-		bcma_chipco_chipctl_maskset(cc, 4, ~0x80000000, 0);
-		for (i = 0; i < 5; i++) {
-			bgmac_phy_write(bgmac, i, 0x1f, 0x000f);
-			bgmac_phy_write(bgmac, i, 0x16, 0x5284);
-			bgmac_phy_write(bgmac, i, 0x1f, 0x000b);
-			bgmac_phy_write(bgmac, i, 0x17, 0x0010);
-			bgmac_phy_write(bgmac, i, 0x1f, 0x000f);
-			bgmac_phy_write(bgmac, i, 0x16, 0x5296);
-			bgmac_phy_write(bgmac, i, 0x17, 0x1073);
-			bgmac_phy_write(bgmac, i, 0x17, 0x9073);
-			bgmac_phy_write(bgmac, i, 0x16, 0x52b6);
-			bgmac_phy_write(bgmac, i, 0x17, 0x9273);
-			bgmac_phy_write(bgmac, i, 0x1f, 0x000b);
-		}
-	}
-}
-
-/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphyreset */
-static void bgmac_phy_reset(struct bgmac *bgmac)
-{
-	if (bgmac->phyaddr == BGMAC_PHY_NOREGS)
-		return;
-
-	bgmac_phy_write(bgmac, bgmac->phyaddr, MII_BMCR, BMCR_RESET);
-	udelay(100);
-	if (bgmac_phy_read(bgmac, bgmac->phyaddr, MII_BMCR) & BMCR_RESET)
-		dev_err(bgmac->dev, "PHY reset failed\n");
-	bgmac_phy_init(bgmac);
-}
 
 /**************************************************
  * Chip ops
@@ -1155,7 +1011,8 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 	else
 		bgmac_set(bgmac, BGMAC_PHY_CNTL, BGMAC_PC_MTE);
 	bgmac_miiconfig(bgmac);
-	bgmac_phy_init(bgmac);
+	if (bgmac->mii_bus)
+		bgmac->mii_bus->reset(bgmac->mii_bus);
 
 	netdev_reset_queue(bgmac->net_dev);
 }
@@ -1532,17 +1389,6 @@ static const struct ethtool_ops bgmac_ethtool_ops = {
  * MII
  **************************************************/
 
-static int bgmac_mii_read(struct mii_bus *bus, int mii_id, int regnum)
-{
-	return bgmac_phy_read(bus->priv, mii_id, regnum);
-}
-
-static int bgmac_mii_write(struct mii_bus *bus, int mii_id, int regnum,
-			   u16 value)
-{
-	return bgmac_phy_write(bus->priv, mii_id, regnum, value);
-}
-
 static void bgmac_adjust_link(struct net_device *net_dev)
 {
 	struct bgmac *bgmac = netdev_priv(net_dev);
@@ -1567,7 +1413,7 @@ static void bgmac_adjust_link(struct net_device *net_dev)
 	}
 }
 
-static int bgmac_fixed_phy_register(struct bgmac *bgmac)
+static int bgmac_phy_connect_direct(struct bgmac *bgmac)
 {
 	struct fixed_phy_status fphy_status = {
 		.link = 1,
@@ -1593,70 +1439,24 @@ static int bgmac_fixed_phy_register(struct bgmac *bgmac)
 	return err;
 }
 
-static int bgmac_mii_register(struct bgmac *bgmac)
+static int bgmac_phy_connect(struct bgmac *bgmac)
 {
-	struct mii_bus *mii_bus;
 	struct phy_device *phy_dev;
 	char bus_id[MII_BUS_ID_SIZE + 3];
-	int err = 0;
-
-	if (bgmac_is_bcm4707_family(bgmac))
-		return bgmac_fixed_phy_register(bgmac);
-
-	mii_bus = mdiobus_alloc();
-	if (!mii_bus)
-		return -ENOMEM;
-
-	mii_bus->name = "bgmac mii bus";
-	sprintf(mii_bus->id, "%s-%d-%d", "bgmac", bgmac->core->bus->num,
-		bgmac->core->core_unit);
-	mii_bus->priv = bgmac;
-	mii_bus->read = bgmac_mii_read;
-	mii_bus->write = bgmac_mii_write;
-	mii_bus->parent = &bgmac->core->dev;
-	mii_bus->phy_mask = ~(1 << bgmac->phyaddr);
-
-	err = mdiobus_register(mii_bus);
-	if (err) {
-		dev_err(bgmac->dev, "Registration of mii bus failed\n");
-		goto err_free_bus;
-	}
-
-	bgmac->mii_bus = mii_bus;
 
 	/* Connect to the PHY */
-	snprintf(bus_id, sizeof(bus_id), PHY_ID_FMT, mii_bus->id,
+	snprintf(bus_id, sizeof(bus_id), PHY_ID_FMT, bgmac->mii_bus->id,
 		 bgmac->phyaddr);
 	phy_dev = phy_connect(bgmac->net_dev, bus_id, &bgmac_adjust_link,
 			      PHY_INTERFACE_MODE_MII);
 	if (IS_ERR(phy_dev)) {
 		dev_err(bgmac->dev, "PHY connecton failed\n");
-		err = PTR_ERR(phy_dev);
-		goto err_unregister_bus;
+		return PTR_ERR(phy_dev);
 	}
 
-	return err;
-
-err_unregister_bus:
-	mdiobus_unregister(mii_bus);
-err_free_bus:
-	mdiobus_free(mii_bus);
-	return err;
-}
-
-static void bgmac_mii_unregister(struct bgmac *bgmac)
-{
-	struct mii_bus *mii_bus = bgmac->mii_bus;
-
-	mdiobus_unregister(mii_bus);
-	mdiobus_free(mii_bus);
+	return 0;
 }
 
-/**************************************************
- * BCMA bus ops
- **************************************************/
-
-/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipattach */
 static int bgmac_probe(struct bcma_device *core)
 {
 	struct net_device *net_dev;
@@ -1777,9 +1577,6 @@ static int bgmac_probe(struct bcma_device *core)
 	if (bcm47xx_nvram_getenv("et0_no_txint", NULL, 0) == 0)
 		bgmac->int_mask &= ~BGMAC_IS_TX_MASK;
 
-	/* TODO: reset the external phy. Specs are needed */
-	bgmac_phy_reset(bgmac);
-
 	bgmac->has_robosw = !!(core->bus->sprom.boardflags_lo &
 			       BGMAC_BFL_ENETROBO);
 	if (bgmac->has_robosw)
@@ -1790,10 +1587,25 @@ static int bgmac_probe(struct bcma_device *core)
 
 	netif_napi_add(net_dev, &bgmac->napi, bgmac_poll, BGMAC_WEIGHT);
 
-	err = bgmac_mii_register(bgmac);
+	if (!bgmac_is_bcm4707_family(bgmac)) {
+		struct mii_bus *mii_bus;
+
+		mii_bus = bcma_mdio_mii_register(core, bgmac->phyaddr);
+		if (!IS_ERR(mii_bus)) {
+			err = PTR_ERR(mii_bus);
+			goto err_dma_free;
+		}
+
+		bgmac->mii_bus = mii_bus;
+	}
+
+	if (!bgmac->mii_bus)
+		err = bgmac_phy_connect_direct(bgmac);
+	else
+		err = bgmac_phy_connect(bgmac);
 	if (err) {
 		dev_err(bgmac->dev, "Cannot connect to phy\n");
-		goto err_dma_free;
+		goto err_mii_unregister;
 	}
 
 	net_dev->features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
@@ -1803,18 +1615,19 @@ static int bgmac_probe(struct bcma_device *core)
 	err = register_netdev(bgmac->net_dev);
 	if (err) {
 		dev_err(bgmac->dev, "Cannot register net device\n");
-		goto err_mii_unregister;
+		goto err_phy_disconnect;
 	}
 
 	netif_carrier_off(net_dev);
 
 	return 0;
 
+err_phy_disconnect:
+	phy_disconnect(net_dev->phydev);
 err_mii_unregister:
-	bgmac_mii_unregister(bgmac);
+	bcma_mdio_mii_unregister(bgmac->mii_bus);
 err_dma_free:
 	bgmac_dma_free(bgmac);
-
 err_netdev_free:
 	bcma_set_drvdata(core, NULL);
 	free_netdev(net_dev);
@@ -1827,7 +1640,8 @@ static void bgmac_remove(struct bcma_device *core)
 	struct bgmac *bgmac = bcma_get_drvdata(core);
 
 	unregister_netdev(bgmac->net_dev);
-	bgmac_mii_unregister(bgmac);
+	phy_disconnect(bgmac->net_dev->phydev);
+	bcma_mdio_mii_unregister(bgmac->mii_bus);
 	netif_napi_del(&bgmac->napi);
 	bgmac_dma_free(bgmac);
 	bcma_set_drvdata(core, NULL);
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index fd20018..191e64a 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -456,6 +456,9 @@ struct bgmac {
 	bool loopback;
 };
 
+struct mii_bus *bcma_mdio_mii_register(struct bcma_device *core, u8 phyaddr);
+void bcma_mdio_mii_unregister(struct mii_bus *mii_bus);
+
 static inline u32 bgmac_read(struct bgmac *bgmac, u16 offset)
 {
 	return bcma_read32(bgmac->core, offset);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC 4/7] net: ethernet: bgmac: convert to feature flags
  2016-06-28 19:34 [RFC 0/7] net: ethernet: bgmac: Add platform device support Jon Mason
                   ` (2 preceding siblings ...)
  2016-06-28 19:34 ` [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file Jon Mason
@ 2016-06-28 19:34 ` Jon Mason
  2016-06-28 19:34 ` [RFC 5/7] net: ethernet: bgmac: Add platform device support Jon Mason
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 25+ messages in thread
From: Jon Mason @ 2016-06-28 19:34 UTC (permalink / raw)
  To: zajec5
  Cc: davem, f.fainelli, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

The bgmac driver is using the bcma provides device ID and revision, as
well as the SoC ID and package, to determine which features are
necessary to enable, reset, etc in the driver.   In anticipation of
removing the bcma requirement for this driver, these must be changed to
not reference that struct.  In place of that, each "feature" has been
given a flag, and the flags are enabled for their respective device and
SoC.

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac.c | 167 ++++++++++++++++++++++++----------
 drivers/net/ethernet/broadcom/bgmac.h |  21 ++++-
 2 files changed, 140 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 88b9513..4346994 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -109,7 +109,7 @@ static void bgmac_dma_tx_enable(struct bgmac *bgmac,
 	u32 ctl;
 
 	ctl = bgmac_read(bgmac, ring->mmio_base + BGMAC_DMA_TX_CTL);
-	if (bgmac->core->id.rev >= 4) {
+	if (bgmac->feature_flags & BGMAC_FEAT_TX_MASK_SETUP) {
 		ctl &= ~BGMAC_DMA_TX_BL_MASK;
 		ctl |= BGMAC_DMA_TX_BL_128 << BGMAC_DMA_TX_BL_SHIFT;
 
@@ -330,7 +330,7 @@ static void bgmac_dma_rx_enable(struct bgmac *bgmac,
 	u32 ctl;
 
 	ctl = bgmac_read(bgmac, ring->mmio_base + BGMAC_DMA_RX_CTL);
-	if (bgmac->core->id.rev >= 4) {
+	if (bgmac->feature_flags & BGMAC_FEAT_RX_MASK_SETUP) {
 		ctl &= ~BGMAC_DMA_RX_BL_MASK;
 		ctl |= BGMAC_DMA_RX_BL_128 << BGMAC_DMA_RX_BL_SHIFT;
 
@@ -768,14 +768,20 @@ static void bgmac_cmdcfg_maskset(struct bgmac *bgmac, u32 mask, u32 set,
 {
 	u32 cmdcfg = bgmac_read(bgmac, BGMAC_CMDCFG);
 	u32 new_val = (cmdcfg & mask) | set;
+	u32 cmdcfg_sr;
 
-	bgmac_set(bgmac, BGMAC_CMDCFG, BGMAC_CMDCFG_SR(bgmac->core->id.rev));
+	if (bgmac->feature_flags & BGMAC_FEAT_CMDCFG_SR_REV4)
+		cmdcfg_sr = BGMAC_CMDCFG_SR_REV4;
+	else
+		cmdcfg_sr = BGMAC_CMDCFG_SR_REV0;
+
+	bgmac_set(bgmac, BGMAC_CMDCFG, cmdcfg_sr);
 	udelay(2);
 
 	if (new_val != cmdcfg || force)
 		bgmac_write(bgmac, BGMAC_CMDCFG, new_val);
 
-	bgmac_mask(bgmac, BGMAC_CMDCFG, ~BGMAC_CMDCFG_SR(bgmac->core->id.rev));
+	bgmac_mask(bgmac, BGMAC_CMDCFG, ~cmdcfg_sr);
 	udelay(2);
 }
 
@@ -804,7 +810,7 @@ static void bgmac_chip_stats_update(struct bgmac *bgmac)
 {
 	int i;
 
-	if (bgmac->core->id.id != BCMA_CORE_4706_MAC_GBIT) {
+	if (!(bgmac->feature_flags & BGMAC_FEAT_NO_CLR_MIB)) {
 		for (i = 0; i < BGMAC_NUM_MIB_TX_REGS; i++)
 			bgmac->mib_tx_regs[i] =
 				bgmac_read(bgmac,
@@ -823,7 +829,7 @@ static void bgmac_clear_mib(struct bgmac *bgmac)
 {
 	int i;
 
-	if (bgmac->core->id.id == BCMA_CORE_4706_MAC_GBIT)
+	if (bgmac->feature_flags & BGMAC_FEAT_NO_CLR_MIB)
 		return;
 
 	bgmac_set(bgmac, BGMAC_DEV_CTL, BGMAC_DC_MROR);
@@ -866,9 +872,8 @@ static void bgmac_mac_speed(struct bgmac *bgmac)
 static void bgmac_miiconfig(struct bgmac *bgmac)
 {
 	struct bcma_device *core = bgmac->core;
-	u8 imode;
 
-	if (bgmac_is_bcm4707_family(bgmac)) {
+	if (bgmac->feature_flags & BGMAC_FEAT_FORCE_SPEED_2500) {
 		bcma_awrite32(core, BCMA_IOCTL,
 			      bcma_aread32(core, BCMA_IOCTL) | 0x40 |
 			      BGMAC_BCMA_IOCTL_SW_CLKEN);
@@ -876,6 +881,8 @@ static void bgmac_miiconfig(struct bgmac *bgmac)
 		bgmac->mac_duplex = DUPLEX_FULL;
 		bgmac_mac_speed(bgmac);
 	} else {
+		u8 imode;
+
 		imode = (bgmac_read(bgmac, BGMAC_DEV_STATUS) &
 			BGMAC_DS_MM_MASK) >> BGMAC_DS_MM_SHIFT;
 		if (imode == 0 || imode == 1) {
@@ -890,9 +897,7 @@ static void bgmac_miiconfig(struct bgmac *bgmac)
 static void bgmac_chip_reset(struct bgmac *bgmac)
 {
 	struct bcma_device *core = bgmac->core;
-	struct bcma_bus *bus = core->bus;
-	struct bcma_chipinfo *ci = &bus->chipinfo;
-	u32 flags;
+	u32 cmdcfg_sr;
 	u32 iost;
 	int i;
 
@@ -915,15 +920,12 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 	}
 
 	iost = bcma_aread32(core, BCMA_IOST);
-	if ((ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg == BCMA_PKG_ID_BCM47186) ||
-	    (ci->id == BCMA_CHIP_ID_BCM4749 && ci->pkg == 10) ||
-	    (ci->id == BCMA_CHIP_ID_BCM53572 && ci->pkg == BCMA_PKG_ID_BCM47188))
+	if (bgmac->feature_flags & BGMAC_FEAT_IOST_ATTACHED)
 		iost &= ~BGMAC_BCMA_IOST_ATTACHED;
 
 	/* 3GMAC: for BCM4707 & BCM47094, only do core reset at bgmac_probe() */
-	if (ci->id != BCMA_CHIP_ID_BCM4707 &&
-	    ci->id != BCMA_CHIP_ID_BCM47094) {
-		flags = 0;
+	if (!(bgmac->feature_flags & BGMAC_FEAT_NO_RESET)) {
+		u32 flags = 0;
 		if (iost & BGMAC_BCMA_IOST_ATTACHED) {
 			flags = BGMAC_BCMA_IOCTL_SW_CLKEN;
 			if (!bgmac->has_robosw)
@@ -933,7 +935,7 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 	}
 
 	/* Request Misc PLL for corerev > 2 */
-	if (core->id.rev > 2 && !bgmac_is_bcm4707_family(bgmac)) {
+	if (bgmac->feature_flags & BGMAC_FEAT_MISC_PLL_REQ) {
 		bgmac_set(bgmac, BCMA_CLKCTLST,
 			  BGMAC_BCMA_CLKCTLST_MISC_PLL_REQ);
 		bgmac_wait_value(bgmac->core, BCMA_CLKCTLST,
@@ -942,9 +944,7 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 				 1000);
 	}
 
-	if (ci->id == BCMA_CHIP_ID_BCM5357 ||
-	    ci->id == BCMA_CHIP_ID_BCM4749 ||
-	    ci->id == BCMA_CHIP_ID_BCM53572) {
+	if (bgmac->feature_flags & BGMAC_FEAT_SW_TYPE_PHY) {
 		struct bcma_drv_cc *cc = &bgmac->core->bus->drv_cc;
 		u8 et_swtype = 0;
 		u8 sw_type = BGMAC_CHIPCTL_1_SW_TYPE_EPHY |
@@ -958,11 +958,9 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 			et_swtype &= 0x0f;
 			et_swtype <<= 4;
 			sw_type = et_swtype;
-		} else if (ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg == BCMA_PKG_ID_BCM5358) {
+		} else if (bgmac->feature_flags & BGMAC_FEAT_SW_TYPE_EPHYRMII) {
 			sw_type = BGMAC_CHIPCTL_1_SW_TYPE_EPHYRMII;
-		} else if ((ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg == BCMA_PKG_ID_BCM47186) ||
-			   (ci->id == BCMA_CHIP_ID_BCM4749 && ci->pkg == 10) ||
-			   (ci->id == BCMA_CHIP_ID_BCM53572 && ci->pkg == BCMA_PKG_ID_BCM47188)) {
+		} else if (bgmac->feature_flags & BGMAC_FEAT_SW_TYPE_RGMII) {
 			sw_type = BGMAC_CHIPCTL_1_IF_TYPE_RGMII |
 				  BGMAC_CHIPCTL_1_SW_TYPE_RGMII;
 		}
@@ -982,6 +980,11 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 	 * BGMAC_CMDCFG is read _after_ putting chip in a reset. So it has to
 	 * be keps until taking MAC out of the reset.
 	 */
+	if (bgmac->feature_flags & BGMAC_FEAT_CMDCFG_SR_REV4)
+		cmdcfg_sr = BGMAC_CMDCFG_SR_REV4;
+	else
+		cmdcfg_sr = BGMAC_CMDCFG_SR_REV0;
+
 	bgmac_cmdcfg_maskset(bgmac,
 			     ~(BGMAC_CMDCFG_TE |
 			       BGMAC_CMDCFG_RE |
@@ -999,13 +1002,13 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 			     BGMAC_CMDCFG_PROM |
 			     BGMAC_CMDCFG_NLC |
 			     BGMAC_CMDCFG_CFE |
-			     BGMAC_CMDCFG_SR(core->id.rev),
+			     cmdcfg_sr,
 			     false);
 	bgmac->mac_speed = SPEED_UNKNOWN;
 	bgmac->mac_duplex = DUPLEX_UNKNOWN;
 
 	bgmac_clear_mib(bgmac);
-	if (core->id.id == BCMA_CORE_4706_MAC_GBIT)
+	if (bgmac->feature_flags & BGMAC_FEAT_CMN_PHY_CTL)
 		bcma_maskset32(bgmac->cmn, BCMA_GMAC_CMN_PHY_CTL, ~0,
 			       BCMA_GMAC_CMN_PC_MTE);
 	else
@@ -1031,46 +1034,48 @@ static void bgmac_chip_intrs_off(struct bgmac *bgmac)
 /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/gmac_enable */
 static void bgmac_enable(struct bgmac *bgmac)
 {
-	struct bcma_chipinfo *ci = &bgmac->core->bus->chipinfo;
+	u32 cmdcfg_sr;
 	u32 cmdcfg;
 	u32 mode;
-	u32 rxq_ctl;
-	u32 fl_ctl;
-	u16 bp_clk;
-	u8 mdp;
+
+	if (bgmac->feature_flags & BGMAC_FEAT_CMDCFG_SR_REV4)
+		cmdcfg_sr = BGMAC_CMDCFG_SR_REV4;
+	else
+		cmdcfg_sr = BGMAC_CMDCFG_SR_REV0;
 
 	cmdcfg = bgmac_read(bgmac, BGMAC_CMDCFG);
 	bgmac_cmdcfg_maskset(bgmac, ~(BGMAC_CMDCFG_TE | BGMAC_CMDCFG_RE),
-			     BGMAC_CMDCFG_SR(bgmac->core->id.rev), true);
+			     cmdcfg_sr, true);
 	udelay(2);
 	cmdcfg |= BGMAC_CMDCFG_TE | BGMAC_CMDCFG_RE;
 	bgmac_write(bgmac, BGMAC_CMDCFG, cmdcfg);
 
 	mode = (bgmac_read(bgmac, BGMAC_DEV_STATUS) & BGMAC_DS_MM_MASK) >>
 		BGMAC_DS_MM_SHIFT;
-	if (ci->id != BCMA_CHIP_ID_BCM47162 || mode != 0)
+	if (bgmac->feature_flags & BGMAC_FEAT_CLKCTLST || mode != 0)
 		bgmac_set(bgmac, BCMA_CLKCTLST, BCMA_CLKCTLST_FORCEHT);
-	if (ci->id == BCMA_CHIP_ID_BCM47162 && mode == 2)
+	if (bgmac->feature_flags & BGMAC_FEAT_CLKCTLST && mode == 2)
 		bcma_chipco_chipctl_maskset(&bgmac->core->bus->drv_cc, 1, ~0,
 					    BGMAC_CHIPCTL_1_RXC_DLL_BYPASS);
 
-	switch (ci->id) {
-	case BCMA_CHIP_ID_BCM5357:
-	case BCMA_CHIP_ID_BCM4749:
-	case BCMA_CHIP_ID_BCM53572:
-	case BCMA_CHIP_ID_BCM4716:
-	case BCMA_CHIP_ID_BCM47162:
-		fl_ctl = 0x03cb04cb;
-		if (ci->id == BCMA_CHIP_ID_BCM5357 ||
-		    ci->id == BCMA_CHIP_ID_BCM4749 ||
-		    ci->id == BCMA_CHIP_ID_BCM53572)
+	if (bgmac->feature_flags & (BGMAC_FEAT_FLW_CTRL1 |
+				    BGMAC_FEAT_FLW_CTRL2)) {
+		u32 fl_ctl;
+
+		if (bgmac->feature_flags & BGMAC_FEAT_FLW_CTRL1)
 			fl_ctl = 0x2300e1;
+		else
+			fl_ctl = 0x03cb04cb;
+
 		bgmac_write(bgmac, BGMAC_FLOW_CTL_THRESH, fl_ctl);
 		bgmac_write(bgmac, BGMAC_PAUSE_CTL, 0x27fff);
-		break;
 	}
 
-	if (!bgmac_is_bcm4707_family(bgmac)) {
+	if (bgmac->feature_flags & BGMAC_FEAT_SET_RXQ_CLK) {
+		u32 rxq_ctl;
+		u16 bp_clk;
+		u8 mdp;
+
 		rxq_ctl = bgmac_read(bgmac, BGMAC_RXQ_CTL);
 		rxq_ctl &= ~BGMAC_RXQ_CTL_MDP_MASK;
 		bp_clk = bcma_pmu_get_bus_clock(&bgmac->core->bus->drv_cc) /
@@ -1585,6 +1590,74 @@ static int bgmac_probe(struct bcma_device *core)
 	if (core->bus->sprom.boardflags_lo & BGMAC_BFL_ENETADM)
 		dev_warn(bgmac->dev, "Support for ADMtek ethernet switch not implemented\n");
 
+	/* Feature Flags */
+	switch (core->bus->chipinfo.id) {
+	case BCMA_CHIP_ID_BCM5357:
+		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
+		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
+		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
+		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
+		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM47186) {
+			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
+			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
+		}
+		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM5358)
+			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_EPHYRMII;
+		break;
+	case BCMA_CHIP_ID_BCM53572:
+		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
+		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
+		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
+		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
+		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM47188) {
+			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
+			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
+		}
+		break;
+	case BCMA_CHIP_ID_BCM4749:
+		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
+		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
+		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
+		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
+		if (core->bus->chipinfo.pkg == 10) {
+			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
+			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
+		}
+		break;
+	case BCMA_CHIP_ID_BCM4716:
+		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
+		/* fallthrough */
+	case BCMA_CHIP_ID_BCM47162:
+		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL2;
+		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
+		break;
+	/* bcm4707_family */
+	case BCMA_CHIP_ID_BCM4707:
+	case BCMA_CHIP_ID_BCM47094:
+	case BCMA_CHIP_ID_BCM53018:
+		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
+		bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
+		bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500;
+		break;
+	default:
+		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
+		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
+	}
+
+	if (!bgmac_is_bcm4707_family(bgmac) && core->id.rev > 2)
+		bgmac->feature_flags |= BGMAC_FEAT_MISC_PLL_REQ;
+
+	if (core->id.id == BCMA_CORE_4706_MAC_GBIT) {
+		bgmac->feature_flags |= BGMAC_FEAT_CMN_PHY_CTL;
+		bgmac->feature_flags |= BGMAC_FEAT_NO_CLR_MIB;
+	}
+
+	if (core->id.rev >= 4) {
+		bgmac->feature_flags |= BGMAC_FEAT_CMDCFG_SR_REV4;
+		bgmac->feature_flags |= BGMAC_FEAT_TX_MASK_SETUP;
+		bgmac->feature_flags |= BGMAC_FEAT_RX_MASK_SETUP;
+	}
+
 	netif_napi_add(net_dev, &bgmac->napi, bgmac_poll, BGMAC_WEIGHT);
 
 	if (!bgmac_is_bcm4707_family(bgmac)) {
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 191e64a..89dfee1 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -190,7 +190,6 @@
 #define  BGMAC_CMDCFG_HD_SHIFT			10
 #define  BGMAC_CMDCFG_SR_REV0			0x00000800	/* Set to reset mode, for core rev 0-3 */
 #define  BGMAC_CMDCFG_SR_REV4			0x00002000	/* Set to reset mode, for core rev >= 4 */
-#define  BGMAC_CMDCFG_SR(rev)  ((rev >= 4) ? BGMAC_CMDCFG_SR_REV4 : BGMAC_CMDCFG_SR_REV0)
 #define  BGMAC_CMDCFG_ML			0x00008000	/* Set to activate mac loopback mode */
 #define  BGMAC_CMDCFG_AE			0x00400000
 #define  BGMAC_CMDCFG_CFE			0x00800000
@@ -376,6 +375,24 @@
 
 #define ETHER_MAX_LEN   1518
 
+/* Feature Flags */
+#define BGMAC_FEAT_TX_MASK_SETUP	BIT(0)
+#define BGMAC_FEAT_RX_MASK_SETUP	BIT(1)
+#define BGMAC_FEAT_IOST_ATTACHED	BIT(2)
+#define BGMAC_FEAT_NO_RESET		BIT(3)
+#define BGMAC_FEAT_MISC_PLL_REQ		BIT(4)
+#define BGMAC_FEAT_SW_TYPE_PHY		BIT(5)
+#define BGMAC_FEAT_SW_TYPE_EPHYRMII	BIT(6)
+#define BGMAC_FEAT_SW_TYPE_RGMII	BIT(7)
+#define BGMAC_FEAT_CMN_PHY_CTL		BIT(8)
+#define BGMAC_FEAT_FLW_CTRL1		BIT(9)
+#define BGMAC_FEAT_FLW_CTRL2		BIT(10)
+#define BGMAC_FEAT_SET_RXQ_CLK		BIT(11)
+#define BGMAC_FEAT_CLKCTLST		BIT(12)
+#define BGMAC_FEAT_NO_CLR_MIB		BIT(13)
+#define BGMAC_FEAT_FORCE_SPEED_2500	BIT(14)
+#define BGMAC_FEAT_CMDCFG_SR_REV4	BIT(15)
+
 struct bgmac_slot_info {
 	union {
 		struct sk_buff *skb;
@@ -430,6 +447,8 @@ struct bgmac {
 
 	struct device *dev;
 	struct device *dma_dev;
+	u32 feature_flags;
+
 	struct net_device *net_dev;
 	struct napi_struct napi;
 	struct mii_bus *mii_bus;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC 5/7] net: ethernet: bgmac: Add platform device support
  2016-06-28 19:34 [RFC 0/7] net: ethernet: bgmac: Add platform device support Jon Mason
                   ` (3 preceding siblings ...)
  2016-06-28 19:34 ` [RFC 4/7] net: ethernet: bgmac: convert to feature flags Jon Mason
@ 2016-06-28 19:34 ` Jon Mason
  2016-06-29 18:51   ` Florian Fainelli
  2016-06-30 17:58   ` Ray Jui
  2016-06-28 19:34 ` [RFC 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac Jon Mason
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 25+ messages in thread
From: Jon Mason @ 2016-06-28 19:34 UTC (permalink / raw)
  To: zajec5
  Cc: davem, f.fainelli, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

The bcma portion of the driver has been split off into a bcma specific
driver.  This has been mirrored for the platform driver.  The last
references to the bcma core struct have been changed into a generic
function call.  These function calls are wrappers to either the original
bcma code or new platform functions that access the same areas via MMIO.
This necessitated adding function pointers for both platform and bcma to
hide which backend is being used from the generic bgmac code.

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/ethernet/broadcom/Kconfig          |  23 +-
 drivers/net/ethernet/broadcom/Makefile         |   4 +-
 drivers/net/ethernet/broadcom/bgmac-bcma.c     | 315 ++++++++++++++++++++++++
 drivers/net/ethernet/broadcom/bgmac-platform.c | 208 ++++++++++++++++
 drivers/net/ethernet/broadcom/bgmac.c          | 327 ++++---------------------
 drivers/net/ethernet/broadcom/bgmac.h          |  73 +++++-
 6 files changed, 666 insertions(+), 284 deletions(-)
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma.c
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-platform.c

diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index d74a92e..bd8c80c 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -140,10 +140,18 @@ config BNX2X_SRIOV
 	  allows for virtual function acceleration in virtual environments.
 
 config BGMAC
-	tristate "BCMA bus GBit core support"
+	tristate
+	help
+	  This enables the integrated ethernet controller support for many
+	  Broadcom (mostly iProc) SoCs. An appropriate bus interface driver
+	  needs to be enabled to select this.
+
+config BGMAC_BCMA
+	tristate "Broadcom iProc GBit BCMA support"
 	depends on BCMA && BCMA_HOST_SOC
 	depends on HAS_DMA
 	depends on BCM47XX || ARCH_BCM_5301X || COMPILE_TEST
+	select BGMAC
 	select PHYLIB
 	select FIXED_PHY
 	---help---
@@ -152,6 +160,19 @@ config BGMAC
 	  In case of using this driver on BCM4706 it's also requires to enable
 	  BCMA_DRIVER_GMAC_CMN to make it work.
 
+config BGMAC_PLATFORM
+	tristate "Broadcom iProc GBit platform support"
+	depends on HAS_DMA
+	depends on ARCH_BCM_IPROC || COMPILE_TEST
+	depends on OF
+	select BGMAC
+	select PHYLIB
+	select FIXED_PHY
+	default ARCH_BCM_IPROC
+	---help---
+	  Say Y here if you want to use the Broadcom iProc Gigabit Ethernet
+	  controller through the generic platform interface
+
 config SYSTEMPORT
 	tristate "Broadcom SYSTEMPORT internal MAC support"
 	depends on OF
diff --git a/drivers/net/ethernet/broadcom/Makefile b/drivers/net/ethernet/broadcom/Makefile
index f559794..79f2372 100644
--- a/drivers/net/ethernet/broadcom/Makefile
+++ b/drivers/net/ethernet/broadcom/Makefile
@@ -10,6 +10,8 @@ obj-$(CONFIG_CNIC) += cnic.o
 obj-$(CONFIG_BNX2X) += bnx2x/
 obj-$(CONFIG_SB1250_MAC) += sb1250-mac.o
 obj-$(CONFIG_TIGON3) += tg3.o
-obj-$(CONFIG_BGMAC) += bgmac.o bgmac-bcma-mdio.o
+obj-$(CONFIG_BGMAC) += bgmac.o
+obj-$(CONFIG_BGMAC_BCMA) += bgmac-bcma.o bgmac-bcma-mdio.o
+obj-$(CONFIG_BGMAC_PLATFORM) += bgmac-platform.o
 obj-$(CONFIG_SYSTEMPORT) += bcmsysport.o
 obj-$(CONFIG_BNXT) += bnxt/
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c b/drivers/net/ethernet/broadcom/bgmac-bcma.c
new file mode 100644
index 0000000..9a9745c4
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -0,0 +1,315 @@
+/*
+ * Driver for (BCM4706)? GBit MAC core on BCMA bus.
+ *
+ * Copyright (C) 2012 Rafał Miłecki <zajec5@gmail.com>
+ *
+ * Licensed under the GNU/GPL. See COPYING for details.
+ */
+
+#define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
+
+#include <linux/bcma/bcma.h>
+#include <linux/brcmphy.h>
+#include <linux/etherdevice.h>
+#include "bgmac.h"
+
+static inline bool bgmac_is_bcm4707_family(struct bcma_device *core)
+{
+	switch (core->bus->chipinfo.id) {
+	case BCMA_CHIP_ID_BCM4707:
+	case BCMA_CHIP_ID_BCM47094:
+	case BCMA_CHIP_ID_BCM53018:
+		return true;
+	default:
+		return false;
+	}
+}
+
+/**************************************************
+ * BCMA bus ops
+ **************************************************/
+
+static u32 bcma_bgmac_read(struct bgmac *bgmac, u16 offset)
+{
+	return bcma_read32(bgmac->bcma.core, offset);
+}
+
+static void bcma_bgmac_write(struct bgmac *bgmac, u16 offset, u32 value)
+{
+	bcma_write32(bgmac->bcma.core, offset, value);
+}
+
+static u32 bcma_bgmac_idm_read(struct bgmac *bgmac, u16 offset)
+{
+	return bcma_aread32(bgmac->bcma.core, offset);
+}
+
+static void bcma_bgmac_idm_write(struct bgmac *bgmac, u16 offset, u32 value)
+{
+	return bcma_awrite32(bgmac->bcma.core, offset, value);
+}
+
+static bool bcma_bgmac_clk_enabled(struct bgmac *bgmac)
+{
+	return bcma_core_is_enabled(bgmac->bcma.core);
+}
+
+static void bcma_bgmac_clk_enable(struct bgmac *bgmac, u32 flags)
+{
+	bcma_core_enable(bgmac->bcma.core, flags);
+}
+
+static void bcma_bgmac_cco_ctl_maskset(struct bgmac *bgmac, u32 offset,
+				       u32 mask, u32 set)
+{
+	struct bcma_drv_cc *cc = &bgmac->bcma.core->bus->drv_cc;
+
+	bcma_chipco_chipctl_maskset(cc, offset, mask, set);
+}
+
+static u32 bcma_bgmac_get_bus_clock(struct bgmac *bgmac)
+{
+	struct bcma_drv_cc *cc = &bgmac->bcma.core->bus->drv_cc;
+
+	return bcma_pmu_get_bus_clock(cc);
+}
+
+static void bcma_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset, u32 mask,
+				     u32 set)
+{
+	bcma_maskset32(bgmac->bcma.cmn, offset, mask, set);
+}
+
+static const struct bcma_device_id bgmac_bcma_tbl[] = {
+	BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_4706_MAC_GBIT,
+		  BCMA_ANY_REV, BCMA_ANY_CLASS),
+	BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_MAC_GBIT, BCMA_ANY_REV,
+		  BCMA_ANY_CLASS),
+	{},
+};
+MODULE_DEVICE_TABLE(bcma, bgmac_bcma_tbl);
+
+/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipattach */
+static int bgmac_probe(struct bcma_device *core)
+{
+	struct ssb_sprom *sprom = &core->bus->sprom;
+	struct mii_bus *mii_bus;
+	struct bgmac *bgmac;
+	u8 *mac;
+	int err;
+
+	bgmac = kzalloc(sizeof(*bgmac), GFP_KERNEL);
+	if (!bgmac)
+		return -ENOMEM;
+
+	bgmac->bcma.core = core;
+	bgmac->dev = &core->dev;
+	bgmac->dma_dev = core->dma_dev;
+	bgmac->irq = core->irq;
+
+	bcma_set_drvdata(core, bgmac);
+
+	switch (core->core_unit) {
+	case 0:
+		mac = sprom->et0mac;
+		break;
+	case 1:
+		mac = sprom->et1mac;
+		break;
+	case 2:
+		mac = sprom->et2mac;
+		break;
+	default:
+		dev_err(bgmac->dev, "Unsupported core_unit %d\n",
+			core->core_unit);
+		err = -ENOTSUPP;
+		goto err;
+	}
+
+	ether_addr_copy(bgmac->mac_addr, mac);
+
+	/* On BCM4706 we need common core to access PHY */
+	if (core->id.id == BCMA_CORE_4706_MAC_GBIT &&
+	    !core->bus->drv_gmac_cmn.core) {
+		dev_err(bgmac->dev, "GMAC CMN core not found (required for BCM4706)\n");
+		err = -ENODEV;
+		goto err;
+	}
+	bgmac->bcma.cmn = core->bus->drv_gmac_cmn.core;
+
+	switch (core->core_unit) {
+	case 0:
+		bgmac->phyaddr = sprom->et0phyaddr;
+		break;
+	case 1:
+		bgmac->phyaddr = sprom->et1phyaddr;
+		break;
+	case 2:
+		bgmac->phyaddr = sprom->et2phyaddr;
+		break;
+	}
+	bgmac->phyaddr &= BGMAC_PHY_MASK;
+	if (bgmac->phyaddr == BGMAC_PHY_MASK) {
+		dev_err(bgmac->dev, "No PHY found\n");
+		err = -ENODEV;
+		goto err;
+	}
+	dev_info(bgmac->dev, "Found PHY addr: %d%s\n", bgmac->phyaddr,
+		 bgmac->phyaddr == BGMAC_PHY_NOREGS ? " (NOREGS)" : "");
+
+	if (!bgmac_is_bcm4707_family(core)) {
+		mii_bus = bcma_mdio_mii_register(core, bgmac->phyaddr);
+		if (!IS_ERR(mii_bus)) {
+			err = PTR_ERR(mii_bus);
+			goto err;
+		}
+
+		bgmac->mii_bus = mii_bus;
+	}
+
+	if (core->bus->hosttype == BCMA_HOSTTYPE_PCI) {
+		dev_err(bgmac->dev, "PCI setup not implemented\n");
+		err = -ENOTSUPP;
+		goto err1;
+	}
+
+	bgmac->has_robosw = !!(core->bus->sprom.boardflags_lo &
+			       BGMAC_BFL_ENETROBO);
+	if (bgmac->has_robosw)
+		dev_warn(bgmac->dev, "Support for Roboswitch not implemented\n");
+
+	if (core->bus->sprom.boardflags_lo & BGMAC_BFL_ENETADM)
+		dev_warn(bgmac->dev, "Support for ADMtek ethernet switch not implemented\n");
+
+	/* Feature Flags */
+	switch (core->bus->chipinfo.id) {
+	case BCMA_CHIP_ID_BCM5357:
+		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
+		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
+		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
+		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
+		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM47186) {
+			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
+			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
+		}
+		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM5358)
+			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_EPHYRMII;
+		break;
+	case BCMA_CHIP_ID_BCM53572:
+		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
+		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
+		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
+		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
+		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM47188) {
+			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
+			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
+		}
+		break;
+	case BCMA_CHIP_ID_BCM4749:
+		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
+		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
+		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
+		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
+		if (core->bus->chipinfo.pkg == 10) {
+			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
+			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
+		}
+		break;
+	case BCMA_CHIP_ID_BCM4716:
+		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
+		/* fallthrough */
+	case BCMA_CHIP_ID_BCM47162:
+		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL2;
+		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
+		break;
+	/* bcm4707_family */
+	case BCMA_CHIP_ID_BCM4707:
+	case BCMA_CHIP_ID_BCM47094:
+	case BCMA_CHIP_ID_BCM53018:
+		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
+		bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
+		bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500;
+		break;
+	default:
+		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
+		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
+	}
+
+	if (!bgmac_is_bcm4707_family(core) && core->id.rev > 2)
+		bgmac->feature_flags |= BGMAC_FEAT_MISC_PLL_REQ;
+
+	if (core->id.id == BCMA_CORE_4706_MAC_GBIT) {
+		bgmac->feature_flags |= BGMAC_FEAT_CMN_PHY_CTL;
+		bgmac->feature_flags |= BGMAC_FEAT_NO_CLR_MIB;
+	}
+
+	if (core->id.rev >= 4) {
+		bgmac->feature_flags |= BGMAC_FEAT_CMDCFG_SR_REV4;
+		bgmac->feature_flags |= BGMAC_FEAT_TX_MASK_SETUP;
+		bgmac->feature_flags |= BGMAC_FEAT_RX_MASK_SETUP;
+	}
+
+	bgmac->read = bcma_bgmac_read;
+	bgmac->write = bcma_bgmac_write;
+	bgmac->idm_read = bcma_bgmac_idm_read;
+	bgmac->idm_write = bcma_bgmac_idm_write;
+	bgmac->clk_enabled = bcma_bgmac_clk_enabled;
+	bgmac->clk_enable = bcma_bgmac_clk_enable;
+	bgmac->cco_ctl_maskset = bcma_bgmac_cco_ctl_maskset;
+	bgmac->get_bus_clock = bcma_bgmac_get_bus_clock;
+	bgmac->cmn_maskset32 = bcma_bgmac_cmn_maskset32;
+
+	err = bgmac_enet_probe(bgmac);
+	if (err)
+		goto err1;
+
+	return 0;
+
+err1:
+	bcma_mdio_mii_unregister(bgmac->mii_bus);
+err:
+	kfree(bgmac);
+	bcma_set_drvdata(core, NULL);
+
+	return err;
+}
+
+static void bgmac_remove(struct bcma_device *core)
+{
+	struct bgmac *bgmac = bcma_get_drvdata(core);
+
+	bcma_mdio_mii_unregister(bgmac->mii_bus);
+	bgmac_enet_remove(bgmac);
+	bcma_set_drvdata(core, NULL);
+	kfree(bgmac);
+}
+
+static struct bcma_driver bgmac_bcma_driver = {
+	.name		= KBUILD_MODNAME,
+	.id_table	= bgmac_bcma_tbl,
+	.probe		= bgmac_probe,
+	.remove		= bgmac_remove,
+};
+
+static int __init bgmac_init(void)
+{
+	int err;
+
+	err = bcma_driver_register(&bgmac_bcma_driver);
+	if (err)
+		return err;
+	pr_info("Broadcom 47xx GBit MAC driver loaded\n");
+
+	return 0;
+}
+
+static void __exit bgmac_exit(void)
+{
+	bcma_driver_unregister(&bgmac_bcma_driver);
+}
+
+module_init(bgmac_init)
+module_exit(bgmac_exit)
+
+MODULE_AUTHOR("Rafał Miłecki");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
new file mode 100644
index 0000000..fac93c0
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
@@ -0,0 +1,208 @@
+/*
+ * Copyright (C) 2016 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
+
+#include <linux/bcma/bcma.h>
+#include <linux/etherdevice.h>
+#include <linux/of_address.h>
+#include <linux/of_net.h>
+#include "bgmac.h"
+
+static u32 platform_bgmac_read(struct bgmac *bgmac, u16 offset)
+{
+	return readl(bgmac->plat.base + offset);
+}
+
+static void platform_bgmac_write(struct bgmac *bgmac, u16 offset, u32 value)
+{
+	writel(value, bgmac->plat.base + offset);
+}
+
+static u32 platform_bgmac_idm_read(struct bgmac *bgmac, u16 offset)
+{
+	return readl(bgmac->plat.idm_base + offset);
+}
+
+static void platform_bgmac_idm_write(struct bgmac *bgmac, u16 offset, u32 value)
+{
+	return writel(value, bgmac->plat.idm_base + offset);
+}
+
+static bool platform_bgmac_clk_enabled(struct bgmac *bgmac)
+{
+	if ((bgmac_idm_read(bgmac, BCMA_IOCTL) &
+	     (BCMA_IOCTL_CLK | BCMA_IOCTL_FGC)) != BCMA_IOCTL_CLK)
+		return false;
+	if (bgmac_idm_read(bgmac, BCMA_RESET_CTL) & BCMA_RESET_CTL_RESET)
+		return false;
+	return true;
+}
+
+static void platform_bgmac_clk_enable(struct bgmac *bgmac, u32 flags)
+{
+	bgmac_idm_write(bgmac, BCMA_IOCTL,
+			(BCMA_IOCTL_CLK | BCMA_IOCTL_FGC | flags));
+	bgmac_idm_read(bgmac, BCMA_IOCTL);
+
+	bgmac_idm_write(bgmac, BCMA_RESET_CTL, 0);
+	bgmac_idm_read(bgmac, BCMA_RESET_CTL);
+	udelay(1);
+
+	bgmac_idm_write(bgmac, BCMA_IOCTL, (BCMA_IOCTL_CLK | flags));
+	bgmac_idm_read(bgmac, BCMA_IOCTL);
+	udelay(1);
+}
+
+static void platform_bgmac_cco_ctl_maskset(struct bgmac *bgmac, u32 offset,
+					   u32 mask, u32 set)
+{
+	/* This shouldn't be encountered */
+	WARN_ON(1);
+}
+
+static u32 platform_bgmac_get_bus_clock(struct bgmac *bgmac)
+{
+	/* This shouldn't be encountered */
+	WARN_ON(1);
+
+	return 0;
+}
+
+static void platform_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset,
+					 u32 mask, u32 set)
+{
+	/* This shouldn't be encountered */
+	WARN_ON(1);
+}
+
+static int bgmac_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct bgmac *bgmac;
+	struct resource regs;
+	const u8 *mac_addr;
+	int rc;
+
+	bgmac = kzalloc(sizeof(*bgmac), GFP_KERNEL);
+	if (!bgmac)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, bgmac);
+
+	/* Set the features of the 4707 family */
+	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
+	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
+	bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500;
+	bgmac->feature_flags |= BGMAC_FEAT_CMDCFG_SR_REV4;
+	bgmac->feature_flags |= BGMAC_FEAT_TX_MASK_SETUP;
+	bgmac->feature_flags |= BGMAC_FEAT_RX_MASK_SETUP;
+
+	bgmac->dev = &pdev->dev;
+	bgmac->dma_dev = &pdev->dev;
+
+	mac_addr = of_get_mac_address(np);
+	if (mac_addr)
+		ether_addr_copy(bgmac->mac_addr, mac_addr);
+	else
+		dev_warn(&pdev->dev, "MAC address not present in device tree\n");
+
+	bgmac->irq = platform_get_irq(pdev, 0);
+	if (bgmac->irq < 0) {
+		rc = bgmac->irq;
+		dev_err(&pdev->dev, "Unable to obtain IRQ\n");
+		goto err;
+	}
+
+	rc = of_address_to_resource(np, 0, &regs);
+	if (rc < 0) {
+		dev_err(&pdev->dev, "Unable to obtain base resource\n");
+		goto err;
+	}
+
+	bgmac->plat.base = ioremap(regs.start, resource_size(&regs));
+	if (!bgmac->plat.base) {
+		dev_err(&pdev->dev, "Unable to map base resource\n");
+		rc = -ENOMEM;
+		goto err;
+	}
+
+	rc = of_address_to_resource(np, 1, &regs);
+	if (rc < 0) {
+		dev_err(&pdev->dev, "Unable to obtain idm resource\n");
+		goto err1;
+	}
+
+	bgmac->plat.idm_base = ioremap(regs.start, resource_size(&regs));
+	if (!bgmac->plat.base) {
+		dev_err(&pdev->dev, "Unable to map idm resource\n");
+		rc = -ENOMEM;
+		goto err1;
+	}
+
+	bgmac->read = platform_bgmac_read;
+	bgmac->write = platform_bgmac_write;
+	bgmac->idm_read = platform_bgmac_idm_read;
+	bgmac->idm_write = platform_bgmac_idm_write;
+	bgmac->clk_enabled = platform_bgmac_clk_enabled;
+	bgmac->clk_enable = platform_bgmac_clk_enable;
+	bgmac->cco_ctl_maskset = platform_bgmac_cco_ctl_maskset;
+	bgmac->get_bus_clock = platform_bgmac_get_bus_clock;
+	bgmac->cmn_maskset32 = platform_bgmac_cmn_maskset32;
+
+	rc = bgmac_enet_probe(bgmac);
+	if (rc)
+		goto err2;
+
+	return 0;
+
+err2:
+	iounmap(bgmac->plat.idm_base);
+err1:
+	iounmap(bgmac->plat.base);
+err:
+	kfree(bgmac);
+
+	return rc;
+}
+
+static int bgmac_remove(struct platform_device *pdev)
+{
+	struct bgmac *bgmac = platform_get_drvdata(pdev);
+
+	bgmac_enet_remove(bgmac);
+	iounmap(bgmac->plat.idm_base);
+	iounmap(bgmac->plat.base);
+	kfree(bgmac);
+
+	return 0;
+}
+
+static const struct of_device_id bgmac_of_enet_match[] = {
+	{.compatible = "brcm,bgmac-enet",},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, bgmac_of_enet_match);
+
+static struct platform_driver bgmac_enet_driver = {
+	.driver = {
+		.name  = "bgmac-enet",
+		.of_match_table = bgmac_of_enet_match,
+	},
+	.probe = bgmac_probe,
+	.remove = bgmac_remove,
+};
+
+module_platform_driver(bgmac_enet_driver);
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 4346994..bba9749 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -6,51 +6,27 @@
  * Licensed under the GNU/GPL. See COPYING for details.
  */
 
-#include "bgmac.h"
 
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/delay.h>
+#define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
+
+#include <linux/bcma/bcma.h>
 #include <linux/etherdevice.h>
-#include <linux/mii.h>
-#include <linux/phy.h>
-#include <linux/phy_fixed.h>
-#include <linux/interrupt.h>
-#include <linux/dma-mapping.h>
 #include <linux/bcm47xx_nvram.h>
+#include "bgmac.h"
 
-static const struct bcma_device_id bgmac_bcma_tbl[] = {
-	BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_4706_MAC_GBIT, BCMA_ANY_REV, BCMA_ANY_CLASS),
-	BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_MAC_GBIT, BCMA_ANY_REV, BCMA_ANY_CLASS),
-	{},
-};
-MODULE_DEVICE_TABLE(bcma, bgmac_bcma_tbl);
-
-static inline bool bgmac_is_bcm4707_family(struct bgmac *bgmac)
-{
-	switch (bgmac->core->bus->chipinfo.id) {
-	case BCMA_CHIP_ID_BCM4707:
-	case BCMA_CHIP_ID_BCM47094:
-	case BCMA_CHIP_ID_BCM53018:
-		return true;
-	default:
-		return false;
-	}
-}
-
-static bool bgmac_wait_value(struct bcma_device *core, u16 reg, u32 mask,
+static bool bgmac_wait_value(struct bgmac *bgmac, u16 reg, u32 mask,
 			     u32 value, int timeout)
 {
 	u32 val;
 	int i;
 
 	for (i = 0; i < timeout / 10; i++) {
-		val = bcma_read32(core, reg);
+		val = bgmac_read(bgmac, reg);
 		if ((val & mask) == value)
 			return true;
 		udelay(10);
 	}
-	dev_err(&core->dev, "Timeout waiting for reg 0x%X\n", reg);
+	dev_err(bgmac->dev, "Timeout waiting for reg 0x%X\n", reg);
 	return false;
 }
 
@@ -89,7 +65,7 @@ static void bgmac_dma_tx_reset(struct bgmac *bgmac, struct bgmac_dma_ring *ring)
 
 	/* Remove SUSPEND bit */
 	bgmac_write(bgmac, ring->mmio_base + BGMAC_DMA_TX_CTL, 0);
-	if (!bgmac_wait_value(bgmac->core,
+	if (!bgmac_wait_value(bgmac,
 			      ring->mmio_base + BGMAC_DMA_TX_STATUS,
 			      BGMAC_DMA_TX_STAT, BGMAC_DMA_TX_STAT_DISABLED,
 			      10000)) {
@@ -316,7 +292,7 @@ static void bgmac_dma_rx_reset(struct bgmac *bgmac, struct bgmac_dma_ring *ring)
 		return;
 
 	bgmac_write(bgmac, ring->mmio_base + BGMAC_DMA_RX_CTL, 0);
-	if (!bgmac_wait_value(bgmac->core,
+	if (!bgmac_wait_value(bgmac,
 			      ring->mmio_base + BGMAC_DMA_RX_STATUS,
 			      BGMAC_DMA_RX_STAT, BGMAC_DMA_RX_STAT_DISABLED,
 			      10000))
@@ -639,7 +615,7 @@ static int bgmac_dma_alloc(struct bgmac *bgmac)
 	BUILD_BUG_ON(BGMAC_MAX_TX_RINGS > ARRAY_SIZE(ring_base));
 	BUILD_BUG_ON(BGMAC_MAX_RX_RINGS > ARRAY_SIZE(ring_base));
 
-	if (!(bcma_aread32(bgmac->core, BCMA_IOST) & BCMA_IOST_DMA64)) {
+	if (!(bgmac_idm_read(bgmac, BCMA_IOST) & BCMA_IOST_DMA64)) {
 		dev_err(bgmac->dev, "Core does not report 64-bit DMA\n");
 		return -ENOTSUPP;
 	}
@@ -871,12 +847,10 @@ static void bgmac_mac_speed(struct bgmac *bgmac)
 
 static void bgmac_miiconfig(struct bgmac *bgmac)
 {
-	struct bcma_device *core = bgmac->core;
-
 	if (bgmac->feature_flags & BGMAC_FEAT_FORCE_SPEED_2500) {
-		bcma_awrite32(core, BCMA_IOCTL,
-			      bcma_aread32(core, BCMA_IOCTL) | 0x40 |
-			      BGMAC_BCMA_IOCTL_SW_CLKEN);
+		bgmac_idm_write(bgmac, BCMA_IOCTL,
+				bgmac_idm_read(bgmac, BCMA_IOCTL) | 0x40 |
+				BGMAC_BCMA_IOCTL_SW_CLKEN);
 		bgmac->mac_speed = SPEED_2500;
 		bgmac->mac_duplex = DUPLEX_FULL;
 		bgmac_mac_speed(bgmac);
@@ -896,12 +870,11 @@ static void bgmac_miiconfig(struct bgmac *bgmac)
 /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipreset */
 static void bgmac_chip_reset(struct bgmac *bgmac)
 {
-	struct bcma_device *core = bgmac->core;
 	u32 cmdcfg_sr;
 	u32 iost;
 	int i;
 
-	if (bcma_core_is_enabled(core)) {
+	if (bgmac_clk_enabled(bgmac)) {
 		if (!bgmac->stats_grabbed) {
 			/* bgmac_chip_stats_update(bgmac); */
 			bgmac->stats_grabbed = true;
@@ -919,7 +892,7 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 		/* TODO: Clear software multicast filter list */
 	}
 
-	iost = bcma_aread32(core, BCMA_IOST);
+	iost = bgmac_idm_read(bgmac, BCMA_IOST);
 	if (bgmac->feature_flags & BGMAC_FEAT_IOST_ATTACHED)
 		iost &= ~BGMAC_BCMA_IOST_ATTACHED;
 
@@ -931,21 +904,20 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 			if (!bgmac->has_robosw)
 				flags |= BGMAC_BCMA_IOCTL_SW_RESET;
 		}
-		bcma_core_enable(core, flags);
+		bgmac_clk_enable(bgmac, flags);
 	}
 
 	/* Request Misc PLL for corerev > 2 */
 	if (bgmac->feature_flags & BGMAC_FEAT_MISC_PLL_REQ) {
 		bgmac_set(bgmac, BCMA_CLKCTLST,
 			  BGMAC_BCMA_CLKCTLST_MISC_PLL_REQ);
-		bgmac_wait_value(bgmac->core, BCMA_CLKCTLST,
+		bgmac_wait_value(bgmac, BCMA_CLKCTLST,
 				 BGMAC_BCMA_CLKCTLST_MISC_PLL_ST,
 				 BGMAC_BCMA_CLKCTLST_MISC_PLL_ST,
 				 1000);
 	}
 
 	if (bgmac->feature_flags & BGMAC_FEAT_SW_TYPE_PHY) {
-		struct bcma_drv_cc *cc = &bgmac->core->bus->drv_cc;
 		u8 et_swtype = 0;
 		u8 sw_type = BGMAC_CHIPCTL_1_SW_TYPE_EPHY |
 			     BGMAC_CHIPCTL_1_IF_TYPE_MII;
@@ -964,16 +936,15 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 			sw_type = BGMAC_CHIPCTL_1_IF_TYPE_RGMII |
 				  BGMAC_CHIPCTL_1_SW_TYPE_RGMII;
 		}
-		bcma_chipco_chipctl_maskset(cc, 1,
-					    ~(BGMAC_CHIPCTL_1_IF_TYPE_MASK |
-					      BGMAC_CHIPCTL_1_SW_TYPE_MASK),
-					    sw_type);
+		bgmac_cco_ctl_maskset(bgmac, 1, ~(BGMAC_CHIPCTL_1_IF_TYPE_MASK |
+						  BGMAC_CHIPCTL_1_SW_TYPE_MASK),
+				      sw_type);
 	}
 
 	if (iost & BGMAC_BCMA_IOST_ATTACHED && !bgmac->has_robosw)
-		bcma_awrite32(core, BCMA_IOCTL,
-			      bcma_aread32(core, BCMA_IOCTL) &
-			      ~BGMAC_BCMA_IOCTL_SW_RESET);
+		bgmac_idm_write(bgmac, BCMA_IOCTL,
+				bgmac_idm_read(bgmac, BCMA_IOCTL) &
+				~BGMAC_BCMA_IOCTL_SW_RESET);
 
 	/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/gmac_reset
 	 * Specs don't say about using BGMAC_CMDCFG_SR, but in this routine
@@ -1009,8 +980,8 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 
 	bgmac_clear_mib(bgmac);
 	if (bgmac->feature_flags & BGMAC_FEAT_CMN_PHY_CTL)
-		bcma_maskset32(bgmac->cmn, BCMA_GMAC_CMN_PHY_CTL, ~0,
-			       BCMA_GMAC_CMN_PC_MTE);
+		bgmac_cmn_maskset32(bgmac, BCMA_GMAC_CMN_PHY_CTL, ~0,
+				    BCMA_GMAC_CMN_PC_MTE);
 	else
 		bgmac_set(bgmac, BGMAC_PHY_CNTL, BGMAC_PC_MTE);
 	bgmac_miiconfig(bgmac);
@@ -1055,8 +1026,8 @@ static void bgmac_enable(struct bgmac *bgmac)
 	if (bgmac->feature_flags & BGMAC_FEAT_CLKCTLST || mode != 0)
 		bgmac_set(bgmac, BCMA_CLKCTLST, BCMA_CLKCTLST_FORCEHT);
 	if (bgmac->feature_flags & BGMAC_FEAT_CLKCTLST && mode == 2)
-		bcma_chipco_chipctl_maskset(&bgmac->core->bus->drv_cc, 1, ~0,
-					    BGMAC_CHIPCTL_1_RXC_DLL_BYPASS);
+		bgmac_cco_ctl_maskset(bgmac, 1, ~0,
+				      BGMAC_CHIPCTL_1_RXC_DLL_BYPASS);
 
 	if (bgmac->feature_flags & (BGMAC_FEAT_FLW_CTRL1 |
 				    BGMAC_FEAT_FLW_CTRL2)) {
@@ -1078,8 +1049,7 @@ static void bgmac_enable(struct bgmac *bgmac)
 
 		rxq_ctl = bgmac_read(bgmac, BGMAC_RXQ_CTL);
 		rxq_ctl &= ~BGMAC_RXQ_CTL_MDP_MASK;
-		bp_clk = bcma_pmu_get_bus_clock(&bgmac->core->bus->drv_cc) /
-				1000000;
+		bp_clk = bgmac_get_bus_clock(bgmac) / 1000000;
 		mdp = (bp_clk * 128 / 1000) - 3;
 		rxq_ctl |= (mdp << BGMAC_RXQ_CTL_MDP_SHIFT);
 		bgmac_write(bgmac, BGMAC_RXQ_CTL, rxq_ctl);
@@ -1174,7 +1144,7 @@ static int bgmac_open(struct net_device *net_dev)
 	/* Specs say about reclaiming rings here, but we do that in DMA init */
 	bgmac_chip_init(bgmac);
 
-	err = request_irq(bgmac->core->irq, bgmac_interrupt, IRQF_SHARED,
+	err = request_irq(bgmac->irq, bgmac_interrupt, IRQF_SHARED,
 			  KBUILD_MODNAME, net_dev);
 	if (err < 0) {
 		dev_err(bgmac->dev, "IRQ request error: %d!\n", err);
@@ -1199,7 +1169,7 @@ static int bgmac_stop(struct net_device *net_dev)
 
 	napi_disable(&bgmac->napi);
 	bgmac_chip_intrs_off(bgmac);
-	free_irq(bgmac->core->irq, net_dev);
+	free_irq(bgmac->irq, net_dev);
 
 	bgmac_chip_reset(bgmac);
 	bgmac_dma_cleanup(bgmac);
@@ -1462,116 +1432,41 @@ static int bgmac_phy_connect(struct bgmac *bgmac)
 	return 0;
 }
 
-static int bgmac_probe(struct bcma_device *core)
+int bgmac_enet_probe(struct bgmac *info)
 {
 	struct net_device *net_dev;
 	struct bgmac *bgmac;
-	struct ssb_sprom *sprom = &core->bus->sprom;
-	u8 *mac;
 	int err;
 
-	switch (core->core_unit) {
-	case 0:
-		mac = sprom->et0mac;
-		break;
-	case 1:
-		mac = sprom->et1mac;
-		break;
-	case 2:
-		mac = sprom->et2mac;
-		break;
-	default:
-		dev_err(&core->dev, "Unsupported core_unit %d\n",
-			core->core_unit);
-		return -ENOTSUPP;
-	}
-
-	if (!is_valid_ether_addr(mac)) {
-		dev_err(&core->dev, "Invalid MAC addr: %pM\n", mac);
-		eth_random_addr(mac);
-		dev_warn(&core->dev, "Using random MAC: %pM\n", mac);
-	}
-
-	/* This (reset &) enable is not preset in specs or reference driver but
-	 * Broadcom does it in arch PCI code when enabling fake PCI device.
-	 */
-	bcma_core_enable(core, 0);
-
 	/* Allocation and references */
 	net_dev = alloc_etherdev(sizeof(*bgmac));
 	if (!net_dev)
 		return -ENOMEM;
+
 	net_dev->netdev_ops = &bgmac_netdev_ops;
-	net_dev->irq = core->irq;
 	net_dev->ethtool_ops = &bgmac_ethtool_ops;
 	bgmac = netdev_priv(net_dev);
-	bgmac->dev = &core->dev;
-	bgmac->dma_dev = core->dma_dev;
+	memcpy(bgmac, info, sizeof(*bgmac));
 	bgmac->net_dev = net_dev;
-	bgmac->core = core;
-	bcma_set_drvdata(core, bgmac);
-	SET_NETDEV_DEV(net_dev, &core->dev);
-
-	/* Defaults */
-	memcpy(bgmac->net_dev->dev_addr, mac, ETH_ALEN);
-
-	/* On BCM4706 we need common core to access PHY */
-	if (core->id.id == BCMA_CORE_4706_MAC_GBIT &&
-	    !core->bus->drv_gmac_cmn.core) {
-		dev_err(bgmac->dev, "GMAC CMN core not found (required for BCM4706)\n");
-		err = -ENODEV;
-		goto err_netdev_free;
-	}
-	bgmac->cmn = core->bus->drv_gmac_cmn.core;
-
-	switch (core->core_unit) {
-	case 0:
-		bgmac->phyaddr = sprom->et0phyaddr;
-		break;
-	case 1:
-		bgmac->phyaddr = sprom->et1phyaddr;
-		break;
-	case 2:
-		bgmac->phyaddr = sprom->et2phyaddr;
-		break;
-	}
-	bgmac->phyaddr &= BGMAC_PHY_MASK;
-	if (bgmac->phyaddr == BGMAC_PHY_MASK) {
-		dev_err(bgmac->dev, "No PHY found\n");
-		err = -ENODEV;
-		goto err_netdev_free;
+	net_dev->irq = bgmac->irq;
+	SET_NETDEV_DEV(net_dev, bgmac->dev);
+
+	if (!is_valid_ether_addr(bgmac->mac_addr)) {
+		dev_err(bgmac->dev, "Invalid MAC addr: %pM\n",
+			bgmac->mac_addr);
+		eth_random_addr(bgmac->mac_addr);
+		dev_warn(bgmac->dev, "Using random MAC: %pM\n",
+			 bgmac->mac_addr);
 	}
-	dev_info(bgmac->dev, "Found PHY addr: %d%s\n", bgmac->phyaddr,
-		 bgmac->phyaddr == BGMAC_PHY_NOREGS ? " (NOREGS)" : "");
+	ether_addr_copy(net_dev->dev_addr, bgmac->mac_addr);
 
-	if (core->bus->hosttype == BCMA_HOSTTYPE_PCI) {
-		dev_err(bgmac->dev, "PCI setup not implemented\n");
-		err = -ENOTSUPP;
-		goto err_netdev_free;
-	}
+	/* This (reset &) enable is not preset in specs or reference driver but
+	 * Broadcom does it in arch PCI code when enabling fake PCI device.
+	 */
+	bgmac_clk_enable(bgmac, 0);
 
 	bgmac_chip_reset(bgmac);
 
-	/* For Northstar, we have to take all GMAC core out of reset */
-	if (bgmac_is_bcm4707_family(bgmac)) {
-		struct bcma_device *ns_core;
-		int ns_gmac;
-
-		/* Northstar has 4 GMAC cores */
-		for (ns_gmac = 0; ns_gmac < 4; ns_gmac++) {
-			/* As Northstar requirement, we have to reset all GMACs
-			 * before accessing one. bgmac_chip_reset() call
-			 * bcma_core_enable() for this core. Then the other
-			 * three GMACs didn't reset.  We do it here.
-			 */
-			ns_core = bcma_find_core_unit(core->bus,
-						      BCMA_CORE_MAC_GBIT,
-						      ns_gmac);
-			if (ns_core && !bcma_core_is_enabled(ns_core))
-				bcma_core_enable(ns_core, 0);
-		}
-	}
-
 	err = bgmac_dma_alloc(bgmac);
 	if (err) {
 		dev_err(bgmac->dev, "Unable to alloc memory for DMA\n");
@@ -1582,103 +1477,15 @@ static int bgmac_probe(struct bcma_device *core)
 	if (bcm47xx_nvram_getenv("et0_no_txint", NULL, 0) == 0)
 		bgmac->int_mask &= ~BGMAC_IS_TX_MASK;
 
-	bgmac->has_robosw = !!(core->bus->sprom.boardflags_lo &
-			       BGMAC_BFL_ENETROBO);
-	if (bgmac->has_robosw)
-		dev_warn(bgmac->dev, "Support for Roboswitch not implemented\n");
-
-	if (core->bus->sprom.boardflags_lo & BGMAC_BFL_ENETADM)
-		dev_warn(bgmac->dev, "Support for ADMtek ethernet switch not implemented\n");
-
-	/* Feature Flags */
-	switch (core->bus->chipinfo.id) {
-	case BCMA_CHIP_ID_BCM5357:
-		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
-		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
-		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
-		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
-		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM47186) {
-			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
-			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
-		}
-		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM5358)
-			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_EPHYRMII;
-		break;
-	case BCMA_CHIP_ID_BCM53572:
-		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
-		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
-		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
-		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
-		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM47188) {
-			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
-			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
-		}
-		break;
-	case BCMA_CHIP_ID_BCM4749:
-		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
-		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
-		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
-		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
-		if (core->bus->chipinfo.pkg == 10) {
-			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
-			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
-		}
-		break;
-	case BCMA_CHIP_ID_BCM4716:
-		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
-		/* fallthrough */
-	case BCMA_CHIP_ID_BCM47162:
-		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL2;
-		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
-		break;
-	/* bcm4707_family */
-	case BCMA_CHIP_ID_BCM4707:
-	case BCMA_CHIP_ID_BCM47094:
-	case BCMA_CHIP_ID_BCM53018:
-		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
-		bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
-		bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500;
-		break;
-	default:
-		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
-		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
-	}
-
-	if (!bgmac_is_bcm4707_family(bgmac) && core->id.rev > 2)
-		bgmac->feature_flags |= BGMAC_FEAT_MISC_PLL_REQ;
-
-	if (core->id.id == BCMA_CORE_4706_MAC_GBIT) {
-		bgmac->feature_flags |= BGMAC_FEAT_CMN_PHY_CTL;
-		bgmac->feature_flags |= BGMAC_FEAT_NO_CLR_MIB;
-	}
-
-	if (core->id.rev >= 4) {
-		bgmac->feature_flags |= BGMAC_FEAT_CMDCFG_SR_REV4;
-		bgmac->feature_flags |= BGMAC_FEAT_TX_MASK_SETUP;
-		bgmac->feature_flags |= BGMAC_FEAT_RX_MASK_SETUP;
-	}
-
 	netif_napi_add(net_dev, &bgmac->napi, bgmac_poll, BGMAC_WEIGHT);
 
-	if (!bgmac_is_bcm4707_family(bgmac)) {
-		struct mii_bus *mii_bus;
-
-		mii_bus = bcma_mdio_mii_register(core, bgmac->phyaddr);
-		if (!IS_ERR(mii_bus)) {
-			err = PTR_ERR(mii_bus);
-			goto err_dma_free;
-		}
-
-		bgmac->mii_bus = mii_bus;
-	}
-
 	if (!bgmac->mii_bus)
 		err = bgmac_phy_connect_direct(bgmac);
 	else
 		err = bgmac_phy_connect(bgmac);
 	if (err) {
 		dev_err(bgmac->dev, "Cannot connect to phy\n");
-		goto err_mii_unregister;
+		goto err_dma_free;
 	}
 
 	net_dev->features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
@@ -1697,56 +1504,24 @@ static int bgmac_probe(struct bcma_device *core)
 
 err_phy_disconnect:
 	phy_disconnect(net_dev->phydev);
-err_mii_unregister:
-	bcma_mdio_mii_unregister(bgmac->mii_bus);
 err_dma_free:
 	bgmac_dma_free(bgmac);
 err_netdev_free:
-	bcma_set_drvdata(core, NULL);
 	free_netdev(net_dev);
 
 	return err;
 }
+EXPORT_SYMBOL_GPL(bgmac_enet_probe);
 
-static void bgmac_remove(struct bcma_device *core)
+void bgmac_enet_remove(struct bgmac *bgmac)
 {
-	struct bgmac *bgmac = bcma_get_drvdata(core);
-
 	unregister_netdev(bgmac->net_dev);
 	phy_disconnect(bgmac->net_dev->phydev);
-	bcma_mdio_mii_unregister(bgmac->mii_bus);
 	netif_napi_del(&bgmac->napi);
 	bgmac_dma_free(bgmac);
-	bcma_set_drvdata(core, NULL);
 	free_netdev(bgmac->net_dev);
 }
-
-static struct bcma_driver bgmac_bcma_driver = {
-	.name		= KBUILD_MODNAME,
-	.id_table	= bgmac_bcma_tbl,
-	.probe		= bgmac_probe,
-	.remove		= bgmac_remove,
-};
-
-static int __init bgmac_init(void)
-{
-	int err;
-
-	err = bcma_driver_register(&bgmac_bcma_driver);
-	if (err)
-		return err;
-	pr_info("Broadcom 47xx GBit MAC driver loaded\n");
-
-	return 0;
-}
-
-static void __exit bgmac_exit(void)
-{
-	bcma_driver_unregister(&bgmac_bcma_driver);
-}
-
-module_init(bgmac_init)
-module_exit(bgmac_exit)
+EXPORT_SYMBOL_GPL(bgmac_enet_remove);
 
 MODULE_AUTHOR("Rafał Miłecki");
 MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 89dfee1..24a25026 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -1,8 +1,6 @@
 #ifndef _BGMAC_H
 #define _BGMAC_H
 
-#include <linux/bcma/bcma.h>
-#include <linux/brcmphy.h>
 #include <linux/netdevice.h>
 
 #define BGMAC_DEV_CTL				0x000
@@ -442,11 +440,21 @@ struct bgmac_rx_header {
 };
 
 struct bgmac {
-	struct bcma_device *core;
-	struct bcma_device *cmn; /* Reference to CMN core for BCM4706 */
+	union {
+		struct {
+			void *base;
+			void *idm_base;
+		} plat;
+		struct {
+			struct bcma_device *core;
+			/* Reference to CMN core for BCM4706 */
+			struct bcma_device *cmn;
+		} bcma;
+	};
 
 	struct device *dev;
 	struct device *dma_dev;
+	unsigned char mac_addr[ETH_ALEN];
 	u32 feature_flags;
 
 	struct net_device *net_dev;
@@ -463,6 +471,7 @@ struct bgmac {
 	u32 mib_rx_regs[BGMAC_NUM_MIB_RX_REGS];
 
 	/* Int */
+	int irq;
 	u32 int_mask;
 
 	/* Current MAC state */
@@ -473,19 +482,71 @@ struct bgmac {
 	bool has_robosw;
 
 	bool loopback;
+
+	u32 (*read)(struct bgmac *bgmac, u16 offset);
+	void (*write)(struct bgmac *bgmac, u16 offset, u32 value);
+	u32 (*idm_read)(struct bgmac *bgmac, u16 offset);
+	void (*idm_write)(struct bgmac *bgmac, u16 offset, u32 value);
+	bool (*clk_enabled)(struct bgmac *bgmac);
+	void (*clk_enable)(struct bgmac *bgmac, u32 flags);
+	void (*cco_ctl_maskset)(struct bgmac *bgmac, u32 offset, u32 mask,
+				u32 set);
+	u32 (*get_bus_clock)(struct bgmac *bgmac);
+	void (*cmn_maskset32)(struct bgmac *bgmac, u16 offset, u32 mask,
+			      u32 set);
 };
 
+int bgmac_enet_probe(struct bgmac *info);
+void bgmac_enet_remove(struct bgmac *bgmac);
+
 struct mii_bus *bcma_mdio_mii_register(struct bcma_device *core, u8 phyaddr);
 void bcma_mdio_mii_unregister(struct mii_bus *mii_bus);
 
 static inline u32 bgmac_read(struct bgmac *bgmac, u16 offset)
 {
-	return bcma_read32(bgmac->core, offset);
+	return bgmac->read(bgmac, offset);
 }
 
 static inline void bgmac_write(struct bgmac *bgmac, u16 offset, u32 value)
 {
-	bcma_write32(bgmac->core, offset, value);
+	bgmac->write(bgmac, offset, value);
+}
+
+static inline u32 bgmac_idm_read(struct bgmac *bgmac, u16 offset)
+{
+	return bgmac->idm_read(bgmac, offset);
+}
+
+static inline void bgmac_idm_write(struct bgmac *bgmac, u16 offset, u32 value)
+{
+	bgmac->idm_write(bgmac, offset, value);
+}
+
+static inline bool bgmac_clk_enabled(struct bgmac *bgmac)
+{
+	return bgmac->clk_enabled(bgmac);
+}
+
+static inline void bgmac_clk_enable(struct bgmac *bgmac, u32 flags)
+{
+	bgmac->clk_enable(bgmac, flags);
+}
+
+static inline void bgmac_cco_ctl_maskset(struct bgmac *bgmac, u32 offset,
+					 u32 mask, u32 set)
+{
+	bgmac->cco_ctl_maskset(bgmac, offset, mask, set);
+}
+
+static inline u32 bgmac_get_bus_clock(struct bgmac *bgmac)
+{
+	return bgmac->get_bus_clock(bgmac);
+}
+
+static inline void bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset,
+				       u32 mask, u32 set)
+{
+	bgmac->cmn_maskset32(bgmac, offset, mask, set);
 }
 
 static inline void bgmac_maskset(struct bgmac *bgmac, u16 offset, u32 mask,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac
  2016-06-28 19:34 [RFC 0/7] net: ethernet: bgmac: Add platform device support Jon Mason
                   ` (4 preceding siblings ...)
  2016-06-28 19:34 ` [RFC 5/7] net: ethernet: bgmac: Add platform device support Jon Mason
@ 2016-06-28 19:34 ` Jon Mason
  2016-06-28 20:11   ` Sergei Shtylyov
                     ` (2 more replies)
  2016-06-28 19:34 ` [RFC 7/7] ARM: dts: NSP: Add bgmac entries Jon Mason
  2016-06-29 18:52 ` [RFC 0/7] net: ethernet: bgmac: Add platform device support Florian Fainelli
  7 siblings, 3 replies; 25+ messages in thread
From: Jon Mason @ 2016-06-28 19:34 UTC (permalink / raw)
  To: zajec5
  Cc: davem, f.fainelli, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 .../devicetree/bindings/net/brcm,bgmac-enet.txt     | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt

diff --git a/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt b/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
new file mode 100644
index 0000000..efd36d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
@@ -0,0 +1,21 @@
+Broadcom GMAC Ethernet Controller Device Tree Bindings
+-------------------------------------------------------------
+
+Required properties:
+ - compatible:	"brcm,bgmac-enet"
+ - reg:		Address and length of the GMAC registers,
+		Address and length of the GMAC IDM registers
+ - interrupts:	Interrupt number
+
+Optional properties:
+- mac-address:	mac address to be assigned to the device
+
+Examples:
+
+gmac0: enet@18022000 {
+	compatible = "brcm,bgmac-enet";
+	reg = <0x18022000 0x1000>,
+	      <0x18110000 0x1000>;
+	interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
+	status = "disabled";
+};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC 7/7] ARM: dts: NSP: Add bgmac entries
  2016-06-28 19:34 [RFC 0/7] net: ethernet: bgmac: Add platform device support Jon Mason
                   ` (5 preceding siblings ...)
  2016-06-28 19:34 ` [RFC 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac Jon Mason
@ 2016-06-28 19:34 ` Jon Mason
  2016-06-29 18:52 ` [RFC 0/7] net: ethernet: bgmac: Add platform device support Florian Fainelli
  7 siblings, 0 replies; 25+ messages in thread
From: Jon Mason @ 2016-06-28 19:34 UTC (permalink / raw)
  To: zajec5
  Cc: davem, f.fainelli, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

Add device tree entries for the ethernet devices present on the
Broadcom Northstar Plus SoCs

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 arch/arm/boot/dts/bcm-nsp.dtsi   | 16 ++++++++++++++++
 arch/arm/boot/dts/bcm958625k.dts |  8 ++++++++
 2 files changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
index def9e78..9b4e45e 100644
--- a/arch/arm/boot/dts/bcm-nsp.dtsi
+++ b/arch/arm/boot/dts/bcm-nsp.dtsi
@@ -192,6 +192,22 @@
 			status = "disabled";
 		};
 
+		gmac0: enet@22000 {
+			compatible = "brcm,bgmac-enet";
+			reg = <0x022000 0x1000>,
+			      <0x110000 0x1000>;
+			interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
+
+		gmac1: enet@23000 {
+			compatible = "brcm,bgmac-enet";
+			reg = <0x023000 0x1000>,
+			      <0x111000 0x1000>;
+			interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
+
 		nand: nand@26000 {
 			compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1";
 			reg = <0x026000 0x600>,
diff --git a/arch/arm/boot/dts/bcm958625k.dts b/arch/arm/boot/dts/bcm958625k.dts
index e298450..d16ab53 100644
--- a/arch/arm/boot/dts/bcm958625k.dts
+++ b/arch/arm/boot/dts/bcm958625k.dts
@@ -56,6 +56,14 @@
 	status = "okay";
 };
 
+&gmac0 {
+	status = "okay";
+};
+
+&gmac1 {
+	status = "okay";
+};
+
 &pcie0 {
 	status = "okay";
 };
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [RFC 1/7] net: ethernet: bgmac: change bgmac_* prints to dev_* prints
  2016-06-28 19:34 ` [RFC 1/7] net: ethernet: bgmac: change bgmac_* prints to dev_* prints Jon Mason
@ 2016-06-28 19:43   ` Joe Perches
  2016-06-29 20:10     ` Jon Mason
  0 siblings, 1 reply; 25+ messages in thread
From: Joe Perches @ 2016-06-28 19:43 UTC (permalink / raw)
  To: Jon Mason, zajec5
  Cc: davem, f.fainelli, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

On Tue, 2016-06-28 at 15:34 -0400, Jon Mason wrote:
> The bgmac_* print wrappers call dev_* prints with the dev pointer from
> the bcma core.  In anticipation of removing the bcma requirement for
> this driver, these must be changed to not reference that struct.  So,
> simply change all of the bgmac_* prints to their dev_* counterparts.  In
> some cases netdev_* prints are more appropriate, so change those as
> well.

Thanks, but...

> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
[]
> @@ -1515,7 +1516,7 @@ static void bgmac_get_drvinfo(struct net_device *net_dev,
>  			      struct ethtool_drvinfo *info)
>  {
>  	strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
> -	strlcpy(info->bus_info, "BCMA", sizeof(info->bus_info));
> +	strlcpy(info->bus_info, "AXI", sizeof(info->bus_info));
>  }

Unrelated change?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file
  2016-06-28 19:34 ` [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file Jon Mason
@ 2016-06-28 20:02   ` Andrew Lunn
  2016-06-29 14:13   ` Andrew Lunn
  1 sibling, 0 replies; 25+ messages in thread
From: Andrew Lunn @ 2016-06-28 20:02 UTC (permalink / raw)
  To: Jon Mason
  Cc: zajec5, davem, f.fainelli, hauke, bcm-kernel-feedback-list,
	netdev, linux-kernel

On Tue, Jun 28, 2016 at 03:34:40PM -0400, Jon Mason wrote:
> Move the BCMA MDIO phy into a separate file, as it is very tightly
> coupled with the BCMA bus.  This will help with the upcoming BCMA
> removal from the bgmac driver.  Optimally, this should be moved into
> phy drivers, but it is too tightly coupled with the bgmac driver to
> effectively move it without more changes to the driver.

It is quite common to have the MII bus driver as a sub driver of the
MAC driver, if they are tightly coupled. The MII drivers in
drivers/net/phy are all independent of the MAC driver and use a
different space in the register sets. This does not seem the case
here, so i think the split you have made is O.K, and i don't see a
need to move it into phy.

     Andrew

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac
  2016-06-28 19:34 ` [RFC 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac Jon Mason
@ 2016-06-28 20:11   ` Sergei Shtylyov
  2016-06-29 18:37   ` Florian Fainelli
  2016-06-30 18:06   ` Ray Jui
  2 siblings, 0 replies; 25+ messages in thread
From: Sergei Shtylyov @ 2016-06-28 20:11 UTC (permalink / raw)
  To: Jon Mason, zajec5
  Cc: davem, f.fainelli, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

Hello.

On 06/28/2016 10:34 PM, Jon Mason wrote:

> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
>  .../devicetree/bindings/net/brcm,bgmac-enet.txt     | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
>
> diff --git a/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt b/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
> new file mode 100644
> index 0000000..efd36d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
> @@ -0,0 +1,21 @@
> +Broadcom GMAC Ethernet Controller Device Tree Bindings
> +-------------------------------------------------------------
> +
> +Required properties:
> + - compatible:	"brcm,bgmac-enet"
> + - reg:		Address and length of the GMAC registers,
> +		Address and length of the GMAC IDM registers
> + - interrupts:	Interrupt number
> +
> +Optional properties:
> +- mac-address:	mac address to be assigned to the device

    Refer to ethernet.txt in the same directory.

> +
> +Examples:
> +
> +gmac0: enet@18022000 {

    The node name should be "ethernet@..." to comply eith the ePAPR standard.

WBR, Sergei

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file
  2016-06-28 19:34 ` [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file Jon Mason
  2016-06-28 20:02   ` Andrew Lunn
@ 2016-06-29 14:13   ` Andrew Lunn
  2016-06-29 18:35     ` Florian Fainelli
  1 sibling, 1 reply; 25+ messages in thread
From: Andrew Lunn @ 2016-06-29 14:13 UTC (permalink / raw)
  To: Jon Mason
  Cc: zajec5, davem, f.fainelli, hauke, bcm-kernel-feedback-list,
	netdev, linux-kernel

Hi Jon

I know you are just refactoring code, but at some point it would be
good to take a closer look at this MDIO bus driver.

The MDIO bus driver should be generic, allowing access to all 32
addresses on the bus, if that makes sense. You could for example have
a B53 switch hanging off the MDIO bus.... The basic read/write
functions seem to allow this.

> +/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphyinit */
> +static void bcma_mdio_phy_init(struct bcma_mdio *bcma_mdio)
> +{
> +	struct bcma_chipinfo *ci = &bcma_mdio->core->bus->chipinfo;
> +	u8 i;
> +
> +	if (ci->id == BCMA_CHIP_ID_BCM5356) {
> +		for (i = 0; i < 5; i++) {
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x008b);
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x15, 0x0100);
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000f);
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x12, 0x2aaa);
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000b);

It would be good to document what these writes are doing. Also, could
they be moved into the phy driver? Does the PHY implement an ID in
registers 2 and 3 that can be used to determine these are needed?

Also, why is it iterating over 5 PHYs? Is this actually a switch with
5 ports?

> +		}
> +	}
> +	if ((ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg != 10) ||
> +	    (ci->id == BCMA_CHIP_ID_BCM4749 && ci->pkg != 10) ||
> +	    (ci->id == BCMA_CHIP_ID_BCM53572 && ci->pkg != 9)) {
> +		struct bcma_drv_cc *cc = &bcma_mdio->core->bus->drv_cc;
> +
> +		bcma_chipco_chipctl_maskset(cc, 2, ~0xc0000000, 0);
> +		bcma_chipco_chipctl_maskset(cc, 4, ~0x80000000, 0);
> +		for (i = 0; i < 5; i++) {
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000f);
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x16, 0x5284);
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000b);
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x0010);
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000f);
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x16, 0x5296);
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x1073);
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x9073);
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x16, 0x52b6);
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x9273);
> +			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000b);

Same comment here.

> +/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphyreset */
> +static int bcma_mdio_phy_reset(struct mii_bus *bus)
> +{
> +	struct bcma_mdio *bcma_mdio = bus->priv;
> +	u8 phyaddr = bcma_mdio->phyaddr;
> +
> +	if (bcma_mdio->phyaddr == BGMAC_PHY_NOREGS)
> +		return 0;
> +
> +	bcma_mdio_phy_write(bcma_mdio, phyaddr, MII_BMCR, BMCR_RESET);
> +	udelay(100);
> +	if (bcma_mdio_phy_read(bcma_mdio, phyaddr, MII_BMCR) & BMCR_RESET)
> +		dev_err(&bcma_mdio->core->dev, "PHY reset failed\n");
> +	bcma_mdio_phy_init(bcma_mdio);

This appears to be resetting one PHY. What about the others? Why not
put this in the PHY driver?

> +struct mii_bus *bcma_mdio_mii_register(struct bcma_device *core, u8 phyaddr)
> +{
> +	struct bcma_mdio *bcma_mdio;
> +	struct mii_bus *mii_bus;
> +	int err;
> +
> +	bcma_mdio = kzalloc(sizeof(*bcma_mdio), GFP_KERNEL);
> +	if (!bcma_mdio)
> +		return ERR_PTR(-ENOMEM);
> +
> +	mii_bus = mdiobus_alloc();
> +	if (!mii_bus) {
> +		err = -ENOMEM;
> +		goto err;
> +	}
> +
> +	mii_bus->name = "bcma_mdio mii bus";
> +	sprintf(mii_bus->id, "%s-%d-%d", "bcma_mdio", core->bus->num,
> +		core->core_unit);
> +	mii_bus->priv = bcma_mdio;
> +	mii_bus->read = bcma_mdio_mii_read;
> +	mii_bus->write = bcma_mdio_mii_write;
> +	mii_bus->reset = bcma_mdio_phy_reset;
> +	mii_bus->parent = &core->dev;
> +	mii_bus->phy_mask = ~(1 << phyaddr);

Is there a need to limit this to just the one PHY?

> +
> +	bcma_mdio->core = core;
> +	bcma_mdio->phyaddr = phyaddr;
> +
> +	err = mdiobus_register(mii_bus);

Something which appears to be missing from the later patch which adds
device tree support. If you have a device node pointer, you should use
of_mdiobus_register() and document in the binding that PHYs should be
listed in the usual way.

       Andrew

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file
  2016-06-29 14:13   ` Andrew Lunn
@ 2016-06-29 18:35     ` Florian Fainelli
  2016-06-29 18:46       ` Andrew Lunn
  0 siblings, 1 reply; 25+ messages in thread
From: Florian Fainelli @ 2016-06-29 18:35 UTC (permalink / raw)
  To: Andrew Lunn, Jon Mason
  Cc: zajec5, davem, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

On 06/29/2016 07:13 AM, Andrew Lunn wrote:
> Hi Jon
> 
> I know you are just refactoring code, but at some point it would be
> good to take a closer look at this MDIO bus driver.
> 
> The MDIO bus driver should be generic, allowing access to all 32
> addresses on the bus, if that makes sense. You could for example have
> a B53 switch hanging off the MDIO bus.... The basic read/write
> functions seem to allow this.

That's the case on MIPS-based SoCs actually.

> 
>> +/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphyinit */
>> +static void bcma_mdio_phy_init(struct bcma_mdio *bcma_mdio)
>> +{
>> +	struct bcma_chipinfo *ci = &bcma_mdio->core->bus->chipinfo;
>> +	u8 i;
>> +
>> +	if (ci->id == BCMA_CHIP_ID_BCM5356) {
>> +		for (i = 0; i < 5; i++) {
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x008b);
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x15, 0x0100);
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000f);
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x12, 0x2aaa);
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000b);
> 
> It would be good to document what these writes are doing. Also, could
> they be moved into the phy driver? Does the PHY implement an ID in
> registers 2 and 3 that can be used to determine these are needed?
> 
> Also, why is it iterating over 5 PHYs? Is this actually a switch with
> 5 ports?

Yes, this is applying workarounds to the integrated PHYs, the plan is to
move that to a proper PHY driver so we can get rid of that.

> 
>> +		}
>> +	}
>> +	if ((ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg != 10) ||
>> +	    (ci->id == BCMA_CHIP_ID_BCM4749 && ci->pkg != 10) ||
>> +	    (ci->id == BCMA_CHIP_ID_BCM53572 && ci->pkg != 9)) {
>> +		struct bcma_drv_cc *cc = &bcma_mdio->core->bus->drv_cc;
>> +
>> +		bcma_chipco_chipctl_maskset(cc, 2, ~0xc0000000, 0);
>> +		bcma_chipco_chipctl_maskset(cc, 4, ~0x80000000, 0);
>> +		for (i = 0; i < 5; i++) {
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000f);
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x16, 0x5284);
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000b);
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x0010);
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000f);
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x16, 0x5296);
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x1073);
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x9073);
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x16, 0x52b6);
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x9273);
>> +			bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000b);
> 
> Same comment here.
> 
>> +/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphyreset */
>> +static int bcma_mdio_phy_reset(struct mii_bus *bus)
>> +{
>> +	struct bcma_mdio *bcma_mdio = bus->priv;
>> +	u8 phyaddr = bcma_mdio->phyaddr;
>> +
>> +	if (bcma_mdio->phyaddr == BGMAC_PHY_NOREGS)
>> +		return 0;
>> +
>> +	bcma_mdio_phy_write(bcma_mdio, phyaddr, MII_BMCR, BMCR_RESET);
>> +	udelay(100);
>> +	if (bcma_mdio_phy_read(bcma_mdio, phyaddr, MII_BMCR) & BMCR_RESET)
>> +		dev_err(&bcma_mdio->core->dev, "PHY reset failed\n");
>> +	bcma_mdio_phy_init(bcma_mdio);
> 
> This appears to be resetting one PHY. What about the others? Why not
> put this in the PHY driver?

Probably should be there, keep in mind this driver was mostly reverse
engineered, from an internal code base which usually did not plan on
utilizing existing APIs.

> 
>> +struct mii_bus *bcma_mdio_mii_register(struct bcma_device *core, u8 phyaddr)
>> +{
>> +	struct bcma_mdio *bcma_mdio;
>> +	struct mii_bus *mii_bus;
>> +	int err;
>> +
>> +	bcma_mdio = kzalloc(sizeof(*bcma_mdio), GFP_KERNEL);
>> +	if (!bcma_mdio)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	mii_bus = mdiobus_alloc();
>> +	if (!mii_bus) {
>> +		err = -ENOMEM;
>> +		goto err;
>> +	}
>> +
>> +	mii_bus->name = "bcma_mdio mii bus";
>> +	sprintf(mii_bus->id, "%s-%d-%d", "bcma_mdio", core->bus->num,
>> +		core->core_unit);
>> +	mii_bus->priv = bcma_mdio;
>> +	mii_bus->read = bcma_mdio_mii_read;
>> +	mii_bus->write = bcma_mdio_mii_write;
>> +	mii_bus->reset = bcma_mdio_phy_reset;
>> +	mii_bus->parent = &core->dev;
>> +	mii_bus->phy_mask = ~(1 << phyaddr);
> 
> Is there a need to limit this to just the one PHY?

Usually this is only relevant for an external PHY, and Jon is just
moving code around without making functional changes to existing BCMA
code, so all of that can be changed at a later time.

> 
>> +
>> +	bcma_mdio->core = core;
>> +	bcma_mdio->phyaddr = phyaddr;
>> +
>> +	err = mdiobus_register(mii_bus);
> 
> Something which appears to be missing from the later patch which adds
> device tree support. If you have a device node pointer, you should use
> of_mdiobus_register() and document in the binding that PHYs should be
> listed in the usual way.

This is just moving the code around for now.

I don't think of_mdiobus_register() is going to be used at all, because
on the ARM-based BCM5301X SoCs that use Device Tree, we have yet to see
a platform which requires this MDIO bus driver, all PHYs are internal,
and hanging off the switch register access block.

On MIPS-based BCM53xx/47xx, we need this driver to drive the external
switches, but those are not Device Tree aware, nor is there a need to
make that happen.

And, to re-iterate all of your points are valid, but this is premature
-- 
Florian

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac
  2016-06-28 19:34 ` [RFC 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac Jon Mason
  2016-06-28 20:11   ` Sergei Shtylyov
@ 2016-06-29 18:37   ` Florian Fainelli
  2016-06-30 18:06   ` Ray Jui
  2 siblings, 0 replies; 25+ messages in thread
From: Florian Fainelli @ 2016-06-29 18:37 UTC (permalink / raw)
  To: Jon Mason, zajec5
  Cc: davem, f.fainelli, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

On 06/28/2016 12:34 PM, Jon Mason wrote:
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
>  .../devicetree/bindings/net/brcm,bgmac-enet.txt     | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt b/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
> new file mode 100644
> index 0000000..efd36d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
> @@ -0,0 +1,21 @@
> +Broadcom GMAC Ethernet Controller Device Tree Bindings
> +-------------------------------------------------------------
> +
> +Required properties:
> + - compatible:	"brcm,bgmac-enet"

You might want something a bit more explicit like "brcm,bgmac-nsp" for
instance and also define a compatible string for the Northstar family maybe?

> + - reg:		Address and length of the GMAC registers,
> +		Address and length of the GMAC IDM registers
> + - interrupts:	Interrupt number
> +
> +Optional properties:
> +- mac-address:	mac address to be assigned to the device
> +
> +Examples:
> +
> +gmac0: enet@18022000 {
> +	compatible = "brcm,bgmac-enet";
> +	reg = <0x18022000 0x1000>,
> +	      <0x18110000 0x1000>;
> +	interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
> +	status = "disabled";
> +};
> 


-- 
Florian

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file
  2016-06-29 18:35     ` Florian Fainelli
@ 2016-06-29 18:46       ` Andrew Lunn
  2016-06-29 20:08         ` Jon Mason
  0 siblings, 1 reply; 25+ messages in thread
From: Andrew Lunn @ 2016-06-29 18:46 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Jon Mason, zajec5, davem, hauke, bcm-kernel-feedback-list,
	netdev, linux-kernel

On Wed, Jun 29, 2016 at 11:35:28AM -0700, Florian Fainelli wrote:
> On 06/29/2016 07:13 AM, Andrew Lunn wrote:
> > Hi Jon
> > 
> > I know you are just refactoring code, but at some point it would be
> > good to take a closer look at this MDIO bus driver.
 
> And, to re-iterate all of your points are valid, but this is premature

We agree then :-)

   Andrew

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 5/7] net: ethernet: bgmac: Add platform device support
  2016-06-28 19:34 ` [RFC 5/7] net: ethernet: bgmac: Add platform device support Jon Mason
@ 2016-06-29 18:51   ` Florian Fainelli
  2016-06-30 17:58   ` Ray Jui
  1 sibling, 0 replies; 25+ messages in thread
From: Florian Fainelli @ 2016-06-29 18:51 UTC (permalink / raw)
  To: Jon Mason, zajec5
  Cc: davem, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

On 06/28/2016 12:34 PM, Jon Mason wrote:
> The bcma portion of the driver has been split off into a bcma specific
> driver.  This has been mirrored for the platform driver.  The last
> references to the bcma core struct have been changed into a generic
> function call.  These function calls are wrappers to either the original
> bcma code or new platform functions that access the same areas via MMIO.
> This necessitated adding function pointers for both platform and bcma to
> hide which backend is being used from the generic bgmac code.
> 
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---

[snip]

> +static int bgmac_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct bgmac *bgmac;
> +	struct resource regs;
> +	const u8 *mac_addr;
> +	int rc;
> +
> +	bgmac = kzalloc(sizeof(*bgmac), GFP_KERNEL);

You could utilize devm_kzalloc() here which simplifies the error path.

> +	if (!bgmac)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, bgmac);
> +
> +	/* Set the features of the 4707 family */
> +	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
> +	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
> +	bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500;
> +	bgmac->feature_flags |= BGMAC_FEAT_CMDCFG_SR_REV4;
> +	bgmac->feature_flags |= BGMAC_FEAT_TX_MASK_SETUP;
> +	bgmac->feature_flags |= BGMAC_FEAT_RX_MASK_SETUP;
> +
> +	bgmac->dev = &pdev->dev;
> +	bgmac->dma_dev = &pdev->dev;
> +
> +	mac_addr = of_get_mac_address(np);
> +	if (mac_addr)
> +		ether_addr_copy(bgmac->mac_addr, mac_addr);
> +	else
> +		dev_warn(&pdev->dev, "MAC address not present in device tree\n");
> +
> +	bgmac->irq = platform_get_irq(pdev, 0);
> +	if (bgmac->irq < 0) {
> +		rc = bgmac->irq;
> +		dev_err(&pdev->dev, "Unable to obtain IRQ\n");
> +		goto err;
> +	}
> +
> +	rc = of_address_to_resource(np, 0, &regs);
> +	if (rc < 0) {
> +		dev_err(&pdev->dev, "Unable to obtain base resource\n");
> +		goto err;
> +	}

Here you could fetch the resource using a traditional
platform_get_resource(pdev, IORESOURCE_MEM, 0) and...

> +
> +	bgmac->plat.base = ioremap(regs.start, resource_size(&regs));
> +	if (!bgmac->plat.base) {
> +		dev_err(&pdev->dev, "Unable to map base resource\n");
> +		rc = -ENOMEM;
> +		goto err;
> +	}

... here do a devm_ioremap_resource(), which also does a
request_mem_region, so this shows up nicely in /proc/iomem.

> +
> +	rc = of_address_to_resource(np, 1, &regs);
> +	if (rc < 0) {
> +		dev_err(&pdev->dev, "Unable to obtain idm resource\n");
> +		goto err1;
> +	}
> +
> +	bgmac->plat.idm_base = ioremap(regs.start, resource_size(&regs));
> +	if (!bgmac->plat.base) {
> +		dev_err(&pdev->dev, "Unable to map idm resource\n");
> +		rc = -ENOMEM;
> +		goto err1;
> +	}

Same here.

> +
> +	bgmac->read = platform_bgmac_read;
> +	bgmac->write = platform_bgmac_write;
> +	bgmac->idm_read = platform_bgmac_idm_read;
> +	bgmac->idm_write = platform_bgmac_idm_write;
> +	bgmac->clk_enabled = platform_bgmac_clk_enabled;
> +	bgmac->clk_enable = platform_bgmac_clk_enable;
> +	bgmac->cco_ctl_maskset = platform_bgmac_cco_ctl_maskset;
> +	bgmac->get_bus_clock = platform_bgmac_get_bus_clock;
> +	bgmac->cmn_maskset32 = platform_bgmac_cmn_maskset32;
> +
> +	rc = bgmac_enet_probe(bgmac);
> +	if (rc)
> +		goto err2;
> +
> +	return 0;
> +
> +err2:
> +	iounmap(bgmac->plat.idm_base);
> +err1:
> +	iounmap(bgmac->plat.base);
> +err:
> +	kfree(bgmac);

And with devm_* helpers none of that is needed now.
-- 
Florian

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 0/7] net: ethernet: bgmac: Add platform device support
  2016-06-28 19:34 [RFC 0/7] net: ethernet: bgmac: Add platform device support Jon Mason
                   ` (6 preceding siblings ...)
  2016-06-28 19:34 ` [RFC 7/7] ARM: dts: NSP: Add bgmac entries Jon Mason
@ 2016-06-29 18:52 ` Florian Fainelli
  7 siblings, 0 replies; 25+ messages in thread
From: Florian Fainelli @ 2016-06-29 18:52 UTC (permalink / raw)
  To: Jon Mason, zajec5
  Cc: davem, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

On 06/28/2016 12:34 PM, Jon Mason wrote:
> I'm sending out this RFC to see if this is the direction the maintainers
> would like to go to add support for other, non-bcma iProc SoC's to the
> bgmac driver.  Specifically, we are interested in adding support for the
> NSP, Cygnus, and NS2 families (with more possible down the road).
> 
> To support non-bcma enabled SoCs, we need to add the standard device
> tree "platform device" support.  Unfortunately, this driver is very
> tighly coupled with the bcma bus and much unwinding is needed.  I tried
> to break this up into a number of patches to make it more obvious what
> was being done to add platform device support.  I was able to verify
> that the bcma code still works using a 53012K board (NS SoC), and that
> the platform code works using a 58625K board (NSP SoC).
> 
> It is worth noting that the phy logic present in the driver needs to be
> moved to drivers/phy.  However, I was not able to fully decouple that
> code from the bgmac driver.  I was able to move it into a separate C
> file, with only 2 function calls needed to create and destroy the mii
> bus.  Someone with more knowledge of this and HW to test it needs to do
> it properly.  This would natually dovetail into creating an interface
> which the NSP bgmac can use for the external MDIO Phy to properly
> connect (instead of using the fixed phy).

This looks very good to me, and I just tested this with a BCM58625 w/
b53-srab on top of that, and everything works nicely:

Tested-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file
  2016-06-29 18:46       ` Andrew Lunn
@ 2016-06-29 20:08         ` Jon Mason
  2016-06-29 20:15           ` Andrew Lunn
  0 siblings, 1 reply; 25+ messages in thread
From: Jon Mason @ 2016-06-29 20:08 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, Rafał Miłecki, davem, Hauke Mehrtens,
	BCM Kernel Feedback, netdev, linux-kernel

On Wed, Jun 29, 2016 at 2:46 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Wed, Jun 29, 2016 at 11:35:28AM -0700, Florian Fainelli wrote:
>> On 06/29/2016 07:13 AM, Andrew Lunn wrote:
>> > Hi Jon
>> >
>> > I know you are just refactoring code, but at some point it would be
>> > good to take a closer look at this MDIO bus driver.
>
>> And, to re-iterate all of your points are valid, but this is premature
>
> We agree then :-)
>
>    Andrew

I also agree with all of your points, but hope this is not something
that would prevent this patch series from being acceptable.  It could
be quite an effort to try and properly document the code and what it
is doing, get HW to verify the assumptions from the documentation (as
the MIPS HW was done by a different group, no hardware is available,
etc), relocate the code to the drivers/net/phy and add a standard
interface.  From your original comment, I believe that you agree that
this additional effort is not necessary at this point in time.

Thanks,
Jon

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 1/7] net: ethernet: bgmac: change bgmac_* prints to dev_* prints
  2016-06-28 19:43   ` Joe Perches
@ 2016-06-29 20:10     ` Jon Mason
  0 siblings, 0 replies; 25+ messages in thread
From: Jon Mason @ 2016-06-29 20:10 UTC (permalink / raw)
  To: Joe Perches
  Cc: Rafał Miłecki, davem, Florian Fainelli, Hauke Mehrtens,
	BCM Kernel Feedback, netdev, linux-kernel

On Tue, Jun 28, 2016 at 3:43 PM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2016-06-28 at 15:34 -0400, Jon Mason wrote:
>> The bgmac_* print wrappers call dev_* prints with the dev pointer from
>> the bcma core.  In anticipation of removing the bcma requirement for
>> this driver, these must be changed to not reference that struct.  So,
>> simply change all of the bgmac_* prints to their dev_* counterparts.  In
>> some cases netdev_* prints are more appropriate, so change those as
>> well.
>
> Thanks, but...
>
>> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> []
>> @@ -1515,7 +1516,7 @@ static void bgmac_get_drvinfo(struct net_device *net_dev,
>>                             struct ethtool_drvinfo *info)
>>  {
>>       strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
>> -     strlcpy(info->bus_info, "BCMA", sizeof(info->bus_info));
>> +     strlcpy(info->bus_info, "AXI", sizeof(info->bus_info));
>>  }
>
> Unrelated change?

This is a needed change, but unrelated to this patch.  I'll move it to
the 5/7 patch, where it more logically fits.

Thanks,
Jon

>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file
  2016-06-29 20:08         ` Jon Mason
@ 2016-06-29 20:15           ` Andrew Lunn
  2016-06-29 20:34             ` Jon Mason
  0 siblings, 1 reply; 25+ messages in thread
From: Andrew Lunn @ 2016-06-29 20:15 UTC (permalink / raw)
  To: Jon Mason
  Cc: Florian Fainelli, Rafa?? Mi??ecki, davem, Hauke Mehrtens,
	BCM Kernel Feedback, netdev, linux-kernel

On Wed, Jun 29, 2016 at 04:08:20PM -0400, Jon Mason wrote:
> On Wed, Jun 29, 2016 at 2:46 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Wed, Jun 29, 2016 at 11:35:28AM -0700, Florian Fainelli wrote:
> >> On 06/29/2016 07:13 AM, Andrew Lunn wrote:
> >> > Hi Jon
> >> >
> >> > I know you are just refactoring code, but at some point it would be
> >> > good to take a closer look at this MDIO bus driver.
> >
> >> And, to re-iterate all of your points are valid, but this is premature
> >
> > We agree then :-)
> >
> >    Andrew
> 
> I also agree with all of your points, but hope this is not something
> that would prevent this patch series from being acceptable.

No, it is acceptable as is.

However, it would be nice the clean up the mess, especially if you are
planning on using this code with new platforms, not just legacy stuff
which is bit rotting.

      Andrew

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file
  2016-06-29 20:15           ` Andrew Lunn
@ 2016-06-29 20:34             ` Jon Mason
  0 siblings, 0 replies; 25+ messages in thread
From: Jon Mason @ 2016-06-29 20:34 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, Rafa?? Mi??ecki, davem, Hauke Mehrtens,
	BCM Kernel Feedback, netdev, linux-kernel

On Wed, Jun 29, 2016 at 4:15 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Wed, Jun 29, 2016 at 04:08:20PM -0400, Jon Mason wrote:
>> On Wed, Jun 29, 2016 at 2:46 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > On Wed, Jun 29, 2016 at 11:35:28AM -0700, Florian Fainelli wrote:
>> >> On 06/29/2016 07:13 AM, Andrew Lunn wrote:
>> >> > Hi Jon
>> >> >
>> >> > I know you are just refactoring code, but at some point it would be
>> >> > good to take a closer look at this MDIO bus driver.
>> >
>> >> And, to re-iterate all of your points are valid, but this is premature
>> >
>> > We agree then :-)
>> >
>> >    Andrew
>>
>> I also agree with all of your points, but hope this is not something
>> that would prevent this patch series from being acceptable.
>
> No, it is acceptable as is.
>
> However, it would be nice the clean up the mess, especially if you are
> planning on using this code with new platforms, not just legacy stuff
> which is bit rotting.

I don't believe we'll be using the BCMA infrastructure going forward.
We'll be doing our own MDIO Phys, adding them to the device tree, and
referencing them through the DT/OF method.  This is the current plan
for NSP, NS2, Cygnus, and other upcoming platforms.  If someone can
find an elegant way of splitting off the Phy code from the BCMA core,
then this can be better than it is now.  But, it will take someone
with docs, time, and hardware to give this code the audit and cleanup
it really needs.  BTW, my last comment isn't intended as a slight
against the authors of this code.  It is unfortunate that they were
put in a position of having to reverse engineer drivers for this HW.

Thanks,
Jon

>
>       Andrew

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 5/7] net: ethernet: bgmac: Add platform device support
  2016-06-28 19:34 ` [RFC 5/7] net: ethernet: bgmac: Add platform device support Jon Mason
  2016-06-29 18:51   ` Florian Fainelli
@ 2016-06-30 17:58   ` Ray Jui
  2016-06-30 21:55     ` Jon Mason
  1 sibling, 1 reply; 25+ messages in thread
From: Ray Jui @ 2016-06-30 17:58 UTC (permalink / raw)
  To: Jon Mason, zajec5
  Cc: davem, f.fainelli, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

Hi Jon,

On 6/28/2016 12:34 PM, Jon Mason wrote:
> The bcma portion of the driver has been split off into a bcma specific
> driver.  This has been mirrored for the platform driver.  The last
> references to the bcma core struct have been changed into a generic
> function call.  These function calls are wrappers to either the original
> bcma code or new platform functions that access the same areas via MMIO.
> This necessitated adding function pointers for both platform and bcma to
> hide which backend is being used from the generic bgmac code.
>
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
>  drivers/net/ethernet/broadcom/Kconfig          |  23 +-
>  drivers/net/ethernet/broadcom/Makefile         |   4 +-
>  drivers/net/ethernet/broadcom/bgmac-bcma.c     | 315 ++++++++++++++++++++++++
>  drivers/net/ethernet/broadcom/bgmac-platform.c | 208 ++++++++++++++++
>  drivers/net/ethernet/broadcom/bgmac.c          | 327 ++++---------------------
>  drivers/net/ethernet/broadcom/bgmac.h          |  73 +++++-
>  6 files changed, 666 insertions(+), 284 deletions(-)
>  create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma.c
>  create mode 100644 drivers/net/ethernet/broadcom/bgmac-platform.c
>
> diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
> index d74a92e..bd8c80c 100644
> --- a/drivers/net/ethernet/broadcom/Kconfig
> +++ b/drivers/net/ethernet/broadcom/Kconfig
> @@ -140,10 +140,18 @@ config BNX2X_SRIOV
>  	  allows for virtual function acceleration in virtual environments.
>
>  config BGMAC
> -	tristate "BCMA bus GBit core support"
> +	tristate
> +	help
> +	  This enables the integrated ethernet controller support for many
> +	  Broadcom (mostly iProc) SoCs. An appropriate bus interface driver
> +	  needs to be enabled to select this.
> +
> +config BGMAC_BCMA
> +	tristate "Broadcom iProc GBit BCMA support"
>  	depends on BCMA && BCMA_HOST_SOC
>  	depends on HAS_DMA
>  	depends on BCM47XX || ARCH_BCM_5301X || COMPILE_TEST
> +	select BGMAC
>  	select PHYLIB
>  	select FIXED_PHY
>  	---help---
> @@ -152,6 +160,19 @@ config BGMAC
>  	  In case of using this driver on BCM4706 it's also requires to enable
>  	  BCMA_DRIVER_GMAC_CMN to make it work.
>
> +config BGMAC_PLATFORM
> +	tristate "Broadcom iProc GBit platform support"
> +	depends on HAS_DMA
> +	depends on ARCH_BCM_IPROC || COMPILE_TEST
> +	depends on OF
> +	select BGMAC
> +	select PHYLIB
> +	select FIXED_PHY
> +	default ARCH_BCM_IPROC
> +	---help---
> +	  Say Y here if you want to use the Broadcom iProc Gigabit Ethernet
> +	  controller through the generic platform interface
> +
>  config SYSTEMPORT
>  	tristate "Broadcom SYSTEMPORT internal MAC support"
>  	depends on OF
> diff --git a/drivers/net/ethernet/broadcom/Makefile b/drivers/net/ethernet/broadcom/Makefile
> index f559794..79f2372 100644
> --- a/drivers/net/ethernet/broadcom/Makefile
> +++ b/drivers/net/ethernet/broadcom/Makefile
> @@ -10,6 +10,8 @@ obj-$(CONFIG_CNIC) += cnic.o
>  obj-$(CONFIG_BNX2X) += bnx2x/
>  obj-$(CONFIG_SB1250_MAC) += sb1250-mac.o
>  obj-$(CONFIG_TIGON3) += tg3.o
> -obj-$(CONFIG_BGMAC) += bgmac.o bgmac-bcma-mdio.o
> +obj-$(CONFIG_BGMAC) += bgmac.o
> +obj-$(CONFIG_BGMAC_BCMA) += bgmac-bcma.o bgmac-bcma-mdio.o
> +obj-$(CONFIG_BGMAC_PLATFORM) += bgmac-platform.o
>  obj-$(CONFIG_SYSTEMPORT) += bcmsysport.o
>  obj-$(CONFIG_BNXT) += bnxt/
> diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c b/drivers/net/ethernet/broadcom/bgmac-bcma.c
> new file mode 100644
> index 0000000..9a9745c4
> --- /dev/null
> +++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
> @@ -0,0 +1,315 @@
> +/*
> + * Driver for (BCM4706)? GBit MAC core on BCMA bus.
> + *
> + * Copyright (C) 2012 Rafał Miłecki <zajec5@gmail.com>
> + *
> + * Licensed under the GNU/GPL. See COPYING for details.
> + */
> +
> +#define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
> +
> +#include <linux/bcma/bcma.h>
> +#include <linux/brcmphy.h>
> +#include <linux/etherdevice.h>
> +#include "bgmac.h"
> +
> +static inline bool bgmac_is_bcm4707_family(struct bcma_device *core)
> +{
> +	switch (core->bus->chipinfo.id) {
> +	case BCMA_CHIP_ID_BCM4707:
> +	case BCMA_CHIP_ID_BCM47094:
> +	case BCMA_CHIP_ID_BCM53018:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +/**************************************************
> + * BCMA bus ops
> + **************************************************/
> +
> +static u32 bcma_bgmac_read(struct bgmac *bgmac, u16 offset)
> +{
> +	return bcma_read32(bgmac->bcma.core, offset);
> +}
> +
> +static void bcma_bgmac_write(struct bgmac *bgmac, u16 offset, u32 value)
> +{
> +	bcma_write32(bgmac->bcma.core, offset, value);
> +}
> +
> +static u32 bcma_bgmac_idm_read(struct bgmac *bgmac, u16 offset)
> +{
> +	return bcma_aread32(bgmac->bcma.core, offset);
> +}
> +
> +static void bcma_bgmac_idm_write(struct bgmac *bgmac, u16 offset, u32 value)
> +{
> +	return bcma_awrite32(bgmac->bcma.core, offset, value);
> +}
> +
> +static bool bcma_bgmac_clk_enabled(struct bgmac *bgmac)
> +{
> +	return bcma_core_is_enabled(bgmac->bcma.core);
> +}
> +
> +static void bcma_bgmac_clk_enable(struct bgmac *bgmac, u32 flags)
> +{
> +	bcma_core_enable(bgmac->bcma.core, flags);
> +}
> +
> +static void bcma_bgmac_cco_ctl_maskset(struct bgmac *bgmac, u32 offset,
> +				       u32 mask, u32 set)
> +{
> +	struct bcma_drv_cc *cc = &bgmac->bcma.core->bus->drv_cc;
> +
> +	bcma_chipco_chipctl_maskset(cc, offset, mask, set);
> +}
> +
> +static u32 bcma_bgmac_get_bus_clock(struct bgmac *bgmac)
> +{
> +	struct bcma_drv_cc *cc = &bgmac->bcma.core->bus->drv_cc;
> +
> +	return bcma_pmu_get_bus_clock(cc);
> +}
> +
> +static void bcma_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset, u32 mask,
> +				     u32 set)
> +{
> +	bcma_maskset32(bgmac->bcma.cmn, offset, mask, set);
> +}
> +
> +static const struct bcma_device_id bgmac_bcma_tbl[] = {
> +	BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_4706_MAC_GBIT,
> +		  BCMA_ANY_REV, BCMA_ANY_CLASS),
> +	BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_MAC_GBIT, BCMA_ANY_REV,
> +		  BCMA_ANY_CLASS),
> +	{},
> +};
> +MODULE_DEVICE_TABLE(bcma, bgmac_bcma_tbl);
> +
> +/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipattach */
> +static int bgmac_probe(struct bcma_device *core)
> +{
> +	struct ssb_sprom *sprom = &core->bus->sprom;
> +	struct mii_bus *mii_bus;
> +	struct bgmac *bgmac;
> +	u8 *mac;
> +	int err;
> +
> +	bgmac = kzalloc(sizeof(*bgmac), GFP_KERNEL);
> +	if (!bgmac)
> +		return -ENOMEM;
> +
> +	bgmac->bcma.core = core;
> +	bgmac->dev = &core->dev;
> +	bgmac->dma_dev = core->dma_dev;
> +	bgmac->irq = core->irq;
> +
> +	bcma_set_drvdata(core, bgmac);
> +
> +	switch (core->core_unit) {
> +	case 0:
> +		mac = sprom->et0mac;
> +		break;
> +	case 1:
> +		mac = sprom->et1mac;
> +		break;
> +	case 2:
> +		mac = sprom->et2mac;
> +		break;
> +	default:
> +		dev_err(bgmac->dev, "Unsupported core_unit %d\n",
> +			core->core_unit);
> +		err = -ENOTSUPP;
> +		goto err;
> +	}
> +
> +	ether_addr_copy(bgmac->mac_addr, mac);
> +
> +	/* On BCM4706 we need common core to access PHY */
> +	if (core->id.id == BCMA_CORE_4706_MAC_GBIT &&
> +	    !core->bus->drv_gmac_cmn.core) {
> +		dev_err(bgmac->dev, "GMAC CMN core not found (required for BCM4706)\n");
> +		err = -ENODEV;
> +		goto err;
> +	}
> +	bgmac->bcma.cmn = core->bus->drv_gmac_cmn.core;
> +
> +	switch (core->core_unit) {
> +	case 0:
> +		bgmac->phyaddr = sprom->et0phyaddr;
> +		break;
> +	case 1:
> +		bgmac->phyaddr = sprom->et1phyaddr;
> +		break;
> +	case 2:
> +		bgmac->phyaddr = sprom->et2phyaddr;
> +		break;
> +	}
> +	bgmac->phyaddr &= BGMAC_PHY_MASK;
> +	if (bgmac->phyaddr == BGMAC_PHY_MASK) {
> +		dev_err(bgmac->dev, "No PHY found\n");
> +		err = -ENODEV;
> +		goto err;
> +	}
> +	dev_info(bgmac->dev, "Found PHY addr: %d%s\n", bgmac->phyaddr,
> +		 bgmac->phyaddr == BGMAC_PHY_NOREGS ? " (NOREGS)" : "");
> +
> +	if (!bgmac_is_bcm4707_family(core)) {
> +		mii_bus = bcma_mdio_mii_register(core, bgmac->phyaddr);
> +		if (!IS_ERR(mii_bus)) {
> +			err = PTR_ERR(mii_bus);
> +			goto err;
> +		}
> +
> +		bgmac->mii_bus = mii_bus;
> +	}
> +
> +	if (core->bus->hosttype == BCMA_HOSTTYPE_PCI) {
> +		dev_err(bgmac->dev, "PCI setup not implemented\n");
> +		err = -ENOTSUPP;
> +		goto err1;
> +	}
> +
> +	bgmac->has_robosw = !!(core->bus->sprom.boardflags_lo &
> +			       BGMAC_BFL_ENETROBO);
> +	if (bgmac->has_robosw)
> +		dev_warn(bgmac->dev, "Support for Roboswitch not implemented\n");
> +
> +	if (core->bus->sprom.boardflags_lo & BGMAC_BFL_ENETADM)
> +		dev_warn(bgmac->dev, "Support for ADMtek ethernet switch not implemented\n");
> +
> +	/* Feature Flags */
> +	switch (core->bus->chipinfo.id) {
> +	case BCMA_CHIP_ID_BCM5357:
> +		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
> +		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
> +		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
> +		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
> +		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM47186) {
> +			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
> +			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
> +		}
> +		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM5358)
> +			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_EPHYRMII;
> +		break;
> +	case BCMA_CHIP_ID_BCM53572:
> +		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
> +		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
> +		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
> +		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
> +		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM47188) {
> +			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
> +			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
> +		}
> +		break;
> +	case BCMA_CHIP_ID_BCM4749:
> +		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
> +		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
> +		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
> +		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
> +		if (core->bus->chipinfo.pkg == 10) {
> +			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
> +			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
> +		}
> +		break;
> +	case BCMA_CHIP_ID_BCM4716:
> +		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
> +		/* fallthrough */
> +	case BCMA_CHIP_ID_BCM47162:
> +		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL2;
> +		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
> +		break;
> +	/* bcm4707_family */
> +	case BCMA_CHIP_ID_BCM4707:
> +	case BCMA_CHIP_ID_BCM47094:
> +	case BCMA_CHIP_ID_BCM53018:
> +		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
> +		bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
> +		bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500;
> +		break;
> +	default:
> +		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
> +		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
> +	}
> +
> +	if (!bgmac_is_bcm4707_family(core) && core->id.rev > 2)
> +		bgmac->feature_flags |= BGMAC_FEAT_MISC_PLL_REQ;
> +
> +	if (core->id.id == BCMA_CORE_4706_MAC_GBIT) {
> +		bgmac->feature_flags |= BGMAC_FEAT_CMN_PHY_CTL;
> +		bgmac->feature_flags |= BGMAC_FEAT_NO_CLR_MIB;
> +	}
> +
> +	if (core->id.rev >= 4) {
> +		bgmac->feature_flags |= BGMAC_FEAT_CMDCFG_SR_REV4;
> +		bgmac->feature_flags |= BGMAC_FEAT_TX_MASK_SETUP;
> +		bgmac->feature_flags |= BGMAC_FEAT_RX_MASK_SETUP;
> +	}
> +
> +	bgmac->read = bcma_bgmac_read;
> +	bgmac->write = bcma_bgmac_write;
> +	bgmac->idm_read = bcma_bgmac_idm_read;
> +	bgmac->idm_write = bcma_bgmac_idm_write;
> +	bgmac->clk_enabled = bcma_bgmac_clk_enabled;
> +	bgmac->clk_enable = bcma_bgmac_clk_enable;
> +	bgmac->cco_ctl_maskset = bcma_bgmac_cco_ctl_maskset;
> +	bgmac->get_bus_clock = bcma_bgmac_get_bus_clock;
> +	bgmac->cmn_maskset32 = bcma_bgmac_cmn_maskset32;
> +
> +	err = bgmac_enet_probe(bgmac);
> +	if (err)
> +		goto err1;
> +
> +	return 0;
> +
> +err1:
> +	bcma_mdio_mii_unregister(bgmac->mii_bus);
> +err:
> +	kfree(bgmac);
> +	bcma_set_drvdata(core, NULL);
> +
> +	return err;
> +}
> +
> +static void bgmac_remove(struct bcma_device *core)
> +{
> +	struct bgmac *bgmac = bcma_get_drvdata(core);
> +
> +	bcma_mdio_mii_unregister(bgmac->mii_bus);
> +	bgmac_enet_remove(bgmac);
> +	bcma_set_drvdata(core, NULL);
> +	kfree(bgmac);
> +}
> +
> +static struct bcma_driver bgmac_bcma_driver = {
> +	.name		= KBUILD_MODNAME,
> +	.id_table	= bgmac_bcma_tbl,
> +	.probe		= bgmac_probe,
> +	.remove		= bgmac_remove,
> +};
> +
> +static int __init bgmac_init(void)
> +{
> +	int err;
> +
> +	err = bcma_driver_register(&bgmac_bcma_driver);
> +	if (err)
> +		return err;
> +	pr_info("Broadcom 47xx GBit MAC driver loaded\n");
> +
> +	return 0;
> +}
> +
> +static void __exit bgmac_exit(void)
> +{
> +	bcma_driver_unregister(&bgmac_bcma_driver);
> +}
> +
> +module_init(bgmac_init)
> +module_exit(bgmac_exit)
> +
> +MODULE_AUTHOR("Rafał Miłecki");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
> new file mode 100644
> index 0000000..fac93c0
> --- /dev/null
> +++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
> @@ -0,0 +1,208 @@
> +/*
> + * Copyright (C) 2016 Broadcom
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
> +
> +#include <linux/bcma/bcma.h>
> +#include <linux/etherdevice.h>
> +#include <linux/of_address.h>
> +#include <linux/of_net.h>
> +#include "bgmac.h"
> +
> +static u32 platform_bgmac_read(struct bgmac *bgmac, u16 offset)
> +{
> +	return readl(bgmac->plat.base + offset);
> +}
> +
> +static void platform_bgmac_write(struct bgmac *bgmac, u16 offset, u32 value)
> +{
> +	writel(value, bgmac->plat.base + offset);
> +}
> +
> +static u32 platform_bgmac_idm_read(struct bgmac *bgmac, u16 offset)
> +{
> +	return readl(bgmac->plat.idm_base + offset);
> +}
> +
> +static void platform_bgmac_idm_write(struct bgmac *bgmac, u16 offset, u32 value)
> +{
> +	return writel(value, bgmac->plat.idm_base + offset);
> +}
> +
> +static bool platform_bgmac_clk_enabled(struct bgmac *bgmac)
> +{
> +	if ((bgmac_idm_read(bgmac, BCMA_IOCTL) &
> +	     (BCMA_IOCTL_CLK | BCMA_IOCTL_FGC)) != BCMA_IOCTL_CLK)
> +		return false;
> +	if (bgmac_idm_read(bgmac, BCMA_RESET_CTL) & BCMA_RESET_CTL_RESET)
> +		return false;
> +	return true;
> +}
> +
> +static void platform_bgmac_clk_enable(struct bgmac *bgmac, u32 flags)
> +{
> +	bgmac_idm_write(bgmac, BCMA_IOCTL,
> +			(BCMA_IOCTL_CLK | BCMA_IOCTL_FGC | flags));
> +	bgmac_idm_read(bgmac, BCMA_IOCTL);
> +
> +	bgmac_idm_write(bgmac, BCMA_RESET_CTL, 0);
> +	bgmac_idm_read(bgmac, BCMA_RESET_CTL);
> +	udelay(1);
> +
> +	bgmac_idm_write(bgmac, BCMA_IOCTL, (BCMA_IOCTL_CLK | flags));
> +	bgmac_idm_read(bgmac, BCMA_IOCTL);
> +	udelay(1);
> +}
> +
> +static void platform_bgmac_cco_ctl_maskset(struct bgmac *bgmac, u32 offset,
> +					   u32 mask, u32 set)
> +{
> +	/* This shouldn't be encountered */
> +	WARN_ON(1);
> +}
> +
> +static u32 platform_bgmac_get_bus_clock(struct bgmac *bgmac)
> +{
> +	/* This shouldn't be encountered */
> +	WARN_ON(1);
> +
> +	return 0;
> +}
> +
> +static void platform_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset,
> +					 u32 mask, u32 set)
> +{
> +	/* This shouldn't be encountered */
> +	WARN_ON(1);
> +}
> +
> +static int bgmac_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct bgmac *bgmac;
> +	struct resource regs;
> +	const u8 *mac_addr;
> +	int rc;
> +
> +	bgmac = kzalloc(sizeof(*bgmac), GFP_KERNEL);

The trend has been using devm_xxx based API to let the devm framework 
deals with device resource management. This will help to reduce all the 
code that is currently needed to free the resource below and in the 
remove function.

> +	if (!bgmac)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, bgmac);
> +
> +	/* Set the features of the 4707 family */
> +	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
> +	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
> +	bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500;
> +	bgmac->feature_flags |= BGMAC_FEAT_CMDCFG_SR_REV4;
> +	bgmac->feature_flags |= BGMAC_FEAT_TX_MASK_SETUP;
> +	bgmac->feature_flags |= BGMAC_FEAT_RX_MASK_SETUP;
> +
> +	bgmac->dev = &pdev->dev;
> +	bgmac->dma_dev = &pdev->dev;
> +
> +	mac_addr = of_get_mac_address(np);
> +	if (mac_addr)
> +		ether_addr_copy(bgmac->mac_addr, mac_addr);
> +	else
> +		dev_warn(&pdev->dev, "MAC address not present in device tree\n");
> +
> +	bgmac->irq = platform_get_irq(pdev, 0);

> +	if (bgmac->irq < 0) {
> +		rc = bgmac->irq;
> +		dev_err(&pdev->dev, "Unable to obtain IRQ\n");
> +		goto err;
> +	}
> +
> +	rc = of_address_to_resource(np, 0, &regs);
> +	if (rc < 0) {
> +		dev_err(&pdev->dev, "Unable to obtain base resource\n");
> +		goto err;
> +	}
> +
> +	bgmac->plat.base = ioremap(regs.start, resource_size(&regs));

Again, there's devm based API to use, and the resource should be marked 
reserved so no one else can remap it.

platform_get_resource
devm_ioremap_resource

> +	if (!bgmac->plat.base) {
> +		dev_err(&pdev->dev, "Unable to map base resource\n");
> +		rc = -ENOMEM;
> +		goto err;
> +	}
> +
> +	rc = of_address_to_resource(np, 1, &regs);
> +	if (rc < 0) {
> +		dev_err(&pdev->dev, "Unable to obtain idm resource\n");
> +		goto err1;
> +	}
> +
> +	bgmac->plat.idm_base = ioremap(regs.start, resource_size(&regs));
> +	if (!bgmac->plat.base) {
> +		dev_err(&pdev->dev, "Unable to map idm resource\n");
> +		rc = -ENOMEM;
> +		goto err1;
> +	}

same here

> +
> +	bgmac->read = platform_bgmac_read;
> +	bgmac->write = platform_bgmac_write;
> +	bgmac->idm_read = platform_bgmac_idm_read;
> +	bgmac->idm_write = platform_bgmac_idm_write;
> +	bgmac->clk_enabled = platform_bgmac_clk_enabled;
> +	bgmac->clk_enable = platform_bgmac_clk_enable;
> +	bgmac->cco_ctl_maskset = platform_bgmac_cco_ctl_maskset;
> +	bgmac->get_bus_clock = platform_bgmac_get_bus_clock;
> +	bgmac->cmn_maskset32 = platform_bgmac_cmn_maskset32;
> +
> +	rc = bgmac_enet_probe(bgmac);
> +	if (rc)
> +		goto err2;
> +
> +	return 0;
> +
> +err2:
> +	iounmap(bgmac->plat.idm_base);
> +err1:
> +	iounmap(bgmac->plat.base);
> +err:
> +	kfree(bgmac);

All of the above error handling code can be gone.

> +
> +	return rc;
> +}
> +
> +static int bgmac_remove(struct platform_device *pdev)
> +{
> +	struct bgmac *bgmac = platform_get_drvdata(pdev);
> +
> +	bgmac_enet_remove(bgmac);
> +	iounmap(bgmac->plat.idm_base);
> +	iounmap(bgmac->plat.base);
> +	kfree(bgmac);

Here too.

> +
> +	return 0;
> +}
> +
> +static const struct of_device_id bgmac_of_enet_match[] = {
> +	{.compatible = "brcm,bgmac-enet",},
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, bgmac_of_enet_match);
> +
> +static struct platform_driver bgmac_enet_driver = {
> +	.driver = {
> +		.name  = "bgmac-enet",
> +		.of_match_table = bgmac_of_enet_match,
> +	},
> +	.probe = bgmac_probe,
> +	.remove = bgmac_remove,
> +};
> +
> +module_platform_driver(bgmac_enet_driver);
> +MODULE_LICENSE("GPL");

...
...

Thanks,

Ray

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac
  2016-06-28 19:34 ` [RFC 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac Jon Mason
  2016-06-28 20:11   ` Sergei Shtylyov
  2016-06-29 18:37   ` Florian Fainelli
@ 2016-06-30 18:06   ` Ray Jui
  2016-06-30 21:57     ` Jon Mason
  2 siblings, 1 reply; 25+ messages in thread
From: Ray Jui @ 2016-06-30 18:06 UTC (permalink / raw)
  To: Jon Mason, zajec5
  Cc: davem, f.fainelli, hauke, bcm-kernel-feedback-list, netdev, linux-kernel

Hi Jon,

On 6/28/2016 12:34 PM, Jon Mason wrote:
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
>  .../devicetree/bindings/net/brcm,bgmac-enet.txt     | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
>
> diff --git a/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt b/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
> new file mode 100644
> index 0000000..efd36d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
> @@ -0,0 +1,21 @@
> +Broadcom GMAC Ethernet Controller Device Tree Bindings
> +-------------------------------------------------------------
> +
> +Required properties:
> + - compatible:	"brcm,bgmac-enet"
> + - reg:		Address and length of the GMAC registers,
> +		Address and length of the GMAC IDM registers

As we know there will be additional optional register banks required for 
some of the other SoCs that the current driver has not yet supported. In 
my opinion, we should consider to make "reg-names" a mandatory property 
now and map the register blocks based on names.

I think this will help to make our life easier in the future when new 
optional SoC specific register blocks are added, such that we can map 
the register blocks based on names instead of indices, which will change 
and be different among different SoCs and will require much more complex 
logic in the driver to deal with.

> + - interrupts:	Interrupt number
> +
> +Optional properties:
> +- mac-address:	mac address to be assigned to the device
> +
> +Examples:
> +
> +gmac0: enet@18022000 {
> +	compatible = "brcm,bgmac-enet";
> +	reg = <0x18022000 0x1000>,
> +	      <0x18110000 0x1000>;
> +	interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
> +	status = "disabled";
> +};
>

Btw, I think Rob Herring should be included in the review for device 
tree binding document changes.

Thanks,

Ray

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 5/7] net: ethernet: bgmac: Add platform device support
  2016-06-30 17:58   ` Ray Jui
@ 2016-06-30 21:55     ` Jon Mason
  0 siblings, 0 replies; 25+ messages in thread
From: Jon Mason @ 2016-06-30 21:55 UTC (permalink / raw)
  To: Ray Jui
  Cc: Rafał Miłecki, davem, Florian Fainelli, Hauke Mehrtens,
	BCM Kernel Feedback, netdev, linux-kernel

On Thu, Jun 30, 2016 at 1:58 PM, Ray Jui <ray.jui@broadcom.com> wrote:
> Hi Jon,
>
>
> On 6/28/2016 12:34 PM, Jon Mason wrote:
>>
>> The bcma portion of the driver has been split off into a bcma specific
>> driver.  This has been mirrored for the platform driver.  The last
>> references to the bcma core struct have been changed into a generic
>> function call.  These function calls are wrappers to either the original
>> bcma code or new platform functions that access the same areas via MMIO.
>> This necessitated adding function pointers for both platform and bcma to
>> hide which backend is being used from the generic bgmac code.
>>
>> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>> ---
>>  drivers/net/ethernet/broadcom/Kconfig          |  23 +-
>>  drivers/net/ethernet/broadcom/Makefile         |   4 +-
>>  drivers/net/ethernet/broadcom/bgmac-bcma.c     | 315
>> ++++++++++++++++++++++++
>>  drivers/net/ethernet/broadcom/bgmac-platform.c | 208 ++++++++++++++++
>>  drivers/net/ethernet/broadcom/bgmac.c          | 327
>> ++++---------------------
>>  drivers/net/ethernet/broadcom/bgmac.h          |  73 +++++-
>>  6 files changed, 666 insertions(+), 284 deletions(-)
>>  create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma.c
>>  create mode 100644 drivers/net/ethernet/broadcom/bgmac-platform.c
>>
>> diff --git a/drivers/net/ethernet/broadcom/Kconfig
>> b/drivers/net/ethernet/broadcom/Kconfig
>> index d74a92e..bd8c80c 100644
>> --- a/drivers/net/ethernet/broadcom/Kconfig
>> +++ b/drivers/net/ethernet/broadcom/Kconfig
>> @@ -140,10 +140,18 @@ config BNX2X_SRIOV
>>           allows for virtual function acceleration in virtual
>> environments.
>>
>>  config BGMAC
>> -       tristate "BCMA bus GBit core support"
>> +       tristate
>> +       help
>> +         This enables the integrated ethernet controller support for many
>> +         Broadcom (mostly iProc) SoCs. An appropriate bus interface
>> driver
>> +         needs to be enabled to select this.
>> +
>> +config BGMAC_BCMA
>> +       tristate "Broadcom iProc GBit BCMA support"
>>         depends on BCMA && BCMA_HOST_SOC
>>         depends on HAS_DMA
>>         depends on BCM47XX || ARCH_BCM_5301X || COMPILE_TEST
>> +       select BGMAC
>>         select PHYLIB
>>         select FIXED_PHY
>>         ---help---
>> @@ -152,6 +160,19 @@ config BGMAC
>>           In case of using this driver on BCM4706 it's also requires to
>> enable
>>           BCMA_DRIVER_GMAC_CMN to make it work.
>>
>> +config BGMAC_PLATFORM
>> +       tristate "Broadcom iProc GBit platform support"
>> +       depends on HAS_DMA
>> +       depends on ARCH_BCM_IPROC || COMPILE_TEST
>> +       depends on OF
>> +       select BGMAC
>> +       select PHYLIB
>> +       select FIXED_PHY
>> +       default ARCH_BCM_IPROC
>> +       ---help---
>> +         Say Y here if you want to use the Broadcom iProc Gigabit
>> Ethernet
>> +         controller through the generic platform interface
>> +
>>  config SYSTEMPORT
>>         tristate "Broadcom SYSTEMPORT internal MAC support"
>>         depends on OF
>> diff --git a/drivers/net/ethernet/broadcom/Makefile
>> b/drivers/net/ethernet/broadcom/Makefile
>> index f559794..79f2372 100644
>> --- a/drivers/net/ethernet/broadcom/Makefile
>> +++ b/drivers/net/ethernet/broadcom/Makefile
>> @@ -10,6 +10,8 @@ obj-$(CONFIG_CNIC) += cnic.o
>>  obj-$(CONFIG_BNX2X) += bnx2x/
>>  obj-$(CONFIG_SB1250_MAC) += sb1250-mac.o
>>  obj-$(CONFIG_TIGON3) += tg3.o
>> -obj-$(CONFIG_BGMAC) += bgmac.o bgmac-bcma-mdio.o
>> +obj-$(CONFIG_BGMAC) += bgmac.o
>> +obj-$(CONFIG_BGMAC_BCMA) += bgmac-bcma.o bgmac-bcma-mdio.o
>> +obj-$(CONFIG_BGMAC_PLATFORM) += bgmac-platform.o
>>  obj-$(CONFIG_SYSTEMPORT) += bcmsysport.o
>>  obj-$(CONFIG_BNXT) += bnxt/
>> diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c
>> b/drivers/net/ethernet/broadcom/bgmac-bcma.c
>> new file mode 100644
>> index 0000000..9a9745c4
>> --- /dev/null
>> +++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
>> @@ -0,0 +1,315 @@
>> +/*
>> + * Driver for (BCM4706)? GBit MAC core on BCMA bus.
>> + *
>> + * Copyright (C) 2012 Rafał Miłecki <zajec5@gmail.com>
>> + *
>> + * Licensed under the GNU/GPL. See COPYING for details.
>> + */
>> +
>> +#define pr_fmt(fmt)            KBUILD_MODNAME ": " fmt
>> +
>> +#include <linux/bcma/bcma.h>
>> +#include <linux/brcmphy.h>
>> +#include <linux/etherdevice.h>
>> +#include "bgmac.h"
>> +
>> +static inline bool bgmac_is_bcm4707_family(struct bcma_device *core)
>> +{
>> +       switch (core->bus->chipinfo.id) {
>> +       case BCMA_CHIP_ID_BCM4707:
>> +       case BCMA_CHIP_ID_BCM47094:
>> +       case BCMA_CHIP_ID_BCM53018:
>> +               return true;
>> +       default:
>> +               return false;
>> +       }
>> +}
>> +
>> +/**************************************************
>> + * BCMA bus ops
>> + **************************************************/
>> +
>> +static u32 bcma_bgmac_read(struct bgmac *bgmac, u16 offset)
>> +{
>> +       return bcma_read32(bgmac->bcma.core, offset);
>> +}
>> +
>> +static void bcma_bgmac_write(struct bgmac *bgmac, u16 offset, u32 value)
>> +{
>> +       bcma_write32(bgmac->bcma.core, offset, value);
>> +}
>> +
>> +static u32 bcma_bgmac_idm_read(struct bgmac *bgmac, u16 offset)
>> +{
>> +       return bcma_aread32(bgmac->bcma.core, offset);
>> +}
>> +
>> +static void bcma_bgmac_idm_write(struct bgmac *bgmac, u16 offset, u32
>> value)
>> +{
>> +       return bcma_awrite32(bgmac->bcma.core, offset, value);
>> +}
>> +
>> +static bool bcma_bgmac_clk_enabled(struct bgmac *bgmac)
>> +{
>> +       return bcma_core_is_enabled(bgmac->bcma.core);
>> +}
>> +
>> +static void bcma_bgmac_clk_enable(struct bgmac *bgmac, u32 flags)
>> +{
>> +       bcma_core_enable(bgmac->bcma.core, flags);
>> +}
>> +
>> +static void bcma_bgmac_cco_ctl_maskset(struct bgmac *bgmac, u32 offset,
>> +                                      u32 mask, u32 set)
>> +{
>> +       struct bcma_drv_cc *cc = &bgmac->bcma.core->bus->drv_cc;
>> +
>> +       bcma_chipco_chipctl_maskset(cc, offset, mask, set);
>> +}
>> +
>> +static u32 bcma_bgmac_get_bus_clock(struct bgmac *bgmac)
>> +{
>> +       struct bcma_drv_cc *cc = &bgmac->bcma.core->bus->drv_cc;
>> +
>> +       return bcma_pmu_get_bus_clock(cc);
>> +}
>> +
>> +static void bcma_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset, u32
>> mask,
>> +                                    u32 set)
>> +{
>> +       bcma_maskset32(bgmac->bcma.cmn, offset, mask, set);
>> +}
>> +
>> +static const struct bcma_device_id bgmac_bcma_tbl[] = {
>> +       BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_4706_MAC_GBIT,
>> +                 BCMA_ANY_REV, BCMA_ANY_CLASS),
>> +       BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_MAC_GBIT, BCMA_ANY_REV,
>> +                 BCMA_ANY_CLASS),
>> +       {},
>> +};
>> +MODULE_DEVICE_TABLE(bcma, bgmac_bcma_tbl);
>> +
>> +/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipattach */
>> +static int bgmac_probe(struct bcma_device *core)
>> +{
>> +       struct ssb_sprom *sprom = &core->bus->sprom;
>> +       struct mii_bus *mii_bus;
>> +       struct bgmac *bgmac;
>> +       u8 *mac;
>> +       int err;
>> +
>> +       bgmac = kzalloc(sizeof(*bgmac), GFP_KERNEL);
>> +       if (!bgmac)
>> +               return -ENOMEM;
>> +
>> +       bgmac->bcma.core = core;
>> +       bgmac->dev = &core->dev;
>> +       bgmac->dma_dev = core->dma_dev;
>> +       bgmac->irq = core->irq;
>> +
>> +       bcma_set_drvdata(core, bgmac);
>> +
>> +       switch (core->core_unit) {
>> +       case 0:
>> +               mac = sprom->et0mac;
>> +               break;
>> +       case 1:
>> +               mac = sprom->et1mac;
>> +               break;
>> +       case 2:
>> +               mac = sprom->et2mac;
>> +               break;
>> +       default:
>> +               dev_err(bgmac->dev, "Unsupported core_unit %d\n",
>> +                       core->core_unit);
>> +               err = -ENOTSUPP;
>> +               goto err;
>> +       }
>> +
>> +       ether_addr_copy(bgmac->mac_addr, mac);
>> +
>> +       /* On BCM4706 we need common core to access PHY */
>> +       if (core->id.id == BCMA_CORE_4706_MAC_GBIT &&
>> +           !core->bus->drv_gmac_cmn.core) {
>> +               dev_err(bgmac->dev, "GMAC CMN core not found (required for
>> BCM4706)\n");
>> +               err = -ENODEV;
>> +               goto err;
>> +       }
>> +       bgmac->bcma.cmn = core->bus->drv_gmac_cmn.core;
>> +
>> +       switch (core->core_unit) {
>> +       case 0:
>> +               bgmac->phyaddr = sprom->et0phyaddr;
>> +               break;
>> +       case 1:
>> +               bgmac->phyaddr = sprom->et1phyaddr;
>> +               break;
>> +       case 2:
>> +               bgmac->phyaddr = sprom->et2phyaddr;
>> +               break;
>> +       }
>> +       bgmac->phyaddr &= BGMAC_PHY_MASK;
>> +       if (bgmac->phyaddr == BGMAC_PHY_MASK) {
>> +               dev_err(bgmac->dev, "No PHY found\n");
>> +               err = -ENODEV;
>> +               goto err;
>> +       }
>> +       dev_info(bgmac->dev, "Found PHY addr: %d%s\n", bgmac->phyaddr,
>> +                bgmac->phyaddr == BGMAC_PHY_NOREGS ? " (NOREGS)" : "");
>> +
>> +       if (!bgmac_is_bcm4707_family(core)) {
>> +               mii_bus = bcma_mdio_mii_register(core, bgmac->phyaddr);
>> +               if (!IS_ERR(mii_bus)) {
>> +                       err = PTR_ERR(mii_bus);
>> +                       goto err;
>> +               }
>> +
>> +               bgmac->mii_bus = mii_bus;
>> +       }
>> +
>> +       if (core->bus->hosttype == BCMA_HOSTTYPE_PCI) {
>> +               dev_err(bgmac->dev, "PCI setup not implemented\n");
>> +               err = -ENOTSUPP;
>> +               goto err1;
>> +       }
>> +
>> +       bgmac->has_robosw = !!(core->bus->sprom.boardflags_lo &
>> +                              BGMAC_BFL_ENETROBO);
>> +       if (bgmac->has_robosw)
>> +               dev_warn(bgmac->dev, "Support for Roboswitch not
>> implemented\n");
>> +
>> +       if (core->bus->sprom.boardflags_lo & BGMAC_BFL_ENETADM)
>> +               dev_warn(bgmac->dev, "Support for ADMtek ethernet switch
>> not implemented\n");
>> +
>> +       /* Feature Flags */
>> +       switch (core->bus->chipinfo.id) {
>> +       case BCMA_CHIP_ID_BCM5357:
>> +               bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
>> +               bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>> +               bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
>> +               bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
>> +               if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM47186) {
>> +                       bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
>> +                       bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
>> +               }
>> +               if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM5358)
>> +                       bgmac->feature_flags |=
>> BGMAC_FEAT_SW_TYPE_EPHYRMII;
>> +               break;
>> +       case BCMA_CHIP_ID_BCM53572:
>> +               bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
>> +               bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>> +               bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
>> +               bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
>> +               if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM47188) {
>> +                       bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
>> +                       bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
>> +               }
>> +               break;
>> +       case BCMA_CHIP_ID_BCM4749:
>> +               bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
>> +               bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>> +               bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
>> +               bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
>> +               if (core->bus->chipinfo.pkg == 10) {
>> +                       bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
>> +                       bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
>> +               }
>> +               break;
>> +       case BCMA_CHIP_ID_BCM4716:
>> +               bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>> +               /* fallthrough */
>> +       case BCMA_CHIP_ID_BCM47162:
>> +               bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL2;
>> +               bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
>> +               break;
>> +       /* bcm4707_family */
>> +       case BCMA_CHIP_ID_BCM4707:
>> +       case BCMA_CHIP_ID_BCM47094:
>> +       case BCMA_CHIP_ID_BCM53018:
>> +               bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>> +               bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
>> +               bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500;
>> +               break;
>> +       default:
>> +               bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>> +               bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
>> +       }
>> +
>> +       if (!bgmac_is_bcm4707_family(core) && core->id.rev > 2)
>> +               bgmac->feature_flags |= BGMAC_FEAT_MISC_PLL_REQ;
>> +
>> +       if (core->id.id == BCMA_CORE_4706_MAC_GBIT) {
>> +               bgmac->feature_flags |= BGMAC_FEAT_CMN_PHY_CTL;
>> +               bgmac->feature_flags |= BGMAC_FEAT_NO_CLR_MIB;
>> +       }
>> +
>> +       if (core->id.rev >= 4) {
>> +               bgmac->feature_flags |= BGMAC_FEAT_CMDCFG_SR_REV4;
>> +               bgmac->feature_flags |= BGMAC_FEAT_TX_MASK_SETUP;
>> +               bgmac->feature_flags |= BGMAC_FEAT_RX_MASK_SETUP;
>> +       }
>> +
>> +       bgmac->read = bcma_bgmac_read;
>> +       bgmac->write = bcma_bgmac_write;
>> +       bgmac->idm_read = bcma_bgmac_idm_read;
>> +       bgmac->idm_write = bcma_bgmac_idm_write;
>> +       bgmac->clk_enabled = bcma_bgmac_clk_enabled;
>> +       bgmac->clk_enable = bcma_bgmac_clk_enable;
>> +       bgmac->cco_ctl_maskset = bcma_bgmac_cco_ctl_maskset;
>> +       bgmac->get_bus_clock = bcma_bgmac_get_bus_clock;
>> +       bgmac->cmn_maskset32 = bcma_bgmac_cmn_maskset32;
>> +
>> +       err = bgmac_enet_probe(bgmac);
>> +       if (err)
>> +               goto err1;
>> +
>> +       return 0;
>> +
>> +err1:
>> +       bcma_mdio_mii_unregister(bgmac->mii_bus);
>> +err:
>> +       kfree(bgmac);
>> +       bcma_set_drvdata(core, NULL);
>> +
>> +       return err;
>> +}
>> +
>> +static void bgmac_remove(struct bcma_device *core)
>> +{
>> +       struct bgmac *bgmac = bcma_get_drvdata(core);
>> +
>> +       bcma_mdio_mii_unregister(bgmac->mii_bus);
>> +       bgmac_enet_remove(bgmac);
>> +       bcma_set_drvdata(core, NULL);
>> +       kfree(bgmac);
>> +}
>> +
>> +static struct bcma_driver bgmac_bcma_driver = {
>> +       .name           = KBUILD_MODNAME,
>> +       .id_table       = bgmac_bcma_tbl,
>> +       .probe          = bgmac_probe,
>> +       .remove         = bgmac_remove,
>> +};
>> +
>> +static int __init bgmac_init(void)
>> +{
>> +       int err;
>> +
>> +       err = bcma_driver_register(&bgmac_bcma_driver);
>> +       if (err)
>> +               return err;
>> +       pr_info("Broadcom 47xx GBit MAC driver loaded\n");
>> +
>> +       return 0;
>> +}
>> +
>> +static void __exit bgmac_exit(void)
>> +{
>> +       bcma_driver_unregister(&bgmac_bcma_driver);
>> +}
>> +
>> +module_init(bgmac_init)
>> +module_exit(bgmac_exit)
>> +
>> +MODULE_AUTHOR("Rafał Miłecki");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c
>> b/drivers/net/ethernet/broadcom/bgmac-platform.c
>> new file mode 100644
>> index 0000000..fac93c0
>> --- /dev/null
>> +++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
>> @@ -0,0 +1,208 @@
>> +/*
>> + * Copyright (C) 2016 Broadcom
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + *
>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#define pr_fmt(fmt)            KBUILD_MODNAME ": " fmt
>> +
>> +#include <linux/bcma/bcma.h>
>> +#include <linux/etherdevice.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_net.h>
>> +#include "bgmac.h"
>> +
>> +static u32 platform_bgmac_read(struct bgmac *bgmac, u16 offset)
>> +{
>> +       return readl(bgmac->plat.base + offset);
>> +}
>> +
>> +static void platform_bgmac_write(struct bgmac *bgmac, u16 offset, u32
>> value)
>> +{
>> +       writel(value, bgmac->plat.base + offset);
>> +}
>> +
>> +static u32 platform_bgmac_idm_read(struct bgmac *bgmac, u16 offset)
>> +{
>> +       return readl(bgmac->plat.idm_base + offset);
>> +}
>> +
>> +static void platform_bgmac_idm_write(struct bgmac *bgmac, u16 offset, u32
>> value)
>> +{
>> +       return writel(value, bgmac->plat.idm_base + offset);
>> +}
>> +
>> +static bool platform_bgmac_clk_enabled(struct bgmac *bgmac)
>> +{
>> +       if ((bgmac_idm_read(bgmac, BCMA_IOCTL) &
>> +            (BCMA_IOCTL_CLK | BCMA_IOCTL_FGC)) != BCMA_IOCTL_CLK)
>> +               return false;
>> +       if (bgmac_idm_read(bgmac, BCMA_RESET_CTL) & BCMA_RESET_CTL_RESET)
>> +               return false;
>> +       return true;
>> +}
>> +
>> +static void platform_bgmac_clk_enable(struct bgmac *bgmac, u32 flags)
>> +{
>> +       bgmac_idm_write(bgmac, BCMA_IOCTL,
>> +                       (BCMA_IOCTL_CLK | BCMA_IOCTL_FGC | flags));
>> +       bgmac_idm_read(bgmac, BCMA_IOCTL);
>> +
>> +       bgmac_idm_write(bgmac, BCMA_RESET_CTL, 0);
>> +       bgmac_idm_read(bgmac, BCMA_RESET_CTL);
>> +       udelay(1);
>> +
>> +       bgmac_idm_write(bgmac, BCMA_IOCTL, (BCMA_IOCTL_CLK | flags));
>> +       bgmac_idm_read(bgmac, BCMA_IOCTL);
>> +       udelay(1);
>> +}
>> +
>> +static void platform_bgmac_cco_ctl_maskset(struct bgmac *bgmac, u32
>> offset,
>> +                                          u32 mask, u32 set)
>> +{
>> +       /* This shouldn't be encountered */
>> +       WARN_ON(1);
>> +}
>> +
>> +static u32 platform_bgmac_get_bus_clock(struct bgmac *bgmac)
>> +{
>> +       /* This shouldn't be encountered */
>> +       WARN_ON(1);
>> +
>> +       return 0;
>> +}
>> +
>> +static void platform_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset,
>> +                                        u32 mask, u32 set)
>> +{
>> +       /* This shouldn't be encountered */
>> +       WARN_ON(1);
>> +}
>> +
>> +static int bgmac_probe(struct platform_device *pdev)
>> +{
>> +       struct device_node *np = pdev->dev.of_node;
>> +       struct bgmac *bgmac;
>> +       struct resource regs;
>> +       const u8 *mac_addr;
>> +       int rc;
>> +
>> +       bgmac = kzalloc(sizeof(*bgmac), GFP_KERNEL);
>
>
> The trend has been using devm_xxx based API to let the devm framework deals
> with device resource management. This will help to reduce all the code that
> is currently needed to free the resource below and in the remove function.

Thanks for the review Ray.  This echo's some of Florian's comments to
use devm_*.  I have some changes queued to do exactly what both of you
are requesting.  I'll be pushing it shortly as a "PATCH" instead of an
"RFC".

Thanks,
Jon


>> +       if (!bgmac)
>> +               return -ENOMEM;
>> +
>> +       platform_set_drvdata(pdev, bgmac);
>> +
>> +       /* Set the features of the 4707 family */
>> +       bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>> +       bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
>> +       bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500;
>> +       bgmac->feature_flags |= BGMAC_FEAT_CMDCFG_SR_REV4;
>> +       bgmac->feature_flags |= BGMAC_FEAT_TX_MASK_SETUP;
>> +       bgmac->feature_flags |= BGMAC_FEAT_RX_MASK_SETUP;
>> +
>> +       bgmac->dev = &pdev->dev;
>> +       bgmac->dma_dev = &pdev->dev;
>> +
>> +       mac_addr = of_get_mac_address(np);
>> +       if (mac_addr)
>> +               ether_addr_copy(bgmac->mac_addr, mac_addr);
>> +       else
>> +               dev_warn(&pdev->dev, "MAC address not present in device
>> tree\n");
>> +
>> +       bgmac->irq = platform_get_irq(pdev, 0);
>
>
>> +       if (bgmac->irq < 0) {
>> +               rc = bgmac->irq;
>> +               dev_err(&pdev->dev, "Unable to obtain IRQ\n");
>> +               goto err;
>> +       }
>> +
>> +       rc = of_address_to_resource(np, 0, &regs);
>> +       if (rc < 0) {
>> +               dev_err(&pdev->dev, "Unable to obtain base resource\n");
>> +               goto err;
>> +       }
>> +
>> +       bgmac->plat.base = ioremap(regs.start, resource_size(&regs));
>
>
> Again, there's devm based API to use, and the resource should be marked
> reserved so no one else can remap it.
>
> platform_get_resource
> devm_ioremap_resource
>
>> +       if (!bgmac->plat.base) {
>> +               dev_err(&pdev->dev, "Unable to map base resource\n");
>> +               rc = -ENOMEM;
>> +               goto err;
>> +       }
>> +
>> +       rc = of_address_to_resource(np, 1, &regs);
>> +       if (rc < 0) {
>> +               dev_err(&pdev->dev, "Unable to obtain idm resource\n");
>> +               goto err1;
>> +       }
>> +
>> +       bgmac->plat.idm_base = ioremap(regs.start, resource_size(&regs));
>> +       if (!bgmac->plat.base) {
>> +               dev_err(&pdev->dev, "Unable to map idm resource\n");
>> +               rc = -ENOMEM;
>> +               goto err1;
>> +       }
>
>
> same here
>
>> +
>> +       bgmac->read = platform_bgmac_read;
>> +       bgmac->write = platform_bgmac_write;
>> +       bgmac->idm_read = platform_bgmac_idm_read;
>> +       bgmac->idm_write = platform_bgmac_idm_write;
>> +       bgmac->clk_enabled = platform_bgmac_clk_enabled;
>> +       bgmac->clk_enable = platform_bgmac_clk_enable;
>> +       bgmac->cco_ctl_maskset = platform_bgmac_cco_ctl_maskset;
>> +       bgmac->get_bus_clock = platform_bgmac_get_bus_clock;
>> +       bgmac->cmn_maskset32 = platform_bgmac_cmn_maskset32;
>> +
>> +       rc = bgmac_enet_probe(bgmac);
>> +       if (rc)
>> +               goto err2;
>> +
>> +       return 0;
>> +
>> +err2:
>> +       iounmap(bgmac->plat.idm_base);
>> +err1:
>> +       iounmap(bgmac->plat.base);
>> +err:
>> +       kfree(bgmac);
>
>
> All of the above error handling code can be gone.
>
>> +
>> +       return rc;
>> +}
>> +
>> +static int bgmac_remove(struct platform_device *pdev)
>> +{
>> +       struct bgmac *bgmac = platform_get_drvdata(pdev);
>> +
>> +       bgmac_enet_remove(bgmac);
>> +       iounmap(bgmac->plat.idm_base);
>> +       iounmap(bgmac->plat.base);
>> +       kfree(bgmac);
>
>
> Here too.
>
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct of_device_id bgmac_of_enet_match[] = {
>> +       {.compatible = "brcm,bgmac-enet",},
>> +       {},
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, bgmac_of_enet_match);
>> +
>> +static struct platform_driver bgmac_enet_driver = {
>> +       .driver = {
>> +               .name  = "bgmac-enet",
>> +               .of_match_table = bgmac_of_enet_match,
>> +       },
>> +       .probe = bgmac_probe,
>> +       .remove = bgmac_remove,
>> +};
>> +
>> +module_platform_driver(bgmac_enet_driver);
>> +MODULE_LICENSE("GPL");
>
>
> ...
> ...
>
> Thanks,
>
> Ray

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac
  2016-06-30 18:06   ` Ray Jui
@ 2016-06-30 21:57     ` Jon Mason
  0 siblings, 0 replies; 25+ messages in thread
From: Jon Mason @ 2016-06-30 21:57 UTC (permalink / raw)
  To: Ray Jui
  Cc: Rafał Miłecki, davem, Florian Fainelli, Hauke Mehrtens,
	BCM Kernel Feedback, netdev, linux-kernel

On Thu, Jun 30, 2016 at 2:06 PM, Ray Jui <ray.jui@broadcom.com> wrote:
> Hi Jon,
>
> On 6/28/2016 12:34 PM, Jon Mason wrote:
>>
>> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>> ---
>>  .../devicetree/bindings/net/brcm,bgmac-enet.txt     | 21
>> +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>  create mode 100644
>> Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
>> b/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
>> new file mode 100644
>> index 0000000..efd36d5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/brcm,bgmac-enet.txt
>> @@ -0,0 +1,21 @@
>> +Broadcom GMAC Ethernet Controller Device Tree Bindings
>> +-------------------------------------------------------------
>> +
>> +Required properties:
>> + - compatible: "brcm,bgmac-enet"
>> + - reg:                Address and length of the GMAC registers,
>> +               Address and length of the GMAC IDM registers
>
>
> As we know there will be additional optional register banks required for
> some of the other SoCs that the current driver has not yet supported. In my
> opinion, we should consider to make "reg-names" a mandatory property now and
> map the register blocks based on names.
>
> I think this will help to make our life easier in the future when new
> optional SoC specific register blocks are added, such that we can map the
> register blocks based on names instead of indices, which will change and be
> different among different SoCs and will require much more complex logic in
> the driver to deal with.

I don't have any objection to this.  I'll tweak the patches to do it by name.

>
>> + - interrupts: Interrupt number
>> +
>> +Optional properties:
>> +- mac-address: mac address to be assigned to the device
>> +
>> +Examples:
>> +
>> +gmac0: enet@18022000 {
>> +       compatible = "brcm,bgmac-enet";
>> +       reg = <0x18022000 0x1000>,
>> +             <0x18110000 0x1000>;
>> +       interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
>> +       status = "disabled";
>> +};
>>
>
> Btw, I think Rob Herring should be included in the review for device tree
> binding document changes.

Thanks, I'll add him and the other DT maintainers when I send this out
as a "PATCH" shortly.

Thanks,
Jon

>
> Thanks,
>
> Ray

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2016-06-30 22:04 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-28 19:34 [RFC 0/7] net: ethernet: bgmac: Add platform device support Jon Mason
2016-06-28 19:34 ` [RFC 1/7] net: ethernet: bgmac: change bgmac_* prints to dev_* prints Jon Mason
2016-06-28 19:43   ` Joe Perches
2016-06-29 20:10     ` Jon Mason
2016-06-28 19:34 ` [RFC 2/7] net: ethernet: bgmac: add dma_dev pointer Jon Mason
2016-06-28 19:34 ` [RFC 3/7] net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file Jon Mason
2016-06-28 20:02   ` Andrew Lunn
2016-06-29 14:13   ` Andrew Lunn
2016-06-29 18:35     ` Florian Fainelli
2016-06-29 18:46       ` Andrew Lunn
2016-06-29 20:08         ` Jon Mason
2016-06-29 20:15           ` Andrew Lunn
2016-06-29 20:34             ` Jon Mason
2016-06-28 19:34 ` [RFC 4/7] net: ethernet: bgmac: convert to feature flags Jon Mason
2016-06-28 19:34 ` [RFC 5/7] net: ethernet: bgmac: Add platform device support Jon Mason
2016-06-29 18:51   ` Florian Fainelli
2016-06-30 17:58   ` Ray Jui
2016-06-30 21:55     ` Jon Mason
2016-06-28 19:34 ` [RFC 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac Jon Mason
2016-06-28 20:11   ` Sergei Shtylyov
2016-06-29 18:37   ` Florian Fainelli
2016-06-30 18:06   ` Ray Jui
2016-06-30 21:57     ` Jon Mason
2016-06-28 19:34 ` [RFC 7/7] ARM: dts: NSP: Add bgmac entries Jon Mason
2016-06-29 18:52 ` [RFC 0/7] net: ethernet: bgmac: Add platform device support Florian Fainelli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).