[PATCH net-next v2 4/5] arm64: dts: s32n79-rdb: enable NETC IEP18

[PATCH net-next v2 4/5] arm64: dts: s32n79-rdb: enable NETC IEP18

[Claudiu Manoil]

Enable the NETC v4 ECAM bus #18 on the S32N79-RDB board so that the
ENETC v4 Pseudo-MAC port (PPM) PF and VF instances are probed.

Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v2: none

 arch/arm64/boot/dts/freescale/s32n79-rdb.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/s32n79-rdb.dts b/arch/arm64/boot/dts/freescale/s32n79-rdb.dts
index 1feccd61258e..897ba24ac03a 100644
--- a/arch/arm64/boot/dts/freescale/s32n79-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/s32n79-rdb.dts
@@ -68,3 +68,7 @@ &usdhc0 {
 	no-sdio;
 	status = "okay";
 };
+
+&netc_iep18 {
+	status = "okay";
+};
-- 
2.34.1

Re: [PATCH net-next v2 4/5] arm64: dts: s32n79-rdb: enable NETC IEP18

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] s32n79-rdb.dts enables &netc_iep18 but leaves &smmu disabled,
  even though the bus node says `iommu-map = <0x0 &smmu 0xc0000 0x100>`.
- [Low] At this commit, enabling &netc_iep18 in s32n79-rdb.dts does not
  get the PPM PF to probe.
- [Low] The &netc_iep18 override is appended after &usdhc0 in
  s32n79-rdb.dts.

--

arm64: dts: s32n79-rdb: enable NETC IEP18

This patch enables the NETC v4 ECAM bus #18 (netc_iep18) on the S32N79-RDB
board. The goal is to get the ENETC v4 Pseudo-MAC port (PPM) PF and VF
instances probed.

> diff --git a/arch/arm64/boot/dts/freescale/s32n79-rdb.dts b/arch/arm64/boot/dts/freescale/s32n79-rdb.dts
> index 1feccd61258ef..897ba24ac03ae 100644
> --- a/arch/arm64/boot/dts/freescale/s32n79-rdb.dts
> +++ b/arch/arm64/boot/dts/freescale/s32n79-rdb.dts
> @@ -68,3 +68,7 @@ &usdhc0 {
>  	no-sdio;
>  	status = "okay";
>  };
> +
> +&netc_iep18 {
> +	status = "okay";
> +};

[Severity: Medium]
Should &smmu also be enabled for this board? The netc_iep18 node in
s32n79.dtsi has:

	iommu-map = <0x0 &smmu 0xc0000 0x100>;

The SMMU node is disabled at SoC level, and s32n79-rdb.dts does not
override it:

	smmu: iommu@4fc00000 {
		...
		#iommu-cells = <1>;
		dma-coherent;
		status = "disabled";
	};

DMA setup for the ENETC PF/VFs on this bus goes through
of_dma_configure_id()->of_iommu_configure()->of_map_iommu_id()->
of_iommu_xlate(). of_iommu_xlate() rejects the disabled node:

	if (!of_device_is_available(iommu_spec->np))
		return -ENODEV;

of_dma_configure_id() then falls back to direct DMA and only logs a
dev_dbg() message:

	/* Take all other IOMMU errors to mean we'll just carry on without it */
	dev_dbg(dev, "device is%sbehind an iommu\n",
		!ret ? " " : " not ");

	arch_setup_dma_ops(dev, coherent);

Does this leave every NETC function, including the SR-IOV VFs, doing
untranslated DMA with no iommu_group? If so, the VFs cannot be assigned
through VFIO. Whether DMA works at all would then depend on whether firmware
left the SMMU in bypass or abort mode.

This also seems to contradict the earlier dtsi commit "arm64: dts: s32n79:
add NETC IEP18 ECAM node for ENETC PPM ports", which says:

  DMA is translated by the SMMUv3 (iommu-map)

The last commit in the series does not change this.

[Severity: Low]
Will the PPM PF actually probe at this point in the series? The commit
message says:

  so that the ENETC v4 Pseudo-MAC port (PPM) PF and VF instances are probed.

The ENETC PFs are endpoints on root bus 0 of a pci-host-ecam-generic node
that has no child nodes, so pci_set_of_node() gives them no of_node.

At this commit, pci_bus_add_device() only creates dynamic nodes for
bridges:

	pci_fixup_device(pci_fixup_final, dev);
	if (pci_is_bridge(dev))
		of_pci_make_dev_node(dev);

drivers/pci/quirks.c has no NXP ENETC fixup yet, so
enetc4_link_init()->enetc4_add_ppm_fixed_link() would fail here:

	if (!np)
		return dev_err_probe(priv->dev, -ENODEV,
				     "Missing of_node for Pseudo-MAC port\n");

The last patch in the series, "PCI: Create OF node for NXP ENETC
pseudo-MAC functions", fixes this by adding:

	DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_PHILIPS, 0xe101, of_pci_make_dev_node);
	DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_PHILIPS, 0xe110, of_pci_make_dev_node);

Without that patch applied first, there is a bisection window where the PF
fails to probe.

One gap remains at the end of the series. CONFIG_PCI_DYNAMIC_OF_NODES
(bool, default n) is not set in arch/arm64/configs/defconfig, which has:

	CONFIG_NXP_ENETC4=m
	CONFIG_FSL_ENETC_VF=m

With defconfig, the PF would still fail with -ENODEV. The commit "net:
enetc: Add pseudo-MAC support for ENETCv4 Ports via a fixed-link node"
documents this as an intended runtime dependency rather than a Kconfig
dependency.

[Severity: Low]
This isn't a bug, but should this override go between &irqsteer_coss and
&uart0? Documentation/devicetree/bindings/dts-coding-style.rst says:

  3. When extending nodes in the board DTS via &label, the entries shall be
     ordered either alpha-numerically or by keeping the order from DTSI, where
     the choice depends on the subarchitecture.

The existing overrides in s32n79-rdb.dts are in alphabetical order:
irqsteer_coss, uart0, uart5, uart6, uart7, usdhc0.

Placing &netc_iep18 after &usdhc0 matches neither alphabetical order nor
DTSI order, since netc_iep18 also comes before usdhc0 in s32n79.dtsi.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1791908694.git.claudiu.manoil%40nxp.com