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.wang@xxxxxxx>
> ---
> 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);
> + }
