On 2022-05-29 7:48 AM, Uwe Kleine-König wrote:
Hello Mark,
On Thu, May 26, 2022 at 06:44:38PM +0100, Mark Brown wrote:
FWIW given how hard the 0-day reports have become to read I'd not rely
on people paying attention to things that are clearly pure build
coverage based off a 0-day report alone.
I'm unsure if I understand your sentiment correctly. Are you saying it
doesn't matter if a patch breaks the build on some arch using a
randconfig?
My position is: The commit under discussion (i.e. beed983621fb ("ASoC:
Intel: avs: Machine board registration")) breaks an allmodconfig build
on all platforms where __fls doesn't return a long int (i.e. arc, m68k,
and sparc). This actively hurts people who do build tests using
allmodconfig (or allyesconfig) for their patch series (e.g. me).
I agree that some reports by the 0-day bot are hard to parse. But still,
if there is a build problem someone should look into that. If you (with
your maintainer hat on) don't have the resource to do that, that's IMHO
fine. But I'd wish you'd push back on the patch submitter in that case
and don't apply the patch until the problem is resolved. If this is a
corner case randconfig issue, applying might be fine despite the build
error but breaking allmodconfig on 3 archs is bad.
The fix would be
Hello Uwe,
I don't believe anyone here wanted to break the build-configurations you
did mention above. I also believe it's important to mention that below
is not a fix but a W/A. Developer should be able to use __fls() if
required. Value returned by fls() differs from one returned by __fls(),
and using fls() in below context is misleading (hurts the debug-ability).
Yes, the faulty print should be devoid of __fls() until the function is
fixed for all the archs. It's too late for that and I'm sorry for any
inconvenience caused by the change.
To my knowledge the real fix has been provided on LKML by Amadeo [1] and
is under review since Friday. I'd kindly appreciate helping fix the root
cause of the problem. If there's anything missing in the series let us know.
[1]:
https://lore.kernel.org/lkml/20220527115345.2588775-3-amadeuszx.slawinski@xxxxxxxxxxxxxxx/T/
Regards,
Czarek
diff --git a/sound/soc/intel/avs/board_selection.c b/sound/soc/intel/avs/board_selection.c
index 80cb0164678a..f189f71b8e1e 100644
--- a/sound/soc/intel/avs/board_selection.c
+++ b/sound/soc/intel/avs/board_selection.c
@@ -325,8 +325,8 @@ static int avs_register_i2s_board(struct avs_dev *adev, struct snd_soc_acpi_mach
num_ssps = adev->hw_cfg.i2s_caps.ctrl_count;
if (fls(mach->mach_params.i2s_link_mask) > num_ssps) {
- dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
- num_ssps, mach->drv_name, __fls(mach->mach_params.i2s_link_mask));
+ dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%d\n",
+ num_ssps, mach->drv_name, fls(mach->mach_params.i2s_link_mask));
return -ENODEV;
}
which uses fls instead of __fls (as is done in the test triggering the
error). The former returns an int on all platforms.
Tell me if you don't want to squash this into beed983621fb and prefer a
formal patch.
Best regards
Uwe
