diff mbox

ASoC: rt5670: add HS ground control

Message ID 1482999827-14705-1-git-send-email-bardliao@realtek.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bard Liao Dec. 29, 2016, 8:23 a.m. UTC 
Some project use external circuit to prevent the headset ground noise.
It will need to use GPIO to control it. Dell Wyse 3040 is the only project
that has the external circuit so far and it uses GPIO2 for that. This patch
provide the support fot it.

Signed-off-by: Bard Liao <bardliao@realtek.com>
---
 include/sound/rt5670.h    |  7 +++++
 sound/soc/codecs/rt5670.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 80 insertions(+), 1 deletion(-)

Comments

Takashi Iwai Jan. 2, 2017, 12:03 p.m. UTC | #1 
On Thu, 29 Dec 2016 09:23:47 +0100,
Bard Liao wrote:
> 
> Some project use external circuit to prevent the headset ground noise.
> It will need to use GPIO to control it. Dell Wyse 3040 is the only project
> that has the external circuit so far and it uses GPIO2 for that. This patch
> provide the support fot it.
> 
> Signed-off-by: Bard Liao <bardliao@realtek.com>

Tested-by: Takashi Iwai <tiwai@suse.de>
Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

> ---
>  include/sound/rt5670.h    |  7 +++++
>  sound/soc/codecs/rt5670.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 80 insertions(+), 1 deletion(-)
> 
> diff --git a/include/sound/rt5670.h b/include/sound/rt5670.h
> index b7d6051..0b445b0 100644
> --- a/include/sound/rt5670.h
> +++ b/include/sound/rt5670.h
> @@ -11,12 +11,19 @@
>  #ifndef __LINUX_SND_RT5670_H
>  #define __LINUX_SND_RT5670_H
>  
> +enum rt5670_gpios {
> +	RT5670_GPIO_NONE = 0,
> +	RT5670_GPIO2,
> +};
> +
> +
>  struct rt5670_platform_data {
>  	int jd_mode;
>  	bool in2_diff;
>  	bool dev_gpio;
>  
>  	bool dmic_en;
> +	enum rt5670_gpios hs_ground_control_gpio;
>  	unsigned int dmic1_data_pin;
>  	/* 0 = GPIO6; 1 = IN2P; 3 = GPIO7*/
>  	unsigned int dmic2_data_pin;
> diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c
> index 97bafac..1d94cec 100644
> --- a/sound/soc/codecs/rt5670.c
> +++ b/sound/soc/codecs/rt5670.c
> @@ -420,13 +420,15 @@ static int rt5670_headset_detect(struct snd_soc_codec *codec, int jack_insert)
>  	struct rt5670_priv *rt5670 = snd_soc_codec_get_drvdata(codec);
>  
>  	if (jack_insert) {
> +		if (rt5670->pdata.hs_ground_control_gpio)
> +			snd_soc_dapm_force_enable_pin(dapm, "HS GND Control");
> +
>  		snd_soc_dapm_force_enable_pin(dapm, "Mic Det Power");
>  		snd_soc_dapm_sync(dapm);
>  		snd_soc_update_bits(codec, RT5670_GEN_CTRL3, 0x4, 0x0);
>  		snd_soc_update_bits(codec, RT5670_CJ_CTRL2,
>  			RT5670_CBJ_DET_MODE | RT5670_CBJ_MN_JD,
>  			RT5670_CBJ_MN_JD);
> -		snd_soc_write(codec, RT5670_GPIO_CTRL2, 0x0004);
>  		snd_soc_update_bits(codec, RT5670_GPIO_CTRL1,
>  			RT5670_GP1_PIN_MASK, RT5670_GP1_PIN_IRQ);
>  		snd_soc_update_bits(codec, RT5670_CJ_CTRL1,
> @@ -450,6 +452,10 @@ static int rt5670_headset_detect(struct snd_soc_codec *codec, int jack_insert)
>  			snd_soc_dapm_disable_pin(dapm, "Mic Det Power");
>  			snd_soc_dapm_sync(dapm);
>  		}
> +		if (rt5670->pdata.hs_ground_control_gpio) {
> +			snd_soc_dapm_disable_pin(dapm, "HS GND Control");
> +			snd_soc_dapm_sync(dapm);
> +		}
>  	} else {
>  		snd_soc_update_bits(codec, RT5670_INT_IRQ_ST, 0x8, 0x0);
>  		snd_soc_update_bits(codec, RT5670_GEN_CTRL3, 0x4, 0x4);
> @@ -1542,6 +1548,29 @@ static int rt5670_bst2_event(struct snd_soc_dapm_widget *w,
>  	return 0;
>  }
>  
> +static int rt5670_hs_ground_control_event(struct snd_soc_dapm_widget *w,
> +	struct snd_kcontrol *kcontrol, int event)
> +{
> +	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
> +
> +	switch (event) {
> +	case SND_SOC_DAPM_PRE_PMU:
> +		snd_soc_update_bits(codec, RT5670_GPIO_CTRL2,
> +				RT5670_GP2_OUT_MASK, RT5670_GP2_OUT_HI);
> +		break;
> +
> +	case SND_SOC_DAPM_POST_PMD:
> +		snd_soc_update_bits(codec, RT5670_GPIO_CTRL2,
> +				RT5670_GP2_OUT_MASK, RT5670_GP2_OUT_LO);
> +		break;
> +
> +	default:
> +		return 0;
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct snd_soc_dapm_widget rt5670_dapm_widgets[] = {
>  	SND_SOC_DAPM_SUPPLY("PLL1", RT5670_PWR_ANLG2,
>  			    RT5670_PWR_PLL_BIT, 0, NULL, 0),
> @@ -1917,6 +1946,12 @@ static int rt5670_bst2_event(struct snd_soc_dapm_widget *w,
>  	SND_SOC_DAPM_OUTPUT("SPORN"),
>  };
>  
> +static const struct snd_soc_dapm_widget hs_gnd_cnotrol_widgets[] = {
> +	SND_SOC_DAPM_SUPPLY("HS GND Control", SND_SOC_NOPM, 0, 0,
> +			rt5670_hs_ground_control_event,
> +			SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
> +};
> +
>  static const struct snd_soc_dapm_route rt5670_dapm_routes[] = {
>  	{ "ADC Stereo1 Filter", NULL, "ADC STO1 ASRC", is_using_asrc },
>  	{ "ADC Stereo2 Filter", NULL, "ADC STO2 ASRC", is_using_asrc },
> @@ -2314,6 +2349,11 @@ static int rt5670_bst2_event(struct snd_soc_dapm_widget *w,
>  	{ "SPORN", NULL, "SPO Amp" },
>  };
>  
> +static const struct snd_soc_dapm_route hs_gnd_cnotrol_routes[] = {
> +	{ "BST1", NULL, "HS GND Control" },
> +	{ "HP Amp", NULL, "HS GND Control" },
> +};
> +
>  static int rt5670_hw_params(struct snd_pcm_substream *substream,
>  	struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
>  {
> @@ -2679,6 +2719,13 @@ static int rt5670_probe(struct snd_soc_codec *codec)
>  			"The driver is for RT5670 RT5671 or RT5672 only\n");
>  		return -ENODEV;
>  	}
> +	if (rt5670->pdata.hs_ground_control_gpio) {
> +		snd_soc_dapm_new_controls(dapm, hs_gnd_cnotrol_widgets,
> +			ARRAY_SIZE(hs_gnd_cnotrol_widgets));
> +		snd_soc_dapm_add_routes(dapm, hs_gnd_cnotrol_routes,
> +			ARRAY_SIZE(hs_gnd_cnotrol_routes));
> +	}
> +
>  	rt5670->codec = codec;
>  
>  	return 0;
> @@ -2827,6 +2874,10 @@ static int rt5670_resume(struct snd_soc_codec *codec)
>  			DMI_MATCH(DMI_BOARD_NAME, "Braswell CRB"),
>  		},
>  	},
> +	{}
> +};
> +
> +static const struct dmi_system_id dmi_platform_dell_wyse[] = {
>  	{
>  		.ident = "Dell Wyse 3040",
>  		.matches = {
> @@ -2861,6 +2912,12 @@ static int rt5670_i2c_probe(struct i2c_client *i2c,
>  		rt5670->pdata.dmic1_data_pin = RT5670_DMIC_DATA_IN2P;
>  		rt5670->pdata.dev_gpio = true;
>  		rt5670->pdata.jd_mode = 1;
> +		rt5670->pdata.hs_ground_control_gpio = RT5670_GPIO_NONE;
> +	} else if (dmi_check_system(dmi_platform_dell_wyse)) {
> +		rt5670->pdata.dmic_en = false;
> +		rt5670->pdata.dev_gpio = true;
> +		rt5670->pdata.jd_mode = 1;
> +		rt5670->pdata.hs_ground_control_gpio = RT5670_GPIO2;
>  	}
>  
>  	rt5670->regmap = devm_regmap_init_i2c(i2c, &rt5670_regmap);
> @@ -2886,6 +2943,21 @@ static int rt5670_i2c_probe(struct i2c_client *i2c,
>  
>  	regmap_write(rt5670->regmap, RT5670_RESET, 0);
>  
> +	switch (rt5670->pdata.hs_ground_control_gpio) {
> +	case RT5670_GPIO_NONE:
> +		break;
> +	case RT5670_GPIO2:
> +		regmap_update_bits(rt5670->regmap, RT5670_GPIO_CTRL2,
> +			RT5670_GP2_PF_MASK | RT5670_GP2_OUT_MASK,
> +			RT5670_GP2_PF_OUT | RT5670_GP2_OUT_LO);
> +		break;
> +	default:
> +		rt5670->pdata.hs_ground_control_gpio =
> +			RT5670_GPIO_NONE;
> +		dev_warn(&i2c->dev, "Only Support GPIO2 currently\n");
> +			break;
> +	}
> +
>  	regmap_read(rt5670->regmap, RT5670_VENDOR_ID, &val);
>  	if (val >= 4)
>  		regmap_write(rt5670->regmap, RT5670_GPIO_CTRL3, 0x0980);
> -- 
> 1.8.1.1.439.g50a6b54
>
Takashi Iwai Jan. 30, 2017, 9:58 a.m. UTC | #2 
On Thu, 29 Dec 2016 09:23:47 +0100,
Bard Liao wrote:
> 
> Some project use external circuit to prevent the headset ground noise.
> It will need to use GPIO to control it. Dell Wyse 3040 is the only project
> that has the external circuit so far and it uses GPIO2 for that. This patch
> provide the support fot it.
> 
> Signed-off-by: Bard Liao <bardliao@realtek.com>

Mark, this seems overlooked?

Feel free to add:
  Tested-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

> ---
>  include/sound/rt5670.h    |  7 +++++
>  sound/soc/codecs/rt5670.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 80 insertions(+), 1 deletion(-)
> 
> diff --git a/include/sound/rt5670.h b/include/sound/rt5670.h
> index b7d6051..0b445b0 100644
> --- a/include/sound/rt5670.h
> +++ b/include/sound/rt5670.h
> @@ -11,12 +11,19 @@
>  #ifndef __LINUX_SND_RT5670_H
>  #define __LINUX_SND_RT5670_H
>  
> +enum rt5670_gpios {
> +	RT5670_GPIO_NONE = 0,
> +	RT5670_GPIO2,
> +};
> +
> +
>  struct rt5670_platform_data {
>  	int jd_mode;
>  	bool in2_diff;
>  	bool dev_gpio;
>  
>  	bool dmic_en;
> +	enum rt5670_gpios hs_ground_control_gpio;
>  	unsigned int dmic1_data_pin;
>  	/* 0 = GPIO6; 1 = IN2P; 3 = GPIO7*/
>  	unsigned int dmic2_data_pin;
> diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c
> index 97bafac..1d94cec 100644
> --- a/sound/soc/codecs/rt5670.c
> +++ b/sound/soc/codecs/rt5670.c
> @@ -420,13 +420,15 @@ static int rt5670_headset_detect(struct snd_soc_codec *codec, int jack_insert)
>  	struct rt5670_priv *rt5670 = snd_soc_codec_get_drvdata(codec);
>  
>  	if (jack_insert) {
> +		if (rt5670->pdata.hs_ground_control_gpio)
> +			snd_soc_dapm_force_enable_pin(dapm, "HS GND Control");
> +
>  		snd_soc_dapm_force_enable_pin(dapm, "Mic Det Power");
>  		snd_soc_dapm_sync(dapm);
>  		snd_soc_update_bits(codec, RT5670_GEN_CTRL3, 0x4, 0x0);
>  		snd_soc_update_bits(codec, RT5670_CJ_CTRL2,
>  			RT5670_CBJ_DET_MODE | RT5670_CBJ_MN_JD,
>  			RT5670_CBJ_MN_JD);
> -		snd_soc_write(codec, RT5670_GPIO_CTRL2, 0x0004);
>  		snd_soc_update_bits(codec, RT5670_GPIO_CTRL1,
>  			RT5670_GP1_PIN_MASK, RT5670_GP1_PIN_IRQ);
>  		snd_soc_update_bits(codec, RT5670_CJ_CTRL1,
> @@ -450,6 +452,10 @@ static int rt5670_headset_detect(struct snd_soc_codec *codec, int jack_insert)
>  			snd_soc_dapm_disable_pin(dapm, "Mic Det Power");
>  			snd_soc_dapm_sync(dapm);
>  		}
> +		if (rt5670->pdata.hs_ground_control_gpio) {
> +			snd_soc_dapm_disable_pin(dapm, "HS GND Control");
> +			snd_soc_dapm_sync(dapm);
> +		}
>  	} else {
>  		snd_soc_update_bits(codec, RT5670_INT_IRQ_ST, 0x8, 0x0);
>  		snd_soc_update_bits(codec, RT5670_GEN_CTRL3, 0x4, 0x4);
> @@ -1542,6 +1548,29 @@ static int rt5670_bst2_event(struct snd_soc_dapm_widget *w,
>  	return 0;
>  }
>  
> +static int rt5670_hs_ground_control_event(struct snd_soc_dapm_widget *w,
> +	struct snd_kcontrol *kcontrol, int event)
> +{
> +	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
> +
> +	switch (event) {
> +	case SND_SOC_DAPM_PRE_PMU:
> +		snd_soc_update_bits(codec, RT5670_GPIO_CTRL2,
> +				RT5670_GP2_OUT_MASK, RT5670_GP2_OUT_HI);
> +		break;
> +
> +	case SND_SOC_DAPM_POST_PMD:
> +		snd_soc_update_bits(codec, RT5670_GPIO_CTRL2,
> +				RT5670_GP2_OUT_MASK, RT5670_GP2_OUT_LO);
> +		break;
> +
> +	default:
> +		return 0;
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct snd_soc_dapm_widget rt5670_dapm_widgets[] = {
>  	SND_SOC_DAPM_SUPPLY("PLL1", RT5670_PWR_ANLG2,
>  			    RT5670_PWR_PLL_BIT, 0, NULL, 0),
> @@ -1917,6 +1946,12 @@ static int rt5670_bst2_event(struct snd_soc_dapm_widget *w,
>  	SND_SOC_DAPM_OUTPUT("SPORN"),
>  };
>  
> +static const struct snd_soc_dapm_widget hs_gnd_cnotrol_widgets[] = {
> +	SND_SOC_DAPM_SUPPLY("HS GND Control", SND_SOC_NOPM, 0, 0,
> +			rt5670_hs_ground_control_event,
> +			SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
> +};
> +
>  static const struct snd_soc_dapm_route rt5670_dapm_routes[] = {
>  	{ "ADC Stereo1 Filter", NULL, "ADC STO1 ASRC", is_using_asrc },
>  	{ "ADC Stereo2 Filter", NULL, "ADC STO2 ASRC", is_using_asrc },
> @@ -2314,6 +2349,11 @@ static int rt5670_bst2_event(struct snd_soc_dapm_widget *w,
>  	{ "SPORN", NULL, "SPO Amp" },
>  };
>  
> +static const struct snd_soc_dapm_route hs_gnd_cnotrol_routes[] = {
> +	{ "BST1", NULL, "HS GND Control" },
> +	{ "HP Amp", NULL, "HS GND Control" },
> +};
> +
>  static int rt5670_hw_params(struct snd_pcm_substream *substream,
>  	struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
>  {
> @@ -2679,6 +2719,13 @@ static int rt5670_probe(struct snd_soc_codec *codec)
>  			"The driver is for RT5670 RT5671 or RT5672 only\n");
>  		return -ENODEV;
>  	}
> +	if (rt5670->pdata.hs_ground_control_gpio) {
> +		snd_soc_dapm_new_controls(dapm, hs_gnd_cnotrol_widgets,
> +			ARRAY_SIZE(hs_gnd_cnotrol_widgets));
> +		snd_soc_dapm_add_routes(dapm, hs_gnd_cnotrol_routes,
> +			ARRAY_SIZE(hs_gnd_cnotrol_routes));
> +	}
> +
>  	rt5670->codec = codec;
>  
>  	return 0;
> @@ -2827,6 +2874,10 @@ static int rt5670_resume(struct snd_soc_codec *codec)
>  			DMI_MATCH(DMI_BOARD_NAME, "Braswell CRB"),
>  		},
>  	},
> +	{}
> +};
> +
> +static const struct dmi_system_id dmi_platform_dell_wyse[] = {
>  	{
>  		.ident = "Dell Wyse 3040",
>  		.matches = {
> @@ -2861,6 +2912,12 @@ static int rt5670_i2c_probe(struct i2c_client *i2c,
>  		rt5670->pdata.dmic1_data_pin = RT5670_DMIC_DATA_IN2P;
>  		rt5670->pdata.dev_gpio = true;
>  		rt5670->pdata.jd_mode = 1;
> +		rt5670->pdata.hs_ground_control_gpio = RT5670_GPIO_NONE;
> +	} else if (dmi_check_system(dmi_platform_dell_wyse)) {
> +		rt5670->pdata.dmic_en = false;
> +		rt5670->pdata.dev_gpio = true;
> +		rt5670->pdata.jd_mode = 1;
> +		rt5670->pdata.hs_ground_control_gpio = RT5670_GPIO2;
>  	}
>  
>  	rt5670->regmap = devm_regmap_init_i2c(i2c, &rt5670_regmap);
> @@ -2886,6 +2943,21 @@ static int rt5670_i2c_probe(struct i2c_client *i2c,
>  
>  	regmap_write(rt5670->regmap, RT5670_RESET, 0);
>  
> +	switch (rt5670->pdata.hs_ground_control_gpio) {
> +	case RT5670_GPIO_NONE:
> +		break;
> +	case RT5670_GPIO2:
> +		regmap_update_bits(rt5670->regmap, RT5670_GPIO_CTRL2,
> +			RT5670_GP2_PF_MASK | RT5670_GP2_OUT_MASK,
> +			RT5670_GP2_PF_OUT | RT5670_GP2_OUT_LO);
> +		break;
> +	default:
> +		rt5670->pdata.hs_ground_control_gpio =
> +			RT5670_GPIO_NONE;
> +		dev_warn(&i2c->dev, "Only Support GPIO2 currently\n");
> +			break;
> +	}
> +
>  	regmap_read(rt5670->regmap, RT5670_VENDOR_ID, &val);
>  	if (val >= 4)
>  		regmap_write(rt5670->regmap, RT5670_GPIO_CTRL3, 0x0980);
> -- 
> 1.8.1.1.439.g50a6b54
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
Mark Brown Feb. 1, 2017, 11:47 a.m. UTC | #3 
On Mon, Jan 30, 2017 at 10:58:31AM +0100, Takashi Iwai wrote:
> On Thu, 29 Dec 2016 09:23:47 +0100,

> > Some project use external circuit to prevent the headset ground noise.
> > It will need to use GPIO to control it. Dell Wyse 3040 is the only project
> > that has the external circuit so far and it uses GPIO2 for that. This patch
> > provide the support fot it.

> Mark, this seems overlooked?

No, it's complicated.  IIRC it's not *actually* doing GPIO management,
it's adding an option to wiggle a pin on the one board you've so far
that happens to have this issue but if someone finds a GPIO from
somewhere else it'll run into trouble.  But now when I go look at it
again you sent a content free ping so...
Takashi Iwai Feb. 1, 2017, 12:52 p.m. UTC | #4 
On Wed, 01 Feb 2017 12:47:13 +0100,
Mark Brown wrote:
> 
> On Mon, Jan 30, 2017 at 10:58:31AM +0100, Takashi Iwai wrote:
> > On Thu, 29 Dec 2016 09:23:47 +0100,
> 
> > > Some project use external circuit to prevent the headset ground noise.
> > > It will need to use GPIO to control it. Dell Wyse 3040 is the only project
> > > that has the external circuit so far and it uses GPIO2 for that. This patch
> > > provide the support fot it.
> 
> > Mark, this seems overlooked?
> 
> No, it's complicated.  IIRC it's not *actually* doing GPIO management,
> it's adding an option to wiggle a pin on the one board you've so far
> that happens to have this issue but if someone finds a GPIO from
> somewhere else it'll run into trouble.

How that can happen?  It applies only via DMI matching.


Takashi
Mark Brown Feb. 1, 2017, 2:59 p.m. UTC | #5 
On Wed, Feb 01, 2017 at 01:52:22PM +0100, Takashi Iwai wrote:
> Mark Brown wrote:

> > No, it's complicated.  IIRC it's not *actually* doing GPIO management,
> > it's adding an option to wiggle a pin on the one board you've so far
> > that happens to have this issue but if someone finds a GPIO from
> > somewhere else it'll run into trouble.

> How that can happen?  It applies only via DMI matching.

Sure, it works on that one machine but if someone else did something
similar but using a different GPIO it (from what I remember) might not
work.  I think I was going to try to figure out if it was really a GPIO
or if perhaps the changelog is just not clear.

But anyway, content free ping so...
Takashi Iwai Feb. 1, 2017, 3:32 p.m. UTC | #6 
On Wed, 01 Feb 2017 15:59:56 +0100,
Mark Brown wrote:
> 
> On Wed, Feb 01, 2017 at 01:52:22PM +0100, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > No, it's complicated.  IIRC it's not *actually* doing GPIO management,
> > > it's adding an option to wiggle a pin on the one board you've so far
> > > that happens to have this issue but if someone finds a GPIO from
> > > somewhere else it'll run into trouble.
> 
> > How that can happen?  It applies only via DMI matching.
> 
> Sure, it works on that one machine but if someone else did something
> similar but using a different GPIO it (from what I remember) might not
> work.

For other GPIO pin, you'll need to write another similar codes, sure.
But why do you need to worry so much about it?  It doesn't work even
without this patch, and a complete generalization isn't so trivial,
I'm afraid, as it's anyway pretty device-specific.

> I think I was going to try to figure out if it was really a GPIO
> or if perhaps the changelog is just not clear.
 
The changelog looks fairly short and maybe not sufficiently explaining
the actual code changes.  Basically the patch gives a new DMI-based
quirk entry to add a control of GPIO2 pin via DAPM per headset
detection.

The single deleted line is the removal of the GPIO_CTRL2 reg write at
headset detection, and according to Bard, this was superfluous and
should have been removed.

> But anyway, content free ping so...

I'm not the author of this patch, either, but just a by-stander
casually involved with the development, so I leave the rest to Bard :)


thanks,

Takashi
Mark Brown Feb. 1, 2017, 5:43 p.m. UTC | #7 
On Wed, Feb 01, 2017 at 04:32:38PM +0100, Takashi Iwai wrote:
> Mark Brown wrote:

> > Sure, it works on that one machine but if someone else did something
> > similar but using a different GPIO it (from what I remember) might not
> > work.

> For other GPIO pin, you'll need to write another similar codes, sure.
> But why do you need to worry so much about it?  It doesn't work even
> without this patch, and a complete generalization isn't so trivial,
> I'm afraid, as it's anyway pretty device-specific.

I don't know, I need to figure this stuff out which is why the patch is
still in the queue.  If it is just using a regular GPIO then gpiolib is
pretty trivial to use.
Mark Brown Feb. 4, 2017, 4:11 p.m. UTC | #8 
On Thu, Feb 02, 2017 at 02:38:55AM +0000, Bard Liao wrote:

> It is a codec GPIO, indeed. But we don't register it to gpiolib.
> So there is no other way to touch the GPIO. We are using it to
> do headset ground control and we should control its level according
> to if headset is using. That's why we control it by a DAPM event.

So one option here would be to register it with gpiolib...
diff mbox

Patch

diff --git a/include/sound/rt5670.h b/include/sound/rt5670.h
index b7d6051..0b445b0 100644
--- a/include/sound/rt5670.h
+++ b/include/sound/rt5670.h
@@ -11,12 +11,19 @@ 
 #ifndef __LINUX_SND_RT5670_H
 #define __LINUX_SND_RT5670_H
 
+enum rt5670_gpios {
+	RT5670_GPIO_NONE = 0,
+	RT5670_GPIO2,
+};
+
+
 struct rt5670_platform_data {
 	int jd_mode;
 	bool in2_diff;
 	bool dev_gpio;
 
 	bool dmic_en;
+	enum rt5670_gpios hs_ground_control_gpio;
 	unsigned int dmic1_data_pin;
 	/* 0 = GPIO6; 1 = IN2P; 3 = GPIO7*/
 	unsigned int dmic2_data_pin;
diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c
index 97bafac..1d94cec 100644
--- a/sound/soc/codecs/rt5670.c
+++ b/sound/soc/codecs/rt5670.c
@@ -420,13 +420,15 @@  static int rt5670_headset_detect(struct snd_soc_codec *codec, int jack_insert)
 	struct rt5670_priv *rt5670 = snd_soc_codec_get_drvdata(codec);
 
 	if (jack_insert) {
+		if (rt5670->pdata.hs_ground_control_gpio)
+			snd_soc_dapm_force_enable_pin(dapm, "HS GND Control");
+
 		snd_soc_dapm_force_enable_pin(dapm, "Mic Det Power");
 		snd_soc_dapm_sync(dapm);
 		snd_soc_update_bits(codec, RT5670_GEN_CTRL3, 0x4, 0x0);
 		snd_soc_update_bits(codec, RT5670_CJ_CTRL2,
 			RT5670_CBJ_DET_MODE | RT5670_CBJ_MN_JD,
 			RT5670_CBJ_MN_JD);
-		snd_soc_write(codec, RT5670_GPIO_CTRL2, 0x0004);
 		snd_soc_update_bits(codec, RT5670_GPIO_CTRL1,
 			RT5670_GP1_PIN_MASK, RT5670_GP1_PIN_IRQ);
 		snd_soc_update_bits(codec, RT5670_CJ_CTRL1,
@@ -450,6 +452,10 @@  static int rt5670_headset_detect(struct snd_soc_codec *codec, int jack_insert)
 			snd_soc_dapm_disable_pin(dapm, "Mic Det Power");
 			snd_soc_dapm_sync(dapm);
 		}
+		if (rt5670->pdata.hs_ground_control_gpio) {
+			snd_soc_dapm_disable_pin(dapm, "HS GND Control");
+			snd_soc_dapm_sync(dapm);
+		}
 	} else {
 		snd_soc_update_bits(codec, RT5670_INT_IRQ_ST, 0x8, 0x0);
 		snd_soc_update_bits(codec, RT5670_GEN_CTRL3, 0x4, 0x4);
@@ -1542,6 +1548,29 @@  static int rt5670_bst2_event(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
+static int rt5670_hs_ground_control_event(struct snd_soc_dapm_widget *w,
+	struct snd_kcontrol *kcontrol, int event)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
+
+	switch (event) {
+	case SND_SOC_DAPM_PRE_PMU:
+		snd_soc_update_bits(codec, RT5670_GPIO_CTRL2,
+				RT5670_GP2_OUT_MASK, RT5670_GP2_OUT_HI);
+		break;
+
+	case SND_SOC_DAPM_POST_PMD:
+		snd_soc_update_bits(codec, RT5670_GPIO_CTRL2,
+				RT5670_GP2_OUT_MASK, RT5670_GP2_OUT_LO);
+		break;
+
+	default:
+		return 0;
+	}
+
+	return 0;
+}
+
 static const struct snd_soc_dapm_widget rt5670_dapm_widgets[] = {
 	SND_SOC_DAPM_SUPPLY("PLL1", RT5670_PWR_ANLG2,
 			    RT5670_PWR_PLL_BIT, 0, NULL, 0),
@@ -1917,6 +1946,12 @@  static int rt5670_bst2_event(struct snd_soc_dapm_widget *w,
 	SND_SOC_DAPM_OUTPUT("SPORN"),
 };
 
+static const struct snd_soc_dapm_widget hs_gnd_cnotrol_widgets[] = {
+	SND_SOC_DAPM_SUPPLY("HS GND Control", SND_SOC_NOPM, 0, 0,
+			rt5670_hs_ground_control_event,
+			SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+};
+
 static const struct snd_soc_dapm_route rt5670_dapm_routes[] = {
 	{ "ADC Stereo1 Filter", NULL, "ADC STO1 ASRC", is_using_asrc },
 	{ "ADC Stereo2 Filter", NULL, "ADC STO2 ASRC", is_using_asrc },
@@ -2314,6 +2349,11 @@  static int rt5670_bst2_event(struct snd_soc_dapm_widget *w,
 	{ "SPORN", NULL, "SPO Amp" },
 };
 
+static const struct snd_soc_dapm_route hs_gnd_cnotrol_routes[] = {
+	{ "BST1", NULL, "HS GND Control" },
+	{ "HP Amp", NULL, "HS GND Control" },
+};
+
 static int rt5670_hw_params(struct snd_pcm_substream *substream,
 	struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
 {
@@ -2679,6 +2719,13 @@  static int rt5670_probe(struct snd_soc_codec *codec)
 			"The driver is for RT5670 RT5671 or RT5672 only\n");
 		return -ENODEV;
 	}
+	if (rt5670->pdata.hs_ground_control_gpio) {
+		snd_soc_dapm_new_controls(dapm, hs_gnd_cnotrol_widgets,
+			ARRAY_SIZE(hs_gnd_cnotrol_widgets));
+		snd_soc_dapm_add_routes(dapm, hs_gnd_cnotrol_routes,
+			ARRAY_SIZE(hs_gnd_cnotrol_routes));
+	}
+
 	rt5670->codec = codec;
 
 	return 0;
@@ -2827,6 +2874,10 @@  static int rt5670_resume(struct snd_soc_codec *codec)
 			DMI_MATCH(DMI_BOARD_NAME, "Braswell CRB"),
 		},
 	},
+	{}
+};
+
+static const struct dmi_system_id dmi_platform_dell_wyse[] = {
 	{
 		.ident = "Dell Wyse 3040",
 		.matches = {
@@ -2861,6 +2912,12 @@  static int rt5670_i2c_probe(struct i2c_client *i2c,
 		rt5670->pdata.dmic1_data_pin = RT5670_DMIC_DATA_IN2P;
 		rt5670->pdata.dev_gpio = true;
 		rt5670->pdata.jd_mode = 1;
+		rt5670->pdata.hs_ground_control_gpio = RT5670_GPIO_NONE;
+	} else if (dmi_check_system(dmi_platform_dell_wyse)) {
+		rt5670->pdata.dmic_en = false;
+		rt5670->pdata.dev_gpio = true;
+		rt5670->pdata.jd_mode = 1;
+		rt5670->pdata.hs_ground_control_gpio = RT5670_GPIO2;
 	}
 
 	rt5670->regmap = devm_regmap_init_i2c(i2c, &rt5670_regmap);
@@ -2886,6 +2943,21 @@  static int rt5670_i2c_probe(struct i2c_client *i2c,
 
 	regmap_write(rt5670->regmap, RT5670_RESET, 0);
 
+	switch (rt5670->pdata.hs_ground_control_gpio) {
+	case RT5670_GPIO_NONE:
+		break;
+	case RT5670_GPIO2:
+		regmap_update_bits(rt5670->regmap, RT5670_GPIO_CTRL2,
+			RT5670_GP2_PF_MASK | RT5670_GP2_OUT_MASK,
+			RT5670_GP2_PF_OUT | RT5670_GP2_OUT_LO);
+		break;
+	default:
+		rt5670->pdata.hs_ground_control_gpio =
+			RT5670_GPIO_NONE;
+		dev_warn(&i2c->dev, "Only Support GPIO2 currently\n");
+			break;
+	}
+
 	regmap_read(rt5670->regmap, RT5670_VENDOR_ID, &val);
 	if (val >= 4)
 		regmap_write(rt5670->regmap, RT5670_GPIO_CTRL3, 0x0980);