All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Fix headphone detection on Acer Chromebook 13
@ 2016-02-04  5:30 Jonathan Tinkham
       [not found] ` <1454563862-1971-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-04  5:30 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	swarren-3lzwWm7+Weoh9ZMKESR00Q, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, Jonathan Tinkham

Headphone output works, but headphone detection does not. The problem
is improper inversion of the jack gpio, and improper naming of the
DAPM switch. With these changes, headphone insertion and removal is
picked up by userspace, and the switch properly toggled.

Jonathan Tinkham (4):
  soc/sound: tegra_max98090: do not invert headphone jack
  sound/soc: tegra_max98090: rename headphone jack DAPM
  sound/soc: tegra_max98090: document headphone jack rename
  soc/sound: tegra_max98090: update dts affected by rename

 .../devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt  |  6 +++---
 arch/arm/boot/dts/tegra124-nyan.dtsi                           |  4 ++--
 arch/arm/boot/dts/tegra124-venice2.dts                         |  4 ++--
 sound/soc/tegra/tegra_max98090.c                               | 10 +++++-----
 4 files changed, 12 insertions(+), 12 deletions(-)

-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack
       [not found] ` <1454563862-1971-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-02-04  5:30   ` Jonathan Tinkham
       [not found]     ` <1454563862-1971-2-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-02-04  5:31   ` [PATCH 2/4] sound/soc: tegra_max98090: rename headphone jack DAPM Jonathan Tinkham
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-04  5:30 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	swarren-3lzwWm7+Weoh9ZMKESR00Q, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, Jonathan Tinkham

The headphone jack should not be inverted

Signed-off-by: Jonathan Tinkham <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 sound/soc/tegra/tegra_max98090.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/tegra/tegra_max98090.c b/sound/soc/tegra/tegra_max98090.c
index 902da36..b373d06 100644
--- a/sound/soc/tegra/tegra_max98090.c
+++ b/sound/soc/tegra/tegra_max98090.c
@@ -110,7 +110,7 @@ static struct snd_soc_jack_gpio tegra_max98090_hp_jack_gpio = {
 	.name = "Headphone detection",
 	.report = SND_JACK_HEADPHONE,
 	.debounce_time = 150,
-	.invert = 1,
+	.invert = 0,
 };
 
 static struct snd_soc_jack tegra_max98090_mic_jack;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 2/4] sound/soc: tegra_max98090: rename headphone jack DAPM
       [not found] ` <1454563862-1971-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-02-04  5:30   ` [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack Jonathan Tinkham
@ 2016-02-04  5:31   ` Jonathan Tinkham
       [not found]     ` <1454563862-1971-3-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-02-04  5:31   ` [PATCH 3/4] sound/soc: tegra_max98090: document headphone jack rename Jonathan Tinkham
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-04  5:31 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	swarren-3lzwWm7+Weoh9ZMKESR00Q, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, Jonathan Tinkham

The correct DAPM name for a headphone jack is "Headphone Jack"
This is also what userspace expects, and this fixes headphone detection.

Signed-off-by: Jonathan Tinkham <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 sound/soc/tegra/tegra_max98090.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/tegra/tegra_max98090.c b/sound/soc/tegra/tegra_max98090.c
index b373d06..a015d1b 100644
--- a/sound/soc/tegra/tegra_max98090.c
+++ b/sound/soc/tegra/tegra_max98090.c
@@ -101,7 +101,7 @@ static struct snd_soc_jack tegra_max98090_hp_jack;
 
 static struct snd_soc_jack_pin tegra_max98090_hp_jack_pins[] = {
 	{
-		.pin = "Headphones",
+		.pin = "Headphone Jack",
 		.mask = SND_JACK_HEADPHONE,
 	},
 };
@@ -130,14 +130,14 @@ static struct snd_soc_jack_gpio tegra_max98090_mic_jack_gpio = {
 };
 
 static const struct snd_soc_dapm_widget tegra_max98090_dapm_widgets[] = {
-	SND_SOC_DAPM_HP("Headphones", NULL),
+	SND_SOC_DAPM_HP("Headphone Jack", NULL),
 	SND_SOC_DAPM_SPK("Speakers", NULL),
 	SND_SOC_DAPM_MIC("Mic Jack", NULL),
 	SND_SOC_DAPM_MIC("Int Mic", NULL),
 };
 
 static const struct snd_kcontrol_new tegra_max98090_controls[] = {
-	SOC_DAPM_PIN_SWITCH("Headphones"),
+	SOC_DAPM_PIN_SWITCH("Headphone Jack"),
 	SOC_DAPM_PIN_SWITCH("Speakers"),
 	SOC_DAPM_PIN_SWITCH("Mic Jack"),
 	SOC_DAPM_PIN_SWITCH("Int Mic"),
@@ -148,7 +148,7 @@ static int tegra_max98090_asoc_init(struct snd_soc_pcm_runtime *rtd)
 	struct tegra_max98090 *machine = snd_soc_card_get_drvdata(rtd->card);
 
 	if (gpio_is_valid(machine->gpio_hp_det)) {
-		snd_soc_card_jack_new(rtd->card, "Headphones",
+		snd_soc_card_jack_new(rtd->card, "Headphone Jack",
 				      SND_JACK_HEADPHONE,
 				      &tegra_max98090_hp_jack,
 				      tegra_max98090_hp_jack_pins,
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 3/4] sound/soc: tegra_max98090: document headphone jack rename
       [not found] ` <1454563862-1971-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-02-04  5:30   ` [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack Jonathan Tinkham
  2016-02-04  5:31   ` [PATCH 2/4] sound/soc: tegra_max98090: rename headphone jack DAPM Jonathan Tinkham
@ 2016-02-04  5:31   ` Jonathan Tinkham
       [not found]     ` <1454563862-1971-4-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-02-04  5:31   ` [PATCH 4/4] soc/sound: tegra_max98090: update dts affected by rename Jonathan Tinkham
  2016-02-04 18:03   ` [PATCH 0/4] Fix headphone detection on Acer Chromebook 13 Mark Brown
  4 siblings, 1 reply; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-04  5:31 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	swarren-3lzwWm7+Weoh9ZMKESR00Q, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, Jonathan Tinkham

Update device-tree bindings to reflect the rename of the board's
headphone jack.

Signed-off-by: Jonathan Tinkham <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt       | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt b/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt
index c3495be..7ad911f 100644
--- a/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt
+++ b/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt
@@ -15,7 +15,7 @@ Required properties:
   sinks are the MAX98090's pins (as documented in its binding), and the jacks
   on the board:
 
-  * Headphones
+  * Headphone Jack
   * Speakers
   * Mic Jack
   * Int Mic
@@ -36,8 +36,8 @@ sound {
 	nvidia,model = "NVIDIA Tegra Venice2";
 
 	nvidia,audio-routing =
-		"Headphones", "HPR",
-		"Headphones", "HPL",
+		"Headphone Jack", "HPR",
+		"Headphone Jack", "HPL",
 		"Speakers", "SPKR",
 		"Speakers", "SPKL",
 		"Mic Jack", "MICBIAS",
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 4/4] soc/sound: tegra_max98090: update dts affected by rename
       [not found] ` <1454563862-1971-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (2 preceding siblings ...)
  2016-02-04  5:31   ` [PATCH 3/4] sound/soc: tegra_max98090: document headphone jack rename Jonathan Tinkham
@ 2016-02-04  5:31   ` Jonathan Tinkham
  2016-02-04 18:03   ` [PATCH 0/4] Fix headphone detection on Acer Chromebook 13 Mark Brown
  4 siblings, 0 replies; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-04  5:31 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	swarren-3lzwWm7+Weoh9ZMKESR00Q, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, Jonathan Tinkham

Update the DTs that are affected by the DAPM headphone jack rename

Signed-off-by: Jonathan Tinkham <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/tegra124-nyan.dtsi   | 4 ++--
 arch/arm/boot/dts/tegra124-venice2.dts | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi b/arch/arm/boot/dts/tegra124-nyan.dtsi
index ec1aa64..000b1f6 100644
--- a/arch/arm/boot/dts/tegra124-nyan.dtsi
+++ b/arch/arm/boot/dts/tegra124-nyan.dtsi
@@ -676,8 +676,8 @@
 
 	sound {
 		nvidia,audio-routing =
-			"Headphones", "HPR",
-			"Headphones", "HPL",
+			"Headphone Jack", "HPR",
+			"Headphone Jack", "HPL",
 			"Speakers", "SPKR",
 			"Speakers", "SPKL",
 			"Mic Jack", "MICBIAS",
diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
index cfbdf42..c535d55 100644
--- a/arch/arm/boot/dts/tegra124-venice2.dts
+++ b/arch/arm/boot/dts/tegra124-venice2.dts
@@ -1147,8 +1147,8 @@
 		nvidia,model = "NVIDIA Tegra Venice2";
 
 		nvidia,audio-routing =
-			"Headphones", "HPR",
-			"Headphones", "HPL",
+			"Headphone Jack", "HPR",
+			"Headphone Jack", "HPL",
 			"Speakers", "SPKR",
 			"Speakers", "SPKL",
 			"Mic Jack", "MICBIAS",
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack
       [not found]     ` <1454563862-1971-2-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-02-04 16:36       ` Stephen Warren
       [not found]         ` <56B37E19.6010002-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 38+ messages in thread
From: Stephen Warren @ 2016-02-04 16:36 UTC (permalink / raw)
  To: Jonathan Tinkham
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

On 02/03/2016 10:30 PM, Jonathan Tinkham wrote:
> The headphone jack should not be inverted

Have you tested this on Venice2 as well as Nyan? I'm pretty sure Venice2 
was tested when this driver was written, and whoever added Nyan support 
to the kernel simply assumed it would work. As such, my suspicion is 
that this series will break Venice2 even as it fixes Nyan.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 2/4] sound/soc: tegra_max98090: rename headphone jack DAPM
       [not found]     ` <1454563862-1971-3-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-02-04 16:38       ` Stephen Warren
  0 siblings, 0 replies; 38+ messages in thread
From: Stephen Warren @ 2016-02-04 16:38 UTC (permalink / raw)
  To: Jonathan Tinkham
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

On 02/03/2016 10:31 PM, Jonathan Tinkham wrote:
> The correct DAPM name for a headphone jack is "Headphone Jack"
> This is also what userspace expects, and this fixes headphone detection.

Why doesn't user-space expect what the kernel actually implements? The 
kernel should be defining the control naming.

Which user-space are you using specifically, and which part of it 
expects particular naming?

Perhaps this series needs to be parametrized based on a flag in DT, 
rather than switching the hard-coded values, so that only Venice2 can be 
affected?

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 3/4] sound/soc: tegra_max98090: document headphone jack rename
       [not found]     ` <1454563862-1971-4-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-02-04 16:39       ` Stephen Warren
       [not found]         ` <56B37EAC.5010101-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 38+ messages in thread
From: Stephen Warren @ 2016-02-04 16:39 UTC (permalink / raw)
  To: Jonathan Tinkham
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

On 02/03/2016 10:31 PM, Jonathan Tinkham wrote:
> Update device-tree bindings to reflect the rename of the board's
> headphone jack.

This looks like an incompatible change to the DT. While you've fixed the 
DT, which will fix new installations, old DTs now won't work. This 
breaks DT ABI. Any DT change needs to be backwards compatible, i.e. the 
old name should still work and be documented as a legacy value.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 0/4] Fix headphone detection on Acer Chromebook 13
       [not found] ` <1454563862-1971-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (3 preceding siblings ...)
  2016-02-04  5:31   ` [PATCH 4/4] soc/sound: tegra_max98090: update dts affected by rename Jonathan Tinkham
@ 2016-02-04 18:03   ` Mark Brown
  4 siblings, 0 replies; 38+ messages in thread
From: Mark Brown @ 2016-02-04 18:03 UTC (permalink / raw)
  To: Jonathan Tinkham
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, swarren-3lzwWm7+Weoh9ZMKESR00Q,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

[-- Attachment #1: Type: text/plain, Size: 511 bytes --]

On Wed, Feb 03, 2016 at 10:30:58PM -0700, Jonathan Tinkham wrote:

> Jonathan Tinkham (4):
>   soc/sound: tegra_max98090: do not invert headphone jack
>   sound/soc: tegra_max98090: rename headphone jack DAPM
>   sound/soc: tegra_max98090: document headphone jack rename
>   soc/sound: tegra_max98090: update dts affected by rename

Please try to use subject lines reflecting the style for the subsystem -
this makes it easier for people to spot relevant content.  The above
isn't even internally consistent :(

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack
       [not found]         ` <56B37E19.6010002-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2016-02-04 19:05           ` Jonathan Tinkham
       [not found]             ` <56B3A10E.4010508-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-04 19:05 UTC (permalink / raw)
  To: Stephen Warren
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

On 02/04/2016 09:36 AM, Stephen Warren wrote:
> On 02/03/2016 10:30 PM, Jonathan Tinkham wrote:
>> The headphone jack should not be inverted
>
> Have you tested this on Venice2 as well as Nyan? I'm pretty sure 
> Venice2 was tested when this driver was written, and whoever added 
> Nyan support to the kernel simply assumed it would work. As such, my 
> suspicion is that this series will break Venice2 even as it fixes Nyan.

I have not tested this on Venice2, only on Nyan. That seems like a 
plausible cause and reasonable suspicion.

> Why doesn't user-space expect what the kernel actually implements? The 
> kernel should be defining the control naming.
>
> Which user-space are you using specifically, and which part of it 
> expects particular naming?
>
> Perhaps this series needs to be parametrized based on a flag in DT, 
> rather than switching the hard-coded values, so that only Venice2 can 
> be affected?

Specifically pulse-audio and alsa under Arch Linux.

I was referencing 'Documentation/sound/alsa/soc/dapm.txt' with regards 
to control names. While it is possible to add another entry into the 
user-space configuration, I took this documentation as a definition of 
kernel control naming schemes, and thought the driver had used a 
non-standard naming scheme (or at least, not a consistent one).

> On 02/03/2016 10:31 PM, Jonathan Tinkham wrote:
>> Update device-tree bindings to reflect the rename of the board's
>> headphone jack.
>
> This looks like an incompatible change to the DT. While you've fixed 
> the DT, which will fix new installations, old DTs now won't work. This 
> breaks DT ABI. Any DT change needs to be backwards compatible, i.e. 
> the old name should still work and be documented as a legacy value. 

I see that now. If the inversion behavior differs between venice2 and 
nyan, then another compatible string would need to be added anyways, 
correct? As you mentioned above, this might need to be done anyways for 
the rename.

Something like:
- "compatible = nvidia,tegra-audio-max98090" implements old inversion 
behavior and leaves the switch as "Headphones" to avoid breaking older DTs
- "compatible = nvidia,tegra-{venice2,nyan}-audio-max98090" that 
separates the inversion behavior and also introduces the rename

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack
       [not found]             ` <56B3A10E.4010508-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-02-04 19:17               ` Dylan Reid
       [not found]                 ` <CAEUnVG7bCgzC-f_Anaw6qayGsXi=_NrWZGRy7SSALkLVLPZnUQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2016-02-04 19:37               ` Mark Brown
  2016-02-08 18:59               ` Stephen Warren
  2 siblings, 1 reply; 38+ messages in thread
From: Dylan Reid @ 2016-02-04 19:17 UTC (permalink / raw)
  To: Jonathan Tinkham
  Cc: Stephen Warren, devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Mark Brown,
	Alexandre Courbot, Thierry Reding

On Thu, Feb 4, 2016 at 11:05 AM, Jonathan Tinkham <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 02/04/2016 09:36 AM, Stephen Warren wrote:
>>
>> On 02/03/2016 10:30 PM, Jonathan Tinkham wrote:
>>>
>>> The headphone jack should not be inverted
>>
>>
>> Have you tested this on Venice2 as well as Nyan? I'm pretty sure Venice2
>> was tested when this driver was written, and whoever added Nyan support to
>> the kernel simply assumed it would work. As such, my suspicion is that this
>> series will break Venice2 even as it fixes Nyan.
>
>
> I have not tested this on Venice2, only on Nyan. That seems like a plausible
> cause and reasonable suspicion.
>
>> Why doesn't user-space expect what the kernel actually implements? The
>> kernel should be defining the control naming.
>>
>> Which user-space are you using specifically, and which part of it expects
>> particular naming?
>>
>> Perhaps this series needs to be parametrized based on a flag in DT, rather
>> than switching the hard-coded values, so that only Venice2 can be affected?
>
>
> Specifically pulse-audio and alsa under Arch Linux.
>
> I was referencing 'Documentation/sound/alsa/soc/dapm.txt' with regards to
> control names. While it is possible to add another entry into the user-space
> configuration, I took this documentation as a definition of kernel control
> naming schemes, and thought the driver had used a non-standard naming scheme
> (or at least, not a consistent one).
>
>> On 02/03/2016 10:31 PM, Jonathan Tinkham wrote:
>>>
>>> Update device-tree bindings to reflect the rename of the board's
>>> headphone jack.
>>
>>
>> This looks like an incompatible change to the DT. While you've fixed the
>> DT, which will fix new installations, old DTs now won't work. This breaks DT
>> ABI. Any DT change needs to be backwards compatible, i.e. the old name
>> should still work and be documented as a legacy value.
>
>
> I see that now. If the inversion behavior differs between venice2 and nyan,
> then another compatible string would need to be added anyways, correct? As
> you mentioned above, this might need to be done anyways for the rename.

Venice2 and both nyan platforms do have different polarity of HP detect.

For some boards we have an hd-invert property in DT.

Would setting hp-det-gpio to active low in the pinmux achieve the same thing?

>
> Something like:
> - "compatible = nvidia,tegra-audio-max98090" implements old inversion
> behavior and leaves the switch as "Headphones" to avoid breaking older DTs
> - "compatible = nvidia,tegra-{venice2,nyan}-audio-max98090" that separates
> the inversion behavior and also introduces the rename
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 3/4] sound/soc: tegra_max98090: document headphone jack rename
       [not found]         ` <56B37EAC.5010101-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2016-02-04 19:22           ` Dylan Reid
  0 siblings, 0 replies; 38+ messages in thread
From: Dylan Reid @ 2016-02-04 19:22 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Jonathan Tinkham, devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Mark Brown,
	Alexandre Courbot, Thierry Reding

On Thu, Feb 4, 2016 at 8:39 AM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> On 02/03/2016 10:31 PM, Jonathan Tinkham wrote:
>>
>> Update device-tree bindings to reflect the rename of the board's
>> headphone jack.
>
>
> This looks like an incompatible change to the DT. While you've fixed the DT,
> which will fix new installations, old DTs now won't work. This breaks DT
> ABI. Any DT change needs to be backwards compatible, i.e. the old name
> should still work and be documented as a legacy value.
>

These values are also present in the UCM config for the boards present
in alsa-lib/conf/ucm/GoogleNyan

It's better not to rely on drivers using a "standard" name, better to
let the UCM file abstract it away.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack
       [not found]             ` <56B3A10E.4010508-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-02-04 19:17               ` Dylan Reid
@ 2016-02-04 19:37               ` Mark Brown
  2016-02-08 18:59               ` Stephen Warren
  2 siblings, 0 replies; 38+ messages in thread
From: Mark Brown @ 2016-02-04 19:37 UTC (permalink / raw)
  To: Jonathan Tinkham
  Cc: Stephen Warren, devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

On Thu, Feb 04, 2016 at 12:05:50PM -0700, Jonathan Tinkham wrote:

> I was referencing 'Documentation/sound/alsa/soc/dapm.txt' with regards to
> control names. While it is possible to add another entry into the user-space
> configuration, I took this documentation as a definition of kernel control
> naming schemes, and thought the driver had used a non-standard naming scheme
> (or at least, not a consistent one).

That's *not* ABI documentation, that's a random example of some kernel
internal strings.

> I see that now. If the inversion behavior differs between venice2 and nyan,
> then another compatible string would need to be added anyways, correct? As
> you mentioned above, this might need to be done anyways for the rename.

A property would be nicer.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack
       [not found]                 ` <CAEUnVG7bCgzC-f_Anaw6qayGsXi=_NrWZGRy7SSALkLVLPZnUQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-02-08 18:56                   ` Stephen Warren
       [not found]                     ` <56B8E4D8.8070604-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 38+ messages in thread
From: Stephen Warren @ 2016-02-08 18:56 UTC (permalink / raw)
  To: Dylan Reid, Jonathan Tinkham
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Mark Brown,
	Alexandre Courbot, Thierry Reding

On 02/04/2016 12:17 PM, Dylan Reid wrote:
> On Thu, Feb 4, 2016 at 11:05 AM, Jonathan Tinkham <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 02/04/2016 09:36 AM, Stephen Warren wrote:
>>>
>>> On 02/03/2016 10:30 PM, Jonathan Tinkham wrote:
>>>>
>>>> The headphone jack should not be inverted
>>>
>>>
>>> Have you tested this on Venice2 as well as Nyan? I'm pretty sure Venice2
>>> was tested when this driver was written, and whoever added Nyan support to
>>> the kernel simply assumed it would work. As such, my suspicion is that this
>>> series will break Venice2 even as it fixes Nyan.
>>
>>
>> I have not tested this on Venice2, only on Nyan. That seems like a plausible
>> cause and reasonable suspicion.
>>
>>> Why doesn't user-space expect what the kernel actually implements? The
>>> kernel should be defining the control naming.
>>>
>>> Which user-space are you using specifically, and which part of it expects
>>> particular naming?
>>>
>>> Perhaps this series needs to be parametrized based on a flag in DT, rather
>>> than switching the hard-coded values, so that only Venice2 can be affected?
>>
>>
>> Specifically pulse-audio and alsa under Arch Linux.
>>
>> I was referencing 'Documentation/sound/alsa/soc/dapm.txt' with regards to
>> control names. While it is possible to add another entry into the user-space
>> configuration, I took this documentation as a definition of kernel control
>> naming schemes, and thought the driver had used a non-standard naming scheme
>> (or at least, not a consistent one).
>>
>>> On 02/03/2016 10:31 PM, Jonathan Tinkham wrote:
>>>>
>>>> Update device-tree bindings to reflect the rename of the board's
>>>> headphone jack.
>>>
>>>
>>> This looks like an incompatible change to the DT. While you've fixed the
>>> DT, which will fix new installations, old DTs now won't work. This breaks DT
>>> ABI. Any DT change needs to be backwards compatible, i.e. the old name
>>> should still work and be documented as a legacy value.
>>
>>
>> I see that now. If the inversion behavior differs between venice2 and nyan,
>> then another compatible string would need to be added anyways, correct? As
>> you mentioned above, this might need to be done anyways for the rename.
>
> Venice2 and both nyan platforms do have different polarity of HP detect.
>
> For some boards we have an hd-invert property in DT.
>
> Would setting hp-det-gpio to active low in the pinmux achieve the same thing?

I don't believe we have a pinmux setting for this.

However, there's a GPIO_ACTIVE_HIGH/LOW flag that could be used in DT 
property nvidia,hp-det-gpios's flags field to indicate the polarity. 
That should work, but indeed you could use a separate hp-dt-invert 
property if not.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack
       [not found]             ` <56B3A10E.4010508-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-02-04 19:17               ` Dylan Reid
  2016-02-04 19:37               ` Mark Brown
@ 2016-02-08 18:59               ` Stephen Warren
       [not found]                 ` <56B8E5AD.8040707-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  2 siblings, 1 reply; 38+ messages in thread
From: Stephen Warren @ 2016-02-08 18:59 UTC (permalink / raw)
  To: Jonathan Tinkham
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

On 02/04/2016 12:05 PM, Jonathan Tinkham wrote:
> On 02/04/2016 09:36 AM, Stephen Warren wrote:
>> On 02/03/2016 10:30 PM, Jonathan Tinkham wrote:
>>> The headphone jack should not be inverted
>>
>> Have you tested this on Venice2 as well as Nyan? I'm pretty sure
>> Venice2 was tested when this driver was written, and whoever added
>> Nyan support to the kernel simply assumed it would work. As such, my
>> suspicion is that this series will break Venice2 even as it fixes Nyan.
>
> I have not tested this on Venice2, only on Nyan. That seems like a
> plausible cause and reasonable suspicion.
>
>> Why doesn't user-space expect what the kernel actually implements? The
>> kernel should be defining the control naming.
>>
>> Which user-space are you using specifically, and which part of it
>> expects particular naming?
>>
>> Perhaps this series needs to be parametrized based on a flag in DT,
>> rather than switching the hard-coded values, so that only Venice2 can
>> be affected?
>
> Specifically pulse-audio and alsa under Arch Linux.
>
> I was referencing 'Documentation/sound/alsa/soc/dapm.txt' with regards
> to control names. While it is possible to add another entry into the
> user-space configuration, I took this documentation as a definition of
> kernel control naming schemes, and thought the driver had used a
> non-standard naming scheme (or at least, not a consistent one).
>
>> On 02/03/2016 10:31 PM, Jonathan Tinkham wrote:
>>> Update device-tree bindings to reflect the rename of the board's
>>> headphone jack.
>>
>> This looks like an incompatible change to the DT. While you've fixed
>> the DT, which will fix new installations, old DTs now won't work. This
>> breaks DT ABI. Any DT change needs to be backwards compatible, i.e.
>> the old name should still work and be documented as a legacy value.
>
> I see that now. If the inversion behavior differs between venice2 and
> nyan, then another compatible string would need to be added anyways,
> correct? As you mentioned above, this might need to be done anyways for
> the rename.
>
> Something like:
> - "compatible = nvidia,tegra-audio-max98090" implements old inversion
> behavior and leaves the switch as "Headphones" to avoid breaking older DTs
> - "compatible = nvidia,tegra-{venice2,nyan}-audio-max98090" that
> separates the inversion behavior and also introduces the rename

It'd be preferable to key this off an separate flag rather than the 
compatible value. That would more directly represent the data in 
question, and allow any future boards to be added without having to edit 
the driver to know whether those new boards neded HP DET inversion or not.

However, do note that each of the 3 boards using this binding has a 
board-specific compatible value present already:

nvidia,tegra-audio-max98090-venice2
nvidia,tegra-audio-max98090-nyan-blaze
nvidia,tegra-audio-max98090-nyan-big
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack
       [not found]                 ` <56B8E5AD.8040707-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2016-02-13  2:10                   ` Jonathan Tinkham
       [not found]                     ` <56BE9094.1040500-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-13  2:10 UTC (permalink / raw)
  To: Stephen Warren
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

On 02/08/2016 11:59 AM, Stephen Warren wrote:
> On 02/04/2016 12:05 PM, Jonathan Tinkham wrote:
>> On 02/04/2016 09:36 AM, Stephen Warren wrote:
>>> On 02/03/2016 10:30 PM, Jonathan Tinkham wrote:
>>>> The headphone jack should not be inverted
>>>
>>> Have you tested this on Venice2 as well as Nyan? I'm pretty sure
>>> Venice2 was tested when this driver was written, and whoever added
>>> Nyan support to the kernel simply assumed it would work. As such, my
>>> suspicion is that this series will break Venice2 even as it fixes Nyan.
>>
>> I have not tested this on Venice2, only on Nyan. That seems like a
>> plausible cause and reasonable suspicion.
>>
>>> Why doesn't user-space expect what the kernel actually implements? The
>>> kernel should be defining the control naming.
>>>
>>> Which user-space are you using specifically, and which part of it
>>> expects particular naming?
>>>
>>> Perhaps this series needs to be parametrized based on a flag in DT,
>>> rather than switching the hard-coded values, so that only Venice2 can
>>> be affected?
>>
>> Specifically pulse-audio and alsa under Arch Linux.
>>
>> I was referencing 'Documentation/sound/alsa/soc/dapm.txt' with regards
>> to control names. While it is possible to add another entry into the
>> user-space configuration, I took this documentation as a definition of
>> kernel control naming schemes, and thought the driver had used a
>> non-standard naming scheme (or at least, not a consistent one).
>>
>>> On 02/03/2016 10:31 PM, Jonathan Tinkham wrote:
>>>> Update device-tree bindings to reflect the rename of the board's
>>>> headphone jack.
>>>
>>> This looks like an incompatible change to the DT. While you've fixed
>>> the DT, which will fix new installations, old DTs now won't work. This
>>> breaks DT ABI. Any DT change needs to be backwards compatible, i.e.
>>> the old name should still work and be documented as a legacy value.
>>
>> I see that now. If the inversion behavior differs between venice2 and
>> nyan, then another compatible string would need to be added anyways,
>> correct? As you mentioned above, this might need to be done anyways for
>> the rename.
>>
>> Something like:
>> - "compatible = nvidia,tegra-audio-max98090" implements old inversion
>> behavior and leaves the switch as "Headphones" to avoid breaking 
>> older DTs
>> - "compatible = nvidia,tegra-{venice2,nyan}-audio-max98090" that
>> separates the inversion behavior and also introduces the rename
>
> It'd be preferable to key this off an separate flag rather than the 
> compatible value. That would more directly represent the data in 
> question, and allow any future boards to be added without having to 
> edit the driver to know whether those new boards neded HP DET 
> inversion or not.
>
> However, do note that each of the 3 boards using this binding has a 
> board-specific compatible value present already:
>
> nvidia,tegra-audio-max98090-venice2
> nvidia,tegra-audio-max98090-nyan-blaze
> nvidia,tegra-audio-max98090-nyan-big

Indeed, I noticed those compatible strings after I sent that message.

A property seems the more ideal/desired way to go. However, to avoid 
breaking older DTs, does that mean it must be implemented as assuming 
inversion is default, and set nyan and other boards to "hd-invert = 0"?

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack
       [not found]                     ` <56B8E4D8.8070604-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2016-02-13  2:12                       ` Jonathan Tinkham
       [not found]                         ` <56BE9125.3060104-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-13  2:12 UTC (permalink / raw)
  To: Stephen Warren, Dylan Reid
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Mark Brown,
	Alexandre Courbot, Thierry Reding

On 02/08/2016 11:56 AM, Stephen Warren wrote:
> On 02/04/2016 12:17 PM, Dylan Reid wrote:
>> On Thu, Feb 4, 2016 at 11:05 AM, Jonathan Tinkham 
>> <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On 02/04/2016 09:36 AM, Stephen Warren wrote:
>>>>
>>>> On 02/03/2016 10:30 PM, Jonathan Tinkham wrote:
>>>>>
>>>>> The headphone jack should not be inverted
>>>>
>>>>
>>>> Have you tested this on Venice2 as well as Nyan? I'm pretty sure 
>>>> Venice2
>>>> was tested when this driver was written, and whoever added Nyan 
>>>> support to
>>>> the kernel simply assumed it would work. As such, my suspicion is 
>>>> that this
>>>> series will break Venice2 even as it fixes Nyan.
>>>
>>>
>>> I have not tested this on Venice2, only on Nyan. That seems like a 
>>> plausible
>>> cause and reasonable suspicion.
>>>
>>>> Why doesn't user-space expect what the kernel actually implements? The
>>>> kernel should be defining the control naming.
>>>>
>>>> Which user-space are you using specifically, and which part of it 
>>>> expects
>>>> particular naming?
>>>>
>>>> Perhaps this series needs to be parametrized based on a flag in DT, 
>>>> rather
>>>> than switching the hard-coded values, so that only Venice2 can be 
>>>> affected?
>>>
>>>
>>> Specifically pulse-audio and alsa under Arch Linux.
>>>
>>> I was referencing 'Documentation/sound/alsa/soc/dapm.txt' with 
>>> regards to
>>> control names. While it is possible to add another entry into the 
>>> user-space
>>> configuration, I took this documentation as a definition of kernel 
>>> control
>>> naming schemes, and thought the driver had used a non-standard 
>>> naming scheme
>>> (or at least, not a consistent one).
>>>
>>>> On 02/03/2016 10:31 PM, Jonathan Tinkham wrote:
>>>>>
>>>>> Update device-tree bindings to reflect the rename of the board's
>>>>> headphone jack.
>>>>
>>>>
>>>> This looks like an incompatible change to the DT. While you've 
>>>> fixed the
>>>> DT, which will fix new installations, old DTs now won't work. This 
>>>> breaks DT
>>>> ABI. Any DT change needs to be backwards compatible, i.e. the old name
>>>> should still work and be documented as a legacy value.
>>>
>>>
>>> I see that now. If the inversion behavior differs between venice2 
>>> and nyan,
>>> then another compatible string would need to be added anyways, 
>>> correct? As
>>> you mentioned above, this might need to be done anyways for the rename.
>>
>> Venice2 and both nyan platforms do have different polarity of HP detect.
>>
>> For some boards we have an hd-invert property in DT.
>>
>> Would setting hp-det-gpio to active low in the pinmux achieve the 
>> same thing?
>
> I don't believe we have a pinmux setting for this.
>
> However, there's a GPIO_ACTIVE_HIGH/LOW flag that could be used in DT 
> property nvidia,hp-det-gpios's flags field to indicate the polarity. 
> That should work, but indeed you could use a separate hp-dt-invert 
> property if not.

Yeah, I could find other things that have an "invert" property, but none 
for headphone/mic detect pins.

I'm not sure setting the GPIO_ACTIVE_HIGH/LOW flag would work. Doesn't 
it depend on how the pin is physically wired?

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack
       [not found]                     ` <56BE9094.1040500-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-02-16 21:41                       ` Stephen Warren
       [not found]                         ` <56C3977E.8050001-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 38+ messages in thread
From: Stephen Warren @ 2016-02-16 21:41 UTC (permalink / raw)
  To: Jonathan Tinkham
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

On 02/12/2016 07:10 PM, Jonathan Tinkham wrote:
> On 02/08/2016 11:59 AM, Stephen Warren wrote:
>> On 02/04/2016 12:05 PM, Jonathan Tinkham wrote:
>>> On 02/04/2016 09:36 AM, Stephen Warren wrote:
>>>> On 02/03/2016 10:30 PM, Jonathan Tinkham wrote:
>>>>> The headphone jack should not be inverted
>>>>
>>>> Have you tested this on Venice2 as well as Nyan? I'm pretty sure
>>>> Venice2 was tested when this driver was written, and whoever added
>>>> Nyan support to the kernel simply assumed it would work. As such, my
>>>> suspicion is that this series will break Venice2 even as it fixes Nyan.
>>>
>>> I have not tested this on Venice2, only on Nyan. That seems like a
>>> plausible cause and reasonable suspicion.
>>>
>>>> Why doesn't user-space expect what the kernel actually implements? The
>>>> kernel should be defining the control naming.
>>>>
>>>> Which user-space are you using specifically, and which part of it
>>>> expects particular naming?
>>>>
>>>> Perhaps this series needs to be parametrized based on a flag in DT,
>>>> rather than switching the hard-coded values, so that only Venice2 can
>>>> be affected?
>>>
>>> Specifically pulse-audio and alsa under Arch Linux.
>>>
>>> I was referencing 'Documentation/sound/alsa/soc/dapm.txt' with regards
>>> to control names. While it is possible to add another entry into the
>>> user-space configuration, I took this documentation as a definition of
>>> kernel control naming schemes, and thought the driver had used a
>>> non-standard naming scheme (or at least, not a consistent one).
>>>
>>>> On 02/03/2016 10:31 PM, Jonathan Tinkham wrote:
>>>>> Update device-tree bindings to reflect the rename of the board's
>>>>> headphone jack.
>>>>
>>>> This looks like an incompatible change to the DT. While you've fixed
>>>> the DT, which will fix new installations, old DTs now won't work. This
>>>> breaks DT ABI. Any DT change needs to be backwards compatible, i.e.
>>>> the old name should still work and be documented as a legacy value.
>>>
>>> I see that now. If the inversion behavior differs between venice2 and
>>> nyan, then another compatible string would need to be added anyways,
>>> correct? As you mentioned above, this might need to be done anyways for
>>> the rename.
>>>
>>> Something like:
>>> - "compatible = nvidia,tegra-audio-max98090" implements old inversion
>>> behavior and leaves the switch as "Headphones" to avoid breaking
>>> older DTs
>>> - "compatible = nvidia,tegra-{venice2,nyan}-audio-max98090" that
>>> separates the inversion behavior and also introduces the rename
>>
>> It'd be preferable to key this off an separate flag rather than the
>> compatible value. That would more directly represent the data in
>> question, and allow any future boards to be added without having to
>> edit the driver to know whether those new boards neded HP DET
>> inversion or not.
>>
>> However, do note that each of the 3 boards using this binding has a
>> board-specific compatible value present already:
>>
>> nvidia,tegra-audio-max98090-venice2
>> nvidia,tegra-audio-max98090-nyan-blaze
>> nvidia,tegra-audio-max98090-nyan-big
>
> Indeed, I noticed those compatible strings after I sent that message.
>
> A property seems the more ideal/desired way to go. However, to avoid
> breaking older DTs, does that mean it must be implemented as assuming
> inversion is default, and set nyan and other boards to "hd-invert = 0"?

The default (if no property is present in the DT) should indeed match 
whatever the current behaviour is in order to maintain compatibility.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack
       [not found]                         ` <56BE9125.3060104-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-02-16 21:43                           ` Stephen Warren
  0 siblings, 0 replies; 38+ messages in thread
From: Stephen Warren @ 2016-02-16 21:43 UTC (permalink / raw)
  To: Jonathan Tinkham, Dylan Reid
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Mark Brown,
	Alexandre Courbot, Thierry Reding

On 02/12/2016 07:12 PM, Jonathan Tinkham wrote:
> On 02/08/2016 11:56 AM, Stephen Warren wrote:
>> On 02/04/2016 12:17 PM, Dylan Reid wrote:
>>> On Thu, Feb 4, 2016 at 11:05 AM, Jonathan Tinkham
>>> <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> On 02/04/2016 09:36 AM, Stephen Warren wrote:
>>>>>
>>>>> On 02/03/2016 10:30 PM, Jonathan Tinkham wrote:
>>>>>>
>>>>>> The headphone jack should not be inverted
>>>>>
>>>>>
>>>>> Have you tested this on Venice2 as well as Nyan? I'm pretty sure
>>>>> Venice2
>>>>> was tested when this driver was written, and whoever added Nyan
>>>>> support to
>>>>> the kernel simply assumed it would work. As such, my suspicion is
>>>>> that this
>>>>> series will break Venice2 even as it fixes Nyan.
>>>>
>>>>
>>>> I have not tested this on Venice2, only on Nyan. That seems like a
>>>> plausible
>>>> cause and reasonable suspicion.
>>>>
>>>>> Why doesn't user-space expect what the kernel actually implements? The
>>>>> kernel should be defining the control naming.
>>>>>
>>>>> Which user-space are you using specifically, and which part of it
>>>>> expects
>>>>> particular naming?
>>>>>
>>>>> Perhaps this series needs to be parametrized based on a flag in DT,
>>>>> rather
>>>>> than switching the hard-coded values, so that only Venice2 can be
>>>>> affected?
>>>>
>>>>
>>>> Specifically pulse-audio and alsa under Arch Linux.
>>>>
>>>> I was referencing 'Documentation/sound/alsa/soc/dapm.txt' with
>>>> regards to
>>>> control names. While it is possible to add another entry into the
>>>> user-space
>>>> configuration, I took this documentation as a definition of kernel
>>>> control
>>>> naming schemes, and thought the driver had used a non-standard
>>>> naming scheme
>>>> (or at least, not a consistent one).
>>>>
>>>>> On 02/03/2016 10:31 PM, Jonathan Tinkham wrote:
>>>>>>
>>>>>> Update device-tree bindings to reflect the rename of the board's
>>>>>> headphone jack.
>>>>>
>>>>>
>>>>> This looks like an incompatible change to the DT. While you've
>>>>> fixed the
>>>>> DT, which will fix new installations, old DTs now won't work. This
>>>>> breaks DT
>>>>> ABI. Any DT change needs to be backwards compatible, i.e. the old name
>>>>> should still work and be documented as a legacy value.
>>>>
>>>>
>>>> I see that now. If the inversion behavior differs between venice2
>>>> and nyan,
>>>> then another compatible string would need to be added anyways,
>>>> correct? As
>>>> you mentioned above, this might need to be done anyways for the rename.
>>>
>>> Venice2 and both nyan platforms do have different polarity of HP detect.
>>>
>>> For some boards we have an hd-invert property in DT.
>>>
>>> Would setting hp-det-gpio to active low in the pinmux achieve the
>>> same thing?
>>
>> I don't believe we have a pinmux setting for this.
>>
>> However, there's a GPIO_ACTIVE_HIGH/LOW flag that could be used in DT
>> property nvidia,hp-det-gpios's flags field to indicate the polarity.
>> That should work, but indeed you could use a separate hp-dt-invert
>> property if not.
>
> Yeah, I could find other things that have an "invert" property, but none
> for headphone/mic detect pins.
>
> I'm not sure setting the GPIO_ACTIVE_HIGH/LOW flag would work. Doesn't
> it depend on how the pin is physically wired?

The flag value used has to match the circuit design, yes. However, I 
don't think there's any requirement that the circuit work any particular 
way for the software to rely on using such a flag.

Logically, there's a "headphone present" signal, and SW needs to 
determine the value of that signal. The "present" value could be 
represented by either a low or a high state in the circuit, depending on 
how the circuit is implemented. The ACTIVE_HIGH/LOW flag simply 
indicates which voltage level is associated with the semantic meaning on 
"headphone present".

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 0/3] Fix headphone detection on Acer Chromebook 13
       [not found]                         ` <56C3977E.8050001-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2016-02-22  6:24                           ` Jonathan Tinkham
       [not found]                             ` <1456122253-3039-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-22  6:24 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	swarren-3lzwWm7+Weoh9ZMKESR00Q, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, Jonathan Tinkham

Add a new DT property to disable inversion of headphone detect GPIO.
This fixes the headphone detection on the CB5-311.

Jonathan Tinkham (3):
  dt/bindings: Add 'nvidia,hd-no-invert' option
  sound/soc/tegra: tegra_max98090: add 'nvidia,hd-no-invert' option
  ARM: tegra: Do not invert nyan headphone detection signal

 .../devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt        | 1 +
 arch/arm/boot/dts/tegra124-nyan.dtsi                                 | 1 +
 sound/soc/tegra/tegra_max98090.c                                     | 5 +++++
 3 files changed, 7 insertions(+)

-- 
2.7.1

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/3] dt/bindings: Add 'nvidia,hd-no-invert' option
       [not found]                             ` <1456122253-3039-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-02-22  6:24                               ` Jonathan Tinkham
       [not found]                                 ` <1456122253-3039-2-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-02-22  6:24                               ` [PATCH 2/3] sound/soc/tegra: tegra_max98090: add 'nvidia,hd-no-invert' option Jonathan Tinkham
                                                 ` (2 subsequent siblings)
  3 siblings, 1 reply; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-22  6:24 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	swarren-3lzwWm7+Weoh9ZMKESR00Q, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, Jonathan Tinkham

Document an option to not invert the headphone detect GPIO signal

Signed-off-by: Jonathan Tinkham <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 Documentation/devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt b/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt
index c3495be..5dac704 100644
--- a/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt
+++ b/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt
@@ -26,6 +26,7 @@ Required properties:
 
 Optional properties:
 - nvidia,hp-det-gpios : The GPIO that detect headphones are plugged in
+- nvidia,hd-no-invert : Do not invert headphone detect signal
 - nvidia,mic-det-gpios : The GPIO that detect microphones are plugged in
 
 Example:
-- 
2.7.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 2/3] sound/soc/tegra: tegra_max98090: add 'nvidia,hd-no-invert' option
       [not found]                             ` <1456122253-3039-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-02-22  6:24                               ` [PATCH 1/3] dt/bindings: Add 'nvidia,hd-no-invert' option Jonathan Tinkham
@ 2016-02-22  6:24                               ` Jonathan Tinkham
  2016-02-22  6:24                               ` [PATCH 3/3] ARM: tegra: Do not invert nyan headphone detection signal Jonathan Tinkham
  2016-02-22  9:46                               ` [PATCH 0/3] Fix headphone detection on Acer Chromebook 13 Mark Brown
  3 siblings, 0 replies; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-22  6:24 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	swarren-3lzwWm7+Weoh9ZMKESR00Q, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, Jonathan Tinkham

Add a device-tree property that does not invert the headphone
detect signal

Signed-off-by: Jonathan Tinkham <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 sound/soc/tegra/tegra_max98090.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/tegra/tegra_max98090.c b/sound/soc/tegra/tegra_max98090.c
index 902da36..73beba6 100644
--- a/sound/soc/tegra/tegra_max98090.c
+++ b/sound/soc/tegra/tegra_max98090.c
@@ -238,6 +238,11 @@ static int tegra_max98090_probe(struct platform_device *pdev)
 	if (machine->gpio_hp_det == -EPROBE_DEFER)
 		return -EPROBE_DEFER;
 
+	if (of_property_read_bool(np, "nvidia,hd-no-invert"))
+		tegra_max98090_hp_jack_gpio.invert = 0;
+	else
+		tegra_max98090_hp_jack_gpio.invert = 1;
+
 	machine->gpio_mic_det =
 			of_get_named_gpio(np, "nvidia,mic-det-gpios", 0);
 	if (machine->gpio_mic_det == -EPROBE_DEFER)
-- 
2.7.1

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 3/3] ARM: tegra: Do not invert nyan headphone detection signal
       [not found]                             ` <1456122253-3039-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-02-22  6:24                               ` [PATCH 1/3] dt/bindings: Add 'nvidia,hd-no-invert' option Jonathan Tinkham
  2016-02-22  6:24                               ` [PATCH 2/3] sound/soc/tegra: tegra_max98090: add 'nvidia,hd-no-invert' option Jonathan Tinkham
@ 2016-02-22  6:24                               ` Jonathan Tinkham
  2016-02-22  9:46                               ` [PATCH 0/3] Fix headphone detection on Acer Chromebook 13 Mark Brown
  3 siblings, 0 replies; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-22  6:24 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	swarren-3lzwWm7+Weoh9ZMKESR00Q, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, Jonathan Tinkham

Signed-off-by: Jonathan Tinkham <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/tegra124-nyan.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi b/arch/arm/boot/dts/tegra124-nyan.dtsi
index ec1aa64..1e6e547 100644
--- a/arch/arm/boot/dts/tegra124-nyan.dtsi
+++ b/arch/arm/boot/dts/tegra124-nyan.dtsi
@@ -694,6 +694,7 @@
 		clock-names = "pll_a", "pll_a_out0", "mclk";
 
 		nvidia,hp-det-gpios = <&gpio TEGRA_GPIO(I, 7) GPIO_ACTIVE_HIGH>;
+		nvidia,hd-no-invert;
 		nvidia,mic-det-gpios =
 				<&gpio TEGRA_GPIO(R, 7) GPIO_ACTIVE_HIGH>;
 	};
-- 
2.7.1

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH 0/3] Fix headphone detection on Acer Chromebook 13
       [not found]                             ` <1456122253-3039-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                                                 ` (2 preceding siblings ...)
  2016-02-22  6:24                               ` [PATCH 3/3] ARM: tegra: Do not invert nyan headphone detection signal Jonathan Tinkham
@ 2016-02-22  9:46                               ` Mark Brown
  3 siblings, 0 replies; 38+ messages in thread
From: Mark Brown @ 2016-02-22  9:46 UTC (permalink / raw)
  To: Jonathan Tinkham
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, swarren-3lzwWm7+Weoh9ZMKESR00Q,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

On Sun, Feb 21, 2016 at 11:24:10PM -0700, Jonathan Tinkham wrote:
> Add a new DT property to disable inversion of headphone detect GPIO.
> This fixes the headphone detection on the CB5-311.
> 
> Jonathan Tinkham (3):
>   dt/bindings: Add 'nvidia,hd-no-invert' option
>   sound/soc/tegra: tegra_max98090: add 'nvidia,hd-no-invert' option
>   ARM: tegra: Do not invert nyan headphone detection signal

As I said in reply to your earlier series:

| Please try to use subject lines reflecting the style for the subsystem -
| this makes it easier for people to spot relevant content.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/3] dt/bindings: Add 'nvidia,hd-no-invert' option
       [not found]                                 ` <1456122253-3039-2-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-02-22 19:48                                   ` Stephen Warren
       [not found]                                     ` <56CB6624.6030600-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 38+ messages in thread
From: Stephen Warren @ 2016-02-22 19:48 UTC (permalink / raw)
  To: Jonathan Tinkham
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

On 02/21/2016 11:24 PM, Jonathan Tinkham wrote:
> Document an option to not invert the headphone detect GPIO signal
>

> diff --git a/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt b/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt

>   Optional properties:
>   - nvidia,hp-det-gpios : The GPIO that detect headphones are plugged in
> +- nvidia,hd-no-invert : Do not invert headphone detect signal

We should use "hp" not "hd" to be consistent with other properties.

Why can't we use the GPIO specifier's ACTIVE_LOW/ACTIVE_HIGH flag to 
represent the polarity of this signal?

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/3] dt/bindings: Add 'nvidia,hd-no-invert' option
       [not found]                                     ` <56CB6624.6030600-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2016-02-22 21:12                                       ` Jonathan Tinkham
  2016-02-23 17:19                                       ` [PATCH] sound/soc/tegra: tegra_max98090: fix hp detect on Chromebook 13 Jonathan Tinkham
  1 sibling, 0 replies; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-22 21:12 UTC (permalink / raw)
  To: Stephen Warren
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

Oh! I see now. That should actually work, will test quickly and see.

On 02/22/2016 12:48 PM, Stephen Warren wrote:
> On 02/21/2016 11:24 PM, Jonathan Tinkham wrote:
>> Document an option to not invert the headphone detect GPIO signal
>>
>
>> diff --git 
>> a/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt 
>> b/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-max98090.txt 
>>
>
>>   Optional properties:
>>   - nvidia,hp-det-gpios : The GPIO that detect headphones are plugged in
>> +- nvidia,hd-no-invert : Do not invert headphone detect signal
>
> We should use "hp" not "hd" to be consistent with other properties.
>
> Why can't we use the GPIO specifier's ACTIVE_LOW/ACTIVE_HIGH flag to 
> represent the polarity of this signal?

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH] sound/soc/tegra: tegra_max98090: fix hp detect on Chromebook 13
       [not found]                                     ` <56CB6624.6030600-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  2016-02-22 21:12                                       ` Jonathan Tinkham
@ 2016-02-23 17:19                                       ` Jonathan Tinkham
       [not found]                                         ` <1456247985-5563-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  1 sibling, 1 reply; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-23 17:19 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	swarren-3lzwWm7+Weoh9ZMKESR00Q, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, Jonathan Tinkham

It appears the driver ignores the GPIO flag for the headphone detect pin. By
using this, in a manner similar to tegra_rt5640, the invert flag can be set
appropriately without changing the DTs.

Jonathan Tinkham (1):
  sound/soc/tegra: tegra_max98090: Invert headphone by GPIO flag

 sound/soc/tegra/tegra_max98090.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

-- 
2.7.1

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH] sound/soc/tegra: tegra_max98090: Invert headphone by GPIO flag
       [not found]                                         ` <1456247985-5563-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-02-23 17:19                                           ` Jonathan Tinkham
       [not found]                                             ` <1456247985-5563-2-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-02-24  3:25                                           ` [PATCH] sound/soc/tegra: tegra_max98090: fix hp detect on Chromebook 13 Mark Brown
  1 sibling, 1 reply; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-23 17:19 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	swarren-3lzwWm7+Weoh9ZMKESR00Q, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, Jonathan Tinkham

Set the invert property for the headphone jack depending on the GPIO flag in the
device-tree. This is similar to what is done for tegra_rt5640.

Signed-off-by: Jonathan Tinkham <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 sound/soc/tegra/tegra_max98090.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/soc/tegra/tegra_max98090.c b/sound/soc/tegra/tegra_max98090.c
index 902da36..49b95c7 100644
--- a/sound/soc/tegra/tegra_max98090.c
+++ b/sound/soc/tegra/tegra_max98090.c
@@ -42,6 +42,7 @@
 struct tegra_max98090 {
 	struct tegra_asoc_utils_data util_data;
 	int gpio_hp_det;
+	enum of_gpio_flags gpio_hp_det_flags;
 	int gpio_mic_det;
 };
 
@@ -155,6 +156,7 @@ static int tegra_max98090_asoc_init(struct snd_soc_pcm_runtime *rtd)
 				      ARRAY_SIZE(tegra_max98090_hp_jack_pins));
 
 		tegra_max98090_hp_jack_gpio.gpio = machine->gpio_hp_det;
+		tegra_max98090_hp_jack_gpio.invert = (machine->gpio_hp_det_flags & OF_GPIO_ACTIVE_LOW);
 		snd_soc_jack_add_gpios(&tegra_max98090_hp_jack,
 					1,
 					&tegra_max98090_hp_jack_gpio);
@@ -234,7 +236,8 @@ static int tegra_max98090_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, card);
 	snd_soc_card_set_drvdata(card, machine);
 
-	machine->gpio_hp_det = of_get_named_gpio(np, "nvidia,hp-det-gpios", 0);
+	machine->gpio_hp_det = of_get_named_gpio_flags(
+                np, "nvidia,hp-det-gpios", 0, &machine->gpio_hp_det_flags);
 	if (machine->gpio_hp_det == -EPROBE_DEFER)
 		return -EPROBE_DEFER;
 
-- 
2.7.1

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH] sound/soc/tegra: tegra_max98090: Invert headphone by GPIO flag
       [not found]                                             ` <1456247985-5563-2-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-02-23 17:35                                               ` Stephen Warren
       [not found]                                                 ` <56CC9860.2060003-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 38+ messages in thread
From: Stephen Warren @ 2016-02-23 17:35 UTC (permalink / raw)
  To: Jonathan Tinkham
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

On 02/23/2016 10:19 AM, Jonathan Tinkham wrote:

 > Subject: sound/soc/tegra: tegra_max98090: Invert headphone by GPIO
 > flag

Mark has mentioned quite a few times that this patch subject is 
incorrect. ASoC patch subjects should start with "ASoC: ". You can see 
this by running:

git log -- sound/soc/tegra/

I'd suggest the following:

AsoC: tegra_max98090: honor headphone detect GPIO polarity

> Set the invert property for the headphone jack depending on the GPIO flag in the
> device-tree. This is similar to what is done for tegra_rt5640.
>

> diff --git a/sound/soc/tegra/tegra_max98090.c b/sound/soc/tegra/tegra_max98090.c

> @@ -155,6 +156,7 @@ static int tegra_max98090_asoc_init(struct snd_soc_pcm_runtime *rtd)
>   				      ARRAY_SIZE(tegra_max98090_hp_jack_pins));
>
>   		tegra_max98090_hp_jack_gpio.gpio = machine->gpio_hp_det;
> +		tegra_max98090_hp_jack_gpio.invert = (machine->gpio_hp_det_flags & OF_GPIO_ACTIVE_LOW);

Did you run checkpatch? It should complain about > 80 columns, and I 
suspect about the unnecessary brackets around that expression. In fact, 
checkpatch indicates quite a few other warnings and errors.

The logic in this patch looks OK. Do the relevant DT files all have the 
correct GPIO flags already? That'd be nice!

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] sound/soc/tegra: tegra_max98090: fix hp detect on Chromebook 13
       [not found]                                         ` <1456247985-5563-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-02-23 17:19                                           ` [PATCH] sound/soc/tegra: tegra_max98090: Invert headphone by GPIO flag Jonathan Tinkham
@ 2016-02-24  3:25                                           ` Mark Brown
       [not found]                                             ` <20160224032552.GJ18327-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  1 sibling, 1 reply; 38+ messages in thread
From: Mark Brown @ 2016-02-24  3:25 UTC (permalink / raw)
  To: Jonathan Tinkham
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, swarren-3lzwWm7+Weoh9ZMKESR00Q,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

[-- Attachment #1: Type: text/plain, Size: 639 bytes --]

On Tue, Feb 23, 2016 at 10:19:44AM -0700, Jonathan Tinkham wrote:

> Jonathan Tinkham (1):
>   sound/soc/tegra: tegra_max98090: Invert headphone by GPIO flag

I have *repeatedly* reminded you to use subject lines reflecting the
style for the subsystem and you continue to ignore me.  Please don't
ignore review comments, people are generally making them for a reason
and are likely to have the same concerns if issues remain unaddressed.
Having to repeat the same comments can get repetitive and make people
question the value of time spent reviewing.  If you disagree with the
concerns so that the reviewer can understand your decisions.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] sound/soc/tegra: tegra_max98090: Invert headphone by GPIO flag
       [not found]                                                 ` <56CC9860.2060003-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2016-02-24 16:42                                                   ` Jonathan Tinkham
       [not found]                                                     ` <56CDDD82.7040402-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-03-02 17:43                                                   ` [PATCH] AsoC: tegra_max98090: honor headphone detect GPIO polarity Jonathan Tinkham
  1 sibling, 1 reply; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-24 16:42 UTC (permalink / raw)
  To: Stephen Warren
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

On 02/23/2016 10:35 AM, Stephen Warren wrote:
> On 02/23/2016 10:19 AM, Jonathan Tinkham wrote:
>
> > Subject: sound/soc/tegra: tegra_max98090: Invert headphone by GPIO
> > flag
>
> Mark has mentioned quite a few times that this patch subject is 
> incorrect. ASoC patch subjects should start with "ASoC: ". You can see 
> this by running:
>
> git log -- sound/soc/tegra/
>
> I'd suggest the following:
>
> AsoC: tegra_max98090: honor headphone detect GPIO polarity
>
My apologies, I finally grok what he was saying. Thank you.
>> Set the invert property for the headphone jack depending on the GPIO 
>> flag in the
>> device-tree. This is similar to what is done for tegra_rt5640.
>>
>
>> diff --git a/sound/soc/tegra/tegra_max98090.c 
>> b/sound/soc/tegra/tegra_max98090.c
>
>> @@ -155,6 +156,7 @@ static int tegra_max98090_asoc_init(struct 
>> snd_soc_pcm_runtime *rtd)
>> ARRAY_SIZE(tegra_max98090_hp_jack_pins));
>>
>>           tegra_max98090_hp_jack_gpio.gpio = machine->gpio_hp_det;
>> +        tegra_max98090_hp_jack_gpio.invert = 
>> (machine->gpio_hp_det_flags & OF_GPIO_ACTIVE_LOW);
>
> Did you run checkpatch? It should complain about > 80 columns, and I 
> suspect about the unnecessary brackets around that expression. In 
> fact, checkpatch indicates quite a few other warnings and errors.
>
> The logic in this patch looks OK. Do the relevant DT files all have 
> the correct GPIO flags already? That'd be nice!

The only board is the venice2, which doesn't have a 'nvidia,hp-det-gpio' 
entry at all (how did this even work before?)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] sound/soc/tegra: tegra_max98090: fix hp detect on Chromebook 13
       [not found]                                             ` <20160224032552.GJ18327-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2016-02-24 16:43                                               ` Jonathan Tinkham
  0 siblings, 0 replies; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-24 16:43 UTC (permalink / raw)
  To: Mark Brown
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, swarren-3lzwWm7+Weoh9ZMKESR00Q,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

On 02/23/2016 08:25 PM, Mark Brown wrote:
> On Tue, Feb 23, 2016 at 10:19:44AM -0700, Jonathan Tinkham wrote:
>
>> Jonathan Tinkham (1):
>>    sound/soc/tegra: tegra_max98090: Invert headphone by GPIO flag
> I have *repeatedly* reminded you to use subject lines reflecting the
> style for the subsystem and you continue to ignore me.  Please don't
> ignore review comments, people are generally making them for a reason
> and are likely to have the same concerns if issues remain unaddressed.
> Having to repeat the same comments can get repetitive and make people
> question the value of time spent reviewing.  If you disagree with the
> concerns so that the reviewer can understand your decisions.
Yes, my apologies. I continually misunderstood your comments

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] sound/soc/tegra: tegra_max98090: Invert headphone by GPIO flag
       [not found]                                                     ` <56CDDD82.7040402-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-02-24 16:45                                                       ` Stephen Warren
       [not found]                                                         ` <56CDDE35.1010408-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 38+ messages in thread
From: Stephen Warren @ 2016-02-24 16:45 UTC (permalink / raw)
  To: Jonathan Tinkham
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w

On 02/24/2016 09:42 AM, Jonathan Tinkham wrote:
> On 02/23/2016 10:35 AM, Stephen Warren wrote:
>> On 02/23/2016 10:19 AM, Jonathan Tinkham wrote:
>>
>> > Subject: sound/soc/tegra: tegra_max98090: Invert headphone by GPIO
>> > flag
>>
>> Mark has mentioned quite a few times that this patch subject is
>> incorrect. ASoC patch subjects should start with "ASoC: ". You can see
>> this by running:
>>
>> git log -- sound/soc/tegra/
>>
>> I'd suggest the following:
>>
>> AsoC: tegra_max98090: honor headphone detect GPIO polarity
>>
> My apologies, I finally grok what he was saying. Thank you.
>>> Set the invert property for the headphone jack depending on the GPIO
>>> flag in the
>>> device-tree. This is similar to what is done for tegra_rt5640.
>>>
>>
>>> diff --git a/sound/soc/tegra/tegra_max98090.c
>>> b/sound/soc/tegra/tegra_max98090.c
>>
>>> @@ -155,6 +156,7 @@ static int tegra_max98090_asoc_init(struct
>>> snd_soc_pcm_runtime *rtd)
>>> ARRAY_SIZE(tegra_max98090_hp_jack_pins));
>>>
>>>           tegra_max98090_hp_jack_gpio.gpio = machine->gpio_hp_det;
>>> +        tegra_max98090_hp_jack_gpio.invert =
>>> (machine->gpio_hp_det_flags & OF_GPIO_ACTIVE_LOW);
>>
>> Did you run checkpatch? It should complain about > 80 columns, and I
>> suspect about the unnecessary brackets around that expression. In
>> fact, checkpatch indicates quite a few other warnings and errors.
>>
>> The logic in this patch looks OK. Do the relevant DT files all have
>> the correct GPIO flags already? That'd be nice!
>
> The only board is the venice2, which doesn't have a 'nvidia,hp-det-gpio'
> entry at all (how did this even work before?)

I mean: Is ACTIVE_LOW/ACTIVE_HIGH flag in the existing 
nvidia,hp-det-gpios already correctly set?

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] sound/soc/tegra: tegra_max98090: Invert headphone by GPIO flag
       [not found]                                                         ` <56CDDE35.1010408-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2016-02-24 16:51                                                           ` Jonathan Tinkham
  0 siblings, 0 replies; 38+ messages in thread
From: Jonathan Tinkham @ 2016-02-24 16:51 UTC (permalink / raw)
  To: Stephen Warren
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w



On 02/24/2016 09:45 AM, Stephen Warren wrote:
> On 02/24/2016 09:42 AM, Jonathan Tinkham wrote:
>> On 02/23/2016 10:35 AM, Stephen Warren wrote:
>>> On 02/23/2016 10:19 AM, Jonathan Tinkham wrote:
>>>
>>> > Subject: sound/soc/tegra: tegra_max98090: Invert headphone by GPIO
>>> > flag
>>>
>>> Mark has mentioned quite a few times that this patch subject is
>>> incorrect. ASoC patch subjects should start with "ASoC: ". You can see
>>> this by running:
>>>
>>> git log -- sound/soc/tegra/
>>>
>>> I'd suggest the following:
>>>
>>> AsoC: tegra_max98090: honor headphone detect GPIO polarity
>>>
>> My apologies, I finally grok what he was saying. Thank you.
>>>> Set the invert property for the headphone jack depending on the GPIO
>>>> flag in the
>>>> device-tree. This is similar to what is done for tegra_rt5640.
>>>>
>>>
>>>> diff --git a/sound/soc/tegra/tegra_max98090.c
>>>> b/sound/soc/tegra/tegra_max98090.c
>>>
>>>> @@ -155,6 +156,7 @@ static int tegra_max98090_asoc_init(struct
>>>> snd_soc_pcm_runtime *rtd)
>>>> ARRAY_SIZE(tegra_max98090_hp_jack_pins));
>>>>
>>>>           tegra_max98090_hp_jack_gpio.gpio = machine->gpio_hp_det;
>>>> +        tegra_max98090_hp_jack_gpio.invert =
>>>> (machine->gpio_hp_det_flags & OF_GPIO_ACTIVE_LOW);
>>>
>>> Did you run checkpatch? It should complain about > 80 columns, and I
>>> suspect about the unnecessary brackets around that expression. In
>>> fact, checkpatch indicates quite a few other warnings and errors.
>>>
>>> The logic in this patch looks OK. Do the relevant DT files all have
>>> the correct GPIO flags already? That'd be nice!
>>
>> The only board is the venice2, which doesn't have a 'nvidia,hp-det-gpio'
>> entry at all (how did this even work before?)
>
> I mean: Is ACTIVE_LOW/ACTIVE_HIGH flag in the existing 
> nvidia,hp-det-gpios already correctly set?

Yes. I also have cleaned up the warnings/errors from checkpatch, and can 
resubmit if acceptable.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH] AsoC: tegra_max98090: honor headphone detect GPIO polarity
       [not found]                                                 ` <56CC9860.2060003-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  2016-02-24 16:42                                                   ` Jonathan Tinkham
@ 2016-03-02 17:43                                                   ` Jonathan Tinkham
       [not found]                                                     ` <1456940623-1496-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  1 sibling, 1 reply; 38+ messages in thread
From: Jonathan Tinkham @ 2016-03-02 17:43 UTC (permalink / raw)
  To: linux-tegra-u79uwXL29TY76Z2rM5mHXA, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw
  Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k,
	swarren-3lzwWm7+Weoh9ZMKESR00Q, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	lars-Qo5EllUWu/uELgA04lAiVw, Jonathan Tinkham

Set the invert property for the headphone jack depending on the GPIO polarity
in the device-tree.

Signed-off-by: Jonathan Tinkham <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 sound/soc/tegra/tegra_max98090.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/soc/tegra/tegra_max98090.c b/sound/soc/tegra/tegra_max98090.c
index 902da36..da4e8d1 100644
--- a/sound/soc/tegra/tegra_max98090.c
+++ b/sound/soc/tegra/tegra_max98090.c
@@ -42,6 +42,7 @@
 struct tegra_max98090 {
 	struct tegra_asoc_utils_data util_data;
 	int gpio_hp_det;
+	enum of_gpio_flags gpio_hp_det_flags;
 	int gpio_mic_det;
 };
 
@@ -155,6 +156,8 @@ static int tegra_max98090_asoc_init(struct snd_soc_pcm_runtime *rtd)
 				      ARRAY_SIZE(tegra_max98090_hp_jack_pins));
 
 		tegra_max98090_hp_jack_gpio.gpio = machine->gpio_hp_det;
+		tegra_max98090_hp_jack_gpio.invert =
+			machine->gpio_hp_det_flags & OF_GPIO_ACTIVE_LOW;
 		snd_soc_jack_add_gpios(&tegra_max98090_hp_jack,
 					1,
 					&tegra_max98090_hp_jack_gpio);
@@ -234,7 +237,8 @@ static int tegra_max98090_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, card);
 	snd_soc_card_set_drvdata(card, machine);
 
-	machine->gpio_hp_det = of_get_named_gpio(np, "nvidia,hp-det-gpios", 0);
+	machine->gpio_hp_det = of_get_named_gpio_flags(
+		np, "nvidia,hp-det-gpios", 0, &machine->gpio_hp_det_flags);
 	if (machine->gpio_hp_det == -EPROBE_DEFER)
 		return -EPROBE_DEFER;
 
-- 
2.7.2

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH] AsoC: tegra_max98090: honor headphone detect GPIO polarity
       [not found]                                                     ` <1456940623-1496-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-03-03 18:37                                                       ` Stephen Warren
       [not found]                                                         ` <56D8845A.4050704-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 38+ messages in thread
From: Stephen Warren @ 2016-03-03 18:37 UTC (permalink / raw)
  To: Jonathan Tinkham, Dylan Reid
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	lars-Qo5EllUWu/uELgA04lAiVw

On 03/02/2016 10:43 AM, Jonathan Tinkham wrote:
> Set the invert property for the headphone jack depending on the GPIO polarity
> in the device-tree.
>
> Signed-off-by: Jonathan Tinkham <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---

I think this is an updatd version of a previous patch, so it should say 
"[PATCH V2]" in the subject, and have a description of the changes you 
made between the versions here, below the --- line.

> diff --git a/sound/soc/tegra/tegra_max98090.c b/sound/soc/tegra/tegra_max98090.c
> index 902da36..da4e8d1 100644
> --- a/sound/soc/tegra/tegra_max98090.c
> +++ b/sound/soc/tegra/tegra_max98090.c
> @@ -42,6 +42,7 @@
>   struct tegra_max98090 {
>   	struct tegra_asoc_utils_data util_data;
>   	int gpio_hp_det;
> +	enum of_gpio_flags gpio_hp_det_flags;
>   	int gpio_mic_det;
>   };
>
> @@ -155,6 +156,8 @@ static int tegra_max98090_asoc_init(struct snd_soc_pcm_runtime *rtd)
>   				      ARRAY_SIZE(tegra_max98090_hp_jack_pins));
>
>   		tegra_max98090_hp_jack_gpio.gpio = machine->gpio_hp_det;
> +		tegra_max98090_hp_jack_gpio.invert =
> +			machine->gpio_hp_det_flags & OF_GPIO_ACTIVE_LOW;

So now, this .invert flag is always set directly based on the GPIO 
flags. That's good.

I believe this means you should remove the .invert assignment from 
static struct snd_soc_jack_gpio tegra_max98090_hp_jack_gpio too, since 
it's always over-written, so doesn't need a default value.

Question: Should we make the same change for DT property 
nvidia,mic-det-gpios and value tegra_max98090_mic_jack_gpio.invert? 
tegra124-nyan.dts currently says GPIO_ACTIVE_HIGH for both hp-det-gpios 
and mic-det-gpios. I don't know if the DT is correct for both of those 
(and hence making the same change for the mic-det-gpios would break 
Nyan) or not.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] AsoC: tegra_max98090: honor headphone detect GPIO polarity
       [not found]                                                         ` <56D8845A.4050704-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2016-03-04 17:42                                                           ` Jonathan Tinkham
       [not found]                                                             ` <56D9C921.7080802-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 38+ messages in thread
From: Jonathan Tinkham @ 2016-03-04 17:42 UTC (permalink / raw)
  To: Stephen Warren, Dylan Reid
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	lars-Qo5EllUWu/uELgA04lAiVw



On 03/03/2016 11:37 AM, Stephen Warren wrote:
> On 03/02/2016 10:43 AM, Jonathan Tinkham wrote:
>> Set the invert property for the headphone jack depending on the GPIO 
>> polarity
>> in the device-tree.
>>
>> Signed-off-by: Jonathan Tinkham <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>
> I think this is an updatd version of a previous patch, so it should 
> say "[PATCH V2]" in the subject, and have a description of the changes 
> you made between the versions here, below the --- line.

Ah yes, my apologies. For the next version should I increment that to v3 
or v2 then?
>
>> diff --git a/sound/soc/tegra/tegra_max98090.c 
>> b/sound/soc/tegra/tegra_max98090.c
>> index 902da36..da4e8d1 100644
>> --- a/sound/soc/tegra/tegra_max98090.c
>> +++ b/sound/soc/tegra/tegra_max98090.c
>> @@ -42,6 +42,7 @@
>>   struct tegra_max98090 {
>>       struct tegra_asoc_utils_data util_data;
>>       int gpio_hp_det;
>> +    enum of_gpio_flags gpio_hp_det_flags;
>>       int gpio_mic_det;
>>   };
>>
>> @@ -155,6 +156,8 @@ static int tegra_max98090_asoc_init(struct 
>> snd_soc_pcm_runtime *rtd)
>> ARRAY_SIZE(tegra_max98090_hp_jack_pins));
>>
>>           tegra_max98090_hp_jack_gpio.gpio = machine->gpio_hp_det;
>> +        tegra_max98090_hp_jack_gpio.invert =
>> +            machine->gpio_hp_det_flags & OF_GPIO_ACTIVE_LOW;
>
> So now, this .invert flag is always set directly based on the GPIO 
> flags. That's good.
>
> I believe this means you should remove the .invert assignment from 
> static struct snd_soc_jack_gpio tegra_max98090_hp_jack_gpio too, since 
> it's always over-written, so doesn't need a default value.
>
> Question: Should we make the same change for DT property 
> nvidia,mic-det-gpios and value tegra_max98090_mic_jack_gpio.invert? 
> tegra124-nyan.dts currently says GPIO_ACTIVE_HIGH for both 
> hp-det-gpios and mic-det-gpios. I don't know if the DT is correct for 
> both of those (and hence making the same change for the mic-det-gpios 
> would break Nyan) or not.
Hmmm, it is indeed the same logic. I will implement and test on nyan.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] AsoC: tegra_max98090: honor headphone detect GPIO polarity
       [not found]                                                             ` <56D9C921.7080802-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-03-04 17:47                                                               ` Stephen Warren
  0 siblings, 0 replies; 38+ messages in thread
From: Stephen Warren @ 2016-03-04 17:47 UTC (permalink / raw)
  To: Jonathan Tinkham
  Cc: Dylan Reid, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	lars-Qo5EllUWu/uELgA04lAiVw

On 03/04/2016 10:42 AM, Jonathan Tinkham wrote:
>
>
> On 03/03/2016 11:37 AM, Stephen Warren wrote:
>> On 03/02/2016 10:43 AM, Jonathan Tinkham wrote:
>>> Set the invert property for the headphone jack depending on the GPIO
>>> polarity
>>> in the device-tree.
>>>
>>> Signed-off-by: Jonathan Tinkham <sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>
>> I think this is an updatd version of a previous patch, so it should
>> say "[PATCH V2]" in the subject, and have a description of the changes
>> you made between the versions here, below the --- line.
>
> Ah yes, my apologies. For the next version should I increment that to v3
> or v2 then?

May as well make it V3 since that's what it is:-)

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2016-03-04 17:47 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-04  5:30 [PATCH 0/4] Fix headphone detection on Acer Chromebook 13 Jonathan Tinkham
     [not found] ` <1454563862-1971-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-04  5:30   ` [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack Jonathan Tinkham
     [not found]     ` <1454563862-1971-2-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-04 16:36       ` Stephen Warren
     [not found]         ` <56B37E19.6010002-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2016-02-04 19:05           ` Jonathan Tinkham
     [not found]             ` <56B3A10E.4010508-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-04 19:17               ` Dylan Reid
     [not found]                 ` <CAEUnVG7bCgzC-f_Anaw6qayGsXi=_NrWZGRy7SSALkLVLPZnUQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-08 18:56                   ` Stephen Warren
     [not found]                     ` <56B8E4D8.8070604-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2016-02-13  2:12                       ` Jonathan Tinkham
     [not found]                         ` <56BE9125.3060104-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-16 21:43                           ` Stephen Warren
2016-02-04 19:37               ` Mark Brown
2016-02-08 18:59               ` Stephen Warren
     [not found]                 ` <56B8E5AD.8040707-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2016-02-13  2:10                   ` Jonathan Tinkham
     [not found]                     ` <56BE9094.1040500-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-16 21:41                       ` Stephen Warren
     [not found]                         ` <56C3977E.8050001-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2016-02-22  6:24                           ` [PATCH 0/3] Fix headphone detection on Acer Chromebook 13 Jonathan Tinkham
     [not found]                             ` <1456122253-3039-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-22  6:24                               ` [PATCH 1/3] dt/bindings: Add 'nvidia,hd-no-invert' option Jonathan Tinkham
     [not found]                                 ` <1456122253-3039-2-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-22 19:48                                   ` Stephen Warren
     [not found]                                     ` <56CB6624.6030600-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2016-02-22 21:12                                       ` Jonathan Tinkham
2016-02-23 17:19                                       ` [PATCH] sound/soc/tegra: tegra_max98090: fix hp detect on Chromebook 13 Jonathan Tinkham
     [not found]                                         ` <1456247985-5563-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-23 17:19                                           ` [PATCH] sound/soc/tegra: tegra_max98090: Invert headphone by GPIO flag Jonathan Tinkham
     [not found]                                             ` <1456247985-5563-2-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-23 17:35                                               ` Stephen Warren
     [not found]                                                 ` <56CC9860.2060003-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2016-02-24 16:42                                                   ` Jonathan Tinkham
     [not found]                                                     ` <56CDDD82.7040402-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-24 16:45                                                       ` Stephen Warren
     [not found]                                                         ` <56CDDE35.1010408-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2016-02-24 16:51                                                           ` Jonathan Tinkham
2016-03-02 17:43                                                   ` [PATCH] AsoC: tegra_max98090: honor headphone detect GPIO polarity Jonathan Tinkham
     [not found]                                                     ` <1456940623-1496-1-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-03-03 18:37                                                       ` Stephen Warren
     [not found]                                                         ` <56D8845A.4050704-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2016-03-04 17:42                                                           ` Jonathan Tinkham
     [not found]                                                             ` <56D9C921.7080802-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-03-04 17:47                                                               ` Stephen Warren
2016-02-24  3:25                                           ` [PATCH] sound/soc/tegra: tegra_max98090: fix hp detect on Chromebook 13 Mark Brown
     [not found]                                             ` <20160224032552.GJ18327-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2016-02-24 16:43                                               ` Jonathan Tinkham
2016-02-22  6:24                               ` [PATCH 2/3] sound/soc/tegra: tegra_max98090: add 'nvidia,hd-no-invert' option Jonathan Tinkham
2016-02-22  6:24                               ` [PATCH 3/3] ARM: tegra: Do not invert nyan headphone detection signal Jonathan Tinkham
2016-02-22  9:46                               ` [PATCH 0/3] Fix headphone detection on Acer Chromebook 13 Mark Brown
2016-02-04  5:31   ` [PATCH 2/4] sound/soc: tegra_max98090: rename headphone jack DAPM Jonathan Tinkham
     [not found]     ` <1454563862-1971-3-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-04 16:38       ` Stephen Warren
2016-02-04  5:31   ` [PATCH 3/4] sound/soc: tegra_max98090: document headphone jack rename Jonathan Tinkham
     [not found]     ` <1454563862-1971-4-git-send-email-sctincman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-04 16:39       ` Stephen Warren
     [not found]         ` <56B37EAC.5010101-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2016-02-04 19:22           ` Dylan Reid
2016-02-04  5:31   ` [PATCH 4/4] soc/sound: tegra_max98090: update dts affected by rename Jonathan Tinkham
2016-02-04 18:03   ` [PATCH 0/4] Fix headphone detection on Acer Chromebook 13 Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.