Re: [net-next PATCH 08/19] net: dsa: qca8k: convert qca8k to regmap helper
From: Vladimir Oltean
Date: Sun Nov 21 2021 - 13:32:03 EST
On Fri, Nov 19, 2021 at 02:28:23AM +0100, Ansuel Smith wrote:
> On Fri, Nov 19, 2021 at 03:14:10AM +0200, Vladimir Oltean wrote:
> > On Wed, Nov 17, 2021 at 10:04:40PM +0100, Ansuel Smith wrote:
> > > Convert any qca8k read/write/rmw/set/clear/pool to regmap helper and add
> > > missing config to regmap_config struct.
> > >
> > > Signed-off-by: Ansuel Smith <ansuelsmth@xxxxxxxxx>
> > > ---
> >
> > The important question is "why" and this commit message seems to omit that.
> > Using regmap will be slower than using the equivalent direct I/O.
>
> Yes sorry, will improve the message.
> The transition to regmap is needed to permit the use of common code by
> different switch that have different read/write/rmw function.
> It seems cleaner to use regmap instead of using some helper or putting
> the read/write/rmw in the priv struct.
> Also in theory the overhead created by using regmap should be marginal
> as the internal mdio use dedicated function and bypass regmap. So the
> overhead should be present only in the configuration operation or fdb
> access.
Ok, no problem with that, just consider that reviewers have limited
attention span, and most of them are not in fact familiar with the
switches you're working with or with what you're trying to do with them,
so providing as much relevant information in the commit message as
possible is crucial to having a smooth way forward with your work.
IMHO I shouldn't have to ask "why" on every one of your patch, the "why"
should already be there.