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@xxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxx> > > Link: https://lore.kernel.org/r/Ya7TAHdMe9i41bsC@workstation > > Signed-off-by: Mark Brown <broonie@xxxxxxxxxx> > > --- > > 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); > > + return !is_volatile; > > } else { > > return false; > > } > > > > With that change: > > Reviewed-by: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> 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_jcbg@xxxxxxxxxxxxxx/ 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/?id=c4ff1b5f8bf0 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
