Re: [PATCHv2 net-next 2/4] net: dsa: mv88e6xxx: Monitor and Management tables

[Andrew Lunn <andrew@lunn.ch>]

On Fri, Dec 02, 2016 at 02:32:39PM -0500, Vivien Didelot wrote:
> Hi Andrew,
> 
> Andrew Lunn <andrew@lunn.ch> writes:
> 
> > @@ -3184,6 +3186,8 @@ static const struct mv88e6xxx_ops mv88e6085_ops = {
> >  	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
> >  	.stats_get_strings = mv88e6095_stats_get_strings,
> >  	.stats_get_stats = mv88e6095_stats_get_stats,
> > +	.g1_set_cpu_port = mv88e6095_g1_set_cpu_port,
> > +	.g1_set_egress_port = mv88e6095_g1_set_egress_port,
> >  };
> 
> I like the implementation in this version better. But please explain me
> why you are prefixing these operations with g1_?

The prefix gives some basic grouping. port_ indicates it operates on a
port, and is likely to be found in port.c. stats_ indicates it
operates on statistics, ppu that is operates on the phy polling unit.

We are going to have some things which don't fall into a simple
category, like these two. But it would however be nice to group them,
so i picked which register bank they are in. These operations are
always in g1. It is a useful hint as to where to find the different
variants.

> But let's imagine we can set the CPU port in some Global 2 registers.
> You are going to wrap this in chip.c with something like:
> 
>     int mv88e6xxx_set_cpu_port(struct mv88e6xxx_chip *chip, int port)
>     {
>         if (chip->info->ops->g2_set_cpu_port)
>             return chip->info->ops->g2_set_cpu_port(chip, port);
>         else if (chip->info->ops->g1_set_cpu_port)
>             return chip->info->ops->g1_set_cpu_port(chip, port);
>         else
>             return -EOPNOTSUPP;
>     }

I answered in one of my other emails. Frames with reserved MAC
addresses can be forwarded to the CPU. For most devices, this is a g2
operation. However, for 6390, it is a g1. In that case, my code does
not use a prefix. Not having a prefix, when all the others do, also
gives you information. It means the ops are spread around and you need
to make a bigger effort to go find them.

   Andrew