| Message ID | 20250109-enable-rtc-v3-4-f003e8144419@baylibre.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enable RTC for the MT6357 | expand |
On 11/04/2025 14:35:57+0200, Alexandre Mergnat wrote: > The start time parameters is currently hardcoded to the driver, but > it may not fit with all equivalent RTC that driver is able to support. > > Remove the start_secs and set_start_time value setup because it > will be handled by the rtc_device_get_offset function using the > start-year DTS property. > > Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> > --- > drivers/rtc/rtc-mt6397.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c > index 692c00ff544b2..d47626d47602f 100644 > --- a/drivers/rtc/rtc-mt6397.c > +++ b/drivers/rtc/rtc-mt6397.c > @@ -291,8 +291,6 @@ static int mtk_rtc_probe(struct platform_device *pdev) > 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; > This is going to break the time for people upgrading their kernel, you are unfortunately stuck with this. > return devm_rtc_register_device(rtc->rtc_dev); > } > > -- > 2.25.1 >
On 11/04/2025 15:36:12+0200, Alexandre Belloni wrote: > On 11/04/2025 14:35:57+0200, Alexandre Mergnat wrote: > > The start time parameters is currently hardcoded to the driver, but > > it may not fit with all equivalent RTC that driver is able to support. > > > > Remove the start_secs and set_start_time value setup because it > > will be handled by the rtc_device_get_offset function using the > > start-year DTS property. > > > > Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> > > --- > > drivers/rtc/rtc-mt6397.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c > > index 692c00ff544b2..d47626d47602f 100644 > > --- a/drivers/rtc/rtc-mt6397.c > > +++ b/drivers/rtc/rtc-mt6397.c > > @@ -291,8 +291,6 @@ static int mtk_rtc_probe(struct platform_device *pdev) > > 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; > > > > This is going to break the time for people upgrading their kernel, you > are unfortunately stuck with this. > To be clear, the breakage will happen when upgrading the kernel but not the device tree with 5/5 > > return devm_rtc_register_device(rtc->rtc_dev); > > } > > > > -- > > 2.25.1 > > > > -- > Alexandre Belloni, co-owner and COO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
Il 11/04/25 15:39, Alexandre Belloni ha scritto: > On 11/04/2025 15:36:12+0200, Alexandre Belloni wrote: >> On 11/04/2025 14:35:57+0200, Alexandre Mergnat wrote: >>> The start time parameters is currently hardcoded to the driver, but >>> it may not fit with all equivalent RTC that driver is able to support. >>> >>> Remove the start_secs and set_start_time value setup because it >>> will be handled by the rtc_device_get_offset function using the >>> start-year DTS property. >>> >>> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> >>> --- >>> drivers/rtc/rtc-mt6397.c | 2 -- >>> 1 file changed, 2 deletions(-) >>> >>> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c >>> index 692c00ff544b2..d47626d47602f 100644 >>> --- a/drivers/rtc/rtc-mt6397.c >>> +++ b/drivers/rtc/rtc-mt6397.c >>> @@ -291,8 +291,6 @@ static int mtk_rtc_probe(struct platform_device *pdev) >>> 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; >>> >> >> This is going to break the time for people upgrading their kernel, you >> are unfortunately stuck with this. >> > > To be clear, the breakage will happen when upgrading the kernel but not > the device tree with 5/5 > Yes, you're stuck with this. Devicetree has to be retrocompatible. Besides, this start_secs is what gets used by default, and the start-year devicetree property should take precedence and effectively override the start_secs default. Just keep it there.... :-) Cheers, Angelo >>> return devm_rtc_register_device(rtc->rtc_dev); >>> } >>> >>> -- >>> 2.25.1 >>> >> >> -- >> Alexandre Belloni, co-owner and COO, Bootlin >> Embedded Linux and Kernel engineering >> https://bootlin.com >
On 14/04/2025 13:09, AngeloGioacchino Del Regno wrote: > Il 11/04/25 15:39, Alexandre Belloni ha scritto: >> On 11/04/2025 15:36:12+0200, Alexandre Belloni wrote: >>> On 11/04/2025 14:35:57+0200, Alexandre Mergnat wrote: >>>> The start time parameters is currently hardcoded to the driver, but >>>> it may not fit with all equivalent RTC that driver is able to support. >>>> >>>> Remove the start_secs and set_start_time value setup because it >>>> will be handled by the rtc_device_get_offset function using the >>>> start-year DTS property. >>>> >>>> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> >>>> --- >>>> drivers/rtc/rtc-mt6397.c | 2 -- >>>> 1 file changed, 2 deletions(-) >>>> >>>> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c >>>> index 692c00ff544b2..d47626d47602f 100644 >>>> --- a/drivers/rtc/rtc-mt6397.c >>>> +++ b/drivers/rtc/rtc-mt6397.c >>>> @@ -291,8 +291,6 @@ static int mtk_rtc_probe(struct platform_device *pdev) >>>> 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; >>> >>> This is going to break the time for people upgrading their kernel, you >>> are unfortunately stuck with this. >>> >> >> To be clear, the breakage will happen when upgrading the kernel but not >> the device tree with 5/5 >> > > Yes, you're stuck with this. Devicetree has to be retrocompatible. > > Besides, this start_secs is what gets used by default, and the start-year > devicetree property should take precedence and effectively override the > start_secs default. > > Just keep it there.... :-) When you boot your board for the first time, is the date January 2nd 1968 ? If not, that mean it is used as a finetune offset year. IMHO, mktime64(1968, 1, 2, 0, 0, 0) is a workaround for the rtc framework issue we try to solve in this serie because start_secs is negative (1968 < 1970). Now framework handle the negative value properly, even if you keep mktime64(1968, 1, 2, 0, 0, 0) , the device time will change. I prefer to notify you. :) TBH, it's hard to follow the logic, so I've a question: If I push in my V4 a framework fix that drivers using year < 1970 will need to have a new start_secs or start-year value to stay aligned with there previous value, do you will accept it ? Because drivers implementation are based on a bugged framework, so it's impossible, IMHO, to fix the framework without impacting date values. If you don't want to touch framework, then consider offset year in the drivers to reach above 1970 :) If you have a solution to keep "rtc->rtc_dev->start_secs = mktime64(1968, 1, 2, 0, 0, 0);" without having the date changed, don't hesitate to tell me, I'm not forcing for a specific one, just tell me what you prefer for the V4. Regards, Alex
diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c index 692c00ff544b2..d47626d47602f 100644 --- a/drivers/rtc/rtc-mt6397.c +++ b/drivers/rtc/rtc-mt6397.c @@ -291,8 +291,6 @@ static int mtk_rtc_probe(struct platform_device *pdev) 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); }
The start time parameters is currently hardcoded to the driver, but it may not fit with all equivalent RTC that driver is able to support. Remove the start_secs and set_start_time value setup because it will be handled by the rtc_device_get_offset function using the start-year DTS property. Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> --- drivers/rtc/rtc-mt6397.c | 2 -- 1 file changed, 2 deletions(-)