Re: [RFC patch net-next 3/3] net: dsa: ksz: moved ksz9477 port mirror to ksz_common.c

[Vladimir Oltean]

On Wed, Apr 27, 2022 at 09:53:43PM +0530, Arun Ramadoss wrote:
> Moved the port_mirror_add and port_mirror_del function from ksz9477 to

Present tense (move)

> ksz_common, to make it generic function which can be used by KSZ9477
> based switch.

Presumably you mean "which can be used by other switches" (it can
already be used by ksz9477, so that can't be the argument for moving it)

> 
> Signed-off-by: Arun Ramadoss <arun.ramadoss@xxxxxxxxxxxxx>
> ---

Looks good, except for the spelling mistakes in the code that is being
moved (introduced in patch 1), which I expect you will update in the new
code as well.

Reviewed-by: Vladimir Oltean <olteanv@xxxxxxxxx>

> diff --git a/drivers/net/dsa/microchip/ksz_reg.h b/drivers/net/dsa/microchip/ksz_reg.h
> new file mode 100644
> index 000000000000..ccd4a6568e34
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/ksz_reg.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Microchip KSZ Switch register definitions
> + *
> + * Copyright (C) 2017-2022 Microchip Technology Inc.
> + */
> +
> +#ifndef __KSZ_REGS_H
> +#define __KSZ_REGS_H
> +
> +#define REG_SW_MRI_CTRL_0		0x0370
> +
> +#define SW_IGMP_SNOOP			BIT(6)
> +#define SW_IPV6_MLD_OPTION		BIT(3)
> +#define SW_IPV6_MLD_SNOOP		BIT(2)
> +#define SW_MIRROR_RX_TX			BIT(0)
> +
> +/* 8 - Classification and Policing */
> +#define REG_PORT_MRI_MIRROR_CTRL	0x0800
> +
> +#define PORT_MIRROR_RX			BIT(6)
> +#define PORT_MIRROR_TX			BIT(5)
> +#define PORT_MIRROR_SNIFFER		BIT(1)
> +
> +#define P_MIRROR_CTRL			REG_PORT_MRI_MIRROR_CTRL
> +
> +#define S_MIRROR_CTRL			REG_SW_MRI_CTRL_0

Small comment: if P_MIRROR_CTRL and S_MIRROR_CTRL are expected to be at
the same register offset for all switch families, why is there a macro
behind a macro for their addresses?

> +
> +#endif
> -- 
> 2.33.0
>