[PATCH 1/7] rtc: xgene: fix possible race condition

[PATCH 1/7] rtc: xgene: 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-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

[PATCH 2/7] rtc: xgene: set range

[Alexandre Belloni]

CCVR is a 32bit second counter.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 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

[PATCH 3/7] rtc: xgene: 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-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 <rsahu@apm.com>
  *         Loc Ho <lho@apm.com>
- *
- * 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

Re: [PATCH 3/7] rtc: xgene: convert to SPDX identifier

[Mukesh Ojha]

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 <alexandre.belloni@bootlin.com


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 <mojha@codeaurora.org>

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 <rsahu@apm.com>
>    *         Loc Ho <lho@apm.com>
> - *
> - * 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>

Re: [PATCH 3/7] rtc: xgene: convert to SPDX identifier

[Alexandre Belloni]

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 <alexandre.belloni@bootlin.com
> 
> 
> 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

[PATCH 4/7] rtc: xgene: correct checkpatch issues

[Alexandre Belloni]

Correct trivial whitespace issues. Also sort the headers.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 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 <lho@apm.com>
  */
 
+#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

[PATCH 5/7] rtc: xgene: stop caching alarm_time

[Alexandre Belloni]

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 <alexandre.belloni@bootlin.com>
---
 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

[PATCH 6/7] rtc: xgene: switch to rtc_time64_to_tm/rtc_tm_to_time64

[Alexandre Belloni]

Call the 64bit versions of rtc_tm time conversion as the range is enforced
by the core.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 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

[PATCH 7/7] rtc: xgene: 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-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