2022-10-28 12:23:48

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] ASoC: dmic: Add optional dmic selection

On Fri, Oct 28, 2022 at 06:24:50PM +0800, Ajye Huang wrote:

> + dmic->dmic_sel = devm_gpiod_get_optional(component->dev,
> + "dmic_sel", GPIOD_OUT_LOW);
> + if (IS_ERR(dmic->dmic_sel))
> + return PTR_ERR(dmic->dmic_sel);
> +
> snd_soc_component_set_drvdata(component, dmic);
>
> return 0;
> @@ -125,10 +172,15 @@ static const struct snd_soc_dapm_widget dmic_dapm_widgets[] = {
> SND_SOC_NOPM, 0, 0, dmic_aif_event,
> SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD),
> SND_SOC_DAPM_INPUT("DMic"),
> + SND_SOC_DAPM_MIC("DMIC", NULL),
> + SND_SOC_DAPM_MUX("Dmic Mux", SND_SOC_NOPM, 0, 0, &dmic_mux_control),

If we are doing this then adding the mux needs to be conditional on
having the GPIO, without the GPIO the control is at best confusing to
users.


Attachments:
(No filename) (807.00 B)
signature.asc (499.00 B)
Download all attachments

2022-10-28 13:49:35

by Ajye Huang

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] ASoC: dmic: Add optional dmic selection

Hi Mark Brown,

Thank you for review,
I think it is appropriate to implement on audio machine side, like
this I did before,
commit 3cfbf07c6d27
("ASoC: qcom: sc7180: Modify machine driver for 2mic")

What is your suggestion? Thank you.

On Fri, Oct 28, 2022 at 7:58 PM Mark Brown <[email protected]> wrote:
>
> On Fri, Oct 28, 2022 at 06:24:50PM +0800, Ajye Huang wrote:
>
> > + dmic->dmic_sel = devm_gpiod_get_optional(component->dev,
> > + "dmic_sel", GPIOD_OUT_LOW);
> > + if (IS_ERR(dmic->dmic_sel))
> > + return PTR_ERR(dmic->dmic_sel);
> > +
> > snd_soc_component_set_drvdata(component, dmic);
> >
> > return 0;
> > @@ -125,10 +172,15 @@ static const struct snd_soc_dapm_widget dmic_dapm_widgets[] = {
> > SND_SOC_NOPM, 0, 0, dmic_aif_event,
> > SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD),
> > SND_SOC_DAPM_INPUT("DMic"),
> > + SND_SOC_DAPM_MIC("DMIC", NULL),
> > + SND_SOC_DAPM_MUX("Dmic Mux", SND_SOC_NOPM, 0, 0, &dmic_mux_control),
>
> If we are doing this then adding the mux needs to be conditional on
> having the GPIO, without the GPIO the control is at best confusing to
> users.