Re: [RFC PATCH 3/4] rtc/lib: Provide interfaces to map between 32bit hardware and 64bit time

[pang.xunlei]

On 28 November 2014 at 07:16, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> On Thu, 27 Nov 2014, Xunlei Pang wrote:
>> +/* Rtc epoch year, can be overrided by command line */
>> +static unsigned int rtc_epoch = 1970;
>> +time64_t rtc_epoch_secs_since_1970;
>> +
>> +static int __init
>> +set_rtc_epoch(char *str)
>
> One line please
>
>> +{
>> +     unsigned int epoch;
>> +
>> +     if (kstrtou32(str, 0, &epoch) != 0) {
>> +             printk(KERN_WARNING "Invalid setup epoch %u!\n", epoch);
>
> Sure that makes a lot of sense to print epoch if the string conversion
> failed. epoch is going to be either uninitialized stack value or 0
> depending on the kstrtou32 implementation.
>
>> +     if (rtc_epoch < 1970)
>> +             printk(KERN_WARNING "Epoch %u is smaller than 1970!\n", epoch);
>
>   pr_warn() ....
>
> +__setup("rtc_epoch=", set_rtc_epoch);
>
> Not documented in Documentation/kernel-parameters.txt ...
>
>>  /*
>> + * Convert seconds since rtc epoch to seconds since Unix epoch.
>> + */
>> +time64_t rtc_hw32_to_time64(u32 hwtime)
>> +{
>> +     return (rtc_epoch_secs_since_1970 + hwtime);
>> +}
>> +EXPORT_SYMBOL_GPL(rtc_hw32_to_time64);
>
> So we need a exported function to add a global variable and a function
> argument? inline ?
>
>> +/*
>> + * Convert seconds since Unix epoch to seconds since rtc epoch.
>> + */
>> +int rtc_time64_to_hw32(time64_t time, u32 *hwtime)
>> +{
>> +     /* time must be after rtc epoch */
>> +     if (time < rtc_epoch_secs_since_1970)
>> +             return -EINVAL;
>> +
>> +     /* rtc epoch may be too small? */
>> +     if (time - rtc_epoch_secs_since_1970 > UINT_MAX)
>> +             return -EOVERFLOW;
>
> And what's the caller supposed to do with that information?

This probably means the current rtc hardware has pushed its envelope now,
the users should rely on the rtc_epoch command line to continue use it.

>
>> +     *hwtime = time - rtc_epoch_secs_since_1970;
>> +
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(rtc_time64_to_hw32);
>
> How is that going to be useful w/o a mechanism to adjust the epoch
> offset at runtime by any means?

If the rtc epoch changes at runtime, we'll not get the right time once
the system is rebooting.
So, it can only be altered throught command line, and must stay
invariable ever since unless
it uses NTP or something.

Thanks,
Xunlei

>
> Thanks,
>
>         tglx
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/