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

[Nazle Asmade, Muhammad Nazim Amirul]

On 27/7/2026 4:11 pm, Maxime Chevallier wrote:
> Hi,
>
> On 7/27/26 07:13, muhammad.nazim.amirul.nazle.asmade@xxxxxxxxxx wrote:
>> From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>> Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@xxxxxxxxxx>
>> ---
>>   .../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://elixir.bootlin.com/linux/v7.2-rc4/source/include/linux/bitfield.h#L236
>
> (it's autogenerated and not documented, hence very hard to find IMO)
>
> Maxime