From 907135aaa0cc120a347222c8f274ecc5ca0db641 Mon Sep 17 00:00:00 2001 From: David Brownell Date: Thu, 15 Nov 2007 19:24:01 +0100 Subject: i2c-dev: "how does it work" comments This adds some "how does this work" comments to the i2c-dev driver, plus separators between the three main components: - The parallel list of i2c_adapters ("i2c_dev_list"), each of which gets a "struct i2c_dev" and a /dev/i2c-X character special file. - An i2cdev_driver gets adapter add/remove notifications, which are used to maintain that list of adapters. - Special file operations, which let userspace talk either directly to the adapter (for i2c_msg operations) or through cached addressing info using an anonymous i2c_client (never registered anywhere). Plus there's the usual module load/unload record keeping. After making sense of this code, I think that the anonymous i2c_client is pretty shady. But since it's never registered, using this code with a system set up for "new style" I2C drivers is no more complicated than always using the I2C_SLAVE_FORCE ioctl (instead of I2C_SLAVE). Signed-off-by: David Brownell Signed-off-by: Jean Delvare --- drivers/i2c/i2c-dev.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c index 5a15e50..7360f9c 100644 --- a/drivers/i2c/i2c-dev.c +++ b/drivers/i2c/i2c-dev.c @@ -38,6 +38,15 @@ static struct i2c_driver i2cdev_driver; +/* + * An i2c_dev represents an i2c_adapter ... an I2C or SMBus master, not a + * slave (i2c_client) with which messages will be exchanged. It's coupled + * with a character special file which is accessed by user mode drivers. + * + * The list of i2c_dev structures is parallel to the i2c_adapter lists + * maintained by the driver model, and is updated using notifications + * delivered to the i2cdev_driver. + */ struct i2c_dev { struct list_head list; struct i2c_adapter *adap; @@ -103,6 +112,25 @@ static ssize_t show_adapter_name(struct device *dev, } static DEVICE_ATTR(name, S_IRUGO, show_adapter_name, NULL); +/* ------------------------------------------------------------------------- */ + +/* + * After opening an instance of this character special file, a file + * descriptor starts out associated only with an i2c_adapter (and bus). + * + * Using the I2C_RDWR ioctl(), you can then *immediately* issue i2c_msg + * traffic to any devices on the bus used by that adapter. That's because + * the i2c_msg vectors embed all the addressing information they need, and + * are submitted directly to an i2c_adapter. However, SMBus-only adapters + * don't support that interface. + * + * To use read()/write() system calls on that file descriptor, or to use + * SMBus interfaces (and work with SMBus-only hosts!), you must first issue + * an I2C_SLAVE (or I2C_SLAVE_FORCE) ioctl. That configures an anonymous + * (never registered) i2c_client so it holds the addressing information + * needed by those system calls and by this SMBus interface. + */ + static ssize_t i2cdev_read (struct file *file, char __user *buf, size_t count, loff_t *offset) { @@ -172,6 +200,16 @@ static int i2cdev_ioctl(struct inode *inode, struct file *file, switch ( cmd ) { case I2C_SLAVE: case I2C_SLAVE_FORCE: + /* NOTE: devices set up to work with "new style" drivers + * can't use I2C_SLAVE, even when the device node is not + * bound to a driver. Only I2C_SLAVE_FORCE will work. + * + * Setting the PEC flag here won't affect kernel drivers, + * which will be using the i2c_client node registered with + * the driver model core. Likewise, when that client has + * the PEC flag already set, the i2c-dev driver won't see + * (or use) this setting. + */ if ((arg > 0x3ff) || (((client->flags & I2C_M_TEN) == 0) && arg > 0x7f)) return -EINVAL; @@ -386,6 +424,13 @@ static int i2cdev_open(struct inode *inode, struct file *file) if (!adap) return -ENODEV; + /* This creates an anonymous i2c_client, which may later be + * pointed to some address using I2C_SLAVE or I2C_SLAVE_FORCE. + * + * This client is ** NEVER REGISTERED ** with the driver model + * or I2C core code!! It just holds private copies of addressing + * information and maybe a PEC flag. + */ client = kzalloc(sizeof(*client), GFP_KERNEL); if (!client) { i2c_put_adapter(adap); @@ -394,7 +439,6 @@ static int i2cdev_open(struct inode *inode, struct file *file) snprintf(client->name, I2C_NAME_SIZE, "i2c-dev %d", adap->nr); client->driver = &i2cdev_driver; - /* registered with adapter, passed as client to user */ client->adapter = adap; file->private_data = client; @@ -422,6 +466,14 @@ static const struct file_operations i2cdev_fops = { .release = i2cdev_release, }; +/* ------------------------------------------------------------------------- */ + +/* + * The legacy "i2cdev_driver" is used primarily to get notifications when + * I2C adapters are added or removed, so that each one gets an i2c_dev + * and is thus made available to userspace driver code. + */ + static struct class *i2c_dev_class; static int i2cdev_attach_adapter(struct i2c_adapter *adap) @@ -486,6 +538,12 @@ static struct i2c_driver i2cdev_driver = { .detach_client = i2cdev_detach_client, }; +/* ------------------------------------------------------------------------- */ + +/* + * module load/unload record keeping + */ + static int __init i2c_dev_init(void) { int res; -- cgit v1.1 From bd4217d8c6ef48425c8d6b28d2e089a83e01af04 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Thu, 15 Nov 2007 19:24:01 +0100 Subject: i2c-dev: Unbound new-style i2c clients aren't busy Let i2c-dev deal properly with new-style i2c clients. Instead of considering them always busy, it needs to check wether a driver is bound to them or not. This is still not completely correct, as the client could become busy later, but the same problem already existed before new-style clients were introduced. We'll want to fix it someday. Signed-off-by: Jean Delvare Acked-by: David Brownell --- drivers/i2c/i2c-dev.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c index 7360f9c..c21ae20 100644 --- a/drivers/i2c/i2c-dev.c +++ b/drivers/i2c/i2c-dev.c @@ -182,6 +182,29 @@ static ssize_t i2cdev_write (struct file *file, const char __user *buf, size_t c return ret; } +/* This address checking function differs from the one in i2c-core + in that it considers an address with a registered device, but no + bounded driver, as NOT busy. */ +static int i2cdev_check_addr(struct i2c_adapter *adapter, unsigned int addr) +{ + struct list_head *item; + struct i2c_client *client; + int res = 0; + + mutex_lock(&adapter->clist_lock); + list_for_each(item, &adapter->clients) { + client = list_entry(item, struct i2c_client, list); + if (client->addr == addr) { + if (client->driver) + res = -EBUSY; + break; + } + } + mutex_unlock(&adapter->clist_lock); + + return res; +} + static int i2cdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg) { @@ -213,8 +236,9 @@ static int i2cdev_ioctl(struct inode *inode, struct file *file, if ((arg > 0x3ff) || (((client->flags & I2C_M_TEN) == 0) && arg > 0x7f)) return -EINVAL; - if ((cmd == I2C_SLAVE) && i2c_check_addr(client->adapter,arg)) + if (cmd == I2C_SLAVE && i2cdev_check_addr(client->adapter, arg)) return -EBUSY; + /* REVISIT: address could become busy later */ client->addr = arg; return 0; case I2C_TENBIT: -- cgit v1.1 From 5e31c2bd3c865f8f474811340182795396b99696 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Thu, 15 Nov 2007 19:24:02 +0100 Subject: i2c: Make i2c_check_addr static i2c_check_addr is only used inside i2c-core now, so we can make it static and stop exporting it. Thanks to David Brownell for noticing. Signed-off-by: Jean Delvare --- drivers/i2c/i2c-core.c | 3 +-- include/linux/i2c.h | 5 ----- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 1a4e8dc..b5e13e4 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -673,7 +673,7 @@ static int __i2c_check_addr(struct i2c_adapter *adapter, unsigned int addr) return 0; } -int i2c_check_addr(struct i2c_adapter *adapter, int addr) +static int i2c_check_addr(struct i2c_adapter *adapter, int addr) { int rval; @@ -683,7 +683,6 @@ int i2c_check_addr(struct i2c_adapter *adapter, int addr) return rval; } -EXPORT_SYMBOL(i2c_check_addr); int i2c_attach_client(struct i2c_client *client) { diff --git a/include/linux/i2c.h b/include/linux/i2c.h index 8033e6b..a100c9f 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -400,11 +400,6 @@ extern int i2c_release_client(struct i2c_client *); extern void i2c_clients_command(struct i2c_adapter *adap, unsigned int cmd, void *arg); -/* returns -EBUSY if address has been taken, 0 if not. Note that the only - other place at which this is called is within i2c_attach_client; so - you can cheat by simply not registering. Not recommended, of course! */ -extern int i2c_check_addr (struct i2c_adapter *adapter, int addr); - /* Detect function. It iterates over all possible addresses itself. * It will only call found_proc if some client is connected at the * specific address (unless a 'force' matched); -- cgit v1.1 From ff23f3eabbaa4fc398e0ce109a8688db29d95d78 Mon Sep 17 00:00:00 2001 From: "Robert P. J. Day" Date: Thu, 15 Nov 2007 19:24:02 +0100 Subject: i2c-pasemi: Replace obsolete "driverfs" reference with "sysfs" Signed-off-by: Robert P. J. Day Signed-off-by: Jean Delvare --- drivers/i2c/busses/i2c-pasemi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-pasemi.c b/drivers/i2c/busses/i2c-pasemi.c index 58e3271..79c72dfa 100644 --- a/drivers/i2c/busses/i2c-pasemi.c +++ b/drivers/i2c/busses/i2c-pasemi.c @@ -364,7 +364,7 @@ static int __devinit pasemi_smb_probe(struct pci_dev *dev, smbus->adapter.algo = &smbus_algorithm; smbus->adapter.algo_data = smbus; - /* set up the driverfs linkage to our parent device */ + /* set up the sysfs linkage to our parent device */ smbus->adapter.dev.parent = &dev->dev; reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR | -- cgit v1.1 From be8a1f7cd4501c3b4b32543577a33aee6d2193ac Mon Sep 17 00:00:00 2001 From: Olof Johansson Date: Thu, 15 Nov 2007 19:24:02 +0100 Subject: i2c-pasemi: Fix NACK detection Turns out we don't actually check the status to see if there was a device out there to talk to, just if we had a timeout when doing so. Add the proper check, so we don't falsly think there are devices on the bus that are not there, etc. Signed-off-by: Olof Johansson Signed-off-by: Jean Delvare --- drivers/i2c/busses/i2c-pasemi.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/i2c/busses/i2c-pasemi.c b/drivers/i2c/busses/i2c-pasemi.c index 79c72dfa..ca18e0b 100644 --- a/drivers/i2c/busses/i2c-pasemi.c +++ b/drivers/i2c/busses/i2c-pasemi.c @@ -51,6 +51,7 @@ struct pasemi_smbus { #define MRXFIFO_DATA_M 0x000000ff #define SMSTA_XEN 0x08000000 +#define SMSTA_MTN 0x00200000 #define CTL_MRR 0x00000400 #define CTL_MTR 0x00000200 @@ -98,6 +99,10 @@ static unsigned int pasemi_smb_waitready(struct pasemi_smbus *smbus) status = reg_read(smbus, REG_SMSTA); } + /* Got NACK? */ + if (status & SMSTA_MTN) + return -ENXIO; + if (timeout < 0) { dev_warn(&smbus->dev->dev, "Timeout, status 0x%08x\n", status); reg_write(smbus, REG_SMSTA, status); -- cgit v1.1 From 0f2cbd38aa377e30df3b7602abed69464d1970aa Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Thu, 15 Nov 2007 19:24:03 +0100 Subject: i2c/eeprom: Hide Sony Vaio serial numbers The sysfs interface to DMI data takes care to not make the system serial number and UUID world-readable, presumably due to privacy concerns. For consistency, we should not let the eeprom driver export these same strings to the world on Sony Vaio laptops. Instead, only make them readable by root, as we already do for BIOS passwords. Signed-off-by: Jean Delvare --- drivers/i2c/chips/eeprom.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/i2c/chips/eeprom.c b/drivers/i2c/chips/eeprom.c index d3da1fb..ef8a754 100644 --- a/drivers/i2c/chips/eeprom.c +++ b/drivers/i2c/chips/eeprom.c @@ -128,13 +128,20 @@ static ssize_t eeprom_read(struct kobject *kobj, struct bin_attribute *bin_attr, for (slice = off >> 5; slice <= (off + count - 1) >> 5; slice++) eeprom_update_client(client, slice); - /* Hide Vaio security settings to regular users (16 first bytes) */ - if (data->nature == VAIO && off < 16 && !capable(CAP_SYS_ADMIN)) { - size_t in_row1 = 16 - off; - in_row1 = min(in_row1, count); - memset(buf, 0, in_row1); - if (count - in_row1 > 0) - memcpy(buf + in_row1, &data->data[16], count - in_row1); + /* Hide Vaio private settings to regular users: + - BIOS passwords: bytes 0x00 to 0x0f + - UUID: bytes 0x10 to 0x1f + - Serial number: 0xc0 to 0xdf */ + if (data->nature == VAIO && !capable(CAP_SYS_ADMIN)) { + int i; + + for (i = 0; i < count; i++) { + if ((off + i <= 0x1f) || + (off + i >= 0xc0 && off + i <= 0xdf)) + buf[i] = 0; + else + buf[i] = data->data[off + i]; + } } else { memcpy(buf, &data->data[off], count); } @@ -204,7 +211,7 @@ static int eeprom_detect(struct i2c_adapter *adapter, int address, int kind) && i2c_smbus_read_byte(new_client) == 'G' && i2c_smbus_read_byte(new_client) == '-') { dev_info(&new_client->dev, "Vaio EEPROM detected, " - "enabling password protection\n"); + "enabling privacy protection\n"); data->nature = VAIO; } } -- cgit v1.1 From 8b925a3dd8a4d7451092cb9aa11da727ba69e0f0 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Thu, 15 Nov 2007 19:24:03 +0100 Subject: i2c/eeprom: Recognize VGN as a valid Sony Vaio name prefix Recent (i.e. 2005 and later) Sony Vaio laptops have names beginning with VGN rather than PCG. Update the eeprom driver so that it recognizes these. Signed-off-by: Jean Delvare --- drivers/i2c/chips/eeprom.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/i2c/chips/eeprom.c b/drivers/i2c/chips/eeprom.c index ef8a754..1a7eeeb 100644 --- a/drivers/i2c/chips/eeprom.c +++ b/drivers/i2c/chips/eeprom.c @@ -204,12 +204,16 @@ static int eeprom_detect(struct i2c_adapter *adapter, int address, int kind) goto exit_kfree; /* Detect the Vaio nature of EEPROMs. - We use the "PCG-" prefix as the signature. */ + We use the "PCG-" or "VGN-" prefix as the signature. */ if (address == 0x57) { - 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) == '-') { + char name[4]; + + name[0] = i2c_smbus_read_byte_data(new_client, 0x80); + name[1] = i2c_smbus_read_byte(new_client); + name[2] = i2c_smbus_read_byte(new_client); + name[3] = i2c_smbus_read_byte(new_client); + + if (!memcmp(name, "PCG-", 4) || !memcmp(name, "VGN-", 4)) { dev_info(&new_client->dev, "Vaio EEPROM detected, " "enabling privacy protection\n"); data->nature = VAIO; -- cgit v1.1