Re: [PATCH 5/7] ALSA: usb-audio: us144mkii: Add MIDI support and mixer controls

[Takashi Iwai <tiwai@suse.de>]

On Sun, 10 Aug 2025 14:49:56 +0200,
Šerif Rami wrote:
> +/**
> + * @brief Text descriptions for playback output source options.
> + *
> + * Used by ALSA kcontrol elements to provide user-friendly names for
> + * the playback routing options (e.g., "Playback 1-2", "Playback 3-4").
> + */
> +static const char *const playback_source_texts[] = { "Playback 1-2",
> +						     "Playback 3-4" };
> +
> +/**
> + * @brief Text descriptions for capture input source options.
> + *
> + * Used by ALSA kcontrol elements to provide user-friendly names for
> + * the capture routing options (e.g., "Analog In", "Digital In").
> + */
> +static const char *const capture_source_texts[] = { "Analog In", "Digital In" };
> +
> +/**
> + * tascam_playback_source_info() - ALSA control info callback for playback
> + * source.
> + * @kcontrol: The ALSA kcontrol instance.
> + * @uinfo: The ALSA control element info structure to fill.
> + *
> + * This function provides information about the enumerated playback source
> + * control, including its type, count, and available items (Playback 1-2,
> + * Playback 3-4).
> + *
> + * Return: 0 on success.
> + */
> +static int tascam_playback_source_info(struct snd_kcontrol *kcontrol,
> +				       struct snd_ctl_elem_info *uinfo)
> +{
> +	return snd_ctl_enum_info(uinfo, 1, 2, playback_source_texts);
> +}
> +
> +/**
> + * tascam_line_out_get() - ALSA control get callback for Line Outputs Source.
> + * @kcontrol: The ALSA kcontrol instance.
> + * @ucontrol: The ALSA control element value structure to fill.
> + *
> + * This function retrieves the current selection for the Line Outputs source
> + * (Playback 1-2 or Playback 3-4) from the driver's private data and populates
> + * the ALSA control element value.
> + *
> + * Return: 0 on success.
> + */
> +static int tascam_line_out_get(struct snd_kcontrol *kcontrol,
> +			       struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct tascam_card *tascam = snd_kcontrol_chip(kcontrol);
> +
> +	ucontrol->value.enumerated.item[0] = tascam->line_out_source;
> +	return 0;
> +}
> +
> +/**
> + * tascam_line_out_put() - ALSA control put callback for Line Outputs Source.
> + * @kcontrol: The ALSA kcontrol instance.
> + * @ucontrol: The ALSA control element value structure containing the new value.
> + *
> + * This function sets the Line Outputs source (Playback 1-2 or Playback 3-4)
> + * based on the user's selection from the ALSA control element. It validates
> + * the input and updates the driver's private data.
> + *
> + * Return: 1 if the value was changed, 0 if unchanged, or a negative error code.
> + */
> +static int tascam_line_out_put(struct snd_kcontrol *kcontrol,
> +			       struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct tascam_card *tascam = snd_kcontrol_chip(kcontrol);
> +
> +	if (ucontrol->value.enumerated.item[0] > 1)
> +		return -EINVAL;
> +	if (tascam->line_out_source == ucontrol->value.enumerated.item[0])
> +		return 0;
> +	tascam->line_out_source = ucontrol->value.enumerated.item[0];
> +	return 1;
> +}
> +
> +/**
> + * tascam_line_out_control - ALSA kcontrol definition for Line Outputs Source.
> + *
> + * This defines a new ALSA mixer control named "Line OUTPUTS Source" that allows
> + * the user to select between "Playback 1-2" and "Playback 3-4" for the analog
> + * line outputs of the device. It uses the `tascam_playback_source_info` for
> + * information and `tascam_line_out_get`/`tascam_line_out_put` for value
> + * handling.
> + */
> +static const struct snd_kcontrol_new tascam_line_out_control = {
> +	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> +	.name = "Line OUTPUTS Source",

The control name is always a difficult question.
For following the standards, it'd be better to be a name like
"Line Playback Source" (i.e. prefix + direction + suffix where
direction is either "Playback" or "Capture").

> +static const struct snd_kcontrol_new tascam_digital_out_control = {
> +	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> +	.name = "Digital OUTPUTS Source",

Ditto.

> +static const struct snd_kcontrol_new tascam_capture_12_control = {
> +	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> +	.name = "ch1 and ch2 Source",

This should be "... Capture Source", then, and what comes at the
prefix is another difficult question.  I find better to have capital
letters, or combine like "Ch1/2 Capture Source".

> +static const struct snd_kcontrol_new tascam_capture_34_control = {
> +	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> +	.name = "ch3 and ch4 Source",

Ditto.

> +static int tascam_samplerate_get(struct snd_kcontrol *kcontrol,
> +				 struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct tascam_card *tascam =
> +		(struct tascam_card *)snd_kcontrol_chip(kcontrol);
> +	u8 *buf __free(kfree);

Don't forget NULL initialization.


thanks,

Takashi