All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Filip Bozuta <Filip.Bozuta@rt-rk.com>
Cc: qemu-devel@nongnu.org, laurent@vivier.eu
Subject: Re: [PATCH 1/2] tests/tcg/multiarch: Add tests for implemented rtc ioctls
Date: Tue, 21 Jan 2020 16:57:48 +0000	[thread overview]
Message-ID: <877e1k21df.fsf@linaro.org> (raw)
In-Reply-To: <1579536554-30701-2-git-send-email-Filip.Bozuta@rt-rk.com>


Filip Bozuta <Filip.Bozuta@rt-rk.com> writes:

> This patch adds tests for following 22 implemented rtc ioctls:
>
> * RTC_AIE_ON     * RTC_ALM_SET      * RTC_WKALM_SET
> * RTC_AIE_OFF    * RTC_ALM_READ     * RTC_WKALM_RD
> * RTC_UIE_ON     * RTC_RD_TIME      * RTC_PLL_GET
> * RTC_UIE_OFF    * RTC_SET_TIME     * RTC_PLL_SET
> * RTC_PIE_ON     * RTC_IRQP_READ    * RTC_VL_READ
> * RTC_PIE_OFF    * RTC_IRQP_SET     * RTC_VL_CLR
> * RTC_WIE_ON     * RTC_EPOCH_READ
> * RTC_WIE_OFF    * RTC_EPOCH_SET
>
> Names and descriptions of these ioctls can be found in patches that
> implement them.
>
> Test folder for these ioctl tests is located at
> "tests/tcg/multiarch/rtc-ioctl-tests/"

There is a lot of repetition in these tests - any particular reason not
to just roll up the tests into a single executable with shared setup and
test code?

> Tests for individual ioctls are located in separate folders. These
> folders are arranged by the same way these ioctls are implemented
> in patches. These test files are simple programs that use these ioctls
> to set/read or turn on/off some rtc features.

The default handling of check-tcg won't pick up tests in
sub-directories:

  MULTIARCH_SRCS   =$(notdir $(wildcard $(MULTIARCH_SRC)/*.c))

so I'm guessing it's not actually run in check-tcg.

> Besides tests for individual ioctls, a global rtc ioctl test was
> added at "tests/tcg/multiarch/rtc-ioctl-tests/GlobalTest/rtc-test.c"
> This test file was downloaded from linux kernel and is located at
> "linux/drivers/rtc/rtc-test.c".
> This file was modified a little bit so that it doesn't have styling
> problems identified by "scripts/checkpatch.pl".

This raises an interesting philosophical point about if we should be
porting tests for the linux source tree into QEMU.

> It is used to further test functionalities of some rtc ioctls by
> running rtc clock at different frequencies.
>
> Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
> ---
>  .../Alarm-time-test/ReadAlarm/getAlarm.c           |  33 +++
>  .../Alarm-time-test/ReadTime/getTime.c             |  35 ++++
>  .../Alarm-time-test/SetAlarm/setAlarm.c            |  31 +++
>  .../Alarm-time-test/SetTime/setTime.c              |  33 +++
>  .../AlarmInterrupt/Disable/disableAlarmInterrupt.c |  29 +++
>  .../AlarmInterrupt/Enable/enableAlarmInterrupt.c   |  29 +++
>  .../Disable/disablePeriodicInterrupt.c             |  30 +++
>  .../Enable/enablePeriodicInterrupt.c               |  29 +++
>  .../Disable/disableUpdateInterrupt.c               |  29 +++
>  .../UpdateInterrupt/Enable/enableUpdateInterrupt.c |  29 +++
>  .../Disable/disableWatchdogInterrupt.c             |  30 +++
>  .../Enable/enableWatchdogInterrupt.c               |  31 +++
>  .../rtc-ioctl-tests/GlobalTest/rtc-test.c          | 227 +++++++++++++++++++++
>  .../ReadEpoch/getEpoch.c                           |  32 +++
>  .../ReadPeriodicInterrupt/getPeriodicInterrupt.c   |  31 +++
>  .../SetEpoch/setEpoch.c                            |  32 +++
>  .../SetPeriodicInterrupt/setPeriodicInterrupt.c    |  31 +++
>  .../ReadPllCorrection/getPllCorrection.c           |  35 ++++
>  .../SetPllCorrection/setPllCorrection.c            |  32 +++
>  .../ClearVoltageLow/clearVoltageLow.c              |  32 +++
>  .../ReadVoltageLow/getVoltageLow.c                 |  32 +++
>  .../ReadWakeupAlarm/getWakeupAlarm.c               |  36 ++++
>  .../SetWakeupAlarm/setWakeupAlarm.c                |  34 +++

I can't say I'm keen about having a bunch of camelCased and PascalCased
filenames and directories.
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Alarm-time-test/ReadAlarm/getAlarm.c
> @@ -0,0 +1,33 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDWR | O_NONBLOCK);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Alarm-time-test/ReadTime/getTime.c
> @@ -0,0 +1,35 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDWR | O_NONBLOCK);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Alarm-time-test/SetAlarm/setAlarm.c
> @@ -0,0 +1,31 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDWR);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Alarm-time-test/SetTime/setTime.c
> @@ -0,0 +1,33 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <sys/capability.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDWR | O_NONBLOCK);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Features-test/AlarmInterrupt/Disable/disableAlarmInterrupt.c
> @@ -0,0 +1,29 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDWR | O_NONBLOCK);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Features-test/AlarmInterrupt/Enable/enableAlarmInterrupt.c
> @@ -0,0 +1,29 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDWR | O_NONBLOCK);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Features-test/PeriodicInterrupt/Disable/disablePeriodicInterrupt.c
> @@ -0,0 +1,30 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDWR | O_NONBLOCK);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Features-test/PeriodicInterrupt/Enable/enablePeriodicInterrupt.c
> @@ -0,0 +1,29 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDWR | O_NONBLOCK);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Features-test/UpdateInterrupt/Disable/disableUpdateInterrupt.c
> @@ -0,0 +1,29 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDWR | O_NONBLOCK);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Features-test/UpdateInterrupt/Enable/enableUpdateInterrupt.c
> @@ -0,0 +1,29 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDWR | O_NONBLOCK);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Features-test/WatchdogInterrupt/Disable/disableWatchdogInterrupt.c
> @@ -0,0 +1,30 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <linux/ioctl.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDONLY);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Features-test/WatchdogInterrupt/Enable/enableWatchdogInterrupt.c
> @@ -0,0 +1,31 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <linux/ioctl.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDONLY);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Periodic-interrupt-epoch-test/ReadEpoch/getEpoch.c
> @@ -0,0 +1,32 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <linux/ioctl.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDONLY);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Periodic-interrupt-epoch-test/ReadPeriodicInterrupt/getPeriodicInterrupt.c
> @@ -0,0 +1,31 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDWR | O_NONBLOCK);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
> +
> +    unsigned long interrupt_rate;
> +
> +    if (ioctl(fd, RTC_IRQP_READ, &interrupt_rate) == ERROR) {
> +        perror("ioctl");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Periodic-interrupt-epoch-test/SetEpoch/setEpoch.c
> @@ -0,0 +1,32 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <linux/ioctl.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDWR | O_NONBLOCK);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Periodic-interrupt-epoch-test/SetPeriodicInterrupt/setPeriodicInterrupt.c
> @@ -0,0 +1,31 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDWR | O_NONBLOCK);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Pll-correction-test/ReadPllCorrection/getPllCorrection.c
> @@ -0,0 +1,35 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <linux/ioctl.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDONLY);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Pll-correction-test/SetPllCorrection/setPllCorrection.c
> @@ -0,0 +1,32 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <linux/ioctl.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc0", O_RDONLY);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Voltage-low-test/ClearVoltageLow/clearVoltageLow.c
> @@ -0,0 +1,32 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <linux/ioctl.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDONLY);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Voltage-low-test/ReadVoltageLow/getVoltageLow.c
> @@ -0,0 +1,32 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <linux/ioctl.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/proc/driver/rtc", O_RDONLY);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Wakeup-alarm-test/ReadWakeupAlarm/getWakeupAlarm.c
> @@ -0,0 +1,36 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDWR | O_NONBLOCK);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>
> --- /dev/null
> +++ b/tests/tcg/multiarch/rtc-ioctl-tests/Wakeup-alarm-test/SetWakeupAlarm/setWakeupAlarm.c
> @@ -0,0 +1,34 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/rtc.h>
> +#include <fcntl.h>
> +#include <linux/input.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#define ERROR -1
> +
> +int main()
> +{
> +
> +    int fd = open("/dev/rtc", O_RDONLY);
> +
> +    if (fd == ERROR) {
> +        perror("open");
> +        return -1;
> +    }
<snip>

By all means introduce the patches in multiple steps but I think a
single test file would be preferable and you could then share all the
setup logic (not that there is much).

-- 
Alex Bennée


  reply	other threads:[~2020-01-21 16:59 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-20 16:09 [PATCH 0/2] tests/tcg/multiarch: Add tests for implemented real Filip Bozuta
2020-01-20 16:09 ` [PATCH 1/2] tests/tcg/multiarch: Add tests for implemented rtc ioctls Filip Bozuta
2020-01-21 16:57   ` Alex Bennée [this message]
2020-01-20 16:09 ` [PATCH 2/2] tests/tcg/multiarch: Add tests for implemented alsa sound timer ioctls Filip Bozuta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877e1k21df.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=Filip.Bozuta@rt-rk.com \
    --cc=laurent@vivier.eu \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.