* [PATCH v3] resource: add a simple test for walk_iomem_res_desc()
@ 2024-06-05 19:53 Chia-I Wu
2024-06-05 20:33 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: Chia-I Wu @ 2024-06-05 19:53 UTC (permalink / raw)
To: Greg Kroah-Hartman, Andy Shevchenko, AKASHI Takahiro, Baoquan He,
Dan Williams, Alison Schofield, linux-kernel
This mainly tests that find_next_iomem_res() does not miss resources.
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
---
v2: update subject, use DEFINE_RES_NAMED and hardcoded offsets
v3: really hardcode offsets
---
kernel/resource_kunit.c | 65 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 65 insertions(+)
diff --git a/kernel/resource_kunit.c b/kernel/resource_kunit.c
index 58ab9f914602b..1a562d0978477 100644
--- a/kernel/resource_kunit.c
+++ b/kernel/resource_kunit.c
@@ -137,9 +137,74 @@ static void resource_test_intersection(struct kunit *test)
} while (++i < ARRAY_SIZE(results_for_intersection));
}
+static int resource_walk_count(struct resource *res, void *data)
+{
+ int *count = data;
+ (*count)++;
+ return 0;
+}
+
+static void resource_test_walk_iomem_res_desc(struct kunit *test)
+{
+ struct resource root = {
+ .name = "Resource Walk Test",
+ };
+ struct resource res[8];
+ int count;
+
+ KUNIT_ASSERT_EQ(test, 0,
+ allocate_resource(&iomem_resource, &root, 0x100000,
+ 0, ~0, 0x100000, NULL, NULL));
+
+ /* build the resource tree */
+ res[0] = DEFINE_RES_NAMED(root.start + 0x0000, 0x1000, "SYSRAM 1",
+ IORESOURCE_SYSTEM_RAM);
+ res[1] = DEFINE_RES_NAMED(root.start + 0x1000, 0x1000, "OTHER", 0);
+
+ res[2] = DEFINE_RES_NAMED(root.start + 0x3000, 0x1000, "NESTED", 0);
+ res[3] = DEFINE_RES_NAMED(root.start + 0x3800, 0x0400, "SYSRAM 2",
+ IORESOURCE_SYSTEM_RAM);
+
+ res[4] = DEFINE_RES_NAMED(root.start + 0x4000, 0x1000, "SYSRAM 3",
+ IORESOURCE_SYSTEM_RAM);
+
+ KUNIT_EXPECT_EQ(test, 0, request_resource(&root, &res[0]));
+ KUNIT_EXPECT_EQ(test, 0, request_resource(&root, &res[1]));
+ KUNIT_EXPECT_EQ(test, 0, request_resource(&root, &res[2]));
+ KUNIT_EXPECT_EQ(test, 0, request_resource(&res[2], &res[3]));
+ KUNIT_EXPECT_EQ(test, 0, request_resource(&root, &res[4]));
+
+ /* walk the entire region */
+ count = 0;
+ walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_SYSTEM_RAM,
+ root.start, root.end, &count, resource_walk_count);
+ KUNIT_EXPECT_EQ(test, count, 3);
+
+ /* walk the region requested by res[1] */
+ count = 0;
+ walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_SYSTEM_RAM,
+ res[1].start, res[1].end, &count, resource_walk_count);
+ KUNIT_EXPECT_EQ(test, count, 0);
+
+ /* walk the region requested by res[2] */
+ count = 0;
+ walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_SYSTEM_RAM,
+ res[2].start, res[2].end, &count, resource_walk_count);
+ KUNIT_EXPECT_EQ(test, count, 1);
+
+ /* walk the region requested by res[4] */
+ count = 0;
+ walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_SYSTEM_RAM,
+ res[4].start, res[4].end, &count, resource_walk_count);
+ KUNIT_EXPECT_EQ(test, count, 1);
+
+ release_resource(&root);
+}
+
static struct kunit_case resource_test_cases[] = {
KUNIT_CASE(resource_test_union),
KUNIT_CASE(resource_test_intersection),
+ KUNIT_CASE(resource_test_walk_iomem_res_desc),
{}
};
--
2.45.1.467.gbab1589fc0-goog
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] resource: add a simple test for walk_iomem_res_desc()
2024-06-05 19:53 [PATCH v3] resource: add a simple test for walk_iomem_res_desc() Chia-I Wu
@ 2024-06-05 20:33 ` Andy Shevchenko
2024-06-05 21:32 ` Chia-I Wu
0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2024-06-05 20:33 UTC (permalink / raw)
To: Chia-I Wu
Cc: Greg Kroah-Hartman, AKASHI Takahiro, Baoquan He, Dan Williams,
Alison Schofield, linux-kernel
On Wed, Jun 05, 2024 at 12:53:10PM -0700, Chia-I Wu wrote:
> This mainly tests that find_next_iomem_res() does not miss resources.
...
> v2: update subject, use DEFINE_RES_NAMED and hardcoded offsets
> v3: really hardcode offsets
This doesn't explain why you multiplied previous values.
...
> +static int resource_walk_count(struct resource *res, void *data)
> +{
> + int *count = data;
+ Blank line.
> + (*count)++;
> + return 0;
> +}
...
> +static void resource_test_walk_iomem_res_desc(struct kunit *test)
> +{
> + struct resource root = {
> + .name = "Resource Walk Test",
> + };
> + struct resource res[8];
> + int count;
> +
> + KUNIT_ASSERT_EQ(test, 0,
> + allocate_resource(&iomem_resource, &root, 0x100000,
> + 0, ~0, 0x100000, NULL, NULL));
Shouldn't this use RESOURCE_SIZE_MAX?
Please, split the assertion and allocate_resource() call, so it becomes
readable what exactly you checked against.
> + /* build the resource tree */
> + res[0] = DEFINE_RES_NAMED(root.start + 0x0000, 0x1000, "SYSRAM 1",
> + IORESOURCE_SYSTEM_RAM);
> + res[1] = DEFINE_RES_NAMED(root.start + 0x1000, 0x1000, "OTHER", 0);
> +
> + res[2] = DEFINE_RES_NAMED(root.start + 0x3000, 0x1000, "NESTED", 0);
> + res[3] = DEFINE_RES_NAMED(root.start + 0x3800, 0x0400, "SYSRAM 2",
> + IORESOURCE_SYSTEM_RAM);
> +
> + res[4] = DEFINE_RES_NAMED(root.start + 0x4000, 0x1000, "SYSRAM 3",
> + IORESOURCE_SYSTEM_RAM);
> +
> + KUNIT_EXPECT_EQ(test, 0, request_resource(&root, &res[0]));
> + KUNIT_EXPECT_EQ(test, 0, request_resource(&root, &res[1]));
> + KUNIT_EXPECT_EQ(test, 0, request_resource(&root, &res[2]));
> + KUNIT_EXPECT_EQ(test, 0, request_resource(&res[2], &res[3]));
> + KUNIT_EXPECT_EQ(test, 0, request_resource(&root, &res[4]));
> +
> + /* walk the entire region */
> + count = 0;
> + walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_SYSTEM_RAM,
> + root.start, root.end, &count, resource_walk_count);
> + KUNIT_EXPECT_EQ(test, count, 3);
> +
> + /* walk the region requested by res[1] */
> + count = 0;
> + walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_SYSTEM_RAM,
> + res[1].start, res[1].end, &count, resource_walk_count);
> + KUNIT_EXPECT_EQ(test, count, 0);
> +
> + /* walk the region requested by res[2] */
> + count = 0;
> + walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_SYSTEM_RAM,
> + res[2].start, res[2].end, &count, resource_walk_count);
> + KUNIT_EXPECT_EQ(test, count, 1);
> +
> + /* walk the region requested by res[4] */
> + count = 0;
> + walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_SYSTEM_RAM,
> + res[4].start, res[4].end, &count, resource_walk_count);
> + KUNIT_EXPECT_EQ(test, count, 1);
> +
> + release_resource(&root);
> +}
Other than the above, LGTM. Hopefully next version will be ready to apply.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] resource: add a simple test for walk_iomem_res_desc()
2024-06-05 20:33 ` Andy Shevchenko
@ 2024-06-05 21:32 ` Chia-I Wu
0 siblings, 0 replies; 3+ messages in thread
From: Chia-I Wu @ 2024-06-05 21:32 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, AKASHI Takahiro, Baoquan He, Dan Williams,
Alison Schofield, linux-kernel
On Wed, Jun 5, 2024 at 1:33 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Jun 05, 2024 at 12:53:10PM -0700, Chia-I Wu wrote:
> > This mainly tests that find_next_iomem_res() does not miss resources.
>
> ...
>
> > v2: update subject, use DEFINE_RES_NAMED and hardcoded offsets
> > v3: really hardcode offsets
>
> This doesn't explain why you multiplied previous values.
They were quite randomly picked. They were changed because multiples
of 0x1000 are easier to read than multiples of 0x400.
All suggestions have been incorporated in v4.
>
> ...
>
> > +static int resource_walk_count(struct resource *res, void *data)
> > +{
> > + int *count = data;
>
> + Blank line.
>
> > + (*count)++;
> > + return 0;
> > +}
>
> ...
>
> > +static void resource_test_walk_iomem_res_desc(struct kunit *test)
> > +{
> > + struct resource root = {
> > + .name = "Resource Walk Test",
> > + };
> > + struct resource res[8];
> > + int count;
> > +
> > + KUNIT_ASSERT_EQ(test, 0,
> > + allocate_resource(&iomem_resource, &root, 0x100000,
> > + 0, ~0, 0x100000, NULL, NULL));
>
> Shouldn't this use RESOURCE_SIZE_MAX?
>
> Please, split the assertion and allocate_resource() call, so it becomes
> readable what exactly you checked against.
>
> > + /* build the resource tree */
> > + res[0] = DEFINE_RES_NAMED(root.start + 0x0000, 0x1000, "SYSRAM 1",
> > + IORESOURCE_SYSTEM_RAM);
> > + res[1] = DEFINE_RES_NAMED(root.start + 0x1000, 0x1000, "OTHER", 0);
> > +
> > + res[2] = DEFINE_RES_NAMED(root.start + 0x3000, 0x1000, "NESTED", 0);
> > + res[3] = DEFINE_RES_NAMED(root.start + 0x3800, 0x0400, "SYSRAM 2",
> > + IORESOURCE_SYSTEM_RAM);
> > +
> > + res[4] = DEFINE_RES_NAMED(root.start + 0x4000, 0x1000, "SYSRAM 3",
> > + IORESOURCE_SYSTEM_RAM);
> > +
> > + KUNIT_EXPECT_EQ(test, 0, request_resource(&root, &res[0]));
> > + KUNIT_EXPECT_EQ(test, 0, request_resource(&root, &res[1]));
> > + KUNIT_EXPECT_EQ(test, 0, request_resource(&root, &res[2]));
> > + KUNIT_EXPECT_EQ(test, 0, request_resource(&res[2], &res[3]));
> > + KUNIT_EXPECT_EQ(test, 0, request_resource(&root, &res[4]));
> > +
> > + /* walk the entire region */
> > + count = 0;
> > + walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_SYSTEM_RAM,
> > + root.start, root.end, &count, resource_walk_count);
> > + KUNIT_EXPECT_EQ(test, count, 3);
> > +
> > + /* walk the region requested by res[1] */
> > + count = 0;
> > + walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_SYSTEM_RAM,
> > + res[1].start, res[1].end, &count, resource_walk_count);
> > + KUNIT_EXPECT_EQ(test, count, 0);
> > +
> > + /* walk the region requested by res[2] */
> > + count = 0;
> > + walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_SYSTEM_RAM,
> > + res[2].start, res[2].end, &count, resource_walk_count);
> > + KUNIT_EXPECT_EQ(test, count, 1);
> > +
> > + /* walk the region requested by res[4] */
> > + count = 0;
> > + walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_SYSTEM_RAM,
> > + res[4].start, res[4].end, &count, resource_walk_count);
> > + KUNIT_EXPECT_EQ(test, count, 1);
> > +
> > + release_resource(&root);
> > +}
>
> Other than the above, LGTM. Hopefully next version will be ready to apply.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-06-05 21:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-05 19:53 [PATCH v3] resource: add a simple test for walk_iomem_res_desc() Chia-I Wu
2024-06-05 20:33 ` Andy Shevchenko
2024-06-05 21:32 ` Chia-I Wu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®