Hello Cassio,
On 10/06/2025 21:31:48+0100, Cassio Neri wrote:
Hi all,
Although untested, I'm pretty sure that with very small changes, the previous revision (1d1bb12) can handle dates prior to 1970-01-01 with no need to add extra branches or arithmetic operations. Indeed, 1d1bb12 contains:
<code> /* time must be positive */ days = div_s64_rem(time, 86400, &secs);
/* day of the week, 1970-01-01 was a Thursday */ tm->tm_wday = (days + 4) % 7;
/* long comments */
udays = ((u32) days) + 719468;
</code>
This could have been changed to:
<code> /* time must be >= -719468 * 86400 which corresponds to 0000-03-01 */ udays = div_u64_rem(time + 719468 * 86400, 86400, &secs);
/* day of the week, 0000-03-01 was a Wednesday (in the proleptic Gregorian calendar) */ tm->tm_wday = (days + 3) % 7;
/* long comments */
</code>
Indeed, the addition of 719468 * 86400 to `time` makes `days` to be 719468 more than it should be. Therefore, in the calculation of `udays`, the addition of 719468 becomes unnecessary and thus, `udays == days`. Moreover, this means that `days` can be removed altogether and replaced by `udays`. (Not the other way around because in the remaining code `udays` must be u32.)
Now, 719468 % 7 = 1 and thus tm->wday is 1 day after what it should be and we correct that by adding 3 instead of 4.
Therefore, I suggest these changes on top of 1d1bb12 instead of those made in 7df4cfe. Since you're working on this, can I please kindly suggest two other changes?
- Change the reference provided in the long comment. It should say, "The
following algorithm is, basically, Figure 12 of Neri and Schneider [1]" and [1] should refer to the published article:
Neri C, Schneider L. Euclidean affine functions and their application to calendar algorithms. Softw Pract Exper. 2023;53(4):937-970. doi: 10.1002/spe.3172 https://doi.org/10.1002/spe.3172
The article is much better written and clearer than the pre-print currently referred to.
Thanks for your input, I wanted to look again at your paper and make those optimizations which is why I took so long to review the original patch. Unfortunately, I didn't have the time before the merge window.
I would also gladly take patches for this if you are up for the task.
- Function rtc_time64_to_tm_test_date_range in drivers/rtc/lib_test.c, is
a kunit test that checks the result for everyday in a 160000 years range starting at 1970-01-01. It'd be nice if this test is adapted to the new code and starts at 1900-01-01 (technically, it could start at 0000-03-01 but since tm->year counts from 1900, it would be weird to see tm->year == -1900 to mean that the calendar year is 0.) Also 160000 is definitely an overkill (my bad!) and a couple of thousands of years, say 3000, should be more than safe for anyone. :-)
This is also something on my radar as some have been complaining about the time it takes to run those tests.
Many thanks, Cassio.
On Tue, 10 Jun 2025 at 08:35, Uwe Kleine-König u.kleine-koenig@baylibre.com wrote:
From: Alexandre Mergnat amergnat@baylibre.com
commit 7df4cfef8b351fec3156160bedfc7d6d29de4cce upstream.
Conversion of dates before 1970 is still relevant today because these dates are reused on some hardwares to store dates bigger than the maximal date that is representable in the device's native format. This prominently and very soon affects the hardware covered by the rtc-mt6397 driver that can only natively store dates in the interval 1900-01-01 up to 2027-12-31. So to store the date 2028-01-01 00:00:00 to such a device, rtc_time64_to_tm() must do the right thing for time=-2208988800.
Signed-off-by: Alexandre Mergnat amergnat@baylibre.com Reviewed-by: Uwe Kleine-König u.kleine-koenig@baylibre.com Link: https://lore.kernel.org/r/20250428-enable-rtc-v4-1-2b2f7e3f9349@baylibre.com Signed-off-by: Alexandre Belloni alexandre.belloni@bootlin.com Signed-off-by: Uwe Kleine-König u.kleine-koenig@baylibre.com
drivers/rtc/lib.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/rtc/lib.c b/drivers/rtc/lib.c index fe361652727a..13b5b1f20465 100644 --- a/drivers/rtc/lib.c +++ b/drivers/rtc/lib.c @@ -46,24 +46,38 @@ EXPORT_SYMBOL(rtc_year_days);
- rtc_time64_to_tm - converts time64_t to rtc_time.
- @time: The number of seconds since 01-01-1970 00:00:00.
(Must be positive.)
*/
Works for values since at least 1900- @tm: Pointer to the struct rtc_time.
void rtc_time64_to_tm(time64_t time, struct rtc_time *tm) {
unsigned int secs;int days;
int days, secs; u64 u64tmp; u32 u32tmp, udays, century, day_of_century, year_of_century, year, day_of_year, month, day; bool is_Jan_or_Feb, is_leap_year;
/* time must be positive */
/** Get days and seconds while preserving the sign to* handle negative time values (dates before 1970-01-01)*/ days = div_s64_rem(time, 86400, &secs);/** We need 0 <= secs < 86400 which isn't given for negative* values of time. Fixup accordingly.*/if (secs < 0) {days -= 1;secs += 86400;}/* day of the week, 1970-01-01 was a Thursday */ tm->tm_wday = (days + 4) % 7;/* Ensure tm_wday is always positive */if (tm->tm_wday < 0)tm->tm_wday += 7; /* * The following algorithm is, basically, Proposition 6.3 of Neri@@ -93,7 +107,7 @@ void rtc_time64_to_tm(time64_t time, struct rtc_time *tm) * thus, is slightly different from [1]. */
udays = ((u32) days) + 719468;
udays = days + 719468; u32tmp = 4 * udays + 3; century = u32tmp / 146097;-- 2.49.0
Hi Uwe and Alexandre,
Thank both of you for your replies.
Sure, I understand that you couldn't miss the merge window and that improvements can be made later. I wish I could contribute with a patch but I don't have much bandwidth now. I've obviously done this in the past but I have already forgotten all the processes and figuring that out again would be a steep learning curve for me. In any case, please do not hesitate to get in touch if you think I can help.
I'll be happy if you read my paper but here is another source which gives a very good overview and, I dare to say, is more entertaining :-)
https://www.youtube.com/watch?v=J9KijLyP-yg
(The talk is about the algorithms and has close to nothing to do with C++.)
Thank you again and best wishes, Cassio.
On Wed, 11 Jun 2025 at 08:33, Alexandre Belloni alexandre.belloni@bootlin.com wrote:
Hello Cassio,
On 10/06/2025 21:31:48+0100, Cassio Neri wrote:
Hi all,
Although untested, I'm pretty sure that with very small changes, the previous revision (1d1bb12) can handle dates prior to 1970-01-01 with no need to add extra branches or arithmetic operations. Indeed, 1d1bb12 contains:
<code> /* time must be positive */ days = div_s64_rem(time, 86400, &secs);
/* day of the week, 1970-01-01 was a Thursday */ tm->tm_wday = (days + 4) % 7;
/* long comments */
udays = ((u32) days) + 719468;
</code>
This could have been changed to:
<code> /* time must be >= -719468 * 86400 which corresponds to 0000-03-01 */ udays = div_u64_rem(time + 719468 * 86400, 86400, &secs);
/* day of the week, 0000-03-01 was a Wednesday (in the proleptic Gregorian calendar) */ tm->tm_wday = (days + 3) % 7;
/* long comments */
</code>
Indeed, the addition of 719468 * 86400 to `time` makes `days` to be 719468 more than it should be. Therefore, in the calculation of `udays`, the addition of 719468 becomes unnecessary and thus, `udays == days`. Moreover, this means that `days` can be removed altogether and replaced by `udays`. (Not the other way around because in the remaining code `udays` must be u32.)
Now, 719468 % 7 = 1 and thus tm->wday is 1 day after what it should be and we correct that by adding 3 instead of 4.
Therefore, I suggest these changes on top of 1d1bb12 instead of those made in 7df4cfe. Since you're working on this, can I please kindly suggest two other changes?
- Change the reference provided in the long comment. It should say, "The
following algorithm is, basically, Figure 12 of Neri and Schneider [1]" and [1] should refer to the published article:
Neri C, Schneider L. Euclidean affine functions and their application to calendar algorithms. Softw Pract Exper. 2023;53(4):937-970. doi: 10.1002/spe.3172 https://doi.org/10.1002/spe.3172
The article is much better written and clearer than the pre-print currently referred to.
Thanks for your input, I wanted to look again at your paper and make those optimizations which is why I took so long to review the original patch. Unfortunately, I didn't have the time before the merge window.
I would also gladly take patches for this if you are up for the task.
- Function rtc_time64_to_tm_test_date_range in drivers/rtc/lib_test.c, is
a kunit test that checks the result for everyday in a 160000 years range starting at 1970-01-01. It'd be nice if this test is adapted to the new code and starts at 1900-01-01 (technically, it could start at 0000-03-01 but since tm->year counts from 1900, it would be weird to see tm->year == -1900 to mean that the calendar year is 0.) Also 160000 is definitely an overkill (my bad!) and a couple of thousands of years, say 3000, should be more than safe for anyone. :-)
This is also something on my radar as some have been complaining about the time it takes to run those tests.
Many thanks, Cassio.
On Tue, 10 Jun 2025 at 08:35, Uwe Kleine-König u.kleine-koenig@baylibre.com wrote:
From: Alexandre Mergnat amergnat@baylibre.com
commit 7df4cfef8b351fec3156160bedfc7d6d29de4cce upstream.
Conversion of dates before 1970 is still relevant today because these dates are reused on some hardwares to store dates bigger than the maximal date that is representable in the device's native format. This prominently and very soon affects the hardware covered by the rtc-mt6397 driver that can only natively store dates in the interval 1900-01-01 up to 2027-12-31. So to store the date 2028-01-01 00:00:00 to such a device, rtc_time64_to_tm() must do the right thing for time=-2208988800.
Signed-off-by: Alexandre Mergnat amergnat@baylibre.com Reviewed-by: Uwe Kleine-König u.kleine-koenig@baylibre.com Link: https://lore.kernel.org/r/20250428-enable-rtc-v4-1-2b2f7e3f9349@baylibre.com Signed-off-by: Alexandre Belloni alexandre.belloni@bootlin.com Signed-off-by: Uwe Kleine-König u.kleine-koenig@baylibre.com
drivers/rtc/lib.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/rtc/lib.c b/drivers/rtc/lib.c index fe361652727a..13b5b1f20465 100644 --- a/drivers/rtc/lib.c +++ b/drivers/rtc/lib.c @@ -46,24 +46,38 @@ EXPORT_SYMBOL(rtc_year_days);
- rtc_time64_to_tm - converts time64_t to rtc_time.
- @time: The number of seconds since 01-01-1970 00:00:00.
(Must be positive.)
*/
Works for values since at least 1900- @tm: Pointer to the struct rtc_time.
void rtc_time64_to_tm(time64_t time, struct rtc_time *tm) {
unsigned int secs;int days;
int days, secs; u64 u64tmp; u32 u32tmp, udays, century, day_of_century, year_of_century, year, day_of_year, month, day; bool is_Jan_or_Feb, is_leap_year;
/* time must be positive */
/** Get days and seconds while preserving the sign to* handle negative time values (dates before 1970-01-01)*/ days = div_s64_rem(time, 86400, &secs);/** We need 0 <= secs < 86400 which isn't given for negative* values of time. Fixup accordingly.*/if (secs < 0) {days -= 1;secs += 86400;}/* day of the week, 1970-01-01 was a Thursday */ tm->tm_wday = (days + 4) % 7;/* Ensure tm_wday is always positive */if (tm->tm_wday < 0)tm->tm_wday += 7; /* * The following algorithm is, basically, Proposition 6.3 of Neri@@ -93,7 +107,7 @@ void rtc_time64_to_tm(time64_t time, struct rtc_time *tm) * thus, is slightly different from [1]. */
udays = ((u32) days) + 719468;
udays = days + 719468; u32tmp = 4 * udays + 3; century = u32tmp / 146097;-- 2.49.0
linux-stable-mirror@lists.linaro.org
-
Alexandre Belloni -
Cassio Neri