Hi Andrew,
Andrew Lunn <and...@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 mv88e6xxx_ops structure is agnostic from the implementation. There
is only one way to implement a feature (e.g. setting the switch MAC) in
Marvell switches. So describing the internal location of the said
feature is wrong and brings no value.
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;
}
What's the point of this?
Thanks,
Vivien
