Re: [PATCH v4 14/25] dmaengine: sdxi: Add unit tests for descriptor ring reservations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dmaengine: sdxi: unused mock KUnit device in setup_device()
--

commit 73b5f711479c1dbab69a55b0e9eb01abd747309e
Author: Nathan Lynch <nathan.lynch@amd.com>

dmaengine: sdxi: Add unit tests for descriptor ring reservations

This commit adds hardware-independent KUnit tests for SDXI descriptor ring
reservations, covering full-ring and single-slot reservations, invalid counts,
inconsistent indices, insufficient space, and bounded reservations across ring
wrap.

> diff --git a/drivers/dma/sdxi/ring_kunit.c b/drivers/dma/sdxi/ring_kunit.c
> new file mode 100644
> index 0000000000000..7df85fc5549c6
> --- /dev/null
> +++ b/drivers/dma/sdxi/ring_kunit.c
> @@ -0,0 +1,147 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * SDXI descriptor ring management tests.
> + *
> + * Copyright Advanced Micro Devices, Inc.
> + */
> +#include <kunit/device.h>
> +#include <kunit/test-bug.h>
> +#include <kunit/test.h>
> +#include <linux/container_of.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/module.h>
> +#include <linux/overflow.h>
> +#include <linux/packing.h>
> +#include <linux/string.h>
> +
> +#include "ring.h"
> +#include "submission.h"
> +
> +MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
> +
> +/* A submission queue backed by plain memory; no device or DMA required. */
> +static struct sdxi_sq *alloc_sq(struct kunit *t, u32 count)
> +{
> +	struct sdxi_sq *sq;
> +
> +	sq = kunit_kmalloc(t, struct_size(sq, ring, count),
> +			   GFP_KERNEL | __GFP_ZERO);
> +	if (sq)
> +		sq->count = count;
> +	return sq;
> +}

[ ... ]

> +static int setup_device(struct kunit *t)
> +{
> +	struct device *dev = kunit_device_register(t, "sdxi-mock-device");
> +
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(t, dev);
> +	t->priv = dev;
> +	return 0;
> +}
> +
> +static struct kunit_suite generic_desc_ts = {
> +	.name = "SDXI descriptor ring management",
> +	.test_cases = testcases,
> +	.init = setup_device,
> +};

[Severity: Low]
As noted in the comment above alloc_sq(), the submission queue is backed by
plain memory and does not require a struct device or DMA.

However, generic_desc_ts wires up setup_device() as its .init callback, which
registers "sdxi-mock-device" via kunit_device_register() and stores it in
t->priv before each test case (valid(), invalid(), and
bounded_reservations()), even though nothing in ring_kunit.c ever reads
t->priv.

Is setup_device() needed here, or can it (along with the unused
kunit/device.h, linux/container_of.h, linux/dma-mapping.h, linux/packing.h,
and linux/string.h includes) be dropped?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-sdxi-base-v4-0-3668ccc7aab8@amd.com?part=14