[alsa-devel] [PATCH 00/21] Some fixes and DT enablement for ux500 audio
This patch-set sees some code reinforcement surrounding the recently accepted MOP500 MSP, PCM, AB8500 CODEC and ux500 Audio Machine Driver. It contains some extra error checking pertaining to the ux500 specific drivers themselves, along with bugfixes for issues in core SoC Audio code happened upon along the way. There are also a couple of minor clean-ups relating to ux500 platform code, which are hitching a ride for ease of status tracking.
arch/arm/boot/dts/db8500.dtsi | 48 ++++++ arch/arm/mach-ux500/Kconfig | 1 + arch/arm/mach-ux500/Makefile | 2 +- arch/arm/mach-ux500/board-mop500-audio.c | 194 ++++++++++++++++++++++ arch/arm/mach-ux500/board-mop500-msp.c | 267 ------------------------------ arch/arm/mach-ux500/board-mop500-msp.h | 14 -- arch/arm/mach-ux500/board-mop500.c | 23 ++- arch/arm/mach-ux500/board-mop500.h | 6 + arch/arm/mach-ux500/include/mach/msp.h | 3 +- drivers/mfd/ab8500-core.c | 1 + drivers/mfd/db8500-prcmu.c | 1 + include/linux/mfd/abx500/ab8500-codec.h | 6 +- sound/soc/codecs/ab8500-codec.c | 79 +++++++++ sound/soc/soc-dapm.c | 2 - sound/soc/soc-io.c | 14 +- sound/soc/ux500/mop500.c | 41 +++++ sound/soc/ux500/ux500_msp_dai.c | 11 +- sound/soc/ux500/ux500_msp_i2s.c | 118 ++++++++++--- sound/soc/ux500/ux500_msp_i2s.h | 5 +- sound/soc/ux500/ux500_pcm.c | 6 + 20 files changed, 516 insertions(+), 326 deletions(-)
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),
When booting via platform code the AB8500 platform data is now passed in though the DB8500. However, if pdata_size is not set it will not be subsequently passed onto subordinate devices. This patch correctly populates pdata_size.
Signed-off-by: Lee Jones lee.jones@linaro.org --- drivers/mfd/db8500-prcmu.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c index 4050a1e..7040a00 100644 --- a/drivers/mfd/db8500-prcmu.c +++ b/drivers/mfd/db8500-prcmu.c @@ -3002,6 +3002,7 @@ static int __devinit db8500_prcmu_probe(struct platform_device *pdev) for (i = 0; i < ARRAY_SIZE(db8500_prcmu_devs); i++) { if (!strcmp(db8500_prcmu_devs[i].name, "ab8500-core")) { db8500_prcmu_devs[i].platform_data = ab8500_platdata; + db8500_prcmu_devs[i].pdata_size = sizeof(struct ab8500_platform_data); } }
This patch contains a couple of general MSP clean-ups and a bug-fix. The general clean-ups pertain to layout changes and changing functions to be void instead of int and not to regardlessly return '0'. The bug that's fixed is thought to be caused by a merge error. The code erroneously attempts to platform_device_register a non-existent struct. It's a simple fix, just reference the correct platform_data structure in its place.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/mach-ux500/board-mop500-msp.c | 28 +++++++++++++--------------- arch/arm/mach-ux500/board-mop500-msp.h | 14 -------------- arch/arm/mach-ux500/board-mop500.c | 1 - arch/arm/mach-ux500/board-mop500.h | 3 +++ 4 files changed, 16 insertions(+), 30 deletions(-) delete mode 100644 arch/arm/mach-ux500/board-mop500-msp.h
diff --git a/arch/arm/mach-ux500/board-mop500-msp.c b/arch/arm/mach-ux500/board-mop500-msp.c index 9960480..217df1d 100644 --- a/arch/arm/mach-ux500/board-mop500-msp.c +++ b/arch/arm/mach-ux500/board-mop500-msp.c @@ -192,21 +192,21 @@ static struct platform_device *db8500_add_msp_i2s(struct device *parent, }
/* Platform device for ASoC U8500 machine */ -static struct platform_device snd_soc_u8500 = { - .name = "snd-soc-u8500", - .id = 0, - .dev = { - .platform_data = NULL, - }, +static struct platform_device snd_soc_mop500 = { + .name = "snd-soc-mop500", + .id = 0, + .dev = { + .platform_data = NULL, + }, };
/* Platform device for Ux500-PCM */ static struct platform_device ux500_pcm = { - .name = "ux500-pcm", - .id = 0, - .dev = { - .platform_data = NULL, - }, + .name = "ux500-pcm", + .id = 0, + .dev = { + .platform_data = NULL, + }, };
static struct msp_i2s_platform_data msp2_platform_data = { @@ -223,12 +223,12 @@ static struct msp_i2s_platform_data msp3_platform_data = { .msp_i2s_exit = msp13_i2s_exit, };
-int mop500_msp_init(struct device *parent) +void 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, @@ -262,6 +262,4 @@ int mop500_msp_init(struct device *parent)
pr_info("%s: Register platform-device 'ux500-pcm'\n", __func__); platform_device_register(&ux500_pcm); - - return 0; } diff --git a/arch/arm/mach-ux500/board-mop500-msp.h b/arch/arm/mach-ux500/board-mop500-msp.h deleted file mode 100644 index 6fcfb5e..0000000 --- a/arch/arm/mach-ux500/board-mop500-msp.h +++ /dev/null @@ -1,14 +0,0 @@ -/* - * Copyright (C) ST-Ericsson SA 2012 - * - * Author: Ola Lilja ola.o.lilja@stericsson.com, - * for ST-Ericsson. - * - * License terms: - * - * 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. - */ - -void mop500_msp_init(struct device *parent); diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c index e641003..b87393c 100644 --- a/arch/arm/mach-ux500/board-mop500.c +++ b/arch/arm/mach-ux500/board-mop500.c @@ -54,7 +54,6 @@ #include "devices-db8500.h" #include "board-mop500.h" #include "board-mop500-regulators.h" -#include "board-mop500-msp.h"
static struct gpio_led snowball_led_array[] = { { diff --git a/arch/arm/mach-ux500/board-mop500.h b/arch/arm/mach-ux500/board-mop500.h index d04a8e6..1d7316b 100644 --- a/arch/arm/mach-ux500/board-mop500.h +++ b/arch/arm/mach-ux500/board-mop500.h @@ -92,6 +92,9 @@ void __init mop500_stuib_init(void); void __init mop500_pinmaps_init(void); void __init snowball_pinmaps_init(void); void __init hrefv60_pinmaps_init(void); +void mop500_msp_init(struct device *parent); +/* Due for removal once the MSP driver has been fully DT:ed. */ +void mop500_of_msp_init(struct device *parent);
int __init mop500_uib_init(void); void mop500_uib_i2c_add(int busnum, struct i2c_board_info *info,
Currently there is no out-of-memory error checking after attempting to allocate memory for the ux500_msp or ux500_msp_i2s_drvdata data structures. Instead we go about populating them regardless. This patch applies the necessary error checking to prevent a panic.
Signed-off-by: Lee Jones lee.jones@linaro.org --- sound/soc/ux500/ux500_msp_dai.c | 3 +++ sound/soc/ux500/ux500_msp_i2s.c | 2 ++ 2 files changed, 5 insertions(+)
diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c index 62ac0285..cdbbdaf 100644 --- a/sound/soc/ux500/ux500_msp_dai.c +++ b/sound/soc/ux500/ux500_msp_dai.c @@ -760,6 +760,9 @@ static int __devinit ux500_msp_drv_probe(struct platform_device *pdev) drvdata = devm_kzalloc(&pdev->dev, sizeof(struct ux500_msp_i2s_drvdata), GFP_KERNEL); + if (!drvdata) + return -ENOMEM; + drvdata->fmt = 0; drvdata->slots = 1; drvdata->tx_mask = 0x01; diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c index ee14d2d..4c79850 100644 --- a/sound/soc/ux500/ux500_msp_i2s.c +++ b/sound/soc/ux500/ux500_msp_i2s.c @@ -673,6 +673,8 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,
*msp_p = devm_kzalloc(&pdev->dev, sizeof(struct ux500_msp), GFP_KERNEL); msp = *msp_p; + if (!msp) + return -ENOMEM;
msp->id = platform_data->id; msp->dev = &pdev->dev;
On Thu, Jul 26, 2012 at 11:28:37AM +0100, Lee Jones wrote:
Currently there is no out-of-memory error checking after attempting to allocate memory for the ux500_msp or ux500_msp_i2s_drvdata data structures. Instead we go about populating them regardless. This patch applies the necessary error checking to prevent a panic.
Applied, thanks.
Thought to be another merge error, board-mop500-msp.h has never existed in the upstream kernel, only msp.h. This patch changes the include files to match the existing file name.
Signed-off-by: Lee Jones lee.jones@linaro.org --- sound/soc/ux500/ux500_msp_dai.c | 2 +- sound/soc/ux500/ux500_msp_i2s.c | 2 +- sound/soc/ux500/ux500_msp_i2s.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c index cdbbdaf..772cb19 100644 --- a/sound/soc/ux500/ux500_msp_dai.c +++ b/sound/soc/ux500/ux500_msp_dai.c @@ -21,7 +21,7 @@ #include <linux/mfd/dbx500-prcmu.h>
#include <mach/hardware.h> -#include <mach/board-mop500-msp.h> +#include <mach/msp.h>
#include <sound/soc.h> #include <sound/soc-dai.h> diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c index 4c79850..36be11e 100644 --- a/sound/soc/ux500/ux500_msp_i2s.c +++ b/sound/soc/ux500/ux500_msp_i2s.c @@ -19,7 +19,7 @@ #include <linux/slab.h>
#include <mach/hardware.h> -#include <mach/board-mop500-msp.h> +#include <mach/msp.h>
#include <sound/soc.h>
diff --git a/sound/soc/ux500/ux500_msp_i2s.h b/sound/soc/ux500/ux500_msp_i2s.h index 7f71b4a..2d9136d 100644 --- a/sound/soc/ux500/ux500_msp_i2s.h +++ b/sound/soc/ux500/ux500_msp_i2s.h @@ -17,7 +17,7 @@
#include <linux/platform_device.h>
-#include <mach/board-mop500-msp.h> +#include <mach/msp.h>
#define MSP_INPUT_FREQ_APB 48000000
On Thu, Jul 26, 2012 at 11:28:38AM +0100, Lee Jones wrote:
Thought to be another merge error, board-mop500-msp.h has never existed in the upstream kernel, only msp.h. This patch changes the include files to match the existing file name.
Applied, thanks.
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 Thu, Jul 26, 2012 at 11:28:39AM +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.
Why?
If a sound codec fails to request a regmap, the 'using_regmap' is set as true regardless, despite there being no regmap to use. As a repercussion, when a latter read function checks to see if we are using regmaps, it assumes we are and attempts to. Only the kernel oopes, because regmap_* tries to extract information from a NULL pointer.
Signed-off-by: Lee Jones lee.jones@linaro.org --- sound/soc/soc-io.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/sound/soc/soc-io.c b/sound/soc/soc-io.c index 29183ef..601cb7f 100644 --- a/sound/soc/soc-io.c +++ b/sound/soc/soc-io.c @@ -52,10 +52,13 @@ static unsigned int hw_read(struct snd_soc_codec *codec, unsigned int reg) if (codec->cache_only) return -1;
- ret = regmap_read(codec->control_data, reg, &val); - if (ret == 0) - return val; - else + if (codec->using_regmap) { + ret = regmap_read(codec->control_data, reg, &val); + if (ret == 0) + return val; + else + return -1; + } else return -1; }
@@ -141,11 +144,12 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
case SND_SOC_REGMAP: /* Device has made its own regmap arrangements */ - codec->using_regmap = true; if (!codec->control_data) codec->control_data = dev_get_regmap(codec->dev, NULL);
if (codec->control_data) { + codec->using_regmap = true; + ret = regmap_get_val_bytes(codec->control_data); /* Errors are legitimate for non-integer byte * multiples */
On Thu, Jul 26, 2012 at 11:28:40AM +0100, Lee Jones wrote:
@@ -52,10 +52,13 @@ static unsigned int hw_read(struct snd_soc_codec *codec, unsigned int reg) if (codec->cache_only) return -1;
ret = regmap_read(codec->control_data, reg, &val);if (ret == 0)return val;else
if (codec->using_regmap) {ret = regmap_read(codec->control_data, reg, &val);if (ret == 0)return val;elsereturn -1;} else
No, this makes no sense. There is no non-regmap I/O support in soc-io, anything using the soc-io hw_read() function must be using regmap.
case SND_SOC_REGMAP: /* Device has made its own regmap arrangements */
codec->using_regmap = true;
Again, this makes no sense. If we're explicitly being asked to use regmap then we should be using regmap or just failing to set up I/O (which is obviously a catastrophic failure).
On 26/07/12 12:32, Mark Brown wrote:
On Thu, Jul 26, 2012 at 11:28:40AM +0100, Lee Jones wrote:
@@ -52,10 +52,13 @@ static unsigned int hw_read(struct snd_soc_codec *codec, unsigned int reg) if (codec->cache_only) return -1;
ret = regmap_read(codec->control_data, reg, &val);if (ret == 0)return val;else
if (codec->using_regmap) {ret = regmap_read(codec->control_data, reg, &val);if (ret == 0)return val;elsereturn -1;} elseNo, this makes no sense. There is no non-regmap I/O support in soc-io, anything using the soc-io hw_read() function must be using regmap.
case SND_SOC_REGMAP: /* Device has made its own regmap arrangements */
codec->using_regmap = true;Again, this makes no sense. If we're explicitly being asked to use regmap then we should be using regmap or just failing to set up I/O (which is obviously a catastrophic failure).
How much work is there involved in regmap:ing a device, so that dev_get_regmap() doesn't fail?
On Thu, Jul 26, 2012 at 12:38:17PM +0100, Lee Jones wrote:
On 26/07/12 12:32, Mark Brown wrote:
Again, this makes no sense. If we're explicitly being asked to use regmap then we should be using regmap or just failing to set up I/O (which is obviously a catastrophic failure).
How much work is there involved in regmap:ing a device, so that dev_get_regmap() doesn't fail?
Trivial if it's on a supported bus, otherwise you just need to write the bus. But why do you care if dev_get_regmap() fails? We only try to use regmap if the driver asked for regmap I/O (or doesn't have registers at all in which case it doesn't matter since we never do any I/O). What you appear to be saying here is that you're using regmap on a device which doesn't have a regmap set up which is clearly never going to work terribly well...
On 26/07/12 12:42, Mark Brown wrote:
On Thu, Jul 26, 2012 at 12:38:17PM +0100, Lee Jones wrote:
On 26/07/12 12:32, Mark Brown wrote:
Again, this makes no sense. If we're explicitly being asked to use regmap then we should be using regmap or just failing to set up I/O (which is obviously a catastrophic failure).
How much work is there involved in regmap:ing a device, so that dev_get_regmap() doesn't fail?
Trivial if it's on a supported bus, otherwise you just need to write the bus. But why do you care if dev_get_regmap() fails? We only try to use regmap if the driver asked for regmap I/O (or doesn't have registers at all in which case it doesn't matter since we never do any I/O). What you appear to be saying here is that you're using regmap on a device which doesn't have a regmap set up which is clearly never going to work terribly well...
I don't think we want to use regmap at all, but we're forced to by soc-core. How do we over-ride that behavior? By writing some nonsense into codec->control_data?
On Thu, Jul 26, 2012 at 03:51:13PM +0100, Lee Jones wrote:
I don't think we want to use regmap at all, but we're forced to by soc-core. How do we over-ride that behavior? By writing some nonsense into codec->control_data?
You should use that for your control data, yes - you're not forced to use regmap at all. Like I say we've got a bunch of drivers doing so already.
On 26/07/12 16:12, Mark Brown wrote:
On Thu, Jul 26, 2012 at 03:51:13PM +0100, Lee Jones wrote:
I don't think we want to use regmap at all, but we're forced to by soc-core. How do we over-ride that behavior? By writing some nonsense into codec->control_data?
You should use that for your control data, yes - you're not forced to use regmap at all. Like I say we've got a bunch of drivers doing so already.
What's my 'control data'? It's not used in the original codec patch.
The old way wants to go:
snd_soc_update_bits() -> snd_soc_read() -> ab8500_codec_read_reg()
When then calls back into the abx500.
So what 'control data' should I be storing in the codec struct?
On Thu, Jul 26, 2012 at 04:23:33PM +0100, Lee Jones wrote:
What's my 'control data'? It's not used in the original codec patch.
The old way wants to go:
snd_soc_update_bits() -> snd_soc_read() -> ab8500_codec_read_reg()
When then calls back into the abx500.
So what 'control data' should I be storing in the codec struct?
You're supposed to use it for the data you use to call back into the underlying I/O code.
On 26/07/12 16:25, Mark Brown wrote:
On Thu, Jul 26, 2012 at 04:23:33PM +0100, Lee Jones wrote:
What's my 'control data'? It's not used in the original codec patch.
The old way wants to go:
snd_soc_update_bits() -> snd_soc_read() -> ab8500_codec_read_reg()
When then calls back into the abx500.
So what 'control data' should I be storing in the codec struct?
You're supposed to use it for the data you use to call back into the underlying I/O code.
I don't understand. What 'data'?
Surely if .read and .write are populated in 'struct snd_soc_codec_driver', then it should just call back into those?
On Thu, Jul 26, 2012 at 05:05:51PM +0100, Lee Jones wrote:
On 26/07/12 16:25, Mark Brown wrote:
You're supposed to use it for the data you use to call back into the underlying I/O code.
I don't understand. What 'data'?
Whatever your I/O layer so desires, the core doesn't care. It's generally whatever the lower layer that does your I/O takes to identify the device.
Surely if .read and .write are populated in 'struct snd_soc_codec_driver', then it should just call back into those?
Yes, and in fact that's what we do!
Ensure correct probing and pass though important configuration options to the AB8500 CODEC driver when DT is enabled
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/boot/dts/db8500.dtsi | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/db8500.dtsi b/arch/arm/boot/dts/db8500.dtsi index 7279165..a3bee0a 100644 --- a/arch/arm/boot/dts/db8500.dtsi +++ b/arch/arm/boot/dts/db8500.dtsi @@ -370,6 +370,12 @@ compatible = "stericsson,ab8500-debug"; };
+ codec: ab8500-codec { + compatible = "stericsson,ab8500-codec"; + + stericsson,earpeice-cmv = <950>; /* Units in mV. */ + }; + ab8500-regulators { compatible = "stericsson,ab8500-regulator";
List all four MSP devices which exist on all DB8500 based platforms, to ensure correct device probing and configuration passing when booting with DT.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/boot/dts/db8500.dtsi | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/arch/arm/boot/dts/db8500.dtsi b/arch/arm/boot/dts/db8500.dtsi index a3bee0a..5a72be6 100644 --- a/arch/arm/boot/dts/db8500.dtsi +++ b/arch/arm/boot/dts/db8500.dtsi @@ -591,6 +591,36 @@ status = "disabled"; };
+ msp0: msp@80123000 { + compatible = "stericsson,ux500-msp-i2s"; + reg = <0x80123000 0x1000>; + interrupts = <0 31 0x4>; + v-ape-supply = <&db8500_vape_reg>; + }; + + msp1: msp@80124000 { + compatible = "stericsson,ux500-msp-i2s"; + reg = <0x80124000 0x1000>; + interrupts = <0 62 0x4>; + v-ape-supply = <&db8500_vape_reg>; + stericcson,use-pinctrl; + }; + + // HDMI sound + msp2: msp@80117000 { + compatible = "stericsson,ux500-msp-i2s"; + reg = <0x80117000 0x1000>; + interrupts = <0 98 0x4>; + v-ape-supply = <&db8500_vape_reg>; + }; + + msp3: msp@80125000 { + compatible = "stericsson,ux500-msp-i2s"; + reg = <0x80125000 0x1000>; + interrupts = <0 62 0x4>; + v-ape-supply = <&db8500_vape_reg>; + }; + external-bus@50000000 { compatible = "simple-bus"; reg = <0x50000000 0x4000000>;
Nothing special here. We're only providing a compatible string to ensure the driver is probed using a Device Tree boot.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/boot/dts/db8500.dtsi | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/db8500.dtsi b/arch/arm/boot/dts/db8500.dtsi index 5a72be6..b9d4405 100644 --- a/arch/arm/boot/dts/db8500.dtsi +++ b/arch/arm/boot/dts/db8500.dtsi @@ -591,6 +591,10 @@ status = "disabled"; };
+ pcm: ux500-pcm { + compatible = "stericsson,ux500-pcm"; + }; + msp0: msp@80123000 { compatible = "stericsson,ux500-msp-i2s"; reg = <0x80123000 0x1000>;
This is the node which links together the platform (PCM), codec (AB8500) and the CPU side Digital Audio Interface (MCP) with the machine driver.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/boot/dts/db8500.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/db8500.dtsi b/arch/arm/boot/dts/db8500.dtsi index b9d4405..978dd60 100644 --- a/arch/arm/boot/dts/db8500.dtsi +++ b/arch/arm/boot/dts/db8500.dtsi @@ -595,6 +595,14 @@ compatible = "stericsson,ux500-pcm"; };
+ sound { + compatible = "stericsson,snd-soc-mop500"; + + platform-pcm-dma = <&pcm>; + cpu-dai = <&msp1 &msp3>; + audio-codec = <&codec>; + }; + msp0: msp@80123000 { compatible = "stericsson,ux500-msp-i2s"; reg = <0x80123000 0x1000>;
We've done this before and it worked well last time. Here we're duplicating a complex registration function to ease the process of enabling it for Device Tree. As there are quite a few steps taken during the registration process, it makes sense to break them up into more manageable chunks. This patch will aid us.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/mach-ux500/board-mop500-msp.c | 42 ++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)
diff --git a/arch/arm/mach-ux500/board-mop500-msp.c b/arch/arm/mach-ux500/board-mop500-msp.c index 217df1d..d0bd030 100644 --- a/arch/arm/mach-ux500/board-mop500-msp.c +++ b/arch/arm/mach-ux500/board-mop500-msp.c @@ -223,6 +223,48 @@ static struct msp_i2s_platform_data msp3_platform_data = { .msp_i2s_exit = msp13_i2s_exit, };
+/* Due for removal once the MSP driver has been fully DT:ed. */ +void mop500_of_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_mop500); + + pr_info("Initialize MSP I2S-devices.\n"); + db8500_add_msp_i2s(parent, 0, U8500_MSP0_BASE, IRQ_DB8500_MSP0, + &msp0_platform_data); + msp1 = db8500_add_msp_i2s(parent, 1, U8500_MSP1_BASE, IRQ_DB8500_MSP1, + &msp1_platform_data); + db8500_add_msp_i2s(parent, 2, U8500_MSP2_BASE, IRQ_DB8500_MSP2, + &msp2_platform_data); + db8500_add_msp_i2s(parent, 3, U8500_MSP3_BASE, IRQ_DB8500_MSP1, + &msp3_platform_data); + + /* Get the pinctrl handle for MSP1 */ + if (msp1) { + msp1_p = pinctrl_get(&msp1->dev); + if (IS_ERR(msp1_p)) + dev_err(&msp1->dev, "could not get MSP1 pinctrl\n"); + else { + msp1_def = pinctrl_lookup_state(msp1_p, + PINCTRL_STATE_DEFAULT); + if (IS_ERR(msp1_def)) { + dev_err(&msp1->dev, + "could not get MSP1 defstate\n"); + } + msp1_sleep = pinctrl_lookup_state(msp1_p, + PINCTRL_STATE_SLEEP); + if (IS_ERR(msp1_sleep)) + dev_err(&msp1->dev, + "could not get MSP1 idlestate\n"); + } + } + + pr_info("%s: Register platform-device 'ux500-pcm'\n", __func__); + platform_device_register(&ux500_pcm); +} + void mop500_msp_init(struct device *parent) { struct platform_device *msp1;
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 b87393c..6f53a6f 100644 --- a/arch/arm/mach-ux500/board-mop500.c +++ b/arch/arm/mach-ux500/board-mop500.c @@ -793,6 +793,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, @@ -800,6 +801,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 @@ -811,6 +814,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;
The current kernel commandline for ux500 based devices includes hard-coded allocations for things like mali and hwmem, which actually run over lowmem. Here we enable highmem in order to avoid memory corruption errors.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/mach-ux500/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-ux500/Kconfig b/arch/arm/mach-ux500/Kconfig index c013bbf..f51c351 100644 --- a/arch/arm/mach-ux500/Kconfig +++ b/arch/arm/mach-ux500/Kconfig @@ -28,6 +28,7 @@ config MACH_MOP500 select I2C select I2C_NOMADIK select SOC_BUS + select HIGHMEM help Include support for the MOP500 development platform.
It isn't currently possible to pass all platform specific configuration though Device Tree. Thinks like device names used in the clock infrastructure, call-backs and DMA information have to be passed in via AUXDATA structures and the MSP is no exception. Here we're passing DMA settings.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/mach-ux500/board-mop500-msp.c | 8 ++++---- arch/arm/mach-ux500/board-mop500.c | 9 +++++++++ arch/arm/mach-ux500/board-mop500.h | 5 +++++ 3 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-ux500/board-mop500-msp.c b/arch/arm/mach-ux500/board-mop500-msp.c index d0bd030..a21b9e2 100644 --- a/arch/arm/mach-ux500/board-mop500-msp.c +++ b/arch/arm/mach-ux500/board-mop500-msp.c @@ -96,7 +96,7 @@ static struct stedma40_chan_cfg msp0_dma_tx = { /* data_width is set during configuration */ };
-static struct msp_i2s_platform_data msp0_platform_data = { +struct msp_i2s_platform_data msp0_platform_data = { .id = MSP_I2S_0, .msp_i2s_dma_rx = &msp0_dma_rx, .msp_i2s_dma_tx = &msp0_dma_tx, @@ -128,7 +128,7 @@ static struct stedma40_chan_cfg msp1_dma_tx = { /* data_width is set during configuration */ };
-static struct msp_i2s_platform_data msp1_platform_data = { +struct msp_i2s_platform_data msp1_platform_data = { .id = MSP_I2S_1, .msp_i2s_dma_rx = NULL, .msp_i2s_dma_tx = &msp1_dma_tx, @@ -209,13 +209,13 @@ static struct platform_device ux500_pcm = { }, };
-static struct msp_i2s_platform_data msp2_platform_data = { +struct msp_i2s_platform_data msp2_platform_data = { .id = MSP_I2S_2, .msp_i2s_dma_rx = &msp2_dma_rx, .msp_i2s_dma_tx = &msp2_dma_tx, };
-static struct msp_i2s_platform_data msp3_platform_data = { +struct msp_i2s_platform_data msp3_platform_data = { .id = MSP_I2S_3, .msp_i2s_dma_rx = &msp1_dma_rx, .msp_i2s_dma_tx = NULL, diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c index 6f53a6f..d455a61 100644 --- a/arch/arm/mach-ux500/board-mop500.c +++ b/arch/arm/mach-ux500/board-mop500.c @@ -753,6 +753,15 @@ struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = { OF_DEV_AUXDATA("st,nomadik-i2c", 0x8012a000, "nmk-i2c.4", NULL), /* Requires device name bindings. */ OF_DEV_AUXDATA("stericsson,nmk_pinctrl", 0, "pinctrl-db8500", NULL), + /* Requires clock name and DMA bindings. */ + OF_DEV_AUXDATA("stericsson,ux500-msp-i2s", 0x80123000, + "ux500-msp-i2s.0", &msp0_platform_data), + OF_DEV_AUXDATA("stericsson,ux500-msp-i2s", 0x80124000, + "ux500-msp-i2s.1", &msp1_platform_data), + OF_DEV_AUXDATA("stericsson,ux500-msp-i2s", 0x80117000, + "ux500-msp-i2s.2", &msp2_platform_data), + OF_DEV_AUXDATA("stericsson,ux500-msp-i2s", 0x80125000, + "ux500-msp-i2s.3", &msp3_platform_data), {}, };
diff --git a/arch/arm/mach-ux500/board-mop500.h b/arch/arm/mach-ux500/board-mop500.h index 1d7316b..3fbf48f 100644 --- a/arch/arm/mach-ux500/board-mop500.h +++ b/arch/arm/mach-ux500/board-mop500.h @@ -9,6 +9,7 @@
/* For NOMADIK_NR_GPIO */ #include <mach/irqs.h> +#include <mach/msp.h> #include <linux/amba/mmci.h>
/* Snowball specific GPIO assignments, this board has no GPIO expander */ @@ -81,6 +82,10 @@ struct device; struct i2c_board_info; extern struct mmci_platform_data mop500_sdi0_data; extern struct mmci_platform_data mop500_sdi4_data; +extern struct msp_i2s_platform_data msp0_platform_data; +extern struct msp_i2s_platform_data msp1_platform_data; +extern struct msp_i2s_platform_data msp2_platform_data; +extern struct msp_i2s_platform_data msp3_platform_data; extern struct arm_pmu_platdata db8500_pmu_platdata;
extern void mop500_sdi_init(struct device *parent);
In this patch we stop registering the MOP500 driver from platform code and rely solely on Device Tree to do the probing for us. We also parse the sound node to link together the codec, dma and the CPU-side Digital Audio Interface.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/mach-ux500/board-mop500-msp.c | 3 --- sound/soc/ux500/mop500.c | 41 ++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-ux500/board-mop500-msp.c b/arch/arm/mach-ux500/board-mop500-msp.c index a21b9e2..ce7fff9 100644 --- a/arch/arm/mach-ux500/board-mop500-msp.c +++ b/arch/arm/mach-ux500/board-mop500-msp.c @@ -228,9 +228,6 @@ void mop500_of_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_mop500); - pr_info("Initialize MSP I2S-devices.\n"); db8500_add_msp_i2s(parent, 0, U8500_MSP0_BASE, IRQ_DB8500_MSP0, &msp0_platform_data); diff --git a/sound/soc/ux500/mop500.c b/sound/soc/ux500/mop500.c index 31c4d26..d84a073 100644 --- a/sound/soc/ux500/mop500.c +++ b/sound/soc/ux500/mop500.c @@ -16,6 +16,7 @@ #include <linux/module.h> #include <linux/io.h> #include <linux/spi/spi.h> +#include <linux/of.h>
#include <sound/soc.h> #include <sound/initval.h> @@ -56,14 +57,48 @@ static struct snd_soc_card mop500_card = { .num_links = ARRAY_SIZE(mop500_dai_links), };
+static int __devinit mop500_of_probe(struct platform_device *pdev, + struct device_node *np) +{ + struct device_node *codec_np, *platform_np, *msp_np[2]; + int i; + + platform_np = of_parse_phandle(np, "platform-pcm-dma", 0); + msp_np[0] = of_parse_phandle(np, "cpu-dai", 0); + msp_np[1] = of_parse_phandle(np, "cpu-dai", 1); + codec_np = of_parse_phandle(np, "audio-codec", 0); + + if (!(platform_np && msp_np[0] && msp_np[1] && codec_np)) { + dev_err(&pdev->dev, "Phandle missing or invalid\n"); + return -EINVAL; + } + + for (i = 0; i < 2; i++) { + mop500_dai_links[i].platform_of_node = platform_np; + mop500_dai_links[i].platform_name = NULL; + mop500_dai_links[i].cpu_of_node = msp_np[i]; + mop500_dai_links[i].cpu_dai_name = NULL; + mop500_dai_links[i].codec_of_node = codec_np; + mop500_dai_links[i].codec_name = NULL; + } + + return 0; +} static int __devinit mop500_probe(struct platform_device *pdev) { + struct device_node *np = pdev->dev.of_node; int ret;
pr_debug("%s: Enter.\n", __func__);
dev_dbg(&pdev->dev, "%s: Enter.\n", __func__);
+ if (np) { + ret = mop500_of_probe(pdev, np); + if (ret) + return ret; + } + mop500_card.dev = &pdev->dev;
dev_dbg(&pdev->dev, "%s: Card %s: Set platform drvdata.\n", @@ -101,10 +136,16 @@ static int __devexit mop500_remove(struct platform_device *pdev) return 0; }
+static const struct of_device_id snd_soc_mop500_match[] = { + { .compatible = "stericsson,snd-soc-mop500", }, + {}, +}; + static struct platform_driver snd_soc_mop500_driver = { .driver = { .owner = THIS_MODULE, .name = "snd-soc-mop500", + .of_match_table = snd_soc_mop500_match, }, .probe = mop500_probe, .remove = __devexit_p(mop500_remove),
On Thu, Jul 26, 2012 at 11:28:49AM +0100, Lee Jones wrote:
arch/arm/mach-ux500/board-mop500-msp.c | 3 --- sound/soc/ux500/mop500.c | 41 ++++++++++++++++++++++++++++++++
There is no binding documentation here. All bindings should be documented.
- pr_info("%s: Register platform-device 'snd-soc-u8500'.\n", __func__);
- platform_device_register(&snd_soc_mop500);
This should be done separately - if it's going to be merged with something it should be the patch that adds the relevant DT fragments.
On 26/07/12 12:37, Mark Brown wrote:> On Thu, Jul 26, 2012 at 11:28:49AM +0100, Lee Jones wrote:
arch/arm/mach-ux500/board-mop500-msp.c | 3 --- sound/soc/ux500/mop500.c | 41 ++++++++++++++++++++++++++++++++
There is no binding documentation here. All bindings should be documented.
Yes I know. This is more of an RFC _before_ I waste my time writing documentation (again).
- pr_info("%s: Register platform-device 'snd-soc-u8500'.\n", __func__);
- platform_device_register(&snd_soc_mop500);
This should be done separately - if it's going to be merged with something it should be the patch that adds the relevant DT fragments.
Yes, I can do that.
On Thu, Jul 26, 2012 at 02:51:09PM +0100, Lee Jones wrote:
On 26/07/12 12:37, Mark Brown wrote:> On Thu, Jul 26, 2012 at 11:28:49AM +0100, Lee Jones wrote:
There is no binding documentation here. All bindings should be documented.
Yes I know. This is more of an RFC _before_ I waste my time writing documentation (again).
I can't be the only person who reviews bindings by reading the documentation for the binding...
On 26/07/12 14:53, Mark Brown wrote:
On Thu, Jul 26, 2012 at 02:51:09PM +0100, Lee Jones wrote:
On 26/07/12 12:37, Mark Brown wrote:> On Thu, Jul 26, 2012 at 11:28:49AM +0100, Lee Jones wrote:
There is no binding documentation here. All bindings should be documented.
Yes I know. This is more of an RFC _before_ I waste my time writing documentation (again).
I can't be the only person who reviews bindings by reading the documentation for the binding...
I plan to do it, promise. ;)
Here we pass platform registration from platform code over to Device Tree, when DT is enabled.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/mach-ux500/board-mop500-msp.c | 3 --- sound/soc/ux500/ux500_pcm.c | 6 ++++++ 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-ux500/board-mop500-msp.c b/arch/arm/mach-ux500/board-mop500-msp.c index ce7fff9..890feb3 100644 --- a/arch/arm/mach-ux500/board-mop500-msp.c +++ b/arch/arm/mach-ux500/board-mop500-msp.c @@ -257,9 +257,6 @@ void mop500_of_msp_init(struct device *parent) "could not get MSP1 idlestate\n"); } } - - pr_info("%s: Register platform-device 'ux500-pcm'\n", __func__); - platform_device_register(&ux500_pcm); }
void mop500_msp_init(struct device *parent) diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c index 1a04e24..557da2d 100644 --- a/sound/soc/ux500/ux500_pcm.c +++ b/sound/soc/ux500/ux500_pcm.c @@ -304,10 +304,16 @@ static int __devinit ux500_pcm_drv_remove(struct platform_device *pdev) return 0; }
+static const struct of_device_id ux500_pcm_match[] = { + { .compatible = "stericsson,ux500-pcm", }, + {}, +}; + static struct platform_driver ux500_pcm_driver = { .driver = { .name = "ux500-pcm", .owner = THIS_MODULE, + .of_match_table = ux500_pcm_match, },
.probe = ux500_pcm_drv_probe,
On Thu, Jul 26, 2012 at 11:28:50AM +0100, Lee Jones wrote:
Here we pass platform registration from platform code over to Device Tree, when DT is enabled.
- pr_info("%s: Register platform-device 'ux500-pcm'\n", __func__);
- platform_device_register(&ux500_pcm);
This has the same issue as your last patch... the way you're doing things will break audio on all boards using this driver.
On 26/07/12 12:38, Mark Brown wrote:
On Thu, Jul 26, 2012 at 11:28:50AM +0100, Lee Jones wrote:
Here we pass platform registration from platform code over to Device Tree, when DT is enabled.
- pr_info("%s: Register platform-device 'ux500-pcm'\n", __func__);
- platform_device_register(&ux500_pcm);
This has the same issue as your last patch... the way you're doing things will break audio on all boards using this driver.
It will, why?
On Thu, Jul 26, 2012 at 02:52:09PM +0100, Lee Jones wrote:
On 26/07/12 12:38, Mark Brown wrote:
- pr_info("%s: Register platform-device 'ux500-pcm'\n", __func__);
- platform_device_register(&ux500_pcm);
This has the same issue as your last patch... the way you're doing things will break audio on all boards using this driver.
It will, why?
You've just removed registration of the device and not added anything else to replace that. Even if all boards convert to DT their DTs will need to be updated which you're not doing.
On 26/07/12 15:22, Mark Brown wrote:
On Thu, Jul 26, 2012 at 02:52:09PM +0100, Lee Jones wrote:
On 26/07/12 12:38, Mark Brown wrote:
- pr_info("%s: Register platform-device 'ux500-pcm'\n", __func__);
- platform_device_register(&ux500_pcm);
This has the same issue as your last patch... the way you're doing things will break audio on all boards using this driver.
It will, why?
You've just removed registration of the device and not added anything else to replace that. Even if all boards convert to DT their DTs will need to be updated which you're not doing.
The initialisation function which calls platform_device_register() is only executed during a DT boot. The clue is in the title mop500_of_msp_init(). The DT is populated _before_ this patch, but I guess you mean if they are separated into subsystem trees and are placed into -next/Mainline out of order.
I will merge these patches with the DT population instead to overcome this possibility. It makes more sense to keep the arch/arm stuff together in any case.
In the initial submission of the MSP driver msp1 and msp3's associated pinctrl mechanism was passed back to platform code using a plat_init() call-back routine, but it has no place in platform code. The MSP driver should set this up for the appropriate ports. Instead we use a use_pinctrl identifier which is passed from platform_data/Device Tree which indicates which ports should use pinctrl.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/mach-ux500/board-mop500-msp.c | 101 ++------------------------------ arch/arm/mach-ux500/include/mach/msp.h | 3 +- sound/soc/ux500/ux500_msp_i2s.c | 81 ++++++++++++++++++++----- sound/soc/ux500/ux500_msp_i2s.h | 3 +- 4 files changed, 71 insertions(+), 117 deletions(-)
diff --git a/arch/arm/mach-ux500/board-mop500-msp.c b/arch/arm/mach-ux500/board-mop500-msp.c index 890feb3..391c129 100644 --- a/arch/arm/mach-ux500/board-mop500-msp.c +++ b/arch/arm/mach-ux500/board-mop500-msp.c @@ -23,53 +23,6 @@ #include "devices-db8500.h" #include "pins-db8500.h"
-/* MSP1/3 Tx/Rx usage protection */ -static DEFINE_SPINLOCK(msp_rxtx_lock); - -/* Reference Count */ -static int msp_rxtx_ref; - -/* Pin modes */ -struct pinctrl *msp1_p; -struct pinctrl_state *msp1_def; -struct pinctrl_state *msp1_sleep; - -int msp13_i2s_init(void) -{ - int retval = 0; - unsigned long flags; - - spin_lock_irqsave(&msp_rxtx_lock, flags); - if (msp_rxtx_ref == 0 && !(IS_ERR(msp1_p) || IS_ERR(msp1_def))) { - retval = pinctrl_select_state(msp1_p, msp1_def); - if (retval) - pr_err("could not set MSP1 defstate\n"); - } - if (!retval) - msp_rxtx_ref++; - spin_unlock_irqrestore(&msp_rxtx_lock, flags); - - return retval; -} - -int msp13_i2s_exit(void) -{ - int retval = 0; - unsigned long flags; - - spin_lock_irqsave(&msp_rxtx_lock, flags); - WARN_ON(!msp_rxtx_ref); - msp_rxtx_ref--; - if (msp_rxtx_ref == 0 && !(IS_ERR(msp1_p) || IS_ERR(msp1_sleep))) { - retval = pinctrl_select_state(msp1_p, msp1_sleep); - if (retval) - pr_err("could not set MSP1 sleepstate\n"); - } - spin_unlock_irqrestore(&msp_rxtx_lock, flags); - - return retval; -} - static struct stedma40_chan_cfg msp0_dma_rx = { .high_priority = true, .dir = STEDMA40_PERIPH_TO_MEM, @@ -132,8 +85,7 @@ struct msp_i2s_platform_data msp1_platform_data = { .id = MSP_I2S_1, .msp_i2s_dma_rx = NULL, .msp_i2s_dma_tx = &msp1_dma_tx, - .msp_i2s_init = msp13_i2s_init, - .msp_i2s_exit = msp13_i2s_exit, + .use_pinctrl = true, };
static struct stedma40_chan_cfg msp2_dma_rx = { @@ -219,83 +171,38 @@ struct msp_i2s_platform_data msp3_platform_data = { .id = MSP_I2S_3, .msp_i2s_dma_rx = &msp1_dma_rx, .msp_i2s_dma_tx = NULL, - .msp_i2s_init = msp13_i2s_init, - .msp_i2s_exit = msp13_i2s_exit, + .use_pinctrl = true, };
/* Due for removal once the MSP driver has been fully DT:ed. */ void mop500_of_msp_init(struct device *parent) { - struct platform_device *msp1; - pr_info("Initialize MSP I2S-devices.\n"); db8500_add_msp_i2s(parent, 0, U8500_MSP0_BASE, IRQ_DB8500_MSP0, &msp0_platform_data); - msp1 = db8500_add_msp_i2s(parent, 1, U8500_MSP1_BASE, IRQ_DB8500_MSP1, + db8500_add_msp_i2s(parent, 1, U8500_MSP1_BASE, IRQ_DB8500_MSP1, &msp1_platform_data); db8500_add_msp_i2s(parent, 2, U8500_MSP2_BASE, IRQ_DB8500_MSP2, &msp2_platform_data); db8500_add_msp_i2s(parent, 3, U8500_MSP3_BASE, IRQ_DB8500_MSP1, &msp3_platform_data); - - /* Get the pinctrl handle for MSP1 */ - if (msp1) { - msp1_p = pinctrl_get(&msp1->dev); - if (IS_ERR(msp1_p)) - dev_err(&msp1->dev, "could not get MSP1 pinctrl\n"); - else { - msp1_def = pinctrl_lookup_state(msp1_p, - PINCTRL_STATE_DEFAULT); - if (IS_ERR(msp1_def)) { - dev_err(&msp1->dev, - "could not get MSP1 defstate\n"); - } - msp1_sleep = pinctrl_lookup_state(msp1_p, - PINCTRL_STATE_SLEEP); - if (IS_ERR(msp1_sleep)) - dev_err(&msp1->dev, - "could not get MSP1 idlestate\n"); - } - } }
void 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_mop500);
pr_info("Initialize MSP I2S-devices.\n"); db8500_add_msp_i2s(parent, 0, U8500_MSP0_BASE, IRQ_DB8500_MSP0, &msp0_platform_data); - msp1 = db8500_add_msp_i2s(parent, 1, U8500_MSP1_BASE, IRQ_DB8500_MSP1, + db8500_add_msp_i2s(parent, 1, U8500_MSP1_BASE, IRQ_DB8500_MSP1, &msp1_platform_data); db8500_add_msp_i2s(parent, 2, U8500_MSP2_BASE, IRQ_DB8500_MSP2, &msp2_platform_data); db8500_add_msp_i2s(parent, 3, U8500_MSP3_BASE, IRQ_DB8500_MSP1, &msp3_platform_data);
- /* Get the pinctrl handle for MSP1 */ - if (msp1) { - msp1_p = pinctrl_get(&msp1->dev); - if (IS_ERR(msp1_p)) - dev_err(&msp1->dev, "could not get MSP1 pinctrl\n"); - else { - msp1_def = pinctrl_lookup_state(msp1_p, - PINCTRL_STATE_DEFAULT); - if (IS_ERR(msp1_def)) { - dev_err(&msp1->dev, - "could not get MSP1 defstate\n"); - } - msp1_sleep = pinctrl_lookup_state(msp1_p, - PINCTRL_STATE_SLEEP); - if (IS_ERR(msp1_sleep)) - dev_err(&msp1->dev, - "could not get MSP1 idlestate\n"); - } - } - pr_info("%s: Register platform-device 'ux500-pcm'\n", __func__); platform_device_register(&ux500_pcm); } diff --git a/arch/arm/mach-ux500/include/mach/msp.h b/arch/arm/mach-ux500/include/mach/msp.h index 798be19..95b7deb 100644 --- a/arch/arm/mach-ux500/include/mach/msp.h +++ b/arch/arm/mach-ux500/include/mach/msp.h @@ -22,8 +22,7 @@ struct msp_i2s_platform_data { enum msp_i2s_id id; struct stedma40_chan_cfg *msp_i2s_dma_rx; struct stedma40_chan_cfg *msp_i2s_dma_tx; - int (*msp_i2s_init) (void); - int (*msp_i2s_exit) (void); + bool use_pinctrl; };
#endif diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c index 36be11e..72ad6e8 100644 --- a/sound/soc/ux500/ux500_msp_i2s.c +++ b/sound/soc/ux500/ux500_msp_i2s.c @@ -15,6 +15,7 @@
#include <linux/module.h> #include <linux/platform_device.h> +#include <linux/pinctrl/consumer.h> #include <linux/delay.h> #include <linux/slab.h>
@@ -25,6 +26,17 @@
#include "ux500_msp_i2s.h"
+/* MSP1/3 Tx/Rx usage protection */ +static DEFINE_SPINLOCK(msp_rxtx_lock); + +/* Pin modes */ +struct pinctrl *pinctrl_p; +struct pinctrl_state *pinctrl_def; +struct pinctrl_state *pinctrl_sleep; + +/* Reference Count */ +int pinctrl_rxtx_ref; + /* Protocol desciptors */ static const struct msp_protdesc prot_descs[] = { { /* I2S */ @@ -352,17 +364,23 @@ static int configure_multichannel(struct ux500_msp *msp,
static int enable_msp(struct ux500_msp *msp, struct ux500_msp_config *config) { - int status = 0; + int status = 0, retval = 0; u32 reg_val_DMACR, reg_val_GCR; + unsigned long flags;
/* Check msp state whether in RUN or CONFIGURED Mode */ - if ((msp->msp_state == MSP_STATE_IDLE) && (msp->plat_init)) { - status = msp->plat_init(); - if (status) { - dev_err(msp->dev, "%s: ERROR: Failed to init MSP (%d)!\n", - __func__, status); - return status; + if (msp->msp_state == MSP_STATE_IDLE && msp->use_pinctrl) { + spin_lock_irqsave(&msp_rxtx_lock, flags); + if (pinctrl_rxtx_ref == 0 && + !(IS_ERR(pinctrl_p) || IS_ERR(pinctrl_def))) { + retval = pinctrl_select_state(pinctrl_p, + pinctrl_def); + if (retval) + pr_err("could not set MSP defstate\n"); } + if (!retval) + pinctrl_rxtx_ref++; + spin_unlock_irqrestore(&msp_rxtx_lock, flags); }
/* Configure msp with protocol dependent settings */ @@ -620,7 +638,8 @@ int ux500_msp_i2s_trigger(struct ux500_msp *msp, int cmd, int direction)
int ux500_msp_i2s_close(struct ux500_msp *msp, unsigned int dir) { - int status = 0; + int status = 0, retval = 0; + unsigned long flags;
dev_dbg(msp->dev, "%s: Enter (dir = 0x%01x).\n", __func__, dir);
@@ -631,12 +650,19 @@ int ux500_msp_i2s_close(struct ux500_msp *msp, unsigned int dir) writel((readl(msp->registers + MSP_GCR) & (~(FRAME_GEN_ENABLE | SRG_ENABLE))), msp->registers + MSP_GCR); - if (msp->plat_exit) - status = msp->plat_exit(); - if (status) - dev_warn(msp->dev, - "%s: WARN: ux500_msp_i2s_exit failed (%d)!\n", - __func__, status); + + spin_lock_irqsave(&msp_rxtx_lock, flags); + WARN_ON(!pinctrl_rxtx_ref); + pinctrl_rxtx_ref--; + if (msp->use_pinctrl && pinctrl_rxtx_ref == 0 && + !(IS_ERR(pinctrl_p) || IS_ERR(pinctrl_sleep))) { + retval = pinctrl_select_state(pinctrl_p, + pinctrl_sleep); + if (retval) + pr_err("could not set MSP sleepstate\n"); + } + spin_unlock_irqrestore(&msp_rxtx_lock, flags); + writel(0, msp->registers + MSP_GCR); writel(0, msp->registers + MSP_TCF); writel(0, msp->registers + MSP_RCF); @@ -667,6 +693,7 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev, struct resource *res = NULL; struct i2s_controller *i2s_cont; struct ux500_msp *msp; + static int initialised = false;
dev_dbg(&pdev->dev, "%s: Enter (name: %s, id: %d).\n", __func__, pdev->name, platform_data->id); @@ -678,8 +705,7 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,
msp->id = platform_data->id; msp->dev = &pdev->dev; - msp->plat_init = platform_data->msp_i2s_init; - msp->plat_exit = platform_data->msp_i2s_exit; + msp->use_pinctrl = platform_data->use_pinctrl; msp->dma_cfg_rx = platform_data->msp_i2s_dma_rx; msp->dma_cfg_tx = platform_data->msp_i2s_dma_tx;
@@ -717,6 +743,29 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev, dev_dbg(&pdev->dev, "I2S device-name: '%s'\n", i2s_cont->name); msp->i2s_cont = i2s_cont;
+ /* MSP1 and MSP3 share pins, so we only need to initialise pinctrl once. */ + if (msp->use_pinctrl && !initialised) { + pinctrl_p = pinctrl_get(msp->dev); + if (IS_ERR(pinctrl_p)) + dev_err(&pdev->dev, "could not get MSP pinctrl\n"); + else { + pinctrl_def = pinctrl_lookup_state(pinctrl_p, + PINCTRL_STATE_DEFAULT); + if (IS_ERR(pinctrl_def)) { + dev_err(&pdev->dev, + "could not get MSP defstate (%li)\n", + PTR_ERR(pinctrl_def)); + } + pinctrl_sleep = pinctrl_lookup_state(pinctrl_p, + PINCTRL_STATE_SLEEP); + if (IS_ERR(pinctrl_sleep)) + dev_err(&pdev->dev, + "could not get MSP idlestate (%li)\n", + PTR_ERR(pinctrl_def)); + } + initialised = true; + } + return 0;
err_i2s_cont: diff --git a/sound/soc/ux500/ux500_msp_i2s.h b/sound/soc/ux500/ux500_msp_i2s.h index 2d9136d..aec02b5 100644 --- a/sound/soc/ux500/ux500_msp_i2s.h +++ b/sound/soc/ux500/ux500_msp_i2s.h @@ -524,14 +524,13 @@ struct ux500_msp { struct dma_chan *rx_pipeid; enum msp_state msp_state; int (*transfer) (struct ux500_msp *msp, struct i2s_message *message); - int (*plat_init) (void); - int (*plat_exit) (void); struct timer_list notify_timer; int def_elem_len; unsigned int dir_busy; int loopback_enable; u32 backup_regs[MAX_MSP_BACKUP_REGS]; unsigned int f_bitclk; + bool use_pinctrl; };
struct ux500_msp_dma_params {
Pass registration of both parts of the MSP driver from platform code to Device Tree so that they are probed when Device Tree is enabled. Also, as there is platform data involved, we ensure that there is allocated memory to place the configuration into and that the correct information is extracted from the DT binary.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/mach-ux500/board-mop500-msp.c | 14 -------------- arch/arm/mach-ux500/board-mop500.c | 2 -- arch/arm/mach-ux500/board-mop500.h | 2 -- sound/soc/ux500/ux500_msp_dai.c | 6 ++++++ sound/soc/ux500/ux500_msp_i2s.c | 33 +++++++++++++++++++++++++++++--- 5 files changed, 36 insertions(+), 21 deletions(-)
diff --git a/arch/arm/mach-ux500/board-mop500-msp.c b/arch/arm/mach-ux500/board-mop500-msp.c index 391c129..c20d5d2 100644 --- a/arch/arm/mach-ux500/board-mop500-msp.c +++ b/arch/arm/mach-ux500/board-mop500-msp.c @@ -174,20 +174,6 @@ struct msp_i2s_platform_data msp3_platform_data = { .use_pinctrl = true, };
-/* Due for removal once the MSP driver has been fully DT:ed. */ -void mop500_of_msp_init(struct device *parent) -{ - pr_info("Initialize MSP I2S-devices.\n"); - db8500_add_msp_i2s(parent, 0, U8500_MSP0_BASE, IRQ_DB8500_MSP0, - &msp0_platform_data); - db8500_add_msp_i2s(parent, 1, U8500_MSP1_BASE, IRQ_DB8500_MSP1, - &msp1_platform_data); - db8500_add_msp_i2s(parent, 2, U8500_MSP2_BASE, IRQ_DB8500_MSP2, - &msp2_platform_data); - db8500_add_msp_i2s(parent, 3, U8500_MSP3_BASE, IRQ_DB8500_MSP1, - &msp3_platform_data); -} - void mop500_msp_init(struct device *parent) { pr_info("%s: Register platform-device 'snd-soc-u8500'.\n", __func__); diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c index d455a61..2ef26c8 100644 --- a/arch/arm/mach-ux500/board-mop500.c +++ b/arch/arm/mach-ux500/board-mop500.c @@ -810,8 +810,6 @@ 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 diff --git a/arch/arm/mach-ux500/board-mop500.h b/arch/arm/mach-ux500/board-mop500.h index 3fbf48f..e56c983 100644 --- a/arch/arm/mach-ux500/board-mop500.h +++ b/arch/arm/mach-ux500/board-mop500.h @@ -98,8 +98,6 @@ void __init mop500_pinmaps_init(void); void __init snowball_pinmaps_init(void); void __init hrefv60_pinmaps_init(void); void mop500_msp_init(struct device *parent); -/* Due for removal once the MSP driver has been fully DT:ed. */ -void mop500_of_msp_init(struct device *parent);
int __init mop500_uib_init(void); void mop500_uib_i2c_add(int busnum, struct i2c_board_info *info, diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c index 772cb19..0f7dd49 100644 --- a/sound/soc/ux500/ux500_msp_dai.c +++ b/sound/soc/ux500/ux500_msp_dai.c @@ -833,10 +833,16 @@ static int __devexit ux500_msp_drv_remove(struct platform_device *pdev) return 0; }
+static const struct of_device_id ux500_msp_i2c_match[] = { + { .compatible = "stericsson,ux500-msp-i2s", }, + {}, +}; + static struct platform_driver msp_i2s_driver = { .driver = { .name = "ux500-msp-i2s", .owner = THIS_MODULE, + .of_match_table = ux500_msp_i2c_match, }, .probe = ux500_msp_drv_probe, .remove = ux500_msp_drv_remove, diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c index 72ad6e8..1ecdec0 100644 --- a/sound/soc/ux500/ux500_msp_i2s.c +++ b/sound/soc/ux500/ux500_msp_i2s.c @@ -18,6 +18,7 @@ #include <linux/pinctrl/consumer.h> #include <linux/delay.h> #include <linux/slab.h> +#include <linux/of.h>
#include <mach/hardware.h> #include <mach/msp.h> @@ -685,6 +686,16 @@ int ux500_msp_i2s_close(struct ux500_msp *msp, unsigned int dir)
}
+void ux500_msp_i2s_of_init_msp(struct platform_device *pdev, + struct ux500_msp *msp, + struct device_node *np) +{ + if (of_get_property(np, "stericsson,use-pinctrl", NULL)) + msp->use_pinctrl = true; + else + msp->use_pinctrl = false; +} + int ux500_msp_i2s_init_msp(struct platform_device *pdev, struct ux500_msp **msp_p, struct msp_i2s_platform_data *platform_data) @@ -692,17 +703,33 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev, int ret = 0; struct resource *res = NULL; struct i2s_controller *i2s_cont; + struct device_node *np = pdev->dev.of_node; struct ux500_msp *msp; static int initialised = false;
- dev_dbg(&pdev->dev, "%s: Enter (name: %s, id: %d).\n", __func__, - pdev->name, platform_data->id); - *msp_p = devm_kzalloc(&pdev->dev, sizeof(struct ux500_msp), GFP_KERNEL); msp = *msp_p; if (!msp) return -ENOMEM;
+ if (np) { + if (!platform_data) { + platform_data = devm_kzalloc(&pdev->dev, + sizeof(struct msp_i2s_platform_data), GFP_KERNEL); + if (!platform_data) + ret = -ENOMEM; + } + ux500_msp_i2s_of_init_msp(pdev, msp, np); + } else + if (!platform_data) + ret = -EINVAL; + + if (ret) + goto err_res; + + dev_dbg(&pdev->dev, "%s: Enter (name: %s, id: %d).\n", __func__, + pdev->name, platform_data->id); + msp->id = platform_data->id; msp->dev = &pdev->dev; msp->use_pinctrl = platform_data->use_pinctrl;
On Thu, Jul 26, 2012 at 11:28:52AM +0100, Lee Jones wrote:
arch/arm/mach-ux500/board-mop500-msp.c | 14 -------------- arch/arm/mach-ux500/board-mop500.c | 2 -- arch/arm/mach-ux500/board-mop500.h | 2 -- sound/soc/ux500/ux500_msp_dai.c | 6 ++++++ sound/soc/ux500/ux500_msp_i2s.c | 33 +++++++++++++++++++++++++++++---
Always provide documentation for DT bindings.
+void ux500_msp_i2s_of_init_msp(struct platform_device *pdev,
struct ux500_msp *msp,struct device_node *np)+{
- if (of_get_property(np, "stericsson,use-pinctrl", NULL))
msp->use_pinctrl = true;- else
msp->use_pinctrl = false;+}
This doesn't seem particularly sane... why is this conditional?
We continue to allow the AB8500 CODEC to be registered via the AB8500 Multi Functional Device API, only this time we extract its configuration from the Device Tree binary.
Signed-off-by: Lee Jones lee.jones@linaro.org --- drivers/mfd/ab8500-core.c | 1 + include/linux/mfd/abx500/ab8500-codec.h | 6 ++- sound/soc/codecs/ab8500-codec.c | 79 +++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c index 626b4ec..0c5b70f 100644 --- a/drivers/mfd/ab8500-core.c +++ b/drivers/mfd/ab8500-core.c @@ -1076,6 +1076,7 @@ static struct mfd_cell __devinitdata ab8500_devs[] = { }, { .name = "ab8500-codec", + .of_compatible = "stericsson,ab8500-codec", }, };
diff --git a/include/linux/mfd/abx500/ab8500-codec.h b/include/linux/mfd/abx500/ab8500-codec.h index dc65292..d707941 100644 --- a/include/linux/mfd/abx500/ab8500-codec.h +++ b/include/linux/mfd/abx500/ab8500-codec.h @@ -23,7 +23,8 @@ enum amic_type { /* Mic-biases */ enum amic_micbias { AMIC_MICBIAS_VAMIC1, - AMIC_MICBIAS_VAMIC2 + AMIC_MICBIAS_VAMIC2, + AMIC_MICBIAS_UNKNOWN };
/* Bias-voltage */ @@ -31,7 +32,8 @@ enum ear_cm_voltage { EAR_CMV_0_95V, EAR_CMV_1_10V, EAR_CMV_1_27V, - EAR_CMV_1_58V + EAR_CMV_1_58V, + EAR_CMV_UNKNOWN };
/* Analog microphone settings */ diff --git a/sound/soc/codecs/ab8500-codec.c b/sound/soc/codecs/ab8500-codec.c index 3c79592..e3ae9f5 100644 --- a/sound/soc/codecs/ab8500-codec.c +++ b/sound/soc/codecs/ab8500-codec.c @@ -34,6 +34,7 @@ #include <linux/mfd/abx500/ab8500-sysctrl.h> #include <linux/mfd/abx500/ab8500-codec.h> #include <linux/regulator/consumer.h> +#include <linux/of.h>
#include <sound/core.h> #include <sound/pcm.h> @@ -2394,9 +2395,62 @@ struct snd_soc_dai_driver ab8500_codec_dai[] = { } };
+static void ab8500_codec_of_probe(struct device *dev, struct device_node *np, + struct ab8500_codec_platform_data *codec) +{ + u32 value; + + if (of_get_property(np, "stericsson,amic1-type-single-ended", NULL)) + codec->amics.mic1_type = AMIC_TYPE_SINGLE_ENDED; + else + codec->amics.mic1_type = AMIC_TYPE_DIFFERENTIAL; + + if (of_get_property(np, "stericsson,amic2-type-single-ended", NULL)) + codec->amics.mic2_type = AMIC_TYPE_SINGLE_ENDED; + else + codec->amics.mic2_type = AMIC_TYPE_DIFFERENTIAL; + + /* Has a non-standard Vamic been requested? */ + if(of_get_property(np, "stericsson,amic1a-bias-vamic2", NULL)) + codec->amics.mic1a_micbias = AMIC_MICBIAS_VAMIC2; + else + codec->amics.mic1a_micbias = AMIC_MICBIAS_VAMIC1; + + if (of_get_property(np, "stericsson,amic1b-bias-vamic2", NULL)) + codec->amics.mic1b_micbias = AMIC_MICBIAS_VAMIC2; + else + codec->amics.mic1b_micbias = AMIC_MICBIAS_VAMIC1; + + if (of_get_property(np, "stericsson,amic2-bias-vamic1", NULL)) + codec->amics.mic2_micbias = AMIC_MICBIAS_VAMIC1; + else + codec->amics.mic2_micbias = AMIC_MICBIAS_VAMIC2; + + if (!of_property_read_u32(np, "stericsson,earpeice-cmv", &value)) { + switch (value) { + case 950 : + codec->ear_cmv = EAR_CMV_0_95V; + break; + case 1100 : + codec->ear_cmv = EAR_CMV_1_10V; + break; + case 1270 : + codec->ear_cmv = EAR_CMV_1_27V; + break; + case 1580 : + codec->ear_cmv = EAR_CMV_1_58V; + break; + default : + codec->ear_cmv = EAR_CMV_UNKNOWN; + dev_err(dev, "Unsuitable earpiece voltage found in DT\n"); + } + } +} + static int ab8500_codec_probe(struct snd_soc_codec *codec) { struct device *dev = codec->dev; + struct device_node *np = dev->of_node; struct ab8500_codec_drvdata *drvdata = dev_get_drvdata(dev); struct ab8500_platform_data *pdata; struct filter_control *fc; @@ -2406,6 +2460,31 @@ 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); + + if (np) { + if (!pdata) + pdata = devm_kzalloc(dev, + sizeof(struct ab8500_platform_data), + GFP_KERNEL); + + if (!pdata->codec) + pdata->codec + = devm_kzalloc(dev, + sizeof(struct ab8500_codec_platform_data), + GFP_KERNEL); + + if (!(pdata && pdata->codec)) + return -ENOMEM; + + ab8500_codec_of_probe(dev, np, pdata->codec); + + } else { + if (!(pdata && pdata->codec)) { + dev_err(dev, "No codec platform data or DT found\n"); + return -EINVAL; + } + } + status = ab8500_audio_setup_mics(codec, &pdata->codec->amics); if (status < 0) { pr_err("%s: Failed to setup mics (%d)!\n", __func__, status);
On Thu, Jul 26, 2012 at 11:28:53AM +0100, Lee Jones wrote:
drivers/mfd/ab8500-core.c | 1 + include/linux/mfd/abx500/ab8500-codec.h | 6 ++- sound/soc/codecs/ab8500-codec.c | 79 +++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 2 deletions(-)
Yet again no binding documentation....
{ .name = "ab8500-codec",
.of_compatible = "stericsson,ab8500-codec",
Why are we doing this? The MFD cells are a totally Linux specific thing, there's no reason to represent them in the device tree unless they're in some way reusable and the "ab8500-codec" name suggests that's unlikely. Just put the properties on the parent node and instantiate the MFD cell as normal.
- /* Has a non-standard Vamic been requested? */
- if(of_get_property(np, "stericsson,amic1a-bias-vamic2", NULL))
Coding style.
- if (!of_property_read_u32(np, "stericsson,earpeice-cmv", &value)) {
switch (value) {case 950 :codec->ear_cmv = EAR_CMV_0_95V;break;case 1100 :codec->ear_cmv = EAR_CMV_1_10V;break;case 1270 :codec->ear_cmv = EAR_CMV_1_27V;break;case 1580 :codec->ear_cmv = EAR_CMV_1_58V;break;default :codec->ear_cmv = EAR_CMV_UNKNOWN;dev_err(dev, "Unsuitable earpiece voltage found in DT\n");
The platform data code picks a default, can't the DT code do the same?
On 26/07/12 12:50, Mark Brown wrote:
On Thu, Jul 26, 2012 at 11:28:53AM +0100, Lee Jones wrote:
drivers/mfd/ab8500-core.c | 1 + include/linux/mfd/abx500/ab8500-codec.h | 6 ++- sound/soc/codecs/ab8500-codec.c | 79 +++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 2 deletions(-)
Yet again no binding documentation....
RFC. ;)
I'll write the documentation when/if the properties are accepted.
{ .name = "ab8500-codec",
.of_compatible = "stericsson,ab8500-codec",Why are we doing this? The MFD cells are a totally Linux specific thing, there's no reason to represent them in the device tree unless they're in some way reusable and the "ab8500-codec" name suggests that's unlikely. Just put the properties on the parent node and instantiate the MFD cell as normal.
- /* Has a non-standard Vamic been requested? */
- if(of_get_property(np, "stericsson,amic1a-bias-vamic2", NULL))
Coding style.
Missing space? Sorry, typo, I'll change.
- if (!of_property_read_u32(np, "stericsson,earpeice-cmv", &value)) {
switch (value) {case 950 :codec->ear_cmv = EAR_CMV_0_95V;break;case 1100 :codec->ear_cmv = EAR_CMV_1_10V;break;case 1270 :codec->ear_cmv = EAR_CMV_1_27V;break;case 1580 :codec->ear_cmv = EAR_CMV_1_58V;break;default :codec->ear_cmv = EAR_CMV_UNKNOWN;dev_err(dev, "Unsuitable earpiece voltage found in DT\n");The platform data code picks a default, can't the DT code do the same?
No, I don't think that it does? The original code returns -EINVAL unless a value is specified.
The original author is keen to have a clear error message in case users try to specify non-exact values. I'd rather we fail-out than use incorrect values which would be a great deal harder for a user to debug.
On Thu, Jul 26, 2012 at 03:00:17PM +0100, Lee Jones wrote:
On 26/07/12 12:50, Mark Brown wrote:
Yet again no binding documentation....
RFC. ;)
I'll write the documentation when/if the properties are accepted.
No, write the documentation. It's way too much effort to reverse engineer the bindings from the code.
default :codec->ear_cmv = EAR_CMV_UNKNOWN;dev_err(dev, "Unsuitable earpiece voltage found in DT\n");
The platform data code picks a default, can't the DT code do the same?
No, I don't think that it does? The original code returns -EINVAL unless a value is specified.
The code doesn't specify values for the enumeration so it ought to default to EAR_CMV_0_95V if nothing is specified.
The original author is keen to have a clear error message in case users try to specify non-exact values. I'd rather we fail-out than use incorrect values which would be a great deal harder for a user to debug.
By that argument all the properties should be mandatory but it's only this one IIRC.
On 26/07/12 15:28, Mark Brown wrote:
On Thu, Jul 26, 2012 at 03:00:17PM +0100, Lee Jones wrote:
On 26/07/12 12:50, Mark Brown wrote:
Yet again no binding documentation....
RFC. ;)
I'll write the documentation when/if the properties are accepted.
No, write the documentation. It's way too much effort to reverse engineer the bindings from the code.
default :codec->ear_cmv = EAR_CMV_UNKNOWN;dev_err(dev, "Unsuitable earpiece voltage found in DT\n");The platform data code picks a default, can't the DT code do the same?
No, I don't think that it does? The original code returns -EINVAL unless a value is specified.
The code doesn't specify values for the enumeration so it ought to default to EAR_CMV_0_95V if nothing is specified.
Ah, I see what you mean. I guess we could compromise and print a warning _and_ fall back to the 0th original emum.
The original author is keen to have a clear error message in case users try to specify non-exact values. I'd rather we fail-out than use incorrect values which would be a great deal harder for a user to debug.
By that argument all the properties should be mandatory but it's only this one IIRC.
This is the only value which the user can pick an obscure value, such as 913, thinking they can pick 913mV. I'm happy to fall-back, as long as Ola is too.
On Thu, Jul 26, 2012 at 04:01:14PM +0100, Lee Jones wrote:
This is the only value which the user can pick an obscure value, such as 913, thinking they can pick 913mV. I'm happy to fall-back, as long as Ola is too.
Erroring out if they pick an invalid value is fine, I'm more concerned with the case where no property is supplied at all.
On 26/07/12 16:14, Mark Brown wrote:
On Thu, Jul 26, 2012 at 04:01:14PM +0100, Lee Jones wrote:
This is the only value which the user can pick an obscure value, such as 913, thinking they can pick 913mV. I'm happy to fall-back, as long as Ola is too.
Erroring out if they pick an invalid value is fine, I'm more concerned with the case where no property is supplied at all.
Hmmm... I'll have a think.
Sorry missed this:
{ .name = "ab8500-codec",
.of_compatible = "stericsson,ab8500-codec",Why are we doing this? The MFD cells are a totally Linux specific thing, there's no reason to represent them in the device tree unless they're in some way reusable and the "ab8500-codec" name suggests that's unlikely. Just put the properties on the parent node and instantiate the MFD cell as normal.
We have all of the AB8500 devices into the Device Tree to accurately represent the hardware. We will also be passing configuration information into the AB8500 Codec from Device Tree. The only reason we're still registering them using the MFD API is to overcome addressing issues encountered earlier. Each 'device' still belongs in the 'device' tree.
If we were to take this Device Tree and use it on something non-Linux, that OS will still need to know about each of the AB8500 devices and every associated configuration option. Only in Linux do we continue to register them though a different API, which doesn't affect any other OS.
On Thu, Jul 26, 2012 at 03:15:01PM +0100, Lee Jones wrote:
Sorry missed this:
Why are we doing this? The MFD cells are a totally Linux specific thing, there's no reason to represent them in the device tree unless they're in some way reusable and the "ab8500-codec" name suggests that's unlikely. Just put the properties on the parent node and instantiate the MFD cell as normal.
We have all of the AB8500 devices into the Device Tree to accurately represent the hardware. We will also be passing configuration information into the AB8500 Codec from Device Tree. The only reason we're still registering them using the MFD API is to overcome addressing issues encountered earlier. Each 'device' still belongs in the 'device' tree.
The device here is the AB8500. The fact that Linux chooses to represent it as an MFD with a particular set of subdrivers is a Linux specific decision and may well change over time. For example it's likely that we'll want to migrate the clocks out of the audio driver and into the clock API when that becomes useful. Similarly currently a lot of these devices use ASoC level jack detection but that's going to move over to extcon over time.
There's no way you're going to be able to reuse this for anything that isn't an AB8500, there's no abstraction of the SoC integration here. If you had clearly identifiable, repeatable IPs which you could reasonably bind to a different bit of silicon then that'd be different but there's nothing like that here. We already know that the functionality covered by the driver is going to be present simply by virtue of knowing that there's an AB8500 and similarly there's no real way this driver could ever be used without the core driver. All the "device" in the device tree is doing is serving as a container to place some of the DT properties into, this needs to be separated out from the instantiation of the Linux device driver. There's nothing stopping the driver from looking at the OF node of its parent here.
The goal here isn't just to copy the Linux device model and platform data into device tree bindings, the device tree bindings need to think about what the chip actually is so they can be reused by other OSs and by future versions of Linux.
If we were to take this Device Tree and use it on something non-Linux, that OS will still need to know about each of the AB8500 devices and every associated configuration option. Only in Linux do we continue to register them though a different API, which doesn't affect any other OS.
Another OS might have a different idea about how it's going to split up the chip which better fits with the models which that OS has for the functions present on the device. The reason this is a distinct device in Linux is all to do with how Linux models the hardware.
On 26/07/12 15:43, Mark Brown wrote:
On Thu, Jul 26, 2012 at 03:15:01PM +0100, Lee Jones wrote:
Sorry missed this:
Why are we doing this? The MFD cells are a totally Linux specific thing, there's no reason to represent them in the device tree unless they're in some way reusable and the "ab8500-codec" name suggests that's unlikely. Just put the properties on the parent node and instantiate the MFD cell as normal.
We have all of the AB8500 devices into the Device Tree to accurately represent the hardware. We will also be passing configuration information into the AB8500 Codec from Device Tree. The only reason we're still registering them using the MFD API is to overcome addressing issues encountered earlier. Each 'device' still belongs in the 'device' tree.
The device here is the AB8500. The fact that Linux chooses to represent it as an MFD with a particular set of subdrivers is a Linux specific decision and may well change over time. For example it's likely that we'll want to migrate the clocks out of the audio driver and into the clock API when that becomes useful. Similarly currently a lot of these devices use ASoC level jack detection but that's going to move over to extcon over time.
There's no way you're going to be able to reuse this for anything that isn't an AB8500, there's no abstraction of the SoC integration here. If you had clearly identifiable, repeatable IPs which you could reasonably bind to a different bit of silicon then that'd be different but there's nothing like that here. We already know that the functionality covered by the driver is going to be present simply by virtue of knowing that there's an AB8500 and similarly there's no real way this driver could ever be used without the core driver. All the "device" in the device tree is doing is serving as a container to place some of the DT properties into, this needs to be separated out from the instantiation of the Linux device driver. There's nothing stopping the driver from looking at the OF node of its parent here.
The goal here isn't just to copy the Linux device model and platform data into device tree bindings, the device tree bindings need to think about what the chip actually is so they can be reused by other OSs and by future versions of Linux.
If we were to take this Device Tree and use it on something non-Linux, that OS will still need to know about each of the AB8500 devices and every associated configuration option. Only in Linux do we continue to register them though a different API, which doesn't affect any other OS.
Another OS might have a different idea about how it's going to split up the chip which better fits with the models which that OS has for the functions present on the device. The reason this is a distinct device in Linux is all to do with how Linux models the hardware.
Okay, so your suggestion is to strip out all of the sub-devices under the AB8500. It's doable, but will take some restructuring and thinking about. This is a job for another day. I think it's okay to continue with the current semantics for the time-being. The line we're discussing does need to be split out though. I didn't mean to merge it in with the ASoC stuff.
On Thu, Jul 26, 2012 at 04:19:33PM +0100, Lee Jones wrote:
Okay, so your suggestion is to strip out all of the sub-devices under the AB8500. It's doable, but will take some restructuring and thinking about. This is a job for another day. I think it's okay to continue with the current semantics for the time-being. The line we're discussing does need to be split out though. I didn't mean to merge it in with the ASoC stuff.
Yes, well any that aren't reusable at any rate. If you could relocate the device into another SoC integation that'd be different.
The 'msp' board file does more than just register MSP devices. It also registers some other components necessary to get audio working on ux500 based platforms; such as the PCM and Machine Drivers. For that reason we're changing the filename to be more encompassing - 'audio'.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/mach-ux500/Makefile | 2 +- arch/arm/mach-ux500/board-mop500-audio.c | 194 ++++++++++++++++++++++++++++++ arch/arm/mach-ux500/board-mop500-msp.c | 194 ------------------------------ arch/arm/mach-ux500/board-mop500.c | 10 +- arch/arm/mach-ux500/board-mop500.h | 2 +- 5 files changed, 201 insertions(+), 201 deletions(-) create mode 100644 arch/arm/mach-ux500/board-mop500-audio.c delete mode 100644 arch/arm/mach-ux500/board-mop500-msp.c
diff --git a/arch/arm/mach-ux500/Makefile b/arch/arm/mach-ux500/Makefile index 026086f..1dc2cfa 100644 --- a/arch/arm/mach-ux500/Makefile +++ b/arch/arm/mach-ux500/Makefile @@ -12,6 +12,6 @@ obj-$(CONFIG_MACH_MOP500) += board-mop500.o board-mop500-sdi.o \ board-mop500-uib.o board-mop500-stuib.o \ board-mop500-u8500uib.o \ board-mop500-pins.o \ - board-mop500-msp.o + board-mop500-audio.o obj-$(CONFIG_SMP) += platsmp.o headsmp.o obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o diff --git a/arch/arm/mach-ux500/board-mop500-audio.c b/arch/arm/mach-ux500/board-mop500-audio.c new file mode 100644 index 0000000..4583846 --- /dev/null +++ b/arch/arm/mach-ux500/board-mop500-audio.c @@ -0,0 +1,194 @@ +/* + * Copyright (C) ST-Ericsson SA 2010 + * + * License terms: GNU General Public License (GPL), version 2 + */ + +#include <linux/platform_device.h> +#include <linux/init.h> +#include <linux/gpio.h> +#include <linux/pinctrl/consumer.h> + +#include <plat/gpio-nomadik.h> +#include <plat/pincfg.h> +#include <plat/ste_dma40.h> + +#include <mach/devices.h> +#include <mach/hardware.h> +#include <mach/irqs.h> +#include <mach/msp.h> + +#include "ste-dma40-db8500.h" +#include "board-mop500.h" +#include "devices-db8500.h" +#include "pins-db8500.h" + +static struct stedma40_chan_cfg msp0_dma_rx = { + .high_priority = true, + .dir = STEDMA40_PERIPH_TO_MEM, + + .src_dev_type = DB8500_DMA_DEV31_MSP0_RX_SLIM0_CH0_RX, + .dst_dev_type = STEDMA40_DEV_DST_MEMORY, + + .src_info.psize = STEDMA40_PSIZE_LOG_4, + .dst_info.psize = STEDMA40_PSIZE_LOG_4, + + /* data_width is set during configuration */ +}; + +static struct stedma40_chan_cfg msp0_dma_tx = { + .high_priority = true, + .dir = STEDMA40_MEM_TO_PERIPH, + + .src_dev_type = STEDMA40_DEV_DST_MEMORY, + .dst_dev_type = DB8500_DMA_DEV31_MSP0_TX_SLIM0_CH0_TX, + + .src_info.psize = STEDMA40_PSIZE_LOG_4, + .dst_info.psize = STEDMA40_PSIZE_LOG_4, + + /* data_width is set during configuration */ +}; + +struct msp_i2s_platform_data msp0_platform_data = { + .id = MSP_I2S_0, + .msp_i2s_dma_rx = &msp0_dma_rx, + .msp_i2s_dma_tx = &msp0_dma_tx, +}; + +static struct stedma40_chan_cfg msp1_dma_rx = { + .high_priority = true, + .dir = STEDMA40_PERIPH_TO_MEM, + + .src_dev_type = DB8500_DMA_DEV30_MSP3_RX, + .dst_dev_type = STEDMA40_DEV_DST_MEMORY, + + .src_info.psize = STEDMA40_PSIZE_LOG_4, + .dst_info.psize = STEDMA40_PSIZE_LOG_4, + + /* data_width is set during configuration */ +}; + +static struct stedma40_chan_cfg msp1_dma_tx = { + .high_priority = true, + .dir = STEDMA40_MEM_TO_PERIPH, + + .src_dev_type = STEDMA40_DEV_DST_MEMORY, + .dst_dev_type = DB8500_DMA_DEV30_MSP1_TX, + + .src_info.psize = STEDMA40_PSIZE_LOG_4, + .dst_info.psize = STEDMA40_PSIZE_LOG_4, + + /* data_width is set during configuration */ +}; + +struct msp_i2s_platform_data msp1_platform_data = { + .id = MSP_I2S_1, + .msp_i2s_dma_rx = NULL, + .msp_i2s_dma_tx = &msp1_dma_tx, + .use_pinctrl = true, +}; + +static struct stedma40_chan_cfg msp2_dma_rx = { + .high_priority = true, + .dir = STEDMA40_PERIPH_TO_MEM, + + .src_dev_type = DB8500_DMA_DEV14_MSP2_RX, + .dst_dev_type = STEDMA40_DEV_DST_MEMORY, + + /* MSP2 DMA doesn't work with PSIZE == 4 on DB8500v2 */ + .src_info.psize = STEDMA40_PSIZE_LOG_1, + .dst_info.psize = STEDMA40_PSIZE_LOG_1, + + /* data_width is set during configuration */ +}; + +static struct stedma40_chan_cfg msp2_dma_tx = { + .high_priority = true, + .dir = STEDMA40_MEM_TO_PERIPH, + + .src_dev_type = STEDMA40_DEV_DST_MEMORY, + .dst_dev_type = DB8500_DMA_DEV14_MSP2_TX, + + .src_info.psize = STEDMA40_PSIZE_LOG_4, + .dst_info.psize = STEDMA40_PSIZE_LOG_4, + + .use_fixed_channel = true, + .phy_channel = 1, + + /* data_width is set during configuration */ +}; + +static struct platform_device *db8500_add_msp_i2s(struct device *parent, + int id, + resource_size_t base, int irq, + struct msp_i2s_platform_data *pdata) +{ + struct platform_device *pdev; + struct resource res[] = { + DEFINE_RES_MEM(base, SZ_4K), + DEFINE_RES_IRQ(irq), + }; + + pr_info("Register platform-device 'ux500-msp-i2s', id %d, irq %d\n", + id, irq); + pdev = platform_device_register_resndata(parent, "ux500-msp-i2s", id, + res, ARRAY_SIZE(res), + pdata, sizeof(*pdata)); + if (!pdev) { + pr_err("Failed to register platform-device 'ux500-msp-i2s.%d'!\n", + id); + return NULL; + } + + return pdev; +} + +/* Platform device for ASoC U8500 machine */ +static struct platform_device snd_soc_mop500 = { + .name = "snd-soc-mop500", + .id = 0, + .dev = { + .platform_data = NULL, + }, +}; + +/* Platform device for Ux500-PCM */ +static struct platform_device ux500_pcm = { + .name = "ux500-pcm", + .id = 0, + .dev = { + .platform_data = NULL, + }, +}; + +struct msp_i2s_platform_data msp2_platform_data = { + .id = MSP_I2S_2, + .msp_i2s_dma_rx = &msp2_dma_rx, + .msp_i2s_dma_tx = &msp2_dma_tx, +}; + +struct msp_i2s_platform_data msp3_platform_data = { + .id = MSP_I2S_3, + .msp_i2s_dma_rx = &msp1_dma_rx, + .msp_i2s_dma_tx = NULL, + .use_pinctrl = true, +}; + +void mop500_audio_init(struct device *parent) +{ + pr_info("%s: Register platform-device 'snd-soc-u8500'.\n", __func__); + 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, + &msp0_platform_data); + db8500_add_msp_i2s(parent, 1, U8500_MSP1_BASE, IRQ_DB8500_MSP1, + &msp1_platform_data); + db8500_add_msp_i2s(parent, 2, U8500_MSP2_BASE, IRQ_DB8500_MSP2, + &msp2_platform_data); + db8500_add_msp_i2s(parent, 3, U8500_MSP3_BASE, IRQ_DB8500_MSP1, + &msp3_platform_data); + + pr_info("%s: Register platform-device 'ux500-pcm'\n", __func__); + platform_device_register(&ux500_pcm); +} diff --git a/arch/arm/mach-ux500/board-mop500-msp.c b/arch/arm/mach-ux500/board-mop500-msp.c deleted file mode 100644 index c20d5d2..0000000 --- a/arch/arm/mach-ux500/board-mop500-msp.c +++ /dev/null @@ -1,194 +0,0 @@ -/* - * Copyright (C) ST-Ericsson SA 2010 - * - * License terms: GNU General Public License (GPL), version 2 - */ - -#include <linux/platform_device.h> -#include <linux/init.h> -#include <linux/gpio.h> -#include <linux/pinctrl/consumer.h> - -#include <plat/gpio-nomadik.h> -#include <plat/pincfg.h> -#include <plat/ste_dma40.h> - -#include <mach/devices.h> -#include <mach/hardware.h> -#include <mach/irqs.h> -#include <mach/msp.h> - -#include "ste-dma40-db8500.h" -#include "board-mop500.h" -#include "devices-db8500.h" -#include "pins-db8500.h" - -static struct stedma40_chan_cfg msp0_dma_rx = { - .high_priority = true, - .dir = STEDMA40_PERIPH_TO_MEM, - - .src_dev_type = DB8500_DMA_DEV31_MSP0_RX_SLIM0_CH0_RX, - .dst_dev_type = STEDMA40_DEV_DST_MEMORY, - - .src_info.psize = STEDMA40_PSIZE_LOG_4, - .dst_info.psize = STEDMA40_PSIZE_LOG_4, - - /* data_width is set during configuration */ -}; - -static struct stedma40_chan_cfg msp0_dma_tx = { - .high_priority = true, - .dir = STEDMA40_MEM_TO_PERIPH, - - .src_dev_type = STEDMA40_DEV_DST_MEMORY, - .dst_dev_type = DB8500_DMA_DEV31_MSP0_TX_SLIM0_CH0_TX, - - .src_info.psize = STEDMA40_PSIZE_LOG_4, - .dst_info.psize = STEDMA40_PSIZE_LOG_4, - - /* data_width is set during configuration */ -}; - -struct msp_i2s_platform_data msp0_platform_data = { - .id = MSP_I2S_0, - .msp_i2s_dma_rx = &msp0_dma_rx, - .msp_i2s_dma_tx = &msp0_dma_tx, -}; - -static struct stedma40_chan_cfg msp1_dma_rx = { - .high_priority = true, - .dir = STEDMA40_PERIPH_TO_MEM, - - .src_dev_type = DB8500_DMA_DEV30_MSP3_RX, - .dst_dev_type = STEDMA40_DEV_DST_MEMORY, - - .src_info.psize = STEDMA40_PSIZE_LOG_4, - .dst_info.psize = STEDMA40_PSIZE_LOG_4, - - /* data_width is set during configuration */ -}; - -static struct stedma40_chan_cfg msp1_dma_tx = { - .high_priority = true, - .dir = STEDMA40_MEM_TO_PERIPH, - - .src_dev_type = STEDMA40_DEV_DST_MEMORY, - .dst_dev_type = DB8500_DMA_DEV30_MSP1_TX, - - .src_info.psize = STEDMA40_PSIZE_LOG_4, - .dst_info.psize = STEDMA40_PSIZE_LOG_4, - - /* data_width is set during configuration */ -}; - -struct msp_i2s_platform_data msp1_platform_data = { - .id = MSP_I2S_1, - .msp_i2s_dma_rx = NULL, - .msp_i2s_dma_tx = &msp1_dma_tx, - .use_pinctrl = true, -}; - -static struct stedma40_chan_cfg msp2_dma_rx = { - .high_priority = true, - .dir = STEDMA40_PERIPH_TO_MEM, - - .src_dev_type = DB8500_DMA_DEV14_MSP2_RX, - .dst_dev_type = STEDMA40_DEV_DST_MEMORY, - - /* MSP2 DMA doesn't work with PSIZE == 4 on DB8500v2 */ - .src_info.psize = STEDMA40_PSIZE_LOG_1, - .dst_info.psize = STEDMA40_PSIZE_LOG_1, - - /* data_width is set during configuration */ -}; - -static struct stedma40_chan_cfg msp2_dma_tx = { - .high_priority = true, - .dir = STEDMA40_MEM_TO_PERIPH, - - .src_dev_type = STEDMA40_DEV_DST_MEMORY, - .dst_dev_type = DB8500_DMA_DEV14_MSP2_TX, - - .src_info.psize = STEDMA40_PSIZE_LOG_4, - .dst_info.psize = STEDMA40_PSIZE_LOG_4, - - .use_fixed_channel = true, - .phy_channel = 1, - - /* data_width is set during configuration */ -}; - -static struct platform_device *db8500_add_msp_i2s(struct device *parent, - int id, - resource_size_t base, int irq, - struct msp_i2s_platform_data *pdata) -{ - struct platform_device *pdev; - struct resource res[] = { - DEFINE_RES_MEM(base, SZ_4K), - DEFINE_RES_IRQ(irq), - }; - - pr_info("Register platform-device 'ux500-msp-i2s', id %d, irq %d\n", - id, irq); - pdev = platform_device_register_resndata(parent, "ux500-msp-i2s", id, - res, ARRAY_SIZE(res), - pdata, sizeof(*pdata)); - if (!pdev) { - pr_err("Failed to register platform-device 'ux500-msp-i2s.%d'!\n", - id); - return NULL; - } - - return pdev; -} - -/* Platform device for ASoC U8500 machine */ -static struct platform_device snd_soc_mop500 = { - .name = "snd-soc-mop500", - .id = 0, - .dev = { - .platform_data = NULL, - }, -}; - -/* Platform device for Ux500-PCM */ -static struct platform_device ux500_pcm = { - .name = "ux500-pcm", - .id = 0, - .dev = { - .platform_data = NULL, - }, -}; - -struct msp_i2s_platform_data msp2_platform_data = { - .id = MSP_I2S_2, - .msp_i2s_dma_rx = &msp2_dma_rx, - .msp_i2s_dma_tx = &msp2_dma_tx, -}; - -struct msp_i2s_platform_data msp3_platform_data = { - .id = MSP_I2S_3, - .msp_i2s_dma_rx = &msp1_dma_rx, - .msp_i2s_dma_tx = NULL, - .use_pinctrl = true, -}; - -void mop500_msp_init(struct device *parent) -{ - pr_info("%s: Register platform-device 'snd-soc-u8500'.\n", __func__); - 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, - &msp0_platform_data); - db8500_add_msp_i2s(parent, 1, U8500_MSP1_BASE, IRQ_DB8500_MSP1, - &msp1_platform_data); - db8500_add_msp_i2s(parent, 2, U8500_MSP2_BASE, IRQ_DB8500_MSP2, - &msp2_platform_data); - db8500_add_msp_i2s(parent, 3, U8500_MSP3_BASE, IRQ_DB8500_MSP1, - &msp3_platform_data); - - pr_info("%s: Register platform-device 'ux500-pcm'\n", __func__); - platform_device_register(&ux500_pcm); -} diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c index 2ef26c8..202df4b 100644 --- a/arch/arm/mach-ux500/board-mop500.c +++ b/arch/arm/mach-ux500/board-mop500.c @@ -607,7 +607,7 @@ static void __init mop500_init_machine(void) mop500_i2c_init(parent); mop500_sdi_init(parent); mop500_spi_init(parent); - mop500_msp_init(parent); + mop500_audio_init(parent); mop500_uart_init(parent);
u8500_cryp1_hash1_init(parent); @@ -641,7 +641,7 @@ static void __init snowball_init_machine(void) mop500_i2c_init(parent); snowball_sdi_init(parent); mop500_spi_init(parent); - mop500_msp_init(parent); + mop500_audio_init(parent); mop500_uart_init(parent);
/* This board has full regulator constraints */ @@ -673,7 +673,7 @@ static void __init hrefv60_init_machine(void) mop500_i2c_init(parent); hrefv60_sdi_init(parent); mop500_spi_init(parent); - mop500_msp_init(parent); + mop500_audio_init(parent); mop500_uart_init(parent);
i2c0_devs = ARRAY_SIZE(mop500_i2c0_devices); @@ -802,7 +802,7 @@ static void __init u8500_init_machine(void) ARRAY_SIZE(mop500_platform_devs));
mop500_sdi_init(parent); - mop500_msp_init(parent); + mop500_audio_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, @@ -821,7 +821,7 @@ static void __init u8500_init_machine(void) ARRAY_SIZE(mop500_platform_devs));
hrefv60_sdi_init(parent); - mop500_msp_init(parent); + mop500_audio_init(parent);
i2c0_devs = ARRAY_SIZE(mop500_i2c0_devices); i2c0_devs -= NUM_PRE_V60_I2C0_DEVICES; diff --git a/arch/arm/mach-ux500/board-mop500.h b/arch/arm/mach-ux500/board-mop500.h index e56c983..a0f920d 100644 --- a/arch/arm/mach-ux500/board-mop500.h +++ b/arch/arm/mach-ux500/board-mop500.h @@ -97,7 +97,7 @@ void __init mop500_stuib_init(void); void __init mop500_pinmaps_init(void); void __init snowball_pinmaps_init(void); void __init hrefv60_pinmaps_init(void); -void mop500_msp_init(struct device *parent); +void mop500_audio_init(struct device *parent);
int __init mop500_uib_init(void); void mop500_uib_i2c_add(int busnum, struct i2c_board_info *info,
On Thu, Jul 26, 2012 at 11:28:33AM +0100, Lee Jones wrote:
This patch-set sees some code reinforcement surrounding the recently accepted MOP500 MSP, PCM, AB8500 CODEC and ux500 Audio Machine Driver. It contains some extra error checking pertaining to the ux500 specific drivers themselves, along with bugfixes for issues in core SoC Audio code happened upon along the way. There are also a couple of minor clean-ups relating to ux500 platform code, which are hitching a ride for ease of status tracking.
Please restructure this so that the actual error fixes (which should go into 3.6) are before the random cleanups (which should go into 3.7) rather than mixing them together. It'd also be helpful if you could split out changes to the different subsystems separately when there's no overlap (which mostly looks to be the case here).
On 26/07/12 12:28, Mark Brown wrote:
On Thu, Jul 26, 2012 at 11:28:33AM +0100, Lee Jones wrote:
This patch-set sees some code reinforcement surrounding the recently accepted MOP500 MSP, PCM, AB8500 CODEC and ux500 Audio Machine Driver. It contains some extra error checking pertaining to the ux500 specific drivers themselves, along with bugfixes for issues in core SoC Audio code happened upon along the way. There are also a couple of minor clean-ups relating to ux500 platform code, which are hitching a ride for ease of status tracking.
Please restructure this so that the actual error fixes (which should go into 3.6) are before the random cleanups (which should go into 3.7) rather than mixing them together.
I can.
It'd also be helpful if you could split out changes to the different subsystems separately when there's no overlap (which mostly looks to be the case here).
I can to this too, but you will still have overlap between arch/arm and sound/soc. The only other subsystem in the patch-set is MFD.
participants (2)
-
Lee Jones -
Mark Brown