Looks good. I have a few nits to pick...
On Tue, May 27, 2014 at 10:34:36AM -0700, Jacob Keller wrote:
...
> diff --git a/hwstamp_ctl.8 b/hwstamp_ctl.8
> index cece74f61866..7bfdf4451839 100644
> --- a/hwstamp_ctl.8
> +++ b/hwstamp_ctl.8
> @@ -15,11 +15,10 @@ hwstamp_ctl \- set time stamping policy at the driver
> level
>
> .SH DESCRIPTION
> .B hwstamp_ctl
> -is a program used to set the hardware time stamping policy at the network
> +is a program used to set and get the hardware time stamping policy at the
> network
> driver level with the
> .B SIOCSHWTSTAMP
> .BR ioctl (2).
> -The
I think this sentence is more readable with the "The" at the beginning.
> .I tx-type
> and
> .I rx-filter
...
> @@ -138,15 +141,43 @@ int main(int argc, char *argv[])
> return -1;
> }
>
> - err = ioctl(fd, SIOCSHWTSTAMP, &ifreq);
> + /* First, attempt to get the current settings. */
> + err = ioctl(fd, SIOCGHWTSTAMP, &ifreq);
> if (err < 0) {
> err = errno;
> - perror("SIOCSHWTSTAMP failed");
> - if (err == ERANGE)
> - fprintf(stderr, "The requested time stamping mode is
> not supported by the hardware.\n");
> + if (err == ENOTTY)
> + fprintf(stderr, "Kernel does not have support for
> non-destructive for SIOCGHWTSTAMP.\n");
One "for" too many?
Also the lines with the three error messages are really very long.
> + else if (err == EOPNOTSUPP)
> + fprintf(stderr, "Device driver does not have support
> for non-destructive SIOCGHWTSTAMP.\n");
> + else
> + perror("SIOCGHWTSTAMP failed");
> + } else {
> + printf("current settings:\n" "tx_type %d\n" "rx_filter %d\n",
> + cfg.tx_type, cfg.rx_filter);
> }
>
> - printf("tx_type %d\n" "rx_filter %d\n", cfg.tx_type, cfg.rx_filter);
> + /* Now, attempt to set values. Only change the values actually
> + * requested by user, rather than blindly resetting th zero if
> + * unrequested. */
> + if (settx || setrx) {
> +
> + if (settx)
> + cfg.tx_type = txopt;
> +
> + if (setrx)
> + cfg.rx_filter = rxopt;
> +
> + err = ioctl(fd, SIOCSHWTSTAMP, &ifreq);
> + if (err < 0) {
> + err = errno;
> + perror("SIOCSHWTSTAMP failed");
> + if (err == ERANGE)
> + fprintf(stderr, "The requested time stamping
> mode is not supported by the hardware.\n");
Very long.
> + }
> +
> + printf("new settings:\n" "tx_type %d\n" "rx_filter %d\n",
> cfg.tx_type, cfg.rx_filter);
Also too long.
Thanks,
Richard
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/NeoTech
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
