[i2c] Question on ds1339's oscillator

[David Brownell]

On Friday 15 June 2007, Rodolfo Giometti wrote:
> Hello,
> 
> I'm looking at code rtc-ds1307.c in order to add IRQ management support

Glad to hear it ... do you have the latest patches?

  http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc4
	/2.6.22-rc4-mm2/broken-out/rtc-ds1307-cleanups.patch

plus the appended patch, converting to a new style driver so that
the alarm IRQ is in hand.  As noted, the rtc-rs5c372 driver has
partial support (alarm IRQ not hooked up!) for alarms


> and I don't understand this code:
> 
>                 /* oscillator is off; need to turn it on */
>                 if ((ds1307->regs[0] & DS1337_BIT_nEOSC)
>                                 || (ds1307->regs[1] & DS1337_BIT_OSF)) {
> no_osc_start:
>                         printk(KERN_ERR "no %s oscillator code\n",
>                                 chip->name);
>                         goto exit_free;
>                 }
> 
> Why if the oscillator is off we cannot use the RTC? Disabling such
> code, and the oscillator, the RTC works perfectly.

Say what --- an RTC can function with its oscillator off??

Or are you saying that check is incorrect?  I could imagine
that OSFt should maybe be ignored, other than for purposes
of issuing a warning (before clearing it).


> Anyway if I wish using the IRQ pin I have to disable the square wave
> output (INTCN=1 of register 0Eh) so I can keep the oscillator off...

Yes, certainly.  The general policy should be, I think, that the
RTC init sequence won't change configuration ... so that for example
if you're given system with the square wave output already set up,
the assumption should be that the board is wired to require that.

- Dave



===========	CUT HERE
Convert the rtc-ds1307 driver into a "new style" driver.
Note that this depends on a previous cleanup patch which lays
groundwork for this one.

Signed-off-by: David Brownell <dbrownell at users.sourceforge.net>

Index: at91/drivers/rtc/rtc-ds1307.c
===================================================================
--- at91.orig/drivers/rtc/rtc-ds1307.c	2007-03-08 12:14:46.000000000 -0800
+++ at91/drivers/rtc/rtc-ds1307.c	2007-03-08 12:15:12.000000000 -0800
@@ -27,10 +27,6 @@
  * This is currently a simple no-alarms driver.  If your board has the
  * alarm irq wired up on a ds1337 or ds1339, and you want to use that,
  * then look at the rtc-rs5c372 driver for code to steal...
- *
- * If the I2C "force" mechanism is used, we assume the chip is a ds1337.
- * (Much better would be board-specific tables of I2C devices, along with
- * the platform_data drivers would use to sort such issues out.)
  */
 enum ds_type {
 	unknown = 0,
@@ -43,11 +39,6 @@ enum ds_type {
 	// rs5c372 too?  different address...
 };
 
-static unsigned short normal_i2c[] = { 0x68, I2C_CLIENT_END };
-
-I2C_CLIENT_INSMOD;
-
-
 
 /* RTC registers don't differ much, except for the century flag */
 #define DS1307_REG_SECS		0x00	/* 00-59 */
@@ -244,39 +235,26 @@ static const struct rtc_class_ops ds13xx
 
 static struct i2c_driver ds1307_driver;
 
-static int __devinit
-ds1307_detect(struct i2c_adapter *adapter, int address, int kind)
+static int __devinit ds1307_probe(struct i2c_client *client)
 {
 	struct ds1307		*ds1307;
 	int			err = -ENODEV;
-	struct i2c_client	*client;
 	int			tmp;
 	const struct chip_desc	*chip;
+	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
 
-	if (!(ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL))) {
-		err = -ENOMEM;
-		goto exit;
+	chip = find_chip(client->name);
+	if (!chip) {
+		dev_err(&client->dev, "unknown chip type '%s'\n",
+				client->name);
+		return -ENODEV;
 	}
 
-	/* REVISIT:  pending driver model conversion, set up "client"
-	 * ourselves, and use a hack to determine the RTC type (instead
-	 * of reading the client->name we're given)
-	 */
-	client = &ds1307->dev;
-	client->addr = address;
-	client->adapter = adapter;
-	client->driver = &ds1307_driver;
-
-	/* HACK: "force" implies "needs ds1337-style-oscillator setup", and
-	 * that's the only kind of chip setup we'll know about.  Until the
-	 * driver model conversion, here's where to add any board-specific
-	 * code to say what kind of chip is present...
-	 */
-	if (kind >= 0)
-		chip = find_chip("ds1337");
-	else
-		chip = find_chip("ds1307");
-	strlcpy(client->name, chip->name, I2C_NAME_SIZE);
+	if (!i2c_check_functionality(adapter, I2C_FUNC_I2C))
+		return -EIO;
+
+	if (!(ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL)))
+		return -ENOMEM;
 
 	ds1307->client = client;
 	i2c_set_clientdata(client, ds1307);
@@ -392,56 +370,38 @@ read_rtc:
 				BIN2BCD(tmp));
 	}
 
-	/* Tell the I2C layer a new client has arrived */
-	if ((err = i2c_attach_client(client)))
-		goto exit_free;
-
 	ds1307->rtc = rtc_device_register(client->name, &client->dev,
 				&ds13xx_rtc_ops, THIS_MODULE);
 	if (IS_ERR(ds1307->rtc)) {
 		err = PTR_ERR(ds1307->rtc);
 		dev_err(&client->dev,
 			"unable to register the class device\n");
-		goto exit_detach;
+		goto exit_free;
 	}
 
 	return 0;
 
-exit_detach:
-	i2c_detach_client(client);
 exit_free:
 	kfree(ds1307);
-exit:
 	return err;
 }
 
-static int __devinit
-ds1307_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!i2c_check_functionality(adapter, I2C_FUNC_I2C))
-		return 0;
-	return i2c_probe(adapter, &addr_data, ds1307_detect);
-}
-
-static int __devexit ds1307_detach_client(struct i2c_client *client)
+static int __devexit ds1307_remove(struct i2c_client *client)
 {
-	int		err;
 	struct ds1307	*ds1307 = i2c_get_clientdata(client);
 
 	rtc_device_unregister(ds1307->rtc);
-	if ((err = i2c_detach_client(client)))
-		return err;
 	kfree(ds1307);
 	return 0;
 }
 
 static struct i2c_driver ds1307_driver = {
 	.driver = {
-		.name	= "ds1307",
+		.name	= "rtc-ds1307",
 		.owner	= THIS_MODULE,
 	},
-	.attach_adapter	= ds1307_attach_adapter,
-	.detach_client	= __devexit_p(ds1307_detach_client),
+	.probe		= ds1307_probe,
+	.remove		= __devexit_p(ds1307_remove),
 };
 
 static int __init ds1307_init(void)