On Fri, Jan 14, 2022 at 7:24 PM Angela Czubak [off-list ref] wrote:
Hi Benjamin,
On Thu, Jan 13, 2022 at 10:54 AM Benjamin Tissoires
[off-list ref] wrote:
quoted
On Wed, Jan 12, 2022 at 12:26 PM Angela Czubak [off-list ref] wrote:
quoted
On Wed, Jan 12, 2022 at 10:43 AM Benjamin Tissoires
[off-list ref] wrote:
quoted
On 1/10/22 20:43, Angela Czubak wrote:
quoted
On Fri, Jan 7, 2022 at 11:01 PM Dmitry Torokhov
[off-list ref] wrote:
quoted
On Tue, Dec 21, 2021 at 07:17:30PM +0000, Angela Czubak wrote:
quoted
Move mt_get_feature from hid-multitouch to hid-core as it is a generic
function that can be used by other drivers as well.
Signed-off-by: Angela Czubak <redacted>
---
drivers/hid/hid-core.c | 39 ++++++++++++++++++++++++++++++++++++
drivers/hid/hid-multitouch.c | 38 +++--------------------------------
include/linux/hid.h | 1 +
3 files changed, 43 insertions(+), 35 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index dbed2524fd47..c11cb7324157 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1796,6 +1796,45 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
}
EXPORT_SYMBOL_GPL(hid_report_raw_event);
+/**
+ * hid_get_feature - retrieve feature report from device
+ *
+ * @hdev: hid device
+ * @report: hid report to retrieve
+ */
+void hid_get_feature(struct hid_device *hdev, struct hid_report *report)
If this is a generic API I believe it should return success/error code
so that users can decide what to do.
Does it mean I should also modify hid-multitouch.c so that the return
value is actually checked? Currently it seems to ignore any failures.
quoted
Thanks.
Honestly that function is a hack in hid-multitouch. You can replace it by:
hid_device_io_start(hid);
hid_hw_request(hid, report, HID_REQ_GET_REPORT);
hid_hw_wait(hid);
hid_device_io_stop(hid);
The hack allows to not have to use hid_device_io_{start|stop}(), which
is probably not clean.
As for the return value, hid_hw_request() can be used as asynchronous,
which is why it returns void. However, returning an actual int would
definitively be better because some cases are failing silently (like if
the device is not io started).
I am slightly confused; it is hid_hw_raw_request() that is used and it does
not seem asynchronous to me; is there no guarantee that the response
has already been received?
In the case of usbhid, hid_hw_request() calls directly
__usbhid_submit_report() which is asynchronous.
So no, we have no guarantees that the answer is there.
quoted
It seemed to me that the main purpose of
this function is to retrieve information an have it correctly parsed.
I literally issue it once to learn if auto trigger has been set by default and
to know the durations of waveforms, learn ordinals etc.
I could introduce a new function for the purpose of haptic API, it just
seemed redundant as the one in hid-multitouch.c does what I need.
Again, the one in hid-multitouch is a hack against
hid_device_io_{start|stop}(). So if you need to change something, it's
the hid-multitouch code, not reuse that hack :)
ACK, I will use hid_hw_request() and hid_hw_wait() instead as suggested.
BTW is hid_device_io_{start|stop} required to do anything meaningful?
It just seems to me that it currently sets some variable which is not really
useful anywhere else.
The thing it does is to release the semaphore
hid->driver_input_lock(), which is then in turn used in hid-core.c, in
hid_input_report(). Without this, hid_input_report() aborts earlier
and the field->values[] are not populated because HID considers it
should ignore the incoming reports.
So without this, you can not access the new values apart from doing
what hid-multitouch does: trick the system and force inject an event
in the processing pipeline.
Cheers,
Benjamin