| 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
| ||
|
Message-ID: <CA+V-a8vRXN+2CDQu=FkN_teTDLywzGPn_=8obvKC+3tmwYo4hA@mail.gmail.com>
Date: Wed, 15 Oct 2025 18:05:34 +0100
From: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To: Niklas Söderlund <niklas.soderlund@...natech.se>
Cc: Paul Barker <paul@...rker.dev>, Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Sergei Shtylyov <sergei.shtylyov@...entembedded.com>, netdev@...r.kernel.org,
linux-renesas-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
Biju Das <biju.das.jz@...renesas.com>,
Fabrizio Castro <fabrizio.castro.jz@...esas.com>,
Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>, stable@...r.kernel.org
Subject: Re: [PATCH 1/3] net: ravb: Make DBAT entry count configurable per-SoC
Hi Niklas,
Thank you for the review.
On Wed, Oct 15, 2025 at 4:36 PM Niklas Söderlund
<niklas.soderlund@...natech.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@...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@...r.kernel.org
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...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?
Cheers,
Prabhakar
> Without, but preferably with, this added.
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@...natech.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
Powered by blists - more mailing lists