Hi Shengjiu,

The whole series looks good to me. Just a couple of small
questions inline:

On Tue, Jul 14, 2020 at 05:05:36PM +0800, Shengjiu Wang wrote:
> Use asoc_simple_init_jack function from simple card to implement
> the Headphone and Microphone detection.
> Register notifier to disable Speaker when Headphone is plugged in
> and enable Speaker when Headphone is unplugged.
> Register notifier to disable Digital Microphone when Analog Microphone
> is plugged in and enable DMIC when Analog Microphone is unplugged.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.w...@nxp.com>
> ---
>  sound/soc/fsl/Kconfig         |  1 +
>  sound/soc/fsl/fsl-asoc-card.c | 69 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 68 insertions(+), 2 deletions(-)

>  static int fsl_asoc_card_late_probe(struct snd_soc_card *card)
>  {
>       struct fsl_asoc_card_priv *priv = snd_soc_card_get_drvdata(card);
> @@ -745,8 +789,29 @@ static int fsl_asoc_card_probe(struct platform_device 
> *pdev)
>       snd_soc_card_set_drvdata(&priv->card, priv);
>  
>       ret = devm_snd_soc_register_card(&pdev->dev, &priv->card);
> -     if (ret && ret != -EPROBE_DEFER)
> -             dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
> +     if (ret) {
> +             if (ret != -EPROBE_DEFER)
> +                     dev_err(&pdev->dev, "snd_soc_register_card failed 
> (%d)\n", ret);

I think we may move this EPROBE_DEFER to the asrc_fail label.

> +             goto asrc_fail;
> +     }
> +
> +     if (of_property_read_bool(np, "hp-det-gpio")) {

Could we move this check inside asoc_simple_init_jack? There's no
problem with doing it here though, yet I got a bit confused by it
as I thought it's a boolean type property, which would be against
the DT bindings until I saw asoc_simple_init_jack() uses the same
string to get the GPIO. Just it probably would be a bit tricky as
we need it to be optional here.

Otherwise, I think we may add a line of comments to indicate that
the API would use the same string to get the GPIO.

> +             ret = asoc_simple_init_jack(&priv->card, &priv->hp_jack,
> +                                         1, NULL, "Headphone Jack");
> +             if (ret)
> +                     goto asrc_fail;
> +
> +             snd_soc_jack_notifier_register(&priv->hp_jack.jack, 
> &hp_jack_nb);
> +     }

Reply via email to