On Thu, Feb 20, 2020 at 12:04:45PM +0530, Sameer Pujar wrote:
+++ b/sound/soc/tegra/tegra210_dmic.c @@ -0,0 +1,515 @@ +// SPDX-License-Identifier: GPL-2.0-only +/*
- tegra210_dmic.c - Tegra210 DMIC driver
- Copyright (c) 2020 NVIDIA CORPORATION. All rights reserved.
Please make the entire comment a C++ one so things look more intentional.
- /* Below enables all filters - DCR, LP and SC */
- { TEGRA210_DMIC_DBG_CTRL, 0xe },
So this isn't the hardware default?
- srate = params_rate(params);
- if (dmic->srate_override)
srate = dmic->srate_override;
How does this work for userspace? If we just ignore the sample rate we were asked for I'd expect that the application would get upset.
- if (strstr(kcontrol->id.name, "Boost Gain"))
dmic->boost_gain = value;
Volume controls should end in "Volume".
- else if (strstr(kcontrol->id.name, "Audio Channels"))
dmic->audio_ch_override = value;
This is something that would usually come from hw_params?
- else if (strstr(kcontrol->id.name, "LR Polarity Select"))
dmic->lrsel = value;
This and some of the others look like they're describing details of how the board is wired up so I'd not expect them to be runtime selectable?
- SND_SOC_DAPM_MIC("Dummy Input", NULL),
This is just the microphone that happens to be attached, isn't it? If so that's a weird name.
+static const char * const tegra210_dmic_mono_conv_text[] = {
- "ZERO", "COPY",
+};
It'd be more idiomatic for ALSA to write these as Zero and Copy.
- SOC_ENUM_EXT("Channel Select", tegra210_dmic_ch_enum,
tegra210_dmic_get_control, tegra210_dmic_put_control),- SOC_ENUM_EXT("Mono To Stereo",
tegra210_dmic_mono_conv_enum, tegra210_dmic_get_control,tegra210_dmic_put_control),- SOC_ENUM_EXT("Stereo To Mono",
tegra210_dmic_stereo_conv_enum, tegra210_dmic_get_control,tegra210_dmic_put_control),
I'd expect these to be in DAPM.