Re: [PATCH 7/7] net: dsa: b53: Expose PTP timestamping ioctls to userspace

On Mon, Nov 08, 2021 at 06:48:24AM -0800, Richard Cochran wrote:

On Sun, Nov 07, 2021 at 04:27:03PM +0200, Vladimir Oltean wrote:

quoted

On Sun, Nov 07, 2021 at 06:05:34AM -0800, Richard Cochran wrote:

quoted

        switch (cfg.rx_filter) {
        case HWTSTAMP_FILTER_NONE:
                break;
        case HWTSTAMP_FILTER_ALL:
        case HWTSTAMP_FILTER_SOME:
        case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
        case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
        case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
        case HWTSTAMP_FILTER_NTP_ALL:
        case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
        case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
        case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
        case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
        case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
        case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
        case HWTSTAMP_FILTER_PTP_V2_EVENT:
        case HWTSTAMP_FILTER_PTP_V2_SYNC:
        case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
                cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
                break;
        default:
                mutex_unlock(&ocelot->ptp_lock);
                return -ERANGE;
        }

That is essentially an upgrade to HWTSTAMP_FILTER_PTP_V2_EVENT.  The
change from ALL to HWTSTAMP_FILTER_PTP_V2_EVENT is probably a simple
oversight, and the driver can be easily fixed.

Thanks,
Richard

It's essentially the same pattern as what Martin is introducing for b53.

Uh, no it isn't.  The present patch has:

+       case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
+       case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
+       case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
+       case HWTSTAMP_FILTER_PTP_V2_EVENT:
+       case HWTSTAMP_FILTER_PTP_V2_SYNC:
+       case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
+       case HWTSTAMP_FILTER_ALL:
+               config->rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;

There is an important difference between
HWTSTAMP_FILTER_PTP_V2_L2_EVENT and HWTSTAMP_FILTER_PTP_V2_EVENT

Notice the "L2" in there.

Richard, when the request is PTP_V2_EVENT and the response is
PTP_V2_L2_EVENT, is that an upgrade or a downgrade? PTP_V2_EVENT also
includes PTP_V2_L4_EVENT.