[U-Boot] [PATCH v1 02/11] include: time.h: define time64_t

From: Heinrich Schuchardt <xypron.glpk@gmx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v1 02/11] include: time.h: define time64_t
Date: Thu, 17 Oct 2019 07:51:03 +0200	[thread overview]
Message-ID: <7419c3ef-3cab-90b4-e09e-f2b0cd16d7c8@gmx.de> (raw)
In-Reply-To: <20191017053952.GQ18778@linaro.org>

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
>>>
>>
>