diff mbox

rtc/ds1307: add ds3231

Message ID 1236783236-10335-1-git-send-email-w.sang@pengutronix.de
State Superseded, archived
Headers show

Commit Message

Wolfram Sang March 11, 2009, 2:53 p.m. UTC
Add ds3231 variant. For that, the BBSQI usage was changed from a
simple define into a lookup-array as it differs. This also removes
writing to an unused bit in case of the ds1337.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/rtc/rtc-ds1307.c |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

Comments

David Brownell March 12, 2009, 2:05 a.m. UTC | #1
On Wednesday 11 March 2009, Wolfram Sang wrote:
> @@ -534,6 +541,11 @@ static int __devinit ds1307_probe(struct i2c_client *client,
>         struct i2c_adapter      *adapter = to_i2c_adapter(client->dev.parent);
>         int                     want_irq = false;
>         unsigned char           *buf;
> +       const int               bbsqi_bitpos[] = {

static const ...

> +               [ds_1337] = 0,
> +               [ds_1339] = DS1339_BIT_BBSQI,
> +               [ds_3231] = DS3231_BIT_BBSQW,
> +       };

.... otherwise this looks plausible.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
-~----------~----~----~----~------~----~------~--~---
Wolfram Sang March 18, 2009, 3:16 p.m. UTC | #2
Hello David,

On Wed, Mar 11, 2009 at 06:05:02PM -0800, David Brownell wrote:
> On Wednesday 11 March 2009, Wolfram Sang wrote:
> > @@ -534,6 +541,11 @@ static int __devinit ds1307_probe(struct i2c_client *client,
> >         struct i2c_adapter      *adapter = to_i2c_adapter(client->dev.parent);
> >         int                     want_irq = false;
> >         unsigned char           *buf;
> > +       const int               bbsqi_bitpos[] = {
> 
> static const ...
> 
> > +               [ds_1337] = 0,
> > +               [ds_1339] = DS1339_BIT_BBSQI,
> > +               [ds_3231] = DS3231_BIT_BBSQW,
> > +       };
> 
> .... otherwise this looks plausible.

I'm not sure if I understood you correctly, so I better ask: Do you
think it is OK? :)

Regards,

   Wolfram
David Brownell March 18, 2009, 8:26 p.m. UTC | #3
On Wednesday 18 March 2009, Wolfram Sang wrote:
> Hello David,
> 
> On Wed, Mar 11, 2009 at 06:05:02PM -0800, David Brownell wrote:
> > On Wednesday 11 March 2009, Wolfram Sang wrote:
> > > @@ -534,6 +541,11 @@ static int __devinit ds1307_probe(struct i2c_client *client,
> > >         struct i2c_adapter      *adapter = to_i2c_adapter(client->dev.parent);
> > >         int                     want_irq = false;
> > >         unsigned char           *buf;
> > > +       const int               bbsqi_bitpos[] = {
> > 
> > static const ...

Meaning:  change this to be static const, instead of
telling the compiler to allocate a new copy on the
stack for each probe().


> > > +               [ds_1337] = 0,
> > > +               [ds_1339] = DS1339_BIT_BBSQI,
> > > +               [ds_3231] = DS3231_BIT_BBSQW,
> > > +       };
> > 
> > .... otherwise this looks plausible.
> 
> I'm not sure if I understood you correctly, so I better ask: Do you
> think it is OK? :)

If you make that change, yes.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
-~----------~----~----~----~------~----~------~--~---
Wolfram Sang March 19, 2009, 10:18 a.m. UTC | #4
Hello David,

> > > .... otherwise this looks plausible.
> > 
> > I'm not sure if I understood you correctly, so I better ask: Do you
> > think it is OK? :)
> 
> If you make that change, yes.

Ah, okay. I knew what the proposed 'static' does; I wasn't sure if your
'otherwise' meant something like 'on the other hand' and was confused.
Will submit a new patch in a minute.

Thanks!
diff mbox

Patch

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 7e5155e..ffc0bbb 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -30,6 +30,7 @@  enum ds_type {
 	ds_1338,
 	ds_1339,
 	ds_1340,
+	ds_3231,
 	m41t00,
 	// rs5c372 too?  different address...
 };
@@ -64,6 +65,7 @@  enum ds_type {
 #define DS1337_REG_CONTROL	0x0e
 #	define DS1337_BIT_nEOSC		0x80
 #	define DS1339_BIT_BBSQI		0x20
+#	define DS3231_BIT_BBSQW		0x40 /* same as BBSQI */
 #	define DS1337_BIT_RS2		0x10
 #	define DS1337_BIT_RS1		0x08
 #	define DS1337_BIT_INTCN		0x04
@@ -116,6 +118,9 @@  static const struct chip_desc chips[] = {
 },
 [ds_1340] = {
 },
+[ds_3231] = {
+	.alarm		= 1,
+},
 [m41t00] = {
 }, };
 
@@ -124,6 +129,7 @@  static const struct i2c_device_id ds1307_id[] = {
 	{ "ds1337", ds_1337 },
 	{ "ds1338", ds_1338 },
 	{ "ds1339", ds_1339 },
+	{ "ds3231", ds_3231 },
 	{ "ds1340", ds_1340 },
 	{ "m41t00", m41t00 },
 	{ }
@@ -265,6 +271,7 @@  static int ds1307_set_time(struct device *dev, struct rtc_time *t)
 	switch (ds1307->type) {
 	case ds_1337:
 	case ds_1339:
+	case ds_3231:
 		buf[DS1307_REG_MONTH] |= DS1337_BIT_CENTURY;
 		break;
 	case ds_1340:
@@ -534,6 +541,11 @@  static int __devinit ds1307_probe(struct i2c_client *client,
 	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
 	int			want_irq = false;
 	unsigned char		*buf;
+	const int		bbsqi_bitpos[] = {
+		[ds_1337] = 0,
+		[ds_1339] = DS1339_BIT_BBSQI,
+		[ds_3231] = DS3231_BIT_BBSQW,
+	};
 
 	if (!i2c_check_functionality(adapter,
 			I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
@@ -551,6 +563,7 @@  static int __devinit ds1307_probe(struct i2c_client *client,
 	switch (ds1307->type) {
 	case ds_1337:
 	case ds_1339:
+	case ds_3231:
 		/* has IRQ? */
 		if (ds1307->client->irq > 0 && chip->alarm) {
 			INIT_WORK(&ds1307->work, ds1307_work);
@@ -570,12 +583,12 @@  static int __devinit ds1307_probe(struct i2c_client *client,
 			ds1307->regs[0] &= ~DS1337_BIT_nEOSC;
 
 		/* Using IRQ?  Disable the square wave and both alarms.
-		 * For ds1339, be sure alarms can trigger when we're
-		 * running on Vbackup (BBSQI); we assume ds1337 will
-		 * ignore that bit
+		 * For some variants, be sure alarms can trigger when we're
+		 * running on Vbackup (BBSQI/BBSQW)
 		 */
 		if (want_irq) {
-			ds1307->regs[0] |= DS1337_BIT_INTCN | DS1339_BIT_BBSQI;
+			ds1307->regs[0] |= DS1337_BIT_INTCN
+					| bbsqi_bitpos[ds1307->type];
 			ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
 		}
 
@@ -651,6 +664,7 @@  read_rtc:
 		break;
 	case ds_1337:
 	case ds_1339:
+	case ds_3231:
 		break;
 	}