Hi
Marek Vasut wrote:Dne So 19. Äervna 2010 07:08:20 Lars-Peter Clausen napsal(a):Well IF it will ever die, you'll notice cause your rtc clock won't work
This patch adds support for the RTC unit on JZ4740 SoCs.if (!timeout) {
Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
Cc: Alessandro Zummo <a.zummo@xxxxxxxxxxxx>
Cc: Paul Gortmaker <p_gortmaker@xxxxxxxxx>
Cc: Wan ZongShun <mcuos.com@xxxxxxxxx>
Cc: Marek Vasut <marek.vasut@xxxxxxxxx>
Cc: rtc-linux@xxxxxxxxxxxxxxxx
---
Changes since v1
- Use dev_get_drvdata directly instead of wrapping it in dev_to_rtc
- Add common implementation for jz4740_rtc_{alarm,update}_irq_enable
- Check whether rtc structure could be allocated
- Fix deadlocks which could occur if the HW was broken
---
drivers/rtc/Kconfig | 11 ++
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-jz4740.c | 341
++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 353
insertions(+), 0 deletions(-)
create mode 100644 drivers/rtc/rtc-jz4740.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 10ba12c..d0ed7e6 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -905,4 +905,15 @@ config RTC_DRV_MPC5121
This driver can also be built as a module. If so, the module
will be called rtc-mpc5121.
+config RTC_DRV_JZ4740
+ tristate "Ingenic JZ4740 SoC"
+ depends on RTC_CLASS
+ depends on MACH_JZ4740
+ help
+ If you say yes here you get support for the Ingenic JZ4740 SoC RTC
+ controller.
+
+ This driver can also be buillt as a module. If so, the module
+ will be called rtc-jz4740.
+
endif # RTC_CLASS
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 5adbba7..fedf9bb 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_RTC_DRV_EP93XX) += rtc-ep93xx.o
obj-$(CONFIG_RTC_DRV_FM3130) += rtc-fm3130.o
obj-$(CONFIG_RTC_DRV_GENERIC) += rtc-generic.o
obj-$(CONFIG_RTC_DRV_ISL1208) += rtc-isl1208.o
+obj-$(CONFIG_RTC_DRV_JZ4740) += rtc-jz4740.o
obj-$(CONFIG_RTC_DRV_M41T80) += rtc-m41t80.o
obj-$(CONFIG_RTC_DRV_M41T94) += rtc-m41t94.o
obj-$(CONFIG_RTC_DRV_M48T35) += rtc-m48t35.o
diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
new file mode 100644
index 0000000..720afb2
--- /dev/null
+++ b/drivers/rtc/rtc-jz4740.c
@@ -0,0 +1,341 @@
+/*
+ * Copyright (C) 2009-2010, Lars-Peter Clausen <lars@xxxxxxxxxx>
+ * JZ4740 SoC RTC driver
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
along + * with this program; if not, write to the Free Software
Foundation, Inc., + * 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#define JZ_REG_RTC_CTRL 0x00
+#define JZ_REG_RTC_SEC 0x04
+#define JZ_REG_RTC_SEC_ALARM 0x08
+#define JZ_REG_RTC_REGULATOR 0x0C
+#define JZ_REG_RTC_HIBERNATE 0x20
+#define JZ_REG_RTC_SCRATCHPAD 0x34
+
+#define JZ_RTC_CTRL_WRDY BIT(7)
+#define JZ_RTC_CTRL_1HZ BIT(6)
+#define JZ_RTC_CTRL_1HZ_IRQ BIT(5)
+#define JZ_RTC_CTRL_AF BIT(4)
+#define JZ_RTC_CTRL_AF_IRQ BIT(3)
+#define JZ_RTC_CTRL_AE BIT(2)
+#define JZ_RTC_CTRL_ENABLE BIT(0)
+
+struct jz4740_rtc {
+ struct resource *mem;
+ void __iomem *base;
+
+ struct rtc_device *rtc;
+
+ unsigned int irq;
+
+ spinlock_t lock;
+};
+
+static inline uint32_t jz4740_rtc_reg_read(struct jz4740_rtc *rtc, size_t
reg) +{
+ return readl(rtc->base + reg);
+}
+
+static inline void jz4740_rtc_wait_write_ready(struct jz4740_rtc *rtc)
+{
+ uint32_t ctrl;
+ int timeout = 1000;
+
+ do {
+ ctrl = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_CTRL);
+ } while (!(ctrl & JZ_RTC_CTRL_WRDY) && --timeout);
scream_and_die_in_pain();
dev_err("I died");
... or something like that ... what if it times out, in this implementation, noone will know this failed.
I haven't looked through the whole source code, but can't this be wrapped into the reg_write() ?
}
anymore.
It could be wrapped into reg_write, but there is a different version of
the SoC with the only difference of the RTC unit being that a different
mechanism is used determine whether it is ok to write or not. So it
makes sense to keep it seperate.
Why would that be preferable? In the non-debug, non-rt case this will+}Can't we use local_irq_save()/local_irq_restore() ?
+
+static inline void jz4740_rtc_reg_write(struct jz4740_rtc *rtc, size_t
reg, + uint32_t val)
+{
+ jz4740_rtc_wait_write_ready(rtc);
+ writel(val, rtc->base + reg);
+}
+
+static void jz4740_rtc_ctrl_set_bits(struct jz4740_rtc *rtc, uint32_t
mask, + uint32_t val)
+{
+ unsigned long flags;
+ uint32_t ctrl;
+
+ spin_lock_irqsave(&rtc->lock, flags);
expand to local_irq_{save,restore} anyway, but you'll lose the semantics
of an lock.
Strictly speaking not.+Is the typecast here necessary ?
+ ctrl = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_CTRL);
+
+ /* Don't clear interrupt flags by accident */
+ ctrl |= JZ_RTC_CTRL_1HZ | JZ_RTC_CTRL_AF;
+
+ ctrl &= ~mask;
+ ctrl |= val;
+
+ jz4740_rtc_reg_write(rtc, JZ_REG_RTC_CTRL, ctrl);
+
+ spin_unlock_irqrestore(&rtc->lock, flags);
+}
+
+static int jz4740_rtc_read_time(struct device *dev, struct rtc_time *time)
+{
+ struct jz4740_rtc *rtc = dev_get_drvdata(dev);
+ uint32_t secs, secs2;
+ int timeout = 5;
+
+ /* If the seconds register is read while it is updated, it can contain a
+ * bogus value. This can be avoided by making sure that two consecutive
+ * reads have the same value.
+ */
+ secs = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SEC);
+ secs2 = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SEC);
+
+ while (secs != secs2 && --timeout) {
+ secs = secs2;
+ secs2 = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SEC);
+ }
+
+ if (timeout == 0)
+ return -EIO;
+
+ rtc_time_to_tm(secs, time);
+
+ return rtc_valid_tm(time);
+}
+
+static int jz4740_rtc_set_mmss(struct device *dev, unsigned long secs)
+{
+ struct jz4740_rtc *rtc = dev_get_drvdata(dev);
+
+ if ((uint32_t)secs != secs)
+ return -EINVAL;
To quote rtc.h "/* 0 = alarm disabled, 1 = alarm enabled */", so yes.+Is the double negation (!!) here necessary ?
+ jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SEC, secs);
+
+ return 0;
+}
+
+static int jz4740_rtc_read_alarm(struct device *dev, struct rtc_wkalrm
*alrm) +{
+ struct jz4740_rtc *rtc = dev_get_drvdata(dev);
+ uint32_t secs;
+ uint32_t ctrl;
+
+ secs = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SEC_ALARM);
+
+ ctrl = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_CTRL);
+
+ alrm->enabled = !!(ctrl & JZ_RTC_CTRL_AE);
+ alrm->pending = !!(ctrl & JZ_RTC_CTRL_AF);
+
+ rtc_time_to_tm(secs, &alrm->time);DTTO above
+
+ return rtc_valid_tm(&alrm->time);
+}
+
+static int jz4740_rtc_set_alarm(struct device *dev, struct rtc_wkalrm
*alrm) +{
+ struct jz4740_rtc *rtc = dev_get_drvdata(dev);
+ unsigned long secs;
+
+ rtc_tm_to_time(&alrm->time, &secs);
+
+ if ((uint32_t)secs != secs)
+ return -EINVAL;
+DTTO
+ jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SEC_ALARM, (uint32_t)secs);
+ jz4740_rtc_ctrl_set_bits(rtc, JZ_RTC_CTRL_AE,Possibly the double negation above wasn't necessary
+ alrm->enabled ? JZ_RTC_CTRL_AE : 0);
+
+ return 0;
+}
+
+static inline int jz4740_irq_enable(struct device *dev, int irq,
+ unsigned int enable)
+{
+ struct jz4740_rtc *rtc = dev_get_drvdata(dev);
+ jz4740_rtc_ctrl_set_bits(rtc, irq, enable ? irq : 0);
+
+ return 0;
+}
+
+static int jz4740_rtc_update_irq_enable(struct device *dev, unsigned int
enable) +{
+ return jz4740_irq_enable(dev, JZ_RTC_CTRL_1HZ_IRQ, enable);
+}
+
+static int jz4740_rtc_alarm_irq_enable(struct device *dev, unsigned int
enable) +{
+ return jz4740_irq_enable(dev, JZ_RTC_CTRL_AF_IRQ, enable);
+}
+
+static struct rtc_class_ops jz4740_rtc_ops = {
+ .read_time = jz4740_rtc_read_time,
+ .set_mmss = jz4740_rtc_set_mmss,
+ .read_alarm = jz4740_rtc_read_alarm,
+ .set_alarm = jz4740_rtc_set_alarm,
+ .update_irq_enable = jz4740_rtc_update_irq_enable,
+ .alarm_irq_enable = jz4740_rtc_alarm_irq_enable,
+};
+
+static irqreturn_t jz4740_rtc_irq(int irq, void *data)
+{
+ struct jz4740_rtc *rtc = data;
+ uint32_t ctrl;
+ unsigned long events = 0;
+ ctrl = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_CTRL);
+
+ if (ctrl & JZ_RTC_CTRL_1HZ)
+ events |= (RTC_UF | RTC_IRQF);
+
+ if (ctrl & JZ_RTC_CTRL_AF)
+ events |= (RTC_AF | RTC_IRQF);
+
+ rtc_update_irq(rtc->rtc, 1, events);
+
+ jz4740_rtc_ctrl_set_bits(rtc, JZ_RTC_CTRL_1HZ | JZ_RTC_CTRL_AF, 0);
+
+ return IRQ_HANDLED;
+}
+
+void jz4740_rtc_poweroff(struct device *dev)
+{
+ struct jz4740_rtc *rtc = dev_get_drvdata(dev);
+ jz4740_rtc_reg_write(rtc, JZ_REG_RTC_HIBERNATE, 1);
+}
+EXPORT_SYMBOL_GPL(jz4740_rtc_poweroff);
+
+static int __devinit jz4740_rtc_probe(struct platform_device *pdev)
+{
+ int ret;
+ struct jz4740_rtc *rtc;
+ uint32_t scratchpad;
+
+ rtc = kmalloc(sizeof(*rtc), GFP_KERNEL);
In fact, I prefer you can use this IRQ flags, such as IRQF_DISABLED, IRQF_SHARED.No.+ if (!rtc)dev_set_drvdata()?
+ return -ENOMEM;
+
+ rtc->irq = platform_get_irq(pdev, 0);
+ if (rtc->irq < 0) {
+ ret = -ENOENT;
+ dev_err(&pdev->dev, "Failed to get platform irq\n");
+ goto err_free;
+ }
+
+ rtc->mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!rtc->mem) {
+ ret = -ENOENT;
+ dev_err(&pdev->dev, "Failed to get platform mmio memory\n");
+ goto err_free;
+ }
+
+ rtc->mem = request_mem_region(rtc->mem->start, resource_size(rtc->mem),
+ pdev->name);
+ if (!rtc->mem) {
+ ret = -EBUSY;
+ dev_err(&pdev->dev, "Failed to request mmio memory region\n");
+ goto err_free;
+ }
+
+ rtc->base = ioremap_nocache(rtc->mem->start, resource_size(rtc->mem));
+ if (!rtc->base) {
+ ret = -EBUSY;
+ dev_err(&pdev->dev, "Failed to ioremap mmio memory\n");
+ goto err_release_mem_region;
+ }
+
+ spin_lock_init(&rtc->lock);
+
+ platform_set_drvdata(pdev, rtc);
+
+ rtc->rtc = rtc_device_register(pdev->name, &pdev->dev, &jz4740_rtc_ops,
+ THIS_MODULE);
+ if (IS_ERR(rtc->rtc)) {
+ ret = PTR_ERR(rtc->rtc);
+ dev_err(&pdev->dev, "Failed to register rtc device: %d\n", ret);
+ goto err_iounmap;
+ }
+
+ ret = request_irq(rtc->irq, jz4740_rtc_irq, 0,
+ pdev->name, rtc);
Thanks for reviewing+ if (ret) {dev_get_drvdata();
+ dev_err(&pdev->dev, "Failed to request rtc irq: %d\n", ret);
+ goto err_unregister_rtc;
+ }
+
+ scratchpad = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD);
+ if (scratchpad != 0x12345678) {
+ jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SCRATCHPAD, 0x12345678);
+ jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SEC, 0);
+ }
+
+ return 0;
+
+err_unregister_rtc:
+ rtc_device_unregister(rtc->rtc);
+err_iounmap:
+ platform_set_drvdata(pdev, NULL);
+ iounmap(rtc->base);
+err_release_mem_region:
+ release_mem_region(rtc->mem->start, resource_size(rtc->mem));
+err_free:
+ kfree(rtc);
+
+ return ret;
+}
+
+static int __devexit jz4740_rtc_remove(struct platform_device *pdev)
+{
+ struct jz4740_rtc *rtc = platform_get_drvdata(pdev);
+DTTO
+ free_irq(rtc->irq, rtc);
+
+ rtc_device_unregister(rtc->rtc);
+
+ iounmap(rtc->base);
+ release_mem_region(rtc->mem->start, resource_size(rtc->mem));
+
+ kfree(rtc);
+
+ platform_set_drvdata(pdev, NULL);
+Cheers
+ return 0;
+}
+
+struct platform_driver jz4740_rtc_driver = {
+ .probe = jz4740_rtc_probe,
+ .remove = __devexit_p(jz4740_rtc_remove),
+ .driver = {
+ .name = "jz4740-rtc",
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init jz4740_rtc_init(void)
+{
+ return platform_driver_register(&jz4740_rtc_driver);
+}
+module_init(jz4740_rtc_init);
+
+static void __exit jz4740_rtc_exit(void)
+{
+ platform_driver_unregister(&jz4740_rtc_driver);
+}
+module_exit(jz4740_rtc_exit);
+
+MODULE_AUTHOR("Lars-Peter Clausen <lars@xxxxxxxxxx>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("RTC driver for the JZ4740 SoC\n");
+MODULE_ALIAS("platform:jz4740-rtc");
- Lars