Re: [PATCH v2] kselftest: alsa: Add simplistic test for ALSA mixer controls kselftest

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

>> +// This test will iterate over all cards detected in the system, exercising
> 
> would it make sense to test only specific cards? People doing automated
> tests might have a USB device for capture of analog loopbacks, or
> injection of specific streams for capture, and usually care about

typo: Usually don't care about testing such devices.

'testing the tester' is a separate endeavor.

> testing such devices - which do need manual setups and wiring btw.
> 
>> +	switch (snd_ctl_elem_info_get_type(ctl->info)) {
>> +	case SND_CTL_ELEM_TYPE_NONE:
>> +		ksft_print_msg("%s Invalid control type NONE\n", ctl->name);
>> +		err = -1;
>> +		break;
>> +
>> +	case SND_CTL_ELEM_TYPE_BOOLEAN:
>> +		int_val = snd_ctl_elem_value_get_boolean(ctl->def_val, 0);
>> +		switch (int_val) {
>> +		case 0:
>> +		case 1:
>> +			break;
>> +		default:
>> +			ksft_print_msg("%s Invalid boolean value %ld\n",
>> +				       ctl->name, int_val);
>> +			err = -1;
>> +			break;
>> +		}
>> +		break;
>> +
>> +	case SND_CTL_ELEM_TYPE_INTEGER:
>> +		int_val = snd_ctl_elem_value_get_integer(ctl->def_val, 0);
>> +
>> +		if (int_val < snd_ctl_elem_info_get_min(ctl->info)) {
>> +			ksft_print_msg("%s value %ld less than minimum %ld\n",
>> +				       ctl->name, int_val,
>> +				       snd_ctl_elem_info_get_min(ctl->info));
>> +			err = -1;
>> +		}
>> +
>> +		if (int_val > snd_ctl_elem_info_get_max(ctl->info)) {
>> +			ksft_print_msg("%s value %ld more than maximum %ld\n",
>> +				       ctl->name, int_val,
>> +				       snd_ctl_elem_info_get_max(ctl->info));
>> +			err = -1;
>> +		}
>> +
>> +		/* Only check step size if there is one and we're in bounds */
>> +		if (err >= 0 && snd_ctl_elem_info_get_step(ctl->info) &&
>> +		    (int_val - snd_ctl_elem_info_get_min(ctl->info) %
>> +		     snd_ctl_elem_info_get_step(ctl->info))) {
>> +			ksft_print_msg("%s value %ld invalid for step %ld minimum %ld\n",
>> +				       ctl->name, int_val,
>> +				       snd_ctl_elem_info_get_step(ctl->info),
>> +				       snd_ctl_elem_info_get_min(ctl->info));
>> +			err = -1;
>> +		}
>> +		break;
>> +
>> +	case SND_CTL_ELEM_TYPE_INTEGER64:
>> +		int64_val = snd_ctl_elem_value_get_integer64(ctl->def_val, 0);
>> +
>> +		if (int64_val < snd_ctl_elem_info_get_min64(ctl->info)) {
>> +			ksft_print_msg("%s value %lld less than minimum %lld\n",
>> +				       ctl->name, int64_val,
>> +				       snd_ctl_elem_info_get_min64(ctl->info));
>> +			err = -1;
>> +		}
>> +
>> +		if (int64_val > snd_ctl_elem_info_get_max64(ctl->info)) {
>> +			ksft_print_msg("%s value %lld more than maximum %lld\n",
>> +				       ctl->name, int64_val,
>> +				       snd_ctl_elem_info_get_max(ctl->info));
>> +			err = -1;
>> +		}
>> +
>> +		/* Only check step size if there is one and we're in bounds */
>> +		if (err >= 0 && snd_ctl_elem_info_get_step64(ctl->info) &&
>> +		    (int64_val - snd_ctl_elem_info_get_min64(ctl->info)) %
>> +		    snd_ctl_elem_info_get_step64(ctl->info)) {
>> +			ksft_print_msg("%s value %lld invalid for step %lld minimum %lld\n",
>> +				       ctl->name, int64_val,
>> +				       snd_ctl_elem_info_get_step64(ctl->info),
>> +				       snd_ctl_elem_info_get_min64(ctl->info));
>> +			err = -1;
>> +		}
>> +		break;
>> +
>> +	default:
>> +		/* No tests for other types */
> 
> these types include ENUMERATED, BYTES and IEC958, but see below for
> ENUMERATED...
> 
>> +		ksft_test_result_skip("get_value.%d.%d\n",
>> +				      ctl->card->card, ctl->elem);
>> +		return;
>> +	}
>> +
>> +out:
>> +	ksft_test_result(err >= 0, "get_value.%d.%d\n",
>> +			 ctl->card->card, ctl->elem);
>> +}
>> +
>> +bool show_mismatch(struct ctl_data *ctl, int index,
>> +		   snd_ctl_elem_value_t *read_val,
>> +		   snd_ctl_elem_value_t *expected_val)
>> +{
>> +	long long expected_int, read_int;
>> +
>> +	/*
>> +	 * We factor out the code to compare values representable as
>> +	 * integers, ensure that check doesn't log otherwise.
>> +	 */
>> +	expected_int = 0;
>> +	read_int = 0;
>> +
>> +	switch (snd_ctl_elem_info_get_type(ctl->info)) {
>> +	case SND_CTL_ELEM_TYPE_BOOLEAN:
>> +		expected_int = snd_ctl_elem_value_get_boolean(expected_val,
>> +							      index);
>> +		read_int = snd_ctl_elem_value_get_boolean(read_val, index);
>> +		break;
>> +
>> +	case SND_CTL_ELEM_TYPE_INTEGER:
>> +		expected_int = snd_ctl_elem_value_get_integer(expected_val,
>> +							      index);
>> +		read_int = snd_ctl_elem_value_get_integer(read_val, index);
>> +		break;
>> +
>> +	case SND_CTL_ELEM_TYPE_INTEGER64:
>> +		expected_int = snd_ctl_elem_value_get_integer64(expected_val,
>> +								index);
>> +		read_int = snd_ctl_elem_value_get_integer64(read_val,
>> +							    index);
>> +		break;
>> +
>> +	case SND_CTL_ELEM_TYPE_ENUMERATED:
> 
> ... here you are handling ENUMERATED types?
> 
>> +		expected_int = snd_ctl_elem_value_get_enumerated(expected_val,
>> +								 index);
>> +		read_int = snd_ctl_elem_value_get_enumerated(read_val,
>> +							     index);
>> +		break;
>> +
>> +	default:
>> +		break;
>> +	}
>> +
>> +	if (expected_int != read_int) {
>> +		ksft_print_msg("%s.%d expected %lld but read %lld\n",
>> +			       ctl->name, index, expected_int, read_int);
>> +		return true;
>> +	} else {
>> +		return false;
>> +	}
>> +}
>