On 9/16/19 7:57 AM, Eugeniu Rosca wrote:
Hi Shuah,
CC George
On Mon, Sep 16, 2019 at 07:26:41AM -0600, shuah wrote:
[..]
case 'f':
/* Handled above */
break;
+ case 'i':
+ /*
+ * watchdog_info was obtained as part of file open
+ * validation. So we just show it here.
+ */
+ oneshot = 1;
+ printf("watchdog_info:\n");
+ printf(" identity:\t\t%s\n", info.identity);
+ printf(" firmware_version:\t%u\n",
+ info.firmware_version);
+ printf(" options:\t\t%08x\n", info.options);
+ break;
default:
usage(argv[0]);
I would like to see these combined. Please don't add another argument.
Combine patch and 1&2.
With all my appreciation for your comment, why do you think it is better
to get rid of the new argument? I don't think it is user-friendly to
always report the watchdog_info to the user. Just look at outputs [1-2]
and imagine that the watchdog_info part would pop up unconditionally.
It looks too busy to me.
Yes it does look busy. I am okay with adding a second options
based on what you both said.
I don't like the commit log.
Unfortunately this thread no longer contains the commit log.
I would like to see the commit log without any references to side
effects. It make it rather confusing.
"A side of affect of commit "selftests: watchdog: Add optional file
argument" is that arbitrary files may be opened for watchdog testing, e.g.
/dev/null. To prevent watchdog-test from operating on non-watchdog device
files, commit "selftests: watchdog: Validate optional file argument" was
added to validate that a file is indeed a watchdog device via an
ioctl(WDIOC_GETSUPPORT) call. Since the watchdog_info is available as a
result of the ioctl(WDIOC_GETSUPPORT) call, add a command line option to
show the watchdog_info."
I would drop all references to that.
thanks,
-- Shuah
