On 4/29/22 05:07, Xiaojuan Yang wrote:
+/*
+ * Shift bits and filed mask
+ */
+#define TOY_MON_MASK 0x3f
+#define TOY_DAY_MASK 0x1f
+#define TOY_HOUR_MASK 0x1f
+#define TOY_MIN_MASK 0x3f
+#define TOY_SEC_MASK 0x3f
+#define TOY_MSEC_MASK 0xf
+
+#define TOY_MON_SHIFT 26
+#define TOY_DAY_SHIFT 21
+#define TOY_HOUR_SHIFT 16
+#define TOY_MIN_SHIFT 10
+#define TOY_SEC_SHIFT 4
+#define TOY_MSEC_SHIFT 0
+
+#define TOY_MATCH_YEAR_MASK 0x3f
+#define TOY_MATCH_MON_MASK 0xf
+#define TOY_MATCH_DAY_MASK 0x1f
+#define TOY_MATCH_HOUR_MASK 0x1f
+#define TOY_MATCH_MIN_MASK 0x3f
+#define TOY_MATCH_SEC_MASK 0x3f
+
+#define TOY_MATCH_YEAR_SHIFT 26
+#define TOY_MATCH_MON_SHIFT 22
+#define TOY_MATCH_DAY_SHIFT 17
+#define TOY_MATCH_HOUR_SHIFT 12
+#define TOY_MATCH_MIN_SHIFT 6
+#define TOY_MATCH_SEC_SHIFT 0
While this is ok, it would be better to use <hw/registerfields.h>
This becomes
FIELD(TOY, MON, 26, 6)
FIELD(TOY, DAY, 21, 5)
FIELD(TOY, HOUR, 16, 5)
You then extract with FIELD_EX32(val, TOY, MON), etc.
I will also mention that "millisec" is misnamed in the documentation. Since it represents
units of 0.1 seconds, the prefix should be "deci".
+#define TOY_ENABLE_BIT (1U << 11)
...
+enum {
+ TOYEN = 1UL << 11,
+ RTCEN = 1UL << 13,
+};
You have two copies of the same bit. It would be best to fill in the rest of the bits in
RTCCTRL, using FIELD().
+ case SYS_RTCREAD0:
+ val = s->rtccount;
+ break;
Surely this needs to examine qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) and scale the offset
back to 32kHz.
+ case SYS_TOYMATCH0:
+ s->toymatch[0] = val;
+ qemu_get_timedate(&tm, s->offset);
+ tm.tm_sec = (val >> TOY_MATCH_SEC_SHIFT) & TOY_MATCH_SEC_MASK;
+ tm.tm_min = (val >> TOY_MATCH_MIN_SHIFT) & TOY_MATCH_MIN_MASK;
+ tm.tm_hour = ((val >> TOY_MATCH_HOUR_SHIFT) & TOY_MATCH_HOUR_MASK);
+ tm.tm_mday = ((val >> TOY_MATCH_DAY_SHIFT) & TOY_MATCH_DAY_MASK);
+ tm.tm_mon = ((val >> TOY_MATCH_MON_SHIFT) & TOY_MATCH_MON_MASK) - 1;
+ year_diff = ((val >> TOY_MATCH_YEAR_SHIFT) & TOY_MATCH_YEAR_MASK);
+ year_diff = year_diff - (tm.tm_year & TOY_MATCH_YEAR_MASK);
+ tm.tm_year = tm.tm_year + year_diff;
+ alarm_offset = qemu_timedate_diff(&tm) - s->offset;
+ if ((alarm_offset < 0) && (alarm_offset > -5)) {
+ alarm_offset = 0;
+ }
+ expire_time = qemu_clock_get_ms(rtc_clock);
+ expire_time += ((alarm_offset * 1000) + 100);
+ timer_mod(s->timer, expire_time);
+ break;
+ case SYS_TOYMATCH1:
+ s->toymatch[1] = val;
+ break;
+ case SYS_TOYMATCH2:
+ s->toymatch[2] = val;
+ break;
Why does only register 0 affect expire time, and not all 3 registers?
+ case SYS_RTCCTRL:
+ s->cntrctl = val;
+ break;
Need to check REN, TEN, and EO fields.
+ case SYS_RTCWRTIE0:
+ s->rtccount = val;
+ break;
Need to compute a new rtc_offset from QEMU_CLOCK_VIRTUAL, to match RTCREAD0
above.
+ case SYS_RTCMATCH0:
+ s->rtcmatch[0] = val;
+ break;
+ case SYS_RTCMATCH1:
+ val = s->rtcmatch[1];
+ break;
+ case SYS_RTCMATCH2:
+ val = s->rtcmatch[2];
+ break;
Why do these not affect expire time?
+ d->timer = timer_new_ms(rtc_clock, toy_timer, d);
+ timer_mod(d->timer, qemu_clock_get_ms(rtc_clock) + 100);
Where does this number come from? In any case, after reset I would expect (1) the rtc and
toy to be disabled (documentation says must initialize rtcctrl bits) and (2) I would
expect all of the comparison registers to be 0 and thus no timer enabled.
I guess this 100 milliseconds thing is trying to match 1 decisecond from
TOYMATCH0?
r~
