linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/6] net: dsa: abstract PHY accesses
@ 2016-08-15 21:18 Vivien Didelot
  2016-08-15 21:18 ` [PATCH net-next 1/6] net: dsa: mv88e6xxx: rename _mv88e6xxx_wait Vivien Didelot
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Vivien Didelot @ 2016-08-15 21:18 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

The Marvell 88E6xxx switch chips have different way to access the PHY
devices registers. 

Old chips use a direct access to the PHY registers. Next chips have a
PHY Polling Unit (PPU) which needs to be disabled before accessing PHY
registers. Newer chips have an indirect access to the PHY devices so
that disabling the PPU is not necessary.

This patchset abstracts these accesses behind a new mv88e6xxx_phy_* API.

It also has the side effect to fix the temperature access code for
88E61xx chips which were using the wrong PHY access functions.

Vivien Didelot (6):
  net: dsa: mv88e6xxx: rename _mv88e6xxx_wait
  net: dsa: mv88e6xxx: describe Multi-chip registers
  net: dsa: mv88e6xxx: rework Global2 SMI PHY access
  net: dsa: mv88e6xxx: abstract PHY ops
  net: dsa: mv88e6xxx: describe PHY page and SerDes
  net: dsa: mv88e6xxx: use the new PHY API

 drivers/net/dsa/mv88e6xxx/chip.c      | 541 ++++++++++++++++++----------------
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 117 +++++---
 2 files changed, 369 insertions(+), 289 deletions(-)

-- 
2.9.3

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH net-next 1/6] net: dsa: mv88e6xxx: rename _mv88e6xxx_wait
  2016-08-15 21:18 [PATCH net-next 0/6] net: dsa: abstract PHY accesses Vivien Didelot
@ 2016-08-15 21:18 ` Vivien Didelot
  2016-08-15 21:18 ` [PATCH net-next 2/6] net: dsa: mv88e6xxx: describe Multi-chip registers Vivien Didelot
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Vivien Didelot @ 2016-08-15 21:18 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

Now that there is no locked version of the wait routine anymore, rename
the _ prefixed version and make it use the new read API.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 63 +++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 30 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d36aedd..a5a96bc 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -216,6 +216,28 @@ static int mv88e6xxx_write(struct mv88e6xxx_chip *chip,
 	return 0;
 }
 
+static int mv88e6xxx_wait(struct mv88e6xxx_chip *chip, int addr, int reg,
+			  u16 mask)
+{
+	unsigned long timeout = jiffies + HZ / 10;
+
+	while (time_before(jiffies, timeout)) {
+		u16 val;
+		int err;
+
+		err = mv88e6xxx_read(chip, addr, reg, &val);
+		if (err)
+			return err;
+
+		if (!(val & mask))
+			return 0;
+
+		usleep_range(1000, 2000);
+	}
+
+	return -ETIMEDOUT;
+}
+
 /* Indirect write to single pointer-data register with an Update bit */
 static int mv88e6xxx_update(struct mv88e6xxx_chip *chip, int addr, int reg,
 			    u16 update)
@@ -819,35 +841,16 @@ static void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
 	mutex_unlock(&chip->reg_lock);
 }
 
-static int _mv88e6xxx_wait(struct mv88e6xxx_chip *chip, int reg, int offset,
-			   u16 mask)
-{
-	unsigned long timeout = jiffies + HZ / 10;
-
-	while (time_before(jiffies, timeout)) {
-		int ret;
-
-		ret = _mv88e6xxx_reg_read(chip, reg, offset);
-		if (ret < 0)
-			return ret;
-		if (!(ret & mask))
-			return 0;
-
-		usleep_range(1000, 2000);
-	}
-	return -ETIMEDOUT;
-}
-
 static int mv88e6xxx_mdio_wait(struct mv88e6xxx_chip *chip)
 {
-	return _mv88e6xxx_wait(chip, REG_GLOBAL2, GLOBAL2_SMI_OP,
-			       GLOBAL2_SMI_OP_BUSY);
+	return mv88e6xxx_wait(chip, REG_GLOBAL2, GLOBAL2_SMI_OP,
+			      GLOBAL2_SMI_OP_BUSY);
 }
 
 static int _mv88e6xxx_atu_wait(struct mv88e6xxx_chip *chip)
 {
-	return _mv88e6xxx_wait(chip, REG_GLOBAL, GLOBAL_ATU_OP,
-			       GLOBAL_ATU_OP_BUSY);
+	return mv88e6xxx_wait(chip, REG_GLOBAL, GLOBAL_ATU_OP,
+			      GLOBAL_ATU_OP_BUSY);
 }
 
 static int mv88e6xxx_mdio_read_indirect(struct mv88e6xxx_chip *chip,
@@ -1227,8 +1230,8 @@ static int _mv88e6xxx_port_pvid_set(struct mv88e6xxx_chip *chip,
 
 static int _mv88e6xxx_vtu_wait(struct mv88e6xxx_chip *chip)
 {
-	return _mv88e6xxx_wait(chip, REG_GLOBAL, GLOBAL_VTU_OP,
-			       GLOBAL_VTU_OP_BUSY);
+	return mv88e6xxx_wait(chip, REG_GLOBAL, GLOBAL_VTU_OP,
+			      GLOBAL_VTU_OP_BUSY);
 }
 
 static int _mv88e6xxx_vtu_cmd(struct mv88e6xxx_chip *chip, u16 op)
@@ -2949,8 +2952,8 @@ static int mv88e6xxx_g2_clear_irl(struct mv88e6xxx_chip *chip)
 			break;
 
 		/* Wait for the operation to complete */
-		err = _mv88e6xxx_wait(chip, REG_GLOBAL2, GLOBAL2_IRL_CMD,
-				      GLOBAL2_IRL_CMD_BUSY);
+		err = mv88e6xxx_wait(chip, REG_GLOBAL2, GLOBAL2_IRL_CMD,
+				     GLOBAL2_IRL_CMD_BUSY);
 		if (err)
 			break;
 	}
@@ -3004,9 +3007,9 @@ static int mv88e6xxx_g2_clear_pot(struct mv88e6xxx_chip *chip)
 
 static int mv88e6xxx_g2_eeprom_wait(struct mv88e6xxx_chip *chip)
 {
-	return _mv88e6xxx_wait(chip, REG_GLOBAL2, GLOBAL2_EEPROM_CMD,
-			       GLOBAL2_EEPROM_CMD_BUSY |
-			       GLOBAL2_EEPROM_CMD_RUNNING);
+	return mv88e6xxx_wait(chip, REG_GLOBAL2, GLOBAL2_EEPROM_CMD,
+			      GLOBAL2_EEPROM_CMD_BUSY |
+			      GLOBAL2_EEPROM_CMD_RUNNING);
 }
 
 static int mv88e6xxx_g2_eeprom_cmd(struct mv88e6xxx_chip *chip, u16 cmd)
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH net-next 2/6] net: dsa: mv88e6xxx: describe Multi-chip registers
  2016-08-15 21:18 [PATCH net-next 0/6] net: dsa: abstract PHY accesses Vivien Didelot
  2016-08-15 21:18 ` [PATCH net-next 1/6] net: dsa: mv88e6xxx: rename _mv88e6xxx_wait Vivien Didelot
@ 2016-08-15 21:18 ` Vivien Didelot
  2016-08-15 21:18 ` [PATCH net-next 3/6] net: dsa: mv88e6xxx: rework Global2 SMI PHY access Vivien Didelot
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Vivien Didelot @ 2016-08-15 21:18 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

Add flags to describe the presence of SMI Command and Data registers
used to indirectly access internal SMI devices registers when the switch
SMI address is not zero.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c      |  2 +-
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 40 ++++++++++++++++++++++-------------
 2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index a5a96bc..44debf2 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3883,7 +3883,7 @@ static int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip,
 
 	if (sw_addr == 0)
 		chip->smi_ops = &mv88e6xxx_smi_single_chip_ops;
-	else if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_MULTI_CHIP))
+	else if (mv88e6xxx_has(chip, MV88E6XXX_FLAGS_MULTI_CHIP))
 		chip->smi_ops = &mv88e6xxx_smi_multi_chip_ops;
 	else
 		return -EINVAL;
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 48d6ea7..527a880 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -388,6 +388,13 @@ enum mv88e6xxx_cap {
 	 */
 	MV88E6XXX_CAP_EEE,
 
+	/* Multi-chip Addressing Mode.
+	 * Some chips respond to only 2 registers of its own SMI device address
+	 * when it is non-zero, and use indirect access to internal registers.
+	 */
+	MV88E6XXX_CAP_SMI_CMD,		/* (0x00) SMI Command */
+	MV88E6XXX_CAP_SMI_DATA,		/* (0x01) SMI Data */
+
 	/* Switch Global 2 Registers.
 	 * The device contains a second set of global 16-bit registers.
 	 */
@@ -403,12 +410,6 @@ enum mv88e6xxx_cap {
 	MV88E6XXX_CAP_G2_EEPROM_CMD,	/* (0x14) EEPROM Command */
 	MV88E6XXX_CAP_G2_EEPROM_DATA,	/* (0x15) EEPROM Data */
 
-	/* Multi-chip Addressing Mode.
-	 * Some chips require an indirect SMI access when their SMI device
-	 * address is not zero. See SMI_CMD and SMI_DATA.
-	 */
-	MV88E6XXX_CAP_MULTI_CHIP,
-
 	/* PHY Polling Unit.
 	 * See GLOBAL_CONTROL_PPU_ENABLE and GLOBAL_STATUS_PPU_POLLING.
 	 */
@@ -441,6 +442,10 @@ enum mv88e6xxx_cap {
 
 /* Bitmask of capabilities */
 #define MV88E6XXX_FLAG_EEE		BIT(MV88E6XXX_CAP_EEE)
+
+#define MV88E6XXX_FLAG_SMI_CMD		BIT(MV88E6XXX_CAP_SMI_CMD)
+#define MV88E6XXX_FLAG_SMI_DATA		BIT(MV88E6XXX_CAP_SMI_DATA)
+
 #define MV88E6XXX_FLAG_GLOBAL2		BIT(MV88E6XXX_CAP_GLOBAL2)
 #define MV88E6XXX_FLAG_G2_MGMT_EN_2X	BIT(MV88E6XXX_CAP_G2_MGMT_EN_2X)
 #define MV88E6XXX_FLAG_G2_MGMT_EN_0X	BIT(MV88E6XXX_CAP_G2_MGMT_EN_0X)
@@ -452,7 +457,7 @@ enum mv88e6xxx_cap {
 #define MV88E6XXX_FLAG_G2_POT		BIT(MV88E6XXX_CAP_G2_POT)
 #define MV88E6XXX_FLAG_G2_EEPROM_CMD	BIT(MV88E6XXX_CAP_G2_EEPROM_CMD)
 #define MV88E6XXX_FLAG_G2_EEPROM_DATA	BIT(MV88E6XXX_CAP_G2_EEPROM_DATA)
-#define MV88E6XXX_FLAG_MULTI_CHIP	BIT(MV88E6XXX_CAP_MULTI_CHIP)
+
 #define MV88E6XXX_FLAG_PPU		BIT(MV88E6XXX_CAP_PPU)
 #define MV88E6XXX_FLAG_PPU_ACTIVE	BIT(MV88E6XXX_CAP_PPU_ACTIVE)
 #define MV88E6XXX_FLAG_SMI_PHY		BIT(MV88E6XXX_CAP_SMI_PHY)
@@ -471,6 +476,11 @@ enum mv88e6xxx_cap {
 	(MV88E6XXX_FLAG_G2_IRL_CMD |	\
 	 MV88E6XXX_FLAG_G2_IRL_DATA)
 
+/* Multi-chip Addressing Mode */
+#define MV88E6XXX_FLAGS_MULTI_CHIP	\
+	(MV88E6XXX_FLAG_SMI_CMD |	\
+	 MV88E6XXX_FLAG_SMI_DATA)
+
 /* Cross-chip Port VLAN Table */
 #define MV88E6XXX_FLAGS_PVT		\
 	(MV88E6XXX_FLAG_G2_PVT_ADDR |	\
@@ -479,20 +489,20 @@ enum mv88e6xxx_cap {
 #define MV88E6XXX_FLAGS_FAMILY_6095	\
 	(MV88E6XXX_FLAG_GLOBAL2 |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
-	 MV88E6XXX_FLAG_MULTI_CHIP |	\
 	 MV88E6XXX_FLAG_PPU |		\
-	 MV88E6XXX_FLAG_VTU)
+	 MV88E6XXX_FLAG_VTU |		\
+	 MV88E6XXX_FLAGS_MULTI_CHIP)
 
 #define MV88E6XXX_FLAGS_FAMILY_6097	\
 	(MV88E6XXX_FLAG_GLOBAL2 |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_2X |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
 	 MV88E6XXX_FLAG_G2_POT |	\
-	 MV88E6XXX_FLAG_MULTI_CHIP |	\
 	 MV88E6XXX_FLAG_PPU |		\
 	 MV88E6XXX_FLAG_STU |		\
 	 MV88E6XXX_FLAG_VTU |		\
 	 MV88E6XXX_FLAGS_IRL |		\
+	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
 	 MV88E6XXX_FLAGS_PVT)
 
 #define MV88E6XXX_FLAGS_FAMILY_6165	\
@@ -501,17 +511,17 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
 	 MV88E6XXX_FLAG_G2_SWITCH_MAC |	\
 	 MV88E6XXX_FLAG_G2_POT |	\
-	 MV88E6XXX_FLAG_MULTI_CHIP |	\
 	 MV88E6XXX_FLAG_STU |		\
 	 MV88E6XXX_FLAG_TEMP |		\
 	 MV88E6XXX_FLAG_VTU |		\
 	 MV88E6XXX_FLAGS_IRL |		\
+	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
 	 MV88E6XXX_FLAGS_PVT)
 
 #define MV88E6XXX_FLAGS_FAMILY_6185	\
 	(MV88E6XXX_FLAG_GLOBAL2 |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
-	 MV88E6XXX_FLAG_MULTI_CHIP |	\
+	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
 	 MV88E6XXX_FLAG_PPU |		\
 	 MV88E6XXX_FLAG_VTU)
 
@@ -522,7 +532,6 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
 	 MV88E6XXX_FLAG_G2_SWITCH_MAC |	\
 	 MV88E6XXX_FLAG_G2_POT |	\
-	 MV88E6XXX_FLAG_MULTI_CHIP |	\
 	 MV88E6XXX_FLAG_PPU_ACTIVE |	\
 	 MV88E6XXX_FLAG_SMI_PHY |	\
 	 MV88E6XXX_FLAG_TEMP |		\
@@ -530,6 +539,7 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAG_VTU |		\
 	 MV88E6XXX_FLAGS_EEPROM16 |	\
 	 MV88E6XXX_FLAGS_IRL |		\
+	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
 	 MV88E6XXX_FLAGS_PVT)
 
 #define MV88E6XXX_FLAGS_FAMILY_6351	\
@@ -538,13 +548,13 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
 	 MV88E6XXX_FLAG_G2_SWITCH_MAC |	\
 	 MV88E6XXX_FLAG_G2_POT |	\
-	 MV88E6XXX_FLAG_MULTI_CHIP |	\
 	 MV88E6XXX_FLAG_PPU_ACTIVE |	\
 	 MV88E6XXX_FLAG_SMI_PHY |	\
 	 MV88E6XXX_FLAG_STU |		\
 	 MV88E6XXX_FLAG_TEMP |		\
 	 MV88E6XXX_FLAG_VTU |		\
 	 MV88E6XXX_FLAGS_IRL |		\
+	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
 	 MV88E6XXX_FLAGS_PVT)
 
 #define MV88E6XXX_FLAGS_FAMILY_6352	\
@@ -554,7 +564,6 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
 	 MV88E6XXX_FLAG_G2_SWITCH_MAC |	\
 	 MV88E6XXX_FLAG_G2_POT |	\
-	 MV88E6XXX_FLAG_MULTI_CHIP |	\
 	 MV88E6XXX_FLAG_PPU_ACTIVE |	\
 	 MV88E6XXX_FLAG_SMI_PHY |	\
 	 MV88E6XXX_FLAG_STU |		\
@@ -563,6 +572,7 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAG_VTU |		\
 	 MV88E6XXX_FLAGS_EEPROM16 |	\
 	 MV88E6XXX_FLAGS_IRL |		\
+	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
 	 MV88E6XXX_FLAGS_PVT)
 
 struct mv88e6xxx_info {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH net-next 3/6] net: dsa: mv88e6xxx: rework Global2 SMI PHY access
  2016-08-15 21:18 [PATCH net-next 0/6] net: dsa: abstract PHY accesses Vivien Didelot
  2016-08-15 21:18 ` [PATCH net-next 1/6] net: dsa: mv88e6xxx: rename _mv88e6xxx_wait Vivien Didelot
  2016-08-15 21:18 ` [PATCH net-next 2/6] net: dsa: mv88e6xxx: describe Multi-chip registers Vivien Didelot
@ 2016-08-15 21:18 ` Vivien Didelot
  2016-08-15 21:19 ` [PATCH net-next 4/6] net: dsa: mv88e6xxx: abstract PHY ops Vivien Didelot
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Vivien Didelot @ 2016-08-15 21:18 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

Describe the presence of the Global2 SMI PHY registers, used to
indirectly access the internal SMI devices registers on some chips.

Also temporarily forward declare mv88e6xxx_g2_smi_phy_{read,write} to
use them in mv88e6xxx_mdio_{read,write}_indirect, before getting rid of
the later.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 97 +++++++++++++++++++++++------------
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 49 +++++++++---------
 2 files changed, 90 insertions(+), 56 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 44debf2..924a2af 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -841,52 +841,34 @@ static void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
 	mutex_unlock(&chip->reg_lock);
 }
 
-static int mv88e6xxx_mdio_wait(struct mv88e6xxx_chip *chip)
-{
-	return mv88e6xxx_wait(chip, REG_GLOBAL2, GLOBAL2_SMI_OP,
-			      GLOBAL2_SMI_OP_BUSY);
-}
-
 static int _mv88e6xxx_atu_wait(struct mv88e6xxx_chip *chip)
 {
 	return mv88e6xxx_wait(chip, REG_GLOBAL, GLOBAL_ATU_OP,
 			      GLOBAL_ATU_OP_BUSY);
 }
 
+static int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip, int addr,
+				     int reg, u16 *val);
+static int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip, int addr,
+				      int reg, u16 val);
+
 static int mv88e6xxx_mdio_read_indirect(struct mv88e6xxx_chip *chip,
 					int addr, int regnum)
 {
-	int ret;
-
-	ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL2, GLOBAL2_SMI_OP,
-				   GLOBAL2_SMI_OP_22_READ | (addr << 5) |
-				   regnum);
-	if (ret < 0)
-		return ret;
-
-	ret = mv88e6xxx_mdio_wait(chip);
-	if (ret < 0)
-		return ret;
+	u16 val;
+	int err;
 
-	ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL2, GLOBAL2_SMI_DATA);
+	err = mv88e6xxx_g2_smi_phy_read(chip, addr, regnum, &val);
+	if (err)
+		return err;
 
-	return ret;
+	return val;
 }
 
 static int mv88e6xxx_mdio_write_indirect(struct mv88e6xxx_chip *chip,
 					 int addr, int regnum, u16 val)
 {
-	int ret;
-
-	ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL2, GLOBAL2_SMI_DATA, val);
-	if (ret < 0)
-		return ret;
-
-	ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL2, GLOBAL2_SMI_OP,
-				   GLOBAL2_SMI_OP_22_WRITE | (addr << 5) |
-				   regnum);
-
-	return mv88e6xxx_mdio_wait(chip);
+	return mv88e6xxx_g2_smi_phy_write(chip, addr, regnum, val);
 }
 
 static int mv88e6xxx_get_eee(struct dsa_switch *ds, int port,
@@ -3057,6 +3039,57 @@ static int mv88e6xxx_g2_eeprom_write16(struct mv88e6xxx_chip *chip,
 	return mv88e6xxx_g2_eeprom_cmd(chip, cmd);
 }
 
+static int mv88e6xxx_g2_smi_phy_wait(struct mv88e6xxx_chip *chip)
+{
+	return mv88e6xxx_wait(chip, REG_GLOBAL2, GLOBAL2_SMI_PHY_CMD,
+			      GLOBAL2_SMI_PHY_CMD_BUSY);
+}
+
+static int mv88e6xxx_g2_smi_phy_cmd(struct mv88e6xxx_chip *chip, u16 cmd)
+{
+	int err;
+
+	err = mv88e6xxx_write(chip, REG_GLOBAL2, GLOBAL2_SMI_PHY_CMD, cmd);
+	if (err)
+		return err;
+
+	return mv88e6xxx_g2_smi_phy_wait(chip);
+}
+
+static int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip, int addr,
+				     int reg, u16 *val)
+{
+	u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_READ_DATA | (addr << 5) | reg;
+	int err;
+
+	err = mv88e6xxx_g2_smi_phy_wait(chip);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_g2_smi_phy_cmd(chip, cmd);
+	if (err)
+		return err;
+
+	return mv88e6xxx_read(chip, REG_GLOBAL2, GLOBAL2_SMI_PHY_DATA, val);
+}
+
+static int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip, int addr,
+				      int reg, u16 val)
+{
+	u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA | (addr << 5) | reg;
+	int err;
+
+	err = mv88e6xxx_g2_smi_phy_wait(chip);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_write(chip, REG_GLOBAL2, GLOBAL2_SMI_PHY_DATA, val);
+	if (err)
+		return err;
+
+	return mv88e6xxx_g2_smi_phy_cmd(chip, cmd);
+}
+
 static int mv88e6xxx_g2_setup(struct mv88e6xxx_chip *chip)
 {
 	u16 reg;
@@ -3236,7 +3269,7 @@ static int mv88e6xxx_mdio_read(struct mii_bus *bus, int port, int regnum)
 
 	if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_PPU))
 		ret = mv88e6xxx_mdio_read_ppu(chip, addr, regnum);
-	else if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_SMI_PHY))
+	else if (mv88e6xxx_has(chip, MV88E6XXX_FLAGS_SMI_PHY))
 		ret = mv88e6xxx_mdio_read_indirect(chip, addr, regnum);
 	else
 		ret = mv88e6xxx_mdio_read_direct(chip, addr, regnum);
@@ -3259,7 +3292,7 @@ static int mv88e6xxx_mdio_write(struct mii_bus *bus, int port, int regnum,
 
 	if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_PPU))
 		ret = mv88e6xxx_mdio_write_ppu(chip, addr, regnum, val);
-	else if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_SMI_PHY))
+	else if (mv88e6xxx_has(chip, MV88E6XXX_FLAGS_SMI_PHY))
 		ret = mv88e6xxx_mdio_write_indirect(chip, addr, regnum, val);
 	else
 		ret = mv88e6xxx_mdio_write_direct(chip, addr, regnum, val);
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 527a880..8be0f36 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -329,17 +329,16 @@
 #define GLOBAL2_EEPROM_DATA	0x15
 #define GLOBAL2_PTP_AVB_OP	0x16
 #define GLOBAL2_PTP_AVB_DATA	0x17
-#define GLOBAL2_SMI_OP		0x18
-#define GLOBAL2_SMI_OP_BUSY		BIT(15)
-#define GLOBAL2_SMI_OP_CLAUSE_22	BIT(12)
-#define GLOBAL2_SMI_OP_22_WRITE		((1 << 10) | GLOBAL2_SMI_OP_BUSY | \
-					 GLOBAL2_SMI_OP_CLAUSE_22)
-#define GLOBAL2_SMI_OP_22_READ		((2 << 10) | GLOBAL2_SMI_OP_BUSY | \
-					 GLOBAL2_SMI_OP_CLAUSE_22)
-#define GLOBAL2_SMI_OP_45_WRITE_ADDR	((0 << 10) | GLOBAL2_SMI_OP_BUSY)
-#define GLOBAL2_SMI_OP_45_WRITE_DATA	((1 << 10) | GLOBAL2_SMI_OP_BUSY)
-#define GLOBAL2_SMI_OP_45_READ_DATA	((2 << 10) | GLOBAL2_SMI_OP_BUSY)
-#define GLOBAL2_SMI_DATA	0x19
+#define GLOBAL2_SMI_PHY_CMD			0x18
+#define GLOBAL2_SMI_PHY_CMD_BUSY		BIT(15)
+#define GLOBAL2_SMI_PHY_CMD_MODE_22		BIT(12)
+#define GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA	((0x1 << 10) | \
+						 GLOBAL2_SMI_PHY_CMD_MODE_22 | \
+						 GLOBAL2_SMI_PHY_CMD_BUSY)
+#define GLOBAL2_SMI_PHY_CMD_OP_22_READ_DATA	((0x2 << 10) | \
+						 GLOBAL2_SMI_PHY_CMD_MODE_22 | \
+						 GLOBAL2_SMI_PHY_CMD_BUSY)
+#define GLOBAL2_SMI_PHY_DATA			0x19
 #define GLOBAL2_SCRATCH_MISC	0x1a
 #define GLOBAL2_SCRATCH_BUSY		BIT(15)
 #define GLOBAL2_SCRATCH_REGISTER_SHIFT	8
@@ -409,6 +408,8 @@ enum mv88e6xxx_cap {
 	MV88E6XXX_CAP_G2_POT,		/* (0x0f) Priority Override Table */
 	MV88E6XXX_CAP_G2_EEPROM_CMD,	/* (0x14) EEPROM Command */
 	MV88E6XXX_CAP_G2_EEPROM_DATA,	/* (0x15) EEPROM Data */
+	MV88E6XXX_CAP_G2_SMI_PHY_CMD,	/* (0x18) SMI PHY Command */
+	MV88E6XXX_CAP_G2_SMI_PHY_DATA,	/* (0x19) SMI PHY Data */
 
 	/* PHY Polling Unit.
 	 * See GLOBAL_CONTROL_PPU_ENABLE and GLOBAL_STATUS_PPU_POLLING.
@@ -416,12 +417,6 @@ enum mv88e6xxx_cap {
 	MV88E6XXX_CAP_PPU,
 	MV88E6XXX_CAP_PPU_ACTIVE,
 
-	/* SMI PHY Command and Data registers.
-	 * This requires an indirect access to PHY registers through
-	 * GLOBAL2_SMI_OP, otherwise direct access to PHY registers is done.
-	 */
-	MV88E6XXX_CAP_SMI_PHY,
-
 	/* Per VLAN Spanning Tree Unit (STU).
 	 * The Port State database, if present, is accessed through VTU
 	 * operations and dedicated SID registers. See GLOBAL_VTU_SID.
@@ -457,10 +452,11 @@ enum mv88e6xxx_cap {
 #define MV88E6XXX_FLAG_G2_POT		BIT(MV88E6XXX_CAP_G2_POT)
 #define MV88E6XXX_FLAG_G2_EEPROM_CMD	BIT(MV88E6XXX_CAP_G2_EEPROM_CMD)
 #define MV88E6XXX_FLAG_G2_EEPROM_DATA	BIT(MV88E6XXX_CAP_G2_EEPROM_DATA)
+#define MV88E6XXX_FLAG_G2_SMI_PHY_CMD	BIT(MV88E6XXX_CAP_G2_SMI_PHY_CMD)
+#define MV88E6XXX_FLAG_G2_SMI_PHY_DATA	BIT(MV88E6XXX_CAP_G2_SMI_PHY_DATA)
 
 #define MV88E6XXX_FLAG_PPU		BIT(MV88E6XXX_CAP_PPU)
 #define MV88E6XXX_FLAG_PPU_ACTIVE	BIT(MV88E6XXX_CAP_PPU_ACTIVE)
-#define MV88E6XXX_FLAG_SMI_PHY		BIT(MV88E6XXX_CAP_SMI_PHY)
 #define MV88E6XXX_FLAG_STU		BIT(MV88E6XXX_CAP_STU)
 #define MV88E6XXX_FLAG_TEMP		BIT(MV88E6XXX_CAP_TEMP)
 #define MV88E6XXX_FLAG_TEMP_LIMIT	BIT(MV88E6XXX_CAP_TEMP_LIMIT)
@@ -486,6 +482,11 @@ enum mv88e6xxx_cap {
 	(MV88E6XXX_FLAG_G2_PVT_ADDR |	\
 	 MV88E6XXX_FLAG_G2_PVT_DATA)
 
+/* Indirect PHY access via Global2 SMI PHY registers */
+#define MV88E6XXX_FLAGS_SMI_PHY		\
+	(MV88E6XXX_FLAG_G2_SMI_PHY_CMD |\
+	 MV88E6XXX_FLAG_G2_SMI_PHY_DATA)
+
 #define MV88E6XXX_FLAGS_FAMILY_6095	\
 	(MV88E6XXX_FLAG_GLOBAL2 |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
@@ -533,14 +534,14 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAG_G2_SWITCH_MAC |	\
 	 MV88E6XXX_FLAG_G2_POT |	\
 	 MV88E6XXX_FLAG_PPU_ACTIVE |	\
-	 MV88E6XXX_FLAG_SMI_PHY |	\
 	 MV88E6XXX_FLAG_TEMP |		\
 	 MV88E6XXX_FLAG_TEMP_LIMIT |	\
 	 MV88E6XXX_FLAG_VTU |		\
 	 MV88E6XXX_FLAGS_EEPROM16 |	\
 	 MV88E6XXX_FLAGS_IRL |		\
 	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
-	 MV88E6XXX_FLAGS_PVT)
+	 MV88E6XXX_FLAGS_PVT |		\
+	 MV88E6XXX_FLAGS_SMI_PHY)
 
 #define MV88E6XXX_FLAGS_FAMILY_6351	\
 	(MV88E6XXX_FLAG_GLOBAL2 |	\
@@ -549,13 +550,13 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAG_G2_SWITCH_MAC |	\
 	 MV88E6XXX_FLAG_G2_POT |	\
 	 MV88E6XXX_FLAG_PPU_ACTIVE |	\
-	 MV88E6XXX_FLAG_SMI_PHY |	\
 	 MV88E6XXX_FLAG_STU |		\
 	 MV88E6XXX_FLAG_TEMP |		\
 	 MV88E6XXX_FLAG_VTU |		\
 	 MV88E6XXX_FLAGS_IRL |		\
 	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
-	 MV88E6XXX_FLAGS_PVT)
+	 MV88E6XXX_FLAGS_PVT |		\
+	 MV88E6XXX_FLAGS_SMI_PHY)
 
 #define MV88E6XXX_FLAGS_FAMILY_6352	\
 	(MV88E6XXX_FLAG_EEE |		\
@@ -565,7 +566,6 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAG_G2_SWITCH_MAC |	\
 	 MV88E6XXX_FLAG_G2_POT |	\
 	 MV88E6XXX_FLAG_PPU_ACTIVE |	\
-	 MV88E6XXX_FLAG_SMI_PHY |	\
 	 MV88E6XXX_FLAG_STU |		\
 	 MV88E6XXX_FLAG_TEMP |		\
 	 MV88E6XXX_FLAG_TEMP_LIMIT |	\
@@ -573,7 +573,8 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAGS_EEPROM16 |	\
 	 MV88E6XXX_FLAGS_IRL |		\
 	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
-	 MV88E6XXX_FLAGS_PVT)
+	 MV88E6XXX_FLAGS_PVT |		\
+	 MV88E6XXX_FLAGS_SMI_PHY)
 
 struct mv88e6xxx_info {
 	enum mv88e6xxx_family family;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH net-next 4/6] net: dsa: mv88e6xxx: abstract PHY ops
  2016-08-15 21:18 [PATCH net-next 0/6] net: dsa: abstract PHY accesses Vivien Didelot
                   ` (2 preceding siblings ...)
  2016-08-15 21:18 ` [PATCH net-next 3/6] net: dsa: mv88e6xxx: rework Global2 SMI PHY access Vivien Didelot
@ 2016-08-15 21:19 ` Vivien Didelot
  2016-08-15 21:19 ` [PATCH net-next 5/6] net: dsa: mv88e6xxx: describe PHY page and SerDes Vivien Didelot
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Vivien Didelot @ 2016-08-15 21:19 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

Old chips use a direct access to the PHY devices registers. Next chips
have a PHY Polling Unit (PPU) which needs to be disabled before
accessing PHY registers. Newer chips have an indirect access to the PHY
devices so that disabling the PPU is not necessary.

Introduce a new phy_ops structure in the chip to describe the required
PHY access routines.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 131 +++++++++++++++++++++-------------
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |   1 +
 2 files changed, 81 insertions(+), 51 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 924a2af..3b0bc88 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -216,6 +216,28 @@ static int mv88e6xxx_write(struct mv88e6xxx_chip *chip,
 	return 0;
 }
 
+static int mv88e6xxx_phy_read(struct mv88e6xxx_chip *chip, int phy,
+			      int reg, u16 *val)
+{
+	int addr = phy; /* PHY devices addresses start at 0x0 */
+
+	if (!chip->phy_ops)
+		return -EOPNOTSUPP;
+
+	return chip->phy_ops->read(chip, addr, reg, val);
+}
+
+static int mv88e6xxx_phy_write(struct mv88e6xxx_chip *chip, int phy,
+			       int reg, u16 val)
+{
+	int addr = phy; /* PHY devices addresses start at 0x0 */
+
+	if (!chip->phy_ops)
+		return -EOPNOTSUPP;
+
+	return chip->phy_ops->write(chip, addr, reg, val);
+}
+
 static int mv88e6xxx_wait(struct mv88e6xxx_chip *chip, int addr, int reg,
 			  u16 mask)
 {
@@ -422,34 +444,39 @@ static void mv88e6xxx_ppu_state_init(struct mv88e6xxx_chip *chip)
 	chip->ppu_timer.function = mv88e6xxx_ppu_reenable_timer;
 }
 
-static int mv88e6xxx_mdio_read_ppu(struct mv88e6xxx_chip *chip, int addr,
-				   int regnum)
+static int mv88e6xxx_phy_ppu_read(struct mv88e6xxx_chip *chip, int addr,
+				  int reg, u16 *val)
 {
-	int ret;
+	int err;
 
-	ret = mv88e6xxx_ppu_access_get(chip);
-	if (ret >= 0) {
-		ret = _mv88e6xxx_reg_read(chip, addr, regnum);
+	err = mv88e6xxx_ppu_access_get(chip);
+	if (!err) {
+		err = mv88e6xxx_read(chip, addr, reg, val);
 		mv88e6xxx_ppu_access_put(chip);
 	}
 
-	return ret;
+	return err;
 }
 
-static int mv88e6xxx_mdio_write_ppu(struct mv88e6xxx_chip *chip, int addr,
-				    int regnum, u16 val)
+static int mv88e6xxx_phy_ppu_write(struct mv88e6xxx_chip *chip, int addr,
+				   int reg, u16 val)
 {
-	int ret;
+	int err;
 
-	ret = mv88e6xxx_ppu_access_get(chip);
-	if (ret >= 0) {
-		ret = _mv88e6xxx_reg_write(chip, addr, regnum, val);
+	err = mv88e6xxx_ppu_access_get(chip);
+	if (!err) {
+		err = mv88e6xxx_write(chip, addr, reg, val);
 		mv88e6xxx_ppu_access_put(chip);
 	}
 
-	return ret;
+	return err;
 }
 
+static const struct mv88e6xxx_ops mv88e6xxx_phy_ppu_ops = {
+	.read = mv88e6xxx_phy_ppu_read,
+	.write = mv88e6xxx_phy_ppu_write,
+};
+
 static bool mv88e6xxx_6065_family(struct mv88e6xxx_chip *chip)
 {
 	return chip->info->family == MV88E6XXX_FAMILY_6065;
@@ -3090,6 +3117,11 @@ static int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip, int addr,
 	return mv88e6xxx_g2_smi_phy_cmd(chip, cmd);
 }
 
+static const struct mv88e6xxx_ops mv88e6xxx_g2_smi_phy_ops = {
+	.read = mv88e6xxx_g2_smi_phy_read,
+	.write = mv88e6xxx_g2_smi_phy_write,
+};
+
 static int mv88e6xxx_g2_setup(struct mv88e6xxx_chip *chip)
 {
 	u16 reg;
@@ -3249,56 +3281,35 @@ static int mv88e6xxx_mdio_page_write(struct dsa_switch *ds, int port, int page,
 	return ret;
 }
 
-static int mv88e6xxx_port_to_mdio_addr(struct mv88e6xxx_chip *chip, int port)
-{
-	if (port >= 0 && port < chip->info->num_ports)
-		return port;
-	return -EINVAL;
-}
-
-static int mv88e6xxx_mdio_read(struct mii_bus *bus, int port, int regnum)
+static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
 {
 	struct mv88e6xxx_chip *chip = bus->priv;
-	int addr = mv88e6xxx_port_to_mdio_addr(chip, port);
-	int ret;
+	u16 val;
+	int err;
 
-	if (addr < 0)
+	if (phy >= chip->info->num_ports)
 		return 0xffff;
 
 	mutex_lock(&chip->reg_lock);
-
-	if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_PPU))
-		ret = mv88e6xxx_mdio_read_ppu(chip, addr, regnum);
-	else if (mv88e6xxx_has(chip, MV88E6XXX_FLAGS_SMI_PHY))
-		ret = mv88e6xxx_mdio_read_indirect(chip, addr, regnum);
-	else
-		ret = mv88e6xxx_mdio_read_direct(chip, addr, regnum);
-
+	err = mv88e6xxx_phy_read(chip, phy, reg, &val);
 	mutex_unlock(&chip->reg_lock);
-	return ret;
+
+	return err ? err : val;
 }
 
-static int mv88e6xxx_mdio_write(struct mii_bus *bus, int port, int regnum,
-				u16 val)
+static int mv88e6xxx_mdio_write(struct mii_bus *bus, int phy, int reg, u16 val)
 {
 	struct mv88e6xxx_chip *chip = bus->priv;
-	int addr = mv88e6xxx_port_to_mdio_addr(chip, port);
-	int ret;
+	int err;
 
-	if (addr < 0)
+	if (phy >= chip->info->num_ports)
 		return 0xffff;
 
 	mutex_lock(&chip->reg_lock);
-
-	if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_PPU))
-		ret = mv88e6xxx_mdio_write_ppu(chip, addr, regnum, val);
-	else if (mv88e6xxx_has(chip, MV88E6XXX_FLAGS_SMI_PHY))
-		ret = mv88e6xxx_mdio_write_indirect(chip, addr, regnum, val);
-	else
-		ret = mv88e6xxx_mdio_write_direct(chip, addr, regnum, val);
-
+	err = mv88e6xxx_phy_write(chip, phy, reg, val);
 	mutex_unlock(&chip->reg_lock);
-	return ret;
+
+	return err;
 }
 
 static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
@@ -3308,9 +3319,6 @@ static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
 	struct mii_bus *bus;
 	int err;
 
-	if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_PPU))
-		mv88e6xxx_ppu_state_init(chip);
-
 	if (np)
 		chip->mdio_np = of_get_child_by_name(np, "mdio");
 
@@ -3907,6 +3915,23 @@ static struct mv88e6xxx_chip *mv88e6xxx_alloc_chip(struct device *dev)
 	return chip;
 }
 
+static const struct mv88e6xxx_ops mv88e6xxx_phy_ops = {
+	.read = mv88e6xxx_read,
+	.write = mv88e6xxx_write,
+};
+
+static void mv88e6xxx_phy_init(struct mv88e6xxx_chip *chip)
+{
+	if (mv88e6xxx_has(chip, MV88E6XXX_FLAGS_SMI_PHY)) {
+		chip->phy_ops = &mv88e6xxx_g2_smi_phy_ops;
+	} else if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_PPU)) {
+		chip->phy_ops = &mv88e6xxx_phy_ppu_ops;
+		mv88e6xxx_ppu_state_init(chip);
+	} else {
+		chip->phy_ops = &mv88e6xxx_phy_ops;
+	}
+}
+
 static int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip,
 			      struct mii_bus *bus, int sw_addr)
 {
@@ -3954,6 +3979,8 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 	if (err)
 		goto free;
 
+	mv88e6xxx_phy_init(chip);
+
 	err = mv88e6xxx_mdio_register(chip, NULL);
 	if (err)
 		goto free;
@@ -4055,6 +4082,8 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	if (err)
 		return err;
 
+	mv88e6xxx_phy_init(chip);
+
 	chip->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
 	if (IS_ERR(chip->reset))
 		return PTR_ERR(chip->reset);
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 8be0f36..1dd96e7 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -634,6 +634,7 @@ struct mv88e6xxx_chip {
 	/* Handles automatic disabling and re-enabling of the PHY
 	 * polling unit.
 	 */
+	const struct mv88e6xxx_ops *phy_ops;
 	struct mutex		ppu_mutex;
 	int			ppu_disabled;
 	struct work_struct	ppu_work;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH net-next 5/6] net: dsa: mv88e6xxx: describe PHY page and SerDes
  2016-08-15 21:18 [PATCH net-next 0/6] net: dsa: abstract PHY accesses Vivien Didelot
                   ` (3 preceding siblings ...)
  2016-08-15 21:19 ` [PATCH net-next 4/6] net: dsa: mv88e6xxx: abstract PHY ops Vivien Didelot
@ 2016-08-15 21:19 ` Vivien Didelot
  2016-08-16  1:16   ` Andrew Lunn
  2016-08-15 21:19 ` [PATCH net-next 6/6] net: dsa: mv88e6xxx: use the new PHY API Vivien Didelot
  2016-08-15 23:44 ` [PATCH net-next 0/6] net: dsa: abstract PHY accesses David Miller
  6 siblings, 1 reply; 10+ messages in thread
From: Vivien Didelot @ 2016-08-15 21:19 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

Add mv88e6xxx_phy_page_{read,write} routines and use them to access the
SerDes PHY device registers.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 95 +++++++++++++++++++++++++++++------
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 27 ++++++++--
 2 files changed, 105 insertions(+), 17 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 3b0bc88..faa0751 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -238,6 +238,74 @@ static int mv88e6xxx_phy_write(struct mv88e6xxx_chip *chip, int phy,
 	return chip->phy_ops->write(chip, addr, reg, val);
 }
 
+static int mv88e6xxx_phy_page_get(struct mv88e6xxx_chip *chip, int phy, u8 page)
+{
+	if (!mv88e6xxx_has(chip, MV88E6XXX_FLAG_PHY_PAGE))
+		return -EOPNOTSUPP;
+
+	return mv88e6xxx_phy_write(chip, phy, PHY_PAGE, page);
+}
+
+static void mv88e6xxx_phy_page_put(struct mv88e6xxx_chip *chip, int phy)
+{
+	int err;
+
+	/* Restore PHY page Copper 0x0 for access via the registered MDIO bus */
+	err = mv88e6xxx_phy_write(chip, phy, PHY_PAGE, PHY_PAGE_COPPER);
+	if (unlikely(err)) {
+		dev_err(chip->dev, "failed to restore PHY %d page Copper (%d)\n",
+			phy, err);
+	}
+}
+
+static int mv88e6xxx_phy_page_read(struct mv88e6xxx_chip *chip, int phy,
+				   u8 page, int reg, u16 *val)
+{
+	int err;
+
+	/* There is no paging for registers 22 */
+	if (reg == PHY_PAGE)
+		return -EINVAL;
+
+	err = mv88e6xxx_phy_page_get(chip, phy, page);
+	if (!err) {
+		err = mv88e6xxx_phy_read(chip, phy, reg, val);
+		mv88e6xxx_phy_page_put(chip, phy);
+	}
+
+	return err;
+}
+
+static int mv88e6xxx_phy_page_write(struct mv88e6xxx_chip *chip, int phy,
+				    u8 page, int reg, u16 val)
+{
+	int err;
+
+	/* There is no paging for registers 22 */
+	if (reg == PHY_PAGE)
+		return -EINVAL;
+
+	err = mv88e6xxx_phy_page_get(chip, phy, page);
+	if (!err) {
+		err = mv88e6xxx_phy_write(chip, phy, PHY_PAGE, page);
+		mv88e6xxx_phy_page_put(chip, phy);
+	}
+
+	return err;
+}
+
+static int mv88e6xxx_serdes_read(struct mv88e6xxx_chip *chip, int reg, u16 *val)
+{
+	return mv88e6xxx_phy_page_read(chip, ADDR_SERDES, SERDES_PAGE_FIBER,
+				       reg, val);
+}
+
+static int mv88e6xxx_serdes_write(struct mv88e6xxx_chip *chip, int reg, u16 val)
+{
+	return mv88e6xxx_phy_page_write(chip, ADDR_SERDES, SERDES_PAGE_FIBER,
+					reg, val);
+}
+
 static int mv88e6xxx_wait(struct mv88e6xxx_chip *chip, int addr, int reg,
 			  u16 mask)
 {
@@ -2408,23 +2476,22 @@ static int mv88e6xxx_switch_reset(struct mv88e6xxx_chip *chip)
 	return ret;
 }
 
-static int mv88e6xxx_power_on_serdes(struct mv88e6xxx_chip *chip)
+static int mv88e6xxx_serdes_power_on(struct mv88e6xxx_chip *chip)
 {
-	int ret;
+	u16 val;
+	int err;
 
-	ret = _mv88e6xxx_mdio_page_read(chip, REG_FIBER_SERDES,
-					PAGE_FIBER_SERDES, MII_BMCR);
-	if (ret < 0)
-		return ret;
+	/* Clear Power Down bit */
+	err = mv88e6xxx_serdes_read(chip, MII_BMCR, &val);
+	if (err)
+		return err;
 
-	if (ret & BMCR_PDOWN) {
-		ret &= ~BMCR_PDOWN;
-		ret = _mv88e6xxx_mdio_page_write(chip, REG_FIBER_SERDES,
-						 PAGE_FIBER_SERDES, MII_BMCR,
-						 ret);
+	if (val & BMCR_PDOWN) {
+		val &= ~BMCR_PDOWN;
+		err = mv88e6xxx_serdes_write(chip, MII_BMCR, val);
 	}
 
-	return ret;
+	return err;
 }
 
 static int mv88e6xxx_port_read(struct mv88e6xxx_chip *chip, int port,
@@ -2547,7 +2614,7 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 	/* If this port is connected to a SerDes, make sure the SerDes is not
 	 * powered down.
 	 */
-	if (mv88e6xxx_6352_family(chip)) {
+	if (mv88e6xxx_has(chip, MV88E6XXX_FLAGS_SERDES)) {
 		ret = _mv88e6xxx_reg_read(chip, REG_PORT(port), PORT_STATUS);
 		if (ret < 0)
 			return ret;
@@ -2555,7 +2622,7 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 		if ((ret == PORT_STATUS_CMODE_100BASE_X) ||
 		    (ret == PORT_STATUS_CMODE_1000BASE_X) ||
 		    (ret == PORT_STATUS_CMODE_SGMII)) {
-			ret = mv88e6xxx_power_on_serdes(chip);
+			ret = mv88e6xxx_serdes_power_on(chip);
 			if (ret < 0)
 				return ret;
 		}
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 1dd96e7..1f9bab5 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -30,9 +30,12 @@
 #define SMI_CMD_OP_45_READ_DATA_INC	((3 << 10) | SMI_CMD_BUSY)
 #define SMI_DATA		0x01
 
-/* Fiber/SERDES Registers are located at SMI address F, page 1 */
-#define REG_FIBER_SERDES	0x0f
-#define PAGE_FIBER_SERDES	0x01
+/* PHY Registers */
+#define PHY_PAGE		0x16
+#define PHY_PAGE_COPPER		0x00
+
+#define ADDR_SERDES		0x0f
+#define SERDES_PAGE_FIBER	0x01
 
 #define REG_PORT(p)		(0x10 + (p))
 #define PORT_STATUS		0x00
@@ -394,6 +397,14 @@ enum mv88e6xxx_cap {
 	MV88E6XXX_CAP_SMI_CMD,		/* (0x00) SMI Command */
 	MV88E6XXX_CAP_SMI_DATA,		/* (0x01) SMI Data */
 
+	/* PHY Registers.
+	 */
+	MV88E6XXX_CAP_PHY_PAGE,		/* (0x16) Page Register */
+
+	/* Fiber/SERDES Registers (SMI address F).
+	 */
+	MV88E6XXX_CAP_SERDES,
+
 	/* Switch Global 2 Registers.
 	 * The device contains a second set of global 16-bit registers.
 	 */
@@ -441,6 +452,10 @@ enum mv88e6xxx_cap {
 #define MV88E6XXX_FLAG_SMI_CMD		BIT(MV88E6XXX_CAP_SMI_CMD)
 #define MV88E6XXX_FLAG_SMI_DATA		BIT(MV88E6XXX_CAP_SMI_DATA)
 
+#define MV88E6XXX_FLAG_PHY_PAGE		BIT(MV88E6XXX_CAP_PHY_PAGE)
+
+#define MV88E6XXX_FLAG_SERDES		BIT(MV88E6XXX_CAP_SERDES)
+
 #define MV88E6XXX_FLAG_GLOBAL2		BIT(MV88E6XXX_CAP_GLOBAL2)
 #define MV88E6XXX_FLAG_G2_MGMT_EN_2X	BIT(MV88E6XXX_CAP_G2_MGMT_EN_2X)
 #define MV88E6XXX_FLAG_G2_MGMT_EN_0X	BIT(MV88E6XXX_CAP_G2_MGMT_EN_0X)
@@ -482,6 +497,11 @@ enum mv88e6xxx_cap {
 	(MV88E6XXX_FLAG_G2_PVT_ADDR |	\
 	 MV88E6XXX_FLAG_G2_PVT_DATA)
 
+/* Fiber/SERDES Registers at SMI address F, page 1 */
+#define MV88E6XXX_FLAGS_SERDES		\
+	(MV88E6XXX_FLAG_PHY_PAGE |	\
+	 MV88E6XXX_FLAG_SERDES)
+
 /* Indirect PHY access via Global2 SMI PHY registers */
 #define MV88E6XXX_FLAGS_SMI_PHY		\
 	(MV88E6XXX_FLAG_G2_SMI_PHY_CMD |\
@@ -574,6 +594,7 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAGS_IRL |		\
 	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
 	 MV88E6XXX_FLAGS_PVT |		\
+	 MV88E6XXX_FLAGS_SERDES |	\
 	 MV88E6XXX_FLAGS_SMI_PHY)
 
 struct mv88e6xxx_info {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH net-next 6/6] net: dsa: mv88e6xxx: use the new PHY API
  2016-08-15 21:18 [PATCH net-next 0/6] net: dsa: abstract PHY accesses Vivien Didelot
                   ` (4 preceding siblings ...)
  2016-08-15 21:19 ` [PATCH net-next 5/6] net: dsa: mv88e6xxx: describe PHY page and SerDes Vivien Didelot
@ 2016-08-15 21:19 ` Vivien Didelot
  2016-08-15 23:44 ` [PATCH net-next 0/6] net: dsa: abstract PHY accesses David Miller
  6 siblings, 0 replies; 10+ messages in thread
From: Vivien Didelot @ 2016-08-15 21:19 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

This commit replaces every MDIO direct or indirect access with the new
generic mv88e6xxx_phy_* routines.

This allows us to get rid of the mv88e6xxx_mdio_{read,write}_{,in}direct
and {_,}mv88e6xxx_mdio_page_{read,write} functions.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 185 +++++++++++----------------------------
 1 file changed, 50 insertions(+), 135 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index faa0751..a230fcb 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -372,22 +372,6 @@ static int _mv88e6xxx_reg_write(struct mv88e6xxx_chip *chip, int addr,
 	return mv88e6xxx_write(chip, addr, reg, val);
 }
 
-static int mv88e6xxx_mdio_read_direct(struct mv88e6xxx_chip *chip,
-				      int addr, int regnum)
-{
-	if (addr >= 0)
-		return _mv88e6xxx_reg_read(chip, addr, regnum);
-	return 0xffff;
-}
-
-static int mv88e6xxx_mdio_write_direct(struct mv88e6xxx_chip *chip,
-				       int addr, int regnum, u16 val)
-{
-	if (addr >= 0)
-		return _mv88e6xxx_reg_write(chip, addr, regnum, val);
-	return 0;
-}
-
 static int mv88e6xxx_ppu_disable(struct mv88e6xxx_chip *chip)
 {
 	int ret;
@@ -942,87 +926,63 @@ static int _mv88e6xxx_atu_wait(struct mv88e6xxx_chip *chip)
 			      GLOBAL_ATU_OP_BUSY);
 }
 
-static int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip, int addr,
-				     int reg, u16 *val);
-static int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip, int addr,
-				      int reg, u16 val);
-
-static int mv88e6xxx_mdio_read_indirect(struct mv88e6xxx_chip *chip,
-					int addr, int regnum)
-{
-	u16 val;
-	int err;
-
-	err = mv88e6xxx_g2_smi_phy_read(chip, addr, regnum, &val);
-	if (err)
-		return err;
-
-	return val;
-}
-
-static int mv88e6xxx_mdio_write_indirect(struct mv88e6xxx_chip *chip,
-					 int addr, int regnum, u16 val)
-{
-	return mv88e6xxx_g2_smi_phy_write(chip, addr, regnum, val);
-}
-
 static int mv88e6xxx_get_eee(struct dsa_switch *ds, int port,
 			     struct ethtool_eee *e)
 {
 	struct mv88e6xxx_chip *chip = ds_to_priv(ds);
-	int reg;
+	u16 reg;
+	int err;
 
 	if (!mv88e6xxx_has(chip, MV88E6XXX_FLAG_EEE))
 		return -EOPNOTSUPP;
 
 	mutex_lock(&chip->reg_lock);
 
-	reg = mv88e6xxx_mdio_read_indirect(chip, port, 16);
-	if (reg < 0)
+	err = mv88e6xxx_phy_read(chip, port, 16, &reg);
+	if (err)
 		goto out;
 
 	e->eee_enabled = !!(reg & 0x0200);
 	e->tx_lpi_enabled = !!(reg & 0x0100);
 
-	reg = _mv88e6xxx_reg_read(chip, REG_PORT(port), PORT_STATUS);
-	if (reg < 0)
+	err = mv88e6xxx_read(chip, REG_PORT(port), PORT_STATUS, &reg);
+	if (err)
 		goto out;
 
 	e->eee_active = !!(reg & PORT_STATUS_EEE);
-	reg = 0;
-
 out:
 	mutex_unlock(&chip->reg_lock);
-	return reg;
+
+	return err;
 }
 
 static int mv88e6xxx_set_eee(struct dsa_switch *ds, int port,
 			     struct phy_device *phydev, struct ethtool_eee *e)
 {
 	struct mv88e6xxx_chip *chip = ds_to_priv(ds);
-	int reg;
-	int ret;
+	u16 reg;
+	int err;
 
 	if (!mv88e6xxx_has(chip, MV88E6XXX_FLAG_EEE))
 		return -EOPNOTSUPP;
 
 	mutex_lock(&chip->reg_lock);
 
-	ret = mv88e6xxx_mdio_read_indirect(chip, port, 16);
-	if (ret < 0)
+	err = mv88e6xxx_phy_read(chip, port, 16, &reg);
+	if (err)
 		goto out;
 
-	reg = ret & ~0x0300;
+	reg &= ~0x0300;
 	if (e->eee_enabled)
 		reg |= 0x0200;
 	if (e->tx_lpi_enabled)
 		reg |= 0x0100;
 
-	ret = mv88e6xxx_mdio_write_indirect(chip, port, 16, reg);
+	err = mv88e6xxx_phy_write(chip, port, 16, reg);
 out:
 	mutex_unlock(&chip->reg_lock);
 
-	return ret;
+	return err;
 }
 
 static int _mv88e6xxx_atu_cmd(struct mv88e6xxx_chip *chip, u16 fid, u16 cmd)
@@ -2382,38 +2342,6 @@ static void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port)
 	mutex_unlock(&chip->reg_lock);
 }
 
-static int _mv88e6xxx_mdio_page_write(struct mv88e6xxx_chip *chip,
-				      int port, int page, int reg, int val)
-{
-	int ret;
-
-	ret = mv88e6xxx_mdio_write_indirect(chip, port, 0x16, page);
-	if (ret < 0)
-		goto restore_page_0;
-
-	ret = mv88e6xxx_mdio_write_indirect(chip, port, reg, val);
-restore_page_0:
-	mv88e6xxx_mdio_write_indirect(chip, port, 0x16, 0x0);
-
-	return ret;
-}
-
-static int _mv88e6xxx_mdio_page_read(struct mv88e6xxx_chip *chip,
-				     int port, int page, int reg)
-{
-	int ret;
-
-	ret = mv88e6xxx_mdio_write_indirect(chip, port, 0x16, page);
-	if (ret < 0)
-		goto restore_page_0;
-
-	ret = mv88e6xxx_mdio_read_indirect(chip, port, reg);
-restore_page_0:
-	mv88e6xxx_mdio_write_indirect(chip, port, 0x16, 0x0);
-
-	return ret;
-}
-
 static int mv88e6xxx_switch_reset(struct mv88e6xxx_chip *chip)
 {
 	bool ppu_active = mv88e6xxx_has(chip, MV88E6XXX_FLAG_PPU_ACTIVE);
@@ -3322,32 +3250,6 @@ static int mv88e6xxx_set_addr(struct dsa_switch *ds, u8 *addr)
 	return err;
 }
 
-static int mv88e6xxx_mdio_page_read(struct dsa_switch *ds, int port, int page,
-				    int reg)
-{
-	struct mv88e6xxx_chip *chip = ds_to_priv(ds);
-	int ret;
-
-	mutex_lock(&chip->reg_lock);
-	ret = _mv88e6xxx_mdio_page_read(chip, port, page, reg);
-	mutex_unlock(&chip->reg_lock);
-
-	return ret;
-}
-
-static int mv88e6xxx_mdio_page_write(struct dsa_switch *ds, int port, int page,
-				     int reg, int val)
-{
-	struct mv88e6xxx_chip *chip = ds_to_priv(ds);
-	int ret;
-
-	mutex_lock(&chip->reg_lock);
-	ret = _mv88e6xxx_mdio_page_write(chip, port, page, reg, val);
-	mutex_unlock(&chip->reg_lock);
-
-	return ret;
-}
-
 static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
 {
 	struct mv88e6xxx_chip *chip = bus->priv;
@@ -3441,44 +3343,42 @@ static void mv88e6xxx_mdio_unregister(struct mv88e6xxx_chip *chip)
 static int mv88e61xx_get_temp(struct dsa_switch *ds, int *temp)
 {
 	struct mv88e6xxx_chip *chip = ds_to_priv(ds);
+	u16 val;
 	int ret;
-	int val;
 
 	*temp = 0;
 
 	mutex_lock(&chip->reg_lock);
 
-	ret = mv88e6xxx_mdio_write_direct(chip, 0x0, 0x16, 0x6);
+	ret = mv88e6xxx_phy_write(chip, 0x0, 0x16, 0x6);
 	if (ret < 0)
 		goto error;
 
 	/* Enable temperature sensor */
-	ret = mv88e6xxx_mdio_read_direct(chip, 0x0, 0x1a);
+	ret = mv88e6xxx_phy_read(chip, 0x0, 0x1a, &val);
 	if (ret < 0)
 		goto error;
 
-	ret = mv88e6xxx_mdio_write_direct(chip, 0x0, 0x1a, ret | (1 << 5));
+	ret = mv88e6xxx_phy_write(chip, 0x0, 0x1a, val | (1 << 5));
 	if (ret < 0)
 		goto error;
 
 	/* Wait for temperature to stabilize */
 	usleep_range(10000, 12000);
 
-	val = mv88e6xxx_mdio_read_direct(chip, 0x0, 0x1a);
-	if (val < 0) {
-		ret = val;
+	ret = mv88e6xxx_phy_read(chip, 0x0, 0x1a, &val);
+	if (ret < 0)
 		goto error;
-	}
 
 	/* Disable temperature sensor */
-	ret = mv88e6xxx_mdio_write_direct(chip, 0x0, 0x1a, ret & ~(1 << 5));
+	ret = mv88e6xxx_phy_write(chip, 0x0, 0x1a, val & ~(1 << 5));
 	if (ret < 0)
 		goto error;
 
 	*temp = ((val & 0x1f) - 5) * 5;
 
 error:
-	mv88e6xxx_mdio_write_direct(chip, 0x0, 0x16, 0x0);
+	mv88e6xxx_phy_write(chip, 0x0, 0x16, 0x0);
 	mutex_unlock(&chip->reg_lock);
 	return ret;
 }
@@ -3487,15 +3387,18 @@ static int mv88e63xx_get_temp(struct dsa_switch *ds, int *temp)
 {
 	struct mv88e6xxx_chip *chip = ds_to_priv(ds);
 	int phy = mv88e6xxx_6320_family(chip) ? 3 : 0;
+	u16 val;
 	int ret;
 
 	*temp = 0;
 
-	ret = mv88e6xxx_mdio_page_read(ds, phy, 6, 27);
+	mutex_lock(&chip->reg_lock);
+	ret = mv88e6xxx_phy_page_read(chip, phy, 6, 27, &val);
+	mutex_unlock(&chip->reg_lock);
 	if (ret < 0)
 		return ret;
 
-	*temp = (ret & 0xff) - 25;
+	*temp = (val & 0xff) - 25;
 
 	return 0;
 }
@@ -3517,6 +3420,7 @@ static int mv88e6xxx_get_temp_limit(struct dsa_switch *ds, int *temp)
 {
 	struct mv88e6xxx_chip *chip = ds_to_priv(ds);
 	int phy = mv88e6xxx_6320_family(chip) ? 3 : 0;
+	u16 val;
 	int ret;
 
 	if (!mv88e6xxx_has(chip, MV88E6XXX_FLAG_TEMP_LIMIT))
@@ -3524,11 +3428,13 @@ static int mv88e6xxx_get_temp_limit(struct dsa_switch *ds, int *temp)
 
 	*temp = 0;
 
-	ret = mv88e6xxx_mdio_page_read(ds, phy, 6, 26);
+	mutex_lock(&chip->reg_lock);
+	ret = mv88e6xxx_phy_page_read(chip, phy, 6, 26, &val);
+	mutex_unlock(&chip->reg_lock);
 	if (ret < 0)
 		return ret;
 
-	*temp = (((ret >> 8) & 0x1f) * 5) - 25;
+	*temp = (((val >> 8) & 0x1f) * 5) - 25;
 
 	return 0;
 }
@@ -3537,23 +3443,30 @@ static int mv88e6xxx_set_temp_limit(struct dsa_switch *ds, int temp)
 {
 	struct mv88e6xxx_chip *chip = ds_to_priv(ds);
 	int phy = mv88e6xxx_6320_family(chip) ? 3 : 0;
-	int ret;
+	u16 val;
+	int err;
 
 	if (!mv88e6xxx_has(chip, MV88E6XXX_FLAG_TEMP_LIMIT))
 		return -EOPNOTSUPP;
 
-	ret = mv88e6xxx_mdio_page_read(ds, phy, 6, 26);
-	if (ret < 0)
-		return ret;
+	mutex_lock(&chip->reg_lock);
+	err = mv88e6xxx_phy_page_read(chip, phy, 6, 26, &val);
+	if (err)
+		goto unlock;
 	temp = clamp_val(DIV_ROUND_CLOSEST(temp, 5) + 5, 0, 0x1f);
-	return mv88e6xxx_mdio_page_write(ds, phy, 6, 26,
-					 (ret & 0xe0ff) | (temp << 8));
+	err = mv88e6xxx_phy_page_write(chip, phy, 6, 26,
+				       (val & 0xe0ff) | (temp << 8));
+unlock:
+	mutex_unlock(&chip->reg_lock);
+
+	return err;
 }
 
 static int mv88e6xxx_get_temp_alarm(struct dsa_switch *ds, bool *alarm)
 {
 	struct mv88e6xxx_chip *chip = ds_to_priv(ds);
 	int phy = mv88e6xxx_6320_family(chip) ? 3 : 0;
+	u16 val;
 	int ret;
 
 	if (!mv88e6xxx_has(chip, MV88E6XXX_FLAG_TEMP_LIMIT))
@@ -3561,11 +3474,13 @@ static int mv88e6xxx_get_temp_alarm(struct dsa_switch *ds, bool *alarm)
 
 	*alarm = false;
 
-	ret = mv88e6xxx_mdio_page_read(ds, phy, 6, 26);
+	mutex_lock(&chip->reg_lock);
+	ret = mv88e6xxx_phy_page_read(chip, phy, 6, 26, &val);
+	mutex_unlock(&chip->reg_lock);
 	if (ret < 0)
 		return ret;
 
-	*alarm = !!(ret & 0x40);
+	*alarm = !!(val & 0x40);
 
 	return 0;
 }
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH net-next 0/6] net: dsa: abstract PHY accesses
  2016-08-15 21:18 [PATCH net-next 0/6] net: dsa: abstract PHY accesses Vivien Didelot
                   ` (5 preceding siblings ...)
  2016-08-15 21:19 ` [PATCH net-next 6/6] net: dsa: mv88e6xxx: use the new PHY API Vivien Didelot
@ 2016-08-15 23:44 ` David Miller
  6 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2016-08-15 23:44 UTC (permalink / raw)
  To: vivien.didelot; +Cc: netdev, linux-kernel, kernel, f.fainelli, andrew

From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Mon, 15 Aug 2016 17:18:56 -0400

> The Marvell 88E6xxx switch chips have different way to access the PHY
> devices registers. 
> 
> Old chips use a direct access to the PHY registers. Next chips have a
> PHY Polling Unit (PPU) which needs to be disabled before accessing PHY
> registers. Newer chips have an indirect access to the PHY devices so
> that disabling the PPU is not necessary.
> 
> This patchset abstracts these accesses behind a new mv88e6xxx_phy_* API.
> 
> It also has the side effect to fix the temperature access code for
> 88E61xx chips which were using the wrong PHY access functions.

Looks good, series applied, thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH net-next 5/6] net: dsa: mv88e6xxx: describe PHY page and SerDes
  2016-08-15 21:19 ` [PATCH net-next 5/6] net: dsa: mv88e6xxx: describe PHY page and SerDes Vivien Didelot
@ 2016-08-16  1:16   ` Andrew Lunn
  2016-08-16 14:57     ` Vivien Didelot
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2016-08-16  1:16 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli

On Mon, Aug 15, 2016 at 05:19:01PM -0400, Vivien Didelot wrote:
> Add mv88e6xxx_phy_page_{read,write} routines and use them to access the
> SerDes PHY device registers.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c      | 95 +++++++++++++++++++++++++++++------
>  drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 27 ++++++++--
>  2 files changed, 105 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 3b0bc88..faa0751 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -238,6 +238,74 @@ static int mv88e6xxx_phy_write(struct mv88e6xxx_chip *chip, int phy,
>  	return chip->phy_ops->write(chip, addr, reg, val);
>  }
>  
> +static int mv88e6xxx_phy_page_get(struct mv88e6xxx_chip *chip, int phy, u8 page)
> +{
> +	if (!mv88e6xxx_has(chip, MV88E6XXX_FLAG_PHY_PAGE))
> +		return -EOPNOTSUPP;
> +
> +	return mv88e6xxx_phy_write(chip, phy, PHY_PAGE, page);
> +}
> +
> +static void mv88e6xxx_phy_page_put(struct mv88e6xxx_chip *chip, int phy)
> +{
> +	int err;
> +
> +	/* Restore PHY page Copper 0x0 for access via the registered MDIO bus */
> +	err = mv88e6xxx_phy_write(chip, phy, PHY_PAGE, PHY_PAGE_COPPER);
> +	if (unlikely(err)) {
> +		dev_err(chip->dev, "failed to restore PHY %d page Copper (%d)\n",
> +			phy, err);
> +	}
> +}
> +
> +static int mv88e6xxx_phy_page_read(struct mv88e6xxx_chip *chip, int phy,
> +				   u8 page, int reg, u16 *val)
> +{
> +	int err;
> +
> +	/* There is no paging for registers 22 */
> +	if (reg == PHY_PAGE)
> +		return -EINVAL;

Hi Vivien

This whole paging scheme only works for internal PHYs, or external
PHYs which happen to be Marvell PHYs. We need to be a little bit
careful here and ensure these functions don't get used for external
PHYs when we don't know who manufactured them. 

At the moment the code is O.K, we only access SERDES or temperature
sensors for a given port. But i wounder if adding a comment would be
wise?

	Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH net-next 5/6] net: dsa: mv88e6xxx: describe PHY page and SerDes
  2016-08-16  1:16   ` Andrew Lunn
@ 2016-08-16 14:57     ` Vivien Didelot
  0 siblings, 0 replies; 10+ messages in thread
From: Vivien Didelot @ 2016-08-16 14:57 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> On Mon, Aug 15, 2016 at 05:19:01PM -0400, Vivien Didelot wrote:
>> +static int mv88e6xxx_phy_page_read(struct mv88e6xxx_chip *chip, int phy,
>> +				   u8 page, int reg, u16 *val)
>> +{
>> +	int err;
>> +
>> +	/* There is no paging for registers 22 */
>> +	if (reg == PHY_PAGE)
>> +		return -EINVAL;
>
> This whole paging scheme only works for internal PHYs, or external
> PHYs which happen to be Marvell PHYs. We need to be a little bit
> careful here and ensure these functions don't get used for external
> PHYs when we don't know who manufactured them. 
>
> At the moment the code is O.K, we only access SERDES or temperature
> sensors for a given port. But i wounder if adding a comment would be
> wise?

That is a good point, I thought about that too. I was thinking about
adding an internal_phys bitmask to the chip info structures and check it
in mv88e6xxx_phy_page_get(), so we could return -EINVAL for external
PHYs, since the switch driver isn't supposed to access their pages.

But I also think that most of the PHY code should be moved to a proper
PHY driver, since they are valid Marvell chips with their own PHY IDs.

Until we move the PHY and SERDES code out of the mv88e6xxx driver, I
think we are safe.

Thanks,

        Vivien

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-08-16 14:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-15 21:18 [PATCH net-next 0/6] net: dsa: abstract PHY accesses Vivien Didelot
2016-08-15 21:18 ` [PATCH net-next 1/6] net: dsa: mv88e6xxx: rename _mv88e6xxx_wait Vivien Didelot
2016-08-15 21:18 ` [PATCH net-next 2/6] net: dsa: mv88e6xxx: describe Multi-chip registers Vivien Didelot
2016-08-15 21:18 ` [PATCH net-next 3/6] net: dsa: mv88e6xxx: rework Global2 SMI PHY access Vivien Didelot
2016-08-15 21:19 ` [PATCH net-next 4/6] net: dsa: mv88e6xxx: abstract PHY ops Vivien Didelot
2016-08-15 21:19 ` [PATCH net-next 5/6] net: dsa: mv88e6xxx: describe PHY page and SerDes Vivien Didelot
2016-08-16  1:16   ` Andrew Lunn
2016-08-16 14:57     ` Vivien Didelot
2016-08-15 21:19 ` [PATCH net-next 6/6] net: dsa: mv88e6xxx: use the new PHY API Vivien Didelot
2016-08-15 23:44 ` [PATCH net-next 0/6] net: dsa: abstract PHY accesses David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).