[alsa-devel] [PATCH] OMAP general SOC driver.
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 +
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.
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
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
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.
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
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
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
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.
On Wednesday 26 November 2008, Mark Brown wrote:
I wonder ifit 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. :)
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.
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.
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.
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
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.
* 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
participants (8)
-
Arun KS -
David Brownell -
Jarkko Nikula -
Koen Kooi -
Mark Brown -
Stanley.Miao -
Steve Sakoman -
Tony Lindgren