On 28/11/2019 11:04:19-0500, Jean-Francois Dagenais wrote: > Hi Michal, all, > > > On Nov 28, 2019, at 03:19, Michal Simek <michal.simek@xxxxxxxxxx> wrote: > > > >> in CURRENT_TIME and proceeds to use SET_TIME_READ (RTC_SET_TM_RD in the > >> code). This register contains garbage at this moment and this is > >> returned as the current time. > > > > How did you test this? > > Remember the linux trampoline code... jumping from FSBL to the kernel without > u-boot? Well, got it working and it speeds our boot substantially. This means we > now reach rtc-zynqmp.c's probe within one second of power on. When we boot the > board without a psbatt, and no internet connection (which means > systemd-timesyncd cannot fix the bunkers date), we were ending up with a date in > the very distant future (>50 years into the future). Aside from the date in our > UI, all our SSL connections were failing because of certificate dates. This got > our attention. > > We drilled down quite a lot to find the real root cause. We used JTAG with xsct > with our board simply powered on but not booted (bootselect SD without an SD > card). This means no FSBL/psu_init code has run. > > Exhibit A: > > xsct% rrd rtc > set_time_write set_time_read: 00000000 > calib_write calib_read: 00000000 > current_time: 00000000 alarm: 00000000 > rtc_int_status: 00 rtc_int_mask: 03 > rtc_int_en rtc_int_dis > addr_error: 00 addr_error_int_mask: 01 > addr_error_int_en addr_error_int_dis > control: 01000000 safety_chk: 00000000 > > > Then we enable the RTC: > > xsct% rwr rtc control 0x81000000 > Doesn't that enable battery switchover instead of simply enabling the rtc, I though you didn't have a battery. Or does that mean that your previous read of control returning bit 24 set is also bogus? I ask because the simpler solution would simply to return -EINVAL in xlnx_rtc_read_time when you detect a power on condition. > The counter now counts, set_time_read and calib_read are garbage: > xsct% rrd rtc > set_time_write set_time_read: fffe6bff > calib_write calib_read: 000f7fff > current_time: 00000002 alarm: 00000000 > rtc_int_status: 01 rtc_int_mask: 03 > rtc_int_en rtc_int_dis > addr_error: 00 addr_error_int_mask: 01 > addr_error_int_en addr_error_int_dis > control: 81000000 safety_chk: 00000000 > xsct% rrd rtc > set_time_write set_time_read: fffe6bff > calib_write calib_read: 000f7fff > current_time: 00000005 alarm: 00000000 > rtc_int_status: 01 rtc_int_mask: 03 > rtc_int_en rtc_int_dis > addr_error: 00 addr_error_int_mask: 01 > addr_error_int_en addr_error_int_dis > control: 81000000 safety_chk: 00000000 > xsct% rrd rtc > set_time_write set_time_read: fffe6bff > calib_write calib_read: 000f7fff > current_time: 00000008 alarm: 00000000 > rtc_int_status: 01 rtc_int_mask: 03 > rtc_int_en rtc_int_dis > addr_error: 00 addr_error_int_mask: 01 > addr_error_int_en addr_error_int_dis > control: 81000000 safety_chk: 00000000 > > We tested further, inserted a psbatt, enabled the RTC, but never touched > set_time_write. Powered our board on and off. The observations is that that > set_time_read and calib_read registers are not initialized to 0x0 upon power on. > Once set though, they keep their values as long as the system is powered on or > psbatt keeps the RTC alive. This is a contradiction of the register POR values > specification. > > So for rtc-zynqmp.c, it means that using the set_time_read register is a bad > idea unless the code can definitely say that the set_time_write has been written > to. And this is exactly what our patch does, by use of the seconds interrupt > being enabled which signals the read_time() function that there is a pending > write, and therefore the set_time_read can reliably be read. > > Hope this helps! > -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
