Re: [alsa-devel] Question about pxa2xx-ac97-lib.c
Marek Vasut marek.vasut@gmail.com writes:
One more thing, I'd like to ask how to change the reset_gpio in the above file. Will I have to make copy of pxa_ac97_dai[PXA2XX_DAI_AC97_HIFI] in palm27x.c and change the .probe pointer to my own function that sets the proper platform data? That is, something like the following? Thanks
I'll add Mark to the discussion here, for the ASoC point of view. Mark, the original mail is in [1].
I originaly thought we would change pxa_set_ac97_info(), to add a parameter which will be the platform data for the gpio.
IOW, have something like : void __init pxa_set_ac97_info(struct pxa2xx_ac97_platform_data *pdata) { pxa_register_device(&pxa_device_ac97, ops); }
The trouble is, I saw references to the platform data as pxa2xx_audio_ops_t*, mostly to suspend and resume a card. I've seen them in pxa2xx_ac97_do_suspend(), pxa2xx_ac97_do_resume().
Mark, could you please have a glance at it, and tell us the right approach to have the struct pxa2xx_ac97_platform_data passed to pxa2xx-ac97 please ?
Cheers.
-- Robert
[1] :
diff --git a/sound/soc/pxa/palm27x.c b/sound/soc/pxa/palm27x.c index 48a73f6..f134f95 100644 --- a/sound/soc/pxa/palm27x.c +++ b/sound/soc/pxa/palm27x.c @@ -19,11 +19,13 @@ #include <linux/gpio.h> #include <linux/interrupt.h> #include <linux/irq.h> +#include <linux/platform_device.h>
#include <sound/core.h> #include <sound/pcm.h> #include <sound/soc.h> #include <sound/soc-dapm.h> +#include <sound/pxa2xx-lib.h>
#include <asm/mach-types.h> #include <mach/audio.h> @@ -168,6 +170,21 @@ static int palm27x_ac97_init(struct snd_soc_codec *codec) return 0; } // this is the altered probe function +/* Palms use GPIO95 for AC97 reset */ +static int palm_ac97_probe(struct platform_device *pdev,
struct snd_soc_dai *dai)+{
struct platform_device *pd = to_platform_device(dai->dev);static struct pxa2xx_ac97_platform_data pdata = {.reset_gpio = 95,};printk("%s[%i]\n", __FUNCTION__, __LINE__);pd->dev.platform_data = &pdata;return pxa2xx_ac97_hw_probe(pd);+}
+static struct snd_soc_dai hifi_cpu_dai;
static struct snd_soc_dai_link palm27x_dai[] = { { .name = "AC97 HiFi", @@ -204,6 +221,8 @@ static int __init palm27x_asoc_init(void) { int ret; // here it'll be copied
pxa_ac97_dai[PXA2XX_DAI_AC97_HIFI].probe = palm_ac97_probe;if (!(machine_is_palmtx() || machine_is_palmt5() || machine_is_palmld())) return -ENODEV;
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
On Sun, Apr 12, 2009 at 06:36:49PM +0200, Robert Jarzmik wrote:
I originaly thought we would change pxa_set_ac97_info(), to add a parameter which will be the platform data for the gpio.
Yes, this is the approach that should be adopted. The platform data should not change per board.
IOW, have something like : void __init pxa_set_ac97_info(struct pxa2xx_ac97_platform_data *pdata) { pxa_register_device(&pxa_device_ac97, ops); }
Note that we already have platform data for the PXA AC97 devices.
The trouble is, I saw references to the platform data as pxa2xx_audio_ops_t*, mostly to suspend and resume a card. I've seen them in pxa2xx_ac97_do_suspend(), pxa2xx_ac97_do_resume().
Mark, could you please have a glance at it, and tell us the right approach to have the struct pxa2xx_ac97_platform_data passed to pxa2xx-ac97 please ?
Those are platform data, currently the only mainline use I'm aware of is for mainstone - it's been present since before the pxa2xx-ac97-lib was introduced.
Looking at the code it appears we've broken mainstone, though unfortunately my mainstone has some independant problem with resume so I can't test but I'll make a patch merging the two structures on Monday.
On Sunday 12 of April 2009 18:36:49 Robert Jarzmik wrote:
Marek Vasut marek.vasut@gmail.com writes:
One more thing, I'd like to ask how to change the reset_gpio in the above file. Will I have to make copy of pxa_ac97_dai[PXA2XX_DAI_AC97_HIFI] in palm27x.c and change the .probe pointer to my own function that sets the proper platform data? That is, something like the following? Thanks
I'll add Mark to the discussion here, for the ASoC point of view. Mark, the original mail is in [1].
I originaly thought we would change pxa_set_ac97_info(), to add a parameter which will be the platform data for the gpio.
IOW, have something like : void __init pxa_set_ac97_info(struct pxa2xx_ac97_platform_data *pdata) { pxa_register_device(&pxa_device_ac97, ops); }
The trouble is, I saw references to the platform data as pxa2xx_audio_ops_t*, mostly to suspend and resume a card. I've seen them in pxa2xx_ac97_do_suspend(), pxa2xx_ac97_do_resume().
Mark, could you please have a glance at it, and tell us the right approach to have the struct pxa2xx_ac97_platform_data passed to pxa2xx-ac97 please ?
Cheers.
-- Robert
I've already noticed this way of doing it is very bad ;-) I made a patch that should be correct and I'm sending it to LAK in a while. I'll CC you for your opinion, thanks.
[1] :
diff --git a/sound/soc/pxa/palm27x.c b/sound/soc/pxa/palm27x.c index 48a73f6..f134f95 100644 --- a/sound/soc/pxa/palm27x.c +++ b/sound/soc/pxa/palm27x.c @@ -19,11 +19,13 @@ #include <linux/gpio.h> #include <linux/interrupt.h> #include <linux/irq.h> +#include <linux/platform_device.h>
#include <sound/core.h> #include <sound/pcm.h> #include <sound/soc.h> #include <sound/soc-dapm.h> +#include <sound/pxa2xx-lib.h>
#include <asm/mach-types.h> #include <mach/audio.h> @@ -168,6 +170,21 @@ static int palm27x_ac97_init(struct snd_soc_codec *codec) return 0; } // this is the altered probe function +/* Palms use GPIO95 for AC97 reset */ +static int palm_ac97_probe(struct platform_device *pdev,
struct snd_soc_dai *dai)+{
struct platform_device *pd = to_platform_device(dai->dev);static struct pxa2xx_ac97_platform_data pdata = {.reset_gpio = 95,};printk("%s[%i]\n", __FUNCTION__, __LINE__);pd->dev.platform_data = &pdata;return pxa2xx_ac97_hw_probe(pd);+}
+static struct snd_soc_dai hifi_cpu_dai;
static struct snd_soc_dai_link palm27x_dai[] = { { .name = "AC97 HiFi", @@ -204,6 +221,8 @@ static int __init palm27x_asoc_init(void) { int ret; // here it'll be copied
pxa_ac97_dai[PXA2XX_DAI_AC97_HIFI].probe = palm_ac97_probe;if (!(machine_is_palmtx() || machine_is_palmt5() || machine_is_palmld())) return -ENODEV;
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
participants (3)
-
Marek Vasut -
Mark Brown -
Robert Jarzmik