Re: [PATCH edk2-platforms v4 2/9] Silicon/Broadcom/BcmGenetDxe: add support for broadcast filtering

["Jeremy Linton" <jeremy.linton@arm.com>]

Hi,

On 5/11/20 9:55 AM, Ard Biesheuvel wrote:
> Add a helper to configure the first MDF filter for filtering the
> broadcast Ethernet address.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>   Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h |  7 +++++
>   Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c | 33 ++++++++++++++++++++
>   2 files changed, 40 insertions(+)
> 
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h
> index b21a284b6221..b491ea4665b0 100644
> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h
> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h
> @@ -107,6 +107,7 @@
>   #define GENET_UMAC_MDF_CTRL                     0xe50
>   #define GENET_UMAC_MDF_ADDR0(n)                 (0xe54 + (n) * 0x8)
>   #define GENET_UMAC_MDF_ADDR1(n)                 (0xe58 + (n) * 0x8)
> +#define GENET_MAX_MDF_FILTER                    17
>   
>   #define GENET_DMA_DESC_COUNT                    256
>   #define GENET_DMA_DESC_SIZE                     12
> @@ -300,6 +301,12 @@ GenetSetPromisc (
>     IN BOOLEAN            Enable
>     );
>   
> +VOID
> +GenetEnableBroadcastFilter (
> +  IN GENET_PRIVATE_DATA   *Genet,
> +  IN BOOLEAN              Enable
> +  );
> +
>   VOID
>   GenetDmaInitRings (
>     IN GENET_PRIVATE_DATA *Genet
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c
> index 7ae9acec4c78..71c659e7f882 100644
> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c
> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c
> @@ -492,6 +492,39 @@ GenetSetPromisc (
>     GenetMmioWrite (Genet, GENET_UMAC_CMD, Value);
>   }
>   
> +/**
> +  Enable the MAC filter for the Ethernet broadcast address
> +
> +  @param  Genet[in]   Pointer to GENET_PRIVATE_DATA.
> +  @param  Enable[in]  Promiscuous mode state.
> +
> +**/
> +VOID
> +GenetEnableBroadcastFilter (
> +  IN GENET_PRIVATE_DATA   *Genet,
> +  IN BOOLEAN              Enable
> +  )
> +{
> +  CONST EFI_MAC_ADDRESS   *MacAddr = &Genet->SnpMode.CurrentAddress;
> +  UINT32                  Value;
> +
> +  if (Enable) {
> +    GenetMmioWrite (Genet, GENET_UMAC_MDF_ADDR0 (0),
> +      MacAddr->Addr[1] | MacAddr->Addr[0] << 8);
> +    GenetMmioWrite (Genet, GENET_UMAC_MDF_ADDR1 (0),
> +      MacAddr->Addr[5] | MacAddr->Addr[4] << 8 |
> +      MacAddr->Addr[3] << 16 | MacAddr->Addr[2] << 24);
> +
> +    GenetMmioWrite (Genet, GENET_UMAC_MDF_ADDR0 (1), 0xffff);
> +    GenetMmioWrite (Genet, GENET_UMAC_MDF_ADDR1 (1), 0xffffffff);
> +
> +    Value = (1U << GENET_MAX_MDF_FILTER) & ~(BIT1 | BIT0);
Is this right? Looking at the linux code, it looks like the a filter is 
enabled in a big endian field starting at bit 16 and working its way down.

AFAIK, you want bit 16, and bit 15 set for two filters?



> +  } else {
> +    Value = 0;
> +  }
> +  GenetMmioWrite (Genet, GENET_UMAC_MDF_CTRL, Value);
> +}
> +
>   /**
>     Configure DMA TX and RX queues, enabling them.
>   
>