linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFT 0/2] pci: clean up cpu_addr_fixup() for visconti
@ 2025-06-13 21:33 Frank Li
  2025-06-13 21:33 ` [PATCH RFT 1/2] arm64: dts: toshiba: Update SoC and PCIe ranges to reflect hardware behavior Frank Li
  2025-06-13 21:33 ` [PATCH RFT 2/2] PCI: dwc: visconti: Remove cpu_addr_fix() after DTS fix ranges Frank Li
  0 siblings, 2 replies; 6+ messages in thread
From: Frank Li @ 2025-06-13 21:33 UTC (permalink / raw)
  To: Nobuhiro Iwamatsu, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Bjorn Helgaas
  Cc: linux-arm-kernel, devicetree, linux-kernel, linux-pci, Frank Li

Since
7db02f725df44 PCI: dwc: Use devicetree 'reg[config]' to derive CPU -> ATU addr offset

dwc common code have handled address translate by bus fabric.

1. Correct dts
2. remove cpu_addr_fixup()

dts change need be merge firstly.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Frank Li (2):
      arm64: dts: toshiba: Update SoC and PCIe ranges to reflect hardware behavior
      PCI: dwc: visconti: Remove cpu_addr_fix() after DTS fix ranges

 arch/arm64/boot/dts/toshiba/tmpv7708.dtsi  | 16 ++++++++++++----
 drivers/pci/controller/dwc/pcie-visconti.c | 13 -------------
 2 files changed, 12 insertions(+), 17 deletions(-)
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20250613-tmpv-f1cc6e463343

Best regards,
---
Frank Li <Frank.Li@nxp.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH RFT 1/2] arm64: dts: toshiba: Update SoC and PCIe ranges to reflect hardware behavior
  2025-06-13 21:33 [PATCH RFT 0/2] pci: clean up cpu_addr_fixup() for visconti Frank Li
@ 2025-06-13 21:33 ` Frank Li
  2025-07-28  5:38   ` nobuhiro1.iwamatsu
  2025-06-13 21:33 ` [PATCH RFT 2/2] PCI: dwc: visconti: Remove cpu_addr_fix() after DTS fix ranges Frank Li
  1 sibling, 1 reply; 6+ messages in thread
From: Frank Li @ 2025-06-13 21:33 UTC (permalink / raw)
  To: Nobuhiro Iwamatsu, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Bjorn Helgaas
  Cc: linux-arm-kernel, devicetree, linux-kernel, linux-pci, Frank Li

tmpv7708 trim address bit[31:30] in tmpv7708 before passing to the PCIe
controller. So add a 'ranges' entry under the parent bus 'soc' to map
address 0x0 to 0x40000000.

Update the PCIe node's 'config' and 'ranges' properties to use the real
upstream bus address.

Ensure there is no functional impact on the final address translation
result.

Prepare for the removal of the driver’s cpu_addr_fixup().

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 arch/arm64/boot/dts/toshiba/tmpv7708.dtsi | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi b/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
index 39806f0ae5133..2a18aa93d4723 100644
--- a/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
+++ b/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
@@ -147,7 +147,15 @@ soc {
 		#size-cells = <2>;
 		compatible = "simple-bus";
 		interrupt-parent = <&gic>;
-		ranges;
+		ranges = /* register 1:1 map */
+			 <0x0 0x24000000 0x0 0x24000000 0x0 0x10000000>,
+			 /*
+			  * bus fabric mask address bit 30 and 31 to 0
+			  * before send to PCIe controller.
+			  *
+			  * PCIe map address 0 to cpu's 0x40000000
+			  */
+			 <0x0 0x00000000 0x0 0x40000000 0x0 0x40000000>;
 
 		gic: interrupt-controller@24001000 {
 			compatible = "arm,gic-400";
@@ -481,7 +489,7 @@ pwm: pwm@241c0000 {
 		pcie: pcie@28400000 {
 			compatible = "toshiba,visconti-pcie";
 			reg = <0x0 0x28400000 0x0 0x00400000>,
-			      <0x0 0x70000000 0x0 0x10000000>,
+			      <0x0 0x30000000 0x0 0x10000000>,
 			      <0x0 0x28050000 0x0 0x00010000>,
 			      <0x0 0x24200000 0x0 0x00002000>,
 			      <0x0 0x24162000 0x0 0x00001000>;
@@ -494,8 +502,8 @@ pcie: pcie@28400000 {
 			#address-cells = <3>;
 			#size-cells = <2>;
 			#interrupt-cells = <1>;
-			ranges = <0x81000000 0 0x40000000 0 0x40000000 0 0x00010000
-				  0x82000000 0 0x50000000 0 0x50000000 0 0x20000000>;
+			ranges = <0x81000000 0 0x00000000 0 0x00000000 0 0x00010000
+				  0x82000000 0 0x10000000 0 0x10000000 0 0x20000000>;
 			interrupts = <GIC_SPI 211 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 215 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "msi", "intr";

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH RFT 2/2] PCI: dwc: visconti: Remove cpu_addr_fix() after DTS fix ranges
  2025-06-13 21:33 [PATCH RFT 0/2] pci: clean up cpu_addr_fixup() for visconti Frank Li
  2025-06-13 21:33 ` [PATCH RFT 1/2] arm64: dts: toshiba: Update SoC and PCIe ranges to reflect hardware behavior Frank Li
@ 2025-06-13 21:33 ` Frank Li
  1 sibling, 0 replies; 6+ messages in thread
From: Frank Li @ 2025-06-13 21:33 UTC (permalink / raw)
  To: Nobuhiro Iwamatsu, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Bjorn Helgaas
  Cc: linux-arm-kernel, devicetree, linux-kernel, linux-pci, Frank Li

Remove cpu_addr_fix() since it is no longer needed. The PCIe ranges
property has been corrected in the DTS, and the DesignWare common code now
handles address translation properly without requiring this workaround.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/dwc/pcie-visconti.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-visconti.c b/drivers/pci/controller/dwc/pcie-visconti.c
index cdeac6177143c..9c3386c965948 100644
--- a/drivers/pci/controller/dwc/pcie-visconti.c
+++ b/drivers/pci/controller/dwc/pcie-visconti.c
@@ -171,20 +171,7 @@ static void visconti_pcie_stop_link(struct dw_pcie *pci)
 	visconti_mpu_writel(pcie, val | MPU_MP_EN_DISABLE, PCIE_MPU_REG_MP_EN);
 }
 
-/*
- * In this SoC specification, the CPU bus outputs the offset value from
- * 0x40000000 to the PCIe bus, so 0x40000000 is subtracted from the CPU
- * bus address. This 0x40000000 is also based on io_base from DT.
- */
-static u64 visconti_pcie_cpu_addr_fixup(struct dw_pcie *pci, u64 cpu_addr)
-{
-	struct dw_pcie_rp *pp = &pci->pp;
-
-	return cpu_addr & ~pp->io_base;
-}
-
 static const struct dw_pcie_ops dw_pcie_ops = {
-	.cpu_addr_fixup = visconti_pcie_cpu_addr_fixup,
 	.link_up = visconti_pcie_link_up,
 	.start_link = visconti_pcie_start_link,
 	.stop_link = visconti_pcie_stop_link,

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH RFT 1/2] arm64: dts: toshiba: Update SoC and PCIe ranges to reflect hardware behavior
  2025-06-13 21:33 ` [PATCH RFT 1/2] arm64: dts: toshiba: Update SoC and PCIe ranges to reflect hardware behavior Frank Li
@ 2025-07-28  5:38   ` nobuhiro1.iwamatsu
  2025-07-28 14:26     ` Frank Li
  0 siblings, 1 reply; 6+ messages in thread
From: nobuhiro1.iwamatsu @ 2025-07-28  5:38 UTC (permalink / raw)
  To: Frank.Li, robh, krzk+dt, conor+dt, lpieralisi, kwilczynski, mani,
	bhelgaas
  Cc: linux-arm-kernel, devicetree, linux-kernel, linux-pci

Hi Frank,

Thanks for your patch, and sorry reply was too late.

> -----Original Message-----
> From: Frank Li <Frank.Li@nxp.com>
> Sent: Saturday, June 14, 2025 6:33 AM
> To: iwamatsu nobuhiro(岩松 信洋 □DITC○CPT)
> <nobuhiro1.iwamatsu@toshiba.co.jp>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Lorenzo Pieralisi <lpieralisi@kernel.org>; Krzysztof
> Wilczyński <kwilczynski@kernel.org>; Manivannan Sadhasivam
> <mani@kernel.org>; Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; Frank Li
> <Frank.Li@nxp.com>
> Subject: [PATCH RFT 1/2] arm64: dts: toshiba: Update SoC and PCIe ranges to
> reflect hardware behavior
> 
> tmpv7708 trim address bit[31:30] in tmpv7708 before passing to the PCIe
> controller. So add a 'ranges' entry under the parent bus 'soc' to map address 0x0
> to 0x40000000.
> 
> Update the PCIe node's 'config' and 'ranges' properties to use the real upstream
> bus address.
> 
> Ensure there is no functional impact on the final address translation result.
> 
> Prepare for the removal of the driver’s cpu_addr_fixup().
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  arch/arm64/boot/dts/toshiba/tmpv7708.dtsi | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
> b/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
> index 39806f0ae5133..2a18aa93d4723 100644
> --- a/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
> +++ b/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
> @@ -147,7 +147,15 @@ soc {
>  		#size-cells = <2>;
>  		compatible = "simple-bus";
>  		interrupt-parent = <&gic>;
> -		ranges;
> +		ranges = /* register 1:1 map */
> +			 <0x0 0x24000000 0x0 0x24000000 0x0 0x10000000>,
> +			 /*
> +			  * bus fabric mask address bit 30 and 31 to 0
> +			  * before send to PCIe controller.
> +			  *
> +			  * PCIe map address 0 to cpu's 0x40000000
> +			  */
> +			 <0x0 0x00000000 0x0 0x40000000 0x0 0x40000000>;
> 
>  		gic: interrupt-controller@24001000 {
>  			compatible = "arm,gic-400";
> @@ -481,7 +489,7 @@ pwm: pwm@241c0000 {
>  		pcie: pcie@28400000 {
>  			compatible = "toshiba,visconti-pcie";
>  			reg = <0x0 0x28400000 0x0 0x00400000>,
> -			      <0x0 0x70000000 0x0 0x10000000>,
> +			      <0x0 0x30000000 0x0 0x10000000>,

If my understanding is correct, this setting conflicts with the address space this patch changed
ranges above. Therefore, it does not work.

0x24000000 + 0x10000000 > 0x30000000

By reducing 0x10000000 to 0xc000000, it will fit within the 0x30000000 range.  
And by adding the following to the driver:
```  
pci->use_parent_dt_ranges = true;  
```

the PCIe will work, but this setting prevents access to devices located after 0x30000000.
Is there any other DT method to avoid this?

Best regards,
  Nobuhiro

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFT 1/2] arm64: dts: toshiba: Update SoC and PCIe ranges to reflect hardware behavior
  2025-07-28  5:38   ` nobuhiro1.iwamatsu
@ 2025-07-28 14:26     ` Frank Li
  2025-07-30  0:13       ` nobuhiro1.iwamatsu
  0 siblings, 1 reply; 6+ messages in thread
From: Frank Li @ 2025-07-28 14:26 UTC (permalink / raw)
  To: nobuhiro1.iwamatsu
  Cc: robh, krzk+dt, conor+dt, lpieralisi, kwilczynski, mani, bhelgaas,
	linux-arm-kernel, devicetree, linux-kernel, linux-pci

On Mon, Jul 28, 2025 at 05:38:25AM +0000, nobuhiro1.iwamatsu@toshiba.co.jp wrote:
> Hi Frank,
>
> Thanks for your patch, and sorry reply was too late.
>
> > -----Original Message-----
> > From: Frank Li <Frank.Li@nxp.com>
> > Sent: Saturday, June 14, 2025 6:33 AM
> > To: iwamatsu nobuhiro(岩松 信洋 □DITC○CPT)
> > <nobuhiro1.iwamatsu@toshiba.co.jp>; Rob Herring <robh@kernel.org>;
> > Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> > <conor+dt@kernel.org>; Lorenzo Pieralisi <lpieralisi@kernel.org>; Krzysztof
> > Wilczyński <kwilczynski@kernel.org>; Manivannan Sadhasivam
> > <mani@kernel.org>; Bjorn Helgaas <bhelgaas@google.com>
> > Cc: linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
> > linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; Frank Li
> > <Frank.Li@nxp.com>
> > Subject: [PATCH RFT 1/2] arm64: dts: toshiba: Update SoC and PCIe ranges to
> > reflect hardware behavior
> >
> > tmpv7708 trim address bit[31:30] in tmpv7708 before passing to the PCIe
> > controller. So add a 'ranges' entry under the parent bus 'soc' to map address 0x0
> > to 0x40000000.
> >
> > Update the PCIe node's 'config' and 'ranges' properties to use the real upstream
> > bus address.
> >
> > Ensure there is no functional impact on the final address translation result.
> >
> > Prepare for the removal of the driver’s cpu_addr_fixup().
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  arch/arm64/boot/dts/toshiba/tmpv7708.dtsi | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
> > b/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
> > index 39806f0ae5133..2a18aa93d4723 100644
> > --- a/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
> > +++ b/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
> > @@ -147,7 +147,15 @@ soc {
> >  		#size-cells = <2>;
> >  		compatible = "simple-bus";
> >  		interrupt-parent = <&gic>;
> > -		ranges;
> > +		ranges = /* register 1:1 map */
> > +			 <0x0 0x24000000 0x0 0x24000000 0x0 0x10000000>,
> > +			 /*
> > +			  * bus fabric mask address bit 30 and 31 to 0
> > +			  * before send to PCIe controller.
> > +			  *
> > +			  * PCIe map address 0 to cpu's 0x40000000
> > +			  */
> > +			 <0x0 0x00000000 0x0 0x40000000 0x0 0x40000000>;
> >
> >  		gic: interrupt-controller@24001000 {
> >  			compatible = "arm,gic-400";
> > @@ -481,7 +489,7 @@ pwm: pwm@241c0000 {
> >  		pcie: pcie@28400000 {
> >  			compatible = "toshiba,visconti-pcie";
> >  			reg = <0x0 0x28400000 0x0 0x00400000>,
> > -			      <0x0 0x70000000 0x0 0x10000000>,
> > +			      <0x0 0x30000000 0x0 0x10000000>,
>
> If my understanding is correct, this setting conflicts with the address space this patch changed
> ranges above. Therefore, it does not work.
>
> 0x24000000 + 0x10000000 > 0x30000000

You are right.  Address map should only happen at pci fabic.

So we should not touch soc's ranges.

soc {
	...

	pci-bus {
		 ranges = /* register 1:1 map */
                 <0x0 0x24000000 0x0 0x24000000 0x0 0x10000000>,
                 /*
                  * bus fabric mask address bit 30 and 31 to 0
                  * before send to PCIe controller.
                  *
                  * PCIe map address 0 to cpu's 0x40000000
                  */
                 <0x0 0x00000000 0x0 0x40000000 0x0 0x40000000>;


		pci@0x24000000 {
			...
		}
	}
}

Do you need me send v2 for your test?

Frank
>
> By reducing 0x10000000 to 0xc000000, it will fit within the 0x30000000 range.
> And by adding the following to the driver:
> ```
> pci->use_parent_dt_ranges = true;
> ```
>
> the PCIe will work, but this setting prevents access to devices located after 0x30000000.
> Is there any other DT method to avoid this?
>
> Best regards,
>   Nobuhiro

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH RFT 1/2] arm64: dts: toshiba: Update SoC and PCIe ranges to reflect hardware behavior
  2025-07-28 14:26     ` Frank Li
@ 2025-07-30  0:13       ` nobuhiro1.iwamatsu
  0 siblings, 0 replies; 6+ messages in thread
From: nobuhiro1.iwamatsu @ 2025-07-30  0:13 UTC (permalink / raw)
  To: Frank.li
  Cc: robh, krzk+dt, conor+dt, lpieralisi, kwilczynski, mani, bhelgaas,
	linux-arm-kernel, devicetree, linux-kernel, linux-pci

Hi Frank,

> > > diff --git a/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
> > > b/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
> > > index 39806f0ae5133..2a18aa93d4723 100644
> > > --- a/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
> > > +++ b/arch/arm64/boot/dts/toshiba/tmpv7708.dtsi
> > > @@ -147,7 +147,15 @@ soc {
> > >  		#size-cells = <2>;
> > >  		compatible = "simple-bus";
> > >  		interrupt-parent = <&gic>;
> > > -		ranges;
> > > +		ranges = /* register 1:1 map */
> > > +			 <0x0 0x24000000 0x0 0x24000000 0x0 0x10000000>,
> > > +			 /*
> > > +			  * bus fabric mask address bit 30 and 31 to 0
> > > +			  * before send to PCIe controller.
> > > +			  *
> > > +			  * PCIe map address 0 to cpu's 0x40000000
> > > +			  */
> > > +			 <0x0 0x00000000 0x0 0x40000000 0x0 0x40000000>;
> > >
> > >  		gic: interrupt-controller@24001000 {
> > >  			compatible = "arm,gic-400";
> > > @@ -481,7 +489,7 @@ pwm: pwm@241c0000 {
> > >  		pcie: pcie@28400000 {
> > >  			compatible = "toshiba,visconti-pcie";
> > >  			reg = <0x0 0x28400000 0x0 0x00400000>,
> > > -			      <0x0 0x70000000 0x0 0x10000000>,
> > > +			      <0x0 0x30000000 0x0 0x10000000>,
> >
> > If my understanding is correct, this setting conflicts with the
> > address space this patch changed ranges above. Therefore, it does not work.
> >
> > 0x24000000 + 0x10000000 > 0x30000000
> 
> You are right.  Address map should only happen at pci fabic.
> 
> So we should not touch soc's ranges.
> 
> soc {
> 	...
> 
> 	pci-bus {
> 		 ranges = /* register 1:1 map */
>                  <0x0 0x24000000 0x0 0x24000000 0x0 0x10000000>,
>                  /*
>                   * bus fabric mask address bit 30 and 31 to 0
>                   * before send to PCIe controller.
>                   *
>                   * PCIe map address 0 to cpu's 0x40000000
>                   */
>                  <0x0 0x00000000 0x0 0x40000000 0x0 0x40000000>;
> 
> 
> 		pci@0x24000000 {
> 			...
> 		}
> 	}
> }

Does this mean that a pci subbus is defined under soc, right?

> 
> Do you need me send v2 for your test?
> 

I will create a patch, so please review it.
Thank you.

> Frank

Best regards,
  Nobuhiro

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-07-30  2:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-13 21:33 [PATCH RFT 0/2] pci: clean up cpu_addr_fixup() for visconti Frank Li
2025-06-13 21:33 ` [PATCH RFT 1/2] arm64: dts: toshiba: Update SoC and PCIe ranges to reflect hardware behavior Frank Li
2025-07-28  5:38   ` nobuhiro1.iwamatsu
2025-07-28 14:26     ` Frank Li
2025-07-30  0:13       ` nobuhiro1.iwamatsu
2025-06-13 21:33 ` [PATCH RFT 2/2] PCI: dwc: visconti: Remove cpu_addr_fix() after DTS fix ranges Frank Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).