[PATCH 0/3] netdev/phy/of: Improve 10G Ethernet PHY support.

[PATCH 0/3] netdev/phy/of: Improve 10G Ethernet PHY support.

[David Daney]

The PHY driver core needs a couple of adjustments to be able to
support 10G PHYs.  The main issue being that they use a different MDIO
bus protocol (IEEE802.3 clause 45).

The changes are not large:

1) The addr argument to get_phy_id() and get_phy_device() can be
   flagged to indicate clause 45 addressing.

2) The device tree helper of_mdiobus_register() uses a new
   "compatible" value to hook up the 10G phys.

3) A driver for the BCM8706 which makes use of it all.

David Daney (3):
  netdev/phy: Handle IEEE802.3 clause 45 Ethernet PHYs
  netdev/phy/of: Handle IEEE802.3 clause 45 Ethernet PHYs in
    of_mdiobus_register()
  netdev/phy: Add driver for Broadcom BCM8706 10G Ethernet PHY

 .../devicetree/bindings/net/broadcom-bcm8706.txt   |   28 +++
 Documentation/devicetree/bindings/net/phy.txt      |   12 +-
 drivers/net/phy/Kconfig                            |    5 +
 drivers/net/phy/Makefile                           |    1 +
 drivers/net/phy/bcm8706.c                          |  212 ++++++++++++++++++++
 drivers/net/phy/phy_device.c                       |   25 ++-
 drivers/of/of_mdio.c                               |    4 +
 include/linux/brcmphy.h                            |    1 +
 include/linux/phy.h                                |    3 +
 9 files changed, 287 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
 create mode 100644 drivers/net/phy/bcm8706.c

-- 
1.7.2.3

[PATCH 1/3] netdev/phy: Handle IEEE802.3 clause 45 Ethernet PHYs

[David Daney]

The IEEE802.3 clause 45 MDIO bus protocol allows for directly
addressing PHY registers using a 21 bit address, and is used by many
10G Ethernet PHYS.  Already existing is the ability of MDIO bus
drivers to use clause 45, with the MII_ADDR_C45 flag.  Here we add
some support in the PHY and device tree infrastructure to use these
PHYs.

Normally the MII_ADDR_C45 flag is ORed with the register address to
indicate a clause 45 transaction.  Here we also use this flag in the
*device* address passed to get_phy_id() and get_phy_device() to
indicate that probing should be done with clause 45 transactions.  If
a PHY is successfully probed with MII_ADDR_C45, the new struct
phy_device is_c45 flag is set for the PHY.

Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
 drivers/net/phy/phy_device.c |   25 ++++++++++++++++++++++---
 include/linux/phy.h          |    3 +++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 83a5a5a..7e4d61b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -171,6 +171,8 @@ static struct phy_device* phy_device_create(struct mii_bus *bus,
 
 	dev->autoneg = AUTONEG_ENABLE;
 
+	dev->is_c45 = (addr & MII_ADDR_C45) != 0;
+	addr &= ~MII_ADDR_C45;
 	dev->addr = addr;
 	dev->phy_id = phy_id;
 	dev->bus = bus;
@@ -205,15 +207,24 @@ static struct phy_device* phy_device_create(struct mii_bus *bus,
  * @phy_id: where to store the ID retrieved.
  *
  * Description: Reads the ID registers of the PHY at @addr on the
- *   @bus, stores it in @phy_id and returns zero on success.
+ *   @bus, stores it in @phy_id and returns zero on success.  If the
+ *   @addr has been ORed with MII_ADDR_C45, mdio clause 45 data
+ *   transfer is used to read ID from the PHY device, otherwise the
+ *   standard protocol (clause 22) is used.
  */
 int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id)
 {
 	int phy_reg;
+	u32 c45_reg_base = 0;
 
+	if (addr & MII_ADDR_C45) {
+		addr &= ~MII_ADDR_C45;
+		/* Access the PHY's PHY XS registers with C45 mode. */
+		c45_reg_base = MII_ADDR_C45 | 0x40000;
+	}
 	/* Grab the bits from PHYIR1, and put them
 	 * in the upper half */
-	phy_reg = mdiobus_read(bus, addr, MII_PHYSID1);
+	phy_reg = mdiobus_read(bus, addr, c45_reg_base | MII_PHYSID1);
 
 	if (phy_reg < 0)
 		return -EIO;
@@ -221,7 +232,7 @@ int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id)
 	*phy_id = (phy_reg & 0xffff) << 16;
 
 	/* Grab the bits from PHYIR2, and put them in the lower half */
-	phy_reg = mdiobus_read(bus, addr, MII_PHYSID2);
+	phy_reg = mdiobus_read(bus, addr, c45_reg_base | MII_PHYSID2);
 
 	if (phy_reg < 0)
 		return -EIO;
@@ -239,6 +250,9 @@ EXPORT_SYMBOL(get_phy_id);
  *
  * Description: Reads the ID registers of the PHY at @addr on the
  *   @bus, then allocates and returns the phy_device to represent it.
+ *   If the @addr has been ORed with MII_ADDR_C45, mdio clause 45 data
+ *   transfer is used to read the PHY device, otherwise the standard
+ *   protocol (clause 22) is used.
  */
 struct phy_device * get_phy_device(struct mii_bus *bus, int addr)
 {
@@ -447,6 +461,11 @@ static int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	/* Assume that if there is no driver, that it doesn't
 	 * exist, and we should use the genphy driver. */
 	if (NULL == d->driver) {
+		if (phydev->is_c45) {
+			pr_err("No driver for phy %x\n", phydev->phy_id);
+			return -ENODEV;
+		}
+
 		d->driver = &genphy_driver.driver;
 
 		err = d->driver->probe(d);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e4c3844..0a25e2c 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -246,6 +246,7 @@ struct sk_buff;
  * phy_id: UID for this device found during discovery
  * state: state of the PHY for management purposes
  * dev_flags: Device-specific flags used by the PHY driver.
+ * is_c45:  Set to 1 if this phy uses clause 45 addressing.
  * addr: Bus address of PHY
  * link_timeout: The number of timer firings to wait before the
  * giving up on the current attempt at acquiring a link
@@ -283,6 +284,8 @@ struct phy_device {
 
 	u32 dev_flags;
 
+	unsigned int is_c45:1;
+
 	phy_interface_t interface;
 
 	/* Bus address of the PHY (0-31) */
-- 
1.7.2.3

[PATCH 2/3] netdev/phy/of: Handle IEEE802.3 clause 45 Ethernet PHYs in of_mdiobus_register()

[David Daney]

Define two new "compatible" values for Ethernet
PHYs. "ethernet-phy-ieee802.3-c22" and "ethernet-phy-ieee802.3-c45"
are used to indicate a PHY uses the corresponding protocol.

If a PHY is "compatible" with "ethernet-phy-ieee802.3-c45", we
indicate this so that get_phy_device() can properly probe the device.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 Documentation/devicetree/bindings/net/phy.txt |   12 +++++++++++-
 drivers/of/of_mdio.c                          |    4 ++++
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index bb8c742..de9cb32 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -16,8 +16,18 @@ Required properties:
 
 Example:
 
+Optional Properties:
+
+- compatible: Compatible list, may contain "ethernet-phy-ieee802.3-c22" or
+              "ethernet-phy-ieee802.3-c45" for PHYs that implement
+              IEEE802.3 clause 22 or IEEE802.3 clause 45
+              specifications.  If neither of these are specified, the
+              default is to assume clause 22.  The compatible list may
+              also contain other elements.
+
 ethernet-phy@0 {
-	linux,phandle = <2452000>
+	compatible = "ethernet-phy-ieee802.3-c22";
+	linux,phandle = <2452000>;
 	interrupt-parent = <40000>;
 	interrupts = <35 1>;
 	reg = <0>;
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 7c28e8c..f837a7f 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -79,6 +79,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 				mdio->irq[addr] = PHY_POLL;
 		}
 
+		if (of_device_is_compatible(child,
+					    "ethernet-phy-ieee802.3-c45"))
+			addr |= MII_ADDR_C45;
+
 		phy = get_phy_device(mdio, addr);
 		if (!phy || IS_ERR(phy)) {
 			dev_err(&mdio->dev, "error probing PHY at address %i\n",
-- 
1.7.2.3

[PATCH 3/3] netdev/phy: Add driver for Broadcom BCM8706 10G Ethernet PHY

[David Daney]

Add a driver and PHY_ID number for said device.  This is a 10Gig PHY
which uses MII_ADDR_C45 addressing, it is always 10G full duplex, so
there is no autonegotiation.  All we do is report link state and send
interrupts when it changes.

If the PHY has a device tree of_node associated with it, the
"broadcom,c45-reg-init" property is used to supply register
initialization values when config_init() is called.

Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
 .../devicetree/bindings/net/broadcom-bcm8706.txt   |   28 +++
 drivers/net/phy/Kconfig                            |    5 +
 drivers/net/phy/Makefile                           |    1 +
 drivers/net/phy/bcm8706.c                          |  212 ++++++++++++++++++++
 include/linux/brcmphy.h                            |    1 +
 5 files changed, 247 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
 create mode 100644 drivers/net/phy/bcm8706.c

diff --git a/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt b/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
new file mode 100644
index 0000000..d58bea9
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
@@ -0,0 +1,28 @@
+The Broadcom BCM8706 is a 10G Ethernet PHY.  It has these bindings in
+addition to the standard PHY bindings.
+
+Compatible: Should contain "broadcom,bcm8706" and
+            "ethernet-phy-ieee802.3-c45"
+
+Optional Properties:
+
+- broadcom,c45-reg-init : one of more sets of 4 cells.  The first cell
+  is the device type, the second a register address, the third cell
+  contains a mask to be ANDed with the existing register value, and
+  the fourth cell is ORed with he result to yield the new register
+  value.
+
+Example:
+
+	ethernet-phy@5 {
+		reg = <5>;
+		compatible = "broadcom,bcm8706", "ethernet-phy-ieee802.3-c45";
+		interrupt-parent = <&gpio>;
+		interrupts = <12 8>; /* Pin 12, active low */
+		/*
+		 * Set PMD Digital Control Register for
+		 * GPIO[1] Tx/Rx
+		 * GPIO[0] R64 Sync Acquired
+		 */
+		broadcom,c45-reg-init = <1 0xc808 0xff8f 0x70>;
+	};
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 59b3b17..a99885f 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -62,6 +62,11 @@ config BCM63XX_PHY
 	---help---
 	  Currently supports the 6348 and 6358 PHYs.
 
+config BCM8706_PHY
+	tristate "Driver for Broadcom BCM8706 10G Ethernet PHY"
+	help
+	  Currently supports only the BCM8706 PHY.
+
 config ICPLUS_PHY
 	tristate "Drivers for ICPlus PHYs"
 	---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index d1a1927..ec46398 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SMSC_PHY)		+= smsc.o
 obj-$(CONFIG_VITESSE_PHY)	+= vitesse.o
 obj-$(CONFIG_BROADCOM_PHY)	+= broadcom.o
 obj-$(CONFIG_BCM63XX_PHY)	+= bcm63xx.o
+obj-$(CONFIG_BCM8706_PHY)	+= bcm8706.o
 obj-$(CONFIG_ICPLUS_PHY)	+= icplus.o
 obj-$(CONFIG_REALTEK_PHY)	+= realtek.o
 obj-$(CONFIG_LSI_ET1011C_PHY)	+= et1011c.o
diff --git a/drivers/net/phy/bcm8706.c b/drivers/net/phy/bcm8706.c
new file mode 100644
index 0000000..3a23e04
--- /dev/null
+++ b/drivers/net/phy/bcm8706.c
@@ -0,0 +1,212 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (C) 2011 Cavium, Inc.
+ */
+
+#include <linux/module.h>
+#include <linux/phy.h>
+#include <linux/brcmphy.h>
+#include <linux/of.h>
+
+#define BCM8706_PMD_RX_SIGNAL_DETECT	(MII_ADDR_C45 | 0x1000a)
+#define BCM8706_10GBASER_PCS_STATUS	(MII_ADDR_C45 | 0x30020)
+#define BCM8706_XGXS_LANE_STATUS	(MII_ADDR_C45 | 0x40018)
+
+#define BCM8706_LASI_CONTROL		(MII_ADDR_C45 | 0x39002)
+#define BCM8706_LASI_STATUS		(MII_ADDR_C45 | 0x39005)
+
+#ifdef CONFIG_OF_MDIO
+/*
+ * Set and/or override some configuration registers based on the
+ * broadcom,c45-reg-init property stored in the of_node for the phydev.
+ *
+ * broadcom,c45-reg-init = <devid reg mask value>,...;
+ *
+ * There may be one or more sets of <devid reg mask value>:
+ *
+ * devid: which sub-device to use.
+ * reg: the register.
+ * mask: if non-zero, ANDed with existing register value.
+ * value: ORed with the masked value and written to the regiser.
+ *
+ */
+static int bcm8706_of_reg_init(struct phy_device *phydev)
+{
+	const __be32 *paddr;
+	int len, i, ret;
+
+	if (!phydev->dev.of_node)
+		return 0;
+
+	paddr = of_get_property(phydev->dev.of_node,
+				"broadcom,c45-reg-init", &len);
+	if (!paddr || len < (4 * sizeof(*paddr)))
+		return 0;
+
+	ret = 0;
+	len /= sizeof(*paddr);
+	for (i = 0; i < len - 3; i += 4) {
+		u16 devid = be32_to_cpup(paddr + i);
+		u16 reg = be32_to_cpup(paddr + i + 1);
+		u16 mask = be32_to_cpup(paddr + i + 2);
+		u16 val_bits = be32_to_cpup(paddr + i + 3);
+		int val;
+		u32 regnum = MII_ADDR_C45 | (devid << 16) | reg;
+		val = 0;
+		if (mask) {
+			val = phy_read(phydev, regnum);
+			if (val < 0) {
+				ret = val;
+				goto err;
+			}
+			val &= mask;
+		}
+		val |= val_bits;
+
+		ret = phy_write(phydev, regnum, val);
+		if (ret < 0)
+			goto err;
+
+	}
+err:
+	return ret;
+}
+#else
+static int bcm8706_of_reg_init(struct phy_device *phydev)
+{
+	return 0;
+}
+#endif /* CONFIG_OF_MDIO */
+
+static int bcm8706_config_init(struct phy_device *phydev)
+{
+	phydev->supported = SUPPORTED_10000baseR_FEC;
+	phydev->advertising = ADVERTISED_10000baseR_FEC;
+	phydev->state = PHY_NOLINK;
+
+	bcm8706_of_reg_init(phydev);
+
+	return 0;
+}
+
+static int bcm8706_config_aneg(struct phy_device *phydev)
+{
+	return -EINVAL;
+}
+
+static int bcm8706_read_status(struct phy_device *phydev)
+{
+	int rx_signal_detect;
+	int pcs_status;
+	int xgxs_lane_status;
+
+	rx_signal_detect = phy_read(phydev, BCM8706_PMD_RX_SIGNAL_DETECT);
+	if (rx_signal_detect < 0)
+		return rx_signal_detect;
+
+	if ((rx_signal_detect & 1) == 0)
+		goto no_link;
+
+	pcs_status = phy_read(phydev, BCM8706_10GBASER_PCS_STATUS);
+	if (pcs_status < 0)
+		return pcs_status;
+
+	if ((pcs_status & 1) == 0)
+		goto no_link;
+
+	xgxs_lane_status = phy_read(phydev, BCM8706_XGXS_LANE_STATUS);
+	if (xgxs_lane_status < 0)
+		return xgxs_lane_status;
+
+	if ((xgxs_lane_status & 0x1000) == 0)
+		goto no_link;
+
+	phydev->speed = 10000;
+	phydev->link = 1;
+	phydev->duplex = 1;
+	return 0;
+
+no_link:
+	phydev->link = 0;
+	return 0;
+}
+
+static int bcm8706_config_intr(struct phy_device *phydev)
+{
+	int reg, err;
+
+	reg = phy_read(phydev, BCM8706_LASI_CONTROL);
+
+	if (reg < 0)
+		return reg;
+
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+		reg |= 1;
+	else
+		reg &= ~1;
+
+	err = phy_write(phydev, BCM8706_LASI_CONTROL, reg);
+	return err;
+}
+
+static int bcm8706_did_interrupt(struct phy_device *phydev)
+{
+	int reg;
+
+	reg = phy_read(phydev, BCM8706_LASI_STATUS);
+
+	if (reg < 0) {
+		dev_err(&phydev->dev,
+			"Error: Read of BCM8706_LASI_STATUS failed: %d\n", reg);
+		return 0;
+	}
+	return (reg & 1) != 0;
+}
+
+static int bcm8706_ack_interrupt(struct phy_device *phydev)
+{
+	/* Reading the LASI status clears it. */
+	bcm8706_did_interrupt(phydev);
+	return 0;
+}
+
+
+static struct phy_driver bcm8706_driver = {
+	.phy_id		= PHY_ID_BCM8706,
+	.phy_id_mask	= 0xffffffff,
+	.name		= "Broadcom BCM8706",
+	.flags		= PHY_HAS_INTERRUPT,
+	.config_init	= bcm8706_config_init,
+	.config_aneg	= bcm8706_config_aneg,
+	.read_status	= bcm8706_read_status,
+	.ack_interrupt	= bcm8706_ack_interrupt,
+	.config_intr	= bcm8706_config_intr,
+	.did_interrupt	= bcm8706_did_interrupt,
+	.driver		= { .owner = THIS_MODULE },
+};
+
+static int __init bcm8706_init(void)
+{
+	int ret;
+
+	ret = phy_driver_register(&bcm8706_driver);
+
+	return ret;
+}
+module_init(bcm8706_init);
+
+static void __exit bcm8706_exit(void)
+{
+	phy_driver_unregister(&bcm8706_driver);
+}
+module_exit(bcm8706_exit);
+
+static struct mdio_device_id __maybe_unused bcm8706_tbl[] = {
+	{ PHY_ID_BCM8706, 0xffffffff },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(mdio, bcm8706_tbl);
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index b840a49..e06a56a 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -9,6 +9,7 @@
 #define PHY_ID_BCM5464			0x002060b0
 #define PHY_ID_BCM5461			0x002060c0
 #define PHY_ID_BCM57780			0x03625d90
+#define PHY_ID_BCM8706			0x0143bdc1
 
 #define PHY_BCM_OUI_MASK		0xfffffc00
 #define PHY_BCM_OUI_1			0x00206000
-- 
1.7.2.3

Re: [PATCH 1/3] netdev/phy: Handle IEEE802.3 clause 45 Ethernet PHYs

[Grant Likely]

On Wed, Oct 12, 2011 at 11:06:21AM -0700, David Daney wrote:
> The IEEE802.3 clause 45 MDIO bus protocol allows for directly
> addressing PHY registers using a 21 bit address, and is used by many
> 10G Ethernet PHYS.  Already existing is the ability of MDIO bus
> drivers to use clause 45, with the MII_ADDR_C45 flag.  Here we add
> some support in the PHY and device tree infrastructure to use these
> PHYs.
> 
> Normally the MII_ADDR_C45 flag is ORed with the register address to
> indicate a clause 45 transaction.  Here we also use this flag in the
> *device* address passed to get_phy_id() and get_phy_device() to
> indicate that probing should be done with clause 45 transactions.  If
> a PHY is successfully probed with MII_ADDR_C45, the new struct
> phy_device is_c45 flag is set for the PHY.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>

Minor comment below, but otherwise,

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index e4c3844..0a25e2c 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -246,6 +246,7 @@ struct sk_buff;
>   * phy_id: UID for this device found during discovery
>   * state: state of the PHY for management purposes
>   * dev_flags: Device-specific flags used by the PHY driver.
> + * is_c45:  Set to 1 if this phy uses clause 45 addressing.

"Set to true if ..."

>   * addr: Bus address of PHY
>   * link_timeout: The number of timer firings to wait before the
>   * giving up on the current attempt at acquiring a link
> @@ -283,6 +284,8 @@ struct phy_device {
>  
>  	u32 dev_flags;
>  
> +	unsigned int is_c45:1;
> +

bool

>  	phy_interface_t interface;
>  
>  	/* Bus address of the PHY (0-31) */
> -- 
> 1.7.2.3
> 

Re: [PATCH 2/3] netdev/phy/of: Handle IEEE802.3 clause 45 Ethernet PHYs in of_mdiobus_register()

[Grant Likely]

On Wed, Oct 12, 2011 at 11:06:22AM -0700, David Daney wrote:
> Define two new "compatible" values for Ethernet
> PHYs. "ethernet-phy-ieee802.3-c22" and "ethernet-phy-ieee802.3-c45"
> are used to indicate a PHY uses the corresponding protocol.
> 
> If a PHY is "compatible" with "ethernet-phy-ieee802.3-c45", we
> indicate this so that get_phy_device() can properly probe the device.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>

I'm okay with this binding.  I'd like to get opinions from other
developers before it is committed to though.

Acked-by: Grant Likely <grant.likely@secretlab.ca>

g.

> ---
>  Documentation/devicetree/bindings/net/phy.txt |   12 +++++++++++-
>  drivers/of/of_mdio.c                          |    4 ++++
>  2 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
> index bb8c742..de9cb32 100644
> --- a/Documentation/devicetree/bindings/net/phy.txt
> +++ b/Documentation/devicetree/bindings/net/phy.txt
> @@ -16,8 +16,18 @@ Required properties:
>  
>  Example:
>  
> +Optional Properties:
> +
> +- compatible: Compatible list, may contain "ethernet-phy-ieee802.3-c22" or
> +              "ethernet-phy-ieee802.3-c45" for PHYs that implement
> +              IEEE802.3 clause 22 or IEEE802.3 clause 45
> +              specifications.  If neither of these are specified, the
> +              default is to assume clause 22.  The compatible list may
> +              also contain other elements.
> +
>  ethernet-phy@0 {
> -	linux,phandle = <2452000>
> +	compatible = "ethernet-phy-ieee802.3-c22";
> +	linux,phandle = <2452000>;
>  	interrupt-parent = <40000>;
>  	interrupts = <35 1>;
>  	reg = <0>;
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 7c28e8c..f837a7f 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -79,6 +79,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
>  				mdio->irq[addr] = PHY_POLL;
>  		}
>  
> +		if (of_device_is_compatible(child,
> +					    "ethernet-phy-ieee802.3-c45"))
> +			addr |= MII_ADDR_C45;
> +
>  		phy = get_phy_device(mdio, addr);
>  		if (!phy || IS_ERR(phy)) {
>  			dev_err(&mdio->dev, "error probing PHY at address %i\n",
> -- 
> 1.7.2.3
> 

Re: [PATCH 3/3] netdev/phy: Add driver for Broadcom BCM8706 10G Ethernet PHY

[Grant Likely]

On Wed, Oct 12, 2011 at 11:06:23AM -0700, David Daney wrote:
> Add a driver and PHY_ID number for said device.  This is a 10Gig PHY
> which uses MII_ADDR_C45 addressing, it is always 10G full duplex, so
> there is no autonegotiation.  All we do is report link state and send
> interrupts when it changes.
> 
> If the PHY has a device tree of_node associated with it, the
> "broadcom,c45-reg-init" property is used to supply register
> initialization values when config_init() is called.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  .../devicetree/bindings/net/broadcom-bcm8706.txt   |   28 +++
>  drivers/net/phy/Kconfig                            |    5 +
>  drivers/net/phy/Makefile                           |    1 +
>  drivers/net/phy/bcm8706.c                          |  212 ++++++++++++++++++++
>  include/linux/brcmphy.h                            |    1 +
>  5 files changed, 247 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
>  create mode 100644 drivers/net/phy/bcm8706.c
> 
> diff --git a/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt b/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
> new file mode 100644
> index 0000000..d58bea9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
> @@ -0,0 +1,28 @@
> +The Broadcom BCM8706 is a 10G Ethernet PHY.  It has these bindings in
> +addition to the standard PHY bindings.
> +
> +Compatible: Should contain "broadcom,bcm8706" and
> +            "ethernet-phy-ieee802.3-c45"
> +
> +Optional Properties:
> +
> +- broadcom,c45-reg-init : one of more sets of 4 cells.  The first cell
> +  is the device type, the second a register address, the third cell
> +  contains a mask to be ANDed with the existing register value, and
> +  the fourth cell is ORed with he result to yield the new register
> +  value.

... a mask value of '0' should also guarantee that the driver does not do a read before the write.

What have we got so far in this regard for other phys and devices?  I
don't think it necessary to put 'c45' in the property name.  reg-init
should be sufficient.  I'd like to hear from others if it would be
valuable to have a 'reg-init-sequence' property of the above format.

What does the device type cell indicate?  Wouldn't the driver
naturally have the device id from the address of the cell?

> +static int __init bcm8706_init(void)
> +{
> +	int ret;
> +
> +	ret = phy_driver_register(&bcm8706_driver);
> +
> +	return ret;
> +}
> +module_init(bcm8706_init);

or simply:
static int __init bcm8706_init(void)
{
	return phy_driver_register(&bcm8706_driver);
}
module_init(bcm8706_init);

Otherwise the driver code seems to be fine.

g.

Re: [PATCH 3/3] netdev/phy: Add driver for Broadcom BCM8706 10G Ethernet PHY

[David Daney]

On 10/12/2011 05:31 PM, Grant Likely wrote:
> On Wed, Oct 12, 2011 at 11:06:23AM -0700, David Daney wrote:
>> Add a driver and PHY_ID number for said device.  This is a 10Gig PHY
>> which uses MII_ADDR_C45 addressing, it is always 10G full duplex, so
>> there is no autonegotiation.  All we do is report link state and send
>> interrupts when it changes.
>>
>> If the PHY has a device tree of_node associated with it, the
>> "broadcom,c45-reg-init" property is used to supply register
>> initialization values when config_init() is called.
>>
>> Signed-off-by: David Daney<david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>> ---
>>
[...]
>> +The Broadcom BCM8706 is a 10G Ethernet PHY.  It has these bindings in
>> +addition to the standard PHY bindings.
>> +
>> +Compatible: Should contain "broadcom,bcm8706" and
>> +            "ethernet-phy-ieee802.3-c45"
>> +
>> +Optional Properties:
>> +
>> +- broadcom,c45-reg-init : one of more sets of 4 cells.  The first cell
>> +  is the device type, the second a register address, the third cell
>> +  contains a mask to be ANDed with the existing register value, and
>> +  the fourth cell is ORed with he result to yield the new register
>> +  value.
> ... a mask value of '0' should also guarantee that the driver does not do a read before the write.

The implementation does that, I will update the binding text to reflect 
this.

> What have we got so far in this regard for other phys and devices?

http://devicetree.org/Compatible%3Dmarvell,88e1149r

This is basically the same thing adapted for the page select register 
specific to Marvell PHYs.

>    I
> don't think it necessary to put 'c45' in the property name.  reg-init
> should be sufficient.

10M/100M/1G PHYs from different manufacturers and even within a single 
manufacturer have a wide variety of ways to multiplex many registers 
into the 5 bit addressing scheme allowed by clause 22.  The Marvell 
scheme already implemented doesn't work for Broadcom.

For clause 45, there are more address bits...

>    I'd like to hear from others if it would be
> valuable to have a 'reg-init-sequence' property of the above format.

A clause 45 specific property might work for *all* 10G PHYs, the same 
cannot be said for clause 22, hence my idea to put 'c45' in the name

> What does the device type cell indicate?  Wouldn't the driver
> naturally have the device id from the address of the cell?
>

There are three portions to a clause 45 address:

phy_id:  Denoted by the "reg" property is a 5-bit value that identifies 
a particular PHY on the MDIO bus.

device id: Really a sub-device within a given PHY, another 5-bit value 
contained in the first cell of the proposed register init sequence.  
Clause 45 defines several different standard device ids.

register id: a 16-bit address that identifies a particular 16-bit 
register within the 'device' (or sub-device if you will.

Does that answer your question?

In theory we could compose the 5-bit device id and 16-bit register 
address into a single 32-bit cell in the init sequence property, but I 
chose to have them separate.

>> +static int __init bcm8706_init(void)
>> +{
>> +	int ret;
>> +
>> +	ret = phy_driver_register(&bcm8706_driver);
>> +
>> +	return ret;
>> +}
>> +module_init(bcm8706_init);
> or simply:
> static int __init bcm8706_init(void)
> {
> 	return phy_driver_register(&bcm8706_driver);
> }
> module_init(bcm8706_init);
>

Yes, I will make that change.

David Daney

Re: [PATCH 1/3] netdev/phy: Handle IEEE802.3 clause 45 Ethernet PHYs

[Ben Hutchings]

On Wed, 2011-10-12 at 11:06 -0700, David Daney wrote:
> The IEEE802.3 clause 45 MDIO bus protocol allows for directly
> addressing PHY registers using a 21 bit address, and is used by many
> 10G Ethernet PHYS.  Already existing is the ability of MDIO bus
> drivers to use clause 45, with the MII_ADDR_C45 flag.  Here we add
> some support in the PHY and device tree infrastructure to use these
> PHYs.
> 
> Normally the MII_ADDR_C45 flag is ORed with the register address to
> indicate a clause 45 transaction.  Here we also use this flag in the
> *device* address passed to get_phy_id() and get_phy_device() to
> indicate that probing should be done with clause 45 transactions.  If
> a PHY is successfully probed with MII_ADDR_C45, the new struct
> phy_device is_c45 flag is set for the PHY.

That deserves a comment next to the definition of the macro.

> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  drivers/net/phy/phy_device.c |   25 ++++++++++++++++++++++---
>  include/linux/phy.h          |    3 +++
>  2 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 83a5a5a..7e4d61b 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -171,6 +171,8 @@ static struct phy_device* phy_device_create(struct mii_bus *bus,
>  
>  	dev->autoneg = AUTONEG_ENABLE;
>  
> +	dev->is_c45 = (addr & MII_ADDR_C45) != 0;
> +	addr &= ~MII_ADDR_C45;
>  	dev->addr = addr;
>  	dev->phy_id = phy_id;
>  	dev->bus = bus;
> @@ -205,15 +207,24 @@ static struct phy_device* phy_device_create(struct mii_bus *bus,
>   * @phy_id: where to store the ID retrieved.
>   *
>   * Description: Reads the ID registers of the PHY at @addr on the
> - *   @bus, stores it in @phy_id and returns zero on success.
> + *   @bus, stores it in @phy_id and returns zero on success.  If the
> + *   @addr has been ORed with MII_ADDR_C45, mdio clause 45 data
> + *   transfer is used to read ID from the PHY device, otherwise the
> + *   standard protocol (clause 22) is used.
>   */
>  int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id)
>  {
>  	int phy_reg;
> +	u32 c45_reg_base = 0;
>  
> +	if (addr & MII_ADDR_C45) {
> +		addr &= ~MII_ADDR_C45;
> +		/* Access the PHY's PHY XS registers with C45 mode. */
> +		c45_reg_base = MII_ADDR_C45 | 0x40000;
> +	}
[...]

I'm not sure it's a safe assumption that every PHY has a PHY XS block.
That said, I've not seen any that don't.  mdio45_probe() should work out
which blocks are there, if you can set up an mdio_if_info for it.

It would be nice if someone would try to improve integration between
mdio/mii and phylib rather than duplicating logic and interfaces.  I'm
afraid I'm not going to be spending time on MDIO, though, since we've
putting PHY drivers in firmware rather than on the host now.

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.