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