sound/spitz: Properly handle MIC GPIO

PATCH New Marek Vasut 6 participants 12 messages Apr 2, 2011 – Apr 10, 2011

Subsystems (6)

Sound ASoC Framebuffer layer PXA2xx/PXA3xx support +2 more subsystems

lore

Share & tools

lore

Marek Vasut

author

2011-04-02 01:43 UTC #0 lore raw

PATCH 1/3 sound/spitz: Properly handle MIC GPIO

This patch firstly restructurizes the code a bit by getting rid of continuous checking for machine type in spitz_mic_bias().

Then the patch properly requests the MIC GPIO in the spitz_init() and frees it in spitz_exit().

Signed-off-by Marek Vasut <[email protected]>

---
 sound/soc/pxa/spitz.c |   41 +++++++++++++++++++++++++++++------------
 1 files changed, 29 insertions(+), 12 deletions(-)

sound/soc/pxa/spitz.c +29 ~~-12~~

index 8e15713..b253d86 100644
@@ -42,6 +42,7 @@
 
 static int spitz_jack_func;
 static int spitz_spk_func;
+static int spitz_mic_gpio;
 
 static void spitz_ext_control(struct snd_soc_codec *codec)
 {

@@ -217,14 +218,7 @@ static int spitz_set_spk(struct snd_kcontrol *kcontrol,
 static int spitz_mic_bias(struct snd_soc_dapm_widget *w,
 	struct snd_kcontrol *k, int event)
 {
-	if (machine_is_borzoi() || machine_is_spitz())
-		gpio_set_value(SPITZ_GPIO_MIC_BIAS,
-				SND_SOC_DAPM_EVENT_ON(event));
-
-	if (machine_is_akita())
-		gpio_set_value(AKITA_GPIO_MIC_BIAS,
-				SND_SOC_DAPM_EVENT_ON(event));
-
+	gpio_set_value_cansleep(spitz_mic_gpio, SND_SOC_DAPM_EVENT_ON(event));
 	return 0;
 }

@@ -339,22 +333,45 @@ static int __init spitz_init(void)
 	if (!(machine_is_spitz() || machine_is_borzoi() || machine_is_akita()))
 		return -ENODEV;
 
+	if (machine_is_borzoi() || machine_is_spitz())
+		spitz_mic_gpio = SPITZ_GPIO_MIC_BIAS;
+	else
+		spitz_mic_gpio = AKITA_GPIO_MIC_BIAS;
+
+	ret = gpio_request(spitz_mic_gpio, "MIC GPIO");
+	if (ret)
+		goto err1;
+
+	ret = gpio_direction_output(spitz_mic_gpio, 0);
+	if (ret)
+		goto err2;
+
 	spitz_snd_device = platform_device_alloc("soc-audio", -1);
-	if (!spitz_snd_device)
-		return -ENOMEM;
+	if (!spitz_snd_device) {
+		ret = -ENOMEM;
+		goto err2;
+	}
 
 	platform_set_drvdata(spitz_snd_device, &snd_soc_spitz);
-	ret = platform_device_add(spitz_snd_device);
 
+	ret = platform_device_add(spitz_snd_device);
 	if (ret)
-		platform_device_put(spitz_snd_device);
+		goto err3;
+
+	return 0;
 
+err3:
+	platform_device_put(spitz_snd_device);
+err2:
+	gpio_free(spitz_mic_gpio);
+err1:
 	return ret;
 }
 
 static void __exit spitz_exit(void)
 {
 	platform_device_unregister(spitz_snd_device);
+	gpio_free(spitz_mic_gpio);
 }
 
 module_init(spitz_init);

Signature · 1 line

1.7.4.1

Marek Vasut

author

2011-04-02 01:43 UTC #1 lore raw

PATCH 2/3 Use gpio_set_value_cansleep() in corgi_lcd.c

Signed-off-by Marek Vasut <[email protected]>

---
 drivers/video/backlight/corgi_lcd.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/video/backlight/corgi_lcd.c +2 -2

index 1e71c35..8c80cb7 100644
@@ -409,10 +409,10 @@ static int corgi_bl_set_intensity(struct corgi_lcd *lcd, int intensity)
 	cont = !!(intensity & 0x20) ^ lcd->gpio_backlight_cont_inverted;
 
 	if (gpio_is_valid(lcd->gpio_backlight_cont))
-		gpio_set_value(lcd->gpio_backlight_cont, cont);
+		gpio_set_value_cansleep(lcd->gpio_backlight_cont, cont);
 
 	if (gpio_is_valid(lcd->gpio_backlight_on))
-		gpio_set_value(lcd->gpio_backlight_on, intensity);
+		gpio_set_value_cansleep(lcd->gpio_backlight_on, intensity);
 
 	if (lcd->kick_battery)
 		lcd->kick_battery();

Signature · 1 line

1.7.4.1

Mark Brown

2011-04-04 12:06 UTC #3 lore raw

PATCH 2/3 Use gpio_set_value_cansleep() in corgi_lcd.c

Quoted 3 lines · Eric Miao

Personally, I don't quite like the idea of an explicit _cansleep() version, which is just confusing. A programmer shouldn't be aware of which version to use.

When working in interrupt context you need to pay attention to this stuff; personally I'd prefer an explict IRQ safe version but YMMV. The only use the code makes of this is to generate warnings to make problems easier to diagnose.

Quoted 1 line

David, you have any justification?

Sadly David passed away recently.

Eric Miao

2011-04-04 12:33 UTC #4 lore raw

PATCH 2/3 Use gpio_set_value_cansleep() in corgi_lcd.c

Quoted 5 lines · Mark Brown

Quoted 3 lines · Eric Miao
Personally, I don't quite like the idea of an explicit _cansleep() version, which is just confusing. A programmer shouldn't be aware of which version to use.

Quoted 9 lines

When working in interrupt context you need to pay attention to this stuff; personally I'd prefer an explict IRQ safe version but YMMV. ?The only use the code makes of this is to generate warnings to make problems easier to diagnose.
Quoted 1 line
David, you have any justification?
Sadly David passed away recently.

What????!!!!????? I didn't know that.

Marek Vasut

2011-04-04 12:44 UTC #5 lore raw

PATCH 2/3 Use gpio_set_value_cansleep() in corgi_lcd.c

Quoted 12 lines

Quoted 10 lines · Mark Brown
Quoted 3 lines · Eric Miao
Personally, I don't quite like the idea of an explicit _cansleep() version, which is just confusing. A programmer shouldn't be aware of which version to use.
When working in interrupt context you need to pay attention to this stuff; personally I'd prefer an explict IRQ safe version but YMMV. ?The only use the code makes of this is to generate warnings to make problems easier to diagnose.

Quoted 6 lines

Quoted 4 lines
Quoted 1 line
David, you have any justification?
Sadly David passed away recently.
What????!!!!????? I didn't know that.

Me neither. I'm shocked. What happened?

Russell King - ARM Linux

2011-04-04 12:49 UTC #6 lore raw

PATCH 2/3 Use gpio_set_value_cansleep() in corgi_lcd.c

Quoted 20 lines · Marek Vasut

Quoted 12 lines
Quoted 10 lines · Mark Brown
Quoted 3 lines · Eric Miao
Personally, I don't quite like the idea of an explicit _cansleep() version, which is just confusing. A programmer shouldn't be aware of which version to use.
When working in interrupt context you need to pay attention to this stuff; personally I'd prefer an explict IRQ safe version but YMMV. ?The only use the code makes of this is to generate warnings to make problems easier to diagnose.
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
David, you have any justification?
Sadly David passed away recently.
What????!!!!????? I didn't know that.
Me neither. I'm shocked. What happened?

Marek Vasut

2011-04-04 12:52 UTC #7 lore raw

PATCH 2/3 Use gpio_set_value_cansleep() in corgi_lcd.c

Quoted 20 lines · Marek Vasut

Quoted 19 lines
Quoted 18 lines
Quoted 5 lines · Mark Brown
Quoted 3 lines · Eric Miao
Personally, I don't quite like the idea of an explicit _cansleep() version, which is just confusing. A programmer shouldn't be aware of which version to use.
Quoted 9 lines
When working in interrupt context you need to pay attention to this stuff; personally I'd prefer an explict IRQ safe version but YMMV. ?The only use the code makes of this is to generate warnings to make problems easier to diagnose.
Quoted 1 line
David, you have any justification?
Sadly David passed away recently.
What????!!!!????? I didn't know that.

Quoted 9 lines

Quoted 1 line
Me neither. I'm shocked. What happened?
From what I can tell, so far it's been passed around in private mail only, and so people are hearing about it 3rd, 4th or more hand - which really isn't the way to go about giving this news.
So, I'm not going to pass on the details which I've heard, instead I'd suggest people wait for (or try to find) an official announcement.

My condolences :-(

Russell King - ARM Linux

2011-04-04 12:59 UTC #8 lore raw

PATCH 2/3 Use gpio_set_value_cansleep() in corgi_lcd.c

In reply to Russell King - ARM Linux

Quoted 20 lines · Russell King - ARM Linux

Quoted 19 lines · Marek Vasut
Quoted 18 lines
Quoted 5 lines · Mark Brown
Quoted 3 lines · Eric Miao
Personally, I don't quite like the idea of an explicit _cansleep() version, which is just confusing. A programmer shouldn't be aware of which version to use.
Quoted 9 lines
When working in interrupt context you need to pay attention to this stuff; personally I'd prefer an explict IRQ safe version but YMMV. ?The only use the code makes of this is to generate warnings to make problems easier to diagnose.
Quoted 1 line
David, you have any justification?
Sadly David passed away recently.
What????!!!!????? I didn't know that.

Quoted 8 lines

Quoted 3 lines
Me neither. I'm shocked. What happened?
From what I can tell, so far it's been passed around in private mail
only, and so people are hearing about it 3rd, 4th or more hand - which really isn't the way to go about giving this news.
So, I'm not going to pass on the details which I've heard, instead I'd suggest people wait for (or try to find) an official announcement.

Additionally, I'd like to say that I've requested from the person I heard the news from the details for sending condolences, but so far I'm none the wiser.

Pavel Machek

2011-04-10 07:58 UTC #9 lore raw

PATCH 2/3 Use gpio_set_value_cansleep() in corgi_lcd.c

In reply to Eric Miao

Quoted 5 lines · Eric Miao

Personally, I don't quite like the idea of an explicit _cansleep() version, which is just confusing. A programmer shouldn't be aware of which version to use.
David, you have any justification?

Whether it is possible to sleep in given context is not something that can be detected automatically in general case; so two different functions are needed.
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Marek Vasut

author

2011-04-02 01:43 UTC #10 lore raw

PATCH 3/3 PXA: Reorganise pxa2xx_base init() function

In reply to Marek Vasut

This kills dead code and kills some redundant code.

Original idea by: Cyril Hrubis <[email protected]>

Signed-off-by Marek Vasut <[email protected]>

---
 drivers/pcmcia/pxa2xx_base.c |   30 +++++++++++++-----------------
 1 files changed, 13 insertions(+), 17 deletions(-)

drivers/pcmcia/pxa2xx_base.c +13 ~~-17~~

index 2c54054..687d203 100644
@@ -296,18 +296,20 @@ static int pxa2xx_drv_pcmcia_probe(struct platform_device *dev)
 		goto err0;
 	}
 
-	clk = clk_get(&dev->dev, NULL);
-	if (!clk)
-		return -ENODEV;
-
-	pxa2xx_drv_pcmcia_ops(ops);
-
 	sinfo = kzalloc(SKT_DEV_INFO_SIZE(ops->nr), GFP_KERNEL);
 	if (!sinfo) {
-		clk_put(clk);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto err0;
 	}
 
+	clk = clk_get(&dev->dev, NULL);
+	if (!clk) {
+		ret = -ENODEV;
+		goto err0;
+	}
+
+	pxa2xx_drv_pcmcia_ops(ops);
+
 	sinfo->nskt = ops->nr;
 	sinfo->clk = clk;

@@ -327,15 +329,8 @@ static int pxa2xx_drv_pcmcia_probe(struct platform_device *dev)
 			goto err1;
 	}
 
-	if (ret) {
-		while (--i >= 0)
-			soc_pcmcia_remove_one(&sinfo->skt[i]);
-		kfree(sinfo);
-		clk_put(clk);
-	} else {
-		pxa2xx_configure_sockets(&dev->dev);
-		dev_set_drvdata(&dev->dev, sinfo);
-	}
+	pxa2xx_configure_sockets(&dev->dev);
+	dev_set_drvdata(&dev->dev, sinfo);
 
 	return 0;

@@ -343,6 +338,7 @@ err1:
 	while (--i >= 0)
 		soc_pcmcia_remove_one(&sinfo->skt[i]);
 	kfree(sinfo);
+	clk_put(clk);
 err0:
 	return ret;
 }

Signature · 1 line

1.7.4.1

Mark Brown

Applied

2011-04-03 13:18 UTC #11 lore raw

PATCH 1/3 sound/spitz: Properly handle MIC GPIO

In reply to Marek Vasut

Quoted 7 lines · Marek Vasut

This patch firstly restructurizes the code a bit by getting rid of continuous checking for machine type in spitz_mic_bias().
Then the patch properly requests the MIC GPIO in the spitz_init() and frees it in spitz_exit().
Signed-off-by Marek Vasut <[email protected]>

Applied, thanks.