On Wed, Jan 12, 2022 at 10:43 AM Benjamin Tissoires
<benjamin.tissoires@xxxxxxxxxx> wrote:
>
> On 1/10/22 20:43, Angela Czubak wrote:
> > On Fri, Jan 7, 2022 at 11:01 PM Dmitry Torokhov
> > <dmitry.torokhov@xxxxxxxxx> wrote:
> >>
> >> On Tue, Dec 21, 2021 at 07:17:30PM +0000, Angela Czubak wrote:
> >>> 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 <acz@xxxxxxxxxxxx>
> >>> ---
> >>> 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.
> >> 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? 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.
> Cheers,
> Benjamin
>
> >
> >>
> >> --
> >> Dmitry
>
