RE: [PATCH net-next 06/11] net: dsa: debugfs: add port registers

[Woojung.Huh]

Vivien,

> Subject: [PATCH net-next 06/11] net: dsa: debugfs: add port registers
> 
> Add a debug filesystem "regs" entry to query a port's hardware registers
> through the .get_regs_len and .get_regs_len switch operations.
> 
> This is very convenient because it allows one to dump the registers of
> DSA links, which are not exposed to userspace.
This series will be very useful to get various debug information.
Do you have any plan to expand 32bit register access and switch-level registers
In addition to port-level registers?

> +static void dsa_debugfs_regs_read_count(struct dsa_switch *ds, int id,
> +					struct seq_file *seq, int count)
> +{
> +	u16 data[count * ETH_GSTRING_LEN];
I think this should be u16 data[count] because it is value of registers.

> +	struct ethtool_regs regs;
> +	int i;
> +
> +	ds->ops->get_regs(ds, id, &regs, data);
> +
> +	for (i = 0; i < count / 2; i++)
> +		seq_printf(seq, "%2d: %04x\n", i, data[i]);
> +}
> +
> +static int dsa_debugfs_regs_read(struct dsa_switch *ds, int id,
> +				 struct seq_file *seq)
> +{
> +	int count;
> +
> +	if (!ds->ops->get_regs_len || !ds->ops->get_regs)
> +		return -EOPNOTSUPP;
> +
> +	count = ds->ops->get_regs_len(ds, id);
> +	if (count < 0)
> +		return count;
Because get_regs_len returns length than count per mv88e6xxx/chip.c,
it requires some math before passing to dsa_debugfs_regs_read_count().

It does "count/2" in dsa-debugfs_regs_read_count(), however,
As commented above, "count" is used at "u16 data[...]".
So, it would be nice to match with name. Ie, count or size/length.

Thanks.
Woojung