netdev - Re: [net-next PATCH 08/19] net: dsa: qca8k: convert qca8k to regmap helper

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211121183157.nbpkqznmockbh6ck@skbuf>
Date:   Sun, 21 Nov 2021 20:31:57 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Ansuel Smith <ansuelsmth@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [net-next PATCH 08/19] net: dsa: qca8k: convert qca8k to regmap
 helper

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@...il.com>
> > > ---
> > 
> > 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.