linux-kernel - Re: [PATCH net-next v5 1/5] net: stmmac: Add HDMA mapping for dw25gmac support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7foqi3vdgc3kvyw5rrnqsqsakgfgcrhw5sihnqwza4okdnh5dd@pdsdjn32ya6u>
Date: Tue, 10 Sep 2024 21:37:52 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: jitendra.vegiraju@...adcom.com
Cc: netdev@...r.kernel.org, alexandre.torgue@...s.st.com, 
	joabreu@...opsys.com, davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, 
	pabeni@...hat.com, mcoquelin.stm32@...il.com, bcm-kernel-feedback-list@...adcom.com, 
	richardcochran@...il.com, ast@...nel.org, daniel@...earbox.net, hawk@...nel.org, 
	john.fastabend@...il.com, rmk+kernel@...linux.org.uk, ahalaney@...hat.com, 
	xiaolei.wang@...driver.com, rohan.g.thomas@...el.com, Jianheng.Zhang@...opsys.com, 
	linux-kernel@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com, 
	linux-arm-kernel@...ts.infradead.org, bpf@...r.kernel.org, andrew@...n.ch, linux@...linux.org.uk, 
	horms@...nel.org, florian.fainelli@...adcom.com
Subject: Re: [PATCH net-next v5 1/5] net: stmmac: Add HDMA mapping for
 dw25gmac support

Hi Jitendra

On Tue, Sep 03, 2024 at 10:48:11PM -0700, jitendra.vegiraju@...adcom.com wrote:
> From: Jitendra Vegiraju <jitendra.vegiraju@...adcom.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@...adcom.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
>