On Wed, Dec 08, 2021 at 08:21:31PM +0000, Mark Brown wrote: > On Thu, Dec 09, 2021 at 12:28:49AM +0530, Ajit Kumar Pandey wrote: > > > +static int speaker_mute_put(struct snd_kcontrol *kcontrol, > > + struct snd_ctl_elem_value *ucontrol) > > +{ > > + struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol); > > + struct max98357a_priv *max98357a = snd_soc_component_get_drvdata(component); > > + int mode = ucontrol->value.enumerated.item[0]; > > + > > + max98357a->sdmode_switch = mode; > > + gpiod_set_value_cansleep(max98357a->sdmode, mode); > > + dev_dbg(component->dev, "set sdmode to %d", mode); > > This looks like it should just be a DAPM widget - it's just a generic > GPIO control, there's no connection with the CODEC that I can see so it > definitely shouldn't be in the CODEC driver. Often trivial stuff like > this is done in the machine driver, though the simple-amplifier driver > is probably a good fit here. Actually now I look again the only control interface this driver has is GPIOs but it does expose a digital interface with constraints as input so doesn't fit within simple-amplifier. However this is just powering off the amplifier to achieve mute rather than a separate mute control so it'd probably be better to use a SND_SOC_DAPM_PIN_SWITCH() for the Speaker widget to do this, this would also end up addressing Jaroslav's thing with the naming as a side effect. Sorry about the confusion there.