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

[PATCH V2 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      |   6 +-
 drivers/net/ethernet/broadcom/bgmac-platform.c  |   2 +-
 drivers/net/ethernet/broadcom/bgmac.c           |  24 ++++--
 drivers/net/ethernet/broadcom/bgmac.h           |   5 +-
 5 files changed, 72 insertions(+), 73 deletions(-)

-- 
2.11.0

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

[Rafał Miłecki]

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

So far were were allocating struct bgmac in 3 places: platform code,
bcma code and shared bgmac_enet_probe function. The reason for this was
bgmac_enet_probe:
1) Requiring early-filled struct bgmac
2) Calling alloc_etherdev on its own in order to use netdev_priv later

This solution got few drawbacks:
1) Was duplicating allocating code
2) Required copying early-filled struct
3) Resulted in platform/bcma code having access only to unused struct

Solve this situation by simply extracting some probe code into the new
bgmac_alloc function.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Add bgmac_alloc function instead of hacking alloc_etherdev and netdev_priv

Important: this patch depends on:
[PATCH net-next] net: add devm version of alloc_etherdev_mqs function
and is the first user of devm_alloc_etherdev.
---
 drivers/net/ethernet/broadcom/bgmac-bcma.c     |  4 +---
 drivers/net/ethernet/broadcom/bgmac-platform.c |  2 +-
 drivers/net/ethernet/broadcom/bgmac.c          | 24 ++++++++++++++++--------
 drivers/net/ethernet/broadcom/bgmac.h          |  3 ++-
 4 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c b/drivers/net/ethernet/broadcom/bgmac-bcma.c
index 4a4ffc0c4c65..9281abda4026 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -117,12 +117,11 @@ static int bgmac_probe(struct bcma_device *core)
 	u8 *mac;
 	int err;
 
-	bgmac = kzalloc(sizeof(*bgmac), GFP_KERNEL);
+	bgmac = bgmac_alloc(&core->dev);
 	if (!bgmac)
 		return -ENOMEM;
 
 	bgmac->bcma.core = core;
-	bgmac->dev = &core->dev;
 	bgmac->dma_dev = core->dma_dev;
 	bgmac->irq = core->irq;
 
@@ -307,7 +306,6 @@ static int bgmac_probe(struct bcma_device *core)
 err1:
 	bcma_mdio_mii_unregister(bgmac->mii_bus);
 err:
-	kfree(bgmac);
 	bcma_set_drvdata(core, NULL);
 
 	return err;
diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
index 6f736c19872f..805e6ed6c390 100644
--- a/drivers/net/ethernet/broadcom/bgmac-platform.c
+++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
@@ -151,7 +151,7 @@ static int bgmac_probe(struct platform_device *pdev)
 	struct resource *regs;
 	const u8 *mac_addr;
 
-	bgmac = devm_kzalloc(&pdev->dev, sizeof(*bgmac), GFP_KERNEL);
+	bgmac = bgmac_alloc(&pdev->dev);
 	if (!bgmac)
 		return -ENOMEM;
 
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 0e066dc6b8cc..632d4d7b5a5b 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1446,22 +1446,31 @@ int bgmac_phy_connect_direct(struct bgmac *bgmac)
 }
 EXPORT_SYMBOL_GPL(bgmac_phy_connect_direct);
 
-int bgmac_enet_probe(struct bgmac *info)
+struct bgmac *bgmac_alloc(struct device *dev)
 {
 	struct net_device *net_dev;
 	struct bgmac *bgmac;
-	int err;
 
 	/* Allocation and references */
-	net_dev = alloc_etherdev(sizeof(*bgmac));
+	net_dev = devm_alloc_etherdev(dev, sizeof(*bgmac));
 	if (!net_dev)
-		return -ENOMEM;
+		return NULL;
 
 	net_dev->netdev_ops = &bgmac_netdev_ops;
 	net_dev->ethtool_ops = &bgmac_ethtool_ops;
+
 	bgmac = netdev_priv(net_dev);
-	memcpy(bgmac, info, sizeof(*bgmac));
+	bgmac->dev = dev;
 	bgmac->net_dev = net_dev;
+
+	return bgmac;
+}
+
+int bgmac_enet_probe(struct bgmac *bgmac)
+{
+	struct net_device *net_dev = bgmac->net_dev;
+	int err;
+
 	net_dev->irq = bgmac->irq;
 	SET_NETDEV_DEV(net_dev, bgmac->dev);
 
@@ -1488,7 +1497,7 @@ int bgmac_enet_probe(struct bgmac *info)
 	err = bgmac_dma_alloc(bgmac);
 	if (err) {
 		dev_err(bgmac->dev, "Unable to alloc memory for DMA\n");
-		goto err_netdev_free;
+		goto err_out;
 	}
 
 	bgmac->int_mask = BGMAC_IS_ERRMASK | BGMAC_IS_RX | BGMAC_IS_TX_MASK;
@@ -1521,8 +1530,7 @@ int bgmac_enet_probe(struct bgmac *info)
 	phy_disconnect(net_dev->phydev);
 err_dma_free:
 	bgmac_dma_free(bgmac);
-err_netdev_free:
-	free_netdev(net_dev);
+err_out:
 
 	return err;
 }
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 71f493f2451f..dfebaded3b52 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -517,7 +517,8 @@ struct bgmac {
 	int (*phy_connect)(struct bgmac *bgmac);
 };
 
-int bgmac_enet_probe(struct bgmac *info);
+struct bgmac *bgmac_alloc(struct device *dev);
+int bgmac_enet_probe(struct bgmac *bgmac);
 void bgmac_enet_remove(struct bgmac *bgmac);
 void bgmac_adjust_link(struct net_device *net_dev);
 int bgmac_phy_connect_direct(struct bgmac *bgmac);
-- 
2.11.0

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

[kbuild test robot]

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

Hi Rafał,

[auto build test ERROR on net-next/master]
[also build test ERROR on v4.10-rc5 next-20170125]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Rafa-Mi-ecki/net-next-use-one-struct-bgmac-add-PHY-support/20170129-062241
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   drivers/net/ethernet/broadcom/bgmac.c: In function 'bgmac_alloc':
>> drivers/net/ethernet/broadcom/bgmac.c:1455:12: error: implicit declaration of function 'devm_alloc_etherdev' [-Werror=implicit-function-declaration]
     net_dev = devm_alloc_etherdev(dev, sizeof(*bgmac));
               ^~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/broadcom/bgmac.c:1455:10: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     net_dev = devm_alloc_etherdev(dev, sizeof(*bgmac));
             ^
   cc1: some warnings being treated as errors

vim +/devm_alloc_etherdev +1455 drivers/net/ethernet/broadcom/bgmac.c

  1449	struct bgmac *bgmac_alloc(struct device *dev)
  1450	{
  1451		struct net_device *net_dev;
  1452		struct bgmac *bgmac;
  1453	
  1454		/* Allocation and references */
> 1455		net_dev = devm_alloc_etherdev(dev, sizeof(*bgmac));
  1456		if (!net_dev)
  1457			return NULL;
  1458	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 57922 bytes --]

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

[Rafał Miłecki]

On 01/29/2017 12:40 AM, kbuild test robot wrote:
> [auto build test ERROR on net-next/master]
> [also build test ERROR on v4.10-rc5 next-20170125]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Rafa-Mi-ecki/net-next-use-one-struct-bgmac-add-PHY-support/20170129-062241
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386
>
> All error/warnings (new ones prefixed by >>):
>
>    drivers/net/ethernet/broadcom/bgmac.c: In function 'bgmac_alloc':
>>> drivers/net/ethernet/broadcom/bgmac.c:1455:12: error: implicit declaration of function 'devm_alloc_etherdev' [-Werror=implicit-function-declaration]
>      net_dev = devm_alloc_etherdev(dev, sizeof(*bgmac));
>                ^~~~~~~~~~~~~~~~~~~

Thanks, this is expected as this patch depends on
[PATCH net-next] net: add devm version of alloc_etherdev_mqs function

Let's see if net guys will find my devm_alloc_etherdev implementation
acceptable.

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

[Florian Fainelli]

On 01/28/2017 01:08 PM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> So far were were allocating struct bgmac in 3 places: platform code,
> bcma code and shared bgmac_enet_probe function. The reason for this was
> bgmac_enet_probe:
> 1) Requiring early-filled struct bgmac
> 2) Calling alloc_etherdev on its own in order to use netdev_priv later
> 
> This solution got few drawbacks:
> 1) Was duplicating allocating code
> 2) Required copying early-filled struct
> 3) Resulted in platform/bcma code having access only to unused struct
> 
> Solve this situation by simply extracting some probe code into the new
> bgmac_alloc function.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

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

[PATCH V2 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 9281abda4026..5ef60d4f12b4 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -177,7 +177,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 dfebaded3b52..ab2db76e4fb8 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -523,7 +523,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

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

[Florian Fainelli]

On 01/28/2017 01:08 PM, Rafał Miłecki wrote:
> 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>

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

[PATCH V2 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 V2 3/3] net: bgmac: use PHY subsystem for initializing PHY

[Florian Fainelli]

On 01/28/2017 01:08 PM, Rafał Miłecki wrote:
> 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;

That part is clearly initializing the built-in Ethernet switch's PHYs,
and so the natural place for that would be to stick these init values
into the Broadcom PHY driver. When b53-srab/b53_common attaches the
switch, it will scan all of these port's builtin PHYs and bind to an
appropriate PHY driver which could have this initialization as part of
the config_init routine for instance. Right now, we are most likely
using the Generic PHY.

Here are the different PHY IDs you should read from these models if you
want to make a subsequent patch that moves this initialization down to
the Broadcom PHY driver:

5356: 0x03625DA0
5357/53572: 0x03625F00
4749: could either be 0x600D85F0 or the same as 53010 (0x600D8760),
unclear where that product came from... Jon, would you know by chance?
-- 
Florian

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

[Rafał Miłecki]

On 01/29/2017 04:08 AM, Florian Fainelli wrote:
> On 01/28/2017 01:08 PM, Rafał Miłecki wrote:
>> 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;
>
> That part is clearly initializing the built-in Ethernet switch's PHYs,
> and so the natural place for that would be to stick these init values
> into the Broadcom PHY driver. When b53-srab/b53_common attaches the
> switch, it will scan all of these port's builtin PHYs and bind to an
> appropriate PHY driver which could have this initialization as part of
> the config_init routine for instance. Right now, we are most likely
> using the Generic PHY.

I don't think this code is for switch's PHYs. I believe this code is for
wireless access points that have no switch and have Ethernet interface connected
directly to some single-port PHY. I saw 2 or 3 devices like this. They often
also use PoE.


> Here are the different PHY IDs you should read from these models if you
> want to make a subsequent patch that moves this initialization down to
> the Broadcom PHY driver:
>
> 5356: 0x03625DA0
> 5357/53572: 0x03625F00
> 4749: could either be 0x600D85F0 or the same as 53010 (0x600D8760),
> unclear where that product came from... Jon, would you know by chance?

This is very valuable info, thank you! I'll definitely work on this. So far I
tried using bgmac.ko + broadcom.ko on AP device with BCM47186B0 but it doesn't
work for some reason, I'll work on this & keep moving PHY code out of bgmac.

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

[Rafał Miłecki]

On 29 January 2017 at 21:22, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 01/29/2017 12:14 PM, Rafał Miłecki wrote:
>> On 01/29/2017 04:08 AM, Florian Fainelli wrote:
>>> On 01/28/2017 01:08 PM, Rafał Miłecki wrote:
>>>> 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;
>>>
>>> That part is clearly initializing the built-in Ethernet switch's PHYs,
>>> and so the natural place for that would be to stick these init values
>>> into the Broadcom PHY driver. When b53-srab/b53_common attaches the
>>> switch, it will scan all of these port's builtin PHYs and bind to an
>>> appropriate PHY driver which could have this initialization as part of
>>> the config_init routine for instance. Right now, we are most likely
>>> using the Generic PHY.
>>
>> I don't think this code is for switch's PHYs. I believe this code is for
>> wireless access points that have no switch and have Ethernet interface
>> connected
>> directly to some single-port PHY. I saw 2 or 3 devices like this. They
>> often
>> also use PoE.
>
> Humm, built-in PHYs would typically appear as 0-5 on the MDIO bus,
> whereas external PHYs would have a different (and non conflicting)
> address, also, there are restrictions on the Roboswitch devices as to
> where you could wire these external PHYs (to port 5, or 7, 8).

>From BCM47186B0:
[    0.942419] bgmac_bcma bcma0:2: Found PHY addr: 25

>From BCM47189:
[    1.758079] bgmac_bcma bcma0:5: Found PHY addr: 0

I got one more AP device but I bricked it by corrupting NVRAM
(bootloader doesn't start due to that).


My BCM47186B0 seems to not have any switch. It isn't that clear in
BCM47189 case. You may be right, some sources say BCM47189 has
built-in switch so maybe it's indeed BCM54210E connected to switch's
port. On the other hand I'm not using any switch driver on my BCM47189
AP board, so how does it work? Just an accidentally working setup left
by the bootloader?

-- 
Rafał

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

[Florian Fainelli]

Le 01/29/17 à 13:31, Rafał Miłecki a écrit :
> On 29 January 2017 at 21:22, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 01/29/2017 12:14 PM, Rafał Miłecki wrote:
>>> On 01/29/2017 04:08 AM, Florian Fainelli wrote:
>>>> On 01/28/2017 01:08 PM, Rafał Miłecki wrote:
>>>>> 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;
>>>>
>>>> That part is clearly initializing the built-in Ethernet switch's PHYs,
>>>> and so the natural place for that would be to stick these init values
>>>> into the Broadcom PHY driver. When b53-srab/b53_common attaches the
>>>> switch, it will scan all of these port's builtin PHYs and bind to an
>>>> appropriate PHY driver which could have this initialization as part of
>>>> the config_init routine for instance. Right now, we are most likely
>>>> using the Generic PHY.
>>>
>>> I don't think this code is for switch's PHYs. I believe this code is for
>>> wireless access points that have no switch and have Ethernet interface
>>> connected
>>> directly to some single-port PHY. I saw 2 or 3 devices like this. They
>>> often
>>> also use PoE.
>>
>> Humm, built-in PHYs would typically appear as 0-5 on the MDIO bus,
>> whereas external PHYs would have a different (and non conflicting)
>> address, also, there are restrictions on the Roboswitch devices as to
>> where you could wire these external PHYs (to port 5, or 7, 8).
> 
> From BCM47186B0:
> [    0.942419] bgmac_bcma bcma0:2: Found PHY addr: 25
> 
> From BCM47189:
> [    1.758079] bgmac_bcma bcma0:5: Found PHY addr: 0

Address 0 is usually a broadcast address, although with most Broadcom
switches, it just maps to the first port's built-in PHY.

> 
> I got one more AP device but I bricked it by corrupting NVRAM
> (bootloader doesn't start due to that).
> 
> 
> My BCM47186B0 seems to not have any switch. It isn't that clear in
> BCM47189 case. You may be right, some sources say BCM47189 has
> built-in switch so maybe it's indeed BCM54210E connected to switch's
> port. On the other hand I'm not using any switch driver on my BCM47189
> AP board, so how does it work? Just an accidentally working setup left
> by the bootloader?

That's what I suspect is happening yes. Do you have the different FCC
IDs of these routers so maybe looking at pictures of the inside would
tell us what kind of internal vs. external PHYs are populated?
-- 
Florian

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

[Rafał Miłecki]

On 29 January 2017 at 23:36, Florian Fainelli <f.fainelli@gmail.com> wrote:
> Le 01/29/17 à 13:31, Rafał Miłecki a écrit :
>> On 29 January 2017 at 21:22, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>> On 01/29/2017 12:14 PM, Rafał Miłecki wrote:
>>>> On 01/29/2017 04:08 AM, Florian Fainelli wrote:
>>>>> On 01/28/2017 01:08 PM, Rafał Miłecki wrote:
>>>>>> 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;
>>>>>
>>>>> That part is clearly initializing the built-in Ethernet switch's PHYs,
>>>>> and so the natural place for that would be to stick these init values
>>>>> into the Broadcom PHY driver. When b53-srab/b53_common attaches the
>>>>> switch, it will scan all of these port's builtin PHYs and bind to an
>>>>> appropriate PHY driver which could have this initialization as part of
>>>>> the config_init routine for instance. Right now, we are most likely
>>>>> using the Generic PHY.
>>>>
>>>> I don't think this code is for switch's PHYs. I believe this code is for
>>>> wireless access points that have no switch and have Ethernet interface
>>>> connected
>>>> directly to some single-port PHY. I saw 2 or 3 devices like this. They
>>>> often
>>>> also use PoE.
>>>
>>> Humm, built-in PHYs would typically appear as 0-5 on the MDIO bus,
>>> whereas external PHYs would have a different (and non conflicting)
>>> address, also, there are restrictions on the Roboswitch devices as to
>>> where you could wire these external PHYs (to port 5, or 7, 8).
>>
>> From BCM47186B0:
>> [    0.942419] bgmac_bcma bcma0:2: Found PHY addr: 25
>>
>> From BCM47189:
>> [    1.758079] bgmac_bcma bcma0:5: Found PHY addr: 0
>
> Address 0 is usually a broadcast address, although with most Broadcom
> switches, it just maps to the first port's built-in PHY.
>
>>
>> I got one more AP device but I bricked it by corrupting NVRAM
>> (bootloader doesn't start due to that).
>>
>>
>> My BCM47186B0 seems to not have any switch. It isn't that clear in
>> BCM47189 case. You may be right, some sources say BCM47189 has
>> built-in switch so maybe it's indeed BCM54210E connected to switch's
>> port. On the other hand I'm not using any switch driver on my BCM47189
>> AP board, so how does it work? Just an accidentally working setup left
>> by the bootloader?
>
> That's what I suspect is happening yes. Do you have the different FCC
> IDs of these routers so maybe looking at pictures of the inside would
> tell us what kind of internal vs. external PHYs are populated?

I don't. I read all chipset IDs I could see:
1) 2 Winbond RAM chips
2) MXIC MX25L25635F flash
3) BCM47189 SoC
4) B50212E PHY
5) BCM43217 WiFi

There clearly isn't external switch (like BCM43125 or so) but we can't
say (even from the photos) where that B50212E is connected to. It's
most likely built-in switch's port as you said.

-- 
Rafał

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

[Florian Fainelli]

On 01/29/2017 11:02 PM, Rafał Miłecki wrote:
> On 29 January 2017 at 23:36, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> Le 01/29/17 à 13:31, Rafał Miłecki a écrit :
>>> On 29 January 2017 at 21:22, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>> On 01/29/2017 12:14 PM, Rafał Miłecki wrote:
>>>>> On 01/29/2017 04:08 AM, Florian Fainelli wrote:
>>>>>> On 01/28/2017 01:08 PM, Rafał Miłecki wrote:
>>>>>>> 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;
>>>>>>
>>>>>> That part is clearly initializing the built-in Ethernet switch's PHYs,
>>>>>> and so the natural place for that would be to stick these init values
>>>>>> into the Broadcom PHY driver. When b53-srab/b53_common attaches the
>>>>>> switch, it will scan all of these port's builtin PHYs and bind to an
>>>>>> appropriate PHY driver which could have this initialization as part of
>>>>>> the config_init routine for instance. Right now, we are most likely
>>>>>> using the Generic PHY.
>>>>>
>>>>> I don't think this code is for switch's PHYs. I believe this code is for
>>>>> wireless access points that have no switch and have Ethernet interface
>>>>> connected
>>>>> directly to some single-port PHY. I saw 2 or 3 devices like this. They
>>>>> often
>>>>> also use PoE.
>>>>
>>>> Humm, built-in PHYs would typically appear as 0-5 on the MDIO bus,
>>>> whereas external PHYs would have a different (and non conflicting)
>>>> address, also, there are restrictions on the Roboswitch devices as to
>>>> where you could wire these external PHYs (to port 5, or 7, 8).
>>>
>>> From BCM47186B0:
>>> [    0.942419] bgmac_bcma bcma0:2: Found PHY addr: 25
>>>
>>> From BCM47189:
>>> [    1.758079] bgmac_bcma bcma0:5: Found PHY addr: 0
>>
>> Address 0 is usually a broadcast address, although with most Broadcom
>> switches, it just maps to the first port's built-in PHY.
>>
>>>
>>> I got one more AP device but I bricked it by corrupting NVRAM
>>> (bootloader doesn't start due to that).
>>>
>>>
>>> My BCM47186B0 seems to not have any switch. It isn't that clear in
>>> BCM47189 case. You may be right, some sources say BCM47189 has
>>> built-in switch so maybe it's indeed BCM54210E connected to switch's
>>> port. On the other hand I'm not using any switch driver on my BCM47189
>>> AP board, so how does it work? Just an accidentally working setup left
>>> by the bootloader?
>>
>> That's what I suspect is happening yes. Do you have the different FCC
>> IDs of these routers so maybe looking at pictures of the inside would
>> tell us what kind of internal vs. external PHYs are populated?
> 
> I don't. I read all chipset IDs I could see:
> 1) 2 Winbond RAM chips
> 2) MXIC MX25L25635F flash
> 3) BCM47189 SoC
> 4) B50212E PHY
> 5) BCM43217 WiFi
> 
> There clearly isn't external switch (like BCM43125 or so) but we can't
> say (even from the photos) where that B50212E is connected to. It's
> most likely built-in switch's port as you said.

Yes, that's what I suspect, or, there is a second GMAC available to
which we can directly connect this 50212E external PHY. I think Jon has
a 4749 that could be used to confirm/infirm what PHY IDs we read on that
device.
-- 
Florian

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

[Jon Mason]

On Sat, Jan 28, 2017 at 10:08 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
> On 01/28/2017 01:08 PM, Rafał Miłecki wrote:
>> 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;
>
> That part is clearly initializing the built-in Ethernet switch's PHYs,
> and so the natural place for that would be to stick these init values
> into the Broadcom PHY driver. When b53-srab/b53_common attaches the
> switch, it will scan all of these port's builtin PHYs and bind to an
> appropriate PHY driver which could have this initialization as part of
> the config_init routine for instance. Right now, we are most likely
> using the Generic PHY.
>
> Here are the different PHY IDs you should read from these models if you
> want to make a subsequent patch that moves this initialization down to
> the Broadcom PHY driver:
>
> 5356: 0x03625DA0
> 5357/53572: 0x03625F00
> 4749: could either be 0x600D85F0 or the same as 53010 (0x600D8760),
> unclear where that product came from... Jon, would you know by chance?

The 4749 I have has a switch chip (bcm53115, according to the
datasheet).  So, I do not have any idea what PHYs this is referring
to.

> --
> Florian

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

[David Miller]

This entire series is being held up because there is still questions about
whether this is programming the internal PHYs and therefore whether the
code should be placed elsewhere.

Can we please move the discussion forward?

Thank you.

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

[Florian Fainelli]

On 01/31/2017 10:04 AM, David Miller wrote:
> 
> This entire series is being held up because there is still questions about
> whether this is programming the internal PHYs and therefore whether the
> code should be placed elsewhere.
> 
> Can we please move the discussion forward?

The patches are good as-is, we can resolve the PHY programming later on
when we have figured out the details of the 4749 chip we derailed on.

Thanks
-- 
Florian

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

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 31 Jan 2017 10:06:34 -0800

> On 01/31/2017 10:04 AM, David Miller wrote:
>> 
>> This entire series is being held up because there is still questions about
>> whether this is programming the internal PHYs and therefore whether the
>> code should be placed elsewhere.
>> 
>> Can we please move the discussion forward?
> 
> The patches are good as-is, we can resolve the PHY programming later on
> when we have figured out the details of the 4749 chip we derailed on.

Ok great, series applied, thanks.

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

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Tue, 31 Jan 2017 13:14:52 -0500 (EST)

> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Tue, 31 Jan 2017 10:06:34 -0800
> 
>> On 01/31/2017 10:04 AM, David Miller wrote:
>>> 
>>> This entire series is being held up because there is still questions about
>>> whether this is programming the internal PHYs and therefore whether the
>>> code should be placed elsewhere.
>>> 
>>> Can we please move the discussion forward?
>> 
>> The patches are good as-is, we can resolve the PHY programming later on
>> when we have figured out the details of the 4749 chip we derailed on.
> 
> Ok great, series applied, thanks.

Actually, reverted, I get build failures:

[davem@dhcp-10-15-49-210 net-next]$ make -s -j8
  DESCEND  objtool
Setup is 17084 bytes (padded to 17408 bytes).
System is 10211 kB
CRC 68863231
Kernel: arch/x86/boot/bzImage is ready  (#139)
ERROR: "bgmac_alloc" [drivers/net/ethernet/broadcom/bgmac-platform.ko] undefined!
ERROR: "bgmac_alloc" [drivers/net/ethernet/broadcom/bgmac-bcma.ko] undefined!
scripts/Makefile.modpost:91: recipe for target '__modpost' failed
make[1]: *** [__modpost] Error 1
Makefile:1196: recipe for target 'modules' failed
make: *** [modules] Error 2

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

[Rafał Miłecki]

On 31 January 2017 at 19:16, David Miller <davem@davemloft.net> wrote:
> From: David Miller <davem@davemloft.net>
> Date: Tue, 31 Jan 2017 13:14:52 -0500 (EST)
>
>> From: Florian Fainelli <f.fainelli@gmail.com>
>> Date: Tue, 31 Jan 2017 10:06:34 -0800
>>
>>> On 01/31/2017 10:04 AM, David Miller wrote:
>>>>
>>>> This entire series is being held up because there is still questions about
>>>> whether this is programming the internal PHYs and therefore whether the
>>>> code should be placed elsewhere.
>>>>
>>>> Can we please move the discussion forward?
>>>
>>> The patches are good as-is, we can resolve the PHY programming later on
>>> when we have figured out the details of the 4749 chip we derailed on.
>>
>> Ok great, series applied, thanks.
>
> Actually, reverted, I get build failures:
>
> [davem@dhcp-10-15-49-210 net-next]$ make -s -j8
>   DESCEND  objtool
> Setup is 17084 bytes (padded to 17408 bytes).
> System is 10211 kB
> CRC 68863231
> Kernel: arch/x86/boot/bzImage is ready  (#139)
> ERROR: "bgmac_alloc" [drivers/net/ethernet/broadcom/bgmac-platform.ko] undefined!
> ERROR: "bgmac_alloc" [drivers/net/ethernet/broadcom/bgmac-bcma.ko] undefined!
> scripts/Makefile.modpost:91: recipe for target '__modpost' failed
> make[1]: *** [__modpost] Error 1
> Makefile:1196: recipe for target 'modules' failed
> make: *** [modules] Error 2

Sorry! :( I'm working on this. Shit, I should have resent this after
adding devm_alloc_etherdev to let kbuild test it.

-- 
Rafał

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

[Florian Fainelli]

On 01/28/2017 01:08 PM, Rafał Miłecki wrote:
> 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>

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

[PATCH V3 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      |   6 +-
 drivers/net/ethernet/broadcom/bgmac-platform.c  |   2 +-
 drivers/net/ethernet/broadcom/bgmac.c           |  25 ++++--
 drivers/net/ethernet/broadcom/bgmac.h           |   5 +-
 5 files changed, 73 insertions(+), 73 deletions(-)

-- 
2.11.0

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

[Rafał Miłecki]

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

So far were were allocating struct bgmac in 3 places: platform code,
bcma code and shared bgmac_enet_probe function. The reason for this was
bgmac_enet_probe:
1) Requiring early-filled struct bgmac
2) Calling alloc_etherdev on its own in order to use netdev_priv later

This solution got few drawbacks:
1) Was duplicating allocating code
2) Required copying early-filled struct
3) Resulted in platform/bcma code having access only to unused struct

Solve this situation by simply extracting some probe code into the new
bgmac_alloc function.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
V2: Add bgmac_alloc function instead of hacking alloc_etherdev and netdev_priv
V3: Add missing EXPORT_SYMBOL_GPL to allow compiling as modules
---
 drivers/net/ethernet/broadcom/bgmac-bcma.c     |  4 +---
 drivers/net/ethernet/broadcom/bgmac-platform.c |  2 +-
 drivers/net/ethernet/broadcom/bgmac.c          | 25 +++++++++++++++++--------
 drivers/net/ethernet/broadcom/bgmac.h          |  3 ++-
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c b/drivers/net/ethernet/broadcom/bgmac-bcma.c
index 4a4ffc0c4c65..9281abda4026 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -117,12 +117,11 @@ static int bgmac_probe(struct bcma_device *core)
 	u8 *mac;
 	int err;
 
-	bgmac = kzalloc(sizeof(*bgmac), GFP_KERNEL);
+	bgmac = bgmac_alloc(&core->dev);
 	if (!bgmac)
 		return -ENOMEM;
 
 	bgmac->bcma.core = core;
-	bgmac->dev = &core->dev;
 	bgmac->dma_dev = core->dma_dev;
 	bgmac->irq = core->irq;
 
@@ -307,7 +306,6 @@ static int bgmac_probe(struct bcma_device *core)
 err1:
 	bcma_mdio_mii_unregister(bgmac->mii_bus);
 err:
-	kfree(bgmac);
 	bcma_set_drvdata(core, NULL);
 
 	return err;
diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
index 6f736c19872f..805e6ed6c390 100644
--- a/drivers/net/ethernet/broadcom/bgmac-platform.c
+++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
@@ -151,7 +151,7 @@ static int bgmac_probe(struct platform_device *pdev)
 	struct resource *regs;
 	const u8 *mac_addr;
 
-	bgmac = devm_kzalloc(&pdev->dev, sizeof(*bgmac), GFP_KERNEL);
+	bgmac = bgmac_alloc(&pdev->dev);
 	if (!bgmac)
 		return -ENOMEM;
 
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 9122608df16e..fe88126b1e0c 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1446,22 +1446,32 @@ int bgmac_phy_connect_direct(struct bgmac *bgmac)
 }
 EXPORT_SYMBOL_GPL(bgmac_phy_connect_direct);
 
-int bgmac_enet_probe(struct bgmac *info)
+struct bgmac *bgmac_alloc(struct device *dev)
 {
 	struct net_device *net_dev;
 	struct bgmac *bgmac;
-	int err;
 
 	/* Allocation and references */
-	net_dev = alloc_etherdev(sizeof(*bgmac));
+	net_dev = devm_alloc_etherdev(dev, sizeof(*bgmac));
 	if (!net_dev)
-		return -ENOMEM;
+		return NULL;
 
 	net_dev->netdev_ops = &bgmac_netdev_ops;
 	net_dev->ethtool_ops = &bgmac_ethtool_ops;
+
 	bgmac = netdev_priv(net_dev);
-	memcpy(bgmac, info, sizeof(*bgmac));
+	bgmac->dev = dev;
 	bgmac->net_dev = net_dev;
+
+	return bgmac;
+}
+EXPORT_SYMBOL_GPL(bgmac_alloc);
+
+int bgmac_enet_probe(struct bgmac *bgmac)
+{
+	struct net_device *net_dev = bgmac->net_dev;
+	int err;
+
 	net_dev->irq = bgmac->irq;
 	SET_NETDEV_DEV(net_dev, bgmac->dev);
 
@@ -1488,7 +1498,7 @@ int bgmac_enet_probe(struct bgmac *info)
 	err = bgmac_dma_alloc(bgmac);
 	if (err) {
 		dev_err(bgmac->dev, "Unable to alloc memory for DMA\n");
-		goto err_netdev_free;
+		goto err_out;
 	}
 
 	bgmac->int_mask = BGMAC_IS_ERRMASK | BGMAC_IS_RX | BGMAC_IS_TX_MASK;
@@ -1521,8 +1531,7 @@ int bgmac_enet_probe(struct bgmac *info)
 	phy_disconnect(net_dev->phydev);
 err_dma_free:
 	bgmac_dma_free(bgmac);
-err_netdev_free:
-	free_netdev(net_dev);
+err_out:
 
 	return err;
 }
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 71f493f2451f..dfebaded3b52 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -517,7 +517,8 @@ struct bgmac {
 	int (*phy_connect)(struct bgmac *bgmac);
 };
 
-int bgmac_enet_probe(struct bgmac *info);
+struct bgmac *bgmac_alloc(struct device *dev);
+int bgmac_enet_probe(struct bgmac *bgmac);
 void bgmac_enet_remove(struct bgmac *bgmac);
 void bgmac_adjust_link(struct net_device *net_dev);
 int bgmac_phy_connect_direct(struct bgmac *bgmac);
-- 
2.11.0

[PATCH V3 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>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 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 9281abda4026..5ef60d4f12b4 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -177,7 +177,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 dfebaded3b52..ab2db76e4fb8 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -523,7 +523,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 V3 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>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 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 V3 0/3] net-next: use one struct bgmac & add PHY support

[David Miller]

From: Rafał Miłecki <zajec5@gmail.com>
Date: Tue, 31 Jan 2017 19:37:53 +0100

> 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.

Series applied, thanks.