[alsa-devel] [Patch v2 1/6] ASoC: Tegra: wm8903 probe: Don't call machine_is_*()
This machine driver is a platform driver, and hence will only be instantiated on the correct machines. Hence, there is no need to check the current machine during probe.
(Applying Mark's TrimSlice review comments to the existing driver)
Signed-off-by: Stephen Warren swarren@nvidia.com --- sound/soc/tegra/tegra_wm8903.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/sound/soc/tegra/tegra_wm8903.c b/sound/soc/tegra/tegra_wm8903.c index 37f6010..1eb0632 100644 --- a/sound/soc/tegra/tegra_wm8903.c +++ b/sound/soc/tegra/tegra_wm8903.c @@ -391,13 +391,6 @@ static __devinit int tegra_wm8903_driver_probe(struct platform_device *pdev) struct tegra_wm8903_platform_data *pdata; int ret;
- if (!machine_is_harmony() && !machine_is_ventana() && - !machine_is_seaboard() && !machine_is_kaen() && - !machine_is_aebl()) { - dev_err(&pdev->dev, "Not running on a supported board!\n"); - return -ENODEV; - } - pdata = pdev->dev.platform_data; if (!pdata) { dev_err(&pdev->dev, "No platform data supplied\n");
When the driver is not initialized/registered, nothing should be touching these fields anyway, so there's no point clearing them out.
(Applying Mark's TrimSlice review comments to the existing driver)
Signed-off-by: Stephen Warren swarren@nvidia.com --- sound/soc/tegra/tegra_wm8903.c | 11 ++--------- 1 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/sound/soc/tegra/tegra_wm8903.c b/sound/soc/tegra/tegra_wm8903.c index 1eb0632..b12b1fd 100644 --- a/sound/soc/tegra/tegra_wm8903.c +++ b/sound/soc/tegra/tegra_wm8903.c @@ -417,15 +417,12 @@ static __devinit int tegra_wm8903_driver_probe(struct platform_device *pdev) if (ret) { dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret); - goto err_clear_drvdata; + goto err_fini_utils; }
return 0;
-err_clear_drvdata: - snd_soc_card_set_drvdata(card, NULL); - platform_set_drvdata(pdev, NULL); - card->dev = NULL; +err_fini_utils: tegra_asoc_utils_fini(&machine->util_data); err_free_machine: kfree(machine); @@ -440,10 +437,6 @@ static int __devexit tegra_wm8903_driver_remove(struct platform_device *pdev)
snd_soc_unregister_card(card);
- snd_soc_card_set_drvdata(card, NULL); - platform_set_drvdata(pdev, NULL); - card->dev = NULL; - tegra_asoc_utils_fini(&machine->util_data);
if (machine->gpio_requested & GPIO_EXT_MIC_EN)
Only the clock programming code needs to know whether the clocks changed, and that is encapsulated within tegra_asoc_utils_set_rate(). The machine driver's call to snd_soc_dai_set_sysclk(codec_dai, ...) is safe irrespective of whether the clocks changed.
(Applying Mark's TrimSlice review comments to the existing driver)
Signed-off-by: Stephen Warren swarren@nvidia.com --- sound/soc/tegra/tegra_asoc_utils.c | 9 +++++---- sound/soc/tegra/tegra_asoc_utils.h | 2 +- sound/soc/tegra/tegra_wm8903.c | 17 +++++++---------- 3 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/sound/soc/tegra/tegra_asoc_utils.c b/sound/soc/tegra/tegra_asoc_utils.c index 52f0a3f..dfa85cb 100644 --- a/sound/soc/tegra/tegra_asoc_utils.c +++ b/sound/soc/tegra/tegra_asoc_utils.c @@ -28,9 +28,10 @@ #include "tegra_asoc_utils.h"
int tegra_asoc_utils_set_rate(struct tegra_asoc_utils_data *data, int srate, - int mclk, int *mclk_change) + int mclk) { int new_baseclock; + bool clk_change; int err;
switch (srate) { @@ -52,10 +53,10 @@ int tegra_asoc_utils_set_rate(struct tegra_asoc_utils_data *data, int srate, return -EINVAL; }
- *mclk_change = ((new_baseclock != data->set_baseclock) || + clk_change = ((new_baseclock != data->set_baseclock) || (mclk != data->set_mclk)); - if (!*mclk_change) - return 0; + if (!clk_change) + return 0;
data->set_baseclock = 0; data->set_mclk = 0; diff --git a/sound/soc/tegra/tegra_asoc_utils.h b/sound/soc/tegra/tegra_asoc_utils.h index bbba7af..4818195 100644 --- a/sound/soc/tegra/tegra_asoc_utils.h +++ b/sound/soc/tegra/tegra_asoc_utils.h @@ -36,7 +36,7 @@ struct tegra_asoc_utils_data { };
int tegra_asoc_utils_set_rate(struct tegra_asoc_utils_data *data, int srate, - int mclk, int *mclk_change); + int mclk); int tegra_asoc_utils_init(struct tegra_asoc_utils_data *data, struct device *dev); void tegra_asoc_utils_fini(struct tegra_asoc_utils_data *data); diff --git a/sound/soc/tegra/tegra_wm8903.c b/sound/soc/tegra/tegra_wm8903.c index b12b1fd..988ff50 100644 --- a/sound/soc/tegra/tegra_wm8903.c +++ b/sound/soc/tegra/tegra_wm8903.c @@ -72,7 +72,7 @@ static int tegra_wm8903_hw_params(struct snd_pcm_substream *substream, struct snd_soc_codec *codec = rtd->codec; struct snd_soc_card *card = codec->card; struct tegra_wm8903 *machine = snd_soc_card_get_drvdata(card); - int srate, mclk, mclk_change; + int srate, mclk; int err;
srate = params_rate(params); @@ -90,8 +90,7 @@ static int tegra_wm8903_hw_params(struct snd_pcm_substream *substream, while (mclk < 6000000) mclk *= 2;
- err = tegra_asoc_utils_set_rate(&machine->util_data, srate, mclk, - &mclk_change); + err = tegra_asoc_utils_set_rate(&machine->util_data, srate, mclk); if (err < 0) { dev_err(card->dev, "Can't configure clocks\n"); return err; @@ -115,13 +114,11 @@ static int tegra_wm8903_hw_params(struct snd_pcm_substream *substream, return err; }
- if (mclk_change) { - err = snd_soc_dai_set_sysclk(codec_dai, 0, mclk, - SND_SOC_CLOCK_IN); - if (err < 0) { - dev_err(card->dev, "codec_dai clock not set\n"); - return err; - } + err = snd_soc_dai_set_sysclk(codec_dai, 0, mclk, + SND_SOC_CLOCK_IN); + if (err < 0) { + dev_err(card->dev, "codec_dai clock not set\n"); + return err; }
return 0;
Card widgets are created in the card's DAPM context, not any codec's DAPM context. Hence, w->codec==NULL. Instead, find the card from the widget through the DAPM context of the widget, not the codec of the widget.
Signed-off-by: Stephen Warren swarren@nvidia.com --- sound/soc/tegra/tegra_wm8903.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/tegra/tegra_wm8903.c b/sound/soc/tegra/tegra_wm8903.c index 988ff50..9a439fb 100644 --- a/sound/soc/tegra/tegra_wm8903.c +++ b/sound/soc/tegra/tegra_wm8903.c @@ -156,8 +156,8 @@ static struct snd_soc_jack_pin tegra_wm8903_mic_jack_pins[] = { static int tegra_wm8903_event_int_spk(struct snd_soc_dapm_widget *w, struct snd_kcontrol *k, int event) { - struct snd_soc_codec *codec = w->codec; - struct snd_soc_card *card = codec->card; + struct snd_soc_dapm_context *dapm = w->dapm; + struct snd_soc_card *card = dapm->card; struct tegra_wm8903 *machine = snd_soc_card_get_drvdata(card); struct tegra_wm8903_platform_data *pdata = machine->pdata;
@@ -173,8 +173,8 @@ static int tegra_wm8903_event_int_spk(struct snd_soc_dapm_widget *w, static int tegra_wm8903_event_hp(struct snd_soc_dapm_widget *w, struct snd_kcontrol *k, int event) { - struct snd_soc_codec *codec = w->codec; - struct snd_soc_card *card = codec->card; + struct snd_soc_dapm_context *dapm = w->dapm; + struct snd_soc_card *card = dapm->card; struct tegra_wm8903 *machine = snd_soc_card_get_drvdata(card); struct tegra_wm8903_platform_data *pdata = machine->pdata;
Not all widgets on a card are within the codec's DAPM context. Fix snd_soc_dapm_get_pin_status to search all contexts when looking for a widget.
This change is required when modifying tegra_wm8903 to use snd_soc_card.widgets rather than calling snd_soc_dapm_new_controls; the former adds the widgets to the card's DAPM context, whereas tegra_wm8903 uses the codec's DAPM context when calling snd_soc_dapm_new_controls.
By code inspection, I suspect this also applies to Samsung Speyside.
Signed-off-by: Stephen Warren swarren@nvidia.com --- sound/soc/soc-dapm.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 2ee738c..4c868f9 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -2403,6 +2403,12 @@ int snd_soc_dapm_get_pin_status(struct snd_soc_dapm_context *dapm, return w->connected; }
+ /* Try again in other contexts */ + list_for_each_entry(w, &dapm->card->widgets, list) { + if (!strcmp(w->name, pin)) + return w->connected; + } + return 0; } EXPORT_SYMBOL_GPL(snd_soc_dapm_get_pin_status);
Replace calls to a variety of registration functions by updating struct snd_soc_card snd_soc_tegra_wm8903 to directly point at the various control/widget/map tables instead. The ASoC core now performs any required registration based on these data fields.
(Applying Mark's TrimSlice review comments to the existing driver)
Signed-off-by: Stephen Warren swarren@nvidia.com --- sound/soc/tegra/tegra_wm8903.c | 41 ++++++++++++++++++--------------------- 1 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/sound/soc/tegra/tegra_wm8903.c b/sound/soc/tegra/tegra_wm8903.c index 9a439fb..0d6738a 100644 --- a/sound/soc/tegra/tegra_wm8903.c +++ b/sound/soc/tegra/tegra_wm8903.c @@ -294,28 +294,6 @@ static int tegra_wm8903_init(struct snd_soc_pcm_runtime *rtd) gpio_direction_output(pdata->gpio_ext_mic_en, 0); }
- ret = snd_soc_add_controls(codec, tegra_wm8903_controls, - ARRAY_SIZE(tegra_wm8903_controls)); - if (ret < 0) - return ret; - - snd_soc_dapm_new_controls(dapm, tegra_wm8903_dapm_widgets, - ARRAY_SIZE(tegra_wm8903_dapm_widgets)); - - if (machine_is_harmony() || machine_is_ventana()) { - snd_soc_dapm_add_routes(dapm, harmony_audio_map, - ARRAY_SIZE(harmony_audio_map)); - } else if (machine_is_seaboard()) { - snd_soc_dapm_add_routes(dapm, seaboard_audio_map, - ARRAY_SIZE(seaboard_audio_map)); - } else if (machine_is_kaen()) { - snd_soc_dapm_add_routes(dapm, kaen_audio_map, - ARRAY_SIZE(kaen_audio_map)); - } else { - snd_soc_dapm_add_routes(dapm, aebl_audio_map, - ARRAY_SIZE(aebl_audio_map)); - } - if (gpio_is_valid(pdata->gpio_hp_det)) { tegra_wm8903_hp_jack_gpio.gpio = pdata->gpio_hp_det; snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE, @@ -379,6 +357,11 @@ static struct snd_soc_card snd_soc_tegra_wm8903 = { .name = "tegra-wm8903", .dai_link = &tegra_wm8903_dai, .num_links = 1, + + .controls = tegra_wm8903_controls, + .num_controls = ARRAY_SIZE(tegra_wm8903_controls), + .dapm_widgets = tegra_wm8903_dapm_widgets, + .num_dapm_widgets = ARRAY_SIZE(tegra_wm8903_dapm_widgets), };
static __devinit int tegra_wm8903_driver_probe(struct platform_device *pdev) @@ -410,6 +393,20 @@ static __devinit int tegra_wm8903_driver_probe(struct platform_device *pdev) platform_set_drvdata(pdev, card); snd_soc_card_set_drvdata(card, machine);
+ if (machine_is_harmony() || machine_is_ventana()) { + card->dapm_routes = harmony_audio_map; + card->num_dapm_routes = ARRAY_SIZE(harmony_audio_map); + } else if (machine_is_seaboard()) { + card->dapm_routes = seaboard_audio_map; + card->num_dapm_routes = ARRAY_SIZE(seaboard_audio_map); + } else if (machine_is_kaen()) { + card->dapm_routes = kaen_audio_map; + card->num_dapm_routes = ARRAY_SIZE(kaen_audio_map); + } else { + card->dapm_routes = aebl_audio_map; + card->num_dapm_routes = ARRAY_SIZE(aebl_audio_map); + } + ret = snd_soc_register_card(card); if (ret) { dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n",
On Tue, 2011-04-19 at 15:25 -0600, Stephen Warren wrote:
This machine driver is a platform driver, and hence will only be instantiated on the correct machines. Hence, there is no need to check the current machine during probe.
(Applying Mark's TrimSlice review comments to the existing driver)
Signed-off-by: Stephen Warren swarren@nvidia.com
sound/soc/tegra/tegra_wm8903.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/sound/soc/tegra/tegra_wm8903.c b/sound/soc/tegra/tegra_wm8903.c index 37f6010..1eb0632 100644 --- a/sound/soc/tegra/tegra_wm8903.c +++ b/sound/soc/tegra/tegra_wm8903.c @@ -391,13 +391,6 @@ static __devinit int tegra_wm8903_driver_probe(struct platform_device *pdev) struct tegra_wm8903_platform_data *pdata; int ret;
- if (!machine_is_harmony() && !machine_is_ventana() &&
!machine_is_seaboard() && !machine_is_kaen() &&!machine_is_aebl()) {dev_err(&pdev->dev, "Not running on a supported board!\n");return -ENODEV;- }
- pdata = pdev->dev.platform_data; if (!pdata) { dev_err(&pdev->dev, "No platform data supplied\n");
All
Acked-by: Liam Girdwood lrg@ti.com
On Tue, Apr 19, 2011 at 03:25:07PM -0600, Stephen Warren wrote:
This machine driver is a platform driver, and hence will only be instantiated on the correct machines. Hence, there is no need to check the current machine during probe.
(Applying Mark's TrimSlice review comments to the existing driver)
Applied all, thanks.
participants (3)
-
Liam Girdwood -
Mark Brown -
Stephen Warren