Re: [PATCH v2] ALSA: hda/ca0132: Add Sound BlasterX AE-5 external RGB LED strip support

[Takashi Iwai <tiwai@suse.de>]

On Fri, 02 Oct 2026 03:41:50 +0200,
dchristensen8 wrote:
> 
> 
> Takashi,
> 
> Thank you for your assistance on this! I updated it to use CLASS
> (snd_hda_power_pm, pm)(codec) for the power management call, which cleaned up
> all the manual power-down error exits nicely.
> 
> Attached is the proper v2 patch for upstreaming.

Now reviewing further in details, and some issues found:

> --- a/sound/hda/codecs/ca0132.c
> +++ b/sound/hda/codecs/ca0132.c
> @@ -35,6 +35,8 @@
>  #include <sound/tlv.h>
>  #endif
>  
> +#include <linux/dma-mapping.h>

Let's move this into the block with other header file inclusions.
Not needed to be alone here.


> +static struct hdac_stream *ae5_strip_find_stream(struct hda_codec *codec)
> +{
> +	struct ca0132_spec *spec = codec->spec;
> +	struct hdac_bus *bus = &codec->bus->core;
> +	struct hdac_stream *s, *dsp = NULL, *first = NULL;
> +
> +	guard(spinlock_irqsave)(&bus->reg_lock);
> +	list_for_each_entry(s, &bus->stream_list, list) {
> +		if (s->direction != SNDRV_PCM_STREAM_PLAYBACK)
> +			continue;
> +		if (s->running || s->locked)
> +			continue;

It should also check s->opened, too.

> +	for (;;) {
> +		hstr = ae5_strip_find_stream(codec);
> +		if (!hstr) {
> +			if (--retries <= 0) {
> +				codec_warn(codec, "AE5 strip: no free azx stream after retries\n");
> +				return -EBUSY;
> +			}
> +			usleep_range(5000, 10000);
> +			continue;
> +		}
> +
> +		stream_tag = snd_hdac_dsp_prepare(hstr, format, AE5_STRIP_DMA_SIZE, &dmab);

The function needs CONFIG_SND_HDA_DSP_LOADER, and it's not always
selected.  Although the compile would works, it makes no sense to keep
the code without the support code.


> +static int ae5_strip_ctl_put(struct snd_kcontrol *kcontrol,
> +			     struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> +	struct ca0132_spec *spec = codec->spec;
> +	u32 colors[AE5_STRIP_MAX_LEDS];
> +	int i, num_leds;
> +
> +	guard(mutex)(&spec->ae5_strip_mutex);
> +	num_leds = spec->ae5_strip_cur_num_leds;
> +	for (i = 0; i < num_leds; i++) {
> +		u8 r = ucontrol->value.bytes.data[i * 3 + 0];
> +		u8 g = ucontrol->value.bytes.data[i * 3 + 1];
> +		u8 b = ucontrol->value.bytes.data[i * 3 + 2];
> +
> +		colors[i] = ((u32)g << 16) | ((u32)r << 8) | (u32)b;
> +	}
> +	memcpy(spec->ae5_strip_cur_colors, colors, sizeof(u32) * num_leds);
> +	ae5_strip_send_frame(codec, colors, num_leds);
> +	return 0;

In general, the put callback should return 1 if the values are
modified.


thanks,

Takashi