* [PATCH 1/4] rtc: digicolor: fix possible race condition
@ 2019-04-30 9:32 Alexandre Belloni
2019-04-30 9:32 ` [PATCH 2/4] rtc: digicolor: set range Alexandre Belloni
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Alexandre Belloni @ 2019-04-30 9:32 UTC (permalink / raw)
To: linux-rtc; +Cc: Baruch Siach, linux-arm-kernel, linux-kernel, Alexandre Belloni
The IRQ is requested before the struct rtc is allocated and registered, but
this struct is used in the IRQ handler. This may lead to a NULL pointer
dereference.
Switch to devm_rtc_allocate_device/rtc_register_device to allocate the rtc
struct before requesting the IRQ.
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
drivers/rtc/rtc-digicolor.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
index b253bf1b3531..5bb14c56bc9a 100644
--- a/drivers/rtc/rtc-digicolor.c
+++ b/drivers/rtc/rtc-digicolor.c
@@ -192,6 +192,10 @@ static int __init dc_rtc_probe(struct platform_device *pdev)
if (IS_ERR(rtc->regs))
return PTR_ERR(rtc->regs);
+ rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
+ if (IS_ERR(rtc->rtc_dev))
+ return PTR_ERR(rtc->rtc_dev);
+
irq = platform_get_irq(pdev, 0);
if (irq < 0)
return irq;
@@ -200,12 +204,10 @@ static int __init dc_rtc_probe(struct platform_device *pdev)
return ret;
platform_set_drvdata(pdev, rtc);
- rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, pdev->name,
- &dc_rtc_ops, THIS_MODULE);
- if (IS_ERR(rtc->rtc_dev))
- return PTR_ERR(rtc->rtc_dev);
- return 0;
+ rtc->rtc_dev->ops = &dc_rtc_ops;
+
+ return rtc_register_device(rtc->rtc_dev);
}
static const struct of_device_id dc_dt_ids[] = {
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/4] rtc: digicolor: set range
2019-04-30 9:32 [PATCH 1/4] rtc: digicolor: fix possible race condition Alexandre Belloni
@ 2019-04-30 9:32 ` Alexandre Belloni
2019-04-30 11:36 ` Baruch Siach
2019-04-30 9:32 ` [PATCH 3/4] rtc: digicolor: use .set_time Alexandre Belloni
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Alexandre Belloni @ 2019-04-30 9:32 UTC (permalink / raw)
To: linux-rtc; +Cc: Baruch Siach, linux-arm-kernel, linux-kernel, Alexandre Belloni
While the range of REFERENCE + TIME is actually 33 bits, the counter
itself (TIME) is a 32-bits seconds counter.
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
drivers/rtc/rtc-digicolor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
index 5bb14c56bc9a..e6e16aaac254 100644
--- a/drivers/rtc/rtc-digicolor.c
+++ b/drivers/rtc/rtc-digicolor.c
@@ -206,6 +206,7 @@ static int __init dc_rtc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, rtc);
rtc->rtc_dev->ops = &dc_rtc_ops;
+ rtc->rtc_dev->range_max = U32_MAX;
return rtc_register_device(rtc->rtc_dev);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/4] rtc: digicolor: use .set_time
2019-04-30 9:32 [PATCH 1/4] rtc: digicolor: fix possible race condition Alexandre Belloni
2019-04-30 9:32 ` [PATCH 2/4] rtc: digicolor: set range Alexandre Belloni
@ 2019-04-30 9:32 ` Alexandre Belloni
2019-04-30 12:21 ` Baruch Siach
2019-04-30 9:32 ` [PATCH 4/4] rtc: digicolor: convert to SPDX identifier Alexandre Belloni
2019-04-30 11:25 ` [PATCH 1/4] rtc: digicolor: fix possible race condition Baruch Siach
3 siblings, 1 reply; 13+ messages in thread
From: Alexandre Belloni @ 2019-04-30 9:32 UTC (permalink / raw)
To: linux-rtc; +Cc: Baruch Siach, linux-arm-kernel, linux-kernel, Alexandre Belloni
Use .set_time instead of the deprecated .set_mmss.
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
drivers/rtc/rtc-digicolor.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
index e6e16aaac254..ed2fc1adafd5 100644
--- a/drivers/rtc/rtc-digicolor.c
+++ b/drivers/rtc/rtc-digicolor.c
@@ -106,11 +106,11 @@ static int dc_rtc_read_time(struct device *dev, struct rtc_time *tm)
return 0;
}
-static int dc_rtc_set_mmss(struct device *dev, unsigned long secs)
+static int dc_rtc_set_time(struct device *dev, struct rtc_time *tm)
{
struct dc_rtc *rtc = dev_get_drvdata(dev);
- return dc_rtc_write(rtc, secs);
+ return dc_rtc_write(rtc, rtc_tm_to_time64(tm));
}
static int dc_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
@@ -161,7 +161,7 @@ static int dc_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
static const struct rtc_class_ops dc_rtc_ops = {
.read_time = dc_rtc_read_time,
- .set_mmss = dc_rtc_set_mmss,
+ .set_time = dc_rtc_set_time,
.read_alarm = dc_rtc_read_alarm,
.set_alarm = dc_rtc_set_alarm,
.alarm_irq_enable = dc_rtc_alarm_irq_enable,
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/4] rtc: digicolor: convert to SPDX identifier
2019-04-30 9:32 [PATCH 1/4] rtc: digicolor: fix possible race condition Alexandre Belloni
2019-04-30 9:32 ` [PATCH 2/4] rtc: digicolor: set range Alexandre Belloni
2019-04-30 9:32 ` [PATCH 3/4] rtc: digicolor: use .set_time Alexandre Belloni
@ 2019-04-30 9:32 ` Alexandre Belloni
2019-04-30 12:22 ` Baruch Siach
2019-04-30 11:25 ` [PATCH 1/4] rtc: digicolor: fix possible race condition Baruch Siach
3 siblings, 1 reply; 13+ messages in thread
From: Alexandre Belloni @ 2019-04-30 9:32 UTC (permalink / raw)
To: linux-rtc; +Cc: Baruch Siach, linux-arm-kernel, linux-kernel, Alexandre Belloni
Use SPDX-License-Identifier instead of a verbose license text.
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
drivers/rtc/rtc-digicolor.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
index ed2fc1adafd5..0aecc3f8e721 100644
--- a/drivers/rtc/rtc-digicolor.c
+++ b/drivers/rtc/rtc-digicolor.c
@@ -1,14 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0+
/*
* Real Time Clock driver for Conexant Digicolor
*
* Copyright (C) 2015 Paradox Innovation Ltd.
*
* Author: Baruch Siach <baruch@tkos.co.il>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the
- * Free Software Foundation; either version 2 of the License, or (at your
- * option) any later version.
*/
#include <linux/io.h>
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] rtc: digicolor: fix possible race condition
2019-04-30 9:32 [PATCH 1/4] rtc: digicolor: fix possible race condition Alexandre Belloni
` (2 preceding siblings ...)
2019-04-30 9:32 ` [PATCH 4/4] rtc: digicolor: convert to SPDX identifier Alexandre Belloni
@ 2019-04-30 11:25 ` Baruch Siach
3 siblings, 0 replies; 13+ messages in thread
From: Baruch Siach @ 2019-04-30 11:25 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc, linux-arm-kernel, linux-kernel
Hi Alexandre,
On Tue, Apr 30 2019, Alexandre Belloni wrote:
> The IRQ is requested before the struct rtc is allocated and registered, but
> this struct is used in the IRQ handler. This may lead to a NULL pointer
> dereference.
>
> Switch to devm_rtc_allocate_device/rtc_register_device to allocate the rtc
> struct before requesting the IRQ.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Acked-by: Baruch Siach <baruch@tkos.co.il>
baruch
> ---
> drivers/rtc/rtc-digicolor.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
> index b253bf1b3531..5bb14c56bc9a 100644
> --- a/drivers/rtc/rtc-digicolor.c
> +++ b/drivers/rtc/rtc-digicolor.c
> @@ -192,6 +192,10 @@ static int __init dc_rtc_probe(struct platform_device *pdev)
> if (IS_ERR(rtc->regs))
> return PTR_ERR(rtc->regs);
>
> + rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
> + if (IS_ERR(rtc->rtc_dev))
> + return PTR_ERR(rtc->rtc_dev);
> +
> irq = platform_get_irq(pdev, 0);
> if (irq < 0)
> return irq;
> @@ -200,12 +204,10 @@ static int __init dc_rtc_probe(struct platform_device *pdev)
> return ret;
>
> platform_set_drvdata(pdev, rtc);
> - rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, pdev->name,
> - &dc_rtc_ops, THIS_MODULE);
> - if (IS_ERR(rtc->rtc_dev))
> - return PTR_ERR(rtc->rtc_dev);
>
> - return 0;
> + rtc->rtc_dev->ops = &dc_rtc_ops;
> +
> + return rtc_register_device(rtc->rtc_dev);
> }
>
> static const struct of_device_id dc_dt_ids[] = {
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] rtc: digicolor: set range
2019-04-30 9:32 ` [PATCH 2/4] rtc: digicolor: set range Alexandre Belloni
@ 2019-04-30 11:36 ` Baruch Siach
2019-04-30 11:47 ` Alexandre Belloni
0 siblings, 1 reply; 13+ messages in thread
From: Baruch Siach @ 2019-04-30 11:36 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc, linux-kernel, linux-arm-kernel
Hi Alexandre,
On Tue, Apr 30 2019, Alexandre Belloni wrote:
> While the range of REFERENCE + TIME is actually 33 bits, the counter
> itself (TIME) is a 32-bits seconds counter.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> drivers/rtc/rtc-digicolor.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
> index 5bb14c56bc9a..e6e16aaac254 100644
> --- a/drivers/rtc/rtc-digicolor.c
> +++ b/drivers/rtc/rtc-digicolor.c
> @@ -206,6 +206,7 @@ static int __init dc_rtc_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, rtc);
>
> rtc->rtc_dev->ops = &dc_rtc_ops;
> + rtc->rtc_dev->range_max = U32_MAX;
Where can I find documentation on the meaning and usage of the range_max
value? I could not find anything in the kernel source.
baruch
>
> return rtc_register_device(rtc->rtc_dev);
> }
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] rtc: digicolor: set range
2019-04-30 11:36 ` Baruch Siach
@ 2019-04-30 11:47 ` Alexandre Belloni
2019-04-30 12:20 ` Baruch Siach
0 siblings, 1 reply; 13+ messages in thread
From: Alexandre Belloni @ 2019-04-30 11:47 UTC (permalink / raw)
To: Baruch Siach; +Cc: linux-rtc, linux-kernel, linux-arm-kernel
On 30/04/2019 14:36:24+0300, Baruch Siach wrote:
> Hi Alexandre,
>
> On Tue, Apr 30 2019, Alexandre Belloni wrote:
>
> > While the range of REFERENCE + TIME is actually 33 bits, the counter
> > itself (TIME) is a 32-bits seconds counter.
> >
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > ---
> > drivers/rtc/rtc-digicolor.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
> > index 5bb14c56bc9a..e6e16aaac254 100644
> > --- a/drivers/rtc/rtc-digicolor.c
> > +++ b/drivers/rtc/rtc-digicolor.c
> > @@ -206,6 +206,7 @@ static int __init dc_rtc_probe(struct platform_device *pdev)
> > platform_set_drvdata(pdev, rtc);
> >
> > rtc->rtc_dev->ops = &dc_rtc_ops;
> > + rtc->rtc_dev->range_max = U32_MAX;
>
> Where can I find documentation on the meaning and usage of the range_max
> value? I could not find anything in the kernel source.
>
It should be set to the maximum UNIX timestamp the RTC can be set to
while keeping range_min to range_max contiguous.
In the digicolor case, you could go up to 8589934590 (Wed Mar 16
12:56:30 UTC 2242) but the driver only writes DC_RTC_REFERENCE and I'm
not sure it can also update DC_RTC_TIME safely.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] rtc: digicolor: set range
2019-04-30 11:47 ` Alexandre Belloni
@ 2019-04-30 12:20 ` Baruch Siach
2019-04-30 13:05 ` Alexandre Belloni
0 siblings, 1 reply; 13+ messages in thread
From: Baruch Siach @ 2019-04-30 12:20 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc, linux-kernel, linux-arm-kernel
Hi Alexandre,
On Tue, Apr 30 2019, Alexandre Belloni wrote:
> On 30/04/2019 14:36:24+0300, Baruch Siach wrote:
>> Hi Alexandre,
>>
>> On Tue, Apr 30 2019, Alexandre Belloni wrote:
>>
>> > While the range of REFERENCE + TIME is actually 33 bits, the counter
>> > itself (TIME) is a 32-bits seconds counter.
>> >
>> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> > ---
>> > drivers/rtc/rtc-digicolor.c | 1 +
>> > 1 file changed, 1 insertion(+)
>> >
>> > diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
>> > index 5bb14c56bc9a..e6e16aaac254 100644
>> > --- a/drivers/rtc/rtc-digicolor.c
>> > +++ b/drivers/rtc/rtc-digicolor.c
>> > @@ -206,6 +206,7 @@ static int __init dc_rtc_probe(struct platform_device *pdev)
>> > platform_set_drvdata(pdev, rtc);
>> >
>> > rtc->rtc_dev->ops = &dc_rtc_ops;
>> > + rtc->rtc_dev->range_max = U32_MAX;
>>
>> Where can I find documentation on the meaning and usage of the range_max
>> value? I could not find anything in the kernel source.
>
> It should be set to the maximum UNIX timestamp the RTC can be set to
> while keeping range_min to range_max contiguous.
>
> In the digicolor case, you could go up to 8589934590 (Wed Mar 16
> 12:56:30 UTC 2242) but the driver only writes DC_RTC_REFERENCE and I'm
> not sure it can also update DC_RTC_TIME safely.
DC_RTC_TIME resets to zero whenever dc_rtc_write writes CMD_RESET to the
DC_RTC_CONTROL register. DC_RTC_REFERENCE keeps the value that
dc_rtc_write stores there. So the driver will return values larger than
U32_MAX if you happen to cross this point in time between dc_rtc_write
and dc_rtc_read. But you can't store a value larger than U32_MAX in
DC_RTC_REFERENCE.
Will the core RTC code handle the U32_MAX cross correctly?
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] rtc: digicolor: use .set_time
2019-04-30 9:32 ` [PATCH 3/4] rtc: digicolor: use .set_time Alexandre Belloni
@ 2019-04-30 12:21 ` Baruch Siach
0 siblings, 0 replies; 13+ messages in thread
From: Baruch Siach @ 2019-04-30 12:21 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc, linux-kernel, linux-arm-kernel
Hi Alexandre,
On Tue, Apr 30 2019, Alexandre Belloni wrote:
> Use .set_time instead of the deprecated .set_mmss.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Acked-by: Baruch Siach <baruch@tkos.co.il>
baruch
> ---
> drivers/rtc/rtc-digicolor.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
> index e6e16aaac254..ed2fc1adafd5 100644
> --- a/drivers/rtc/rtc-digicolor.c
> +++ b/drivers/rtc/rtc-digicolor.c
> @@ -106,11 +106,11 @@ static int dc_rtc_read_time(struct device *dev, struct rtc_time *tm)
> return 0;
> }
>
> -static int dc_rtc_set_mmss(struct device *dev, unsigned long secs)
> +static int dc_rtc_set_time(struct device *dev, struct rtc_time *tm)
> {
> struct dc_rtc *rtc = dev_get_drvdata(dev);
>
> - return dc_rtc_write(rtc, secs);
> + return dc_rtc_write(rtc, rtc_tm_to_time64(tm));
> }
>
> static int dc_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> @@ -161,7 +161,7 @@ static int dc_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
>
> static const struct rtc_class_ops dc_rtc_ops = {
> .read_time = dc_rtc_read_time,
> - .set_mmss = dc_rtc_set_mmss,
> + .set_time = dc_rtc_set_time,
> .read_alarm = dc_rtc_read_alarm,
> .set_alarm = dc_rtc_set_alarm,
> .alarm_irq_enable = dc_rtc_alarm_irq_enable,
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] rtc: digicolor: convert to SPDX identifier
2019-04-30 9:32 ` [PATCH 4/4] rtc: digicolor: convert to SPDX identifier Alexandre Belloni
@ 2019-04-30 12:22 ` Baruch Siach
0 siblings, 0 replies; 13+ messages in thread
From: Baruch Siach @ 2019-04-30 12:22 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc, linux-kernel, linux-arm-kernel
Hi Alexandre,
On Tue, Apr 30 2019, Alexandre Belloni wrote:
> Use SPDX-License-Identifier instead of a verbose license text.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Acked-by: Baruch Siach <baruch@tkos.co.il>
baruch
> ---
> drivers/rtc/rtc-digicolor.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
> index ed2fc1adafd5..0aecc3f8e721 100644
> --- a/drivers/rtc/rtc-digicolor.c
> +++ b/drivers/rtc/rtc-digicolor.c
> @@ -1,14 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0+
> /*
> * Real Time Clock driver for Conexant Digicolor
> *
> * Copyright (C) 2015 Paradox Innovation Ltd.
> *
> * Author: Baruch Siach <baruch@tkos.co.il>
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms of the GNU General Public License as published by the
> - * Free Software Foundation; either version 2 of the License, or (at your
> - * option) any later version.
> */
>
> #include <linux/io.h>
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] rtc: digicolor: set range
2019-04-30 12:20 ` Baruch Siach
@ 2019-04-30 13:05 ` Alexandre Belloni
2019-04-30 15:25 ` Baruch Siach
0 siblings, 1 reply; 13+ messages in thread
From: Alexandre Belloni @ 2019-04-30 13:05 UTC (permalink / raw)
To: Baruch Siach; +Cc: linux-rtc, linux-kernel, linux-arm-kernel
On 30/04/2019 15:20:08+0300, Baruch Siach wrote:
> Hi Alexandre,
>
> On Tue, Apr 30 2019, Alexandre Belloni wrote:
> > On 30/04/2019 14:36:24+0300, Baruch Siach wrote:
> >> Hi Alexandre,
> >>
> >> On Tue, Apr 30 2019, Alexandre Belloni wrote:
> >>
> >> > While the range of REFERENCE + TIME is actually 33 bits, the counter
> >> > itself (TIME) is a 32-bits seconds counter.
> >> >
> >> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> >> > ---
> >> > drivers/rtc/rtc-digicolor.c | 1 +
> >> > 1 file changed, 1 insertion(+)
> >> >
> >> > diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
> >> > index 5bb14c56bc9a..e6e16aaac254 100644
> >> > --- a/drivers/rtc/rtc-digicolor.c
> >> > +++ b/drivers/rtc/rtc-digicolor.c
> >> > @@ -206,6 +206,7 @@ static int __init dc_rtc_probe(struct platform_device *pdev)
> >> > platform_set_drvdata(pdev, rtc);
> >> >
> >> > rtc->rtc_dev->ops = &dc_rtc_ops;
> >> > + rtc->rtc_dev->range_max = U32_MAX;
> >>
> >> Where can I find documentation on the meaning and usage of the range_max
> >> value? I could not find anything in the kernel source.
> >
> > It should be set to the maximum UNIX timestamp the RTC can be set to
> > while keeping range_min to range_max contiguous.
> >
> > In the digicolor case, you could go up to 8589934590 (Wed Mar 16
> > 12:56:30 UTC 2242) but the driver only writes DC_RTC_REFERENCE and I'm
> > not sure it can also update DC_RTC_TIME safely.
>
> DC_RTC_TIME resets to zero whenever dc_rtc_write writes CMD_RESET to the
> DC_RTC_CONTROL register. DC_RTC_REFERENCE keeps the value that
> dc_rtc_write stores there. So the driver will return values larger than
> U32_MAX if you happen to cross this point in time between dc_rtc_write
> and dc_rtc_read. But you can't store a value larger than U32_MAX in
> DC_RTC_REFERENCE.
>
> Will the core RTC code handle the U32_MAX cross correctly?
>
Yes, this is ok to return a valid value that is higher than range_max.
However, at that time, you will not be able to set any alarms anymore as
the core doesn't allow to set alarms after range_max.
I would think that this is fine because this will happen in 2106 and we
have a way to offset the time (the whole goal of setting the range)
using device tree.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] rtc: digicolor: set range
2019-04-30 13:05 ` Alexandre Belloni
@ 2019-04-30 15:25 ` Baruch Siach
2019-04-30 19:10 ` Alexandre Belloni
0 siblings, 1 reply; 13+ messages in thread
From: Baruch Siach @ 2019-04-30 15:25 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc, linux-kernel, linux-arm-kernel
Hi Alexandre,
On Tue, Apr 30 2019, Alexandre Belloni wrote:
> On 30/04/2019 15:20:08+0300, Baruch Siach wrote:
>> On Tue, Apr 30 2019, Alexandre Belloni wrote:
>> > On 30/04/2019 14:36:24+0300, Baruch Siach wrote:
>> >> Hi Alexandre,
>> >>
>> >> On Tue, Apr 30 2019, Alexandre Belloni wrote:
>> >>
>> >> > While the range of REFERENCE + TIME is actually 33 bits, the counter
>> >> > itself (TIME) is a 32-bits seconds counter.
>> >> >
>> >> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> >> > ---
>> >> > drivers/rtc/rtc-digicolor.c | 1 +
>> >> > 1 file changed, 1 insertion(+)
>> >> >
>> >> > diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
>> >> > index 5bb14c56bc9a..e6e16aaac254 100644
>> >> > --- a/drivers/rtc/rtc-digicolor.c
>> >> > +++ b/drivers/rtc/rtc-digicolor.c
>> >> > @@ -206,6 +206,7 @@ static int __init dc_rtc_probe(struct platform_device *pdev)
>> >> > platform_set_drvdata(pdev, rtc);
>> >> >
>> >> > rtc->rtc_dev->ops = &dc_rtc_ops;
>> >> > + rtc->rtc_dev->range_max = U32_MAX;
>> >>
>> >> Where can I find documentation on the meaning and usage of the range_max
>> >> value? I could not find anything in the kernel source.
>> >
>> > It should be set to the maximum UNIX timestamp the RTC can be set to
>> > while keeping range_min to range_max contiguous.
>> >
>> > In the digicolor case, you could go up to 8589934590 (Wed Mar 16
>> > 12:56:30 UTC 2242) but the driver only writes DC_RTC_REFERENCE and I'm
>> > not sure it can also update DC_RTC_TIME safely.
>>
>> DC_RTC_TIME resets to zero whenever dc_rtc_write writes CMD_RESET to the
>> DC_RTC_CONTROL register. DC_RTC_REFERENCE keeps the value that
>> dc_rtc_write stores there. So the driver will return values larger than
>> U32_MAX if you happen to cross this point in time between dc_rtc_write
>> and dc_rtc_read. But you can't store a value larger than U32_MAX in
>> DC_RTC_REFERENCE.
>>
>> Will the core RTC code handle the U32_MAX cross correctly?
>
> Yes, this is ok to return a valid value that is higher than range_max.
> However, at that time, you will not be able to set any alarms anymore as
> the core doesn't allow to set alarms after range_max.
>
> I would think that this is fine because this will happen in 2106 and we
> have a way to offset the time (the whole goal of setting the range)
> using device tree.
That's the sort of documentation that I'm missing. The 'start-year'
property is mentioned in the DT binding documentation. But I don't see
where range_max is documented as a facility for the time offset feature.
Anyway,
Acked-by: Baruch Siach <baruch@tkos.co.il>
Thanks,
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] rtc: digicolor: set range
2019-04-30 15:25 ` Baruch Siach
@ 2019-04-30 19:10 ` Alexandre Belloni
0 siblings, 0 replies; 13+ messages in thread
From: Alexandre Belloni @ 2019-04-30 19:10 UTC (permalink / raw)
To: Baruch Siach; +Cc: linux-rtc, linux-kernel, linux-arm-kernel
On 30/04/2019 18:25:52+0300, Baruch Siach wrote:
> > Yes, this is ok to return a valid value that is higher than range_max.
> > However, at that time, you will not be able to set any alarms anymore as
> > the core doesn't allow to set alarms after range_max.
> >
> > I would think that this is fine because this will happen in 2106 and we
> > have a way to offset the time (the whole goal of setting the range)
> > using device tree.
>
> That's the sort of documentation that I'm missing. The 'start-year'
> property is mentioned in the DT binding documentation. But I don't see
> where range_max is documented as a facility for the time offset feature.
>
Sure, I'm planning to document better how a proper RTC driver should be
written. I needed to cleanup the digicolor driver because I4m removing
.set_mmss and .set_mmss64 this cycle.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-04-30 19:10 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-30 9:32 [PATCH 1/4] rtc: digicolor: fix possible race condition Alexandre Belloni
2019-04-30 9:32 ` [PATCH 2/4] rtc: digicolor: set range Alexandre Belloni
2019-04-30 11:36 ` Baruch Siach
2019-04-30 11:47 ` Alexandre Belloni
2019-04-30 12:20 ` Baruch Siach
2019-04-30 13:05 ` Alexandre Belloni
2019-04-30 15:25 ` Baruch Siach
2019-04-30 19:10 ` Alexandre Belloni
2019-04-30 9:32 ` [PATCH 3/4] rtc: digicolor: use .set_time Alexandre Belloni
2019-04-30 12:21 ` Baruch Siach
2019-04-30 9:32 ` [PATCH 4/4] rtc: digicolor: convert to SPDX identifier Alexandre Belloni
2019-04-30 12:22 ` Baruch Siach
2019-04-30 11:25 ` [PATCH 1/4] rtc: digicolor: fix possible race condition Baruch Siach
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).