Skip to contentSkip to conversation

net: phy: add device-type abstraction

PATCH In review Johan Hovold 4 participants 14 messages Nov 19, 2014 – Nov 21, 2014

Subsystems (4)
Networking Network drivers Device tree bindings Ethernet PHY
lore
Share & tools
lore

Johan Hovold

author
#0 lore raw

cover net: phy: add device-type abstraction

This series adds device and device-type abstractions to the micrel driver, and enables support for RMII-reference clock selection for KSZ8081 and KSZ8091 devices.

While adding support for more features for the Micrel PHYs mentioned above, it became apparent that the configuration space is much too large and that adding type-specific callbacks will simply not scale. Instead I added a driver_data field to struct phy_device, which can be used to store static device type data that can be parsed and acted on in generic driver callbacks. This allows a lot of duplicated code to be removed, and should make it much easier to add new features or deal with device-type quirks in the future.

The series has been tested on a dual KSZ8081 setup. Further testing on other Micrel PHYs would be much appreciated.

The recent commit a95a18afe4c8 ("phy/micrel: KSZ8031RNL RMII clock reconfiguration bug") currently prevents KSZ8031 PHYs from using the generic config-init. Bruno, who is the author of that patch, has agreed to test this series and some follow-up diagnostic patches to determine how best to incorporate these devices as well. I intend to send a follow-up patch that removes the custom 8031 config-init and documents this quirk, but the current series can be applied meanwhile.

These patches are against net-next which contains some already merged prerequisite patches to the driver.

Johan

Johan Hovold (10):
net: phy: add static data field to struct phy_driver
net: phy: micrel: add device-type abstraction
net: phy: micrel: parse of nodes at probe
net: phy: micrel: add has-broadcast-disable flag to type data
net: phy: micrel: add generic clock-mode-select support
net: phy: micrel: add support for clock-mode select to KSZ8081/KSZ8091 dt/bindings: reformat micrel eth-phy documentation dt/bindings: add clock-select function property to micrel phy binding
net: phy: micrel: refactor interrupt config
net: phy: micrel: add copyright entry

 Documentation/devicetree/bindings/net/micrel.txt |  37 ++--
 drivers/net/phy/micrel.c                         | 262 ++++++++++++++---------
 include/linux/micrel_phy.h                       |   1 -
 include/linux/phy.h                              |   2 +
 4 files changed, 184 insertions(+), 118 deletions(-)
Signature · 1 line
2.0.4
author
#1 lore raw

PATCH 1/10 net: phy: add static data field to struct phy_driver

Add static driver-data field to struct phy_driver, which can be used to store structured device-type information.

Signed-off-by Johan Hovold <[email protected]>
---
 include/linux/phy.h | 2 ++
 1 file changed, 2 insertions(+)
include/linux/phy.h +2 -0
index 07794e720139..22af8f8f5802 100644
@@ -433,6 +433,7 @@ struct phy_device {
  *   by this PHY
  * flags: A bitfield defining certain other features this PHY
  *   supports (like interrupts)
+ * driver_data: static driver data
  *
  * The drivers must implement config_aneg and read_status.  All
  * other functions are optional. Note that none of these

@@ -448,6 +449,7 @@ struct phy_driver {
 	unsigned int phy_id_mask;
 	u32 features;
 	u32 flags;
+	const void *driver_data;
 
 	/*
 	 * Called to issue a PHY software reset
Signature · 1 line
2.0.4
author
#2 lore raw

PATCH 2/10 net: phy: micrel: add device-type abstraction

In reply to Johan Hovold

Add structured device-type information and support for generic led-mode setup to the generic config_init callback.

This is a first step in ultimately getting rid of device-type specific callbacks.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 83 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 70 insertions(+), 13 deletions(-)
drivers/net/phy/micrel.c +70 -13
index 30e894d6ffbd..1b528137afd9 100644
@@ -73,6 +73,30 @@
 
 #define PS_TO_REG				200
 
+struct kszphy_type {
+	u32 led_mode_reg;
+};
+
+struct kszphy_priv {
+	const struct kszphy_type *type;
+};
+
+static const struct kszphy_type ksz8021_type = {
+	.led_mode_reg		= MII_KSZPHY_CTRL_2,
+};
+
+static const struct kszphy_type ksz8041_type = {
+	.led_mode_reg		= MII_KSZPHY_CTRL_1,
+};
+
+static const struct kszphy_type ksz8051_type = {
+	.led_mode_reg		= MII_KSZPHY_CTRL_2,
+};
+
+static const struct kszphy_type ksz8081_type = {
+	.led_mode_reg		= MII_KSZPHY_CTRL_2,
+};
+
 static int ksz_config_flags(struct phy_device *phydev)
 {
 	int regval;

@@ -229,19 +253,25 @@ out:
 
 static int kszphy_config_init(struct phy_device *phydev)
 {
-	return 0;
-}
+	struct kszphy_priv *priv = phydev->priv;
+	const struct kszphy_type *type;
 
-static int kszphy_config_init_led8041(struct phy_device *phydev)
-{
-	return kszphy_setup_led(phydev, MII_KSZPHY_CTRL_1);
+	if (!priv)
+		return 0;
+
+	type = priv->type;
+
+	if (type->led_mode_reg)
+		kszphy_setup_led(phydev, type->led_mode_reg);
+
+	return 0;
 }
 
 static int ksz8021_config_init(struct phy_device *phydev)
 {
 	int rc;
 
-	kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
+	kszphy_config_init(phydev);
 
 	rc = ksz_config_flags(phydev);
 	if (rc < 0)

@@ -256,7 +286,7 @@ static int ks8051_config_init(struct phy_device *phydev)
 {
 	int rc;
 
-	kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
+	kszphy_config_init(phydev);
 
 	rc = ksz_config_flags(phydev);
 	return rc < 0 ? rc : 0;

@@ -265,9 +295,8 @@ static int ks8051_config_init(struct phy_device *phydev)
 static int ksz8081_config_init(struct phy_device *phydev)
 {
 	kszphy_broadcast_disable(phydev);
-	kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
 
-	return 0;
+	return kszphy_config_init(phydev);
 }
 
 static int ksz9021_load_values_from_of(struct phy_device *phydev,

@@ -499,6 +528,22 @@ ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
 {
 }
 
+static int kszphy_probe(struct phy_device *phydev)
+{
+	const struct kszphy_type *type = phydev->drv->driver_data;
+	struct kszphy_priv *priv;
+
+	priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	phydev->priv = priv;
+
+	priv->type = type;
+
+	return 0;
+}
+
 static int ksz8021_probe(struct phy_device *phydev)
 {
 	struct clk *clk;

@@ -517,7 +562,7 @@ static int ksz8021_probe(struct phy_device *phydev)
 		}
 	}
 
-	return 0;
+	return kszphy_probe(phydev);
 }
 
 static struct phy_driver ksphy_driver[] = {

@@ -542,6 +587,7 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause |
 			   SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.driver_data	= &ksz8021_type,
 	.probe		= ksz8021_probe,
 	.config_init	= ksz8021_config_init,
 	.config_aneg	= genphy_config_aneg,

@@ -558,6 +604,7 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause |
 			   SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.driver_data	= &ksz8021_type,
 	.probe		= ksz8021_probe,
 	.config_init	= ksz8021_config_init,
 	.config_aneg	= genphy_config_aneg,

@@ -574,7 +621,9 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause
 				| SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init_led8041,
+	.driver_data	= &ksz8041_type,
+	.probe		= kszphy_probe,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,

@@ -589,7 +638,9 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= PHY_BASIC_FEATURES |
 			  SUPPORTED_Pause | SUPPORTED_Asym_Pause,
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init_led8041,
+	.driver_data	= &ksz8041_type,
+	.probe		= kszphy_probe,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,

@@ -604,6 +655,8 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause
 				| SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.driver_data	= &ksz8051_type,
+	.probe		= kszphy_probe,
 	.config_init	= ks8051_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,

@@ -618,7 +671,9 @@ static struct phy_driver ksphy_driver[] = {
 	.phy_id_mask	= 0x00ffffff,
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init_led8041,
+	.driver_data	= &ksz8041_type,
+	.probe		= kszphy_probe,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,

@@ -632,6 +687,8 @@ static struct phy_driver ksphy_driver[] = {
 	.phy_id_mask	= 0x00fffff0,
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.driver_data	= &ksz8081_type,
+	.probe		= kszphy_probe,
 	.config_init	= ksz8081_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
Signature · 1 line
2.0.4
author
#3 lore raw

PATCH 3/10 net: phy: micrel: parse of nodes at probe

In reply to Johan Hovold

Parse the "micrel,led-mode" property at probe, rather than at config_init time in the led-setup helper itself.

Note that the bogus parent->of_node bit is removed.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)
drivers/net/phy/micrel.c +21 -18
index 1b528137afd9..6a81aaca5b1c 100644
@@ -79,6 +79,7 @@ struct kszphy_type {
 
 struct kszphy_priv {
 	const struct kszphy_type *type;
+	int led_mode;
 };
 
 static const struct kszphy_type ksz8021_type = {

@@ -186,24 +187,9 @@ static int ks8737_config_intr(struct phy_device *phydev)
 	return rc < 0 ? rc : 0;
 }
 
-static int kszphy_setup_led(struct phy_device *phydev, u32 reg)
+static int kszphy_setup_led(struct phy_device *phydev, u32 reg, int val)
 {
-
-	struct device *dev = &phydev->dev;
-	struct device_node *of_node = dev->of_node;
 	int rc, temp, shift;
-	u32 val;
-
-	if (!of_node && dev->parent->of_node)
-		of_node = dev->parent->of_node;
-
-	if (of_property_read_u32(of_node, "micrel,led-mode", &val))
-		return 0;
-
-	if (val > 3) {
-		dev_err(&phydev->dev, "invalid led mode: 0x%02x\n", val);
-		return -EINVAL;
-	}
 
 	switch (reg) {
 	case MII_KSZPHY_CTRL_1:

@@ -261,8 +247,8 @@ static int kszphy_config_init(struct phy_device *phydev)
 
 	type = priv->type;
 
-	if (type->led_mode_reg)
-		kszphy_setup_led(phydev, type->led_mode_reg);
+	if (priv->led_mode >= 0)
+		kszphy_setup_led(phydev, type->led_mode_reg, priv->led_mode);
 
 	return 0;
 }

@@ -531,7 +517,9 @@ ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
 static int kszphy_probe(struct phy_device *phydev)
 {
 	const struct kszphy_type *type = phydev->drv->driver_data;
+	struct device_node *np = phydev->dev.of_node;
 	struct kszphy_priv *priv;
+	int ret;
 
 	priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)

@@ -541,6 +529,21 @@ static int kszphy_probe(struct phy_device *phydev)
 
 	priv->type = type;
 
+	if (type->led_mode_reg) {
+		ret = of_property_read_u32(np, "micrel,led-mode",
+				&priv->led_mode);
+		if (ret)
+			priv->led_mode = -1;
+
+		if (priv->led_mode > 3) {
+			dev_err(&phydev->dev, "invalid led mode: 0x%02x\n",
+					priv->led_mode);
+			priv->led_mode = -1;
+		}
+	} else {
+		priv->led_mode = -1;
+	}
+
 	return 0;
 }
Signature · 1 line
2.0.4
author
#4 lore raw

PATCH 4/10 net: phy: micrel: add has-broadcast-disable flag to type data

In reply to Johan Hovold

Add has_broadcast_disable flag to type-data and generic config_init.

This allows us to remove the ksz8081 config_init callback.

Note that ksz8021_config_init is kept for now due to a95a18afe4c8 ("phy/micrel: KSZ8031RNL RMII clock reconfiguration bug").

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)
drivers/net/phy/micrel.c +6 -8
index 6a81aaca5b1c..a0e944099020 100644
@@ -75,6 +75,7 @@
 
 struct kszphy_type {
 	u32 led_mode_reg;
+	bool has_broadcast_disable;
 };
 
 struct kszphy_priv {

@@ -96,6 +97,7 @@ static const struct kszphy_type ksz8051_type = {
 
 static const struct kszphy_type ksz8081_type = {
 	.led_mode_reg		= MII_KSZPHY_CTRL_2,
+	.has_broadcast_disable	= true,
 };
 
 static int ksz_config_flags(struct phy_device *phydev)

@@ -247,6 +249,9 @@ static int kszphy_config_init(struct phy_device *phydev)
 
 	type = priv->type;
 
+	if (type->has_broadcast_disable)
+		kszphy_broadcast_disable(phydev);
+
 	if (priv->led_mode >= 0)
 		kszphy_setup_led(phydev, type->led_mode_reg, priv->led_mode);
 

@@ -278,13 +283,6 @@ static int ks8051_config_init(struct phy_device *phydev)
 	return rc < 0 ? rc : 0;
 }
 
-static int ksz8081_config_init(struct phy_device *phydev)
-{
-	kszphy_broadcast_disable(phydev);
-
-	return kszphy_config_init(phydev);
-}
-
 static int ksz9021_load_values_from_of(struct phy_device *phydev,
 				       struct device_node *of_node, u16 reg,
 				       char *field1, char *field2,

@@ -692,7 +690,7 @@ static struct phy_driver ksphy_driver[] = {
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.driver_data	= &ksz8081_type,
 	.probe		= kszphy_probe,
-	.config_init	= ksz8081_config_init,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
Signature · 1 line
2.0.4
author
#5 lore raw

PATCH 5/10 net: phy: micrel: add generic clock-mode-select support

In reply to Johan Hovold

Add generic RMII-Reference-Clock-Select support.

Several Micrel PHY have an RMII-Reference-Clock-Select bit to select 25 MHz or 50 MHz clock mode. Recently, support for configuring this through device tree for KSZ8021 and KSZ8031 was added.

Generalise this support so that it can be configured for other PHY types as well.

Note that some PHY revisions (of the same type) has this bit inverted. This should be either configurable through a new device-tree property, or preferably, determined based on PHY ID if possible.

Also note that this removes support for setting 25 MHz mode from board files which was also added by the above mentioned commit 45f56cb82e45 ("net/phy: micrel: Add clock support for KSZ8021/KSZ8031").

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c   | 93 +++++++++++++++++++++++++---------------------
 include/linux/micrel_phy.h |  1 -
 2 files changed, 50 insertions(+), 44 deletions(-)
drivers/net/phy/micrel.c +50 -43
index a0e944099020..d2e790cd3651 100644
@@ -57,7 +57,7 @@
 #define KSZPHY_CTRL_INT_ACTIVE_HIGH		BIT(9)
 #define KSZ9021_CTRL_INT_ACTIVE_HIGH		BIT(14)
 #define KS8737_CTRL_INT_ACTIVE_HIGH		BIT(14)
-#define KSZ8051_RMII_50MHZ_CLK			BIT(7)
+#define KSZPHY_RMII_REF_CLK_SEL			BIT(7)
 
 /* Write/read to/from extended registers */
 #define MII_KSZPHY_EXTREG                       0x0b

@@ -76,15 +76,19 @@
 struct kszphy_type {
 	u32 led_mode_reg;
 	bool has_broadcast_disable;
+	bool has_rmii_ref_clk_sel;
 };
 
 struct kszphy_priv {
 	const struct kszphy_type *type;
 	int led_mode;
+	bool rmii_ref_clk_sel;
+	bool rmii_ref_clk_sel_val;
 };
 
 static const struct kszphy_type ksz8021_type = {
 	.led_mode_reg		= MII_KSZPHY_CTRL_2,
+	.has_rmii_ref_clk_sel	= true,
 };
 
 static const struct kszphy_type ksz8041_type = {

@@ -100,21 +104,6 @@ static const struct kszphy_type ksz8081_type = {
 	.has_broadcast_disable	= true,
 };
 
-static int ksz_config_flags(struct phy_device *phydev)
-{
-	int regval;
-
-	if (phydev->dev_flags & (MICREL_PHY_50MHZ_CLK | MICREL_PHY_25MHZ_CLK)) {
-		regval = phy_read(phydev, MII_KSZPHY_CTRL);
-		if (phydev->dev_flags & MICREL_PHY_50MHZ_CLK)
-			regval |= KSZ8051_RMII_50MHZ_CLK;
-		else
-			regval &= ~KSZ8051_RMII_50MHZ_CLK;
-		return phy_write(phydev, MII_KSZPHY_CTRL, regval);
-	}
-	return 0;
-}
-
 static int kszphy_extended_write(struct phy_device *phydev,
 				u32 regnum, u16 val)
 {

@@ -189,6 +178,22 @@ static int ks8737_config_intr(struct phy_device *phydev)
 	return rc < 0 ? rc : 0;
 }
 
+static int kszphy_rmii_clk_sel(struct phy_device *phydev, bool val)
+{
+	int ctrl;
+
+	ctrl = phy_read(phydev, MII_KSZPHY_CTRL);
+	if (ctrl < 0)
+		return ctrl;
+
+	if (val)
+		ctrl |= KSZPHY_RMII_REF_CLK_SEL;
+	else
+		ctrl &= ~KSZPHY_RMII_REF_CLK_SEL;
+
+	return phy_write(phydev, MII_KSZPHY_CTRL, ctrl);
+}
+
 static int kszphy_setup_led(struct phy_device *phydev, u32 reg, int val)
 {
 	int rc, temp, shift;

@@ -243,6 +248,7 @@ static int kszphy_config_init(struct phy_device *phydev)
 {
 	struct kszphy_priv *priv = phydev->priv;
 	const struct kszphy_type *type;
+	int ret;
 
 	if (!priv)
 		return 0;

@@ -252,6 +258,14 @@ static int kszphy_config_init(struct phy_device *phydev)
 	if (type->has_broadcast_disable)
 		kszphy_broadcast_disable(phydev);
 
+	if (priv->rmii_ref_clk_sel) {
+		ret = kszphy_rmii_clk_sel(phydev, priv->rmii_ref_clk_sel_val);
+		if (ret) {
+			dev_err(&phydev->dev, "failed to set rmii reference clock\n");
+			return ret;
+		}
+	}
+
 	if (priv->led_mode >= 0)
 		kszphy_setup_led(phydev, type->led_mode_reg, priv->led_mode);
 

@@ -262,10 +276,8 @@ static int ksz8021_config_init(struct phy_device *phydev)
 {
 	int rc;
 
-	kszphy_config_init(phydev);
-
-	rc = ksz_config_flags(phydev);
-	if (rc < 0)
+	rc = kszphy_config_init(phydev);
+	if (rc)
 		return rc;
 
 	rc = kszphy_broadcast_disable(phydev);

@@ -273,16 +285,6 @@ static int ksz8021_config_init(struct phy_device *phydev)
 	return rc < 0 ? rc : 0;
 }
 
-static int ks8051_config_init(struct phy_device *phydev)
-{
-	int rc;
-
-	kszphy_config_init(phydev);
-
-	rc = ksz_config_flags(phydev);
-	return rc < 0 ? rc : 0;
-}
-
 static int ksz9021_load_values_from_of(struct phy_device *phydev,
 				       struct device_node *of_node, u16 reg,
 				       char *field1, char *field2,

@@ -517,6 +519,7 @@ static int kszphy_probe(struct phy_device *phydev)
 	const struct kszphy_type *type = phydev->drv->driver_data;
 	struct device_node *np = phydev->dev.of_node;
 	struct kszphy_priv *priv;
+	struct clk *clk;
 	int ret;
 
 	priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);

@@ -542,28 +545,32 @@ static int kszphy_probe(struct phy_device *phydev)
 		priv->led_mode = -1;
 	}
 
-	return 0;
-}
-
-static int ksz8021_probe(struct phy_device *phydev)
-{
-	struct clk *clk;
-
 	clk = devm_clk_get(&phydev->dev, "rmii-ref");
 	if (!IS_ERR(clk)) {
 		unsigned long rate = clk_get_rate(clk);
 
+		priv->rmii_ref_clk_sel = type->has_rmii_ref_clk_sel;
+
+		/* FIXME: add support for PHY revisions that have this bit
+		 * inverted (e.g. through new property or based on PHY ID).
+		 */
 		if (rate > 24500000 && rate < 25500000) {
-			phydev->dev_flags |= MICREL_PHY_25MHZ_CLK;
+			priv->rmii_ref_clk_sel_val = false;
 		} else if (rate > 49500000 && rate < 50500000) {
-			phydev->dev_flags |= MICREL_PHY_50MHZ_CLK;
+			priv->rmii_ref_clk_sel_val = true;
 		} else {
 			dev_err(&phydev->dev, "Clock rate out of range: %ld\n", rate);
 			return -EINVAL;
 		}
 	}
 
-	return kszphy_probe(phydev);
+	/* Support legacy board-file configuration */
+	if (phydev->dev_flags & MICREL_PHY_50MHZ_CLK) {
+		priv->rmii_ref_clk_sel = true;
+		priv->rmii_ref_clk_sel_val = true;
+	}
+
+	return 0;
 }
 
 static struct phy_driver ksphy_driver[] = {

@@ -589,7 +596,7 @@ static struct phy_driver ksphy_driver[] = {
 			   SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.driver_data	= &ksz8021_type,
-	.probe		= ksz8021_probe,
+	.probe		= kszphy_probe,
 	.config_init	= ksz8021_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,

@@ -606,7 +613,7 @@ static struct phy_driver ksphy_driver[] = {
 			   SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.driver_data	= &ksz8021_type,
-	.probe		= ksz8021_probe,
+	.probe		= kszphy_probe,
 	.config_init	= ksz8021_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,

@@ -658,7 +665,7 @@ static struct phy_driver ksphy_driver[] = {
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.driver_data	= &ksz8051_type,
 	.probe		= kszphy_probe,
-	.config_init	= ks8051_config_init,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
include/linux/micrel_phy.h +0 -1
index 53d33dee70e1..2e5b194b9b19 100644
@@ -37,7 +37,6 @@
 
 /* struct phy_device dev_flags definitions */
 #define MICREL_PHY_50MHZ_CLK	0x00000001
-#define MICREL_PHY_25MHZ_CLK	0x00000002
 
 #define MICREL_KSZ9021_EXTREG_CTRL	0xB
 #define MICREL_KSZ9021_EXTREG_DATA_WRITE	0xC
Signature · 1 line
2.0.4
author
#6 lore raw

PATCH 6/10 net: phy: micrel: add support for clock-mode select to KSZ8081/KSZ8091

In reply to Johan Hovold

Micrel KSZ8081 and KSZ8091 PHYs have the RMII Reference Clock Select bit, which is used to select 25 or 50 MHz clock mode.

Note that on some revisions of the PHY (e.g. KSZ8081RND) the function of this bit is inverted so that setting it enables 25 rather than 50 MHz mode. Add a new device-tree property "micrel,rmii-reference-clock-select-25-mhz" to describe this.

Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/micrel.txt |  4 ++--
 drivers/net/phy/micrel.c                         | 11 ++++++-----
 2 files changed, 8 insertions(+), 7 deletions(-)
Documentation/devicetree/bindings/net/micrel.txt +2 -2
index 30062fae5623..a1bab5eaae02 100644
@@ -22,5 +22,5 @@ Optional properties:
  - clocks, clock-names: contains clocks according to the common clock bindings.
 
               supported clocks:
-	      - KSZ8021, KSZ8031: "rmii-ref": The RMII refence input clock. Used
-		to determine the XI input clock.
+	      - KSZ8021, KSZ8031, KSZ8081, KSZ8091: "rmii-ref": The RMII
+		refence input clock. Used to determine the XI input clock.
drivers/net/phy/micrel.c +6 -5
index d2e790cd3651..04fbee846b66 100644
@@ -102,6 +102,7 @@ static const struct kszphy_type ksz8051_type = {
 static const struct kszphy_type ksz8081_type = {
 	.led_mode_reg		= MII_KSZPHY_CTRL_2,
 	.has_broadcast_disable	= true,
+	.has_rmii_ref_clk_sel	= true,
 };
 
 static int kszphy_extended_write(struct phy_device *phydev,

@@ -548,16 +549,16 @@ static int kszphy_probe(struct phy_device *phydev)
 	clk = devm_clk_get(&phydev->dev, "rmii-ref");
 	if (!IS_ERR(clk)) {
 		unsigned long rate = clk_get_rate(clk);
+		bool rmii_ref_clk_sel_25_mhz;
 
 		priv->rmii_ref_clk_sel = type->has_rmii_ref_clk_sel;
+		rmii_ref_clk_sel_25_mhz = of_property_read_bool(np,
+				"micrel,rmii-reference-clock-select-25-mhz");
 
-		/* FIXME: add support for PHY revisions that have this bit
-		 * inverted (e.g. through new property or based on PHY ID).
-		 */
 		if (rate > 24500000 && rate < 25500000) {
-			priv->rmii_ref_clk_sel_val = false;
+			priv->rmii_ref_clk_sel_val = rmii_ref_clk_sel_25_mhz;
 		} else if (rate > 49500000 && rate < 50500000) {
-			priv->rmii_ref_clk_sel_val = true;
+			priv->rmii_ref_clk_sel_val = !rmii_ref_clk_sel_25_mhz;
 		} else {
 			dev_err(&phydev->dev, "Clock rate out of range: %ld\n", rate);
 			return -EINVAL;
Signature · 1 line
2.0.4
author
#7 lore raw

PATCH 7/10 dt/bindings: reformat micrel eth-phy documentation

In reply to Johan Hovold

Reduce indentation of Micrel PHY binding documentations somewhat.

Also fix "reference input clock" typo while at it.

Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/micrel.txt | 26 ++++++++++++------------
 1 file changed, 13 insertions(+), 13 deletions(-)
Documentation/devicetree/bindings/net/micrel.txt +13 -13
index a1bab5eaae02..20a6cac7abc6 100644
@@ -6,21 +6,21 @@ Optional properties:
 
  - micrel,led-mode : LED mode value to set for PHYs with configurable LEDs.
 
-              Configure the LED mode with single value. The list of PHYs and
-	      the bits that are currently supported:
+	Configure the LED mode with single value. The list of PHYs and the
+	bits that are currently supported:
 
-	      KSZ8001: register 0x1e, bits 15..14
-	      KSZ8041: register 0x1e, bits 15..14
-	      KSZ8021: register 0x1f, bits 5..4
-	      KSZ8031: register 0x1f, bits 5..4
-	      KSZ8051: register 0x1f, bits 5..4
-	      KSZ8081: register 0x1f, bits 5..4
-	      KSZ8091: register 0x1f, bits 5..4
+	KSZ8001: register 0x1e, bits 15..14
+	KSZ8041: register 0x1e, bits 15..14
+	KSZ8021: register 0x1f, bits 5..4
+	KSZ8031: register 0x1f, bits 5..4
+	KSZ8051: register 0x1f, bits 5..4
+	KSZ8081: register 0x1f, bits 5..4
+	KSZ8091: register 0x1f, bits 5..4
 
-              See the respective PHY datasheet for the mode values.
+	See the respective PHY datasheet for the mode values.
 
  - clocks, clock-names: contains clocks according to the common clock bindings.
 
-              supported clocks:
-	      - KSZ8021, KSZ8031, KSZ8081, KSZ8091: "rmii-ref": The RMII
-		refence input clock. Used to determine the XI input clock.
+	supported clocks:
+	- KSZ8021, KSZ8031, KSZ8081, KSZ8091: "rmii-ref": The RMII reference
+	  input clock. Used to determine the XI input clock.
Signature · 1 line
2.0.4
author
#8 lore raw

PATCH 8/10 dt/bindings: add clock-select function property to micrel phy binding

In reply to Johan Hovold

Add "micrel,rmii-reference-clock-select-25-mhz" to Micrel ethernet PHY binding documentation.

This property is needed to properly describe some revisions of Micrel PHYs which has the function of this configuration bit inverted so that setting it enables 25 MHz rather than 50 MHz clock mode.

Note that a clock reference ("rmii-ref") is still needed to actually select either mode.

Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/micrel.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)
Documentation/devicetree/bindings/net/micrel.txt +11 -0
index 20a6cac7abc6..87496a8c64ab 100644
@@ -19,6 +19,17 @@ Optional properties:
 
 	See the respective PHY datasheet for the mode values.
 
+ - micrel,rmii-reference-clock-select-25-mhz: RMII Reference Clock Select
+						bit selects 25 MHz mode
+
+	Setting the RMII Reference Clock Select bit enables 25 MHz rather
+	than 50 MHz clock mode.
+
+	Note that this option in only needed for certain PHY revisions with a
+	non-standard, inverted function of this configuration bit.
+	Specifically, a clock reference ("rmii-ref" below) is always needed to
+	actually select a mode.
+
  - clocks, clock-names: contains clocks according to the common clock bindings.
 
 	supported clocks:
Signature · 1 line
2.0.4
Acked-by on v1 8/10
#9 lore raw

Re: [PATCH 08/10] dt/bindings: add clock-select function property to micrel phy binding

Quoted 24 lines · Johan Hovold

Add "micrel,rmii-reference-clock-select-25-mhz" to Micrel ethernet PHY binding documentation.

This property is needed to properly describe some revisions of Micrel PHYs which has the function of this configuration bit inverted so that setting it enables 25 MHz rather than 50 MHz clock mode.

Note that a clock reference ("rmii-ref") is still needed to actually select either mode.

Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/micrel.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)
Documentation/devicetree/bindings/net/micrel.txt +0 -0
index 20a6cac7abc6..87496a8c64ab 100644
@@ -19,6 +19,17 @@ Optional properties:
 
 	See the respective PHY datasheet for the mode values.

Documentation/devicetree/bindings/net/micrel.txt Optional properties: · line 31

Quoted 10 lines
Documentation/devicetree/bindings/net/micrel.txt +10 -0
+ - micrel,rmii-reference-clock-select-25-mhz: RMII Reference Clock Select
+						bit selects 25 MHz mode
+
+	Setting the RMII Reference Clock Select bit enables 25 MHz rather
+	than 50 MHz clock mode.
+
+	Note that this option in only needed for certain PHY revisions with a
+	non-standard, inverted function of this configuration bit.
+	Specifically, a clock reference ("rmii-ref" below) is always needed to
+	actually select a mode.

Nice, that should be clear.

Acked-by Sascha Hauer <[email protected]>

Sascha

Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
author
#10 lore raw

PATCH 9/10 net: phy: micrel: refactor interrupt config

In reply to Johan Hovold

Add generic interrupt-config callback and store interrupt-level bitmask in type data for PHY types not using bit 9.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 71 ++++++++++++++++++++----------------------------
 1 file changed, 29 insertions(+), 42 deletions(-)
drivers/net/phy/micrel.c +29 -42
index 04fbee846b66..d7eb7b0e6d4e 100644
@@ -55,8 +55,6 @@
 #define	MII_KSZPHY_CTRL				MII_KSZPHY_CTRL_2
 /* bitmap of PHY register to set interrupt mode */
 #define KSZPHY_CTRL_INT_ACTIVE_HIGH		BIT(9)
-#define KSZ9021_CTRL_INT_ACTIVE_HIGH		BIT(14)
-#define KS8737_CTRL_INT_ACTIVE_HIGH		BIT(14)
 #define KSZPHY_RMII_REF_CLK_SEL			BIT(7)
 
 /* Write/read to/from extended registers */

@@ -75,6 +73,7 @@
 
 struct kszphy_type {
 	u32 led_mode_reg;
+	u16 interrupt_level_mask;
 	bool has_broadcast_disable;
 	bool has_rmii_ref_clk_sel;
 };

@@ -105,6 +104,14 @@ static const struct kszphy_type ksz8081_type = {
 	.has_rmii_ref_clk_sel	= true,
 };
 
+static const struct kszphy_type ks8737_type = {
+	.interrupt_level_mask	= BIT(14),
+};
+
+static const struct kszphy_type ksz9021_type = {
+	.interrupt_level_mask	= BIT(14),
+};
+
 static int kszphy_extended_write(struct phy_device *phydev,
 				u32 regnum, u16 val)
 {

@@ -129,54 +136,31 @@ static int kszphy_ack_interrupt(struct phy_device *phydev)
 	return (rc < 0) ? rc : 0;
 }
 
-static int kszphy_set_interrupt(struct phy_device *phydev)
-{
-	int temp;
-	temp = (PHY_INTERRUPT_ENABLED == phydev->interrupts) ?
-		KSZPHY_INTCS_ALL : 0;
-	return phy_write(phydev, MII_KSZPHY_INTCS, temp);
-}
-
 static int kszphy_config_intr(struct phy_device *phydev)
 {
-	int temp, rc;
-
-	/* set the interrupt pin active low */
-	temp = phy_read(phydev, MII_KSZPHY_CTRL);
-	if (temp < 0)
-		return temp;
-	temp &= ~KSZPHY_CTRL_INT_ACTIVE_HIGH;
-	phy_write(phydev, MII_KSZPHY_CTRL, temp);
-	rc = kszphy_set_interrupt(phydev);
-	return rc < 0 ? rc : 0;
-}
+	const struct kszphy_type *type = phydev->drv->driver_data;
+	int temp;
+	u16 mask;
 
-static int ksz9021_config_intr(struct phy_device *phydev)
-{
-	int temp, rc;
+	if (type && type->interrupt_level_mask)
+		mask = type->interrupt_level_mask;
+	else
+		mask = KSZPHY_CTRL_INT_ACTIVE_HIGH;
 
 	/* set the interrupt pin active low */
 	temp = phy_read(phydev, MII_KSZPHY_CTRL);
 	if (temp < 0)
 		return temp;
-	temp &= ~KSZ9021_CTRL_INT_ACTIVE_HIGH;
+	temp &= ~mask;
 	phy_write(phydev, MII_KSZPHY_CTRL, temp);
-	rc = kszphy_set_interrupt(phydev);
-	return rc < 0 ? rc : 0;
-}
 
-static int ks8737_config_intr(struct phy_device *phydev)
-{
-	int temp, rc;
+	/* enable / disable interrupts */
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+		temp = KSZPHY_INTCS_ALL;
+	else
+		temp = 0;
 
-	/* set the interrupt pin active low */
-	temp = phy_read(phydev, MII_KSZPHY_CTRL);
-	if (temp < 0)
-		return temp;
-	temp &= ~KS8737_CTRL_INT_ACTIVE_HIGH;
-	phy_write(phydev, MII_KSZPHY_CTRL, temp);
-	rc = kszphy_set_interrupt(phydev);
-	return rc < 0 ? rc : 0;
+	return phy_write(phydev, MII_KSZPHY_INTCS, temp);
 }
 
 static int kszphy_rmii_clk_sel(struct phy_device *phydev, bool val)

@@ -581,11 +565,12 @@ static struct phy_driver ksphy_driver[] = {
 	.name		= "Micrel KS8737",
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.driver_data	= &ks8737_type,
 	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= ks8737_config_intr,
+	.config_intr	= kszphy_config_intr,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE,},

@@ -726,11 +711,12 @@ static struct phy_driver ksphy_driver[] = {
 	.name		= "Micrel KSZ9021 Gigabit PHY",
 	.features	= (PHY_GBIT_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.driver_data	= &ksz9021_type,
 	.config_init	= ksz9021_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= ksz9021_config_intr,
+	.config_intr	= kszphy_config_intr,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.read_mmd_indirect = ksz9021_rd_mmd_phyreg,

@@ -742,11 +728,12 @@ static struct phy_driver ksphy_driver[] = {
 	.name		= "Micrel KSZ9031 Gigabit PHY",
 	.features	= (PHY_GBIT_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.driver_data	= &ksz9021_type,
 	.config_init	= ksz9031_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= ksz9021_config_intr,
+	.config_intr	= kszphy_config_intr,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE, },
Signature · 1 line
2.0.4
author
#11 lore raw

PATCH 10/10 net: phy: micrel: add copyright entry

In reply to Johan Hovold

Add myself to the list of copyright holders.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 1 +
 1 file changed, 1 insertion(+)
drivers/net/phy/micrel.c +1 -0
index d7eb7b0e6d4e..c530de1e63f5 100644
@@ -6,6 +6,7 @@
  * Author: David J. Choi
  *
  * Copyright (c) 2010-2013 Micrel, Inc.
+ * Copyright (c) 2014 Johan Hovold <[email protected]>
  *
  * 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
Signature · 1 line
2.0.4
Reviewed-by on the v1 series
#12 lore raw
In reply to Johan Hovold
Quoted 24 lines · Johan Hovold

This series adds device and device-type abstractions to the micrel driver, and enables support for RMII-reference clock selection for KSZ8081 and KSZ8091 devices.

While adding support for more features for the Micrel PHYs mentioned above, it became apparent that the configuration space is much too large and that adding type-specific callbacks will simply not scale. Instead I added a driver_data field to struct phy_device, which can be used to store static device type data that can be parsed and acted on in generic driver callbacks. This allows a lot of duplicated code to be removed, and should make it much easier to add new features or deal with device-type quirks in the future.

The series has been tested on a dual KSZ8081 setup. Further testing on other Micrel PHYs would be much appreciated.

The recent commit a95a18afe4c8 ("phy/micrel: KSZ8031RNL RMII clock reconfiguration bug") currently prevents KSZ8031 PHYs from using the generic config-init. Bruno, who is the author of that patch, has agreed to test this series and some follow-up diagnostic patches to determine how best to incorporate these devices as well. I intend to send a follow-up patch that removes the custom 8031 config-init and documents this quirk, but the current series can be applied meanwhile.

Quoted 2 lines

These patches are against net-next which contains some already merged prerequisite patches to the driver.

LGTM, thanks Johan!

Reviewed-by Florian Fainelli <[email protected]>
Quoted 22 lines

Johan

Johan Hovold (10):
net: phy: add static data field to struct phy_driver
net: phy: micrel: add device-type abstraction
net: phy: micrel: parse of nodes at probe
net: phy: micrel: add has-broadcast-disable flag to type data
net: phy: micrel: add generic clock-mode-select support
net: phy: micrel: add support for clock-mode select to KSZ8081/KSZ8091 dt/bindings: reformat micrel eth-phy documentation dt/bindings: add clock-select function property to micrel phy binding
net: phy: micrel: refactor interrupt config
net: phy: micrel: add copyright entry

 Documentation/devicetree/bindings/net/micrel.txt |  37 ++--
 drivers/net/phy/micrel.c                         | 262 ++++++++++++++---------
 include/linux/micrel_phy.h                       |   1 -
 include/linux/phy.h                              |   2 +
 4 files changed, 184 insertions(+), 118 deletions(-)
maintainer
#13 lore raw
In reply to Johan Hovold

From: Johan Hovold <[email protected]>
Date: Wed, 19 Nov 2014 12:59:13 +0100

Quoted 3 lines

This series adds device and device-type abstractions to the micrel driver, and enables support for RMII-reference clock selection for KSZ8081 and KSZ8091 devices.

Series applied, thanks Johan.