On Tue, 2013-08-20 at 21:18 +0100, Russell King - ARM Linux wrote:

On Tue, Aug 20, 2013 at 07:50:19PM +0100, Mark Brown wrote:

quoted

On Tue, Aug 20, 2013 at 02:31:43PM +0100, Russell King - ARM Linux wrote:

quoted

Let's be a little more clear about that: I don't know how to do that
because that's the approach taken by _these_ very patches which you've
rejected for "abusing the ASoC core".  That's why I'm asking Liam

The patches I can recall seeing recently have all had some workarounds
in the core which would need to be resolved differently, though it's
possible I missed that being done in some version in your mails as there
have generally also been a lot of modifications adding debug statements
in the core.

The "workarounds in the core" are because there's bugs in the core that
I have no idea how to solve.  You are allegedly the maintainer for the
core code, and so you should understand that code, so you are best placed
to say how the core code should be fixed.  I'm willing to do the patch
generation to fix them but *you* need to give some guidance here -
something that you seem incapable to do.  At the moment, the only fix I
can see being workable is to comment out the broken bit in the core code.

I'll fix this issue as I've replied privately, but you know it's not
appropriate to just comment stuff out in core code (especially if you
don't fully understand it). I'm sure you would complain loudly to me if
I tried to do a similar HACK in the ARM core.

If the problem is that you don't understand the issue, then you could try
replying with some questions about it.

If the problem is that you don't understand the code, well... there's not
much point in continuing this discussion until you've had time to study
and understand that code.

quoted

If you've got code you think is in a good state to submit then please do
send it as a normal patch series, the last one I've got here has "ASoC:
HACK: avoid creating duplicated widgets" as part of it for example.

That patch still hasn't gone away, and is still required, because there
has been no guidance or comments about the problem.  Let's explain it
yet again...

You have said "there is no problem registering the platform and the CPU
dai from the same device structure".  Let's assume that's a fact and see
what happens in the core code:

static int soc_probe_platform(struct snd_soc_card *card,
                           struct snd_soc_platform *platform)
{
        /* Create DAPM widgets for each DAI stream */
        list_for_each_entry(dai, &dai_list, list) {
                if (dai->dev != platform->dev)
                        continue;

                snd_soc_dapm_new_dai_widgets(&platform->dapm, dai);
        }
}

static int soc_probe_link_dais(struct snd_soc_card *card, int num, int order)
{
        if (!cpu_dai->probed &&
                        cpu_dai->driver->probe_order == order) {
                if (!cpu_dai->codec) {
                        cpu_dai->dapm.card = card;
                        if (!try_module_get(cpu_dai->dev->driver->owner))
                                return -ENODEV;

                        list_add(&cpu_dai->dapm.list, &card->dapm_list);
                        snd_soc_dapm_new_dai_widgets(&cpu_dai->dapm, cpu_dai);
                }

Now, the CPU DAI is added to the dai_list (it has to be there to be found
so the DAI link can bind it, and so soc_probe_link_dais() can be called.)

Think about what happens with the above code if platform->dev is the same
as the device used for the CPU DAI (dai->dev) - which can happen when the
platform and CPU DAI are registered from the same platform_device, which
you claim is legal with ASoC.

Now, look at snd_soc_dapm_new_dai_widgets():

int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm,
                                 struct snd_soc_dai *dai)
{
        if (dai->driver->playback.stream_name) {
...
                dai->playback_widget = w;
        }
        if (dai->driver->capture.stream_name) {
...
                dai->capture_widget = w;
        }

What happens if the widgets which are bound to are the first set that
are created, but they're overwritten when the second set get created?
(And that _does_ happen.)  The second set are the ones activated when
the audio device is opened, not the first set.

Now, there's nothing new in the above, I've already explained all the
above to you several times.  I've had nothing of any help what so ever
back from you on this.  I've asked you how to solve this.  I've had
absolutely nothing back.  So what am I supposed to do here?  Stuff
doesn't work with the core code how it is, so I took the only solution
_you_ left me by your silence, which is to hack the core code.

It does seem that your configuration is different to the configurations
that work well on Haswell, OMAP4 and Qualcomm and that's probably why
you are the only person reporting this atm. I also think the tight
coupling between the I2S and SPDIF HW made your problem far more complex
and therefore more difficult (for me at least) to follow when the
signal to noise ratio of this and related threads started to
deteriorate.

Both Mark and are are happy to fix things, but please remember that we
can't just jump and schedule this work in as top priority, we have to
prioritise work on severity and impact alongside that of our employers
and customers. I'm sure if things were the other way around (e.g the
problem was in the ARM core) then Mark would have to wait for you to
respond and fix the issue in your time frame. I'm also certain Mark
would not start making the conversation personal either.

As I've said, I'll do a proper fix for patch 4 and CC you on the
submission. The rest of the series looked ok and then I'm sure Mark will
take it.

Liam