linux-kernel - Re: [net-next RFC PATCH 1/4] net: dsa: qca8k: drop qca8k

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 18 Jul 2022 19:55:26 +0200
From:   Christian Marangi <ansuelsmth@...il.com>
To:     Vladimir Oltean <olteanv@...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>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Russell King <linux@...linux.org.uk>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jens Axboe <axboe@...nel.dk>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [net-next RFC PATCH 1/4] net: dsa: qca8k: drop
 qca8k_read/write/rmw for regmap variant

On Mon, Jul 18, 2022 at 09:04:52PM +0300, Vladimir Oltean wrote:
> On Sat, Jul 16, 2022 at 07:49:55PM +0200, Christian Marangi wrote:
> > In preparation for code split, drop the remaining qca8k_read/write/rmw
> > and use regmap helper directly.
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@...il.com>
> > ---
> >  drivers/net/dsa/qca/qca8k.c | 206 +++++++++++++++++-------------------
> >  1 file changed, 95 insertions(+), 111 deletions(-)
> > 
> > diff --git a/drivers/net/dsa/qca/qca8k.c b/drivers/net/dsa/qca/qca8k.c
> > index 1cbb05b0323f..2d34e15c2e6f 100644
> > --- a/drivers/net/dsa/qca/qca8k.c
> > +++ b/drivers/net/dsa/qca/qca8k.c
> > @@ -184,24 +184,6 @@ qca8k_set_page(struct qca8k_priv *priv, u16 page)
> >  	return 0;
> >  }
> >  
> > -static int
> > -qca8k_read(struct qca8k_priv *priv, u32 reg, u32 *val)
> > -{
> > -	return regmap_read(priv->regmap, reg, val);
> > -}
> > -
> > -static int
> > -qca8k_write(struct qca8k_priv *priv, u32 reg, u32 val)
> > -{
> > -	return regmap_write(priv->regmap, reg, val);
> > -}
> > -
> > -static int
> > -qca8k_rmw(struct qca8k_priv *priv, u32 reg, u32 mask, u32 write_val)
> > -{
> > -	return regmap_update_bits(priv->regmap, reg, mask, write_val);
> > -}
> > -
> 
> Could you please explain slowly to me why this change is needed? I don't get it.
> Can't qca8k_read(), qca8k_write() and qca8k_rmw() be part of qca8k-common.c?

Sure.
When the regmap conversion was done at times, to limit patch delta it
was suggested to keep these function. This was to not get crazy with
eventual backports and fixes.

The logic here is:
As we are moving these function AND the function will use regmap api
anyway, we can finally drop them and user the regmap api directly
instead of these additional function.

When the regmap conversion was done, I pointed out that in the future
the driver had to be split in specific and common code and it was said
that only at that times there was a good reason to make all these
changes and drop these special functions.

Now these function are used by both setup function for qca8k and by
common function that will be moved to a different file.

If we really want I can skip the dropping of these function and move
them to qca8k common code.

An alternative is to keep them for qca8k specific code and migrate the
common function to regmap api.

So it's really a choice of drop these additional function or keep using
them for the sake of not modifying too much source.

Hope it's clear now the reason of this change.

-- 
	Ansuel