diff mbox series

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

Message ID 20240923100010.97470-3-angelogioacchino.delregno@collabora.com (mailing list archive)
State New
Headers show
Series rtc: mt6359: Cleanup and support start-year property | expand

Commit Message

AngeloGioacchino Del Regno Sept. 23, 2024, 10 a.m. UTC
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(-)

Comments

Macpaul Lin Sept. 24, 2024, 4:08 a.m. UTC | #1
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?

Thanks
Macpaul Lin
Macpaul Lin Sept. 24, 2024, 7:05 a.m. UTC | #2
On 9/24/24 12:08, Macpaul Lin wrote:
> 
> 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?
> 
> Thanks
> Macpaul Lin
> 

After discussing these change with ZhanZhan, MediaTek think use 
RTC_TIMESTAMP_BEGIN_1900 and the other changes are okay.

Reviewed-by: Macpaul Lin <macpaul.lin@mediatek.com>
Reviewed-by: ZhanZhan Ge <zhanzhan.ge@mediatek.com>

Thanks!
Macpaul Lin
AngeloGioacchino Del Regno Sept. 24, 2024, 9:28 a.m. UTC | #3
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
AngeloGioacchino Del Regno Sept. 24, 2024, 9:30 a.m. UTC | #4
Il 24/09/24 09:05, Macpaul Lin ha scritto:
> 
> On 9/24/24 12:08, Macpaul Lin wrote:
>>
>> 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?
>>
>> Thanks
>> Macpaul Lin
>>
> 
> After discussing these change with ZhanZhan, MediaTek think use 
> RTC_TIMESTAMP_BEGIN_1900 and the other changes are okay.
> 
> Reviewed-by: Macpaul Lin <macpaul.lin@mediatek.com>
> Reviewed-by: ZhanZhan Ge <zhanzhan.ge@mediatek.com>
> 

Thank you Macpaul, ZhanZhan :-)

Cheers,
Angelo
diff mbox series

Patch

diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index 1617063669cc..4785af123a7f 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -96,12 +96,6 @@  static int mtk_rtc_read_time(struct device *dev, struct rtc_time *tm)
 			goto exit;
 	} while (sec < tm->tm_sec);
 
-	/* HW register use 7 bits to store year data, minus
-	 * RTC_MIN_YEAR_OFFSET before write year data to register, and plus
-	 * RTC_MIN_YEAR_OFFSET back after read year from register
-	 */
-	tm->tm_year += RTC_MIN_YEAR_OFFSET;
-
 	/* HW register start mon from one, but tm_mon start from zero. */
 	tm->tm_mon--;
 	time = rtc_tm_to_time64(tm);
@@ -122,7 +116,6 @@  static int mtk_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	int ret;
 	u16 data[RTC_OFFSET_COUNT];
 
-	tm->tm_year -= RTC_MIN_YEAR_OFFSET;
 	tm->tm_mon++;
 
 	data[RTC_OFFSET_SEC] = tm->tm_sec;
@@ -178,7 +171,6 @@  static int mtk_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
 	tm->tm_mon = data[RTC_OFFSET_MTH] & RTC_AL_MTH_MASK;
 	tm->tm_year = data[RTC_OFFSET_YEAR] & RTC_AL_YEA_MASK;
 
-	tm->tm_year += RTC_MIN_YEAR_OFFSET;
 	tm->tm_mon--;
 
 	return 0;
@@ -194,7 +186,6 @@  static int mtk_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
 	int ret;
 	u16 data[RTC_OFFSET_COUNT];
 
-	tm->tm_year -= RTC_MIN_YEAR_OFFSET;
 	tm->tm_mon++;
 
 	mutex_lock(&rtc->lock);
@@ -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);
 }