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@xxxxxxxxxxxxxxxxxxxxx> 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
