Re: [PATCH] ASoC: core: add snd_BUG_ON() check in devm_snd_soc_register_card()

[Cezary Rojewski <cezary.rojewski@intel.com>]

On 8/4/2026 5:19 AM, Bui Duc Phuc wrote:

>> snd_BUG_ON() translates to WARN_ON() in debug conditions.  After reading
>> the macro documentation, pr_xxx() or equivalents are recommended when
>> dealing with invalid arguments.  Same results after grepping for WARNs
>> in sound/soc or in general references such as driver/base/core.c - it's
>> clear to me the macro is not used as plain null-arg-check.
>>
> 
> I couldn't find anything in the snd_BUG_ON() documentation that
> explicitly says the
> macro should not be used for plain NULL argument checks.

My message reads: translates to WARN_ON() (..)
And WARN_ON's doc is what I meant.

> Also, there are quite a few places in the sound subsystem where
> WARN_ON() is used to
> validate NULL arguments, for example:
> 
> https://elixir.bootlin.com/linux/v7.2-rc5/source/sound/usb/endpoint.c#L857
> 
> https://elixir.bootlin.com/linux/v7.2-rc5/source/sound/soc/ti/davinci-mcasp.c#L2476
> 
> https://elixir.bootlin.com/linux/v7.2-rc5/source/sound/soc/meson/meson-codec-glue.c#L59

Nah, these are not plain null checks and if I missed something and any 
of them are, that's a patch candidate.

> and even in the Intel code that you maintain:
> 
> https://elixir.bootlin.com/linux/v7.2-rc5/source/sound/soc/intel/atom/sst-mfld-platform-pcm.c#L31

That's a clear example of when _not to_ use WARN_ON(), good catch. :)
Maintain != reviewed/acked. Many maintainers maintain code that they 
inherited and were not there when it was merged.

> That said, I'm also fine with not using snd_BUG_ON() here. Replacing
> it with a simple argument check such as:
> 
> if (!dev || !card) {
>        pr_err("invalid arguments\n");
>        return -EINVAL;
> }
> 
> is perfectly reasonable to me. It's not a big issue either way.

While not a fan, still better than WARN_ON().

>> Updating just one function with the check raises the question when
>> reading the file - Why just one devm_snd_xxx() has the check and the
>> rest do not?
>>
> 
> I think the remaining helper functions can be updated gradually over time.
> 
> To me, this discussion itself is a good example. You and I interpreted
> the intended use of snd_BUG_ON() differently.
> I don't think the macro is wrong; rather, its intended usage is not
> clearly documented or consistently reflected by existing code.

Not a fan of snd_BUG_ON() in general, I'd rather have it removed and 
WARN_ON(s) updated is one believes something is missing or wrong with them.

Anyhow, it's up to Mark. I'd avoid selecting just one function and 
filling it with null-arg-checks.