Re: [PATCH net-next] net: stmmac: fpe: Keep pmac always enabled

["Nazle Asmade, Muhammad Nazim Amirul" <muhammad.nazim.amirul.nazle.asmade@altera.com>]

On 28/7/2026 3:18 pm, Nazle Asmade, Muhammad Nazim Amirul wrote:
> On 27/7/2026 4:11 pm, Maxime Chevallier wrote:
>> Hi,
>>
>> On 7/27/26 07:13, muhammad.nazim.amirul.nazle.asmade@altera.com wrote:
>>> From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
>>>
>>> The preemptive MAC in DWMAC is always enabled, meaning DWMAC is always
>>> ready to receive preemptive mPackets. Keep pmac_enabled true at init
>>> time and reject attempts to disable it via ethtool set_mm.
>>>
>>> The FPE residual queue (fprq) controls which Rx queue receives residual
>>> preemptive mPackets from the pmac. Move its configuration from
>>> stmmac_fpe_configure_tx() to stmmac_fpe_configure_pmac() where it
>>> logically belongs.
>>>
>>> Also add a missing return in stmmac_fpe_init() so that the pmac_enabled
>>> initialization is not reached on hardware where FPE is not supported by
>>> the driver.
>>
>> Can you split this up into individual patches, one for the 
>> stmmac_set_mm() and
>> stmmac_fpe_init() to validate that we keep pmac on, one for the 
>> missing return,
>> and one for the pmac configuration in the right fonction ?
> 
> sure will split as per your suggestion>
>> Also one comment bellow :>
>>> Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
>>> Signed-off-by: Nazim Amirul 
>>> <muhammad.nazim.amirul.nazle.asmade@altera.com>
>>> ---
>>>   .../ethernet/stmicro/stmmac/stmmac_ethtool.c  |  7 ++++
>>>   .../net/ethernet/stmicro/stmmac/stmmac_fpe.c  | 34 ++++++++++++-------
>>>   2 files changed, 29 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/ 
>>> drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
>>> index 92585d27ab88..8ebdd249e5c8 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
>>> @@ -1064,6 +1064,13 @@ static int stmmac_set_mm(struct net_device 
>>> *ndev, struct ethtool_mm_cfg *cfg,
>>>       if (err)
>>>           return err;
>>> +    if (!priv->dma_cap.fpesel)
>>> +        return -EOPNOTSUPP;
>>> +
>>> +    /* DWMAC always have preemptible MAC enabled */
>>> +    if (!cfg->pmac_enabled)
>>> +        return -EINVAL;
>>> +
>>>       stmmac_fpe_set_add_frag_size(priv, frag_size);
>>>       ethtool_mmsv_set_mm(&priv->fpe_cfg.mmsv, cfg);
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c b/ 
>>> drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c
>>> index c54c70224351..2bfa84ad04a6 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c
>>> @@ -47,20 +47,13 @@ static void stmmac_fpe_configure_tx(struct 
>>> ethtool_mmsv *mmsv, bool tx_enable)
>>>       struct stmmac_fpe_cfg *cfg = container_of(mmsv, struct 
>>> stmmac_fpe_cfg, mmsv);
>>>       struct stmmac_priv *priv = container_of(cfg, struct 
>>> stmmac_priv, fpe_cfg);
>>>       const struct stmmac_fpe_reg *reg = cfg->reg;
>>> -    u32 num_rxq = priv->plat->rx_queues_to_use;
>>>       void __iomem *ioaddr = priv->ioaddr;
>>> -    u32 value;
>>> -    if (tx_enable) {
>>> +    if (tx_enable)
>>>           cfg->fpe_csr = STMMAC_MAC_FPE_CTRL_STS_EFPE;
>>> -        value = readl(ioaddr + reg->rxq_ctrl1_reg);
>>> -        value &= ~reg->fprq_mask;
>>> -        /* Keep this SHIFT, FIELD_PREP() expects a constant mask :-/ */
>>> -        value |= (num_rxq - 1) << __ffs(reg->fprq_mask);
>>> -        writel(value, ioaddr + reg->rxq_ctrl1_reg);
>>> -    } else {
>>> +    else
>>>           cfg->fpe_csr = 0;
>>> -    }
>>> +
>>>       writel(cfg->fpe_csr, ioaddr + reg->mac_fpe_reg);
>>>   }
>>> @@ -68,10 +61,11 @@ static void stmmac_fpe_configure_pmac(struct 
>>> ethtool_mmsv *mmsv, bool pmac_enabl
>>>   {
>>>       struct stmmac_fpe_cfg *cfg = container_of(mmsv, struct 
>>> stmmac_fpe_cfg, mmsv);
>>>       struct stmmac_priv *priv = container_of(cfg, struct 
>>> stmmac_priv, fpe_cfg);
>>> +    u32 num_rxq = priv->plat->rx_queues_to_use;
>>>       const struct stmmac_fpe_reg *reg = cfg->reg;
>>>       void __iomem *ioaddr = priv->ioaddr;
>>>       unsigned long flags;
>>> -    u32 value;
>>> +    u32 value, value_2;
>>>       spin_lock_irqsave(&priv->hw->irq_ctrl_lock, flags);
>>>       value = readl(ioaddr + reg->int_en_reg);
>>> @@ -83,6 +77,14 @@ static void stmmac_fpe_configure_pmac(struct 
>>> ethtool_mmsv *mmsv, bool pmac_enabl
>>>               value |= reg->int_en_bit;
>>>           }
>>> +        /* Frame Preemption Residue Queue is the Rx Queue to which
>>> +         * residual preemptive mPackets must be forwarded from the 
>>> pmac.
>>> +         */
>>> +        value_2 = readl(ioaddr + reg->rxq_ctrl1_reg);
>>> +        value_2 &= ~reg->fprq_mask;
>>> +        /* Keep this SHIFT, FIELD_PREP() expects a constant mask :-/ */
>>
>> you can use u32_replace_bits for non constant expr :
> will update and repost it, Thanks Maxime!>
>> https://nam10.safelinks.protection.outlook.com/? 
>> url=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv7.2- 
>> rc4%2Fsource%2Finclude%2Flinux%2Fbitfield.h%23L236&data=05%7C02%7Cmuhammad.nazim.amirul.nazle.asmade%40altera.com%7Ccc409a6e8ac34d18f82d08deebb6b578%7Cfbd72e03d4a54110adce614d51f2077a%7C0%7C0%7C639207367157805862%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=blOrVXSlgOqDnVPdnNnLXXdrIhz9iVeb98SAp2kI3AQ%3D&reserved=0
>>
>> (it's autogenerated and not documented, hence very hard to find IMO)
>>
>> Maxime
> 
Hi Maxime,

You may refer to the lore below,

https://lore.kernel.org/all/20260729015247.25774-1-muhammad.nazim.amirul.nazle.asmade@altera.com/

Hope this is the expectation, Thanks!