On Tue, Mar 13, 2012 at 04:11:43PM +0100, Ola Lilja wrote:
+config SND_SOC_UX500_AB8500
- bool "Codec/Machine - AB8500 (MSA)"
Wny non-modular?
- depends on SND_SOC_UX500_MSP_I2S && SND_SOC_UX500_PLATFORM && AB8500_CORE && AB8500_GPADC
Word wrapping and this should select pretty much all of this with the possible exception of the MFD core.
+ifdef CONFIG_SND_SOC_UX500_AB8500 +snd-soc-ux500-machine-objs += ux500_ab8500.o +endif
Again, why are you doing this non-idiomatic stuff?
+/* Create dummy devices for platform drivers */
+static struct platform_device ux500_pcm = {
.name = "ux500-pcm",.id = 0,.dev = {.platform_data = NULL,},+};
The SoC or CPU side drivers should be taking care of stuff like this.
+/* Define the whole U8500 soundcard, linking platform to the codec-drivers */ +struct snd_soc_dai_link u8500_dai_links[] = {
- #ifdef CONFIG_SND_SOC_UX500_AB8500
What's this ifdef doing...? It looks very, very suspicous.
- {
- .name = "ab8500_0",
Your indentation is also fairly broken here.
- pdev = platform_device_alloc("soc-audio", -1);
- if (!pdev)
return -ENOMEM;
No, probe using the normal device model and register using snd_soc_register_card().
+++ b/sound/soc/ux500/ux500_ab8500.c
The fact that you've got multiple files for a machine driver is worrying...
+/* ANC States */ +static const char * const enum_anc_state[] = {
- "Unconfigured",
- "Apply FIR and IIR",
- "FIR and IIR are configured",
- "Apply FIR",
- "FIR is configured",
- "Apply IIR",
- "IIR is configured"
+};
Why is this not part of the CODEC driver? What makes this machine specific?
+/* Regulators */ +enum regulator_idx {
- REGULATOR_AUDIO,
- REGULATOR_DMIC,
- REGULATOR_AMIC1,
- REGULATOR_AMIC2
+}; +static struct regulator_bulk_data reg_info[4] = {
- { .consumer = NULL, .supply = "V-AUD" },
- { .consumer = NULL, .supply = "V-DMIC" },
- { .consumer = NULL, .supply = "V-AMIC1" },
- { .consumer = NULL, .supply = "V-AMIC2" }
+}; +static bool reg_enabled[4] = {
- false,
- false,
- false,
- false
+}; +static int reg_claim[4]; +enum amic_idx { AMIC_1A, AMIC_1B, AMIC_2 }; +struct amic_conf {
- enum regulator_idx reg_id;
- bool enabled;
- char *name;
I have no idea what this code is intended to do but it looks *extremely* confused, it's especially surprising given that your CODEC makes no use of regulators even for basic power.
+static int enable_regulator(struct device *dev, enum regulator_idx idx) +{
You appear to be defining some sort of subsystem on top of the regulator API here but I'm not sure what it's intended to do or why you're doing it...
+static const struct snd_soc_dapm_widget ux500_ab8500_dapm_widgets[] = {
- SND_SOC_DAPM_SUPPLY("AUDIO Regulator",
SND_SOC_NOPM, 0, 0, dapm_audioreg_event,SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
DAPM has native support for regulator supplies.
- /* Enable gpio.1-clock (needed by DSP in burst mode) */
- ret = clk_enable(clk_ptr_gpio1);
- if (ret) {
dev_err(dev, "%s: ERROR: clk_enable(gpio.1) failed (ret = %d)!",__func__, ret);return ret;- }
Why can't the DSP figure out that it needs to enable the clock?