[alsa-devel] [PATCH 0/5] DT support for N900 soundcard (rx51-audio)
Hi,
This patchset adds DT support in rx51-audio. I tested it on my Nokia N900 and was able to play a wav file using aplay. I have not tested the whole functionality, but output via speakers, headphones and the related enable controls seem to work.
The patches can be applied cleanly to torvalds/linux.git:master and broonie/sound.git:for-next.
-- Sebastian
Pali Rohár (1): ASoC: omap: rx51: Use devm_snd_soc_register_card
Sebastian Reichel (4): ASoC: Allow Aux Codecs to be specified using DT ASoC: RX-51: Add DT support to sound driver ASoC: RX-51: Convert to table based DAPM setup ASoC: tlv320aic3x: fix shared reset pin for DT
.../devicetree/bindings/sound/nokia,rx51.txt | 27 +++ include/sound/rx51.h | 18 ++ include/sound/soc.h | 13 +- sound/soc/codecs/tlv320aic3x.c | 9 +- sound/soc/omap/omap-mcbsp.c | 5 +- sound/soc/omap/omap-mcbsp.h | 2 +- sound/soc/omap/rx51.c | 248 +++++++++++++-------- sound/soc/soc-core.c | 68 +++--- 8 files changed, 266 insertions(+), 124 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/nokia,rx51.txt create mode 100644 include/sound/rx51.h
From: Pali Rohár pali.rohar@gmail.com
This patch converts the rx51 ASoC module to use devm_snd_soc_register_card. It also adds module alias to support driver autoloading.
Signed-off-by: Pali Rohár pali.rohar@gmail.com Signed-off-by: Sebastian Reichel sre@kernel.org --- sound/soc/omap/rx51.c | 48 ++++++++++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c index 7fb3d4b..58f5e12 100644 --- a/sound/soc/omap/rx51.c +++ b/sound/soc/omap/rx51.c @@ -394,10 +394,9 @@ static struct snd_soc_card rx51_sound_card = { .num_configs = ARRAY_SIZE(rx51_codec_conf), };
-static struct platform_device *rx51_snd_device; - -static int __init rx51_soc_init(void) +static int rx51_soc_probe(struct platform_device *pdev) { + struct snd_soc_card *card = &rx51_sound_card; int err;
if (!machine_is_nokia_rx51() && !of_machine_is_compatible("nokia,omap3-n900")) @@ -412,22 +411,17 @@ static int __init rx51_soc_init(void) if (err) goto err_gpio_eci_sw;
- rx51_snd_device = platform_device_alloc("soc-audio", -1); - if (!rx51_snd_device) { - err = -ENOMEM; - goto err1; - } - - platform_set_drvdata(rx51_snd_device, &rx51_sound_card); + card->dev = &pdev->dev;
- err = platform_device_add(rx51_snd_device); - if (err) - goto err2; + err = devm_snd_soc_register_card(card->dev, card); + if (err) { + dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", err); + goto err_snd; + }
return 0; -err2: - platform_device_put(rx51_snd_device); -err1: +err_snd: + card->dev = NULL; gpio_free(RX51_ECI_SW_GPIO); err_gpio_eci_sw: gpio_free(RX51_TVOUT_SEL_GPIO); @@ -436,19 +430,33 @@ err_gpio_tvout_sel: return err; }
-static void __exit rx51_soc_exit(void) +static int rx51_soc_remove(struct platform_device *pdev) { + struct snd_soc_card *card = platform_get_drvdata(pdev); + snd_soc_jack_free_gpios(&rx51_av_jack, ARRAY_SIZE(rx51_av_jack_gpios), rx51_av_jack_gpios);
- platform_device_unregister(rx51_snd_device); + card->dev = NULL; + gpio_free(RX51_ECI_SW_GPIO); gpio_free(RX51_TVOUT_SEL_GPIO); + + return 0; }
-module_init(rx51_soc_init); -module_exit(rx51_soc_exit); +static struct platform_driver rx51_soc_driver = { + .driver = { + .name = "rx51-audio", + .owner = THIS_MODULE, + }, + .probe = rx51_soc_probe, + .remove = rx51_soc_remove, +}; + +module_platform_driver(rx51_soc_driver);
MODULE_AUTHOR("Nokia Corporation"); MODULE_DESCRIPTION("ALSA SoC Nokia RX-51"); MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:rx51-audio");
On Sat, Apr 05, 2014 at 11:35:49PM +0200, Sebastian Reichel wrote:
From: Pali Rohár pali.rohar@gmail.com
This patch converts the rx51 ASoC module to use devm_snd_soc_register_card. It also adds module alias to support driver autoloading.
This doesn't apply against current code and since it does multiple things it should be multiple patches.
+static int rx51_soc_remove(struct platform_device *pdev) {
- struct snd_soc_card *card = platform_get_drvdata(pdev);
- snd_soc_jack_free_gpios(&rx51_av_jack, ARRAY_SIZE(rx51_av_jack_gpios), rx51_av_jack_gpios);
- platform_device_unregister(rx51_snd_device);
- card->dev = NULL;
Why is this assignment being done?
On 4/6/2014 3:05 AM, Sebastian Reichel wrote:
From: Pali Rohár pali.rohar@gmail.com
This patch converts the rx51 ASoC module to use devm_snd_soc_register_card. It also adds module alias to support driver autoloading.
Signed-off-by: Pali Rohár pali.rohar@gmail.com Signed-off-by: Sebastian Reichel sre@kernel.org
sound/soc/omap/rx51.c | 48 ++++++++++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c index 7fb3d4b..58f5e12 100644 --- a/sound/soc/omap/rx51.c +++ b/sound/soc/omap/rx51.c @@ -394,10 +394,9 @@ static struct snd_soc_card rx51_sound_card = { .num_configs = ARRAY_SIZE(rx51_codec_conf), };
-static struct platform_device *rx51_snd_device;
-static int __init rx51_soc_init(void) +static int rx51_soc_probe(struct platform_device *pdev) {
struct snd_soc_card *card = &rx51_sound_card; int err;
if (!machine_is_nokia_rx51() && !of_machine_is_compatible("nokia,omap3-n900"))
@@ -412,22 +411,17 @@ static int __init rx51_soc_init(void) if (err) goto err_gpio_eci_sw;
- rx51_snd_device = platform_device_alloc("soc-audio", -1);
- if (!rx51_snd_device) {
err = -ENOMEM;
goto err1;
- }
- platform_set_drvdata(rx51_snd_device, &rx51_sound_card);
- card->dev = &pdev->dev;
- err = platform_device_add(rx51_snd_device);
- if (err)
goto err2;
err = devm_snd_soc_register_card(card->dev, card);
if (err) {
dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", err);
goto err_snd;
}
return 0;
-err2:
- platform_device_put(rx51_snd_device);
-err1: +err_snd:
- card->dev = NULL;
Why this is required???
~Rajeev
gpio_free(RX51_ECI_SW_GPIO); err_gpio_eci_sw: gpio_free(RX51_TVOUT_SEL_GPIO); @@ -436,19 +430,33 @@ err_gpio_tvout_sel: return err; }
-static void __exit rx51_soc_exit(void) +static int rx51_soc_remove(struct platform_device *pdev) {
- struct snd_soc_card *card = platform_get_drvdata(pdev);
- snd_soc_jack_free_gpios(&rx51_av_jack, ARRAY_SIZE(rx51_av_jack_gpios), rx51_av_jack_gpios);
- platform_device_unregister(rx51_snd_device);
- card->dev = NULL;
- gpio_free(RX51_ECI_SW_GPIO); gpio_free(RX51_TVOUT_SEL_GPIO);
- return 0; }
-module_init(rx51_soc_init); -module_exit(rx51_soc_exit); +static struct platform_driver rx51_soc_driver = {
- .driver = {
.name = "rx51-audio",
.owner = THIS_MODULE,
- },
- .probe = rx51_soc_probe,
- .remove = rx51_soc_remove,
+};
+module_platform_driver(rx51_soc_driver);
MODULE_AUTHOR("Nokia Corporation"); MODULE_DESCRIPTION("ALSA SoC Nokia RX-51"); MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:rx51-audio");
This patch adds support for specifying auxiliary codecs and codec configuration via device tree phandles.
This change adds new fields to snd_soc_aux_dev and snd_soc_codec_conf and adds support for the changes to SoC core methods.
Signed-off-by: Sebastian Reichel sre@kernel.org Signed-off-by: Pavel Machek pavel@ucw.cz --- include/sound/soc.h | 13 +++++++++- sound/soc/soc-core.c | 68 ++++++++++++++++++++++++++++++++-------------------- 2 files changed, 54 insertions(+), 27 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 0b83168..d371ae1 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -931,7 +931,12 @@ struct snd_soc_dai_link { };
struct snd_soc_codec_conf { + /* + * specify device either by device name, or by + * DT/OF node, but not both. + */ const char *dev_name; + const struct device_node *of_node;
/* * optional map of kcontrol, widget and path name prefixes that are @@ -942,7 +947,13 @@ struct snd_soc_codec_conf {
struct snd_soc_aux_dev { const char *name; /* Codec name */ - const char *codec_name; /* for multi-codec */ + + /* + * specify multi-codec either by device name, or by + * DT/OF node, but not both. + */ + const char *codec_name; + const struct device_node *codec_of_node;
/* codec/machine specific init - e.g. add machine controls */ int (*init)(struct snd_soc_dapm_context *dapm); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 051c006..448a607 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1104,10 +1104,12 @@ static void soc_set_name_prefix(struct snd_soc_card *card,
for (i = 0; i < card->num_configs; i++) { struct snd_soc_codec_conf *map = &card->codec_conf[i]; - if (map->dev_name && !strcmp(codec->name, map->dev_name)) { - codec->name_prefix = map->name_prefix; - break; - } + if (map->of_node && codec->dev->of_node != map->of_node) + continue; + if (map->dev_name && strcmp(codec->name, map->dev_name)) + continue; + codec->name_prefix = map->name_prefix; + break; } }
@@ -1541,52 +1543,66 @@ static void soc_unregister_ac97_dai_link(struct snd_soc_codec *codec) } #endif
-static int soc_check_aux_dev(struct snd_soc_card *card, int num) +struct snd_soc_codec *soc_find_matching_codec(struct snd_soc_card *card, int num) { struct snd_soc_aux_dev *aux_dev = &card->aux_dev[num]; struct snd_soc_codec *codec;
- /* find CODEC from registered CODECs*/ + /* find CODEC from registered CODECs */ list_for_each_entry(codec, &codec_list, list) { - if (!strcmp(codec->name, aux_dev->codec_name)) - return 0; + if (aux_dev->codec_of_node && + (codec->dev->of_node != aux_dev->codec_of_node)) + continue; + if (aux_dev->codec_name && strcmp(codec->name, aux_dev->codec_name)) + continue; + return codec; }
- dev_err(card->dev, "ASoC: %s not registered\n", aux_dev->codec_name); + return NULL; +} + +static int soc_check_aux_dev(struct snd_soc_card *card, int num) +{ + struct snd_soc_aux_dev *aux_dev = &card->aux_dev[num]; + const char *codecname = aux_dev->codec_name; + struct snd_soc_codec *codec = soc_find_matching_codec(card, num);
+ if (codec) + return 0; + if (aux_dev->codec_of_node) + codecname = of_node_full_name(aux_dev->codec_of_node); + + dev_err(card->dev, "ASoC: %s not registered\n", codecname); return -EPROBE_DEFER; }
static int soc_probe_aux_dev(struct snd_soc_card *card, int num) { struct snd_soc_aux_dev *aux_dev = &card->aux_dev[num]; - struct snd_soc_codec *codec; + const char *codecname = aux_dev->codec_name; int ret = -ENODEV; + struct snd_soc_codec *codec = soc_find_matching_codec(card, num);
- /* find CODEC from registered CODECs*/ - list_for_each_entry(codec, &codec_list, list) { - if (!strcmp(codec->name, aux_dev->codec_name)) { - if (codec->probed) { - dev_err(codec->dev, - "ASoC: codec already probed"); - ret = -EBUSY; - goto out; - } - goto found; - } + if (!codec) { + if (aux_dev->codec_of_node) + codecname = of_node_full_name(aux_dev->codec_of_node); + + /* codec not found */ + dev_err(card->dev, "ASoC: codec %s not found", codecname); + return -EPROBE_DEFER; + } + + if (codec->probed) { + dev_err(codec->dev, "ASoC: codec already probed"); + return -EBUSY; } - /* codec not found */ - dev_err(card->dev, "ASoC: codec %s not found", aux_dev->codec_name); - return -EPROBE_DEFER;
-found: ret = soc_probe_codec(card, codec); if (ret < 0) return ret;
ret = soc_post_component_init(card, codec, num, 1);
-out: return ret; }
On Sat, Apr 05, 2014 at 11:35:50PM +0200, Sebastian Reichel wrote:
This patch adds support for specifying auxiliary codecs and codec configuration via device tree phandles.
This change adds new fields to snd_soc_aux_dev and snd_soc_codec_conf and adds support for the changes to SoC core methods.
What binding is supposed to be implemented by this?
This patch adds device tree support to the Nokia N900 audio driver. It also removes GPIO defines and gets them from platform data / device tree, since some GPIO numbers may be different with DT boot.
The binding also changes a helper function in omap-mcbsp, which is currently only used by the Nokia N900. This change is needed, because DT instanced omap-mcbsp driver has no valid id assigned.
Last but not least the DT binding is documented.
Signed-off-by: Sebastian Reichel sre@kernel.org --- .../devicetree/bindings/sound/nokia,rx51.txt | 27 ++++ include/sound/rx51.h | 18 +++ sound/soc/omap/omap-mcbsp.c | 5 +- sound/soc/omap/omap-mcbsp.h | 2 +- sound/soc/omap/rx51.c | 176 ++++++++++++++++----- 5 files changed, 183 insertions(+), 45 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/nokia,rx51.txt create mode 100644 include/sound/rx51.h
diff --git a/Documentation/devicetree/bindings/sound/nokia,rx51.txt b/Documentation/devicetree/bindings/sound/nokia,rx51.txt new file mode 100644 index 0000000..72f93d9 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/nokia,rx51.txt @@ -0,0 +1,27 @@ +* Nokia N900 audio setup + +Required properties: +- compatible: Should contain "nokia,n900-audio" +- nokia,cpu-dai: phandle for the McBSP node +- nokia,audio-codec: phandles for the main TLV320AIC3X node and the + auxiliary TLV320AIC3X node (in this order) +- nokia,headphone-amplifier: phandle for the TPA6130A2 node +- tvout-selection-gpios: GPIO for tvout selection +- jack-detection-gpios: GPIO for jack detection +- eci-switch-gpios: GPIO for ECI (Enhancement Control Interface) switch +- speaker-amplifier-gpios: GPIO for speaker amplifier + +Example: + +sound { + compatible = "nokia,n900-audio"; + + nokia,cpu-dai = <&mcbsp2>; + nokia,audio-codec = <&tlv320aic3x>, <&tlv320aic3x_aux>; + nokia,headphone-amplifier = <&tpa6130a2>; + + tvout-selection-gpios = <&gpio2 8 GPIO_ACTIVE_HIGH>; /* 40 */ + jack-detection-gpios = <&gpio6 17 GPIO_ACTIVE_HIGH>; /* 177 */ + eci-switch-gpios = <&gpio6 22 GPIO_ACTIVE_HIGH>; /* 182 */ + speaker-amplifier-gpios = <&twl_gpio 7 GPIO_ACTIVE_HIGH>; +}; diff --git a/include/sound/rx51.h b/include/sound/rx51.h new file mode 100644 index 0000000..190d745 --- /dev/null +++ b/include/sound/rx51.h @@ -0,0 +1,18 @@ +/* + * Platform data for Nokia RX-51 SoC audio + * + * 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. + */ +#ifndef __RX51_AUDIO_H__ +#define __RX51_AUDIO_H__ + +struct rx51_audio_pdata { + int tvout_selection_gpio; + int jack_detection_gpio; + int eci_sw_gpio; + int speaker_amp_gpio; +}; + +#endif diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 6c19bba..f88e31e 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -691,7 +691,7 @@ OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP" #port " Sidetone Channel 1 Volume", \ OMAP_MCBSP_ST_CONTROLS(2); OMAP_MCBSP_ST_CONTROLS(3);
-int omap_mcbsp_st_add_controls(struct snd_soc_pcm_runtime *rtd) +int omap_mcbsp_st_add_controls(struct snd_soc_pcm_runtime *rtd, int port_id) { struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct omap_mcbsp *mcbsp = snd_soc_dai_get_drvdata(cpu_dai); @@ -701,7 +701,7 @@ int omap_mcbsp_st_add_controls(struct snd_soc_pcm_runtime *rtd) return 0; }
- switch (mcbsp->id) { + switch (port_id) { case 2: /* McBSP 2 */ return snd_soc_add_dai_controls(cpu_dai, omap_mcbsp2_st_controls, @@ -711,6 +711,7 @@ int omap_mcbsp_st_add_controls(struct snd_soc_pcm_runtime *rtd) omap_mcbsp3_st_controls, ARRAY_SIZE(omap_mcbsp3_st_controls)); default: + dev_err(mcbsp->dev, "Port %d not supported\n", port_id); break; }
diff --git a/sound/soc/omap/omap-mcbsp.h b/sound/soc/omap/omap-mcbsp.h index ba8386a..2e3369c 100644 --- a/sound/soc/omap/omap-mcbsp.h +++ b/sound/soc/omap/omap-mcbsp.h @@ -39,6 +39,6 @@ enum omap_mcbsp_div { OMAP_MCBSP_CLKGDV, /* Sample rate generator divider */ };
-int omap_mcbsp_st_add_controls(struct snd_soc_pcm_runtime *rtd); +int omap_mcbsp_st_add_controls(struct snd_soc_pcm_runtime *rtd, int port_id);
#endif diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c index 58f5e12..89deeb7 100644 --- a/sound/soc/omap/rx51.c +++ b/sound/soc/omap/rx51.c @@ -26,11 +26,13 @@ #include <linux/delay.h> #include <linux/gpio.h> #include <linux/platform_device.h> +#include <linux/of_gpio.h> #include <linux/module.h> #include <sound/core.h> #include <sound/jack.h> #include <sound/pcm.h> #include <sound/soc.h> +#include <sound/rx51.h> #include <linux/platform_data/asoc-ti-mcbsp.h> #include "../codecs/tpa6130a2.h"
@@ -38,15 +40,6 @@
#include "omap-mcbsp.h"
-#define RX51_TVOUT_SEL_GPIO 40 -#define RX51_JACK_DETECT_GPIO 177 -#define RX51_ECI_SW_GPIO 182 -/* - * REVISIT: TWL4030 GPIO base in RX-51. Now statically defined to 192. This - * gpio is reserved in arch/arm/mach-omap2/board-rx51-peripherals.c - */ -#define RX51_SPEAKER_AMP_TWL_GPIO (192 + 7) - enum { RX51_JACK_DISABLED, RX51_JACK_TVOUT, /* tv-out with stereo output */ @@ -60,6 +53,8 @@ static int rx51_jack_func;
static void rx51_ext_control(struct snd_soc_dapm_context *dapm) { + struct snd_soc_card *card = dapm->card; + struct rx51_audio_pdata *pdata = snd_soc_card_get_drvdata(card); int hp = 0, hs = 0, tvout = 0;
switch (rx51_jack_func) { @@ -93,7 +88,7 @@ static void rx51_ext_control(struct snd_soc_dapm_context *dapm) else snd_soc_dapm_disable_pin_unlocked(dapm, "HS Mic");
- gpio_set_value(RX51_TVOUT_SEL_GPIO, tvout); + gpio_set_value(pdata->tvout_selection_gpio, tvout);
snd_soc_dapm_sync_unlocked(dapm);
@@ -154,10 +149,12 @@ static int rx51_set_spk(struct snd_kcontrol *kcontrol, static int rx51_spk_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *k, int event) { - if (SND_SOC_DAPM_EVENT_ON(event)) - gpio_set_value_cansleep(RX51_SPEAKER_AMP_TWL_GPIO, 1); - else - gpio_set_value_cansleep(RX51_SPEAKER_AMP_TWL_GPIO, 0); + struct snd_soc_dapm_context *dapm = w->dapm; + struct snd_soc_card *card = dapm->card; + struct rx51_audio_pdata *pdata = snd_soc_card_get_drvdata(card); + + gpio_set_value_cansleep(pdata->speaker_amp_gpio, + !!SND_SOC_DAPM_EVENT_ON(event));
return 0; } @@ -223,7 +220,6 @@ static struct snd_soc_jack rx51_av_jack;
static struct snd_soc_jack_gpio rx51_av_jack_gpios[] = { { - .gpio = RX51_JACK_DETECT_GPIO, .name = "avdet-gpio", .report = SND_JACK_HEADSET, .invert = 1, @@ -290,6 +286,9 @@ static const struct snd_kcontrol_new aic34_rx51_controlsb[] = { static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_codec *codec = rtd->codec; + struct snd_soc_card *card = codec->card; + struct rx51_audio_pdata *pdata = snd_soc_card_get_drvdata(card); + struct snd_soc_dapm_context *dapm = &codec->dapm; int err;
@@ -301,8 +300,10 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd) /* Add RX-51 specific controls */ err = snd_soc_add_card_controls(rtd->card, aic34_rx51_controls, ARRAY_SIZE(aic34_rx51_controls)); - if (err < 0) + if (err < 0) { + dev_err(card->dev, "Failed to add RX-51 specific controls\n"); return err; + }
/* Add RX-51 specific widgets */ snd_soc_dapm_new_controls(dapm, aic34_dapm_widgets, @@ -312,25 +313,39 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_add_routes(dapm, audio_map, ARRAY_SIZE(audio_map));
err = tpa6130a2_add_controls(codec); - if (err < 0) + if (err < 0) { + dev_err(card->dev, "Failed to add TPA6130A2 controls\n"); return err; + } snd_soc_limit_volume(codec, "TPA6130A2 Headphone Playback Volume", 42);
- err = omap_mcbsp_st_add_controls(rtd); - if (err < 0) + err = omap_mcbsp_st_add_controls(rtd, 2); + if (err < 0) { + dev_err(card->dev, "Failed to add MCBSP controls\n"); return err; + }
/* AV jack detection */ err = snd_soc_jack_new(codec, "AV Jack", SND_JACK_HEADSET | SND_JACK_VIDEOOUT, &rx51_av_jack); - if (err) + if (err) { + dev_err(card->dev, "Failed to add AV Jack\n"); return err; + + } + + rx51_av_jack_gpios[0].gpio = pdata->jack_detection_gpio; + err = snd_soc_jack_add_gpios(&rx51_av_jack, - ARRAY_SIZE(rx51_av_jack_gpios), - rx51_av_jack_gpios); + ARRAY_SIZE(rx51_av_jack_gpios), + rx51_av_jack_gpios); + if (err) { + dev_err(card->dev, "Failed to add GPIOs\n"); + return err; + }
- return err; + return 0; }
static int rx51_aic34b_init(struct snd_soc_dapm_context *dapm) @@ -396,37 +411,107 @@ static struct snd_soc_card rx51_sound_card = {
static int rx51_soc_probe(struct platform_device *pdev) { + struct rx51_audio_pdata *pdata = pdev->dev.platform_data; + struct device_node *np = pdev->dev.of_node; struct snd_soc_card *card = &rx51_sound_card; int err;
if (!machine_is_nokia_rx51() && !of_machine_is_compatible("nokia,omap3-n900")) return -ENODEV;
- err = gpio_request_one(RX51_TVOUT_SEL_GPIO, + card->dev = &pdev->dev; + + if (np) { + struct device_node *dai_node; + + pdata = devm_kzalloc(&pdev->dev, + sizeof(struct rx51_audio_pdata), GFP_KERNEL); + + if (pdata == NULL) { + dev_err(card->dev, "failed to create private data\n"); + return -ENOMEM; + } + + pdata->tvout_selection_gpio = + of_get_named_gpio(np, "tvout-selection-gpios", 0); + pdata->jack_detection_gpio = + of_get_named_gpio(np, "jack-detection-gpios", 0); + pdata->eci_sw_gpio = + of_get_named_gpio(np, "eci-switch-gpios", 0); + pdata->speaker_amp_gpio = + of_get_named_gpio(np, "speaker-amplifier-gpios", 0); + + dai_node = of_parse_phandle(np, "nokia,cpu-dai", 0); + if (!dai_node) { + dev_err(&pdev->dev, "McBSP node is not provided\n"); + return -EINVAL; + } + rx51_dai[0].cpu_dai_name = NULL; + rx51_dai[0].cpu_of_node = dai_node; + + dai_node = of_parse_phandle(np, "nokia,audio-codec", 0); + if (!dai_node) { + dev_err(&pdev->dev, "Codec node is not provided\n"); + return -EINVAL; + } + rx51_dai[0].codec_name = NULL; + rx51_dai[0].codec_of_node = dai_node; + + dai_node = of_parse_phandle(np, "nokia,audio-codec", 1); + if (!dai_node) { + dev_err(&pdev->dev, "Auxiliary Codec node is not provided\n"); + return -EINVAL; + } + rx51_aux_dev[0].codec_name = NULL; + rx51_aux_dev[0].codec_of_node = dai_node; + rx51_codec_conf[0].dev_name = NULL; + rx51_codec_conf[0].of_node = dai_node; + + if (pdata->tvout_selection_gpio == -EPROBE_DEFER || + pdata->jack_detection_gpio == -EPROBE_DEFER || + pdata->eci_sw_gpio == -EPROBE_DEFER || + pdata->speaker_amp_gpio == -EPROBE_DEFER) { + devm_kfree(card->dev, pdata); + pdev->dev.platform_data = NULL; + return -EPROBE_DEFER; + } + + pdev->dev.platform_data = pdata; + } else if (!pdata) { + dev_err(card->dev, "platform data missing"); + err = -ENODEV; + goto error; + } + + err = devm_gpio_request_one(card->dev, pdata->tvout_selection_gpio, GPIOF_DIR_OUT | GPIOF_INIT_LOW, "tvout_sel"); - if (err) - goto err_gpio_tvout_sel; - err = gpio_request_one(RX51_ECI_SW_GPIO, + if (err) { + dev_err(card->dev, "could not setup tvout_sel\n"); + goto error; + } + err = devm_gpio_request_one(card->dev, pdata->eci_sw_gpio, GPIOF_DIR_OUT | GPIOF_INIT_HIGH, "eci_sw"); - if (err) - goto err_gpio_eci_sw; - - card->dev = &pdev->dev; + if (err) { + dev_err(card->dev, "could not setup eci_sw\n"); + goto error; + } + err = devm_gpio_request_one(card->dev, pdata->speaker_amp_gpio, + GPIOF_DIR_OUT | GPIOF_INIT_LOW, "speaker_en"); + if (err) { + dev_err(card->dev, "could not setup speaker_en\n"); + goto error; + }
+ snd_soc_card_set_drvdata(card, pdata); err = devm_snd_soc_register_card(card->dev, card); if (err) { - dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", err); - goto err_snd; + dev_err(card->dev, "snd_soc_register_card failed (%d)\n", err); + goto error; }
return 0; -err_snd: +error: card->dev = NULL; - gpio_free(RX51_ECI_SW_GPIO); -err_gpio_eci_sw: - gpio_free(RX51_TVOUT_SEL_GPIO); -err_gpio_tvout_sel: - return err; }
@@ -439,16 +524,23 @@ static int rx51_soc_remove(struct platform_device *pdev)
card->dev = NULL;
- gpio_free(RX51_ECI_SW_GPIO); - gpio_free(RX51_TVOUT_SEL_GPIO); - return 0; }
+#if defined(CONFIG_OF) +static const struct of_device_id rx51_audio_of_match[] = { + { .compatible = "nokia,n900-audio", }, + {}, +}; +MODULE_DEVICE_TABLE(of, rx51_audio_of_match); +#endif + + static struct platform_driver rx51_soc_driver = { .driver = { .name = "rx51-audio", .owner = THIS_MODULE, + .of_match_table = of_match_ptr(rx51_audio_of_match), }, .probe = rx51_soc_probe, .remove = rx51_soc_remove,
On Sat, Apr 05, 2014 at 11:35:51PM +0200, Sebastian Reichel wrote:
This patch adds device tree support to the Nokia N900 audio driver. It also removes GPIO defines and gets them from platform data / device tree, since some GPIO numbers may be different with DT boot.
This binding looks mostly fine, a few code things though which may influence the binding. The main thing is that you're doing a lot of changes here which aren't really related to adding the binding which aren't mentioned and make it harder to review - as well as making the change larger one of the things that's done in review is to check that the change did what it was described as doing. At least some of the time there isn't even any code overlap.
@@ -290,6 +286,9 @@ static const struct snd_kcontrol_new aic34_rx51_controlsb[] = { static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_codec *codec = rtd->codec;
- struct snd_soc_card *card = codec->card;
- struct rx51_audio_pdata *pdata = snd_soc_card_get_drvdata(card);
- struct snd_soc_dapm_context *dapm = &codec->dapm; int err;
Random blank line added here.
@@ -301,8 +300,10 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd) /* Add RX-51 specific controls */ err = snd_soc_add_card_controls(rtd->card, aic34_rx51_controls, ARRAY_SIZE(aic34_rx51_controls));
- if (err < 0)
if (err < 0) {
dev_err(card->dev, "Failed to add RX-51 specific controls\n");
return err;
}
/* Add RX-51 specific widgets */ snd_soc_dapm_new_controls(dapm, aic34_dapm_widgets,
This is nothing to do with DT support, separate patch please. There's quite a few other things like this. You're also not printing any error codes in the messages.
@@ -312,25 +313,39 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_add_routes(dapm, audio_map, ARRAY_SIZE(audio_map));
err = tpa6130a2_add_controls(codec);
- if (err < 0)
- if (err < 0) {
return err;dev_err(card->dev, "Failed to add TPA6130A2 controls\n");
- }
If this is converted to DT and you've added aux CODEC support as part of that then why are we still manually adding the controls for the headphone amp? I would have expected this to be registered as an aux CODEC. This seems likely to feed through into the binding for referencing the components.
- err = omap_mcbsp_st_add_controls(rtd, 2);
- if (err < 0) {
return err;dev_err(card->dev, "Failed to add MCBSP controls\n");
- }
Refactoring this as a separate patch would also help.
- err = gpio_request_one(RX51_ECI_SW_GPIO,
- if (err) {
dev_err(card->dev, "could not setup tvout_sel\n");
goto error;
- }
- err = devm_gpio_request_one(card->dev, pdata->eci_sw_gpio, GPIOF_DIR_OUT | GPIOF_INIT_HIGH, "eci_sw");
Again, moving this refactoring into a separate patch will help a lot with review.
Use table based setup to register the DAPM widgets and routes. This on one hand makes the code a bit cleaner and on the other hand the board level DAPM elements get registered in the card's DAPM context rather than in the CODEC's DAPM context. This fixes an issue with double prefixing of routes.
Signed-off-by: Sebastian Reichel sre@kernel.org --- sound/soc/omap/rx51.c | 48 ++++++------------------------------------------ 1 file changed, 6 insertions(+), 42 deletions(-)
diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c index 89deeb7..418bd41 100644 --- a/sound/soc/omap/rx51.c +++ b/sound/soc/omap/rx51.c @@ -233,9 +233,6 @@ static const struct snd_soc_dapm_widget aic34_dapm_widgets[] = { SND_SOC_DAPM_HP("Headphone Jack", rx51_hp_event), SND_SOC_DAPM_MIC("HS Mic", NULL), SND_SOC_DAPM_LINE("FM Transmitter", NULL), -}; - -static const struct snd_soc_dapm_widget aic34_dapm_widgetsb[] = { SND_SOC_DAPM_SPK("Earphone", NULL), };
@@ -249,9 +246,7 @@ static const struct snd_soc_dapm_route audio_map[] = {
{"DMic Rate 64", NULL, "Mic Bias"}, {"Mic Bias", NULL, "DMic"}, -};
-static const struct snd_soc_dapm_route audio_mapb[] = { {"b LINE2R", NULL, "MONO_LOUT"}, {"Earphone", NULL, "b HPLOUT"},
@@ -277,9 +272,6 @@ static const struct snd_kcontrol_new aic34_rx51_controls[] = { SOC_ENUM_EXT("Jack Function", rx51_enum[2], rx51_get_jack, rx51_set_jack), SOC_DAPM_PIN_SWITCH("FM Transmitter"), -}; - -static const struct snd_kcontrol_new aic34_rx51_controlsb[] = { SOC_DAPM_PIN_SWITCH("Earphone"), };
@@ -297,21 +289,6 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_nc_pin(dapm, "MIC3R"); snd_soc_dapm_nc_pin(dapm, "LINE1R");
- /* Add RX-51 specific controls */ - err = snd_soc_add_card_controls(rtd->card, aic34_rx51_controls, - ARRAY_SIZE(aic34_rx51_controls)); - if (err < 0) { - dev_err(card->dev, "Failed to add RX-51 specific controls\n"); - return err; - } - - /* Add RX-51 specific widgets */ - snd_soc_dapm_new_controls(dapm, aic34_dapm_widgets, - ARRAY_SIZE(aic34_dapm_widgets)); - - /* Set up RX-51 specific audio path audio_map */ - snd_soc_dapm_add_routes(dapm, audio_map, ARRAY_SIZE(audio_map)); - err = tpa6130a2_add_controls(codec); if (err < 0) { dev_err(card->dev, "Failed to add TPA6130A2 controls\n"); @@ -348,24 +325,6 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd) return 0; }
-static int rx51_aic34b_init(struct snd_soc_dapm_context *dapm) -{ - int err; - - err = snd_soc_add_card_controls(dapm->card, aic34_rx51_controlsb, - ARRAY_SIZE(aic34_rx51_controlsb)); - if (err < 0) - return err; - - err = snd_soc_dapm_new_controls(dapm, aic34_dapm_widgetsb, - ARRAY_SIZE(aic34_dapm_widgetsb)); - if (err < 0) - return 0; - - return snd_soc_dapm_add_routes(dapm, audio_mapb, - ARRAY_SIZE(audio_mapb)); -} - /* Digital audio interface glue - connects codec <--> CPU */ static struct snd_soc_dai_link rx51_dai[] = { { @@ -386,7 +345,6 @@ static struct snd_soc_aux_dev rx51_aux_dev[] = { { .name = "TLV320AIC34b", .codec_name = "tlv320aic3x-codec.2-0019", - .init = rx51_aic34b_init, }, };
@@ -407,6 +365,12 @@ static struct snd_soc_card rx51_sound_card = { .num_aux_devs = ARRAY_SIZE(rx51_aux_dev), .codec_conf = rx51_codec_conf, .num_configs = ARRAY_SIZE(rx51_codec_conf), + .dapm_widgets = aic34_dapm_widgets, + .num_dapm_widgets = ARRAY_SIZE(aic34_dapm_widgets), + .controls = aic34_rx51_controls, + .num_controls = ARRAY_SIZE(aic34_rx51_controls), + .dapm_routes = audio_map, + .num_dapm_routes = ARRAY_SIZE(audio_map), };
static int rx51_soc_probe(struct platform_device *pdev)
On Sat, Apr 05, 2014 at 11:35:52PM +0200, Sebastian Reichel wrote:
Use table based setup to register the DAPM widgets and routes. This on one hand makes the code a bit cleaner and on the other hand the board level DAPM elements get registered in the card's DAPM context rather than in the CODEC's DAPM context. This fixes an issue with double prefixing of routes.
Simple patches like this, especially ones that are bug fixes, should come at the start of the series to allow them to be applied first. This cuts down on review if multiple iterations are needed and allows bug fixes to be merged as such.
Currently the second tlv320aic3x instance fails to be probed from DT if the reset pin is shared with the first one.
This patch fixes it by moving the list add of the reset pin into the i2c_probe method.
Signed-off-by: Sebastian Reichel sre@kernel.org --- sound/soc/codecs/tlv320aic3x.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index b183510..d7349bc 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -1399,7 +1399,6 @@ static int aic3x_probe(struct snd_soc_codec *codec) }
aic3x_add_widgets(codec); - list_add(&aic3x->list, &reset_list);
return 0;
@@ -1569,7 +1568,13 @@ static int aic3x_i2c_probe(struct i2c_client *i2c,
ret = snd_soc_register_codec(&i2c->dev, &soc_codec_dev_aic3x, &aic3x_dai, 1); - return ret; + + if (ret != 0) + goto err_gpio; + + list_add(&aic3x->list, &reset_list); + + return 0;
err_gpio: if (gpio_is_valid(aic3x->gpio_reset) &&
On Sat, Apr 05, 2014 at 11:35:53PM +0200, Sebastian Reichel wrote:
Currently the second tlv320aic3x instance fails to be probed from DT if the reset pin is shared with the first one.
Applied, thanks.
participants (3)
-
Mark Brown
-
Rajeev kumar
-
Sebastian Reichel