On 21/02/2020 13:00, Mark Brown wrote: > 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. Tegra has a hardware sample rate converter (though driver not yet upstream or part of this initial series) and if using the sample-rate converter, then the actual rate captured by the DMIC interface could be different from the resulting sample-rate. So we want a way to indicate to the DMIC it is capturing at rate X, while the resulting sample-rate is Y. I am not sure if there is a better way to do this? Ideally, the DMIC would query the rate from the upstream MUX it is connected to, but I am not sure if there is a way to do that. So right now it is a manual process and the user has to configure these which are not ideal. Cheers Jon -- nvpublic