* [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x
@ 2024-09-04 5:48 jitendra.vegiraju
2024-09-04 5:48 ` [PATCH net-next v5 1/5] net: stmmac: Add HDMA mapping for dw25gmac support jitendra.vegiraju
` (6 more replies)
0 siblings, 7 replies; 20+ messages in thread
From: jitendra.vegiraju @ 2024-09-04 5:48 UTC (permalink / raw)
To: netdev
Cc: alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, jitendra.vegiraju, bcm-kernel-feedback-list,
richardcochran, ast, daniel, hawk, john.fastabend, fancer.lancer,
rmk+kernel, ahalaney, xiaolei.wang, rohan.g.thomas,
Jianheng.Zhang, linux-kernel, linux-stm32, linux-arm-kernel, bpf,
andrew, linux, horms, florian.fainelli
From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
This patchset adds basic PCI ethernet device driver support for Broadcom
BCM8958x Automotive Ethernet switch SoC devices.
This SoC device has PCIe ethernet MAC attached to an integrated ethernet
switch using XGMII interface. The PCIe ethernet controller is presented to
the Linux host as PCI network device.
The following block diagram gives an overview of the application.
+=================================+
| Host CPU/Linux |
+=================================+
|| PCIe
||
+==========================================+
| +--------------+ |
| | PCIE Endpoint| |
| | Ethernet | |
| | Controller | |
| | DMA | |
| +--------------+ |
| | MAC | BCM8958X |
| +--------------+ SoC |
| || XGMII |
| || |
| +--------------+ |
| | Ethernet | |
| | switch | |
| +--------------+ |
| || || || || |
+==========================================+
|| || || || More external interfaces
The MAC block on BCM8958x is based on Synopsis XGMAC 4.00a core. This
MAC IP introduces new DMA architecture called Hyper-DMA for virtualization
scalability.
Driver functionality specific to new MAC (DW25GMAC) is implemented in
new file dw25gmac.c.
Management of integrated ethernet switch on this SoC is not handled by
the PCIe interface.
This SoC device has PCIe ethernet MAC directly attached to an integrated
ethernet switch using XGMII interface.
v4->v5:
Summary of changes in this patch series:
As suggested by Serge Semin, defined common setup function for dw25gmac.
To accommodate early adopter DW25GMAC used in BCM8958x device, provide
a mechanism to override snps_id and snps_dev_id used for driver entry
matching in hwif.c
Patch1:
Added plat_stmmacenet_data::snps_id,snps_dev_id fields - Serge Semin
Patch2:
Define common setup function for dw25gmac_setup() - Serge Semin
Support DW25GMAC IPs with varying VDMA/PDMA count - Abhishek Chauhan
Allocate and initialize hdma mapping configuration data dynamically
based on device's VDMA/PDMA feature capabilities in dw25gmac_setup().
Spelling errors in commit log, lower case 0x for hex -Amit Singh Tomar
Patch3:
Glue support in hwif.c for DW25GMAC in hwif.c - Serge Semin
Provide an option to override snps_id and snps_dev_id when the device
reports version info not conformant with driver's expectations as is
the case with BCM8958x device. - Serge Semin
Patch4:
Remove setup function in the glue driver - Serge Semin
Remove unnecessary calls pci_enable_device() and pci_set_master()
in dwxgmac_brcm_pci_resume() - Jakub Kicinski
Merge variable definitions to single line - Amit Singh Tomar
v3->v4:
Based on Serge's questions, received a confirmation from Synopsys that
the MAC IP is indeed the new 25GMAC design.
Renamed all references of XGMAC4 to 25GMAC.
The patch series is rearranged slightly as follows.
Patch1 (new): Define HDMA mapping data structure in kernel's stmmac.h
Patch2 (v3 Patch1): Adds dma_ops for dw25gmac in stmmac core
Renamed new files dwxgmac4.* to dw25gmac.* - Serge Semin
Defined new Synopsis version and device id macros for DW25GMAC.
Converted bit operations to FIELD_PREP macros - Russell King
Moved hwif.h to this patch, Sparse flagged warning - Simon Horman
Defined macros for hardcoded values TDPS etc - Serge Semin
Read number of PDMAs/VDMAs from hardware - Serge Semin
Patch3 (v3 Patch2): Hooks in hardware interface handling for dw25gmac
Resolved user_version quirks questions - Serge, Russell, Andrew
Added new stmmac_hw entry for DW25GMAC. - Serge
Added logic to override synopsis_dev_id by glue driver.
Patch4 (v3 Patch3): Adds PCI driver for BCM8958x device
Define bitmmap macros for hardcoded values - Andrew Lunn
Added per device software node - Andrew Lunn
Patch5(new/split): Adds BCM8958x driver to build system
https://lore.kernel.org/netdev/20240814221818.2612484-1-jitendra.vegiraju@broadcom.com/
v2->v3:
Addressed v2 comments from Andrew, Jakub, Russel and Simon.
Based on suggestion by Russel and Andrew, added software node to create
phylink in fixed-link mode.
Moved dwxgmac4 specific functions to new files dwxgmac4.c and dwxgmac4.h
in stmmac core module.
Reorganized the code to use the existing glue logic support for xgmac in
hwif.c and override ops functions for dwxgmac4 specific functions.
The patch is split into three parts.
Patch#1 Adds dma_ops for dwxgmac4 in stmmac core
Patch#2 Hooks in the hardware interface handling for dwxgmac4
Patch#3 Adds PCI driver for BCM8958x device
https://lore.kernel.org/netdev/20240802031822.1862030-1-jitendra.vegiraju@broadcom.com/
v1->v2:
Minor fixes to address coding style issues.
Sent v2 too soon by mistake, without waiting for review comments.
Received feedback on this version.
https://lore.kernel.org/netdev/20240511015924.41457-1-jitendra.vegiraju@broadcom.com/
v1:
https://lore.kernel.org/netdev/20240510000331.154486-1-jitendra.vegiraju@broadcom.com/
Jitendra Vegiraju (5):
Add HDMA mapping for dw25gmac support
Add basic dw25gmac support in stmmac core
Integrate dw25gmac into stmmac hwif handling
Add PCI driver support for BCM8958x
Add BCM8958x driver to build system
MAINTAINERS | 8 +
drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 3 +-
drivers/net/ethernet/stmicro/stmmac/common.h | 4 +
.../net/ethernet/stmicro/stmmac/dw25gmac.c | 224 ++++++++
.../net/ethernet/stmicro/stmmac/dw25gmac.h | 92 ++++
.../net/ethernet/stmicro/stmmac/dwmac-brcm.c | 507 ++++++++++++++++++
.../net/ethernet/stmicro/stmmac/dwxgmac2.h | 1 +
.../ethernet/stmicro/stmmac/dwxgmac2_core.c | 43 ++
.../ethernet/stmicro/stmmac/dwxgmac2_dma.c | 31 ++
drivers/net/ethernet/stmicro/stmmac/hwif.c | 26 +-
drivers/net/ethernet/stmicro/stmmac/hwif.h | 1 +
include/linux/stmmac.h | 48 ++
13 files changed, 997 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw25gmac.c
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw25gmac.h
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c
--
2.34.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH net-next v5 1/5] net: stmmac: Add HDMA mapping for dw25gmac support
2024-09-04 5:48 [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x jitendra.vegiraju
@ 2024-09-04 5:48 ` jitendra.vegiraju
2024-09-10 18:37 ` Serge Semin
2024-09-04 5:48 ` [PATCH net-next v5 2/5] net: stmmac: Add basic dw25gmac support in stmmac core jitendra.vegiraju
` (5 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: jitendra.vegiraju @ 2024-09-04 5:48 UTC (permalink / raw)
To: netdev
Cc: alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, jitendra.vegiraju, bcm-kernel-feedback-list,
richardcochran, ast, daniel, hawk, john.fastabend, fancer.lancer,
rmk+kernel, ahalaney, xiaolei.wang, rohan.g.thomas,
Jianheng.Zhang, linux-kernel, linux-stm32, linux-arm-kernel, bpf,
andrew, linux, horms, florian.fainelli
From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
Add hdma configuration support in include/linux/stmmac.h file.
The hdma configuration includes mapping of virtual DMAs to physical DMAs.
Define a new data structure stmmac_hdma_cfg to provide the mapping.
Introduce new plat_stmmacenet_data::snps_id,snps_dev_id to allow glue
drivers to specify synopsys ID and device id respectively.
These values take precedence over reading from HW register. This facility
provides a mechanism to use setup function from stmmac core module and yet
override MAC.VERSION CSR if the glue driver chooses to do so.
Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
---
include/linux/stmmac.h | 48 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 338991c08f00..eb8136680a7b 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -89,6 +89,51 @@ struct stmmac_mdio_bus_data {
bool needs_reset;
};
+/* DW25GMAC Hyper-DMA Overview
+ * Hyper-DMA allows support for large number of Virtual DMA(VDMA)
+ * channels using a smaller set of physical DMA channels(PDMA).
+ * This is supported by the mapping of VDMAs to Traffic Class(TC)
+ * and PDMA to TC in each traffic direction as shown below.
+ *
+ * VDMAs Traffic Class PDMA
+ * +--------+ +------+ +-----------+
+ * |VDMA0 |--------->| TC0 |-------->|PDMA0/TXQ0 |
+ *TX +--------+ |----->+------+ +-----------+
+ *Host=> +--------+ | +------+ +-----------+ => MAC
+ *SW |VDMA1 |---+ | TC1 | +--->|PDMA1/TXQ1 |
+ * +--------+ +------+ | +-----------+
+ * +--------+ +------+----+ +-----------+
+ * |VDMA2 |--------->| TC2 |-------->|PDMA2/TXQ1 |
+ * +--------+ +------+ +-----------+
+ * . . .
+ * +--------+ +------+ +-----------+
+ * |VDMAn-1 |--------->| TCx-1|-------->|PDMAm/TXQm |
+ * +--------+ +------+ +-----------+
+ *
+ * +------+ +------+ +------+
+ * |PDMA0 |--------->| TC0 |-------->|VDMA0 |
+ * +------+ |----->+------+ +------+
+ *MAC => +------+ | +------+ +------+
+ *RXQs |PDMA1 |---+ | TC1 | +--->|VDMA1 | => Host
+ * +------+ +------+ | +------+
+ * . . .
+ */
+
+/* Hyper-DMA mapping configuration
+ * Traffic Class associated with each VDMA/PDMA mapping
+ * is stored in corresponding array entry.
+ */
+struct stmmac_hdma_cfg {
+ u32 tx_vdmas; /* TX VDMA count */
+ u32 rx_vdmas; /* RX VDMA count */
+ u32 tx_pdmas; /* TX PDMA count */
+ u32 rx_pdmas; /* RX PDMA count */
+ u8 *tvdma_tc; /* Tx VDMA to TC mapping array */
+ u8 *rvdma_tc; /* Rx VDMA to TC mapping array */
+ u8 *tpdma_tc; /* Tx PDMA to TC mapping array */
+ u8 *rpdma_tc; /* Rx PDMA to TC mapping array */
+};
+
struct stmmac_dma_cfg {
int pbl;
int txpbl;
@@ -101,6 +146,7 @@ struct stmmac_dma_cfg {
bool multi_msi_en;
bool dche;
bool atds;
+ struct stmmac_hdma_cfg *hdma_cfg;
};
#define AXI_BLEN 7
@@ -303,5 +349,7 @@ struct plat_stmmacenet_data {
int msi_tx_base_vec;
const struct dwmac4_addrs *dwmac4_addrs;
unsigned int flags;
+ u32 snps_id;
+ u32 snps_dev_id;
};
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next v5 2/5] net: stmmac: Add basic dw25gmac support in stmmac core
2024-09-04 5:48 [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x jitendra.vegiraju
2024-09-04 5:48 ` [PATCH net-next v5 1/5] net: stmmac: Add HDMA mapping for dw25gmac support jitendra.vegiraju
@ 2024-09-04 5:48 ` jitendra.vegiraju
2024-09-10 19:24 ` Serge Semin
2024-09-04 5:48 ` [PATCH net-next v5 3/5] net: stmmac: Integrate dw25gmac into stmmac hwif handling jitendra.vegiraju
` (4 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: jitendra.vegiraju @ 2024-09-04 5:48 UTC (permalink / raw)
To: netdev
Cc: alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, jitendra.vegiraju, bcm-kernel-feedback-list,
richardcochran, ast, daniel, hawk, john.fastabend, fancer.lancer,
rmk+kernel, ahalaney, xiaolei.wang, rohan.g.thomas,
Jianheng.Zhang, linux-kernel, linux-stm32, linux-arm-kernel, bpf,
andrew, linux, horms, florian.fainelli
From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
The BCM8958x uses early adopter version of DWC_xgmac version 4.00a for
Ethernet MAC. The DW25GMAC introduced in this version adds new DMA
architecture called Hyper-DMA (HDMA) for virtualization scalability.
This is realized by decoupling physical DMA channels(PDMA) from potentially
large number of virtual DMA channels (VDMA). The VDMAs are software
abstractions that map to PDMAs for frame transmission and reception.
Define new macros DW25GMAC_CORE_4_00 and DW25GMAC_ID to identify 25GMAC
device.
To support the new HDMA architecture, a new instance of stmmac_dma_ops
dw25gmac400_dma_ops is added.
Most of the other dma operation functions in existing dwxgamc2_dma.c file
are reused where applicable.
Added setup function for DW25GMAC's stmmac_hwif_entry in stmmac core.
Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
---
drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +-
drivers/net/ethernet/stmicro/stmmac/common.h | 4 +
.../net/ethernet/stmicro/stmmac/dw25gmac.c | 224 ++++++++++++++++++
.../net/ethernet/stmicro/stmmac/dw25gmac.h | 92 +++++++
.../net/ethernet/stmicro/stmmac/dwxgmac2.h | 1 +
.../ethernet/stmicro/stmmac/dwxgmac2_core.c | 43 ++++
.../ethernet/stmicro/stmmac/dwxgmac2_dma.c | 31 +++
drivers/net/ethernet/stmicro/stmmac/hwif.h | 1 +
8 files changed, 397 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw25gmac.c
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw25gmac.h
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index c2f0e91f6bf8..967e8a9aa432 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -6,7 +6,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o \
mmc_core.o stmmac_hwtstamp.o stmmac_ptp.o dwmac4_descs.o \
dwmac4_dma.o dwmac4_lib.o dwmac4_core.o dwmac5.o hwif.o \
stmmac_tc.o dwxgmac2_core.o dwxgmac2_dma.o dwxgmac2_descs.o \
- stmmac_xdp.o stmmac_est.o \
+ stmmac_xdp.o stmmac_est.o dw25gmac.o \
$(stmmac-y)
stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 684489156dce..9a747b89ba51 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -38,9 +38,11 @@
#define DWXGMAC_CORE_2_10 0x21
#define DWXGMAC_CORE_2_20 0x22
#define DWXLGMAC_CORE_2_00 0x20
+#define DW25GMAC_CORE_4_00 0x40
/* Device ID */
#define DWXGMAC_ID 0x76
+#define DW25GMAC_ID 0x55
#define DWXLGMAC_ID 0x27
#define STMMAC_CHAN0 0 /* Always supported and default for all chips */
@@ -563,6 +565,7 @@ struct mac_link {
u32 speed2500;
u32 speed5000;
u32 speed10000;
+ u32 speed25000;
} xgmii;
struct {
u32 speed25000;
@@ -621,6 +624,7 @@ int dwmac100_setup(struct stmmac_priv *priv);
int dwmac1000_setup(struct stmmac_priv *priv);
int dwmac4_setup(struct stmmac_priv *priv);
int dwxgmac2_setup(struct stmmac_priv *priv);
+int dw25gmac_setup(struct stmmac_priv *priv);
int dwxlgmac2_setup(struct stmmac_priv *priv);
void stmmac_set_mac_addr(void __iomem *ioaddr, const u8 addr[6],
diff --git a/drivers/net/ethernet/stmicro/stmmac/dw25gmac.c b/drivers/net/ethernet/stmicro/stmmac/dw25gmac.c
new file mode 100644
index 000000000000..adb33700ffbb
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dw25gmac.c
@@ -0,0 +1,224 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024 Broadcom Corporation
+ */
+#include "stmmac.h"
+#include "dwxgmac2.h"
+#include "dw25gmac.h"
+
+static u32 decode_vdma_count(u32 regval)
+{
+ /* compressed encoding for vdma count
+ * regval: VDMA count
+ * 0-15 : 1 - 16
+ * 16-19 : 20, 24, 28, 32
+ * 20-23 : 40, 48, 56, 64
+ * 24-27 : 80, 96, 112, 128
+ */
+ if (regval < 16)
+ return regval + 1;
+ return (4 << ((regval - 16) / 4)) * ((regval % 4) + 5);
+}
+
+static void dw25gmac_read_hdma_limits(void __iomem *ioaddr,
+ struct stmmac_hdma_cfg *hdma)
+{
+ u32 hw_cap;
+
+ /* Get VDMA/PDMA counts from HW */
+ hw_cap = readl(ioaddr + XGMAC_HW_FEATURE2);
+ hdma->tx_vdmas = decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_TXCNT,
+ hw_cap));
+ hdma->rx_vdmas = decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_RXCNT,
+ hw_cap));
+ hdma->tx_pdmas = FIELD_GET(XGMAC_HWFEAT_TXQCNT, hw_cap) + 1;
+ hdma->rx_pdmas = FIELD_GET(XGMAC_HWFEAT_RXQCNT, hw_cap) + 1;
+}
+
+int dw25gmac_hdma_cfg_init(struct stmmac_priv *priv)
+{
+ struct plat_stmmacenet_data *plat = priv->plat;
+ struct device *dev = priv->device;
+ struct stmmac_hdma_cfg *hdma;
+ int i;
+
+ hdma = devm_kzalloc(dev,
+ sizeof(*plat->dma_cfg->hdma_cfg),
+ GFP_KERNEL);
+ if (!hdma)
+ return -ENOMEM;
+
+ dw25gmac_read_hdma_limits(priv->ioaddr, hdma);
+
+ hdma->tvdma_tc = devm_kzalloc(dev,
+ sizeof(*hdma->tvdma_tc) * hdma->tx_vdmas,
+ GFP_KERNEL);
+ if (!hdma->tvdma_tc)
+ return -ENOMEM;
+
+ hdma->rvdma_tc = devm_kzalloc(dev,
+ sizeof(*hdma->rvdma_tc) * hdma->rx_vdmas,
+ GFP_KERNEL);
+ if (!hdma->rvdma_tc)
+ return -ENOMEM;
+
+ hdma->tpdma_tc = devm_kzalloc(dev,
+ sizeof(*hdma->tpdma_tc) * hdma->tx_pdmas,
+ GFP_KERNEL);
+ if (!hdma->tpdma_tc)
+ return -ENOMEM;
+
+ hdma->rpdma_tc = devm_kzalloc(dev,
+ sizeof(*hdma->rpdma_tc) * hdma->rx_pdmas,
+ GFP_KERNEL);
+ if (!hdma->rpdma_tc)
+ return -ENOMEM;
+
+ /* Initialize one-to-one mapping for each of the used queues */
+ for (i = 0; i < plat->tx_queues_to_use; i++) {
+ hdma->tvdma_tc[i] = i;
+ hdma->tpdma_tc[i] = i;
+ }
+ for (i = 0; i < plat->rx_queues_to_use; i++) {
+ hdma->rvdma_tc[i] = i;
+ hdma->rpdma_tc[i] = i;
+ }
+ plat->dma_cfg->hdma_cfg = hdma;
+
+ return 0;
+}
+
+static int rd_dma_ch_ind(void __iomem *ioaddr, u8 mode, u32 channel)
+{
+ u32 reg_val = 0;
+
+ reg_val |= FIELD_PREP(XXVGMAC_MODE_SELECT, mode);
+ reg_val |= FIELD_PREP(XXVGMAC_ADDR_OFFSET, channel);
+ reg_val |= XXVGMAC_CMD_TYPE | XXVGMAC_OB;
+ writel(reg_val, ioaddr + XXVGMAC_DMA_CH_IND_CONTROL);
+ return readl(ioaddr + XXVGMAC_DMA_CH_IND_DATA);
+}
+
+static void wr_dma_ch_ind(void __iomem *ioaddr, u8 mode, u32 channel, u32 val)
+{
+ u32 reg_val = 0;
+
+ writel(val, ioaddr + XXVGMAC_DMA_CH_IND_DATA);
+ reg_val |= FIELD_PREP(XXVGMAC_MODE_SELECT, mode);
+ reg_val |= FIELD_PREP(XXVGMAC_ADDR_OFFSET, channel);
+ reg_val |= XGMAC_OB;
+ writel(reg_val, ioaddr + XXVGMAC_DMA_CH_IND_CONTROL);
+}
+
+static void xgmac4_tp2tc_map(void __iomem *ioaddr, u8 pdma_ch, u32 tc_num)
+{
+ u32 val = 0;
+
+ val = rd_dma_ch_ind(ioaddr, MODE_TXEXTCFG, pdma_ch);
+ val &= ~XXVGMAC_TP2TCMP;
+ val |= FIELD_PREP(XXVGMAC_TP2TCMP, tc_num);
+ wr_dma_ch_ind(ioaddr, MODE_TXEXTCFG, pdma_ch, val);
+}
+
+static void xgmac4_rp2tc_map(void __iomem *ioaddr, u8 pdma_ch, u32 tc_num)
+{
+ u32 val = 0;
+
+ val = rd_dma_ch_ind(ioaddr, MODE_RXEXTCFG, pdma_ch);
+ val &= ~XXVGMAC_RP2TCMP;
+ val |= FIELD_PREP(XXVGMAC_RP2TCMP, tc_num);
+ wr_dma_ch_ind(ioaddr, MODE_RXEXTCFG, pdma_ch, val);
+}
+
+void dw25gmac_dma_init(void __iomem *ioaddr,
+ struct stmmac_dma_cfg *dma_cfg)
+{
+ u32 value;
+ u32 i;
+
+ value = readl(ioaddr + XGMAC_DMA_SYSBUS_MODE);
+ value &= ~(XGMAC_AAL | XGMAC_EAME);
+ if (dma_cfg->aal)
+ value |= XGMAC_AAL;
+ if (dma_cfg->eame)
+ value |= XGMAC_EAME;
+ writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
+
+ for (i = 0; i < dma_cfg->hdma_cfg->tx_vdmas; i++) {
+ value = rd_dma_ch_ind(ioaddr, MODE_TXDESCCTRL, i);
+ value &= ~XXVGMAC_TXDCSZ;
+ value |= FIELD_PREP(XXVGMAC_TXDCSZ,
+ XXVGMAC_TXDCSZ_256BYTES);
+ value &= ~XXVGMAC_TDPS;
+ value |= FIELD_PREP(XXVGMAC_TDPS, XXVGMAC_TDPS_HALF);
+ wr_dma_ch_ind(ioaddr, MODE_TXDESCCTRL, i, value);
+ }
+
+ for (i = 0; i < dma_cfg->hdma_cfg->rx_vdmas; i++) {
+ value = rd_dma_ch_ind(ioaddr, MODE_RXDESCCTRL, i);
+ value &= ~XXVGMAC_RXDCSZ;
+ value |= FIELD_PREP(XXVGMAC_RXDCSZ,
+ XXVGMAC_RXDCSZ_256BYTES);
+ value &= ~XXVGMAC_RDPS;
+ value |= FIELD_PREP(XXVGMAC_TDPS, XXVGMAC_RDPS_HALF);
+ wr_dma_ch_ind(ioaddr, MODE_RXDESCCTRL, i, value);
+ }
+
+ for (i = 0; i < dma_cfg->hdma_cfg->tx_pdmas; i++) {
+ value = rd_dma_ch_ind(ioaddr, MODE_TXEXTCFG, i);
+ value &= ~(XXVGMAC_TXPBL | XXVGMAC_TPBLX8_MODE);
+ if (dma_cfg->pblx8)
+ value |= XXVGMAC_TPBLX8_MODE;
+ value |= FIELD_PREP(XXVGMAC_TXPBL, dma_cfg->pbl);
+ wr_dma_ch_ind(ioaddr, MODE_TXEXTCFG, i, value);
+ xgmac4_tp2tc_map(ioaddr, i, dma_cfg->hdma_cfg->tpdma_tc[i]);
+ }
+
+ for (i = 0; i < dma_cfg->hdma_cfg->rx_pdmas; i++) {
+ value = rd_dma_ch_ind(ioaddr, MODE_RXEXTCFG, i);
+ value &= ~(XXVGMAC_RXPBL | XXVGMAC_RPBLX8_MODE);
+ if (dma_cfg->pblx8)
+ value |= XXVGMAC_RPBLX8_MODE;
+ value |= FIELD_PREP(XXVGMAC_RXPBL, dma_cfg->pbl);
+ wr_dma_ch_ind(ioaddr, MODE_RXEXTCFG, i, value);
+ xgmac4_rp2tc_map(ioaddr, i, dma_cfg->hdma_cfg->rpdma_tc[i]);
+ }
+}
+
+void dw25gmac_dma_init_tx_chan(struct stmmac_priv *priv,
+ void __iomem *ioaddr,
+ struct stmmac_dma_cfg *dma_cfg,
+ dma_addr_t dma_addr, u32 chan)
+{
+ u32 value;
+
+ value = readl(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
+ value &= ~XXVGMAC_TVDMA2TCMP;
+ value |= FIELD_PREP(XXVGMAC_TVDMA2TCMP,
+ dma_cfg->hdma_cfg->tvdma_tc[chan]);
+ writel(value, ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
+
+ writel(upper_32_bits(dma_addr),
+ ioaddr + XGMAC_DMA_CH_TxDESC_HADDR(chan));
+ writel(lower_32_bits(dma_addr),
+ ioaddr + XGMAC_DMA_CH_TxDESC_LADDR(chan));
+}
+
+void dw25gmac_dma_init_rx_chan(struct stmmac_priv *priv,
+ void __iomem *ioaddr,
+ struct stmmac_dma_cfg *dma_cfg,
+ dma_addr_t dma_addr, u32 chan)
+{
+ u32 value;
+
+ value = readl(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
+ value &= ~XXVGMAC_RVDMA2TCMP;
+ value |= FIELD_PREP(XXVGMAC_RVDMA2TCMP,
+ dma_cfg->hdma_cfg->rvdma_tc[chan]);
+ writel(value, ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
+
+ writel(upper_32_bits(dma_addr),
+ ioaddr + XGMAC_DMA_CH_RxDESC_HADDR(chan));
+ writel(lower_32_bits(dma_addr),
+ ioaddr + XGMAC_DMA_CH_RxDESC_LADDR(chan));
+}
diff --git a/drivers/net/ethernet/stmicro/stmmac/dw25gmac.h b/drivers/net/ethernet/stmicro/stmmac/dw25gmac.h
new file mode 100644
index 000000000000..76c80a7ed025
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dw25gmac.h
@@ -0,0 +1,92 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright (c) 2024 Broadcom Corporation
+ * DW25GMAC definitions.
+ */
+#ifndef __STMMAC_DW25GMAC_H__
+#define __STMMAC_DW25GMAC_H__
+
+/* Hardware features */
+#define XXVGMAC_HWFEAT_VDMA_RXCNT GENMASK(16, 12)
+#define XXVGMAC_HWFEAT_VDMA_TXCNT GENMASK(22, 18)
+
+/* DMA Indirect Registers*/
+#define XXVGMAC_DMA_CH_IND_CONTROL 0x00003080
+#define XXVGMAC_MODE_SELECT GENMASK(27, 24)
+enum dma_ch_ind_modes {
+ MODE_TXEXTCFG = 0x0, /* Tx Extended Config */
+ MODE_RXEXTCFG = 0x1, /* Rx Extended Config */
+ MODE_TXDBGSTS = 0x2, /* Tx Debug Status */
+ MODE_RXDBGSTS = 0x3, /* Rx Debug Status */
+ MODE_TXDESCCTRL = 0x4, /* Tx Descriptor control */
+ MODE_RXDESCCTRL = 0x5, /* Rx Descriptor control */
+};
+
+#define XXVGMAC_ADDR_OFFSET GENMASK(14, 8)
+#define XXVGMAC_AUTO_INCR GENMASK(5, 4)
+#define XXVGMAC_CMD_TYPE BIT(1)
+#define XXVGMAC_OB BIT(0)
+#define XXVGMAC_DMA_CH_IND_DATA 0x00003084
+
+/* TX Config definitions */
+#define XXVGMAC_TXPBL GENMASK(29, 24)
+#define XXVGMAC_TPBLX8_MODE BIT(19)
+#define XXVGMAC_TP2TCMP GENMASK(18, 16)
+#define XXVGMAC_ORRQ GENMASK(13, 8)
+
+/* RX Config definitions */
+#define XXVGMAC_RXPBL GENMASK(29, 24)
+#define XXVGMAC_RPBLX8_MODE BIT(19)
+#define XXVGMAC_RP2TCMP GENMASK(18, 16)
+#define XXVGMAC_OWRQ GENMASK(13, 8)
+
+/* Tx Descriptor control */
+#define XXVGMAC_TXDCSZ GENMASK(2, 0)
+#define XXVGMAC_TXDCSZ_0BYTES 0
+#define XXVGMAC_TXDCSZ_64BYTES 1
+#define XXVGMAC_TXDCSZ_128BYTES 2
+#define XXVGMAC_TXDCSZ_256BYTES 3
+#define XXVGMAC_TDPS GENMASK(5, 3)
+#define XXVGMAC_TDPS_ZERO 0
+#define XXVGMAC_TDPS_1_8TH 1
+#define XXVGMAC_TDPS_1_4TH 2
+#define XXVGMAC_TDPS_HALF 3
+#define XXVGMAC_TDPS_3_4TH 4
+
+/* Rx Descriptor control */
+#define XXVGMAC_RXDCSZ GENMASK(2, 0)
+#define XXVGMAC_RXDCSZ_0BYTES 0
+#define XXVGMAC_RXDCSZ_64BYTES 1
+#define XXVGMAC_RXDCSZ_128BYTES 2
+#define XXVGMAC_RXDCSZ_256BYTES 3
+#define XXVGMAC_RDPS GENMASK(5, 3)
+#define XXVGMAC_RDPS_ZERO 0
+#define XXVGMAC_RDPS_1_8TH 1
+#define XXVGMAC_RDPS_1_4TH 2
+#define XXVGMAC_RDPS_HALF 3
+#define XXVGMAC_RDPS_3_4TH 4
+
+/* DWCXG_DMA_CH(#i) Registers*/
+#define XXVGMAC_DSL GENMASK(20, 18)
+#define XXVGMAC_MSS GENMASK(13, 0)
+#define XXVGMAC_TFSEL GENMASK(30, 29)
+#define XXVGMAC_TQOS GENMASK(27, 24)
+#define XXVGMAC_IPBL BIT(15)
+#define XXVGMAC_TVDMA2TCMP GENMASK(6, 4)
+#define XXVGMAC_RPF BIT(31)
+#define XXVGMAC_RVDMA2TCMP GENMASK(30, 28)
+#define XXVGMAC_RQOS GENMASK(27, 24)
+
+int dw25gmac_hdma_cfg_init(struct stmmac_priv *priv);
+
+void dw25gmac_dma_init(void __iomem *ioaddr,
+ struct stmmac_dma_cfg *dma_cfg);
+
+void dw25gmac_dma_init_tx_chan(struct stmmac_priv *priv,
+ void __iomem *ioaddr,
+ struct stmmac_dma_cfg *dma_cfg,
+ dma_addr_t dma_addr, u32 chan);
+void dw25gmac_dma_init_rx_chan(struct stmmac_priv *priv,
+ void __iomem *ioaddr,
+ struct stmmac_dma_cfg *dma_cfg,
+ dma_addr_t dma_addr, u32 chan);
+#endif /* __STMMAC_DW25GMAC_H__ */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index 6a2c7d22df1e..c9424c5a6ce5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -17,6 +17,7 @@
#define XGMAC_CONFIG_SS_OFF 29
#define XGMAC_CONFIG_SS_MASK GENMASK(31, 29)
#define XGMAC_CONFIG_SS_10000 (0x0 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_25000 (0x1 << XGMAC_CONFIG_SS_OFF)
#define XGMAC_CONFIG_SS_2500_GMII (0x2 << XGMAC_CONFIG_SS_OFF)
#define XGMAC_CONFIG_SS_1000_GMII (0x3 << XGMAC_CONFIG_SS_OFF)
#define XGMAC_CONFIG_SS_100_MII (0x4 << XGMAC_CONFIG_SS_OFF)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index cbf2dd976ab1..98b94b6e54db 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -11,6 +11,7 @@
#include "stmmac_ptp.h"
#include "dwxlgmac2.h"
#include "dwxgmac2.h"
+#include "dw25gmac.h"
static void dwxgmac2_core_init(struct mac_device_info *hw,
struct net_device *dev)
@@ -1669,6 +1670,48 @@ int dwxgmac2_setup(struct stmmac_priv *priv)
return 0;
}
+int dw25gmac_setup(struct stmmac_priv *priv)
+{
+ struct mac_device_info *mac = priv->hw;
+
+ dev_info(priv->device, "\tDW25GMAC\n");
+
+ priv->dev->priv_flags |= IFF_UNICAST_FLT;
+ mac->pcsr = priv->ioaddr;
+ mac->multicast_filter_bins = priv->plat->multicast_filter_bins;
+ mac->unicast_filter_entries = priv->plat->unicast_filter_entries;
+ mac->mcast_bits_log2 = 0;
+
+ if (mac->multicast_filter_bins)
+ mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
+
+ mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
+ MAC_1000FD | MAC_2500FD | MAC_5000FD |
+ MAC_10000FD | MAC_25000FD;
+ mac->link.duplex = 0;
+ mac->link.speed10 = XGMAC_CONFIG_SS_10_MII;
+ mac->link.speed100 = XGMAC_CONFIG_SS_100_MII;
+ mac->link.speed1000 = XGMAC_CONFIG_SS_1000_GMII;
+ mac->link.speed2500 = XGMAC_CONFIG_SS_2500_GMII;
+ mac->link.xgmii.speed2500 = XGMAC_CONFIG_SS_2500;
+ mac->link.xgmii.speed5000 = XGMAC_CONFIG_SS_5000;
+ mac->link.xgmii.speed10000 = XGMAC_CONFIG_SS_10000;
+ mac->link.xgmii.speed25000 = XGMAC_CONFIG_SS_25000;
+ mac->link.speed_mask = XGMAC_CONFIG_SS_MASK;
+
+ mac->mii.addr = XGMAC_MDIO_ADDR;
+ mac->mii.data = XGMAC_MDIO_DATA;
+ mac->mii.addr_shift = 16;
+ mac->mii.addr_mask = GENMASK(20, 16);
+ mac->mii.reg_shift = 0;
+ mac->mii.reg_mask = GENMASK(15, 0);
+ mac->mii.clk_csr_shift = 19;
+ mac->mii.clk_csr_mask = GENMASK(21, 19);
+
+ /* Allocate and initialize hdma mapping */
+ return dw25gmac_hdma_cfg_init(priv);
+}
+
int dwxlgmac2_setup(struct stmmac_priv *priv)
{
struct mac_device_info *mac = priv->hw;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 7840bc403788..02abfdd40270 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -7,6 +7,7 @@
#include <linux/iopoll.h>
#include "stmmac.h"
#include "dwxgmac2.h"
+#include "dw25gmac.h"
static int dwxgmac2_dma_reset(void __iomem *ioaddr)
{
@@ -641,3 +642,33 @@ const struct stmmac_dma_ops dwxgmac210_dma_ops = {
.enable_sph = dwxgmac2_enable_sph,
.enable_tbs = dwxgmac2_enable_tbs,
};
+
+const struct stmmac_dma_ops dw25gmac400_dma_ops = {
+ .reset = dwxgmac2_dma_reset,
+ .init = dw25gmac_dma_init,
+ .init_chan = dwxgmac2_dma_init_chan,
+ .init_rx_chan = dw25gmac_dma_init_rx_chan,
+ .init_tx_chan = dw25gmac_dma_init_tx_chan,
+ .axi = dwxgmac2_dma_axi,
+ .dump_regs = dwxgmac2_dma_dump_regs,
+ .dma_rx_mode = dwxgmac2_dma_rx_mode,
+ .dma_tx_mode = dwxgmac2_dma_tx_mode,
+ .enable_dma_irq = dwxgmac2_enable_dma_irq,
+ .disable_dma_irq = dwxgmac2_disable_dma_irq,
+ .start_tx = dwxgmac2_dma_start_tx,
+ .stop_tx = dwxgmac2_dma_stop_tx,
+ .start_rx = dwxgmac2_dma_start_rx,
+ .stop_rx = dwxgmac2_dma_stop_rx,
+ .dma_interrupt = dwxgmac2_dma_interrupt,
+ .get_hw_feature = dwxgmac2_get_hw_feature,
+ .rx_watchdog = dwxgmac2_rx_watchdog,
+ .set_rx_ring_len = dwxgmac2_set_rx_ring_len,
+ .set_tx_ring_len = dwxgmac2_set_tx_ring_len,
+ .set_rx_tail_ptr = dwxgmac2_set_rx_tail_ptr,
+ .set_tx_tail_ptr = dwxgmac2_set_tx_tail_ptr,
+ .enable_tso = dwxgmac2_enable_tso,
+ .qmode = dwxgmac2_qmode,
+ .set_bfsize = dwxgmac2_set_bfsize,
+ .enable_sph = dwxgmac2_enable_sph,
+ .enable_tbs = dwxgmac2_enable_tbs,
+};
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 7e90f34b8c88..9764eadf72c2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -682,6 +682,7 @@ extern const struct stmmac_desc_ops dwxgmac210_desc_ops;
extern const struct stmmac_mmc_ops dwmac_mmc_ops;
extern const struct stmmac_mmc_ops dwxgmac_mmc_ops;
extern const struct stmmac_est_ops dwmac510_est_ops;
+extern const struct stmmac_dma_ops dw25gmac400_dma_ops;
#define GMAC_VERSION 0x00000020 /* GMAC CORE Version */
#define GMAC4_VERSION 0x00000110 /* GMAC4+ CORE Version */
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next v5 3/5] net: stmmac: Integrate dw25gmac into stmmac hwif handling
2024-09-04 5:48 [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x jitendra.vegiraju
2024-09-04 5:48 ` [PATCH net-next v5 1/5] net: stmmac: Add HDMA mapping for dw25gmac support jitendra.vegiraju
2024-09-04 5:48 ` [PATCH net-next v5 2/5] net: stmmac: Add basic dw25gmac support in stmmac core jitendra.vegiraju
@ 2024-09-04 5:48 ` jitendra.vegiraju
2024-09-04 5:48 ` [PATCH net-next v5 4/5] net: stmmac: Add PCI driver support for BCM8958x jitendra.vegiraju
` (3 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: jitendra.vegiraju @ 2024-09-04 5:48 UTC (permalink / raw)
To: netdev
Cc: alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, jitendra.vegiraju, bcm-kernel-feedback-list,
richardcochran, ast, daniel, hawk, john.fastabend, fancer.lancer,
rmk+kernel, ahalaney, xiaolei.wang, rohan.g.thomas,
Jianheng.Zhang, linux-kernel, linux-stm32, linux-arm-kernel, bpf,
andrew, linux, horms, florian.fainelli
From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
Integrate dw25gmac support into stmmac hardware interface handling.
Added a new entry to the stmmac_hw table in hwif.c.
Since BCM8958x is an early adopter device, the synopsis_id reported in HW
is 0x32 and device_id is DWXGMAC_ID. Provide override support by giving
preference to snps_id, snps_dev_id values when initialized to non-zero
values by glue driver.
Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
---
drivers/net/ethernet/stmicro/stmmac/hwif.c | 26 +++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c
index 29367105df54..ad76dbab6622 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.c
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c
@@ -257,6 +257,27 @@ static const struct stmmac_hwif_entry {
.est = &dwmac510_est_ops,
.setup = dwxgmac2_setup,
.quirks = NULL,
+ }, {
+ .gmac = false,
+ .gmac4 = false,
+ .xgmac = true,
+ .min_id = DW25GMAC_CORE_4_00,
+ .dev_id = DW25GMAC_ID,
+ .regs = {
+ .ptp_off = PTP_XGMAC_OFFSET,
+ .mmc_off = MMC_XGMAC_OFFSET,
+ .est_off = EST_XGMAC_OFFSET,
+ },
+ .desc = &dwxgmac210_desc_ops,
+ .dma = &dw25gmac400_dma_ops,
+ .mac = &dwxgmac210_ops,
+ .hwtimestamp = &stmmac_ptp,
+ .mode = NULL,
+ .tc = &dwmac510_tc_ops,
+ .mmc = &dwxgmac_mmc_ops,
+ .est = &dwmac510_est_ops,
+ .setup = dw25gmac_setup,
+ .quirks = NULL,
}, {
.gmac = false,
.gmac4 = false,
@@ -292,7 +313,10 @@ int stmmac_hwif_init(struct stmmac_priv *priv)
u32 id, dev_id = 0;
int i, ret;
- if (needs_gmac) {
+ if (priv->plat->snps_id && priv->plat->snps_dev_id) {
+ id = priv->plat->snps_id;
+ dev_id = priv->plat->snps_dev_id;
+ } else if (needs_gmac) {
id = stmmac_get_id(priv, GMAC_VERSION);
} else if (needs_gmac4 || needs_xgmac) {
id = stmmac_get_id(priv, GMAC4_VERSION);
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next v5 4/5] net: stmmac: Add PCI driver support for BCM8958x
2024-09-04 5:48 [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x jitendra.vegiraju
` (2 preceding siblings ...)
2024-09-04 5:48 ` [PATCH net-next v5 3/5] net: stmmac: Integrate dw25gmac into stmmac hwif handling jitendra.vegiraju
@ 2024-09-04 5:48 ` jitendra.vegiraju
2024-09-10 19:51 ` Serge Semin
2024-09-04 5:48 ` [PATCH net-next v5 5/5] net: stmmac: Add BCM8958x driver to build system jitendra.vegiraju
` (2 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: jitendra.vegiraju @ 2024-09-04 5:48 UTC (permalink / raw)
To: netdev
Cc: alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, jitendra.vegiraju, bcm-kernel-feedback-list,
richardcochran, ast, daniel, hawk, john.fastabend, fancer.lancer,
rmk+kernel, ahalaney, xiaolei.wang, rohan.g.thomas,
Jianheng.Zhang, linux-kernel, linux-stm32, linux-arm-kernel, bpf,
andrew, linux, horms, florian.fainelli
From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
Add PCI ethernet driver support for Broadcom BCM8958x SoC devices used
in automotive applications.
This SoC device has PCIe ethernet MAC attached to an integrated ethernet
switch using XGMII interface. The PCIe ethernet controller is presented to
the Linux host as PCI network device.
The following block diagram gives an overview of the application.
+=================================+
| Host CPU/Linux |
+=================================+
|| PCIe
||
+==========================================+
| +--------------+ |
| | PCIE Endpoint| |
| | Ethernet | |
| | Controller | |
| | DMA | |
| +--------------+ |
| | MAC | BCM8958X |
| +--------------+ SoC |
| || XGMII |
| || |
| +--------------+ |
| | Ethernet | |
| | switch | |
| +--------------+ |
| || || || || |
+==========================================+
|| || || || More external interfaces
The MAC IP block on BCM8958x is based on Synopsis XGMAC 4.00a core. This
driver uses common dwxgmac2 code where applicable.
Driver functionality specific to this MAC is implemented in dw25gmac.c.
The glue driver is responsible for setting up hdma mappings.
Management of integrated ethernet switch on this SoC is not handled by
the PCIe interface.
Since BCM8958x is an early adopter device, override the hardware reported
synopsis versions with actual DW25MAC versions that support hdma.
This SoC device has PCIe ethernet MAC directly attached to an integrated
ethernet switch using XGMII interface. Since device tree support is not
available on this platform, a software node is created to enable
fixed-link support using phylink driver.
Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
---
.../net/ethernet/stmicro/stmmac/dwmac-brcm.c | 507 ++++++++++++++++++
1 file changed, 507 insertions(+)
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c
new file mode 100644
index 000000000000..b5dd97d9b938
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c
@@ -0,0 +1,507 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2024 Broadcom Corporation
+ *
+ * PCI driver for ethernet interface of BCM8958X automotive switch chip.
+ *
+ * High level block diagram of the device.
+ * +=================================+
+ * | Host CPU/Linux |
+ * +=================================+
+ * || PCIe
+ * ||
+ * +==========================================+
+ * | +--------------+ |
+ * | | PCIE Endpoint| |
+ * | | Ethernet | |
+ * | | Controller | |
+ * | | DMA | |
+ * | +--------------+ |
+ * | | MAC | BCM8958X |
+ * | +--------------+ SoC |
+ * | || XGMII |
+ * | || |
+ * | +--------------+ |
+ * | | Ethernet | |
+ * | | switch | |
+ * | +--------------+ |
+ * | || || || || |
+ * +==========================================+
+ * || || || || More external interfaces
+ *
+ * This SoC device has PCIe ethernet MAC directly attached to an integrated
+ * ethernet switch using XGMII interface. Since devicetree support is not
+ * available on this platform, a software node is created to enable
+ * fixed-link support using phylink driver.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/dmi.h>
+#include <linux/pci.h>
+#include <linux/phy.h>
+
+#include "stmmac.h"
+#include "dwxgmac2.h"
+#include "dw25gmac.h"
+
+#define PCI_DEVICE_ID_BROADCOM_BCM8958X 0xa00d
+#define BRCM_MAX_MTU 1500
+#define READ_POLL_DELAY_US 100
+#define READ_POLL_TIMEOUT_US 10000
+#define DWMAC_125MHZ 125000000
+#define DWMAC_250MHZ 250000000
+#define BRCM_XGMAC_NUM_VLAN_FILTERS 32
+
+/* TX and RX Queue counts */
+#define BRCM_TX_Q_COUNT 4
+#define BRCM_RX_Q_COUNT 4
+
+#define BRCM_XGMAC_DMA_TX_SIZE 1024
+#define BRCM_XGMAC_DMA_RX_SIZE 1024
+#define BRCM_XGMAC_BAR0_MASK BIT(0)
+
+#define BRCM_XGMAC_IOMEM_MISC_REG_OFFSET 0x0
+#define BRCM_XGMAC_IOMEM_MBOX_REG_OFFSET 0x1000
+#define BRCM_XGMAC_IOMEM_CFG_REG_OFFSET 0x3000
+
+#define XGMAC_MMC_CTRL_RCHM_DISABLE BIT(31)
+#define XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LOW 0x940
+#define XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LO_VALUE 0x00000001
+#define XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HIGH 0x944
+#define XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HI_VALUE 0x88000000
+
+#define XGMAC_PCIE_MISC_MII_CTRL_OFFSET 0x4
+#define XGMAC_PCIE_MISC_MII_CTRL_PAUSE_RX BIT(0)
+#define XGMAC_PCIE_MISC_MII_CTRL_PAUSE_TX BIT(1)
+#define XGMAC_PCIE_MISC_MII_CTRL_LINK_UP BIT(2)
+#define XGMAC_PCIE_MISC_PCIESS_CTRL_OFFSET 0x8
+#define XGMAC_PCIE_MISC_PCIESS_CTRL_EN_MSI_MSIX BIT(9)
+#define XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_OFFSET 0x90
+#define XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_VALUE 0x00000001
+#define XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_OFFSET 0x94
+#define XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_VALUE 0x88000000
+#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST0_OFFSET 0x700
+#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST0_VALUE 1
+#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST1_OFFSET 0x704
+#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST1_VALUE 1
+#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST_DBELL_OFFSET 0x728
+#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST_DBELL_VALUE 1
+#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_SBD_ALL_OFFSET 0x740
+#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_SBD_ALL_VALUE 0
+
+#define XGMAC_PCIE_MISC_FUNC_RESOURCES_PF0_OFFSET 0x804
+
+/* MSIX Vector map register starting offsets */
+#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_RX0_PF0_OFFSET 0x840
+#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_TX0_PF0_OFFSET 0x890
+#define BRCM_MAX_DMA_CHANNEL_PAIRS 4
+
+#define BRCM_XGMAC_MSI_MAC_VECTOR 0
+#define BRCM_XGMAC_MSI_RX_VECTOR_START 9
+#define BRCM_XGMAC_MSI_TX_VECTOR_START 10
+
+static char *fixed_link_node_name = "fixed-link";
+
+static const struct property_entry fixed_link_properties[] = {
+ PROPERTY_ENTRY_U32("speed", 10000),
+ PROPERTY_ENTRY_BOOL("full-duplex"),
+ PROPERTY_ENTRY_BOOL("pause"),
+ { }
+};
+
+struct brcm_priv_data {
+ void __iomem *mbox_regs; /* MBOX Registers*/
+ void __iomem *misc_regs; /* MISC Registers*/
+ void __iomem *xgmac_regs; /* XGMAC Registers*/
+ struct software_node fixed_link_node;
+};
+
+struct dwxgmac_brcm_pci_info {
+ int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat);
+};
+
+static void misc_iowrite(struct brcm_priv_data *brcm_priv,
+ u32 reg, u32 val)
+{
+ iowrite32(val, brcm_priv->misc_regs + reg);
+}
+
+static void dwxgmac_brcm_common_default_data(struct plat_stmmacenet_data *plat)
+{
+ int i;
+
+ plat->has_xgmac = 1;
+ plat->force_sf_dma_mode = 1;
+ plat->mac_port_sel_speed = SPEED_10000;
+ plat->clk_ptp_rate = DWMAC_125MHZ;
+ plat->clk_ref_rate = DWMAC_250MHZ;
+ plat->tx_coe = 1;
+ plat->rx_coe = 1;
+ plat->max_speed = SPEED_10000;
+
+ /* Set default value for multicast hash bins */
+ plat->multicast_filter_bins = HASH_TABLE_SIZE;
+
+ /* Set default value for unicast filter entries */
+ plat->unicast_filter_entries = 1;
+
+ /* Set the maxmtu to device's default */
+ plat->maxmtu = BRCM_MAX_MTU;
+
+ /* Set default number of RX and TX queues to use */
+ plat->tx_queues_to_use = BRCM_TX_Q_COUNT;
+ plat->rx_queues_to_use = BRCM_RX_Q_COUNT;
+
+ plat->tx_sched_algorithm = MTL_TX_ALGORITHM_SP;
+ for (i = 0; i < plat->tx_queues_to_use; i++) {
+ plat->tx_queues_cfg[i].use_prio = false;
+ plat->tx_queues_cfg[i].prio = 0;
+ plat->tx_queues_cfg[i].mode_to_use = MTL_QUEUE_AVB;
+ }
+
+ plat->rx_sched_algorithm = MTL_RX_ALGORITHM_SP;
+ for (i = 0; i < plat->rx_queues_to_use; i++) {
+ plat->rx_queues_cfg[i].use_prio = false;
+ plat->rx_queues_cfg[i].mode_to_use = MTL_QUEUE_AVB;
+ plat->rx_queues_cfg[i].pkt_route = 0x0;
+ plat->rx_queues_cfg[i].chan = i;
+ }
+}
+
+static int dwxgmac_brcm_default_data(struct pci_dev *pdev,
+ struct plat_stmmacenet_data *plat)
+{
+ /* Set common default data first */
+ dwxgmac_brcm_common_default_data(plat);
+
+ plat->snps_id = DW25GMAC_CORE_4_00;
+ plat->snps_dev_id = DW25GMAC_ID;
+ plat->bus_id = 0;
+ plat->phy_addr = 0;
+ plat->phy_interface = PHY_INTERFACE_MODE_USXGMII;
+
+ plat->dma_cfg->pbl = 32;
+ plat->dma_cfg->pblx8 = 0;
+ plat->dma_cfg->aal = 0;
+ plat->dma_cfg->eame = 1;
+
+ plat->axi->axi_wr_osr_lmt = 31;
+ plat->axi->axi_rd_osr_lmt = 31;
+ plat->axi->axi_fb = 0;
+ plat->axi->axi_blen[0] = 4;
+ plat->axi->axi_blen[1] = 8;
+ plat->axi->axi_blen[2] = 16;
+ plat->axi->axi_blen[3] = 32;
+ plat->axi->axi_blen[4] = 64;
+ plat->axi->axi_blen[5] = 128;
+ plat->axi->axi_blen[6] = 256;
+
+ plat->msi_mac_vec = BRCM_XGMAC_MSI_MAC_VECTOR;
+ plat->msi_rx_base_vec = BRCM_XGMAC_MSI_RX_VECTOR_START;
+ plat->msi_tx_base_vec = BRCM_XGMAC_MSI_TX_VECTOR_START;
+
+ return 0;
+}
+
+static struct dwxgmac_brcm_pci_info dwxgmac_brcm_pci_info = {
+ .setup = dwxgmac_brcm_default_data,
+};
+
+static void brcm_config_misc_regs(struct pci_dev *pdev,
+ struct brcm_priv_data *brcm_priv)
+{
+ pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LOW,
+ XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LO_VALUE);
+ pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HIGH,
+ XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HI_VALUE);
+
+ misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_OFFSET,
+ XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_VALUE);
+ misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_OFFSET,
+ XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_VALUE);
+
+ /* Enable Switch Link */
+ misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MII_CTRL_OFFSET,
+ XGMAC_PCIE_MISC_MII_CTRL_PAUSE_RX |
+ XGMAC_PCIE_MISC_MII_CTRL_PAUSE_TX |
+ XGMAC_PCIE_MISC_MII_CTRL_LINK_UP);
+}
+
+static int brcm_config_multi_msi(struct pci_dev *pdev,
+ struct plat_stmmacenet_data *plat,
+ struct stmmac_resources *res)
+{
+ int ret, i;
+
+ if (plat->msi_rx_base_vec >= STMMAC_MSI_VEC_MAX ||
+ plat->msi_tx_base_vec >= STMMAC_MSI_VEC_MAX) {
+ dev_err(&pdev->dev, "%s: Invalid RX & TX vector defined\n",
+ __func__);
+ return -EINVAL;
+ }
+
+ ret = pci_alloc_irq_vectors(pdev, 2, STMMAC_MSI_VEC_MAX,
+ PCI_IRQ_MSI | PCI_IRQ_MSIX);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "%s: multi MSI enablement failed\n",
+ __func__);
+ return ret;
+ }
+
+ /* For RX MSI */
+ for (i = 0; i < plat->rx_queues_to_use; i++)
+ res->rx_irq[i] = pci_irq_vector(pdev,
+ plat->msi_rx_base_vec + i * 2);
+
+ /* For TX MSI */
+ for (i = 0; i < plat->tx_queues_to_use; i++)
+ res->tx_irq[i] = pci_irq_vector(pdev,
+ plat->msi_tx_base_vec + i * 2);
+
+ if (plat->msi_mac_vec < STMMAC_MSI_VEC_MAX)
+ res->irq = pci_irq_vector(pdev, plat->msi_mac_vec);
+
+ plat->flags |= STMMAC_FLAG_MULTI_MSI_EN;
+ plat->flags |= STMMAC_FLAG_TSO_EN;
+
+ return 0;
+}
+
+static int dwxgmac_brcm_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *id)
+{
+ struct dwxgmac_brcm_pci_info *info =
+ (struct dwxgmac_brcm_pci_info *)id->driver_data;
+ struct plat_stmmacenet_data *plat;
+ struct brcm_priv_data *brcm_priv;
+ struct stmmac_resources res;
+ struct device *dev;
+ int rx_offset;
+ int tx_offset;
+ int vector;
+ int ret;
+
+ dev = &pdev->dev;
+
+ brcm_priv = devm_kzalloc(&pdev->dev, sizeof(*brcm_priv), GFP_KERNEL);
+ if (!brcm_priv)
+ return -ENOMEM;
+
+ plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL);
+ if (!plat)
+ return -ENOMEM;
+
+ plat->dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*plat->dma_cfg),
+ GFP_KERNEL);
+ if (!plat->dma_cfg)
+ return -ENOMEM;
+
+ plat->axi = devm_kzalloc(&pdev->dev, sizeof(*plat->axi), GFP_KERNEL);
+ if (!plat->axi)
+ return -ENOMEM;
+
+ /* This device is directly attached to the switch chip internal to the
+ * SoC using XGMII interface. Since no MDIO is present, register
+ * fixed-link software_node to create phylink.
+ */
+ plat->port_node = fwnode_create_software_node(NULL, NULL);
+ brcm_priv->fixed_link_node.name = fixed_link_node_name;
+ brcm_priv->fixed_link_node.properties = fixed_link_properties;
+ brcm_priv->fixed_link_node.parent = to_software_node(plat->port_node);
+ device_add_software_node(dev, &brcm_priv->fixed_link_node);
+
+ /* Disable D3COLD as our device does not support it */
+ pci_d3cold_disable(pdev);
+
+ /* Enable PCI device */
+ ret = pcim_enable_device(pdev);
+ if (ret) {
+ dev_err(&pdev->dev, "%s: ERROR: failed to enable device\n",
+ __func__);
+ return ret;
+ }
+
+ /* Get the base address of device */
+ ret = pcim_iomap_regions(pdev, BRCM_XGMAC_BAR0_MASK, pci_name(pdev));
+ if (ret)
+ return ret;
+ pci_set_master(pdev);
+
+ memset(&res, 0, sizeof(res));
+ res.addr = pcim_iomap_table(pdev)[0];
+ /* MISC Regs */
+ brcm_priv->misc_regs = res.addr + BRCM_XGMAC_IOMEM_MISC_REG_OFFSET;
+ /* MBOX Regs */
+ brcm_priv->mbox_regs = res.addr + BRCM_XGMAC_IOMEM_MBOX_REG_OFFSET;
+ /* XGMAC config Regs */
+ res.addr += BRCM_XGMAC_IOMEM_CFG_REG_OFFSET;
+ brcm_priv->xgmac_regs = res.addr;
+
+ plat->bsp_priv = brcm_priv;
+
+ /* Initialize all MSI vectors to invalid so that it can be set
+ * according to platform data settings below.
+ * Note: MSI vector takes value from 0 up to 31 (STMMAC_MSI_VEC_MAX)
+ */
+ plat->msi_mac_vec = STMMAC_MSI_VEC_MAX;
+ plat->msi_wol_vec = STMMAC_MSI_VEC_MAX;
+ plat->msi_lpi_vec = STMMAC_MSI_VEC_MAX;
+ plat->msi_sfty_ce_vec = STMMAC_MSI_VEC_MAX;
+ plat->msi_sfty_ue_vec = STMMAC_MSI_VEC_MAX;
+ plat->msi_rx_base_vec = STMMAC_MSI_VEC_MAX;
+ plat->msi_tx_base_vec = STMMAC_MSI_VEC_MAX;
+
+ ret = info->setup(pdev, plat);
+ if (ret)
+ return ret;
+
+ pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LOW,
+ XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LO_VALUE);
+ pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HIGH,
+ XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HI_VALUE);
+
+ misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_OFFSET,
+ XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_VALUE);
+ misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_OFFSET,
+ XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_VALUE);
+
+ /* SBD Interrupt */
+ misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_SBD_ALL_OFFSET,
+ XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_SBD_ALL_VALUE);
+ /* EP_DOORBELL Interrupt */
+ misc_iowrite(brcm_priv,
+ XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST_DBELL_OFFSET,
+ XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST_DBELL_VALUE);
+ /* EP_H0 Interrupt */
+ misc_iowrite(brcm_priv,
+ XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST0_OFFSET,
+ XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST0_VALUE);
+ /* EP_H1 Interrupt */
+ misc_iowrite(brcm_priv,
+ XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST1_OFFSET,
+ XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST1_VALUE);
+
+ rx_offset = XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_RX0_PF0_OFFSET;
+ tx_offset = XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_TX0_PF0_OFFSET;
+ vector = BRCM_XGMAC_MSI_RX_VECTOR_START;
+ for (int i = 0; i < BRCM_MAX_DMA_CHANNEL_PAIRS; i++) {
+ /* RX Interrupt */
+ misc_iowrite(brcm_priv, rx_offset, vector++);
+ /* TX Interrupt */
+ misc_iowrite(brcm_priv, tx_offset, vector++);
+ rx_offset += 4;
+ tx_offset += 4;
+ }
+
+ /* Enable Switch Link */
+ misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MII_CTRL_OFFSET,
+ XGMAC_PCIE_MISC_MII_CTRL_PAUSE_RX |
+ XGMAC_PCIE_MISC_MII_CTRL_PAUSE_TX |
+ XGMAC_PCIE_MISC_MII_CTRL_LINK_UP);
+ /* Enable MSI-X */
+ misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_PCIESS_CTRL_OFFSET,
+ XGMAC_PCIE_MISC_PCIESS_CTRL_EN_MSI_MSIX);
+
+ ret = brcm_config_multi_msi(pdev, plat, &res);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "%s: ERROR: failed to enable IRQ\n", __func__);
+ goto err_disable_msi;
+ }
+
+ ret = stmmac_dvr_probe(&pdev->dev, plat, &res);
+ if (ret)
+ goto err_disable_msi;
+
+ return ret;
+
+err_disable_msi:
+ pci_free_irq_vectors(pdev);
+
+ return ret;
+}
+
+static void dwxgmac_brcm_software_node_remove(struct pci_dev *pdev)
+{
+ struct fwnode_handle *fwnode;
+ struct stmmac_priv *priv;
+ struct net_device *ndev;
+ struct device *dev;
+
+ dev = &pdev->dev;
+ ndev = dev_get_drvdata(dev);
+ priv = netdev_priv(ndev);
+ fwnode = priv->plat->port_node;
+
+ fwnode_remove_software_node(fwnode);
+ device_remove_software_node(dev);
+}
+
+static void dwxgmac_brcm_pci_remove(struct pci_dev *pdev)
+{
+ stmmac_dvr_remove(&pdev->dev);
+ pci_free_irq_vectors(pdev);
+ pcim_iounmap_regions(pdev, BRCM_XGMAC_BAR0_MASK);
+ pci_clear_master(pdev);
+ dwxgmac_brcm_software_node_remove(pdev);
+}
+
+static int __maybe_unused dwxgmac_brcm_pci_suspend(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ int ret;
+
+ ret = stmmac_suspend(dev);
+ if (ret)
+ return ret;
+
+ ret = pci_save_state(pdev);
+ if (ret)
+ return ret;
+
+ pci_disable_device(pdev);
+ pci_wake_from_d3(pdev, true);
+
+ return 0;
+}
+
+static int __maybe_unused dwxgmac_brcm_pci_resume(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct stmmac_priv *priv;
+ struct net_device *ndev;
+
+ ndev = dev_get_drvdata(dev);
+ priv = netdev_priv(ndev);
+ brcm_config_misc_regs(pdev, priv->plat->bsp_priv);
+
+ pci_restore_state(pdev);
+ pci_set_power_state(pdev, PCI_D0);
+
+ return stmmac_resume(dev);
+}
+
+static SIMPLE_DEV_PM_OPS(dwxgmac_brcm_pm_ops,
+ dwxgmac_brcm_pci_suspend,
+ dwxgmac_brcm_pci_resume);
+
+static const struct pci_device_id dwxgmac_brcm_id_table[] = {
+ { PCI_DEVICE_DATA(BROADCOM, BCM8958X, &dwxgmac_brcm_pci_info) },
+ {}
+};
+
+MODULE_DEVICE_TABLE(pci, dwxgmac_brcm_id_table);
+
+static struct pci_driver dwxgmac_brcm_pci_driver = {
+ .name = "brcm-bcm8958x",
+ .id_table = dwxgmac_brcm_id_table,
+ .probe = dwxgmac_brcm_pci_probe,
+ .remove = dwxgmac_brcm_pci_remove,
+ .driver = {
+ .pm = &dwxgmac_brcm_pm_ops,
+ },
+};
+
+module_pci_driver(dwxgmac_brcm_pci_driver);
+
+MODULE_DESCRIPTION("Broadcom 10G Automotive Ethernet PCIe driver");
+MODULE_LICENSE("GPL");
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next v5 5/5] net: stmmac: Add BCM8958x driver to build system
2024-09-04 5:48 [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x jitendra.vegiraju
` (3 preceding siblings ...)
2024-09-04 5:48 ` [PATCH net-next v5 4/5] net: stmmac: Add PCI driver support for BCM8958x jitendra.vegiraju
@ 2024-09-04 5:48 ` jitendra.vegiraju
2024-09-06 12:14 ` [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x Serge Semin
2024-09-10 11:29 ` Paolo Abeni
6 siblings, 0 replies; 20+ messages in thread
From: jitendra.vegiraju @ 2024-09-04 5:48 UTC (permalink / raw)
To: netdev
Cc: alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, jitendra.vegiraju, bcm-kernel-feedback-list,
richardcochran, ast, daniel, hawk, john.fastabend, fancer.lancer,
rmk+kernel, ahalaney, xiaolei.wang, rohan.g.thomas,
Jianheng.Zhang, linux-kernel, linux-stm32, linux-arm-kernel, bpf,
andrew, linux, horms, florian.fainelli
From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
Add PCI driver for BCM8958x to the linux build system and
update MAINTAINERS file.
Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
---
MAINTAINERS | 8 ++++++++
drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 +++++++++++
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
3 files changed, 20 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index baf88e74c907..199fe7699365 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4360,6 +4360,14 @@ N: brcmstb
N: bcm7038
N: bcm7120
+BROADCOM BCM8958X ETHERNET DRIVER
+M: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
+R: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
+L: netdev@vger.kernel.org
+S: Maintained
+F: drivers/net/ethernet/stmicro/stmmac/dw25gmac.*
+F: drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c
+
BROADCOM BCMBCA ARM ARCHITECTURE
M: William Zhang <william.zhang@broadcom.com>
M: Anand Gore <anand.gore@broadcom.com>
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 05cc07b8f48c..47c9db123b03 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -298,6 +298,17 @@ config DWMAC_LOONGSON
This selects the LOONGSON PCI bus support for the stmmac driver,
Support for ethernet controller on Loongson-2K1000 SoC and LS7A1000 bridge.
+config DWMAC_BRCM
+ tristate "Broadcom XGMAC support"
+ depends on STMMAC_ETH && PCI
+ depends on COMMON_CLK
+ help
+ Support for ethernet controllers on Broadcom BCM8958x SoCs.
+
+ This selects Broadcom XGMAC specific PCI bus support for the
+ stmmac driver. This driver provides the glue layer on top of the
+ stmmac driver required for the Broadcom BCM8958x SoC devices.
+
config STMMAC_PCI
tristate "STMMAC PCI bus support"
depends on STMMAC_ETH && PCI
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index 967e8a9aa432..517981b9e93a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -41,4 +41,5 @@ dwmac-altr-socfpga-objs := dwmac-socfpga.o
obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
obj-$(CONFIG_DWMAC_INTEL) += dwmac-intel.o
obj-$(CONFIG_DWMAC_LOONGSON) += dwmac-loongson.o
+obj-$(CONFIG_DWMAC_BRCM) += dwmac-brcm.o
stmmac-pci-objs:= stmmac_pci.o
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x
2024-09-04 5:48 [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x jitendra.vegiraju
` (4 preceding siblings ...)
2024-09-04 5:48 ` [PATCH net-next v5 5/5] net: stmmac: Add BCM8958x driver to build system jitendra.vegiraju
@ 2024-09-06 12:14 ` Serge Semin
2024-09-10 11:29 ` Paolo Abeni
6 siblings, 0 replies; 20+ messages in thread
From: Serge Semin @ 2024-09-06 12:14 UTC (permalink / raw)
To: jitendra.vegiraju
Cc: netdev, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, bcm-kernel-feedback-list, richardcochran, ast,
daniel, hawk, john.fastabend, rmk+kernel, ahalaney, xiaolei.wang,
rohan.g.thomas, Jianheng.Zhang, linux-kernel, linux-stm32,
linux-arm-kernel, bpf, andrew, linux, horms, florian.fainelli
Hi Jitendra
On Tue, Sep 03, 2024 at 10:48:10PM -0700, jitendra.vegiraju@broadcom.com wrote:
> From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
>
> This patchset adds basic PCI ethernet device driver support for Broadcom
> BCM8958x Automotive Ethernet switch SoC devices.
>
> This SoC device has PCIe ethernet MAC attached to an integrated ethernet
> switch using XGMII interface. The PCIe ethernet controller is presented to
> the Linux host as PCI network device.
>
> The following block diagram gives an overview of the application.
> +=================================+
> | Host CPU/Linux |
> +=================================+
> || PCIe
> ||
> +==========================================+
> | +--------------+ |
> | | PCIE Endpoint| |
> | | Ethernet | |
> | | Controller | |
> | | DMA | |
> | +--------------+ |
> | | MAC | BCM8958X |
> | +--------------+ SoC |
> | || XGMII |
> | || |
> | +--------------+ |
> | | Ethernet | |
> | | switch | |
> | +--------------+ |
> | || || || || |
> +==========================================+
> || || || || More external interfaces
>
> The MAC block on BCM8958x is based on Synopsis XGMAC 4.00a core. This
> MAC IP introduces new DMA architecture called Hyper-DMA for virtualization
> scalability.
>
> Driver functionality specific to new MAC (DW25GMAC) is implemented in
> new file dw25gmac.c.
>
> Management of integrated ethernet switch on this SoC is not handled by
> the PCIe interface.
> This SoC device has PCIe ethernet MAC directly attached to an integrated
> ethernet switch using XGMII interface.
>
> v4->v5:
> Summary of changes in this patch series:
> As suggested by Serge Semin, defined common setup function for dw25gmac.
> To accommodate early adopter DW25GMAC used in BCM8958x device, provide
> a mechanism to override snps_id and snps_dev_id used for driver entry
> matching in hwif.c
>
> Patch1:
> Added plat_stmmacenet_data::snps_id,snps_dev_id fields - Serge Semin
> Patch2:
> Define common setup function for dw25gmac_setup() - Serge Semin
> Support DW25GMAC IPs with varying VDMA/PDMA count - Abhishek Chauhan
> Allocate and initialize hdma mapping configuration data dynamically
> based on device's VDMA/PDMA feature capabilities in dw25gmac_setup().
> Spelling errors in commit log, lower case 0x for hex -Amit Singh Tomar
> Patch3:
> Glue support in hwif.c for DW25GMAC in hwif.c - Serge Semin
> Provide an option to override snps_id and snps_dev_id when the device
> reports version info not conformant with driver's expectations as is
> the case with BCM8958x device. - Serge Semin
> Patch4:
> Remove setup function in the glue driver - Serge Semin
> Remove unnecessary calls pci_enable_device() and pci_set_master()
> in dwxgmac_brcm_pci_resume() - Jakub Kicinski
> Merge variable definitions to single line - Amit Singh Tomar
Thanks for the update. I'll have a closer look at the series early
next week.
-Serge(y)
> [...]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x
2024-09-04 5:48 [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x jitendra.vegiraju
` (5 preceding siblings ...)
2024-09-06 12:14 ` [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x Serge Semin
@ 2024-09-10 11:29 ` Paolo Abeni
2024-09-10 11:55 ` Serge Semin
6 siblings, 1 reply; 20+ messages in thread
From: Paolo Abeni @ 2024-09-10 11:29 UTC (permalink / raw)
To: fancer.lancer
Cc: alexandre.torgue, joabreu, davem, edumazet, kuba, mcoquelin.stm32,
bcm-kernel-feedback-list, richardcochran, ast, daniel, hawk,
john.fastabend, rmk+kernel, ahalaney, xiaolei.wang,
rohan.g.thomas, Jianheng.Zhang, linux-kernel, linux-stm32,
linux-arm-kernel, bpf, andrew, linux, horms, florian.fainelli,
jitendra.vegiraju, netdev
On 9/4/24 07:48, jitendra.vegiraju@broadcom.com wrote:
> From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
>
> This patchset adds basic PCI ethernet device driver support for Broadcom
> BCM8958x Automotive Ethernet switch SoC devices.
>
> This SoC device has PCIe ethernet MAC attached to an integrated ethernet
> switch using XGMII interface. The PCIe ethernet controller is presented to
> the Linux host as PCI network device.
>
> The following block diagram gives an overview of the application.
> +=================================+
> | Host CPU/Linux |
> +=================================+
> || PCIe
> ||
> +==========================================+
> | +--------------+ |
> | | PCIE Endpoint| |
> | | Ethernet | |
> | | Controller | |
> | | DMA | |
> | +--------------+ |
> | | MAC | BCM8958X |
> | +--------------+ SoC |
> | || XGMII |
> | || |
> | +--------------+ |
> | | Ethernet | |
> | | switch | |
> | +--------------+ |
> | || || || || |
> +==========================================+
> || || || || More external interfaces
>
> The MAC block on BCM8958x is based on Synopsis XGMAC 4.00a core. This
> MAC IP introduces new DMA architecture called Hyper-DMA for virtualization
> scalability.
>
> Driver functionality specific to new MAC (DW25GMAC) is implemented in
> new file dw25gmac.c.
>
> Management of integrated ethernet switch on this SoC is not handled by
> the PCIe interface.
> This SoC device has PCIe ethernet MAC directly attached to an integrated
> ethernet switch using XGMII interface.
>
> v4->v5:
> Summary of changes in this patch series:
> As suggested by Serge Semin, defined common setup function for dw25gmac.
> To accommodate early adopter DW25GMAC used in BCM8958x device, provide
> a mechanism to override snps_id and snps_dev_id used for driver entry
> matching in hwif.c
>
> Patch1:
> Added plat_stmmacenet_data::snps_id,snps_dev_id fields - Serge Semin
> Patch2:
> Define common setup function for dw25gmac_setup() - Serge Semin
> Support DW25GMAC IPs with varying VDMA/PDMA count - Abhishek Chauhan
> Allocate and initialize hdma mapping configuration data dynamically
> based on device's VDMA/PDMA feature capabilities in dw25gmac_setup().
> Spelling errors in commit log, lower case 0x for hex -Amit Singh Tomar
> Patch3:
> Glue support in hwif.c for DW25GMAC in hwif.c - Serge Semin
> Provide an option to override snps_id and snps_dev_id when the device
> reports version info not conformant with driver's expectations as is
> the case with BCM8958x device. - Serge Semin
> Patch4:
> Remove setup function in the glue driver - Serge Semin
> Remove unnecessary calls pci_enable_device() and pci_set_master()
> in dwxgmac_brcm_pci_resume() - Jakub Kicinski
> Merge variable definitions to single line - Amit Singh Tomar
>
> v3->v4:
> Based on Serge's questions, received a confirmation from Synopsys that
> the MAC IP is indeed the new 25GMAC design.
> Renamed all references of XGMAC4 to 25GMAC.
> The patch series is rearranged slightly as follows.
> Patch1 (new): Define HDMA mapping data structure in kernel's stmmac.h
> Patch2 (v3 Patch1): Adds dma_ops for dw25gmac in stmmac core
> Renamed new files dwxgmac4.* to dw25gmac.* - Serge Semin
> Defined new Synopsis version and device id macros for DW25GMAC.
> Converted bit operations to FIELD_PREP macros - Russell King
> Moved hwif.h to this patch, Sparse flagged warning - Simon Horman
> Defined macros for hardcoded values TDPS etc - Serge Semin
> Read number of PDMAs/VDMAs from hardware - Serge Semin
> Patch3 (v3 Patch2): Hooks in hardware interface handling for dw25gmac
> Resolved user_version quirks questions - Serge, Russell, Andrew
> Added new stmmac_hw entry for DW25GMAC. - Serge
> Added logic to override synopsis_dev_id by glue driver.
> Patch4 (v3 Patch3): Adds PCI driver for BCM8958x device
> Define bitmmap macros for hardcoded values - Andrew Lunn
> Added per device software node - Andrew Lunn
> Patch5(new/split): Adds BCM8958x driver to build system
> https://lore.kernel.org/netdev/20240814221818.2612484-1-jitendra.vegiraju@broadcom.com/
>
> v2->v3:
> Addressed v2 comments from Andrew, Jakub, Russel and Simon.
> Based on suggestion by Russel and Andrew, added software node to create
> phylink in fixed-link mode.
> Moved dwxgmac4 specific functions to new files dwxgmac4.c and dwxgmac4.h
> in stmmac core module.
> Reorganized the code to use the existing glue logic support for xgmac in
> hwif.c and override ops functions for dwxgmac4 specific functions.
> The patch is split into three parts.
> Patch#1 Adds dma_ops for dwxgmac4 in stmmac core
> Patch#2 Hooks in the hardware interface handling for dwxgmac4
> Patch#3 Adds PCI driver for BCM8958x device
> https://lore.kernel.org/netdev/20240802031822.1862030-1-jitendra.vegiraju@broadcom.com/
>
> v1->v2:
> Minor fixes to address coding style issues.
> Sent v2 too soon by mistake, without waiting for review comments.
> Received feedback on this version.
> https://lore.kernel.org/netdev/20240511015924.41457-1-jitendra.vegiraju@broadcom.com/
>
> v1:
> https://lore.kernel.org/netdev/20240510000331.154486-1-jitendra.vegiraju@broadcom.com/
>
> Jitendra Vegiraju (5):
> Add HDMA mapping for dw25gmac support
> Add basic dw25gmac support in stmmac core
> Integrate dw25gmac into stmmac hwif handling
> Add PCI driver support for BCM8958x
> Add BCM8958x driver to build system
>
> MAINTAINERS | 8 +
> drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 +
> drivers/net/ethernet/stmicro/stmmac/Makefile | 3 +-
> drivers/net/ethernet/stmicro/stmmac/common.h | 4 +
> .../net/ethernet/stmicro/stmmac/dw25gmac.c | 224 ++++++++
> .../net/ethernet/stmicro/stmmac/dw25gmac.h | 92 ++++
> .../net/ethernet/stmicro/stmmac/dwmac-brcm.c | 507 ++++++++++++++++++
> .../net/ethernet/stmicro/stmmac/dwxgmac2.h | 1 +
> .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 43 ++
> .../ethernet/stmicro/stmmac/dwxgmac2_dma.c | 31 ++
> drivers/net/ethernet/stmicro/stmmac/hwif.c | 26 +-
> drivers/net/ethernet/stmicro/stmmac/hwif.h | 1 +
> include/linux/stmmac.h | 48 ++
> 13 files changed, 997 insertions(+), 2 deletions(-)
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw25gmac.c
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw25gmac.h
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c
Hi Serge, to you think you will have time to review this series soon?
We are sort in a rush to flush the net-next material before the upcoming
merge window.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x
2024-09-10 11:29 ` Paolo Abeni
@ 2024-09-10 11:55 ` Serge Semin
0 siblings, 0 replies; 20+ messages in thread
From: Serge Semin @ 2024-09-10 11:55 UTC (permalink / raw)
To: Paolo Abeni
Cc: alexandre.torgue, joabreu, davem, edumazet, kuba, mcoquelin.stm32,
bcm-kernel-feedback-list, richardcochran, ast, daniel, hawk,
john.fastabend, rmk+kernel, ahalaney, xiaolei.wang,
rohan.g.thomas, Jianheng.Zhang, linux-kernel, linux-stm32,
linux-arm-kernel, bpf, andrew, linux, horms, florian.fainelli,
jitendra.vegiraju, netdev
Hi Paolo
On Tue, Sep 10, 2024 at 01:29:34PM +0200, Paolo Abeni wrote:
> On 9/4/24 07:48, jitendra.vegiraju@broadcom.com wrote:
> > From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
> >
> > This patchset adds basic PCI ethernet device driver support for Broadcom
> > BCM8958x Automotive Ethernet switch SoC devices.
> >
> > This SoC device has PCIe ethernet MAC attached to an integrated ethernet
> > switch using XGMII interface. The PCIe ethernet controller is presented to
> > the Linux host as PCI network device.
> >
> > The following block diagram gives an overview of the application.
> > +=================================+
> > | Host CPU/Linux |
> > +=================================+
> > || PCIe
> > ||
> > +==========================================+
> > | +--------------+ |
> > | | PCIE Endpoint| |
> > | | Ethernet | |
> > | | Controller | |
> > | | DMA | |
> > | +--------------+ |
> > | | MAC | BCM8958X |
> > | +--------------+ SoC |
> > | || XGMII |
> > | || |
> > | +--------------+ |
> > | | Ethernet | |
> > | | switch | |
> > | +--------------+ |
> > | || || || || |
> > +==========================================+
> > || || || || More external interfaces
> >
> > The MAC block on BCM8958x is based on Synopsis XGMAC 4.00a core. This
> > MAC IP introduces new DMA architecture called Hyper-DMA for virtualization
> > scalability.
> >
> > Driver functionality specific to new MAC (DW25GMAC) is implemented in
> > new file dw25gmac.c.
> >
> > Management of integrated ethernet switch on this SoC is not handled by
> > the PCIe interface.
> > This SoC device has PCIe ethernet MAC directly attached to an integrated
> > ethernet switch using XGMII interface.
> >
> > v4->v5:
> > Summary of changes in this patch series:
> > As suggested by Serge Semin, defined common setup function for dw25gmac.
> > To accommodate early adopter DW25GMAC used in BCM8958x device, provide
> > a mechanism to override snps_id and snps_dev_id used for driver entry
> > matching in hwif.c
> >
> > Patch1:
> > Added plat_stmmacenet_data::snps_id,snps_dev_id fields - Serge Semin
> > Patch2:
> > Define common setup function for dw25gmac_setup() - Serge Semin
> > Support DW25GMAC IPs with varying VDMA/PDMA count - Abhishek Chauhan
> > Allocate and initialize hdma mapping configuration data dynamically
> > based on device's VDMA/PDMA feature capabilities in dw25gmac_setup().
> > Spelling errors in commit log, lower case 0x for hex -Amit Singh Tomar
> > Patch3:
> > Glue support in hwif.c for DW25GMAC in hwif.c - Serge Semin
> > Provide an option to override snps_id and snps_dev_id when the device
> > reports version info not conformant with driver's expectations as is
> > the case with BCM8958x device. - Serge Semin
> > Patch4:
> > Remove setup function in the glue driver - Serge Semin
> > Remove unnecessary calls pci_enable_device() and pci_set_master()
> > in dwxgmac_brcm_pci_resume() - Jakub Kicinski
> > Merge variable definitions to single line - Amit Singh Tomar
> >
> > v3->v4:
> > Based on Serge's questions, received a confirmation from Synopsys that
> > the MAC IP is indeed the new 25GMAC design.
> > Renamed all references of XGMAC4 to 25GMAC.
> > The patch series is rearranged slightly as follows.
> > Patch1 (new): Define HDMA mapping data structure in kernel's stmmac.h
> > Patch2 (v3 Patch1): Adds dma_ops for dw25gmac in stmmac core
> > Renamed new files dwxgmac4.* to dw25gmac.* - Serge Semin
> > Defined new Synopsis version and device id macros for DW25GMAC.
> > Converted bit operations to FIELD_PREP macros - Russell King
> > Moved hwif.h to this patch, Sparse flagged warning - Simon Horman
> > Defined macros for hardcoded values TDPS etc - Serge Semin
> > Read number of PDMAs/VDMAs from hardware - Serge Semin
> > Patch3 (v3 Patch2): Hooks in hardware interface handling for dw25gmac
> > Resolved user_version quirks questions - Serge, Russell, Andrew
> > Added new stmmac_hw entry for DW25GMAC. - Serge
> > Added logic to override synopsis_dev_id by glue driver.
> > Patch4 (v3 Patch3): Adds PCI driver for BCM8958x device
> > Define bitmmap macros for hardcoded values - Andrew Lunn
> > Added per device software node - Andrew Lunn
> > Patch5(new/split): Adds BCM8958x driver to build system
> > https://lore.kernel.org/netdev/20240814221818.2612484-1-jitendra.vegiraju@broadcom.com/
> >
> > v2->v3:
> > Addressed v2 comments from Andrew, Jakub, Russel and Simon.
> > Based on suggestion by Russel and Andrew, added software node to create
> > phylink in fixed-link mode.
> > Moved dwxgmac4 specific functions to new files dwxgmac4.c and dwxgmac4.h
> > in stmmac core module.
> > Reorganized the code to use the existing glue logic support for xgmac in
> > hwif.c and override ops functions for dwxgmac4 specific functions.
> > The patch is split into three parts.
> > Patch#1 Adds dma_ops for dwxgmac4 in stmmac core
> > Patch#2 Hooks in the hardware interface handling for dwxgmac4
> > Patch#3 Adds PCI driver for BCM8958x device
> > https://lore.kernel.org/netdev/20240802031822.1862030-1-jitendra.vegiraju@broadcom.com/
> >
> > v1->v2:
> > Minor fixes to address coding style issues.
> > Sent v2 too soon by mistake, without waiting for review comments.
> > Received feedback on this version.
> > https://lore.kernel.org/netdev/20240511015924.41457-1-jitendra.vegiraju@broadcom.com/
> >
> > v1:
> > https://lore.kernel.org/netdev/20240510000331.154486-1-jitendra.vegiraju@broadcom.com/
> >
> > Jitendra Vegiraju (5):
> > Add HDMA mapping for dw25gmac support
> > Add basic dw25gmac support in stmmac core
> > Integrate dw25gmac into stmmac hwif handling
> > Add PCI driver support for BCM8958x
> > Add BCM8958x driver to build system
> >
> > MAINTAINERS | 8 +
> > drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 +
> > drivers/net/ethernet/stmicro/stmmac/Makefile | 3 +-
> > drivers/net/ethernet/stmicro/stmmac/common.h | 4 +
> > .../net/ethernet/stmicro/stmmac/dw25gmac.c | 224 ++++++++
> > .../net/ethernet/stmicro/stmmac/dw25gmac.h | 92 ++++
> > .../net/ethernet/stmicro/stmmac/dwmac-brcm.c | 507 ++++++++++++++++++
> > .../net/ethernet/stmicro/stmmac/dwxgmac2.h | 1 +
> > .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 43 ++
> > .../ethernet/stmicro/stmmac/dwxgmac2_dma.c | 31 ++
> > drivers/net/ethernet/stmicro/stmmac/hwif.c | 26 +-
> > drivers/net/ethernet/stmicro/stmmac/hwif.h | 1 +
> > include/linux/stmmac.h | 48 ++
> > 13 files changed, 997 insertions(+), 2 deletions(-)
> > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw25gmac.c
> > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw25gmac.h
> > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c
>
> Hi Serge, to you think you will have time to review this series soon?
>
> We are sort in a rush to flush the net-next material before the upcoming
> merge window.
I'll get back to reviewing the series today. Sorry for the
inconvenience.
-Serge(y)
>
> Thanks,
>
> Paolo
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v5 1/5] net: stmmac: Add HDMA mapping for dw25gmac support
2024-09-04 5:48 ` [PATCH net-next v5 1/5] net: stmmac: Add HDMA mapping for dw25gmac support jitendra.vegiraju
@ 2024-09-10 18:37 ` Serge Semin
2024-09-11 21:50 ` Jitendra Vegiraju
0 siblings, 1 reply; 20+ messages in thread
From: Serge Semin @ 2024-09-10 18:37 UTC (permalink / raw)
To: jitendra.vegiraju
Cc: netdev, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, bcm-kernel-feedback-list, richardcochran, ast,
daniel, hawk, john.fastabend, rmk+kernel, ahalaney, xiaolei.wang,
rohan.g.thomas, Jianheng.Zhang, linux-kernel, linux-stm32,
linux-arm-kernel, bpf, andrew, linux, horms, florian.fainelli
Hi Jitendra
On Tue, Sep 03, 2024 at 10:48:11PM -0700, jitendra.vegiraju@broadcom.com wrote:
> From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
>
> Add hdma configuration support in include/linux/stmmac.h file.
> The hdma configuration includes mapping of virtual DMAs to physical DMAs.
> Define a new data structure stmmac_hdma_cfg to provide the mapping.
>
> Introduce new plat_stmmacenet_data::snps_id,snps_dev_id to allow glue
> drivers to specify synopsys ID and device id respectively.
> These values take precedence over reading from HW register. This facility
> provides a mechanism to use setup function from stmmac core module and yet
> override MAC.VERSION CSR if the glue driver chooses to do so.
>
> Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
> ---
> include/linux/stmmac.h | 48 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 338991c08f00..eb8136680a7b 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -89,6 +89,51 @@ struct stmmac_mdio_bus_data {
> bool needs_reset;
> };
>
> +/* DW25GMAC Hyper-DMA Overview
> + * Hyper-DMA allows support for large number of Virtual DMA(VDMA)
> + * channels using a smaller set of physical DMA channels(PDMA).
> + * This is supported by the mapping of VDMAs to Traffic Class(TC)
> + * and PDMA to TC in each traffic direction as shown below.
> + *
> + * VDMAs Traffic Class PDMA
> + * +--------+ +------+ +-----------+
> + * |VDMA0 |--------->| TC0 |-------->|PDMA0/TXQ0 |
> + *TX +--------+ |----->+------+ +-----------+
> + *Host=> +--------+ | +------+ +-----------+ => MAC
> + *SW |VDMA1 |---+ | TC1 | +--->|PDMA1/TXQ1 |
> + * +--------+ +------+ | +-----------+
> + * +--------+ +------+----+ +-----------+
> + * |VDMA2 |--------->| TC2 |-------->|PDMA2/TXQ1 |
> + * +--------+ +------+ +-----------+
> + * . . .
> + * +--------+ +------+ +-----------+
> + * |VDMAn-1 |--------->| TCx-1|-------->|PDMAm/TXQm |
> + * +--------+ +------+ +-----------+
> + *
> + * +------+ +------+ +------+
> + * |PDMA0 |--------->| TC0 |-------->|VDMA0 |
> + * +------+ |----->+------+ +------+
> + *MAC => +------+ | +------+ +------+
> + *RXQs |PDMA1 |---+ | TC1 | +--->|VDMA1 | => Host
> + * +------+ +------+ | +------+
> + * . . .
> + */
> +
> +/* Hyper-DMA mapping configuration
> + * Traffic Class associated with each VDMA/PDMA mapping
> + * is stored in corresponding array entry.
> + */
> +struct stmmac_hdma_cfg {
> + u32 tx_vdmas; /* TX VDMA count */
> + u32 rx_vdmas; /* RX VDMA count */
> + u32 tx_pdmas; /* TX PDMA count */
> + u32 rx_pdmas; /* RX PDMA count */
> + u8 *tvdma_tc; /* Tx VDMA to TC mapping array */
> + u8 *rvdma_tc; /* Rx VDMA to TC mapping array */
> + u8 *tpdma_tc; /* Tx PDMA to TC mapping array */
> + u8 *rpdma_tc; /* Rx PDMA to TC mapping array */
> +};
> +
> struct stmmac_dma_cfg {
> int pbl;
> int txpbl;
> @@ -101,6 +146,7 @@ struct stmmac_dma_cfg {
> bool multi_msi_en;
> bool dche;
> bool atds;
> + struct stmmac_hdma_cfg *hdma_cfg;
Based on what you are implementing the _static_ VDMA-TC-PDMA channels
mapping I really don't see a value of adding all of these data here.
The whole implementation gets to be needlessly overcomplicated.
Moreover AFAICS there are some channels left misconfigured in the
Patch 2 code. Please see my comments there for more details.
> };
>
> #define AXI_BLEN 7
> @@ -303,5 +349,7 @@ struct plat_stmmacenet_data {
> int msi_tx_base_vec;
> const struct dwmac4_addrs *dwmac4_addrs;
> unsigned int flags;
> + u32 snps_id;
> + u32 snps_dev_id;
Please move these fields to the head of the structure as the kind of
crucial ones, and convert snps_dev_id to just dev_id.
snps_id field name was selected based on the VERSION.SNPSVER field
name (see SNPS prefix). Following that logic the VERSION.DEVID field
should be converted to the dev_id name.
-Serge(y)
> };
> #endif
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v5 2/5] net: stmmac: Add basic dw25gmac support in stmmac core
2024-09-04 5:48 ` [PATCH net-next v5 2/5] net: stmmac: Add basic dw25gmac support in stmmac core jitendra.vegiraju
@ 2024-09-10 19:24 ` Serge Semin
2024-09-16 23:32 ` Jitendra Vegiraju
0 siblings, 1 reply; 20+ messages in thread
From: Serge Semin @ 2024-09-10 19:24 UTC (permalink / raw)
To: jitendra.vegiraju
Cc: netdev, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, bcm-kernel-feedback-list, richardcochran, ast,
daniel, hawk, john.fastabend, rmk+kernel, ahalaney, xiaolei.wang,
rohan.g.thomas, Jianheng.Zhang, linux-kernel, linux-stm32,
linux-arm-kernel, bpf, andrew, linux, horms, florian.fainelli
On Tue, Sep 03, 2024 at 10:48:12PM -0700, jitendra.vegiraju@broadcom.com wrote:
> From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
>
> The BCM8958x uses early adopter version of DWC_xgmac version 4.00a for
> Ethernet MAC. The DW25GMAC introduced in this version adds new DMA
> architecture called Hyper-DMA (HDMA) for virtualization scalability.
> This is realized by decoupling physical DMA channels(PDMA) from potentially
> large number of virtual DMA channels (VDMA). The VDMAs are software
> abstractions that map to PDMAs for frame transmission and reception.
>
> Define new macros DW25GMAC_CORE_4_00 and DW25GMAC_ID to identify 25GMAC
> device.
> To support the new HDMA architecture, a new instance of stmmac_dma_ops
> dw25gmac400_dma_ops is added.
> Most of the other dma operation functions in existing dwxgamc2_dma.c file
> are reused where applicable.
> Added setup function for DW25GMAC's stmmac_hwif_entry in stmmac core.
>
> Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +-
> drivers/net/ethernet/stmicro/stmmac/common.h | 4 +
> .../net/ethernet/stmicro/stmmac/dw25gmac.c | 224 ++++++++++++++++++
> .../net/ethernet/stmicro/stmmac/dw25gmac.h | 92 +++++++
> .../net/ethernet/stmicro/stmmac/dwxgmac2.h | 1 +
> .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 43 ++++
> .../ethernet/stmicro/stmmac/dwxgmac2_dma.c | 31 +++
> drivers/net/ethernet/stmicro/stmmac/hwif.h | 1 +
> 8 files changed, 397 insertions(+), 1 deletion(-)
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw25gmac.c
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw25gmac.h
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index c2f0e91f6bf8..967e8a9aa432 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -6,7 +6,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o \
> mmc_core.o stmmac_hwtstamp.o stmmac_ptp.o dwmac4_descs.o \
> dwmac4_dma.o dwmac4_lib.o dwmac4_core.o dwmac5.o hwif.o \
> stmmac_tc.o dwxgmac2_core.o dwxgmac2_dma.o dwxgmac2_descs.o \
> - stmmac_xdp.o stmmac_est.o \
> + stmmac_xdp.o stmmac_est.o dw25gmac.o \
> $(stmmac-y)
>
> stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index 684489156dce..9a747b89ba51 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -38,9 +38,11 @@
> #define DWXGMAC_CORE_2_10 0x21
> #define DWXGMAC_CORE_2_20 0x22
> #define DWXLGMAC_CORE_2_00 0x20
> +#define DW25GMAC_CORE_4_00 0x40
>
> /* Device ID */
> #define DWXGMAC_ID 0x76
> +#define DW25GMAC_ID 0x55
> #define DWXLGMAC_ID 0x27
>
> #define STMMAC_CHAN0 0 /* Always supported and default for all chips */
> @@ -563,6 +565,7 @@ struct mac_link {
> u32 speed2500;
> u32 speed5000;
> u32 speed10000;
> + u32 speed25000;
> } xgmii;
> struct {
> u32 speed25000;
> @@ -621,6 +624,7 @@ int dwmac100_setup(struct stmmac_priv *priv);
> int dwmac1000_setup(struct stmmac_priv *priv);
> int dwmac4_setup(struct stmmac_priv *priv);
> int dwxgmac2_setup(struct stmmac_priv *priv);
> +int dw25gmac_setup(struct stmmac_priv *priv);
> int dwxlgmac2_setup(struct stmmac_priv *priv);
>
> void stmmac_set_mac_addr(void __iomem *ioaddr, const u8 addr[6],
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dw25gmac.c b/drivers/net/ethernet/stmicro/stmmac/dw25gmac.c
> new file mode 100644
> index 000000000000..adb33700ffbb
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dw25gmac.c
> @@ -0,0 +1,224 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2024 Broadcom Corporation
> + */
> +#include "stmmac.h"
> +#include "dwxgmac2.h"
> +#include "dw25gmac.h"
> +
> +static u32 decode_vdma_count(u32 regval)
> +{
> + /* compressed encoding for vdma count
> + * regval: VDMA count
> + * 0-15 : 1 - 16
> + * 16-19 : 20, 24, 28, 32
> + * 20-23 : 40, 48, 56, 64
> + * 24-27 : 80, 96, 112, 128
> + */
> + if (regval < 16)
> + return regval + 1;
> + return (4 << ((regval - 16) / 4)) * ((regval % 4) + 5);
The shortest code isn't always the best one. This one gives me a
headache in trying to decipher whether it really matches to what is
described in the comment. What about just:
if (regval < 16) /* Direct mapping */
return regval + 1;
else if (regval < 20) /* 20, 24, 28, 32 */
return 20 + (regval - 16) * 4;
else if (regval < 24) /* 40, 48, 56, 64 */
return 40 + (regval - 20) * 8;
else if (regval < 28) /* 80, 96, 112, 128 */
return 80 + (regval - 24) * 16;
?
> +}
> +
> +static void dw25gmac_read_hdma_limits(void __iomem *ioaddr,
> + struct stmmac_hdma_cfg *hdma)
> +{
> + u32 hw_cap;
> +
> + /* Get VDMA/PDMA counts from HW */
> + hw_cap = readl(ioaddr + XGMAC_HW_FEATURE2);
> + hdma->tx_vdmas = decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_TXCNT,
> + hw_cap));
> + hdma->rx_vdmas = decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_RXCNT,
> + hw_cap));
> + hdma->tx_pdmas = FIELD_GET(XGMAC_HWFEAT_TXQCNT, hw_cap) + 1;
> + hdma->rx_pdmas = FIELD_GET(XGMAC_HWFEAT_RXQCNT, hw_cap) + 1;
Hmm, these are the Tx/Rx DMA-channels and Tx/Rx MTL-queues count
fields. Can't you just use the
dma_features::{number_tx_channel,number_tx_queues} and
dma_features::{number_rx_channel,number_rx_queues} fields to store the
retrieved data?
Moreover why not to add the code above to the dwxgmac2_get_hw_feature() method?
> +}
> +
> +int dw25gmac_hdma_cfg_init(struct stmmac_priv *priv)
> +{
> + struct plat_stmmacenet_data *plat = priv->plat;
> + struct device *dev = priv->device;
> + struct stmmac_hdma_cfg *hdma;
> + int i;
> +
> + hdma = devm_kzalloc(dev,
> + sizeof(*plat->dma_cfg->hdma_cfg),
> + GFP_KERNEL);
> + if (!hdma)
> + return -ENOMEM;
> +
> + dw25gmac_read_hdma_limits(priv->ioaddr, hdma);
> +
> + hdma->tvdma_tc = devm_kzalloc(dev,
> + sizeof(*hdma->tvdma_tc) * hdma->tx_vdmas,
> + GFP_KERNEL);
> + if (!hdma->tvdma_tc)
> + return -ENOMEM;
> +
> + hdma->rvdma_tc = devm_kzalloc(dev,
> + sizeof(*hdma->rvdma_tc) * hdma->rx_vdmas,
> + GFP_KERNEL);
> + if (!hdma->rvdma_tc)
> + return -ENOMEM;
> +
> + hdma->tpdma_tc = devm_kzalloc(dev,
> + sizeof(*hdma->tpdma_tc) * hdma->tx_pdmas,
> + GFP_KERNEL);
> + if (!hdma->tpdma_tc)
> + return -ENOMEM;
> +
> + hdma->rpdma_tc = devm_kzalloc(dev,
> + sizeof(*hdma->rpdma_tc) * hdma->rx_pdmas,
> + GFP_KERNEL);
> + if (!hdma->rpdma_tc)
> + return -ENOMEM;
> +
> + /* Initialize one-to-one mapping for each of the used queues */
> + for (i = 0; i < plat->tx_queues_to_use; i++) {
> + hdma->tvdma_tc[i] = i;
> + hdma->tpdma_tc[i] = i;
> + }
> + for (i = 0; i < plat->rx_queues_to_use; i++) {
> + hdma->rvdma_tc[i] = i;
> + hdma->rpdma_tc[i] = i;
> + }
So the Traffic Class ID is initialized for the
tx_queues_to_use/rx_queues_to_use number of channels only, right? What
about the Virtual and Physical DMA-channels with numbers greater than
these values?
> + plat->dma_cfg->hdma_cfg = hdma;
> +
> + return 0;
> +}
> +
> +static int rd_dma_ch_ind(void __iomem *ioaddr, u8 mode, u32 channel)
> +{
> + u32 reg_val = 0;
> +
> + reg_val |= FIELD_PREP(XXVGMAC_MODE_SELECT, mode);
> + reg_val |= FIELD_PREP(XXVGMAC_ADDR_OFFSET, channel);
> + reg_val |= XXVGMAC_CMD_TYPE | XXVGMAC_OB;
> + writel(reg_val, ioaddr + XXVGMAC_DMA_CH_IND_CONTROL);
> + return readl(ioaddr + XXVGMAC_DMA_CH_IND_DATA);
> +}
> +
> +static void wr_dma_ch_ind(void __iomem *ioaddr, u8 mode, u32 channel, u32 val)
> +{
> + u32 reg_val = 0;
> +
> + writel(val, ioaddr + XXVGMAC_DMA_CH_IND_DATA);
> + reg_val |= FIELD_PREP(XXVGMAC_MODE_SELECT, mode);
> + reg_val |= FIELD_PREP(XXVGMAC_ADDR_OFFSET, channel);
> + reg_val |= XGMAC_OB;
> + writel(reg_val, ioaddr + XXVGMAC_DMA_CH_IND_CONTROL);
> +}
> +
> +static void xgmac4_tp2tc_map(void __iomem *ioaddr, u8 pdma_ch, u32 tc_num)
> +{
> + u32 val = 0;
> +
> + val = rd_dma_ch_ind(ioaddr, MODE_TXEXTCFG, pdma_ch);
> + val &= ~XXVGMAC_TP2TCMP;
> + val |= FIELD_PREP(XXVGMAC_TP2TCMP, tc_num);
> + wr_dma_ch_ind(ioaddr, MODE_TXEXTCFG, pdma_ch, val);
> +}
> +
> +static void xgmac4_rp2tc_map(void __iomem *ioaddr, u8 pdma_ch, u32 tc_num)
> +{
> + u32 val = 0;
> +
> + val = rd_dma_ch_ind(ioaddr, MODE_RXEXTCFG, pdma_ch);
> + val &= ~XXVGMAC_RP2TCMP;
> + val |= FIELD_PREP(XXVGMAC_RP2TCMP, tc_num);
> + wr_dma_ch_ind(ioaddr, MODE_RXEXTCFG, pdma_ch, val);
> +}
> +
> +void dw25gmac_dma_init(void __iomem *ioaddr,
> + struct stmmac_dma_cfg *dma_cfg)
> +{
> + u32 value;
> + u32 i;
> +
> + value = readl(ioaddr + XGMAC_DMA_SYSBUS_MODE);
> + value &= ~(XGMAC_AAL | XGMAC_EAME);
> + if (dma_cfg->aal)
> + value |= XGMAC_AAL;
> + if (dma_cfg->eame)
> + value |= XGMAC_EAME;
> + writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
> +
> + for (i = 0; i < dma_cfg->hdma_cfg->tx_vdmas; i++) {
> + value = rd_dma_ch_ind(ioaddr, MODE_TXDESCCTRL, i);
> + value &= ~XXVGMAC_TXDCSZ;
> + value |= FIELD_PREP(XXVGMAC_TXDCSZ,
> + XXVGMAC_TXDCSZ_256BYTES);
> + value &= ~XXVGMAC_TDPS;
> + value |= FIELD_PREP(XXVGMAC_TDPS, XXVGMAC_TDPS_HALF);
> + wr_dma_ch_ind(ioaddr, MODE_TXDESCCTRL, i, value);
> + }
> +
> + for (i = 0; i < dma_cfg->hdma_cfg->rx_vdmas; i++) {
> + value = rd_dma_ch_ind(ioaddr, MODE_RXDESCCTRL, i);
> + value &= ~XXVGMAC_RXDCSZ;
> + value |= FIELD_PREP(XXVGMAC_RXDCSZ,
> + XXVGMAC_RXDCSZ_256BYTES);
> + value &= ~XXVGMAC_RDPS;
> + value |= FIELD_PREP(XXVGMAC_TDPS, XXVGMAC_RDPS_HALF);
> + wr_dma_ch_ind(ioaddr, MODE_RXDESCCTRL, i, value);
> + }
> +
> + for (i = 0; i < dma_cfg->hdma_cfg->tx_pdmas; i++) {
> + value = rd_dma_ch_ind(ioaddr, MODE_TXEXTCFG, i);
> + value &= ~(XXVGMAC_TXPBL | XXVGMAC_TPBLX8_MODE);
> + if (dma_cfg->pblx8)
> + value |= XXVGMAC_TPBLX8_MODE;
> + value |= FIELD_PREP(XXVGMAC_TXPBL, dma_cfg->pbl);
> + wr_dma_ch_ind(ioaddr, MODE_TXEXTCFG, i, value);
> + xgmac4_tp2tc_map(ioaddr, i, dma_cfg->hdma_cfg->tpdma_tc[i]);
> + }
> +
> + for (i = 0; i < dma_cfg->hdma_cfg->rx_pdmas; i++) {
> + value = rd_dma_ch_ind(ioaddr, MODE_RXEXTCFG, i);
> + value &= ~(XXVGMAC_RXPBL | XXVGMAC_RPBLX8_MODE);
> + if (dma_cfg->pblx8)
> + value |= XXVGMAC_RPBLX8_MODE;
> + value |= FIELD_PREP(XXVGMAC_RXPBL, dma_cfg->pbl);
> + wr_dma_ch_ind(ioaddr, MODE_RXEXTCFG, i, value);
> + xgmac4_rp2tc_map(ioaddr, i, dma_cfg->hdma_cfg->rpdma_tc[i]);
What if tx_pdmas doesn't match plat_stmmacenet_data::tx_queues_to_use
and rx_pdmas doesn't match to plat_stmmacenet_data::rx_queues_to_use?
If they don't then you'll get out of the initialized tpdma_tc/rpdma_tc
fields and these channels will be pre-initialized with the zero TC. Is
that what expected? I doubt so.
> + }
> +}
> +
> +void dw25gmac_dma_init_tx_chan(struct stmmac_priv *priv,
> + void __iomem *ioaddr,
> + struct stmmac_dma_cfg *dma_cfg,
> + dma_addr_t dma_addr, u32 chan)
> +{
> + u32 value;
> +
> + value = readl(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
> + value &= ~XXVGMAC_TVDMA2TCMP;
> + value |= FIELD_PREP(XXVGMAC_TVDMA2TCMP,
> + dma_cfg->hdma_cfg->tvdma_tc[chan]);
> + writel(value, ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
Please note this will have only first
plat_stmmacenet_data::{tx_queues_to_use,rx_queues_to_use} VDMA
channels initialized. Don't you have much more than just 4 channels?
> +
> + writel(upper_32_bits(dma_addr),
> + ioaddr + XGMAC_DMA_CH_TxDESC_HADDR(chan));
> + writel(lower_32_bits(dma_addr),
> + ioaddr + XGMAC_DMA_CH_TxDESC_LADDR(chan));
> +}
> +
> +void dw25gmac_dma_init_rx_chan(struct stmmac_priv *priv,
> + void __iomem *ioaddr,
> + struct stmmac_dma_cfg *dma_cfg,
> + dma_addr_t dma_addr, u32 chan)
> +{
> + u32 value;
> +
> + value = readl(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
> + value &= ~XXVGMAC_RVDMA2TCMP;
> + value |= FIELD_PREP(XXVGMAC_RVDMA2TCMP,
> + dma_cfg->hdma_cfg->rvdma_tc[chan]);
> + writel(value, ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
The same question.
> +
> + writel(upper_32_bits(dma_addr),
> + ioaddr + XGMAC_DMA_CH_RxDESC_HADDR(chan));
> + writel(lower_32_bits(dma_addr),
> + ioaddr + XGMAC_DMA_CH_RxDESC_LADDR(chan));
> +}
These methods are called for each
plat_stmmacenet_data::{tx_queues_to_use,rx_queues_to_use}
DMA-channel/Queue. The static mapping means you'll have each
PDMA/Queue assigned a static traffic class ID corresponding to the
channel ID. Meanwhile the VDMA channels are supposed to be initialized
with the TC ID corresponding to the matching PDMA ID.
The TC ID in this case is passed as the DMA/Queue channel ID. Then the
Tx/Rx DMA-channels init methods can be converted to:
dw25gmac_dma_init_Xx_chan(chan)
{
/* Map each chan-th VDMA to the single chan PDMA by assigning
* the static TC ID.
*/
for (i = chan; i < Xx_vdmas; i += (Xx_vdmas / Xx_queues_to_use)) {
/* Initialize VDMA channels */
XXVGMAC_TVDMA2TCMP = chan;
}
/* Assign the static TC ID to the specified PDMA channel */
xgmac4_rp2tc_map(chan, chan)
}
, where X={t,r}.
Thus you can redistribute the loops implemented in dw25gmac_dma_init()
to the respective Tx/Rx DMA-channel init methods.
Am I missing something?
-Serge()
> [...]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v5 4/5] net: stmmac: Add PCI driver support for BCM8958x
2024-09-04 5:48 ` [PATCH net-next v5 4/5] net: stmmac: Add PCI driver support for BCM8958x jitendra.vegiraju
@ 2024-09-10 19:51 ` Serge Semin
2024-09-16 23:43 ` Jitendra Vegiraju
0 siblings, 1 reply; 20+ messages in thread
From: Serge Semin @ 2024-09-10 19:51 UTC (permalink / raw)
To: jitendra.vegiraju
Cc: netdev, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, bcm-kernel-feedback-list, richardcochran, ast,
daniel, hawk, john.fastabend, rmk+kernel, ahalaney, xiaolei.wang,
rohan.g.thomas, Jianheng.Zhang, linux-kernel, linux-stm32,
linux-arm-kernel, bpf, andrew, linux, horms, florian.fainelli
On Tue, Sep 03, 2024 at 10:48:14PM -0700, jitendra.vegiraju@broadcom.com wrote:
> From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
>
> Add PCI ethernet driver support for Broadcom BCM8958x SoC devices used
> in automotive applications.
>
> This SoC device has PCIe ethernet MAC attached to an integrated ethernet
> switch using XGMII interface. The PCIe ethernet controller is presented to
> the Linux host as PCI network device.
>
> The following block diagram gives an overview of the application.
> +=================================+
> | Host CPU/Linux |
> +=================================+
> || PCIe
> ||
> +==========================================+
> | +--------------+ |
> | | PCIE Endpoint| |
> | | Ethernet | |
> | | Controller | |
> | | DMA | |
> | +--------------+ |
> | | MAC | BCM8958X |
> | +--------------+ SoC |
> | || XGMII |
> | || |
> | +--------------+ |
> | | Ethernet | |
> | | switch | |
> | +--------------+ |
> | || || || || |
> +==========================================+
> || || || || More external interfaces
>
> The MAC IP block on BCM8958x is based on Synopsis XGMAC 4.00a core. This
> driver uses common dwxgmac2 code where applicable.
> Driver functionality specific to this MAC is implemented in dw25gmac.c.
> The glue driver is responsible for setting up hdma mappings.
>
> Management of integrated ethernet switch on this SoC is not handled by
> the PCIe interface.
> Since BCM8958x is an early adopter device, override the hardware reported
> synopsis versions with actual DW25MAC versions that support hdma.
>
> This SoC device has PCIe ethernet MAC directly attached to an integrated
> ethernet switch using XGMII interface. Since device tree support is not
> available on this platform, a software node is created to enable
> fixed-link support using phylink driver.
>
> Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
> ---
> .../net/ethernet/stmicro/stmmac/dwmac-brcm.c | 507 ++++++++++++++++++
> 1 file changed, 507 insertions(+)
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c
> new file mode 100644
> index 000000000000..b5dd97d9b938
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c
> @@ -0,0 +1,507 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (c) 2024 Broadcom Corporation
> + *
> + * PCI driver for ethernet interface of BCM8958X automotive switch chip.
> + *
> + * High level block diagram of the device.
> + * +=================================+
> + * | Host CPU/Linux |
> + * +=================================+
> + * || PCIe
> + * ||
> + * +==========================================+
> + * | +--------------+ |
> + * | | PCIE Endpoint| |
> + * | | Ethernet | |
> + * | | Controller | |
> + * | | DMA | |
> + * | +--------------+ |
> + * | | MAC | BCM8958X |
> + * | +--------------+ SoC |
> + * | || XGMII |
> + * | || |
> + * | +--------------+ |
> + * | | Ethernet | |
> + * | | switch | |
> + * | +--------------+ |
> + * | || || || || |
> + * +==========================================+
> + * || || || || More external interfaces
> + *
> + * This SoC device has PCIe ethernet MAC directly attached to an integrated
> + * ethernet switch using XGMII interface. Since devicetree support is not
> + * available on this platform, a software node is created to enable
> + * fixed-link support using phylink driver.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/dmi.h>
> +#include <linux/pci.h>
> +#include <linux/phy.h>
> +
> +#include "stmmac.h"
> +#include "dwxgmac2.h"
> +#include "dw25gmac.h"
> +
> +#define PCI_DEVICE_ID_BROADCOM_BCM8958X 0xa00d
> +#define BRCM_MAX_MTU 1500
> +#define READ_POLL_DELAY_US 100
> +#define READ_POLL_TIMEOUT_US 10000
These macros are unused. Why do you need them here?
> +#define DWMAC_125MHZ 125000000
> +#define DWMAC_250MHZ 250000000
Drop these and use the literals directly.
> +#define BRCM_XGMAC_NUM_VLAN_FILTERS 32
> +
> +/* TX and RX Queue counts */
> +#define BRCM_TX_Q_COUNT 4
> +#define BRCM_RX_Q_COUNT 4
> +
> +#define BRCM_XGMAC_DMA_TX_SIZE 1024
> +#define BRCM_XGMAC_DMA_RX_SIZE 1024
Unused.
> +#define BRCM_XGMAC_BAR0_MASK BIT(0)
> +
> +#define BRCM_XGMAC_IOMEM_MISC_REG_OFFSET 0x0
> +#define BRCM_XGMAC_IOMEM_MBOX_REG_OFFSET 0x1000
> +#define BRCM_XGMAC_IOMEM_CFG_REG_OFFSET 0x3000
> +
> +#define XGMAC_MMC_CTRL_RCHM_DISABLE BIT(31)
> +#define XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LOW 0x940
> +#define XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LO_VALUE 0x00000001
> +#define XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HIGH 0x944
> +#define XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HI_VALUE 0x88000000
> +
> +#define XGMAC_PCIE_MISC_MII_CTRL_OFFSET 0x4
> +#define XGMAC_PCIE_MISC_MII_CTRL_PAUSE_RX BIT(0)
> +#define XGMAC_PCIE_MISC_MII_CTRL_PAUSE_TX BIT(1)
> +#define XGMAC_PCIE_MISC_MII_CTRL_LINK_UP BIT(2)
> +#define XGMAC_PCIE_MISC_PCIESS_CTRL_OFFSET 0x8
> +#define XGMAC_PCIE_MISC_PCIESS_CTRL_EN_MSI_MSIX BIT(9)
> +#define XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_OFFSET 0x90
> +#define XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_VALUE 0x00000001
> +#define XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_OFFSET 0x94
> +#define XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_VALUE 0x88000000
> +#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST0_OFFSET 0x700
> +#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST0_VALUE 1
> +#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST1_OFFSET 0x704
> +#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST1_VALUE 1
> +#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST_DBELL_OFFSET 0x728
> +#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST_DBELL_VALUE 1
> +#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_SBD_ALL_OFFSET 0x740
> +#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_SBD_ALL_VALUE 0
> +
> +#define XGMAC_PCIE_MISC_FUNC_RESOURCES_PF0_OFFSET 0x804
> +
> +/* MSIX Vector map register starting offsets */
> +#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_RX0_PF0_OFFSET 0x840
> +#define XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_TX0_PF0_OFFSET 0x890
> +#define BRCM_MAX_DMA_CHANNEL_PAIRS 4
> +
> +#define BRCM_XGMAC_MSI_MAC_VECTOR 0
> +#define BRCM_XGMAC_MSI_RX_VECTOR_START 9
> +#define BRCM_XGMAC_MSI_TX_VECTOR_START 10
> +
> +static char *fixed_link_node_name = "fixed-link";
> +
> +static const struct property_entry fixed_link_properties[] = {
> + PROPERTY_ENTRY_U32("speed", 10000),
> + PROPERTY_ENTRY_BOOL("full-duplex"),
> + PROPERTY_ENTRY_BOOL("pause"),
> + { }
> +};
> +
> +struct brcm_priv_data {
> + void __iomem *mbox_regs; /* MBOX Registers*/
> + void __iomem *misc_regs; /* MISC Registers*/
> + void __iomem *xgmac_regs; /* XGMAC Registers*/
> + struct software_node fixed_link_node;
> +};
> +
> +struct dwxgmac_brcm_pci_info {
> + int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat);
> +};
> +
> +static void misc_iowrite(struct brcm_priv_data *brcm_priv,
> + u32 reg, u32 val)
> +{
> + iowrite32(val, brcm_priv->misc_regs + reg);
> +}
> +
> +static void dwxgmac_brcm_common_default_data(struct plat_stmmacenet_data *plat)
> +{
> + int i;
> +
> + plat->has_xgmac = 1;
> + plat->force_sf_dma_mode = 1;
> + plat->mac_port_sel_speed = SPEED_10000;
> + plat->clk_ptp_rate = DWMAC_125MHZ;
> + plat->clk_ref_rate = DWMAC_250MHZ;
Just 125000000 and 250000000. There is no need in defining the macro
with the names matching the numerical literals.
> + plat->tx_coe = 1;
> + plat->rx_coe = 1;
> + plat->max_speed = SPEED_10000;
> +
> + /* Set default value for multicast hash bins */
> + plat->multicast_filter_bins = HASH_TABLE_SIZE;
> +
> + /* Set default value for unicast filter entries */
> + plat->unicast_filter_entries = 1;
> +
> + /* Set the maxmtu to device's default */
> + plat->maxmtu = BRCM_MAX_MTU;
> +
> + /* Set default number of RX and TX queues to use */
> + plat->tx_queues_to_use = BRCM_TX_Q_COUNT;
> + plat->rx_queues_to_use = BRCM_RX_Q_COUNT;
> +
> + plat->tx_sched_algorithm = MTL_TX_ALGORITHM_SP;
> + for (i = 0; i < plat->tx_queues_to_use; i++) {
> + plat->tx_queues_cfg[i].use_prio = false;
> + plat->tx_queues_cfg[i].prio = 0;
> + plat->tx_queues_cfg[i].mode_to_use = MTL_QUEUE_AVB;
> + }
> +
> + plat->rx_sched_algorithm = MTL_RX_ALGORITHM_SP;
> + for (i = 0; i < plat->rx_queues_to_use; i++) {
> + plat->rx_queues_cfg[i].use_prio = false;
> + plat->rx_queues_cfg[i].mode_to_use = MTL_QUEUE_AVB;
> + plat->rx_queues_cfg[i].pkt_route = 0x0;
> + plat->rx_queues_cfg[i].chan = i;
> + }
> +}
> +
> +static int dwxgmac_brcm_default_data(struct pci_dev *pdev,
> + struct plat_stmmacenet_data *plat)
> +{
> + /* Set common default data first */
> + dwxgmac_brcm_common_default_data(plat);
> +
> + plat->snps_id = DW25GMAC_CORE_4_00;
> + plat->snps_dev_id = DW25GMAC_ID;
> + plat->bus_id = 0;
> + plat->phy_addr = 0;
> + plat->phy_interface = PHY_INTERFACE_MODE_USXGMII;
Really, USXGMII? Universal Serial XGMII? Synopsys call it just XGMII:
https://www.synopsys.com/dw/ipdir.php?ds=dwc_25g_ethernet_mac_ip
> +
> + plat->dma_cfg->pbl = 32;
> + plat->dma_cfg->pblx8 = 0;
> + plat->dma_cfg->aal = 0;
> + plat->dma_cfg->eame = 1;
> +
> + plat->axi->axi_wr_osr_lmt = 31;
> + plat->axi->axi_rd_osr_lmt = 31;
> + plat->axi->axi_fb = 0;
> + plat->axi->axi_blen[0] = 4;
> + plat->axi->axi_blen[1] = 8;
> + plat->axi->axi_blen[2] = 16;
> + plat->axi->axi_blen[3] = 32;
> + plat->axi->axi_blen[4] = 64;
> + plat->axi->axi_blen[5] = 128;
> + plat->axi->axi_blen[6] = 256;
> +
> + plat->msi_mac_vec = BRCM_XGMAC_MSI_MAC_VECTOR;
> + plat->msi_rx_base_vec = BRCM_XGMAC_MSI_RX_VECTOR_START;
> + plat->msi_tx_base_vec = BRCM_XGMAC_MSI_TX_VECTOR_START;
Please see my next comments about these fields utilization.
> +
> + return 0;
> +}
> +
> +static struct dwxgmac_brcm_pci_info dwxgmac_brcm_pci_info = {
> + .setup = dwxgmac_brcm_default_data,
> +};
> +
> +static void brcm_config_misc_regs(struct pci_dev *pdev,
> + struct brcm_priv_data *brcm_priv)
> +{
> + pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LOW,
> + XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LO_VALUE);
> + pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HIGH,
> + XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HI_VALUE);
> +
> + misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_OFFSET,
> + XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_VALUE);
> + misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_OFFSET,
> + XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_VALUE);
> +
> + /* Enable Switch Link */
> + misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MII_CTRL_OFFSET,
> + XGMAC_PCIE_MISC_MII_CTRL_PAUSE_RX |
> + XGMAC_PCIE_MISC_MII_CTRL_PAUSE_TX |
> + XGMAC_PCIE_MISC_MII_CTRL_LINK_UP);
> +}
> +
> +static int brcm_config_multi_msi(struct pci_dev *pdev,
> + struct plat_stmmacenet_data *plat,
> + struct stmmac_resources *res)
> +{
> + int ret, i;
> +
> + if (plat->msi_rx_base_vec >= STMMAC_MSI_VEC_MAX ||
> + plat->msi_tx_base_vec >= STMMAC_MSI_VEC_MAX) {
Please see my next comment about these fields and STMMAC_MSI_VEC_MAX
utilization.
> + dev_err(&pdev->dev, "%s: Invalid RX & TX vector defined\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + ret = pci_alloc_irq_vectors(pdev, 2, STMMAC_MSI_VEC_MAX,
> + PCI_IRQ_MSI | PCI_IRQ_MSIX);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "%s: multi MSI enablement failed\n",
> + __func__);
> + return ret;
> + }
> +
> + /* For RX MSI */
> + for (i = 0; i < plat->rx_queues_to_use; i++)
> + res->rx_irq[i] = pci_irq_vector(pdev,
> + plat->msi_rx_base_vec + i * 2);
> +
> + /* For TX MSI */
> + for (i = 0; i < plat->tx_queues_to_use; i++)
> + res->tx_irq[i] = pci_irq_vector(pdev,
> + plat->msi_tx_base_vec + i * 2);
> +
> + if (plat->msi_mac_vec < STMMAC_MSI_VEC_MAX)
> + res->irq = pci_irq_vector(pdev, plat->msi_mac_vec);
What if msi_mac_vec is greater than STMMAC_MSI_VEC_MAX? Will your
device work without delivering the MAC IRQs? I doubt so.
In anyway see my next comment.
> +
> + plat->flags |= STMMAC_FLAG_MULTI_MSI_EN;
> + plat->flags |= STMMAC_FLAG_TSO_EN;
> +
> + return 0;
> +}
> +
> +static int dwxgmac_brcm_pci_probe(struct pci_dev *pdev,
> + const struct pci_device_id *id)
> +{
> + struct dwxgmac_brcm_pci_info *info =
> + (struct dwxgmac_brcm_pci_info *)id->driver_data;
> + struct plat_stmmacenet_data *plat;
> + struct brcm_priv_data *brcm_priv;
> + struct stmmac_resources res;
> + struct device *dev;
> + int rx_offset;
> + int tx_offset;
> + int vector;
> + int ret;
> +
> + dev = &pdev->dev;
> +
> + brcm_priv = devm_kzalloc(&pdev->dev, sizeof(*brcm_priv), GFP_KERNEL);
> + if (!brcm_priv)
> + return -ENOMEM;
> +
> + plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL);
> + if (!plat)
> + return -ENOMEM;
> +
> + plat->dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*plat->dma_cfg),
> + GFP_KERNEL);
> + if (!plat->dma_cfg)
> + return -ENOMEM;
> +
> + plat->axi = devm_kzalloc(&pdev->dev, sizeof(*plat->axi), GFP_KERNEL);
> + if (!plat->axi)
> + return -ENOMEM;
> +
> + /* This device is directly attached to the switch chip internal to the
> + * SoC using XGMII interface. Since no MDIO is present, register
> + * fixed-link software_node to create phylink.
> + */
> + plat->port_node = fwnode_create_software_node(NULL, NULL);
> + brcm_priv->fixed_link_node.name = fixed_link_node_name;
> + brcm_priv->fixed_link_node.properties = fixed_link_properties;
> + brcm_priv->fixed_link_node.parent = to_software_node(plat->port_node);
> + device_add_software_node(dev, &brcm_priv->fixed_link_node);
> +
> + /* Disable D3COLD as our device does not support it */
> + pci_d3cold_disable(pdev);
> +
> + /* Enable PCI device */
> + ret = pcim_enable_device(pdev);
> + if (ret) {
> + dev_err(&pdev->dev, "%s: ERROR: failed to enable device\n",
> + __func__);
> + return ret;
> + }
> +
> + /* Get the base address of device */
> + ret = pcim_iomap_regions(pdev, BRCM_XGMAC_BAR0_MASK, pci_name(pdev));
> + if (ret)
> + return ret;
> + pci_set_master(pdev);
> +
> + memset(&res, 0, sizeof(res));
> + res.addr = pcim_iomap_table(pdev)[0];
> + /* MISC Regs */
> + brcm_priv->misc_regs = res.addr + BRCM_XGMAC_IOMEM_MISC_REG_OFFSET;
> + /* MBOX Regs */
> + brcm_priv->mbox_regs = res.addr + BRCM_XGMAC_IOMEM_MBOX_REG_OFFSET;
> + /* XGMAC config Regs */
> + res.addr += BRCM_XGMAC_IOMEM_CFG_REG_OFFSET;
> + brcm_priv->xgmac_regs = res.addr;
> +
> + plat->bsp_priv = brcm_priv;
> +
> + /* Initialize all MSI vectors to invalid so that it can be set
> + * according to platform data settings below.
> + * Note: MSI vector takes value from 0 up to 31 (STMMAC_MSI_VEC_MAX)
> + */
> + plat->msi_mac_vec = STMMAC_MSI_VEC_MAX;
> + plat->msi_wol_vec = STMMAC_MSI_VEC_MAX;
> + plat->msi_lpi_vec = STMMAC_MSI_VEC_MAX;
> + plat->msi_sfty_ce_vec = STMMAC_MSI_VEC_MAX;
> + plat->msi_sfty_ue_vec = STMMAC_MSI_VEC_MAX;
> + plat->msi_rx_base_vec = STMMAC_MSI_VEC_MAX;
> + plat->msi_tx_base_vec = STMMAC_MSI_VEC_MAX;
Please don't use these fields and the STMMAC_MSI_VEC_MAX macro. Either
have the BRCM_XGMAC_MSI* macros utilized directly or define the
device-specific data in the glue driver (in brcm_priv_data if you
wish). Really the MSI vectors aren't related to any DW *MAC IP-core
these are the pure vendor platform-specific settings.
The fields originally have been introduced by the Intel developers,
who AFAICS just found it easier to extend the generic platform-data
structure instead of introducing the new Intel MAC-specific data.
I am going to drop these fields in a future cleanup patchset so to
reduce the plat_stmmacenet_data structure complexity.
-Serge(y)
> [...]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v5 1/5] net: stmmac: Add HDMA mapping for dw25gmac support
2024-09-10 18:37 ` Serge Semin
@ 2024-09-11 21:50 ` Jitendra Vegiraju
0 siblings, 0 replies; 20+ messages in thread
From: Jitendra Vegiraju @ 2024-09-11 21:50 UTC (permalink / raw)
To: Serge Semin
Cc: netdev, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, bcm-kernel-feedback-list, richardcochran, ast,
daniel, hawk, john.fastabend, rmk+kernel, ahalaney, xiaolei.wang,
rohan.g.thomas, Jianheng.Zhang, linux-kernel, linux-stm32,
linux-arm-kernel, bpf, andrew, linux, horms, florian.fainelli
Hi Serge,
Thank you for taking the time to review the patches.
On Tue, Sep 10, 2024 at 11:37 AM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> Hi Jitendra
>
> On Tue, Sep 03, 2024 at 10:48:11PM -0700, jitendra.vegiraju@broadcom.com wrote:
> > From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
> >
> > Add hdma configuration support in include/linux/stmmac.h file.
> > The hdma configuration includes mapping of virtual DMAs to physical DMAs.
> > Define a new data structure stmmac_hdma_cfg to provide the mapping.
> >
> > Introduce new plat_stmmacenet_data::snps_id,snps_dev_id to allow glue
> > drivers to specify synopsys ID and device id respectively.
> > These values take precedence over reading from HW register. This facility
> > provides a mechanism to use setup function from stmmac core module and yet
> > override MAC.VERSION CSR if the glue driver chooses to do so.
> >
> > Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
> > ---
> > include/linux/stmmac.h | 48 ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 48 insertions(+)
> >
> > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> > index 338991c08f00..eb8136680a7b 100644
> > --- a/include/linux/stmmac.h
> > +++ b/include/linux/stmmac.h
> > @@ -89,6 +89,51 @@ struct stmmac_mdio_bus_data {
> > bool needs_reset;
> > };
> >
> > +/* DW25GMAC Hyper-DMA Overview
> > + * Hyper-DMA allows support for large number of Virtual DMA(VDMA)
> > + * channels using a smaller set of physical DMA channels(PDMA).
> > + * This is supported by the mapping of VDMAs to Traffic Class(TC)
> > + * and PDMA to TC in each traffic direction as shown below.
> > + *
> > + * VDMAs Traffic Class PDMA
> > + * +--------+ +------+ +-----------+
> > + * |VDMA0 |--------->| TC0 |-------->|PDMA0/TXQ0 |
> > + *TX +--------+ |----->+------+ +-----------+
> > + *Host=> +--------+ | +------+ +-----------+ => MAC
> > + *SW |VDMA1 |---+ | TC1 | +--->|PDMA1/TXQ1 |
> > + * +--------+ +------+ | +-----------+
> > + * +--------+ +------+----+ +-----------+
> > + * |VDMA2 |--------->| TC2 |-------->|PDMA2/TXQ1 |
> > + * +--------+ +------+ +-----------+
> > + * . . .
> > + * +--------+ +------+ +-----------+
> > + * |VDMAn-1 |--------->| TCx-1|-------->|PDMAm/TXQm |
> > + * +--------+ +------+ +-----------+
> > + *
> > + * +------+ +------+ +------+
> > + * |PDMA0 |--------->| TC0 |-------->|VDMA0 |
> > + * +------+ |----->+------+ +------+
> > + *MAC => +------+ | +------+ +------+
> > + *RXQs |PDMA1 |---+ | TC1 | +--->|VDMA1 | => Host
> > + * +------+ +------+ | +------+
> > + * . . .
> > + */
> > +
>
> > +/* Hyper-DMA mapping configuration
> > + * Traffic Class associated with each VDMA/PDMA mapping
> > + * is stored in corresponding array entry.
> > + */
> > +struct stmmac_hdma_cfg {
> > + u32 tx_vdmas; /* TX VDMA count */
> > + u32 rx_vdmas; /* RX VDMA count */
> > + u32 tx_pdmas; /* TX PDMA count */
> > + u32 rx_pdmas; /* RX PDMA count */
> > + u8 *tvdma_tc; /* Tx VDMA to TC mapping array */
> > + u8 *rvdma_tc; /* Rx VDMA to TC mapping array */
> > + u8 *tpdma_tc; /* Tx PDMA to TC mapping array */
> > + u8 *rpdma_tc; /* Rx PDMA to TC mapping array */
> > +};
> > +
> > struct stmmac_dma_cfg {
> > int pbl;
> > int txpbl;
> > @@ -101,6 +146,7 @@ struct stmmac_dma_cfg {
> > bool multi_msi_en;
> > bool dche;
> > bool atds;
> > + struct stmmac_hdma_cfg *hdma_cfg;
>
> Based on what you are implementing the _static_ VDMA-TC-PDMA channels
> mapping I really don't see a value of adding all of these data here.
> The whole implementation gets to be needlessly overcomplicated.
> Moreover AFAICS there are some channels left misconfigured in the
> Patch 2 code. Please see my comments there for more details.
>
I agree, with _static_ VDMA-TC-PDMA channels, maintaining the mapping data
appears complicated.
The real need comes when adding virtualization (SRIOV) capabilities.
I am analyzing your comments in patch2 and will respond after re-evaluation.
> > };
> >
> > #define AXI_BLEN 7
> > @@ -303,5 +349,7 @@ struct plat_stmmacenet_data {
> > int msi_tx_base_vec;
> > const struct dwmac4_addrs *dwmac4_addrs;
> > unsigned int flags;
>
> > + u32 snps_id;
> > + u32 snps_dev_id;
>
> Please move these fields to the head of the structure as the kind of
> crucial ones, and convert snps_dev_id to just dev_id.
>
> snps_id field name was selected based on the VERSION.SNPSVER field
> name (see SNPS prefix). Following that logic the VERSION.DEVID field
> should be converted to the dev_id name.
>
Thanks for explaining the thinking behind the field naming. That makes sense.
I was thinking of it as a prefix for synopsys fields.
Will make the change.
> -Serge(y)
>
> > };
> > #endif
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v5 2/5] net: stmmac: Add basic dw25gmac support in stmmac core
2024-09-10 19:24 ` Serge Semin
@ 2024-09-16 23:32 ` Jitendra Vegiraju
2024-10-04 16:05 ` Jitendra Vegiraju
2024-10-10 23:55 ` Serge Semin
0 siblings, 2 replies; 20+ messages in thread
From: Jitendra Vegiraju @ 2024-09-16 23:32 UTC (permalink / raw)
To: Serge Semin
Cc: netdev, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, bcm-kernel-feedback-list, richardcochran, ast,
daniel, hawk, john.fastabend, rmk+kernel, ahalaney, xiaolei.wang,
rohan.g.thomas, Jianheng.Zhang, linux-kernel, linux-stm32,
linux-arm-kernel, bpf, andrew, linux, horms, florian.fainelli
Hi Serge,
On Tue, Sep 10, 2024 at 12:25 PM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> > +static u32 decode_vdma_count(u32 regval)
> > +{
> > + /* compressed encoding for vdma count
> > + * regval: VDMA count
> > + * 0-15 : 1 - 16
> > + * 16-19 : 20, 24, 28, 32
> > + * 20-23 : 40, 48, 56, 64
> > + * 24-27 : 80, 96, 112, 128
> > + */
> > + if (regval < 16)
> > + return regval + 1;
> > + return (4 << ((regval - 16) / 4)) * ((regval % 4) + 5);
>
> The shortest code isn't always the best one. This one gives me a
> headache in trying to decipher whether it really matches to what is
> described in the comment. What about just:
>
> if (regval < 16) /* Direct mapping */
> return regval + 1;
> else if (regval < 20) /* 20, 24, 28, 32 */
> return 20 + (regval - 16) * 4;
> else if (regval < 24) /* 40, 48, 56, 64 */
> return 40 + (regval - 20) * 8;
> else if (regval < 28) /* 80, 96, 112, 128 */
> return 80 + (regval - 24) * 16;
>
> ?
Couldn't agree more :)
Thanks, I will replace it with your code, which is definitely more readable.
>
> > +}
> > +
> > +static void dw25gmac_read_hdma_limits(void __iomem *ioaddr,
> > + struct stmmac_hdma_cfg *hdma)
> > +{
> > + u32 hw_cap;
> > +
> > + /* Get VDMA/PDMA counts from HW */
> > + hw_cap = readl(ioaddr + XGMAC_HW_FEATURE2);
>
>
> > + hdma->tx_vdmas = decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_TXCNT,
> > + hw_cap));
> > + hdma->rx_vdmas = decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_RXCNT,
> > + hw_cap));
> > + hdma->tx_pdmas = FIELD_GET(XGMAC_HWFEAT_TXQCNT, hw_cap) + 1;
> > + hdma->rx_pdmas = FIELD_GET(XGMAC_HWFEAT_RXQCNT, hw_cap) + 1;
>
> Hmm, these are the Tx/Rx DMA-channels and Tx/Rx MTL-queues count
> fields. Can't you just use the
> dma_features::{number_tx_channel,number_tx_queues} and
> dma_features::{number_rx_channel,number_rx_queues} fields to store the
> retrieved data?
>
> Moreover why not to add the code above to the dwxgmac2_get_hw_feature() method?
>
Thanks, I missed the reuse of existing fields.
However, since the VDMA count has a slightly bigger bitmask, we need to extract
VDMA channel count as per DW25GMAC spec.
Instead of duplicating dwxgmac2_get_hw_feature(), should we add wrapper for
dw25gmac, something like the following?
dw25gmac_get_hw_feature(ioaddr, dma_cap)
{
u32 hw_cap;
int rc;
rc = dwxgmac2_get_hw_feature(ioaddr, dma_cap);
/* Get VDMA counts from HW */
hw_cap = readl(ioaddr + XGMAC_HW_FEATURE2);
dma_cap->num_tx_channels =
decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_TXCNT,
hw_cap));
dma_cap->num_rx_channels =
decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_RXCNT,
hw_cap));
return rc;
}
> > +}
> > +
> > +int dw25gmac_hdma_cfg_init(struct stmmac_priv *priv)
> > +{
> > + struct plat_stmmacenet_data *plat = priv->plat;
> > + struct device *dev = priv->device;
> > + struct stmmac_hdma_cfg *hdma;
> > + int i;
> > +
> > + hdma = devm_kzalloc(dev,
> > + sizeof(*plat->dma_cfg->hdma_cfg),
> > + GFP_KERNEL);
> > + if (!hdma)
> > + return -ENOMEM;
> > +
> > + dw25gmac_read_hdma_limits(priv->ioaddr, hdma);
> > +
> > + hdma->tvdma_tc = devm_kzalloc(dev,
> > + sizeof(*hdma->tvdma_tc) * hdma->tx_vdmas,
> > + GFP_KERNEL);
> > + if (!hdma->tvdma_tc)
> > + return -ENOMEM;
> > +
> > + hdma->rvdma_tc = devm_kzalloc(dev,
> > + sizeof(*hdma->rvdma_tc) * hdma->rx_vdmas,
> > + GFP_KERNEL);
> > + if (!hdma->rvdma_tc)
> > + return -ENOMEM;
> > +
> > + hdma->tpdma_tc = devm_kzalloc(dev,
> > + sizeof(*hdma->tpdma_tc) * hdma->tx_pdmas,
> > + GFP_KERNEL);
> > + if (!hdma->tpdma_tc)
> > + return -ENOMEM;
> > +
> > + hdma->rpdma_tc = devm_kzalloc(dev,
> > + sizeof(*hdma->rpdma_tc) * hdma->rx_pdmas,
> > + GFP_KERNEL);
> > + if (!hdma->rpdma_tc)
> > + return -ENOMEM;
> > +
>
> > + /* Initialize one-to-one mapping for each of the used queues */
> > + for (i = 0; i < plat->tx_queues_to_use; i++) {
> > + hdma->tvdma_tc[i] = i;
> > + hdma->tpdma_tc[i] = i;
> > + }
> > + for (i = 0; i < plat->rx_queues_to_use; i++) {
> > + hdma->rvdma_tc[i] = i;
> > + hdma->rpdma_tc[i] = i;
> > + }
>
> So the Traffic Class ID is initialized for the
> tx_queues_to_use/rx_queues_to_use number of channels only, right? What
> about the Virtual and Physical DMA-channels with numbers greater than
> these values?
>
You have brought up a question that applies to remaining comments in
this file as well.
How the VDMA/PDMA mapping is used depends on the device/glue driver.
For example in
our SoC the remaining VDMAs are meant to be used with SRIOV virtual
functions and not
all of them are available for physical function.
Since additional VDMAs/PDMAs remain unused in hardware I let them stay at their
default values. No traffic is expected to be mapped to unused V/PDMAs.
I couldn't think of a reason for it to be an issue from a driver perspective.
Please let me know, if I am missing something or we need to address a
use case with bigger scope.
The responses for following comments also depend on what approach we take here.
> > + plat->dma_cfg->hdma_cfg = hdma;
> > +
> > + return 0;
> > +}
> > +
> > +
> > +void dw25gmac_dma_init(void __iomem *ioaddr,
> > + struct stmmac_dma_cfg *dma_cfg)
> > +{
> > + u32 value;
> > + u32 i;
> > +
> > + value = readl(ioaddr + XGMAC_DMA_SYSBUS_MODE);
> > + value &= ~(XGMAC_AAL | XGMAC_EAME);
> > + if (dma_cfg->aal)
> > + value |= XGMAC_AAL;
> > + if (dma_cfg->eame)
> > + value |= XGMAC_EAME;
> > + writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
> > +
> > + for (i = 0; i < dma_cfg->hdma_cfg->tx_vdmas; i++) {
> > + value = rd_dma_ch_ind(ioaddr, MODE_TXDESCCTRL, i);
> > + value &= ~XXVGMAC_TXDCSZ;
> > + value |= FIELD_PREP(XXVGMAC_TXDCSZ,
> > + XXVGMAC_TXDCSZ_256BYTES);
> > + value &= ~XXVGMAC_TDPS;
> > + value |= FIELD_PREP(XXVGMAC_TDPS, XXVGMAC_TDPS_HALF);
> > + wr_dma_ch_ind(ioaddr, MODE_TXDESCCTRL, i, value);
> > + }
> > +
> > + for (i = 0; i < dma_cfg->hdma_cfg->rx_vdmas; i++) {
> > + value = rd_dma_ch_ind(ioaddr, MODE_RXDESCCTRL, i);
> > + value &= ~XXVGMAC_RXDCSZ;
> > + value |= FIELD_PREP(XXVGMAC_RXDCSZ,
> > + XXVGMAC_RXDCSZ_256BYTES);
> > + value &= ~XXVGMAC_RDPS;
> > + value |= FIELD_PREP(XXVGMAC_TDPS, XXVGMAC_RDPS_HALF);
> > + wr_dma_ch_ind(ioaddr, MODE_RXDESCCTRL, i, value);
> > + }
> > +
>
> > + for (i = 0; i < dma_cfg->hdma_cfg->tx_pdmas; i++) {
> > + value = rd_dma_ch_ind(ioaddr, MODE_TXEXTCFG, i);
> > + value &= ~(XXVGMAC_TXPBL | XXVGMAC_TPBLX8_MODE);
> > + if (dma_cfg->pblx8)
> > + value |= XXVGMAC_TPBLX8_MODE;
> > + value |= FIELD_PREP(XXVGMAC_TXPBL, dma_cfg->pbl);
> > + wr_dma_ch_ind(ioaddr, MODE_TXEXTCFG, i, value);
> > + xgmac4_tp2tc_map(ioaddr, i, dma_cfg->hdma_cfg->tpdma_tc[i]);
> > + }
> > +
> > + for (i = 0; i < dma_cfg->hdma_cfg->rx_pdmas; i++) {
> > + value = rd_dma_ch_ind(ioaddr, MODE_RXEXTCFG, i);
> > + value &= ~(XXVGMAC_RXPBL | XXVGMAC_RPBLX8_MODE);
> > + if (dma_cfg->pblx8)
> > + value |= XXVGMAC_RPBLX8_MODE;
> > + value |= FIELD_PREP(XXVGMAC_RXPBL, dma_cfg->pbl);
> > + wr_dma_ch_ind(ioaddr, MODE_RXEXTCFG, i, value);
> > + xgmac4_rp2tc_map(ioaddr, i, dma_cfg->hdma_cfg->rpdma_tc[i]);
>
> What if tx_pdmas doesn't match plat_stmmacenet_data::tx_queues_to_use
> and rx_pdmas doesn't match to plat_stmmacenet_data::rx_queues_to_use?
>
> If they don't then you'll get out of the initialized tpdma_tc/rpdma_tc
> fields and these channels will be pre-initialized with the zero TC. Is
> that what expected? I doubt so.
>
As mentioned in the previous response the remaining resources are unused
and no traffic is mapped to those resources.
> > + }
> > +}
> > +
>
> > +void dw25gmac_dma_init_tx_chan(struct stmmac_priv *priv,
> > + void __iomem *ioaddr,
> > + struct stmmac_dma_cfg *dma_cfg,
> > + dma_addr_t dma_addr, u32 chan)
> > +{
> > + u32 value;
> > +
>
> > + value = readl(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
> > + value &= ~XXVGMAC_TVDMA2TCMP;
> > + value |= FIELD_PREP(XXVGMAC_TVDMA2TCMP,
> > + dma_cfg->hdma_cfg->tvdma_tc[chan]);
> > + writel(value, ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
>
> Please note this will have only first
> plat_stmmacenet_data::{tx_queues_to_use,rx_queues_to_use} VDMA
> channels initialized. Don't you have much more than just 4 channels?
>
Yes, there are 32 VDMA channels on this device. In our application the
additional channels are partitioned for use with SRIOV virtual functions.
Similar to PDMA comment above, the additional VDMAs are not enabled,
and left in default state.
My thinking is, when another 25gmac device comes along that requires a
different mapping we may need to add the ability to set the mapping in
glue driver.
We can support this by adding a check in dw25gmac_setup()
@@ -1708,8 +1708,10 @@ int dw25gmac_setup(struct stmmac_priv *priv)
mac->mii.clk_csr_shift = 19;
mac->mii.clk_csr_mask = GENMASK(21, 19);
- /* Allocate and initialize hdma mapping */
- return dw25gmac_hdma_cfg_init(priv);
+ /* Allocate and initialize hdma mapping, if not done by glue driver. */
+ if (!priv->plat->dma_cfg->hdma_cfg)
+ return dw25gmac_hdma_cfg_init(priv);
+ return 0;
}
> > +
> > + writel(upper_32_bits(dma_addr),
> > + ioaddr + XGMAC_DMA_CH_TxDESC_HADDR(chan));
> > + writel(lower_32_bits(dma_addr),
> > + ioaddr + XGMAC_DMA_CH_TxDESC_LADDR(chan));
> > +}
> > +
> > +void dw25gmac_dma_init_rx_chan(struct stmmac_priv *priv,
> > + void __iomem *ioaddr,
> > + struct stmmac_dma_cfg *dma_cfg,
> > + dma_addr_t dma_addr, u32 chan)
> > +{
> > + u32 value;
> > +
>
> > + value = readl(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
> > + value &= ~XXVGMAC_RVDMA2TCMP;
> > + value |= FIELD_PREP(XXVGMAC_RVDMA2TCMP,
> > + dma_cfg->hdma_cfg->rvdma_tc[chan]);
> > + writel(value, ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
>
> The same question.
>
> > +
> > + writel(upper_32_bits(dma_addr),
> > + ioaddr + XGMAC_DMA_CH_RxDESC_HADDR(chan));
> > + writel(lower_32_bits(dma_addr),
> > + ioaddr + XGMAC_DMA_CH_RxDESC_LADDR(chan));
> > +}
>
> These methods are called for each
> plat_stmmacenet_data::{tx_queues_to_use,rx_queues_to_use}
> DMA-channel/Queue. The static mapping means you'll have each
> PDMA/Queue assigned a static traffic class ID corresponding to the
> channel ID. Meanwhile the VDMA channels are supposed to be initialized
> with the TC ID corresponding to the matching PDMA ID.
>
> The TC ID in this case is passed as the DMA/Queue channel ID. Then the
> Tx/Rx DMA-channels init methods can be converted to:
>
> dw25gmac_dma_init_Xx_chan(chan)
> {
> /* Map each chan-th VDMA to the single chan PDMA by assigning
> * the static TC ID.
> */
> for (i = chan; i < Xx_vdmas; i += (Xx_vdmas / Xx_queues_to_use)) {
> /* Initialize VDMA channels */
> XXVGMAC_TVDMA2TCMP = chan;
> }
>
> /* Assign the static TC ID to the specified PDMA channel */
> xgmac4_rp2tc_map(chan, chan)
> }
>
> , where X={t,r}.
>
> Thus you can redistribute the loops implemented in dw25gmac_dma_init()
> to the respective Tx/Rx DMA-channel init methods.
>
> Am I missing something?
I think your visualization of HDMA may be going beyond the application
I understand.
We are allocating a VDMA for each of the TX/RX channels. The use of
additional VDMAs
depends on how the device is partitioned for virtualization.
In the non-SRIOV case the remaining VDMAs will remain unused.
Please let me know if I missed your question.
>
> -Serge()
>
> > [...]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v5 4/5] net: stmmac: Add PCI driver support for BCM8958x
2024-09-10 19:51 ` Serge Semin
@ 2024-09-16 23:43 ` Jitendra Vegiraju
0 siblings, 0 replies; 20+ messages in thread
From: Jitendra Vegiraju @ 2024-09-16 23:43 UTC (permalink / raw)
To: Serge Semin
Cc: netdev, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, bcm-kernel-feedback-list, richardcochran, ast,
daniel, hawk, john.fastabend, rmk+kernel, ahalaney, xiaolei.wang,
rohan.g.thomas, Jianheng.Zhang, linux-kernel, linux-stm32,
linux-arm-kernel, bpf, andrew, linux, horms, florian.fainelli
Hi Serge
Thanks for reviewing the patches.
On Tue, Sep 10, 2024 at 12:52 PM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> On Tue, Sep 03, 2024 at 10:48:14PM -0700, jitendra.vegiraju@broadcom.com wrote:
> > From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
> >
>
> > +#define READ_POLL_DELAY_US 100
> > +#define READ_POLL_TIMEOUT_US 10000
>
> These macros are unused. Why do you need them here?
>
Thanks, missed the cleaning these up.
> > +#define DWMAC_125MHZ 125000000
> > +#define DWMAC_250MHZ 250000000
>
> Drop these and use the literals directly.
>
Ack
> > +#define BRCM_XGMAC_NUM_VLAN_FILTERS 32
> > +
> > +/* TX and RX Queue counts */
> > +#define BRCM_TX_Q_COUNT 4
> > +#define BRCM_RX_Q_COUNT 4
> > +
>
> > +#define BRCM_XGMAC_DMA_TX_SIZE 1024
> > +#define BRCM_XGMAC_DMA_RX_SIZE 1024
>
> Unused.
>
Ack
> > +static void dwxgmac_brcm_common_default_data(struct plat_stmmacenet_data *plat)
> > +{
> > + int i;
> > +
> > + plat->has_xgmac = 1;
> > + plat->force_sf_dma_mode = 1;
> > + plat->mac_port_sel_speed = SPEED_10000;
>
> > + plat->clk_ptp_rate = DWMAC_125MHZ;
> > + plat->clk_ref_rate = DWMAC_250MHZ;
>
> Just 125000000 and 250000000. There is no need in defining the macro
> with the names matching the numerical literals.
>
Ack
> > +static int dwxgmac_brcm_default_data(struct pci_dev *pdev,
> > + struct plat_stmmacenet_data *plat)
> > +{
> > + /* Set common default data first */
> > + dwxgmac_brcm_common_default_data(plat);
> > +
> > + plat->snps_id = DW25GMAC_CORE_4_00;
> > + plat->snps_dev_id = DW25GMAC_ID;
> > + plat->bus_id = 0;
> > + plat->phy_addr = 0;
>
> > + plat->phy_interface = PHY_INTERFACE_MODE_USXGMII;
>
> Really, USXGMII? Universal Serial XGMII? Synopsys call it just XGMII:
> https://www.synopsys.com/dw/ipdir.php?ds=dwc_25g_ethernet_mac_ip
>
Thanks for pointing out. It was a misunderstanding on our part.
I will change it to XGMII and add corresponding handling for XGMII in
stmmac_mac_link_up().
> > +
>
> > + plat->msi_mac_vec = BRCM_XGMAC_MSI_MAC_VECTOR;
> > + plat->msi_rx_base_vec = BRCM_XGMAC_MSI_RX_VECTOR_START;
> > + plat->msi_tx_base_vec = BRCM_XGMAC_MSI_TX_VECTOR_START;
>
> Please see my next comments about these fields utilization.
>
Ack
> > +
> > + return 0;
> > +}
> > +
> > +static struct dwxgmac_brcm_pci_info dwxgmac_brcm_pci_info = {
> > + .setup = dwxgmac_brcm_default_data,
> > +};
> > +
> > +static void brcm_config_misc_regs(struct pci_dev *pdev,
> > + struct brcm_priv_data *brcm_priv)
> > +{
> > + pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LOW,
> > + XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LO_VALUE);
> > + pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HIGH,
> > + XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HI_VALUE);
> > +
> > + misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_OFFSET,
> > + XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_VALUE);
> > + misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_OFFSET,
> > + XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_VALUE);
> > +
> > + /* Enable Switch Link */
> > + misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MII_CTRL_OFFSET,
> > + XGMAC_PCIE_MISC_MII_CTRL_PAUSE_RX |
> > + XGMAC_PCIE_MISC_MII_CTRL_PAUSE_TX |
> > + XGMAC_PCIE_MISC_MII_CTRL_LINK_UP);
> > +}
> > +
> > +static int brcm_config_multi_msi(struct pci_dev *pdev,
> > + struct plat_stmmacenet_data *plat,
> > + struct stmmac_resources *res)
> > +{
> > + int ret, i;
> > +
>
> > + if (plat->msi_rx_base_vec >= STMMAC_MSI_VEC_MAX ||
> > + plat->msi_tx_base_vec >= STMMAC_MSI_VEC_MAX) {
>
> Please see my next comment about these fields and STMMAC_MSI_VEC_MAX
> utilization.
>
Ack
> > + dev_err(&pdev->dev, "%s: Invalid RX & TX vector defined\n",
> > + __func__);
> > + return -EINVAL;
> > + }
> > +
> > + ret = pci_alloc_irq_vectors(pdev, 2, STMMAC_MSI_VEC_MAX,
> > + PCI_IRQ_MSI | PCI_IRQ_MSIX);
> > + if (ret < 0) {
> > + dev_err(&pdev->dev, "%s: multi MSI enablement failed\n",
> > + __func__);
> > + return ret;
> > + }
> > +
> > + /* For RX MSI */
> > + for (i = 0; i < plat->rx_queues_to_use; i++)
> > + res->rx_irq[i] = pci_irq_vector(pdev,
> > + plat->msi_rx_base_vec + i * 2);
> > +
> > + /* For TX MSI */
> > + for (i = 0; i < plat->tx_queues_to_use; i++)
> > + res->tx_irq[i] = pci_irq_vector(pdev,
> > + plat->msi_tx_base_vec + i * 2);
> > +
>
> > + if (plat->msi_mac_vec < STMMAC_MSI_VEC_MAX)
> > + res->irq = pci_irq_vector(pdev, plat->msi_mac_vec);
>
> What if msi_mac_vec is greater than STMMAC_MSI_VEC_MAX? Will your
> device work without delivering the MAC IRQs? I doubt so.
>
> In anyway see my next comment.
>
Ack
> > +
> > +static int dwxgmac_brcm_pci_probe(struct pci_dev *pdev,
> > + const struct pci_device_id *id)
>
> > + plat->msi_mac_vec = STMMAC_MSI_VEC_MAX;
> > + plat->msi_wol_vec = STMMAC_MSI_VEC_MAX;
> > + plat->msi_lpi_vec = STMMAC_MSI_VEC_MAX;
> > + plat->msi_sfty_ce_vec = STMMAC_MSI_VEC_MAX;
> > + plat->msi_sfty_ue_vec = STMMAC_MSI_VEC_MAX;
> > + plat->msi_rx_base_vec = STMMAC_MSI_VEC_MAX;
> > + plat->msi_tx_base_vec = STMMAC_MSI_VEC_MAX;
>
> Please don't use these fields and the STMMAC_MSI_VEC_MAX macro. Either
> have the BRCM_XGMAC_MSI* macros utilized directly or define the
> device-specific data in the glue driver (in brcm_priv_data if you
> wish). Really the MSI vectors aren't related to any DW *MAC IP-core
> these are the pure vendor platform-specific settings.
>
> The fields originally have been introduced by the Intel developers,
> who AFAICS just found it easier to extend the generic platform-data
> structure instead of introducing the new Intel MAC-specific data.
>
> I am going to drop these fields in a future cleanup patchset so to
> reduce the plat_stmmacenet_data structure complexity.
>
Thanks for the explanation. Will follow your guidelines in next patch.
> -Serge(y)
>
> > [...]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v5 2/5] net: stmmac: Add basic dw25gmac support in stmmac core
2024-09-16 23:32 ` Jitendra Vegiraju
@ 2024-10-04 16:05 ` Jitendra Vegiraju
2024-10-04 23:45 ` Serge Semin
2024-10-11 0:01 ` Serge Semin
2024-10-10 23:55 ` Serge Semin
1 sibling, 2 replies; 20+ messages in thread
From: Jitendra Vegiraju @ 2024-10-04 16:05 UTC (permalink / raw)
To: Serge Semin
Cc: netdev, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, bcm-kernel-feedback-list, richardcochran, ast,
daniel, hawk, john.fastabend, rmk+kernel, ahalaney, xiaolei.wang,
rohan.g.thomas, Jianheng.Zhang, linux-kernel, linux-stm32,
linux-arm-kernel, bpf, andrew, linux, horms, florian.fainelli
Hi Serge,
On Mon, Sep 16, 2024 at 4:32 PM Jitendra Vegiraju
<jitendra.vegiraju@broadcom.com> wrote:
>
> Hi Serge,
>
> On Tue, Sep 10, 2024 at 12:25 PM Serge Semin <fancer.lancer@gmail.com> wrote:
> >
> > > +static u32 decode_vdma_count(u32 regval)
> > > +{
> > > + /* compressed encoding for vdma count
> > > + * regval: VDMA count
> > > + * 0-15 : 1 - 16
> > > + * 16-19 : 20, 24, 28, 32
> > > + * 20-23 : 40, 48, 56, 64
> > > + * 24-27 : 80, 96, 112, 128
> > > + */
> > > + if (regval < 16)
> > > + return regval + 1;
> > > + return (4 << ((regval - 16) / 4)) * ((regval % 4) + 5);
> >
> > The shortest code isn't always the best one. This one gives me a
> > headache in trying to decipher whether it really matches to what is
> > described in the comment. What about just:
> >
> > if (regval < 16) /* Direct mapping */
> > return regval + 1;
> > else if (regval < 20) /* 20, 24, 28, 32 */
> > return 20 + (regval - 16) * 4;
> > else if (regval < 24) /* 40, 48, 56, 64 */
> > return 40 + (regval - 20) * 8;
> > else if (regval < 28) /* 80, 96, 112, 128 */
> > return 80 + (regval - 24) * 16;
> >
> > ?
> Couldn't agree more :)
> Thanks, I will replace it with your code, which is definitely more readable.
>
> >
> > > +}
> > > +
> > > +static void dw25gmac_read_hdma_limits(void __iomem *ioaddr,
> > > + struct stmmac_hdma_cfg *hdma)
> > > +{
> > > + u32 hw_cap;
> > > +
> > > + /* Get VDMA/PDMA counts from HW */
> > > + hw_cap = readl(ioaddr + XGMAC_HW_FEATURE2);
> >
> >
> > > + hdma->tx_vdmas = decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_TXCNT,
> > > + hw_cap));
> > > + hdma->rx_vdmas = decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_RXCNT,
> > > + hw_cap));
> > > + hdma->tx_pdmas = FIELD_GET(XGMAC_HWFEAT_TXQCNT, hw_cap) + 1;
> > > + hdma->rx_pdmas = FIELD_GET(XGMAC_HWFEAT_RXQCNT, hw_cap) + 1;
> >
> > Hmm, these are the Tx/Rx DMA-channels and Tx/Rx MTL-queues count
> > fields. Can't you just use the
> > dma_features::{number_tx_channel,number_tx_queues} and
> > dma_features::{number_rx_channel,number_rx_queues} fields to store the
> > retrieved data?
> >
> > Moreover why not to add the code above to the dwxgmac2_get_hw_feature() method?
> >
> Thanks, I missed the reuse of existing fields.
> However, since the VDMA count has a slightly bigger bitmask, we need to extract
> VDMA channel count as per DW25GMAC spec.
> Instead of duplicating dwxgmac2_get_hw_feature(), should we add wrapper for
> dw25gmac, something like the following?
> dw25gmac_get_hw_feature(ioaddr, dma_cap)
> {
> u32 hw_cap;
> int rc;
> rc = dwxgmac2_get_hw_feature(ioaddr, dma_cap);
> /* Get VDMA counts from HW */
> hw_cap = readl(ioaddr + XGMAC_HW_FEATURE2);
> dma_cap->num_tx_channels =
> decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_TXCNT,
> hw_cap));
> dma_cap->num_rx_channels =
> decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_RXCNT,
> hw_cap));
> return rc;
> }
>
> > > +}
> > > +
> > > +int dw25gmac_hdma_cfg_init(struct stmmac_priv *priv)
> > > +{
> > > + struct plat_stmmacenet_data *plat = priv->plat;
> > > + struct device *dev = priv->device;
> > > + struct stmmac_hdma_cfg *hdma;
> > > + int i;
> > > +
> > > + hdma = devm_kzalloc(dev,
> > > + sizeof(*plat->dma_cfg->hdma_cfg),
> > > + GFP_KERNEL);
> > > + if (!hdma)
> > > + return -ENOMEM;
> > > +
> > > + dw25gmac_read_hdma_limits(priv->ioaddr, hdma);
> > > +
> > > + hdma->tvdma_tc = devm_kzalloc(dev,
> > > + sizeof(*hdma->tvdma_tc) * hdma->tx_vdmas,
> > > + GFP_KERNEL);
> > > + if (!hdma->tvdma_tc)
> > > + return -ENOMEM;
> > > +
> > > + hdma->rvdma_tc = devm_kzalloc(dev,
> > > + sizeof(*hdma->rvdma_tc) * hdma->rx_vdmas,
> > > + GFP_KERNEL);
> > > + if (!hdma->rvdma_tc)
> > > + return -ENOMEM;
> > > +
> > > + hdma->tpdma_tc = devm_kzalloc(dev,
> > > + sizeof(*hdma->tpdma_tc) * hdma->tx_pdmas,
> > > + GFP_KERNEL);
> > > + if (!hdma->tpdma_tc)
> > > + return -ENOMEM;
> > > +
> > > + hdma->rpdma_tc = devm_kzalloc(dev,
> > > + sizeof(*hdma->rpdma_tc) * hdma->rx_pdmas,
> > > + GFP_KERNEL);
> > > + if (!hdma->rpdma_tc)
> > > + return -ENOMEM;
> > > +
> >
> > > + /* Initialize one-to-one mapping for each of the used queues */
> > > + for (i = 0; i < plat->tx_queues_to_use; i++) {
> > > + hdma->tvdma_tc[i] = i;
> > > + hdma->tpdma_tc[i] = i;
> > > + }
> > > + for (i = 0; i < plat->rx_queues_to_use; i++) {
> > > + hdma->rvdma_tc[i] = i;
> > > + hdma->rpdma_tc[i] = i;
> > > + }
> >
> > So the Traffic Class ID is initialized for the
> > tx_queues_to_use/rx_queues_to_use number of channels only, right? What
> > about the Virtual and Physical DMA-channels with numbers greater than
> > these values?
> >
> You have brought up a question that applies to remaining comments in
> this file as well.
> How the VDMA/PDMA mapping is used depends on the device/glue driver.
> For example in
> our SoC the remaining VDMAs are meant to be used with SRIOV virtual
> functions and not
> all of them are available for physical function.
> Since additional VDMAs/PDMAs remain unused in hardware I let them stay at their
> default values. No traffic is expected to be mapped to unused V/PDMAs.
> I couldn't think of a reason for it to be an issue from a driver perspective.
> Please let me know, if I am missing something or we need to address a
> use case with bigger scope.
> The responses for following comments also depend on what approach we take here.
>
> > > + plat->dma_cfg->hdma_cfg = hdma;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +
> > > +void dw25gmac_dma_init(void __iomem *ioaddr,
> > > + struct stmmac_dma_cfg *dma_cfg)
> > > +{
> > > + u32 value;
> > > + u32 i;
> > > +
> > > + value = readl(ioaddr + XGMAC_DMA_SYSBUS_MODE);
> > > + value &= ~(XGMAC_AAL | XGMAC_EAME);
> > > + if (dma_cfg->aal)
> > > + value |= XGMAC_AAL;
> > > + if (dma_cfg->eame)
> > > + value |= XGMAC_EAME;
> > > + writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
> > > +
> > > + for (i = 0; i < dma_cfg->hdma_cfg->tx_vdmas; i++) {
> > > + value = rd_dma_ch_ind(ioaddr, MODE_TXDESCCTRL, i);
> > > + value &= ~XXVGMAC_TXDCSZ;
> > > + value |= FIELD_PREP(XXVGMAC_TXDCSZ,
> > > + XXVGMAC_TXDCSZ_256BYTES);
> > > + value &= ~XXVGMAC_TDPS;
> > > + value |= FIELD_PREP(XXVGMAC_TDPS, XXVGMAC_TDPS_HALF);
> > > + wr_dma_ch_ind(ioaddr, MODE_TXDESCCTRL, i, value);
> > > + }
> > > +
> > > + for (i = 0; i < dma_cfg->hdma_cfg->rx_vdmas; i++) {
> > > + value = rd_dma_ch_ind(ioaddr, MODE_RXDESCCTRL, i);
> > > + value &= ~XXVGMAC_RXDCSZ;
> > > + value |= FIELD_PREP(XXVGMAC_RXDCSZ,
> > > + XXVGMAC_RXDCSZ_256BYTES);
> > > + value &= ~XXVGMAC_RDPS;
> > > + value |= FIELD_PREP(XXVGMAC_TDPS, XXVGMAC_RDPS_HALF);
> > > + wr_dma_ch_ind(ioaddr, MODE_RXDESCCTRL, i, value);
> > > + }
> > > +
> >
> > > + for (i = 0; i < dma_cfg->hdma_cfg->tx_pdmas; i++) {
> > > + value = rd_dma_ch_ind(ioaddr, MODE_TXEXTCFG, i);
> > > + value &= ~(XXVGMAC_TXPBL | XXVGMAC_TPBLX8_MODE);
> > > + if (dma_cfg->pblx8)
> > > + value |= XXVGMAC_TPBLX8_MODE;
> > > + value |= FIELD_PREP(XXVGMAC_TXPBL, dma_cfg->pbl);
> > > + wr_dma_ch_ind(ioaddr, MODE_TXEXTCFG, i, value);
> > > + xgmac4_tp2tc_map(ioaddr, i, dma_cfg->hdma_cfg->tpdma_tc[i]);
> > > + }
> > > +
> > > + for (i = 0; i < dma_cfg->hdma_cfg->rx_pdmas; i++) {
> > > + value = rd_dma_ch_ind(ioaddr, MODE_RXEXTCFG, i);
> > > + value &= ~(XXVGMAC_RXPBL | XXVGMAC_RPBLX8_MODE);
> > > + if (dma_cfg->pblx8)
> > > + value |= XXVGMAC_RPBLX8_MODE;
> > > + value |= FIELD_PREP(XXVGMAC_RXPBL, dma_cfg->pbl);
> > > + wr_dma_ch_ind(ioaddr, MODE_RXEXTCFG, i, value);
> > > + xgmac4_rp2tc_map(ioaddr, i, dma_cfg->hdma_cfg->rpdma_tc[i]);
> >
> > What if tx_pdmas doesn't match plat_stmmacenet_data::tx_queues_to_use
> > and rx_pdmas doesn't match to plat_stmmacenet_data::rx_queues_to_use?
> >
> > If they don't then you'll get out of the initialized tpdma_tc/rpdma_tc
> > fields and these channels will be pre-initialized with the zero TC. Is
> > that what expected? I doubt so.
> >
> As mentioned in the previous response the remaining resources are unused
> and no traffic is mapped to those resources.
>
> > > + }
> > > +}
> > > +
> >
> > > +void dw25gmac_dma_init_tx_chan(struct stmmac_priv *priv,
> > > + void __iomem *ioaddr,
> > > + struct stmmac_dma_cfg *dma_cfg,
> > > + dma_addr_t dma_addr, u32 chan)
> > > +{
> > > + u32 value;
> > > +
> >
> > > + value = readl(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
> > > + value &= ~XXVGMAC_TVDMA2TCMP;
> > > + value |= FIELD_PREP(XXVGMAC_TVDMA2TCMP,
> > > + dma_cfg->hdma_cfg->tvdma_tc[chan]);
> > > + writel(value, ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
> >
> > Please note this will have only first
> > plat_stmmacenet_data::{tx_queues_to_use,rx_queues_to_use} VDMA
> > channels initialized. Don't you have much more than just 4 channels?
> >
> Yes, there are 32 VDMA channels on this device. In our application the
> additional channels are partitioned for use with SRIOV virtual functions.
> Similar to PDMA comment above, the additional VDMAs are not enabled,
> and left in default state.
> My thinking is, when another 25gmac device comes along that requires a
> different mapping we may need to add the ability to set the mapping in
> glue driver.
> We can support this by adding a check in dw25gmac_setup()
> @@ -1708,8 +1708,10 @@ int dw25gmac_setup(struct stmmac_priv *priv)
> mac->mii.clk_csr_shift = 19;
> mac->mii.clk_csr_mask = GENMASK(21, 19);
>
> - /* Allocate and initialize hdma mapping */
> - return dw25gmac_hdma_cfg_init(priv);
> + /* Allocate and initialize hdma mapping, if not done by glue driver. */
> + if (!priv->plat->dma_cfg->hdma_cfg)
> + return dw25gmac_hdma_cfg_init(priv);
> + return 0;
> }
>
> > > +
> > > + writel(upper_32_bits(dma_addr),
> > > + ioaddr + XGMAC_DMA_CH_TxDESC_HADDR(chan));
> > > + writel(lower_32_bits(dma_addr),
> > > + ioaddr + XGMAC_DMA_CH_TxDESC_LADDR(chan));
> > > +}
> > > +
> > > +void dw25gmac_dma_init_rx_chan(struct stmmac_priv *priv,
> > > + void __iomem *ioaddr,
> > > + struct stmmac_dma_cfg *dma_cfg,
> > > + dma_addr_t dma_addr, u32 chan)
> > > +{
> > > + u32 value;
> > > +
> >
> > > + value = readl(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
> > > + value &= ~XXVGMAC_RVDMA2TCMP;
> > > + value |= FIELD_PREP(XXVGMAC_RVDMA2TCMP,
> > > + dma_cfg->hdma_cfg->rvdma_tc[chan]);
> > > + writel(value, ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
> >
> > The same question.
> >
> > > +
> > > + writel(upper_32_bits(dma_addr),
> > > + ioaddr + XGMAC_DMA_CH_RxDESC_HADDR(chan));
> > > + writel(lower_32_bits(dma_addr),
> > > + ioaddr + XGMAC_DMA_CH_RxDESC_LADDR(chan));
> > > +}
> >
> > These methods are called for each
> > plat_stmmacenet_data::{tx_queues_to_use,rx_queues_to_use}
> > DMA-channel/Queue. The static mapping means you'll have each
> > PDMA/Queue assigned a static traffic class ID corresponding to the
> > channel ID. Meanwhile the VDMA channels are supposed to be initialized
> > with the TC ID corresponding to the matching PDMA ID.
> >
> > The TC ID in this case is passed as the DMA/Queue channel ID. Then the
> > Tx/Rx DMA-channels init methods can be converted to:
> >
> > dw25gmac_dma_init_Xx_chan(chan)
> > {
> > /* Map each chan-th VDMA to the single chan PDMA by assigning
> > * the static TC ID.
> > */
> > for (i = chan; i < Xx_vdmas; i += (Xx_vdmas / Xx_queues_to_use)) {
> > /* Initialize VDMA channels */
> > XXVGMAC_TVDMA2TCMP = chan;
> > }
> >
> > /* Assign the static TC ID to the specified PDMA channel */
> > xgmac4_rp2tc_map(chan, chan)
> > }
> >
> > , where X={t,r}.
> >
> > Thus you can redistribute the loops implemented in dw25gmac_dma_init()
> > to the respective Tx/Rx DMA-channel init methods.
> >
> > Am I missing something?
> I think your visualization of HDMA may be going beyond the application
> I understand.
> We are allocating a VDMA for each of the TX/RX channels. The use of
> additional VDMAs
> depends on how the device is partitioned for virtualization.
> In the non-SRIOV case the remaining VDMAs will remain unused.
> Please let me know if I missed your question.
> >
> > -Serge()
> >
> > > [...]
When you get a chance, I would like to get your input on the approach we need
to take to incrementally add dw25gmac support.
In the last conversation there were some open questions around the case of
initializing unused VDMA channels and related combination scenarios.
The hdma mapping provides flexibility for virtualization. However, our
SoC device cannot use all VDMAs with one PCI function. The VDMAs are
partitioned for SRIOV use in the firmware. This SoC defaults to 8 functions
with 4 VDMA channels each. The initial effort is to support one PCI physical
function with 4 VDMA channels.
Also, currently the stmmac driver has inferred one-to-one relation between
netif channels and physical DMAs. It would be a complex change to support
each VDMA as its own netif channel and mapping fewer physical DMAs.
Hence, for initial submission one-to-one mapping is assumed.
As you mentioned, a static one-to-one mapping of VDMA-TC-PDMA doesn't
require the additional complexity of managing these mappings as proposed
in the current patch series with *struct stmmac_hdma_cfg*.
To introduce dw25gmac incrementally, I am thinking of two approaches,
1. Take the current patch series forward using *struct stmmac_hdma_cfg*,
keeping the unused VDMAs in default state. We need to fix the
initialization
loops to only initialize the VDMA and PDMAs being used.
2. Simplify the initial patch by removing *struct hdma_cfg* from the patch
series and still use static VDMA-TC-PDMA mapping.
Please share your thoughts.
If it helps, I can send patch series with option 2 above after
addressing all other
review comments.
Appreciate your guidance!
-Jitendra
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v5 2/5] net: stmmac: Add basic dw25gmac support in stmmac core
2024-10-04 16:05 ` Jitendra Vegiraju
@ 2024-10-04 23:45 ` Serge Semin
2024-10-11 0:01 ` Serge Semin
1 sibling, 0 replies; 20+ messages in thread
From: Serge Semin @ 2024-10-04 23:45 UTC (permalink / raw)
To: Jitendra Vegiraju
Cc: netdev, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, bcm-kernel-feedback-list, richardcochran, ast,
daniel, hawk, john.fastabend, rmk+kernel, ahalaney, xiaolei.wang,
rohan.g.thomas, Jianheng.Zhang, linux-kernel, linux-stm32,
linux-arm-kernel, bpf, andrew, linux, horms, florian.fainelli
Hi Jitendra
On Fri, Oct 04, 2024 at 09:05:36AM GMT, Jitendra Vegiraju wrote:
>
> When you get a chance, I would like to get your input on the approach we need
> to take to incrementally add dw25gmac support.
>
Sorry for the delay with response. I've been quite busy lately. I'll
get back to this patch set review early next week and give you my
thoughts regarding all your questions.
-Serge(y)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v5 2/5] net: stmmac: Add basic dw25gmac support in stmmac core
2024-09-16 23:32 ` Jitendra Vegiraju
2024-10-04 16:05 ` Jitendra Vegiraju
@ 2024-10-10 23:55 ` Serge Semin
2024-10-14 18:24 ` Jitendra Vegiraju
1 sibling, 1 reply; 20+ messages in thread
From: Serge Semin @ 2024-10-10 23:55 UTC (permalink / raw)
To: Jitendra Vegiraju
Cc: netdev, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, bcm-kernel-feedback-list, richardcochran, ast,
daniel, hawk, john.fastabend, rmk+kernel, ahalaney, xiaolei.wang,
rohan.g.thomas, Jianheng.Zhang, linux-kernel, linux-stm32,
linux-arm-kernel, bpf, andrew, linux, horms, florian.fainelli
On Mon, Sep 16, 2024 at 04:32:33PM GMT, Jitendra Vegiraju wrote:
> Hi Serge,
>
> On Tue, Sep 10, 2024 at 12:25 PM Serge Semin <fancer.lancer@gmail.com> wrote:
> >
> > > +static u32 decode_vdma_count(u32 regval)
> > > +{
> > > + /* compressed encoding for vdma count
> > > + * regval: VDMA count
> > > + * 0-15 : 1 - 16
> > > + * 16-19 : 20, 24, 28, 32
> > > + * 20-23 : 40, 48, 56, 64
> > > + * 24-27 : 80, 96, 112, 128
> > > + */
> > > + if (regval < 16)
> > > + return regval + 1;
> > > + return (4 << ((regval - 16) / 4)) * ((regval % 4) + 5);
> >
> > The shortest code isn't always the best one. This one gives me a
> > headache in trying to decipher whether it really matches to what is
> > described in the comment. What about just:
> >
> > if (regval < 16) /* Direct mapping */
> > return regval + 1;
> > else if (regval < 20) /* 20, 24, 28, 32 */
> > return 20 + (regval - 16) * 4;
> > else if (regval < 24) /* 40, 48, 56, 64 */
> > return 40 + (regval - 20) * 8;
> > else if (regval < 28) /* 80, 96, 112, 128 */
> > return 80 + (regval - 24) * 16;
> >
> > ?
> Couldn't agree more :)
> Thanks, I will replace it with your code, which is definitely more readable.
>
> >
> > > +}
> > > +
> > > +static void dw25gmac_read_hdma_limits(void __iomem *ioaddr,
> > > + struct stmmac_hdma_cfg *hdma)
> > > +{
> > > + u32 hw_cap;
> > > +
> > > + /* Get VDMA/PDMA counts from HW */
> > > + hw_cap = readl(ioaddr + XGMAC_HW_FEATURE2);
> >
> >
> > > + hdma->tx_vdmas = decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_TXCNT,
> > > + hw_cap));
> > > + hdma->rx_vdmas = decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_RXCNT,
> > > + hw_cap));
> > > + hdma->tx_pdmas = FIELD_GET(XGMAC_HWFEAT_TXQCNT, hw_cap) + 1;
> > > + hdma->rx_pdmas = FIELD_GET(XGMAC_HWFEAT_RXQCNT, hw_cap) + 1;
> >
> > Hmm, these are the Tx/Rx DMA-channels and Tx/Rx MTL-queues count
> > fields. Can't you just use the
> > dma_features::{number_tx_channel,number_tx_queues} and
> > dma_features::{number_rx_channel,number_rx_queues} fields to store the
> > retrieved data?
> >
> > Moreover why not to add the code above to the dwxgmac2_get_hw_feature() method?
> >
> Thanks, I missed the reuse of existing fields.
> However, since the VDMA count has a slightly bigger bitmask, we need to extract
> VDMA channel count as per DW25GMAC spec.
> Instead of duplicating dwxgmac2_get_hw_feature(), should we add wrapper for
> dw25gmac, something like the following?
Yeah, and there is the encoded fields value. Your suggestion sounds
reasonable.
> dw25gmac_get_hw_feature(ioaddr, dma_cap)
> {
> u32 hw_cap;
> int rc;
s/rc/ret
+ newline
> rc = dwxgmac2_get_hw_feature(ioaddr, dma_cap);
newline
> /* Get VDMA counts from HW */
> hw_cap = readl(ioaddr + XGMAC_HW_FEATURE2);
> dma_cap->num_tx_channels =
> decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_TXCNT,
> hw_cap));
> dma_cap->num_rx_channels =
> decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_RXCNT,
> hw_cap));
newline
> return rc;
> }
>
> > > +}
> > > +
> > > +int dw25gmac_hdma_cfg_init(struct stmmac_priv *priv)
> > > +{
> > > + struct plat_stmmacenet_data *plat = priv->plat;
> > > + struct device *dev = priv->device;
> > > + struct stmmac_hdma_cfg *hdma;
> > > + int i;
> > > +
> > > + hdma = devm_kzalloc(dev,
> > > + sizeof(*plat->dma_cfg->hdma_cfg),
> > > + GFP_KERNEL);
> > > + if (!hdma)
> > > + return -ENOMEM;
> > > +
> > > + dw25gmac_read_hdma_limits(priv->ioaddr, hdma);
> > > +
> > > + hdma->tvdma_tc = devm_kzalloc(dev,
> > > + sizeof(*hdma->tvdma_tc) * hdma->tx_vdmas,
> > > + GFP_KERNEL);
> > > + if (!hdma->tvdma_tc)
> > > + return -ENOMEM;
> > > +
> > > + hdma->rvdma_tc = devm_kzalloc(dev,
> > > + sizeof(*hdma->rvdma_tc) * hdma->rx_vdmas,
> > > + GFP_KERNEL);
> > > + if (!hdma->rvdma_tc)
> > > + return -ENOMEM;
> > > +
> > > + hdma->tpdma_tc = devm_kzalloc(dev,
> > > + sizeof(*hdma->tpdma_tc) * hdma->tx_pdmas,
> > > + GFP_KERNEL);
> > > + if (!hdma->tpdma_tc)
> > > + return -ENOMEM;
> > > +
> > > + hdma->rpdma_tc = devm_kzalloc(dev,
> > > + sizeof(*hdma->rpdma_tc) * hdma->rx_pdmas,
> > > + GFP_KERNEL);
> > > + if (!hdma->rpdma_tc)
> > > + return -ENOMEM;
> > > +
> >
> > > + /* Initialize one-to-one mapping for each of the used queues */
> > > + for (i = 0; i < plat->tx_queues_to_use; i++) {
> > > + hdma->tvdma_tc[i] = i;
> > > + hdma->tpdma_tc[i] = i;
> > > + }
> > > + for (i = 0; i < plat->rx_queues_to_use; i++) {
> > > + hdma->rvdma_tc[i] = i;
> > > + hdma->rpdma_tc[i] = i;
> > > + }
> >
> > So the Traffic Class ID is initialized for the
> > tx_queues_to_use/rx_queues_to_use number of channels only, right? What
> > about the Virtual and Physical DMA-channels with numbers greater than
> > these values?
> >
> You have brought up a question that applies to remaining comments in
> this file as well.
> How the VDMA/PDMA mapping is used depends on the device/glue driver.
> For example in
> our SoC the remaining VDMAs are meant to be used with SRIOV virtual
> functions and not
> all of them are available for physical function.
> Since additional VDMAs/PDMAs remain unused in hardware I let them stay at their
> default values. No traffic is expected to be mapped to unused V/PDMAs.
> I couldn't think of a reason for it to be an issue from a driver perspective.
> Please let me know, if I am missing something or we need to address a
> use case with bigger scope.
> The responses for following comments also depend on what approach we take here.
If they are unused, then I suggest to follow the KISS principle. See
my last comment for details.
>
> > > + plat->dma_cfg->hdma_cfg = hdma;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +
> > > +void dw25gmac_dma_init(void __iomem *ioaddr,
> > > + struct stmmac_dma_cfg *dma_cfg)
> > > +{
> > > + u32 value;
> > > + u32 i;
> > > +
> > > + value = readl(ioaddr + XGMAC_DMA_SYSBUS_MODE);
> > > + value &= ~(XGMAC_AAL | XGMAC_EAME);
> > > + if (dma_cfg->aal)
> > > + value |= XGMAC_AAL;
> > > + if (dma_cfg->eame)
> > > + value |= XGMAC_EAME;
> > > + writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
> > > +
> > > + for (i = 0; i < dma_cfg->hdma_cfg->tx_vdmas; i++) {
> > > + value = rd_dma_ch_ind(ioaddr, MODE_TXDESCCTRL, i);
> > > + value &= ~XXVGMAC_TXDCSZ;
> > > + value |= FIELD_PREP(XXVGMAC_TXDCSZ,
> > > + XXVGMAC_TXDCSZ_256BYTES);
> > > + value &= ~XXVGMAC_TDPS;
> > > + value |= FIELD_PREP(XXVGMAC_TDPS, XXVGMAC_TDPS_HALF);
> > > + wr_dma_ch_ind(ioaddr, MODE_TXDESCCTRL, i, value);
> > > + }
> > > +
> > > + for (i = 0; i < dma_cfg->hdma_cfg->rx_vdmas; i++) {
> > > + value = rd_dma_ch_ind(ioaddr, MODE_RXDESCCTRL, i);
> > > + value &= ~XXVGMAC_RXDCSZ;
> > > + value |= FIELD_PREP(XXVGMAC_RXDCSZ,
> > > + XXVGMAC_RXDCSZ_256BYTES);
> > > + value &= ~XXVGMAC_RDPS;
> > > + value |= FIELD_PREP(XXVGMAC_TDPS, XXVGMAC_RDPS_HALF);
> > > + wr_dma_ch_ind(ioaddr, MODE_RXDESCCTRL, i, value);
> > > + }
> > > +
> >
> > > + for (i = 0; i < dma_cfg->hdma_cfg->tx_pdmas; i++) {
> > > + value = rd_dma_ch_ind(ioaddr, MODE_TXEXTCFG, i);
> > > + value &= ~(XXVGMAC_TXPBL | XXVGMAC_TPBLX8_MODE);
> > > + if (dma_cfg->pblx8)
> > > + value |= XXVGMAC_TPBLX8_MODE;
> > > + value |= FIELD_PREP(XXVGMAC_TXPBL, dma_cfg->pbl);
> > > + wr_dma_ch_ind(ioaddr, MODE_TXEXTCFG, i, value);
> > > + xgmac4_tp2tc_map(ioaddr, i, dma_cfg->hdma_cfg->tpdma_tc[i]);
> > > + }
> > > +
> > > + for (i = 0; i < dma_cfg->hdma_cfg->rx_pdmas; i++) {
> > > + value = rd_dma_ch_ind(ioaddr, MODE_RXEXTCFG, i);
> > > + value &= ~(XXVGMAC_RXPBL | XXVGMAC_RPBLX8_MODE);
> > > + if (dma_cfg->pblx8)
> > > + value |= XXVGMAC_RPBLX8_MODE;
> > > + value |= FIELD_PREP(XXVGMAC_RXPBL, dma_cfg->pbl);
> > > + wr_dma_ch_ind(ioaddr, MODE_RXEXTCFG, i, value);
> > > + xgmac4_rp2tc_map(ioaddr, i, dma_cfg->hdma_cfg->rpdma_tc[i]);
> >
> > What if tx_pdmas doesn't match plat_stmmacenet_data::tx_queues_to_use
> > and rx_pdmas doesn't match to plat_stmmacenet_data::rx_queues_to_use?
> >
> > If they don't then you'll get out of the initialized tpdma_tc/rpdma_tc
> > fields and these channels will be pre-initialized with the zero TC. Is
> > that what expected? I doubt so.
> >
> As mentioned in the previous response the remaining resources are unused
> and no traffic is mapped to those resources.
>
> > > + }
> > > +}
> > > +
> >
> > > +void dw25gmac_dma_init_tx_chan(struct stmmac_priv *priv,
> > > + void __iomem *ioaddr,
> > > + struct stmmac_dma_cfg *dma_cfg,
> > > + dma_addr_t dma_addr, u32 chan)
> > > +{
> > > + u32 value;
> > > +
> >
> > > + value = readl(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
> > > + value &= ~XXVGMAC_TVDMA2TCMP;
> > > + value |= FIELD_PREP(XXVGMAC_TVDMA2TCMP,
> > > + dma_cfg->hdma_cfg->tvdma_tc[chan]);
> > > + writel(value, ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
> >
> > Please note this will have only first
> > plat_stmmacenet_data::{tx_queues_to_use,rx_queues_to_use} VDMA
> > channels initialized. Don't you have much more than just 4 channels?
> >
> Yes, there are 32 VDMA channels on this device. In our application the
> additional channels are partitioned for use with SRIOV virtual functions.
> Similar to PDMA comment above, the additional VDMAs are not enabled,
> and left in default state.
> My thinking is, when another 25gmac device comes along that requires a
> different mapping we may need to add the ability to set the mapping in
> glue driver.
> We can support this by adding a check in dw25gmac_setup()
> @@ -1708,8 +1708,10 @@ int dw25gmac_setup(struct stmmac_priv *priv)
> mac->mii.clk_csr_shift = 19;
> mac->mii.clk_csr_mask = GENMASK(21, 19);
>
> - /* Allocate and initialize hdma mapping */
> - return dw25gmac_hdma_cfg_init(priv);
> + /* Allocate and initialize hdma mapping, if not done by glue driver. */
> + if (!priv->plat->dma_cfg->hdma_cfg)
> + return dw25gmac_hdma_cfg_init(priv);
> + return 0;
> }
So the use-case is actually hypothetical. Then I don't see a point in
adding the stmmac_hdma_cfg structure. See my last comment for details.
>
> > > +
> > > + writel(upper_32_bits(dma_addr),
> > > + ioaddr + XGMAC_DMA_CH_TxDESC_HADDR(chan));
> > > + writel(lower_32_bits(dma_addr),
> > > + ioaddr + XGMAC_DMA_CH_TxDESC_LADDR(chan));
> > > +}
> > > +
> > > +void dw25gmac_dma_init_rx_chan(struct stmmac_priv *priv,
> > > + void __iomem *ioaddr,
> > > + struct stmmac_dma_cfg *dma_cfg,
> > > + dma_addr_t dma_addr, u32 chan)
> > > +{
> > > + u32 value;
> > > +
> >
> > > + value = readl(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
> > > + value &= ~XXVGMAC_RVDMA2TCMP;
> > > + value |= FIELD_PREP(XXVGMAC_RVDMA2TCMP,
> > > + dma_cfg->hdma_cfg->rvdma_tc[chan]);
> > > + writel(value, ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
> >
> > The same question.
> >
> > > +
> > > + writel(upper_32_bits(dma_addr),
> > > + ioaddr + XGMAC_DMA_CH_RxDESC_HADDR(chan));
> > > + writel(lower_32_bits(dma_addr),
> > > + ioaddr + XGMAC_DMA_CH_RxDESC_LADDR(chan));
> > > +}
> >
> > These methods are called for each
> > plat_stmmacenet_data::{tx_queues_to_use,rx_queues_to_use}
> > DMA-channel/Queue. The static mapping means you'll have each
> > PDMA/Queue assigned a static traffic class ID corresponding to the
> > channel ID. Meanwhile the VDMA channels are supposed to be initialized
> > with the TC ID corresponding to the matching PDMA ID.
> >
> > The TC ID in this case is passed as the DMA/Queue channel ID. Then the
> > Tx/Rx DMA-channels init methods can be converted to:
> >
> > dw25gmac_dma_init_Xx_chan(chan)
> > {
> > /* Map each chan-th VDMA to the single chan PDMA by assigning
> > * the static TC ID.
> > */
> > for (i = chan; i < Xx_vdmas; i += (Xx_vdmas / Xx_queues_to_use)) {
> > /* Initialize VDMA channels */
> > XXVGMAC_TVDMA2TCMP = chan;
> > }
> >
> > /* Assign the static TC ID to the specified PDMA channel */
> > xgmac4_rp2tc_map(chan, chan)
> > }
> >
> > , where X={t,r}.
> >
> > Thus you can redistribute the loops implemented in dw25gmac_dma_init()
> > to the respective Tx/Rx DMA-channel init methods.
> >
> > Am I missing something?
> I think your visualization of HDMA may be going beyond the application
> I understand.
> We are allocating a VDMA for each of the TX/RX channels. The use of
> additional VDMAs
> depends on how the device is partitioned for virtualization.
> In the non-SRIOV case the remaining VDMAs will remain unused.
> Please let me know if I missed your question.
So you say that you need a 1:1 mapping between
First-VDMAs:PDMAs/Queues, and there are only
tx_queues_to_use/rx_queues_to_use pairs utilized. Right? If so I don't
really see a need in implementing the overcomplicated solution you
suggest, especially seeing the core driver already supports an
infrastructure for the DMA-Queue managing:
1. Rx path: dwxgmac2_map_mtl_to_dma() - set VDMA-Rx-Queue TC
dwxgmac2_rx_queue_enable()
2. Tx path: dwxgmac2_dma_tx_mode() - set VDMA-Tx-Queue TC
In the first case the mapping can be changed via the MTL DT-nodes. In
the second case the mapping is one-on-one static. Thus you'll be able
to keep the dw25gmac_dma_init_tx_chan()/dw25gmac_dma_init_rx_chan()
method simple initializing the PBL/Descriptor/etc-related stuff only,
as it's done for the DW QoS Eth and XGMAC/XLGMAC IP-cores. You won't
need to introduce a new structure stmmac_hdma_cfg, especially either
seeing it is anyway redundant for your use-case.
BTW could you clarify:
1. is the TCes specified for the VDMA/PDMA-queue mapping the same as
the TCs assigned to the Tx-Queues in the MTL_TxQ0_Operation_Mode
register? If they aren't, then what is the relation between them?
2. Similarly, if there is a TC-based Rx VDMA/Queue mapping, then
what's the function of the MTL_RxQ_DMA_MAP* registers?
-Serge(y)
> >
> > -Serge()
> >
> > > [...]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v5 2/5] net: stmmac: Add basic dw25gmac support in stmmac core
2024-10-04 16:05 ` Jitendra Vegiraju
2024-10-04 23:45 ` Serge Semin
@ 2024-10-11 0:01 ` Serge Semin
1 sibling, 0 replies; 20+ messages in thread
From: Serge Semin @ 2024-10-11 0:01 UTC (permalink / raw)
To: Jitendra Vegiraju
Cc: netdev, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, bcm-kernel-feedback-list, richardcochran, ast,
daniel, hawk, john.fastabend, rmk+kernel, ahalaney, xiaolei.wang,
rohan.g.thomas, Jianheng.Zhang, linux-kernel, linux-stm32,
linux-arm-kernel, bpf, andrew, linux, horms, florian.fainelli
Hi Jitendra
On Fri, Oct 04, 2024 at 09:05:36AM GMT, Jitendra Vegiraju wrote:
> Hi Serge,
>
> On Mon, Sep 16, 2024 at 4:32 PM Jitendra Vegiraju
> <jitendra.vegiraju@broadcom.com> wrote:
> >
> ...
>
> When you get a chance, I would like to get your input on the approach we need
> to take to incrementally add dw25gmac support.
>
> In the last conversation there were some open questions around the case of
> initializing unused VDMA channels and related combination scenarios.
>
> The hdma mapping provides flexibility for virtualization. However, our
> SoC device cannot use all VDMAs with one PCI function. The VDMAs are
> partitioned for SRIOV use in the firmware. This SoC defaults to 8 functions
> with 4 VDMA channels each. The initial effort is to support one PCI physical
> function with 4 VDMA channels.
> Also, currently the stmmac driver has inferred one-to-one relation between
> netif channels and physical DMAs. It would be a complex change to support
> each VDMA as its own netif channel and mapping fewer physical DMAs.
> Hence, for initial submission one-to-one mapping is assumed.
>
> As you mentioned, a static one-to-one mapping of VDMA-TC-PDMA doesn't
> require the additional complexity of managing these mappings as proposed
> in the current patch series with *struct stmmac_hdma_cfg*.
>
> To introduce dw25gmac incrementally, I am thinking of two approaches,
> 1. Take the current patch series forward using *struct stmmac_hdma_cfg*,
> keeping the unused VDMAs in default state. We need to fix the
> initialization
> loops to only initialize the VDMA and PDMAs being used.
> 2. Simplify the initial patch by removing *struct hdma_cfg* from the patch
> series and still use static VDMA-TC-PDMA mapping.
> Please share your thoughts.
> If it helps, I can send patch series with option 2 above after
> addressing all other
> review comments.
IMO approach 2 seems more preferable. Please find my comments to your
previous email in this thread:
https://lore.kernel.org/netdev/sn5epdl4jdwj4t6mo55w4poz6vkdcuzceezsmpb7447hmaj2ot@gmlxst7gdcix/
>
> Appreciate your guidance!
Always welcome. Thank you for submitting the patches and your patience
to proceed with the review process.
-Serge(y)
> -Jitendra
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v5 2/5] net: stmmac: Add basic dw25gmac support in stmmac core
2024-10-10 23:55 ` Serge Semin
@ 2024-10-14 18:24 ` Jitendra Vegiraju
0 siblings, 0 replies; 20+ messages in thread
From: Jitendra Vegiraju @ 2024-10-14 18:24 UTC (permalink / raw)
To: Serge Semin
Cc: netdev, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, bcm-kernel-feedback-list, richardcochran, ast,
daniel, hawk, john.fastabend, rmk+kernel, ahalaney, xiaolei.wang,
rohan.g.thomas, Jianheng.Zhang, linux-kernel, linux-stm32,
linux-arm-kernel, bpf, andrew, linux, horms, florian.fainelli
Hi Serge,
Thanks for the feedback.
On Thu, Oct 10, 2024 at 4:55 PM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> On Mon, Sep 16, 2024 at 04:32:33PM GMT, Jitendra Vegiraju wrote:
> > Hi Serge,
> >
> > On Tue, Sep 10, 2024 at 12:25 PM Serge Semin <fancer.lancer@gmail.com> wrote:
> > >
> > > > +static u32 decode_vdma_count(u32 regval)
> > > > +{
> > > > + /* compressed encoding for vdma count
> > > > + * regval: VDMA count
> > > > + * 0-15 : 1 - 16
> > > > + * 16-19 : 20, 24, 28, 32
> > > > + * 20-23 : 40, 48, 56, 64
> > > > + * 24-27 : 80, 96, 112, 128
> > > > + */
> > > > + if (regval < 16)
> > > > + return regval + 1;
> > > > + return (4 << ((regval - 16) / 4)) * ((regval % 4) + 5);
> > >
> > > The shortest code isn't always the best one. This one gives me a
> > > headache in trying to decipher whether it really matches to what is
> > > described in the comment. What about just:
> > >
> > > if (regval < 16) /* Direct mapping */
> > > return regval + 1;
> > > else if (regval < 20) /* 20, 24, 28, 32 */
> > > return 20 + (regval - 16) * 4;
> > > else if (regval < 24) /* 40, 48, 56, 64 */
> > > return 40 + (regval - 20) * 8;
> > > else if (regval < 28) /* 80, 96, 112, 128 */
> > > return 80 + (regval - 24) * 16;
> > >
> > > ?
> > Couldn't agree more :)
> > Thanks, I will replace it with your code, which is definitely more readable.
> >
> > >
> > > > +}
> > > > +
> > > > +static void dw25gmac_read_hdma_limits(void __iomem *ioaddr,
> > > > + struct stmmac_hdma_cfg *hdma)
> > > > +{
> > > > + u32 hw_cap;
> > > > +
> > > > + /* Get VDMA/PDMA counts from HW */
> > > > + hw_cap = readl(ioaddr + XGMAC_HW_FEATURE2);
> > >
> > >
> > > > + hdma->tx_vdmas = decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_TXCNT,
> > > > + hw_cap));
> > > > + hdma->rx_vdmas = decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_RXCNT,
> > > > + hw_cap));
> > > > + hdma->tx_pdmas = FIELD_GET(XGMAC_HWFEAT_TXQCNT, hw_cap) + 1;
> > > > + hdma->rx_pdmas = FIELD_GET(XGMAC_HWFEAT_RXQCNT, hw_cap) + 1;
> > >
> > > Hmm, these are the Tx/Rx DMA-channels and Tx/Rx MTL-queues count
> > > fields. Can't you just use the
> > > dma_features::{number_tx_channel,number_tx_queues} and
> > > dma_features::{number_rx_channel,number_rx_queues} fields to store the
> > > retrieved data?
> > >
> > > Moreover why not to add the code above to the dwxgmac2_get_hw_feature() method?
> > >
> > Thanks, I missed the reuse of existing fields.
>
> > However, since the VDMA count has a slightly bigger bitmask, we need to extract
> > VDMA channel count as per DW25GMAC spec.
> > Instead of duplicating dwxgmac2_get_hw_feature(), should we add wrapper for
> > dw25gmac, something like the following?
>
> Yeah, and there is the encoded fields value. Your suggestion sounds
> reasonable.
>
> > dw25gmac_get_hw_feature(ioaddr, dma_cap)
> > {
> > u32 hw_cap;
>
> > int rc;
>
> s/rc/ret
>
> + newline
>
> > rc = dwxgmac2_get_hw_feature(ioaddr, dma_cap);
>
> newline
>
> > /* Get VDMA counts from HW */
> > hw_cap = readl(ioaddr + XGMAC_HW_FEATURE2);
> > dma_cap->num_tx_channels =
> > decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_TXCNT,
> > hw_cap));
> > dma_cap->num_rx_channels =
> > decode_vdma_count(FIELD_GET(XXVGMAC_HWFEAT_VDMA_RXCNT,
> > hw_cap));
>
> newline
>
Ack
> > return rc;
> > }
> >
> > > > +}
> > > > +
> > > > +int dw25gmac_hdma_cfg_init(struct stmmac_priv *priv)
> > > > +{
> > > > + struct plat_stmmacenet_data *plat = priv->plat;
> > > > + struct device *dev = priv->device;
> > > > + struct stmmac_hdma_cfg *hdma;
> > > > + int i;
> > > > +
> > > > + hdma = devm_kzalloc(dev,
> > > > + sizeof(*plat->dma_cfg->hdma_cfg),
> > > > + GFP_KERNEL);
> > > > + if (!hdma)
> > > > + return -ENOMEM;
> > > > +
> > > > + dw25gmac_read_hdma_limits(priv->ioaddr, hdma);
> > > > +
> > > > + hdma->tvdma_tc = devm_kzalloc(dev,
> > > > + sizeof(*hdma->tvdma_tc) * hdma->tx_vdmas,
> > > > + GFP_KERNEL);
> > > > + if (!hdma->tvdma_tc)
> > > > + return -ENOMEM;
> > > > +
> > > > + hdma->rvdma_tc = devm_kzalloc(dev,
> > > > + sizeof(*hdma->rvdma_tc) * hdma->rx_vdmas,
> > > > + GFP_KERNEL);
> > > > + if (!hdma->rvdma_tc)
> > > > + return -ENOMEM;
> > > > +
> > > > + hdma->tpdma_tc = devm_kzalloc(dev,
> > > > + sizeof(*hdma->tpdma_tc) * hdma->tx_pdmas,
> > > > + GFP_KERNEL);
> > > > + if (!hdma->tpdma_tc)
> > > > + return -ENOMEM;
> > > > +
> > > > + hdma->rpdma_tc = devm_kzalloc(dev,
> > > > + sizeof(*hdma->rpdma_tc) * hdma->rx_pdmas,
> > > > + GFP_KERNEL);
> > > > + if (!hdma->rpdma_tc)
> > > > + return -ENOMEM;
> > > > +
> > >
> > > > + /* Initialize one-to-one mapping for each of the used queues */
> > > > + for (i = 0; i < plat->tx_queues_to_use; i++) {
> > > > + hdma->tvdma_tc[i] = i;
> > > > + hdma->tpdma_tc[i] = i;
> > > > + }
> > > > + for (i = 0; i < plat->rx_queues_to_use; i++) {
> > > > + hdma->rvdma_tc[i] = i;
> > > > + hdma->rpdma_tc[i] = i;
> > > > + }
> > >
> > > So the Traffic Class ID is initialized for the
> > > tx_queues_to_use/rx_queues_to_use number of channels only, right? What
> > > about the Virtual and Physical DMA-channels with numbers greater than
> > > these values?
> > >
>
> > You have brought up a question that applies to remaining comments in
> > this file as well.
> > How the VDMA/PDMA mapping is used depends on the device/glue driver.
> > For example in
> > our SoC the remaining VDMAs are meant to be used with SRIOV virtual
> > functions and not
> > all of them are available for physical function.
> > Since additional VDMAs/PDMAs remain unused in hardware I let them stay at their
> > default values. No traffic is expected to be mapped to unused V/PDMAs.
> > I couldn't think of a reason for it to be an issue from a driver perspective.
> > Please let me know, if I am missing something or we need to address a
> > use case with bigger scope.
> > The responses for following comments also depend on what approach we take here.
>
> If they are unused, then I suggest to follow the KISS principle. See
> my last comment for details.
>
> >
> > > > + plat->dma_cfg->hdma_cfg = hdma;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +
> > > > +void dw25gmac_dma_init(void __iomem *ioaddr,
> > > > + struct stmmac_dma_cfg *dma_cfg)
> > > > +{
> > > > + u32 value;
> > > > + u32 i;
> > > > +
> > > > + value = readl(ioaddr + XGMAC_DMA_SYSBUS_MODE);
> > > > + value &= ~(XGMAC_AAL | XGMAC_EAME);
> > > > + if (dma_cfg->aal)
> > > > + value |= XGMAC_AAL;
> > > > + if (dma_cfg->eame)
> > > > + value |= XGMAC_EAME;
> > > > + writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
> > > > +
> > > > + for (i = 0; i < dma_cfg->hdma_cfg->tx_vdmas; i++) {
> > > > + value = rd_dma_ch_ind(ioaddr, MODE_TXDESCCTRL, i);
> > > > + value &= ~XXVGMAC_TXDCSZ;
> > > > + value |= FIELD_PREP(XXVGMAC_TXDCSZ,
> > > > + XXVGMAC_TXDCSZ_256BYTES);
> > > > + value &= ~XXVGMAC_TDPS;
> > > > + value |= FIELD_PREP(XXVGMAC_TDPS, XXVGMAC_TDPS_HALF);
> > > > + wr_dma_ch_ind(ioaddr, MODE_TXDESCCTRL, i, value);
> > > > + }
> > > > +
> > > > + for (i = 0; i < dma_cfg->hdma_cfg->rx_vdmas; i++) {
> > > > + value = rd_dma_ch_ind(ioaddr, MODE_RXDESCCTRL, i);
> > > > + value &= ~XXVGMAC_RXDCSZ;
> > > > + value |= FIELD_PREP(XXVGMAC_RXDCSZ,
> > > > + XXVGMAC_RXDCSZ_256BYTES);
> > > > + value &= ~XXVGMAC_RDPS;
> > > > + value |= FIELD_PREP(XXVGMAC_TDPS, XXVGMAC_RDPS_HALF);
> > > > + wr_dma_ch_ind(ioaddr, MODE_RXDESCCTRL, i, value);
> > > > + }
> > > > +
> > >
> > > > + for (i = 0; i < dma_cfg->hdma_cfg->tx_pdmas; i++) {
> > > > + value = rd_dma_ch_ind(ioaddr, MODE_TXEXTCFG, i);
> > > > + value &= ~(XXVGMAC_TXPBL | XXVGMAC_TPBLX8_MODE);
> > > > + if (dma_cfg->pblx8)
> > > > + value |= XXVGMAC_TPBLX8_MODE;
> > > > + value |= FIELD_PREP(XXVGMAC_TXPBL, dma_cfg->pbl);
> > > > + wr_dma_ch_ind(ioaddr, MODE_TXEXTCFG, i, value);
> > > > + xgmac4_tp2tc_map(ioaddr, i, dma_cfg->hdma_cfg->tpdma_tc[i]);
> > > > + }
> > > > +
> > > > + for (i = 0; i < dma_cfg->hdma_cfg->rx_pdmas; i++) {
> > > > + value = rd_dma_ch_ind(ioaddr, MODE_RXEXTCFG, i);
> > > > + value &= ~(XXVGMAC_RXPBL | XXVGMAC_RPBLX8_MODE);
> > > > + if (dma_cfg->pblx8)
> > > > + value |= XXVGMAC_RPBLX8_MODE;
> > > > + value |= FIELD_PREP(XXVGMAC_RXPBL, dma_cfg->pbl);
> > > > + wr_dma_ch_ind(ioaddr, MODE_RXEXTCFG, i, value);
> > > > + xgmac4_rp2tc_map(ioaddr, i, dma_cfg->hdma_cfg->rpdma_tc[i]);
> > >
> > > What if tx_pdmas doesn't match plat_stmmacenet_data::tx_queues_to_use
> > > and rx_pdmas doesn't match to plat_stmmacenet_data::rx_queues_to_use?
> > >
> > > If they don't then you'll get out of the initialized tpdma_tc/rpdma_tc
> > > fields and these channels will be pre-initialized with the zero TC. Is
> > > that what expected? I doubt so.
> > >
> > As mentioned in the previous response the remaining resources are unused
> > and no traffic is mapped to those resources.
> >
> > > > + }
> > > > +}
> > > > +
> > >
> > > > +void dw25gmac_dma_init_tx_chan(struct stmmac_priv *priv,
> > > > + void __iomem *ioaddr,
> > > > + struct stmmac_dma_cfg *dma_cfg,
> > > > + dma_addr_t dma_addr, u32 chan)
> > > > +{
> > > > + u32 value;
> > > > +
> > >
> > > > + value = readl(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
> > > > + value &= ~XXVGMAC_TVDMA2TCMP;
> > > > + value |= FIELD_PREP(XXVGMAC_TVDMA2TCMP,
> > > > + dma_cfg->hdma_cfg->tvdma_tc[chan]);
> > > > + writel(value, ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
> > >
> > > Please note this will have only first
> > > plat_stmmacenet_data::{tx_queues_to_use,rx_queues_to_use} VDMA
> > > channels initialized. Don't you have much more than just 4 channels?
> > >
> > Yes, there are 32 VDMA channels on this device. In our application the
> > additional channels are partitioned for use with SRIOV virtual functions.
> > Similar to PDMA comment above, the additional VDMAs are not enabled,
> > and left in default state.
> > My thinking is, when another 25gmac device comes along that requires a
> > different mapping we may need to add the ability to set the mapping in
> > glue driver.
> > We can support this by adding a check in dw25gmac_setup()
> > @@ -1708,8 +1708,10 @@ int dw25gmac_setup(struct stmmac_priv *priv)
> > mac->mii.clk_csr_shift = 19;
> > mac->mii.clk_csr_mask = GENMASK(21, 19);
> >
> > - /* Allocate and initialize hdma mapping */
> > - return dw25gmac_hdma_cfg_init(priv);
> > + /* Allocate and initialize hdma mapping, if not done by glue driver. */
> > + if (!priv->plat->dma_cfg->hdma_cfg)
> > + return dw25gmac_hdma_cfg_init(priv);
> > + return 0;
> > }
>
> So the use-case is actually hypothetical. Then I don't see a point in
> adding the stmmac_hdma_cfg structure. See my last comment for details.
>
Yes, It's better to not over-complicate the patch for the hypothetical case.
I will remove the new struct in next update.
> >
> > > > +
> > > > + writel(upper_32_bits(dma_addr),
> > > > + ioaddr + XGMAC_DMA_CH_TxDESC_HADDR(chan));
> > > > + writel(lower_32_bits(dma_addr),
> > > > + ioaddr + XGMAC_DMA_CH_TxDESC_LADDR(chan));
> > > > +}
> > > > +
> > > > +void dw25gmac_dma_init_rx_chan(struct stmmac_priv *priv,
> > > > + void __iomem *ioaddr,
> > > > + struct stmmac_dma_cfg *dma_cfg,
> > > > + dma_addr_t dma_addr, u32 chan)
> > > > +{
> > > > + u32 value;
> > > > +
> > >
> > > > + value = readl(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
> > > > + value &= ~XXVGMAC_RVDMA2TCMP;
> > > > + value |= FIELD_PREP(XXVGMAC_RVDMA2TCMP,
> > > > + dma_cfg->hdma_cfg->rvdma_tc[chan]);
> > > > + writel(value, ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
> > >
> > > The same question.
> > >
> > > > +
> > > > + writel(upper_32_bits(dma_addr),
> > > > + ioaddr + XGMAC_DMA_CH_RxDESC_HADDR(chan));
> > > > + writel(lower_32_bits(dma_addr),
> > > > + ioaddr + XGMAC_DMA_CH_RxDESC_LADDR(chan));
> > > > +}
> > >
> > > These methods are called for each
> > > plat_stmmacenet_data::{tx_queues_to_use,rx_queues_to_use}
> > > DMA-channel/Queue. The static mapping means you'll have each
> > > PDMA/Queue assigned a static traffic class ID corresponding to the
> > > channel ID. Meanwhile the VDMA channels are supposed to be initialized
> > > with the TC ID corresponding to the matching PDMA ID.
> > >
> > > The TC ID in this case is passed as the DMA/Queue channel ID. Then the
> > > Tx/Rx DMA-channels init methods can be converted to:
> > >
> > > dw25gmac_dma_init_Xx_chan(chan)
> > > {
> > > /* Map each chan-th VDMA to the single chan PDMA by assigning
> > > * the static TC ID.
> > > */
> > > for (i = chan; i < Xx_vdmas; i += (Xx_vdmas / Xx_queues_to_use)) {
> > > /* Initialize VDMA channels */
> > > XXVGMAC_TVDMA2TCMP = chan;
> > > }
> > >
> > > /* Assign the static TC ID to the specified PDMA channel */
> > > xgmac4_rp2tc_map(chan, chan)
> > > }
> > >
> > > , where X={t,r}.
> > >
> > > Thus you can redistribute the loops implemented in dw25gmac_dma_init()
> > > to the respective Tx/Rx DMA-channel init methods.
> > >
> > > Am I missing something?
> > I think your visualization of HDMA may be going beyond the application
> > I understand.
> > We are allocating a VDMA for each of the TX/RX channels. The use of
> > additional VDMAs
> > depends on how the device is partitioned for virtualization.
> > In the non-SRIOV case the remaining VDMAs will remain unused.
> > Please let me know if I missed your question.
>
> So you say that you need a 1:1 mapping between
> First-VDMAs:PDMAs/Queues, and there are only
> tx_queues_to_use/rx_queues_to_use pairs utilized. Right? If so I don't
> really see a need in implementing the overcomplicated solution you
> suggest, especially seeing the core driver already supports an
> infrastructure for the DMA-Queue managing:
> 1. Rx path: dwxgmac2_map_mtl_to_dma() - set VDMA-Rx-Queue TC
> dwxgmac2_rx_queue_enable()
> 2. Tx path: dwxgmac2_dma_tx_mode() - set VDMA-Tx-Queue TC
>
> In the first case the mapping can be changed via the MTL DT-nodes. In
> the second case the mapping is one-on-one static. Thus you'll be able
> to keep the dw25gmac_dma_init_tx_chan()/dw25gmac_dma_init_rx_chan()
> method simple initializing the PBL/Descriptor/etc-related stuff only,
> as it's done for the DW QoS Eth and XGMAC/XLGMAC IP-cores. You won't
> need to introduce a new structure stmmac_hdma_cfg, especially either
> seeing it is anyway redundant for your use-case.
>
> BTW could you clarify:
>
> 1. is the TCes specified for the VDMA/PDMA-queue mapping the same as
> the TCs assigned to the Tx-Queues in the MTL_TxQ0_Operation_Mode
> register? If they aren't, then what is the relation between them?
>
In register documentation for HDMA XGMAC IP, the Q2TCMAP field in
MTL_TxQ0_Operation_Mode is marked as *reserved" and is ignored.
The VDMA->TC->PDMA mapping is used for DMA scheduling.
This static mapping can be done in dw25gmac_dma_init_tx_chan() and
dw25gmac_dma_init_rx_chan().
> 2. Similarly, if there is a TC-based Rx VDMA/Queue mapping, then
> what's the function of the MTL_RxQ_DMA_MAP* registers?
In the RX direction MTL_RxQ_DMA_MAP* decides the VDMA channel for a
given RXQ.
The TC for VDMA channel is decided by VDMA/TC mapping. Then TC to
PDMA mapping is used to pick PDMA for actual DMA operation.
>
> -Serge(y)
>
> > >
> > > -Serge()
> > >
> > > > [...]
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2024-10-14 18:26 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-04 5:48 [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x jitendra.vegiraju
2024-09-04 5:48 ` [PATCH net-next v5 1/5] net: stmmac: Add HDMA mapping for dw25gmac support jitendra.vegiraju
2024-09-10 18:37 ` Serge Semin
2024-09-11 21:50 ` Jitendra Vegiraju
2024-09-04 5:48 ` [PATCH net-next v5 2/5] net: stmmac: Add basic dw25gmac support in stmmac core jitendra.vegiraju
2024-09-10 19:24 ` Serge Semin
2024-09-16 23:32 ` Jitendra Vegiraju
2024-10-04 16:05 ` Jitendra Vegiraju
2024-10-04 23:45 ` Serge Semin
2024-10-11 0:01 ` Serge Semin
2024-10-10 23:55 ` Serge Semin
2024-10-14 18:24 ` Jitendra Vegiraju
2024-09-04 5:48 ` [PATCH net-next v5 3/5] net: stmmac: Integrate dw25gmac into stmmac hwif handling jitendra.vegiraju
2024-09-04 5:48 ` [PATCH net-next v5 4/5] net: stmmac: Add PCI driver support for BCM8958x jitendra.vegiraju
2024-09-10 19:51 ` Serge Semin
2024-09-16 23:43 ` Jitendra Vegiraju
2024-09-04 5:48 ` [PATCH net-next v5 5/5] net: stmmac: Add BCM8958x driver to build system jitendra.vegiraju
2024-09-06 12:14 ` [net-next v5 0/5] net: stmmac: Add PCI driver support for BCM8958x Serge Semin
2024-09-10 11:29 ` Paolo Abeni
2024-09-10 11:55 ` Serge Semin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox