On 09/28/2015 03:10 AM, David Herrmann wrote:
Hi
On Mon, Sep 28, 2015 at 2:23 AM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
[...]
}
static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
[snip]
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index 2c14812..33280f3 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -1173,7 +1173,7 @@ static int rmi_populate(struct hid_device *hdev)
return 0;
}
-static void rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
+static int rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
{
struct rmi_data *data = hid_get_drvdata(hdev);
struct input_dev *input = hi->input;
@@ -1185,10 +1185,10 @@ static void rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
hid_dbg(hdev, "Opening low level driver\n");
ret = hid_hw_open(hdev);
if (ret)
- return;
+ return ret;
if (!(data->device_flags & RMI_DEVICE))
- return;
+ return -ENXIO;
We should return 0 here. Otherwise, this will break certain keyboards on
composite USB devices which share a VID and PID with our touchpad. If
the RMI_DEVICE flag is not set then hid-rmi will pass those reports onto
hid-input for processing.
/* Allow incoming hid reports */
hid_device_io_start(hdev);
@@ -1228,7 +1228,9 @@ static void rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 0x0f, 0, 0);
input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0, 0x0f, 0, 0);
- input_mt_init_slots(input, data->max_fingers, INPUT_MT_POINTER);
+ ret = input_mt_init_slots(input, data->max_fingers, INPUT_MT_POINTER);
+ if (ret < 0)
+ goto exit;
if (data->button_count) {
__set_bit(EV_KEY, input->evbit);
@@ -1244,6 +1246,7 @@ static void rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
exit:
hid_device_io_stop(hdev);
hid_hw_close(hdev);
+ return ret;
rmi_probe() has an explicit comment that it *wants* hid_probe() to
continue on failure, to make sure hidraw is loaded. Not sure what to
make with that, but please either remove the comment in rmi_probe() or
make sure to always return 0 from rmi_input_configured().
I think that comment is erroneous since the fact that we could not
attach hidinput interface should not affect hidraw in any shape or form.
The comment might indeed be correct. If rmi_input_configured() failed,
probing is continued, but the device-internal state might be
different. rmi_probe() checks that, and explicitly continues device
probing in that case (if it didn't, the device would indeed be
rejected).
Sorry for the confusion. Your changes to rmi do look correct.
I will fix the comment, it does incorrectly imply that returning an
error from rmi_probe() would disconnect hidraw. It is also hard to
understand without the context of the previous version. If you look at
the change (daebdd7) it was the call to hid_hw_stop() which disconnected
hidraw and not the return code, if rmi_populate() can't find F11 on the
device.
Otherwise, the changes to rmi look correct.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
