* [PATCH] input: cros_ec_keyb: fix button/switch capability reports
@ 2018-11-07 22:38 Brian Norris
2018-11-08 8:55 ` Heiko Stuebner
2018-11-12 19:24 ` Dmitry Torokhov
0 siblings, 2 replies; 4+ messages in thread
From: Brian Norris @ 2018-11-07 22:38 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Enric Balletbo i Serra, linux-kernel, linux-input, Doug Anderson
The cros_ec_keyb_bs array lists buttons and switches together, expecting
that its users will match the appropriate type and bit fields. But
cros_ec_keyb_register_bs() only checks the 'bit' field, which causes
misreported input capabilities in some cases. For example, tablets
(e.g., Scarlet -- a.k.a. Acer Chromebook Tab 10) were reporting a SW_LID
capability, because EC_MKBP_POWER_BUTTON and EC_MKBP_LID_OPEN happen to
share the same bit.
(This has comedic effect on a tablet, in which a power-management daemon
then thinks this "lid" is closed, and so puts the system to sleep as
soon as it boots!)
To fix this, check both the 'ev_type' and 'bit' fields before reporting
the capability.
Tested with a lid (Kevin / Samsung Chromebook Plus) and without a lid
(Scarlet / Acer Chromebook Tab 10).
This error got introduced when porting the feature from the downstream
Chromium OS kernel to be upstreamed.
Fixes: cdd7950e7aa4 ("input: cros_ec_keyb: Add non-matrix buttons and switches")
Cc: <stable@vger.kernel.org>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
drivers/input/keyboard/cros_ec_keyb.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 81be6f781f0b..d56001181598 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -493,7 +493,8 @@ static int cros_ec_keyb_register_bs(struct cros_ec_keyb *ckdev)
for (i = 0; i < ARRAY_SIZE(cros_ec_keyb_bs); i++) {
const struct cros_ec_bs_map *map = &cros_ec_keyb_bs[i];
- if (buttons & BIT(map->bit))
+ if ((map->ev_type == EV_KEY && (buttons & BIT(map->bit))) ||
+ (map->ev_type == EV_SW && (switches & BIT(map->bit))))
input_set_capability(idev, map->ev_type, map->code);
}
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] input: cros_ec_keyb: fix button/switch capability reports
2018-11-07 22:38 [PATCH] input: cros_ec_keyb: fix button/switch capability reports Brian Norris
@ 2018-11-08 8:55 ` Heiko Stuebner
2018-11-12 19:24 ` Dmitry Torokhov
1 sibling, 0 replies; 4+ messages in thread
From: Heiko Stuebner @ 2018-11-08 8:55 UTC (permalink / raw)
To: Brian Norris
Cc: Dmitry Torokhov, Enric Balletbo i Serra, linux-kernel,
linux-input, Doug Anderson
Am Mittwoch, 7. November 2018, 23:38:13 CET schrieb Brian Norris:
> The cros_ec_keyb_bs array lists buttons and switches together, expecting
> that its users will match the appropriate type and bit fields. But
> cros_ec_keyb_register_bs() only checks the 'bit' field, which causes
> misreported input capabilities in some cases. For example, tablets
> (e.g., Scarlet -- a.k.a. Acer Chromebook Tab 10) were reporting a SW_LID
> capability, because EC_MKBP_POWER_BUTTON and EC_MKBP_LID_OPEN happen to
> share the same bit.
>
> (This has comedic effect on a tablet, in which a power-management daemon
> then thinks this "lid" is closed, and so puts the system to sleep as
> soon as it boots!)
>
> To fix this, check both the 'ev_type' and 'bit' fields before reporting
> the capability.
>
> Tested with a lid (Kevin / Samsung Chromebook Plus) and without a lid
> (Scarlet / Acer Chromebook Tab 10).
>
> This error got introduced when porting the feature from the downstream
> Chromium OS kernel to be upstreamed.
>
> Fixes: cdd7950e7aa4 ("input: cros_ec_keyb: Add non-matrix buttons and switches")
> Cc: <stable@vger.kernel.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> ---
> drivers/input/keyboard/cros_ec_keyb.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index 81be6f781f0b..d56001181598 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -493,7 +493,8 @@ static int cros_ec_keyb_register_bs(struct cros_ec_keyb *ckdev)
> for (i = 0; i < ARRAY_SIZE(cros_ec_keyb_bs); i++) {
> const struct cros_ec_bs_map *map = &cros_ec_keyb_bs[i];
>
> - if (buttons & BIT(map->bit))
> + if ((map->ev_type == EV_KEY && (buttons & BIT(map->bit))) ||
> + (map->ev_type == EV_SW && (switches & BIT(map->bit))))
> input_set_capability(idev, map->ev_type, map->code);
> }
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] input: cros_ec_keyb: fix button/switch capability reports
2018-11-07 22:38 [PATCH] input: cros_ec_keyb: fix button/switch capability reports Brian Norris
2018-11-08 8:55 ` Heiko Stuebner
@ 2018-11-12 19:24 ` Dmitry Torokhov
1 sibling, 0 replies; 4+ messages in thread
From: Dmitry Torokhov @ 2018-11-12 19:24 UTC (permalink / raw)
To: Brian Norris
Cc: Enric Balletbo i Serra, linux-kernel, linux-input, Doug Anderson
On Wed, Nov 07, 2018 at 02:38:13PM -0800, Brian Norris wrote:
> The cros_ec_keyb_bs array lists buttons and switches together, expecting
> that its users will match the appropriate type and bit fields. But
> cros_ec_keyb_register_bs() only checks the 'bit' field, which causes
> misreported input capabilities in some cases. For example, tablets
> (e.g., Scarlet -- a.k.a. Acer Chromebook Tab 10) were reporting a SW_LID
> capability, because EC_MKBP_POWER_BUTTON and EC_MKBP_LID_OPEN happen to
> share the same bit.
>
> (This has comedic effect on a tablet, in which a power-management daemon
> then thinks this "lid" is closed, and so puts the system to sleep as
> soon as it boots!)
>
> To fix this, check both the 'ev_type' and 'bit' fields before reporting
> the capability.
>
> Tested with a lid (Kevin / Samsung Chromebook Plus) and without a lid
> (Scarlet / Acer Chromebook Tab 10).
>
> This error got introduced when porting the feature from the downstream
> Chromium OS kernel to be upstreamed.
>
> Fixes: cdd7950e7aa4 ("input: cros_ec_keyb: Add non-matrix buttons and switches")
> Cc: <stable@vger.kernel.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
Applied, thank you.
> ---
> drivers/input/keyboard/cros_ec_keyb.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index 81be6f781f0b..d56001181598 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -493,7 +493,8 @@ static int cros_ec_keyb_register_bs(struct cros_ec_keyb *ckdev)
> for (i = 0; i < ARRAY_SIZE(cros_ec_keyb_bs); i++) {
> const struct cros_ec_bs_map *map = &cros_ec_keyb_bs[i];
>
> - if (buttons & BIT(map->bit))
> + if ((map->ev_type == EV_KEY && (buttons & BIT(map->bit))) ||
> + (map->ev_type == EV_SW && (switches & BIT(map->bit))))
> input_set_capability(idev, map->ev_type, map->code);
> }
>
> --
> 2.19.1.930.g4563a0d9d0-goog
>
--
Dmitry
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] input: cros_ec_keyb: fix button/switch capability reports
@ 2018-11-07 22:27 Brian Norris
0 siblings, 0 replies; 4+ messages in thread
From: Brian Norris @ 2018-11-07 22:27 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Enric Balletbo i Serra, linux-kernel, linux-input, Doug Anderson
The cros_ec_keyb_bs array lists buttons and switches together, expecting
that its users will match the appropriate type and bit fields. But
cros_ec_keyb_register_bs() only checks the 'bit' field, which causes
misreported input capabilities in some cases. For example, tablets
(e.g., Scarlet -- a.k.a. Acer Chromebook Tab 10) were reporting a SW_LID
capability, because EC_MKBP_POWER_BUTTON and EC_MKBP_LID_OPEN happen to
share the same bit.
(This has comedic effect on a tablet, in which a power-management daemon
then thinks this "lid" is closed, and so puts the system to sleep as
soon as it boots!)
To fix this, check both the 'ev_type' and 'bit' fields before reporting
the capability.
Tested with a lid (Kevin / Samsung Chromebook Plus) and without a lid
(Scarlet / Acer Chromebook Tab 10).
This error got introduced when porting the feature from the downstream
Chromium OS kernel to be upstreamed.
Fixes: cdd7950e7aa4 ("input: cros_ec_keyb: Add non-matrix buttons and switches")
Cc: <stable@vger.kernel.org>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
drivers/input/keyboard/cros_ec_keyb.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 81be6f781f0b..d56001181598 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -493,7 +493,8 @@ static int cros_ec_keyb_register_bs(struct cros_ec_keyb *ckdev)
for (i = 0; i < ARRAY_SIZE(cros_ec_keyb_bs); i++) {
const struct cros_ec_bs_map *map = &cros_ec_keyb_bs[i];
- if (buttons & BIT(map->bit))
+ if ((map->ev_type == EV_KEY && (buttons & BIT(map->bit))) ||
+ (map->ev_type == EV_SW && (switches & BIT(map->bit))))
input_set_capability(idev, map->ev_type, map->code);
}
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-11-12 19:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-07 22:38 [PATCH] input: cros_ec_keyb: fix button/switch capability reports Brian Norris
2018-11-08 8:55 ` Heiko Stuebner
2018-11-12 19:24 ` Dmitry Torokhov
-- strict thread matches above, loose matches on Subject: below --
2018-11-07 22:27 Brian Norris
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).