2012-06-27 17:34:39

by David Daney

[permalink] [raw]
Subject: [PATCH v2 0/4] netdev/phy: 10G PHY support.

From: David Daney <[email protected]>

The only non-cosmetic change from v1 is to pass an additional argument
to get_phy_device() that indicates that the PHY uses 802.3 clause 45
signaling, previously I had been using a high order bit of the addr
parameter for this.

There are also changes from v1 in the code and comment formatting.
These should now be closer to what David Miller prefers.

>From v1:

The existing PHY driver infrastructure supports IEEE 802.3 Clause 22
PHYs used with 10/100/1000MB Ethernet. For 10G Ethernet, many PHYs
use 802.3 Clause 45. These patches attempt to add core support for
this as well as a driver for BCM87XX 10G PHY devices.

This is reworked from patches I send about 9 months ago:

http://marc.info/?l=linux-netdev&m=131844282403852

Several of the patches have device tree bindings in them, so the
device tree people get to enjoy them too.

David Daney (4):
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/of: Add more methods for binding PHY devices to drivers.
netdev/phy: Add driver for Broadcom BCM87XX 10G Ethernet PHYs

.../devicetree/bindings/net/broadcom-bcm87xx.txt | 29 +++
Documentation/devicetree/bindings/net/phy.txt | 12 +-
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/bcm87xx.c | 238 ++++++++++++++++++++
drivers/net/phy/mdio_bus.c | 9 +-
drivers/net/phy/phy_device.c | 105 ++++++++-
drivers/of/of_mdio.c | 16 +-
include/linux/phy.h | 24 ++-
9 files changed, 424 insertions(+), 15 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/broadcom-bcm87xx.txt
create mode 100644 drivers/net/phy/bcm87xx.c

--
1.7.2.3


2012-06-27 17:33:50

by David Daney

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

From: David Daney <[email protected]>

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.

If get_phy_device() fails, it was probably due to failing the probe of
the PHY identifier registers. Since we have the device tree telling
us the PHY exists, go ahead and add it anyhow with a phy_id of zero.
There may be a driver match based on the "compatible" property.

Signed-off-by: David Daney <[email protected]>
---
Documentation/devicetree/bindings/net/phy.txt | 12 +++++++++++-
drivers/of/of_mdio.c | 16 ++++++++++++----
2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index bb8c742..7cd18fb 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -14,10 +14,20 @@ Required properties:
- linux,phandle : phandle for this node; likely referenced by an
ethernet controller node.

+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.
+
Example:

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 6c24cad..8e6c25f 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -57,6 +57,7 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
const __be32 *paddr;
u32 addr;
int len;
+ bool is_c45;

/* A PHY must have a reg property in the range [0-31] */
paddr = of_get_property(child, "reg", &len);
@@ -79,11 +80,18 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
mdio->irq[addr] = PHY_POLL;
}

- phy = get_phy_device(mdio, addr, false);
+ is_c45 = of_device_is_compatible(child,
+ "ethernet-phy-ieee802.3-c45");
+ phy = get_phy_device(mdio, addr, is_c45);
+
if (!phy || IS_ERR(phy)) {
- dev_err(&mdio->dev, "error probing PHY at address %i\n",
- addr);
- continue;
+ phy = phy_device_create(mdio, addr, 0, false, NULL);
+ if (!phy || IS_ERR(phy)) {
+ dev_err(&mdio->dev,
+ "error creating PHY at address %i\n",
+ addr);
+ continue;
+ }
}

/* Associate the OF node with the device structure so it
--
1.7.2.3

2012-06-27 17:33:49

by David Daney

[permalink] [raw]
Subject: [PATCH v2 3/4] netdev/phy/of: Add more methods for binding PHY devices to drivers.

From: David Daney <[email protected]>

Allow PHY drivers to supply their own device matching function
(match_phy_device()), or to be matched OF compatible properties.

PHYs following IEEE802.3 clause 45 have more than one device
identifier constants, which breaks the default device matching code.
Other 10G PHYs don't follow the standard manufacturer/device
identifier register layout standards, but they do use the standard
MDIO bus protocols for register access. Both of these require
adjustments to the PHY driver to device matching code.

If the there is an of_node associated with such a PHY, we can match it
to its driver using the "compatible" properties, just as we do with
certain platform devices. If the "compatible" property match fails,
first check if there is a driver supplied matching function, and if
not fall back to the existing identifier matching rules.

Signed-off-by: David Daney <[email protected]>
---
drivers/net/phy/mdio_bus.c | 7 +++++++
include/linux/phy.h | 6 ++++++
2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 2cee6d2..170eb41 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -25,6 +25,7 @@
#include <linux/init.h>
#include <linux/delay.h>
#include <linux/device.h>
+#include <linux/of_device.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
#include <linux/skbuff.h>
@@ -308,6 +309,12 @@ static int mdio_bus_match(struct device *dev, struct device_driver *drv)
struct phy_device *phydev = to_phy_device(dev);
struct phy_driver *phydrv = to_phy_driver(drv);

+ if (of_driver_match_device(dev, drv))
+ return 1;
+
+ if (phydrv->match_phy_device)
+ return phydrv->match_phy_device(phydev);
+
return ((phydrv->phy_id & phydrv->phy_id_mask) ==
(phydev->phy_id & phydrv->phy_id_mask));
}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 597d05d..7eac80a 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -426,6 +426,12 @@ struct phy_driver {
/* Clears up any memory if needed */
void (*remove)(struct phy_device *phydev);

+ /* Returns true if this is a suitable driver for the given
+ * phydev. If NULL, matching is based on phy_id and
+ * phy_id_mask.
+ */
+ int (*match_phy_device)(struct phy_device *phydev);
+
/* Handles ethtool queries for hardware time stamping. */
int (*ts_info)(struct phy_device *phydev, struct ethtool_ts_info *ti);

--
1.7.2.3

2012-06-27 17:34:43

by David Daney

[permalink] [raw]
Subject: [PATCH v2 4/4] netdev/phy: Add driver for Broadcom BCM87XX 10G Ethernet PHYs

From: David Daney <[email protected]>

Add a driver for BCM8706 and BCM8727 devices. These are a 10Gig PHYs
which use MII_ADDR_C45 addressing. They are 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 <[email protected]>
---
.../devicetree/bindings/net/broadcom-bcm87xx.txt | 29 +++
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/bcm87xx.c | 238 ++++++++++++++++++++
4 files changed, 273 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/broadcom-bcm87xx.txt
create mode 100644 drivers/net/phy/bcm87xx.c

diff --git a/Documentation/devicetree/bindings/net/broadcom-bcm87xx.txt b/Documentation/devicetree/bindings/net/broadcom-bcm87xx.txt
new file mode 100644
index 0000000..7c86d5e
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/broadcom-bcm87xx.txt
@@ -0,0 +1,29 @@
+The Broadcom BCM87XX devices are a family of 10G Ethernet PHYs. They
+have these bindings in addition to the standard PHY bindings.
+
+Compatible: Should contain "broadcom,bcm8706" or "broadcom,bcm8727" and
+ "ethernet-phy-ieee802.3-c45"
+
+Optional Properties:
+
+- broadcom,c45-reg-init : one of more sets of 4 cells. The first cell
+ is the MDIO Manageable Device (MMD) address, the second a register
+ address within the MMD, 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. If the third cell has a
+ value of zero, no read of the existing value is performed.
+
+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 944cdfb..3090dc6 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -67,6 +67,11 @@ config BCM63XX_PHY
---help---
Currently supports the 6348 and 6358 PHYs.

+config BCM87XX_PHY
+ tristate "Driver for Broadcom BCM8706 and BCM8727 PHYs"
+ help
+ Currently supports the BCM8706 and BCM8727 10G Ethernet PHYs.
+
config ICPLUS_PHY
tristate "Drivers for ICPlus PHYs"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index f51af68..6d2dc6c 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_BCM87XX_PHY) += bcm87xx.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/bcm87xx.c b/drivers/net/phy/bcm87xx.c
new file mode 100644
index 0000000..f5f0562
--- /dev/null
+++ b/drivers/net/phy/bcm87xx.c
@@ -0,0 +1,238 @@
+/*
+ * 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 - 2012 Cavium, Inc.
+ */
+
+#include <linux/module.h>
+#include <linux/phy.h>
+#include <linux/of.h>
+
+#define PHY_ID_BCM8706 0x0143bdc1
+#define PHY_ID_BCM8727 0x0143bff0
+
+#define BCM87XX_PMD_RX_SIGNAL_DETECT (MII_ADDR_C45 | 0x1000a)
+#define BCM87XX_10GBASER_PCS_STATUS (MII_ADDR_C45 | 0x30020)
+#define BCM87XX_XGXS_LANE_STATUS (MII_ADDR_C45 | 0x40018)
+
+#define BCM87XX_LASI_CONTROL (MII_ADDR_C45 | 0x39002)
+#define BCM87XX_LASI_STATUS (MII_ADDR_C45 | 0x39005)
+
+#if IS_ENABLED(CONFIG_OF_MDIO)
+/* Set and/or override some configuration registers based on the
+ * marvell,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 bcm87xx_of_reg_init(struct phy_device *phydev)
+{
+ const __be32 *paddr;
+ const __be32 *paddr_end;
+ int len, ret;
+
+ if (!phydev->dev.of_node)
+ return 0;
+
+ paddr = of_get_property(phydev->dev.of_node,
+ "broadcom,c45-reg-init", &len);
+ if (!paddr)
+ return 0;
+
+ paddr_end = paddr + (len /= sizeof(*paddr));
+
+ ret = 0;
+
+ while (paddr + 3 < paddr_end) {
+ u16 devid = be32_to_cpup(paddr++);
+ u16 reg = be32_to_cpup(paddr++);
+ u16 mask = be32_to_cpup(paddr++);
+ u16 val_bits = be32_to_cpup(paddr++);
+ 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 bcm87xx_of_reg_init(struct phy_device *phydev)
+{
+ return 0;
+}
+#endif /* CONFIG_OF_MDIO */
+
+static int bcm87xx_config_init(struct phy_device *phydev)
+{
+ phydev->supported = SUPPORTED_10000baseR_FEC;
+ phydev->advertising = ADVERTISED_10000baseR_FEC;
+ phydev->state = PHY_NOLINK;
+
+ bcm87xx_of_reg_init(phydev);
+
+ return 0;
+}
+
+static int bcm87xx_config_aneg(struct phy_device *phydev)
+{
+ return -EINVAL;
+}
+
+static int bcm87xx_read_status(struct phy_device *phydev)
+{
+ int rx_signal_detect;
+ int pcs_status;
+ int xgxs_lane_status;
+
+ rx_signal_detect = phy_read(phydev, BCM87XX_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, BCM87XX_10GBASER_PCS_STATUS);
+ if (pcs_status < 0)
+ return pcs_status;
+
+ if ((pcs_status & 1) == 0)
+ goto no_link;
+
+ xgxs_lane_status = phy_read(phydev, BCM87XX_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 bcm87xx_config_intr(struct phy_device *phydev)
+{
+ int reg, err;
+
+ reg = phy_read(phydev, BCM87XX_LASI_CONTROL);
+
+ if (reg < 0)
+ return reg;
+
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+ reg |= 1;
+ else
+ reg &= ~1;
+
+ err = phy_write(phydev, BCM87XX_LASI_CONTROL, reg);
+ return err;
+}
+
+static int bcm87xx_did_interrupt(struct phy_device *phydev)
+{
+ int reg;
+
+ reg = phy_read(phydev, BCM87XX_LASI_STATUS);
+
+ if (reg < 0) {
+ dev_err(&phydev->dev,
+ "Error: Read of BCM87XX_LASI_STATUS failed: %d\n", reg);
+ return 0;
+ }
+ return (reg & 1) != 0;
+}
+
+static int bcm87xx_ack_interrupt(struct phy_device *phydev)
+{
+ /* Reading the LASI status clears it. */
+ bcm87xx_did_interrupt(phydev);
+ return 0;
+}
+
+static int bcm8706_match_phy_device(struct phy_device *phydev)
+{
+ return phydev->c45_ids.device_ids[4] == PHY_ID_BCM8706;
+}
+
+static int bcm8727_match_phy_device(struct phy_device *phydev)
+{
+ return phydev->c45_ids.device_ids[4] == PHY_ID_BCM8727;
+}
+
+static struct phy_driver bcm8706_driver = {
+ .phy_id = PHY_ID_BCM8706,
+ .phy_id_mask = 0xffffffff,
+ .name = "Broadcom BCM8706",
+ .flags = PHY_HAS_INTERRUPT,
+ .config_init = bcm87xx_config_init,
+ .config_aneg = bcm87xx_config_aneg,
+ .read_status = bcm87xx_read_status,
+ .ack_interrupt = bcm87xx_ack_interrupt,
+ .config_intr = bcm87xx_config_intr,
+ .did_interrupt = bcm87xx_did_interrupt,
+ .match_phy_device = bcm8706_match_phy_device,
+ .driver = { .owner = THIS_MODULE },
+};
+
+static struct phy_driver bcm8727_driver = {
+ .phy_id = PHY_ID_BCM8727,
+ .phy_id_mask = 0xffffffff,
+ .name = "Broadcom BCM8727",
+ .flags = PHY_HAS_INTERRUPT,
+ .config_init = bcm87xx_config_init,
+ .config_aneg = bcm87xx_config_aneg,
+ .read_status = bcm87xx_read_status,
+ .ack_interrupt = bcm87xx_ack_interrupt,
+ .config_intr = bcm87xx_config_intr,
+ .did_interrupt = bcm87xx_did_interrupt,
+ .match_phy_device = bcm8727_match_phy_device,
+ .driver = { .owner = THIS_MODULE },
+};
+
+static int __init bcm87xx_init(void)
+{
+ int ret;
+
+ ret = phy_driver_register(&bcm8706_driver);
+ if (ret)
+ goto err;
+
+ ret = phy_driver_register(&bcm8727_driver);
+err:
+ return ret;
+}
+module_init(bcm87xx_init);
+
+static void __exit bcm87xx_exit(void)
+{
+ phy_driver_unregister(&bcm8706_driver);
+ phy_driver_unregister(&bcm8727_driver);
+}
+module_exit(bcm87xx_exit);
--
1.7.2.3

2012-06-27 17:34:41

by David Daney

[permalink] [raw]
Subject: [PATCH v2 1/4] netdev/phy: Handle IEEE802.3 clause 45 Ethernet PHYs

From: David Daney <[email protected]>

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
struct phy_c45_device_ids to hold the device identifier registers
present in clause 45. struct phy_device gets a couple of new fields:
c45_ids to hold the identifiers and is_c45 to signal that it is clause
45.

get_phy_device() gets a new parameter is_c45 to indicate that the PHY
device should use the clause 45 protocol, and its callers are adjusted
to pass false. The follow-on patch to of_mdio.c will pass true where
appropriate.

EXPORT phy_device_create() so that the follow-on patch to of_mdio.c
can use it to create phy devices for PHYs, that have non-standard
device identifier registers, based on the device tree bindings.

Signed-off-by: David Daney <[email protected]>
---
drivers/net/phy/mdio_bus.c | 2 +-
drivers/net/phy/phy_device.c | 105 ++++++++++++++++++++++++++++++++++++++---
drivers/of/of_mdio.c | 2 +-
include/linux/phy.h | 18 +++++++-
4 files changed, 116 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 31470b0..2cee6d2 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -232,7 +232,7 @@ struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
struct phy_device *phydev;
int err;

- phydev = get_phy_device(bus, addr);
+ phydev = get_phy_device(bus, addr, false);
if (IS_ERR(phydev) || phydev == NULL)
return phydev;

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 18ab0da..ef4cdee 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -152,8 +152,8 @@ int phy_scan_fixups(struct phy_device *phydev)
}
EXPORT_SYMBOL(phy_scan_fixups);

-static struct phy_device* phy_device_create(struct mii_bus *bus,
- int addr, int phy_id)
+struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
+ bool is_c45, struct phy_c45_device_ids *c45_ids)
{
struct phy_device *dev;

@@ -174,8 +174,11 @@ static struct phy_device* phy_device_create(struct mii_bus *bus,

dev->autoneg = AUTONEG_ENABLE;

+ dev->is_c45 = is_c45;
dev->addr = addr;
dev->phy_id = phy_id;
+ if (c45_ids)
+ dev->c45_ids = *c45_ids;
dev->bus = bus;
dev->dev.parent = bus->parent;
dev->dev.bus = &mdio_bus_type;
@@ -200,20 +203,99 @@ static struct phy_device* phy_device_create(struct mii_bus *bus,

return dev;
}
+EXPORT_SYMBOL(phy_device_create);
+
+/**
+ * get_phy_c45_ids - reads the specified addr for its 802.3-c45 IDs.
+ * @bus: the target MII bus
+ * @addr: PHY address on the MII bus
+ * @phy_id: where to store the ID retrieved.
+ * @c45_ids: where to store the c45 ID information.
+ *
+ * If the PHY devices-in-package appears to be valid, it and the
+ * corresponding identifiers are stored in @c45_ids, zero is stored
+ * in @phy_id. Otherwise 0xffffffff is stored in @phy_id. Returns
+ * zero on success.
+ *
+ */
+static int get_phy_c45_ids(struct mii_bus *bus, int addr, u32 *phy_id,
+ struct phy_c45_device_ids *c45_ids) {
+ int phy_reg;
+ int i, reg_addr;
+ const int num_ids = ARRAY_SIZE(c45_ids->device_ids);
+
+ /* Find first non-zero Devices In package. Device
+ * zero is reserved, so don't probe it.
+ */
+ for (i = 1;
+ i < num_ids && c45_ids->devices_in_package == 0;
+ i++) {
+ reg_addr = MII_ADDR_C45 | i << 16 | 6;
+ phy_reg = mdiobus_read(bus, addr, reg_addr);
+ if (phy_reg < 0)
+ return -EIO;
+ c45_ids->devices_in_package = (phy_reg & 0xffff) << 16;
+
+ reg_addr = MII_ADDR_C45 | i << 16 | 5;
+ phy_reg = mdiobus_read(bus, addr, reg_addr);
+ if (phy_reg < 0)
+ return -EIO;
+ c45_ids->devices_in_package |= (phy_reg & 0xffff);
+
+ /* If mostly Fs, there is no device there,
+ * let's get out of here.
+ */
+ if ((c45_ids->devices_in_package & 0x1fffffff) == 0x1fffffff) {
+ *phy_id = 0xffffffff;
+ return 0;
+ }
+ }
+
+ /* Now probe Device Identifiers for each device present. */
+ for (i = 1; i < num_ids; i++) {
+ if (!(c45_ids->devices_in_package & (1 << i)))
+ continue;
+
+ reg_addr = MII_ADDR_C45 | i << 16 | MII_PHYSID1;
+ phy_reg = mdiobus_read(bus, addr, reg_addr);
+ if (phy_reg < 0)
+ return -EIO;
+ c45_ids->device_ids[i] = (phy_reg & 0xffff) << 16;
+
+ reg_addr = MII_ADDR_C45 | i << 16 | MII_PHYSID2;
+ phy_reg = mdiobus_read(bus, addr, reg_addr);
+ if (phy_reg < 0)
+ return -EIO;
+ c45_ids->device_ids[i] |= (phy_reg & 0xffff);
+ }
+ *phy_id = 0;
+ return 0;
+}

/**
* get_phy_id - reads the specified addr for its ID.
* @bus: the target MII bus
* @addr: PHY address on the MII bus
* @phy_id: where to store the ID retrieved.
+ * @is_c45: If true the PHY uses the 802.3 clause 45 protocol
+ * @c45_ids: where to store the c45 ID information.
+ *
+ * Description: In the case of a 802.3-c22 PHY, reads the ID registers
+ * of the PHY at @addr on the @bus, stores it in @phy_id and returns
+ * zero on success.
+ *
+ * In the case of a 802.3-c45 PHY, get_phy_c45_ids() is invoked, and
+ * its return value is in turn returned.
*
- * Description: Reads the ID registers of the PHY at @addr on the
- * @bus, stores it in @phy_id and returns zero on success.
*/
-static int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id)
+static int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id,
+ bool is_c45, struct phy_c45_device_ids *c45_ids)
{
int phy_reg;

+ if (is_c45)
+ return get_phy_c45_ids(bus, addr, phy_id, c45_ids);
+
/* Grab the bits from PHYIR1, and put them
* in the upper half */
phy_reg = mdiobus_read(bus, addr, MII_PHYSID1);
@@ -238,17 +320,19 @@ static int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id)
* get_phy_device - reads the specified PHY device and returns its @phy_device struct
* @bus: the target MII bus
* @addr: PHY address on the MII bus
+ * @is_c45: If true the PHY uses the 802.3 clause 45 protocol
*
* Description: Reads the ID registers of the PHY at @addr on the
* @bus, then allocates and returns the phy_device to represent it.
*/
-struct phy_device * get_phy_device(struct mii_bus *bus, int addr)
+struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45)
{
struct phy_device *dev = NULL;
u32 phy_id;
+ struct phy_c45_device_ids c45_ids = {0};
int r;

- r = get_phy_id(bus, addr, &phy_id);
+ r = get_phy_id(bus, addr, &phy_id, is_c45, &c45_ids);
if (r)
return ERR_PTR(r);

@@ -256,7 +340,7 @@ struct phy_device * get_phy_device(struct mii_bus *bus, int addr)
if ((phy_id & 0x1fffffff) == 0x1fffffff)
return NULL;

- dev = phy_device_create(bus, addr, phy_id);
+ dev = phy_device_create(bus, addr, phy_id, is_c45, &c45_ids);

return dev;
}
@@ -449,6 +533,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/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 2574abd..6c24cad 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -79,7 +79,7 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
mdio->irq[addr] = PHY_POLL;
}

- phy = get_phy_device(mdio, addr);
+ phy = get_phy_device(mdio, addr, false);
if (!phy || IS_ERR(phy)) {
dev_err(&mdio->dev, "error probing PHY at address %i\n",
addr);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index c291cae..597d05d 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -243,6 +243,15 @@ enum phy_state {
PHY_RESUMING
};

+/**
+ * struct phy_c45_device_ids - 802.3-c45 Device Identifiers
+ * @devices_in_package: Bit vector of devices present.
+ * @device_ids: The device identifer for each present device.
+ */
+struct phy_c45_device_ids {
+ u32 devices_in_package;
+ u32 device_ids[8];
+};

/* phy_device: An instance of a PHY
*
@@ -250,6 +259,8 @@ enum phy_state {
* bus: Pointer to the bus this PHY is on
* dev: driver model device structure for this PHY
* phy_id: UID for this device found during discovery
+ * c45_ids: 802.3-c45 Device Identifers if is_c45.
+ * is_c45: Set to true if this phy uses clause 45 addressing.
* state: state of the PHY for management purposes
* dev_flags: Device-specific flags used by the PHY driver.
* addr: Bus address of PHY
@@ -285,6 +296,9 @@ struct phy_device {

u32 phy_id;

+ struct phy_c45_device_ids c45_ids;
+ bool is_c45;
+
enum phy_state state;

u32 dev_flags;
@@ -480,7 +494,9 @@ static inline int phy_write(struct phy_device *phydev, u32 regnum, u16 val)
return mdiobus_write(phydev->bus, phydev->addr, regnum, val);
}

-struct phy_device* get_phy_device(struct mii_bus *bus, int addr);
+struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
+ bool is_c45, struct phy_c45_device_ids *c45_ids);
+struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45);
int phy_device_register(struct phy_device *phy);
int phy_init_hw(struct phy_device *phydev);
struct phy_device * phy_attach(struct net_device *dev,
--
1.7.2.3

2012-06-28 04:29:43

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] netdev/phy: 10G PHY support.

From: David Daney <[email protected]>
Date: Wed, 27 Jun 2012 10:33:34 -0700

> From: David Daney <[email protected]>
>
> The only non-cosmetic change from v1 is to pass an additional argument
> to get_phy_device() that indicates that the PHY uses 802.3 clause 45
> signaling, previously I had been using a high order bit of the addr
> parameter for this.
>
> There are also changes from v1 in the code and comment formatting.
> These should now be closer to what David Miller prefers.

Applied, but I had to add the following warning fixup:

--------------------
phy: Fix warning in get_phy_device().

drivers/net/phy/phy_device.c: In function ?get_phy_device?:
drivers/net/phy/phy_device.c:340:14: warning: ?phy_id? may be used uninitialized in this function [-Wmaybe-uninitialized]

GCC can't see that when we return zero we always initialize
phy_id and that's the only path where we use it.

Initialize phy_id to zero to shut it up.

Signed-off-by: David S. Miller <[email protected]>
---
drivers/net/phy/phy_device.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index ef4cdee..47e02e7 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -327,9 +327,9 @@ static int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id,
*/
struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45)
{
- struct phy_device *dev = NULL;
- u32 phy_id;
struct phy_c45_device_ids c45_ids = {0};
+ struct phy_device *dev = NULL;
+ u32 phy_id = 0;
int r;

r = get_phy_id(bus, addr, &phy_id, is_c45, &c45_ids);
--
1.7.10.2

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2012-06-28 17:05:57

by David Daney

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] netdev/phy: 10G PHY support.

On 06/27/2012 09:29 PM, David Miller wrote:
> From: David Daney<[email protected]>
> Date: Wed, 27 Jun 2012 10:33:34 -0700
>
>> From: David Daney<[email protected]>
>>
>> The only non-cosmetic change from v1 is to pass an additional argument
>> to get_phy_device() that indicates that the PHY uses 802.3 clause 45
>> signaling, previously I had been using a high order bit of the addr
>> parameter for this.
>>
>> There are also changes from v1 in the code and comment formatting.
>> These should now be closer to what David Miller prefers.
>
> Applied, but I had to add the following warning fixup:

Thank You.

>
> --------------------
> phy: Fix warning in get_phy_device().
>
> drivers/net/phy/phy_device.c: In function ?get_phy_device?:
> drivers/net/phy/phy_device.c:340:14: warning: ?phy_id? may be used uninitialized in this function [-Wmaybe-uninitialized]
>
> GCC can't see that when we return zero we always initialize
> phy_id and that's the only path where we use it.
>
> Initialize phy_id to zero to shut it up.
>

FWIW: I was testing with GCC-4.6.3 and saw no such warnings.

David Daney


> Signed-off-by: David S. Miller<[email protected]>
> ---
> drivers/net/phy/phy_device.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index ef4cdee..47e02e7 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -327,9 +327,9 @@ static int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id,
> */
> struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45)
> {
> - struct phy_device *dev = NULL;
> - u32 phy_id;
> struct phy_c45_device_ids c45_ids = {0};
> + struct phy_device *dev = NULL;
> + u32 phy_id = 0;
> int r;
>
> r = get_phy_id(bus, addr,&phy_id, is_c45,&c45_ids);