| Message ID | 20191011074200.30269-3-takahiro.akashi@linaro.org |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | import x509/pkcs7 parsers from linux | expand |
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 <takahiro.akashi@linaro.org> > --- > 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, &tm), result); > } > > +/* from <linux>/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 >
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 <takahiro.akashi@linaro.org> > >--- > > 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, &tm), result); > > } > > > >+/* from <linux>/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 > > >
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 <takahiro.akashi@linaro.org> >>> --- >>> 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, &tm), result); >>> } >>> >>> +/* from <linux>/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 >>> >> >
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, &tm), result); } +/* from <linux>/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
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 <takahiro.akashi@linaro.org> --- include/linux/time.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)