Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Date:   Tue, 24 Oct 2023 11:16:27 +0200 (CEST)
From:   Romain Gantois <romain.gantois@bootlin.com>
To:     Andrew Lunn <andrew@lunn.ch>
cc:     Romain Gantois <romain.gantois@bootlin.com>, davem@davemloft.net,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Jakub Kicinski <kuba@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        thomas.petazzoni@bootlin.com,
        Florian Fainelli <f.fainelli@gmail.com>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        linux-arm-kernel@lists.infradead.org,
        Vladimir Oltean <vladimir.oltean@nxp.com>,
        Luka Perkov <luka.perkov@sartura.hr>,
        Robert Marko <robert.marko@sartura.hr>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@somainline.org>,
        Maxime Chevallier <maxime.chevallier@bootlin.com>
Subject: Re: [PATCH net-next 3/5] net: ipqess: introduce the Qualcomm IPQESS
 driver
In-Reply-To: <b8ac3558-b6f0-4658-b406-8ceba062a52c@lunn.ch>
Message-ID: <f4e6dcee-23cf-bf29-deef-cf876e63bb8a@bootlin.com>
References: <20231023155013.512999-1-romain.gantois@bootlin.com> <20231023155013.512999-4-romain.gantois@bootlin.com> <b8ac3558-b6f0-4658-b406-8ceba062a52c@lunn.ch>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

Hello Andrew,

On Mon, 23 Oct 2023, Andrew Lunn wrote:

[...]
> > +	struct qca8k_priv *priv = port->sw->priv;
> > +	struct ipqess_port_dump_ctx dump = {
> > +		.dev = dev,
> > +		.skb = skb,
> > +		.cb = cb,
> > +		.idx = *idx,
> > +	};
> 
> And with a little bit of refactoring, you should be able to use the
> core of qca8k_port_fdb_dump(). All that seems to differ is how you get
> to the struct qca8k_priv *priv.
> 
> That then makes me wounder if there is more code here which could be
> removed with a little refactoring of the DSA driver?

Yes, I think this should be possible for a few more functions, I'll look into it 
for the v2.

> > +static int ipqess_port_get_eeprom_len(struct net_device *dev)
> > +{
> > +	return 0;
> > +}
> 
> Is this actually useful? What does it default to if not provided? 42?

It's not, I'll remove it.

> 
> > +	for (c = 0; c < priv->info->mib_count; c++) {
> > +		mib = &ar8327_mib[c];
> > +		reg = QCA8K_PORT_MIB_COUNTER(port->index) + mib->offset;
> > +
> > +		ret = qca8k_read(priv, reg, &val);
> > +		if (ret < 0)
> > +			continue;
> 
> Given the switch is built in, is this fast? The 8k driver avoids doing
> register reads for this.

Sorry, I don't quite understand what you mean. Are you referring to the existing 
QCA8k DSA driver? From what I've seen, it calls qca8k_get_ethtool_stats defined 
in qca8k-common.c and this uses the same register read.

Thanks,

Romain