Re: [PATCH net-next v2] net: ftmac100: add multicast filtering possibility

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Tue, Aug 08, 2023 at 03:43:07PM +0300, Sergei Antonov wrote:
> If netdev_mc_count() is not zero and not IFF_ALLMULTI, filter
> incoming multicast packets. The chip has a Multicast Address Hash Table
> for allowed multicast addresses, so we fill it.
> 
> Implement .ndo_set_rx_mode and recalculate multicast hash table. Also
> observe change of IFF_PROMISC and IFF_ALLMULTI netdev flags.
> 
> Signed-off-by: Sergei Antonov <saproj@gmail.com>
> ---
> v1 -> v2:
> * fix hashing algorithm (the old one was based on bad testing)
> * observe change of IFF_PROMISC, IFF_ALLMULTI in set_rx_mode
> * u64 and BIT_ULL code simplification suggested by Vladimir Oltean
> ---

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

> +static void ftmac100_set_rx_bits(struct ftmac100 *priv, unsigned int *maccr)
> +{
> +	struct net_device *netdev = priv->netdev;
> +
> +	/* Clear all */
> +	*maccr &= ~(FTMAC100_MACCR_RCV_ALL | FTMAC100_MACCR_RX_MULTIPKT |
> +		   FTMAC100_MACCR_HT_MULTI_EN);
> +
> +	/* Set the requested bits */
> +	if (netdev->flags & IFF_PROMISC)
> +		*maccr |= FTMAC100_MACCR_RCV_ALL;
> +	if (netdev->flags & IFF_ALLMULTI)
> +		*maccr |= FTMAC100_MACCR_RX_MULTIPKT;
> +	else if (netdev_mc_count(netdev)) {
> +		*maccr |= FTMAC100_MACCR_HT_MULTI_EN;
> +		ftmac100_setup_mc_ht(priv);
> +	}

Minor nitpick, you don't need to resend for this: generally the coding
style recommends to open braces for all "if/else" blocks if one of them
requires braces. But it doesn't look horrible here.

> +}