Linux-kselftest Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] selftests: watchdog: Add optional file argument
@ 2019-08-30 12:53 George G. Davis
  2019-08-30 13:38 ` Eugeniu Rosca
  2019-08-30 15:12 ` shuah
  0 siblings, 2 replies; 7+ messages in thread
From: George G. Davis @ 2019-08-30 12:53 UTC (permalink / raw)
  To: Shuah Khan, Jerry Hoemann, Colin Ian King, George G. Davis,
	open list:KERNEL SELFTEST FRAMEWORK, open list
  Cc: Eugeniu Rosca

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]);
-- 
2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] selftests: watchdog: Add optional file argument
  2019-08-30 12:53 [PATCH v2] selftests: watchdog: Add optional file argument George G. Davis
@ 2019-08-30 13:38 ` Eugeniu Rosca
  2019-08-30 14:26   ` George G. Davis
  2019-08-30 15:12 ` shuah
  1 sibling, 1 reply; 7+ messages in thread
From: Eugeniu Rosca @ 2019-08-30 13:38 UTC (permalink / raw)
  To: George G. Davis
  Cc: Shuah Khan, Jerry Hoemann, Colin Ian King,
	open list:KERNEL SELFTEST FRAMEWORK, open list, Eugeniu Rosca,
	Eugeniu Rosca

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:
> - https://lkml.org/lkml/2019/8/29/16
> 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

-- 
Best Regards,
Eugeniu.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] selftests: watchdog: Add optional file argument
  2019-08-30 13:38 ` Eugeniu Rosca
@ 2019-08-30 14:26   ` George G. Davis
  0 siblings, 0 replies; 7+ messages in thread
From: George G. Davis @ 2019-08-30 14:26 UTC (permalink / raw)
  To: Eugeniu Rosca
  Cc: Shuah Khan, Jerry Hoemann, Colin Ian King,
	open list:KERNEL SELFTEST FRAMEWORK, open list, Eugeniu Rosca

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.

-- 
Regards,
George

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] selftests: watchdog: Add optional file argument
  2019-08-30 12:53 [PATCH v2] selftests: watchdog: Add optional file argument George G. Davis
  2019-08-30 13:38 ` Eugeniu Rosca
@ 2019-08-30 15:12 ` shuah
  2019-08-30 16:13   ` George G. Davis
  1 sibling, 1 reply; 7+ messages in thread
From: shuah @ 2019-08-30 15:12 UTC (permalink / raw)
  To: George G. Davis, Jerry Hoemann, Colin Ian King,
	open list:KERNEL SELFTEST FRAMEWORK, open list
  Cc: Eugeniu Rosca, shuah

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:
> - 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");

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] selftests: watchdog: Add optional file argument
  2019-08-30 15:12 ` shuah
@ 2019-08-30 16:13   ` George G. Davis
  2019-08-30 16:20     ` shuah
  0 siblings, 1 reply; 7+ messages in thread
From: George G. Davis @ 2019-08-30 16:13 UTC (permalink / raw)
  To: shuah
  Cc: Jerry Hoemann, Colin Ian King,
	open list:KERNEL SELFTEST FRAMEWORK, open list, Eugeniu Rosca

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!

-- 
Regards,
George

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] selftests: watchdog: Add optional file argument
  2019-08-30 16:13   ` George G. Davis
@ 2019-08-30 16:20     ` shuah
  2019-08-30 19:36       ` George G. Davis
  0 siblings, 1 reply; 7+ messages in thread
From: shuah @ 2019-08-30 16:20 UTC (permalink / raw)
  To: George G. Davis
  Cc: Jerry Hoemann, Colin Ian King,
	open list:KERNEL SELFTEST FRAMEWORK, open list, Eugeniu Rosca,
	shuah

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] selftests: watchdog: Add optional file argument
  2019-08-30 16:20     ` shuah
@ 2019-08-30 19:36       ` George G. Davis
  0 siblings, 0 replies; 7+ messages in thread
From: George G. Davis @ 2019-08-30 19:36 UTC (permalink / raw)
  To: shuah
  Cc: Jerry Hoemann, Colin Ian King,
	open list:KERNEL SELFTEST FRAMEWORK, open list, Eugeniu Rosca

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!

-- 
Regards,
George

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-30 12:53 [PATCH v2] selftests: watchdog: Add optional file argument George G. Davis
2019-08-30 13:38 ` Eugeniu Rosca
2019-08-30 14:26   ` George G. Davis
2019-08-30 15:12 ` shuah
2019-08-30 16:13   ` George G. Davis
2019-08-30 16:20     ` shuah
2019-08-30 19:36       ` George G. Davis

Linux-kselftest Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-kselftest/0 linux-kselftest/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-kselftest linux-kselftest/ https://lore.kernel.org/linux-kselftest \
		linux-kselftest@vger.kernel.org linux-kselftest@archiver.kernel.org
	public-inbox-index linux-kselftest

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kselftest


AGPL code for this site: git clone https://public-inbox.org/ public-inbox