Re: [PATCH net-next v2 3/4] net: dsa: hellcreek: Move common code to helper

[Vladimir Oltean <olteanv@gmail.com>]

On Sat, Mar 13, 2021 at 10:39:38AM +0100, Kurt Kanzenbach wrote:
> There are two functions which need to populate fdb entries. Move that to a
> helper function.
> 
> Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/dsa/hirschmann/hellcreek.c | 85 +++++++++++++-------------
>  1 file changed, 43 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
> index edac39462a07..38ff0f12e8a4 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> @@ -670,6 +670,40 @@ static int __hellcreek_fdb_del(struct hellcreek *hellcreek,
>  	return hellcreek_wait_fdb_ready(hellcreek);
>  }
>  
> +static void hellcreek_populate_fdb_entry(struct hellcreek *hellcreek,
> +					 struct hellcreek_fdb_entry *entry,
> +					 size_t idx)
> +{
> +	unsigned char addr[ETH_ALEN];

You could have assigned:

	unsigned char *addr = entry->mac;

and avoided the memcpy, but it doesn't really matter much.

> +	u16 meta, mac;
> +
> +	/* Read values */
> +	meta	= hellcreek_read(hellcreek, HR_FDBMDRD);
> +	mac	= hellcreek_read(hellcreek, HR_FDBRDL);
> +	addr[5] = mac & 0xff;
> +	addr[4] = (mac & 0xff00) >> 8;
> +	mac	= hellcreek_read(hellcreek, HR_FDBRDM);
> +	addr[3] = mac & 0xff;
> +	addr[2] = (mac & 0xff00) >> 8;
> +	mac	= hellcreek_read(hellcreek, HR_FDBRDH);
> +	addr[1] = mac & 0xff;
> +	addr[0] = (mac & 0xff00) >> 8;
> +
> +	/* Populate @entry */
> +	memcpy(entry->mac, addr, sizeof(addr));
> +	entry->idx	    = idx;
> +	entry->portmask	    = (meta & HR_FDBMDRD_PORTMASK_MASK) >>
> +		HR_FDBMDRD_PORTMASK_SHIFT;
> +	entry->age	    = (meta & HR_FDBMDRD_AGE_MASK) >>
> +		HR_FDBMDRD_AGE_SHIFT;
> +	entry->is_obt	    = !!(meta & HR_FDBMDRD_OBT);
> +	entry->pass_blocked = !!(meta & HR_FDBMDRD_PASS_BLOCKED);
> +	entry->is_static    = !!(meta & HR_FDBMDRD_STATIC);
> +	entry->reprio_tc    = (meta & HR_FDBMDRD_REPRIO_TC_MASK) >>
> +		HR_FDBMDRD_REPRIO_TC_SHIFT;
> +	entry->reprio_en    = !!(meta & HR_FDBMDRD_REPRIO_EN);
> +}
> +

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>