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

["Niklas Söderlund" <niklas.soderlund@ragnatech.se>]

Hi Prabhakar,

Thanks for your work.

On 2025-10-15 16:00:24 +0100, Prabhakar wrote:
> 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>

I have not verified with documentation the setting of 2 for 
gbeth_hw_info. But the change itself is good.

> ---
>  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;

I have been wondering for some time if we shall not start to document 
these fields as they always take so much time to get back to what each 
of them represent. How do you feel about starting a header?

/**
 * dbat_entry_num: Describe me here.
 */

Without, but preferably with, this added.

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

>  	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
> 

-- 
Kind Regards,
Niklas Söderlund