Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 26 Jan 2022 04:53:03 +0100
From:   Ansuel Smith <ansuelsmth@gmail.com>
To:     Florian Fainelli <f.fainelli@gmail.com>
Cc:     Andrew Lunn <andrew@lunn.ch>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org
Subject: Re: [RFC PATCH v7 16/16] net: dsa: qca8k: introduce
 qca8k_bulk_read/write function
Message-ID: <YfDFn0IVjzHfSS87@Ansuel-xps.localdomain>
References: <20220123013337.20945-1-ansuelsmth@gmail.com>
 <20220123013337.20945-17-ansuelsmth@gmail.com>
 <e40b4f78-5ac8-80fe-7c09-127ce01dde92@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <e40b4f78-5ac8-80fe-7c09-127ce01dde92@gmail.com>
Precedence: bulk

On Tue, Jan 25, 2022 at 07:45:29PM -0800, Florian Fainelli wrote:
> 
> 
> On 1/22/2022 5:33 PM, Ansuel Smith wrote:
> > Introduce qca8k_bulk_read/write() function to use mgmt Ethernet way to
> > read/write packet in bulk. Make use of this new function in the fdb
> > function.
> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> 
> [snip]
> 
> >   static int
> >   qca8k_regmap_read(void *ctx, uint32_t reg, uint32_t *val)
> >   {
> > @@ -535,17 +572,13 @@ qca8k_busy_wait(struct qca8k_priv *priv, u32 reg, u32 mask)
> >   static int
> >   qca8k_fdb_read(struct qca8k_priv *priv, struct qca8k_fdb *fdb)
> >   {
> > -	u32 reg[4], val;
> > -	int i, ret;
> > +	u32 reg[4];
> > +	int ret;
> >   	/* load the ARL table into an array */
> > -	for (i = 0; i < 4; i++) {
> > -		ret = qca8k_read(priv, QCA8K_REG_ATU_DATA0 + (i * 4), &val);
> > -		if (ret < 0)
> > -			return ret;
> > -
> > -		reg[i] = val;
> > -	}
> > +	ret = qca8k_bulk_read(priv, QCA8K_REG_ATU_DATA0, reg, 12);
> 
> sizeof(reg)? How did you come up with 12 if we were executing the loop 4
> times before or were we reading too much?
>

Exactly that. The acl table is actualy of 83 bits. Currently we read 128
bits but we only handle 96 as we never read/write in reg[3].

> > +	if (ret)
> > +		return ret;
> >   	/* vid - 83:72 */
> >   	fdb->vid = FIELD_GET(QCA8K_ATU_VID_MASK, reg[2]);
> > @@ -569,7 +602,6 @@ qca8k_fdb_write(struct qca8k_priv *priv, u16 vid, u8 port_mask, const u8 *mac,
> >   		u8 aging)
> >   {
> >   	u32 reg[3] = { 0 };
> > -	int i;
> >   	/* vid - 83:72 */
> >   	reg[2] = FIELD_PREP(QCA8K_ATU_VID_MASK, vid);
> > @@ -586,8 +618,7 @@ qca8k_fdb_write(struct qca8k_priv *priv, u16 vid, u8 port_mask, const u8 *mac,
> >   	reg[0] |= FIELD_PREP(QCA8K_ATU_ADDR5_MASK, mac[5]);
> >   	/* load the array into the ARL table */
> > -	for (i = 0; i < 3; i++)
> > -		qca8k_write(priv, QCA8K_REG_ATU_DATA0 + (i * 4), reg[i]);
> > +	qca8k_bulk_write(priv, QCA8K_REG_ATU_DATA0, reg, 12);
> 
> sizeof(reg) would be more adequate here.
> -- 
> Florian

-- 
	Ansuel