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

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

On 2025-10-15 18:05:34 +0100, Lad, Prabhakar wrote:
> Hi Niklas,
> 
> Thank you for the review.
> 
> On Wed, Oct 15, 2025 at 4:36 PM Niklas Söderlund
> <niklas.soderlund@ragnatech.se> wrote:
> >
> > 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.
> >  */
> >
> I agree, let's take this separately into a different patch as it will
> make things easier to backport. What do you think?

Works for me.

> 
> Cheers,
> Prabhakar
> 
> > 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

-- 
Kind Regards,
Niklas Söderlund