[PATCH] OMAP general SOC driver.

[PATCH] OMAP general SOC driver.

[Stanley.Miao]

Add a shared omap SoC driver to avoid reduplicate code among omap soc drivers.

Signed-off-by: Stanley.Miao <stanley.miao@windriver.com>
---
 sound/soc/omap/Kconfig        |   49 +++++++++++++
 sound/soc/omap/Makefile       |   13 ++++
 sound/soc/omap/omap-general.c |  155 +++++++++++++++++++++++++++++++++++++++++
 sound/soc/omap/omap-general.h |   70 ++++++++++++++++++
 4 files changed, 287 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/omap/omap-general.c
 create mode 100644 sound/soc/omap/omap-general.h

diff --git a/sound/soc/omap/Kconfig b/sound/soc/omap/Kconfig
index 8b7766b..92a557f 100644
--- a/sound/soc/omap/Kconfig
+++ b/sound/soc/omap/Kconfig
@@ -14,6 +14,14 @@ config SND_OMAP_SOC_N810
 	help
 	  Say Y if you want to add support for SoC audio on Nokia N810.
 
+config SND_OMAP_SOC_OMAP3_BEAGLE
+	tristate "SoC Audio support for OMAP3 Beagle"
+	depends on SND_OMAP_SOC && MACH_OMAP3_BEAGLE
+	select SND_OMAP_SOC_MCBSP
+	select SND_SOC_TWL4030
+	help
+	  Say Y if you want to add support for SoC audio on the Beagleboard.
+
 config SND_OMAP_SOC_OSK5912
 	tristate "SoC Audio support for omap osk5912"
 	depends on SND_OMAP_SOC && MACH_OMAP_OSK
@@ -21,3 +29,44 @@ config SND_OMAP_SOC_OSK5912
 	select SND_SOC_TLV320AIC23
 	help
 	  Say Y if you want to add support for SoC audio on osk5912.
+
+config SND_OMAP_SOC_OVERO
+	tristate "SoC Audio support for Gumstix Overo"
+	depends on SND_OMAP_SOC && MACH_OVERO
+	select SND_OMAP_SOC_MCBSP
+	select SND_SOC_TWL4030
+	help
+	  Say Y if you want to add support for SoC audio on the Gumstix Overo.
+
+config SND_OMAP_SOC_3430SDP
+	tristate "SoC Audio support for OMAP 3430SDP"
+	depends on SND_OMAP_SOC && MACH_OMAP_3430SDP
+	select SND_OMAP_SOC_MCBSP
+	select SND_SOC_TWL4030
+	help
+	  Say Y if you want to add support for SoC audio on the OMAP 3430SDP.
+
+config SND_OMAP_SOC_LDP
+	tristate "SoC Audio support for OMAP ZOOM SDK"
+	depends on SND_OMAP_SOC && MACH_OMAP_LDP
+	select SND_OMAP_SOC_MCBSP
+	select SND_SOC_TWL4030
+	help
+	  Say Y if you want to add support for SoC audio on the OMAP ZOOM MDK.
+
+config SND_OMAP_SOC_OMAP2EVM
+	tristate "SoC Audio support for OMAP2EVM"
+	depends on SND_OMAP_SOC && MACH_OMAP2EVM
+	select SND_OMAP_SOC_MCBSP
+	select SND_SOC_TWL4030
+	help
+	  Say Y if you want to add support for SoC audio on the OMAP2EVM.
+
+config SND_OMAP_SOC_OMAP3EVM
+	tristate "SoC Audio support for OMAP3EVM"
+	depends on SND_OMAP_SOC && MACH_OMAP3EVM
+	select SND_OMAP_SOC_MCBSP
+	select SND_SOC_TWL4030
+	help
+	  Say Y if you want to add support for SoC audio on the OMAP3EVM.
+
diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile
index e09d1f2..de9163b 100644
--- a/sound/soc/omap/Makefile
+++ b/sound/soc/omap/Makefile
@@ -7,7 +7,20 @@ obj-$(CONFIG_SND_OMAP_SOC_MCBSP) += snd-soc-omap-mcbsp.o
 
 # OMAP Machine Support
 snd-soc-n810-objs := n810.o
+snd-soc-omap3beagle-objs := omap-general.o
 snd-soc-osk5912-objs := osk5912.o
+snd-soc-overo-objs := omap-general.o
+snd-soc-3430sdp-objs := omap-general.o
+snd-soc-ldp-objs := omap-general.o
+snd-soc-omap2evm-objs := omap-general.o
+snd-soc-omap3evm-objs := omap-general.o
 
 obj-$(CONFIG_SND_OMAP_SOC_N810) += snd-soc-n810.o
+obj-$(CONFIG_SND_OMAP_SOC_OMAP3_BEAGLE) += snd-soc-omap3beagle.o
 obj-$(CONFIG_SND_OMAP_SOC_OSK5912) += snd-soc-osk5912.o
+obj-$(CONFIG_SND_OMAP_SOC_OVERO) += snd-soc-overo.o
+obj-$(CONFIG_SND_OMAP_SOC_3430SDP) += snd-soc-3430sdp.o
+obj-$(CONFIG_SND_OMAP_SOC_LDP) += snd-soc-ldp.o
+obj-$(CONFIG_SND_OMAP_SOC_OMAP2EVM) += snd-soc-omap2evm.o
+obj-$(CONFIG_SND_OMAP_SOC_OMAP3EVM) += snd-soc-omap3evm.o
+
diff --git a/sound/soc/omap/omap-general.c b/sound/soc/omap/omap-general.c
new file mode 100644
index 0000000..a3fa647
--- /dev/null
+++ b/sound/soc/omap/omap-general.c
@@ -0,0 +1,155 @@
+/*
+ * omap-general.c  --  OMAP SoC general machine file.
+ *
+ * Author: Stanley Miao <stanley.miao@windriver.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
+
+#include <asm/mach-types.h>
+#include <mach/hardware.h>
+#include <mach/gpio.h>
+#include <mach/mcbsp.h>
+
+#include "omap-mcbsp.h"
+#include "omap-pcm.h"
+#include "omap-general.h"
+
+static int general_hw_params(struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *codec_dai = rtd->dai->codec_dai;
+	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
+	int ret;
+
+	/* Set codec DAI configuration */
+	ret = snd_soc_dai_set_fmt(codec_dai,
+				  SND_SOC_DAIFMT_I2S |
+				  SND_SOC_DAIFMT_NB_NF |
+				  SND_SOC_DAIFMT_CBM_CFM);
+	if (ret < 0) {
+		printk(KERN_ERR "can't set codec DAI configuration\n");
+		return ret;
+	}
+
+	/* Set cpu DAI configuration */
+	ret = snd_soc_dai_set_fmt(cpu_dai,
+				  SND_SOC_DAIFMT_I2S |
+				  SND_SOC_DAIFMT_NB_NF |
+				  SND_SOC_DAIFMT_CBM_CFM);
+	if (ret < 0) {
+		printk(KERN_ERR "can't set cpu DAI configuration\n");
+		return ret;
+	}
+
+	/* Set the codec system clock for DAC and ADC */
+	ret = snd_soc_dai_set_sysclk(codec_dai, 0, CODEC_SYS_CLOCK,
+					    SND_SOC_CLOCK_IN);
+	if (ret < 0) {
+		printk(KERN_ERR "can't set codec system clock\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct snd_soc_ops general_ops = {
+	.startup = SOC_OPS_STARTUP,
+	.shutdown = SOC_OPS_SHUTDOWN,
+	.hw_params = general_hw_params,
+};
+
+/* Digital audio interface glue - connects codec <--> CPU */
+static struct snd_soc_dai_link general_dai = {
+	.name = CODEC_NAME,
+	.stream_name = STREAM_NAME,
+	.cpu_dai = &omap_mcbsp_dai[0],
+	.codec_dai = CODEC_DAI,
+	.ops = &general_ops,
+	.init = DAI_LINK_INIT,
+};
+
+/* Audio machine driver */
+static struct snd_soc_machine snd_soc_omap_general = {
+	.name = "omap_general",
+	.dai_link = &general_dai,
+	.num_links = 1,
+};
+
+/* Audio subsystem */
+static struct snd_soc_device general_snd_devdata = {
+	.machine = &snd_soc_omap_general,
+	.platform = &omap_soc_platform,
+	.codec_dev = CODEC_DEV,
+	.codec_data = CODEC_SETUP_DATA,
+};
+
+static struct platform_device *general_snd_device;
+
+static int __init general_soc_init(void)
+{
+	int ret;
+
+	if (!MACHINE_IS_OMAP_GENERAL) {
+		pr_debug("Not supported board!\n");
+		return -ENODEV;
+	}
+	printk(KERN_INFO "OMAP General SoC init\n");
+
+	general_snd_device = platform_device_alloc("soc-audio", -1);
+	if (!general_snd_device) {
+		printk(KERN_ERR "Platform device allocation failed\n");
+		return -ENOMEM;
+	}
+
+	platform_set_drvdata(general_snd_device, &general_snd_devdata);
+	general_snd_devdata.dev = &general_snd_device->dev;
+	*(unsigned int *)general_dai.cpu_dai->private_data = 1; /* McBSP2 */
+
+	ret = platform_device_add(general_snd_device);
+	if (ret)
+		goto err1;
+	ret = omap_board_soc_init(general_snd_device);
+	if (ret)
+		goto err1;
+
+	return 0;
+
+err1:
+	platform_device_put(general_snd_device);
+
+	return ret;
+}
+module_init(general_soc_init);
+
+static void __exit general_soc_exit(void)
+{
+	omap_board_soc_exit(general_snd_device);
+	platform_device_unregister(general_snd_device);
+}
+module_exit(general_soc_exit);
+
+MODULE_AUTHOR("Stanley Miao <stanley.miao@windriver.com>");
+MODULE_DESCRIPTION("ALSA SoC OMAP GENERAL");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/omap/omap-general.h b/sound/soc/omap/omap-general.h
new file mode 100644
index 0000000..8b9515c
--- /dev/null
+++ b/sound/soc/omap/omap-general.h
@@ -0,0 +1,70 @@
+/*
+ * omap-general.h
+ *
+ * Copyright (C) 2008 Wind River Systems, Inc.
+ *
+ * Author: Stanley Miao <stanley.miao@windriver.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#ifdef CONFIG_SND_OMAP_SOC_N810
+#include "../codecs/tlv320aic3x.h"
+#define CODEC_SYS_CLOCK		12000000
+#define CODEC_NAME		"TLV320AIC33"
+#define STREAM_NAME		"AIC33"
+#define CODEC_DEV		(&soc_codec_dev_aic3x)
+#define CODEC_DAI		(&aic3x_dai)
+#define CODEC_SETUP_DATA	(&&n810_aic33_setup)
+#define DAI_LINK_INIT		(n810_aic33_init)
+#define SOC_OPS_STARTUP		(n810_startup)
+#define SOC_OPS_SHUTDOWN	(n810_shutdown)
+#else
+#include "../codecs/twl4030.h"
+#define CODEC_SYS_CLOCK		26000000
+#define CODEC_NAME		"TWL4030"
+#define STREAM_NAME		"TWL4030"
+#define CODEC_DEV		(&soc_codec_dev_twl4030)
+#define CODEC_DAI		(&twl4030_dai)
+#define CODEC_SETUP_DATA 	NULL
+#define DAI_LINK_INIT		NULL
+#define SOC_OPS_STARTUP		NULL
+#define SOC_OPS_SHUTDOWN	NULL
+#endif /* CONFIG_SND_OMAP_SOC_N810 */
+
+#ifdef CONFIG_SND_OMAP_SOC_N810
+#define MACHINE_IS_OMAP_GENERAL (machine_is_nokia_n810() || machine_is_nokia_n810_wimax())
+#elif defined(CONFIG_SND_OMAP_SOC_OMAP3_BEAGLE)
+#define MACHINE_IS_OMAP_GENERAL machine_is_omap3_beagle()
+#elif defined(CONFIG_SND_OMAP_SOC_OVERO)
+#define MACHINE_IS_OMAP_GENERAL machine_is_overo()
+#elif defined(CONFIG_SND_OMAP_SOC_3430SDP)
+#define MACHINE_IS_OMAP_GENERAL machine_is_omap_3430sdp()
+#elif defined(CONFIG_SND_OMAP_SOC_LDP)
+#define MACHINE_IS_OMAP_GENERAL machine_is_omap_ldp()
+#elif defined(CONFIG_SND_OMAP_SOC_OMAP2EVM)
+#define MACHINE_IS_OMAP_GENERAL machine_is_omap2evm()
+#elif defined(CONFIG_SND_OMAP_SOC_OMAP3EVM)
+#define MACHINE_IS_OMAP_GENERAL machine_is_omapevm()
+#endif
+
+#ifndef omap_board_soc_init
+#define omap_board_soc_init(x) 0
+#endif
+#ifndef omap_board_soc_exit
+#define omap_board_soc_exit
+#endif
+
-- 
1.5.6.3

Re: [PATCH] OMAP general SOC driver.

[Mark Brown]

On Wed, Nov 26, 2008 at 06:47:58PM +0800, Stanley.Miao wrote:
> Add a shared omap SoC driver to avoid reduplicate code among omap soc drivers.

It's really good to see work on this but I've got some concerns with the
approach being taken here.

> +
> +config SND_OMAP_SOC_OVERO
> +	tristate "SoC Audio support for Gumstix Overo"
> +	depends on SND_OMAP_SOC && MACH_OVERO

This won't apply - this and several of the other machines are already
present.

> +#ifdef CONFIG_SND_OMAP_SOC_N810
> +#include "../codecs/tlv320aic3x.h"
> +#define CODEC_SYS_CLOCK		12000000
> +#define CODEC_NAME		"TLV320AIC33"
> +#define STREAM_NAME		"AIC33"
> +#define CODEC_DEV		(&soc_codec_dev_aic3x)
> +#define CODEC_DAI		(&aic3x_dai)
> +#define CODEC_SETUP_DATA	(&&n810_aic33_setup)
> +#define DAI_LINK_INIT		(n810_aic33_init)
> +#define SOC_OPS_STARTUP		(n810_startup)
> +#define SOC_OPS_SHUTDOWN	(n810_shutdown)

Please change this to use a platform data based approach rather than
these ifdefs: it's not a good approach for readability and it prevents
building multiple board drivers in one kernel which isn't good for
distributions.  The s3c24xx_uda134x.c provides an example of how this
can be done.  It may be more sensible to have a separate driver per
codec.

Re: [PATCH] OMAP general SOC driver.

[Jarkko Nikula]

On Wed, 26 Nov 2008 11:08:55 +0000
"ext Mark Brown" <broonie@sirena.org.uk> wrote:

> > +#ifdef CONFIG_SND_OMAP_SOC_N810
> > +#include "../codecs/tlv320aic3x.h"
> > +#define CODEC_SYS_CLOCK		12000000
> > +#define CODEC_NAME		"TLV320AIC33"
> > +#define STREAM_NAME		"AIC33"
> > +#define CODEC_DEV		(&soc_codec_dev_aic3x)
> > +#define CODEC_DAI		(&aic3x_dai)
> > +#define CODEC_SETUP_DATA	(&&n810_aic33_setup)
> > +#define DAI_LINK_INIT		(n810_aic33_init)
> > +#define SOC_OPS_STARTUP		(n810_startup)
> > +#define SOC_OPS_SHUTDOWN	(n810_shutdown)
> 
> Please change this to use a platform data based approach rather than
> these ifdefs: it's not a good approach for readability and it prevents
> building multiple board drivers in one kernel which isn't good for
> distributions.  The s3c24xx_uda134x.c provides an example of how this
> can be done.  It may be more sensible to have a separate driver per
> codec.
> --
And as a first step, I recommend to start merging only those EVM & SDP
boards with Beagle since they seems to be most close to each other.

Even the Overo seems to be close also, it was discussed earlier to keep
it separated now since Steve was going to send some new features into
it.


Jarkko

Re: [PATCH] OMAP general SOC driver.

[David Brownell]

On Wednesday 26 November 2008, Jarkko Nikula wrote:
> And as a first step, I recommend to start merging only those EVM & SDP
> boards with Beagle since they seems to be most close to each other.
> 
> Even the Overo seems to be close also, it was discussed earlier to keep
> it separated now since Steve was going to send some new features into
> it.

This seems to focus on just the McBSP channel out of the OMAP ...
except that it hides (in that nasty #ifdeffery) the codec data.

The fact that it doesn't allow EAC or a McBSP successor says
to me it's not really "general".  Heck ... it even hard-wires
McBSP2, preventing another channel from being used.  So IMO
a different name would be appropriate...

Re TWL4030 codec configuration ... I count three microphone
input channels not used on Beagle, plus a speaker output
and some other stuff.  So it's obvious that something which
works on Beagle won't handle more interesting configurations
of that audio support.  


Plus, if there's going to be a board-specific platform device
created, and stuffed with data like codec descriptions and
configuration data ... that's what arch/arm/mach-*/board-*.c
files are for.

I'm told that the ASoC stuff "should" go in a separate ASoC
area for some reason.  That still makes no good sense to me,
so if there's a brief explanation as to why it's done that
way, please fling me a URL.  :)

- Dave

Re: [PATCH] OMAP general SOC driver.

[Mark Brown]

On Wed, Nov 26, 2008 at 10:34:41AM -0800, David Brownell wrote:

> Re TWL4030 codec configuration ... I count three microphone
> input channels not used on Beagle, plus a speaker output
> and some other stuff.  So it's obvious that something which
> works on Beagle won't handle more interesting configurations
> of that audio support.  

Can someone post an overview of what the hardware configurations of
these boards are, please?  It's starting to sound like they're not very
similar at all.

> I'm told that the ASoC stuff "should" go in a separate ASoC
> area for some reason.  That still makes no good sense to me,
> so if there's a brief explanation as to why it's done that
> way, please fling me a URL.  :)

The move is in the direction you want but we're not there yet.  The fact
that things are now decomposed enough for us to be able to think about
it is a good sign here.

The biggest part of it is that the degree of coupling between the
various ASoC components is high - this is particularly obvious with v1
where the entire card is probed at once.  This includes coupling between
the drivers and the core where the degree of churn is very high right
now due to v2 merging.  It doesn't feel right to give architectures an
API that they can't rely on from one merge window to the next yet,
partly from the point of view of isolating the code for review purposes
and also due to the merge issues which would doubtless crop up.

Another part of it is that some machine drivers can get involved enough
to sit on or cross the boundary from platform data to being drivers for
substantial distinct hardware but that's very much not the case for
everything.

I've been spending time this week working on getting the ability to load
platform and codec drivers independantly merged.  That will at least
allow the instantiation of the ASoC drivers for those to be pushed out
into the architecture code, which is a start and should substantially
reduce the size of most machine drivers.  At the point where that's been
done it's probably worth looking again at the simpler machine drivers.

Re: [PATCH] OMAP general SOC driver.

[Koen Kooi]

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


Op 26 nov 2008, om 21:07 heeft Mark Brown het volgende geschreven:

> On Wed, Nov 26, 2008 at 10:34:41AM -0800, David Brownell wrote:
>
>> Re TWL4030 codec configuration ... I count three microphone
>> input channels not used on Beagle, plus a speaker output
>> and some other stuff.  So it's obvious that something which
>> works on Beagle won't handle more interesting configurations
>> of that audio support.
>
> Can someone post an overview of what the hardware configurations of
> these boards are, please?  It's starting to sound like they're not  
> very
> similar at all.

On the codec level all these board use a variant of the TWL4030. The  
variants differ slightly in what kind of extra goodies they provide.  
On the board level not every feature of the TWL familiy is used, the  
beagle only analog in and analog out, the overo exposes more  
interfaces, etc.

Other people can fill in the details, but these are the differences I  
can think of.

regards,

Koen 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]

Re: [PATCH] OMAP general SOC driver.

[Arun KS]

On Thu, Nov 27, 2008 at 1:50 AM, Koen Kooi <k.kooi@student.utwente.nl> wrote:
>
> Op 26 nov 2008, om 21:07 heeft Mark Brown het volgende geschreven:
>
>> On Wed, Nov 26, 2008 at 10:34:41AM -0800, David Brownell wrote:
>>
>>> Re TWL4030 codec configuration ... I count three microphone
>>> input channels not used on Beagle, plus a speaker output
>>> and some other stuff.  So it's obvious that something which
>>> works on Beagle won't handle more interesting configurations
>>> of that audio support.
>>
>> Can someone post an overview of what the hardware configurations of
>> these boards are, please?  It's starting to sound like they're not very
>> similar at all.
>
> On the codec level all these board use a variant of the TWL4030. The
> variants differ slightly in what kind of extra goodies they provide. On the
> board level not every feature of the TWL familiy is used, the beagle only
> analog in and analog out, the overo exposes more interfaces, etc.
>
> Other people can fill in the details, but these are the differences I can
> think of.

omap2evm and omap3evm uses only auxiliary input(AUXR/AUXL)  and
Headset stereo output (HSOR/HSOL)

Regards,
Arun
>
> regards,
>
> Koen
>

Re: [PATCH] OMAP general SOC driver.

[Mark Brown]

On Thu, Nov 27, 2008 at 12:42:12PM +0200, Grazvydas Ignotas wrote:

> Pandora board uses different master/slave configuration for McBSP bus,
> compared to most other boards. It also has another audio path (in
> addition to TWL4030) - another McBSP link connected to external DAC.
> The DAC needs 256*Fs clock from TWL4030 to function, and is used for
> main audio output. TWL4030 is to be used for 2 MIC + line inputs only,
> and has no outputs connected.

This definitely needs to be a separate machine driver.

> I have main audio path working (McBSP2 to DAC), but not inputs (McBSP4
> to TWL4030). How can this be implemented using current framework?
> Should I register 2 platform devices? Sorry for possible derail or
> stupid questions.

This should all be one ASoC device, especially given that the fact that
all the paths use the TWL4030 somehow.  Have a look at how, eg, the
neo1973 driver handles the bluetooth.  For now there's not
*particularly* sane support for multiple codecs.  Does the DAC have any
software control?  If not it should be fairly straightforward to handle.

Re: [PATCH] OMAP general SOC driver.

[David Brownell]

On Wednesday 26 November 2008, Mark Brown wrote:
> Can someone post an overview of what the hardware configurations of
> these boards are, please?  It's starting to sound like they're not very
> similar at all.

I only have schematics for Beagle ... which makes VERY minimal
use of the twl4030 audio capabilities:

 - stereo "headset" output, without its mic
 - stereo aux-in

Other twl4030 family boards *could* have:

 - microphone input for that headset
 - stereo "hands-off" speakers
 - mono earpiece output
 - two other microphone input channels

And maybe more; that's just me summarizing unconnected pins,
not the datasheet.  There are also cost-reduced parts with
less audio capability.

I make no claim about audio expertise -- the nearest I come
to it is observing a few years back that there was a huge
framework hole, which ASoC seems to fill -- but I wonder if
it shouldn't suffice just to provide a mask of capabilities
that are wired up on a given board.

- Dave

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

Re: [PATCH] OMAP general SOC driver.

[Mark Brown]

On Wed, Nov 26, 2008 at 12:33:12PM -0800, David Brownell wrote:

> And maybe more; that's just me summarizing unconnected pins,
> not the datasheet.  There are also cost-reduced parts with
> less audio capability.

...

> I make no claim about audio expertise -- the nearest I come
> to it is observing a few years back that there was a huge
> framework hole, which ASoC seems to fill -- but I wonder if
> it shouldn't suffice just to provide a mask of capabilities
> that are wired up on a given board.

It's possible - if the differences can be handled by marking some pins
as connected or disconnected and possible specifying a different system
clock rate to the chip then that approach was what I was expecting.
If there is more substantial differences such as having the clocking
arranged differently then separate drivers start to make more sense.

Re: [PATCH] OMAP general SOC driver.

[David Brownell]

On Wednesday 26 November 2008, Mark Brown wrote:
> >			I wonder if
> > it shouldn't suffice just to provide a mask of capabilities
> > that are wired up on a given board.
> 
> It's possible - if the differences can be handled by marking some pins
> as connected or disconnected and possible specifying a different system
> clock rate to the chip then that approach was what I was expecting.
> If there is more substantial differences such as having the clocking
> arranged differently then separate drivers start to make more sense.

OK, from my non-expert perspective it sounds eminently do-able.
By someone else.  :)

Re: [PATCH] OMAP general SOC driver.

[stanley.miao]

On Wed, 2008-11-26 at 21:16 +0000, Mark Brown wrote:
> On Wed, Nov 26, 2008 at 12:33:12PM -0800, David Brownell wrote:
> 
> > And maybe more; that's just me summarizing unconnected pins,
> > not the datasheet.  There are also cost-reduced parts with
> > less audio capability.
> 
> ...
> 
> > I make no claim about audio expertise -- the nearest I come
> > to it is observing a few years back that there was a huge
> > framework hole, which ASoC seems to fill -- but I wonder if
> > it shouldn't suffice just to provide a mask of capabilities
> > that are wired up on a given board.
> 
> It's possible - if the differences can be handled by marking some pins
> as connected or disconnected and possible specifying a different system
> clock rate to the chip then that approach was what I was expecting.
> If there is more substantial differences such as having the clocking
> arranged differently then separate drivers start to make more sense.

I had considered the differences among every boards. If you want add
some board specific dapm widgets or other features, you can use the
extended interface to add them. If a board has too much differences, you
can separate it from this driver and implement it independently.

Now there are four same driver files in alsa-devel tree and there are
another three same drives waiting to be pushed. So I think this shared
diver is needed. 

After reading all of the suggestions, I reworked the driver and will
send it again. This new version has a better difference-tolerance
ability.

Stanley.

Re: [PATCH] OMAP general SOC driver.

[David Brownell]

On Wednesday 26 November 2008, Mark Brown wrote:
> > I'm told that the ASoC stuff "should" go in a separate ASoC
> > area for some reason.  That still makes no good sense to me,
> > so if there's a brief explanation as to why it's done that
> > way, please fling me a URL.  :)
> 
> The move is in the direction you want but we're not there yet.  The fact
> that things are now decomposed enough for us to be able to think about
> it is a good sign here.
> 
> The biggest part of it is that the degree of coupling between the
> various ASoC components is high - this is particularly obvious with v1
> where the entire card is probed at once.  This includes coupling between
> the drivers and the core where the degree of churn is very high right
> now due to v2 merging.  It doesn't feel right to give architectures an
> API that they can't rely on from one merge window to the next yet,

OK, that seems to be the key point.  And it makes a lot of sense;
I wouldn't call the driver structure here "simple", so it deserves
some iteration on multiple disparate hardware platforms just to make
sure the interfaces is adequate to the problem.

Let me insert a minor plug from the PM perspective that it would be
good to find a way that the codecs can sit in the proper places in
the driver model tree.  Example with twl4030:  it's an I2C device
and thus should be a child of something like

   /sys/devices/platform/i2c_omap.1/i2c-adapter/i2c-1/1-0049

to guarantee that for example nothing touches that codec after its
parent I2C controller gets suspended.

- Dave


> partly from the point of view of isolating the code for review purposes
> and also due to the merge issues which would doubtless crop up.
> 
> Another part of it is that some machine drivers can get involved enough
> to sit on or cross the boundary from platform data to being drivers for
> substantial distinct hardware but that's very much not the case for
> everything.
> 
> I've been spending time this week working on getting the ability to load
> platform and codec drivers independantly merged.  That will at least
> allow the instantiation of the ASoC drivers for those to be pushed out
> into the architecture code, which is a start and should substantially
> reduce the size of most machine drivers.  At the point where that's been
> done it's probably worth looking again at the simpler machine drivers.
> 


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

Re: [PATCH] OMAP general SOC driver.

[Mark Brown]

On Wed, Nov 26, 2008 at 12:44:33PM -0800, David Brownell wrote:

> Let me insert a minor plug from the PM perspective that it would be
> good to find a way that the codecs can sit in the proper places in
> the driver model tree.  Example with twl4030:  it's an I2C device
> and thus should be a child of something like

This is exactly what I'm saying about allowing the machine, codec and
platform drivers to probe separately.  It's the major benefit of v2.

Re: [PATCH] OMAP general SOC driver.

[Steve Sakoman]

On Wed, Nov 26, 2008 at 3:08 AM, Mark Brown <broonie@sirena.org.uk> wrote:
> On Wed, Nov 26, 2008 at 06:47:58PM +0800, Stanley.Miao wrote:
>> Add a shared omap SoC driver to avoid reduplicate code among omap soc drivers.
>
> It's really good to see work on this but I've got some concerns with the
> approach being taken here.

I too have concerns.

The SoC drivers are very much alike at the moment because only the
minimum subset of twl4030 functionality is supported.

As we flesh out the twl4030 features (see the other twl4030 codec
driver threads on this list) I expect the machine specific drivers to
begin to diverge (radically in some cases) based upon the hardware
features of each.

I'm all for factoring out common code, but I think that this approach
will become a maintenance nightmare over time.

Steve

Re: [alsa-devel] [PATCH] OMAP general SOC driver.

[Mark Brown]

On Wed, Nov 26, 2008 at 09:24:03AM -0800, Steve Sakoman wrote:

> I'm all for factoring out common code, but I think that this approach
> will become a maintenance nightmare over time.

I had formed the impression that an awful lot of the boards had very
similar audio subsystems and that even with DAPM support it'd be fairly
straightforward to do standard drivers for certain classes of system.
If that's not the case then doing this sort of shared machine driver
probably isn't worth it at all.

Re: [PATCH] OMAP general SOC driver.

[Tony Lindgren]

* Stanley.Miao <stanley.miao@windriver.com> [081126 02:42]:
> Add a shared omap SoC driver to avoid reduplicate code among omap soc drivers.
> 
> Signed-off-by: Stanley.Miao <stanley.miao@windriver.com>
> ---
>  sound/soc/omap/Kconfig        |   49 +++++++++++++
>  sound/soc/omap/Makefile       |   13 ++++
>  sound/soc/omap/omap-general.c |  155 +++++++++++++++++++++++++++++++++++++++++
>  sound/soc/omap/omap-general.h |   70 ++++++++++++++++++
>  4 files changed, 287 insertions(+), 0 deletions(-)
>  create mode 100644 sound/soc/omap/omap-general.c
>  create mode 100644 sound/soc/omap/omap-general.h
> 

<snip>

> --- /dev/null
> +++ b/sound/soc/omap/omap-general.h
> @@ -0,0 +1,70 @@
> +/*
> + * omap-general.h
> + *
> + * Copyright (C) 2008 Wind River Systems, Inc.
> + *
> + * Author: Stanley Miao <stanley.miao@windriver.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA
> + *
> + */
> +
> +#ifdef CONFIG_SND_OMAP_SOC_N810
> +#include "../codecs/tlv320aic3x.h"
> +#define CODEC_SYS_CLOCK		12000000
> +#define CODEC_NAME		"TLV320AIC33"
> +#define STREAM_NAME		"AIC33"
> +#define CODEC_DEV		(&soc_codec_dev_aic3x)
> +#define CODEC_DAI		(&aic3x_dai)
> +#define CODEC_SETUP_DATA	(&&n810_aic33_setup)
> +#define DAI_LINK_INIT		(n810_aic33_init)
> +#define SOC_OPS_STARTUP		(n810_startup)
> +#define SOC_OPS_SHUTDOWN	(n810_shutdown)
> +#else
> +#include "../codecs/twl4030.h"
> +#define CODEC_SYS_CLOCK		26000000
> +#define CODEC_NAME		"TWL4030"
> +#define STREAM_NAME		"TWL4030"
> +#define CODEC_DEV		(&soc_codec_dev_twl4030)
> +#define CODEC_DAI		(&twl4030_dai)
> +#define CODEC_SETUP_DATA 	NULL
> +#define DAI_LINK_INIT		NULL
> +#define SOC_OPS_STARTUP		NULL
> +#define SOC_OPS_SHUTDOWN	NULL
> +#endif /* CONFIG_SND_OMAP_SOC_N810 */
> +
> +#ifdef CONFIG_SND_OMAP_SOC_N810
> +#define MACHINE_IS_OMAP_GENERAL (machine_is_nokia_n810() || machine_is_nokia_n810_wimax())
> +#elif defined(CONFIG_SND_OMAP_SOC_OMAP3_BEAGLE)
> +#define MACHINE_IS_OMAP_GENERAL machine_is_omap3_beagle()
> +#elif defined(CONFIG_SND_OMAP_SOC_OVERO)
> +#define MACHINE_IS_OMAP_GENERAL machine_is_overo()
> +#elif defined(CONFIG_SND_OMAP_SOC_3430SDP)
> +#define MACHINE_IS_OMAP_GENERAL machine_is_omap_3430sdp()
> +#elif defined(CONFIG_SND_OMAP_SOC_LDP)
> +#define MACHINE_IS_OMAP_GENERAL machine_is_omap_ldp()
> +#elif defined(CONFIG_SND_OMAP_SOC_OMAP2EVM)
> +#define MACHINE_IS_OMAP_GENERAL machine_is_omap2evm()
> +#elif defined(CONFIG_SND_OMAP_SOC_OMAP3EVM)
> +#define MACHINE_IS_OMAP_GENERAL machine_is_omapevm()
> +#endif

Please also remove these ifdef elsif stuff above to compile in support
for multiple boards. The best way to to do it is to pass the
necessary info in platform_data, so you should not need to use
machine_is_omapxxxx() or cpu_is_omapxxxx() in the actual driver code.

Regards,

Tony


> +#ifndef omap_board_soc_init
> +#define omap_board_soc_init(x) 0
> +#endif
> +#ifndef omap_board_soc_exit
> +#define omap_board_soc_exit
> +#endif
> +
> -- 
> 1.5.6.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html