[PATCH 1/2] rtc: Fix handling of missing tm_year data when reading alarms

[PATCH 1/2] rtc: Fix handling of missing tm_year data when reading alarms

[Mark Brown]

When fixing up invalid years rtc_read_alarm() is callign rtc_valid_tm()
as a boolean but rtc_valid_tm() returns zero on success or a negative
number if the time is not valid so the test is inverted.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/rtc/interface.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index 7af60b9..a04c1b6 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -271,7 +271,7 @@ int rtc_read_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
 		dev_dbg(&rtc->dev, "alarm rollover: %s\n", "year");
 		do {
 			alarm->time.tm_year++;
-		} while (!rtc_valid_tm(&alarm->time));
+		} while (rtc_valid_tm(&alarm->time) != 0);
 		break;
 
 	default:
-- 
1.5.6.5

[PATCH 2/2] rtc: rtc-wm8350: Add support for WM8350 RTC

[Mark Brown]

This adds support for the RTC provided by the Wolfson Microelectronics
WM8350.

This driver was originally written by Graeme Gregory and Liam Girdwood,
though it has been modified since then to update it to current mainline
coding standards and for API completeness.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
Updated to use schedule_timeout_uninterruptible() as per Andrew's
suggestion.

 drivers/rtc/Kconfig            |   10 +
 drivers/rtc/Makefile           |    1 +
 drivers/rtc/rtc-wm8350.c       |  514 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/wm8350/rtc.h |    2 +
 4 files changed, 527 insertions(+), 0 deletions(-)
 create mode 100644 drivers/rtc/rtc-wm8350.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 8abbb20..7951ad2 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -468,6 +468,16 @@ config RTC_DRV_V3020
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-v3020.
 
+config RTC_DRV_WM8350
+	tristate "Wolfson Microelectronics WM8350 RTC"
+	depends on MFD_WM8350
+	help
+	  If you say yes here you will get support for the RTC subsystem
+	  of the Wolfson Microelectronics WM8350.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called "rtc-wm8350".
+
 comment "on-CPU RTC drivers"
 
 config RTC_DRV_OMAP
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index e9e8474..7a41201 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -66,4 +66,5 @@ obj-$(CONFIG_RTC_DRV_TEST)	+= rtc-test.o
 obj-$(CONFIG_RTC_DRV_TWL4030)	+= rtc-twl4030.o
 obj-$(CONFIG_RTC_DRV_V3020)	+= rtc-v3020.o
 obj-$(CONFIG_RTC_DRV_VR41XX)	+= rtc-vr41xx.o
+obj-$(CONFIG_RTC_DRV_WM8350)	+= rtc-wm8350.o
 obj-$(CONFIG_RTC_DRV_X1205)	+= rtc-x1205.o
diff --git a/drivers/rtc/rtc-wm8350.c b/drivers/rtc/rtc-wm8350.c
new file mode 100644
index 0000000..0a53652
--- /dev/null
+++ b/drivers/rtc/rtc-wm8350.c
@@ -0,0 +1,514 @@
+/*
+ *	Real Time Clock driver for Wolfson Microelectronics WM8350
+ *
+ *	Copyright (C) 2007, 2008 Wolfson Microelectronics PLC.
+ *
+ *  Author: Liam Girdwood
+ *          linux@wolfsonmicro.com
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/time.h>
+#include <linux/rtc.h>
+#include <linux/bcd.h>
+#include <linux/interrupt.h>
+#include <linux/ioctl.h>
+#include <linux/completion.h>
+#include <linux/mfd/wm8350/rtc.h>
+#include <linux/mfd/wm8350/core.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+
+#define WM8350_SET_ALM_RETRIES	5
+#define WM8350_SET_TIME_RETRIES	5
+#define WM8350_GET_TIME_RETRIES	5
+
+#define to_wm8350_from_rtc_dev(d) container_of(d, struct wm8350, rtc.pdev.dev)
+
+/*
+ * Read current time and date in RTC
+ */
+static int wm8350_rtc_readtime(struct device *dev, struct rtc_time *tm)
+{
+	struct wm8350 *wm8350 = dev_get_drvdata(dev);
+	u16 time1[4], time2[4];
+	int retries = WM8350_GET_TIME_RETRIES, ret;
+
+	/*
+	 * Read the time twice and compare.
+	 * If time1 == time2, then time is valid else retry.
+	 */
+	do {
+		ret = wm8350_block_read(wm8350, WM8350_RTC_SECONDS_MINUTES,
+					4, time1);
+		if (ret < 0)
+			return ret;
+		ret = wm8350_block_read(wm8350, WM8350_RTC_SECONDS_MINUTES,
+					4, time2);
+		if (ret < 0)
+			return ret;
+
+		if (memcmp(time1, time2, sizeof(time1)) == 0) {
+			tm->tm_sec = time1[0] & WM8350_RTC_SECS_MASK;
+
+			tm->tm_min = (time1[0] & WM8350_RTC_MINS_MASK)
+			    >> WM8350_RTC_MINS_SHIFT;
+
+			tm->tm_hour = time1[1] & WM8350_RTC_HRS_MASK;
+
+			tm->tm_wday = ((time1[1] >> WM8350_RTC_DAY_SHIFT)
+				       & 0x7) - 1;
+
+			tm->tm_mon = ((time1[2] & WM8350_RTC_MTH_MASK)
+				      >> WM8350_RTC_MTH_SHIFT) - 1;
+
+			tm->tm_mday = (time1[2] & WM8350_RTC_DATE_MASK);
+
+			tm->tm_year = ((time1[3] & WM8350_RTC_YHUNDREDS_MASK)
+				       >> WM8350_RTC_YHUNDREDS_SHIFT) * 100;
+			tm->tm_year += time1[3] & WM8350_RTC_YUNITS_MASK;
+
+			tm->tm_yday = rtc_year_days(tm->tm_mday, tm->tm_mon,
+						    tm->tm_year);
+			tm->tm_year -= 1900;
+
+			dev_dbg(dev, "Read (%d left): %04x %04x %04x %04x\n",
+				retries,
+				time1[0], time1[1], time1[2], time1[3]);
+
+			return 0;
+		}
+	} while (retries--);
+
+	dev_err(dev, "timed out reading RTC time\n");
+	return -EIO;
+}
+
+/*
+ * Set current time and date in RTC
+ */
+static int wm8350_rtc_settime(struct device *dev, struct rtc_time *tm)
+{
+	struct wm8350 *wm8350 = dev_get_drvdata(dev);
+	u16 time[4];
+	u16 rtc_ctrl;
+	int ret, retries = WM8350_SET_TIME_RETRIES;
+
+	time[0] = tm->tm_sec;
+	time[0] |= tm->tm_min << WM8350_RTC_MINS_SHIFT;
+	time[1] = tm->tm_hour;
+	time[1] |= (tm->tm_wday + 1) << WM8350_RTC_DAY_SHIFT;
+	time[2] = tm->tm_mday;
+	time[2] |= (tm->tm_mon + 1) << WM8350_RTC_MTH_SHIFT;
+	time[3] = ((tm->tm_year + 1900) / 100) << WM8350_RTC_YHUNDREDS_SHIFT;
+	time[3] |= (tm->tm_year + 1900) % 100;
+
+	dev_dbg(dev, "Setting: %04x %04x %04x %04x\n",
+		time[0], time[1], time[2], time[3]);
+
+	/* Set RTC_SET to stop the clock */
+	ret = wm8350_set_bits(wm8350, WM8350_RTC_TIME_CONTROL, WM8350_RTC_SET);
+	if (ret < 0)
+		return ret;
+
+	/* Wait until confirmation of stopping */
+	do {
+		rtc_ctrl = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
+		schedule_timeout_uninterruptible(msecs_to_jiffies(1));
+	} while (retries-- && !(rtc_ctrl & WM8350_RTC_STS));
+
+	if (!retries) {
+		dev_err(dev, "timed out on set confirmation\n");
+		return -EIO;
+	}
+
+	/* Write time to RTC */
+	ret = wm8350_block_write(wm8350, WM8350_RTC_SECONDS_MINUTES, 4, time);
+	if (ret < 0)
+		return ret;
+
+	/* Clear RTC_SET to start the clock */
+	ret = wm8350_clear_bits(wm8350, WM8350_RTC_TIME_CONTROL,
+				WM8350_RTC_SET);
+	return ret;
+}
+
+/*
+ * Read alarm time and date in RTC
+ */
+static int wm8350_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct wm8350 *wm8350 = dev_get_drvdata(dev);
+	struct rtc_time *tm = &alrm->time;
+	u16 time[4];
+	int ret;
+
+	ret = wm8350_block_read(wm8350, WM8350_ALARM_SECONDS_MINUTES, 4, time);
+	if (ret < 0)
+		return ret;
+
+	tm->tm_sec = time[0] & WM8350_RTC_ALMSECS_MASK;
+	if (tm->tm_sec == WM8350_RTC_ALMSECS_MASK)
+		tm->tm_sec = -1;
+
+	tm->tm_min = time[0] & WM8350_RTC_ALMMINS_MASK;
+	if (tm->tm_min == WM8350_RTC_ALMMINS_MASK)
+		tm->tm_min = -1;
+	else
+		tm->tm_min >>= WM8350_RTC_ALMMINS_SHIFT;
+
+	tm->tm_hour = time[1] & WM8350_RTC_ALMHRS_MASK;
+	if (tm->tm_hour == WM8350_RTC_ALMHRS_MASK)
+		tm->tm_hour = -1;
+
+	tm->tm_wday = ((time[1] >> WM8350_RTC_ALMDAY_SHIFT) & 0x7) - 1;
+	if (tm->tm_wday > 7)
+		tm->tm_wday = -1;
+
+	tm->tm_mon = time[2] & WM8350_RTC_ALMMTH_MASK;
+	if (tm->tm_mon == WM8350_RTC_ALMMTH_MASK)
+		tm->tm_mon = -1;
+	else
+		tm->tm_mon = (tm->tm_mon >> WM8350_RTC_ALMMTH_SHIFT) - 1;
+
+	tm->tm_mday = (time[2] & WM8350_RTC_ALMDATE_MASK);
+	if (tm->tm_mday == WM8350_RTC_ALMDATE_MASK)
+		tm->tm_mday = -1;
+
+	tm->tm_year = -1;
+
+	alrm->enabled = !(time[3] & WM8350_RTC_ALMSTS);
+
+	return 0;
+}
+
+static int wm8350_rtc_stop_alarm(struct wm8350 *wm8350)
+{
+	int retries = WM8350_SET_ALM_RETRIES;
+	u16 rtc_ctrl;
+	int ret;
+
+	/* Set RTC_SET to stop the clock */
+	ret = wm8350_set_bits(wm8350, WM8350_RTC_TIME_CONTROL,
+			      WM8350_RTC_ALMSET);
+	if (ret < 0)
+		return ret;
+
+	/* Wait until confirmation of stopping */
+	do {
+		rtc_ctrl = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
+		schedule_timeout_uninterruptible(msecs_to_jiffies(1));
+	} while (retries-- && !(rtc_ctrl & WM8350_RTC_ALMSTS));
+
+	if (!(rtc_ctrl & WM8350_RTC_ALMSTS))
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int wm8350_rtc_start_alarm(struct wm8350 *wm8350)
+{
+	int ret;
+	int retries = WM8350_SET_ALM_RETRIES;
+	u16 rtc_ctrl;
+
+	ret = wm8350_clear_bits(wm8350, WM8350_RTC_TIME_CONTROL,
+				WM8350_RTC_ALMSET);
+	if (ret < 0)
+		return ret;
+
+	/* Wait until confirmation */
+	do {
+		rtc_ctrl = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
+		schedule_timeout_uninterruptible(msecs_to_jiffies(1));
+	} while (retries-- && rtc_ctrl & WM8350_RTC_ALMSTS);
+
+	if (rtc_ctrl & WM8350_RTC_ALMSTS)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int wm8350_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct wm8350 *wm8350 = dev_get_drvdata(dev);
+	struct rtc_time *tm = &alrm->time;
+	u16 time[3];
+	int ret;
+
+	memset(time, 0, sizeof(time));
+
+	if (tm->tm_sec != -1)
+		time[0] |= tm->tm_sec;
+	else
+		time[0] |= WM8350_RTC_ALMSECS_MASK;
+
+	if (tm->tm_min != -1)
+		time[0] |= tm->tm_min << WM8350_RTC_ALMMINS_SHIFT;
+	else
+		time[0] |= WM8350_RTC_ALMMINS_MASK;
+
+	if (tm->tm_hour != -1)
+		time[1] |= tm->tm_hour;
+	else
+		time[1] |= WM8350_RTC_ALMHRS_MASK;
+
+	if (tm->tm_wday != -1)
+		time[1] |= (tm->tm_wday + 1) << WM8350_RTC_ALMDAY_SHIFT;
+	else
+		time[1] |= WM8350_RTC_ALMDAY_MASK;
+
+	if (tm->tm_mday != -1)
+		time[2] |= tm->tm_mday;
+	else
+		time[2] |= WM8350_RTC_ALMDATE_MASK;
+
+	if (tm->tm_mon != -1)
+		time[2] |= (tm->tm_mon + 1) << WM8350_RTC_ALMMTH_SHIFT;
+	else
+		time[2] |= WM8350_RTC_ALMMTH_MASK;
+
+	ret = wm8350_rtc_stop_alarm(wm8350);
+	if (ret < 0)
+		return ret;
+
+	/* Write time to RTC */
+	ret = wm8350_block_write(wm8350, WM8350_ALARM_SECONDS_MINUTES,
+				 3, time);
+	if (ret < 0)
+		return ret;
+
+	if (alrm->enabled)
+		ret = wm8350_rtc_start_alarm(wm8350);
+
+	return ret;
+}
+
+/*
+ * Handle commands from user-space
+ */
+static int wm8350_rtc_ioctl(struct device *dev, unsigned int cmd,
+			    unsigned long arg)
+{
+	struct wm8350 *wm8350 = dev_get_drvdata(dev);
+
+	switch (cmd) {
+	case RTC_AIE_OFF:
+		return wm8350_rtc_stop_alarm(wm8350);
+	case RTC_AIE_ON:
+		return wm8350_rtc_start_alarm(wm8350);
+
+	case RTC_UIE_OFF:
+		wm8350_mask_irq(wm8350, WM8350_IRQ_RTC_SEC);
+		break;
+	case RTC_UIE_ON:
+		wm8350_unmask_irq(wm8350, WM8350_IRQ_RTC_SEC);
+		break;
+
+	default:
+		return -ENOIOCTLCMD;
+	}
+
+	return 0;
+}
+
+static void wm8350_rtc_alarm_handler(struct wm8350 *wm8350, int irq,
+				     void *data)
+{
+	struct rtc_device *rtc = wm8350->rtc.rtc;
+	int ret;
+
+	rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
+
+	/* Make it one shot */
+	ret = wm8350_set_bits(wm8350, WM8350_RTC_TIME_CONTROL,
+			      WM8350_RTC_ALMSET);
+	if (ret != 0) {
+		dev_err(&(wm8350->rtc.pdev->dev),
+			"Failed to disable alarm: %d\n", ret);
+	}
+}
+
+static void wm8350_rtc_update_handler(struct wm8350 *wm8350, int irq,
+				      void *data)
+{
+	struct rtc_device *rtc = wm8350->rtc.rtc;
+
+	rtc_update_irq(rtc, 1, RTC_IRQF | RTC_UF);
+}
+
+static const struct rtc_class_ops wm8350_rtc_ops = {
+	.ioctl = wm8350_rtc_ioctl,
+	.read_time = wm8350_rtc_readtime,
+	.set_time = wm8350_rtc_settime,
+	.read_alarm = wm8350_rtc_readalarm,
+	.set_alarm = wm8350_rtc_setalarm,
+};
+
+#ifdef CONFIG_PM
+static int wm8350_rtc_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	struct wm8350 *wm8350 = dev_get_drvdata(&pdev->dev);
+	int ret = 0;
+	u16 reg;
+
+	reg = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
+
+	if (device_may_wakeup(&wm8350->rtc.pdev->dev) &&
+	    reg & WM8350_RTC_ALMSTS) {
+		ret = wm8350_rtc_stop_alarm(wm8350);
+		if (ret != 0)
+			dev_err(&pdev->dev, "Failed to stop RTC alarm: %d\n",
+				ret);
+	}
+
+	return ret;
+}
+
+static int wm8350_rtc_resume(struct platform_device *pdev)
+{
+	struct wm8350 *wm8350 = dev_get_drvdata(&pdev->dev);
+	int ret;
+
+	if (wm8350->rtc.alarm_enabled) {
+		ret = wm8350_rtc_start_alarm(wm8350);
+		if (ret != 0)
+			dev_err(&pdev->dev,
+				"Failed to restart RTC alarm: %d\n", ret);
+	}
+
+	return 0;
+}
+
+#else
+#define wm8350_rtc_suspend NULL
+#define wm8350_rtc_resume NULL
+#endif
+
+static int wm8350_rtc_probe(struct platform_device *pdev)
+{
+	struct wm8350 *wm8350 = platform_get_drvdata(pdev);
+	struct wm8350_rtc *wm_rtc = &wm8350->rtc;
+	int ret = 0;
+	u16 timectl, power5;
+
+	timectl = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
+	if (timectl & WM8350_RTC_BCD) {
+		dev_err(&pdev->dev, "RTC BCD mode not supported\n");
+		return -EINVAL;
+	}
+	if (timectl & WM8350_RTC_12HR) {
+		dev_err(&pdev->dev, "RTC 12 hour mode not supported\n");
+		return -EINVAL;
+	}
+
+	/* enable the RTC if it's not already enabled */
+	power5 = wm8350_reg_read(wm8350, WM8350_POWER_MGMT_5);
+	if (!(power5 &  WM8350_RTC_TICK_ENA)) {
+		dev_info(wm8350->dev, "Starting RTC\n");
+
+		wm8350_reg_unlock(wm8350);
+
+		ret = wm8350_set_bits(wm8350, WM8350_POWER_MGMT_5,
+				      WM8350_RTC_TICK_ENA);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "failed to enable RTC: %d\n", ret);
+			return ret;
+		}
+
+		wm8350_reg_lock(wm8350);
+	}
+
+	if (timectl & WM8350_RTC_STS) {
+		int retries;
+
+		ret = wm8350_clear_bits(wm8350, WM8350_RTC_TIME_CONTROL,
+					WM8350_RTC_SET);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "failed to start: %d\n", ret);
+			return ret;
+		}
+
+		retries = WM8350_SET_TIME_RETRIES;
+		do {
+			timectl = wm8350_reg_read(wm8350,
+						  WM8350_RTC_TIME_CONTROL);
+		} while (timectl & WM8350_RTC_STS && retries--);
+
+		if (retries == 0) {
+			dev_err(&pdev->dev, "failed to start: timeout\n");
+			return -ENODEV;
+		}
+	}
+
+	device_init_wakeup(&pdev->dev, 1);
+
+	wm_rtc->rtc = rtc_device_register("wm8350", &pdev->dev,
+					  &wm8350_rtc_ops, THIS_MODULE);
+	if (IS_ERR(wm_rtc->rtc)) {
+		ret = PTR_ERR(wm_rtc->rtc);
+		dev_err(&pdev->dev, "failed to register RTC: %d\n", ret);
+		return ret;
+	}
+
+	wm8350_mask_irq(wm8350, WM8350_IRQ_RTC_SEC);
+	wm8350_mask_irq(wm8350, WM8350_IRQ_RTC_PER);
+
+	wm8350_register_irq(wm8350, WM8350_IRQ_RTC_SEC,
+			    wm8350_rtc_update_handler, NULL);
+
+	wm8350_register_irq(wm8350, WM8350_IRQ_RTC_ALM,
+			    wm8350_rtc_alarm_handler, NULL);
+	wm8350_unmask_irq(wm8350, WM8350_IRQ_RTC_ALM);
+
+	return 0;
+}
+
+static int __devexit wm8350_rtc_remove(struct platform_device *pdev)
+{
+	struct wm8350 *wm8350 = platform_get_drvdata(pdev);
+	struct wm8350_rtc *wm_rtc = &wm8350->rtc;
+
+	wm8350_mask_irq(wm8350, WM8350_IRQ_RTC_SEC);
+
+	wm8350_free_irq(wm8350, WM8350_IRQ_RTC_SEC);
+	wm8350_free_irq(wm8350, WM8350_IRQ_RTC_ALM);
+
+	rtc_device_unregister(wm_rtc->rtc);
+
+	return 0;
+}
+
+static struct platform_driver wm8350_rtc_driver = {
+	.probe = wm8350_rtc_probe,
+	.remove = wm8350_rtc_remove,
+	.suspend = wm8350_rtc_suspend,
+	.resume = wm8350_rtc_resume,
+	.driver = {
+		.name = "wm8350-rtc",
+	},
+};
+
+static int __init wm8350_rtc_init(void)
+{
+	return platform_driver_register(&wm8350_rtc_driver);
+}
+module_init(wm8350_rtc_init);
+
+static void __exit wm8350_rtc_exit(void)
+{
+	platform_driver_unregister(&wm8350_rtc_driver);
+}
+module_exit(wm8350_rtc_exit);
+
+MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>");
+MODULE_DESCRIPTION("RTC driver WM8350");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:wm8350-rtc");
diff --git a/include/linux/mfd/wm8350/rtc.h b/include/linux/mfd/wm8350/rtc.h
index dfda69e..24add2b 100644
--- a/include/linux/mfd/wm8350/rtc.h
+++ b/include/linux/mfd/wm8350/rtc.h
@@ -261,6 +261,8 @@
 
 struct wm8350_rtc {
 	struct platform_device *pdev;
+	struct rtc_device *rtc;
+	int alarm_enabled;      /* used over suspend/resume */
 };
 
 #endif
-- 
1.5.6.5

Re: [rtc-linux] [PATCH 2/2] rtc: rtc-wm8350: Add support for WM8350 RTC

[Alessandro Zummo]

On Mon, 27 Oct 2008 15:38:25 +0000
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> 
> This adds support for the RTC provided by the Wolfson Microelectronics
> WM8350.
> 
> This driver was originally written by Graeme Gregory and Liam Girdwood,
> though it has been modified since then to update it to current mainline
> coding standards and for API completeness.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

 Hi Mark,

  a few notes below:


 (and a detailed checklist at http://groups.google.com/group/rtc-linux/web/checklist )

> ---
> Updated to use schedule_timeout_uninterruptible() as per Andrew's
> suggestion.
> 
>  drivers/rtc/Kconfig            |   10 +
>  drivers/rtc/Makefile           |    1 +
>  drivers/rtc/rtc-wm8350.c       |  514 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/wm8350/rtc.h |    2 +
>  4 files changed, 527 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/rtc/rtc-wm8350.c
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 8abbb20..7951ad2 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -468,6 +468,16 @@ config RTC_DRV_V3020
>  	  This driver can also be built as a module. If so, the module
>  	  will be called rtc-v3020.
>  
> +config RTC_DRV_WM8350
> +	tristate "Wolfson Microelectronics WM8350 RTC"
> +	depends on MFD_WM8350
> +	help
> +	  If you say yes here you will get support for the RTC subsystem
> +	  of the Wolfson Microelectronics WM8350.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called "rtc-wm8350".
> +
>  comment "on-CPU RTC drivers"
>  
>  config RTC_DRV_OMAP
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index e9e8474..7a41201 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -66,4 +66,5 @@ obj-$(CONFIG_RTC_DRV_TEST)	+= rtc-test.o
>  obj-$(CONFIG_RTC_DRV_TWL4030)	+= rtc-twl4030.o
>  obj-$(CONFIG_RTC_DRV_V3020)	+= rtc-v3020.o
>  obj-$(CONFIG_RTC_DRV_VR41XX)	+= rtc-vr41xx.o
> +obj-$(CONFIG_RTC_DRV_WM8350)	+= rtc-wm8350.o
>  obj-$(CONFIG_RTC_DRV_X1205)	+= rtc-x1205.o
> diff --git a/drivers/rtc/rtc-wm8350.c b/drivers/rtc/rtc-wm8350.c
> new file mode 100644
> index 0000000..0a53652
> --- /dev/null
> +++ b/drivers/rtc/rtc-wm8350.c
> @@ -0,0 +1,514 @@
> +/*
> + *	Real Time Clock driver for Wolfson Microelectronics WM8350
> + *
> + *	Copyright (C) 2007, 2008 Wolfson Microelectronics PLC.
> + *
> + *  Author: Liam Girdwood
> + *          linux@wolfsonmicro.com
> + *
> + *  This program is free software; you can redistribute  it and/or modify it
> + *  under  the terms of  the GNU General  Public License as published by the
> + *  Free Software Foundation;  either version 2 of the  License, or (at your
> + *  option) any later version.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/time.h>
> +#include <linux/rtc.h>
> +#include <linux/bcd.h>
> +#include <linux/interrupt.h>
> +#include <linux/ioctl.h>
> +#include <linux/completion.h>
> +#include <linux/mfd/wm8350/rtc.h>
> +#include <linux/mfd/wm8350/core.h>
> +#include <linux/delay.h>
> +#include <linux/platform_device.h>
> +
> +#define WM8350_SET_ALM_RETRIES	5
> +#define WM8350_SET_TIME_RETRIES	5
> +#define WM8350_GET_TIME_RETRIES	5
> +
> +#define to_wm8350_from_rtc_dev(d) container_of(d, struct wm8350, rtc.pdev.dev)
> +
> +/*
> + * Read current time and date in RTC
> + */
> +static int wm8350_rtc_readtime(struct device *dev, struct rtc_time *tm)
> +{
> +	struct wm8350 *wm8350 = dev_get_drvdata(dev);
> +	u16 time1[4], time2[4];
> +	int retries = WM8350_GET_TIME_RETRIES, ret;
> +
> +	/*
> +	 * Read the time twice and compare.
> +	 * If time1 == time2, then time is valid else retry.
> +	 */
> +	do {
> +		ret = wm8350_block_read(wm8350, WM8350_RTC_SECONDS_MINUTES,
> +					4, time1);
> +		if (ret < 0)
> +			return ret;
> +		ret = wm8350_block_read(wm8350, WM8350_RTC_SECONDS_MINUTES,
> +					4, time2);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (memcmp(time1, time2, sizeof(time1)) == 0) {
> +			tm->tm_sec = time1[0] & WM8350_RTC_SECS_MASK;
> +
> +			tm->tm_min = (time1[0] & WM8350_RTC_MINS_MASK)
> +			    >> WM8350_RTC_MINS_SHIFT;
> +
> +			tm->tm_hour = time1[1] & WM8350_RTC_HRS_MASK;
> +
> +			tm->tm_wday = ((time1[1] >> WM8350_RTC_DAY_SHIFT)
> +				       & 0x7) - 1;
> +
> +			tm->tm_mon = ((time1[2] & WM8350_RTC_MTH_MASK)
> +				      >> WM8350_RTC_MTH_SHIFT) - 1;
> +
> +			tm->tm_mday = (time1[2] & WM8350_RTC_DATE_MASK);
> +
> +			tm->tm_year = ((time1[3] & WM8350_RTC_YHUNDREDS_MASK)
> +				       >> WM8350_RTC_YHUNDREDS_SHIFT) * 100;
> +			tm->tm_year += time1[3] & WM8350_RTC_YUNITS_MASK;
> +
> +			tm->tm_yday = rtc_year_days(tm->tm_mday, tm->tm_mon,
> +						    tm->tm_year);
> +			tm->tm_year -= 1900;
> +
> +			dev_dbg(dev, "Read (%d left): %04x %04x %04x %04x\n",
> +				retries,
> +				time1[0], time1[1], time1[2], time1[3]);
> +
> +			return 0;
> +		}
> +	} while (retries--);
> +
> +	dev_err(dev, "timed out reading RTC time\n");
> +	return -EIO;
> +}
> +
> +/*
> + * Set current time and date in RTC
> + */
> +static int wm8350_rtc_settime(struct device *dev, struct rtc_time *tm)
> +{
> +	struct wm8350 *wm8350 = dev_get_drvdata(dev);
> +	u16 time[4];
> +	u16 rtc_ctrl;
> +	int ret, retries = WM8350_SET_TIME_RETRIES;
> +
> +	time[0] = tm->tm_sec;
> +	time[0] |= tm->tm_min << WM8350_RTC_MINS_SHIFT;
> +	time[1] = tm->tm_hour;
> +	time[1] |= (tm->tm_wday + 1) << WM8350_RTC_DAY_SHIFT;
> +	time[2] = tm->tm_mday;
> +	time[2] |= (tm->tm_mon + 1) << WM8350_RTC_MTH_SHIFT;
> +	time[3] = ((tm->tm_year + 1900) / 100) << WM8350_RTC_YHUNDREDS_SHIFT;
> +	time[3] |= (tm->tm_year + 1900) % 100;
> +
> +	dev_dbg(dev, "Setting: %04x %04x %04x %04x\n",
> +		time[0], time[1], time[2], time[3]);
> +
> +	/* Set RTC_SET to stop the clock */
> +	ret = wm8350_set_bits(wm8350, WM8350_RTC_TIME_CONTROL, WM8350_RTC_SET);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Wait until confirmation of stopping */
> +	do {
> +		rtc_ctrl = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
> +		schedule_timeout_uninterruptible(msecs_to_jiffies(1));
> +	} while (retries-- && !(rtc_ctrl & WM8350_RTC_STS));
> +
> +	if (!retries) {
> +		dev_err(dev, "timed out on set confirmation\n");
> +		return -EIO;
> +	}
> +
> +	/* Write time to RTC */
> +	ret = wm8350_block_write(wm8350, WM8350_RTC_SECONDS_MINUTES, 4, time);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Clear RTC_SET to start the clock */
> +	ret = wm8350_clear_bits(wm8350, WM8350_RTC_TIME_CONTROL,
> +				WM8350_RTC_SET);
> +	return ret;
> +}
> +
> +/*
> + * Read alarm time and date in RTC
> + */
> +static int wm8350_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> +	struct wm8350 *wm8350 = dev_get_drvdata(dev);
> +	struct rtc_time *tm = &alrm->time;
> +	u16 time[4];
> +	int ret;
> +
> +	ret = wm8350_block_read(wm8350, WM8350_ALARM_SECONDS_MINUTES, 4, time);
> +	if (ret < 0)
> +		return ret;
> +
> +	tm->tm_sec = time[0] & WM8350_RTC_ALMSECS_MASK;
> +	if (tm->tm_sec == WM8350_RTC_ALMSECS_MASK)
> +		tm->tm_sec = -1;
> +
> +	tm->tm_min = time[0] & WM8350_RTC_ALMMINS_MASK;
> +	if (tm->tm_min == WM8350_RTC_ALMMINS_MASK)
> +		tm->tm_min = -1;
> +	else
> +		tm->tm_min >>= WM8350_RTC_ALMMINS_SHIFT;
> +
> +	tm->tm_hour = time[1] & WM8350_RTC_ALMHRS_MASK;
> +	if (tm->tm_hour == WM8350_RTC_ALMHRS_MASK)
> +		tm->tm_hour = -1;
> +
> +	tm->tm_wday = ((time[1] >> WM8350_RTC_ALMDAY_SHIFT) & 0x7) - 1;
> +	if (tm->tm_wday > 7)
> +		tm->tm_wday = -1;
> +
> +	tm->tm_mon = time[2] & WM8350_RTC_ALMMTH_MASK;
> +	if (tm->tm_mon == WM8350_RTC_ALMMTH_MASK)
> +		tm->tm_mon = -1;
> +	else
> +		tm->tm_mon = (tm->tm_mon >> WM8350_RTC_ALMMTH_SHIFT) - 1;
> +
> +	tm->tm_mday = (time[2] & WM8350_RTC_ALMDATE_MASK);
> +	if (tm->tm_mday == WM8350_RTC_ALMDATE_MASK)
> +		tm->tm_mday = -1;
> +
> +	tm->tm_year = -1;
> +
> +	alrm->enabled = !(time[3] & WM8350_RTC_ALMSTS);
> +
> +	return 0;
> +}
> +
> +static int wm8350_rtc_stop_alarm(struct wm8350 *wm8350)
> +{
> +	int retries = WM8350_SET_ALM_RETRIES;
> +	u16 rtc_ctrl;
> +	int ret;
> +
> +	/* Set RTC_SET to stop the clock */
> +	ret = wm8350_set_bits(wm8350, WM8350_RTC_TIME_CONTROL,
> +			      WM8350_RTC_ALMSET);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Wait until confirmation of stopping */
> +	do {
> +		rtc_ctrl = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
> +		schedule_timeout_uninterruptible(msecs_to_jiffies(1));
> +	} while (retries-- && !(rtc_ctrl & WM8350_RTC_ALMSTS));
> +
> +	if (!(rtc_ctrl & WM8350_RTC_ALMSTS))
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
> +static int wm8350_rtc_start_alarm(struct wm8350 *wm8350)
> +{
> +	int ret;
> +	int retries = WM8350_SET_ALM_RETRIES;
> +	u16 rtc_ctrl;
> +
> +	ret = wm8350_clear_bits(wm8350, WM8350_RTC_TIME_CONTROL,
> +				WM8350_RTC_ALMSET);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Wait until confirmation */
> +	do {
> +		rtc_ctrl = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
> +		schedule_timeout_uninterruptible(msecs_to_jiffies(1));
> +	} while (retries-- && rtc_ctrl & WM8350_RTC_ALMSTS);
> +
> +	if (rtc_ctrl & WM8350_RTC_ALMSTS)
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
> +static int wm8350_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> +	struct wm8350 *wm8350 = dev_get_drvdata(dev);
> +	struct rtc_time *tm = &alrm->time;
> +	u16 time[3];
> +	int ret;
> +
> +	memset(time, 0, sizeof(time));
> +
> +	if (tm->tm_sec != -1)
> +		time[0] |= tm->tm_sec;
> +	else
> +		time[0] |= WM8350_RTC_ALMSECS_MASK;
> +
> +	if (tm->tm_min != -1)
> +		time[0] |= tm->tm_min << WM8350_RTC_ALMMINS_SHIFT;
> +	else
> +		time[0] |= WM8350_RTC_ALMMINS_MASK;
> +
> +	if (tm->tm_hour != -1)
> +		time[1] |= tm->tm_hour;
> +	else
> +		time[1] |= WM8350_RTC_ALMHRS_MASK;
> +
> +	if (tm->tm_wday != -1)
> +		time[1] |= (tm->tm_wday + 1) << WM8350_RTC_ALMDAY_SHIFT;
> +	else
> +		time[1] |= WM8350_RTC_ALMDAY_MASK;
> +
> +	if (tm->tm_mday != -1)
> +		time[2] |= tm->tm_mday;
> +	else
> +		time[2] |= WM8350_RTC_ALMDATE_MASK;
> +
> +	if (tm->tm_mon != -1)
> +		time[2] |= (tm->tm_mon + 1) << WM8350_RTC_ALMMTH_SHIFT;
> +	else
> +		time[2] |= WM8350_RTC_ALMMTH_MASK;
> +
> +	ret = wm8350_rtc_stop_alarm(wm8350);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Write time to RTC */
> +	ret = wm8350_block_write(wm8350, WM8350_ALARM_SECONDS_MINUTES,
> +				 3, time);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (alrm->enabled)
> +		ret = wm8350_rtc_start_alarm(wm8350);
> +
> +	return ret;
> +}
> +
> +/*
> + * Handle commands from user-space
> + */
> +static int wm8350_rtc_ioctl(struct device *dev, unsigned int cmd,
> +			    unsigned long arg)
> +{
> +	struct wm8350 *wm8350 = dev_get_drvdata(dev);
> +
> +	switch (cmd) {
> +	case RTC_AIE_OFF:
> +		return wm8350_rtc_stop_alarm(wm8350);
> +	case RTC_AIE_ON:
> +		return wm8350_rtc_start_alarm(wm8350);
> +
> +	case RTC_UIE_OFF:
> +		wm8350_mask_irq(wm8350, WM8350_IRQ_RTC_SEC);
> +		break;
> +	case RTC_UIE_ON:
> +		wm8350_unmask_irq(wm8350, WM8350_IRQ_RTC_SEC);
> +		break;
> +
> +	default:
> +		return -ENOIOCTLCMD;
> +	}
> +
> +	return 0;
> +}
> +
> +static void wm8350_rtc_alarm_handler(struct wm8350 *wm8350, int irq,
> +				     void *data)
> +{
> +	struct rtc_device *rtc = wm8350->rtc.rtc;
> +	int ret;
> +
> +	rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
> +
> +	/* Make it one shot */
> +	ret = wm8350_set_bits(wm8350, WM8350_RTC_TIME_CONTROL,
> +			      WM8350_RTC_ALMSET);
> +	if (ret != 0) {
> +		dev_err(&(wm8350->rtc.pdev->dev),
> +			"Failed to disable alarm: %d\n", ret);
> +	}
> +}
> +
> +static void wm8350_rtc_update_handler(struct wm8350 *wm8350, int irq,
> +				      void *data)
> +{
> +	struct rtc_device *rtc = wm8350->rtc.rtc;
> +
> +	rtc_update_irq(rtc, 1, RTC_IRQF | RTC_UF);
> +}
> +
> +static const struct rtc_class_ops wm8350_rtc_ops = {
> +	.ioctl = wm8350_rtc_ioctl,
> +	.read_time = wm8350_rtc_readtime,
> +	.set_time = wm8350_rtc_settime,
> +	.read_alarm = wm8350_rtc_readalarm,
> +	.set_alarm = wm8350_rtc_setalarm,
> +};
> +
> +#ifdef CONFIG_PM
> +static int wm8350_rtc_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> +	struct wm8350 *wm8350 = dev_get_drvdata(&pdev->dev);
> +	int ret = 0;
> +	u16 reg;
> +
> +	reg = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
> +
> +	if (device_may_wakeup(&wm8350->rtc.pdev->dev) &&
> +	    reg & WM8350_RTC_ALMSTS) {
> +		ret = wm8350_rtc_stop_alarm(wm8350);
> +		if (ret != 0)
> +			dev_err(&pdev->dev, "Failed to stop RTC alarm: %d\n",
> +				ret);
> +	}
> +
> +	return ret;
> +}
> +
> +static int wm8350_rtc_resume(struct platform_device *pdev)
> +{
> +	struct wm8350 *wm8350 = dev_get_drvdata(&pdev->dev);
> +	int ret;
> +
> +	if (wm8350->rtc.alarm_enabled) {
> +		ret = wm8350_rtc_start_alarm(wm8350);
> +		if (ret != 0)
> +			dev_err(&pdev->dev,
> +				"Failed to restart RTC alarm: %d\n", ret);
> +	}
> +
> +	return 0;
> +}
> +
> +#else
> +#define wm8350_rtc_suspend NULL
> +#define wm8350_rtc_resume NULL
> +#endif
> +
> +static int wm8350_rtc_probe(struct platform_device *pdev)
> +{
> +	struct wm8350 *wm8350 = platform_get_drvdata(pdev);
> +	struct wm8350_rtc *wm_rtc = &wm8350->rtc;
> +	int ret = 0;
> +	u16 timectl, power5;
> +
> +	timectl = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
> +	if (timectl & WM8350_RTC_BCD) {
> +		dev_err(&pdev->dev, "RTC BCD mode not supported\n");
> +		return -EINVAL;
> +	}
> +	if (timectl & WM8350_RTC_12HR) {
> +		dev_err(&pdev->dev, "RTC 12 hour mode not supported\n");
> +		return -EINVAL;
> +	}
> +
> +	/* enable the RTC if it's not already enabled */
> +	power5 = wm8350_reg_read(wm8350, WM8350_POWER_MGMT_5);
> +	if (!(power5 &  WM8350_RTC_TICK_ENA)) {
> +		dev_info(wm8350->dev, "Starting RTC\n");
> +
> +		wm8350_reg_unlock(wm8350);
> +
> +		ret = wm8350_set_bits(wm8350, WM8350_POWER_MGMT_5,
> +				      WM8350_RTC_TICK_ENA);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev, "failed to enable RTC: %d\n", ret);
> +			return ret;
> +		}
> +
> +		wm8350_reg_lock(wm8350);
> +	}
> +
> +	if (timectl & WM8350_RTC_STS) {
> +		int retries;
> +
> +		ret = wm8350_clear_bits(wm8350, WM8350_RTC_TIME_CONTROL,
> +					WM8350_RTC_SET);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev, "failed to start: %d\n", ret);
> +			return ret;
> +		}
> +
> +		retries = WM8350_SET_TIME_RETRIES;
> +		do {
> +			timectl = wm8350_reg_read(wm8350,
> +						  WM8350_RTC_TIME_CONTROL);
> +		} while (timectl & WM8350_RTC_STS && retries--);
> +
> +		if (retries == 0) {
> +			dev_err(&pdev->dev, "failed to start: timeout\n");
> +			return -ENODEV;
> +		}
> +	}
> +
> +	device_init_wakeup(&pdev->dev, 1);
> +
> +	wm_rtc->rtc = rtc_device_register("wm8350", &pdev->dev,
> +					  &wm8350_rtc_ops, THIS_MODULE);
> +	if (IS_ERR(wm_rtc->rtc)) {
> +		ret = PTR_ERR(wm_rtc->rtc);
> +		dev_err(&pdev->dev, "failed to register RTC: %d\n", ret);
> +		return ret;
> +	}
> +
> +	wm8350_mask_irq(wm8350, WM8350_IRQ_RTC_SEC);
> +	wm8350_mask_irq(wm8350, WM8350_IRQ_RTC_PER);
> +
> +	wm8350_register_irq(wm8350, WM8350_IRQ_RTC_SEC,
> +			    wm8350_rtc_update_handler, NULL);
> +
> +	wm8350_register_irq(wm8350, WM8350_IRQ_RTC_ALM,
> +			    wm8350_rtc_alarm_handler, NULL);
> +	wm8350_unmask_irq(wm8350, WM8350_IRQ_RTC_ALM);
> +
> +	return 0;
> +}
> +
> +static int __devexit wm8350_rtc_remove(struct platform_device *pdev)
> +{
> +	struct wm8350 *wm8350 = platform_get_drvdata(pdev);
> +	struct wm8350_rtc *wm_rtc = &wm8350->rtc;
> +
> +	wm8350_mask_irq(wm8350, WM8350_IRQ_RTC_SEC);
> +
> +	wm8350_free_irq(wm8350, WM8350_IRQ_RTC_SEC);
> +	wm8350_free_irq(wm8350, WM8350_IRQ_RTC_ALM);
> +
> +	rtc_device_unregister(wm_rtc->rtc);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver wm8350_rtc_driver = {
> +	.probe = wm8350_rtc_probe,
> +	.remove = wm8350_rtc_remove,

 mark with __devexit_p


> +	.suspend = wm8350_rtc_suspend,
> +	.resume = wm8350_rtc_resume,
> +	.driver = {
> +		.name = "wm8350-rtc",

 I'd like rtc-wm8350 here, if it doesn't hurt anything else.


> +	},
> +};
> +
> +static int __init wm8350_rtc_init(void)
> +{
> +	return platform_driver_register(&wm8350_rtc_driver);
> +}
> +module_init(wm8350_rtc_init);
> +
> +static void __exit wm8350_rtc_exit(void)
> +{
> +	platform_driver_unregister(&wm8350_rtc_driver);
> +}
> +module_exit(wm8350_rtc_exit);
> +
> +MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>");
> +MODULE_DESCRIPTION("RTC driver WM8350");

 driver _for_ 

> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:wm8350-rtc");
> diff --git a/include/linux/mfd/wm8350/rtc.h b/include/linux/mfd/wm8350/rtc.h
> index dfda69e..24add2b 100644
> --- a/include/linux/mfd/wm8350/rtc.h
> +++ b/include/linux/mfd/wm8350/rtc.h
> @@ -261,6 +261,8 @@
>  
>  struct wm8350_rtc {
>  	struct platform_device *pdev;
> +	struct rtc_device *rtc;
> +	int alarm_enabled;      /* used over suspend/resume */
>  };
>  
>  #endif
> -- 
> 1.5.6.5


-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it

Re: [rtc-linux] Re: [PATCH 2/2] rtc: rtc-wm8350: Add support for WM8350 RTC

[Mark Brown]

On Mon, Oct 27, 2008 at 06:04:32PM +0100, Alessandro Zummo wrote:

> > +	.resume = wm8350_rtc_resume,
> > +	.driver = {
> > +		.name = "wm8350-rtc",

>  I'd like rtc-wm8350 here, if it doesn't hurt anything else.

Due to needing to go through the MFD tree as well it'd make life easier
if we could leave as-is and avoid creating any merging issues (it's also
consistent with all the other WM8350 drivers).  Would that be OK?

Re: [rtc-linux] Re: [PATCH 2/2] rtc: rtc-wm8350: Add support for WM8350 RTC

[Alessandro Zummo]

On Mon, 27 Oct 2008 17:24:01 +0000
Mark Brown <broonie@sirena.org.uk> wrote:

> 
> On Mon, Oct 27, 2008 at 06:04:32PM +0100, Alessandro Zummo wrote:
> 
> > > +	.resume = wm8350_rtc_resume,
> > > +	.driver = {
> > > +		.name = "wm8350-rtc",
> 
> >  I'd like rtc-wm8350 here, if it doesn't hurt anything else.
> 
> Due to needing to go through the MFD tree as well it'd make life easier
> if we could leave as-is and avoid creating any merging issues (it's also
> consistent with all the other WM8350 drivers).  Would that be OK?

 Ok, leave it that way: that's classified as "hurting" :)


-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it

[PATCH 2/2] rtc: rtc-wm8350: Add support for WM8350 RTC

[Mark Brown]

This adds support for the RTC provided by the Wolfson Microelectronics
WM8350.

This driver was originally written by Graeme Gregory and Liam Girdwood,
though it has been modified since then to update it to current mainline
coding standards and for API completeness.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
This revision:
 - Marks remove() with __devexit_p()
 - Fixes grammar in MODULE_DESCRIPTION()

which should address all the feedback from Alessandro.

 drivers/rtc/Kconfig            |   10 +
 drivers/rtc/Makefile           |    1 +
 drivers/rtc/rtc-wm8350.c       |  514 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/wm8350/rtc.h |    2 +
 4 files changed, 527 insertions(+), 0 deletions(-)
 create mode 100644 drivers/rtc/rtc-wm8350.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 8abbb20..7951ad2 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -468,6 +468,16 @@ config RTC_DRV_V3020
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-v3020.
 
+config RTC_DRV_WM8350
+	tristate "Wolfson Microelectronics WM8350 RTC"
+	depends on MFD_WM8350
+	help
+	  If you say yes here you will get support for the RTC subsystem
+	  of the Wolfson Microelectronics WM8350.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called "rtc-wm8350".
+
 comment "on-CPU RTC drivers"
 
 config RTC_DRV_OMAP
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index e9e8474..7a41201 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -66,4 +66,5 @@ obj-$(CONFIG_RTC_DRV_TEST)	+= rtc-test.o
 obj-$(CONFIG_RTC_DRV_TWL4030)	+= rtc-twl4030.o
 obj-$(CONFIG_RTC_DRV_V3020)	+= rtc-v3020.o
 obj-$(CONFIG_RTC_DRV_VR41XX)	+= rtc-vr41xx.o
+obj-$(CONFIG_RTC_DRV_WM8350)	+= rtc-wm8350.o
 obj-$(CONFIG_RTC_DRV_X1205)	+= rtc-x1205.o
diff --git a/drivers/rtc/rtc-wm8350.c b/drivers/rtc/rtc-wm8350.c
new file mode 100644
index 0000000..5c5e3aa
--- /dev/null
+++ b/drivers/rtc/rtc-wm8350.c
@@ -0,0 +1,514 @@
+/*
+ *	Real Time Clock driver for Wolfson Microelectronics WM8350
+ *
+ *	Copyright (C) 2007, 2008 Wolfson Microelectronics PLC.
+ *
+ *  Author: Liam Girdwood
+ *          linux@wolfsonmicro.com
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/time.h>
+#include <linux/rtc.h>
+#include <linux/bcd.h>
+#include <linux/interrupt.h>
+#include <linux/ioctl.h>
+#include <linux/completion.h>
+#include <linux/mfd/wm8350/rtc.h>
+#include <linux/mfd/wm8350/core.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+
+#define WM8350_SET_ALM_RETRIES	5
+#define WM8350_SET_TIME_RETRIES	5
+#define WM8350_GET_TIME_RETRIES	5
+
+#define to_wm8350_from_rtc_dev(d) container_of(d, struct wm8350, rtc.pdev.dev)
+
+/*
+ * Read current time and date in RTC
+ */
+static int wm8350_rtc_readtime(struct device *dev, struct rtc_time *tm)
+{
+	struct wm8350 *wm8350 = dev_get_drvdata(dev);
+	u16 time1[4], time2[4];
+	int retries = WM8350_GET_TIME_RETRIES, ret;
+
+	/*
+	 * Read the time twice and compare.
+	 * If time1 == time2, then time is valid else retry.
+	 */
+	do {
+		ret = wm8350_block_read(wm8350, WM8350_RTC_SECONDS_MINUTES,
+					4, time1);
+		if (ret < 0)
+			return ret;
+		ret = wm8350_block_read(wm8350, WM8350_RTC_SECONDS_MINUTES,
+					4, time2);
+		if (ret < 0)
+			return ret;
+
+		if (memcmp(time1, time2, sizeof(time1)) == 0) {
+			tm->tm_sec = time1[0] & WM8350_RTC_SECS_MASK;
+
+			tm->tm_min = (time1[0] & WM8350_RTC_MINS_MASK)
+			    >> WM8350_RTC_MINS_SHIFT;
+
+			tm->tm_hour = time1[1] & WM8350_RTC_HRS_MASK;
+
+			tm->tm_wday = ((time1[1] >> WM8350_RTC_DAY_SHIFT)
+				       & 0x7) - 1;
+
+			tm->tm_mon = ((time1[2] & WM8350_RTC_MTH_MASK)
+				      >> WM8350_RTC_MTH_SHIFT) - 1;
+
+			tm->tm_mday = (time1[2] & WM8350_RTC_DATE_MASK);
+
+			tm->tm_year = ((time1[3] & WM8350_RTC_YHUNDREDS_MASK)
+				       >> WM8350_RTC_YHUNDREDS_SHIFT) * 100;
+			tm->tm_year += time1[3] & WM8350_RTC_YUNITS_MASK;
+
+			tm->tm_yday = rtc_year_days(tm->tm_mday, tm->tm_mon,
+						    tm->tm_year);
+			tm->tm_year -= 1900;
+
+			dev_dbg(dev, "Read (%d left): %04x %04x %04x %04x\n",
+				retries,
+				time1[0], time1[1], time1[2], time1[3]);
+
+			return 0;
+		}
+	} while (retries--);
+
+	dev_err(dev, "timed out reading RTC time\n");
+	return -EIO;
+}
+
+/*
+ * Set current time and date in RTC
+ */
+static int wm8350_rtc_settime(struct device *dev, struct rtc_time *tm)
+{
+	struct wm8350 *wm8350 = dev_get_drvdata(dev);
+	u16 time[4];
+	u16 rtc_ctrl;
+	int ret, retries = WM8350_SET_TIME_RETRIES;
+
+	time[0] = tm->tm_sec;
+	time[0] |= tm->tm_min << WM8350_RTC_MINS_SHIFT;
+	time[1] = tm->tm_hour;
+	time[1] |= (tm->tm_wday + 1) << WM8350_RTC_DAY_SHIFT;
+	time[2] = tm->tm_mday;
+	time[2] |= (tm->tm_mon + 1) << WM8350_RTC_MTH_SHIFT;
+	time[3] = ((tm->tm_year + 1900) / 100) << WM8350_RTC_YHUNDREDS_SHIFT;
+	time[3] |= (tm->tm_year + 1900) % 100;
+
+	dev_dbg(dev, "Setting: %04x %04x %04x %04x\n",
+		time[0], time[1], time[2], time[3]);
+
+	/* Set RTC_SET to stop the clock */
+	ret = wm8350_set_bits(wm8350, WM8350_RTC_TIME_CONTROL, WM8350_RTC_SET);
+	if (ret < 0)
+		return ret;
+
+	/* Wait until confirmation of stopping */
+	do {
+		rtc_ctrl = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
+		schedule_timeout_uninterruptible(msecs_to_jiffies(1));
+	} while (retries-- && !(rtc_ctrl & WM8350_RTC_STS));
+
+	if (!retries) {
+		dev_err(dev, "timed out on set confirmation\n");
+		return -EIO;
+	}
+
+	/* Write time to RTC */
+	ret = wm8350_block_write(wm8350, WM8350_RTC_SECONDS_MINUTES, 4, time);
+	if (ret < 0)
+		return ret;
+
+	/* Clear RTC_SET to start the clock */
+	ret = wm8350_clear_bits(wm8350, WM8350_RTC_TIME_CONTROL,
+				WM8350_RTC_SET);
+	return ret;
+}
+
+/*
+ * Read alarm time and date in RTC
+ */
+static int wm8350_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct wm8350 *wm8350 = dev_get_drvdata(dev);
+	struct rtc_time *tm = &alrm->time;
+	u16 time[4];
+	int ret;
+
+	ret = wm8350_block_read(wm8350, WM8350_ALARM_SECONDS_MINUTES, 4, time);
+	if (ret < 0)
+		return ret;
+
+	tm->tm_sec = time[0] & WM8350_RTC_ALMSECS_MASK;
+	if (tm->tm_sec == WM8350_RTC_ALMSECS_MASK)
+		tm->tm_sec = -1;
+
+	tm->tm_min = time[0] & WM8350_RTC_ALMMINS_MASK;
+	if (tm->tm_min == WM8350_RTC_ALMMINS_MASK)
+		tm->tm_min = -1;
+	else
+		tm->tm_min >>= WM8350_RTC_ALMMINS_SHIFT;
+
+	tm->tm_hour = time[1] & WM8350_RTC_ALMHRS_MASK;
+	if (tm->tm_hour == WM8350_RTC_ALMHRS_MASK)
+		tm->tm_hour = -1;
+
+	tm->tm_wday = ((time[1] >> WM8350_RTC_ALMDAY_SHIFT) & 0x7) - 1;
+	if (tm->tm_wday > 7)
+		tm->tm_wday = -1;
+
+	tm->tm_mon = time[2] & WM8350_RTC_ALMMTH_MASK;
+	if (tm->tm_mon == WM8350_RTC_ALMMTH_MASK)
+		tm->tm_mon = -1;
+	else
+		tm->tm_mon = (tm->tm_mon >> WM8350_RTC_ALMMTH_SHIFT) - 1;
+
+	tm->tm_mday = (time[2] & WM8350_RTC_ALMDATE_MASK);
+	if (tm->tm_mday == WM8350_RTC_ALMDATE_MASK)
+		tm->tm_mday = -1;
+
+	tm->tm_year = -1;
+
+	alrm->enabled = !(time[3] & WM8350_RTC_ALMSTS);
+
+	return 0;
+}
+
+static int wm8350_rtc_stop_alarm(struct wm8350 *wm8350)
+{
+	int retries = WM8350_SET_ALM_RETRIES;
+	u16 rtc_ctrl;
+	int ret;
+
+	/* Set RTC_SET to stop the clock */
+	ret = wm8350_set_bits(wm8350, WM8350_RTC_TIME_CONTROL,
+			      WM8350_RTC_ALMSET);
+	if (ret < 0)
+		return ret;
+
+	/* Wait until confirmation of stopping */
+	do {
+		rtc_ctrl = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
+		schedule_timeout_uninterruptible(msecs_to_jiffies(1));
+	} while (retries-- && !(rtc_ctrl & WM8350_RTC_ALMSTS));
+
+	if (!(rtc_ctrl & WM8350_RTC_ALMSTS))
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int wm8350_rtc_start_alarm(struct wm8350 *wm8350)
+{
+	int ret;
+	int retries = WM8350_SET_ALM_RETRIES;
+	u16 rtc_ctrl;
+
+	ret = wm8350_clear_bits(wm8350, WM8350_RTC_TIME_CONTROL,
+				WM8350_RTC_ALMSET);
+	if (ret < 0)
+		return ret;
+
+	/* Wait until confirmation */
+	do {
+		rtc_ctrl = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
+		schedule_timeout_uninterruptible(msecs_to_jiffies(1));
+	} while (retries-- && rtc_ctrl & WM8350_RTC_ALMSTS);
+
+	if (rtc_ctrl & WM8350_RTC_ALMSTS)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int wm8350_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct wm8350 *wm8350 = dev_get_drvdata(dev);
+	struct rtc_time *tm = &alrm->time;
+	u16 time[3];
+	int ret;
+
+	memset(time, 0, sizeof(time));
+
+	if (tm->tm_sec != -1)
+		time[0] |= tm->tm_sec;
+	else
+		time[0] |= WM8350_RTC_ALMSECS_MASK;
+
+	if (tm->tm_min != -1)
+		time[0] |= tm->tm_min << WM8350_RTC_ALMMINS_SHIFT;
+	else
+		time[0] |= WM8350_RTC_ALMMINS_MASK;
+
+	if (tm->tm_hour != -1)
+		time[1] |= tm->tm_hour;
+	else
+		time[1] |= WM8350_RTC_ALMHRS_MASK;
+
+	if (tm->tm_wday != -1)
+		time[1] |= (tm->tm_wday + 1) << WM8350_RTC_ALMDAY_SHIFT;
+	else
+		time[1] |= WM8350_RTC_ALMDAY_MASK;
+
+	if (tm->tm_mday != -1)
+		time[2] |= tm->tm_mday;
+	else
+		time[2] |= WM8350_RTC_ALMDATE_MASK;
+
+	if (tm->tm_mon != -1)
+		time[2] |= (tm->tm_mon + 1) << WM8350_RTC_ALMMTH_SHIFT;
+	else
+		time[2] |= WM8350_RTC_ALMMTH_MASK;
+
+	ret = wm8350_rtc_stop_alarm(wm8350);
+	if (ret < 0)
+		return ret;
+
+	/* Write time to RTC */
+	ret = wm8350_block_write(wm8350, WM8350_ALARM_SECONDS_MINUTES,
+				 3, time);
+	if (ret < 0)
+		return ret;
+
+	if (alrm->enabled)
+		ret = wm8350_rtc_start_alarm(wm8350);
+
+	return ret;
+}
+
+/*
+ * Handle commands from user-space
+ */
+static int wm8350_rtc_ioctl(struct device *dev, unsigned int cmd,
+			    unsigned long arg)
+{
+	struct wm8350 *wm8350 = dev_get_drvdata(dev);
+
+	switch (cmd) {
+	case RTC_AIE_OFF:
+		return wm8350_rtc_stop_alarm(wm8350);
+	case RTC_AIE_ON:
+		return wm8350_rtc_start_alarm(wm8350);
+
+	case RTC_UIE_OFF:
+		wm8350_mask_irq(wm8350, WM8350_IRQ_RTC_SEC);
+		break;
+	case RTC_UIE_ON:
+		wm8350_unmask_irq(wm8350, WM8350_IRQ_RTC_SEC);
+		break;
+
+	default:
+		return -ENOIOCTLCMD;
+	}
+
+	return 0;
+}
+
+static void wm8350_rtc_alarm_handler(struct wm8350 *wm8350, int irq,
+				     void *data)
+{
+	struct rtc_device *rtc = wm8350->rtc.rtc;
+	int ret;
+
+	rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
+
+	/* Make it one shot */
+	ret = wm8350_set_bits(wm8350, WM8350_RTC_TIME_CONTROL,
+			      WM8350_RTC_ALMSET);
+	if (ret != 0) {
+		dev_err(&(wm8350->rtc.pdev->dev),
+			"Failed to disable alarm: %d\n", ret);
+	}
+}
+
+static void wm8350_rtc_update_handler(struct wm8350 *wm8350, int irq,
+				      void *data)
+{
+	struct rtc_device *rtc = wm8350->rtc.rtc;
+
+	rtc_update_irq(rtc, 1, RTC_IRQF | RTC_UF);
+}
+
+static const struct rtc_class_ops wm8350_rtc_ops = {
+	.ioctl = wm8350_rtc_ioctl,
+	.read_time = wm8350_rtc_readtime,
+	.set_time = wm8350_rtc_settime,
+	.read_alarm = wm8350_rtc_readalarm,
+	.set_alarm = wm8350_rtc_setalarm,
+};
+
+#ifdef CONFIG_PM
+static int wm8350_rtc_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	struct wm8350 *wm8350 = dev_get_drvdata(&pdev->dev);
+	int ret = 0;
+	u16 reg;
+
+	reg = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
+
+	if (device_may_wakeup(&wm8350->rtc.pdev->dev) &&
+	    reg & WM8350_RTC_ALMSTS) {
+		ret = wm8350_rtc_stop_alarm(wm8350);
+		if (ret != 0)
+			dev_err(&pdev->dev, "Failed to stop RTC alarm: %d\n",
+				ret);
+	}
+
+	return ret;
+}
+
+static int wm8350_rtc_resume(struct platform_device *pdev)
+{
+	struct wm8350 *wm8350 = dev_get_drvdata(&pdev->dev);
+	int ret;
+
+	if (wm8350->rtc.alarm_enabled) {
+		ret = wm8350_rtc_start_alarm(wm8350);
+		if (ret != 0)
+			dev_err(&pdev->dev,
+				"Failed to restart RTC alarm: %d\n", ret);
+	}
+
+	return 0;
+}
+
+#else
+#define wm8350_rtc_suspend NULL
+#define wm8350_rtc_resume NULL
+#endif
+
+static int wm8350_rtc_probe(struct platform_device *pdev)
+{
+	struct wm8350 *wm8350 = platform_get_drvdata(pdev);
+	struct wm8350_rtc *wm_rtc = &wm8350->rtc;
+	int ret = 0;
+	u16 timectl, power5;
+
+	timectl = wm8350_reg_read(wm8350, WM8350_RTC_TIME_CONTROL);
+	if (timectl & WM8350_RTC_BCD) {
+		dev_err(&pdev->dev, "RTC BCD mode not supported\n");
+		return -EINVAL;
+	}
+	if (timectl & WM8350_RTC_12HR) {
+		dev_err(&pdev->dev, "RTC 12 hour mode not supported\n");
+		return -EINVAL;
+	}
+
+	/* enable the RTC if it's not already enabled */
+	power5 = wm8350_reg_read(wm8350, WM8350_POWER_MGMT_5);
+	if (!(power5 &  WM8350_RTC_TICK_ENA)) {
+		dev_info(wm8350->dev, "Starting RTC\n");
+
+		wm8350_reg_unlock(wm8350);
+
+		ret = wm8350_set_bits(wm8350, WM8350_POWER_MGMT_5,
+				      WM8350_RTC_TICK_ENA);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "failed to enable RTC: %d\n", ret);
+			return ret;
+		}
+
+		wm8350_reg_lock(wm8350);
+	}
+
+	if (timectl & WM8350_RTC_STS) {
+		int retries;
+
+		ret = wm8350_clear_bits(wm8350, WM8350_RTC_TIME_CONTROL,
+					WM8350_RTC_SET);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "failed to start: %d\n", ret);
+			return ret;
+		}
+
+		retries = WM8350_SET_TIME_RETRIES;
+		do {
+			timectl = wm8350_reg_read(wm8350,
+						  WM8350_RTC_TIME_CONTROL);
+		} while (timectl & WM8350_RTC_STS && retries--);
+
+		if (retries == 0) {
+			dev_err(&pdev->dev, "failed to start: timeout\n");
+			return -ENODEV;
+		}
+	}
+
+	device_init_wakeup(&pdev->dev, 1);
+
+	wm_rtc->rtc = rtc_device_register("wm8350", &pdev->dev,
+					  &wm8350_rtc_ops, THIS_MODULE);
+	if (IS_ERR(wm_rtc->rtc)) {
+		ret = PTR_ERR(wm_rtc->rtc);
+		dev_err(&pdev->dev, "failed to register RTC: %d\n", ret);
+		return ret;
+	}
+
+	wm8350_mask_irq(wm8350, WM8350_IRQ_RTC_SEC);
+	wm8350_mask_irq(wm8350, WM8350_IRQ_RTC_PER);
+
+	wm8350_register_irq(wm8350, WM8350_IRQ_RTC_SEC,
+			    wm8350_rtc_update_handler, NULL);
+
+	wm8350_register_irq(wm8350, WM8350_IRQ_RTC_ALM,
+			    wm8350_rtc_alarm_handler, NULL);
+	wm8350_unmask_irq(wm8350, WM8350_IRQ_RTC_ALM);
+
+	return 0;
+}
+
+static int __devexit wm8350_rtc_remove(struct platform_device *pdev)
+{
+	struct wm8350 *wm8350 = platform_get_drvdata(pdev);
+	struct wm8350_rtc *wm_rtc = &wm8350->rtc;
+
+	wm8350_mask_irq(wm8350, WM8350_IRQ_RTC_SEC);
+
+	wm8350_free_irq(wm8350, WM8350_IRQ_RTC_SEC);
+	wm8350_free_irq(wm8350, WM8350_IRQ_RTC_ALM);
+
+	rtc_device_unregister(wm_rtc->rtc);
+
+	return 0;
+}
+
+static struct platform_driver wm8350_rtc_driver = {
+	.probe = wm8350_rtc_probe,
+	.remove = __devexit_p(wm8350_rtc_remove),
+	.suspend = wm8350_rtc_suspend,
+	.resume = wm8350_rtc_resume,
+	.driver = {
+		.name = "wm8350-rtc",
+	},
+};
+
+static int __init wm8350_rtc_init(void)
+{
+	return platform_driver_register(&wm8350_rtc_driver);
+}
+module_init(wm8350_rtc_init);
+
+static void __exit wm8350_rtc_exit(void)
+{
+	platform_driver_unregister(&wm8350_rtc_driver);
+}
+module_exit(wm8350_rtc_exit);
+
+MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>");
+MODULE_DESCRIPTION("RTC driver for the WM8350");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:wm8350-rtc");
diff --git a/include/linux/mfd/wm8350/rtc.h b/include/linux/mfd/wm8350/rtc.h
index dfda69e..24add2b 100644
--- a/include/linux/mfd/wm8350/rtc.h
+++ b/include/linux/mfd/wm8350/rtc.h
@@ -261,6 +261,8 @@
 
 struct wm8350_rtc {
 	struct platform_device *pdev;
+	struct rtc_device *rtc;
+	int alarm_enabled;      /* used over suspend/resume */
 };
 
 #endif
-- 
1.5.6.5

Re: [PATCH 2/2] rtc: rtc-wm8350: Add support for WM8350 RTC

[Andrew Morton]

On Mon, 27 Oct 2008 17:40:01 +0000 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> This adds support for the RTC provided by the Wolfson Microelectronics
> WM8350.
> 
> This driver was originally written by Graeme Gregory and Liam Girdwood,
> though it has been modified since then to update it to current mainline
> coding standards and for API completeness.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
> This revision:
>  - Marks remove() with __devexit_p()
>  - Fixes grammar in MODULE_DESCRIPTION()
> 
> which should address all the feedback from Alessandro.

This version did this:

From: Mark Brown <broonie@opensource.wolfsonmicro.com>

 - Marks remove() with __devexit_p()
 - Fixes grammar in MODULE_DESCRIPTION()

which should address all the feedback from Alessandro.

Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: David Brownell <david-b@pacbell.net>
Cc: Liam Girdwood <linux@wolfsonmicro.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/rtc/rtc-wm8350.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -puN drivers/rtc/rtc-wm8350.c~rtc-rtc-wm8350-add-support-for-wm8350-rtc-update-2 drivers/rtc/rtc-wm8350.c
--- a/drivers/rtc/rtc-wm8350.c~rtc-rtc-wm8350-add-support-for-wm8350-rtc-update-2
+++ a/drivers/rtc/rtc-wm8350.c
@@ -488,7 +488,7 @@ static int __devexit wm8350_rtc_remove(s
 
 static struct platform_driver wm8350_rtc_driver = {
 	.probe = wm8350_rtc_probe,
-	.remove = wm8350_rtc_remove,
+	.remove = __devexit_p(wm8350_rtc_remove),
 	.suspend = wm8350_rtc_suspend,
 	.resume = wm8350_rtc_resume,
 	.driver = {
@@ -509,6 +509,6 @@ static void __exit wm8350_rtc_exit(void)
 module_exit(wm8350_rtc_exit);
 
 MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>");
-MODULE_DESCRIPTION("RTC driver WM8350");
+MODULE_DESCRIPTION("RTC driver for the WM8350");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:wm8350-rtc");
_

Re: [rtc-linux] [PATCH 2/2] rtc: rtc-wm8350: Add support for WM8350 RTC

[David Brownell]

On Monday 27 October 2008, Alessandro Zummo wrote:
>  (and a detailed checklist at http://groups.google.com/group/rtc-linux/web/checklist )

The comment about probe()/__devinit and remove()/__devexit/__devexit_p
isn't right ... when the driver uses platform_driver_probe(), the "__dev"
variants should not be used.  (It's a Good Thing to shrink runtime code
footprints, by using platform_driver_probe for non-hotpluggable devices.)

Also:  rtc_ops.ioctl() method should only handle RTC_{AIE,UIE}_{ON,OFF},
since the other standard ioctls have rtc_ops equivalents.


(This code is fine in both respects.  Though since I've received three
copies of the driver since it was merged to MM, maybe I missed something.)

- Dave

Re: [rtc-linux] [PATCH 2/2] rtc: rtc-wm8350: Add support for WM8350 RTC

[Mark Brown]

On Mon, Oct 27, 2008 at 01:03:21PM -0700, David Brownell wrote:
> On Monday 27 October 2008, Alessandro Zummo wrote:
> >  (and a detailed checklist at http://groups.google.com/group/rtc-linux/web/checklist )

> Also:  rtc_ops.ioctl() method should only handle RTC_{AIE,UIE}_{ON,OFF},
> since the other standard ioctls have rtc_ops equivalents.

> (This code is fine in both respects.  Though since I've received three
> copies of the driver since it was merged to MM, maybe I missed something.)

Unfortunately I added the __devexit_p() annotation in response to
Alessandro's comments.  I'll remove it again, other than that all that's
changed is a tweak to the module description and Andrew's change to use
schedule_timeout_uninterruptible().

How should these drivers be merged?

Re: [rtc-linux] [PATCH 2/2] rtc: rtc-wm8350: Add support for WM8350 RTC

[David Brownell]

On Monday 27 October 2008, Mark Brown wrote:
> On Mon, Oct 27, 2008 at 01:03:21PM -0700, David Brownell wrote:
> > On Monday 27 October 2008, Alessandro Zummo wrote:
> > >  (and a detailed checklist at http://groups.google.com/group/rtc-linux/web/checklist )
> 
> > Also:  rtc_ops.ioctl() method should only handle RTC_{AIE,UIE}_{ON,OFF},
> > since the other standard ioctls have rtc_ops equivalents.
> 
> > (This code is fine in both respects.  Though since I've received three
> > copies of the driver since it was merged to MM, maybe I missed something.)
> 
> Unfortunately I added the __devexit_p() annotation in response to
> Alessandro's comments.  I'll remove it again,

No, don't -- that's why I said "this code is fine..." !!

You're not using platform_driver_probe(), so it's appropriate to
use the __devexit() route.


> other than that all that's 
> changed is a tweak to the module description and Andrew's change to use
> schedule_timeout_uninterruptible().
> 
> How should these drivers be merged?

Andrew is the keeper of the RTC patch queue, so I suggest you issue
one patch against the two in MM:

	rtc-rtc-wm8350-add-support-for-wm8350-rtc.patch
	rtc-rtc-wm8350-add-support-for-wm8350-rtc-fix.patch

That would probably cause the least amount of extra work for Andrew.

- Dave

 

Re: [PATCH 2/2] rtc: rtc-wm8350: Add support for WM8350 RTC

[Andrew Morton]

On Mon, 27 Oct 2008 15:38:25 +0000 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> This adds support for the RTC provided by the Wolfson Microelectronics
> WM8350.
> 
> This driver was originally written by Graeme Gregory and Liam Girdwood,
> though it has been modified since then to update it to current mainline
> coding standards and for API completeness.

Graeme doesn't seem to get a mention anywhere in the driver?

> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
> Updated to use schedule_timeout_uninterruptible() as per Andrew's
> suggestion.

You also did this:

--- a/drivers/rtc/rtc-wm8350.c~rtc-rtc-wm8350-add-support-for-wm8350-rtc-update
+++ a/drivers/rtc/rtc-wm8350.c
@@ -471,7 +471,7 @@ static int wm8350_rtc_probe(struct platf
 	return 0;
 }
 
-static int __exit wm8350_rtc_remove(struct platform_device *pdev)
+static int __devexit wm8350_rtc_remove(struct platform_device *pdev)
 {
 	struct wm8350 *wm8350 = platform_get_drvdata(pdev);
 	struct wm8350_rtc *wm_rtc = &wm8350->rtc;
@@ -500,16 +500,15 @@ static int __init wm8350_rtc_init(void)
 {
 	return platform_driver_register(&wm8350_rtc_driver);
 }
+module_init(wm8350_rtc_init);
 
 static void __exit wm8350_rtc_exit(void)
 {
 	platform_driver_unregister(&wm8350_rtc_driver);
 }
-
-module_init(wm8350_rtc_init);
 module_exit(wm8350_rtc_exit);
 
-MODULE_AUTHOR("Liam Girdwood");
+MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>");
 MODULE_DESCRIPTION("RTC driver WM8350");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:wm8350-rtc");
_

Re: [PATCH 2/2] rtc: rtc-wm8350: Add support for WM8350 RTC

[Mark Brown]

On Mon, Oct 27, 2008 at 11:22:12PM -0700, Andrew Morton wrote:
> On Mon, 27 Oct 2008 15:38:25 +0000 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> > This driver was originally written by Graeme Gregory and Liam Girdwood,
> > though it has been modified since then to update it to current mainline
> > coding standards and for API completeness.

> Graeme doesn't seem to get a mention anywhere in the driver?

Hrm, yeah.  That should probably read Liam and Graeme rather than the
way it is - AFAICT Graeme's work was bugfixing and updates.

> > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > ---
> > Updated to use schedule_timeout_uninterruptible() as per Andrew's
> > suggestion.

> You also did this:

Oops, sorry.  I had thought I'd posted a version with those changes in
already.

Re: [PATCH 1/2] rtc: Fix handling of missing tm_year data when reading alarms

[Alessandro Zummo]

On Mon, 27 Oct 2008 15:38:24 +0000
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> When fixing up invalid years rtc_read_alarm() is callign rtc_valid_tm()
> as a boolean but rtc_valid_tm() returns zero on success or a negative
> number if the time is not valid so the test is inverted.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

 Acked-by: Alessandro Zummo <a.zummo@towertech.it>

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it