What about playback configurations?
Title for the overall series fits better than the one chosen for actual patches. "channel constraint support" is misleading. Constraints are added or removed but certainly not supported.
The FE DAI supports stereo playback only so I don't install the constraint for playback stream.
static struct snd_soc_dai_driver hsw_dais[] = { { .name = "System Pin", .id = HSW_PCM_DAI_ID_SYSTEM, .playback = { .stream_name = "System Playback", .channels_min = 2, .channels_max = 2, .rates = SNDRV_PCM_RATE_48000, .formats = SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE, },
Signed-off-by: Brent Lu brent.lu@intel.com
sound/soc/intel/boards/bdw-rt5650.c | 34
++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/sound/soc/intel/boards/bdw-rt5650.c b/sound/soc/intel/boards/bdw-rt5650.c index af2f502..dd4f219 100644 --- a/sound/soc/intel/boards/bdw-rt5650.c +++ b/sound/soc/intel/boards/bdw-rt5650.c @@ -21,6 +21,9 @@
#include "../../codecs/rt5645.h"
+#define DUAL_CHANNEL 2 +#define QUAD_CHANNEL 4
Remove, we need not additional too-obvious macro. One could argue 'STEREO' is a better choice for '2' channel configuration too.
struct bdw_rt5650_priv { struct gpio_desc *gpio_hp_en; struct snd_soc_component *component; @@ -162,6 +165,36 @@
static
int bdw_rt5650_rtd_init(struct snd_soc_pcm_runtime *rtd) } #endif
+static const unsigned int channels[] = {
- DUAL_CHANNEL, QUAD_CHANNEL,
Inline as stated above.
+};
+static const struct snd_pcm_hw_constraint_list constraints_channels = {
- .count = ARRAY_SIZE(channels),
- .list = channels,
- .mask = 0,
+};
+static int bdw_fe_startup(struct snd_pcm_substream *substream)
Entire file uses 'bdw_rt5650_' naming convention. Let's not stray away from that path now.
+{
- struct snd_pcm_runtime *runtime = substream->runtime;
Missing hw.channels_max assignment from rt5677 - inconsistency/ copy error?
I just copy the assignment from other machine driver but I think the assignment is redundant. The runtime->hw will be initialized in the dpcm_init_runtime_hw() function and the channel_max will be overwritten.
dpcm_fe_dai_startup -> dpcm_be_dai_startup -> soc_pcm_open => here our bdw_fe_startup () is called -> dpcm_set_fe_runtime ---> dpcm_init_runtime_hw => here runtime->hw is initialized
- /*
* On this platform for PCM device we support,
* 2 or 4 channel capture
*/
Sometimes you add a newline add and before, while other times just one, before the comment. Please streamline the format across all patches in the series. Comment can be more strict too /* Board supports stereo and quad configurations */
- if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
snd_pcm_hw_constraint_list(runtime, 0,
SNDRV_PCM_HW_PARAM_CHANNELS,
&constraints_channels);
Redesign to reduce indentation and improve readability - if (stream != capture) return 0;
return snd_pcm_hw_contraint_list(...);
- return 0;
+}
+static const struct snd_soc_ops bdw_rt5650_fe_ops = {
- .startup = bdw_fe_startup,
+};
- static int bdw_rt5650_init(struct snd_soc_pcm_runtime *rtd) { struct bdw_rt5650_priv *bdw_rt5650 = @@ -234,6 +267,7 @@ static
struct snd_soc_dai_link bdw_rt5650_dais[] = { .name = "System PCM", .stream_name = "System Playback", .dynamic = 1,
#if !IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL) .init = bdw_rt5650_rtd_init, #endif.ops = &bdw_rt5650_fe_ops,