Re: [PATCH] ns16550: ensure polling timer is disarmed

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.07.2025 23:42, dmkhn@xxxxxxxxx wrote:
> On Thu, Jul 31, 2025 at 08:54:10AM +0200, Jan Beulich wrote:
>> On 30.07.2025 20:31, dmkhn@xxxxxxxxx wrote:
>>> On Wed, Jul 30, 2025 at 10:12:54AM +0200, Jan Beulich wrote:
>>>> On 30.07.2025 05:13, dmkhn@xxxxxxxxx wrote:
>>>>> From: Denis Mukhin <dmukhin@xxxxxxxx>
>>>>>
>>>>> As it stands, polling timer is kept in the list of timers even after the
>>>>> interrupts have been enabled / polling disabled on ns16550-compatible 
>>>>> UART.
>>>>>
>>>>> Ensure polling timer is removed from the timer list once UART interrupts 
>>>>> are
>>>>> enabled.
>>>>>
>>>>> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
>>>>
>>>> Wasn't it Andrew(?) who suggested something along these lines? That would
>>>> want reflecting by a tag then.
>>>
>>> Yes, indeed.
>>>
>>>>
>>>> Also, what's the real problem you want to solve here? The timer function
>>>> would be run one more time after ->intr_works is set, and then the timer
>>>> will be permanently inactive (up to a possible S3 resume). Is it being on
>>>> an inactive list an actual problem? (IOW I'd like to understand if the
>>>> change is merely cosmetic, or if there is some actual benefit.)
>>>
>>> My understanding is running polling timer one more time after the interrupts
>>> are enabled is the issue: if there's a pending timer when it is known the
>>> timer not needed, then the timer should be canceled.
>>
>> And the effort of canceling outweighs the one extra running of the timer?
> 
> I think so, because intr_works will not flip at run-time once set.
> If so, no need to keep the timer ready to be rearmed.

Well, to me it looks like a code size increase with extremely limited benefit.
Hence while likely I wouldn't outright NAK such a change, I also wouldn't ACK
it.

Jan