From: David Daney <[email protected]>
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 | 239 ++++++++++++++++++++
drivers/net/phy/mdio_bus.c | 7 +
drivers/net/phy/phy_device.c | 110 +++++++++-
drivers/of/of_mdio.c | 14 +-
include/linux/phy.h | 32 +++-
9 files changed, 436 insertions(+), 13 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/broadcom-bcm87xx.txt
create mode 100644 drivers/net/phy/bcm87xx.c
--
1.7.2.3
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.
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_device() to indicate that probing
should be done with clause 45 transactions.
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/phy_device.c | 110 +++++++++++++++++++++++++++++++++++++++---
include/linux/phy.h | 25 +++++++++-
2 files changed, 126 insertions(+), 9 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 18ab0da..fa0f558 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,
+ struct phy_c45_device_ids *c45_ids)
{
struct phy_device *dev;
@@ -174,8 +174,12 @@ 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;
+ if (c45_ids)
+ dev->c45_ids = *c45_ids;
dev->bus = bus;
dev->dev.parent = bus->parent;
dev->dev.bus = &mdio_bus_type;
@@ -200,20 +204,104 @@ 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;
+
+ /*
+ * Find first non-zero Devices In package. Device
+ * zero is reserved, so don't probe it.
+ */
+ for (i = 1;
+ i < ARRAY_SIZE(c45_ids->device_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 < ARRAY_SIZE(c45_ids->device_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.
+ * @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,
+ struct phy_c45_device_ids *c45_ids)
{
int phy_reg;
+ if (addr & MII_ADDR_C45) {
+ addr &= ~MII_ADDR_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);
@@ -241,14 +329,17 @@ static int get_phy_id(struct mii_bus *bus, int addr, u32 *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 @addr & MII_ADDR_C45 is not zero, the PHY is assumed to be
+ * 802.3-c45.
*/
struct phy_device * get_phy_device(struct mii_bus *bus, int addr)
{
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, &c45_ids);
if (r)
return ERR_PTR(r);
@@ -256,7 +347,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, &c45_ids);
return dev;
}
@@ -449,6 +540,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 c291cae..92d53ee 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -83,8 +83,12 @@ typedef enum {
*/
#define MII_BUS_ID_SIZE (20 - 3)
-/* Or MII_ADDR_C45 into regnum for read/write on mii_bus to enable the 21 bit
- IEEE 802.3ae clause 45 addressing mode used by 10GIGE phy chips. */
+/*
+ * Or MII_ADDR_C45 into regnum for read/write on mii_bus to enable the
+ * 21 bit IEEE 802.3ae clause 45 addressing mode used by 10GIGE phy
+ * chips. Also may be ORed into the device address in
+ * get_phy_device().
+ */
#define MII_ADDR_C45 (1<<30)
struct device;
@@ -243,6 +247,16 @@ enum phy_state {
PHY_RESUMING
};
+/*
+ * 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 +264,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 +301,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,6 +499,8 @@ 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 *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
+ struct phy_c45_device_ids *c45_ids);
struct phy_device* get_phy_device(struct mii_bus *bus, int addr);
int phy_device_register(struct phy_device *phy);
int phy_init_hw(struct phy_device *phydev);
--
1.7.2.3
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 | 239 ++++++++++++++++++++
4 files changed, 274 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..b2f6a43
--- /dev/null
+++ b/drivers/net/phy/bcm87xx.c
@@ -0,0 +1,239 @@
+/*
+ * 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
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 | 7 +++++++
2 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 31470b0..9a283b4 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 92d53ee..bbd9e7f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -434,6 +434,13 @@ struct phy_driver {
/* Handles ethtool queries for hardware time stamping. */
int (*ts_info)(struct phy_device *phydev, struct ethtool_ts_info *ti);
+ /*
+ * 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 SIOCSHWTSTAMP ioctl for hardware time stamping. */
int (*hwtstamp)(struct phy_device *phydev, struct ifreq *ifr);
--
1.7.2.3
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 | 14 +++++++++++---
2 files changed, 22 insertions(+), 4 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 2574abd..0f08aaf 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -79,11 +79,19 @@ 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",
- addr);
- continue;
+ phy = phy_device_create(mdio, addr, 0, 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
From: David Daney <[email protected]>
Date: Fri, 22 Jun 2012 17:24:13 -0700
> 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.
>
> 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_device() to indicate that probing
> should be done with clause 45 transactions.
>
> 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]>
I see no value in having two ways to say that clause-45 transactions
should be used.
Either make it a PHY device attribute, or specify it in the address
in the register accesses, but not both.
Also your patch is full of coding style errors, I simply couldn't
stomache applying this even if I agreed with the substance of the
changes:
> + i < ARRAY_SIZE(c45_ids->device_ids) &&
> + c45_ids->devices_in_package == 0;
c45_ids on the second line should line up with the initial 'i'
on the first line.
> + c45_ids->devices_in_package = (phy_reg & 0xffff) << 16;
> +
> +
> + reg_addr = MII_ADDR_C45 | i << 16 | 5;
There is not reason in the world to have two empty lines there, it
looks awful.
> + /*
> + * If mostly Fs, there is no device there,
> + * let's get out of here.
> + */
Format comments:
/* Like
* this.
*/
Not.
/*
* Like
* this.
*/
> + c45_ids->device_ids[i] = (phy_reg & 0xffff) << 16;
> +
> +
> + reg_addr = MII_ADDR_C45 | i << 16 | MII_PHYSID2;
Two empty lines. This is extremely irritating, it looks like you
had some kind of debugging code here and then were very lazy about
removing it.
> +/*
> + * Or MII_ADDR_C45 into regnum for read/write on mii_bus to enable the
> + * 21 bit IEEE 802.3ae clause 45 addressing mode used by 10GIGE phy
> + * chips. Also may be ORed into the device address in
> + * get_phy_device().
> + */
Comment formatting.
> +/*
> + * 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.
> + */
If you're going to list the struct members use the correct kerneldoc
format to do so.
On 06/25/2012 03:34 PM, David Miller wrote:
> From: David Daney<[email protected]>
> Date: Fri, 22 Jun 2012 17:24:13 -0700
>
>> 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.
>>
>> 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_device() to indicate that probing
>> should be done with clause 45 transactions.
>>
>> 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]>
>
> I see no value in having two ways to say that clause-45 transactions
> should be used.
>
> Either make it a PHY device attribute, or specify it in the address
> in the register accesses, but not both.
>
Do you realize that at the time get_phy_device() is called, there is no
PHY device? So there can be no attribute, nor are we passing a register
address. Neither of these suggestions apply to this situation.
We need to know a priori if it is c22 or c45. So we need to communicate
the type somehow to get_phy_device(). I chose an unused bit in the addr
parameter to do this, another option would be to add a separate
parameter to get_phy_device() specifying the type.
> Also your patch is full of coding style errors, I simply couldn't
> stomache applying this even if I agreed with the substance of the
> changes:
>
>> + i< ARRAY_SIZE(c45_ids->device_ids)&&
>> + c45_ids->devices_in_package == 0;
>
> c45_ids on the second line should line up with the initial 'i'
> on the first line.
>
>> + c45_ids->devices_in_package = (phy_reg& 0xffff)<< 16;
>> +
>> +
>> + reg_addr = MII_ADDR_C45 | i<< 16 | 5;
>
> There is not reason in the world to have two empty lines there, it
> looks awful.
OK, I will fix those...
>
>> + /*
>> + * If mostly Fs, there is no device there,
>> + * let's get out of here.
>> + */
>
> Format comments:
>
> /* Like
> * this.
> */
>
> Not.
>
> /*
> * Like
> * this.
> */
... and this one too I guess. Really you and Linus should come to a
consensus on this one.
[...]
>
>> +/*
>> + * 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.
>> + */
>
> If you're going to list the struct members use the correct kerneldoc
> format to do so.
OK.
David Daney
From: David Daney <[email protected]>
Date: Mon, 25 Jun 2012 16:11:23 -0700
> Do you realize that at the time get_phy_device() is called, there is
> no PHY device? So there can be no attribute, nor are we passing a
> register address. Neither of these suggestions apply to this
> situation.
>
> We need to know a priori if it is c22 or c45. So we need to
> communicate the type somehow to get_phy_device(). I chose an unused
> bit in the addr parameter to do this, another option would be to add a
> separate parameter to get_phy_device() specifying the type.
Then pass it in to the get() routine and store the attribute there
in the device we end up with.
There are many parameters that go into a PHY register access, so
we'll probably some day end up with a descriptor struct that the
caller prepares on-stack to pass into the actual read/write ops
via reference.
> ... and this one too I guess. Really you and Linus should come to a
> consensus on this one.
We did come up with a consensus, which is that subsystem maintainers
such as myself are at liberty to enforce localized coding style for
the bodies of code they maintain.
On 06/25/2012 04:33 PM, David Miller wrote:
> From: David Daney<[email protected]>
> Date: Mon, 25 Jun 2012 16:11:23 -0700
>
>> Do you realize that at the time get_phy_device() is called, there is
>> no PHY device? So there can be no attribute, nor are we passing a
>> register address. Neither of these suggestions apply to this
>> situation.
>>
>> We need to know a priori if it is c22 or c45. So we need to
>> communicate the type somehow to get_phy_device(). I chose an unused
>> bit in the addr parameter to do this, another option would be to add a
>> separate parameter to get_phy_device() specifying the type.
>
> Then pass it in to the get() routine and store the attribute there
> in the device we end up with.
OK.
addr has only 5 significant bits, and the patch *does* pass the
information (c22 vs. c45) in one of the high order bits. So it is
essentially as you say, but you don't like the idea of multiplexing the
arguments into a single int.
Therefore, I am going to propose that we add a 'flags' parameter to
get_phy_device() and change the (two) callers.
Does that seem better (or at least acceptable)?
Or do you really want to pass the address of a (one bit) structure instead?
David Daney
>
> There are many parameters that go into a PHY register access, so
> we'll probably some day end up with a descriptor struct that the
> caller prepares on-stack to pass into the actual read/write ops
> via reference.
>
From: David Daney <[email protected]>
Date: Mon, 25 Jun 2012 16:48:40 -0700
> Therefore, I am going to propose that we add a 'flags' parameter to
> get_phy_device() and change the (two) callers.
Since there is only one flag, make it simply a bool for now.