Re: [PATCH 01/27] ALSA: hda: Move widget capability macros into hdaudio.h

Cezary Rojewski
2025-07-14T08:41:05Z

On 2025-07-09 6:04 PM, Takashi Iwai wrote:
> The get_wcaps() and co are used not only by HD-audio core but also

What is 'co'? Meaning get_wpacs() and friends?

> other driver code, hence it'd be better to put into the common header
> instead of local.h.
>
> OTOH, there are macros of the same name like get_wcaps() that are
> still used in sound/pci/hda/* locally, and those conflict with each
> other. So we need to rename get_wcaps() (to be moved from hda-core)
> with the proper snd_hdac prefix for avoiding name conflicts, and
> define in the common hdaudio.h.

No problems found, just some nitpicks.

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>

>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
> include/sound/hdaudio.h | 24 ++++++++++++++++++++++++
> sound/hda/hdac_device.c | 8 ++++----
> sound/hda/hdac_sysfs.c | 14 +++++++-------
> sound/hda/local.h | 21 ---------------------
> sound/soc/codecs/hdac_hdmi.c | 21 ++++++++++-----------
> 5 files changed, 45 insertions(+), 43 deletions(-)
>
> diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
> index 25668eee65cf..d38234f8fe44 100644
> --- a/include/sound/hdaudio.h
> +++ b/include/sound/hdaudio.h
> @@ -680,6 +680,30 @@ static inline void snd_hdac_dsp_cleanup(struct hdac_stream *azx_dev,
> }
> #endif /* CONFIG_SND_HDA_DSP_LOADER */
>
> +/*
> + * Easy macros for widget capabilities
> + */

Nitpick: there is just one macro here.

TBH perhaps there is a better location for it - where all
snd_hdac_read_parm() could lie - but that's probably a subject for a
different tale.

> +#define snd_hdac_get_wcaps(codec, nid) \
> + snd_hdac_read_parm(codec, nid, AC_PAR_AUDIO_WIDGET_CAP)

In future wording for "parameter" could also be streamlined. Typically I
see 'param' in the code. "parm" is more of an outlier.

> +
> +/* get the widget type from widget capability bits */
> +static inline int snd_hdac_get_wcaps_type(unsigned int wcaps)
> +{
> + if (!wcaps)
> + return -1; /* invalid type */
> + return (wcaps & AC_WCAP_TYPE) >> AC_WCAP_TYPE_SHIFT;
> +}
> +
> +/* get the number of supported channels */
> +static inline unsigned int snd_hdac_get_wcaps_channels(u32 wcaps)
> +{
> + unsigned int chans;
> +
> + chans = (wcaps & AC_WCAP_CHAN_CNT_EXT) >> 13;
> + chans = (chans + 1) * 2;
> +
> + return chans;
> +}