Hello Mark,
> On Fri, Mar 11, 2022 at 11:06:27AM +0100, Gregory CLEMENT wrote:
>
>> +static const char * const aic3x_dmic_rates[] = { "off", "128x", "64x", "32x" };
>> +static SOC_ENUM_SINGLE_DECL(aic3x_dmic_rates_enum, AIC3X_ASD_INTF_CTRLA, 0,
>> + aic3x_dmic_rates);
>> +static const struct snd_kcontrol_new aic3x_dmic_rates_controls =
>> + SOC_DAPM_ENUM("Route", aic3x_dmic_rates_enum);
>
> ...
>
>> + {"GPIO1 dmic modclk", NULL, "DMic Rate"},
>> + {"DMic Rate", "128x", "DMIC"},
>> + {"DMic Rate", "64x", "DMIC"},
>> + {"DMic Rate", "32x", "DMIC"},
>> + {"DMic Rate", "off", "DMIC"},
>
> This isn't really idiomatic and won't be power efficient since we don't
> automatically manage the power so we'll have the DMIC clock running even
> when no recording is in progress. What would be better would be to have
> an enum equivalent of the _AUTODISABLE() controls, providing an enum
> with the clock rates backed by a field in the driver data and then a
> DAPM widget which writes the value to the hardware when the widget is
> enabled and sets it back to off on disable. It'd be fine to open code
> that in the driver for now rather than actually implementing a generic
> thing if that's too much hassle.
Thanks for tour feedback, I will work on a second version based on this.
Gregory
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
