[PATCHv3 0/5] rtc: s3c: Refactoring s3c-rtc driver and support Exynos3250 RTC

[PATCHv3 0/5] rtc: s3c: Refactoring s3c-rtc driver and support Exynos3250 RTC

[Chanwoo Choi]

This patchset clean up codes to improve readability as following and support
the RTC of Exynos3250 SoC.
- Remove global variables and then use new s3c_rtc structure
- Remove warn message with checking checkpatch script
- Use variant structure according to SoC type instead of legacy enum variable(s3c_cpu_type)

Changes from v2:
- Drop the checking statement of 'info->base' because it is un-needed

Changes from v1:
- Fix NULL pointer panic of s3c_rtc_settime()
- Check info->base value to protect NULL pointer panic

Chanwoo Choi (5):
  rtc: s3c: Define s3c_rtc structure to remove global variables.
  rtc: s3c: Remove warning message when checking coding style with checkpatch script
  rtc: s3c: Add s3c_rtc_data structure to use variant data instead of s3c_cpu_type
  rtc: s3c: Add support for RTC of Exynos3250 SoC
  ARM: dts: Fix wrong compatible string of Exynos3250 RTC dt node

 Documentation/devicetree/bindings/rtc/s3c-rtc.txt |   3 +
 arch/arm/boot/dts/exynos3250.dtsi                 |   2 +-
 drivers/rtc/rtc-s3c.c                             | 851 ++++++++++++++--------
 3 files changed, 535 insertions(+), 321 deletions(-)

-- 
1.8.0

[PATCHv3 1/5] rtc: s3c: Define s3c_rtc structure to remove global variables.

[Chanwoo Choi]

This patch define s3c_rtc structure including necessary variables for S3C RTC
device instead of global variables. This patch improves the readability by
removing global variables.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Kukjin Kim <kgene.kim@samsung.com>
---
 drivers/rtc/rtc-s3c.c | 431 +++++++++++++++++++++++++-------------------------
 1 file changed, 216 insertions(+), 215 deletions(-)

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index 4958a36..2f71b13 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -43,125 +43,132 @@ struct s3c_rtc_drv_data {
 	int cpu_type;
 };
 
-/* I have yet to find an S3C implementation with more than one
- * of these rtc blocks in */
+struct s3c_rtc {
+	struct device *dev;
+	struct rtc_device *rtc;
+
+	void __iomem *base;
+	struct clk *rtc_clk;
+	bool enabled;
+
+	enum s3c_cpu_type cpu_type;
 
-static struct clk *rtc_clk;
-static void __iomem *s3c_rtc_base;
-static int s3c_rtc_alarmno;
-static int s3c_rtc_tickno;
-static enum s3c_cpu_type s3c_rtc_cpu_type;
+	int irq_alarm;
+	int irq_tick;
 
-static DEFINE_SPINLOCK(s3c_rtc_pie_lock);
+	spinlock_t pie_lock;
+	spinlock_t alarm_clk_lock;
 
-static void s3c_rtc_alarm_clk_enable(bool enable)
+	int ticnt_save, ticnt_en_save;
+	bool wake_en;
+};
+
+static void s3c_rtc_alarm_clk_enable(struct s3c_rtc *info, bool enable)
 {
-	static DEFINE_SPINLOCK(s3c_rtc_alarm_clk_lock);
-	static bool alarm_clk_enabled;
 	unsigned long irq_flags;
 
-	spin_lock_irqsave(&s3c_rtc_alarm_clk_lock, irq_flags);
+	spin_lock_irqsave(&info->alarm_clk_lock, irq_flags);
 	if (enable) {
-		if (!alarm_clk_enabled) {
-			clk_enable(rtc_clk);
-			alarm_clk_enabled = true;
+		if (!info->enabled) {
+			clk_enable(info->rtc_clk);
+			info->enabled = true;
 		}
 	} else {
-		if (alarm_clk_enabled) {
-			clk_disable(rtc_clk);
-			alarm_clk_enabled = false;
+		if (info->enabled) {
+			clk_disable(info->rtc_clk);
+			info->enabled = false;
 		}
 	}
-	spin_unlock_irqrestore(&s3c_rtc_alarm_clk_lock, irq_flags);
+	spin_unlock_irqrestore(&info->alarm_clk_lock, irq_flags);
 }
 
 /* IRQ Handlers */
-
 static irqreturn_t s3c_rtc_alarmirq(int irq, void *id)
 {
-	struct rtc_device *rdev = id;
+	struct s3c_rtc *info = (struct s3c_rtc *)id;
 
-	clk_enable(rtc_clk);
-	rtc_update_irq(rdev, 1, RTC_AF | RTC_IRQF);
+	clk_enable(info->rtc_clk);
+	rtc_update_irq(info->rtc, 1, RTC_AF | RTC_IRQF);
 
-	if (s3c_rtc_cpu_type == TYPE_S3C64XX)
-		writeb(S3C2410_INTP_ALM, s3c_rtc_base + S3C2410_INTP);
+	if (info->cpu_type == TYPE_S3C64XX)
+		writeb(S3C2410_INTP_ALM, info->base + S3C2410_INTP);
 
-	clk_disable(rtc_clk);
+	clk_disable(info->rtc_clk);
 
-	s3c_rtc_alarm_clk_enable(false);
+	s3c_rtc_alarm_clk_enable(info, false);
 
 	return IRQ_HANDLED;
 }
 
 static irqreturn_t s3c_rtc_tickirq(int irq, void *id)
 {
-	struct rtc_device *rdev = id;
+	struct s3c_rtc *info = (struct s3c_rtc *)id;
 
-	clk_enable(rtc_clk);
-	rtc_update_irq(rdev, 1, RTC_PF | RTC_IRQF);
+	clk_enable(info->rtc_clk);
+	rtc_update_irq(info->rtc, 1, RTC_PF | RTC_IRQF);
 
-	if (s3c_rtc_cpu_type == TYPE_S3C64XX)
-		writeb(S3C2410_INTP_TIC, s3c_rtc_base + S3C2410_INTP);
+	if (info->cpu_type == TYPE_S3C64XX)
+		writeb(S3C2410_INTP_TIC, info->base + S3C2410_INTP);
+
+	clk_disable(info->rtc_clk);
 
-	clk_disable(rtc_clk);
 	return IRQ_HANDLED;
 }
 
 /* Update control registers */
 static int s3c_rtc_setaie(struct device *dev, unsigned int enabled)
 {
+	struct s3c_rtc *info = dev_get_drvdata(dev);
 	unsigned int tmp;
 
-	dev_dbg(dev, "%s: aie=%d\n", __func__, enabled);
+	dev_dbg(info->dev, "%s: aie=%d\n", __func__, enabled);
 
-	clk_enable(rtc_clk);
-	tmp = readb(s3c_rtc_base + S3C2410_RTCALM) & ~S3C2410_RTCALM_ALMEN;
+	clk_enable(info->rtc_clk);
+	tmp = readb(info->base + S3C2410_RTCALM) & ~S3C2410_RTCALM_ALMEN;
 
 	if (enabled)
 		tmp |= S3C2410_RTCALM_ALMEN;
 
-	writeb(tmp, s3c_rtc_base + S3C2410_RTCALM);
-	clk_disable(rtc_clk);
+	writeb(tmp, info->base + S3C2410_RTCALM);
+	clk_disable(info->rtc_clk);
 
-	s3c_rtc_alarm_clk_enable(enabled);
+	s3c_rtc_alarm_clk_enable(info, enabled);
 
 	return 0;
 }
 
-static int s3c_rtc_setfreq(struct device *dev, int freq)
+static int s3c_rtc_setfreq(struct s3c_rtc *info, int freq)
 {
-	struct platform_device *pdev = to_platform_device(dev);
-	struct rtc_device *rtc_dev = platform_get_drvdata(pdev);
 	unsigned int tmp = 0;
 	int val;
 
 	if (!is_power_of_2(freq))
 		return -EINVAL;
 
-	clk_enable(rtc_clk);
-	spin_lock_irq(&s3c_rtc_pie_lock);
+	clk_enable(info->rtc_clk);
+	spin_lock_irq(&info->pie_lock);
 
-	if (s3c_rtc_cpu_type != TYPE_S3C64XX) {
-		tmp = readb(s3c_rtc_base + S3C2410_TICNT);
+	if (info->cpu_type != TYPE_S3C64XX) {
+		tmp = readb(info->base + S3C2410_TICNT);
 		tmp &= S3C2410_TICNT_ENABLE;
 	}
 
-	val = (rtc_dev->max_user_freq / freq) - 1;
+	val = (info->rtc->max_user_freq / freq) - 1;
 
-	if (s3c_rtc_cpu_type == TYPE_S3C2416 || s3c_rtc_cpu_type == TYPE_S3C2443) {
+	if (info->cpu_type == TYPE_S3C2416 || info->cpu_type == TYPE_S3C2443) {
 		tmp |= S3C2443_TICNT_PART(val);
-		writel(S3C2443_TICNT1_PART(val), s3c_rtc_base + S3C2443_TICNT1);
+		writel(S3C2443_TICNT1_PART(val), info->base + S3C2443_TICNT1);
 
-		if (s3c_rtc_cpu_type == TYPE_S3C2416)
-			writel(S3C2416_TICNT2_PART(val), s3c_rtc_base + S3C2416_TICNT2);
+		if (info->cpu_type == TYPE_S3C2416)
+			writel(S3C2416_TICNT2_PART(val),
+				info->base + S3C2416_TICNT2);
 	} else {
 		tmp |= val;
 	}
 
-	writel(tmp, s3c_rtc_base + S3C2410_TICNT);
-	spin_unlock_irq(&s3c_rtc_pie_lock);
-	clk_disable(rtc_clk);
+	writel(tmp, info->base + S3C2410_TICNT);
+	spin_unlock_irq(&info->pie_lock);
+	clk_disable(info->rtc_clk);
 
 	return 0;
 }
@@ -170,17 +177,17 @@ static int s3c_rtc_setfreq(struct device *dev, int freq)
 
 static int s3c_rtc_gettime(struct device *dev, struct rtc_time *rtc_tm)
 {
+	struct s3c_rtc *info = dev_get_drvdata(dev);
 	unsigned int have_retried = 0;
-	void __iomem *base = s3c_rtc_base;
 
-	clk_enable(rtc_clk);
+	clk_enable(info->rtc_clk);
  retry_get_time:
-	rtc_tm->tm_min  = readb(base + S3C2410_RTCMIN);
-	rtc_tm->tm_hour = readb(base + S3C2410_RTCHOUR);
-	rtc_tm->tm_mday = readb(base + S3C2410_RTCDATE);
-	rtc_tm->tm_mon  = readb(base + S3C2410_RTCMON);
-	rtc_tm->tm_year = readb(base + S3C2410_RTCYEAR);
-	rtc_tm->tm_sec  = readb(base + S3C2410_RTCSEC);
+	rtc_tm->tm_min  = readb(info->base + S3C2410_RTCMIN);
+	rtc_tm->tm_hour = readb(info->base + S3C2410_RTCHOUR);
+	rtc_tm->tm_mday = readb(info->base + S3C2410_RTCDATE);
+	rtc_tm->tm_mon  = readb(info->base + S3C2410_RTCMON);
+	rtc_tm->tm_year = readb(info->base + S3C2410_RTCYEAR);
+	rtc_tm->tm_sec  = readb(info->base + S3C2410_RTCSEC);
 
 	/* the only way to work out whether the system was mid-update
 	 * when we read it is to check the second counter, and if it
@@ -207,13 +214,14 @@ static int s3c_rtc_gettime(struct device *dev, struct rtc_time *rtc_tm)
 
 	rtc_tm->tm_mon -= 1;
 
-	clk_disable(rtc_clk);
+	clk_disable(info->rtc_clk);
+
 	return rtc_valid_tm(rtc_tm);
 }
 
 static int s3c_rtc_settime(struct device *dev, struct rtc_time *tm)
 {
-	void __iomem *base = s3c_rtc_base;
+	struct s3c_rtc *info = dev_get_drvdata(dev);
 	int year = tm->tm_year - 100;
 
 	dev_dbg(dev, "set time %04d.%02d.%02d %02d:%02d:%02d\n",
@@ -227,33 +235,35 @@ static int s3c_rtc_settime(struct device *dev, struct rtc_time *tm)
 		return -EINVAL;
 	}
 
-	clk_enable(rtc_clk);
-	writeb(bin2bcd(tm->tm_sec),  base + S3C2410_RTCSEC);
-	writeb(bin2bcd(tm->tm_min),  base + S3C2410_RTCMIN);
-	writeb(bin2bcd(tm->tm_hour), base + S3C2410_RTCHOUR);
-	writeb(bin2bcd(tm->tm_mday), base + S3C2410_RTCDATE);
-	writeb(bin2bcd(tm->tm_mon + 1), base + S3C2410_RTCMON);
-	writeb(bin2bcd(year), base + S3C2410_RTCYEAR);
-	clk_disable(rtc_clk);
+	clk_enable(info->rtc_clk);
+
+	writeb(bin2bcd(tm->tm_sec),  info->base + S3C2410_RTCSEC);
+	writeb(bin2bcd(tm->tm_min),  info->base + S3C2410_RTCMIN);
+	writeb(bin2bcd(tm->tm_hour), info->base + S3C2410_RTCHOUR);
+	writeb(bin2bcd(tm->tm_mday), info->base + S3C2410_RTCDATE);
+	writeb(bin2bcd(tm->tm_mon + 1), info->base + S3C2410_RTCMON);
+	writeb(bin2bcd(year), info->base + S3C2410_RTCYEAR);
+
+	clk_disable(info->rtc_clk);
 
 	return 0;
 }
 
 static int s3c_rtc_getalarm(struct device *dev, struct rtc_wkalrm *alrm)
 {
+	struct s3c_rtc *info = dev_get_drvdata(dev);
 	struct rtc_time *alm_tm = &alrm->time;
-	void __iomem *base = s3c_rtc_base;
 	unsigned int alm_en;
 
-	clk_enable(rtc_clk);
-	alm_tm->tm_sec  = readb(base + S3C2410_ALMSEC);
-	alm_tm->tm_min  = readb(base + S3C2410_ALMMIN);
-	alm_tm->tm_hour = readb(base + S3C2410_ALMHOUR);
-	alm_tm->tm_mon  = readb(base + S3C2410_ALMMON);
-	alm_tm->tm_mday = readb(base + S3C2410_ALMDATE);
-	alm_tm->tm_year = readb(base + S3C2410_ALMYEAR);
+	clk_enable(info->rtc_clk);
+	alm_tm->tm_sec  = readb(info->base + S3C2410_ALMSEC);
+	alm_tm->tm_min  = readb(info->base + S3C2410_ALMMIN);
+	alm_tm->tm_hour = readb(info->base + S3C2410_ALMHOUR);
+	alm_tm->tm_mon  = readb(info->base + S3C2410_ALMMON);
+	alm_tm->tm_mday = readb(info->base + S3C2410_ALMDATE);
+	alm_tm->tm_year = readb(info->base + S3C2410_ALMYEAR);
 
-	alm_en = readb(base + S3C2410_RTCALM);
+	alm_en = readb(info->base + S3C2410_RTCALM);
 
 	alrm->enabled = (alm_en & S3C2410_RTCALM_ALMEN) ? 1 : 0;
 
@@ -297,65 +307,67 @@ static int s3c_rtc_getalarm(struct device *dev, struct rtc_wkalrm *alrm)
 	else
 		alm_tm->tm_year = -1;
 
-	clk_disable(rtc_clk);
+	clk_disable(info->rtc_clk);
 	return 0;
 }
 
 static int s3c_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
 {
+	struct s3c_rtc *info = dev_get_drvdata(dev);
 	struct rtc_time *tm = &alrm->time;
-	void __iomem *base = s3c_rtc_base;
 	unsigned int alrm_en;
 
-	clk_enable(rtc_clk);
+	clk_enable(info->rtc_clk);
 	dev_dbg(dev, "s3c_rtc_setalarm: %d, %04d.%02d.%02d %02d:%02d:%02d\n",
 		 alrm->enabled,
 		 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday,
 		 tm->tm_hour, tm->tm_min, tm->tm_sec);
 
-	alrm_en = readb(base + S3C2410_RTCALM) & S3C2410_RTCALM_ALMEN;
-	writeb(0x00, base + S3C2410_RTCALM);
+	alrm_en = readb(info->base + S3C2410_RTCALM) & S3C2410_RTCALM_ALMEN;
+	writeb(0x00, info->base + S3C2410_RTCALM);
 
 	if (tm->tm_sec < 60 && tm->tm_sec >= 0) {
 		alrm_en |= S3C2410_RTCALM_SECEN;
-		writeb(bin2bcd(tm->tm_sec), base + S3C2410_ALMSEC);
+		writeb(bin2bcd(tm->tm_sec), info->base + S3C2410_ALMSEC);
 	}
 
 	if (tm->tm_min < 60 && tm->tm_min >= 0) {
 		alrm_en |= S3C2410_RTCALM_MINEN;
-		writeb(bin2bcd(tm->tm_min), base + S3C2410_ALMMIN);
+		writeb(bin2bcd(tm->tm_min), info->base + S3C2410_ALMMIN);
 	}
 
 	if (tm->tm_hour < 24 && tm->tm_hour >= 0) {
 		alrm_en |= S3C2410_RTCALM_HOUREN;
-		writeb(bin2bcd(tm->tm_hour), base + S3C2410_ALMHOUR);
+		writeb(bin2bcd(tm->tm_hour), info->base + S3C2410_ALMHOUR);
 	}
 
 	dev_dbg(dev, "setting S3C2410_RTCALM to %08x\n", alrm_en);
 
-	writeb(alrm_en, base + S3C2410_RTCALM);
+	writeb(alrm_en, info->base + S3C2410_RTCALM);
 
 	s3c_rtc_setaie(dev, alrm->enabled);
 
-	clk_disable(rtc_clk);
+	clk_disable(info->rtc_clk);
+
 	return 0;
 }
 
 static int s3c_rtc_proc(struct device *dev, struct seq_file *seq)
 {
+	struct s3c_rtc *info = dev_get_drvdata(dev);
 	unsigned int ticnt;
 
-	clk_enable(rtc_clk);
-	if (s3c_rtc_cpu_type == TYPE_S3C64XX) {
-		ticnt = readw(s3c_rtc_base + S3C2410_RTCCON);
+	clk_enable(info->rtc_clk);
+	if (info->cpu_type == TYPE_S3C64XX) {
+		ticnt = readw(info->base + S3C2410_RTCCON);
 		ticnt &= S3C64XX_RTCCON_TICEN;
 	} else {
-		ticnt = readb(s3c_rtc_base + S3C2410_TICNT);
+		ticnt = readb(info->base + S3C2410_TICNT);
 		ticnt &= S3C2410_TICNT_ENABLE;
 	}
 
 	seq_printf(seq, "periodic_IRQ\t: %s\n", ticnt  ? "yes" : "no");
-	clk_disable(rtc_clk);
+	clk_disable(info->rtc_clk);
 	return 0;
 }
 
@@ -368,63 +380,61 @@ static const struct rtc_class_ops s3c_rtcops = {
 	.alarm_irq_enable = s3c_rtc_setaie,
 };
 
-static void s3c_rtc_enable(struct platform_device *pdev, int en)
+static void s3c_rtc_enable(struct s3c_rtc *info, int en)
 {
-	void __iomem *base = s3c_rtc_base;
 	unsigned int tmp;
 
-	if (s3c_rtc_base == NULL)
-		return;
-
-	clk_enable(rtc_clk);
+	clk_enable(info->rtc_clk);
 	if (!en) {
-		tmp = readw(base + S3C2410_RTCCON);
-		if (s3c_rtc_cpu_type == TYPE_S3C64XX)
+		tmp = readw(info->base + S3C2410_RTCCON);
+		if (info->cpu_type == TYPE_S3C64XX)
 			tmp &= ~S3C64XX_RTCCON_TICEN;
 		tmp &= ~S3C2410_RTCCON_RTCEN;
-		writew(tmp, base + S3C2410_RTCCON);
+		writew(tmp, info->base + S3C2410_RTCCON);
 
-		if (s3c_rtc_cpu_type != TYPE_S3C64XX) {
-			tmp = readb(base + S3C2410_TICNT);
+		if (info->cpu_type != TYPE_S3C64XX) {
+			tmp = readb(info->base + S3C2410_TICNT);
 			tmp &= ~S3C2410_TICNT_ENABLE;
-			writeb(tmp, base + S3C2410_TICNT);
+			writeb(tmp, info->base + S3C2410_TICNT);
 		}
 	} else {
 		/* re-enable the device, and check it is ok */
 
-		if ((readw(base+S3C2410_RTCCON) & S3C2410_RTCCON_RTCEN) == 0) {
-			dev_info(&pdev->dev, "rtc disabled, re-enabling\n");
+		if ((readw(info->base + S3C2410_RTCCON) & S3C2410_RTCCON_RTCEN) == 0) {
+			dev_info(info->dev, "rtc disabled, re-enabling\n");
 
-			tmp = readw(base + S3C2410_RTCCON);
+			tmp = readw(info->base + S3C2410_RTCCON);
 			writew(tmp | S3C2410_RTCCON_RTCEN,
-				base + S3C2410_RTCCON);
+				info->base + S3C2410_RTCCON);
 		}
 
-		if ((readw(base + S3C2410_RTCCON) & S3C2410_RTCCON_CNTSEL)) {
-			dev_info(&pdev->dev, "removing RTCCON_CNTSEL\n");
+		if ((readw(info->base + S3C2410_RTCCON) & S3C2410_RTCCON_CNTSEL)) {
+			dev_info(info->dev, "removing RTCCON_CNTSEL\n");
 
-			tmp = readw(base + S3C2410_RTCCON);
+			tmp = readw(info->base + S3C2410_RTCCON);
 			writew(tmp & ~S3C2410_RTCCON_CNTSEL,
-				base + S3C2410_RTCCON);
+				info->base + S3C2410_RTCCON);
 		}
 
-		if ((readw(base + S3C2410_RTCCON) & S3C2410_RTCCON_CLKRST)) {
-			dev_info(&pdev->dev, "removing RTCCON_CLKRST\n");
+		if ((readw(info->base + S3C2410_RTCCON) & S3C2410_RTCCON_CLKRST)) {
+			dev_info(info->dev, "removing RTCCON_CLKRST\n");
 
-			tmp = readw(base + S3C2410_RTCCON);
+			tmp = readw(info->base + S3C2410_RTCCON);
 			writew(tmp & ~S3C2410_RTCCON_CLKRST,
-				base + S3C2410_RTCCON);
+				info->base + S3C2410_RTCCON);
 		}
 	}
-	clk_disable(rtc_clk);
+	clk_disable(info->rtc_clk);
 }
 
-static int s3c_rtc_remove(struct platform_device *dev)
+static int s3c_rtc_remove(struct platform_device *pdev)
 {
-	s3c_rtc_setaie(&dev->dev, 0);
+	struct s3c_rtc *info = platform_get_drvdata(pdev);
+
+	s3c_rtc_setaie(info->dev, 0);
 
-	clk_unprepare(rtc_clk);
-	rtc_clk = NULL;
+	clk_unprepare(info->rtc_clk);
+	info->rtc_clk = NULL;
 
 	return 0;
 }
@@ -447,73 +457,85 @@ static inline int s3c_rtc_get_driver_data(struct platform_device *pdev)
 
 static int s3c_rtc_probe(struct platform_device *pdev)
 {
-	struct rtc_device *rtc;
+	struct s3c_rtc *info = NULL;
 	struct rtc_time rtc_tm;
 	struct resource *res;
 	int ret;
 	int tmp;
 
-	dev_dbg(&pdev->dev, "%s: probe=%p\n", __func__, pdev);
+	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
 
 	/* find the IRQs */
-
-	s3c_rtc_tickno = platform_get_irq(pdev, 1);
-	if (s3c_rtc_tickno < 0) {
+	info->irq_tick = platform_get_irq(pdev, 1);
+	if (info->irq_tick < 0) {
 		dev_err(&pdev->dev, "no irq for rtc tick\n");
-		return s3c_rtc_tickno;
+		return info->irq_tick;
 	}
 
-	s3c_rtc_alarmno = platform_get_irq(pdev, 0);
-	if (s3c_rtc_alarmno < 0) {
+	info->dev = &pdev->dev;
+	info->cpu_type = s3c_rtc_get_driver_data(pdev);
+	spin_lock_init(&info->pie_lock);
+	spin_lock_init(&info->alarm_clk_lock);
+
+	platform_set_drvdata(pdev, info);
+
+	info->irq_alarm = platform_get_irq(pdev, 0);
+	if (info->irq_alarm < 0) {
 		dev_err(&pdev->dev, "no irq for alarm\n");
-		return s3c_rtc_alarmno;
+		return info->irq_alarm;
 	}
 
 	dev_dbg(&pdev->dev, "s3c2410_rtc: tick irq %d, alarm irq %d\n",
-		 s3c_rtc_tickno, s3c_rtc_alarmno);
+		 info->irq_tick, info->irq_alarm);
 
 	/* get the memory region */
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	s3c_rtc_base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(s3c_rtc_base))
-		return PTR_ERR(s3c_rtc_base);
+	info->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(info->base))
+		return PTR_ERR(info->base);
 
-	rtc_clk = devm_clk_get(&pdev->dev, "rtc");
-	if (IS_ERR(rtc_clk)) {
+	info->rtc_clk = devm_clk_get(&pdev->dev, "rtc");
+	if (IS_ERR(info->rtc_clk)) {
 		dev_err(&pdev->dev, "failed to find rtc clock source\n");
-		ret = PTR_ERR(rtc_clk);
-		rtc_clk = NULL;
-		return ret;
+		return PTR_ERR(info->rtc_clk);
 	}
-
-	clk_prepare_enable(rtc_clk);
+	clk_prepare_enable(info->rtc_clk);
 
 	/* check to see if everything is setup correctly */
-
-	s3c_rtc_enable(pdev, 1);
+	s3c_rtc_enable(info, 1);
 
 	dev_dbg(&pdev->dev, "s3c2410_rtc: RTCCON=%02x\n",
-		 readw(s3c_rtc_base + S3C2410_RTCCON));
+		 readw(info->base + S3C2410_RTCCON));
 
 	device_init_wakeup(&pdev->dev, 1);
 
 	/* register RTC and exit */
-
-	rtc = devm_rtc_device_register(&pdev->dev, "s3c", &s3c_rtcops,
+	info->rtc = devm_rtc_device_register(&pdev->dev, "s3c", &s3c_rtcops,
 				  THIS_MODULE);
-
-	if (IS_ERR(rtc)) {
+	if (IS_ERR(info->rtc)) {
 		dev_err(&pdev->dev, "cannot attach rtc\n");
-		ret = PTR_ERR(rtc);
+		ret = PTR_ERR(info->rtc);
 		goto err_nortc;
 	}
 
-	s3c_rtc_cpu_type = s3c_rtc_get_driver_data(pdev);
+	ret = devm_request_irq(&pdev->dev, info->irq_alarm, s3c_rtc_alarmirq,
+			  0,  "s3c2410-rtc alarm", info);
+	if (ret) {
+		dev_err(&pdev->dev, "IRQ%d error %d\n", info->irq_alarm, ret);
+		goto err_nortc;
+	}
 
-	/* Check RTC Time */
+	ret = devm_request_irq(&pdev->dev, info->irq_tick, s3c_rtc_tickirq,
+			  0,  "s3c2410-rtc tick", info);
+	if (ret) {
+		dev_err(&pdev->dev, "IRQ%d error %d\n", info->irq_tick, ret);
+		goto err_nortc;
+	}
 
-	s3c_rtc_gettime(NULL, &rtc_tm);
+	/* Check RTC Time */
+	s3c_rtc_gettime(&pdev->dev, &rtc_tm);
 
 	if (rtc_valid_tm(&rtc_tm)) {
 		rtc_tm.tm_year	= 100;
@@ -523,111 +545,90 @@ static int s3c_rtc_probe(struct platform_device *pdev)
 		rtc_tm.tm_min	= 0;
 		rtc_tm.tm_sec	= 0;
 
-		s3c_rtc_settime(NULL, &rtc_tm);
+		s3c_rtc_settime(&pdev->dev, &rtc_tm);
 
 		dev_warn(&pdev->dev, "warning: invalid RTC value so initializing it\n");
 	}
 
-	if (s3c_rtc_cpu_type != TYPE_S3C2410)
-		rtc->max_user_freq = 32768;
+	if (info->cpu_type != TYPE_S3C2410)
+		info->rtc->max_user_freq = 32768;
 	else
-		rtc->max_user_freq = 128;
+		info->rtc->max_user_freq = 128;
 
-	if (s3c_rtc_cpu_type == TYPE_S3C2416 || s3c_rtc_cpu_type == TYPE_S3C2443) {
-		tmp = readw(s3c_rtc_base + S3C2410_RTCCON);
+	if (info->cpu_type == TYPE_S3C2416 || info->cpu_type == TYPE_S3C2443) {
+		tmp = readw(info->base + S3C2410_RTCCON);
 		tmp |= S3C2443_RTCCON_TICSEL;
-		writew(tmp, s3c_rtc_base + S3C2410_RTCCON);
-	}
-
-	platform_set_drvdata(pdev, rtc);
-
-	s3c_rtc_setfreq(&pdev->dev, 1);
-
-	ret = devm_request_irq(&pdev->dev, s3c_rtc_alarmno, s3c_rtc_alarmirq,
-			  0,  "s3c2410-rtc alarm", rtc);
-	if (ret) {
-		dev_err(&pdev->dev, "IRQ%d error %d\n", s3c_rtc_alarmno, ret);
-		goto err_nortc;
+		writew(tmp, info->base + S3C2410_RTCCON);
 	}
 
-	ret = devm_request_irq(&pdev->dev, s3c_rtc_tickno, s3c_rtc_tickirq,
-			  0,  "s3c2410-rtc tick", rtc);
-	if (ret) {
-		dev_err(&pdev->dev, "IRQ%d error %d\n", s3c_rtc_tickno, ret);
-		goto err_nortc;
-	}
+	s3c_rtc_setfreq(info, 1);
 
-	clk_disable(rtc_clk);
+	clk_disable(info->rtc_clk);
 
 	return 0;
 
  err_nortc:
-	s3c_rtc_enable(pdev, 0);
-	clk_disable_unprepare(rtc_clk);
+	s3c_rtc_enable(info, 0);
+	clk_disable_unprepare(info->rtc_clk);
 
 	return ret;
 }
 
 #ifdef CONFIG_PM_SLEEP
-/* RTC Power management control */
-
-static int ticnt_save, ticnt_en_save;
-static bool wake_en;
 
 static int s3c_rtc_suspend(struct device *dev)
 {
-	struct platform_device *pdev = to_platform_device(dev);
+	struct s3c_rtc *info = dev_get_drvdata(dev);
 
-	clk_enable(rtc_clk);
+	clk_enable(info->rtc_clk);
 	/* save TICNT for anyone using periodic interrupts */
-	if (s3c_rtc_cpu_type == TYPE_S3C64XX) {
-		ticnt_en_save = readw(s3c_rtc_base + S3C2410_RTCCON);
-		ticnt_en_save &= S3C64XX_RTCCON_TICEN;
-		ticnt_save = readl(s3c_rtc_base + S3C2410_TICNT);
+	if (info->cpu_type == TYPE_S3C64XX) {
+		info->ticnt_en_save = readw(info->base + S3C2410_RTCCON);
+		info->ticnt_en_save &= S3C64XX_RTCCON_TICEN;
+		info->ticnt_save = readl(info->base + S3C2410_TICNT);
 	} else {
-		ticnt_save = readb(s3c_rtc_base + S3C2410_TICNT);
+		info->ticnt_save = readb(info->base + S3C2410_TICNT);
 	}
-	s3c_rtc_enable(pdev, 0);
+	s3c_rtc_enable(info, 0);
 
-	if (device_may_wakeup(dev) && !wake_en) {
-		if (enable_irq_wake(s3c_rtc_alarmno) == 0)
-			wake_en = true;
+	if (device_may_wakeup(dev) && !info->wake_en) {
+		if (enable_irq_wake(info->irq_alarm) == 0)
+			info->wake_en = true;
 		else
 			dev_err(dev, "enable_irq_wake failed\n");
 	}
-	clk_disable(rtc_clk);
+	clk_disable(info->rtc_clk);
 
 	return 0;
 }
 
 static int s3c_rtc_resume(struct device *dev)
 {
-	struct platform_device *pdev = to_platform_device(dev);
+	struct s3c_rtc *info = dev_get_drvdata(dev);
 	unsigned int tmp;
 
-	clk_enable(rtc_clk);
-	s3c_rtc_enable(pdev, 1);
-	if (s3c_rtc_cpu_type == TYPE_S3C64XX) {
-		writel(ticnt_save, s3c_rtc_base + S3C2410_TICNT);
-		if (ticnt_en_save) {
-			tmp = readw(s3c_rtc_base + S3C2410_RTCCON);
-			writew(tmp | ticnt_en_save,
-					s3c_rtc_base + S3C2410_RTCCON);
+	clk_enable(info->rtc_clk);
+	s3c_rtc_enable(info, 1);
+	if (info->cpu_type == TYPE_S3C64XX) {
+		writel(info->ticnt_save, info->base + S3C2410_TICNT);
+		if (info->ticnt_en_save) {
+			tmp = readw(info->base + S3C2410_RTCCON);
+			writew(tmp | info->ticnt_en_save,
+					info->base + S3C2410_RTCCON);
 		}
 	} else {
-		writeb(ticnt_save, s3c_rtc_base + S3C2410_TICNT);
+		writeb(info->ticnt_save, info->base + S3C2410_TICNT);
 	}
 
-	if (device_may_wakeup(dev) && wake_en) {
-		disable_irq_wake(s3c_rtc_alarmno);
-		wake_en = false;
+	if (device_may_wakeup(dev) && info->wake_en) {
+		disable_irq_wake(info->irq_alarm);
+		info->wake_en = false;
 	}
-	clk_disable(rtc_clk);
+	clk_disable(info->rtc_clk);
 
 	return 0;
 }
 #endif
-
 static SIMPLE_DEV_PM_OPS(s3c_rtc_pm_ops, s3c_rtc_suspend, s3c_rtc_resume);
 
 #ifdef CONFIG_OF
-- 
1.8.0

[PATCHv3 2/5] rtc: s3c: Remove warning message when checking coding style with checkpatch script

[Chanwoo Choi]

This patch remove warning message when checking codeing style with checkpatch
script and reduce un-necessary i2c read operation on s3c_rtc_enable.

	WARNING: line over 80 characters
	#406: FILE: drivers/rtc/rtc-s3c.c:406:
	+		if ((readw(info->base + S3C2410_RTCCON) & S3C2410_RTCCON_RTCEN) == 0) {

	WARNING: line over 80 characters
	#414: FILE: drivers/rtc/rtc-s3c.c:414:
	+		if ((readw(info->base + S3C2410_RTCCON) & S3C2410_RTCCON_CNTSEL)) {

	WARNING: line over 80 characters
	#422: FILE: drivers/rtc/rtc-s3c.c:422:
	+		if ((readw(info->base + S3C2410_RTCCON) & S3C2410_RTCCON_CLKRST)) {

	WARNING: Missing a blank line after declarations
	#451: FILE: drivers/rtc/rtc-s3c.c:451:
	+	struct s3c_rtc_drv_data *data;
	+	if (pdev->dev.of_node) {

	WARNING: Missing a blank line after declarations
	#453: FILE: drivers/rtc/rtc-s3c.c:453:
	+		const struct of_device_id *match;
	+		match = of_match_node(s3c_rtc_dt_match, pdev->dev.of_node);

	WARNING: DT compatible string "samsung,s3c2416-rtc" appears un-documented -- check ./Documentation/devicetree/bindings/
	#650: FILE: drivers/rtc/rtc-s3c.c:650:
	+		.compatible = "samsung,s3c2416-rtc",

	WARNING: DT compatible string "samsung,s3c2443-rtc" appears un-documented -- check ./Documentation/devicetree/bindings/
	#653: FILE: drivers/rtc/rtc-s3c.c:653:
	+		.compatible = "samsung,s3c2443-rtc",

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 Documentation/devicetree/bindings/rtc/s3c-rtc.txt |  2 ++
 drivers/rtc/rtc-s3c.c                             | 26 ++++++++++++-----------
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/s3c-rtc.txt b/Documentation/devicetree/bindings/rtc/s3c-rtc.txt
index 7ac7259..06db446 100644
--- a/Documentation/devicetree/bindings/rtc/s3c-rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/s3c-rtc.txt
@@ -3,6 +3,8 @@
 Required properties:
 - compatible: should be one of the following.
     * "samsung,s3c2410-rtc" - for controllers compatible with s3c2410 rtc.
+    * "samsung,s3c2416-rtc" - for controllers compatible with s3c2416 rtc.
+    * "samsung,s3c2443-rtc" - for controllers compatible with s3c2443 rtc.
     * "samsung,s3c6410-rtc" - for controllers compatible with s3c6410 rtc.
 - reg: physical base address of the controller and length of memory mapped
   region.
diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index 2f71b13..4e95cca 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -382,25 +382,25 @@ static const struct rtc_class_ops s3c_rtcops = {
 
 static void s3c_rtc_enable(struct s3c_rtc *info, int en)
 {
-	unsigned int tmp;
+	unsigned int con, tmp;
 
 	clk_enable(info->rtc_clk);
+
+	con = readw(info->base + S3C2410_RTCCON);
 	if (!en) {
-		tmp = readw(info->base + S3C2410_RTCCON);
 		if (info->cpu_type == TYPE_S3C64XX)
-			tmp &= ~S3C64XX_RTCCON_TICEN;
-		tmp &= ~S3C2410_RTCCON_RTCEN;
-		writew(tmp, info->base + S3C2410_RTCCON);
+			con &= ~S3C64XX_RTCCON_TICEN;
+		con &= ~S3C2410_RTCCON_RTCEN;
+		writew(con, info->base + S3C2410_RTCCON);
 
 		if (info->cpu_type != TYPE_S3C64XX) {
-			tmp = readb(info->base + S3C2410_TICNT);
-			tmp &= ~S3C2410_TICNT_ENABLE;
-			writeb(tmp, info->base + S3C2410_TICNT);
+			con = readb(info->base + S3C2410_TICNT);
+			con &= ~S3C2410_TICNT_ENABLE;
+			writeb(con, info->base + S3C2410_TICNT);
 		}
 	} else {
 		/* re-enable the device, and check it is ok */
-
-		if ((readw(info->base + S3C2410_RTCCON) & S3C2410_RTCCON_RTCEN) == 0) {
+		if ((con & S3C2410_RTCCON_RTCEN) == 0) {
 			dev_info(info->dev, "rtc disabled, re-enabling\n");
 
 			tmp = readw(info->base + S3C2410_RTCCON);
@@ -408,7 +408,7 @@ static void s3c_rtc_enable(struct s3c_rtc *info, int en)
 				info->base + S3C2410_RTCCON);
 		}
 
-		if ((readw(info->base + S3C2410_RTCCON) & S3C2410_RTCCON_CNTSEL)) {
+		if (con & S3C2410_RTCCON_CNTSEL) {
 			dev_info(info->dev, "removing RTCCON_CNTSEL\n");
 
 			tmp = readw(info->base + S3C2410_RTCCON);
@@ -416,7 +416,7 @@ static void s3c_rtc_enable(struct s3c_rtc *info, int en)
 				info->base + S3C2410_RTCCON);
 		}
 
-		if ((readw(info->base + S3C2410_RTCCON) & S3C2410_RTCCON_CLKRST)) {
+		if (con & S3C2410_RTCCON_CLKRST) {
 			dev_info(info->dev, "removing RTCCON_CLKRST\n");
 
 			tmp = readw(info->base + S3C2410_RTCCON);
@@ -445,8 +445,10 @@ static inline int s3c_rtc_get_driver_data(struct platform_device *pdev)
 {
 #ifdef CONFIG_OF
 	struct s3c_rtc_drv_data *data;
+
 	if (pdev->dev.of_node) {
 		const struct of_device_id *match;
+
 		match = of_match_node(s3c_rtc_dt_match, pdev->dev.of_node);
 		data = (struct s3c_rtc_drv_data *) match->data;
 		return data->cpu_type;
-- 
1.8.0

[PATCHv3 3/5] rtc: s3c: Add s3c_rtc_data structure to use variant data instead of s3c_cpu_type

[Chanwoo Choi]

This patch add s3c_rtc_data structure to variant data according to SoC type.
The s3c_rtc_data structure includes some functions to control RTC operation
and specific data dependent on SoC type.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 drivers/rtc/rtc-s3c.c | 461 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 289 insertions(+), 172 deletions(-)

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index 4e95cca..0d90892 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -32,17 +32,6 @@
 #include <asm/irq.h>
 #include "rtc-s3c.h"
 
-enum s3c_cpu_type {
-	TYPE_S3C2410,
-	TYPE_S3C2416,
-	TYPE_S3C2443,
-	TYPE_S3C64XX,
-};
-
-struct s3c_rtc_drv_data {
-	int cpu_type;
-};
-
 struct s3c_rtc {
 	struct device *dev;
 	struct rtc_device *rtc;
@@ -51,7 +40,7 @@ struct s3c_rtc {
 	struct clk *rtc_clk;
 	bool enabled;
 
-	enum s3c_cpu_type cpu_type;
+	struct s3c_rtc_data *data;
 
 	int irq_alarm;
 	int irq_tick;
@@ -63,6 +52,19 @@ struct s3c_rtc {
 	bool wake_en;
 };
 
+struct s3c_rtc_data {
+	int max_user_freq;
+
+	void (*irq_handler) (struct s3c_rtc *info, int mask);
+	void (*set_freq) (struct s3c_rtc *info, int freq);
+	void (*enable_tick) (struct s3c_rtc *info, struct seq_file *seq);
+	void (*select_tick_clk) (struct s3c_rtc *info);
+	void (*save_tick_cnt) (struct s3c_rtc *info);
+	void (*restore_tick_cnt) (struct s3c_rtc *info);
+	void (*enable) (struct s3c_rtc *info);
+	void (*disable) (struct s3c_rtc *info);
+};
+
 static void s3c_rtc_alarm_clk_enable(struct s3c_rtc *info, bool enable)
 {
 	unsigned long irq_flags;
@@ -83,34 +85,22 @@ static void s3c_rtc_alarm_clk_enable(struct s3c_rtc *info, bool enable)
 }
 
 /* IRQ Handlers */
-static irqreturn_t s3c_rtc_alarmirq(int irq, void *id)
+static irqreturn_t s3c_rtc_tickirq(int irq, void *id)
 {
 	struct s3c_rtc *info = (struct s3c_rtc *)id;
 
-	clk_enable(info->rtc_clk);
-	rtc_update_irq(info->rtc, 1, RTC_AF | RTC_IRQF);
-
-	if (info->cpu_type == TYPE_S3C64XX)
-		writeb(S3C2410_INTP_ALM, info->base + S3C2410_INTP);
-
-	clk_disable(info->rtc_clk);
-
-	s3c_rtc_alarm_clk_enable(info, false);
+	if (info->data->irq_handler)
+		info->data->irq_handler(info, S3C2410_INTP_TIC);
 
 	return IRQ_HANDLED;
 }
 
-static irqreturn_t s3c_rtc_tickirq(int irq, void *id)
+static irqreturn_t s3c_rtc_alarmirq(int irq, void *id)
 {
 	struct s3c_rtc *info = (struct s3c_rtc *)id;
 
-	clk_enable(info->rtc_clk);
-	rtc_update_irq(info->rtc, 1, RTC_PF | RTC_IRQF);
-
-	if (info->cpu_type == TYPE_S3C64XX)
-		writeb(S3C2410_INTP_TIC, info->base + S3C2410_INTP);
-
-	clk_disable(info->rtc_clk);
+	if (info->data->irq_handler)
+		info->data->irq_handler(info, S3C2410_INTP_ALM);
 
 	return IRQ_HANDLED;
 }
@@ -137,36 +127,18 @@ static int s3c_rtc_setaie(struct device *dev, unsigned int enabled)
 	return 0;
 }
 
+/* Set RTC frequency */
 static int s3c_rtc_setfreq(struct s3c_rtc *info, int freq)
 {
-	unsigned int tmp = 0;
-	int val;
-
 	if (!is_power_of_2(freq))
 		return -EINVAL;
 
 	clk_enable(info->rtc_clk);
 	spin_lock_irq(&info->pie_lock);
 
-	if (info->cpu_type != TYPE_S3C64XX) {
-		tmp = readb(info->base + S3C2410_TICNT);
-		tmp &= S3C2410_TICNT_ENABLE;
-	}
-
-	val = (info->rtc->max_user_freq / freq) - 1;
-
-	if (info->cpu_type == TYPE_S3C2416 || info->cpu_type == TYPE_S3C2443) {
-		tmp |= S3C2443_TICNT_PART(val);
-		writel(S3C2443_TICNT1_PART(val), info->base + S3C2443_TICNT1);
+	if (info->data->set_freq)
+		info->data->set_freq(info, freq);
 
-		if (info->cpu_type == TYPE_S3C2416)
-			writel(S3C2416_TICNT2_PART(val),
-				info->base + S3C2416_TICNT2);
-	} else {
-		tmp |= val;
-	}
-
-	writel(tmp, info->base + S3C2410_TICNT);
 	spin_unlock_irq(&info->pie_lock);
 	clk_disable(info->rtc_clk);
 
@@ -174,7 +146,6 @@ static int s3c_rtc_setfreq(struct s3c_rtc *info, int freq)
 }
 
 /* Time read/write */
-
 static int s3c_rtc_gettime(struct device *dev, struct rtc_time *rtc_tm)
 {
 	struct s3c_rtc *info = dev_get_drvdata(dev);
@@ -355,19 +326,14 @@ static int s3c_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
 static int s3c_rtc_proc(struct device *dev, struct seq_file *seq)
 {
 	struct s3c_rtc *info = dev_get_drvdata(dev);
-	unsigned int ticnt;
 
 	clk_enable(info->rtc_clk);
-	if (info->cpu_type == TYPE_S3C64XX) {
-		ticnt = readw(info->base + S3C2410_RTCCON);
-		ticnt &= S3C64XX_RTCCON_TICEN;
-	} else {
-		ticnt = readb(info->base + S3C2410_TICNT);
-		ticnt &= S3C2410_TICNT_ENABLE;
-	}
 
-	seq_printf(seq, "periodic_IRQ\t: %s\n", ticnt  ? "yes" : "no");
+	if (info->data->enable_tick)
+		info->data->enable_tick(info, seq);
+
 	clk_disable(info->rtc_clk);
+
 	return 0;
 }
 
@@ -380,50 +346,69 @@ static const struct rtc_class_ops s3c_rtcops = {
 	.alarm_irq_enable = s3c_rtc_setaie,
 };
 
-static void s3c_rtc_enable(struct s3c_rtc *info, int en)
+static void s3c24xx_rtc_enable(struct s3c_rtc *info)
 {
 	unsigned int con, tmp;
 
 	clk_enable(info->rtc_clk);
 
 	con = readw(info->base + S3C2410_RTCCON);
-	if (!en) {
-		if (info->cpu_type == TYPE_S3C64XX)
-			con &= ~S3C64XX_RTCCON_TICEN;
-		con &= ~S3C2410_RTCCON_RTCEN;
-		writew(con, info->base + S3C2410_RTCCON);
-
-		if (info->cpu_type != TYPE_S3C64XX) {
-			con = readb(info->base + S3C2410_TICNT);
-			con &= ~S3C2410_TICNT_ENABLE;
-			writeb(con, info->base + S3C2410_TICNT);
-		}
-	} else {
-		/* re-enable the device, and check it is ok */
-		if ((con & S3C2410_RTCCON_RTCEN) == 0) {
-			dev_info(info->dev, "rtc disabled, re-enabling\n");
+	/* re-enable the device, and check it is ok */
+	if ((con & S3C2410_RTCCON_RTCEN) == 0) {
+		dev_info(info->dev, "rtc disabled, re-enabling\n");
 
-			tmp = readw(info->base + S3C2410_RTCCON);
-			writew(tmp | S3C2410_RTCCON_RTCEN,
-				info->base + S3C2410_RTCCON);
-		}
+		tmp = readw(info->base + S3C2410_RTCCON);
+		writew(tmp | S3C2410_RTCCON_RTCEN,
+			info->base + S3C2410_RTCCON);
+	}
 
-		if (con & S3C2410_RTCCON_CNTSEL) {
-			dev_info(info->dev, "removing RTCCON_CNTSEL\n");
+	if (con & S3C2410_RTCCON_CNTSEL) {
+		dev_info(info->dev, "removing RTCCON_CNTSEL\n");
 
-			tmp = readw(info->base + S3C2410_RTCCON);
-			writew(tmp & ~S3C2410_RTCCON_CNTSEL,
-				info->base + S3C2410_RTCCON);
-		}
+		tmp = readw(info->base + S3C2410_RTCCON);
+		writew(tmp & ~S3C2410_RTCCON_CNTSEL,
+			info->base + S3C2410_RTCCON);
+	}
 
-		if (con & S3C2410_RTCCON_CLKRST) {
-			dev_info(info->dev, "removing RTCCON_CLKRST\n");
+	if (con & S3C2410_RTCCON_CLKRST) {
+		dev_info(info->dev, "removing RTCCON_CLKRST\n");
 
-			tmp = readw(info->base + S3C2410_RTCCON);
-			writew(tmp & ~S3C2410_RTCCON_CLKRST,
-				info->base + S3C2410_RTCCON);
-		}
+		tmp = readw(info->base + S3C2410_RTCCON);
+		writew(tmp & ~S3C2410_RTCCON_CLKRST,
+			info->base + S3C2410_RTCCON);
 	}
+
+	clk_disable(info->rtc_clk);
+}
+
+static void s3c24xx_rtc_disable(struct s3c_rtc *info)
+{
+	unsigned int con;
+
+	clk_enable(info->rtc_clk);
+
+	con = readw(info->base + S3C2410_RTCCON);
+	con &= ~S3C2410_RTCCON_RTCEN;
+	writew(con, info->base + S3C2410_RTCCON);
+
+	con = readb(info->base + S3C2410_TICNT);
+	con &= ~S3C2410_TICNT_ENABLE;
+	writeb(con, info->base + S3C2410_TICNT);
+
+	clk_disable(info->rtc_clk);
+}
+
+static void s3c6410_rtc_disable(struct s3c_rtc *info)
+{
+	unsigned int con;
+
+	clk_enable(info->rtc_clk);
+
+	con = readw(info->base + S3C2410_RTCCON);
+	con &= ~S3C64XX_RTCCON_TICEN;
+	con &= ~S3C2410_RTCCON_RTCEN;
+	writew(con, info->base + S3C2410_RTCCON);
+
 	clk_disable(info->rtc_clk);
 }
 
@@ -441,20 +426,12 @@ static int s3c_rtc_remove(struct platform_device *pdev)
 
 static const struct of_device_id s3c_rtc_dt_match[];
 
-static inline int s3c_rtc_get_driver_data(struct platform_device *pdev)
+static struct s3c_rtc_data *s3c_rtc_get_data(struct platform_device *pdev)
 {
-#ifdef CONFIG_OF
-	struct s3c_rtc_drv_data *data;
+	const struct of_device_id *match;
 
-	if (pdev->dev.of_node) {
-		const struct of_device_id *match;
-
-		match = of_match_node(s3c_rtc_dt_match, pdev->dev.of_node);
-		data = (struct s3c_rtc_drv_data *) match->data;
-		return data->cpu_type;
-	}
-#endif
-	return platform_get_device_id(pdev)->driver_data;
+	match = of_match_node(s3c_rtc_dt_match, pdev->dev.of_node);
+	return (struct s3c_rtc_data *)match->data;
 }
 
 static int s3c_rtc_probe(struct platform_device *pdev)
@@ -463,7 +440,6 @@ static int s3c_rtc_probe(struct platform_device *pdev)
 	struct rtc_time rtc_tm;
 	struct resource *res;
 	int ret;
-	int tmp;
 
 	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
 	if (!info)
@@ -477,7 +453,11 @@ static int s3c_rtc_probe(struct platform_device *pdev)
 	}
 
 	info->dev = &pdev->dev;
-	info->cpu_type = s3c_rtc_get_driver_data(pdev);
+	info->data = s3c_rtc_get_data(pdev);
+	if (!info->data) {
+		dev_err(&pdev->dev, "failed getting s3c_rtc_data\n");
+		return -EINVAL;
+	}
 	spin_lock_init(&info->pie_lock);
 	spin_lock_init(&info->alarm_clk_lock);
 
@@ -506,7 +486,8 @@ static int s3c_rtc_probe(struct platform_device *pdev)
 	clk_prepare_enable(info->rtc_clk);
 
 	/* check to see if everything is setup correctly */
-	s3c_rtc_enable(info, 1);
+	if (info->data->enable)
+		info->data->enable(info);
 
 	dev_dbg(&pdev->dev, "s3c2410_rtc: RTCCON=%02x\n",
 		 readw(info->base + S3C2410_RTCCON));
@@ -552,16 +533,8 @@ static int s3c_rtc_probe(struct platform_device *pdev)
 		dev_warn(&pdev->dev, "warning: invalid RTC value so initializing it\n");
 	}
 
-	if (info->cpu_type != TYPE_S3C2410)
-		info->rtc->max_user_freq = 32768;
-	else
-		info->rtc->max_user_freq = 128;
-
-	if (info->cpu_type == TYPE_S3C2416 || info->cpu_type == TYPE_S3C2443) {
-		tmp = readw(info->base + S3C2410_RTCCON);
-		tmp |= S3C2443_RTCCON_TICSEL;
-		writew(tmp, info->base + S3C2410_RTCCON);
-	}
+	if (info->data->select_tick_clk)
+		info->data->select_tick_clk(info);
 
 	s3c_rtc_setfreq(info, 1);
 
@@ -570,7 +543,8 @@ static int s3c_rtc_probe(struct platform_device *pdev)
 	return 0;
 
  err_nortc:
-	s3c_rtc_enable(info, 0);
+	if (info->data->disable)
+		info->data->disable(info);
 	clk_disable_unprepare(info->rtc_clk);
 
 	return ret;
@@ -583,15 +557,13 @@ static int s3c_rtc_suspend(struct device *dev)
 	struct s3c_rtc *info = dev_get_drvdata(dev);
 
 	clk_enable(info->rtc_clk);
+
 	/* save TICNT for anyone using periodic interrupts */
-	if (info->cpu_type == TYPE_S3C64XX) {
-		info->ticnt_en_save = readw(info->base + S3C2410_RTCCON);
-		info->ticnt_en_save &= S3C64XX_RTCCON_TICEN;
-		info->ticnt_save = readl(info->base + S3C2410_TICNT);
-	} else {
-		info->ticnt_save = readb(info->base + S3C2410_TICNT);
-	}
-	s3c_rtc_enable(info, 0);
+	if (info->data->save_tick_cnt)
+		info->data->save_tick_cnt(info);
+
+	if (info->data->disable)
+		info->data->disable(info);
 
 	if (device_may_wakeup(dev) && !info->wake_en) {
 		if (enable_irq_wake(info->irq_alarm) == 0)
@@ -599,6 +571,7 @@ static int s3c_rtc_suspend(struct device *dev)
 		else
 			dev_err(dev, "enable_irq_wake failed\n");
 	}
+
 	clk_disable(info->rtc_clk);
 
 	return 0;
@@ -607,25 +580,20 @@ static int s3c_rtc_suspend(struct device *dev)
 static int s3c_rtc_resume(struct device *dev)
 {
 	struct s3c_rtc *info = dev_get_drvdata(dev);
-	unsigned int tmp;
 
 	clk_enable(info->rtc_clk);
-	s3c_rtc_enable(info, 1);
-	if (info->cpu_type == TYPE_S3C64XX) {
-		writel(info->ticnt_save, info->base + S3C2410_TICNT);
-		if (info->ticnt_en_save) {
-			tmp = readw(info->base + S3C2410_RTCCON);
-			writew(tmp | info->ticnt_en_save,
-					info->base + S3C2410_RTCCON);
-		}
-	} else {
-		writeb(info->ticnt_save, info->base + S3C2410_TICNT);
-	}
+
+	if (info->data->enable)
+		info->data->enable(info);
+
+	if (info->data->restore_tick_cnt)
+		info->data->restore_tick_cnt(info);
 
 	if (device_may_wakeup(dev) && info->wake_en) {
 		disable_irq_wake(info->irq_alarm);
 		info->wake_en = false;
 	}
+
 	clk_disable(info->rtc_clk);
 
 	return 0;
@@ -633,56 +601,206 @@ static int s3c_rtc_resume(struct device *dev)
 #endif
 static SIMPLE_DEV_PM_OPS(s3c_rtc_pm_ops, s3c_rtc_suspend, s3c_rtc_resume);
 
-#ifdef CONFIG_OF
-static struct s3c_rtc_drv_data s3c_rtc_drv_data_array[] = {
-	[TYPE_S3C2410] = { TYPE_S3C2410 },
-	[TYPE_S3C2416] = { TYPE_S3C2416 },
-	[TYPE_S3C2443] = { TYPE_S3C2443 },
-	[TYPE_S3C64XX] = { TYPE_S3C64XX },
+static void s3c24xx_rtc_irq(struct s3c_rtc *info, int mask)
+{
+	clk_enable(info->rtc_clk);
+	rtc_update_irq(info->rtc, 1, RTC_AF | RTC_IRQF);
+	clk_disable(info->rtc_clk);
+
+	s3c_rtc_alarm_clk_enable(info, false);
+}
+
+static void s3c6410_rtc_irq(struct s3c_rtc *info, int mask)
+{
+	clk_enable(info->rtc_clk);
+	rtc_update_irq(info->rtc, 1, RTC_AF | RTC_IRQF);
+	writeb(mask, info->base + S3C2410_INTP);
+	clk_disable(info->rtc_clk);
+
+	s3c_rtc_alarm_clk_enable(info, false);
+}
+
+static void s3c2410_rtc_setfreq(struct s3c_rtc *info, int freq)
+{
+	unsigned int tmp = 0;
+	int val;
+
+	tmp = readb(info->base + S3C2410_TICNT);
+	tmp &= S3C2410_TICNT_ENABLE;
+
+	val = (info->rtc->max_user_freq / freq) - 1;
+	tmp |= val;
+
+	writel(tmp, info->base + S3C2410_TICNT);
+}
+
+static void s3c2416_rtc_setfreq(struct s3c_rtc *info, int freq)
+{
+	unsigned int tmp = 0;
+	int val;
+
+	tmp = readb(info->base + S3C2410_TICNT);
+	tmp &= S3C2410_TICNT_ENABLE;
+
+	val = (info->rtc->max_user_freq / freq) - 1;
+
+	tmp |= S3C2443_TICNT_PART(val);
+	writel(S3C2443_TICNT1_PART(val), info->base + S3C2443_TICNT1);
+
+	writel(S3C2416_TICNT2_PART(val), info->base + S3C2416_TICNT2);
+
+	writel(tmp, info->base + S3C2410_TICNT);
+}
+
+static void s3c2443_rtc_setfreq(struct s3c_rtc *info, int freq)
+{
+	unsigned int tmp = 0;
+	int val;
+
+	tmp = readb(info->base + S3C2410_TICNT);
+	tmp &= S3C2410_TICNT_ENABLE;
+
+	val = (info->rtc->max_user_freq / freq) - 1;
+
+	tmp |= S3C2443_TICNT_PART(val);
+	writel(S3C2443_TICNT1_PART(val), info->base + S3C2443_TICNT1);
+
+	writel(tmp, info->base + S3C2410_TICNT);
+}
+
+static void s3c6410_rtc_setfreq(struct s3c_rtc *info, int freq)
+{
+	int val;
+
+	val = (info->rtc->max_user_freq / freq) - 1;
+	writel(val, info->base + S3C2410_TICNT);
+}
+
+static void s3c24xx_rtc_enable_tick(struct s3c_rtc *info, struct seq_file *seq)
+{
+	unsigned int ticnt;
+
+	ticnt = readb(info->base + S3C2410_TICNT);
+	ticnt &= S3C2410_TICNT_ENABLE;
+
+	seq_printf(seq, "periodic_IRQ\t: %s\n", ticnt  ? "yes" : "no");
+}
+
+static void s3c2416_rtc_select_tick_clk(struct s3c_rtc *info)
+{
+	unsigned int con;
+
+	con = readw(info->base + S3C2410_RTCCON);
+	con |= S3C2443_RTCCON_TICSEL;
+	writew(con, info->base + S3C2410_RTCCON);
+}
+
+static void s3c6410_rtc_enable_tick(struct s3c_rtc *info, struct seq_file *seq)
+{
+	unsigned int ticnt;
+
+	ticnt = readw(info->base + S3C2410_RTCCON);
+	ticnt &= S3C64XX_RTCCON_TICEN;
+
+	seq_printf(seq, "periodic_IRQ\t: %s\n", ticnt  ? "yes" : "no");
+}
+
+static void s3c24xx_rtc_save_tick_cnt(struct s3c_rtc *info)
+{
+	info->ticnt_save = readb(info->base + S3C2410_TICNT);
+}
+
+static void s3c24xx_rtc_restore_tick_cnt(struct s3c_rtc *info)
+{
+	writeb(info->ticnt_save, info->base + S3C2410_TICNT);
+}
+
+static void s3c6410_rtc_save_tick_cnt(struct s3c_rtc *info)
+{
+	info->ticnt_en_save = readw(info->base + S3C2410_RTCCON);
+	info->ticnt_en_save &= S3C64XX_RTCCON_TICEN;
+	info->ticnt_save = readl(info->base + S3C2410_TICNT);
+}
+
+static void s3c6410_rtc_restore_tick_cnt(struct s3c_rtc *info)
+{
+	unsigned int con;
+
+	writel(info->ticnt_save, info->base + S3C2410_TICNT);
+	if (info->ticnt_en_save) {
+		con = readw(info->base + S3C2410_RTCCON);
+		writew(con | info->ticnt_en_save,
+				info->base + S3C2410_RTCCON);
+	}
+}
+
+static struct s3c_rtc_data const s3c2410_rtc_data = {
+	.max_user_freq		= 128,
+	.irq_handler		= s3c24xx_rtc_irq,
+	.set_freq		= s3c2410_rtc_setfreq,
+	.enable_tick		= s3c24xx_rtc_enable_tick,
+	.save_tick_cnt		= s3c24xx_rtc_save_tick_cnt,
+	.restore_tick_cnt	= s3c24xx_rtc_restore_tick_cnt,
+	.enable			= s3c24xx_rtc_enable,
+	.disable		= s3c24xx_rtc_disable,
+};
+
+static struct s3c_rtc_data const s3c2416_rtc_data = {
+	.max_user_freq		= 32768,
+	.irq_handler		= s3c24xx_rtc_irq,
+	.set_freq		= s3c2416_rtc_setfreq,
+	.enable_tick		= s3c24xx_rtc_enable_tick,
+	.select_tick_clk	= s3c2416_rtc_select_tick_clk,
+	.save_tick_cnt		= s3c24xx_rtc_save_tick_cnt,
+	.restore_tick_cnt	= s3c24xx_rtc_restore_tick_cnt,
+	.enable			= s3c24xx_rtc_enable,
+	.disable		= s3c24xx_rtc_disable,
+};
+
+static struct s3c_rtc_data const s3c2443_rtc_data = {
+	.max_user_freq		= 32768,
+	.irq_handler		= s3c24xx_rtc_irq,
+	.set_freq		= s3c2443_rtc_setfreq,
+	.enable_tick		= s3c24xx_rtc_enable_tick,
+	.select_tick_clk	= s3c2416_rtc_select_tick_clk,
+	.save_tick_cnt		= s3c24xx_rtc_save_tick_cnt,
+	.restore_tick_cnt	= s3c24xx_rtc_restore_tick_cnt,
+	.enable			= s3c24xx_rtc_enable,
+	.disable		= s3c24xx_rtc_disable,
+};
+
+static struct s3c_rtc_data const s3c6410_rtc_data = {
+	.max_user_freq		= 32768,
+	.irq_handler		= s3c6410_rtc_irq,
+	.set_freq		= s3c6410_rtc_setfreq,
+	.enable_tick		= s3c6410_rtc_enable_tick,
+	.save_tick_cnt		= s3c6410_rtc_save_tick_cnt,
+	.restore_tick_cnt	= s3c6410_rtc_restore_tick_cnt,
+	.enable			= s3c24xx_rtc_enable,
+	.disable		= s3c6410_rtc_disable,
 };
 
 static const struct of_device_id s3c_rtc_dt_match[] = {
 	{
 		.compatible = "samsung,s3c2410-rtc",
-		.data = &s3c_rtc_drv_data_array[TYPE_S3C2410],
+		.data = (void *)&s3c2410_rtc_data,
 	}, {
 		.compatible = "samsung,s3c2416-rtc",
-		.data = &s3c_rtc_drv_data_array[TYPE_S3C2416],
+		.data = (void *)&s3c2416_rtc_data,
 	}, {
 		.compatible = "samsung,s3c2443-rtc",
-		.data = &s3c_rtc_drv_data_array[TYPE_S3C2443],
+		.data = (void *)&s3c2443_rtc_data,
 	}, {
 		.compatible = "samsung,s3c6410-rtc",
-		.data = &s3c_rtc_drv_data_array[TYPE_S3C64XX],
+		.data = (void *)&s3c6410_rtc_data,
 	},
-	{},
+	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, s3c_rtc_dt_match);
-#endif
-
-static struct platform_device_id s3c_rtc_driver_ids[] = {
-	{
-		.name		= "s3c2410-rtc",
-		.driver_data	= TYPE_S3C2410,
-	}, {
-		.name		= "s3c2416-rtc",
-		.driver_data	= TYPE_S3C2416,
-	}, {
-		.name		= "s3c2443-rtc",
-		.driver_data	= TYPE_S3C2443,
-	}, {
-		.name		= "s3c64xx-rtc",
-		.driver_data	= TYPE_S3C64XX,
-	},
-	{ }
-};
-
-MODULE_DEVICE_TABLE(platform, s3c_rtc_driver_ids);
 
 static struct platform_driver s3c_rtc_driver = {
 	.probe		= s3c_rtc_probe,
 	.remove		= s3c_rtc_remove,
-	.id_table	= s3c_rtc_driver_ids,
 	.driver		= {
 		.name	= "s3c-rtc",
 		.owner	= THIS_MODULE,
@@ -690,7 +808,6 @@ static struct platform_driver s3c_rtc_driver = {
 		.of_match_table	= of_match_ptr(s3c_rtc_dt_match),
 	},
 };
-
 module_platform_driver(s3c_rtc_driver);
 
 MODULE_DESCRIPTION("Samsung S3C RTC Driver");
-- 
1.8.0

[PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Chanwoo Choi]

This patch add support for RTC of Exynos3250 SoC. The Exynos3250 needs source
clock(32.768KHz) for RTC block. If source clock of RTC is registerd on clock
list of common clk framework, Exynos RTC drvier have to control this clock.

Clock list for s3c-rtc device:
- rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC.
- rtc_src : XrtcXTI is 32.768.kHz source clock for RTC.
 (XRTCXTI: Specifies a clock from 32.768 kHz crystal pad with XRTCXTI and
 XRTCXTO pins. RTC uses this clock as the source of a real-time clock.)

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 Documentation/devicetree/bindings/rtc/s3c-rtc.txt |  1 +
 drivers/rtc/rtc-s3c.c                             | 93 ++++++++++++++++++++++-
 2 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/rtc/s3c-rtc.txt b/Documentation/devicetree/bindings/rtc/s3c-rtc.txt
index 06db446..ab757b84 100644
--- a/Documentation/devicetree/bindings/rtc/s3c-rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/s3c-rtc.txt
@@ -6,6 +6,7 @@ Required properties:
     * "samsung,s3c2416-rtc" - for controllers compatible with s3c2416 rtc.
     * "samsung,s3c2443-rtc" - for controllers compatible with s3c2443 rtc.
     * "samsung,s3c6410-rtc" - for controllers compatible with s3c6410 rtc.
+    * "samsung,exynos3250-rtc" - for controllers compatible with exynos3250 rtc.
 - reg: physical base address of the controller and length of memory mapped
   region.
 - interrupts: Two interrupt numbers to the cpu should be specified. First
diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index 0d90892..a6b1252 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -38,6 +38,7 @@ struct s3c_rtc {
 
 	void __iomem *base;
 	struct clk *rtc_clk;
+	struct clk *rtc_src_clk;
 	bool enabled;
 
 	struct s3c_rtc_data *data;
@@ -54,6 +55,7 @@ struct s3c_rtc {
 
 struct s3c_rtc_data {
 	int max_user_freq;
+	bool needs_src_clk;
 
 	void (*irq_handler) (struct s3c_rtc *info, int mask);
 	void (*set_freq) (struct s3c_rtc *info, int freq);
@@ -73,10 +75,14 @@ static void s3c_rtc_alarm_clk_enable(struct s3c_rtc *info, bool enable)
 	if (enable) {
 		if (!info->enabled) {
 			clk_enable(info->rtc_clk);
+			if (info->data->needs_src_clk)
+				clk_enable(info->rtc_src_clk);
 			info->enabled = true;
 		}
 	} else {
 		if (info->enabled) {
+			if (info->data->needs_src_clk)
+				clk_disable(info->rtc_src_clk);
 			clk_disable(info->rtc_clk);
 			info->enabled = false;
 		}
@@ -114,12 +120,16 @@ static int s3c_rtc_setaie(struct device *dev, unsigned int enabled)
 	dev_dbg(info->dev, "%s: aie=%d\n", __func__, enabled);
 
 	clk_enable(info->rtc_clk);
+	if (info->data->needs_src_clk)
+		clk_enable(info->rtc_src_clk);
 	tmp = readb(info->base + S3C2410_RTCALM) & ~S3C2410_RTCALM_ALMEN;
 
 	if (enabled)
 		tmp |= S3C2410_RTCALM_ALMEN;
 
 	writeb(tmp, info->base + S3C2410_RTCALM);
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
 
 	s3c_rtc_alarm_clk_enable(info, enabled);
@@ -134,12 +144,16 @@ static int s3c_rtc_setfreq(struct s3c_rtc *info, int freq)
 		return -EINVAL;
 
 	clk_enable(info->rtc_clk);
+	if (info->data->needs_src_clk)
+		clk_enable(info->rtc_src_clk);
 	spin_lock_irq(&info->pie_lock);
 
 	if (info->data->set_freq)
 		info->data->set_freq(info, freq);
 
 	spin_unlock_irq(&info->pie_lock);
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
 
 	return 0;
@@ -152,6 +166,9 @@ static int s3c_rtc_gettime(struct device *dev, struct rtc_time *rtc_tm)
 	unsigned int have_retried = 0;
 
 	clk_enable(info->rtc_clk);
+	if (info->data->needs_src_clk)
+		clk_enable(info->rtc_src_clk);
+
  retry_get_time:
 	rtc_tm->tm_min  = readb(info->base + S3C2410_RTCMIN);
 	rtc_tm->tm_hour = readb(info->base + S3C2410_RTCHOUR);
@@ -185,6 +202,8 @@ static int s3c_rtc_gettime(struct device *dev, struct rtc_time *rtc_tm)
 
 	rtc_tm->tm_mon -= 1;
 
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
 
 	return rtc_valid_tm(rtc_tm);
@@ -207,6 +226,8 @@ static int s3c_rtc_settime(struct device *dev, struct rtc_time *tm)
 	}
 
 	clk_enable(info->rtc_clk);
+	if (info->data->needs_src_clk)
+		clk_enable(info->rtc_src_clk);
 
 	writeb(bin2bcd(tm->tm_sec),  info->base + S3C2410_RTCSEC);
 	writeb(bin2bcd(tm->tm_min),  info->base + S3C2410_RTCMIN);
@@ -215,6 +236,8 @@ static int s3c_rtc_settime(struct device *dev, struct rtc_time *tm)
 	writeb(bin2bcd(tm->tm_mon + 1), info->base + S3C2410_RTCMON);
 	writeb(bin2bcd(year), info->base + S3C2410_RTCYEAR);
 
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
 
 	return 0;
@@ -227,6 +250,9 @@ static int s3c_rtc_getalarm(struct device *dev, struct rtc_wkalrm *alrm)
 	unsigned int alm_en;
 
 	clk_enable(info->rtc_clk);
+	if (info->data->needs_src_clk)
+		clk_enable(info->rtc_src_clk);
+
 	alm_tm->tm_sec  = readb(info->base + S3C2410_ALMSEC);
 	alm_tm->tm_min  = readb(info->base + S3C2410_ALMMIN);
 	alm_tm->tm_hour = readb(info->base + S3C2410_ALMHOUR);
@@ -278,7 +304,10 @@ static int s3c_rtc_getalarm(struct device *dev, struct rtc_wkalrm *alrm)
 	else
 		alm_tm->tm_year = -1;
 
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
+
 	return 0;
 }
 
@@ -289,6 +318,9 @@ static int s3c_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
 	unsigned int alrm_en;
 
 	clk_enable(info->rtc_clk);
+	if (info->data->needs_src_clk)
+		clk_enable(info->rtc_src_clk);
+
 	dev_dbg(dev, "s3c_rtc_setalarm: %d, %04d.%02d.%02d %02d:%02d:%02d\n",
 		 alrm->enabled,
 		 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday,
@@ -318,6 +350,8 @@ static int s3c_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
 
 	s3c_rtc_setaie(dev, alrm->enabled);
 
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
 
 	return 0;
@@ -328,10 +362,14 @@ static int s3c_rtc_proc(struct device *dev, struct seq_file *seq)
 	struct s3c_rtc *info = dev_get_drvdata(dev);
 
 	clk_enable(info->rtc_clk);
+	if (info->data->needs_src_clk)
+		clk_enable(info->rtc_src_clk);
 
 	if (info->data->enable_tick)
 		info->data->enable_tick(info, seq);
 
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
 
 	return 0;
@@ -351,6 +389,8 @@ static void s3c24xx_rtc_enable(struct s3c_rtc *info)
 	unsigned int con, tmp;
 
 	clk_enable(info->rtc_clk);
+	if (info->data->needs_src_clk)
+		clk_enable(info->rtc_src_clk);
 
 	con = readw(info->base + S3C2410_RTCCON);
 	/* re-enable the device, and check it is ok */
@@ -378,6 +418,8 @@ static void s3c24xx_rtc_enable(struct s3c_rtc *info)
 			info->base + S3C2410_RTCCON);
 	}
 
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
 }
 
@@ -386,6 +428,8 @@ static void s3c24xx_rtc_disable(struct s3c_rtc *info)
 	unsigned int con;
 
 	clk_enable(info->rtc_clk);
+	if (info->data->needs_src_clk)
+		clk_enable(info->rtc_src_clk);
 
 	con = readw(info->base + S3C2410_RTCCON);
 	con &= ~S3C2410_RTCCON_RTCEN;
@@ -395,6 +439,8 @@ static void s3c24xx_rtc_disable(struct s3c_rtc *info)
 	con &= ~S3C2410_TICNT_ENABLE;
 	writeb(con, info->base + S3C2410_TICNT);
 
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
 }
 
@@ -403,12 +449,16 @@ static void s3c6410_rtc_disable(struct s3c_rtc *info)
 	unsigned int con;
 
 	clk_enable(info->rtc_clk);
+	if (info->data->needs_src_clk)
+		clk_enable(info->rtc_src_clk);
 
 	con = readw(info->base + S3C2410_RTCCON);
 	con &= ~S3C64XX_RTCCON_TICEN;
 	con &= ~S3C2410_RTCCON_RTCEN;
 	writew(con, info->base + S3C2410_RTCCON);
 
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
 }
 
@@ -480,11 +530,19 @@ static int s3c_rtc_probe(struct platform_device *pdev)
 
 	info->rtc_clk = devm_clk_get(&pdev->dev, "rtc");
 	if (IS_ERR(info->rtc_clk)) {
-		dev_err(&pdev->dev, "failed to find rtc clock source\n");
+		dev_err(&pdev->dev, "failed to find rtc clock\n");
 		return PTR_ERR(info->rtc_clk);
 	}
 	clk_prepare_enable(info->rtc_clk);
 
+	info->rtc_src_clk = devm_clk_get(&pdev->dev, "rtc_src");
+	if (IS_ERR(info->rtc_src_clk)) {
+		dev_err(&pdev->dev, "failed to find rtc source clock\n");
+		return PTR_ERR(info->rtc_src_clk);
+	}
+	clk_prepare_enable(info->rtc_src_clk);
+
+
 	/* check to see if everything is setup correctly */
 	if (info->data->enable)
 		info->data->enable(info);
@@ -538,6 +596,8 @@ static int s3c_rtc_probe(struct platform_device *pdev)
 
 	s3c_rtc_setfreq(info, 1);
 
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
 
 	return 0;
@@ -557,6 +617,8 @@ static int s3c_rtc_suspend(struct device *dev)
 	struct s3c_rtc *info = dev_get_drvdata(dev);
 
 	clk_enable(info->rtc_clk);
+	if (info->data->needs_src_clk)
+		clk_enable(info->rtc_src_clk);
 
 	/* save TICNT for anyone using periodic interrupts */
 	if (info->data->save_tick_cnt)
@@ -572,6 +634,8 @@ static int s3c_rtc_suspend(struct device *dev)
 			dev_err(dev, "enable_irq_wake failed\n");
 	}
 
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
 
 	return 0;
@@ -582,6 +646,8 @@ static int s3c_rtc_resume(struct device *dev)
 	struct s3c_rtc *info = dev_get_drvdata(dev);
 
 	clk_enable(info->rtc_clk);
+	if (info->data->needs_src_clk)
+		clk_enable(info->rtc_src_clk);
 
 	if (info->data->enable)
 		info->data->enable(info);
@@ -594,6 +660,8 @@ static int s3c_rtc_resume(struct device *dev)
 		info->wake_en = false;
 	}
 
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
 
 	return 0;
@@ -604,7 +672,11 @@ static SIMPLE_DEV_PM_OPS(s3c_rtc_pm_ops, s3c_rtc_suspend, s3c_rtc_resume);
 static void s3c24xx_rtc_irq(struct s3c_rtc *info, int mask)
 {
 	clk_enable(info->rtc_clk);
+	if (info->data->needs_src_clk)
+		clk_enable(info->rtc_src_clk);
 	rtc_update_irq(info->rtc, 1, RTC_AF | RTC_IRQF);
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
 
 	s3c_rtc_alarm_clk_enable(info, false);
@@ -613,8 +685,12 @@ static void s3c24xx_rtc_irq(struct s3c_rtc *info, int mask)
 static void s3c6410_rtc_irq(struct s3c_rtc *info, int mask)
 {
 	clk_enable(info->rtc_clk);
+	if (info->data->needs_src_clk)
+		clk_enable(info->rtc_src_clk);
 	rtc_update_irq(info->rtc, 1, RTC_AF | RTC_IRQF);
 	writeb(mask, info->base + S3C2410_INTP);
+	if (info->data->needs_src_clk)
+		clk_disable(info->rtc_src_clk);
 	clk_disable(info->rtc_clk);
 
 	s3c_rtc_alarm_clk_enable(info, false);
@@ -780,6 +856,18 @@ static struct s3c_rtc_data const s3c6410_rtc_data = {
 	.disable		= s3c6410_rtc_disable,
 };
 
+static struct s3c_rtc_data const exynos3250_rtc_data = {
+	.max_user_freq		= 32768,
+	.needs_src_clk		= true,
+	.irq_handler		= s3c6410_rtc_irq,
+	.set_freq		= s3c6410_rtc_setfreq,
+	.enable_tick		= s3c6410_rtc_enable_tick,
+	.save_tick_cnt		= s3c6410_rtc_save_tick_cnt,
+	.restore_tick_cnt	= s3c6410_rtc_restore_tick_cnt,
+	.enable			= s3c24xx_rtc_enable,
+	.disable		= s3c6410_rtc_disable,
+};
+
 static const struct of_device_id s3c_rtc_dt_match[] = {
 	{
 		.compatible = "samsung,s3c2410-rtc",
@@ -793,6 +881,9 @@ static const struct of_device_id s3c_rtc_dt_match[] = {
 	}, {
 		.compatible = "samsung,s3c6410-rtc",
 		.data = (void *)&s3c6410_rtc_data,
+	}, {
+		.compatible = "samsung,exynos3250-rtc",
+		.data = (void *)&exynos3250_rtc_data,
 	},
 	{ /* sentinel */ },
 };
-- 
1.8.0

[PATCHv3 5/5] ARM: dts: Fix wrong compatible string of Exynos3250 RTC dt node

[Chanwoo Choi]

This patch fix wrong compatible string of Exynos3250 RTC (Real-Time Clock) dt
node. The RTC of Exynos3250 must need additional source clock (XrtcXTI).

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 arch/arm/boot/dts/exynos3250.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
index 1d52de6..429a6c6 100644
--- a/arch/arm/boot/dts/exynos3250.dtsi
+++ b/arch/arm/boot/dts/exynos3250.dtsi
@@ -164,7 +164,7 @@
 		};
 
 		rtc: rtc@10070000 {
-			compatible = "samsung,s3c6410-rtc";
+			compatible = "samsung,exynos3250-rtc";
 			reg = <0x10070000 0x100>;
 			interrupts = <0 73 0>, <0 74 0>;
 			status = "disabled";
-- 
1.8.0

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Javier Martinez Canillas]

Hello Chanwoo,

On Thu, Aug 28, 2014 at 11:02 AM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> This patch add support for RTC of Exynos3250 SoC. The Exynos3250 needs source
> clock(32.768KHz) for RTC block. If source clock of RTC is registerd on clock
> list of common clk framework, Exynos RTC drvier have to control this clock.
>
> Clock list for s3c-rtc device:
> - rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC.
> - rtc_src : XrtcXTI is 32.768.kHz source clock for RTC.

Is this RTC source clock needed for all Exynos SoCs?

> @@ -480,11 +530,19 @@ static int s3c_rtc_probe(struct platform_device *pdev)
>
>         info->rtc_clk = devm_clk_get(&pdev->dev, "rtc");
>         if (IS_ERR(info->rtc_clk)) {
> -               dev_err(&pdev->dev, "failed to find rtc clock source\n");
> +               dev_err(&pdev->dev, "failed to find rtc clock\n");
>                 return PTR_ERR(info->rtc_clk);
>         }
>         clk_prepare_enable(info->rtc_clk);
>
> +       info->rtc_src_clk = devm_clk_get(&pdev->dev, "rtc_src");
> +       if (IS_ERR(info->rtc_src_clk)) {
> +               dev_err(&pdev->dev, "failed to find rtc source clock\n");
> +               return PTR_ERR(info->rtc_src_clk);

On an Exynos5420 Peach Pit machine I'm having the following error and
the driver fails to probe:

[    2.095700] s3c-rtc: probe of 101e0000.rtc failed with error -2

Reverting this patch fixes the issue and the s3c RTC works again on
this machine.

Best regards,
Javier

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Javier Martinez Canillas]

Hello Chanwoo,

On Thu, Aug 28, 2014 at 11:02 AM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> This patch add support for RTC of Exynos3250 SoC. The Exynos3250 needs source
> clock(32.768KHz) for RTC block. If source clock of RTC is registerd on clock
> list of common clk framework, Exynos RTC drvier have to control this clock.
>
> Clock list for s3c-rtc device:
> - rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC.
> - rtc_src : XrtcXTI is 32.768.kHz source clock for RTC.

Is this RTC source clock needed for all Exynos SoCs?

> @@ -480,11 +530,19 @@ static int s3c_rtc_probe(struct platform_device *pdev)
>
>         info->rtc_clk = devm_clk_get(&pdev->dev, "rtc");
>         if (IS_ERR(info->rtc_clk)) {
> -               dev_err(&pdev->dev, "failed to find rtc clock source\n");
> +               dev_err(&pdev->dev, "failed to find rtc clock\n");
>                 return PTR_ERR(info->rtc_clk);
>         }
>         clk_prepare_enable(info->rtc_clk);
>
> +       info->rtc_src_clk = devm_clk_get(&pdev->dev, "rtc_src");
> +       if (IS_ERR(info->rtc_src_clk)) {
> +               dev_err(&pdev->dev, "failed to find rtc source clock\n");
> +               return PTR_ERR(info->rtc_src_clk);

On an Exynos5420 Peach Pit machine I'm having the following error and
the driver fails to probe:

[    2.095700] s3c-rtc: probe of 101e0000.rtc failed with error -2

Reverting this patch fixes the issue and the s3c RTC works again on
this machine.

Best regards,
Javier

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Daniel Drake]

On Tue, Sep 16, 2014 at 7:48 AM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
>> Clock list for s3c-rtc device:
>> - rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC.
>> - rtc_src : XrtcXTI is 32.768.kHz source clock for RTC.
>
> Is this RTC source clock needed for all Exynos SoCs?

It is at least needed on Exynos4412, which has the XrtcXTI thing
exactly as you describe. However the very standard setup there is to
hook it up to the CP clock output of the MAX76686 PMIC. This CP clock
is on by default, so you can potentially live without that detail
being present in the DT.

However... one small issue with this setup is that when you enable
CONFIG_COMMON_CLK_MAX77686, the CP clock gets exposed in Linux's
common clock framework, and Linux then turns it off because it
believes it is unused. Then the RTC stops ticking.

So the rtc_src idea would also be good for Exynos4412. Maybe it would
make sense to drop the needs_src_clk flag, and simply require/enable
the src clock whenever it is present in the DT.

Also, are you sure about the way you are treating this clock, all
those enable/disable calls? You only seem to enable it when doing some
particular driver operations e.g. reading the time, leaving it
disabled at all other times. However I believe on Exynos4412 that if
you disable this clock then the RTC will not tick.

Daniel

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Daniel Drake]

On Tue, Sep 16, 2014 at 7:48 AM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
>> Clock list for s3c-rtc device:
>> - rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC.
>> - rtc_src : XrtcXTI is 32.768.kHz source clock for RTC.
>
> Is this RTC source clock needed for all Exynos SoCs?

It is at least needed on Exynos4412, which has the XrtcXTI thing
exactly as you describe. However the very standard setup there is to
hook it up to the CP clock output of the MAX76686 PMIC. This CP clock
is on by default, so you can potentially live without that detail
being present in the DT.

However... one small issue with this setup is that when you enable
CONFIG_COMMON_CLK_MAX77686, the CP clock gets exposed in Linux's
common clock framework, and Linux then turns it off because it
believes it is unused. Then the RTC stops ticking.

So the rtc_src idea would also be good for Exynos4412. Maybe it would
make sense to drop the needs_src_clk flag, and simply require/enable
the src clock whenever it is present in the DT.

Also, are you sure about the way you are treating this clock, all
those enable/disable calls? You only seem to enable it when doing some
particular driver operations e.g. reading the time, leaving it
disabled at all other times. However I believe on Exynos4412 that if
you disable this clock then the RTC will not tick.

Daniel

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Javier Martinez Canillas]

[adding Doug Anderson as cc]

On Tue, Sep 16, 2014 at 3:48 PM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> On Thu, Aug 28, 2014 at 11:02 AM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>> This patch add support for RTC of Exynos3250 SoC. The Exynos3250 needs source
>> clock(32.768KHz) for RTC block. If source clock of RTC is registerd on clock
>> list of common clk framework, Exynos RTC drvier have to control this clock.
>>
>> Clock list for s3c-rtc device:
>> - rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC.
>> - rtc_src : XrtcXTI is 32.768.kHz source clock for RTC.
>
> Is this RTC source clock needed for all Exynos SoCs?
>

By looking at the Exynos5420 user manual I see that there is indeed a
XrtcXTI clock pad and the doc says:

"XRTCXTI: Specifies a clock from 32.768 kHz crystal pad with XRTCXTI
and XRTCXTO pins. RTC uses this
clock as the source of a real-time clock."

Also by looking at the board schematic I see that the 32KHZAP clock
from the Maxim77802 PMIC is used as the external 32.768.kHz source
clock for the RTC but that information is not in the downstream
ChromeOS DTS since AFAIU the Maxim driver in that tree predates the
common clock framework so the PMIC clocks were modeled as regulators.

>> @@ -480,11 +530,19 @@ static int s3c_rtc_probe(struct platform_device *pdev)
>>
>>         info->rtc_clk = devm_clk_get(&pdev->dev, "rtc");
>>         if (IS_ERR(info->rtc_clk)) {
>> -               dev_err(&pdev->dev, "failed to find rtc clock source\n");
>> +               dev_err(&pdev->dev, "failed to find rtc clock\n");
>>                 return PTR_ERR(info->rtc_clk);
>>         }
>>         clk_prepare_enable(info->rtc_clk);
>>
>> +       info->rtc_src_clk = devm_clk_get(&pdev->dev, "rtc_src");
>> +       if (IS_ERR(info->rtc_src_clk)) {
>> +               dev_err(&pdev->dev, "failed to find rtc source clock\n");
>> +               return PTR_ERR(info->rtc_src_clk);
>
> On an Exynos5420 Peach Pit machine I'm having the following error and
> the driver fails to probe:
>
> [    2.095700] s3c-rtc: probe of 101e0000.rtc failed with error -2
>
> Reverting this patch fixes the issue and the s3c RTC works again on
> this machine.
>

The following change [0] in the Peach Pit DTS makes the RTC to claim
the external clock and is working again but I'm not sure if $subject
is correct in making the "rtc_src" property mandatory since it breaks
backward DT compatibility.

Best regards,
Javier

[0]
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts
b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 9a23382..47d7a5b 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -12,6 +12,7 @@
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/clock/maxim,max77802.h>
 #include "exynos5420.dtsi"

 / {
@@ -151,7 +152,7 @@
        status = "okay";
        clock-frequency = <400000>;

-       max77802-pmic@9 {
+       max77802: max77802-pmic@9 {
                compatible = "maxim,max77802";
                interrupt-parent = <&gpx3>;
                interrupts = <1 IRQ_TYPE_NONE>;
@@ -726,6 +727,8 @@
 };

 &rtc {
+       clocks = <&clock CLK_RTC>, <&max77802 MAX77802_CLK_32K_AP>;
+       clock-names = "rtc", "rtc_src";
        status = "okay";
 };

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Javier Martinez Canillas]

[adding Doug Anderson as cc]

On Tue, Sep 16, 2014 at 3:48 PM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> On Thu, Aug 28, 2014 at 11:02 AM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>> This patch add support for RTC of Exynos3250 SoC. The Exynos3250 needs source
>> clock(32.768KHz) for RTC block. If source clock of RTC is registerd on clock
>> list of common clk framework, Exynos RTC drvier have to control this clock.
>>
>> Clock list for s3c-rtc device:
>> - rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC.
>> - rtc_src : XrtcXTI is 32.768.kHz source clock for RTC.
>
> Is this RTC source clock needed for all Exynos SoCs?
>

By looking at the Exynos5420 user manual I see that there is indeed a
XrtcXTI clock pad and the doc says:

"XRTCXTI: Specifies a clock from 32.768 kHz crystal pad with XRTCXTI
and XRTCXTO pins. RTC uses this
clock as the source of a real-time clock."

Also by looking at the board schematic I see that the 32KHZAP clock
from the Maxim77802 PMIC is used as the external 32.768.kHz source
clock for the RTC but that information is not in the downstream
ChromeOS DTS since AFAIU the Maxim driver in that tree predates the
common clock framework so the PMIC clocks were modeled as regulators.

>> @@ -480,11 +530,19 @@ static int s3c_rtc_probe(struct platform_device *pdev)
>>
>>         info->rtc_clk = devm_clk_get(&pdev->dev, "rtc");
>>         if (IS_ERR(info->rtc_clk)) {
>> -               dev_err(&pdev->dev, "failed to find rtc clock source\n");
>> +               dev_err(&pdev->dev, "failed to find rtc clock\n");
>>                 return PTR_ERR(info->rtc_clk);
>>         }
>>         clk_prepare_enable(info->rtc_clk);
>>
>> +       info->rtc_src_clk = devm_clk_get(&pdev->dev, "rtc_src");
>> +       if (IS_ERR(info->rtc_src_clk)) {
>> +               dev_err(&pdev->dev, "failed to find rtc source clock\n");
>> +               return PTR_ERR(info->rtc_src_clk);
>
> On an Exynos5420 Peach Pit machine I'm having the following error and
> the driver fails to probe:
>
> [    2.095700] s3c-rtc: probe of 101e0000.rtc failed with error -2
>
> Reverting this patch fixes the issue and the s3c RTC works again on
> this machine.
>

The following change [0] in the Peach Pit DTS makes the RTC to claim
the external clock and is working again but I'm not sure if $subject
is correct in making the "rtc_src" property mandatory since it breaks
backward DT compatibility.

Best regards,
Javier

[0]
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts
b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 9a23382..47d7a5b 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -12,6 +12,7 @@
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/clock/maxim,max77802.h>
 #include "exynos5420.dtsi"

 / {
@@ -151,7 +152,7 @@
        status = "okay";
        clock-frequency = <400000>;

-       max77802-pmic@9 {
+       max77802: max77802-pmic@9 {
                compatible = "maxim,max77802";
                interrupt-parent = <&gpx3>;
                interrupts = <1 IRQ_TYPE_NONE>;
@@ -726,6 +727,8 @@
 };

 &rtc {
+       clocks = <&clock CLK_RTC>, <&max77802 MAX77802_CLK_32K_AP>;
+       clock-names = "rtc", "rtc_src";
        status = "okay";
 };

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Javier Martinez Canillas]

Hello Daniel,

On Tue, Sep 16, 2014 at 5:03 PM, Daniel Drake <drake@endlessm.com> wrote:
> On Tue, Sep 16, 2014 at 7:48 AM, Javier Martinez Canillas
> <javier@dowhile0.org> wrote:
>>> Clock list for s3c-rtc device:
>>> - rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC.
>>> - rtc_src : XrtcXTI is 32.768.kHz source clock for RTC.
>>
>> Is this RTC source clock needed for all Exynos SoCs?
>
> It is at least needed on Exynos4412, which has the XrtcXTI thing
> exactly as you describe. However the very standard setup there is to
> hook it up to the CP clock output of the MAX76686 PMIC. This CP clock
> is on by default, so you can potentially live without that detail
> being present in the DT.
>

Thanks for confirming for Exynos4412, I just answered my own email
saying that I found to be needed on Exynos5420 as well and as you
said, it was just working because the Maxim clocks were left on by
default.

> However... one small issue with this setup is that when you enable
> CONFIG_COMMON_CLK_MAX77686, the CP clock gets exposed in Linux's
> common clock framework, and Linux then turns it off because it
> believes it is unused. Then the RTC stops ticking.
>

Indeed, this is an issue about relying on default state. We had a
quite long discussion a couple of weeks ago about simplefb relying on
clocks and regulators left enabled by the bootloader but once these
were know to the kernel, the frameworks disable them because were
unused making simplefb to fail.

> So the rtc_src idea would also be good for Exynos4412. Maybe it would
> make sense to drop the needs_src_clk flag, and simply require/enable
> the src clock whenever it is present in the DT.
>

That sounds more sensible to me as well. I wonder what should happen
in this case with DT backward compatibility though. But as you said,
the external clock is required and the kernel will disable this clock
once is know to the CCF since is not used so maybe will be hard to
maintain DT backward compatibility in this case.

> Also, are you sure about the way you are treating this clock, all
> those enable/disable calls? You only seem to enable it when doing some
> particular driver operations e.g. reading the time, leaving it
> disabled at all other times. However I believe on Exynos4412 that if
> you disable this clock then the RTC will not tick.
>
> Daniel

Best regards,
Javier

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Javier Martinez Canillas]

Hello Daniel,

On Tue, Sep 16, 2014 at 5:03 PM, Daniel Drake <drake@endlessm.com> wrote:
> On Tue, Sep 16, 2014 at 7:48 AM, Javier Martinez Canillas
> <javier@dowhile0.org> wrote:
>>> Clock list for s3c-rtc device:
>>> - rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC.
>>> - rtc_src : XrtcXTI is 32.768.kHz source clock for RTC.
>>
>> Is this RTC source clock needed for all Exynos SoCs?
>
> It is at least needed on Exynos4412, which has the XrtcXTI thing
> exactly as you describe. However the very standard setup there is to
> hook it up to the CP clock output of the MAX76686 PMIC. This CP clock
> is on by default, so you can potentially live without that detail
> being present in the DT.
>

Thanks for confirming for Exynos4412, I just answered my own email
saying that I found to be needed on Exynos5420 as well and as you
said, it was just working because the Maxim clocks were left on by
default.

> However... one small issue with this setup is that when you enable
> CONFIG_COMMON_CLK_MAX77686, the CP clock gets exposed in Linux's
> common clock framework, and Linux then turns it off because it
> believes it is unused. Then the RTC stops ticking.
>

Indeed, this is an issue about relying on default state. We had a
quite long discussion a couple of weeks ago about simplefb relying on
clocks and regulators left enabled by the bootloader but once these
were know to the kernel, the frameworks disable them because were
unused making simplefb to fail.

> So the rtc_src idea would also be good for Exynos4412. Maybe it would
> make sense to drop the needs_src_clk flag, and simply require/enable
> the src clock whenever it is present in the DT.
>

That sounds more sensible to me as well. I wonder what should happen
in this case with DT backward compatibility though. But as you said,
the external clock is required and the kernel will disable this clock
once is know to the CCF since is not used so maybe will be hard to
maintain DT backward compatibility in this case.

> Also, are you sure about the way you are treating this clock, all
> those enable/disable calls? You only seem to enable it when doing some
> particular driver operations e.g. reading the time, leaving it
> disabled at all other times. However I believe on Exynos4412 that if
> you disable this clock then the RTC will not tick.
>
> Daniel

Best regards,
Javier

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Doug Anderson]

Hi,

On Tue, Sep 16, 2014 at 8:20 AM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Daniel,
>
> On Tue, Sep 16, 2014 at 5:03 PM, Daniel Drake <drake@endlessm.com> wrote:
>> On Tue, Sep 16, 2014 at 7:48 AM, Javier Martinez Canillas
>> <javier@dowhile0.org> wrote:
>>>> Clock list for s3c-rtc device:
>>>> - rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC.
>>>> - rtc_src : XrtcXTI is 32.768.kHz source clock for RTC.
>>>
>>> Is this RTC source clock needed for all Exynos SoCs?
>>
>> It is at least needed on Exynos4412, which has the XrtcXTI thing
>> exactly as you describe. However the very standard setup there is to
>> hook it up to the CP clock output of the MAX76686 PMIC. This CP clock
>> is on by default, so you can potentially live without that detail
>> being present in the DT.
>>
>
> Thanks for confirming for Exynos4412, I just answered my own email
> saying that I found to be needed on Exynos5420 as well and as you
> said, it was just working because the Maxim clocks were left on by
> default.
>
>> However... one small issue with this setup is that when you enable
>> CONFIG_COMMON_CLK_MAX77686, the CP clock gets exposed in Linux's
>> common clock framework, and Linux then turns it off because it
>> believes it is unused. Then the RTC stops ticking.
>>
>
> Indeed, this is an issue about relying on default state. We had a
> quite long discussion a couple of weeks ago about simplefb relying on
> clocks and regulators left enabled by the bootloader but once these
> were know to the kernel, the frameworks disable them because were
> unused making simplefb to fail.
>
>> So the rtc_src idea would also be good for Exynos4412. Maybe it would
>> make sense to drop the needs_src_clk flag, and simply require/enable
>> the src clock whenever it is present in the DT.
>>
>
> That sounds more sensible to me as well. I wonder what should happen
> in this case with DT backward compatibility though. But as you said,
> the external clock is required and the kernel will disable this clock
> once is know to the CCF since is not used so maybe will be hard to
> maintain DT backward compatibility in this case.

I think you can turn off CONFIG_COMMON_CLK_MAX77686 and then this
clock will be left at whatever the bootloader set it to, right?  Then
there will be no auto-disabling by the CCF and the RTC will work.
That's one argument for making the clock "optional".

NOTE: I don't think that the builtin RTC is terribly important for any
exynos-based Chromebooks that I'm aware of.  We rely on the RTC that's
part of the Maxim PMIC itself and pretty much ignore the one built-in
to the exynos.  I think there are some cases it was used (as a
fallback wakeup source in certain test scripts), but nothing very
important.

-Doug

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Doug Anderson]

Hi,

On Tue, Sep 16, 2014 at 8:20 AM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Daniel,
>
> On Tue, Sep 16, 2014 at 5:03 PM, Daniel Drake <drake@endlessm.com> wrote:
>> On Tue, Sep 16, 2014 at 7:48 AM, Javier Martinez Canillas
>> <javier@dowhile0.org> wrote:
>>>> Clock list for s3c-rtc device:
>>>> - rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC.
>>>> - rtc_src : XrtcXTI is 32.768.kHz source clock for RTC.
>>>
>>> Is this RTC source clock needed for all Exynos SoCs?
>>
>> It is at least needed on Exynos4412, which has the XrtcXTI thing
>> exactly as you describe. However the very standard setup there is to
>> hook it up to the CP clock output of the MAX76686 PMIC. This CP clock
>> is on by default, so you can potentially live without that detail
>> being present in the DT.
>>
>
> Thanks for confirming for Exynos4412, I just answered my own email
> saying that I found to be needed on Exynos5420 as well and as you
> said, it was just working because the Maxim clocks were left on by
> default.
>
>> However... one small issue with this setup is that when you enable
>> CONFIG_COMMON_CLK_MAX77686, the CP clock gets exposed in Linux's
>> common clock framework, and Linux then turns it off because it
>> believes it is unused. Then the RTC stops ticking.
>>
>
> Indeed, this is an issue about relying on default state. We had a
> quite long discussion a couple of weeks ago about simplefb relying on
> clocks and regulators left enabled by the bootloader but once these
> were know to the kernel, the frameworks disable them because were
> unused making simplefb to fail.
>
>> So the rtc_src idea would also be good for Exynos4412. Maybe it would
>> make sense to drop the needs_src_clk flag, and simply require/enable
>> the src clock whenever it is present in the DT.
>>
>
> That sounds more sensible to me as well. I wonder what should happen
> in this case with DT backward compatibility though. But as you said,
> the external clock is required and the kernel will disable this clock
> once is know to the CCF since is not used so maybe will be hard to
> maintain DT backward compatibility in this case.

I think you can turn off CONFIG_COMMON_CLK_MAX77686 and then this
clock will be left at whatever the bootloader set it to, right?  Then
there will be no auto-disabling by the CCF and the RTC will work.
That's one argument for making the clock "optional".

NOTE: I don't think that the builtin RTC is terribly important for any
exynos-based Chromebooks that I'm aware of.  We rely on the RTC that's
part of the Maxim PMIC itself and pretty much ignore the one built-in
to the exynos.  I think there are some cases it was used (as a
fallback wakeup source in certain test scripts), but nothing very
important.

-Doug

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Javier Martinez Canillas]

Hello Doug,

On Wed, Sep 17, 2014 at 12:15 AM, Doug Anderson <dianders@chromium.org> wrote:
>
> I think you can turn off CONFIG_COMMON_CLK_MAX77686 and then this
> clock will be left at whatever the bootloader set it to, right?  Then
> there will be no auto-disabling by the CCF and the RTC will work.

Yes, that's how Daniel was working around the issue.

> That's one argument for making the clock "optional".
>

Indeed.

> NOTE: I don't think that the builtin RTC is terribly important for any
> exynos-based Chromebooks that I'm aware of.  We rely on the RTC that's
> part of the Maxim PMIC itself and pretty much ignore the one built-in
> to the exynos.  I think there are some cases it was used (as a
> fallback wakeup source in certain test scripts), but nothing very
> important.
>

Ok, I'll post the patch I shared before that makes the rtc to claim
the max77802 32kHz clock as "rtc_src" anyways since that is the right
thing to do even if the "rtc_src" ends being optional due DT backward
compat.

> -Doug

Best regards,
Javier

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Javier Martinez Canillas]

Hello Doug,

On Wed, Sep 17, 2014 at 12:15 AM, Doug Anderson <dianders@chromium.org> wrote:
>
> I think you can turn off CONFIG_COMMON_CLK_MAX77686 and then this
> clock will be left at whatever the bootloader set it to, right?  Then
> there will be no auto-disabling by the CCF and the RTC will work.

Yes, that's how Daniel was working around the issue.

> That's one argument for making the clock "optional".
>

Indeed.

> NOTE: I don't think that the builtin RTC is terribly important for any
> exynos-based Chromebooks that I'm aware of.  We rely on the RTC that's
> part of the Maxim PMIC itself and pretty much ignore the one built-in
> to the exynos.  I think there are some cases it was used (as a
> fallback wakeup source in certain test scripts), but nothing very
> important.
>

Ok, I'll post the patch I shared before that makes the rtc to claim
the max77802 32kHz clock as "rtc_src" anyways since that is the right
thing to do even if the "rtc_src" ends being optional due DT backward
compat.

> -Doug

Best regards,
Javier

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Daniel Drake]

On Tue, Sep 16, 2014 at 4:15 PM, Doug Anderson <dianders@chromium.org> wrote:
> NOTE: I don't think that the builtin RTC is terribly important for any
> exynos-based Chromebooks that I'm aware of.  We rely on the RTC that's
> part of the Maxim PMIC itself and pretty much ignore the one built-in
> to the exynos.  I think there are some cases it was used (as a
> fallback wakeup source in certain test scripts), but nothing very
> important.

That's not true for all hardware though, at least the board I'm
working on now has the SoC RTC as battery-backed and the PMIC one with
no battery. So in this case at least, the interesting RTC is the SoC
one.

Daniel

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Daniel Drake]

On Tue, Sep 16, 2014 at 4:15 PM, Doug Anderson <dianders@chromium.org> wrote:
> NOTE: I don't think that the builtin RTC is terribly important for any
> exynos-based Chromebooks that I'm aware of.  We rely on the RTC that's
> part of the Maxim PMIC itself and pretty much ignore the one built-in
> to the exynos.  I think there are some cases it was used (as a
> fallback wakeup source in certain test scripts), but nothing very
> important.

That's not true for all hardware though, at least the board I'm
working on now has the SoC RTC as battery-backed and the PMIC one with
no battery. So in this case at least, the interesting RTC is the SoC
one.

Daniel

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Doug Anderson]

Daniel,

On Wed, Sep 17, 2014 at 9:49 AM, Daniel Drake <drake@endlessm.com> wrote:
> On Tue, Sep 16, 2014 at 4:15 PM, Doug Anderson <dianders@chromium.org> wrote:
>> NOTE: I don't think that the builtin RTC is terribly important for any
>> exynos-based Chromebooks that I'm aware of.  We rely on the RTC that's
>> part of the Maxim PMIC itself and pretty much ignore the one built-in
>> to the exynos.  I think there are some cases it was used (as a
>> fallback wakeup source in certain test scripts), but nothing very
>> important.
>
> That's not true for all hardware though, at least the board I'm
> working on now has the SoC RTC as battery-backed and the PMIC one with
> no battery. So in this case at least, the interesting RTC is the SoC
> one.

Yup, I can totally believe that.  My statement was meant only to apply
to the boards I knew about firsthand...

-Doug

Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC

[Doug Anderson]

Daniel,

On Wed, Sep 17, 2014 at 9:49 AM, Daniel Drake <drake@endlessm.com> wrote:
> On Tue, Sep 16, 2014 at 4:15 PM, Doug Anderson <dianders@chromium.org> wrote:
>> NOTE: I don't think that the builtin RTC is terribly important for any
>> exynos-based Chromebooks that I'm aware of.  We rely on the RTC that's
>> part of the Maxim PMIC itself and pretty much ignore the one built-in
>> to the exynos.  I think there are some cases it was used (as a
>> fallback wakeup source in certain test scripts), but nothing very
>> important.
>
> That's not true for all hardware though, at least the board I'm
> working on now has the SoC RTC as battery-backed and the PMIC one with
> no battery. So in this case at least, the interesting RTC is the SoC
> one.

Yup, I can totally believe that.  My statement was meant only to apply
to the boards I knew about firsthand...

-Doug