diff mbox series

[net-next,v5,1/5] net: stmmac: Add HDMA mapping for dw25gmac support

Message ID 20240904054815.1341712-2-jitendra.vegiraju@broadcom.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: stmmac: Add PCI driver support for BCM8958x | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 16 this patch: 16
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 16 this patch: 16
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 26 this patch: 26
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 65 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-09-07--06-00 (tests: 722)

Commit Message

Jitendra Vegiraju Sept. 4, 2024, 5:48 a.m. UTC 
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(+)

Comments

Serge Semin Sept. 10, 2024, 6:37 p.m. UTC | #1 
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
>
Jitendra Vegiraju Sept. 11, 2024, 9:50 p.m. UTC | #2 
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
> >
diff mbox series

Patch

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