Re: [PATCH] HID: i2c-hid: fix the report-id passed in via set_or_send_report

[Tero Kristo <tero.kristo@linux.intel.com>]

Hello,

Yes, I think this is a problem. Maybe we need to introduce some sort of 
quirk for the devices that are broken and expect the 0xf device id to be 
passed to them?

-Tero

On 18/08/2022 13:27, Angela Czubak wrote:
> Hi Tero,
>
> I believe this is not the right solution. To my mind the spec does not
> mention anything that the ID in the payload of SET_REPORT command
> should be altered; you have a full byte to us, so why not?
> Other than that this will result into problems with sending reports
> via output register: for any report with report ID >=0xF we will get
> the same report ID in the payload, so how could the device know which
> one we have in mind?
> Dmitry's rework was intended to actually solve problems with large
> report IDs being incorrectly overwritten with 0xF.
>
> Regards,
> Angela
>
> On Tue, Jul 26, 2022 at 10:05 AM Tero Kristo
> <tero.kristo@linux.intel.com> wrote:
>> The formatting of the data passed to the i2c HID data register was
>> changed with the re-work of the i2c-hid-core. Previously the report ID
>> passed in was encoded as 0xF if the report-id was greater than 0xF
>> (similar to what is done with the command portion.) Now with the rework,
>> a full report-id is passed in always, and this causes the messages to be
>> rejected by the i2c controller. Fix this by encoding the report-id
>> field in the same manner as previously was done.
>>
>> Fixes: dbe0dd5fd2e0 ("HID: i2c-hid: explicitly code setting and sending
>> reports")
>> Signed-off-by: Tero Kristo <tero.kristo@linux.intel.com>
>> ---
>>   drivers/hid/i2c-hid/i2c-hid-core.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
>> index c078f09a2318..156b12f840c4 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
>> @@ -296,6 +296,9 @@ static size_t i2c_hid_format_report(u8 *buf, int report_id,
>>   {
>>          size_t length = sizeof(__le16); /* reserve space to store size */
>>
>> +       if (report_id > 0xF)
>> +               report_id = 0xF;
>> +
>>          if (report_id)
>>                  buf[length++] = report_id;
>>
>> --
>> 2.25.1
>>