Hi Charles,
On Tue, Feb 15, 2022 at 7:20 AM Charles Keepax ckeepax@opensource.cirrus.com wrote:
Looking through the code I think its probably better to combine the two controls here. It looks like you would need to set both to enable the SPDIF and I don't really see any reason for them to be different. I think you can just move the register bits onto the DAPM widget and have DAPM control them.
This patch also probably needs a fixes tag:
Fixes: f853d6b3ba34 ("ASoC: cs4265: Add a S/PDIF enable switch")
Apologies for missing this in my review of the original patch. Let me know if you want me to have a bash at combining them.
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.
Thanks