* [PATCH 0/2] rt5651: Enable jack detection
@ 2017-10-19 11:03 Carlo Caione
2017-10-19 11:03 ` [PATCH 1/2] ASoC: rt5651: Enable jack detection on JD1_1 Carlo Caione
2017-10-19 11:03 ` [PATCH 2/2] ASoC: intel: byt: Add headset jack Carlo Caione
0 siblings, 2 replies; 8+ messages in thread
From: Carlo Caione @ 2017-10-19 11:03 UTC (permalink / raw)
To: linux, bardliao, oder_chiou, pierre-louis.bossart, broonie,
alsa-devel, linux-kernel, tiwai, albertchen, edgar_shen
Cc: Carlo Caione
From: Carlo Caione <carlo@endlessm.com>
Small set of patches to enable jack detection for the rt5651 codec.
Please note that:
- The jack detection is restricted to the JD1_1 pin only (the only
hardware I have uses this pin for jack detection)
- AFAICT (reading the rt5651 datasheet) there is no way to detect
runtime the type of the jack inserted as done for the others rt56*
codecs, so in the driver I always assume that we are dealing with an
headset.
Carlo Caione (2):
ASoC: rt5651: Enable jack detection on JD1_1
ASoC: intel: byt: Add headset jack
include/sound/rt5651.h | 7 +++
sound/soc/codecs/rt5651.c | 91 ++++++++++++++++++++++++++++++++++-
sound/soc/codecs/rt5651.h | 3 ++
sound/soc/intel/boards/bytcr_rt5651.c | 24 +++++++++
4 files changed, 123 insertions(+), 2 deletions(-)
--
2.14.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] ASoC: rt5651: Enable jack detection on JD1_1
2017-10-19 11:03 [PATCH 0/2] rt5651: Enable jack detection Carlo Caione
@ 2017-10-19 11:03 ` Carlo Caione
2017-10-19 15:47 ` [alsa-devel] " Pierre-Louis Bossart
2017-10-20 2:23 ` Bard Liao
2017-10-19 11:03 ` [PATCH 2/2] ASoC: intel: byt: Add headset jack Carlo Caione
1 sibling, 2 replies; 8+ messages in thread
From: Carlo Caione @ 2017-10-19 11:03 UTC (permalink / raw)
To: linux, bardliao, oder_chiou, pierre-louis.bossart, broonie,
alsa-devel, linux-kernel, tiwai, albertchen, edgar_shen
Cc: Carlo Caione
From: Carlo Caione <carlo@endlessm.com>
Enable jack detection or the RT5651 codec on the JD1_1 pin.
The codec has no means to detect the type of the jack connected so we
assume that the jack is always an headset jack.
Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
include/sound/rt5651.h | 7 ++++
sound/soc/codecs/rt5651.c | 91 +++++++++++++++++++++++++++++++++++++++++++++--
sound/soc/codecs/rt5651.h | 3 ++
3 files changed, 99 insertions(+), 2 deletions(-)
diff --git a/include/sound/rt5651.h b/include/sound/rt5651.h
index d35de758dfb5..c563383149c4 100644
--- a/include/sound/rt5651.h
+++ b/include/sound/rt5651.h
@@ -11,11 +11,18 @@
#ifndef __LINUX_SND_RT5651_H
#define __LINUX_SND_RT5651_H
+enum rt5651_jd_src {
+ RT5651_JD_NULL,
+ RT5651_JD1_1,
+};
+
struct rt5651_platform_data {
/* IN2 can optionally be differential */
bool in2_diff;
bool dmic_en;
+
+ enum rt5651_jd_src jd_src;
};
#endif
diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 28f7210cec91..9bc7f31af1a4 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -26,6 +26,7 @@
#include <sound/soc-dapm.h>
#include <sound/initval.h>
#include <sound/tlv.h>
+#include <sound/jack.h>
#include "rl6231.h"
#include "rt5651.h"
@@ -880,6 +881,9 @@ static const struct snd_soc_dapm_widget rt5651_dapm_widgets[] = {
SND_SOC_DAPM_SUPPLY("PLL1", RT5651_PWR_ANLG2,
RT5651_PWR_PLL_BIT, 0, NULL, 0),
/* Input Side */
+ SND_SOC_DAPM_SUPPLY("JD Power", RT5651_PWR_ANLG2,
+ RT5651_PWM_JD_M_BIT, 0, NULL, 0),
+
/* micbias */
SND_SOC_DAPM_SUPPLY("LDO", RT5651_PWR_ANLG1,
RT5651_PWR_LDO_BIT, 0, NULL, 0),
@@ -1528,6 +1532,8 @@ static int rt5651_set_dai_pll(struct snd_soc_dai *dai, int pll_id, int source,
static int rt5651_set_bias_level(struct snd_soc_codec *codec,
enum snd_soc_bias_level level)
{
+ struct rt5651_priv *rt5651 = snd_soc_codec_get_drvdata(codec);
+
switch (level) {
case SND_SOC_BIAS_PREPARE:
if (SND_SOC_BIAS_STANDBY == snd_soc_codec_get_bias_level(codec)) {
@@ -1556,8 +1562,13 @@ static int rt5651_set_bias_level(struct snd_soc_codec *codec,
snd_soc_write(codec, RT5651_PWR_DIG2, 0x0000);
snd_soc_write(codec, RT5651_PWR_VOL, 0x0000);
snd_soc_write(codec, RT5651_PWR_MIXER, 0x0000);
- snd_soc_write(codec, RT5651_PWR_ANLG1, 0x0000);
- snd_soc_write(codec, RT5651_PWR_ANLG2, 0x0000);
+ if (rt5651->pdata.jd_src) {
+ snd_soc_write(codec, RT5651_PWR_ANLG2, 0x0204);
+ snd_soc_write(codec, RT5651_PWR_ANLG1, 0x0002);
+ } else {
+ snd_soc_write(codec, RT5651_PWR_ANLG1, 0x0000);
+ snd_soc_write(codec, RT5651_PWR_ANLG2, 0x0000);
+ }
break;
default:
@@ -1570,6 +1581,7 @@ static int rt5651_set_bias_level(struct snd_soc_codec *codec,
static int rt5651_probe(struct snd_soc_codec *codec)
{
struct rt5651_priv *rt5651 = snd_soc_codec_get_drvdata(codec);
+ struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
rt5651->codec = codec;
@@ -1585,6 +1597,13 @@ static int rt5651_probe(struct snd_soc_codec *codec)
snd_soc_codec_force_bias_level(codec, SND_SOC_BIAS_OFF);
+ if (rt5651->pdata.jd_src) {
+ snd_soc_dapm_force_enable_pin(dapm, "JD Power");
+ snd_soc_dapm_force_enable_pin(dapm, "PLL1");
+ snd_soc_dapm_force_enable_pin(dapm, "LDO");
+ snd_soc_dapm_sync(dapm);
+ }
+
return 0;
}
@@ -1728,6 +1747,42 @@ static int rt5651_parse_dt(struct rt5651_priv *rt5651, struct device_node *np)
return 0;
}
+static irqreturn_t rt5651_irq(int irq, void *data)
+{
+ struct rt5651_priv *rt5651 = data;
+ int val, report = 0;
+
+ if (!rt5651->codec)
+ return IRQ_HANDLED;
+
+ switch (rt5651->pdata.jd_src) {
+ case RT5651_JD1_1:
+ val = snd_soc_read(rt5651->codec, RT5651_INT_IRQ_ST) & 0x1000;
+ break;
+ default:
+ return IRQ_HANDLED;
+ }
+
+ if (!val)
+ report = SND_JACK_HEADSET;
+
+ snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
+
+ return IRQ_HANDLED;
+}
+
+int rt5651_set_jack_detect(struct snd_soc_codec *codec,
+ struct snd_soc_jack *hp_jack)
+{
+ struct rt5651_priv *rt5651 = snd_soc_codec_get_drvdata(codec);
+
+ rt5651->hp_jack = hp_jack;
+ rt5651_irq(0, rt5651);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(rt5651_set_jack_detect);
+
static int rt5651_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)
{
@@ -1779,6 +1834,38 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
rt5651->hp_mute = 1;
+ switch (rt5651->pdata.jd_src) {
+ case RT5651_JD1_1:
+ /* IRQ output on GPIO1 */
+ regmap_update_bits(rt5651->regmap, RT5651_GPIO_CTRL1,
+ RT5651_GP1_PIN_MASK, RT5651_GP1_PIN_IRQ);
+
+ regmap_update_bits(rt5651->regmap, RT5651_JD_CTRL2,
+ RT5651_JD_TRG_SEL_MASK,
+ RT5651_JD_TRG_SEL_JD1_1);
+ regmap_update_bits(rt5651->regmap, RT5651_IRQ_CTRL1,
+ RT5651_JD1_1_IRQ_EN,
+ RT5651_JD1_1_IRQ_EN);
+ break;
+ case RT5651_JD_NULL:
+ break;
+ default:
+ dev_warn(&i2c->dev, "Currently only JD1_1 is supported\n");
+ break;
+ }
+
+ if (i2c->irq) {
+ ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
+ rt5651_irq,
+ IRQF_TRIGGER_RISING |
+ IRQF_TRIGGER_FALLING |
+ IRQF_ONESHOT, "rt5651", rt5651);
+ if (ret) {
+ dev_err(&i2c->dev, "Failed to reguest IRQ: %d\n", ret);
+ return ret;
+ }
+ }
+
ret = snd_soc_register_codec(&i2c->dev, &soc_codec_dev_rt5651,
rt5651_dai, ARRAY_SIZE(rt5651_dai));
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index 1bd33cfa6411..3c135f1869d4 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -2062,6 +2062,7 @@ struct rt5651_priv {
struct snd_soc_codec *codec;
struct rt5651_platform_data pdata;
struct regmap *regmap;
+ struct snd_soc_jack *hp_jack;
int sysclk;
int sysclk_src;
@@ -2077,4 +2078,6 @@ struct rt5651_priv {
bool hp_mute;
};
+int rt5651_set_jack_detect(struct snd_soc_codec *codec,
+ struct snd_soc_jack *hp_jack);
#endif /* __RT5651_H__ */
--
2.14.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] ASoC: intel: byt: Add headset jack
2017-10-19 11:03 [PATCH 0/2] rt5651: Enable jack detection Carlo Caione
2017-10-19 11:03 ` [PATCH 1/2] ASoC: rt5651: Enable jack detection on JD1_1 Carlo Caione
@ 2017-10-19 11:03 ` Carlo Caione
2017-10-19 15:56 ` Pierre-Louis Bossart
1 sibling, 1 reply; 8+ messages in thread
From: Carlo Caione @ 2017-10-19 11:03 UTC (permalink / raw)
To: linux, bardliao, oder_chiou, pierre-louis.bossart, broonie,
alsa-devel, linux-kernel, tiwai, albertchen, edgar_shen
Cc: Carlo Caione
From: Carlo Caione <carlo@endlessm.com>
Introduce an headset jack in the machine driver and register it to the
codec driver.
Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
sound/soc/intel/boards/bytcr_rt5651.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 1dad5c98c9ef..0fc850e20fc0 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -49,6 +49,8 @@ struct byt_rt5651_private {
struct clk *mclk;
};
+static struct snd_soc_jack jack;
+
static unsigned long byt_rt5651_quirk = BYT_RT5651_DMIC_MAP |
BYT_RT5651_DMIC_EN |
BYT_RT5651_MCLK_EN;
@@ -177,6 +179,17 @@ static const struct snd_kcontrol_new byt_rt5651_controls[] = {
SOC_DAPM_PIN_SWITCH("Speaker"),
};
+static struct snd_soc_jack_pin bytcr_jack_pins[] = {
+ {
+ .pin = "Headphone",
+ .mask = SND_JACK_HEADPHONE,
+ },
+ {
+ .pin = "Headset Mic",
+ .mask = SND_JACK_MICROPHONE,
+ },
+};
+
static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)
{
@@ -244,6 +257,7 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
{
struct snd_soc_card *card = runtime->card;
+ struct snd_soc_codec *codec = runtime->codec;
struct byt_rt5651_private *priv = snd_soc_card_get_drvdata(card);
const struct snd_soc_dapm_route *custom_map;
int num_routes;
@@ -301,6 +315,16 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
dev_err(card->dev, "unable to set MCLK rate\n");
}
+ ret = snd_soc_card_jack_new(runtime->card, "Headset",
+ SND_JACK_HEADSET, &jack,
+ bytcr_jack_pins, ARRAY_SIZE(bytcr_jack_pins));
+ if (ret) {
+ dev_err(runtime->dev, "Headset jack creation failed %d\n", ret);
+ return ret;
+ }
+
+ rt5651_set_jack_detect(codec, &jack);
+
return ret;
}
--
2.14.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [alsa-devel] [PATCH 1/2] ASoC: rt5651: Enable jack detection on JD1_1
2017-10-19 11:03 ` [PATCH 1/2] ASoC: rt5651: Enable jack detection on JD1_1 Carlo Caione
@ 2017-10-19 15:47 ` Pierre-Louis Bossart
2017-10-19 15:54 ` Carlo Caione
2017-10-20 2:23 ` Bard Liao
1 sibling, 1 reply; 8+ messages in thread
From: Pierre-Louis Bossart @ 2017-10-19 15:47 UTC (permalink / raw)
To: Carlo Caione, linux, bardliao, oder_chiou, broonie, alsa-devel,
linux-kernel, tiwai, albertchen, edgar_shen
Cc: Carlo Caione
On 10/19/17 6:03 AM, Carlo Caione wrote:
> From: Carlo Caione <carlo@endlessm.com>
>
> Enable jack detection or the RT5651 codec on the JD1_1 pin.
Nice, but the codec supports a second jack detection on JD1 and has a
second JD2 pin. I will bet that some devices will have a different
routing and I wonder if we could just add support for all options.
> The codec has no means to detect the type of the jack connected so we
> assume that the jack is always an headset jack.
that's odd, was this confirmed by Realtek?
>
> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> ---
> include/sound/rt5651.h | 7 ++++
> sound/soc/codecs/rt5651.c | 91 +++++++++++++++++++++++++++++++++++++++++++++--
> sound/soc/codecs/rt5651.h | 3 ++
> 3 files changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/include/sound/rt5651.h b/include/sound/rt5651.h
> index d35de758dfb5..c563383149c4 100644
> --- a/include/sound/rt5651.h
> +++ b/include/sound/rt5651.h
> @@ -11,11 +11,18 @@
> #ifndef __LINUX_SND_RT5651_H
> #define __LINUX_SND_RT5651_H
>
> +enum rt5651_jd_src {
> + RT5651_JD_NULL,
> + RT5651_JD1_1,
> +};
> +
> struct rt5651_platform_data {
> /* IN2 can optionally be differential */
> bool in2_diff;
>
> bool dmic_en;
> +
> + enum rt5651_jd_src jd_src;
I don't see code that sets this platform data, is there a quirk or
of_property missing in this patchset?
> };
>
> #endif
> diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
> index 28f7210cec91..9bc7f31af1a4 100644
> --- a/sound/soc/codecs/rt5651.c
> +++ b/sound/soc/codecs/rt5651.c
> @@ -26,6 +26,7 @@
> #include <sound/soc-dapm.h>
> #include <sound/initval.h>
> #include <sound/tlv.h>
> +#include <sound/jack.h>
>
> #include "rl6231.h"
> #include "rt5651.h"
> @@ -880,6 +881,9 @@ static const struct snd_soc_dapm_widget rt5651_dapm_widgets[] = {
> SND_SOC_DAPM_SUPPLY("PLL1", RT5651_PWR_ANLG2,
> RT5651_PWR_PLL_BIT, 0, NULL, 0),
> /* Input Side */
> + SND_SOC_DAPM_SUPPLY("JD Power", RT5651_PWR_ANLG2,
> + RT5651_PWM_JD_M_BIT, 0, NULL, 0),
> +
> /* micbias */
> SND_SOC_DAPM_SUPPLY("LDO", RT5651_PWR_ANLG1,
> RT5651_PWR_LDO_BIT, 0, NULL, 0),
> @@ -1528,6 +1532,8 @@ static int rt5651_set_dai_pll(struct snd_soc_dai *dai, int pll_id, int source,
> static int rt5651_set_bias_level(struct snd_soc_codec *codec,
> enum snd_soc_bias_level level)
> {
> + struct rt5651_priv *rt5651 = snd_soc_codec_get_drvdata(codec);
> +
> switch (level) {
> case SND_SOC_BIAS_PREPARE:
> if (SND_SOC_BIAS_STANDBY == snd_soc_codec_get_bias_level(codec)) {
> @@ -1556,8 +1562,13 @@ static int rt5651_set_bias_level(struct snd_soc_codec *codec,
> snd_soc_write(codec, RT5651_PWR_DIG2, 0x0000);
> snd_soc_write(codec, RT5651_PWR_VOL, 0x0000);
> snd_soc_write(codec, RT5651_PWR_MIXER, 0x0000);
> - snd_soc_write(codec, RT5651_PWR_ANLG1, 0x0000);
> - snd_soc_write(codec, RT5651_PWR_ANLG2, 0x0000);
> + if (rt5651->pdata.jd_src) {
> + snd_soc_write(codec, RT5651_PWR_ANLG2, 0x0204);
> + snd_soc_write(codec, RT5651_PWR_ANLG1, 0x0002);
> + } else {
> + snd_soc_write(codec, RT5651_PWR_ANLG1, 0x0000);
> + snd_soc_write(codec, RT5651_PWR_ANLG2, 0x0000);
> + }
> break;
>
> default:
> @@ -1570,6 +1581,7 @@ static int rt5651_set_bias_level(struct snd_soc_codec *codec,
> static int rt5651_probe(struct snd_soc_codec *codec)
> {
> struct rt5651_priv *rt5651 = snd_soc_codec_get_drvdata(codec);
> + struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
>
> rt5651->codec = codec;
>
> @@ -1585,6 +1597,13 @@ static int rt5651_probe(struct snd_soc_codec *codec)
>
> snd_soc_codec_force_bias_level(codec, SND_SOC_BIAS_OFF);
>
> + if (rt5651->pdata.jd_src) {
> + snd_soc_dapm_force_enable_pin(dapm, "JD Power");
> + snd_soc_dapm_force_enable_pin(dapm, "PLL1");
> + snd_soc_dapm_force_enable_pin(dapm, "LDO");
> + snd_soc_dapm_sync(dapm);
> + }
> +
> return 0;
> }
>
> @@ -1728,6 +1747,42 @@ static int rt5651_parse_dt(struct rt5651_priv *rt5651, struct device_node *np)
> return 0;
> }
>
> +static irqreturn_t rt5651_irq(int irq, void *data)
> +{
> + struct rt5651_priv *rt5651 = data;
> + int val, report = 0;
> +
> + if (!rt5651->codec)
> + return IRQ_HANDLED;
> +
> + switch (rt5651->pdata.jd_src) {
> + case RT5651_JD1_1:
> + val = snd_soc_read(rt5651->codec, RT5651_INT_IRQ_ST) & 0x1000;
> + break;
> + default:
> + return IRQ_HANDLED;
> + }
> +
> + if (!val)
> + report = SND_JACK_HEADSET;
> +
> + snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
> +
> + return IRQ_HANDLED;
> +}
> +
> +int rt5651_set_jack_detect(struct snd_soc_codec *codec,
> + struct snd_soc_jack *hp_jack)
> +{
> + struct rt5651_priv *rt5651 = snd_soc_codec_get_drvdata(codec);
> +
> + rt5651->hp_jack = hp_jack;
> + rt5651_irq(0, rt5651);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(rt5651_set_jack_detect);
> +
> static int rt5651_i2c_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id)
> {
> @@ -1779,6 +1834,38 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
>
> rt5651->hp_mute = 1;
>
> + switch (rt5651->pdata.jd_src) {
> + case RT5651_JD1_1:
> + /* IRQ output on GPIO1 */
> + regmap_update_bits(rt5651->regmap, RT5651_GPIO_CTRL1,
> + RT5651_GP1_PIN_MASK, RT5651_GP1_PIN_IRQ);
> +
> + regmap_update_bits(rt5651->regmap, RT5651_JD_CTRL2,
> + RT5651_JD_TRG_SEL_MASK,
> + RT5651_JD_TRG_SEL_JD1_1);
> + regmap_update_bits(rt5651->regmap, RT5651_IRQ_CTRL1,
> + RT5651_JD1_1_IRQ_EN,
> + RT5651_JD1_1_IRQ_EN);
> + break;
> + case RT5651_JD_NULL:
> + break;
> + default:
> + dev_warn(&i2c->dev, "Currently only JD1_1 is supported\n");
> + break;
> + }
> +
> + if (i2c->irq) {
> + ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
> + rt5651_irq,
> + IRQF_TRIGGER_RISING |
> + IRQF_TRIGGER_FALLING |
> + IRQF_ONESHOT, "rt5651", rt5651);
> + if (ret) {
> + dev_err(&i2c->dev, "Failed to reguest IRQ: %d\n", ret);
> + return ret;
> + }
> + }
> +
> ret = snd_soc_register_codec(&i2c->dev, &soc_codec_dev_rt5651,
> rt5651_dai, ARRAY_SIZE(rt5651_dai));
>
> diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
> index 1bd33cfa6411..3c135f1869d4 100644
> --- a/sound/soc/codecs/rt5651.h
> +++ b/sound/soc/codecs/rt5651.h
> @@ -2062,6 +2062,7 @@ struct rt5651_priv {
> struct snd_soc_codec *codec;
> struct rt5651_platform_data pdata;
> struct regmap *regmap;
> + struct snd_soc_jack *hp_jack;
>
> int sysclk;
> int sysclk_src;
> @@ -2077,4 +2078,6 @@ struct rt5651_priv {
> bool hp_mute;
> };
>
> +int rt5651_set_jack_detect(struct snd_soc_codec *codec,
> + struct snd_soc_jack *hp_jack);
> #endif /* __RT5651_H__ */
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [alsa-devel] [PATCH 1/2] ASoC: rt5651: Enable jack detection on JD1_1
2017-10-19 15:47 ` [alsa-devel] " Pierre-Louis Bossart
@ 2017-10-19 15:54 ` Carlo Caione
2017-10-20 2:10 ` Bard Liao
0 siblings, 1 reply; 8+ messages in thread
From: Carlo Caione @ 2017-10-19 15:54 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Carlo Caione, Linux Upstreaming Team, Bard Liao, Oder Chiou,
Mark Brown, alsa-devel, linux-kernel, tiwai, Albert Chen,
Edgar Shen
On Thu, Oct 19, 2017 at 4:47 PM, Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
> On 10/19/17 6:03 AM, Carlo Caione wrote:
>>
>> From: Carlo Caione <carlo@endlessm.com>
>>
>> Enable jack detection or the RT5651 codec on the JD1_1 pin.
>
> Nice, but the codec supports a second jack detection on JD1 and has a second
> JD2 pin. I will bet that some devices will have a different routing and I
> wonder if we could just add support for all options.
I can write support for that but I have no hardware to actually test
it, that's why I left those cases out.
>> The codec has no means to detect the type of the jack connected so we
>> assume that the jack is always an headset jack.
>
> that's odd, was this confirmed by Realtek?
The Realtek people are in CC :)
Probably there is way but in the datasheet there is nothing about that
(or did I miss it?)
>> Signed-off-by: Carlo Caione <carlo@endlessm.com>
>> ---
>> include/sound/rt5651.h | 7 ++++
>> sound/soc/codecs/rt5651.c | 91
>> +++++++++++++++++++++++++++++++++++++++++++++--
>> sound/soc/codecs/rt5651.h | 3 ++
>> 3 files changed, 99 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/sound/rt5651.h b/include/sound/rt5651.h
>> index d35de758dfb5..c563383149c4 100644
>> --- a/include/sound/rt5651.h
>> +++ b/include/sound/rt5651.h
>> @@ -11,11 +11,18 @@
>> #ifndef __LINUX_SND_RT5651_H
>> #define __LINUX_SND_RT5651_H
>> +enum rt5651_jd_src {
>> + RT5651_JD_NULL,
>> + RT5651_JD1_1,
>> +};
>> +
>> struct rt5651_platform_data {
>> /* IN2 can optionally be differential */
>> bool in2_diff;
>> bool dmic_en;
>> +
>> + enum rt5651_jd_src jd_src;
>
>
> I don't see code that sets this platform data, is there a quirk or
> of_property missing in this patchset?
Yes, it is supposed to be enabled by a quirk. In general (personal
taste) I prefer to post the quirk enabling code after the base code
has been ACKed. If you feel like it I can post also the quirk code
together with the next respin of this patchset.
Thank you,
--
Carlo Caione | +39.340.80.30.096 | Endless
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] ASoC: intel: byt: Add headset jack
2017-10-19 11:03 ` [PATCH 2/2] ASoC: intel: byt: Add headset jack Carlo Caione
@ 2017-10-19 15:56 ` Pierre-Louis Bossart
0 siblings, 0 replies; 8+ messages in thread
From: Pierre-Louis Bossart @ 2017-10-19 15:56 UTC (permalink / raw)
To: Carlo Caione, linux, bardliao, oder_chiou, broonie, alsa-devel,
linux-kernel, tiwai, albertchen, edgar_shen
Cc: Carlo Caione
On 10/19/17 6:03 AM, Carlo Caione wrote:
> From: Carlo Caione <carlo@endlessm.com>
>
> Introduce an headset jack in the machine driver and register it to the
> codec driver.
>
> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> ---
> sound/soc/intel/boards/bytcr_rt5651.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
> index 1dad5c98c9ef..0fc850e20fc0 100644
> --- a/sound/soc/intel/boards/bytcr_rt5651.c
> +++ b/sound/soc/intel/boards/bytcr_rt5651.c
> @@ -49,6 +49,8 @@ struct byt_rt5651_private {
> struct clk *mclk;
> };
>
> +static struct snd_soc_jack jack;
nit-pick: we usually push the jack into the structure just above.
> +
> static unsigned long byt_rt5651_quirk = BYT_RT5651_DMIC_MAP |
> BYT_RT5651_DMIC_EN |
> BYT_RT5651_MCLK_EN;
wondering if we need quirks here to manage the IN2P/JD2 exclusion at
some point.
> @@ -177,6 +179,17 @@ static const struct snd_kcontrol_new byt_rt5651_controls[] = {
> SOC_DAPM_PIN_SWITCH("Speaker"),
> };
>
> +static struct snd_soc_jack_pin bytcr_jack_pins[] = {
> + {
> + .pin = "Headphone",
> + .mask = SND_JACK_HEADPHONE,
> + },
> + {
> + .pin = "Headset Mic",
> + .mask = SND_JACK_MICROPHONE,
> + },
> +};
> +
> static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
> struct snd_pcm_hw_params *params)
> {
> @@ -244,6 +257,7 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
> static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
> {
> struct snd_soc_card *card = runtime->card;
> + struct snd_soc_codec *codec = runtime->codec;
> struct byt_rt5651_private *priv = snd_soc_card_get_drvdata(card);
> const struct snd_soc_dapm_route *custom_map;
> int num_routes;
> @@ -301,6 +315,16 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
> dev_err(card->dev, "unable to set MCLK rate\n");
> }
>
> + ret = snd_soc_card_jack_new(runtime->card, "Headset",
> + SND_JACK_HEADSET, &jack,
> + bytcr_jack_pins, ARRAY_SIZE(bytcr_jack_pins));
> + if (ret) {
> + dev_err(runtime->dev, "Headset jack creation failed %d\n", ret);
> + return ret;
> + }
> +
> + rt5651_set_jack_detect(codec, &jack);
> +
> return ret;
> }
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [alsa-devel] [PATCH 1/2] ASoC: rt5651: Enable jack detection on JD1_1
2017-10-19 15:54 ` Carlo Caione
@ 2017-10-20 2:10 ` Bard Liao
0 siblings, 0 replies; 8+ messages in thread
From: Bard Liao @ 2017-10-20 2:10 UTC (permalink / raw)
To: Carlo Caione, Pierre-Louis Bossart
Cc: Carlo Caione, Linux Upstreaming Team, Oder Chiou, Mark Brown,
alsa-devel, linux-kernel, tiwai, Albert Chen, Edgar Shen
> -----Original Message-----
> From: Carlo Caione [mailto:carlo@endlessm.com]
> Sent: Thursday, October 19, 2017 11:55 PM
> To: Pierre-Louis Bossart
> Cc: Carlo Caione; Linux Upstreaming Team; Bard Liao; Oder Chiou; Mark
> Brown; alsa-devel@alsa-project.org; linux-kernel@vger.kernel.org;
> tiwai@suse.com; Albert Chen; Edgar Shen
> Subject: Re: [alsa-devel] [PATCH 1/2] ASoC: rt5651: Enable jack detection on
> JD1_1
>
> On Thu, Oct 19, 2017 at 4:47 PM, Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
> > On 10/19/17 6:03 AM, Carlo Caione wrote:
> >>
> >> From: Carlo Caione <carlo@endlessm.com>
> >>
> >> Enable jack detection or the RT5651 codec on the JD1_1 pin.
> >
> > Nice, but the codec supports a second jack detection on JD1 and has a
> second
> > JD2 pin. I will bet that some devices will have a different routing and I
> > wonder if we could just add support for all options.
>
> I can write support for that but I have no hardware to actually test
> it, that's why I left those cases out.
>
> >> The codec has no means to detect the type of the jack connected so we
> >> assume that the jack is always an headset jack.
> >
> > that's odd, was this confirmed by Realtek?
>
> The Realtek people are in CC :)
> Probably there is way but in the datasheet there is nothing about that
> (or did I miss it?)
Yes, rt5651 has the capability of jack type detection.
Please see the following code for reference.
+static int rt5651_jack_detect(struct snd_soc_codec *codec, int jack_insert)
+{
+ struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
+ int jack_type;
+
+ if (jack_insert) {
+ snd_soc_dapm_force_enable_pin(dapm, "LDO");
+ snd_soc_dapm_sync(dapm);
+
+ snd_soc_update_bits(codec, RT5651_MICBIAS,
+ RT5651_MIC1_OVCD_MASK | RT5651_MIC1_OVTH_MASK |
+ RT5651_PWR_CLK12M_MASK | RT5651_PWR_MB_MASK,
+ RT5651_MIC1_OVCD_EN | RT5651_MIC1_OVTH_600UA |
+ RT5651_PWR_MB_PU | RT5651_PWR_CLK12M_PU);
+ msleep(100);
+ if (snd_soc_read(codec, RT5651_IRQ_CTRL2) & RT5651_MB1_OC_CLR)
+ jack_type = SND_JACK_HEADPHONE;
+ else
+ jack_type = SND_JACK_HEADSET;
+ snd_soc_update_bits(codec, RT5651_IRQ_CTRL2,
+ RT5651_MB1_OC_CLR, 0);
+ } else { /* jack out */
+ jack_type = 0;
+
+ snd_soc_update_bits(codec, RT5651_MICBIAS,
+ RT5651_MIC1_OVCD_MASK, RT5651_MIC1_OVCD_DIS);
+ }
+
+ return jack_type;
+}
>
> >> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> >> ---
> >> include/sound/rt5651.h | 7 ++++
> >> sound/soc/codecs/rt5651.c | 91
> >> +++++++++++++++++++++++++++++++++++++++++++++--
> >> sound/soc/codecs/rt5651.h | 3 ++
> >> 3 files changed, 99 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/sound/rt5651.h b/include/sound/rt5651.h
> >> index d35de758dfb5..c563383149c4 100644
> >> --- a/include/sound/rt5651.h
> >> +++ b/include/sound/rt5651.h
> >> @@ -11,11 +11,18 @@
> >> #ifndef __LINUX_SND_RT5651_H
> >> #define __LINUX_SND_RT5651_H
> >> +enum rt5651_jd_src {
> >> + RT5651_JD_NULL,
> >> + RT5651_JD1_1,
> >> +};
> >> +
> >> struct rt5651_platform_data {
> >> /* IN2 can optionally be differential */
> >> bool in2_diff;
> >> bool dmic_en;
> >> +
> >> + enum rt5651_jd_src jd_src;
> >
> >
> > I don't see code that sets this platform data, is there a quirk or
> > of_property missing in this patchset?
>
> Yes, it is supposed to be enabled by a quirk. In general (personal
> taste) I prefer to post the quirk enabling code after the base code
> has been ACKed. If you feel like it I can post also the quirk code
> together with the next respin of this patchset.
>
> Thank you,
>
> --
> Carlo Caione | +39.340.80.30.096 | Endless
>
> ------Please consider the environment before printing this e-mail.
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 1/2] ASoC: rt5651: Enable jack detection on JD1_1
2017-10-19 11:03 ` [PATCH 1/2] ASoC: rt5651: Enable jack detection on JD1_1 Carlo Caione
2017-10-19 15:47 ` [alsa-devel] " Pierre-Louis Bossart
@ 2017-10-20 2:23 ` Bard Liao
1 sibling, 0 replies; 8+ messages in thread
From: Bard Liao @ 2017-10-20 2:23 UTC (permalink / raw)
To: Carlo Caione, linux, Oder Chiou, pierre-louis.bossart, broonie,
alsa-devel, linux-kernel, tiwai, Albert Chen, Edgar Shen
Cc: Carlo Caione
> -----Original Message-----
> From: Carlo Caione [mailto:carlo.caione@gmail.com] On Behalf Of Carlo
> Caione
> Sent: Thursday, October 19, 2017 7:03 PM
> To: linux@endlessm.com; Bard Liao; Oder Chiou;
> pierre-louis.bossart@linux.intel.com; broonie@kernel.org;
> alsa-devel@alsa-project.org; linux-kernel@vger.kernel.org; tiwai@suse.com;
> Albert Chen; Edgar Shen
> Cc: Carlo Caione
> Subject: [PATCH 1/2] ASoC: rt5651: Enable jack detection on JD1_1
>
> From: Carlo Caione <carlo@endlessm.com>
>
> Enable jack detection or the RT5651 codec on the JD1_1 pin.
> The codec has no means to detect the type of the jack connected so we
> assume that the jack is always an headset jack.
>
> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> ---
> include/sound/rt5651.h | 7 ++++
> sound/soc/codecs/rt5651.c | 91
> +++++++++++++++++++++++++++++++++++++++++++++--
> sound/soc/codecs/rt5651.h | 3 ++
> 3 files changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/include/sound/rt5651.h b/include/sound/rt5651.h
> index d35de758dfb5..c563383149c4 100644
> --- a/include/sound/rt5651.h
> +++ b/include/sound/rt5651.h
> @@ -11,11 +11,18 @@
> #ifndef __LINUX_SND_RT5651_H
> #define __LINUX_SND_RT5651_H
>
> +enum rt5651_jd_src {
> + RT5651_JD_NULL,
> + RT5651_JD1_1,
> +};
> +
> struct rt5651_platform_data {
> /* IN2 can optionally be differential */
> bool in2_diff;
>
> bool dmic_en;
> +
> + enum rt5651_jd_src jd_src;
> };
>
> #endif
> diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
> index 28f7210cec91..9bc7f31af1a4 100644
> --- a/sound/soc/codecs/rt5651.c
> +++ b/sound/soc/codecs/rt5651.c
> @@ -26,6 +26,7 @@
> #include <sound/soc-dapm.h>
> #include <sound/initval.h>
> #include <sound/tlv.h>
> +#include <sound/jack.h>
>
> #include "rl6231.h"
> #include "rt5651.h"
> @@ -880,6 +881,9 @@ static const struct snd_soc_dapm_widget
> rt5651_dapm_widgets[] = {
> SND_SOC_DAPM_SUPPLY("PLL1", RT5651_PWR_ANLG2,
> RT5651_PWR_PLL_BIT, 0, NULL, 0),
> /* Input Side */
> + SND_SOC_DAPM_SUPPLY("JD Power", RT5651_PWR_ANLG2,
> + RT5651_PWM_JD_M_BIT, 0, NULL, 0),
> +
> /* micbias */
> SND_SOC_DAPM_SUPPLY("LDO", RT5651_PWR_ANLG1,
> RT5651_PWR_LDO_BIT, 0, NULL, 0),
> @@ -1528,6 +1532,8 @@ static int rt5651_set_dai_pll(struct snd_soc_dai *dai,
> int pll_id, int source,
> static int rt5651_set_bias_level(struct snd_soc_codec *codec,
> enum snd_soc_bias_level level)
> {
> + struct rt5651_priv *rt5651 = snd_soc_codec_get_drvdata(codec);
> +
> switch (level) {
> case SND_SOC_BIAS_PREPARE:
> if (SND_SOC_BIAS_STANDBY ==
> snd_soc_codec_get_bias_level(codec)) {
> @@ -1556,8 +1562,13 @@ static int rt5651_set_bias_level(struct
> snd_soc_codec *codec,
> snd_soc_write(codec, RT5651_PWR_DIG2, 0x0000);
> snd_soc_write(codec, RT5651_PWR_VOL, 0x0000);
> snd_soc_write(codec, RT5651_PWR_MIXER, 0x0000);
> - snd_soc_write(codec, RT5651_PWR_ANLG1, 0x0000);
> - snd_soc_write(codec, RT5651_PWR_ANLG2, 0x0000);
> + if (rt5651->pdata.jd_src) {
> + snd_soc_write(codec, RT5651_PWR_ANLG2, 0x0204);
> + snd_soc_write(codec, RT5651_PWR_ANLG1, 0x0002);
> + } else {
> + snd_soc_write(codec, RT5651_PWR_ANLG1, 0x0000);
> + snd_soc_write(codec, RT5651_PWR_ANLG2, 0x0000);
> + }
> break;
>
> default:
> @@ -1570,6 +1581,7 @@ static int rt5651_set_bias_level(struct
> snd_soc_codec *codec,
> static int rt5651_probe(struct snd_soc_codec *codec)
> {
> struct rt5651_priv *rt5651 = snd_soc_codec_get_drvdata(codec);
> + struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
>
> rt5651->codec = codec;
>
> @@ -1585,6 +1597,13 @@ static int rt5651_probe(struct snd_soc_codec
> *codec)
>
> snd_soc_codec_force_bias_level(codec, SND_SOC_BIAS_OFF);
>
> + if (rt5651->pdata.jd_src) {
> + snd_soc_dapm_force_enable_pin(dapm, "JD Power");
> + snd_soc_dapm_force_enable_pin(dapm, "PLL1");
"PLL1" is not needed here. What we need is
+ regmap_update_bits(rt5651->regmap, RT5651_MICBIAS,
+ 0x38, 0x38);
It is not showing in the datasheet. Sorry about that.
> + snd_soc_dapm_force_enable_pin(dapm, "LDO");
> + snd_soc_dapm_sync(dapm);
> + }
> +
> return 0;
> }
>
> @@ -1728,6 +1747,42 @@ static int rt5651_parse_dt(struct rt5651_priv
> *rt5651, struct device_node *np)
> return 0;
> }
>
> +static irqreturn_t rt5651_irq(int irq, void *data)
> +{
> + struct rt5651_priv *rt5651 = data;
> + int val, report = 0;
> +
> + if (!rt5651->codec)
> + return IRQ_HANDLED;
> +
> + switch (rt5651->pdata.jd_src) {
> + case RT5651_JD1_1:
> + val = snd_soc_read(rt5651->codec, RT5651_INT_IRQ_ST) & 0x1000;
> + break;
> + default:
> + return IRQ_HANDLED;
> + }
> +
> + if (!val)
> + report = SND_JACK_HEADSET;
> +
> + snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
> +
> + return IRQ_HANDLED;
> +}
> +
> +int rt5651_set_jack_detect(struct snd_soc_codec *codec,
> + struct snd_soc_jack *hp_jack)
> +{
> + struct rt5651_priv *rt5651 = snd_soc_codec_get_drvdata(codec);
> +
> + rt5651->hp_jack = hp_jack;
> + rt5651_irq(0, rt5651);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(rt5651_set_jack_detect);
> +
> static int rt5651_i2c_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id)
> {
> @@ -1779,6 +1834,38 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
>
> rt5651->hp_mute = 1;
>
> + switch (rt5651->pdata.jd_src) {
> + case RT5651_JD1_1:
> + /* IRQ output on GPIO1 */
> + regmap_update_bits(rt5651->regmap, RT5651_GPIO_CTRL1,
> + RT5651_GP1_PIN_MASK, RT5651_GP1_PIN_IRQ);
> +
> + regmap_update_bits(rt5651->regmap, RT5651_JD_CTRL2,
> + RT5651_JD_TRG_SEL_MASK,
> + RT5651_JD_TRG_SEL_JD1_1);
> + regmap_update_bits(rt5651->regmap, RT5651_IRQ_CTRL1,
> + RT5651_JD1_1_IRQ_EN,
> + RT5651_JD1_1_IRQ_EN);
> + break;
> + case RT5651_JD_NULL:
> + break;
> + default:
> + dev_warn(&i2c->dev, "Currently only JD1_1 is supported\n");
> + break;
> + }
> +
> + if (i2c->irq) {
> + ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
> + rt5651_irq,
> + IRQF_TRIGGER_RISING |
> + IRQF_TRIGGER_FALLING |
> + IRQF_ONESHOT, "rt5651", rt5651);
> + if (ret) {
> + dev_err(&i2c->dev, "Failed to reguest IRQ: %d\n", ret);
> + return ret;
> + }
> + }
> +
> ret = snd_soc_register_codec(&i2c->dev, &soc_codec_dev_rt5651,
> rt5651_dai, ARRAY_SIZE(rt5651_dai));
>
> diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
> index 1bd33cfa6411..3c135f1869d4 100644
> --- a/sound/soc/codecs/rt5651.h
> +++ b/sound/soc/codecs/rt5651.h
> @@ -2062,6 +2062,7 @@ struct rt5651_priv {
> struct snd_soc_codec *codec;
> struct rt5651_platform_data pdata;
> struct regmap *regmap;
> + struct snd_soc_jack *hp_jack;
>
> int sysclk;
> int sysclk_src;
> @@ -2077,4 +2078,6 @@ struct rt5651_priv {
> bool hp_mute;
> };
>
> +int rt5651_set_jack_detect(struct snd_soc_codec *codec,
> + struct snd_soc_jack *hp_jack);
> #endif /* __RT5651_H__ */
> --
> 2.14.2
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-10-20 2:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-19 11:03 [PATCH 0/2] rt5651: Enable jack detection Carlo Caione
2017-10-19 11:03 ` [PATCH 1/2] ASoC: rt5651: Enable jack detection on JD1_1 Carlo Caione
2017-10-19 15:47 ` [alsa-devel] " Pierre-Louis Bossart
2017-10-19 15:54 ` Carlo Caione
2017-10-20 2:10 ` Bard Liao
2017-10-20 2:23 ` Bard Liao
2017-10-19 11:03 ` [PATCH 2/2] ASoC: intel: byt: Add headset jack Carlo Caione
2017-10-19 15:56 ` Pierre-Louis Bossart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).