On Fri, Jul 15, 2022 at 06:49:46AM -0700, Guenter Roeck wrote:
On 7/15/22 06:26, Alexander Lobakin wrote:
From: Guenter Roeck <linux@xxxxxxxxxxxx>
Date: Thu, 14 Jul 2022 17:04:02 -0700
On Fri, Jun 24, 2022 at 02:13:10PM +0200, Alexander Lobakin wrote:
Currently, many architecture-specific non-atomic bitop
implementations use inline asm or other hacks which are faster or
[...]
Cc: Mark Rutland <mark.rutland@xxxxxxx>
Signed-off-by: Alexander Lobakin <alexandr.lobakin@xxxxxxxxx>
Reviewed-by: Marco Elver <elver@xxxxxxxxxx>
Building i386:allyesconfig ... failed
--------------
Error log:
arch/x86/platform/olpc/olpc-xo1-sci.c: In function 'send_ebook_state':
arch/x86/platform/olpc/olpc-xo1-sci.c:83:63: error: logical not is only applied to the left hand side of comparison
Looks like a trigger, not a cause... Anyway, this construct:
unsigned char state;
[...]
if (!!test_bit(SW_TABLET_MODE, ebook_switch_idev->sw) == state)
doesn't look legit enough.
That redundant double-negation [of boolean value], together with
comparing boolean to char, provokes compilers to think the author
made logical mistakes here, although it works as expected.
Could you please try (if it's not automated build which you can't
modify) the following:
Agreed, the existing code seems wrong. The change below looks correct
and fixes the problem. Feel free to add
Reviewed-and-tested-by: Guenter Roeck <linux@xxxxxxxxxxxx>
to the real patch.
Thanks,
Guenter
--- a/arch/x86/platform/olpc/olpc-xo1-sci.c
+++ b/arch/x86/platform/olpc/olpc-xo1-sci.c
@@ -80,7 +80,7 @@ static void send_ebook_state(void)
return;
}
- if (!!test_bit(SW_TABLET_MODE, ebook_switch_idev->sw) == state)
+ if (test_bit(SW_TABLET_MODE, ebook_switch_idev->sw) == !!state)
return; /* Nothing new to report. */
input_report_switch(ebook_switch_idev, SW_TABLET_MODE, state);
---
We'd take it into the bitmap tree then. The series revealed
a fistful of existing code issues already :)
Would you like me to add your signed-off-by and apply, or you prefer
to send it out as a patch?
Thanks,
Yury
