Re: [PATCH net-next 3/4] net: enetc: add i.MX952 ENETC support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: enetc: wrong PCI bus lookup in enetc_get_phc_index_by_pdev() on i.MX952
--

commit 1b7e801c5a0f70e9aaf3045a92e91874399e99b2
Author: Clark Wang <xiaoning.wang@nxp.com>

net: enetc: add i.MX952 ENETC support

This commit adds i.MX952 (ENETC v4.6) PF and VF driver data with a 133MHz
NETC system clock to enetc_info, and updates enetc_get_phc_index_by_pdev()
to look up the PHC index for ENETC_REV_4_6.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> index 7965dfd06f5f6..3c6c6fc0ab8cf 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> @@ -1149,6 +1149,7 @@ static int enetc_get_phc_index_by_pdev(struct enetc_si *si)
>  		devfn = PCI_DEVFN(24, 0);
>  		break;
>  	case ENETC_REV_4_3:
> +	case ENETC_REV_4_6:
>  		devfn = PCI_DEVFN(0, 1);
>  		break;

[Severity: Medium]
When enetc_get_ts_info() -> enetc_get_phc_index() falls back to
enetc_get_phc_index_by_pdev() (such as on an SR-IOV VF where
si->pdev->dev.of_node is NULL, or on a PF when the optional ptp-timer
phandle is omitted from the Device Tree), it looks up the timer device on
the caller's own PCI domain and bus (si->pdev->bus):

drivers/net/ethernet/freescale/enetc/enetc_ethtool.c:enetc_get_phc_index_by_pdev() {
	struct pci_bus *bus = si->pdev->bus;
	...
	timer_pdev = pci_get_domain_bus_and_slot(pci_domain_nr(bus),
						 bus->number, devfn);
	if (!timer_pdev)
		return -1;
}

On i.MX94 (ENETC_REV_4_3), ENETC 0/1/2 reside on PCIe bus 1 alongside
Timer 1 (0x101) and ENETC 3 resides on PCIe bus 0 alongside Timer 0 (0x1),
so every ENETC has a timer at PCI_DEVFN(0, 1) on its own si->pdev->bus.

On i.MX952 (ENETC_REV_4_6), however, ENETC 0 and ENETC 1 are on different
PCIe buses:

drivers/net/ethernet/freescale/enetc/netc_blk_ctrl.c:
	#define IMX952_ENETC0_BUS_DEVFN		0x0
	#define IMX952_ENETC1_BUS_DEVFN		0x100

Since the PTP timer on i.MX952 is at function 1 of device 0 on the PCIe bus
of ENETC 0 (bus 0), won't calling enetc_get_phc_index_by_pdev() for ENETC 1
or its VFs search for PCI_DEVFN(0, 1) on ENETC 1's bus (bus 1) instead of
bus 0 and fail to find timer_pdev, causing enetc_get_ts_info() to return -1
for phc_index and fall back to software timestamping?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104092217.1730351-1-xiaoning.wang@oss.nxp.com?part=3