Bug#794266: rtc-s35390a: the big mess or inconsistencies during startup
Hello,
there was a bug reported against the Debian kernel that seems related to
the rtc-s35390a driver/chip. See https://bugs.debian.org/794266.
I looked a bit into the driver now, and there are several problems. As I
don't have access to such a chip I just want to tell what I found and
how I think it should be tackled.
- The rtc-s35390a chip's alarm uses only minute, hour and dow.
The .read_alarm callback (s35390a_read_alarm()) returns -EINVAL if
the alarm is not enabled. I think it should just set alm->enabled = 0
in this case. Further it only sets alm->time.tm_wday,
alm->time.tm_hour and alm->time.tm_min which isn't handled in a sane
way by __rtc_read_alarm. Maybe rtc_read_alarm_internal should better
initialize all fields of alarm->time to -1 instead of 0?
- During startup we saw:
[ 2.257418] rtc rtc0: invalid alarm value: 1900-1-29 1193031:57:16
I don't see how this big hour value can be found, looking at the
driver it sets alm->time.tm_hour at most to
bcd2bin(reg & 0x3f) + 12
where reg is a char. Then in __rtc_read_alarm we get into the missing
= day case (because it doesn't handle an initialized wday).
So it must be rtc_time64_to_tm that returns that big hour, probably
because rtc_tm_to_time64(&alarm->time) is < 0 which rtc_time64_to_tm
cannot handle?
So the action items are:
- let rtc_read_alarm_internal initialize alarm->time to 9*{-1} (or fix
s35390a_read_alarm to set the uninitialized values to -1).
- let s35390a_read_alarm set alm->pending and alm->enabled.
- teach __rtc_read_alarm to handle mday = -1 but wday >= 0.
- tell hw-engineers not to use read-to-clear events (use a big clue
stick).
- debug rtc_tm_to_time64 + rtc_time64_to_tm for dates < 1970
(or clamp t_alm in __rtc_read_alarm to >= 0?)
Any volunteers?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Reply to: