[PATCH] ASoC: samsung: drop unknown Kconfig symbol

[PATCH] ASoC: samsung: drop unknown Kconfig symbol

[Paul Bolle]

There's no Kconfig symbol 'MACH_NEO1973_GTA01'. So drop all Kconfig
references to that symbol and all code that depends on
CONFIG_MACH_NEO1973_GTA01. This also allows for some related cleanups
(eg, drop some checks for CONFIG_MACH_NEO1973_GTA02, as they have become
useless). Update a few strings too, to make clear that the Openmoko Neo
Freerunner (GTA02) is supported while the Openmoko Neo 1973 (GTA01)
isn't.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
0) This needs thorough testing: I have got neither the hardware nor the
toolchain needed to do that. Also note that the ASoC subsystem is a bit
of a mystery to me.  

1) There were a surprising number of GTA01 related commits in mainline.
I'm not going to list all these here. But there were no commits that
actually added support for that machine. See the output of this command:
    git log --no-merges -p | grep -w -e MACH_GTA01 -e MACH_NEO1973_GTA01

(Is there an easier - or quicker - way to do that?) That won't contain a
line like:
    +config MACH_NEO1973_GTA01

which one would expect if support had ever been added.

2) A follow up to this patch might be a patch to drop support for the
lm4857 codec, as it seems GTA01 specific. (There's currently still a few
references to lm4857 in the code, which look a bit strange to me. I
simply wasn't sure if they are actually - elaborate - nops and could
also be removed.)

 sound/soc/samsung/Kconfig          |    9 ++--
 sound/soc/samsung/neo1973_wm8753.c |   94 +++++------------------------------
 2 files changed, 18 insertions(+), 85 deletions(-)

diff --git a/sound/soc/samsung/Kconfig b/sound/soc/samsung/Kconfig
index 53aaa69..2983ed6 100644
--- a/sound/soc/samsung/Kconfig
+++ b/sound/soc/samsung/Kconfig
@@ -35,15 +35,14 @@ config SND_SAMSUNG_I2S
 	tristate
 
 config SND_SOC_SAMSUNG_NEO1973_WM8753
-	tristate "Audio support for Openmoko Neo1973 Smartphones (GTA01/GTA02)"
-	depends on SND_SOC_SAMSUNG && (MACH_NEO1973_GTA01 || MACH_NEO1973_GTA02)
+	tristate "Audio support for Openmoko Neo Freerunner Smartphone (GTA02)"
+	depends on SND_SOC_SAMSUNG && MACH_NEO1973_GTA02
 	select SND_S3C24XX_I2S
 	select SND_SOC_WM8753
-	select SND_SOC_LM4857 if MACH_NEO1973_GTA01
 	select SND_SOC_DFBMCS320
 	help
-	  Say Y here to enable audio support for the Openmoko Neo1973
-	  Smartphones.
+	  Say Y here to enable audio support for the Openmoko Neo Freerunner
+	  Smartphone (GTA02).
 
 config SND_SOC_SAMSUNG_JIVE_WM8750
 	tristate "SoC I2S Audio support for Jive"
diff --git a/sound/soc/samsung/neo1973_wm8753.c b/sound/soc/samsung/neo1973_wm8753.c
index 7207189..b1c6c79 100644
--- a/sound/soc/samsung/neo1973_wm8753.c
+++ b/sound/soc/samsung/neo1973_wm8753.c
@@ -1,5 +1,5 @@
 /*
- * neo1973_wm8753.c  --  SoC audio for Openmoko Neo1973 and Freerunner devices
+ * neo1973_wm8753.c  --  SoC audio for Openmoko Neo Freerunner devices
  *
  * Copyright 2007 Openmoko Inc
  * Author: Graeme Gregory <graeme@openmoko.org>
@@ -228,10 +228,6 @@ static const struct snd_kcontrol_new neo1973_wm8753_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Handset Mic"),
 };
 
-/* GTA02 specific routes and controls */
-
-#ifdef CONFIG_MACH_NEO1973_GTA02
-
 static int gta02_speaker_enabled;
 
 static int lm4853_set_spk(struct snd_kcontrol *kcontrol,
@@ -311,10 +307,6 @@ static int neo1973_gta02_wm8753_init(struct snd_soc_codec *codec)
 	return 0;
 }
 
-#else
-static int neo1973_gta02_wm8753_init(struct snd_soc_code *codec) { return 0; }
-#endif
-
 static int neo1973_wm8753_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct snd_soc_codec *codec = rtd->codec;
@@ -322,10 +314,6 @@ static int neo1973_wm8753_init(struct snd_soc_pcm_runtime *rtd)
 	int ret;
 
 	/* set up NC codec pins */
-	if (machine_is_neo1973_gta01()) {
-		snd_soc_dapm_nc_pin(dapm, "LOUT2");
-		snd_soc_dapm_nc_pin(dapm, "ROUT2");
-	}
 	snd_soc_dapm_nc_pin(dapm, "OUT3");
 	snd_soc_dapm_nc_pin(dapm, "OUT4");
 	snd_soc_dapm_nc_pin(dapm, "LINE1");
@@ -361,58 +349,14 @@ static int neo1973_wm8753_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_ignore_suspend(dapm, "Headset Mic");
 	snd_soc_dapm_ignore_suspend(dapm, "Handset Mic");
 
-	if (machine_is_neo1973_gta02()) {
-		ret = neo1973_gta02_wm8753_init(codec);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-
-/* GTA01 specific controls */
-
-#ifdef CONFIG_MACH_NEO1973_GTA01
-
-static const struct snd_soc_dapm_route neo1973_lm4857_routes[] = {
-	{"Amp IN", NULL, "ROUT1"},
-	{"Amp IN", NULL, "LOUT1"},
-
-	{"Handset Spk", NULL, "Amp EP"},
-	{"Stereo Out", NULL, "Amp LS"},
-	{"Headphone", NULL, "Amp HP"},
-};
-
-static const struct snd_soc_dapm_widget neo1973_lm4857_dapm_widgets[] = {
-	SND_SOC_DAPM_SPK("Handset Spk", NULL),
-	SND_SOC_DAPM_SPK("Stereo Out", NULL),
-	SND_SOC_DAPM_HP("Headphone", NULL),
-};
-
-static int neo1973_lm4857_init(struct snd_soc_dapm_context *dapm)
-{
-	int ret;
-
-	ret = snd_soc_dapm_new_controls(dapm, neo1973_lm4857_dapm_widgets,
-			ARRAY_SIZE(neo1973_lm4857_dapm_widgets));
+	ret = neo1973_gta02_wm8753_init(codec);
 	if (ret)
 		return ret;
 
-	ret = snd_soc_dapm_add_routes(dapm, neo1973_lm4857_routes,
-			ARRAY_SIZE(neo1973_lm4857_routes));
-	if (ret)
-		return ret;
-
-	snd_soc_dapm_ignore_suspend(dapm, "Stereo Out");
-	snd_soc_dapm_ignore_suspend(dapm, "Handset Spk");
-	snd_soc_dapm_ignore_suspend(dapm, "Headphone");
-
 	return 0;
 }
 
-#else
 static int neo1973_lm4857_init(struct snd_soc_dapm_context *dapm) { return 0; };
-#endif
 
 static struct snd_soc_dai_link neo1973_dai[] = {
 { /* Hifi Playback - for similatious use with voice below */
@@ -454,14 +398,10 @@ static struct snd_soc_codec_conf neo1973_codec_conf[] = {
 	},
 };
 
-#ifdef CONFIG_MACH_NEO1973_GTA02
 static const struct gpio neo1973_gta02_gpios[] = {
 	{ GTA02_GPIO_HP_IN, GPIOF_OUT_INIT_HIGH, "GTA02_HP_IN" },
 	{ GTA02_GPIO_AMP_SHUT, GPIOF_OUT_INIT_HIGH, "GTA02_AMP_SHUT" },
 };
-#else
-static const struct gpio neo1973_gta02_gpios[] = {};
-#endif
 
 static struct snd_soc_card neo1973 = {
 	.name = "neo1973",
@@ -479,18 +419,16 @@ static int __init neo1973_init(void)
 {
 	int ret;
 
-	if (!machine_is_neo1973_gta01() && !machine_is_neo1973_gta02())
+	if (!machine_is_neo1973_gta02())
 		return -ENODEV;
 
-	if (machine_is_neo1973_gta02()) {
-		neo1973.name = "neo1973gta02";
-		neo1973.num_aux_devs = 1;
+	neo1973.name = "neo1973gta02";
+	neo1973.num_aux_devs = 1;
 
-		ret = gpio_request_array(neo1973_gta02_gpios,
-				ARRAY_SIZE(neo1973_gta02_gpios));
-		if (ret)
-			return ret;
-	}
+	ret = gpio_request_array(neo1973_gta02_gpios,
+			ARRAY_SIZE(neo1973_gta02_gpios));
+	if (ret)
+		return ret;
 
 	neo1973_snd_device = platform_device_alloc("soc-audio", -1);
 	if (!neo1973_snd_device) {
@@ -509,10 +447,8 @@ static int __init neo1973_init(void)
 err_put_device:
 	platform_device_put(neo1973_snd_device);
 err_gpio_free:
-	if (machine_is_neo1973_gta02()) {
-		gpio_free_array(neo1973_gta02_gpios,
-				ARRAY_SIZE(neo1973_gta02_gpios));
-	}
+	gpio_free_array(neo1973_gta02_gpios,
+			ARRAY_SIZE(neo1973_gta02_gpios));
 	return ret;
 }
 module_init(neo1973_init);
@@ -521,14 +457,12 @@ static void __exit neo1973_exit(void)
 {
 	platform_device_unregister(neo1973_snd_device);
 
-	if (machine_is_neo1973_gta02()) {
-		gpio_free_array(neo1973_gta02_gpios,
-				ARRAY_SIZE(neo1973_gta02_gpios));
-	}
+	gpio_free_array(neo1973_gta02_gpios,
+			ARRAY_SIZE(neo1973_gta02_gpios));
 }
 module_exit(neo1973_exit);
 
 /* Module information */
 MODULE_AUTHOR("Graeme Gregory, graeme@openmoko.org, www.openmoko.org");
-MODULE_DESCRIPTION("ALSA SoC WM8753 Neo1973 and Frerunner");
+MODULE_DESCRIPTION("ALSA SoC WM8753 Neo Freerunner");
 MODULE_LICENSE("GPL");
-- 
1.7.7.3

Re: [PATCH] ASoC: samsung: drop unknown Kconfig symbol

[Mark Brown]

On Thu, Nov 24, 2011 at 06:17:17PM +0100, Paul Bolle wrote:
> There's no Kconfig symbol 'MACH_NEO1973_GTA01'. So drop all Kconfig
> references to that symbol and all code that depends on
> CONFIG_MACH_NEO1973_GTA01. This also allows for some related cleanups
> (eg, drop some checks for CONFIG_MACH_NEO1973_GTA02, as they have become
> useless). Update a few strings too, to make clear that the Openmoko Neo
> Freerunner (GTA02) is supported while the Openmoko Neo 1973 (GTA01)
> isn't.

There's people somewhat actively working on the OpenMoko devices out of
tree (and I personally have a GTA01) and I'd rather not do anything that
causes any more hassle for them.  I'd suggest discussing this with the
OpenMoko guys if you want to do it.

Re: [PATCH] ASoC: samsung: drop unknown Kconfig symbol

[Paul Bolle]

On Thu, 2011-11-24 at 17:22 +0000, Mark Brown wrote:
> On Thu, Nov 24, 2011 at 06:17:17PM +0100, Paul Bolle wrote:
> There's people somewhat actively working on the OpenMoko devices out of
> tree (and I personally have a GTA01) and I'd rather not do anything that
> causes any more hassle for them.

I don't own a GTA01. Neither do I like to cause hassle for the people
working on OpenMoko.

But the history of the code my patch drops, seems to start with
(mainline) commits 74930bb6db ("[ALSA] ASoC S3C24xx machine drivers -
Openmoko Neo1973") and 80ab1c0e9e ("[ALSA] ASoC S3C24xx machine drivers
- Kconfig"), both dated May 14, 2007. So I'm touching code here that has
been unbuildable (in the mainline tree) for over four years. Isn't that
in itself sufficient to drop this code (and the incomplete kbuild stuff
supporting it)?

> I'd suggest discussing this with the OpenMoko guys if you want to do it.

Who would that be?


Paul Bolle

Re: [PATCH] ASoC: samsung: drop unknown Kconfig symbol

[Mark Brown]

On Thu, Nov 24, 2011 at 08:09:14PM +0100, Paul Bolle wrote:

> - Kconfig"), both dated May 14, 2007. So I'm touching code here that has
> been unbuildable (in the mainline tree) for over four years. Isn't that
> in itself sufficient to drop this code (and the incomplete kbuild stuff
> supporting it)?

No, not really.  None of the OpenMoko code (including GTA02) has ever
been usable in mainline as there's a reasonable patch stack for it out
of tree and the more of their code is in mainline the more likely it is
that that situation will improve.  The cost of extra platforms is really
very low and people actually send me patches for the OpenMoko devices
from time to time (much more frequently than for most other boards) so
at least someone cares.

> > I'd suggest discussing this with the OpenMoko guys if you want to do it.

> Who would that be?

Google tells me http://www.openmoko.org is a good start - you might want
to ask on the openmoko-kernel list.

Re: [PATCH] ASoC: samsung: drop unknown Kconfig symbol

[Paul Bolle]

On Thu, 2011-11-24 at 19:19 +0000, Mark Brown wrote:
> On Thu, Nov 24, 2011 at 08:09:14PM +0100, Paul Bolle wrote:
> > - Kconfig"), both dated May 14, 2007. So I'm touching code here that has
> > been unbuildable (in the mainline tree) for over four years. Isn't that
> > in itself sufficient to drop this code (and the incomplete kbuild stuff
> > supporting it)?
> 
> No, not really.  None of the OpenMoko code (including GTA02) has ever
> been usable in mainline as there's a reasonable patch stack for it out
> of tree and the more of their code is in mainline the more likely it is
> that that situation will improve.

A reasonable out of tree patch stack doesn't make the mainline tree
itself buildable. And it could be that having code in mainline makes it
more likely that this situation will improve. But, when I look at the
current situation of the mainline tree, I just see code that has been
unbuildable for over four years.

> The cost of extra platforms is really
> very low and people actually send me patches for the OpenMoko devices
> from time to time (much more frequently than for most other boards) so
> at least someone cares.

But - repeating my message - none of those patches made the code in the
mainline tree buildable.

I don't have any stake in the GTA01 code. So I don't really mind
deferring this discussion to (say) the end of the v3.3 merge period.
But, surely, there must be a point where one can conclude that
unbuildable code is just that?


Paul Bolle

Re: [PATCH] ASoC: samsung: drop unknown Kconfig symbol

[Mark Brown]

On Thu, Nov 24, 2011 at 08:59:08PM +0100, Paul Bolle wrote:
> On Thu, 2011-11-24 at 19:19 +0000, Mark Brown wrote:

> > The cost of extra platforms is really
> > very low and people actually send me patches for the OpenMoko devices
> > from time to time (much more frequently than for most other boards) so
> > at least someone cares.

> But - repeating my message - none of those patches made the code in the
> mainline tree buildable.

Which is a problem because...  (and note that you mean "selectable in
Kconfig" here).

> I don't have any stake in the GTA01 code. So I don't really mind
> deferring this discussion to (say) the end of the v3.3 merge period.
> But, surely, there must be a point where one can conclude that
> unbuildable code is just that?

No.  Removing this code has negative value.  There is no meaningful cost
in having it in mainline; your patch has been the biggest piece of work
so far other than updates done by people actively working on the
machine.  On the other hand the OpenMoko guys from time to time send
useful patches and report interesting bugs (they sent a patch only last
week in fact) and are definitely a user for the code (which is more than
can be said for most of the legacy boards in mainline).

Please take a step back, think about what value would be brought by
making this change.

Re: [PATCH] ASoC: samsung: drop unknown Kconfig symbol

[Paul Bolle]

On Thu, 2011-11-24 at 22:22 +0000, Mark Brown wrote:
> > But - repeating my message - none of those patches made the code in the
> > mainline tree buildable.
> 
> Which is a problem because...

Because in mainline there's code that can't be built and can't be run.

> (and note that you mean "selectable in
> Kconfig" here).

The Kconfig files in mainline don't define a symbol MACH_NEO1973_GTA01.
The related macro won't show up in a .config file generated by the
config tools, etc. So in mainline we have code that is basically
#ifdef'd out. "Can't be set with the config tools" and "unbuildable"
mean the same thing here.

> No.  Removing this code has negative value.  There is no meaningful cost
> in having it in mainline; your patch has been the biggest piece of work
> so far other than updates done by people actively working on the
> machine.  On the other hand the OpenMoko guys from time to time send
> useful patches and report interesting bugs (they sent a patch only last
> week in fact) and are definitely a user for the code (which is more than
> can be said for most of the legacy boards in mainline).
> 
> Please take a step back, think about what value would be brought by
> making this change.

My claim is just that code that has been unbuildable in the mainline
tree for over four years can be removed. But, as I said in a previous
message, I don't have any stake in the GTA01 code. I don't mind
revisiting all this at (say) the end of the v3.3 merge period.


Paul Bolle

Re: [PATCH] ASoC: samsung: drop unknown Kconfig symbol

[Mark Brown]

On Fri, Nov 25, 2011 at 10:41:06AM +0100, Paul Bolle wrote:
> On Thu, 2011-11-24 at 22:22 +0000, Mark Brown wrote:
> > > But - repeating my message - none of those patches made the code in the
> > > mainline tree buildable.

> > Which is a problem because...

> Because in mainline there's code that can't be built and can't be run.

Which is a problem because...

> > (and note that you mean "selectable in
> > Kconfig" here).

> The Kconfig files in mainline don't define a symbol MACH_NEO1973_GTA01.

Yes, that would be an example fo the sort of thing that would make it
impossible to select in Kconfig.

> My claim is just that code that has been unbuildable in the mainline
> tree for over four years can be removed. But, as I said in a previous
> message, I don't have any stake in the GTA01 code. I don't mind
> revisiting all this at (say) the end of the v3.3 merge period.

No.  What I'm telling you is not to revisit this, this is not a helpful
change.