[alsa-devel] [PATCH V2 1/2] ALSA: Alchemy AC97C/I2SC audio support

[Manuel Lauss]

On Fri, Jul 22, 2011 at 1:54 AM, Lars-Peter Clausen <lars at metafoo.de> wrote:
>> diff --git a/sound/soc/au1x/db1000.c b/sound/soc/au1x/db1000.c
>> +     ret = -ENOMEM;
>> +     db1000_asoc97_dev = platform_device_alloc("soc-audio", 0);
>
> New drivers shouldn't user soc-audio anymore, just register a normal platform
> device driver.

Can you point to an example of the new way?


>> diff --git a/sound/soc/au1x/dma.c b/sound/soc/au1x/dma.c
>> new file mode 100644
>> index 0000000..0f7d90a
>> --- /dev/null
>> +++ b/sound/soc/au1x/dma.c
>> @@ -0,0 +1,470 @@
>> [...]
>> +
>> +static struct platform_driver alchemy_ac97pcm_driver = {
>> +     .driver = {
>> +             .name   = AC97C_DMANAME,
>> +             .owner  = THIS_MODULE,
>> +     },
>> +     .probe          = alchemy_pcm_drvprobe,
>> +     .remove         = __devexit_p(alchemy_pcm_drvremove),
>> +};
>> +
>> +static struct platform_driver alchemy_i2spcm_driver = {
>> +     .driver = {
>> +             .name   = I2SC_DMANAME,
>> +             .owner  = THIS_MODULE,
>> +     },
>> +     .probe          = alchemy_pcm_drvprobe,
>> +     .remove         = __devexit_p(alchemy_pcm_drvremove),
>> +};
>
> You shouldn't really have to register two identical drivers for this. If you
> really want to be able to instantiate the driver with two different names use
> platform_device_id. But in my opinion it should be enough to just have one
> generic name, since there is nothing AC97 or I2S specific in this driver.

I need a unique name for the DMA device in soc_dai_link.  This was the
easiest way. Especially since both ac97 and i2s can be active at
runtime.


>> [...]
>> +
>> +struct platform_device *alchemy_pcm_add(struct platform_device *pdev, int type)
>> +{
> +       struct resource *res, *r;
> +       struct platform_device *pd;
> +       char *pdevname;
> +       int id[2];
> +       int ret;
> +
> +       r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
> +       if (!r)
> +               return NULL;
> +       id[0] = r->start;
> +
> +       r = platform_get_resource(pdev, IORESOURCE_DMA, 1);
> +       if (!r)
> +               return NULL;
> +       id[1] = r->start;
> +
> +       res = kzalloc(sizeof(struct resource) * 2, GFP_KERNEL);
> +       if (!res)
> +               return NULL;
> +
> +       res[0].start = res[0].end = id[0];
> +       res[1].start = res[1].end = id[1];
> +       res[0].flags = res[1].flags = IORESOURCE_DMA;
> +
> +       /* "alchemy-pcm-ac97" or "alchemy-pcm-i2s" */
> +       pdevname = (type == 0) ? AC97C_DMANAME : I2SC_DMANAME;
> +       pd = platform_device_alloc(pdevname, -1);
> +       if (!pd)
> +               goto out;
> +
> +       pd->resource = res;
> +       pd->num_resources = 2;
> +
> +       ret = platform_device_add(pd);
> +       if (!ret)
> +               return pd;
> +
> +       platform_device_put(pd);
> +out:
> +       kfree(res);
> +       return NULL;
>> +}
>
> This function looks a bit fishy. The pcm driver should be registered by the
> platform code file as well. If you need different DMA regions for I2C and AC97
> use snd_soc_dai_set_dma_data and snd_soc_dai_get_dma_data to pass them to the
> PCM driver from the I2S or AC97 driver.

I like to pass the DMA id's along with the ac97/i2s resource
information (since they
belong together anyway). As an added benefit I get a sensibly named dma device
with the correct DMA information, all by simply registering the ac97
platform device.

I'll think about a way to change it.


Thank you!
      Manuel Lauss
--
To unsubscribe from this list: send the line "unsubscribe alsa-devel" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html