Re: [PATCH net 2/3] net: enetc: fix support for PTP_EXTTS_REQUEST2 ioctl

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The concern: drivers/ptp/ptp_qoriq.c `ptp_qoriq_caps` uses the
  same QorIQ 1588 timer, the same `ptp_qoriq_enable()` and the same…

--

net: enetc: fix support for PTP_EXTTS_REQUEST2 ioctl

This patch sets .supported_extts_flags in enetc_ptp_caps to
PTP_RISING_EDGE | PTP_STRICT_FLAGS. Without it, the PTP core rejects
PTP_EXTTS_REQUEST2 requests with -EOPNOTSUPP. The QorIQ 1588 timer that
ptp_qoriq_init() sets up only timestamps the rising edge.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ptp.c b/drivers/net/ethernet/freescale/enetc/enetc_ptp.c
> index 162cbc8017304..5ffee882d6241 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_ptp.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_ptp.c
> @@ -16,6 +16,7 @@ static struct ptp_clock_info enetc_ptp_caps = {
>  	.n_per_out	= 0,
>  	.n_pins		= 0,
>  	.pps		= 1,
> +	.supported_extts_flags = PTP_RISING_EDGE | PTP_STRICT_FLAGS,

[Severity: Low]
Does ptp_qoriq_caps in drivers/ptp/ptp_qoriq.c need the same change?

It uses the same QorIQ 1588 timer, ptp_qoriq_enable() and ptp_qoriq_init()
as ENETC, but this patch leaves its .supported_extts_flags unset.
ptp_extts_request() in drivers/ptp/ptp_chardev.c always adds
PTP_STRICT_FLAGS to PTP_EXTTS_REQUEST2 requests. On eTSEC/FMan clocks
registered through ptp_qoriq_probe(), would every PTP_EXTTS_REQUEST2 still
return -EOPNOTSUPP?

    err = ptp_qoriq_init(ptp_qoriq, base, &ptp_qoriq_caps);

This looks resolved later in the same series. The earlier patch
"dpaa2-ptp: fix support for PTP_EXTTS_REQUEST2 ioctl" covers dpaa2_ptp_caps.
The later patch "ptp: qoriq: fix support for PTP_EXTTS_REQUEST2 ioctl" adds
the same line to ptp_qoriq_caps.

ptp_qoriq_init() has three callers: enetc, dpaa2-ptp and ptp_qoriq. At the
end of the series, all three caps tables declare
PTP_RISING_EDGE | PTP_STRICT_FLAGS. Mentioning that in the commit message
might be useful, since this patch only fixes ENETC.

>  	.adjfine	= ptp_qoriq_adjfine,
>  	.adjtime	= ptp_qoriq_adjtime,
>  	.gettime64	= ptp_qoriq_gettime,

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006-for-upstream-qoriq-ptp-extts-flags-v1-0-b4a161b92832%40free.fr