Re: [PATCH net-next v1 2/5] net: enetc: Enable probing of version 4.4 devices

Re: [PATCH net-next v1 2/5] net: enetc: Enable probing of version 4.4 devices

[netdev-bot+sashiko]

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

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

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] The patch "net: enetc: Enable probing of version 4.4 devices"
  adds ENETC_REV_4_4 entries to enetc_info[] in…

--

net: enetc: Enable probing of version 4.4 devices

This commit adds ENETC_REV_4_4 and new enetc_info[] entries so that the
v4.4 ENETC devices on S32N79 can probe. The PFs (0xe101 and 0xe110) use
enetc4_ppm_data and the VFs (0xef00) use enetc4_vf_data.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index c6571b5cc8425..be7795d4e0c66 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -3862,6 +3862,18 @@ static const struct enetc_platform_info enetc_info[] = {

[ ... ]

> +	{ .revision = ENETC_REV_4_4,
> +	  .dev_id = NXP_ENETC_VF_DEV_ID,
> +	  .data = &enetc4_vf_data,
> +	},
>  };

[Severity: Medium]
With v4.4 VFs now probing, does enetc_get_phc_index_by_pdev() in
enetc_ethtool.c also need an ENETC_REV_4_4 case? Right now its switch
only covers the older revisions:

drivers/net/ethernet/freescale/enetc/enetc_ethtool.c:enetc_get_phc_index_by_pdev() {
    ...
	switch (si->revision) {
	case ENETC_REV_1_0:
		devfn = PCI_DEVFN(0, 4);
		break;
	case ENETC_REV_4_1:
		devfn = PCI_DEVFN(24, 0);
		break;
	case ENETC_REV_4_3:
		devfn = PCI_DEVFN(0, 1);
		break;
	default:
		return -1;
	}
    ...
}

enetc4_vf_data uses enetc_vf_ethtool_ops, which has .get_ts_info set to
enetc_get_ts_info(). On S32N79, enetc_vf_get_revision() sets
si->revision to 0x0404. enetc_ptp_clock_is_enabled() only looks at
CONFIG_PTP_NETC_V4_TIMER for non-rev1 parts, so the VF continues into
the PHC lookup:

enetc_get_ts_info()
  enetc_get_phc_index()
    enetc_get_phc_index_by_pdev()   <- no OF node or no "ptp-timer" phandle
      default: return -1

The netc_iep18 node in s32n79.dtsi has no child nodes, so the VF cannot
get a ptp-timer phandle from DT. enetc_get_ts_info() then jumps to
timestamp_tx_sw and reports only SOF_TIMESTAMPING_TX_SOFTWARE with no
PHC.

enetc_hwtstamp_set() still accepts HWTSTAMP_TX_ON on the same VF,
because it only checks enetc_ptp_clock_is_enabled(). So what ethtool -T
reports and what ndo_hwtstamp_set accepts would not agree.

When v4.3 was enabled, a devfn case was added to this switch alongside
it. Is there a NETC timer function (0x1131:0xee02) on the S32N79 IEP
bus that should be listed here? If not, would it make sense to note
that in the commit message, and to also keep enetc_hwtstamp_set() from
accepting hardware timestamping on these VFs?

The v4.4 PFs don't seem to be affected, because enetc4_ppm_ethtool_ops
has no .get_ts_info callback.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> index 2c9d9042eb0bb..64789bd0f33ed 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h

[ ... ]

-- 
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 2/5] net: enetc: Enable probing of version 4.4 devices

[Claudiu Manoil]

The ENETC v4.4 devices are present on S32N79 SoCs and feature only
Pseudo-MAC ENETC ports, whose PFs expose both the 0xe101 and 0xe110
PCI device IDs, while the VFs answer to the usual device ID of 0xef00.

Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c    | 12 ++++++++++++
 drivers/net/ethernet/freescale/enetc/enetc_hw.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index c6571b5cc842..be7795d4e0c6 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -3862,6 +3862,18 @@ static const struct enetc_platform_info enetc_info[] = {
 	  .dev_id = NXP_ENETC_VF_DEV_ID,
 	  .data = &enetc4_vf_data,
 	},
+	{ .revision = ENETC_REV_4_4,
+	  .dev_id = NXP_ENETC_PPM_DEV_ID,
+	  .data = &enetc4_ppm_data,
+	},
+	{ .revision = ENETC_REV_4_4,
+	  .dev_id = NXP_ENETC_PF_DEV_ID,
+	  .data = &enetc4_ppm_data,
+	},
+	{ .revision = ENETC_REV_4_4,
+	  .dev_id = NXP_ENETC_VF_DEV_ID,
+	  .data = &enetc4_vf_data,
+	},
 };
 
 int enetc_get_driver_data(struct enetc_si *si)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
index 2c9d9042eb0b..64789bd0f33e 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
@@ -414,6 +414,7 @@ enum enetc_bdr_type {TX, RX};
 #define ENETC_REV_1_0		0x0100
 #define ENETC_REV_4_1		0X0401
 #define ENETC_REV_4_3		0x0403
+#define ENETC_REV_4_4		0x0404
 
 #define ENETC_G_EIPBRR1		0x0bfc
 #define ENETC_G_EPFBLPR(n)	(0xd00 + 4 * (n))
-- 
2.34.1

RE: [PATCH net-next v1 2/5] net: enetc: Enable probing of version 4.4 devices

[Wei Fang]

> The ENETC v4.4 devices are present on S32N79 SoCs and feature only
> Pseudo-MAC ENETC ports, whose PFs expose both the 0xe101 and 0xe110

I think the device ID of the ENETC PF with the pseudo MAC should be 0xe110.
The device ID 0xe101 should be only used for the standalone ENETCs (without
pseudo MAC). Is this a hardware issue? If so, a workaround is to set these PF's
device ID to 0xe110 in IERB module. Then the device ID 0xe110 will be present
in the PCIe ECAM space.

> PCI device IDs, while the VFs answer to the usual device ID of 0xef00.
> 
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>