Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <8b71b3a0-28ed-6894-5566-139d7fc03223@linux.intel.com>
Date:   Thu, 18 Aug 2022 16:33:53 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Subject: Re: [PATCH] HID: i2c-hid: fix the report-id passed in via
 set_or_send_report
Content-Language: en-US
To:     Angela Czubak <acz@semihalf.com>
Cc:     linux-input@vger.kernel.org, benjamin.tissoires@redhat.com,
        jikos@kernel.org, linux-kernel@vger.kernel.org,
        dmitry.torokhov@gmail.com
References: <20220726080504.4185715-1-tero.kristo@linux.intel.com>
 <CAB4aORWWL7w1rVCrFw-Q2aof6WeQZR5iYRFg2UrvQQwHZzy7hA@mail.gmail.com>
From:   Tero Kristo <tero.kristo@linux.intel.com>
In-Reply-To: <CAB4aORWWL7w1rVCrFw-Q2aof6WeQZR5iYRFg2UrvQQwHZzy7hA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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