[PATCH v2 0/5] ASoC: Improve coverage in default KUnit runs
We have some KUnit tests for ASoC but they're not being run as much as they should be since ASoC isn't enabled in the configs used by default with KUnit and in the case of the topology tests there is no way to enable them without enabling drivers that use them. This series provides a Kconfig option which KUnit can use directly rather than worry about drivers.
Further, since KUnit is typically run in UML but ALSA prevents build with UML we need to remove that Kconfig conflict. As far as I can tell the motiviation for this is that many ALSA drivers use iomem APIs which are not available under UML and it's more trouble than it's worth to go through and add per driver dependencies. In order to avoid these issues we also provide stubs for these APIs so there are no build time issues if a driver relies on iomem but does not depend on it. With these stubs I am able to build all the sound drivers available in a UML defconfig (UML allmodconfig appears to have substantial other issues in a quick test).
With this series I am able to run the topology KUnit tests as part of a kunit --alltests run.
Signed-off-by: Mark Brown broonie@kernel.org --- Changes in v2: - Add support for building ALSA with UML. - Link to v1: https://lore.kernel.org/r/20230712-asoc-topology-kunit-enable-v1-0-b9f2da9dc...
--- Mark Brown (5): driver core: Provide stubs for !IOMEM builds platform: Provide stubs for !HAS_IOMEM builds ALSA: Enable build with UML kunit: Enable ASoC in all_tests.config ASoC: topology: Add explicit build option
include/linux/device.h | 26 ++++++++++++++++++++++++++ include/linux/platform_device.h | 28 ++++++++++++++++++++++++++++ sound/Kconfig | 4 ---- sound/soc/Kconfig | 11 +++++++++++ tools/testing/kunit/configs/all_tests.config | 5 +++++ 5 files changed, 70 insertions(+), 4 deletions(-) --- base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 change-id: 20230701-asoc-topology-kunit-enable-5e8dd50d0ed7
Best regards,
The various _ioremap_resource functions are not built when CONFIG_HAS_IOMEM is disabled but no stubs are provided. Given how widespread IOMEM usage is in drivers and how rare !IOMEM configurations are in practical use let's just provide some stubs so users will build without having to add explicit dependencies on HAS_IOMEM.
The most likely use case is builds with UML for KUnit testing.
Signed-off-by: Mark Brown broonie@kernel.org --- include/linux/device.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/include/linux/device.h b/include/linux/device.h index bbaeabd04b0d..6731d7dc1a2a 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -349,6 +349,7 @@ unsigned long devm_get_free_pages(struct device *dev, gfp_t gfp_mask, unsigned int order); void devm_free_pages(struct device *dev, unsigned long addr);
+#ifdef CONFIG_HAS_IOMEM void __iomem *devm_ioremap_resource(struct device *dev, const struct resource *res); void __iomem *devm_ioremap_resource_wc(struct device *dev, @@ -357,6 +358,31 @@ void __iomem *devm_ioremap_resource_wc(struct device *dev, void __iomem *devm_of_iomap(struct device *dev, struct device_node *node, int index, resource_size_t *size); +#else + +static inline +void __iomem *devm_ioremap_resource(struct device *dev, + const struct resource *res) +{ + return ERR_PTR(-EINVAL); +} + +static inline +void __iomem *devm_ioremap_resource_wc(struct device *dev, + const struct resource *res) +{ + return ERR_PTR(-EINVAL); +} + +static inline +void __iomem *devm_of_iomap(struct device *dev, + struct device_node *node, int index, + resource_size_t *size) +{ + return ERR_PTR(-EINVAL); +} + +#endif
/* allows to add/remove a custom action to devres stack */ void devm_remove_action(struct device *dev, void (*action)(void *), void *data);
On Tue, 18 Jul 2023 at 08:29, Mark Brown broonie@kernel.org wrote:
The various _ioremap_resource functions are not built when CONFIG_HAS_IOMEM is disabled but no stubs are provided. Given how widespread IOMEM usage is in drivers and how rare !IOMEM configurations are in practical use let's just provide some stubs so users will build without having to add explicit dependencies on HAS_IOMEM.
The most likely use case is builds with UML for KUnit testing.
Signed-off-by: Mark Brown broonie@kernel.org
This is really nice, thanks: we've definitely wasted^W spent a lot of time adding the appropriate IOMEM dependencies when trying to setup KUnit configs, so I'm looking forward to not worrying about that again.
We have considered implementing fake versions of these specifically aimed at testing, and are looking into the PCI-over-virtio implementation in UML's LOGIC_IOMEM feature, which allows redirecting IOMEM accesses to something more test-friendly.
Neither of those conflict with this as a fallback, though, and I'm all for it.
Reviewed-by: David Gow davidgow@google.com
Cheers, -- David
include/linux/device.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/include/linux/device.h b/include/linux/device.h index bbaeabd04b0d..6731d7dc1a2a 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -349,6 +349,7 @@ unsigned long devm_get_free_pages(struct device *dev, gfp_t gfp_mask, unsigned int order); void devm_free_pages(struct device *dev, unsigned long addr);
+#ifdef CONFIG_HAS_IOMEM void __iomem *devm_ioremap_resource(struct device *dev, const struct resource *res); void __iomem *devm_ioremap_resource_wc(struct device *dev, @@ -357,6 +358,31 @@ void __iomem *devm_ioremap_resource_wc(struct device *dev, void __iomem *devm_of_iomap(struct device *dev, struct device_node *node, int index, resource_size_t *size); +#else
+static inline +void __iomem *devm_ioremap_resource(struct device *dev,
const struct resource *res)+{
return ERR_PTR(-EINVAL);+}
+static inline +void __iomem *devm_ioremap_resource_wc(struct device *dev,
const struct resource *res)+{
return ERR_PTR(-EINVAL);+}
+static inline +void __iomem *devm_of_iomap(struct device *dev,
struct device_node *node, int index,resource_size_t *size)+{
return ERR_PTR(-EINVAL);+}
+#endif
/* allows to add/remove a custom action to devres stack */ void devm_remove_action(struct device *dev, void (*action)(void *), void *data);
-- 2.39.2
On Tue, Jul 18, 2023 at 01:28:42AM +0100, Mark Brown wrote:
The various _ioremap_resource functions are not built when CONFIG_HAS_IOMEM is disabled but no stubs are provided. Given how widespread IOMEM usage is in drivers and how rare !IOMEM configurations are in practical use let's just provide some stubs so users will build without having to add explicit dependencies on HAS_IOMEM.
The most likely use case is builds with UML for KUnit testing.
Signed-off-by: Mark Brown broonie@kernel.org
include/linux/device.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
Reviewed-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
The various _ioremap_resource functions are not built when CONFIG_HAS_IOMEM is disabled but no stubs are provided. Given how widespread IOMEM usage is in drivers and how rare !IOMEM configurations are in practical use let's just provide some stubs so users will build without having to add explicit dependencies on IOMEM.
The most likely use case is builds with UML for KUnit testing.
Signed-off-by: Mark Brown broonie@kernel.org --- include/linux/platform_device.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index b845fd83f429..7a41c72c1959 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -63,6 +63,8 @@ extern struct resource *platform_get_mem_or_io(struct platform_device *, extern struct device * platform_find_device_by_driver(struct device *start, const struct device_driver *drv); + +#ifdef CONFIG_HAS_IOMEM extern void __iomem * devm_platform_get_and_ioremap_resource(struct platform_device *pdev, unsigned int index, struct resource **res); @@ -72,6 +74,32 @@ devm_platform_ioremap_resource(struct platform_device *pdev, extern void __iomem * devm_platform_ioremap_resource_byname(struct platform_device *pdev, const char *name); +#else + +static inline void __iomem * +devm_platform_get_and_ioremap_resource(struct platform_device *pdev, + unsigned int index, struct resource **res) +{ + return ERR_PTR(-EINVAL); +} + + +static inline void __iomem * +devm_platform_ioremap_resource(struct platform_device *pdev, + unsigned int index) +{ + return ERR_PTR(-EINVAL); +} + +static inline void __iomem * +devm_platform_ioremap_resource_byname(struct platform_device *pdev, + const char *name) +{ + return ERR_PTR(-EINVAL); +} + +#endif + extern int platform_get_irq(struct platform_device *, unsigned int); extern int platform_get_irq_optional(struct platform_device *, unsigned int); extern int platform_irq_count(struct platform_device *);
On Tue, 18 Jul 2023 at 08:30, Mark Brown broonie@kernel.org wrote:
The various _ioremap_resource functions are not built when CONFIG_HAS_IOMEM is disabled but no stubs are provided. Given how widespread IOMEM usage is in drivers and how rare !IOMEM configurations are in practical use let's just provide some stubs so users will build without having to add explicit dependencies on IOMEM.
The most likely use case is builds with UML for KUnit testing.
Signed-off-by: Mark Brown broonie@kernel.org
As with the previous patch, I'm all for this.
Reviewed-by: David Gow davidgow@google.com
Cheers, -- David
include/linux/platform_device.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index b845fd83f429..7a41c72c1959 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -63,6 +63,8 @@ extern struct resource *platform_get_mem_or_io(struct platform_device *, extern struct device * platform_find_device_by_driver(struct device *start, const struct device_driver *drv);
+#ifdef CONFIG_HAS_IOMEM extern void __iomem * devm_platform_get_and_ioremap_resource(struct platform_device *pdev, unsigned int index, struct resource **res); @@ -72,6 +74,32 @@ devm_platform_ioremap_resource(struct platform_device *pdev, extern void __iomem * devm_platform_ioremap_resource_byname(struct platform_device *pdev, const char *name); +#else
+static inline void __iomem * +devm_platform_get_and_ioremap_resource(struct platform_device *pdev,
unsigned int index, struct resource **res)+{
return ERR_PTR(-EINVAL);+}
+static inline void __iomem * +devm_platform_ioremap_resource(struct platform_device *pdev,
unsigned int index)+{
return ERR_PTR(-EINVAL);+}
+static inline void __iomem * +devm_platform_ioremap_resource_byname(struct platform_device *pdev,
const char *name)+{
return ERR_PTR(-EINVAL);+}
+#endif
extern int platform_get_irq(struct platform_device *, unsigned int); extern int platform_get_irq_optional(struct platform_device *, unsigned int); extern int platform_irq_count(struct platform_device *);
-- 2.39.2
On Tue, Jul 18, 2023 at 01:28:43AM +0100, Mark Brown wrote:
The various _ioremap_resource functions are not built when CONFIG_HAS_IOMEM is disabled but no stubs are provided. Given how widespread IOMEM usage is in drivers and how rare !IOMEM configurations are in practical use let's just provide some stubs so users will build without having to add explicit dependencies on IOMEM.
The most likely use case is builds with UML for KUnit testing.
Signed-off-by: Mark Brown broonie@kernel.org
include/linux/platform_device.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
Reviewed-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
In order to facilitate testing using KUnit allow ALSA to build with UML, it's not super useful at runtime but that's a user problem rather than an actual dependency. The apparent reason for the dependency was the widespread use of iomem APIs in ALSA drivers, earlier patches in this series have provided stubs for these APIs so that there are no build time issues even without individual drivers having IOMEM dependencies added.
Signed-off-by: Mark Brown broonie@kernel.org --- sound/Kconfig | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/sound/Kconfig b/sound/Kconfig index 0ddfb717b81d..f0e15822e858 100644 --- a/sound/Kconfig +++ b/sound/Kconfig @@ -39,8 +39,6 @@ config SOUND_OSS_CORE_PRECLAIM
source "sound/oss/dmasound/Kconfig"
-if !UML - menuconfig SND tristate "Advanced Linux Sound Architecture" help @@ -103,8 +101,6 @@ source "sound/virtio/Kconfig"
endif # SND
-endif # !UML - endif # SOUND
config AC97_BUS
On Tue, 18 Jul 2023 at 08:30, Mark Brown broonie@kernel.org wrote:
In order to facilitate testing using KUnit allow ALSA to build with UML, it's not super useful at runtime but that's a user problem rather than an actual dependency. The apparent reason for the dependency was the widespread use of iomem APIs in ALSA drivers, earlier patches in this series have provided stubs for these APIs so that there are no build time issues even without individual drivers having IOMEM dependencies added.
Signed-off-by: Mark Brown broonie@kernel.org
This works well here, and I'd love to see it go through. I'll leave it to the ALSA folks in case there are other issues with UML they're worried about, though.
Tested-by: David Gow davidgow@google.com
Cheers, -- David
sound/Kconfig | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/sound/Kconfig b/sound/Kconfig index 0ddfb717b81d..f0e15822e858 100644 --- a/sound/Kconfig +++ b/sound/Kconfig @@ -39,8 +39,6 @@ config SOUND_OSS_CORE_PRECLAIM
source "sound/oss/dmasound/Kconfig"
-if !UML
menuconfig SND tristate "Advanced Linux Sound Architecture" help @@ -103,8 +101,6 @@ source "sound/virtio/Kconfig"
endif # SND
-endif # !UML
endif # SOUND
config AC97_BUS
-- 2.39.2
There are KUnit tests for some of the ASoC utility functions which are not enabled in the KUnit all_tests.config, do so.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/kunit/configs/all_tests.config | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/tools/testing/kunit/configs/all_tests.config b/tools/testing/kunit/configs/all_tests.config index 0393940c706a..13d15bc693fb 100644 --- a/tools/testing/kunit/configs/all_tests.config +++ b/tools/testing/kunit/configs/all_tests.config @@ -35,3 +35,7 @@ CONFIG_DAMON_DBGFS=y
CONFIG_SECURITY=y CONFIG_SECURITY_APPARMOR=y + +CONFIG_SOUND=y +CONFIG_SND=y +CONFIG_SND_SOC=y
On Tue, 18 Jul 2023 at 08:30, Mark Brown broonie@kernel.org wrote:
There are KUnit tests for some of the ASoC utility functions which are not enabled in the KUnit all_tests.config, do so.
Signed-off-by: Mark Brown broonie@kernel.org
If the other patches go through, this is great for me. (I've already used it to test some in-progress changes with the struct device handling.)
I'm happy to take this via the KUnit tree, but if it's easier to take all of these via ALSA or another tree, let me know.
Reviewed-by: David Gow davidgow@google.com
Cheers, -- David
tools/testing/kunit/configs/all_tests.config | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/tools/testing/kunit/configs/all_tests.config b/tools/testing/kunit/configs/all_tests.config index 0393940c706a..13d15bc693fb 100644 --- a/tools/testing/kunit/configs/all_tests.config +++ b/tools/testing/kunit/configs/all_tests.config @@ -35,3 +35,7 @@ CONFIG_DAMON_DBGFS=y
CONFIG_SECURITY=y CONFIG_SECURITY_APPARMOR=y
+CONFIG_SOUND=y +CONFIG_SND=y +CONFIG_SND_SOC=y
-- 2.39.2
On Tue, Jul 18, 2023 at 05:03:41PM +0800, David Gow wrote:
I'm happy to take this via the KUnit tree, but if it's easier to take all of these via ALSA or another tree, let me know.
I was hoping to do a bit more KUnit coverage for ASoC so it'd be kind of handy to have them in the ASoC tree to make life easier there, though it's an open question if I actually get round to doing that before the next merge window. It looks like everyone is OK with me applying them so I'll queue them for my CI, please shout if that's an issue.
The default KUnit build options are not supposed to enable any subsystems that were not already enabled but the topology code is a library which is generally selected by drivers that want to use it. Since KUnit is frequently run in virtual environments with minimal driver support this makes it difficult to enable the toplogy tests so provide an explicit Kconfig option which can be directly enabled when using KUnit, and also include this in the KUnit all_tests.config.
Reviewed-by: David Gow davidgow@google.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/Kconfig | 11 +++++++++++ tools/testing/kunit/configs/all_tests.config | 1 + 2 files changed, 12 insertions(+)
diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig index bfa9622e1ab1..439fa631c342 100644 --- a/sound/soc/Kconfig +++ b/sound/soc/Kconfig @@ -38,6 +38,17 @@ config SND_SOC_TOPOLOGY bool select SND_DYNAMIC_MINORS
+config SND_SOC_TOPOLOGY_BUILD + bool "Build topology core" + select SND_SOC_TOPOLOGY + depends on KUNIT + help + This option exists to facilitate running the KUnit tests for + the topology core, KUnit is frequently tested in virtual + environments with minimal drivers enabled but the topology + core is usually selected by drivers. There is little reason + to enable it if not doing a KUnit build. + config SND_SOC_TOPOLOGY_KUNIT_TEST tristate "KUnit tests for SoC topology" depends on KUNIT diff --git a/tools/testing/kunit/configs/all_tests.config b/tools/testing/kunit/configs/all_tests.config index 13d15bc693fb..b8adb59455ef 100644 --- a/tools/testing/kunit/configs/all_tests.config +++ b/tools/testing/kunit/configs/all_tests.config @@ -39,3 +39,4 @@ CONFIG_SECURITY_APPARMOR=y CONFIG_SOUND=y CONFIG_SND=y CONFIG_SND_SOC=y +CONFIG_SND_SOC_TOPOLOGY_BUILD=y
On Tue, 18 Jul 2023 02:28:41 +0200, Mark Brown wrote:
We have some KUnit tests for ASoC but they're not being run as much as they should be since ASoC isn't enabled in the configs used by default with KUnit and in the case of the topology tests there is no way to enable them without enabling drivers that use them. This series provides a Kconfig option which KUnit can use directly rather than worry about drivers.
Further, since KUnit is typically run in UML but ALSA prevents build with UML we need to remove that Kconfig conflict. As far as I can tell the motiviation for this is that many ALSA drivers use iomem APIs which are not available under UML and it's more trouble than it's worth to go through and add per driver dependencies. In order to avoid these issues we also provide stubs for these APIs so there are no build time issues if a driver relies on iomem but does not depend on it. With these stubs I am able to build all the sound drivers available in a UML defconfig (UML allmodconfig appears to have substantial other issues in a quick test).
With this series I am able to run the topology KUnit tests as part of a kunit --alltests run.
Signed-off-by: Mark Brown broonie@kernel.org
Changes in v2:
- Add support for building ALSA with UML.
- Link to v1: https://lore.kernel.org/r/20230712-asoc-topology-kunit-enable-v1-0-b9f2da9dc...
Mark Brown (5): driver core: Provide stubs for !IOMEM builds platform: Provide stubs for !HAS_IOMEM builds ALSA: Enable build with UML kunit: Enable ASoC in all_tests.config ASoC: topology: Add explicit build option
Those look like sensible changes.
Reviewed-by: Takashi Iwai tiwai@suse.de
thanks,
Takashi
On Tue, 18 Jul 2023 01:28:41 +0100, Mark Brown wrote:
We have some KUnit tests for ASoC but they're not being run as much as they should be since ASoC isn't enabled in the configs used by default with KUnit and in the case of the topology tests there is no way to enable them without enabling drivers that use them. This series provides a Kconfig option which KUnit can use directly rather than worry about drivers.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/5] driver core: Provide stubs for !IOMEM builds commit: da7c07b1083809888c82522e74370f962fb7685e [2/5] platform: Provide stubs for !HAS_IOMEM builds commit: a0c74f6c9ea9cebd7a8f38142bf87e7c12c2905d [3/5] ALSA: Enable build with UML commit: 512d092d78823f9813f4af38090b33c454137a4c [4/5] kunit: Enable ASoC in all_tests.config commit: 5aaa4024e14f8b878a348338a74b4c97bc2478b1 [5/5] ASoC: topology: Add explicit build option commit: b7dc237ef8b0897f5750a738d2c57469909a6a15
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (4)
-
David Gow -
Greg Kroah-Hartman -
Mark Brown -
Takashi Iwai