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 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>