Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1583752

Re: [PATCH net-next v2 2/6] net: dsa: mv88e6xxx: move ATU code in its own file

Show all headers | View raw

On Fri, Feb 17, 2017 at 10:05:27AM -0500, Vivien Didelot wrote:
> Move the Global (1) ATU related code in its own file, and export the
> necessary primitives.
> 
> Use that opportunity to provide a cleaner API for the ATU, by renaming a
> few underscore prefixed functions, and members of the
> mv88e6xxx_atu_entry structures.

>  static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
>  					const unsigned char *addr, u16 vid,
>  					u8 state)
>  {
> +	struct mv88e6xxx_atu_entry entry = { 0 };
>  	struct mv88e6xxx_vtu_entry vlan;
> -	struct mv88e6xxx_atu_entry entry;
>  	int err;
>  
>  	/* Null VLAN ID corresponds to the port private database */
> @@ -2107,21 +1918,32 @@ static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
>  	if (err)
>  		return err;
>  
> -	err = mv88e6xxx_atu_get(chip, vlan.fid, addr, &entry);
> +	entry.fid = vlan.fid;
> +	ether_addr_copy(entry.mac, addr);
> +	eth_addr_dec(entry.mac);
> +	err = mv88e6xxx_atu_getnext(chip, &entry);
>  	if (err)
>  		return err;
>  
> +	/* Initialize a fresh ATU entry if it isn't found */
> +	if (entry.state == GLOBAL_ATU_DATA_STATE_UNUSED ||
> +	    !ether_addr_equal(entry.mac, addr)) {
> +		memset(&entry, 0, sizeof(entry));
> +		ether_addr_copy(entry.mac, addr);
> +		entry.fid = vlan.fid;
> +	}
> +

This seems to be more than renaming a few functions. There looks to be
real changes here. I think these changes should be split out into a
separate patch with an explanation what is being changed. Keep this
patch for plain renames.

It would also be easier to review if the patch just moved the code, no
changes. Then have patches which clean up the API. It is hard to see
what is move and what is cleanup. Plain move needs little review,
cleanup needs more review. With the current patch, it is hard to see
which is which.

      Andrew

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH net-next v2 0/6] net: dsa: mv88e6xxx: 88E6390 ATU/VTU Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-02-17 16:10 +0100
  [PATCH net-next v2 1/6] net: dsa: mv88e6xxx: add port mask helper Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-02-17 16:10 +0100
    Re: [PATCH net-next v2 1/6] net: dsa: mv88e6xxx: add port mask helper Andrew Lunn <andrew@lunn.ch> - 2017-02-17 22:30 +0100
  [PATCH net-next v2 6/6] net: dsa: mv88e6xxx: add support for 6390 VTU Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-02-17 16:10 +0100
    Re: [PATCH net-next v2 6/6] net: dsa: mv88e6xxx: add support for  6390 VTU Andrew Lunn <andrew@lunn.ch> - 2017-02-17 23:50 +0100
  [PATCH net-next v2 2/6] net: dsa: mv88e6xxx: move ATU code in its own file Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-02-17 16:10 +0100
    Re: [PATCH net-next v2 2/6] net: dsa: mv88e6xxx: move ATU code in  its own file Andrew Lunn <andrew@lunn.ch> - 2017-02-17 22:40 +0100
      Re: [PATCH net-next v2 2/6] net: dsa: mv88e6xxx: move ATU code in its own file Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-02-17 23:00 +0100
        Re: [PATCH net-next v2 2/6] net: dsa: mv88e6xxx: move ATU code in  its own file Andrew Lunn <andrew@lunn.ch> - 2017-02-17 23:40 +0100
          Re: [PATCH net-next v2 2/6] net: dsa: mv88e6xxx: move ATU code in  its own file David Miller <davem@davemloft.net> - 2017-02-18 00:50 +0100
  [PATCH net-next v2 4/6] net: dsa: mv88e6xxx: move VTU code to its own file Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-02-17 16:10 +0100
    Re: [PATCH net-next v2 4/6] net: dsa: mv88e6xxx: move VTU code to  its own file Andrew Lunn <andrew@lunn.ch> - 2017-02-17 22:50 +0100

csiph-web