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

[Tero Kristo]

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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> > ---
> >   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