Re: [PATCH v1 2/3] rtc: mt6359: Add RTC hardware range and add support for start-year

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 24/09/24 06:08, Macpaul Lin ha scritto:
> 
> On 9/23/24 18:00, AngeloGioacchino Del Regno wrote:
>> Add the RTC hardware range parameters to enable the possibility
>> of using the `start-year` devicetree property which, if present,
>> will set the start_secs parameter by overriding the defaults
>> that this driver is setting;
>>
>> To keep compatibility with (hence have the same date/time reading
>> as) the old behavior, set:
>>   - range_min to 1900-01-01 00:00:00
>>   - range_max to 2027-12-31 23:59:59 (HW year max range is 0-127)
>>   - start_secs defaulting to 1968-01-02 00:00:00
>>
>> Please note that the oddness of starting from January 2nd is not
>> a hardware quirk and it's done only to get the same date/time
>> reading as an RTC which time was set before this commit.
>>
>> Also remove the RTC_MIN_YEAR_OFFSET addition and subtraction in
>> callbacks set_time() and read_time() respectively, as now this
>> is already done by the API.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/rtc/rtc-mt6397.c | 13 ++++---------
>>   1 file changed, 4 insertions(+), 9 deletions(-)
> 
> [snip]
> 
> Thanks for helping add new patch fix for RTC.
> 
>> @@ -302,6 +293,10 @@ static int mtk_rtc_probe(struct platform_device *pdev)
>>       device_init_wakeup(&pdev->dev, 1);
>>       rtc->rtc_dev->ops = &mtk_rtc_ops;
>> +    rtc->rtc_dev->range_min = RTC_TIMESTAMP_BEGIN_1900;
>> +    rtc->rtc_dev->range_max = mktime64(2027, 12, 31, 23, 59, 59);
>> +    rtc->rtc_dev->start_secs = mktime64(1968, 1, 2, 0, 0, 0);
>> +    rtc->rtc_dev->set_start_time = true;
>>       return devm_rtc_register_device(rtc->rtc_dev);
>>   }
> 
> Dear @Zhanhan, Please help to leave comment if you think there is something need to 
> be clarify. For example, I've found some relate origin defines
> in "include/linux/mfd/mt6397/rtc.h"
> #define RTC_MIN_YEAR    1968
> #define RTC_BASE_YEAR    1900
> #define RTC_NUM_YEAR    128
> #define RTC_MIN_YEAR_OFFSET    (RTC_MIN_YEAR - RTC_BASE_YEAR)
> 
> Should MediaTek remove RTC_MIN_YEAR and RTC_BASE_YEAR in next patch?
> And since there may not exist any smartphone/tablet/TV using mt6397
> RTC earlier than 2010? Is it possible to change
> RTC_TIMESTAMP_BEGIN_1900 to RTC_TIMESTAMP_BEGIN_2000 without breaking
> compatibility for these devices?

Adding some context:

Being clear, the RTC timestamp starting at year 1900 doesn't mean that the
last achievable date is 2027, as the time is anyway shifted by 68 years in
this case - so, by default, the year range goes from 1968 to 2095 (unless
overridden by the start-year property, of course).

This means that setting the RTC to start at a different year doesn't really
give any measurable improvement, as that would avoid just one addition and
one subtraction operation at read and write respectively, which is something
that happens "rarely in a boot life" (RTC usually gets read once at kernel
boot, and written every time you change the date/time, which normally happens
once per day, if not even less...!).

Cheers,
Angelo