Re: [PATCH net] net: ethernet: ti: am65-cpts: fix support for PTP_EXTTS_REQUEST2 ioctl

[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] Adding PTP_STRICT_FLAGS to am65_ptp_info.supported_extts_flags
  in drivers/net/ethernet/ti/am65-cpts.c also changes the legacy…

--

net: ethernet: ti: am65-cpts: fix support for PTP_EXTTS_REQUEST2 ioctl

This patch sets .supported_extts_flags for am65-cpts to
PTP_RISING_EDGE | PTP_STRICT_FLAGS. The PTP core then stops rejecting every
PTP_EXTTS_REQUEST2 for this driver. After the change, PTP_EXTTS_REQUEST2
succeeds for the rising edge and fails with -EOPNOTSUPP for the falling
edge or for both edges.

> diff --git a/drivers/net/ethernet/ti/am65-cpts.c b/drivers/net/ethernet/ti/am65-cpts.c
> index efbf2ef976a3..df5659ead72f 100644
> --- a/drivers/net/ethernet/ti/am65-cpts.c
> +++ b/drivers/net/ethernet/ti/am65-cpts.c
> @@ -779,6 +779,7 @@ static long am65_cpts_ts_work(struct ptp_clock_info *ptp);
>  static struct ptp_clock_info am65_ptp_info = {
>  	.owner		= THIS_MODULE,
>  	.name		= "CTPS timer",
> +	.supported_extts_flags = PTP_RISING_EDGE | PTP_STRICT_FLAGS,

[Severity: Medium]
Does this also change what the legacy PTP_EXTTS_REQUEST ioctl accepts?

In ptp_extts_request(), the v1 branch keeps the edge bits and never sets
PTP_STRICT_FLAGS. Whether the edge bits count only as hints depends just on
the driver's mask:

drivers/ptp/ptp_chardev.c:ptp_extts_request() {
    ...
	} else {
		req.extts.flags &= PTP_EXTTS_V1_VALID_FLAGS;
		memset(req.extts.rsv, 0, sizeof(req.extts.rsv));
	}
    ...
	if (!(supported_extts_flags & PTP_STRICT_FLAGS))
		supported_extts_flags |= PTP_EXTTS_EDGES;
	/* Reject unsupported flags */
	if (req.extts.flags & ~supported_extts_flags)
		return -EOPNOTSUPP;
    ...
}

Before this patch, supported_extts_flags was 0 for am65-cpts. A v1 request
with PTP_ENABLE_FEATURE | PTP_FALLING_EDGE, or with both edges set, was
accepted and reached am65_cpts_extts_enable(), which ignores the flags.

Now the mask has PTP_STRICT_FLAGS but not PTP_FALLING_EDGE, so those same
v1 requests return -EOPNOTSUPP before ops->enable() is ever called.

The same check runs when PTP_ENABLE_FEATURE is clear. Through either ioctl,
a disable request that still carries PTP_FALLING_EDGE is now rejected.

One in-tree example is tools/testing/selftests/ptp/testptp.c run with
-e N -E 2 or -E 3. That sends a legacy request with PTP_FALLING_EDGE set:

			extts_request.flags = PTP_ENABLE_FEATURE | edge;
			if (ioctl(fd, PTP_EXTTS_REQUEST, &extts_request)) {
				perror("PTP_EXTTS_REQUEST");

On am65-cpts this worked before the patch. It now fails with
"PTP_EXTTS_REQUEST: Operation not supported".

Rejecting falling edge requests that the hardware can't honour matches how
the core treats other strict, rising-only drivers. However, the commit
message says:

    Only the PTP_EXTTS_REQUEST ioctl still works.

This suggests v1 behaviour does not change. The patch has a Fixes: tag and
is likely to go to stable. Could the commit message say that v1 requests
carrying PTP_FALLING_EDGE are now rejected as well?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261020003525.1237742-1-jjc%40jclark.com