Received-SPF: pass (google.com: domain of linux-kernel+bounces-150648-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <d26436f0-b85e-4102-a3b3-a31816f0452d@gmail.com>
Date: Thu, 18 Apr 2024 20:46:09 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] HID: uclogic: Remove useless loop
To: Nikolai Kondrashov <spbnick@gmail.com>, Jiri Kosina <jikos@kernel.org>
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20240401004757.22708-1-stefanberzl@gmail.com>
 <nycvar.YFH.7.76.2404121751250.5680@cbobk.fhfr.pm>
 <4ae4be2f-4edd-4d1e-87e9-df5687627d00@gmail.com>
 <f2429c78-9189-410d-9c6a-644ae8a4d12c@gmail.com>
Content-Language: en-US
From: Stefan Berzl <stefanberzl@gmail.com>
In-Reply-To: <f2429c78-9189-410d-9c6a-644ae8a4d12c@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

Hi!

On 18/04/2024 19:04, Nikolai Kondrashov wrote:
> Hi Jiri, Stefan,
> 
> On 4/18/24 4:31 PM, Stefan Berzl wrote:
>>
>> On 12/04/2024 17:52, Jiri Kosina wrote:
>>> On Mon, 1 Apr 2024, Stefan Berzl wrote:
>>>
>>>> The while in question does nothing except provide the possibility
>>>> to have an infinite loop in case the subreport id is actually the same
>>>> as the pen id.
>>>>
>>>> Signed-off-by: Stefan Berzl <stefanberzl@gmail.com>
>>>
>>> Let me CC Nicolai, the author of the code of question (8b013098be2c9).
>>
>> I agree that Nicolai's opinion would be invaluable, but even without it,
>> the patch is trivially correct. If we have a subreport that matches the
>> packet, we change the report_id accordingly. If we then loop back to the
>> beginning, either the report_id is different or we are caught in an
>> infinite loop. None of these are hardware registers where the access
>> itself would matter.
> 
> Yes, Stefan is right. I was trying to implement general rewrite logic, and if
> we really had that, then the fix would need to be checking that the new ID is
> different. As such there's really no need, and Stefan's fix is fine.
> 
> Only perhaps amend that comment to something like
> 
>     /* Change to the (non-pen) subreport ID, and continue */
> 
> Or at least remove ", and restart".
> 

Will do! I'll send a v2 with the comment updated.

Regards