Re: [PATCH net-next 1/3] net: dsa: yt921x: Refactor long register helpers

[Andrew Lunn]

On Wed, Feb 25, 2026 at 10:12:36PM +0800, David Yang wrote:
> On Wed, Feb 25, 2026 at 9:52 PM Andrew Lunn <andrew@xxxxxxx> wrote:
> >
> > > -/* Some registers, like VLANn_CTRL, should always be written in 64-bit, even if
> > > - * you are to write only the lower / upper 32 bits.
> > > +/* Some registers, like VLANn_CTRL, should always be written in chunks, even if
> > > + * you only want to write parts of 32 bits.
> > >   *
> > > - * There is no such restriction for reading, but we still provide 64-bit read
> > > - * wrappers so that we always handle u64 values.
> > > + * There is no such restriction for reading, but we still provide multi-chunk
> > > + * read wrappers so that we can handle them consistently.
> > >   */
> > >
> > > -static int yt921x_reg64_read(struct yt921x_priv *priv, u32 reg, u64 *valp)
> > > +static int
> > > +yt921x_longreg_read(struct yt921x_priv *priv, u32 reg, u32 *vals,
> > > +                 unsigned int span)
> >
> > Just looking at this, it is not obvious what units span is. Many bulk
> > read operations takes bytes, even if the underlying code works in
> > words. I would probably rename span to something like num_u32.
> >
> > I would also keep yt921x_reg64_read, yt921x_reg64_write,
> > yt921x_reg64_update_bits() and just makes them wrappers which call
> > yt921x_longreg_read().
> >
> 
> I don't think keeping reg64_* would make it better. There will be more
> 96-bit regs as subsequent series progressing. It would be more messy
> to have three types of regs: u32, u64, and u32[].

The lower level helper takes a u32[], but your higher level is always
yt921x_reg32_read(), yt921x_reg64_read(), or yt921x_reg96_read(). That
makes it obvious what size you are reading, and keeps your patch
small.

      Andrew