* [PATCH] ASoC: ams-delta: fix card initalization failure
@ 2012-08-27 21:28 Janusz Krzysztofik
2012-08-27 21:38 ` Mark Brown
0 siblings, 1 reply; 8+ messages in thread
From: Janusz Krzysztofik @ 2012-08-27 21:28 UTC (permalink / raw)
To: alsa-devel
Cc: Liam Girdwood, Mark Brown, linux-omap, linux-kernel, Janusz Krzysztofik
Since commit 0998d0631001288a5974afc0b2a5f568bcdecb4d, 'device-core:
Ensure drvdata = NULL when no driver is bound', the Amstrad Delta sound
card no longer initializes correctly due to drvdata reset to NULL by an
upper layer before the codec device, required for successful card setup,
is registered. Fix this by moving the codec registration bits up, before
the card is probed for.
Created and tested against linux-3.6-rc3
Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
---
sound/soc/omap/ams-delta.c | 18 ++++++++++--------
1 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/sound/soc/omap/ams-delta.c b/sound/soc/omap/ams-delta.c
index 7d4fa8e..270de9c 100644
--- a/sound/soc/omap/ams-delta.c
+++ b/sound/soc/omap/ams-delta.c
@@ -590,20 +590,22 @@ static int __init ams_delta_module_init(void)
if (!ams_delta_audio_platform_device)
return -ENOMEM;
- platform_set_drvdata(ams_delta_audio_platform_device,
- &ams_delta_audio_card);
-
- ret = platform_device_add(ams_delta_audio_platform_device);
- if (ret)
- goto err;
-
/*
* Codec platform device could be registered from elsewhere (board?),
* but I do it here as it makes sense only if used with the card.
+ * Moreover, it must be registered before the card is probed for,
+ * or the card setup fails due to drvdata reset by upper layers.
*/
cx20442_platform_device =
platform_device_register_simple("cx20442-codec", -1, NULL, 0);
- return 0;
+
+ platform_set_drvdata(ams_delta_audio_platform_device,
+ &ams_delta_audio_card);
+
+ ret = platform_device_add(ams_delta_audio_platform_device);
+ if (!ret)
+ return ret;
+
err:
platform_device_put(ams_delta_audio_platform_device);
return ret;
--
1.7.3.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] ASoC: ams-delta: fix card initalization failure
2012-08-27 21:28 [PATCH] ASoC: ams-delta: fix card initalization failure Janusz Krzysztofik
@ 2012-08-27 21:38 ` Mark Brown
2012-08-28 15:13 ` [alsa-devel] " Janusz Krzysztofik
0 siblings, 1 reply; 8+ messages in thread
From: Mark Brown @ 2012-08-27 21:38 UTC (permalink / raw)
To: Janusz Krzysztofik; +Cc: alsa-devel, Liam Girdwood, linux-omap, linux-kernel
On Mon, Aug 27, 2012 at 11:28:30PM +0200, Janusz Krzysztofik wrote:
> - platform_set_drvdata(ams_delta_audio_platform_device,
> - &ams_delta_audio_card);
> -
> - ret = platform_device_add(ams_delta_audio_platform_device);
> - if (ret)
> - goto err;
The real fix here is that you should be using platform data here, not
driver data. Is there some reason not to do that?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [alsa-devel] [PATCH] ASoC: ams-delta: fix card initalization failure
2012-08-27 21:38 ` Mark Brown
@ 2012-08-28 15:13 ` Janusz Krzysztofik
2012-08-28 18:13 ` Mark Brown
0 siblings, 1 reply; 8+ messages in thread
From: Janusz Krzysztofik @ 2012-08-28 15:13 UTC (permalink / raw)
To: alsa-devel; +Cc: Mark Brown, linux-omap, Liam Girdwood, linux-kernel
On Mon, 27 Aug 2012 14:38:35 Mark Brown wrote:
> On Mon, Aug 27, 2012 at 11:28:30PM +0200, Janusz Krzysztofik wrote:
>
> > - platform_set_drvdata(ams_delta_audio_platform_device,
> > - &ams_delta_audio_card);
> > -
> > - ret = platform_device_add(ams_delta_audio_platform_device);
> > - if (ret)
> > - goto err;
>
> The real fix here is that you should be using platform data here, not
> driver data. Is there some reason not to do that?
Mark,
Do you think the change you propose is suitable for the rc cycle? I'm
trying to fix a regression in the first place. Converting the ams-delta
asoc to a platform driver is on my todo list and I'm going to take care
of this as soon as I have enough spare time.
Thanks,
Janusz
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [alsa-devel] [PATCH] ASoC: ams-delta: fix card initalization failure
2012-08-28 15:13 ` [alsa-devel] " Janusz Krzysztofik
@ 2012-08-28 18:13 ` Mark Brown
2012-08-29 5:04 ` Janusz Krzysztofik
0 siblings, 1 reply; 8+ messages in thread
From: Mark Brown @ 2012-08-28 18:13 UTC (permalink / raw)
To: Janusz Krzysztofik; +Cc: alsa-devel, linux-omap, Liam Girdwood, linux-kernel
On Tue, Aug 28, 2012 at 05:13:05PM +0200, Janusz Krzysztofik wrote:
> On Mon, 27 Aug 2012 14:38:35 Mark Brown wrote:
> > On Mon, Aug 27, 2012 at 11:28:30PM +0200, Janusz Krzysztofik wrote:
> > > - platform_set_drvdata(ams_delta_audio_platform_device,
> > > - &ams_delta_audio_card);
> > The real fix here is that you should be using platform data here, not
> > driver data. Is there some reason not to do that?
> Do you think the change you propose is suitable for the rc cycle? I'm
> trying to fix a regression in the first place. Converting the ams-delta
> asoc to a platform driver is on my todo list and I'm going to take care
> of this as soon as I have enough spare time.
The above looks like you already have a platform driver? All I'm
suggesting is changing the above to use platform rather than driver
data.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [alsa-devel] [PATCH] ASoC: ams-delta: fix card initalization failure
2012-08-28 18:13 ` Mark Brown
@ 2012-08-29 5:04 ` Janusz Krzysztofik
2012-08-31 21:31 ` Mark Brown
0 siblings, 1 reply; 8+ messages in thread
From: Janusz Krzysztofik @ 2012-08-29 5:04 UTC (permalink / raw)
To: alsa-devel; +Cc: Mark Brown, linux-omap, Liam Girdwood, linux-kernel
On Tue, 28 Aug 2012 11:13:39 Mark Brown wrote:
> On Tue, Aug 28, 2012 at 05:13:05PM +0200, Janusz Krzysztofik wrote:
> > On Mon, 27 Aug 2012 14:38:35 Mark Brown wrote:
> > > On Mon, Aug 27, 2012 at 11:28:30PM +0200, Janusz Krzysztofik wrote:
>
> > > > - platform_set_drvdata(ams_delta_audio_platform_device,
> > > > - &ams_delta_audio_card);
>
> > > The real fix here is that you should be using platform data here,
not
> > > driver data. Is there some reason not to do that?
>
> > Do you think the change you propose is suitable for the rc cycle?
I'm
> > trying to fix a regression in the first place. Converting the ams-
delta
> > asoc to a platform driver is on my todo list and I'm going to take
care
> > of this as soon as I have enough spare time.
>
> The above looks like you already have a platform driver? All I'm
> suggesting is changing the above to use platform rather than driver
> data.
The ams-delta asoc driver doesn't use snd_soc_register_card() so far,
but relays solely on soc_probe() doing this for it, which in turn
expects to find a snc_soc_card structure in drvdata. How is it supposed
to find that structure if I pass it over platform data instead? Am I
missing something?
Thanks,
Janusz
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [alsa-devel] [PATCH] ASoC: ams-delta: fix card initalization failure
2012-08-29 5:04 ` Janusz Krzysztofik
@ 2012-08-31 21:31 ` Mark Brown
2012-09-01 9:09 ` Janusz Krzysztofik
0 siblings, 1 reply; 8+ messages in thread
From: Mark Brown @ 2012-08-31 21:31 UTC (permalink / raw)
To: Janusz Krzysztofik; +Cc: alsa-devel, linux-omap, Liam Girdwood, linux-kernel
On Wed, Aug 29, 2012 at 07:04:48AM +0200, Janusz Krzysztofik wrote:
> On Tue, 28 Aug 2012 11:13:39 Mark Brown wrote:
> > The above looks like you already have a platform driver? All I'm
> > suggesting is changing the above to use platform rather than driver
> > data.
> The ams-delta asoc driver doesn't use snd_soc_register_card() so far,
> but relays solely on soc_probe() doing this for it, which in turn
> expects to find a snc_soc_card structure in drvdata. How is it supposed
> to find that structure if I pass it over platform data instead? Am I
> missing something?
s/drvdata/platdata/ in the code. If you can't do this then just
referencing the data directly in the code would be better than this
bodge, it'd be much less fragile.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [alsa-devel] [PATCH] ASoC: ams-delta: fix card initalization failure
2012-08-31 21:31 ` Mark Brown
@ 2012-09-01 9:09 ` Janusz Krzysztofik
2012-09-06 7:36 ` Mark Brown
0 siblings, 1 reply; 8+ messages in thread
From: Janusz Krzysztofik @ 2012-09-01 9:09 UTC (permalink / raw)
To: alsa-devel; +Cc: Mark Brown, linux-omap, Liam Girdwood, linux-kernel
Dnia piątek, 31 sierpnia 2012 14:31:04 Mark Brown pisze:
> On Wed, Aug 29, 2012 at 07:04:48AM +0200, Janusz Krzysztofik wrote:
> > On Tue, 28 Aug 2012 11:13:39 Mark Brown wrote:
>
> > > The above looks like you already have a platform driver?
Mark,
I should have rather answered: No, ams-delta.c is not a platform driver,
only a module which registeres a soc-audio device and provides device
specific data and callbacks to the soc-audio platform driver.
> > > All I'm
> > > suggesting is changing the above to use platform rather than
driver
> > > data.
> ...
> s/drvdata/platdata/ in the code.
Taking the above into account, I would have to apply this substitue to
the soc-core.c, which is actually the platform driver for the ams-delta
ASoC device, and this would break a lot of other cards.
> If you can't do this then just
> referencing the data directly in the code would be better than this
> bodge, it'd be much less fragile.
I see your point, however for now I can see no better way of referencing
the data (of type struct snd_soc_card) then passing it to
snd_soc_register_card(). But for this to work, I would have to register
successfully an ams-delta specific platform device first, not the soc-
audio. This, even if still done from the sound/soc/omap/ams-delta.c, not
from an arch board file, would require now not existing ams-delta ASoC
platform driver probe/remove callbacks at least. I'm still not convinced
if such modification would be acceptable in the middle of the rc cycle.
If there is a simpler, less intrusive way to do this, then sorry, I
still can't see it.
Thanks,
Janusz
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [alsa-devel] [PATCH] ASoC: ams-delta: fix card initalization failure
2012-09-01 9:09 ` Janusz Krzysztofik
@ 2012-09-06 7:36 ` Mark Brown
0 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2012-09-06 7:36 UTC (permalink / raw)
To: Janusz Krzysztofik; +Cc: alsa-devel, linux-omap, Liam Girdwood, linux-kernel
On Sat, Sep 01, 2012 at 11:09:18AM +0200, Janusz Krzysztofik wrote:
> I see your point, however for now I can see no better way of referencing
> the data (of type struct snd_soc_card) then passing it to
> snd_soc_register_card(). But for this to work, I would have to register
> successfully an ams-delta specific platform device first, not the soc-
> audio. This, even if still done from the sound/soc/omap/ams-delta.c, not
> from an arch board file, would require now not existing ams-delta ASoC
> platform driver probe/remove callbacks at least. I'm still not convinced
> if such modification would be acceptable in the middle of the rc cycle.
> If there is a simpler, less intrusive way to do this, then sorry, I
> still can't see it.
Like I already said just make it a static variable.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-09-06 7:56 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-27 21:28 [PATCH] ASoC: ams-delta: fix card initalization failure Janusz Krzysztofik
2012-08-27 21:38 ` Mark Brown
2012-08-28 15:13 ` [alsa-devel] " Janusz Krzysztofik
2012-08-28 18:13 ` Mark Brown
2012-08-29 5:04 ` Janusz Krzysztofik
2012-08-31 21:31 ` Mark Brown
2012-09-01 9:09 ` Janusz Krzysztofik
2012-09-06 7:36 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).