[alsa-devel] [PATCH v3 0/4] Tegra: Harmony: Add internal speaker support
Harmony has headers on the board for an internal speaker and internal mic. This patch series adds various infra-structure, and enables usage of the internal speaker.
The patch series includes both changes to code within the ASoC subsystem and Tegra subsystem, the latter provided only for context when reading the ASoC changes. Would you prefer I separated these so that the ASoC maintainers can grab a whole series and apply it, rather than just a subset of the patches?
While patch 2 is physically located in the Tegra subsystem, I'd consider it part of the ASoC driver. Can such a change be checked into the ASoC tree? This new header file will be required to build to ASoC driver once the following patches are in. I was considering placing this in include/sound, following the example of various codecs, but it doesn't seem a good idea to pollute that directory with platform-specific files. The current location for the file follows the example of s3c24xx/simtec. Perhaps it should be in sound/soc/tegra?
Changelog:
* v3: * Reverted change to WM8903_GPIO_NO_CONFIG * Program WM8903_GP1_FN_MASK in direction_{input,output} * Simplify harmony_event_int_spk * Didn't remove GPIO entries from harmony_wm8903_pdata, since this also sets up all the other fields to 0 too.
Stephen Warren (4): ASoC: WM8903: Expose GPIOs through gpiolib ARM: tegra: Add Harmony sound platform data type ARM: tegra: Platform data fixes for ASoC driver updates ASoC: tegra: Harmony: Support the internal speaker
arch/arm/mach-tegra/board-harmony.c | 31 ++++++ arch/arm/mach-tegra/gpio-names.h | 2 + arch/arm/mach-tegra/include/mach/harmony_audio.h | 19 ++++ include/sound/wm8903.h | 20 ++++- sound/soc/codecs/wm8903.c | 126 +++++++++++++++++++++- sound/soc/tegra/harmony.c | 87 +++++++++++++-- 6 files changed, 272 insertions(+), 13 deletions(-) create mode 100644 arch/arm/mach-tegra/include/mach/harmony_audio.h
Also, fix platform_data GPIO handling to have an explicit "don't touch this pin" option.
Add #defines for the GPIO pin functions.
Signed-off-by: Stephen Warren swarren@nvidia.com --- include/sound/wm8903.h | 20 +++++++- sound/soc/codecs/wm8903.c | 126 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 144 insertions(+), 2 deletions(-)
diff --git a/include/sound/wm8903.h b/include/sound/wm8903.h index b4a0db2..86172cf 100644 --- a/include/sound/wm8903.h +++ b/include/sound/wm8903.h @@ -37,6 +37,21 @@ #define WM8903_MICBIAS_ENA_WIDTH 1 /* MICBIAS_ENA */
/* + * WM8903_GPn_FN values + * + * See datasheets for list of valid values per pin + */ +#define WM8903_GPn_FN_GPIO_OUTPUT 0 +#define WM8903_GPn_FN_BCLK 1 +#define WM8903_GPn_FN_IRQ_OUTPT 2 +#define WM8903_GPn_FN_GPIO_INPUT 3 +#define WM8903_GPn_FN_MICBIAS_CURRENT_DETECT 4 +#define WM8903_GPn_FN_MICBIAS_SHORT_DETECT 5 +#define WM8903_GPn_FN_DMIC_LR_CLK_OUTPUT 6 +#define WM8903_GPn_FN_FLL_LOCK_OUTPUT 8 +#define WM8903_GPn_FN_FLL_CLOCK_OUTPUT 9 + +/* * R116 (0x74) - GPIO Control 1 */ #define WM8903_GP1_FN_MASK 0x1F00 /* GP1_FN - [12:8] */ @@ -231,6 +246,8 @@ #define WM8903_GP5_DB_SHIFT 0 /* GP5_DB */ #define WM8903_GP5_DB_WIDTH 1 /* GP5_DB */
+#define WM8903_NUM_GPIO 5 + struct wm8903_platform_data { bool irq_active_low; /* Set if IRQ active low, default high */
@@ -243,7 +260,8 @@ struct wm8903_platform_data {
int micdet_delay; /* Delay after microphone detection (ms) */
- u32 gpio_cfg[5]; /* Default register values for GPIO pin mux */ + int gpio_base; + u32 gpio_cfg[WM8903_NUM_GPIO]; /* Default register values for GPIO pin mux */ };
#endif diff --git a/sound/soc/codecs/wm8903.c b/sound/soc/codecs/wm8903.c index a2a446c..9c4f2c4 100644 --- a/sound/soc/codecs/wm8903.c +++ b/sound/soc/codecs/wm8903.c @@ -2,6 +2,7 @@ * wm8903.c -- WM8903 ALSA SoC Audio driver * * Copyright 2008 Wolfson Microelectronics + * Copyright 2011 NVIDIA, Inc. * * Author: Mark Brown broonie@opensource.wolfsonmicro.com * @@ -19,6 +20,7 @@ #include <linux/init.h> #include <linux/completion.h> #include <linux/delay.h> +#include <linux/gpio.h> #include <linux/pm.h> #include <linux/i2c.h> #include <linux/platform_device.h> @@ -213,6 +215,7 @@ static u16 wm8903_reg_defaults[] = { };
struct wm8903_priv { + struct snd_soc_codec *codec;
int sysclk; int irq; @@ -230,6 +233,10 @@ struct wm8903_priv { int mic_short; int mic_last_report; int mic_delay; + +#ifdef CONFIG_GPIOLIB + struct gpio_chip gpio_chip; +#endif };
static int wm8903_volatile_register(struct snd_soc_codec *codec, unsigned int reg) @@ -1635,6 +1642,119 @@ static int wm8903_resume(struct snd_soc_codec *codec) return 0; }
+#ifdef CONFIG_GPIOLIB +static inline struct wm8903_priv *gpio_to_wm8903(struct gpio_chip *chip) +{ + return container_of(chip, struct wm8903_priv, gpio_chip); +} + +static int wm8903_gpio_request(struct gpio_chip *chip, unsigned offset) +{ + if (offset >= WM8903_NUM_GPIO) + return -EINVAL; + + return 0; +} + +static int wm8903_gpio_direction_in(struct gpio_chip *chip, unsigned offset) +{ + struct wm8903_priv *wm8903 = gpio_to_wm8903(chip); + struct snd_soc_codec *codec = wm8903->codec; + unsigned int mask, val; + + mask = WM8903_GP1_FN_MASK | WM8903_GP1_DIR_MASK; + val = (WM8903_GPn_FN_GPIO_INPUT << WM8903_GP1_FN_SHIFT) | + WM8903_GP1_DIR; + + return snd_soc_update_bits(codec, WM8903_GPIO_CONTROL_1 + offset, + mask, val); +} + +static int wm8903_gpio_get(struct gpio_chip *chip, unsigned offset) +{ + struct wm8903_priv *wm8903 = gpio_to_wm8903(chip); + struct snd_soc_codec *codec = wm8903->codec; + int reg; + + reg = snd_soc_read(codec, WM8903_GPIO_CONTROL_1 + offset); + + return (reg & WM8903_GP1_LVL_MASK) >> WM8903_GP1_LVL_SHIFT; +} + +static int wm8903_gpio_direction_out(struct gpio_chip *chip, + unsigned offset, int value) +{ + struct wm8903_priv *wm8903 = gpio_to_wm8903(chip); + struct snd_soc_codec *codec = wm8903->codec; + unsigned int mask, val; + + mask = WM8903_GP1_FN_MASK | WM8903_GP1_DIR_MASK | WM8903_GP1_LVL_MASK; + val = (WM8903_GPn_FN_GPIO_OUTPUT << WM8903_GP1_FN_SHIFT) | + (value << WM8903_GP2_LVL_SHIFT); + + return snd_soc_update_bits(codec, WM8903_GPIO_CONTROL_1 + offset, + mask, val); +} + +static void wm8903_gpio_set(struct gpio_chip *chip, unsigned offset, int value) +{ + struct wm8903_priv *wm8903 = gpio_to_wm8903(chip); + struct snd_soc_codec *codec = wm8903->codec; + + snd_soc_update_bits(codec, WM8903_GPIO_CONTROL_1 + offset, + WM8903_GP1_LVL_MASK, value << WM8903_GP1_LVL_SHIFT); +} + +static struct gpio_chip wm8903_template_chip = { + .label = "wm8903", + .owner = THIS_MODULE, + .request = wm8903_gpio_request, + .direction_input = wm8903_gpio_direction_in, + .get = wm8903_gpio_get, + .direction_output = wm8903_gpio_direction_out, + .set = wm8903_gpio_set, + .can_sleep = 1, +}; + +static void wm8903_init_gpio(struct snd_soc_codec *codec) +{ + struct wm8903_priv *wm8903 = snd_soc_codec_get_drvdata(codec); + struct wm8903_platform_data *pdata = dev_get_platdata(codec->dev); + int ret; + + wm8903->gpio_chip = wm8903_template_chip; + wm8903->gpio_chip.ngpio = WM8903_NUM_GPIO; + wm8903->gpio_chip.dev = codec->dev; + + if (pdata && pdata->gpio_base) + wm8903->gpio_chip.base = pdata->gpio_base; + else + wm8903->gpio_chip.base = -1; + + ret = gpiochip_add(&wm8903->gpio_chip); + if (ret != 0) + dev_err(codec->dev, "Failed to add GPIOs: %d\n", ret); +} + +static void wm8903_free_gpio(struct snd_soc_codec *codec) +{ + struct wm8903_priv *wm8903 = snd_soc_codec_get_drvdata(codec); + int ret; + + ret = gpiochip_remove(&wm8903->gpio_chip); + if (ret != 0) + dev_err(codec->dev, "Failed to remove GPIOs: %d\n", ret); +} +#else +static void wm8903_init_gpio(struct snd_soc_codec *codec) +{ +} + +static void wm8903_free_gpio(struct snd_soc_codec *codec) +{ +} +#endif + static int wm8903_probe(struct snd_soc_codec *codec) { struct wm8903_platform_data *pdata = dev_get_platdata(codec->dev); @@ -1643,6 +1763,7 @@ static int wm8903_probe(struct snd_soc_codec *codec) int trigger, irq_pol; u16 val;
+ wm8903->codec = codec; init_completion(&wm8903->wseq);
ret = snd_soc_codec_set_cache_io(codec, 8, 16, SND_SOC_I2C); @@ -1667,7 +1788,7 @@ static int wm8903_probe(struct snd_soc_codec *codec) /* Set up GPIOs and microphone detection */ if (pdata) { for (i = 0; i < ARRAY_SIZE(pdata->gpio_cfg); i++) { - if (!pdata->gpio_cfg[i]) + if (pdata->gpio_cfg[i] == WM8903_GPIO_NO_CONFIG) continue;
snd_soc_write(codec, WM8903_GPIO_CONTROL_1 + i, @@ -1749,12 +1870,15 @@ static int wm8903_probe(struct snd_soc_codec *codec) ARRAY_SIZE(wm8903_snd_controls)); wm8903_add_widgets(codec);
+ wm8903_init_gpio(codec); + return ret; }
/* power down chip */ static int wm8903_remove(struct snd_soc_codec *codec) { + wm8903_free_gpio(codec); wm8903_set_bias_level(codec, SND_SOC_BIAS_OFF); return 0; }
On Thu, Jan 20, 2011 at 01:52:08PM -0700, Stephen Warren wrote:
Also, fix platform_data GPIO handling to have an explicit "don't touch this pin" option.
Add #defines for the GPIO pin functions.
Signed-off-by: Stephen Warren swarren@nvidia.com
Applied this one, thanks.
Signed-off-by: Stephen Warren swarren@nvidia.com --- arch/arm/mach-tegra/include/mach/harmony_audio.h | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-) create mode 100644 arch/arm/mach-tegra/include/mach/harmony_audio.h
diff --git a/arch/arm/mach-tegra/include/mach/harmony_audio.h b/arch/arm/mach-tegra/include/mach/harmony_audio.h new file mode 100644 index 0000000..5c46391 --- /dev/null +++ b/arch/arm/mach-tegra/include/mach/harmony_audio.h @@ -0,0 +1,19 @@ +/* + * arch/arm/mach-tegra/include/mach/harmony_audio.h + * + * Copyright 2011 NVIDIA, Inc. + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * 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. + * + */ + +struct harmony_audio_platform_data { + int gpio_spkr_en; +};
On Thu, Jan 20, 2011 at 12:52 PM, Stephen Warren swarren@nvidia.com wrote:
Signed-off-by: Stephen Warren swarren@nvidia.com
arch/arm/mach-tegra/include/mach/harmony_audio.h | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-) create mode 100644 arch/arm/mach-tegra/include/mach/harmony_audio.h
diff --git a/arch/arm/mach-tegra/include/mach/harmony_audio.h b/arch/arm/mach-tegra/include/mach/harmony_audio.h new file mode 100644 index 0000000..5c46391 --- /dev/null +++ b/arch/arm/mach-tegra/include/mach/harmony_audio.h @@ -0,0 +1,19 @@ +/*
- arch/arm/mach-tegra/include/mach/harmony_audio.h
- Copyright 2011 NVIDIA, Inc.
- This software is licensed under the terms of the GNU General Public
- License version 2, as published by the Free Software Foundation, and
- may be copied, distributed, and modified under those terms.
- 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.
- */
+struct harmony_audio_platform_data {
- int gpio_spkr_en;
+};
1.7.1
-- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Does this file really belong in mach-tegra if its for a driver that is in a common directory (drivers/sound/soc)? Putting it in include/linux would solve all your tree-ordering problems.
Colin Cross wrote on Friday, January 21, 2011 11:43 AM:
On Thu, Jan 20, 2011 at 12:52 PM, Stephen Warren wrote:
create mode 100644 arch/arm/mach-tegra/include/mach/harmony_audio.h
Does this file really belong in mach-tegra if its for a driver that is in a common directory (drivers/sound/soc)? Putting it in include/linux would solve all your tree-ordering problems.
Colin,
Re: The location I chose; I followed an existing example of another ASoC driver. Perhaps there is a more appropriate location, although the ASoC maintainers didn't point one out.
I'm afraid I don't understand how moving the file would solve the dependency issues though:
There's a change in sound/soc/... that needs this new file.
There's a change in arch/arm/mach-tegra/... that needs this new file.
Both those two changes would typically get checked into two separate git repositories.
As far as I can tell, we could:
a) Check the new file into both repositories, and git will hopefully just merge them together in the next merge window. Mark pointed out a long time ago that this is generally frowned upon though.
b) Check the change into just one repository, e.g. ASoC since it's ready to accept the sound/soc/... changes that rely on it, and then have Tegra's for-next merge that (or merge linux-next after it's filtered up there, although I'm not sure if merging linux-next is workable with git?) (or merge directly from ASoC's for-2.6.39) and only then apply the arch/arm/tegra changes that rely on it. Mark pointed out a long time ago that such cross- subsystem merges are very rare, although it sounded like they do happen sometimes.
c) Only check in the sound/soc/... changes, wait until they're in 2.6.39-rc1 and hence can be picked up from there for Tegra's for-next, and then check in the Tegra changes that rely on it. This would delay the complete changes until 2.6.40:-(
I'm still in the dark how this normally works; it seems like it must be pretty common to add a new driver to the driver repository, with that driver defining a new platform_data type, and also needing to add code to the architecture/chip repository to provide that platform_data, and hopefully have both those things go in in parallel, rather than being staged across two kernel releases.
On Fri, Jan 21, 2011 at 2:35 PM, Stephen Warren swarren@nvidia.com wrote:
Colin Cross wrote on Friday, January 21, 2011 11:43 AM:
On Thu, Jan 20, 2011 at 12:52 PM, Stephen Warren wrote:
create mode 100644 arch/arm/mach-tegra/include/mach/harmony_audio.h
Does this file really belong in mach-tegra if its for a driver that is in a common directory (drivers/sound/soc)? Putting it in include/linux would solve all your tree-ordering problems.
Colin,
Re: The location I chose; I followed an existing example of another ASoC driver. Perhaps there is a more appropriate location, although the ASoC maintainers didn't point one out.
I'm afraid I don't understand how moving the file would solve the dependency issues though:
There's a change in sound/soc/... that needs this new file.
There's a change in arch/arm/mach-tegra/... that needs this new file.
Both those two changes would typically get checked into two separate git repositories.
As far as I can tell, we could:
a) Check the new file into both repositories, and git will hopefully just merge them together in the next merge window. Mark pointed out a long time ago that this is generally frowned upon though.
b) Check the change into just one repository, e.g. ASoC since it's ready to accept the sound/soc/... changes that rely on it, and then have Tegra's for-next merge that (or merge linux-next after it's filtered up there, although I'm not sure if merging linux-next is workable with git?) (or merge directly from ASoC's for-2.6.39) and only then apply the arch/arm/tegra changes that rely on it. Mark pointed out a long time ago that such cross- subsystem merges are very rare, although it sounded like they do happen sometimes.
c) Only check in the sound/soc/... changes, wait until they're in 2.6.39-rc1 and hence can be picked up from there for Tegra's for-next, and then check in the Tegra changes that rely on it. This would delay the complete changes until 2.6.40:-(
I'm still in the dark how this normally works; it seems like it must be pretty common to add a new driver to the driver repository, with that driver defining a new platform_data type, and also needing to add code to the architecture/chip repository to provide that platform_data, and hopefully have both those things go in in parallel, rather than being staged across two kernel releases.
Could the same platform_data patch go into both ASoC and Tegra? It would fall out in the merge, no matter who gets pulled first. If we go that route, whichever place you put the header, one of the trees will have an include file that is in the other tree's directory.
I still think the header makes more sense in include/sound than mach-tegra, but if the ASoC maintainers prefer it in mach-tegra, consider the patch Acked-by: Colin Cross ccross@android.com for inclusion in the ASoC tree.
On Fri, Jan 21, 2011 at 02:41:08PM -0800, Colin Cross wrote:
Could the same platform_data patch go into both ASoC and Tegra? It would fall out in the merge, no matter who gets pulled first. If we go that route, whichever place you put the header, one of the trees will have an include file that is in the other tree's directory.
The usual thing here would be a branch that gets merged into both trees (to avoid the commit getting duplicated) or to only apply in one tree and let things sort themselves out in -next and the merge window.
I still think the header makes more sense in include/sound than mach-tegra, but if the ASoC maintainers prefer it in mach-tegra, consider the patch Acked-by: Colin Cross ccross@android.com for inclusion in the ASoC tree.
The general style for architecture specific headers is that they get included in architecture specific directories to avoid general namespace pollution.
Mark Brown wrote on Sent: Friday, January 21, 2011 4:42 PM:
On Fri, Jan 21, 2011 at 02:41:08PM -0800, Colin Cross wrote:
Could the same platform_data patch go into both ASoC and Tegra? It would fall out in the merge, no matter who gets pulled first. If we go that route, whichever place you put the header, one of the trees will have an include file that is in the other tree's directory.
The usual thing here would be a branch that gets merged into both trees (to avoid the commit getting duplicated) or to only apply in one tree and let things sort themselves out in -next and the merge window.
Ah yes, I do remember you mentioning that before. Now I realize exactly what you mean.
I'm quite happy to create a branch for this, cherry-pick my change onto it, and tell you where it is, so you can merge from it.
What should I base the branch off; since it's a simple file add, I guess the current mainline tree would be fine.
I assume there are no specific requirements for where the branch is hosted; i.e. something you can git clone over HTTP from my personal server would be fine?
Thanks!
On Fri, Jan 21, 2011 at 3:49 PM, Stephen Warren swarren@nvidia.com wrote:
Mark Brown wrote on Sent: Friday, January 21, 2011 4:42 PM:
The usual thing here would be a branch that gets merged into both trees (to avoid the commit getting duplicated) or to only apply in one tree and let things sort themselves out in -next and the merge window.
Ah yes, I do remember you mentioning that before. Now I realize exactly what you mean.
Sounds overly complicated to me since you don't already have a tree hosted somewhere.
Mark; why don't you apply it with Colin's acked-by, and Colin can just cherry-pick from that for-next branch?
-Olof
Olof Johansson wrote on Friday, January 21, 2011 10:15 PM:
On Fri, Jan 21, 2011 at 3:49 PM, Stephen Warren wrote:
Mark Brown wrote on Sent: Friday, January 21, 2011 4:42 PM:
The usual thing here would be a branch that gets merged into both trees (to avoid the commit getting duplicated) or to only apply in one tree and let things sort themselves out in -next and the merge window.
Ah yes, I do remember you mentioning that before. Now I realize exactly what you mean.
Sounds overly complicated to me since you don't already have a tree hosted somewhere.
Well, I did push a kernel repo to my personal server, which would do fine for this purpose assuming there's no requirement for it to stick around for any specific amount of time etc.
On Fri, Jan 21, 2011 at 9:34 PM, Stephen Warren swarren@nvidia.com wrote:
Olof Johansson wrote on Friday, January 21, 2011 10:15 PM:
On Fri, Jan 21, 2011 at 3:49 PM, Stephen Warren wrote:
Mark Brown wrote on Sent: Friday, January 21, 2011 4:42 PM:
The usual thing here would be a branch that gets merged into both trees (to avoid the commit getting duplicated) or to only apply in one tree and let things sort themselves out in -next and the merge window.
Ah yes, I do remember you mentioning that before. Now I realize exactly what you mean.
Sounds overly complicated to me since you don't already have a tree hosted somewhere.
Well, I did push a kernel repo to my personal server, which would do fine for this purpose assuming there's no requirement for it to stick around for any specific amount of time etc.
Ok, that works too :)
-Olof
The WM8903 driver now needs platform data in order to correctly expose its GPIOs through gpiolib.
The top-level ASoC driver now needs a platform device in order to provide infra-structure to pass in platform data.
The top-level ASoC driver now needs platform data in order to be informed which GPIO IDs to use.
This is all needed to allow the top-level ASoC driver to configure the WM8903's GPIO3 as an output GPIO, and enable it as appropriate. This is the enable signal for the external speaker amplifier chips.
Signed-off-by: Stephen Warren swarren@nvidia.com --- arch/arm/mach-tegra/board-harmony.c | 31 +++++++++++++++++++++++++++++++ arch/arm/mach-tegra/gpio-names.h | 2 ++ 2 files changed, 33 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-tegra/board-harmony.c b/arch/arm/mach-tegra/board-harmony.c index 2696a20..c5a4ae0 100644 --- a/arch/arm/mach-tegra/board-harmony.c +++ b/arch/arm/mach-tegra/board-harmony.c @@ -2,6 +2,7 @@ * arch/arm/mach-tegra/board-harmony.c * * Copyright (C) 2010 Google, Inc. + * Copyright (C) 2011 NVIDIA, Inc. * * This software is licensed under the terms of the GNU General Public * License version 2, as published by the Free Software Foundation, and @@ -24,12 +25,15 @@ #include <linux/i2c.h> #include <linux/io.h>
+#include <sound/wm8903.h> + #include <asm/mach-types.h> #include <asm/mach/arch.h> #include <asm/mach/time.h> #include <asm/setup.h>
#include <mach/gpio.h> +#include <mach/harmony_audio.h> #include <mach/i2c.h> #include <mach/iomap.h> #include <mach/irqs.h> @@ -53,6 +57,8 @@ #define ATAG_NVIDIA_PRESERVED_MEM_N 2 #define ATAG_NVIDIA_FORCE_32 0x7fffffff
+#define GPIO_WM8903(_x_) (TEGRA_MAX_GPIO + 1 + (_x_)) + struct tag_tegra { __u32 bootarg_key; __u32 bootarg_len; @@ -112,9 +118,33 @@ static struct tegra_i2c_platform_data harmony_i2c1_platform_data = { .bus_clk_rate = { 400000, 0 }, };
+static struct wm8903_platform_data harmony_wm8903_pdata = { + .gpio_base = GPIO_WM8903(0), + .gpio_cfg = { + WM8903_GPIO_NO_CONFIG, + WM8903_GPIO_NO_CONFIG, + 0, + WM8903_GPIO_NO_CONFIG, + WM8903_GPIO_NO_CONFIG, + }, +}; + static struct i2c_board_info __initdata harmony_i2c_bus1_board_info[] = { { I2C_BOARD_INFO("wm8903", 0x1a), + .platform_data = &harmony_wm8903_pdata, + }, +}; + +static struct harmony_audio_platform_data harmony_audio_pdata = { + .gpio_spkr_en = GPIO_WM8903(2), +}; + +static struct platform_device harmony_audio_device = { + .name = "tegra-snd-harmony", + .id = 0, + .dev = { + .platform_data = &harmony_audio_pdata, }, };
@@ -127,6 +157,7 @@ static struct platform_device *harmony_devices[] __initdata = { &tegra_i2s_device1, &tegra_das_device, &tegra_pcm_device, + &harmony_audio_device, };
static void __init tegra_harmony_fixup(struct machine_desc *desc, diff --git a/arch/arm/mach-tegra/gpio-names.h b/arch/arm/mach-tegra/gpio-names.h index f28220a..e8b5cf3 100644 --- a/arch/arm/mach-tegra/gpio-names.h +++ b/arch/arm/mach-tegra/gpio-names.h @@ -244,4 +244,6 @@ #define TEGRA_GPIO_PBB6 222 #define TEGRA_GPIO_PBB7 223
+#define TEGRA_MAX_GPIO TEGRA_GPIO_PBB7 + #endif
On Thu, Jan 20, 2011 at 01:52:10PM -0700, Stephen Warren wrote:
The WM8903 driver now needs platform data in order to correctly expose its GPIOs through gpiolib.
This isn't really a fix - it's enabling a new feature that wasn't previously supported, describing it as a fix is misleading as that'd suggest it should be applied to the release branch as a bugfix which would cause build issues.
I'm happy with this version of the series, I guess Liam will be too. Ideally someone from the Tegra side should sign off on the arch/arm side - that'll need to go through the same tree as the ASoC side for build dependencies.
On Thu, 2011-01-20 at 21:22 +0000, Mark Brown wrote:
On Thu, Jan 20, 2011 at 01:52:10PM -0700, Stephen Warren wrote:
The WM8903 driver now needs platform data in order to correctly expose its GPIOs through gpiolib.
This isn't really a fix - it's enabling a new feature that wasn't previously supported, describing it as a fix is misleading as that'd suggest it should be applied to the release branch as a bugfix which would cause build issues.
I'm happy with this version of the series, I guess Liam will be too. Ideally someone from the Tegra side should sign off on the arch/arm side - that'll need to go through the same tree as the ASoC side for build dependencies.
I'm happy with this too.
Acked-by: Liam girdwood lrg@slimlogic.co.uk
[I apologize if the email formatting is terrible; my usual system is broken right now. I hope this is plain text]
Mark Brown wrote on Thursday, January 20, 2011 14:22:
On Thu, Jan 20, 2011 at 01:52:10PM -0700, Stephen Warren wrote:
The WM8903 driver now needs platform data in order to correctly expose its GPIOs through gpiolib.
This isn't really a fix - it's enabling a new feature that wasn't previously supported
Agreed. I'll reword this when I submit it for inclusion in the Tegra tree.
I'm happy with this version of the series, I guess Liam will be too. Ideally someone from the Tegra side should sign off on the arch/arm side - that'll need to go through the same tree as the ASoC side for build dependencies.
I assume you're only talking about:
a882c75eadfcdc56f47962dd3f7870d6def62847 ARM: tegra: Add Harmony sound platform data type
The other arch/arm patches won't apply anywhere yet since they build on top of various other patches that haven't been submitted for or applied to the linux-tegra tree yet.
CC += Colin/Erik to ack/nack the header addition mentioned above.
Thanks
On Fri, Jan 21, 2011 at 09:45:56AM -0800, Stephen Warren wrote:
I'm happy with this version of the series, I guess Liam will be too. Ideally someone from the Tegra side should sign off on the arch/arm side - that'll need to go through the same tree as the ASoC side for build dependencies.
I assume you're only talking about:
a882c75eadfcdc56f47962dd3f7870d6def62847 ARM: tegra: Add Harmony sound platform data type
That and the addition of the defines for function codes IIRC.
The other arch/arm patches won't apply anywhere yet since they build on top of various other patches that haven't been submitted for or applied to the linux-tegra tree yet.
Oh, I see. I hadn't realised there wer eother dependencies, please ensure you mention any such dependencies in the changlogs so we avoid introducing breakage.
Mark Brown wrote on Friday, January 21, 2011 10:50:
On Fri, Jan 21, 2011 at 09:45:56AM -0800, Stephen Warren wrote:
I'm happy with this version of the series, I guess Liam will be too. Ideally someone from the Tegra side should sign off on the arch/arm side - that'll need to go through the same tree as the ASoC side for build dependencies.
I assume you're only talking about:
a882c75eadfcdc56f47962dd3f7870d6def62847 ARM: tegra: Add Harmony sound platform data type
That and the addition of the defines for function codes IIRC.
The wm8903 GPIO function defines? They're in wm8903.h and don't rely on anything else. The arch/arm patches don't rely on these defines.
The other arch/arm patches won't apply anywhere yet since they build on top of various other patches that haven't been submitted for or applied to the linux-tegra tree yet.
Oh, I see. I hadn't realised there wer eother dependencies, please ensure you mention any such dependencies in the changlogs so we avoid introducing breakage.
That was the intent of what I wrote in the "cover letter" "patch".
To be explicit, the only dependency is that the patch quoted above, which adds some platform data types, needs to be checked in to te ASoC tree rather than the usual ARM tree, before the following ASoC patch is applied:
ASoC: tegra: Harmony: Support th internal speaker
Every other ASoC patch I've submitted doesn't rely on anything outside the ASoC tree to compile, although actually running it usefully obviously requires the arch/arm changes.
Hope this clears things up!
-- nvpublic
On Fri, Jan 21, 2011 at 10:06:33AM -0800, Stephen Warren wrote:
Mark Brown wrote on Friday, January 21, 2011 10:50:
Oh, I see. I hadn't realised there wer eother dependencies, please ensure you mention any such dependencies in the changlogs so we avoid introducing breakage.
That was the intent of what I wrote in the "cover letter" "patch".
It doesn't refer to anything outside the series itself and the current state of the relevant trees, which seemed to be what you were saying...
It'd also be helpful if you could stop burying patches in old threads - it makes it hard to find the most recent stuff.
To be explicit, the only dependency is that the patch quoted above, which adds some platform data types, needs to be checked in to te ASoC tree rather than the usual ARM tree, before the following ASoC patch is applied:
...but possibly not?
Mark Brown wrote:
It'd also be helpful if you could stop burying patches in old threads - it makes it hard to find the most recent stuff.
Sorry. I read somewhere that threading updates into the existing thread was the preferred way. I'll stop doing it.
To be explicit, the only dependency is that the patch quoted above, which adds some platform data types, needs to be checked in to te ASoC tree rather than the usual ARM tree, before the following ASoC patch is applied:
...but possibly not?
Sorry, I don't follow. Possibly not what? Are you saying:
a) Possibly not the only dependency? It's certainly the only one I know of.
b) Possibly not that it needs to be checked into the ASoC git repo, even though the header is located somewhere the Tegra git repo owns?
I'm fine moving the header into some directory that ASoC owns, and fixing my arch/arm/mach-tegra patch for the new location. That would mean that when I submit the Tegra changes to the Tegra tree, the Tegra trees needs to have picked up the ASoC changes first, but that's fine I think? At the least, it defers any issue until later.
Otherwise, yes, that patch is needed befoer the ASoC patch that uses it, or there will be a build break, if anyone enables building that driver.
On Fri, Jan 21, 2011 at 10:22:11AM -0800, Stephen Warren wrote:
Mark Brown wrote:
Sorry, I don't follow. Possibly not what? Are you saying:
a) Possibly not the only dependency? It's certainly the only one I know of.
b) Possibly not that it needs to be checked into the ASoC git repo, even though the header is located somewhere the Tegra git repo owns?
You said in 74CDBE0F657A3D45AFBB94109FB122FF0310AD4251@HQMAIL01.nvidia.com:
| The other arch/arm patches won't apply anywhere yet since they build | on top of various other patches that haven't been submitted for or | applied to the linux-tegra tree yet.
referring to patches 2 and 3 of the series. To me that says that these patches won't apply to either the Tegra or ASoC trees as they stand as they depend on patches you have not yet submitted.
Mark Brown wrote:
Sorry, I don't follow. Possibly not what? Are you saying:
a) Possibly not the only dependency? It's certainly the only one I know of.
b) Possibly not that it needs to be checked into the ASoC git repo, even though the header is located somewhere the Tegra git repo owns?
You said in 74CDBE0F657A3D45AFBB94109FB122FF0310AD4251@HQMAIL01.nvidia.com:
| The other arch/arm patches won't apply anywhere yet since they build | on top of various other patches that haven't been submitted for or | applied to the linux-tegra tree yet.
referring to patches 2 and 3 of the series. To me that says that these patches won't apply to either the Tegra or ASoC trees as they stand as they depend on patches you have not yet submitted.
Patch 1 (wm8903 GPIO support) has no dependencies.
Patch 2 (add header to define Harmony ASoC driver's platform_data) doesn't depend on anything else, but is required by patches 3 and 4.
This is the patch which is in a path owned by the Tegra tree, but needs to be checked into the ASoC tree to enable patch 4 to be checked in there.
Patch 3 (Adds various platform data to arch/arm/mach-tegra) depends on patch 2, and should be checked into the Tegra for-next tree at some later time. You can ignore this patch; I presented it simply to show a complete set of patches. Sorry if the cover letter wasn't clear on this point.
Patch 4 (Adds internal speaker support to ASoC driver) depends on patch 2 being in the ASoC tree somehow. Hence, why you asked the Tegra maintainers to sign off on checking patch 2 into the ASoC tree, as I understand it.
On Fri, Jan 21, 2011 at 10:36:42AM -0800, Stephen Warren wrote:
Patch 3 (Adds various platform data to arch/arm/mach-tegra) depends on patch 2, and should be checked into the Tegra for-next tree at some later time. You can ignore this patch; I presented it simply to show a complete set of patches. Sorry if the cover letter wasn't clear on this point.
I've no problem carrying that in ASoC if Colin is happy with it, it seems much easier to do that than to have to remember to apply it to the Tegra tree after the 2.6.39 merge window.
Patch 4 (Adds internal speaker support to ASoC driver) depends on patch 2 being in the ASoC tree somehow. Hence, why you asked the Tegra maintainers to sign off on checking patch 2 into the ASoC tree, as I understand it.
Yes. It was your subsequent reply about stuff not having been submitted that made me think that there were still more dependencies I wasn't aware of.
Mark Brown wrote:
On Fri, Jan 21, 2011 at 10:36:42AM -0800, Stephen Warren wrote:
Patch 3 (Adds various platform data to arch/arm/mach-tegra) depends on patch 2, and should be checked into the Tegra for-next tree at some later time. You can ignore this patch; I presented it simply to show a complete set of patches. Sorry if the cover letter wasn't clear on this point.
I've no problem carrying that in ASoC if Colin is happy with it, it seems much easier to do that than to have to remember to apply it to the Tegra tree after the 2.6.39 merge window.
I'd like to get everything, or at least most of this, into 2.6.39 if at all possible. I guess getting just the basic driver (i.e. before this addition of internal speaker support) would be OK though, since none of the patches for the basic driver have any cross-subsystem dependencies.
For the basic driver: Everything is already in the ASoC tree. I'll be pushing patches to support it (i.e. add the required platform_data, and possibly various Tegra infra-structure such as clock configuration additions and fixes etc.) to the Tegra for-next tree once it's open to accept patches:
The problem is that the Tegra for-next branch isn't yet open for submissions, (I was originally directed to provide patches against linux-tegra-2.6.36 and just recently directed to provided patches against linux-tegra-2.6.37) so I've been queueing up all the arch/arm-mach-tegra patches for submission once Tegra for-next opens. Colin and Erik have said offline that this will be soon.
Hence, the arch/arm-mach-tegra changes in this patchset (excluding the simple standalone addition of that one new header) build on code that is neither in ASoC's nor Tegra's for-next branch. Hence, you won't be able to carry that patch yourself since it won't apply, even with manual merging I suspect.
Sorry the situation is so confusing. I hope this explains some of the background.
On Fri, Jan 21, 2011 at 10:51:15AM -0800, Stephen Warren wrote:
Hence, the arch/arm-mach-tegra changes in this patchset (excluding the simple standalone addition of that one new header) build on code that is neither in ASoC's nor Tegra's for-next branch. Hence, you won't be able to carry that patch yourself since it won't apply, even with manual merging I suspect.
So just to be absolutely clear, there really are external dependencies for the arch/arm bits of this patch series that go beyond the patch series itself? That's the information I was asking to be flagged on future submissions.
Mark Brown wrote on Friday, January 21, 2011 11:58 AM:
On Fri, Jan 21, 2011 at 10:51:15AM -0800, Stephen Warren wrote:
Hence, the arch/arm-mach-tegra changes in this patchset (excluding the simple standalone addition of that one new header) build on code that is neither in ASoC's nor Tegra's for-next branch. Hence, you won't be able to carry that patch yourself since it won't apply, even with manual merging I suspect.
So just to be absolutely clear, there really are external dependencies for the arch/arm bits of this patch series that go beyond the patch series itself? That's the information I was asking to be flagged on future submissions.
Yes, the arch/arm bits (except that standalone patch to add the new header) depend on a variety of other patches from before this patchset. Those other patches fix a few missing things in the mainline Tegra support and/or simply add platform devices and platform data for the new ASoC driver. At least some of those patches will have to be reworked a little once Tegra's for-next comes on line.
The sound/soc bits don't depend on anything except what's already in the ACoC for-2.6.39 branch.
Add DAPM widget definitions, and code to control the speaker amplifier GPIO to the Harmony machine driver. Currently, this path is always enabled, while playback is active, since jack detection isn't yet implemented.
The driver initialization path has been reworked a little, and a new driver/device introduced, in order to allow platform data to be passed in, currently containing just the speaker enable GPIO ID.
The GPIO is only requested during _init, since that's the first time it is guaranteed that the WM8903 module is loaded, probed, and hence has exported its GPIO chip.
Signed-off-by: Stephen Warren swarren@nvidia.com --- sound/soc/tegra/harmony.c | 87 +++++++++++++++++++++++++++++++++++++++------ 1 files changed, 76 insertions(+), 11 deletions(-)
diff --git a/sound/soc/tegra/harmony.c b/sound/soc/tegra/harmony.c index b160b71..e424997 100644 --- a/sound/soc/tegra/harmony.c +++ b/sound/soc/tegra/harmony.c @@ -2,7 +2,7 @@ * harmony.c - Harmony machine ASoC driver * * Author: Stephen Warren swarren@nvidia.com - * Copyright (C) 2010 - NVIDIA, Inc. + * Copyright (C) 2010-2011 - NVIDIA, Inc. * * Based on code copyright/by: * @@ -29,7 +29,14 @@ */
#include <asm/mach-types.h> + +#include <linux/gpio.h> #include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + +#include <mach/harmony_audio.h> + #include <sound/core.h> #include <sound/pcm.h> #include <sound/pcm_params.h> @@ -40,9 +47,12 @@ #include "tegra_pcm.h" #include "tegra_asoc_utils.h"
-#define PREFIX "ASoC Harmony: " +#define DRV_NAME "tegra-snd-harmony" +#define PREFIX DRV_NAME ": "
+static struct harmony_audio_platform_data *pdata; static struct platform_device *harmony_snd_device; +static int gpio_spkr_en_requested;
static int harmony_asoc_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) @@ -107,7 +117,17 @@ static struct snd_soc_ops harmony_asoc_ops = { .hw_params = harmony_asoc_hw_params, };
+static int harmony_event_int_spk(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *k, int event) +{ + gpio_set_value_cansleep(pdata->gpio_spkr_en, + !!SND_SOC_DAPM_EVENT_ON(event)); + + return 0; +} + static const struct snd_soc_dapm_widget harmony_dapm_widgets[] = { + SND_SOC_DAPM_SPK("Int Spk", harmony_event_int_spk), SND_SOC_DAPM_HP("Headphone Jack", NULL), SND_SOC_DAPM_MIC("Mic Jack", NULL), }; @@ -115,6 +135,10 @@ static const struct snd_soc_dapm_widget harmony_dapm_widgets[] = { static const struct snd_soc_dapm_route harmony_audio_map[] = { {"Headphone Jack", NULL, "HPOUTR"}, {"Headphone Jack", NULL, "HPOUTL"}, + {"Int Spk", NULL, "ROP"}, + {"Int Spk", NULL, "RON"}, + {"Int Spk", NULL, "LOP"}, + {"Int Spk", NULL, "LON"}, {"Mic Bias", NULL, "Mic Jack"}, {"IN1L", NULL, "Mic Bias"}, }; @@ -123,6 +147,7 @@ static int harmony_asoc_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_codec *codec = rtd->codec; struct snd_soc_dapm_context *dapm = &codec->dapm; + int ret;
snd_soc_dapm_new_controls(dapm, harmony_dapm_widgets, ARRAY_SIZE(harmony_dapm_widgets)); @@ -131,9 +156,19 @@ static int harmony_asoc_init(struct snd_soc_pcm_runtime *rtd) ARRAY_SIZE(harmony_audio_map));
snd_soc_dapm_enable_pin(dapm, "Headphone Jack"); + snd_soc_dapm_enable_pin(dapm, "Int Spk"); snd_soc_dapm_enable_pin(dapm, "Mic Jack"); snd_soc_dapm_sync(dapm);
+ ret = gpio_request(pdata->gpio_spkr_en, "spkr_en"); + if (ret) { + pr_err(PREFIX "cannot get spkr_en gpio\n"); + return ret; + } + gpio_spkr_en_requested = 1; + + gpio_direction_output(pdata->gpio_spkr_en, 0); + return 0; }
@@ -154,13 +189,19 @@ static struct snd_soc_card snd_soc_harmony = { .num_links = 1, };
-static int __init harmony_soc_modinit(void) +static __devinit int tegra_snd_harmony_probe(struct platform_device *pdev) { int ret;
- if (!machine_is_harmony()) { - pr_err(PREFIX "Not running on Tegra Harmony!\n"); - return -ENODEV; + if (pdev->id != 0) { + dev_err(&pdev->dev, "ID %d out of range\n", pdev->id); + return -EINVAL; + } + + pdata = pdev->dev.platform_data; + if (!pdata) { + dev_err(&pdev->dev, "no platform data supplied\n"); + return -EINVAL; }
ret = tegra_asoc_utils_init(); @@ -173,7 +214,7 @@ static int __init harmony_soc_modinit(void) */ harmony_snd_device = platform_device_alloc("soc-audio", -1); if (harmony_snd_device == NULL) { - pr_err(PREFIX "platform_device_alloc failed\n"); + dev_err(&pdev->dev, "platform_device_alloc failed\n"); ret = -ENOMEM; goto err_clock_utils; } @@ -182,7 +223,7 @@ static int __init harmony_soc_modinit(void)
ret = platform_device_add(harmony_snd_device); if (ret) { - pr_err(PREFIX "platform_device_add failed (%d)\n", + dev_err(&pdev->dev, "platform_device_add failed (%d)\n", ret); goto err_device_put; } @@ -195,15 +236,39 @@ err_clock_utils: tegra_asoc_utils_fini(); return ret; } -module_init(harmony_soc_modinit);
-static void __exit harmony_soc_modexit(void) +static int __devexit tegra_snd_harmony_remove(struct platform_device *pdev) { platform_device_unregister(harmony_snd_device);
+ if (gpio_spkr_en_requested) + gpio_free(pdata->gpio_spkr_en); + tegra_asoc_utils_fini(); + + return 0; +} + +static struct platform_driver tegra_snd_harmony_driver = { + .driver = { + .name = DRV_NAME, + .owner = THIS_MODULE, + }, + .probe = tegra_snd_harmony_probe, + .remove = __devexit_p(tegra_snd_harmony_remove), +}; + +static int __init snd_tegra_harmony_init(void) +{ + return platform_driver_register(&tegra_snd_harmony_driver); +} +module_init(snd_tegra_harmony_init); + +static void __exit snd_tegra_harmony_exit(void) +{ + platform_driver_unregister(&tegra_snd_harmony_driver); } -module_exit(harmony_soc_modexit); +module_exit(snd_tegra_harmony_exit);
MODULE_AUTHOR("Stephen Warren swarren@nvidia.com"); MODULE_DESCRIPTION("Harmony machine ASoC driver");
participants (5)
-
Colin Cross -
Liam Girdwood -
Mark Brown -
Olof Johansson -
Stephen Warren