Skip to contentSkip to conversation

share sa1100 rtc driver to arch-mmp

PATCH Superseded Haojian Zhuang 8 participants 40 messages Feb 21, 2012 – Feb 23, 2012 1 message not in the archive

Subsystems (5)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover share sa1100 rtc driver to arch-mmp

Remove the hardcode definition of rtc registers, use ioremap() instead. Append clk support for arch-mmp, so add dummy clk in arch-sa1100/arch-pxa.

#1 lore raw

PATCH 1/7 rtc: sa1100: remove periodic code

Since periodic timer is already supported by hrtimer in rtc interface, we needn't keep the duplicated code in rtc-sa1100.c any more.

---
 drivers/rtc/rtc-sa1100.c |   15 ---------------
 1 files changed, 0 insertions(+), 15 deletions(-)
drivers/rtc/rtc-sa1100.c +0 -15
index cb9a585..fef52c8 100644
@@ -42,19 +42,8 @@
 #define RTC_DEF_TRIM		0
 
 static const unsigned long RTC_FREQ = 1024;
-static struct rtc_time rtc_alarm;
 static DEFINE_SPINLOCK(sa1100_rtc_lock);
 
-static inline int rtc_periodic_alarm(struct rtc_time *tm)
-{
-	return  (tm->tm_year == -1) ||
-		((unsigned)tm->tm_mon >= 12) ||
-		((unsigned)(tm->tm_mday - 1) >= 31) ||
-		((unsigned)tm->tm_hour > 23) ||
-		((unsigned)tm->tm_min > 59) ||
-		((unsigned)tm->tm_sec > 59);
-}
-
 /*
  * Calculate the next alarm time given the requested alarm time mask
  * and the current time.
@@ -146,9 +135,6 @@ static irqreturn_t sa1100_rtc_interrupt(int irq, void *dev_id)
 
 	rtc_update_irq(rtc, 1, events);
 
-	if (rtsr & RTSR_AL && rtc_periodic_alarm(&rtc_alarm))
-		rtc_update_alarm(&rtc_alarm);
-
 	spin_unlock(&sa1100_rtc_lock);
 
 	return IRQ_HANDLED;
@@ -225,7 +211,6 @@ static int sa1100_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 {
 	u32	rtsr;
 
-	memcpy(&alrm->time, &rtc_alarm, sizeof(struct rtc_time));
 	rtsr = RTSR;
 	alrm->enabled = (rtsr & RTSR_ALE) ? 1 : 0;
 	alrm->pending = (rtsr & RTSR_AL) ? 1 : 0;
Signature · 1 line
1.7.0.4
#2 lore raw

PATCH 2/7 rtc: sa1100: remove verification code of alarm

In reply to Haojian Zhuang

Since next alarm time is already calculated in rtc common interface, we needn't keep this logic in rtc-sa1100.c any more.

---
 drivers/rtc/rtc-sa1100.c |   66 +++++++--------------------------------------
 1 files changed, 11 insertions(+), 55 deletions(-)
drivers/rtc/rtc-sa1100.c +11 -55
index fef52c8..91d58bd 100644
@@ -44,54 +44,6 @@
 static const unsigned long RTC_FREQ = 1024;
 static DEFINE_SPINLOCK(sa1100_rtc_lock);
 
-/*
- * Calculate the next alarm time given the requested alarm time mask
- * and the current time.
- */
-static void rtc_next_alarm_time(struct rtc_time *next, struct rtc_time *now,
-	struct rtc_time *alrm)
-{
-	unsigned long next_time;
-	unsigned long now_time;
-
-	next->tm_year = now->tm_year;
-	next->tm_mon = now->tm_mon;
-	next->tm_mday = now->tm_mday;
-	next->tm_hour = alrm->tm_hour;
-	next->tm_min = alrm->tm_min;
-	next->tm_sec = alrm->tm_sec;
-
-	rtc_tm_to_time(now, &now_time);
-	rtc_tm_to_time(next, &next_time);
-
-	if (next_time < now_time) {
-		/* Advance one day */
-		next_time += 60 * 60 * 24;
-		rtc_time_to_tm(next_time, next);
-	}
-}
-
-static int rtc_update_alarm(struct rtc_time *alrm)
-{
-	struct rtc_time alarm_tm, now_tm;
-	unsigned long now, time;
-	int ret;
-
-	do {
-		now = RCNR;
-		rtc_time_to_tm(now, &now_tm);
-		rtc_next_alarm_time(&alarm_tm, &now_tm, alrm);
-		ret = rtc_tm_to_time(&alarm_tm, &time);
-		if (ret != 0)
-			break;
-
-		RTSR = RTSR & (RTSR_HZE|RTSR_ALE|RTSR_AL);
-		RTAR = time;
-	} while (now != RCNR);
-
-	return ret;
-}
-
 static irqreturn_t sa1100_rtc_interrupt(int irq, void *dev_id)
 {
 	struct platform_device *pdev = to_platform_device(dev_id);
@@ -219,16 +171,20 @@ static int sa1100_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 
 static int sa1100_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 {
+	unsigned long time;
 	int ret;
 
 	spin_lock_irq(&sa1100_rtc_lock);
-	ret = rtc_update_alarm(&alrm->time);
-	if (ret == 0) {
-		if (alrm->enabled)
-			RTSR |= RTSR_ALE;
-		else
-			RTSR &= ~RTSR_ALE;
-	}
+	ret = rtc_tm_to_time(&alrm->time, &time);
+	if (ret != 0)
+		goto out;
+	RTSR = RTSR & (RTSR_HZE|RTSR_ALE|RTSR_AL);
+	RTAR = time;
+	if (alrm->enabled)
+		RTSR |= RTSR_ALE;
+	else
+		RTSR &= ~RTSR_ALE;
+out:
 	spin_unlock_irq(&sa1100_rtc_lock);
 
 	return ret;
Signature · 1 line
1.7.0.4
#3 lore raw

PATCH 3/7 rtc: sa1100: use ioremap to map registers

In reply to Haojian Zhuang

Add resource definition of sa1100 rtc. And use ioremap to map rtc registers to avoid including address definition of rtc register. So the same driver could be shared on sa1100/pxa/mmp series.

Since there's two RTC wrappers on RTC module among PXA27x/PXA3xx/PXA95x, keep treating the two wrappers as two RTC devices will result issues of registering platform device. So only keep one RTC wrapper in these silicons.

---
 arch/arm/mach-pxa/devices.c    |    6 +-
 arch/arm/mach-pxa/pxa27x.c     |    1 -
 arch/arm/mach-pxa/pxa3xx.c     |    1 -
 arch/arm/mach-pxa/pxa95x.c     |    1 -
 arch/arm/mach-sa1100/generic.c |   20 ++++
 drivers/rtc/Kconfig            |    4 +-
 drivers/rtc/rtc-sa1100.c       |  243 ++++++++++++++++++++++++++++++----------
 7 files changed, 211 insertions(+), 65 deletions(-)
arch/arm/mach-pxa/devices.c +5 -1
index 5bc1312..9fe7d6a 100644
@@ -406,18 +406,22 @@ static struct resource pxa_rtc_resources[] = {
 	[1] = {
 		.start  = IRQ_RTC1Hz,
 		.end    = IRQ_RTC1Hz,
+		.name	= "rtc 1Hz",
 		.flags  = IORESOURCE_IRQ,
 	},
 	[2] = {
 		.start  = IRQ_RTCAlrm,
 		.end    = IRQ_RTCAlrm,
+		.name	= "rtc alarm",
 		.flags  = IORESOURCE_IRQ,
 	},
 };
 
 struct platform_device sa1100_device_rtc = {
-	.name		= "sa1100-rtc",
+	.name		= "pxa25x-rtc",
 	.id		= -1,
+	.num_resources	= ARRAY_SIZE(pxa_rtc_resources),
+	.resource	= pxa_rtc_resources,
 };
 
 struct platform_device pxa_device_rtc = {
arch/arm/mach-pxa/pxa27x.c +0 -1
index aed6cbc..345fa7b 100644
@@ -431,7 +431,6 @@ static struct platform_device *devices[] __initdata = {
 	&pxa_device_asoc_ssp3,
 	&pxa_device_asoc_platform,
 	&sa1100_device_rtc,
-	&pxa_device_rtc,
 	&pxa27x_device_ssp1,
 	&pxa27x_device_ssp2,
 	&pxa27x_device_ssp3,
arch/arm/mach-pxa/pxa3xx.c +0 -1
index 4f402af..d2437a8 100644
@@ -428,7 +428,6 @@ static struct platform_device *devices[] __initdata = {
 	&pxa_device_asoc_ssp4,
 	&pxa_device_asoc_platform,
 	&sa1100_device_rtc,
-	&pxa_device_rtc,
 	&pxa27x_device_ssp1,
 	&pxa27x_device_ssp2,
 	&pxa27x_device_ssp3,
arch/arm/mach-pxa/pxa95x.c +0 -1
index d082a58..b41e9f3 100644
@@ -249,7 +249,6 @@ void __init pxa95x_set_i2c_power_info(struct i2c_pxa_platform_data *info)
 
 static struct platform_device *devices[] __initdata = {
 	&pxa_device_gpio,
-	&sa1100_device_rtc,
 	&pxa_device_rtc,
 	&pxa27x_device_ssp1,
 	&pxa27x_device_ssp2,
arch/arm/mach-sa1100/generic.c +20 -0
index bb10ee2..04a62a3 100644
@@ -345,9 +345,29 @@ void sa11x0_register_irda(struct irda_platform_data *irda)
 	sa11x0_register_device(&sa11x0ir_device, irda);
 }
 
+static struct resource sa1100_rtc_resources[] = {
+	{
+		.start	= 0x90010000,
+		.end	= 0x9001003f,
+		.flags	= IORESOURCE_MEM,
+	}, {
+		.start  = IRQ_RTC1Hz,
+		.end    = IRQ_RTC1Hz,
+		.name	= "rtc 1Hz",
+		.flags  = IORESOURCE_IRQ,
+	}, {
+		.start  = IRQ_RTCAlrm,
+		.end    = IRQ_RTCAlrm,
+		.name	= "rtc alarm",
+		.flags  = IORESOURCE_IRQ,
+	},
+};
+
 static struct platform_device sa11x0rtc_device = {
 	.name		= "sa1100-rtc",
 	.id		= -1,
+	.num_resources	= ARRAY_SIZE(sa1100_rtc_resources),
+	.resource	= sa1100_rtc_resources,
 };
 
 static struct platform_device *sa11x0_devices[] __initdata = {
drivers/rtc/Kconfig +2 -2
index 3a125b8..59efc63 100644
@@ -773,8 +773,8 @@ config RTC_DRV_EP93XX
 	  will be called rtc-ep93xx.
 
 config RTC_DRV_SA1100
-	tristate "SA11x0/PXA2xx"
-	depends on ARCH_SA1100 || ARCH_PXA
+	tristate "SA11x0/PXA2xx/PXA910"
+	depends on ARCH_SA1100 || ARCH_PXA || ARCH_MMP
 	help
 	  If you say Y here you will get access to the real time clock
 	  built into your SA11x0 or PXA2xx CPU.
drivers/rtc/rtc-sa1100.c +184 -59

418 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#4 lore raw

PATCH 3/7 rtc: sa1100: use ioremap to map registers

Quoted 11 lines · Haojian Zhuang

Add resource definition of sa1100 rtc. And use ioremap to map rtc registers to avoid including address definition of rtc register. So the same driver could be shared on sa1100/pxa/mmp series.

Since there's two RTC wrappers on RTC module among PXA27x/PXA3xx/PXA95x, keep treating the two wrappers as two RTC devices will result issues of registering platform device. So only keep one RTC wrapper in these silicons.

I believe I reviewed this in the past and it looks good to me in its present version, so

#5 lore raw

PATCH 3/7 rtc: sa1100: use ioremap to map registers

In reply to Haojian Zhuang
Quoted 15 lines · Haojian Zhuang
arch/arm/mach-sa1100/generic.c +7 -0
index bb10ee2..04a62a3 100644
@@ -345,9 +345,29 @@ void sa11x0_register_irda(struct irda_platform_data *irda)
 	sa11x0_register_device(&sa11x0ir_device, irda);
 }
 
+static struct resource sa1100_rtc_resources[] = {
+	{
+		.start	= 0x90010000,
+		.end	= 0x9001003f,
+		.flags	= IORESOURCE_MEM,
+	}, {
+		.start  = IRQ_RTC1Hz,

arch/arm/mach-sa1100/generic.c void sa11x0_register_irda(struct irda_platform_data *irda) · line 364

Quoted 10 lines
arch/arm/mach-sa1100/generic.c +10 -0
+		.end    = IRQ_RTC1Hz,
+		.name	= "rtc 1Hz",
+		.flags  = IORESOURCE_IRQ,
+	}, {
+		.start  = IRQ_RTCAlrm,
+		.end    = IRQ_RTCAlrm,
+		.name	= "rtc alarm",
+		.flags  = IORESOURCE_IRQ,
+	},
+};

As I've converted all resources in this file to use the DEFINE_RES_foo() macros, it would be a shame to have this initialization reintroduced. Please use DEFINE_RES_foo() here.

maintainer
#6 lore raw

PATCH 3/7 rtc: sa1100: use ioremap to map registers

Quoted 22 lines · Russell King - ARM Linux
Quoted 21 lines · Haojian Zhuang
arch/arm/mach-sa1100/generic.c +13 -0
index bb10ee2..04a62a3 100644
@@ -345,9 +345,29 @@ void sa11x0_register_irda(struct irda_platform_data *irda)
 ? ? ? sa11x0_register_device(&sa11x0ir_device, irda);
 ?}
 
+static struct resource sa1100_rtc_resources[] = {
+ ? ? {
+ ? ? ? ? ? ? .start ?= 0x90010000,
+ ? ? ? ? ? ? .end ? ?= 0x9001003f,
+ ? ? ? ? ? ? .flags ?= IORESOURCE_MEM,
+ ? ? }, {
+ ? ? ? ? ? ? .start ?= IRQ_RTC1Hz,
+ ? ? ? ? ? ? .end ? ?= IRQ_RTC1Hz,
+ ? ? ? ? ? ? .name ? = "rtc 1Hz",
+ ? ? ? ? ? ? .flags ?= IORESOURCE_IRQ,
+ ? ? }, {
+ ? ? ? ? ? ? .start ?= IRQ_RTCAlrm,
+ ? ? ? ? ? ? .end ? ?= IRQ_RTCAlrm,
Quoted 9 lines

arch/arm/mach-sa1100/generic.c void sa11x0_register_irda(struct irda_platform_data *irda) · line 364

Quoted 4 lines
arch/arm/mach-sa1100/generic.c +4 -0
+ ? ? ? ? ? ? .name ? = "rtc alarm",
+ ? ? ? ? ? ? .flags ?= IORESOURCE_IRQ,
+ ? ? },
+};

As I've converted all resources in this file to use the DEFINE_RES_foo() macros, it would be a shame to have this initialization reintroduced. Please use DEFINE_RES_foo() here.

Sure. I'll update it right now.

Thanks
Haojian

#7 lore raw

PATCH v2 3/7 rtc: sa1100: use ioremap to map registers

In reply to Haojian Zhuang

Add resource definition of sa1100 rtc. And use ioremap to map rtc registers to avoid including address definition of rtc register. So the same driver could be shared on sa1100/pxa/mmp series.

Since there's two RTC wrappers on RTC module among PXA27x/PXA3xx/PXA95x, keep treating the two wrappers as two RTC devices will result issues of registering platform device. So only keep one RTC wrapper in these silicons.

---
 arch/arm/mach-pxa/devices.c    |    6 +-
 arch/arm/mach-pxa/pxa27x.c     |    1 -
 arch/arm/mach-pxa/pxa3xx.c     |    1 -
 arch/arm/mach-pxa/pxa95x.c     |    1 -
 arch/arm/mach-sa1100/generic.c |    8 ++
 drivers/rtc/Kconfig            |    4 +-
 drivers/rtc/rtc-sa1100.c       |  243 ++++++++++++++++++++++++++++++----------
 7 files changed, 199 insertions(+), 65 deletions(-)
arch/arm/mach-pxa/devices.c +5 -1
index 5bc1312..9fe7d6a 100644
@@ -406,18 +406,22 @@ static struct resource pxa_rtc_resources[] = {
 	[1] = {
 		.start  = IRQ_RTC1Hz,
 		.end    = IRQ_RTC1Hz,
+		.name	= "rtc 1Hz",
 		.flags  = IORESOURCE_IRQ,
 	},
 	[2] = {
 		.start  = IRQ_RTCAlrm,
 		.end    = IRQ_RTCAlrm,
+		.name	= "rtc alarm",
 		.flags  = IORESOURCE_IRQ,
 	},
 };
 
 struct platform_device sa1100_device_rtc = {
-	.name		= "sa1100-rtc",
+	.name		= "pxa25x-rtc",
 	.id		= -1,
+	.num_resources	= ARRAY_SIZE(pxa_rtc_resources),
+	.resource	= pxa_rtc_resources,
 };
 
 struct platform_device pxa_device_rtc = {
arch/arm/mach-pxa/pxa27x.c +0 -1
index aed6cbc..345fa7b 100644
@@ -431,7 +431,6 @@ static struct platform_device *devices[] __initdata = {
 	&pxa_device_asoc_ssp3,
 	&pxa_device_asoc_platform,
 	&sa1100_device_rtc,
-	&pxa_device_rtc,
 	&pxa27x_device_ssp1,
 	&pxa27x_device_ssp2,
 	&pxa27x_device_ssp3,
arch/arm/mach-pxa/pxa3xx.c +0 -1
index 4f402af..d2437a8 100644
@@ -428,7 +428,6 @@ static struct platform_device *devices[] __initdata = {
 	&pxa_device_asoc_ssp4,
 	&pxa_device_asoc_platform,
 	&sa1100_device_rtc,
-	&pxa_device_rtc,
 	&pxa27x_device_ssp1,
 	&pxa27x_device_ssp2,
 	&pxa27x_device_ssp3,
arch/arm/mach-pxa/pxa95x.c +0 -1
index d082a58..b41e9f3 100644
@@ -249,7 +249,6 @@ void __init pxa95x_set_i2c_power_info(struct i2c_pxa_platform_data *info)
 
 static struct platform_device *devices[] __initdata = {
 	&pxa_device_gpio,
-	&sa1100_device_rtc,
 	&pxa_device_rtc,
 	&pxa27x_device_ssp1,
 	&pxa27x_device_ssp2,
arch/arm/mach-sa1100/generic.c +8 -0
index bb10ee2..7c1ebf4 100644
@@ -345,9 +345,17 @@ void sa11x0_register_irda(struct irda_platform_data *irda)
 	sa11x0_register_device(&sa11x0ir_device, irda);
 }
 
+static struct resource sa1100_rtc_resources[] = {
+	DEFINE_RES_MEM(0x90010000, 0x9001003f),
+	DEFINE_RES_IRQ_NAMED(IRQ_RTC1Hz, "rtc 1Hz"),
+	DEFINE_RES_IRQ_NAMED(IRQ_RTCAlrm, "rtc alarm"),
+};
+
 static struct platform_device sa11x0rtc_device = {
 	.name		= "sa1100-rtc",
 	.id		= -1,
+	.num_resources	= ARRAY_SIZE(sa1100_rtc_resources),
+	.resource	= sa1100_rtc_resources,
 };
 
 static struct platform_device *sa11x0_devices[] __initdata = {
drivers/rtc/Kconfig +2 -2
index 3a125b8..59efc63 100644
@@ -773,8 +773,8 @@ config RTC_DRV_EP93XX
 	  will be called rtc-ep93xx.
 
 config RTC_DRV_SA1100
-	tristate "SA11x0/PXA2xx"
-	depends on ARCH_SA1100 || ARCH_PXA
+	tristate "SA11x0/PXA2xx/PXA910"
+	depends on ARCH_SA1100 || ARCH_PXA || ARCH_MMP
 	help
 	  If you say Y here you will get access to the real time clock
 	  built into your SA11x0 or PXA2xx CPU.
drivers/rtc/rtc-sa1100.c +184 -59

418 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#8 lore raw

PATCH v2 3/7 rtc: sa1100: use ioremap to map registers

drivers/rtc/rtc-sa1100.c static int sa1100_rtc_open(struct device *dev) · line 214

Quoted 8 lines · Haojian Zhuang

static int sa1100_rtc_read_time(struct device *dev, struct rtc_time *tm) {

drivers/rtc/rtc-sa1100.c +5 -1
-	rtc_time_to_tm(RCNR, tm);
+	struct sa1100_rtc *info = dev_get_drvdata(dev);
+	unsigned long rcnr;
+
+	rcnr = readl_relaxed(info->reg_rcnr);
+	rtc_time_to_tm(rcnr, tm);

Just one nit - this could be:

	struct sa1100_rtc *info = dev_get_drvdata(dev);

	rtc_time_to_tm(readl_relaxed(info->reg_rcnr), tm);
#9 lore raw

PATCH 4/7 ARM: sa1100: clean up of the clock support

In reply to Haojian Zhuang

From: Jett.Zhou <[email protected]>

---
 arch/arm/Kconfig             |    2 +-
 arch/arm/mach-sa1100/clock.c |   91 ++++++++++++++++++++++++++++++-----------
 2 files changed, 67 insertions(+), 26 deletions(-)
arch/arm/Kconfig +1 -1
index a48aecc..6e40039 100644
@@ -754,7 +754,7 @@ config ARCH_SA1100
 	select ARCH_HAS_CPUFREQ
 	select CPU_FREQ
 	select GENERIC_CLOCKEVENTS
-	select HAVE_CLK
+	select CLKDEV_LOOKUP
 	select HAVE_SCHED_CLOCK
 	select TICK_ONESHOT
 	select ARCH_REQUIRE_GPIOLIB
arch/arm/mach-sa1100/clock.c +66 -25

134 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#10 lore raw

PATCH 4/7 ARM: sa1100: clean up of the clock support

Quoted 8 lines · Haojian Zhuang

From: Jett.Zhou <[email protected]>

---
 arch/arm/Kconfig             |    2 +-
 arch/arm/mach-sa1100/clock.c |   91 ++++++++++++++++++++++++++++++-----------
 2 files changed, 67 insertions(+), 26 deletions(-)

I don't see anything wrong with the patch, but you really need to add a description here, because it's anything but obvious why a "cleanup" would add three times the lines it removes.

Arnd

#11 lore raw

PATCH 4/7 ARM: sa1100: clean up of the clock support

In reply to Haojian Zhuang
Quoted 32 lines · Haojian Zhuang

From: Jett.Zhou <[email protected]>

---
 arch/arm/Kconfig             |    2 +-
 arch/arm/mach-sa1100/clock.c |   91 ++++++++++++++++++++++++++++++-----------
 2 files changed, 67 insertions(+), 26 deletions(-)
arch/arm/Kconfig +1 -1
index a48aecc..6e40039 100644
@@ -754,7 +754,7 @@ config ARCH_SA1100
 	select ARCH_HAS_CPUFREQ
 	select CPU_FREQ
 	select GENERIC_CLOCKEVENTS
-	select HAVE_CLK
+	select CLKDEV_LOOKUP
 	select HAVE_SCHED_CLOCK
 	select TICK_ONESHOT
 	select ARCH_REQUIRE_GPIOLIB
arch/arm/mach-sa1100/clock.c +2 -0
index dab3c63..d6df9f6 100644
@@ -11,17 +11,39 @@
 #include <linux/clk.h>
 #include <linux/spinlock.h>
 #include <linux/mutex.h>
+#include <linux/io.h>
+#include <linux/clkdev.h>

arch/arm/mach-sa1100/clock.c line 22

Quoted 10 lines
arch/arm/mach-sa1100/clock.c +4 -3
 
 #include <mach/hardware.h>
 
-/*
- * Very simple clock implementation - we only have one clock to deal with.
- */
+struct clkops {
+	void			(*enable)(struct clk *);
+	void			(*disable)(struct clk *);
+	unsigned long		(*getrate)(struct clk *);

Is getrate() used? If not, please get rid of it.

Quoted 5 lines
arch/arm/mach-sa1100/clock.c +4 -0
+};
+
 struct clk {
+	const struct clkops	*ops;
+	unsigned long		rate;

arch/arm/mach-sa1100/clock.c line 36

Quoted 10 lines
arch/arm/mach-sa1100/clock.c +6 -1
 	unsigned int		enabled;
 };
 
-static void clk_gpio27_enable(void)
+#define INIT_CLKREG(_clk, _devname, _conname)		\
+	{						\
+		.clk		= _clk,			\
+		.dev_id		= _devname,		\
+		.con_id		= _conname,		\
+	}

This should use CLKDEV_INIT() rather than defining its own.

Quoted 36 lines
arch/arm/mach-sa1100/clock.c +11 -12
+
+#define DEFINE_CLK(_name, _ops, _rate)			\
+struct clk clk_##_name = {				\
+		.ops	= _ops,				\
+		.rate	= _rate,			\
+	}
+
+static DEFINE_SPINLOCK(clocks_lock);
+
+static void clk_gpio27_enable(struct clk *clk)
 {
 	/*
 	 * First, set up the 3.6864MHz clock on GPIO 27 for the SA-1111:
@@ -32,38 +54,22 @@ static void clk_gpio27_enable(void)
 	TUCR = TUCR_3_6864MHz;
 }
 
-static void clk_gpio27_disable(void)
+static void clk_gpio27_disable(struct clk *clk)
 {
 	TUCR = 0;
 	GPDR &= ~GPIO_32_768kHz;
 	GAFR &= ~GPIO_32_768kHz;
 }
 
-static struct clk clk_gpio27;
-
-static DEFINE_SPINLOCK(clocks_lock);
-
-struct clk *clk_get(struct device *dev, const char *id)
-{
-	const char *devname = dev_name(dev);
-
-	return strcmp(devname, "sa1111.0") ? ERR_PTR(-ENOENT) : &clk_gpio27;
-}
-EXPORT_SYMBOL(clk_get);

arch/arm/mach-sa1100/clock.c static void clk_gpio27_enable(void) · line 67

Quoted 10 lines
arch/arm/mach-sa1100/clock.c +0 -6
-
-void clk_put(struct clk *clk)
-{
-}
-EXPORT_SYMBOL(clk_put);
-
 int clk_enable(struct clk *clk)
 {
 	unsigned long flags;
 
	if (clk) {

arch/arm/mach-sa1100/clock.c static void clk_gpio27_enable(void) · line 71

Quoted 5 lines
arch/arm/mach-sa1100/clock.c +1 -1
 	spin_lock_irqsave(&clocks_lock, flags);
 	if (clk->enabled++ == 0)
-		clk_gpio27_enable();
+		clk->ops->enable(clk);
 	spin_unlock_irqrestore(&clocks_lock, flags);
	}

arch/arm/mach-sa1100/clock.c void clk_disable(struct clk *clk) · line 82

Quoted 6 lines
arch/arm/mach-sa1100/clock.c +1 -0
+
 	return 0;
 }
 EXPORT_SYMBOL(clk_enable);
@@ -76,13 +82,48 @@ void clk_disable(struct clk *clk)
 
	if (clk) {

arch/arm/mach-sa1100/clock.c void clk_disable(struct clk *clk) · line 86

Quoted 5 lines
arch/arm/mach-sa1100/clock.c +1 -1
 	spin_lock_irqsave(&clocks_lock, flags);
 	if (--clk->enabled == 0)
-		clk_gpio27_disable();
+		clk->ops->disable(clk);
 	spin_unlock_irqrestore(&clocks_lock, flags);
	}

arch/arm/mach-sa1100/clock.c void clk_disable(struct clk *clk) · line 93

Quoted 8 lines
arch/arm/mach-sa1100/clock.c +2 -1
 }
 EXPORT_SYMBOL(clk_disable);
 
 unsigned long clk_get_rate(struct clk *clk)
 {
-	return 3686400;
+	unsigned long rate;
+
	unsigned long rate = 0;
	if (clk) {

arch/arm/mach-sa1100/clock.c void clk_disable(struct clk *clk) · line 96

Quoted 3 lines
arch/arm/mach-sa1100/clock.c +3 -0
+	rate = clk->rate;
+	if (clk->ops->getrate)
+		rate = clk->ops->getrate(clk);
	}
Quoted 10 lines
arch/arm/mach-sa1100/clock.c +8 -0
+
+	return rate;
 }
 EXPORT_SYMBOL(clk_get_rate);
+
+const struct clkops clk_gpio27_ops = {
+	.enable		= clk_gpio27_enable,
+	.disable	= clk_gpio27_disable,
+};
+

arch/arm/mach-sa1100/clock.c void clk_disable(struct clk *clk) · line 116

Quoted 10 lines
arch/arm/mach-sa1100/clock.c +10 -0
+static void clk_dummy_enable(struct clk *clk) { }
+static void clk_dummy_disable(struct clk *clk) { }
+
+const struct clkops clk_dummy_ops = {
+	.enable		= clk_dummy_enable,
+	.disable	= clk_dummy_disable,
+};
+
+static DEFINE_CLK(gpio27, &clk_gpio27_ops, 3686400);
+static DEFINE_CLK(dummy, &clk_dummy_ops, 0);

Get rid of the dummy clock...

arch/arm/mach-sa1100/clock.c void clk_disable(struct clk *clk) · line 120

Quoted 4 lines
arch/arm/mach-sa1100/clock.c +4 -0
+
+static struct clk_lookup sa11xx_clkregs[] = {
+	INIT_CLKREG(&clk_gpio27, "sa1111.0", NULL),
+	INIT_CLKREG(&clk_dummy, "sa1100-rtc", NULL),
	CLKDEV_INIT("sa1100-rtc", NULL, NULL),

arch/arm/mach-sa1100/clock.c void clk_disable(struct clk *clk) · line 129

Quoted 9 lines
arch/arm/mach-sa1100/clock.c +9 -0
+};
+
+static int __init sa11xx_clk_init(void)
+{
+	clkdev_add_table(sa11xx_clkregs, ARRAY_SIZE(sa11xx_clkregs));
+	return 0;
+}
+
+postcore_initcall(sa11xx_clk_init);

Why postcore? There's nothing stopping this being done earlier.

#12 lore raw

PATCH 5/7 ARM: pxa: add rtc dummy clock

In reply to Haojian Zhuang

sa1100-rtc driver could be shared among sa1100/pxa/mmp series silicon. Since clk is used in mmp series silicon, add dummy clock support in pxa also.

---
 arch/arm/mach-pxa/pxa25x.c |    1 +
 arch/arm/mach-pxa/pxa27x.c |    1 +
 arch/arm/mach-pxa/pxa3xx.c |    1 +
 arch/arm/mach-pxa/pxa95x.c |    1 +
 4 files changed, 4 insertions(+), 0 deletions(-)
arch/arm/mach-pxa/pxa25x.c +1 -0
index 91e4f6c..8d8c3ef 100644
@@ -209,6 +209,7 @@ static struct clk_lookup pxa25x_clkregs[] = {
 	INIT_CLKREG(&clk_pxa25x_gpio11, NULL, "GPIO11_CLK"),
 	INIT_CLKREG(&clk_pxa25x_gpio12, NULL, "GPIO12_CLK"),
 	INIT_CLKREG(&clk_pxa25x_mem, "pxa2xx-pcmcia", NULL),
+	INIT_CLKREG(&clk_dummy, "pxa25x-rtc", NULL),
 };
 
 static struct clk_lookup pxa25x_hwuart_clkreg =
arch/arm/mach-pxa/pxa27x.c +1 -0
index 345fa7b..f95977f 100644
@@ -230,6 +230,7 @@ static struct clk_lookup pxa27x_clkregs[] = {
 	INIT_CLKREG(&clk_pxa27x_im, NULL, "IMCLK"),
 	INIT_CLKREG(&clk_pxa27x_memc, NULL, "MEMCLK"),
 	INIT_CLKREG(&clk_pxa27x_mem, "pxa2xx-pcmcia", NULL),
+	INIT_CLKREG(&clk_dummy, "pxa25x-rtc", NULL),
 };
 
 #ifdef CONFIG_PM
arch/arm/mach-pxa/pxa3xx.c +1 -0
index d2437a8..4b01eee 100644
@@ -89,6 +89,7 @@ static struct clk_lookup pxa3xx_clkregs[] = {
 	INIT_CLKREG(&clk_pxa3xx_mmc2, "pxa2xx-mci.1", NULL),
 	INIT_CLKREG(&clk_pxa3xx_smemc, "pxa2xx-pcmcia", NULL),
 	INIT_CLKREG(&clk_pxa3xx_gpio, "pxa-gpio", NULL),
+	INIT_CLKREG(&clk_dummy, "pxa25x-rtc", NULL),
 };
 
 #ifdef CONFIG_PM
arch/arm/mach-pxa/pxa95x.c +1 -0
index b41e9f3..0273801 100644
@@ -231,6 +231,7 @@ static struct clk_lookup pxa95x_clkregs[] = {
 	INIT_CLKREG(&clk_pxa95x_pwm0, "pxa27x-pwm.0", NULL),
 	INIT_CLKREG(&clk_pxa95x_pwm1, "pxa27x-pwm.1", NULL),
 	INIT_CLKREG(&clk_pxa95x_gpio, "pxa-gpio", NULL),
+	INIT_CLKREG(&clk_dummy, "pxa25x-rtc", NULL),
 };
 
 void __init pxa95x_init_irq(void)
Signature · 1 line
1.7.0.4
#13 lore raw

PATCH 6/7 rtc: sa1100: enable clk support

In reply to Haojian Zhuang

Add clk support in sa1100 rtc.

---
 drivers/rtc/rtc-sa1100.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)
drivers/rtc/rtc-sa1100.c +14 -0
index 90425ce..f031f4d 100644
@@ -23,6 +23,7 @@
 
 #include <linux/platform_device.h>
 #include <linux/module.h>
+#include <linux/clk.h>
 #include <linux/rtc.h>
 #include <linux/init.h>
 #include <linux/io.h>
@@ -63,6 +64,7 @@ struct sa1100_rtc {
 	int			irq_1hz;
 	int			irq_alarm;
 	struct rtc_device	*rtc;
+	struct clk		*clk;
 
 	void __iomem		*reg_base;
 	void __iomem		*reg_rcnr;
@@ -306,6 +308,13 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 	if (!info)
 		return -ENOMEM;
 
+	info->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(info->clk)) {
+		dev_err(&pdev->dev, "failed to find rtc clock source\n");
+		ret = PTR_ERR(info->clk);
+		goto err_clk;
+	}
+	clk_prepare_enable(info->clk);
 	info->iobase = res->start;
 	info->iosize = resource_size(res);
 	info->irq_1hz = irq_1hz;
@@ -379,6 +388,9 @@ err_dev:
 	iounmap(info->reg_base);
 err_map:
 	platform_set_drvdata(pdev, NULL);
+	clk_disable_unprepare(info->clk);
+	clk_put(info->clk);
+err_clk:
 	kfree(info);
 	return ret;
 }
@@ -391,6 +403,8 @@ static int sa1100_rtc_remove(struct platform_device *pdev)
 		rtc_device_unregister(info->rtc);
 		platform_set_drvdata(pdev, NULL);
 		iounmap(info->reg_base);
+		clk_disable_unprepare(info->clk);
+		clk_put(info->clk);
 		kfree(info);
 	}
 
Signature · 1 line
1.7.0.4
#14 lore raw

PATCH 6/7 rtc: sa1100: enable clk support

Quoted 15 lines · Haojian Zhuang
drivers/rtc/rtc-sa1100.c +7 -0
@@ -306,6 +308,13 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
        if (!info)
                return -ENOMEM;
 
+       info->clk = clk_get(&pdev->dev, NULL);
+       if (IS_ERR(info->clk)) {
+               dev_err(&pdev->dev, "failed to find rtc clock source\n");
+               ret = PTR_ERR(info->clk);
+               goto err_clk;
+       }
+       clk_prepare_enable(info->clk);
        info->iobase = res->start;
        info->iosize = resource_size(res);
        info->irq_1hz = irq_1hz;

drivers/rtc/rtc-sa1100.c err_dev: · line 395

Quoted 8 lines
drivers/rtc/rtc-sa1100.c +3 -0
@@ -379,6 +388,9 @@ err_dev:
        iounmap(info->reg_base);
 err_map:
        platform_set_drvdata(pdev, NULL);
+       clk_disable_unprepare(info->clk);
+       clk_put(info->clk);
+err_clk:
        kfree(info);
        return ret;

I wonder whether it would be easier to just make the clk handling conditional here, like

	info->clk = clk_get(&pdev->dev, NULL);
	if (PTR_ERR(info->clk) == -ENOENT)
		info->clk = NULL;
	if (IS_ERR(info->clk)) {
		ret =  PTR_ERR(info->clk);
		goto err_clk;
	}

	if (info->clk)
		clk_prepare_enable(info->clk);
	...
	if (info->clk) {
		clk_disable_unprepare(info->clk);
		clk_put(info->clk);
	}

With this, you would no longer need to add dummy clocks in platforms that really have no clock handling.

Arnd

maintainer
#15 lore raw

PATCH 6/7 rtc: sa1100: enable clk support

Quoted 44 lines · Arnd Bergmann
Quoted 16 lines · Haojian Zhuang
(quoted diff) +7 -0
@@ -306,6 +308,13 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 ? ? ? ? if (!info)
 ? ? ? ? ? ? ? ? return -ENOMEM;
 
+ ? ? ? info->clk = clk_get(&pdev->dev, NULL);
+ ? ? ? if (IS_ERR(info->clk)) {
+ ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to find rtc clock source\n");
+ ? ? ? ? ? ? ? ret = PTR_ERR(info->clk);
+ ? ? ? ? ? ? ? goto err_clk;
+ ? ? ? }
+ ? ? ? clk_prepare_enable(info->clk);
 ? ? ? ? info->iobase = res->start;
 ? ? ? ? info->iosize = resource_size(res);
 ? ? ? ? info->irq_1hz = irq_1hz;
@@ -379,6 +388,9 @@ err_dev:
 ? ? ? ? iounmap(info->reg_base);
Quoted 7 lines

?err_map:
? ? ? ? platform_set_drvdata(pdev, NULL);
+ ? ? ? clk_disable_unprepare(info->clk);
+ ? ? ? clk_put(info->clk);
+err_clk:
? ? ? ? kfree(info);
? ? ? ? return ret;

I wonder whether it would be easier to just make the clk handling conditional here, like

? ? ? ?info->clk = clk_get(&pdev->dev, NULL);
? ? ? ?if (PTR_ERR(info->clk) == -ENOENT)
? ? ? ? ? ? ? ?info->clk = NULL;
? ? ? ?if (IS_ERR(info->clk)) {
? ? ? ? ? ? ? ?ret = ?PTR_ERR(info->clk);
? ? ? ? ? ? ? ?goto err_clk;
? ? ? ?}

? ? ? ?if (info->clk)
? ? ? ? ? ? ? ?clk_prepare_enable(info->clk);
? ? ? ?...
? ? ? ?if (info->clk) {
? ? ? ? ? ? ? ?clk_disable_unprepare(info->clk);
? ? ? ? ? ? ? ?clk_put(info->clk);
? ? ? ?}

Quoted 4 lines

With this, you would no longer need to add dummy clocks in platforms that really have no clock handling.

? ? ? ?Arnd

It sounds good. I'll update it.

Thanks
Haojian

#16 lore raw

PATCH 6/7 rtc: sa1100: enable clk support

In reply to Arnd Bergmann
Quoted 19 lines · Arnd Bergmann
Quoted 18 lines · Haojian Zhuang
drivers/rtc/rtc-sa1100.c +7 -0
@@ -306,6 +308,13 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
        if (!info)
                return -ENOMEM;
 
+       info->clk = clk_get(&pdev->dev, NULL);
+       if (IS_ERR(info->clk)) {
+               dev_err(&pdev->dev, "failed to find rtc clock source\n");
+               ret = PTR_ERR(info->clk);
+               goto err_clk;
+       }
+       clk_prepare_enable(info->clk);
        info->iobase = res->start;
        info->iosize = resource_size(res);
        info->irq_1hz = irq_1hz;
@@ -379,6 +388,9 @@ err_dev:
        iounmap(info->reg_base);
 err_map:
        platform_set_drvdata(pdev, NULL);
Quoted 8 lines

drivers/rtc/rtc-sa1100.c err_dev: · line 395

Quoted 5 lines
drivers/rtc/rtc-sa1100.c +3 -0
+       clk_disable_unprepare(info->clk);
+       clk_put(info->clk);
+err_clk:
        kfree(info);
        return ret;

I wonder whether it would be easier to just make the clk handling conditional here, like

Eww. No, just keep this hidden beneath the clk API. Return a NULL clock for this device, and ensure that all the standard clk API functions know internally that that means 'no action required'.

There's no need to pollute drivers with this platform specific knowledge.

#17 lore raw

PATCH 6/7 rtc: sa1100: enable clk support

Quoted 5 lines · Russell King - ARM Linux

Eww. No, just keep this hidden beneath the clk API. Return a NULL clock for this device, and ensure that all the standard clk API functions know internally that that means 'no action required'.

There's no need to pollute drivers with this platform specific knowledge.

Good point, that does sound much cleaner than my suggestion.

Thanks,

Arnd

#18 lore raw

PATCH 6/7 rtc: sa1100: enable clk support

In reply to Haojian Zhuang

drivers/rtc/rtc-sa1100.c static int sa1100_rtc_probe(struct platform_device *pdev) · line 317

Quoted 11 lines · Haojian Zhuang
drivers/rtc/rtc-sa1100.c +7 -0
@@ -306,6 +308,13 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 	if (!info)
 		return -ENOMEM;
 
+	info->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(info->clk)) {
+		dev_err(&pdev->dev, "failed to find rtc clock source\n");
+		ret = PTR_ERR(info->clk);
+		goto err_clk;
+	}
+	clk_prepare_enable(info->clk);

What about checking for errors from clk_prepare_enable() ?

I assume that this is the clock required to access the peripheral, rather than the timekeeping clock? If so, does it need to be kept enabled all the time the driver is probed, or can the clock be prepared & enabled and disabled & unprepared when the device is opened/closed ?

maintainer
#19 lore raw

PATCH 6/7 rtc: sa1100: enable clk support

Quoted 15 lines · Russell King - ARM Linux

Review static int sa1100_rtc_probe(struct platform_device *pdev) · line 317

Quoted 11 lines · Haojian Zhuang
(quoted diff) +7 -0
@@ -306,6 +308,13 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 ? ? ? if (!info)
 ? ? ? ? ? ? ? return -ENOMEM;
 
+ ? ? info->clk = clk_get(&pdev->dev, NULL);
+ ? ? if (IS_ERR(info->clk)) {
+ ? ? ? ? ? ? dev_err(&pdev->dev, "failed to find rtc clock source\n");
+ ? ? ? ? ? ? ret = PTR_ERR(info->clk);
+ ? ? ? ? ? ? goto err_clk;
+ ? ? }
+ ? ? clk_prepare_enable(info->clk);

What about checking for errors from clk_prepare_enable() ?

Quoted 5 lines

I assume that this is the clock required to access the peripheral, rather than the timekeeping clock? ?If so, does it need to be kept enabled all the time the driver is probed, or can the clock be prepared & enabled and disabled & unprepared when the device is opened/closed ?

Since some platform code is using RTC register in their suspend/resume routine, I have to always enable the clock of rtc module.

#20 lore raw

PATCH 6/7 rtc: sa1100: enable clk support

Quoted 17 lines · Haojian Zhuang
Quoted 15 lines · Russell King - ARM Linux

Review static int sa1100_rtc_probe(struct platform_device *pdev) · line 317

Quoted 11 lines · Haojian Zhuang
(quoted diff) +7 -0
@@ -306,6 +308,13 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 ? ? ? if (!info)
 ? ? ? ? ? ? ? return -ENOMEM;
 
+ ? ? info->clk = clk_get(&pdev->dev, NULL);
+ ? ? if (IS_ERR(info->clk)) {
+ ? ? ? ? ? ? dev_err(&pdev->dev, "failed to find rtc clock source\n");
+ ? ? ? ? ? ? ret = PTR_ERR(info->clk);
+ ? ? ? ? ? ? goto err_clk;
+ ? ? }
+ ? ? clk_prepare_enable(info->clk);

What about checking for errors from clk_prepare_enable() ?

Quoted 7 lines
Quoted 5 lines

I assume that this is the clock required to access the peripheral, rather than the timekeeping clock? ?If so, does it need to be kept enabled all the time the driver is probed, or can the clock be prepared & enabled and disabled & unprepared when the device is opened/closed ?

Since some platform code is using RTC register in their suspend/resume routine, I have to always enable the clock of rtc module.

So what happens if they don't have the RTC module loaded in their kernel? The proper solution to this is that they need to take care of that clock themselves and stop relying on a module being loaded or built-in.

#21 lore raw

PATCH 7/7 ARM: mmp: enable rtc

In reply to Haojian Zhuang

Enable sa1100 rtc driver on pxa910.

---
 arch/arm/mach-mmp/include/mach/pxa910.h    |    1 +
 arch/arm/mach-mmp/include/mach/regs-apbc.h |    1 +
 arch/arm/mach-mmp/pxa910.c                 |   27 +++++++++++++++++++++++++++
 arch/arm/mach-mmp/ttc_dkb.c                |    1 +
 4 files changed, 30 insertions(+), 0 deletions(-)
arch/arm/mach-mmp/include/mach/pxa910.h +1 -0
index 4de13ab..e2e1f1e 100644
@@ -22,6 +22,7 @@ extern struct pxa_device_desc pxa910_device_pwm4;
 extern struct pxa_device_desc pxa910_device_nand;
 
 extern struct platform_device pxa910_device_gpio;
+extern struct platform_device pxa910_device_rtc;
 
 static inline int pxa910_add_uart(int id)
 {
arch/arm/mach-mmp/include/mach/regs-apbc.h +1 -0
index 1a96585..8a37fb0 100644
@@ -57,6 +57,7 @@
 #define APBC_PXA910_SSP1	APBC_REG(0x01c)
 #define APBC_PXA910_SSP2	APBC_REG(0x020)
 #define APBC_PXA910_IPC		APBC_REG(0x024)
+#define APBC_PXA910_RTC		APBC_REG(0x028)
 #define APBC_PXA910_TWSI0	APBC_REG(0x02c)
 #define APBC_PXA910_KPC		APBC_REG(0x030)
 #define APBC_PXA910_TIMERS	APBC_REG(0x034)
arch/arm/mach-mmp/pxa910.c +27 -0
index 3241a25..d147265 100644
@@ -92,6 +92,7 @@ static APBC_CLK(pwm2, PXA910_PWM2, 1, 13000000);
 static APBC_CLK(pwm3, PXA910_PWM3, 1, 13000000);
 static APBC_CLK(pwm4, PXA910_PWM4, 1, 13000000);
 static APBC_CLK(gpio, PXA910_GPIO, 0, 13000000);
+static APBC_CLK(rtc, PXA910_RTC, 8, 32768);
 
 static APMU_CLK(nand, NAND, 0x19b, 156000000);
 static APMU_CLK(u2o, USB, 0x1b, 480000000);
@@ -109,6 +110,7 @@ static struct clk_lookup pxa910_clkregs[] = {
 	INIT_CLKREG(&clk_nand, "pxa3xx-nand", NULL),
 	INIT_CLKREG(&clk_gpio, "pxa-gpio", NULL),
 	INIT_CLKREG(&clk_u2o, "pxa-u2o", "U2OCLK"),
+	INIT_CLKREG(&clk_rtc, "pxa910-rtc", NULL),
 };
 
 static int __init pxa910_init(void)
@@ -183,3 +185,28 @@ struct platform_device pxa910_device_gpio = {
 	.num_resources	= ARRAY_SIZE(pxa910_resource_gpio),
 	.resource	= pxa910_resource_gpio,
 };
+
+static struct resource pxa910_resource_rtc[] = {
+	{
+		.start	= 0xd4010000,
+		.end	= 0xd401003f,
+		.flags	= IORESOURCE_MEM,
+	}, {
+		.start	= IRQ_PXA910_RTC_INT,
+		.end	= IRQ_PXA910_RTC_INT,
+		.name	= "rtc 1Hz",
+		.flags	= IORESOURCE_IRQ,
+	}, {
+		.start	= IRQ_PXA910_RTC_ALARM,
+		.end	= IRQ_PXA910_RTC_ALARM,
+		.name	= "rtc alarm",
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+struct platform_device pxa910_device_rtc = {
+	.name		= "pxa910-rtc",
+	.id		= -1,
+	.num_resources	= ARRAY_SIZE(pxa910_resource_rtc),
+	.resource	= pxa910_resource_rtc,
+};
arch/arm/mach-mmp/ttc_dkb.c +1 -0
index 5ac5d58..e72c709 100644
@@ -124,6 +124,7 @@ static struct platform_device ttc_dkb_device_onenand = {
 
 static struct platform_device *ttc_dkb_devices[] = {
 	&pxa910_device_gpio,
+	&pxa910_device_rtc,
 	&ttc_dkb_device_onenand,
 };
 
Signature · 1 line
1.7.0.4

A message that is not in the archive (sent off-list or not yet received): [email protected]

#23 lore raw

[HELP REQUIRED] : Andorid FIQ Debugger Porting issue on Kernel2.6.36

?
Dear All,

This is regarding Android FIQ Debugger porting for another Linux mobile with kernel2.6.36. I have successfully ported the fiq_debugger.c, fiq_glue.S and others and resolved the compilation issues. I have enabled all the configs as below:
CONFIG_FIQ=y
CONFIG_FIQ_GLUE=y
CONFIG_FIQ_DEBUGGER=y
CONFIG_FIQ_DEBUGGER_NO_SLEEP=y
CONFIG_FIQ_DEBUGGER_WAKEUP_IRQ_ALWAYS_ON=y
CONFIG_FIQ_DEBUGGER_CONSOLE=y

But when I load the kernel on the device, the fiq handler is not getting called after I press ENTER?key many times during the following prompt. <hit enter to activate fiq debugger>
---
---

What could be the problem?

Can anybody provide some pointers to debug further?

Thanks, Regards,
Pintu

#24 lore raw

cover share sa1100 rtc driver to arch-mmp

In reply to Haojian Zhuang
Quoted 2 lines · Haojian Zhuang

Remove the hardcode definition of rtc registers, use ioremap() instead. Append clk support for arch-mmp, so add dummy clk in arch-sa1100/arch-pxa.

looks gine

Best Regards,
J.

#25 lore raw

cover share sa1100 rtc driver to arch-mmp

On 02/21/2012 04:37 PM, Jean-Christophe PLAGNIOL-VILLARD :

Quoted 4 lines · Haojian Zhuang
Quoted 2 lines

Remove the hardcode definition of rtc registers, use ioremap() instead. Append clk support for arch-mmp, so add dummy clk in arch-sa1100/arch-pxa.

looks gine

gine, or fone, maybe?...

Quoted 8 lines

Best Regards,
J.

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

Signature · 1 line
Nicolas Ferre
#26 lore raw

cover share sa1100 rtc driver to arch-mmp

In reply to Haojian Zhuang
Quoted 2 lines · Haojian Zhuang

Remove the hardcode definition of rtc registers, use ioremap() instead. Append clk support for arch-mmp, so add dummy clk in arch-sa1100/arch-pxa.

What is the status of the drivers/rtc/rtc-pxa.c driver after this series? Is is still required for some pxa machines or can it get removed now? It seems a little strange to register two platform devices for the rtc on each pxa machine, with the same resources.

Arnd

maintainer
#27 lore raw

cover share sa1100 rtc driver to arch-mmp

Quoted 10 lines · Arnd Bergmann
Quoted 2 lines · Haojian Zhuang

Remove the hardcode definition of rtc registers, use ioremap() instead. Append clk support for arch-mmp, so add dummy clk in arch-sa1100/arch-pxa.

What is the status of the drivers/rtc/rtc-pxa.c driver after this series? Is is still required for some pxa machines or can it get removed now? It seems a little strange to register two platform devices for the rtc on each pxa machine, with the same resources.

? ? ? ?Arnd

Yes, it's very strange to support two RTC wrappers at the same time. So rtc-pxa is removed in pxa27x/pxa3xx after this patch series. It only exists in pxa95x.

Thanks
Haojian

#28 lore raw

cover share sa1100 rtc driver to arch-mmp

Quoted 5 lines · Haojian Zhuang
Quoted 4 lines · Arnd Bergmann
Quoted 2 lines · Haojian Zhuang

Remove the hardcode definition of rtc registers, use ioremap() instead. Append clk support for arch-mmp, so add dummy clk in arch-sa1100/arch-pxa.

Quoted 8 lines
Quoted 4 lines

What is the status of the drivers/rtc/rtc-pxa.c driver after this series? Is is still required for some pxa machines or can it get removed now? It seems a little strange to register two platform devices for the rtc on each pxa machine, with the same resources.

Yes, it's very strange to support two RTC wrappers at the same time. So rtc-pxa is removed in pxa27x/pxa3xx after this patch series. It only exists in pxa95x.

What is the reason for this? Wouldn't it be easier to move pxa95x over to use the same driver as well? I guess that can be a series for another time, but it seems like you are almost at the point where you can remove the other driver entirely.

Arnd

maintainer
#29 lore raw

cover share sa1100 rtc driver to arch-mmp

Quoted 15 lines · Arnd Bergmann
Quoted 5 lines · Haojian Zhuang
Quoted 4 lines · Arnd Bergmann
Quoted 2 lines · Haojian Zhuang

Remove the hardcode definition of rtc registers, use ioremap() instead. Append clk support for arch-mmp, so add dummy clk in arch-sa1100/arch-pxa.

Quoted 8 lines
Quoted 4 lines

What is the status of the drivers/rtc/rtc-pxa.c driver after this series? Is is still required for some pxa machines or can it get removed now? It seems a little strange to register two platform devices for the rtc on each pxa machine, with the same resources.

Yes, it's very strange to support two RTC wrappers at the same time. So rtc-pxa is removed in pxa27x/pxa3xx after this patch series. It only exists in pxa95x.

Quoted 6 lines

What is the reason for this? Wouldn't it be easier to move pxa95x over to use the same driver as well? I guess that can be a series for another time, but it seems like you are almost at the point where you can remove the other driver entirely.

? ? ? ?Arnd

I can remove the rtc-pxa support entirely. I still want to leave this since this may be the only one choice in the future silicons.

Since some boards with pxa27x/pxa3xx are accessing RCNR (the old rtc wrapper) directly, I use rtc-sa1100 to keep consistently. In pxa95x, nobody is using the old rtc wrapper directly. So I only use it in pxa95x.

Quoted 1 line

From another view, there're more features in new wrapper. It should be

better to overcome the overflow of RTC time.

Best Regards
Haojian

maintainer
#30 lore raw

cover share sa1100 rtc driver to arch-mmp

Quoted 7 lines · Haojian Zhuang

I can remove the rtc-pxa support entirely. I still want to leave this since this may be the only one choice in the future silicons.

Since some boards with pxa27x/pxa3xx are accessing RCNR (the old rtc wrapper) directly, I use rtc-sa1100 to keep consistently. In pxa95x, nobody is using the old rtc wrapper directly. So I only use it in pxa95x.

No, I don't agree there.

The PXA RTC silicon support 2 different set of registers to access time :
- RCNR (rtc-sa1100)
- RYCR/RDCR (rtc-pxa)

The rtc-pxa driver was created to provide support for boards using the RYCR/RDCR rather than RCNR.

Therefore I'm against removing rtc-pxa, unless there is a driver for pxa, which provides RTC using RYCR/RDCR registers.

We had that discussion a long time ago, in [1].

Cheers.

--
Robert

[1] Linux Arm Kernel Mailing List, 2008/09/30

    http://lists.arm.linux.org.uk/lurker/message/20080930.193554.96feb95f.en.html
maintainer
#31 lore raw

cover share sa1100 rtc driver to arch-mmp

Quoted 16 lines · Robert Jarzmik
Quoted 7 lines · Haojian Zhuang

I can remove the rtc-pxa support entirely. I still want to leave this since this may be the only one choice in the future silicons.

Since some boards with pxa27x/pxa3xx are accessing RCNR (the old rtc wrapper) directly, I use rtc-sa1100 to keep consistently. In pxa95x, nobody is using the old rtc wrapper directly. So I only use it in pxa95x.

No, I don't agree there.

The PXA RTC silicon support 2 different set of registers to access time :
?- RCNR (rtc-sa1100)
?- RYCR/RDCR (rtc-pxa)

Quoted 8 lines

The rtc-pxa driver was created to provide support for boards using the RYCR/RDCR rather than RCNR.

Therefore I'm against removing rtc-pxa, unless there is a driver for pxa, which provides RTC using RYCR/RDCR registers.

We had that discussion a long time ago, in [1].

Hi Robert,

rtc-sa1100 driver is the only one choice on sa1100/pxa25x/pxa910/mmp2. And it's optional to pxa27x/pxa3xx/pxa95x.

Whatever we use RCNR or RDYR registers, they're in the same I/O range. It's impossible to define same resource range for two rtc device. We have to make a choice between rtc-sa1100 or rtc-pxa on pxa27x/pxa3xx/pxa95x.

I hadn't any plan to remove rtc-pxa driver. I only disabled it for pxa27x/pxa3xx. Please check the code of arch/arm/mach-pxa/sharpsl_pm.c. If we only enable rtc-pxa on pxa27x, we'll break the behavior on sharpsl since it's using rtc-sa1100. In order to keep code consistently, I disable rtc-pxa on pxa27x/pxa3xx. And use it on pxa95x since there's no legacy code issue.

Best Regards
Haojian

maintainer
#32 lore raw

cover share sa1100 rtc driver to arch-mmp

Quoted 7 lines · Haojian Zhuang

Hi Robert,

I hadn't any plan to remove rtc-pxa driver. I only disabled it for pxa27x/pxa3xx. Please check the code of arch/arm/mach-pxa/sharpsl_pm.c. If we only enable rtc-pxa on pxa27x, we'll break the behavior on sharpsl since it's using rtc-sa1100. In order to keep code consistently, I disable rtc-pxa on pxa27x/pxa3xx.

Then you'll break mioa701 board, which is pxa270 based. The RYCR/RDCR were introduced on PXA27x chips, so the rtc-pxa driver should work on pxa27x SoCs.

The behaviour should be as before, the config should be able to choose between rtc-pxa and rtc-sa1100 for a pxa27x based board. For sharpsl_pm, I suppose the rtc-sa1100 will be chosen in the .config, and for mioa701, it will be rtc-pxa.

Just to clear, I'm against removing "resources, clock" from pxa27x platform code which would break rtc-pxa driver on pxa27x. Now I don't care if you're talking about changing some "defconfigs".

Cheers.

Signature · 1 line
Robert
maintainer
#33 lore raw

cover share sa1100 rtc driver to arch-mmp

Quoted 13 lines · Robert Jarzmik
Quoted 7 lines · Haojian Zhuang

Hi Robert,

I hadn't any plan to remove rtc-pxa driver. I only disabled it for pxa27x/pxa3xx. Please check the code of arch/arm/mach-pxa/sharpsl_pm.c. If we only enable rtc-pxa on pxa27x, we'll break the behavior on sharpsl since it's using rtc-sa1100. In order to keep code consistently, I disable rtc-pxa on pxa27x/pxa3xx.

Then you'll break mioa701 board, which is pxa270 based. The RYCR/RDCR were introduced on PXA27x chips, so the rtc-pxa driver should work on pxa27x SoCs.

Quoted 9 lines

The behaviour should be as before, the config should be able to choose between rtc-pxa and rtc-sa1100 for a pxa27x based board. For sharpsl_pm, I suppose the rtc-sa1100 will be chosen in the .config, and for mioa701, it will be rtc-pxa.

Just to clear, I'm against removing "resources, clock" from pxa27x platform code which would break rtc-pxa driver on pxa27x. Now I don't care if you're talking about changing some "defconfigs".

How about to move the rtc-sa1100 or rtc-pxa device register from pxa27x.c to board file? Then we'll be both happy.

Thanks
Haojian

#34 lore raw

cover share sa1100 rtc driver to arch-mmp

Quoted 6 lines · Haojian Zhuang
Quoted 5 lines · Robert Jarzmik

The behaviour should be as before, the config should be able to choose between rtc-pxa and rtc-sa1100 for a pxa27x based board. For sharpsl_pm, I suppose the rtc-sa1100 will be chosen in the .config, and for mioa701, it will be rtc-pxa.

Quoted 7 lines
Quoted 4 lines

Just to clear, I'm against removing "resources, clock" from pxa27x platform code which would break rtc-pxa driver on pxa27x. Now I don't care if you're talking about changing some "defconfigs".

How about to move the rtc-sa1100 or rtc-pxa device register from pxa27x.c to board file? Then we'll be both happy.

How about stopping thinking about "one or other" and start thinking "both" ? The RTC library can and does cope with more than one RTC, and each RTC is exposed uniquely to userspace. So there shouldn't be any problem in keeping both around, or even registering both together.

You just have to make sure you don't use both of them together as their open functions will want to claim the same IRQ. That's not really a problem because that should provide exclusivity between the two.

maintainer
#35 lore raw

cover share sa1100 rtc driver to arch-mmp

Quoted 15 lines · Russell King - ARM Linux
Quoted 6 lines · Haojian Zhuang
Quoted 5 lines · Robert Jarzmik

The behaviour should be as before, the config should be able to choose between rtc-pxa and rtc-sa1100 for a pxa27x based board. For sharpsl_pm, I suppose the rtc-sa1100 will be chosen in the .config, and for mioa701, it will be rtc-pxa.

Quoted 7 lines
Quoted 4 lines

Just to clear, I'm against removing "resources, clock" from pxa27x platform code which would break rtc-pxa driver on pxa27x. Now I don't care if you're talking about changing some "defconfigs".

How about to move the rtc-sa1100 or rtc-pxa device register from pxa27x.c to board file? Then we'll be both happy.

Quoted 8 lines

How about stopping thinking about "one or other" and start thinking "both" ? The RTC library can and does cope with more than one RTC, and each RTC is exposed uniquely to userspace. ?So there shouldn't be any problem in keeping both around, or even registering both together.

You just have to make sure you don't use both of them together as their open functions will want to claim the same IRQ. ?That's not really a problem because that should provide exclusivity between the two.

Resource is bind with platform device. We'll meet resource conflict if we register them both.

Thanks
Haojian

#36 lore raw

cover share sa1100 rtc driver to arch-mmp

Quoted 22 lines · Haojian Zhuang
Quoted 20 lines · Russell King - ARM Linux
Quoted 6 lines · Haojian Zhuang
Quoted 5 lines · Robert Jarzmik

The behaviour should be as before, the config should be able to choose between rtc-pxa and rtc-sa1100 for a pxa27x based board. For sharpsl_pm, I suppose the rtc-sa1100 will be chosen in the .config, and for mioa701, it will be rtc-pxa.

Quoted 7 lines
Quoted 4 lines

Just to clear, I'm against removing "resources, clock" from pxa27x platform code which would break rtc-pxa driver on pxa27x. Now I don't care if you're talking about changing some "defconfigs".

How about to move the rtc-sa1100 or rtc-pxa device register from pxa27x.c to board file? Then we'll be both happy.

How about stopping thinking about "one or other" and start thinking "both" ? The RTC library can and does cope with more than one RTC, and each RTC is exposed uniquely to userspace. ?So there shouldn't be any problem in keeping both around, or even registering both together.

Quoted 6 lines
Quoted 3 lines

You just have to make sure you don't use both of them together as their open functions will want to claim the same IRQ. ?That's not really a problem because that should provide exclusivity between the two.

Resource is bind with platform device. We'll meet resource conflict if we register them both.

Ok, in that case we need patch 3 split up such that we can move forward and get some of the build errors fixed.

That probably means that the first half of patch 3 should be providing the IRQs and only the IRQs to the driver, and the driver making use of that information. Nothing more than that - not even the differing register offsets.

The next stage would be to introduce the different device names, and use that to chose the different register layouts - but still using a hard coded base address. That will involve adding a new define for this to both arch/arm/mach-sa1100/include/mach/SA-1100.h and arch/arm/mach-pxa/include/mach/regs-rtc.h. At the same time, this patch should move the register definitions from those headers into the sa1100 RTC driver, and get rid of the virtual address definitions for these registers.

The clocking issues can also be sorted out as well without solving the base address issue - and that gets most of this patch set ready for merging without causing Robert any problems.

I would like to see this ASAP so that the current build regression with PXA can be fixed.

#37 lore raw

cover share sa1100 rtc driver to arch-mmp

Quoted 38 lines · Russell King - ARM Linux
Quoted 22 lines · Haojian Zhuang
Quoted 20 lines · Russell King - ARM Linux
Quoted 6 lines · Haojian Zhuang
Quoted 5 lines · Robert Jarzmik

The behaviour should be as before, the config should be able to choose between rtc-pxa and rtc-sa1100 for a pxa27x based board. For sharpsl_pm, I suppose the rtc-sa1100 will be chosen in the .config, and for mioa701, it will be rtc-pxa.

Quoted 7 lines
Quoted 4 lines

Just to clear, I'm against removing "resources, clock" from pxa27x platform code which would break rtc-pxa driver on pxa27x. Now I don't care if you're talking about changing some "defconfigs".

How about to move the rtc-sa1100 or rtc-pxa device register from pxa27x.c to board file? Then we'll be both happy.

How about stopping thinking about "one or other" and start thinking "both" ? The RTC library can and does cope with more than one RTC, and each RTC is exposed uniquely to userspace. ?So there shouldn't be any problem in keeping both around, or even registering both together.

Quoted 6 lines
Quoted 3 lines

You just have to make sure you don't use both of them together as their open functions will want to claim the same IRQ. ?That's not really a problem because that should provide exclusivity between the two.

Resource is bind with platform device. We'll meet resource conflict if we register them both.

Ok, in that case we need patch 3 split up such that we can move forward and get some of the build errors fixed.

That probably means that the first half of patch 3 should be providing the IRQs and only the IRQs to the driver, and the driver making use of that information. Nothing more than that - not even the differing register offsets.

Quoted 7 lines

The next stage would be to introduce the different device names, and use that to chose the different register layouts - but still using a hard coded base address. That will involve adding a new define for this to both arch/arm/mach-sa1100/include/mach/SA-1100.h and arch/arm/mach-pxa/include/mach/regs-rtc.h. At the same time, this patch should move the register definitions from those headers into the sa1100 RTC driver, and get rid of the virtual address definitions for these registers.

Note: I think that should be done even though sharpsl_pm and miao both access the RTC registers directly, even though it means duplicating the definitions.

In the long run, someone needs to look at whether sharpsl_pm can be made to use the alarmtimer stuff (kernel/time/alarmtimer.c) but that's a project for someone with a Zaurus to do. No one else can do it.

Quoted 11 lines

The clocking issues can also be sorted out as well without solving the base address issue - and that gets most of this patch set ready for merging without causing Robert any problems.

I would like to see this ASAP so that the current build regression with PXA can be fixed.

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

maintainer
#38 lore raw

cover share sa1100 rtc driver to arch-mmp

In reply to Russell King - ARM Linux
Quoted 38 lines · Russell King - ARM Linux
Quoted 22 lines · Haojian Zhuang
Quoted 20 lines · Russell King - ARM Linux
Quoted 6 lines · Haojian Zhuang
Quoted 5 lines · Robert Jarzmik

The behaviour should be as before, the config should be able to choose between rtc-pxa and rtc-sa1100 for a pxa27x based board. For sharpsl_pm, I suppose the rtc-sa1100 will be chosen in the .config, and for mioa701, it will be rtc-pxa.

Quoted 7 lines
Quoted 4 lines

Just to clear, I'm against removing "resources, clock" from pxa27x platform code which would break rtc-pxa driver on pxa27x. Now I don't care if you're talking about changing some "defconfigs".

How about to move the rtc-sa1100 or rtc-pxa device register from pxa27x.c to board file? Then we'll be both happy.

How about stopping thinking about "one or other" and start thinking "both" ? The RTC library can and does cope with more than one RTC, and each RTC is exposed uniquely to userspace. ?So there shouldn't be any problem in keeping both around, or even registering both together.

Quoted 6 lines
Quoted 3 lines

You just have to make sure you don't use both of them together as their open functions will want to claim the same IRQ. ?That's not really a problem because that should provide exclusivity between the two.

Resource is bind with platform device. We'll meet resource conflict if we register them both.

Ok, in that case we need patch 3 split up such that we can move forward and get some of the build errors fixed.

That probably means that the first half of patch 3 should be providing the IRQs and only the IRQs to the driver, and the driver making use of that information. ?Nothing more than that - not even the differing register offsets.

Quoted 7 lines

The next stage would be to introduce the different device names, and use that to chose the different register layouts - but still using a hard coded base address. ?That will involve adding a new define for this to both arch/arm/mach-sa1100/include/mach/SA-1100.h and arch/arm/mach-pxa/include/mach/regs-rtc.h. ?At the same time, this patch should move the register definitions from those headers into the sa1100 RTC driver, and get rid of the virtual address definitions for these registers.

OK. I can keep the hard coded register definition and format patch right now. The rtc register base of arch-sa1100, arch-pxa, arch-mmp are different. It means
that I need use #ifdef CONFIG_ARCH to distinguish different arch. But I have a question that removing #ifdef CONFIG_ARCH in driver code is our target, isn't it?

Quoted 7 lines

The clocking issues can also be sorted out as well without solving the base address issue - and that gets most of this patch set ready for merging without causing Robert any problems.

I would like to see this ASAP so that the current build regression with PXA can be fixed.

OK. I'll move the clock operation in open()/close() function.

#39 lore raw

cover share sa1100 rtc driver to arch-mmp

Quoted 6 lines · Haojian Zhuang

OK. I can keep the hard coded register definition and format patch right now. The rtc register base of arch-sa1100, arch-pxa, arch-mmp are different. It means
that I need use #ifdef CONFIG_ARCH to distinguish different arch. But I have a question that removing #ifdef CONFIG_ARCH in driver code is our target, isn't it?

Yes, but we can't do that until the pxa-rtc vs sa1100-rtc issue is resolved.

#40 lore raw

cover share sa1100 rtc driver to arch-mmp

In reply to Haojian Zhuang
Quoted 2 lines · Haojian Zhuang

Remove the hardcode definition of rtc registers, use ioremap() instead. Append clk support for arch-mmp, so add dummy clk in arch-sa1100/arch-pxa.

One final comment: we need to get this sorted, because as things currently stand, my builds for PXA are failing:

http://www.arm.linux.org.uk/developer/build/result.php?type=build&idx=185

drivers/rtc/rtc-sa1100.c: In function 'sa1100_rtc_open':
drivers/rtc/rtc-sa1100.c:163: error: 'IRQ_RTC1Hz' undeclared (first use in this function) drivers/rtc/rtc-sa1100.c:163: error: (Each undeclared identifier is reported only once drivers/rtc/rtc-sa1100.c:163: error: for each function it appears in.) drivers/rtc/rtc-sa1100.c:169: error: 'IRQ_RTCAlrm' undeclared (first use in this function) drivers/rtc/rtc-sa1100.c: In function 'sa1100_rtc_release':
drivers/rtc/rtc-sa1100.c:192: error: 'IRQ_RTCAlrm' undeclared (first use in this function) drivers/rtc/rtc-sa1100.c:193: error: 'IRQ_RTC1Hz' undeclared (first use in this function) drivers/rtc/rtc-sa1100.c: In function 'sa1100_rtc_suspend':
drivers/rtc/rtc-sa1100.c:341: error: 'IRQ_RTCAlrm' undeclared (first use in this function) drivers/rtc/rtc-sa1100.c: In function 'sa1100_rtc_resume':
drivers/rtc/rtc-sa1100.c:348: error: 'IRQ_RTCAlrm' undeclared (first use in this function)

which means PXA will be broken in the next merge window. As that's mostly down to Rob's IRQ stuff, the simple solution is to get the conversion to use platform resources in place ASAP, and then start worrying about the differences between the various SoCs.

What I don't want to be doing is going into the next merge window with this in its current state.