[PATCH 0/3] ASoC: fsl-asoc-card: Support hp and mic detection
Support hp and mic detection. Add a parameter for asoc_simple_init_jack.
Shengjiu Wang (3): ASoC: simple-card-utils: Support configure pin_name for asoc_simple_init_jack ASoC: bindings: fsl-asoc-card: Support hp-det-gpio and mic-det-gpio ASoC: fsl-asoc-card: Support Headphone and Microphone Jack detection
.../bindings/sound/fsl-asoc-card.txt | 3 + include/sound/simple_card_utils.h | 6 +- sound/soc/fsl/Kconfig | 1 + sound/soc/fsl/fsl-asoc-card.c | 69 ++++++++++++++++++- sound/soc/generic/simple-card-utils.c | 7 +- 5 files changed, 78 insertions(+), 8 deletions(-)
Currently the pin_name is fixed in asoc_simple_init_jack, but some driver may use a different pin_name. So add a new parameter in asoc_simple_init_jack for configuring pin_name.
If this parameter is NULL, then the default pin_name is used.
Signed-off-by: Shengjiu Wang shengjiu.wang@nxp.com --- include/sound/simple_card_utils.h | 6 +++--- sound/soc/generic/simple-card-utils.c | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h index bbdd1542d6f1..86a1e956991e 100644 --- a/include/sound/simple_card_utils.h +++ b/include/sound/simple_card_utils.h @@ -12,9 +12,9 @@ #include <sound/soc.h>
#define asoc_simple_init_hp(card, sjack, prefix) \ - asoc_simple_init_jack(card, sjack, 1, prefix) + asoc_simple_init_jack(card, sjack, 1, prefix, NULL) #define asoc_simple_init_mic(card, sjack, prefix) \ - asoc_simple_init_jack(card, sjack, 0, prefix) + asoc_simple_init_jack(card, sjack, 0, prefix, NULL)
struct asoc_simple_dai { const char *name; @@ -131,7 +131,7 @@ int asoc_simple_parse_pin_switches(struct snd_soc_card *card,
int asoc_simple_init_jack(struct snd_soc_card *card, struct asoc_simple_jack *sjack, - int is_hp, char *prefix); + int is_hp, char *prefix, char *pin); int asoc_simple_init_priv(struct asoc_simple_priv *priv, struct link_info *li);
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c index 8c54dc6710fe..b408cb5ed644 100644 --- a/sound/soc/generic/simple-card-utils.c +++ b/sound/soc/generic/simple-card-utils.c @@ -540,7 +540,8 @@ EXPORT_SYMBOL_GPL(asoc_simple_parse_pin_switches);
int asoc_simple_init_jack(struct snd_soc_card *card, struct asoc_simple_jack *sjack, - int is_hp, char *prefix) + int is_hp, char *prefix, + char *pin) { struct device *dev = card->dev; enum of_gpio_flags flags; @@ -557,12 +558,12 @@ int asoc_simple_init_jack(struct snd_soc_card *card,
if (is_hp) { snprintf(prop, sizeof(prop), "%shp-det-gpio", prefix); - pin_name = "Headphones"; + pin_name = pin ? pin : "Headphones"; gpio_name = "Headphone detection"; mask = SND_JACK_HEADPHONE; } else { snprintf(prop, sizeof(prop), "%smic-det-gpio", prefix); - pin_name = "Mic Jack"; + pin_name = pin ? pin : "Mic Jack"; gpio_name = "Mic detection"; mask = SND_JACK_MICROPHONE; }
Add headphone and microphone detection GPIO support.
Signed-off-by: Shengjiu Wang shengjiu.wang@nxp.com --- Documentation/devicetree/bindings/sound/fsl-asoc-card.txt | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt index 133d7e14a4d0..8a6a3d0fda5e 100644 --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt @@ -69,6 +69,9 @@ Optional properties: coexisting in order to support the old bindings of wm8962 and sgtl5000.
+ - hp-det-gpio : The GPIO that detect headphones are plugged in + - mic-det-gpio : The GPIO that detect microphones are plugged in + Optional unless SSI is selected as a CPU DAI:
- mux-int-port : The internal port of the i.MX audio muxer (AUDMUX)
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@nxp.com --- sound/soc/fsl/Kconfig | 1 + sound/soc/fsl/fsl-asoc-card.c | 69 ++++++++++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig index ea7b4787a8af..1c4ca5ec8caf 100644 --- a/sound/soc/fsl/Kconfig +++ b/sound/soc/fsl/Kconfig @@ -315,6 +315,7 @@ config SND_SOC_FSL_ASOC_CARD depends on OF && I2C # enforce SND_SOC_FSL_ASOC_CARD=m if SND_AC97_CODEC=m: depends on SND_AC97_CODEC || SND_AC97_CODEC=n + select SND_SIMPLE_CARD_UTILS select SND_SOC_IMX_AUDMUX select SND_SOC_IMX_PCM_DMA select SND_SOC_FSL_ESAI diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c index faac6ce9a82c..313058789ea9 100644 --- a/sound/soc/fsl/fsl-asoc-card.c +++ b/sound/soc/fsl/fsl-asoc-card.c @@ -15,6 +15,8 @@ #endif #include <sound/pcm_params.h> #include <sound/soc.h> +#include <sound/jack.h> +#include <sound/simple_card_utils.h>
#include "fsl_esai.h" #include "fsl_sai.h" @@ -65,6 +67,8 @@ struct cpu_priv { /** * struct fsl_asoc_card_priv - Freescale Generic ASOC card private data * @dai_link: DAI link structure including normal one and DPCM link + * @hp_jack: Headphone Jack structure + * @mic_jack: Microphone Jack structure * @pdev: platform device pointer * @codec_priv: CODEC private data * @cpu_priv: CPU private data @@ -79,6 +83,8 @@ struct cpu_priv {
struct fsl_asoc_card_priv { struct snd_soc_dai_link dai_link[3]; + struct asoc_simple_jack hp_jack; + struct asoc_simple_jack mic_jack; struct platform_device *pdev; struct codec_priv codec_priv; struct cpu_priv cpu_priv; @@ -445,6 +451,44 @@ static int fsl_asoc_card_audmux_init(struct device_node *np, return 0; }
+static int hp_jack_event(struct notifier_block *nb, unsigned long event, + void *data) +{ + struct snd_soc_jack *jack = (struct snd_soc_jack *)data; + struct snd_soc_dapm_context *dapm = &jack->card->dapm; + + if (event & SND_JACK_HEADPHONE) + /* Disable speaker if headphone is plugged in */ + snd_soc_dapm_disable_pin(dapm, "Ext Spk"); + else + snd_soc_dapm_enable_pin(dapm, "Ext Spk"); + + return 0; +} + +static struct notifier_block hp_jack_nb = { + .notifier_call = hp_jack_event, +}; + +static int mic_jack_event(struct notifier_block *nb, unsigned long event, + void *data) +{ + struct snd_soc_jack *jack = (struct snd_soc_jack *)data; + struct snd_soc_dapm_context *dapm = &jack->card->dapm; + + if (event & SND_JACK_MICROPHONE) + /* Disable dmic if microphone is plugged in */ + snd_soc_dapm_disable_pin(dapm, "DMIC"); + else + snd_soc_dapm_enable_pin(dapm, "DMIC"); + + return 0; +} + +static struct notifier_block mic_jack_nb = { + .notifier_call = mic_jack_event, +}; + 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); + goto asrc_fail; + } + + if (of_property_read_bool(np, "hp-det-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); + } + + if (of_property_read_bool(np, "mic-det-gpio")) { + ret = asoc_simple_init_jack(&priv->card, &priv->mic_jack, + 0, NULL, "Mic Jack"); + if (ret) + goto asrc_fail; + + snd_soc_jack_notifier_register(&priv->mic_jack.jack, &mic_jack_nb); + }
asrc_fail: of_node_put(asrc_np);
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@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);- }
On Wed, Jul 15, 2020 at 5:16 AM Nicolin Chen nicoleotsuka@gmail.com wrote:
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@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.
If we move this to asrc_fail label, then it will be hard to define the error message. There are many places that goto asrc_fail.
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.
In asoc_simple_init_jack, gpio_is_valid() will be invalid when there is no "hp-det-gpio" property, and asoc_simple_init_jack will return 0.
The reason why I add a check here is mostly for snd_soc_jack_notifier_register(). when there is no jack created, there will be a kernel dump.
or I can use this code:
- if (of_property_read_bool(np, "hp-det-gpio")) { - ret = asoc_simple_init_jack(&priv->card, &priv->hp_jack, - 1, NULL, "Headphone Jack"); - if (ret) - goto asrc_fail; + ret = asoc_simple_init_jack(&priv->card, &priv->hp_jack, + 1, NULL, "Headphone Jack"); + if (ret) + goto asrc_fail;
+ if (priv->hp_jack.jack.jack) snd_soc_jack_notifier_register(&priv->hp_jack.jack, &hp_jack_nb); - }
what do you think?
best regards wang shengjiu
On Wed, Jul 15, 2020 at 12:14:01PM +0800, Shengjiu Wang wrote:
On Wed, Jul 15, 2020 at 5:16 AM Nicolin Chen nicoleotsuka@gmail.com wrote:
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@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.
If we move this to asrc_fail label, then it will be hard to define the error message. There are many places that goto asrc_fail.
Oh...good point...
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.
In asoc_simple_init_jack, gpio_is_valid() will be invalid when there is no "hp-det-gpio" property, and asoc_simple_init_jack will return 0.
The reason why I add a check here is mostly for snd_soc_jack_notifier_register(). when there is no jack created, there will be a kernel dump.
or I can use this code:
if (of_property_read_bool(np, "hp-det-gpio")) {ret = asoc_simple_init_jack(&priv->card, &priv->hp_jack,1, NULL, "Headphone Jack");if (ret)goto asrc_fail;
ret = asoc_simple_init_jack(&priv->card, &priv->hp_jack,1, NULL, "Headphone Jack");if (ret)goto asrc_fail;if (priv->hp_jack.jack.jack) snd_soc_jack_notifier_register(&priv->hp_jack.jack,
It's pretty clean but not very obvious for the "optional" part. So I think that it'd be slightly better to go for your previous solution, but with a line of comments to show: these properties are optional and asoc_simple_init_jack() uses the same strings.
Please add to all three changes once the comments being added:
Acked-by: Nicolin Chen nicoleotsuka@gmail.com
Thanks
participants (3)
-
Nicolin Chen -
Shengjiu Wang -
Shengjiu Wang