Re: [PATCH net-next 3/5] net: ipqess: introduce the Qualcomm IPQESS driver

[Romain Gantois]

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