[PATCH v3 11/15] ASoC: Intel: avs: Machine board registration

[Uwe Kleine-König]

On Sun, May 29, 2022 at 03:24:48PM +0200, Cezary Rojewski wrote:
> 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.

Hmm, I doubt this (i.e. unify the return value of __fls) will be merged
during this cycle. I'd want that to cook a bit in next before it hits
mainline, there might be similar problems introduced by changing __fls
to the one under discussion here.

A more short-term fix would be:

diff --git a/sound/soc/intel/avs/board_selection.c b/sound/soc/intel/avs/board_selection.c
--- 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));
+			num_ssps, mach->drv_name, (unsigned long)__fls(mach->mach_params.i2s_link_mask));
  		return -ENODEV;
  	}

i.e. explicitly cast the return value of __fls to unsigned long.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20220529/15d3b234/attachment.sig>