Share the code that prints out a register field with the function that prints out the "special" fields.

There were two arrays the register dump list, one with reg number and name, another with a pointer to the field table and the table size. These two arrays had have each entry match what register is referred to. Combine them into just one table. Now they can't not match and there is just one table.

Add some missing consts to pointers to string literals.

The dump code was ignoring the regno field in the description table and assuming register 0 was at index 0, etc. Have it use the field.

Change reg > max+1 into reg >= max, which doesn't fail if max+1 could overflow, besides just making more sense.

Signed-off-by: Trent Piepho tpiepho@impinj.com --- cmd/mii.c | 154 ++++++++++++++++++++++++++++---------------------------------- 1 file changed, 69 insertions(+), 85 deletions(-)

diff --git a/cmd/mii.c b/cmd/mii.c index c0c42a851f..fcc677b485 100644 --- a/cmd/mii.c +++ b/cmd/mii.c @@ -12,25 +12,11 @@ #include <command.h> #include <miiphy.h>

-typedef struct _MII_reg_desc_t { - ushort regno; - char * name; -} MII_reg_desc_t; - -static const MII_reg_desc_t reg_0_5_desc_tbl[] = { - { MII_BMCR, "PHY control register" }, - { MII_BMSR, "PHY status register" }, - { MII_PHYSID1, "PHY ID 1 register" }, - { MII_PHYSID2, "PHY ID 2 register" }, - { MII_ADVERTISE, "Autonegotiation advertisement register" }, - { MII_LPA, "Autonegotiation partner abilities register" }, -}; - typedef struct _MII_field_desc_t { ushort hi; ushort lo; ushort mask; - char * name; + const char *name; } MII_field_desc_t;

static const MII_field_desc_t reg_0_desc_tbl[] = { @@ -87,7 +73,7 @@ static const MII_field_desc_t reg_4_desc_tbl[] = { { 7, 7, 0x01, "100BASE-TX able" }, { 6, 6, 0x01, "10BASE-T full duplex able" }, { 5, 5, 0x01, "10BASE-T able" }, - { 4, 0, 0x1f, "xxx to do" }, + { 4, 0, 0x1f, "selector" }, };

static const MII_field_desc_t reg_5_desc_tbl[] = { @@ -102,50 +88,65 @@ static const MII_field_desc_t reg_5_desc_tbl[] = { { 7, 7, 0x01, "100BASE-TX able" }, { 6, 6, 0x01, "10BASE-T full duplex able" }, { 5, 5, 0x01, "10BASE-T able" }, - { 4, 0, 0x1f, "xxx to do" }, + { 4, 0, 0x1f, "partner selector" }, }; -typedef struct _MII_field_desc_and_len_t { + +typedef struct _MII_reg_desc_t { + ushort regno; const MII_field_desc_t *pdesc; ushort len; -} MII_field_desc_and_len_t; - -static const MII_field_desc_and_len_t desc_and_len_tbl[] = { - { reg_0_desc_tbl, ARRAY_SIZE(reg_0_desc_tbl) }, - { reg_1_desc_tbl, ARRAY_SIZE(reg_1_desc_tbl) }, - { reg_2_desc_tbl, ARRAY_SIZE(reg_2_desc_tbl) }, - { reg_3_desc_tbl, ARRAY_SIZE(reg_3_desc_tbl) }, - { reg_4_desc_tbl, ARRAY_SIZE(reg_4_desc_tbl) }, - { reg_5_desc_tbl, ARRAY_SIZE(reg_5_desc_tbl) }, + const char *name; +} MII_reg_desc_t; + +static const MII_reg_desc_t mii_reg_desc_tbl[] = { + { MII_BMCR, reg_0_desc_tbl, ARRAY_SIZE(reg_0_desc_tbl), + "PHY control register" }, + { MII_BMSR, reg_1_desc_tbl, ARRAY_SIZE(reg_1_desc_tbl), + "PHY status register" }, + { MII_PHYSID1, reg_2_desc_tbl, ARRAY_SIZE(reg_2_desc_tbl), + "PHY ID 1 register" }, + { MII_PHYSID2, reg_3_desc_tbl, ARRAY_SIZE(reg_3_desc_tbl), + "PHY ID 2 register" }, + { MII_ADVERTISE, reg_4_desc_tbl, ARRAY_SIZE(reg_4_desc_tbl), + "Autonegotiation advertisement register" }, + { MII_LPA, reg_5_desc_tbl, ARRAY_SIZE(reg_5_desc_tbl), + "Autonegotiation partner abilities register" }, };

static void dump_reg( ushort regval, - const MII_reg_desc_t *prd, - const MII_field_desc_and_len_t *pdl); - -static int special_field( - ushort regno, - const MII_field_desc_t *pdesc, - ushort regval); - -static void MII_dump_0_to_5( - ushort regvals[6], - uchar reglo, - uchar reghi) + const MII_reg_desc_t *prd); + +static bool special_field(ushort regno, const MII_field_desc_t *pdesc, + ushort regval); + +static void MII_dump(const ushort *regvals, uchar reglo, uchar reghi) { ulong i;

- for (i = 0; i < 6; i++) { - if ((reglo <= i) && (i <= reghi)) - dump_reg(regvals[i], &reg_0_5_desc_tbl[i], - &desc_and_len_tbl[i]); + for (i = 0; i < ARRAY_SIZE(mii_reg_desc_tbl); i++) { + const uchar reg = mii_reg_desc_tbl[i].regno; + + if (reg >= reglo && reg <= reghi) + dump_reg(regvals[reg - reglo], &mii_reg_desc_tbl[i]); } }

+/* Print out field position, value, name */ +static void dump_field(const MII_field_desc_t *pdesc, ushort regval) +{ + if (pdesc->hi == pdesc->lo) + printf("%2u ", pdesc->lo); + else + printf("%2u-%2u", pdesc->hi, pdesc->lo); + + printf(" = %5u %s", (regval >> pdesc->lo) & pdesc->mask, + pdesc->name); +} + static void dump_reg( ushort regval, - const MII_reg_desc_t *prd, - const MII_field_desc_and_len_t *pdl) + const MII_reg_desc_t *prd) { ulong i; ushort mask_in_place; @@ -154,8 +155,8 @@ static void dump_reg( printf("%u. (%04hx) -- %s --\n", prd->regno, regval, prd->name);

- for (i = 0; i < pdl->len; i++) { - pdesc = &pdl->pdesc[i]; + for (i = 0; i < prd->len; i++) { + pdesc = &prd->pdesc[i];

mask_in_place = pdesc->mask << pdesc->lo;

@@ -164,17 +165,8 @@ static void dump_reg( regval & mask_in_place, prd->regno);

- if (special_field(prd->regno, pdesc, regval)) { - } - else { - if (pdesc->hi == pdesc->lo) - printf("%2u ", pdesc->lo); - else - printf("%2u-%2u", pdesc->hi, pdesc->lo); - printf(" = %5u %s", - (regval & mask_in_place) >> pdesc->lo, - pdesc->name); - } + if (!special_field(prd->regno, pdesc, regval)) + dump_field(pdesc, regval); printf("\n");

} @@ -190,11 +182,11 @@ static void dump_reg( ** 5.4-0 */

-static int special_field( - ushort regno, - const MII_field_desc_t *pdesc, - ushort regval) +static bool special_field(ushort regno, const MII_field_desc_t *pdesc, + ushort regval) { + const ushort sel_bits = (regval >> pdesc->lo) & pdesc->mask; + if ((regno == MII_BMCR) && (pdesc->lo == 6)) { ushort speed_bits = regval & (BMCR_SPEED1000 | BMCR_SPEED100); printf("%2u,%2u = b%u%u speed selection = %s Mbps", @@ -208,34 +200,26 @@ static int special_field( }

else if ((regno == MII_BMCR) && (pdesc->lo == 8)) { - printf("%2u = %5u duplex = %s", - pdesc->lo, - (regval >> pdesc->lo) & 1, - ((regval >> pdesc->lo) & 1) ? "full" : "half"); + dump_field(pdesc, regval); + printf(" = %s", ((regval >> pdesc->lo) & 1) ? "full" : "half"); return 1; }

else if ((regno == MII_ADVERTISE) && (pdesc->lo == 0)) { - ushort sel_bits = (regval >> pdesc->lo) & pdesc->mask; - printf("%2u-%2u = %5u selector = %s", - pdesc->hi, pdesc->lo, sel_bits, - sel_bits == PHY_ANLPAR_PSB_802_3 ? - "IEEE 802.3" : - sel_bits == PHY_ANLPAR_PSB_802_9 ? - "IEEE 802.9 ISLAN-16T" : - "???"); + dump_field(pdesc, regval); + printf(" = %s", + sel_bits == PHY_ANLPAR_PSB_802_3 ? "IEEE 802.3 CSMA/CD" : + sel_bits == PHY_ANLPAR_PSB_802_9 ? "IEEE 802.9 ISLAN-16T" : + "???"); return 1; }

else if ((regno == MII_LPA) && (pdesc->lo == 0)) { - ushort sel_bits = (regval >> pdesc->lo) & pdesc->mask; - printf("%2u-%2u = %u selector = %s", - pdesc->hi, pdesc->lo, sel_bits, - sel_bits == PHY_ANLPAR_PSB_802_3 ? - "IEEE 802.3" : - sel_bits == PHY_ANLPAR_PSB_802_9 ? - "IEEE 802.9 ISLAN-16T" : - "???"); + dump_field(pdesc, regval); + printf(" = %s", + sel_bits == PHY_ANLPAR_PSB_802_3 ? "IEEE 802.3 CSMA/CD" : + sel_bits == PHY_ANLPAR_PSB_802_9 ? "IEEE 802.9 ISLAN-16T" : + "???"); return 1; }

@@ -415,8 +399,8 @@ static int do_mii(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return 1; } for (addr = addrlo; addr <= addrhi; addr++) { - for (reg = reglo; reg < reghi + 1; reg++) { - if (miiphy_read(devname, addr, reg, &regs[reg]) != 0) { + for (reg = reglo; reg <= reghi; reg++) { + if (miiphy_read(devname, addr, reg, &regs[reg - reglo]) != 0) { ok = 0; printf( "Error reading from the PHY addr=%02x reg=%02x\n", @@ -425,7 +409,7 @@ static int do_mii(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } } if (ok) - MII_dump_0_to_5(regs, reglo, reghi); + MII_dump(regs, reglo, reghi); printf("\n"); } } else if (strncmp(op, "de", 2) == 0) {