Re: [PATCH net-next 1/2] net: stmmac: dwmac4: Read the UC filter size from hardware capabilities

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

On 8/31/26 14:04, Nicolai Buchwitz wrote:
> Hi Maxime
> 
> On 31.8.2026 09:01, Maxime Chevallier wrote:
>> dwmac4 has multiple banks of perfect filter entries, independently
>> configurable during IP integration.
>>
>> The multi_addr bank reports a number between 0 and 31 corresponding to
>> the actual number of entries in that bank, while the 32 and 64 banks
>> are all-or-nothing.
>>
>> Expose these caps over debugfs as well.
>>
>> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
>> ---
> 
>> [...]
> 
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>> index 6382836828ba..89368e34a388 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>> @@ -176,7 +176,9 @@ enum power_event {
>>
>>  /* MAC HW features0 bitmap */
>>  #define GMAC_HW_FEAT_SAVLANINS        BIT(27)
>> -#define GMAC_HW_FEAT_ADDMAC        BIT(18)
>> +#define GMAC_HW_FEAT_MACADR64SEL    BIT(24)
>> +#define GMAC_HW_FEAT_MACADR32SEL    BIT(23)
>> +#define GMAC_HW_FEAT_ADDMAC        GENMASK(22, 18)
>>  #define GMAC_HW_FEAT_RXCOESEL        BIT(16)
>>  #define GMAC_HW_FEAT_TXCOSEL        BIT(14)
>>  #define GMAC_HW_FEAT_EEESEL        BIT(13)
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>> index 23ffe1adcd0d..14ac3f0e51f7 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>> @@ -388,6 +388,8 @@ static int dwmac4_get_hw_feature(void __iomem *ioaddr,
>>      dma_cap->half_duplex = (hw_cap & GMAC_HW_FEAT_HDSEL) >> 2;
>>      dma_cap->vlhash = (hw_cap & GMAC_HW_FEAT_VLHASH) >> 4;
>>      dma_cap->multi_addr = (hw_cap & GMAC_HW_FEAT_ADDMAC) >> 18;
> 
> Now that ADDMAC has grown from single bit to a mask, the hardcoded 18 has to match
> dwmac4.h. So IMHO it would make sense to use FIELD_GET() here (like actphyif)?

It's hardcoded all over, if you look at how all the other fields from the features
are read. I can change it if you really think it's worth, however I'd rather
do a proper cleanup of how all the properties are read at once :)

> 
>> +    dma_cap->additional_32_addr = (hw_cap & GMAC_HW_FEAT_MACADR32SEL) >> 23;
>> +    dma_cap->additional_64_addr = (hw_cap & GMAC_HW_FEAT_MACADR64SEL) >> 24;
>>      dma_cap->pcs = (hw_cap & GMAC_HW_FEAT_PCSSEL) >> 3;
>>      dma_cap->sma_mdio = (hw_cap & GMAC_HW_FEAT_SMASEL) >> 5;
>>      dma_cap->pmt_remote_wake_up = (hw_cap & GMAC_HW_FEAT_RWKSEL) >> 6;
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index f2fc89176654..a885f8cfef21 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -6580,6 +6580,18 @@ static int stmmac_dma_cap_show(struct seq_file *seq, void *v)
>>          seq_printf(seq,
>>                 "\tNumber of Additional MAC address registers: %d\n",
>>                 priv->dma_cap.multi_addr);
>> +    } else if (priv->plat->core_type == DWMAC_CORE_GMAC4) {
>> +        seq_printf(seq,
>> +               "\tNumber of MAC address registers (1-31): %d\n",
>> +               priv->dma_cap.multi_addr);
>> +        seq_printf(seq,
>> +               "\tAdditional 32 MAC address registers (32-63): %s\n",
>> +               priv->dma_cap.additional_32_addr ? "Y" : "N");
>> +        seq_printf(seq,
>> +               "\tAdditional 64 MAC address registers (64-127): %s\n",
>> +               priv->dma_cap.additional_64_addr ? "Y" : "N");
>> +        seq_printf(seq, "\tHash Filter: %s\n",
>> +               (priv->dma_cap.hash_filter) ? "Y" : "N");
> 
> hash_filter seems to be only set in dwmac1000_dma.c? So on dwmac4 it would always print N?

Yeah but dwmac4 has a hash filter, but same as the UC filter it's just not plumbed
in :/

Let's leave this flag here, hash filter addition for MC filtering is coming-up, so let's not
drop it now only to re-enable it after, one of the other wonderful discoveries found by
running the selftests...

> 
>>      } else {
>>          seq_printf(seq, "\tHash Filter: %s\n",
>>                 (priv->dma_cap.hash_filter) ? "Y" : "N");
> 
> Thanks,
> Nicolai

Maxime