ASoC: rsnd: Drop obsolete dependency on COMPILE_TEST

Message ID 20221127193441.0b54484d@endymion.delvare
State New
Headers
Series ASoC: rsnd: Drop obsolete dependency on COMPILE_TEST |

Commit Message

Jean Delvare Nov. 27, 2022, 6:34 p.m. UTC
  Since commit 0166dc11be91 ("of: make CONFIG_OF user selectable"), it
is possible to test-build any driver which depends on OF on any
architecture by explicitly selecting OF. Therefore depending on
COMPILE_TEST as an alternative is no longer needed.

It is actually better to always build such drivers with OF enabled,
so that the test builds are closer to how each driver will actually be
built on its intended target. Building them without OF may not test
much as the compiler will optimize out potentially large parts of the
code. In the worst case, this could even pop false positive warnings.
Dropping COMPILE_TEST here improves the quality of our testing and
avoids wasting time on non-existent issues.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
---
 sound/soc/sh/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Mark Brown Nov. 28, 2022, 12:33 p.m. UTC | #1
On Sun, Nov 27, 2022 at 07:34:41PM +0100, Jean Delvare wrote:

> It is actually better to always build such drivers with OF enabled,
> so that the test builds are closer to how each driver will actually be
> built on its intended target. Building them without OF may not test
> much as the compiler will optimize out potentially large parts of the
> code. In the worst case, this could even pop false positive warnings.
> Dropping COMPILE_TEST here improves the quality of our testing and
> avoids wasting time on non-existent issues.

As ever building without OF does not preclude building with OF.
  
Jean Delvare Nov. 28, 2022, 1:56 p.m. UTC | #2
Hi Mark,

On Mon, 28 Nov 2022 12:33:35 +0000, Mark Brown wrote:
> On Sun, Nov 27, 2022 at 07:34:41PM +0100, Jean Delvare wrote:
> > It is actually better to always build such drivers with OF enabled,
> > so that the test builds are closer to how each driver will actually be
> > built on its intended target. Building them without OF may not test
> > much as the compiler will optimize out potentially large parts of the
> > code. In the worst case, this could even pop false positive warnings.
> > Dropping COMPILE_TEST here improves the quality of our testing and
> > avoids wasting time on non-existent issues.  
> 
> As ever building without OF does not preclude building with OF.

I'm sorry, I'm not sure I understand what point you are trying to make
here.

Of course you can build a kernel with and without OF, and without my
patch, you could build the driver with and without OF. My point is that
there is no value in allowing that.

There are 2 use cases for COMPILE_TEST. The first use case is kernel
developers who make changes to a driver and want to be able to
test-build it. Now they can just enable OF and they will be able to
test-build the driver (and a better version of it, as explained in my
patch description). It is no different from enabling I2C if you need to
test-build an I2C driver, or enabling SPI if you need to test-build an
SPI driver, etc.

The second use case is the compilation farms. These will typically run
pre-defined real kernel configurations or allmodconfig or randconfig.
The first two options are not really affected by this change, only
randconfig is.

For randconfig, the limiting factor is the build power of the farm. So,
in a way, yes building without OF does preclude building with OF,
because you can test only one combination of options at once. Whenever
you build your driver without OF, you are wasting an opportunity to
build it with OF instead, which would test the code as it will actually
be used on its intended target, and thus is a better test.

You may argue that statistically, randconfig will select the driver
more often if it depends on OF || COMPILE_TEST rather than just OF.
That's true, but it's a matter of quantity versus quality. Would you
rather test build the code twice in its crippled form, which may
trigger false-positive warnings or hide actual warnings, or just once
in its proper form, where all warnings and build failures are real? I
definitely believe the latter is a better use of our resources.

Thanks,
  
Mark Brown Nov. 28, 2022, 4:39 p.m. UTC | #3
On Sun, 27 Nov 2022 19:34:41 +0100, Jean Delvare wrote:
> Since commit 0166dc11be91 ("of: make CONFIG_OF user selectable"), it
> is possible to test-build any driver which depends on OF on any
> architecture by explicitly selecting OF. Therefore depending on
> COMPILE_TEST as an alternative is no longer needed.
> 
> It is actually better to always build such drivers with OF enabled,
> so that the test builds are closer to how each driver will actually be
> built on its intended target. Building them without OF may not test
> much as the compiler will optimize out potentially large parts of the
> code. In the worst case, this could even pop false positive warnings.
> Dropping COMPILE_TEST here improves the quality of our testing and
> avoids wasting time on non-existent issues.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: rsnd: Drop obsolete dependency on COMPILE_TEST
      commit: d695d089e35e28f3f0ed4595a242922cc28f9b20

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
  
Mark Brown Nov. 28, 2022, 5:56 p.m. UTC | #4
On Mon, Nov 28, 2022 at 02:56:12PM +0100, Jean Delvare wrote:
> On Mon, 28 Nov 2022 12:33:35 +0000, Mark Brown wrote:
> > On Sun, Nov 27, 2022 at 07:34:41PM +0100, Jean Delvare wrote:

> > > It is actually better to always build such drivers with OF enabled,
> > > so that the test builds are closer to how each driver will actually be
> > > built on its intended target. Building them without OF may not test
> > > much as the compiler will optimize out potentially large parts of the
> > > code. In the worst case, this could even pop false positive warnings.
> > > Dropping COMPILE_TEST here improves the quality of our testing and
> > > avoids wasting time on non-existent issues.  

> > As ever building without OF does not preclude building with OF.

> I'm sorry, I'm not sure I understand what point you are trying to make
> here.

You're overselling what the change does here in a way that's getting a
bit silly.  It's just cutting down the amount of stuff the randconfig
people do, that's all.  It's not particularly bad to compile without the
DT support, I suppose you could argue that it's preserving our ability
to work with other firmware interfaces although that's a bit of a push
(but then a lot of the stuff generated by randconfig is in a similar
ballpark of course).  The whole point with COMPILE_TEST is that it's
enabling unrealistic things that probably aren't practically useful.

> That's true, but it's a matter of quantity versus quality. Would you
> rather test build the code twice in its crippled form, which may
> trigger false-positive warnings or hide actual warnings, or just once
> in its proper form, where all warnings and build failures are real? I
> definitely believe the latter is a better use of our resources.

I'm not saying don't do the change, I'm saying don't oversell it.
  

Patch

--- linux-6.0.orig/sound/soc/sh/Kconfig
+++ linux-6.0/sound/soc/sh/Kconfig
@@ -39,7 +39,7 @@  config SND_SOC_SH4_SIU
 config SND_SOC_RCAR
 	tristate "R-Car series SRU/SCU/SSIU/SSI support"
 	depends on COMMON_CLK
-	depends on OF || COMPILE_TEST
+	depends on OF
 	select SND_SIMPLE_CARD_UTILS
 	select REGMAP_MMIO
 	help