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

[Nicolai Buchwitz <nb@tipi-net.de>]

On 31.8.2026 14:15, Maxime Chevallier wrote:
> 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 :)

Perfectly fine for me. Ran into a similar, but unrelated issue recently, 
so I just
wanted to flag it (for the future) :)

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

Also fine with me.

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

With both comments addressed and agreed to keep it as is until a cleanup 
in the future:

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai