linux-kernel - Re: [RFC patch net-next 3/3] net: dsa: ksz: moved ksz9477 port mirror to ksz

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 27 Apr 2022 19:57:22 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Arun Ramadoss <arun.ramadoss@...rochip.com>
Cc:     linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        Paolo Abeni <pabeni@...hat.com>,
        Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Andrew Lunn <andrew@...n.ch>, UNGLinuxDriver@...rochip.com,
        Woojung Huh <woojung.huh@...rochip.com>
Subject: Re: [RFC patch net-next 3/3] net: dsa: ksz: moved ksz9477 port
 mirror to ksz_common.c

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@...rochip.com>
> ---

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@...il.com>

> 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
>