Re: [PATCH v1 net-next 1/5] net: dsa: mv88e6xxx: Reserved Management frames to CPU

[Andrew Lunn <andrew@lunn.ch>]

> > +int mv88e6095_g2_mgmt_rsvd2cpu(struct mv88e6xxx_chip *chip)
> > +{
> > +	int err;
> > +
> > +	/* Consider the frames with reserved multicast destination
> > +	 * addresses matching 01:80:c2:00:00:2x as MGMT.
> > +	 */
> > +	if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_MGMT_EN_2X)) {
> 
> Please don't just move the old code. You shouldn't need flags anymore.

Hi Vivien

My aim is to get the 6390 supported. Refactoring is secondary. If it
helps implementing the 6390 code, i will refactor stuff. If it does
not help, i will leave it alone. It does not help here, so i left it
alone. Please feel free to submit a follow up patch refactoring this
further.

> The mv88e6xxx_ops actually implements the *features*. They can be
> prefixed for clarity (e.g. .ppu_*, port_*, .atu_*, etc.). They don't
> describe the register layout.
> 
> But we can discuss two ways of seeing this structure implementation:

or

3) We have a prefix for us humans to help us find the code. Now we
have ops, i cannot simply do M-. and emacs will take me to the
implementation. I have to search for it a bit. Having the hint g1_
tells me to go look in global1.c. Having the hint g2_ tells me to go
look in global2.c. Having the port_ tells me to go look in port.c.
Having no prefix tells me the code is scattered around and grep is my
friend.

The prefix is just a hint where the function is in the source
code. Nothing more.

      Andrew