Nothing checks that the put() callback of an IEC958 control reports a change, because write_valid skips these controls.
Where the device publishes a mask, toggle every bit in it and check each write. A bit the mask advertises that does not stick fails the test, the way a rejected value fails for the other control types. Where none is found, keep the old single guess at the non audio bit, which the device is free to ignore, so only the notification can be counted against it. That path writes without checking so drop_events() clears what those writes leave behind. On an HDA card with four HDMI PCMs the Con Mask is 0f ff, so each control walks eleven bits instead of one and all eleven stick: -# Totals: pass:212 fail:5 xfail:0 xpass:0 skip:56 error:0 +# Totals: pass:216 fail:5 xfail:0 xpass:0 skip:52 error:0 A driver whose put() never reports a change still shows up as event_missing. hdmi-codec and img-spdif-out store every byte and return 0, so boards using them report one event_missing per walked bit, and hdmi-codec advertises the whole block so that is 191 of them. A driver whose mask claims more than its put() keeps now fails write_valid outright: au88x0 and ice1712 on Delta 1010 and Mediastation advertise bytes they never store, cs46xx, oxygen and ice1724 advertise bits their put() drops, and the RME cards regenerate the whole emphasis field from one bit so the rest of it never reads back. None of that hardware was to hand, the paths were read rather than run. write_invalid stays a skip. Every bit pattern is a valid channel status. Signed-off-by: HyeongJun An <[email protected]> Assisted-by: Claude:claude-opus-5 --- Changes in v3: - Say check rather than grading. - Where a mask is found, walk every bit it advertises instead of one and fail a bit that does not stick. - Split the two cases into test_ctl_write_valid_iec958_mask() and test_ctl_write_valid_iec958_guess(), so drop_events() now runs only on the guess path. - Keep the guess for the four drivers with no mask. Adding the mask controls to fsl_spdif, fsl_xcvr, uniperif_player and stm32_sai_sub is a separate change for each of them. - Report when the mask lookup falls back to the non audio guess, and report a drop_events() failure with the raw return value since three of the wait_for_event() paths hand back a bare -1. - Name the drivers whose mask claims more than their put() keeps, which a full walk widens beyond the three v2 mentioned. v2: https://lore.kernel.org/all/[email protected]/ v1: https://lore.kernel.org/all/[email protected]/ tools/testing/selftests/alsa/mixer-test.c | 255 ++++++++++++++++++++++ 1 file changed, 255 insertions(+) diff --git a/tools/testing/selftests/alsa/mixer-test.c b/tools/testing/selftests/alsa/mixer-test.c index 0857d64c322a..53a72753bb08 100644 --- a/tools/testing/selftests/alsa/mixer-test.c +++ b/tools/testing/selftests/alsa/mixer-test.c @@ -30,6 +30,12 @@ #define TESTS_PER_CONTROL 7 +/* Suffixes of the SNDRV_CTL_NAME_IEC958() names, not exported to userspace */ +#define IEC958_DEFAULT "Default" +#define IEC958_CON_MASK "Con Mask" +#define IEC958_PRO_MASK "Pro Mask" +#define IEC958_MASK "Mask" + struct card_data { snd_ctl_t *handle; int card; @@ -842,6 +848,251 @@ static bool test_ctl_write_valid_enumerated(struct ctl_data *ctl) return !fail; } +/* + * Find the read only mask control for an IEC958 value control. The two + * share device, subdevice and index but not always the interface. + */ +static struct ctl_data *find_iec958_mask_ctl(struct ctl_data *ctl, + const char *suffix) +{ + char name[64]; + int stem; + struct ctl_data *mask; + + stem = strlen(ctl->name) - strlen(IEC958_DEFAULT); + if (snprintf(name, sizeof(name), "%.*s%s", stem, ctl->name, suffix) >= + (int)sizeof(name)) + return NULL; + + for (mask = ctl_list; mask != NULL; mask = mask->next) { + if (mask->card != ctl->card) + continue; + if (snd_ctl_elem_info_get_type(mask->info) != + SND_CTL_ELEM_TYPE_IEC958) + continue; + if (snd_ctl_elem_info_is_inactive(mask->info)) + continue; + if (!snd_ctl_elem_info_is_readable(mask->info)) + continue; + if (snd_ctl_elem_id_get_device(mask->id) != + snd_ctl_elem_id_get_device(ctl->id)) + continue; + if (snd_ctl_elem_id_get_subdevice(mask->id) != + snd_ctl_elem_id_get_subdevice(ctl->id)) + continue; + if (snd_ctl_elem_id_get_index(mask->id) != + snd_ctl_elem_id_get_index(ctl->id)) + continue; + if (strcmp(mask->name, name) == 0) + return mask; + } + + return NULL; +} + +/* + * Read the bits the device says it implements. Bit 0 of the first status + * byte picks which mask applies, some devices publish only a plain Mask. + */ +static bool read_iec958_mask(struct ctl_data *ctl, + const snd_aes_iec958_t *cur, + snd_aes_iec958_t *mask) +{ + int err; + struct ctl_data *mask_ctl; + snd_ctl_elem_value_t *val; + + if (!strend(ctl->name, IEC958_DEFAULT)) + return false; + + if (cur->status[0] & IEC958_AES0_PROFESSIONAL) + mask_ctl = find_iec958_mask_ctl(ctl, IEC958_PRO_MASK); + else + mask_ctl = find_iec958_mask_ctl(ctl, IEC958_CON_MASK); + if (!mask_ctl) + mask_ctl = find_iec958_mask_ctl(ctl, IEC958_MASK); + if (!mask_ctl) + return false; + + snd_ctl_elem_value_alloca(&val); + snd_ctl_elem_value_set_id(val, mask_ctl->id); + + err = snd_ctl_elem_read(mask_ctl->card->handle, val); + if (err < 0) { + ksft_print_msg("snd_ctl_elem_read() failed for %s: %s\n", + mask_ctl->name, snd_strerror(err)); + return false; + } + + snd_ctl_elem_value_get_iec958(val, mask); + + return true; +} + +/* + * Throw away the events from a write we are not checking, one left behind + * would make a missing notification look like a notification we got. + */ +static int drop_events(struct ctl_data *ctl) +{ + int err; + + do { + err = wait_for_event(ctl, 0); + } while (err > 0); + + return err; +} + +/* + * Toggle every bit the device advertises, one at a time. Each one starts + * from the value we read since a driver can look at the rest of the block + * when it stores a bit, and bit 0 of the first status byte selects the + * layout the others are read in so leave that one alone. + */ +static bool test_ctl_write_valid_iec958_mask(struct ctl_data *ctl, + snd_ctl_elem_value_t *orig_val, + const snd_aes_iec958_t *mask) +{ + int err, j; + unsigned int byte; + unsigned char bit; + bool fail = false, found = false; + snd_aes_iec958_t iec958; + snd_ctl_elem_value_t *val; + snd_ctl_elem_value_alloca(&val); + + snd_ctl_elem_value_copy(val, orig_val); + + for (byte = 0; byte < sizeof(mask->status); byte++) { + for (j = 0; j < 8; j++) { + bit = 1 << j; + + if (byte == 0 && bit == IEC958_AES0_PROFESSIONAL) + continue; + if (!(mask->status[byte] & bit)) + continue; + found = true; + + snd_ctl_elem_value_get_iec958(orig_val, &iec958); + iec958.status[byte] ^= bit; + snd_ctl_elem_value_set_iec958(val, &iec958); + + err = write_and_verify(ctl, val, NULL); + if (err != 0) { + ksft_print_msg("%s failed to set advertised status[%u] 0x%02x\n", + ctl->name, byte, bit); + fail = true; + } + } + } + + if (!found) + ksft_print_msg("%s implements no settable status bits\n", + ctl->name); + + return !fail; +} + +/* + * With nothing advertised all we can do is try non audio, and the device + * need not implement even that. Write it once and ignore the result, then + * compare against what came back so the value cannot fail, which leaves the + * notification counted by event_missing. + */ +static bool test_ctl_write_valid_iec958_guess(struct ctl_data *ctl, + snd_ctl_elem_value_t *orig_val) +{ + int err; + snd_aes_iec958_t iec958; + snd_ctl_elem_value_t *val, *read_val, *w_val; + snd_ctl_elem_value_alloca(&val); + snd_ctl_elem_value_alloca(&read_val); + snd_ctl_elem_value_alloca(&w_val); + + snd_ctl_elem_value_get_iec958(orig_val, &iec958); + iec958.status[0] ^= IEC958_AES0_NONAUDIO; + snd_ctl_elem_value_copy(val, orig_val); + snd_ctl_elem_value_set_iec958(val, &iec958); + + /* Writing can modify the value so keep a copy to write from */ + snd_ctl_elem_value_copy(w_val, val); + err = snd_ctl_elem_write(ctl->card->handle, w_val); + if (err < 0) { + ksft_print_msg("snd_ctl_elem_write() failed: %s\n", + snd_strerror(err)); + return false; + } + + snd_ctl_elem_value_set_id(read_val, ctl->id); + err = snd_ctl_elem_read(ctl->card->handle, read_val); + if (err < 0) { + ksft_print_msg("snd_ctl_elem_read() failed: %s\n", + snd_strerror(err)); + return false; + } + + /* Put it back where we found it, then drop the events from both */ + snd_ctl_elem_value_copy(w_val, orig_val); + err = snd_ctl_elem_write(ctl->card->handle, w_val); + if (err < 0) { + ksft_print_msg("snd_ctl_elem_write() failed: %s\n", + snd_strerror(err)); + return false; + } + + err = drop_events(ctl); + if (err < 0) { + ksft_print_msg("drop_events() failed for %s: %d\n", + ctl->name, err); + return false; + } + + if (snd_ctl_elem_value_compare(val, read_val)) { + ksft_print_msg("%s does not implement status[0] 0x%02x\n", + ctl->name, IEC958_AES0_NONAUDIO); + + /* Expect what came back, the notification is still checked */ + return write_and_verify(ctl, val, read_val) == 0; + } + + return write_and_verify(ctl, val, NULL) == 0; +} + +/* + * Write the channel status bits and check that userspace is told about it. + * Where a mask was found every bit in it has to stick, otherwise we fall + * back to one guessed bit the device is free to ignore. + */ +static bool test_ctl_write_valid_iec958(struct ctl_data *ctl) +{ + int err; + snd_aes_iec958_t iec958, mask; + snd_ctl_elem_value_t *orig_val; + snd_ctl_elem_value_alloca(&orig_val); + + /* + * The bytes past the ones a driver implements are compared too, + * so start from a read rather than building a value here. + */ + snd_ctl_elem_value_set_id(orig_val, ctl->id); + err = snd_ctl_elem_read(ctl->card->handle, orig_val); + if (err < 0) { + ksft_print_msg("snd_ctl_elem_read() failed: %s\n", + snd_strerror(err)); + return false; + } + + snd_ctl_elem_value_get_iec958(orig_val, &iec958); + + if (!read_iec958_mask(ctl, &iec958, &mask)) { + ksft_print_msg("%s falling back to non audio\n", ctl->name); + return test_ctl_write_valid_iec958_guess(ctl, orig_val); + } + + return test_ctl_write_valid_iec958_mask(ctl, orig_val, &mask); +} + static void test_ctl_write_valid(struct ctl_data *ctl) { bool pass; @@ -878,6 +1129,10 @@ static void test_ctl_write_valid(struct ctl_data *ctl) pass = test_ctl_write_valid_enumerated(ctl); break; + case SND_CTL_ELEM_TYPE_IEC958: + pass = test_ctl_write_valid_iec958(ctl); + break; + default: /* No tests for this yet */ ksft_test_result_skip("write_valid.%s.%d\n", -- 2.43.0
