[PATCH v2 1/4] net: ravb: Make DBAT entry count configurable per-SoC

[PATCH v2 1/4] net: ravb: Make DBAT entry count configurable per-SoC

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

The number of CDARq (Current Descriptor Address Register) registers is not
fixed to 22 across all SoC variants. For example, the GBETH implementation
uses only two entries. Hardcoding the value leads to incorrect resource
allocation on such platforms.

Pass the DBAT entry count through the per-SoC hardware info struct and use
it during probe instead of relying on a fixed constant. This ensures
correct descriptor table sizing and initialization across different SoCs.

Fixes: feab85c7ccea ("ravb: Add support for RZ/G2L SoC")
Cc: stable@vger.kernel.org
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
v1->v2:
- Added Reviewed-by tag from Niklas.
---
 drivers/net/ethernet/renesas/ravb.h      | 2 +-
 drivers/net/ethernet/renesas/ravb_main.c | 9 +++++++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 7b48060c250b..d65cd83ddd16 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -1017,7 +1017,6 @@ enum CSR2_BIT {
 #define CSR2_CSUM_ENABLE (CSR2_RTCP4 | CSR2_RUDP4 | CSR2_RICMP4 | \
 			  CSR2_RTCP6 | CSR2_RUDP6 | CSR2_RICMP6)
 
-#define DBAT_ENTRY_NUM	22
 #define RX_QUEUE_OFFSET	4
 #define NUM_RX_QUEUE	2
 #define NUM_TX_QUEUE	2
@@ -1062,6 +1061,7 @@ struct ravb_hw_info {
 	u32 rx_max_frame_size;
 	u32 rx_buffer_size;
 	u32 rx_desc_size;
+	u32 dbat_entry_num;
 	unsigned aligned_tx: 1;
 	unsigned coalesce_irqs:1;	/* Needs software IRQ coalescing */
 
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 9d3bd65b85ff..69d382e8757d 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2694,6 +2694,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.rx_buffer_size = SZ_2K +
 			  SKB_DATA_ALIGN(sizeof(struct skb_shared_info)),
 	.rx_desc_size = sizeof(struct ravb_ex_rx_desc),
+	.dbat_entry_num = 22,
 	.aligned_tx = 1,
 	.gptp = 1,
 	.nc_queues = 1,
@@ -2717,6 +2718,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.rx_buffer_size = SZ_2K +
 			  SKB_DATA_ALIGN(sizeof(struct skb_shared_info)),
 	.rx_desc_size = sizeof(struct ravb_ex_rx_desc),
+	.dbat_entry_num = 22,
 	.internal_delay = 1,
 	.tx_counters = 1,
 	.multi_irqs = 1,
@@ -2743,6 +2745,7 @@ static const struct ravb_hw_info ravb_gen4_hw_info = {
 	.rx_buffer_size = SZ_2K +
 			  SKB_DATA_ALIGN(sizeof(struct skb_shared_info)),
 	.rx_desc_size = sizeof(struct ravb_ex_rx_desc),
+	.dbat_entry_num = 22,
 	.internal_delay = 1,
 	.tx_counters = 1,
 	.multi_irqs = 1,
@@ -2769,6 +2772,7 @@ static const struct ravb_hw_info ravb_rzv2m_hw_info = {
 	.rx_buffer_size = SZ_2K +
 			  SKB_DATA_ALIGN(sizeof(struct skb_shared_info)),
 	.rx_desc_size = sizeof(struct ravb_ex_rx_desc),
+	.dbat_entry_num = 22,
 	.multi_irqs = 1,
 	.err_mgmt_irqs = 1,
 	.gptp = 1,
@@ -2794,6 +2798,7 @@ static const struct ravb_hw_info gbeth_hw_info = {
 	.rx_max_frame_size = SZ_8K,
 	.rx_buffer_size = SZ_2K,
 	.rx_desc_size = sizeof(struct ravb_rx_desc),
+	.dbat_entry_num = 2,
 	.aligned_tx = 1,
 	.coalesce_irqs = 1,
 	.tx_counters = 1,
@@ -3025,7 +3030,7 @@ static int ravb_probe(struct platform_device *pdev)
 	ravb_parse_delay_mode(np, ndev);
 
 	/* Allocate descriptor base address table */
-	priv->desc_bat_size = sizeof(struct ravb_desc) * DBAT_ENTRY_NUM;
+	priv->desc_bat_size = sizeof(struct ravb_desc) * info->dbat_entry_num;
 	priv->desc_bat = dma_alloc_coherent(ndev->dev.parent, priv->desc_bat_size,
 					    &priv->desc_bat_dma, GFP_KERNEL);
 	if (!priv->desc_bat) {
@@ -3035,7 +3040,7 @@ static int ravb_probe(struct platform_device *pdev)
 		error = -ENOMEM;
 		goto out_rpm_put;
 	}
-	for (q = RAVB_BE; q < DBAT_ENTRY_NUM; q++)
+	for (q = RAVB_BE; q < info->dbat_entry_num; q++)
 		priv->desc_bat[q].die_dt = DT_EOS;
 
 	/* Initialise HW timestamp list */
-- 
2.43.0

Re: [PATCH v2 1/4] net: ravb: Make DBAT entry count configurable per-SoC

[Jakub Kicinski]

On Fri, 17 Oct 2025 16:18:27 +0100 Prabhakar wrote:
> The number of CDARq (Current Descriptor Address Register) registers is not
> fixed to 22 across all SoC variants. For example, the GBETH implementation
> uses only two entries. Hardcoding the value leads to incorrect resource
> allocation on such platforms.

What is the user-visible problem? "Incorrect resource allocation" could
mean anything from slight waste of system memory to the device falling
over.

If it's the former this is not a fix..

> Pass the DBAT entry count through the per-SoC hardware info struct and use
> it during probe instead of relying on a fixed constant. This ensures
> correct descriptor table sizing and initialization across different SoCs.
> 
> Fixes: feab85c7ccea ("ravb: Add support for RZ/G2L SoC")
> Cc: stable@vger.kernel.org

Re: [PATCH v2 1/4] net: ravb: Make DBAT entry count configurable per-SoC

[Lad, Prabhakar]

Hi Jakub,

Thank you for the review.

On Thu, Oct 23, 2025 at 2:13 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 17 Oct 2025 16:18:27 +0100 Prabhakar wrote:
> > The number of CDARq (Current Descriptor Address Register) registers is not
> > fixed to 22 across all SoC variants. For example, the GBETH implementation
> > uses only two entries. Hardcoding the value leads to incorrect resource
> > allocation on such platforms.
>
> What is the user-visible problem? "Incorrect resource allocation" could
> mean anything from slight waste of system memory to the device falling
> over.
>
> If it's the former this is not a fix..
>
Ok, I'll drop the fixes and cc to stable and send it for net-next.

Cheers,
Prabhakar

[PATCH v2 2/4] net: ravb: Allocate correct number of queues based on SoC support

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

On SoCs that only support the best-effort queue and not the network
control queue, calling alloc_etherdev_mqs() with fixed values for
TX/RX queues is not appropriate. Use the nc_queues flag from the
per-SoC match data to determine whether the network control queue
is available, and fall back to a single TX/RX queue when it is not.
This ensures correct queue allocation across all supported SoCs.

Fixes: a92f4f0662bf ("ravb: Add nc_queue to struct ravb_hw_info")
Cc: stable@vger.kernel.org
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
v1->v2:
- Added Reviewed-by tag from Niklas.
---
 drivers/net/ethernet/renesas/ravb_main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 69d382e8757d..a200e205825a 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2926,13 +2926,14 @@ static int ravb_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(rstc),
 				     "failed to get cpg reset\n");
 
+	info = of_device_get_match_data(&pdev->dev);
+
 	ndev = alloc_etherdev_mqs(sizeof(struct ravb_private),
-				  NUM_TX_QUEUE, NUM_RX_QUEUE);
+				  info->nc_queues ? NUM_TX_QUEUE : 1,
+				  info->nc_queues ? NUM_RX_QUEUE : 1);
 	if (!ndev)
 		return -ENOMEM;
 
-	info = of_device_get_match_data(&pdev->dev);
-
 	ndev->features = info->net_features;
 	ndev->hw_features = info->net_hw_features;
 	ndev->vlan_features = info->vlan_features;
-- 
2.43.0

Re: [PATCH v2 2/4] net: ravb: Allocate correct number of queues based on SoC support

[Jakub Kicinski]

On Fri, 17 Oct 2025 16:18:28 +0100 Prabhakar wrote:
> On SoCs that only support the best-effort queue and not the network
> control queue, calling alloc_etherdev_mqs() with fixed values for
> TX/RX queues is not appropriate. Use the nc_queues flag from the
> per-SoC match data to determine whether the network control queue
> is available, and fall back to a single TX/RX queue when it is not.
> This ensures correct queue allocation across all supported SoCs.

Same comment as on patch 1, what is the _real_ problem?
Allocating a bit too much memory is not an stable-worthy issue.

Re: [PATCH v2 2/4] net: ravb: Allocate correct number of queues based on SoC support

[Lad, Prabhakar]

Hi Jakub,

Thank you for the review.

On Thu, Oct 23, 2025 at 2:13 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 17 Oct 2025 16:18:28 +0100 Prabhakar wrote:
> > On SoCs that only support the best-effort queue and not the network
> > control queue, calling alloc_etherdev_mqs() with fixed values for
> > TX/RX queues is not appropriate. Use the nc_queues flag from the
> > per-SoC match data to determine whether the network control queue
> > is available, and fall back to a single TX/RX queue when it is not.
> > This ensures correct queue allocation across all supported SoCs.
>
> Same comment as on patch 1, what is the _real_ problem?
> Allocating a bit too much memory is not an stable-worthy issue.

Ok, I will drop the fixes tag and cc to stable and post it for net-next.

Cheers,
Prabhakar

[PATCH v2 3/4] net: ravb: Enforce descriptor type ordering

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Ensure the TX descriptor type fields are published in a safe order so the
DMA engine never begins processing a descriptor chain before all descriptor
fields are fully initialised.

For multi-descriptor transmits the driver writes DT_FEND into the last
descriptor and DT_FSTART into the first. The DMA engine begins processing
when it observes DT_FSTART. Move the dma_wmb() barrier so it executes
immediately after DT_FEND and immediately before writing DT_FSTART
(and before DT_FSINGLE in the single-descriptor case). This guarantees
that all prior CPU writes to the descriptor memory are visible to the
device before DT_FSTART is seen.

This avoids a situation where compiler/CPU reordering could publish
DT_FSTART ahead of DT_FEND or other descriptor fields, allowing the DMA to
start on a partially initialised chain and causing corrupted transmissions
or TX timeouts. Such a failure was observed on RZ/G2L with an RT kernel as
transmit queue timeouts and device resets.

Fixes: 2f45d1902acf ("ravb: minimize TX data copying")
Cc: stable@vger.kernel.org
Co-developed-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- Reflowed the code and updated the comment to clarify the ordering
  requirements.
- Updated commit message.
- Split up adding memory barrier change before ringing doorbell
  into a separate patch.
---
 drivers/net/ethernet/renesas/ravb_main.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index a200e205825a..0e40001f64b4 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2211,13 +2211,25 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 
 		skb_tx_timestamp(skb);
 	}
-	/* Descriptor type must be set after all the above writes */
-	dma_wmb();
+
 	if (num_tx_desc > 1) {
 		desc->die_dt = DT_FEND;
 		desc--;
+		/* When using multi-descriptors, DT_FEND needs to get written
+		 * before DT_FSTART, but the compiler may reorder the memory
+		 * writes in an attempt to optimize the code.
+		 * Use a dma_wmb() barrier to make sure DT_FEND and DT_FSTART
+		 * are written exactly in the order shown in the code.
+		 * This is particularly important for cases where the DMA engine
+		 * is already running when we are running this code. If the DMA
+		 * sees DT_FSTART without the corresponding DT_FEND it will enter
+		 * an error condition.
+		 */
+		dma_wmb();
 		desc->die_dt = DT_FSTART;
 	} else {
+		/* Descriptor type must be set after all the above writes */
+		dma_wmb();
 		desc->die_dt = DT_FSINGLE;
 	}
 	ravb_modify(ndev, TCCR, TCCR_TSRQ0 << q, TCCR_TSRQ0 << q);
-- 
2.43.0

Re: [PATCH v2 3/4] net: ravb: Enforce descriptor type ordering

[Niklas Söderlund]

Hi Lad,

Thanks for reworking this and making it very clear what's going on.

On 2025-10-17 16:18:29 +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Ensure the TX descriptor type fields are published in a safe order so the
> DMA engine never begins processing a descriptor chain before all descriptor
> fields are fully initialised.
> 
> For multi-descriptor transmits the driver writes DT_FEND into the last
> descriptor and DT_FSTART into the first. The DMA engine begins processing
> when it observes DT_FSTART. Move the dma_wmb() barrier so it executes
> immediately after DT_FEND and immediately before writing DT_FSTART
> (and before DT_FSINGLE in the single-descriptor case). This guarantees
> that all prior CPU writes to the descriptor memory are visible to the
> device before DT_FSTART is seen.
> 
> This avoids a situation where compiler/CPU reordering could publish
> DT_FSTART ahead of DT_FEND or other descriptor fields, allowing the DMA to
> start on a partially initialised chain and causing corrupted transmissions
> or TX timeouts. Such a failure was observed on RZ/G2L with an RT kernel as
> transmit queue timeouts and device resets.
> 
> Fixes: 2f45d1902acf ("ravb: minimize TX data copying")
> Cc: stable@vger.kernel.org
> Co-developed-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> v1->v2:
> - Reflowed the code and updated the comment to clarify the ordering
>   requirements.
> - Updated commit message.
> - Split up adding memory barrier change before ringing doorbell
>   into a separate patch.
> ---
>  drivers/net/ethernet/renesas/ravb_main.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index a200e205825a..0e40001f64b4 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -2211,13 +2211,25 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>  
>  		skb_tx_timestamp(skb);
>  	}
> -	/* Descriptor type must be set after all the above writes */
> -	dma_wmb();
> +
>  	if (num_tx_desc > 1) {
>  		desc->die_dt = DT_FEND;
>  		desc--;
> +		/* When using multi-descriptors, DT_FEND needs to get written
> +		 * before DT_FSTART, but the compiler may reorder the memory
> +		 * writes in an attempt to optimize the code.
> +		 * Use a dma_wmb() barrier to make sure DT_FEND and DT_FSTART
> +		 * are written exactly in the order shown in the code.
> +		 * This is particularly important for cases where the DMA engine
> +		 * is already running when we are running this code. If the DMA
> +		 * sees DT_FSTART without the corresponding DT_FEND it will enter
> +		 * an error condition.
> +		 */
> +		dma_wmb();
>  		desc->die_dt = DT_FSTART;
>  	} else {
> +		/* Descriptor type must be set after all the above writes */
> +		dma_wmb();
>  		desc->die_dt = DT_FSINGLE;
>  	}
>  	ravb_modify(ndev, TCCR, TCCR_TSRQ0 << q, TCCR_TSRQ0 << q);
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

[PATCH v2 4/4] net: ravb: Ensure memory write completes before ringing TX doorbell

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add a final dma_wmb() barrier before triggering the transmit request
(TCCR_TSRQ) to ensure all descriptor and buffer writes are visible to
the DMA engine.

According to the hardware manual, a read-back operation is required
before writing to the doorbell register to guarantee completion of
previous writes. Instead of performing a dummy read, a dma_wmb() is
used to both enforce the same ordering semantics on the CPU side and
also to ensure completion of writes.

Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
Cc: stable@vger.kernel.org
Co-developed-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- New patch added to separate out the memory barrier change
  before ringing the doorbell.
---
 drivers/net/ethernet/renesas/ravb_main.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 0e40001f64b4..c3fc15f9ec85 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2232,6 +2232,14 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 		dma_wmb();
 		desc->die_dt = DT_FSINGLE;
 	}
+
+	/* Before ringing the doorbell we need to make sure that the latest
+	 * writes have been committed to memory, otherwise it could delay
+	 * things until the doorbell is rang again.
+	 * This is in replacement of the read operation mentioned in the HW
+	 * manuals.
+	 */
+	dma_wmb();
 	ravb_modify(ndev, TCCR, TCCR_TSRQ0 << q, TCCR_TSRQ0 << q);
 
 	priv->cur_tx[q] += num_tx_desc;
-- 
2.43.0

Re: [PATCH v2 4/4] net: ravb: Ensure memory write completes before ringing TX doorbell

[Niklas Söderlund]

Hello Lad,

Thanks for your work.

On 2025-10-17 16:18:30 +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Add a final dma_wmb() barrier before triggering the transmit request
> (TCCR_TSRQ) to ensure all descriptor and buffer writes are visible to
> the DMA engine.
> 
> According to the hardware manual, a read-back operation is required
> before writing to the doorbell register to guarantee completion of
> previous writes. Instead of performing a dummy read, a dma_wmb() is
> used to both enforce the same ordering semantics on the CPU side and
> also to ensure completion of writes.
> 
> Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
> Cc: stable@vger.kernel.org
> Co-developed-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> v1->v2:
> - New patch added to separate out the memory barrier change
>   before ringing the doorbell.
> ---
>  drivers/net/ethernet/renesas/ravb_main.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 0e40001f64b4..c3fc15f9ec85 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -2232,6 +2232,14 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>  		dma_wmb();
>  		desc->die_dt = DT_FSINGLE;
>  	}
> +
> +	/* Before ringing the doorbell we need to make sure that the latest
> +	 * writes have been committed to memory, otherwise it could delay
> +	 * things until the doorbell is rang again.
> +	 * This is in replacement of the read operation mentioned in the HW

nit: I would spell out hardware here, if you do a v3.

> +	 * manuals.
> +	 */
> +	dma_wmb();
>  	ravb_modify(ndev, TCCR, TCCR_TSRQ0 << q, TCCR_TSRQ0 << q);
>  
>  	priv->cur_tx[q] += num_tx_desc;
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund