RE: [PATCH net 1/1] net: fec: fix the PTP periodic output sysfs interface
From: Buday Csaba <hidden>
Date: 2026-03-19 08:04:40
Also in:
imx, lkml
quoted hunk ↗ jump to hunk
quoted
Fixes: bf8ca67e2167 ("net: fec: refactor PPS channel configuration") Signed-off-by: Buday Csaba <redacted> --- drivers/net/ethernet/freescale/fec_ptp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/drivers/net/ethernet/freescale/fec_ptp.cb/drivers/net/ethernet/freescale/fec_ptp.c index 7f6b574320716..c1af81002b8fa 100644--- a/drivers/net/ethernet/freescale/fec_ptp.c +++ b/drivers/net/ethernet/freescale/fec_ptp.c@@ -534,7 +534,7 @@ static int fec_ptp_enable(struct ptp_clock_info *ptp, if (rq->perout.flags) return -EOPNOTSUPP; - if (rq->perout.index != fep->pps_channel) + if (rq->perout.index != 0) return -EOPNOTSUPP; period.tv_sec = rq->perout.period.sec; --2.39.5I think the correct fix should update the n_per_out value like below.@@ -756,7 +756,7 @@ void fec_ptp_init(struct platform_device *pdev, int irq_idx) fep->ptp_caps.max_adj = 250000000; fep->ptp_caps.n_alarm = 0; fep->ptp_caps.n_ext_ts = 0; - fep->ptp_caps.n_per_out = 1; + fep->ptp_caps.n_per_out = 4;The commit 566c2d83887f ("net: fec: make PPS channel configurable") added the property "fsl,pps-channel" to indicate which pulse channel is available. So we should not fix the pulse output channel to 0.
But is it wise to expose four channels, when only one is usable by this driver? This patch does not affect the channel selection. That can be still made through the DT. The driver uses `fep->pps_channel` everywhere else, while rq->perout.index is only checked in this single line, and never used anywhere else. So the question is whether userspace is supposed to reference the hardware channel number directly, or the mapping is meant to be left to the devicetree and the driver internally. I hope the netdev maintainers and/or Richard Cochran can clarify the intended purpose of `n_per_out` and the expected channel indexing.