[PATCH RFC NOT TESTED 0/2] PCI: artpec6: Try to clean up artpec6_pcie_cpu_addr_fixup()

[PATCH RFC NOT TESTED 0/2] PCI: artpec6: Try to clean up artpec6_pcie_cpu_addr_fixup()

[Frank Li]

This patches basic on
https://lore.kernel.org/imx/20250128-pci_fixup_addr-v9-0-3c4bb506f665@nxp.com/

I have not hardware to test and there are not axis,artpec7-pcie in kernel
tree.

Look for driver owner, who help test this and start move forward to remove
cpu_addr_fixup() work.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Frank Li (2):
      ARM: dts: artpec6: Move PCIe nodes under bus@c0000000
      PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()

 arch/arm/boot/dts/axis/artpec6.dtsi       | 92 +++++++++++++++++--------------
 drivers/pci/controller/dwc/pcie-artpec6.c | 20 +------
 2 files changed, 52 insertions(+), 60 deletions(-)
---
base-commit: 1552be4855dacca5ea39b15b1ef0b96c91dbea0d
change-id: 20250304-axis-6d12970976b4

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

[PATCH RFC NOT TESTED 1/2] ARM: dts: artpec6: Move PCIe nodes under bus@c0000000

[Frank Li]

Move PCIe nodes under bus@c0000000 to correctly reflect hardware behavior.
Use ranges in bus@c0000000 to indicate address translation, as the bus
fabric trims CPU address 0xc000_0000..0xdfff_ffff to
0x0000_0000..0x1fff_ffff.

Set 'config' and 'addr_space' reg values to 0.
Change parent bus address of downstream I/O and non-prefetchable memory to
0.

Ensure 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/arm/boot/dts/axis/artpec6.dtsi | 92 ++++++++++++++++++++-----------------
 1 file changed, 50 insertions(+), 42 deletions(-)

diff --git a/arch/arm/boot/dts/axis/artpec6.dtsi b/arch/arm/boot/dts/axis/artpec6.dtsi
index 037157e6c5ee3..399e87f72865f 100644
--- a/arch/arm/boot/dts/axis/artpec6.dtsi
+++ b/arch/arm/boot/dts/axis/artpec6.dtsi
@@ -155,49 +155,57 @@ pmu {
 		interrupt-affinity = <&cpu0>, <&cpu1>;
 	};
 
-	/*
-	 * Both pci nodes cannot be enabled at the same time,
-	 * leave the unwanted node as disabled.
-	 */
-	pcie: pcie@f8050000 {
-		compatible = "axis,artpec6-pcie", "snps,dw-pcie";
-		reg = <0xf8050000 0x2000
-		       0xf8040000 0x1000
-		       0xc0000000 0x2000>;
-		reg-names = "dbi", "phy", "config";
-		#address-cells = <3>;
-		#size-cells = <2>;
-		device_type = "pci";
-			  /* downstream I/O */
-		ranges = <0x81000000 0 0 0xc0002000 0 0x00010000
-			  /* non-prefetchable memory */
-			  0x82000000 0 0xc0012000 0xc0012000 0 0x1ffee000>;
-		num-lanes = <2>;
-		bus-range = <0x00 0xff>;
-		interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-names = "msi";
-		#interrupt-cells = <1>;
-		interrupt-map-mask = <0 0 0 0x7>;
-		interrupt-map = <0 0 0 1 &intc GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>,
-				<0 0 0 2 &intc GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>,
-				<0 0 0 3 &intc GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>,
-				<0 0 0 4 &intc GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
-		axis,syscon-pcie = <&syscon>;
-		status = "disabled";
-	};
+	bus@c0000000 {
+		compatible = "simple-bus";
+		ranges = <0x0 0xc0000000 0x20000000>,
+			 <0xf8000000 0xf8000000 0x80000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
 
-	pcie_ep: pcie_ep@f8050000 {
-		compatible = "axis,artpec6-pcie-ep", "snps,dw-pcie";
-		reg = <0xf8050000 0x2000
-		       0xf8051000 0x2000
-		       0xf8040000 0x1000
-		       0xc0000000 0x20000000>;
-		reg-names = "dbi", "dbi2", "phy", "addr_space";
-		num-ib-windows = <6>;
-		num-ob-windows = <2>;
-		num-lanes = <2>;
-		axis,syscon-pcie = <&syscon>;
-		status = "disabled";
+		/*
+		 * Both pci nodes cannot be enabled at the same time,
+		 * leave the unwanted node as disabled.
+		 */
+		pcie: pcie@f8050000 {
+			compatible = "axis,artpec6-pcie", "snps,dw-pcie";
+			reg = <0xf8050000 0x2000
+			       0xf8040000 0x1000
+			       0x00000000 0x2000>;
+			reg-names = "dbi", "phy", "config";
+			#address-cells = <3>;
+			#size-cells = <2>;
+			device_type = "pci";
+				  /* downstream I/O */
+			ranges = <0x81000000 0 0 0x00002000 0 0x00010000
+				  /* non-prefetchable memory */
+				  0x82000000 0 0xc0012000 0x00012000 0 0x1ffee000>;
+			num-lanes = <2>;
+			bus-range = <0x00 0xff>;
+			interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi";
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 0x7>;
+			interrupt-map = <0 0 0 1 &intc GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &intc GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &intc GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &intc GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
+			axis,syscon-pcie = <&syscon>;
+			status = "disabled";
+		};
+
+		pcie_ep: pcie_ep@f8050000 {
+			compatible = "axis,artpec6-pcie-ep", "snps,dw-pcie";
+			reg = <0xf8050000 0x2000
+			       0xf8051000 0x2000
+			       0xf8040000 0x1000
+			       0x00000000 0x20000000>;
+			reg-names = "dbi", "dbi2", "phy", "addr_space";
+			num-ib-windows = <6>;
+			num-ob-windows = <2>;
+			num-lanes = <2>;
+			axis,syscon-pcie = <&syscon>;
+			status = "disabled";
+		};
 	};
 
 	pinctrl: pinctrl@f801d000 {

-- 
2.34.1

[PATCH RFC NOT TESTED 2/2] PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()

[Frank Li]

Remove artpec6_pcie_cpu_addr_fixup() as the DT bus fabric should provide correct
address translation. Set use_parent_dt_ranges to allow the DWC core driver to
fetch address translation from the device tree.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/dwc/pcie-artpec6.c | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c b/drivers/pci/controller/dwc/pcie-artpec6.c
index 234c8cbcae3af..d2a628a0fdc17 100644
--- a/drivers/pci/controller/dwc/pcie-artpec6.c
+++ b/drivers/pci/controller/dwc/pcie-artpec6.c
@@ -94,23 +94,6 @@ static void artpec6_pcie_writel(struct artpec6_pcie *artpec6_pcie, u32 offset, u
 	regmap_write(artpec6_pcie->regmap, offset, val);
 }
 
-static u64 artpec6_pcie_cpu_addr_fixup(struct dw_pcie *pci, u64 cpu_addr)
-{
-	struct artpec6_pcie *artpec6_pcie = to_artpec6_pcie(pci);
-	struct dw_pcie_rp *pp = &pci->pp;
-	struct dw_pcie_ep *ep = &pci->ep;
-
-	switch (artpec6_pcie->mode) {
-	case DW_PCIE_RC_TYPE:
-		return cpu_addr - pp->cfg0_base;
-	case DW_PCIE_EP_TYPE:
-		return cpu_addr - ep->phys_base;
-	default:
-		dev_err(pci->dev, "UNKNOWN device type\n");
-	}
-	return cpu_addr;
-}
-
 static int artpec6_pcie_establish_link(struct dw_pcie *pci)
 {
 	struct artpec6_pcie *artpec6_pcie = to_artpec6_pcie(pci);
@@ -134,7 +117,6 @@ static void artpec6_pcie_stop_link(struct dw_pcie *pci)
 }
 
 static const struct dw_pcie_ops dw_pcie_ops = {
-	.cpu_addr_fixup = artpec6_pcie_cpu_addr_fixup,
 	.start_link = artpec6_pcie_establish_link,
 	.stop_link = artpec6_pcie_stop_link,
 };
@@ -433,6 +415,8 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, artpec6_pcie);
 
+	pci->use_parent_dt_ranges = true;
+
 	switch (artpec6_pcie->mode) {
 	case DW_PCIE_RC_TYPE:
 		if (!IS_ENABLED(CONFIG_PCIE_ARTPEC6_HOST))

-- 
2.34.1

Re: [PATCH RFC NOT TESTED 2/2] PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()

[Bjorn Helgaas]

On Tue, Mar 04, 2025 at 12:49:36PM -0500, Frank Li wrote:
> Remove artpec6_pcie_cpu_addr_fixup() as the DT bus fabric should provide correct
> address translation. Set use_parent_dt_ranges to allow the DWC core driver to
> fetch address translation from the device tree.

Shouldn't we be able to detect platforms where DT doesn't describe the
translation correctly?  E.g., by running .cpu_addr_fixup() on a
res.start value and comparing the result to the parent_bus_addr()?
Then we could complain about it if they don't match.

Bjorn

Re: [PATCH RFC NOT TESTED 2/2] PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()

[Frank Li]

On Tue, Mar 04, 2025 at 01:08:16PM -0600, Bjorn Helgaas wrote:
> On Tue, Mar 04, 2025 at 12:49:36PM -0500, Frank Li wrote:
> > Remove artpec6_pcie_cpu_addr_fixup() as the DT bus fabric should provide correct
> > address translation. Set use_parent_dt_ranges to allow the DWC core driver to
> > fetch address translation from the device tree.
>
> Shouldn't we be able to detect platforms where DT doesn't describe the
> translation correctly?  E.g., by running .cpu_addr_fixup() on a
> res.start value and comparing the result to the parent_bus_addr()?
> Then we could complain about it if they don't match.

Can't detect because:

There are case, driver have not provide .cpu_addr_fixup, but dts still be
wrong. such as

bus@10000000
{
	ranges = <0xdeaddead 0x1000000 size>;
	pci@90000000 {

		reg = <...>, <0xdeaddead>;
		reg-names = <...>, <config>;
	}

};

above dts can work with current driver, but parent bus address 0xdeaddead
is totally fake address. We can't detect this case because no
.cpu_addr_fixup() at all.

Frank

>
> Bjorn

Re: [PATCH RFC NOT TESTED 2/2] PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()

[Bjorn Helgaas]

On Tue, Mar 04, 2025 at 03:12:11PM -0500, Frank Li wrote:
> On Tue, Mar 04, 2025 at 01:08:16PM -0600, Bjorn Helgaas wrote:
> > On Tue, Mar 04, 2025 at 12:49:36PM -0500, Frank Li wrote:
> > > Remove artpec6_pcie_cpu_addr_fixup() as the DT bus fabric should provide correct
> > > address translation. Set use_parent_dt_ranges to allow the DWC core driver to
> > > fetch address translation from the device tree.
> >
> > Shouldn't we be able to detect platforms where DT doesn't describe the
> > translation correctly?  E.g., by running .cpu_addr_fixup() on a
> > res.start value and comparing the result to the parent_bus_addr()?
> > Then we could complain about it if they don't match.
> 
> Can't detect because:
> 
> There are case, driver have not provide .cpu_addr_fixup, but dts still be
> wrong. such as
> 
> bus@10000000
> {
> 	ranges = <0xdeaddead 0x1000000 size>;
> 	pci@90000000 {
> 
> 		reg = <...>, <0xdeaddead>;
> 		reg-names = <...>, <config>;
> 	}
> 
> };
> 
> above dts can work with current driver, but parent bus address 0xdeaddead
> is totally fake address. We can't detect this case because no
> .cpu_addr_fixup() at all.

If there's no .cpu_addr_fixup(), primary-side ATU addresses must be
identical to CPU addresses.  If the DT parent bus address is
different, can't we assume the DT is broken?

Re: [PATCH RFC NOT TESTED 2/2] PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()

[Frank Li]

On Tue, Mar 04, 2025 at 02:31:46PM -0600, Bjorn Helgaas wrote:
> On Tue, Mar 04, 2025 at 03:12:11PM -0500, Frank Li wrote:
> > On Tue, Mar 04, 2025 at 01:08:16PM -0600, Bjorn Helgaas wrote:
> > > On Tue, Mar 04, 2025 at 12:49:36PM -0500, Frank Li wrote:
> > > > Remove artpec6_pcie_cpu_addr_fixup() as the DT bus fabric should provide correct
> > > > address translation. Set use_parent_dt_ranges to allow the DWC core driver to
> > > > fetch address translation from the device tree.
> > >
> > > Shouldn't we be able to detect platforms where DT doesn't describe the
> > > translation correctly?  E.g., by running .cpu_addr_fixup() on a
> > > res.start value and comparing the result to the parent_bus_addr()?
> > > Then we could complain about it if they don't match.
> >
> > Can't detect because:
> >
> > There are case, driver have not provide .cpu_addr_fixup, but dts still be
> > wrong. such as
> >
> > bus@10000000
> > {
> > 	ranges = <0xdeaddead 0x1000000 size>;
> > 	pci@90000000 {
> >
> > 		reg = <...>, <0xdeaddead>;
> > 		reg-names = <...>, <config>;
> > 	}
> >
> > };
> >
> > above dts can work with current driver, but parent bus address 0xdeaddead
> > is totally fake address. We can't detect this case because no
> > .cpu_addr_fixup() at all.
>
> If there's no .cpu_addr_fixup(), primary-side ATU addresses must be
> identical to CPU addresses.  If the DT parent bus address is
> different, can't we assume the DT is broken?

I think so.

Frank

Re: [PATCH RFC NOT TESTED 0/2] PCI: artpec6: Try to clean up artpec6_pcie_cpu_addr_fixup()

[Niklas Cassel]

Hello Frank, all,

On Tue, Mar 04, 2025 at 12:49:34PM -0500, Frank Li wrote:
> This patches basic on
> https://lore.kernel.org/imx/20250128-pci_fixup_addr-v9-0-3c4bb506f665@nxp.com/
> 
> I have not hardware to test and there are not axis,artpec7-pcie in kernel
> tree.

If you do a simple:
$ git grep artpec7

You will see that there are just two drivers that support this SoC:
drivers/pci/controller/dwc/pcie-artpec6.c
drivers/crypto/axis/artpec6_crypto.c
and their matching DT bindings:
Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
Documentation/devicetree/bindings/crypto/artpec6-crypto.txt

I think that at some point in the there was an intent to upstream support
for the ARTPEC-7 SoC, but now many years later, that hasn't happened,
as there is not even a artpec7 dtsi.

I think the nicest thing to the community is to drop artpec7 support from
these two drivers, and deprecate the artpec7 compatibles in the DT bindings,
as it is obviously making your life harder when trying to do improvements.


> 
> Look for driver owner, who help test this and start move forward to remove
> cpu_addr_fixup() work.

While I'm the original author of this PCIe driver, I do no longer have
access to the hardware and documentation, so I cannot test.

For anyone interested in the cleanup Frank is doing, see:
https://lore.kernel.org/linux-pci/Z8d96Qbggv117LlO@lizhi-Precision-Tower-5810/T/#m0ff14edaf871293ba16acd85e7942adacb603c6c

Looking at your cover-letter for the series above,
creating a simple-bus with:
ranges = <0x0 0xc0000000 0x20000000>

and handling that in PCIe DWC common code does look nicer compared to
having a .cpu_addr_fixup() callback in each PCIe DWC glue driver.

Hopefully someone with access to the hardware can test your series +
this RFC.


Kind regards,
Niklas

Re: [PATCH RFC NOT TESTED 0/2] PCI: artpec6: Try to clean up artpec6_pcie_cpu_addr_fixup()

[Niklas Cassel]

On Wed, Mar 05, 2025 at 12:18:56PM +0100, Niklas Cassel wrote:
> Hello Frank, all,
> 
> On Tue, Mar 04, 2025 at 12:49:34PM -0500, Frank Li wrote:
> > This patches basic on
> > https://lore.kernel.org/imx/20250128-pci_fixup_addr-v9-0-3c4bb506f665@nxp.com/
> > 
> > I have not hardware to test and there are not axis,artpec7-pcie in kernel
> > tree.
> 
> If you do a simple:
> $ git grep artpec7
> 
> You will see that there are just two drivers that support this SoC:
> drivers/pci/controller/dwc/pcie-artpec6.c
> drivers/crypto/axis/artpec6_crypto.c
> and their matching DT bindings:
> Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
> Documentation/devicetree/bindings/crypto/artpec6-crypto.txt
> 
> I think that at some point in the there was an intent to upstream support
> for the ARTPEC-7 SoC, but now many years later, that hasn't happened,
> as there is not even a artpec7 dtsi.
> 
> I think the nicest thing to the community is to drop artpec7 support from
> these two drivers, and deprecate the artpec7 compatibles in the DT bindings,
> as it is obviously making your life harder when trying to do improvements.
> 
> 
> > 
> > Look for driver owner, who help test this and start move forward to remove
> > cpu_addr_fixup() work.
> 
> While I'm the original author of this PCIe driver, I do no longer have
> access to the hardware and documentation, so I cannot test.
> 
> For anyone interested in the cleanup Frank is doing, see:
> https://lore.kernel.org/linux-pci/Z8d96Qbggv117LlO@lizhi-Precision-Tower-5810/T/#m0ff14edaf871293ba16acd85e7942adacb603c6c
> 
> Looking at your cover-letter for the series above,
> creating a simple-bus with:
> ranges = <0x0 0xc0000000 0x20000000>
> 
> and handling that in PCIe DWC common code does look nicer compared to
> having a .cpu_addr_fixup() callback in each PCIe DWC glue driver.
> 
> Hopefully someone with access to the hardware can test your series +
> this RFC.

Oh, and you should probably send a similar RFC for:
drivers/pci/controller/dwc/pci-dra7xx.c
drivers/pci/controller/dwc/pcie-intel-gw.c
drivers/pci/controller/dwc/pcie-visconti.c

which all make use of the .cpu_addr_fixup() callback.
(IIRC dra7xx was the first driver that introduced this callback.)


Kind regards,
Niklas

Re: [PATCH RFC NOT TESTED 0/2] PCI: artpec6: Try to clean up artpec6_pcie_cpu_addr_fixup()

[Jesper Nilsson]

Hi Frank,

I'm the current maintainer of this driver. As Niklas Cassel wrote in
another email, artpec-7 was supposed to be upstreamed, as it is in most
parts identical to the artpec-6, but reality got in the way. I don't
think there is very much left to support it at the same level as artpec-6,
but give me some time to see if the best thing is to drop the artpec-7
support as Niklas suggested.

Unfortunately, I'm travelling right now and don't have access to any
of my boards. I'll perform some testing next week when I'm back and
help to clean this up.

Best regards,

/Jesper


On Tue, Mar 04, 2025 at 12:49:34PM -0500, Frank Li wrote:
> This patches basic on
> https://lore.kernel.org/imx/20250128-pci_fixup_addr-v9-0-3c4bb506f665@nxp.com/
> 
> I have not hardware to test and there are not axis,artpec7-pcie in kernel
> tree.
> 
> Look for driver owner, who help test this and start move forward to remove
> cpu_addr_fixup() work.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Frank Li (2):
>       ARM: dts: artpec6: Move PCIe nodes under bus@c0000000
>       PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()
> 
>  arch/arm/boot/dts/axis/artpec6.dtsi       | 92 +++++++++++++++++--------------
>  drivers/pci/controller/dwc/pcie-artpec6.c | 20 +------
>  2 files changed, 52 insertions(+), 60 deletions(-)
> ---
> base-commit: 1552be4855dacca5ea39b15b1ef0b96c91dbea0d
> change-id: 20250304-axis-6d12970976b4
> 
> Best regards,
> ---
> Frank Li <Frank.Li@nxp.com>

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

Re: [PATCH RFC NOT TESTED 0/2] PCI: artpec6: Try to clean up artpec6_pcie_cpu_addr_fixup()

[Jesper Nilsson]

Hi again Frank!

I've now tested this patch-set together with your v9 on-top of the
next-branch of the pci tree, and seems to be working good on my
ARTPEC-6 set as RC:

# lspci
00:00.0 PCI bridge: Renesas Technology Corp. Device 0024
01:00.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
02:01.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
02:02.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
03:00.0 Non-Volatile memory controller: Phison Electronics Corporation E18 PCIe4 NVMe Controller (rev 01)

However, when running as EP, I found that the DT setup for pcie_ep
wasn't correct:

diff --git a/arch/arm/boot/dts/axis/artpec6.dtsi b/arch/arm/boot/dts/axis/artpec6.dtsi
index 399e87f72865..6d52f60d402d 100644
--- a/arch/arm/boot/dts/axis/artpec6.dtsi
+++ b/arch/arm/boot/dts/axis/artpec6.dtsi
@@ -195,8 +195,8 @@ pcie: pcie@f8050000 {
 
                pcie_ep: pcie_ep@f8050000 {
                        compatible = "axis,artpec6-pcie-ep", "snps,dw-pcie";
-                       reg = <0xf8050000 0x2000
-                              0xf8051000 0x2000
+                       reg = <0xf8050000 0x1000
+                              0xf8051000 0x1000
                               0xf8040000 0x1000
                               0x00000000 0x20000000>;
                        reg-names = "dbi", "dbi2", "phy", "addr_space";

Even with this fix, I get a panic in dw_pcie_read_dbi() in EP-setup,
both with and without:

"PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()"

so it looks like the ARTPEC-6 EP functionality wasn't completely tested
with this config.

The ARTPEC-7 variant does work as EP with our local config, I'll try
to see what I can do to correct ARTPEC-6 using the setup for ARTPEC-7,
and test your patchset on the ARTPEC-7.

Best regards,

/Jesper


On Wed, Mar 05, 2025 at 04:33:18PM +0100, Jesper Nilsson wrote:
> Hi Frank,
> 
> I'm the current maintainer of this driver. As Niklas Cassel wrote in
> another email, artpec-7 was supposed to be upstreamed, as it is in most
> parts identical to the artpec-6, but reality got in the way. I don't
> think there is very much left to support it at the same level as artpec-6,
> but give me some time to see if the best thing is to drop the artpec-7
> support as Niklas suggested.
> 
> Unfortunately, I'm travelling right now and don't have access to any
> of my boards. I'll perform some testing next week when I'm back and
> help to clean this up.
> 
> Best regards,
> 
> /Jesper
> 
> 
> On Tue, Mar 04, 2025 at 12:49:34PM -0500, Frank Li wrote:
> > This patches basic on
> > https://lore.kernel.org/imx/20250128-pci_fixup_addr-v9-0-3c4bb506f665@nxp.com/
> > 
> > I have not hardware to test and there are not axis,artpec7-pcie in kernel
> > tree.
> > 
> > Look for driver owner, who help test this and start move forward to remove
> > cpu_addr_fixup() work.
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > Frank Li (2):
> >       ARM: dts: artpec6: Move PCIe nodes under bus@c0000000
> >       PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()
> > 
> >  arch/arm/boot/dts/axis/artpec6.dtsi       | 92 +++++++++++++++++--------------
> >  drivers/pci/controller/dwc/pcie-artpec6.c | 20 +------
> >  2 files changed, 52 insertions(+), 60 deletions(-)
> > ---
> > base-commit: 1552be4855dacca5ea39b15b1ef0b96c91dbea0d
> > change-id: 20250304-axis-6d12970976b4
> > 
> > Best regards,
> > ---
> > Frank Li <Frank.Li@nxp.com>
> 
> /^JN - Jesper Nilsson
> -- 
>                Jesper Nilsson -- jesper.nilsson@axis.com

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

Re: [PATCH RFC NOT TESTED 0/2] PCI: artpec6: Try to clean up artpec6_pcie_cpu_addr_fixup()

[Frank Li]

On Mon, Mar 10, 2025 at 05:47:03PM +0100, Jesper Nilsson wrote:
> Hi again Frank!
>
> I've now tested this patch-set together with your v9 on-top of the
> next-branch of the pci tree, and seems to be working good on my
> ARTPEC-6 set as RC:
>
> # lspci
> 00:00.0 PCI bridge: Renesas Technology Corp. Device 0024
> 01:00.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> 02:01.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> 02:02.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> 03:00.0 Non-Volatile memory controller: Phison Electronics Corporation E18 PCIe4 NVMe Controller (rev 01)
>

Thank you very much!

Frank

> However, when running as EP, I found that the DT setup for pcie_ep
> wasn't correct:
>
> diff --git a/arch/arm/boot/dts/axis/artpec6.dtsi b/arch/arm/boot/dts/axis/artpec6.dtsi
> index 399e87f72865..6d52f60d402d 100644
> --- a/arch/arm/boot/dts/axis/artpec6.dtsi
> +++ b/arch/arm/boot/dts/axis/artpec6.dtsi
> @@ -195,8 +195,8 @@ pcie: pcie@f8050000 {
>
>                 pcie_ep: pcie_ep@f8050000 {
>                         compatible = "axis,artpec6-pcie-ep", "snps,dw-pcie";
> -                       reg = <0xf8050000 0x2000
> -                              0xf8051000 0x2000
> +                       reg = <0xf8050000 0x1000
> +                              0xf8051000 0x1000
>                                0xf8040000 0x1000
>                                0x00000000 0x20000000>;
>                         reg-names = "dbi", "dbi2", "phy", "addr_space";
>
> Even with this fix, I get a panic in dw_pcie_read_dbi() in EP-setup,
> both with and without:
>
> "PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()"
>
> so it looks like the ARTPEC-6 EP functionality wasn't completely tested
> with this config.
>
> The ARTPEC-7 variant does work as EP with our local config, I'll try
> to see what I can do to correct ARTPEC-6 using the setup for ARTPEC-7,
> and test your patchset on the ARTPEC-7.
>
> Best regards,
>
> /Jesper
>
>
> On Wed, Mar 05, 2025 at 04:33:18PM +0100, Jesper Nilsson wrote:
> > Hi Frank,
> >
> > I'm the current maintainer of this driver. As Niklas Cassel wrote in
> > another email, artpec-7 was supposed to be upstreamed, as it is in most
> > parts identical to the artpec-6, but reality got in the way. I don't
> > think there is very much left to support it at the same level as artpec-6,
> > but give me some time to see if the best thing is to drop the artpec-7
> > support as Niklas suggested.
> >
> > Unfortunately, I'm travelling right now and don't have access to any
> > of my boards. I'll perform some testing next week when I'm back and
> > help to clean this up.
> >
> > Best regards,
> >
> > /Jesper
> >
> >
> > On Tue, Mar 04, 2025 at 12:49:34PM -0500, Frank Li wrote:
> > > This patches basic on
> > > https://lore.kernel.org/imx/20250128-pci_fixup_addr-v9-0-3c4bb506f665@nxp.com/
> > >
> > > I have not hardware to test and there are not axis,artpec7-pcie in kernel
> > > tree.
> > >
> > > Look for driver owner, who help test this and start move forward to remove
> > > cpu_addr_fixup() work.
> > >
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > Frank Li (2):
> > >       ARM: dts: artpec6: Move PCIe nodes under bus@c0000000
> > >       PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()
> > >
> > >  arch/arm/boot/dts/axis/artpec6.dtsi       | 92 +++++++++++++++++--------------
> > >  drivers/pci/controller/dwc/pcie-artpec6.c | 20 +------
> > >  2 files changed, 52 insertions(+), 60 deletions(-)
> > > ---
> > > base-commit: 1552be4855dacca5ea39b15b1ef0b96c91dbea0d
> > > change-id: 20250304-axis-6d12970976b4
> > >
> > > Best regards,
> > > ---
> > > Frank Li <Frank.Li@nxp.com>
> >
> > /^JN - Jesper Nilsson
> > --
> >                Jesper Nilsson -- jesper.nilsson@axis.com
>
> /^JN - Jesper Nilsson
> --
>                Jesper Nilsson -- jesper.nilsson@axis.com

Re: [PATCH RFC NOT TESTED 0/2] PCI: artpec6: Try to clean up artpec6_pcie_cpu_addr_fixup()

[Bjorn Helgaas]

On Mon, Mar 10, 2025 at 05:47:03PM +0100, Jesper Nilsson wrote:
> Hi again Frank!
> 
> I've now tested this patch-set together with your v9 on-top of the
> next-branch of the pci tree, and seems to be working good on my
> ARTPEC-6 set as RC:
> 
> # lspci
> 00:00.0 PCI bridge: Renesas Technology Corp. Device 0024
> 01:00.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> 02:01.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> 02:02.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> 03:00.0 Non-Volatile memory controller: Phison Electronics Corporation E18 PCIe4 NVMe Controller (rev 01)
> 
> However, when running as EP, I found that the DT setup for pcie_ep
> wasn't correct:
> 
> diff --git a/arch/arm/boot/dts/axis/artpec6.dtsi b/arch/arm/boot/dts/axis/artpec6.dtsi
> index 399e87f72865..6d52f60d402d 100644
> --- a/arch/arm/boot/dts/axis/artpec6.dtsi
> +++ b/arch/arm/boot/dts/axis/artpec6.dtsi
> @@ -195,8 +195,8 @@ pcie: pcie@f8050000 {
>  
>                 pcie_ep: pcie_ep@f8050000 {
>                         compatible = "axis,artpec6-pcie-ep", "snps,dw-pcie";
> -                       reg = <0xf8050000 0x2000
> -                              0xf8051000 0x2000
> +                       reg = <0xf8050000 0x1000
> +                              0xf8051000 0x1000
>                                0xf8040000 0x1000
>                                0x00000000 0x20000000>;
>                         reg-names = "dbi", "dbi2", "phy", "addr_space";
> 
> Even with this fix, I get a panic in dw_pcie_read_dbi() in EP-setup,
> both with and without:
> 
> "PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()"
> 
> so it looks like the ARTPEC-6 EP functionality wasn't completely tested
> with this config.
> 
> The ARTPEC-7 variant does work as EP with our local config, I'll try
> to see what I can do to correct ARTPEC-6 using the setup for ARTPEC-7,
> and test your patchset on the ARTPEC-7.

Where are we at with this?

First priority: I plan to merge v12 of Frank's series [1] for v6.15.
I hope this works with existing DTs on artpec6, both for RC and EP.
If not, we need to figure it out ASAP.

Second priority: For this series of:

  ARM: dts: artpec6: Move PCIe nodes under bus@c0000000
  PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()

it looks like there's an open issue with the dts patch that Rob
noticed [2].  It would be great if we could fix that issue and get it
queued up if it's safe to merge independently of Frank's v12 series.

It looks like the artpec6_pcie_cpu_addr_fixup() removal probably needs
to be delayed until we know all DTs in the field are fixed?  This
might mean that we can *never* remove artpec6_pcie_cpu_addr_fixup()
unless we can identify and work around the broken DTs in the kernel.

[1] https://lore.kernel.org/r/20250315201548.858189-1-helgaas@kernel.org
[2] https://lore.kernel.org/r/174170613961.3566466.13045709851799071104.robh@kernel.org

> On Wed, Mar 05, 2025 at 04:33:18PM +0100, Jesper Nilsson wrote:
> > Hi Frank,
> > 
> > I'm the current maintainer of this driver. As Niklas Cassel wrote in
> > another email, artpec-7 was supposed to be upstreamed, as it is in most
> > parts identical to the artpec-6, but reality got in the way. I don't
> > think there is very much left to support it at the same level as artpec-6,
> > but give me some time to see if the best thing is to drop the artpec-7
> > support as Niklas suggested.
> > 
> > Unfortunately, I'm travelling right now and don't have access to any
> > of my boards. I'll perform some testing next week when I'm back and
> > help to clean this up.
> > 
> > Best regards,
> > 
> > /Jesper
> > 
> > 
> > On Tue, Mar 04, 2025 at 12:49:34PM -0500, Frank Li wrote:
> > > This patches basic on
> > > https://lore.kernel.org/imx/20250128-pci_fixup_addr-v9-0-3c4bb506f665@nxp.com/
> > > 
> > > I have not hardware to test and there are not axis,artpec7-pcie in kernel
> > > tree.
> > > 
> > > Look for driver owner, who help test this and start move forward to remove
> > > cpu_addr_fixup() work.
> > > 
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > Frank Li (2):
> > >       ARM: dts: artpec6: Move PCIe nodes under bus@c0000000
> > >       PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()
> > > 
> > >  arch/arm/boot/dts/axis/artpec6.dtsi       | 92 +++++++++++++++++--------------
> > >  drivers/pci/controller/dwc/pcie-artpec6.c | 20 +------
> > >  2 files changed, 52 insertions(+), 60 deletions(-)
> > > ---
> > > base-commit: 1552be4855dacca5ea39b15b1ef0b96c91dbea0d
> > > change-id: 20250304-axis-6d12970976b4
> > > 
> > > Best regards,
> > > ---
> > > Frank Li <Frank.Li@nxp.com>
> > 
> > /^JN - Jesper Nilsson
> > -- 
> >                Jesper Nilsson -- jesper.nilsson@axis.com
> 
> /^JN - Jesper Nilsson
> -- 
>                Jesper Nilsson -- jesper.nilsson@axis.com

Re: [PATCH RFC NOT TESTED 0/2] PCI: artpec6: Try to clean up artpec6_pcie_cpu_addr_fixup()

[Niklas Cassel]

Hello Bjorn, Jesper,

On Mon, Mar 17, 2025 at 12:54:19PM -0500, Bjorn Helgaas wrote:
> On Mon, Mar 10, 2025 at 05:47:03PM +0100, Jesper Nilsson wrote:
> > I've now tested this patch-set together with your v9 on-top of the
> > next-branch of the pci tree, and seems to be working good on my
> > ARTPEC-6 set as RC:
> > 
> > # lspci
> > 00:00.0 PCI bridge: Renesas Technology Corp. Device 0024
> > 01:00.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> > 02:01.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> > 02:02.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> > 03:00.0 Non-Volatile memory controller: Phison Electronics Corporation E18 PCIe4 NVMe Controller (rev 01)
> > 
> > However, when running as EP, I found that the DT setup for pcie_ep
> > wasn't correct:
> > 
> > diff --git a/arch/arm/boot/dts/axis/artpec6.dtsi b/arch/arm/boot/dts/axis/artpec6.dtsi
> > index 399e87f72865..6d52f60d402d 100644
> > --- a/arch/arm/boot/dts/axis/artpec6.dtsi
> > +++ b/arch/arm/boot/dts/axis/artpec6.dtsi
> > @@ -195,8 +195,8 @@ pcie: pcie@f8050000 {
> >  
> >                 pcie_ep: pcie_ep@f8050000 {
> >                         compatible = "axis,artpec6-pcie-ep", "snps,dw-pcie";
> > -                       reg = <0xf8050000 0x2000
> > -                              0xf8051000 0x2000
> > +                       reg = <0xf8050000 0x1000
> > +                              0xf8051000 0x1000
> >                                0xf8040000 0x1000
> >                                0x00000000 0x20000000>;
> >                         reg-names = "dbi", "dbi2", "phy", "addr_space";
> > 
> > Even with this fix, I get a panic in dw_pcie_read_dbi() in EP-setup,
> > both with and without:

Your fix looks correct to me.

You should even be able keep dbi as 0x2000, and simply remove the dbi2
from "reg" and "reg-names", as the driver should be able to infer dbi2
automatically:
https://github.com/torvalds/linux/blob/v6.14-rc7/drivers/pci/controller/dwc/pcie-designware.c#L119-L128

But your fix seems more correct.
You should probably also change the size of "dbi" to 0x1000 in the RC node.


For the panic, could you please share the stack trace?
(Perhaps we can help)


> > 
> > "PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()"
> > 
> > so it looks like the ARTPEC-6 EP functionality wasn't completely tested
> > with this config.
> > 
> > The ARTPEC-7 variant does work as EP with our local config, I'll try
> > to see what I can do to correct ARTPEC-6 using the setup for ARTPEC-7,
> > and test your patchset on the ARTPEC-7.
> 
> Where are we at with this?

My recommendation would be to:
1) Get artpec6 EP mode working with v6.14-rc7
2) Try v6.14-rc7 + v12 of Frank's series
3) Try v6.14-rc7 + v12 of Frank's series + this series


> 
> First priority: I plan to merge v12 of Frank's series [1] for v6.15.
> I hope this works with existing DTs on artpec6, both for RC and EP.
> If not, we need to figure it out ASAP.
> 
> Second priority: For this series of:
> 
>   ARM: dts: artpec6: Move PCIe nodes under bus@c0000000
>   PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()
> 
> it looks like there's an open issue with the dts patch that Rob
> noticed [2].  It would be great if we could fix that issue and get it
> queued up if it's safe to merge independently of Frank's v12 series.

Rob's bot is simply complaining that there is no DT schema.

This is because the DT schema for axis,artpec6-pcie has not been
converted to YAML.

It would be nice to convert it, but I don't think it should stop
other improvements for this driver.


> It looks like the artpec6_pcie_cpu_addr_fixup() removal probably needs
> to be delayed until we know all DTs in the field are fixed?  This
> might mean that we can *never* remove artpec6_pcie_cpu_addr_fixup()
> unless we can identify and work around the broken DTs in the kernel.

Jesper should be able to answer this, but as far as I know, artpec6 is only
used in-house, so they have full control of the DTBs.

(i.e. artpec6_pcie_cpu_addr_fixup() can probably be killed quite quickly,
once "v6.14-rc7 + v12 of Frank's series + this series" gets working.)


Kind regards,
Niklas

Re: [PATCH RFC NOT TESTED 0/2] PCI: artpec6: Try to clean up artpec6_pcie_cpu_addr_fixup()

[Bjorn Helgaas]

On Tue, Mar 18, 2025 at 10:01:48AM +0100, Niklas Cassel wrote:
> On Mon, Mar 17, 2025 at 12:54:19PM -0500, Bjorn Helgaas wrote:
> > On Mon, Mar 10, 2025 at 05:47:03PM +0100, Jesper Nilsson wrote:
> > > I've now tested this patch-set together with your v9 on-top of the
> > > next-branch of the pci tree, and seems to be working good on my
> > > ARTPEC-6 set as RC:
> > > 
> > > # lspci
> > > 00:00.0 PCI bridge: Renesas Technology Corp. Device 0024
> > > 01:00.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> > > 02:01.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> > > 02:02.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> > > 03:00.0 Non-Volatile memory controller: Phison Electronics Corporation E18 PCIe4 NVMe Controller (rev 01)
> > > 
> > > However, when running as EP, I found that the DT setup for pcie_ep
> > > wasn't correct:
> > > 
> > > diff --git a/arch/arm/boot/dts/axis/artpec6.dtsi b/arch/arm/boot/dts/axis/artpec6.dtsi
> > > index 399e87f72865..6d52f60d402d 100644
> > > --- a/arch/arm/boot/dts/axis/artpec6.dtsi
> > > +++ b/arch/arm/boot/dts/axis/artpec6.dtsi
> > > @@ -195,8 +195,8 @@ pcie: pcie@f8050000 {
> > >  
> > >                 pcie_ep: pcie_ep@f8050000 {
> > >                         compatible = "axis,artpec6-pcie-ep", "snps,dw-pcie";
> > > -                       reg = <0xf8050000 0x2000
> > > -                              0xf8051000 0x2000
> > > +                       reg = <0xf8050000 0x1000
> > > +                              0xf8051000 0x1000
> > >                                0xf8040000 0x1000
> > >                                0x00000000 0x20000000>;
> > >                         reg-names = "dbi", "dbi2", "phy", "addr_space";
> > > 
> > > Even with this fix, I get a panic in dw_pcie_read_dbi() in EP-setup,
> > > both with and without:
> 
> Your fix looks correct to me.
> 
> You should even be able keep dbi as 0x2000, and simply remove the dbi2
> from "reg" and "reg-names", as the driver should be able to infer dbi2
> automatically:
> https://github.com/torvalds/linux/blob/v6.14-rc7/drivers/pci/controller/dwc/pcie-designware.c#L119-L128
> 
> But your fix seems more correct.
> You should probably also change the size of "dbi" to 0x1000 in the RC node.

Just a ping to see if there's any chance of getting this into v6.15?

To do that, I think we'd need to confirm that:

  - the endpoint issue is fixed

  - artpec6 is only used in-house

  - the DTBs are either already OK or OK after [PATCH 1/2]

  - everybody in-house is OK with updating to the new DTB.

I haven't seen anything about the endpoint part, and haven't seen
confirmation of the others.

Bjorn

Re: [PATCH RFC NOT TESTED 0/2] PCI: artpec6: Try to clean up artpec6_pcie_cpu_addr_fixup()

[Jesper Nilsson]

On Thu, Mar 20, 2025 at 04:54:05PM -0500, Bjorn Helgaas wrote:
> On Tue, Mar 18, 2025 at 10:01:48AM +0100, Niklas Cassel wrote:
> > On Mon, Mar 17, 2025 at 12:54:19PM -0500, Bjorn Helgaas wrote:
> > > On Mon, Mar 10, 2025 at 05:47:03PM +0100, Jesper Nilsson wrote:
> > > > I've now tested this patch-set together with your v9 on-top of the
> > > > next-branch of the pci tree, and seems to be working good on my
> > > > ARTPEC-6 set as RC:
> > > > 
> > > > # lspci
> > > > 00:00.0 PCI bridge: Renesas Technology Corp. Device 0024
> > > > 01:00.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> > > > 02:01.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> > > > 02:02.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> > > > 03:00.0 Non-Volatile memory controller: Phison Electronics Corporation E18 PCIe4 NVMe Controller (rev 01)
> > > > 
> > > > However, when running as EP, I found that the DT setup for pcie_ep
> > > > wasn't correct:
> > > > 
> > > > diff --git a/arch/arm/boot/dts/axis/artpec6.dtsi b/arch/arm/boot/dts/axis/artpec6.dtsi
> > > > index 399e87f72865..6d52f60d402d 100644
> > > > --- a/arch/arm/boot/dts/axis/artpec6.dtsi
> > > > +++ b/arch/arm/boot/dts/axis/artpec6.dtsi
> > > > @@ -195,8 +195,8 @@ pcie: pcie@f8050000 {
> > > >  
> > > >                 pcie_ep: pcie_ep@f8050000 {
> > > >                         compatible = "axis,artpec6-pcie-ep", "snps,dw-pcie";
> > > > -                       reg = <0xf8050000 0x2000
> > > > -                              0xf8051000 0x2000
> > > > +                       reg = <0xf8050000 0x1000
> > > > +                              0xf8051000 0x1000
> > > >                                0xf8040000 0x1000
> > > >                                0x00000000 0x20000000>;
> > > >                         reg-names = "dbi", "dbi2", "phy", "addr_space";
> > > > 
> > > > Even with this fix, I get a panic in dw_pcie_read_dbi() in EP-setup,
> > > > both with and without:
> > 
> > Your fix looks correct to me.
> > 
> > You should even be able keep dbi as 0x2000, and simply remove the dbi2
> > from "reg" and "reg-names", as the driver should be able to infer dbi2
> > automatically:
> > https://github.com/torvalds/linux/blob/v6.14-rc7/drivers/pci/controller/dwc/pcie-designware.c#L119-L128
> > 
> > But your fix seems more correct.
> > You should probably also change the size of "dbi" to 0x1000 in the RC node.
> 
> Just a ping to see if there's any chance of getting this into v6.15?
> 
> To do that, I think we'd need to confirm that:
> 
>   - the endpoint issue is fixed

Unfortunately, I've been unable to get any resolution to this problem.

- Tested on units without the PCIe switch - still fail
- Tested on another units - still fail
- Tested with 6.12-rc7 - fail
- Tested with 6.6-rc6 - fail
- Tested with 6.1 - fail
- Older kernels fail with toolchain or boot problems due to my hacky
  tools for booting upstream kernels so I can't easily test
- Tested with original bringup kernel 4.19 - no crash, but endpoint is
  not enumerated from RC. This contains lots of hacked around code
  which could also come into play, and there is an untested hardware
  adapter in between the two boards.

The panic looks like this (with some minor variations due to kernel version)

=============
[   16.601883] 8<--- cut here ---
[   16.604937] Unhandled fault: external abort on non-linefetch (0x008) at 0xf0bdd00e
[   16.612502] [f0bdd00e] *pgd=029d6811, *pte=f8050243, *ppte=f8050013
[   16.618775] Internal error: : 8 [#1] SMP ARM
[   16.623041] Modules linked in:
[   16.626092] CPU: 0 UID: 0 PID: 1 Comm: sh Not tainted 6.14.0-rc4-g98c0bcfef512-dirty #26
[   16.634181] Hardware name: Axis ARTPEC-6 Platform
[   16.638877] PC is at dw_pcie_read_dbi+0x60/0xa4
[   16.643414] LR is at 0x0
[   16.645942] pc : [<c05d9e64>]    lr : [<00000000>]    psr: 60000013
[   16.652203] sp : f0819bc0  ip : c1e2b840  fp : 00000000
[   16.657420] r10: c1d9f000  r9 : c1efdbc0  r8 : c1e2b930
[   16.662638] r7 : c0d2f188  r6 : c1e2b840  r5 : c1e2b930  r4 : 00000000
[   16.669159] r3 : 00000001  r2 : 00000000  r1 : f0bdd00e  r0 : c1e2b840
[   16.675679] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[   16.682810] Control: 10c5387d  Table: 02bec04a  DAC: 00000051
[   16.688548] Register r0 information: slab kmalloc-2k start c1e2b800 pointer offset 64 size 2048
[   16.697257] Register r1 information: 0-page vmalloc region starting at 0xf0bdd000 allocated at devm_pci_remap_cfgspace+0x4c/0x80
[   16.708826] Register r2 information: NULL pointer
[   16.713527] Register r3 information: non-paged memory
[   16.718573] Register r4 information: NULL pointer
[   16.723272] Register r5 information: slab kmalloc-2k start c1e2b800 pointer offset 304 size 2048
[   16.732064] Register r6 information: slab kmalloc-2k start c1e2b800 pointer offset 64 size 2048
[   16.740769] Register r7 information: non-slab/vmalloc memory
[   16.746424] Register r8 information: slab kmalloc-2k start c1e2b800 pointer offset 304 size 2048
[   16.755215] Register r9 information: slab kmalloc-128 start c1efdb80 pointer offset 64 size 128
[   16.763919] Register r10 information: slab kmalloc-1k start c1d9f000 pointer offset 0 size 1024
[   16.772624] Register r11 information: NULL pointer
[   16.777410] Register r12 information: slab kmalloc-2k start c1e2b800 pointer offset 64 size 2048
[   16.786201] Process sh (pid: 1, stack limit = 0xf7eac871)
[   16.791596] Stack: (0xf0819bc0 to 0xf081a000)
[   16.795951] 9bc0: 00000000 c05dd354 c1d9f410 c12ef02c 00000000 00000000 c1d9f410 c1e2b840
[   16.804125] 9be0: c0d2f188 c1e2b930 c1efdbc0 c12ef02c 00000000 c010b70c 01040800 3841aac4
[   16.812298] 9c00: c1d9f410 00000000 c1d9f410 c12e01a4 c1317740 00000000 00000000 c06f7200
[   16.820471] 9c20: 00000000 c1d9f410 c12e01a4 c06f464c c1d9f410 00000000 c1d9f410 c12e01a4
[   16.828643] 9c40: c1d9f410 00000006 c1317428 c06f49d4 60000013 00000000 c138a5f8 c12e01a4
[   16.836816] 9c60: c1d9f410 00000006 c1317428 c06f4be4 00000001 c12e01a4 f0819ccc c1d9f410
[   16.844989] 9c80: c1317428 c06f4d1c 00000000 f0819ccc c06f4c74 c1418100 c1317428 c06f2714
[   16.853162] 9ca0: c1418100 c141816c c15a8cb8 3841aac4 00000000 c1d9f410 00000001 c1d9f454
[   16.861334] 9cc0: c1418100 c06f50ec 00000000 c1d9f410 00000001 3841aac4 c1d9f410 c1d9f410
[   16.869507] 9ce0: cfef0e4c c06f35f0 c1d9f410 00000000 cfef0e4c c16a5810 c1317428 c06f0ea0
[   16.877680] 9d00: 00000000 00000000 00000000 00000000 00000000 3841aac4 c1d9f400 00000000
[   16.885852] 9d20: 00000000 c16a5810 cfef0ea4 ffffffff c0fdff38 c0a0a6d8 f0819da0 00000000
[   16.894025] 9d40: c16a5800 f0819da0 c1308d3c c0a0a81c 00000000 3841aac4 c1308d30 c12dea88
[   16.902198] 9d60: 00000005 c015642c f0819da0 00000005 cfef0e40 cfef1080 c1ed4d40 c0fdd378
[   16.910371] 9d80: c0fdff38 c0a0f0a8 c1ed4554 c0a08e44 c1ed4540 c1ed4540 00000005 c0a0f240
[   16.918543] 9da0: cfef0e40 c1ed4d40 cfef1080 c0a0f4e8 00000000 00000000 00000000 00000000
[   16.926715] 9dc0: 00000000 00000000 00000000 00000000 00000000 3841aac4 c1ed4540 00000000
[   16.934888] 9de0: c1ed4c68 c1328858 00000001 c0a100bc 00000000 c1ed4c40 c1ed4c68 c1ed4c68
[   16.943060] 9e00: 00000001 c0a15e5c 00000000 c0ffa474 00008060 c1ed4c40 00000000 c0fdfd74
[   16.951233] 9e20: c1efd1c8 c1328f60 c138d3dc c1ed4c68 c1e76400 00000000 00000000 cfef0e40
[   16.959406] 9e40: 00000001 3841aac4 00000000 00000000 c1308df0 c1308ddc c1e76400 00000015
[   16.967579] 9e60: c1efd180 c1efd188 00000000 c0a1640c c1ed4940 7265766f 2d79616c 50646e45
[   16.975751] 9e80: 746e696f 6274642e 00000000 00000000 00000000 00000000 00000000 00000000
[   16.983924] 9ea0: 00000000 00000000 00000000 00000000 00000000 3841aac4 c1ed4e80 c1efd600
[   16.992096] 9ec0: 00000015 f0819f28 c1efd610 00000000 00000000 c02f9704 00000000 00000000
[   17.000269] 9ee0: c1eca400 f0819f80 c02f9608 c1448000 00000015 c0d0b648 00000000 c026fc44
[   17.008441] 9f00: 00000000 00000000 00000000 00000000 00010000 00000015 005a8fb8 00000000
[   17.016613] 9f20: 00000001 00000000 c1eca400 00000000 00000000 00000000 00000000 00000000
[   17.024786] 9f40: 00000000 00000000 00000000 00000000 00000000 3841aac4 00000000 c1eca400
[   17.032958] 9f60: c1eca400 00000000 00000000 c01002c4 c1448000 00000004 00000000 c026ff60
[   17.041131] 9f80: 00000000 00000000 00000000 3841aac4 00000003 00000015 005a8fb8 b6f1fbb0
[   17.049304] 9fa0: 00000004 c0100060 00000015 005a8fb8 00000001 005a8fb8 00000015 00000001
[   17.057476] 9fc0: 00000015 005a8fb8 b6f1fbb0 00000004 00000000 0057cdb0 00000000 00000000
[   17.065649] 9fe0: 00000004 beb0aa08 b6ec0d0f b6e3d5e6 40000030 00000001 00000000 00000000
[   17.073818] Call trace:
[   17.073828]  dw_pcie_read_dbi from dw_pcie_ep_init_registers+0x2c/0x3bc
[   17.082978]  dw_pcie_ep_init_registers from artpec6_pcie_probe+0x164/0x1dc
[   17.089867]  artpec6_pcie_probe from platform_probe+0x5c/0xb0
[   17.095621]  platform_probe from really_probe+0xe0/0x3cc
[   17.100937]  really_probe from __driver_probe_device+0x9c/0x1e4
[   17.106865]  __driver_probe_device from driver_probe_device+0x30/0xc0
[   17.113313]  driver_probe_device from __device_attach_driver+0xa8/0x120
[   17.119934]  __device_attach_driver from bus_for_each_drv+0x90/0xe4
[   17.126208]  bus_for_each_drv from __device_attach+0xa8/0x1d4
[   17.131961]  __device_attach from bus_probe_device+0x88/0x8c
[   17.137626]  bus_probe_device from device_add+0x5f0/0x804
[   17.143030]  device_add from of_platform_device_create_pdata+0xa8/0xec
[   17.149568]  of_platform_device_create_pdata from of_platform_notify+0xf4/0x168
[   17.156886]  of_platform_notify from blocking_notifier_call_chain+0x6c/0x90
[   17.163861]  blocking_notifier_call_chain from of_reconfig_notify+0x38/0xa0
[   17.170831]  of_reconfig_notify from __of_changeset_entry_notify+0x130/0x14c
[   17.177882]  __of_changeset_entry_notify from __of_changeset_apply_notify+0x44/0xa8
[   17.185542]  __of_changeset_apply_notify from of_overlay_fdt_apply+0x8f4/0xa74
[   17.192771]  of_overlay_fdt_apply from overlays_store+0x160/0x21c
[   17.198872]  overlays_store from kernfs_fop_write_iter+0xfc/0x1e8
[   17.204978]  kernfs_fop_write_iter from vfs_write+0x244/0x414
[   17.210731]  vfs_write from ksys_write+0x6c/0xdc
[   17.215347]  ksys_write from ret_fast_syscall+0x0/0x54
[   17.220485] Exception stack(0xf0819fa8 to 0xf0819ff0)
[   17.225533] 9fa0:                   00000015 005a8fb8 00000001 005a8fb8 00000015 00000001
[   17.233706] 9fc0: 00000015 005a8fb8 b6f1fbb0 00000004 00000000 0057cdb0 00000000 00000000
[   17.241877] 9fe0: 00000004 beb0aa08 b6ec0d0f b6e3d5e6
[   17.246926] Code: e3530002 0a000005 e3530001 1a00000a (e5d10000)
[   17.253013] ---[ end trace 0000000000000000 ]---
[   17.257624] note: sh[1] exited with irqs disabled
[   17.262440] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[   17.274976] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
=============

As you might see, I trigger loading with some overlay magic for debugging,
but it's the same if I have it compiled in.

>   - artpec6 is only used in-house

Correct.

>   - the DTBs are either already OK or OK after [PATCH 1/2]

After my patch, yes.

>   - everybody in-house is OK with updating to the new DTB.

OK for me.

> I haven't seen anything about the endpoint part, and haven't seen
> confirmation of the others.
> 
> Bjorn

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

Re: [PATCH RFC NOT TESTED 0/2] PCI: artpec6: Try to clean up artpec6_pcie_cpu_addr_fixup()

[Bjorn Helgaas]

On Fri, Mar 21, 2025 at 06:17:42PM +0100, Jesper Nilsson wrote:
> On Thu, Mar 20, 2025 at 04:54:05PM -0500, Bjorn Helgaas wrote:
> > On Tue, Mar 18, 2025 at 10:01:48AM +0100, Niklas Cassel wrote:
> > > On Mon, Mar 17, 2025 at 12:54:19PM -0500, Bjorn Helgaas wrote:
> > > > On Mon, Mar 10, 2025 at 05:47:03PM +0100, Jesper Nilsson wrote:
> > > > > I've now tested this patch-set together with your v9 on-top of the
> > > > > next-branch of the pci tree, and seems to be working good on my
> > > > > ARTPEC-6 set as RC:
> > > > > 
> > > > > # lspci
> > > > > 00:00.0 PCI bridge: Renesas Technology Corp. Device 0024
> > > > > 01:00.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> > > > > 02:01.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> > > > > 02:02.0 PCI bridge: Pericom Semiconductor PI7C9X2G304 EL/SL PCIe2 3-Port/4-Lane Packet Switch (rev 05)
> > > > > 03:00.0 Non-Volatile memory controller: Phison Electronics Corporation E18 PCIe4 NVMe Controller (rev 01)
> > > > > 
> > > > > However, when running as EP, I found that the DT setup for pcie_ep
> > > > > wasn't correct:
> > > > > 
> > > > > diff --git a/arch/arm/boot/dts/axis/artpec6.dtsi b/arch/arm/boot/dts/axis/artpec6.dtsi
> > > > > index 399e87f72865..6d52f60d402d 100644
> > > > > --- a/arch/arm/boot/dts/axis/artpec6.dtsi
> > > > > +++ b/arch/arm/boot/dts/axis/artpec6.dtsi
> > > > > @@ -195,8 +195,8 @@ pcie: pcie@f8050000 {
> > > > >  
> > > > >                 pcie_ep: pcie_ep@f8050000 {
> > > > >                         compatible = "axis,artpec6-pcie-ep", "snps,dw-pcie";
> > > > > -                       reg = <0xf8050000 0x2000
> > > > > -                              0xf8051000 0x2000
> > > > > +                       reg = <0xf8050000 0x1000
> > > > > +                              0xf8051000 0x1000
> > > > >                                0xf8040000 0x1000
> > > > >                                0x00000000 0x20000000>;
> > > > >                         reg-names = "dbi", "dbi2", "phy", "addr_space";
> > > > > 
> > > > > Even with this fix, I get a panic in dw_pcie_read_dbi() in EP-setup,
> > > > > both with and without:
> > > 
> > > Your fix looks correct to me.
> > > 
> > > You should even be able keep dbi as 0x2000, and simply remove the dbi2
> > > from "reg" and "reg-names", as the driver should be able to infer dbi2
> > > automatically:
> > > https://github.com/torvalds/linux/blob/v6.14-rc7/drivers/pci/controller/dwc/pcie-designware.c#L119-L128
> > > 
> > > But your fix seems more correct.
> > > You should probably also change the size of "dbi" to 0x1000 in the RC node.
> > 
> > Just a ping to see if there's any chance of getting this into v6.15?
> > 
> > To do that, I think we'd need to confirm that:
> > 
> >   - the endpoint issue is fixed
> 
> Unfortunately, I've been unable to get any resolution to this problem.
> 
> - Tested on units without the PCIe switch - still fail
> - Tested on another units - still fail
> - Tested with 6.12-rc7 - fail
> - Tested with 6.6-rc6 - fail
> - Tested with 6.1 - fail
> - Older kernels fail with toolchain or boot problems due to my hacky
>   tools for booting upstream kernels so I can't easily test
> - Tested with original bringup kernel 4.19 - no crash, but endpoint is
>   not enumerated from RC. This contains lots of hacked around code
>   which could also come into play, and there is an untested hardware
>   adapter in between the two boards.

So it sounds like current kernels are already broken for endpoints,
regardless of Frank's series?

If that's the case, I'd be open to merging the removal of
artpec6_pcie_cpu_addr_fixup() because we wouldn't be breaking a
currently-working configuration

And if we don't have artpec6_pcie_cpu_addr_fixup() in the mix, we'll
have a better chance of fixing it nicely, by deriving intermediate
addresses from the devicetree instead of forcing them with
.cpu_addr_fixup().

> The panic looks like this (with some minor variations due to kernel version)

If you had Frank's series included, it should log some useful debug
about the intermediate address offset it derives.  But I don't think
you're getting far enough for this to be the problem.

> =============
> [   16.601883] 8<--- cut here ---
> [   16.604937] Unhandled fault: external abort on non-linefetch (0x008) at 0xf0bdd00e

My guess is:

  artpec6_pcie_probe
    dw_pcie_ep_init
      dw_pcie_ep_get_resources
        dw_pcie_get_resources
          res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi")
          pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res)
      dw_pcie_ep_init_registers
        dw_pcie_readb_dbi(PCI_HEADER_TYPE)    # 0x0e
          dw_pcie_read_dbi
            dw_pcie_read(addr = pci->dbi_base + PCI_HEADER_TYPE)
              readb(addr)

It looks like pci->dbi_base *should* be initialized by this time.  I
assume the hardware does support single-byte, non-aligned reads?

> [   16.612502] [f0bdd00e] *pgd=029d6811, *pte=f8050243, *ppte=f8050013
> [   16.618775] Internal error: : 8 [#1] SMP ARM
> [   16.623041] Modules linked in:
> [   16.626092] CPU: 0 UID: 0 PID: 1 Comm: sh Not tainted 6.14.0-rc4-g98c0bcfef512-dirty #26
> [   16.634181] Hardware name: Axis ARTPEC-6 Platform
> [   16.638877] PC is at dw_pcie_read_dbi+0x60/0xa4
> [   16.643414] LR is at 0x0
> [   16.645942] pc : [<c05d9e64>]    lr : [<00000000>]    psr: 60000013
> [   16.652203] sp : f0819bc0  ip : c1e2b840  fp : 00000000
> [   16.657420] r10: c1d9f000  r9 : c1efdbc0  r8 : c1e2b930
> [   16.662638] r7 : c0d2f188  r6 : c1e2b840  r5 : c1e2b930  r4 : 00000000
> [   16.669159] r3 : 00000001  r2 : 00000000  r1 : f0bdd00e  r0 : c1e2b840
> [   16.675679] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> [   16.682810] Control: 10c5387d  Table: 02bec04a  DAC: 00000051
> ...

> [   17.073818] Call trace:
> [   17.073828]  dw_pcie_read_dbi from dw_pcie_ep_init_registers+0x2c/0x3bc
> [   17.082978]  dw_pcie_ep_init_registers from artpec6_pcie_probe+0x164/0x1dc
> [   17.089867]  artpec6_pcie_probe from platform_probe+0x5c/0xb0
> [   17.095621]  platform_probe from really_probe+0xe0/0x3cc

Re: [PATCH RFC NOT TESTED 0/2] PCI: artpec6: Try to clean up artpec6_pcie_cpu_addr_fixup()

[Rob Herring (Arm)]

On Wed, 05 Mar 2025 16:33:18 +0100, Jesper Nilsson wrote:
> Hi Frank,
> 
> I'm the current maintainer of this driver. As Niklas Cassel wrote in
> another email, artpec-7 was supposed to be upstreamed, as it is in most
> parts identical to the artpec-6, but reality got in the way. I don't
> think there is very much left to support it at the same level as artpec-6,
> but give me some time to see if the best thing is to drop the artpec-7
> support as Niklas suggested.
> 
> Unfortunately, I'm travelling right now and don't have access to any
> of my boards. I'll perform some testing next week when I'm back and
> help to clean this up.
> 
> Best regards,
> 
> /Jesper
> 
> 
> On Tue, Mar 04, 2025 at 12:49:34PM -0500, Frank Li wrote:
> > This patches basic on
> > https://lore.kernel.org/imx/20250128-pci_fixup_addr-v9-0-3c4bb506f665@nxp.com/
> >
> > I have not hardware to test and there are not axis,artpec7-pcie in kernel
> > tree.
> >
> > Look for driver owner, who help test this and start move forward to remove
> > cpu_addr_fixup() work.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > Frank Li (2):
> >       ARM: dts: artpec6: Move PCIe nodes under bus@c0000000
> >       PCI: artpec6: Use use_parent_dt_ranges and clean up artpec6_pcie_cpu_addr_fixup()
> >
> >  arch/arm/boot/dts/axis/artpec6.dtsi       | 92 +++++++++++++++++--------------
> >  drivers/pci/controller/dwc/pcie-artpec6.c | 20 +------
> >  2 files changed, 52 insertions(+), 60 deletions(-)
> > ---
> > base-commit: 1552be4855dacca5ea39b15b1ef0b96c91dbea0d
> > change-id: 20250304-axis-6d12970976b4
> >
> > Best regards,
> > ---
> > Frank Li <Frank.Li@nxp.com>
> 
> /^JN - Jesper Nilsson
> --
>                Jesper Nilsson -- jesper.nilsson@axis.com
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y for arch/arm/boot/dts/axis/' for Z8huvkENIBxyPKJv@axis.com:

arch/arm/boot/dts/axis/artpec6-devboard.dtb: /bus@c0000000/pcie@f8050000: failed to match any schema with compatible: ['axis,artpec6-pcie', 'snps,dw-pcie']
arch/arm/boot/dts/axis/artpec6-devboard.dtb: /bus@c0000000/pcie_ep@f8050000: failed to match any schema with compatible: ['axis,artpec6-pcie-ep', 'snps,dw-pcie']