All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 0/3] Add MMD PHY helpers
@ 2019-01-23 13:13 Carlo Caione
  2019-01-23 13:13 ` [U-Boot] [PATCH v2 1/3] net: phy: Add support for accessing MMD PHY registers Carlo Caione
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Carlo Caione @ 2019-01-23 13:13 UTC (permalink / raw)
  To: u-boot

Introduce phy_(read|write)_mmd() generic 802.3 clause 45 register
accessors for Clause 22 PHYs, using the indirect method. Allow this
behaviour to be overriden by PHY drivers where necessary.

Carlo Caione (3):
  net: phy: Add support for accessing MMD PHY registers
  net: phy: ti: use generic helpers to access MMD registers
  cmd: mdio: Add new parameter to access MMD PHY registers

 cmd/mdio.c            |  33 +++++++++--
 drivers/net/phy/phy.c |   4 ++
 drivers/net/phy/ti.c  | 130 ++++++++----------------------------------
 include/phy.h         |  62 ++++++++++++++++++++
 4 files changed, 118 insertions(+), 111 deletions(-)

-- 
2.19.1

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

* [U-Boot] [PATCH v2 1/3] net: phy: Add support for accessing MMD PHY registers
  2019-01-23 13:13 [U-Boot] [PATCH v2 0/3] Add MMD PHY helpers Carlo Caione
@ 2019-01-23 13:13 ` Carlo Caione
  2019-01-23 14:12   ` Joe Hershberger
  2019-01-23 13:13 ` [U-Boot] [PATCH v2 2/3] net: phy: ti: use generic helpers to access MMD registers Carlo Caione
  2019-01-23 13:13 ` [U-Boot] [PATCH v2 3/3] cmd: mdio: Add new parameter to access MMD PHY registers Carlo Caione
  2 siblings, 1 reply; 7+ messages in thread
From: Carlo Caione @ 2019-01-23 13:13 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 two 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. 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         | 62 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 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..0ce41661fa 100644
--- a/include/phy.h
+++ b/include/phy.h
@@ -101,6 +101,13 @@ 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 +171,61 @@ 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_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)
+{
+	int ret;
+
+	if (regnum > (u16)~0 || devad > 32)
+		return -EINVAL;
+
+	if (phydev->drv->read_mmd) {
+		ret = phydev->drv->read_mmd(phydev, devad, regnum);
+	} else {
+		phy_mmd_indirect(phydev, devad, regnum);
+
+		/* Read the content of the MMD's selected register */
+		ret = phy_read(phydev, MDIO_DEVAD_NONE, MII_MMD_DATA);
+	}
+
+	return ret;
+}
+
+static inline int phy_write_mmd(struct phy_device *phydev, int devad,
+				int regnum, u16 val)
+{
+	int ret;
+
+	if (regnum > (u16)~0 || devad > 32)
+		return -EINVAL;
+
+	if (phydev->drv->write_mmd) {
+		ret = phydev->drv->write_mmd(phydev, devad, regnum, val);
+	} else {
+		phy_mmd_indirect(phydev, devad, regnum);
+
+		/* Write the data into MMD's selected register */
+		phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_DATA, val);
+
+		ret = 0;
+	}
+
+	return ret;
+}
+
 #ifdef CONFIG_PHYLIB_10G
 extern struct phy_driver gen10g_driver;
 
-- 
2.19.1

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

* [U-Boot] [PATCH v2 2/3] net: phy: ti: use generic helpers to access MMD registers
  2019-01-23 13:13 [U-Boot] [PATCH v2 0/3] Add MMD PHY helpers Carlo Caione
  2019-01-23 13:13 ` [U-Boot] [PATCH v2 1/3] net: phy: Add support for accessing MMD PHY registers Carlo Caione
@ 2019-01-23 13:13 ` Carlo Caione
  2019-01-23 14:15   ` Joe Hershberger
  2019-01-23 13:13 ` [U-Boot] [PATCH v2 3/3] cmd: mdio: Add new parameter to access MMD PHY registers Carlo Caione
  2 siblings, 1 reply; 7+ messages in thread
From: Carlo Caione @ 2019-01-23 13:13 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>
---
 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] 7+ messages in thread

* [U-Boot] [PATCH v2 3/3] cmd: mdio: Add new parameter to access MMD PHY registers
  2019-01-23 13:13 [U-Boot] [PATCH v2 0/3] Add MMD PHY helpers Carlo Caione
  2019-01-23 13:13 ` [U-Boot] [PATCH v2 1/3] net: phy: Add support for accessing MMD PHY registers Carlo Caione
  2019-01-23 13:13 ` [U-Boot] [PATCH v2 2/3] net: phy: ti: use generic helpers to access MMD registers Carlo Caione
@ 2019-01-23 13:13 ` Carlo Caione
  2019-01-23 14:01   ` Joe Hershberger
  2 siblings, 1 reply; 7+ messages in thread
From: Carlo Caione @ 2019-01-23 13:13 UTC (permalink / raw)
  To: u-boot

Two new parameters (rmmd and wmmd) are added to allow the `mdio` command
to access the content of the MMD PHY registers.

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

diff --git a/cmd/mdio.c b/cmd/mdio.c
index 184868063a..010632b562 100644
--- a/cmd/mdio.c
+++ b/cmd/mdio.c
@@ -43,7 +43,7 @@ static int mdio_write_ranges(struct phy_device *phydev, struct mii_dev *bus,
 			     int addrlo,
 			     int addrhi, int devadlo, int devadhi,
 			     int reglo, int reghi, unsigned short data,
-			     int extended)
+			     int extended, int mmd)
 {
 	int addr, devad, reg;
 	int err = 0;
@@ -51,7 +51,9 @@ static int mdio_write_ranges(struct phy_device *phydev, struct mii_dev *bus,
 	for (addr = addrlo; addr <= addrhi; addr++) {
 		for (devad = devadlo; devad <= devadhi; devad++) {
 			for (reg = reglo; reg <= reghi; reg++) {
-				if (!extended)
+				if (mmd)
+					err = phy_write_mmd(phydev, devad, reg, data);
+				else if (!extended)
 					err = bus->write(bus, addr, devad,
 							 reg, data);
 				else
@@ -71,7 +73,7 @@ err_out:
 static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
 			    int addrlo,
 			    int addrhi, int devadlo, int devadhi,
-			    int reglo, int reghi, int extended)
+			    int reglo, int reghi, int extended, int mmd)
 {
 	int addr, devad, reg;
 
@@ -83,7 +85,9 @@ static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
 			for (reg = reglo; reg <= reghi; reg++) {
 				int val;
 
-				if (!extended)
+				if (mmd)
+					val = phy_read_mmd(phydev, devad, reg);
+				else if (!extended)
 					val = bus->read(bus, addr, devad, reg);
 				else
 					val = phydev->drv->readext(phydev, addr,
@@ -189,6 +193,7 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	struct mii_dev *bus;
 	struct phy_device *phydev = NULL;
 	int extended = 0;
+	int mmd = 0;
 
 	if (argc < 2)
 		return CMD_RET_USAGE;
@@ -232,6 +237,18 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 				return -1;
 			}
 		}
+		if (op[1] == 'm') {
+			phydev = mdio_phydev_for_ethname(argv[2]);
+
+			if (phydev) {
+				addrlo = phydev->addr;
+				addrhi = addrlo;
+				bus = phydev->bus;
+				mmd = 1;
+			} else {
+				return -1;
+			}
+		}
 	}
 
 	switch (op[0]) {
@@ -265,12 +282,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,
-				  reglo, reghi, data, extended);
+				  reglo, reghi, data, extended, mmd);
 		break;
 
 	case 'r':
 		mdio_read_ranges(phydev, bus, addrlo, addrhi, devadlo, devadhi,
-				 reglo, reghi, extended);
+				 reglo, reghi, extended, mmd);
 		break;
 	}
 
@@ -303,6 +320,10 @@ U_BOOT_CMD(
 		"read PHY's extended register at <devad>.<reg>\n"
 	"mdio wx <phydev> [<devad>.]<reg> <data> - "
 		"write PHY's extended register at <devad>.<reg>\n"
+	"mdio rmmd <phydev> [<devad>.]<reg> - "
+		"read PHY's extended register at <devad>.<reg>\n"
+	"mdio wmmd <phydev> [<devad>.]<reg> <data> - "
+		"write PHY's extended register at <devad>.<reg>\n"
 	"<phydev> may be:\n"
 	"   <busname>  <addr>\n"
 	"   <addr>\n"
-- 
2.19.1

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

* [U-Boot] [PATCH v2 3/3] cmd: mdio: Add new parameter to access MMD PHY registers
  2019-01-23 13:13 ` [U-Boot] [PATCH v2 3/3] cmd: mdio: Add new parameter to access MMD PHY registers Carlo Caione
@ 2019-01-23 14:01   ` Joe Hershberger
  0 siblings, 0 replies; 7+ messages in thread
From: Joe Hershberger @ 2019-01-23 14:01 UTC (permalink / raw)
  To: u-boot

On Wed, Jan 23, 2019 at 7:15 AM Carlo Caione <ccaione@baylibre.com> wrote:
>
> Two new parameters (rmmd and wmmd) are added to allow the `mdio` command
> to access the content of the MMD PHY registers.
>
> Signed-off-by: Carlo Caione <ccaione@baylibre.com>
> ---
>  cmd/mdio.c | 33 +++++++++++++++++++++++++++------
>  1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/cmd/mdio.c b/cmd/mdio.c
> index 184868063a..010632b562 100644
> --- a/cmd/mdio.c
> +++ b/cmd/mdio.c
> @@ -43,7 +43,7 @@ static int mdio_write_ranges(struct phy_device *phydev, struct mii_dev *bus,
>                              int addrlo,
>                              int addrhi, int devadlo, int devadhi,
>                              int reglo, int reghi, unsigned short data,
> -                            int extended)
> +                            int extended, int mmd)
>  {
>         int addr, devad, reg;
>         int err = 0;
> @@ -51,7 +51,9 @@ static int mdio_write_ranges(struct phy_device *phydev, struct mii_dev *bus,
>         for (addr = addrlo; addr <= addrhi; addr++) {
>                 for (devad = devadlo; devad <= devadhi; devad++) {
>                         for (reg = reglo; reg <= reghi; reg++) {
> -                               if (!extended)
> +                               if (mmd)
> +                                       err = phy_write_mmd(phydev, devad, reg, data);
> +                               else if (!extended)

Please don't keep the negative logic and switch these last two.

>                                         err = bus->write(bus, addr, devad,
>                                                          reg, data);
>                                 else
> @@ -71,7 +73,7 @@ err_out:
>  static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
>                             int addrlo,
>                             int addrhi, int devadlo, int devadhi,
> -                           int reglo, int reghi, int extended)
> +                           int reglo, int reghi, int extended, int mmd)
>  {
>         int addr, devad, reg;
>
> @@ -83,7 +85,9 @@ static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
>                         for (reg = reglo; reg <= reghi; reg++) {
>                                 int val;
>
> -                               if (!extended)
> +                               if (mmd)
> +                                       val = phy_read_mmd(phydev, devad, reg);
> +                               else if (!extended)

Please don't keep the negative logic and switch these last two.

>                                         val = bus->read(bus, addr, devad, reg);
>                                 else
>                                         val = phydev->drv->readext(phydev, addr,
> @@ -189,6 +193,7 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>         struct mii_dev *bus;
>         struct phy_device *phydev = NULL;
>         int extended = 0;
> +       int mmd = 0;
>
>         if (argc < 2)
>                 return CMD_RET_USAGE;
> @@ -232,6 +237,18 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>                                 return -1;
>                         }
>                 }
> +               if (op[1] == 'm') {
> +                       phydev = mdio_phydev_for_ethname(argv[2]);
> +
> +                       if (phydev) {
> +                               addrlo = phydev->addr;
> +                               addrhi = addrlo;
> +                               bus = phydev->bus;
> +                               mmd = 1;
> +                       } else {
> +                               return -1;

Please use a valid command response macro. CMD_RET_FAILURE in this
case. Feel free to fix it in the other places in this command as well.

> +                       }
> +               }
>         }
>
>         switch (op[0]) {
> @@ -265,12 +282,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,
> -                                 reglo, reghi, data, extended);
> +                                 reglo, reghi, data, extended, mmd);
>                 break;
>
>         case 'r':
>                 mdio_read_ranges(phydev, bus, addrlo, addrhi, devadlo, devadhi,
> -                                reglo, reghi, extended);
> +                                reglo, reghi, extended, mmd);
>                 break;
>         }
>
> @@ -303,6 +320,10 @@ U_BOOT_CMD(
>                 "read PHY's extended register at <devad>.<reg>\n"
>         "mdio wx <phydev> [<devad>.]<reg> <data> - "
>                 "write PHY's extended register at <devad>.<reg>\n"
> +       "mdio rmmd <phydev> [<devad>.]<reg> - "
> +               "read PHY's extended register at <devad>.<reg>\n"
> +       "mdio wmmd <phydev> [<devad>.]<reg> <data> - "
> +               "write PHY's extended register at <devad>.<reg>\n"
>         "<phydev> may be:\n"
>         "   <busname>  <addr>\n"
>         "   <addr>\n"
> --
> 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] 7+ messages in thread

* [U-Boot] [PATCH v2 1/3] net: phy: Add support for accessing MMD PHY registers
  2019-01-23 13:13 ` [U-Boot] [PATCH v2 1/3] net: phy: Add support for accessing MMD PHY registers Carlo Caione
@ 2019-01-23 14:12   ` Joe Hershberger
  0 siblings, 0 replies; 7+ messages in thread
From: Joe Hershberger @ 2019-01-23 14:12 UTC (permalink / raw)
  To: u-boot

On Wed, Jan 23, 2019 at 7:14 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 two 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. 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         | 62 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 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..0ce41661fa 100644
> --- a/include/phy.h
> +++ b/include/phy.h
> @@ -101,6 +101,13 @@ 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 +171,61 @@ 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_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)
> +{
> +       int ret;
> +
> +       if (regnum > (u16)~0 || devad > 32)
> +               return -EINVAL;
> +
> +       if (phydev->drv->read_mmd) {
> +               ret = phydev->drv->read_mmd(phydev, devad, regnum);
> +       } else {
> +               phy_mmd_indirect(phydev, devad, regnum);

I think this function should have a name that more clearly conveys
that it doesn't actually complete an indirect access.

Maybe phy_mmd_start_indirect() ?

> +
> +               /* Read the content of the MMD's selected register */
> +               ret = phy_read(phydev, MDIO_DEVAD_NONE, MII_MMD_DATA);
> +       }
> +
> +       return ret;

You shouldn't need this variable... just return from the 2 sites that
return errors.

> +}
> +
> +static inline int phy_write_mmd(struct phy_device *phydev, int devad,
> +                               int regnum, u16 val)
> +{
> +       int ret;
> +
> +       if (regnum > (u16)~0 || devad > 32)
> +               return -EINVAL;
> +
> +       if (phydev->drv->write_mmd) {
> +               ret = phydev->drv->write_mmd(phydev, devad, regnum, val);

You could simply return from here.

> +       } else {
> +               phy_mmd_indirect(phydev, devad, regnum);
> +
> +               /* Write the data into MMD's selected register */
> +               phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_DATA, val);
> +
> +               ret = 0;

You should return any error from phy_write above and get rid of the
ret variable.

> +       }
> +
> +       return ret;
> +}
> +
>  #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] 7+ messages in thread

* [U-Boot] [PATCH v2 2/3] net: phy: ti: use generic helpers to access MMD registers
  2019-01-23 13:13 ` [U-Boot] [PATCH v2 2/3] net: phy: ti: use generic helpers to access MMD registers Carlo Caione
@ 2019-01-23 14:15   ` Joe Hershberger
  0 siblings, 0 replies; 7+ messages in thread
From: Joe Hershberger @ 2019-01-23 14:15 UTC (permalink / raw)
  To: u-boot

On Wed, Jan 23, 2019 at 7:15 AM Carlo Caione <ccaione@baylibre.com> wrote:
>
> 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>

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

end of thread, other threads:[~2019-01-23 14:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-23 13:13 [U-Boot] [PATCH v2 0/3] Add MMD PHY helpers Carlo Caione
2019-01-23 13:13 ` [U-Boot] [PATCH v2 1/3] net: phy: Add support for accessing MMD PHY registers Carlo Caione
2019-01-23 14:12   ` Joe Hershberger
2019-01-23 13:13 ` [U-Boot] [PATCH v2 2/3] net: phy: ti: use generic helpers to access MMD registers Carlo Caione
2019-01-23 14:15   ` Joe Hershberger
2019-01-23 13:13 ` [U-Boot] [PATCH v2 3/3] cmd: mdio: Add new parameter to access MMD PHY registers Carlo Caione
2019-01-23 14:01   ` 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.