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.
