On Tue, Feb 15, 2022 at 07:52:07AM -0300, Fabio Estevam wrote:
On Tue, Feb 15, 2022 at 7:20 AM Charles Keepax ckeepax@opensource.cirrus.com wrote: Do you mean something like this?
--- a/sound/soc/codecs/cs4265.c +++ b/sound/soc/codecs/cs4265.c @@ -122,7 +122,7 @@ static const struct snd_kcontrol_new loopback_ctl = SOC_DAPM_SINGLE("Switch", CS4265_SIG_SEL, 1, 1, 0);
static const struct snd_kcontrol_new spdif_switch =
SOC_DAPM_SINGLE("Switch", SND_SOC_NOPM, 0, 0, 0);
SOC_DAPM_SINGLE("Switch", CS4265_SPDIF_CTL2, 5, 1, 1);static const struct snd_kcontrol_new dac_switch = SOC_DAPM_SINGLE("Switch", CS4265_PWRCTL, 1, 1, 0); @@ -150,7 +150,6 @@ static const struct snd_kcontrol_new cs4265_snd_controls[] = { SOC_SINGLE("E to F Buffer Disable Switch", CS4265_SPDIF_CTL1, 6, 1, 0), SOC_ENUM("C Data Access", cam_mode_enum),
SOC_SINGLE("SPDIF Switch", CS4265_SPDIF_CTL2, 5, 1, 1), SOC_SINGLE("Validity Bit Control Switch", CS4265_SPDIF_CTL2, 3, 1, 0), SOC_ENUM("SPDIF Mono/Stereo", spdif_mono_stereo_enum),@@ -186,7 +185,7 @@ static const struct snd_soc_dapm_widget cs4265_dapm_widgets[] = {
SND_SOC_DAPM_SWITCH("Loopback", SND_SOC_NOPM, 0, 0, &loopback_ctl),
SND_SOC_DAPM_SWITCH("SPDIF", SND_SOC_NOPM, 0, 0,
SND_SOC_DAPM_SWITCH("SPDIF", CS4265_SPDIF_CTL2, 5, 1, &spdif_switch), SND_SOC_DAPM_SWITCH("DAC", CS4265_PWRCTL, 1, 1, &dac_switch),If this is not what you meant, please feel free to submit a proper patch.
Yeah that is almost exactly what I meant. Only thing is you don't need to put the register on the both SOC_DAPM_SINGLE and on the SND_SOC_DAPM_SWITCH. Little bit of a judgement call on which of the two to put it on.
Putting it on the SOC_DAPM_SINGLE is much closer to the existing code.
But putting it on the SND_SOC_DAPM_SWITCH feels more correct to me, ie. only enabling it when the SPDIF is in use. I would suggest this way, as the existing code is clearly not tested so it doesn't feel very valuable to stick to what the existing code does and this seems more correct.
Thanks, Charles