On Mon, Jun 25, 2018 at 12:27:18AM +0200, Erez Geva wrote:
> Add global option for the hardware time stamp setting.
> The function could:
> Normally set the filters as the PTP daemon require.
> Check that the filters are proper but do not change them.
> Full, set the RX filter to all and the TX filter as the PTP daemon require.
This patch now applies cleanly. Comments below...
> @@ -51,16 +52,39 @@ static int hwts_init(int fd, const char *device, int
> rx_filter, int tx_type)
>
> memset(&ifreq, 0, sizeof(ifreq));
> memset(&cfg, 0, sizeof(cfg));
> + memset(&req, 0, sizeof(req));
>
> strncpy(ifreq.ifr_name, device, sizeof(ifreq.ifr_name) - 1);
>
> ifreq.ifr_data = (void *) &cfg;
> - cfg.tx_type = tx_type;
> - cfg.rx_filter = rx_filter;
> - req = cfg;
> - err = ioctl(fd, SIOCSHWTSTAMP, &ifreq);
> - if (err < 0)
> - return err;
> +
> + req.tx_type = tx_type;
> + req.rx_filter = rx_filter;
> +
> + switch (sk_hwts_filter_mode) {
> + case HWTS_FILTER_CHECK:
> + err = ioctl(fd, SIOCGHWTSTAMP, &ifreq);
> + if (err < 0) {
> + pr_err("ioctl SIOCGHWTSTAMP failed: %m");
> + return err;
> + }
> + break;
> + case HWTS_FILTER_FULL:
> + cfg.tx_type = tx_type;
> + cfg.rx_filter = HWTSTAMP_FILTER_ALL;
> + err = ioctl(fd, SIOCSHWTSTAMP, &ifreq);
> + if (err < 0)
> + return err;
> + break;
> + default:
We don't want `default` here. When doing a `switch` over an enum, gcc
will nicely warn if there are unhandled cases, but only if there is no
default case. This is super useful when adding new enum values later on.
> + case HWTS_FILTER_NORMAL:
> + cfg.tx_type = tx_type;
> + cfg.rx_filter = rx_filter;
> + err = ioctl(fd, SIOCSHWTSTAMP, &ifreq);
> + if (err < 0)
> + return err;
> + break;
> + }
Consider the calling function, sk_timestamping_init(). It first tries
the more general Rx filter, then the specific one. But the second try
doesn't make sense with HWTS_FILTER_FULL. How about this?
err = hwts_init(fd, device, filter1, tx_type);
if (err && sk_hwts_filter_mode != HWTS_FILTER_FULL) {
pr_info("driver rejected most general HWTSTAMP filter");
err = hwts_init(fd, device, filter2, tx_type);
}
if (err) {
pr_err("ioctl SIOCSHWTSTAMP failed: %m");
return err;
}
Thanks,
Richard
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
