Re: [PATCH 2/2] spi: Add KUnit coverage for DMA mapping error paths

[Mark Brown]

On Wed, Aug 05, 2026 at 11:14:56PM +0800, Honghui Jiang wrote:
> Add KUnit tests for the __spi_map_msg() error paths.  The tests verify
> that mappings created before a later TX or RX failure are unwound, their
> flags are cleared, and cur_{tx,rx}_dma_dev point to the device used for
> the mapping.

The tests themselves look good but some style/integration stuff:

> --- /dev/null
> +++ b/drivers/spi/.kunitconfig
> @@ -0,0 +1,4 @@
> +CONFIG_KUNIT=y
> +CONFIG_SPI=y
> +CONFIG_SPI_MASTER=y
> +CONFIG_SPI_KUNIT_TEST=y

This also needs enabling in the KUnit defconfigs in
tools/testing/kunit/configs.

> index 05a852494..42e337c54 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -5146,3 +5146,7 @@ static int __init spi_init(void)
>   * include needing to have boardinfo data structures be much more public.
>   */
>  postcore_initcall(spi_init);
> +
> +#ifdef CONFIG_SPI_KUNIT_TEST
> +#include "tests/spi_kunit.c"
> +#endif

If this is just so we can peer into the internals it'd be better to add
a local header for internals and build as a separate translation unit
like normal rather than doing this sort of bodge.

> diff --git a/drivers/spi/tests/spi_kunit.c b/drivers/spi/tests/spi_kunit.c

Everywhere else we use - as a separator.

> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit tests for the SPI core DMA mapping error paths.
> + *

Please make the entire comment a C++ one so things look more intentional.

> + * The invariant under test:
> + *
> + *   When __spi_map_msg() returns an error, no transfer in the message may
> + *   still claim a DMA mapping.  Any transfer that was mapped before the
> + *   failure must have an empty SG table and a cleared *_sg_mapped flag.
> + *   ctlr->cur_{tx,rx}_dma_dev must identify the device used for this map,
> + *   rather than a device retained from an earlier message.

This is for one specific test, either this file should have a test
specific name with everything else namespaced to make room for further
tests or all this should go with the specific test.  This looks like a
DMA subsuite so possibly named after that?  The same applies to internal
identifiers.

Attachment: signature.asc
Description: PGP signature