From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Horatiu Vultur <horatiu.vultur@microchip.com>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "f.fainelli@gmail.com" <f.fainelli@gmail.com>,
        "vivien.didelot@gmail.com" <vivien.didelot@gmail.com>,
        "andrew@lunn.ch" <andrew@lunn.ch>
Subject: Re: [PATCH net-next v2 1/3] net: lan966x: Add function
 lan966x_mac_ip_learn()
Thread-Topic: [PATCH net-next v2 1/3] net: lan966x: Add function
 lan966x_mac_ip_learn()
Thread-Index: AQHYAVQvYoQu8Rr2iUS6g2YiiJid86xStpIA
Date:   Tue, 4 Jan 2022 11:17:10 +0000
Message-ID: <20220104111710.twaqos2fbmjfv5yu@skbuf>
References: <20220104101849.229195-1-horatiu.vultur@microchip.com>
 <20220104101849.229195-2-horatiu.vultur@microchip.com>
In-Reply-To: <20220104101849.229195-2-horatiu.vultur@microchip.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?r6liSzJPnjt4nr0NqjMIwFxYIEGoQ8w7t+S0U4Wux0prXfXNqwJnM1H2XyDP?=
 =?us-ascii?Q?AMuaKyBY/ElgT2/WGELc5bUECjCAvTy7gB5zVGZS0PyPSBzSl8Ygts8pIX9l?=
 =?us-ascii?Q?RMcfUVYtxVIJNl/Mb7AhuwcBXs0QkyCavUpem+bAIK8VVz1eCwQFnJYrtb+U?=
 =?us-ascii?Q?vvFqY+W5VFgVs/YX5+bPpq2TAklCceFSbNZGjpO1hapktPDib4Ml0WZwdowG?=
 =?us-ascii?Q?H5wK4QNiFS5UJ6+jN5rQwyu8nW5LxyzOvmOeZYE/TceeVB6IYGTqw7F0/DWs?=
 =?us-ascii?Q?gDmIMxdQrjJwD0qAFk2FUeIzDuGmo+/6Rulo7AUWXOWAkJMi0m3ydrfhHL5D?=
 =?us-ascii?Q?q5HvtCF9VI8zEeAR1lPBy3dFZI8OR4wiE/ewBz7G9q/GGK/ozWh63mIE13Sb?=
 =?us-ascii?Q?fUfXPHLEJhqk/SrzusFZfwHZvnAiVzwL/GCKDNi1S0um2anZZ+F5U5ksLiPL?=
 =?us-ascii?Q?g28PQ1wGWlHumjkW9YuBDsmb7b9fvlqQY1KQECPoAazcbR6DI+dF/OlL+oNx?=
 =?us-ascii?Q?axkkdZnvDksxNCj9K677dv2AuUXMLhWudubJy0YLYethZ9hp9H2t0h++cy9s?=
 =?us-ascii?Q?XAy5MIZC26js7xy8Kj2jFOVIqstX+CwZqgPFKqOQWFS1qzXFbbJ+GVUspYw/?=
 =?us-ascii?Q?YuiLXmpoacMD6rUJXy+lV6BtJrTwwfiGDgV3/VL4Heq6Xj/S3jEzDVZsgONQ?=
 =?us-ascii?Q?pduR8twkMsjEu0FvqPzu9SggJ/gUhM+og1M6Av8rXF9moCkI2/QUuWV+d1G8?=
 =?us-ascii?Q?z+NN6inkAT8mXthmEeB/w5lGnkLINhdinTtFDf4RL4e1kCLUUX9/Csln+bfV?=
 =?us-ascii?Q?XwZpBKE8cuyBuJL7IxiuOsuOAv+m3MFF1woQIO0yuvlshQ+CZREzhdd+JVra?=
 =?us-ascii?Q?UEQV7wloComIbBocDAk0qy8yYNJAPli9zPeWHL2tPI438cNu5hCoG1kaaSJk?=
 =?us-ascii?Q?kTGyp2QqSPkzNmgRlM8ZifxXmCEaC4ssjJE3XKUq/2v/OYOKmMODJ4NDzlA3?=
 =?us-ascii?Q?24xsLP7++XPYcNES6uHD75fDvFZU3XAbn+Ylu246xLd4pDdsQ2xSPXuwjh/H?=
 =?us-ascii?Q?THvFIevQFk9mdX593cGyaCaU+Ml2kDemiLfR0hQBN3QSDiGBV+6uvzm1yfoh?=
 =?us-ascii?Q?q2hoR1k1epIK5wTja9NH7qN9nZaur8PGAHqaR2awsiD/yQ8y7n7Q/I0j3VUm?=
 =?us-ascii?Q?9Yw0soZEgoNJr4mpfVNWY6KUB/suidcpkNqJxk5wO/4rqQ+cWeyGohkaIPSi?=
 =?us-ascii?Q?MK0Flxjdr25hpBq+PDrDygSeFgCbaOsIlEMuKY9MIOMdPaqv5hT8Ir/9JZI6?=
 =?us-ascii?Q?wPJqXNco/Gdr+sY0zPVllRfcUTT9taLL/AqIgLX7jjldenRen8PrCmHjFKPc?=
 =?us-ascii?Q?DiPyLtCE7tNAsmxFvYMqdowzN1LuHd1ZVc9HQZZZBaVuFae0+AVGGN06OrW5?=
 =?us-ascii?Q?x/YB02G198DS6wDeRIxxuDXITa9XvPQV1LW5QtrKSvpeKbJYkLlOnt4rFRqz?=
 =?us-ascii?Q?VK9zeF2OB3c2TiTYlhWlYCjnfBsQx61f5KN9uxOqsa8ieeu7Oryv/PUMBs9d?=
 =?us-ascii?Q?cEAbG/UA5pfjGnFLN8BZEKIw0NOBBnTTZNXxlrzznC9vifYK7/DbMKK0ND5j?=
 =?us-ascii?Q?R0V8FJ5HWOKk+DU4L06e79U=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <B3E2DBEE51D42248934080CE8EB37EBE@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5136.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a4a37be1-3ff1-4ab3-e492-08d9cf73c066
X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Jan 2022 11:17:10.4543
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: SeAFjyui7Nua0LBiuE6THBJvoN0soGq1N+z5G2zq6fIJIYTsMH9S6TO6xCeXv/v8dtCw+EVEDpd5qhhRxNzTFA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB4685
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Jan 04, 2022 at 11:18:47AM +0100, Horatiu Vultur wrote:
> Extend mac functionality with the function lan966x_mac_ip_learn. This
> function adds an entry in the MAC table for IP multicast addresses.
> These entries can copy a frame to the CPU but also can forward on the
> front ports.
> This functionality is needed for mdb support. In case the CPU and some
> of the front ports subscribe to an IP multicast address.
>=20
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  .../ethernet/microchip/lan966x/lan966x_mac.c  | 33 ++++++++++++++++---
>  .../ethernet/microchip/lan966x/lan966x_main.h |  5 +++
>  .../ethernet/microchip/lan966x/lan966x_regs.h |  6 ++++
>  3 files changed, 39 insertions(+), 5 deletions(-)
>=20
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c b/drive=
rs/net/ethernet/microchip/lan966x/lan966x_mac.c
> index efadb8d326cc..82eb6606e17f 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c
> @@ -68,17 +68,19 @@ static void lan966x_mac_select(struct lan966x *lan966=
x,
>  	lan_wr(mach, lan966x, ANA_MACHDATA);
>  }
> =20
> -int lan966x_mac_learn(struct lan966x *lan966x, int port,
> -		      const unsigned char mac[ETH_ALEN],
> -		      unsigned int vid,
> -		      enum macaccess_entry_type type)
> +static int __lan966x_mac_learn(struct lan966x *lan966x, int pgid,
> +			       bool cpu_copy,
> +			       const unsigned char mac[ETH_ALEN],
> +			       unsigned int vid,
> +			       enum macaccess_entry_type type)
>  {
>  	lan966x_mac_select(lan966x, mac, vid);
> =20
>  	/* Issue a write command */
>  	lan_wr(ANA_MACACCESS_VALID_SET(1) |
>  	       ANA_MACACCESS_CHANGE2SW_SET(0) |
> -	       ANA_MACACCESS_DEST_IDX_SET(port) |
> +	       ANA_MACACCESS_MAC_CPU_COPY_SET(cpu_copy) |
> +	       ANA_MACACCESS_DEST_IDX_SET(pgid) |
>  	       ANA_MACACCESS_ENTRYTYPE_SET(type) |
>  	       ANA_MACACCESS_MAC_TABLE_CMD_SET(MACACCESS_CMD_LEARN),
>  	       lan966x, ANA_MACACCESS);
> @@ -86,6 +88,27 @@ int lan966x_mac_learn(struct lan966x *lan966x, int por=
t,
>  	return lan966x_mac_wait_for_completion(lan966x);
>  }
> =20
> +int lan966x_mac_ip_learn(struct lan966x *lan966x,
> +			 bool cpu_copy,
> +			 const unsigned char mac[ETH_ALEN],
> +			 unsigned int vid,
> +			 enum macaccess_entry_type type)

I think it's worth mentioning in a comment above this function that the
mask of front ports should be encoded into the address by now, via a
call to lan966x_mdb_encode_mac().

> +{
> +	WARN_ON(type !=3D ENTRYTYPE_MACV4 && type !=3D ENTRYTYPE_MACV6);
> +
> +	return __lan966x_mac_learn(lan966x, 0, cpu_copy, mac, vid, type);
> +}
> +
> +int lan966x_mac_learn(struct lan966x *lan966x, int port,
> +		      const unsigned char mac[ETH_ALEN],
> +		      unsigned int vid,
> +		      enum macaccess_entry_type type)
> +{
> +	WARN_ON(type !=3D ENTRYTYPE_NORMAL && type !=3D ENTRYTYPE_LOCKED);
> +
> +	return __lan966x_mac_learn(lan966x, port, false, mac, vid, type);
> +}
> +
>  int lan966x_mac_forget(struct lan966x *lan966x,
>  		       const unsigned char mac[ETH_ALEN],
>  		       unsigned int vid,
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/driv=
ers/net/ethernet/microchip/lan966x/lan966x_main.h
> index c399b1256edc..f70e54526f53 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> @@ -157,6 +157,11 @@ int lan966x_port_pcs_set(struct lan966x_port *port,
>  			 struct lan966x_port_config *config);
>  void lan966x_port_init(struct lan966x_port *port);
> =20
> +int lan966x_mac_ip_learn(struct lan966x *lan966x,
> +			 bool cpu_copy,
> +			 const unsigned char mac[ETH_ALEN],
> +			 unsigned int vid,
> +			 enum macaccess_entry_type type);
>  int lan966x_mac_learn(struct lan966x *lan966x, int port,
>  		      const unsigned char mac[ETH_ALEN],
>  		      unsigned int vid,
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h b/driv=
ers/net/ethernet/microchip/lan966x/lan966x_regs.h
> index a13c469e139a..797560172aca 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h
> @@ -169,6 +169,12 @@ enum lan966x_target {
>  #define ANA_MACACCESS_CHANGE2SW_GET(x)\
>  	FIELD_GET(ANA_MACACCESS_CHANGE2SW, x)
> =20
> +#define ANA_MACACCESS_MAC_CPU_COPY               BIT(16)
> +#define ANA_MACACCESS_MAC_CPU_COPY_SET(x)\
> +	FIELD_PREP(ANA_MACACCESS_MAC_CPU_COPY, x)
> +#define ANA_MACACCESS_MAC_CPU_COPY_GET(x)\
> +	FIELD_GET(ANA_MACACCESS_MAC_CPU_COPY, x)

Could you please add a space between (x) and \.

> +
>  #define ANA_MACACCESS_VALID                      BIT(12)
>  #define ANA_MACACCESS_VALID_SET(x)\
>  	FIELD_PREP(ANA_MACACCESS_VALID, x)
> --=20
> 2.33.0
>=