[{"id":1774338,"web_url":"http://patchwork.ozlabs.org/comment/1774338/","msgid":"<CAPnjgZ107c6Kr4BoEyXniUVRRtqCzvLdmT5DjHQdM3C9Bd3y8Q@mail.gmail.com>","list_archive_url":null,"date":"2017-09-25T02:12:02","subject":"Re: [U-Boot] [PATCH 08/10] efi_selftest: test task priority levels","submitter":{"id":6170,"url":"http://patchwork.ozlabs.org/api/people/6170/","name":"Simon Glass","email":"sjg@chromium.org"},"content":"On 15 September 2017 at 02:06, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:\n> Run a 10 ms periodic timer and check that it is called 10 times\n> while waiting for 100 ms single shot timer.\n>\n> Raise the TPL level to the level of the 10 ms timer and observe\n> that the notification function is not called again.\n>\n> Lower the TPL level and check that the queued notification\n> function is called.\n>\n> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>\n> ---\n>  lib/efi_selftest/Makefile           |   5 +-\n>  lib/efi_selftest/efi_selftest_tpl.c | 214 ++++++++++++++++++++++++++++++++++++\n>  2 files changed, 218 insertions(+), 1 deletion(-)\n>  create mode 100644 lib/efi_selftest/efi_selftest_tpl.c\n>\n> diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile\n> index a71c8bf937..ddf304e1fa 100644\n> --- a/lib/efi_selftest/Makefile\n> +++ b/lib/efi_selftest/Makefile\n> @@ -13,8 +13,11 @@ CFLAGS_efi_selftest_console.o := $(CFLAGS_EFI)\n>  CFLAGS_REMOVE_efi_selftest_console.o := $(CFLAGS_NON_EFI)\n>  CFLAGS_efi_selftest_events.o := $(CFLAGS_EFI)\n>  CFLAGS_REMOVE_efi_selftest_events.o := $(CFLAGS_NON_EFI)\n> +CFLAGS_efi_selftest_tpl.o := $(CFLAGS_EFI)\n> +CFLAGS_REMOVE_efi_selftest_tpl.o := $(CFLAGS_NON_EFI)\n>\n>  obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += \\\n>  efi_selftest.o \\\n>  efi_selftest_console.o \\\n> -efi_selftest_events.o\n> +efi_selftest_events.o \\\n> +efi_selftest_tpl.o\n> diff --git a/lib/efi_selftest/efi_selftest_tpl.c b/lib/efi_selftest/efi_selftest_tpl.c\n> new file mode 100644\n> index 0000000000..90ace0f51e\n> --- /dev/null\n> +++ b/lib/efi_selftest/efi_selftest_tpl.c\n> @@ -0,0 +1,214 @@\n> +/*\n> + * efi_selftest_events\n> + *\n> + * Copyright (c) 2017 Heinrich Schuchardt <xypron.glpk@gmx.de>\n> + *\n> + * SPDX-License-Identifier:     GPL-2.0+\n> + *\n> + * This unit test uses timer events to check the handling of\n> + * task priority levels.\n> + */\n> +\n> +#include <efi_selftest.h>\n> +\n> +static struct efi_event *event_notify;\n> +static struct efi_event *event_wait;\n> +static unsigned int counter;\n> +static struct efi_boot_services *boottime;\n> +\n> +/*\n> + * Notification function, increments a counter.\n> + *\n> + * @event      notified event\n> + * @context    pointer to the counter\n> + */\n> +static void EFIAPI notify(struct efi_event *event, void *context)\n> +{\n> +       if (!context)\n> +               return;\n\nDoes this ever happen in practice? If not, why check it?\n\n> +       ++*(unsigned int *)context;\n\nSimilar comment to previous patch about using a struct here.\n\n> +}\n> +\n> +/*\n> + * Setup unit test.\n> + *\n> + * Create two timer events.\n> + * One with EVT_NOTIFY_SIGNAL, the other with EVT_NOTIFY_WAIT.\n> + *\n> + * @handle:    handle of the loaded image\n> + * @systable:  system table\n\n@return ...\n\n> + */\n> +static int setup(const efi_handle_t handle,\n> +                const struct efi_system_table *systable)\n> +{\n> +       efi_status_t ret;\n> +\n> +       boottime = systable->boottime;\n> +\n> +       ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_SIGNAL,\n> +                                    TPL_CALLBACK, notify, (void *)&counter,\n> +                                    &event_notify);\n> +       if (ret != EFI_SUCCESS) {\n> +               efi_st_error(\"could not create event\\n\");\n> +               return 1;\n\nWhy not return ret?\n\n> +       }\n> +       ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_WAIT,\n> +                                    TPL_HIGH_LEVEL, notify, NULL, &event_wait);\n> +       if (ret != EFI_SUCCESS) {\n> +               efi_st_error(\"could not create event\\n\");\n> +               return 1;\n> +       }\n> +       return 0;\n> +}\n> +\n> +/*\n> + * Tear down unit test.\n> + *\n> + * Close the events created in setup.\n> + */\n> +static int teardown(void)\n> +{\n> +       efi_status_t ret;\n> +\n> +       if (event_notify) {\n> +               ret = boottime->close_event(event_notify);\n> +               event_notify = NULL;\n> +               if (ret != EFI_SUCCESS) {\n> +                       efi_st_error(\"could not close event\\n\");\n> +                       return 1;\n> +               }\n> +       }\n> +       if (event_wait) {\n> +               ret = boottime->close_event(event_wait);\n> +               event_wait = NULL;\n> +               if (ret != EFI_SUCCESS) {\n> +                       efi_st_error(\"could not close event\\n\");\n> +                       return 1;\n> +               }\n> +       }\n> +       boottime->restore_tpl(TPL_APPLICATION);\n> +       return 0;\n> +}\n> +\n> +/*\n> + * Execute unit test.\n> + *\n> + * Run a 10 ms periodic timer and check that it is called 10 times\n> + * while waiting for 100 ms single shot timer.\n> + *\n> + * Raise the TPL level to the level of the 10 ms timer and observe\n> + * that the notification function is not called again.\n> + *\n> + * Lower the TPL level and check that the queued notification\n> + * function is called.\n> + */\n> +static int execute(void)\n> +{\n> +       unsigned long index;\n> +       efi_status_t ret;\n> +       UINTN old_tpl;\n> +\n> +       /* Set 10 ms timer */\n> +       counter = 0;\n> +       ret = boottime->set_timer(event_notify, EFI_TIMER_PERIODIC, 100000);\n> +       if (ret != EFI_SUCCESS) {\n> +               efi_st_error(\"Could not set timer\\n\");\n> +               return 1;\n> +       }\n> +       /* Set 100 ms timer */\n> +       ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000000);\n> +       if (ret != EFI_SUCCESS) {\n> +               efi_st_error(\"Could not set timer\\n\");\n> +               return 1;\n> +       }\n> +       index = 5;\n> +       ret = boottime->wait_for_event(1, &event_wait, &index);\n> +       if (ret != EFI_SUCCESS) {\n> +               efi_st_error(\"Could not wait for event\\n\");\n> +               return 1;\n> +       }\n> +       ret = boottime->check_event(event_wait);\n> +       if (ret != EFI_NOT_READY) {\n> +               efi_st_error(\"Signaled state was not cleared.\\n\");\n> +               efi_st_printf(\"ret = %u\\n\", (unsigned int)ret);\n> +               return 1;\n> +       }\n> +       if (index != 0) {\n> +               efi_st_error(\"WaitForEvent returned wrong index\\n\");\n> +               return 1;\n> +       }\n> +       efi_st_printf(\"Counter with TPL level TPL_APPLICATION: %u\\n\", counter);\n> +       if (counter < 8 || counter > 12) {\n> +               efi_st_error(\"Incorrect timing of events\\n\");\n> +               return 1;\n> +       }\n> +       ret = boottime->set_timer(event_notify, EFI_TIMER_STOP, 0);\n> +       if (index != 0) {\n> +               efi_st_error(\"Could not cancel timer\\n\");\n> +               return 1;\n> +       }\n> +       /* Raise TPL level */\n> +       old_tpl = boottime->raise_tpl(TPL_CALLBACK);\n> +       if (old_tpl != TPL_APPLICATION) {\n> +               efi_st_error(\"Initial TPL level was not TPL_APPLICATION\");\n> +               return 1;\n> +       }\n> +       /* Set 10 ms timer */\n> +       counter = 0;\n> +       ret = boottime->set_timer(event_notify, EFI_TIMER_PERIODIC, 100000);\n> +       if (index != 0) {\n> +               efi_st_error(\"Could not set timer\\n\");\n> +               return 1;\n> +       }\n> +       /* Set 100 ms timer */\n> +       ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000000);\n> +       if (ret != EFI_SUCCESS) {\n> +               efi_st_error(\"Could not set timer\\n\");\n> +               return 1;\n> +       }\n> +       do {\n> +               ret = boottime->check_event(event_wait);\n> +       } while (ret == EFI_NOT_READY);\n> +       if (ret != EFI_SUCCESS) {\n> +               efi_st_error(\"Could not check event\\n\");\n> +               return 1;\n> +       }\n> +       efi_st_printf(\"Counter with TPL level TPL_CALLBACK: %u\\n\", counter);\n> +       if (counter != 0) {\n> +               efi_st_error(\"Suppressed timer fired\\n\");\n> +               return 1;\n> +       }\n> +       /* Set 1 ms timer */\n> +       ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000);\n> +       if (ret != EFI_SUCCESS) {\n> +               efi_st_error(\"Could not set timer\\n\");\n> +               return 1;\n> +       }\n> +       /* Restore the old TPL level */\n> +       boottime->restore_tpl(TPL_APPLICATION);\n> +       ret = boottime->wait_for_event(1, &event_wait, &index);\n> +       if (ret != EFI_SUCCESS) {\n> +               efi_st_error(\"Could not wait for event\\n\");\n> +               return 1;\n> +       }\n> +       efi_st_printf(\"Counter with TPL level TPL_APPLICATION: %u\\n\", counter);\n> +       if (counter < 1) {\n> +               efi_st_error(\"Queued timer event did not fire\\n\");\n> +               return 1;\n> +       }\n> +       ret = boottime->set_timer(event_wait, EFI_TIMER_STOP, 0);\n> +       if (index != 0) {\n> +               efi_st_error(\"Could not cancel timer\\n\");\n> +               return 1;\n> +       }\n> +\n> +       return 0;\n> +}\n> +\n> +EFI_UNIT_TEST(tpl) = {\n> +       .name = \"task priority levels\",\n> +       .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,\n> +       .setup = setup,\n> +       .execute = execute,\n> +       .teardown = teardown,\n\nIn general I suggest you put the filename as a prefix on these\nfunction names. It makes __func__ much more useful in the function,\nplus the name is more meaningful.\n\nE.g. efi_selftest_tpl_execute()\n\nRegards,\nSimon","headers":{"Return-Path":"<u-boot-bounces@lists.denx.de>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"my2nYevt\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"iN4BBxrz\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3y0nlS0yt7z9t3t\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 25 Sep 2017 12:16:52 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid BCF11C21C41; Mon, 25 Sep 2017 02:13:50 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id E5ED1C22128;\n\tMon, 25 Sep 2017 02:13:29 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 063FDC21C26; Mon, 25 Sep 2017 02:12:28 +0000 (UTC)","from mail-qt0-f172.google.com (mail-qt0-f172.google.com\n\t[209.85.216.172])\n\tby lists.denx.de (Postfix) with ESMTPS id D5085C22151\n\tfor <u-boot@lists.denx.de>; Mon, 25 Sep 2017 02:12:24 +0000 (UTC)","by mail-qt0-f172.google.com with SMTP id b1so5552277qtc.4\n\tfor <u-boot@lists.denx.de>; Sun, 24 Sep 2017 19:12:24 -0700 (PDT)","by 10.200.37.200 with HTTP; Sun, 24 Sep 2017 19:12:02 -0700 (PDT)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.0 required=5.0 tests=RCVD_IN_DNSWL_NONE,\n\tRCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,\n\tT_DKIM_INVALID autolearn=unavailable\n\tautolearn_force=no version=3.4.0","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n\ts=20161025; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=oV34pGApiByXeLnZyV1KIIbJOo75wf0Q0f8AOqHnq0s=;\n\tb=my2nYevthPsRx+lMCy2bVl2/WQuwJLETbmH+hW39q/L0OToqR1mEpXx3n0gn0huVFa\n\tkwSWFYE/FXLfNNiwRqhxu+Q3S1rkNtBVggo99zmleEjd5J1l+qROtdb7/nfEExu0AFEA\n\tNb/VKokEGLG0s2atpCxW6CIso7UTqYuk1jjV85B2wJQqlVWpfL8FU49qH7/oM7lHkD+z\n\t0Dliq8k4WSz2vArIx4Mm1rVbHCllv8bgftQGGO4NzY5Ibjvc9zo+zxrcvhcj0uAKULLj\n\tAE+Xr2et28pFDXCZsug41IM3ui9KXg4fsie4GRRkQ+X2yfM4BlxiQ5VyjTj1e0bBsC5r\n\trXvg==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=oV34pGApiByXeLnZyV1KIIbJOo75wf0Q0f8AOqHnq0s=;\n\tb=iN4BBxrzzuSBkd0f9/E5Dbey44F2h5rdYwgN2Bnl2bKtFtfI3osbELxdXAoO+qJrjp\n\t+ItP6SxNRupoX5cFBp9iDQ/1SwwgUlAI0FYvMTJBh1KOZlwUD+QgcxZo/jD3zyoCNsU+\n\thuywUryK3s/regoYOhZyPQgNLguR2OpFdwPRw="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc;\n\tbh=oV34pGApiByXeLnZyV1KIIbJOo75wf0Q0f8AOqHnq0s=;\n\tb=cFfRV92SA9IFkhhXmwmDMvI2s9g7WbgbLmPRD0898UAz4/YL/+Z2PtDbwHscmeV4ur\n\tHMlZXh4HK0z5IYsFtZSAEA6RpKxoizdg3CpoenxCf9e1AtuKPaPnPQ35wB+y+w5TmqCJ\n\tr89ASWPL7XFNnDgCFsMWfbBYFQKVpqIp3rGmcYGVERWsmlYpkJTnd0N71lUrtlTaPgUa\n\tskGeEzbqYWc77ovgPXq9bEjzVnKStntxCEvL0DqUyd35jvLCcIyyzEPXBOKBOB3e9J06\n\t6BhvNk995xE9KWQpuSksg6e6DJiQSmhBXh5zVfz9CPPW/tm6rDKTDOf94KbTgJPvUgiU\n\t9fKA==","X-Gm-Message-State":"AHPjjUiqZZYYH6LCC8sWcV3qx+KVzkE3wzG/l3jL3T6oUp/XJPMYUoqa\n\ttQyG5rHSw0JxfNJH0CykyPeSzeYqljcPgrXw3nbrQA==","X-Google-Smtp-Source":"AOwi7QD1tFVn+Yf96nakTHsI/4K6qMfVRZ4RM8Cv8vSnfIy9NwyHXOjf5mx5IAG2YTcy+7RtTfvqwBuXtz3fiBwk/oo=","X-Received":"by 10.237.35.238 with SMTP id k43mr9215239qtc.214.1506305543322; \n\tSun, 24 Sep 2017 19:12:23 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170915080619.25250-9-xypron.glpk@gmx.de>","References":"<20170915080619.25250-1-xypron.glpk@gmx.de>\n\t<20170915080619.25250-9-xypron.glpk@gmx.de>","From":"Simon Glass <sjg@chromium.org>","Date":"Sun, 24 Sep 2017 22:12:02 -0400","X-Google-Sender-Auth":"GSCexOMvGzCh97WN7YL_yHqD8ds","Message-ID":"<CAPnjgZ107c6Kr4BoEyXniUVRRtqCzvLdmT5DjHQdM3C9Bd3y8Q@mail.gmail.com>","To":"Heinrich Schuchardt <xypron.glpk@gmx.de>","Cc":"=?utf-8?q?=C5=81ukasz_Majewski?= <l.majewski@samsung.com>,\n\tAndy Shevchenko <andriy.shevchenko@linux.intel.com>, U-Boot Mailing List\n\t<u-boot@lists.denx.de>,  Fabio Estevam <fabio.estevam@nxp.com>,\n\tMaxime Ripard <maxime.ripard@free-electrons.com>","Subject":"Re: [U-Boot] [PATCH 08/10] efi_selftest: test task priority levels","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion <u-boot.lists.denx.de>","List-Unsubscribe":"<https://lists.denx.de/options/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=unsubscribe>","List-Archive":"<http://lists.denx.de/pipermail/u-boot/>","List-Post":"<mailto:u-boot@lists.denx.de>","List-Help":"<mailto:u-boot-request@lists.denx.de?subject=help>","List-Subscribe":"<https://lists.denx.de/listinfo/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=subscribe>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" <u-boot-bounces@lists.denx.de>"}},{"id":1774439,"web_url":"http://patchwork.ozlabs.org/comment/1774439/","msgid":"<d498e238-b466-13b3-c066-f7252d4f84f7@gmx.de>","list_archive_url":null,"date":"2017-09-25T06:18:35","subject":"Re: [U-Boot] [PATCH 08/10] efi_selftest: test task priority levels","submitter":{"id":61270,"url":"http://patchwork.ozlabs.org/api/people/61270/","name":"Heinrich Schuchardt","email":"xypron.glpk@gmx.de"},"content":"On 09/25/2017 04:12 AM, Simon Glass wrote:\n> On 15 September 2017 at 02:06, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:\n>> Run a 10 ms periodic timer and check that it is called 10 times\n>> while waiting for 100 ms single shot timer.\n>>\n>> Raise the TPL level to the level of the 10 ms timer and observe\n>> that the notification function is not called again.\n>>\n>> Lower the TPL level and check that the queued notification\n>> function is called.\n>>\n>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>\n>> ---\n>>  lib/efi_selftest/Makefile           |   5 +-\n>>  lib/efi_selftest/efi_selftest_tpl.c | 214 ++++++++++++++++++++++++++++++++++++\n>>  2 files changed, 218 insertions(+), 1 deletion(-)\n>>  create mode 100644 lib/efi_selftest/efi_selftest_tpl.c\n>>\n>> diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile\n>> index a71c8bf937..ddf304e1fa 100644\n>> --- a/lib/efi_selftest/Makefile\n>> +++ b/lib/efi_selftest/Makefile\n>> @@ -13,8 +13,11 @@ CFLAGS_efi_selftest_console.o := $(CFLAGS_EFI)\n>>  CFLAGS_REMOVE_efi_selftest_console.o := $(CFLAGS_NON_EFI)\n>>  CFLAGS_efi_selftest_events.o := $(CFLAGS_EFI)\n>>  CFLAGS_REMOVE_efi_selftest_events.o := $(CFLAGS_NON_EFI)\n>> +CFLAGS_efi_selftest_tpl.o := $(CFLAGS_EFI)\n>> +CFLAGS_REMOVE_efi_selftest_tpl.o := $(CFLAGS_NON_EFI)\n>>\n>>  obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += \\\n>>  efi_selftest.o \\\n>>  efi_selftest_console.o \\\n>> -efi_selftest_events.o\n>> +efi_selftest_events.o \\\n>> +efi_selftest_tpl.o\n>> diff --git a/lib/efi_selftest/efi_selftest_tpl.c b/lib/efi_selftest/efi_selftest_tpl.c\n>> new file mode 100644\n>> index 0000000000..90ace0f51e\n>> --- /dev/null\n>> +++ b/lib/efi_selftest/efi_selftest_tpl.c\n>> @@ -0,0 +1,214 @@\n>> +/*\n>> + * efi_selftest_events\n>> + *\n>> + * Copyright (c) 2017 Heinrich Schuchardt <xypron.glpk@gmx.de>\n>> + *\n>> + * SPDX-License-Identifier:     GPL-2.0+\n>> + *\n>> + * This unit test uses timer events to check the handling of\n>> + * task priority levels.\n>> + */\n>> +\n>> +#include <efi_selftest.h>\n>> +\n>> +static struct efi_event *event_notify;\n>> +static struct efi_event *event_wait;\n>> +static unsigned int counter;\n>> +static struct efi_boot_services *boottime;\n>> +\n>> +/*\n>> + * Notification function, increments a counter.\n>> + *\n>> + * @event      notified event\n>> + * @context    pointer to the counter\n>> + */\n>> +static void EFIAPI notify(struct efi_event *event, void *context)\n>> +{\n>> +       if (!context)\n>> +               return;\n> \n> Does this ever happen in practice? If not, why check it?\n> \n>> +       ++*(unsigned int *)context;\n> \n> Similar comment to previous patch about using a struct here.\n> \n>> +}\n>> +\n>> +/*\n>> + * Setup unit test.\n>> + *\n>> + * Create two timer events.\n>> + * One with EVT_NOTIFY_SIGNAL, the other with EVT_NOTIFY_WAIT.\n>> + *\n>> + * @handle:    handle of the loaded image\n>> + * @systable:  system table\n> \n> @return ...\n> \n>> + */\n>> +static int setup(const efi_handle_t handle,\n>> +                const struct efi_system_table *systable)\n>> +{\n>> +       efi_status_t ret;\n>> +\n>> +       boottime = systable->boottime;\n>> +\n>> +       ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_SIGNAL,\n>> +                                    TPL_CALLBACK, notify, (void *)&counter,\n>> +                                    &event_notify);\n>> +       if (ret != EFI_SUCCESS) {\n>> +               efi_st_error(\"could not create event\\n\");\n>> +               return 1;\n> \n> Why not return ret?\n\nWe return int an not efi_status_t which is UINTN (= size_t).\n\nThe code might be more readable by using\n\n#define EFI_ST_SUCCESS 0\n#define EFI_ST_FAILURE 1\n\nRegards\n\nHeinrich\n\n> \n>> +       }\n>> +       ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_WAIT,\n>> +                                    TPL_HIGH_LEVEL, notify, NULL, &event_wait);\n>> +       if (ret != EFI_SUCCESS) {\n>> +               efi_st_error(\"could not create event\\n\");\n>> +               return 1;\n>> +       }\n>> +       return 0;\n>> +}\n>> +\n>> +/*\n>> + * Tear down unit test.\n>> + *\n>> + * Close the events created in setup.\n>> + */\n>> +static int teardown(void)\n>> +{\n>> +       efi_status_t ret;\n>> +\n>> +       if (event_notify) {\n>> +               ret = boottime->close_event(event_notify);\n>> +               event_notify = NULL;\n>> +               if (ret != EFI_SUCCESS) {\n>> +                       efi_st_error(\"could not close event\\n\");\n>> +                       return 1;\n>> +               }\n>> +       }\n>> +       if (event_wait) {\n>> +               ret = boottime->close_event(event_wait);\n>> +               event_wait = NULL;\n>> +               if (ret != EFI_SUCCESS) {\n>> +                       efi_st_error(\"could not close event\\n\");\n>> +                       return 1;\n>> +               }\n>> +       }\n>> +       boottime->restore_tpl(TPL_APPLICATION);\n>> +       return 0;\n>> +}\n>> +\n>> +/*\n>> + * Execute unit test.\n>> + *\n>> + * Run a 10 ms periodic timer and check that it is called 10 times\n>> + * while waiting for 100 ms single shot timer.\n>> + *\n>> + * Raise the TPL level to the level of the 10 ms timer and observe\n>> + * that the notification function is not called again.\n>> + *\n>> + * Lower the TPL level and check that the queued notification\n>> + * function is called.\n>> + */\n>> +static int execute(void)\n>> +{\n>> +       unsigned long index;\n>> +       efi_status_t ret;\n>> +       UINTN old_tpl;\n>> +\n>> +       /* Set 10 ms timer */\n>> +       counter = 0;\n>> +       ret = boottime->set_timer(event_notify, EFI_TIMER_PERIODIC, 100000);\n>> +       if (ret != EFI_SUCCESS) {\n>> +               efi_st_error(\"Could not set timer\\n\");\n>> +               return 1;\n>> +       }\n>> +       /* Set 100 ms timer */\n>> +       ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000000);\n>> +       if (ret != EFI_SUCCESS) {\n>> +               efi_st_error(\"Could not set timer\\n\");\n>> +               return 1;\n>> +       }\n>> +       index = 5;\n>> +       ret = boottime->wait_for_event(1, &event_wait, &index);\n>> +       if (ret != EFI_SUCCESS) {\n>> +               efi_st_error(\"Could not wait for event\\n\");\n>> +               return 1;\n>> +       }\n>> +       ret = boottime->check_event(event_wait);\n>> +       if (ret != EFI_NOT_READY) {\n>> +               efi_st_error(\"Signaled state was not cleared.\\n\");\n>> +               efi_st_printf(\"ret = %u\\n\", (unsigned int)ret);\n>> +               return 1;\n>> +       }\n>> +       if (index != 0) {\n>> +               efi_st_error(\"WaitForEvent returned wrong index\\n\");\n>> +               return 1;\n>> +       }\n>> +       efi_st_printf(\"Counter with TPL level TPL_APPLICATION: %u\\n\", counter);\n>> +       if (counter < 8 || counter > 12) {\n>> +               efi_st_error(\"Incorrect timing of events\\n\");\n>> +               return 1;\n>> +       }\n>> +       ret = boottime->set_timer(event_notify, EFI_TIMER_STOP, 0);\n>> +       if (index != 0) {\n>> +               efi_st_error(\"Could not cancel timer\\n\");\n>> +               return 1;\n>> +       }\n>> +       /* Raise TPL level */\n>> +       old_tpl = boottime->raise_tpl(TPL_CALLBACK);\n>> +       if (old_tpl != TPL_APPLICATION) {\n>> +               efi_st_error(\"Initial TPL level was not TPL_APPLICATION\");\n>> +               return 1;\n>> +       }\n>> +       /* Set 10 ms timer */\n>> +       counter = 0;\n>> +       ret = boottime->set_timer(event_notify, EFI_TIMER_PERIODIC, 100000);\n>> +       if (index != 0) {\n>> +               efi_st_error(\"Could not set timer\\n\");\n>> +               return 1;\n>> +       }\n>> +       /* Set 100 ms timer */\n>> +       ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000000);\n>> +       if (ret != EFI_SUCCESS) {\n>> +               efi_st_error(\"Could not set timer\\n\");\n>> +               return 1;\n>> +       }\n>> +       do {\n>> +               ret = boottime->check_event(event_wait);\n>> +       } while (ret == EFI_NOT_READY);\n>> +       if (ret != EFI_SUCCESS) {\n>> +               efi_st_error(\"Could not check event\\n\");\n>> +               return 1;\n>> +       }\n>> +       efi_st_printf(\"Counter with TPL level TPL_CALLBACK: %u\\n\", counter);\n>> +       if (counter != 0) {\n>> +               efi_st_error(\"Suppressed timer fired\\n\");\n>> +               return 1;\n>> +       }\n>> +       /* Set 1 ms timer */\n>> +       ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000);\n>> +       if (ret != EFI_SUCCESS) {\n>> +               efi_st_error(\"Could not set timer\\n\");\n>> +               return 1;\n>> +       }\n>> +       /* Restore the old TPL level */\n>> +       boottime->restore_tpl(TPL_APPLICATION);\n>> +       ret = boottime->wait_for_event(1, &event_wait, &index);\n>> +       if (ret != EFI_SUCCESS) {\n>> +               efi_st_error(\"Could not wait for event\\n\");\n>> +               return 1;\n>> +       }\n>> +       efi_st_printf(\"Counter with TPL level TPL_APPLICATION: %u\\n\", counter);\n>> +       if (counter < 1) {\n>> +               efi_st_error(\"Queued timer event did not fire\\n\");\n>> +               return 1;\n>> +       }\n>> +       ret = boottime->set_timer(event_wait, EFI_TIMER_STOP, 0);\n>> +       if (index != 0) {\n>> +               efi_st_error(\"Could not cancel timer\\n\");\n>> +               return 1;\n>> +       }\n>> +\n>> +       return 0;\n>> +}\n>> +\n>> +EFI_UNIT_TEST(tpl) = {\n>> +       .name = \"task priority levels\",\n>> +       .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,\n>> +       .setup = setup,\n>> +       .execute = execute,\n>> +       .teardown = teardown,\n> \n> In general I suggest you put the filename as a prefix on these\n> function names. It makes __func__ much more useful in the function,\n> plus the name is more meaningful.\n> \n> E.g. efi_selftest_tpl_execute()\n> \n> Regards,\n> Simon\n>","headers":{"Return-Path":"<u-boot-bounces@lists.denx.de>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=<UNKNOWN>)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3y0v7L0QfCz9s7c\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 25 Sep 2017 16:19:25 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid C0764C22208; Mon, 25 Sep 2017 06:19:22 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 70673C21C40;\n\tMon, 25 Sep 2017 06:19:19 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 6CDE2C21C40; Mon, 25 Sep 2017 06:19:18 +0000 (UTC)","from mout.gmx.net (mout.gmx.net [212.227.17.22])\n\tby lists.denx.de (Postfix) with ESMTPS id 17443C21C34\n\tfor <u-boot@lists.denx.de>; Mon, 25 Sep 2017 06:19:18 +0000 (UTC)","from [192.168.123.31] ([84.118.154.110]) by mail.gmx.com (mrgmx103\n\t[212.227.17.168]) with ESMTPSA (Nemesis) id\n\t0MKpQ4-1dwMj81gUS-0007e3; Mon, 25 Sep 2017 08:18:47 +0200"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.7 required=5.0 tests=FREEMAIL_FROM,\n\tRCVD_IN_DNSWL_LOW,\n\tRCVD_IN_MSPIKE_H2 autolearn=unavailable autolearn_force=no\n\tversion=3.4.0","To":"Simon Glass <sjg@chromium.org>","References":"<20170915080619.25250-1-xypron.glpk@gmx.de>\n\t<20170915080619.25250-9-xypron.glpk@gmx.de>\n\t<CAPnjgZ107c6Kr4BoEyXniUVRRtqCzvLdmT5DjHQdM3C9Bd3y8Q@mail.gmail.com>","From":"Heinrich Schuchardt <xypron.glpk@gmx.de>","Message-ID":"<d498e238-b466-13b3-c066-f7252d4f84f7@gmx.de>","Date":"Mon, 25 Sep 2017 08:18:35 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<CAPnjgZ107c6Kr4BoEyXniUVRRtqCzvLdmT5DjHQdM3C9Bd3y8Q@mail.gmail.com>","Content-Language":"en-US","X-Provags-ID":"V03:K0:JiJUrbBZnBBEpZMn+Lz1WpmQKTW/8aHIakw4slz8W2rC/Wvc9di\n\tZFQA3yk9iLGVH9nra+gI9FqLh/wJuRykg8gEtCVpDaTVKIlHa4mKbvLAvKmIFiAUys0e4pn\n\tOLsDG95VglwjHFCeIlkzcINQpOYUFENhWQbvhqmoYtNGa5w5uDHyqXUbDKtgioZPkU1UYC8\n\tDPhstA819YViat1qppw9A==","X-UI-Out-Filterresults":"notjunk:1; V01:K0:bFEzLS77/mI=:dQoKUTuXbGGxtX7PUt6TyS\n\tdpa/JPIFGx/eOy28woaV3HeMVz7/ZgaG8qHIHpjLXWFhukcU7U5RcntL9EefNIz4TSwtQ6eXY\n\tHTJma+Qpf2SpkZVxQuQcjHWMYfeXS7ctKQ6FSS0gwu2ZfImPhKhEdktPhpraALY3MxpCR2wJh\n\tAavIDwGEiW894sAK032z1m9zVcyPqNMVnSyaGIR/jcGSct0Q/5SnKlcZ0gLmBRgufIOS5Vtz9\n\tNIkn0IvdPLTP5dUI9gnNulNadj/HKl+vB8D51M8tgnJONP+SpkrRyxIHhzL0xPUWk2CpyaTUa\n\tSxoPyf8Cr2cDO0JVRcT25l1toPF7nxKu0YiRDeOnjOXiuuIUksH4+CicjLv80N9HNOIk8RBlr\n\tda3mAxM0SzCz4ZXMYN3VnDHOPnuA5zfQGKjftiG2aAtHmKCWFa0b5431zy9LuavbeEyjylmaf\n\tM4qwmzRCa9plkNvIAbD+cKS7XW/Tf+8RPQgfX16eUywrbgkbT7lDccvrkt/SNxAYrCIfBDGqL\n\tSsr39qC/XchaToxBxIX6cCnwcHNB86IPiBGLmM82D8aDZxQWHNuifrp35VSr8AXGWjeHvBpvM\n\tUr4SoJeITovaknXxgRUUiZVQw6QRC5OiTDY1Uycrvu4pj1A9JELiOYx2MBykanA1cA/EIx6f+\n\tpOFLCGiatUuzWpl2a3dKMskFHDeIoYOt/XI9oGFtyd/i+717YhtQYrvH5OfGC0Pwa4uWT+/zb\n\tloOkG9WiFfEaoUfgozurcOKrMvhNUecKEsAqMrZhwIBdhx03LR4BmShvZmriG5QiWnDAzjopo\n\tQv3NaIVfyQGhnMF9HHmyK+1i50N2XdI/T52zDs2nSnD4Sp/Mwk=","Cc":"=?utf-8?q?=C5=81ukasz_Majewski?= <l.majewski@samsung.com>,\n\tAndy Shevchenko <andriy.shevchenko@linux.intel.com>, U-Boot Mailing List\n\t<u-boot@lists.denx.de>,  Fabio Estevam <fabio.estevam@nxp.com>,\n\tMaxime Ripard <maxime.ripard@free-electrons.com>","Subject":"Re: [U-Boot] [PATCH 08/10] efi_selftest: test task priority levels","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion <u-boot.lists.denx.de>","List-Unsubscribe":"<https://lists.denx.de/options/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=unsubscribe>","List-Archive":"<http://lists.denx.de/pipermail/u-boot/>","List-Post":"<mailto:u-boot@lists.denx.de>","List-Help":"<mailto:u-boot-request@lists.denx.de?subject=help>","List-Subscribe":"<https://lists.denx.de/listinfo/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=subscribe>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" <u-boot-bounces@lists.denx.de>"}}]