net: stmmac: FPE via ethtool + tc
Subsystems (4)
v7 cover net: stmmac: FPE via ethtool + tc
Move the Frame Preemption(FPE) over to the new standard API which uses ethtool-mm/tc-mqprio/tc-taprio.
Changes in v7:
1. code style fixes and clean up warnings reported by patchwork netdev checks, no functional change intended
Changes in v6:
1. new FPE verification process based on Vladimir Oltean's proposal
2. embed ethtool_mm_state into stmmac_fpe_cfg
3. convert some bit ops to u32_replace_bits
4. register name and function name update to be more descriptive
5. split up stmmac_tc_ops of dwmac4+ and dwxgmac, they have different implementations about mqprio
6. some code style fixes
Changes in v5:
1. fix typo in commit message
2. drop FPE capability check in tc-mqprio/tc-taprio
Changes in v4:
1. reorder FPE-related declarations and definitions into clean groups
2. move mm_lock to stmmac_fpe_cfg.lock
3. protect user configurations across NIC up/down
4. block stmmac_set_mm() when fpe_task is in progress to finish
5. convert to ethtool_dev_mm_supported() to check FPE capability in tc-mqprio/tc-taprio
6. silence FPE workqueue start/stop logs
Changes in v3:
1. avoid races among ISR, workqueue, link update and register configuration.
2. update FPE verification retry logic, so it retries and fails as expected.
Changes in v2:
1. refactor FPE verification process
2. suspend/resume and kselftest-ethtool_mm, all test cases passed
3. handle TC:TXQ remapping for DWMAC CORE4+
Furong Xu (7):
net: stmmac: move stmmac_fpe_cfg to stmmac_priv data
net: stmmac: drop stmmac_fpe_handshake
net: stmmac: refactor FPE verification process
net: stmmac: configure FPE via ethtool-mm
net: stmmac: support fp parameter of tc-mqprio
net: stmmac: support fp parameter of tc-taprio
net: stmmac: silence FPE kernel logs
.../net/ethernet/stmicro/stmmac/dwmac4_core.c | 10 +- drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 96 +++++- drivers/net/ethernet/stmicro/stmmac/dwmac5.h | 12 +- .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 9 +- drivers/net/ethernet/stmicro/stmmac/hwif.c | 4 +- drivers/net/ethernet/stmicro/stmmac/hwif.h | 21 +- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 30 +- .../ethernet/stmicro/stmmac/stmmac_ethtool.c | 91 ++++++ .../net/ethernet/stmicro/stmmac/stmmac_main.c | 275 ++++++++---------- .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 130 ++++++--- include/linux/stmmac.h | 28 -- 11 files changed, 467 insertions(+), 239 deletions(-)
Signature · 1 line
2.34.1
PATCH v7 1/7 net: stmmac: move stmmac_fpe_cfg to stmmac_priv data
By moving the fpe_cfg field to the stmmac_priv data, stmmac_fpe_cfg becomes platform-data eventually, instead of a run-time config.
--- drivers/net/ethernet/stmicro/stmmac/hwif.h | 2 ++ drivers/net/ethernet/stmicro/stmmac/stmmac.h | 30 ++++++++++++++++++- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 20 ++++++------- .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 16 ++-------- include/linux/stmmac.h | 28 ----------------- 5 files changed, 44 insertions(+), 52 deletions(-)
drivers/net/ethernet/stmicro/stmmac/hwif.h +2 -0
@@ -28,6 +28,8 @@struct stmmac_extra_stats ;struct stmmac_priv ;struct stmmac_safety_stats ; +struct stmmac_fpe_cfg ; +enum stmmac_mpacket_type ;struct dma_desc ;struct dma_extended_desc ;struct dma_edesc ;
drivers/net/ethernet/stmicro/stmmac/stmmac.h +29 -1
@@ -146,6 +146,33 @@ struct stmmac_channel {u32 index; }; +/* FPE link state */ +enum stmmac_fpe_state { +FPE_STATE_OFF =0 , +FPE_STATE_CAPABLE =1 , +FPE_STATE_ENTERING_ON =2 , +FPE_STATE_ON =3 , +}; + +/* FPE link-partner hand-shaking mPacket type */ +enum stmmac_mpacket_type { +MPACKET_VERIFY =0 , +MPACKET_RESPONSE =1 , +}; + +enum stmmac_fpe_task_state_t { + __FPE_REMOVING, + __FPE_TASK_SCHED, +}; + +struct stmmac_fpe_cfg { +bool enable;/* FPE enable */ +bool hs_enable;/* FPE handshake enable */ +enum stmmac_fpe_state lp_fpe_state;/* Link Partner FPE state */ +enum stmmac_fpe_state lo_fpe_state;/* Local station FPE state */ +u32 fpe_csr;/* MAC_FPE_CTRL_STS reg cache */ +}; +struct stmmac_tc_entry {bool in_use;bool in_hw;
@@ -339,11 +366,12 @@ struct stmmac_priv {struct workqueue_struct *wq;struct work_struct service_task; -/* Workqueue for handling FPE hand-shaking */ +/* Frame Preemption feature (FPE) */ unsigned long fpe_task_state;struct workqueue_struct *fpe_wq;struct work_struct fpe_task;char wq_name[IFNAMSIZ +4 ]; +struct stmmac_fpe_cfg fpe_cfg;/* TC Handling */ unsigned int tc_entries_max;
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10 -10
@@ -968,7 +968,7 @@ static void stmmac_mac_config(struct phylink_config *config, unsigned int mode,static void stmmac_fpe_link_state_handle (struct stmmac_priv *priv,bool is_up) { -struct stmmac_fpe_cfg *fpe_cfg = priv->plat->fpe_cfg; +struct stmmac_fpe_cfg *fpe_cfg = &priv->fpe_cfg;enum stmmac_fpe_state *lo_state = &fpe_cfg->lo_fpe_state;enum stmmac_fpe_state *lp_state = &fpe_cfg->lp_fpe_state;bool *hs_enable = &fpe_cfg->hs_enable;
@@ -3536,7 +3536,7 @@ static int stmmac_hw_setup(struct net_device *dev, bool ptp_register)if (priv->dma_cap.fpesel) {stmmac_fpe_start_wq (priv); -if (priv->plat->fpe_cfg->enable) +if (priv->fpe_cfg.enable)stmmac_fpe_handshake (priv, true); }
@@ -5982,7 +5982,7 @@ static int stmmac_set_features(struct net_device *netdev,static void stmmac_fpe_event_status (struct stmmac_priv *priv,int status) { -struct stmmac_fpe_cfg *fpe_cfg = priv->plat->fpe_cfg; +struct stmmac_fpe_cfg *fpe_cfg = &priv->fpe_cfg;enum stmmac_fpe_state *lo_state = &fpe_cfg->lo_fpe_state;enum stmmac_fpe_state *lp_state = &fpe_cfg->lp_fpe_state;bool *hs_enable = &fpe_cfg->hs_enable;
@@ -7381,7 +7381,7 @@ static void stmmac_fpe_lp_task(struct work_struct *work) { struct stmmac_priv *priv = container_of(work, struct stmmac_priv, fpe_task); -struct stmmac_fpe_cfg *fpe_cfg = priv->plat->fpe_cfg; +struct stmmac_fpe_cfg *fpe_cfg = &priv->fpe_cfg;enum stmmac_fpe_state *lo_state = &fpe_cfg->lo_fpe_state;enum stmmac_fpe_state *lp_state = &fpe_cfg->lp_fpe_state;bool *hs_enable = &fpe_cfg->hs_enable;
@@ -7427,17 +7427,17 @@ static void stmmac_fpe_lp_task(struct work_struct *work)void stmmac_fpe_handshake (struct stmmac_priv *priv,bool enable) { -if (priv->plat->fpe_cfg->hs_enable != enable) { +if (priv->fpe_cfg.hs_enable != enable) {if (enable) {stmmac_fpe_send_mpacket (priv, priv->ioaddr, - priv->plat->fpe_cfg, + &priv->fpe_cfg,MPACKET_VERIFY ); }else { - priv->plat->fpe_cfg->lo_fpe_state =FPE_STATE_OFF ; - priv->plat->fpe_cfg->lp_fpe_state =FPE_STATE_OFF ; + priv->fpe_cfg.lo_fpe_state =FPE_STATE_OFF ; + priv->fpe_cfg.lp_fpe_state =FPE_STATE_OFF ; } - priv->plat->fpe_cfg->hs_enable = enable; + priv->fpe_cfg.hs_enable = enable; } }
@@ -7898,7 +7898,7 @@ int stmmac_suspend(struct device *dev)if (priv->dma_cap.fpesel) {/* Disable FPE */ stmmac_fpe_configure (priv, priv->ioaddr, - priv->plat->fpe_cfg, + &priv->fpe_cfg, priv->plat->tx_queues_to_use, priv->plat->rx_queues_to_use, false);
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +3 -13
@@ -282,16 +282,6 @@ static int tc_init(struct stmmac_priv *priv)if (ret)return -ENOMEM ; -if (!priv->plat->fpe_cfg) { - priv->plat->fpe_cfg =devm_kzalloc (priv->device, -sizeof (*priv->plat->fpe_cfg), -GFP_KERNEL ); -if (!priv->plat->fpe_cfg) -return -ENOMEM ; - }else { -memset (priv->plat->fpe_cfg,0 ,sizeof (*priv->plat->fpe_cfg)); - } -/* Fail silently as we can still use remaining features, e.g. CBS */ if (!dma_cap->frpsel)return 0 ;
@@ -1076,7 +1066,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv,/* Actual FPE register configuration will be done after FPE handshake * is success. */ - priv->plat->fpe_cfg->enable = fpe; + priv->fpe_cfg.enable = fpe; ret =stmmac_est_configure (priv, priv, priv->est, priv->plat->clk_ptp_rate);
@@ -1109,9 +1099,9 @@ static int tc_taprio_configure(struct stmmac_priv *priv,mutex_unlock (&priv->est_lock); } - priv->plat->fpe_cfg->enable = false; + priv->fpe_cfg.enable = false;stmmac_fpe_configure (priv, priv->ioaddr, - priv->plat->fpe_cfg, + &priv->fpe_cfg, priv->plat->tx_queues_to_use, priv->plat->rx_queues_to_use, false);
include/linux/stmmac.h +0 -28
@@ -138,33 +138,6 @@ struct stmmac_txq_cfg {int tbs_en; }; -/* FPE link state */ -enum stmmac_fpe_state { -FPE_STATE_OFF =0 , -FPE_STATE_CAPABLE =1 , -FPE_STATE_ENTERING_ON =2 , -FPE_STATE_ON =3 , -}; - -/* FPE link-partner hand-shaking mPacket type */ -enum stmmac_mpacket_type { -MPACKET_VERIFY =0 , -MPACKET_RESPONSE =1 , -}; - -enum stmmac_fpe_task_state_t { - __FPE_REMOVING, - __FPE_TASK_SCHED, -}; - -struct stmmac_fpe_cfg { -bool enable;/* FPE enable */ -bool hs_enable;/* FPE handshake enable */ -enum stmmac_fpe_state lp_fpe_state;/* Link Partner FPE state */ -enum stmmac_fpe_state lo_fpe_state;/* Local station FPE state */ -u32 fpe_csr;/* MAC_FPE_CTRL_STS reg cache */ -}; -struct stmmac_safety_feature_cfg {u32 tsoee;u32 mrxpee;
@@ -232,7 +205,6 @@ struct plat_stmmacenet_data {struct fwnode_handle *port_node;struct device_node *mdio_node;struct stmmac_dma_cfg *dma_cfg; -struct stmmac_fpe_cfg *fpe_cfg;struct stmmac_safety_feature_cfg *safety_feat_cfg;int clk_csr;int has_gmac;
Signature · 1 line
2.34.1
PATCH v7 2/7 net: stmmac: drop stmmac_fpe_handshake
In reply to Furong Xuethtool --set-mm can trigger FPE verification process by calling stmmac_fpe_send_mpacket, stmmac_fpe_handshake should be gone.
--- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 - .../net/ethernet/stmicro/stmmac/stmmac_main.c | 23 +------------------ .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 8 ------- 3 files changed, 1 insertion(+), 31 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac.h +0 -1
@@ -425,7 +425,6 @@ bool stmmac_eee_init(struct stmmac_priv *priv);int stmmac_reinit_queues (struct net_device *dev,u32 rx_cnt,u32 tx_cnt);int stmmac_reinit_ringparam (struct net_device *dev,u32 rx_size,u32 tx_size);int stmmac_bus_clks_config (struct stmmac_priv *priv,bool enabled); -void stmmac_fpe_handshake (struct stmmac_priv *priv,bool enable);static inline bool stmmac_xdp_is_enabled (struct stmmac_priv *priv) {
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +1 -22
@@ -3533,13 +3533,9 @@ static int stmmac_hw_setup(struct net_device *dev, bool ptp_register)stmmac_set_hw_vlan_mode (priv, priv->hw); -if (priv->dma_cap.fpesel) { +if (priv->dma_cap.fpesel)stmmac_fpe_start_wq (priv); -if (priv->fpe_cfg.enable) -stmmac_fpe_handshake (priv, true); - } -return 0 ; }
@@ -7425,22 +7421,6 @@ static void stmmac_fpe_lp_task(struct work_struct *work)clear_bit (__FPE_TASK_SCHED, &priv->fpe_task_state); } -void stmmac_fpe_handshake (struct stmmac_priv *priv,bool enable) -{ -if (priv->fpe_cfg.hs_enable != enable) { -if (enable) { -stmmac_fpe_send_mpacket (priv, priv->ioaddr, - &priv->fpe_cfg, -MPACKET_VERIFY ); - }else { - priv->fpe_cfg.lo_fpe_state =FPE_STATE_OFF ; - priv->fpe_cfg.lp_fpe_state =FPE_STATE_OFF ; - } - - priv->fpe_cfg.hs_enable = enable; - } -} -static int stmmac_xdp_rx_timestamp (const struct xdp_md *_ctx,u64 *timestamp) {const struct stmmac_xdp_buff *ctx = (void *)_ctx;
@@ -7902,7 +7882,6 @@ int stmmac_suspend(struct device *dev) priv->plat->tx_queues_to_use, priv->plat->rx_queues_to_use, false); -stmmac_fpe_handshake (priv, false);stmmac_fpe_stop_wq (priv); }
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +0 -8
@@ -1078,11 +1078,6 @@ static int tc_taprio_configure(struct stmmac_priv *priv,netdev_info (priv->dev,"configured EST\n" ); -if (fpe) { -stmmac_fpe_handshake (priv, true); -netdev_info (priv->dev,"start FPE handshake\n" ); - } -return 0 ; disable:
@@ -1107,9 +1102,6 @@ static int tc_taprio_configure(struct stmmac_priv *priv, false);netdev_info (priv->dev,"disabled FPE\n" ); -stmmac_fpe_handshake (priv, false); -netdev_info (priv->dev,"stop FPE handshake\n" ); -return ret; }
Signature · 1 line
2.34.1
PATCH v7 3/7 net: stmmac: refactor FPE verification process
In reply to Furong XuDrop driver defined stmmac_fpe_state, and switch to common ethtool_mm_verify_status for local TX verification status.
Local side and remote side verification processes are completely independent. There is no reason at all to keep a local state and a remote state.
Add a spinlock to avoid races among ISR, timer, link update and register configuration.
This patch is based on Vladimir Oltean's proposal.
Vladimir Oltean says:
====================
In the INITIAL state, the timer sends MPACKET_VERIFY. Eventually the stmmac_fpe_event_status() IRQ fires and advances the state to VERIFYING, then rearms the timer after verify_time ms. If a subsequent IRQ comes in and modifies the state to SUCCEEDED after getting MPACKET_RESPONSE, the timer sees this. It must enable the EFPE bit now. Otherwise, it decrements the verify_limit counter and tries again. Eventually it moves the status to FAILED, from which the IRQ cannot move it anywhere else, except for another stmmac_fpe_apply() call.
====================
--- .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 4 - drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 19 +- drivers/net/ethernet/stmicro/stmmac/dwmac5.h | 2 +- .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 9 +- drivers/net/ethernet/stmicro/stmmac/hwif.h | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 31 +-- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 258 +++++++++--------- .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 8 +- 8 files changed, 164 insertions(+), 169 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +0 -4
@@ -58,10 +58,6 @@ static void dwmac4_core_init(struct mac_device_info *hw,if (hw->pcs) value |=GMAC_PCS_IRQ_DEFAULT ; -/* Enable FPE interrupt */ -if ((GMAC_HW_FEAT_FPESEL &readl (ioaddr +GMAC_HW_FEATURE3 )) >>26 ) - value |=GMAC_INT_FPE_EN ; -writel (value, ioaddr +GMAC_INT_EN );if (GMAC_INT_DEFAULT_ENABLE &GMAC_INT_TSIE )
drivers/net/ethernet/stmicro/stmmac/dwmac5.c +17 -2
@@ -575,11 +575,11 @@ int dwmac5_flex_pps_config(void __iomem *ioaddr, int index,void dwmac5_fpe_configure (void __iomem *ioaddr,struct stmmac_fpe_cfg *cfg,u32 num_txq,u32 num_rxq, -bool enable) +bool tx_enable,bool pmac_enable) {u32 value; -if (enable) { +if (tx_enable) { cfg->fpe_csr =EFPE ; value =readl (ioaddr +GMAC_RXQ_CTRL1 ); value &= ~GMAC_RXQCTRL_FPRQ ;
@@ -589,6 +589,21 @@ void dwmac5_fpe_configure(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg, cfg->fpe_csr =0 ; }writel (cfg->fpe_csr, ioaddr +MAC_FPE_CTRL_STS ); + + value =readl (ioaddr +GMAC_INT_EN ); + +if (pmac_enable) { +if (!(value &GMAC_INT_FPE_EN )) { +/* Dummy read to clear any pending masked interrupts */ +readl (ioaddr +MAC_FPE_CTRL_STS ); + + value |=GMAC_INT_FPE_EN ; + } + }else { + value &= ~GMAC_INT_FPE_EN ; + } + +writel (value, ioaddr +GMAC_INT_EN ); }int dwmac5_fpe_irq_status (void __iomem *ioaddr,struct net_device *dev)
drivers/net/ethernet/stmicro/stmmac/dwmac5.h +1 -1
@@ -104,7 +104,7 @@ int dwmac5_flex_pps_config(void __iomem *ioaddr, int index, u32 sub_second_inc, u32 systime_flags);void dwmac5_fpe_configure (void __iomem *ioaddr,struct stmmac_fpe_cfg *cfg,u32 num_txq,u32 num_rxq, -bool enable); +bool tx_enable,bool pmac_enable);void dwmac5_fpe_send_mpacket (void __iomem *ioaddr,struct stmmac_fpe_cfg *cfg,enum stmmac_mpacket_type type);
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +5 -4
@@ -1504,13 +1504,14 @@ static void dwxgmac2_set_arp_offload(struct mac_device_info *hw, bool en,writel (value, ioaddr +XGMAC_RX_CONFIG ); } -static void dwxgmac3_fpe_configure (void __iomem *ioaddr,struct stmmac_fpe_cfg *cfg, -u32 num_txq, -u32 num_rxq,bool enable) +static void dwxgmac3_fpe_configure (void __iomem *ioaddr, +struct stmmac_fpe_cfg *cfg, +u32 num_txq,u32 num_rxq, +bool tx_enable,bool pmac_enable) {u32 value; -if (!enable) { +if (!tx_enable) { value =readl (ioaddr +XGMAC_FPE_CTRL_STS ); value &= ~XGMAC_EFPE ;
drivers/net/ethernet/stmicro/stmmac/hwif.h +1 -1
@@ -421,7 +421,7 @@ struct stmmac_ops {void (*set_arp_offload)(struct mac_device_info *hw,bool en,u32 addr);void (*fpe_configure)(void __iomem *ioaddr,struct stmmac_fpe_cfg *cfg,u32 num_txq,u32 num_rxq, -bool enable); +bool tx_enable,bool pmac_enable);void (*fpe_send_mpacket)(void __iomem *ioaddr,struct stmmac_fpe_cfg *cfg,enum stmmac_mpacket_type type);
drivers/net/ethernet/stmicro/stmmac/stmmac.h +11 -20
@@ -146,31 +146,25 @@ struct stmmac_channel {u32 index; }; -/* FPE link state */ -enum stmmac_fpe_state { -FPE_STATE_OFF =0 , -FPE_STATE_CAPABLE =1 , -FPE_STATE_ENTERING_ON =2 , -FPE_STATE_ON =3 , -}; -/* FPE link-partner hand-shaking mPacket type */ enum stmmac_mpacket_type {MPACKET_VERIFY =0 ,MPACKET_RESPONSE =1 , }; -enum stmmac_fpe_task_state_t { - __FPE_REMOVING, - __FPE_TASK_SCHED, -}; +#define STMMAC_FPE_MM_MAX_VERIFY_RETRIES 3 +#define STMMAC_FPE_MM_MAX_VERIFY_TIME_MS 128struct stmmac_fpe_cfg { -bool enable;/* FPE enable */ -bool hs_enable;/* FPE handshake enable */ -enum stmmac_fpe_state lp_fpe_state;/* Link Partner FPE state */ -enum stmmac_fpe_state lo_fpe_state;/* Local station FPE state */ +/* Serialize access to MAC Merge state between ethtool requests +* and link state updates. +*/ +spinlock_t lock; +u32 fpe_csr;/* MAC_FPE_CTRL_STS reg cache */ +struct timer_list verify_timer; +struct ethtool_mm_state state; +int verify_retries; };struct stmmac_tc_entry {
@@ -367,10 +361,6 @@ struct stmmac_priv {struct work_struct service_task;/* Frame Preemption feature (FPE) */ -unsigned long fpe_task_state; -struct workqueue_struct *fpe_wq; -struct work_struct fpe_task; -char wq_name[IFNAMSIZ +4 ];struct stmmac_fpe_cfg fpe_cfg;/* TC Handling */
@@ -425,6 +415,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv);int stmmac_reinit_queues (struct net_device *dev,u32 rx_cnt,u32 tx_cnt);int stmmac_reinit_ringparam (struct net_device *dev,u32 rx_size,u32 tx_size);int stmmac_bus_clks_config (struct stmmac_priv *priv,bool enabled); +void stmmac_fpe_apply (struct stmmac_priv *priv);static inline bool stmmac_xdp_is_enabled (struct stmmac_priv *priv) {
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +128 -130
337 lines not on this page · raw message
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +1 -7
@@ -1063,11 +1063,6 @@ static int tc_taprio_configure(struct stmmac_priv *priv,return -EOPNOTSUPP ; } -/* Actual FPE register configuration will be done after FPE handshake -* is success. -*/ - priv->fpe_cfg.enable = fpe; - ret =stmmac_est_configure (priv, priv, priv->est, priv->plat->clk_ptp_rate);mutex_unlock (&priv->est_lock);
@@ -1094,12 +1089,11 @@ static int tc_taprio_configure(struct stmmac_priv *priv,mutex_unlock (&priv->est_lock); } - priv->fpe_cfg.enable = false;stmmac_fpe_configure (priv, priv->ioaddr, &priv->fpe_cfg, priv->plat->tx_queues_to_use, priv->plat->rx_queues_to_use, - false); + false, false);netdev_info (priv->dev,"disabled FPE\n" );return ret;
Signature · 1 line
2.34.1
Re: [PATCH net-next v7 3/7] net: stmmac: refactor FPE verification process
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c int stmmac_reinit_ringparam(struct net_device *dev, u32 rx_size, u32 tx_size) · line 7401
Quoted 12 lines · Furong Xu
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12
-0+static void stmmac_fpe_verify_timer_arm (struct stmmac_fpe_cfg *fpe_cfg) +{ +struct ethtool_mm_state *state = &fpe_cfg->state; + +if (state->pmac_enabled && state->tx_enabled && + state->verify_enabled && + state->verify_status !=ETHTOOL_MM_VERIFY_STATUS_FAILED && + state->verify_status !=ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED ) { +/* give caller a chance to release the spinlock */ +mod_timer (&fpe_cfg->verify_timer, jiffies +1 ); + } +}
Why do you need to give the caller a chance to release the spinlock? Isn't the timer code blocked anyway, as stmmac_fpe_verify_timer_arm() runs under irqsoff?
Re: [PATCH net-next v7 3/7] net: stmmac: refactor FPE verification process
In reply to Furong XuQuoted 59 lines · Furong Xu
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +20
-35+/** +* stmmac_fpe_verify_timer - Timer for MAC Merge verification +* @t: timer_list struct containing private info +* +* Verify the MAC Merge capability in the local TX direction, by +* transmitting Verify mPackets up to 3 times. Wait until link +* partner responds with a Response mPacket, otherwise fail. +*/ +static void stmmac_fpe_verify_timer (struct timer_list *t) { - struct stmmac_priv *priv = container_of(work, struct stmmac_priv, - fpe_task); -struct stmmac_fpe_cfg *fpe_cfg = &priv->fpe_cfg; -enum stmmac_fpe_state *lo_state = &fpe_cfg->lo_fpe_state; -enum stmmac_fpe_state *lp_state = &fpe_cfg->lp_fpe_state; -bool *hs_enable = &fpe_cfg->hs_enable; -bool *enable = &fpe_cfg->enable; -int retries =20 ; - -while (retries-- >0 ) { -/* Bail out immediately if FPE handshake is OFF */ -if (*lo_state ==FPE_STATE_OFF || !*hs_enable) -break ; - -if (*lo_state ==FPE_STATE_ENTERING_ON && - *lp_state ==FPE_STATE_ENTERING_ON ) { -stmmac_fpe_configure (priv, priv->ioaddr, - fpe_cfg, - priv->plat->tx_queues_to_use, - priv->plat->rx_queues_to_use, - *enable); - -netdev_info (priv->dev,"configured FPE\n" ); +struct stmmac_fpe_cfg *fpe_cfg =from_timer (fpe_cfg, t, verify_timer); + struct stmmac_priv *priv = container_of(fpe_cfg, struct stmmac_priv, + fpe_cfg); +struct ethtool_mm_state *state = &fpe_cfg->state; +unsigned long flags; +bool rearm = false; - *lo_state =FPE_STATE_ON ; - *lp_state =FPE_STATE_ON ; -netdev_info (priv->dev,"!!! BOTH FPE stations ON\n" ); -break ; - } +spin_lock_irqsave (&fpe_cfg->lock, flags); -if ((*lo_state ==FPE_STATE_CAPABLE || - *lo_state ==FPE_STATE_ENTERING_ON ) && - *lp_state !=FPE_STATE_ON ) { -netdev_info (priv->dev,SEND_VERIFY_MPAKCET_FMT , - *lo_state, *lp_state); +switch (state->verify_status) { +case ETHTOOL_MM_VERIFY_STATUS_INITIAL : +case ETHTOOL_MM_VERIFY_STATUS_VERIFYING : +if (fpe_cfg->verify_retries !=0 ) { stmmac_fpe_send_mpacket(priv, priv->ioaddr, - fpe_cfg, -MPACKET_VERIFY );
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c int stmmac_reinit_ringparam(struct net_device *dev, u32 rx_size, u32 tx_size) · line 7369
Quoted 10 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +7
-2+ fpe_cfg, MPACKET_VERIFY); + rearm = true; + }else { + state->verify_status =ETHTOOL_MM_VERIFY_STATUS_FAILED ; } -/* Sleep then retry */ -msleep (500 ); + + fpe_cfg->verify_retries--; +break ;
Odd indentation... "break;" should be on the same level with the code, not with the "case" statements. Not sure which editor you use, but even if you hit "==" in vim on this line, it will shift it by one tab to the right.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c int stmmac_reinit_ringparam(struct net_device *dev, u32 rx_size, u32 tx_size) · line 7376
Quoted 7 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +7
-0+ +case ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED : +stmmac_fpe_configure (priv, priv->ioaddr, fpe_cfg, + priv->plat->tx_queues_to_use, + priv->plat->rx_queues_to_use, + true, true); +break ;
Same comment here and below.
Quoted 13 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10
-1+ +default : +break ; + } + +if (rearm) { +mod_timer (&fpe_cfg->verify_timer, + jiffies +msecs_to_jiffies (state->verify_time)); } -clear_bit (__FPE_TASK_SCHED, &priv->fpe_task_state); +spin_unlock_irqrestore (&fpe_cfg->lock, flags); +}
Re: [PATCH net-next v7 3/7] net: stmmac: refactor FPE verification process
In reply to Furong XuQuoted 7 lines · Furong Xu
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +0
-0@@ -969,17 +969,30 @@ static void stmmac_mac_config(struct phylink_config *config, unsigned int mode,static void stmmac_fpe_link_state_handle (struct stmmac_priv *priv,bool is_up) {
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static void stmmac_mac_config(struct phylink_config *config, unsigned int mode, · line 974
Quoted 10 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +2
-6struct stmmac_fpe_cfg *fpe_cfg = &priv->fpe_cfg; -enum stmmac_fpe_state *lo_state = &fpe_cfg->lo_fpe_state; -enum stmmac_fpe_state *lp_state = &fpe_cfg->lp_fpe_state; -bool *hs_enable = &fpe_cfg->hs_enable; +unsigned long flags; -if (is_up && *hs_enable) { -stmmac_fpe_send_mpacket (priv, priv->ioaddr, fpe_cfg, -MPACKET_VERIFY ); +del_timer_sync (&fpe_cfg->verify_timer);
Interesting comments in include/linux/timer.h:
* Do not use in new code. Use timer_delete_sync() instead.
Also, interesting comment in the timer_delete_sync() kernel-doc:
Callers must prevent restarting of the timer, otherwise this function is meaningless.
I don't think you have any restart prevention mechanism. So between the timer deletion and the spin_lock_irqsave(), another thread has enough time to acquire fpe_cfg->lock first, and run stmmac_fpe_verify_timer_arm().
PATCH v7 4/7 net: stmmac: configure FPE via ethtool-mm
In reply to Furong XuImplement ethtool --show-mm and --set-mm callbacks.
NIC up/down, link up/down, suspend/resume, kselftest-ethtool_mm, all tested okay.
--- .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 4 + drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 14 +++ drivers/net/ethernet/stmicro/stmmac/dwmac5.h | 6 ++ drivers/net/ethernet/stmicro/stmmac/hwif.h | 6 ++ .../ethernet/stmicro/stmmac/stmmac_ethtool.c | 91 +++++++++++++++++++ 5 files changed, 121 insertions(+)
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +4 -0
@@ -1264,6 +1264,8 @@ const struct stmmac_ops dwmac410_ops = { .fpe_configure = dwmac5_fpe_configure, .fpe_send_mpacket = dwmac5_fpe_send_mpacket, .fpe_irq_status = dwmac5_fpe_irq_status, + .fpe_get_add_frag_size = dwmac5_fpe_get_add_frag_size, + .fpe_set_add_frag_size = dwmac5_fpe_set_add_frag_size, .add_hw_vlan_rx_fltr = dwmac4_add_hw_vlan_rx_fltr, .del_hw_vlan_rx_fltr = dwmac4_del_hw_vlan_rx_fltr, .restore_hw_vlan_rx_fltr = dwmac4_restore_hw_vlan_rx_fltr,
@@ -1316,6 +1318,8 @@ const struct stmmac_ops dwmac510_ops = { .fpe_configure = dwmac5_fpe_configure, .fpe_send_mpacket = dwmac5_fpe_send_mpacket, .fpe_irq_status = dwmac5_fpe_irq_status, + .fpe_get_add_frag_size = dwmac5_fpe_get_add_frag_size, + .fpe_set_add_frag_size = dwmac5_fpe_set_add_frag_size, .add_hw_vlan_rx_fltr = dwmac4_add_hw_vlan_rx_fltr, .del_hw_vlan_rx_fltr = dwmac4_del_hw_vlan_rx_fltr, .restore_hw_vlan_rx_fltr = dwmac4_restore_hw_vlan_rx_fltr,
drivers/net/ethernet/stmicro/stmmac/dwmac5.c +14 -0
@@ -653,3 +653,17 @@ void dwmac5_fpe_send_mpacket(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg,writel (value, ioaddr +MAC_FPE_CTRL_STS ); } + +int dwmac5_fpe_get_add_frag_size (const void __iomem *ioaddr) +{ +return FIELD_GET (DWMAC5_ADD_FRAG_SZ ,readl (ioaddr +MTL_FPE_CTRL_STS )); +} + +void dwmac5_fpe_set_add_frag_size (void __iomem *ioaddr,u32 add_frag_size) +{ +u32 value; + + value =readl (ioaddr +MTL_FPE_CTRL_STS ); +writel (u32_replace_bits (value, add_frag_size,DWMAC5_ADD_FRAG_SZ ), + ioaddr +MTL_FPE_CTRL_STS ); +}
drivers/net/ethernet/stmicro/stmmac/dwmac5.h +6 -0
@@ -39,6 +39,10 @@#define MAC_PPSx_INTERVAL (x) (0x00000b88 + ((x) * 0x10))#define MAC_PPSx_WIDTH (x) (0x00000b8c + ((x) * 0x10)) +#define MTL_FPE_CTRL_STS 0x00000c90 +/* Additional Fragment Size of preempted frames */ +#define DWMAC5_ADD_FRAG_SZ GENMASK(1, 0) +#define MTL_RXP_CONTROL_STATUS 0x00000ca0#define RXPI BIT(31)#define NPE GENMASK(23, 16)
@@ -109,5 +113,7 @@ void dwmac5_fpe_send_mpacket(void __iomem *ioaddr,struct stmmac_fpe_cfg *cfg, enum stmmac_mpacket_type type);int dwmac5_fpe_irq_status (void __iomem *ioaddr,struct net_device *dev); +int dwmac5_fpe_get_add_frag_size (const void __iomem *ioaddr); +void dwmac5_fpe_set_add_frag_size (void __iomem *ioaddr,u32 add_frag_size);#endif /* __DWMAC5_H__ */
drivers/net/ethernet/stmicro/stmmac/hwif.h +6 -0
@@ -426,6 +426,8 @@ struct stmmac_ops {struct stmmac_fpe_cfg *cfg, enum stmmac_mpacket_type type);int (*fpe_irq_status)(void __iomem *ioaddr,struct net_device *dev); +int (*fpe_get_add_frag_size)(const void __iomem *ioaddr); +void (*fpe_set_add_frag_size)(void __iomem *ioaddr,u32 add_frag_size); }; #define stmmac_core_init(__priv, __args...) \
@@ -530,6 +532,10 @@ struct stmmac_ops { stmmac_do_void_callback(__priv, mac, fpe_send_mpacket, __args) #define stmmac_fpe_irq_status(__priv, __args...) \ stmmac_do_callback(__priv, mac, fpe_irq_status, __args) +#define stmmac_fpe_get_add_frag_size(__priv, __args...) \ + stmmac_do_callback(__priv, mac, fpe_get_add_frag_size, __args) +#define stmmac_fpe_set_add_frag_size(__priv, __args...) \ + stmmac_do_void_callback(__priv, mac, fpe_set_add_frag_size, __args)/* PTP and HW Timer helpers */ struct stmmac_hwtimestamp {
drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +91 -0
109 lines not on this page · raw message
Signature · 1 line
2.34.1
PATCH v7 5/7 net: stmmac: support fp parameter of tc-mqprio
In reply to Furong Xutc-mqprio can select whether traffic classes are express or preemptible.
After some traffic tests, MAC merge layer statistics are all good.
Local device:
ethtool --include-statistics --json --show-mm eth1
[ {
"ifname": "eth1",
"pmac-enabled": true,
"tx-enabled": true,
"tx-active": true,
"tx-min-frag-size": 60,
"rx-min-frag-size": 60,
"verify-enabled": true,
"verify-time": 100,
"max-verify-time": 128,
"verify-status": "SUCCEEDED",
"statistics": {
"MACMergeFrameAssErrorCount": 0,
"MACMergeFrameSmdErrorCount": 0,
"MACMergeFrameAssOkCount": 0,
"MACMergeFragCountRx": 0,
"MACMergeFragCountTx": 35105,
"MACMergeHoldCount": 0
}
} ]Remote device:
ethtool --include-statistics --json --show-mm end1
[ {
"ifname": "end1",
"pmac-enabled": true,
"tx-enabled": true,
"tx-active": true,
"tx-min-frag-size": 60,
"rx-min-frag-size": 60,
"verify-enabled": true,
"verify-time": 100,
"max-verify-time": 128,
"verify-status": "SUCCEEDED",
"statistics": {
"MACMergeFrameAssErrorCount": 0,
"MACMergeFrameSmdErrorCount": 0,
"MACMergeFrameAssOkCount": 35105,
"MACMergeFragCountRx": 35105,
"MACMergeFragCountTx": 0,
"MACMergeHoldCount": 0
}
} ]Tested on DWMAC CORE 5.10a
--- .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 2 + drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 55 ++++++++++++ drivers/net/ethernet/stmicro/stmmac/dwmac5.h | 4 + drivers/net/ethernet/stmicro/stmmac/hwif.c | 4 +- drivers/net/ethernet/stmicro/stmmac/hwif.h | 11 +++ .../net/ethernet/stmicro/stmmac/stmmac_main.c | 2 + .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 83 +++++++++++++++++++ 7 files changed, 159 insertions(+), 2 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +2 -0
@@ -1266,6 +1266,7 @@ const struct stmmac_ops dwmac410_ops = { .fpe_irq_status = dwmac5_fpe_irq_status, .fpe_get_add_frag_size = dwmac5_fpe_get_add_frag_size, .fpe_set_add_frag_size = dwmac5_fpe_set_add_frag_size, + .fpe_map_preemption_class = dwmac5_fpe_map_preemption_class, .add_hw_vlan_rx_fltr = dwmac4_add_hw_vlan_rx_fltr, .del_hw_vlan_rx_fltr = dwmac4_del_hw_vlan_rx_fltr, .restore_hw_vlan_rx_fltr = dwmac4_restore_hw_vlan_rx_fltr,
@@ -1320,6 +1321,7 @@ const struct stmmac_ops dwmac510_ops = { .fpe_irq_status = dwmac5_fpe_irq_status, .fpe_get_add_frag_size = dwmac5_fpe_get_add_frag_size, .fpe_set_add_frag_size = dwmac5_fpe_set_add_frag_size, + .fpe_map_preemption_class = dwmac5_fpe_map_preemption_class, .add_hw_vlan_rx_fltr = dwmac4_add_hw_vlan_rx_fltr, .del_hw_vlan_rx_fltr = dwmac4_del_hw_vlan_rx_fltr, .restore_hw_vlan_rx_fltr = dwmac4_restore_hw_vlan_rx_fltr,
drivers/net/ethernet/stmicro/stmmac/dwmac5.c +55 -0
58 lines not on this page · raw message
drivers/net/ethernet/stmicro/stmmac/dwmac5.h +4 -0
@@ -40,6 +40,8 @@#define MAC_PPSx_WIDTH (x) (0x00000b8c + ((x) * 0x10))#define MTL_FPE_CTRL_STS 0x00000c90 +/* Preemption Classification */ +#define DWMAC5_PREEMPTION_CLASS GENMASK(15, 8)/* Additional Fragment Size of preempted frames */ #define DWMAC5_ADD_FRAG_SZ GENMASK(1, 0)
@@ -115,5 +117,7 @@ void dwmac5_fpe_send_mpacket(void __iomem *ioaddr,int dwmac5_fpe_irq_status (void __iomem *ioaddr,struct net_device *dev);int dwmac5_fpe_get_add_frag_size (const void __iomem *ioaddr);void dwmac5_fpe_set_add_frag_size (void __iomem *ioaddr,u32 add_frag_size); +int dwmac5_fpe_map_preemption_class (struct net_device *ndev, +struct netlink_ext_ack *extack,u32 pclass);#endif /* __DWMAC5_H__ */
drivers/net/ethernet/stmicro/stmmac/hwif.c +2 -2
@@ -252,7 +252,7 @@ static const struct stmmac_hwif_entry { .mac = &dwxgmac210_ops, .hwtimestamp = &stmmac_ptp, .mode = NULL, - .tc = &dwmac510_tc_ops, + .tc = &dwxgmac_tc_ops, .mmc = &dwxgmac_mmc_ops, .est = &dwmac510_est_ops, .setup = dwxgmac2_setup,
@@ -273,7 +273,7 @@ static const struct stmmac_hwif_entry { .mac = &dwxlgmac2_ops, .hwtimestamp = &stmmac_ptp, .mode = NULL, - .tc = &dwmac510_tc_ops, + .tc = &dwxgmac_tc_ops, .mmc = &dwxgmac_mmc_ops, .est = &dwmac510_est_ops, .setup = dwxlgmac2_setup,
drivers/net/ethernet/stmicro/stmmac/hwif.h +11 -0
@@ -7,6 +7,7 @@#include <linux/netdevice.h> #include <linux/stmmac.h> +#include <net/pkt_cls.h> #define stmmac_do_void_callback(__priv, __module, __cname, __arg0, __args...) \ ({ \
@@ -428,6 +429,9 @@ struct stmmac_ops {int (*fpe_irq_status)(void __iomem *ioaddr,struct net_device *dev);int (*fpe_get_add_frag_size)(const void __iomem *ioaddr);void (*fpe_set_add_frag_size)(void __iomem *ioaddr,u32 add_frag_size); +int (*fpe_map_preemption_class)(struct net_device *ndev, +struct netlink_ext_ack *extack, +u32 pclass); }; #define stmmac_core_init(__priv, __args...) \
@@ -536,6 +540,8 @@ struct stmmac_ops { stmmac_do_callback(__priv, mac, fpe_get_add_frag_size, __args) #define stmmac_fpe_set_add_frag_size(__priv, __args...) \ stmmac_do_void_callback(__priv, mac, fpe_set_add_frag_size, __args) +#define stmmac_fpe_map_preemption_class(__priv, __args...) \ + stmmac_do_void_callback(__priv, mac, fpe_map_preemption_class, __args)/* PTP and HW Timer helpers */ struct stmmac_hwtimestamp {
@@ -623,6 +629,8 @@ struct stmmac_tc_ops { struct tc_etf_qopt_offload *qopt);int (*query_caps)(struct stmmac_priv *priv,struct tc_query_caps_base *base); +int (*setup_mqprio)(struct stmmac_priv *priv, +struct tc_mqprio_qopt_offload *qopt); }; #define stmmac_tc_init(__priv, __args...) \
@@ -639,6 +647,8 @@ struct stmmac_tc_ops { stmmac_do_callback(__priv, tc, setup_etf, __args) #define stmmac_tc_query_caps(__priv, __args...) \ stmmac_do_callback(__priv, tc, query_caps, __args) +#define stmmac_tc_setup_mqprio(__priv, __args...) \ + stmmac_do_callback(__priv, tc, setup_mqprio, __args)struct stmmac_counters ;
@@ -683,6 +693,7 @@ extern const struct stmmac_ops dwmac410_ops;extern const struct stmmac_dma_ops dwmac410_dma_ops;extern const struct stmmac_ops dwmac510_ops;extern const struct stmmac_tc_ops dwmac510_tc_ops; +extern const struct stmmac_tc_ops dwxgmac_tc_ops;extern const struct stmmac_ops dwxgmac210_ops;extern const struct stmmac_ops dwxlgmac2_ops;extern const struct stmmac_dma_ops dwxgmac210_dma_ops;
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +2 -0
@@ -6216,6 +6216,8 @@ static int stmmac_setup_tc(struct net_device *ndev, enum tc_setup_type type,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);case TC_SETUP_BLOCK : return flow_block_cb_setup_simple(type_data, &stmmac_block_cb_list,
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +83 -0
99 lines not on this page · raw message
Signature · 1 line
2.34.1
Re: [PATCH net-next v7 5/7] net: stmmac: support fp parameter of tc-mqprio
Quoted 37 lines · Furong Xu
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +37
-0+static void stmmac_reset_tc_mqprio (struct net_device *ndev, +struct netlink_ext_ack *extack) +{ +struct stmmac_priv *priv =netdev_priv (ndev); + +netdev_reset_tc (ndev); +netif_set_real_num_tx_queues (ndev, priv->plat->tx_queues_to_use); +stmmac_fpe_map_preemption_class (priv, ndev, extack,0 ); +} + +static int tc_setup_dwmac510_mqprio (struct stmmac_priv *priv, +struct tc_mqprio_qopt_offload *mqprio) +{ +struct netlink_ext_ack *extack = mqprio->extack; +struct tc_mqprio_qopt *qopt = &mqprio->qopt; +u32 offset, count, num_stack_tx_queues =0 ; +struct net_device *ndev = priv->dev; +u32 num_tc = qopt->num_tc; +int err; + +if (!num_tc) { +stmmac_reset_tc_mqprio (ndev, extack); +return 0 ; + } + + err =netdev_set_num_tc (ndev, num_tc); +if (err) +return err; + +for (u32 tc =0 ; tc < num_tc; tc++) { + offset = qopt->offset[tc]; + count = qopt->count[tc]; + num_stack_tx_queues += count; + + err =netdev_set_tc_queue (ndev, tc, count, offset); +if (err) + goto err_reset_tc;
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c static int tc_query_caps(struct stmmac_priv *priv, · line 1246
Quoted 10 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +10
-0+ } + + err =netif_set_real_num_tx_queues (ndev, num_stack_tx_queues); +if (err) + goto err_reset_tc; + + err =stmmac_fpe_map_preemption_class (priv, ndev, extack, + mqprio->preemptible_tcs); +if (err) + goto err_reset_tc;
I appreciate the improvement with the separate tc_ops, but I'm still not in love with this.
This stmmac_hw entry (copied with line numbers because it lacks a name by which I can easily reference it):
159 » }, {
160 » » .gmac = false,
161 » » .gmac4 = true,
162 » » .xgmac = false,
163 » » .min_id = 0,
164 » » .regs = {
165 » » » .ptp_off = PTP_GMAC4_OFFSET,
166 » » » .mmc_off = MMC_GMAC4_OFFSET,
167 » » » .est_off = EST_GMAC4_OFFSET,
168 » » },
169 » » .desc = &dwmac4_desc_ops,
170 » » .dma = &dwmac4_dma_ops,
171 » » .mac = &dwmac4_ops,
172 » » .hwtimestamp = &stmmac_ptp,
173 » » .mode = NULL,
174 » » .tc = &dwmac510_tc_ops,
175 » » .mmc = &dwmac_mmc_ops,
176 » » .est = &dwmac510_est_ops,
177 » » .setup = dwmac4_setup,
178 » » .quirks = stmmac_dwmac4_quirks,
179 » }, {has .mac = &dwmac4_ops, so it does not implement .fpe_map_preemption_class(). But it also has .tc = &dwmac510_tc_ops, so tc_setup_dwmac510_mqprio() will get called.
Thus, I suppose that the stmmac_fpe_map_preemption_class() -> stmmac_do_void_callback() mechanism will return -EINVAL for dwmac4_ops, and this will make mqprio offload fail, for the sole reason that FPE is not supported, _EVEN IF_ FPE was never requested (mqprio->preemptible_tcs is 0), and the offload could have otherwise been applied just fine.
Not to mention my previous complaint still applies, that the test for the presence of stmmac_ops :: fpe_map_preemption_class() is unnaturally late relative to the flow of the tc_setup_dwmac510_mqprio() function.
Thus, I really recommend you to replace the stmmac_do_void_callback() anti-pattern with something like:
// early
if (mqprio->preemptible_tcs && !priv->hw->ops->fpe_map_preemption_class) {
NL_SET_ERR_MSG_MOD(mqprio->extack,
"Cannot map preemptible TCs to TXQs");
return -EOPNOTSUPP;
}
// late
if (priv->hw->ops->fpe_map_preemption_class) {
err = priv->hw->ops->fpe_map_preemption_class(priv->dev,
mqprio->preemptible_tcs,
extack);
if (err)
goto err_reset_tc;
}WARNING: code not tested. The idea is that the early check makes sure that only dwmac410_ops and dwmac510_ops permit mqprio->preemptible_tcs to go to a non-zero value (which can later be reset to zero if desired, in the late code path). For dwmac4_ops, mqprio->preemptible_tcs = 0 is the only supported value (for which nothing needs to be done), and the late code path is bypassed, due to the "if" condition returning false.
Either organize like this, or if you really, really, really insist to use the stmmac_do_callback() anti-pattern in new code, at least don't share the setup_mqprio() code path between MACs that implement fpe_map_preemption_class() and MACs that don't.
PATCH v7 6/7 net: stmmac: support fp parameter of tc-taprio
In reply to Furong Xutc-taprio can select whether traffic classes are express or preemptible.
0) tc qdisc add dev eth1 parent root handle 100 taprio \
num_tc 4 \
map 0 1 2 3 2 2 2 2 2 2 2 2 2 2 2 3 \
queues 1@0 1@1 1@2 1@3 \
base-time 1000000000 \
sched-entry S 03 10000000 \
sched-entry S 0e 10000000 \
flags 0x2 fp P E E E1) After some traffic tests, MAC merge layer statistics are all good.
Local device:
[ {
"ifname": "eth1",
"pmac-enabled": true,
"tx-enabled": true,
"tx-active": true,
"tx-min-frag-size": 60,
"rx-min-frag-size": 60,
"verify-enabled": true,
"verify-time": 100,
"max-verify-time": 128,
"verify-status": "SUCCEEDED",
"statistics": {
"MACMergeFrameAssErrorCount": 0,
"MACMergeFrameSmdErrorCount": 0,
"MACMergeFrameAssOkCount": 0,
"MACMergeFragCountRx": 0,
"MACMergeFragCountTx": 17837,
"MACMergeHoldCount": 18639
}
} ]Remote device:
[ {
"ifname": "end1",
"pmac-enabled": true,
"tx-enabled": true,
"tx-active": true,
"tx-min-frag-size": 60,
"rx-min-frag-size": 60,
"verify-enabled": true,
"verify-time": 100,
"max-verify-time": 128,
"verify-status": "SUCCEEDED",
"statistics": {
"MACMergeFrameAssErrorCount": 0,
"MACMergeFrameSmdErrorCount": 0,
"MACMergeFrameAssOkCount": 17189,
"MACMergeFragCountRx": 17837,
"MACMergeFragCountTx": 0,
"MACMergeHoldCount": 0
}
} ]Tested on DWMAC CORE 5.10a
--- .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +8 -15
@@ -931,9 +931,9 @@ static int tc_taprio_configure(struct stmmac_priv *priv, struct tc_taprio_qopt_offload *qopt) {u32 size, wid = priv->dma_cap.estwid, dep = priv->dma_cap.estdep; +struct netlink_ext_ack *extack = qopt->mqprio.extack;struct timespec64 time, current_time, qopt_time;ktime_t current_time_ns; -bool fpe = false;int i, ret =0 ;u64 ctr;
@@ -1018,16 +1018,12 @@ static int tc_taprio_configure(struct stmmac_priv *priv,switch (qopt->entries[i].command) {case TC_TAPRIO_CMD_SET_GATES : -if (fpe) -return -EINVAL ;break ;case TC_TAPRIO_CMD_SET_AND_HOLD : gates |=BIT (0 ); - fpe = true;break ;case TC_TAPRIO_CMD_SET_AND_RELEASE : gates &= ~BIT (0 ); - fpe = true;break ;default :return -EOPNOTSUPP ;
@@ -1058,11 +1054,6 @@ static int tc_taprio_configure(struct stmmac_priv *priv,tc_taprio_map_maxsdu_txq (priv, qopt); -if (fpe && !priv->dma_cap.fpesel) { -mutex_unlock (&priv->est_lock); -return -EOPNOTSUPP ; - } - ret =stmmac_est_configure (priv, priv, priv->est, priv->plat->clk_ptp_rate);mutex_unlock (&priv->est_lock);
@@ -1071,6 +1062,11 @@ static int tc_taprio_configure(struct stmmac_priv *priv, goto disable; } + ret =stmmac_fpe_map_preemption_class (priv, priv->dev, extack, + qopt->mqprio.preemptible_tcs); +if (ret) + goto disable; +netdev_info (priv->dev,"configured EST\n" );return 0 ;
@@ -1089,11 +1085,8 @@ static int tc_taprio_configure(struct stmmac_priv *priv,mutex_unlock (&priv->est_lock); } -stmmac_fpe_configure (priv, priv->ioaddr, - &priv->fpe_cfg, - priv->plat->tx_queues_to_use, - priv->plat->rx_queues_to_use, - false, false); +stmmac_fpe_map_preemption_class (priv, priv->dev, extack,0 ); +netdev_info (priv->dev,"disabled FPE\n" );return ret;
Signature · 1 line
2.34.1
Re: [PATCH net-next v7 6/7] net: stmmac: support fp parameter of tc-taprio
Quoted 48 lines · Furong Xu
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-)drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +1
-10@@ -931,9 +931,9 @@ static int tc_taprio_configure(struct stmmac_priv *priv, struct tc_taprio_qopt_offload *qopt) {u32 size, wid = priv->dma_cap.estwid, dep = priv->dma_cap.estdep; +struct netlink_ext_ack *extack = qopt->mqprio.extack;struct timespec64 time, current_time, qopt_time;ktime_t current_time_ns; -bool fpe = false;int i, ret =0 ;u64 ctr;@@ -1018,16 +1018,12 @@ static int tc_taprio_configure(struct stmmac_priv *priv,switch (qopt->entries[i].command) {case TC_TAPRIO_CMD_SET_GATES : -if (fpe) -return -EINVAL ;break ;case TC_TAPRIO_CMD_SET_AND_HOLD : gates |=BIT (0 ); - fpe = true;break ;case TC_TAPRIO_CMD_SET_AND_RELEASE : gates &= ~BIT (0 ); - fpe = true;break ;default :return -EOPNOTSUPP ;@@ -1058,11 +1054,6 @@ static int tc_taprio_configure(struct stmmac_priv *priv,tc_taprio_map_maxsdu_txq (priv, qopt); -if (fpe && !priv->dma_cap.fpesel) { -mutex_unlock (&priv->est_lock); -return -EOPNOTSUPP ; - } - ret =stmmac_est_configure (priv, priv, priv->est, priv->plat->clk_ptp_rate);mutex_unlock (&priv->est_lock);
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c static int tc_taprio_configure(struct stmmac_priv *priv, · line 1069
Quoted 8 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +5
-0@@ -1071,6 +1062,11 @@ static int tc_taprio_configure(struct stmmac_priv *priv, goto disable; } + ret =stmmac_fpe_map_preemption_class (priv, priv->dev, extack, + qopt->mqprio.preemptible_tcs); +if (ret) + goto disable; +
Doesn't this break taprio for those callers of tc_setup_taprio() which do not implement fpe_map_preemption_class(), but at least want taprio without FPE nonetheless? As in the earlier mqprio patch, they will return -EINVAL here.
Through code inspection, those users are:
- All users of .tc = dwxgmac_tc_ops: they have .mac = &dwxlgmac2_ops or .mac = &dwxgmac210_ops, neither of which implements fpe_map_preemption_class().
- The users of .tc = &dwmac510_tc_ops which have .mac = &dwmac4_ops. Again, this does not implement .fpe_map_preemption_class().
I can only rely on code inspection, because I don't know what is the priv->dma_cap.estsel value that the above cores were synthesized with. Thus, I don't know if we break a feature which was available before, or one that was already unavailable.
My suggestion is similar: allow mqprio.preemptible_tcs to go to non-zero only if fpe_map_preemption_class() is implemented. Then, program it with any value (including 0) only if fpe_map_preemption_class() is implemented.
Quoted 3 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +0
-0netdev_info (priv->dev,"configured EST\n" );return 0 ;
PATCH v7 7/7 net: stmmac: silence FPE kernel logs
In reply to Furong Xuethtool --show-mm can get real-time state of FPE.
Those kernel logs should keep quiet.
--- drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac5.c +4 -4
@@ -620,22 +620,22 @@ int dwmac5_fpe_irq_status(void __iomem *ioaddr, struct net_device *dev)if (value &TRSP ) { status |=FPE_EVENT_TRSP ; -netdev_info (dev,"FPE: Respond mPacket is transmitted\n" ); +netdev_dbg (dev,"FPE: Respond mPacket is transmitted\n" ); }if (value &TVER ) { status |=FPE_EVENT_TVER ; -netdev_info (dev,"FPE: Verify mPacket is transmitted\n" ); +netdev_dbg (dev,"FPE: Verify mPacket is transmitted\n" ); }if (value &RRSP ) { status |=FPE_EVENT_RRSP ; -netdev_info (dev,"FPE: Respond mPacket is received\n" ); +netdev_dbg (dev,"FPE: Respond mPacket is received\n" ); }if (value &RVER ) { status |=FPE_EVENT_RVER ; -netdev_info (dev,"FPE: Verify mPacket is received\n" ); +netdev_dbg (dev,"FPE: Verify mPacket is received\n" ); }return status;
Signature · 1 line
2.34.1
Re: [PATCH net-next v7 7/7] net: stmmac: silence FPE kernel logs
Quoted 5 lines · Furong Xu
ethtool --show-mm can get real-time state of FPE.
Those kernel logs should keep quiet.Signed-off-by Furong Xu <[email protected]>---
I don't have a stmmac-based setup to judge in person. But to me, these still look too chatty, for being things that user space can always query through netlink.
1070 » netdev_info(priv->dev, "configured EST\n");
1090 » netdev_info(priv->dev, "disabled FPE\n");
Also, they don't seem to be balanced. We don't have "disabled EST" and "enabled FPE"?! I wonder if "disabled FPE" is actually a typo and should have been "disabled EST"?
What do you think, should these also be suppressed / deleted?