netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] b44: add support for external PHY
@ 2013-12-15 18:41 Hauke Mehrtens
  2013-12-15 18:41 ` [PATCH 1/8] b44: cheek register instead of PHY address to detect " Hauke Mehrtens
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Hauke Mehrtens @ 2013-12-15 18:41 UTC (permalink / raw)
  To: davem; +Cc: zambrano, netdev, Hauke Mehrtens

This adds support for an external phy connected to the mac controlled 
by b44. This is used on home routers of the BCM47xx line where this MAC 
core was used and is contended to an external switch core through MII. 
These patches are in OpenWrt for some time and are tested by different 
users with different devices.

Hauke Mehrtens (8):
  b44: cheek register instead of PHY address to detect external PHY
  b44: rename B44_PHY_ADDR_NO_PHY to B44_PHY_ADDR_NO_LOCAL_PHY
  b44: abort when no PHY is available at all
  b44: rename b44_mii_{read,write} to b44_mdio_{read,write}_mii
  b44: add phylib support
  b44: activate PHY when MAC is off
  b44: do not set PHY address to 30 for every ext PHY
  b44: add dummy PHY device if we do not find any

 drivers/net/ethernet/broadcom/Kconfig |    1 +
 drivers/net/ethernet/broadcom/b44.c   |  254 ++++++++++++++++++++++++++++++---
 drivers/net/ethernet/broadcom/b44.h   |   15 +-
 3 files changed, 244 insertions(+), 26 deletions(-)

-- 
1.7.10.4

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

* [PATCH 1/8] b44: cheek register instead of PHY address to detect external PHY
  2013-12-15 18:41 [PATCH 0/8] b44: add support for external PHY Hauke Mehrtens
@ 2013-12-15 18:41 ` Hauke Mehrtens
  2013-12-15 20:04   ` Sergei Shtylyov
  2013-12-15 18:41 ` [PATCH 2/8] b44: rename B44_PHY_ADDR_NO_PHY to B44_PHY_ADDR_NO_LOCAL_PHY Hauke Mehrtens
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Hauke Mehrtens @ 2013-12-15 18:41 UTC (permalink / raw)
  To: davem; +Cc: zambrano, netdev, Hauke Mehrtens

The Ethernet core supported by b44 supports an internal PHY integrated
into the mac core, which is supported by the b44 driver and an external
PHY to which the mac core is connected. This external PHY could be a
switch connected through MII, which is often the case when this core is
used on home routers. The usage of an external PHY was assumed when the
PHY address 30 was used and an internal PHY was assumed when the PHY
address was different. To verify that b44_phy_reset() was called and
checked if it worked, otherwise PHY address 30 was assumed, an external
PHY. It is better to check the register which says which PHY is
connected to the MAC instead of checking the PHY address.

This also changes B44_FLAG_INTERNAL_PHY to B44_FLAG_EXTERNAL_PHY, it is
easier to check.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/b44.c |   18 +++++++++---------
 drivers/net/ethernet/broadcom/b44.h |    2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index 90e54d5..3c7909e 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -284,7 +284,7 @@ static int __b44_writephy(struct b44 *bp, int phy_addr, int reg, u32 val)
 
 static inline int b44_readphy(struct b44 *bp, int reg, u32 *val)
 {
-	if (bp->phy_addr == B44_PHY_ADDR_NO_PHY)
+	if (bp->flags & B44_FLAG_EXTERNAL_PHY)
 		return 0;
 
 	return __b44_readphy(bp, bp->phy_addr, reg, val);
@@ -292,7 +292,7 @@ static inline int b44_readphy(struct b44 *bp, int reg, u32 *val)
 
 static inline int b44_writephy(struct b44 *bp, int reg, u32 val)
 {
-	if (bp->phy_addr == B44_PHY_ADDR_NO_PHY)
+	if (bp->flags & B44_FLAG_EXTERNAL_PHY)
 		return 0;
 
 	return __b44_writephy(bp, bp->phy_addr, reg, val);
@@ -321,7 +321,7 @@ static int b44_phy_reset(struct b44 *bp)
 	u32 val;
 	int err;
 
-	if (bp->phy_addr == B44_PHY_ADDR_NO_PHY)
+	if (bp->flags & B44_FLAG_EXTERNAL_PHY)
 		return 0;
 	err = b44_writephy(bp, MII_BMCR, BMCR_RESET);
 	if (err)
@@ -423,7 +423,7 @@ static int b44_setup_phy(struct b44 *bp)
 
 	b44_wap54g10_workaround(bp);
 
-	if (bp->phy_addr == B44_PHY_ADDR_NO_PHY)
+	if (bp->flags & B44_FLAG_EXTERNAL_PHY)
 		return 0;
 	if ((err = b44_readphy(bp, B44_MII_ALEDCTRL, &val)) != 0)
 		goto out;
@@ -521,7 +521,7 @@ static void b44_check_phy(struct b44 *bp)
 {
 	u32 bmsr, aux;
 
-	if (bp->phy_addr == B44_PHY_ADDR_NO_PHY) {
+	if (bp->flags & B44_FLAG_EXTERNAL_PHY) {
 		bp->flags |= B44_FLAG_100_BASE_T;
 		bp->flags |= B44_FLAG_FULL_DUPLEX;
 		if (!netif_carrier_ok(bp->dev)) {
@@ -1315,7 +1315,7 @@ static void b44_chip_reset(struct b44 *bp, int reset_kind)
 	if (!(br32(bp, B44_DEVCTRL) & DEVCTRL_IPP)) {
 		bw32(bp, B44_ENET_CTRL, ENET_CTRL_EPSEL);
 		br32(bp, B44_ENET_CTRL);
-		bp->flags &= ~B44_FLAG_INTERNAL_PHY;
+		bp->flags |= B44_FLAG_EXTERNAL_PHY;
 	} else {
 		u32 val = br32(bp, B44_DEVCTRL);
 
@@ -1324,7 +1324,7 @@ static void b44_chip_reset(struct b44 *bp, int reset_kind)
 			br32(bp, B44_DEVCTRL);
 			udelay(100);
 		}
-		bp->flags |= B44_FLAG_INTERNAL_PHY;
+		bp->flags &= ~B44_FLAG_EXTERNAL_PHY;
 	}
 }
 
@@ -1828,8 +1828,8 @@ static int b44_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 		DUPLEX_FULL : DUPLEX_HALF;
 	cmd->port = 0;
 	cmd->phy_address = bp->phy_addr;
-	cmd->transceiver = (bp->flags & B44_FLAG_INTERNAL_PHY) ?
-		XCVR_INTERNAL : XCVR_EXTERNAL;
+	cmd->transceiver = (bp->flags & B44_FLAG_EXTERNAL_PHY) ?
+		XCVR_EXTERNAL : XCVR_INTERNAL;
 	cmd->autoneg = (bp->flags & B44_FLAG_FORCE_LINK) ?
 		AUTONEG_DISABLE : AUTONEG_ENABLE;
 	if (cmd->autoneg == AUTONEG_ENABLE)
diff --git a/drivers/net/ethernet/broadcom/b44.h b/drivers/net/ethernet/broadcom/b44.h
index 8993d72..8ed7d6b 100644
--- a/drivers/net/ethernet/broadcom/b44.h
+++ b/drivers/net/ethernet/broadcom/b44.h
@@ -376,7 +376,7 @@ struct b44 {
 #define B44_FLAG_ADV_10FULL	0x02000000
 #define B44_FLAG_ADV_100HALF	0x04000000
 #define B44_FLAG_ADV_100FULL	0x08000000
-#define B44_FLAG_INTERNAL_PHY	0x10000000
+#define B44_FLAG_EXTERNAL_PHY	0x10000000
 #define B44_FLAG_RX_RING_HACK	0x20000000
 #define B44_FLAG_TX_RING_HACK	0x40000000
 #define B44_FLAG_WOL_ENABLE	0x80000000
-- 
1.7.10.4

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

* [PATCH 2/8] b44: rename B44_PHY_ADDR_NO_PHY to B44_PHY_ADDR_NO_LOCAL_PHY
  2013-12-15 18:41 [PATCH 0/8] b44: add support for external PHY Hauke Mehrtens
  2013-12-15 18:41 ` [PATCH 1/8] b44: cheek register instead of PHY address to detect " Hauke Mehrtens
@ 2013-12-15 18:41 ` Hauke Mehrtens
  2013-12-16 15:51   ` Ben Hutchings
  2013-12-16 18:40   ` Florian Fainelli
  2013-12-15 18:42 ` [PATCH 3/8] b44: abort when no PHY is available at all Hauke Mehrtens
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 17+ messages in thread
From: Hauke Mehrtens @ 2013-12-15 18:41 UTC (permalink / raw)
  To: davem; +Cc: zambrano, netdev, Hauke Mehrtens

The PHY address 30 means there is no local PHY, but there could be an
external PHY like a switch connected via MII. This is the case on most
embedded home routers where this driver is used.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/b44.c |    2 +-
 drivers/net/ethernet/broadcom/b44.h |    6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index 3c7909e..fce36dd 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -2237,7 +2237,7 @@ static int b44_init_one(struct ssb_device *sdev,
 
 	/* do a phy reset to test if there is an active phy */
 	if (b44_phy_reset(bp) < 0)
-		bp->phy_addr = B44_PHY_ADDR_NO_PHY;
+		bp->phy_addr = B44_PHY_ADDR_NO_LOACL_PHY;
 
 	netdev_info(dev, "%s %pM\n", DRV_DESCRIPTION, dev->dev_addr);
 
diff --git a/drivers/net/ethernet/broadcom/b44.h b/drivers/net/ethernet/broadcom/b44.h
index 8ed7d6b..ade80d6 100644
--- a/drivers/net/ethernet/broadcom/b44.h
+++ b/drivers/net/ethernet/broadcom/b44.h
@@ -280,9 +280,9 @@ struct ring_info {
 	dma_addr_t	mapping;
 };
 
-#define B44_MCAST_TABLE_SIZE	32
-#define B44_PHY_ADDR_NO_PHY	30
-#define B44_MDC_RATIO		5000000
+#define B44_MCAST_TABLE_SIZE		32
+#define B44_PHY_ADDR_NO_LOACL_PHY	30 /* no local phy regs */
+#define B44_MDC_RATIO			5000000
 
 #define	B44_STAT_REG_DECLARE		\
 	_B44(tx_good_octets)		\
-- 
1.7.10.4

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

* [PATCH 3/8] b44: abort when no PHY is available at all
  2013-12-15 18:41 [PATCH 0/8] b44: add support for external PHY Hauke Mehrtens
  2013-12-15 18:41 ` [PATCH 1/8] b44: cheek register instead of PHY address to detect " Hauke Mehrtens
  2013-12-15 18:41 ` [PATCH 2/8] b44: rename B44_PHY_ADDR_NO_PHY to B44_PHY_ADDR_NO_LOCAL_PHY Hauke Mehrtens
@ 2013-12-15 18:42 ` Hauke Mehrtens
  2013-12-15 18:42 ` [PATCH 4/8] b44: rename b44_mii_{read,write} to b44_mdio_{read,write}_mii Hauke Mehrtens
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Hauke Mehrtens @ 2013-12-15 18:42 UTC (permalink / raw)
  To: davem; +Cc: zambrano, netdev, Hauke Mehrtens

When the phy address is 31, this means that there is no PHY connected
to this MAC at all, no internal and no external PHY. Reading these PHY
registers causes a system reset on some routers.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/b44.c |    6 ++++++
 drivers/net/ethernet/broadcom/b44.h |    1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index fce36dd..6cffe65 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -2206,6 +2206,12 @@ static int b44_init_one(struct ssb_device *sdev,
 		goto err_out_powerdown;
 	}
 
+	if (bp->phy_addr == B44_PHY_ADDR_NO_PHY) {
+		dev_err(sdev->dev, "No PHY present on this MAC, aborting\n");
+		err = -ENODEV;
+		goto err_out_powerdown;
+	}
+
 	bp->mii_if.dev = dev;
 	bp->mii_if.mdio_read = b44_mii_read;
 	bp->mii_if.mdio_write = b44_mii_write;
diff --git a/drivers/net/ethernet/broadcom/b44.h b/drivers/net/ethernet/broadcom/b44.h
index ade80d6..e3ee9ca2 100644
--- a/drivers/net/ethernet/broadcom/b44.h
+++ b/drivers/net/ethernet/broadcom/b44.h
@@ -282,6 +282,7 @@ struct ring_info {
 
 #define B44_MCAST_TABLE_SIZE		32
 #define B44_PHY_ADDR_NO_LOACL_PHY	30 /* no local phy regs */
+#define B44_PHY_ADDR_NO_PHY		31 /* no phy present at all */
 #define B44_MDC_RATIO			5000000
 
 #define	B44_STAT_REG_DECLARE		\
-- 
1.7.10.4

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

* [PATCH 4/8] b44: rename b44_mii_{read,write} to b44_mdio_{read,write}_mii
  2013-12-15 18:41 [PATCH 0/8] b44: add support for external PHY Hauke Mehrtens
                   ` (2 preceding siblings ...)
  2013-12-15 18:42 ` [PATCH 3/8] b44: abort when no PHY is available at all Hauke Mehrtens
@ 2013-12-15 18:42 ` Hauke Mehrtens
  2013-12-15 18:42 ` [PATCH 5/8] b44: add phylib support Hauke Mehrtens
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Hauke Mehrtens @ 2013-12-15 18:42 UTC (permalink / raw)
  To: davem; +Cc: zambrano, netdev, Hauke Mehrtens

The next patch will add these functions for phylib, and we should
rename the old ones before. This now indicates that these functions are
used for the mdio registers and on the mii interface.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/b44.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index 6cffe65..c19319c 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -299,7 +299,7 @@ static inline int b44_writephy(struct b44 *bp, int reg, u32 val)
 }
 
 /* miilib interface */
-static int b44_mii_read(struct net_device *dev, int phy_id, int location)
+static int b44_mdio_read_mii(struct net_device *dev, int phy_id, int location)
 {
 	u32 val;
 	struct b44 *bp = netdev_priv(dev);
@@ -309,8 +309,8 @@ static int b44_mii_read(struct net_device *dev, int phy_id, int location)
 	return val;
 }
 
-static void b44_mii_write(struct net_device *dev, int phy_id, int location,
-			 int val)
+static void b44_mdio_write_mii(struct net_device *dev, int phy_id, int location,
+			       int val)
 {
 	struct b44 *bp = netdev_priv(dev);
 	__b44_writephy(bp, phy_id, location, val);
@@ -2213,8 +2213,8 @@ static int b44_init_one(struct ssb_device *sdev,
 	}
 
 	bp->mii_if.dev = dev;
-	bp->mii_if.mdio_read = b44_mii_read;
-	bp->mii_if.mdio_write = b44_mii_write;
+	bp->mii_if.mdio_read = b44_mdio_read_mii;
+	bp->mii_if.mdio_write = b44_mdio_write_mii;
 	bp->mii_if.phy_id = bp->phy_addr;
 	bp->mii_if.phy_id_mask = 0x1f;
 	bp->mii_if.reg_num_mask = 0x1f;
-- 
1.7.10.4

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

* [PATCH 5/8] b44: add phylib support
  2013-12-15 18:41 [PATCH 0/8] b44: add support for external PHY Hauke Mehrtens
                   ` (3 preceding siblings ...)
  2013-12-15 18:42 ` [PATCH 4/8] b44: rename b44_mii_{read,write} to b44_mdio_{read,write}_mii Hauke Mehrtens
@ 2013-12-15 18:42 ` Hauke Mehrtens
  2013-12-15 18:42 ` [PATCH 6/8] b44: activate PHY when MAC is off Hauke Mehrtens
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Hauke Mehrtens @ 2013-12-15 18:42 UTC (permalink / raw)
  To: davem; +Cc: zambrano, netdev, Hauke Mehrtens

Most of the older home routers based on the Broadcom BCM47XX SoC series
are using a MAC that is supported by b44. On most of these routers not
the internal PHY of this MAC core is used, but a switch sometimes on an
external chip or integrated into the same SoC as the Ethernet core.
For this switch a special PHY driver is needed which should not be
integrated into b44 as the same switches are also used by other
Broadcom home networking SoCs which are using different Ethernet MAC
drivers. This was tested with the b53 switch driver which is currently
on its way to mainline.

If the internal PHY is not used, b44 will now search on the MDIO bus
for a phy and use the Linux phylib subsystem to register a driver.
Support for the internal PHY must stay here, because there are some
device which are suing the internal phy.

With this patch we scan the mdio bus when the sprom or nvram says that
the PHY address is 30, if a PHY was found at this address b44 uses it.

This was tested with a BCM4704, BCM4712 and BCM5354.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/Kconfig |    1 +
 drivers/net/ethernet/broadcom/b44.c   |  193 ++++++++++++++++++++++++++++++++-
 drivers/net/ethernet/broadcom/b44.h   |    3 +
 3 files changed, 192 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index 2fa5b86..3f97d9f 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -23,6 +23,7 @@ config B44
 	depends on SSB_POSSIBLE && HAS_DMA
 	select SSB
 	select MII
+	select PHYLIB
 	---help---
 	  If you have a network (Ethernet) controller of this type, say Y
 	  or M and read the Ethernet-HOWTO, available from
diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index c19319c..7aa720e 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2006 Felix Fietkau (nbd@openwrt.org)
  * Copyright (C) 2006 Broadcom Corporation.
  * Copyright (C) 2007 Michael Buesch <m@bues.ch>
+ * Copyright (C) 2013 Hauke Mehrtens <hauke@hauke-m.de>
  *
  * Distribute under GPL.
  */
@@ -29,6 +30,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/ssb/ssb.h>
 #include <linux/slab.h>
+#include <linux/phy.h>
 
 #include <asm/uaccess.h>
 #include <asm/io.h>
@@ -316,6 +318,23 @@ static void b44_mdio_write_mii(struct net_device *dev, int phy_id, int location,
 	__b44_writephy(bp, phy_id, location, val);
 }
 
+static int b44_mdio_read_phylib(struct mii_bus *bus, int phy_id, int location)
+{
+	u32 val;
+	struct b44 *bp = bus->priv;
+	int rc = __b44_readphy(bp, phy_id, location, &val);
+	if (rc)
+		return 0xffffffff;
+	return val;
+}
+
+static int b44_mdio_write_phylib(struct mii_bus *bus, int phy_id, int location,
+				 u16 val)
+{
+	struct b44 *bp = bus->priv;
+	return __b44_writephy(bp, phy_id, location, val);
+}
+
 static int b44_phy_reset(struct b44 *bp)
 {
 	u32 val;
@@ -523,10 +542,12 @@ static void b44_check_phy(struct b44 *bp)
 
 	if (bp->flags & B44_FLAG_EXTERNAL_PHY) {
 		bp->flags |= B44_FLAG_100_BASE_T;
-		bp->flags |= B44_FLAG_FULL_DUPLEX;
 		if (!netif_carrier_ok(bp->dev)) {
 			u32 val = br32(bp, B44_TX_CTRL);
-			val |= TX_CTRL_DUPLEX;
+			if (bp->flags & B44_FLAG_FULL_DUPLEX)
+				val |= TX_CTRL_DUPLEX;
+			else
+				val &= ~TX_CTRL_DUPLEX;
 			bw32(bp, B44_TX_CTRL, val);
 			netif_carrier_on(bp->dev);
 			b44_link_report(bp);
@@ -1805,6 +1826,11 @@ static int b44_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 {
 	struct b44 *bp = netdev_priv(dev);
 
+	if (bp->flags & B44_FLAG_EXTERNAL_PHY) {
+		BUG_ON(!bp->phydev);
+		return phy_ethtool_gset(bp->phydev, cmd);
+	}
+
 	cmd->supported = (SUPPORTED_Autoneg);
 	cmd->supported |= (SUPPORTED_100baseT_Half |
 			  SUPPORTED_100baseT_Full |
@@ -1846,7 +1872,23 @@ static int b44_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 static int b44_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 {
 	struct b44 *bp = netdev_priv(dev);
-	u32 speed = ethtool_cmd_speed(cmd);
+	u32 speed;
+	int ret;
+
+	if (bp->flags & B44_FLAG_EXTERNAL_PHY) {
+		BUG_ON(!bp->phydev);
+		spin_lock_irq(&bp->lock);
+		if (netif_running(dev))
+			b44_setup_phy(bp);
+
+		ret = phy_ethtool_sset(bp->phydev, cmd);
+
+		spin_unlock_irq(&bp->lock);
+
+		return ret;
+	}
+
+	speed = ethtool_cmd_speed(cmd);
 
 	/* We do not support gigabit. */
 	if (cmd->autoneg == AUTONEG_ENABLE) {
@@ -2076,7 +2118,6 @@ static const struct ethtool_ops b44_ethtool_ops = {
 
 static int b44_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 {
-	struct mii_ioctl_data *data = if_mii(ifr);
 	struct b44 *bp = netdev_priv(dev);
 	int err = -EINVAL;
 
@@ -2084,7 +2125,12 @@ static int b44_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 		goto out;
 
 	spin_lock_irq(&bp->lock);
-	err = generic_mii_ioctl(&bp->mii_if, data, cmd, NULL);
+	if (bp->flags & B44_FLAG_EXTERNAL_PHY) {
+		BUG_ON(!bp->phydev);
+		err = phy_mii_ioctl(bp->phydev, ifr, cmd);
+	} else {
+		err = generic_mii_ioctl(&bp->mii_if, if_mii(ifr), cmd, NULL);
+	}
 	spin_unlock_irq(&bp->lock);
 out:
 	return err;
@@ -2146,6 +2192,130 @@ static const struct net_device_ops b44_netdev_ops = {
 #endif
 };
 
+static void b44_adjust_link(struct net_device *dev)
+{
+	struct b44 *bp = netdev_priv(dev);
+	struct phy_device *phydev = bp->phydev;
+	bool status_changed = 0;
+
+	BUG_ON(!phydev);
+
+	if (bp->old_link != phydev->link) {
+		status_changed = 1;
+		bp->old_link = phydev->link;
+	}
+
+	/* reflect duplex change */
+	if (phydev->link) {
+		if ((phydev->duplex == DUPLEX_HALF) &&
+		    (bp->flags & B44_FLAG_FULL_DUPLEX)) {
+			status_changed = 1;
+			bp->flags &= ~B44_FLAG_FULL_DUPLEX;
+		} else if ((phydev->duplex == DUPLEX_FULL) &&
+			   !(bp->flags & B44_FLAG_FULL_DUPLEX)) {
+			status_changed = 1;
+			bp->flags |= B44_FLAG_FULL_DUPLEX;
+		}
+	}
+
+	if (status_changed) {
+		b44_check_phy(bp);
+		phy_print_status(phydev);
+	}
+}
+
+static int b44_register_phy_one(struct b44 *bp)
+{
+	struct mii_bus *mii_bus;
+	struct ssb_device *sdev = bp->sdev;
+	struct phy_device *phydev;
+	int err;
+
+	mii_bus = mdiobus_alloc();
+	if (!mii_bus) {
+		dev_err(sdev->dev, "mdiobus_alloc() failed\n");
+		err = -ENOMEM;
+		goto err_out;
+	}
+
+	mii_bus->priv = bp;
+	mii_bus->read = b44_mdio_read_phylib;
+	mii_bus->write = b44_mdio_write_phylib;
+	mii_bus->name = "b44_eth_mii";
+	mii_bus->parent = sdev->dev;
+	mii_bus->phy_mask = ~(1 << bp->phy_addr);
+	snprintf(mii_bus->id, MII_BUS_ID_SIZE, "%x", instance);
+	mii_bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
+	if (!mii_bus->irq) {
+		dev_err(sdev->dev, "mii_bus irq allocation failed\n");
+		err = -ENOMEM;
+		goto err_out_mdiobus;
+	}
+
+	memset(mii_bus->irq, PHY_POLL, sizeof(int) * PHY_MAX_ADDR);
+
+	bp->mii_bus = mii_bus;
+
+	err = mdiobus_register(mii_bus);
+	if (err) {
+		dev_err(sdev->dev, "failed to register MII bus\n");
+		goto err_out_mdiobus_irq;
+	}
+
+	phydev = bp->mii_bus->phy_map[bp->phy_addr];
+	if (!phydev) {
+		dev_err(sdev->dev, "could not find PHY at %i\n", bp->phy_addr);
+		err = -ENODEV;
+		goto err_out_mdiobus_unregister;
+	}
+
+	err = phy_connect_direct(bp->dev, phydev, &b44_adjust_link,
+				 PHY_INTERFACE_MODE_MII);
+	if (err < 0) {
+		dev_err(sdev->dev, "could not attach PHY at %i\n",
+			bp->phy_addr);
+		goto err_out_mdiobus_unregister;
+	}
+
+	/* mask with MAC supported features */
+	phydev->supported &= (SUPPORTED_100baseT_Half |
+			      SUPPORTED_100baseT_Full |
+			      SUPPORTED_Autoneg |
+			      SUPPORTED_MII);
+	phydev->advertising = phydev->supported;
+
+	bp->phydev = phydev;
+	bp->old_link = 0;
+	bp->phy_addr = phydev->addr;
+
+	dev_info(sdev->dev, "attached PHY driver [%s] (mii_bus:phy_addr=%s)\n",
+		 phydev->drv->name, dev_name(&phydev->dev));
+
+	return 0;
+
+err_out_mdiobus_unregister:
+	mdiobus_unregister(mii_bus);
+
+err_out_mdiobus_irq:
+	kfree(mii_bus->irq);
+
+err_out_mdiobus:
+	mdiobus_free(mii_bus);
+
+err_out:
+	return err;
+}
+
+static void b44_unregister_phy_one(struct b44 *bp)
+{
+	struct mii_bus *mii_bus = bp->mii_bus;
+
+	phy_disconnect(bp->phydev);
+	mdiobus_unregister(mii_bus);
+	kfree(mii_bus->irq);
+	mdiobus_free(mii_bus);
+}
+
 static int b44_init_one(struct ssb_device *sdev,
 			const struct ssb_device_id *ent)
 {
@@ -2245,10 +2415,20 @@ static int b44_init_one(struct ssb_device *sdev,
 	if (b44_phy_reset(bp) < 0)
 		bp->phy_addr = B44_PHY_ADDR_NO_LOACL_PHY;
 
+	if (bp->flags & B44_FLAG_EXTERNAL_PHY) {
+		err = b44_register_phy_one(bp);
+		if (err) {
+			dev_err(sdev->dev, "Cannot register PHY, aborting\n");
+			goto err_out_unregister_netdev;
+		}
+	}
+
 	netdev_info(dev, "%s %pM\n", DRV_DESCRIPTION, dev->dev_addr);
 
 	return 0;
 
+err_out_unregister_netdev:
+	unregister_netdev(dev);
 err_out_powerdown:
 	ssb_bus_may_powerdown(sdev->bus);
 
@@ -2262,8 +2442,11 @@ out:
 static void b44_remove_one(struct ssb_device *sdev)
 {
 	struct net_device *dev = ssb_get_drvdata(sdev);
+	struct b44 *bp = netdev_priv(dev);
 
 	unregister_netdev(dev);
+	if (bp->flags & B44_FLAG_EXTERNAL_PHY)
+		b44_unregister_phy_one(bp);
 	ssb_device_disable(sdev, 0);
 	ssb_bus_may_powerdown(sdev->bus);
 	free_netdev(dev);
diff --git a/drivers/net/ethernet/broadcom/b44.h b/drivers/net/ethernet/broadcom/b44.h
index e3ee9ca2..c5ec9b4 100644
--- a/drivers/net/ethernet/broadcom/b44.h
+++ b/drivers/net/ethernet/broadcom/b44.h
@@ -397,6 +397,9 @@ struct b44 {
 	u32			tx_pending;
 	u8			phy_addr;
 	u8			force_copybreak;
+	struct phy_device	*phydev;
+	struct mii_bus		*mii_bus;
+	int			old_link;
 	struct mii_if_info	mii_if;
 };
 
-- 
1.7.10.4

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

* [PATCH 6/8] b44: activate PHY when MAC is off
  2013-12-15 18:41 [PATCH 0/8] b44: add support for external PHY Hauke Mehrtens
                   ` (4 preceding siblings ...)
  2013-12-15 18:42 ` [PATCH 5/8] b44: add phylib support Hauke Mehrtens
@ 2013-12-15 18:42 ` Hauke Mehrtens
  2013-12-15 18:42 ` [PATCH 7/8] b44: do not set PHY address to 30 for every ext PHY Hauke Mehrtens
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Hauke Mehrtens @ 2013-12-15 18:42 UTC (permalink / raw)
  To: davem; +Cc: zambrano, netdev, Hauke Mehrtens

Without this patch we can not access the PHY when the MAC is switched
off. This PHY access is needed to configure the switch, which is done
through PHY registers.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/b44.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index 7aa720e..7f243aa 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -1360,7 +1360,10 @@ static void b44_halt(struct b44 *bp)
 	bw32(bp, B44_MAC_CTRL, MAC_CTRL_PHY_PDOWN);
 	/* now reset the chip, but without enabling the MAC&PHY
 	 * part of it. This has to be done _after_ we shut down the PHY */
-	b44_chip_reset(bp, B44_CHIP_RESET_PARTIAL);
+	if (bp->flags & B44_FLAG_EXTERNAL_PHY)
+		b44_chip_reset(bp, B44_CHIP_RESET_FULL);
+	else
+		b44_chip_reset(bp, B44_CHIP_RESET_PARTIAL);
 }
 
 /* bp->lock is held. */
-- 
1.7.10.4

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

* [PATCH 7/8] b44: do not set PHY address to 30 for every ext PHY
  2013-12-15 18:41 [PATCH 0/8] b44: add support for external PHY Hauke Mehrtens
                   ` (5 preceding siblings ...)
  2013-12-15 18:42 ` [PATCH 6/8] b44: activate PHY when MAC is off Hauke Mehrtens
@ 2013-12-15 18:42 ` Hauke Mehrtens
  2013-12-15 18:42 ` [PATCH 8/8] b44: add dummy PHY device if we do not find any Hauke Mehrtens
  2013-12-15 21:26 ` [PATCH 0/8] b44: add support for external PHY Florian Fainelli
  8 siblings, 0 replies; 17+ messages in thread
From: Hauke Mehrtens @ 2013-12-15 18:42 UTC (permalink / raw)
  To: davem; +Cc: zambrano, netdev, Hauke Mehrtens

b44_phy_reset() will fail for an external PHY and only work with the
internal PHY, this was an old workaround when the detection of an
external switch based on the PHY address failed and it is not needed
any more.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/b44.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index 7f243aa..b65a463 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -2415,8 +2415,11 @@ static int b44_init_one(struct ssb_device *sdev,
 	b44_chip_reset(bp, B44_CHIP_RESET_FULL);
 
 	/* do a phy reset to test if there is an active phy */
-	if (b44_phy_reset(bp) < 0)
-		bp->phy_addr = B44_PHY_ADDR_NO_LOACL_PHY;
+	err = b44_phy_reset(bp);
+	if (err < 0) {
+		dev_err(sdev->dev, "phy reset failed\n");
+		goto err_out_unregister_netdev;
+	}
 
 	if (bp->flags & B44_FLAG_EXTERNAL_PHY) {
 		err = b44_register_phy_one(bp);
-- 
1.7.10.4

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

* [PATCH 8/8] b44: add dummy PHY device if we do not find any
  2013-12-15 18:41 [PATCH 0/8] b44: add support for external PHY Hauke Mehrtens
                   ` (6 preceding siblings ...)
  2013-12-15 18:42 ` [PATCH 7/8] b44: do not set PHY address to 30 for every ext PHY Hauke Mehrtens
@ 2013-12-15 18:42 ` Hauke Mehrtens
  2013-12-15 21:26   ` Florian Fainelli
  2013-12-15 21:26 ` [PATCH 0/8] b44: add support for external PHY Florian Fainelli
  8 siblings, 1 reply; 17+ messages in thread
From: Hauke Mehrtens @ 2013-12-15 18:42 UTC (permalink / raw)
  To: davem; +Cc: zambrano, netdev, Hauke Mehrtens

The ADM6996L switches used on some routers do not return a valid value
when reading the PHY id register and Linux thinks there is not PHY at
all, but that is wrong. This created a dummy PHY and uses that instead.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/b44.c |   23 +++++++++++++++++++----
 drivers/net/ethernet/broadcom/b44.h |    3 +++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index b65a463..07e58c2 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -2233,6 +2233,8 @@ static int b44_register_phy_one(struct b44 *bp)
 	struct ssb_device *sdev = bp->sdev;
 	struct phy_device *phydev;
 	int err;
+	struct phy_c45_device_ids c45_ids = {0};
+	struct ssb_sprom *sprom = &sdev->bus->sprom;
 
 	mii_bus = mdiobus_alloc();
 	if (!mii_bus) {
@@ -2266,10 +2268,23 @@ static int b44_register_phy_one(struct b44 *bp)
 	}
 
 	phydev = bp->mii_bus->phy_map[bp->phy_addr];
-	if (!phydev) {
-		dev_err(sdev->dev, "could not find PHY at %i\n", bp->phy_addr);
-		err = -ENODEV;
-		goto err_out_mdiobus_unregister;
+	if (!phydev &&
+	    (sprom->boardflags_lo & (B44_BOARDFLAG_ROBO | B44_BOARDFLAG_ADM))) {
+		dev_info(sdev->dev, "could not find PHY at %i, create dummy one\n",
+			 bp->phy_addr);
+
+		phydev = phy_device_create(bp->mii_bus, bp->phy_addr, 0x0,
+					   false, &c45_ids);
+		if (IS_ERR(phydev)) {
+			err = PTR_ERR(phydev);
+			dev_err(sdev->dev, "Can not create dummy PHY\n");
+			goto err_out_mdiobus_unregister;
+		}
+		err = phy_device_register(phydev);
+		if (err) {
+			dev_err(sdev->dev, "failed to register MII bus\n");
+			goto err_out_mdiobus_unregister;
+		}
 	}
 
 	err = phy_connect_direct(bp->dev, phydev, &b44_adjust_link,
diff --git a/drivers/net/ethernet/broadcom/b44.h b/drivers/net/ethernet/broadcom/b44.h
index c5ec9b4..e6780fe 100644
--- a/drivers/net/ethernet/broadcom/b44.h
+++ b/drivers/net/ethernet/broadcom/b44.h
@@ -345,6 +345,9 @@ B44_STAT_REG_DECLARE
 	struct u64_stats_sync	syncp;
 };
 
+#define	B44_BOARDFLAG_ROBO		0x0010  /* Board has robo switch */
+#define	B44_BOARDFLAG_ADM		0x0080  /* Board has ADMtek switch */
+
 struct ssb_device;
 
 struct b44 {
-- 
1.7.10.4

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

* Re: [PATCH 1/8] b44: cheek register instead of PHY address to detect external PHY
  2013-12-15 18:41 ` [PATCH 1/8] b44: cheek register instead of PHY address to detect " Hauke Mehrtens
@ 2013-12-15 20:04   ` Sergei Shtylyov
  0 siblings, 0 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2013-12-15 20:04 UTC (permalink / raw)
  To: Hauke Mehrtens, davem; +Cc: zambrano, netdev

Hello.

On 12/15/2013 09:41 PM, Hauke Mehrtens wrote:

    s/cheek/check/ in the subject. :-)

> The Ethernet core supported by b44 supports an internal PHY integrated
> into the mac core, which is supported by the b44 driver and an external
> PHY to which the mac core is connected. This external PHY could be a
> switch connected through MII, which is often the case when this core is
> used on home routers. The usage of an external PHY was assumed when the
> PHY address 30 was used and an internal PHY was assumed when the PHY
> address was different. To verify that b44_phy_reset() was called and
> checked if it worked, otherwise PHY address 30 was assumed, an external
> PHY. It is better to check the register which says which PHY is
> connected to the MAC instead of checking the PHY address.

> This also changes B44_FLAG_INTERNAL_PHY to B44_FLAG_EXTERNAL_PHY, it is
> easier to check.

> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>

WBR, Sergei

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

* Re: [PATCH 8/8] b44: add dummy PHY device if we do not find any
  2013-12-15 18:42 ` [PATCH 8/8] b44: add dummy PHY device if we do not find any Hauke Mehrtens
@ 2013-12-15 21:26   ` Florian Fainelli
  2013-12-15 23:31     ` Hauke Mehrtens
  0 siblings, 1 reply; 17+ messages in thread
From: Florian Fainelli @ 2013-12-15 21:26 UTC (permalink / raw)
  To: Hauke Mehrtens; +Cc: davem, zambrano, netdev

Hi Hauke,

Le dimanche 15 décembre 2013, 19:42:05 Hauke Mehrtens a écrit :
> The ADM6996L switches used on some routers do not return a valid value
> when reading the PHY id register and Linux thinks there is not PHY at
> all, but that is wrong. This created a dummy PHY and uses that instead.

As far as I read it from the ADM6996L datasheet; the management interface on 
these switches is via a serial EEPROM which was usually wired up to the 
BCM47xx SoC to some GPIOs (kmod-switch driver in OpenWrt). If MII_PHYSID1 and 
MII_PHYSID2 present bad values, I would assume that MII_BMSR would also return 
bad values too and that if you are given a valid link speed/duplex this might 
just be because all bits are set?

In any case this is a case where you should register a fixed PHY device instead 
of creating such a dummy device which will still make the mdiobus driver/PHY 
state machine issue real reads/writes on the MDIO bus to a non-existent PHY.

> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  drivers/net/ethernet/broadcom/b44.c |   23 +++++++++++++++++++----
>  drivers/net/ethernet/broadcom/b44.h |    3 +++
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/b44.c
> b/drivers/net/ethernet/broadcom/b44.c index b65a463..07e58c2 100644
> --- a/drivers/net/ethernet/broadcom/b44.c
> +++ b/drivers/net/ethernet/broadcom/b44.c
> @@ -2233,6 +2233,8 @@ static int b44_register_phy_one(struct b44 *bp)
>  	struct ssb_device *sdev = bp->sdev;
>  	struct phy_device *phydev;
>  	int err;
> +	struct phy_c45_device_ids c45_ids = {0};
> +	struct ssb_sprom *sprom = &sdev->bus->sprom;
> 
>  	mii_bus = mdiobus_alloc();
>  	if (!mii_bus) {
> @@ -2266,10 +2268,23 @@ static int b44_register_phy_one(struct b44 *bp)
>  	}
> 
>  	phydev = bp->mii_bus->phy_map[bp->phy_addr];
> -	if (!phydev) {
> -		dev_err(sdev->dev, "could not find PHY at %i\n", bp->phy_addr);
> -		err = -ENODEV;
> -		goto err_out_mdiobus_unregister;
> +	if (!phydev &&
> +	    (sprom->boardflags_lo & (B44_BOARDFLAG_ROBO | B44_BOARDFLAG_ADM))) {
> +		dev_info(sdev->dev, "could not find PHY at %i, create dummy one\n",
> +			 bp->phy_addr);

Your commit subject says ADM6996L but here you are also treating Broadcom 
switches that way, this might deserve a comment to explain that some BCM53xx 
switches might be SPI/GPIO connected.

> +
> +		phydev = phy_device_create(bp->mii_bus, bp->phy_addr, 0x0,
> +					   false, &c45_ids);
> +		if (IS_ERR(phydev)) {
> +			err = PTR_ERR(phydev);
> +			dev_err(sdev->dev, "Can not create dummy PHY\n");
> +			goto err_out_mdiobus_unregister;
> +		}
> +		err = phy_device_register(phydev);
> +		if (err) {
> +			dev_err(sdev->dev, "failed to register MII bus\n");
> +			goto err_out_mdiobus_unregister;
> +		}
>  	}
> 
>  	err = phy_connect_direct(bp->dev, phydev, &b44_adjust_link,
> diff --git a/drivers/net/ethernet/broadcom/b44.h
> b/drivers/net/ethernet/broadcom/b44.h index c5ec9b4..e6780fe 100644
> --- a/drivers/net/ethernet/broadcom/b44.h
> +++ b/drivers/net/ethernet/broadcom/b44.h
> @@ -345,6 +345,9 @@ B44_STAT_REG_DECLARE
>  	struct u64_stats_sync	syncp;
>  };
> 
> +#define	B44_BOARDFLAG_ROBO		0x0010  /* Board has robo switch */
> +#define	B44_BOARDFLAG_ADM		0x0080  /* Board has ADMtek switch */
> +
>  struct ssb_device;
> 
>  struct b44 {

-- 
Florian

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

* Re: [PATCH 0/8] b44: add support for external PHY
  2013-12-15 18:41 [PATCH 0/8] b44: add support for external PHY Hauke Mehrtens
                   ` (7 preceding siblings ...)
  2013-12-15 18:42 ` [PATCH 8/8] b44: add dummy PHY device if we do not find any Hauke Mehrtens
@ 2013-12-15 21:26 ` Florian Fainelli
  8 siblings, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2013-12-15 21:26 UTC (permalink / raw)
  To: Hauke Mehrtens; +Cc: davem, zambrano, netdev

Hello Hauke,

Le dimanche 15 décembre 2013, 19:41:57 Hauke Mehrtens a écrit :
> This adds support for an external phy connected to the mac controlled
> by b44. This is used on home routers of the BCM47xx line where this MAC
> core was used and is contended to an external switch core through MII.
> These patches are in OpenWrt for some time and are tested by different
> users with different devices.

Besides my comment on patch 8, this series looks fine to me, thanks!

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

> 
> Hauke Mehrtens (8):
>   b44: cheek register instead of PHY address to detect external PHY
>   b44: rename B44_PHY_ADDR_NO_PHY to B44_PHY_ADDR_NO_LOCAL_PHY
>   b44: abort when no PHY is available at all
>   b44: rename b44_mii_{read,write} to b44_mdio_{read,write}_mii
>   b44: add phylib support
>   b44: activate PHY when MAC is off
>   b44: do not set PHY address to 30 for every ext PHY
>   b44: add dummy PHY device if we do not find any
> 
>  drivers/net/ethernet/broadcom/Kconfig |    1 +
>  drivers/net/ethernet/broadcom/b44.c   |  254
> ++++++++++++++++++++++++++++++--- drivers/net/ethernet/broadcom/b44.h   |  
> 15 +-
>  3 files changed, 244 insertions(+), 26 deletions(-)

-- 
Florian

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

* Re: [PATCH 8/8] b44: add dummy PHY device if we do not find any
  2013-12-15 21:26   ` Florian Fainelli
@ 2013-12-15 23:31     ` Hauke Mehrtens
  2013-12-16  3:42       ` Florian Fainelli
  0 siblings, 1 reply; 17+ messages in thread
From: Hauke Mehrtens @ 2013-12-15 23:31 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: davem, zambrano, netdev

On 12/15/2013 10:26 PM, Florian Fainelli wrote:
> Hi Hauke,
> 
> Le dimanche 15 décembre 2013, 19:42:05 Hauke Mehrtens a écrit :
>> The ADM6996L switches used on some routers do not return a valid value
>> when reading the PHY id register and Linux thinks there is not PHY at
>> all, but that is wrong. This created a dummy PHY and uses that instead.
> 
> As far as I read it from the ADM6996L datasheet; the management interface on 
> these switches is via a serial EEPROM which was usually wired up to the 
> BCM47xx SoC to some GPIOs (kmod-switch driver in OpenWrt). If MII_PHYSID1 and 
> MII_PHYSID2 present bad values, I would assume that MII_BMSR would also return 
> bad values too and that if you are given a valid link speed/duplex this might 
> just be because all bits are set?

Yes these registers are returning 0xffff.

> In any case this is a case where you should register a fixed PHY device instead 
> of creating such a dummy device which will still make the mdiobus driver/PHY 
> state machine issue real reads/writes on the MDIO bus to a non-existent PHY.

Yes I will try that.

>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
>> ---
>>  drivers/net/ethernet/broadcom/b44.c |   23 +++++++++++++++++++----
>>  drivers/net/ethernet/broadcom/b44.h |    3 +++
>>  2 files changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/b44.c
>> b/drivers/net/ethernet/broadcom/b44.c index b65a463..07e58c2 100644
>> --- a/drivers/net/ethernet/broadcom/b44.c
>> +++ b/drivers/net/ethernet/broadcom/b44.c
>> @@ -2233,6 +2233,8 @@ static int b44_register_phy_one(struct b44 *bp)
>>  	struct ssb_device *sdev = bp->sdev;
>>  	struct phy_device *phydev;
>>  	int err;
>> +	struct phy_c45_device_ids c45_ids = {0};
>> +	struct ssb_sprom *sprom = &sdev->bus->sprom;
>>
>>  	mii_bus = mdiobus_alloc();
>>  	if (!mii_bus) {
>> @@ -2266,10 +2268,23 @@ static int b44_register_phy_one(struct b44 *bp)
>>  	}
>>
>>  	phydev = bp->mii_bus->phy_map[bp->phy_addr];
>> -	if (!phydev) {
>> -		dev_err(sdev->dev, "could not find PHY at %i\n", bp->phy_addr);
>> -		err = -ENODEV;
>> -		goto err_out_mdiobus_unregister;
>> +	if (!phydev &&
>> +	    (sprom->boardflags_lo & (B44_BOARDFLAG_ROBO | B44_BOARDFLAG_ADM))) {
>> +		dev_info(sdev->dev, "could not find PHY at %i, create dummy one\n",
>> +			 bp->phy_addr);
> 
> Your commit subject says ADM6996L but here you are also treating Broadcom 
> switches that way, this might deserve a comment to explain that some BCM53xx 
> switches might be SPI/GPIO connected.

This is also needed for some Broadcom switches. The Broadcom BCM5325F
switch has two MII interfaces and the BCM4704 has two MACs which are
both connected to one of the MII interfaces of the switch. On such
devices we do get a PHY ID for the first MII interface used for the LAN
ports, but when trying to read the PHY ID of the second MII interface
used for the WAN port it just returns 0xffff on all addresses. When
registering this dummy phy we are able to talk to the interface.

This WAN port seams to be hard wired to the second MII interface in the
switch.

>> +
>> +		phydev = phy_device_create(bp->mii_bus, bp->phy_addr, 0x0,
>> +					   false, &c45_ids);
>> +		if (IS_ERR(phydev)) {
>> +			err = PTR_ERR(phydev);
>> +			dev_err(sdev->dev, "Can not create dummy PHY\n");
>> +			goto err_out_mdiobus_unregister;
>> +		}
>> +		err = phy_device_register(phydev);
>> +		if (err) {
>> +			dev_err(sdev->dev, "failed to register MII bus\n");
>> +			goto err_out_mdiobus_unregister;
>> +		}
>>  	}
>>
>>  	err = phy_connect_direct(bp->dev, phydev, &b44_adjust_link,
>> diff --git a/drivers/net/ethernet/broadcom/b44.h
>> b/drivers/net/ethernet/broadcom/b44.h index c5ec9b4..e6780fe 100644
>> --- a/drivers/net/ethernet/broadcom/b44.h
>> +++ b/drivers/net/ethernet/broadcom/b44.h
>> @@ -345,6 +345,9 @@ B44_STAT_REG_DECLARE
>>  	struct u64_stats_sync	syncp;
>>  };
>>
>> +#define	B44_BOARDFLAG_ROBO		0x0010  /* Board has robo switch */
>> +#define	B44_BOARDFLAG_ADM		0x0080  /* Board has ADMtek switch */
>> +
>>  struct ssb_device;
>>
>>  struct b44 {
> 

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

* Re: [PATCH 8/8] b44: add dummy PHY device if we do not find any
  2013-12-15 23:31     ` Hauke Mehrtens
@ 2013-12-16  3:42       ` Florian Fainelli
  0 siblings, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2013-12-16  3:42 UTC (permalink / raw)
  To: Hauke Mehrtens; +Cc: davem, zambrano, netdev

Le lundi 16 décembre 2013, 00:31:34 Hauke Mehrtens a écrit :
> On 12/15/2013 10:26 PM, Florian Fainelli wrote:
> > Hi Hauke,
> > 
> > Le dimanche 15 décembre 2013, 19:42:05 Hauke Mehrtens a écrit :
> >> The ADM6996L switches used on some routers do not return a valid value
> >> when reading the PHY id register and Linux thinks there is not PHY at
> >> all, but that is wrong. This created a dummy PHY and uses that instead.
> > 
> > As far as I read it from the ADM6996L datasheet; the management interface
> > on these switches is via a serial EEPROM which was usually wired up to
> > the BCM47xx SoC to some GPIOs (kmod-switch driver in OpenWrt). If
> > MII_PHYSID1 and MII_PHYSID2 present bad values, I would assume that
> > MII_BMSR would also return bad values too and that if you are given a
> > valid link speed/duplex this might just be because all bits are set?
> 
> Yes these registers are returning 0xffff.

Ok, yes that's pretty typical of the absence of a physical connection between 
the MAC and PHY/switch.

> 
> > In any case this is a case where you should register a fixed PHY device
> > instead of creating such a dummy device which will still make the mdiobus
> > driver/PHY state machine issue real reads/writes on the MDIO bus to a
> > non-existent PHY.
> Yes I will try that.

Thanks! Note that a fixed PHY must be registered before the fixed MDIO bus is 
probe, you can take a look at how AR7 does it for instance 
(arch/mips/ar7/platform.c and drivers/net/ethernet/ti/cpmac.c). We might want 
to be able to change that in the future and re-scan the fixed MDIO bus though.

> 
> >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> >> ---
> >> 
> >>  drivers/net/ethernet/broadcom/b44.c |   23 +++++++++++++++++++----
> >>  drivers/net/ethernet/broadcom/b44.h |    3 +++
> >>  2 files changed, 22 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/drivers/net/ethernet/broadcom/b44.c
> >> b/drivers/net/ethernet/broadcom/b44.c index b65a463..07e58c2 100644
> >> --- a/drivers/net/ethernet/broadcom/b44.c
> >> +++ b/drivers/net/ethernet/broadcom/b44.c
> >> @@ -2233,6 +2233,8 @@ static int b44_register_phy_one(struct b44 *bp)
> >> 
> >>  	struct ssb_device *sdev = bp->sdev;
> >>  	struct phy_device *phydev;
> >>  	int err;
> >> 
> >> +	struct phy_c45_device_ids c45_ids = {0};
> >> +	struct ssb_sprom *sprom = &sdev->bus->sprom;
> >> 
> >>  	mii_bus = mdiobus_alloc();
> >>  	if (!mii_bus) {
> >> 
> >> @@ -2266,10 +2268,23 @@ static int b44_register_phy_one(struct b44 *bp)
> >> 
> >>  	}
> >>  	
> >>  	phydev = bp->mii_bus->phy_map[bp->phy_addr];
> >> 
> >> -	if (!phydev) {
> >> -		dev_err(sdev->dev, "could not find PHY at %i\n", bp->phy_addr);
> >> -		err = -ENODEV;
> >> -		goto err_out_mdiobus_unregister;
> >> +	if (!phydev &&
> >> +	    (sprom->boardflags_lo & (B44_BOARDFLAG_ROBO | B44_BOARDFLAG_ADM)))
> >> {
> >> +		dev_info(sdev->dev, "could not find PHY at %i, create dummy one\n",
> >> +			 bp->phy_addr);
> > 
> > Your commit subject says ADM6996L but here you are also treating Broadcom
> > switches that way, this might deserve a comment to explain that some
> > BCM53xx switches might be SPI/GPIO connected.
> 
> This is also needed for some Broadcom switches. The Broadcom BCM5325F
> switch has two MII interfaces and the BCM4704 has two MACs which are
> both connected to one of the MII interfaces of the switch. On such
> devices we do get a PHY ID for the first MII interface used for the LAN
> ports, but when trying to read the PHY ID of the second MII interface
> used for the WAN port it just returns 0xffff on all addresses. When
> registering this dummy phy we are able to talk to the interface.

Most likely you get 0xffff because the switch is only connected to the first 
Ethernet MAC MDIO bus.

In that specific case it might be better to make the second Ethernet MAC probe 
for PHYs on the first Ethernet MAC mdio bus or simply register a fixed PHY 
device for the WAN port since you only want one Ethernet MAC to be in control 
of the switch at a time.

> 
> This WAN port seams to be hard wired to the second MII interface in the
> switch.

This is quite likely, most Broadcom switches support one or two IMP (CPU port) 
configurations.
-- 
Florian

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

* Re: [PATCH 2/8] b44: rename B44_PHY_ADDR_NO_PHY to B44_PHY_ADDR_NO_LOCAL_PHY
  2013-12-15 18:41 ` [PATCH 2/8] b44: rename B44_PHY_ADDR_NO_PHY to B44_PHY_ADDR_NO_LOCAL_PHY Hauke Mehrtens
@ 2013-12-16 15:51   ` Ben Hutchings
  2013-12-16 18:40   ` Florian Fainelli
  1 sibling, 0 replies; 17+ messages in thread
From: Ben Hutchings @ 2013-12-16 15:51 UTC (permalink / raw)
  To: Hauke Mehrtens; +Cc: davem, zambrano, netdev

On Sun, 2013-12-15 at 19:41 +0100, Hauke Mehrtens wrote:
> The PHY address 30 means there is no local PHY, but there could be an
> external PHY like a switch connected via MII. This is the case on most
> embedded home routers where this driver is used.
> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  drivers/net/ethernet/broadcom/b44.c |    2 +-
>  drivers/net/ethernet/broadcom/b44.h |    6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
> index 3c7909e..fce36dd 100644
> --- a/drivers/net/ethernet/broadcom/b44.c
> +++ b/drivers/net/ethernet/broadcom/b44.c
> @@ -2237,7 +2237,7 @@ static int b44_init_one(struct ssb_device *sdev,
>  
>  	/* do a phy reset to test if there is an active phy */
>  	if (b44_phy_reset(bp) < 0)
> -		bp->phy_addr = B44_PHY_ADDR_NO_PHY;
> +		bp->phy_addr = B44_PHY_ADDR_NO_LOACL_PHY;

LOACL?

>  	netdev_info(dev, "%s %pM\n", DRV_DESCRIPTION, dev->dev_addr);
>  
> diff --git a/drivers/net/ethernet/broadcom/b44.h b/drivers/net/ethernet/broadcom/b44.h
> index 8ed7d6b..ade80d6 100644
> --- a/drivers/net/ethernet/broadcom/b44.h
> +++ b/drivers/net/ethernet/broadcom/b44.h
> @@ -280,9 +280,9 @@ struct ring_info {
>  	dma_addr_t	mapping;
>  };
>  
> -#define B44_MCAST_TABLE_SIZE	32
> -#define B44_PHY_ADDR_NO_PHY	30
> -#define B44_MDC_RATIO		5000000
> +#define B44_MCAST_TABLE_SIZE		32
> +#define B44_PHY_ADDR_NO_LOACL_PHY	30 /* no local phy regs */
> +#define B44_MDC_RATIO			5000000
>  
>  #define	B44_STAT_REG_DECLARE		\
>  	_B44(tx_good_octets)		\

At least it's spelt consistently wrongly. :-)

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

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

* Re: [PATCH 2/8] b44: rename B44_PHY_ADDR_NO_PHY to B44_PHY_ADDR_NO_LOCAL_PHY
  2013-12-15 18:41 ` [PATCH 2/8] b44: rename B44_PHY_ADDR_NO_PHY to B44_PHY_ADDR_NO_LOCAL_PHY Hauke Mehrtens
  2013-12-16 15:51   ` Ben Hutchings
@ 2013-12-16 18:40   ` Florian Fainelli
  2013-12-19  1:21     ` Hauke Mehrtens
  1 sibling, 1 reply; 17+ messages in thread
From: Florian Fainelli @ 2013-12-16 18:40 UTC (permalink / raw)
  To: Hauke Mehrtens; +Cc: David Miller, zambrano, netdev

2013/12/15 Hauke Mehrtens <hauke@hauke-m.de>:
> The PHY address 30 means there is no local PHY, but there could be an
> external PHY like a switch connected via MII. This is the case on most
> embedded home routers where this driver is used.

Looking at this some more, I do not think the b44 driver was always
clear that (at least in  Broadcom terminology):

- address 0 is a "special" broadcast MDIO address to make Roboswitch
devices act as standard PHYs such that you could read the MII_PHYSID1
and MII_PHYSID2 registers and determine if that is a switch or a PHY
- address 30 is the switch pseudo-PHY address through which all
switch-specifc settings must be routed from/to

So saying that address 30 means no "external PHY" is sort of correct,
although not quite. Hopefully the remaining patches try to remove that
ambiguity.

>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  drivers/net/ethernet/broadcom/b44.c |    2 +-
>  drivers/net/ethernet/broadcom/b44.h |    6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
> index 3c7909e..fce36dd 100644
> --- a/drivers/net/ethernet/broadcom/b44.c
> +++ b/drivers/net/ethernet/broadcom/b44.c
> @@ -2237,7 +2237,7 @@ static int b44_init_one(struct ssb_device *sdev,
>
>         /* do a phy reset to test if there is an active phy */
>         if (b44_phy_reset(bp) < 0)
> -               bp->phy_addr = B44_PHY_ADDR_NO_PHY;
> +               bp->phy_addr = B44_PHY_ADDR_NO_LOACL_PHY;
>
>         netdev_info(dev, "%s %pM\n", DRV_DESCRIPTION, dev->dev_addr);
>
> diff --git a/drivers/net/ethernet/broadcom/b44.h b/drivers/net/ethernet/broadcom/b44.h
> index 8ed7d6b..ade80d6 100644
> --- a/drivers/net/ethernet/broadcom/b44.h
> +++ b/drivers/net/ethernet/broadcom/b44.h
> @@ -280,9 +280,9 @@ struct ring_info {
>         dma_addr_t      mapping;
>  };
>
> -#define B44_MCAST_TABLE_SIZE   32
> -#define B44_PHY_ADDR_NO_PHY    30
> -#define B44_MDC_RATIO          5000000
> +#define B44_MCAST_TABLE_SIZE           32
> +#define B44_PHY_ADDR_NO_LOACL_PHY      30 /* no local phy regs */
> +#define B44_MDC_RATIO                  5000000
>
>  #define        B44_STAT_REG_DECLARE            \
>         _B44(tx_good_octets)            \
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Florian

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

* Re: [PATCH 2/8] b44: rename B44_PHY_ADDR_NO_PHY to B44_PHY_ADDR_NO_LOCAL_PHY
  2013-12-16 18:40   ` Florian Fainelli
@ 2013-12-19  1:21     ` Hauke Mehrtens
  0 siblings, 0 replies; 17+ messages in thread
From: Hauke Mehrtens @ 2013-12-19  1:21 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David Miller, zambrano, netdev

On 12/16/2013 07:40 PM, Florian Fainelli wrote:
> 2013/12/15 Hauke Mehrtens <hauke@hauke-m.de>:
>> The PHY address 30 means there is no local PHY, but there could be an
>> external PHY like a switch connected via MII. This is the case on most
>> embedded home routers where this driver is used.
> 
> Looking at this some more, I do not think the b44 driver was always
> clear that (at least in  Broadcom terminology):
> 
> - address 0 is a "special" broadcast MDIO address to make Roboswitch
> devices act as standard PHYs such that you could read the MII_PHYSID1
> and MII_PHYSID2 registers and determine if that is a switch or a PHY
> - address 30 is the switch pseudo-PHY address through which all
> switch-specifc settings must be routed from/to
> 
> So saying that address 30 means no "external PHY" is sort of correct,
> although not quite. Hopefully the remaining patches try to remove that
> ambiguity.

After all these patches B44_PHY_ADDR_NO_LOACL_PHY is not used any more
by b44.

The external interface was only activated when the register which will
not be used indicated that there is no internal phy.

>>
>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
>> ---
>>  drivers/net/ethernet/broadcom/b44.c |    2 +-
>>  drivers/net/ethernet/broadcom/b44.h |    6 +++---
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
>> index 3c7909e..fce36dd 100644
>> --- a/drivers/net/ethernet/broadcom/b44.c
>> +++ b/drivers/net/ethernet/broadcom/b44.c
>> @@ -2237,7 +2237,7 @@ static int b44_init_one(struct ssb_device *sdev,
>>
>>         /* do a phy reset to test if there is an active phy */
>>         if (b44_phy_reset(bp) < 0)
>> -               bp->phy_addr = B44_PHY_ADDR_NO_PHY;
>> +               bp->phy_addr = B44_PHY_ADDR_NO_LOACL_PHY;
>>
>>         netdev_info(dev, "%s %pM\n", DRV_DESCRIPTION, dev->dev_addr);
>>
>> diff --git a/drivers/net/ethernet/broadcom/b44.h b/drivers/net/ethernet/broadcom/b44.h
>> index 8ed7d6b..ade80d6 100644
>> --- a/drivers/net/ethernet/broadcom/b44.h
>> +++ b/drivers/net/ethernet/broadcom/b44.h
>> @@ -280,9 +280,9 @@ struct ring_info {
>>         dma_addr_t      mapping;
>>  };
>>
>> -#define B44_MCAST_TABLE_SIZE   32
>> -#define B44_PHY_ADDR_NO_PHY    30
>> -#define B44_MDC_RATIO          5000000
>> +#define B44_MCAST_TABLE_SIZE           32
>> +#define B44_PHY_ADDR_NO_LOACL_PHY      30 /* no local phy regs */
>> +#define B44_MDC_RATIO                  5000000
>>
>>  #define        B44_STAT_REG_DECLARE            \
>>         _B44(tx_good_octets)            \
>> --
>> 1.7.10.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 

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

end of thread, other threads:[~2013-12-19  1:21 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-15 18:41 [PATCH 0/8] b44: add support for external PHY Hauke Mehrtens
2013-12-15 18:41 ` [PATCH 1/8] b44: cheek register instead of PHY address to detect " Hauke Mehrtens
2013-12-15 20:04   ` Sergei Shtylyov
2013-12-15 18:41 ` [PATCH 2/8] b44: rename B44_PHY_ADDR_NO_PHY to B44_PHY_ADDR_NO_LOCAL_PHY Hauke Mehrtens
2013-12-16 15:51   ` Ben Hutchings
2013-12-16 18:40   ` Florian Fainelli
2013-12-19  1:21     ` Hauke Mehrtens
2013-12-15 18:42 ` [PATCH 3/8] b44: abort when no PHY is available at all Hauke Mehrtens
2013-12-15 18:42 ` [PATCH 4/8] b44: rename b44_mii_{read,write} to b44_mdio_{read,write}_mii Hauke Mehrtens
2013-12-15 18:42 ` [PATCH 5/8] b44: add phylib support Hauke Mehrtens
2013-12-15 18:42 ` [PATCH 6/8] b44: activate PHY when MAC is off Hauke Mehrtens
2013-12-15 18:42 ` [PATCH 7/8] b44: do not set PHY address to 30 for every ext PHY Hauke Mehrtens
2013-12-15 18:42 ` [PATCH 8/8] b44: add dummy PHY device if we do not find any Hauke Mehrtens
2013-12-15 21:26   ` Florian Fainelli
2013-12-15 23:31     ` Hauke Mehrtens
2013-12-16  3:42       ` Florian Fainelli
2013-12-15 21:26 ` [PATCH 0/8] b44: add support for external PHY Florian Fainelli

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