On 9/20/23 19:55, Takashi Iwai wrote:
+ spin_lock(&marian->reglock);
+ marian_generic_set_dco(marian, ucontrol->value.integer.value[0]);
+ spin_unlock(&marian->reglock);
The control get/put callbacks can sleep, hence usually it's
spin_lock_irq(). Or if the all places for this lock are sleepable
context, use a mutex instead.
Alright, it looks like reglock is used in sleepable context only, so
I'll replace it with mutex. Thanks!
+static int marian_control_pcm_loopback_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN;
+ uinfo->count = 1;
+ uinfo->value.integer.min = 0;
+ uinfo->value.integer.max = 1;
+
+ return 0;
This can be replaced with snd_ctl_boolean_mono_info.
Oh, I forgot to use 'snd_ctl_boolean_mono_info' here. Will be done.
+}
+
+static int marian_control_pcm_loopback_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct marian_card *marian = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.integer.value[0] = marian->loopback;
+
+ return 0;
+}
+
+static int marian_control_pcm_loopback_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct marian_card *marian = snd_kcontrol_chip(kcontrol);
+
+ marian->loopback = ucontrol->value.integer.value[0];
Better to normalize with !!ucontrol->value.integer.value[0].
The value check isn't done as default.
Will be fixed, thanks!
+static int marian_control_pcm_loopback_create(struct marian_card *marian)
+{
+ struct snd_kcontrol_new c = {
+ .iface = SNDRV_CTL_ELEM_IFACE_PCM,
+ .name = "Loopback",
Better to have "Switch" suffix.
Yeah, and I guess some other controls also have to be renamed. Will be
done :)
+static int marian_m2_output_frame_mode_create(struct marian_card *marian, char *label, u32 idx)
+{
+ struct snd_kcontrol_new c = {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = label,
+ .private_value = idx,
+ .access = SNDRV_CTL_ELEM_ACCESS_READWRITE | SNDRV_CTL_ELEM_ACCESS_VOLATILE,
Does this have to be VOLATILE? Some others look also dubious.
Basically you set the value via this mixer element, then it's
persistent.
As I understand, some controls which depend on external inputs (like the
'Input 1 Freq', which measures frequency on the Input 1), should be
volatile, right?
This one definitely shouldn't, so I will change it to persistent.
Thank you for your prompt reply and review!
--
Kind regards,
Ivan Orlov
thanks,
Takashi
