[PATCH] asoc: add dummy codec

[PATCH] asoc: add dummy codec

[Stas Sergeev]

[-- Attachment #1: Type: text/plain, Size: 681 bytes --]

Hi.

The attached patch adds a dummy codec for the
asoc subsystem.
I use it with the driver that programs the soc to drive
I2S from an internal PLLs, and the audio then goes to
HDMI, so no external codec is used.
I've found spdif_transciever.c, which exactly is such a
dummy driver, but it is bound to the particular plat
device, so I made the device name a config option, and
if not set, the plat device is created with the default
name.

It may be possible to avoid the usage of the codec driver
at all, but I haven't succeeded with this, and the existance
of spdif_transciever.c suggests there is no such way.
Thoughts?

Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>

[-- Attachment #2: 0001-asoc-add-dummy-codec.patch --]
[-- Type: application/x-patch, Size: 4553 bytes --]

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] asoc: add dummy codec

[Lars-Peter Clausen]

On 04/11/2013 12:36 PM, Stas Sergeev wrote:
> Hi.
> 
> The attached patch adds a dummy codec for the
> asoc subsystem.
> I use it with the driver that programs the soc to drive
> I2S from an internal PLLs, and the audio then goes to
> HDMI, so no external codec is used.
> I've found spdif_transciever.c, which exactly is such a
> dummy driver, but it is bound to the particular plat
> device, so I made the device name a config option, and
> if not set, the plat device is created with the default
> name.
> 
> It may be possible to avoid the usage of the codec driver
> at all, but I haven't succeeded with this, and the existance
> of spdif_transciever.c suggests there is no such way.
> Thoughts?
> 
> Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
> 

Hi,

There is already a dummy codec driver.

Please take a look at sound/soc/soc-utils.c

- Lars

Re: [PATCH] asoc: add dummy codec

[Stas Sergeev]

11.04.2013 14:41, Lars-Peter Clausen пишет:
> On 04/11/2013 12:36 PM, Stas Sergeev wrote:
>> Hi.
>>
>> The attached patch adds a dummy codec for the
>> asoc subsystem.
>> I use it with the driver that programs the soc to drive
>> I2S from an internal PLLs, and the audio then goes to
>> HDMI, so no external codec is used.
>> I've found spdif_transciever.c, which exactly is such a
>> dummy driver, but it is bound to the particular plat
>> device, so I made the device name a config option, and
>> if not set, the plat device is created with the default
>> name.
>>
>> It may be possible to avoid the usage of the codec driver
>> at all, but I haven't succeeded with this, and the existance
>> of spdif_transciever.c suggests there is no such way.
>> Thoughts?
>>
>> Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
>>
> Hi,
>
> There is already a dummy codec driver.
>
> Please take a look at sound/soc/soc-utils.c
Hi, I've seen that, but how can I get use of it?
It doesn't do snd_soc_register_codec(), and in debugfs
I can do 'cat asoc/codecs' and it shows only 'dummy-codec',
which is installed by my driver.
So could you please clarify?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] asoc: add dummy codec

[Lars-Peter Clausen]

On 04/11/2013 03:57 PM, Stas Sergeev wrote:
> 11.04.2013 14:41, Lars-Peter Clausen пишет:
>> On 04/11/2013 12:36 PM, Stas Sergeev wrote:
>>> Hi.
>>>
>>> The attached patch adds a dummy codec for the
>>> asoc subsystem.
>>> I use it with the driver that programs the soc to drive
>>> I2S from an internal PLLs, and the audio then goes to
>>> HDMI, so no external codec is used.
>>> I've found spdif_transciever.c, which exactly is such a
>>> dummy driver, but it is bound to the particular plat
>>> device, so I made the device name a config option, and
>>> if not set, the plat device is created with the default
>>> name.
>>>
>>> It may be possible to avoid the usage of the codec driver
>>> at all, but I haven't succeeded with this, and the existance
>>> of spdif_transciever.c suggests there is no such way.
>>> Thoughts?
>>>
>>> Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
>>>
>> Hi,
>>
>> There is already a dummy codec driver.
>>
>> Please take a look at sound/soc/soc-utils.c
> Hi, I've seen that, but how can I get use of it?
> It doesn't do snd_soc_register_codec(), and in debugfs

It does:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/sound/soc/soc-utils.c#n101

- Lars

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] asoc: add dummy codec

[Stas Sergeev]

11.04.2013 18:21, Lars-Peter Clausen пишет:
> On 04/11/2013 03:57 PM, Stas Sergeev wrote:
>> 11.04.2013 14:41, Lars-Peter Clausen пишет:
>>> On 04/11/2013 12:36 PM, Stas Sergeev wrote:
>>>> Hi.
>>>>
>>>> The attached patch adds a dummy codec for the
>>>> asoc subsystem.
>>>> I use it with the driver that programs the soc to drive
>>>> I2S from an internal PLLs, and the audio then goes to
>>>> HDMI, so no external codec is used.
>>>> I've found spdif_transciever.c, which exactly is such a
>>>> dummy driver, but it is bound to the particular plat
>>>> device, so I made the device name a config option, and
>>>> if not set, the plat device is created with the default
>>>> name.
>>>>
>>>> It may be possible to avoid the usage of the codec driver
>>>> at all, but I haven't succeeded with this, and the existance
>>>> of spdif_transciever.c suggests there is no such way.
>>>> Thoughts?
>>>>
>>>> Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
>>>>
>>> Hi,
>>>
>>> There is already a dummy codec driver.
>>>
>>> Please take a look at sound/soc/soc-utils.c
>> Hi, I've seen that, but how can I get use of it?
>> It doesn't do snd_soc_register_codec(), and in debugfs
> It does:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/sound/soc/soc-utils.c#n101
Arr, that's it, thank you!
It doesn't do so on an android kernel I have here
(based on linux-3.0), but it indeed does so in a git kernel...
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] asoc: add dummy codec

[Stas Sergeev]

[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]

11.04.2013 14:41, Lars-Peter Clausen пишет:
> On 04/11/2013 12:36 PM, Stas Sergeev wrote:
>> Hi.
>>
>> The attached patch adds a dummy codec for the
>> asoc subsystem.
>> I use it with the driver that programs the soc to drive
>> I2S from an internal PLLs, and the audio then goes to
>> HDMI, so no external codec is used.
>> I've found spdif_transciever.c, which exactly is such a
>> dummy driver, but it is bound to the particular plat
>> device, so I made the device name a config option, and
>> if not set, the plat device is created with the default
>> name.
>>
>> It may be possible to avoid the usage of the codec driver
>> at all, but I haven't succeeded with this, and the existance
>> of spdif_transciever.c suggests there is no such way.
>> Thoughts?
>>
>> Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
>>
> Hi,
>
> There is already a dummy codec driver.
>
> Please take a look at sound/soc/soc-utils.c
>
> - Lars
Lars, indeed, this codec is present in a git kernel and
it wasn't in mine android kernel.
But, that driver is too dummy to even work at all.

The attached patch adds a playback stream for the dummy
codec. Would it be possible to apply this?

Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>


[-- Attachment #2: 0001-asoc-define-playback-stream-in-dummy-codec.patch --]
[-- Type: application/x-patch, Size: 900 bytes --]

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] asoc: add dummy codec

[Gabriel M. Beddingfield]

On 04/11/2013 08:11 AM, Stas Sergeev wrote:
>> There is already a dummy codec driver.
>>
>> Please take a look at sound/soc/soc-utils.c
>>
>> - Lars
> Lars, indeed, this codec is present in a git kernel and
> it wasn't in mine android kernel.
> But, that driver is too dummy to even work at all.
> 
> The attached patch adds a playback stream for the dummy
> codec. Would it be possible to apply this?

No... not like that.

Instead, wouldn't it be better to extend it by defining some platform data?  Maybe something like:

diff --git a/sound/soc/soc-utils.c b/sound/soc/soc-utils.c
index fe4541d..9d22718 100644
--- a/sound/soc/soc-utils.c
+++ b/sound/soc/soc-utils.c
@@ -96,9 +96,14 @@ static struct snd_soc_dai_driver dummy_dai = {
 
 static int snd_soc_dummy_probe(struct platform_device *pdev)
 {
+       struct snd_soc_dummy_dai_pdata *pdata = dev_get_platdata(&pdev->dev);
        int ret;
 
-       ret = snd_soc_register_codec(&pdev->dev, &dummy_codec, &dummy_dai, 1);
+       if (pdata->n_dai > 0)
+               ret = snd_soc_register_codec(&pdev->dev, &dummy_codec,
+                                            &pdata->dai, &pdata->n_dai);
+       else
+               ret = snd_soc_register_codec(&pdev->dev, &dummy_codec, &dummy_dai, 1);
        if (ret < 0)
                return ret;
 

...then you can define whatever kind of DAI you want in the machine driver.

-gabriel

Re: [PATCH] asoc: add dummy codec

[Stas Sergeev]

11.04.2013 19:48, Gabriel M. Beddingfield пишет:
> On 04/11/2013 08:11 AM, Stas Sergeev wrote:
>>> There is already a dummy codec driver.
>>>
>>> Please take a look at sound/soc/soc-utils.c
>>>
>>> - Lars
>> Lars, indeed, this codec is present in a git kernel and
>> it wasn't in mine android kernel.
>> But, that driver is too dummy to even work at all.
>>
>> The attached patch adds a playback stream for the dummy
>> codec. Would it be possible to apply this?
> No... not like that.
>
> Instead, wouldn't it be better to extend it by defining some platform data?  Maybe something like:
>
> diff --git a/sound/soc/soc-utils.c b/sound/soc/soc-utils.c
> index fe4541d..9d22718 100644
> --- a/sound/soc/soc-utils.c
> +++ b/sound/soc/soc-utils.c
> @@ -96,9 +96,14 @@ static struct snd_soc_dai_driver dummy_dai = {
>  
>  static int snd_soc_dummy_probe(struct platform_device *pdev)
>  {
> +       struct snd_soc_dummy_dai_pdata *pdata = dev_get_platdata(&pdev->dev);
>         int ret;
>  
> -       ret = snd_soc_register_codec(&pdev->dev, &dummy_codec, &dummy_dai, 1);
> +       if (pdata->n_dai > 0)
> +               ret = snd_soc_register_codec(&pdev->dev, &dummy_codec,
> +                                            &pdata->dai, &pdata->n_dai);
> +       else
> +               ret = snd_soc_register_codec(&pdev->dev, &dummy_codec, &dummy_dai, 1);
>         if (ret < 0)
>                 return ret;
>  
>
> ...then you can define whatever kind of DAI you want in the machine driver.
But that's a _dummy_ codec, it simply does this:
---
soc_dummy_dev = platform_device_alloc("snd-soc-dummy", -1);
if (!soc_dummy_dev)
return -ENOMEM;
ret = platform_device_add(soc_dummy_dev);
---
in the very same source file.
What is the reason of adding platform data for a dummy driver?
The code snippet I propose, is copied from the spdif_receiver.c,
so its not something I invented myself. :)
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] asoc: add dummy codec

[Gabriel M. Beddingfield]

On 04/11/2013 09:03 AM, Stas Sergeev wrote:
> What is the reason of adding platform data for a dummy driver?

Because some audio "codecs" have a static configuration or are 
configured by magic.  (E.g. a modem in a phone.)  I think that being 
able to declare a board-specific set of "dummy DAI's" in the board file 
is helpful.

-gabriel

Re: [PATCH] asoc: add dummy codec

[Stas Sergeev]

11.04.2013 20:13, Gabriel M. Beddingfield пишет:
> On 04/11/2013 09:03 AM, Stas Sergeev wrote:
>> What is the reason of adding platform data for a dummy driver?
>
> Because some audio "codecs" have a static configuration or are
> configured by magic.  (E.g. a modem in a phone.)  I think that being
> able to declare a board-specific set of "dummy DAI's" in the board
> file is helpful.
I don't understand your suggestion yet.
Currently this driver creates the platform device on its own.
Do you propose to remove this first, and rely on the boards
to define "snd-soc-dummy" device with plat data?
Could you please also give a practical example where the
current "snd-soc-dummy-dai" is not enough? My driver
defines 2 DAIs, but I linked them both to "snd-soc-dummy-dai",
and that seem to work. But I am not familiar with this subsystem,
so maybe I did something wrong.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] asoc: add dummy codec

[Lars-Peter Clausen]

On 04/11/2013 05:48 PM, Gabriel M. Beddingfield wrote:
> On 04/11/2013 08:11 AM, Stas Sergeev wrote:
>>> There is already a dummy codec driver.
>>>
>>> Please take a look at sound/soc/soc-utils.c
>>>
>>> - Lars
>> Lars, indeed, this codec is present in a git kernel and
>> it wasn't in mine android kernel.
>> But, that driver is too dummy to even work at all.
>>
>> The attached patch adds a playback stream for the dummy
>> codec. Would it be possible to apply this?
> 
> No... not like that.
> 
> Instead, wouldn't it be better to extend it by defining some platform data?  Maybe something like:

Either you want a dummy or you want one with specific requirements. For the
former case use the dummy codec, for the later case add a proper driver.
Putting these kinds of things in your board file is in my opinion a bad habit.

- Lars

> 
> diff --git a/sound/soc/soc-utils.c b/sound/soc/soc-utils.c
> index fe4541d..9d22718 100644
> --- a/sound/soc/soc-utils.c
> +++ b/sound/soc/soc-utils.c
> @@ -96,9 +96,14 @@ static struct snd_soc_dai_driver dummy_dai = {
>  
>  static int snd_soc_dummy_probe(struct platform_device *pdev)
>  {
> +       struct snd_soc_dummy_dai_pdata *pdata = dev_get_platdata(&pdev->dev);
>         int ret;
>  
> -       ret = snd_soc_register_codec(&pdev->dev, &dummy_codec, &dummy_dai, 1);
> +       if (pdata->n_dai > 0)
> +               ret = snd_soc_register_codec(&pdev->dev, &dummy_codec,
> +                                            &pdata->dai, &pdata->n_dai);
> +       else
> +               ret = snd_soc_register_codec(&pdev->dev, &dummy_codec, &dummy_dai, 1);
>         if (ret < 0)
>                 return ret;
>  
> 
> ...then you can define whatever kind of DAI you want in the machine driver.
> 
> -gabriel
> 

Re: [PATCH] asoc: add dummy codec

[Gabriel M. Beddingfield]

On 04/11/2013 11:02 AM, Lars-Peter Clausen wrote:
>> Instead, wouldn't it be better to extend it by defining some platform data?  Maybe something like:
>
> Either you want a dummy or you want one with specific requirements. For the
> former case use the dummy codec, for the later case add a proper driver.
> Putting these kinds of things in your board file is in my opinion a bad habit.

Well, I've seen a couple cases where a dummy codec /is/ the proper 
driver and all that's needed is to restrict the formats that the DAI 
supports (sample rate, etc).

Yes, we can always write a board-specific dummy codec with the 
restricted formats -- but since there's already one in ASoC, why not 
make it extensible?

-gabriel

Re: [PATCH] asoc: add dummy codec

[Gabriel M. Beddingfield]

On 04/11/2013 08:48 AM, Gabriel M. Beddingfield wrote:
>> The attached patch adds a playback stream for the dummy
>> codec. Would it be possible to apply this?
>
> No... not like that.

I was wrong.  Sorry.

That patch is on the right track.  As Lars said, it needs a Capture DAI, 
too.  Might also consider adding more audio formats.

> Instead, wouldn't it be better to extend it by defining some platform data?
 > Maybe something like:

I still think that making this extensible is a good idea (platform 
data), but fall back on a very permissive default DAI (like in your patch).

-gabriel

Re: [PATCH] asoc: add dummy codec

[Stas Sergeev]

12.04.2013 06:50, Gabriel M. Beddingfield пишет:
> I still think that making this extensible is a good idea (platform
> data), but fall back on a very permissive default DAI (like in your
> patch).
OK, I see your point, and I think it would be nice
to have some quick way to add the format restrictions
to the DAI without writing a new dummy codec every time.
Maybe even the runtime-adjustable restrictions would be
nice in some cases (when you plug and unplug different
devices to the HDMI transmitter, which is my use-case now).
But I am afraid this will require much more considerations
than just a quick hack on a currently existing dummy codec.
This is out of the scope for my patch. I'll go the short route. :)
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] asoc: add dummy codec

[Lars-Peter Clausen]

On 04/11/2013 05:11 PM, Stas Sergeev wrote:
> 11.04.2013 14:41, Lars-Peter Clausen пишет:
>> On 04/11/2013 12:36 PM, Stas Sergeev wrote:
>>> Hi.
>>>
>>> The attached patch adds a dummy codec for the
>>> asoc subsystem.
>>> I use it with the driver that programs the soc to drive
>>> I2S from an internal PLLs, and the audio then goes to
>>> HDMI, so no external codec is used.
>>> I've found spdif_transciever.c, which exactly is such a
>>> dummy driver, but it is bound to the particular plat
>>> device, so I made the device name a config option, and
>>> if not set, the plat device is created with the default
>>> name.
>>>
>>> It may be possible to avoid the usage of the codec driver
>>> at all, but I haven't succeeded with this, and the existance
>>> of spdif_transciever.c suggests there is no such way.
>>> Thoughts?
>>>
>>> Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
>>>
>> Hi,
>>
>> There is already a dummy codec driver.
>>
>> Please take a look at sound/soc/soc-utils.c
>>
>> - Lars
> Lars, indeed, this codec is present in a git kernel and
> it wasn't in mine android kernel.
> But, that driver is too dummy to even work at all.
> 
> The attached patch adds a playback stream for the dummy
> codec. Would it be possible to apply this?

Looks fine to me, although it might make sense to also add a dummy capture
stream. But you need to send the patch to Mark and Liam who are the ASoC
maintainers. Also use the proper subsystem prefix, which is "ASoC" not "asoc",
add a patch description, sign off on the patch itself and try to send it inline
instead of an attachment.

- Lars
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel