On Tue, Dec 12, 2017 at 04:51:35PM -0600, Steven Eckhoff wrote: > > > > +static int tscs42xx_mute_stream(struct snd_soc_dai *dai, int mute, int > > > > stream) > > > > +{ > > > > + struct snd_soc_codec *codec = dai->codec; > > > > + int ret; > > > > + > > > > + if (mute) > > > > + if (stream == SNDRV_PCM_STREAM_PLAYBACK) > > > > + ret = dac_mute(codec); > > > > + else > > > > + ret = adc_mute(codec); > > > > + else > > > > + if (stream == SNDRV_PCM_STREAM_PLAYBACK) > > > > + ret = dac_unmute(codec); > > > > + else > > > > + ret = adc_unmute(codec); > > > > + > > > > + return ret; > > > > +} > > > > > > All these mute functions also shut down the PLLs which since... > > > > > This is a bit funky since it looks like if you unmuted the dac and then > > muted the adc that the PLLs would be powered off on the playback stream, > > but the logic in the chip is a bit funky in that it wont allow this unless > > the playback interface is also powered off. > > > > This should definitely be fixed since it is confusing. The PLL > > powered up/down stuff will be removed from the mute functions in the > > next version. > > > > What are your thoughts on turning the PLL into a DAPM widget and using > > the event callback to power up/down the PLLs? I have tested this and it > > seems to work fine. > > > > > > + switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { > > > > + case SND_SOC_DAIFMT_CBM_CFM: > > > > + ret = snd_soc_update_bits(codec, R_AIC1, RM_AIC1_MS, > > > > + RV_AIC1_MS_MASTER); > > > > + if (ret < 0) > > > > + dev_err(codec->dev, > > > > + "Failed to set codec DAI master > > > > (%d)\n", ret); > > > > + else > > > > + ret = 0; > > > > + break; > > > > + default: > > > > > > ...we only support the CODEC being the clock master seems like it might > > > mean we stop clocking the DAI? If that's the case it's better to just > > > not have the mute control and allow the user to just control these as > > > normal mutes. > > > > > I am going to put slave mode support in, but I may need some time to > > test how it works. > > > Okay I had to refresh my memory on why slave was not being supported. > Our slave mode needs the audio clocks to stay active to avoid pops. This > has something to do with how the charge pump is setup. In master mode > this is not an issue. I will keep the slave mode support out of the next > version. > > Also I am not sure what you mean with the mute controls. Could you > elaborate more on this? > Okay I belive I know what this was all about.
Were you asking why the mute functions are powering up/down the PLLs? The answer is that is was a vestige of the very first version of the driver where I was told I needed to power PLLs before unmuting and muting before powering them down, which is correct I just didn't have them in the correct callback. In the next version I have moved the powering of the PLLs into the event call back of a DAPM supply widget. If there is a better way I can implement that. The previous comment on slave mode is still valid, so the next version will only suppor the codec in master mode. Sorry for any confusion that I may have caused.