[PATCH 0/5] Add phylib support for MV88X3310 10G phy

[PATCH 0/5] Add phylib support for MV88X3310 10G phy

[Russell King - ARM Linux]

Hi,

This patch series adds support for the Marvell 88x3310 PHY found on
the SolidRun Macchiatobin board.

The first patch introduces a set of generic Clause 45 PHY helpers that
C45 PHY drivers can make use of if they wish.

Patch 2 fixes the aneg restart to be compatible with C45 PHYs - it can
currently only cope with C22 PHYs.

Patch 3 moves the "gen10g" driver into the Clause 45 code, grouping all
core clause 45 code together.

Patch 4 adds the phy_interface_t types for XAUI and 10GBase-KR links.
As 10GBase-KR appears to be compatible with XFI and SFI, XFI and SFI,
I currently see no reason to add XFI and SFI interface modes.  There
seems to be vendor code out there using these, but they all alias back
to the same hardware settings.

Patch 5 adds support for the MV88X3310 PHY, which supports both the
copper and fiber interfaces.  It should be noted that the MV88X3310
automatically switches its MAC facing interface between 10GBase-KR
and SGMII depending on the negotiated speed.  This was discussed with
Florian, and we agreed to update the phy interface mode depending on
the properties of the actual link mode to the PHY.

 Documentation/devicetree/bindings/net/ethernet.txt |   2 +
 MAINTAINERS                                        |   6 +
 drivers/net/phy/Makefile                           |   4 +-
 drivers/net/phy/marvell10g.c                       | 364 +++++++++++++++++++++
 drivers/net/phy/phy-c45.c                          | 295 +++++++++++++++++
 drivers/net/phy/phy.c                              |  23 +-
 drivers/net/phy/phy_device.c                       | 113 ++-----
 include/linux/phy.h                                |  20 ++
 8 files changed, 729 insertions(+), 98 deletions(-)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 1/5] net: phy: add 802.3 clause 45 support to phylib

[Russell King]

Add generic helpers for 802.3 clause 45 PHYs for >= 10Gbps support.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/Makefile     |   2 +-
 drivers/net/phy/phy-c45.c    | 231 +++++++++++++++++++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c |  20 ++--
 include/linux/phy.h          |  12 +++
 4 files changed, 250 insertions(+), 15 deletions(-)
 create mode 100644 drivers/net/phy/phy-c45.c

diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index e36db9a2ba38..19eddf758c88 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -1,6 +1,6 @@
 # Makefile for Linux PHY drivers and MDIO bus drivers
 
-libphy-y			:= phy.o phy-core.o phy_device.o
+libphy-y			:= phy.o phy-c45.o phy-core.o phy_device.o
 mdio-bus-y			+= mdio_bus.o mdio_device.o
 
 ifdef CONFIG_MDIO_DEVICE
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
new file mode 100644
index 000000000000..252e49864215
--- /dev/null
+++ b/drivers/net/phy/phy-c45.c
@@ -0,0 +1,231 @@
+/*
+ * Clause 45 PHY support
+ */
+#include <linux/ethtool.h>
+#include <linux/export.h>
+#include <linux/mdio.h>
+#include <linux/mii.h>
+#include <linux/phy.h>
+
+/**
+ * genphy_c45_setup_forced - configures a forced speed
+ * @phydev: target phy_device struct
+ */
+int genphy_c45_pma_setup_forced(struct phy_device *phydev)
+{
+	int ctrl1, ctrl2, ret;
+
+	/* Half duplex is not supported */
+	if (phydev->duplex != DUPLEX_FULL)
+		return -EINVAL;
+
+	ctrl1 = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1);
+	if (ctrl1 < 0)
+		return ctrl1;
+
+	ctrl2 = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL2);
+	if (ctrl2 < 0)
+		return ctrl2;
+
+	ctrl1 &= ~MDIO_CTRL1_SPEEDSEL;
+	/* PMA/PMD type selection is 1.7.5:0 not 1.7.3:0.  See 45.2.1.6.1. */
+	ctrl2 &= ~(MDIO_PMA_CTRL2_TYPE | 0x30);
+
+	switch (phydev->speed) {
+	case SPEED_10:
+		ctrl2 |= MDIO_PMA_CTRL2_10BT;
+		break;
+	case SPEED_100:
+		ctrl1 |= MDIO_PMA_CTRL1_SPEED100;
+		ctrl2 |= MDIO_PMA_CTRL2_100BTX;
+		break;
+	case SPEED_1000:
+		ctrl1 |= MDIO_PMA_CTRL1_SPEED1000;
+		/* Assume 1000base-T */
+		ctrl2 |= MDIO_PMA_CTRL2_1000BT;
+		break;
+	case SPEED_10000:
+		ctrl1 |= MDIO_CTRL1_SPEED10G;
+		/* Assume 10Gbase-T */
+		ctrl2 |= MDIO_PMA_CTRL2_10GBT;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1, ctrl1);
+	if (ret < 0)
+		return ret;
+
+	return phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL2, ctrl2);
+}
+EXPORT_SYMBOL_GPL(genphy_c45_pma_setup_forced);
+
+/**
+ * genphy_c45_an_disable_aneg - disable auto-negotiation
+ * @phydev: target phy_device struct
+ *
+ * Disable auto-negotiation in the Clause 45 PHY. The link parameters
+ * parameters are controlled through the PMA/PMD MMD registers.
+ *
+ * Returns zero on success, negative errno code on failure.
+ */
+int genphy_c45_an_disable_aneg(struct phy_device *phydev)
+{
+	int val;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1);
+	if (val < 0)
+		return val;
+
+	val &= ~(MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART);
+
+	return phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1, val);
+}
+EXPORT_SYMBOL_GPL(genphy_c45_an_disable_aneg);
+
+/**
+ * genphy_c45_restart_aneg - Enable and restart auto-negotiation
+ * @phydev: target phy_device struct
+ *
+ * This assumes that the auto-negotiation MMD is present.
+ *
+ * Enable and restart auto-negotiation.
+ */
+int genphy_c45_restart_aneg(struct phy_device *phydev)
+{
+	int val;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1);
+	if (val < 0)
+		return val;
+
+	val |= MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART;
+
+	return phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1, val);
+}
+EXPORT_SYMBOL_GPL(genphy_c45_restart_aneg);
+
+/**
+ * genphy_c45_aneg_done - return auto-negotiation complete status
+ * @phydev: target phy_device struct
+ *
+ * This assumes that the auto-negotiation MMD is present.
+ *
+ * Reads the status register from the auto-negotiation MMD, returning:
+ * - positive if auto-negotiation is complete
+ * - negative errno code on error
+ * - zero otherwise
+ */
+int genphy_c45_aneg_done(struct phy_device *phydev)
+{
+	int val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
+
+	return val < 0 ? val : val & MDIO_AN_STAT1_COMPLETE ? 1 : 0;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_aneg_done);
+
+/**
+ * genphy_c45_read_link - read the overall link status from the MMDs
+ * @phydev: target phy_device struct
+ * @mmd_mask: MMDs to read status from
+ *
+ * Read the link status from the specified MMDs, and if they all indicate
+ * that the link is up, return positive.  If an error is encountered,
+ * a negative errno will be returned, otherwise zero.
+ */
+int genphy_c45_read_link(struct phy_device *phydev, u32 mmd_mask)
+{
+	int val, devad;
+	bool link = true;
+
+	while (mmd_mask) {
+		devad = __ffs(mmd_mask);
+		mmd_mask &= ~BIT(devad);
+
+		/* The link state is latched low so that momentary link
+		 * drops can be detected.  Do not double-read the status
+		 * register if the link is down.
+		 */
+		val = phy_read_mmd(phydev, devad, MDIO_STAT1);
+		if (val < 0)
+			return val;
+
+		if (!(val & MDIO_STAT1_LSTATUS))
+			link = false;
+	}
+
+	return link;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_read_link);
+
+/**
+ * genphy_c45_read_lpa - read the link partner advertisment and pause
+ * @phydev: target phy_device struct
+ *
+ * Read the Clause 45 defined base (7.19) and 10G (7.33) status registers,
+ * filling in the link partner advertisment, pause and asym_pause members
+ * in @phydev.  This assumes that the auto-negotiation MMD is present, and
+ * the backplane bit (7.48.0) is clear.  Clause 45 PHY drivers are expected
+ * to fill in the remainder of the link partner advert from vendor registers.
+ */
+int genphy_c45_read_lpa(struct phy_device *phydev)
+{
+	int val;
+
+	/* Read the link partner's base page advertisment */
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_LPA);
+	if (val < 0)
+		return val;
+
+	phydev->lp_advertising = mii_lpa_to_ethtool_lpa_t(val);
+	phydev->pause = val & LPA_PAUSE_CAP ? 1 : 0;
+	phydev->asym_pause = val & LPA_PAUSE_ASYM ? 1 : 0;
+
+	/* Read the link partner's 10G advertisment */
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_STAT);
+	if (val < 0)
+		return val;
+
+	if (val & MDIO_AN_10GBT_STAT_LP10G)
+		phydev->lp_advertising |= ADVERTISED_10000baseT_Full;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_read_lpa);
+
+/**
+ * genphy_c45_read_pma - read link speed etc from PMA
+ * @phydev: target phy_device struct
+ */
+int genphy_c45_read_pma(struct phy_device *phydev)
+{
+	int val;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1);
+	if (val < 0)
+		return val;
+
+	switch (val & MDIO_CTRL1_SPEEDSEL) {
+	case 0:
+		phydev->speed = SPEED_10;
+		break;
+	case MDIO_PMA_CTRL1_SPEED100:
+		phydev->speed = SPEED_100;
+		break;
+	case MDIO_PMA_CTRL1_SPEED1000:
+		phydev->speed = SPEED_1000;
+		break;
+	case MDIO_CTRL1_SPEED10G:
+		phydev->speed = SPEED_10000;
+		break;
+	default:
+		phydev->speed = SPEED_UNKNOWN;
+		break;
+	}
+
+	phydev->duplex = DUPLEX_FULL;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_read_pma);
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 1219eeab69d1..040575dba98c 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1483,27 +1483,19 @@ EXPORT_SYMBOL(genphy_read_status);
 
 static int gen10g_read_status(struct phy_device *phydev)
 {
-	int devad, reg;
 	u32 mmd_mask = phydev->c45_ids.devices_in_package;
-
-	phydev->link = 1;
+	int ret;
 
 	/* For now just lie and say it's 10G all the time */
 	phydev->speed = SPEED_10000;
 	phydev->duplex = DUPLEX_FULL;
 
-	for (devad = 0; mmd_mask; devad++, mmd_mask = mmd_mask >> 1) {
-		if (!(mmd_mask & 1))
-			continue;
+	/* Avoid reading the vendor MMDs */
+	mmd_mask &= ~(BIT(MDIO_MMD_VEND1) | BIT(MDIO_MMD_VEND2));
 
-		/* Read twice because link state is latched and a
-		 * read moves the current state into the register
-		 */
-		phy_read_mmd(phydev, devad, MDIO_STAT1);
-		reg = phy_read_mmd(phydev, devad, MDIO_STAT1);
-		if (reg < 0 || !(reg & MDIO_STAT1_LSTATUS))
-			phydev->link = 0;
-	}
+	ret = genphy_c45_read_link(phydev, mmd_mask);
+
+	phydev->link = ret > 0 ? 1 : 0;
 
 	return 0;
 }
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e76e4adbc7c7..735ff1f98db3 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -816,6 +816,8 @@ static inline const char *phydev_name(const struct phy_device *phydev)
 void phy_attached_print(struct phy_device *phydev, const char *fmt, ...)
 	__printf(2, 3);
 void phy_attached_info(struct phy_device *phydev);
+
+/* Clause 22 PHY */
 int genphy_config_init(struct phy_device *phydev);
 int genphy_setup_forced(struct phy_device *phydev);
 int genphy_restart_aneg(struct phy_device *phydev);
@@ -830,6 +832,16 @@ static inline int genphy_no_soft_reset(struct phy_device *phydev)
 {
 	return 0;
 }
+
+/* Clause 45 PHY */
+int genphy_c45_restart_aneg(struct phy_device *phydev);
+int genphy_c45_aneg_done(struct phy_device *phydev);
+int genphy_c45_read_link(struct phy_device *phydev, u32 mmd_mask);
+int genphy_c45_read_lpa(struct phy_device *phydev);
+int genphy_c45_read_pma(struct phy_device *phydev);
+int genphy_c45_pma_setup_forced(struct phy_device *phydev);
+int genphy_c45_an_disable_aneg(struct phy_device *phydev);
+
 void phy_driver_unregister(struct phy_driver *drv);
 void phy_drivers_unregister(struct phy_driver *drv, int n);
 int phy_driver_register(struct phy_driver *new_driver, struct module *owner);
-- 
2.7.4

Re: [PATCH 1/5] net: phy: add 802.3 clause 45 support to phylib

[Andrew Lunn]

On Thu, Jun 01, 2017 at 11:26:31AM +0100, Russell King wrote:
> Add generic helpers for 802.3 clause 45 PHYs for >= 10Gbps support.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH 1/5] net: phy: add 802.3 clause 45 support to phylib

[Florian Fainelli]

On 06/01/2017 03:26 AM, Russell King wrote:
> Add generic helpers for 802.3 clause 45 PHYs for >= 10Gbps support.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---

> +	ctrl1 &= ~MDIO_CTRL1_SPEEDSEL;
> +	/* PMA/PMD type selection is 1.7.5:0 not 1.7.3:0.  See 45.2.1.6.1. */
> +	ctrl2 &= ~(MDIO_PMA_CTRL2_TYPE | 0x30);

You could add a define for this 0x30 to indicate that you are masking
off the reserved bits (there are more entries that are reserved
actually, but that would do for now).

With that:

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

> +
> +	switch (phydev->speed) {
> +	case SPEED_10:
> +		ctrl2 |= MDIO_PMA_CTRL2_10BT;
> +		break;
> +	case SPEED_100:
> +		ctrl1 |= MDIO_PMA_CTRL1_SPEED100;
> +		ctrl2 |= MDIO_PMA_CTRL2_100BTX;
> +		break;
> +	case SPEED_1000:
> +		ctrl1 |= MDIO_PMA_CTRL1_SPEED1000;
> +		/* Assume 1000base-T */
> +		ctrl2 |= MDIO_PMA_CTRL2_1000BT;
> +		break;
> +	case SPEED_10000:
> +		ctrl1 |= MDIO_CTRL1_SPEED10G;
> +		/* Assume 10Gbase-T */
> +		ctrl2 |= MDIO_PMA_CTRL2_10GBT;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1, ctrl1);
> +	if (ret < 0)
> +		return ret;
> +
> +	return phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL2, ctrl2);
> +}
> +EXPORT_SYMBOL_GPL(genphy_c45_pma_setup_forced);
> +
> +/**
> + * genphy_c45_an_disable_aneg - disable auto-negotiation
> + * @phydev: target phy_device struct
> + *
> + * Disable auto-negotiation in the Clause 45 PHY. The link parameters
> + * parameters are controlled through the PMA/PMD MMD registers.
> + *
> + * Returns zero on success, negative errno code on failure.
> + */
> +int genphy_c45_an_disable_aneg(struct phy_device *phydev)
> +{
> +	int val;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1);
> +	if (val < 0)
> +		return val;
> +
> +	val &= ~(MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART);
> +
> +	return phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1, val);
> +}
> +EXPORT_SYMBOL_GPL(genphy_c45_an_disable_aneg);
> +
> +/**
> + * genphy_c45_restart_aneg - Enable and restart auto-negotiation
> + * @phydev: target phy_device struct
> + *
> + * This assumes that the auto-negotiation MMD is present.
> + *
> + * Enable and restart auto-negotiation.
> + */
> +int genphy_c45_restart_aneg(struct phy_device *phydev)
> +{
> +	int val;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1);
> +	if (val < 0)
> +		return val;
> +
> +	val |= MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART;
> +
> +	return phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1, val);
> +}
> +EXPORT_SYMBOL_GPL(genphy_c45_restart_aneg);
> +
> +/**
> + * genphy_c45_aneg_done - return auto-negotiation complete status
> + * @phydev: target phy_device struct
> + *
> + * This assumes that the auto-negotiation MMD is present.
> + *
> + * Reads the status register from the auto-negotiation MMD, returning:
> + * - positive if auto-negotiation is complete
> + * - negative errno code on error
> + * - zero otherwise
> + */
> +int genphy_c45_aneg_done(struct phy_device *phydev)
> +{
> +	int val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
> +
> +	return val < 0 ? val : val & MDIO_AN_STAT1_COMPLETE ? 1 : 0;
> +}
> +EXPORT_SYMBOL_GPL(genphy_c45_aneg_done);
> +
> +/**
> + * genphy_c45_read_link - read the overall link status from the MMDs
> + * @phydev: target phy_device struct
> + * @mmd_mask: MMDs to read status from
> + *
> + * Read the link status from the specified MMDs, and if they all indicate
> + * that the link is up, return positive.  If an error is encountered,
> + * a negative errno will be returned, otherwise zero.
> + */
> +int genphy_c45_read_link(struct phy_device *phydev, u32 mmd_mask)
> +{
> +	int val, devad;
> +	bool link = true;
> +
> +	while (mmd_mask) {
> +		devad = __ffs(mmd_mask);
> +		mmd_mask &= ~BIT(devad);
> +
> +		/* The link state is latched low so that momentary link
> +		 * drops can be detected.  Do not double-read the status
> +		 * register if the link is down.
> +		 */
> +		val = phy_read_mmd(phydev, devad, MDIO_STAT1);
> +		if (val < 0)
> +			return val;
> +
> +		if (!(val & MDIO_STAT1_LSTATUS))
> +			link = false;
> +	}
> +
> +	return link;
> +}
> +EXPORT_SYMBOL_GPL(genphy_c45_read_link);
> +
> +/**
> + * genphy_c45_read_lpa - read the link partner advertisment and pause
> + * @phydev: target phy_device struct
> + *
> + * Read the Clause 45 defined base (7.19) and 10G (7.33) status registers,
> + * filling in the link partner advertisment, pause and asym_pause members
> + * in @phydev.  This assumes that the auto-negotiation MMD is present, and
> + * the backplane bit (7.48.0) is clear.  Clause 45 PHY drivers are expected
> + * to fill in the remainder of the link partner advert from vendor registers.
> + */
> +int genphy_c45_read_lpa(struct phy_device *phydev)
> +{
> +	int val;
> +
> +	/* Read the link partner's base page advertisment */
> +	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_LPA);
> +	if (val < 0)
> +		return val;
> +
> +	phydev->lp_advertising = mii_lpa_to_ethtool_lpa_t(val);
> +	phydev->pause = val & LPA_PAUSE_CAP ? 1 : 0;
> +	phydev->asym_pause = val & LPA_PAUSE_ASYM ? 1 : 0;
> +
> +	/* Read the link partner's 10G advertisment */
> +	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_STAT);
> +	if (val < 0)
> +		return val;
> +
> +	if (val & MDIO_AN_10GBT_STAT_LP10G)
> +		phydev->lp_advertising |= ADVERTISED_10000baseT_Full;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(genphy_c45_read_lpa);
> +
> +/**
> + * genphy_c45_read_pma - read link speed etc from PMA
> + * @phydev: target phy_device struct
> + */
> +int genphy_c45_read_pma(struct phy_device *phydev)
> +{
> +	int val;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1);
> +	if (val < 0)
> +		return val;
> +
> +	switch (val & MDIO_CTRL1_SPEEDSEL) {
> +	case 0:
> +		phydev->speed = SPEED_10;
> +		break;
> +	case MDIO_PMA_CTRL1_SPEED100:
> +		phydev->speed = SPEED_100;
> +		break;
> +	case MDIO_PMA_CTRL1_SPEED1000:
> +		phydev->speed = SPEED_1000;
> +		break;
> +	case MDIO_CTRL1_SPEED10G:
> +		phydev->speed = SPEED_10000;
> +		break;
> +	default:
> +		phydev->speed = SPEED_UNKNOWN;
> +		break;
> +	}
> +
> +	phydev->duplex = DUPLEX_FULL;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(genphy_c45_read_pma);
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 1219eeab69d1..040575dba98c 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1483,27 +1483,19 @@ EXPORT_SYMBOL(genphy_read_status);
>  
>  static int gen10g_read_status(struct phy_device *phydev)
>  {
> -	int devad, reg;
>  	u32 mmd_mask = phydev->c45_ids.devices_in_package;
> -
> -	phydev->link = 1;
> +	int ret;
>  
>  	/* For now just lie and say it's 10G all the time */
>  	phydev->speed = SPEED_10000;
>  	phydev->duplex = DUPLEX_FULL;
>  
> -	for (devad = 0; mmd_mask; devad++, mmd_mask = mmd_mask >> 1) {
> -		if (!(mmd_mask & 1))
> -			continue;
> +	/* Avoid reading the vendor MMDs */
> +	mmd_mask &= ~(BIT(MDIO_MMD_VEND1) | BIT(MDIO_MMD_VEND2));
>  
> -		/* Read twice because link state is latched and a
> -		 * read moves the current state into the register
> -		 */
> -		phy_read_mmd(phydev, devad, MDIO_STAT1);
> -		reg = phy_read_mmd(phydev, devad, MDIO_STAT1);
> -		if (reg < 0 || !(reg & MDIO_STAT1_LSTATUS))
> -			phydev->link = 0;
> -	}
> +	ret = genphy_c45_read_link(phydev, mmd_mask);
> +
> +	phydev->link = ret > 0 ? 1 : 0;
>  
>  	return 0;
>  }
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index e76e4adbc7c7..735ff1f98db3 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -816,6 +816,8 @@ static inline const char *phydev_name(const struct phy_device *phydev)
>  void phy_attached_print(struct phy_device *phydev, const char *fmt, ...)
>  	__printf(2, 3);
>  void phy_attached_info(struct phy_device *phydev);
> +
> +/* Clause 22 PHY */
>  int genphy_config_init(struct phy_device *phydev);
>  int genphy_setup_forced(struct phy_device *phydev);
>  int genphy_restart_aneg(struct phy_device *phydev);
> @@ -830,6 +832,16 @@ static inline int genphy_no_soft_reset(struct phy_device *phydev)
>  {
>  	return 0;
>  }
> +
> +/* Clause 45 PHY */
> +int genphy_c45_restart_aneg(struct phy_device *phydev);
> +int genphy_c45_aneg_done(struct phy_device *phydev);
> +int genphy_c45_read_link(struct phy_device *phydev, u32 mmd_mask);
> +int genphy_c45_read_lpa(struct phy_device *phydev);
> +int genphy_c45_read_pma(struct phy_device *phydev);
> +int genphy_c45_pma_setup_forced(struct phy_device *phydev);
> +int genphy_c45_an_disable_aneg(struct phy_device *phydev);
> +
>  void phy_driver_unregister(struct phy_driver *drv);
>  void phy_drivers_unregister(struct phy_driver *drv, int n);
>  int phy_driver_register(struct phy_driver *new_driver, struct module *owner);
> 


-- 
Florian

Re: [PATCH 1/5] net: phy: add 802.3 clause 45 support to phylib

[Russell King - ARM Linux]

On Thu, Jun 01, 2017 at 10:15:28AM -0700, Florian Fainelli wrote:
> On 06/01/2017 03:26 AM, Russell King wrote:
> > Add generic helpers for 802.3 clause 45 PHYs for >= 10Gbps support.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> 
> > +	ctrl1 &= ~MDIO_CTRL1_SPEEDSEL;
> > +	/* PMA/PMD type selection is 1.7.5:0 not 1.7.3:0.  See 45.2.1.6.1. */
> > +	ctrl2 &= ~(MDIO_PMA_CTRL2_TYPE | 0x30);
> 
> You could add a define for this 0x30 to indicate that you are masking
> off the reserved bits (there are more entries that are reserved
> actually, but that would do for now).

They are not reserved according to 802.3-2015 and 802.3-2012:

802.3-2012: 45.2.1.6.1 PMA/PMD type selection (1.7.5:0)
802.3-2015: 45.2.1.6.3 PMA/PMD type selection (1.7.5:0)

which defines the type selection as occupying six bits, but the definition
in the UAPI mdio.h header file is:

#define MDIO_PMA_CTRL2_TYPE             0x000f  /* PMA/PMD type selection */

I presume that this is according to an earlier version of 802.3.
Changing this definition may cause userspace and/or kernel regressions,
so I propose something like:

#define MDIO_PMA_CTRL2_TYPE_2012        0x003f  /* PMA/PMD type selection */

instead.  The rest of the reserved entries are not specified as being
part of the type selection (yet) so it would be inappropriate to change
their value.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 2/5] net: phy: hook up clause 45 autonegotiation restart

[Russell King]

genphy_restart_aneg() can only restart autonegotiation on clause 22
PHYs.  Add a phy_restart_aneg() function which selects between the
clause 22 and clause 45 restart functionality depending on the PHY
type.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phy.c | 23 +++++++++++++++++++++--
 include/linux/phy.h   |  1 +
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 82ab8fb82587..25b24789a409 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -149,6 +149,25 @@ static int phy_config_interrupt(struct phy_device *phydev, u32 interrupts)
 	return 0;
 }
 
+/**
+ * phy_restart_aneg - restart auto-negotiation
+ * @phydev: target phy_device struct
+ *
+ * Restart the autonegotiation on @phydev.  Returns >= 0 on success or
+ * negative errno on error.
+ */
+int phy_restart_aneg(struct phy_device *phydev)
+{
+	int ret;
+
+	if (phydev->is_c45)
+		ret = genphy_c45_restart_aneg(phydev);
+	else
+		ret = genphy_restart_aneg(phydev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phy_restart_aneg);
 
 /**
  * phy_aneg_done - return auto-negotiation status
@@ -1415,7 +1434,7 @@ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
 		/* Restart autonegotiation so the new modes get sent to the
 		 * link partner.
 		 */
-		ret = genphy_restart_aneg(phydev);
+		ret = phy_restart_aneg(phydev);
 		if (ret < 0)
 			return ret;
 	}
@@ -1474,6 +1493,6 @@ int phy_ethtool_nway_reset(struct net_device *ndev)
 	if (!phydev->drv)
 		return -EIO;
 
-	return genphy_restart_aneg(phydev);
+	return phy_restart_aneg(phydev);
 }
 EXPORT_SYMBOL(phy_ethtool_nway_reset);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 735ff1f98db3..45dfe1b2b003 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -793,6 +793,7 @@ int phy_start_aneg(struct phy_device *phydev);
 int phy_aneg_done(struct phy_device *phydev);
 
 int phy_stop_interrupts(struct phy_device *phydev);
+int phy_restart_aneg(struct phy_device *phydev);
 
 static inline int phy_read_status(struct phy_device *phydev)
 {
-- 
2.7.4

Re: [PATCH 2/5] net: phy: hook up clause 45 autonegotiation restart

[Andrew Lunn]

On Thu, Jun 01, 2017 at 11:26:36AM +0100, Russell King wrote:
> genphy_restart_aneg() can only restart autonegotiation on clause 22
> PHYs.  Add a phy_restart_aneg() function which selects between the
> clause 22 and clause 45 restart functionality depending on the PHY
> type.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/phy/phy.c | 23 +++++++++++++++++++++--
>  include/linux/phy.h   |  1 +
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 82ab8fb82587..25b24789a409 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -149,6 +149,25 @@ static int phy_config_interrupt(struct phy_device *phydev, u32 interrupts)
>  	return 0;
>  }
>  
> +/**
> + * phy_restart_aneg - restart auto-negotiation
> + * @phydev: target phy_device struct
> + *
> + * Restart the autonegotiation on @phydev.  Returns >= 0 on success or
> + * negative errno on error.
> + */
> +int phy_restart_aneg(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	if (phydev->is_c45)
> +		ret = genphy_c45_restart_aneg(phydev);
> +	else
> +		ret = genphy_restart_aneg(phydev);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_restart_aneg);

Hi Russell

Isn't the same sort of thing needed in phy_aneg_done()?

Thanks
	Andrew

Re: [PATCH 2/5] net: phy: hook up clause 45 autonegotiation restart

[Russell King - ARM Linux]

On Thu, Jun 01, 2017 at 02:23:50PM +0200, Andrew Lunn wrote:
> On Thu, Jun 01, 2017 at 11:26:36AM +0100, Russell King wrote:
> > genphy_restart_aneg() can only restart autonegotiation on clause 22
> > PHYs.  Add a phy_restart_aneg() function which selects between the
> > clause 22 and clause 45 restart functionality depending on the PHY
> > type.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/net/phy/phy.c | 23 +++++++++++++++++++++--
> >  include/linux/phy.h   |  1 +
> >  2 files changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index 82ab8fb82587..25b24789a409 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> > @@ -149,6 +149,25 @@ static int phy_config_interrupt(struct phy_device *phydev, u32 interrupts)
> >  	return 0;
> >  }
> >  
> > +/**
> > + * phy_restart_aneg - restart auto-negotiation
> > + * @phydev: target phy_device struct
> > + *
> > + * Restart the autonegotiation on @phydev.  Returns >= 0 on success or
> > + * negative errno on error.
> > + */
> > +int phy_restart_aneg(struct phy_device *phydev)
> > +{
> > +	int ret;
> > +
> > +	if (phydev->is_c45)
> > +		ret = genphy_c45_restart_aneg(phydev);
> > +	else
> > +		ret = genphy_restart_aneg(phydev);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(phy_restart_aneg);
> 
> Hi Russell
> 
> Isn't the same sort of thing needed in phy_aneg_done()?

No, because phy_aneg_done() already has hooks in it which PHY drivers
can use to override the default C22 implementation.

I did toy with providing a similar conditional in phy_aneg_done(), but
decided it was better to use the existing hooks where present, rather
than needlessly adding additional code.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/5] net: phy: hook up clause 45 autonegotiation restart

[Andrew Lunn]

> > > +/**
> > > + * phy_restart_aneg - restart auto-negotiation
> > > + * @phydev: target phy_device struct
> > > + *
> > > + * Restart the autonegotiation on @phydev.  Returns >= 0 on success or
> > > + * negative errno on error.
> > > + */
> > > +int phy_restart_aneg(struct phy_device *phydev)
> > > +{
> > > +	int ret;
> > > +
> > > +	if (phydev->is_c45)
> > > +		ret = genphy_c45_restart_aneg(phydev);
> > > +	else
> > > +		ret = genphy_restart_aneg(phydev);
> > > +
> > > +	return ret;
> > > +}
> > > +EXPORT_SYMBOL_GPL(phy_restart_aneg);
> > 
> > Hi Russell
> > 
> > Isn't the same sort of thing needed in phy_aneg_done()?
> 
> No, because phy_aneg_done() already has hooks in it which PHY drivers
> can use to override the default C22 implementation.
> 
> I did toy with providing a similar conditional in phy_aneg_done(), but
> decided it was better to use the existing hooks where present, rather
> than needlessly adding additional code.

Hi Russell

So you are saying a 10G PHY driver always needs to have a aneg_done
callback, even if it just needs to call phygen_c45_aneg_done?

This seems a bit error prone. I can see somebody writing a 10G driver,
leaving out aneg_done() and having the c22 version called. Is the read
of MII_BMSR likely to return 0xffff, since the register does not
exist? If so, genphy_aneg_done() is likely to always return
BMSR_ANEGCOMPLETE.

Seems like a trap waiting for somebody to fall into it. The additional
code might be worth it to avoid placing this trap.

      Andrew

Re: [PATCH 2/5] net: phy: hook up clause 45 autonegotiation restart

[Russell King - ARM Linux]

On Thu, Jun 01, 2017 at 03:05:27PM +0200, Andrew Lunn wrote:
> So you are saying a 10G PHY driver always needs to have a aneg_done
> callback, even if it just needs to call phygen_c45_aneg_done?
> 
> This seems a bit error prone. I can see somebody writing a 10G driver,
> leaving out aneg_done() and having the c22 version called. Is the read
> of MII_BMSR likely to return 0xffff, since the register does not
> exist? If so, genphy_aneg_done() is likely to always return
> BMSR_ANEGCOMPLETE.

Don't forget that the read will fail, so phy_read() will return a
negative number.  This practically (and I've tested it) means that
genphy_aneg_done() never indicates negotiation completion, and the
link never comes up.

> Seems like a trap waiting for somebody to fall into it. The additional
> code might be worth it to avoid placing this trap.

The "trap" means a non-functional driver, since the link doesn't
come up.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/5] net: phy: hook up clause 45 autonegotiation restart

[Andrew Lunn]

On Thu, Jun 01, 2017 at 02:09:00PM +0100, Russell King - ARM Linux wrote:
> On Thu, Jun 01, 2017 at 03:05:27PM +0200, Andrew Lunn wrote:
> > So you are saying a 10G PHY driver always needs to have a aneg_done
> > callback, even if it just needs to call phygen_c45_aneg_done?
> > 
> > This seems a bit error prone. I can see somebody writing a 10G driver,
> > leaving out aneg_done() and having the c22 version called. Is the read
> > of MII_BMSR likely to return 0xffff, since the register does not
> > exist? If so, genphy_aneg_done() is likely to always return
> > BMSR_ANEGCOMPLETE.
> 
> Don't forget that the read will fail, so phy_read() will return a
> negative number.

By fail, you mean return something like -EIO or -ETIMEOUT? Is this
guaranteed in the code somewhere? This particular Marvell PHY only
does c45. But i could imagine some other PHYs answering a c22 request
with 0xffff.

     Andrew

Re: [PATCH 2/5] net: phy: hook up clause 45 autonegotiation restart

[Russell King - ARM Linux]

On Thu, Jun 01, 2017 at 03:19:55PM +0200, Andrew Lunn wrote:
> On Thu, Jun 01, 2017 at 02:09:00PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Jun 01, 2017 at 03:05:27PM +0200, Andrew Lunn wrote:
> > > So you are saying a 10G PHY driver always needs to have a aneg_done
> > > callback, even if it just needs to call phygen_c45_aneg_done?
> > > 
> > > This seems a bit error prone. I can see somebody writing a 10G driver,
> > > leaving out aneg_done() and having the c22 version called. Is the read
> > > of MII_BMSR likely to return 0xffff, since the register does not
> > > exist? If so, genphy_aneg_done() is likely to always return
> > > BMSR_ANEGCOMPLETE.
> > 
> > Don't forget that the read will fail, so phy_read() will return a
> > negative number.
> 
> By fail, you mean return something like -EIO or -ETIMEOUT? Is this
> guaranteed in the code somewhere? This particular Marvell PHY only
> does c45. But i could imagine some other PHYs answering a c22 request
> with 0xffff.

Yes, C45 allows the PHYs to answer C22 as well, but then they have to
implement the C22 register set.  Such a PHY would be out of spec,
especially as what you're suggesting is that it answers C22 cycles
and fails to implement MII_BMSR.  I also think that there's a comment
in the 802.3 specs that says that unimplemented registers are to
return zero, not 0xffff.

So, if there is a non-compliant PHY like that, I think the time to
address it is when such a broken situation appears, rather than
engineering additional complexity before we know we need it.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/5] net: phy: hook up clause 45 autonegotiation restart

[Russell King - ARM Linux]

On Thu, Jun 01, 2017 at 04:47:35PM +0100, Russell King - ARM Linux wrote:
> On Thu, Jun 01, 2017 at 03:19:55PM +0200, Andrew Lunn wrote:
> > On Thu, Jun 01, 2017 at 02:09:00PM +0100, Russell King - ARM Linux wrote:
> > > On Thu, Jun 01, 2017 at 03:05:27PM +0200, Andrew Lunn wrote:
> > > > So you are saying a 10G PHY driver always needs to have a aneg_done
> > > > callback, even if it just needs to call phygen_c45_aneg_done?
> > > > 
> > > > This seems a bit error prone. I can see somebody writing a 10G driver,
> > > > leaving out aneg_done() and having the c22 version called. Is the read
> > > > of MII_BMSR likely to return 0xffff, since the register does not
> > > > exist? If so, genphy_aneg_done() is likely to always return
> > > > BMSR_ANEGCOMPLETE.
> > > 
> > > Don't forget that the read will fail, so phy_read() will return a
> > > negative number.
> > 
> > By fail, you mean return something like -EIO or -ETIMEOUT? Is this
> > guaranteed in the code somewhere? This particular Marvell PHY only
> > does c45. But i could imagine some other PHYs answering a c22 request
> > with 0xffff.
> 
> Yes, C45 allows the PHYs to answer C22 as well, but then they have to
> implement the C22 register set.  Such a PHY would be out of spec,
> especially as what you're suggesting is that it answers C22 cycles
> and fails to implement MII_BMSR.  I also think that there's a comment
> in the 802.3 specs that says that unimplemented registers are to
> return zero, not 0xffff.

Checking 802.3-2015, in "22.2.4 Management functions", the first
sentence requires all PHYs that respond to Clause 22 cycles to
implement BMCR and BMSR.  However, my statement about unimplemented
registers returning zero seems to be a C45 thing, not a C22 thing,
according to the C22 PICS and "45.2 MDIO Interface Registers"

However, digging a bit further, "22.2.4.2.10 Auto-Negotiation complete"
states that bit 5 shall be zero if aneg has not completed or if aneg is
unimplemented.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/5] net: phy: hook up clause 45 autonegotiation restart

[Florian Fainelli]

On 06/01/2017 09:24 AM, Russell King - ARM Linux wrote:
> On Thu, Jun 01, 2017 at 04:47:35PM +0100, Russell King - ARM Linux wrote:
>> On Thu, Jun 01, 2017 at 03:19:55PM +0200, Andrew Lunn wrote:
>>> On Thu, Jun 01, 2017 at 02:09:00PM +0100, Russell King - ARM Linux wrote:
>>>> On Thu, Jun 01, 2017 at 03:05:27PM +0200, Andrew Lunn wrote:
>>>>> So you are saying a 10G PHY driver always needs to have a aneg_done
>>>>> callback, even if it just needs to call phygen_c45_aneg_done?
>>>>>
>>>>> This seems a bit error prone. I can see somebody writing a 10G driver,
>>>>> leaving out aneg_done() and having the c22 version called. Is the read
>>>>> of MII_BMSR likely to return 0xffff, since the register does not
>>>>> exist? If so, genphy_aneg_done() is likely to always return
>>>>> BMSR_ANEGCOMPLETE.
>>>>
>>>> Don't forget that the read will fail, so phy_read() will return a
>>>> negative number.
>>>
>>> By fail, you mean return something like -EIO or -ETIMEOUT? Is this
>>> guaranteed in the code somewhere? This particular Marvell PHY only
>>> does c45. But i could imagine some other PHYs answering a c22 request
>>> with 0xffff.
>>
>> Yes, C45 allows the PHYs to answer C22 as well, but then they have to
>> implement the C22 register set.  Such a PHY would be out of spec,
>> especially as what you're suggesting is that it answers C22 cycles
>> and fails to implement MII_BMSR.  I also think that there's a comment
>> in the 802.3 specs that says that unimplemented registers are to
>> return zero, not 0xffff.
> 
> Checking 802.3-2015, in "22.2.4 Management functions", the first
> sentence requires all PHYs that respond to Clause 22 cycles to
> implement BMCR and BMSR.  However, my statement about unimplemented
> registers returning zero seems to be a C45 thing, not a C22 thing,
> according to the C22 PICS and "45.2 MDIO Interface Registers"
> 
> However, digging a bit further, "22.2.4.2.10 Auto-Negotiation complete"
> states that bit 5 shall be zero if aneg has not completed or if aneg is
> unimplemented.
> 

So how about we are more defensive than that and if we are presented
with a C45 PHY driver that does not have an aneg_done() function pointer
set we return an error/warning during driver registration?
-- 
Florian

Re: [PATCH 2/5] net: phy: hook up clause 45 autonegotiation restart

[Russell King - ARM Linux]

On Thu, Jun 01, 2017 at 10:16:17AM -0700, Florian Fainelli wrote:
> On 06/01/2017 09:24 AM, Russell King - ARM Linux wrote:
> > Checking 802.3-2015, in "22.2.4 Management functions", the first
> > sentence requires all PHYs that respond to Clause 22 cycles to
> > implement BMCR and BMSR.  However, my statement about unimplemented
> > registers returning zero seems to be a C45 thing, not a C22 thing,
> > according to the C22 PICS and "45.2 MDIO Interface Registers"
> > 
> > However, digging a bit further, "22.2.4.2.10 Auto-Negotiation complete"
> > states that bit 5 shall be zero if aneg has not completed or if aneg is
> > unimplemented.
> > 
> 
> So how about we are more defensive than that and if we are presented
> with a C45 PHY driver that does not have an aneg_done() function pointer
> set we return an error/warning during driver registration?

We don't mark drivers as being C22 or C45, we rely on them matching
the IDs and/or the probe function making that decision, so it's a
tad difficult to make that decision at driver registration time.

We could reject an attempt to probe a C45 phy with a driver that
does not provide an aneg_done() pointer.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/5] net: phy: hook up clause 45 autonegotiation restart

[Andrew Lunn]

On Fri, Jun 02, 2017 at 01:43:39PM +0100, Russell King - ARM Linux wrote:
> On Thu, Jun 01, 2017 at 10:16:17AM -0700, Florian Fainelli wrote:
> > On 06/01/2017 09:24 AM, Russell King - ARM Linux wrote:
> > > Checking 802.3-2015, in "22.2.4 Management functions", the first
> > > sentence requires all PHYs that respond to Clause 22 cycles to
> > > implement BMCR and BMSR.  However, my statement about unimplemented
> > > registers returning zero seems to be a C45 thing, not a C22 thing,
> > > according to the C22 PICS and "45.2 MDIO Interface Registers"
> > > 
> > > However, digging a bit further, "22.2.4.2.10 Auto-Negotiation complete"
> > > states that bit 5 shall be zero if aneg has not completed or if aneg is
> > > unimplemented.
> > > 
> > 
> > So how about we are more defensive than that and if we are presented
> > with a C45 PHY driver that does not have an aneg_done() function pointer
> > set we return an error/warning during driver registration?
> 
> We don't mark drivers as being C22 or C45, we rely on them matching
> the IDs and/or the probe function making that decision, so it's a
> tad difficult to make that decision at driver registration time.
> 
> We could reject an attempt to probe a C45 phy with a driver that
> does not provide an aneg_done() pointer.

Just an idea.

We could make phy_read/phy_write look to see if the is_c45 flag is
set. If so, return -EINVAL if the register being accessed does not
have MII_ADDR_C45.

That should catch all these sort of problems.

     Andrew

Re: [PATCH 2/5] net: phy: hook up clause 45 autonegotiation restart

[Russell King - ARM Linux]

On Fri, Jun 02, 2017 at 03:46:32PM +0200, Andrew Lunn wrote:
> On Fri, Jun 02, 2017 at 01:43:39PM +0100, Russell King - ARM Linux wrote:
> > We don't mark drivers as being C22 or C45, we rely on them matching
> > the IDs and/or the probe function making that decision, so it's a
> > tad difficult to make that decision at driver registration time.
> > 
> > We could reject an attempt to probe a C45 phy with a driver that
> > does not provide an aneg_done() pointer.
> 
> Just an idea.
> 
> We could make phy_read/phy_write look to see if the is_c45 flag is
> set. If so, return -EINVAL if the register being accessed does not
> have MII_ADDR_C45.
> 
> That should catch all these sort of problems.

I'm not so sure we want to do that.  Clause 45 PHYs are allowed to
respond to Clause 22 accesses, and MMD 29 contain a set of Clause 22
extension registers.

It seems it's possible for a PHY to support Clause 22 accesses along
with Clause 45 and MMD29 to extend the features beyond those supported
by the Clause 22 definition without providing the other MMDs.

Note that the Clause 45 definitions omit the 1000baseT registers
(MII_CTRL1000 / MII_STAT1000, or even registers that give the equivalent
information) from the 802.3 definition - this is why I've ended up
having to use the Marvell vendor specific 0xa016 register to determine
the PHY operating mode.  So, a conventional 10/100/1G PHY but with C45
support would still want to access C22 registers.

Hence, I don't think we can block C22 accesses in the way you're
suggesting.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 3/5] net: phy: split out 10G genphy support

[Russell King]

Move the old 10G genphy support to sit beside the new clause 45 library
functions, so all the 10G phy code is together.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phy-c45.c    |  64 ++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c | 105 ++++++++-----------------------------------
 2 files changed, 83 insertions(+), 86 deletions(-)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 252e49864215..ed77b1cb61de 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -229,3 +229,67 @@ int genphy_c45_read_pma(struct phy_device *phydev)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(genphy_c45_read_pma);
+
+/* The gen10g_* functions are the old Clause 45 stub */
+
+static int gen10g_config_aneg(struct phy_device *phydev)
+{
+	return 0;
+}
+
+static int gen10g_read_status(struct phy_device *phydev)
+{
+	u32 mmd_mask = phydev->c45_ids.devices_in_package;
+	int ret;
+
+	/* For now just lie and say it's 10G all the time */
+	phydev->speed = SPEED_10000;
+	phydev->duplex = DUPLEX_FULL;
+
+	/* Avoid reading the vendor MMDs */
+	mmd_mask &= ~(BIT(MDIO_MMD_VEND1) | BIT(MDIO_MMD_VEND2));
+
+	ret = genphy_c45_read_link(phydev, mmd_mask);
+
+	phydev->link = ret > 0 ? 1 : 0;
+
+	return 0;
+}
+
+static int gen10g_soft_reset(struct phy_device *phydev)
+{
+	/* Do nothing for now */
+	return 0;
+}
+
+static int gen10g_config_init(struct phy_device *phydev)
+{
+	/* Temporarily just say we support everything */
+	phydev->supported = SUPPORTED_10000baseT_Full;
+	phydev->advertising = SUPPORTED_10000baseT_Full;
+
+	return 0;
+}
+
+static int gen10g_suspend(struct phy_device *phydev)
+{
+	return 0;
+}
+
+static int gen10g_resume(struct phy_device *phydev)
+{
+	return 0;
+}
+
+struct phy_driver genphy_10g_driver = {
+	.phy_id         = 0xffffffff,
+	.phy_id_mask    = 0xffffffff,
+	.name           = "Generic 10G PHY",
+	.soft_reset	= gen10g_soft_reset,
+	.config_init    = gen10g_config_init,
+	.features       = 0,
+	.config_aneg    = gen10g_config_aneg,
+	.read_status    = gen10g_read_status,
+	.suspend        = gen10g_suspend,
+	.resume         = gen10g_resume,
+};
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 040575dba98c..cedd1fd91626 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -69,13 +69,8 @@ static void phy_mdio_device_remove(struct mdio_device *mdiodev)
 	phy_device_remove(phydev);
 }
 
-enum genphy_driver {
-	GENPHY_DRV_1G,
-	GENPHY_DRV_10G,
-	GENPHY_DRV_MAX
-};
-
-static struct phy_driver genphy_driver[GENPHY_DRV_MAX];
+static struct phy_driver genphy_driver;
+extern struct phy_driver genphy_10g_driver;
 
 static LIST_HEAD(phy_fixup_list);
 static DEFINE_MUTEX(phy_fixup_lock);
@@ -928,11 +923,9 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	 */
 	if (!d->driver) {
 		if (phydev->is_c45)
-			d->driver =
-				&genphy_driver[GENPHY_DRV_10G].mdiodrv.driver;
+			d->driver = &genphy_10g_driver.mdiodrv.driver;
 		else
-			d->driver =
-				&genphy_driver[GENPHY_DRV_1G].mdiodrv.driver;
+			d->driver = &genphy_driver.mdiodrv.driver;
 
 		using_genphy = true;
 	}
@@ -1048,7 +1041,6 @@ void phy_detach(struct phy_device *phydev)
 	struct net_device *dev = phydev->attached_dev;
 	struct module *ndev_owner = dev->dev.parent->driver->owner;
 	struct mii_bus *bus;
-	int i;
 
 	phydev->attached_dev->phydev = NULL;
 	phydev->attached_dev = NULL;
@@ -1063,13 +1055,9 @@ void phy_detach(struct phy_device *phydev)
 	 * from the generic driver so that there's a chance a
 	 * real driver could be loaded
 	 */
-	for (i = 0; i < ARRAY_SIZE(genphy_driver); i++) {
-		if (phydev->mdio.dev.driver ==
-		    &genphy_driver[i].mdiodrv.driver) {
-			device_release_driver(&phydev->mdio.dev);
-			break;
-		}
-	}
+	if (phydev->mdio.dev.driver == &genphy_10g_driver.mdiodrv.driver ||
+	    phydev->mdio.dev.driver == &genphy_driver.mdiodrv.driver)
+		device_release_driver(&phydev->mdio.dev);
 
 	/*
 	 * The phydev might go away on the put_device() below, so avoid
@@ -1343,11 +1331,6 @@ int genphy_aneg_done(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(genphy_aneg_done);
 
-static int gen10g_config_aneg(struct phy_device *phydev)
-{
-	return 0;
-}
-
 /**
  * genphy_update_link - update link status in @phydev
  * @phydev: target phy_device struct
@@ -1481,25 +1464,6 @@ int genphy_read_status(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(genphy_read_status);
 
-static int gen10g_read_status(struct phy_device *phydev)
-{
-	u32 mmd_mask = phydev->c45_ids.devices_in_package;
-	int ret;
-
-	/* For now just lie and say it's 10G all the time */
-	phydev->speed = SPEED_10000;
-	phydev->duplex = DUPLEX_FULL;
-
-	/* Avoid reading the vendor MMDs */
-	mmd_mask &= ~(BIT(MDIO_MMD_VEND1) | BIT(MDIO_MMD_VEND2));
-
-	ret = genphy_c45_read_link(phydev, mmd_mask);
-
-	phydev->link = ret > 0 ? 1 : 0;
-
-	return 0;
-}
-
 /**
  * genphy_soft_reset - software reset the PHY via BMCR_RESET bit
  * @phydev: target phy_device struct
@@ -1563,23 +1527,8 @@ int genphy_config_init(struct phy_device *phydev)
 
 	return 0;
 }
-
-static int gen10g_soft_reset(struct phy_device *phydev)
-{
-	/* Do nothing for now */
-	return 0;
-}
 EXPORT_SYMBOL(genphy_config_init);
 
-static int gen10g_config_init(struct phy_device *phydev)
-{
-	/* Temporarily just say we support everything */
-	phydev->supported = SUPPORTED_10000baseT_Full;
-	phydev->advertising = SUPPORTED_10000baseT_Full;
-
-	return 0;
-}
-
 int genphy_suspend(struct phy_device *phydev)
 {
 	int value;
@@ -1595,11 +1544,6 @@ int genphy_suspend(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(genphy_suspend);
 
-static int gen10g_suspend(struct phy_device *phydev)
-{
-	return 0;
-}
-
 int genphy_resume(struct phy_device *phydev)
 {
 	int value;
@@ -1615,11 +1559,6 @@ int genphy_resume(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(genphy_resume);
 
-static int gen10g_resume(struct phy_device *phydev)
-{
-	return 0;
-}
-
 static int __set_phy_supported(struct phy_device *phydev, u32 max_speed)
 {
 	/* The default values for phydev->supported are provided by the PHY
@@ -1851,8 +1790,7 @@ void phy_drivers_unregister(struct phy_driver *drv, int n)
 }
 EXPORT_SYMBOL(phy_drivers_unregister);
 
-static struct phy_driver genphy_driver[] = {
-{
+static struct phy_driver genphy_driver = {
 	.phy_id		= 0xffffffff,
 	.phy_id_mask	= 0xffffffff,
 	.name		= "Generic PHY",
@@ -1866,18 +1804,7 @@ static struct phy_driver genphy_driver[] = {
 	.read_status	= genphy_read_status,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
-}, {
-	.phy_id         = 0xffffffff,
-	.phy_id_mask    = 0xffffffff,
-	.name           = "Generic 10G PHY",
-	.soft_reset	= gen10g_soft_reset,
-	.config_init    = gen10g_config_init,
-	.features       = 0,
-	.config_aneg    = gen10g_config_aneg,
-	.read_status    = gen10g_read_status,
-	.suspend        = gen10g_suspend,
-	.resume         = gen10g_resume,
-} };
+};
 
 static int __init phy_init(void)
 {
@@ -1887,18 +1814,24 @@ static int __init phy_init(void)
 	if (rc)
 		return rc;
 
-	rc = phy_drivers_register(genphy_driver,
-				  ARRAY_SIZE(genphy_driver), THIS_MODULE);
+	rc = phy_driver_register(&genphy_10g_driver, THIS_MODULE);
 	if (rc)
+		goto err_10g;
+
+	rc = phy_driver_register(&genphy_driver, THIS_MODULE);
+	if (rc) {
+		phy_driver_unregister(&genphy_10g_driver);
+err_10g:
 		mdio_bus_exit();
+	}
 
 	return rc;
 }
 
 static void __exit phy_exit(void)
 {
-	phy_drivers_unregister(genphy_driver,
-			       ARRAY_SIZE(genphy_driver));
+	phy_driver_unregister(&genphy_10g_driver);
+	phy_driver_unregister(&genphy_driver);
 	mdio_bus_exit();
 }
 
-- 
2.7.4

Re: [PATCH 3/5] net: phy: split out 10G genphy support

[Andrew Lunn]

On Thu, Jun 01, 2017 at 11:26:41AM +0100, Russell King wrote:
> Move the old 10G genphy support to sit beside the new clause 45 library
> functions, so all the 10G phy code is together.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH 3/5] net: phy: split out 10G genphy support

[Florian Fainelli]

On 06/01/2017 03:26 AM, Russell King wrote:
> Move the old 10G genphy support to sit beside the new clause 45 library
> functions, so all the 10G phy code is together.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

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

[PATCH 4/5] net: phy: add XAUI and 10GBASE-KR PHY connection types

[Russell King]

XAUI allows XGMII to reach an extended distance by using a XGXS layer at
each end of the MAC to PHY link, operating over four Serdes lanes.

10GBASE-KR is a single lane Serdes backplane ethernet connection method
with autonegotiation on the link.  Some PHYs use this to connect to the
ethernet interface at 10G speeds, switching to other connection types
when utilising slower speeds.

10GBASE-KR is also used for XFI and SFI to connect to XFP and SFP fiber
modules.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 Documentation/devicetree/bindings/net/ethernet.txt | 2 ++
 include/linux/phy.h                                | 7 +++++++
 2 files changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt
index 3a6916909d90..d4abe9a98109 100644
--- a/Documentation/devicetree/bindings/net/ethernet.txt
+++ b/Documentation/devicetree/bindings/net/ethernet.txt
@@ -32,6 +32,8 @@
   * "2000base-x",
   * "2500base-x",
   * "rxaui"
+  * "xaui"
+  * "10gbase-kr" (10GBASE-KR, XFI, SFI)
 - phy-connection-type: the same as "phy-mode" property but described in ePAPR;
 - phy-handle: phandle, specifies a reference to a node representing a PHY
   device; this property is described in ePAPR and so preferred;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 45dfe1b2b003..45728d0a0d0e 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -84,6 +84,9 @@ typedef enum {
 	PHY_INTERFACE_MODE_1000BASEX,
 	PHY_INTERFACE_MODE_2500BASEX,
 	PHY_INTERFACE_MODE_RXAUI,
+	PHY_INTERFACE_MODE_XAUI,
+	/* 10GBASE-KR, XFI, SFI - single lane 10G Serdes */
+	PHY_INTERFACE_MODE_10GKR,
 	PHY_INTERFACE_MODE_MAX,
 } phy_interface_t;
 
@@ -150,6 +153,10 @@ static inline const char *phy_modes(phy_interface_t interface)
 		return "2500base-x";
 	case PHY_INTERFACE_MODE_RXAUI:
 		return "rxaui";
+	case PHY_INTERFACE_MODE_XAUI:
+		return "xaui";
+	case PHY_INTERFACE_MODE_10GKR:
+		return "10gbase-kr";
 	default:
 		return "unknown";
 	}
-- 
2.7.4

Re: [PATCH 4/5] net: phy: add XAUI and 10GBASE-KR PHY connection types

[Andrew Lunn]

On Thu, Jun 01, 2017 at 11:26:47AM +0100, Russell King wrote:
> XAUI allows XGMII to reach an extended distance by using a XGXS layer at
> each end of the MAC to PHY link, operating over four Serdes lanes.
> 
> 10GBASE-KR is a single lane Serdes backplane ethernet connection method
> with autonegotiation on the link.  Some PHYs use this to connect to the
> ethernet interface at 10G speeds, switching to other connection types
> when utilising slower speeds.
> 
> 10GBASE-KR is also used for XFI and SFI to connect to XFP and SFP fiber
> modules.
> 
> Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>

Reviewed-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>

    Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] net: phy: add XAUI and 10GBASE-KR PHY connection types

[Florian Fainelli]

On 06/01/2017 03:26 AM, Russell King wrote:
> XAUI allows XGMII to reach an extended distance by using a XGXS layer at
> each end of the MAC to PHY link, operating over four Serdes lanes.
> 
> 10GBASE-KR is a single lane Serdes backplane ethernet connection method
> with autonegotiation on the link.  Some PHYs use this to connect to the
> ethernet interface at 10G speeds, switching to other connection types
> when utilising slower speeds.
> 
> 10GBASE-KR is also used for XFI and SFI to connect to XFP and SFP fiber
> modules.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

This looks good, but you also need to update
Documentation/ABI/testing/sysfs-class-net-phydev since we report
phy_interface from sysfs.

With that fixed:

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

Re: [PATCH 4/5] net: phy: add XAUI and 10GBASE-KR PHY connection types

[Russell King - ARM Linux]

On Thu, Jun 01, 2017 at 09:56:48AM -0700, Florian Fainelli wrote:
> On 06/01/2017 03:26 AM, Russell King wrote:
> > XAUI allows XGMII to reach an extended distance by using a XGXS layer at
> > each end of the MAC to PHY link, operating over four Serdes lanes.
> > 
> > 10GBASE-KR is a single lane Serdes backplane ethernet connection method
> > with autonegotiation on the link.  Some PHYs use this to connect to the
> > ethernet interface at 10G speeds, switching to other connection types
> > when utilising slower speeds.
> > 
> > 10GBASE-KR is also used for XFI and SFI to connect to XFP and SFP fiber
> > modules.
> > 
> > Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
> 
> This looks good, but you also need to update
> Documentation/ABI/testing/sysfs-class-net-phydev since we report
> phy_interface from sysfs.

Hmm, seems to be a new file recently merged into net-next.  Ok, I'll
provide an update for that.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] net: phy: add Marvell Alaska X 88X3310 10Gigabit PHY support

[Russell King]

Add phylib support for the Marvell Alaska X 10 Gigabit PHY (MV88X3310).
This phy is able to operate at 10G, 1G, 100M and 10M speeds, and only
supports Clause 45 accesses.

The PHY appears (based on the vendor IDs) to be two different vendors
IP, with each devad containing several instances.

This PHY driver has only been tested with the RJ45 copper port, fiber
port and a Marvell Armada 8040-based ethernet interface.

It should be noted that to use the full range of speeds, MAC drivers
need to also reconfigure the link mode as per phydev->interface, since
the PHY automatically changes its interface mode depending on the
negotiated speed.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 MAINTAINERS                  |   6 +
 drivers/net/phy/Makefile     |   2 +-
 drivers/net/phy/marvell10g.c | 364 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 371 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/phy/marvell10g.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f7d568b8f133..9b6c3347d6d9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7971,6 +7971,12 @@ S:	Maintained
 F:	drivers/net/ethernet/marvell/mv643xx_eth.*
 F:	include/linux/mv643xx.h
 
+MARVELL MV88X3310 PHY DRIVER
+M:	Russell King <rmk@armlinux.org.uk>
+L:	netdev@vger.kernel.org
+S:	Maintained
+F:	drivers/net/phy/marvell10g.c
+
 MARVELL MVNETA ETHERNET DRIVER
 M:	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
 L:	netdev@vger.kernel.org
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 19eddf758c88..905990fece28 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -55,7 +55,7 @@ obj-$(CONFIG_ICPLUS_PHY)	+= icplus.o
 obj-$(CONFIG_INTEL_XWAY_PHY)	+= intel-xway.o
 obj-$(CONFIG_LSI_ET1011C_PHY)	+= et1011c.o
 obj-$(CONFIG_LXT_PHY)		+= lxt.o
-obj-$(CONFIG_MARVELL_PHY)	+= marvell.o
+obj-$(CONFIG_MARVELL_PHY)	+= marvell.o marvell10g.o
 obj-$(CONFIG_MESON_GXL_PHY)	+= meson-gxl.o
 obj-$(CONFIG_MICREL_KS8995MA)	+= spi_ks8995.o
 obj-$(CONFIG_MICREL_PHY)	+= micrel.o
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
new file mode 100644
index 000000000000..f43e03dada6a
--- /dev/null
+++ b/drivers/net/phy/marvell10g.c
@@ -0,0 +1,364 @@
+/*
+ * Marvell 10G 88x3310 PHY driver
+ *
+ * Based upon the ID registers, this PHY appears to be a mixture of IPs
+ * from two different companies.
+ *
+ * There appears to be several different data paths through the PHY which
+ * are automatically managed by the PHY.  The following has been determined
+ * via observation and experimentation:
+ *
+ *       SGMII PHYXS -- BASE-T PCS -- 10G PMA -- AN -- Copper (for <= 1G)
+ *  10GBASE-KR PHYXS -- BASE-T PCS -- 10G PMA -- AN -- Copper (for 10G)
+ *  10GBASE-KR PHYXS -- BASE-R PCS -- Fiber
+ *
+ * If both the fiber and copper ports are connected, the first to gain
+ * link takes priority and the other port is completely locked out.
+ */
+#include <linux/phy.h>
+
+enum {
+	MV_PCS_BASE_T		= 0x0000,
+	MV_PCS_BASE_R		= 0x1000,
+	MV_PCS_1000BASEX	= 0x2000,
+
+	/* These registers appear at 0x800X and 0xa00X - the 0xa00X control
+	 * registers appear to set themselves to the 0x800X when AN is
+	 * restarted, but status registers appear readable from either.
+	 */
+	MV_AN_CTRL1000		= 0x8000, /* 1000base-T control register */
+	MV_AN_STAT1000		= 0x8001, /* 1000base-T status register */
+
+	/* This register appears to reflect the copper status */
+	MV_AN_RESULT		= 0xa016,
+	MV_AN_RESULT_SPD_10	= BIT(12),
+	MV_AN_RESULT_SPD_100	= BIT(13),
+	MV_AN_RESULT_SPD_1000	= BIT(14),
+	MV_AN_RESULT_SPD_10000	= BIT(15),
+};
+
+static int mv3310_modify(struct phy_device *phydev, int devad, u16 reg,
+			 u16 mask, u16 bits)
+{
+	int old, val, ret;
+
+	old = phy_read_mmd(phydev, devad, reg);
+	if (old < 0)
+		return old;
+
+	val = (old & ~mask) | (bits & mask);
+	if (val == old)
+		return 0;
+
+	ret = phy_write_mmd(phydev, devad, reg, val);
+
+	return ret < 0 ? ret : 1;
+}
+
+static int mv3310_probe(struct phy_device *phydev)
+{
+	u32 mmd_mask = MDIO_DEVS_PMAPMD | MDIO_DEVS_AN;
+
+	if (!phydev->is_c45 ||
+	    (phydev->c45_ids.devices_in_package & mmd_mask) != mmd_mask)
+		return -ENODEV;
+
+	return 0;
+}
+
+static int mv3310_soft_reset(struct phy_device *phydev)
+{
+	return 0;
+}
+
+static int mv3310_config_init(struct phy_device *phydev)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
+	u32 mask;
+	int val;
+
+	/* Check that the PHY interface type is compatible */
+	if (phydev->interface != PHY_INTERFACE_MODE_SGMII &&
+	    phydev->interface != PHY_INTERFACE_MODE_XGMII &&
+	    phydev->interface != PHY_INTERFACE_MODE_XAUI &&
+	    phydev->interface != PHY_INTERFACE_MODE_RXAUI &&
+	    phydev->interface != PHY_INTERFACE_MODE_10GKR)
+		return -ENODEV;
+
+	__set_bit(ETHTOOL_LINK_MODE_Pause_BIT, supported);
+	__set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, supported);
+
+	if (phydev->c45_ids.devices_in_package & MDIO_DEVS_AN) {
+		val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
+		if (val < 0)
+			return val;
+
+		if (val & MDIO_AN_STAT1_ABLE)
+			__set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, supported);
+	}
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_STAT2);
+	if (val < 0)
+		return val;
+
+	/* Ethtool does not support the WAN mode bits */
+	if (val & (MDIO_PMA_STAT2_10GBSR | MDIO_PMA_STAT2_10GBLR |
+		   MDIO_PMA_STAT2_10GBER | MDIO_PMA_STAT2_10GBLX4 |
+		   MDIO_PMA_STAT2_10GBSW | MDIO_PMA_STAT2_10GBLW |
+		   MDIO_PMA_STAT2_10GBEW))
+		__set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, supported);
+	if (val & MDIO_PMA_STAT2_10GBSR)
+		__set_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT, supported);
+	if (val & MDIO_PMA_STAT2_10GBLR)
+		__set_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT, supported);
+	if (val & MDIO_PMA_STAT2_10GBER)
+		__set_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT, supported);
+
+	if (val & MDIO_PMA_STAT2_EXTABLE) {
+		val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_EXTABLE);
+		if (val < 0)
+			return val;
+
+		if (val & (MDIO_PMA_EXTABLE_10GBT | MDIO_PMA_EXTABLE_1000BT |
+			   MDIO_PMA_EXTABLE_100BTX | MDIO_PMA_EXTABLE_10BT))
+			__set_bit(ETHTOOL_LINK_MODE_TP_BIT, supported);
+		if (val & MDIO_PMA_EXTABLE_10GBLRM)
+			__set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, supported);
+		if (val & (MDIO_PMA_EXTABLE_10GBKX4 | MDIO_PMA_EXTABLE_10GBKR |
+			   MDIO_PMA_EXTABLE_1000BKX))
+			__set_bit(ETHTOOL_LINK_MODE_Backplane_BIT, supported);
+		if (val & MDIO_PMA_EXTABLE_10GBLRM)
+			__set_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT,
+				  supported);
+		if (val & MDIO_PMA_EXTABLE_10GBT)
+			__set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+				  supported);
+		if (val & MDIO_PMA_EXTABLE_10GBKX4)
+			__set_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+				  supported);
+		if (val & MDIO_PMA_EXTABLE_10GBKR)
+			__set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+				  supported);
+		if (val & MDIO_PMA_EXTABLE_1000BT)
+			__set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+				  supported);
+		if (val & MDIO_PMA_EXTABLE_1000BKX)
+			__set_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+				  supported);
+		if (val & MDIO_PMA_EXTABLE_100BTX)
+			__set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+				  supported);
+		if (val & MDIO_PMA_EXTABLE_10BT)
+			__set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+				  supported);
+	}
+
+	if (!ethtool_convert_link_mode_to_legacy_u32(&mask, supported))
+		dev_warn(&phydev->mdio.dev,
+			 "PHY supports (%*pb) more modes than phylib supports, some modes not supported.\n",
+			 __ETHTOOL_LINK_MODE_MASK_NBITS, supported);
+
+	phydev->supported &= mask;
+	phydev->advertising &= phydev->supported;
+
+	return 0;
+}
+
+static int mv3310_config_aneg(struct phy_device *phydev)
+{
+	bool changed = false;
+	u32 advertising;
+	int ret;
+
+	if (phydev->autoneg == AUTONEG_DISABLE) {
+		ret = genphy_c45_pma_setup_forced(phydev);
+		if (ret < 0)
+			return ret;
+
+		return genphy_c45_an_disable_aneg(phydev);
+	}
+
+	phydev->advertising &= phydev->supported;
+	advertising = phydev->advertising;
+
+	ret = mv3310_modify(phydev, MDIO_MMD_AN, MDIO_AN_ADVERTISE,
+			    ADVERTISE_ALL | ADVERTISE_100BASE4 |
+			    ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM,
+			    ethtool_adv_to_mii_adv_t(advertising));
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		changed = true;
+
+	ret = mv3310_modify(phydev, MDIO_MMD_AN, MV_AN_CTRL1000,
+			    ADVERTISE_1000FULL | ADVERTISE_1000HALF,
+			    ethtool_adv_to_mii_ctrl1000_t(advertising));
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		changed = true;
+
+	/* 10G control register */
+	ret = mv3310_modify(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_CTRL,
+			    MDIO_AN_10GBT_CTRL_ADV10G,
+			    advertising & ADVERTISED_10000baseT_Full ?
+				MDIO_AN_10GBT_CTRL_ADV10G : 0);
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		changed = true;
+
+	if (changed)
+		ret = genphy_c45_restart_aneg(phydev);
+
+	return ret;
+}
+
+static int mv3310_aneg_done(struct phy_device *phydev)
+{
+	int val;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_BASE_R + MDIO_STAT1);
+	if (val < 0)
+		return val;
+
+	if (val & MDIO_STAT1_LSTATUS)
+		return 1;
+
+	return genphy_c45_aneg_done(phydev);
+}
+
+/* 10GBASE-ER,LR,LRM,SR do not support autonegotiation. */
+static int mv3310_read_10gbr_status(struct phy_device *phydev)
+{
+	phydev->link = 1;
+	phydev->speed = SPEED_10000;
+	phydev->duplex = DUPLEX_FULL;
+
+	if (phydev->interface == PHY_INTERFACE_MODE_SGMII)
+		phydev->interface = PHY_INTERFACE_MODE_10GKR;
+
+	return 0;
+}
+
+static int mv3310_read_status(struct phy_device *phydev)
+{
+	u32 mmd_mask = phydev->c45_ids.devices_in_package;
+	int val;
+
+	/* The vendor devads do not report link status.  Avoid the PHYXS
+	 * instance as there are three, and its status depends on the MAC
+	 * being appropriately configured for the negotiated speed.
+	 */
+	mmd_mask &= ~(BIT(MDIO_MMD_VEND1) | BIT(MDIO_MMD_VEND2) |
+		      BIT(MDIO_MMD_PHYXS));
+
+	phydev->speed = SPEED_UNKNOWN;
+	phydev->duplex = DUPLEX_UNKNOWN;
+	phydev->lp_advertising = 0;
+	phydev->link = 0;
+	phydev->pause = 0;
+	phydev->asym_pause = 0;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_BASE_R + MDIO_STAT1);
+	if (val < 0)
+		return val;
+
+	if (val & MDIO_STAT1_LSTATUS)
+		return mv3310_read_10gbr_status(phydev);
+
+	val = genphy_c45_read_link(phydev, mmd_mask);
+	if (val < 0)
+		return val;
+
+	phydev->link = val > 0 ? 1 : 0;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
+	if (val < 0)
+		return val;
+
+	if (val & MDIO_AN_STAT1_COMPLETE) {
+		val = genphy_c45_read_lpa(phydev);
+		if (val < 0)
+			return val;
+
+		/* Read the link partner's 1G advertisment */
+		val = phy_read_mmd(phydev, MDIO_MMD_AN, MV_AN_STAT1000);
+		if (val < 0)
+			return val;
+
+		phydev->lp_advertising |= mii_stat1000_to_ethtool_lpa_t(val);
+
+		if (phydev->autoneg == AUTONEG_ENABLE) {
+			val = phy_read_mmd(phydev, MDIO_MMD_AN, MV_AN_RESULT);
+			if (val < 0)
+				return val;
+
+			if (val & MV_AN_RESULT_SPD_10000)
+				phydev->speed = SPEED_10000;
+			else if (val & MV_AN_RESULT_SPD_1000)
+				phydev->speed = SPEED_1000;
+			else if (val & MV_AN_RESULT_SPD_100)
+				phydev->speed = SPEED_100;
+			else if (val & MV_AN_RESULT_SPD_10)
+				phydev->speed = SPEED_10;
+
+			phydev->duplex = DUPLEX_FULL;
+		}
+	}
+
+	if (phydev->autoneg != AUTONEG_ENABLE) {
+		val = genphy_c45_read_pma(phydev);
+		if (val < 0)
+			return val;
+	}
+
+	if ((phydev->interface == PHY_INTERFACE_MODE_SGMII ||
+	     phydev->interface == PHY_INTERFACE_MODE_10GKR) && phydev->link) {
+		/* The PHY automatically switches its serdes interface (and
+		 * active PHYXS instance) between Cisco SGMII and 10GBase-KR
+		 * modes according to the speed.  Florian suggests setting
+		 * phydev->interface to communicate this to the MAC. Only do
+		 * this if we are already in either SGMII or 10GBase-KR mode.
+		 */
+		if (phydev->speed == SPEED_10000)
+			phydev->interface = PHY_INTERFACE_MODE_10GKR;
+		else if (phydev->speed >= SPEED_10 &&
+			 phydev->speed < SPEED_10000)
+			phydev->interface = PHY_INTERFACE_MODE_SGMII;
+	}
+
+	return 0;
+}
+
+static struct phy_driver mv3310_drivers[] = {
+	{
+		.phy_id		= 0x002b09aa,
+		.phy_id_mask	= 0xffffffff,
+		.name		= "mv88x3310",
+		.features	= SUPPORTED_10baseT_Full |
+				  SUPPORTED_100baseT_Full |
+				  SUPPORTED_1000baseT_Full |
+				  SUPPORTED_Autoneg |
+				  SUPPORTED_TP |
+				  SUPPORTED_FIBRE |
+				  SUPPORTED_10000baseT_Full |
+				  SUPPORTED_Backplane,
+		.probe		= mv3310_probe,
+		.soft_reset	= mv3310_soft_reset,
+		.config_init	= mv3310_config_init,
+		.config_aneg	= mv3310_config_aneg,
+		.aneg_done	= mv3310_aneg_done,
+		.read_status	= mv3310_read_status,
+	},
+};
+
+module_phy_driver(mv3310_drivers);
+
+static struct mdio_device_id __maybe_unused mv3310_tbl[] = {
+	{ 0x002b09aa, 0xffffffff },
+	{ },
+};
+MODULE_DEVICE_TABLE(mdio, mv3310_tbl);
+MODULE_DESCRIPTION("Marvell Alaska X 10Gigabit Ethernet PHY driver (MV88X3310)");
+MODULE_LICENSE("GPL");
-- 
2.7.4

Re: [PATCH 5/5] net: phy: add Marvell Alaska X 88X3310 10Gigabit PHY support

[Andrew Lunn]

> +static int mv3310_read_status(struct phy_device *phydev)
> +{
> +	u32 mmd_mask = phydev->c45_ids.devices_in_package;
> +	int val;
> +
> +	/* The vendor devads do not report link status.  Avoid the PHYXS
> +	 * instance as there are three, and its status depends on the MAC
> +	 * being appropriately configured for the negotiated speed.
> +	 */
> +	mmd_mask &= ~(BIT(MDIO_MMD_VEND1) | BIT(MDIO_MMD_VEND2) |
> +		      BIT(MDIO_MMD_PHYXS));
> +
> +	phydev->speed = SPEED_UNKNOWN;
> +	phydev->duplex = DUPLEX_UNKNOWN;
> +	phydev->lp_advertising = 0;
> +	phydev->link = 0;
> +	phydev->pause = 0;
> +	phydev->asym_pause = 0;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_BASE_R + MDIO_STAT1);
> +	if (val < 0)
> +		return val;
> +
> +	if (val & MDIO_STAT1_LSTATUS)
> +		return mv3310_read_10gbr_status(phydev);
> +
> +	val = genphy_c45_read_link(phydev, mmd_mask);
> +	if (val < 0)
> +		return val;
> +
> +	phydev->link = val > 0 ? 1 : 0;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
> +	if (val < 0)
> +		return val;
> +
> +	if (val & MDIO_AN_STAT1_COMPLETE) {
> +		val = genphy_c45_read_lpa(phydev);
> +		if (val < 0)
> +			return val;
> +
> +		/* Read the link partner's 1G advertisment */
> +		val = phy_read_mmd(phydev, MDIO_MMD_AN, MV_AN_STAT1000);
> +		if (val < 0)
> +			return val;
> +
> +		phydev->lp_advertising |= mii_stat1000_to_ethtool_lpa_t(val);
> +
> +		if (phydev->autoneg == AUTONEG_ENABLE) {
> +			val = phy_read_mmd(phydev, MDIO_MMD_AN, MV_AN_RESULT);
> +			if (val < 0)
> +				return val;
> +
> +			if (val & MV_AN_RESULT_SPD_10000)
> +				phydev->speed = SPEED_10000;
> +			else if (val & MV_AN_RESULT_SPD_1000)
> +				phydev->speed = SPEED_1000;
> +			else if (val & MV_AN_RESULT_SPD_100)
> +				phydev->speed = SPEED_100;
> +			else if (val & MV_AN_RESULT_SPD_10)
> +				phydev->speed = SPEED_10;
> +
> +			phydev->duplex = DUPLEX_FULL;
> +		}
> +	}
> +
> +	if (phydev->autoneg != AUTONEG_ENABLE) {
> +		val = genphy_c45_read_pma(phydev);
> +		if (val < 0)
> +			return val;
> +	}
> +
> +	if ((phydev->interface == PHY_INTERFACE_MODE_SGMII ||
> +	     phydev->interface == PHY_INTERFACE_MODE_10GKR) && phydev->link) {
> +		/* The PHY automatically switches its serdes interface (and
> +		 * active PHYXS instance) between Cisco SGMII and 10GBase-KR
> +		 * modes according to the speed.  Florian suggests setting
> +		 * phydev->interface to communicate this to the MAC. Only do
> +		 * this if we are already in either SGMII or 10GBase-KR mode.
> +		 */

Hi Russell

Just for my understanding. The MAC should check phydev->interface in
its adjust_link function? Can we document this here please. I wounder
if there is somewhere in the generic code we should also document
this?

> +static struct phy_driver mv3310_drivers[] = {
> +	{
> +		.phy_id		= 0x002b09aa,
> +		.phy_id_mask	= 0xffffffff,

How about adding this ID to include/linux/marvell_phy.h? Or do you
think this is not actually a Marvell ID, but some 3rd party which
Marvell has licensed it from?

	Andrew

Re: [PATCH 5/5] net: phy: add Marvell Alaska X 88X3310 10Gigabit PHY support

[Russell King - ARM Linux]

On Thu, Jun 01, 2017 at 02:51:50PM +0200, Andrew Lunn wrote:
> > +static int mv3310_read_status(struct phy_device *phydev)
> > +{
> > +	u32 mmd_mask = phydev->c45_ids.devices_in_package;
> > +	int val;
> > +
> > +	/* The vendor devads do not report link status.  Avoid the PHYXS
> > +	 * instance as there are three, and its status depends on the MAC
> > +	 * being appropriately configured for the negotiated speed.
> > +	 */
> > +	mmd_mask &= ~(BIT(MDIO_MMD_VEND1) | BIT(MDIO_MMD_VEND2) |
> > +		      BIT(MDIO_MMD_PHYXS));
> > +
> > +	phydev->speed = SPEED_UNKNOWN;
> > +	phydev->duplex = DUPLEX_UNKNOWN;
> > +	phydev->lp_advertising = 0;
> > +	phydev->link = 0;
> > +	phydev->pause = 0;
> > +	phydev->asym_pause = 0;
> > +
> > +	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_BASE_R + MDIO_STAT1);
> > +	if (val < 0)
> > +		return val;
> > +
> > +	if (val & MDIO_STAT1_LSTATUS)
> > +		return mv3310_read_10gbr_status(phydev);
> > +
> > +	val = genphy_c45_read_link(phydev, mmd_mask);
> > +	if (val < 0)
> > +		return val;
> > +
> > +	phydev->link = val > 0 ? 1 : 0;
> > +
> > +	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
> > +	if (val < 0)
> > +		return val;
> > +
> > +	if (val & MDIO_AN_STAT1_COMPLETE) {
> > +		val = genphy_c45_read_lpa(phydev);
> > +		if (val < 0)
> > +			return val;
> > +
> > +		/* Read the link partner's 1G advertisment */
> > +		val = phy_read_mmd(phydev, MDIO_MMD_AN, MV_AN_STAT1000);
> > +		if (val < 0)
> > +			return val;
> > +
> > +		phydev->lp_advertising |= mii_stat1000_to_ethtool_lpa_t(val);
> > +
> > +		if (phydev->autoneg == AUTONEG_ENABLE) {
> > +			val = phy_read_mmd(phydev, MDIO_MMD_AN, MV_AN_RESULT);
> > +			if (val < 0)
> > +				return val;
> > +
> > +			if (val & MV_AN_RESULT_SPD_10000)
> > +				phydev->speed = SPEED_10000;
> > +			else if (val & MV_AN_RESULT_SPD_1000)
> > +				phydev->speed = SPEED_1000;
> > +			else if (val & MV_AN_RESULT_SPD_100)
> > +				phydev->speed = SPEED_100;
> > +			else if (val & MV_AN_RESULT_SPD_10)
> > +				phydev->speed = SPEED_10;
> > +
> > +			phydev->duplex = DUPLEX_FULL;
> > +		}
> > +	}
> > +
> > +	if (phydev->autoneg != AUTONEG_ENABLE) {
> > +		val = genphy_c45_read_pma(phydev);
> > +		if (val < 0)
> > +			return val;
> > +	}
> > +
> > +	if ((phydev->interface == PHY_INTERFACE_MODE_SGMII ||
> > +	     phydev->interface == PHY_INTERFACE_MODE_10GKR) && phydev->link) {
> > +		/* The PHY automatically switches its serdes interface (and
> > +		 * active PHYXS instance) between Cisco SGMII and 10GBase-KR
> > +		 * modes according to the speed.  Florian suggests setting
> > +		 * phydev->interface to communicate this to the MAC. Only do
> > +		 * this if we are already in either SGMII or 10GBase-KR mode.
> > +		 */
> 
> Hi Russell
> 
> Just for my understanding. The MAC should check phydev->interface in
> its adjust_link function? Can we document this here please. I wounder
> if there is somewhere in the generic code we should also document
> this?

Yes, it would need to, since it would be unreliable to determine from
the link mode bits.

> > +static struct phy_driver mv3310_drivers[] = {
> > +	{
> > +		.phy_id		= 0x002b09aa,
> > +		.phy_id_mask	= 0xffffffff,
> 
> How about adding this ID to include/linux/marvell_phy.h? Or do you
> think this is not actually a Marvell ID, but some 3rd party which
> Marvell has licensed it from?

The vendor part is not the conventional Marvell ID.  The 3310 appears
to be two or three different vendors IP all glued together, some of
which is in a way which violates the clause 45 register definitions.
Eg, there's multiple instances of PHYXS at 4K offsets, but C45
indicates that these are "reserved" and the place for vendor specific
stuff is in the two vendor MMDs.  Same goes for some of the other
MMDs as well.  I'll call these "MMD sub-blocks".

Some MMD sub-blocks have a Marvell ID in them, but others do not.

The OUI for the ID listed above is apparently "FiberHome Digital
Technology Co.,Ltd." but you'll also find the ID "0141 0daa" in some
of the MMD sub-blocks which corresponds with Marvell.

We can't use the sub-blocks though, because as I say, that's rather
non-standard and against the Clause 45 spec.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 5/5] net: phy: add Marvell Alaska X 88X3310 10Gigabit PHY support

[Florian Fainelli]

On 06/01/2017 03:26 AM, Russell King wrote:
> Add phylib support for the Marvell Alaska X 10 Gigabit PHY (MV88X3310).
> This phy is able to operate at 10G, 1G, 100M and 10M speeds, and only
> supports Clause 45 accesses.
> 
> The PHY appears (based on the vendor IDs) to be two different vendors
> IP, with each devad containing several instances.
> 
> This PHY driver has only been tested with the RJ45 copper port, fiber
> port and a Marvell Armada 8040-based ethernet interface.
> 
> It should be noted that to use the full range of speeds, MAC drivers
> need to also reconfigure the link mode as per phydev->interface, since
> the PHY automatically changes its interface mode depending on the
> negotiated speed.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---

>  MARVELL MVNETA ETHERNET DRIVER
>  M:	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>  L:	netdev@vger.kernel.org
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 19eddf758c88..905990fece28 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -55,7 +55,7 @@ obj-$(CONFIG_ICPLUS_PHY)	+= icplus.o
>  obj-$(CONFIG_INTEL_XWAY_PHY)	+= intel-xway.o
>  obj-$(CONFIG_LSI_ET1011C_PHY)	+= et1011c.o
>  obj-$(CONFIG_LXT_PHY)		+= lxt.o
> -obj-$(CONFIG_MARVELL_PHY)	+= marvell.o
> +obj-$(CONFIG_MARVELL_PHY)	+= marvell.o marvell10g.o

Could we introduce a CONFIG_MARVELL_10G_PHY symbol?


> +enum {
> +	MV_PCS_BASE_T		= 0x0000,
> +	MV_PCS_BASE_R		= 0x1000,
> +	MV_PCS_1000BASEX	= 0x2000,
> +
> +	/* These registers appear at 0x800X and 0xa00X - the 0xa00X control
> +	 * registers appear to set themselves to the 0x800X when AN is
> +	 * restarted, but status registers appear readable from either.
> +	 */
> +	MV_AN_CTRL1000		= 0x8000, /* 1000base-T control register */
> +	MV_AN_STAT1000		= 0x8001, /* 1000base-T status register */
> +
> +	/* This register appears to reflect the copper status */
> +	MV_AN_RESULT		= 0xa016,
> +	MV_AN_RESULT_SPD_10	= BIT(12),
> +	MV_AN_RESULT_SPD_100	= BIT(13),
> +	MV_AN_RESULT_SPD_1000	= BIT(14),
> +	MV_AN_RESULT_SPD_10000	= BIT(15),
> +};

Personal preference is not to use enums but jut plain #define, but
whatever works, really.


> +
> +static int mv3310_soft_reset(struct phy_device *phydev)
> +{
> +	return 0;
> +}

You could use genphy_no_soft_reset(), although genphy sort of implies
C22 now, or we could export genphy10g_soft_reset() which also does nothing.


> +	/* Ethtool does not support the WAN mode bits */
> +	if (val & (MDIO_PMA_STAT2_10GBSR | MDIO_PMA_STAT2_10GBLR |
> +		   MDIO_PMA_STAT2_10GBER | MDIO_PMA_STAT2_10GBLX4 |
> +		   MDIO_PMA_STAT2_10GBSW | MDIO_PMA_STAT2_10GBLW |
> +		   MDIO_PMA_STAT2_10GBEW))
> +		__set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, supported);
> +	if (val & MDIO_PMA_STAT2_10GBSR)
> +		__set_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT, supported);
> +	if (val & MDIO_PMA_STAT2_10GBLR)
> +		__set_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT, supported);
> +	if (val & MDIO_PMA_STAT2_10GBER)
> +		__set_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT, supported);
> +
> +	if (val & MDIO_PMA_STAT2_EXTABLE) {
> +		val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_EXTABLE);
> +		if (val < 0)
> +			return val;
> +
> +		if (val & (MDIO_PMA_EXTABLE_10GBT | MDIO_PMA_EXTABLE_1000BT |
> +			   MDIO_PMA_EXTABLE_100BTX | MDIO_PMA_EXTABLE_10BT))
> +			__set_bit(ETHTOOL_LINK_MODE_TP_BIT, supported);
> +		if (val & MDIO_PMA_EXTABLE_10GBLRM)
> +			__set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, supported);
> +		if (val & (MDIO_PMA_EXTABLE_10GBKX4 | MDIO_PMA_EXTABLE_10GBKR |
> +			   MDIO_PMA_EXTABLE_1000BKX))
> +			__set_bit(ETHTOOL_LINK_MODE_Backplane_BIT, supported);
> +		if (val & MDIO_PMA_EXTABLE_10GBLRM)
> +			__set_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT,
> +				  supported);
> +		if (val & MDIO_PMA_EXTABLE_10GBT)
> +			__set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
> +				  supported);
> +		if (val & MDIO_PMA_EXTABLE_10GBKX4)
> +			__set_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
> +				  supported);
> +		if (val & MDIO_PMA_EXTABLE_10GBKR)
> +			__set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
> +				  supported);
> +		if (val & MDIO_PMA_EXTABLE_1000BT)
> +			__set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
> +				  supported);
> +		if (val & MDIO_PMA_EXTABLE_1000BKX)
> +			__set_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
> +				  supported);
> +		if (val & MDIO_PMA_EXTABLE_100BTX)
> +			__set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
> +				  supported);
> +		if (val & MDIO_PMA_EXTABLE_10BT)
> +			__set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
> +				  supported);
> +	}
> +
> +	if (!ethtool_convert_link_mode_to_legacy_u32(&mask, supported))
> +		dev_warn(&phydev->mdio.dev,
> +			 "PHY supports (%*pb) more modes than phylib supports, some modes not supported.\n",
> +			 __ETHTOOL_LINK_MODE_MASK_NBITS, supported);
> +
> +	phydev->supported &= mask;
> +	phydev->advertising &= phydev->supported;

Is not this a candidate for being moved into drivers/net/phy/phy-10g.c?
Past the phy interface mode check, the rest does not appear to be
particularly Marvell specific here.

Other than that, and making it a separate Kconfig option, this looks great!
-- 
Florian

Re: [PATCH 5/5] net: phy: add Marvell Alaska X 88X3310 10Gigabit PHY support

[Russell King - ARM Linux]

On Thu, Jun 01, 2017 at 10:28:04AM -0700, Florian Fainelli wrote:
> On 06/01/2017 03:26 AM, Russell King wrote:
> > Add phylib support for the Marvell Alaska X 10 Gigabit PHY (MV88X3310).
> > This phy is able to operate at 10G, 1G, 100M and 10M speeds, and only
> > supports Clause 45 accesses.
> > 
> > The PHY appears (based on the vendor IDs) to be two different vendors
> > IP, with each devad containing several instances.
> > 
> > This PHY driver has only been tested with the RJ45 copper port, fiber
> > port and a Marvell Armada 8040-based ethernet interface.
> > 
> > It should be noted that to use the full range of speeds, MAC drivers
> > need to also reconfigure the link mode as per phydev->interface, since
> > the PHY automatically changes its interface mode depending on the
> > negotiated speed.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> 
> >  MARVELL MVNETA ETHERNET DRIVER
> >  M:	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> >  L:	netdev@vger.kernel.org
> > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> > index 19eddf758c88..905990fece28 100644
> > --- a/drivers/net/phy/Makefile
> > +++ b/drivers/net/phy/Makefile
> > @@ -55,7 +55,7 @@ obj-$(CONFIG_ICPLUS_PHY)	+= icplus.o
> >  obj-$(CONFIG_INTEL_XWAY_PHY)	+= intel-xway.o
> >  obj-$(CONFIG_LSI_ET1011C_PHY)	+= et1011c.o
> >  obj-$(CONFIG_LXT_PHY)		+= lxt.o
> > -obj-$(CONFIG_MARVELL_PHY)	+= marvell.o
> > +obj-$(CONFIG_MARVELL_PHY)	+= marvell.o marvell10g.o
> 
> Could we introduce a CONFIG_MARVELL_10G_PHY symbol?

Thanks, I'll do that.

> > +enum {
> > +	MV_PCS_BASE_T		= 0x0000,
> > +	MV_PCS_BASE_R		= 0x1000,
> > +	MV_PCS_1000BASEX	= 0x2000,
> > +
> > +	/* These registers appear at 0x800X and 0xa00X - the 0xa00X control
> > +	 * registers appear to set themselves to the 0x800X when AN is
> > +	 * restarted, but status registers appear readable from either.
> > +	 */
> > +	MV_AN_CTRL1000		= 0x8000, /* 1000base-T control register */
> > +	MV_AN_STAT1000		= 0x8001, /* 1000base-T status register */
> > +
> > +	/* This register appears to reflect the copper status */
> > +	MV_AN_RESULT		= 0xa016,
> > +	MV_AN_RESULT_SPD_10	= BIT(12),
> > +	MV_AN_RESULT_SPD_100	= BIT(13),
> > +	MV_AN_RESULT_SPD_1000	= BIT(14),
> > +	MV_AN_RESULT_SPD_10000	= BIT(15),
> > +};
> 
> Personal preference is not to use enums but jut plain #define, but
> whatever works, really.

I've come to prefer them as it avoids hiding them from the compiler
itself.

> > +
> > +static int mv3310_soft_reset(struct phy_device *phydev)
> > +{
> > +	return 0;
> > +}
> 
> You could use genphy_no_soft_reset(), although genphy sort of implies
> C22 now, or we could export genphy10g_soft_reset() which also does nothing.

The reason 88x3310 has no reset function is that setting the reset bit
appears to cause the PHY to become completely inaccessible over MDIO.
I'll add a comment here to that effect - but that would need the function
to remain here, otherwise the comment is rather lost amongst the code.

> > +	/* Ethtool does not support the WAN mode bits */
> > +	if (val & (MDIO_PMA_STAT2_10GBSR | MDIO_PMA_STAT2_10GBLR |
> > +		   MDIO_PMA_STAT2_10GBER | MDIO_PMA_STAT2_10GBLX4 |
> > +		   MDIO_PMA_STAT2_10GBSW | MDIO_PMA_STAT2_10GBLW |
> > +		   MDIO_PMA_STAT2_10GBEW))
> > +		__set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, supported);
> > +	if (val & MDIO_PMA_STAT2_10GBSR)
> > +		__set_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT, supported);
> > +	if (val & MDIO_PMA_STAT2_10GBLR)
> > +		__set_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT, supported);
> > +	if (val & MDIO_PMA_STAT2_10GBER)
> > +		__set_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT, supported);
> > +
> > +	if (val & MDIO_PMA_STAT2_EXTABLE) {
> > +		val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_EXTABLE);
> > +		if (val < 0)
> > +			return val;
> > +
> > +		if (val & (MDIO_PMA_EXTABLE_10GBT | MDIO_PMA_EXTABLE_1000BT |
> > +			   MDIO_PMA_EXTABLE_100BTX | MDIO_PMA_EXTABLE_10BT))
> > +			__set_bit(ETHTOOL_LINK_MODE_TP_BIT, supported);
> > +		if (val & MDIO_PMA_EXTABLE_10GBLRM)
> > +			__set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, supported);
> > +		if (val & (MDIO_PMA_EXTABLE_10GBKX4 | MDIO_PMA_EXTABLE_10GBKR |
> > +			   MDIO_PMA_EXTABLE_1000BKX))
> > +			__set_bit(ETHTOOL_LINK_MODE_Backplane_BIT, supported);
> > +		if (val & MDIO_PMA_EXTABLE_10GBLRM)
> > +			__set_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT,
> > +				  supported);
> > +		if (val & MDIO_PMA_EXTABLE_10GBT)
> > +			__set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
> > +				  supported);
> > +		if (val & MDIO_PMA_EXTABLE_10GBKX4)
> > +			__set_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
> > +				  supported);
> > +		if (val & MDIO_PMA_EXTABLE_10GBKR)
> > +			__set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
> > +				  supported);
> > +		if (val & MDIO_PMA_EXTABLE_1000BT)
> > +			__set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
> > +				  supported);
> > +		if (val & MDIO_PMA_EXTABLE_1000BKX)
> > +			__set_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
> > +				  supported);
> > +		if (val & MDIO_PMA_EXTABLE_100BTX)
> > +			__set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
> > +				  supported);
> > +		if (val & MDIO_PMA_EXTABLE_10BT)
> > +			__set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
> > +				  supported);
> > +	}
> > +
> > +	if (!ethtool_convert_link_mode_to_legacy_u32(&mask, supported))
> > +		dev_warn(&phydev->mdio.dev,
> > +			 "PHY supports (%*pb) more modes than phylib supports, some modes not supported.\n",
> > +			 __ETHTOOL_LINK_MODE_MASK_NBITS, supported);
> > +
> > +	phydev->supported &= mask;
> > +	phydev->advertising &= phydev->supported;
> 
> Is not this a candidate for being moved into drivers/net/phy/phy-10g.c?
> Past the phy interface mode check, the rest does not appear to be
> particularly Marvell specific here.

I'm not entirely sure.  This is decoding what the PMA/PMD supports,
which may not be solely appropriate for a generic case.  Clause 45
is full of different blocks, each advertising their own capabilities.
(Eg, see the highly modified mii-diag output below.)  The blocks
that are in use for the copper connection shown are the first PMA 0:1,
first PCS 0:3, second PHYXS 0:4, first AN 0:7 (for copper side) and
third AN (for MAC facing side).

Eg, the PCS says it supports 10GbaseR, 10GbaseX and 40GbaseR, but
the PMA says it doesn't support any 40G modes.

So, I wouldn't like to push code into a generic file which isn't the
correct approach.

Does anyone have a view on how Clause 45 PHYs should be handled wrt
what ethtool modes they support?  Do we (eg) need to scan all
implemented MMDs and come up with a common subset of features?


mii-diag.c:v2.11 3/21/2005 Donald Becker (becker@scyld.com)
 http://www.scyld.com/diag/index.html
  Using the new SIOCGMIIPHY value on PHY 8 (BMCR 0x2040).
Using the specified MII PHY index 32768.
  No MII transceiver present!.
  Use '--force' to view the information anyway.
libmii.c:v2.11 2/28/2005  Donald Becker (becker@scyld.com)
 http://www.scyld.com/diag/index.html

 MII PHY #0:1 transceiver registers:
   2040 0006 002b 09aa 0071 009a c000 0009
   9301 0000 0000 01a4 0000 0000 002b 09aa
   0000 0000 0000 0000 0000 0003 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000.
 Vendor ID is 00:0a:c2:--:--:--, model 26 rev. 10.
   Vendor/Part: Marvell Semiconductor 88x3310.
 PMA/PMD Control 1 register 2040: Speed determined by auto-negotiation.
 PMA/PMD Status register 0006 ... 0006.
   Receive link status: established.
 PMA/PMD Speed capability 0071: 10G, 100M, 10M, 10G/1G, 10G.
 PMA/PMD Control 2 register 0009: Type determined by auto-negotiation.
 PMA/PMD Status 2 register 9301 ... 9301
   Abilities: Local loopback, Transmit disable, Receive fault.
 PMA/PMD Extended Ability register 01a4
   Abilities: 10GBASE-T, 1000BASE-T, 100BASE-TX, 10BASE-T.
 Pair swap 0003: MDI no crossover, A normal, B normal, C normal, D normal.
 TX backoff b400: LP TX backoff 10dB TX backoff 10dB.
 Operating SNR: A:   7.8dB  B:   2.2dB  C:   4.5dB  D:   4.4dB.
 Minimum SNR  : A:   7.8dB  B:   2.2dB  C:   4.5dB  D:   4.4dB.
 Skew A>B -75.00ns A>C -80.00ns A>D -80.00ns
 Fast retrain 0012.

 MII PHY #0:3 transceiver registers:
   2040 0006 002b 09aa 0001 009a c000 0003
   8008 0000 0000 0000 0000 0000 002b 09aa
   0000 0000 0000 0000 000e 0003 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   1001 8000 0000 0000 0000 0000 0000 0000
   0000 0be3 0b83 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000.
 Vendor ID is 00:0a:c2:--:--:--, model 26 rev. 10.
   Vendor/Part: Marvell Semiconductor 88x3310.
 PCS Control 1 register 2040: Speed 10Gb/s.
 PCS Status register 0006 ... 0006.
   Receive link status: established.
 PCS Speed capability 0001: 10G.
 PCS Control 2 register 0003: Type 10GBASE-T.
 PCS Status 2 register 8008 ... 8008
   Abilities: 10GBASE-T.
 PCS BASE-R or 10GBASE-T status 1001 8000
   PCS receive link up
   Block lock
   Block lock (latched).

 MII PHY #0:3 Subdevice #2 transceiver registers
   2040 0082 002b 09aa 0005 009e c000 0000
   8c13 0000 0000 0000 0000 0000 002b 09aa
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   000c 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000.
 Vendor ID is 00:0a:c2:--:--:--, model 26 rev. 10.
   Vendor/Part: Marvell Semiconductor 88x3310.
 PCS Control 1 register 2040: Speed 10Gb/s.
 PCS Status register 0082 ... 0082.
   Receive link status: not established.
   *Fault condition detected*
 PCS Speed capability 0005: 10G, 40G.
 PCS Control 2 register 0000: Type 10GBASE-R.
 PCS Status 2 register 8c13 ... 8c13
   Abilities: 10GBASE-R, 10GBASE-X, 40GBASE-R.
   *Transmit fault reported*.
   *Receive fault reported*.
 PCS BASE-R or 10GBASE-T status 000c 0000
   PRBS9 pattern testing
   PRBS31 pattern testing.

 MII PHY #0:3 Subdevice #3 transceiver registers
   1140 0149 0141 0daa 0020 0000 0004 2001
   0000 0000 0000 0000 0000 0000 0000 8000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   000c 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000.
 Vendor ID is 00:50:43:--:--:--, model 26 rev. 10.
   Vendor/Part: Marvell Semiconductor 88x3310.
 Basic mode control register 1140: Auto-negotiation enabled.
 Basic mode status register 0149 ... 0149.
   With extended status register 8000.
   Link status: not established.
   Capable of 1000baseX-FD.
   Able to perform Auto-negotiation, negotiation not complete.
 I'm advertising 0020: 1000baseX-FD.

 MII PHY #0:4 transceiver registers:
   2040 0082 0141 0daa 0001 001a 4000 0001
   8403 0000 0000 0000 0000 0000 0141 0daa
   0000 0000 0000 0000 0000 0000 0000 0000
   0c00 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000.
 Vendor ID is 00:50:43:--:--:--, model 26 rev. 10.
   Vendor/Part: Marvell Semiconductor 88x3310.
 PHYXS Control 1 register 2040: Speed 10Gb/s.
 PHYXS Status register 0082 ... 0082.
   Receive link status: not established.
   *Fault condition detected*
 PHYXS Speed capability 0001: 10G.

 MII PHY #0:4 Subdevice #2 transceiver registers
   2040 0006 002b 09aa 0005 009e c000 0000
   8013 0000 0000 0000 0000 0000 002b 09aa
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   100d 8000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000.
 Vendor ID is 00:0a:c2:--:--:--, model 26 rev. 10.
   Vendor/Part: Marvell Semiconductor 88x3310.
 PHYXS Control 1 register 2040: Speed 10Gb/s.
 PHYXS Status register 0006 ... 0006.
   Receive link status: established.
 PHYXS Speed capability 0005: 10G.

 MII PHY #0:4 Subdevice #3 transceiver registers
   1140 0149 0141 0daa 0020 0000 0004 2001
   0000 0000 0000 0000 0000 0000 0000 8000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000.
 Vendor ID is 00:50:43:--:--:--, model 26 rev. 10.
   Vendor/Part: Marvell Semiconductor 88x3310.
 Basic mode control register 1140: Auto-negotiation enabled.
 Basic mode status register 0149 ... 0149.
   With extended status register 8000.
   Link status: not established.
   Capable of 1000baseX-FD.
   Able to perform Auto-negotiation, negotiation not complete.
 SGMII Link status 0020: Link down, reserved bits are set
 SGMII MAC acknowledgement 0000: 

 MII PHY #0:7 transceiver registers:
   3000 00ad 002b 09aa 0000 009a c000 0000
   0000 0000 0000 0000 0000 0000 002b 09aa
   1d41 0000 0000 dd41 0000 0000 2001 0000
   0000 4c00 0003 0000 0000 0000 0000 0000
   1181 7c60 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000.
 Vendor ID is 00:0a:c2:--:--:--, model 26 rev. 10.
   Vendor/Part: Marvell Semiconductor 88x3310.
 AN Control 1 register 3000.
 AN Status register 00ad ... 00ad.
   Link status: established.
   Able to perform Auto-negotiation, negotiation complete.
   LP Auto-negotiation, Ext Next Pg.
 I'm advertising 1d41 1181.
   10Gbase-T AsymPause Pause 100baseTx-FD 10baseT-FD 
   I'm part of a single-port device.
   LD loop timing.
   Advertising no additional info pages.
   IEEE 802.3 CSMA/CD protocol.
 Link partner advertisment is dd41.
   Advertising AsymPause Pause 100baseTx-FD 10baseT-FD.
   Negotiation completed.
 XNP advert is 2001: message page 1
 XNP link partner is 4c00: unformatted code 400
 10GBASE-T status 7c60: Local PHY master LP 10GBASE-T LP loop timing.

 MII PHY #0:7 Subdevice #2 transceiver registers
   0000 000c 0141 0d90 0000 009e c000 0000
   0000 0000 0000 0000 0000 0000 0141 0d90
   0001 008d 0000 0000 0000 0000 2001 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0009 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000.
 Vendor ID is 00:50:43:--:--:--, model 25 rev. 0.
   Vendor/Part: Marvell Semiconductor (unknown).
 AN Control 1 register 0000.
 AN Status register 000c ... 000c.
   Link status: established.
   Able to perform Auto-negotiation, negotiation not complete.
 I'm advertising 0000008d0001.
   Advertising 10GBASE-KR.
   IEEE 802.3 CSMA/CD protocol.
 BASE-R Status: 10GBASE-KR.

 MII PHY #0:7 Subdevice #3 transceiver registers
   3000 00ad 0141 0c00 0000 008a 0000 0000
   0000 0000 0000 0000 0000 0000 0141 0c00
   1d41 0000 0000 dd41 0000 0000 2001 0000
   0000 4c00 0003 0000 0000 0000 0000 0000
   1181 4c60 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000
   0000 0000 0000 0000 0000 0000 0000 0000.
 Vendor ID is 00:50:43:--:--:--, model 0 rev. 0.
   Vendor/Part: Marvell Semiconductor (unknown).
 AN Control 1 register 3000.
 AN Status register 00ad ... 00ad.
   Link status: established.
   Able to perform Auto-negotiation, negotiation complete.
   LP Auto-negotiation, Ext Next Pg.
 I'm advertising 1d41 1181.
   10Gbase-T AsymPause Pause 100baseTx-FD 10baseT-FD 
   I'm part of a single-port device.
   LD loop timing.
   Advertising no additional info pages.
   IEEE 802.3 CSMA/CD protocol.
 Link partner advertisment is dd41.
   Advertising AsymPause Pause 100baseTx-FD 10baseT-FD.
   Negotiation completed.
 XNP advert is 2001: message page 1
 XNP link partner is 4c00: unformatted code 400
 10GBASE-T status 4c60: Local PHY master Local RX not ok Remote RX not ok LP 10GBASE-T LP loop timing.

 Other registers
  8000: 0210 0b00 7973 0000 0404 0000 0000 800c
  8008: 0000 1000 0000 0000 0101 2909 0000 0077

  9000: 0010 014a 0000 0000 0000 0000 0000 0000
  9008: 0000 0000 0000 0002 0001 0000 0000 0000

  9800: 0010 014a 0000 0000 0000 0000 0000 0000
  9808: 0000 0000 0000 0002 0001 0000 0000 0000

  a000: 0210 0b00 7973 0000 0404 0000 0000 800c
  a008: 0000 1000 0000 0000 0101 2909 2000 0010
  a010: 0010 0000 0000 0000 0000 0000 8056 e009
  a018: 402c 1803 0000 0000 0000 0000 0000 0000
  a020: a400 0000 0000 0521 0200 0001 8521 0200
  a028: 2901 8415 e02c 8000 1000 0000 0000 0000
  a030: 073e 00fa 0000 0000 0000 0000 0000 0000

This produces:

root@mcbin:~# /shared/git/ethtool/build-arm64/ethtool eth0
Settings for eth0:
        Supported ports: [ TP ]
        Supported link modes:   10baseT/Full
                                100baseT/Full
                                1000baseT/Full
                                10000baseT/Full
        Supported pause frame use: Symmetric Receive-only
        Supports auto-negotiation: Yes
        Advertised link modes:  10baseT/Full
                                100baseT/Full
                                1000baseT/Full
                                10000baseT/Full
        Advertised pause frame use: Symmetric Receive-only
        Advertised auto-negotiation: Yes
        Link partner advertised link modes:  10baseT/Full
                                             100baseT/Full
                                             1000baseT/Full
                                             10000baseT/Full
        Link partner advertised pause frame use: Symmetric Receive-only
        Link partner advertised auto-negotiation: Yes
        Speed: 10000Mb/s
        Duplex: Full
        Port: MII
        PHYAD: 0
        Transceiver: internal
        Auto-negotiation: on
        Link detected: yes

Note the lack of fiber modes - the PMA doesn't appear to be used for
fiber, and indicates that it doesn't support fiber.

So, with all this uncertainty, I'd rather not push the above block of
code as a generic helper just yet - at least not until we see more
10G PHYs, so we can get an idea of the correct direction for this.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 0/5] Add phylib support for MV88X3310 10G phy

[David Miller]

From: Russell King - ARM Linux <linux@armlinux.org.uk>
Date: Thu, 1 Jun 2017 11:23:27 +0100

> This patch series adds support for the Marvell 88x3310 PHY found on
> the SolidRun Macchiatobin board.

Andrew has asked for some comment documentation additions to patch #5
so I'm expecting at least one respin of this series ;-)

Re: [PATCH 0/5] Add phylib support for MV88X3310 10G phy

[Russell King - ARM Linux]

On Thu, Jun 01, 2017 at 12:07:36PM -0400, David Miller wrote:
> From: Russell King - ARM Linux <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
> Date: Thu, 1 Jun 2017 11:23:27 +0100
> 
> > This patch series adds support for the Marvell 88x3310 PHY found on
> > the SolidRun Macchiatobin board.
> 
> Andrew has asked for some comment documentation additions to patch #5
> so I'm expecting at least one respin of this series ;-)

Thanks.

It would help if Documentation/networking/phy.txt already contained
documentation about the adjust_link function beyond describing that it
exists.  I'll try to write something to cover the existing usage and
this change over the next day or so.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/6] Add phylib support for MV88X3310 10G phy

[Russell King - ARM Linux]

Hi,

This patch series adds support for the Marvell 88x3310 PHY found on
the SolidRun Macchiatobin board.

The first patch introduces a set of generic Clause 45 PHY helpers that
C45 PHY drivers can make use of if they wish.

Patch 2 ensures that the Clause 22 aneg_done function will not be
called for incompatible Clause 45 PHYs.

Patch 3 fixes the aneg restart to be compatible with C45 PHYs - it can
currently only cope with C22 PHYs.

Patch 4 moves the "gen10g" driver into the Clause 45 code, grouping all
core clause 45 code together.

Patch 5 adds the phy_interface_t types for XAUI and 10GBase-KR links.
As 10GBase-KR appears to be compatible with XFI and SFI, XFI and SFI,
I currently see no reason to add XFI and SFI interface modes.  There
seems to be vendor code out there using these, but they all alias back
to the same hardware settings.

Patch 6 adds support for the MV88X3310 PHY, which supports both the
copper and fiber interfaces.  It should be noted that the MV88X3310
automatically switches its MAC facing interface between 10GBase-KR
and SGMII depending on the negotiated speed.  This was discussed with
Florian, and we agreed to update the phy interface mode depending on
the properties of the actual link mode to the PHY.

v2:
- update sysfs-class-net-phydev documentation
- avoid genphy_aneg_done for non-C22 PHYs
- expand comment about 0x30 constant
- add comment about lack of reset
- configure driver using MARVELL_10G_PHY

 Documentation/ABI/testing/sysfs-class-net-phydev   |   2 +-
 Documentation/devicetree/bindings/net/ethernet.txt |   2 +
 MAINTAINERS                                        |   6 +
 drivers/net/phy/Kconfig                            |   5 +
 drivers/net/phy/Makefile                           |   3 +-
 drivers/net/phy/marvell10g.c                       | 368 +++++++++++++++++++++
 drivers/net/phy/phy-c45.c                          | 298 +++++++++++++++++
 drivers/net/phy/phy.c                              |  29 +-
 drivers/net/phy/phy_device.c                       | 113 ++-----
 include/linux/phy.h                                |  20 ++
 10 files changed, 748 insertions(+), 98 deletions(-)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/6] net: phy: add 802.3 clause 45 support to phylib

[Russell King]

Add generic helpers for 802.3 clause 45 PHYs for >= 10Gbps support.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/Makefile     |   2 +-
 drivers/net/phy/phy-c45.c    | 234 +++++++++++++++++++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c |  20 ++--
 include/linux/phy.h          |  12 +++
 4 files changed, 253 insertions(+), 15 deletions(-)
 create mode 100644 drivers/net/phy/phy-c45.c

diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index e2fde094f63d..ae58f507aba9 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -1,6 +1,6 @@
 # Makefile for Linux PHY drivers and MDIO bus drivers
 
-libphy-y			:= phy.o phy-core.o phy_device.o
+libphy-y			:= phy.o phy-c45.o phy-core.o phy_device.o
 mdio-bus-y			+= mdio_bus.o mdio_device.o
 
 ifdef CONFIG_MDIO_DEVICE
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
new file mode 100644
index 000000000000..d311d6e6141c
--- /dev/null
+++ b/drivers/net/phy/phy-c45.c
@@ -0,0 +1,234 @@
+/*
+ * Clause 45 PHY support
+ */
+#include <linux/ethtool.h>
+#include <linux/export.h>
+#include <linux/mdio.h>
+#include <linux/mii.h>
+#include <linux/phy.h>
+
+/**
+ * genphy_c45_setup_forced - configures a forced speed
+ * @phydev: target phy_device struct
+ */
+int genphy_c45_pma_setup_forced(struct phy_device *phydev)
+{
+	int ctrl1, ctrl2, ret;
+
+	/* Half duplex is not supported */
+	if (phydev->duplex != DUPLEX_FULL)
+		return -EINVAL;
+
+	ctrl1 = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1);
+	if (ctrl1 < 0)
+		return ctrl1;
+
+	ctrl2 = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL2);
+	if (ctrl2 < 0)
+		return ctrl2;
+
+	ctrl1 &= ~MDIO_CTRL1_SPEEDSEL;
+	/*
+	 * PMA/PMD type selection is 1.7.5:0 not 1.7.3:0.  See 45.2.1.6.1
+	 * in 802.3-2012 and 802.3-2015.
+	 */
+	ctrl2 &= ~(MDIO_PMA_CTRL2_TYPE | 0x30);
+
+	switch (phydev->speed) {
+	case SPEED_10:
+		ctrl2 |= MDIO_PMA_CTRL2_10BT;
+		break;
+	case SPEED_100:
+		ctrl1 |= MDIO_PMA_CTRL1_SPEED100;
+		ctrl2 |= MDIO_PMA_CTRL2_100BTX;
+		break;
+	case SPEED_1000:
+		ctrl1 |= MDIO_PMA_CTRL1_SPEED1000;
+		/* Assume 1000base-T */
+		ctrl2 |= MDIO_PMA_CTRL2_1000BT;
+		break;
+	case SPEED_10000:
+		ctrl1 |= MDIO_CTRL1_SPEED10G;
+		/* Assume 10Gbase-T */
+		ctrl2 |= MDIO_PMA_CTRL2_10GBT;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1, ctrl1);
+	if (ret < 0)
+		return ret;
+
+	return phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL2, ctrl2);
+}
+EXPORT_SYMBOL_GPL(genphy_c45_pma_setup_forced);
+
+/**
+ * genphy_c45_an_disable_aneg - disable auto-negotiation
+ * @phydev: target phy_device struct
+ *
+ * Disable auto-negotiation in the Clause 45 PHY. The link parameters
+ * parameters are controlled through the PMA/PMD MMD registers.
+ *
+ * Returns zero on success, negative errno code on failure.
+ */
+int genphy_c45_an_disable_aneg(struct phy_device *phydev)
+{
+	int val;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1);
+	if (val < 0)
+		return val;
+
+	val &= ~(MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART);
+
+	return phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1, val);
+}
+EXPORT_SYMBOL_GPL(genphy_c45_an_disable_aneg);
+
+/**
+ * genphy_c45_restart_aneg - Enable and restart auto-negotiation
+ * @phydev: target phy_device struct
+ *
+ * This assumes that the auto-negotiation MMD is present.
+ *
+ * Enable and restart auto-negotiation.
+ */
+int genphy_c45_restart_aneg(struct phy_device *phydev)
+{
+	int val;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1);
+	if (val < 0)
+		return val;
+
+	val |= MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART;
+
+	return phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1, val);
+}
+EXPORT_SYMBOL_GPL(genphy_c45_restart_aneg);
+
+/**
+ * genphy_c45_aneg_done - return auto-negotiation complete status
+ * @phydev: target phy_device struct
+ *
+ * This assumes that the auto-negotiation MMD is present.
+ *
+ * Reads the status register from the auto-negotiation MMD, returning:
+ * - positive if auto-negotiation is complete
+ * - negative errno code on error
+ * - zero otherwise
+ */
+int genphy_c45_aneg_done(struct phy_device *phydev)
+{
+	int val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
+
+	return val < 0 ? val : val & MDIO_AN_STAT1_COMPLETE ? 1 : 0;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_aneg_done);
+
+/**
+ * genphy_c45_read_link - read the overall link status from the MMDs
+ * @phydev: target phy_device struct
+ * @mmd_mask: MMDs to read status from
+ *
+ * Read the link status from the specified MMDs, and if they all indicate
+ * that the link is up, return positive.  If an error is encountered,
+ * a negative errno will be returned, otherwise zero.
+ */
+int genphy_c45_read_link(struct phy_device *phydev, u32 mmd_mask)
+{
+	int val, devad;
+	bool link = true;
+
+	while (mmd_mask) {
+		devad = __ffs(mmd_mask);
+		mmd_mask &= ~BIT(devad);
+
+		/* The link state is latched low so that momentary link
+		 * drops can be detected.  Do not double-read the status
+		 * register if the link is down.
+		 */
+		val = phy_read_mmd(phydev, devad, MDIO_STAT1);
+		if (val < 0)
+			return val;
+
+		if (!(val & MDIO_STAT1_LSTATUS))
+			link = false;
+	}
+
+	return link;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_read_link);
+
+/**
+ * genphy_c45_read_lpa - read the link partner advertisment and pause
+ * @phydev: target phy_device struct
+ *
+ * Read the Clause 45 defined base (7.19) and 10G (7.33) status registers,
+ * filling in the link partner advertisment, pause and asym_pause members
+ * in @phydev.  This assumes that the auto-negotiation MMD is present, and
+ * the backplane bit (7.48.0) is clear.  Clause 45 PHY drivers are expected
+ * to fill in the remainder of the link partner advert from vendor registers.
+ */
+int genphy_c45_read_lpa(struct phy_device *phydev)
+{
+	int val;
+
+	/* Read the link partner's base page advertisment */
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_LPA);
+	if (val < 0)
+		return val;
+
+	phydev->lp_advertising = mii_lpa_to_ethtool_lpa_t(val);
+	phydev->pause = val & LPA_PAUSE_CAP ? 1 : 0;
+	phydev->asym_pause = val & LPA_PAUSE_ASYM ? 1 : 0;
+
+	/* Read the link partner's 10G advertisment */
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_STAT);
+	if (val < 0)
+		return val;
+
+	if (val & MDIO_AN_10GBT_STAT_LP10G)
+		phydev->lp_advertising |= ADVERTISED_10000baseT_Full;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_read_lpa);
+
+/**
+ * genphy_c45_read_pma - read link speed etc from PMA
+ * @phydev: target phy_device struct
+ */
+int genphy_c45_read_pma(struct phy_device *phydev)
+{
+	int val;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1);
+	if (val < 0)
+		return val;
+
+	switch (val & MDIO_CTRL1_SPEEDSEL) {
+	case 0:
+		phydev->speed = SPEED_10;
+		break;
+	case MDIO_PMA_CTRL1_SPEED100:
+		phydev->speed = SPEED_100;
+		break;
+	case MDIO_PMA_CTRL1_SPEED1000:
+		phydev->speed = SPEED_1000;
+		break;
+	case MDIO_CTRL1_SPEED10G:
+		phydev->speed = SPEED_10000;
+		break;
+	default:
+		phydev->speed = SPEED_UNKNOWN;
+		break;
+	}
+
+	phydev->duplex = DUPLEX_FULL;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_read_pma);
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 37a1e98908e3..6a79e24fa312 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1508,27 +1508,19 @@ EXPORT_SYMBOL(genphy_read_status);
 
 static int gen10g_read_status(struct phy_device *phydev)
 {
-	int devad, reg;
 	u32 mmd_mask = phydev->c45_ids.devices_in_package;
-
-	phydev->link = 1;
+	int ret;
 
 	/* For now just lie and say it's 10G all the time */
 	phydev->speed = SPEED_10000;
 	phydev->duplex = DUPLEX_FULL;
 
-	for (devad = 0; mmd_mask; devad++, mmd_mask = mmd_mask >> 1) {
-		if (!(mmd_mask & 1))
-			continue;
+	/* Avoid reading the vendor MMDs */
+	mmd_mask &= ~(BIT(MDIO_MMD_VEND1) | BIT(MDIO_MMD_VEND2));
 
-		/* Read twice because link state is latched and a
-		 * read moves the current state into the register
-		 */
-		phy_read_mmd(phydev, devad, MDIO_STAT1);
-		reg = phy_read_mmd(phydev, devad, MDIO_STAT1);
-		if (reg < 0 || !(reg & MDIO_STAT1_LSTATUS))
-			phydev->link = 0;
-	}
+	ret = genphy_c45_read_link(phydev, mmd_mask);
+
+	phydev->link = ret > 0 ? 1 : 0;
 
 	return 0;
 }
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 58f1b45a4c44..e089253f98a2 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -827,6 +827,8 @@ static inline const char *phydev_name(const struct phy_device *phydev)
 void phy_attached_print(struct phy_device *phydev, const char *fmt, ...)
 	__printf(2, 3);
 void phy_attached_info(struct phy_device *phydev);
+
+/* Clause 22 PHY */
 int genphy_config_init(struct phy_device *phydev);
 int genphy_setup_forced(struct phy_device *phydev);
 int genphy_restart_aneg(struct phy_device *phydev);
@@ -841,6 +843,16 @@ static inline int genphy_no_soft_reset(struct phy_device *phydev)
 {
 	return 0;
 }
+
+/* Clause 45 PHY */
+int genphy_c45_restart_aneg(struct phy_device *phydev);
+int genphy_c45_aneg_done(struct phy_device *phydev);
+int genphy_c45_read_link(struct phy_device *phydev, u32 mmd_mask);
+int genphy_c45_read_lpa(struct phy_device *phydev);
+int genphy_c45_read_pma(struct phy_device *phydev);
+int genphy_c45_pma_setup_forced(struct phy_device *phydev);
+int genphy_c45_an_disable_aneg(struct phy_device *phydev);
+
 void phy_driver_unregister(struct phy_driver *drv);
 void phy_drivers_unregister(struct phy_driver *drv, int n);
 int phy_driver_register(struct phy_driver *new_driver, struct module *owner);
-- 
2.7.4

Re: [PATCH 1/6] net: phy: add 802.3 clause 45 support to phylib

[Florian Fainelli]

On 06/05/2017 04:22 AM, Russell King wrote:
> Add generic helpers for 802.3 clause 45 PHYs for >= 10Gbps support.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

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

[PATCH 2/6] net: phy: avoid genphy_aneg_done() for PHYs without clause 22 support

[Russell King]

Avoid calling genphy_aneg_done() for PHYs that do not implement the
Clause 22 register set.

Clause 45 PHYs may implement the Clause 22 register set along with the
Clause 22 extension MMD.  Hence, we can't simply block access to the
Clause 22 functions based on the PHY being a Clause 45 PHY.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phy.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 82ab8fb82587..788da83a5c7e 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -163,6 +163,12 @@ int phy_aneg_done(struct phy_device *phydev)
 	if (phydev->drv && phydev->drv->aneg_done)
 		return phydev->drv->aneg_done(phydev);
 
+	/* Avoid genphy_aneg_done() if the Clause 45 PHY does not
+	 * implement Clause 22 registers
+	 */
+	if (phydev->is_c45 && !(phydev->c45_ids.devices_in_package & BIT(0)))
+		return -EINVAL;
+
 	return genphy_aneg_done(phydev);
 }
 EXPORT_SYMBOL(phy_aneg_done);
-- 
2.7.4

Re: [PATCH 2/6] net: phy: avoid genphy_aneg_done() for PHYs without clause 22 support

[Andrew Lunn]

On Mon, Jun 05, 2017 at 12:22:55PM +0100, Russell King wrote:
> Avoid calling genphy_aneg_done() for PHYs that do not implement the
> Clause 22 register set.
> 
> Clause 45 PHYs may implement the Clause 22 register set along with the
> Clause 22 extension MMD.  Hence, we can't simply block access to the
> Clause 22 functions based on the PHY being a Clause 45 PHY.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH 2/6] net: phy: avoid genphy_aneg_done() for PHYs without clause 22 support

[Florian Fainelli]

On 06/05/2017 04:22 AM, Russell King wrote:
> Avoid calling genphy_aneg_done() for PHYs that do not implement the
> Clause 22 register set.
> 
> Clause 45 PHYs may implement the Clause 22 register set along with the
> Clause 22 extension MMD.  Hence, we can't simply block access to the
> Clause 22 functions based on the PHY being a Clause 45 PHY.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

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

[PATCH 3/6] net: phy: hook up clause 45 autonegotiation restart

[Russell King]

genphy_restart_aneg() can only restart autonegotiation on clause 22
PHYs.  Add a phy_restart_aneg() function which selects between the
clause 22 and clause 45 restart functionality depending on the PHY
type and whether the Clause 45 PHY supports the Clause 22 register set.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phy.c | 23 +++++++++++++++++++++--
 include/linux/phy.h   |  1 +
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 788da83a5c7e..117eced4c08d 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -149,6 +149,25 @@ static int phy_config_interrupt(struct phy_device *phydev, u32 interrupts)
 	return 0;
 }
 
+/**
+ * phy_restart_aneg - restart auto-negotiation
+ * @phydev: target phy_device struct
+ *
+ * Restart the autonegotiation on @phydev.  Returns >= 0 on success or
+ * negative errno on error.
+ */
+int phy_restart_aneg(struct phy_device *phydev)
+{
+	int ret;
+
+	if (phydev->is_c45 && !(phydev->c45_ids.devices_in_package & BIT(0)))
+		ret = genphy_c45_restart_aneg(phydev);
+	else
+		ret = genphy_restart_aneg(phydev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phy_restart_aneg);
 
 /**
  * phy_aneg_done - return auto-negotiation status
@@ -1421,7 +1440,7 @@ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
 		/* Restart autonegotiation so the new modes get sent to the
 		 * link partner.
 		 */
-		ret = genphy_restart_aneg(phydev);
+		ret = phy_restart_aneg(phydev);
 		if (ret < 0)
 			return ret;
 	}
@@ -1480,6 +1499,6 @@ int phy_ethtool_nway_reset(struct net_device *ndev)
 	if (!phydev->drv)
 		return -EIO;
 
-	return genphy_restart_aneg(phydev);
+	return phy_restart_aneg(phydev);
 }
 EXPORT_SYMBOL(phy_ethtool_nway_reset);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e089253f98a2..e6fcee188c8d 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -804,6 +804,7 @@ int phy_start_aneg(struct phy_device *phydev);
 int phy_aneg_done(struct phy_device *phydev);
 
 int phy_stop_interrupts(struct phy_device *phydev);
+int phy_restart_aneg(struct phy_device *phydev);
 
 static inline int phy_read_status(struct phy_device *phydev)
 {
-- 
2.7.4

Re: [PATCH 3/6] net: phy: hook up clause 45 autonegotiation restart

[Andrew Lunn]

On Mon, Jun 05, 2017 at 12:23:00PM +0100, Russell King wrote:
> genphy_restart_aneg() can only restart autonegotiation on clause 22
> PHYs.  Add a phy_restart_aneg() function which selects between the
> clause 22 and clause 45 restart functionality depending on the PHY
> type and whether the Clause 45 PHY supports the Clause 22 register set.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH 3/6] net: phy: hook up clause 45 autonegotiation restart

[Florian Fainelli]

On 06/05/2017 04:23 AM, Russell King wrote:
> genphy_restart_aneg() can only restart autonegotiation on clause 22
> PHYs.  Add a phy_restart_aneg() function which selects between the
> clause 22 and clause 45 restart functionality depending on the PHY
> type and whether the Clause 45 PHY supports the Clause 22 register set.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

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

[PATCH 4/6] net: phy: split out 10G genphy support

[Russell King]

Move the old 10G genphy support to sit beside the new clause 45 library
functions, so all the 10G phy code is together.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phy-c45.c    |  64 ++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c | 105 ++++++++-----------------------------------
 2 files changed, 83 insertions(+), 86 deletions(-)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index d311d6e6141c..dada819c6b78 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -232,3 +232,67 @@ int genphy_c45_read_pma(struct phy_device *phydev)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(genphy_c45_read_pma);
+
+/* The gen10g_* functions are the old Clause 45 stub */
+
+static int gen10g_config_aneg(struct phy_device *phydev)
+{
+	return 0;
+}
+
+static int gen10g_read_status(struct phy_device *phydev)
+{
+	u32 mmd_mask = phydev->c45_ids.devices_in_package;
+	int ret;
+
+	/* For now just lie and say it's 10G all the time */
+	phydev->speed = SPEED_10000;
+	phydev->duplex = DUPLEX_FULL;
+
+	/* Avoid reading the vendor MMDs */
+	mmd_mask &= ~(BIT(MDIO_MMD_VEND1) | BIT(MDIO_MMD_VEND2));
+
+	ret = genphy_c45_read_link(phydev, mmd_mask);
+
+	phydev->link = ret > 0 ? 1 : 0;
+
+	return 0;
+}
+
+static int gen10g_soft_reset(struct phy_device *phydev)
+{
+	/* Do nothing for now */
+	return 0;
+}
+
+static int gen10g_config_init(struct phy_device *phydev)
+{
+	/* Temporarily just say we support everything */
+	phydev->supported = SUPPORTED_10000baseT_Full;
+	phydev->advertising = SUPPORTED_10000baseT_Full;
+
+	return 0;
+}
+
+static int gen10g_suspend(struct phy_device *phydev)
+{
+	return 0;
+}
+
+static int gen10g_resume(struct phy_device *phydev)
+{
+	return 0;
+}
+
+struct phy_driver genphy_10g_driver = {
+	.phy_id         = 0xffffffff,
+	.phy_id_mask    = 0xffffffff,
+	.name           = "Generic 10G PHY",
+	.soft_reset	= gen10g_soft_reset,
+	.config_init    = gen10g_config_init,
+	.features       = 0,
+	.config_aneg    = gen10g_config_aneg,
+	.read_status    = gen10g_read_status,
+	.suspend        = gen10g_suspend,
+	.resume         = gen10g_resume,
+};
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 6a79e24fa312..acf00f071c9a 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -69,13 +69,8 @@ static void phy_mdio_device_remove(struct mdio_device *mdiodev)
 	phy_device_remove(phydev);
 }
 
-enum genphy_driver {
-	GENPHY_DRV_1G,
-	GENPHY_DRV_10G,
-	GENPHY_DRV_MAX
-};
-
-static struct phy_driver genphy_driver[GENPHY_DRV_MAX];
+static struct phy_driver genphy_driver;
+extern struct phy_driver genphy_10g_driver;
 
 static LIST_HEAD(phy_fixup_list);
 static DEFINE_MUTEX(phy_fixup_lock);
@@ -928,11 +923,9 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	 */
 	if (!d->driver) {
 		if (phydev->is_c45)
-			d->driver =
-				&genphy_driver[GENPHY_DRV_10G].mdiodrv.driver;
+			d->driver = &genphy_10g_driver.mdiodrv.driver;
 		else
-			d->driver =
-				&genphy_driver[GENPHY_DRV_1G].mdiodrv.driver;
+			d->driver = &genphy_driver.mdiodrv.driver;
 
 		using_genphy = true;
 	}
@@ -1069,7 +1062,6 @@ void phy_detach(struct phy_device *phydev)
 	struct net_device *dev = phydev->attached_dev;
 	struct module *ndev_owner = dev->dev.parent->driver->owner;
 	struct mii_bus *bus;
-	int i;
 
 	if (phydev->sysfs_links) {
 		sysfs_remove_link(&dev->dev.kobj, "phydev");
@@ -1088,13 +1080,9 @@ void phy_detach(struct phy_device *phydev)
 	 * from the generic driver so that there's a chance a
 	 * real driver could be loaded
 	 */
-	for (i = 0; i < ARRAY_SIZE(genphy_driver); i++) {
-		if (phydev->mdio.dev.driver ==
-		    &genphy_driver[i].mdiodrv.driver) {
-			device_release_driver(&phydev->mdio.dev);
-			break;
-		}
-	}
+	if (phydev->mdio.dev.driver == &genphy_10g_driver.mdiodrv.driver ||
+	    phydev->mdio.dev.driver == &genphy_driver.mdiodrv.driver)
+		device_release_driver(&phydev->mdio.dev);
 
 	/*
 	 * The phydev might go away on the put_device() below, so avoid
@@ -1368,11 +1356,6 @@ int genphy_aneg_done(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(genphy_aneg_done);
 
-static int gen10g_config_aneg(struct phy_device *phydev)
-{
-	return 0;
-}
-
 /**
  * genphy_update_link - update link status in @phydev
  * @phydev: target phy_device struct
@@ -1506,25 +1489,6 @@ int genphy_read_status(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(genphy_read_status);
 
-static int gen10g_read_status(struct phy_device *phydev)
-{
-	u32 mmd_mask = phydev->c45_ids.devices_in_package;
-	int ret;
-
-	/* For now just lie and say it's 10G all the time */
-	phydev->speed = SPEED_10000;
-	phydev->duplex = DUPLEX_FULL;
-
-	/* Avoid reading the vendor MMDs */
-	mmd_mask &= ~(BIT(MDIO_MMD_VEND1) | BIT(MDIO_MMD_VEND2));
-
-	ret = genphy_c45_read_link(phydev, mmd_mask);
-
-	phydev->link = ret > 0 ? 1 : 0;
-
-	return 0;
-}
-
 /**
  * genphy_soft_reset - software reset the PHY via BMCR_RESET bit
  * @phydev: target phy_device struct
@@ -1590,21 +1554,6 @@ int genphy_config_init(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(genphy_config_init);
 
-static int gen10g_soft_reset(struct phy_device *phydev)
-{
-	/* Do nothing for now */
-	return 0;
-}
-
-static int gen10g_config_init(struct phy_device *phydev)
-{
-	/* Temporarily just say we support everything */
-	phydev->supported = SUPPORTED_10000baseT_Full;
-	phydev->advertising = SUPPORTED_10000baseT_Full;
-
-	return 0;
-}
-
 int genphy_suspend(struct phy_device *phydev)
 {
 	int value;
@@ -1620,11 +1569,6 @@ int genphy_suspend(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(genphy_suspend);
 
-static int gen10g_suspend(struct phy_device *phydev)
-{
-	return 0;
-}
-
 int genphy_resume(struct phy_device *phydev)
 {
 	int value;
@@ -1640,11 +1584,6 @@ int genphy_resume(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(genphy_resume);
 
-static int gen10g_resume(struct phy_device *phydev)
-{
-	return 0;
-}
-
 static int __set_phy_supported(struct phy_device *phydev, u32 max_speed)
 {
 	/* The default values for phydev->supported are provided by the PHY
@@ -1876,8 +1815,7 @@ void phy_drivers_unregister(struct phy_driver *drv, int n)
 }
 EXPORT_SYMBOL(phy_drivers_unregister);
 
-static struct phy_driver genphy_driver[] = {
-{
+static struct phy_driver genphy_driver = {
 	.phy_id		= 0xffffffff,
 	.phy_id_mask	= 0xffffffff,
 	.name		= "Generic PHY",
@@ -1891,18 +1829,7 @@ static struct phy_driver genphy_driver[] = {
 	.read_status	= genphy_read_status,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
-}, {
-	.phy_id         = 0xffffffff,
-	.phy_id_mask    = 0xffffffff,
-	.name           = "Generic 10G PHY",
-	.soft_reset	= gen10g_soft_reset,
-	.config_init    = gen10g_config_init,
-	.features       = 0,
-	.config_aneg    = gen10g_config_aneg,
-	.read_status    = gen10g_read_status,
-	.suspend        = gen10g_suspend,
-	.resume         = gen10g_resume,
-} };
+};
 
 static int __init phy_init(void)
 {
@@ -1912,18 +1839,24 @@ static int __init phy_init(void)
 	if (rc)
 		return rc;
 
-	rc = phy_drivers_register(genphy_driver,
-				  ARRAY_SIZE(genphy_driver), THIS_MODULE);
+	rc = phy_driver_register(&genphy_10g_driver, THIS_MODULE);
 	if (rc)
+		goto err_10g;
+
+	rc = phy_driver_register(&genphy_driver, THIS_MODULE);
+	if (rc) {
+		phy_driver_unregister(&genphy_10g_driver);
+err_10g:
 		mdio_bus_exit();
+	}
 
 	return rc;
 }
 
 static void __exit phy_exit(void)
 {
-	phy_drivers_unregister(genphy_driver,
-			       ARRAY_SIZE(genphy_driver));
+	phy_driver_unregister(&genphy_10g_driver);
+	phy_driver_unregister(&genphy_driver);
 	mdio_bus_exit();
 }
 
-- 
2.7.4

[PATCH 5/6] net: phy: add XAUI and 10GBASE-KR PHY connection types

[Russell King]

XAUI allows XGMII to reach an extended distance by using a XGXS layer at
each end of the MAC to PHY link, operating over four Serdes lanes.

10GBASE-KR is a single lane Serdes backplane ethernet connection method
with autonegotiation on the link.  Some PHYs use this to connect to the
ethernet interface at 10G speeds, switching to other connection types
when utilising slower speeds.

10GBASE-KR is also used for XFI and SFI to connect to XFP and SFP fiber
modules.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 Documentation/ABI/testing/sysfs-class-net-phydev   | 2 +-
 Documentation/devicetree/bindings/net/ethernet.txt | 2 ++
 include/linux/phy.h                                | 7 +++++++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-class-net-phydev b/Documentation/ABI/testing/sysfs-class-net-phydev
index c768d5fd8496..6ebabfb27912 100644
--- a/Documentation/ABI/testing/sysfs-class-net-phydev
+++ b/Documentation/ABI/testing/sysfs-class-net-phydev
@@ -32,5 +32,5 @@ Contact:	netdev@vger.kernel.org
 		<empty> (not available), mii, gmii, sgmii, tbi, rev-mii,
 		rmii, rgmii, rgmii-id, rgmii-rxid, rgmii-txid, rtbi, smii
 		xgmii, moca, qsgmii, trgmii, 1000base-x, 2500base-x, rxaui,
-		unknown
+		xaui, 10gbase-kr, unknown
 
diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt
index 3a6916909d90..d4abe9a98109 100644
--- a/Documentation/devicetree/bindings/net/ethernet.txt
+++ b/Documentation/devicetree/bindings/net/ethernet.txt
@@ -32,6 +32,8 @@
   * "2000base-x",
   * "2500base-x",
   * "rxaui"
+  * "xaui"
+  * "10gbase-kr" (10GBASE-KR, XFI, SFI)
 - phy-connection-type: the same as "phy-mode" property but described in ePAPR;
 - phy-handle: phandle, specifies a reference to a node representing a PHY
   device; this property is described in ePAPR and so preferred;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e6fcee188c8d..37feb502e1ae 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -83,6 +83,9 @@ typedef enum {
 	PHY_INTERFACE_MODE_1000BASEX,
 	PHY_INTERFACE_MODE_2500BASEX,
 	PHY_INTERFACE_MODE_RXAUI,
+	PHY_INTERFACE_MODE_XAUI,
+	/* 10GBASE-KR, XFI, SFI - single lane 10G Serdes */
+	PHY_INTERFACE_MODE_10GKR,
 	PHY_INTERFACE_MODE_MAX,
 } phy_interface_t;
 
@@ -149,6 +152,10 @@ static inline const char *phy_modes(phy_interface_t interface)
 		return "2500base-x";
 	case PHY_INTERFACE_MODE_RXAUI:
 		return "rxaui";
+	case PHY_INTERFACE_MODE_XAUI:
+		return "xaui";
+	case PHY_INTERFACE_MODE_10GKR:
+		return "10gbase-kr";
 	default:
 		return "unknown";
 	}
-- 
2.7.4

Re: [PATCH 5/6] net: phy: add XAUI and 10GBASE-KR PHY connection types

[Andrew Lunn]

On Mon, Jun 05, 2017 at 12:23:10PM +0100, Russell King wrote:
> XAUI allows XGMII to reach an extended distance by using a XGXS layer at
> each end of the MAC to PHY link, operating over four Serdes lanes.
> 
> 10GBASE-KR is a single lane Serdes backplane ethernet connection method
> with autonegotiation on the link.  Some PHYs use this to connect to the
> ethernet interface at 10G speeds, switching to other connection types
> when utilising slower speeds.
> 
> 10GBASE-KR is also used for XFI and SFI to connect to XFP and SFP fiber
> modules.
> 
> Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>

Reviewed-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>

    Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] net: phy: add XAUI and 10GBASE-KR PHY connection types

[Florian Fainelli]

On 06/05/2017 04:23 AM, Russell King wrote:
> XAUI allows XGMII to reach an extended distance by using a XGXS layer at
> each end of the MAC to PHY link, operating over four Serdes lanes.
> 
> 10GBASE-KR is a single lane Serdes backplane ethernet connection method
> with autonegotiation on the link.  Some PHYs use this to connect to the
> ethernet interface at 10G speeds, switching to other connection types
> when utilising slower speeds.
> 
> 10GBASE-KR is also used for XFI and SFI to connect to XFP and SFP fiber
> modules.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

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

[PATCH 6/6] net: phy: add Marvell Alaska X 88X3310 10Gigabit PHY support

[Russell King]

Add phylib support for the Marvell Alaska X 10 Gigabit PHY (MV88X3310).
This phy is able to operate at 10G, 1G, 100M and 10M speeds, and only
supports Clause 45 accesses.

The PHY appears (based on the vendor IDs) to be two different vendors
IP, with each devad containing several instances.

This PHY driver has only been tested with the RJ45 copper port, fiber
port and a Marvell Armada 8040-based ethernet interface.

It should be noted that to use the full range of speeds, MAC drivers
need to also reconfigure the link mode as per phydev->interface, since
the PHY automatically changes its interface mode depending on the
negotiated speed.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 MAINTAINERS                  |   6 +
 drivers/net/phy/Kconfig      |   5 +
 drivers/net/phy/Makefile     |   1 +
 drivers/net/phy/marvell10g.c | 368 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 380 insertions(+)
 create mode 100644 drivers/net/phy/marvell10g.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6b7625ff9875..3cf8b0a22019 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7979,6 +7979,12 @@ S:	Maintained
 F:	drivers/net/ethernet/marvell/mv643xx_eth.*
 F:	include/linux/mv643xx.h
 
+MARVELL MV88X3310 PHY DRIVER
+M:	Russell King <rmk@armlinux.org.uk>
+L:	netdev@vger.kernel.org
+S:	Maintained
+F:	drivers/net/phy/marvell10g.c
+
 MARVELL MVNETA ETHERNET DRIVER
 M:	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
 L:	netdev@vger.kernel.org
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 0c516d3229d0..65af31f24f01 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -292,6 +292,11 @@ config MARVELL_PHY
 	---help---
 	  Currently has a driver for the 88E1011S
 
+config MARVELL_10G_PHY
+	tristate "Marvell Alaska 10Gbit PHYs"
+	---help---
+	  Support for the Marvell Alaska MV88X3310 and compatible PHYs.
+
 config MESON_GXL_PHY
 	tristate "Amlogic Meson GXL Internal PHY"
 	depends on ARCH_MESON || COMPILE_TEST
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index ae58f507aba9..8e9b9f349384 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_INTEL_XWAY_PHY)	+= intel-xway.o
 obj-$(CONFIG_LSI_ET1011C_PHY)	+= et1011c.o
 obj-$(CONFIG_LXT_PHY)		+= lxt.o
 obj-$(CONFIG_MARVELL_PHY)	+= marvell.o
+obj-$(CONFIG_MARVELL_10G_PHY)	+= marvell10g.o
 obj-$(CONFIG_MESON_GXL_PHY)	+= meson-gxl.o
 obj-$(CONFIG_MICREL_KS8995MA)	+= spi_ks8995.o
 obj-$(CONFIG_MICREL_PHY)	+= micrel.o
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
new file mode 100644
index 000000000000..aebc08beceba
--- /dev/null
+++ b/drivers/net/phy/marvell10g.c
@@ -0,0 +1,368 @@
+/*
+ * Marvell 10G 88x3310 PHY driver
+ *
+ * Based upon the ID registers, this PHY appears to be a mixture of IPs
+ * from two different companies.
+ *
+ * There appears to be several different data paths through the PHY which
+ * are automatically managed by the PHY.  The following has been determined
+ * via observation and experimentation:
+ *
+ *       SGMII PHYXS -- BASE-T PCS -- 10G PMA -- AN -- Copper (for <= 1G)
+ *  10GBASE-KR PHYXS -- BASE-T PCS -- 10G PMA -- AN -- Copper (for 10G)
+ *  10GBASE-KR PHYXS -- BASE-R PCS -- Fiber
+ *
+ * If both the fiber and copper ports are connected, the first to gain
+ * link takes priority and the other port is completely locked out.
+ */
+#include <linux/phy.h>
+
+enum {
+	MV_PCS_BASE_T		= 0x0000,
+	MV_PCS_BASE_R		= 0x1000,
+	MV_PCS_1000BASEX	= 0x2000,
+
+	/* These registers appear at 0x800X and 0xa00X - the 0xa00X control
+	 * registers appear to set themselves to the 0x800X when AN is
+	 * restarted, but status registers appear readable from either.
+	 */
+	MV_AN_CTRL1000		= 0x8000, /* 1000base-T control register */
+	MV_AN_STAT1000		= 0x8001, /* 1000base-T status register */
+
+	/* This register appears to reflect the copper status */
+	MV_AN_RESULT		= 0xa016,
+	MV_AN_RESULT_SPD_10	= BIT(12),
+	MV_AN_RESULT_SPD_100	= BIT(13),
+	MV_AN_RESULT_SPD_1000	= BIT(14),
+	MV_AN_RESULT_SPD_10000	= BIT(15),
+};
+
+static int mv3310_modify(struct phy_device *phydev, int devad, u16 reg,
+			 u16 mask, u16 bits)
+{
+	int old, val, ret;
+
+	old = phy_read_mmd(phydev, devad, reg);
+	if (old < 0)
+		return old;
+
+	val = (old & ~mask) | (bits & mask);
+	if (val == old)
+		return 0;
+
+	ret = phy_write_mmd(phydev, devad, reg, val);
+
+	return ret < 0 ? ret : 1;
+}
+
+static int mv3310_probe(struct phy_device *phydev)
+{
+	u32 mmd_mask = MDIO_DEVS_PMAPMD | MDIO_DEVS_AN;
+
+	if (!phydev->is_c45 ||
+	    (phydev->c45_ids.devices_in_package & mmd_mask) != mmd_mask)
+		return -ENODEV;
+
+	return 0;
+}
+
+/*
+ * Resetting the MV88X3310 causes it to become non-responsive.  Avoid
+ * setting the reset bit(s).
+ */
+static int mv3310_soft_reset(struct phy_device *phydev)
+{
+	return 0;
+}
+
+static int mv3310_config_init(struct phy_device *phydev)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
+	u32 mask;
+	int val;
+
+	/* Check that the PHY interface type is compatible */
+	if (phydev->interface != PHY_INTERFACE_MODE_SGMII &&
+	    phydev->interface != PHY_INTERFACE_MODE_XGMII &&
+	    phydev->interface != PHY_INTERFACE_MODE_XAUI &&
+	    phydev->interface != PHY_INTERFACE_MODE_RXAUI &&
+	    phydev->interface != PHY_INTERFACE_MODE_10GKR)
+		return -ENODEV;
+
+	__set_bit(ETHTOOL_LINK_MODE_Pause_BIT, supported);
+	__set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, supported);
+
+	if (phydev->c45_ids.devices_in_package & MDIO_DEVS_AN) {
+		val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
+		if (val < 0)
+			return val;
+
+		if (val & MDIO_AN_STAT1_ABLE)
+			__set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, supported);
+	}
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_STAT2);
+	if (val < 0)
+		return val;
+
+	/* Ethtool does not support the WAN mode bits */
+	if (val & (MDIO_PMA_STAT2_10GBSR | MDIO_PMA_STAT2_10GBLR |
+		   MDIO_PMA_STAT2_10GBER | MDIO_PMA_STAT2_10GBLX4 |
+		   MDIO_PMA_STAT2_10GBSW | MDIO_PMA_STAT2_10GBLW |
+		   MDIO_PMA_STAT2_10GBEW))
+		__set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, supported);
+	if (val & MDIO_PMA_STAT2_10GBSR)
+		__set_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT, supported);
+	if (val & MDIO_PMA_STAT2_10GBLR)
+		__set_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT, supported);
+	if (val & MDIO_PMA_STAT2_10GBER)
+		__set_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT, supported);
+
+	if (val & MDIO_PMA_STAT2_EXTABLE) {
+		val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_EXTABLE);
+		if (val < 0)
+			return val;
+
+		if (val & (MDIO_PMA_EXTABLE_10GBT | MDIO_PMA_EXTABLE_1000BT |
+			   MDIO_PMA_EXTABLE_100BTX | MDIO_PMA_EXTABLE_10BT))
+			__set_bit(ETHTOOL_LINK_MODE_TP_BIT, supported);
+		if (val & MDIO_PMA_EXTABLE_10GBLRM)
+			__set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, supported);
+		if (val & (MDIO_PMA_EXTABLE_10GBKX4 | MDIO_PMA_EXTABLE_10GBKR |
+			   MDIO_PMA_EXTABLE_1000BKX))
+			__set_bit(ETHTOOL_LINK_MODE_Backplane_BIT, supported);
+		if (val & MDIO_PMA_EXTABLE_10GBLRM)
+			__set_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT,
+				  supported);
+		if (val & MDIO_PMA_EXTABLE_10GBT)
+			__set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+				  supported);
+		if (val & MDIO_PMA_EXTABLE_10GBKX4)
+			__set_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+				  supported);
+		if (val & MDIO_PMA_EXTABLE_10GBKR)
+			__set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+				  supported);
+		if (val & MDIO_PMA_EXTABLE_1000BT)
+			__set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+				  supported);
+		if (val & MDIO_PMA_EXTABLE_1000BKX)
+			__set_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+				  supported);
+		if (val & MDIO_PMA_EXTABLE_100BTX)
+			__set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+				  supported);
+		if (val & MDIO_PMA_EXTABLE_10BT)
+			__set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+				  supported);
+	}
+
+	if (!ethtool_convert_link_mode_to_legacy_u32(&mask, supported))
+		dev_warn(&phydev->mdio.dev,
+			 "PHY supports (%*pb) more modes than phylib supports, some modes not supported.\n",
+			 __ETHTOOL_LINK_MODE_MASK_NBITS, supported);
+
+	phydev->supported &= mask;
+	phydev->advertising &= phydev->supported;
+
+	return 0;
+}
+
+static int mv3310_config_aneg(struct phy_device *phydev)
+{
+	bool changed = false;
+	u32 advertising;
+	int ret;
+
+	if (phydev->autoneg == AUTONEG_DISABLE) {
+		ret = genphy_c45_pma_setup_forced(phydev);
+		if (ret < 0)
+			return ret;
+
+		return genphy_c45_an_disable_aneg(phydev);
+	}
+
+	phydev->advertising &= phydev->supported;
+	advertising = phydev->advertising;
+
+	ret = mv3310_modify(phydev, MDIO_MMD_AN, MDIO_AN_ADVERTISE,
+			    ADVERTISE_ALL | ADVERTISE_100BASE4 |
+			    ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM,
+			    ethtool_adv_to_mii_adv_t(advertising));
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		changed = true;
+
+	ret = mv3310_modify(phydev, MDIO_MMD_AN, MV_AN_CTRL1000,
+			    ADVERTISE_1000FULL | ADVERTISE_1000HALF,
+			    ethtool_adv_to_mii_ctrl1000_t(advertising));
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		changed = true;
+
+	/* 10G control register */
+	ret = mv3310_modify(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_CTRL,
+			    MDIO_AN_10GBT_CTRL_ADV10G,
+			    advertising & ADVERTISED_10000baseT_Full ?
+				MDIO_AN_10GBT_CTRL_ADV10G : 0);
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		changed = true;
+
+	if (changed)
+		ret = genphy_c45_restart_aneg(phydev);
+
+	return ret;
+}
+
+static int mv3310_aneg_done(struct phy_device *phydev)
+{
+	int val;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_BASE_R + MDIO_STAT1);
+	if (val < 0)
+		return val;
+
+	if (val & MDIO_STAT1_LSTATUS)
+		return 1;
+
+	return genphy_c45_aneg_done(phydev);
+}
+
+/* 10GBASE-ER,LR,LRM,SR do not support autonegotiation. */
+static int mv3310_read_10gbr_status(struct phy_device *phydev)
+{
+	phydev->link = 1;
+	phydev->speed = SPEED_10000;
+	phydev->duplex = DUPLEX_FULL;
+
+	if (phydev->interface == PHY_INTERFACE_MODE_SGMII)
+		phydev->interface = PHY_INTERFACE_MODE_10GKR;
+
+	return 0;
+}
+
+static int mv3310_read_status(struct phy_device *phydev)
+{
+	u32 mmd_mask = phydev->c45_ids.devices_in_package;
+	int val;
+
+	/* The vendor devads do not report link status.  Avoid the PHYXS
+	 * instance as there are three, and its status depends on the MAC
+	 * being appropriately configured for the negotiated speed.
+	 */
+	mmd_mask &= ~(BIT(MDIO_MMD_VEND1) | BIT(MDIO_MMD_VEND2) |
+		      BIT(MDIO_MMD_PHYXS));
+
+	phydev->speed = SPEED_UNKNOWN;
+	phydev->duplex = DUPLEX_UNKNOWN;
+	phydev->lp_advertising = 0;
+	phydev->link = 0;
+	phydev->pause = 0;
+	phydev->asym_pause = 0;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_BASE_R + MDIO_STAT1);
+	if (val < 0)
+		return val;
+
+	if (val & MDIO_STAT1_LSTATUS)
+		return mv3310_read_10gbr_status(phydev);
+
+	val = genphy_c45_read_link(phydev, mmd_mask);
+	if (val < 0)
+		return val;
+
+	phydev->link = val > 0 ? 1 : 0;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
+	if (val < 0)
+		return val;
+
+	if (val & MDIO_AN_STAT1_COMPLETE) {
+		val = genphy_c45_read_lpa(phydev);
+		if (val < 0)
+			return val;
+
+		/* Read the link partner's 1G advertisment */
+		val = phy_read_mmd(phydev, MDIO_MMD_AN, MV_AN_STAT1000);
+		if (val < 0)
+			return val;
+
+		phydev->lp_advertising |= mii_stat1000_to_ethtool_lpa_t(val);
+
+		if (phydev->autoneg == AUTONEG_ENABLE) {
+			val = phy_read_mmd(phydev, MDIO_MMD_AN, MV_AN_RESULT);
+			if (val < 0)
+				return val;
+
+			if (val & MV_AN_RESULT_SPD_10000)
+				phydev->speed = SPEED_10000;
+			else if (val & MV_AN_RESULT_SPD_1000)
+				phydev->speed = SPEED_1000;
+			else if (val & MV_AN_RESULT_SPD_100)
+				phydev->speed = SPEED_100;
+			else if (val & MV_AN_RESULT_SPD_10)
+				phydev->speed = SPEED_10;
+
+			phydev->duplex = DUPLEX_FULL;
+		}
+	}
+
+	if (phydev->autoneg != AUTONEG_ENABLE) {
+		val = genphy_c45_read_pma(phydev);
+		if (val < 0)
+			return val;
+	}
+
+	if ((phydev->interface == PHY_INTERFACE_MODE_SGMII ||
+	     phydev->interface == PHY_INTERFACE_MODE_10GKR) && phydev->link) {
+		/* The PHY automatically switches its serdes interface (and
+		 * active PHYXS instance) between Cisco SGMII and 10GBase-KR
+		 * modes according to the speed.  Florian suggests setting
+		 * phydev->interface to communicate this to the MAC. Only do
+		 * this if we are already in either SGMII or 10GBase-KR mode.
+		 */
+		if (phydev->speed == SPEED_10000)
+			phydev->interface = PHY_INTERFACE_MODE_10GKR;
+		else if (phydev->speed >= SPEED_10 &&
+			 phydev->speed < SPEED_10000)
+			phydev->interface = PHY_INTERFACE_MODE_SGMII;
+	}
+
+	return 0;
+}
+
+static struct phy_driver mv3310_drivers[] = {
+	{
+		.phy_id		= 0x002b09aa,
+		.phy_id_mask	= 0xffffffff,
+		.name		= "mv88x3310",
+		.features	= SUPPORTED_10baseT_Full |
+				  SUPPORTED_100baseT_Full |
+				  SUPPORTED_1000baseT_Full |
+				  SUPPORTED_Autoneg |
+				  SUPPORTED_TP |
+				  SUPPORTED_FIBRE |
+				  SUPPORTED_10000baseT_Full |
+				  SUPPORTED_Backplane,
+		.probe		= mv3310_probe,
+		.soft_reset	= mv3310_soft_reset,
+		.config_init	= mv3310_config_init,
+		.config_aneg	= mv3310_config_aneg,
+		.aneg_done	= mv3310_aneg_done,
+		.read_status	= mv3310_read_status,
+	},
+};
+
+module_phy_driver(mv3310_drivers);
+
+static struct mdio_device_id __maybe_unused mv3310_tbl[] = {
+	{ 0x002b09aa, 0xffffffff },
+	{ },
+};
+MODULE_DEVICE_TABLE(mdio, mv3310_tbl);
+MODULE_DESCRIPTION("Marvell Alaska X 10Gigabit Ethernet PHY driver (MV88X3310)");
+MODULE_LICENSE("GPL");
-- 
2.7.4

Re: [PATCH 6/6] net: phy: add Marvell Alaska X 88X3310 10Gigabit PHY support

[Andrew Lunn]

On Mon, Jun 05, 2017 at 12:23:16PM +0100, Russell King wrote:
> Add phylib support for the Marvell Alaska X 10 Gigabit PHY (MV88X3310).
> This phy is able to operate at 10G, 1G, 100M and 10M speeds, and only
> supports Clause 45 accesses.
> 
> The PHY appears (based on the vendor IDs) to be two different vendors
> IP, with each devad containing several instances.
> 
> This PHY driver has only been tested with the RJ45 copper port, fiber
> port and a Marvell Armada 8040-based ethernet interface.
> 
> It should be noted that to use the full range of speeds, MAC drivers
> need to also reconfigure the link mode as per phydev->interface, since
> the PHY automatically changes its interface mode depending on the
> negotiated speed.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH 6/6] net: phy: add Marvell Alaska X 88X3310 10Gigabit PHY support

[Florian Fainelli]

On 06/05/2017 04:23 AM, Russell King wrote:
> Add phylib support for the Marvell Alaska X 10 Gigabit PHY (MV88X3310).
> This phy is able to operate at 10G, 1G, 100M and 10M speeds, and only
> supports Clause 45 accesses.
> 
> The PHY appears (based on the vendor IDs) to be two different vendors
> IP, with each devad containing several instances.
> 
> This PHY driver has only been tested with the RJ45 copper port, fiber
> port and a Marvell Armada 8040-based ethernet interface.
> 
> It should be noted that to use the full range of speeds, MAC drivers
> need to also reconfigure the link mode as per phydev->interface, since
> the PHY automatically changes its interface mode depending on the
> negotiated speed.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

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

Re: [PATCH 6/6] net: phy: add Marvell Alaska X 88X3310 10Gigabit PHY support

[Andrew Lunn]

On Mon, Jun 05, 2017 at 12:23:16PM +0100, Russell King wrote:
> Add phylib support for the Marvell Alaska X 10 Gigabit PHY (MV88X3310).
> This phy is able to operate at 10G, 1G, 100M and 10M speeds, and only
> supports Clause 45 accesses.
> 
> The PHY appears (based on the vendor IDs) to be two different vendors
> IP, with each devad containing several instances.
> 
> This PHY driver has only been tested with the RJ45 copper port, fiber
> port and a Marvell Armada 8040-based ethernet interface.
> 
> It should be noted that to use the full range of speeds, MAC drivers
> need to also reconfigure the link mode as per phydev->interface, since
> the PHY automatically changes its interface mode depending on the
> negotiated speed.

Hi Russell

We are still missing documentation for the additions to adjust_link()
callback, but lets get this merged, and documentation can follow in a
separate patch.

	  Andrew

Re: [PATCH 6/6] net: phy: add Marvell Alaska X 88X3310 10Gigabit PHY support

[Russell King - ARM Linux]

On Mon, Jun 05, 2017 at 08:21:57PM +0200, Andrew Lunn wrote:
> On Mon, Jun 05, 2017 at 12:23:16PM +0100, Russell King wrote:
> > Add phylib support for the Marvell Alaska X 10 Gigabit PHY (MV88X3310).
> > This phy is able to operate at 10G, 1G, 100M and 10M speeds, and only
> > supports Clause 45 accesses.
> > 
> > The PHY appears (based on the vendor IDs) to be two different vendors
> > IP, with each devad containing several instances.
> > 
> > This PHY driver has only been tested with the RJ45 copper port, fiber
> > port and a Marvell Armada 8040-based ethernet interface.
> > 
> > It should be noted that to use the full range of speeds, MAC drivers
> > need to also reconfigure the link mode as per phydev->interface, since
> > the PHY automatically changes its interface mode depending on the
> > negotiated speed.
> 
> Hi Russell
> 
> We are still missing documentation for the additions to adjust_link()
> callback, but lets get this merged, and documentation can follow in a
> separate patch.

As I mentioned when you brought that up, are you asking me to document
the _entire_ adjust_link() callback, because _no_ documentation of it
currently exists.  The only thing I can find that documents this function
is Documentation/networking/phy.txt:

" First, you need a function to react to changes in the link state.  This
 function follows this protocol:

   static void adjust_link(struct net_device *dev);"

which tells people nothing about what's required from that function.



-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v2 0/6] Add phylib support for MV88X3310 10G phy

[David Miller]

From: Russell King - ARM Linux <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
Date: Mon, 5 Jun 2017 12:22:03 +0100

> This patch series adds support for the Marvell 88x3310 PHY found on
> the SolidRun Macchiatobin board.

Series applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html