[PATCH v4 0/6] SA1100/PXA RTC clean-up

[PATCH v4 0/6] SA1100/PXA RTC clean-up

[Rob Herring]

This series enables building the SA1100 RTC driver for ARM64 in
preparation for enabling CONFIG_ARCH_MMP on ARM64. This is needed for
supporting the PXA1928 SOC. Clean-up of SA1100 RTC is complicated by the
fact that some PXA chips use both sa1100-rtc and pxa-rtc drivers and
share the same registers. This does not work well with the Linux driver
model. So the first step is removing this sharing by the 2 drivers and
making sa1100-rtc and pxa-rtc mutually exclusive.

The previous versions can be found here[1][2][3]. Patches 1 and 2 are new.
Patches 3, 5, and 6 are unchanged. Patch 4 is mostly the same, but
adjusted due to patches 1 and 2.

A git branch is here[4]. I've tested sa1100-rtc on PXA1928. I need help
testing on PXA27x/3xx.

Rob

[1] http://comments.gmane.org/gmane.linux.ports.arm.kernel/391410
[2] http://comments.gmane.org/gmane.linux.ports.arm.kernel/409740
[3] http://comments.gmane.org/gmane.linux.ports.arm.kernel/412047
[4] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git marvell/rtc

Rob Herring (6):
  rtc: sa1100: prepare to share sa1100_rtc_ops
  rtc: pxa: convert to use shared sa1100 functions
  ARM: pxa: add memory resource to SA1100 RTC device
  rtc: sa1100/pxa: convert to run-time register mapping
  ARM: sa1100: remove unused RTC register definitions
  ARM: mmp: remove unused RTC register definitions

 arch/arm/mach-mmp/include/mach/regs-rtc.h   |  23 -----
 arch/arm/mach-pxa/devices.c                 |  18 +---
 arch/arm/mach-pxa/pxa27x.c                  |   1 -
 arch/arm/mach-pxa/pxa3xx.c                  |   1 -
 arch/arm/mach-sa1100/include/mach/SA-1100.h |  34 -------
 drivers/rtc/Kconfig                         |   4 +-
 drivers/rtc/rtc-pxa.c                       |  55 +++++------
 drivers/rtc/rtc-sa1100.c                    | 139 +++++++++++++++++-----------
 drivers/rtc/rtc-sa1100.h                    |  23 +++++
 9 files changed, 140 insertions(+), 158 deletions(-)
 delete mode 100644 arch/arm/mach-mmp/include/mach/regs-rtc.h
 create mode 100644 drivers/rtc/rtc-sa1100.h

--
2.1.0

[PATCH v4 1/6] rtc: sa1100: prepare to share sa1100_rtc_ops

[Rob Herring]

Factor out the RTC initialization from the platform device specific
parts in order to share the RTC device ops with other drivers.
Specifically, it will be shared with rtc-pxa driver.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: rtc-linux at googlegroups.com
---
 drivers/rtc/rtc-sa1100.c | 59 ++++++++++++++++++++++++------------------------
 drivers/rtc/rtc-sa1100.h | 19 ++++++++++++++++
 2 files changed, 49 insertions(+), 29 deletions(-)
 create mode 100644 drivers/rtc/rtc-sa1100.h

diff --git a/drivers/rtc/rtc-sa1100.c b/drivers/rtc/rtc-sa1100.c
index b6e1ca0..abc19ab 100644
--- a/drivers/rtc/rtc-sa1100.c
+++ b/drivers/rtc/rtc-sa1100.c
@@ -42,17 +42,12 @@
 #include <mach/regs-rtc.h>
 #endif
 
+#include "rtc-sa1100.h"
+
 #define RTC_DEF_DIVIDER		(32768 - 1)
 #define RTC_DEF_TRIM		0
 #define RTC_FREQ		1024
 
-struct sa1100_rtc {
-	spinlock_t		lock;
-	int			irq_1hz;
-	int			irq_alarm;
-	struct rtc_device	*rtc;
-	struct clk		*clk;
-};
 
 static irqreturn_t sa1100_rtc_interrupt(int irq, void *dev_id)
 {
@@ -223,29 +218,18 @@ static const struct rtc_class_ops sa1100_rtc_ops = {
 	.alarm_irq_enable = sa1100_rtc_alarm_irq_enable,
 };
 
-static int sa1100_rtc_probe(struct platform_device *pdev)
+int sa1100_rtc_init(struct platform_device *pdev, struct sa1100_rtc *info)
 {
 	struct rtc_device *rtc;
-	struct sa1100_rtc *info;
-	int irq_1hz, irq_alarm, ret = 0;
+	int ret;
 
-	irq_1hz = platform_get_irq_byname(pdev, "rtc 1Hz");
-	irq_alarm = platform_get_irq_byname(pdev, "rtc alarm");
-	if (irq_1hz < 0 || irq_alarm < 0)
-		return -ENODEV;
+	spin_lock_init(&info->lock);
 
-	info = devm_kzalloc(&pdev->dev, sizeof(struct sa1100_rtc), GFP_KERNEL);
-	if (!info)
-		return -ENOMEM;
 	info->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(info->clk)) {
 		dev_err(&pdev->dev, "failed to find rtc clock source\n");
 		return PTR_ERR(info->clk);
 	}
-	info->irq_1hz = irq_1hz;
-	info->irq_alarm = irq_alarm;
-	spin_lock_init(&info->lock);
-	platform_set_drvdata(pdev, info);
 
 	ret = clk_prepare_enable(info->clk);
 	if (ret)
@@ -265,14 +249,11 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 		RCNR = 0;
 	}
 
-	device_init_wakeup(&pdev->dev, 1);
-
 	rtc = devm_rtc_device_register(&pdev->dev, pdev->name, &sa1100_rtc_ops,
 					THIS_MODULE);
-
 	if (IS_ERR(rtc)) {
-		ret = PTR_ERR(rtc);
-		goto err_dev;
+		clk_disable_unprepare(info->clk);
+		return PTR_ERR(rtc);
 	}
 	info->rtc = rtc;
 
@@ -301,9 +282,29 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 	RTSR = RTSR_AL | RTSR_HZ;
 
 	return 0;
-err_dev:
-	clk_disable_unprepare(info->clk);
-	return ret;
+}
+EXPORT_SYMBOL_GPL(sa1100_rtc_init);
+
+static int sa1100_rtc_probe(struct platform_device *pdev)
+{
+	struct sa1100_rtc *info;
+	int irq_1hz, irq_alarm;
+
+	irq_1hz = platform_get_irq_byname(pdev, "rtc 1Hz");
+	irq_alarm = platform_get_irq_byname(pdev, "rtc alarm");
+	if (irq_1hz < 0 || irq_alarm < 0)
+		return -ENODEV;
+
+	info = devm_kzalloc(&pdev->dev, sizeof(struct sa1100_rtc), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+	info->irq_1hz = irq_1hz;
+	info->irq_alarm = irq_alarm;
+
+	platform_set_drvdata(pdev, info);
+	device_init_wakeup(&pdev->dev, 1);
+
+	return sa1100_rtc_init(pdev, info);
 }
 
 static int sa1100_rtc_remove(struct platform_device *pdev)
diff --git a/drivers/rtc/rtc-sa1100.h b/drivers/rtc/rtc-sa1100.h
new file mode 100644
index 0000000..665d054
--- /dev/null
+++ b/drivers/rtc/rtc-sa1100.h
@@ -0,0 +1,19 @@
+#ifndef __RTC_SA1100_H__
+#define __RTC_SA1100_H__
+
+#include <linux/kernel.h>
+
+struct clk;
+struct platform_device;
+
+struct sa1100_rtc {
+	spinlock_t		lock;
+	int			irq_1hz;
+	int			irq_alarm;
+	struct rtc_device	*rtc;
+	struct clk		*clk;
+};
+
+int sa1100_rtc_init(struct platform_device *pdev, struct sa1100_rtc *info);
+
+#endif
-- 
2.1.0

[PATCH v4 2/6] rtc: pxa: convert to use shared sa1100 functions

[Rob Herring]

Currently, the rtc-sa1100 and rtc-pxa drivers co-exist as rtc-pxa has a
superset of functionality. Having 2 drivers sharing the same memory
resource is not allowed by the driver model if resources are properly
declared. This problem was avoided by not adding memory resources to the
SA1100 RTC driver, but that prevents clean-up of the SA1100 driver.

This commit converts the PXA RTC to use the exported SA1100 RTC
functions. Now the sa1100-rtc and pxa-rtc devices are mutually
exclusive, so we must remove the sa1100-rtc from pxa27x and pxa3xx.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Daniel Mack <daniel@zonque.org>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: rtc-linux at googlegroups.com
---
 arch/arm/mach-pxa/pxa27x.c |  1 -
 arch/arm/mach-pxa/pxa3xx.c |  1 -
 drivers/rtc/Kconfig        |  4 +++-
 drivers/rtc/rtc-pxa.c      | 51 ++++++++++++++++++++++------------------------
 4 files changed, 27 insertions(+), 30 deletions(-)

diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
index af423a4..cd59959 100644
--- a/arch/arm/mach-pxa/pxa27x.c
+++ b/arch/arm/mach-pxa/pxa27x.c
@@ -449,7 +449,6 @@ static struct platform_device *devices[] __initdata = {
 	&pxa_device_asoc_ssp2,
 	&pxa_device_asoc_ssp3,
 	&pxa_device_asoc_platform,
-	&sa1100_device_rtc,
 	&pxa_device_rtc,
 	&pxa27x_device_ssp1,
 	&pxa27x_device_ssp2,
diff --git a/arch/arm/mach-pxa/pxa3xx.c b/arch/arm/mach-pxa/pxa3xx.c
index edcbd9c..ea2ebcf3 100644
--- a/arch/arm/mach-pxa/pxa3xx.c
+++ b/arch/arm/mach-pxa/pxa3xx.c
@@ -450,7 +450,6 @@ static struct platform_device *devices[] __initdata = {
 	&pxa_device_asoc_ssp3,
 	&pxa_device_asoc_ssp4,
 	&pxa_device_asoc_platform,
-	&sa1100_device_rtc,
 	&pxa_device_rtc,
 	&pxa3xx_device_ssp1,
 	&pxa3xx_device_ssp2,
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 0fe4ad8..e77f18c 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1301,9 +1301,11 @@ config RTC_DRV_GENERIC
 config RTC_DRV_PXA
        tristate "PXA27x/PXA3xx"
        depends on ARCH_PXA
+       select RTC_DRV_SA1100
        help
          If you say Y here you will get access to the real time clock
-         built into your PXA27x or PXA3xx CPU.
+         built into your PXA27x or PXA3xx CPU. This RTC is actually 2 RTCs
+         consisting of an SA1100 compatible RTC and the extended PXA RTC.
 
          This RTC driver uses PXA RTC registers available since pxa27x
          series (RDxR, RYxR) instead of legacy RCNR, RTAR.
diff --git a/drivers/rtc/rtc-pxa.c b/drivers/rtc/rtc-pxa.c
index 4561f37..fb9b3a7 100644
--- a/drivers/rtc/rtc-pxa.c
+++ b/drivers/rtc/rtc-pxa.c
@@ -32,6 +32,8 @@
 
 #include <mach/hardware.h>
 
+#include "rtc-sa1100.h"
+
 #define RTC_DEF_DIVIDER		(32768 - 1)
 #define RTC_DEF_TRIM		0
 #define MAXFREQ_PERIODIC	1000
@@ -86,10 +88,9 @@
 	__raw_writel((value), (pxa_rtc)->base + (reg))
 
 struct pxa_rtc {
+	struct sa1100_rtc sa1100_rtc;
 	struct resource	*ress;
 	void __iomem		*base;
-	int			irq_1Hz;
-	int			irq_Alrm;
 	struct rtc_device	*rtc;
 	spinlock_t		lock;		/* Protects this structure */
 };
@@ -184,25 +185,25 @@ static int pxa_rtc_open(struct device *dev)
 	struct pxa_rtc *pxa_rtc = dev_get_drvdata(dev);
 	int ret;
 
-	ret = request_irq(pxa_rtc->irq_1Hz, pxa_rtc_irq, 0,
+	ret = request_irq(pxa_rtc->sa1100_rtc.irq_1hz, pxa_rtc_irq, 0,
 			  "rtc 1Hz", dev);
 	if (ret < 0) {
-		dev_err(dev, "can't get irq %i, err %d\n", pxa_rtc->irq_1Hz,
-			ret);
+		dev_err(dev, "can't get irq %i, err %d\n",
+			pxa_rtc->sa1100_rtc.irq_1hz, ret);
 		goto err_irq_1Hz;
 	}
-	ret = request_irq(pxa_rtc->irq_Alrm, pxa_rtc_irq, 0,
+	ret = request_irq(pxa_rtc->sa1100_rtc.irq_alarm, pxa_rtc_irq, 0,
 			  "rtc Alrm", dev);
 	if (ret < 0) {
-		dev_err(dev, "can't get irq %i, err %d\n", pxa_rtc->irq_Alrm,
-			ret);
+		dev_err(dev, "can't get irq %i, err %d\n",
+			pxa_rtc->sa1100_rtc.irq_alarm, ret);
 		goto err_irq_Alrm;
 	}
 
 	return 0;
 
 err_irq_Alrm:
-	free_irq(pxa_rtc->irq_1Hz, dev);
+	free_irq(pxa_rtc->sa1100_rtc.irq_1hz, dev);
 err_irq_1Hz:
 	return ret;
 }
@@ -215,8 +216,8 @@ static void pxa_rtc_release(struct device *dev)
 	rtsr_clear_bits(pxa_rtc, RTSR_PIALE | RTSR_RDALE1 | RTSR_HZE);
 	spin_unlock_irq(&pxa_rtc->lock);
 
-	free_irq(pxa_rtc->irq_Alrm, dev);
-	free_irq(pxa_rtc->irq_1Hz, dev);
+	free_irq(pxa_rtc->sa1100_rtc.irq_1hz, dev);
+	free_irq(pxa_rtc->sa1100_rtc.irq_alarm, dev);
 }
 
 static int pxa_alarm_irq_enable(struct device *dev, unsigned int enabled)
@@ -320,12 +321,13 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct pxa_rtc *pxa_rtc;
+	struct sa1100_rtc *sa1100_rtc;
 	int ret;
-	u32 rttr;
 
 	pxa_rtc = devm_kzalloc(dev, sizeof(*pxa_rtc), GFP_KERNEL);
 	if (!pxa_rtc)
 		return -ENOMEM;
+	sa1100_rtc = &pxa_rtc->sa1100_rtc;
 
 	spin_lock_init(&pxa_rtc->lock);
 	platform_set_drvdata(pdev, pxa_rtc);
@@ -336,13 +338,13 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
-	pxa_rtc->irq_1Hz = platform_get_irq(pdev, 0);
-	if (pxa_rtc->irq_1Hz < 0) {
+	sa1100_rtc->irq_1hz = platform_get_irq(pdev, 0);
+	if (sa1100_rtc->irq_1hz < 0) {
 		dev_err(dev, "No 1Hz IRQ resource defined\n");
 		return -ENXIO;
 	}
-	pxa_rtc->irq_Alrm = platform_get_irq(pdev, 1);
-	if (pxa_rtc->irq_Alrm < 0) {
+	sa1100_rtc->irq_alarm = platform_get_irq(pdev, 1);
+	if (sa1100_rtc->irq_alarm < 0) {
 		dev_err(dev, "No alarm IRQ resource defined\n");
 		return -ENXIO;
 	}
@@ -354,15 +356,10 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	/*
-	 * If the clock divider is uninitialized then reset it to the
-	 * default value to get the 1Hz clock.
-	 */
-	if (rtc_readl(pxa_rtc, RTTR) == 0) {
-		rttr = RTC_DEF_DIVIDER + (RTC_DEF_TRIM << 16);
-		rtc_writel(pxa_rtc, RTTR, rttr);
-		dev_warn(dev, "warning: initializing default clock"
-			 " divider/trim value\n");
+	ret = sa1100_rtc_init(pdev, sa1100_rtc);
+	if (!ret) {
+		dev_err(dev, "Unable to init SA1100 RTC sub-device\n");
+		return ret;
 	}
 
 	rtsr_clear_bits(pxa_rtc, RTSR_PIALE | RTSR_RDALE1 | RTSR_HZE);
@@ -402,7 +399,7 @@ static int pxa_rtc_suspend(struct device *dev)
 	struct pxa_rtc *pxa_rtc = dev_get_drvdata(dev);
 
 	if (device_may_wakeup(dev))
-		enable_irq_wake(pxa_rtc->irq_Alrm);
+		enable_irq_wake(pxa_rtc->sa1100_rtc.irq_alarm);
 	return 0;
 }
 
@@ -411,7 +408,7 @@ static int pxa_rtc_resume(struct device *dev)
 	struct pxa_rtc *pxa_rtc = dev_get_drvdata(dev);
 
 	if (device_may_wakeup(dev))
-		disable_irq_wake(pxa_rtc->irq_Alrm);
+		disable_irq_wake(pxa_rtc->sa1100_rtc.irq_alarm);
 	return 0;
 }
 #endif
-- 
2.1.0

[PATCH v4 3/6] ARM: pxa: add memory resource to SA1100 RTC device

[Rob Herring]

The drivers for the SA1100 and PXA RTCs are now mutually exclusive, so
add the memory resource for the sa1100-rtc device. Since the memory
resource is already present in the pxa_rtc_resources, that makes
sa1100_rtc_resources and pxa_rtc_resources equivalent, so use
pxa_rtc_resources for both devices and remove the duplicate
sa1100_rtc_resources.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Daniel Mack <daniel@zonque.org>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
---
 arch/arm/mach-pxa/devices.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-pxa/devices.c b/arch/arm/mach-pxa/devices.c
index 3543466..b598db6 100644
--- a/arch/arm/mach-pxa/devices.c
+++ b/arch/arm/mach-pxa/devices.c
@@ -439,25 +439,11 @@ struct platform_device pxa_device_rtc = {
 	.resource       = pxa_rtc_resources,
 };
 
-static struct resource sa1100_rtc_resources[] = {
-	{
-		.start  = IRQ_RTC1Hz,
-		.end    = IRQ_RTC1Hz,
-		.name	= "rtc 1Hz",
-		.flags  = IORESOURCE_IRQ,
-	}, {
-		.start  = IRQ_RTCAlrm,
-		.end    = IRQ_RTCAlrm,
-		.name	= "rtc alarm",
-		.flags  = IORESOURCE_IRQ,
-	},
-};
-
 struct platform_device sa1100_device_rtc = {
 	.name		= "sa1100-rtc",
 	.id		= -1,
-	.num_resources	= ARRAY_SIZE(sa1100_rtc_resources),
-	.resource	= sa1100_rtc_resources,
+	.num_resources  = ARRAY_SIZE(pxa_rtc_resources),
+	.resource       = pxa_rtc_resources,
 };
 
 static struct resource pxa_ac97_resources[] = {
-- 
2.1.0

[PATCH v4 4/6] rtc: sa1100/pxa: convert to run-time register mapping

[Rob Herring]

SA1100 and PXA differ only in register offsets which are currently
hardcoded in a machine specific header. Some arm64 platforms (PXA1928)
have this RTC block as well (and not the PXA270 variant).

Convert the driver to use ioremap and set the register offsets dynamically.
Since we are touching all the register accesses, convert them all to
readl_relaxed/writel_relaxed.

Signed-off-by: Rob Herring <robh@kernel.org>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: rtc-linux at googlegroups.com
---
 drivers/rtc/rtc-pxa.c    |  4 +++
 drivers/rtc/rtc-sa1100.c | 82 ++++++++++++++++++++++++++++++++----------------
 drivers/rtc/rtc-sa1100.h |  4 +++
 3 files changed, 63 insertions(+), 27 deletions(-)

diff --git a/drivers/rtc/rtc-pxa.c b/drivers/rtc/rtc-pxa.c
index fb9b3a7..fe4985b 100644
--- a/drivers/rtc/rtc-pxa.c
+++ b/drivers/rtc/rtc-pxa.c
@@ -356,6 +356,10 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	sa1100_rtc->rcnr = pxa_rtc->base + 0x0;
+	sa1100_rtc->rtsr = pxa_rtc->base + 0x8;
+	sa1100_rtc->rtar = pxa_rtc->base + 0x4;
+	sa1100_rtc->rttr = pxa_rtc->base + 0xc;
 	ret = sa1100_rtc_init(pdev, sa1100_rtc);
 	if (!ret) {
 		dev_err(dev, "Unable to init SA1100 RTC sub-device\n");
diff --git a/drivers/rtc/rtc-sa1100.c b/drivers/rtc/rtc-sa1100.c
index abc19ab..c2187bf 100644
--- a/drivers/rtc/rtc-sa1100.c
+++ b/drivers/rtc/rtc-sa1100.c
@@ -35,12 +35,10 @@
 #include <linux/bitops.h>
 #include <linux/io.h>
 
-#include <mach/hardware.h>
-#include <mach/irqs.h>
-
-#if defined(CONFIG_ARCH_PXA) || defined(CONFIG_ARCH_MMP)
-#include <mach/regs-rtc.h>
-#endif
+#define RTSR_HZE		BIT(3)	/* HZ interrupt enable */
+#define RTSR_ALE		BIT(2)	/* RTC alarm interrupt enable */
+#define RTSR_HZ			BIT(1)	/* HZ rising-edge detected */
+#define RTSR_AL			BIT(0)	/* RTC alarm detected */
 
 #include "rtc-sa1100.h"
 
@@ -58,16 +56,16 @@ static irqreturn_t sa1100_rtc_interrupt(int irq, void *dev_id)
 
 	spin_lock(&info->lock);
 
-	rtsr = RTSR;
+	rtsr = readl_relaxed(info->rtsr);
 	/* clear interrupt sources */
-	RTSR = 0;
+	writel_relaxed(0, info->rtsr);
 	/* Fix for a nasty initialization problem the in SA11xx RTSR register.
 	 * See also the comments in sa1100_rtc_probe(). */
 	if (rtsr & (RTSR_ALE | RTSR_HZE)) {
 		/* This is the original code, before there was the if test
 		 * above. This code does not clear interrupts that were not
 		 * enabled. */
-		RTSR = (RTSR_AL | RTSR_HZ) & (rtsr >> 2);
+		writel_relaxed((RTSR_AL | RTSR_HZ) & (rtsr >> 2), info->rtsr);
 	} else {
 		/* For some reason, it is possible to enter this routine
 		 * without interruptions enabled, it has been tested with
@@ -76,13 +74,13 @@ static irqreturn_t sa1100_rtc_interrupt(int irq, void *dev_id)
 		 * This situation leads to an infinite "loop" of interrupt
 		 * routine calling and as a result the processor seems to
 		 * lock on its first call to open(). */
-		RTSR = RTSR_AL | RTSR_HZ;
+		writel_relaxed(RTSR_AL | RTSR_HZ, info->rtsr);
 	}
 
 	/* clear alarm interrupt if it has occurred */
 	if (rtsr & RTSR_AL)
 		rtsr &= ~RTSR_ALE;
-	RTSR = rtsr & (RTSR_ALE | RTSR_HZE);
+	writel_relaxed(rtsr & (RTSR_ALE | RTSR_HZE), info->rtsr);
 
 	/* update irq data & counter */
 	if (rtsr & RTSR_AL)
@@ -130,7 +128,7 @@ static void sa1100_rtc_release(struct device *dev)
 	struct sa1100_rtc *info = dev_get_drvdata(dev);
 
 	spin_lock_irq(&info->lock);
-	RTSR = 0;
+	writel_relaxed(0, info->rtsr);
 	spin_unlock_irq(&info->lock);
 
 	free_irq(info->irq_alarm, dev);
@@ -139,39 +137,46 @@ static void sa1100_rtc_release(struct device *dev)
 
 static int sa1100_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
 {
+	u32 rtsr;
 	struct sa1100_rtc *info = dev_get_drvdata(dev);
 
 	spin_lock_irq(&info->lock);
+	rtsr = readl_relaxed(info->rtsr);
 	if (enabled)
-		RTSR |= RTSR_ALE;
+		rtsr |= RTSR_ALE;
 	else
-		RTSR &= ~RTSR_ALE;
+		rtsr &= ~RTSR_ALE;
+	writel_relaxed(rtsr, info->rtsr);
 	spin_unlock_irq(&info->lock);
 	return 0;
 }
 
 static int sa1100_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
-	rtc_time_to_tm(RCNR, tm);
+	struct sa1100_rtc *info = dev_get_drvdata(dev);
+
+	rtc_time_to_tm(readl_relaxed(info->rcnr), tm);
 	return 0;
 }
 
 static int sa1100_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
+	struct sa1100_rtc *info = dev_get_drvdata(dev);
 	unsigned long time;
 	int ret;
 
 	ret = rtc_tm_to_time(tm, &time);
 	if (ret == 0)
-		RCNR = time;
+		writel_relaxed(time, info->rcnr);
 	return ret;
 }
 
 static int sa1100_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 {
 	u32	rtsr;
+	struct sa1100_rtc *info = dev_get_drvdata(dev);
 
-	rtsr = RTSR;
+	rtsr = readl_relaxed(info->rtsr);
 	alrm->enabled = (rtsr & RTSR_ALE) ? 1 : 0;
 	alrm->pending = (rtsr & RTSR_AL) ? 1 : 0;
 	return 0;
@@ -187,12 +192,13 @@ static int sa1100_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	ret = rtc_tm_to_time(&alrm->time, &time);
 	if (ret != 0)
 		goto out;
-	RTSR = RTSR & (RTSR_HZE|RTSR_ALE|RTSR_AL);
-	RTAR = time;
+	writel_relaxed(readl_relaxed(info->rtsr) &
+		(RTSR_HZE | RTSR_ALE | RTSR_AL), info->rtsr);
+	writel_relaxed(time, info->rtar);
 	if (alrm->enabled)
-		RTSR |= RTSR_ALE;
+		writel_relaxed(readl_relaxed(info->rtsr) | RTSR_ALE, info->rtsr);
 	else
-		RTSR &= ~RTSR_ALE;
+		writel_relaxed(readl_relaxed(info->rtsr) & ~RTSR_ALE, info->rtsr);
 out:
 	spin_unlock_irq(&info->lock);
 
@@ -201,8 +207,10 @@ out:
 
 static int sa1100_rtc_proc(struct device *dev, struct seq_file *seq)
 {
-	seq_printf(seq, "trim/divider\t\t: 0x%08x\n", (u32) RTTR);
-	seq_printf(seq, "RTSR\t\t\t: 0x%08x\n", (u32)RTSR);
+	struct sa1100_rtc *info = dev_get_drvdata(dev);
+
+	seq_printf(seq, "trim/divider\t\t: 0x%08x\n", readl_relaxed(info->rttr));
+	seq_printf(seq, "RTSR\t\t\t: 0x%08x\n", readl_relaxed(info->rtsr));
 
 	return 0;
 }
@@ -241,12 +249,12 @@ int sa1100_rtc_init(struct platform_device *pdev, struct sa1100_rtc *info)
 	 * If the clock divider is uninitialized then reset it to the
 	 * default value to get the 1Hz clock.
 	 */
-	if (RTTR == 0) {
-		RTTR = RTC_DEF_DIVIDER + (RTC_DEF_TRIM << 16);
+	if (readl_relaxed(info->rttr) == 0) {
+		writel_relaxed(RTC_DEF_DIVIDER + (RTC_DEF_TRIM << 16), info->rttr);
 		dev_warn(&pdev->dev, "warning: "
 			"initializing default clock divider/trim value\n");
 		/* The current RTC value probably doesn't make sense either */
-		RCNR = 0;
+		writel_relaxed(0, info->rcnr);
 	}
 
 	rtc = devm_rtc_device_register(&pdev->dev, pdev->name, &sa1100_rtc_ops,
@@ -279,7 +287,7 @@ int sa1100_rtc_init(struct platform_device *pdev, struct sa1100_rtc *info)
 	 *
 	 * Notice that clearing bit 1 and 0 is accomplished by writting ONES to
 	 * the corresponding bits in RTSR. */
-	RTSR = RTSR_AL | RTSR_HZ;
+	writel_relaxed(RTSR_AL | RTSR_HZ, info->rtsr);
 
 	return 0;
 }
@@ -288,6 +296,8 @@ EXPORT_SYMBOL_GPL(sa1100_rtc_init);
 static int sa1100_rtc_probe(struct platform_device *pdev)
 {
 	struct sa1100_rtc *info;
+	struct resource *iores;
+	void __iomem *base;
 	int irq_1hz, irq_alarm;
 
 	irq_1hz = platform_get_irq_byname(pdev, "rtc 1Hz");
@@ -301,6 +311,24 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 	info->irq_1hz = irq_1hz;
 	info->irq_alarm = irq_alarm;
 
+	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, iores);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	if (IS_ENABLED(CONFIG_ARCH_SA1100) ||
+	    of_device_is_compatible(pdev->dev.of_node, "mrvl,sa1100-rtc")) {
+		info->rcnr = base + 0x04;
+		info->rtsr = base + 0x10;
+		info->rtar = base + 0x00;
+		info->rttr = base + 0x08;
+	} else {
+		info->rcnr = base + 0x0;
+		info->rtsr = base + 0x8;
+		info->rtar = base + 0x4;
+		info->rttr = base + 0xc;
+	}
+
 	platform_set_drvdata(pdev, info);
 	device_init_wakeup(&pdev->dev, 1);
 
diff --git a/drivers/rtc/rtc-sa1100.h b/drivers/rtc/rtc-sa1100.h
index 665d054..2c79c0c 100644
--- a/drivers/rtc/rtc-sa1100.h
+++ b/drivers/rtc/rtc-sa1100.h
@@ -8,6 +8,10 @@ struct platform_device;
 
 struct sa1100_rtc {
 	spinlock_t		lock;
+	void __iomem		*rcnr;
+	void __iomem		*rtar;
+	void __iomem		*rtsr;
+	void __iomem		*rttr;
 	int			irq_1hz;
 	int			irq_alarm;
 	struct rtc_device	*rtc;
-- 
2.1.0

[PATCH v4 5/6] ARM: sa1100: remove unused RTC register definitions

[Rob Herring]

Now that register definitions have been moved to the driver, we can remove
them from machine specific code.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
---
 arch/arm/mach-sa1100/include/mach/SA-1100.h | 34 -----------------------------
 1 file changed, 34 deletions(-)

diff --git a/arch/arm/mach-sa1100/include/mach/SA-1100.h b/arch/arm/mach-sa1100/include/mach/SA-1100.h
index 0ac6cc0..7972617 100644
--- a/arch/arm/mach-sa1100/include/mach/SA-1100.h
+++ b/arch/arm/mach-sa1100/include/mach/SA-1100.h
@@ -858,40 +858,6 @@
 
 
 /*
- * Real-Time Clock (RTC) control registers
- *
- * Registers
- *    RTAR      	Real-Time Clock (RTC) Alarm Register (read/write).
- *    RCNR      	Real-Time Clock (RTC) CouNt Register (read/write).
- *    RTTR      	Real-Time Clock (RTC) Trim Register (read/write).
- *    RTSR      	Real-Time Clock (RTC) Status Register (read/write).
- *
- * Clocks
- *    frtx, Trtx	Frequency, period of the real-time clock crystal
- *              	(32.768 kHz nominal).
- *    frtc, Trtc	Frequency, period of the real-time clock counter
- *              	(1 Hz nominal).
- */
-
-#define RTAR		__REG(0x90010000)  /* RTC Alarm Reg. */
-#define RCNR		__REG(0x90010004)  /* RTC CouNt Reg. */
-#define RTTR		__REG(0x90010008)  /* RTC Trim Reg. */
-#define RTSR		__REG(0x90010010)  /* RTC Status Reg. */
-
-#define RTTR_C  	Fld (16, 0)	/* clock divider Count - 1         */
-#define RTTR_D  	Fld (10, 16)	/* trim Delete count               */
-                	        	/* frtc = (1023*(C + 1) - D)*frtx/ */
-                	        	/*        (1023*(C + 1)^2)         */
-                	        	/* Trtc = (1023*(C + 1)^2)*Trtx/   */
-                	        	/*        (1023*(C + 1) - D)       */
-
-#define RTSR_AL 	0x00000001	/* ALarm detected                  */
-#define RTSR_HZ 	0x00000002	/* 1 Hz clock detected             */
-#define RTSR_ALE	0x00000004	/* ALarm interrupt Enable          */
-#define RTSR_HZE	0x00000008	/* 1 Hz clock interrupt Enable     */
-
-
-/*
  * Power Manager (PM) control registers
  *
  * Registers
-- 
2.1.0

[PATCH v4 6/6] ARM: mmp: remove unused RTC register definitions

[Rob Herring]

Now that register definitions have been moved to the driver, regs-rtc.h is
no longer used and can be removed.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
---
 arch/arm/mach-mmp/include/mach/regs-rtc.h | 23 -----------------------
 1 file changed, 23 deletions(-)
 delete mode 100644 arch/arm/mach-mmp/include/mach/regs-rtc.h

diff --git a/arch/arm/mach-mmp/include/mach/regs-rtc.h b/arch/arm/mach-mmp/include/mach/regs-rtc.h
deleted file mode 100644
index 5bff886..0000000
--- a/arch/arm/mach-mmp/include/mach/regs-rtc.h
+++ /dev/null
@@ -1,23 +0,0 @@
-#ifndef __ASM_MACH_REGS_RTC_H
-#define __ASM_MACH_REGS_RTC_H
-
-#include <mach/addr-map.h>
-
-#define RTC_VIRT_BASE	(APB_VIRT_BASE + 0x10000)
-#define RTC_REG(x)	(*((volatile u32 __iomem *)(RTC_VIRT_BASE + (x))))
-
-/*
- * Real Time Clock
- */
-
-#define RCNR		RTC_REG(0x00)	/* RTC Count Register */
-#define RTAR		RTC_REG(0x04)	/* RTC Alarm Register */
-#define RTSR		RTC_REG(0x08)	/* RTC Status Register */
-#define RTTR		RTC_REG(0x0C)	/* RTC Timer Trim Register */
-
-#define RTSR_HZE	(1 << 3)	/* HZ interrupt enable */
-#define RTSR_ALE	(1 << 2)	/* RTC alarm interrupt enable */
-#define RTSR_HZ		(1 << 1)	/* HZ rising-edge detected */
-#define RTSR_AL		(1 << 0)	/* RTC alarm detected */
-
-#endif /* __ASM_MACH_REGS_RTC_H */
-- 
2.1.0

[PATCH v4 0/6] SA1100/PXA RTC clean-up

[Robert Jarzmik]

Rob Herring <robh@kernel.org> writes:

> A git branch is here[4]. I've tested sa1100-rtc on PXA1928. I need help
> testing on PXA27x/3xx.
Hi Rob,

I tested on PXA27x. It works for the boards which used pxa-rtc.
For the others, their defconfig become broken, because the pxa boards with
sa1100-rtc do not work anymore. And of course no .config manipulation will fix
this, because PXA architecture is denied the sa1100-rtc driver.

As a consequence, these boards should transition from sa1100-rtc to pxa-rtc to
get back a working rtc, amongst which :
	cm_x2xx_defconfig
	em_x270_defconfig
	h5000_defconfig
	jornada720_defconfig
	magician_defconfig
	palmz72_defconfig
	pcm027_defconfig
	trizeps4_defconfig
	viper_defconfig
	xcep_defconfig

I think you should at least Cc the maintainers of these boards to see their
reaction. They should be aware of the impacts and comment.

I'll make a test for pxa3xx in the next week.

Cheers.

--
Robert

[PATCH v4 0/6] SA1100/PXA RTC clean-up

[Rob Herring]

On Fri, May 15, 2015 at 6:13 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>> A git branch is here[4]. I've tested sa1100-rtc on PXA1928. I need help
>> testing on PXA27x/3xx.
> Hi Rob,
>
> I tested on PXA27x. It works for the boards which used pxa-rtc.
> For the others, their defconfig become broken, because the pxa boards with
> sa1100-rtc do not work anymore. And of course no .config manipulation will fix
> this, because PXA architecture is denied the sa1100-rtc driver.

Ah, I didn't realize there is a mixture of use.

I could just do something like this instead of removing
sa1100_device_rtc registration:

#ifndef CONFIG_RTC_DRV_PXA
       &sa1100_device_rtc,
#else
        &pxa_device_rtc,
#endif

Rob

[PATCH v4 0/6] SA1100/PXA RTC clean-up

[Robert Jarzmik]

Rob Herring <robh@kernel.org> writes:

> On Fri, May 15, 2015 at 6:13 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
>> Rob Herring <robh@kernel.org> writes:
>>
>>> A git branch is here[4]. I've tested sa1100-rtc on PXA1928. I need help
>>> testing on PXA27x/3xx.
>> Hi Rob,
>>
>> I tested on PXA27x. It works for the boards which used pxa-rtc.
>> For the others, their defconfig become broken, because the pxa boards with
>> sa1100-rtc do not work anymore. And of course no .config manipulation will fix
>> this, because PXA architecture is denied the sa1100-rtc driver.
>
> Ah, I didn't realize there is a mixture of use.
>
> I could just do something like this instead of removing
> sa1100_device_rtc registration:
>
> #ifndef CONFIG_RTC_DRV_PXA
>        &sa1100_device_rtc,
> #else
>         &pxa_device_rtc,
> #endif
I ... don't like that much, especially when you consider both rtc might be
modules for example. I rather like that patch in pxa as they are.

I'm rather in a mood for a more aggressive approach :

 - you fire an incremental patch to patch the 10 defconfigs on PXA architecture
   (pxa27x and pxa3xx)
 - in this patch, you replace 's/CONFIG_RTC_DRV_SA1100/CONFIG_RTC_DRV_PXA/'
 - you send this patch to the maintainers and me
 - you explain in the commit message that from a userland perspective, nothing
   changes, except that the RTC IP will change, and any dependency on a
   bootloader fidling with RTC should be considered as a source of regression.
   Moreover the first reboot will have the wrong date, until it is set.

After that, if no board maintainer complains, or doesn't answer (and therefore
doesn't care), and you have my blessing to make the switch. If one complains and
demonstrates why this change might break his board, we'll see.

Cheers.

-- 
Robert

[PATCH v4 0/6] SA1100/PXA RTC clean-up

[Rob Herring]

On Mon, May 18, 2015 at 1:43 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>> On Fri, May 15, 2015 at 6:13 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
>>> Rob Herring <robh@kernel.org> writes:
>>>
>>>> A git branch is here[4]. I've tested sa1100-rtc on PXA1928. I need help
>>>> testing on PXA27x/3xx.
>>> Hi Rob,
>>>
>>> I tested on PXA27x. It works for the boards which used pxa-rtc.
>>> For the others, their defconfig become broken, because the pxa boards with
>>> sa1100-rtc do not work anymore. And of course no .config manipulation will fix
>>> this, because PXA architecture is denied the sa1100-rtc driver.
>>
>> Ah, I didn't realize there is a mixture of use.
>>
>> I could just do something like this instead of removing
>> sa1100_device_rtc registration:
>>
>> #ifndef CONFIG_RTC_DRV_PXA
>>        &sa1100_device_rtc,
>> #else
>>         &pxa_device_rtc,
>> #endif
> I ... don't like that much, especially when you consider both rtc might be
> modules for example. I rather like that patch in pxa as they are.
>
> I'm rather in a mood for a more aggressive approach :
>
>  - you fire an incremental patch to patch the 10 defconfigs on PXA architecture
>    (pxa27x and pxa3xx)

Here's the breakdown of configs which enable SA1100 RTC only. Only the
last 6 need to change by my count:

pxa25x
arch/arm/configs/h5000_defconfig
arch/arm/configs/xcep_defconfig
arch/arm/configs/viper_defconfig

pxa255/270
arch/arm/configs/cm_x2xx_defconfig

pxa270
arch/arm/configs/em_x270_defconfig
arch/arm/configs/magician_defconfig
arch/arm/configs/palmz72_defconfig
arch/arm/configs/pcm027_defconfig
arch/arm/configs/trizeps4_defconfig

>  - in this patch, you replace 's/CONFIG_RTC_DRV_SA1100/CONFIG_RTC_DRV_PXA/'
>  - you send this patch to the maintainers and me
>  - you explain in the commit message that from a userland perspective, nothing
>    changes, except that the RTC IP will change, and any dependency on a

The IP does not change here. rtc0 is still going to be the SA1100 RTC
being registered first. The only change will be the addition of rtc1.

>    bootloader fidling with RTC should be considered as a source of regression.

I'm not sure that I follow.

>    Moreover the first reboot will have the wrong date, until it is set.

Only for rtc1, right?

Rob

[PATCH v4 0/6] SA1100/PXA RTC clean-up

[Robert Jarzmik]

Rob Herring <robh@kernel.org> writes:

>> I'm rather in a mood for a more aggressive approach :
>>
>>  - you fire an incremental patch to patch the 10 defconfigs on PXA architecture
>>    (pxa27x and pxa3xx)
>
> Here's the breakdown of configs which enable SA1100 RTC only. Only the
> last 6 need to change by my count:
>
> pxa25x
> arch/arm/configs/h5000_defconfig
> arch/arm/configs/xcep_defconfig
> arch/arm/configs/viper_defconfig
>
> pxa255/270
> arch/arm/configs/cm_x2xx_defconfig
>
> pxa270
> arch/arm/configs/em_x270_defconfig
> arch/arm/configs/magician_defconfig
> arch/arm/configs/palmz72_defconfig
> arch/arm/configs/pcm027_defconfig
> arch/arm/configs/trizeps4_defconfig
And arch/arm/configs/jornada720_defconfig I counted wrongly as it's sa1100.
So I agree with your list of last 6 defconfigs.

>>  - in this patch, you replace 's/CONFIG_RTC_DRV_SA1100/CONFIG_RTC_DRV_PXA/'
>>  - you send this patch to the maintainers and me
>>  - you explain in the commit message that from a userland perspective, nothing
>>    changes, except that the RTC IP will change, and any dependency on a
>
> The IP does not change here. rtc0 is still going to be the SA1100 RTC
> being registered first. The only change will be the addition of rtc1.
For boards which were only using rtc-pxa.c (as mioa701 for example), they relied
on the fact that rtc0 == pxa_rtc. Their time is stored in PXA IP. Therefore,
each of their hwclock will end up on sa1100-rtc instead of pxa-rtc.

So for these boards, ie. for all boards where only rtc-pxa.c was used, the IP
addressed changes from a casual userspace perspective.

>>    bootloader fidling with RTC should be considered as a source of regression.
>
> I'm not sure that I follow.
Let's talk about how a double boot windows + linux box works.
The bootloader ensures that :
 - sa1100-rtc holds the number of seconds since the OS start (think jiffies)
 - pxa-rtc holds the wall clock time

Upon each reboot, sa1100-rtc is checked to see how much time has passed. If an
"oustanding number" is detected, for example 10 years, the firmware resets the
data partition.

Now think what will happen when this change will be commited, upon the first
reboot after the linux kernel has change sa1100-rtc time.

>>    Moreover the first reboot will have the wrong date, until it is set.
> Only for rtc1, right?
Yes, right, unless the bootloader logic is ... over-engineered.

All of this to say maintainers should be forwarned at least. After that, up to
them to react.

Cheers.

-- 
Robert

[PATCH v4 0/6] SA1100/PXA RTC clean-up

[Rob Herring]

On Fri, Jun 5, 2015 at 2:43 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>>> I'm rather in a mood for a more aggressive approach :
>>>
>>>  - you fire an incremental patch to patch the 10 defconfigs on PXA architecture
>>>    (pxa27x and pxa3xx)
>>
>> Here's the breakdown of configs which enable SA1100 RTC only. Only the
>> last 6 need to change by my count:
>>
>> pxa25x
>> arch/arm/configs/h5000_defconfig
>> arch/arm/configs/xcep_defconfig
>> arch/arm/configs/viper_defconfig
>>
>> pxa255/270
>> arch/arm/configs/cm_x2xx_defconfig
>>
>> pxa270
>> arch/arm/configs/em_x270_defconfig
>> arch/arm/configs/magician_defconfig
>> arch/arm/configs/palmz72_defconfig
>> arch/arm/configs/pcm027_defconfig
>> arch/arm/configs/trizeps4_defconfig
> And arch/arm/configs/jornada720_defconfig I counted wrongly as it's sa1100.
> So I agree with your list of last 6 defconfigs.
>
>>>  - in this patch, you replace 's/CONFIG_RTC_DRV_SA1100/CONFIG_RTC_DRV_PXA/'
>>>  - you send this patch to the maintainers and me
>>>  - you explain in the commit message that from a userland perspective, nothing
>>>    changes, except that the RTC IP will change, and any dependency on a
>>
>> The IP does not change here. rtc0 is still going to be the SA1100 RTC
>> being registered first. The only change will be the addition of rtc1.
> For boards which were only using rtc-pxa.c (as mioa701 for example), they relied
> on the fact that rtc0 == pxa_rtc. Their time is stored in PXA IP. Therefore,
> each of their hwclock will end up on sa1100-rtc instead of pxa-rtc.
>
> So for these boards, ie. for all boards where only rtc-pxa.c was used, the IP
> addressed changes from a casual userspace perspective.

Okay, so this is the case where the time will be wrong.

I could remove the select of the sa1100-rtc and do an empty function
for sa1100_rtc_init. This would preserve current behavior.

>
>>>    bootloader fidling with RTC should be considered as a source of regression.
>>
>> I'm not sure that I follow.
> Let's talk about how a double boot windows + linux box works.
> The bootloader ensures that :
>  - sa1100-rtc holds the number of seconds since the OS start (think jiffies)
>  - pxa-rtc holds the wall clock time
>
> Upon each reboot, sa1100-rtc is checked to see how much time has passed. If an
> "oustanding number" is detected, for example 10 years, the firmware resets the
> data partition.
>
> Now think what will happen when this change will be commited, upon the first
> reboot after the linux kernel has change sa1100-rtc time.

That would be bad. But on these platforms, the kernel has been using
both RTCs right? Presumably on platforms only using 1 of the RTCs, the
bootloader does not touch the RTCs.

Rob

>
>>>    Moreover the first reboot will have the wrong date, until it is set.
>> Only for rtc1, right?
> Yes, right, unless the bootloader logic is ... over-engineered.
>
> All of this to say maintainers should be forwarned at least. After that, up to
> them to react.
>
> Cheers.
>
> --
> Robert

[PATCH v4 0/6] SA1100/PXA RTC clean-up

[Robert Jarzmik]

Rob Herring <robh@kernel.org> writes:

>>>>  - you explain in the commit message that from a userland perspective, nothing
>>>>    changes, except that the RTC IP will change, and any dependency on a
>>>
>>> The IP does not change here. rtc0 is still going to be the SA1100 RTC
>>> being registered first. The only change will be the addition of rtc1.
>> For boards which were only using rtc-pxa.c (as mioa701 for example), they relied
>> on the fact that rtc0 == pxa_rtc. Their time is stored in PXA IP. Therefore,
>> each of their hwclock will end up on sa1100-rtc instead of pxa-rtc.
>>
>> So for these boards, ie. for all boards where only rtc-pxa.c was used, the IP
>> addressed changes from a casual userspace perspective.
>
> Okay, so this is the case where the time will be wrong.
>
> I could remove the select of the sa1100-rtc and do an empty function
> for sa1100_rtc_init. This would preserve current behavior.
Please don't. I only ask for a sentence in the commit message.

>>>>    bootloader fidling with RTC should be considered as a source of regression.
>>>
>>> I'm not sure that I follow.
>> Let's talk about how a double boot windows + linux box works.
>> The bootloader ensures that :
>>  - sa1100-rtc holds the number of seconds since the OS start (think jiffies)
>>  - pxa-rtc holds the wall clock time
>>
>> Upon each reboot, sa1100-rtc is checked to see how much time has passed. If an
>> "oustanding number" is detected, for example 10 years, the firmware resets the
>> data partition.
>>
>> Now think what will happen when this change will be commited, upon the first
>> reboot after the linux kernel has change sa1100-rtc time.
>
> That would be bad. But on these platforms, the kernel has been using
> both RTCs right? Presumably on platforms only using 1 of the RTCs, the
> bootloader does not touch the RTCs.
No, the kernel has been using only pxa-rtc. The sa1100-rtc is not used, and yet
it accounts the passed time in it.

On platforms where all the OSes use only 1 RTC, the bootloader only touches one
RTC, that's correct. Or said differently : the bootloader touches the union of
all the RTCs the OSes it launches do touch.

>> All of this to say maintainers should be forwarned at least. After that, up to
>> them to react.

Cheers.

-- 
Robert

[PATCH v4 0/6] SA1100/PXA RTC clean-up

[Rob Herring]

On Sat, Jun 6, 2015 at 4:25 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>>>>>  - you explain in the commit message that from a userland perspective, nothing
>>>>>    changes, except that the RTC IP will change, and any dependency on a
>>>>
>>>> The IP does not change here. rtc0 is still going to be the SA1100 RTC
>>>> being registered first. The only change will be the addition of rtc1.
>>> For boards which were only using rtc-pxa.c (as mioa701 for example), they relied
>>> on the fact that rtc0 == pxa_rtc. Their time is stored in PXA IP. Therefore,
>>> each of their hwclock will end up on sa1100-rtc instead of pxa-rtc.
>>>
>>> So for these boards, ie. for all boards where only rtc-pxa.c was used, the IP
>>> addressed changes from a casual userspace perspective.
>>
>> Okay, so this is the case where the time will be wrong.
>>
>> I could remove the select of the sa1100-rtc and do an empty function
>> for sa1100_rtc_init. This would preserve current behavior.
> Please don't. I only ask for a sentence in the commit message.
>
>>>>>    bootloader fidling with RTC should be considered as a source of regression.
>>>>
>>>> I'm not sure that I follow.
>>> Let's talk about how a double boot windows + linux box works.
>>> The bootloader ensures that :
>>>  - sa1100-rtc holds the number of seconds since the OS start (think jiffies)
>>>  - pxa-rtc holds the wall clock time
>>>
>>> Upon each reboot, sa1100-rtc is checked to see how much time has passed. If an
>>> "oustanding number" is detected, for example 10 years, the firmware resets the
>>> data partition.
>>>
>>> Now think what will happen when this change will be commited, upon the first
>>> reboot after the linux kernel has change sa1100-rtc time.
>>
>> That would be bad. But on these platforms, the kernel has been using
>> both RTCs right? Presumably on platforms only using 1 of the RTCs, the
>> bootloader does not touch the RTCs.
> No, the kernel has been using only pxa-rtc. The sa1100-rtc is not used, and yet
> it accounts the passed time in it.
>
> On platforms where all the OSes use only 1 RTC, the bootloader only touches one
> RTC, that's correct. Or said differently : the bootloader touches the union of
> all the RTCs the OSes it launches do touch.

Sorry, I'm being dense here, but I'm not any less confused as to which 
scenarios you think have an issue. Rather than go around again on this, 
perhaps you can just provide the warning for the commit text. Here's 
what I have.

Rob


8<-------------------------------------------------------------------
>From e43db14b27486faef8d8e74e4aa555aba7f31a0e Mon Sep 17 00:00:00 2001
From: Rob Herring <robh@kernel.org>
Date: Mon, 1 Jun 2015 07:53:01 -0500
Subject: [PATCH] ARM: config: Switch PXA27x platforms to use PXA RTC driver

With the SA1100 and PXA RTC drivers be mutually exclusive and no
longer sharing hardware, PXA27x/PXA3xx platforms must use the PXA RTC
driver as the SA1100 platform device is no longer registered.

This change should be transparent to userspace, but the first reboot
after the switch will have the wrong time.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Daniel Mack <daniel@zonque.org>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Sergey Lapin <slapin@ossfans.org>
Cc: Russell King <linux@arm.linux.org.uk>
---
 arch/arm/configs/cm_x2xx_defconfig  | 2 +-
 arch/arm/configs/em_x270_defconfig  | 2 +-
 arch/arm/configs/magician_defconfig | 2 +-
 arch/arm/configs/palmz72_defconfig  | 2 +-
 arch/arm/configs/pcm027_defconfig   | 2 +-
 arch/arm/configs/trizeps4_defconfig | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/configs/cm_x2xx_defconfig b/arch/arm/configs/cm_x2xx_defconfig
index dc01c04..3b32d5f 100644
--- a/arch/arm/configs/cm_x2xx_defconfig
+++ b/arch/arm/configs/cm_x2xx_defconfig
@@ -157,7 +157,7 @@ CONFIG_LEDS_TRIGGERS=y
 CONFIG_LEDS_TRIGGER_HEARTBEAT=y
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_V3020=y
-CONFIG_RTC_DRV_SA1100=y
+CONFIG_RTC_DRV_PXA=y
 CONFIG_EXT2_FS=y
 CONFIG_EXT3_FS=y
 CONFIG_INOTIFY=y
diff --git a/arch/arm/configs/em_x270_defconfig b/arch/arm/configs/em_x270_defconfig
index 4560c9c..8e10df7 100644
--- a/arch/arm/configs/em_x270_defconfig
+++ b/arch/arm/configs/em_x270_defconfig
@@ -157,7 +157,7 @@ CONFIG_LEDS_TRIGGERS=y
 CONFIG_LEDS_TRIGGER_HEARTBEAT=y
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_V3020=y
-CONFIG_RTC_DRV_SA1100=y
+CONFIG_RTC_DRV_PXA=y
 CONFIG_EXT2_FS=y
 CONFIG_EXT3_FS=y
 CONFIG_INOTIFY=y
diff --git a/arch/arm/configs/magician_defconfig b/arch/arm/configs/magician_defconfig
index 557dd29..a5b4920 100644
--- a/arch/arm/configs/magician_defconfig
+++ b/arch/arm/configs/magician_defconfig
@@ -150,7 +150,7 @@ CONFIG_LEDS_TRIGGERS=y
 CONFIG_LEDS_TRIGGER_BACKLIGHT=y
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DEBUG=y
-CONFIG_RTC_DRV_SA1100=y
+CONFIG_RTC_DRV_PXA=y
 CONFIG_EXT2_FS=y
 CONFIG_INOTIFY=y
 CONFIG_MSDOS_FS=m
diff --git a/arch/arm/configs/palmz72_defconfig b/arch/arm/configs/palmz72_defconfig
index 4baa83c..83c135e 100644
--- a/arch/arm/configs/palmz72_defconfig
+++ b/arch/arm/configs/palmz72_defconfig
@@ -67,7 +67,7 @@ CONFIG_MMC=y
 CONFIG_MMC_DEBUG=y
 CONFIG_MMC_PXA=y
 CONFIG_RTC_CLASS=y
-CONFIG_RTC_DRV_SA1100=y
+CONFIG_RTC_DRV_PXA=y
 CONFIG_EXT2_FS=y
 CONFIG_EXT3_FS=y
 # CONFIG_DNOTIFY is not set
diff --git a/arch/arm/configs/pcm027_defconfig b/arch/arm/configs/pcm027_defconfig
index 0a847d0..b5624e3 100644
--- a/arch/arm/configs/pcm027_defconfig
+++ b/arch/arm/configs/pcm027_defconfig
@@ -82,7 +82,7 @@ CONFIG_MMC=y
 CONFIG_MMC_PXA=y
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_PCF8563=m
-CONFIG_RTC_DRV_SA1100=m
+CONFIG_RTC_DRV_PXA=m
 CONFIG_EXT2_FS=m
 CONFIG_EXT3_FS=m
 # CONFIG_DNOTIFY is not set
diff --git a/arch/arm/configs/trizeps4_defconfig b/arch/arm/configs/trizeps4_defconfig
index 932ee4e..4bc8700 100644
--- a/arch/arm/configs/trizeps4_defconfig
+++ b/arch/arm/configs/trizeps4_defconfig
@@ -177,7 +177,7 @@ CONFIG_NEW_LEDS=y
 CONFIG_RTC_CLASS=y
 # CONFIG_RTC_HCTOSYS is not set
 CONFIG_RTC_DRV_PCF8583=m
-CONFIG_RTC_DRV_SA1100=y
+CONFIG_RTC_DRV_PXA=y
 CONFIG_EXT2_FS=y
 CONFIG_EXT2_FS_XATTR=y
 CONFIG_EXT2_FS_POSIX_ACL=y
-- 
2.1.0

[PATCH v4 0/6] SA1100/PXA RTC clean-up

[Robert Jarzmik]

Rob Herring <robherring2@gmail.com> writes:

>> On platforms where all the OSes use only 1 RTC, the bootloader only touches one
>> RTC, that's correct. Or said differently : the bootloader touches the union of
>> all the RTCs the OSes it launches do touch.
>
> Sorry, I'm being dense here, but I'm not any less confused as to which 
> scenarios you think have an issue. Rather than go around again on this, 
> perhaps you can just provide the warning for the commit text. Here's 
> what I have.

> 8<-------------------------------------------------------------------
> From e43db14b27486faef8d8e74e4aa555aba7f31a0e Mon Sep 17 00:00:00 2001
> From: Rob Herring <robh@kernel.org>
> Date: Mon, 1 Jun 2015 07:53:01 -0500
> Subject: [PATCH] ARM: config: Switch PXA27x platforms to use PXA RTC driver
>
> With the SA1100 and PXA RTC drivers be mutually exclusive and no
> longer sharing hardware, PXA27x/PXA3xx platforms must use the PXA RTC
> driver as the SA1100 platform device is no longer registered.
>
> This change should be transparent to userspace, but the first reboot
> after the switch will have the wrong time.

I'll switch that last sentence to :
"This change should be almost transparent to userspace. Former users of pxa-rtc
should be aware that 2 rtc will be available on their kernels, rtc0 being
sa1100-rtc and rtc1 being pxa-rtc. Any userspace relying on the fact that rtc0
was pxa-rtc should be fixed.

As a consequence :
 - the first reboot after the switch will have the wrong time,
 - on dual boot platform where the other OS programs some logic into the sa1100
   rtc IP, a lack of fix in userspace, ie. a kernel changing sa1100-rtc thinking
   it is pxa-rtc could have dire consequence, such as wiping the other OS data
   partition.
"

And add :
Cc: Mike Rapoport <mike@compulab.co.il>
Cc: Philipp Zabel <philipp.zabel@gmail.com>

With that, I'll be happy. I'll throw this serie at my jenkins, but there's no
reason for it to complain, and unless I've forgotten some unaddressed comment,
you'll have my ack within the next couple of days.

Cheers.

--
Robert