On Fri, Oct 30, 2020 at 5:23 PM Tzung-Bi Shih <tzungbi@xxxxxxxxxx> wrote: > > On Fri, Oct 30, 2020 at 3:57 PM Ajye Huang <ajye.huang@xxxxxxxxx> wrote: > > +static struct gpio_desc *dmic_sel; > > +static int dmic_switch; > > If you really need them, you should put them in struct sc7180_snd_data. > Thank you, I will do it. > > +static int dmic_set(struct snd_kcontrol *kcontrol, > > + struct snd_ctl_elem_value *ucontrol) > > +{ > > + if (dmic_sel) { > > if (IS_ERR(dmic_sel)) > But I think you don't need to check dmic_sel. Suppose your _probe() > already returned error, the code here shouldn't be called. > OK, I will remove the discriminant if (dmic_sel) { > > + dmic_switch = ucontrol->value.integer.value[0]; > > Looks like it can be a local variable. You don't need to save dmic_switch. But dmic_get() will need dmic_switch, should i keep dmic_switch? static int dmic_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { ucontrol->value.integer.value[0] = dmic_switch; return 0; }