[alsa-devel] [PATCH 08/10] ASoC: ux500_pcm: Differentiate between pdata and DT initialisation
If booting with full DT support (i.e. DMA too, the last piece of the puzzle), then we don't need to use the compatible request channel call back and, due to the work we laid down earlier in this patch-set, we can use core function calls to populate the DMA slave_config. We also require slightly different flags to inform the core that we 'are' booting with DT.
Cc: alsa-devel@alsa-project.org Cc: Mark Brown broonie@kernel.org Acked-by: Linus Walleij linus.walleij@linaro.org Signed-off-by: Lee Jones lee.jones@linaro.org --- sound/soc/ux500/ux500_pcm.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c index ce554de..acfec98 100644 --- a/sound/soc/ux500/ux500_pcm.c +++ b/sound/soc/ux500/ux500_pcm.c @@ -139,15 +139,33 @@ static const struct snd_dmaengine_pcm_config ux500_dmaengine_pcm_config = { .prepare_slave_config = ux500_pcm_prepare_slave_config, };
+static const struct snd_dmaengine_pcm_config ux500_dmaengine_of_pcm_config = { + .pcm_hardware = &ux500_pcm_hw, + .prealloc_buffer_size = 128 * 1024, + .prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config, +}; + int ux500_pcm_register_platform(struct platform_device *pdev) { + const struct snd_dmaengine_pcm_config *pcm_config; + struct device_node *np = pdev->dev.of_node; + unsigned int pcm_flags; int ret;
- ret = snd_dmaengine_pcm_register(&pdev->dev, - &ux500_dmaengine_pcm_config, - SND_DMAENGINE_PCM_FLAG_NO_RESIDUE | - SND_DMAENGINE_PCM_FLAG_COMPAT | - SND_DMAENGINE_PCM_FLAG_NO_DT); + if (np) { + pcm_config = &ux500_dmaengine_of_pcm_config; + + pcm_flags = SND_DMAENGINE_PCM_FLAG_NO_RESIDUE | + SND_DMAENGINE_PCM_FLAG_COMPAT; + } else { + pcm_config = &ux500_dmaengine_pcm_config; + + pcm_flags = SND_DMAENGINE_PCM_FLAG_NO_RESIDUE | + SND_DMAENGINE_PCM_FLAG_NO_DT | + SND_DMAENGINE_PCM_FLAG_COMPAT; + } + + ret = snd_dmaengine_pcm_register(&pdev->dev, pcm_config, pcm_flags); if (ret < 0) { dev_err(&pdev->dev, "%s: ERROR: Failed to register platform '%s' (%d)!\n",
On Tue, Nov 19, 2013 at 11:07:47AM +0000, Lee Jones wrote:
require slightly different flags to inform the core that we 'are' booting with DT.
Is there some situation when we would want to say we're booting from DT when we aren't? Just wondering about the quotes.
+static const struct snd_dmaengine_pcm_config ux500_dmaengine_of_pcm_config = {
- .pcm_hardware = &ux500_pcm_hw,
- .prealloc_buffer_size = 128 * 1024,
You shouldn't need to set this explicitly, the generic code should be able to pick a number for you - if you do need this number please explain why the number was chosen in the comments (or fix the core to guess better). At the minute the core just makes up a number too but at least then it's a consistent random number between platforms.
Can you also get away without the pcm_hardware - the core should also have support for discovering this by querying the DMA controller?
On Tue, 19 Nov 2013, Mark Brown wrote:
On Tue, Nov 19, 2013 at 11:07:47AM +0000, Lee Jones wrote:
require slightly different flags to inform the core that we 'are' booting with DT.
Is there some situation when we would want to say we're booting from DT when we aren't? Just wondering about the quotes.
The quotes do 'not' mean anything special. :)
+static const struct snd_dmaengine_pcm_config ux500_dmaengine_of_pcm_config = {
- .pcm_hardware = &ux500_pcm_hw,
- .prealloc_buffer_size = 128 * 1024,
You shouldn't need to set this explicitly, the generic code should be able to pick a number for you - if you do need this number please explain why the number was chosen in the comments (or fix the core to guess better). At the minute the core just makes up a number too but at least then it's a consistent random number between platforms.
Can you also get away without the pcm_hardware - the core should also have support for discovering this by querying the DMA controller?
Despite the '+'s, I'm not actually adding these parameters, I'm duplicating the pdata version and removing the stuff I 'know' that's not required. I don't know what happens when/if these two parameters are removed. I can add this to my TODO when I rip out platform data support, which will happen when this stuff lands.
On 11/19/2013 08:33 PM, Lee Jones wrote:
On Tue, 19 Nov 2013, Mark Brown wrote:
On Tue, Nov 19, 2013 at 11:07:47AM +0000, Lee Jones wrote:
require slightly different flags to inform the core that we 'are' booting with DT.
Is there some situation when we would want to say we're booting from DT when we aren't? Just wondering about the quotes.
The quotes do 'not' mean anything special. :)
+static const struct snd_dmaengine_pcm_config ux500_dmaengine_of_pcm_config = {
- .pcm_hardware = &ux500_pcm_hw,
- .prealloc_buffer_size = 128 * 1024,
You shouldn't need to set this explicitly, the generic code should be able to pick a number for you - if you do need this number please explain why the number was chosen in the comments (or fix the core to guess better). At the minute the core just makes up a number too but at least then it's a consistent random number between platforms.
Can you also get away without the pcm_hardware - the core should also have support for discovering this by querying the DMA controller?
Despite the '+'s, I'm not actually adding these parameters, I'm duplicating the pdata version and removing the stuff I 'know' that's not required. I don't know what happens when/if these two parameters are removed. I can add this to my TODO when I rip out platform data support, which will happen when this stuff lands.
I think the patch is fine for now. Once non-DT support has been removed for ux500 we should be able to remove the whole ux500_pcm.c file (Assuming that the ux500 DMA engine driver gains dma_slave_caps support).
- Lars
On Tue, Nov 19, 2013 at 08:40:00PM +0100, Lars-Peter Clausen wrote:
On 11/19/2013 08:33 PM, Lee Jones wrote:
Despite the '+'s, I'm not actually adding these parameters, I'm duplicating the pdata version and removing the stuff I 'know' that's not required. I don't know what happens when/if these two parameters are removed. I can add this to my TODO when I rip out platform data support, which will happen when this stuff lands.
I think the patch is fine for now. Once non-DT support has been removed for ux500 we should be able to remove the whole ux500_pcm.c file (Assuming that the ux500 DMA engine driver gains dma_slave_caps support).
Yeah, it shouldn't break anything but I'd rather at least have a comment on any new code that specifies these things by hand explaining why the automatic stuff doesn't work, it's much nicer to not have to specify the magic numbers by hand and this will help encourage new code to do the right thing.
participants (3)
-
Lars-Peter Clausen -
Lee Jones -
Mark Brown