Re: [PATCH v2 1/2] x86/time: split CMOS time fetching into wrapper

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

On Tue, Sep 03, 2024 at 08:24:18AM +0200, Jan Beulich wrote:
> On 30.08.2024 11:52, Roger Pau Monne wrote:
> > @@ -1285,33 +1270,56 @@ static unsigned long get_cmos_time(void)
> >          } while ( (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
> >                    t2 < MILLISECS(3) );
> >  
> > -        __get_cmos_time(&rtc);
> > +        __get_cmos_time(rtc);
> >  
> >          spin_unlock_irqrestore(&rtc_lock, flags);
> >  
> > -        if ( likely(!cmos_rtc_probe) ||
> > -             t1 > SECONDS(1) || t2 >= MILLISECS(3) ||
> > -             rtc.sec >= 60 || rtc.min >= 60 || rtc.hour >= 24 ||
> > -             !rtc.day || rtc.day > 31 ||
> > -             !rtc.mon || rtc.mon > 12 )
> > -            break;
> > +        if ( likely(!cmos_rtc_probe) )
> > +            return true;
> > +
> > +        if ( t1 > SECONDS(1) || t2 >= MILLISECS(3) ||
> > +             rtc->sec >= 60 || rtc->min >= 60 || rtc->hour >= 24 ||
> > +             !rtc->day || rtc->day > 31 ||
> > +             !rtc->mon || rtc->mon > 12 )
> > +            return false;
> >  
> >          if ( seconds < 60 )
> >          {
> > -            if ( rtc.sec != seconds )
> > -            {
> > -                cmos_rtc_probe = false;
> 
> This clearing of the variable is lost, which looks wrong to me.

Note the code in get_cmos_time() is modified, so the variable is no
longer used past the call to read_cmos_time().  Instead the signaling
of whether the CMOS is functional or not is done using the return
value of the newly introduced read_cmos_time() function.

Thanks, Roger.