lkml.org 
[lkml]   [2022]   [Feb]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 02/12] HID: i2c-hid: fix GET/SET_REPORT for unnumbered reports
    On Tue, Jan 18, 2022 at 8:26 AM Dmitry Torokhov
    <dmitry.torokhov@gmail.com> wrote:
    >
    > Internally kernel prepends all report buffers, for both numbered and
    > unnumbered reports, with report ID, therefore to properly handle unnumbered
    > reports we should

    Nitpick but it seems that this sentence is not

    :)

    Cheers,
    Benjamin

    >
    > For the same reason we should skip the first byte of the buffer when
    > calling i2c_hid_set_or_send_report() which then will take care of properly
    > formatting the transfer buffer based on its separate report ID argument
    > along with report payload.
    >
    > Fixes: 9b5a9ae88573 ("HID: i2c-hid: implement ll_driver transport-layer callbacks")
    > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
    > ---
    > drivers/hid/i2c-hid/i2c-hid-core.c | 32 ++++++++++++++++++++++--------
    > 1 file changed, 24 insertions(+), 8 deletions(-)
    >
    > diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
    > index bd7b0eeca3ea..b383003ff676 100644
    > --- a/drivers/hid/i2c-hid/i2c-hid-core.c
    > +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
    > @@ -611,6 +611,17 @@ static int i2c_hid_get_raw_report(struct hid_device *hid,
    > if (report_type == HID_OUTPUT_REPORT)
    > return -EINVAL;
    >
    > + /*
    > + * In case of unnumbered reports the response from the device will
    > + * not have the report ID that the upper layers expect, so we need
    > + * to stash it the buffer ourselves and adjust the data size.
    > + */
    > + if (!report_number) {
    > + buf[0] = 0;
    > + buf++;
    > + count--;
    > + }
    > +
    > /* +2 bytes to include the size of the reply in the query buffer */
    > ask_count = min(count + 2, (size_t)ihid->bufsize);
    >
    > @@ -632,6 +643,9 @@ static int i2c_hid_get_raw_report(struct hid_device *hid,
    > count = min(count, ret_count - 2);
    > memcpy(buf, ihid->rawbuf + 2, count);
    >
    > + if (!report_number)
    > + count++;
    > +
    > return count;
    > }
    >
    > @@ -648,17 +662,19 @@ static int i2c_hid_output_raw_report(struct hid_device *hid, __u8 *buf,
    >
    > mutex_lock(&ihid->reset_lock);
    >
    > - if (report_id) {
    > - buf++;
    > - count--;
    > - }
    > -
    > + /*
    > + * Note that both numbered and unnumbered reports passed here
    > + * are supposed to have report ID stored in the 1st byte of the
    > + * buffer, so we strip it off unconditionally before passing payload
    > + * to i2c_hid_set_or_send_report which takes care of encoding
    > + * everything properly.
    > + */
    > ret = i2c_hid_set_or_send_report(client,
    > report_type == HID_FEATURE_REPORT ? 0x03 : 0x02,
    > - report_id, buf, count, use_data);
    > + report_id, buf + 1, count - 1, use_data);
    >
    > - if (report_id && ret >= 0)
    > - ret++; /* add report_id to the number of transfered bytes */
    > + if (ret >= 0)
    > + ret++; /* add report_id to the number of transferred bytes */
    >
    > mutex_unlock(&ihid->reset_lock);
    >
    > --
    > 2.34.1.703.g22d0c6ccf7-goog
    >

    \
     
     \ /
      Last update: 2022-02-03 15:17    [W:6.047 / U:1.144 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site