[alsa-devel] [PATCH 0/6] Bugfixes and clean-ups bound for the v3.6 RCs
This patch-set includes some important changes which should make their way to the Mainline Release Candidates for the v3.6 release. Without them Audio doesn't even probe (at all) when booting with Device Tree selected. The kernel can't compile with ux500 audio enabled. Even if we can get that far SoC Core will assume we're using regmaps and attempt to use them; however, none exist for this driver, so the kernel will oops.
arch/arm/mach-ux500/board-mop500-msp.c | 2 +- arch/arm/mach-ux500/board-mop500.c | 9 ++++----- sound/soc/codecs/ab8500-codec.c | 4 ++++ sound/soc/soc-dapm.c | 2 -- 4 files changed, 9 insertions(+), 8 deletions(-)
If a list of widgets is provided and one of them fails to be added as a control, the present semantics fail all subsequent widgets. A better solution would be to only fail that widget, but pursue in attempting to add the rest of the list.
Signed-off-by: Lee Jones lee.jones@linaro.org --- sound/soc/soc-dapm.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index eded657..7d9c154 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3095,8 +3095,6 @@ int snd_soc_dapm_new_controls(struct snd_soc_dapm_context *dapm, dev_err(dapm->dev, "ASoC: Failed to create DAPM control %s\n", widget->name); - ret = -ENOMEM; - break; } widget++; }
On Tue, Jul 31, 2012 at 03:45:40PM +0100, Lee Jones wrote:
If a list of widgets is provided and one of them fails to be added as a control, the present semantics fail all subsequent widgets. A better solution would be to only fail that widget, but pursue in attempting to add the rest of the list.
To reiterate, this is in *no* way urgent or even a bug fix.
dev_err(dapm->dev, "ASoC: Failed to create DAPM control %s\n", widget->name);
ret = -ENOMEM;break;
Indeed, removing the error return is a regression.
On 31/07/12 15:56, Mark Brown wrote:
On Tue, Jul 31, 2012 at 03:45:40PM +0100, Lee Jones wrote:
If a list of widgets is provided and one of them fails to be added as a control, the present semantics fail all subsequent widgets. A better solution would be to only fail that widget, but pursue in attempting to add the rest of the list.
To reiterate, this is in *no* way urgent or even a bug fix.
It fixes sound in our driver.
Without this the card failes to instantiate.
dev_err(dapm->dev, "ASoC: Failed to create DAPM control %s\n", widget->name);
ret = -ENOMEM;break;Indeed, removing the error return is a regression.
Isn't the return code incorrect? There are a multitude of reasons why snd_soc_dapm_new_control() would fail. No-memory is just one of them, so why do we force this probable lie?
On Tue, Jul 31, 2012 at 04:09:08PM +0100, Lee Jones wrote:
On 31/07/12 15:56, Mark Brown wrote:
To reiterate, this is in *no* way urgent or even a bug fix.
It fixes sound in our driver.
Without this the card failes to instantiate.
You're kidding, right? Fix the actual error.
Isn't the return code incorrect? There are a multitude of reasons why snd_soc_dapm_new_control() would fail. No-memory is just one of them, so why do we force this probable lie?
I don't think anyone actually cares what the error code is, feel free to pick another random number.
If codec->control_data is not populated SoC Core assumes we want to use regmap, which fails catastrophically, as we don't have one:
Unable to handle kernel NULL pointer dereference at virtual address 00000080 pgd = c0004000 [00000080] *pgd=00000000 Internal error: Oops: 17 [#1] PREEMPT SMP ARM Modules linked in: CPU: 1 Not tainted (3.5.0-rc6-00884-g0b2419e-dirty #130) PC is at regmap_read+0x10/0x5c LR is at hw_read+0x80/0x90 pc : [<c01a91b8>] lr : [<c0216804>] psr: 60000013
Signed-off-by: Lee Jones lee.jones@linaro.org --- sound/soc/codecs/ab8500-codec.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/sound/soc/codecs/ab8500-codec.c b/sound/soc/codecs/ab8500-codec.c index 3c79592..23b4018 100644 --- a/sound/soc/codecs/ab8500-codec.c +++ b/sound/soc/codecs/ab8500-codec.c @@ -2406,6 +2406,10 @@ static int ab8500_codec_probe(struct snd_soc_codec *codec)
/* Setup AB8500 according to board-settings */ pdata = (struct ab8500_platform_data *)dev_get_platdata(dev->parent); + + /* Inform SoC Core that we have our own I/O arrangements. */ + codec->control_data = (void *)true; + status = ab8500_audio_setup_mics(codec, &pdata->codec->amics); if (status < 0) { pr_err("%s: Failed to setup mics (%d)!\n", __func__, status);
On Tue, Jul 31, 2012 at 03:45:41PM +0100, Lee Jones wrote:
If codec->control_data is not populated SoC Core assumes we want to use regmap, which fails catastrophically, as we don't have one:
Applied. If you could do resends at a rate somewhat lower than once per working day that'd be helpful, especially given the number of large patch serieses that you're sending right now.
This was left over during a recent clean-up which removed Device Tree helper structs. There is no longer a requirement for it, so we can just remove it.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/mach-ux500/board-mop500.c | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c index 80b4f0e..e641003 100644 --- a/arch/arm/mach-ux500/board-mop500.c +++ b/arch/arm/mach-ux500/board-mop500.c @@ -726,11 +726,6 @@ MACHINE_END
#ifdef CONFIG_MACH_UX500_DT
-static struct platform_device *snowball_of_platform_devs[] __initdata = { - &snowball_led_dev, - &snowball_key_dev, -}; - struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = { /* Requires call-back bindings. */ OF_DEV_AUXDATA("arm,cortex-a9-pmu", 0, "arm-pmu", &db8500_pmu_platdata),
The platform attempts to register platform device 'snd_soc_u8500' which doesn't actually exist. Here we change the reference to the correct one 'snd_soc_mop500'.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/mach-ux500/board-mop500-msp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-ux500/board-mop500-msp.c b/arch/arm/mach-ux500/board-mop500-msp.c index 9960480..1b6a193 100644 --- a/arch/arm/mach-ux500/board-mop500-msp.c +++ b/arch/arm/mach-ux500/board-mop500-msp.c @@ -228,7 +228,7 @@ int mop500_msp_init(struct device *parent) struct platform_device *msp1;
pr_info("%s: Register platform-device 'snd-soc-u8500'.\n", __func__); - platform_device_register(&snd_soc_u8500); + platform_device_register(&snd_soc_mop500);
pr_info("Initialize MSP I2S-devices.\n"); db8500_add_msp_i2s(parent, 0, U8500_MSP0_BASE, IRQ_DB8500_MSP0,
Previous attempts to add platform probing of the Audio related devices only call from non-DT initialisation functions. This patch extends that functionality to the Device Tree related ones too.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/mach-ux500/board-mop500.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c index e641003..87a5cd7 100644 --- a/arch/arm/mach-ux500/board-mop500.c +++ b/arch/arm/mach-ux500/board-mop500.c @@ -794,6 +794,7 @@ static void __init u8500_init_machine(void) ARRAY_SIZE(mop500_platform_devs));
mop500_sdi_init(parent); + mop500_msp_init(parent); i2c0_devs = ARRAY_SIZE(mop500_i2c0_devices); i2c_register_board_info(0, mop500_i2c0_devices, i2c0_devs); i2c_register_board_info(2, mop500_i2c2_devices, @@ -801,6 +802,8 @@ static void __init u8500_init_machine(void)
mop500_uib_init();
+ } else if (of_machine_is_compatible("calaosystems,snowball-a9500")) { + mop500_of_msp_init(parent); } else if (of_machine_is_compatible("st-ericsson,hrefv60+")) { /* * The HREFv60 board removed a GPIO expander and routed @@ -812,6 +815,7 @@ static void __init u8500_init_machine(void) ARRAY_SIZE(mop500_platform_devs));
hrefv60_sdi_init(parent); + mop500_msp_init(parent);
i2c0_devs = ARRAY_SIZE(mop500_i2c0_devices); i2c0_devs -= NUM_PRE_V60_I2C0_DEVICES;
participants (2)
-
Lee Jones -
Mark Brown