Re: [PATCH net-next] net: stmmac: xgmac3+: add FPE handshaking support
From: Serge Semin
Date: Mon Dec 11 2023 - 06:14:11 EST
Hi Jianheng, Jakub
On Mon, Dec 11, 2023 at 06:13:21AM +0000, Jianheng Zhang wrote:
> Adds the HW specific support for Frame Preemption handshaking on XGMAC3+
> cores.
Thanks for the patch. No objection about the change:
Reviewed-by: Serge Semin <fancer.lancer@xxxxxxxxx>
The only note is that the DW XGMAC v3.x and DW QoS Eth v5.x FPE
implementations are now identical (see the attached diff). What about
factoring out the common parts to a separate file - stmmac_fpe.c
(perhaps together with the handshaking algo from the stmmac_main.c)
and send it out as an additional patch on top of this one? A similar
thing has been recently done for EST:
https://lore.kernel.org/netdev/20231201055252.1302-1-rohan.g.thomas@xxxxxxxxx/
Although in this case AFAICS the implementation is simpler and the
only difference is in the CSR offset and the frame preemption residue
queue ID setting. All of that can be easily solved in the same way as
it was done for EST (see the link above).
Jakub, what do you think?
-Serge(y)
>
> Signed-off-by: Jianheng Zhang <Jianheng.Zhang@xxxxxxxxxxxx>
> ---
> drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 6 ++
> .../net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 65 ++++++++++++++++++----
> 2 files changed, 60 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> index 207ff17..306d15b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> @@ -194,6 +194,12 @@
> #define XGMAC_MDIO_DATA 0x00000204
> #define XGMAC_MDIO_C22P 0x00000220
> #define XGMAC_FPE_CTRL_STS 0x00000280
> +#define XGMAC_TRSP BIT(19)
> +#define XGMAC_TVER BIT(18)
> +#define XGMAC_RRSP BIT(17)
> +#define XGMAC_RVER BIT(16)
> +#define XGMAC_SRSP BIT(2)
> +#define XGMAC_SVER BIT(1)
> #define XGMAC_EFPE BIT(0)
> #define XGMAC_ADDRx_HIGH(x) (0x00000300 + (x) * 0x8)
> #define XGMAC_ADDR_MAX 32
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> index eb48211..091d932 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> @@ -1439,22 +1439,63 @@ static void dwxgmac3_fpe_configure(void __iomem *ioaddr, struct stmmac_fpe_cfg *
> {
> u32 value;
>
> - if (!enable) {
> - value = readl(ioaddr + XGMAC_FPE_CTRL_STS);
> + if (enable) {
> + cfg->fpe_csr = XGMAC_EFPE;
> + value = readl(ioaddr + XGMAC_RXQ_CTRL1);
> + value &= ~XGMAC_RQ;
> + value |= (num_rxq - 1) << XGMAC_RQ_SHIFT;
> + writel(value, ioaddr + XGMAC_RXQ_CTRL1);
> + } else {
> + cfg->fpe_csr = 0;
> + }
> + writel(cfg->fpe_csr, ioaddr + XGMAC_FPE_CTRL_STS);
> +}
>
> - value &= ~XGMAC_EFPE;
> +static int dwxgmac3_fpe_irq_status(void __iomem *ioaddr, struct net_device *dev)
> +{
> + u32 value;
> + int status;
>
> - writel(value, ioaddr + XGMAC_FPE_CTRL_STS);
> - return;
> + status = FPE_EVENT_UNKNOWN;
> +
> + /* Reads from the XGMAC_FPE_CTRL_STS register should only be performed
> + * here, since the status flags of MAC_FPE_CTRL_STS are "clear on read"
> + */
> + value = readl(ioaddr + XGMAC_FPE_CTRL_STS);
> +
> + if (value & XGMAC_TRSP) {
> + status |= FPE_EVENT_TRSP;
> + netdev_info(dev, "FPE: Respond mPacket is transmitted\n");
> }
>
> - value = readl(ioaddr + XGMAC_RXQ_CTRL1);
> - value &= ~XGMAC_RQ;
> - value |= (num_rxq - 1) << XGMAC_RQ_SHIFT;
> - writel(value, ioaddr + XGMAC_RXQ_CTRL1);
> + if (value & XGMAC_TVER) {
> + status |= FPE_EVENT_TVER;
> + netdev_info(dev, "FPE: Verify mPacket is transmitted\n");
> + }
> +
> + if (value & XGMAC_RRSP) {
> + status |= FPE_EVENT_RRSP;
> + netdev_info(dev, "FPE: Respond mPacket is received\n");
> + }
> +
> + if (value & XGMAC_RVER) {
> + status |= FPE_EVENT_RVER;
> + netdev_info(dev, "FPE: Verify mPacket is received\n");
> + }
> +
> + return status;
> +}
> +
> +static void dwxgmac3_fpe_send_mpacket(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg,
> + enum stmmac_mpacket_type type)
> +{
> + u32 value = cfg->fpe_csr;
> +
> + if (type == MPACKET_VERIFY)
> + value |= XGMAC_SVER;
> + else if (type == MPACKET_RESPONSE)
> + value |= XGMAC_SRSP;
>
> - value = readl(ioaddr + XGMAC_FPE_CTRL_STS);
> - value |= XGMAC_EFPE;
> writel(value, ioaddr + XGMAC_FPE_CTRL_STS);
> }
>
> @@ -1503,6 +1544,8 @@ static void dwxgmac3_fpe_configure(void __iomem *ioaddr, struct stmmac_fpe_cfg *
> .config_l4_filter = dwxgmac2_config_l4_filter,
> .set_arp_offload = dwxgmac2_set_arp_offload,
> .fpe_configure = dwxgmac3_fpe_configure,
> + .fpe_send_mpacket = dwxgmac3_fpe_send_mpacket,
> + .fpe_irq_status = dwxgmac3_fpe_irq_status,
> };
>
> static void dwxlgmac2_rx_queue_enable(struct mac_device_info *hw, u8 mode,
> --
> 1.8.3.1
>
>
--- drivers/net/ethernet/stmicro/stmmac/dwmac5_fpe.c 2023-12-11 14:01:26.888400348 +0300
+++ drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core_fpe.c 2023-12-11 14:01:49.538644889 +0300
@@ -1,64 +1,64 @@
-#define MAC_FPE_CTRL_STS 0x00000234
-#define TRSP BIT(19)
-#define TVER BIT(18)
-#define RRSP BIT(17)
-#define RVER BIT(16)
-#define SRSP BIT(2)
-#define SVER BIT(1)
-#define EFPE BIT(0)
-
-#define GMAC_RXQ_CTRL0 0x000000a0
-#define GMAC_RXQ_CTRL1 0x000000a4
-
-#define GMAC_RXQCTRL_FPRQ GENMASK(26, 24)
-#define GMAC_RXQCTRL_FPRQ_SHIFT 24
-
-void dwmac5_fpe_configure(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg,
- u32 num_txq, u32 num_rxq,
- bool enable)
+#define XGMAC_FPE_CTRL_STS 0x00000280
+#define XGMAC_TRSP BIT(19)
+#define XGMAC_TVER BIT(18)
+#define XGMAC_RRSP BIT(17)
+#define XGMAC_RVER BIT(16)
+#define XGMAC_SRSP BIT(2)
+#define XGMAC_SVER BIT(1)
+#define XGMAC_EFPE BIT(0)
+
+#define XGMAC_RXQ_CTRL0 0x000000a0
+#define XGMAC_RXQ_CTRL1 0x000000a4
+
+#define XGMAC_RQ GENMASK(7, 4)
+#define XGMAC_RQ_SHIFT 4
+
+static void dwxgmac3_fpe_configure(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg,
+ u32 num_txq,
+ u32 num_rxq, bool enable)
{
u32 value;
if (enable) {
- cfg->fpe_csr = EFPE;
- value = readl(ioaddr + GMAC_RXQ_CTRL1);
- value &= ~GMAC_RXQCTRL_FPRQ;
- value |= (num_rxq - 1) << GMAC_RXQCTRL_FPRQ_SHIFT;
- writel(value, ioaddr + GMAC_RXQ_CTRL1);
+ cfg->fpe_csr = XGMAC_EFPE;
+ value = readl(ioaddr + XGMAC_RXQ_CTRL1);
+ value &= ~XGMAC_RQ;
+ value |= (num_rxq - 1) << XGMAC_RQ_SHIFT;
+ writel(value, ioaddr + XGMAC_RXQ_CTRL1);
} else {
cfg->fpe_csr = 0;
}
- writel(cfg->fpe_csr, ioaddr + MAC_FPE_CTRL_STS);
+ writel(cfg->fpe_csr, ioaddr + XGMAC_FPE_CTRL_STS);
}
-int dwmac5_fpe_irq_status(void __iomem *ioaddr, struct net_device *dev)
+static int dwxgmac3_fpe_irq_status(void __iomem *ioaddr, struct net_device *dev)
{
u32 value;
int status;
status = FPE_EVENT_UNKNOWN;
- /* Reads from the MAC_FPE_CTRL_STS register should only be performed
+ /* Reads from the XGMAC_FPE_CTRL_STS register should only be performed
* here, since the status flags of MAC_FPE_CTRL_STS are "clear on read"
*/
- value = readl(ioaddr + MAC_FPE_CTRL_STS);
+ value = readl(ioaddr + XGMAC_FPE_CTRL_STS);
- if (value & TRSP) {
+ if (value & XGMAC_TRSP) {
status |= FPE_EVENT_TRSP;
netdev_info(dev, "FPE: Respond mPacket is transmitted\n");
}
- if (value & TVER) {
+ if (value & XGMAC_TVER) {
status |= FPE_EVENT_TVER;
netdev_info(dev, "FPE: Verify mPacket is transmitted\n");
}
- if (value & RRSP) {
+ if (value & XGMAC_RRSP) {
status |= FPE_EVENT_RRSP;
netdev_info(dev, "FPE: Respond mPacket is received\n");
}
- if (value & RVER) {
+ if (value & XGMAC_RVER) {
status |= FPE_EVENT_RVER;
netdev_info(dev, "FPE: Verify mPacket is received\n");
}
@@ -66,15 +66,15 @@
return status;
}
-void dwmac5_fpe_send_mpacket(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg,
- enum stmmac_mpacket_type type)
+static void dwxgmac3_fpe_send_mpacket(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg,
+ enum stmmac_mpacket_type type)
{
u32 value = cfg->fpe_csr;
if (type == MPACKET_VERIFY)
- value |= SVER;
+ value |= XGMAC_SVER;
else if (type == MPACKET_RESPONSE)
- value |= SRSP;
+ value |= XGMAC_SRSP;
- writel(value, ioaddr + MAC_FPE_CTRL_STS);
+ writel(value, ioaddr + XGMAC_FPE_CTRL_STS);
}