On Thu, Jul 16, 2020 at 10:55:26AM -0700, Chris Healy wrote: > From: Andrew Lunn <and...@lunn.ch> > > In addition to the port registers, the device can provide the > SERDES/PCS registers. Dump these, and for a few of the important > SGMII/1000Base-X registers decode the bits. > > Signed-off-by: Andrew Lunn <and...@lunn.ch> > Signed-off-by: Chris Healy <cphe...@gmail.com> > --- [...] > + case 32 + 0: > + REG(reg - 32, "Fiber Control", val);
Could you give these "32" (and similar below) a name?
[...]
> @@ -667,6 +850,17 @@ static int dsa_mv88e6xxx_dump_regs(struct ethtool_regs
> *regs)
> else
> REG(i, "", data[i]);
>
> + /* Dump the SERDES registers, if provided */
> + if (regs->len > 32 * 2) {
sizeof(u16) would be easier to read, IMHO
> + printf("\n%s Switch Port SERDES Registers\n", sw->name);
> + printf("-------------------------------------\n");
> + for (i = 32; i < regs->len / 2; i++)
> + if (sw->dump)
> + sw->dump(i, data[i]);
> + else
> + REG(i, "", data[i]);
In the dump handler above you subtract 32 (offset of SERDES registers,
IIUC) from register number but in the generic branch you don't, this
seems inconsistent.
Michal
> + }
> +
> return 0;
> }
>
> --
> 2.21.3
>
signature.asc
Description: PGP signature
