On Mon, 1 May 2023 at 18:30, Martin Pecka <pecka...@fel.cvut.cz> wrote:
>
> This code in hwts_init is wrong:
>
> cfg.flags = HWTSTAMP_FLAG_BONDED_PHC_INDEX;
> /* Fall back without flag if user run new build on old kernel */
> if (ioctl(fd, SIOCGHWTSTAMP, &ifreq) == -EINVAL)
> init_ifreq(&ifreq, &cfg, device);
>
> As `man ioctl` says:
>
> RETURN VALUE
> Usually, on success zero is returned. A few ioctl() requests use the
> return value as an output parameter and return a nonnegative value on
> success. On error, -1 is returned, and errno is set appropriately.
>
> So I think what you meant to write is this:
>
> cfg.flags = HWTSTAMP_FLAG_BONDED_PHC_INDEX;
> err = ioctl(fd, SIOCGHWTSTAMP, &ifreq);
> if (err < 0) {
> /* Fall back without flag if user run new build on old kernel */
> if (errno == EINVAL) {
> init_ifreq(&ifreq, &cfg, device);
> } else {
> pr_err("ioctl SIOCGHWTSTAMP failed: %m");
> return err;
> }
> }
>
>
> Agree. Applying this fix reveals the real error - EOPNOTSUPP.
>
> @Martin would that also fix your issue?
>
> No. Getting the real error code, I went after the source of EOPNOTSUPP,
> and...
>
> $ sudo hwstamp_ctl -i eth0
> Device driver does not have support for non-destructive SIOCGHWTSTAMP.
>
> This is it! The nvidia eqos driver does not support SIOCGHWTSTAMP (but it
> does support SIOCSHWTSTAMP). Evidence can be found in
> https://github.com/OE4T/linux-tegra-4.9/blob/oe4t-patches-l4t-r32.7.3/nvidia/drivers/net/ethernet/nvidia/eqos/drv.c#L3979
> (function eqos_ioctl).
>
> According to
> https://www.kernel.org/doc/Documentation/networking/timestamping.txt,
> section 3.1:
>
> A driver which supports hardware time stamping must support the
> SIOCSHWTSTAMP ioctl and update the supplied struct hwtstamp_config with
> the actual values as described in the section on SIOCSHWTSTAMP. It
> should also support SIOCGHWTSTAMP.
>
> The support for SIOCGHWTSTAMP is optional.
>
> So it seems to me wrong to test for the bonded PHC support using this
> ioctl, which is only optional.
>
> To add importance to this issue, I've found out it not only affects the
> pretty old Jetson TX2 I used for the test, but all Jetson models up to the
> newest AGX Orin (included). Meaning no Jetson can run ptp4l from master
> branch now. I really wonder I'm the first one complaining...
>
> Regarding possible solutions I can think of:
>
> 1. Check errno not only for EINVAL, but also for EOPNOTSUPP. This would
> solve the issue for me (and Jetsons in general), but would probably leave
> some users with bonded PHCs whose drivers do not support SIOCGHWTSTAMP
> without the possibility to use the bonded PHC.
>
On a second review.
I think you have a point.
This get ioctl (SIOCGHWTSTAMP) was added for VLAN over bond support.
On normal run, the get ioctl is only used with HWTS_FILTER_CHECK.
So the user can switch to HWTS_FILTER_FULL or HWTS_FILTER_NORMAL and skip
the get ioctl.
But the get ioctl with bonded PHC index, can not be skipped.
> 2. Find another way to test for the bonded PHC feature.
>
Perhaps a flag?
Erez
> Martin
>
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
