On 21-10-18, 22:42, Pierre-Louis Bossart wrote: > > On 10/2/18 12:57 AM, Jenny TC wrote: > > Some DMICs need clock to be running for a specified duration as per the > > DMIC spec to complete the mode transition like sleep to mormal, off to normal > > etc. > > > > Jenny TC (2): > > ASoC: dmic: Enable ACPI device entry > > ASoC: dmic: Introduce mode switch delay > > > > sound/soc/codecs/dmic.c | 38 ++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 38 insertions(+) > > > Sorry for the late feedback. > > Allowing some timing adjustments for the clock transitions is a good thing. > The way it's done is questionable and raises a number of concerns. > > First, there was an Intel internal discussion before my extended break on > why this 'dmic-codec' is needed on Intel platforms. To the best of my > knowledge we don't control the mics with GPIOs, which was the initial > purpose of this driver. We have experimental evidence on ApolloLake and > GeminiLake that using the soc-dummy/soc-dummy-dai definitions are enough, > and it may be a good thing to agree on the direction here. If you want a > parameter, you can still use a machine driver DMI-based kernel quirk and/or > pass a kernel parameter, the need to extend this dmic-codec is far from > obvious to me. I think in this case you can go with dummy codec. You are modelling an endpoint here, nothing else > Assuming you still want to use this codec, then there are still major > concerns about the ACPI directions. As Mark noted it, "DMIC" does not follow > any of the guidelines or accepted definitions with an unambiguous vendor and > part ID. We know we already have conflicts between Intel-defined ACPI IDs, > e.g. for RT298 on multiple platforms, let's be careful here, shall we? > > And I am coming to my last point. The Skylake driver already contains code > to create the dmic devices by hand (see below the git grep results). So I > wonder what happens if you use both ACPI-based enumeration AND manually > create the dmic device - I view these solutions as mutually incompatible. > Either you have not tested against the upstream code or something is missing > from your patchset. What am I missing? That is very valid point. Also DMIC clock is generated by Audio block and not really a system clock, so why shouldn't this be described in audio block. Furthermore I recall we have NHLT table and some description for DMICs. The driver currently parses that information to get number of dmics to use (see skl_get_dmic_geo). I would think that adding this delay to NHLT would be more sensible and pass it on to whatever entity wishes to use it. -- ~Vinod _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
