On Sun, Sep 22, 2019 at 1:05 PM Alexandre Belloni <alexandre.bell...@bootlin.com> wrote: > > On 22/09/2019 18:13:06+0200, Pavel Machek wrote: > > On Mon 2019-09-16 12:12:15, Nick Crews wrote: > > > The tm_yday and tm_wday fields are not used by userspace, > > > so since they aren't needed within the driver, don't > > > bother calculating them. This is especially needed since > > > the rtc_year_days() call was crashing if the HW returned > > > an invalid time. > > > > > > Signed-off-by: Nick Crews <ncr...@chromium.org> > > > --- > > > drivers/rtc/rtc-wilco-ec.c | 4 ---- > > > 1 file changed, 4 deletions(-) > > > > > > diff --git a/drivers/rtc/rtc-wilco-ec.c b/drivers/rtc/rtc-wilco-ec.c > > > index 8ad4c4e6d557..e84faa268caf 100644 > > > --- a/drivers/rtc/rtc-wilco-ec.c > > > +++ b/drivers/rtc/rtc-wilco-ec.c > > > @@ -110,10 +110,6 @@ static int wilco_ec_rtc_read(struct device *dev, > > > struct rtc_time *tm) > > > tm->tm_mday = rtc.day; > > > tm->tm_mon = rtc.month - 1; > > > tm->tm_year = rtc.year + (rtc.century * 100) - 1900; > > > - tm->tm_yday = rtc_year_days(tm->tm_mday, tm->tm_mon, tm->tm_year); > > > - > > > - /* Don't compute day of week, we don't need it. */ > > > - tm->tm_wday = -1; > > > > > > return 0; > > > > Are you sure? It would be bad to pass unititialized memory to userspace... > > > > This problem doesn't exist because userspace is passing the memory, not > the other way around. > > > If userspace does not need those fields, why are they there? > > > > This is coming from struct tm, it is part of C89 but I think I was not > born when this decision was made. man rtc properly reports that those > fields are unused and no userspace tools are actually making use of > them. Nobody cares about the broken down representation of the time. > What is done is use the ioctl then mktime to have a UNIX timestamp. > > "The mktime function ignores the specified contents of the tm_wday, > tm_yday, tm_gmtoff, and tm_zone members of the broken-down time > structure. It uses the values of the other components to determine the > calendar time; it’s permissible for these components to have > unnormalized values outside their normal ranges. The last thing that > mktime does is adjust the components of the brokentime structure, > including the members that were initially ignored."
This is very non-obvious and I only knew this from talking to you, Alexandre. Perhaps we should add this note to the RTC core, such as in the description for rtc_class_ops? For this patch, do you want me to make any further changes? Thanks, Nick > > > -- > Alexandre Belloni, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
