Mark
Thanks for the review
On 2/10/20 7:52 AM, Mark Brown wrote:
On Fri, Feb 07, 2020 at 01:45:33PM -0600, Dan Murphy wrote:
- /* interface format */
- switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
- case SND_SOC_DAIFMT_I2S:
iface_reg1 |= ADCX140_I2S_MODE_BIT;break;- case SND_SOC_DAIFMT_LEFT_J:
iface_reg1 |= ADCX140_LEFT_JUST_BIT;break;- case SND_SOC_DAIFMT_DSP_A:
- case SND_SOC_DAIFMT_DSP_B:
break;_DSP_A and _DSP_B are two different format so I'd expect the device to be configured differently for them, or for only one to be supported.
Ack. I will need to adjust the TX offset for these. I will do add a prepare call back and set the offset there. These can fall through since TDM is default.
+static int adcx140_mute(struct snd_soc_dai *codec_dai, int mute) +{
- struct snd_soc_component *component = codec_dai->component;
- int config_reg;
- int mic_enable;
- int i;
- /* There is not a single register to mute. Each enabled path has to be
* muted individually. Read which path is enabled and mute it.*/- snd_soc_component_read(component, ADCX140_IN_CH_EN, &mic_enable);
- if (!mic_enable)
return 0;You could also just offer this control to userspace, it's not *essential* to have this operation though it can help with glitching during stream startup.
- for (i = 0; i < ADCX140_MAX_CHANNELS; i++) {
config_reg = ADCX140_CH8_CFG2 - (5 * i);if (!(mic_enable & BIT(i)))continue;if (mute)snd_soc_component_write(component, config_reg, 0);- }
How does the unmute work?
This is unmuted through volume control. I can remove this as the device is muted when the volume register is set to 0. That will force the user to have to set a volume.
- internal_reg = device_property_present(adcx140->dev,
"ti,use-internal-areg");- if (internal_reg)
sleep_cfg_val |= ADCX140_AREG_INTERNAL;Does this actually need a specific property or could you support the regulator API and then use regulator_get_optional() to figure out if an external AVDD is attached?
Yes we could set internal AREG bit if no supply is given.
+static int adcx140_codec_probe(struct snd_soc_component *component) +{
- struct adcx140_priv *adcx140 = snd_soc_component_get_drvdata(component);
- return adc5410_init(adcx140);
+}
Does the separate init function buy us anything?
No that is an artifact of my development I can move everything to codec_probe
Dan