Skip to contentSkip to conversation

net: phy: micrel: refactoring and KSZ8081/KSZ8091 features

PATCH In review Johan Hovold 4 participants 14 messages Nov 11, 2014 – Nov 12, 2014

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

Johan Hovold

author
#0 lore raw

cover net: phy: micrel: refactoring and KSZ8081/KSZ8091 features

This series cleans up and refactors parts of the micrel PHY driver, and adds support for broadcast-address-disable and led-mode configuration for KSZ8081 and KSZ8091 PHYs.

Specifically, this enables dual KSZ8081 setups (which are limited to using address 0 and 3).

A follow up series will add device-type abstraction which will allow for further refactoring and shared initialisation code.

Johan

Johan Hovold (9):
dt/bindings: fix documentation of ethernet-phy compatible property
net: phy: micrel: fix config_intr error handling
net: phy: micrel: use BIT macro
net: phy: micrel: refactor broadcast disable
net: phy: micrel: disable broadcast for KSZ8081/KSZ8091
net: phy: micrel: add led-mode sanity check
net: phy: micrel: refactor led-mode error handling
net: phy: micrel: clean up led-mode setup
net: phy: micrel: enable led-mode for KSZ8081/KSZ8091

 Documentation/devicetree/bindings/net/micrel.txt |   2 +
 Documentation/devicetree/bindings/net/phy.txt    |   3 +-
 drivers/net/phy/micrel.c                         | 125 ++++++++++++++++-------
 3 files changed, 93 insertions(+), 37 deletions(-)
Signature · 1 line
2.0.4
author
#1 lore raw

PATCH 1/9 dt/bindings: fix documentation of ethernet-phy compatible property

A recent commit extended the documentation of the ethernet-phy compatible property, but placed the new paragraph under the max-speed property.

Fixes f00e756ed12d ("dt: Document a compatible entry for MDIO ethernet

Phys")

Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/phy.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/net/phy.txt +2 -1
index 5b8c58903077..40831fbaff72 100644
@@ -19,7 +19,6 @@ Optional Properties:
   specifications. If neither of these are specified, the default is to
   assume clause 22. The compatible list may also contain other
   elements.
-- max-speed: Maximum PHY supported speed (10, 100, 1000...)
 
   If the phy's identifier is known then the list may contain an entry
   of the form: "ethernet-phy-idAAAA.BBBB" where

@@ -29,6 +28,8 @@ Optional Properties:
             4 hex digits. This is the chip vendor OUI bits 19:24,
             followed by 10 bits of a vendor specific ID.
 
+- max-speed: Maximum PHY supported speed (10, 100, 1000...)
+
 Example:
 
 ethernet-phy@0 {
Signature · 1 line
2.0.4
author
#2 lore raw

PATCH 2/9 net: phy: micrel: fix config_intr error handling

In reply to Johan Hovold

Make sure never to update the control register with random data (an error code) by checking the return value after reading it.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/phy/micrel.c +6 -0
index bcc6c0ea75fa..62ca9613a514 100644
@@ -122,6 +122,8 @@ static int kszphy_config_intr(struct phy_device *phydev)
 
 	/* 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);

@@ -134,6 +136,8 @@ static int ksz9021_config_intr(struct phy_device *phydev)
 
 	/* set the interrupt pin active low */
 	temp = phy_read(phydev, MII_KSZPHY_CTRL);
+	if (temp < 0)
+		return temp;
 	temp &= ~KSZ9021_CTRL_INT_ACTIVE_HIGH;
 	phy_write(phydev, MII_KSZPHY_CTRL, temp);
 	rc = kszphy_set_interrupt(phydev);

@@ -146,6 +150,8 @@ static int ks8737_config_intr(struct phy_device *phydev)
 
 	/* 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);
Signature · 1 line
2.0.4
author
#3 lore raw

PATCH 3/9 net: phy: micrel: use BIT macro

In reply to Johan Hovold

Use BIT macro for bitmask definitions.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)
drivers/net/phy/micrel.c +17 -17
index 62ca9613a514..d962a2866bba 100644
@@ -30,30 +30,30 @@
 
 /* Operation Mode Strap Override */
 #define MII_KSZPHY_OMSO				0x16
-#define KSZPHY_OMSO_B_CAST_OFF			(1 << 9)
-#define KSZPHY_OMSO_RMII_OVERRIDE		(1 << 1)
-#define KSZPHY_OMSO_MII_OVERRIDE		(1 << 0)
+#define KSZPHY_OMSO_B_CAST_OFF			BIT(9)
+#define KSZPHY_OMSO_RMII_OVERRIDE		BIT(1)
+#define KSZPHY_OMSO_MII_OVERRIDE		BIT(0)
 
 /* general Interrupt control/status reg in vendor specific block. */
 #define MII_KSZPHY_INTCS			0x1B
-#define	KSZPHY_INTCS_JABBER			(1 << 15)
-#define	KSZPHY_INTCS_RECEIVE_ERR		(1 << 14)
-#define	KSZPHY_INTCS_PAGE_RECEIVE		(1 << 13)
-#define	KSZPHY_INTCS_PARELLEL			(1 << 12)
-#define	KSZPHY_INTCS_LINK_PARTNER_ACK		(1 << 11)
-#define	KSZPHY_INTCS_LINK_DOWN			(1 << 10)
-#define	KSZPHY_INTCS_REMOTE_FAULT		(1 << 9)
-#define	KSZPHY_INTCS_LINK_UP			(1 << 8)
+#define	KSZPHY_INTCS_JABBER			BIT(15)
+#define	KSZPHY_INTCS_RECEIVE_ERR		BIT(14)
+#define	KSZPHY_INTCS_PAGE_RECEIVE		BIT(13)
+#define	KSZPHY_INTCS_PARELLEL			BIT(12)
+#define	KSZPHY_INTCS_LINK_PARTNER_ACK		BIT(11)
+#define	KSZPHY_INTCS_LINK_DOWN			BIT(10)
+#define	KSZPHY_INTCS_REMOTE_FAULT		BIT(9)
+#define	KSZPHY_INTCS_LINK_UP			BIT(8)
 #define	KSZPHY_INTCS_ALL			(KSZPHY_INTCS_LINK_UP |\
 						KSZPHY_INTCS_LINK_DOWN)
 
 /* general PHY control reg in vendor specific block. */
 #define	MII_KSZPHY_CTRL			0x1F
 /* bitmap of PHY register to set interrupt mode */
-#define KSZPHY_CTRL_INT_ACTIVE_HIGH		(1 << 9)
-#define KSZ9021_CTRL_INT_ACTIVE_HIGH		(1 << 14)
-#define KS8737_CTRL_INT_ACTIVE_HIGH		(1 << 14)
-#define KSZ8051_RMII_50MHZ_CLK			(1 << 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)
 
 /* Write/read to/from extended registers */
 #define MII_KSZPHY_EXTREG                       0x0b

@@ -400,8 +400,8 @@ static int ksz9031_config_init(struct phy_device *phydev)
 }
 
 #define KSZ8873MLL_GLOBAL_CONTROL_4	0x06
-#define KSZ8873MLL_GLOBAL_CONTROL_4_DUPLEX	(1 << 6)
-#define KSZ8873MLL_GLOBAL_CONTROL_4_SPEED	(1 << 4)
+#define KSZ8873MLL_GLOBAL_CONTROL_4_DUPLEX	BIT(6)
+#define KSZ8873MLL_GLOBAL_CONTROL_4_SPEED	BIT(4)
 static int ksz8873mll_read_status(struct phy_device *phydev)
 {
 	int regval;
Signature · 1 line
2.0.4
author
#4 lore raw

PATCH 4/9 net: phy: micrel: refactor broadcast disable

In reply to Johan Hovold

Refactor and clean up broadcast disable.

Some Micrel PHYs have a broadcast-off bit in the Operation Mode Strap Override register which can be used to disable PHY address 0 as the broadcast address, so that it can be used as a unique (non-broadcast) address on a shared bus.

Note that the KSZPHY_OMSO_RMII_OVERRIDE bit is set by default on KSZ8021/8031.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)
drivers/net/phy/micrel.c +22 -2
index d962a2866bba..21abe5ade3df 100644
@@ -184,6 +184,25 @@ static int kszphy_setup_led(struct phy_device *phydev,
 	return rc < 0 ? rc : 0;
 }
 
+/* Disable PHY address 0 as the broadcast address, so that it can be used as a
+ * unique (non-broadcast) address on a shared bus.
+ */
+static int kszphy_broadcast_disable(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_read(phydev, MII_KSZPHY_OMSO);
+	if (ret < 0)
+		goto out;
+
+	ret = phy_write(phydev, MII_KSZPHY_OMSO, ret | KSZPHY_OMSO_B_CAST_OFF);
+out:
+	if (ret)
+		dev_err(&phydev->dev, "failed to disable broadcast address\n");
+
+	return ret;
+}
+
 static int kszphy_config_init(struct phy_device *phydev)
 {
 	return 0;

@@ -197,7 +216,6 @@ static int kszphy_config_init_led8041(struct phy_device *phydev)
 
 static int ksz8021_config_init(struct phy_device *phydev)
 {
-	const u16 val = KSZPHY_OMSO_B_CAST_OFF | KSZPHY_OMSO_RMII_OVERRIDE;
 	int rc;
 
 	rc = kszphy_setup_led(phydev, 0x1f, 4);

@@ -207,7 +225,9 @@ static int ksz8021_config_init(struct phy_device *phydev)
 	rc = ksz_config_flags(phydev);
 	if (rc < 0)
 		return rc;
-	rc = phy_write(phydev, MII_KSZPHY_OMSO, val);
+
+	rc = kszphy_broadcast_disable(phydev);
+
 	return rc < 0 ? rc : 0;
 }
Signature · 1 line
2.0.4
author
#5 lore raw

PATCH 5/9 net: phy: micrel: disable broadcast for KSZ8081/KSZ8091

In reply to Johan Hovold

Disable PHY address 0 as the broadcast address, so that it can be used as a unique (non-broadcast) address on a shared bus.

Note that this can also be configured using the B-CAST_OFF pin on KSZ9091, but that KSZ8081 lacks this pin and is also limited to addresses 0 and 3.

Specifically, this allows for dual KSZ8081 setups.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/net/phy/micrel.c +8 -1
index 21abe5ade3df..16135ac18bfe 100644
@@ -243,6 +243,13 @@ 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 0;
+}
+
 static int ksz9021_load_values_from_of(struct phy_device *phydev,
 				       struct device_node *of_node, u16 reg,
 				       char *field1, char *field2,

@@ -605,7 +612,7 @@ static struct phy_driver ksphy_driver[] = {
 	.phy_id_mask	= 0x00fffff0,
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init,
+	.config_init	= ksz8081_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
Signature · 1 line
2.0.4
author
#6 lore raw

PATCH 6/9 net: phy: micrel: add led-mode sanity check

In reply to Johan Hovold

Make sure never to update more than two bits when setting the led mode, something which could for example change the reference-clock setting.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/net/phy/micrel.c +5 -0
index 16135ac18bfe..1b3985cdc64c 100644
@@ -173,6 +173,11 @@ static int kszphy_setup_led(struct phy_device *phydev,
 	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;
+	}
+
 	temp = phy_read(phydev, reg);
 	if (temp < 0)
 		return temp;
Signature · 1 line
2.0.4
author
#7 lore raw

PATCH 7/9 net: phy: micrel: refactor led-mode error handling

In reply to Johan Hovold

Refactor led-mode error handling.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)
drivers/net/phy/micrel.c +10 -9
index 1b3985cdc64c..ec9ce35e934b 100644
@@ -179,14 +179,19 @@ static int kszphy_setup_led(struct phy_device *phydev,
 	}
 
 	temp = phy_read(phydev, reg);
-	if (temp < 0)
-		return temp;
+	if (temp < 0) {
+		rc = temp;
+		goto out;
+	}
 
 	temp &= ~(3 << shift);
 	temp |= val << shift;
 	rc = phy_write(phydev, reg, temp);
+out:
+	if (rc < 0)
+		dev_err(&phydev->dev, "failed to set led mode\n");
 
-	return rc < 0 ? rc : 0;
+	return rc;
 }
 
 /* Disable PHY address 0 as the broadcast address, so that it can be used as a

@@ -223,9 +228,7 @@ static int ksz8021_config_init(struct phy_device *phydev)
 {
 	int rc;
 
-	rc = kszphy_setup_led(phydev, 0x1f, 4);
-	if (rc)
-		dev_err(&phydev->dev, "failed to set led mode\n");
+	kszphy_setup_led(phydev, 0x1f, 4);
 
 	rc = ksz_config_flags(phydev);
 	if (rc < 0)

@@ -240,9 +243,7 @@ static int ks8051_config_init(struct phy_device *phydev)
 {
 	int rc;
 
-	rc = kszphy_setup_led(phydev, 0x1f, 4);
-	if (rc)
-		dev_err(&phydev->dev, "failed to set led mode\n");
+	kszphy_setup_led(phydev, 0x1f, 4);
 
 	rc = ksz_config_flags(phydev);
 	return rc < 0 ? rc : 0;
Signature · 1 line
2.0.4
author
#8 lore raw

PATCH 8/9 net: phy: micrel: clean up led-mode setup

In reply to Johan Hovold

Clean up led-mode setup by introducing proper defines for PHY Control registers 1 and 2 and only passing the register to the setup function.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)
drivers/net/phy/micrel.c +22 -9
index ec9ce35e934b..12e18f7273ce 100644
@@ -47,8 +47,12 @@
 #define	KSZPHY_INTCS_ALL			(KSZPHY_INTCS_LINK_UP |\
 						KSZPHY_INTCS_LINK_DOWN)
 
-/* general PHY control reg in vendor specific block. */
-#define	MII_KSZPHY_CTRL			0x1F
+/* PHY Control 1 */
+#define	MII_KSZPHY_CTRL_1			0x1e
+
+/* PHY Control 2 / PHY Control (if no PHY Control 1) */
+#define	MII_KSZPHY_CTRL_2			0x1f
+#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)

@@ -158,13 +162,12 @@ static int ks8737_config_intr(struct phy_device *phydev)
 	return rc < 0 ? rc : 0;
 }
 
-static int kszphy_setup_led(struct phy_device *phydev,
-			    unsigned int reg, unsigned int shift)
+static int kszphy_setup_led(struct phy_device *phydev, u32 reg)
 {
 
 	struct device *dev = &phydev->dev;
 	struct device_node *of_node = dev->of_node;
-	int rc, temp;
+	int rc, temp, shift;
 	u32 val;
 
 	if (!of_node && dev->parent->of_node)

@@ -178,6 +181,17 @@ static int kszphy_setup_led(struct phy_device *phydev,
 		return -EINVAL;
 	}
 
+	switch (reg) {
+	case MII_KSZPHY_CTRL_1:
+		shift = 14;
+		break;
+	case MII_KSZPHY_CTRL_2:
+		shift = 4;
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	temp = phy_read(phydev, reg);
 	if (temp < 0) {
 		rc = temp;

@@ -220,15 +234,14 @@ static int kszphy_config_init(struct phy_device *phydev)
 
 static int kszphy_config_init_led8041(struct phy_device *phydev)
 {
-	/* single led control, register 0x1e bits 15..14 */
-	return kszphy_setup_led(phydev, 0x1e, 14);
+	return kszphy_setup_led(phydev, MII_KSZPHY_CTRL_1);
 }
 
 static int ksz8021_config_init(struct phy_device *phydev)
 {
 	int rc;
 
-	kszphy_setup_led(phydev, 0x1f, 4);
+	kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
 
 	rc = ksz_config_flags(phydev);
 	if (rc < 0)

@@ -243,7 +256,7 @@ static int ks8051_config_init(struct phy_device *phydev)
 {
 	int rc;
 
-	kszphy_setup_led(phydev, 0x1f, 4);
+	kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
 
 	rc = ksz_config_flags(phydev);
 	return rc < 0 ? rc : 0;
Signature · 1 line
2.0.4
#9 lore raw

Re: [PATCH 8/9] net: phy: micrel: clean up led-mode setup

Hello.

Quoted 2 lines · Johan Hovold

Clean up led-mode setup by introducing proper defines for PHY Control registers 1 and 2 and only passing the register to the setup function.

Not sure that's really better that it was before (modulo naming).

Quoted 77 lines
Signed-off-by Johan Hovold <[email protected]>
---
  drivers/net/phy/micrel.c | 31 ++++++++++++++++++++++---------
  1 file changed, 22 insertions(+), 9 deletions(-)
drivers/net/phy/micrel.c +21 -8
index ec9ce35e934b..12e18f7273ce 100644
@@ -47,8 +47,12 @@
  #define	KSZPHY_INTCS_ALL			(KSZPHY_INTCS_LINK_UP |\
  						KSZPHY_INTCS_LINK_DOWN)
 
-/* general PHY control reg in vendor specific block. */
-#define	MII_KSZPHY_CTRL			0x1F
+/* PHY Control 1 */
+#define	MII_KSZPHY_CTRL_1			0x1e
+
+/* PHY Control 2 / PHY Control (if no PHY Control 1) */
+#define	MII_KSZPHY_CTRL_2			0x1f
+#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)

@@ -158,13 +162,12 @@ static int ks8737_config_intr(struct phy_device *phydev)
  	return rc < 0 ? rc : 0;
  }
 
-static int kszphy_setup_led(struct phy_device *phydev,
-			    unsigned int reg, unsigned int shift)
+static int kszphy_setup_led(struct phy_device *phydev, u32 reg)
  {
 
  	struct device *dev = &phydev->dev;
  	struct device_node *of_node = dev->of_node;
-	int rc, temp;
+	int rc, temp, shift;
  	u32 val;
 
  	if (!of_node && dev->parent->of_node)

@@ -178,6 +181,17 @@ static int kszphy_setup_led(struct phy_device *phydev,
  		return -EINVAL;
  	}
 
+	switch (reg) {
+	case MII_KSZPHY_CTRL_1:
+		shift = 14;
+		break;
+	case MII_KSZPHY_CTRL_2:
+		shift = 4;
+		break;
+	default:
+		return -EINVAL;
+	}
+
  	temp = phy_read(phydev, reg);
  	if (temp < 0) {
  		rc = temp;

@@ -220,15 +234,14 @@ static int kszphy_config_init(struct phy_device *phydev)
 
  static int kszphy_config_init_led8041(struct phy_device *phydev)
  {
-	/* single led control, register 0x1e bits 15..14 */
-	return kszphy_setup_led(phydev, 0x1e, 14);
+	return kszphy_setup_led(phydev, MII_KSZPHY_CTRL_1);
  }
 
  static int ksz8021_config_init(struct phy_device *phydev)
  {
  	int rc;
 
-	kszphy_setup_led(phydev, 0x1f, 4);
+	kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
 
  	rc = ksz_config_flags(phydev);
  	if (rc < 0)

drivers/net/phy/micrel.c static int ks8051_config_init(struct phy_device *phydev) · line 262

Quoted 8 lines
drivers/net/phy/micrel.c +1 -1
@@ -243,7 +256,7 @@ static int ks8051_config_init(struct phy_device *phydev)
  {
  	int rc;
 
-	kszphy_setup_led(phydev, 0x1f, 4);
+	kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
 
  	rc = ksz_config_flags(phydev);
  	return rc < 0 ? rc : 0;

WBR, Sergei

#10 lore raw

Re: [PATCH 8/9] net: phy: micrel: clean up led-mode setup

Quoted 8 lines · Sergei Shtylyov

Hello.

Quoted 2 lines · Johan Hovold

Clean up led-mode setup by introducing proper defines for PHY Control registers 1 and 2 and only passing the register to the setup function.

Not sure that's really better that it was before (modulo naming).

We do have proper error handling in kszphy_setup_led() which is already an improvement.

Quoted 93 lines
Quoted 89 lines
Signed-off-by Johan Hovold <[email protected]>
---
  drivers/net/phy/micrel.c | 31 ++++++++++++++++++++++---------
  1 file changed, 22 insertions(+), 9 deletions(-)
drivers/net/phy/micrel.c +8 -5
index ec9ce35e934b..12e18f7273ce 100644
@@ -47,8 +47,12 @@
  #define    KSZPHY_INTCS_ALL            (KSZPHY_INTCS_LINK_UP |\
                          KSZPHY_INTCS_LINK_DOWN)
 
-/* general PHY control reg in vendor specific block. */
-#define    MII_KSZPHY_CTRL            0x1F
+/* PHY Control 1 */
+#define    MII_KSZPHY_CTRL_1            0x1e
+
+/* PHY Control 2 / PHY Control (if no PHY Control 1) */
+#define    MII_KSZPHY_CTRL_2            0x1f
+#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)

@@ -158,13 +162,12 @@ static int ks8737_config_intr(struct phy_device
 *phydev)
      return rc < 0 ? rc : 0;
  }
 
-static int kszphy_setup_led(struct phy_device *phydev,
-                unsigned int reg, unsigned int shift)
+static int kszphy_setup_led(struct phy_device *phydev, u32 reg)
  {
 
      struct device *dev = &phydev->dev;
      struct device_node *of_node = dev->of_node;
-    int rc, temp;
+    int rc, temp, shift;
      u32 val;
 

      if (!of_node && dev->parent->of_node)
drivers/net/phy/micrel.c +11 -0
@@ -178,6 +181,17 @@ static int kszphy_setup_led(struct phy_device
 *phydev,
          return -EINVAL;
      }
 
+    switch (reg) {
+    case MII_KSZPHY_CTRL_1:
+        shift = 14;
+        break;
+    case MII_KSZPHY_CTRL_2:
+        shift = 4;
+        break;
+    default:
+        return -EINVAL;
+    }
+
      temp = phy_read(phydev, reg);
      if (temp < 0) {

          rc = temp;
drivers/net/phy/micrel.c +2 -3
@@ -220,15 +234,14 @@ static int kszphy_config_init(struct phy_device
 *phydev)
 
  static int kszphy_config_init_led8041(struct phy_device *phydev)
  {
-    /* single led control, register 0x1e bits 15..14 */
-    return kszphy_setup_led(phydev, 0x1e, 14);
+    return kszphy_setup_led(phydev, MII_KSZPHY_CTRL_1);
  }
 
  static int ksz8021_config_init(struct phy_device *phydev)
  {
      int rc;
 
-    kszphy_setup_led(phydev, 0x1f, 4);
+    kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
 
      rc = ksz_config_flags(phydev);

      if (rc < 0)
drivers/net/phy/micrel.c +1 -1
@@ -243,7 +256,7 @@ static int ks8051_config_init(struct phy_device
 *phydev)
  {
      int rc;
 
-    kszphy_setup_led(phydev, 0x1f, 4);
+    kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
 
      rc = ksz_config_flags(phydev);
Quoted 1 line
      return rc < 0 ? rc : 0;

WBR, Sergei

author
#11 lore raw

Re: [PATCH 8/9] net: phy: micrel: clean up led-mode setup

Quoted 5 lines · Florian Fainelli
Quoted 4 lines · Sergei Shtylyov

Hello.

On 11/11/2014 10:00 PM, Johan Hovold wrote:

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Clean up led-mode setup by introducing proper defines for PHY Control registers 1 and 2 and only passing the register to the setup function.

Not sure that's really better that it was before (modulo naming).

We do have proper error handling in kszphy_setup_led() which is already an improvement.

This also means handling the led mode in one place rather than spreading it all over the driver, with multiple config functions providing one of the same two combinations of register and shift.

It's usefulness will perhaps become more apparent if you look at the follow up patches, which does further refactoring and store the led-mode register in the type data.

Johan

author
#12 lore raw

PATCH 9/9 net: phy: micrel: enable led-mode for KSZ8081/KSZ8091

In reply to Johan Hovold

Enable led-mode configuration for KSZ8081 and KSZ8091.

Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/micrel.txt | 2 ++
 drivers/net/phy/micrel.c                         | 1 +
 2 files changed, 3 insertions(+)
Documentation/devicetree/bindings/net/micrel.txt +2 -0
index e1d99b95c4ec..30062fae5623 100644
@@ -14,6 +14,8 @@ Optional properties:
 	      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.
drivers/net/phy/micrel.c +1 -0
index 12e18f7273ce..30e894d6ffbd 100644
@@ -265,6 +265,7 @@ 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;
 }
Signature · 1 line
2.0.4
maintainer
#13 lore raw
In reply to Johan Hovold

From: Johan Hovold <[email protected]>
Date: Tue, 11 Nov 2014 20:00:06 +0100

Quoted 9 lines

This series cleans up and refactors parts of the micrel PHY driver, and adds support for broadcast-address-disable and led-mode configuration for KSZ8081 and KSZ8091 PHYs.

Specifically, this enables dual KSZ8081 setups (which are limited to using address 0 and 3).

A follow up series will add device-type abstraction which will allow for further refactoring and shared initialisation code.

Series applied, thanks Johan.