Re: [PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

2005-03-04 Thread Jean Delvare

Hi James, all,

> Revised patch is attached.

Looks all OK to me.

Thanks,
--
Jean Delvare
-
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: 2.6.11-rc5] i2c chips: ds1337 RTC driver

2005-03-03 Thread James Chapman
Revised patch is attached.
Jean Delvare wrote:
Hi James,

A revised ds1337 patch addressing all of Jean's comments is attached.

Fine with me except for:

+   if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
+I2C_FUNC_SMBUS_I2C_BLOCK))

I don't this it is correct. You are using master_xfer, not
i2c_smbus_{read,write}_i2c_block_data. Adapter which declare themselves
I2C_FUNC_SMBUS_I2C_BLOCK-capable may not implement master_xfer. You
really need to check for I2C_FUNC_I2C.
i2c: add ds1337 RTC chip support

Signed-off-by: James Chapman <[EMAIL PROTECTED]>

Index: linux-2.6.i2c/drivers/i2c/chips/ds1337.c
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6.i2c/drivers/i2c/chips/ds1337.c2005-03-04 00:01:44.0 
+
@@ -0,0 +1,380 @@
+/*
+ *  linux/drivers/i2c/chips/ds1337.c
+ *
+ *  Copyright (C) 2005 James Chapman <[EMAIL PROTECTED]>
+ *
+ * based on linux/drivers/acron/char/pcf8583.c
+ *  Copyright (C) 2000 Russell King
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Driver for Dallas Semiconductor DS1337 real time clock chip
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include  /* get the user-level API */
+#include 
+#include 
+
+#define DS1337_NUM_REGS16
+#define DS1337_REG_CONFIG  2
+
+/* FIXME - how do we export these interface constants? */
+#define DS1337_GET_DATE0
+#define DS1337_SET_DATE1
+
+/*
+ * Functions declaration
+ */
+static unsigned short normal_i2c[] = { 0x68, I2C_CLIENT_END };
+static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
+
+SENSORS_INSMOD_1(ds1337);
+
+static int ds1337_attach_adapter(struct i2c_adapter *adapter);
+static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind);
+static void ds1337_init_client(struct i2c_client *client);
+static int ds1337_detach_client(struct i2c_client *client);
+static int ds1337_command(struct i2c_client *client, unsigned int cmd,
+ void *arg);
+
+/*
+ * Driver data (common to all clients)
+ */
+static struct i2c_driver ds1337_driver = {
+   .owner  = THIS_MODULE,
+   .name   = "ds1337",
+   .flags  = I2C_DF_NOTIFY,
+   .attach_adapter = ds1337_attach_adapter,
+   .detach_client  = ds1337_detach_client,
+   .command= ds1337_command,
+};
+
+/*
+ * Client data (each client gets its own)
+ */
+struct ds1337_data {
+   struct i2c_client client;
+   struct list_head list;
+   int id;
+};
+
+/*
+ * Internal variables
+ */
+static int ds1337_id;
+static LIST_HEAD(ds1337_clients);
+
+static inline int ds1337_read(struct i2c_client *client, u8 reg, u8 *value)
+{
+   s32 tmp = i2c_smbus_read_byte_data(client, reg);
+
+   if (tmp < 0)
+   return -EIO;
+
+   *value = tmp;
+
+   return 0;
+}
+
+/*
+ * Chip access functions
+ */
+static int ds1337_get_datetime(struct i2c_client *client, struct rtc_time *dt)
+{
+   struct ds1337_data *data = i2c_get_clientdata(client);
+   int result;
+   u8 buf[7];
+   u8 val;
+   struct i2c_msg msg[2];
+   u8 offs = 0;
+
+   if (!dt) {
+   dev_dbg(&client->adapter->dev, "%s: EINVAL: dt=NULL\n",
+   __FUNCTION__);
+
+   return -EINVAL;
+   }
+
+   msg[0].addr = client->addr;
+   msg[0].flags = 0;
+   msg[0].len = 1;
+   msg[0].buf = &offs;
+
+   msg[1].addr = client->addr;
+   msg[1].flags = I2C_M_RD;
+   msg[1].len = sizeof(buf);
+   msg[1].buf = &buf[0];
+
+   result = client->adapter->algo->master_xfer(client->adapter,
+   &msg[0], 2);
+
+   dev_dbg(&client->adapter->dev,
+   "%s: [%d] %02x %02x %02x %02x %02x %02x %02x\n",
+   __FUNCTION__, result, buf[0], buf[1], buf[2], buf[3],
+   buf[4], buf[5], buf[6]);
+
+   if (result >= 0) {
+   dt->tm_sec = BCD_TO_BIN(buf[0]);
+   dt->tm_min = BCD_TO_BIN(buf[1]);
+   val = buf[2] & 0x3f;
+   dt->tm_hour = BCD_TO_BIN(val);
+   dt->tm_wday = BCD_TO_BIN(buf[3]) - 1;
+   dt->tm_mday = BCD_TO_BIN(buf[4]);
+   val = buf[5] & 0x7f;
+   dt->tm_mon = BCD_TO_BIN(val);
+   dt->tm_year = 1900 + BCD_TO_BIN(buf[6]);
+   if (buf[5] & 0x80)
+   dt->tm_year += 100;
+
+   dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, "
+   "hours=%d, mday=%d, mon=%d, year=%d, wday=%d\n",
+   __FUNCTION__, dt->tm_sec, dt->tm_min,
+   dt->tm_hour, dt->tm_mday,
+ 

Re: [PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

2005-03-03 Thread Jean Delvare
Hi James,

> A revised ds1337 patch addressing all of Jean's comments is attached.

Fine with me except for:

> + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
> +  I2C_FUNC_SMBUS_I2C_BLOCK))

I don't this it is correct. You are using master_xfer, not
i2c_smbus_{read,write}_i2c_block_data. Adapter which declare themselves
I2C_FUNC_SMBUS_I2C_BLOCK-capable may not implement master_xfer. You
really need to check for I2C_FUNC_I2C.

Now I agree that the transfers you do ARE i2c block transfers, and I
find it highly questionable that our implementation of
i2c_smbus_read_i2c_block_data will always read 32 bytes of data from the
chip. It would be much more convenient to allow I2C block reads of
arbitrary length, (just like we do with I2C block writes) so that
clients can use this function instead of master_xfer.

It should be a quite simple fix if I correctly remember the i2c-core
code, with the only drawback that it alters the API. That being said,
the only kernel user (in kernel) of this function that I could find is
the eeprom driver (this can be easily explained by the fact that this
function, as it is now, is essentially useless), so I wouldn't mind the
risk. The net benefit would be that i2c chip drivers could start using
this function instead of master_xfer, so they would possibly work with
more than just the fully I2C-capable adapters (not that many of them,
see list right below).

i2c-dev might be a problem if we go that way, because we will silently
change the way I2C block reads requested from userspace are handled. Not
sure it is a big issue though, because as underlined before, the
function as it is now is rather useless. I doubt that anything but
i2cdump uses it in userspace.

Then we would need to fix bus drivers that implement the call by
themselves (as opposed to emulation), but in fact only a few of them do
(i2c-amd8111 and i2c-nforce2) so that should be quickly done.
[Reading the two bus drivers code...]

And it turns out that both bus drivers ALREADY honor the length
requested by the caller, which is not consistent with the emulated
variant of the call. Something definitely must be done.

Any thoughts anyone?

Thanks,
-- 
Jean Delvare
-
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: 2.6.11-rc5] i2c chips: ds1337 RTC driver

2005-03-03 Thread James Chapman
A revised ds1337 patch addressing all of Jean's comments is attached.
i2c: add ds1337 RTC chip support

Signed-off-by: James Chapman <[EMAIL PROTECTED]>

Index: linux-2.6.i2c/drivers/i2c/chips/ds1337.c
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6.i2c/drivers/i2c/chips/ds1337.c2005-03-03 16:33:37.0 
+
@@ -0,0 +1,380 @@
+/*
+ *  linux/drivers/i2c/chips/ds1337.c
+ *
+ *  Copyright (C) 2005 James Chapman <[EMAIL PROTECTED]>
+ *
+ * based on linux/drivers/acron/char/pcf8583.c
+ *  Copyright (C) 2000 Russell King
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Driver for Dallas Semiconductor DS1337 real time clock chip
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include  /* get the user-level API */
+#include 
+#include 
+
+#define DS1337_NUM_REGS16
+#define DS1337_REG_CONFIG  2
+
+/* FIXME - how do we export these interface constants? */
+#define DS1337_GET_DATE0
+#define DS1337_SET_DATE1
+
+/*
+ * Functions declaration
+ */
+static unsigned short normal_i2c[] = { 0x68, I2C_CLIENT_END };
+static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
+
+SENSORS_INSMOD_1(ds1337);
+
+static int ds1337_attach_adapter(struct i2c_adapter *adapter);
+static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind);
+static void ds1337_init_client(struct i2c_client *client);
+static int ds1337_detach_client(struct i2c_client *client);
+static int ds1337_command(struct i2c_client *client, unsigned int cmd,
+ void *arg);
+
+/*
+ * Driver data (common to all clients)
+ */
+static struct i2c_driver ds1337_driver = {
+   .owner  = THIS_MODULE,
+   .name   = "ds1337",
+   .flags  = I2C_DF_NOTIFY,
+   .attach_adapter = ds1337_attach_adapter,
+   .detach_client  = ds1337_detach_client,
+   .command= ds1337_command,
+};
+
+/*
+ * Client data (each client gets its own)
+ */
+struct ds1337_data {
+   struct i2c_client client;
+   struct list_head list;
+   int id;
+};
+
+/*
+ * Internal variables
+ */
+static int ds1337_id;
+static LIST_HEAD(ds1337_clients);
+
+static inline int ds1337_read(struct i2c_client *client, u8 reg, u8 *value)
+{
+   s32 tmp = i2c_smbus_read_byte_data(client, reg);
+
+   if (tmp < 0)
+   return -EIO;
+
+   *value = tmp;
+
+   return 0;
+}
+
+/*
+ * Chip access functions
+ */
+static int ds1337_get_datetime(struct i2c_client *client, struct rtc_time *dt)
+{
+   struct ds1337_data *data = i2c_get_clientdata(client);
+   int result;
+   u8 buf[7];
+   u8 val;
+   struct i2c_msg msg[2];
+   u8 offs = 0;
+
+   if (!dt) {
+   dev_dbg(&client->adapter->dev, "%s: EINVAL: dt=NULL\n",
+   __FUNCTION__);
+
+   return -EINVAL;
+   }
+
+   msg[0].addr = client->addr;
+   msg[0].flags = 0;
+   msg[0].len = 1;
+   msg[0].buf = &offs;
+
+   msg[1].addr = client->addr;
+   msg[1].flags = I2C_M_RD;
+   msg[1].len = sizeof(buf);
+   msg[1].buf = &buf[0];
+
+   result = client->adapter->algo->master_xfer(client->adapter,
+   &msg[0], 2);
+
+   dev_dbg(&client->adapter->dev,
+   "%s: [%d] %02x %02x %02x %02x %02x %02x %02x\n",
+   __FUNCTION__, result, buf[0], buf[1], buf[2], buf[3],
+   buf[4], buf[5], buf[6]);
+
+   if (result >= 0) {
+   dt->tm_sec = BCD_TO_BIN(buf[0]);
+   dt->tm_min = BCD_TO_BIN(buf[1]);
+   val = buf[2] & 0x3f;
+   dt->tm_hour = BCD_TO_BIN(val);
+   dt->tm_wday = BCD_TO_BIN(buf[3]) - 1;
+   dt->tm_mday = BCD_TO_BIN(buf[4]);
+   val = buf[5] & 0x7f;
+   dt->tm_mon = BCD_TO_BIN(val);
+   dt->tm_year = 1900 + BCD_TO_BIN(buf[6]);
+   if (buf[5] & 0x80)
+   dt->tm_year += 100;
+
+   dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, "
+   "hours=%d, mday=%d, mon=%d, year=%d, wday=%d\n",
+   __FUNCTION__, dt->tm_sec, dt->tm_min,
+   dt->tm_hour, dt->tm_mday,
+   dt->tm_mon, dt->tm_year, dt->tm_wday);
+   } else {
+   dev_err(&client->adapter->dev, "ds1337[%d]: error reading "
+   "data! %d\n", data->id, result);
+   result = -EIO;
+   }
+
+   return result;
+}
+
+static int ds1337_set_datetime(struct i2c_client *client, struct rtc_time *dt)
+{
+   struct ds1337_data *data = i2c_get_clientdata(client);
+   int result;
+   u8 bu

Re: [PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

2005-03-02 Thread Andrew Morton
James Chapman <[EMAIL PROTECTED]> wrote:
>
> Add DS1337 RTC chip driver.
> 

drivers/i2c/chips/ds1337.c:60: `I2C_DRIVERID_DS1337' undeclared here (not in a 
function)


Also, there are changes in Greg's i2c tree which break your new driver:

drivers/i2c/chips/ds1337.c:60: initializer element is not constant
drivers/i2c/chips/ds1337.c:60: (near initialization for `ds1337_driver.id')
drivers/i2c/chips/ds1337.c: In function `ds1337_get_datetime':
drivers/i2c/chips/ds1337.c:155: structure has no member named `id'
drivers/i2c/chips/ds1337.c: In function `ds1337_set_datetime':
drivers/i2c/chips/ds1337.c:206: structure has no member named `id'
drivers/i2c/chips/ds1337.c: In function `ds1337_detect':
drivers/i2c/chips/ds1337.c:333: structure has no member named `id'
drivers/i2c/chips/ds1337.c:343: structure has no member named `id'

-
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: 2.6.11-rc5] i2c chips: ds1337 RTC driver

2005-03-02 Thread Greg KH
On Tue, Mar 01, 2005 at 07:21:56PM +, James Chapman wrote:
> Revised ds1337 chip driver patch.
> 
> Signed-off-by: James Chapman <[EMAIL PROTECTED]>
> 
> - change all driver log messages to use dev_dbg() or dev_err()
> - remove debug module parameter

Hm, doesn't seem to apply at all:

drivers/i2c/chips/Kconfig 1.78: 418 lines
patching file drivers/i2c/chips/Kconfig
drivers/i2c/chips/Makefile 1.61: 46 lines
patching file drivers/i2c/chips/Makefile
patching file drivers/i2c/chips/ds1337.c
patch:  malformed patch at line 420: 


Care to respin it again?

thanks,

greg k-h
-
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: 2.6.11-rc5] i2c chips: ds1337 RTC driver

2005-03-02 Thread Jean Delvare

Hi James,

> diff -Nru a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
> --- a/drivers/i2c/chips/Kconfig   2005-02-27 20:42:22 +00:00
> +++ b/drivers/i2c/chips/Kconfig   2005-02-27 20:42:22 +00:00
> @@ -62,6 +62,17 @@
> This driver can also be built as a module.  If so, the module
> will be called asb100.
> 
> +config SENSORS_DS1337
> + tristate "Dallas Semiconductor DS1337 Real Time Clock"
> + depends on I2C && EXPERIMENTAL
> + select I2C_SENSOR
> + help
> +   If you say yes here you get support for Dallas Semiconductor
> +   DS1337 real-time clock chips.
> +
> +   This driver can also be built as a module.  If so, the module
> +   will be called ds1337.
> +
>  config SENSORS_DS1621
>   tristate "Dallas Semiconductor DS1621 and DS1625"
>   depends on I2C && EXPERIMENTAL

I don't think it belongs there. The DS1337 is not a hardware monitoring
chip, please move it down to the "Other I2C Chip support" menu. I also
think I see an indentation issue on the "tristate" line, seemingly
copied from the SENSORS_DS1621 section which would need to be fixed as
well.

> +static struct i2c_driver ds1337_driver = {
> + .owner  = THIS_MODULE,
> + .name   = "ds1337",
> + .id = I2C_DRIVERID_DS1337,
> + .flags  = I2C_DF_NOTIFY,
> + .attach_adapter = ds1337_attach_adapter,
> + .detach_client  = ds1337_detach_client,
> + .command= ds1337_command,
> +};

I2C_DRIVERID_DS1337 isn't defined anywhere that I can see, so your
driver will hardly compile. You don't seem to actually need a driver
id, so you can simply omit it.

> +static inline int ds1337_read(struct i2c_client *client, u8 reg, u8
>   *value)
> +{
> + s32 tmp = i2c_smbus_read_byte_data(client, reg);
> +
> + if (tmp < 0)
> + return -EIO;
> +
> + *value = (u8) (tmp & 0xff);
> +
> + return 0;
> +}

The bitmasking is a no-op, and so is probably the cast as I'd expect the
compiler to do the right thing on its own.

> +static int ds1337_get_datetime(struct i2c_client *client, struct rtc_time >  
> *dt)
> +{
> (...)
> + if (!dt || !client) {

How could client be NULL? Same question for ds1337_set_datetime.

> + memset(buf, 0, sizeof(buf));

Why is this memset needed at all?

> + result = client->adapter->algo->master_xfer(client->adapter,
> + &msg[0],
> + 2);

You did not check that the adapter was capable of raw I2C transfers.

> +static int ds1337_attach_adapter(struct i2c_adapter *adapter)
> +{
> + if (!(adapter->class & I2C_CLASS_HWMON))
> + return 0;

You do not want to check that. The DS1337 is not a hardware monitoring
device!

> +static int ds1337_detect(struct i2c_adapter *adapter, int address, int
>   kind)
+{
+   struct i2c_client *new_client;
+   struct ds1337_data *data;
+   int err = 0;
+   const char *name = "";
+
+   if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+   goto exit;

You must verify the ability to do raw I2C transfers here, as said above.

+   err = -ENODEV;

Not correct. The detection function should NOT return a non-zero value
unless it meets a critical error (such as memory shortage). Just finding
a different device from what you were looking for isn't an error.

> + /* Check that control register bits 5-6 are zero */
> + if (buf[14] & ((1 << 5) | (1 << 6)))
> + goto exit_free;
> +
> + /* Check that status register bits 2-6 are zero */
> + if (buf[15] & ((1 << 2) | (1 << 3) | (1 << 4) |
> +(1 << 5) | (1 << 6)))
> + goto exit_free;

These would be much more readable as "& 0x60" and "& 0x7c" if you
want my opinion (but maybe it's just me). Also, I usually write "bits
6-2", not "2-6" (but again that might be just me).

> + new_client->id = ds1337_id++;

Client ids are not used by the core and were dropped recently. Don't use
that struct member.

> +static void ds1337_init_client(struct i2c_client *client)
> +{
> + u8 val;
> +
> + /* Ensure that device is set in 24-hour mode */
> + val = i2c_smbus_read_byte_data(client, 2);
> + i2c_smbus_write_byte_data(client, 2, val | (1 << 6));
> +}

You can probably spare a write if the device was already in 24-hour mode.
It might also be nice to give register 2 a name (DS1337_REG_CONFIG?) and
use that.

Also note that you are not handling errors properly here. You could get
-1 returned from the read, forcing all bits of the register to 1 on
subsequent write.

--
Jean Delvare
-
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: 2.6.11-rc5] i2c chips: ds1337 RTC driver

2005-03-01 Thread James Chapman
Revised ds1337 chip driver patch.
Signed-off-by: James Chapman <[EMAIL PROTECTED]>
- change all driver log messages to use dev_dbg() or dev_err()
- remove debug module parameter
Greg KH wrote:
On Mon, Feb 28, 2005 at 05:14:25PM +, James Chapman wrote:
+/* Define to compile in pr_debug() trace */
+#undef DEBUG

Not needed, we do this in the makefile now.

+   if (debug >= 1)
+   pr_debug("%s: client=%p, dt=%p\n", __FUNCTION__, client, dt);

Please use the dev_dbg(), dev_err() and friend functions instead of
pr_debug().  It provides a sane user interface that all of the other
drivers use.
thanks,
greg k-h
diff -Nru a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
--- a/drivers/i2c/chips/Kconfig 2005-02-27 20:42:22 +00:00
+++ b/drivers/i2c/chips/Kconfig 2005-02-27 20:42:22 +00:00
@@ -62,6 +62,17 @@
  This driver can also be built as a module.  If so, the module
  will be called asb100.
 
+config SENSORS_DS1337
+   tristate "Dallas Semiconductor DS1337 Real Time Clock"
+   depends on I2C && EXPERIMENTAL
+   select I2C_SENSOR
+   help
+ If you say yes here you get support for Dallas Semiconductor
+ DS1337 real-time clock chips. 
+
+ This driver can also be built as a module.  If so, the module
+ will be called ds1337.
+
 config SENSORS_DS1621
tristate "Dallas Semiconductor DS1621 and DS1625"
depends on I2C && EXPERIMENTAL
diff -Nru a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile
--- a/drivers/i2c/chips/Makefile2005-02-27 20:42:22 +00:00
+++ b/drivers/i2c/chips/Makefile2005-02-27 20:42:22 +00:00
@@ -11,6 +11,7 @@
 obj-$(CONFIG_SENSORS_ADM1025)  += adm1025.o
 obj-$(CONFIG_SENSORS_ADM1026)  += adm1026.o
 obj-$(CONFIG_SENSORS_ADM1031)  += adm1031.o
+obj-$(CONFIG_SENSORS_DS1337)   += ds1337.o
 obj-$(CONFIG_SENSORS_DS1621)   += ds1621.o
 obj-$(CONFIG_SENSORS_EEPROM)   += eeprom.o
 obj-$(CONFIG_SENSORS_FSCHER)   += fscher.o
diff -Nru a/drivers/i2c/chips/ds1337.c b/drivers/i2c/chips/ds1337.c
--- /dev/null   Wed Dec 31 16:00:00 196900
+++ b/drivers/i2c/chips/ds1337.c2005-02-27 20:42:22 +00:00
@@ -0,0 +1,384 @@
+/*
+ *  linux/drivers/i2c/chips/ds1337.c
+ *
+ *  Copyright (C) 2005 James Chapman <[EMAIL PROTECTED]>
+ *
+ * based on linux/drivers/acron/char/pcf8583.c
+ *  Copyright (C) 2000 Russell King
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Driver for Dallas Semiconductor DS1337 real time clock chip
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include  /* get the user-level API */
+#include 
+#include 
+
+#define DS1337_NUM_REGS16
+
+/* FIXME - how do we export these interface constants? */
+#define DS1337_GET_DATE0
+#define DS1337_SET_DATE1
+
+/*
+ * Functions declaration
+ */
+static unsigned short normal_i2c[] = { 0x68, I2C_CLIENT_END };
+static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
+
+SENSORS_INSMOD_1(ds1337);
+
+static int ds1337_attach_adapter(struct i2c_adapter *adapter);
+static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind);
+static void ds1337_init_client(struct i2c_client *client);
+static int ds1337_detach_client(struct i2c_client *client);
+static int ds1337_command(struct i2c_client *client, unsigned int cmd, void 
*arg);
+
+/*
+ * Driver data (common to all clients)
+ */
+static struct i2c_driver ds1337_driver = {
+   .owner  = THIS_MODULE,
+   .name   = "ds1337",
+   .id = I2C_DRIVERID_DS1337,
+   .flags  = I2C_DF_NOTIFY,
+   .attach_adapter = ds1337_attach_adapter,
+   .detach_client  = ds1337_detach_client,
+   .command= ds1337_command,
+};
+
+/*
+ * Client data (each client gets its own)
+ */
+struct ds1337_data {
+   struct i2c_client client;
+   struct list_head list;
+   int id;
+};
+
+/*
+ * Internal variables
+ */
+static int ds1337_id = 0;
+static LIST_HEAD(ds1337_clients);
+
+static inline int ds1337_read(struct i2c_client *client, u8 reg, u8 *value)
+{
+   s32 tmp = i2c_smbus_read_byte_data(client, reg);
+
+   if (tmp < 0)
+   return -EIO;
+
+   *value = (u8) (tmp & 0xff);
+
+   return 0;
+}
+
+/*
+ * Chip access functions
+ */
+static int ds1337_get_datetime(struct i2c_client *client, struct rtc_time *dt)
+{
+   int result;
+   u8 buf[7];
+   u8 val;
+   struct i2c_msg msg[2];
+   u8 offs = 0;
+
+   if (!dt || !client) {
+   dev_dbg(&client->adapter->dev, "%s: EINVAL: client=%p, 
dt=%p\n", 
+   __FUNCTION__, client, dt);
+
+   return -EINVAL;
+   }
+
+   memset(buf, 0, sizeof(buf));
+
+   msg[0].addr = client->addr;
+   msg[0].flags = 0;
+   msg[0].len = 1;
+   

Re: [PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

2005-03-01 Thread Greg KH
On Mon, Feb 28, 2005 at 05:14:25PM +, James Chapman wrote:
> +/* Define to compile in pr_debug() trace */
> +#undef DEBUG

Not needed, we do this in the makefile now.

> + if (debug >= 1)
> + pr_debug("%s: client=%p, dt=%p\n", __FUNCTION__, client, dt);

Please use the dev_dbg(), dev_err() and friend functions instead of
pr_debug().  It provides a sane user interface that all of the other
drivers use.

thanks,

greg k-h
-
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: 2.6.11-rc5] i2c chips: ds1337 RTC driver

2005-02-28 Thread James Chapman
Add DS1337 RTC chip driver.
Signed-off-by: James Chapman <[EMAIL PROTECTED]>

diff -Nru a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
--- a/drivers/i2c/chips/Kconfig 2005-02-27 20:42:22 +00:00
+++ b/drivers/i2c/chips/Kconfig 2005-02-27 20:42:22 +00:00
@@ -62,6 +62,17 @@
  This driver can also be built as a module.  If so, the module
  will be called asb100.
 
+config SENSORS_DS1337
+   tristate "Dallas Semiconductor DS1337 Real Time Clock"
+   depends on I2C && EXPERIMENTAL
+   select I2C_SENSOR
+   help
+ If you say yes here you get support for Dallas Semiconductor
+ DS1337 real-time clock chips. 
+
+ This driver can also be built as a module.  If so, the module
+ will be called ds1337.
+
 config SENSORS_DS1621
tristate "Dallas Semiconductor DS1621 and DS1625"
depends on I2C && EXPERIMENTAL
diff -Nru a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile
--- a/drivers/i2c/chips/Makefile2005-02-27 20:42:22 +00:00
+++ b/drivers/i2c/chips/Makefile2005-02-27 20:42:22 +00:00
@@ -11,6 +11,7 @@
 obj-$(CONFIG_SENSORS_ADM1025)  += adm1025.o
 obj-$(CONFIG_SENSORS_ADM1026)  += adm1026.o
 obj-$(CONFIG_SENSORS_ADM1031)  += adm1031.o
+obj-$(CONFIG_SENSORS_DS1337)   += ds1337.o
 obj-$(CONFIG_SENSORS_DS1621)   += ds1621.o
 obj-$(CONFIG_SENSORS_EEPROM)   += eeprom.o
 obj-$(CONFIG_SENSORS_FSCHER)   += fscher.o
diff -Nru a/drivers/i2c/chips/ds1337.c b/drivers/i2c/chips/ds1337.c
--- /dev/null   Wed Dec 31 16:00:00 196900
+++ b/drivers/i2c/chips/ds1337.c2005-02-27 20:42:22 +00:00
@@ -0,0 +1,394 @@
+/*
+ *  linux/drivers/i2c/chips/ds1337.c
+ *
+ *  Copyright (C) 2005 James Chapman <[EMAIL PROTECTED]>
+ *
+ * based on linux/drivers/acron/char/pcf8583.c
+ *  Copyright (C) 2000 Russell King
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Driver for Dallas Semiconductor DS1337 real time clock chip
+ */
+
+/* Define to compile in pr_debug() trace */
+#undef DEBUG
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include  /* get the user-level API */
+#include 
+#include 
+
+#define DS1337_NUM_REGS16
+
+/* FIXME - how do we export these interface constants? */
+#define DS1337_GET_DATE0
+#define DS1337_SET_DATE1
+
+static int debug = 0;
+module_param(debug, int, S_IRUGO | S_IWUSR);
+
+/*
+ * Functions declaration
+ */
+static unsigned short normal_i2c[] = { 0x68, I2C_CLIENT_END };
+static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
+
+SENSORS_INSMOD_1(ds1337);
+
+static int ds1337_attach_adapter(struct i2c_adapter *adapter);
+static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind);
+static void ds1337_init_client(struct i2c_client *client);
+static int ds1337_detach_client(struct i2c_client *client);
+static int ds1337_command(struct i2c_client *client, unsigned int cmd, void 
*arg);
+
+/*
+ * Driver data (common to all clients)
+ */
+static struct i2c_driver ds1337_driver = {
+   .owner  = THIS_MODULE,
+   .name   = "ds1337",
+   .id = I2C_DRIVERID_DS1337,
+   .flags  = I2C_DF_NOTIFY,
+   .attach_adapter = ds1337_attach_adapter,
+   .detach_client  = ds1337_detach_client,
+   .command= ds1337_command,
+};
+
+/*
+ * Client data (each client gets its own)
+ */
+struct ds1337_data {
+   struct i2c_client client;
+   struct list_head list;
+   int id;
+};
+
+/*
+ * Internal variables
+ */
+static int ds1337_id = 0;
+static LIST_HEAD(ds1337_clients);
+
+static inline int ds1337_read(struct i2c_client *client, u8 reg, u8 *value)
+{
+   s32 tmp = i2c_smbus_read_byte_data(client, reg);
+
+   if (tmp < 0)
+   return -EIO;
+
+   *value = (u8) (tmp & 0xff);
+
+   return 0;
+}
+
+/*
+ * Chip access functions
+ */
+static int ds1337_get_datetime(struct i2c_client *client, struct rtc_time *dt)
+{
+   int result;
+   u8 buf[7];
+   u8 val;
+   struct i2c_msg msg[2];
+   u8 offs = 0;
+
+   if (debug >= 1)
+   pr_debug("%s: client=%p, dt=%p\n", __FUNCTION__, client, dt);
+
+   if (!dt || !client)
+   return -EINVAL;
+
+   memset(buf, 0, sizeof(buf));
+
+   msg[0].addr = client->addr;
+   msg[0].flags = 0;
+   msg[0].len = 1;
+   msg[0].buf = &offs;
+
+   msg[1].addr = client->addr;
+   msg[1].flags = I2C_M_RD;
+   msg[1].len = sizeof(buf);
+   msg[1].buf = &buf[0];
+
+   result = client->adapter->algo->master_xfer(client->adapter,
+   &msg[0],
+   2);
+
+   if (debug >= 2)
+   pr_debug("%s: [%d] %02x %02x %02x %02x %02x %02x %02x\n",
+