RE: [rtc-linux] [PATCH 1/2] rtc: rtc-lpc32xx: Introduce RTC driver for the LPC32XX SoC

Previous thread: Contact: Mr. Well Paulson/Call Tel: +44-7035-924-138. by Uk National Lottey on Monday, August 9, 2010 - 4:14 pm. (1 message)

Next thread: Re by Cindy Howard. on Monday, August 9, 2010 - 10:12 am. (1 message)
From: wellsk40
Date: Monday, August 9, 2010 - 9:17 am

This patch set adds support for the LPC32xx built-in RTC. This is
a persistent RTC with alarm on it's own power domain and a
dedicated oscillator. UIE emulation is used for *_UIE_* ioctls.

--

From: wellsk40
Date: Monday, August 9, 2010 - 9:17 am

From: Kevin Wells <wellsk40@gmail.com>

This patch contains the RTC driver for the built-in RTC in
the LPC32XX SoC.

Signed-off-by: Kevin Wells <wellsk40@gmail.com>
Signed-off-by: Durgesh Pattamatta <durgesh.pattamatta@nxp.com>
---
 drivers/rtc/rtc-lpc32xx.c |  391 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 391 insertions(+), 0 deletions(-)
 create mode 100644 drivers/rtc/rtc-lpc32xx.c

diff --git a/drivers/rtc/rtc-lpc32xx.c b/drivers/rtc/rtc-lpc32xx.c
new file mode 100644
index 0000000..7803c68
--- /dev/null
+++ b/drivers/rtc/rtc-lpc32xx.c
@@ -0,0 +1,391 @@
+/*
+ * Copyright (C) 2010 NXP Semiconductors
+ *
+ * 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/init.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/rtc.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+
+/*
+ * Clock and Power control register offsets
+ */
+#define RTC_UCOUNT		0x00
+#define RTC_DCOUNT		0x04
+#define RTC_MATCH0		0x08
+#define RTC_MATCH1		0x0C
+#define RTC_CTRL		0x10
+#define RTC_INTSTAT		0x14
+#define RTC_KEY			0x18
+#define RTC_SRAM		0x80
+
+#define RTC_MATCH0_EN		(1 << 0)
+#define RTC_MATCH1_EN		(1 << 1)
+#define RTC_ONSW_MATCH0_EN	(1 << 2)
+#define RTC_ONSW_MATCH1_EN	(1 << 3)
+#define RTC_SW_RESET		(1 << 4)
+#define RTC_CNTR_DIS		(1 << 6)
+#define RTC_ONSW_FORCE_HIGH	(1 << 7)
+
+#define RTC_MATCH0_INT_STS	(1 << 0)
+#define RTC_MATCH1_INT_STS	(1 << 1)
+#define RTC_ONSW_INT_STS	(1 << 2)
+
+#define ...
From: Wan ZongShun
Date: Monday, August 9, 2010 - 6:55 pm

Hi Kevin ,

This is really a natty rtc patch:).



I think this 'wkalrm->enabled ' will always be set '1', so 'if'




-- 
*linux-arm-kernel mailing list
mail addr:linux-arm-kernel@lists.infradead.org
you can subscribe by:
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

* linux-arm-NUC900 mailing list
mail addr:NUC900@googlegroups.com
main web: https://groups.google.com/group/NUC900
you can subscribe it by sending me mail:
mcuos.com@gmail.com
--

From: Kevin Wells
Date: Tuesday, August 10, 2010 - 12:08 pm

Hi Zongshun,


From the urban dictionary...
"natty"
1) originating from rastafarian culture. meaning 'good','cool' and
most importantly, 'elite'.
2) A slang for Natural Light beer, a favorite of poor college students
the world over.

I hope it's the first one, but I guess the 2nd isn't that bad if
you’re a college student o)
I'll merge both patches into a single patch for v2 and repost in a

Thanks for catching this. The '= 1' assignment shouldn't be there.
From: Wan ZongShun
Date: Tuesday, August 10, 2010 - 6:40 pm

-- 
*linux-arm-kernel mailing list
mail addr:linux-arm-kernel@lists.infradead.org
you can subscribe by:
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

* linux-arm-NUC900 mailing list
mail addr:NUC900@googlegroups.com
main web: https://groups.google.com/group/NUC900
you can subscribe it by sending me mail:
mcuos.com@gmail.com
--

From: Wolfram Sang
Date: Tuesday, August 10, 2010 - 3:25 am

Hi Kevin,

driver seems to work fine here, a few comments though.




How about using managed resources (devm_*)? They usually simplify the

I saw that a number of rtc-drivers register their irq after they
register the device. I wonder if this is OK here? Couldn't it happen
that after rtc_device_register() there is a preemption and another
process could set the alarm? Then there is a race between interrupts

Minor nitpick: The trend seems to be to put those lines directly after

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
From: Uwe Kleine-König
Date: Tuesday, August 10, 2010 - 5:00 am

If you do it the other way around the irq might trigger and the handler
reports an irq for a device that doesn't exist yet.
 
Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--

From: Wolfram Sang
Date: Tuesday, August 10, 2010 - 6:40 am

Well, I was assuming that you initially have all interrupts disabled...

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
From: Kevin Wells
Date: Tuesday, August 10, 2010 - 12:08 pm

Hi Wolfram,

Thanks for helping review this.

The 32xx RTC registers can save states across power or reset
cycles and are only initialized if they have never been previously
initialized. When waking up from suspend, power on, or reset - the
alarm IRQ may have fired and may be enabled from a previous
initialization, so the RTC device needs to be registered first.

You bring up a good point though, I wonder if going through probe
requires that the alarm be disabled (ie, suspend won't go via probe,
but a hard system reset will). I'll review this a bit more.

--

From: Alessandro Zummo
Date: Tuesday, August 10, 2010 - 6:34 am

On Mon,  9 Aug 2010 09:17:29 -0700

 if the driver can work without the alarm irq you
 can convert this to a non fatal warning.

 having an rtc without alarms is much better than having none.

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it

--

From: Kevin Wells
Date: Tuesday, August 10, 2010 - 12:08 pm

Hi Alessandro,



--

From: wellsk40
Date: Monday, August 9, 2010 - 9:17 am

From: Kevin Wells <wellsk40@gmail.com>

Enable build RTC support for the LPC32XX in makefile/Kconfig

Signed-off-by: Kevin Wells <wellsk40@gmail.com>
Signed-off-by: Durgesh Pattamatta <durgesh.pattamatta@nxp.com>
---
 drivers/rtc/Kconfig  |    9 +++++++++
 drivers/rtc/Makefile |    1 +
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 4301a6c..8452788 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -916,4 +916,13 @@ config RTC_DRV_JZ4740
 	  This driver can also be buillt as a module. If so, the module
 	  will be called rtc-jz4740.
 
+config RTC_DRV_LPC32XX
+        depends on ARCH_LPC32XX
+        tristate "NXP LPC32XX RTC"
+        help
+          This enables support for the NXP RTC in the LPC32XX
+
+	  This driver can also be buillt as a module. If so, the module
+	  will be called rtc-lpc32xx.
+
 endif # RTC_CLASS
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index fedf9bb..3b8af95 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -48,6 +48,7 @@ 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_LPC32XX)   += rtc-lpc32xx.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
-- 
1.7.1.1

--

From: Wan ZongShun
Date: Monday, August 9, 2010 - 6:45 pm

Hi Kevin ,


I think you can intergrate two patches [PATCH 1/2 2/2] into one rtc



-- 
*linux-arm-kernel mailing list
mail addr:linux-arm-kernel@lists.infradead.org
you can subscribe by:
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

* linux-arm-NUC900 mailing list
mail addr:NUC900@googlegroups.com
main web: https://groups.google.com/group/NUC900
you can subscribe it by sending me mail:
mcuos.com@gmail.com
--

From: Wolfram Sang
Date: Tuesday, August 10, 2010 - 2:25 am

+1. Also, please make proper use of tabs like the surrounding entries.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Previous thread: Contact: Mr. Well Paulson/Call Tel: +44-7035-924-138. by Uk National Lottey on Monday, August 9, 2010 - 4:14 pm. (1 message)

Next thread: Re by Cindy Howard. on Monday, August 9, 2010 - 10:12 am. (1 message)