Re: [PATCH 06/15] ASoC: Intel: replace dpcm_playback/capture to playback/capture_only

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: "Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>,
	"Alper Nebi Yasak" <alpernebiyasak@gmail.com>,
	"AngeloGioacchino Del Regno"
	<angelogioacchino.delregno@collabora.com>,
	"Banajit Goswami" <bgoswami@quicinc.com>,
	"Bard Liao" <yung-chuan.liao@linux.intel.com>,
	"Brent Lu" <brent.lu@intel.com>,
	"Cezary Rojewski" <cezary.rojewski@intel.com>,
	"Cristian Ciocaltea" <cristian.ciocaltea@collabora.com>,
	"Daniel Baluta" <daniel.baluta@nxp.com>,
	"Hans de Goede" <hdegoede@redhat.com>,
	"Jaroslav Kysela" <perex@perex.cz>,
	"Jerome Brunet" <jbrunet@baylibre.com>,
	"Kai Vehmanen" <kai.vehmanen@linux.intel.com>,
	"Kevin Hilman" <khilman@baylibre.com>,
	"Liam Girdwood" <lgirdwood@gmail.com>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Mark Brown" <broonie@kernel.org>,
	"Maso Huang" <maso.huang@mediatek.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Peter Ujfalusi" <peter.ujfalusi@linux.intel.com>,
	"Ranjani Sridharan" <ranjani.sridharan@linux.intel.com>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Shawn Guo" <shawnguo@kernel.org>,
	"Shengjiu Wang" <shengjiu.wang@gmail.com>,
	"Srinivas Kandagatla" <srinivas.kandagatla@linaro.org>,
	"Sylwester Nawrocki" <s.nawrocki@samsung.com>,
	"Takashi Iwai" <tiwai@suse.com>,
	"Trevor Wu" <trevor.wu@mediatek.com>,
	"Vinod Koul" <vkoul@kernel.org>, "Xiubo Li" <Xiubo.Lee@gmail.com>,
	alsa-devel@alsa-project.org, imx@lists.linux.dev,
	linux-sound@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com
Subject: Re: [PATCH 06/15] ASoC: Intel: replace dpcm_playback/capture to playback/capture_only
Date: Wed, 27 Mar 2024 01:18:49 +0000	[thread overview]
Message-ID: <87ttkscw07.wl-kuninori.morimoto.gx@renesas.com> (raw)
In-Reply-To: <1c437229-cbc7-4e0c-987c-9ca31800a572@linux.intel.com>

Hi Pierre-Louis

Thank you for your feedback

> In addition, I have a bit of heartburn with the concept of flipping the
> logic from a clear description of playback or capture supported, to an
> implicit 'both supported unless stated otherwise'. We are going to miss
> some cases and have regressions, it's pretty much a given.

Amadeusz indicated similar things, but it is not so strange for me.
Because it has driver to use it, default "supported" is very natural.
And, normal connection case is using this style (= default supported"),
only DPCM needs special flag.

But I have no special objection about flag direction, I just followed
existing style. If you and/or Amadeusz want different style (and there was
no objection from other reviews/maintainor), I can follow to it.

> > -	.dpcm_capture = 1, /* Capture stream provides Feedback */
(snip)
> > -	link->dpcm_capture = 1; /* feedback stream or firmware-generated echo reference */
(snip)
> > -	.dpcm_capture = 1, /* IV feedback */
(snip)
> where we lose comments on what the capture stream is. That's not so
> good, we added those comments on purpose to explain that the capture
> stream isn't a regular PCM format, I don't see a good reason to see them go?

Ah, thank you for poining these.
Indeed the comments should not be removed.
I will fix it on v2

Thank you for your help !!

Best regards
---
Renesas Electronics
Ph.D. Kuninori Morimoto