linux-kernel - Re: [PATCH v5 2/6] PCI: rzg3s-host: Add Renesas RZ/G3S SoC host driver

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: <CAMuHMdXx=5YQSmSsw1+3otw9S_Hf+Tv+N1Y1iHiU0OOTyz4bjw@mail.gmail.com>
Date: Thu, 23 Oct 2025 16:21:11 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Claudiu Beznea <claudiu.beznea@...on.dev>
Cc: lpieralisi@...nel.org, kwilczynski@...nel.org, mani@...nel.org, 
	robh@...nel.org, bhelgaas@...gle.com, krzk+dt@...nel.org, conor+dt@...nel.org, 
	magnus.damm@...il.com, p.zabel@...gutronix.de, linux-pci@...r.kernel.org, 
	linux-renesas-soc@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, 
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>, 
	Wolfram Sang <wsa+renesas@...g-engineering.com>
Subject: Re: [PATCH v5 2/6] PCI: rzg3s-host: Add Renesas RZ/G3S SoC host driver

Hi Claudiu,

On Thu, 23 Oct 2025 at 16:13, Claudiu Beznea <claudiu.beznea@...on.dev> wrote:
> On 10/23/25 14:02, Geert Uytterhoeven wrote:
> > On Thu, 23 Oct 2025 at 12:54, Claudiu Beznea <claudiu.beznea@...on.dev> wrote:
> >> On 10/23/25 11:00, Geert Uytterhoeven wrote:
> >>> On Tue, 7 Oct 2025 at 15:37, Claudiu <claudiu.beznea@...on.dev> wrote:
> >>>> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> >>>>
> >>>> The Renesas RZ/G3S features a PCIe IP that complies with the PCI Express
> >>>> Base Specification 4.0 and supports speeds of up to 5 GT/s. It functions
> >>>> only as a root complex, with a single-lane (x1) configuration. The
> >>>> controller includes Type 1 configuration registers, as well as IP
> >>>> specific registers (called AXI registers) required for various adjustments.
> >>>>
> >>>> Hardware manual can be downloaded from the address in the "Link" section.
> >>>> The following steps should be followed to access the manual:
> >>>> 1/ Click the "User Manual" button
> >>>> 2/ Click "Confirm"; this will start downloading an archive
> >>>> 3/ Open the downloaded archive
> >>>> 4/ Navigate to r01uh1014ej*-rzg3s-users-manual-hardware -> Deliverables
> >>>> 5/ Open the file r01uh1014ej*-rzg3s.pdf
> >>>>
> >>>> Link: https://www.renesas.com/en/products/rz-g3s?queryID=695cc067c2d89e3f271d43656ede4d12
> >>>> Tested-by: Wolfram Sang <wsa+renesas@...g-engineering.com>
> >>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> >>>
> >>> Thanks for your patch!
> >>>
> >>>> --- /dev/null
> >>>> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> >>>
> >>>> +static void rzg3s_pcie_irq_compose_msi_msg(struct irq_data *data,
> >>>> +                                          struct msi_msg *msg)
> >>>> +{
> >>>> +       struct rzg3s_pcie_msi *msi = irq_data_get_irq_chip_data(data);
> >>>> +       struct rzg3s_pcie_host *host = rzg3s_msi_to_host(msi);
> >>>> +       u32 drop_mask = RZG3S_PCI_MSIRCVWADRL_ENA |
> >>>> +                       RZG3S_PCI_MSIRCVWADRL_MSG_DATA_ENA;
> >>>
> >>> This should include bit 2 (which is hardwired to zero (for now)),
> >>> so I think you better add
> >>>
> >>>     #define RZG3S_PCI_MSIRCVWADRL_ADDR  GENMASK(31, 3)
> >>>
> >>>> +       u32 lo, hi;
> >>>> +
> >>>> +       /*
> >>>> +        * Enable and msg data enable bits are part of the address lo. Drop
> >>>> +        * them.
> >>>> +        */
> >>>> +       lo = readl_relaxed(host->axi + RZG3S_PCI_MSIRCVWADRL) & ~drop_mask;
> >>>
> >>> ... and use FIELD_GET() with the new definition here.
> >>
> >> Bits 31..3 of RZG3S_PCI_MSIRCVWADRL contains only bits 31..3 of the MSI
> >> receive window address low, AFAIU. Using FIELD_GET() for bits 31..3 on the
> >> value read from RZG3S_PCI_MSIRCVWADRL and passing this value to
> >> msg->address_lo will lead to an NVMe device not working.
> >
> > Oops, yes you are right, I went a bit too far with the FIELD_GET()
> > suggestion. But replacing drop_mask by RZG3S_PCI_MSIRCVWADRL_ADDR
> > would still be worthwhile, IMHO.
>
> OK, you mean updating it like:
>
> +#define RZG3S_PCI_MSIRCVWADRL_ADDR  GENMASK(31, 3)
>
> // ...
>
>
> -    lo = readl_relaxed(host->axi + RZG3S_PCI_MSIRCVWADRL) & ~drop_mask;
> +    lo = readl_relaxed(host->axi + RZG3S_PCI_MSIRCVWADRL) &
>           RZG3S_PCI_MSIRCVWADRL_ADDR;

Exactly.
Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds