mop500 related Audio reorganisation and fixups

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 <[email protected]>

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 <[email protected]>

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

CC [email protected]

Signed-off-by Lee Jones <[email protected]>

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

CC [email protected]

Signed-off-by Lee Jones <[email protected]>

---
 sound/soc/codecs/ab8500-codec.c |    4 ++++
 1 file changed, 4 insertions(+)

sound/soc/codecs/ab8500-codec.c +4 -0

index 3c79592..23b4018 100644
@@ -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);

Signature · 6 lines

1.7.9.5

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

If codec->control_data is not populated SoC Core assumes we want to use regmap, which fails catastrophically, as we don't have one:

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.

CC [email protected]

Signed-off-by Lee Jones <[email protected]>

Quoted 9 lines · Lee Jones

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.

CC [email protected]

Signed-off-by Lee Jones <[email protected]>

This is very good because it rids platform code and makes the driver self-contained without strange platform data hooks.

Now details...

Quoted 9 lines

sound/soc/ux500/ux500_msp_i2s.c +9 -0

+/* 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 */

Quoted 1 line

+int pinctrl_rxtx_ref;

But wait. These are just local statics. Surelty you can put these into struct ux500_msp instead? Here it looks like these will be used for all ports, so MSP2 will enable the pins requested by MSP1 etc completely broken. So put it into the struct ux500_msp state container from ux500_msp_i2s.h where it belongs.

Quoted 9 lines · Lee Jones

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.

CC alsa-devel at alsa-project.org

Signed-off-by Lee Jones <[email protected]>

This is very good because it rids platform code and makes the driver self-contained without strange platform data hooks.

Now details...

sound/soc/ux500/ux500_msp_i2s.c line 38

Quoted 10 lines

sound/soc/ux500/ux500_msp_i2s.c +10 -0

+/* 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;

But wait. These are just local statics. Surelty you can put these into struct ux500_msp instead? Here it looks like these will be used for all ports, so MSP2 will enable the pins requested by MSP1 etc completely broken. So put it into the struct ux500_msp state container from ux500_msp_i2s.h where it belongs.

Refer to drivers/tty/serial/amba-pl011.c when in trouble. This one is a good pinctrl example.

Quoted 6 lines · Linus Walleij

But wait. These are just local statics. Surelty you can put these into struct ux500_msp instead? Here it looks like these will be used for all ports, so MSP2 will enable the pins requested by MSP1 etc completely broken. So put it into the struct ux500_msp state container from ux500_msp_i2s.h where it belongs.

Quoted 2 lines

Refer to drivers/tty/serial/amba-pl011.c when in trouble. This one is a good pinctrl example.

How do you see the MSP1 and MSP3 usage protection working if I hide it all away in MSP specific structs?

Quoted 8 lines · Lee Jones

Quoted 6 lines · Linus Walleij

But wait. These are just local statics. Surelty you can put these into struct ux500_msp instead? Here it looks like these will be used for all ports, so MSP2 will enable the pins requested by MSP1 etc completely broken. So put it into the struct ux500_msp state container from ux500_msp_i2s.h where it belongs.

Quoted 5 lines

Quoted 2 lines

Refer to drivers/tty/serial/amba-pl011.c when in trouble. This one is a good pinctrl example.

How do you see the MSP1 and MSP3 usage protection working if I hide it all away in MSP specific structs?

Usage protection? Sorry not following. Please elaborate...

Here we ensure that the MOP500 audio driver will be probed during a Device Tree boot. We also parse the sound node to link together the codec, dma and the CPU-side Digital Audio Interface.

CC [email protected]

Signed-off-by Lee Jones <[email protected]>

Quoted 6 lines · Lee Jones

Here we ensure that the MOP500 audio driver will be probed during a Device Tree boot. We also parse the sound node to link together the codec, dma and the CPU-side Digital Audio Interface.

CC alsa-devel at alsa-project.org

Signed-off-by Lee Jones <[email protected]>

Acked-by Linus Walleij <[email protected]>

But only if Mark agrees with the overall approach.

Yours,
Linus Walleij

Register both parts of the MSP driver from 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.

CC [email protected]

Signed-off-by Lee Jones <[email protected]>

Quoted 8 lines · Lee Jones

Register both parts of the MSP driver from 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.

CC alsa-devel at alsa-project.org

Signed-off-by Lee Jones <[email protected]>

Looks like it's doing what's intended...

Acked-by Linus Walleij <[email protected]>

Yours,
Linus Walleij

The PCM is a pseudo-device. It doesn't have any of it's own registers or hardware. It rather acts as a layer of abstraction for DMA transfers. Hence, instead of classifying it as a device in its own right, we call the initialisation from the MSP driver.

CC [email protected]

Signed-off-by Lee Jones <[email protected]>

Quoted 8 lines

Quoted 7 lines

I'm sorry but this patch is breaking the design of ASoC. The ASoC-platform is the DMA-block (in combination with the MSP-block), and there should be a platform-driver for the DMA/PCM. The
platform-driver then has a DAI which is the MSP. The ASoC DAI-link-struct should have one driver for each of these, so the dummy-driver for PCM should be there.

Quoted 5 lines

Just thinking about this now. I converted it to the current format at the request of Mark. If this isn't the correct method I'm not quite sure what is. If you want it to be registered as a device, then it needs to go into the Device Tree, but Mark doesn't want it in there because it doesn't actually represent hardware.

Quoted 5 lines

Quoted 5 lines

Just thinking about this now. I converted it to the current format at the request of Mark. If this isn't the correct method I'm not quite sure what is. If you want it to be registered as a device, then it needs to go into the Device Tree, but Mark doesn't want it in there because it doesn't actually represent hardware.

I've just taken a closer look at this with a view to finding the most suitable solution. My conclusion is that although the PCM doesn't contain any registers, or represent hardware it should be a device and therefore be present in the Device Tree.

PCM devices already represented in DTs:
fsl,mpc5200-pcm - written by Grant Likely, the author of Device Tree
phytec,pcm030 - written by Grant Likely, the author of Device Tree

PCM devices which register as actual devices (should be in DT):

I propose to represent the PCM in the Device Tree again and have it probe just like all the other PCM devices in sound/soc.

If you give me the nod, I'll revert this patch, enable the PCM for DT again and resent the patch-set in full.

I say I don't understand the motivation for this change. All the modern DT bindings are perfectly happy handling this without an explicit shim in the device tree to bodge things for Linux, adding them in seems like it'd be a retrograde step. What benefit do you believe this brings?

Quoted 4 lines

I say I don't understand the motivation for this change. All the modern DT bindings are perfectly happy handling this without an explicit shim in the device tree to bodge things for Linux, adding them in seems like it'd be a retrograde step. What benefit do you believe this brings?

How do the all the other DT:ed audio drivers handle the PCM then? More importantly, how would you like to see it handled? Ola has NACKed this patch and explained why:

"I'm sorry but this patch is breaking the design of ASoC. The ASoC- platform is the DMA-block (in combination with the MSP-block), and there should be a platform-driver for the DMA/PCM. The platform-driver then has a DAI which is the MSP. The ASoC DAI-link-struct should have one driver for each of these, so the dummy-driver for PCM should be there."

So I don't really know where to go with it. Any ideas?

Quoted 6 lines

"I'm sorry but this patch is breaking the design of ASoC. The ASoC- platform is the DMA-block (in combination with the MSP-block), and there should be a platform-driver for the DMA/PCM. The platform-driver then has a DAI which is the MSP. The ASoC DAI-link-struct should have one driver for each of these, so the dummy-driver for PCM should be there."

Quoted 1 line

So I don't really know where to go with it. Any ideas?

I think Ola is suggesting probing the DMA driver from the machine which will also work though I'm not 100% sure if I'm parsing the above correctly. The issue in DT terms is that if the DMA controller is shared with a bunch of other IPs then it should have one node shared between them all and not a bunch of shim nodes inserted in the middle which only exists due to the way Linux instantiates stuff.

Quoted 3 lines

How do the all the other DT:ed audio drivers handle the PCM then? More importantly, how would you like to see it handled? Ola has NACKed this patch and explained why:

They instantiate the PCM driver dynamically from the DAI when it's probed which is pretty much what you're patch is doing.

Quoted 6 lines

I think Ola is suggesting probing the DMA driver from the machine which will also work though I'm not 100% sure if I'm parsing the above correctly. The issue in DT terms is that if the DMA controller is shared with a bunch of other IPs then it should have one node shared between them all and not a bunch of shim nodes inserted in the middle which only exists due to the way Linux instantiates stuff.

When you say 'machine', do you mean from arch/<arch>/mach-*? If so, I'm keen for that not to happen.

Quoted 2 lines

They instantiate the PCM driver dynamically from the DAI when it's probed which is pretty much what you're patch is doing.

So they do it in the same why I have with this patch? Do you know why Ola might think this is a bad idea?

Quoted 6 lines · Lee Jones

Quoted 6 lines

I think Ola is suggesting probing the DMA driver from the machine which will also work though I'm not 100% sure if I'm parsing the above correctly. The issue in DT terms is that if the DMA controller is shared with a bunch of other IPs then it should have one node shared between them all and not a bunch of shim nodes inserted in the middle which only exists due to the way Linux instantiates stuff.

Quoted 2 lines

When you say 'machine', do you mean from arch/<arch>/mach-*? If so, I'm keen for that not to happen.

No, sound/soc/ux500/snowball.c or whatever. At least that's my guess.

Quoted 5 lines

Quoted 2 lines

They instantiate the PCM driver dynamically from the DAI when it's probed which is pretty much what you're patch is doing.

So they do it in the same why I have with this patch? Do you know why Ola might think this is a bad idea?

I'm not 100% sure, I'm guessing it might be down to the fact that you end up with multiple PCM drivers. We could avoid that with refcounting but nobody's really worried about it.

I'll attempt to re-jig the patch-set, as this is a blocker atm.

Quoted 4 lines · Lee Jones

Quoted 4 lines

I say I don't understand the motivation for this change. All the modern DT bindings are perfectly happy handling this without an explicit shim in the device tree to bodge things for Linux, adding them in seems like it'd be a retrograde step. What benefit do you believe this brings?

Quoted 3 lines

How do the all the other DT:ed audio drivers handle the PCM then? More importantly, how would you like to see it handled? Ola has NACKed this patch and explained why:

They instantiate the PCM driver dynamically from the DAI when it's probed which is pretty much what you're patch is doing.

Hi Ola,

As requested:

Author: Lee Jones <[email protected]>
Date:   Tue Jul 31 14:06:19 2012 +0100

ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

The PCM is a pseudo-device. It doesn't have any of it's own registers or hardware. It rather acts as a layer of abstraction for DMA transfers. Hence, instead of classifying it as a device in its own right, we call the initialisation from the MSP driver.

Signed-off-by Lee Jones <[email protected]>

sound/soc/ux500/mop500.c +2 -2

index 31c4d26..9707f4a 100644
@@ -32,7 +32,7 @@ struct snd_soc_dai_link mop500_dai_links[] = {
 		.stream_name = "ab8500_0",
 		.cpu_dai_name = "ux500-msp-i2s.1",
 		.codec_dai_name = "ab8500-codec-dai.0",
-		.platform_name = "ux500-pcm.0",
+		.platform_name = "ux500-msp-i2s.1",
 		.codec_name = "ab8500-codec.0",
 		.init = mop500_ab8500_machine_init,
 		.ops = mop500_ab8500_ops,

@@ -42,7 +42,7 @@ struct snd_soc_dai_link mop500_dai_links[] = {
 		.stream_name = "ab8500_1",
 		.cpu_dai_name = "ux500-msp-i2s.3",
 		.codec_dai_name = "ab8500-codec-dai.1",
-		.platform_name = "ux500-pcm.0",
+		.platform_name = "ux500-msp-i2s.3",
 		.codec_name = "ab8500-codec.0",
 		.init = NULL,
 		.ops = mop500_ab8500_ops,

sound/soc/ux500/ux500_msp_dai.c +0 -0

Lee Jones - Sept. 19, 2012, 12:29 p.m.

Quoted 10 lines · Mark Brown

Quoted 8 lines · Lee Jones

Quoted 4 lines

I say I don't understand the motivation for this change. All the modern DT bindings are perfectly happy handling this without an explicit shim in the device tree to bodge things for Linux, adding them in seems like it'd be a retrograde step. What benefit do you believe this brings?

How do the all the other DT:ed audio drivers handle the PCM then? More importantly, how would you like to see it handled? Ola has NACKed this patch and explained why:

Quoted 2 lines

They instantiate the PCM driver dynamically from the DAI when it's probed which is pretty much what you're patch is doing.

Can we have some closure on this patch please, as it's blocking the patch-set? I'm fairly sure the patch is doing the correct thing, as seconded by Mark.

I still don't like this. It is the dai_link-struct that bothers me. We have "ux500-msp-i2s.1" as name of the platform AND the cpu_dai. The MSP I2S-block is not the platform and it is certainly not both platform and cpu-DAI at the same time.

Mark: Did you have a solution for this? Couldn't we just put NULL on the platform_name instead?

Quoted 3 lines

Can we have some closure on this patch please, as it's blocking the patch-set? I'm fairly sure the patch is doing the correct thing, as seconded by Mark.

I still don't like this. It is the dai_link-struct that bothers me. We have "ux500-msp-i2s.1" as name of the platform AND the cpu_dai. The MSP I2S-block is not the platform and it is certainly not both platform and cpu-DAI at the same time.
Mark: Did you have a solution for this? Couldn't we just put NULL on the platform_name instead?

Quoted 9 lines

Quoted 3 lines

Can we have some closure on this patch please, as it's blocking the patch-set? I'm fairly sure the patch is doing the correct thing, as seconded by Mark.

I still don't like this. It is the dai_link-struct that bothers me. We have "ux500-msp-i2s.1" as name of the platform AND the cpu_dai. The MSP I2S-block is not the platform and it is certainly not both platform and cpu-DAI at the same time.
Mark: Did you have a solution for this? Couldn't we just put NULL on the platform_name instead?

There are other drivers which do this already.

I don't think it's an issue to do this.

Quoted 9 lines · Lee Jones

Quoted 9 lines

Quoted 3 lines

Can we have some closure on this patch please, as it's blocking the patch-set? I'm fairly sure the patch is doing the correct thing, as seconded by Mark.

I still don't like this. It is the dai_link-struct that bothers me. We have "ux500-msp-i2s.1" as name of the platform AND the cpu_dai. The MSP I2S-block is not the platform and it is certainly not both platform and cpu-DAI at the same time.
Mark: Did you have a solution for this? Couldn't we just put NULL on the platform_name instead?

Quoted 3 lines

There are other drivers which do this already.

I don't think it's an issue to do this.

Has anyone had any more bright ideas on how we might handle this?

The device is still being handled in platform code and I'm desperate to get it out of there.

Quoted 13 lines · Lee Jones

Quoted 11 lines · Lee Jones

Quoted 9 lines

Quoted 3 lines

Can we have some closure on this patch please, as it's blocking the patch-set? I'm fairly sure the patch is doing the correct thing, as seconded by Mark.

I still don't like this. It is the dai_link-struct that bothers me. We have "ux500-msp-i2s.1" as name of the platform AND the cpu_dai. The MSP I2S-block is not the platform and it is certainly not both platform and cpu-DAI at the same time.
Mark: Did you have a solution for this? Couldn't we just put NULL on the platform_name instead?

There are other drivers which do this already.

Quoted 6 lines

Quoted 1 line

I don't think it's an issue to do this.

Has anyone had any more bright ideas on how we might handle this?

The device is still being handled in platform code and I'm desperate to get it out of there.

Why do we need any ideas? Just implement it already.

Quoted 1 line · Mark Brown

Why do we need any ideas? Just implement it already.

How do you mean? I've written the patch, just accept it. ;)

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.

CC [email protected]

Signed-off-by Lee Jones <[email protected]>

sound/soc/codecs/ab8500-codec.c +17 -0

+			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;
+		}
+	}
+
 	/* Inform SoC Core that we have our own I/O arrangements. */
 	codec->control_data = (void *)true;
 

Signature · 6 lines

1.7.9.5

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

From: Lee Jones <[email protected]>
Date: Fri, 27 Jul 2012 08:50:05 +0100
Subject: [PATCH 1/1] ASoC: codecs: Enable AB8500 CODEC for Device Tree

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.

Quoted 7 lines · Lee Jones

From: Lee Jones <[email protected]>
Date: Fri, 27 Jul 2012 08:50:05 +0100
Subject: [PATCH 1/1] ASoC: codecs: Enable AB8500 CODEC for Device Tree

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.

Please resend this series with all the acks you've got rather than mixing incremental updates in like this.

Please also send patches in
the format documented in SubmittingPatches.

Quoted 2 lines · Mark Brown

Quoted 2 lines

From: Lee Jones <[email protected]>
Date: Fri, 27 Jul 2012 08:50:05 +0100

Quoted 2 lines

Please resend this series with all the acks you've got rather than mixing incremental updates in like this.

I'm waiting until I have a few more Acks before I resend the entire patch-set again. Actually it's you and one other that I'm waiting for to review (and Ack as necessary) the ones requested by Linus, then resend with the corrections.

Quoted 2 lines

Please also send patches in
the format documented in SubmittingPatches.

That's a big document, most of which I guess I'm adhering to. Care to be a little more specific?

Quoted 7 lines · Lee Jones

Quoted 5 lines · Mark Brown

Quoted 2 lines

From: Lee Jones <[email protected]>
Date: Fri, 27 Jul 2012 08:50:05 +0100

Please resend this series with all the acks you've got rather than mixing incremental updates in like this.

Quoted 4 lines

I'm waiting until I have a few more Acks before I resend the entire patch-set again. Actually it's you and one other that I'm waiting for to review (and Ack as necessary) the ones requested by Linus, then resend with the corrections.

Well, I'm the person who's going to apply the patches so I'm unlikely to ack them... I was waiting for arch/arm review before I looked at

them due to the number of resends.

Quoted 5 lines

Quoted 2 lines

Please also send patches in
the format documented in SubmittingPatches.

That's a big document, most of which I guess I'm adhering to. Care to be a little more specific?

The bit I quoted is the main example, you're including random mail headers in the body of the mail.

Quoted 2 lines · Mark Brown

The bit I quoted is the main example, you're including random mail headers in the body of the mail.

They're not mail headers per-say, they're git format-patch headers. I thought this was acceptable for single patches, hence why I've done it lots of times and had no complaints (until now).

If there are some changes required in a single patch, I usually fix it up, create a patch with git format-patch and send it as a reply to either the original patch in the series or the mail containing the suggestion. If this is wrong please educate me as I thought this was

acceptable, as I thought it would be less pain than sending the entire patch-set again for just one change?

This patch contains a couple of general MSP clean-ups pertaining to layout changes and changing functions to be void instead of int instead of regardlessly returning '0'.

Signed-off-by Lee Jones <[email protected]>

Ensure correct probing and pass though important configuration options to the AB8500 CODEC driver when DT is enabled

Signed-off-by Lee Jones <[email protected]>

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 <[email protected]>

Quoted 6 lines · Lee Jones

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.

Quoted 1 line

Signed-off-by Lee Jones <[email protected]>

I understand you have used this approach before so:

Acked-by Linus Walleij <[email protected]>

Quoted 8 lines · Linus Walleij

Quoted 6 lines · Lee Jones

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.

Quoted 4 lines

Quoted 1 line

Signed-off-by Lee Jones <[email protected]>

I understand you have used this approach before so:

Acked-by Linus Walleij <[email protected]>

Does this comment take back your previous one:

NOTE: it seems this patch set contains some churn. First you add in the forked device init, put in a big chunk of code and then in the *same* patch set delete it again. It's not like we're dying to see all the development history... can this be squashed down a bit?

... hence leave the patch-set as it is?

Quoted 22 lines · Lee Jones

Quoted 8 lines · Linus Walleij

Quoted 6 lines · Lee Jones

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.

Quoted 4 lines

Quoted 1 line

Signed-off-by Lee Jones <[email protected]>

I understand you have used this approach before so:

Acked-by Linus Walleij <[email protected]>

Does this comment take back your previous one:

NOTE: it seems this patch set contains some churn. First you add in the forked device init, put in a big chunk of code and then in the *same* patch set delete it again. It's not like we're dying to see all the development history... can this be squashed down a bit?

Quoted 1 line

... hence leave the patch-set as it is?

No. I just meant leave it like that for the devices outside of this set.

If you're adding and then removing *all* of them in this set, why add them in the first place?

Quoted 2 lines · Linus Walleij

If you're adding and then removing *all* of them in this set, why add them in the first place?

So that there's no breakage during bisection.

You should be able to roll the kernel back in between each of these patches and there to be full compatibility at each point. At least that was the intention. Is that wrong?

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 <[email protected]>

In this patch we prevent MOP500 driver registration from platform code and rely solely on Device Tree to do the probing for us.

Signed-off-by Lee Jones <[email protected]>

We now initialise the PCM driver through the MSP DAI, so there is no need to register it though platform code anymore. This patch strips out all PCM platform registration.

Signed-off-by Lee Jones <[email protected]>

This patch removes platform device registration of all 4 MSP devices. It also takes care of all redundant infrastructure now that each of the ux500 audio components have been Device Tree enabled.

Signed-off-by Lee Jones <[email protected]>

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 <[email protected]>

Quoted 6 lines · Lee Jones

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'.

Quoted 1 line

Signed-off-by Lee Jones <[email protected]>

Ideally I want Ola to confirm this but it looks OK to me.

Acked-by Linus Walleij <[email protected]>

This patch inserts all known MSP devices into the DBx5x0 Device Tree disabled and enables the two important ones for the Snowball low-cost development board.

Signed-off-by Lee Jones <[email protected]>

This is where we link together all of the SoC sound components for a given platform. This all encompassing sound node is only found in the very lowest hierarchical DTS file, since the component mix may well change from board variant to board variant.

Signed-off-by Lee Jones <[email protected]>

Here we document the AB8500 Audio CODEC in the existing MFD AB8500 document.

CC [email protected]

Signed-off-by Lee Jones <[email protected]>

Another fine instalment of Audio fixes and Device Tree enablement surrounding the mop500 sound driver. Hopefully containing all of the recommendations suggested by Mark Brown.

Quoted 3 lines · Lee Jones

Another fine instalment of Audio fixes and Device Tree enablement surrounding the mop500 sound driver. Hopefully containing all of the recommendations suggested by Mark Brown.

Can we have some review of this from the ARM side please? It's pretty big...

Quoted 7 lines · Mark Brown

Quoted 3 lines · Lee Jones

Another fine instalment of Audio fixes and Device Tree enablement surrounding the mop500 sound driver. Hopefully containing all of the recommendations suggested by Mark Brown.

Can we have some review of this from the ARM side please? It's pretty big...

I don't expect Linus will do anything until he returns back from vacation.

Quoted 6 lines · Lee Jones

Quoted 5 lines · Mark Brown

Quoted 3 lines · Lee Jones

Another fine instalment of Audio fixes and Device Tree enablement surrounding the mop500 sound driver. Hopefully containing all of the recommendations suggested by Mark Brown.

Quoted 4 lines

Quoted 2 lines

Can we have some review of this from the ARM side please? It's pretty big...

I don't expect Linus will do anything until he returns back from vacation.

I'm back from vacation, and catching up. Now there is a few different DT patch bombs from you in my inbox, which patchset shall I review first? This one? Or is there a more urgent one?

Quoted 3 lines · Lee Jones

Another fine instalment of Audio fixes and Device Tree enablement surrounding the mop500 sound driver. Hopefully containing all of the recommendations suggested by Mark Brown.

Can we have some review of this from the ARM side please? It's pretty big...