Hi,
Here is a I2C update for 2.6.12-rc1. It includes a lot of bugfixes and
documenation updates. It also has two new i2c drivers. All of these
patches have been in the past few -mm releases.
Please pull from:
bk://kernel.bkbits.net/gregkh/linux/i2c-2.6
Patches will be posted to linux-kernel and sensors as a follow-up thread
for those who want to see them.
thanks,
greg k-h
Documentation/i2c/i2c-parport | 156 ----------
Documentation/i2c/busses/i2c-ali1535 | 42 ++
Documentation/i2c/busses/i2c-ali1563 | 27 +
Documentation/i2c/busses/i2c-ali15x3 | 112 +++++++
Documentation/i2c/busses/i2c-amd756 | 25 +
Documentation/i2c/busses/i2c-amd8111 | 41 ++
Documentation/i2c/busses/i2c-i801 | 80 +++++
Documentation/i2c/busses/i2c-i810 | 46 +++
Documentation/i2c/busses/i2c-nforce2 | 41 ++
Documentation/i2c/busses/i2c-parport | 174 +++++++++++
Documentation/i2c/busses/i2c-parport-light | 11
Documentation/i2c/busses/i2c-pca-isa | 23 +
Documentation/i2c/busses/i2c-piix4 | 72 ++++
Documentation/i2c/busses/i2c-prosavage | 23 +
Documentation/i2c/busses/i2c-savage4 | 26 +
Documentation/i2c/busses/i2c-sis5595 | 59 ++++
Documentation/i2c/busses/i2c-sis630 | 49 +++
Documentation/i2c/busses/i2c-sis69x | 73 +++++
Documentation/i2c/busses/i2c-via | 34 ++
Documentation/i2c/busses/i2c-viapro | 47 +++
Documentation/i2c/busses/i2c-voodoo3 | 62 ++++
Documentation/i2c/busses/scx200_acb | 14
drivers/i2c/algos/i2c-algo-ite.c | 13
drivers/i2c/algos/i2c-algo-pcf.c | 44 ++-
drivers/i2c/algos/i2c-algo-sibyte.c | 13
drivers/i2c/busses/Kconfig | 38 +-
drivers/i2c/busses/i2c-elektor.c | 9
drivers/i2c/busses/i2c-ibm_iic.c | 4
drivers/i2c/busses/i2c-ite.c | 7
drivers/i2c/busses/i2c-mv64xxx.c | 2
drivers/i2c/busses/i2c-s3c2410.c | 15 -
drivers/i2c/busses/i2c-viapro.c | 17 -
drivers/i2c/chips/Kconfig | 25 +
drivers/i2c/chips/Makefile | 2
drivers/i2c/chips/adm1021.c | 14
drivers/i2c/chips/ds1337.c | 402 +++++++++++++++++++++++++++
drivers/i2c/chips/eeprom.c | 3
drivers/i2c/chips/it87.c | 10
drivers/i2c/chips/lm85.c | 104 +++++--
drivers/i2c/chips/lm87.c | 20 -
drivers/i2c/chips/lm90.c | 44 ++-
drivers/i2c/chips/lm92.c | 423 +++++++++++++++++++++++++++++
drivers/i2c/chips/m41t00.c | 1
drivers/i2c/chips/w83627hf.c | 5
drivers/i2c/chips/w83781d.c | 100 ------
drivers/i2c/i2c-core.c | 10
include/linux/i2c.h | 13
47 files changed, 2175 insertions(+), 400 deletions(-)
-----
<frank.beesley:aeroflex.com>:
o I2C: Clean up of i2c-elektor.c build
<grant_nospam:dodo.com.au>:
o I2C: Drop useless w83781d RT feature
o I2C: group Intel on I2C Hardware Bus support
Domen Puncer:
o i2c/i2c-elektor: remove interruptible_sleep_on_timeout() usage
o i2c/i2c-ite: remove interruptible_sleep_on_timeout() usage
Eric Brower:
o I2C: lost arbitration detection for PCF8584
James Chapman:
o i2c: add adt7461 chip support to lm90 driver
o i2c: new driver for ds1337 RTC
Jean Delvare:
o I2C: Fix indentation of lm87 driver
o I2C: Fix broken force parameter handling
o i2c: add adt7461 chip support to lm90 driver's Kconfig entry
o I2C: i2c-s3c2410 functionality and fixes
o I2C: Fix race condition in it87 driver
o I2C: Delete useless instruction in it87
o I2C: Fix Vaio EEPROM detection
o I2C: Recognize new revision of the ADT7463 chip
o I2C: Avoid repeated resets of i2c-viapro
o I2C: Kill outdated defines in i2c.h
o I2C: Fix some i2c algorithm initialization
o I2C: Skip broken detection step in it87
o I2C: Make master_xfer debug messages more useful
o I2C: Kill unused struct members in w83627hf driver
o I2C: Fix adm1021 alarms mask
o I2C: Cleanup adm1021 unused defines
o I2C: New lm92 chip driver
Mark A. Greer:
o i2c: i2c-mv64xxx - set adapter owner and class fields
o I2C: Fix breakage in m41t00 i2c rtc driver
Rafael Ávila de Espíndola:
o I2C: lsb in emc6d102 and adm1027
Rudolf Marek:
o I2C: busses documentation update 2 of 2
o I2C: busses documentation update 1 of 2
ChangeSet 1.2350, 2005/03/31 14:32:55-08:00, [email protected]
[PATCH] I2C: Drop useless w83781d RT feature
This patch removes useless RT feature from w83781d driver.
Patch applies after the recent "I2C: Fix a common race condition
in hardware monitoring" series.
Signed-off-by: Grant Coady <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/w83781d.c | 100 --------------------------------------------
1 files changed, 100 deletions(-)
diff -Nru a/drivers/i2c/chips/w83781d.c b/drivers/i2c/chips/w83781d.c
--- a/drivers/i2c/chips/w83781d.c 2005-03-31 15:15:56 -08:00
+++ b/drivers/i2c/chips/w83781d.c 2005-03-31 15:15:56 -08:00
@@ -46,9 +46,6 @@
#include <asm/io.h>
#include "lm75.h"
-/* RT Table support #defined so we can take it out if it gets bothersome */
-#define W83781D_RT 1
-
/* Addresses to scan */
static unsigned short normal_i2c[] = { 0x20, 0x21, 0x22, 0x23, 0x24, 0x25,
0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b,
@@ -258,9 +255,6 @@
3000-5000 = thermistor beta.
Default = 3435.
Other Betas unimplemented */
-#ifdef W83781D_RT
- u8 rt[3][32]; /* Register value */
-#endif
u8 vrm;
};
@@ -834,66 +828,6 @@
device_create_file(&client->dev, &dev_attr_temp##offset##_type); \
} while (0)
-#ifdef W83781D_RT
-static ssize_t
-show_rt_reg(struct device *dev, char *buf, int nr)
-{
- struct w83781d_data *data = w83781d_update_device(dev);
- int i, j = 0;
-
- for (i = 0; i < 32; i++) {
- if (i > 0)
- j += sprintf(buf, " %ld", (long) data->rt[nr - 1][i]);
- else
- j += sprintf(buf, "%ld", (long) data->rt[nr - 1][i]);
- }
- j += sprintf(buf, "\n");
-
- return j;
-}
-
-static ssize_t
-store_rt_reg(struct device *dev, const char *buf, size_t count, int nr)
-{
- struct i2c_client *client = to_i2c_client(dev);
- struct w83781d_data *data = i2c_get_clientdata(client);
- u32 val, i;
-
- for (i = 0; i < count; i++) {
- val = simple_strtoul(buf + count, NULL, 10);
-
- /* fixme: no bounds checking 0-255 */
- data->rt[nr - 1][i] = val & 0xff;
- w83781d_write_value(client, W83781D_REG_RT_IDX, i);
- w83781d_write_value(client, W83781D_REG_RT_VAL,
- data->rt[nr - 1][i]);
- }
-
- return count;
-}
-
-#define sysfs_rt(offset) \
-static ssize_t show_regs_rt_##offset (struct device *dev, char *buf) \
-{ \
- return show_rt_reg(dev, buf, offset); \
-} \
-static ssize_t store_regs_rt_##offset (struct device *dev, const char *buf, size_t count) \
-{ \
- return store_rt_reg(dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(rt##offset, S_IRUGO | S_IWUSR, show_regs_rt_##offset, store_regs_rt_##offset);
-
-sysfs_rt(1);
-sysfs_rt(2);
-sysfs_rt(3);
-
-#define device_create_file_rt(client, offset) \
-do { \
-device_create_file(&client->dev, &dev_attr_rt##offset); \
-} while (0)
-
-#endif /* ifdef W83781D_RT */
-
/* This function is called when:
* w83781d_driver is inserted (when this module is loaded), for each
available adapter
@@ -1304,13 +1238,6 @@
if (kind != w83783s && kind != w83697hf)
device_create_file_sensor(new_client, 3);
}
-#ifdef W83781D_RT
- if (kind == w83781d) {
- device_create_file_rt(new_client, 1);
- device_create_file_rt(new_client, 2);
- device_create_file_rt(new_client, 3);
- }
-#endif
return 0;
@@ -1535,33 +1462,6 @@
break;
}
}
-#ifdef W83781D_RT
-/*
- Fill up the RT Tables.
- We assume that they are 32 bytes long, in order for temp 1-3.
- Data sheet documentation is sparse.
- We also assume that it is only for the 781D although I suspect
- that the others support it as well....
-*/
-
- if (init && type == w83781d) {
- u16 k = 0;
-/*
- Auto-indexing doesn't seem to work...
- w83781d_write_value(client,W83781D_REG_RT_IDX,0);
-*/
- for (i = 0; i < 3; i++) {
- int j;
- for (j = 0; j < 32; j++) {
- w83781d_write_value(client,
- W83781D_REG_RT_IDX, k++);
- data->rt[i][j] =
- w83781d_read_value(client,
- W83781D_REG_RT_VAL);
- }
- }
- }
-#endif /* W83781D_RT */
if (init && type != as99127f) {
/* Enable temp2 */
ChangeSet 1.2342, 2005/03/31 14:30:24-08:00, [email protected]
[PATCH] I2C: lost arbitration detection for PCF8584
[PATCH] lost arbitration detection for PCF8584 algo driver
Patch against a slightly-dated linux-2.6 BK tree
This patch provides lost arbitration detection for the PCF8584
I2C algorithm driver. The PCF8584 LAB bit is set whenever lost
arbitration is detected, so we check the bit in the wait_for_pin
function and if LAB is detected we return -EINTR. The -EINTR
value bubbles-up all the way to the master_xfer API call so
callers may detect this condition explicitly. LAB could be checked
more often, at the expense of code readability/maintainability.
Signed-off-by: Eric Brower <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/algos/i2c-algo-pcf.c | 44 ++++++++++++++++++++++++++++++++++++---
1 files changed, 41 insertions(+), 3 deletions(-)
diff -Nru a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
--- a/drivers/i2c/algos/i2c-algo-pcf.c 2005-03-31 15:16:56 -08:00
+++ b/drivers/i2c/algos/i2c-algo-pcf.c 2005-03-31 15:16:56 -08:00
@@ -78,7 +78,6 @@
set_pcf(adap, 1, I2C_PCF_STOP);
}
-
static int wait_for_bb(struct i2c_algo_pcf_data *adap) {
int timeout = DEF_TIMEOUT;
@@ -109,6 +108,26 @@
adap->waitforpin();
*status = get_pcf(adap, 1);
}
+ if (*status & I2C_PCF_LAB) {
+ DEB2(printk(KERN_INFO
+ "i2c-algo-pcf.o: lost arbitration (CSR 0x%02x)\n",
+ *status));
+ /* Cleanup from LAB-- reset and enable ESO.
+ * This resets the PCF8584; since we've lost the bus, no
+ * further attempts should be made by callers to clean up
+ * (no i2c_stop() etc.)
+ */
+ set_pcf(adap, 1, I2C_PCF_PIN);
+ set_pcf(adap, 1, I2C_PCF_ESO);
+ /* TODO: we should pause for a time period sufficient for any
+ * running I2C transaction to complete-- the arbitration
+ * logic won't work properly until the next START is seen.
+ */
+ DEB2(printk(KERN_INFO
+ "i2c-algo-pcf.o: reset LAB condition (CSR 0x%02x)\n",
+ get_pcf(adap,1)));
+ return(-EINTR);
+ }
#endif
if (timeout <= 0)
return(-1);
@@ -188,16 +207,22 @@
unsigned char addr, int retries)
{
int i, status, ret = -1;
+ int wfp;
for (i=0;i<retries;i++) {
i2c_outb(adap, addr);
i2c_start(adap);
status = get_pcf(adap, 1);
- if (wait_for_pin(adap, &status) >= 0) {
+ if ((wfp = wait_for_pin(adap, &status)) >= 0) {
if ((status & I2C_PCF_LRB) == 0) {
i2c_stop(adap);
break; /* success! */
}
}
+ if (wfp == -EINTR) {
+ /* arbitration lost */
+ udelay(adap->udelay);
+ return -EINTR;
+ }
i2c_stop(adap);
udelay(adap->udelay);
}
@@ -219,6 +244,10 @@
i2c_outb(adap, buf[wrcount]);
timeout = wait_for_pin(adap, &status);
if (timeout) {
+ if (timeout == -EINTR) {
+ /* arbitration lost */
+ return -EINTR;
+ }
i2c_stop(adap);
dev_err(&i2c_adap->dev, "i2c_write: error - timeout.\n");
return -EREMOTEIO; /* got a better one ?? */
@@ -247,11 +276,16 @@
{
int i, status;
struct i2c_algo_pcf_data *adap = i2c_adap->algo_data;
+ int wfp;
/* increment number of bytes to read by one -- read dummy byte */
for (i = 0; i <= count; i++) {
- if (wait_for_pin(adap, &status)) {
+ if ((wfp = wait_for_pin(adap, &status))) {
+ if (wfp == -EINTR) {
+ /* arbitration lost */
+ return -EINTR;
+ }
i2c_stop(adap);
dev_err(&i2c_adap->dev, "pcf_readbytes timed out.\n");
return (-1);
@@ -366,6 +400,10 @@
/* Wait for PIN (pending interrupt NOT) */
timeout = wait_for_pin(adap, &status);
if (timeout) {
+ if (timeout == -EINTR) {
+ /* arbitration lost */
+ return (-EINTR);
+ }
i2c_stop(adap);
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: Timeout waiting "
"for PIN(1) in pcf_xfer\n");)
ChangeSet 1.2344, 2005/03/31 14:31:02-08:00, [email protected]
[PATCH] I2C: Delete useless instruction in it87
The IT8705F doesn't support VID, so it's quite pointless to give a value
to it (and an arbitrary one at that). I think that this instruction was
there for compatibility reasons some times ago, but the reasons went
away while the instruction was left in place. We can safely delete it
now.
Thanks to Rudolf Marek for testing the patch (you never know).
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/it87.c | 3 ---
1 files changed, 3 deletions(-)
diff -Nru a/drivers/i2c/chips/it87.c b/drivers/i2c/chips/it87.c
--- a/drivers/i2c/chips/it87.c 2005-03-31 15:16:38 -08:00
+++ b/drivers/i2c/chips/it87.c 2005-03-31 15:16:38 -08:00
@@ -1122,9 +1122,6 @@
it87_read_value(client, IT87_REG_TEMP_LOW(i));
}
- /* The 8705 does not have VID capability */
- data->vid = 0x1f;
-
i = it87_read_value(client, IT87_REG_FAN_DIV);
data->fan_div[0] = i & 0x07;
data->fan_div[1] = (i >> 3) & 0x07;
ChangeSet 1.2345, 2005/03/31 14:31:21-08:00, [email protected]
[PATCH] I2C: Fix race condition in it87 driver
I noticed a race condition in the it87, affecting the IT8712F chip only.
The VRM value is currently initialized *after* creating the vrm and vid
sysfs files. This leaves a theorical room for reading from these files
and get an invalid value. It's not critical, but let's still fix it.
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/it87.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
diff -Nru a/drivers/i2c/chips/it87.c b/drivers/i2c/chips/it87.c
--- a/drivers/i2c/chips/it87.c 2005-03-31 15:16:31 -08:00
+++ b/drivers/i2c/chips/it87.c 2005-03-31 15:16:31 -08:00
@@ -889,9 +889,9 @@
}
if (data->type == it8712) {
+ data->vrm = i2c_which_vrm();
device_create_file_vrm(new_client);
device_create_file_vid(new_client);
- data->vrm = i2c_which_vrm();
}
return 0;
ChangeSet 1.2336, 2005/03/31 14:09:38-08:00, [email protected]
[PATCH] I2C: Kill outdated defines in i2c.h
Some defines in i2c.h (I2C_CLIENT_MODPARM and friends) are now useless.
They should have been removed when the i2c client parameters were
converted from MODULE_PARAM to module_parm_array, but where not. This
patch removes them now.
Additionally, it moves the definition of I2C_CLIENT_MAX_OPTS next to
where it is used rather than 220 lines before, which is preferable IMHO.
As a side note, I think that there is a bug in the way these options are
handled. The i2c code looks for I2C_CLIENT_END as a list terminator, but
if the maximum number of parameters are actually provided, no terminator
will be left. It's rather unlikely to happen because nobody will
probably ever provide that many parameters, but this should probably be
fixed. I'll address this issue later, since I plan to completely rewrite
the way these parameters are handled anyway.
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
include/linux/i2c.h | 13 +++----------
1 files changed, 3 insertions(+), 10 deletions(-)
diff -Nru a/include/linux/i2c.h b/include/linux/i2c.h
--- a/include/linux/i2c.h 2005-03-31 15:17:39 -08:00
+++ b/include/linux/i2c.h 2005-03-31 15:17:39 -08:00
@@ -306,9 +306,6 @@
#define ANY_I2C_BUS 0xffff
#define ANY_I2C_ISA_BUS 9191
-/* The length of the option lists */
-#define I2C_CLIENT_MAX_OPTS 48
-
/* ----- functions exported by i2c.o */
@@ -526,6 +523,9 @@
#define I2C_MAJOR 89 /* Device major number */
/* These defines are used for probing i2c client addresses */
+/* The length of the option lists */
+#define I2C_CLIENT_MAX_OPTS 48
+
/* Default fill of many variables */
#define I2C_CLIENT_DEFAULTS {I2C_CLIENT_END, I2C_CLIENT_END, I2C_CLIENT_END, \
I2C_CLIENT_END, I2C_CLIENT_END, I2C_CLIENT_END, \
@@ -544,19 +544,12 @@
I2C_CLIENT_END, I2C_CLIENT_END, I2C_CLIENT_END, \
I2C_CLIENT_END, I2C_CLIENT_END, I2C_CLIENT_END}
-/* This is ugly. We need to evaluate I2C_CLIENT_MAX_OPTS before it is
- stringified */
-#define I2C_CLIENT_MODPARM_AUX1(x) "1-" #x "h"
-#define I2C_CLIENT_MODPARM_AUX(x) I2C_CLIENT_MODPARM_AUX1(x)
-#define I2C_CLIENT_MODPARM I2C_CLIENT_MODPARM_AUX(I2C_CLIENT_MAX_OPTS)
-
/* I2C_CLIENT_MODULE_PARM creates a module parameter, and puts it in the
module header */
#define I2C_CLIENT_MODULE_PARM(var,desc) \
static unsigned short var[I2C_CLIENT_MAX_OPTS] = I2C_CLIENT_DEFAULTS; \
static unsigned int var##_num; \
- /*MODULE_PARM(var,I2C_CLIENT_MODPARM);*/ \
module_param_array(var, short, &var##_num, 0); \
MODULE_PARM_DESC(var,desc)
ChangeSet 1.2331, 2005/03/31 14:08:02-08:00, [email protected]
[PATCH] i2c: new driver for ds1337 RTC
Signed-off-by: James Chapman <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/Kconfig | 11 +
drivers/i2c/chips/Makefile | 1
drivers/i2c/chips/ds1337.c | 402 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 414 insertions(+)
diff -Nru a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
--- a/drivers/i2c/chips/Kconfig 2005-03-31 15:18:15 -08:00
+++ b/drivers/i2c/chips/Kconfig 2005-03-31 15:18:15 -08:00
@@ -362,6 +362,17 @@
menu "Other I2C Chip support"
depends on I2C
+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_EEPROM
tristate "EEPROM reader"
depends on I2C && EXPERIMENTAL
diff -Nru a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile
--- a/drivers/i2c/chips/Makefile 2005-03-31 15:18:15 -08:00
+++ b/drivers/i2c/chips/Makefile 2005-03-31 15:18:15 -08: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.c 2005-03-31 15:18:15 -08:00
@@ -0,0 +1,402 @@
+/*
+ * 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 <linux/config.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/i2c-sensor.h>
+#include <linux/string.h>
+#include <linux/rtc.h> /* get the user-level API */
+#include <linux/bcd.h>
+#include <linux/list.h>
+
+/* Device registers */
+#define DS1337_REG_HOUR 2
+#define DS1337_REG_DAY 3
+#define DS1337_REG_DATE 4
+#define DS1337_REG_MONTH 5
+#define DS1337_REG_CONTROL 14
+#define DS1337_REG_STATUS 15
+
+/* FIXME - how do we export these interface constants? */
+#define DS1337_GET_DATE 0
+#define DS1337_SET_DATE 1
+
+/*
+ * 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 buf[8];
+ u8 val;
+ struct i2c_msg msg[1];
+
+ if (!dt) {
+ dev_dbg(&client->adapter->dev, "%s: EINVAL: dt=NULL\n",
+ __FUNCTION__);
+
+ return -EINVAL;
+ }
+
+ 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);
+
+ buf[0] = 0; /* reg offset */
+ buf[1] = BIN_TO_BCD(dt->tm_sec);
+ buf[2] = BIN_TO_BCD(dt->tm_min);
+ buf[3] = BIN_TO_BCD(dt->tm_hour) | (1 << 6);
+ buf[4] = BIN_TO_BCD(dt->tm_wday) + 1;
+ buf[5] = BIN_TO_BCD(dt->tm_mday);
+ buf[6] = BIN_TO_BCD(dt->tm_mon);
+ if (dt->tm_year >= 2000) {
+ val = dt->tm_year - 2000;
+ buf[6] |= (1 << 7);
+ } else {
+ val = dt->tm_year - 1900;
+ }
+ buf[7] = BIN_TO_BCD(val);
+
+ msg[0].addr = client->addr;
+ msg[0].flags = 0;
+ msg[0].len = sizeof(buf);
+ msg[0].buf = &buf[0];
+
+ result = client->adapter->algo->master_xfer(client->adapter,
+ &msg[0], 1);
+ if (result < 0) {
+ dev_err(&client->adapter->dev, "ds1337[%d]: error "
+ "writing data! %d\n", data->id, result);
+ result = -EIO;
+ } else {
+ result = 0;
+ }
+
+ return result;
+}
+
+static int ds1337_command(struct i2c_client *client, unsigned int cmd,
+ void *arg)
+{
+ dev_dbg(&client->adapter->dev, "%s: cmd=%d\n", __FUNCTION__, cmd);
+
+ switch (cmd) {
+ case DS1337_GET_DATE:
+ return ds1337_get_datetime(client, arg);
+
+ case DS1337_SET_DATE:
+ return ds1337_set_datetime(client, arg);
+
+ default:
+ return -EINVAL;
+ }
+}
+
+/*
+ * Public API for access to specific device. Useful for low-level
+ * RTC access from kernel code.
+ */
+int ds1337_do_command(int id, int cmd, void *arg)
+{
+ struct list_head *walk;
+ struct list_head *tmp;
+ struct ds1337_data *data;
+
+ list_for_each_safe(walk, tmp, &ds1337_clients) {
+ data = list_entry(walk, struct ds1337_data, list);
+ if (data->id == id)
+ return ds1337_command(&data->client, cmd, arg);
+ }
+
+ return -ENODEV;
+}
+
+static int ds1337_attach_adapter(struct i2c_adapter *adapter)
+{
+ return i2c_detect(adapter, &addr_data, ds1337_detect);
+}
+
+/*
+ * The following function does more than just detection. If detection
+ * succeeds, it also registers the new chip.
+ */
+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 |
+ I2C_FUNC_I2C))
+ goto exit;
+
+ if (!(data = kmalloc(sizeof(struct ds1337_data), GFP_KERNEL))) {
+ err = -ENOMEM;
+ goto exit;
+ }
+ memset(data, 0, sizeof(struct ds1337_data));
+ INIT_LIST_HEAD(&data->list);
+
+ /* The common I2C client data is placed right before the
+ * DS1337-specific data.
+ */
+ new_client = &data->client;
+ i2c_set_clientdata(new_client, data);
+ new_client->addr = address;
+ new_client->adapter = adapter;
+ new_client->driver = &ds1337_driver;
+ new_client->flags = 0;
+
+ /*
+ * Now we do the remaining detection. A negative kind means that
+ * the driver was loaded with no force parameter (default), so we
+ * must both detect and identify the chip. A zero kind means that
+ * the driver was loaded with the force parameter, the detection
+ * step shall be skipped. A positive kind means that the driver
+ * was loaded with the force parameter and a given kind of chip is
+ * requested, so both the detection and the identification steps
+ * are skipped.
+ *
+ * For detection, we read registers that are most likely to cause
+ * detection failure, i.e. those that have more bits with fixed
+ * or reserved values.
+ */
+
+ /* Default to an DS1337 if forced */
+ if (kind == 0)
+ kind = ds1337;
+
+ if (kind < 0) { /* detection and identification */
+ u8 data;
+
+ /* Check that status register bits 6-2 are zero */
+ if ((ds1337_read(new_client, DS1337_REG_STATUS, &data) < 0) ||
+ (data & 0x7c))
+ goto exit_free;
+
+ /* Check for a valid day register value */
+ if ((ds1337_read(new_client, DS1337_REG_DAY, &data) < 0) ||
+ (data == 0) || (data & 0xf8))
+ goto exit_free;
+
+ /* Check for a valid date register value */
+ if ((ds1337_read(new_client, DS1337_REG_DATE, &data) < 0) ||
+ (data == 0) || (data & 0xc0) || ((data & 0x0f) > 9) ||
+ (data >= 0x32))
+ goto exit_free;
+
+ /* Check for a valid month register value */
+ if ((ds1337_read(new_client, DS1337_REG_MONTH, &data) < 0) ||
+ (data == 0) || (data & 0x60) || ((data & 0x0f) > 9) ||
+ ((data >= 0x13) && (data <= 0x19)))
+ goto exit_free;
+
+ /* Check that control register bits 6-5 are zero */
+ if ((ds1337_read(new_client, DS1337_REG_CONTROL, &data) < 0) ||
+ (data & 0x60))
+ goto exit_free;
+
+ kind = ds1337;
+ }
+
+ if (kind == ds1337)
+ name = "ds1337";
+
+ /* We can fill in the remaining client fields */
+ strlcpy(new_client->name, name, I2C_NAME_SIZE);
+
+ /* Tell the I2C layer a new client has arrived */
+ if ((err = i2c_attach_client(new_client)))
+ goto exit_free;
+
+ /* Initialize the DS1337 chip */
+ ds1337_init_client(new_client);
+
+ /* Add client to local list */
+ data->id = ds1337_id++;
+ list_add(&data->list, &ds1337_clients);
+
+ return 0;
+
+exit_free:
+ kfree(data);
+exit:
+ return err;
+}
+
+static void ds1337_init_client(struct i2c_client *client)
+{
+ s32 val;
+
+ /* Ensure that device is set in 24-hour mode */
+ val = i2c_smbus_read_byte_data(client, DS1337_REG_HOUR);
+ if ((val >= 0) && (val & (1 << 6)) == 0)
+ i2c_smbus_write_byte_data(client, DS1337_REG_HOUR,
+ val | (1 << 6));
+}
+
+static int ds1337_detach_client(struct i2c_client *client)
+{
+ int err;
+ struct ds1337_data *data = i2c_get_clientdata(client);
+
+ if ((err = i2c_detach_client(client))) {
+ dev_err(&client->dev, "Client deregistration failed, "
+ "client not detached.\n");
+ return err;
+ }
+
+ list_del(&data->list);
+ kfree(data);
+ return 0;
+}
+
+static int __init ds1337_init(void)
+{
+ return i2c_add_driver(&ds1337_driver);
+}
+
+static void __exit ds1337_exit(void)
+{
+ i2c_del_driver(&ds1337_driver);
+}
+
+MODULE_AUTHOR("James Chapman <[email protected]>");
+MODULE_DESCRIPTION("DS1337 RTC driver");
+MODULE_LICENSE("GPL");
+
+module_init(ds1337_init);
+module_exit(ds1337_exit);
ChangeSet 1.2338, 2005/03/31 14:29:05-08:00, [email protected]
[PATCH] I2C: Recognize new revision of the ADT7463 chip
This simple patch to the lm85 driver adds recognition of a new revision
of the ADT7463 chip.
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/lm85.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletion(-)
diff -Nru a/drivers/i2c/chips/lm85.c b/drivers/i2c/chips/lm85.c
--- a/drivers/i2c/chips/lm85.c 2005-03-31 15:17:25 -08:00
+++ b/drivers/i2c/chips/lm85.c 2005-03-31 15:17:25 -08:00
@@ -74,6 +74,7 @@
#define LM85_VERSTEP_LM85B 0x62
#define LM85_VERSTEP_ADM1027 0x60
#define LM85_VERSTEP_ADT7463 0x62
+#define LM85_VERSTEP_ADT7463C 0x6A
#define LM85_VERSTEP_EMC6D100_A0 0x60
#define LM85_VERSTEP_EMC6D100_A1 0x61
@@ -1089,7 +1090,8 @@
&& verstep == LM85_VERSTEP_ADM1027 ) {
kind = adm1027 ;
} else if( company == LM85_COMPANY_ANALOG_DEV
- && verstep == LM85_VERSTEP_ADT7463 ) {
+ && (verstep == LM85_VERSTEP_ADT7463
+ || verstep == LM85_VERSTEP_ADT7463C) ) {
kind = adt7463 ;
} else if( company == LM85_COMPANY_ANALOG_DEV
&& (verstep & LM85_VERSTEP_VMASK) == LM85_VERSTEP_GENERIC ) {
ChangeSet 1.2337, 2005/03/31 14:28:46-08:00, [email protected]
[PATCH] I2C: Avoid repeated resets of i2c-viapro
It was reported that the i2c-viapro SMBus driver sometimes has trouble
on recent systems (VT8237 south bridge). The "Host Status" register has
at least one additional bit used when compared with older south bridges
of this family. The driver currently considers this additional bit as an
error condition when it's set, causing repeated bus resets and sometimes
read failures.
This patch makes the driver ignore the bits of the "Host Status"
register for which no definition is known. I wish I had a datasheet for
the VIA VT8237, so that I could check what the additional bit is
supposed to mean, but I don't. If someone has a datasheet or good
contacts at VIA, please let me know.
The patch was reported to fix the problem on a system with the VT8237,
and was also tested not to break the driver on older VIA south bridges,
so it seems to be safe. Thanks to Aurelien Jarno for the tests.
Additionally, the patch makes the post-transaction bus reset slightly
more efficient by sparing a few unneeded I/O operations.
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/busses/i2c-viapro.c | 17 +++++++++--------
1 files changed, 9 insertions(+), 8 deletions(-)
diff -Nru a/drivers/i2c/busses/i2c-viapro.c b/drivers/i2c/busses/i2c-viapro.c
--- a/drivers/i2c/busses/i2c-viapro.c 2005-03-31 15:17:32 -08:00
+++ b/drivers/i2c/busses/i2c-viapro.c 2005-03-31 15:17:32 -08:00
@@ -121,12 +121,12 @@
inb_p(SMBHSTDAT1));
/* Make sure the SMBus host is ready to start transmitting */
- if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
+ if ((temp = inb_p(SMBHSTSTS)) & 0x1F) {
dev_dbg(&vt596_adapter.dev, "SMBus busy (0x%02x). "
"Resetting...\n", temp);
outb_p(temp, SMBHSTSTS);
- if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
+ if ((temp = inb_p(SMBHSTSTS)) & 0x1F) {
dev_dbg(&vt596_adapter.dev, "Failed! (0x%02x)\n", temp);
return -1;
@@ -168,13 +168,14 @@
dev_dbg(&vt596_adapter.dev, "Error: no response!\n");
}
- if (inb_p(SMBHSTSTS) != 0x00)
- outb_p(inb(SMBHSTSTS), SMBHSTSTS);
-
- if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
- dev_dbg(&vt596_adapter.dev, "Failed reset at end of "
- "transaction (%02x)\n", temp);
+ if ((temp = inb_p(SMBHSTSTS)) & 0x1F) {
+ outb_p(temp, SMBHSTSTS);
+ if ((temp = inb_p(SMBHSTSTS)) & 0x1F) {
+ dev_warn(&vt596_adapter.dev, "Failed reset at end "
+ "of transaction (%02x)\n", temp);
+ }
}
+
dev_dbg(&vt596_adapter.dev, "Transaction (post): CNT=%02x, CMD=%02x, "
"ADD=%02x, DAT0=%02x, DAT1=%02x\n", inb_p(SMBHSTCNT),
inb_p(SMBHSTCMD), inb_p(SMBHSTADD), inb_p(SMBHSTDAT0),
ChangeSet 1.2333, 2005/03/31 14:08:41-08:00, [email protected]
[PATCH] I2C: Clean up of i2c-elektor.c build
This patch changes the flags variable type from long to unsigned long in
one function. This removes a couple of warnings from the compile
messages for elektor i2c bus driver.
Signed-off-by: Frank Beesley <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/busses/i2c-elektor.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
diff -Nru a/drivers/i2c/busses/i2c-elektor.c b/drivers/i2c/busses/i2c-elektor.c
--- a/drivers/i2c/busses/i2c-elektor.c 2005-03-31 15:18:01 -08:00
+++ b/drivers/i2c/busses/i2c-elektor.c 2005-03-31 15:18:01 -08:00
@@ -112,7 +112,7 @@
static void pcf_isa_waitforpin(void) {
DEFINE_WAIT(wait);
int timeout = 2;
- long flags;
+ unsigned long flags;
if (irq > 0) {
spin_lock_irqsave(&lock, flags);
ChangeSet 1.2339, 2005/03/31 14:29:23-08:00, [email protected]
[PATCH] I2C: Fix Vaio EEPROM detection
This fixes a bug in the eeprom driver, which made all EEPROMs at
location 0x57 be erroneously treated as Vaio EEPROMs. I have to say I'm
quite ashamed that I introduced the bug in the first place, as this was
a really stupid one.
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/eeprom.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletion(-)
diff -Nru a/drivers/i2c/chips/eeprom.c b/drivers/i2c/chips/eeprom.c
--- a/drivers/i2c/chips/eeprom.c 2005-03-31 15:17:18 -08:00
+++ b/drivers/i2c/chips/eeprom.c 2005-03-31 15:17:18 -08:00
@@ -210,10 +210,11 @@
if (i2c_smbus_read_byte_data(new_client, 0x80) == 'P'
&& i2c_smbus_read_byte(new_client) == 'C'
&& i2c_smbus_read_byte(new_client) == 'G'
- && i2c_smbus_read_byte(new_client) == '-')
+ && i2c_smbus_read_byte(new_client) == '-') {
dev_info(&new_client->dev, "Vaio EEPROM detected, "
"enabling password protection\n");
data->nature = VAIO;
+ }
}
/* create the sysfs eeprom file */
ChangeSet 1.2328, 2005/03/31 14:07:05-08:00, [email protected]
[PATCH] I2C: Make master_xfer debug messages more useful
While working on the recent saa7110 mess, I found that the debug message
displayed when calling master_xfer wasn't as useful as it could be. Here
is a patch improving this.
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/i2c-core.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletion(-)
diff -Nru a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
--- a/drivers/i2c/i2c-core.c 2005-03-31 15:18:36 -08:00
+++ b/drivers/i2c/i2c-core.c 2005-03-31 15:18:36 -08:00
@@ -587,7 +587,13 @@
int ret;
if (adap->algo->master_xfer) {
- dev_dbg(&adap->dev, "master_xfer: with %d msgs.\n", num);
+#ifdef DEBUG
+ for (ret = 0; ret < num; ret++) {
+ dev_dbg(&adap->dev, "master_xfer[%d] %c, addr=0x%02x, "
+ "len=%d\n", ret, msgs[ret].flags & I2C_M_RD ?
+ 'R' : 'W', msgs[ret].addr, msgs[ret].len);
+ }
+#endif
down(&adap->bus_lock);
ret = adap->algo->master_xfer(adap,msgs,num);
ChangeSet 1.2329, 2005/03/31 14:07:24-08:00, [email protected]
[PATCH] I2C: Skip broken detection step in it87
One of the detection steps in the it87 chip driver was reported to be
broken for some revisions of the IT8712F chip [1] [2]. This detection
step is a legacy from the lm78 driver and the documentation available
for the IT8705F and IT8712F chips does not mention it at all. For this
reason, I propose to skip this detection step for Super-I/O chips.
Super-I/O chips have already been identified when we reach this step, so
it is redundant (additionally do being broken). This closes bug #4335.
[1] http://bugzilla.kernel.org/show_bug.cgi?id=4335
[2] http://archives.andrew.net.au/lm-sensors/msg29962.html
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/it87.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff -Nru a/drivers/i2c/chips/it87.c b/drivers/i2c/chips/it87.c
--- a/drivers/i2c/chips/it87.c 2005-03-31 15:18:29 -08:00
+++ b/drivers/i2c/chips/it87.c 2005-03-31 15:18:29 -08:00
@@ -734,10 +734,9 @@
goto ERROR0;
/* Probe whether there is anything available on this address. Already
- done for SMBus clients */
+ done for SMBus and Super-I/O clients */
if (kind < 0) {
- if (is_isa) {
-
+ if (is_isa && !chip_type) {
#define REALLY_SLOW_IO
/* We need the timeouts for at least some IT87-like chips. But only
if we read 'undefined' registers. */
ChangeSet 1.2326, 2005/03/31 14:06:28-08:00, [email protected]
[PATCH] I2C: Fix adm1021 alarms mask
This patch fixes an incorrect bitmasking on the status register in the
adm1021 driver, which was causing high alarm on remote temperature to be
hidden.
This bug was found and reported by Jayakrishnan:
http://bugzilla.kernel.org/show_bug.cgi?id=4285
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/adm1021.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
diff -Nru a/drivers/i2c/chips/adm1021.c b/drivers/i2c/chips/adm1021.c
--- a/drivers/i2c/chips/adm1021.c 2005-03-31 15:18:52 -08:00
+++ b/drivers/i2c/chips/adm1021.c 2005-03-31 15:18:52 -08:00
@@ -368,7 +368,7 @@
data->remote_temp_input = adm1021_read_value(client, ADM1021_REG_REMOTE_TEMP);
data->remote_temp_max = adm1021_read_value(client, ADM1021_REG_REMOTE_TOS_R);
data->remote_temp_hyst = adm1021_read_value(client, ADM1021_REG_REMOTE_THYST_R);
- data->alarms = adm1021_read_value(client, ADM1021_REG_STATUS) & 0xec;
+ data->alarms = adm1021_read_value(client, ADM1021_REG_STATUS) & 0x7c;
if (data->type == adm1021)
data->die_code = adm1021_read_value(client, ADM1021_REG_DIE_CODE);
if (data->type == adm1023) {
ChangeSet 1.2341, 2005/03/31 14:30:05-08:00, [email protected]
[PATCH] I2C: busses documentation update 2 of 2
Patch contains promised documentation update for i2c bus drivers.
I would like to thank Jean Delvare and Aurelien Jarno for their
comments.
Signed-off-by: Rudolf Marek <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Documentation/i2c/busses/i2c-ali1535 | 42 ++++++++++
Documentation/i2c/busses/i2c-ali1563 | 27 ++++++
Documentation/i2c/busses/i2c-ali15x3 | 112 +++++++++++++++++++++++++++++
Documentation/i2c/busses/i2c-amd756 | 25 ++++++
Documentation/i2c/busses/i2c-amd8111 | 41 ++++++++++
Documentation/i2c/busses/i2c-i801 | 80 ++++++++++++++++++++
Documentation/i2c/busses/i2c-i810 | 46 +++++++++++
Documentation/i2c/busses/i2c-nforce2 | 41 ++++++++++
Documentation/i2c/busses/i2c-parport | 18 ++--
Documentation/i2c/busses/i2c-parport-light | 11 ++
Documentation/i2c/busses/i2c-pca-isa | 23 +++++
Documentation/i2c/busses/i2c-piix4 | 72 ++++++++++++++++++
Documentation/i2c/busses/i2c-prosavage | 23 +++++
Documentation/i2c/busses/i2c-savage4 | 26 ++++++
Documentation/i2c/busses/i2c-sis5595 | 59 +++++++++++++++
Documentation/i2c/busses/i2c-sis630 | 49 ++++++++++++
Documentation/i2c/busses/i2c-sis69x | 73 ++++++++++++++++++
Documentation/i2c/busses/i2c-via | 34 ++++++++
Documentation/i2c/busses/i2c-viapro | 47 ++++++++++++
Documentation/i2c/busses/i2c-voodoo3 | 62 ++++++++++++++++
Documentation/i2c/busses/scx200_acb | 14 +++
21 files changed, 915 insertions(+), 10 deletions(-)
diff -Nru a/Documentation/i2c/busses/i2c-ali1535 b/Documentation/i2c/busses/i2c-ali1535
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-ali1535 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,42 @@
+Kernel driver i2c-ali1535
+
+Supported adapters:
+ * Acer Labs, Inc. ALI 1535 (south bridge)
+ Datasheet: Now under NDA
+ http://www.ali.com.tw/eng/support/datasheet_request.php
+
+Authors:
+ Frodo Looijaard <[email protected]>,
+ Philip Edelbrock <[email protected]>,
+ Mark D. Studebaker <[email protected]>,
+ Dan Eaton <[email protected]>,
+ Stephen Rousset<[email protected]>
+
+Description
+-----------
+
+This is the driver for the SMB Host controller on Acer Labs Inc. (ALI)
+M1535 South Bridge.
+
+The M1535 is a South bridge for portable systems. It is very similar to the
+M15x3 South bridges also produced by Acer Labs Inc. Some of the registers
+within the part have moved and some have been redefined slightly.
+Additionally, the sequencing of the SMBus transactions has been modified to
+be more consistent with the sequencing recommended by the manufacturer and
+observed through testing. These changes are reflected in this driver and
+can be identified by comparing this driver to the i2c-ali15x3 driver. For
+an overview of these chips see http://www.acerlabs.com
+
+The SMB controller is part of the M7101 device, which is an ACPI-compliant
+Power Management Unit (PMU).
+
+The whole M7101 device has to be enabled for the SMB to work. You can't
+just enable the SMB alone. The SMB and the ACPI have separate I/O spaces.
+We make sure that the SMB is enabled. We leave the ACPI alone.
+
+
+Features
+--------
+
+This driver controls the SMB Host only. This driver does not use
+interrupts.
diff -Nru a/Documentation/i2c/busses/i2c-ali1563 b/Documentation/i2c/busses/i2c-ali1563
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-ali1563 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,27 @@
+Kernel driver i2c-ali1563
+
+Supported adapters:
+ * Acer Labs, Inc. ALI 1563 (south bridge)
+ Datasheet: Now under NDA
+ http://www.ali.com.tw/eng/support/datasheet_request.php
+
+Author: Patrick Mochel <[email protected]>
+
+Description
+-----------
+
+This is the driver for the SMB Host controller on Acer Labs Inc. (ALI)
+M1563 South Bridge.
+
+For an overview of these chips see http://www.acerlabs.com
+
+The M1563 southbridge is deceptively similar to the M1533, with a few
+notable exceptions. One of those happens to be the fact they upgraded the
+i2c core to be SMBus 2.0 compliant, and happens to be almost identical to
+the i2c controller found in the Intel 801 south bridges.
+
+Features
+--------
+
+This driver controls the SMB Host only. This driver does not use
+interrupts.
diff -Nru a/Documentation/i2c/busses/i2c-ali15x3 b/Documentation/i2c/busses/i2c-ali15x3
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-ali15x3 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,112 @@
+Kernel driver i2c-ali15x3
+
+Supported adapters:
+ * Acer Labs, Inc. ALI 1533 and 1543C (south bridge)
+ Datasheet: Now under NDA
+ http://www.ali.com.tw/eng/support/datasheet_request.php
+
+Authors:
+ Frodo Looijaard <[email protected]>,
+ Philip Edelbrock <[email protected]>,
+ Mark D. Studebaker <[email protected]>
+
+Module Parameters
+-----------------
+
+* force_addr: int
+ Initialize the base address of the i2c controller
+
+
+Notes
+-----
+
+The force_addr parameter is useful for boards that don't set the address in
+the BIOS. Does not do a PCI force; the device must still be present in
+lspci. Don't use this unless the driver complains that the base address is
+not set.
+
+Example: 'modprobe i2c-ali15x3 force_addr=0xe800'
+
+SMBus periodically hangs on ASUS P5A motherboards and can only be cleared
+by a power cycle. Cause unknown (see Issues below).
+
+
+Description
+-----------
+
+This is the driver for the SMB Host controller on Acer Labs Inc. (ALI)
+M1541 and M1543C South Bridges.
+
+The M1543C is a South bridge for desktop systems.
+The M1541 is a South bridge for portable systems.
+They are part of the following ALI chipsets:
+
+ * "Aladdin Pro 2" includes the M1621 Slot 1 North bridge with AGP and
+ 100MHz CPU Front Side bus
+ * "Aladdin V" includes the M1541 Socket 7 North bridge with AGP and 100MHz
+ CPU Front Side bus
+ Some Aladdin V motherboards:
+ Asus P5A
+ Atrend ATC-5220
+ BCM/GVC VP1541
+ Biostar M5ALA
+ Gigabyte GA-5AX (** Generally doesn't work because the BIOS doesn't
+ enable the 7101 device! **)
+ Iwill XA100 Plus
+ Micronics C200
+ Microstar (MSI) MS-5169
+
+ * "Aladdin IV" includes the M1541 Socket 7 North bridge
+ with host bus up to 83.3 MHz.
+
+For an overview of these chips see http://www.acerlabs.com. At this time the
+full data sheets on the web site are password protected, however if you
+contact the ALI office in San Jose they may give you the password.
+
+The M1533/M1543C devices appear as FOUR separate devices on the PCI bus. An
+output of lspci will show something similar to the following:
+
+ 00:02.0 USB Controller: Acer Laboratories Inc. M5237 (rev 03)
+ 00:03.0 Bridge: Acer Laboratories Inc. M7101 <= THIS IS THE ONE WE NEED
+ 00:07.0 ISA bridge: Acer Laboratories Inc. M1533 (rev c3)
+ 00:0f.0 IDE interface: Acer Laboratories Inc. M5229 (rev c1)
+
+** IMPORTANT **
+** If you have a M1533 or M1543C on the board and you get
+** "ali15x3: Error: Can't detect ali15x3!"
+** then run lspci.
+** If you see the 1533 and 5229 devices but NOT the 7101 device,
+** then you must enable ACPI, the PMU, SMB, or something similar
+** in the BIOS.
+** The driver won't work if it can't find the M7101 device.
+
+The SMB controller is part of the M7101 device, which is an ACPI-compliant
+Power Management Unit (PMU).
+
+The whole M7101 device has to be enabled for the SMB to work. You can't
+just enable the SMB alone. The SMB and the ACPI have separate I/O spaces.
+We make sure that the SMB is enabled. We leave the ACPI alone.
+
+Features
+--------
+
+This driver controls the SMB Host only. The SMB Slave
+controller on the M15X3 is not enabled. This driver does not use
+interrupts.
+
+
+Issues
+------
+
+This driver requests the I/O space for only the SMB
+registers. It doesn't use the ACPI region.
+
+On the ASUS P5A motherboard, there are several reports that
+the SMBus will hang and this can only be resolved by
+powering off the computer. It appears to be worse when the board
+gets hot, for example under heavy CPU load, or in the summer.
+There may be electrical problems on this board.
+On the P5A, the W83781D sensor chip is on both the ISA and
+SMBus. Therefore the SMBus hangs can generally be avoided
+by accessing the W83781D on the ISA bus only.
+
diff -Nru a/Documentation/i2c/busses/i2c-amd756 b/Documentation/i2c/busses/i2c-amd756
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-amd756 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,25 @@
+Kernel driver i2c-amd756
+
+Supported adapters:
+ * AMD 756
+ * AMD 766
+ * AMD 768
+ * AMD 8111
+ Datasheets: Publicly available on AMD website
+
+ * nVidia nForce
+ Datasheet: Unavailable
+
+Authors:
+ Frodo Looijaard <[email protected]>,
+ Philip Edelbrock <[email protected]>
+
+Description
+-----------
+
+This driver supports the AMD 756, 766, 768 and 8111 Peripheral Bus
+Controllers, and the nVidia nForce.
+
+Note that for the 8111, there are two SMBus adapters. The SMBus 1.0 adapter
+is supported by this driver, and the SMBus 2.0 adapter is supported by the
+i2c-amd8111 driver.
diff -Nru a/Documentation/i2c/busses/i2c-amd8111 b/Documentation/i2c/busses/i2c-amd8111
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-amd8111 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,41 @@
+Kernel driver i2c-adm8111
+
+Supported adapters:
+ * AMD-8111 SMBus 2.0 PCI interface
+
+Datasheets:
+ AMD datasheet not yet available, but almost everything can be found
+ in publically available ACPI 2.0 specification, which the adapter
+ follows.
+
+Author: Vojtech Pavlik <[email protected]>
+
+Description
+-----------
+
+If you see something like this:
+
+00:07.2 SMBus: Advanced Micro Devices [AMD] AMD-8111 SMBus 2.0 (rev 02)
+ Subsystem: Advanced Micro Devices [AMD] AMD-8111 SMBus 2.0
+ Flags: medium devsel, IRQ 19
+ I/O ports at d400 [size=32]
+
+in your 'lspci -v', then this driver is for your chipset.
+
+Process Call Support
+--------------------
+
+Supported.
+
+SMBus 2.0 Support
+-----------------
+
+Supported. Both PEC and block process call support is implemented. Slave
+mode or host notification are not yet implemented.
+
+Notes
+-----
+
+Note that for the 8111, there are two SMBus adapters. The SMBus 2.0 adapter
+is supported by this driver, and the SMBus 1.0 adapter is supported by the
+i2c-amd756 driver.
diff -Nru a/Documentation/i2c/busses/i2c-i801 b/Documentation/i2c/busses/i2c-i801
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-i801 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,80 @@
+Kernel driver i2c-i801
+
+Supported adapters:
+ * Intel 82801AA and 82801AB (ICH and ICH0 - part of the
+ '810' and '810E' chipsets)
+ * Intel 82801BA (ICH2 - part of the '815E' chipset)
+ * Intel 82801CA/CAM (ICH3)
+ * Intel 82801DB (ICH4) (HW PEC supported, 32 byte buffer not supported)
+ * Intel 82801EB/ER (ICH5) (HW PEC supported, 32 byte buffer not supported)
+ * Intel 6300ESB
+ * Intel 82801FB/FR/FW/FRW (ICH6)
+ * Intel ICH7
+ Datasheets: Publicly available at the Intel website
+
+Authors:
+ Frodo Looijaard <[email protected]>,
+ Philip Edelbrock <[email protected]>,
+ Mark Studebaker <[email protected]>
+
+
+Module Parameters
+-----------------
+
+* force_addr: int
+ Forcibly enable the ICH at the given address. EXTREMELY DANGEROUS!
+
+
+Description
+-----------
+
+The ICH (properly known as the 82801AA), ICH0 (82801AB), ICH2 (82801BA),
+ICH3 (82801CA/CAM) and later devices are Intel chips that are a part of
+Intel's '810' chipset for Celeron-based PCs, '810E' chipset for
+Pentium-based PCs, '815E' chipset, and others.
+
+The ICH chips contain at least SEVEN separate PCI functions in TWO logical
+PCI devices. An output of lspci will show something similar to the
+following:
+
+ 00:1e.0 PCI bridge: Intel Corporation: Unknown device 2418 (rev 01)
+ 00:1f.0 ISA bridge: Intel Corporation: Unknown device 2410 (rev 01)
+ 00:1f.1 IDE interface: Intel Corporation: Unknown device 2411 (rev 01)
+ 00:1f.2 USB Controller: Intel Corporation: Unknown device 2412 (rev 01)
+ 00:1f.3 Unknown class [0c05]: Intel Corporation: Unknown device 2413 (rev 01)
+
+The SMBus controller is function 3 in device 1f. Class 0c05 is SMBus Serial
+Controller.
+
+If you do NOT see the 24x3 device at function 3, and you can't figure out
+any way in the BIOS to enable it,
+
+The ICH chips are quite similar to Intel's PIIX4 chip, at least in the
+SMBus controller.
+
+See the file i2c-piix4 for some additional information.
+
+
+Process Call Support
+--------------------
+
+Not supported.
+
+
+I2C Block Read Support
+----------------------
+
+Not supported at the moment.
+
+
+SMBus 2.0 Support
+-----------------
+
+The 82801DB (ICH4) and later chips support several SMBus 2.0 features.
+
+**********************
+The lm_sensors project gratefully acknowledges the support of Texas
+Instruments in the initial development of this driver.
+
+The lm_sensors project gratefully acknowledges the support of Intel in the
+development of SMBus 2.0 / ICH4 features of this driver.
diff -Nru a/Documentation/i2c/busses/i2c-i810 b/Documentation/i2c/busses/i2c-i810
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-i810 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,46 @@
+Kernel driver i2c-i810
+
+Supported adapters:
+ * Intel 82810, 82810-DC100, 82810E, and 82815 (GMCH)
+
+Authors:
+ Frodo Looijaard <[email protected]>,
+ Philip Edelbrock <[email protected]>,
+ Ky?sti M?lkki <[email protected]>,
+ Ralph Metzler <[email protected]>,
+ Mark D. Studebaker <[email protected]>
+
+Main contact: Mark Studebaker <[email protected]>
+
+Description
+-----------
+
+WARNING: If you have an '810' or '815' motherboard, your standard I2C
+temperature sensors are most likely on the 801's I2C bus. You want the
+i2c-i801 driver for those, not this driver.
+
+Now for the i2c-i810...
+
+The GMCH chip contains two I2C interfaces.
+
+The first interface is used for DDC (Data Display Channel) which is a
+serial channel through the VGA monitor connector to a DDC-compliant
+monitor. This interface is defined by the Video Electronics Standards
+Association (VESA). The standards are available for purchase at
+http://www.vesa.org .
+
+The second interface is a general-purpose I2C bus. It may be connected to a
+TV-out chip such as the BT869 or possibly to a digital flat-panel display.
+
+Features
+--------
+
+Both busses use the i2c-algo-bit driver for 'bit banging'
+and support for specific transactions is provided by i2c-algo-bit.
+
+Issues
+------
+
+If you enable bus testing in i2c-algo-bit (insmod i2c-algo-bit bit_test=1),
+the test may fail; if so, the i2c-i810 driver won't be inserted. However,
+we think this has been fixed.
diff -Nru a/Documentation/i2c/busses/i2c-nforce2 b/Documentation/i2c/busses/i2c-nforce2
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-nforce2 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,41 @@
+Kernel driver i2c-nforce2
+
+Supported adapters:
+ * nForce2 MCP 10de:0064
+ * nForce2 Ultra 400 MCP 10de:0084
+ * nForce3 Pro150 MCP 10de:00D4
+ * nForce3 250Gb MCP 10de:00E4
+ * nForce4 MCP 10de:0052
+
+Datasheet: not publically available, but seems to be similar to the
+ AMD-8111 SMBus 2.0 adapter.
+
+Authors:
+ Hans-Frieder Vogt <[email protected]>,
+ Thomas Leibold <[email protected]>,
+ Patrick Dreker <[email protected]>
+
+Description
+-----------
+
+i2c-nforce2 is a driver for the SMBuses included in the nVidia nForce2 MCP.
+
+If your 'lspci -v' listing shows something like the following,
+
+00:01.1 SMBus: nVidia Corporation: Unknown device 0064 (rev a2)
+ Subsystem: Asustek Computer, Inc.: Unknown device 0c11
+ Flags: 66Mhz, fast devsel, IRQ 5
+ I/O ports at c000 [size=32]
+ Capabilities: <available only to root>
+
+then this driver should support the SMBuses of your motherboard.
+
+
+Notes
+-----
+
+The SMBus adapter in the nForce2 chipset seems to be very similar to the
+SMBus 2.0 adapter in the AMD-8111 southbridge. However, I could only get
+the driver to work with direct I/O access, which is different to the EC
+interface of the AMD-8111. Tested on Asus A7N8X. The ACPI DSDT table of the
+Asus A7N8X lists two SMBuses, both of which are supported by this driver.
diff -Nru a/Documentation/i2c/busses/i2c-parport b/Documentation/i2c/busses/i2c-parport
--- a/Documentation/i2c/busses/i2c-parport 2005-03-31 15:17:04 -08:00
+++ b/Documentation/i2c/busses/i2c-parport 2005-03-31 15:17:04 -08:00
@@ -1,8 +1,6 @@
-==================
-i2c-parport driver
-==================
+Kernel driver i2c-parport
-2004-07-06, Jean Delvare
+Author: Jean Delvare <[email protected]>
This is a unified driver for several i2c-over-parallel-port adapters,
such as the ones made by Philips, Velleman or ELV. This driver is
@@ -126,14 +124,14 @@
Similar (but different) drivers
-------------------------------
-This driver is NOT the same as the i2c-pport driver found in the i2c package.
-The i2c-pport driver makes use of modern parallel port features so that
-you don't need additional electronics. It has other restrictions however, and
-was not ported to Linux 2.6 (yet).
+This driver is NOT the same as the i2c-pport driver found in the i2c
+package. The i2c-pport driver makes use of modern parallel port features so
+that you don't need additional electronics. It has other restrictions
+however, and was not ported to Linux 2.6 (yet).
This driver is also NOT the same as the i2c-pcf-epp driver found in the
-lm_sensors package. The i2c-pcf-epp driver doesn't use the parallel port
-as an I2C bus directly. Instead, it uses it to control an external I2C bus
+lm_sensors package. The i2c-pcf-epp driver doesn't use the parallel port as
+an I2C bus directly. Instead, it uses it to control an external I2C bus
master. That driver was not ported to Linux 2.6 (yet) either.
diff -Nru a/Documentation/i2c/busses/i2c-parport-light b/Documentation/i2c/busses/i2c-parport-light
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-parport-light 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,11 @@
+Kernel driver i2c-parport-light
+
+Author: Jean Delvare <[email protected]>
+
+This driver is a light version of i2c-parport. It doesn't depend
+on the parport driver, and uses direct I/O access instead. This might be
+prefered on embedded systems where wasting memory for the clean but heavy
+parport handling is not an option. The drawback is a reduced portability
+and the impossibility to daisy-chain other parallel port devices.
+
+Please see i2c-parport for documentation.
diff -Nru a/Documentation/i2c/busses/i2c-pca-isa b/Documentation/i2c/busses/i2c-pca-isa
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-pca-isa 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,23 @@
+Kernel driver i2c-pca-isa
+
+Supported adapters:
+This driver supports ISA boards using the Philips PCA 9564
+Parallel bus to I2C bus controller
+
+Author: Ian Campbell <[email protected]>, Arcom Control Systems
+
+Module Parameters
+-----------------
+
+* base int
+ I/O base address
+* irq int
+ IRQ interrupt
+* clock int
+ Clock rate as described in table 1 of PCA9564 datasheet
+
+Description
+-----------
+
+This driver supports ISA boards using the Philips PCA 9564
+Parallel bus to I2C bus controller
diff -Nru a/Documentation/i2c/busses/i2c-piix4 b/Documentation/i2c/busses/i2c-piix4
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-piix4 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,72 @@
+Kernel driver i2c-piix4
+
+Supported adapters:
+ * Intel 82371AB PIIX4 and PIIX4E
+ * Intel 82443MX (440MX)
+ Datasheet: Publicly available at the Intel website
+ * ServerWorks OSB4, CSB5 and CSB6 southbridges
+ Datasheet: Only available via NDA from ServerWorks
+ * Standard Microsystems (SMSC) SLC90E66 (Victory66) southbridge
+ Datasheet: Publicly available at the SMSC website http://www.smsc.com
+
+Authors:
+ Frodo Looijaard <[email protected]>
+ Philip Edelbrock <[email protected]>
+
+
+Module Parameters
+-----------------
+
+* force: int
+ Forcibly enable the PIIX4. DANGEROUS!
+* force_addr: int
+ Forcibly enable the PIIX4 at the given address. EXTREMELY DANGEROUS!
+* fix_hstcfg: int
+ Fix config register. Needed on some boards (Force CPCI735).
+
+
+Description
+-----------
+
+The PIIX4 (properly known as the 82371AB) is an Intel chip with a lot of
+functionality. Among other things, it implements the PCI bus. One of its
+minor functions is implementing a System Management Bus. This is a true
+SMBus - you can not access it on I2C levels. The good news is that it
+natively understands SMBus commands and you do not have to worry about
+timing problems. The bad news is that non-SMBus devices connected to it can
+confuse it mightily. Yes, this is known to happen...
+
+Do 'lspci -v' and see whether it contains an entry like this:
+
+0000:00:02.3 Bridge: Intel Corp. 82371AB/EB/MB PIIX4 ACPI (rev 02)
+ Flags: medium devsel, IRQ 9
+
+Bus and device numbers may differ, but the function number must be
+identical (like many PCI devices, the PIIX4 incorporates a number of
+different 'functions', which can be considered as separate devices). If you
+find such an entry, you have a PIIX4 SMBus controller.
+
+On some computers (most notably, some Dells), the SMBus is disabled by
+default. If you use the insmod parameter 'force=1', the kernel module will
+try to enable it. THIS IS VERY DANGEROUS! If the BIOS did not set up a
+correct address for this module, you could get in big trouble (read:
+crashes, data corruption, etc.). Try this only as a last resort (try BIOS
+updates first, for example), and backup first! An even more dangerous
+option is 'force_addr=<IOPORT>'. This will not only enable the PIIX4 like
+'force' foes, but it will also set a new base I/O port address. The SMBus
+parts of the PIIX4 needs a range of 8 of these addresses to function
+correctly. If these addresses are already reserved by some other device,
+you will get into big trouble! DON'T USE THIS IF YOU ARE NOT VERY SURE
+ABOUT WHAT YOU ARE DOING!
+
+The PIIX4E is just an new version of the PIIX4; it is supported as well.
+The PIIX/PIIX3 does not implement an SMBus or I2C bus, so you can't use
+this driver on those mainboards.
+
+The ServerWorks Southbridges, the Intel 440MX, and the Victory766 are
+identical to the PIIX4 in I2C/SMBus support.
+
+A few OSB4 southbridges are known to be misconfigured by the BIOS. In this
+case, you have you use the fix_hstcfg module parameter. Do not use it
+unless you know you have to, because in some cases it also breaks
+configuration on southbridges that don't need it.
diff -Nru a/Documentation/i2c/busses/i2c-prosavage b/Documentation/i2c/busses/i2c-prosavage
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-prosavage 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,23 @@
+Kernel driver i2c-prosavage
+
+Supported adapters:
+
+ S3/VIA KM266/VT8375 aka ProSavage8
+ S3/VIA KM133/VT8365 aka Savage4
+
+Author: Henk Vergonet <[email protected]>
+
+Description
+-----------
+
+The Savage4 chips contain two I2C interfaces (aka a I2C 'master' or
+'host').
+
+The first interface is used for DDC (Data Display Channel) which is a
+serial channel through the VGA monitor connector to a DDC-compliant
+monitor. This interface is defined by the Video Electronics Standards
+Association (VESA). The standards are available for purchase at
+http://www.vesa.org . The second interface is a general-purpose I2C bus.
+
+Usefull for gaining access to the TV Encoder chips.
+
diff -Nru a/Documentation/i2c/busses/i2c-savage4 b/Documentation/i2c/busses/i2c-savage4
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-savage4 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,26 @@
+Kernel driver i2c-savage4
+
+Supported adapters:
+ * Savage4
+ * Savage2000
+
+Authors:
+ Alexander Wold <[email protected]>,
+ Mark D. Studebaker <[email protected]>
+
+Description
+-----------
+
+The Savage4 chips contain two I2C interfaces (aka a I2C 'master'
+or 'host').
+
+The first interface is used for DDC (Data Display Channel) which is a
+serial channel through the VGA monitor connector to a DDC-compliant
+monitor. This interface is defined by the Video Electronics Standards
+Association (VESA). The standards are available for purchase at
+http://www.vesa.org . The DDC bus is not yet supported because its register
+is not directly memory-mapped.
+
+The second interface is a general-purpose I2C bus. This is the only
+interface supported by the driver at the moment.
+
diff -Nru a/Documentation/i2c/busses/i2c-sis5595 b/Documentation/i2c/busses/i2c-sis5595
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-sis5595 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,59 @@
+Kernel driver i2c-sis5595
+
+Authors:
+ Frodo Looijaard <[email protected]>,
+ Mark D. Studebaker <[email protected]>,
+ Philip Edelbrock <[email protected]>
+
+Supported adapters:
+ * Silicon Integrated Systems Corp. SiS5595 Southbridge
+ Datasheet: Publicly available at the Silicon Integrated Systems Corp. site.
+
+Note: all have mfr. ID 0x1039.
+
+ SUPPORTED PCI ID
+ 5595 0008
+
+ Note: these chips contain a 0008 device which is incompatible with the
+ 5595. We recognize these by the presence of the listed
+ "blacklist" PCI ID and refuse to load.
+
+ NOT SUPPORTED PCI ID BLACKLIST PCI ID
+ 540 0008 0540
+ 550 0008 0550
+ 5513 0008 5511
+ 5581 0008 5597
+ 5582 0008 5597
+ 5597 0008 5597
+ 5598 0008 5597/5598
+ 630 0008 0630
+ 645 0008 0645
+ 646 0008 0646
+ 648 0008 0648
+ 650 0008 0650
+ 651 0008 0651
+ 730 0008 0730
+ 735 0008 0735
+ 745 0008 0745
+ 746 0008 0746
+
+Module Parameters
+-----------------
+
+* force_addr=0xaddr Set the I/O base address. Useful for boards
+ that don't set the address in the BIOS. Does not do a
+ PCI force; the device must still be present in lspci.
+ Don't use this unless the driver complains that the
+ base address is not set.
+
+Description
+-----------
+
+i2c-sis5595 is a true SMBus host driver for motherboards with the SiS5595
+southbridges.
+
+WARNING: If you are trying to access the integrated sensors on the SiS5595
+chip, you want the sis5595 driver for those, not this driver. This driver
+is a BUS driver, not a CHIP driver. A BUS driver is used by other CHIP
+drivers to access chips on the bus.
+
diff -Nru a/Documentation/i2c/busses/i2c-sis630 b/Documentation/i2c/busses/i2c-sis630
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-sis630 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,49 @@
+Kernel driver i2c-sis630
+
+Supported adapters:
+ * Silicon Integrated Systems Corp (SiS)
+ 630 chipset (Datasheet: available at http://amalysh.bei.t-online.de/docs/SIS/)
+ 730 chipset
+ * Possible other SiS chipsets ?
+
+Author: Alexander Malysh <[email protected]>
+
+Module Parameters
+-----------------
+
+* force = [1|0] Forcibly enable the SIS630. DANGEROUS!
+ This can be interesting for chipsets not named
+ above to check if it works for you chipset, but DANGEROUS!
+
+* high_clock = [1|0] Forcibly set Host Master Clock to 56KHz (default,
+ what your BIOS use). DANGEROUS! This should be a bit
+ faster, but freeze some systems (i.e. my Laptop).
+
+
+Description
+-----------
+
+This SMBus only driver is known to work on motherboards with the above
+named chipsets.
+
+If you see something like this:
+
+00:00.0 Host bridge: Silicon Integrated Systems [SiS] 630 Host (rev 31)
+00:01.0 ISA bridge: Silicon Integrated Systems [SiS] 85C503/5513
+
+or like this:
+
+00:00.0 Host bridge: Silicon Integrated Systems [SiS] 730 Host (rev 02)
+00:01.0 ISA bridge: Silicon Integrated Systems [SiS] 85C503/5513
+
+in your 'lspci' output , then this driver is for your chipset.
+
+Thank You
+---------
+Philip Edelbrock <[email protected]>
+- testing SiS730 support
+Mark M. Hoffman <[email protected]>
+- bug fixes
+
+To anyone else which I forgot here ;), thanks!
+
diff -Nru a/Documentation/i2c/busses/i2c-sis69x b/Documentation/i2c/busses/i2c-sis69x
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-sis69x 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,73 @@
+Kernel driver i2c-sis96x
+
+Replaces 2.4.x i2c-sis645
+
+Supported adapters:
+ * Silicon Integrated Systems Corp (SiS)
+ Any combination of these host bridges:
+ 645, 645DX (aka 646), 648, 650, 651, 655, 735, 745, 746
+ and these south bridges:
+ 961, 962, 963(L)
+
+Author: Mark M. Hoffman <[email protected]>
+
+Description
+-----------
+
+This SMBus only driver is known to work on motherboards with the above
+named chipset combinations. The driver was developed without benefit of a
+proper datasheet from SiS. The SMBus registers are assumed compatible with
+those of the SiS630, although they are located in a completely different
+place. Thanks to Alexander Malysh <[email protected]> for providing the
+SiS630 datasheet (and driver).
+
+The command "lspci" as root should produce something like these lines:
+
+00:00.0 Host bridge: Silicon Integrated Systems [SiS]: Unknown device 0645
+00:02.0 ISA bridge: Silicon Integrated Systems [SiS] 85C503/5513
+00:02.1 SMBus: Silicon Integrated Systems [SiS]: Unknown device 0016
+
+or perhaps this...
+
+00:00.0 Host bridge: Silicon Integrated Systems [SiS]: Unknown device 0645
+00:02.0 ISA bridge: Silicon Integrated Systems [SiS]: Unknown device 0961
+00:02.1 SMBus: Silicon Integrated Systems [SiS]: Unknown device 0016
+
+(kernel versions later than 2.4.18 may fill in the "Unknown"s)
+
+If you cant see it please look on quirk_sis_96x_smbus
+(drivers/pci/quirks.c) (also if southbridge detection fails)
+
+I suspect that this driver could be made to work for the following SiS
+chipsets as well: 635, and 635T. If anyone owns a board with those chips
+AND is willing to risk crashing & burning an otherwise well-behaved kernel
+in the name of progress... please contact me at <[email protected]> or
+via the project's mailing list: <[email protected]>. Please
+send bug reports and/or success stories as well.
+
+
+TO DOs
+------
+
+* The driver does not support SMBus block reads/writes; I may add them if a
+scenario is found where they're needed.
+
+
+Thank You
+---------
+
+Mark D. Studebaker <[email protected]>
+ - design hints and bug fixes
+Alexander Maylsh <[email protected]>
+ - ditto, plus an important datasheet... almost the one I really wanted
+Hans-G?nter L?tke Uphues <[email protected]>
+ - patch for SiS735
+Robert Zwerus <[email protected]>
+ - testing for SiS645DX
+Kianusch Sayah Karadji <[email protected]>
+ - patch for SiS645DX/962
+Ken Healy
+ - patch for SiS655
+
+To anyone else who has written w/ feedback, thanks!
+
diff -Nru a/Documentation/i2c/busses/i2c-via b/Documentation/i2c/busses/i2c-via
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-via 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,34 @@
+Kernel driver i2c-via
+
+Supported adapters:
+ * VIA Technologies, InC. VT82C586B
+ Datasheet: Publicly available at the VIA website
+
+Author: Ky?sti M?lkki <[email protected]>
+
+Description
+-----------
+
+i2c-via is an i2c bus driver for motherboards with VIA chipset.
+
+The following VIA pci chipsets are supported:
+ - MVP3, VP3, VP2/97, VPX/97
+ - others with South bridge VT82C586B
+
+Your lspci listing must show this :
+
+ Bridge: VIA Technologies, Inc. VT82C586B ACPI (rev 10)
+
+ Problems?
+
+ Q: You have VT82C586B on the motherboard, but not in the listing.
+
+ A: Go to your BIOS setup, section PCI devices or similar.
+ Turn USB support on, and try again.
+
+ Q: No error messages, but still i2c doesn't seem to work.
+
+ A: This can happen. This driver uses the pins VIA recommends in their
+ datasheets, but there are several ways the motherboard manufacturer
+ can actually wire the lines.
+
diff -Nru a/Documentation/i2c/busses/i2c-viapro b/Documentation/i2c/busses/i2c-viapro
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-viapro 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,47 @@
+Kernel driver i2c-viapro
+
+Supported adapters:
+ * VIA Technologies, Inc. VT82C596A/B
+ Datasheet: Sometimes available at the VIA website
+
+ * VIA Technologies, Inc. VT82C686A/B
+ Datasheet: Sometimes available at the VIA website
+
+ * VIA Technologies, Inc. VT8231, VT8233, VT8233A, VT8235, VT8237
+ Datasheet: available on request from Via
+
+Authors:
+ Frodo Looijaard <[email protected]>,
+ Philip Edelbrock <[email protected]>,
+ Ky?sti M?lkki <[email protected]>,
+ Mark D. Studebaker <[email protected]>
+
+Module Parameters
+-----------------
+
+* force: int
+ Forcibly enable the SMBus controller. DANGEROUS!
+* force_addr: int
+ Forcibly enable the SMBus at the given address. EXTREMELY DANGEROUS!
+
+Description
+-----------
+
+i2c-viapro is a true SMBus host driver for motherboards with one of the
+supported VIA southbridges.
+
+Your lspci -n listing must show one of these :
+
+ device 1106:3050 (VT82C596 function 3)
+ device 1106:3051 (VT82C596 function 3)
+ device 1106:3057 (VT82C686 function 4)
+ device 1106:3074 (VT8233)
+ device 1106:3147 (VT8233A)
+ device 1106:8235 (VT8231)
+ devide 1106:3177 (VT8235)
+ devide 1106:3227 (VT8237)
+
+If none of these show up, you should look in the BIOS for settings like
+enable ACPI / SMBus or even USB.
+
+
diff -Nru a/Documentation/i2c/busses/i2c-voodoo3 b/Documentation/i2c/busses/i2c-voodoo3
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-voodoo3 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,62 @@
+Kernel driver i2c-voodoo3
+
+Supported adapters:
+ * 3dfx Voodoo3 based cards
+ * Voodoo Banshee based cards
+
+Authors:
+ Frodo Looijaard <[email protected]>,
+ Philip Edelbrock <[email protected]>,
+ Ralph Metzler <[email protected]>,
+ Mark D. Studebaker <[email protected]>
+
+Main contact: Philip Edelbrock <[email protected]>
+
+The code is based upon Ralph's test code (he did the hard stuff ;')
+
+Description
+-----------
+
+The 3dfx Voodoo3 chip contains two I2C interfaces (aka a I2C 'master' or
+'host').
+
+The first interface is used for DDC (Data Display Channel) which is a
+serial channel through the VGA monitor connector to a DDC-compliant
+monitor. This interface is defined by the Video Electronics Standards
+Association (VESA). The standards are available for purchase at
+http://www.vesa.org .
+
+The second interface is a general-purpose I2C bus. The intent by 3dfx was
+to allow manufacturers to add extra chips to the video card such as a
+TV-out chip such as the BT869 or possibly even I2C based temperature
+sensors like the ADM1021 or LM75.
+
+Stability
+---------
+
+Seems to be stable on the test machine, but needs more testing on other
+machines. Simultaneous accesses of the DDC and I2C busses may cause errors.
+
+Supported Devices
+-----------------
+
+Specifically, this driver was written and tested on the '3dfx Voodoo3 AGP
+3000' which has a tv-out feature (s-video or composite). According to the
+docs and discussions, this code should work for any Voodoo3 based cards as
+well as Voodoo Banshee based cards. The DDC interface has been tested on a
+Voodoo Banshee card.
+
+Issues
+------
+
+Probably many, but it seems to work OK on my system. :')
+
+
+External Device Connection
+--------------------------
+
+The digital video input jumpers give availability to the I2C bus.
+Specifically, pins 13 and 25 (bottom row middle, and bottom right-end) are
+the I2C clock and I2C data lines, respectively. +5V and GND are probably
+also easily available making the addition of extra I2C/SMBus devices easy
+to implement.
diff -Nru a/Documentation/i2c/busses/scx200_acb b/Documentation/i2c/busses/scx200_acb
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/scx200_acb 2005-03-31 15:17:04 -08:00
@@ -0,0 +1,14 @@
+Kernel driver scx200_acb
+
+Author: Christer Weinigel <[email protected]>
+
+Module Parameters
+-----------------
+
+* base: int
+ Base addresses for the ACCESS.bus controllers
+
+Description
+-----------
+
+Enable the use of the ACCESS.bus controllers of a SCx200 processor.
ChangeSet 1.2351, 2005/03/31 14:33:14-08:00, [email protected]
[PATCH] i2c: i2c-mv64xxx - set adapter owner and class fields
This patch adds the correct values for the 'owner' and 'class' fields of
the adapter structure in the mv64xxx i2c bus driver. The missing class
field caused some i2c chip drivers to refuse to attempt a probe on the
mv64xxx i2c bus.
Signed-off-by: Chris Elston <[email protected]>
Signed-off-by: Mark A. Greer <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/busses/i2c-mv64xxx.c | 2 ++
1 files changed, 2 insertions(+)
diff -Nru a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
--- a/drivers/i2c/busses/i2c-mv64xxx.c 2005-03-31 15:15:49 -08:00
+++ b/drivers/i2c/busses/i2c-mv64xxx.c 2005-03-31 15:15:49 -08:00
@@ -525,6 +525,8 @@
drv_data->irq = platform_get_irq(pd, 0);
drv_data->adapter.id = I2C_ALGO_MV64XXX | I2C_HW_MV64XXX;
drv_data->adapter.algo = &mv64xxx_i2c_algo;
+ drv_data->adapter.owner = THIS_MODULE;
+ drv_data->adapter.class = I2C_CLASS_HWMON;
drv_data->adapter.timeout = pdata->timeout;
drv_data->adapter.retries = pdata->retries;
dev_set_drvdata(dev, drv_data);
ChangeSet 1.2346, 2005/03/31 14:31:40-08:00, [email protected]
[PATCH] I2C: i2c-s3c2410 functionality and fixes
This patch does the following updates to the i2c-s3c2410 bus driver:
* Properly report the i2c functionality by adding to the
`.functionality` field of the adapter
* Change the dev_err() call on no-ack to an dev_dbg() to make
it less noisy when the bus is being probed by i2cdetect, etc.
* Add I2C_M_REV_DIR_ADDR to fully implement the
I2C_FUNC_PROTOCOLO_MANGLING.
* Ensure that the adapter owner field is set to THIS_MODULE
Please apply, thanks.
(Once this is applied, all i2c bus drivers will be properly reporting
their functionality so I'll be able to go on with the i2c functionality
core cleanups.)
Signed-off-by: Ben Dooks <[email protected]>
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/busses/i2c-s3c2410.c | 15 +++++++++++++--
1 files changed, 13 insertions(+), 2 deletions(-)
diff -Nru a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
--- a/drivers/i2c/busses/i2c-s3c2410.c 2005-03-31 15:16:24 -08:00
+++ b/drivers/i2c/busses/i2c-s3c2410.c 2005-03-31 15:16:24 -08:00
@@ -1,6 +1,6 @@
/* linux/drivers/i2c/busses/i2c-s3c2410.c
*
- * Copyright (C) 2004 Simtec Electronics
+ * Copyright (C) 2004,2005 Simtec Electronics
* Ben Dooks <[email protected]>
*
* S3C2410 I2C Controller
@@ -188,6 +188,9 @@
} else
stat |= S3C2410_IICSTAT_MASTER_TX;
+ if (msg->flags & I2C_M_REV_DIR_ADDR)
+ addr ^= 1;
+
// todo - check for wether ack wanted or not
s3c24xx_i2c_enable_ack(i2c);
@@ -287,7 +290,7 @@
!(i2c->msg->flags & I2C_M_IGNORE_NAK)) {
/* ack was not received... */
- dev_err(i2c->dev, "ack was not received\n" );
+ dev_dbg(i2c->dev, "ack was not received\n");
s3c24xx_i2c_stop(i2c, -EREMOTEIO);
goto out_ack;
}
@@ -555,11 +558,18 @@
return -EREMOTEIO;
}
+/* declare our i2c functionality */
+static u32 s3c24xx_i2c_func(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
+}
+
/* i2c bus registration info */
static struct i2c_algorithm s3c24xx_i2c_algorithm = {
.name = "S3C2410-I2C-Algorithm",
.master_xfer = s3c24xx_i2c_xfer,
+ .functionality = s3c24xx_i2c_func,
};
static struct s3c24xx_i2c s3c24xx_i2c = {
@@ -567,6 +577,7 @@
.wait = __WAIT_QUEUE_HEAD_INITIALIZER(s3c24xx_i2c.wait),
.adap = {
.name = "s3c2410-i2c",
+ .owner = THIS_MODULE,
.algo = &s3c24xx_i2c_algorithm,
.retries = 2,
.class = I2C_CLASS_HWMON,
ChangeSet 1.2324, 2005/03/31 14:05:50-08:00, [email protected]
[PATCH] I2C: New lm92 chip driver
This is a new i2c chip driver named lm92. It supports the National
Semiconductor LM92 and Maxim MAX6635 chips. This time I did not port the
driver from the lm_sensors project but instead rewrote it. The reason is
that the original driver has a different structure from the other i2c
chip drivers, which would have made maintenance harder.
I don't have a compatible chip myself but could test my code thanks to
Mark Hoffman's i2c-stub fake bus driver. Later on, James Chapman tested
it on a real chip, successfully.
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/Kconfig | 11 +
drivers/i2c/chips/Makefile | 1
drivers/i2c/chips/lm92.c | 423 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 435 insertions(+)
diff -Nru a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
--- a/drivers/i2c/chips/Kconfig 2005-03-31 15:19:07 -08:00
+++ b/drivers/i2c/chips/Kconfig 2005-03-31 15:19:07 -08:00
@@ -236,6 +236,17 @@
This driver can also be built as a module. If so, the module
will be called lm90.
+config SENSORS_LM92
+ tristate "National Semiconductor LM92 and compatibles"
+ depends on I2C && EXPERIMENTAL
+ select I2C_SENSOR
+ help
+ If you say yes here you get support for National Semiconductor LM92
+ and Maxim MAX6635 sensor chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called lm92.
+
config SENSORS_MAX1619
tristate "Maxim MAX1619 sensor chip"
depends on I2C && EXPERIMENTAL
diff -Nru a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile
--- a/drivers/i2c/chips/Makefile 2005-03-31 15:19:07 -08:00
+++ b/drivers/i2c/chips/Makefile 2005-03-31 15:19:07 -08:00
@@ -27,6 +27,7 @@
obj-$(CONFIG_SENSORS_LM85) += lm85.o
obj-$(CONFIG_SENSORS_LM87) += lm87.o
obj-$(CONFIG_SENSORS_LM90) += lm90.o
+obj-$(CONFIG_SENSORS_LM92) += lm92.o
obj-$(CONFIG_SENSORS_MAX1619) += max1619.o
obj-$(CONFIG_SENSORS_M41T00) += m41t00.o
obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
diff -Nru a/drivers/i2c/chips/lm92.c b/drivers/i2c/chips/lm92.c
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/drivers/i2c/chips/lm92.c 2005-03-31 15:19:07 -08:00
@@ -0,0 +1,423 @@
+/*
+ * lm92 - Hardware monitoring driver
+ * Copyright (C) 2005 Jean Delvare <[email protected]>
+ *
+ * Based on the lm90 driver, with some ideas taken from the lm_sensors
+ * lm92 driver as well.
+ *
+ * The LM92 is a sensor chip made by National Semiconductor. It reports
+ * its own temperature with a 0.0625 deg resolution and a 0.33 deg
+ * accuracy. Complete datasheet can be obtained from National's website
+ * at:
+ * http://www.national.com/pf/LM/LM92.html
+ *
+ * This driver also supports the MAX6635 sensor chip made by Maxim.
+ * This chip is compatible with the LM92, but has a lesser accuracy
+ * (1.0 deg). Complete datasheet can be obtained from Maxim's website
+ * at:
+ * http://www.maxim-ic.com/quick_view2.cfm/qv_pk/3074
+ *
+ * Since the LM92 was the first chipset supported by this driver, most
+ * comments will refer to this chipset, but are actually general and
+ * concern all supported chipsets, unless mentioned otherwise.
+ *
+ * Support could easily be added for the National Semiconductor LM76
+ * and Maxim MAX6633 and MAX6634 chips, which are mostly compatible
+ * with the LM92.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/i2c-sensor.h>
+
+
+/* The LM92 and MAX6635 have 2 two-state pins for address selection,
+ resulting in 4 possible addresses. */
+static unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b,
+ I2C_CLIENT_END };
+static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
+
+/* Insmod parameters */
+SENSORS_INSMOD_1(lm92);
+
+/* The LM92 registers */
+#define LM92_REG_CONFIG 0x01 /* 8-bit, RW */
+#define LM92_REG_TEMP 0x00 /* 16-bit, RO */
+#define LM92_REG_TEMP_HYST 0x02 /* 16-bit, RW */
+#define LM92_REG_TEMP_CRIT 0x03 /* 16-bit, RW */
+#define LM92_REG_TEMP_LOW 0x04 /* 16-bit, RW */
+#define LM92_REG_TEMP_HIGH 0x05 /* 16-bit, RW */
+#define LM92_REG_MAN_ID 0x07 /* 16-bit, RO, LM92 only */
+
+/* The LM92 uses signed 13-bit values with LSB = 0.0625 degree Celsius,
+ left-justified in 16-bit registers. No rounding is done, with such
+ a resolution it's just not worth it. Note that the MAX6635 doesn't
+ make use of the 4 lower bits for limits (i.e. effective resolution
+ for limits is 1 degree Celsius). */
+static inline int TEMP_FROM_REG(s16 reg)
+{
+ return reg / 8 * 625 / 10;
+}
+
+static inline s16 TEMP_TO_REG(int val)
+{
+ if (val <= -60000)
+ return -60000 * 10 / 625 * 8;
+ if (val >= 160000)
+ return 160000 * 10 / 625 * 8;
+ return val * 10 / 625 * 8;
+}
+
+/* Alarm flags are stored in the 3 LSB of the temperature register */
+static inline u8 ALARMS_FROM_REG(s16 reg)
+{
+ return reg & 0x0007;
+}
+
+/* Driver data (common to all clients) */
+static struct i2c_driver lm92_driver;
+
+/* Client data (each client gets its own) */
+struct lm92_data {
+ struct i2c_client client;
+ struct semaphore update_lock;
+ char valid; /* zero until following fields are valid */
+ unsigned long last_updated; /* in jiffies */
+
+ /* registers values */
+ s16 temp1_input, temp1_crit, temp1_min, temp1_max, temp1_hyst;
+};
+
+
+/*
+ * Sysfs attributes and callback functions
+ */
+
+static struct lm92_data *lm92_update_device(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm92_data *data = i2c_get_clientdata(client);
+
+ down(&data->update_lock);
+
+ if (time_after(jiffies, data->last_updated + HZ)
+ || !data->valid) {
+ dev_dbg(&client->dev, "Updating lm92 data\n");
+ data->temp1_input = swab16(i2c_smbus_read_word_data(client,
+ LM92_REG_TEMP));
+ data->temp1_hyst = swab16(i2c_smbus_read_word_data(client,
+ LM92_REG_TEMP_HYST));
+ data->temp1_crit = swab16(i2c_smbus_read_word_data(client,
+ LM92_REG_TEMP_CRIT));
+ data->temp1_min = swab16(i2c_smbus_read_word_data(client,
+ LM92_REG_TEMP_LOW));
+ data->temp1_max = swab16(i2c_smbus_read_word_data(client,
+ LM92_REG_TEMP_HIGH));
+
+ data->last_updated = jiffies;
+ data->valid = 1;
+ }
+
+ up(&data->update_lock);
+
+ return data;
+}
+
+#define show_temp(value) \
+static ssize_t show_##value(struct device *dev, char *buf) \
+{ \
+ struct lm92_data *data = lm92_update_device(dev); \
+ return sprintf(buf, "%d\n", TEMP_FROM_REG(data->value)); \
+}
+show_temp(temp1_input);
+show_temp(temp1_crit);
+show_temp(temp1_min);
+show_temp(temp1_max);
+
+#define set_temp(value, reg) \
+static ssize_t set_##value(struct device *dev, const char *buf, \
+ size_t count) \
+{ \
+ struct i2c_client *client = to_i2c_client(dev); \
+ struct lm92_data *data = i2c_get_clientdata(client); \
+ long val = simple_strtol(buf, NULL, 10); \
+ data->value = TEMP_TO_REG(val); \
+ i2c_smbus_write_word_data(client, reg, swab16(data->value)); \
+ return count; \
+}
+set_temp(temp1_crit, LM92_REG_TEMP_CRIT);
+set_temp(temp1_min, LM92_REG_TEMP_LOW);
+set_temp(temp1_max, LM92_REG_TEMP_HIGH);
+
+static ssize_t show_temp1_crit_hyst(struct device *dev, char *buf)
+{
+ struct lm92_data *data = lm92_update_device(dev);
+ return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp1_crit)
+ - TEMP_FROM_REG(data->temp1_hyst));
+}
+static ssize_t show_temp1_max_hyst(struct device *dev, char *buf)
+{
+ struct lm92_data *data = lm92_update_device(dev);
+ return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp1_max)
+ - TEMP_FROM_REG(data->temp1_hyst));
+}
+static ssize_t show_temp1_min_hyst(struct device *dev, char *buf)
+{
+ struct lm92_data *data = lm92_update_device(dev);
+ return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp1_min)
+ + TEMP_FROM_REG(data->temp1_hyst));
+}
+
+static ssize_t set_temp1_crit_hyst(struct device *dev, const char *buf,
+ size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm92_data *data = i2c_get_clientdata(client);
+ data->temp1_hyst = TEMP_FROM_REG(data->temp1_crit) -
+ simple_strtol(buf, NULL, 10);
+ i2c_smbus_write_word_data(client, LM92_REG_TEMP_HYST,
+ swab16(TEMP_TO_REG(data->temp1_hyst)));
+ return count;
+}
+
+static ssize_t show_alarms(struct device *dev, char *buf)
+{
+ struct lm92_data *data = lm92_update_device(dev);
+ return sprintf(buf, "%d\n", ALARMS_FROM_REG(data->temp1_input));
+}
+
+static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp1_input, NULL);
+static DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO, show_temp1_crit,
+ set_temp1_crit);
+static DEVICE_ATTR(temp1_crit_hyst, S_IWUSR | S_IRUGO, show_temp1_crit_hyst,
+ set_temp1_crit_hyst);
+static DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO, show_temp1_min,
+ set_temp1_min);
+static DEVICE_ATTR(temp1_min_hyst, S_IRUGO, show_temp1_min_hyst, NULL);
+static DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp1_max,
+ set_temp1_max);
+static DEVICE_ATTR(temp1_max_hyst, S_IRUGO, show_temp1_max_hyst, NULL);
+static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
+
+
+/*
+ * Detection and registration
+ */
+
+static void lm92_init_client(struct i2c_client *client)
+{
+ u8 config;
+
+ /* Start the conversions if needed */
+ config = i2c_smbus_read_byte_data(client, LM92_REG_CONFIG);
+ if (config & 0x01)
+ i2c_smbus_write_byte_data(client, LM92_REG_CONFIG,
+ config & 0xFE);
+}
+
+/* The MAX6635 has no identification register, so we have to use tricks
+ to identify it reliably. This is somewhat slow.
+ Note that we do NOT rely on the 2 MSB of the configuration register
+ always reading 0, as suggested by the datasheet, because it was once
+ reported not to be true. */
+static int max6635_check(struct i2c_client *client)
+{
+ u16 temp_low, temp_high, temp_hyst, temp_crit;
+ u8 conf;
+ int i;
+
+ /* No manufacturer ID register, so a read from this address will
+ always return the last read value. */
+ temp_low = i2c_smbus_read_word_data(client, LM92_REG_TEMP_LOW);
+ if (i2c_smbus_read_word_data(client, LM92_REG_MAN_ID) != temp_low)
+ return 0;
+ temp_high = i2c_smbus_read_word_data(client, LM92_REG_TEMP_HIGH);
+ if (i2c_smbus_read_word_data(client, LM92_REG_MAN_ID) != temp_high)
+ return 0;
+
+ /* Limits are stored as integer values (signed, 9-bit). */
+ if ((temp_low & 0x7f00) || (temp_high & 0x7f00))
+ return 0;
+ temp_hyst = i2c_smbus_read_word_data(client, LM92_REG_TEMP_HYST);
+ temp_crit = i2c_smbus_read_word_data(client, LM92_REG_TEMP_CRIT);
+ if ((temp_hyst & 0x7f00) || (temp_crit & 0x7f00))
+ return 0;
+
+ /* Registers addresses were found to cycle over 16-byte boundaries.
+ We don't test all registers with all offsets so as to save some
+ reads and time, but this should still be sufficient to dismiss
+ non-MAX6635 chips. */
+ conf = i2c_smbus_read_byte_data(client, LM92_REG_CONFIG);
+ for (i=16; i<96; i*=2) {
+ if (temp_hyst != i2c_smbus_read_word_data(client,
+ LM92_REG_TEMP_HYST + i - 16)
+ || temp_crit != i2c_smbus_read_word_data(client,
+ LM92_REG_TEMP_CRIT + i)
+ || temp_low != i2c_smbus_read_word_data(client,
+ LM92_REG_TEMP_LOW + i + 16)
+ || temp_high != i2c_smbus_read_word_data(client,
+ LM92_REG_TEMP_HIGH + i + 32)
+ || conf != i2c_smbus_read_byte_data(client,
+ LM92_REG_CONFIG + i))
+ return 0;
+ }
+
+ return 1;
+}
+
+/* The following function does more than just detection. If detection
+ succeeds, it also registers the new chip. */
+static int lm92_detect(struct i2c_adapter *adapter, int address, int kind)
+{
+ struct i2c_client *new_client;
+ struct lm92_data *data;
+ int err = 0;
+ char *name;
+
+ if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA
+ | I2C_FUNC_SMBUS_WORD_DATA))
+ goto exit;
+
+ if (!(data = kmalloc(sizeof(struct lm92_data), GFP_KERNEL))) {
+ err = -ENOMEM;
+ goto exit;
+ }
+ memset(data, 0, sizeof(struct lm92_data));
+
+ /* Fill in enough client fields so that we can read from the chip,
+ which is required for identication */
+ new_client = &data->client;
+ i2c_set_clientdata(new_client, data);
+ new_client->addr = address;
+ new_client->adapter = adapter;
+ new_client->driver = &lm92_driver;
+ new_client->flags = 0;
+
+ /* A negative kind means that the driver was loaded with no force
+ parameter (default), so we must identify the chip. */
+ if (kind < 0) {
+ u8 config = i2c_smbus_read_byte_data(new_client,
+ LM92_REG_CONFIG);
+ u16 man_id = i2c_smbus_read_word_data(new_client,
+ LM92_REG_MAN_ID);
+
+ if ((config & 0xe0) == 0x00
+ && man_id == 0x0180) {
+ pr_info("lm92: Found National Semiconductor LM92 chip\n");
+ kind = lm92;
+ } else
+ if (max6635_check(new_client)) {
+ pr_info("lm92: Found Maxim MAX6635 chip\n");
+ kind = lm92; /* No separate prefix */
+ }
+ else
+ goto exit_free;
+ } else
+ if (kind == 0) /* Default to an LM92 if forced */
+ kind = lm92;
+
+ /* Give it the proper name */
+ if (kind == lm92) {
+ name = "lm92";
+ } else { /* Supposedly cannot happen */
+ dev_dbg(&new_client->dev, "Kind out of range?\n");
+ goto exit_free;
+ }
+
+ /* Fill in the remaining client fields */
+ strlcpy(new_client->name, name, I2C_NAME_SIZE);
+ data->valid = 0;
+ init_MUTEX(&data->update_lock);
+
+ /* Tell the i2c subsystem a new client has arrived */
+ if ((err = i2c_attach_client(new_client)))
+ goto exit_free;
+
+ /* Initialize the chipset */
+ lm92_init_client(new_client);
+
+ /* Register sysfs hooks */
+ device_create_file(&new_client->dev, &dev_attr_temp1_input);
+ device_create_file(&new_client->dev, &dev_attr_temp1_crit);
+ device_create_file(&new_client->dev, &dev_attr_temp1_crit_hyst);
+ device_create_file(&new_client->dev, &dev_attr_temp1_min);
+ device_create_file(&new_client->dev, &dev_attr_temp1_min_hyst);
+ device_create_file(&new_client->dev, &dev_attr_temp1_max);
+ device_create_file(&new_client->dev, &dev_attr_temp1_max_hyst);
+ device_create_file(&new_client->dev, &dev_attr_alarms);
+
+ return 0;
+
+exit_free:
+ kfree(data);
+exit:
+ return err;
+}
+
+static int lm92_attach_adapter(struct i2c_adapter *adapter)
+{
+ if (!(adapter->class & I2C_CLASS_HWMON))
+ return 0;
+ return i2c_detect(adapter, &addr_data, lm92_detect);
+}
+
+static int lm92_detach_client(struct i2c_client *client)
+{
+ int err;
+
+ if ((err = i2c_detach_client(client))) {
+ dev_err(&client->dev, "Client deregistration failed, "
+ "client not detached.\n");
+ return err;
+ }
+
+ kfree(i2c_get_clientdata(client));
+ return 0;
+}
+
+
+/*
+ * Module and driver stuff
+ */
+
+static struct i2c_driver lm92_driver = {
+ .owner = THIS_MODULE,
+ .name = "lm92",
+ .id = I2C_DRIVERID_LM92,
+ .flags = I2C_DF_NOTIFY,
+ .attach_adapter = lm92_attach_adapter,
+ .detach_client = lm92_detach_client,
+};
+
+static int __init sensors_lm92_init(void)
+{
+ return i2c_add_driver(&lm92_driver);
+}
+
+static void __exit sensors_lm92_exit(void)
+{
+ i2c_del_driver(&lm92_driver);
+}
+
+MODULE_AUTHOR("Jean Delvare <[email protected]>");
+MODULE_DESCRIPTION("LM92/MAX6635 driver");
+MODULE_LICENSE("GPL");
+
+module_init(sensors_lm92_init);
+module_exit(sensors_lm92_exit);
ChangeSet 1.2322, 2005/03/31 14:05:14-08:00, [email protected]
[PATCH] i2c/i2c-ite: remove interruptible_sleep_on_timeout() usage
Replace deprecated interruptible_sleep_on_timeout() with direct
wait-queue usage. Patch is compile-tested, sort of; the driver does not build in
vanilla kernel either, but I don't seem to add any warnings..
Signed-off-by: Nishanth Aravamudan <[email protected]>
Signed-off-by: Domen Puncer <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/busses/i2c-ite.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff -Nru a/drivers/i2c/busses/i2c-ite.c b/drivers/i2c/busses/i2c-ite.c
--- a/drivers/i2c/busses/i2c-ite.c 2005-03-31 15:19:21 -08:00
+++ b/drivers/i2c/busses/i2c-ite.c 2005-03-31 15:19:21 -08:00
@@ -40,6 +40,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/init.h>
+#include <linux/wait.h>
#include <asm/irq.h>
#include <asm/io.h>
@@ -107,7 +108,7 @@
* IIC controller interrupts.
*/
static void iic_ite_waitforpin(void) {
-
+ DEFINE_WAIT(wait);
int timeout = 2;
long flags;
@@ -121,13 +122,15 @@
spin_lock_irqsave(&lock, flags);
if (iic_pending == 0) {
spin_unlock_irqrestore(&lock, flags);
- if (interruptible_sleep_on_timeout(&iic_wait, timeout*HZ)) {
+ prepare_to_wait(&iic_wait, &wait, TASK_INTERRUPTIBLE);
+ if (schedule_timeout(timeout*HZ)) {
spin_lock_irqsave(&lock, flags);
if (iic_pending == 1) {
iic_pending = 0;
}
spin_unlock_irqrestore(&lock, flags);
}
+ finish_wait(&iic_wait, &wait);
} else {
iic_pending = 0;
spin_unlock_irqrestore(&lock, flags);
ChangeSet 1.2335, 2005/03/31 14:09:20-08:00, [email protected]
[PATCH] I2C: Fix some i2c algorithm initialization
While searching for i2c_algorithm declarations missing their
.functionality member, I found three of them which were not properly
initialized. i2c-algo-ite and i2c_sibyte_algo do not use the C99
initialization syntax, and i2c-ibm_iic.c explicitely initializes NULL
members. Following patch puts some order in there.
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/algos/i2c-algo-ite.c | 13 +++++--------
drivers/i2c/algos/i2c-algo-sibyte.c | 13 +++++--------
drivers/i2c/busses/i2c-ibm_iic.c | 4 ----
3 files changed, 10 insertions(+), 20 deletions(-)
diff -Nru a/drivers/i2c/algos/i2c-algo-ite.c b/drivers/i2c/algos/i2c-algo-ite.c
--- a/drivers/i2c/algos/i2c-algo-ite.c 2005-03-31 15:17:46 -08:00
+++ b/drivers/i2c/algos/i2c-algo-ite.c 2005-03-31 15:17:46 -08:00
@@ -713,14 +713,11 @@
/* -----exported algorithm data: ------------------------------------- */
static struct i2c_algorithm iic_algo = {
- "ITE IIC algorithm",
- I2C_ALGO_IIC,
- iic_xfer, /* master_xfer */
- NULL, /* smbus_xfer */
- NULL, /* slave_xmit */
- NULL, /* slave_recv */
- algo_control, /* ioctl */
- iic_func, /* functionality */
+ .name = "ITE IIC algorithm",
+ .id = I2C_ALGO_IIC,
+ .master_xfer = iic_xfer,
+ .algo_control = algo_control, /* ioctl */
+ .functionality = iic_func,
};
diff -Nru a/drivers/i2c/algos/i2c-algo-sibyte.c b/drivers/i2c/algos/i2c-algo-sibyte.c
--- a/drivers/i2c/algos/i2c-algo-sibyte.c 2005-03-31 15:17:46 -08:00
+++ b/drivers/i2c/algos/i2c-algo-sibyte.c 2005-03-31 15:17:46 -08:00
@@ -136,14 +136,11 @@
/* -----exported algorithm data: ------------------------------------- */
static struct i2c_algorithm i2c_sibyte_algo = {
- "SiByte algorithm",
- I2C_ALGO_SIBYTE,
- NULL, /* master_xfer */
- smbus_xfer, /* smbus_xfer */
- NULL, /* slave_xmit */
- NULL, /* slave_recv */
- algo_control, /* ioctl */
- bit_func, /* functionality */
+ .name = "SiByte algorithm",
+ .id = I2C_ALGO_SIBYTE,
+ .smbus_xfer = smbus_xfer,
+ .algo_control = algo_control, /* ioctl */
+ .functionality = bit_func,
};
/*
diff -Nru a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
--- a/drivers/i2c/busses/i2c-ibm_iic.c 2005-03-31 15:17:46 -08:00
+++ b/drivers/i2c/busses/i2c-ibm_iic.c 2005-03-31 15:17:46 -08:00
@@ -630,10 +630,6 @@
.name = "IBM IIC algorithm",
.id = I2C_ALGO_OCP,
.master_xfer = iic_xfer,
- .smbus_xfer = NULL,
- .slave_send = NULL,
- .slave_recv = NULL,
- .algo_control = NULL,
.functionality = iic_func
};
ChangeSet 1.2347, 2005/03/31 14:31:59-08:00, [email protected]
[PATCH] i2c: add adt7461 chip support to lm90 driver's Kconfig entry
Hi Greg, James, all,
> > > Attached is another version of my adt7461 patch, for inclusion in
> > > the 2.6 tree. Reviewed by Jean.
>
> May we have an additional patch to Kconfig for this one?
Here it finally comes.
This simple patch adds a mention to the ADT7461 chip in Kconfig, now
that the lm90 driver supports it.
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/Kconfig | 3 +++
1 files changed, 3 insertions(+)
diff -Nru a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
--- a/drivers/i2c/chips/Kconfig 2005-03-31 15:16:17 -08:00
+++ b/drivers/i2c/chips/Kconfig 2005-03-31 15:16:17 -08:00
@@ -233,6 +233,9 @@
LM86, LM89 and LM99, Analog Devices ADM1032 and Maxim MAX6657 and
MAX6658 sensor chips.
+ The Analog Devices ADT7461 sensor chip is also supported, but only
+ if found in ADM1032 compatibility mode.
+
This driver can also be built as a module. If so, the module
will be called lm90.
ChangeSet 1.2325, 2005/03/31 14:06:09-08:00, [email protected]
[PATCH] I2C: Cleanup adm1021 unused defines
While working on the adm1021 driver, I found that this driver has a
number of unused (and useless) defines we could get rid of.
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/adm1021.c | 12 ------------
1 files changed, 12 deletions(-)
diff -Nru a/drivers/i2c/chips/adm1021.c b/drivers/i2c/chips/adm1021.c
--- a/drivers/i2c/chips/adm1021.c 2005-03-31 15:19:00 -08:00
+++ b/drivers/i2c/chips/adm1021.c 2005-03-31 15:19:00 -08:00
@@ -28,18 +28,6 @@
#include <linux/i2c-sensor.h>
-/* Registers */
-#define ADM1021_SYSCTL_TEMP 1200
-#define ADM1021_SYSCTL_REMOTE_TEMP 1201
-#define ADM1021_SYSCTL_DIE_CODE 1202
-#define ADM1021_SYSCTL_ALARMS 1203
-
-#define ADM1021_ALARM_TEMP_HIGH 0x40
-#define ADM1021_ALARM_TEMP_LOW 0x20
-#define ADM1021_ALARM_RTEMP_HIGH 0x10
-#define ADM1021_ALARM_RTEMP_LOW 0x08
-#define ADM1021_ALARM_RTEMP_NA 0x04
-
/* Addresses to scan */
static unsigned short normal_i2c[] = { 0x18, 0x19, 0x1a,
0x29, 0x2a, 0x2b,
ChangeSet 1.2327, 2005/03/31 14:06:47-08:00, [email protected]
[PATCH] I2C: Kill unused struct members in w83627hf driver
I just noticed that the pwmenable struct members in the w83627hf driver
are not used anywhere (and quite rightly so, as PWM cannot be disabled
in these chips as far as I know). Let's just get rid of them and save
some bytes of memory.
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/w83627hf.c | 5 -----
1 files changed, 5 deletions(-)
diff -Nru a/drivers/i2c/chips/w83627hf.c b/drivers/i2c/chips/w83627hf.c
--- a/drivers/i2c/chips/w83627hf.c 2005-03-31 15:18:44 -08:00
+++ b/drivers/i2c/chips/w83627hf.c 2005-03-31 15:18:44 -08:00
@@ -304,7 +304,6 @@
u32 beep_mask; /* Register encoding, combined */
u8 beep_enable; /* Boolean */
u8 pwm[3]; /* Register value */
- u8 pwmenable[3]; /* bool */
u16 sens[3]; /* 782D/783S only.
1 = pentium diode; 2 = 3904 diode;
3000-5000 = thermistor beta.
@@ -1316,10 +1315,6 @@
if ((type == w83697hf) && (i == 2))
break;
}
-
- data->pwmenable[0] = 1;
- data->pwmenable[1] = 1;
- data->pwmenable[2] = 1;
if(init) {
/* Enable temp2 */
ChangeSet 1.2330, 2005/03/31 14:07:43-08:00, [email protected]
[PATCH] I2C: group Intel on I2C Hardware Bus support
From an end-user perspective it is easy to miss the third Intel PIIX
entry on the menuconfig "I2C Hardware Bus support" screen.
Also the Intel 801 menu item does not mention ICH.
This trivial patch groups three Intel entries together, adds ICH to
menu item, and ICH5/ICH5R to the help section. Includes suggestions
from Jean Delvare.
Signed-off-by: Grant Coady <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/busses/Kconfig | 38 +++++++++++++++++++-------------------
1 files changed, 19 insertions(+), 19 deletions(-)
diff -Nru a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
--- a/drivers/i2c/busses/Kconfig 2005-03-31 15:18:22 -08:00
+++ b/drivers/i2c/busses/Kconfig 2005-03-31 15:18:22 -08:00
@@ -108,7 +108,7 @@
will be called i2c-hydra.
config I2C_I801
- tristate "Intel 801"
+ tristate "Intel 82801 (ICH)"
depends on I2C && PCI && EXPERIMENTAL
help
If you say yes to this option, support will be included for the Intel
@@ -119,7 +119,7 @@
82801BA
82801CA/CAM
82801DB
- 82801EB
+ 82801EB/ER (ICH5/ICH5R)
6300ESB
ICH6
ICH7
@@ -143,6 +143,23 @@
This driver can also be built as a module. If so, the module
will be called i2c-i810.
+config I2C_PIIX4
+ tristate "Intel PIIX4"
+ depends on I2C && PCI
+ help
+ If you say yes to this option, support will be included for the Intel
+ PIIX4 family of mainboard I2C interfaces. Specifically, the following
+ versions of the chipset are supported:
+ Intel PIIX4
+ Intel 440MX
+ Serverworks OSB4
+ Serverworks CSB5
+ Serverworks CSB6
+ SMSC Victory66
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-piix4.
+
config I2C_IBM_IIC
tristate "IBM PPC 4xx on-chip I2C interface"
depends on IBM_OCP && I2C
@@ -284,23 +301,6 @@
This support is also available as a module. If so, the module
will be called i2c-parport-light.
-
-config I2C_PIIX4
- tristate "Intel PIIX4"
- depends on I2C && PCI && EXPERIMENTAL
- help
- If you say yes to this option, support will be included for the Intel
- PIIX4 family of mainboard I2C interfaces. Specifically, the following
- versions of the chipset are supported:
- Intel PIIX4
- Intel 440MX
- Serverworks OSB4
- Serverworks CSB5
- Serverworks CSB6
- SMSC Victory66
-
- This driver can also be built as a module. If so, the module
- will be called i2c-piix4.
config I2C_PROSAVAGE
tristate "S3/VIA (Pro)Savage"
ChangeSet 1.2343, 2005/03/31 14:30:43-08:00, [email protected]
[PATCH] I2C: lsb in emc6d102 and adm1027
The attached patches add support for reading the lsb from the emc6d102
and change how they are read from the adm1027.
The lm85_update_device function decodes the LSBs to temp_ext and in_ext.
This strategy was suggested by Philip Pokorny.
The patch also changes some macros to use the SCALE macro. I think that
they become more readable this way.
Signed-off-by: Rafael ?vila de Esp?ndola <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/lm85.c | 100 ++++++++++++++++++++++++++++++++++++-----------
1 files changed, 77 insertions(+), 23 deletions(-)
diff -Nru a/drivers/i2c/chips/lm85.c b/drivers/i2c/chips/lm85.c
--- a/drivers/i2c/chips/lm85.c 2005-03-31 15:16:46 -08:00
+++ b/drivers/i2c/chips/lm85.c 2005-03-31 15:16:46 -08:00
@@ -37,7 +37,7 @@
static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
/* Insmod parameters */
-SENSORS_INSMOD_5(lm85b, lm85c, adm1027, adt7463, emc6d100);
+SENSORS_INSMOD_6(lm85b, lm85c, adm1027, adt7463, emc6d100, emc6d102);
/* The LM85 registers */
@@ -77,6 +77,7 @@
#define LM85_VERSTEP_ADT7463C 0x6A
#define LM85_VERSTEP_EMC6D100_A0 0x60
#define LM85_VERSTEP_EMC6D100_A1 0x61
+#define LM85_VERSTEP_EMC6D102 0x65
#define LM85_REG_CONFIG 0x40
@@ -113,9 +114,13 @@
#define EMC6D100_REG_ALARM3 0x7d
/* IN5, IN6 and IN7 */
-#define EMC6D100_REG_IN(nr) (0x70 + ((nr)-5))
-#define EMC6D100_REG_IN_MIN(nr) (0x73 + ((nr)-5) * 2)
-#define EMC6D100_REG_IN_MAX(nr) (0x74 + ((nr)-5) * 2)
+#define EMC6D100_REG_IN(nr) (0x70 + ((nr)-5))
+#define EMC6D100_REG_IN_MIN(nr) (0x73 + ((nr)-5) * 2)
+#define EMC6D100_REG_IN_MAX(nr) (0x74 + ((nr)-5) * 2)
+#define EMC6D102_REG_EXTEND_ADC1 0x85
+#define EMC6D102_REG_EXTEND_ADC2 0x86
+#define EMC6D102_REG_EXTEND_ADC3 0x87
+#define EMC6D102_REG_EXTEND_ADC4 0x88
#define LM85_ALARM_IN0 0x0001
#define LM85_ALARM_IN1 0x0002
@@ -140,35 +145,36 @@
these macros are called: arguments may be evaluated more than once.
*/
-/* IN are scaled 1.000 == 0xc0, mag = 3 */
-#define IN_TO_REG(val) (SENSORS_LIMIT((((val)*0xc0+500)/1000),0,255))
-#define INEXT_FROM_REG(val,ext) (((val)*1000 + (ext)*250 + 96)/0xc0)
-#define IN_FROM_REG(val) (INEXT_FROM_REG(val,0))
-
/* IN are scaled acording to built-in resistors */
static int lm85_scaling[] = { /* .001 Volts */
2500, 2250, 3300, 5000, 12000,
3300, 1500, 1800 /*EMC6D100*/
};
#define SCALE(val,from,to) (((val)*(to) + ((from)/2))/(from))
-#define INS_TO_REG(n,val) (SENSORS_LIMIT(SCALE(val,lm85_scaling[n],192),0,255))
-#define INSEXT_FROM_REG(n,val,ext) (SCALE((val)*4 + (ext),192*4,lm85_scaling[n]))
-#define INS_FROM_REG(n,val) (INSEXT_FROM_REG(n,val,0))
+
+#define INS_TO_REG(n,val) \
+ SENSORS_LIMIT(SCALE(val,lm85_scaling[n],192),0,255)
+
+#define INSEXT_FROM_REG(n,val,ext,scale) \
+ SCALE((val)*(scale) + (ext),192*(scale),lm85_scaling[n])
+
+#define INS_FROM_REG(n,val) INSEXT_FROM_REG(n,val,0,1)
/* FAN speed is measured using 90kHz clock */
#define FAN_TO_REG(val) (SENSORS_LIMIT( (val)<=0?0: 5400000/(val),0,65534))
#define FAN_FROM_REG(val) ((val)==0?-1:(val)==0xffff?0:5400000/(val))
/* Temperature is reported in .001 degC increments */
-#define TEMP_TO_REG(val) (SENSORS_LIMIT(((val)+500)/1000,-127,127))
-#define TEMPEXT_FROM_REG(val,ext) ((val)*1000 + (ext)*250)
-#define TEMP_FROM_REG(val) (TEMPEXT_FROM_REG(val,0))
-#define EXTTEMP_TO_REG(val) (SENSORS_LIMIT((val)/250,-127,127))
+#define TEMP_TO_REG(val) \
+ SENSORS_LIMIT(SCALE(val,1000,1),-127,127)
+#define TEMPEXT_FROM_REG(val,ext,scale) \
+ SCALE((val)*scale + (ext),scale,1000)
+#define TEMP_FROM_REG(val) \
+ TEMPEXT_FROM_REG(val,0,1)
#define PWM_TO_REG(val) (SENSORS_LIMIT(val,0,255))
#define PWM_FROM_REG(val) (val)
-#define EXT_FROM_REG(val,sensor) (((val)>>(sensor * 2))&0x03)
/* ZONEs have the following parameters:
* Limit (low) temp, 1. degC
@@ -356,7 +362,9 @@
u8 pwm[3]; /* Register value */
u8 spinup_ctl; /* Register encoding, combined */
u8 tach_mode; /* Register encoding, combined */
- u16 extend_adc; /* Register value */
+ u8 temp_ext[3]; /* Decoded values */
+ u8 in_ext[8]; /* Decoded values */
+ u8 adc_scale; /* ADC Extended bits scaling factor */
u8 fan_ppr; /* Register value */
u8 smooth[3]; /* Register encoding */
u8 vid; /* Register value */
@@ -537,7 +545,10 @@
static ssize_t show_in(struct device *dev, char *buf, int nr)
{
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf,"%d\n", INS_FROM_REG(nr, data->in[nr]) );
+ return sprintf( buf, "%d\n", INSEXT_FROM_REG(nr,
+ data->in[nr],
+ data->in_ext[nr],
+ data->adc_scale) );
}
static ssize_t show_in_min(struct device *dev, char *buf, int nr)
{
@@ -618,7 +629,9 @@
static ssize_t show_temp(struct device *dev, char *buf, int nr)
{
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf,"%d\n", TEMP_FROM_REG(data->temp[nr]) );
+ return sprintf(buf,"%d\n", TEMPEXT_FROM_REG(data->temp[nr],
+ data->temp_ext[nr],
+ data->adc_scale) );
}
static ssize_t show_temp_min(struct device *dev, char *buf, int nr)
{
@@ -1109,6 +1122,9 @@
*/
kind = emc6d100 ;
} else if( company == LM85_COMPANY_SMSC
+ && verstep == LM85_VERSTEP_EMC6D102) {
+ kind = emc6d102 ;
+ } else if( company == LM85_COMPANY_SMSC
&& (verstep & LM85_VERSTEP_VMASK) == LM85_VERSTEP_GENERIC) {
dev_err(&adapter->dev, "lm85: Detected SMSC chip\n");
dev_err(&adapter->dev, "lm85: Unrecognized version/stepping 0x%02x"
@@ -1144,6 +1160,8 @@
type_name = "adt7463";
} else if ( kind == emc6d100){
type_name = "emc6d100";
+ } else if ( kind == emc6d102 ) {
+ type_name = "emc6d102";
}
strlcpy(new_client->name, type_name, I2C_NAME_SIZE);
@@ -1261,7 +1279,6 @@
case LM85_REG_FAN_MIN(2) :
case LM85_REG_FAN_MIN(3) :
case LM85_REG_ALARM1 : /* Read both bytes at once */
- case ADM1027_REG_EXTEND_ADC1 : /* Read two bytes at once */
res = i2c_smbus_read_byte_data(client, reg) & 0xff ;
res |= i2c_smbus_read_byte_data(client, reg+1) << 8 ;
break ;
@@ -1365,10 +1382,25 @@
* more significant bits that are read later.
*/
if ( (data->type == adm1027) || (data->type == adt7463) ) {
- data->extend_adc =
- lm85_read_value(client, ADM1027_REG_EXTEND_ADC1);
+ int ext1 = lm85_read_value(client,
+ ADM1027_REG_EXTEND_ADC1);
+ int ext2 = lm85_read_value(client,
+ ADM1027_REG_EXTEND_ADC2);
+ int val = (ext1 << 8) + ext2;
+
+ for(i = 0; i <= 4; i++)
+ data->in_ext[i] = (val>>(i * 2))&0x03;
+
+ for(i = 0; i <= 2; i++)
+ data->temp_ext[i] = (val>>((i + 5) * 2))&0x03;
}
+ /* adc_scale is 2^(number of LSBs). There are 4 extra bits in
+ the emc6d102 and 2 in the adt7463 and adm1027. In all
+ other chips ext is always 0 and the value of scale is
+ irrelevant. So it is left in 4*/
+ data->adc_scale = (data->type == emc6d102 ) ? 16 : 4;
+
for (i = 0; i <= 4; ++i) {
data->in[i] =
lm85_read_value(client, LM85_REG_IN(i));
@@ -1405,6 +1437,28 @@
/* More alarm bits */
data->alarms |=
lm85_read_value(client, EMC6D100_REG_ALARM3) << 16;
+ } else if (data->type == emc6d102 ) {
+ /* Have to read LSB bits after the MSB ones because
+ the reading of the MSB bits has frozen the
+ LSBs (backward from the ADM1027).
+ */
+ int ext1 = lm85_read_value(client,
+ EMC6D102_REG_EXTEND_ADC1);
+ int ext2 = lm85_read_value(client,
+ EMC6D102_REG_EXTEND_ADC2);
+ int ext3 = lm85_read_value(client,
+ EMC6D102_REG_EXTEND_ADC3);
+ int ext4 = lm85_read_value(client,
+ EMC6D102_REG_EXTEND_ADC4);
+ data->in_ext[0] = ext3 & 0x0f;
+ data->in_ext[1] = ext4 & 0x0f;
+ data->in_ext[2] = (ext4 >> 4) & 0x0f;
+ data->in_ext[3] = (ext3 >> 4) & 0x0f;
+ data->in_ext[4] = (ext2 >> 4) & 0x0f;
+
+ data->temp_ext[0] = ext1 & 0x0f;
+ data->temp_ext[1] = ext2 & 0x0f;
+ data->temp_ext[2] = (ext1 >> 4) & 0x0f;
}
data->last_reading = jiffies ;
ChangeSet 1.2332, 2005/03/31 14:08:21-08:00, [email protected]
[PATCH] i2c: add adt7461 chip support to lm90 driver
i2c: add adt7461 chip support
The Analog Devices ADT7461 temperature sensor chip is compatible with
the lm90 device provided its extended temperature range is not
enabled. The chip will be ignored if the boot firmware enables
extended temperature range.
Also, since the adt7461 treats temp values <0 as 0 and >127 as 127,
the driver prevents temperature values outside the supported range
from being set.
Signed-off-by: James Chapman <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/lm90.c | 44 +++++++++++++++++++++++++++++++++++++++++---
1 files changed, 41 insertions(+), 3 deletions(-)
diff -Nru a/drivers/i2c/chips/lm90.c b/drivers/i2c/chips/lm90.c
--- a/drivers/i2c/chips/lm90.c 2005-03-31 15:18:08 -08:00
+++ b/drivers/i2c/chips/lm90.c 2005-03-31 15:18:08 -08:00
@@ -43,6 +43,14 @@
* variants. The extra address and features of the MAX6659 are not
* supported by this driver.
*
+ * This driver also supports the ADT7461 chip from Analog Devices but
+ * only in its "compatability mode". If an ADT7461 chip is found but
+ * is configured in non-compatible mode (where its temperature
+ * register values are decoded differently) it is ignored by this
+ * driver. Complete datasheet can be obtained from Analog's website
+ * at:
+ * http://products.analog.com/products/info.asp?product=ADT7461
+ *
* Since the LM90 was the first chipset supported by this driver, most
* comments will refer to this chipset, but are actually general and
* concern all supported chipsets, unless mentioned otherwise.
@@ -77,6 +85,7 @@
* LM86, LM89, LM90, LM99, ADM1032, MAX6657 and MAX6658 have address 0x4c.
* LM89-1, and LM99-1 have address 0x4d.
* MAX6659 can have address 0x4c, 0x4d or 0x4e (unsupported).
+ * ADT7461 always has address 0x4c.
*/
static unsigned short normal_i2c[] = { 0x4c, 0x4d, I2C_CLIENT_END };
@@ -86,7 +95,7 @@
* Insmod parameters
*/
-SENSORS_INSMOD_5(lm90, adm1032, lm99, lm86, max6657);
+SENSORS_INSMOD_6(lm90, adm1032, lm99, lm86, max6657, adt7461);
/*
* The LM90 registers
@@ -148,6 +157,19 @@
#define HYST_TO_REG(val) ((val) <= 0 ? 0 : (val) >= 30500 ? 31 : \
((val) + 500) / 1000)
+/*
+ * ADT7461 is almost identical to LM90 except that attempts to write
+ * values that are outside the range 0 < temp < 127 are treated as
+ * the boundary value.
+ */
+
+#define TEMP1_TO_REG_ADT7461(val) ((val) <= 0 ? 0 : \
+ (val) >= 127000 ? 127 : \
+ ((val) + 500) / 1000)
+#define TEMP2_TO_REG_ADT7461(val) ((val) <= 0 ? 0 : \
+ (val) >= 127750 ? 0x7FC0 : \
+ ((val) + 125) / 250 * 64)
+
/*
* Functions declaration
*/
@@ -181,6 +203,7 @@
struct semaphore update_lock;
char valid; /* zero until following fields are valid */
unsigned long last_updated; /* in jiffies */
+ int kind;
/* registers values */
s8 temp_input1, temp_low1, temp_high1; /* local */
@@ -216,7 +239,10 @@
struct i2c_client *client = to_i2c_client(dev); \
struct lm90_data *data = i2c_get_clientdata(client); \
long val = simple_strtol(buf, NULL, 10); \
- data->value = TEMP1_TO_REG(val); \
+ if (data->kind == adt7461) \
+ data->value = TEMP1_TO_REG_ADT7461(val); \
+ else \
+ data->value = TEMP1_TO_REG(val); \
i2c_smbus_write_byte_data(client, reg, data->value); \
return count; \
}
@@ -227,7 +253,10 @@
struct i2c_client *client = to_i2c_client(dev); \
struct lm90_data *data = i2c_get_clientdata(client); \
long val = simple_strtol(buf, NULL, 10); \
- data->value = TEMP2_TO_REG(val); \
+ if (data->kind == adt7461) \
+ data->value = TEMP2_TO_REG_ADT7461(val); \
+ else \
+ data->value = TEMP2_TO_REG(val); \
i2c_smbus_write_byte_data(client, regh, data->value >> 8); \
i2c_smbus_write_byte_data(client, regl, data->value & 0xff); \
return count; \
@@ -381,6 +410,12 @@
&& (reg_config1 & 0x3F) == 0x00
&& reg_convrate <= 0x0A) {
kind = adm1032;
+ } else
+ if (address == 0x4c
+ && chip_id == 0x51 /* ADT7461 */
+ && (reg_config1 & 0x1F) == 0x00 /* check compat mode */
+ && reg_convrate <= 0x0A) {
+ kind = adt7461;
}
} else
if (man_id == 0x4D) { /* Maxim */
@@ -418,11 +453,14 @@
name = "lm86";
} else if (kind == max6657) {
name = "max6657";
+ } else if (kind == adt7461) {
+ name = "adt7461";
}
/* We can fill in the remaining client fields */
strlcpy(new_client->name, name, I2C_NAME_SIZE);
data->valid = 0;
+ data->kind = kind;
init_MUTEX(&data->update_lock);
/* Tell the I2C layer a new client has arrived */
ChangeSet 1.2340, 2005/03/31 14:29:46-08:00, [email protected]
[PATCH] I2C: busses documentation update 1 of 2
This patch just moves i2c-parport file to busses directory.
Patch for other busses documentation will follow.
Signed-off-by: Rudolf Marek <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Documentation/i2c/i2c-parport | 156 -----------------------------------
Documentation/i2c/busses/i2c-parport | 156 +++++++++++++++++++++++++++++++++++
2 files changed, 156 insertions(+), 156 deletions(-)
diff -Nru a/Documentation/i2c/busses/i2c-parport b/Documentation/i2c/busses/i2c-parport
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/Documentation/i2c/busses/i2c-parport 2005-03-31 15:17:11 -08:00
@@ -0,0 +1,156 @@
+==================
+i2c-parport driver
+==================
+
+2004-07-06, Jean Delvare
+
+This is a unified driver for several i2c-over-parallel-port adapters,
+such as the ones made by Philips, Velleman or ELV. This driver is
+meant as a replacement for the older, individual drivers:
+ * i2c-philips-par
+ * i2c-elv
+ * i2c-velleman
+ * video/i2c-parport (NOT the same as this one, dedicated to home brew
+ teletext adapters)
+
+It currently supports the following devices:
+ * Philips adapter
+ * home brew teletext adapter
+ * Velleman K8000 adapter
+ * ELV adapter
+ * Analog Devices evaluation boards (ADM1025, ADM1030, ADM1031, ADM1032)
+
+These devices use different pinout configurations, so you have to tell
+the driver what you have, using the type module parameter. There is no
+way to autodetect the devices. Support for different pinout configurations
+can be easily added when needed.
+
+
+Building your own adapter
+-------------------------
+
+If you want to build you own i2c-over-parallel-port adapter, here is
+a sample electronics schema (credits go to Sylvain Munaut):
+
+Device PC
+Side ___________________Vdd (+) Side
+ | | |
+ --- --- ---
+ | | | | | |
+ |R| |R| |R|
+ | | | | | |
+ --- --- ---
+ | | |
+ | | /| |
+SCL ----------x--------o |-----------x------------------- pin 2
+ | \| | |
+ | | |
+ | |\ | |
+SDA ----------x----x---| o---x--------------------------- pin 13
+ | |/ |
+ | |
+ | /| |
+ ---------o |----------------x-------------- pin 3
+ \| | |
+ | |
+ --- ---
+ | | | |
+ |R| |R|
+ | | | |
+ --- ---
+ | |
+ ### ###
+ GND GND
+
+Remarks:
+ - This is the exact pinout and electronics used on the Analog Devices
+ evaluation boards.
+ /|
+ - All inverters -o |- must be 74HC05, they must be open collector output.
+ \|
+ - All resitors are 10k.
+ - Pins 18-25 of the parallel port connected to GND.
+ - Pins 4-9 (D2-D7) could be used as VDD is the driver drives them high.
+ The ADM1032 evaluation board uses D4-D7. Beware that the amount of
+ current you can draw from the parallel port is limited. Also note that
+ all connected lines MUST BE driven at the same state, else you'll short
+ circuit the output buffers! So plugging the I2C adapter after loading
+ the i2c-parport module might be a good safety since data line state
+ prior to init may be unknown.
+ - This is 5V!
+ - Obviously you cannot read SCL (so it's not really standard-compliant).
+ Pretty easy to add, just copy the SDA part and use another input pin.
+ That would give (ELV compatible pinout):
+
+
+Device PC
+Side ______________________________Vdd (+) Side
+ | | | |
+ --- --- --- ---
+ | | | | | | | |
+ |R| |R| |R| |R|
+ | | | | | | | |
+ --- --- --- ---
+ | | | |
+ | | |\ | |
+SCL ----------x--------x--| o---x------------------------ pin 15
+ | | |/ |
+ | | |
+ | | /| |
+ | ---o |-------------x-------------- pin 2
+ | \| | |
+ | | |
+ | | |
+ | |\ | |
+SDA ---------------x---x--| o--------x------------------- pin 10
+ | |/ |
+ | |
+ | /| |
+ ---o |------------------x--------- pin 3
+ \| | |
+ | |
+ --- ---
+ | | | |
+ |R| |R|
+ | | | |
+ --- ---
+ | |
+ ### ###
+ GND GND
+
+
+If possible, you should use the same pinout configuration as existing
+adapters do, so you won't even have to change the code.
+
+
+Similar (but different) drivers
+-------------------------------
+
+This driver is NOT the same as the i2c-pport driver found in the i2c package.
+The i2c-pport driver makes use of modern parallel port features so that
+you don't need additional electronics. It has other restrictions however, and
+was not ported to Linux 2.6 (yet).
+
+This driver is also NOT the same as the i2c-pcf-epp driver found in the
+lm_sensors package. The i2c-pcf-epp driver doesn't use the parallel port
+as an I2C bus directly. Instead, it uses it to control an external I2C bus
+master. That driver was not ported to Linux 2.6 (yet) either.
+
+
+Legacy documentation for Velleman adapter
+-----------------------------------------
+
+Useful links:
+Velleman http://www.velleman.be/
+Velleman K8000 Howto http://howto.htlw16.ac.at/k8000-howto.html
+
+The project has lead to new libs for the Velleman K8000 and K8005:
+ LIBK8000 v1.99.1 and LIBK8005 v0.21
+With these libs, you can control the K8000 interface card and the K8005
+stepper motor card with the simple commands which are in the original
+Velleman software, like SetIOchannel, ReadADchannel, SendStepCCWFull and
+many more, using /dev/velleman.
+ http://home.wanadoo.nl/hihihi/libk8000.htm
+ http://home.wanadoo.nl/hihihi/libk8005.htm
+ http://struyve.mine.nu:8080/index.php?block=k8000
+ http://sourceforge.net/projects/libk8005/
diff -Nru a/Documentation/i2c/i2c-parport b/Documentation/i2c/i2c-parport
--- a/Documentation/i2c/i2c-parport 2005-03-31 15:17:11 -08:00
+++ /dev/null Wed Dec 31 16:00:00 196900
@@ -1,156 +0,0 @@
-==================
-i2c-parport driver
-==================
-
-2004-07-06, Jean Delvare
-
-This is a unified driver for several i2c-over-parallel-port adapters,
-such as the ones made by Philips, Velleman or ELV. This driver is
-meant as a replacement for the older, individual drivers:
- * i2c-philips-par
- * i2c-elv
- * i2c-velleman
- * video/i2c-parport (NOT the same as this one, dedicated to home brew
- teletext adapters)
-
-It currently supports the following devices:
- * Philips adapter
- * home brew teletext adapter
- * Velleman K8000 adapter
- * ELV adapter
- * Analog Devices evaluation boards (ADM1025, ADM1030, ADM1031, ADM1032)
-
-These devices use different pinout configurations, so you have to tell
-the driver what you have, using the type module parameter. There is no
-way to autodetect the devices. Support for different pinout configurations
-can be easily added when needed.
-
-
-Building your own adapter
--------------------------
-
-If you want to build you own i2c-over-parallel-port adapter, here is
-a sample electronics schema (credits go to Sylvain Munaut):
-
-Device PC
-Side ___________________Vdd (+) Side
- | | |
- --- --- ---
- | | | | | |
- |R| |R| |R|
- | | | | | |
- --- --- ---
- | | |
- | | /| |
-SCL ----------x--------o |-----------x------------------- pin 2
- | \| | |
- | | |
- | |\ | |
-SDA ----------x----x---| o---x--------------------------- pin 13
- | |/ |
- | |
- | /| |
- ---------o |----------------x-------------- pin 3
- \| | |
- | |
- --- ---
- | | | |
- |R| |R|
- | | | |
- --- ---
- | |
- ### ###
- GND GND
-
-Remarks:
- - This is the exact pinout and electronics used on the Analog Devices
- evaluation boards.
- /|
- - All inverters -o |- must be 74HC05, they must be open collector output.
- \|
- - All resitors are 10k.
- - Pins 18-25 of the parallel port connected to GND.
- - Pins 4-9 (D2-D7) could be used as VDD is the driver drives them high.
- The ADM1032 evaluation board uses D4-D7. Beware that the amount of
- current you can draw from the parallel port is limited. Also note that
- all connected lines MUST BE driven at the same state, else you'll short
- circuit the output buffers! So plugging the I2C adapter after loading
- the i2c-parport module might be a good safety since data line state
- prior to init may be unknown.
- - This is 5V!
- - Obviously you cannot read SCL (so it's not really standard-compliant).
- Pretty easy to add, just copy the SDA part and use another input pin.
- That would give (ELV compatible pinout):
-
-
-Device PC
-Side ______________________________Vdd (+) Side
- | | | |
- --- --- --- ---
- | | | | | | | |
- |R| |R| |R| |R|
- | | | | | | | |
- --- --- --- ---
- | | | |
- | | |\ | |
-SCL ----------x--------x--| o---x------------------------ pin 15
- | | |/ |
- | | |
- | | /| |
- | ---o |-------------x-------------- pin 2
- | \| | |
- | | |
- | | |
- | |\ | |
-SDA ---------------x---x--| o--------x------------------- pin 10
- | |/ |
- | |
- | /| |
- ---o |------------------x--------- pin 3
- \| | |
- | |
- --- ---
- | | | |
- |R| |R|
- | | | |
- --- ---
- | |
- ### ###
- GND GND
-
-
-If possible, you should use the same pinout configuration as existing
-adapters do, so you won't even have to change the code.
-
-
-Similar (but different) drivers
--------------------------------
-
-This driver is NOT the same as the i2c-pport driver found in the i2c package.
-The i2c-pport driver makes use of modern parallel port features so that
-you don't need additional electronics. It has other restrictions however, and
-was not ported to Linux 2.6 (yet).
-
-This driver is also NOT the same as the i2c-pcf-epp driver found in the
-lm_sensors package. The i2c-pcf-epp driver doesn't use the parallel port
-as an I2C bus directly. Instead, it uses it to control an external I2C bus
-master. That driver was not ported to Linux 2.6 (yet) either.
-
-
-Legacy documentation for Velleman adapter
------------------------------------------
-
-Useful links:
-Velleman http://www.velleman.be/
-Velleman K8000 Howto http://howto.htlw16.ac.at/k8000-howto.html
-
-The project has lead to new libs for the Velleman K8000 and K8005:
- LIBK8000 v1.99.1 and LIBK8005 v0.21
-With these libs, you can control the K8000 interface card and the K8005
-stepper motor card with the simple commands which are in the original
-Velleman software, like SetIOchannel, ReadADchannel, SendStepCCWFull and
-many more, using /dev/velleman.
- http://home.wanadoo.nl/hihihi/libk8000.htm
- http://home.wanadoo.nl/hihihi/libk8005.htm
- http://struyve.mine.nu:8080/index.php?block=k8000
- http://sourceforge.net/projects/libk8005/
ChangeSet 1.2323, 2005/03/31 14:05:31-08:00, [email protected]
[PATCH] i2c/i2c-elektor: remove interruptible_sleep_on_timeout() usage
Replace deprecated interruptible_sleep_on_timeout() with direct
wait-queue usage. Patch is compile-tested.
Signed-off-by: Nishanth Aravamudan <[email protected]>
Signed-off-by: Domen Puncer <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/busses/i2c-elektor.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff -Nru a/drivers/i2c/busses/i2c-elektor.c b/drivers/i2c/busses/i2c-elektor.c
--- a/drivers/i2c/busses/i2c-elektor.c 2005-03-31 15:19:14 -08:00
+++ b/drivers/i2c/busses/i2c-elektor.c 2005-03-31 15:19:14 -08:00
@@ -110,7 +110,7 @@
}
static void pcf_isa_waitforpin(void) {
-
+ DEFINE_WAIT(wait);
int timeout = 2;
long flags;
@@ -118,14 +118,15 @@
spin_lock_irqsave(&lock, flags);
if (pcf_pending == 0) {
spin_unlock_irqrestore(&lock, flags);
- if (interruptible_sleep_on_timeout(&pcf_wait,
- timeout*HZ)) {
+ prepare_to_wait(&pcf_wait, &wait, TASK_INTERRUPTIBLE);
+ if (schedule_timeout(timeout*HZ)) {
spin_lock_irqsave(&lock, flags);
if (pcf_pending == 1) {
pcf_pending = 0;
}
spin_unlock_irqrestore(&lock, flags);
}
+ finish_wait(&pcf_wait, &wait);
} else {
pcf_pending = 0;
spin_unlock_irqrestore(&lock, flags);
ChangeSet 1.2334, 2005/03/31 14:09:00-08:00, [email protected]
[PATCH] I2C: Fix breakage in m41t00 i2c rtc driver
Remove setting of deleted i2c_client structure member.
The latest include/linux/i2c.h:i2c_client structure no longer has an
'id' member. This patch removes the setting of that no longer existing
member.
Signed-off-by: Mark A. Greer <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/m41t00.c | 1 -
1 files changed, 1 deletion(-)
diff -Nru a/drivers/i2c/chips/m41t00.c b/drivers/i2c/chips/m41t00.c
--- a/drivers/i2c/chips/m41t00.c 2005-03-31 15:17:53 -08:00
+++ b/drivers/i2c/chips/m41t00.c 2005-03-31 15:17:53 -08:00
@@ -184,7 +184,6 @@
memset(client, 0, sizeof(struct i2c_client));
strncpy(client->name, M41T00_DRV_NAME, I2C_NAME_SIZE);
- client->id = m41t00_driver.id;
client->flags = I2C_DF_NOTIFY;
client->addr = addr;
client->adapter = adap;
ChangeSet 1.2349, 2005/03/31 14:32:36-08:00, [email protected]
[PATCH] I2C: Fix indentation of lm87 driver
This trivial patch fixes indentation in the lm87 driver. I need this
'cause I'll soon post patches affecting these portions of code, and I'd
like these patches to be easily readable.
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/chips/lm87.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff -Nru a/drivers/i2c/chips/lm87.c b/drivers/i2c/chips/lm87.c
--- a/drivers/i2c/chips/lm87.c 2005-03-31 15:16:03 -08:00
+++ b/drivers/i2c/chips/lm87.c 2005-03-31 15:16:03 -08:00
@@ -317,20 +317,20 @@
static void set_temp_low(struct device *dev, const char *buf, int nr)
{
- struct i2c_client *client = to_i2c_client(dev);
- struct lm87_data *data = i2c_get_clientdata(client);
- long val = simple_strtol(buf, NULL, 10);
- data->temp_low[nr] = TEMP_TO_REG(val);
- lm87_write_value(client, LM87_REG_TEMP_LOW[nr], data->temp_low[nr]);
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm87_data *data = i2c_get_clientdata(client);
+ long val = simple_strtol(buf, NULL, 10);
+ data->temp_low[nr] = TEMP_TO_REG(val);
+ lm87_write_value(client, LM87_REG_TEMP_LOW[nr], data->temp_low[nr]);
}
static void set_temp_high(struct device *dev, const char *buf, int nr)
{
- struct i2c_client *client = to_i2c_client(dev);
- struct lm87_data *data = i2c_get_clientdata(client);
- long val = simple_strtol(buf, NULL, 10);
- data->temp_high[nr] = TEMP_TO_REG(val);
- lm87_write_value(client, LM87_REG_TEMP_HIGH[nr], data->temp_high[nr]);
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm87_data *data = i2c_get_clientdata(client);
+ long val = simple_strtol(buf, NULL, 10);
+ data->temp_high[nr] = TEMP_TO_REG(val);
+ lm87_write_value(client, LM87_REG_TEMP_HIGH[nr], data->temp_high[nr]);
}
#define set_temp(offset) \
ChangeSet 1.2348, 2005/03/31 14:32:17-08:00, [email protected]
[PATCH] I2C: Fix broken force parameter handling
I just noticed a nasty bug in the way the "force" parameter is handled
for non-sensors i2c chip drivers. The "force" parameter supposedy is a
list of adapter, address *pairs* where supported chips are
unquestionably assumed to be. However, after handling one pair, the i2c
core code searches for the next one *three* values later, not two. So
with the current code, the second and third pairs wouldn't be properly
handled. The fourth one would be, and so on.
As a side note, this questions the need of an array parameter handling
up to 24 of such pairs, when obviously nobody ever required more than
one for the past 6 years.
Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
drivers/i2c/i2c-core.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
diff -Nru a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
--- a/drivers/i2c/i2c-core.c 2005-03-31 15:16:10 -08:00
+++ b/drivers/i2c/i2c-core.c 2005-03-31 15:16:10 -08:00
@@ -715,7 +715,7 @@
at all */
found = 0;
- for (i = 0; !found && (address_data->force[i] != I2C_CLIENT_END); i += 3) {
+ for (i = 0; !found && (address_data->force[i] != I2C_CLIENT_END); i += 2) {
if (((adap_id == address_data->force[i]) ||
(address_data->force[i] == ANY_I2C_BUS)) &&
(addr == address_data->force[i+1])) {
On Thu, Mar 31, 2005 at 03:23:11PM -0800, Greg KH wrote:
> ChangeSet 1.2331, 2005/03/31 14:08:02-08:00, [email protected]
>
> [PATCH] i2c: new driver for ds1337 RTC
Hi,
I know it is bad time to send any patches, but lets try anyway :)
My embedded device is using DS1339 I2C RTC clock which differs from
DS1337 only in one register at address 10h which enables battery charge.
Originaly I was using my own driver, but decided to extend existing
one. Chip detection is done automaticaly, see ds1337_is_ds1339 function
and comment above. While playing with code, few things seemed strange to
me, so here is comment:
* driver is using adapter->algo->master_xfer function without checking
it is present and without holding bus lock. That looks insane to me.
fixed.
* BCD_TO_BIN is inteded to use this way: BCD_TO_BIN(val); Drive changed
to use BCD2BIN.
* Changed dev_dbg to print "dev" of device this driver is for not
adapter's dev.
* Changed get/set time to be compatible with other RTC drivers, ie.
epoch is handled separately. That should be done in RTC interface
part [*].
Driver is tested and works with DS1339, detection algoritm was tested
with extending address space to 18 bytes in DS1339. Please test on
DS1337 as well. Comments and suggestions welcome.
Now questions. It seems most I2C RTC drivers are based on Russell King's
drivers/acorn/char/pcf8583.c, but lacks RTC interface part [*] found in
drivers/acorn/char/i2c.c. How are they intended to be used? Is there any
generic interface to I2C RTC drivers? (Shall I write one? ;-))
Best regards,
ladis
===== drivers/i2c/chips/ds1337.c 1.1 vs edited =====
--- 1.1/drivers/i2c/chips/ds1337.c 2005-03-31 22:58:08 +02:00
+++ edited/drivers/i2c/chips/ds1337.c 2005-04-07 11:28:06 +02:00
@@ -2,15 +2,16 @@
* linux/drivers/i2c/chips/ds1337.c
*
* Copyright (C) 2005 James Chapman <[email protected]>
+ * Copyright (C) 2005 Ladislav Michl <[email protected]>
*
- * based on linux/drivers/acron/char/pcf8583.c
+ * based on linux/drivers/acorn/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
+ * Driver for Dallas Semiconductor DS1337 and DS1339 real time clock chip
*/
#include <linux/config.h>
@@ -25,12 +26,13 @@
#include <linux/list.h>
/* Device registers */
-#define DS1337_REG_HOUR 2
-#define DS1337_REG_DAY 3
-#define DS1337_REG_DATE 4
-#define DS1337_REG_MONTH 5
-#define DS1337_REG_CONTROL 14
-#define DS1337_REG_STATUS 15
+#define DS1337_REG_HOUR 0x02
+#define DS1337_REG_DAY 0x03
+#define DS1337_REG_DATE 0x04
+#define DS1337_REG_MONTH 0x05
+#define DS1337_REG_CONTROL 0x0e
+#define DS1337_REG_STATUS 0x0f
+#define DS1339_REG_CHARGE 0x10
/* FIXME - how do we export these interface constants? */
#define DS1337_GET_DATE 0
@@ -42,7 +44,7 @@
static unsigned short normal_i2c[] = { 0x68, I2C_CLIENT_END };
static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
-SENSORS_INSMOD_1(ds1337);
+SENSORS_INSMOD_2(ds1337, ds1339);
static int ds1337_attach_adapter(struct i2c_adapter *adapter);
static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind);
@@ -78,6 +80,8 @@
static int ds1337_id;
static LIST_HEAD(ds1337_clients);
+static int ds1339_charge = 0xaa;
+
static inline int ds1337_read(struct i2c_client *client, u8 reg, u8 *value)
{
s32 tmp = i2c_smbus_read_byte_data(client, reg);
@@ -90,119 +94,100 @@
return 0;
}
-/*
- * Chip access functions
+/*
+ * DS1339 has Trickle Charge register at address 10h. During a multibyte
+ * access, when the address pointer reaches the end of the register space,
+ * it wraps around to location 00h.
+ * We read 17 bytes from device and compare first and last one. If they are
+ * same it is most likely DS1337 chip.
*/
-static int ds1337_get_datetime(struct i2c_client *client, struct rtc_time *dt)
+static int ds1337_is_ds1339(struct i2c_client *client)
{
- 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__);
+ unsigned char buf[17] = { 0, }, addr[1] = { 0 };
+ struct i2c_msg msgs[2] = {
+ { client->addr, 0, 1, addr },
+ { client->addr, I2C_M_RD, 17, buf }
+ };
+ int result = i2c_transfer(client->adapter, msgs, 2);
- 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];
+ if (result < 0) {
+ dev_err(&client->dev, "error reading data! %d\n", result);
+ return 0;
+ } else
+ return (buf[0] == buf[16]) ? 0 : 1;
+}
- result = client->adapter->algo->master_xfer(client->adapter,
- &msg[0], 2);
+/*
+ * Chip access functions
+ */
+static int ds1337_get_datetime(struct i2c_client *client, struct rtc_time *tm)
+{
+ unsigned char buf[7] = { 0, }, addr[1] = { 0 };
+ struct i2c_msg msgs[2] = {
+ { client->addr, 0, 1, addr },
+ { client->addr, I2C_M_RD, 7, buf }
+ };
+ int result = i2c_transfer(client->adapter, msgs, 2);
- dev_dbg(&client->adapter->dev,
+ dev_dbg(&client->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 (result < 0) {
+ dev_err(&client->dev, "error reading data! %d\n", result);
+ result = -EIO;
+ } else {
+ tm->tm_sec = BCD2BIN(buf[0]);
+ tm->tm_min = BCD2BIN(buf[1]);
+ tm->tm_hour = BCD2BIN(buf[2] & 0x3f);
+ tm->tm_wday = BCD2BIN(buf[3]);
+ tm->tm_mday = BCD2BIN(buf[4]);
+ tm->tm_mon = BCD2BIN(buf[5] & 0x7f) - 1;
+ tm->tm_year = BCD2BIN(buf[6]);
if (buf[5] & 0x80)
- dt->tm_year += 100;
+ tm->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;
+ dev_dbg(&client->dev, "%s: secs=%d, mins=%d, hours=%d, "
+ "mday=%d, mon=%d, year=%d, wday=%d\n", __FUNCTION__,
+ tm->tm_sec, tm->tm_min, tm->tm_hour,
+ tm->tm_mday, tm->tm_mon, tm->tm_year, tm->tm_wday);
+
+ result = 0;
}
return result;
}
-static int ds1337_set_datetime(struct i2c_client *client, struct rtc_time *dt)
+static int ds1337_set_datetime(struct i2c_client *client, struct rtc_time *tm)
{
- struct ds1337_data *data = i2c_get_clientdata(client);
+ unsigned char buf[8];
int result;
- u8 buf[8];
- u8 val;
- struct i2c_msg msg[1];
-
- if (!dt) {
- dev_dbg(&client->adapter->dev, "%s: EINVAL: dt=NULL\n",
- __FUNCTION__);
-
- return -EINVAL;
- }
- dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, hours=%d, "
+ dev_dbg(&client->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);
+ tm->tm_sec, tm->tm_min, tm->tm_hour,
+ tm->tm_mday, tm->tm_mon, tm->tm_year, tm->tm_wday);
buf[0] = 0; /* reg offset */
- buf[1] = BIN_TO_BCD(dt->tm_sec);
- buf[2] = BIN_TO_BCD(dt->tm_min);
- buf[3] = BIN_TO_BCD(dt->tm_hour) | (1 << 6);
- buf[4] = BIN_TO_BCD(dt->tm_wday) + 1;
- buf[5] = BIN_TO_BCD(dt->tm_mday);
- buf[6] = BIN_TO_BCD(dt->tm_mon);
- if (dt->tm_year >= 2000) {
- val = dt->tm_year - 2000;
+ buf[1] = BIN2BCD(tm->tm_sec);
+ buf[2] = BIN2BCD(tm->tm_min);
+ buf[3] = BIN2BCD(tm->tm_hour) | (1 << 6);
+ buf[4] = BIN2BCD(tm->tm_wday);
+ buf[5] = BIN2BCD(tm->tm_mday);
+ buf[6] = BIN2BCD(tm->tm_mon + 1);
+ if (tm->tm_year >= 100) {
+ tm->tm_year -= 100;
buf[6] |= (1 << 7);
- } else {
- val = dt->tm_year - 1900;
}
- buf[7] = BIN_TO_BCD(val);
-
- msg[0].addr = client->addr;
- msg[0].flags = 0;
- msg[0].len = sizeof(buf);
- msg[0].buf = &buf[0];
+ buf[7] = BIN2BCD(tm->tm_year);
- result = client->adapter->algo->master_xfer(client->adapter,
- &msg[0], 1);
+ result = i2c_master_send(client, (char *)buf, sizeof(buf));
if (result < 0) {
- dev_err(&client->adapter->dev, "ds1337[%d]: error "
- "writing data! %d\n", data->id, result);
+ dev_err(&client->dev, "error writing data! %d\n", result);
result = -EIO;
- } else {
+ } else
result = 0;
- }
return result;
}
@@ -210,7 +195,14 @@
static int ds1337_command(struct i2c_client *client, unsigned int cmd,
void *arg)
{
- dev_dbg(&client->adapter->dev, "%s: cmd=%d\n", __FUNCTION__, cmd);
+ dev_dbg(&client->dev, "%s: cmd=%d\n", __FUNCTION__, cmd);
+
+ if (!arg) {
+ dev_dbg(&client->dev, "%s: EINVAL: arg=NULL\n",
+ __FUNCTION__);
+
+ return -EINVAL;
+ }
switch (cmd) {
case DS1337_GET_DATE:
@@ -254,10 +246,10 @@
*/
static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind)
{
- struct i2c_client *new_client;
+ struct i2c_client *client;
struct ds1337_data *data;
+ char *name;
int err = 0;
- const char *name = "";
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
I2C_FUNC_I2C))
@@ -273,12 +265,12 @@
/* The common I2C client data is placed right before the
* DS1337-specific data.
*/
- new_client = &data->client;
- i2c_set_clientdata(new_client, data);
- new_client->addr = address;
- new_client->adapter = adapter;
- new_client->driver = &ds1337_driver;
- new_client->flags = 0;
+ client = &data->client;
+ i2c_set_clientdata(client, data);
+ client->addr = address;
+ client->adapter = adapter;
+ client->driver = &ds1337_driver;
+ client->flags = 0;
/*
* Now we do the remaining detection. A negative kind means that
@@ -303,51 +295,72 @@
u8 data;
/* Check that status register bits 6-2 are zero */
- if ((ds1337_read(new_client, DS1337_REG_STATUS, &data) < 0) ||
+ if ((ds1337_read(client, DS1337_REG_STATUS, &data) < 0) ||
(data & 0x7c))
goto exit_free;
/* Check for a valid day register value */
- if ((ds1337_read(new_client, DS1337_REG_DAY, &data) < 0) ||
+ if ((ds1337_read(client, DS1337_REG_DAY, &data) < 0) ||
(data == 0) || (data & 0xf8))
goto exit_free;
/* Check for a valid date register value */
- if ((ds1337_read(new_client, DS1337_REG_DATE, &data) < 0) ||
+ if ((ds1337_read(client, DS1337_REG_DATE, &data) < 0) ||
(data == 0) || (data & 0xc0) || ((data & 0x0f) > 9) ||
(data >= 0x32))
goto exit_free;
/* Check for a valid month register value */
- if ((ds1337_read(new_client, DS1337_REG_MONTH, &data) < 0) ||
+ if ((ds1337_read(client, DS1337_REG_MONTH, &data) < 0) ||
(data == 0) || (data & 0x60) || ((data & 0x0f) > 9) ||
((data >= 0x13) && (data <= 0x19)))
goto exit_free;
/* Check that control register bits 6-5 are zero */
- if ((ds1337_read(new_client, DS1337_REG_CONTROL, &data) < 0) ||
+ if ((ds1337_read(client, DS1337_REG_CONTROL, &data) < 0) ||
(data & 0x60))
goto exit_free;
- kind = ds1337;
+ /* Check whenever we have DS1339 */
+ if (ds1337_is_ds1339(client))
+ kind = ds1339;
+ else
+ kind = ds1337;
}
- if (kind == ds1337)
+ switch (kind) {
+ case ds1337:
name = "ds1337";
+ break;
+ case ds1339:
+ name = "ds1339";
+ break;
+ default:
+ name = "";
+ }
/* We can fill in the remaining client fields */
- strlcpy(new_client->name, name, I2C_NAME_SIZE);
+ strlcpy(client->name, name, I2C_NAME_SIZE);
/* Tell the I2C layer a new client has arrived */
- if ((err = i2c_attach_client(new_client)))
+ if ((err = i2c_attach_client(client)))
goto exit_free;
/* Initialize the DS1337 chip */
- ds1337_init_client(new_client);
+ ds1337_init_client(client);
/* Add client to local list */
data->id = ds1337_id++;
list_add(&data->list, &ds1337_clients);
+
+ /* Be nice to battery */
+ if (kind == ds1339 && ds1339_charge) {
+ char buf[] = { DS1339_REG_CHARGE, ds1339_charge };
+
+ if (i2c_master_send(client, buf, sizeof(buf)) != sizeof(buf))
+ dev_err(&client->dev,
+ "failed to enable trickle charge.\n");
+ }
return 0;
Hi Ladislav,
> I know it is bad time to send any patches, but lets try anyway :)
>
> My embedded device is using DS1339 I2C RTC clock which differs from
> DS1337 only in one register at address 10h which enables battery charge.
> Originaly I was using my own driver, but decided to extend existing
> one. Chip detection is done automaticaly, see ds1337_is_ds1339 function
> and comment above. While playing with code, few things seemed strange to
> me, so here is comment:
>
> * driver is using adapter->algo->master_xfer function without checking
> it is present and without holding bus lock. That looks insane to me.
> fixed.
>
> * BCD_TO_BIN is inteded to use this way: BCD_TO_BIN(val); Drive changed
> to use BCD2BIN.
>
> * Changed dev_dbg to print "dev" of device this driver is for not
> adapter's dev.
>
> * Changed get/set time to be compatible with other RTC drivers, ie.
> epoch is handled separately. That should be done in RTC interface
> part [*].
>
> Driver is tested and works with DS1339, detection algoritm was tested
> with extending address space to 18 bytes in DS1339. Please test on
> DS1337 as well. Comments and suggestions welcome.
Please slice this into separarte patches. Adding support for the DS1339
is one thing, bug fixes are another. I can't review patches which do
that many different things at once.
As a side note, please avoid noise in your patch. Converting constants
from decimal to hexadecimal or renaming variables makes my review job
way harder, as it distracts me from the real point of your patch.
Once you realize that the time I need to review a patch increases in a
quadratic way (if not worse) relatively to the length of the patch, I am
sure you will see why it is important to send small patches doing just
one thing without noise.
Thanks,
--
Jean Delvare
On Thu, Apr 07, 2005 at 11:59:05AM +0200, Jean Delvare wrote:
> Please slice this into separarte patches. Adding support for the DS1339
> is one thing, bug fixes are another. I can't review patches which do
> that many different things at once.
I have objection ;-) Nothing in kernel seems to use that driver and
driver wasn't reviewed well enough before including. Therefore I'm still
considering it as a new driver which can be easily reviewed as whole.
> As a side note, please avoid noise in your patch. Converting constants
> from decimal to hexadecimal or renaming variables makes my review job
> way harder, as it distracts me from the real point of your patch.
Hexadecimal constant are there to match datasheet, local variables
should be short and with new_client is was reaching 80 column limit,
that's coding style and that's why I made that change.
Please consider that I would suggest these changes to driver author
before his patch went in tree in case I would read all that mailing
lists around. I'm lazy, my bad... Driver's author should have probably
last word anyway.
> Once you realize that the time I need to review a patch increases in a
> quadratic way (if not worse) relatively to the length of the patch, I am
> sure you will see why it is important to send small patches doing just
> one thing without noise.
Ok, sending cleanup fixes only. Support for ds1339 will be in separate
patch. Sorry I won't do more, because my time is also limited.
Here is patch which does cleanup/fixes only. It is still hard to review,
but that's living. I won't split it to smaller parts just because I had
to touch each single line in get/set date functions. Sorry if it sounds
impolite, but now I can't spend more time on it. Perhaps later, if
anyone don't buy it as is meanwhile... Please let me know when/if you will
accept support for DS1339.
Best regards,
ladis
===== drivers/i2c/chips/ds1337.c 1.1 vs edited =====
--- 1.1/drivers/i2c/chips/ds1337.c 2005-03-31 22:58:08 +02:00
+++ edited/drivers/i2c/chips/ds1337.c 2005-04-07 13:02:18 +02:00
@@ -2,8 +2,9 @@
* linux/drivers/i2c/chips/ds1337.c
*
* Copyright (C) 2005 James Chapman <[email protected]>
+ * Copyright (C) 2005 Ladislav Michl <[email protected]>
*
- * based on linux/drivers/acron/char/pcf8583.c
+ * based on linux/drivers/acorn/char/pcf8583.c
* Copyright (C) 2000 Russell King
*
* This program is free software; you can redistribute it and/or modify
@@ -25,12 +26,13 @@
#include <linux/list.h>
/* Device registers */
-#define DS1337_REG_HOUR 2
-#define DS1337_REG_DAY 3
-#define DS1337_REG_DATE 4
-#define DS1337_REG_MONTH 5
-#define DS1337_REG_CONTROL 14
-#define DS1337_REG_STATUS 15
+#define DS1337_REG_HOUR 0x02
+#define DS1337_REG_DAY 0x03
+#define DS1337_REG_DATE 0x04
+#define DS1337_REG_MONTH 0x05
+#define DS1337_REG_CONTROL 0x0e
+#define DS1337_REG_STATUS 0x0f
+#define DS1339_REG_CHARGE 0x10
/* FIXME - how do we export these interface constants? */
#define DS1337_GET_DATE 0
@@ -93,116 +95,74 @@
/*
* Chip access functions
*/
-static int ds1337_get_datetime(struct i2c_client *client, struct rtc_time *dt)
+static int ds1337_get_datetime(struct i2c_client *client, struct rtc_time *tm)
{
- 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);
+ unsigned char buf[7] = { 0, }, addr[1] = { 0 };
+ struct i2c_msg msgs[2] = {
+ { client->addr, 0, 1, addr },
+ { client->addr, I2C_M_RD, 7, buf }
+ };
+ int result = i2c_transfer(client->adapter, msgs, 2);
- dev_dbg(&client->adapter->dev,
+ dev_dbg(&client->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 (result < 0) {
+ dev_err(&client->dev, "error reading data! %d\n", result);
+ result = -EIO;
+ } else {
+ tm->tm_sec = BCD2BIN(buf[0]);
+ tm->tm_min = BCD2BIN(buf[1]);
+ tm->tm_hour = BCD2BIN(buf[2] & 0x3f);
+ tm->tm_wday = BCD2BIN(buf[3]);
+ tm->tm_mday = BCD2BIN(buf[4]);
+ tm->tm_mon = BCD2BIN(buf[5] & 0x7f) - 1;
+ tm->tm_year = BCD2BIN(buf[6]);
if (buf[5] & 0x80)
- dt->tm_year += 100;
+ tm->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;
+ dev_dbg(&client->dev, "%s: secs=%d, mins=%d, hours=%d, "
+ "mday=%d, mon=%d, year=%d, wday=%d\n", __FUNCTION__,
+ tm->tm_sec, tm->tm_min, tm->tm_hour,
+ tm->tm_mday, tm->tm_mon, tm->tm_year, tm->tm_wday);
+
+ result = 0;
}
return result;
}
-static int ds1337_set_datetime(struct i2c_client *client, struct rtc_time *dt)
+static int ds1337_set_datetime(struct i2c_client *client, struct rtc_time *tm)
{
- struct ds1337_data *data = i2c_get_clientdata(client);
+ unsigned char buf[8];
int result;
- u8 buf[8];
- u8 val;
- struct i2c_msg msg[1];
-
- if (!dt) {
- dev_dbg(&client->adapter->dev, "%s: EINVAL: dt=NULL\n",
- __FUNCTION__);
- return -EINVAL;
- }
-
- dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, hours=%d, "
+ dev_dbg(&client->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);
+ tm->tm_sec, tm->tm_min, tm->tm_hour,
+ tm->tm_mday, tm->tm_mon, tm->tm_year, tm->tm_wday);
buf[0] = 0; /* reg offset */
- buf[1] = BIN_TO_BCD(dt->tm_sec);
- buf[2] = BIN_TO_BCD(dt->tm_min);
- buf[3] = BIN_TO_BCD(dt->tm_hour) | (1 << 6);
- buf[4] = BIN_TO_BCD(dt->tm_wday) + 1;
- buf[5] = BIN_TO_BCD(dt->tm_mday);
- buf[6] = BIN_TO_BCD(dt->tm_mon);
- if (dt->tm_year >= 2000) {
- val = dt->tm_year - 2000;
+ buf[1] = BIN2BCD(tm->tm_sec);
+ buf[2] = BIN2BCD(tm->tm_min);
+ buf[3] = BIN2BCD(tm->tm_hour) | (1 << 6);
+ buf[4] = BIN2BCD(tm->tm_wday);
+ buf[5] = BIN2BCD(tm->tm_mday);
+ buf[6] = BIN2BCD(tm->tm_mon + 1);
+ if (tm->tm_year >= 100) {
+ tm->tm_year -= 100;
buf[6] |= (1 << 7);
- } else {
- val = dt->tm_year - 1900;
}
- buf[7] = BIN_TO_BCD(val);
+ buf[7] = BIN2BCD(tm->tm_year);
- msg[0].addr = client->addr;
- msg[0].flags = 0;
- msg[0].len = sizeof(buf);
- msg[0].buf = &buf[0];
-
- result = client->adapter->algo->master_xfer(client->adapter,
- &msg[0], 1);
+ result = i2c_master_send(client, (char *)buf, sizeof(buf));
if (result < 0) {
- dev_err(&client->adapter->dev, "ds1337[%d]: error "
- "writing data! %d\n", data->id, result);
+ dev_err(&client->dev, "error writing data! %d\n", result);
result = -EIO;
- } else {
+ } else
result = 0;
- }
return result;
}
@@ -210,7 +170,7 @@
static int ds1337_command(struct i2c_client *client, unsigned int cmd,
void *arg)
{
- dev_dbg(&client->adapter->dev, "%s: cmd=%d\n", __FUNCTION__, cmd);
+ dev_dbg(&client->dev, "%s: cmd=%d\n", __FUNCTION__, cmd);
switch (cmd) {
case DS1337_GET_DATE:
@@ -254,10 +214,10 @@
*/
static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind)
{
- struct i2c_client *new_client;
+ struct i2c_client *client;
struct ds1337_data *data;
+ char *name;
int err = 0;
- const char *name = "";
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
I2C_FUNC_I2C))
@@ -273,12 +233,12 @@
/* The common I2C client data is placed right before the
* DS1337-specific data.
*/
- new_client = &data->client;
- i2c_set_clientdata(new_client, data);
- new_client->addr = address;
- new_client->adapter = adapter;
- new_client->driver = &ds1337_driver;
- new_client->flags = 0;
+ client = &data->client;
+ i2c_set_clientdata(client, data);
+ client->addr = address;
+ client->adapter = adapter;
+ client->driver = &ds1337_driver;
+ client->flags = 0;
/*
* Now we do the remaining detection. A negative kind means that
@@ -303,47 +263,52 @@
u8 data;
/* Check that status register bits 6-2 are zero */
- if ((ds1337_read(new_client, DS1337_REG_STATUS, &data) < 0) ||
+ if ((ds1337_read(client, DS1337_REG_STATUS, &data) < 0) ||
(data & 0x7c))
goto exit_free;
/* Check for a valid day register value */
- if ((ds1337_read(new_client, DS1337_REG_DAY, &data) < 0) ||
+ if ((ds1337_read(client, DS1337_REG_DAY, &data) < 0) ||
(data == 0) || (data & 0xf8))
goto exit_free;
/* Check for a valid date register value */
- if ((ds1337_read(new_client, DS1337_REG_DATE, &data) < 0) ||
+ if ((ds1337_read(client, DS1337_REG_DATE, &data) < 0) ||
(data == 0) || (data & 0xc0) || ((data & 0x0f) > 9) ||
(data >= 0x32))
goto exit_free;
/* Check for a valid month register value */
- if ((ds1337_read(new_client, DS1337_REG_MONTH, &data) < 0) ||
+ if ((ds1337_read(client, DS1337_REG_MONTH, &data) < 0) ||
(data == 0) || (data & 0x60) || ((data & 0x0f) > 9) ||
((data >= 0x13) && (data <= 0x19)))
goto exit_free;
/* Check that control register bits 6-5 are zero */
- if ((ds1337_read(new_client, DS1337_REG_CONTROL, &data) < 0) ||
+ if ((ds1337_read(client, DS1337_REG_CONTROL, &data) < 0) ||
(data & 0x60))
goto exit_free;
kind = ds1337;
}
- if (kind == ds1337)
+ switch (kind) {
+ case ds1337:
name = "ds1337";
+ break;
+ default:
+ name = "";
+ }
/* We can fill in the remaining client fields */
- strlcpy(new_client->name, name, I2C_NAME_SIZE);
+ strlcpy(client->name, name, I2C_NAME_SIZE);
/* Tell the I2C layer a new client has arrived */
- if ((err = i2c_attach_client(new_client)))
+ if ((err = i2c_attach_client(client)))
goto exit_free;
/* Initialize the DS1337 chip */
- ds1337_init_client(new_client);
+ ds1337_init_client(client);
/* Add client to local list */
data->id = ds1337_id++;
Hi Ladislav,
> > Please slice this into separarte patches. Adding support for the DS1339
> > is one thing, bug fixes are another. I can't review patches which do
> > that many different things at once.
>
> I have objection ;-) Nothing in kernel seems to use that driver (...)
How would you know? It has a user-space interface
(ds1337_driver.command), which anyone might be using.
> (...) and driver wasn't reviewed well enough before including.
I did review this driver. Thanks for the compliment, I really appreciate.
The driver went through several review and update steps between James
Chapman and I, part of which can be seen here (there were a couple of
additional private mails IIRC):
http://archives.andrew.net.au/lm-sensors/msg29709.html
It got as good a review as one would ask for, or so I believe.
> Therefore I'm still considering it as a new driver which can be easily
> reviewed as whole.
Reviewing small, individual patches is always easier than reviewing one
big patch doing several things at once, it has nothing to do with the
age of the code. And at any rate you shouldn't consider anything is
easy when you are not the one doing it.
> > As a side note, please avoid noise in your patch. Converting constants
> > from decimal to hexadecimal or renaming variables makes my review job
> > way harder, as it distracts me from the real point of your patch.
>
> Hexadecimal constant are there to match datasheet, (...)
And anyone may argue that decimal values are easier to read, or whatever,
and send a revert patch next week? James did it that way, values are
correct. I see no point in changing them.
> (...) local variables should be short and with new_client is was
> reaching 80 column limit, that's coding style and that's why I made
> that change.
You changed other variable names with no reason (unless you are going to
argue that "tm" is shorter than "dt", of course).
I *do* agree that "new_client" is a poor variable name in this context,
and "client" carries the very same amount of information. That's not
specific to this driver though, almost all i2c chip driver use that name.
> Please consider that I would suggest these changes to driver author
> before his patch went in tree in case I would read all that mailing
> lists around. I'm lazy, my bad...
Sure you would have been welcome to review James Chapman's original
patch. But you didn't, so the fact that you would have objected if you
did is irrelevant. The code is in the kernel tree now.
You are very welcome to review future patches, though.
> Driver's author should have probably last word anyway.
Not necessarily. I had similarly hairsplitting objections to James'
driver when he first posted his patch, and we reached a state of
agreement on everything quite easily. He is a very pleasant person to
work with.
> Here is patch which does cleanup/fixes only. It is still hard to review,
> but that's living. I won't split it to smaller parts just because I had
> to touch each single line in get/set date functions. Sorry if it sounds
> impolite, but now I can't spend more time on it. Perhaps later, if
> anyone don't buy it as is meanwhile...
I'm not sure if it qualifies as impolite, but I can tell you won't get
anywhere with such an attitude. Trust me, I've been there before, and
changed.
Cleanups and fixes belong to separate patches. Your current patch, even
without the DS1339 support, is a mess, mixing useful cleanups, useless
cleanups and actual fixes - unfortunately undocumented so we can't even
say what exactly you are trying to fix.
> Please let me know when/if you will accept support for DS1339.
After this patch has been split, reviewed and modified enough to be
accepted. Your time might be limited, this won't change a thing, sorry.
Patches are accepted when we think they are good, and not before. You
can't tell that the original ds1337 driver wasn't properly reviewed
and at the same time ask me to accept this random patch of yours while I
an not able to review it in its current state.
Thanks,
--
Jean Delvare
Dear Jean,
On Thu, Apr 07, 2005 at 03:07:40PM +0200, Jean Delvare wrote:
> > I have objection ;-) Nothing in kernel seems to use that driver (...)
>
> How would you know? It has a user-space interface
> (ds1337_driver.command), which anyone might be using.
I asked how is this driver supposed to interface with the rest of world.
pcf8583.c nor rtc8564.c are returing year as is, both needs epoch
to be added, so whoever is using this interface has to be aware of this
"feature". That's something what should be resolved. James?
> > (...) and driver wasn't reviewed well enough before including.
>
> I did review this driver. Thanks for the compliment, I really appreciate.
I'm sorry, it wasn't mean to be personal. What was IMHO overlooked is in
changelog below.
[snip]
> > Hexadecimal constant are there to match datasheet, (...)
>
> And anyone may argue that decimal values are easier to read, (...)
I'm always trying to follow manual and I agree not everyone does the
same. Change droped. (Note: This way you do not need to know reading,
comparing shapes is enough :))
> (...) or whatever, and send a revert patch next week? James did it
> that way, values are correct. I see no point in changing them.
[snip]
> I'm not sure if it qualifies as impolite, but I can tell you won't get
> anywhere with such an attitude. Trust me, I've been there before, and
> changed.
Okay, let's finally keep only technical aspects. I want to use James
driver instead of mine even if it means more work for me. If that won't
work out, I won't get anywhere and will use what works for me :)
Here is yet another patch this time fixes only.
CHANGELOG:
* use i2_tranfer function instead of adapter->algo->master_xfer, so
we have proper bus locking.
* BCD2BIN and BIN2BCD are proper macros to use here, see linux/bcd.h
* Move NULL argument checking from get/set date functions to
ds1337_command function, so it is only at one place. Note that other
drivers do not this checking at all and I think it is pointess,
because you have to know that you are passing struct rtc_time anyway.
* dev_dbg should probably print info about device driver we are
debugging so client->dev looks as better choice than
client->adapter->dev.
Best regards,
ladis
===== drivers/i2c/chips/ds1337.c 1.1 vs edited =====
--- 1.1/drivers/i2c/chips/ds1337.c 2005-03-31 22:58:08 +02:00
+++ edited/drivers/i2c/chips/ds1337.c 2005-04-07 15:47:26 +02:00
@@ -2,8 +2,9 @@
* linux/drivers/i2c/chips/ds1337.c
*
* Copyright (C) 2005 James Chapman <[email protected]>
+ * Copyright (C) 2005 Ladislav Michl <[email protected]>
*
- * based on linux/drivers/acron/char/pcf8583.c
+ * based on linux/drivers/acorn/char/pcf8583.c
* Copyright (C) 2000 Russell King
*
* This program is free software; you can redistribute it and/or modify
@@ -95,60 +96,38 @@
*/
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];
+ unsigned char buf[7] = { 0, }, addr[1] = { 0 };
+ struct i2c_msg msgs[2] = {
+ { client->addr, 0, 1, addr },
+ { client->addr, I2C_M_RD, 7, buf }
+ };
+ int result = i2c_transfer(client->adapter, msgs, 2);
- result = client->adapter->algo->master_xfer(client->adapter,
- &msg[0], 2);
-
- dev_dbg(&client->adapter->dev,
+ dev_dbg(&client->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 (result < 0) {
+ dev_err(&client->dev, "error reading data! %d\n", result);
+ result = -EIO;
+ } else {
+ dt->tm_sec = BCD2BIN(buf[0]);
+ dt->tm_min = BCD2BIN(buf[1]);
+ dt->tm_hour = BCD2BIN(buf[2] & 0x3f);
+ dt->tm_wday = BCD2BIN(buf[3]) - 1;
+ dt->tm_mday = BCD2BIN(buf[4]);
+ dt->tm_mon = BCD2BIN(buf[5] & 0x7f) - 1;
+ dt->tm_year = BCD2BIN(buf[6]) + 1900;
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;
+ dev_dbg(&client->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);
+
+ result = 0;
}
return result;
@@ -156,53 +135,33 @@
static int ds1337_set_datetime(struct i2c_client *client, struct rtc_time *dt)
{
- struct ds1337_data *data = i2c_get_clientdata(client);
+ unsigned char buf[8];
int result;
- u8 buf[8];
- u8 val;
- struct i2c_msg msg[1];
- if (!dt) {
- dev_dbg(&client->adapter->dev, "%s: EINVAL: dt=NULL\n",
- __FUNCTION__);
-
- return -EINVAL;
- }
-
- dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, hours=%d, "
+ dev_dbg(&client->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);
buf[0] = 0; /* reg offset */
- buf[1] = BIN_TO_BCD(dt->tm_sec);
- buf[2] = BIN_TO_BCD(dt->tm_min);
- buf[3] = BIN_TO_BCD(dt->tm_hour) | (1 << 6);
- buf[4] = BIN_TO_BCD(dt->tm_wday) + 1;
- buf[5] = BIN_TO_BCD(dt->tm_mday);
- buf[6] = BIN_TO_BCD(dt->tm_mon);
+ buf[1] = BIN2BCD(dt->tm_sec);
+ buf[2] = BIN2BCD(dt->tm_min);
+ buf[3] = BIN2BCD(dt->tm_hour) | (1 << 6);
+ buf[4] = BIN2BCD(dt->tm_wday);
+ buf[5] = BIN2BCD(dt->tm_mday) + 1;
+ buf[6] = BIN2BCD(dt->tm_mon + 1);
if (dt->tm_year >= 2000) {
- val = dt->tm_year - 2000;
buf[6] |= (1 << 7);
- } else {
- val = dt->tm_year - 1900;
- }
- buf[7] = BIN_TO_BCD(val);
+ buf[7] = BIN2BCD(dt->tm_year - 2000);
+ } else
+ buf[7] = BIN2BCD(dt->tm_year - 1900);
- msg[0].addr = client->addr;
- msg[0].flags = 0;
- msg[0].len = sizeof(buf);
- msg[0].buf = &buf[0];
-
- result = client->adapter->algo->master_xfer(client->adapter,
- &msg[0], 1);
+ result = i2c_master_send(client, (char *)buf, sizeof(buf));
if (result < 0) {
- dev_err(&client->adapter->dev, "ds1337[%d]: error "
- "writing data! %d\n", data->id, result);
+ dev_err(&client->dev, "error writing data! %d\n", result);
result = -EIO;
- } else {
+ } else
result = 0;
- }
return result;
}
@@ -210,7 +169,14 @@
static int ds1337_command(struct i2c_client *client, unsigned int cmd,
void *arg)
{
- dev_dbg(&client->adapter->dev, "%s: cmd=%d\n", __FUNCTION__, cmd);
+ dev_dbg(&client->dev, "%s: cmd=%d\n", __FUNCTION__, cmd);
+
+ if (!arg) {
+ dev_dbg(&client->dev, "%s: EINVAL: arg=NULL\n",
+ __FUNCTION__);
+
+ return -EINVAL;
+ }
switch (cmd) {
case DS1337_GET_DATE:
On Thu, Apr 07, 2005 at 04:28:04PM +0200, Ladislav Michl wrote:
> Here is yet another patch this time fixes only.
> CHANGELOG:
> * use i2_tranfer function instead of adapter->algo->master_xfer, so
> we have proper bus locking.
> * BCD2BIN and BIN2BCD are proper macros to use here, see linux/bcd.h
> * Move NULL argument checking from get/set date functions to
> ds1337_command function, so it is only at one place. Note that other
> drivers do not this checking at all and I think it is pointess,
> because you have to know that you are passing struct rtc_time anyway.
> * dev_dbg should probably print info about device driver we are
> debugging so client->dev looks as better choice than
> client->adapter->dev.
Jean's point is that you should send an individual patch for each type
of individual change. It's ok to say "patch 3 requires you to have
applied patches 1 and 2" and so on. Please split this up better.
thanks,
greg k-h
Hi Ladislav,
> Here is yet another patch this time fixes only.
> CHANGELOG:
> * use i2_tranfer function instead of adapter->algo->master_xfer, so
> we have proper bus locking.
You are absolutely right. My mistake, I should have noticed when first
reviewing the code, as calling master_xfer directly is unseen - for a
reason.
> * BCD2BIN and BIN2BCD are proper macros to use here, see linux/bcd.h
Looks correct to me as well.
> * Move NULL argument checking from get/set date functions to
> ds1337_command function, so it is only at one place. Note that other
> drivers do not this checking at all and I think it is pointess,
> because you have to know that you are passing struct rtc_time
> anyway.
I am not certain these are the right things to do (moving the check or
removing it). I am not a specialist of ioctl, but it looks to me that
ds1337_command acts as a dispatcher, branching to various functions
depending on the value of cmd. I can imagine that some functions take an
argument, and some don't, so checking for NULL pointer in the dispatcher
doesn't make much sense. Now it is correct that for now all (two)
functions need a parameter, but what if later a function is added, which
takes no parameter? You'd have to undo your change and move the check in
each function again.
As for the check itself, the pointer somehow comes from user-space as I
understand it, so you can't tell whether it's NULL or not - so checking
makes full sense to me. If you take a look at the rtc8564 driver you'll
see it *does* check for NULL pointers too.
> * dev_dbg should probably print info about device driver we are
> debugging so client->dev looks as better choice than
> client->adapter->dev.
You're correct.
My comments to the code itself follows.
> @@ -95,60 +96,38 @@
> */
> 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];
> + unsigned char buf[7] = { 0, }, addr[1] = { 0 };
> + struct i2c_msg msgs[2] = {
> + { client->addr, 0, 1, addr },
> + { client->addr, I2C_M_RD, 7, buf }
> + };
> + int result = i2c_transfer(client->adapter, msgs, 2);
>
> - result = client->adapter->algo->master_xfer(client->adapter,
> - &msg[0], 2);
You are doing much more than just using i2c_transfer instead of
master_xfer. You are also rewriting the way the message data is
initialized. I see no reason to do that, as the previous code was
correct as far as I can see.
> - dev_dbg(&client->adapter->dev,
> + dev_dbg(&client->dev,
Yes, that's correct.
> - if (result >= 0) {
(...)
> + if (result < 0) {
By changing this you are making your patch much bigger and harder to
review. Why do you do that?
> - val = buf[2] & 0x3f;
> - dt->tm_hour = BCD_TO_BIN(val);
(...)
> + dt->tm_hour = BCD2BIN(buf[2] & 0x3f);
No, James is correct. BCD2BIN (or BCD_TO_BIN for that matter) is a
macro which evaluates its argument more than once. Using a temporary
variable makes sense.
> - val = buf[5] & 0x7f;
> - dt->tm_mon = BCD_TO_BIN(val);
(...)
> + dt->tm_mon = BCD2BIN(buf[5] & 0x7f) - 1;
Looks to me like you are silently changing the code here. Was there a
bug? If so, please tell so.
> + unsigned char buf[8];
> int result;
> - u8 buf[8];
Wow, what a useful change. Please please please... Focus on making your
patch compact, have it do just the thing it is supposed (and advertised)
to do. You know, I'll repeat it until you get it. No matter how many
tries it takes.
> if (dt->tm_year >= 2000) {
> - val = dt->tm_year - 2000;
> buf[6] |= (1 << 7);
> - } else {
> - val = dt->tm_year - 1900;
> - }
> - buf[7] = BIN_TO_BCD(val);
> + buf[7] = BIN2BCD(dt->tm_year - 2000);
> + } else
> + buf[7] = BIN2BCD(dt->tm_year - 1900);
Same as before, the use of a temporary variable makes full sense, don't
change that. And you're again adding noise by dropping a pair of curly
braces.
> - } else {
> + } else
> result = 0;
> - }
And that's noise again.
Please resubmit your patch without all the noise. Don't change things
just because you like them the other way. Fix things which are broken,
and only these.
Thanks,
--
Jean Delvare
Jean,
I'll comment your mail first and then send separate patches (somehow
I can't sleep this night :))
On Thu, Apr 07, 2005 at 11:29:08PM +0200, Jean Delvare wrote:
> > * Move NULL argument checking from get/set date functions to
> > ds1337_command function, so it is only at one place. Note that other
> > drivers do not this checking at all and I think it is pointess,
> > because you have to know that you are passing struct rtc_time
> > anyway.
>
> I am not certain these are the right things to do (moving the check or
> removing it). I am not a specialist of ioctl, but it looks to me that
> ds1337_command acts as a dispatcher, branching to various functions
> depending on the value of cmd. I can imagine that some functions take an
> argument, and some don't, so checking for NULL pointer in the dispatcher
> doesn't make much sense. Now it is correct that for now all (two)
> functions need a parameter, but what if later a function is added, which
> takes no parameter? You'd have to undo your change and move the check in
> each function again.
>
> As for the check itself, the pointer somehow comes from user-space as I
> understand it, so you can't tell whether it's NULL or not - so checking
> makes full sense to me. If you take a look at the rtc8564 driver you'll
> see it *does* check for NULL pointers too.
You can't tell if memory it points to is valid. Okay, probably better
than nothing.
> > @@ -95,60 +96,38 @@
> > */
> > 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];
> > + unsigned char buf[7] = { 0, }, addr[1] = { 0 };
> > + struct i2c_msg msgs[2] = {
> > + { client->addr, 0, 1, addr },
> > + { client->addr, I2C_M_RD, 7, buf }
> > + };
> > + int result = i2c_transfer(client->adapter, msgs, 2);
> >
> > - result = client->adapter->algo->master_xfer(client->adapter,
> > - &msg[0], 2);
>
> You are doing much more than just using i2c_transfer instead of
> master_xfer. You are also rewriting the way the message data is
> initialized. I see no reason to do that, as the previous code was
> correct as far as I can see.
Right, I just made it shorter. One more point for you, my way is not
struct i2c_msg change proof. I'll drop it.
> > - if (result >= 0) {
> (...)
> > + if (result < 0) {
>
> By changing this you are making your patch much bigger and harder to
> review. Why do you do that?
Here you need to look at patched code. Now conditions in both
ds1337_get_datetime and ds1337_set_datetime look similar, so code is
IHMO easily readable. I'm fine with droping this change.
> > - val = buf[2] & 0x3f;
> > - dt->tm_hour = BCD_TO_BIN(val);
> (...)
> > + dt->tm_hour = BCD2BIN(buf[2] & 0x3f);
>
> No, James is correct. BCD2BIN (or BCD_TO_BIN for that matter) is a
> macro which evaluates its argument more than once. Using a temporary
> variable makes sense.
Agree.
> > + unsigned char buf[8];
> > int result;
> > - u8 buf[8];
>
> Wow, what a useful change. Please please please... Focus on making your
> patch compact, have it do just the thing it is supposed (and advertised)
> to do. You know, I'll repeat it until you get it. No matter how many
> tries it takes.
Save your time I got it. buf is supposed to be char, that's what function
expects. I wrongly made it unsigned. u8, u16 etc. are used in case
when you for example need to generate say 8 bit bus access or need same
width on all architectures. Neither is case here and using u8 makes no
sense. Anyway, will drop change.
> > if (dt->tm_year >= 2000) {
> > - val = dt->tm_year - 2000;
> > buf[6] |= (1 << 7);
> > - } else {
> > - val = dt->tm_year - 1900;
> > - }
> > - buf[7] = BIN_TO_BCD(val);
> > + buf[7] = BIN2BCD(dt->tm_year - 2000);
> > + } else
> > + buf[7] = BIN2BCD(dt->tm_year - 1900);
>
> Same as before, the use of a temporary variable makes full sense, don't
> change that. And you're again adding noise by dropping a pair of curly
> braces.
That's only because I read mail by jgarzik suggesting to remove such
braces few hours ago :) Also, i'll drop this change.
Best regards,
ladis
dev_{dbg,err} functions should print client's device name. data->id can
be dropped from message, because device is determined by bus it hangs on
(it has fixed address).
--- linux-omap/drivers/i2c/chips/ds1337.c.orig 2005-04-08 00:36:15.072302800 +0200
+++ linux-omap/drivers/i2c/chips/ds1337.c 2005-04-08 00:44:44.130914128 +0200
@@ -95,7 +95,6 @@ static inline int ds1337_read(struct i2c
*/
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;
@@ -103,9 +102,7 @@ static int ds1337_get_datetime(struct i2
u8 offs = 0;
if (!dt) {
- dev_dbg(&client->adapter->dev, "%s: EINVAL: dt=NULL\n",
- __FUNCTION__);
-
+ dev_dbg(&client->dev, "%s: EINVAL: dt=NULL\n", __FUNCTION__);
return -EINVAL;
}
@@ -121,8 +118,7 @@ static int ds1337_get_datetime(struct i2
result = i2c_transfer(client->adapter, msg, 2);
- dev_dbg(&client->adapter->dev,
- "%s: [%d] %02x %02x %02x %02x %02x %02x %02x\n",
+ dev_dbg(&client->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]);
@@ -139,14 +135,13 @@ static int ds1337_get_datetime(struct i2
if (buf[5] & 0x80)
dt->tm_year += 100;
- dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, "
+ dev_dbg(&client->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);
+ dev_err(&client->dev, "error reading data! %d\n", result);
result = -EIO;
}
@@ -155,20 +150,17 @@ static int ds1337_get_datetime(struct i2
static int ds1337_set_datetime(struct i2c_client *client, struct rtc_time *dt)
{
- struct ds1337_data *data = i2c_get_clientdata(client);
int result;
u8 buf[8];
u8 val;
struct i2c_msg msg[1];
if (!dt) {
- dev_dbg(&client->adapter->dev, "%s: EINVAL: dt=NULL\n",
- __FUNCTION__);
-
+ dev_dbg(&client->dev, "%s: EINVAL: dt=NULL\n", __FUNCTION__);
return -EINVAL;
}
- dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, hours=%d, "
+ dev_dbg(&client->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);
@@ -195,8 +187,7 @@ static int ds1337_set_datetime(struct i2
result = i2c_transfer(client->adapter, msg, 1);
if (result < 0) {
- dev_err(&client->adapter->dev, "ds1337[%d]: error "
- "writing data! %d\n", data->id, result);
+ dev_err(&client->dev, "error writing data! %d\n", result);
result = -EIO;
} else {
result = 0;
@@ -208,7 +199,7 @@ static int ds1337_set_datetime(struct i2
static int ds1337_command(struct i2c_client *client, unsigned int cmd,
void *arg)
{
- dev_dbg(&client->adapter->dev, "%s: cmd=%d\n", __FUNCTION__, cmd);
+ dev_dbg(&client->dev, "%s: cmd=%d\n", __FUNCTION__, cmd);
switch (cmd) {
case DS1337_GET_DATE:
Use correct macros to convert between bdc and bin. See linux/bcd.h
--- linux-omap/drivers/i2c/chips/ds1337.c.orig 2005-04-08 00:32:45.234203040 +0200
+++ linux-omap/drivers/i2c/chips/ds1337.c 2005-04-08 00:34:58.457949952 +0200
@@ -127,15 +127,15 @@
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]);
+ dt->tm_sec = BCD2BIN(buf[0]);
+ dt->tm_min = BCD2BIN(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]);
+ dt->tm_hour = BCD2BIN(val);
+ dt->tm_wday = BCD2BIN(buf[3]) - 1;
+ dt->tm_mday = BCD2BIN(buf[4]);
val = buf[5] & 0x7f;
- dt->tm_mon = BCD_TO_BIN(val);
- dt->tm_year = 1900 + BCD_TO_BIN(buf[6]);
+ dt->tm_mon = BCD2BIN(val);
+ dt->tm_year = 1900 + BCD2BIN(buf[6]);
if (buf[5] & 0x80)
dt->tm_year += 100;
@@ -174,19 +174,19 @@
dt->tm_mday, dt->tm_mon, dt->tm_year, dt->tm_wday);
buf[0] = 0; /* reg offset */
- buf[1] = BIN_TO_BCD(dt->tm_sec);
- buf[2] = BIN_TO_BCD(dt->tm_min);
- buf[3] = BIN_TO_BCD(dt->tm_hour) | (1 << 6);
- buf[4] = BIN_TO_BCD(dt->tm_wday) + 1;
- buf[5] = BIN_TO_BCD(dt->tm_mday);
- buf[6] = BIN_TO_BCD(dt->tm_mon);
+ buf[1] = BIN2BCD(dt->tm_sec);
+ buf[2] = BIN2BCD(dt->tm_min);
+ buf[3] = BIN2BCD(dt->tm_hour) | (1 << 6);
+ buf[4] = BIN2BCD(dt->tm_wday) + 1;
+ buf[5] = BIN2BCD(dt->tm_mday);
+ buf[6] = BIN2BCD(dt->tm_mon);
if (dt->tm_year >= 2000) {
val = dt->tm_year - 2000;
buf[6] |= (1 << 7);
} else {
val = dt->tm_year - 1900;
}
- buf[7] = BIN_TO_BCD(val);
+ buf[7] = BIN2BCD(val);
msg[0].addr = client->addr;
msg[0].flags = 0;
Add support for DS1339. The only difference against DS1337 is Trickle
Charge register at address 10h, which is used to enable battery or gold
cap charging. Please note that value may vary for different batteries,
so it should be made module parameter. 0xaa is sane default and also
matches my board ;-)
--- linux-omap/drivers/i2c/chips/ds1337.c.orig 2005-04-08 00:47:15.655878832 +0200
+++ linux-omap/drivers/i2c/chips/ds1337.c 2005-04-08 01:10:41.662133392 +0200
@@ -31,6 +31,7 @@
#define DS1337_REG_MONTH 5
#define DS1337_REG_CONTROL 14
#define DS1337_REG_STATUS 15
+#define DS1339_REG_CHARGE 16
/* FIXME - how do we export these interface constants? */
#define DS1337_GET_DATE 0
@@ -42,7 +43,7 @@
static unsigned short normal_i2c[] = { 0x68, I2C_CLIENT_END };
static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
-SENSORS_INSMOD_1(ds1337);
+SENSORS_INSMOD_2(ds1337, ds1339);
static int ds1337_attach_adapter(struct i2c_adapter *adapter);
static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind);
@@ -78,6 +79,8 @@ struct ds1337_data {
static int ds1337_id;
static LIST_HEAD(ds1337_clients);
+static int ds1339_charge = 0xaa;
+
static inline int ds1337_read(struct i2c_client *client, u8 reg, u8 *value)
{
s32 tmp = i2c_smbus_read_byte_data(client, reg);
@@ -90,6 +93,37 @@ static inline int ds1337_read(struct i2c
return 0;
}
+/*
+ * DS1339 has Trickle Charge register at address 10h. During a multibyte
+ * access, when the address pointer reaches the end of the register space,
+ * it wraps around to location 00h.
+ * We read 17 bytes from device and compare first and last one. If they are
+ * same it is most likely DS1337 chip.
+ */
+static int ds1337_is_ds1339(struct i2c_client *client)
+{
+ char buf[17], addr = 0;
+ struct i2c_msg msg[2];
+ int result;
+
+ msg[0].addr = client->addr;
+ msg[0].flags = 0;
+ msg[0].len = 1;
+ msg[0].buf = &addr;
+
+ msg[1].addr = client->addr;
+ msg[1].flags = I2C_M_RD;
+ msg[1].len = sizeof(buf);
+ msg[1].buf = buf;
+
+ result = i2c_transfer(client->adapter, msg, 2);
+ if (result < 0) {
+ dev_err(&client->dev, "error reading data! %d\n", result);
+ return 0;
+ } else
+ return (buf[0] == buf[16]) ? 0 : 1;
+}
+
/*
* Chip access functions
*/
@@ -246,7 +280,7 @@ static int ds1337_detect(struct i2c_adap
struct i2c_client *new_client;
struct ds1337_data *data;
int err = 0;
- const char *name = "";
+ const char *name;
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
I2C_FUNC_I2C))
@@ -318,11 +352,24 @@ static int ds1337_detect(struct i2c_adap
(data & 0x60))
goto exit_free;
- kind = ds1337;
+ /* Check whenever we have DS1339 */
+ if (ds1337_is_ds1339(new_client))
+ kind = ds1339;
+ else
+ kind = ds1337;
+
}
- if (kind == ds1337)
+ switch (kind) {
+ case ds1337:
name = "ds1337";
+ break;
+ case ds1339:
+ name = "ds1339";
+ break;
+ default:
+ name = "";
+ }
/* We can fill in the remaining client fields */
strlcpy(new_client->name, name, I2C_NAME_SIZE);
@@ -334,6 +381,15 @@ static int ds1337_detect(struct i2c_adap
/* Initialize the DS1337 chip */
ds1337_init_client(new_client);
+ /* Be nice to battery */
+ if (kind == ds1339 && ds1339_charge) {
+ char buf[] = { DS1339_REG_CHARGE, ds1339_charge };
+
+ if (i2c_master_send(new_client, buf, 2) != 2)
+ dev_err(&new_client->dev,
+ "failed to enable trickle charge.\n");
+ }
+
/* Add client to local list */
data->id = ds1337_id++;
list_add(&data->list, &ds1337_clients);
On Thu, Apr 07, 2005 at 02:18:39PM -0700, Greg KH wrote:
> Jean's point is that you should send an individual patch for each type
> of individual change. It's ok to say "patch 3 requires you to have
> applied patches 1 and 2" and so on. Please split this up better.
Here it is...
Use i2c_transfer to send message, so we get proper bus locking.
===== drivers/i2c/chips/ds1337.c 1.1 vs edited =====
--- 1.1/drivers/i2c/chips/ds1337.c 2005-03-31 22:58:08 +02:00
+++ edited/drivers/i2c/chips/ds1337.c 2005-04-08 00:18:45 +02:00
@@ -3,7 +3,7 @@
*
* Copyright (C) 2005 James Chapman <[email protected]>
*
- * based on linux/drivers/acron/char/pcf8583.c
+ * based on linux/drivers/acorn/char/pcf8583.c
* Copyright (C) 2000 Russell King
*
* This program is free software; you can redistribute it and/or modify
@@ -119,8 +119,7 @@
msg[1].len = sizeof(buf);
msg[1].buf = &buf[0];
- result = client->adapter->algo->master_xfer(client->adapter,
- &msg[0], 2);
+ result = i2c_transfer(client->adapter, msg, 2);
dev_dbg(&client->adapter->dev,
"%s: [%d] %02x %02x %02x %02x %02x %02x %02x\n",
@@ -194,8 +193,7 @@
msg[0].len = sizeof(buf);
msg[0].buf = &buf[0];
- result = client->adapter->algo->master_xfer(client->adapter,
- &msg[0], 1);
+ result = i2c_transfer(client->adapter, msg, 1);
if (result < 0) {
dev_err(&client->adapter->dev, "ds1337[%d]: error "
"writing data! %d\n", data->id, result);
On Fri, Apr 08, 2005 at 01:17:58AM +0200, Ladislav Michl wrote:
> On Thu, Apr 07, 2005 at 02:18:39PM -0700, Greg KH wrote:
> > Jean's point is that you should send an individual patch for each type
> > of individual change. It's ok to say "patch 3 requires you to have
> > applied patches 1 and 2" and so on. Please split this up better.
>
> Here it is...
>
> Use i2c_transfer to send message, so we get proper bus locking.
Oops, you forgot to add a Signed-off-by: line for every patch, as per
Documentation/SubmittingPatches. Care to redo them?
thanks,
greg k-h
Hi Ladislav,
> Use i2c_transfer to send message, so we get proper bus locking.
Looks all OK to me, let alone the lack of Signed-off-by line, as Greg
underlined elsewhere. Please resent the patches with the Signed-off-by
line after I finish reviewing them.
Thanks,
--
Jean Delvare
Hi Ladislav,
> Use correct macros to convert between bdc and bin. See linux/bcd.h
Yes, this one is OK with me too.
Thanks,
--
Jean Delvare
Hi Ladislav,
> dev_{dbg,err} functions should print client's device name. data->id can
> be dropped from message, because device is determined by bus it hangs on
> (it has fixed address).
Looks OK to me.
Thanks,
--
Jean Delvare
Hi Ladislav,
> Add support for DS1339. The only difference against DS1337 is Trickle
> Charge register at address 10h, which is used to enable battery or gold
> cap charging. Please note that value may vary for different batteries,
> so it should be made module parameter. 0xaa is sane default and also
> matches my board ;-)
"Sane default" is a non-sense here. A sane default is that loading a
real-time clock driver should not affect the battery at all IMHO.
Can you tell us more on that battery thing? I admit I don't exatcly get
what it is. Which type of battery are we talking about? Are there
similar drivers in the kernel tree already?
Sounds weird to me that loading a driver would enable charging of a
battery, and removing it wouldn't disable it. And since the driver
might not be removed, it would possibly make more sense to have an entry
in /sys to enable and disable this thing.
Also, arbitrarily picking one of the 6 possible charging modes just
because it matches your board is a bad idea. It looks like a value which
should be set on a per-board basis, rather than picked randomly by the
user, so as to avoid accidental hardware breakage.
> +static int ds1339_charge = 0xaa;
I see little reason why this would be a global variable rather than a
define (let alone the fact that this shouldn't be hardcoded at all
IMHO, as I just explained).
> +/*
> + * DS1339 has Trickle Charge register at address 10h. During a multibyte
> + * access, when the address pointer reaches the end of the register space,
> + * it wraps around to location 00h.
> + * We read 17 bytes from device and compare first and last one. If they
> + * are same it is most likely DS1337 chip.
> + */
> +static int ds1337_is_ds1339(struct i2c_client *client)
> +{
> + char buf[17], addr = 0;
> + struct i2c_msg msg[2];
> + int result;
> +
> + msg[0].addr = client->addr;
> + msg[0].flags = 0;
> + msg[0].len = 1;
> + msg[0].buf = &addr;
> +
> + msg[1].addr = client->addr;
> + msg[1].flags = I2C_M_RD;
> + msg[1].len = sizeof(buf);
> + msg[1].buf = buf;
> +
> + result = i2c_transfer(client->adapter, msg, 2);
> + if (result < 0) {
> + dev_err(&client->dev, "error reading data! %d\n", result);
> + return 0;
> + } else
> + return (buf[0] == buf[16]) ? 0 : 1;
> +}
This will fail eventually. The first register is the seconds count, which
by definition is changing. I2C is slow, by the time you wrap over the
register range, the value could have changed. The datasheet explicitely
says that the register cache will refresh on address wrapping.
Also, 0x00 is a possible value for both the seconds count and the battery
register, so you could miss a DS1339 at times.
One possible check to start with would be on the value of the additional
register itself. It has only 7 possible values. But of course it would
be better if we could default to a DS1337 and find a way to identify the
DS1339, rather than the (unsafe) other way around. I also don't know
what a DS1337 will answer for this missing register. Maybe James can
help?
One possibility would be to start reading at 0x0E instead of 0x00,
because register 0x00 is the control register and is the only one which
will not change in our back as far as I can see. Oh, and the additional
battery register too. But this still doesn't sound like a bulletproof
method to me (we depend on the seconds and possibly minutes count
again). So it would be better to additionally perform the same tests
that were done on the non-wrapped registers for the regular DS1337
detection, but on the wrapped area.
The problem here is that all this will significantly increase the
detection delay.
Yet another method would be to write a non-significant value to the
battery register, such as 0x80. If we can read it back then it has to be
a DS1339. But what effect will it have on the DS1337? I'd hope none,
but this better be verified. And in general I don't much like using
register writes in detection methods.
Could you please provide the output of i2cdump in b and c modes for your
DS1339 chip? This might help finding an detection method. A similar dump
for the DS1337 would help too.
> - const char *name = "";
> + const char *name;
(...)
> + default:
> + name = "";
> + }
This is noise.
Thanks,
--
Jean Delvare
On Fri, Apr 08, 2005 at 01:08:46PM +0200, Jean Delvare wrote:
> > Add support for DS1339. The only difference against DS1337 is Trickle
> > Charge register at address 10h, which is used to enable battery or gold
> > cap charging. Please note that value may vary for different batteries,
> > so it should be made module parameter. 0xaa is sane default and also
> > matches my board ;-)
>
> "Sane default" is a non-sense here. A sane default is that loading a
> real-time clock driver should not affect the battery at all IMHO.
>
> Can you tell us more on that battery thing? I admit I don't exatcly get
> what it is. Which type of battery are we talking about? Are there
> similar drivers in the kernel tree already?
Sorry, I have no clue what devices are using it and what may come to
board designer's mind. This chip will be most likely used in embedded,
where every sane developer is expected to review drivers he is using.
Also in such situation driver is compiled staticaly. (And in ideal world
firmware (such as U-Boot, RedBoot, etc.) should set this register).
> Sounds weird to me that loading a driver would enable charging of a
> battery, and removing it wouldn't disable it. And since the driver
> might not be removed, it would possibly make more sense to have an entry
> in /sys to enable and disable this thing.
Disabling battery charge makes no sense. The only reason I can think
about is when suspending device, so it is likely pm job. /sys entry
might help as well, but I do not see any point making driver more
complicated and bigger just to make someone else happy.
Golden rule is: implement features as needed :)
> Also, arbitrarily picking one of the 6 possible charging modes just
> because it matches your board is a bad idea. It looks like a value which
> should be set on a per-board basis, rather than picked randomly by the
> user, so as to avoid accidental hardware breakage.
Well free to provide that way, so far I'm the only user so I did what is
usefull for me [*]. Everyone is welcome to change it to more generic
way.
> > +static int ds1339_charge = 0xaa;
>
> I see little reason why this would be a global variable rather than a
> define (let alone the fact that this shouldn't be hardcoded at all
> IMHO, as I just explained).
[*] This way it can be easily turned into module parameter or whatever
someone who will need different value may want to do.
Lets choose define, later it can be converted to module parameter.
Gcc is smart enough to optimize charge enable code away.
#define ds1339_charge 0
> > +/*
> > + * DS1339 has Trickle Charge register at address 10h. During a multibyte
> > + * access, when the address pointer reaches the end of the register space,
> > + * it wraps around to location 00h.
> > + * We read 17 bytes from device and compare first and last one. If they
> > + * are same it is most likely DS1337 chip.
> > + */
> > +static int ds1337_is_ds1339(struct i2c_client *client)
> > +{
> > + char buf[17], addr = 0;
> > + struct i2c_msg msg[2];
> > + int result;
> > +
> > + msg[0].addr = client->addr;
> > + msg[0].flags = 0;
> > + msg[0].len = 1;
> > + msg[0].buf = &addr;
> > +
> > + msg[1].addr = client->addr;
> > + msg[1].flags = I2C_M_RD;
> > + msg[1].len = sizeof(buf);
> > + msg[1].buf = buf;
> > +
> > + result = i2c_transfer(client->adapter, msg, 2);
> > + if (result < 0) {
> > + dev_err(&client->dev, "error reading data! %d\n", result);
> > + return 0;
> > + } else
> > + return (buf[0] == buf[16]) ? 0 : 1;
> > +}
>
> This will fail eventually. The first register is the seconds count, which
> by definition is changing. I2C is slow, by the time you wrap over the
> register range, the value could have changed. The datasheet explicitely
> says that the register cache will refresh on address wrapping.
I was running test overnight and didn't meet any single case when this
happen. Perhaps device also needs to see start condition?
> Also, 0x00 is a possible value for both the seconds count and the battery
> register, so you could miss a DS1339 at times.
>
> One possible check to start with would be on the value of the additional
> register itself. It has only 7 possible values. (...)
Eh? Register is 8bit, that's 256 combinations.
> (...) But of course it would be better if we could default to a DS1337
> and find a way to identify the DS1339, rather than the (unsafe) other
> way around. I also don't know what a DS1337 will answer for this
> missing register. Maybe James can help?
>
> One possibility would be to start reading at 0x0E instead of 0x00,
> because register 0x00 is the control register and is the only one which
> will not change in our back as far as I can see. Oh, and the additional
> battery register too. But this still doesn't sound like a bulletproof
> method to me (we depend on the seconds and possibly minutes count
> again). So it would be better to additionally perform the same tests
> that were done on the non-wrapped registers for the regular DS1337
> detection, but on the wrapped area.
>
> The problem here is that all this will significantly increase the
> detection delay.
That's probably overkill, see above.
Look, the only difference between ds1339 and ds1337 is Trickle Charge
register. We won't touch it by default and if anyone wants to use it, he
need to provide its value. In that case he also knows it is DS1339 and
also knows what battery is wired, so he knows charging current.
I hope it is also clear that without picking one "sane default" there is
no point to do any detection at all.
> Yet another method would be to write a non-significant value to the
> battery register, such as 0x80. If we can read it back then it has to be
> a DS1339. But what effect will it have on the DS1337? I'd hope none,
> but this better be verified. And in general I don't much like using
> register writes in detection methods.
You will change register 00h.
> Could you please provide the output of i2cdump in b and c modes for your
> DS1339 chip? This might help finding an detection method. A similar dump
> for the DS1337 would help too.
Sorry it seems there is no easy way to cross-compile this package. I
have admit I haven't tried too hard.
> > - const char *name = "";
> > + const char *name;
> (...)
> > + default:
> > + name = "";
> > + }
>
> This is noise.
No it is not. Look at generated machine code for each case and try to do
it for at least two architectures different from i386 ;-) I'm bored with
arguing about such changes, do what you think is right, but it will
remain in patch. The rest is up to you.
Also note that this very way of rejecting tiny cleanups leads to hardly
readable and unoptimized code and after some greater amount of time code
rewrite occurs anyway (once someone is sufficienly upset).
Have a nice weekend,
ladis
On Thu, Apr 07, 2005 at 04:36:29PM -0700, Greg KH wrote:
> Oops, you forgot to add a Signed-off-by: line for every patch, as per
> Documentation/SubmittingPatches. Care to redo them?
Here it is (I'm sorry about that).
Use i2c_transfer to send message, so we get proper bus locking.
Signed-off-by: Ladislav Michl <[email protected]>
===== drivers/i2c/chips/ds1337.c 1.1 vs edited =====
--- 1.1/drivers/i2c/chips/ds1337.c 2005-03-31 22:58:08 +02:00
+++ edited/drivers/i2c/chips/ds1337.c 2005-04-08 00:18:45 +02:00
@@ -3,7 +3,7 @@
*
* Copyright (C) 2005 James Chapman <[email protected]>
*
- * based on linux/drivers/acron/char/pcf8583.c
+ * based on linux/drivers/acorn/char/pcf8583.c
* Copyright (C) 2000 Russell King
*
* This program is free software; you can redistribute it and/or modify
@@ -119,8 +119,7 @@
msg[1].len = sizeof(buf);
msg[1].buf = &buf[0];
- result = client->adapter->algo->master_xfer(client->adapter,
- &msg[0], 2);
+ result = i2c_transfer(client->adapter, msg, 2);
dev_dbg(&client->adapter->dev,
"%s: [%d] %02x %02x %02x %02x %02x %02x %02x\n",
@@ -194,8 +193,7 @@
msg[0].len = sizeof(buf);
msg[0].buf = &buf[0];
- result = client->adapter->algo->master_xfer(client->adapter,
- &msg[0], 1);
+ result = i2c_transfer(client->adapter, msg, 1);
if (result < 0) {
dev_err(&client->adapter->dev, "ds1337[%d]: error "
"writing data! %d\n", data->id, result);
On Fri, Apr 08, 2005 at 10:51:00AM +0200, Jean Delvare wrote:
> Yes, this one is OK with me too.
Ok, here it is with signed off line.
Use correct macros to convert between bdc and bin. See linux/bcd.h
Signed-off-by: Ladislav Michl <[email protected]>
--- linux-omap/drivers/i2c/chips/ds1337.c.orig 2005-04-08 00:32:45.234203040 +0200
+++ linux-omap/drivers/i2c/chips/ds1337.c 2005-04-08 00:34:58.457949952 +0200
@@ -127,15 +127,15 @@
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]);
+ dt->tm_sec = BCD2BIN(buf[0]);
+ dt->tm_min = BCD2BIN(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]);
+ dt->tm_hour = BCD2BIN(val);
+ dt->tm_wday = BCD2BIN(buf[3]) - 1;
+ dt->tm_mday = BCD2BIN(buf[4]);
val = buf[5] & 0x7f;
- dt->tm_mon = BCD_TO_BIN(val);
- dt->tm_year = 1900 + BCD_TO_BIN(buf[6]);
+ dt->tm_mon = BCD2BIN(val);
+ dt->tm_year = 1900 + BCD2BIN(buf[6]);
if (buf[5] & 0x80)
dt->tm_year += 100;
@@ -174,19 +174,19 @@
dt->tm_mday, dt->tm_mon, dt->tm_year, dt->tm_wday);
buf[0] = 0; /* reg offset */
- buf[1] = BIN_TO_BCD(dt->tm_sec);
- buf[2] = BIN_TO_BCD(dt->tm_min);
- buf[3] = BIN_TO_BCD(dt->tm_hour) | (1 << 6);
- buf[4] = BIN_TO_BCD(dt->tm_wday) + 1;
- buf[5] = BIN_TO_BCD(dt->tm_mday);
- buf[6] = BIN_TO_BCD(dt->tm_mon);
+ buf[1] = BIN2BCD(dt->tm_sec);
+ buf[2] = BIN2BCD(dt->tm_min);
+ buf[3] = BIN2BCD(dt->tm_hour) | (1 << 6);
+ buf[4] = BIN2BCD(dt->tm_wday) + 1;
+ buf[5] = BIN2BCD(dt->tm_mday);
+ buf[6] = BIN2BCD(dt->tm_mon);
if (dt->tm_year >= 2000) {
val = dt->tm_year - 2000;
buf[6] |= (1 << 7);
} else {
val = dt->tm_year - 1900;
}
- buf[7] = BIN_TO_BCD(val);
+ buf[7] = BIN2BCD(val);
msg[0].addr = client->addr;
msg[0].flags = 0;
On Fri, Apr 08, 2005 at 12:08:38PM +0200, Jean Delvare wrote:
> Looks OK to me.
Ok, I have few more fixes for this driver and will send them later
when I find time to split them out into smaller chunks. Again, here is
patch with signed off line.
dev_{dbg,err} functions should print client's device name. data->id can
be dropped from message, because device is determined by bus it hangs on
(it has fixed address).
Signed-off-by: Ladislav Michl <[email protected]>
--- linux-omap/drivers/i2c/chips/ds1337.c.orig 2005-04-08 00:36:15.072302800 +0200
+++ linux-omap/drivers/i2c/chips/ds1337.c 2005-04-08 00:44:44.130914128 +0200
@@ -95,7 +95,6 @@ static inline int ds1337_read(struct i2c
*/
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;
@@ -103,9 +102,7 @@ static int ds1337_get_datetime(struct i2
u8 offs = 0;
if (!dt) {
- dev_dbg(&client->adapter->dev, "%s: EINVAL: dt=NULL\n",
- __FUNCTION__);
-
+ dev_dbg(&client->dev, "%s: EINVAL: dt=NULL\n", __FUNCTION__);
return -EINVAL;
}
@@ -121,8 +118,7 @@ static int ds1337_get_datetime(struct i2
result = i2c_transfer(client->adapter, msg, 2);
- dev_dbg(&client->adapter->dev,
- "%s: [%d] %02x %02x %02x %02x %02x %02x %02x\n",
+ dev_dbg(&client->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]);
@@ -139,14 +135,13 @@ static int ds1337_get_datetime(struct i2
if (buf[5] & 0x80)
dt->tm_year += 100;
- dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, "
+ dev_dbg(&client->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);
+ dev_err(&client->dev, "error reading data! %d\n", result);
result = -EIO;
}
@@ -155,20 +150,17 @@ static int ds1337_get_datetime(struct i2
static int ds1337_set_datetime(struct i2c_client *client, struct rtc_time *dt)
{
- struct ds1337_data *data = i2c_get_clientdata(client);
int result;
u8 buf[8];
u8 val;
struct i2c_msg msg[1];
if (!dt) {
- dev_dbg(&client->adapter->dev, "%s: EINVAL: dt=NULL\n",
- __FUNCTION__);
-
+ dev_dbg(&client->dev, "%s: EINVAL: dt=NULL\n", __FUNCTION__);
return -EINVAL;
}
- dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, hours=%d, "
+ dev_dbg(&client->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);
@@ -195,8 +187,7 @@ static int ds1337_set_datetime(struct i2
result = i2c_transfer(client->adapter, msg, 1);
if (result < 0) {
- dev_err(&client->adapter->dev, "ds1337[%d]: error "
- "writing data! %d\n", data->id, result);
+ dev_err(&client->dev, "error writing data! %d\n", result);
result = -EIO;
} else {
result = 0;
@@ -208,7 +199,7 @@ static int ds1337_set_datetime(struct i2
static int ds1337_command(struct i2c_client *client, unsigned int cmd,
void *arg)
{
- dev_dbg(&client->adapter->dev, "%s: cmd=%d\n", __FUNCTION__, cmd);
+ dev_dbg(&client->dev, "%s: cmd=%d\n", __FUNCTION__, cmd);
switch (cmd) {
case DS1337_GET_DATE:
Hi Ladislav,
> (...) (And in ideal world
> firmware (such as U-Boot, RedBoot, etc.) should set this register).
Thanks for pointing the obvious out to me. You convinced me, this simply
doesn't belong to the kernel. So your patch is not needed.
I would still welcome a patch documenting the fact that the DS1339 is
compatible with the DS1337 and is supported by the ds1337 driver.
Thanks,
--
Jean Delvare
Sorry for joining this thread late.
Patches 1-3 are fine with me.
/james
Ladislav Michl wrote:
> On Thu, Apr 07, 2005 at 04:36:29PM -0700, Greg KH wrote:
>
>>Oops, you forgot to add a Signed-off-by: line for every patch, as per
>>Documentation/SubmittingPatches. Care to redo them?
>
>
> Here it is (I'm sorry about that).
>
> Use i2c_transfer to send message, so we get proper bus locking.
>
> Signed-off-by: Ladislav Michl <[email protected]>
>
Ladislav Michl wrote:
> On Fri, Apr 08, 2005 at 01:08:46PM +0200, Jean Delvare wrote:
>
>>>Add support for DS1339. The only difference against DS1337 is Trickle
>>>Charge register at address 10h, which is used to enable battery or gold
>>>cap charging. Please note that value may vary for different batteries,
>>>so it should be made module parameter. 0xaa is sane default and also
>>>matches my board ;-)
>>
>>"Sane default" is a non-sense here. A sane default is that loading a
>>real-time clock driver should not affect the battery at all IMHO.
>>
>>Can you tell us more on that battery thing? I admit I don't exatcly get
>>what it is. Which type of battery are we talking about? Are there
>>similar drivers in the kernel tree already?
>
>
> Sorry, I have no clue what devices are using it and what may come to
> board designer's mind. This chip will be most likely used in embedded,
> where every sane developer is expected to review drivers he is using.
> Also in such situation driver is compiled staticaly. (And in ideal world
> firmware (such as U-Boot, RedBoot, etc.) should set this register).
>
>
>>Sounds weird to me that loading a driver would enable charging of a
>>battery, and removing it wouldn't disable it. And since the driver
>>might not be removed, it would possibly make more sense to have an entry
>>in /sys to enable and disable this thing.
>
>
> Disabling battery charge makes no sense.
If it causes more power to be drawn unnecessarily then battery charge
should be enabled only when needed.
> The only reason I can think
> about is when suspending device, so it is likely pm job. /sys entry
> might help as well, but I do not see any point making driver more
> complicated and bigger just to make someone else happy.
Why not add a new /sys entry for it? Is there a generic battery charge
control /sys API?
> Golden rule is: implement features as needed :)
But when adding code, try to cover all reasonable cases, otherwise we'll
see patches from people trying to add platform specific ifdefs in here.
>>Also, arbitrarily picking one of the 6 possible charging modes just
>>because it matches your board is a bad idea. It looks like a value which
>>should be set on a per-board basis, rather than picked randomly by the
>>user, so as to avoid accidental hardware breakage.
>
>
> Well free to provide that way, so far I'm the only user so I did what is
> usefull for me [*]. Everyone is welcome to change it to more generic
> way.
I agree with Jean. You should provide an API for this. Don't take
shortcuts just because you were the first to support the chip. It'll be
more useful to others if you provide a way to set a value per platform.
>>>+/*
>>>+ * DS1339 has Trickle Charge register at address 10h. During a multibyte
>>>+ * access, when the address pointer reaches the end of the register space,
>>>+ * it wraps around to location 00h.
>>>+ * We read 17 bytes from device and compare first and last one. If they
>>>+ * are same it is most likely DS1337 chip.
>>>+ */
>>>+static int ds1337_is_ds1339(struct i2c_client *client)
>>>+{
>>>+ char buf[17], addr = 0;
>>>+ struct i2c_msg msg[2];
>>>+ int result;
>>>+
>>>+ msg[0].addr = client->addr;
>>>+ msg[0].flags = 0;
>>>+ msg[0].len = 1;
>>>+ msg[0].buf = &addr;
>>>+
>>>+ msg[1].addr = client->addr;
>>>+ msg[1].flags = I2C_M_RD;
>>>+ msg[1].len = sizeof(buf);
>>>+ msg[1].buf = buf;
>>>+
>>>+ result = i2c_transfer(client->adapter, msg, 2);
>>>+ if (result < 0) {
>>>+ dev_err(&client->dev, "error reading data! %d\n", result);
>>>+ return 0;
>>>+ } else
>>>+ return (buf[0] == buf[16]) ? 0 : 1;
>>>+}
>>
>>This will fail eventually. The first register is the seconds count, which
>>by definition is changing. I2C is slow, by the time you wrap over the
>>register range, the value could have changed. The datasheet explicitely
>>says that the register cache will refresh on address wrapping.
>
> I was running test overnight and didn't meet any single case when this
> happen. Perhaps device also needs to see start condition?
Just because it runs overnight doesn't mean there's no bug.
>>Also, 0x00 is a possible value for both the seconds count and the battery
>>register, so you could miss a DS1339 at times.
>>
>>One possible check to start with would be on the value of the additional
>>register itself. It has only 7 possible values. (...)
>
>
> Eh? Register is 8bit, that's 256 combinations.
Reserved bits have fixed values that you can test for.
>>(...) But of course it would be better if we could default to a DS1337
>>and find a way to identify the DS1339, rather than the (unsafe) other
>>way around.
I agree.
I also don't know what a DS1337 will answer for this
>>missing register. Maybe James can help?
I don't know. If I can find some time, I'll apply your patches and find out.
>>
>>One possibility would be to start reading at 0x0E instead of 0x00,
>>because register 0x00 is the control register and is the only one which
>>will not change in our back as far as I can see. Oh, and the additional
>>battery register too. But this still doesn't sound like a bulletproof
>>method to me (we depend on the seconds and possibly minutes count
>>again). So it would be better to additionally perform the same tests
>>that were done on the non-wrapped registers for the regular DS1337
>>detection, but on the wrapped area.
>>
>>The problem here is that all this will significantly increase the
>>detection delay.
>
> That's probably overkill, see above.
>
> Look, the only difference between ds1339 and ds1337 is Trickle Charge
> register. We won't touch it by default and if anyone wants to use it, he
> need to provide its value. In that case he also knows it is DS1339 and
> also knows what battery is wired, so he knows charging current.
There are lots of i2c devices which differ only by one or two registers.
You are modifying this driver to report the new device so the device
detection must be reliable.
The problem here is how to distinguish the difference between a read of
ds1339's battery register 0x10 and a ds1337's register 0x00.
>
> I hope it is also clear that without picking one "sane default" there is
> no point to do any detection at all.
I don't understand what you are trying to say.
>>Yet another method would be to write a non-significant value to the
>>battery register, such as 0x80. If we can read it back then it has to be
>>a DS1339. But what effect will it have on the DS1337? I'd hope none,
>>but this better be verified. And in general I don't much like using
>>register writes in detection methods.
>
>
> You will change register 00h.
Is writing 0x80 to register 0x10 valid for the ds1339?
This might be a good test but should be verified. According to ds1337's
docs, writing 0x80 to register 0x00 would try to set a reserved bit so
should read back 0 on a ds1337. But docs can be wrong and trying to set
that bit might cause problems. I'll try to find out on my hardware.
/james
On Fri, Apr 08, 2005 at 06:44:53PM +0100, James Chapman wrote:
> > The only reason I can think
> >about is when suspending device, so it is likely pm job. /sys entry
> >might help as well, but I do not see any point making driver more
> >complicated and bigger just to make someone else happy.
>
> Why not add a new /sys entry for it? Is there a generic battery charge
> control /sys API?
I do not know about any and I do not think it would be usefull here. How
would you convert value passed by API into register value? Driver has no
chance to know about hardware design.
> >Golden rule is: implement features as needed :)
>
> But when adding code, try to cover all reasonable cases, otherwise we'll
> see patches from people trying to add platform specific ifdefs in here.
I'd like to, but simply do not have enough informations to do it. In other
worlds I still do not know what is reasonable here.
> >>Also, arbitrarily picking one of the 6 possible charging modes just
> >>because it matches your board is a bad idea. It looks like a value which
> >>should be set on a per-board basis, rather than picked randomly by the
> >>user, so as to avoid accidental hardware breakage.
> >
> >
> >Well free to provide that way, so far I'm the only user so I did what is
> >usefull for me [*]. Everyone is welcome to change it to more generic
> >way.
>
> I agree with Jean. You should provide an API for this. Don't take
> shortcuts just because you were the first to support the chip. It'll be
> more useful to others if you provide a way to set a value per platform.
That's not about taking any shortcuts, that's about finding sane API. When
time shows it wrong and it will need change people will complain about
compatibility. See my questions about API bellow [*].
Based on your and Jean's input, following so far sounds reasonable:
Create "charge" sysfs entry for ds1339 when it is detected. Do not write
any value to Trickle Charge register, until its value is written to this
entry.
> >I was running test overnight and didn't meet any single case when this
> >happen. Perhaps device also needs to see start condition?
>
> Just because it runs overnight doesn't mean there's no bug.
Agree, but with probability near the certainty I can tell that device works
a bit differently than described in datasheet. Anyway, new 100% reliable
test is done, so it could be eventually used if ds1339 support finds its
way into driver.
> >>Also, 0x00 is a possible value for both the seconds count and the battery
> >>register, so you could miss a DS1339 at times.
> >>
> >>One possible check to start with would be on the value of the additional
> >>register itself. It has only 7 possible values. (...)
> >
> >
> >Eh? Register is 8bit, that's 256 combinations.
>
> Reserved bits have fixed values that you can test for.
Think about this register as about NVRAM address. It can have any value,
but only certain values will enable charge.
[*] Now lets forget ds1339, there are few things I'd like to know about
this driver.
How are you using this driver? There is non-static function
ds1337_do_command which expects id. How do you know which id belongs
to which chip? Do you actually have machine with more than one ds1337?
Chip has fixed address, so only one can hang on one bus (am I right?)
Also it is unlikely that machine will have more that one RTC. The only
exception which comes on mind are NUMA systems which have one RTC per
node. Anything else? Why date format returned/expected by this driver
differs from other drivers?
Thanks in advance,
ladis
PS. I'm sorry about some formulations I used in earlier mails. I was
overworked and tired and that affected my otherwise (hopefully) good
decorum :)
Hi Ladislav,
> Driver has no chance to know about hardware design.
If you want the driver to somehow interact with the battery charging
register, it will have to. We just can't let the user write random value
to this register.
> Based on your and Jean's input, following so far sounds reasonable:
> Create "charge" sysfs entry for ds1339 when it is detected. Do not
> write any value to Trickle Charge register, until its value is written
> to this entry.
While I admit I had this in mind in the first place, the more I think of
it and the less I like it. It's slightly better than changing the
charging rate right when loading the driver, but that's still dangerous.
Users could write a value which doesn't match the hardware design, and
bad things could happen.
> Agree, but with probability near the certainty I can tell that device
> works a bit differently than described in datasheet. Anyway, new 100%
> reliable test is done, so it could be eventually used if ds1339
> support finds its way into driver.
"100% realiable" here means that it works for you, which isn't enough.
At least James would have to check how it works with his DS1337, and
there might be other revisions of both chips which behave differently.
There might be other mostly-compatible chips in the game too.
> > > Eh? Register is 8bit, that's 256 combinations.
> >
> > Reserved bits have fixed values that you can test for.
>
> Think about this register as about NVRAM address. It can have any
> value, but only certain values will enable charge.
Most of which nobody has any interest in writing. Some I2C devices are
hard to detect and the DS1337/DS1339 are of these. We use the tricks we
find. Sure, a strict check on this register might miss a DS1339, but
that's better than detecting a different chip as a DS1339.
> How are you using this driver? There is non-static function
> ds1337_do_command which expects id. How do you know which id belongs
> to which chip?
I second this question, as it stroke me too. This function doesn't sound
exactly usable to me. Identifying the device by bus and address would
make more sense than an arbitrary id you have no way to learn about.
> Do you actually have machine with more than one ds1337?
> Chip has fixed address, so only one can hang on one bus (am I right?)
You are.
> PS. I'm sorry about some formulations I used in earlier mails. I was
> overworked and tired and that affected my otherwise (hopefully) good
> decorum :)
I'm really happy to read this :) You are of course forgiven. We all have
bad days.
Back to the issue, some random thoughts summarizing my opinion:
1* Initializing the battery charge register is a firmware/bios issue, as
you underlined earlier. It would make sense (and would be easier) to
just ignore it at the driver level.
2* If it makes sense to stop the charge, then we should provide a simple
*switch* to the user, from the default charging register value (as
previously set by the firmware/bios) to 0 and back. The switch would
probably be a sysfs file unless a different API already exists.
3* Having the driver write an arbitrary non-0 value to the register
should not be done unless the system has been identified. I have no idea
how your system can be identified (DMI?), but if it can't, then I'd
better see the register ignored altogether.
4* Remember that you can always write a simple C tool relying on the
i2c-dev interface to do the job. The advantage of this approach is that
you can put big fat warnings and request user confirmation before any
action.
Hope that helps,
--
Jean Delvare
Jean Delvare wrote:
>>Based on your and Jean's input, following so far sounds reasonable:
>>Create "charge" sysfs entry for ds1339 when it is detected. Do not
>>write any value to Trickle Charge register, until its value is written
>>to this entry.
>
> While I admit I had this in mind in the first place, the more I think of
> it and the less I like it. It's slightly better than changing the
> charging rate right when loading the driver, but that's still dangerous.
> Users could write a value which doesn't match the hardware design, and
> bad things could happen.
I had assumed Ladislav wanted to be able to change this charge rate at
any time, which was the motivation behind adding ds1339 support.
>>How are you using this driver? There is non-static function
>>ds1337_do_command which expects id. How do you know which id belongs
>>to which chip?
>
> I second this question, as it stroke me too. This function doesn't sound
> exactly usable to me. Identifying the device by bus and address would
> make more sense than an arbitrary id you have no way to learn about.
It is used by the Radstone ppc7d platform, arch/ppc/radstone_ppc7d.c
but wasn't added until very recently (2.6.12-rc2 I think).
To be honest, I meant to remove the 'id' thing before submitting the
driver. There's no need to support more than one of these devices.
>>Do you actually have machine with more than one ds1337?
>>Chip has fixed address, so only one can hang on one bus (am I right?)
>
> You are.
Yep. I think the id should be removed asap.
> Back to the issue, some random thoughts summarizing my opinion:
>
> 1* Initializing the battery charge register is a firmware/bios issue, as
> you underlined earlier. It would make sense (and would be easier) to
> just ignore it at the driver level.
Initializing the charge register should be done by the bios if possible.
However, I assume Ladislav still wants to be able to change the register
at runtime so some kernel support is needed?
> 2* If it makes sense to stop the charge, then we should provide a simple
> *switch* to the user, from the default charging register value (as
> previously set by the firmware/bios) to 0 and back. The switch would
> probably be a sysfs file unless a different API already exists.
>
> 3* Having the driver write an arbitrary non-0 value to the register
> should not be done unless the system has been identified. I have no idea
> how your system can be identified (DMI?), but if it can't, then I'd
> better see the register ignored altogether.
>
> 4* Remember that you can always write a simple C tool relying on the
> i2c-dev interface to do the job. The advantage of this approach is that
> you can put big fat warnings and request user confirmation before any
> action.
This makes sense. Ladislav, would this work for you? I guess we'd still
add code to the ds1337 driver to detect ds1339 in order to ensure that
this tool could not modify register 0 of a ds1337 by accident?
/james
On Tue, Apr 12, 2005 at 07:10:55PM +0100, James Chapman wrote:
[snip]
> It is used by the Radstone ppc7d platform, arch/ppc/radstone_ppc7d.c
> but wasn't added until very recently (2.6.12-rc2 I think).
>
> To be honest, I meant to remove the 'id' thing before submitting the
> driver. There's no need to support more than one of these devices.
Patch bellow remove ds1337_do_command function and things needed by it.
I think device should be identified by bus and address as Jean said.
Please let me know if that fits your needs.
I'm assuming that you want to use drivers/char/genrtc.c to access ds1337
from userspace, but in arch/ppc/platforms/radstone_ppc7d.c
ppc_md.get_rtc_time used by genrtc via get_rtc_time in asm-ppc/rtc.h
is set to NULL (same for set_rtc_time) and I didn't find where (if)
ds1337 registers to ppc_md.get_rtc_time.
Functions in asm-ppc/rtc.h also do magic with tm_mon and tm_year
so this driver doesn't need to handle epoch separately and doesn't need
to be aware that tm_mon starts from zero...
m68k, mips and parisc does the same in asm/rtc.h unlike arm, so I this
driver probably won't work for me without some tweaks to arm code.
[snip]
> >Back to the issue, some random thoughts summarizing my opinion:
> >
> >1* Initializing the battery charge register is a firmware/bios issue, as
> >you underlined earlier. It would make sense (and would be easier) to
> >just ignore it at the driver level.
>
> Initializing the charge register should be done by the bios if possible.
> However, I assume Ladislav still wants to be able to change the register
> at runtime so some kernel support is needed?
>
> >2* If it makes sense to stop the charge, then we should provide a simple
> >*switch* to the user, from the default charging register value (as
> >previously set by the firmware/bios) to 0 and back. The switch would
> >probably be a sysfs file unless a different API already exists.
> >
> >3* Having the driver write an arbitrary non-0 value to the register
> >should not be done unless the system has been identified. I have no idea
> >how your system can be identified (DMI?), but if it can't, then I'd
> >better see the register ignored altogether.
My board is OMAP (ARM core) based and there are ARM specific functions
(if (machine_is_xxx()) do_something(); ), but it is not what you want to
see in generic driver. It may be possible to use platform_data to pass
information to driver, but I do not like this idea.
So, if we use entry in sysfs, then only root can write it and root is
allowed to do weird things. Device itself refuses any action until high
four bits are 0xa. If that is still not enough I just found this patch
http://groups-beta.google.com/group/fa.linux.kernel/msg/06e0368f86c8f824
so you can use configfs to explicitly create "charge" entry. (
* I'm considering that an overkill
* I'm not sure if it can be easily done with configfs)
I'd add config option (disabled by default) for "charge" entry, if you
feel it is too dangerous. However I think that people should be a bit
responsible for their actions and not writing any randoms values to any
random files in /sys :)
> >4* Remember that you can always write a simple C tool relying on the
> >i2c-dev interface to do the job. The advantage of this approach is that
> >you can put big fat warnings and request user confirmation before any
> >action.
>
> This makes sense. Ladislav, would this work for you? I guess we'd still
> add code to the ds1337 driver to detect ds1339 in order to ensure that
> this tool could not modify register 0 of a ds1337 by accident?
Yes, that would definitely work for me and I'm fine with that in case
proposal above would be rejected.
Remove nowhere referenced ds1337_do_command function. Apply after ds1337
patches 1-3.
Signed-off-by: Ladislav Michl <[email protected]>
--- linux-omap/drivers/i2c/chips/ds1337.c.orig 2005-04-13 11:48:32.511011224 +0200
+++ linux-omap/drivers/i2c/chips/ds1337.c 2005-04-13 12:00:22.328102624 +0200
@@ -63,21 +63,6 @@ static struct i2c_driver ds1337_driver =
.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);
@@ -213,25 +198,6 @@ static int ds1337_command(struct i2c_cli
}
}
-/*
- * Public API for access to specific device. Useful for low-level
- * RTC access from kernel code.
- */
-int ds1337_do_command(int id, int cmd, void *arg)
-{
- struct list_head *walk;
- struct list_head *tmp;
- struct ds1337_data *data;
-
- list_for_each_safe(walk, tmp, &ds1337_clients) {
- data = list_entry(walk, struct ds1337_data, list);
- if (data->id == id)
- return ds1337_command(&data->client, cmd, arg);
- }
-
- return -ENODEV;
-}
-
static int ds1337_attach_adapter(struct i2c_adapter *adapter)
{
return i2c_detect(adapter, &addr_data, ds1337_detect);
@@ -244,7 +210,6 @@ static int ds1337_attach_adapter(struct
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 = "";
@@ -252,18 +217,10 @@ static int ds1337_detect(struct i2c_adap
I2C_FUNC_I2C))
goto exit;
- if (!(data = kmalloc(sizeof(struct ds1337_data), GFP_KERNEL))) {
- err = -ENOMEM;
- goto exit;
- }
- memset(data, 0, sizeof(struct ds1337_data));
- INIT_LIST_HEAD(&data->list);
+ new_client = kmalloc(sizeof(struct i2c_client), GFP_KERNEL);
+ if (!new_client)
+ return -ENOMEM;
- /* The common I2C client data is placed right before the
- * DS1337-specific data.
- */
- new_client = &data->client;
- i2c_set_clientdata(new_client, data);
new_client->addr = address;
new_client->adapter = adapter;
new_client->driver = &ds1337_driver;
@@ -334,14 +291,10 @@ static int ds1337_detect(struct i2c_adap
/* Initialize the DS1337 chip */
ds1337_init_client(new_client);
- /* Add client to local list */
- data->id = ds1337_id++;
- list_add(&data->list, &ds1337_clients);
-
return 0;
exit_free:
- kfree(data);
+ kfree(new_client);
exit:
return err;
}
@@ -360,7 +313,6 @@ static void ds1337_init_client(struct i2
static int ds1337_detach_client(struct i2c_client *client)
{
int err;
- struct ds1337_data *data = i2c_get_clientdata(client);
if ((err = i2c_detach_client(client))) {
dev_err(&client->dev, "Client deregistration failed, "
@@ -368,8 +320,7 @@ static int ds1337_detach_client(struct i
return err;
}
- list_del(&data->list);
- kfree(data);
+ kfree(client);
return 0;
}
Ladislav Michl wrote:
> On Tue, Apr 12, 2005 at 07:10:55PM +0100, James Chapman wrote:
> [snip]
>
>>It is used by the Radstone ppc7d platform, arch/ppc/radstone_ppc7d.c
>>but wasn't added until very recently (2.6.12-rc2 I think).
>>
>>To be honest, I meant to remove the 'id' thing before submitting the
>>driver. There's no need to support more than one of these devices.
>
> Patch bellow remove ds1337_do_command function and things needed by it.
> I think device should be identified by bus and address as Jean said.
> Please let me know if that fits your needs.
I think you misunderstood what I meant by "remove the 'id' thing"
(probably my fault). ds1337_do_command() is needed by ppc7d so don't
remove it. I meant remove the id parameter from the call and change the
ds1337 driver to support only one instance of the device.
> I'm assuming that you want to use drivers/char/genrtc.c to access ds1337
> from userspace, but in arch/ppc/platforms/radstone_ppc7d.c
> ppc_md.get_rtc_time used by genrtc via get_rtc_time in asm-ppc/rtc.h
> is set to NULL (same for set_rtc_time) and I didn't find where (if)
> ds1337 registers to ppc_md.get_rtc_time.
For ppc at least, it's the platform code that hooks up get_rtc_time().
Last time I looked in -mm, get_rtc_time() and set_rtc_time() were being
set up in ppc7d to use this driver. I won't be able to check until the
end of the week so please bear with me.
> Functions in asm-ppc/rtc.h also do magic with tm_mon and tm_year
> so this driver doesn't need to handle epoch separately and doesn't need
> to be aware that tm_mon starts from zero...
I don't understand. What code in ds1337 is unneeded?
> m68k, mips and parisc does the same in asm/rtc.h unlike arm, so I this
> driver probably won't work for me without some tweaks to arm code.
>
> [snip]
>
>>>Back to the issue, some random thoughts summarizing my opinion:
>>>
[snip]
>>>3* Having the driver write an arbitrary non-0 value to the register
>>>should not be done unless the system has been identified. I have no idea
>>>how your system can be identified (DMI?), but if it can't, then I'd
>>>better see the register ignored altogether.
>
> My board is OMAP (ARM core) based and there are ARM specific functions
> (if (machine_is_xxx()) do_something(); ), but it is not what you want to
> see in generic driver. It may be possible to use platform_data to pass
> information to driver, but I do not like this idea.
>
> So, if we use entry in sysfs, then only root can write it and root is
> allowed to do weird things. Device itself refuses any action until high
> four bits are 0xa. If that is still not enough I just found this patch
> http://groups-beta.google.com/group/fa.linux.kernel/msg/06e0368f86c8f824
> so you can use configfs to explicitly create "charge" entry. (
> * I'm considering that an overkill
> * I'm not sure if it can be easily done with configfs)
>
> I'd add config option (disabled by default) for "charge" entry, if you
> feel it is too dangerous. However I think that people should be a bit
> responsible for their actions and not writing any randoms values to any
> random files in /sys :)
>
>>>4* Remember that you can always write a simple C tool relying on the
>>>i2c-dev interface to do the job. The advantage of this approach is that
>>>you can put big fat warnings and request user confirmation before any
>>>action.
>>
>>This makes sense. Ladislav, would this work for you? I guess we'd still
>>add code to the ds1337 driver to detect ds1339 in order to ensure that
>>this tool could not modify register 0 of a ds1337 by accident?
>
>
> Yes, that would definitely work for me and I'm fine with that in case
> proposal above would be rejected.
Ok. Jean, what do you think? Do we really want a "charge" sysfs entry? I
don't have a strong opinion on this.
> Remove nowhere referenced ds1337_do_command function. Apply after ds1337
> patches 1-3.
Please don't apply this patch. I will modify the ds1337_do_command() API
to remove the "id" parameter and fixup ppc7d platform accordingly.
/james
On Wed, Apr 13, 2005 at 08:02:53PM +0100, James Chapman wrote:
> Ladislav Michl wrote:
[snip]
> >Patch bellow remove ds1337_do_command function and things needed by it.
> >I think device should be identified by bus and address as Jean said.
> >Please let me know if that fits your needs.
>
> I think you misunderstood what I meant by "remove the 'id' thing"
> (probably my fault). ds1337_do_command() is needed by ppc7d so don't
> remove it. I meant remove the id parameter from the call and change the
> ds1337 driver to support only one instance of the device.
No it is not your fault. I understood it perfectly but removed this
function because you should find your RTC on i2c bus then then do
something like:
if (rtc_client)
rtc_client->driver->command(rtc_client, cmd, data);
else
return -ENODEV;
and that should be done outside driver.
I do not think it is good idea to limit your driver to support only one
instance, because it is unecessary and there could exist some NUMA system
which could have one such RTC per node.
> >I'm assuming that you want to use drivers/char/genrtc.c to access ds1337
> >from userspace, but in arch/ppc/platforms/radstone_ppc7d.c
> >ppc_md.get_rtc_time used by genrtc via get_rtc_time in asm-ppc/rtc.h
> >is set to NULL (same for set_rtc_time) and I didn't find where (if)
> >ds1337 registers to ppc_md.get_rtc_time.
>
> For ppc at least, it's the platform code that hooks up get_rtc_time().
> Last time I looked in -mm, get_rtc_time() and set_rtc_time() were being
> set up in ppc7d to use this driver. I won't be able to check until the
> end of the week so please bear with me.
No problem, no patches will probably go in until SCM saga gets resolved :)
> >Functions in asm-ppc/rtc.h also do magic with tm_mon and tm_year
> >so this driver doesn't need to handle epoch separately and doesn't need
> >to be aware that tm_mon starts from zero...
>
> I don't understand. What code in ds1337 is unneeded?
It's not about unneeded code. Other RTC drivers returns tm_year in range
0..199 and tm_mon in range 0..11. Your driver does it different way,
because it hooks directly to [gs]et_rtc_time functions.
I'm not sure what is right solution here. Personaly I'd make time format
compatible with other RTC drivers and do such tweaks in arch code when
calling command function.
[snip]
> >Remove nowhere referenced ds1337_do_command function. Apply after ds1337
> >patches 1-3.
>
> Please don't apply this patch. I will modify the ds1337_do_command() API
> to remove the "id" parameter and fixup ppc7d platform accordingly.
See above...
Best regards,
ladis
On Fri, Apr 08, 2005 at 03:00:21PM +0200, Ladislav Michl wrote:
> On Thu, Apr 07, 2005 at 04:36:29PM -0700, Greg KH wrote:
> > Oops, you forgot to add a Signed-off-by: line for every patch, as per
> > Documentation/SubmittingPatches. Care to redo them?
>
> Here it is (I'm sorry about that).
>
> Use i2c_transfer to send message, so we get proper bus locking.
>
> Signed-off-by: Ladislav Michl <[email protected]>
Applied, thanks.
greg k-h
On Fri, Apr 08, 2005 at 03:02:16PM +0200, Ladislav Michl wrote:
> On Fri, Apr 08, 2005 at 10:51:00AM +0200, Jean Delvare wrote:
> > Yes, this one is OK with me too.
>
> Ok, here it is with signed off line.
>
> Use correct macros to convert between bdc and bin. See linux/bcd.h
>
> Signed-off-by: Ladislav Michl <[email protected]>
Applied, thanks.
greg k-h
On Fri, Apr 08, 2005 at 03:06:39PM +0200, Ladislav Michl wrote:
> On Fri, Apr 08, 2005 at 12:08:38PM +0200, Jean Delvare wrote:
> > Looks OK to me.
>
> Ok, I have few more fixes for this driver and will send them later
> when I find time to split them out into smaller chunks. Again, here is
> patch with signed off line.
>
>
> dev_{dbg,err} functions should print client's device name. data->id can
> be dropped from message, because device is determined by bus it hangs on
> (it has fixed address).
>
> Signed-off-by: Ladislav Michl <[email protected]>
Applied, thanks.
greg k-h
Greg,
thanks for applying changes. Here is next batch of incremental
patches for ds1337. These were created against 2.6.12-rc2 patched with
ds1337 [123]/4. Changes were discussed with James Chapman privately
and he sent me his Signed-off-by line for submition.
Make time format consistent with other RTC drivers.
Signed-off-by: Ladislav Michl <[email protected]>
Signed-off-by: James Chapman <[email protected]>
--- linux-omap/drivers/i2c/chips/ds1337.c.orig 2005-04-13 11:48:32.000000000 +0200
+++ linux-omap/drivers/i2c/chips/ds1337.c 2005-04-18 09:22:45.531771744 +0200
@@ -130,8 +130,8 @@ static int ds1337_get_datetime(struct i2
dt->tm_wday = BCD2BIN(buf[3]) - 1;
dt->tm_mday = BCD2BIN(buf[4]);
val = buf[5] & 0x7f;
- dt->tm_mon = BCD2BIN(val);
- dt->tm_year = 1900 + BCD2BIN(buf[6]);
+ dt->tm_mon = BCD2BIN(val) - 1;
+ dt->tm_year = BCD2BIN(buf[6]);
if (buf[5] & 0x80)
dt->tm_year += 100;
@@ -171,12 +171,11 @@ static int ds1337_set_datetime(struct i2
buf[3] = BIN2BCD(dt->tm_hour) | (1 << 6);
buf[4] = BIN2BCD(dt->tm_wday) + 1;
buf[5] = BIN2BCD(dt->tm_mday);
- buf[6] = BIN2BCD(dt->tm_mon);
- if (dt->tm_year >= 2000) {
- val = dt->tm_year - 2000;
+ buf[6] = BIN2BCD(dt->tm_mon) + 1;
+ val = dt->tm_year;
+ if (val >= 100) {
+ val -= 100;
buf[6] |= (1 << 7);
- } else {
- val = dt->tm_year - 1900;
}
buf[7] = BIN2BCD(val);
i2c_transfer returns number of sucessfully transfered messages. Change
error checking to accordingly. (ds1337_set_datetime never returned
sucess)
Signed-off-by: Ladislav Michl <[email protected]>
Signed-off-by: James Chapman <[email protected]>
--- linux-omap/drivers/i2c/chips/ds1337.c.orig 2005-04-20 20:08:46.580603672 +0200
+++ linux-omap/drivers/i2c/chips/ds1337.c 2005-04-20 20:34:31.622721568 +0200
@@ -122,7 +122,7 @@
__FUNCTION__, result, buf[0], buf[1], buf[2], buf[3],
buf[4], buf[5], buf[6]);
- if (result >= 0) {
+ if (result == 2) {
dt->tm_sec = BCD2BIN(buf[0]);
dt->tm_min = BCD2BIN(buf[1]);
val = buf[2] & 0x3f;
@@ -140,12 +140,12 @@
__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->dev, "error reading data! %d\n", result);
- result = -EIO;
+
+ return 0;
}
- return result;
+ dev_err(&client->dev, "error reading data! %d\n", result);
+ return -EIO;
}
static int ds1337_set_datetime(struct i2c_client *client, struct rtc_time *dt)
@@ -185,14 +185,11 @@
msg[0].buf = &buf[0];
result = i2c_transfer(client->adapter, msg, 1);
- if (result < 0) {
- dev_err(&client->dev, "error writing data! %d\n", result);
- result = -EIO;
- } else {
- result = 0;
- }
+ if (result == 1)
+ return 0;
- return result;
+ dev_err(&client->dev, "error writing data! %d\n", result);
+ return -EIO;
}
static int ds1337_command(struct i2c_client *client, unsigned int cmd,
Chip is searched by bus number rather than its own proprietary id.
Signed-off-by: Ladislav Michl <[email protected]>
Signed-off-by: James Chapman <[email protected]>
--- linux-omap/drivers/i2c/chips/ds1337.c.orig 2005-04-20 20:34:31.622721568 +0200
+++ linux-omap/drivers/i2c/chips/ds1337.c 2005-04-20 20:50:05.043819992 +0200
@@ -69,13 +69,11 @@
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)
@@ -213,7 +211,7 @@
* Public API for access to specific device. Useful for low-level
* RTC access from kernel code.
*/
-int ds1337_do_command(int id, int cmd, void *arg)
+int ds1337_do_command(int bus, int cmd, void *arg)
{
struct list_head *walk;
struct list_head *tmp;
@@ -221,7 +219,7 @@
list_for_each_safe(walk, tmp, &ds1337_clients) {
data = list_entry(walk, struct ds1337_data, list);
- if (data->id == id)
+ if (data->client.adapter->nr == bus)
return ds1337_command(&data->client, cmd, arg);
}
@@ -331,7 +329,6 @@
ds1337_init_client(new_client);
/* Add client to local list */
- data->id = ds1337_id++;
list_add(&data->list, &ds1337_clients);
return 0;
Hi Ladislav, all,
> Here is next batch of incremental patches for ds1337.
> (...)
> Make time format consistent with other RTC drivers.
Reviewed, looks OK to me.
Thanks,
--
Jean Delvare
Hi Ladislav, hi all,
> i2c_transfer returns number of sucessfully transfered messages. Change
> error checking to accordingly. (ds1337_set_datetime never returned
> sucess)
I think you mean ds1337_GET_datetime, not ds1337_SET_datetime, never
returned success?
Anyway I like the patch, it makes the driver treat the value returned by
i2c_transfer() properly and makes the code overall more consistent.
Thanks,
--
Jean Delvare
Hi Ladislav, hi all,
> Chip is searched by bus number rather than its own proprietary id.
Yes, I think it makes much more sense (especially since the proprietary
id was not known by anyone outside of the ds1337 driver).
I think I understand that ds1337_do_command() will be called from some
other kernel driver. Why isn't it exported then? I'd expect:
EXPORT_SYMBOL(ds1337_do_command);
next to the end of the ds1337 driver. Maybe it would also make sense to
have a ds1337.h header file declaring this function?
Additionally, I would welcome an additional patch documenting the fact
that the ds1337 driver will work fine with the Dallas DS1339 real-time
clock chip.
Thanks,
--
Jean Delvare
On Wed, May 04, 2005 at 12:07:11PM +0200, Jean Delvare wrote:
> > Chip is searched by bus number rather than its own proprietary id.
>
> Yes, I think it makes much more sense (especially since the proprietary
> id was not known by anyone outside of the ds1337 driver).
>
> I think I understand that ds1337_do_command() will be called from some
> other kernel driver. Why isn't it exported then? I'd expect:
> EXPORT_SYMBOL(ds1337_do_command);
RTC is hooked early in boot process. It should be available even sooner
than rootfs is mounted. Therefore RTC drivers are usualy compiled in
kernel. Anyway, exporting that function shouldn't hurt :)
> next to the end of the ds1337 driver. Maybe it would also make sense to
> have a ds1337.h header file declaring this function?
I'm not sure if adding yet another driver specific header is a good
idea. Perhaps we should consolidate I2C RTC drivers a bit more and
create common header for them?
> Additionally, I would welcome an additional patch documenting the fact
> that the ds1337 driver will work fine with the Dallas DS1339 real-time
> clock chip.
Document the fact that ds1337 driver works also with DS1339 real-time
clock chip.
Signed-off-by: Ladislav Michl <[email protected]>
--- linux-omap/drivers/i2c/chips/Kconfig.orig 2005-05-10 13:51:44.417092640 +0200
+++ linux-omap/drivers/i2c/chips/Kconfig 2005-05-10 13:52:33.148684312 +0200
@@ -366,12 +366,12 @@
depends on I2C
config SENSORS_DS1337
- tristate "Dallas Semiconductor DS1337 Real Time Clock"
+ tristate "Dallas Semiconductor DS1337 and DS1339 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.
+ DS1337 and DS1339 real-time clock chips.
This driver can also be built as a module. If so, the module
will be called ds1337.
--- linux-omap/drivers/i2c/chips/ds1337.c.orig 2005-05-10 13:50:25.003165392 +0200
+++ linux-omap/drivers/i2c/chips/ds1337.c 2005-05-10 13:50:57.199270840 +0200
@@ -10,7 +10,7 @@
* 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
+ * Driver for Dallas Semiconductor DS1337 and DS1339 real time clock chip
*/
#include <linux/config.h>
Hi Ladislav,
> > next to the end of the ds1337 driver. Maybe it would also make sense to
> > have a ds1337.h header file declaring this function?
>
> I'm not sure if adding yet another driver specific header is a good
> idea. Perhaps we should consolidate I2C RTC drivers a bit more and
> create common header for them?
Agreed, although I would go one step further. I see no reason why it
matters whether these are *I2C* RTC or not. I have no concrete proposal
but it really sounds like a common RTC interface would be needed here -
if it doesn't exist already. Can't say more unfortunately, as I never
had to use a RTC myself.
> Document the fact that ds1337 driver works also with DS1339 real-time
> clock chip.
Good work, thanks.
--
Jean Delvare
On Tue, May 10, 2005 at 02:08:04PM +0200, Ladislav Michl wrote:
> On Wed, May 04, 2005 at 12:07:11PM +0200, Jean Delvare wrote:
> > > Chip is searched by bus number rather than its own proprietary id.
> >
> > Yes, I think it makes much more sense (especially since the proprietary
> > id was not known by anyone outside of the ds1337 driver).
> >
> > I think I understand that ds1337_do_command() will be called from some
> > other kernel driver. Why isn't it exported then? I'd expect:
> > EXPORT_SYMBOL(ds1337_do_command);
>
> RTC is hooked early in boot process. It should be available even sooner
> than rootfs is mounted. Therefore RTC drivers are usualy compiled in
> kernel. Anyway, exporting that function shouldn't hurt :)
You don't have to set the kernel time during time_init(), and this
is especially true with I2C RTCs. The only thing that time_init()
has to do is to start the kernel tick going.
There is no reason why it can't be initialised during the normal
driver initialisation - which is how we do it on a number of ARM
platforms.
There are some ARM platforms where we can only get the time via
ntpdate, and this is also satisfactory.
So please don't get hung up on the x86 way of doing things. It's
both conceptually wrong and actually unnecessary.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core