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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
