Re: [PATCH net-next v1 5/5] PCI: Create OF node for NXP ENETC pseudo-MAC functions

Re: [PATCH net-next v1 5/5] PCI: Create OF node for NXP ENETC pseudo-MAC functions

[netdev-bot+sashiko]

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

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

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] Overlay changeset leak when of_overlay_fdt_apply() fails in
  the pseudo-MAC probe path that this patch makes reachable.
- [Low] enetc4_link_init() assumes any OF_DYNAMIC node is 'an empty node
  created by the PCI core' by the quirk this patch adds.

--

PCI: Create OF node for NXP ENETC pseudo-MAC functions

This patch registers of_pci_make_dev_node() as a final PCI fixup for the
NXP ENETC4 device IDs 0xe101 and 0xe110. The PCI core then creates an OF
node for pseudo-MAC functions that have no static DT node. The ENETC4
driver applies a fixed-link overlay to that node.

> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index de9bbccda21fd..86f0a182224b2 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -6391,6 +6391,8 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5020, of_pci_make_dev_node);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5021, of_pci_make_dev_node);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REDHAT, 0x0005, of_pci_make_dev_node);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_EFAR, 0x9660, of_pci_make_dev_node);
> +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);

[Severity: Low]
This quirk provides the node that enetc4_link_init() in
drivers/net/ethernet/freescale/enetc/enetc4_pf.c uses. Is OF_DYNAMIC
specific enough to identify that node?

drivers/net/ethernet/freescale/enetc/enetc4_pf.c:enetc4_link_init() {
	bool dynamic = node && of_node_check_flag(node, OF_DYNAMIC);
    ...
	/* Pseudo-MAC ENETCs are described by a runtime fixed-link overlay
	 * rather than static DT. This covers both a missing OF node and a
	 * PCI-synthesized (OF_DYNAMIC) node, which is an empty node created
	 * by the PCI core and thus carries no fixed-link description.
	 */
	if (enetc_is_pseudo_mac(priv->si) && (!node || dynamic)) {
		pf->if_mode = PHY_INTERFACE_MODE_INTERNAL;

		err = enetc4_apply_overlay(priv);
    ...
}

The node from of_pci_make_dev_node() is not empty. It is filled in by
of_pci_add_properties() and is tagged through np->data:

drivers/pci/of.c:of_pci_make_dev_node() {
    ...
	ret = of_pci_add_properties(pdev, cset, np);
    ...
	np->data = cset;
    ...
}

OF_DYNAMIC is also set on any node created by a runtime overlay or
of_changeset. Suppose a pseudo-MAC function's node comes from such an
overlay and already has phy-mode or fixed-link. Then of_get_phy_mode()
is skipped, PHY_INTERFACE_MODE_INTERNAL is forced, and the driver
applies its own overlay on top. If a fixed-link child already exists,
that overlay changes the existing node instead of adding a new one.

The stacked overlay would also block removal of the user's overlay while
the driver is bound.

Would a stricter check help here? For example, the driver could check
that no phy-mode or fixed-link is present, or it could recognise the
PCI-core changeset node. Also, the "empty node" wording in the comment
does not match what of_pci_make_dev_node() creates.

[Severity: Medium]
With this quirk, pseudo-MAC functions now reach the overlay apply path
in the ENETC4 driver. Can the overlay changeset leak there if
of_overlay_fdt_apply() fails?

drivers/net/ethernet/freescale/enetc/enetc4_pf.c:enetc4_apply_overlay() {
    ...
	err = of_overlay_fdt_apply(__dtbo_enetc4_pseudo_mac_begin, size,
				   &priv->ovcs_id, np);
	if (err)
		return dev_err_probe(priv->dev, err,
				     "Failed to apply fixed-link overlay\n");
    ...
}

If of_overlay_apply() fails in drivers/of/overlay.c, the changeset is
kept on purpose. of_overlay_fdt_apply() still sets *ret_ovcs_id, and its
documentation says the caller should call of_overlay_remove() with that
id. This covers resolve or init failures, PRE_APPLY or POST_APPLY
notifier errors, and -ENOMEM from build_changeset.

None of the callers removes the changeset:

enetc4_pf_netdev_create()
  enetc4_link_init()
    enetc4_apply_overlay()   <- returns err, priv->ovcs_id still set
    return err;              <- before err_mdiobus_create, so
                                enetc4_put_overlay() is skipped
  goto err_link_init;        <- skips enetc4_link_deinit()
  free_netdev(ndev);         <- frees priv and the only copy of ovcs_id

After this, the ovcs, its IDR entry, the copied FDT, the unflattened
overlay tree and the changeset node references can no longer be freed.
In the POST_APPLY failure case, the fixed-link node also stays attached
to the PCI function's node. This repeats on every failed bind attempt.

Should the error branch in enetc4_apply_overlay() call
enetc4_put_overlay(priv) before it returns?

>  
>  /*
>   * Devices known to require a longer delay before first config space access

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

[PATCH net-next v1 0/5] net: enetc: S32N79 pseudo-MAC (PPM) networking support

[Claudiu Manoil]

This series adds networking support for the ENETCv4.4 pseudo-MAC (PPM)
ports found on the NXP S32N79 SoC. On S32N79, the ENETCv4 ports are
not wired to any external PHY/MII; instead they are internal "pseudo-MAC"
links into the on-chip NETC switch. These links are proprietary, expose
no standard IEEE interface, and run at a speed fixed at boot time by the
Port PCR[PSPEED] register.

A key use case is a port that has no "ethernet" DT node at all: NETC is
reached over PCI from a host that probes the ENETC, while the internal
NETC switch it connects to is not owned by Linux. Such a port must have
its fixed link synthesized at probe time.

Rather than hand-building a software node, the driver describes the
fixed link with a self-contained DT overlay (following the lan966x PCI
approach), applied unmodified via of_overlay_fdt_apply() onto the PCI
function's dynamic OF node (CONFIG_PCI_DYNAMIC_OF_NODES). phylink then
picks up the fixed-link node through dev_fwnode(); the real operating
speed is sourced live from PCR[PSPEED].

The series touches three subsystems:
  - net/enetc:  pseudo-MAC overlay support + probing of 4.4 devices
  - arm64 dts:  NETC IEP18 ECAM node on S32N79 and its RDB board enable
  - PCI:        a quirk creating the OF node for the ENETC PMAC functions

Because this is primarily a networking feature and the pieces are
interdependent at runtime, it would be most convenient to merge the
whole series through the netdev tree.

Patch 1 has a runtime (not Kconfig) dependency on
CONFIG_PCI_DYNAMIC_OF_NODES; absent that, the pseudo-MAC probe path
fails gracefully with -ENODEV.

v1: initial submission.

Claudiu Manoil (5):
  net: enetc: Add pseudo-MAC support for ENETCv4 Ports via a DT overlay
  net: enetc: Enable probing of version 4.4 devices
  arm64: dts: s32n79: add NETC IEP18 ECAM node for ENETC PPM ports
  arm64: dts: s32n79-rdb: enable NETC IEP18
  PCI: Create OF node for NXP ENETC pseudo-MAC functions

 arch/arm64/boot/dts/freescale/s32n79-rdb.dts  |  4 +
 arch/arm64/boot/dts/freescale/s32n79.dtsi     | 24 +++++
 drivers/net/ethernet/freescale/enetc/Kconfig  |  1 +
 drivers/net/ethernet/freescale/enetc/Makefile |  1 +
 drivers/net/ethernet/freescale/enetc/enetc.c  | 12 +++
 drivers/net/ethernet/freescale/enetc/enetc.h  |  1 +
 .../net/ethernet/freescale/enetc/enetc4_pf.c  | 98 +++++++++++++++++--
 .../freescale/enetc/enetc4_pseudo_mac.dtso    | 32 ++++++
 .../net/ethernet/freescale/enetc/enetc_hw.h   |  1 +
 .../freescale/enetc/enetc_pf_common.c         | 41 +++++++-
 .../freescale/enetc/enetc_pf_common.h         |  1 +
 drivers/pci/quirks.c                          |  2 +
 12 files changed, 209 insertions(+), 9 deletions(-)
 create mode 100644 drivers/net/ethernet/freescale/enetc/enetc4_pseudo_mac.dtso


base-commit: d8674294aefef02266c4d47ad10131f1bffbe534
-- 
2.34.1

[PATCH net-next v1 5/5] PCI: Create OF node for NXP ENETC pseudo-MAC functions

[Claudiu Manoil]

NXP ENETCv4 pseudo-MAC (PPM) ports can be probed as pure PCI endpoints,
without any "ethernet" node in the static device tree. The ENETC4 driver
describes their fixed link by applying a device-tree overlay onto the PCI
function's own OF node, which requires that node to exist.

Request the PCI core to synthesize a per-function OF node for these
devices by registering of_pci_make_dev_node() as a final fixup, following
the existing precedent for the Xilinx, Red Hat and EFAR devices. The
helper is a no-op when the function already has a static OF node, so this
has no effect on platforms that describe ENETC via static DT. It only
takes effect when CONFIG_PCI_DYNAMIC_OF_NODES is enabled.

Both ENETC4 PCI device IDs are covered (0xe101 and 0xe110), since either
can be instantiated as a pseudo-MAC port on S32N79.

Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
 drivers/pci/quirks.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index de9bbccda21f..86f0a182224b 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -6391,6 +6391,8 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5020, of_pci_make_dev_node);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5021, of_pci_make_dev_node);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REDHAT, 0x0005, of_pci_make_dev_node);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_EFAR, 0x9660, of_pci_make_dev_node);
+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);
 
 /*
  * Devices known to require a longer delay before first config space access
-- 
2.34.1

Re: [PATCH net-next v1 5/5] PCI: Create OF node for NXP ENETC pseudo-MAC functions

[Bjorn Helgaas]

On Fri, Oct 09, 2026 at 03:40:34PM +0300, Claudiu Manoil wrote:
> NXP ENETCv4 pseudo-MAC (PPM) ports can be probed as pure PCI endpoints,
> without any "ethernet" node in the static device tree. The ENETC4 driver
> describes their fixed link by applying a device-tree overlay onto the PCI
> function's own OF node, which requires that node to exist.
> 
> Request the PCI core to synthesize a per-function OF node for these
> devices by registering of_pci_make_dev_node() as a final fixup, following
> the existing precedent for the Xilinx, Red Hat and EFAR devices. The
> helper is a no-op when the function already has a static OF node, so this
> has no effect on platforms that describe ENETC via static DT. It only
> takes effect when CONFIG_PCI_DYNAMIC_OF_NODES is enabled.
> 
> Both ENETC4 PCI device IDs are covered (0xe101 and 0xe110), since either
> can be instantiated as a pseudo-MAC port on S32N79.
> 
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/pci/quirks.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index de9bbccda21f..86f0a182224b 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -6391,6 +6391,8 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5020, of_pci_make_dev_node);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5021, of_pci_make_dev_node);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REDHAT, 0x0005, of_pci_make_dev_node);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_EFAR, 0x9660, of_pci_make_dev_node);
> +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);
>  
>  /*
>   * Devices known to require a longer delay before first config space access
> -- 
> 2.34.1
>