Re: [PATCH v2] HID: google: Get HID report on probe to confirm tablet switch state

2021-02-02 Thread Jiri Kosina
On Fri, 15 Jan 2021, Nicolas Boichat wrote:

> This forces reading the base folded state anytime the device is
> probed, to make sure it's in sync.
> 
> This is useful after a reboot, if the device re-enumerates for
> any reason (e.g. ESD shock), or if the driver is unbound/rebound
> (debugging/testing).
> 
> Without this, the tablet switch state is only synchronized after a
> key is pressed (since the device would then send a report that
> includes the switch state), leading to strange UX (e.g. UI
> mode changes when a key is pressed after reboot).
> 
> This is not a problem on detachable base attach, as the device,
> by itself, sends a report after it is booted up.
> 
> Signed-off-by: Nicolas Boichat 

Applied, thanks Nicolas.

-- 
Jiri Kosina
SUSE Labs



[PATCH v2] HID: google: Get HID report on probe to confirm tablet switch state

2021-01-14 Thread Nicolas Boichat
This forces reading the base folded state anytime the device is
probed, to make sure it's in sync.

This is useful after a reboot, if the device re-enumerates for
any reason (e.g. ESD shock), or if the driver is unbound/rebound
(debugging/testing).

Without this, the tablet switch state is only synchronized after a
key is pressed (since the device would then send a report that
includes the switch state), leading to strange UX (e.g. UI
mode changes when a key is pressed after reboot).

This is not a problem on detachable base attach, as the device,
by itself, sends a report after it is booted up.

Signed-off-by: Nicolas Boichat 
---
Instead of all this manual parsing, it'd be easier to just call:
hid_hw_request(hdev, report, HID_REQ_GET_REPORT);
However, that fails silently as hdev->driver_input_lock is held
during probe (or even some callbacks like input_configured.

Changes in v2:
 - Improve commit message description.

 drivers/hid/hid-google-hammer.c | 85 +
 1 file changed, 66 insertions(+), 19 deletions(-)

diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
index 85a054f1ce38..d9319622da44 100644
--- a/drivers/hid/hid-google-hammer.c
+++ b/drivers/hid/hid-google-hammer.c
@@ -392,30 +392,34 @@ static int hammer_input_mapping(struct hid_device *hdev, 
struct hid_input *hi,
return 0;
 }
 
-static int hammer_event(struct hid_device *hid, struct hid_field *field,
-   struct hid_usage *usage, __s32 value)
+static void hammer_folded_event(struct hid_device *hdev, bool folded)
 {
unsigned long flags;
 
-   if (usage->hid == HID_USAGE_KBD_FOLDED) {
-   spin_lock_irqsave(_ec_lock, flags);
+   spin_lock_irqsave(_ec_lock, flags);
 
-   /*
-* If we are getting events from Whiskers that means that it
-* is attached to the lid.
-*/
-   cbas_ec.base_present = true;
-   cbas_ec.base_folded = value;
-   hid_dbg(hid, "%s: base: %d, folded: %d\n", __func__,
-   cbas_ec.base_present, cbas_ec.base_folded);
-
-   if (cbas_ec.input) {
-   input_report_switch(cbas_ec.input,
-   SW_TABLET_MODE, value);
-   input_sync(cbas_ec.input);
-   }
+   /*
+* If we are getting events from Whiskers that means that it
+* is attached to the lid.
+*/
+   cbas_ec.base_present = true;
+   cbas_ec.base_folded = folded;
+   hid_dbg(hdev, "%s: base: %d, folded: %d\n", __func__,
+   cbas_ec.base_present, cbas_ec.base_folded);
 
-   spin_unlock_irqrestore(_ec_lock, flags);
+   if (cbas_ec.input) {
+   input_report_switch(cbas_ec.input, SW_TABLET_MODE, folded);
+   input_sync(cbas_ec.input);
+   }
+
+   spin_unlock_irqrestore(_ec_lock, flags);
+}
+
+static int hammer_event(struct hid_device *hid, struct hid_field *field,
+   struct hid_usage *usage, __s32 value)
+{
+   if (usage->hid == HID_USAGE_KBD_FOLDED) {
+   hammer_folded_event(hid, value);
return 1; /* We handled this event */
}
 
@@ -457,6 +461,47 @@ static bool hammer_has_backlight_control(struct hid_device 
*hdev)
HID_GD_KEYBOARD, HID_AD_BRIGHTNESS);
 }
 
+static void hammer_get_folded_state(struct hid_device *hdev)
+{
+   struct hid_report *report;
+   char *buf;
+   int len, rlen;
+   int a;
+
+   report = hdev->report_enum[HID_INPUT_REPORT].report_id_hash[0x0];
+
+   if (!report || report->maxfield < 1)
+   return;
+
+   len = hid_report_len(report) + 1;
+
+   buf = kmalloc(len, GFP_KERNEL);
+   if (!buf)
+   return;
+
+   rlen = hid_hw_raw_request(hdev, report->id, buf, len, report->type, 
HID_REQ_GET_REPORT);
+
+   if (rlen != len) {
+   hid_warn(hdev, "Unable to read base folded state: %d (expected 
%d)\n", rlen, len);
+   goto out;
+   }
+
+   for (a = 0; a < report->maxfield; a++) {
+   struct hid_field *field = report->field[a];
+
+   if (field->usage->hid == HID_USAGE_KBD_FOLDED) {
+   u32 value = hid_field_extract(hdev, buf+1,
+   field->report_offset, 
field->report_size);
+
+   hammer_folded_event(hdev, value);
+   break;
+   }
+   }
+
+out:
+   kfree(buf);
+}
+
 static int hammer_probe(struct hid_device *hdev,
const struct hid_device_id *id)
 {
@@ -481,6 +526,8 @@ static int hammer_probe(struct hid_device *hdev,
error = hid_hw_open(hdev);
if (error)
return error;
+
+   hammer_get_folded_state(hdev);
}
 
if