[PATCH net-next 0/5] Initial SFP support patches

[PATCH net-next 0/5] Initial SFP support patches

[Russell King - ARM Linux]

Hi David,

Please review and merge this initial patch set, which is part of a
larger set previously posted adding SFP support to phy and mvneta.

This initial set are focused on cleaning up and reorganising the
fixed-phy code to allow the core software-phy code to be re-used.

These are based on net-next.

Thanks.

 drivers/net/phy/Kconfig     |   4 +
 drivers/net/phy/Makefile    |   3 +-
 drivers/net/phy/fixed_phy.c | 153 +++++++------------------------------
 drivers/net/phy/swphy.c     | 179 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/net/phy/swphy.h     |   9 +++
 5 files changed, 222 insertions(+), 126 deletions(-)

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH net-next 1/5] phy: move fixed_phy MII register generation to a library

[Russell King]

Move the fixed_phy MII register generation to a library to allow other
software phy implementations to use this code.

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/phy/Kconfig     |   4 ++
 drivers/net/phy/Makefile    |   3 +-
 drivers/net/phy/fixed_phy.c |  95 ++-------------------------------
 drivers/net/phy/swphy.c     | 126 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/net/phy/swphy.h     |   8 +++
 5 files changed, 143 insertions(+), 93 deletions(-)
 create mode 100644 drivers/net/phy/swphy.c
 create mode 100644 drivers/net/phy/swphy.h

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 8dac88abbc39..f96829415ce6 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -12,6 +12,9 @@ menuconfig PHYLIB
 
 if PHYLIB
 
+config SWPHY
+	bool
+
 comment "MII PHY device drivers"
 
 config AQUANTIA_PHY
@@ -159,6 +162,7 @@ config MICROCHIP_PHY
 config FIXED_PHY
 	tristate "Driver for MDIO Bus/PHY emulation with fixed speed/link PHYs"
 	depends on PHYLIB
+	select SWPHY
 	---help---
 	  Adds the platform "fixed" MDIO Bus to cover the boards that use
 	  PHYs that are not connected to the real MDIO bus.
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 4170642a2035..7158274327d0 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -1,6 +1,7 @@
 # Makefile for Linux PHY drivers
 
-libphy-objs			:= phy.o phy_device.o mdio_bus.o mdio_device.o
+libphy-y			:= phy.o phy_device.o mdio_bus.o mdio_device.o
+libphy-$(CONFIG_SWPHY)		+= swphy.o
 
 obj-$(CONFIG_PHYLIB)		+= libphy.o
 obj-$(CONFIG_AQUANTIA_PHY)	+= aquantia.o
diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 2d2e4339f0df..d98a0d90b5a5 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -24,6 +24,8 @@
 #include <linux/of.h>
 #include <linux/gpio.h>
 
+#include "swphy.h"
+
 #define MII_REGS_NUM 29
 
 struct fixed_mdio_bus {
@@ -48,101 +50,10 @@ static struct fixed_mdio_bus platform_fmb = {
 
 static int fixed_phy_update_regs(struct fixed_phy *fp)
 {
-	u16 bmsr = BMSR_ANEGCAPABLE;
-	u16 bmcr = 0;
-	u16 lpagb = 0;
-	u16 lpa = 0;
-
 	if (gpio_is_valid(fp->link_gpio))
 		fp->status.link = !!gpio_get_value_cansleep(fp->link_gpio);
 
-	if (fp->status.duplex) {
-		switch (fp->status.speed) {
-		case 1000:
-			bmsr |= BMSR_ESTATEN;
-			break;
-		case 100:
-			bmsr |= BMSR_100FULL;
-			break;
-		case 10:
-			bmsr |= BMSR_10FULL;
-			break;
-		default:
-			break;
-		}
-	} else {
-		switch (fp->status.speed) {
-		case 1000:
-			bmsr |= BMSR_ESTATEN;
-			break;
-		case 100:
-			bmsr |= BMSR_100HALF;
-			break;
-		case 10:
-			bmsr |= BMSR_10HALF;
-			break;
-		default:
-			break;
-		}
-	}
-
-	if (fp->status.link) {
-		bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
-
-		if (fp->status.duplex) {
-			bmcr |= BMCR_FULLDPLX;
-
-			switch (fp->status.speed) {
-			case 1000:
-				bmcr |= BMCR_SPEED1000;
-				lpagb |= LPA_1000FULL;
-				break;
-			case 100:
-				bmcr |= BMCR_SPEED100;
-				lpa |= LPA_100FULL;
-				break;
-			case 10:
-				lpa |= LPA_10FULL;
-				break;
-			default:
-				pr_warn("fixed phy: unknown speed\n");
-				return -EINVAL;
-			}
-		} else {
-			switch (fp->status.speed) {
-			case 1000:
-				bmcr |= BMCR_SPEED1000;
-				lpagb |= LPA_1000HALF;
-				break;
-			case 100:
-				bmcr |= BMCR_SPEED100;
-				lpa |= LPA_100HALF;
-				break;
-			case 10:
-				lpa |= LPA_10HALF;
-				break;
-			default:
-				pr_warn("fixed phy: unknown speed\n");
-			return -EINVAL;
-			}
-		}
-
-		if (fp->status.pause)
-			lpa |= LPA_PAUSE_CAP;
-
-		if (fp->status.asym_pause)
-			lpa |= LPA_PAUSE_ASYM;
-	}
-
-	fp->regs[MII_PHYSID1] = 0;
-	fp->regs[MII_PHYSID2] = 0;
-
-	fp->regs[MII_BMSR] = bmsr;
-	fp->regs[MII_BMCR] = bmcr;
-	fp->regs[MII_LPA] = lpa;
-	fp->regs[MII_STAT1000] = lpagb;
-
-	return 0;
+	return swphy_update_regs(fp->regs, &fp->status);
 }
 
 static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
diff --git a/drivers/net/phy/swphy.c b/drivers/net/phy/swphy.c
new file mode 100644
index 000000000000..0551a79a2454
--- /dev/null
+++ b/drivers/net/phy/swphy.c
@@ -0,0 +1,126 @@
+/*
+ * Software PHY emulation
+ *
+ * Code taken from fixed_phy.c by Russell King <rmk+kernel@arm.linux.org.uk>
+ *
+ * Author: Vitaly Bordug <vbordug@ru.mvista.com>
+ *         Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * Copyright (c) 2006-2007 MontaVista Software, Inc.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+#include <linux/export.h>
+#include <linux/mii.h>
+#include <linux/phy.h>
+#include <linux/phy_fixed.h>
+
+#include "swphy.h"
+
+/**
+ * swphy_update_regs - update MII register array with fixed phy state
+ * @regs: array of 32 registers to update
+ * @state: fixed phy status
+ *
+ * Update the array of MII registers with the fixed phy link, speed,
+ * duplex and pause mode settings.
+ */
+int swphy_update_regs(u16 *regs, const struct fixed_phy_status *state)
+{
+	u16 bmsr = BMSR_ANEGCAPABLE;
+	u16 bmcr = 0;
+	u16 lpagb = 0;
+	u16 lpa = 0;
+
+	if (state->duplex) {
+		switch (state->speed) {
+		case 1000:
+			bmsr |= BMSR_ESTATEN;
+			break;
+		case 100:
+			bmsr |= BMSR_100FULL;
+			break;
+		case 10:
+			bmsr |= BMSR_10FULL;
+			break;
+		default:
+			break;
+		}
+	} else {
+		switch (state->speed) {
+		case 1000:
+			bmsr |= BMSR_ESTATEN;
+			break;
+		case 100:
+			bmsr |= BMSR_100HALF;
+			break;
+		case 10:
+			bmsr |= BMSR_10HALF;
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (state->link) {
+		bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
+
+		if (state->duplex) {
+			bmcr |= BMCR_FULLDPLX;
+
+			switch (state->speed) {
+			case 1000:
+				bmcr |= BMCR_SPEED1000;
+				lpagb |= LPA_1000FULL;
+				break;
+			case 100:
+				bmcr |= BMCR_SPEED100;
+				lpa |= LPA_100FULL;
+				break;
+			case 10:
+				lpa |= LPA_10FULL;
+				break;
+			default:
+				pr_warn("swphy: unknown speed\n");
+				return -EINVAL;
+			}
+		} else {
+			switch (state->speed) {
+			case 1000:
+				bmcr |= BMCR_SPEED1000;
+				lpagb |= LPA_1000HALF;
+				break;
+			case 100:
+				bmcr |= BMCR_SPEED100;
+				lpa |= LPA_100HALF;
+				break;
+			case 10:
+				lpa |= LPA_10HALF;
+				break;
+			default:
+				pr_warn("swphy: unknown speed\n");
+				return -EINVAL;
+			}
+		}
+
+		if (state->pause)
+			lpa |= LPA_PAUSE_CAP;
+
+		if (state->asym_pause)
+			lpa |= LPA_PAUSE_ASYM;
+	}
+
+	regs[MII_PHYSID1] = 0;
+	regs[MII_PHYSID2] = 0;
+
+	regs[MII_BMSR] = bmsr;
+	regs[MII_BMCR] = bmcr;
+	regs[MII_LPA] = lpa;
+	regs[MII_STAT1000] = lpagb;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(swphy_update_regs);
diff --git a/drivers/net/phy/swphy.h b/drivers/net/phy/swphy.h
new file mode 100644
index 000000000000..feaa38ff86a2
--- /dev/null
+++ b/drivers/net/phy/swphy.h
@@ -0,0 +1,8 @@
+#ifndef SWPHY_H
+#define SWPHY_H
+
+struct fixed_phy_status;
+
+int swphy_update_regs(u16 *regs, const struct fixed_phy_status *state);
+
+#endif
-- 
2.1.0

[PATCH net-next 2/5] phy: convert swphy register generation to tabular form

[Russell King]

Convert the swphy register generation to tabular form which allows us
to eliminate multiple switch() statements.  This results in a smaller
object code size, more efficient, and easier to add support for faster
speeds.

Before:

Idx Name          Size      VMA       LMA       File off  Algn
  0 .text         00000164  00000000  00000000  00000034  2**2

   text    data     bss     dec     hex filename
    388       0       0     388     184 swphy.o

After:

Idx Name          Size      VMA       LMA       File off  Algn
  0 .text         000000fc  00000000  00000000  00000034  2**2
  5 .rodata       00000028  00000000  00000000  00000138  2**2

   text    data     bss     dec     hex filename
    324       0       0     324     144 swphy.o

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/phy/swphy.c | 143 ++++++++++++++++++++++++++----------------------
 1 file changed, 78 insertions(+), 65 deletions(-)

diff --git a/drivers/net/phy/swphy.c b/drivers/net/phy/swphy.c
index 0551a79a2454..c88a194b4cb6 100644
--- a/drivers/net/phy/swphy.c
+++ b/drivers/net/phy/swphy.c
@@ -20,6 +20,72 @@
 
 #include "swphy.h"
 
+struct swmii_regs {
+	u16 bmcr;
+	u16 bmsr;
+	u16 lpa;
+	u16 lpagb;
+};
+
+enum {
+	SWMII_SPEED_10 = 0,
+	SWMII_SPEED_100,
+	SWMII_SPEED_1000,
+	SWMII_DUPLEX_HALF = 0,
+	SWMII_DUPLEX_FULL,
+};
+
+/*
+ * These two tables get bitwise-anded together to produce the final result.
+ * This means the speed table must contain both duplex settings, and the
+ * duplex table must contain all speed settings.
+ */
+static const struct swmii_regs speed[] = {
+	[SWMII_SPEED_10] = {
+		.bmcr  = BMCR_FULLDPLX,
+		.lpa   = LPA_10FULL | LPA_10HALF,
+	},
+	[SWMII_SPEED_100] = {
+		.bmcr  = BMCR_FULLDPLX | BMCR_SPEED100,
+		.bmsr  = BMSR_100FULL | BMSR_100HALF,
+		.lpa   = LPA_100FULL | LPA_100HALF,
+	},
+	[SWMII_SPEED_1000] = {
+		.bmcr  = BMCR_FULLDPLX | BMCR_SPEED1000,
+		.bmsr  = BMSR_ESTATEN,
+		.lpagb = LPA_1000FULL | LPA_1000HALF,
+	},
+};
+
+static const struct swmii_regs duplex[] = {
+	[SWMII_DUPLEX_HALF] = {
+		.bmcr  = ~BMCR_FULLDPLX,
+		.bmsr  = BMSR_ESTATEN | BMSR_100HALF,
+		.lpa   = LPA_10HALF | LPA_100HALF,
+		.lpagb = LPA_1000HALF,
+	},
+	[SWMII_DUPLEX_FULL] = {
+		.bmcr  = ~0,
+		.bmsr  = BMSR_ESTATEN | BMSR_100FULL,
+		.lpa   = LPA_10FULL | LPA_100FULL,
+		.lpagb = LPA_1000FULL,
+	},
+};
+
+static int swphy_decode_speed(int speed)
+{
+	switch (speed) {
+	case 1000:
+		return SWMII_SPEED_1000;
+	case 100:
+		return SWMII_SPEED_100;
+	case 10:
+		return SWMII_SPEED_10;
+	default:
+		return -EINVAL;
+	}
+}
+
 /**
  * swphy_update_regs - update MII register array with fixed phy state
  * @regs: array of 32 registers to update
@@ -30,81 +96,28 @@
  */
 int swphy_update_regs(u16 *regs, const struct fixed_phy_status *state)
 {
+	int speed_index, duplex_index;
 	u16 bmsr = BMSR_ANEGCAPABLE;
 	u16 bmcr = 0;
 	u16 lpagb = 0;
 	u16 lpa = 0;
 
-	if (state->duplex) {
-		switch (state->speed) {
-		case 1000:
-			bmsr |= BMSR_ESTATEN;
-			break;
-		case 100:
-			bmsr |= BMSR_100FULL;
-			break;
-		case 10:
-			bmsr |= BMSR_10FULL;
-			break;
-		default:
-			break;
-		}
-	} else {
-		switch (state->speed) {
-		case 1000:
-			bmsr |= BMSR_ESTATEN;
-			break;
-		case 100:
-			bmsr |= BMSR_100HALF;
-			break;
-		case 10:
-			bmsr |= BMSR_10HALF;
-			break;
-		default:
-			break;
-		}
+	speed_index = swphy_decode_speed(state->speed);
+	if (speed_index < 0) {
+		pr_warn("swphy: unknown speed\n");
+		return -EINVAL;
 	}
 
+	duplex_index = state->duplex ? SWMII_DUPLEX_FULL : SWMII_DUPLEX_HALF;
+
+	bmsr |= speed[speed_index].bmsr & duplex[duplex_index].bmsr;
+
 	if (state->link) {
 		bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
 
-		if (state->duplex) {
-			bmcr |= BMCR_FULLDPLX;
-
-			switch (state->speed) {
-			case 1000:
-				bmcr |= BMCR_SPEED1000;
-				lpagb |= LPA_1000FULL;
-				break;
-			case 100:
-				bmcr |= BMCR_SPEED100;
-				lpa |= LPA_100FULL;
-				break;
-			case 10:
-				lpa |= LPA_10FULL;
-				break;
-			default:
-				pr_warn("swphy: unknown speed\n");
-				return -EINVAL;
-			}
-		} else {
-			switch (state->speed) {
-			case 1000:
-				bmcr |= BMCR_SPEED1000;
-				lpagb |= LPA_1000HALF;
-				break;
-			case 100:
-				bmcr |= BMCR_SPEED100;
-				lpa |= LPA_100HALF;
-				break;
-			case 10:
-				lpa |= LPA_10HALF;
-				break;
-			default:
-				pr_warn("swphy: unknown speed\n");
-				return -EINVAL;
-			}
-		}
+		bmcr  |= speed[speed_index].bmcr  & duplex[duplex_index].bmcr;
+		lpa   |= speed[speed_index].lpa   & duplex[duplex_index].lpa;
+		lpagb |= speed[speed_index].lpagb & duplex[duplex_index].lpagb;
 
 		if (state->pause)
 			lpa |= LPA_PAUSE_CAP;
-- 
2.1.0

[PATCH net-next 3/5] phy: separate swphy state validation from register generation

[Russell King]

Separate out the generation of MII registers from the state validation.
This allows us to simplify the error handing in fixed_phy() by allowing
earlier error detection.

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/phy/fixed_phy.c | 15 +++++++--------
 drivers/net/phy/swphy.c     | 33 ++++++++++++++++++++++++++-------
 drivers/net/phy/swphy.h     |  3 ++-
 3 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index d98a0d90b5a5..d84e30c46824 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -48,12 +48,12 @@ static struct fixed_mdio_bus platform_fmb = {
 	.phys = LIST_HEAD_INIT(platform_fmb.phys),
 };
 
-static int fixed_phy_update_regs(struct fixed_phy *fp)
+static void fixed_phy_update_regs(struct fixed_phy *fp)
 {
 	if (gpio_is_valid(fp->link_gpio))
 		fp->status.link = !!gpio_get_value_cansleep(fp->link_gpio);
 
-	return swphy_update_regs(fp->regs, &fp->status);
+	swphy_update_regs(fp->regs, &fp->status);
 }
 
 static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
@@ -160,6 +160,10 @@ int fixed_phy_add(unsigned int irq, int phy_addr,
 	struct fixed_mdio_bus *fmb = &platform_fmb;
 	struct fixed_phy *fp;
 
+	ret = swphy_validate_state(status);
+	if (ret < 0)
+		return ret;
+
 	fp = kzalloc(sizeof(*fp), GFP_KERNEL);
 	if (!fp)
 		return -ENOMEM;
@@ -180,17 +184,12 @@ int fixed_phy_add(unsigned int irq, int phy_addr,
 			goto err_regs;
 	}
 
-	ret = fixed_phy_update_regs(fp);
-	if (ret)
-		goto err_gpio;
+	fixed_phy_update_regs(fp);
 
 	list_add_tail(&fp->node, &fmb->phys);
 
 	return 0;
 
-err_gpio:
-	if (gpio_is_valid(fp->link_gpio))
-		gpio_free(fp->link_gpio);
 err_regs:
 	kfree(fp);
 	return ret;
diff --git a/drivers/net/phy/swphy.c b/drivers/net/phy/swphy.c
index c88a194b4cb6..21a9bd8a7830 100644
--- a/drivers/net/phy/swphy.c
+++ b/drivers/net/phy/swphy.c
@@ -87,6 +87,29 @@ static int swphy_decode_speed(int speed)
 }
 
 /**
+ * swphy_validate_state - validate the software phy status
+ * @state: software phy status
+ *
+ * This checks that we can represent the state stored in @state can be
+ * represented in the emulated MII registers.  Returns 0 if it can,
+ * otherwise returns -EINVAL.
+ */
+int swphy_validate_state(const struct fixed_phy_status *state)
+{
+	int err;
+
+	if (state->link) {
+		err = swphy_decode_speed(state->speed);
+		if (err < 0) {
+			pr_warn("swphy: unknown speed\n");
+			return -EINVAL;
+		}
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(swphy_validate_state);
+
+/**
  * swphy_update_regs - update MII register array with fixed phy state
  * @regs: array of 32 registers to update
  * @state: fixed phy status
@@ -94,7 +117,7 @@ static int swphy_decode_speed(int speed)
  * Update the array of MII registers with the fixed phy link, speed,
  * duplex and pause mode settings.
  */
-int swphy_update_regs(u16 *regs, const struct fixed_phy_status *state)
+void swphy_update_regs(u16 *regs, const struct fixed_phy_status *state)
 {
 	int speed_index, duplex_index;
 	u16 bmsr = BMSR_ANEGCAPABLE;
@@ -103,10 +126,8 @@ int swphy_update_regs(u16 *regs, const struct fixed_phy_status *state)
 	u16 lpa = 0;
 
 	speed_index = swphy_decode_speed(state->speed);
-	if (speed_index < 0) {
-		pr_warn("swphy: unknown speed\n");
-		return -EINVAL;
-	}
+	if (WARN_ON(speed_index < 0))
+		return;
 
 	duplex_index = state->duplex ? SWMII_DUPLEX_FULL : SWMII_DUPLEX_HALF;
 
@@ -133,7 +154,5 @@ int swphy_update_regs(u16 *regs, const struct fixed_phy_status *state)
 	regs[MII_BMCR] = bmcr;
 	regs[MII_LPA] = lpa;
 	regs[MII_STAT1000] = lpagb;
-
-	return 0;
 }
 EXPORT_SYMBOL_GPL(swphy_update_regs);
diff --git a/drivers/net/phy/swphy.h b/drivers/net/phy/swphy.h
index feaa38ff86a2..33d2e061896e 100644
--- a/drivers/net/phy/swphy.h
+++ b/drivers/net/phy/swphy.h
@@ -3,6 +3,7 @@
 
 struct fixed_phy_status;
 
-int swphy_update_regs(u16 *regs, const struct fixed_phy_status *state);
+int swphy_validate_state(const struct fixed_phy_status *state);
+void swphy_update_regs(u16 *regs, const struct fixed_phy_status *state);
 
 #endif
-- 
2.1.0

[PATCH net-next 4/5] phy: generate swphy registers on the fly

[Russell King]

Generate software phy registers as and when requested, rather than
duplicating the state in fixed_phy.  This allows us to eliminate
the duplicate storage of of the same data, which is only different
in format.

As fixed_phy_update_regs() no longer updates register state, rename
it to fixed_phy_update().

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/phy/fixed_phy.c | 31 +++++-------------------------
 drivers/net/phy/swphy.c     | 47 ++++++++++++++++++++++++++++++++-------------
 drivers/net/phy/swphy.h     |  2 +-
 3 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index d84e30c46824..0dfed86bdb5a 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -26,8 +26,6 @@
 
 #include "swphy.h"
 
-#define MII_REGS_NUM 29
-
 struct fixed_mdio_bus {
 	struct mii_bus *mii_bus;
 	struct list_head phys;
@@ -35,7 +33,6 @@ struct fixed_mdio_bus {
 
 struct fixed_phy {
 	int addr;
-	u16 regs[MII_REGS_NUM];
 	struct phy_device *phydev;
 	struct fixed_phy_status status;
 	int (*link_update)(struct net_device *, struct fixed_phy_status *);
@@ -48,12 +45,10 @@ static struct fixed_mdio_bus platform_fmb = {
 	.phys = LIST_HEAD_INIT(platform_fmb.phys),
 };
 
-static void fixed_phy_update_regs(struct fixed_phy *fp)
+static void fixed_phy_update(struct fixed_phy *fp)
 {
 	if (gpio_is_valid(fp->link_gpio))
 		fp->status.link = !!gpio_get_value_cansleep(fp->link_gpio);
-
-	swphy_update_regs(fp->regs, &fp->status);
 }
 
 static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
@@ -61,29 +56,15 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
 	struct fixed_mdio_bus *fmb = bus->priv;
 	struct fixed_phy *fp;
 
-	if (reg_num >= MII_REGS_NUM)
-		return -1;
-
-	/* We do not support emulating Clause 45 over Clause 22 register reads
-	 * return an error instead of bogus data.
-	 */
-	switch (reg_num) {
-	case MII_MMD_CTRL:
-	case MII_MMD_DATA:
-		return -1;
-	default:
-		break;
-	}
-
 	list_for_each_entry(fp, &fmb->phys, node) {
 		if (fp->addr == phy_addr) {
 			/* Issue callback if user registered it. */
 			if (fp->link_update) {
 				fp->link_update(fp->phydev->attached_dev,
 						&fp->status);
-				fixed_phy_update_regs(fp);
+				fixed_phy_update(fp);
 			}
-			return fp->regs[reg_num];
+			return swphy_read_reg(reg_num, &fp->status);
 		}
 	}
 
@@ -143,7 +124,7 @@ int fixed_phy_update_state(struct phy_device *phydev,
 			_UPD(pause);
 			_UPD(asym_pause);
 #undef _UPD
-			fixed_phy_update_regs(fp);
+			fixed_phy_update(fp);
 			return 0;
 		}
 	}
@@ -168,8 +149,6 @@ int fixed_phy_add(unsigned int irq, int phy_addr,
 	if (!fp)
 		return -ENOMEM;
 
-	memset(fp->regs, 0xFF,  sizeof(fp->regs[0]) * MII_REGS_NUM);
-
 	if (irq != PHY_POLL)
 		fmb->mii_bus->irq[phy_addr] = irq;
 
@@ -184,7 +163,7 @@ int fixed_phy_add(unsigned int irq, int phy_addr,
 			goto err_regs;
 	}
 
-	fixed_phy_update_regs(fp);
+	fixed_phy_update(fp);
 
 	list_add_tail(&fp->node, &fmb->phys);
 
diff --git a/drivers/net/phy/swphy.c b/drivers/net/phy/swphy.c
index 21a9bd8a7830..34f58f2349e9 100644
--- a/drivers/net/phy/swphy.c
+++ b/drivers/net/phy/swphy.c
@@ -20,6 +20,8 @@
 
 #include "swphy.h"
 
+#define MII_REGS_NUM 29
+
 struct swmii_regs {
 	u16 bmcr;
 	u16 bmsr;
@@ -110,14 +112,13 @@ int swphy_validate_state(const struct fixed_phy_status *state)
 EXPORT_SYMBOL_GPL(swphy_validate_state);
 
 /**
- * swphy_update_regs - update MII register array with fixed phy state
- * @regs: array of 32 registers to update
+ * swphy_read_reg - return a MII register from the fixed phy state
+ * @reg: MII register
  * @state: fixed phy status
  *
- * Update the array of MII registers with the fixed phy link, speed,
- * duplex and pause mode settings.
+ * Return the MII @reg register generated from the fixed phy state @state.
  */
-void swphy_update_regs(u16 *regs, const struct fixed_phy_status *state)
+int swphy_read_reg(int reg, const struct fixed_phy_status *state)
 {
 	int speed_index, duplex_index;
 	u16 bmsr = BMSR_ANEGCAPABLE;
@@ -125,9 +126,12 @@ void swphy_update_regs(u16 *regs, const struct fixed_phy_status *state)
 	u16 lpagb = 0;
 	u16 lpa = 0;
 
+	if (reg > MII_REGS_NUM)
+		return -1;
+
 	speed_index = swphy_decode_speed(state->speed);
 	if (WARN_ON(speed_index < 0))
-		return;
+		return 0;
 
 	duplex_index = state->duplex ? SWMII_DUPLEX_FULL : SWMII_DUPLEX_HALF;
 
@@ -147,12 +151,29 @@ void swphy_update_regs(u16 *regs, const struct fixed_phy_status *state)
 			lpa |= LPA_PAUSE_ASYM;
 	}
 
-	regs[MII_PHYSID1] = 0;
-	regs[MII_PHYSID2] = 0;
+	switch (reg) {
+	case MII_BMCR:
+		return bmcr;
+	case MII_BMSR:
+		return bmsr;
+	case MII_PHYSID1:
+	case MII_PHYSID2:
+		return 0;
+	case MII_LPA:
+		return lpa;
+	case MII_STAT1000:
+		return lpagb;
+
+	/*
+	 * We do not support emulating Clause 45 over Clause 22 register
+	 * reads.  Return an error instead of bogus data.
+	 */
+	case MII_MMD_CTRL:
+	case MII_MMD_DATA:
+		return -1;
 
-	regs[MII_BMSR] = bmsr;
-	regs[MII_BMCR] = bmcr;
-	regs[MII_LPA] = lpa;
-	regs[MII_STAT1000] = lpagb;
+	default:
+		return 0xffff;
+	}
 }
-EXPORT_SYMBOL_GPL(swphy_update_regs);
+EXPORT_SYMBOL_GPL(swphy_read_reg);
diff --git a/drivers/net/phy/swphy.h b/drivers/net/phy/swphy.h
index 33d2e061896e..2f09ac324e18 100644
--- a/drivers/net/phy/swphy.h
+++ b/drivers/net/phy/swphy.h
@@ -4,6 +4,6 @@
 struct fixed_phy_status;
 
 int swphy_validate_state(const struct fixed_phy_status *state);
-void swphy_update_regs(u16 *regs, const struct fixed_phy_status *state);
+int swphy_read_reg(int reg, const struct fixed_phy_status *state);
 
 #endif
-- 
2.1.0

[PATCH net-next 5/5] phy: improve safety of fixed-phy MII register reading

[Russell King]

There is no prevention of a concurrent call to both fixed_mdio_read()
and fixed_phy_update_state(), which can result in the state being
modified while it's being inspected.  Fix this by using a seqcount
to detect modifications, and memcpy()ing the state.

We remain slightly naughty here, calling link_update() and updating
the link status within the read-side loop - which would need rework
of the design to change.

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/phy/fixed_phy.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 0dfed86bdb5a..b376ada83598 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -23,6 +23,7 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/gpio.h>
+#include <linux/seqlock.h>
 
 #include "swphy.h"
 
@@ -34,6 +35,7 @@ struct fixed_mdio_bus {
 struct fixed_phy {
 	int addr;
 	struct phy_device *phydev;
+	seqcount_t seqcount;
 	struct fixed_phy_status status;
 	int (*link_update)(struct net_device *, struct fixed_phy_status *);
 	struct list_head node;
@@ -58,13 +60,21 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
 
 	list_for_each_entry(fp, &fmb->phys, node) {
 		if (fp->addr == phy_addr) {
-			/* Issue callback if user registered it. */
-			if (fp->link_update) {
-				fp->link_update(fp->phydev->attached_dev,
-						&fp->status);
-				fixed_phy_update(fp);
-			}
-			return swphy_read_reg(reg_num, &fp->status);
+			struct fixed_phy_status state;
+			int s;
+
+			do {
+				s = read_seqcount_begin(&fp->seqcount);
+				/* Issue callback if user registered it. */
+				if (fp->link_update) {
+					fp->link_update(fp->phydev->attached_dev,
+							&fp->status);
+					fixed_phy_update(fp);
+				}
+				state = fp->status;
+			} while (read_seqcount_retry(&fp->seqcount, s));
+
+			return swphy_read_reg(reg_num, &state);
 		}
 	}
 
@@ -116,6 +126,7 @@ int fixed_phy_update_state(struct phy_device *phydev,
 
 	list_for_each_entry(fp, &fmb->phys, node) {
 		if (fp->addr == phydev->mdio.addr) {
+			write_seqcount_begin(&fp->seqcount);
 #define _UPD(x) if (changed->x) \
 	fp->status.x = status->x
 			_UPD(link);
@@ -125,6 +136,7 @@ int fixed_phy_update_state(struct phy_device *phydev,
 			_UPD(asym_pause);
 #undef _UPD
 			fixed_phy_update(fp);
+			write_seqcount_end(&fp->seqcount);
 			return 0;
 		}
 	}
@@ -149,6 +161,8 @@ int fixed_phy_add(unsigned int irq, int phy_addr,
 	if (!fp)
 		return -ENOMEM;
 
+	seqcount_init(&fp->seqcount);
+
 	if (irq != PHY_POLL)
 		fmb->mii_bus->irq[phy_addr] = irq;
 
-- 
2.1.0

Re: [PATCH net-next 0/5] Initial SFP support patches

[David Miller]

From: Russell King - ARM Linux <linux@armlinux.org.uk>
Date: Thu, 23 Jun 2016 14:49:36 +0100

> Please review and merge this initial patch set, which is part of a
> larger set previously posted adding SFP support to phy and mvneta.
> 
> This initial set are focused on cleaning up and reorganising the
> fixed-phy code to allow the core software-phy code to be re-used.
> 
> These are based on net-next.

Series applied, thanks.