On Wed, Sep 10, 2025 at 07:00:00PM +0200, Gabriele Paoloni wrote:
From: Alessandro Carminati acarmina@redhat.com
This patch introduces a new series of tests for devmem. Test cases are mapped against the tested Function's expectations defined in /drivers/char/mem.c.
Cool, but:
Signed-off-by: Alessandro Carminati acarmina@redhat.com
tools/testing/selftests/Makefile | 1 + tools/testing/selftests/devmem/Makefile | 13 + tools/testing/selftests/devmem/debug.c | 25 + tools/testing/selftests/devmem/debug.h | 14 + tools/testing/selftests/devmem/devmem.c | 200 ++++++++ tools/testing/selftests/devmem/ram_map.c | 250 ++++++++++ tools/testing/selftests/devmem/ram_map.h | 38 ++ tools/testing/selftests/devmem/secret.c | 46 ++ tools/testing/selftests/devmem/secret.h | 13 + tools/testing/selftests/devmem/tests.c | 569 +++++++++++++++++++++++ tools/testing/selftests/devmem/tests.h | 45 ++ tools/testing/selftests/devmem/utils.c | 379 +++++++++++++++ tools/testing/selftests/devmem/utils.h | 119 +++++
That's a lot of files for a "simple" test. Doesn't LTP have tests for this api already? Why not use that here instead?
Also, this is userspace testing, not kunit testing, right, is that intentional? You are documenting internal apis and then writing userspace tests for those apis, which feels a bit odd.
Also /dev/mem should not be used on "modern" systems, so how was this tested?
+// SPDX-License-Identifier: GPL-2.0+
Are you _sure_ you want GPLv2+? I have to ask, sorry.
+/*
- devmem test debug.c
- Copyright (C) 2025 Red Hat, Inc. All Rights Reserved.
- Written by Alessandro Carminati (acarmina@redhat.com)
- */
+#include <stdio.h> +#include <stdarg.h>
+#define DEBUG_FLAG 0 +int pdebug = DEBUG_FLAG;
That's a funny define that is never used elsewhere. I'm guessing this was cut/pasted from some other userspace code somewhere?
+void deb_printf(const char *fmt, ...)
Who is "deb"? You have more letters, always use them :)
Also, why debugging for just this one set of tests? Don't kselftests already have debugging logic? if not, why is this unique to require it?
And am I missing something, or does this new tool not tie into the kselftest framework properly? I see lots of printing to output, but not in the proper test framework format, am I just missing that somewhere?
thanks,
greg k-h