Re: [PATCH v1 2/2] ASoC: mediatek: mt8186-rt5682: Modify machine driver for two DMICs case

[Mark Brown]

On Sat, Oct 29, 2022 at 12:07:33AM +0800, Ajye Huang wrote:

> +static const char * const dmic_mux_text[] = {
> +	"FrontMic",
> +	"RearMic",
> +};

> +	ret = snd_soc_dapm_new_controls(&card->dapm, dmic_widgets,
> +					ARRAY_SIZE(dmic_widgets));
> +	if (ret) {
> +		dev_err(card->dev, "DMic widget addition failed: %d\n", ret);
> +		/* Don't need to add routes if widget addition failed */
> +		return ret;
> +	}

> +       if (of_property_read_bool(pdev->dev.of_node, "dmic-gpios")) {
> +               mach_priv->dmic_sel = devm_gpiod_get_optional(&pdev->dev,
> +                                                       "dmic", GPIOD_OUT_LOW);

My prior comments about it being nicer to make the addition of the
control depend on the property being there and the description of the
options configurable do stand (I appreciate that the other driver might
not have done this which was probably an oversight).

Also do we really need the of_property_read_bool() there?  I'd have
expected that devm_gpiod_get_optional() would be handling that for us.

Attachment: signature.asc
Description: PGP signature