Re: [U-Boot] [PATCH v1 02/11] include: time.h: define time64_t
On 10/17/19 7:39 AM, AKASHI Takahiro wrote:
On Sat, Oct 12, 2019 at 01:40:32PM +0200, Heinrich Schuchardt wrote:
On 10/11/19 9:41 AM, AKASHI Takahiro wrote:
Adding time64_t definition will help improve portability of linux kernel
code (in my case, lib/crypto/x509_cert_parser.c).
Signed-off-by: AKASHI Takahiro
---
include/linux/time.h | 24
1 file changed, 24 insertions(+)
diff --git a/include/linux/time.h b/include/linux/time.h
index b8d298eb4d68..6186985856d7 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -150,4 +150,28 @@ _DEFUN (ctime_r, (tim_p, result),
return asctime_r (localtime_r (tim_p, ), result);
}
+/* from /kernel/time/time.c */
+typedef __s64 time64_t;
Wouldn't we want to put these definitions into a file
include/linux/time64.h?
Obviously, there is no problem at following your suggestion, but
I hesitate to do so as it adds only one line header file.
Moreover, mktime64(), which is the only reason for this patch,
is also declared in "linux/time.h" even in linux code.
# Please note that "linux/time64.h" is mainly for timespec64 helper
functions, which are never used in U-Boot.
So I'd like to leave as it is. I think that we can re-visit this issue
in the future when other definitions in time64.h are required.
+
+inline time64_t mktime64(const unsigned int year0, const unsigned int mon0,
+const unsigned int day, const unsigned int hour,
+const unsigned int min, const unsigned int sec)
Where is the function description?
The Linux kernel does not make this function an inline function. Why
should we inline it in U-Boot?
+{
+ unsigned int mon = mon0, year = year0;
+
+ /* 1..12 -> 11,12,1..10 */
+ mon -= 2;
+ if (0 >= (int)mon) {
+ mon += 12; /* Puts Feb last since it has leap day */
+ year -= 1;
+ }
+
+ return time64_t)
+ (year / 4 - year / 100 + year / 400 + 367 * mon / 12 + day) +
+ year * 365 - 719499
+ ) * 24 + hour /* now have hours - midnight tomorrow handled here */
+ ) * 60 + min /* now have minutes */
+ ) * 60 + sec; /* finally seconds */
This code is duplicate to rtc_mktime().
Duplication could be avoided by letting rtc_mktime() call mktime64().
Okay, but as an inline function in this case.
Inline use in two places adds more bytes to the binary. U-Boot should
stay as small as possible.
Best regards
Heinrich
In addition, drivers/rtc/date.c will be moved to lib/ for general use
with CONFIG_LIB_DATE.
Thanks,
-Takahiro Akashi
Best regards
Heinrich
+}
+
#endif
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 02/11] include: time.h: define time64_t
On Sat, Oct 12, 2019 at 01:40:32PM +0200, Heinrich Schuchardt wrote:
> On 10/11/19 9:41 AM, AKASHI Takahiro wrote:
> >Adding time64_t definition will help improve portability of linux kernel
> >code (in my case, lib/crypto/x509_cert_parser.c).
> >
> >Signed-off-by: AKASHI Takahiro
> >---
> > include/linux/time.h | 24
> > 1 file changed, 24 insertions(+)
> >
> >diff --git a/include/linux/time.h b/include/linux/time.h
> >index b8d298eb4d68..6186985856d7 100644
> >--- a/include/linux/time.h
> >+++ b/include/linux/time.h
> >@@ -150,4 +150,28 @@ _DEFUN (ctime_r, (tim_p, result),
> > return asctime_r (localtime_r (tim_p, ), result);
> > }
> >
> >+/* from /kernel/time/time.c */
> >+typedef __s64 time64_t;
>
> Wouldn't we want to put these definitions into a file
> include/linux/time64.h?
Obviously, there is no problem at following your suggestion, but
I hesitate to do so as it adds only one line header file.
Moreover, mktime64(), which is the only reason for this patch,
is also declared in "linux/time.h" even in linux code.
# Please note that "linux/time64.h" is mainly for timespec64 helper
functions, which are never used in U-Boot.
So I'd like to leave as it is. I think that we can re-visit this issue
in the future when other definitions in time64.h are required.
> >+
> >+inline time64_t mktime64(const unsigned int year0, const unsigned int mon0,
> >+ const unsigned int day, const unsigned int hour,
> >+ const unsigned int min, const unsigned int sec)
>
>
> Where is the function description?
>
> The Linux kernel does not make this function an inline function. Why
> should we inline it in U-Boot?
>
> >+{
> >+unsigned int mon = mon0, year = year0;
> >+
> >+/* 1..12 -> 11,12,1..10 */
> >+mon -= 2;
> >+if (0 >= (int)mon) {
> >+mon += 12; /* Puts Feb last since it has leap day */
> >+year -= 1;
> >+}
> >+
> >+return time64_t)
> >+ (year / 4 - year / 100 + year / 400 + 367 * mon / 12 + day) +
> >+ year * 365 - 719499
> >+ ) * 24 + hour /* now have hours - midnight tomorrow handled here */
> >+ ) * 60 + min /* now have minutes */
> >+) * 60 + sec; /* finally seconds */
>
> This code is duplicate to rtc_mktime().
>
> Duplication could be avoided by letting rtc_mktime() call mktime64().
Okay, but as an inline function in this case.
In addition, drivers/rtc/date.c will be moved to lib/ for general use
with CONFIG_LIB_DATE.
Thanks,
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> >+}
> >+
> > #endif
> >
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 02/11] include: time.h: define time64_t
On 10/11/19 9:41 AM, AKASHI Takahiro wrote:
Adding time64_t definition will help improve portability of linux kernel
code (in my case, lib/crypto/x509_cert_parser.c).
Signed-off-by: AKASHI Takahiro
---
include/linux/time.h | 24
1 file changed, 24 insertions(+)
diff --git a/include/linux/time.h b/include/linux/time.h
index b8d298eb4d68..6186985856d7 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -150,4 +150,28 @@ _DEFUN (ctime_r, (tim_p, result),
return asctime_r (localtime_r (tim_p, ), result);
}
+/* from /kernel/time/time.c */
+typedef __s64 time64_t;
Wouldn't we want to put these definitions into a file
include/linux/time64.h?
+
+inline time64_t mktime64(const unsigned int year0, const unsigned int mon0,
+const unsigned int day, const unsigned int hour,
+const unsigned int min, const unsigned int sec)
Where is the function description?
The Linux kernel does not make this function an inline function. Why
should we inline it in U-Boot?
+{
+ unsigned int mon = mon0, year = year0;
+
+ /* 1..12 -> 11,12,1..10 */
+ mon -= 2;
+ if (0 >= (int)mon) {
+ mon += 12; /* Puts Feb last since it has leap day */
+ year -= 1;
+ }
+
+ return time64_t)
+ (year / 4 - year / 100 + year / 400 + 367 * mon / 12 + day) +
+ year * 365 - 719499
+ ) * 24 + hour /* now have hours - midnight tomorrow handled here */
+ ) * 60 + min /* now have minutes */
+ ) * 60 + sec; /* finally seconds */
This code is duplicate to rtc_mktime().
Duplication could be avoided by letting rtc_mktime() call mktime64().
Best regards
Heinrich
+}
+
#endif
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v1 02/11] include: time.h: define time64_t
Adding time64_t definition will help improve portability of linux kernel
code (in my case, lib/crypto/x509_cert_parser.c).
Signed-off-by: AKASHI Takahiro
---
include/linux/time.h | 24
1 file changed, 24 insertions(+)
diff --git a/include/linux/time.h b/include/linux/time.h
index b8d298eb4d68..6186985856d7 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -150,4 +150,28 @@ _DEFUN (ctime_r, (tim_p, result),
return asctime_r (localtime_r (tim_p, ), result);
}
+/* from /kernel/time/time.c */
+typedef __s64 time64_t;
+
+inline time64_t mktime64(const unsigned int year0, const unsigned int mon0,
+const unsigned int day, const unsigned int hour,
+const unsigned int min, const unsigned int sec)
+{
+ unsigned int mon = mon0, year = year0;
+
+ /* 1..12 -> 11,12,1..10 */
+ mon -= 2;
+ if (0 >= (int)mon) {
+ mon += 12; /* Puts Feb last since it has leap day */
+ year -= 1;
+ }
+
+ return time64_t)
+ (year / 4 - year / 100 + year / 400 + 367 * mon / 12 + day) +
+ year * 365 - 719499
+ ) * 24 + hour /* now have hours - midnight tomorrow handled here */
+ ) * 60 + min /* now have minutes */
+ ) * 60 + sec; /* finally seconds */
+}
+
#endif
--
2.21.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
