[PATCH RE-SEND 0/6] rtc: rtc-s3c: Fix rtc-s3c

[PATCH RE-SEND 0/6] rtc: rtc-s3c: Fix rtc-s3c

[Kukjin Kim]

This patches already have been submitted regarding mailing list.
Now...I'm re-sending this for handling by Andrew Morton.

Thanks.

[PATCH 1/6] rtc: rtc-s3c: Fix access unit from byte to word on RTCCON
[PATCH 2/6] rtc: rtc-s3c: Fix setting missing field of getalarm
[PATCH 3/6] rtc: rtc-s3c: Fix on support RTC Alarm
[PATCH 4/6] rtc: rtc-s3c: Fix debug message format on RTC
[PATCH 5/6] rtc: rtc-s3c: Fix on RTC initialization method
[PATCH 6/6] rtc: rtc-s3c: Add rtc_valid_tm in s3c_rtc_gettime()

[PATCH 1/6] rtc: rtc-s3c: Fix access unit from byte to word on RTCCON

[Kukjin Kim]

From: Changhwan Youn <chaos.youn@samsung.com>

S3C2410_RTCCON of TYPE_S3C64XX RTC should be read/written by
readw and writew, because TYPE_S3C64XX RTC uses bit 8 and 9.
And TYPE_S3C2410 RTC also can access it by readw and writew.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
[atul.dahiya at samsung.com: tested on smdk2416]
Tested-by: Atul Dahiya <atul.dahiya@samsung.com>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
Cc: Ben Dooks <ben-linux@fluff.org>
---
 drivers/rtc/rtc-s3c.c |   36 ++++++++++++++++++------------------
 1 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index f57a87f..4a0b875 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -100,7 +100,7 @@ static int s3c_rtc_setpie(struct device *dev, int enabled)
 	spin_lock_irq(&s3c_rtc_pie_lock);
 
 	if (s3c_rtc_cpu_type == TYPE_S3C64XX) {
-		tmp = readb(s3c_rtc_base + S3C2410_RTCCON);
+		tmp = readw(s3c_rtc_base + S3C2410_RTCCON);
 		tmp &= ~S3C64XX_RTCCON_TICEN;
 
 		if (enabled)
@@ -318,7 +318,7 @@ static int s3c_rtc_proc(struct device *dev, struct seq_file *seq)
 	unsigned int ticnt;
 
 	if (s3c_rtc_cpu_type == TYPE_S3C64XX) {
-		ticnt = readb(s3c_rtc_base + S3C2410_RTCCON);
+		ticnt = readw(s3c_rtc_base + S3C2410_RTCCON);
 		ticnt &= S3C64XX_RTCCON_TICEN;
 	} else {
 		ticnt = readb(s3c_rtc_base + S3C2410_TICNT);
@@ -391,11 +391,11 @@ static void s3c_rtc_enable(struct platform_device *pdev, int en)
 		return;
 
 	if (!en) {
-		tmp = readb(base + S3C2410_RTCCON);
+		tmp = readw(base + S3C2410_RTCCON);
 		if (s3c_rtc_cpu_type == TYPE_S3C64XX)
 			tmp &= ~S3C64XX_RTCCON_TICEN;
 		tmp &= ~S3C2410_RTCCON_RTCEN;
-		writeb(tmp, base + S3C2410_RTCCON);
+		writew(tmp, base + S3C2410_RTCCON);
 
 		if (s3c_rtc_cpu_type == TYPE_S3C2410) {
 			tmp = readb(base + S3C2410_TICNT);
@@ -405,25 +405,25 @@ static void s3c_rtc_enable(struct platform_device *pdev, int en)
 	} else {
 		/* re-enable the device, and check it is ok */
 
-		if ((readb(base+S3C2410_RTCCON) & S3C2410_RTCCON_RTCEN) == 0){
+		if ((readw(base+S3C2410_RTCCON) & S3C2410_RTCCON_RTCEN) == 0){
 			dev_info(&pdev->dev, "rtc disabled, re-enabling\n");
 
-			tmp = readb(base + S3C2410_RTCCON);
-			writeb(tmp|S3C2410_RTCCON_RTCEN, base+S3C2410_RTCCON);
+			tmp = readw(base + S3C2410_RTCCON);
+			writew(tmp|S3C2410_RTCCON_RTCEN, base+S3C2410_RTCCON);
 		}
 
-		if ((readb(base + S3C2410_RTCCON) & S3C2410_RTCCON_CNTSEL)){
+		if ((readw(base + S3C2410_RTCCON) & S3C2410_RTCCON_CNTSEL)){
 			dev_info(&pdev->dev, "removing RTCCON_CNTSEL\n");
 
-			tmp = readb(base + S3C2410_RTCCON);
-			writeb(tmp& ~S3C2410_RTCCON_CNTSEL, base+S3C2410_RTCCON);
+			tmp = readw(base + S3C2410_RTCCON);
+			writew(tmp& ~S3C2410_RTCCON_CNTSEL, base+S3C2410_RTCCON);
 		}
 
-		if ((readb(base + S3C2410_RTCCON) & S3C2410_RTCCON_CLKRST)){
+		if ((readw(base + S3C2410_RTCCON) & S3C2410_RTCCON_CLKRST)){
 			dev_info(&pdev->dev, "removing RTCCON_CLKRST\n");
 
-			tmp = readb(base + S3C2410_RTCCON);
-			writeb(tmp & ~S3C2410_RTCCON_CLKRST, base+S3C2410_RTCCON);
+			tmp = readw(base + S3C2410_RTCCON);
+			writew(tmp & ~S3C2410_RTCCON_CLKRST, base+S3C2410_RTCCON);
 		}
 	}
 }
@@ -514,8 +514,8 @@ static int __devinit s3c_rtc_probe(struct platform_device *pdev)
 
 	s3c_rtc_enable(pdev, 1);
 
- 	pr_debug("s3c2410_rtc: RTCCON=%02x\n",
-		 readb(s3c_rtc_base + S3C2410_RTCCON));
+	pr_debug("s3c2410_rtc: RTCCON=%02x\n",
+		 readw(s3c_rtc_base + S3C2410_RTCCON));
 
 	device_init_wakeup(&pdev->dev, 1);
 
@@ -578,7 +578,7 @@ static int s3c_rtc_suspend(struct platform_device *pdev, pm_message_t state)
 	/* save TICNT for anyone using periodic interrupts */
 	ticnt_save = readb(s3c_rtc_base + S3C2410_TICNT);
 	if (s3c_rtc_cpu_type == TYPE_S3C64XX) {
-		ticnt_en_save = readb(s3c_rtc_base + S3C2410_RTCCON);
+		ticnt_en_save = readw(s3c_rtc_base + S3C2410_RTCCON);
 		ticnt_en_save &= S3C64XX_RTCCON_TICEN;
 	}
 	s3c_rtc_enable(pdev, 0);
@@ -596,8 +596,8 @@ static int s3c_rtc_resume(struct platform_device *pdev)
 	s3c_rtc_enable(pdev, 1);
 	writeb(ticnt_save, s3c_rtc_base + S3C2410_TICNT);
 	if (s3c_rtc_cpu_type == TYPE_S3C64XX && ticnt_en_save) {
-		tmp = readb(s3c_rtc_base + S3C2410_RTCCON);
-		writeb(tmp | ticnt_en_save, s3c_rtc_base + S3C2410_RTCCON);
+		tmp = readw(s3c_rtc_base + S3C2410_RTCCON);
+		writew(tmp | ticnt_en_save, s3c_rtc_base + S3C2410_RTCCON);
 	}
 
 	if (device_may_wakeup(&pdev->dev))
-- 
1.6.2.5

[PATCH 2/6] rtc: rtc-s3c: Fix setting missing field of getalarm

[Kukjin Kim]

From: Changhwan Youn <chaos.youn@samsung.com>

Current s3c_rtc_getalarm() sets missing field of alarm time with 0xff.
But this value should be -1 according to drivers/rtc/interface.c.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
Acked-by: Ben Dooks <ben-linux@fluff.org>
---
 drivers/rtc/rtc-s3c.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index 4a0b875..fd08876 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -242,34 +242,34 @@ static int s3c_rtc_getalarm(struct device *dev, struct rtc_wkalrm *alrm)
 	if (alm_en & S3C2410_RTCALM_SECEN)
 		alm_tm->tm_sec = bcd2bin(alm_tm->tm_sec);
 	else
-		alm_tm->tm_sec = 0xff;
+		alm_tm->tm_sec = -1;
 
 	if (alm_en & S3C2410_RTCALM_MINEN)
 		alm_tm->tm_min = bcd2bin(alm_tm->tm_min);
 	else
-		alm_tm->tm_min = 0xff;
+		alm_tm->tm_min = -1;
 
 	if (alm_en & S3C2410_RTCALM_HOUREN)
 		alm_tm->tm_hour = bcd2bin(alm_tm->tm_hour);
 	else
-		alm_tm->tm_hour = 0xff;
+		alm_tm->tm_hour = -1;
 
 	if (alm_en & S3C2410_RTCALM_DAYEN)
 		alm_tm->tm_mday = bcd2bin(alm_tm->tm_mday);
 	else
-		alm_tm->tm_mday = 0xff;
+		alm_tm->tm_mday = -1;
 
 	if (alm_en & S3C2410_RTCALM_MONEN) {
 		alm_tm->tm_mon = bcd2bin(alm_tm->tm_mon);
 		alm_tm->tm_mon -= 1;
 	} else {
-		alm_tm->tm_mon = 0xff;
+		alm_tm->tm_mon = -1;
 	}
 
 	if (alm_en & S3C2410_RTCALM_YEAREN)
 		alm_tm->tm_year = bcd2bin(alm_tm->tm_year);
 	else
-		alm_tm->tm_year = 0xffff;
+		alm_tm->tm_year = -1;
 
 	return 0;
 }
-- 
1.6.2.5

[PATCH 3/6] rtc: rtc-s3c: Fix on support RTC Alarm

[Kukjin Kim]

From: Changhwan Youn <chaos.youn@samsung.com>

The alarm_irq_enable function should be implemented to support RTC alarm.
And fixes tab instead of white space abound proc field.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
Acked-by: Ben Dooks <ben-linux@fluff.org>
---
 drivers/rtc/rtc-s3c.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index fd08876..39fa5b0 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -379,7 +379,8 @@ static const struct rtc_class_ops s3c_rtcops = {
 	.set_alarm	= s3c_rtc_setalarm,
 	.irq_set_freq	= s3c_rtc_setfreq,
 	.irq_set_state	= s3c_rtc_setpie,
-	.proc	        = s3c_rtc_proc,
+	.proc		= s3c_rtc_proc,
+	.alarm_irq_enable = s3c_rtc_setaie,
 };
 
 static void s3c_rtc_enable(struct platform_device *pdev, int en)
-- 
1.6.2.5

[PATCH 4/6] rtc: rtc-s3c: Fix debug message format on RTC

[Kukjin Kim]

This patch fixes debug message format on rtc-s3c.

Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
Acked-by: Ben Dooks <ben-linux@fluff.org>
---
 drivers/rtc/rtc-s3c.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index 39fa5b0..276b7c1 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -171,8 +171,8 @@ static int s3c_rtc_gettime(struct device *dev, struct rtc_time *rtc_tm)
 		goto retry_get_time;
 	}
 
-	pr_debug("read time %02x.%02x.%02x %02x/%02x/%02x\n",
-		 rtc_tm->tm_year, rtc_tm->tm_mon, rtc_tm->tm_mday,
+	pr_debug("read time %04d.%02d.%02d %02d:%02d:%02d\n",
+		 1900 + rtc_tm->tm_year, rtc_tm->tm_mon, rtc_tm->tm_mday,
 		 rtc_tm->tm_hour, rtc_tm->tm_min, rtc_tm->tm_sec);
 
 	rtc_tm->tm_sec = bcd2bin(rtc_tm->tm_sec);
@@ -193,8 +193,8 @@ static int s3c_rtc_settime(struct device *dev, struct rtc_time *tm)
 	void __iomem *base = s3c_rtc_base;
 	int year = tm->tm_year - 100;
 
-	pr_debug("set time %02d.%02d.%02d %02d/%02d/%02d\n",
-		 tm->tm_year, tm->tm_mon, tm->tm_mday,
+	pr_debug("set time %04d.%02d.%02d %02d:%02d:%02d\n",
+		 1900 + tm->tm_year, tm->tm_mon, tm->tm_mday,
 		 tm->tm_hour, tm->tm_min, tm->tm_sec);
 
 	/* we get around y2k by simply not supporting it */
@@ -231,9 +231,9 @@ static int s3c_rtc_getalarm(struct device *dev, struct rtc_wkalrm *alrm)
 
 	alrm->enabled = (alm_en & S3C2410_RTCALM_ALMEN) ? 1 : 0;
 
-	pr_debug("read alarm %02x %02x.%02x.%02x %02x/%02x/%02x\n",
+	pr_debug("read alarm %d, %04d.%02d.%02d %02d:%02d:%02d\n",
 		 alm_en,
-		 alm_tm->tm_year, alm_tm->tm_mon, alm_tm->tm_mday,
+		 1900 + alm_tm->tm_year, alm_tm->tm_mon, alm_tm->tm_mday,
 		 alm_tm->tm_hour, alm_tm->tm_min, alm_tm->tm_sec);
 
 
@@ -280,10 +280,10 @@ static int s3c_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
 	void __iomem *base = s3c_rtc_base;
 	unsigned int alrm_en;
 
-	pr_debug("s3c_rtc_setalarm: %d, %02x/%02x/%02x %02x.%02x.%02x\n",
+	pr_debug("s3c_rtc_setalarm: %d, %04d.%02d.%02d %02d:%02d:%02d\n",
 		 alrm->enabled,
-		 tm->tm_mday & 0xff, tm->tm_mon & 0xff, tm->tm_year & 0xff,
-		 tm->tm_hour & 0xff, tm->tm_min & 0xff, tm->tm_sec);
+		 1900 + tm->tm_year, tm->tm_mon, tm->tm_mday,
+		 tm->tm_hour, tm->tm_min, tm->tm_sec);
 
 
 	alrm_en = readb(base + S3C2410_RTCALM) & S3C2410_RTCALM_ALMEN;
-- 
1.6.2.5

[PATCH 5/6] rtc: rtc-s3c: Fix on RTC initialization method

[Kukjin Kim]

From: Changhwan Youn <chaos.youn@samsung.com>

This patch changes RTC initialization method on probe(). The
'rtc_valid_tm(tm)' can check whether RTC BCD is valid or not.
And should be changed the method of check because previous
method cannot validate RTC BCD registers properly.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
Cc: Ben Dooks <ben-linux@fluff.org>
---
 drivers/rtc/rtc-s3c.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index 276b7c1..51e7b25 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -453,8 +453,8 @@ static int __devexit s3c_rtc_remove(struct platform_device *dev)
 static int __devinit s3c_rtc_probe(struct platform_device *pdev)
 {
 	struct rtc_device *rtc;
+	struct rtc_time rtc_tm;
 	struct resource *res;
-	unsigned int tmp, i;
 	int ret;
 
 	pr_debug("%s: probe=%p\n", __func__, pdev);
@@ -535,11 +535,19 @@ static int __devinit s3c_rtc_probe(struct platform_device *pdev)
 
 	/* Check RTC Time */
 
-	for (i = S3C2410_RTCSEC; i <= S3C2410_RTCYEAR; i += 0x4) {
-		tmp = readb(s3c_rtc_base + i);
+	s3c_rtc_gettime(NULL, &rtc_tm);
 
-		if ((tmp & 0xf) > 0x9 || ((tmp >> 4) & 0xf) > 0x9)
-			writeb(0, s3c_rtc_base + i);
+	if (rtc_valid_tm(&rtc_tm)) {
+		rtc_tm.tm_year	= 100;
+		rtc_tm.tm_mon	= 0;
+		rtc_tm.tm_mday	= 1;
+		rtc_tm.tm_hour	= 0;
+		rtc_tm.tm_min	= 0;
+		rtc_tm.tm_sec	= 0;
+
+		s3c_rtc_settime(NULL, &rtc_tm);
+
+		dev_warn(&pdev->dev, "warning: invalid RTC value so initializing it\n");
 	}
 
 	if (s3c_rtc_cpu_type == TYPE_S3C64XX)
-- 
1.6.2.5

[PATCH 6/6] rtc: rtc-s3c: Add rtc_valid_tm in s3c_rtc_gettime()

[Kukjin Kim]

This patch adds "rtc_valid_tm" in s3c_rtc_gettime()
as per Wan ZongShun's suggestion.

Suggested-by: Wan ZongShun <mcuos.com@gmail.com>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
Cc: Ben Dooks <ben-linux@fluff.org>
---
 drivers/rtc/rtc-s3c.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index 51e7b25..7c7db81 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -185,7 +185,7 @@ static int s3c_rtc_gettime(struct device *dev, struct rtc_time *rtc_tm)
 	rtc_tm->tm_year += 100;
 	rtc_tm->tm_mon -= 1;
 
-	return 0;
+	return rtc_valid_tm(rtc_tm);
 }
 
 static int s3c_rtc_settime(struct device *dev, struct rtc_time *tm)
-- 
1.6.2.5

[PATCH 1/6] rtc: rtc-s3c: Fix access unit from byte to word on RTCCON

[MyungJoo Ham]

On Fri, Oct 8, 2010 at 8:41 AM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> From: Changhwan Youn <chaos.youn@samsung.com>
>
> S3C2410_RTCCON of TYPE_S3C64XX RTC should be read/written by
> readw and writew, because TYPE_S3C64XX RTC uses bit 8 and 9.
> And TYPE_S3C2410 RTC also can access it by readw and writew.
>
> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> [atul.dahiya at samsung.com: tested on smdk2416]
> Tested-by: Atul Dahiya <atul.dahiya@samsung.com>
> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> Cc: Ben Dooks <ben-linux@fluff.org>
> ---
> ?drivers/rtc/rtc-s3c.c | ? 36 ++++++++++++++++++------------------
> ?1 files changed, 18 insertions(+), 18 deletions(-)

Hello,


Sorry for a late reply...


Anyway, I have a small question in this rtc-s3c.c driver.

Is there any reason to use read/write b/w to access registers of rtc-s3c?

Why don't we use readl/writel when accessing registers in this drivers
and just forget which registers require at least 8 or 16 or 32 bits?

In fact, it appears that readw/writew accesses 16bits, not 32bits in
ARM machines, which may incur problems with TICCNT/CURTICCNT
registers.


>
> diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
> index f57a87f..4a0b875 100644
> --- a/drivers/rtc/rtc-s3c.c
> +++ b/drivers/rtc/rtc-s3c.c
> @@ -100,7 +100,7 @@ static int s3c_rtc_setpie(struct device *dev, int enabled)
> ? ? ? ?spin_lock_irq(&s3c_rtc_pie_lock);
>
> ? ? ? ?if (s3c_rtc_cpu_type == TYPE_S3C64XX) {
> - ? ? ? ? ? ? ? tmp = readb(s3c_rtc_base + S3C2410_RTCCON);
> + ? ? ? ? ? ? ? tmp = readw(s3c_rtc_base + S3C2410_RTCCON);
> ? ? ? ? ? ? ? ?tmp &= ~S3C64XX_RTCCON_TICEN;
>
> ? ? ? ? ? ? ? ?if (enabled)
> @@ -318,7 +318,7 @@ static int s3c_rtc_proc(struct device *dev, struct seq_file *seq)
> ? ? ? ?unsigned int ticnt;
>
> ? ? ? ?if (s3c_rtc_cpu_type == TYPE_S3C64XX) {
> - ? ? ? ? ? ? ? ticnt = readb(s3c_rtc_base + S3C2410_RTCCON);
> + ? ? ? ? ? ? ? ticnt = readw(s3c_rtc_base + S3C2410_RTCCON);
> ? ? ? ? ? ? ? ?ticnt &= S3C64XX_RTCCON_TICEN;
> ? ? ? ?} else {
> ? ? ? ? ? ? ? ?ticnt = readb(s3c_rtc_base + S3C2410_TICNT);
> @@ -391,11 +391,11 @@ static void s3c_rtc_enable(struct platform_device *pdev, int en)
> ? ? ? ? ? ? ? ?return;
>
> ? ? ? ?if (!en) {
> - ? ? ? ? ? ? ? tmp = readb(base + S3C2410_RTCCON);
> + ? ? ? ? ? ? ? tmp = readw(base + S3C2410_RTCCON);
> ? ? ? ? ? ? ? ?if (s3c_rtc_cpu_type == TYPE_S3C64XX)
> ? ? ? ? ? ? ? ? ? ? ? ?tmp &= ~S3C64XX_RTCCON_TICEN;
> ? ? ? ? ? ? ? ?tmp &= ~S3C2410_RTCCON_RTCEN;
> - ? ? ? ? ? ? ? writeb(tmp, base + S3C2410_RTCCON);
> + ? ? ? ? ? ? ? writew(tmp, base + S3C2410_RTCCON);
>
> ? ? ? ? ? ? ? ?if (s3c_rtc_cpu_type == TYPE_S3C2410) {
> ? ? ? ? ? ? ? ? ? ? ? ?tmp = readb(base + S3C2410_TICNT);
> @@ -405,25 +405,25 @@ static void s3c_rtc_enable(struct platform_device *pdev, int en)
> ? ? ? ?} else {
> ? ? ? ? ? ? ? ?/* re-enable the device, and check it is ok */
>
> - ? ? ? ? ? ? ? if ((readb(base+S3C2410_RTCCON) & S3C2410_RTCCON_RTCEN) == 0){
> + ? ? ? ? ? ? ? if ((readw(base+S3C2410_RTCCON) & S3C2410_RTCCON_RTCEN) == 0){
> ? ? ? ? ? ? ? ? ? ? ? ?dev_info(&pdev->dev, "rtc disabled, re-enabling\n");
>
> - ? ? ? ? ? ? ? ? ? ? ? tmp = readb(base + S3C2410_RTCCON);
> - ? ? ? ? ? ? ? ? ? ? ? writeb(tmp|S3C2410_RTCCON_RTCEN, base+S3C2410_RTCCON);
> + ? ? ? ? ? ? ? ? ? ? ? tmp = readw(base + S3C2410_RTCCON);
> + ? ? ? ? ? ? ? ? ? ? ? writew(tmp|S3C2410_RTCCON_RTCEN, base+S3C2410_RTCCON);
> ? ? ? ? ? ? ? ?}
>
> - ? ? ? ? ? ? ? if ((readb(base + S3C2410_RTCCON) & S3C2410_RTCCON_CNTSEL)){
> + ? ? ? ? ? ? ? if ((readw(base + S3C2410_RTCCON) & S3C2410_RTCCON_CNTSEL)){
> ? ? ? ? ? ? ? ? ? ? ? ?dev_info(&pdev->dev, "removing RTCCON_CNTSEL\n");
>
> - ? ? ? ? ? ? ? ? ? ? ? tmp = readb(base + S3C2410_RTCCON);
> - ? ? ? ? ? ? ? ? ? ? ? writeb(tmp& ~S3C2410_RTCCON_CNTSEL, base+S3C2410_RTCCON);
> + ? ? ? ? ? ? ? ? ? ? ? tmp = readw(base + S3C2410_RTCCON);
> + ? ? ? ? ? ? ? ? ? ? ? writew(tmp& ~S3C2410_RTCCON_CNTSEL, base+S3C2410_RTCCON);
> ? ? ? ? ? ? ? ?}
>
> - ? ? ? ? ? ? ? if ((readb(base + S3C2410_RTCCON) & S3C2410_RTCCON_CLKRST)){
> + ? ? ? ? ? ? ? if ((readw(base + S3C2410_RTCCON) & S3C2410_RTCCON_CLKRST)){
> ? ? ? ? ? ? ? ? ? ? ? ?dev_info(&pdev->dev, "removing RTCCON_CLKRST\n");
>
> - ? ? ? ? ? ? ? ? ? ? ? tmp = readb(base + S3C2410_RTCCON);
> - ? ? ? ? ? ? ? ? ? ? ? writeb(tmp & ~S3C2410_RTCCON_CLKRST, base+S3C2410_RTCCON);
> + ? ? ? ? ? ? ? ? ? ? ? tmp = readw(base + S3C2410_RTCCON);
> + ? ? ? ? ? ? ? ? ? ? ? writew(tmp & ~S3C2410_RTCCON_CLKRST, base+S3C2410_RTCCON);
> ? ? ? ? ? ? ? ?}
> ? ? ? ?}
> ?}
> @@ -514,8 +514,8 @@ static int __devinit s3c_rtc_probe(struct platform_device *pdev)
>
> ? ? ? ?s3c_rtc_enable(pdev, 1);
>
> - ? ? ? pr_debug("s3c2410_rtc: RTCCON=%02x\n",
> - ? ? ? ? ? ? ? ?readb(s3c_rtc_base + S3C2410_RTCCON));
> + ? ? ? pr_debug("s3c2410_rtc: RTCCON=%02x\n",
> + ? ? ? ? ? ? ? ?readw(s3c_rtc_base + S3C2410_RTCCON));
>
> ? ? ? ?device_init_wakeup(&pdev->dev, 1);
>
> @@ -578,7 +578,7 @@ static int s3c_rtc_suspend(struct platform_device *pdev, pm_message_t state)
> ? ? ? ?/* save TICNT for anyone using periodic interrupts */
> ? ? ? ?ticnt_save = readb(s3c_rtc_base + S3C2410_TICNT);
> ? ? ? ?if (s3c_rtc_cpu_type == TYPE_S3C64XX) {
> - ? ? ? ? ? ? ? ticnt_en_save = readb(s3c_rtc_base + S3C2410_RTCCON);
> + ? ? ? ? ? ? ? ticnt_en_save = readw(s3c_rtc_base + S3C2410_RTCCON);
> ? ? ? ? ? ? ? ?ticnt_en_save &= S3C64XX_RTCCON_TICEN;
> ? ? ? ?}
> ? ? ? ?s3c_rtc_enable(pdev, 0);
> @@ -596,8 +596,8 @@ static int s3c_rtc_resume(struct platform_device *pdev)
> ? ? ? ?s3c_rtc_enable(pdev, 1);
> ? ? ? ?writeb(ticnt_save, s3c_rtc_base + S3C2410_TICNT);
> ? ? ? ?if (s3c_rtc_cpu_type == TYPE_S3C64XX && ticnt_en_save) {
> - ? ? ? ? ? ? ? tmp = readb(s3c_rtc_base + S3C2410_RTCCON);
> - ? ? ? ? ? ? ? writeb(tmp | ticnt_en_save, s3c_rtc_base + S3C2410_RTCCON);
> + ? ? ? ? ? ? ? tmp = readw(s3c_rtc_base + S3C2410_RTCCON);
> + ? ? ? ? ? ? ? writew(tmp | ticnt_en_save, s3c_rtc_base + S3C2410_RTCCON);
> ? ? ? ?}
>
> ? ? ? ?if (device_may_wakeup(&pdev->dev))
> --
> 1.6.2.5
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>



-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

[PATCH 1/6] rtc: rtc-s3c: Fix access unit from byte to word on RTCCON

[Kukjin Kim]

MyungJoo Ham wrote:
> 
> On Fri, Oct 8, 2010 at 8:41 AM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> > From: Changhwan Youn <chaos.youn@samsung.com>
> >
> > S3C2410_RTCCON of TYPE_S3C64XX RTC should be read/written by
> > readw and writew, because TYPE_S3C64XX RTC uses bit 8 and 9.
> > And TYPE_S3C2410 RTC also can access it by readw and writew.
> >
> > Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> > [atul.dahiya at samsung.com: tested on smdk2416]
> > Tested-by: Atul Dahiya <atul.dahiya@samsung.com>
> > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> > Cc: Ben Dooks <ben-linux@fluff.org>
> > ---
> > ?drivers/rtc/rtc-s3c.c | ? 36 ++++++++++++++++++------------------
> > ?1 files changed, 18 insertions(+), 18 deletions(-)
> 
> Hello,
> 
Hi,
> 
> Sorry for a late reply...
> 
Yeah, too late :-(

> Anyway, I have a small question in this rtc-s3c.c driver.
> 
> Is there any reason to use read/write b/w to access registers of rtc-s3c?
> 
See the git comment.

> Why don't we use readl/writel when accessing registers in this drivers

I don't know why we should use readl/writel for all case...
even though we can use just word or byte access.

> and just forget which registers require at least 8 or 16 or 32 bits?
> 
> In fact, it appears that readw/writew accesses 16bits, not 32bits in

Yes...

> ARM machines, which may incur problems with TICCNT/CURTICCNT
> registers.
> 
I can't get your comment...

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 1/6] rtc: rtc-s3c: Fix access unit from byte to word on RTCCON

[MyungJoo Ham]

On Wed, Oct 27, 2010 at 4:58 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> MyungJoo Ham wrote:
>>
>> On Fri, Oct 8, 2010 at 8:41 AM, Kukjin Kim <kgene.kim@samsung.com> wrote:
>> > From: Changhwan Youn <chaos.youn@samsung.com>
>> >
>> > S3C2410_RTCCON of TYPE_S3C64XX RTC should be read/written by
>> > readw and writew, because TYPE_S3C64XX RTC uses bit 8 and 9.
>> > And TYPE_S3C2410 RTC also can access it by readw and writew.
>> >
>> > Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
>> > [atul.dahiya at samsung.com: tested on smdk2416]
>> > Tested-by: Atul Dahiya <atul.dahiya@samsung.com>
>> > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
>> > Cc: Ben Dooks <ben-linux@fluff.org>
>> > ---
>> > ?drivers/rtc/rtc-s3c.c | ? 36 ++++++++++++++++++------------------
>> > ?1 files changed, 18 insertions(+), 18 deletions(-)
>>
>> Hello,
>>
> Hi,
>>
>> Sorry for a late reply...
>>
> Yeah, too late :-(
>
>> Anyway, I have a small question in this rtc-s3c.c driver.
>>
>> Is there any reason to use read/write b/w to access registers of rtc-s3c?
>>
> See the git comment.
>
>> Why don't we use readl/writel when accessing registers in this drivers
>
> I don't know why we should use readl/writel for all case...
> even though we can use just word or byte access.
>
>> and just forget which registers require at least 8 or 16 or 32 bits?
>>
>> In fact, it appears that readw/writew accesses 16bits, not 32bits in
>
> Yes...
>
>> ARM machines, which may incur problems with TICCNT(S3C2410_TICNT in the code)/CURTICCNT
>> registers.
>>
> I can't get your comment...

It is because TICCNT and CURTICCNT RTC registers of S5PC210 require
32bit access according to the manual while this patch still uses 16bit
accesses for them.

Besides, ALMYEAR/BCDYEAR(S3C2410_RTCYEAR) requires at least 16bit access.


This issue was found today while testing suspend-to-mem with s5pc210
and rtc-wakeup loop. My guess is that saving and restoring only 8 bits
of TICNT at suspend/resume function incurred the instable
suspend-to-mem (kernel hangs with about 1/100 probability) Using
readl/writel for TICNT solved the kernel hang issue.

>
> Thanks.
>
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
>
>



-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

[PATCH 1/6] rtc: rtc-s3c: Fix access unit from byte to word on RTCCON

[Kukjin Kim]

MyungJoo Ham wrote:
> 
> On Wed, Oct 27, 2010 at 4:58 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> > MyungJoo Ham wrote:
> >>
> >> On Fri, Oct 8, 2010 at 8:41 AM, Kukjin Kim <kgene.kim@samsung.com>
wrote:
> >> > From: Changhwan Youn <chaos.youn@samsung.com>
> >> >
> >> > S3C2410_RTCCON of TYPE_S3C64XX RTC should be read/written by
> >> > readw and writew, because TYPE_S3C64XX RTC uses bit 8 and 9.
> >> > And TYPE_S3C2410 RTC also can access it by readw and writew.
> >> >
> >> > Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> >> > [atul.dahiya at samsung.com: tested on smdk2416]
> >> > Tested-by: Atul Dahiya <atul.dahiya@samsung.com>
> >> > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> >> > Cc: Ben Dooks <ben-linux@fluff.org>
> >> > ---
> >> > ?drivers/rtc/rtc-s3c.c | ? 36 ++++++++++++++++++------------------
> >> > ?1 files changed, 18 insertions(+), 18 deletions(-)
> >>
> >> Hello,
> >>
> > Hi,
> >>
> >> Sorry for a late reply...
> >>
> > Yeah, too late :-(
> >
> >> Anyway, I have a small question in this rtc-s3c.c driver.
> >>
> >> Is there any reason to use read/write b/w to access registers of
rtc-s3c?
> >>
> > See the git comment.
> >
> >> Why don't we use readl/writel when accessing registers in this drivers
> >
> > I don't know why we should use readl/writel for all case...
> > even though we can use just word or byte access.
> >
> >> and just forget which registers require at least 8 or 16 or 32 bits?
> >>
> >> In fact, it appears that readw/writew accesses 16bits, not 32bits in
> >
> > Yes...
> >
> >> ARM machines, which may incur problems with TICCNT(S3C2410_TICNT in the
> code)/CURTICCNT
> >> registers.
> >>
> > I can't get your comment...
> 
> It is because TICCNT and CURTICCNT RTC registers of S5PC210 require
> 32bit access according to the manual while this patch still uses 16bit
> accesses for them.
> 
> Besides, ALMYEAR/BCDYEAR(S3C2410_RTCYEAR) requires at least 16bit access.
> 
> 
> This issue was found today while testing suspend-to-mem with s5pc210
> and rtc-wakeup loop. My guess is that saving and restoring only 8 bits
> of TICNT at suspend/resume function incurred the instable
> suspend-to-mem (kernel hangs with about 1/100 probability) Using
> readl/writel for TICNT solved the kernel hang issue.
> 
I got it.
You mean need to fix readb(TICNT) of s3c_rtc_suspend() and
s3c_rtc_resume()...
Ok...but it's different some kind of bug fix with this patch.
And...need to check side effect to all of Samsung SoCs.

Hmm...and first of all, not supported S5PV310/S5PC210 PM in mainline now.

Anyway if any bug, we can fix it during -rc.
I always welcome to bug fix to our codes. :-)

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.