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
