[PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Mark Brown]

This series provides a bunch of quick updates which should make the
coverage from pcm-test a bit more useful, it adds some support for
skipping tests when the hardware/driver is unable to support the
requested configuration and then expands the set of cases we cover to
include more sample rates and channel counts.  This should exercise
switching between 8kHz and 44.1kHz based rates and ensure that clocking
doesn't get confused by non-stereo channel counts, both of which are I
expect common real world errors, at least for embedded cards.

Mark Brown (6):
  kselftest/alsa: Refactor pcm-test to list the tests to run in a struct
  kselftest/alsa: Report failures to set the requested sample rate as
    skips
  kselftest/alsa: Report failures to set the requested channels as skips
  kselftest/alsa: Don't any configuration in the sample config
  kselftest/alsa: Provide more meaningful names for tests
  kselftest/alsa: Add more coverage of sample rates and channel counts

 .../alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf  | 35 ++++----
 tools/testing/selftests/alsa/pcm-test.c       | 88 +++++++++++++------
 2 files changed, 81 insertions(+), 42 deletions(-)


base-commit: 1d8025ec722d5e011f9299c46274eb21fb54a428
-- 
2.30.2

[PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Mark Brown]

This series provides a bunch of quick updates which should make the
coverage from pcm-test a bit more useful, it adds some support for
skipping tests when the hardware/driver is unable to support the
requested configuration and then expands the set of cases we cover to
include more sample rates and channel counts.  This should exercise
switching between 8kHz and 44.1kHz based rates and ensure that clocking
doesn't get confused by non-stereo channel counts, both of which are I
expect common real world errors, at least for embedded cards.

Mark Brown (6):
  kselftest/alsa: Refactor pcm-test to list the tests to run in a struct
  kselftest/alsa: Report failures to set the requested sample rate as
    skips
  kselftest/alsa: Report failures to set the requested channels as skips
  kselftest/alsa: Don't any configuration in the sample config
  kselftest/alsa: Provide more meaningful names for tests
  kselftest/alsa: Add more coverage of sample rates and channel counts

 .../alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf  | 35 ++++----
 tools/testing/selftests/alsa/pcm-test.c       | 88 +++++++++++++------
 2 files changed, 81 insertions(+), 42 deletions(-)


base-commit: 1d8025ec722d5e011f9299c46274eb21fb54a428
-- 
2.30.2

[PATCH v1 1/6] kselftest/alsa: Refactor pcm-test to list the tests to run in a struct

[Mark Brown]

In order to help make the list of tests a bit easier to maintain refactor
things so we pass the tests around as a struct with the parameters in,
enabling us to add new tests by adding to a table with comments saying
what each of the number are. We could also use named initializers if we get
more parameters.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/alsa/pcm-test.c | 53 +++++++++++++++----------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/alsa/pcm-test.c b/tools/testing/selftests/alsa/pcm-test.c
index 2814d8f74f82..b8923e494e50 100644
--- a/tools/testing/selftests/alsa/pcm-test.c
+++ b/tools/testing/selftests/alsa/pcm-test.c
@@ -57,6 +57,15 @@ struct pcm_data *pcm_list = NULL;
 int num_missing = 0;
 struct pcm_data *pcm_missing = NULL;
 
+struct time_test_def {
+	const char *cfg_prefix;
+	const char *format;
+	long rate;
+	long channels;
+	long period_size;
+	long buffer_size;
+};
+
 void timestamp_now(timestamp_t *tstamp)
 {
 	if (clock_gettime(CLOCK_MONOTONIC_RAW, tstamp))
@@ -290,9 +299,7 @@ static void find_pcms(void)
 }
 
 static void test_pcm_time1(struct pcm_data *data,
-			   const char *cfg_prefix, const char *sformat,
-			   long srate, long schannels,
-			   long speriod_size, long sbuffer_size)
+			   const struct time_test_def *test)
 {
 	char name[64], key[128], msg[256];
 	const char *cs;
@@ -314,20 +321,20 @@ static void test_pcm_time1(struct pcm_data *data,
 	snd_pcm_hw_params_alloca(&hw_params);
 	snd_pcm_sw_params_alloca(&sw_params);
 
-	cs = conf_get_string(data->pcm_config, cfg_prefix, "format", sformat);
+	cs = conf_get_string(data->pcm_config, test->cfg_prefix, "format", test->format);
 	format = snd_pcm_format_value(cs);
 	if (format == SND_PCM_FORMAT_UNKNOWN)
 		ksft_exit_fail_msg("Wrong format '%s'\n", cs);
-	rate = conf_get_long(data->pcm_config, cfg_prefix, "rate", srate);
-	channels = conf_get_long(data->pcm_config, cfg_prefix, "channels", schannels);
-	period_size = conf_get_long(data->pcm_config, cfg_prefix, "period_size", speriod_size);
-	buffer_size = conf_get_long(data->pcm_config, cfg_prefix, "buffer_size", sbuffer_size);
+	rate = conf_get_long(data->pcm_config, test->cfg_prefix, "rate", test->rate);
+	channels = conf_get_long(data->pcm_config, test->cfg_prefix, "channels", test->channels);
+	period_size = conf_get_long(data->pcm_config, test->cfg_prefix, "period_size", test->period_size);
+	buffer_size = conf_get_long(data->pcm_config, test->cfg_prefix, "buffer_size", test->buffer_size);
 
-	automatic = strcmp(sformat, snd_pcm_format_name(format)) == 0 &&
-			srate == rate &&
-			schannels == channels &&
-			speriod_size == period_size &&
-			sbuffer_size == buffer_size;
+	automatic = strcmp(test->format, snd_pcm_format_name(format)) == 0 &&
+			test->rate == rate &&
+			test->channels == channels &&
+			test->period_size == period_size &&
+			test->buffer_size == buffer_size;
 
 	samples = malloc((rate * channels * snd_pcm_format_physical_width(format)) / 8);
 	if (!samples)
@@ -363,7 +370,7 @@ static void test_pcm_time1(struct pcm_data *data,
 		if (automatic && format == SND_PCM_FORMAT_S16_LE) {
 			format = SND_PCM_FORMAT_S32_LE;
 			ksft_print_msg("%s.%d.%d.%d.%s.%s format S16_LE -> S32_LE\n",
-					 cfg_prefix,
+					 test->cfg_prefix,
 					 data->card, data->device, data->subdevice,
 					 snd_pcm_stream_name(data->stream),
 					 snd_pcm_access_name(access));
@@ -432,7 +439,7 @@ static void test_pcm_time1(struct pcm_data *data,
 	}
 
 	ksft_print_msg("%s.%d.%d.%d.%s hw_params.%s.%s.%ld.%ld.%ld.%ld sw_params.%ld\n",
-			 cfg_prefix,
+			 test->cfg_prefix,
 			 data->card, data->device, data->subdevice,
 			 snd_pcm_stream_name(data->stream),
 			 snd_pcm_access_name(access),
@@ -481,7 +488,7 @@ static void test_pcm_time1(struct pcm_data *data,
 	pass = true;
 __close:
 	ksft_test_result(pass, "%s.%d.%d.%d.%s%s%s\n",
-			 cfg_prefix,
+			 test->cfg_prefix,
 			 data->card, data->device, data->subdevice,
 			 snd_pcm_stream_name(data->stream),
 			 msg[0] ? " " : "", msg);
@@ -490,11 +497,16 @@ static void test_pcm_time1(struct pcm_data *data,
 		snd_pcm_close(handle);
 }
 
-#define TESTS_PER_PCM 2
+static const struct time_test_def time_tests[] = {
+	/* name          format     rate   chan  period  buffer */
+	{ "test.time1",  "S16_LE",  48000, 2,      512,    4096 },
+	{ "test.time2",  "S16_LE",  48000, 2,    24000,  192000 },
+};
 
 int main(void)
 {
 	struct pcm_data *pcm;
+	int i;
 
 	ksft_print_header();
 
@@ -502,7 +514,7 @@ int main(void)
 
 	find_pcms();
 
-	ksft_set_plan(num_missing + num_pcms * TESTS_PER_PCM);
+	ksft_set_plan(num_missing + num_pcms * ARRAY_SIZE(time_tests));
 
 	for (pcm = pcm_missing; pcm != NULL; pcm = pcm->next) {
 		ksft_test_result(false, "test.missing.%d.%d.%d.%s\n",
@@ -511,8 +523,9 @@ int main(void)
 	}
 
 	for (pcm = pcm_list; pcm != NULL; pcm = pcm->next) {
-		test_pcm_time1(pcm, "test.time1", "S16_LE", 48000, 2, 512, 4096);
-		test_pcm_time1(pcm, "test.time2", "S16_LE", 48000, 2, 24000, 192000);
+		for (i = 0; i < ARRAY_SIZE(time_tests); i++) {
+			test_pcm_time1(pcm, &time_tests[i]);
+		}
 	}
 
 	conf_free();
-- 
2.30.2

[PATCH v1 1/6] kselftest/alsa: Refactor pcm-test to list the tests to run in a struct

[Mark Brown]

In order to help make the list of tests a bit easier to maintain refactor
things so we pass the tests around as a struct with the parameters in,
enabling us to add new tests by adding to a table with comments saying
what each of the number are. We could also use named initializers if we get
more parameters.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/alsa/pcm-test.c | 53 +++++++++++++++----------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/alsa/pcm-test.c b/tools/testing/selftests/alsa/pcm-test.c
index 2814d8f74f82..b8923e494e50 100644
--- a/tools/testing/selftests/alsa/pcm-test.c
+++ b/tools/testing/selftests/alsa/pcm-test.c
@@ -57,6 +57,15 @@ struct pcm_data *pcm_list = NULL;
 int num_missing = 0;
 struct pcm_data *pcm_missing = NULL;
 
+struct time_test_def {
+	const char *cfg_prefix;
+	const char *format;
+	long rate;
+	long channels;
+	long period_size;
+	long buffer_size;
+};
+
 void timestamp_now(timestamp_t *tstamp)
 {
 	if (clock_gettime(CLOCK_MONOTONIC_RAW, tstamp))
@@ -290,9 +299,7 @@ static void find_pcms(void)
 }
 
 static void test_pcm_time1(struct pcm_data *data,
-			   const char *cfg_prefix, const char *sformat,
-			   long srate, long schannels,
-			   long speriod_size, long sbuffer_size)
+			   const struct time_test_def *test)
 {
 	char name[64], key[128], msg[256];
 	const char *cs;
@@ -314,20 +321,20 @@ static void test_pcm_time1(struct pcm_data *data,
 	snd_pcm_hw_params_alloca(&hw_params);
 	snd_pcm_sw_params_alloca(&sw_params);
 
-	cs = conf_get_string(data->pcm_config, cfg_prefix, "format", sformat);
+	cs = conf_get_string(data->pcm_config, test->cfg_prefix, "format", test->format);
 	format = snd_pcm_format_value(cs);
 	if (format == SND_PCM_FORMAT_UNKNOWN)
 		ksft_exit_fail_msg("Wrong format '%s'\n", cs);
-	rate = conf_get_long(data->pcm_config, cfg_prefix, "rate", srate);
-	channels = conf_get_long(data->pcm_config, cfg_prefix, "channels", schannels);
-	period_size = conf_get_long(data->pcm_config, cfg_prefix, "period_size", speriod_size);
-	buffer_size = conf_get_long(data->pcm_config, cfg_prefix, "buffer_size", sbuffer_size);
+	rate = conf_get_long(data->pcm_config, test->cfg_prefix, "rate", test->rate);
+	channels = conf_get_long(data->pcm_config, test->cfg_prefix, "channels", test->channels);
+	period_size = conf_get_long(data->pcm_config, test->cfg_prefix, "period_size", test->period_size);
+	buffer_size = conf_get_long(data->pcm_config, test->cfg_prefix, "buffer_size", test->buffer_size);
 
-	automatic = strcmp(sformat, snd_pcm_format_name(format)) == 0 &&
-			srate == rate &&
-			schannels == channels &&
-			speriod_size == period_size &&
-			sbuffer_size == buffer_size;
+	automatic = strcmp(test->format, snd_pcm_format_name(format)) == 0 &&
+			test->rate == rate &&
+			test->channels == channels &&
+			test->period_size == period_size &&
+			test->buffer_size == buffer_size;
 
 	samples = malloc((rate * channels * snd_pcm_format_physical_width(format)) / 8);
 	if (!samples)
@@ -363,7 +370,7 @@ static void test_pcm_time1(struct pcm_data *data,
 		if (automatic && format == SND_PCM_FORMAT_S16_LE) {
 			format = SND_PCM_FORMAT_S32_LE;
 			ksft_print_msg("%s.%d.%d.%d.%s.%s format S16_LE -> S32_LE\n",
-					 cfg_prefix,
+					 test->cfg_prefix,
 					 data->card, data->device, data->subdevice,
 					 snd_pcm_stream_name(data->stream),
 					 snd_pcm_access_name(access));
@@ -432,7 +439,7 @@ static void test_pcm_time1(struct pcm_data *data,
 	}
 
 	ksft_print_msg("%s.%d.%d.%d.%s hw_params.%s.%s.%ld.%ld.%ld.%ld sw_params.%ld\n",
-			 cfg_prefix,
+			 test->cfg_prefix,
 			 data->card, data->device, data->subdevice,
 			 snd_pcm_stream_name(data->stream),
 			 snd_pcm_access_name(access),
@@ -481,7 +488,7 @@ static void test_pcm_time1(struct pcm_data *data,
 	pass = true;
 __close:
 	ksft_test_result(pass, "%s.%d.%d.%d.%s%s%s\n",
-			 cfg_prefix,
+			 test->cfg_prefix,
 			 data->card, data->device, data->subdevice,
 			 snd_pcm_stream_name(data->stream),
 			 msg[0] ? " " : "", msg);
@@ -490,11 +497,16 @@ static void test_pcm_time1(struct pcm_data *data,
 		snd_pcm_close(handle);
 }
 
-#define TESTS_PER_PCM 2
+static const struct time_test_def time_tests[] = {
+	/* name          format     rate   chan  period  buffer */
+	{ "test.time1",  "S16_LE",  48000, 2,      512,    4096 },
+	{ "test.time2",  "S16_LE",  48000, 2,    24000,  192000 },
+};
 
 int main(void)
 {
 	struct pcm_data *pcm;
+	int i;
 
 	ksft_print_header();
 
@@ -502,7 +514,7 @@ int main(void)
 
 	find_pcms();
 
-	ksft_set_plan(num_missing + num_pcms * TESTS_PER_PCM);
+	ksft_set_plan(num_missing + num_pcms * ARRAY_SIZE(time_tests));
 
 	for (pcm = pcm_missing; pcm != NULL; pcm = pcm->next) {
 		ksft_test_result(false, "test.missing.%d.%d.%d.%s\n",
@@ -511,8 +523,9 @@ int main(void)
 	}
 
 	for (pcm = pcm_list; pcm != NULL; pcm = pcm->next) {
-		test_pcm_time1(pcm, "test.time1", "S16_LE", 48000, 2, 512, 4096);
-		test_pcm_time1(pcm, "test.time2", "S16_LE", 48000, 2, 24000, 192000);
+		for (i = 0; i < ARRAY_SIZE(time_tests); i++) {
+			test_pcm_time1(pcm, &time_tests[i]);
+		}
 	}
 
 	conf_free();
-- 
2.30.2

[PATCH v1 2/6] kselftest/alsa: Report failures to set the requested sample rate as skips

[Mark Brown]

If constraint selection gives us a sample rate other than the one that we
asked for that isn't a failure, that is the device implementing sample
rate constraints and advertising that it can't support whatever we asked
for. Report such cases as a test skip rather than failure so we don't have
false positives.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/alsa/pcm-test.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/alsa/pcm-test.c b/tools/testing/selftests/alsa/pcm-test.c
index b8923e494e50..6a9e4af828ee 100644
--- a/tools/testing/selftests/alsa/pcm-test.c
+++ b/tools/testing/selftests/alsa/pcm-test.c
@@ -317,6 +317,7 @@ static void test_pcm_time1(struct pcm_data *data,
 	bool pass = false, automatic = true;
 	snd_pcm_hw_params_t *hw_params;
 	snd_pcm_sw_params_t *sw_params;
+	bool skip = false;
 
 	snd_pcm_hw_params_alloca(&hw_params);
 	snd_pcm_sw_params_alloca(&sw_params);
@@ -391,7 +392,8 @@ static void test_pcm_time1(struct pcm_data *data,
 		goto __close;
 	}
 	if (rrate != rate) {
-		snprintf(msg, sizeof(msg), "rate mismatch %ld != %ld", rate, rrate);
+		snprintf(msg, sizeof(msg), "rate unsupported %ld != %ld", rate, rrate);
+		skip = true;
 		goto __close;
 	}
 	rperiod_size = period_size;
@@ -487,11 +489,20 @@ static void test_pcm_time1(struct pcm_data *data,
 	msg[0] = '\0';
 	pass = true;
 __close:
-	ksft_test_result(pass, "%s.%d.%d.%d.%s%s%s\n",
-			 test->cfg_prefix,
-			 data->card, data->device, data->subdevice,
-			 snd_pcm_stream_name(data->stream),
-			 msg[0] ? " " : "", msg);
+	if (!skip) {
+		ksft_test_result(pass, "%s.%d.%d.%d.%s%s%s\n",
+				 test->cfg_prefix,
+				 data->card, data->device, data->subdevice,
+				 snd_pcm_stream_name(data->stream),
+				 msg[0] ? " " : "", msg);
+	} else {
+		ksft_test_result_skip("%s.%d.%d.%d.%s%s%s\n",
+				      test->cfg_prefix,
+				      data->card, data->device,
+				      data->subdevice,
+				      snd_pcm_stream_name(data->stream),
+				      msg[0] ? " " : "", msg);
+	}
 	free(samples);
 	if (handle)
 		snd_pcm_close(handle);
-- 
2.30.2

[PATCH v1 2/6] kselftest/alsa: Report failures to set the requested sample rate as skips

[Mark Brown]

If constraint selection gives us a sample rate other than the one that we
asked for that isn't a failure, that is the device implementing sample
rate constraints and advertising that it can't support whatever we asked
for. Report such cases as a test skip rather than failure so we don't have
false positives.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/alsa/pcm-test.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/alsa/pcm-test.c b/tools/testing/selftests/alsa/pcm-test.c
index b8923e494e50..6a9e4af828ee 100644
--- a/tools/testing/selftests/alsa/pcm-test.c
+++ b/tools/testing/selftests/alsa/pcm-test.c
@@ -317,6 +317,7 @@ static void test_pcm_time1(struct pcm_data *data,
 	bool pass = false, automatic = true;
 	snd_pcm_hw_params_t *hw_params;
 	snd_pcm_sw_params_t *sw_params;
+	bool skip = false;
 
 	snd_pcm_hw_params_alloca(&hw_params);
 	snd_pcm_sw_params_alloca(&sw_params);
@@ -391,7 +392,8 @@ static void test_pcm_time1(struct pcm_data *data,
 		goto __close;
 	}
 	if (rrate != rate) {
-		snprintf(msg, sizeof(msg), "rate mismatch %ld != %ld", rate, rrate);
+		snprintf(msg, sizeof(msg), "rate unsupported %ld != %ld", rate, rrate);
+		skip = true;
 		goto __close;
 	}
 	rperiod_size = period_size;
@@ -487,11 +489,20 @@ static void test_pcm_time1(struct pcm_data *data,
 	msg[0] = '\0';
 	pass = true;
 __close:
-	ksft_test_result(pass, "%s.%d.%d.%d.%s%s%s\n",
-			 test->cfg_prefix,
-			 data->card, data->device, data->subdevice,
-			 snd_pcm_stream_name(data->stream),
-			 msg[0] ? " " : "", msg);
+	if (!skip) {
+		ksft_test_result(pass, "%s.%d.%d.%d.%s%s%s\n",
+				 test->cfg_prefix,
+				 data->card, data->device, data->subdevice,
+				 snd_pcm_stream_name(data->stream),
+				 msg[0] ? " " : "", msg);
+	} else {
+		ksft_test_result_skip("%s.%d.%d.%d.%s%s%s\n",
+				      test->cfg_prefix,
+				      data->card, data->device,
+				      data->subdevice,
+				      snd_pcm_stream_name(data->stream),
+				      msg[0] ? " " : "", msg);
+	}
 	free(samples);
 	if (handle)
 		snd_pcm_close(handle);
-- 
2.30.2

[PATCH v1 3/6] kselftest/alsa: Report failures to set the requested channels as skips

[Mark Brown]

If constraint selection gives us a number of channels other than the one
that we asked for that isn't a failure, that is the device implementing
constraints and advertising that it can't support whatever we asked
for. Report such cases as a test skip rather than failure so we don't have
false positives.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/alsa/pcm-test.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/alsa/pcm-test.c b/tools/testing/selftests/alsa/pcm-test.c
index 6a9e4af828ee..bc0cb3c481f2 100644
--- a/tools/testing/selftests/alsa/pcm-test.c
+++ b/tools/testing/selftests/alsa/pcm-test.c
@@ -311,6 +311,7 @@ static void test_pcm_time1(struct pcm_data *data,
 	snd_pcm_sframes_t frames;
 	long long ms;
 	long rate, channels, period_size, buffer_size;
+	unsigned int rchannels;
 	unsigned int rrate;
 	snd_pcm_uframes_t rperiod_size, rbuffer_size, start_threshold;
 	timestamp_t tstamp;
@@ -380,11 +381,17 @@ static void test_pcm_time1(struct pcm_data *data,
 					   snd_pcm_format_name(format), snd_strerror(err));
 		goto __close;
 	}
-	err = snd_pcm_hw_params_set_channels(handle, hw_params, channels);
+	rchannels = channels;
+	err = snd_pcm_hw_params_set_channels_near(handle, hw_params, &rchannels);
 	if (err < 0) {
 		snprintf(msg, sizeof(msg), "snd_pcm_hw_params_set_channels %ld: %s", channels, snd_strerror(err));
 		goto __close;
 	}
+	if (rchannels != channels) {
+		snprintf(msg, sizeof(msg), "channels unsupported %ld != %ld", channels, rchannels);
+		skip = true;
+		goto __close;
+	}
 	rrate = rate;
 	err = snd_pcm_hw_params_set_rate_near(handle, hw_params, &rrate, 0);
 	if (err < 0) {
-- 
2.30.2

[PATCH v1 3/6] kselftest/alsa: Report failures to set the requested channels as skips

[Mark Brown]

If constraint selection gives us a number of channels other than the one
that we asked for that isn't a failure, that is the device implementing
constraints and advertising that it can't support whatever we asked
for. Report such cases as a test skip rather than failure so we don't have
false positives.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/alsa/pcm-test.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/alsa/pcm-test.c b/tools/testing/selftests/alsa/pcm-test.c
index 6a9e4af828ee..bc0cb3c481f2 100644
--- a/tools/testing/selftests/alsa/pcm-test.c
+++ b/tools/testing/selftests/alsa/pcm-test.c
@@ -311,6 +311,7 @@ static void test_pcm_time1(struct pcm_data *data,
 	snd_pcm_sframes_t frames;
 	long long ms;
 	long rate, channels, period_size, buffer_size;
+	unsigned int rchannels;
 	unsigned int rrate;
 	snd_pcm_uframes_t rperiod_size, rbuffer_size, start_threshold;
 	timestamp_t tstamp;
@@ -380,11 +381,17 @@ static void test_pcm_time1(struct pcm_data *data,
 					   snd_pcm_format_name(format), snd_strerror(err));
 		goto __close;
 	}
-	err = snd_pcm_hw_params_set_channels(handle, hw_params, channels);
+	rchannels = channels;
+	err = snd_pcm_hw_params_set_channels_near(handle, hw_params, &rchannels);
 	if (err < 0) {
 		snprintf(msg, sizeof(msg), "snd_pcm_hw_params_set_channels %ld: %s", channels, snd_strerror(err));
 		goto __close;
 	}
+	if (rchannels != channels) {
+		snprintf(msg, sizeof(msg), "channels unsupported %ld != %ld", channels, rchannels);
+		skip = true;
+		goto __close;
+	}
 	rrate = rate;
 	err = snd_pcm_hw_params_set_rate_near(handle, hw_params, &rrate, 0);
 	if (err < 0) {
-- 
2.30.2

[PATCH v1 4/6] kselftest/alsa: Don't any configuration in the sample config

[Mark Brown]

The values in the one example configuration file we currently have are the
default values for the two tests we have so there's no need to actually set
them. Comment them out as examples, with a rename for the tests so that we
can update the tests in the code more easily.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf  | 35 ++++++++++---------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf b/tools/testing/selftests/alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf
index 0a83f35d43eb..9eca985e0c08 100644
--- a/tools/testing/selftests/alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf
+++ b/tools/testing/selftests/alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf
@@ -39,22 +39,25 @@ card.hda {
 	#
 	pcm.0.0 {
 		PLAYBACK {
-			test.time1 {
-				access RW_INTERLEAVED	# can be omitted - default
-				format S16_LE		# can be omitted - default
-				rate 48000		# can be omitted - default
-				channels 2		# can be omitted - default
-				period_size 512
-				buffer_size 4096
-			}
-			test.time2 {
-				access RW_INTERLEAVED
-				format S16_LE
-				rate 48000
-				channels 2
-				period_size 24000
-				buffer_size 192000
-			}
+			#
+			# Uncomment to override values for specific tests
+			#
+			#test_name1 {
+			#	access RW_INTERLEAVED
+			#	format S16_LE
+			#	rate 48000
+			#	channels 2
+			#	period_size 512
+			#	buffer_size 4096
+			#}
+			#test_name2 {
+			#	access RW_INTERLEAVED
+			#	format S16_LE
+			#	rate 48000
+			#	channels 2
+			#	period_size 24000
+			#	buffer_size 192000
+			#}
 		}
 		CAPTURE {
 			# use default tests, check for the presence
-- 
2.30.2

[PATCH v1 4/6] kselftest/alsa: Don't any configuration in the sample config

[Mark Brown]

The values in the one example configuration file we currently have are the
default values for the two tests we have so there's no need to actually set
them. Comment them out as examples, with a rename for the tests so that we
can update the tests in the code more easily.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf  | 35 ++++++++++---------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf b/tools/testing/selftests/alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf
index 0a83f35d43eb..9eca985e0c08 100644
--- a/tools/testing/selftests/alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf
+++ b/tools/testing/selftests/alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf
@@ -39,22 +39,25 @@ card.hda {
 	#
 	pcm.0.0 {
 		PLAYBACK {
-			test.time1 {
-				access RW_INTERLEAVED	# can be omitted - default
-				format S16_LE		# can be omitted - default
-				rate 48000		# can be omitted - default
-				channels 2		# can be omitted - default
-				period_size 512
-				buffer_size 4096
-			}
-			test.time2 {
-				access RW_INTERLEAVED
-				format S16_LE
-				rate 48000
-				channels 2
-				period_size 24000
-				buffer_size 192000
-			}
+			#
+			# Uncomment to override values for specific tests
+			#
+			#test_name1 {
+			#	access RW_INTERLEAVED
+			#	format S16_LE
+			#	rate 48000
+			#	channels 2
+			#	period_size 512
+			#	buffer_size 4096
+			#}
+			#test_name2 {
+			#	access RW_INTERLEAVED
+			#	format S16_LE
+			#	rate 48000
+			#	channels 2
+			#	period_size 24000
+			#	buffer_size 192000
+			#}
 		}
 		CAPTURE {
 			# use default tests, check for the presence
-- 
2.30.2

[PATCH v1 5/6] kselftest/alsa: Provide more meaningful names for tests

[Mark Brown]

Rather than just numbering the tests try to provide semi descriptive names
for what the tests are trying to cover. This also has the advantage of
meaning we can add more tests without having to keep the list of tests
ordered by existing number which should make it easier to understand what
we're testing and why.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/alsa/pcm-test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/alsa/pcm-test.c b/tools/testing/selftests/alsa/pcm-test.c
index bc0cb3c481f2..614ccc52cece 100644
--- a/tools/testing/selftests/alsa/pcm-test.c
+++ b/tools/testing/selftests/alsa/pcm-test.c
@@ -516,9 +516,9 @@ static void test_pcm_time1(struct pcm_data *data,
 }
 
 static const struct time_test_def time_tests[] = {
-	/* name          format     rate   chan  period  buffer */
-	{ "test.time1",  "S16_LE",  48000, 2,      512,    4096 },
-	{ "test.time2",  "S16_LE",  48000, 2,    24000,  192000 },
+	/* name              format     rate   chan  period  buffer */
+	{ "S16.48k.2.small", "S16_LE",  48000, 2,      512,    4096 },
+	{ "S16.48k.2.big",   "S16_LE",  48000, 2,    24000,  192000 },
 };
 
 int main(void)
-- 
2.30.2

[PATCH v1 5/6] kselftest/alsa: Provide more meaningful names for tests

[Mark Brown]

Rather than just numbering the tests try to provide semi descriptive names
for what the tests are trying to cover. This also has the advantage of
meaning we can add more tests without having to keep the list of tests
ordered by existing number which should make it easier to understand what
we're testing and why.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/alsa/pcm-test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/alsa/pcm-test.c b/tools/testing/selftests/alsa/pcm-test.c
index bc0cb3c481f2..614ccc52cece 100644
--- a/tools/testing/selftests/alsa/pcm-test.c
+++ b/tools/testing/selftests/alsa/pcm-test.c
@@ -516,9 +516,9 @@ static void test_pcm_time1(struct pcm_data *data,
 }
 
 static const struct time_test_def time_tests[] = {
-	/* name          format     rate   chan  period  buffer */
-	{ "test.time1",  "S16_LE",  48000, 2,      512,    4096 },
-	{ "test.time2",  "S16_LE",  48000, 2,    24000,  192000 },
+	/* name              format     rate   chan  period  buffer */
+	{ "S16.48k.2.small", "S16_LE",  48000, 2,      512,    4096 },
+	{ "S16.48k.2.big",   "S16_LE",  48000, 2,    24000,  192000 },
 };
 
 int main(void)
-- 
2.30.2

[PATCH v1 6/6] kselftest/alsa: Add more coverage of sample rates and channel counts

[Mark Brown]

Now that we can skip unsupported configurations add some more test cases
using that, cover 8kHz, 44.1kHz and 96kHz plus 8kHz mono and 48kHz 6
channel.

44.1kHz is a different clock base to the existing 48kHz tests and may
therefore show problems with the clock configuration if only 8kHz based
rates are really available (or help diagnose if bad clocking is due to
only 44.1kHz based rates being supported). 8kHz mono and 48Hz 6 channel
are real world formats and should show if clocking does not account for
channel count properly.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/alsa/pcm-test.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/alsa/pcm-test.c b/tools/testing/selftests/alsa/pcm-test.c
index 614ccc52cece..d76a6f15ce25 100644
--- a/tools/testing/selftests/alsa/pcm-test.c
+++ b/tools/testing/selftests/alsa/pcm-test.c
@@ -517,8 +517,13 @@ static void test_pcm_time1(struct pcm_data *data,
 
 static const struct time_test_def time_tests[] = {
 	/* name              format     rate   chan  period  buffer */
-	{ "S16.48k.2.small", "S16_LE",  48000, 2,      512,    4096 },
-	{ "S16.48k.2.big",   "S16_LE",  48000, 2,    24000,  192000 },
+	{ "8k.1.big",    "S16_LE",   8000, 2,     2000,   16000 },
+	{ "8k.2.big",    "S16_LE",   8000, 2,     4000,   32000 },
+	{ "44k1.2.big",  "S16_LE",  44100, 2,    22050,  192000 },
+	{ "48k.2.small", "S16_LE",  48000, 2,      512,    4096 },
+	{ "48k.2.big",   "S16_LE",  48000, 2,    24000,  192000 },
+	{ "48k.6.big",   "S16_LE",  48000, 6,    48000,  576000 },
+	{ "96k.2.big",   "S16_LE",  96000, 2,    48000,  384000 },
 };
 
 int main(void)
-- 
2.30.2

[PATCH v1 6/6] kselftest/alsa: Add more coverage of sample rates and channel counts

[Mark Brown]

Now that we can skip unsupported configurations add some more test cases
using that, cover 8kHz, 44.1kHz and 96kHz plus 8kHz mono and 48kHz 6
channel.

44.1kHz is a different clock base to the existing 48kHz tests and may
therefore show problems with the clock configuration if only 8kHz based
rates are really available (or help diagnose if bad clocking is due to
only 44.1kHz based rates being supported). 8kHz mono and 48Hz 6 channel
are real world formats and should show if clocking does not account for
channel count properly.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/alsa/pcm-test.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/alsa/pcm-test.c b/tools/testing/selftests/alsa/pcm-test.c
index 614ccc52cece..d76a6f15ce25 100644
--- a/tools/testing/selftests/alsa/pcm-test.c
+++ b/tools/testing/selftests/alsa/pcm-test.c
@@ -517,8 +517,13 @@ static void test_pcm_time1(struct pcm_data *data,
 
 static const struct time_test_def time_tests[] = {
 	/* name              format     rate   chan  period  buffer */
-	{ "S16.48k.2.small", "S16_LE",  48000, 2,      512,    4096 },
-	{ "S16.48k.2.big",   "S16_LE",  48000, 2,    24000,  192000 },
+	{ "8k.1.big",    "S16_LE",   8000, 2,     2000,   16000 },
+	{ "8k.2.big",    "S16_LE",   8000, 2,     4000,   32000 },
+	{ "44k1.2.big",  "S16_LE",  44100, 2,    22050,  192000 },
+	{ "48k.2.small", "S16_LE",  48000, 2,      512,    4096 },
+	{ "48k.2.big",   "S16_LE",  48000, 2,    24000,  192000 },
+	{ "48k.6.big",   "S16_LE",  48000, 6,    48000,  576000 },
+	{ "96k.2.big",   "S16_LE",  96000, 2,    48000,  384000 },
 };
 
 int main(void)
-- 
2.30.2

Re: [PATCH v1 6/6] kselftest/alsa: Add more coverage of sample rates and channel counts

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 814 bytes --]

On Wed, Nov 30, 2022 at 12:06:08AM +0000, Mark Brown wrote:

> +	{ "8k.1.big",    "S16_LE",   8000, 2,     2000,   16000 },
> +	{ "8k.2.big",    "S16_LE",   8000, 2,     4000,   32000 },
> +	{ "44k1.2.big",  "S16_LE",  44100, 2,    22050,  192000 },
> +	{ "48k.2.small", "S16_LE",  48000, 2,      512,    4096 },
> +	{ "48k.2.big",   "S16_LE",  48000, 2,    24000,  192000 },
> +	{ "48k.6.big",   "S16_LE",  48000, 6,    48000,  576000 },
> +	{ "96k.2.big",   "S16_LE",  96000, 2,    48000,  384000 },

It looks like the period/buffer size numbers need some tuning other than
for 44.1kHz and 48kHz 6 channel, if I do any more fiddling with this
series I'll split the problematic ones out into a separate commit if I
don't get things sorted (but we could just leave things as-is and fix
incrementally too I guess).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 6/6] kselftest/alsa: Add more coverage of sample rates and channel counts

[Takashi Iwai]

On Wed, 30 Nov 2022 14:42:49 +0100,
Mark Brown wrote:
> 
> On Wed, Nov 30, 2022 at 12:06:08AM +0000, Mark Brown wrote:
> 
> > +	{ "8k.1.big",    "S16_LE",   8000, 2,     2000,   16000 },
> > +	{ "8k.2.big",    "S16_LE",   8000, 2,     4000,   32000 },
> > +	{ "44k1.2.big",  "S16_LE",  44100, 2,    22050,  192000 },
> > +	{ "48k.2.small", "S16_LE",  48000, 2,      512,    4096 },
> > +	{ "48k.2.big",   "S16_LE",  48000, 2,    24000,  192000 },
> > +	{ "48k.6.big",   "S16_LE",  48000, 6,    48000,  576000 },
> > +	{ "96k.2.big",   "S16_LE",  96000, 2,    48000,  384000 },
> 
> It looks like the period/buffer size numbers need some tuning other than
> for 44.1kHz and 48kHz 6 channel, if I do any more fiddling with this
> series I'll split the problematic ones out into a separate commit if I
> don't get things sorted (but we could just leave things as-is and fix
> incrementally too I guess).

I'm fine in either way.  I wait for Jaroslav's review for now.


thanks,

Takashi

Re: [PATCH v1 6/6] kselftest/alsa: Add more coverage of sample rates and channel counts

[Takashi Iwai]

On Wed, 30 Nov 2022 14:42:49 +0100,
Mark Brown wrote:
> 
> On Wed, Nov 30, 2022 at 12:06:08AM +0000, Mark Brown wrote:
> 
> > +	{ "8k.1.big",    "S16_LE",   8000, 2,     2000,   16000 },
> > +	{ "8k.2.big",    "S16_LE",   8000, 2,     4000,   32000 },
> > +	{ "44k1.2.big",  "S16_LE",  44100, 2,    22050,  192000 },
> > +	{ "48k.2.small", "S16_LE",  48000, 2,      512,    4096 },
> > +	{ "48k.2.big",   "S16_LE",  48000, 2,    24000,  192000 },
> > +	{ "48k.6.big",   "S16_LE",  48000, 6,    48000,  576000 },
> > +	{ "96k.2.big",   "S16_LE",  96000, 2,    48000,  384000 },
> 
> It looks like the period/buffer size numbers need some tuning other than
> for 44.1kHz and 48kHz 6 channel, if I do any more fiddling with this
> series I'll split the problematic ones out into a separate commit if I
> don't get things sorted (but we could just leave things as-is and fix
> incrementally too I guess).

I'm fine in either way.  I wait for Jaroslav's review for now.


thanks,

Takashi

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Jaroslav Kysela]

On 30. 11. 22 1:06, Mark Brown wrote:
> This series provides a bunch of quick updates which should make the
> coverage from pcm-test a bit more useful, it adds some support for
> skipping tests when the hardware/driver is unable to support the
> requested configuration and then expands the set of cases we cover to
> include more sample rates and channel counts.  This should exercise
> switching between 8kHz and 44.1kHz based rates and ensure that clocking
> doesn't get confused by non-stereo channel counts, both of which are I
> expect common real world errors, at least for embedded cards.

The current code allows to override "test.time1 {} test.time2 {}" blocks in 
the configuration files which is equivalent to "test { time1 {} time2 {} }". 
This changeset will introduce configuration lookups like 
"pcm.0.0.PLAYBACK.44k1.2.big {}" which creates another configuration 
structure. The '.' (compound level delimiter) should not be used in the test name.

My original idea for the next improvement was to parse the 
"pcm.0.0.PLAYBACK.test" compound and gather the tests for the given pcm. If 
this compound is missing, we can continue with the hard-coded defaults.

About the skips - the test should probably keep to support also the exact 
parameters. For example - if the hardware must support 6 channels, it should 
not be a skip but an error. Everything may be broken, including the PCM 
configuration refining.

I just sent the patch with my changes for comments [1]. It's just the base 
code which may be extended with your requirements. The skips may be 
implemented using configuration field like 'skip_if_rate_error yes' or so.
Let me know, if I can stack your changes on top, or perhaps, you may be 
willing to adapt them.

					Jaroslav

[1] https://lore.kernel.org/alsa-devel/20221201173333.2494019-1-perex@perex.cz/

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 3025 bytes --]

On Thu, Dec 01, 2022 at 06:42:22PM +0100, Jaroslav Kysela wrote:

> The current code allows to override "test.time1 {} test.time2 {}" blocks in
> the configuration files which is equivalent to "test { time1 {} time2 {} }".

Right, I was leaving that in place but just renaming so that the intent
of the test was clearer and expanding the standard coverage - trying to
make it clearer what the test was trying to accomplish when someone
comes along trying to do something later on.  It did however cross my
mind that we might be better off having the tests read from the config
file be in addition to the standard tests rather than overriding them,
I think that'd work out a lot clearer in the end.

> This changeset will introduce configuration lookups like
> "pcm.0.0.PLAYBACK.44k1.2.big {}" which creates another configuration
> structure. The '.' (compound level delimiter) should not be used in the test
> name.

I see, we could use another delimiter there easily enough (though if we
segregated the built in and loaded test configurations I'm not sure it'd
matter so much).

> My original idea for the next improvement was to parse the
> "pcm.0.0.PLAYBACK.test" compound and gather the tests for the given pcm. If
> this compound is missing, we can continue with the hard-coded defaults.

While it is useful to be able to specify additional tests through
configuration I don't think we should be relying on that for coverage,
we should have a more substantial baseline of tests so that systems like
KernelCI get reasonable coverage without having to get changes
individually integrated for boards (and then wait for them to filter out
into the trees being tested).  It doesn't scale out so well over the
number of systems that we might be running on, especially if we come up
with new tests and have to loop back over existing boards, and isn't
really idiomatic for kselftest.

I'm also a bit worried about the way we currently override the built in
tests, it creates additional potential for confusion when looking at
results if the test might've been turned into something different by the
configuration file.

> About the skips - the test should probably keep to support also the exact
> parameters. For example - if the hardware must support 6 channels, it should
> not be a skip but an error. Everything may be broken, including the PCM
> configuration refining.

Yes, there's a tension there between hard coded tests and the explicitly
specified per board ones.  I think the solution here is to add two tests
for things we read from the configuration file rather than just adding
by default, one verifying that we managed to configure the settings we
asked for and one for the actual test.

> I just sent the patch with my changes for comments [1]. It's just the base
> code which may be extended with your requirements. The skips may be
> implemented using configuration field like 'skip_if_rate_error yes' or so.
> Let me know, if I can stack your changes on top, or perhaps, you may be
> willing to adapt them.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 3025 bytes --]

On Thu, Dec 01, 2022 at 06:42:22PM +0100, Jaroslav Kysela wrote:

> The current code allows to override "test.time1 {} test.time2 {}" blocks in
> the configuration files which is equivalent to "test { time1 {} time2 {} }".

Right, I was leaving that in place but just renaming so that the intent
of the test was clearer and expanding the standard coverage - trying to
make it clearer what the test was trying to accomplish when someone
comes along trying to do something later on.  It did however cross my
mind that we might be better off having the tests read from the config
file be in addition to the standard tests rather than overriding them,
I think that'd work out a lot clearer in the end.

> This changeset will introduce configuration lookups like
> "pcm.0.0.PLAYBACK.44k1.2.big {}" which creates another configuration
> structure. The '.' (compound level delimiter) should not be used in the test
> name.

I see, we could use another delimiter there easily enough (though if we
segregated the built in and loaded test configurations I'm not sure it'd
matter so much).

> My original idea for the next improvement was to parse the
> "pcm.0.0.PLAYBACK.test" compound and gather the tests for the given pcm. If
> this compound is missing, we can continue with the hard-coded defaults.

While it is useful to be able to specify additional tests through
configuration I don't think we should be relying on that for coverage,
we should have a more substantial baseline of tests so that systems like
KernelCI get reasonable coverage without having to get changes
individually integrated for boards (and then wait for them to filter out
into the trees being tested).  It doesn't scale out so well over the
number of systems that we might be running on, especially if we come up
with new tests and have to loop back over existing boards, and isn't
really idiomatic for kselftest.

I'm also a bit worried about the way we currently override the built in
tests, it creates additional potential for confusion when looking at
results if the test might've been turned into something different by the
configuration file.

> About the skips - the test should probably keep to support also the exact
> parameters. For example - if the hardware must support 6 channels, it should
> not be a skip but an error. Everything may be broken, including the PCM
> configuration refining.

Yes, there's a tension there between hard coded tests and the explicitly
specified per board ones.  I think the solution here is to add two tests
for things we read from the configuration file rather than just adding
by default, one verifying that we managed to configure the settings we
asked for and one for the actual test.

> I just sent the patch with my changes for comments [1]. It's just the base
> code which may be extended with your requirements. The skips may be
> implemented using configuration field like 'skip_if_rate_error yes' or so.
> Let me know, if I can stack your changes on top, or perhaps, you may be
> willing to adapt them.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Takashi Iwai]

On Thu, 01 Dec 2022 18:42:22 +0100,
Jaroslav Kysela wrote:
> 
> Let me know, if I can stack your changes on top, or perhaps, you may
> be willing to adapt them.

As Mark has already sent a v2 series, I applied his v2 at first.
Could you rebase and resubmit on top of my for-next branch?


thanks,

Takashi

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Takashi Iwai]

On Thu, 01 Dec 2022 18:42:22 +0100,
Jaroslav Kysela wrote:
> 
> Let me know, if I can stack your changes on top, or perhaps, you may
> be willing to adapt them.

As Mark has already sent a v2 series, I applied his v2 at first.
Could you rebase and resubmit on top of my for-next branch?


thanks,

Takashi

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 639 bytes --]

On Thu, Dec 01, 2022 at 08:06:22PM +0100, Takashi Iwai wrote:
> On Thu, 01 Dec 2022 18:42:22 +0100,
> Jaroslav Kysela wrote:
> > 
> > Let me know, if I can stack your changes on top, or perhaps, you may
> > be willing to adapt them.
> 
> As Mark has already sent a v2 series, I applied his v2 at first.
> Could you rebase and resubmit on top of my for-next branch?

Oh, this is getting a little confusing - I'd just picked Jaroslav's
patch into my tree and was in the middle redoing my ideas on top of his
code!  I might have something more later this evening...  I think we can
converge here, let me continue taking a look.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 639 bytes --]

On Thu, Dec 01, 2022 at 08:06:22PM +0100, Takashi Iwai wrote:
> On Thu, 01 Dec 2022 18:42:22 +0100,
> Jaroslav Kysela wrote:
> > 
> > Let me know, if I can stack your changes on top, or perhaps, you may
> > be willing to adapt them.
> 
> As Mark has already sent a v2 series, I applied his v2 at first.
> Could you rebase and resubmit on top of my for-next branch?

Oh, this is getting a little confusing - I'd just picked Jaroslav's
patch into my tree and was in the middle redoing my ideas on top of his
code!  I might have something more later this evening...  I think we can
converge here, let me continue taking a look.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Takashi Iwai]

On Thu, 01 Dec 2022 21:29:48 +0100,
Mark Brown wrote:
> 
> On Thu, Dec 01, 2022 at 08:06:22PM +0100, Takashi Iwai wrote:
> > On Thu, 01 Dec 2022 18:42:22 +0100,
> > Jaroslav Kysela wrote:
> > > 
> > > Let me know, if I can stack your changes on top, or perhaps, you may
> > > be willing to adapt them.
> > 
> > As Mark has already sent a v2 series, I applied his v2 at first.
> > Could you rebase and resubmit on top of my for-next branch?
> 
> Oh, this is getting a little confusing - I'd just picked Jaroslav's
> patch into my tree and was in the middle redoing my ideas on top of his
> code!  I might have something more later this evening...  I think we can
> converge here, let me continue taking a look.

Ah then it was my misunderstanding, and everything should be fine now
;)  Thanks!


Takashi

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Takashi Iwai]

On Thu, 01 Dec 2022 21:29:48 +0100,
Mark Brown wrote:
> 
> On Thu, Dec 01, 2022 at 08:06:22PM +0100, Takashi Iwai wrote:
> > On Thu, 01 Dec 2022 18:42:22 +0100,
> > Jaroslav Kysela wrote:
> > > 
> > > Let me know, if I can stack your changes on top, or perhaps, you may
> > > be willing to adapt them.
> > 
> > As Mark has already sent a v2 series, I applied his v2 at first.
> > Could you rebase and resubmit on top of my for-next branch?
> 
> Oh, this is getting a little confusing - I'd just picked Jaroslav's
> patch into my tree and was in the middle redoing my ideas on top of his
> code!  I might have something more later this evening...  I think we can
> converge here, let me continue taking a look.

Ah then it was my misunderstanding, and everything should be fine now
;)  Thanks!


Takashi

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Takashi Iwai]

On Fri, 02 Dec 2022 08:52:03 +0100,
Takashi Iwai wrote:
> 
> On Thu, 01 Dec 2022 21:29:48 +0100,
> Mark Brown wrote:
> > 
> > On Thu, Dec 01, 2022 at 08:06:22PM +0100, Takashi Iwai wrote:
> > > On Thu, 01 Dec 2022 18:42:22 +0100,
> > > Jaroslav Kysela wrote:
> > > > 
> > > > Let me know, if I can stack your changes on top, or perhaps, you may
> > > > be willing to adapt them.
> > > 
> > > As Mark has already sent a v2 series, I applied his v2 at first.
> > > Could you rebase and resubmit on top of my for-next branch?
> > 
> > Oh, this is getting a little confusing - I'd just picked Jaroslav's
> > patch into my tree and was in the middle redoing my ideas on top of his
> > code!  I might have something more later this evening...  I think we can
> > converge here, let me continue taking a look.
> 
> Ah then it was my misunderstanding, and everything should be fine now
> ;)  Thanks!

Erm, you meant sent as *v3*.  I've seen now.

As the v2 patches were already merged, could you rather rebase and
resubmit?  I'd like to avoid rebase the full series that are already
included in linux-next.

Apologies for the mess.


Takashi

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Takashi Iwai]

On Fri, 02 Dec 2022 08:52:03 +0100,
Takashi Iwai wrote:
> 
> On Thu, 01 Dec 2022 21:29:48 +0100,
> Mark Brown wrote:
> > 
> > On Thu, Dec 01, 2022 at 08:06:22PM +0100, Takashi Iwai wrote:
> > > On Thu, 01 Dec 2022 18:42:22 +0100,
> > > Jaroslav Kysela wrote:
> > > > 
> > > > Let me know, if I can stack your changes on top, or perhaps, you may
> > > > be willing to adapt them.
> > > 
> > > As Mark has already sent a v2 series, I applied his v2 at first.
> > > Could you rebase and resubmit on top of my for-next branch?
> > 
> > Oh, this is getting a little confusing - I'd just picked Jaroslav's
> > patch into my tree and was in the middle redoing my ideas on top of his
> > code!  I might have something more later this evening...  I think we can
> > converge here, let me continue taking a look.
> 
> Ah then it was my misunderstanding, and everything should be fine now
> ;)  Thanks!

Erm, you meant sent as *v3*.  I've seen now.

As the v2 patches were already merged, could you rather rebase and
resubmit?  I'd like to avoid rebase the full series that are already
included in linux-next.

Apologies for the mess.


Takashi

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Jaroslav Kysela]

On 02. 12. 22 8:54, Takashi Iwai wrote:
> On Fri, 02 Dec 2022 08:52:03 +0100,
> Takashi Iwai wrote:
>>
>> On Thu, 01 Dec 2022 21:29:48 +0100,
>> Mark Brown wrote:
>>>
>>> On Thu, Dec 01, 2022 at 08:06:22PM +0100, Takashi Iwai wrote:
>>>> On Thu, 01 Dec 2022 18:42:22 +0100,
>>>> Jaroslav Kysela wrote:
>>>>>
>>>>> Let me know, if I can stack your changes on top, or perhaps, you may
>>>>> be willing to adapt them.
>>>>
>>>> As Mark has already sent a v2 series, I applied his v2 at first.
>>>> Could you rebase and resubmit on top of my for-next branch?
>>>
>>> Oh, this is getting a little confusing - I'd just picked Jaroslav's
>>> patch into my tree and was in the middle redoing my ideas on top of his
>>> code!  I might have something more later this evening...  I think we can
>>> converge here, let me continue taking a look.
>>
>> Ah then it was my misunderstanding, and everything should be fine now
>> ;)  Thanks!
> 
> Erm, you meant sent as *v3*.  I've seen now.
> 
> As the v2 patches were already merged, could you rather rebase and
> resubmit?  I'd like to avoid rebase the full series that are already
> included in linux-next.

It's rebased. The first patch from the set drops the previous Mark's changes.

						Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Jaroslav Kysela]

On 02. 12. 22 8:54, Takashi Iwai wrote:
> On Fri, 02 Dec 2022 08:52:03 +0100,
> Takashi Iwai wrote:
>>
>> On Thu, 01 Dec 2022 21:29:48 +0100,
>> Mark Brown wrote:
>>>
>>> On Thu, Dec 01, 2022 at 08:06:22PM +0100, Takashi Iwai wrote:
>>>> On Thu, 01 Dec 2022 18:42:22 +0100,
>>>> Jaroslav Kysela wrote:
>>>>>
>>>>> Let me know, if I can stack your changes on top, or perhaps, you may
>>>>> be willing to adapt them.
>>>>
>>>> As Mark has already sent a v2 series, I applied his v2 at first.
>>>> Could you rebase and resubmit on top of my for-next branch?
>>>
>>> Oh, this is getting a little confusing - I'd just picked Jaroslav's
>>> patch into my tree and was in the middle redoing my ideas on top of his
>>> code!  I might have something more later this evening...  I think we can
>>> converge here, let me continue taking a look.
>>
>> Ah then it was my misunderstanding, and everything should be fine now
>> ;)  Thanks!
> 
> Erm, you meant sent as *v3*.  I've seen now.
> 
> As the v2 patches were already merged, could you rather rebase and
> resubmit?  I'd like to avoid rebase the full series that are already
> included in linux-next.

It's rebased. The first patch from the set drops the previous Mark's changes.

						Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

On Fri, Dec 02, 2022 at 09:56:39AM +0100, Jaroslav Kysela wrote:
> On 02. 12. 22 8:54, Takashi Iwai wrote:
> > Takashi Iwai wrote:

> > > > Oh, this is getting a little confusing - I'd just picked Jaroslav's
> > > > patch into my tree and was in the middle redoing my ideas on top of his
> > > > code!  I might have something more later this evening...  I think we can
> > > > converge here, let me continue taking a look.

> > > Ah then it was my misunderstanding, and everything should be fine now
> > > ;)  Thanks!

> > Erm, you meant sent as *v3*.  I've seen now.

> > As the v2 patches were already merged, could you rather rebase and
> > resubmit?  I'd like to avoid rebase the full series that are already
> > included in linux-next.

> It's rebased. The first patch from the set drops the previous Mark's changes.

Indeed, there was so many collisions with Jaroslav's patches
which it just seemed like the most straightforward way to do
things (plus I'd already written a good chunk of the new version
by the time you applied my v2).  Probably only a small bit of the
skipping code would end up remaining anyway.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 0/6] kselftest/alsa: pcm-test improvements

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

On Fri, Dec 02, 2022 at 09:56:39AM +0100, Jaroslav Kysela wrote:
> On 02. 12. 22 8:54, Takashi Iwai wrote:
> > Takashi Iwai wrote:

> > > > Oh, this is getting a little confusing - I'd just picked Jaroslav's
> > > > patch into my tree and was in the middle redoing my ideas on top of his
> > > > code!  I might have something more later this evening...  I think we can
> > > > converge here, let me continue taking a look.

> > > Ah then it was my misunderstanding, and everything should be fine now
> > > ;)  Thanks!

> > Erm, you meant sent as *v3*.  I've seen now.

> > As the v2 patches were already merged, could you rather rebase and
> > resubmit?  I'd like to avoid rebase the full series that are already
> > included in linux-next.

> It's rebased. The first patch from the set drops the previous Mark's changes.

Indeed, there was so many collisions with Jaroslav's patches
which it just seemed like the most straightforward way to do
things (plus I'd already written a good chunk of the new version
by the time you applied my v2).  Probably only a small bit of the
skipping code would end up remaining anyway.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]