[PATCH 1/4] rtc: digicolor: fix possible race condition

[PATCH 1/4] rtc: digicolor: fix possible race condition

[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

[PATCH 1/4] rtc: digicolor: fix possible race condition

[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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/4] rtc: digicolor: set range

[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

[PATCH 2/4] rtc: digicolor: set range

[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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] rtc: digicolor: set range

[Baruch Siach]

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 -

Re: [PATCH 2/4] rtc: digicolor: set range

[Baruch Siach]

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 -

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] rtc: digicolor: set range

[Alexandre Belloni]

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

Re: [PATCH 2/4] rtc: digicolor: set range

[Alexandre Belloni]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] rtc: digicolor: set range

[Baruch Siach]

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 -

Re: [PATCH 2/4] rtc: digicolor: set range

[Baruch Siach]

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 -

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] rtc: digicolor: set range

[Alexandre Belloni]

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

Re: [PATCH 2/4] rtc: digicolor: set range

[Alexandre Belloni]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] rtc: digicolor: set range

[Baruch Siach]

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 -

Re: [PATCH 2/4] rtc: digicolor: set range

[Baruch Siach]

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 -

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] rtc: digicolor: set range

[Alexandre Belloni]

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

Re: [PATCH 2/4] rtc: digicolor: set range

[Alexandre Belloni]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/4] rtc: digicolor: use .set_time

[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

[PATCH 3/4] rtc: digicolor: use .set_time

[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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/4] rtc: digicolor: use .set_time

[Baruch Siach]

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 -

Re: [PATCH 3/4] rtc: digicolor: use .set_time

[Baruch Siach]

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 -

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/4] rtc: digicolor: convert to SPDX identifier

[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

[PATCH 4/4] rtc: digicolor: convert to SPDX identifier

[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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] rtc: digicolor: convert to SPDX identifier

[Baruch Siach]

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 -

Re: [PATCH 4/4] rtc: digicolor: convert to SPDX identifier

[Baruch Siach]

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 -

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] rtc: digicolor: fix possible race condition

[Baruch Siach]

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 -

Re: [PATCH 1/4] rtc: digicolor: fix possible race condition

[Baruch Siach]

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 -

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel