[PATCH RFT 0/2] pci: clean up cpu_addr_fixup() for visconti

[PATCH RFT 0/2] pci: clean up cpu_addr_fixup() for visconti

[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>

[PATCH RFT 1/2] arm64: dts: toshiba: Update SoC and PCIe ranges to reflect hardware behavior

[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

RE: [PATCH RFT 1/2] arm64: dts: toshiba: Update SoC and PCIe ranges to reflect hardware behavior

[nobuhiro1.iwamatsu]

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(岩松 信洋 □ＤＩＴＣ○ＣＰＴ)
> <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

Re: [PATCH RFT 1/2] arm64: dts: toshiba: Update SoC and PCIe ranges to reflect hardware behavior

[Frank Li]

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(岩松 信洋 □ＤＩＴＣ○ＣＰＴ)
> > <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

RE: [PATCH RFT 1/2] arm64: dts: toshiba: Update SoC and PCIe ranges to reflect hardware behavior

[nobuhiro1.iwamatsu]

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

[PATCH RFT 2/2] PCI: dwc: visconti: Remove cpu_addr_fix() after DTS fix ranges

[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