Re: [PATCH v4 6/8] x86/HPET: simplify "expire" check a little in reprogram_hpet_evt_channel()

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Jan 22, 2026 at 10:28:51AM +0100, Jan Beulich wrote:
> On 22.01.2026 10:18, Roger Pau Monné wrote:
> > On Mon, Nov 17, 2025 at 03:39:30PM +0100, Jan Beulich wrote:
> >> When this was added, the log message was updated correctly, but the zero
> >> case was needlessly checked separately: hpet_broadcast_enter() had a zero
> >> check added at the same time, while handle_hpet_broadcast() can't possibly
> >> pass 0 here anyway.
> >>
> >> Fixes: 7145897cfb81 ("cpuidle: Fix for timer_deadline==0 case")
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > 
> > Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Thanks.
> 
> > Similar to the previous commit, I wonder whether it would make sense
> > to add an ASSERT_UNREACHABLE() if that error path is not reachable
> > given the logic in the callers.
> 
> That would mean
> 
>     if ( unlikely(expire < 0) )
>     {
>         printk(KERN_DEBUG "reprogram: expire <= 0\n");
>         return -ETIME;
>     }
> 
>     if ( unlikely(expire == 0) )
>     {
>         ASSERT_UNREACHABLE();
>         return -ETIME;
>     }
> 
> which I fear I don't like (for going too far). Even
> 
>     if ( unlikely(expire <= 0) )
>     {
>         printk(KERN_DEBUG "reprogram: expire <= 0\n");
>         ASSERT(expire);
>         return -ETIME;
>     }
> 
> I'd be uncertain about, as that needlessly gives 0 a meaning that isn't
> required anymore in this function.

Hm, OK, I was under the impression that both < 0 and 0 should never be
passed by the callers.  If expire == 0 is a possible input then I
don't think the ASSERT() is that helpful.

Thanks, Roger.