> 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 > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 34 ++++++++++++++++++++--- > 1 file changed, 30 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index b2b7d0242dd3..4baf40fb01dc 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -6392,9 +6392,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); > @@ -6411,6 +6415,8 @@ static int stmmac_setup_tc_block_cb(enum tc_setup_type type, void *type_data, > } > > stmmac_enable_all_queues(priv); > + pm_runtime_put(priv->device); commenting on sashiko's report: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824-stmmac-setup-tc-enable-pm-v1-1-45172d241a4b%40oss.qualcomm.com - Dropping the -EOPNOTSUPP initializer of ret changes what this callback returns for tc_setup_type values the driver does not handle. ret is now first assigned by pm_runtime_resume_and_get(), which returns exactly 0 on success - I will fix it in v2 > + > return ret; > } > > @@ -6420,26 +6426,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); - This is a pre-existing issue, but the patch now explicitly sanctions running taprio (and cls_u32/cls_flower in the block callback) with only the bus/CSR clocks resumed, without the rest of the hardware state those sequences depend on - This is fixed in the following patch: https://lore.kernel.org/netdev/20260825-stmmac-est-reapply-after-open-v1-1-dfa80735e0a1@oss.qualcomm.com/ Regards, Lorenzo > + break; > case TC_SETUP_QDISC_ETF: > return stmmac_tc_setup_etf(priv, priv, type_data); > default: > return -EOPNOTSUPP; > } > + > + pm_runtime_put(priv->device); > + > + return ret; > } > > static u16 stmmac_select_queue(struct net_device *dev, struct sk_buff *skb, > > --- > base-commit: 7cbfb180945ce529608e4d4e24a6d483699fab1e > change-id: 20260824-stmmac-setup-tc-enable-pm-149aa563d797 > > Best regards, > -- > Lorenzo Bianconi >