> The qdisc offload callbacks invoked by stmmac_setup_tc() program > MTL/MAC registers, but they can be reached while the interface is down, > when stmmac_release() has dropped the runtime PM usage counter and the > device may be suspended with its clocks gated. Accessing the registers > in that state can trigger a bus error. > > Hold a runtime PM reference while configuring the register-touching > qdisc offloads (mqprio, cbs and taprio) so the device is active, and its > clocks enabled, whenever the MTL/MAC registers are programmed. > > The TC block callback stmmac_setup_tc_block_cb() programs the MTL/MAC > registers as well, but it runs asynchronously from stmmac_setup_tc(), > outside the runtime PM reference held there. Hold a runtime PM reference > for the whole stmmac_setup_tc_block_cb() call as well, covering the > cls_u32/cls_flower setup and the queue enable/disable accesses. > > No reference is held for the TC_SETUP_BLOCK bookkeeping itself, the > TC_QUERY_CAPS query or the tc-etf path, since none of them touch the > registers synchronously. In particular the block bind/unbind must reach > flow_block_cb_setup_simple() even when the device is suspended, so the > driver never leaves a stale flow_block_cb on its block list. > > Fixes: 1f705bc61aee ("net: stmmac: Add support for CBS QDISC") > Fixes: 4dbbe8dde848 ("net: stmmac: Add support for U32 TC filter using Flexible RX Parser") > Signed-off-by: Lorenzo Bianconi Commenting on sashiko's report: https://sashiko.dev/#/patchset/20260827-stmmac-setup-tc-enable-pm-v2-1-a9b8a5948f41%40oss.qualcomm.com > --- > Changes in v2: > - Return -EOPNOTSUPP in stmmac_setup_tc_block_cb() for unsupported TC > blocks. > - Link to v1: https://lore.kernel.org/r/20260824-stmmac-setup-tc-enable-pm-v1-1-45172d241a4b@oss.qualcomm.com > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 35 ++++++++++++++++++++--- > 1 file changed, 31 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index efa35cfecc4f..90753d5af6d6 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -6393,9 +6393,13 @@ static int stmmac_setup_tc_block_cb(enum tc_setup_type type, void *type_data, > void *cb_priv) > { > struct stmmac_priv *priv = cb_priv; > - int ret = -EOPNOTSUPP; > + int ret; > > if (!tc_cls_can_offload_and_chain0(priv->dev, type_data)) > + return -EOPNOTSUPP; > + > + ret = pm_runtime_resume_and_get(priv->device); > + if (ret < 0) > return ret; > > __stmmac_disable_all_queues(priv); > @@ -6408,10 +6412,13 @@ static int stmmac_setup_tc_block_cb(enum tc_setup_type type, void *type_data, > ret = stmmac_tc_setup_cls(priv, priv, type_data); > break; > default: > + ret = -EOPNOTSUPP; > break; > } > > stmmac_enable_all_queues(priv); > + pm_runtime_put(priv->device); > + > return ret; > } > > @@ -6421,26 +6428,46 @@ static int stmmac_setup_tc(struct net_device *ndev, enum tc_setup_type type, > void *type_data) > { > struct stmmac_priv *priv = netdev_priv(ndev); > + int ret; > > switch (type) { > case TC_QUERY_CAPS: > return stmmac_tc_query_caps(priv, priv, type_data); > case TC_SETUP_QDISC_MQPRIO: > - return stmmac_tc_setup_mqprio(priv, priv, type_data); > + ret = pm_runtime_resume_and_get(priv->device); > + if (ret < 0) > + return ret; > + > + ret = stmmac_tc_setup_mqprio(priv, priv, type_data); > + break; > case TC_SETUP_BLOCK: > return flow_block_cb_setup_simple(type_data, > &stmmac_block_cb_list, > stmmac_setup_tc_block_cb, > priv, priv, true); > case TC_SETUP_QDISC_CBS: > - return stmmac_tc_setup_cbs(priv, priv, type_data); > + ret = pm_runtime_resume_and_get(priv->device); > + if (ret < 0) > + return ret; > + > + ret = stmmac_tc_setup_cbs(priv, priv, type_data); > + break; > case TC_SETUP_QDISC_TAPRIO: > - return stmmac_tc_setup_taprio(priv, priv, type_data); > + ret = pm_runtime_resume_and_get(priv->device); > + if (ret < 0) > + return ret; > + > + ret = stmmac_tc_setup_taprio(priv, priv, type_data); > + break; > case TC_SETUP_QDISC_ETF: > return stmmac_tc_setup_etf(priv, priv, type_data); > default: > return -EOPNOTSUPP; > } > + > + pm_runtime_put(priv->device); > + - Does configuring TC offloads while the interface is down result in silent data loss when the interface is later brought up? By wrapping these accesses in pm_runtime_resume_and_get(), the driver now successfully programs the MAC/MTL registers while offline. However, when the interface is brought up, stmmac_open() calls stmmac_hw_setup(), which performs a hardware reset via stmmac_reset(). This hardware reset wipes all MAC and MTL registers back to default values. Since the driver lacks a mechanism to restore the TAPRIO, MQPRIO, or RXP configurations during stmmac_open() or system resume, the TC offloads are silently wiped from the hardware, leaving the software TC state completely diverged from the hardware state. - This issues have not been introduced by this patch and they should be addressed with a dedicated patches. Moreover, EST issue has been already addressed in this patch: https://lore.kernel.org/netdev/20260829-stmmac-est-reapply-after-open-v2-1-5e5ccb185e92@oss.qualcomm.com/ Regards, Lorenzo > + return ret; > } > > static u16 stmmac_select_queue(struct net_device *dev, struct sk_buff *skb, > > --- > base-commit: f967455fb2a5a2079b9eb5823e9ccf359174bf9f > change-id: 20260824-stmmac-setup-tc-enable-pm-149aa563d797 > > Best regards, > -- > Lorenzo Bianconi >