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 <[email protected]>
---
drivers/rtc/rtc-xgene.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/rtc/rtc-xgene.c b/drivers/rtc/rtc-xgene.c
index 153820876a82..2f741f455c30 100644
--- a/drivers/rtc/rtc-xgene.c
+++ b/drivers/rtc/rtc-xgene.c
@@ -168,6 +168,10 @@ static int xgene_rtc_probe(struct platform_device *pdev)
if (IS_ERR(pdata->csr_base))
return PTR_ERR(pdata->csr_base);
+ pdata->rtc = devm_rtc_allocate_device(&pdev->dev);
+ if (IS_ERR(pdata->rtc))
+ return PTR_ERR(pdata->rtc);
+
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(&pdev->dev, "No IRQ resource\n");
@@ -198,15 +202,15 @@ static int xgene_rtc_probe(struct platform_device *pdev)
return ret;
}
- pdata->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
- &xgene_rtc_ops, THIS_MODULE);
- if (IS_ERR(pdata->rtc)) {
- clk_disable_unprepare(pdata->clk);
- return PTR_ERR(pdata->rtc);
- }
-
/* HW does not support update faster than 1 seconds */
pdata->rtc->uie_unsupported = 1;
+ pdata->rtc->ops = &xgene_rtc_ops;
+
+ ret = rtc_register_device(pdata->rtc);
+ if (ret) {
+ clk_disable_unprepare(pdata->clk);
+ return ret;
+ }
return 0;
}
--
2.20.1
Use SPDX-License-Identifier instead of a verbose license text.
Signed-off-by: Alexandre Belloni <[email protected]>
---
drivers/rtc/rtc-xgene.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/rtc/rtc-xgene.c b/drivers/rtc/rtc-xgene.c
index e360f8917556..ba9121d02f02 100644
--- a/drivers/rtc/rtc-xgene.c
+++ b/drivers/rtc/rtc-xgene.c
@@ -1,23 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0+
/*
* APM X-Gene SoC Real Time Clock Driver
*
* Copyright (c) 2014, Applied Micro Circuits Corporation
* Author: Rameshwar Prasad Sahu <[email protected]>
* Loc Ho <[email protected]>
- *
- * 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.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- *
*/
#include <linux/init.h>
--
2.20.1
There is no point in caching alarm_time for .read_alarm because
.read_alarm is only called at boo time and thus alarm_time is always 0.
Signed-off-by: Alexandre Belloni <[email protected]>
---
drivers/rtc/rtc-xgene.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/rtc/rtc-xgene.c b/drivers/rtc/rtc-xgene.c
index eb745deda936..6f7d7648a9bd 100644
--- a/drivers/rtc/rtc-xgene.c
+++ b/drivers/rtc/rtc-xgene.c
@@ -35,7 +35,6 @@
struct xgene_rtc_dev {
struct rtc_device *rtc;
struct device *dev;
- unsigned long alarm_time;
void __iomem *csr_base;
struct clk *clk;
unsigned int irq_wake;
@@ -68,7 +67,8 @@ static int xgene_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
{
struct xgene_rtc_dev *pdata = dev_get_drvdata(dev);
- rtc_time_to_tm(pdata->alarm_time, &alrm->time);
+ /* If possible, CMR should be read here */
+ rtc_time_to_tm(0, &alrm->time);
alrm->enabled = readl(pdata->csr_base + RTC_CCR) & RTC_CCR_IE;
return 0;
@@ -105,8 +105,7 @@ static int xgene_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
unsigned long alarm_time;
rtc_tm_to_time(&alrm->time, &alarm_time);
- pdata->alarm_time = alarm_time;
- writel((u32)pdata->alarm_time, pdata->csr_base + RTC_CMR);
+ writel((u32)alarm_time, pdata->csr_base + RTC_CMR);
xgene_rtc_alarm_irq_enable(dev, alrm->enabled);
--
2.20.1
Correct trivial whitespace issues. Also sort the headers.
Signed-off-by: Alexandre Belloni <[email protected]>
---
drivers/rtc/rtc-xgene.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/rtc/rtc-xgene.c b/drivers/rtc/rtc-xgene.c
index ba9121d02f02..eb745deda936 100644
--- a/drivers/rtc/rtc-xgene.c
+++ b/drivers/rtc/rtc-xgene.c
@@ -7,15 +7,15 @@
* Loc Ho <[email protected]>
*/
+#include <linux/clk.h>
+#include <linux/delay.h>
#include <linux/init.h>
+#include <linux/io.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
-#include <linux/io.h>
-#include <linux/slab.h>
-#include <linux/clk.h>
-#include <linux/delay.h>
#include <linux/rtc.h>
+#include <linux/slab.h>
/* RTC CSR Registers */
#define RTC_CCVR 0x00
@@ -58,7 +58,7 @@ static int xgene_rtc_set_mmss(struct device *dev, unsigned long secs)
* NOTE: After the following write, the RTC_CCVR is only reflected
* after the update cycle of 1 seconds.
*/
- writel((u32) secs, pdata->csr_base + RTC_CLR);
+ writel((u32)secs, pdata->csr_base + RTC_CLR);
readl(pdata->csr_base + RTC_CLR); /* Force a barrier */
return 0;
@@ -106,7 +106,7 @@ static int xgene_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
rtc_tm_to_time(&alrm->time, &alarm_time);
pdata->alarm_time = alarm_time;
- writel((u32) pdata->alarm_time, pdata->csr_base + RTC_CMR);
+ writel((u32)pdata->alarm_time, pdata->csr_base + RTC_CMR);
xgene_rtc_alarm_irq_enable(dev, alrm->enabled);
@@ -123,7 +123,7 @@ static const struct rtc_class_ops xgene_rtc_ops = {
static irqreturn_t xgene_rtc_interrupt(int irq, void *id)
{
- struct xgene_rtc_dev *pdata = (struct xgene_rtc_dev *) id;
+ struct xgene_rtc_dev *pdata = id;
/* Check if interrupt asserted */
if (!(readl(pdata->csr_base + RTC_STAT) & RTC_STAT_BIT))
--
2.20.1
Use .set_time instead of the deprecated .set_mmss.
Signed-off-by: Alexandre Belloni <[email protected]>
---
drivers/rtc/rtc-xgene.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/rtc/rtc-xgene.c b/drivers/rtc/rtc-xgene.c
index aef338428668..9888383f0088 100644
--- a/drivers/rtc/rtc-xgene.c
+++ b/drivers/rtc/rtc-xgene.c
@@ -49,7 +49,7 @@ static int xgene_rtc_read_time(struct device *dev, struct rtc_time *tm)
return 0;
}
-static int xgene_rtc_set_mmss(struct device *dev, unsigned long secs)
+static int xgene_rtc_set_time(struct device *dev, struct rtc_time *tm)
{
struct xgene_rtc_dev *pdata = dev_get_drvdata(dev);
@@ -57,7 +57,7 @@ static int xgene_rtc_set_mmss(struct device *dev, unsigned long secs)
* NOTE: After the following write, the RTC_CCVR is only reflected
* after the update cycle of 1 seconds.
*/
- writel((u32)secs, pdata->csr_base + RTC_CLR);
+ writel((u32)rtc_tm_to_time64(tm), pdata->csr_base + RTC_CLR);
readl(pdata->csr_base + RTC_CLR); /* Force a barrier */
return 0;
@@ -112,7 +112,7 @@ static int xgene_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
static const struct rtc_class_ops xgene_rtc_ops = {
.read_time = xgene_rtc_read_time,
- .set_mmss = xgene_rtc_set_mmss,
+ .set_time = xgene_rtc_set_time,
.read_alarm = xgene_rtc_read_alarm,
.set_alarm = xgene_rtc_set_alarm,
.alarm_irq_enable = xgene_rtc_alarm_irq_enable,
--
2.20.1
CCVR is a 32bit second counter.
Signed-off-by: Alexandre Belloni <[email protected]>
---
drivers/rtc/rtc-xgene.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/rtc/rtc-xgene.c b/drivers/rtc/rtc-xgene.c
index 2f741f455c30..e360f8917556 100644
--- a/drivers/rtc/rtc-xgene.c
+++ b/drivers/rtc/rtc-xgene.c
@@ -205,6 +205,7 @@ static int xgene_rtc_probe(struct platform_device *pdev)
/* HW does not support update faster than 1 seconds */
pdata->rtc->uie_unsupported = 1;
pdata->rtc->ops = &xgene_rtc_ops;
+ pdata->rtc->range_max = U32_MAX;
ret = rtc_register_device(pdata->rtc);
if (ret) {
--
2.20.1
Call the 64bit versions of rtc_tm time conversion as the range is enforced
by the core.
Signed-off-by: Alexandre Belloni <[email protected]>
---
drivers/rtc/rtc-xgene.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/rtc/rtc-xgene.c b/drivers/rtc/rtc-xgene.c
index 6f7d7648a9bd..aef338428668 100644
--- a/drivers/rtc/rtc-xgene.c
+++ b/drivers/rtc/rtc-xgene.c
@@ -45,7 +45,7 @@ static int xgene_rtc_read_time(struct device *dev, struct rtc_time *tm)
{
struct xgene_rtc_dev *pdata = dev_get_drvdata(dev);
- rtc_time_to_tm(readl(pdata->csr_base + RTC_CCVR), tm);
+ rtc_time64_to_tm(readl(pdata->csr_base + RTC_CCVR), tm);
return 0;
}
@@ -68,7 +68,7 @@ static int xgene_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
struct xgene_rtc_dev *pdata = dev_get_drvdata(dev);
/* If possible, CMR should be read here */
- rtc_time_to_tm(0, &alrm->time);
+ rtc_time64_to_tm(0, &alrm->time);
alrm->enabled = readl(pdata->csr_base + RTC_CCR) & RTC_CCR_IE;
return 0;
@@ -102,10 +102,8 @@ static int xgene_rtc_alarm_irq_enabled(struct device *dev)
static int xgene_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
{
struct xgene_rtc_dev *pdata = dev_get_drvdata(dev);
- unsigned long alarm_time;
- rtc_tm_to_time(&alrm->time, &alarm_time);
- writel((u32)alarm_time, pdata->csr_base + RTC_CMR);
+ writel((u32)rtc_tm_to_time64(&alrm->time), pdata->csr_base + RTC_CMR);
xgene_rtc_alarm_irq_enable(dev, alrm->enabled);
--
2.20.1
On 3/20/2019 6:02 PM, Alexandre Belloni wrote:
> Use SPDX-License-Identifier instead of a verbose license text.
>
> Signed-off-by: Alexandre Belloni <[email protected]
Please refer https://lkml.org/lkml/2019/2/13/570 for more discussion on
this.
once you fix that in all your SPDX license patches,you can take mine.,
Reviewed-by: Mukesh Ojha <[email protected]>
Cheers,
-Mukesh
> ---
> drivers/rtc/rtc-xgene.c | 15 +--------------
> 1 file changed, 1 insertion(+), 14 deletions(-)
>
> diff --git a/drivers/rtc/rtc-xgene.c b/drivers/rtc/rtc-xgene.c
> index e360f8917556..ba9121d02f02 100644
> --- a/drivers/rtc/rtc-xgene.c
> +++ b/drivers/rtc/rtc-xgene.c
> @@ -1,23 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0+
> /*
> * APM X-Gene SoC Real Time Clock Driver
> *
> * Copyright (c) 2014, Applied Micro Circuits Corporation
> * Author: Rameshwar Prasad Sahu <[email protected]>
> * Loc Ho <[email protected]>
> - *
> - * 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.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program. If not, see <http://www.gnu.org/licenses/>.
> - *
> */
>
> #include <linux/init.h>
On 01/04/2019 11:20:23+0530, Mukesh Ojha wrote:
>
> On 3/20/2019 6:02 PM, Alexandre Belloni wrote:
> > Use SPDX-License-Identifier instead of a verbose license text.
> >
> > Signed-off-by: Alexandre Belloni <[email protected]
>
>
> Please refer https://lkml.org/lkml/2019/2/13/570 for more discussion on
> this.
> once you fix that in all your SPDX license patches,you can take mine.,
>
You refer to a totally irrelevant discussion. Please point to what you
think is wrong with that patch.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com