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] [day] [month] [year] [list]
Message-ID: <p4k4gkolfudxhvnrj4kkwpnyphdulga3fjovxqjqhmkhnts37o@y3jxdljb5oeo>
Date: Tue, 21 Oct 2025 07:46:06 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Claudiu Beznea <claudiu.beznea@...on.dev>
Cc: lpieralisi@...nel.org, kwilczynski@...nel.org, robh@...nel.org, 
	bhelgaas@...gle.com, krzk+dt@...nel.org, conor+dt@...nel.org, geert+renesas@...der.be, 
	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 3/6] arm64: dts: renesas: r9a08g045: Add PCIe node

On Mon, Oct 20, 2025 at 09:15:47AM +0300, Claudiu Beznea wrote:
> Hi, Mani,
> 
> On 10/19/25 09:57, Manivannan Sadhasivam wrote:
> > On Tue, Oct 07, 2025 at 04:36:54PM +0300, Claudiu wrote:
> >> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> >>
> >> The RZ/G3S SoC has a variant (R9A08G045S33) which supports PCIe. Add the
> >> PCIe node.
> >>
> >> Tested-by: Wolfram Sang <wsa+renesas@...g-engineering.com>
> >> Reviewed-by: Geert Uytterhoeven <geert+renesas@...der.be>
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> >> ---
> >>
> >> Changes in v5:
> >> - updated the last part of ranges and dma-ranges
> >> - collected tags
> >>
> >> Changes in v4:
> >> - moved the node to r9a08g045.dtsi
> >> - dropped the "s33" from the compatible string
> >> - added port node
> >> - re-ordered properties to have them grouped together
> >>
> >> Changes in v3:
> >> - collected tags
> >> - changed the ranges flags
> >>
> >> Changes in v2:
> >> - updated the dma-ranges to reflect the SoC capability; added a
> >>   comment about it.
> >> - updated clock-names, interrupt names
> >> - dropped legacy-interrupt-controller node
> >> - added interrupt-controller property
> >> - moved renesas,sysc at the end of the node to comply with
> >>   DT coding style
> >>
> >>  arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 66 ++++++++++++++++++++++
> >>  1 file changed, 66 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
> >> index 16e6ac614417..00b43377877e 100644
> >> --- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
> >> +++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
> >> @@ -717,6 +717,72 @@ eth1: ethernet@...40000 {
> >>  			status = "disabled";
> >>  		};
> >>  
> >> +		pcie: pcie@...40000 {
> >> +			compatible = "renesas,r9a08g045-pcie";
> >> +			reg = <0 0x11e40000 0 0x10000>;
> >> +			ranges = <0x02000000 0 0x30000000 0 0x30000000 0 0x08000000>;
> >> +			/* Map all possible DRAM ranges (4 GB). */
> >> +			dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 1 0x00000000>;
> >> +			bus-range = <0x0 0xff>;
> >> +			interrupts = <GIC_SPI 395 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 396 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 397 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 398 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 399 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 401 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 402 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 403 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 404 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 405 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 407 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 408 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 409 IRQ_TYPE_LEVEL_HIGH>,
> >> +				     <GIC_SPI 410 IRQ_TYPE_LEVEL_HIGH>;
> >> +			interrupt-names = "serr", "serr_cor", "serr_nonfatal",
> >> +					  "serr_fatal", "axi_err", "inta",
> >> +					  "intb", "intc", "intd", "msi",
> >> +					  "link_bandwidth", "pm_pme", "dma",
> >> +					  "pcie_evt", "msg", "all";
> >> +			#interrupt-cells = <1>;
> >> +			interrupt-controller;
> >> +			interrupt-map-mask = <0 0 0 7>;
> >> +			interrupt-map = <0 0 0 1 &pcie 0 0 0 0>, /* INTA */
> >> +					<0 0 0 2 &pcie 0 0 0 1>, /* INTB */
> >> +					<0 0 0 3 &pcie 0 0 0 2>, /* INTC */
> >> +					<0 0 0 4 &pcie 0 0 0 3>; /* INTD */
> > 
> > Why can't you describe the SPI interrupt for INTx in 'interrupt-map' itself?
> 
> Because the INTx need to be controlled at PCIe controller level, too.
> Driver implements struct irq_chip::{irq_ack, irq_mask, irq_unmask} for
> these interrupts. Describing them like:
> 
> interrupt-map = <0 0 0 1 &gic GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH>,
>                 <0 0 0 2 &gic GIC_SPI 401 IRQ_TYPE_LEVEL_HIGH>,
>                 <0 0 0 3 &gic GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH>,
>                 <0 0 0 4 &gic GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH>;
> 
> wouldn't allow for this, AFAICT.
> 

Ah, so these interrupts are masked at the PCI controller level... Fine then.

> > 
> >> +			clocks = <&cpg CPG_MOD R9A08G045_PCI_ACLK>,
> >> +				 <&cpg CPG_MOD R9A08G045_PCI_CLKL1PM>;
> >> +			clock-names = "aclk", "pm";
> >> +			resets = <&cpg R9A08G045_PCI_ARESETN>,
> >> +				 <&cpg R9A08G045_PCI_RST_B>,
> >> +				 <&cpg R9A08G045_PCI_RST_GP_B>,
> >> +				 <&cpg R9A08G045_PCI_RST_PS_B>,
> >> +				 <&cpg R9A08G045_PCI_RST_RSM_B>,
> >> +				 <&cpg R9A08G045_PCI_RST_CFG_B>,
> >> +				 <&cpg R9A08G045_PCI_RST_LOAD_B>;
> >> +			reset-names = "aresetn", "rst_b", "rst_gp_b", "rst_ps_b",
> >> +				      "rst_rsm_b", "rst_cfg_b", "rst_load_b";
> >> +			power-domains = <&cpg>;
> >> +			device_type = "pci";
> >> +			#address-cells = <3>;
> >> +			#size-cells = <2>;
> >> +			max-link-speed = <2>;
> > 
> > 'max-link-speed' is used to limit the link speed. Why do you need to limit the
> > link speed all the time for this controller?
> 
> I though it is to describe the max link speed supported by controller. This
> controller max link speed is 5GT/s. Should I drop this property?
> 

Yes. The driver should be able to query the Link Capabilities register and get
the max supported link speed of the hardware. This property should only be used
when the firmware decides to override the hardware supported max link speed.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ