Received-SPF: pass (google.com: domain of linux-kernel+bounces-150536-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <f2429c78-9189-410d-9c6a-644ae8a4d12c@gmail.com>
Date: Thu, 18 Apr 2024 20:04:11 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] HID: uclogic: Remove useless loop
Content-Language: en-US
To: Stefan Berzl <stefanberzl@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>
From: Nikolai Kondrashov <spbnick@gmail.com>
In-Reply-To: <4ae4be2f-4edd-4d1e-87e9-df5687627d00@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

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

Thank you, Stefan and Jiri!
Nick