[PATCH 0/3] net-next: use one struct bgmac & add PHY support

[PATCH 0/3] net-next: use one struct bgmac & add PHY support

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

This patchset adds support for initializing PHY using PHY subsystem.
It's required e.g. for wireless access point devices that use bgmac
supported Ethernet device connected to some external PHY.

Implementing this required accessing phydev in bcma specific code which
wasn't possible with core code allocating struct bgmac on its own. This
is why I needed to modify alloc_etherdev usage first.

Rafał Miłecki (3):
  net: bgmac: allocate struct bgmac just once & don't copy it
  net: bgmac: drop struct bcma_mdio we don't need anymore
  net: bgmac: use PHY subsystem for initializing PHY

 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c | 108 +++++++++++-------------
 drivers/net/ethernet/broadcom/bgmac-bcma.c      |   2 +-
 drivers/net/ethernet/broadcom/bgmac-platform.c  |   2 +-
 drivers/net/ethernet/broadcom/bgmac.c           |  24 +++---
 drivers/net/ethernet/broadcom/bgmac.h           |   2 +-
 5 files changed, 64 insertions(+), 74 deletions(-)

-- 
2.11.0

[PATCH 1/3] net: bgmac: allocate struct bgmac just once & don't copy it

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

To share as much code as possible in bgmac we call alloc_etherdev from
bgmac.c which is used by both: platform and bcma code. The easiest
solution was to use it for allocating whole struct bgmac but it doesn't
work well as we already get early-filled struct bgmac as an argument.

So far we were solving this by copying received struct into newly
allocated one. The problem is it means storing 2 allocated structs,
using only 1 of them and non-shared code not having access to it.

This patch solves it by using alloc_etherdev to allocate *pointer* for
the already allocated struct. The only downside of this is we have to be
careful when using netdev_priv.

Another solution was to call alloc_etherdev in platform/bcma specific
code but Jon advised against it due to sharing less code that way.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/net/ethernet/broadcom/bgmac-platform.c |  2 +-
 drivers/net/ethernet/broadcom/bgmac.c          | 24 +++++++++++-------------
 2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
index 6f736c19872f..4fefd1a74fcb 100644
--- a/drivers/net/ethernet/broadcom/bgmac-platform.c
+++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
@@ -98,7 +98,7 @@ static void platform_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset,
 
 static void bgmac_nicpm_speed_set(struct net_device *net_dev)
 {
-	struct bgmac *bgmac = netdev_priv(net_dev);
+	struct bgmac *bgmac = *(struct bgmac **)netdev_priv(net_dev);
 	u32 val;
 
 	if (!bgmac->plat.nicpm_base)
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 0e066dc6b8cc..73d679337903 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -777,7 +777,7 @@ static void bgmac_write_mac_address(struct bgmac *bgmac, u8 *addr)
 
 static void bgmac_set_rx_mode(struct net_device *net_dev)
 {
-	struct bgmac *bgmac = netdev_priv(net_dev);
+	struct bgmac *bgmac = *(struct bgmac **)netdev_priv(net_dev);
 
 	if (net_dev->flags & IFF_PROMISC)
 		bgmac_cmdcfg_maskset(bgmac, ~0, BGMAC_CMDCFG_PROM, true);
@@ -1112,7 +1112,7 @@ static void bgmac_chip_init(struct bgmac *bgmac)
 
 static irqreturn_t bgmac_interrupt(int irq, void *dev_id)
 {
-	struct bgmac *bgmac = netdev_priv(dev_id);
+	struct bgmac *bgmac = *(struct bgmac **)netdev_priv(dev_id);
 
 	u32 int_status = bgmac_read(bgmac, BGMAC_INT_STATUS);
 	int_status &= bgmac->int_mask;
@@ -1161,7 +1161,7 @@ static int bgmac_poll(struct napi_struct *napi, int weight)
 
 static int bgmac_open(struct net_device *net_dev)
 {
-	struct bgmac *bgmac = netdev_priv(net_dev);
+	struct bgmac *bgmac = *(struct bgmac **)netdev_priv(net_dev);
 	int err = 0;
 
 	bgmac_chip_reset(bgmac);
@@ -1191,7 +1191,7 @@ static int bgmac_open(struct net_device *net_dev)
 
 static int bgmac_stop(struct net_device *net_dev)
 {
-	struct bgmac *bgmac = netdev_priv(net_dev);
+	struct bgmac *bgmac = *(struct bgmac **)netdev_priv(net_dev);
 
 	netif_carrier_off(net_dev);
 
@@ -1210,7 +1210,7 @@ static int bgmac_stop(struct net_device *net_dev)
 static netdev_tx_t bgmac_start_xmit(struct sk_buff *skb,
 				    struct net_device *net_dev)
 {
-	struct bgmac *bgmac = netdev_priv(net_dev);
+	struct bgmac *bgmac = *(struct bgmac **)netdev_priv(net_dev);
 	struct bgmac_dma_ring *ring;
 
 	/* No QOS support yet */
@@ -1220,7 +1220,7 @@ static netdev_tx_t bgmac_start_xmit(struct sk_buff *skb,
 
 static int bgmac_set_mac_address(struct net_device *net_dev, void *addr)
 {
-	struct bgmac *bgmac = netdev_priv(net_dev);
+	struct bgmac *bgmac = *(struct bgmac **)netdev_priv(net_dev);
 	int ret;
 
 	ret = eth_prepare_mac_addr_change(net_dev, addr);
@@ -1356,7 +1356,7 @@ static void bgmac_get_strings(struct net_device *dev, u32 stringset,
 static void bgmac_get_ethtool_stats(struct net_device *dev,
 				    struct ethtool_stats *ss, uint64_t *data)
 {
-	struct bgmac *bgmac = netdev_priv(dev);
+	struct bgmac *bgmac = *(struct bgmac **)netdev_priv(dev);
 	const struct bgmac_stat *s;
 	unsigned int i;
 	u64 val;
@@ -1396,7 +1396,7 @@ static const struct ethtool_ops bgmac_ethtool_ops = {
 
 void bgmac_adjust_link(struct net_device *net_dev)
 {
-	struct bgmac *bgmac = netdev_priv(net_dev);
+	struct bgmac *bgmac = *(struct bgmac **)netdev_priv(net_dev);
 	struct phy_device *phy_dev = net_dev->phydev;
 	bool update = false;
 
@@ -1446,21 +1446,19 @@ int bgmac_phy_connect_direct(struct bgmac *bgmac)
 }
 EXPORT_SYMBOL_GPL(bgmac_phy_connect_direct);
 
-int bgmac_enet_probe(struct bgmac *info)
+int bgmac_enet_probe(struct bgmac *bgmac)
 {
 	struct net_device *net_dev;
-	struct bgmac *bgmac;
 	int err;
 
 	/* Allocation and references */
-	net_dev = alloc_etherdev(sizeof(*bgmac));
+	net_dev = alloc_etherdev(sizeof(struct bgmac **));
 	if (!net_dev)
 		return -ENOMEM;
 
 	net_dev->netdev_ops = &bgmac_netdev_ops;
 	net_dev->ethtool_ops = &bgmac_ethtool_ops;
-	bgmac = netdev_priv(net_dev);
-	memcpy(bgmac, info, sizeof(*bgmac));
+	*(struct bgmac **)netdev_priv(net_dev) = bgmac;
 	bgmac->net_dev = net_dev;
 	net_dev->irq = bgmac->irq;
 	SET_NETDEV_DEV(net_dev, bgmac->dev);
-- 
2.11.0

[PATCH 2/3] net: bgmac: drop struct bcma_mdio we don't need anymore

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Adding struct bcma_mdio was a workaround for bcma code not having access
to the struct bgmac used in the core code. Now we don't duplicate this
struct we can just use it internally in bcma code.

This simplifies code & allows access to all bgmac driver details from
all places in bcma code.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c | 98 ++++++++++---------------
 drivers/net/ethernet/broadcom/bgmac-bcma.c      |  2 +-
 drivers/net/ethernet/broadcom/bgmac.h           |  2 +-
 3 files changed, 42 insertions(+), 60 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
index 7c19c8e2bf91..9d9984999dce 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
@@ -12,11 +12,6 @@
 #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)
 {
@@ -37,7 +32,7 @@ static bool bcma_mdio_wait_value(struct bcma_device *core, u16 reg, u32 mask,
  * PHY ops
  **************************************************/
 
-static u16 bcma_mdio_phy_read(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg)
+static u16 bcma_mdio_phy_read(struct bgmac *bgmac, u8 phyaddr, u8 reg)
 {
 	struct bcma_device *core;
 	u16 phy_access_addr;
@@ -56,12 +51,12 @@ static u16 bcma_mdio_phy_read(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg)
 	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;
+	if (bgmac->bcma.core->id.id == BCMA_CORE_4706_MAC_GBIT) {
+		core = bgmac->bcma.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;
+		core = bgmac->bcma.core;
 		phy_access_addr = BGMAC_PHY_ACCESS;
 		phy_ctl_addr = BGMAC_PHY_CNTL;
 	}
@@ -87,7 +82,7 @@ static u16 bcma_mdio_phy_read(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg)
 }
 
 /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphywr */
-static int bcma_mdio_phy_write(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg,
+static int bcma_mdio_phy_write(struct bgmac *bgmac, u8 phyaddr, u8 reg,
 			       u16 value)
 {
 	struct bcma_device *core;
@@ -95,12 +90,12 @@ static int bcma_mdio_phy_write(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg,
 	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;
+	if (bgmac->bcma.core->id.id == BCMA_CORE_4706_MAC_GBIT) {
+		core = bgmac->bcma.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;
+		core = bgmac->bcma.core;
 		phy_access_addr = BGMAC_PHY_ACCESS;
 		phy_ctl_addr = BGMAC_PHY_CNTL;
 	}
@@ -110,8 +105,8 @@ static int bcma_mdio_phy_write(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg,
 	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)
+	bcma_write32(bgmac->bcma.core, BGMAC_INT_STATUS, BGMAC_IS_MDIO);
+	if (bcma_read32(bgmac->bcma.core, BGMAC_INT_STATUS) & BGMAC_IS_MDIO)
 		dev_warn(&core->dev, "Error setting MDIO int\n");
 
 	tmp = BGMAC_PA_START;
@@ -132,39 +127,39 @@ static int bcma_mdio_phy_write(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg,
 }
 
 /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphyinit */
-static void bcma_mdio_phy_init(struct bcma_mdio *bcma_mdio)
+static void bcma_mdio_phy_init(struct bgmac *bgmac)
 {
-	struct bcma_chipinfo *ci = &bcma_mdio->core->bus->chipinfo;
+	struct bcma_chipinfo *ci = &bgmac->bcma.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);
+			bcma_mdio_phy_write(bgmac, i, 0x1f, 0x008b);
+			bcma_mdio_phy_write(bgmac, i, 0x15, 0x0100);
+			bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000f);
+			bcma_mdio_phy_write(bgmac, i, 0x12, 0x2aaa);
+			bcma_mdio_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)) {
-		struct bcma_drv_cc *cc = &bcma_mdio->core->bus->drv_cc;
+		struct bcma_drv_cc *cc = &bgmac->bcma.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);
+			bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000f);
+			bcma_mdio_phy_write(bgmac, i, 0x16, 0x5284);
+			bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000b);
+			bcma_mdio_phy_write(bgmac, i, 0x17, 0x0010);
+			bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000f);
+			bcma_mdio_phy_write(bgmac, i, 0x16, 0x5296);
+			bcma_mdio_phy_write(bgmac, i, 0x17, 0x1073);
+			bcma_mdio_phy_write(bgmac, i, 0x17, 0x9073);
+			bcma_mdio_phy_write(bgmac, i, 0x16, 0x52b6);
+			bcma_mdio_phy_write(bgmac, i, 0x17, 0x9273);
+			bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000b);
 		}
 	}
 }
@@ -172,17 +167,17 @@ static void bcma_mdio_phy_init(struct bcma_mdio *bcma_mdio)
 /* 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;
+	struct bgmac *bgmac = bus->priv;
+	u8 phyaddr = bgmac->phyaddr;
 
-	if (bcma_mdio->phyaddr == BGMAC_PHY_NOREGS)
+	if (phyaddr == BGMAC_PHY_NOREGS)
 		return 0;
 
-	bcma_mdio_phy_write(bcma_mdio, phyaddr, MII_BMCR, BMCR_RESET);
+	bcma_mdio_phy_write(bgmac, 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);
+	if (bcma_mdio_phy_read(bgmac, phyaddr, MII_BMCR) & BMCR_RESET)
+		dev_err(bgmac->dev, "PHY reset failed\n");
+	bcma_mdio_phy_init(bgmac);
 
 	return 0;
 }
@@ -202,16 +197,12 @@ static int bcma_mdio_mii_write(struct mii_bus *bus, int mii_id, int regnum,
 	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 mii_bus *bcma_mdio_mii_register(struct bgmac *bgmac)
 {
-	struct bcma_mdio *bcma_mdio;
+	struct bcma_device *core = bgmac->bcma.core;
 	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;
@@ -221,15 +212,12 @@ struct mii_bus *bcma_mdio_mii_register(struct bcma_device *core, u8 phyaddr)
 	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->priv = bgmac;
 	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;
+	mii_bus->phy_mask = ~(1 << bgmac->phyaddr);
 
 	err = mdiobus_register(mii_bus);
 	if (err) {
@@ -242,23 +230,17 @@ struct mii_bus *bcma_mdio_mii_register(struct bcma_device *core, u8 phyaddr)
 err_free_bus:
 	mdiobus_free(mii_bus);
 err:
-	kfree(bcma_mdio);
 	return ERR_PTR(err);
 }
 EXPORT_SYMBOL_GPL(bcma_mdio_mii_register);
 
 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);
 }
 EXPORT_SYMBOL_GPL(bcma_mdio_mii_unregister);
 
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c b/drivers/net/ethernet/broadcom/bgmac-bcma.c
index 4a4ffc0c4c65..bcd565ba9661 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -178,7 +178,7 @@ static int bgmac_probe(struct bcma_device *core)
 
 	if (!bgmac_is_bcm4707_family(core) &&
 	    !(ci->id == BCMA_CHIP_ID_BCM53573 && core->core_unit == 1)) {
-		mii_bus = bcma_mdio_mii_register(core, bgmac->phyaddr);
+		mii_bus = bcma_mdio_mii_register(bgmac);
 		if (IS_ERR(mii_bus)) {
 			err = PTR_ERR(mii_bus);
 			goto err;
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 71f493f2451f..36b782b4b959 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -522,7 +522,7 @@ void bgmac_enet_remove(struct bgmac *bgmac);
 void bgmac_adjust_link(struct net_device *net_dev);
 int bgmac_phy_connect_direct(struct bgmac *bgmac);
 
-struct mii_bus *bcma_mdio_mii_register(struct bcma_device *core, u8 phyaddr);
+struct mii_bus *bcma_mdio_mii_register(struct bgmac *bgmac);
 void bcma_mdio_mii_unregister(struct mii_bus *mii_bus);
 
 static inline u32 bgmac_read(struct bgmac *bgmac, u16 offset)
-- 
2.11.0

[PATCH 3/3] net: bgmac: use PHY subsystem for initializing PHY

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

This adds support for using bgmac with PHYs supported by standalone PHY
drivers. Having any PHY initialization in bgmac is hacky and shouldn't
be extended but rather removed if anyone has hardware to test it.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
index 9d9984999dce..6ce80cbcb48e 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
@@ -132,6 +132,10 @@ static void bcma_mdio_phy_init(struct bgmac *bgmac)
 	struct bcma_chipinfo *ci = &bgmac->bcma.core->bus->chipinfo;
 	u8 i;
 
+	/* For some legacy hardware we do chipset-based PHY initialization here
+	 * without even detecting PHY ID. It's hacky and should be cleaned as
+	 * soon as someone can test it.
+	 */
 	if (ci->id == BCMA_CHIP_ID_BCM5356) {
 		for (i = 0; i < 5; i++) {
 			bcma_mdio_phy_write(bgmac, i, 0x1f, 0x008b);
@@ -140,6 +144,7 @@ static void bcma_mdio_phy_init(struct bgmac *bgmac)
 			bcma_mdio_phy_write(bgmac, i, 0x12, 0x2aaa);
 			bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000b);
 		}
+		return;
 	}
 	if ((ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg != 10) ||
 	    (ci->id == BCMA_CHIP_ID_BCM4749 && ci->pkg != 10) ||
@@ -161,7 +166,12 @@ static void bcma_mdio_phy_init(struct bgmac *bgmac)
 			bcma_mdio_phy_write(bgmac, i, 0x17, 0x9273);
 			bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000b);
 		}
+		return;
 	}
+
+	/* For all other hw do initialization using PHY subsystem. */
+	if (bgmac->net_dev && bgmac->net_dev->phydev)
+		phy_init_hw(bgmac->net_dev->phydev);
 }
 
 /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphyreset */
-- 
2.11.0

Re: [PATCH 1/3] net: bgmac: allocate struct bgmac just once & don't copy it

[Felix Fietkau]

On 2017-01-27 10:20, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> To share as much code as possible in bgmac we call alloc_etherdev from
> bgmac.c which is used by both: platform and bcma code. The easiest
> solution was to use it for allocating whole struct bgmac but it doesn't
> work well as we already get early-filled struct bgmac as an argument.
> 
> So far we were solving this by copying received struct into newly
> allocated one. The problem is it means storing 2 allocated structs,
> using only 1 of them and non-shared code not having access to it.
> 
> This patch solves it by using alloc_etherdev to allocate *pointer* for
> the already allocated struct. The only downside of this is we have to be
> careful when using netdev_priv.
> 
> Another solution was to call alloc_etherdev in platform/bcma specific
> code but Jon advised against it due to sharing less code that way.
How does that lead to sharing less code?
I find this pointer indirection rather ugly and uncommon, and I think it
would be much cleaner to split the probe into bgmac_enet_alloc and
bgmac_enet_probe (with bgmac_enet_alloc calling alloc_etherdev and doing
basic setup).

- Felix

Re: [PATCH 1/3] net: bgmac: allocate struct bgmac just once & don't copy it

[David Miller]

From: Felix Fietkau <nbd@nbd.name>
Date: Fri, 27 Jan 2017 17:02:33 +0100

> On 2017-01-27 10:20, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>> 
>> To share as much code as possible in bgmac we call alloc_etherdev from
>> bgmac.c which is used by both: platform and bcma code. The easiest
>> solution was to use it for allocating whole struct bgmac but it doesn't
>> work well as we already get early-filled struct bgmac as an argument.
>> 
>> So far we were solving this by copying received struct into newly
>> allocated one. The problem is it means storing 2 allocated structs,
>> using only 1 of them and non-shared code not having access to it.
>> 
>> This patch solves it by using alloc_etherdev to allocate *pointer* for
>> the already allocated struct. The only downside of this is we have to be
>> careful when using netdev_priv.
>> 
>> Another solution was to call alloc_etherdev in platform/bcma specific
>> code but Jon advised against it due to sharing less code that way.
> How does that lead to sharing less code?
> I find this pointer indirection rather ugly and uncommon, and I think it
> would be much cleaner to split the probe into bgmac_enet_alloc and
> bgmac_enet_probe (with bgmac_enet_alloc calling alloc_etherdev and doing
> basic setup).

I agree, it would be so much better if bgmac_probe() and friends
initialized a real bgmac object which was the private of a netdev
struct, then passed that down into bgmac_enet_probe().

Re: [PATCH 1/3] net: bgmac: allocate struct bgmac just once & don't copy it

[Rafał Miłecki]

On 27 January 2017 at 17:14, David Miller <davem@davemloft.net> wrote:
> From: Felix Fietkau <nbd@nbd.name>
> Date: Fri, 27 Jan 2017 17:02:33 +0100
>
>> On 2017-01-27 10:20, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> To share as much code as possible in bgmac we call alloc_etherdev from
>>> bgmac.c which is used by both: platform and bcma code. The easiest
>>> solution was to use it for allocating whole struct bgmac but it doesn't
>>> work well as we already get early-filled struct bgmac as an argument.
>>>
>>> So far we were solving this by copying received struct into newly
>>> allocated one. The problem is it means storing 2 allocated structs,
>>> using only 1 of them and non-shared code not having access to it.
>>>
>>> This patch solves it by using alloc_etherdev to allocate *pointer* for
>>> the already allocated struct. The only downside of this is we have to be
>>> careful when using netdev_priv.
>>>
>>> Another solution was to call alloc_etherdev in platform/bcma specific
>>> code but Jon advised against it due to sharing less code that way.
>> How does that lead to sharing less code?
>> I find this pointer indirection rather ugly and uncommon, and I think it
>> would be much cleaner to split the probe into bgmac_enet_alloc and
>> bgmac_enet_probe (with bgmac_enet_alloc calling alloc_etherdev and doing
>> basic setup).
>
> I agree, it would be so much better if bgmac_probe() and friends
> initialized a real bgmac object which was the private of a netdev
> struct, then passed that down into bgmac_enet_probe().

I'll work on V2, thanks.

-- 
Rafał