All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v5 0/3] Add MMD PHY helpers
@ 2019-02-08 17:25 Carlo Caione
  2019-02-08 17:25 ` [U-Boot] [PATCH v5 1/3] net: phy: Add generic helpers to access MMD PHY registers Carlo Caione
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Carlo Caione @ 2019-02-08 17:25 UTC (permalink / raw)
  To: u-boot

Introduce phy_{read|write}_mmd() helpers and modify the mdio command to 
make good use of them. Fix the ti driver in the same patchset.

Carlo Caione (3):
  net: phy: Add generic helpers to access MMD PHY registers
  net: phy: ti: use generic helpers to access MMD registers
  cmd: mdio: Switch to generic helpers when accessing the registers

 cmd/mdio.c            |  27 +++++----
 drivers/net/phy/phy.c |   4 ++
 drivers/net/phy/ti.c  | 130 ++++++++----------------------------------
 include/phy.h         |  70 +++++++++++++++++++++++
 4 files changed, 115 insertions(+), 116 deletions(-)

-- 
2.19.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v5 1/3] net: phy: Add generic helpers to access MMD PHY registers
  2019-02-08 17:25 [U-Boot] [PATCH v5 0/3] Add MMD PHY helpers Carlo Caione
@ 2019-02-08 17:25 ` Carlo Caione
  2019-02-19 18:14   ` Joe Hershberger
  2019-03-05 18:04   ` [U-Boot] " Joe Hershberger
  2019-02-08 17:25 ` [U-Boot] [PATCH v5 2/3] net: phy: ti: use generic helpers to access MMD registers Carlo Caione
  2019-02-08 17:25 ` [U-Boot] [PATCH v5 3/3] cmd: mdio: Switch to generic helpers when accessing the registers Carlo Caione
  2 siblings, 2 replies; 12+ messages in thread
From: Carlo Caione @ 2019-02-08 17:25 UTC (permalink / raw)
  To: u-boot

Two new helper functions (phy_read_mmd() and phy_write_mmd()) are added
to allow access to the MMD PHY registers.

The MMD PHY registers can be accessed by several means:

1. Using two new MMD access function hooks in the PHY driver. These
functions can be implemented when the PHY driver does not support the
standard IEEE Compatible clause 45 access mechanism described in clause
22 or if the PHY uses its own non-standard access mechanism.

2. Direct access for C45 PHYs and C22 PHYs when accessing the reachable
DEVADs.

3. The standard clause 45 access extensions to the MMD registers through
the indirection registers (clause 22) in all the other cases.

Signed-off-by: Carlo Caione <ccaione@baylibre.com>
---
 drivers/net/phy/phy.c |  4 +++
 include/phy.h         | 70 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index cda4caa803..6769047407 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -549,6 +549,10 @@ int phy_register(struct phy_driver *drv)
 		drv->readext += gd->reloc_off;
 	if (drv->writeext)
 		drv->writeext += gd->reloc_off;
+	if (drv->read_mmd)
+		drv->read_mmd += gd->reloc_off;
+	if (drv->write_mmd)
+		drv->write_mmd += gd->reloc_off;
 #endif
 	return 0;
 }
diff --git a/include/phy.h b/include/phy.h
index b86fdfb2ce..7ec2b4e86c 100644
--- a/include/phy.h
+++ b/include/phy.h
@@ -101,6 +101,14 @@ struct phy_driver {
 	int (*readext)(struct phy_device *phydev, int addr, int devad, int reg);
 	int (*writeext)(struct phy_device *phydev, int addr, int devad, int reg,
 			u16 val);
+
+	/* Phy specific driver override for reading a MMD register */
+	int (*read_mmd)(struct phy_device *phydev, int devad, int reg);
+
+	/* Phy specific driver override for writing a MMD register */
+	int (*write_mmd)(struct phy_device *phydev, int devad, int reg,
+			 u16 val);
+
 	struct list_head list;
 };
 
@@ -164,6 +172,68 @@ static inline int phy_write(struct phy_device *phydev, int devad, int regnum,
 	return bus->write(bus, phydev->addr, devad, regnum, val);
 }
 
+static inline void phy_mmd_start_indirect(struct phy_device *phydev, int devad,
+					  int regnum)
+{
+	/* Write the desired MMD Devad */
+	phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_CTRL, devad);
+
+	/* Write the desired MMD register address */
+	phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_DATA, regnum);
+
+	/* Select the Function : DATA with no post increment */
+	phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_CTRL,
+		  (devad | MII_MMD_CTRL_NOINCR));
+}
+
+static inline int phy_read_mmd(struct phy_device *phydev, int devad,
+			       int regnum)
+{
+	struct phy_driver *drv = phydev->drv;
+
+	if (regnum > (u16)~0 || devad > 32)
+		return -EINVAL;
+
+	/* driver-specific access */
+	if (drv->read_mmd)
+		return drv->read_mmd(phydev, devad, regnum);
+
+	/* direct C45 / C22 access */
+	if ((drv->features & PHY_10G_FEATURES) == PHY_10G_FEATURES ||
+	    devad == MDIO_DEVAD_NONE || !devad)
+		return phy_read(phydev, devad, regnum);
+
+	/* indirect C22 access */
+	phy_mmd_start_indirect(phydev, devad, regnum);
+
+	/* Read the content of the MMD's selected register */
+	return phy_read(phydev, MDIO_DEVAD_NONE, MII_MMD_DATA);
+}
+
+static inline int phy_write_mmd(struct phy_device *phydev, int devad,
+				int regnum, u16 val)
+{
+	struct phy_driver *drv = phydev->drv;
+
+	if (regnum > (u16)~0 || devad > 32)
+		return -EINVAL;
+
+	/* driver-specific access */
+	if (drv->write_mmd)
+		return drv->write_mmd(phydev, devad, regnum, val);
+
+	/* direct C45 / C22 access */
+	if ((drv->features & PHY_10G_FEATURES) == PHY_10G_FEATURES ||
+	    devad == MDIO_DEVAD_NONE || !devad)
+		return phy_write(phydev, devad, regnum, val);
+
+	/* indirect C22 access */
+	phy_mmd_start_indirect(phydev, devad, regnum);
+
+	/* Write the data into MMD's selected register */
+	return phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_DATA, val);
+}
+
 #ifdef CONFIG_PHYLIB_10G
 extern struct phy_driver gen10g_driver;
 
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v5 2/3] net: phy: ti: use generic helpers to access MMD registers
  2019-02-08 17:25 [U-Boot] [PATCH v5 0/3] Add MMD PHY helpers Carlo Caione
  2019-02-08 17:25 ` [U-Boot] [PATCH v5 1/3] net: phy: Add generic helpers to access MMD PHY registers Carlo Caione
@ 2019-02-08 17:25 ` Carlo Caione
  2019-03-05 18:04   ` [U-Boot] " Joe Hershberger
  2019-02-08 17:25 ` [U-Boot] [PATCH v5 3/3] cmd: mdio: Switch to generic helpers when accessing the registers Carlo Caione
  2 siblings, 1 reply; 12+ messages in thread
From: Carlo Caione @ 2019-02-08 17:25 UTC (permalink / raw)
  To: u-boot

Now that generic helpers are available, use those instead of relying on
ti specific functions.

Signed-off-by: Carlo Caione <ccaione@baylibre.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Acked-by: Joe Hershberger <joe.hershberger@ni.com>
---
 drivers/net/phy/ti.c | 130 +++++++++----------------------------------
 1 file changed, 25 insertions(+), 105 deletions(-)

diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
index 6db6edd0d0..6ac890a7f5 100644
--- a/drivers/net/phy/ti.c
+++ b/drivers/net/phy/ti.c
@@ -73,16 +73,6 @@
 #define MII_DP83867_CFG2_SPEEDOPT_INTLOW	0x2000
 #define MII_DP83867_CFG2_MASK			0x003F
 
-#define MII_MMD_CTRL	0x0d /* MMD Access Control Register */
-#define MII_MMD_DATA	0x0e /* MMD Access Data Register */
-
-/* MMD Access Control register fields */
-#define MII_MMD_CTRL_DEVAD_MASK	0x1f /* Mask MMD DEVAD*/
-#define MII_MMD_CTRL_ADDR	0x0000 /* Address */
-#define MII_MMD_CTRL_NOINCR	0x4000 /* no post increment */
-#define MII_MMD_CTRL_INCR_RDWT	0x8000 /* post increment on reads & writes */
-#define MII_MMD_CTRL_INCR_ON_WT	0xC000 /* post increment on writes only */
-
 /* User setting - can be taken from DTS */
 #define DEFAULT_RX_ID_DELAY	DP83867_RGMIIDCTL_2_25_NS
 #define DEFAULT_TX_ID_DELAY	DP83867_RGMIIDCTL_2_75_NS
@@ -116,88 +106,20 @@ struct dp83867_private {
 	int clk_output_sel;
 };
 
-/**
- * phy_read_mmd_indirect - reads data from the MMD registers
- * @phydev: The PHY device bus
- * @prtad: MMD Address
- * @devad: MMD DEVAD
- * @addr: PHY address on the MII bus
- *
- * Description: it reads data from the MMD registers (clause 22 to access to
- * clause 45) of the specified phy address.
- * To read these registers we have:
- * 1) Write reg 13 // DEVAD
- * 2) Write reg 14 // MMD Address
- * 3) Write reg 13 // MMD Data Command for MMD DEVAD
- * 3) Read  reg 14 // Read MMD data
- */
-int phy_read_mmd_indirect(struct phy_device *phydev, int prtad,
-			  int devad, int addr)
-{
-	int value = -1;
-
-	/* Write the desired MMD Devad */
-	phy_write(phydev, addr, MII_MMD_CTRL, devad);
-
-	/* Write the desired MMD register address */
-	phy_write(phydev, addr, MII_MMD_DATA, prtad);
-
-	/* Select the Function : DATA with no post increment */
-	phy_write(phydev, addr, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
-
-	/* Read the content of the MMD's selected register */
-	value = phy_read(phydev, addr, MII_MMD_DATA);
-	return value;
-}
-
-/**
- * phy_write_mmd_indirect - writes data to the MMD registers
- * @phydev: The PHY device
- * @prtad: MMD Address
- * @devad: MMD DEVAD
- * @addr: PHY address on the MII bus
- * @data: data to write in the MMD register
- *
- * Description: Write data from the MMD registers of the specified
- * phy address.
- * To write these registers we have:
- * 1) Write reg 13 // DEVAD
- * 2) Write reg 14 // MMD Address
- * 3) Write reg 13 // MMD Data Command for MMD DEVAD
- * 3) Write reg 14 // Write MMD data
- */
-void phy_write_mmd_indirect(struct phy_device *phydev, int prtad,
-			    int devad, int addr, u32 data)
-{
-	/* Write the desired MMD Devad */
-	phy_write(phydev, addr, MII_MMD_CTRL, devad);
-
-	/* Write the desired MMD register address */
-	phy_write(phydev, addr, MII_MMD_DATA, prtad);
-
-	/* Select the Function : DATA with no post increment */
-	phy_write(phydev, addr, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
-
-	/* Write the data into MMD's selected register */
-	phy_write(phydev, addr, MII_MMD_DATA, data);
-}
-
 static int dp83867_config_port_mirroring(struct phy_device *phydev)
 {
 	struct dp83867_private *dp83867 =
 		(struct dp83867_private *)phydev->priv;
 	u16 val;
 
-	val = phy_read_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR,
-				    phydev->addr);
+	val = phy_read_mmd(phydev, DP83867_DEVADDR, DP83867_CFG4);
 
 	if (dp83867->port_mirroring == DP83867_PORT_MIRRORING_EN)
 		val |= DP83867_CFG4_PORT_MIRROR_EN;
 	else
 		val &= ~DP83867_CFG4_PORT_MIRROR_EN;
 
-	phy_write_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR,
-			       phydev->addr, val);
+	phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_CFG4, val);
 
 	return 0;
 }
@@ -257,13 +179,13 @@ static int dp83867_of_init(struct phy_device *phydev)
 
 	/* Clock output selection if muxing property is set */
 	if (dp83867->clk_output_sel != DP83867_CLK_O_SEL_REF_CLK) {
-		val = phy_read_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
-					    DP83867_DEVADDR, phydev->addr);
+		val = phy_read_mmd(phydev, DP83867_DEVADDR,
+				   DP83867_IO_MUX_CFG);
 		val &= ~DP83867_IO_MUX_CFG_CLK_O_SEL_MASK;
 		val |= (dp83867->clk_output_sel <<
 			DP83867_IO_MUX_CFG_CLK_O_SEL_SHIFT);
-		phy_write_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
-				       DP83867_DEVADDR, phydev->addr, val);
+		phy_write_mmd(phydev, DP83867_DEVADDR,
+			      DP83867_IO_MUX_CFG, val);
 	}
 
 	return 0;
@@ -308,11 +230,11 @@ static int dp83867_config(struct phy_device *phydev)
 
 	/* Mode 1 or 2 workaround */
 	if (dp83867->rxctrl_strap_quirk) {
-		val = phy_read_mmd_indirect(phydev, DP83867_CFG4,
-					    DP83867_DEVADDR, phydev->addr);
+		val = phy_read_mmd(phydev, DP83867_DEVADDR,
+				   DP83867_CFG4);
 		val &= ~BIT(7);
-		phy_write_mmd_indirect(phydev, DP83867_CFG4,
-				       DP83867_DEVADDR, phydev->addr, val);
+		phy_write_mmd(phydev, DP83867_DEVADDR,
+			      DP83867_CFG4, val);
 	}
 
 	if (phy_interface_is_rgmii(phydev)) {
@@ -332,8 +254,8 @@ static int dp83867_config(struct phy_device *phydev)
 		 * register's bit 11 (marked as RESERVED).
 		 */
 
-		bs = phy_read_mmd_indirect(phydev, DP83867_STRAP_STS1,
-					   DP83867_DEVADDR, phydev->addr);
+		bs = phy_read_mmd(phydev, DP83867_DEVADDR,
+				  DP83867_STRAP_STS1);
 		val = phy_read(phydev, MDIO_DEVAD_NONE, MII_DP83867_PHYCTRL);
 		if (bs & DP83867_STRAP_STS1_RESERVED) {
 			val &= ~DP83867_PHYCR_RESERVED_MASK;
@@ -354,8 +276,8 @@ static int dp83867_config(struct phy_device *phydev)
 			 MII_DP83867_CFG2_SPEEDOPT_INTLOW);
 		phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83867_CFG2, cfg2);
 
-		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
-				       DP83867_DEVADDR, phydev->addr, 0x0);
+		phy_write_mmd(phydev, DP83867_DEVADDR,
+			      DP83867_RGMIICTL, 0x0);
 
 		phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83867_PHYCTRL,
 			  DP83867_PHYCTRL_SGMIIEN |
@@ -367,8 +289,8 @@ static int dp83867_config(struct phy_device *phydev)
 	}
 
 	if (phy_interface_is_rgmii(phydev)) {
-		val = phy_read_mmd_indirect(phydev, DP83867_RGMIICTL,
-					    DP83867_DEVADDR, phydev->addr);
+		val = phy_read_mmd(phydev, DP83867_DEVADDR,
+				   DP83867_RGMIICTL);
 
 		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
 			val |= (DP83867_RGMII_TX_CLK_DELAY_EN |
@@ -380,26 +302,24 @@ static int dp83867_config(struct phy_device *phydev)
 		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
 			val |= DP83867_RGMII_RX_CLK_DELAY_EN;
 
-		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
-				       DP83867_DEVADDR, phydev->addr, val);
+		phy_write_mmd(phydev, DP83867_DEVADDR,
+			      DP83867_RGMIICTL, val);
 
 		delay = (dp83867->rx_id_delay |
 			 (dp83867->tx_id_delay << DP83867_RGMII_TX_CLK_DELAY_SHIFT));
 
-		phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
-				       DP83867_DEVADDR, phydev->addr, delay);
+		phy_write_mmd(phydev, DP83867_DEVADDR,
+			      DP83867_RGMIIDCTL, delay);
 
 		if (dp83867->io_impedance >= 0) {
-			val = phy_read_mmd_indirect(phydev,
-						    DP83867_IO_MUX_CFG,
-						    DP83867_DEVADDR,
-						    phydev->addr);
+			val = phy_read_mmd(phydev,
+					   DP83867_DEVADDR,
+					   DP83867_IO_MUX_CFG);
 			val &= ~DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
 			val |= dp83867->io_impedance &
 			       DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
-			phy_write_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
-					       DP83867_DEVADDR, phydev->addr,
-					       val);
+			phy_write_mmd(phydev, DP83867_DEVADDR,
+				      DP83867_IO_MUX_CFG, val);
 		}
 	}
 
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v5 3/3] cmd: mdio: Switch to generic helpers when accessing the registers
  2019-02-08 17:25 [U-Boot] [PATCH v5 0/3] Add MMD PHY helpers Carlo Caione
  2019-02-08 17:25 ` [U-Boot] [PATCH v5 1/3] net: phy: Add generic helpers to access MMD PHY registers Carlo Caione
  2019-02-08 17:25 ` [U-Boot] [PATCH v5 2/3] net: phy: ti: use generic helpers to access MMD registers Carlo Caione
@ 2019-02-08 17:25 ` Carlo Caione
  2019-02-12 12:20   ` Vladimir Oltean
                     ` (2 more replies)
  2 siblings, 3 replies; 12+ messages in thread
From: Carlo Caione @ 2019-02-08 17:25 UTC (permalink / raw)
  To: u-boot

Switch to use the generic helpers to access the MMD registers so that we
can used the same command also for C45 PHYs, C22 PHYs with direct and
indirect access and PHYs implementing a custom way to access the
registers.

Signed-off-by: Carlo Caione <ccaione@baylibre.com>
---
 cmd/mdio.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/cmd/mdio.c b/cmd/mdio.c
index 184868063a..efe8c9ef09 100644
--- a/cmd/mdio.c
+++ b/cmd/mdio.c
@@ -39,21 +39,24 @@ static int extract_range(char *input, int *plo, int *phi)
 	return 0;
 }
 
-static int mdio_write_ranges(struct phy_device *phydev, struct mii_dev *bus,
+static int mdio_write_ranges(struct mii_dev *bus,
 			     int addrlo,
 			     int addrhi, int devadlo, int devadhi,
 			     int reglo, int reghi, unsigned short data,
 			     int extended)
 {
+	struct phy_device *phydev;
 	int addr, devad, reg;
 	int err = 0;
 
 	for (addr = addrlo; addr <= addrhi; addr++) {
+		phydev = bus->phymap[addr];
+
 		for (devad = devadlo; devad <= devadhi; devad++) {
 			for (reg = reglo; reg <= reghi; reg++) {
 				if (!extended)
-					err = bus->write(bus, addr, devad,
-							 reg, data);
+					err = phy_write_mmd(phydev, devad,
+							    reg, data);
 				else
 					err = phydev->drv->writeext(phydev,
 							addr, devad, reg, data);
@@ -68,15 +71,17 @@ err_out:
 	return err;
 }
 
-static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
+static int mdio_read_ranges(struct mii_dev *bus,
 			    int addrlo,
 			    int addrhi, int devadlo, int devadhi,
 			    int reglo, int reghi, int extended)
 {
 	int addr, devad, reg;
+	struct phy_device *phydev;
 
 	printf("Reading from bus %s\n", bus->name);
 	for (addr = addrlo; addr <= addrhi; addr++) {
+		phydev = bus->phymap[addr];
 		printf("PHY at address %x:\n", addr);
 
 		for (devad = devadlo; devad <= devadhi; devad++) {
@@ -84,7 +89,7 @@ static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
 				int val;
 
 				if (!extended)
-					val = bus->read(bus, addr, devad, reg);
+					val = phy_read_mmd(phydev, devad, reg);
 				else
 					val = phydev->drv->readext(phydev, addr,
 						devad, reg);
@@ -222,14 +227,14 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 				bus = phydev->bus;
 				extended = 1;
 			} else {
-				return -1;
+				return CMD_RET_FAILURE;
 			}
 
 			if (!phydev->drv ||
 			    (!phydev->drv->writeext && (op[0] == 'w')) ||
 			    (!phydev->drv->readext && (op[0] == 'r'))) {
 				puts("PHY does not have extended functions\n");
-				return -1;
+				return CMD_RET_FAILURE;
 			}
 		}
 	}
@@ -242,13 +247,13 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		if (pos > 1)
 			if (extract_reg_range(argv[pos--], &devadlo, &devadhi,
 					      &reglo, &reghi))
-				return -1;
+				return CMD_RET_FAILURE;
 
 	default:
 		if (pos > 1)
 			if (extract_phy_range(&argv[2], pos - 1, &bus,
 					      &phydev, &addrlo, &addrhi))
-				return -1;
+				return CMD_RET_FAILURE;
 
 		break;
 	}
@@ -264,12 +269,12 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 	switch (op[0]) {
 	case 'w':
-		mdio_write_ranges(phydev, bus, addrlo, addrhi, devadlo, devadhi,
+		mdio_write_ranges(bus, addrlo, addrhi, devadlo, devadhi,
 				  reglo, reghi, data, extended);
 		break;
 
 	case 'r':
-		mdio_read_ranges(phydev, bus, addrlo, addrhi, devadlo, devadhi,
+		mdio_read_ranges(bus, addrlo, addrhi, devadlo, devadhi,
 				 reglo, reghi, extended);
 		break;
 	}
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v5 3/3] cmd: mdio: Switch to generic helpers when accessing the registers
  2019-02-08 17:25 ` [U-Boot] [PATCH v5 3/3] cmd: mdio: Switch to generic helpers when accessing the registers Carlo Caione
@ 2019-02-12 12:20   ` Vladimir Oltean
  2019-02-15 22:46     ` Vladimir Oltean
  2019-02-19 18:06   ` Joe Hershberger
  2019-03-05 18:04   ` [U-Boot] " Joe Hershberger
  2 siblings, 1 reply; 12+ messages in thread
From: Vladimir Oltean @ 2019-02-12 12:20 UTC (permalink / raw)
  To: u-boot

On 08.02.2019 19:26, Carlo Caione wrote:
> Switch to use the generic helpers to access the MMD registers so that we
> can used the same command also for C45 PHYs, C22 PHYs with direct and
> indirect access and PHYs implementing a custom way to access the
> registers.
> 
> Signed-off-by: Carlo Caione <ccaione@baylibre.com>
> ---
>   cmd/mdio.c | 27 ++++++++++++++++-----------
>   1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/cmd/mdio.c b/cmd/mdio.c
> index 184868063a..efe8c9ef09 100644
> --- a/cmd/mdio.c
> +++ b/cmd/mdio.c
> @@ -39,21 +39,24 @@ static int extract_range(char *input, int *plo, int *phi)
>   	return 0;
>   }
>   
> -static int mdio_write_ranges(struct phy_device *phydev, struct mii_dev *bus,
> +static int mdio_write_ranges(struct mii_dev *bus,
>   			     int addrlo,
>   			     int addrhi, int devadlo, int devadhi,
>   			     int reglo, int reghi, unsigned short data,
>   			     int extended)
>   {
> +	struct phy_device *phydev;
>   	int addr, devad, reg;
>   	int err = 0;
>   
>   	for (addr = addrlo; addr <= addrhi; addr++) {
> +		phydev = bus->phymap[addr];
> +
>   		for (devad = devadlo; devad <= devadhi; devad++) {
>   			for (reg = reglo; reg <= reghi; reg++) {
>   				if (!extended)
> -					err = bus->write(bus, addr, devad,
> -							 reg, data);
> +					err = phy_write_mmd(phydev, devad,
> +							    reg, data);
>   				else
>   					err = phydev->drv->writeext(phydev,
>   							addr, devad, reg, data);
> @@ -68,15 +71,17 @@ err_out:
>   	return err;
>   }
>   
> -static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
> +static int mdio_read_ranges(struct mii_dev *bus,
>   			    int addrlo,
>   			    int addrhi, int devadlo, int devadhi,
>   			    int reglo, int reghi, int extended)
>   {
>   	int addr, devad, reg;
> +	struct phy_device *phydev;
>   
>   	printf("Reading from bus %s\n", bus->name);
>   	for (addr = addrlo; addr <= addrhi; addr++) {
> +		phydev = bus->phymap[addr];
>   		printf("PHY at address %x:\n", addr);
>   
>   		for (devad = devadlo; devad <= devadhi; devad++) {
> @@ -84,7 +89,7 @@ static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
>   				int val;
>   
>   				if (!extended)
> -					val = bus->read(bus, addr, devad, reg);
> +					val = phy_read_mmd(phydev, devad, reg);
>   				else
>   					val = phydev->drv->readext(phydev, addr,
>   						devad, reg);
> @@ -222,14 +227,14 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   				bus = phydev->bus;
>   				extended = 1;
>   			} else {
> -				return -1;
> +				return CMD_RET_FAILURE;
>   			}
>   
>   			if (!phydev->drv ||
>   			    (!phydev->drv->writeext && (op[0] == 'w')) ||
>   			    (!phydev->drv->readext && (op[0] == 'r'))) {
>   				puts("PHY does not have extended functions\n");
> -				return -1;
> +				return CMD_RET_FAILURE;
>   			}
>   		}
>   	}
> @@ -242,13 +247,13 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   		if (pos > 1)
>   			if (extract_reg_range(argv[pos--], &devadlo, &devadhi,
>   					      &reglo, &reghi))
> -				return -1;
> +				return CMD_RET_FAILURE;
>   
>   	default:
>   		if (pos > 1)
>   			if (extract_phy_range(&argv[2], pos - 1, &bus,
>   					      &phydev, &addrlo, &addrhi))
> -				return -1;
> +				return CMD_RET_FAILURE;
>   
>   		break;
>   	}
> @@ -264,12 +269,12 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   
>   	switch (op[0]) {
>   	case 'w':
> -		mdio_write_ranges(phydev, bus, addrlo, addrhi, devadlo, devadhi,
> +		mdio_write_ranges(bus, addrlo, addrhi, devadlo, devadhi,
>   				  reglo, reghi, data, extended);
>   		break;
>   
>   	case 'r':
> -		mdio_read_ranges(phydev, bus, addrlo, addrhi, devadlo, devadhi,
> +		mdio_read_ranges(bus, addrlo, addrhi, devadlo, devadhi,
>   				 reglo, reghi, extended);
>   		break;
>   	}
> 

Hi Carlo,

I tested your patch on the NXP LS1088A-RDB board which is equipped with 
2 VSC8514 PHYs and 1 AQR105 PHY. The VSC8514 is a clause 22 QSGMII 
10/100/1000 Base-T PHY, but its 802.1az (EEE) registers are accessible 
through indirect clause 45 (clause 22 registers 13 and 14). The Aquantia 
AQR105 is a 10G Base-T PHY with native clause 45 addressing.

I started off by making sure that what should work (clause 45 
addressing) still works, so I tested the AQR105 without the patchset.

=> mdio list
FSL_MDIO0:
c - Vitesse VSC8514 <--> DPMAC3 at qsgmii
d - Vitesse VSC8514 <--> DPMAC4 at qsgmii
e - Vitesse VSC8514 <--> DPMAC5 at qsgmii
f - Vitesse VSC8514 <--> DPMAC6 at qsgmii
1c - Vitesse VSC8514 <--> DPMAC7 at qsgmii
1d - Vitesse VSC8514 <--> DPMAC8 at qsgmii
1e - Vitesse VSC8514 <--> DPMAC9 at qsgmii
1f - Vitesse VSC8514 <--> DPMAC10 at qsgmii
FSL_MDIO1:
0 - Generic PHY <--> DPMAC2 at xgmii

# AQR105: PMA Standard Device Identifier 1 & 2 registers
=> mdio read DPMAC2 at xgmii 1.2-3
Reading from bus FSL_MDIO1
PHY at address 0:
1.2 - 0x3a1
1.3 - 0xb4a3

Then I applied the patchset and went on to test both PHYs (since now 
mdio read MMD is supposed to work on both, not just the AQR).

# VSC8514: EEE Capability register
=> mdio read DPMAC3 at qsgmii 3.14
Reading from bus FSL_MDIO0
PHY at address c:
3.20 - 0x6

So indirect addressing works, I will no longer refer to this and instead 
focus on the native C45 PHYs.

=> mdio read DPMAC2 at xgmii 1.2-3
Reading from bus FSL_MDIO1
PHY at address 0:
1.2 - 0xffff
1.3 - 0xffff

Oops, the AQR105 no longer works.
But notice that U-boot probes it as "Generic PHY", so this is a board 
defconfig problem. So I went on to enable CONFIG_PHYLIB_10G=y and repeat 
the test.

=> mdio list
FSL_MDIO0:
c - Vitesse VSC8514 <--> DPMAC3 at qsgmii
d - Vitesse VSC8514 <--> DPMAC4 at qsgmii
e - Vitesse VSC8514 <--> DPMAC5 at qsgmii
f - Vitesse VSC8514 <--> DPMAC6 at qsgmii
1c - Vitesse VSC8514 <--> DPMAC7 at qsgmii
1d - Vitesse VSC8514 <--> DPMAC8 at qsgmii
1e - Vitesse VSC8514 <--> DPMAC9 at qsgmii
1f - Vitesse VSC8514 <--> DPMAC10 at qsgmii
FSL_MDIO1:
0 - Generic 10G PHY <--> DPMAC2 at xgmii

Notice that the AQR105 is now probed as "Generic 10G PHY".
But:
# AQR105: PMA Standard Device Identifier 1 & 2 registers
=> mdio read DPMAC2 at xgmii 1.2-3
Reading from bus FSL_MDIO1
PHY at address 0:
1.2 - 0xffff
1.3 - 0xffff

So it still doesn't work.
This is because in drivers/net/phy/generic_10g.c, the 
gen10g_driver.features is 0 and not PHY_10G_FEATURES as it properly 
should (comments?)

Then I went on to enable the proper driver for the AQR105, which is 
CONFIG_PHY_AQUANTIA.

=> mdio list
FSL_MDIO0:
c - Vitesse VSC8514 <--> DPMAC3 at qsgmii
d - Vitesse VSC8514 <--> DPMAC4 at qsgmii
e - Vitesse VSC8514 <--> DPMAC5 at qsgmii
f - Vitesse VSC8514 <--> DPMAC6 at qsgmii
1c - Vitesse VSC8514 <--> DPMAC7 at qsgmii
1d - Vitesse VSC8514 <--> DPMAC8 at qsgmii
1e - Vitesse VSC8514 <--> DPMAC9 at qsgmii
1f - Vitesse VSC8514 <--> DPMAC10 at qsgmii
FSL_MDIO1:
0 - Aquantia AQR105 <--> DPMAC2 at xgmii

# AQR105: PMA Standard Device Identifier 1 & 2 registers
=> mdio read DPMAC2 at xgmii 1.2-3
Reading from bus FSL_MDIO1
PHY at address 0:
1.2 - 0x3a1
1.3 - 0xb4a3

So it finally works now with your patchset.

Posting this just to raise awareness that:
* There are boards with 10G PHYs that didn't use to define 
CONFIG_PHYLIB_10G. Their MDIO drivers were working because they were 
only inferring the clause 45 aspect from the fact that a non-zero MMD 
access was being performed. This no longer works so they have to enable 
the 10g phylib support now.
* The generic 10G PHY driver doesn't define PHY_10G_FEATURES and needs 
to be fixed.
* The PHY vendors define their MMD register addresses in decimal. The 
U-boot mdio command has no way of inputting the address in decimal. For 
the VSC8514, to access its EEE registers I have to specify 3.14 (hex) in 
order for it to access 3.20 (decimal). This behavior does not originate 
from this patchset, but I think it's worth pointing it out now since it 
touches this area.

None of the above is a deal breaker IMO and I think that this patchset 
does work as intended (with the mention that more patches are needed).

Thanks,
-Vladimir

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v5 3/3] cmd: mdio: Switch to generic helpers when accessing the registers
  2019-02-12 12:20   ` Vladimir Oltean
@ 2019-02-15 22:46     ` Vladimir Oltean
  2019-02-16 10:57       ` Carlo Caione
  0 siblings, 1 reply; 12+ messages in thread
From: Vladimir Oltean @ 2019-02-15 22:46 UTC (permalink / raw)
  To: u-boot

On 2/12/19 2:20 PM, Vladimir Oltean wrote:
> On 08.02.2019 19:26, Carlo Caione wrote:
>> Switch to use the generic helpers to access the MMD registers so that we
>> can used the same command also for C45 PHYs, C22 PHYs with direct and
>> indirect access and PHYs implementing a custom way to access the
>> registers.
>>
>> Signed-off-by: Carlo Caione <ccaione@baylibre.com>
>> ---
>>    cmd/mdio.c | 27 ++++++++++++++++-----------
>>    1 file changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/cmd/mdio.c b/cmd/mdio.c
>> index 184868063a..efe8c9ef09 100644
>> --- a/cmd/mdio.c
>> +++ b/cmd/mdio.c
>> @@ -39,21 +39,24 @@ static int extract_range(char *input, int *plo, int *phi)
>>    	return 0;
>>    }
>>    
>> -static int mdio_write_ranges(struct phy_device *phydev, struct mii_dev *bus,
>> +static int mdio_write_ranges(struct mii_dev *bus,
>>    			     int addrlo,
>>    			     int addrhi, int devadlo, int devadhi,
>>    			     int reglo, int reghi, unsigned short data,
>>    			     int extended)
>>    {
>> +	struct phy_device *phydev;
>>    	int addr, devad, reg;
>>    	int err = 0;
>>    
>>    	for (addr = addrlo; addr <= addrhi; addr++) {
>> +		phydev = bus->phymap[addr];
>> +
>>    		for (devad = devadlo; devad <= devadhi; devad++) {
>>    			for (reg = reglo; reg <= reghi; reg++) {
>>    				if (!extended)
>> -					err = bus->write(bus, addr, devad,
>> -							 reg, data);
>> +					err = phy_write_mmd(phydev, devad,
>> +							    reg, data);
>>    				else
>>    					err = phydev->drv->writeext(phydev,
>>    							addr, devad, reg, data);
>> @@ -68,15 +71,17 @@ err_out:
>>    	return err;
>>    }
>>    
>> -static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
>> +static int mdio_read_ranges(struct mii_dev *bus,
>>    			    int addrlo,
>>    			    int addrhi, int devadlo, int devadhi,
>>    			    int reglo, int reghi, int extended)
>>    {
>>    	int addr, devad, reg;
>> +	struct phy_device *phydev;
>>    
>>    	printf("Reading from bus %s\n", bus->name);
>>    	for (addr = addrlo; addr <= addrhi; addr++) {
>> +		phydev = bus->phymap[addr];
>>    		printf("PHY at address %x:\n", addr);
>>    
>>    		for (devad = devadlo; devad <= devadhi; devad++) {
>> @@ -84,7 +89,7 @@ static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
>>    				int val;
>>    
>>    				if (!extended)
>> -					val = bus->read(bus, addr, devad, reg);
>> +					val = phy_read_mmd(phydev, devad, reg);
>>    				else
>>    					val = phydev->drv->readext(phydev, addr,
>>    						devad, reg);
>> @@ -222,14 +227,14 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>    				bus = phydev->bus;
>>    				extended = 1;
>>    			} else {
>> -				return -1;
>> +				return CMD_RET_FAILURE;
>>    			}
>>    
>>    			if (!phydev->drv ||
>>    			    (!phydev->drv->writeext && (op[0] == 'w')) ||
>>    			    (!phydev->drv->readext && (op[0] == 'r'))) {
>>    				puts("PHY does not have extended functions\n");
>> -				return -1;
>> +				return CMD_RET_FAILURE;
>>    			}
>>    		}
>>    	}
>> @@ -242,13 +247,13 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>    		if (pos > 1)
>>    			if (extract_reg_range(argv[pos--], &devadlo, &devadhi,
>>    					      &reglo, &reghi))
>> -				return -1;
>> +				return CMD_RET_FAILURE;
>>    
>>    	default:
>>    		if (pos > 1)
>>    			if (extract_phy_range(&argv[2], pos - 1, &bus,
>>    					      &phydev, &addrlo, &addrhi))
>> -				return -1;
>> +				return CMD_RET_FAILURE;
>>    
>>    		break;
>>    	}
>> @@ -264,12 +269,12 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>    
>>    	switch (op[0]) {
>>    	case 'w':
>> -		mdio_write_ranges(phydev, bus, addrlo, addrhi, devadlo, devadhi,
>> +		mdio_write_ranges(bus, addrlo, addrhi, devadlo, devadhi,
>>    				  reglo, reghi, data, extended);
>>    		break;
>>    
>>    	case 'r':
>> -		mdio_read_ranges(phydev, bus, addrlo, addrhi, devadlo, devadhi,
>> +		mdio_read_ranges(bus, addrlo, addrhi, devadlo, devadhi,
>>    				 reglo, reghi, extended);
>>    		break;
>>    	}
>>
> 
> Hi Carlo,
> 
> I tested your patch on the NXP LS1088A-RDB board which is equipped with
> 2 VSC8514 PHYs and 1 AQR105 PHY. The VSC8514 is a clause 22 QSGMII
> 10/100/1000 Base-T PHY, but its 802.1az (EEE) registers are accessible
> through indirect clause 45 (clause 22 registers 13 and 14). The Aquantia
> AQR105 is a 10G Base-T PHY with native clause 45 addressing.
> 
> I started off by making sure that what should work (clause 45
> addressing) still works, so I tested the AQR105 without the patchset.
> 
> => mdio list
> FSL_MDIO0:
> c - Vitesse VSC8514 <--> DPMAC3 at qsgmii
> d - Vitesse VSC8514 <--> DPMAC4 at qsgmii
> e - Vitesse VSC8514 <--> DPMAC5 at qsgmii
> f - Vitesse VSC8514 <--> DPMAC6 at qsgmii
> 1c - Vitesse VSC8514 <--> DPMAC7 at qsgmii
> 1d - Vitesse VSC8514 <--> DPMAC8 at qsgmii
> 1e - Vitesse VSC8514 <--> DPMAC9 at qsgmii
> 1f - Vitesse VSC8514 <--> DPMAC10 at qsgmii
> FSL_MDIO1:
> 0 - Generic PHY <--> DPMAC2 at xgmii
> 
> # AQR105: PMA Standard Device Identifier 1 & 2 registers
> => mdio read DPMAC2 at xgmii 1.2-3
> Reading from bus FSL_MDIO1
> PHY at address 0:
> 1.2 - 0x3a1
> 1.3 - 0xb4a3
> 
> Then I applied the patchset and went on to test both PHYs (since now
> mdio read MMD is supposed to work on both, not just the AQR).
> 
> # VSC8514: EEE Capability register
> => mdio read DPMAC3 at qsgmii 3.14
> Reading from bus FSL_MDIO0
> PHY at address c:
> 3.20 - 0x6
> 
> So indirect addressing works, I will no longer refer to this and instead
> focus on the native C45 PHYs.
> 
> => mdio read DPMAC2 at xgmii 1.2-3
> Reading from bus FSL_MDIO1
> PHY at address 0:
> 1.2 - 0xffff
> 1.3 - 0xffff
> 
> Oops, the AQR105 no longer works.
> But notice that U-boot probes it as "Generic PHY", so this is a board
> defconfig problem. So I went on to enable CONFIG_PHYLIB_10G=y and repeat
> the test.
> 
> => mdio list
> FSL_MDIO0:
> c - Vitesse VSC8514 <--> DPMAC3 at qsgmii
> d - Vitesse VSC8514 <--> DPMAC4 at qsgmii
> e - Vitesse VSC8514 <--> DPMAC5 at qsgmii
> f - Vitesse VSC8514 <--> DPMAC6 at qsgmii
> 1c - Vitesse VSC8514 <--> DPMAC7 at qsgmii
> 1d - Vitesse VSC8514 <--> DPMAC8 at qsgmii
> 1e - Vitesse VSC8514 <--> DPMAC9 at qsgmii
> 1f - Vitesse VSC8514 <--> DPMAC10 at qsgmii
> FSL_MDIO1:
> 0 - Generic 10G PHY <--> DPMAC2 at xgmii
> 
> Notice that the AQR105 is now probed as "Generic 10G PHY".
> But:
> # AQR105: PMA Standard Device Identifier 1 & 2 registers
> => mdio read DPMAC2 at xgmii 1.2-3
> Reading from bus FSL_MDIO1
> PHY at address 0:
> 1.2 - 0xffff
> 1.3 - 0xffff
> 
> So it still doesn't work.
> This is because in drivers/net/phy/generic_10g.c, the
> gen10g_driver.features is 0 and not PHY_10G_FEATURES as it properly
> should (comments?)
> 
> Then I went on to enable the proper driver for the AQR105, which is
> CONFIG_PHY_AQUANTIA.
> 
> => mdio list
> FSL_MDIO0:
> c - Vitesse VSC8514 <--> DPMAC3 at qsgmii
> d - Vitesse VSC8514 <--> DPMAC4 at qsgmii
> e - Vitesse VSC8514 <--> DPMAC5 at qsgmii
> f - Vitesse VSC8514 <--> DPMAC6 at qsgmii
> 1c - Vitesse VSC8514 <--> DPMAC7 at qsgmii
> 1d - Vitesse VSC8514 <--> DPMAC8 at qsgmii
> 1e - Vitesse VSC8514 <--> DPMAC9 at qsgmii
> 1f - Vitesse VSC8514 <--> DPMAC10 at qsgmii
> FSL_MDIO1:
> 0 - Aquantia AQR105 <--> DPMAC2 at xgmii
> 
> # AQR105: PMA Standard Device Identifier 1 & 2 registers
> => mdio read DPMAC2 at xgmii 1.2-3
> Reading from bus FSL_MDIO1
> PHY at address 0:
> 1.2 - 0x3a1
> 1.3 - 0xb4a3
> 
> So it finally works now with your patchset.
> 
> Posting this just to raise awareness that:
> * There are boards with 10G PHYs that didn't use to define
> CONFIG_PHYLIB_10G. Their MDIO drivers were working because they were
> only inferring the clause 45 aspect from the fact that a non-zero MMD
> access was being performed. This no longer works so they have to enable
> the 10g phylib support now.
> * The generic 10G PHY driver doesn't define PHY_10G_FEATURES and needs
> to be fixed.
> * The PHY vendors define their MMD register addresses in decimal. The
> U-boot mdio command has no way of inputting the address in decimal. For
> the VSC8514, to access its EEE registers I have to specify 3.14 (hex) in
> order for it to access 3.20 (decimal). This behavior does not originate
> from this patchset, but I think it's worth pointing it out now since it
> touches this area.
> 
> None of the above is a deal breaker IMO and I think that this patchset
> does work as intended (with the mention that more patches are needed).
> 
> Thanks,
> -Vladimir
> 

Carlo, Joe,

One additional piece of information that for some reason escaped me 
initially is that Pankaj Bansal already added support for struct 
phy_device property is_c45 as part of this patch: 
http://patchwork.ozlabs.org/patch/998770/
Maybe this patchset should go in the direction of using that.

-Vladimir

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v5 3/3] cmd: mdio: Switch to generic helpers when accessing the registers
  2019-02-15 22:46     ` Vladimir Oltean
@ 2019-02-16 10:57       ` Carlo Caione
  0 siblings, 0 replies; 12+ messages in thread
From: Carlo Caione @ 2019-02-16 10:57 UTC (permalink / raw)
  To: u-boot

On 15/02/2019 22:46, Vladimir Oltean wrote:
> On 2/12/19 2:20 PM, Vladimir Oltean wrote:

/cut
> 
> Carlo, Joe,
> 
> One additional piece of information that for some reason escaped me
> initially is that Pankaj Bansal already added support for struct
> phy_device property is_c45 as part of this patch:
> http://patchwork.ozlabs.org/patch/998770/
> Maybe this patchset should go in the direction of using that.

Oh, that's nice.

Joe,
can you review this patchset before I send a new one?

Thanks,

--
Carlo Caione

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v5 3/3] cmd: mdio: Switch to generic helpers when accessing the registers
  2019-02-08 17:25 ` [U-Boot] [PATCH v5 3/3] cmd: mdio: Switch to generic helpers when accessing the registers Carlo Caione
  2019-02-12 12:20   ` Vladimir Oltean
@ 2019-02-19 18:06   ` Joe Hershberger
  2019-03-05 18:04   ` [U-Boot] " Joe Hershberger
  2 siblings, 0 replies; 12+ messages in thread
From: Joe Hershberger @ 2019-02-19 18:06 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 8, 2019 at 11:31 AM Carlo Caione <ccaione@baylibre.com> wrote:
>
> Switch to use the generic helpers to access the MMD registers so that we
> can used the same command also for C45 PHYs, C22 PHYs with direct and
> indirect access and PHYs implementing a custom way to access the
> registers.
>
> Signed-off-by: Carlo Caione <ccaione@baylibre.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v5 1/3] net: phy: Add generic helpers to access MMD PHY registers
  2019-02-08 17:25 ` [U-Boot] [PATCH v5 1/3] net: phy: Add generic helpers to access MMD PHY registers Carlo Caione
@ 2019-02-19 18:14   ` Joe Hershberger
  2019-03-05 18:04   ` [U-Boot] " Joe Hershberger
  1 sibling, 0 replies; 12+ messages in thread
From: Joe Hershberger @ 2019-02-19 18:14 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 8, 2019 at 11:29 AM Carlo Caione <ccaione@baylibre.com> wrote:
>
> Two new helper functions (phy_read_mmd() and phy_write_mmd()) are added
> to allow access to the MMD PHY registers.
>
> The MMD PHY registers can be accessed by several means:
>
> 1. Using two new MMD access function hooks in the PHY driver. These
> functions can be implemented when the PHY driver does not support the
> standard IEEE Compatible clause 45 access mechanism described in clause
> 22 or if the PHY uses its own non-standard access mechanism.
>
> 2. Direct access for C45 PHYs and C22 PHYs when accessing the reachable
> DEVADs.
>
> 3. The standard clause 45 access extensions to the MMD registers through
> the indirection registers (clause 22) in all the other cases.
>
> Signed-off-by: Carlo Caione <ccaione@baylibre.com>

Except for the off-by-ones below...
Acked-by: Joe Hershberger <joe.hershberger@ni.com>

> ---
>  drivers/net/phy/phy.c |  4 +++
>  include/phy.h         | 70 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 74 insertions(+)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index cda4caa803..6769047407 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -549,6 +549,10 @@ int phy_register(struct phy_driver *drv)
>                 drv->readext += gd->reloc_off;
>         if (drv->writeext)
>                 drv->writeext += gd->reloc_off;
> +       if (drv->read_mmd)
> +               drv->read_mmd += gd->reloc_off;
> +       if (drv->write_mmd)
> +               drv->write_mmd += gd->reloc_off;
>  #endif
>         return 0;
>  }
> diff --git a/include/phy.h b/include/phy.h
> index b86fdfb2ce..7ec2b4e86c 100644
> --- a/include/phy.h
> +++ b/include/phy.h
> @@ -101,6 +101,14 @@ struct phy_driver {
>         int (*readext)(struct phy_device *phydev, int addr, int devad, int reg);
>         int (*writeext)(struct phy_device *phydev, int addr, int devad, int reg,
>                         u16 val);
> +
> +       /* Phy specific driver override for reading a MMD register */
> +       int (*read_mmd)(struct phy_device *phydev, int devad, int reg);
> +
> +       /* Phy specific driver override for writing a MMD register */
> +       int (*write_mmd)(struct phy_device *phydev, int devad, int reg,
> +                        u16 val);
> +
>         struct list_head list;
>  };
>
> @@ -164,6 +172,68 @@ static inline int phy_write(struct phy_device *phydev, int devad, int regnum,
>         return bus->write(bus, phydev->addr, devad, regnum, val);
>  }
>
> +static inline void phy_mmd_start_indirect(struct phy_device *phydev, int devad,
> +                                         int regnum)
> +{
> +       /* Write the desired MMD Devad */
> +       phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_CTRL, devad);
> +
> +       /* Write the desired MMD register address */
> +       phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_DATA, regnum);
> +
> +       /* Select the Function : DATA with no post increment */
> +       phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_CTRL,
> +                 (devad | MII_MMD_CTRL_NOINCR));
> +}
> +
> +static inline int phy_read_mmd(struct phy_device *phydev, int devad,
> +                              int regnum)
> +{
> +       struct phy_driver *drv = phydev->drv;
> +
> +       if (regnum > (u16)~0 || devad > 32)

Shouldn't this be >= 32?

> +               return -EINVAL;
> +
> +       /* driver-specific access */
> +       if (drv->read_mmd)
> +               return drv->read_mmd(phydev, devad, regnum);
> +
> +       /* direct C45 / C22 access */
> +       if ((drv->features & PHY_10G_FEATURES) == PHY_10G_FEATURES ||
> +           devad == MDIO_DEVAD_NONE || !devad)
> +               return phy_read(phydev, devad, regnum);
> +
> +       /* indirect C22 access */
> +       phy_mmd_start_indirect(phydev, devad, regnum);
> +
> +       /* Read the content of the MMD's selected register */
> +       return phy_read(phydev, MDIO_DEVAD_NONE, MII_MMD_DATA);
> +}
> +
> +static inline int phy_write_mmd(struct phy_device *phydev, int devad,
> +                               int regnum, u16 val)
> +{
> +       struct phy_driver *drv = phydev->drv;
> +
> +       if (regnum > (u16)~0 || devad > 32)

Same here.

> +               return -EINVAL;
> +
> +       /* driver-specific access */
> +       if (drv->write_mmd)
> +               return drv->write_mmd(phydev, devad, regnum, val);
> +
> +       /* direct C45 / C22 access */
> +       if ((drv->features & PHY_10G_FEATURES) == PHY_10G_FEATURES ||
> +           devad == MDIO_DEVAD_NONE || !devad)
> +               return phy_write(phydev, devad, regnum, val);
> +
> +       /* indirect C22 access */
> +       phy_mmd_start_indirect(phydev, devad, regnum);
> +
> +       /* Write the data into MMD's selected register */
> +       return phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_DATA, val);
> +}
> +
>  #ifdef CONFIG_PHYLIB_10G
>  extern struct phy_driver gen10g_driver;
>
> --
> 2.19.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] net: phy: Add generic helpers to access MMD PHY registers
  2019-02-08 17:25 ` [U-Boot] [PATCH v5 1/3] net: phy: Add generic helpers to access MMD PHY registers Carlo Caione
  2019-02-19 18:14   ` Joe Hershberger
@ 2019-03-05 18:04   ` Joe Hershberger
  1 sibling, 0 replies; 12+ messages in thread
From: Joe Hershberger @ 2019-03-05 18:04 UTC (permalink / raw)
  To: u-boot

Hi Carlo,

https://patchwork.ozlabs.org/patch/1038818/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] net: phy: ti: use generic helpers to access MMD registers
  2019-02-08 17:25 ` [U-Boot] [PATCH v5 2/3] net: phy: ti: use generic helpers to access MMD registers Carlo Caione
@ 2019-03-05 18:04   ` Joe Hershberger
  0 siblings, 0 replies; 12+ messages in thread
From: Joe Hershberger @ 2019-03-05 18:04 UTC (permalink / raw)
  To: u-boot

Hi Carlo,

https://patchwork.ozlabs.org/patch/1038820/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] cmd: mdio: Switch to generic helpers when accessing the registers
  2019-02-08 17:25 ` [U-Boot] [PATCH v5 3/3] cmd: mdio: Switch to generic helpers when accessing the registers Carlo Caione
  2019-02-12 12:20   ` Vladimir Oltean
  2019-02-19 18:06   ` Joe Hershberger
@ 2019-03-05 18:04   ` Joe Hershberger
  2 siblings, 0 replies; 12+ messages in thread
From: Joe Hershberger @ 2019-03-05 18:04 UTC (permalink / raw)
  To: u-boot

Hi Carlo,

https://patchwork.ozlabs.org/patch/1038821/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-03-05 18:04 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-08 17:25 [U-Boot] [PATCH v5 0/3] Add MMD PHY helpers Carlo Caione
2019-02-08 17:25 ` [U-Boot] [PATCH v5 1/3] net: phy: Add generic helpers to access MMD PHY registers Carlo Caione
2019-02-19 18:14   ` Joe Hershberger
2019-03-05 18:04   ` [U-Boot] " Joe Hershberger
2019-02-08 17:25 ` [U-Boot] [PATCH v5 2/3] net: phy: ti: use generic helpers to access MMD registers Carlo Caione
2019-03-05 18:04   ` [U-Boot] " Joe Hershberger
2019-02-08 17:25 ` [U-Boot] [PATCH v5 3/3] cmd: mdio: Switch to generic helpers when accessing the registers Carlo Caione
2019-02-12 12:20   ` Vladimir Oltean
2019-02-15 22:46     ` Vladimir Oltean
2019-02-16 10:57       ` Carlo Caione
2019-02-19 18:06   ` Joe Hershberger
2019-03-05 18:04   ` [U-Boot] " Joe Hershberger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.