From d0a934b764c67b4bf626f5b7cf725a6e3066afd2 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Mon, 13 May 2013 17:01:30 +0200 Subject: HID: input: return ENODATA if reading battery attrs fails power_supply core has the bad habit of calling our battery callbacks from within power_supply_register(). Furthermore, if the callbacks fail with an unhandled error code, it will skip any uevent that it might currently process. So if HID-core registers battery devices, an "add" uevent is generated and the battery callbacks are called. These will gracefully fail due to timeouts as they might still hold locks on event processing. One could argue that this should be fixed in power_supply core, but the least we can do is to signal ENODATA so power_supply core will just skip the property and continue with the uevent. This fixes a bug where "add" and "remove" uevents are skipped for battery devices. upower is unable to track these devices and currently needs to ignore them. This patch also overwrites any other error code. I cannot see any reason why we should forward protocol- or I/O-errors to the power_supply core. We handle these errors in hid_ll_driver later, anyway, so just skip them. power_supply core cannot do anything useful with them, anyway, and we avoid skipping important uevents and confusing user-space. Thanks a lot to Daniel Nicoletti for pushing and investigating on this. Cc: Jiri Kosina Cc: Anton Vorontsov Cc: David Woodhouse Reported-by: Daniel Nicoletti Signed-off-by: David Herrmann Signed-off-by: Jiri Kosina --- drivers/hid/hid-input.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/hid/hid-input.c') diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index 945b8158ec4..c526a3ccbe5 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -354,10 +354,10 @@ static int hidinput_get_battery_property(struct power_supply *psy, dev->battery_report_type); if (ret != 2) { - if (ret >= 0) - ret = -EINVAL; + ret = -ENODATA; break; } + ret = 0; if (dev->battery_min < dev->battery_max && buf[1] >= dev->battery_min && -- cgit v1.2.3-70-g09d2 From 6f1891d01956cad406d2af8ed2e9cef6108bfc5e Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 19 Jun 2013 17:49:05 +0200 Subject: HID: fix false positive out of range values Commit 6da7066906e977d42104a859c490f5f9a300488c introduced in 3.3 "HID: ignore absolute values which don't fit between logical min and max" prevents some Posiflex touch screen to work because they do not provide logical min and max for their buttons. Thus, logical min and max are at 0, discarding the buttons events, and preventing the device to report appropriate X Y. Adding a check on "min < max" solves the problem. Reported-by: Jan Kandziora Tested-by: Jan Kandziora Signed-off-by: Benjamin Tissoires Signed-off-by: Jiri Kosina --- drivers/hid/hid-input.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/hid/hid-input.c') diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index 945b8158ec4..82130cf724e 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -1045,6 +1045,7 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct * section 5.10 and 6.2.25 */ if ((field->flags & HID_MAIN_ITEM_VARIABLE) && + (field->logical_minimum < field->logical_maximum) && (value < field->logical_minimum || value > field->logical_maximum)) { dbg_hid("Ignoring out-of-range value %x\n", value); -- cgit v1.2.3-70-g09d2 From a688393bd3fb27690a77f7ae8607b4969039bac5 Mon Sep 17 00:00:00 2001 From: Jiri Kosina Date: Wed, 19 Jun 2013 23:52:11 +0200 Subject: HID: explain out-of-range check better Extend the comment explaining the condition for discarding out-of-range values to clarify the cases in which devices don't provide any logical min/max. Signed-off-by: Jiri Kosina --- drivers/hid/hid-input.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/hid/hid-input.c') diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index 82130cf724e..9aeca602361 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -1042,7 +1042,11 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct /* * Ignore out-of-range values as per HID specification, - * section 5.10 and 6.2.25 + * section 5.10 and 6.2.25. + * + * The logical_minimum < logical_maximum check is done so that we + * don't unintentionally discard values sent by devices which + * don't specify logical min and max. */ if ((field->flags & HID_MAIN_ITEM_VARIABLE) && (field->logical_minimum < field->logical_maximum) && -- cgit v1.2.3-70-g09d2