Re: [PATCH v4 38/43] hw/loongarch: Add LoongArch ls7a rtc device support

[yangxiaojuan]

On 2022/5/19 上午3:59, Richard Henderson wrote:
> On 5/17/22 04:30, Xiaojuan Yang wrote:
>
> > +static void ls7a_stop_toymatch(LS7ARtcState *s)
> > +{
> > +    int i;
> > +    uint64_t now;
> > +
> > +    now = qemu_clock_get_ms(rtc_clock);
> > +    for (i = 0; i < TIMER_NUMS; i++) {
> > +        if (s->toy_timer[i].flag) {
> > +            s->toy_timer[i].enable_offset = 
> > s->toy_timer[i].timer->expire_time
> > +                                            - now;
> > +            timer_del(s->toy_timer[i].timer);
>
> I don't think you need to check flag here, or update enable_offset.
> Just an unconditional timer_del to stop the timer callback from firing.
Thanks very much, and i fixed it like this: Is this modification 
appropriate?
static void ls7a_rtc_stop(LS7ARtcState *s)
{
    int i;
    int64_t rtc_val, rtc_diff, now;
    now = ls7a_rtc_ticks();

    for (i = 0; i < TIMER_NUMS; i++) {
        if (s->rtc_timer[i].flag) {
            rtc_val = s->rtcmatch[i];
            rtc_diff = rtc_val - now - s->offset_rtc;
            s->rtc_timer[i].save_offset = rtc_diff;
        }
        timer_del(s->rtc_timer[i].timer);
}
> > +    case SYS_RTCWRTIE0:
> > +        s->offset_rtc = val - ls7a_rtc_ticks();
>
> This needs to behave differently when !rtc_enabled, and when 
> rtc_enabled reinit the timers for RTCMATCHn, because the time 
> differential has changed.
Thanks, I fixed it as this:
    case SYS_RTCWRTIE0:
        if (rtc_enabled(s)) {
            s->offset_rtc = val - ls7a_rtc_ticks();
        } else {
            s->rtc_write_save = val;
         }
    break;

static void ls7a_rtc_start(LS7ARtcState *s)
{
    int i;
    int64_t expire_time, now;

    now = ls7a_rtc_ticks();

    if (rtc_save_val) {
        old_offset = s->offset_rtc;
        s->offset_rtc = rtc_save_val - now;
        offset_diff = s->offset_rtc - old_offset;
    }

    for (i = 0; i < TIMER_NUMS; i++) {
        if (s->rtc_timer[i].flag && rtc_enabled(s)) {
            expire_time = ticks + s->rtc_timer[i].save_offset + 
offset_diff;
            expire_time = (expire_time * 1000 / LS7A_RTC_FREQ);
            timer_mod(s->rtc_timer[i].timer, expire_time);
        }
    }
}

Thanks.
Xiaojuan