On Tue, Jan 18, 2022 at 8:26 AM Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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
>
