diff mbox

ASoC: rx51: do not fail if could not get jack detection gpio

Message ID 1422305507-10533-1-git-send-email-pali.rohar@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Pali Rohár Jan. 26, 2015, 8:51 p.m. UTC
qemu does not emulate it

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 sound/soc/omap/rx51.c |   45 +++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

Comments

Peter Ujfalusi Jan. 27, 2015, 1:27 p.m. UTC | #1
Hi,

On 01/26/2015 10:51 PM, Pali Rohár wrote:
> qemu does not emulate it

Not sure about this... How does qemu emulate tlv320aic3106, tpa6130a2a, McBSP,
sDMA for audio to work? What about the other three GPIOs in this machine driver?

The "nokia,n900-audio" compatible mandates that the jack-detection GPIO must
be provided (Documentation/devicetree/bindings/sound/nokia,rx51.txt) since it
means that the driver is running on the Nokia n900 HW.


> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  sound/soc/omap/rx51.c |   45 +++++++++++++++++++++++++--------------------
>  1 file changed, 25 insertions(+), 20 deletions(-)
> 
> diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c
> index 04896d6..a689021 100644
> --- a/sound/soc/omap/rx51.c
> +++ b/sound/soc/omap/rx51.c
> @@ -310,25 +310,27 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd)
>  		return err;
>  	}
>  
> -	/* AV jack detection */
> -	err = snd_soc_jack_new(codec, "AV Jack",
> -			       SND_JACK_HEADSET | SND_JACK_VIDEOOUT,
> -			       &rx51_av_jack);
> -	if (err) {
> -		dev_err(card->dev, "Failed to add AV Jack\n");
> -		return err;
> -	}
> +	if (!IS_ERR(pdata->jack_detection_gpio)) {
> +		/* AV jack detection */
> +		err = snd_soc_jack_new(codec, "AV Jack",
> +				       SND_JACK_HEADSET | SND_JACK_VIDEOOUT,
> +				       &rx51_av_jack);
> +		if (err) {
> +			dev_err(card->dev, "Failed to add AV Jack\n");
> +			return err;
> +		}
>  
> -	/* prepare gpio for snd_soc_jack_add_gpios */
> -	rx51_av_jack_gpios[0].gpio = desc_to_gpio(pdata->jack_detection_gpio);
> -	devm_gpiod_put(card->dev, pdata->jack_detection_gpio);
> +		/* prepare gpio for snd_soc_jack_add_gpios */
> +		rx51_av_jack_gpios[0].gpio = desc_to_gpio(pdata->jack_detection_gpio);
> +		devm_gpiod_put(card->dev, pdata->jack_detection_gpio);
>  
> -	err = snd_soc_jack_add_gpios(&rx51_av_jack,
> -				     ARRAY_SIZE(rx51_av_jack_gpios),
> -				     rx51_av_jack_gpios);
> -	if (err) {
> -		dev_err(card->dev, "Failed to add GPIOs\n");
> -		return err;
> +		err = snd_soc_jack_add_gpios(&rx51_av_jack,
> +					     ARRAY_SIZE(rx51_av_jack_gpios),
> +					     rx51_av_jack_gpios);
> +		if (err) {
> +			dev_err(card->dev, "Failed to add GPIOs\n");
> +			return err;
> +		}
>  	}
>  
>  	return err;
> @@ -336,8 +338,11 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd)
>  
>  static int rx51_card_remove(struct snd_soc_card *card)
>  {
> -	snd_soc_jack_free_gpios(&rx51_av_jack, ARRAY_SIZE(rx51_av_jack_gpios),
> -				rx51_av_jack_gpios);
> +	struct rx51_audio_pdata *pdata = snd_soc_card_get_drvdata(card);
> +
> +	if (!IS_ERR(pdata->jack_detection_gpio))
> +		snd_soc_jack_free_gpios(&rx51_av_jack, ARRAY_SIZE(rx51_av_jack_gpios),
> +					rx51_av_jack_gpios);
>  
>  	return 0;
>  }
> @@ -470,8 +475,8 @@ static int rx51_soc_probe(struct platform_device *pdev)
>  	pdata->jack_detection_gpio = devm_gpiod_get(card->dev,
>  						    "jack-detection");
>  	if (IS_ERR(pdata->jack_detection_gpio)) {
> +		/* Do not fail, qemu does not emulate jack detection gpio */
>  		dev_err(card->dev, "could not get jack detection gpio\n");
> -		return PTR_ERR(pdata->jack_detection_gpio);
>  	}
>  
>  	pdata->eci_sw_gpio = devm_gpiod_get(card->dev, "eci-switch");
>
Pavel Machek Jan. 27, 2015, 1:32 p.m. UTC | #2
On Tue 2015-01-27 15:27:58, Peter Ujfalusi wrote:
> Hi,
> 
> On 01/26/2015 10:51 PM, Pali Rohár wrote:
> > qemu does not emulate it
> 
> Not sure about this... How does qemu emulate tlv320aic3106, tpa6130a2a, McBSP,
> sDMA for audio to work? What about the other three GPIOs in this machine driver?
> 
> The "nokia,n900-audio" compatible mandates that the jack-detection GPIO must
> be provided (Documentation/devicetree/bindings/sound/nokia,rx51.txt) since it
> means that the driver is running on the Nokia n900 HW.

Fix the documentation, then :-). [Actually, GPIO is provided, it just
can't be requested, AFAICT.]

It is quite important to keep qemu working, really. Debugging early
boot on hardware is very hard, for example.

And yes, some day we may need different dts for n900-qemu
(vs. n900-hw), but lets not do it just yet.
							Pavel

> 
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > ---
> >  sound/soc/omap/rx51.c |   45 +++++++++++++++++++++++++--------------------
> >  1 file changed, 25 insertions(+), 20 deletions(-)
> > 
> > diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c
> > index 04896d6..a689021 100644
> > --- a/sound/soc/omap/rx51.c
> > +++ b/sound/soc/omap/rx51.c
> > @@ -310,25 +310,27 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd)
> >  		return err;
> >  	}
> >  
> > -	/* AV jack detection */
> > -	err = snd_soc_jack_new(codec, "AV Jack",
> > -			       SND_JACK_HEADSET | SND_JACK_VIDEOOUT,
> > -			       &rx51_av_jack);
> > -	if (err) {
> > -		dev_err(card->dev, "Failed to add AV Jack\n");
> > -		return err;
> > -	}
> > +	if (!IS_ERR(pdata->jack_detection_gpio)) {
> > +		/* AV jack detection */
> > +		err = snd_soc_jack_new(codec, "AV Jack",
> > +				       SND_JACK_HEADSET | SND_JACK_VIDEOOUT,
> > +				       &rx51_av_jack);
> > +		if (err) {
> > +			dev_err(card->dev, "Failed to add AV Jack\n");
> > +			return err;
> > +		}
> >  
> > -	/* prepare gpio for snd_soc_jack_add_gpios */
> > -	rx51_av_jack_gpios[0].gpio = desc_to_gpio(pdata->jack_detection_gpio);
> > -	devm_gpiod_put(card->dev, pdata->jack_detection_gpio);
> > +		/* prepare gpio for snd_soc_jack_add_gpios */
> > +		rx51_av_jack_gpios[0].gpio = desc_to_gpio(pdata->jack_detection_gpio);
> > +		devm_gpiod_put(card->dev, pdata->jack_detection_gpio);
> >  
> > -	err = snd_soc_jack_add_gpios(&rx51_av_jack,
> > -				     ARRAY_SIZE(rx51_av_jack_gpios),
> > -				     rx51_av_jack_gpios);
> > -	if (err) {
> > -		dev_err(card->dev, "Failed to add GPIOs\n");
> > -		return err;
> > +		err = snd_soc_jack_add_gpios(&rx51_av_jack,
> > +					     ARRAY_SIZE(rx51_av_jack_gpios),
> > +					     rx51_av_jack_gpios);
> > +		if (err) {
> > +			dev_err(card->dev, "Failed to add GPIOs\n");
> > +			return err;
> > +		}
> >  	}
> >  
> >  	return err;
> > @@ -336,8 +338,11 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd)
> >  
> >  static int rx51_card_remove(struct snd_soc_card *card)
> >  {
> > -	snd_soc_jack_free_gpios(&rx51_av_jack, ARRAY_SIZE(rx51_av_jack_gpios),
> > -				rx51_av_jack_gpios);
> > +	struct rx51_audio_pdata *pdata = snd_soc_card_get_drvdata(card);
> > +
> > +	if (!IS_ERR(pdata->jack_detection_gpio))
> > +		snd_soc_jack_free_gpios(&rx51_av_jack, ARRAY_SIZE(rx51_av_jack_gpios),
> > +					rx51_av_jack_gpios);
> >  
> >  	return 0;
> >  }
> > @@ -470,8 +475,8 @@ static int rx51_soc_probe(struct platform_device *pdev)
> >  	pdata->jack_detection_gpio = devm_gpiod_get(card->dev,
> >  						    "jack-detection");
> >  	if (IS_ERR(pdata->jack_detection_gpio)) {
> > +		/* Do not fail, qemu does not emulate jack detection gpio */
> >  		dev_err(card->dev, "could not get jack detection gpio\n");
> > -		return PTR_ERR(pdata->jack_detection_gpio);
> >  	}
> >  
> >  	pdata->eci_sw_gpio = devm_gpiod_get(card->dev, "eci-switch");
> > 
> 
>
Peter Ujfalusi Jan. 27, 2015, 1:41 p.m. UTC | #3
On 01/27/2015 03:32 PM, Pavel Machek wrote:
> On Tue 2015-01-27 15:27:58, Peter Ujfalusi wrote:
>> Hi,
>>
>> On 01/26/2015 10:51 PM, Pali Rohár wrote:
>>> qemu does not emulate it
>>
>> Not sure about this... How does qemu emulate tlv320aic3106, tpa6130a2a, McBSP,
>> sDMA for audio to work? What about the other three GPIOs in this machine driver?
>>
>> The "nokia,n900-audio" compatible mandates that the jack-detection GPIO must
>> be provided (Documentation/devicetree/bindings/sound/nokia,rx51.txt) since it
>> means that the driver is running on the Nokia n900 HW.
> 
> Fix the documentation, then :-).

You mean document a workaround for qemu, right? AFAIK the documentation is fine ;)

> [Actually, GPIO is provided, it just can't be requested, AFAICT.]

So what about the other GPIOs in the machine driver? Why is this
jack-detection GPIO is special for qemu?

> 
> It is quite important to keep qemu working, really. Debugging early
> boot on hardware is very hard, for example.

I understand, but looking at the history of the driver, if it fails now with
the jack-detection GPIO, it should have been failing in the past as well. What
triggered this sudden qemu does not like jack-detection GPIO?
Before the devm_gpiod_get() conversion the driver should have failed in
snd_soc_jack_add_gpios() phase.
Pali Rohár Jan. 27, 2015, 2:11 p.m. UTC | #4
On Tuesday 27 January 2015 14:41:43 Peter Ujfalusi wrote:
> On 01/27/2015 03:32 PM, Pavel Machek wrote:
> > On Tue 2015-01-27 15:27:58, Peter Ujfalusi wrote:
> >> Hi,
> >> 
> >> On 01/26/2015 10:51 PM, Pali Rohár wrote:
> >>> qemu does not emulate it
> >> 
> >> Not sure about this... How does qemu emulate tlv320aic3106,
> >> tpa6130a2a, McBSP, sDMA for audio to work? What about the
> >> other three GPIOs in this machine driver?
> >> 

No idea, but with my patch rx51-audio loads without any problem.

> >> The "nokia,n900-audio" compatible mandates that the
> >> jack-detection GPIO must be provided
> >> (Documentation/devicetree/bindings/sound/nokia,rx51.txt)
> >> since it means that the driver is running on the Nokia
> >> n900 HW.
> > 
> > Fix the documentation, then :-).
> 
> You mean document a workaround for qemu, right? AFAIK the
> documentation is fine ;)
> 
> > [Actually, GPIO is provided, it just can't be requested,
> > AFAICT.]
> 
> So what about the other GPIOs in the machine driver? Why is
> this jack-detection GPIO is special for qemu?
> 

Qemu does not detect it and cause rx51-audio driver to fail.

> > It is quite important to keep qemu working, really.
> > Debugging early boot on hardware is very hard, for example.
> 
> I understand, but looking at the history of the driver, if it
> fails now with the jack-detection GPIO, it should have been
> failing in the past as well. What triggered this sudden qemu
> does not like jack-detection GPIO? Before the
> devm_gpiod_get() conversion the driver should have failed in
> snd_soc_jack_add_gpios() phase.
> 

With 3.12 kernel rx51-audio driver loads without any problem. So 
there is some regression. Here is dmesg log from 3.12 kernel:

[    1.123779] rx51-audio rx51-audio:  tlv320aic3x-hifi <-> omap-
mcbsp.2 mapping ok
[    1.130737] tlv320aic3x-codec 2-0019: ASoC: mux b Right Line1L 
Mux has no paths
[    1.131347] tlv320aic3x-codec 2-0019: ASoC: mux b Left Line1R 
Mux has no paths
[    1.133697] tlv320aic3x-codec 2-0018: ASoC: mux Right Line1L 
Mux has no paths
[    1.134307] tlv320aic3x-codec 2-0018: ASoC: mux Left Line1R 
Mux has no paths
[    1.137451] input: RX-51 AV Jack as /devices/platform/rx51-
audio/sound/card0/input2

I will try to investigate where is problem...

> > And yes, some day we may need different dts for n900-qemu
> > (vs. n900-hw), but lets not do it just yet.
> > 
> > 							Pavel
> >>> 
> >>> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> >>> ---
> >>> 
> >>>  sound/soc/omap/rx51.c |   45
> >>>  +++++++++++++++++++++++++-------------------- 1 file
> >>>  changed, 25 insertions(+), 20 deletions(-)
> >>> 
> >>> diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c
> >>> index 04896d6..a689021 100644
> >>> --- a/sound/soc/omap/rx51.c
> >>> +++ b/sound/soc/omap/rx51.c
> >>> @@ -310,25 +310,27 @@ static int rx51_aic34_init(struct
> >>> snd_soc_pcm_runtime *rtd)
> >>> 
> >>>  		return err;
> >>>  	
> >>>  	}
> >>> 
> >>> -	/* AV jack detection */
> >>> -	err = snd_soc_jack_new(codec, "AV Jack",
> >>> -			       SND_JACK_HEADSET | SND_JACK_VIDEOOUT,
> >>> -			       &rx51_av_jack);
> >>> -	if (err) {
> >>> -		dev_err(card->dev, "Failed to add AV Jack\n");
> >>> -		return err;
> >>> -	}
> >>> +	if (!IS_ERR(pdata->jack_detection_gpio)) {
> >>> +		/* AV jack detection */
> >>> +		err = snd_soc_jack_new(codec, "AV Jack",
> >>> +				       SND_JACK_HEADSET | 
SND_JACK_VIDEOOUT,
> >>> +				       &rx51_av_jack);
> >>> +		if (err) {
> >>> +			dev_err(card->dev, "Failed to add AV Jack\n");
> >>> +			return err;
> >>> +		}
> >>> 
> >>> -	/* prepare gpio for snd_soc_jack_add_gpios */
> >>> -	rx51_av_jack_gpios[0].gpio =
> >>> desc_to_gpio(pdata->jack_detection_gpio);
> >>> -	devm_gpiod_put(card->dev, pdata->jack_detection_gpio);
> >>> +		/* prepare gpio for snd_soc_jack_add_gpios */
> >>> +		rx51_av_jack_gpios[0].gpio =
> >>> desc_to_gpio(pdata->jack_detection_gpio);
> >>> +		devm_gpiod_put(card->dev, pdata-
>jack_detection_gpio);
> >>> 
> >>> -	err = snd_soc_jack_add_gpios(&rx51_av_jack,
> >>> -				     ARRAY_SIZE(rx51_av_jack_gpios),
> >>> -				     rx51_av_jack_gpios);
> >>> -	if (err) {
> >>> -		dev_err(card->dev, "Failed to add GPIOs\n");
> >>> -		return err;
> >>> +		err = snd_soc_jack_add_gpios(&rx51_av_jack,
> >>> +					     ARRAY_SIZE(rx51_av_jack_gpios),
> >>> +					     rx51_av_jack_gpios);
> >>> +		if (err) {
> >>> +			dev_err(card->dev, "Failed to add GPIOs\n");
> >>> +			return err;
> >>> +		}
> >>> 
> >>>  	}
> >>>  	
> >>>  	return err;
> >>> 
> >>> @@ -336,8 +338,11 @@ static int rx51_aic34_init(struct
> >>> snd_soc_pcm_runtime *rtd)
> >>> 
> >>>  static int rx51_card_remove(struct snd_soc_card *card)
> >>>  {
> >>> 
> >>> -	snd_soc_jack_free_gpios(&rx51_av_jack,
> >>> ARRAY_SIZE(rx51_av_jack_gpios), -				
rx51_av_jack_gpios);
> >>> +	struct rx51_audio_pdata *pdata =
> >>> snd_soc_card_get_drvdata(card); +
> >>> +	if (!IS_ERR(pdata->jack_detection_gpio))
> >>> +		snd_soc_jack_free_gpios(&rx51_av_jack,
> >>> ARRAY_SIZE(rx51_av_jack_gpios),
> >>> +					rx51_av_jack_gpios);
> >>> 
> >>>  	return 0;
> >>>  
> >>>  }
> >>> 
> >>> @@ -470,8 +475,8 @@ static int rx51_soc_probe(struct
> >>> platform_device *pdev)
> >>> 
> >>>  	pdata->jack_detection_gpio = devm_gpiod_get(card->dev,
> >>>  	
> >>>  						    "jack-detection");
> >>>  	
> >>>  	if (IS_ERR(pdata->jack_detection_gpio)) {
> >>> 
> >>> +		/* Do not fail, qemu does not emulate jack 
detection
> >>> gpio */
> >>> 
> >>>  		dev_err(card->dev, "could not get jack detection
> >>>  		gpio\n");
> >>> 
> >>> -		return PTR_ERR(pdata->jack_detection_gpio);
> >>> 
> >>>  	}
> >>>  	
> >>>  	pdata->eci_sw_gpio = devm_gpiod_get(card->dev,
> >>>  	"eci-switch");
Pali Rohár Jan. 28, 2015, 2:09 p.m. UTC | #5
> > > It is quite important to keep qemu working, really.
> > > Debugging early boot on hardware is very hard, for
> > > example.
> > 
> > I understand, but looking at the history of the driver, if
> > it fails now with the jack-detection GPIO, it should have
> > been failing in the past as well. What triggered this
> > sudden qemu does not like jack-detection GPIO? Before the
> > devm_gpiod_get() conversion the driver should have failed in
> > snd_soc_jack_add_gpios() phase.
> 
> With 3.12 kernel rx51-audio driver loads without any problem.
> So there is some regression. Here is dmesg log from 3.12
> kernel:
> 
> [    1.123779] rx51-audio rx51-audio:  tlv320aic3x-hifi <->
> omap- mcbsp.2 mapping ok
> [    1.130737] tlv320aic3x-codec 2-0019: ASoC: mux b Right
> Line1L Mux has no paths
> [    1.131347] tlv320aic3x-codec 2-0019: ASoC: mux b Left
> Line1R Mux has no paths
> [    1.133697] tlv320aic3x-codec 2-0018: ASoC: mux Right
> Line1L Mux has no paths
> [    1.134307] tlv320aic3x-codec 2-0018: ASoC: mux Left Line1R
> Mux has no paths
> [    1.137451] input: RX-51 AV Jack as /devices/platform/rx51-
> audio/sound/card0/input2
> 
> I will try to investigate where is problem...
> 

Ok, problem found and solved. Please drop my patch and sorry for 
a noise.

As is visible in above lines AV Jack is working fine on non DT 
kernel (3.12) and fails on DT kernel.

Reason is because I have another non-mainline driver which is 
using GPIOs and in non DT kernel is initialized *after* audio. 
And in DT kernel is initialized *before* audio.

Once I changed my non-mainline driver to not request av jack 
gpio, rx51-audio driver loads fine in qemu.
diff mbox

Patch

diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c
index 04896d6..a689021 100644
--- a/sound/soc/omap/rx51.c
+++ b/sound/soc/omap/rx51.c
@@ -310,25 +310,27 @@  static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd)
 		return err;
 	}
 
-	/* AV jack detection */
-	err = snd_soc_jack_new(codec, "AV Jack",
-			       SND_JACK_HEADSET | SND_JACK_VIDEOOUT,
-			       &rx51_av_jack);
-	if (err) {
-		dev_err(card->dev, "Failed to add AV Jack\n");
-		return err;
-	}
+	if (!IS_ERR(pdata->jack_detection_gpio)) {
+		/* AV jack detection */
+		err = snd_soc_jack_new(codec, "AV Jack",
+				       SND_JACK_HEADSET | SND_JACK_VIDEOOUT,
+				       &rx51_av_jack);
+		if (err) {
+			dev_err(card->dev, "Failed to add AV Jack\n");
+			return err;
+		}
 
-	/* prepare gpio for snd_soc_jack_add_gpios */
-	rx51_av_jack_gpios[0].gpio = desc_to_gpio(pdata->jack_detection_gpio);
-	devm_gpiod_put(card->dev, pdata->jack_detection_gpio);
+		/* prepare gpio for snd_soc_jack_add_gpios */
+		rx51_av_jack_gpios[0].gpio = desc_to_gpio(pdata->jack_detection_gpio);
+		devm_gpiod_put(card->dev, pdata->jack_detection_gpio);
 
-	err = snd_soc_jack_add_gpios(&rx51_av_jack,
-				     ARRAY_SIZE(rx51_av_jack_gpios),
-				     rx51_av_jack_gpios);
-	if (err) {
-		dev_err(card->dev, "Failed to add GPIOs\n");
-		return err;
+		err = snd_soc_jack_add_gpios(&rx51_av_jack,
+					     ARRAY_SIZE(rx51_av_jack_gpios),
+					     rx51_av_jack_gpios);
+		if (err) {
+			dev_err(card->dev, "Failed to add GPIOs\n");
+			return err;
+		}
 	}
 
 	return err;
@@ -336,8 +338,11 @@  static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd)
 
 static int rx51_card_remove(struct snd_soc_card *card)
 {
-	snd_soc_jack_free_gpios(&rx51_av_jack, ARRAY_SIZE(rx51_av_jack_gpios),
-				rx51_av_jack_gpios);
+	struct rx51_audio_pdata *pdata = snd_soc_card_get_drvdata(card);
+
+	if (!IS_ERR(pdata->jack_detection_gpio))
+		snd_soc_jack_free_gpios(&rx51_av_jack, ARRAY_SIZE(rx51_av_jack_gpios),
+					rx51_av_jack_gpios);
 
 	return 0;
 }
@@ -470,8 +475,8 @@  static int rx51_soc_probe(struct platform_device *pdev)
 	pdata->jack_detection_gpio = devm_gpiod_get(card->dev,
 						    "jack-detection");
 	if (IS_ERR(pdata->jack_detection_gpio)) {
+		/* Do not fail, qemu does not emulate jack detection gpio */
 		dev_err(card->dev, "could not get jack detection gpio\n");
-		return PTR_ERR(pdata->jack_detection_gpio);
 	}
 
 	pdata->eci_sw_gpio = devm_gpiod_get(card->dev, "eci-switch");