Some systems have multiple watchdog devices where the first device registered is assigned to the /dev/watchdog device file. In order to test other watchdog devices, add an optional file argument for selecting non-default watchdog devices for testing.
Tested-by: Eugeniu Rosca erosca@de.adit-jv.com Signed-off-by: George G. Davis george_davis@mentor.com --- v1: - https://lkml.org/lkml/2019/8/29/16 v2: - Update printf for ENOENT case based on report from Eugeniu Rosca --- tools/testing/selftests/watchdog/watchdog-test.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/watchdog/watchdog-test.c b/tools/testing/selftests/watchdog/watchdog-test.c index c2333c78cf04..9f17cae61007 100644 --- a/tools/testing/selftests/watchdog/watchdog-test.c +++ b/tools/testing/selftests/watchdog/watchdog-test.c @@ -19,7 +19,7 @@
int fd; const char v = 'V'; -static const char sopts[] = "bdehp:t:Tn:NL"; +static const char sopts[] = "bdehp:t:Tn:NLf:"; static const struct option lopts[] = { {"bootstatus", no_argument, NULL, 'b'}, {"disable", no_argument, NULL, 'd'}, @@ -31,6 +31,7 @@ static const struct option lopts[] = { {"pretimeout", required_argument, NULL, 'n'}, {"getpretimeout", no_argument, NULL, 'N'}, {"gettimeleft", no_argument, NULL, 'L'}, + {"file", required_argument, NULL, 'f'}, {NULL, no_argument, NULL, 0x0} };
@@ -69,6 +70,7 @@ static void term(int sig) static void usage(char *progname) { printf("Usage: %s [options]\n", progname); + printf(" -f, --file Open watchdog device file (default is /dev/watchdog)\n"); printf(" -b, --bootstatus Get last boot status (Watchdog/POR)\n"); printf(" -d, --disable Turn off the watchdog timer\n"); printf(" -e, --enable Turn on the watchdog timer\n"); @@ -92,14 +94,20 @@ int main(int argc, char *argv[]) int ret; int c; int oneshot = 0; + char *file = "/dev/watchdog";
setbuf(stdout, NULL);
- fd = open("/dev/watchdog", O_WRONLY); + while ((c = getopt_long(argc, argv, sopts, lopts, NULL)) != -1) { + if (c == 'f') + file = optarg; + } + + fd = open(file, O_WRONLY);
if (fd == -1) { if (errno == ENOENT) - printf("Watchdog device not enabled.\n"); + printf("Watchdog device (%s) not found.\n", file); else if (errno == EACCES) printf("Run watchdog as root.\n"); else @@ -108,6 +116,8 @@ int main(int argc, char *argv[]) exit(-1); }
+ optind = 0; + while ((c = getopt_long(argc, argv, sopts, lopts, NULL)) != -1) { switch (c) { case 'b': @@ -190,6 +200,9 @@ int main(int argc, char *argv[]) else printf("WDIOC_GETTIMELEFT error '%s'\n", strerror(errno)); break; + case 'f': + /* Handled above */ + break;
default: usage(argv[0]);
Hi George,
On Fri, Aug 30, 2019 at 08:53:16AM -0400, George G. Davis wrote:
Some systems have multiple watchdog devices where the first device registered is assigned to the /dev/watchdog device file. In order to test other watchdog devices, add an optional file argument for selecting non-default watchdog devices for testing.
Tested-by: Eugeniu Rosca erosca@de.adit-jv.com Signed-off-by: George G. Davis george_davis@mentor.com
v1:
v2:
- Update printf for ENOENT case based on report from Eugeniu Rosca
Below interdiff [1] matches my expectations. Thanks!
Reviewed-by: Eugeniu Rosca erosca@de.adit-jv.com
[1] interdiff <(git show patch_v1) <(git show patch_v2) diff -u b/tools/testing/selftests/watchdog/watchdog-test.c b/tools/testing/selftests/watchdog/watchdog-test.c --- b/tools/testing/selftests/watchdog/watchdog-test.c +++ b/tools/testing/selftests/watchdog/watchdog-test.c @@ -107,7 +107,7 @@
if (fd == -1) { if (errno == ENOENT) - printf("Watchdog device not enabled.\n"); + printf("Watchdog device (%s) not found.\n", file); else if (errno == EACCES) printf("Run watchdog as root.\n"); else
Hello Eugeniu,
On Fri, Aug 30, 2019 at 03:38:13PM +0200, Eugeniu Rosca wrote:
Hi George,
On Fri, Aug 30, 2019 at 08:53:16AM -0400, George G. Davis wrote:
v2:
- Update printf for ENOENT case based on report from Eugeniu Rosca
Below interdiff [1] matches my expectations. Thanks!
Thanks for confirming.
On 8/30/19 6:53 AM, George G. Davis wrote:
Some systems have multiple watchdog devices where the first device registered is assigned to the /dev/watchdog device file. In order to test other watchdog devices, add an optional file argument for selecting non-default watchdog devices for testing.
Tested-by: Eugeniu Rosca erosca@de.adit-jv.com Signed-off-by: George G. Davis george_davis@mentor.com
v1:
v2:
- Update printf for ENOENT case based on report from Eugeniu Rosca
tools/testing/selftests/watchdog/watchdog-test.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/watchdog/watchdog-test.c b/tools/testing/selftests/watchdog/watchdog-test.c index c2333c78cf04..9f17cae61007 100644 --- a/tools/testing/selftests/watchdog/watchdog-test.c +++ b/tools/testing/selftests/watchdog/watchdog-test.c @@ -19,7 +19,7 @@ int fd; const char v = 'V'; -static const char sopts[] = "bdehp:t:Tn:NL"; +static const char sopts[] = "bdehp:t:Tn:NLf:"; static const struct option lopts[] = { {"bootstatus", no_argument, NULL, 'b'}, {"disable", no_argument, NULL, 'd'}, @@ -31,6 +31,7 @@ static const struct option lopts[] = { {"pretimeout", required_argument, NULL, 'n'}, {"getpretimeout", no_argument, NULL, 'N'}, {"gettimeleft", no_argument, NULL, 'L'},
- {"file", required_argument, NULL, 'f'}, {NULL, no_argument, NULL, 0x0} };
@@ -69,6 +70,7 @@ static void term(int sig) static void usage(char *progname) { printf("Usage: %s [options]\n", progname);
- printf(" -f, --file Open watchdog device file (default is /dev/watchdog)\n");
Can you split this line into two printf's. Checkpatch doesn't like it.
printf(" -f, --file Open watchdog device file\n"); A second one below for default.
On a separate note, I wish this usage block uses \t instead of spacing things out.
thanks, -- Shuah
On Fri, Aug 30, 2019 at 09:12:10AM -0600, shuah wrote:
On 8/30/19 6:53 AM, George G. Davis wrote:
diff --git a/tools/testing/selftests/watchdog/watchdog-test.c b/tools/testing/selftests/watchdog/watchdog-test.c @@ -69,6 +70,7 @@ static void term(int sig) static void usage(char *progname) { printf("Usage: %s [options]\n", progname);
- printf(" -f, --file Open watchdog device file (default is /dev/watchdog)\n");
Can you split this line into two printf's. Checkpatch doesn't like it.
printf(" -f, --file Open watchdog device file\n"); A second one below for default.
Sure, I'll add the following interdiff in v3:
diff --git a/tools/testing/selftests/watchdog/watchdog-test.c b/tools/testing/selftests/watchdog/watchdog-test.c index 9f17cae61007..6a68b486dd61 100644 --- a/tools/testing/selftests/watchdog/watchdog-test.c +++ b/tools/testing/selftests/watchdog/watchdog-test.c @@ -70,7 +70,8 @@ static void term(int sig) static void usage(char *progname) { printf("Usage: %s [options]\n", progname); - printf(" -f, --file Open watchdog device file (default is /dev/watchdog)\n"); + printf(" -f, --file Open watchdog device file\n"); + printf(" Default is /dev/watchdog\n"); printf(" -b, --bootstatus Get last boot status (Watchdog/POR)\n"); printf(" -d, --disable Turn off the watchdog timer\n"); printf(" -e, --enable Turn on the watchdog timer\n");
On a separate note, I wish this usage block uses \t instead of spacing things out.
I noticed that most of those lines are hard spaced with only one using tabs. To remain consistent with existing CodingStyle, I used hard spaces.
Thanks!
On 8/30/19 10:13 AM, George G. Davis wrote:
On Fri, Aug 30, 2019 at 09:12:10AM -0600, shuah wrote:
On 8/30/19 6:53 AM, George G. Davis wrote:
diff --git a/tools/testing/selftests/watchdog/watchdog-test.c b/tools/testing/selftests/watchdog/watchdog-test.c @@ -69,6 +70,7 @@ static void term(int sig) static void usage(char *progname) { printf("Usage: %s [options]\n", progname);
- printf(" -f, --file Open watchdog device file (default is /dev/watchdog)\n");
Can you split this line into two printf's. Checkpatch doesn't like it.
printf(" -f, --file Open watchdog device file\n"); A second one below for default.
Sure, I'll add the following interdiff in v3:
diff --git a/tools/testing/selftests/watchdog/watchdog-test.c b/tools/testing/selftests/watchdog/watchdog-test.c index 9f17cae61007..6a68b486dd61 100644 --- a/tools/testing/selftests/watchdog/watchdog-test.c +++ b/tools/testing/selftests/watchdog/watchdog-test.c @@ -70,7 +70,8 @@ static void term(int sig) static void usage(char *progname) { printf("Usage: %s [options]\n", progname);
- printf(" -f, --file Open watchdog device file (default is /dev/watchdog)\n");
- printf(" -f, --file Open watchdog device file\n");
- printf(" Default is /dev/watchdog\n");
Thanks. This is what I am looking for. Please send v3 with thsi change.
printf(" -b, --bootstatus Get last boot status (Watchdog/POR)\n"); printf(" -d, --disable Turn off the watchdog timer\n"); printf(" -e, --enable Turn on the watchdog timer\n");
On a separate note, I wish this usage block uses \t instead of spacing things out.
I noticed that most of those lines are hard spaced with only one using tabs. To remain consistent with existing CodingStyle, I used hard spaces.
yes. My comment about using "\t" can be done later and if you would like to send for that patch, I will be happy to take it.
thanks, -- Shuah
On Fri, Aug 30, 2019 at 10:20:23AM -0600, shuah wrote:
On 8/30/19 10:13 AM, George G. Davis wrote:
On Fri, Aug 30, 2019 at 09:12:10AM -0600, shuah wrote:
Can you split this line into two printf's. Checkpatch doesn't like it.
Sure, I'll add the following interdiff in v3:
Thanks. This is what I am looking for. Please send v3 with thsi change.
Done.
On a separate note, I wish this usage block uses \t instead of spacing things out.
I noticed that most of those lines are hard spaced with only one using tabs. To remain consistent with existing CodingStyle, I used hard spaces.
yes. My comment about using "\t" can be done later and if you would like to send for that patch, I will be happy to take it.
I'll send that along in a separate thread.
Thanks!
linux-kselftest-mirror@lists.linaro.org
-
Eugeniu Rosca -
George G. Davis -
shuah