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

[Niklas Cassel <cassel@kernel.org>]

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