| Message ID | 20250304-axis-v1-0-ed475ab3a3ed@nxp.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | PCI: artpec6: Try to clean up artpec6_pcie_cpu_addr_fixup() | expand |
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
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
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
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
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
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']
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>