Re: [PATCH -mm 5/5] Blackfin: on-chip RTC controller driver
On 3/1/07, Paul Mundt <[EMAIL PROTECTED]> wrote:
On Thu, Mar 01, 2007 at 12:15:46PM +0800, Wu, Bryan wrote:
> +#define stamp(fmt, args...) pr_debug("%s:%i: " fmt "\n", __FUNCTION__,
__LINE__, ## args)
> +#define stampit() stamp("here i am")
Are these really necessary for the final driver? It's littered all over
the place, and presumably the driver should be functional enough to not
need this sort of debugging instrumentation.
is there really such a thing as a "final driver" ? :)
keeping the stampit()'s in place means i dont have to re-add and
re-delete them every time some one reports a bug ...
> +static void rtc_bfin_sync_pending(void)
> +{
> + stampit();
> + while (!(bfin_read_RTC_ISTAT() & RTC_ISTAT_WRITE_COMPLETE)) {
> + if (!(bfin_read_RTC_ISTAT() & RTC_ISTAT_WRITE_PENDING))
> + break;
> + }
> + bfin_write_RTC_ISTAT(RTC_ISTAT_WRITE_COMPLETE);
> +}
No timeout? (and superfluous braces)
the ISTAT is reset every clock tick by the hardware itself ... so the
timeout is implicit
> + case RTC_PIE_ON:
> + stampit();
> + spin_lock_irq(>lock);
> + rtc_bfin_sync_pending();
And it's also called under a spinlock each time.. this is a disaster
waiting to happen.
i noted the logic behind this decision in the comments in the driver
... i too think it sucks, but i cant fathom a better idea so i'm
certainly open to suggestions :)
-mike
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 5/5] Blackfin: on-chip RTC controller driver
On Thu, Mar 01, 2007 at 12:15:46PM +0800, Wu, Bryan wrote:
> +#define stamp(fmt, args...) pr_debug("%s:%i: " fmt "\n", __FUNCTION__,
> __LINE__, ## args)
> +#define stampit() stamp("here i am")
> +
Are these really necessary for the final driver? It's littered all over
the place, and presumably the driver should be functional enough to not
need this sort of debugging instrumentation.
[snip]
> +static void rtc_bfin_sync_pending(void)
> +{
> + stampit();
> + while (!(bfin_read_RTC_ISTAT() & RTC_ISTAT_WRITE_COMPLETE)) {
> + if (!(bfin_read_RTC_ISTAT() & RTC_ISTAT_WRITE_PENDING))
> + break;
> + }
> + bfin_write_RTC_ISTAT(RTC_ISTAT_WRITE_COMPLETE);
> +}
> +
No timeout? (and superfluous braces)
> + case RTC_PIE_ON:
> + stampit();
> + spin_lock_irq(>lock);
> + rtc_bfin_sync_pending();
And it's also called under a spinlock each time.. this is a disaster
waiting to happen.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 5/5] Blackfin: on-chip RTC controller driver
On Thu, Mar 01, 2007 at 12:15:46PM +0800, Wu, Bryan wrote:
+#define stamp(fmt, args...) pr_debug(%s:%i: fmt \n, __FUNCTION__,
__LINE__, ## args)
+#define stampit() stamp(here i am)
+
Are these really necessary for the final driver? It's littered all over
the place, and presumably the driver should be functional enough to not
need this sort of debugging instrumentation.
[snip]
+static void rtc_bfin_sync_pending(void)
+{
+ stampit();
+ while (!(bfin_read_RTC_ISTAT() RTC_ISTAT_WRITE_COMPLETE)) {
+ if (!(bfin_read_RTC_ISTAT() RTC_ISTAT_WRITE_PENDING))
+ break;
+ }
+ bfin_write_RTC_ISTAT(RTC_ISTAT_WRITE_COMPLETE);
+}
+
No timeout? (and superfluous braces)
+ case RTC_PIE_ON:
+ stampit();
+ spin_lock_irq(rtc-lock);
+ rtc_bfin_sync_pending();
And it's also called under a spinlock each time.. this is a disaster
waiting to happen.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 5/5] Blackfin: on-chip RTC controller driver
On 3/1/07, Paul Mundt [EMAIL PROTECTED] wrote:
On Thu, Mar 01, 2007 at 12:15:46PM +0800, Wu, Bryan wrote:
+#define stamp(fmt, args...) pr_debug(%s:%i: fmt \n, __FUNCTION__,
__LINE__, ## args)
+#define stampit() stamp(here i am)
Are these really necessary for the final driver? It's littered all over
the place, and presumably the driver should be functional enough to not
need this sort of debugging instrumentation.
is there really such a thing as a final driver ? :)
keeping the stampit()'s in place means i dont have to re-add and
re-delete them every time some one reports a bug ...
+static void rtc_bfin_sync_pending(void)
+{
+ stampit();
+ while (!(bfin_read_RTC_ISTAT() RTC_ISTAT_WRITE_COMPLETE)) {
+ if (!(bfin_read_RTC_ISTAT() RTC_ISTAT_WRITE_PENDING))
+ break;
+ }
+ bfin_write_RTC_ISTAT(RTC_ISTAT_WRITE_COMPLETE);
+}
No timeout? (and superfluous braces)
the ISTAT is reset every clock tick by the hardware itself ... so the
timeout is implicit
+ case RTC_PIE_ON:
+ stampit();
+ spin_lock_irq(rtc-lock);
+ rtc_bfin_sync_pending();
And it's also called under a spinlock each time.. this is a disaster
waiting to happen.
i noted the logic behind this decision in the comments in the driver
... i too think it sucks, but i cant fathom a better idea so i'm
certainly open to suggestions :)
-mike
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH -mm 5/5] Blackfin: on-chip RTC controller driver
Hi folks,
Here is the blackfin on-chip RTC controller driver for Linux.
It's name is blackfin-driver-rtc.patch
[PATCH] Blackfin: on-chip RTC controller driver
This patch implements the driver necessary use the Analog Devices
Blackfin processor's on-chip RTC controller.
Signed-off-by: Bryan Wu <[EMAIL PROTECTED]>
---
Kconfig| 10 +
Makefile |1
rtc-bfin.c | 445 +
3 files changed, 456 insertions(+)
Index: linux-2.6/drivers/rtc/Kconfig
===
--- linux-2.6.orig/drivers/rtc/Kconfig 2007-03-01 11:33:07.0 +0800
+++ linux-2.6/drivers/rtc/Kconfig 2007-03-01 11:40:17.0 +0800
@@ -352,4 +352,14 @@
This driver can also be built as a module. If so, the module
will be called rtc-v3020.
+config RTC_DRV_BFIN
+ tristate "Blackfin On-Chip RTC"
+ depends on RTC_CLASS && BFIN
+ help
+ If you say yes here you will get support for the
+ Blackfin On-Chip Real Time Clock.
+
+ This driver can also be built as a module. If so, the module
+ will be called rtc-bfin.
+
endmenu
Index: linux-2.6/drivers/rtc/Makefile
===
--- linux-2.6.orig/drivers/rtc/Makefile 2007-03-01 11:33:07.0 +0800
+++ linux-2.6/drivers/rtc/Makefile 2007-03-01 11:40:17.0 +0800
@@ -38,3 +38,4 @@
obj-$(CONFIG_RTC_DRV_V3020)+= rtc-v3020.o
obj-$(CONFIG_RTC_DRV_AT91RM9200)+= rtc-at91rm9200.o
obj-$(CONFIG_RTC_DRV_SH) += rtc-sh.o
+obj-$(CONFIG_RTC_DRV_BFIN) += rtc-bfin.o
Index: linux-2.6/drivers/rtc/rtc-bfin.c
===
--- /dev/null 1970-01-01 00:00:00.0 +
+++ linux-2.6/drivers/rtc/rtc-bfin.c2007-03-01 11:40:17.0 +0800
@@ -0,0 +1,445 @@
+/*
+ * Blackfin On-Chip Real Time Clock Driver
+ * Supports BF531/BF532/BF533/BF534/BF536/BF537
+ *
+ * Copyright 2004-2007 Analog Devices Inc.
+ *
+ * Enter bugs at http://blackfin.uclinux.org/
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+/* The biggest issue we deal with in this driver is that register writes are
+ * synced to the RTC frequency of 1Hz. So if you write to a register and
+ * attempt to write again before the first write has completed, the new write
+ * is simply discarded. This can easily be troublesome if userspace disables
+ * one event (say periodic) and then right after enables an event (say alarm).
+ * Since all events are maintained in the same interrupt mask register, if
+ * we wrote to it to disable the first event and then wrote to it again to
+ * enable the second event, that second event would not be enabled as the
+ * write would be discarded and things quickly fall apart.
+ *
+ * To keep this delay from significantly degrading performance (we, in theory,
+ * would have to sleep for up to 1 second everytime we wanted to write a
+ * register), we only check the write pending status before we start to issue
+ * a new write. We bank on the idea that it doesnt matter when the sync
+ * happens so long as we don't attempt another write before it does. The only
+ * time userspace would take this penalty is when they try and do multiple
+ * operations right after another ... but in this case, they need to take the
+ * sync penalty, so we should be OK.
+ *
+ * Also note that the RTC_ISTAT register does not suffer this penalty; its
+ * writes to clear status registers complete immediately.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#define stamp(fmt, args...) pr_debug("%s:%i: " fmt "\n", __FUNCTION__,
__LINE__, ## args)
+#define stampit() stamp("here i am")
+
+struct bfin_rtc {
+ struct rtc_device *rtc_dev;
+ struct rtc_time rtc_alarm;
+ spinlock_t lock;
+};
+
+/* Bit values for the ISTAT / ICTL registers */
+#define RTC_ISTAT_WRITE_COMPLETE 0x8000
+#define RTC_ISTAT_WRITE_PENDING 0x4000
+#define RTC_ISTAT_ALARM_DAY 0x0040
+#define RTC_ISTAT_24HR0x0020
+#define RTC_ISTAT_HOUR0x0010
+#define RTC_ISTAT_MIN 0x0008
+#define RTC_ISTAT_SEC 0x0004
+#define RTC_ISTAT_ALARM 0x0002
+#define RTC_ISTAT_STOPWATCH 0x0001
+
+/* Shift values for RTC_STAT register */
+#define DAY_BITS_OFF17
+#define HOUR_BITS_OFF 12
+#define MIN_BITS_OFF6
+#define SEC_BITS_OFF0
+
+/* Some helper functions to convert between the common RTC notion of time
+ * and the internal Blackfin notion that is stored in 32bits.
+ */
+static inline u32 rtc_time_to_bfin(unsigned long now)
+{
+ u32 sec = (now % 60);
+ u32 min = (now % (60 * 60)) / 60;
+ u32 hour = (now % (60 * 60 * 24)) / (60 * 60);
+ u32 days = (now / (60 * 60 * 24));
+ return (sec << SEC_BITS_OFF) +
+ (min << MIN_BITS_OFF) +
+
[PATCH -mm 5/5] Blackfin: on-chip RTC controller driver
Hi folks,
Here is the blackfin on-chip RTC controller driver for Linux.
It's name is blackfin-driver-rtc.patch
[PATCH] Blackfin: on-chip RTC controller driver
This patch implements the driver necessary use the Analog Devices
Blackfin processor's on-chip RTC controller.
Signed-off-by: Bryan Wu [EMAIL PROTECTED]
---
Kconfig| 10 +
Makefile |1
rtc-bfin.c | 445 +
3 files changed, 456 insertions(+)
Index: linux-2.6/drivers/rtc/Kconfig
===
--- linux-2.6.orig/drivers/rtc/Kconfig 2007-03-01 11:33:07.0 +0800
+++ linux-2.6/drivers/rtc/Kconfig 2007-03-01 11:40:17.0 +0800
@@ -352,4 +352,14 @@
This driver can also be built as a module. If so, the module
will be called rtc-v3020.
+config RTC_DRV_BFIN
+ tristate Blackfin On-Chip RTC
+ depends on RTC_CLASS BFIN
+ help
+ If you say yes here you will get support for the
+ Blackfin On-Chip Real Time Clock.
+
+ This driver can also be built as a module. If so, the module
+ will be called rtc-bfin.
+
endmenu
Index: linux-2.6/drivers/rtc/Makefile
===
--- linux-2.6.orig/drivers/rtc/Makefile 2007-03-01 11:33:07.0 +0800
+++ linux-2.6/drivers/rtc/Makefile 2007-03-01 11:40:17.0 +0800
@@ -38,3 +38,4 @@
obj-$(CONFIG_RTC_DRV_V3020)+= rtc-v3020.o
obj-$(CONFIG_RTC_DRV_AT91RM9200)+= rtc-at91rm9200.o
obj-$(CONFIG_RTC_DRV_SH) += rtc-sh.o
+obj-$(CONFIG_RTC_DRV_BFIN) += rtc-bfin.o
Index: linux-2.6/drivers/rtc/rtc-bfin.c
===
--- /dev/null 1970-01-01 00:00:00.0 +
+++ linux-2.6/drivers/rtc/rtc-bfin.c2007-03-01 11:40:17.0 +0800
@@ -0,0 +1,445 @@
+/*
+ * Blackfin On-Chip Real Time Clock Driver
+ * Supports BF531/BF532/BF533/BF534/BF536/BF537
+ *
+ * Copyright 2004-2007 Analog Devices Inc.
+ *
+ * Enter bugs at http://blackfin.uclinux.org/
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+/* The biggest issue we deal with in this driver is that register writes are
+ * synced to the RTC frequency of 1Hz. So if you write to a register and
+ * attempt to write again before the first write has completed, the new write
+ * is simply discarded. This can easily be troublesome if userspace disables
+ * one event (say periodic) and then right after enables an event (say alarm).
+ * Since all events are maintained in the same interrupt mask register, if
+ * we wrote to it to disable the first event and then wrote to it again to
+ * enable the second event, that second event would not be enabled as the
+ * write would be discarded and things quickly fall apart.
+ *
+ * To keep this delay from significantly degrading performance (we, in theory,
+ * would have to sleep for up to 1 second everytime we wanted to write a
+ * register), we only check the write pending status before we start to issue
+ * a new write. We bank on the idea that it doesnt matter when the sync
+ * happens so long as we don't attempt another write before it does. The only
+ * time userspace would take this penalty is when they try and do multiple
+ * operations right after another ... but in this case, they need to take the
+ * sync penalty, so we should be OK.
+ *
+ * Also note that the RTC_ISTAT register does not suffer this penalty; its
+ * writes to clear status registers complete immediately.
+ */
+
+#include linux/module.h
+#include linux/kernel.h
+#include linux/bcd.h
+#include linux/rtc.h
+#include linux/init.h
+#include linux/platform_device.h
+#include linux/seq_file.h
+#include linux/interrupt.h
+#include linux/spinlock.h
+#include linux/delay.h
+
+#include asm/blackfin.h
+
+#define stamp(fmt, args...) pr_debug(%s:%i: fmt \n, __FUNCTION__,
__LINE__, ## args)
+#define stampit() stamp(here i am)
+
+struct bfin_rtc {
+ struct rtc_device *rtc_dev;
+ struct rtc_time rtc_alarm;
+ spinlock_t lock;
+};
+
+/* Bit values for the ISTAT / ICTL registers */
+#define RTC_ISTAT_WRITE_COMPLETE 0x8000
+#define RTC_ISTAT_WRITE_PENDING 0x4000
+#define RTC_ISTAT_ALARM_DAY 0x0040
+#define RTC_ISTAT_24HR0x0020
+#define RTC_ISTAT_HOUR0x0010
+#define RTC_ISTAT_MIN 0x0008
+#define RTC_ISTAT_SEC 0x0004
+#define RTC_ISTAT_ALARM 0x0002
+#define RTC_ISTAT_STOPWATCH 0x0001
+
+/* Shift values for RTC_STAT register */
+#define DAY_BITS_OFF17
+#define HOUR_BITS_OFF 12
+#define MIN_BITS_OFF6
+#define SEC_BITS_OFF0
+
+/* Some helper functions to convert between the common RTC notion of time
+ * and the internal Blackfin notion that is stored in 32bits.
+ */
+static inline u32 rtc_time_to_bfin(unsigned long now)
+{
+ u32 sec = (now % 60);
+ u32 min = (now % (60 * 60)) / 60;
+ u32 hour = (now % (60 * 60
