To avoid the declaration of a 'kirkwood-pcm-audio' device in the DT,
this patch merges the kirkwood-i2c and kirkwood-dma drivers into one
module associated with 'kirkwood-i2s'.
Signed-off-by: Jean-Francois Moine <[email protected]>
---
arch/arm/mach-kirkwood/common.c | 6 ------
sound/soc/kirkwood/Kconfig | 5 -----
sound/soc/kirkwood/Makefile | 4 +---
sound/soc/kirkwood/kirkwood-dma.c | 30 +-----------------------------
sound/soc/kirkwood/kirkwood-i2s.c | 6 ++++++
sound/soc/kirkwood/kirkwood-openrd.c | 2 +-
sound/soc/kirkwood/kirkwood-t5325.c | 2 +-
sound/soc/kirkwood/kirkwood.h | 1 +
8 files changed, 11 insertion(+), 45 deletion(-)
diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c
index e9238b5..fd02f1b 100644
--- a/arch/arm/mach-kirkwood/common.c
+++ b/arch/arm/mach-kirkwood/common.c
@@ -587,15 +587,9 @@ static struct platform_device kirkwood_i2s_device = {
},
};
-static struct platform_device kirkwood_pcm_device = {
- .name = "kirkwood-pcm-audio",
- .id = -1,
-};
-
void __init kirkwood_audio_init(void)
{
platform_device_register(&kirkwood_i2s_device);
- platform_device_register(&kirkwood_pcm_device);
}
/*****************************************************************************
diff --git a/sound/soc/kirkwood/Kconfig b/sound/soc/kirkwood/Kconfig
index 59085ad..9e1970c 100644
--- a/sound/soc/kirkwood/Kconfig
+++ b/sound/soc/kirkwood/Kconfig
@@ -6,14 +6,10 @@ config SND_KIRKWOOD_SOC
the Kirkwood I2S interface. You will also need to select the
audio interfaces to support below.
-config SND_KIRKWOOD_SOC_I2S
- tristate
-
config SND_KIRKWOOD_SOC_OPENRD
tristate "SoC Audio support for Kirkwood Openrd Client"
depends on SND_KIRKWOOD_SOC && (MACH_OPENRD_CLIENT || MACH_OPENRD_ULTIMATE || COMPILE_TEST)
depends on I2C
- select SND_KIRKWOOD_SOC_I2S
select SND_SOC_CS42L51
help
Say Y if you want to add support for SoC audio on
@@ -22,7 +18,6 @@ config SND_KIRKWOOD_SOC_OPENRD
config SND_KIRKWOOD_SOC_T5325
tristate "SoC Audio support for HP t5325"
depends on SND_KIRKWOOD_SOC && (MACH_T5325 || COMPILE_TEST) && I2C
- select SND_KIRKWOOD_SOC_I2S
select SND_SOC_ALC5623
help
Say Y if you want to add support for SoC audio on
diff --git a/sound/soc/kirkwood/Makefile b/sound/soc/kirkwood/Makefile
index 3e62ae9..cf22ad3 100644
--- a/sound/soc/kirkwood/Makefile
+++ b/sound/soc/kirkwood/Makefile
@@ -1,8 +1,6 @@
-snd-soc-kirkwood-objs := kirkwood-dma.o
-snd-soc-kirkwood-i2s-objs := kirkwood-i2s.o
+snd-soc-kirkwood-objs := kirkwood-i2s.o kirkwood-dma.o
obj-$(CONFIG_SND_KIRKWOOD_SOC) += snd-soc-kirkwood.o
-obj-$(CONFIG_SND_KIRKWOOD_SOC_I2S) += snd-soc-kirkwood-i2s.o
snd-soc-openrd-objs := kirkwood-openrd.o
snd-soc-t5325-objs := kirkwood-t5325.o
diff --git a/sound/soc/kirkwood/kirkwood-dma.c b/sound/soc/kirkwood/kirkwood-dma.c
index a9f1453..f735501 100644
--- a/sound/soc/kirkwood/kirkwood-dma.c
+++ b/sound/soc/kirkwood/kirkwood-dma.c
@@ -366,36 +366,8 @@ static void kirkwood_dma_free_dma_buffers(struct snd_pcm *pcm)
}
}
-static struct snd_soc_platform_driver kirkwood_soc_platform = {
+struct snd_soc_platform_driver kirkwood_soc_platform = {
.ops = &kirkwood_dma_ops,
.pcm_new = kirkwood_dma_new,
.pcm_free = kirkwood_dma_free_dma_buffers,
};
-
-static int kirkwood_soc_platform_probe(struct platform_device *pdev)
-{
- return snd_soc_register_platform(&pdev->dev, &kirkwood_soc_platform);
-}
-
-static int kirkwood_soc_platform_remove(struct platform_device *pdev)
-{
- snd_soc_unregister_platform(&pdev->dev);
- return 0;
-}
-
-static struct platform_driver kirkwood_pcm_driver = {
- .driver = {
- .name = "kirkwood-pcm-audio",
- .owner = THIS_MODULE,
- },
-
- .probe = kirkwood_soc_platform_probe,
- .remove = kirkwood_soc_platform_remove,
-};
-
-module_platform_driver(kirkwood_pcm_driver);
-
-MODULE_AUTHOR("Arnaud Patard <[email protected]>");
-MODULE_DESCRIPTION("Marvell Kirkwood Audio DMA module");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:kirkwood-pcm-audio");
diff --git a/sound/soc/kirkwood/kirkwood-i2s.c b/sound/soc/kirkwood/kirkwood-i2s.c
index 319086f..a13fffb 100644
--- a/sound/soc/kirkwood/kirkwood-i2s.c
+++ b/sound/soc/kirkwood/kirkwood-i2s.c
@@ -453,6 +453,7 @@ static int kirkwood_i2s_dev_remove(struct platform_device *pdev)
{
struct kirkwood_dma_data *priv = dev_get_drvdata(&pdev->dev);
+ snd_soc_unregister_platform(&pdev->dev);
snd_soc_unregister_component(&pdev->dev);
if (!IS_ERR(priv->extclk))
@@ -536,6 +537,11 @@ static int kirkwood_i2s_dev_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "snd_soc_register_component failed\n");
goto fail;
}
+ err = snd_soc_register_platform(&pdev->dev, &kirkwood_soc_platform);;
+ if (err) {
+ dev_err(&pdev->dev, "snd_soc_register_platform failed\n");
+ goto fail;
+ }
return 0;
fail:
diff --git a/sound/soc/kirkwood/kirkwood-openrd.c b/sound/soc/kirkwood/kirkwood-openrd.c
index addbebc..df565d2 100644
--- a/sound/soc/kirkwood/kirkwood-openrd.c
+++ b/sound/soc/kirkwood/kirkwood-openrd.c
@@ -53,7 +53,7 @@ static struct snd_soc_dai_link openrd_client_dai[] = {
.name = "CS42L51",
.stream_name = "CS42L51 HiFi",
.cpu_dai_name = "kirkwood-i2s",
- .platform_name = "kirkwood-pcm-audio",
+ .platform_name = "kirkwood-i2s",
.codec_dai_name = "cs42l51-hifi",
.codec_name = "cs42l51-codec.0-004a",
.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBS_CFS,
diff --git a/sound/soc/kirkwood/kirkwood-t5325.c b/sound/soc/kirkwood/kirkwood-t5325.c
index 4f4cb56..b4647ba 100644
--- a/sound/soc/kirkwood/kirkwood-t5325.c.next
+++ b/sound/soc/kirkwood/kirkwood-t5325.c
@@ -69,7 +69,7 @@ static struct snd_soc_dai_link t5325_dai[] = {
.name = "ALC5621",
.stream_name = "ALC5621 HiFi",
.cpu_dai_name = "kirkwood-i2s",
- .platform_name = "kirkwood-pcm-audio",
+ .platform_name = "kirkwood-i2s",
.codec_dai_name = "alc5621-hifi",
.codec_name = "alc562x-codec.0-001a",
.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBS_CFS,
diff --git a/sound/soc/kirkwood/kirkwood.h b/sound/soc/kirkwood/kirkwood.h
index 4d92637..f3de119 100644
--- a/sound/soc/kirkwood/kirkwood.h
+++ b/sound/soc/kirkwood/kirkwood.h
@@ -133,4 +133,5 @@ struct kirkwood_dma_data {
int burst;
};
+extern struct snd_soc_platform_driver kirkwood_soc_platform;
#endif
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
On Wed, Jul 31, 2013 at 08:18:06AM +0200, Jean-Francois Moine wrote:
> To avoid the declaration of a 'kirkwood-pcm-audio' device in the DT,
> this patch merges the kirkwood-i2c and kirkwood-dma drivers into one
> module associated with 'kirkwood-i2s'.
This is broken unless you use my patch. You can't just stick the
two drivers together because they both make use of the platform
device's driver data field for two incompatible uses. I've already
explained that, and I've even sent you a patch to fix that, but you
seem to ignore anything that you didn't write.
So... I'm going to work on this issue this weekend and merge the two
drivers myself, and send Mark my own patches to do this.
On Sat, Aug 03, 2013 at 12:43:21PM +0100, Russell King - ARM Linux wrote:
> On Wed, Jul 31, 2013 at 08:18:06AM +0200, Jean-Francois Moine wrote:
> > To avoid the declaration of a 'kirkwood-pcm-audio' device in the DT,
> > this patch merges the kirkwood-i2c and kirkwood-dma drivers into one
> > module associated with 'kirkwood-i2s'.
>
> This is broken unless you use my patch. You can't just stick the
> two drivers together because they both make use of the platform
> device's driver data field for two incompatible uses. I've already
> explained that, and I've even sent you a patch to fix that, but you
> seem to ignore anything that you didn't write.
>
> So... I'm going to work on this issue this weekend and merge the two
> drivers myself, and send Mark my own patches to do this.
Sorry, I take that back, your cover email does say that this depends on
my patch. I'll give your patches a go this weekend instead then. Thanks.
On Wed, Jul 31, 2013 at 08:18:06AM +0200, Jean-Francois Moine wrote:
> To avoid the declaration of a 'kirkwood-pcm-audio' device in the DT,
> this patch merges the kirkwood-i2c and kirkwood-dma drivers into one
> module associated with 'kirkwood-i2s'.
I suggest holding off on this stuff at the moment. I think Mark and Liam
(who now have a whole raft of emails from me today) have some work to do
to fix ASoC to work how they're saying it should work, because to me
ASoC seems to contradict what they're telling me. To put it another way,
it must be buggy.
The DAPM stuff seems to be the worst thing since mouldy bread. I'm
chasing what seems to be multiple bugs through this stuff, and many of
them are not particularly nice.
For example - we register multiple copies of DAPM widgets for the same
set of declarations if both a CPU DAI and a platform share the same
struct device, and thereby end up overwriting some pointers in the DAI
structure. It seems that CPU DAIs themselves aren't supposed to have
DAPM widgets, but the core creates some... but there's no explicit
cleanup of them unlike the other DAPMs, and there's no debugfs for them.
The bias levels are stuck at off/standby all the time, which makes any
kind of PM with spdif-dit impossible - or even any routing between
stream input and output.
Basically, I'm tearing my hair out today looking at this stuff and getting
nowhere fast - all I'm doing is finding more and more problems.
For example, where a codec has no input/output widgets declared, it used
to be powered up automatically by ASoC as a matter of course. Things
like UDA134x and other things. Things like spdif-dit. That "mysteriously"
stopped happening.
ASoC: dapm: Treat DAI widgets like AIF widgets for power
seems to be the cause, this results in such setups having zero connected
inputs/outputs reported, which causes them to remain powered down - because
what used to happen before was the DAI links would report their powered
state depending on whether they were active, and they are set active
in soc_dapm_stream_event().
Now, when playback widget for the Codec and CPU DAIs get marked as active.
The playback widget is created as a snd_soc_dapm_dai_in. It's power
check function is set to dapm_dac_check_power. Since this widget is
active, it checks for connected outputs via is_connected_output_ep().
We drop through to the second switch statement (why do we have two there?
They're both switching on the same damned variable and its not like a
widget can change its ID.) This is where the different behaviour has
appeared - when these were just a simple snd_soc_dapm_dai, we used to
just do the snd_soc_dapm_suspend_check() here, but we don't anymore.
This is a snd_soc_dapm_dai_in type of widget, so we fall through this
switch statement now and start searching the paths.
As these codecs have no paths, this ultimately ends up returning no
connections. Hence why these codecs with no DAPM widgets declared but
have PM support via the bias level stuff have stopped working.
Now, about the spdif-dit, if we're going to have to add "pin" widgets
to it, what the output of a SPDIF in terms of DAPM widgets? At a guess,
it's a "codec pin" despite there being no codec and no "pin" on that
codec in reality, and that "pin" is always active.
With that "fixed" (rather, altered to a state where it behaves the
same way as it used to before the above commit) if I set up a DAPM route
between the CPU DAI playback stream, an AIF (for spdif output) and the
spdif-dit playback stream, I see the playback streams marked active and
powered up, but the AIF stream remains powered down:
spdif-dit/dapm/Playback:Playback: On in 1 out 1
spdif-dit/dapm/Playback: stream Playback active
spdif-dit/dapm/Playback: in "static" "spdif-playback"
spdif-dit/dapm/bias_level:On
kirkwood-audio.1/dapm/spdif-playback:spdif-playback: Off in 1 out 0
kirkwood-audio.1/dapm/spdif-playback: in "static" "dma-playback"
kirkwood-audio.1/dapm/spdif-playback: out "static" "Playback"
kirkwood-audio.1/dapm/dma-playback:dma-playback: On in 1 out 1
kirkwood-audio.1/dapm/dma-playback: stream dma-playback active
kirkwood-audio.1/dapm/dma-playback: out "static" "spdif-playback"
kirkwood-audio.1/dapm/dma-playback: out "static" "i2s-playback"
It looks to me like the DAPM stuff is - in one plain and simple word -
buggered.
I've no idea what the right fixes are in this area. It needs someone
like Mark or Liam who supposedly understand this to spend time checking
that it actually operates as they _think_ it should operate, because
at the moment it plainly doesn't.
On Sat, Aug 03, 2013 at 11:23:33PM +0100, Russell King - ARM Linux wrote:
> On Wed, Jul 31, 2013 at 08:18:06AM +0200, Jean-Francois Moine wrote:
> > To avoid the declaration of a 'kirkwood-pcm-audio' device in the DT,
> > this patch merges the kirkwood-i2c and kirkwood-dma drivers into one
> > module associated with 'kirkwood-i2s'.
> I suggest holding off on this stuff at the moment. I think Mark and Liam
> (who now have a whole raft of emails from me today) have some work to do
> to fix ASoC to work how they're saying it should work, because to me
> ASoC seems to contradict what they're telling me. To put it another way,
> it must be buggy.
Or push this stuff in there and help fix any issues that come up, what
issues there are with the CPU side stuff are there mostly because nobody
has been upstreaming anything and hence exercising the code. This merge
is going to be needed for the DT conversion anyway so I can't see any
gain in waiting, other things depend on this rather than the other way
around.
> The DAPM stuff seems to be the worst thing since mouldy bread. I'm
> chasing what seems to be multiple bugs through this stuff, and many of
> them are not particularly nice.
> For example - we register multiple copies of DAPM widgets for the same
> set of declarations if both a CPU DAI and a platform share the same
> struct device, and thereby end up overwriting some pointers in the DAI
> structure. It seems that CPU DAIs themselves aren't supposed to have
> DAPM widgets, but the core creates some... but there's no explicit
> cleanup of them unlike the other DAPMs, and there's no debugfs for them.
CPU DAIs are supposed to be able to have widgets, I'm not sure why you
say they aren't. It's possible this doesn't work since most hardware is
structured such that the control belongs in a DSP which tends to end up
in the platform device so even the out of tree users aren't likely to
exercise this path but from a framework point of view that seems like a
thing I'd expect to work and if it doesn't work we should make that so.
Removal paths are, by and by large, not really exercised since nobody
actually does that in production and most people's development model is
based on rebooting full kernels.
> For example, where a codec has no input/output widgets declared, it used
> to be powered up automatically by ASoC as a matter of course. Things
> like UDA134x and other things. Things like spdif-dit. That "mysteriously"
> stopped happening.
This is DAPMless CODECs, frankly they're more trouble than they're
worth given how trivial it is to implement DAPM and the fact that they
constantly cause problems due to the special casing they need and the
fact that development is pretty much all happening on modern hardware
with CODEC drivers that support DAPM. This isn't the first time they've
been missed, it won't be the last either.
> Now, about the spdif-dit, if we're going to have to add "pin" widgets
> to it, what the output of a SPDIF in terms of DAPM widgets? At a guess,
> it's a "codec pin" despite there being no codec and no "pin" on that
> codec in reality, and that "pin" is always active.
There's an output from the S/PDIF device - the S/PDIF signal it sends,
either electrically or optically (usually the former).
> It looks to me like the DAPM stuff is - in one plain and simple word -
> buggered.
It does seem to be working adequately in a rather large number of
systems. I think you're confusing DAPM with support for non-trivial
SoCs, currently all the mainline DAPM use is in CODEC devices.
As far as I can tell your problems here come from a combination of using
very old devices like the uda134x which haven't ever been particularly
well maintained and trying to do new things with the frameworks. If
you're working on something that's not been done before it's likely that
the frameworks will need extending.
> I've no idea what the right fixes are in this area. It needs someone
> like Mark or Liam who supposedly understand this to spend time checking
> that it actually operates as they _think_ it should operate, because
> at the moment it plainly doesn't.
This is all working fine for me with the systems I have available to me
in mainline - I don't have any systems at all with non-DAPM devices and
obviously nothing in mainline uses DPCM.