From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Charles Keepax <ckeepax@opensource.cirrus.com>,
Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Cc: Linux-ALSA <alsa-devel@alsa-project.org>,
Mark Brown <broonie@kernel.org>,
Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
Vinod Koul <vkoul@kernel.org>
Subject: Re: Question about DPCM private_data of soc-compress.c
Date: Mon, 23 Jan 2023 09:44:35 -0600 [thread overview]
Message-ID: <6a5131ee-bce2-968a-8371-10ce7772f449@linux.intel.com> (raw)
In-Reply-To: <20230123120134.GW36097@ediswmail.ad.cirrus.com>
On 1/23/23 06:01, Charles Keepax wrote:
> On Mon, Jan 23, 2023 at 03:08:49AM +0000, Kuninori Morimoto wrote:
>>
>> Hi ALSA ML
>>
>> I'm not familiar with soc-compress.c, but I think dpcm_capture case don't need "else" ?
>> I think it is same as soc-pcm.c :: soc_new_pcm().
>> I'm happy to post patch if someone acknowledge this question.
>>
>> --- soc-compress.c --------
>> int snd_soc_new_compress(...)
>> {
>> ...
>> if (rtd->dai_link->dynamic) {
>> ...
>> if (rtd->dai_link->dpcm_playback)
>> be_pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream->private_data = rtd;
>> - else if (rtd->dai_link->dpcm_capture)
>> + if (rtd->dai_link->dpcm_capture)
>> be_pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream->private_data = rtd;
>> ...
>> }
>> ...
>> }
>>
>> --- soc-pcm.c ---------------
>> int soc_new_pcm(...)
>> {
>> ...
>> if (rtd->dai_link->no_pcm || rtd->dai_link->params) {
>> if (playback)
>> pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream->private_data = rtd;
>> => if (capture)
>> pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream->private_data = rtd;
>> goto out;
>> }
>> ...
>> }
>>
>
> I would certainly agree, pretty sure the else is redundant.
Agree as well on paper, but the two cases are different.
For the soc-pcm.c case, the configuration is for a Back-End (BE). We do
have usages where both .dpcm_playback and .dpcm_capture are set.
In the soc-compress.c case, the configuration is for a Front-End (FE). I
am not sure if there are any FE dailinks with both capture and playback
support or if there are any hidden restrictions?
We'd need to check that this has no impact on platforms with compressed
support - Srinivas and Vinod can you chime in?
At any rate this change would need a Fixes tag
Fixes: d3268a40d4b1 ("ASoC: soc-compress.c: fix NULL dereference")
prev parent reply other threads:[~2023-01-23 17:15 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-23 3:08 Question about DPCM private_data of soc-compress.c Kuninori Morimoto
2023-01-23 12:01 ` Charles Keepax
2023-01-23 15:44 ` Pierre-Louis Bossart [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6a5131ee-bce2-968a-8371-10ce7772f449@linux.intel.com \
--to=pierre-louis.bossart@linux.intel.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=ckeepax@opensource.cirrus.com \
--cc=kuninori.morimoto.gx@renesas.com \
--cc=srinivas.kandagatla@linaro.org \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.