Dear Shuah Khan,
On Wed, Dec 08, 2021 at 02:25:36PM -0700, Shuah Khan wrote:
On 12/8/21 2:17 PM, Mark Brown wrote:
From: Takashi Sakamoto o-takashi@sakamocchi.jp
The volatile attribute of control element means that the hardware can voluntarily change the state of control element independent of any operation by software. ALSA control core necessarily sends notification to userspace subscribers for any change from userspace application, while it doesn't for the hardware's voluntary change.
This commit adds optimization for the attribute. Even if read value is different from written value, the test reports success as long as the target control element has the attribute. On the other hand, the difference is itself reported for developers' convenience.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp Link: https://lore.kernel.org/r/Ya7TAHdMe9i41bsC@workstation Signed-off-by: Mark Brown broonie@kernel.org
tools/testing/selftests/alsa/mixer-test.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/alsa/mixer-test.c b/tools/testing/selftests/alsa/mixer-test.c index ab51cf7b9e03..171d33692c7b 100644 --- a/tools/testing/selftests/alsa/mixer-test.c +++ b/tools/testing/selftests/alsa/mixer-test.c @@ -307,9 +307,13 @@ bool show_mismatch(struct ctl_data *ctl, int index, } 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;
// NOTE: The volatile attribute means that the hardware can voluntarily change the
// state of control element independent of any operation by software.
Let's stick to kernel comment format :)
bool is_volatile = snd_ctl_elem_info_is_volatile(ctl->info);
ksft_print_msg("%s.%d expected %lld but read %lld, is_volatile %d\n",
ctl->name, index, expected_int, read_int, is_volatile);
} else { return false; }return !is_volatile;
With that change:
Reviewed-by: Shuah Khan skhan@linuxfoundation.org
Thanks for your review. Indeed, when following to existent guideline of coding style, the comment should follow to C89/C90 style. I have no objection to your advice itself, while for the guideline itself I'd like to ask your opinion (and your help if possible).
In section '8) Commenting' in 'Documentation/process/coding-style.rst', we can see no example for comment prefixed with double slashes; '//'. On the other hand, we can see tons of actual usage in whole tree. We have the inconsistency between the guideline and what developers have done.
I think that the decision to allow double-slashes comment or not is left to subsystem maintainers, while I know that it's not allowed in UAPI header since they are built with --std=C90 compiler option (see head of 'usr/include/Makefile'). I can not find such restriction in the other parts of kernel code.
In my reference book about C language, double-slashes comment was officially introduced in C99 (ISO/IEC 9899:1999) therefore it's not specific to C++ nowadays. It's merely out of specification called as 'standard C' or 'ANSI C' (C89/C90, ISO/IEC 9899:1990).
Linux Torvalds appeared as his acceptance of double-slashes comment in the context about his intolerance of multi-line comment such that the introduction of comment, '/*', is just followed by content of comment without line break:
* Re: [patch] crypto: sha256-mb - cleanup a || vs | typo * https://lore.kernel.org/lkml/CA+55aFyQYJerovMsSoSKS7PessZBr4vNp-3QUUwhqk4A4_...
His preference is not necessarily equivalent to collective opinion in kernel development community when seeing the patch applied later:
* commit c4ff1b5f8bf0 ("CodingStyle: add networking specific block comment style") * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
His opinion does not necessarily have complete clout in the community, but overall there is less reason to reject the double-slashes comment.
In my opinion, it's time to modify the coding style documentation in the point of comment so that:
* accept double-slashes comment from C99 in whole tree * except for UAPI header (to keep backward compatibility of userspace applications still built for C89/C90)
...But the discussion about official acceptance of C99 code can itself evolve many developers since it's equivalent to loss of backward compatibility to the environment built just for C89/C90. It's the reason I never work for it since I have limited resources to join in the discussion (I'm unpaid hobbyist with language barrier. My task in sound subsystem is development and maintenance of in-kernel protocol implementation of IEC 61883-1/6 and application drivers, including heavy load for reverse engineering).
I'm glad if getting your assistance somehow for the issue.
Best regards
Takashi Sakamoto