* [PATCH net-next v3 1/7] net: stmmac: move stmmac_fpe_cfg to stmmac_priv data
2024-08-19 7:25 [PATCH net-next v3 0/7] net: stmmac: FPE via ethtool + tc Furong Xu
@ 2024-08-19 7:25 ` Furong Xu
2024-08-19 12:38 ` Serge Semin
2024-08-19 7:25 ` [PATCH net-next v3 2/7] net: stmmac: drop stmmac_fpe_handshake Furong Xu
` (5 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Furong Xu @ 2024-08-19 7:25 UTC (permalink / raw)
To: Serge Semin, Andrew Lunn, Vladimir Oltean, David S. Miller,
Alexandre Torgue, Jose Abreu, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Maxime Coquelin, Joao Pinto
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, xfr,
Furong Xu
By moving the fpe_cfg field to the stmmac_priv data, stmmac_fpe_cfg
becomes platform-data eventually, instead of a run-time config.
Suggested-by: Serge Semin <fancer.lancer@gmail.com>
Signed-off-by: Furong Xu <0x1207@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/hwif.h | 2 ++
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 29 +++++++++++++++++++
.../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(+), 51 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 7e90f34b8c88..28dfc0054a3a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -31,6 +31,8 @@ struct stmmac_safety_stats;
struct dma_desc;
struct dma_extended_desc;
struct dma_edesc;
+struct stmmac_fpe_cfg;
+enum stmmac_mpacket_type;
/* Descriptors helpers */
struct stmmac_desc_ops {
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index b23b920eedb1..2c2181febb39 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -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,6 +366,8 @@ struct stmmac_priv {
struct workqueue_struct *wq;
struct work_struct service_task;
+ struct stmmac_fpe_cfg fpe_cfg;
+
/* Workqueue for handling FPE hand-shaking */
unsigned long fpe_task_state;
struct workqueue_struct *fpe_wq;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d9fca8d1227c..529fe31f8b04 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -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);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 996f2bcd07a2..9cc41ed01882 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -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);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 338991c08f00..d79ff252cfdc 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -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;
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next v3 1/7] net: stmmac: move stmmac_fpe_cfg to stmmac_priv data
2024-08-19 7:25 ` [PATCH net-next v3 1/7] net: stmmac: move stmmac_fpe_cfg to stmmac_priv data Furong Xu
@ 2024-08-19 12:38 ` Serge Semin
0 siblings, 0 replies; 12+ messages in thread
From: Serge Semin @ 2024-08-19 12:38 UTC (permalink / raw)
To: Furong Xu
Cc: Andrew Lunn, Vladimir Oltean, David S. Miller, Alexandre Torgue,
Jose Abreu, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maxime Coquelin, Joao Pinto, netdev, linux-stm32,
linux-arm-kernel, linux-kernel, xfr
On Mon, Aug 19, 2024 at 03:25:14PM +0800, Furong Xu wrote:
> By moving the fpe_cfg field to the stmmac_priv data, stmmac_fpe_cfg
> becomes platform-data eventually, instead of a run-time config.
>
> Suggested-by: Serge Semin <fancer.lancer@gmail.com>
> Signed-off-by: Furong Xu <0x1207@gmail.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/hwif.h | 2 ++
> drivers/net/ethernet/stmicro/stmmac/stmmac.h | 29 +++++++++++++++++++
> .../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(+), 51 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index 7e90f34b8c88..28dfc0054a3a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> @@ -31,6 +31,8 @@ struct stmmac_safety_stats;
> struct dma_desc;
> struct dma_extended_desc;
> struct dma_edesc;
> +struct stmmac_fpe_cfg;
> +enum stmmac_mpacket_type;
Please move these being declared above struct dma_desc, so to group up
the stmmac_-prefixed entities declarations.
>
> /* Descriptors helpers */
> struct stmmac_desc_ops {
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index b23b920eedb1..2c2181febb39 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -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,6 +366,8 @@ struct stmmac_priv {
> struct workqueue_struct *wq;
> struct work_struct service_task;
>
> + struct stmmac_fpe_cfg fpe_cfg;
> +
Could you please move this field to being joined with the
FPE Workqueue-related stuff in the structure? Like this:
- /* 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;
So the FPE-related fields would be grouped together as it's done for
the TC, PPS, RSS, XDP features.
Other than that the change looks good.
Thanks
-Serge(y)
> /* Workqueue for handling FPE hand-shaking */
> unsigned long fpe_task_state;
> struct workqueue_struct *fpe_wq;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index d9fca8d1227c..529fe31f8b04 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -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);
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index 996f2bcd07a2..9cc41ed01882 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> @@ -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);
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 338991c08f00..d79ff252cfdc 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -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;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next v3 2/7] net: stmmac: drop stmmac_fpe_handshake
2024-08-19 7:25 [PATCH net-next v3 0/7] net: stmmac: FPE via ethtool + tc Furong Xu
2024-08-19 7:25 ` [PATCH net-next v3 1/7] net: stmmac: move stmmac_fpe_cfg to stmmac_priv data Furong Xu
@ 2024-08-19 7:25 ` Furong Xu
2024-08-19 7:25 ` [PATCH net-next v3 3/7] net: stmmac: refactor FPE verification processe Furong Xu
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Furong Xu @ 2024-08-19 7:25 UTC (permalink / raw)
To: Serge Semin, Andrew Lunn, Vladimir Oltean, David S. Miller,
Alexandre Torgue, Jose Abreu, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Maxime Coquelin, Joao Pinto
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, xfr,
Furong Xu
ethtool --set-mm can trigger FPE verification processe by calling
stmmac_fpe_send_mpacket, stmmac_fpe_handshake should be gone.
Signed-off-by: Furong Xu <0x1207@gmail.com>
---
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 23 +------------------
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 8 -------
2 files changed, 1 insertion(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 529fe31f8b04..3072ad33b105 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -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);
}
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 9cc41ed01882..b0cc45331ff7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -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;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next v3 3/7] net: stmmac: refactor FPE verification processe
2024-08-19 7:25 [PATCH net-next v3 0/7] net: stmmac: FPE via ethtool + tc Furong Xu
2024-08-19 7:25 ` [PATCH net-next v3 1/7] net: stmmac: move stmmac_fpe_cfg to stmmac_priv data Furong Xu
2024-08-19 7:25 ` [PATCH net-next v3 2/7] net: stmmac: drop stmmac_fpe_handshake Furong Xu
@ 2024-08-19 7:25 ` Furong Xu
2024-08-19 11:31 ` Vladimir Oltean
2024-08-19 7:25 ` [PATCH net-next v3 4/7] net: stmmac: configure FPE via ethtool-mm Furong Xu
` (3 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Furong Xu @ 2024-08-19 7:25 UTC (permalink / raw)
To: Serge Semin, Andrew Lunn, Vladimir Oltean, David S. Miller,
Alexandre Torgue, Jose Abreu, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Maxime Coquelin, Joao Pinto
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, xfr,
Furong Xu
Drop 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, workqueue, link update
and register configuration.
Signed-off-by: Furong Xu <0x1207@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 20 +--
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 169 +++++++++---------
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 6 -
3 files changed, 97 insertions(+), 98 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 2c2181febb39..cb54f65753b2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -146,14 +146,6 @@ 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,
@@ -166,10 +158,10 @@ enum stmmac_fpe_task_state_t {
};
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 */
+ bool pmac_enabled; /* see ethtool_mm_state */
+ bool verify_enabled; /* see ethtool_mm_state */
+ u32 verify_time; /* see ethtool_mm_state */
+ enum ethtool_mm_verify_status status;
u32 fpe_csr; /* MAC_FPE_CTRL_STS reg cache */
};
@@ -366,6 +358,10 @@ struct stmmac_priv {
struct workqueue_struct *wq;
struct work_struct service_task;
+ /* Serialize access to MAC Merge state between ethtool requests
+ * and link state updates.
+ */
+ spinlock_t mm_lock;
struct stmmac_fpe_cfg fpe_cfg;
/* Workqueue for handling FPE hand-shaking */
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3072ad33b105..628354f60c54 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -969,17 +969,21 @@ 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->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;
+
+ spin_lock_irqsave(&priv->mm_lock, flags);
- if (is_up && *hs_enable) {
+ if (!fpe_cfg->pmac_enabled)
+ goto __unlock_out;
+
+ if (is_up && fpe_cfg->verify_enabled)
stmmac_fpe_send_mpacket(priv, priv->ioaddr, fpe_cfg,
MPACKET_VERIFY);
- } else {
- *lo_state = FPE_STATE_OFF;
- *lp_state = FPE_STATE_OFF;
- }
+ else
+ fpe_cfg->status = ETHTOOL_MM_VERIFY_STATUS_DISABLED;
+
+__unlock_out:
+ spin_unlock_irqrestore(&priv->mm_lock, flags);
}
static void stmmac_mac_link_down(struct phylink_config *config,
@@ -3533,9 +3537,19 @@ 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);
+ /* phylink and irq are not enabled yet,
+ * mm_lock is unnecessary here.
+ */
+ stmmac_fpe_configure(priv, priv->ioaddr,
+ &priv->fpe_cfg,
+ priv->plat->tx_queues_to_use,
+ priv->plat->rx_queues_to_use,
+ false);
+ }
+
return 0;
}
@@ -3978,6 +3992,12 @@ static int __stmmac_open(struct net_device *dev,
}
}
+ /* phylink and irq are not enabled yet, mm_lock is unnecessary here */
+ priv->fpe_cfg.pmac_enabled = false;
+ priv->fpe_cfg.verify_time = 128; /* ethtool_mm_state.max_verify_time */
+ priv->fpe_cfg.verify_enabled = false;
+ priv->fpe_cfg.status = ETHTOOL_MM_VERIFY_STATUS_DISABLED;
+
ret = stmmac_hw_setup(dev, true);
if (ret < 0) {
netdev_err(priv->dev, "%s: Hw setup failed\n", __func__);
@@ -4091,11 +4111,19 @@ static int stmmac_release(struct net_device *dev)
stmmac_release_ptp(priv);
- pm_runtime_put(priv->device);
-
- if (priv->dma_cap.fpesel)
+ if (priv->dma_cap.fpesel) {
stmmac_fpe_stop_wq(priv);
+ /* phylink and irq have already disabled,
+ * mm_lock is unnecessary here.
+ */
+ priv->fpe_cfg.pmac_enabled = false;
+ priv->fpe_cfg.verify_enabled = false;
+ priv->fpe_cfg.status = ETHTOOL_MM_VERIFY_STATUS_DISABLED;
+ }
+
+ pm_runtime_put(priv->device);
+
return 0;
}
@@ -5979,44 +6007,34 @@ 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->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;
- if (status == FPE_EVENT_UNKNOWN || !*hs_enable)
- return;
+ spin_lock(&priv->mm_lock);
- /* If LP has sent verify mPacket, LP is FPE capable */
- if ((status & FPE_EVENT_RVER) == FPE_EVENT_RVER) {
- if (*lp_state < FPE_STATE_CAPABLE)
- *lp_state = FPE_STATE_CAPABLE;
+ if (!fpe_cfg->pmac_enabled || status == FPE_EVENT_UNKNOWN)
+ goto __unlock_out;
- /* If user has requested FPE enable, quickly response */
- if (*hs_enable)
- stmmac_fpe_send_mpacket(priv, priv->ioaddr,
- fpe_cfg,
- MPACKET_RESPONSE);
- }
+ /* LP has sent verify mPacket */
+ if ((status & FPE_EVENT_RVER) == FPE_EVENT_RVER)
+ stmmac_fpe_send_mpacket(priv, priv->ioaddr, fpe_cfg,
+ MPACKET_RESPONSE);
- /* If Local has sent verify mPacket, Local is FPE capable */
- if ((status & FPE_EVENT_TVER) == FPE_EVENT_TVER) {
- if (*lo_state < FPE_STATE_CAPABLE)
- *lo_state = FPE_STATE_CAPABLE;
- }
+ /* Local has sent verify mPacket */
+ if ((status & FPE_EVENT_TVER) == FPE_EVENT_TVER &&
+ fpe_cfg->status != ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED)
+ fpe_cfg->status = ETHTOOL_MM_VERIFY_STATUS_VERIFYING;
- /* If LP has sent response mPacket, LP is entering FPE ON */
+ /* LP has sent response mPacket */
if ((status & FPE_EVENT_RRSP) == FPE_EVENT_RRSP)
- *lp_state = FPE_STATE_ENTERING_ON;
-
- /* If Local has sent response mPacket, Local is entering FPE ON */
- if ((status & FPE_EVENT_TRSP) == FPE_EVENT_TRSP)
- *lo_state = FPE_STATE_ENTERING_ON;
+ fpe_cfg->status = ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED;
if (!test_bit(__FPE_REMOVING, &priv->fpe_task_state) &&
!test_and_set_bit(__FPE_TASK_SCHED, &priv->fpe_task_state) &&
priv->fpe_wq) {
queue_work(priv->fpe_wq, &priv->fpe_task);
}
+
+__unlock_out:
+ spin_unlock(&priv->mm_lock);
}
static void stmmac_common_interrupt(struct stmmac_priv *priv)
@@ -7372,50 +7390,47 @@ int stmmac_reinit_ringparam(struct net_device *dev, u32 rx_size, u32 tx_size)
return ret;
}
-#define SEND_VERIFY_MPAKCET_FMT "Send Verify mPacket lo_state=%d lp_state=%d\n"
-static void stmmac_fpe_lp_task(struct work_struct *work)
+static void stmmac_fpe_verify_task(struct work_struct *work)
{
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)
+ int verify_limit = 3; /* defined by 802.3 */
+ unsigned long flags;
+
+ while (1) {
+ msleep(fpe_cfg->verify_time);
+
+ if (!netif_running(priv->dev))
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);
+ spin_lock_irqsave(&priv->mm_lock, flags);
- netdev_info(priv->dev, "configured FPE\n");
+ if (fpe_cfg->status == ETHTOOL_MM_VERIFY_STATUS_DISABLED ||
+ fpe_cfg->status == ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED ||
+ !fpe_cfg->pmac_enabled || !fpe_cfg->verify_enabled) {
+ spin_unlock_irqrestore(&priv->mm_lock, flags);
+ break;
+ }
- *lo_state = FPE_STATE_ON;
- *lp_state = FPE_STATE_ON;
- netdev_info(priv->dev, "!!! BOTH FPE stations ON\n");
+ if (verify_limit == 0) {
+ fpe_cfg->verify_enabled = false;
+ fpe_cfg->status = ETHTOOL_MM_VERIFY_STATUS_FAILED;
+ stmmac_fpe_configure(priv, priv->ioaddr, fpe_cfg,
+ priv->plat->tx_queues_to_use,
+ priv->plat->rx_queues_to_use,
+ false);
+ spin_unlock_irqrestore(&priv->mm_lock, flags);
break;
}
- 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);
- stmmac_fpe_send_mpacket(priv, priv->ioaddr,
- fpe_cfg,
+ if (fpe_cfg->status == ETHTOOL_MM_VERIFY_STATUS_VERIFYING)
+ stmmac_fpe_send_mpacket(priv, priv->ioaddr, fpe_cfg,
MPACKET_VERIFY);
- }
- /* Sleep then retry */
- msleep(500);
+
+ spin_unlock_irqrestore(&priv->mm_lock, flags);
+
+ verify_limit--;
}
clear_bit(__FPE_TASK_SCHED, &priv->fpe_task_state);
@@ -7535,8 +7550,8 @@ int stmmac_dvr_probe(struct device *device,
INIT_WORK(&priv->service_task, stmmac_service_task);
- /* Initialize Link Partner FPE workqueue */
- INIT_WORK(&priv->fpe_task, stmmac_fpe_lp_task);
+ /* Initialize FPE verify workqueue */
+ INIT_WORK(&priv->fpe_task, stmmac_fpe_verify_task);
/* Override with kernel parameters if supplied XXX CRS XXX
* this needs to have multiple instances
@@ -7701,6 +7716,7 @@ int stmmac_dvr_probe(struct device *device,
stmmac_napi_add(ndev);
mutex_init(&priv->lock);
+ spin_lock_init(&priv->mm_lock);
/* If a specific clk_csr value is passed from the platform
* this means that the CSR Clock Range selection cannot be
@@ -7875,15 +7891,8 @@ int stmmac_suspend(struct device *dev)
}
rtnl_unlock();
- if (priv->dma_cap.fpesel) {
- /* Disable FPE */
- stmmac_fpe_configure(priv, priv->ioaddr,
- &priv->fpe_cfg,
- priv->plat->tx_queues_to_use,
- priv->plat->rx_queues_to_use, false);
-
+ if (priv->dma_cap.fpesel)
stmmac_fpe_stop_wq(priv);
- }
priv->speed = SPEED_UNKNOWN;
return 0;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index b0cc45331ff7..783829a6479c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -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,7 +1089,6 @@ 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,
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next v3 3/7] net: stmmac: refactor FPE verification processe
2024-08-19 7:25 ` [PATCH net-next v3 3/7] net: stmmac: refactor FPE verification processe Furong Xu
@ 2024-08-19 11:31 ` Vladimir Oltean
0 siblings, 0 replies; 12+ messages in thread
From: Vladimir Oltean @ 2024-08-19 11:31 UTC (permalink / raw)
To: Furong Xu
Cc: Serge Semin, Andrew Lunn, David S. Miller, Alexandre Torgue,
Jose Abreu, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maxime Coquelin, Joao Pinto, netdev, linux-stm32,
linux-arm-kernel, linux-kernel, xfr
Interesting spelling of "process" in the title.
On Mon, Aug 19, 2024 at 03:25:16PM +0800, Furong Xu wrote:
> Drop 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, workqueue, link update
> and register configuration.
>
> Signed-off-by: Furong Xu <0x1207@gmail.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac.h | 20 +--
> .../net/ethernet/stmicro/stmmac/stmmac_main.c | 169 +++++++++---------
> .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 6 -
> 3 files changed, 97 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index 2c2181febb39..cb54f65753b2 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -146,14 +146,6 @@ 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,
> @@ -166,10 +158,10 @@ enum stmmac_fpe_task_state_t {
> };
>
> 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 */
> + bool pmac_enabled; /* see ethtool_mm_state */
> + bool verify_enabled; /* see ethtool_mm_state */
> + u32 verify_time; /* see ethtool_mm_state */
> + enum ethtool_mm_verify_status status;
> u32 fpe_csr; /* MAC_FPE_CTRL_STS reg cache */
> };
>
> @@ -366,6 +358,10 @@ struct stmmac_priv {
> struct workqueue_struct *wq;
> struct work_struct service_task;
>
> + /* Serialize access to MAC Merge state between ethtool requests
> + * and link state updates.
> + */
> + spinlock_t mm_lock;
Given that it protects members of struct stmmac_fpe_cfg, would it make
sense for it to be placed in that structure instead? fpe_cfg->lock.
> struct stmmac_fpe_cfg fpe_cfg;
>
> /* Workqueue for handling FPE hand-shaking */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 3072ad33b105..628354f60c54 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -969,17 +969,21 @@ 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->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;
> +
> + spin_lock_irqsave(&priv->mm_lock, flags);
>
> - if (is_up && *hs_enable) {
> + if (!fpe_cfg->pmac_enabled)
> + goto __unlock_out;
> +
> + if (is_up && fpe_cfg->verify_enabled)
> stmmac_fpe_send_mpacket(priv, priv->ioaddr, fpe_cfg,
> MPACKET_VERIFY);
> - } else {
> - *lo_state = FPE_STATE_OFF;
> - *lp_state = FPE_STATE_OFF;
> - }
> + else
> + fpe_cfg->status = ETHTOOL_MM_VERIFY_STATUS_DISABLED;
> +
> +__unlock_out:
> + spin_unlock_irqrestore(&priv->mm_lock, flags);
> }
>
> static void stmmac_mac_link_down(struct phylink_config *config,
> @@ -3533,9 +3537,19 @@ 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);
>
> + /* phylink and irq are not enabled yet,
> + * mm_lock is unnecessary here.
> + */
> + stmmac_fpe_configure(priv, priv->ioaddr,
> + &priv->fpe_cfg,
> + priv->plat->tx_queues_to_use,
> + priv->plat->rx_queues_to_use,
> + false);
This is probably unintended, but &priv->fpe_cfg has just been zeroed out
earlier by __stmmac_open().
> + }
> +
> return 0;
> }
>
> @@ -3978,6 +3992,12 @@ static int __stmmac_open(struct net_device *dev,
> }
> }
>
> + /* phylink and irq are not enabled yet, mm_lock is unnecessary here */
> + priv->fpe_cfg.pmac_enabled = false;
> + priv->fpe_cfg.verify_time = 128; /* ethtool_mm_state.max_verify_time */
> + priv->fpe_cfg.verify_enabled = false;
> + priv->fpe_cfg.status = ETHTOOL_MM_VERIFY_STATUS_DISABLED;
> +
stmmac_set_mm() can be called before __stmmac_open(), which is entirely
legal. You'd be overwriting the configuration made by the user in that
case. Same is true for the snippet below from stmmac_release().
Personally I think work items should be put on the fpe_wq only when
netif_running() and during __stmmac_open(), but configuration changes
should also be accepted while down. Maybe this also implies that during
stmmac_get_mm() and stmmac_set_mm() it must temporarily use
pm_runtime_resume_and_get() and pm_runtime_put(), and get whatever
clocks are necessary for the registers to be accessible.
> ret = stmmac_hw_setup(dev, true);
> if (ret < 0) {
> netdev_err(priv->dev, "%s: Hw setup failed\n", __func__);
> @@ -4091,11 +4111,19 @@ static int stmmac_release(struct net_device *dev)
>
> stmmac_release_ptp(priv);
>
> - pm_runtime_put(priv->device);
> -
> - if (priv->dma_cap.fpesel)
> + if (priv->dma_cap.fpesel) {
> stmmac_fpe_stop_wq(priv);
>
> + /* phylink and irq have already disabled,
> + * mm_lock is unnecessary here.
> + */
> + priv->fpe_cfg.pmac_enabled = false;
> + priv->fpe_cfg.verify_enabled = false;
> + priv->fpe_cfg.status = ETHTOOL_MM_VERIFY_STATUS_DISABLED;
> + }
> +
> + pm_runtime_put(priv->device);
> +
> return 0;
> }
>
> @@ -5979,44 +6007,34 @@ 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->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;
>
> - if (status == FPE_EVENT_UNKNOWN || !*hs_enable)
> - return;
> + spin_lock(&priv->mm_lock);
>
> - /* If LP has sent verify mPacket, LP is FPE capable */
> - if ((status & FPE_EVENT_RVER) == FPE_EVENT_RVER) {
> - if (*lp_state < FPE_STATE_CAPABLE)
> - *lp_state = FPE_STATE_CAPABLE;
> + if (!fpe_cfg->pmac_enabled || status == FPE_EVENT_UNKNOWN)
> + goto __unlock_out;
>
> - /* If user has requested FPE enable, quickly response */
> - if (*hs_enable)
> - stmmac_fpe_send_mpacket(priv, priv->ioaddr,
> - fpe_cfg,
> - MPACKET_RESPONSE);
> - }
> + /* LP has sent verify mPacket */
> + if ((status & FPE_EVENT_RVER) == FPE_EVENT_RVER)
> + stmmac_fpe_send_mpacket(priv, priv->ioaddr, fpe_cfg,
> + MPACKET_RESPONSE);
>
> - /* If Local has sent verify mPacket, Local is FPE capable */
> - if ((status & FPE_EVENT_TVER) == FPE_EVENT_TVER) {
> - if (*lo_state < FPE_STATE_CAPABLE)
> - *lo_state = FPE_STATE_CAPABLE;
> - }
> + /* Local has sent verify mPacket */
> + if ((status & FPE_EVENT_TVER) == FPE_EVENT_TVER &&
> + fpe_cfg->status != ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED)
> + fpe_cfg->status = ETHTOOL_MM_VERIFY_STATUS_VERIFYING;
>
> - /* If LP has sent response mPacket, LP is entering FPE ON */
> + /* LP has sent response mPacket */
> if ((status & FPE_EVENT_RRSP) == FPE_EVENT_RRSP)
> - *lp_state = FPE_STATE_ENTERING_ON;
> -
> - /* If Local has sent response mPacket, Local is entering FPE ON */
> - if ((status & FPE_EVENT_TRSP) == FPE_EVENT_TRSP)
> - *lo_state = FPE_STATE_ENTERING_ON;
> + fpe_cfg->status = ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED;
>
> if (!test_bit(__FPE_REMOVING, &priv->fpe_task_state) &&
> !test_and_set_bit(__FPE_TASK_SCHED, &priv->fpe_task_state) &&
> priv->fpe_wq) {
> queue_work(priv->fpe_wq, &priv->fpe_task);
> }
> +
> +__unlock_out:
> + spin_unlock(&priv->mm_lock);
> }
>
> static void stmmac_common_interrupt(struct stmmac_priv *priv)
> @@ -7372,50 +7390,47 @@ int stmmac_reinit_ringparam(struct net_device *dev, u32 rx_size, u32 tx_size)
> return ret;
> }
>
> -#define SEND_VERIFY_MPAKCET_FMT "Send Verify mPacket lo_state=%d lp_state=%d\n"
> -static void stmmac_fpe_lp_task(struct work_struct *work)
> +static void stmmac_fpe_verify_task(struct work_struct *work)
> {
> 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)
> + int verify_limit = 3; /* defined by 802.3 */
> + unsigned long flags;
> +
> + while (1) {
> + msleep(fpe_cfg->verify_time);
> +
Sleep for 1 ms without having done anything prior?
> + if (!netif_running(priv->dev))
> 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);
> + spin_lock_irqsave(&priv->mm_lock, flags);
Thanks for reconsidering the locking.
Unless I'm missing something, it would be good to read fpe_cfg->verify_time
also under the lock. You can save it to a temporary local variable, then
release the lock and go to sleep (waiting for the IRQ to change the FPE
state).
Note that in between fpe_task sleeps, the user could in theory also
change the FPE configuration. I think that in stmmac_set_mm() you should
wait for the fpe_task that's currently in progress to finish, in order
not to change its state from one spin_lock_irqsave() to another.
flush_workqueue() should help with this - but needs to be done without
holding the mm_lock.
>
> - netdev_info(priv->dev, "configured FPE\n");
> + if (fpe_cfg->status == ETHTOOL_MM_VERIFY_STATUS_DISABLED ||
> + fpe_cfg->status == ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED ||
> + !fpe_cfg->pmac_enabled || !fpe_cfg->verify_enabled) {
> + spin_unlock_irqrestore(&priv->mm_lock, flags);
> + break;
> + }
>
> - *lo_state = FPE_STATE_ON;
> - *lp_state = FPE_STATE_ON;
> - netdev_info(priv->dev, "!!! BOTH FPE stations ON\n");
> + if (verify_limit == 0) {
> + fpe_cfg->verify_enabled = false;
> + fpe_cfg->status = ETHTOOL_MM_VERIFY_STATUS_FAILED;
> + stmmac_fpe_configure(priv, priv->ioaddr, fpe_cfg,
> + priv->plat->tx_queues_to_use,
> + priv->plat->rx_queues_to_use,
> + false);
> + spin_unlock_irqrestore(&priv->mm_lock, flags);
> break;
> }
>
> - 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);
> - stmmac_fpe_send_mpacket(priv, priv->ioaddr,
> - fpe_cfg,
> + if (fpe_cfg->status == ETHTOOL_MM_VERIFY_STATUS_VERIFYING)
> + stmmac_fpe_send_mpacket(priv, priv->ioaddr, fpe_cfg,
> MPACKET_VERIFY);
> - }
> - /* Sleep then retry */
> - msleep(500);
> +
> + spin_unlock_irqrestore(&priv->mm_lock, flags);
> +
> + verify_limit--;
> }
>
> clear_bit(__FPE_TASK_SCHED, &priv->fpe_task_state);
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next v3 4/7] net: stmmac: configure FPE via ethtool-mm
2024-08-19 7:25 [PATCH net-next v3 0/7] net: stmmac: FPE via ethtool + tc Furong Xu
` (2 preceding siblings ...)
2024-08-19 7:25 ` [PATCH net-next v3 3/7] net: stmmac: refactor FPE verification processe Furong Xu
@ 2024-08-19 7:25 ` Furong Xu
2024-08-19 7:25 ` [PATCH net-next v3 5/7] net: stmmac: support fp parameter of tc-mqprio Furong Xu
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Furong Xu @ 2024-08-19 7:25 UTC (permalink / raw)
To: Serge Semin, Andrew Lunn, Vladimir Oltean, David S. Miller,
Alexandre Torgue, Jose Abreu, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Maxime Coquelin, Joao Pinto
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, xfr,
Furong Xu
Implement ethtool --show-mm and --set-mm callbacks.
NIC up/down, link up/down, suspend/resume, kselftest-ethtool_mm,
all tested okay.
Signed-off-by: Furong Xu <0x1207@gmail.com>
---
.../net/ethernet/stmicro/stmmac/dwmac4_core.c | 8 +-
drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 36 +++++-
drivers/net/ethernet/stmicro/stmmac/dwmac5.h | 7 +-
.../ethernet/stmicro/stmmac/dwxgmac2_core.c | 4 +-
drivers/net/ethernet/stmicro/stmmac/hwif.h | 8 +-
.../ethernet/stmicro/stmmac/stmmac_ethtool.c | 107 ++++++++++++++++++
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +-
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 2 +-
8 files changed, 163 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 31c387cc5f26..679efcc631f1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -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)
@@ -1268,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,
@@ -1320,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,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
index e02cebc3f1b7..4c91fa766b13 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
@@ -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 */
+ (void)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)
@@ -638,3 +653,20 @@ 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(void __iomem *ioaddr)
+{
+ return FIELD_GET(AFSZ, 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);
+
+ value &= ~AFSZ;
+ value |= FIELD_PREP(AFSZ, add_frag_size);
+
+ writel(value, ioaddr + MTL_FPE_CTRL_STS);
+}
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.h b/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
index bf33a51d229e..e369e65920fc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
@@ -39,6 +39,9 @@
#define MAC_PPSx_INTERVAL(x) (0x00000b88 + ((x) * 0x10))
#define MAC_PPSx_WIDTH(x) (0x00000b8c + ((x) * 0x10))
+#define MTL_FPE_CTRL_STS 0x00000c90
+#define AFSZ GENMASK(1, 0)
+
#define MTL_RXP_CONTROL_STATUS 0x00000ca0
#define RXPI BIT(31)
#define NPE GENMASK(23, 16)
@@ -104,10 +107,12 @@ 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);
int dwmac5_fpe_irq_status(void __iomem *ioaddr, struct net_device *dev);
+int dwmac5_fpe_get_add_frag_size(void __iomem *ioaddr);
+void dwmac5_fpe_set_add_frag_size(void __iomem *ioaddr, u32 add_frag_size);
#endif /* __DWMAC5_H__ */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index cbf2dd976ab1..55a175ced77f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -1506,11 +1506,11 @@ static void dwxgmac2_set_arp_offload(struct mac_device_info *hw, bool en,
static void dwxgmac3_fpe_configure(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg,
u32 num_txq,
- u32 num_rxq, bool enable)
+ 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;
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 28dfc0054a3a..31767427386b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -421,11 +421,13 @@ 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);
int (*fpe_irq_status)(void __iomem *ioaddr, struct net_device *dev);
+ int (*fpe_get_add_frag_size)(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 {
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index 7008219fd88d..4901ece44e84 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -19,6 +19,7 @@
#include "stmmac.h"
#include "dwmac_dma.h"
#include "dwxgmac2.h"
+#include "dwmac5.h"
#define REG_SPACE_SIZE 0x1060
#define GMAC4_REG_SPACE_SIZE 0x116C
@@ -1270,6 +1271,109 @@ static int stmmac_set_tunable(struct net_device *dev,
return ret;
}
+static int stmmac_get_mm(struct net_device *ndev,
+ struct ethtool_mm_state *state)
+{
+ struct stmmac_priv *priv = netdev_priv(ndev);
+ unsigned long flags;
+ u32 add_frag_size;
+
+ if (!priv->dma_cap.fpesel)
+ return -EOPNOTSUPP;
+
+ spin_lock_irqsave(&priv->mm_lock, flags);
+
+ state->pmac_enabled = priv->fpe_cfg.pmac_enabled;
+ state->verify_time = priv->fpe_cfg.verify_time;
+ state->verify_enabled = priv->fpe_cfg.verify_enabled;
+ state->verify_status = priv->fpe_cfg.status;
+ state->rx_min_frag_size = ETH_ZLEN;
+
+ /* 802.3-2018 clause 30.14.1.6, says that the aMACMergeVerifyTime
+ * variable has a range between 1 and 128 ms inclusive. Limit to that.
+ */
+ state->max_verify_time = 128;
+
+ /* Cannot read MAC_FPE_CTRL_STS register here, or FPE interrupt events
+ * can be lost.
+ *
+ * See commit 37e4b8df27bc ("net: stmmac: fix FPE events losing")
+ */
+ state->tx_enabled = !!(priv->fpe_cfg.fpe_csr == EFPE);
+
+ /* FPE active if common tx_enabled and verification success or disabled (forced) */
+ state->tx_active = state->tx_enabled &&
+ (state->verify_status == ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED ||
+ state->verify_status == ETHTOOL_MM_VERIFY_STATUS_DISABLED);
+
+ add_frag_size = stmmac_fpe_get_add_frag_size(priv, priv->ioaddr);
+ state->tx_min_frag_size = ethtool_mm_frag_size_add_to_min(add_frag_size);
+
+ spin_unlock_irqrestore(&priv->mm_lock, flags);
+
+ return 0;
+}
+
+static int stmmac_set_mm(struct net_device *ndev, struct ethtool_mm_cfg *cfg,
+ struct netlink_ext_ack *extack)
+{
+ struct stmmac_priv *priv = netdev_priv(ndev);
+ struct stmmac_fpe_cfg *fpe_cfg = &priv->fpe_cfg;
+ unsigned long flags;
+ u32 add_frag_size;
+ int err;
+
+ if (!priv->dma_cap.fpesel)
+ return -EOPNOTSUPP;
+
+ err = ethtool_mm_frag_size_min_to_add(cfg->tx_min_frag_size,
+ &add_frag_size, extack);
+ if (err)
+ return err;
+
+ spin_lock_irqsave(&priv->mm_lock, flags);
+
+ fpe_cfg->pmac_enabled = cfg->pmac_enabled;
+ fpe_cfg->verify_time = cfg->verify_time;
+ fpe_cfg->verify_enabled = cfg->verify_enabled;
+
+ stmmac_fpe_configure(priv, priv->ioaddr, fpe_cfg,
+ priv->plat->tx_queues_to_use,
+ priv->plat->rx_queues_to_use,
+ cfg->tx_enabled, cfg->pmac_enabled);
+
+ stmmac_fpe_set_add_frag_size(priv, priv->ioaddr, add_frag_size);
+
+ if (cfg->verify_enabled)
+ stmmac_fpe_send_mpacket(priv, priv->ioaddr, fpe_cfg,
+ MPACKET_VERIFY);
+ else
+ fpe_cfg->status = ETHTOOL_MM_VERIFY_STATUS_DISABLED;
+
+ spin_unlock_irqrestore(&priv->mm_lock, flags);
+
+ return 0;
+}
+
+static void stmmac_get_mm_stats(struct net_device *ndev,
+ struct ethtool_mm_stats *s)
+{
+ struct stmmac_priv *priv = netdev_priv(ndev);
+ struct stmmac_counters *mmc = &priv->mmc;
+
+ if (!priv->dma_cap.rmon)
+ return;
+
+ stmmac_mmc_read(priv, priv->mmcaddr, mmc);
+
+ s->MACMergeFrameAssErrorCount = mmc->mmc_rx_packet_assembly_err_cntr;
+ s->MACMergeFrameSmdErrorCount = mmc->mmc_rx_packet_smd_err_cntr;
+ s->MACMergeFrameAssOkCount = mmc->mmc_rx_packet_assembly_ok_cntr;
+ s->MACMergeFragCountRx = mmc->mmc_rx_fpe_fragment_cntr;
+ s->MACMergeFragCountTx = mmc->mmc_tx_fpe_fragment_cntr;
+ s->MACMergeHoldCount = mmc->mmc_tx_hold_req_cntr;
+}
+
static const struct ethtool_ops stmmac_ethtool_ops = {
.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
ETHTOOL_COALESCE_MAX_FRAMES,
@@ -1309,6 +1413,9 @@ static const struct ethtool_ops stmmac_ethtool_ops = {
.set_tunable = stmmac_set_tunable,
.get_link_ksettings = stmmac_ethtool_get_link_ksettings,
.set_link_ksettings = stmmac_ethtool_set_link_ksettings,
+ .get_mm = stmmac_get_mm,
+ .set_mm = stmmac_set_mm,
+ .get_mm_stats = stmmac_get_mm_stats,
};
void stmmac_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 628354f60c54..30a129db3e1a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3547,7 +3547,7 @@ static int stmmac_hw_setup(struct net_device *dev, bool ptp_register)
&priv->fpe_cfg,
priv->plat->tx_queues_to_use,
priv->plat->rx_queues_to_use,
- false);
+ false, priv->fpe_cfg.pmac_enabled);
}
return 0;
@@ -7419,7 +7419,7 @@ static void stmmac_fpe_verify_task(struct work_struct *work)
stmmac_fpe_configure(priv, priv->ioaddr, fpe_cfg,
priv->plat->tx_queues_to_use,
priv->plat->rx_queues_to_use,
- false);
+ false, fpe_cfg->pmac_enabled);
spin_unlock_irqrestore(&priv->mm_lock, flags);
break;
}
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 783829a6479c..a58282d6458c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -1093,7 +1093,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
&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;
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next v3 5/7] net: stmmac: support fp parameter of tc-mqprio
2024-08-19 7:25 [PATCH net-next v3 0/7] net: stmmac: FPE via ethtool + tc Furong Xu
` (3 preceding siblings ...)
2024-08-19 7:25 ` [PATCH net-next v3 4/7] net: stmmac: configure FPE via ethtool-mm Furong Xu
@ 2024-08-19 7:25 ` Furong Xu
2024-08-19 7:25 ` [PATCH net-next v3 6/7] net: stmmac: support fp parameter of tc-taprio Furong Xu
2024-08-19 7:25 ` [PATCH net-next v3 7/7] net: stmmac: silence FPE kernel logs Furong Xu
6 siblings, 0 replies; 12+ messages in thread
From: Furong Xu @ 2024-08-19 7:25 UTC (permalink / raw)
To: Serge Semin, Andrew Lunn, Vladimir Oltean, David S. Miller,
Alexandre Torgue, Jose Abreu, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Maxime Coquelin, Joao Pinto
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, xfr,
Furong Xu
tc-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
Signed-off-by: Furong Xu <0x1207@gmail.com>
---
.../net/ethernet/stmicro/stmmac/dwmac4_core.c | 2 +
drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 52 +++++++++++++
drivers/net/ethernet/stmicro/stmmac/dwmac5.h | 4 +
drivers/net/ethernet/stmicro/stmmac/hwif.h | 10 +++
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 74 +++++++++++++++++++
6 files changed, 144 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 679efcc631f1..4722bac7e3d4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -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_set_preemptible_tcs = dwmac5_fpe_set_preemptible_tcs,
.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_set_preemptible_tcs = dwmac5_fpe_set_preemptible_tcs,
.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,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
index 4c91fa766b13..b6114de34b31 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
@@ -670,3 +670,55 @@ void dwmac5_fpe_set_add_frag_size(void __iomem *ioaddr, u32 add_frag_size)
writel(value, ioaddr + MTL_FPE_CTRL_STS);
}
+
+int dwmac5_fpe_set_preemptible_tcs(struct net_device *ndev,
+ struct netlink_ext_ack *extack,
+ unsigned long tcs)
+{
+ struct stmmac_priv *priv = netdev_priv(ndev);
+ void __iomem *ioaddr = priv->ioaddr;
+ unsigned long queue_tcs = 0;
+ int num_tc = ndev->num_tc;
+ u32 value, queue_weight;
+ u16 offset, count;
+ int tc, i;
+
+ if (!tcs)
+ goto __update_queue_tcs;
+
+ for (tc = 0; tc < num_tc; tc++) {
+ count = ndev->tc_to_txq[tc].count;
+ offset = ndev->tc_to_txq[tc].offset;
+
+ if (tcs & BIT(tc))
+ queue_tcs |= GENMASK(offset + count - 1, offset);
+
+ /* This is 1:1 mapping, go to next TC */
+ if (count == 1)
+ continue;
+
+ if (priv->plat->tx_sched_algorithm == MTL_TX_ALGORITHM_SP) {
+ NL_SET_ERR_MSG_FMT_MOD(extack, "TX algorithm SP is not suitable for one TC to multiple TXQs mapping");
+ return -EINVAL;
+ }
+
+ queue_weight = priv->plat->tx_queues_cfg[offset].weight;
+ for (i = 1; i < count; i++) {
+ if (queue_weight != priv->plat->tx_queues_cfg[offset + i].weight) {
+ NL_SET_ERR_MSG_FMT_MOD(extack, "TXQ weight [%u] differs across other TXQs in TC: [%u]",
+ queue_weight, tc);
+ return -EINVAL;
+ }
+ }
+ }
+
+__update_queue_tcs:
+ value = readl(ioaddr + MTL_FPE_CTRL_STS);
+
+ value &= ~PEC;
+ value |= FIELD_PREP(PEC, queue_tcs);
+
+ writel(value, ioaddr + MTL_FPE_CTRL_STS);
+
+ return 0;
+}
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.h b/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
index e369e65920fc..d3191c48354d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
@@ -40,6 +40,7 @@
#define MAC_PPSx_WIDTH(x) (0x00000b8c + ((x) * 0x10))
#define MTL_FPE_CTRL_STS 0x00000c90
+#define PEC GENMASK(15, 8)
#define AFSZ GENMASK(1, 0)
#define MTL_RXP_CONTROL_STATUS 0x00000ca0
@@ -114,5 +115,8 @@ 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(void __iomem *ioaddr);
void dwmac5_fpe_set_add_frag_size(void __iomem *ioaddr, u32 add_frag_size);
+int dwmac5_fpe_set_preemptible_tcs(struct net_device *ndev,
+ struct netlink_ext_ack *extack,
+ unsigned long tcs);
#endif /* __DWMAC5_H__ */
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 31767427386b..a02b0f7ed96b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -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)(void __iomem *ioaddr);
void (*fpe_set_add_frag_size)(void __iomem *ioaddr, u32 add_frag_size);
+ int (*fpe_set_preemptible_tcs)(struct net_device *ndev,
+ struct netlink_ext_ack *extack,
+ unsigned long tcs);
};
#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_set_preemptible_tcs(__priv, __args...) \
+ stmmac_do_callback(__priv, mac, fpe_set_preemptible_tcs, __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;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 30a129db3e1a..768af9e534b7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6271,6 +6271,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,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index a58282d6458c..a967c6f01e4e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -1174,6 +1174,13 @@ static int tc_query_caps(struct stmmac_priv *priv,
struct tc_query_caps_base *base)
{
switch (base->type) {
+ case TC_SETUP_QDISC_MQPRIO: {
+ struct tc_mqprio_caps *caps = base->caps;
+
+ caps->validate_queue_counts = true;
+
+ return 0;
+ }
case TC_SETUP_QDISC_TAPRIO: {
struct tc_taprio_caps *caps = base->caps;
@@ -1190,6 +1197,72 @@ static int tc_query_caps(struct stmmac_priv *priv,
}
}
+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_set_preemptible_tcs(priv, ndev, extack, 0);
+}
+
+static int tc_setup_mqprio(struct stmmac_priv *priv,
+ struct tc_mqprio_qopt_offload *mqprio)
+{
+ struct tc_mqprio_qopt *qopt = &mqprio->qopt;
+ struct net_device *ndev = priv->dev;
+ int num_stack_tx_queues = 0;
+ int num_tc = qopt->num_tc;
+ u16 offset, count;
+ int tc, err;
+
+ if (!num_tc) {
+ stmmac_reset_tc_mqprio(ndev, mqprio->extack);
+ return 0;
+ }
+
+ err = netdev_set_num_tc(ndev, num_tc);
+ if (err)
+ return err;
+
+ /* DWMAC CORE4+ can not programming TC:TXQ mapping to hardware.
+ * Synopsys Databook:
+ * "The number of Tx DMA channels is equal to the number of Tx queues,
+ * and is direct one-to-one mapping."
+ *
+ * Luckily, DWXGMAC CORE can.
+ *
+ * Thus preemptible_tcs should be handled in a per core manner.
+ */
+ for (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;
+ }
+
+ err = netif_set_real_num_tx_queues(ndev, num_stack_tx_queues);
+ if (err)
+ goto err_reset_tc;
+
+ err = stmmac_fpe_set_preemptible_tcs(priv, ndev, mqprio->extack,
+ mqprio->preemptible_tcs);
+ if (err)
+ goto err_reset_tc;
+
+ return 0;
+
+err_reset_tc:
+ stmmac_reset_tc_mqprio(ndev, mqprio->extack);
+
+ return err;
+}
+
const struct stmmac_tc_ops dwmac510_tc_ops = {
.init = tc_init,
.setup_cls_u32 = tc_setup_cls_u32,
@@ -1198,4 +1271,5 @@ const struct stmmac_tc_ops dwmac510_tc_ops = {
.setup_taprio = tc_setup_taprio,
.setup_etf = tc_setup_etf,
.query_caps = tc_query_caps,
+ .setup_mqprio = tc_setup_mqprio,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next v3 6/7] net: stmmac: support fp parameter of tc-taprio
2024-08-19 7:25 [PATCH net-next v3 0/7] net: stmmac: FPE via ethtool + tc Furong Xu
` (4 preceding siblings ...)
2024-08-19 7:25 ` [PATCH net-next v3 5/7] net: stmmac: support fp parameter of tc-mqprio Furong Xu
@ 2024-08-19 7:25 ` Furong Xu
2024-08-19 11:42 ` Vladimir Oltean
2024-08-19 7:25 ` [PATCH net-next v3 7/7] net: stmmac: silence FPE kernel logs Furong Xu
6 siblings, 1 reply; 12+ messages in thread
From: Furong Xu @ 2024-08-19 7:25 UTC (permalink / raw)
To: Serge Semin, Andrew Lunn, Vladimir Oltean, David S. Miller,
Alexandre Torgue, Jose Abreu, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Maxime Coquelin, Joao Pinto
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, xfr,
Furong Xu
tc-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 E
1) 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
Signed-off-by: Furong Xu <0x1207@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index a967c6f01e4e..05b870b35947 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -933,7 +933,6 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
u32 size, wid = priv->dma_cap.estwid, dep = priv->dma_cap.estdep;
struct timespec64 time, current_time, qopt_time;
ktime_t current_time_ns;
- bool fpe = false;
int i, ret = 0;
u64 ctr;
@@ -1018,16 +1017,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,7 +1053,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
tc_taprio_map_maxsdu_txq(priv, qopt);
- if (fpe && !priv->dma_cap.fpesel) {
+ if (qopt->mqprio.preemptible_tcs && !priv->dma_cap.fpesel) {
mutex_unlock(&priv->est_lock);
return -EOPNOTSUPP;
}
@@ -1071,6 +1066,9 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
goto disable;
}
+ stmmac_fpe_set_preemptible_tcs(priv, priv->dev, qopt->mqprio.extack,
+ qopt->mqprio.preemptible_tcs);
+
netdev_info(priv->dev, "configured EST\n");
return 0;
@@ -1089,11 +1087,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_set_preemptible_tcs(priv, priv->dev, qopt->mqprio.extack, 0);
+
netdev_info(priv->dev, "disabled FPE\n");
return ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next v3 6/7] net: stmmac: support fp parameter of tc-taprio
2024-08-19 7:25 ` [PATCH net-next v3 6/7] net: stmmac: support fp parameter of tc-taprio Furong Xu
@ 2024-08-19 11:42 ` Vladimir Oltean
0 siblings, 0 replies; 12+ messages in thread
From: Vladimir Oltean @ 2024-08-19 11:42 UTC (permalink / raw)
To: Furong Xu
Cc: Serge Semin, Andrew Lunn, David S. Miller, Alexandre Torgue,
Jose Abreu, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maxime Coquelin, Joao Pinto, netdev, linux-stm32,
linux-arm-kernel, linux-kernel, xfr
On Mon, Aug 19, 2024 at 03:25:19PM +0800, Furong Xu wrote:
> tc-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 E
>
> 1) 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
>
> Signed-off-by: Furong Xu <0x1207@gmail.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index a967c6f01e4e..05b870b35947 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> @@ -933,7 +933,6 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
> u32 size, wid = priv->dma_cap.estwid, dep = priv->dma_cap.estdep;
> struct timespec64 time, current_time, qopt_time;
> ktime_t current_time_ns;
> - bool fpe = false;
> int i, ret = 0;
> u64 ctr;
>
> @@ -1018,16 +1017,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,7 +1053,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
>
> tc_taprio_map_maxsdu_txq(priv, qopt);
>
> - if (fpe && !priv->dma_cap.fpesel) {
> + if (qopt->mqprio.preemptible_tcs && !priv->dma_cap.fpesel) {
> mutex_unlock(&priv->est_lock);
> return -EOPNOTSUPP;
> }
This condition is dealt with by the core, now.
if (have_preemption && !ethtool_dev_mm_supported(dev)) {
NL_SET_ERR_MSG(extack, "Device does not support preemption");
return -EOPNOTSUPP;
}
where ethtool_dev_mm_supported() is implemented by issuing a get_mm()
command and seeing what it returns. There's a check for priv->dma_cap.fpesel
there already.
> @@ -1071,6 +1066,9 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
> goto disable;
> }
>
> + stmmac_fpe_set_preemptible_tcs(priv, priv->dev, qopt->mqprio.extack,
> + qopt->mqprio.preemptible_tcs);
> +
> netdev_info(priv->dev, "configured EST\n");
>
> return 0;
> @@ -1089,11 +1087,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_set_preemptible_tcs(priv, priv->dev, qopt->mqprio.extack, 0);
> +
> netdev_info(priv->dev, "disabled FPE\n");
>
> return ret;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next v3 7/7] net: stmmac: silence FPE kernel logs
2024-08-19 7:25 [PATCH net-next v3 0/7] net: stmmac: FPE via ethtool + tc Furong Xu
` (5 preceding siblings ...)
2024-08-19 7:25 ` [PATCH net-next v3 6/7] net: stmmac: support fp parameter of tc-taprio Furong Xu
@ 2024-08-19 7:25 ` Furong Xu
2024-08-19 11:40 ` Vladimir Oltean
6 siblings, 1 reply; 12+ messages in thread
From: Furong Xu @ 2024-08-19 7:25 UTC (permalink / raw)
To: Serge Semin, Andrew Lunn, Vladimir Oltean, David S. Miller,
Alexandre Torgue, Jose Abreu, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Maxime Coquelin, Joao Pinto
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, xfr,
Furong Xu
ethtool --show-mm can get real-time state of FPE.
Those kernel logs should keep quiet.
Signed-off-by: Furong Xu <0x1207@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
index b6114de34b31..733163c52f97 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
@@ -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;
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next v3 7/7] net: stmmac: silence FPE kernel logs
2024-08-19 7:25 ` [PATCH net-next v3 7/7] net: stmmac: silence FPE kernel logs Furong Xu
@ 2024-08-19 11:40 ` Vladimir Oltean
0 siblings, 0 replies; 12+ messages in thread
From: Vladimir Oltean @ 2024-08-19 11:40 UTC (permalink / raw)
To: Furong Xu
Cc: Serge Semin, Andrew Lunn, David S. Miller, Alexandre Torgue,
Jose Abreu, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maxime Coquelin, Joao Pinto, netdev, linux-stm32,
linux-arm-kernel, linux-kernel, xfr
On Mon, Aug 19, 2024 at 03:25:20PM +0800, Furong Xu wrote:
> ethtool --show-mm can get real-time state of FPE.
> Those kernel logs should keep quiet.
>
> Signed-off-by: Furong Xu <0x1207@gmail.com>
> ---
These prints are equally useless?
netdev_info(priv->dev, "FPE workqueue start");
netdev_info(priv->dev, "FPE workqueue stop");
^ permalink raw reply [flat|nested] 12+ messages in thread