Commit 4b2643d7 authored by Jean Delvare's avatar Jean Delvare Committed by Jean Delvare

i2c: Fix the i2c_smbus_read_i2c_block_data() prototype

Let the drivers specify how many bytes they want to read with
i2c_smbus_read_i2c_block_data(). So far, the block count was
hard-coded to I2C_SMBUS_BLOCK_MAX (32), which did not make much sense.
Many driver authors complained about this before, and I believe it's
about time to fix it. Right now, authors have to do technically stupid
things, such as individual byte reads or full-fledged I2C messaging,
to work around the problem. We do not want to encourage that.

I even found that some bus drivers (e.g. i2c-amd8111) already
implemented I2C block read the "right" way, that is, they didn't
follow the old, broken standard. The fact that it was never noticed
before just shows how little i2c_smbus_read_i2c_block_data() was used,
which isn't that surprising given how broken its prototype was so far.

There are some obvious compatiblity considerations:
* This changes the i2c_smbus_read_i2c_block_data() prototype. Users
  outside the kernel tree will notice at compilation time, and will
  have to update their code.
* User-space has access to i2c_smbus_xfer() directly using i2c-dev, so
  the changed expectations would affect tools such as i2cdump. In order
  to preserve binary compatibility, we give I2C_SMBUS_I2C_BLOCK_DATA
  a new numeric value, and define I2C_SMBUS_I2C_BLOCK_BROKEN with the
  old numeric value. When i2c-dev receives a transaction with the
  old value, it can convert it to the new format on the fly.
Signed-off-by: default avatarJean Delvare <khali@linux-fr.org>
parent ba7fbb72
...@@ -99,7 +99,7 @@ And then read the data ...@@ -99,7 +99,7 @@ And then read the data
or or
count = i2c_smbus_read_i2c_block_data(fd, 0x84, buffer); count = i2c_smbus_read_i2c_block_data(fd, 0x84, 16, buffer);
The block read should read 16 bytes. The block read should read 16 bytes.
0x84 is the block read command. 0x84 is the block read command.
......
...@@ -571,7 +571,7 @@ SMBus communication ...@@ -571,7 +571,7 @@ SMBus communication
u8 command, u8 length, u8 command, u8 length,
u8 *values); u8 *values);
extern s32 i2c_smbus_read_i2c_block_data(struct i2c_client * client, extern s32 i2c_smbus_read_i2c_block_data(struct i2c_client * client,
u8 command, u8 *values); u8 command, u8 length, u8 *values);
These ones were removed in Linux 2.6.10 because they had no users, but could These ones were removed in Linux 2.6.10 because they had no users, but could
be added back later if needed: be added back later if needed:
......
...@@ -121,8 +121,7 @@ static s32 i2c_powermac_smbus_xfer( struct i2c_adapter* adap, ...@@ -121,8 +121,7 @@ static s32 i2c_powermac_smbus_xfer( struct i2c_adapter* adap,
if (rc) if (rc)
goto bail; goto bail;
rc = pmac_i2c_xfer(bus, addrdir, 1, command, rc = pmac_i2c_xfer(bus, addrdir, 1, command,
read ? data->block : &data->block[1], &data->block[1], data->block[0]);
data->block[0]);
break; break;
default: default:
......
...@@ -235,7 +235,7 @@ static s32 vt596_access(struct i2c_adapter *adap, u16 addr, ...@@ -235,7 +235,7 @@ static s32 vt596_access(struct i2c_adapter *adap, u16 addr,
if (!(vt596_features & FEATURE_I2CBLOCK)) if (!(vt596_features & FEATURE_I2CBLOCK))
goto exit_unsupported; goto exit_unsupported;
if (read_write == I2C_SMBUS_READ) if (read_write == I2C_SMBUS_READ)
outb_p(I2C_SMBUS_BLOCK_MAX, SMBHSTDAT0); outb_p(data->block[0], SMBHSTDAT0);
/* Fall through */ /* Fall through */
case I2C_SMBUS_BLOCK_DATA: case I2C_SMBUS_BLOCK_DATA:
outb_p(command, SMBHSTCMD); outb_p(command, SMBHSTCMD);
......
...@@ -310,8 +310,6 @@ static s32 scx200_acb_smbus_xfer(struct i2c_adapter *adapter, ...@@ -310,8 +310,6 @@ static s32 scx200_acb_smbus_xfer(struct i2c_adapter *adapter,
break; break;
case I2C_SMBUS_I2C_BLOCK_DATA: case I2C_SMBUS_I2C_BLOCK_DATA:
if (rw == I2C_SMBUS_READ)
data->block[0] = I2C_SMBUS_BLOCK_MAX; /* For now */
len = data->block[0]; len = data->block[0];
if (len == 0 || len > I2C_SMBUS_BLOCK_MAX) if (len == 0 || len > I2C_SMBUS_BLOCK_MAX)
return -EINVAL; return -EINVAL;
......
...@@ -88,8 +88,10 @@ static void eeprom_update_client(struct i2c_client *client, u8 slice) ...@@ -88,8 +88,10 @@ static void eeprom_update_client(struct i2c_client *client, u8 slice)
dev_dbg(&client->dev, "Starting eeprom update, slice %u\n", slice); dev_dbg(&client->dev, "Starting eeprom update, slice %u\n", slice);
if (i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_READ_I2C_BLOCK)) { if (i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
for (i = slice << 5; i < (slice + 1) << 5; i += I2C_SMBUS_BLOCK_MAX) for (i = slice << 5; i < (slice + 1) << 5; i += 32)
if (i2c_smbus_read_i2c_block_data(client, i, data->data + i) != I2C_SMBUS_BLOCK_MAX) if (i2c_smbus_read_i2c_block_data(client, i,
32, data->data + i)
!= 32)
goto exit; goto exit;
} else { } else {
if (i2c_smbus_write_byte(client, slice << 5)) { if (i2c_smbus_write_byte(client, slice << 5)) {
......
...@@ -106,6 +106,7 @@ static void max6875_update_slice(struct i2c_client *client, int slice) ...@@ -106,6 +106,7 @@ static void max6875_update_slice(struct i2c_client *client, int slice)
I2C_FUNC_SMBUS_READ_I2C_BLOCK)) { I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
if (i2c_smbus_read_i2c_block_data(client, if (i2c_smbus_read_i2c_block_data(client,
MAX6875_CMD_BLK_READ, MAX6875_CMD_BLK_READ,
SLICE_SIZE,
buf) != SLICE_SIZE) { buf) != SLICE_SIZE) {
goto exit_up; goto exit_up;
} }
......
...@@ -1344,10 +1344,14 @@ s32 i2c_smbus_write_block_data(struct i2c_client *client, u8 command, ...@@ -1344,10 +1344,14 @@ s32 i2c_smbus_write_block_data(struct i2c_client *client, u8 command,
EXPORT_SYMBOL(i2c_smbus_write_block_data); EXPORT_SYMBOL(i2c_smbus_write_block_data);
/* Returns the number of read bytes */ /* Returns the number of read bytes */
s32 i2c_smbus_read_i2c_block_data(struct i2c_client *client, u8 command, u8 *values) s32 i2c_smbus_read_i2c_block_data(struct i2c_client *client, u8 command,
u8 length, u8 *values)
{ {
union i2c_smbus_data data; union i2c_smbus_data data;
if (length > I2C_SMBUS_BLOCK_MAX)
length = I2C_SMBUS_BLOCK_MAX;
data.block[0] = length;
if (i2c_smbus_xfer(client->adapter,client->addr,client->flags, if (i2c_smbus_xfer(client->adapter,client->addr,client->flags,
I2C_SMBUS_READ,command, I2C_SMBUS_READ,command,
I2C_SMBUS_I2C_BLOCK_DATA,&data)) I2C_SMBUS_I2C_BLOCK_DATA,&data))
...@@ -1468,7 +1472,7 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter * adapter, u16 addr, ...@@ -1468,7 +1472,7 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter * adapter, u16 addr,
break; break;
case I2C_SMBUS_I2C_BLOCK_DATA: case I2C_SMBUS_I2C_BLOCK_DATA:
if (read_write == I2C_SMBUS_READ) { if (read_write == I2C_SMBUS_READ) {
msg[1].len = I2C_SMBUS_BLOCK_MAX; msg[1].len = data->block[0];
} else { } else {
msg[0].len = data->block[0] + 1; msg[0].len = data->block[0] + 1;
if (msg[0].len > I2C_SMBUS_BLOCK_MAX + 1) { if (msg[0].len > I2C_SMBUS_BLOCK_MAX + 1) {
...@@ -1524,9 +1528,7 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter * adapter, u16 addr, ...@@ -1524,9 +1528,7 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter * adapter, u16 addr,
data->word = msgbuf1[0] | (msgbuf1[1] << 8); data->word = msgbuf1[0] | (msgbuf1[1] << 8);
break; break;
case I2C_SMBUS_I2C_BLOCK_DATA: case I2C_SMBUS_I2C_BLOCK_DATA:
/* fixed at 32 for now */ for (i = 0; i < data->block[0]; i++)
data->block[0] = I2C_SMBUS_BLOCK_MAX;
for (i = 0; i < I2C_SMBUS_BLOCK_MAX; i++)
data->block[i+1] = msgbuf1[i]; data->block[i+1] = msgbuf1[i];
break; break;
case I2C_SMBUS_BLOCK_DATA: case I2C_SMBUS_BLOCK_DATA:
......
...@@ -283,6 +283,7 @@ static int i2cdev_ioctl(struct inode *inode, struct file *file, ...@@ -283,6 +283,7 @@ static int i2cdev_ioctl(struct inode *inode, struct file *file,
(data_arg.size != I2C_SMBUS_WORD_DATA) && (data_arg.size != I2C_SMBUS_WORD_DATA) &&
(data_arg.size != I2C_SMBUS_PROC_CALL) && (data_arg.size != I2C_SMBUS_PROC_CALL) &&
(data_arg.size != I2C_SMBUS_BLOCK_DATA) && (data_arg.size != I2C_SMBUS_BLOCK_DATA) &&
(data_arg.size != I2C_SMBUS_I2C_BLOCK_BROKEN) &&
(data_arg.size != I2C_SMBUS_I2C_BLOCK_DATA) && (data_arg.size != I2C_SMBUS_I2C_BLOCK_DATA) &&
(data_arg.size != I2C_SMBUS_BLOCK_PROC_CALL)) { (data_arg.size != I2C_SMBUS_BLOCK_PROC_CALL)) {
dev_dbg(&client->adapter->dev, dev_dbg(&client->adapter->dev,
...@@ -329,10 +330,18 @@ static int i2cdev_ioctl(struct inode *inode, struct file *file, ...@@ -329,10 +330,18 @@ static int i2cdev_ioctl(struct inode *inode, struct file *file,
if ((data_arg.size == I2C_SMBUS_PROC_CALL) || if ((data_arg.size == I2C_SMBUS_PROC_CALL) ||
(data_arg.size == I2C_SMBUS_BLOCK_PROC_CALL) || (data_arg.size == I2C_SMBUS_BLOCK_PROC_CALL) ||
(data_arg.size == I2C_SMBUS_I2C_BLOCK_DATA) ||
(data_arg.read_write == I2C_SMBUS_WRITE)) { (data_arg.read_write == I2C_SMBUS_WRITE)) {
if (copy_from_user(&temp, data_arg.data, datasize)) if (copy_from_user(&temp, data_arg.data, datasize))
return -EFAULT; return -EFAULT;
} }
if (data_arg.size == I2C_SMBUS_I2C_BLOCK_BROKEN) {
/* Convert old I2C block commands to the new
convention. This preserves binary compatibility. */
data_arg.size = I2C_SMBUS_I2C_BLOCK_DATA;
if (data_arg.read_write == I2C_SMBUS_READ)
temp.block[0] = I2C_SMBUS_BLOCK_MAX;
}
res = i2c_smbus_xfer(client->adapter,client->addr,client->flags, res = i2c_smbus_xfer(client->adapter,client->addr,client->flags,
data_arg.read_write, data_arg.read_write,
data_arg.command,data_arg.size,&temp); data_arg.command,data_arg.size,&temp);
......
...@@ -67,26 +67,6 @@ static struct i2c_driver wf_sat_driver = { ...@@ -67,26 +67,6 @@ static struct i2c_driver wf_sat_driver = {
.detach_client = wf_sat_detach, .detach_client = wf_sat_detach,
}; };
/*
* XXX i2c_smbus_read_i2c_block_data doesn't pass the requested
* length down to the low-level driver, so we use this, which
* works well enough with the SMU i2c driver code...
*/
static int sat_read_block(struct i2c_client *client, u8 command,
u8 *values, int len)
{
union i2c_smbus_data data;
int err;
data.block[0] = len;
err = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
I2C_SMBUS_READ, command, I2C_SMBUS_I2C_BLOCK_DATA,
&data);
if (!err)
memcpy(values, data.block, len);
return err;
}
struct smu_sdbp_header *smu_sat_get_sdb_partition(unsigned int sat_id, int id, struct smu_sdbp_header *smu_sat_get_sdb_partition(unsigned int sat_id, int id,
unsigned int *size) unsigned int *size)
{ {
...@@ -124,8 +104,8 @@ struct smu_sdbp_header *smu_sat_get_sdb_partition(unsigned int sat_id, int id, ...@@ -124,8 +104,8 @@ struct smu_sdbp_header *smu_sat_get_sdb_partition(unsigned int sat_id, int id,
return NULL; return NULL;
for (i = 0; i < len; i += 4) { for (i = 0; i < len; i += 4) {
err = sat_read_block(&sat->i2c, 0xa, data, 4); err = i2c_smbus_read_i2c_block_data(&sat->i2c, 0xa, 4, data);
if (err) { if (err < 0) {
printk(KERN_ERR "smu_sat_get_sdb_part rd err %d\n", printk(KERN_ERR "smu_sat_get_sdb_part rd err %d\n",
err); err);
goto fail; goto fail;
...@@ -157,8 +137,8 @@ static int wf_sat_read_cache(struct wf_sat *sat) ...@@ -157,8 +137,8 @@ static int wf_sat_read_cache(struct wf_sat *sat)
{ {
int err; int err;
err = sat_read_block(&sat->i2c, 0x3f, sat->cache, 16); err = i2c_smbus_read_i2c_block_data(&sat->i2c, 0x3f, 16, sat->cache);
if (err) if (err < 0)
return err; return err;
sat->last_read = jiffies; sat->last_read = jiffies;
#ifdef LOTSA_DEBUG #ifdef LOTSA_DEBUG
......
...@@ -90,7 +90,7 @@ extern s32 i2c_smbus_write_block_data(struct i2c_client * client, ...@@ -90,7 +90,7 @@ extern s32 i2c_smbus_write_block_data(struct i2c_client * client,
const u8 *values); const u8 *values);
/* Returns the number of read bytes */ /* Returns the number of read bytes */
extern s32 i2c_smbus_read_i2c_block_data(struct i2c_client * client, extern s32 i2c_smbus_read_i2c_block_data(struct i2c_client * client,
u8 command, u8 *values); u8 command, u8 length, u8 *values);
extern s32 i2c_smbus_write_i2c_block_data(struct i2c_client * client, extern s32 i2c_smbus_write_i2c_block_data(struct i2c_client * client,
u8 command, u8 length, u8 command, u8 length,
const u8 *values); const u8 *values);
...@@ -524,8 +524,9 @@ union i2c_smbus_data { ...@@ -524,8 +524,9 @@ union i2c_smbus_data {
#define I2C_SMBUS_WORD_DATA 3 #define I2C_SMBUS_WORD_DATA 3
#define I2C_SMBUS_PROC_CALL 4 #define I2C_SMBUS_PROC_CALL 4
#define I2C_SMBUS_BLOCK_DATA 5 #define I2C_SMBUS_BLOCK_DATA 5
#define I2C_SMBUS_I2C_BLOCK_DATA 6 #define I2C_SMBUS_I2C_BLOCK_BROKEN 6
#define I2C_SMBUS_BLOCK_PROC_CALL 7 /* SMBus 2.0 */ #define I2C_SMBUS_BLOCK_PROC_CALL 7 /* SMBus 2.0 */
#define I2C_SMBUS_I2C_BLOCK_DATA 8
/* ----- commands for the ioctl like i2c_command call: /* ----- commands for the ioctl like i2c_command call:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment