On Sat, 7 Jun 2008 09:35:15 +0200, Jean Delvare wrote:
> None of these solutions make me totally happy, but I guess that the
> most reasonable one is the second one (have i2c-core allocate
> i2c_client and pass it to detect callbacks.) Maybe if we document
> properly that the only thing one is allowed to do with this i2c_client
> is call i2c_smbus_read_byte_data() and i2c_smbus_read_word_data(), it's
> acceptable. I welcome comments and ideas on this problem.
Here's what it would look like (patch applies on top of the last series
I posted, for the moment.) That's probably better than my original
proposal. The diffstat clearly shows how it simplifies the detect
callback of each chip driver.
---
drivers/hwmon/f75375s.c | 24 +++++-----------------
drivers/hwmon/lm75.c | 35 ++++++++------------------------
drivers/hwmon/lm90.c | 34 +++++++++----------------------
drivers/i2c/i2c-core.c | 50 +++++++++++++++++++++++++++++------------------
include/linux/i2c.h | 3 --
5 files changed, 57 insertions(+), 89 deletions(-)
--- linux-2.6.26-rc5.orig/drivers/hwmon/f75375s.c 2008-06-07
10:52:59.000000000 +0200
+++ linux-2.6.26-rc5/drivers/hwmon/f75375s.c 2008-06-07 11:14:47.000000000
+0200
@@ -114,7 +114,7 @@ struct f75375_data {
s8 temp_max_hyst[2];
};
-static int f75375_detect(struct i2c_adapter *adapter, int address, int kind,
+static int f75375_detect(struct i2c_client *client, int kind,
struct i2c_board_info *info);
static int f75375_probe(struct i2c_client *client,
const struct i2c_device_id *id);
@@ -679,21 +679,13 @@ static int f75375_remove(struct i2c_clie
}
/* This function is called by i2c_probe */
-static int f75375_detect(struct i2c_adapter *adapter, int address, int kind,
+static int f75375_detect(struct i2c_client *client, int kind,
struct i2c_board_info *info)
{
- struct i2c_client *client;
+ struct i2c_adapter *adapter = client->adapter;
u8 version = 0;
- int err = -ENODEV;
const char *name = "";
- if (!(client = kzalloc(sizeof(*client), GFP_KERNEL))) {
- err = -ENOMEM;
- goto exit;
- }
- client->addr = address;
- client->adapter = adapter;
-
if (kind < 0) {
u16 vendid = f75375_read16(client, F75375_REG_VENDOR);
u16 chipid = f75375_read16(client, F75375_CHIP_ID);
@@ -706,10 +698,9 @@ static int f75375_detect(struct i2c_adap
dev_err(&adapter->dev,
"failed,%02X,%02X,%02X\n",
chipid, version, vendid);
- goto exit_free;
+ return -ENODEV;
}
}
- err = 0; /* detection OK */
if (kind == f75375) {
name = "f75375";
@@ -718,12 +709,9 @@ static int f75375_detect(struct i2c_adap
}
dev_info(&adapter->dev, "found %s version: %02X\n", name, version);
strlcpy(info->type, name, I2C_NAME_SIZE);
- info->addr = address;
+ info->addr = client->addr;
-exit_free:
- kfree(client);
-exit:
- return err;
+ return 0;
}
static int __init sensors_f75375_init(void)
--- linux-2.6.26-rc5.orig/drivers/hwmon/lm75.c 2008-06-06 20:25:48.000000000
+0200
+++ linux-2.6.26-rc5/drivers/hwmon/lm75.c 2008-06-07 11:20:37.000000000
+0200
@@ -217,28 +217,15 @@ static int lm75_remove(struct i2c_client
return 0;
}
-static int lm75_detect(struct i2c_adapter *adapter, int address, int kind,
+static int lm75_detect(struct i2c_client *new_client, int kind,
struct i2c_board_info *info)
{
+ struct i2c_adapter *adapter = new_client->adapter;
int i;
- struct i2c_client *new_client;
- int err = -ENODEV;
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
I2C_FUNC_SMBUS_WORD_DATA))
- goto exit;
-
- /* OK. For now, we presume we have a valid address. We create the
- client structure, even though there may be no sensor present.
- But it allows us to use i2c_smbus_read_*_data() calls. */
- new_client = kzalloc(sizeof *new_client, GFP_KERNEL);
- if (!new_client) {
- err = -ENOMEM;
- goto exit;
- }
-
- new_client->addr = address;
- new_client->adapter = adapter;
+ return -ENODEV;
/* Now, we do the remaining detection. There is no identification-
dedicated register so we have to rely on several tricks:
@@ -258,37 +245,33 @@ static int lm75_detect(struct i2c_adapte
|| i2c_smbus_read_word_data(new_client, 5) != hyst
|| i2c_smbus_read_word_data(new_client, 6) != hyst
|| i2c_smbus_read_word_data(new_client, 7) != hyst)
- goto exit_free;
+ return -ENODEV;
os = i2c_smbus_read_word_data(new_client, 3);
if (i2c_smbus_read_word_data(new_client, 4) != os
|| i2c_smbus_read_word_data(new_client, 5) != os
|| i2c_smbus_read_word_data(new_client, 6) != os
|| i2c_smbus_read_word_data(new_client, 7) != os)
- goto exit_free;
+ return -ENODEV;
/* Unused bits */
if (conf & 0xe0)
- goto exit_free;
+ return -ENODEV;
/* Addresses cycling */
for (i = 8; i < 0xff; i += 8)
if (i2c_smbus_read_byte_data(new_client, i + 1) != conf
|| i2c_smbus_read_word_data(new_client, i + 2) != hyst
|| i2c_smbus_read_word_data(new_client, i + 3) != os)
- goto exit_free;
+ return -ENODEV;
}
- err = 0; /* detection OK */
/* NOTE: we treat "force=..." and "force_lm75=..." the same.
* Only new-style driver binding distinguishes chip types.
*/
strlcpy(info->type, "lm75", I2C_NAME_SIZE);
- info->addr = address;
+ info->addr = new_client->addr;
-exit_free:
- kfree(new_client);
-exit:
- return err;
+ return 0;
}
static const struct i2c_device_id lm75_ids[] = {
--- linux-2.6.26-rc5.orig/drivers/hwmon/lm90.c 2008-06-06 20:25:48.000000000
+0200
+++ linux-2.6.26-rc5/drivers/hwmon/lm90.c 2008-06-07 10:33:03.000000000
+0200
@@ -187,8 +187,8 @@ I2C_CLIENT_INSMOD_7(lm90, adm1032, lm99,
* Functions declaration
*/
-static int lm90_detect(struct i2c_adapter *adapter, int address,
- int kind, struct i2c_board_info *info);
+static int lm90_detect(struct i2c_client *new_client, int kind,
+ struct i2c_board_info *info);
static int lm90_probe(struct i2c_client *client,
const struct i2c_device_id *id);
static void lm90_init_client(struct i2c_client *client);
@@ -494,25 +494,15 @@ static int lm90_read_reg(struct i2c_clie
}
/* Returns -ENODEV if no supported device is detected */
-static int lm90_detect(struct i2c_adapter *adapter, int address, int kind,
+static int lm90_detect(struct i2c_client *new_client, int kind,
struct i2c_board_info *info)
{
- struct i2c_client *new_client;
- int err = -ENODEV;
+ struct i2c_adapter *adapter = new_client->adapter;
+ int address = new_client->addr;
const char *name = "";
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
- goto exit;
-
- new_client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL);
- if (!new_client) {
- err = -ENOMEM;
- goto exit;
- }
-
- /* Enough to use smbus calls */
- new_client->addr = address;
- new_client->adapter = adapter;
+ return -ENODEV;
/*
* Now we do the remaining detection. A negative kind means that
@@ -540,7 +530,7 @@ static int lm90_detect(struct i2c_adapte
LM90_REG_R_CONFIG1)) < 0
|| (reg_convrate = i2c_smbus_read_byte_data(new_client,
LM90_REG_R_CONVRATE)) < 0)
- goto exit_free;
+ return -ENODEV;
if ((address == 0x4C || address == 0x4D)
&& man_id == 0x01) { /* National Semiconductor */
@@ -548,7 +538,7 @@ static int lm90_detect(struct i2c_adapte
if ((reg_config2 = i2c_smbus_read_byte_data(new_client,
LM90_REG_R_CONFIG2)) < 0)
- goto exit_free;
+ return -ENODEV;
if ((reg_config1 & 0x2A) == 0x00
&& (reg_config2 & 0xF8) == 0x00
@@ -612,10 +602,9 @@ static int lm90_detect(struct i2c_adapte
dev_info(&adapter->dev,
"Unsupported chip (man_id=0x%02X, "
"chip_id=0x%02X).\n", man_id, chip_id);
- goto exit_free;
+ return -ENODEV;
}
}
- err = 0; /* detection OK */
/* Fill the i2c board info */
info->addr = address;
@@ -640,10 +629,7 @@ static int lm90_detect(struct i2c_adapte
}
strlcpy(info->type, name, I2C_NAME_SIZE);
-exit_free:
- kfree(new_client);
-exit:
- return err;
+ return 0;
}
static int lm90_probe(struct i2c_client *new_client,
--- linux-2.6.26-rc5.orig/drivers/i2c/i2c-core.c 2008-06-06
20:25:48.000000000 +0200
+++ linux-2.6.26-rc5/drivers/i2c/i2c-core.c 2008-06-07 10:48:06.000000000
+0200
@@ -1226,11 +1226,12 @@ int i2c_probe(struct i2c_adapter *adapte
EXPORT_SYMBOL(i2c_probe);
/* Separate detection function for new-style drivers */
-static int i2c_detect_address(struct i2c_adapter *adapter,
- int addr, int kind, struct i2c_driver *driver)
+static int i2c_detect_address(struct i2c_client *temp_client, int kind,
+ struct i2c_driver *driver)
{
struct i2c_board_info info;
- struct i2c_client *client;
+ struct i2c_adapter *adapter = temp_client->adapter;
+ int addr = temp_client->addr;
int err;
/* Make sure the address is valid */
@@ -1258,7 +1259,7 @@ static int i2c_detect_address(struct i2c
/* Finally call the custom detection function */
memset(&info, 0, sizeof(struct i2c_board_info));
- err = driver->detect(adapter, addr, kind, &info);
+ err = driver->detect(temp_client, kind, &info);
if (err) {
/* -ENODEV can be returned if there is a chip at the given
address
but it isn't supported by this chip driver. We catch it here
as
@@ -1271,6 +1272,8 @@ static int i2c_detect_address(struct i2c
dev_err(&adapter->dev, "Detection function returned "
"inconsistent data for 0x%x\n", addr);
} else {
+ struct i2c_client *client;
+
/* Detection succeeded, instantiate the device */
dev_dbg(&adapter->dev, "Creating %s at 0x%x\n",
info.type, info.addr);
@@ -1283,13 +1286,20 @@ static int i2c_detect_address(struct i2c
static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver)
{
const struct i2c_client_address_data *address_data;
- int i, err;
+ struct i2c_client *temp_client;
+ int i, err = 0;
int adap_id = i2c_adapter_id(adapter);
if (!driver->detect)
return 0;
address_data = driver->address_data;
+ /* Set up a temporary client to help detect callback */
+ temp_client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL);
+ if (!temp_client)
+ return -ENOMEM;
+ temp_client->adapter = adapter;
+
/* Force entries are done first, and are not affected by ignore
entries */
if (address_data->forces) {
@@ -1306,11 +1316,11 @@ static int i2c_detect(struct i2c_adapter
"addr 0x%02x, kind %d\n",
adap_id, forces[kind][i + 1],
kind);
- err = i2c_detect_address(adapter,
- forces[kind][i + 1],
+ temp_client->addr = forces[kind][i + 1];
+ err = i2c_detect_address(temp_client,
kind, driver);
if (err)
- return err;
+ goto exit_free;
}
}
}
@@ -1320,11 +1330,12 @@ static int i2c_detect(struct i2c_adapter
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_QUICK)) {
if (address_data->probe[0] == I2C_CLIENT_END
&& address_data->normal_i2c[0] == I2C_CLIENT_END)
- return 0;
+ goto exit_free;
dev_warn(&adapter->dev, "SMBus Quick command not supported, "
"can't probe for chips\n");
- return -EOPNOTSUPP;
+ err = -EOPNOTSUPP;
+ goto exit_free;
}
/* Probe entries are done second, and are not affected by ignore
@@ -1335,17 +1346,16 @@ static int i2c_detect(struct i2c_adapter
dev_dbg(&adapter->dev, "found probe parameter for "
"adapter %d, addr 0x%02x\n", adap_id,
address_data->probe[i + 1]);
- err = i2c_detect_address(adapter,
- address_data->probe[i + 1],
- -1, driver);
+ temp_client->addr = address_data->probe[i + 1];
+ err = i2c_detect_address(temp_client, -1, driver);
if (err)
- return err;
+ goto exit_free;
}
}
/* Stop here if the classes do not match */
if (!(adapter->class & driver->class))
- return 0;
+ goto exit_free;
/* Normal entries are done last, unless shadowed by an ignore entry */
for (i = 0; address_data->normal_i2c[i] != I2C_CLIENT_END; i += 1) {
@@ -1372,13 +1382,15 @@ static int i2c_detect(struct i2c_adapter
dev_dbg(&adapter->dev, "found normal entry for adapter %d, "
"addr 0x%02x\n", adap_id,
address_data->normal_i2c[i]);
- err = i2c_detect_address(adapter, address_data->normal_i2c[i],
- -1, driver);
+ temp_client->addr = address_data->normal_i2c[i];
+ err = i2c_detect_address(temp_client, -1, driver);
if (err)
- return err;
+ goto exit_free;
}
- return 0;
+ exit_free:
+ kfree(temp_client);
+ return err;
}
struct i2c_client *
--- linux-2.6.26-rc5.orig/include/linux/i2c.h 2008-06-06 20:25:48.000000000
+0200
+++ linux-2.6.26-rc5/include/linux/i2c.h 2008-06-07 09:48:04.000000000
+0200
@@ -148,8 +148,7 @@ struct i2c_driver {
const struct i2c_device_id *id_table;
/* Device detection callback for automatic device creation */
- int (*detect)(struct i2c_adapter *, int address, int kind,
- struct i2c_board_info *);
+ int (*detect)(struct i2c_client *, int kind, struct i2c_board_info *);
const struct i2c_client_address_data *address_data;
struct list_head clients;
};
--
Jean Delvare
_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c
