Re: [PATCH] RTC: Add platform data structure to Ricoh RS5C372 driver

Previous thread: [PATCH] Fix build error introduced by commit 4faac97d44ac27bdbb010a9c3597401a8f89341f by Marc Dionne on Tuesday, September 23, 2008 - 7:40 pm. (2 messages)

Next thread: Re: [PATCH] md: Add support for data integrity to MD by Neil Brown on Tuesday, September 23, 2008 - 7:46 pm. (2 messages)
From: Alexey Kopytko
Date: Tuesday, September 23, 2008 - 7:45 pm

From: Alexey Kopytko <alexey@kopytko.ru>

This patch enables a platform developer to choose which alarm register to use.
It adds and properly initializes platform data structure.

---
RS5C_REG_ALARM_B_MIN is used to store power state in
Buffalo Linkstation Mini and some other Linkstations.
Tested with and without platform data.

Signed-off-by: Alexey Kopytko <alexey@kopytko.ru>

diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
index 56caf6b..97519a0 100644
--- a/drivers/rtc/rtc-rs5c372.c
+++ b/drivers/rtc/rtc-rs5c372.c
@@ -12,6 +12,7 @@
 #include <linux/i2c.h>
 #include <linux/rtc.h>
 #include <linux/bcd.h>
+#include <linux/rtc/rs5c372.h>

 #define DRV_VERSION "0.5"

@@ -33,14 +34,6 @@
 #	define RS5C372_TRIM_XSL		0x80
 #	define RS5C372_TRIM_MASK	0x7F

-#define RS5C_REG_ALARM_A_MIN	8			/* or ALARM_W */
-#define RS5C_REG_ALARM_A_HOURS	9
-#define RS5C_REG_ALARM_A_WDAY	10
-
-#define RS5C_REG_ALARM_B_MIN	11			/* or ALARM_D */
-#define RS5C_REG_ALARM_B_HOURS	12
-#define RS5C_REG_ALARM_B_WDAY	13			/* (ALARM_B only) */
-
 #define RS5C_REG_CTRL1		14
 #	define RS5C_CTRL1_AALE		(1 << 7)	/* or WALE */
 #	define RS5C_CTRL1_BALE		(1 << 6)	/* or DALE */
@@ -91,6 +84,7 @@ struct rs5c372 {
 	unsigned		has_irq:1;
 	char			buf[17];
 	char			*regs;
+	struct rs5c_plat_data	pdata;
 };

 static int rs5c_get_regs(struct rs5c372 *rs5c)
@@ -344,8 +338,8 @@ static int rs5c_read_alarm(struct device *dev,
struct rtc_wkalrm *t)

 	/* report alarm time */
 	t->time.tm_sec = 0;
-	t->time.tm_min = BCD2BIN(rs5c->regs[RS5C_REG_ALARM_A_MIN] & 0x7f);
-	t->time.tm_hour = rs5c_reg2hr(rs5c, rs5c->regs[RS5C_REG_ALARM_A_HOURS]);
+	t->time.tm_min = BCD2BIN(rs5c->regs[rs5c->pdata.alarm_min] & 0x7f);
+	t->time.tm_hour = rs5c_reg2hr(rs5c, rs5c->regs[rs5c->pdata.alarm_hours]);
 	t->time.tm_mday = -1;
 	t->time.tm_mon = -1;
 	t->time.tm_year = -1;
@@ -390,7 +384,7 @@ static int rs5c_set_alarm(struct device *dev,
struct rtc_wkalrm *t)
 	}

 	/* set alarm */
-	buf[0] = ...
From: Paul Mundt
Date: Tuesday, September 23, 2008 - 10:08 pm

I don't think this is a meaningful abstraction. Pushing this sort of
knowledge in to the platform code is pretty ugly, especially when the
only distinction you need is whether to use the A set or the B set.

Given that, you could simply have a flags field in the platform data and
use one bit to test in the driver for using the B set of registers (A is
always the default otherwise).
--

Previous thread: [PATCH] Fix build error introduced by commit 4faac97d44ac27bdbb010a9c3597401a8f89341f by Marc Dionne on Tuesday, September 23, 2008 - 7:40 pm. (2 messages)

Next thread: Re: [PATCH] md: Add support for data integrity to MD by Neil Brown on Tuesday, September 23, 2008 - 7:46 pm. (2 messages)