linux-kernel - Re: [PATCH 0/2] ASoC: sdm845: fix soundwire stream handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a442a21e-47ce-f304-5bfb-06958d078a78@linux.intel.com>
Date:   Tue, 17 Mar 2020 06:05:06 -0500
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        broonie@...nel.org
Cc:     alsa-devel@...a-project.org, lgirdwood@...il.com,
        linux-kernel@...r.kernel.org, vkoul@...nel.org
Subject: Re: [PATCH 0/2] ASoC: sdm845: fix soundwire stream handling

Hi Srinivas,

On 3/17/20 4:53 AM, Srinivas Kandagatla wrote:
> Recent addition of SoundWire stream state-machine checks in linux-next
> have shown an existing issue with handling soundwire streams in codec drivers.
> 
> In general soundwire stream prepare/enable/disable can be called from either
> codec/machine/controller driver. However calling it in codec driver means
> that if multiple instances(Left/Right speakers) of the same codec is
> connected to the same stream then it will endup calling stream
> prepare/enable/disable more than once. This will mess up the stream
> state-machine checks in the soundwire core.

That's a known issue that we've fixed on the Intel side a  month or two 
ago. Unfortunately the review cycle is so slow that you don't benefit 
immediately from our fixes, what can I say.

> Moving this stream handling to machine driver would fix this issue
> and also allow board/platform specfic power sequencing.

It's fine but that's unnecessary, and if you start having multiple 
machine drivers with the same codecs you'll duplicate the stream 
handling code.

All you need to ensure in a multi-codec or multi-cpu dai case is that 
the stream is allocated once, and yes that's typically done as part of 
the dailink .startup callback.

Then it's fine to call the stream prepare or enable multiple times from 
the individual dai level, and only do the transition for the first call.

See patch "soundwire: stream: only change state if needed" that I just 
shared. We've used it both in 'TDM' mode with two codecs hanging off of 
the same link and in 'aggregated' mode with two codecs on separate links.

> Srinivas Kandagatla (2):
>    ASoC: qcom: sdm845: handle soundwire stream
>    ASoC: codecs: wsa881x: remove soundwire stream handling
> 
>   sound/soc/codecs/wsa881x.c | 44 +-----------------------
>   sound/soc/qcom/Kconfig     |  2 +-
>   sound/soc/qcom/sdm845.c    | 69 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 71 insertions(+), 44 deletions(-)
>