Re: [PATCH v4 net-next 05/14] net: enetc: add debugfs interface to dump MAC filter

[Vladimir Oltean]

On Tue, Mar 18, 2025 at 05:28:56AM +0200, Wei Fang wrote:
> > On Tue, Mar 11, 2025 at 01:38:21PM +0800, Wei Fang wrote:
> > > +static void enetc_show_si_mac_hash_filter(struct seq_file *s, int i)
> > > +{
> > > +	struct enetc_si *si = s->private;
> > > +	struct enetc_hw *hw = &si->hw;
> > > +	u32 hash_h, hash_l;
> > > +
> > > +	hash_l = enetc_port_rd(hw, ENETC4_PSIUMHFR0(i));
> > > +	hash_h = enetc_port_rd(hw, ENETC4_PSIUMHFR1(i));
> > > +	seq_printf(s, "SI %d unicast MAC hash filter: 0x%08x%08x\n",
> > > +		   i, hash_h, hash_l);
> > 
> > Maybe the ":" separator between the high and low 32 bits is clearer than "x".
> 
> I want it to be presented as a full 64-bit entry. If it is in the format
> of "%08x:%08x", it may be difficult to understand which is the high
> 32-bit and which is the low 32-bit.

Ah :-/ sorry, I made a mistake. I believed you're printing a literal "x"
as a separator between the high and the low word, which made no sense
(hence my comment). But the "x" was in fact the printf format specifier
for "hexadecimal" (sorry!). What I was saying is that ":" is a clearer
separator than "x", which of course makes no sense now.

Anyway, I made a second mistake, which is that I didn't mean to suggest
":" as a separator, but ",". I had misremembered...

The reason I had made that suggestion is prior experience with phylink,
which prints link mode masks with "%*pb" (the implementation is in the
"pointer()" function in lib/vsprintf.c). This is an error message which
many users may have seen before:

[   61.800079] mv88e6085 d0032004.mdio-mii:12 sfp: validation with support 00,00000000,00000000,00000000 failed: -EINVAL

I think the semantics are also widely accepted for this format, and it's
a little bit easier on the eye. I have searched many times in the past
through enum ethtool_link_mode_bit_indices based on the bits set in this
bitmap, and others may have done that too. It's known that bit zero is
the right-most bit.

But overall, printing the 64-bit number with no separator at all is not
unacceptable, either.