Patchwork [U-Boot,5/7] Add mdio command for new PHY infrastructure

login
register
mail settings
Submitter Andy Fleming
Date March 29, 2011, 7:30 p.m.
Message ID <1301427010-7429-6-git-send-email-afleming@freescale.com>
Download mbox | patch
Permalink /patch/88846/
State Superseded
Headers show

Comments

Andy Fleming - March 29, 2011, 7:30 p.m.
The new mdio command doesn't have all of the features of the mii
command, but it provides the necessary read/write primitives, and allows
users to interact with 10G PHYs, and other PHYs which use Clause 45 of
802.3.  This means that the mdio command requires a "Device Address"
argument, though for clause 22 PHYs, the argument can be "-".

Signed-off-by: Andy Fleming <afleming@freescale.com>
---
 common/Makefile   |    3 +
 common/cmd_mdio.c |  293 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 296 insertions(+), 0 deletions(-)
 create mode 100644 common/cmd_mdio.c
Detlev Zundel - March 30, 2011, 11:55 a.m.
Hi Andy,

> The new mdio command doesn't have all of the features of the mii
> command, but it provides the necessary read/write primitives, and allows
> users to interact with 10G PHYs, and other PHYs which use Clause 45 of
> 802.3.  This means that the mdio command requires a "Device Address"
> argument, though for clause 22 PHYs, the argument can be "-".
>
> Signed-off-by: Andy Fleming <afleming@freescale.com>
> ---
>  common/Makefile   |    3 +
>  common/cmd_mdio.c |  293 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 296 insertions(+), 0 deletions(-)
>  create mode 100644 common/cmd_mdio.c

[...]

> +static int extract_range(char *input, int *plo, int *phi)
> +{
> +	char * end;
> +	*plo = simple_strtol(input, &end, 0);
> +	if (end == input)
> +		return -1;
> +
> +	if (*end == '-') {

What about the case of input="12-"? Shouldn't there be an "&& *(end+1)"?

> +		end++;
> +		*phi = simple_strtol(end, NULL, 0);
> +	} else if (*end == '\0') {
> +		*phi = *plo;
> +	} else
> +		return -1;
> +
> +	return 0;
> +}

Other than that, looks good, so once my above concern is addressed

Acked-by: Detlev Zundel <dzu@denx.de>

Thanks!
  Detlev
Andy Fleming - March 30, 2011, 11:05 p.m.
On Mar 30, 2011, at 6:55 AM, Detlev Zundel wrote:

> Hi Andy,
> 
> 
>> +static int extract_range(char *input, int *plo, int *phi)
>> +{
>> +	char * end;
>> +	*plo = simple_strtol(input, &end, 0);
>> +	if (end == input)
>> +		return -1;
>> +
>> +	if (*end == '-') {
> 
> What about the case of input="12-"? Shouldn't there be an "&& *(end+1)"?


Ok.  It should be noted that cmd_mii.c has the same issue.

I'm about to send out the v2.

Andy
Mike Frysinger - March 30, 2011, 11:18 p.m.
On Tue, Mar 29, 2011 at 3:30 PM, Andy Fleming wrote:
> +       "  <busname>  <addr>\n"
> +       "  <busname>:<addr>\n"

why bother ?  pick one and go with it

[<busname>:]<addr>
-mike

Patch

diff --git a/common/Makefile b/common/Makefile
index 00847ef..a644627 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -116,6 +116,9 @@  COBJS-$(CONFIG_MII) += miiphyutil.o
 COBJS-$(CONFIG_CMD_MII) += miiphyutil.o
 COBJS-$(CONFIG_PHYLIB) += miiphyutil.o
 COBJS-$(CONFIG_CMD_MII) += cmd_mii.o
+ifdef CONFIG_PHYLIB
+COBJS-$(CONFIG_CMD_MII) += cmd_mdio.o
+endif
 COBJS-$(CONFIG_CMD_MISC) += cmd_misc.o
 COBJS-$(CONFIG_CMD_MMC) += cmd_mmc.o
 COBJS-$(CONFIG_MP) += cmd_mp.o
diff --git a/common/cmd_mdio.c b/common/cmd_mdio.c
new file mode 100644
index 0000000..3ab0bef
--- /dev/null
+++ b/common/cmd_mdio.c
@@ -0,0 +1,293 @@ 
+/*
+ * (C) Copyright 2011 Freescale Semiconductor, Inc
+ * Andy Fleming
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+/*
+ * MDIO Commands
+ */
+
+#include <common.h>
+#include <command.h>
+#include <miiphy.h>
+#include <phy.h>
+
+
+static char last_op[2];
+static uint last_data;
+static uint last_addr_lo;
+static uint last_addr_hi;
+static uint last_devad_lo;
+static uint last_devad_hi;
+static uint last_reg_lo;
+static uint last_reg_hi;
+
+static int extract_range(char *input, int *plo, int *phi)
+{
+	char * end;
+	*plo = simple_strtol(input, &end, 0);
+	if (end == input)
+		return -1;
+
+	if (*end == '-') {
+		end++;
+		*phi = simple_strtol(end, NULL, 0);
+	} else if (*end == '\0') {
+		*phi = *plo;
+	} else
+		return -1;
+
+	return 0;
+}
+
+int mdio_write_ranges(struct mii_dev *bus, int addrlo,
+			int addrhi, int devadlo, int devadhi,
+			int reglo, int reghi, unsigned short data)
+{
+	int addr, devad, reg;
+	int err = 0;
+
+	for (addr = addrlo; addr <= addrhi; addr++) {
+		for (devad = devadlo; devad <= devadhi; devad++) {
+			for (reg = reglo; reg <= reghi; reg++) {
+				err = bus->write(bus, addr, devad, reg, data);
+
+				if (err)
+					goto err_out;
+			}
+		}
+	}
+
+err_out:
+	return err;
+}
+
+int mdio_read_ranges(struct mii_dev *bus, int addrlo,
+			int addrhi, int devadlo, int devadhi,
+			int reglo, int reghi)
+{
+	int addr, devad, reg;
+
+	printf("Reading from bus %s\n", bus->name);
+	for (addr = addrlo; addr <= addrhi; addr++) {
+		printf("PHY at address %d:\n", addr);
+				
+		for (devad = devadlo; devad <= devadhi; devad++) {
+			for (reg = reglo; reg <= reghi; reg++) {
+				u16 val;
+				val = bus->read(bus, addr, devad, reg);
+
+				if (val < 0) {
+					printf("Error\n");
+
+					return val;
+				}
+
+				if (devad >= 0)
+					printf("%d.", devad);
+
+				printf("%d - 0x%x\n", reg, val & 0xffff);
+			}
+		}
+	}
+
+	return 0;
+}
+
+int extract_phy_range(char * const argv[], int argc, struct mii_dev **bus,
+		int *addrlo, int *addrhi)
+{
+	struct phy_device *phydev;
+	char *addrstr;
+	int addrlen, namelen;
+	char busname[MDIO_NAME_LEN];
+
+	if ((argc < 1) || (argc > 2))
+		return -1;
+
+	/* If there are two arguments, it's busname addr */
+	if (argc == 2) {
+		*bus = miiphy_get_dev_by_name(argv[0]);
+
+		if (!*bus)
+			return -1;
+
+		return extract_range(argv[1], addrlo, addrhi);
+	}
+
+	/* It must be one argument, here */
+
+	/*
+	 * This argument can be one of three things:
+	 * 1) Ethernet device name
+	 * 2) bus name and address in one string (name:addr)
+	 * 3) Just an address (use the previously-used bus)
+	 *
+	 * We'll assume that no ethernet device will have a name that collides
+	 * with the format for busname:addr.  So we check all buses for a PHY
+	 * which is connected to that ethernet device.  If none are found, we
+	 * look for a ':'.  If it's found, then this is busname:addr. If so,
+	 * we split the string, find the bus, and call extract_range() on the
+	 * addr.  If we didn't find ':', then we just call extract_range() on
+	 * the string, and see if it's an address or address range.
+	 */
+	phydev = mdio_phydev_for_ethname(argv[0]);
+
+	if (phydev) {
+		*addrlo = *addrhi = phydev->addr;
+		*bus = phydev->bus;
+
+		return 0;
+	}
+
+	/* use strrchr to find the last string after a ':' */
+	addrstr = strrchr(argv[0], ':');
+
+	/*
+	 * If it doesn't exist, or is not a valid range
+	 * check to see if the whole string is a valid range
+	 */
+	if (!addrstr || extract_range(++addrstr, addrlo, addrhi))
+		return extract_range(argv[0], addrlo, addrhi);
+
+	/* Use the length of the returned string to determine where it starts */
+	addrlen = strlen(addrstr);
+	namelen = strlen(argv[0]) - addrlen - 1;
+
+	if (namelen >= MDIO_NAME_LEN)
+		namelen = MDIO_NAME_LEN - 1;
+	/* Copy the name string, and search for a bus with that name */
+	strncpy(busname, argv[0], namelen);
+	busname[namelen] = '\0';
+
+	*bus = miiphy_get_dev_by_name(busname);
+
+	if (!*bus)
+		return -1;
+
+	return 0;
+}
+
+/* ---------------------------------------------------------------- */
+static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	char op[2];
+	int addrlo, addrhi, reglo, reghi, devadlo, devadhi;
+	unsigned short	data;
+	int pos = argc - 1;
+	struct mii_dev *bus;
+
+	if (argc < 2)
+		return cmd_usage(cmdtp);
+
+	/*
+	 * We use the last specified parameters, unless new ones are
+	 * entered.
+	 */
+	op[0] = argv[1][0];
+	addrlo = last_addr_lo;
+	addrhi = last_addr_hi;
+	devadlo = last_devad_lo;
+	devadhi = last_devad_hi;
+	reglo  = last_reg_lo;
+	reghi  = last_reg_hi;
+	data   = last_data;
+
+	bus = mdio_get_current_dev();
+
+	if (flag & CMD_FLAG_REPEAT)
+		op[0] = last_op[0];
+
+	switch (op[0]) {
+	case 'w':
+		if (pos > 1)
+			data = simple_strtoul (argv[pos--], NULL, 16);
+	case 'r':
+		if (pos > 1)
+			if (extract_range(argv[pos--], &reglo, &reghi))
+				return -1;
+		if (pos > 1) {
+			if (argv[pos][0] == '-')
+				devadlo = devadhi = MDIO_DEVAD_NONE;
+			else if (extract_range(argv[pos], &devadlo, &devadhi))
+				return -1;
+		}
+	default:
+		if (pos > 2)
+			if (extract_phy_range(&(argv[2]), pos - 2, &bus,
+					&addrlo, &addrhi))
+				return -1;
+
+		break;
+	}
+
+	if (op[0] == 'l') {
+		mdio_list_devices();
+
+		return 0;
+	}
+
+	/* Save the chosen bus */
+	miiphy_set_current_dev(bus->name);
+
+	switch (op[0]) {
+	case 'w':
+		mdio_write_ranges(bus, addrlo, addrhi, devadlo, devadhi,
+				reglo, reghi, data);
+		break;
+
+	case 'r':
+		mdio_read_ranges(bus, addrlo, addrhi, devadlo, devadhi,
+				reglo, reghi);
+		break;
+	}
+
+	/*
+	 * Save the parameters for repeats.
+	 */
+	last_op[0] = op[0];
+	last_addr_lo = addrlo;
+	last_addr_hi = addrhi;
+	last_devad_lo = devadlo;
+	last_devad_hi = devadhi;
+	last_reg_lo  = reglo;
+	last_reg_hi  = reghi;
+	last_data    = data;
+
+	return 0;
+}
+
+/***************************************************/
+
+U_BOOT_CMD(
+	mdio,	7,	1,	do_mdio,
+	"MDIO utility commands",
+	"list			- List MDIO buses\n"
+	"mdio read <phydev> <devad> <reg> - read PHY's register at <devad> <reg>\n"
+	"mdio write <phydev> <devad> <reg> <data> - write PHY's register at <devad> <reg>\n"
+	"<phydev> may be:\n"
+	"  <busname>  <addr>\n"
+	"  <busname>:<addr>\n"
+	"  <addr>\n"
+	"  <eth name>\n"
+	"Addr devad, and reg may be ranges, e.g. 2-7.\n"
+	"devad should be '-' for standard MII (802.3 Clause 22)."
+);