[alsa-devel] [PATCH] ASoC: samsung - Don't setup i2s if already active.

[Dylan Reid]

On Fri, Nov 9, 2012 at 8:08 AM, Mark Brown
<broonie at opensource.wolfsonmicro.com> wrote:
> On Thu, Nov 08, 2012 at 02:46:56PM -0800, Dylan Reid wrote:
>> If the dai is already running when setup is called, skip setup.  Setting
>> up again caused set_sysclk to set rclk_srcrate to other->rclk_srcrate,
>> other would often be set to the default of 44.1.  Playing a 48k stream
>> then adding a 48k capture stream would lead to both streams set to 44.1
>> (The value of other->rclk_srcrate).
>
> This doesn't seem the obvious fix here - surely if the setup comes out
> with the wrong answer the configuration it was asked for doesn't match
> the configuration the device currently has and therefore we should
> return an error as the new stream will have an incorrect setup?

Thanks Mark, sorry I didn't explain this better.  It is getting the
correct answer.  However if the same format is requested for another
stream, startup will clear rclk_srcrate.  rclk_srcrate being cleared
causes i2s_set_fmt to call i2s_set_sysclk.  i2s_set_sysclk assumes it
should use the "other" setting for rclk_srcrate (because something is
active).

for example:

aplay -Dhw:0 some_48k_file.wav
followed by
arecord -Dhw:0 -f dat /tmp/foo.wav

both 48k streams, but when the arecord is started, the aplay will have
its rate set to what ever the unused secondary DAI is using
(initialized to 44.1).

>
> Really this driver should probably be being redone in terms of the DPCM
> code, it predates it and is a very simple case of it but it's in the
> same ballpark feature wise.

That looks promising, not only for this but for a few other issues.
I'll take investigate some more, thanks for the suggestion.

>
>> Similarly in shutdown, if either playback or capture is still active,
>> return instead of turning off the clocks.
>
> This seems sensible; another option is to do clk_enable() for both
> streams then let the clock framework refcount for us.