From 63e4e80218803ff506b3ea4f2349b17916a35730 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Thu, 3 Jun 2010 11:33:51 +0200 Subject: i2c: Share the I2C device presence detection code Use the same I2C device presence detection code for legacy and new device detection functions. This is more consistent and makes the code smaller. Signed-off-by: Jean Delvare Cc: Matthieu Castet --- drivers/i2c/i2c-core.c | 80 +++++++++++++++++++++++++------------------------- 1 file changed, 40 insertions(+), 40 deletions(-) (limited to 'drivers/i2c') diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index e0f833cca3f1..f744f73e3ff5 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -1277,6 +1277,41 @@ EXPORT_SYMBOL(i2c_master_recv); * ---------------------------------------------------- */ +/* + * Legacy default probe function, mostly relevant for SMBus. The default + * probe method is a quick write, but it is known to corrupt the 24RF08 + * EEPROMs due to a state machine bug, and could also irreversibly + * write-protect some EEPROMs, so for address ranges 0x30-0x37 and 0x50-0x5f, + * we use a short byte read instead. Also, some bus drivers don't implement + * quick write, so we fallback to a byte read in that case too. + * On x86, there is another special case for FSC hardware monitoring chips, + * which want regular byte reads (address 0x73.) Fortunately, these are the + * only known chips using this I2C address on PC hardware. + * Returns 1 if probe succeeded, 0 if not. + */ +static int i2c_default_probe(struct i2c_adapter *adap, unsigned short addr) +{ + int err; + union i2c_smbus_data dummy; + +#ifdef CONFIG_X86 + if (addr == 0x73 && (adap->class & I2C_CLASS_HWMON) + && i2c_check_functionality(adap, I2C_FUNC_SMBUS_READ_BYTE_DATA)) + err = i2c_smbus_xfer(adap, addr, 0, I2C_SMBUS_READ, 0, + I2C_SMBUS_BYTE_DATA, &dummy); + else +#endif + if ((addr & ~0x07) == 0x30 || (addr & ~0x0f) == 0x50 + || !i2c_check_functionality(adap, I2C_FUNC_SMBUS_QUICK)) + err = i2c_smbus_xfer(adap, addr, 0, I2C_SMBUS_READ, 0, + I2C_SMBUS_BYTE, &dummy); + else + err = i2c_smbus_xfer(adap, addr, 0, I2C_SMBUS_WRITE, 0, + I2C_SMBUS_QUICK, NULL); + + return err >= 0; +} + static int i2c_detect_address(struct i2c_client *temp_client, struct i2c_driver *driver) { @@ -1297,23 +1332,8 @@ static int i2c_detect_address(struct i2c_client *temp_client, return 0; /* Make sure there is something at this address */ - if (addr == 0x73 && (adapter->class & I2C_CLASS_HWMON)) { - /* Special probe for FSC hwmon chips */ - union i2c_smbus_data dummy; - - if (i2c_smbus_xfer(adapter, addr, 0, I2C_SMBUS_READ, 0, - I2C_SMBUS_BYTE_DATA, &dummy) < 0) - return 0; - } else { - if (i2c_smbus_xfer(adapter, addr, 0, I2C_SMBUS_WRITE, 0, - I2C_SMBUS_QUICK, NULL) < 0) - return 0; - - /* Prevent 24RF08 corruption */ - if ((addr & ~0x0f) == 0x50) - i2c_smbus_xfer(adapter, addr, 0, I2C_SMBUS_WRITE, 0, - I2C_SMBUS_QUICK, NULL); - } + if (!i2c_default_probe(adapter, addr)) + return 0; /* Finally call the custom detection function */ memset(&info, 0, sizeof(struct i2c_board_info)); @@ -1420,29 +1440,9 @@ i2c_new_probed_device(struct i2c_adapter *adap, continue; } - /* Test address responsiveness - The default probe method is a quick write, but it is known - to corrupt the 24RF08 EEPROMs due to a state machine bug, - and could also irreversibly write-protect some EEPROMs, so - for address ranges 0x30-0x37 and 0x50-0x5f, we use a byte - read instead. Also, some bus drivers don't implement - quick write, so we fallback to a byte read it that case - too. */ - if ((addr_list[i] & ~0x07) == 0x30 - || (addr_list[i] & ~0x0f) == 0x50 - || !i2c_check_functionality(adap, I2C_FUNC_SMBUS_QUICK)) { - union i2c_smbus_data data; - - if (i2c_smbus_xfer(adap, addr_list[i], 0, - I2C_SMBUS_READ, 0, - I2C_SMBUS_BYTE, &data) >= 0) - break; - } else { - if (i2c_smbus_xfer(adap, addr_list[i], 0, - I2C_SMBUS_WRITE, 0, - I2C_SMBUS_QUICK, NULL) >= 0) - break; - } + /* Test address responsiveness */ + if (i2c_default_probe(adap, addr_list[i])) + break; } if (addr_list[i] == I2C_CLIENT_END) { -- cgit v1.2.2 From 3a89db5f30576654bf1b0036af9b50ed5ab1b6c5 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Thu, 3 Jun 2010 11:33:52 +0200 Subject: i2c: Check for address validity on client registration Do basic address validity checks when a client is being registered. We already had checks in place for devices which are being detected, but not for devices which are simply instantiated. This is a very basic check. We don't want to do strict checking here because some devices are known to infringe the I2C address constraints (e.g. IR receivers at 7-bit address 0x7a while this value is supposedly reserved for 10-bit addresses.) So we assume the caller knows what it is doing. Signed-off-by: Jean Delvare Cc: Matthieu Castet --- drivers/i2c/i2c-core.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) (limited to 'drivers/i2c') diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index f744f73e3ff5..96f01331544d 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -371,6 +371,22 @@ struct i2c_client *i2c_verify_client(struct device *dev) EXPORT_SYMBOL(i2c_verify_client); +/* This is a permissive address validity check, I2C address map constraints + * are purposedly not enforced, except for the general call address. */ +static int i2c_check_client_addr_validity(const struct i2c_client *client) +{ + if (client->flags & I2C_CLIENT_TEN) { + /* 10-bit address, all values are valid */ + if (client->addr > 0x3ff) + return -EINVAL; + } else { + /* 7-bit address, reject the general call address */ + if (client->addr == 0x00 || client->addr > 0x7f) + return -EINVAL; + } + return 0; +} + /** * i2c_new_device - instantiate an i2c device * @adap: the adapter managing the device @@ -410,6 +426,14 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info) strlcpy(client->name, info->type, sizeof(client->name)); + /* Check for address validity */ + status = i2c_check_client_addr_validity(client); + if (status) { + dev_err(&adap->dev, "Invalid %d-bit I2C address 0x%02hx\n", + client->flags & I2C_CLIENT_TEN ? 10 : 7, client->addr); + goto out_err_silent; + } + /* Check for address business */ status = i2c_check_addr(adap, client->addr); if (status) @@ -436,6 +460,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info) out_err: dev_err(&adap->dev, "Failed to register i2c client %s at 0x%02x " "(%d)\n", client->name, client->addr, status); +out_err_silent: kfree(client); return NULL; } @@ -561,15 +586,9 @@ i2c_sysfs_new_device(struct device *dev, struct device_attribute *attr, return -EINVAL; } - if (info.addr < 0x03 || info.addr > 0x77) { - dev_err(dev, "%s: Invalid I2C address 0x%hx\n", "new_device", - info.addr); - return -EINVAL; - } - client = i2c_new_device(adap, &info); if (!client) - return -EEXIST; + return -EINVAL; /* Keep track of the added device */ i2c_lock_adapter(adap); -- cgit v1.2.2 From 656b8761ab21715eb1a35bb078dfd05e901be4ec Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Thu, 3 Jun 2010 11:33:53 +0200 Subject: i2c: Document reserved I2C addresses Move strict I2C address validity check to a single function, and document the reserved I2C addresses there. Signed-off-by: Jean Delvare Cc: Hans Verkuil --- drivers/i2c/i2c-core.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) (limited to 'drivers/i2c') diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 96f01331544d..af5172486fab 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -387,6 +387,27 @@ static int i2c_check_client_addr_validity(const struct i2c_client *client) return 0; } +/* And this is a strict address validity check, used when probing. If a + * device uses a reserved address, then it shouldn't be probed. 7-bit + * addressing is assumed, 10-bit address devices are rare and should be + * explicitly enumerated. */ +static int i2c_check_addr_validity(unsigned short addr) +{ + /* + * Reserved addresses per I2C specification: + * 0x00 General call address / START byte + * 0x01 CBUS address + * 0x02 Reserved for different bus format + * 0x03 Reserved for future purposes + * 0x04-0x07 Hs-mode master code + * 0x78-0x7b 10-bit slave addressing + * 0x7c-0x7f Reserved for future purposes + */ + if (addr < 0x08 || addr > 0x77) + return -EINVAL; + return 0; +} + /** * i2c_new_device - instantiate an i2c device * @adap: the adapter managing the device @@ -1340,10 +1361,11 @@ static int i2c_detect_address(struct i2c_client *temp_client, int err; /* Make sure the address is valid */ - if (addr < 0x03 || addr > 0x77) { + err = i2c_check_addr_validity(addr); + if (err) { dev_warn(&adapter->dev, "Invalid probe address 0x%02x\n", addr); - return -EINVAL; + return err; } /* Skip if already in use */ @@ -1446,7 +1468,7 @@ i2c_new_probed_device(struct i2c_adapter *adap, for (i = 0; addr_list[i] != I2C_CLIENT_END; i++) { /* Check address validity */ - if (addr_list[i] < 0x03 || addr_list[i] > 0x77) { + if (i2c_check_addr_validity(addr_list[i]) < 0) { dev_warn(&adap->dev, "Invalid 7-bit address " "0x%02x\n", addr_list[i]); continue; -- cgit v1.2.2 From 3b5f794b8b647409155cade4eac2825bc49b4a66 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Thu, 3 Jun 2010 11:33:55 +0200 Subject: i2c: Rename i2c_check_addr to i2c_check_addr_busy Otherwise it's not clear what it is checking. Also move the function to save a forward declaration. Signed-off-by: Jean Delvare --- drivers/i2c/i2c-core.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) (limited to 'drivers/i2c') diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index af5172486fab..1cca2631e5b3 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -47,7 +47,6 @@ static DEFINE_MUTEX(core_lock); static DEFINE_IDR(i2c_adapter_idr); static struct device_type i2c_client_type; -static int i2c_check_addr(struct i2c_adapter *adapter, int addr); static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver); /* ------------------------------------------------------------------------- */ @@ -408,6 +407,22 @@ static int i2c_check_addr_validity(unsigned short addr) return 0; } +static int __i2c_check_addr_busy(struct device *dev, void *addrp) +{ + struct i2c_client *client = i2c_verify_client(dev); + int addr = *(int *)addrp; + + if (client && client->addr == addr) + return -EBUSY; + return 0; +} + +static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr) +{ + return device_for_each_child(&adapter->dev, &addr, + __i2c_check_addr_busy); +} + /** * i2c_new_device - instantiate an i2c device * @adap: the adapter managing the device @@ -456,7 +471,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info) } /* Check for address business */ - status = i2c_check_addr(adap, client->addr); + status = i2c_check_addr_busy(adap, client->addr); if (status) goto out_err; @@ -1064,21 +1079,6 @@ EXPORT_SYMBOL(i2c_del_driver); /* ------------------------------------------------------------------------- */ -static int __i2c_check_addr(struct device *dev, void *addrp) -{ - struct i2c_client *client = i2c_verify_client(dev); - int addr = *(int *)addrp; - - if (client && client->addr == addr) - return -EBUSY; - return 0; -} - -static int i2c_check_addr(struct i2c_adapter *adapter, int addr) -{ - return device_for_each_child(&adapter->dev, &addr, __i2c_check_addr); -} - /** * i2c_use_client - increments the reference count of the i2c client structure * @client: the client being referenced @@ -1369,7 +1369,7 @@ static int i2c_detect_address(struct i2c_client *temp_client, } /* Skip if already in use */ - if (i2c_check_addr(adapter, addr)) + if (i2c_check_addr_busy(adapter, addr)) return 0; /* Make sure there is something at this address */ @@ -1475,7 +1475,7 @@ i2c_new_probed_device(struct i2c_adapter *adap, } /* Check address availability */ - if (i2c_check_addr(adap, addr_list[i])) { + if (i2c_check_addr_busy(adap, addr_list[i])) { dev_dbg(&adap->dev, "Address 0x%02x already in " "use, not probing\n", addr_list[i]); continue; -- cgit v1.2.2 From 35bfc353dcaecc99c277e3646564f3f785760bde Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Thu, 3 Jun 2010 11:33:56 +0200 Subject: i2c/busses: Move two drivers to embedded section And fix a typo while we are here Signed-off-by: Wolfram Sang Signed-off-by: Jean Delvare --- drivers/i2c/busses/Kconfig | 40 ++++++++++++++++++++-------------------- drivers/i2c/busses/Makefile | 6 +++--- 2 files changed, 23 insertions(+), 23 deletions(-) (limited to 'drivers/i2c') diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 87ab0568bb0e..bceafbfa7268 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -475,6 +475,26 @@ config I2C_PASEMI help Supports the PA Semi PWRficient on-chip SMBus interfaces. +config I2C_PCA_PLATFORM + tristate "PCA9564/PCA9665 as platform device" + select I2C_ALGOPCA + default n + help + This driver supports a memory mapped Philips PCA9564/PCA9665 + parallel bus to I2C bus controller. + + This driver can also be built as a module. If so, the module + will be called i2c-pca-platform. + +config I2C_PMCMSP + tristate "PMC MSP I2C TWI Controller" + depends on PMC_MSP + help + This driver supports the PMC TWI controller on MSP devices. + + This driver can also be built as module. If so, the module + will be called i2c-pmcmsp. + config I2C_PNX tristate "I2C bus support for Philips PNX targets" depends on ARCH_PNX4008 @@ -711,26 +731,6 @@ config I2C_PCA_ISA delays when I2C/SMBus chip drivers are loaded (e.g. at boot time). If unsure, say N. -config I2C_PCA_PLATFORM - tristate "PCA9564/PCA9665 as platform device" - select I2C_ALGOPCA - default n - help - This driver supports a memory mapped Philips PCA9564/PCA9665 - parallel bus to I2C bus controller. - - This driver can also be built as a module. If so, the module - will be called i2c-pca-platform. - -config I2C_PMCMSP - tristate "PMC MSP I2C TWI Controller" - depends on PMC_MSP - help - This driver supports the PMC TWI controller on MSP devices. - - This driver can also be built as module. If so, the module - will be called i2c-pmcmsp. - config I2C_SIBYTE tristate "SiByte SMBus interface" depends on SIBYTE_SB1xxx_SOC diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index 097236f631e8..936880bd1dc5 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -27,7 +27,7 @@ obj-$(CONFIG_I2C_VIAPRO) += i2c-viapro.o obj-$(CONFIG_I2C_HYDRA) += i2c-hydra.o obj-$(CONFIG_I2C_POWERMAC) += i2c-powermac.o -# Embebbed system I2C/SMBus host controller drivers +# Embedded system I2C/SMBus host controller drivers obj-$(CONFIG_I2C_AT91) += i2c-at91.o obj-$(CONFIG_I2C_AU1550) += i2c-au1550.o obj-$(CONFIG_I2C_BLACKFIN_TWI) += i2c-bfin-twi.o @@ -46,6 +46,8 @@ obj-$(CONFIG_I2C_NOMADIK) += i2c-nomadik.o obj-$(CONFIG_I2C_OCORES) += i2c-ocores.o obj-$(CONFIG_I2C_OMAP) += i2c-omap.o obj-$(CONFIG_I2C_PASEMI) += i2c-pasemi.o +obj-$(CONFIG_I2C_PCA_PLATFORM) += i2c-pca-platform.o +obj-$(CONFIG_I2C_PMCMSP) += i2c-pmcmsp.o obj-$(CONFIG_I2C_PNX) += i2c-pnx.o obj-$(CONFIG_I2C_PXA) += i2c-pxa.o obj-$(CONFIG_I2C_S3C2410) += i2c-s3c2410.o @@ -68,8 +70,6 @@ obj-$(CONFIG_I2C_TINY_USB) += i2c-tiny-usb.o obj-$(CONFIG_I2C_ACORN) += i2c-acorn.o obj-$(CONFIG_I2C_ELEKTOR) += i2c-elektor.o obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o -obj-$(CONFIG_I2C_PCA_PLATFORM) += i2c-pca-platform.o -obj-$(CONFIG_I2C_PMCMSP) += i2c-pmcmsp.o obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o obj-$(CONFIG_I2C_STUB) += i2c-stub.o obj-$(CONFIG_SCx200_ACB) += scx200_acb.o -- cgit v1.2.2 From fbae3fb1546e199ab0cd185348f8124411a1ca9d Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Thu, 3 Jun 2010 11:33:58 +0200 Subject: i2c: Remove all i2c_set_clientdata(client, NULL) in drivers I2C drivers can use the clientdata-pointer to point to private data. As I2C devices are not really unregistered, but merely detached from their driver, it used to be the drivers obligation to clear this pointer during remove() or a failed probe(). As a couple of drivers forgot to do this, it was agreed that it was cleaner if the i2c-core does this clearance when appropriate, as there is no guarantee for the lifetime of the clientdata-pointer after remove() anyhow. This feature was added to the core with commit e4a7b9b04de15f6b63da5ccdd373ffa3057a3681 to fix the faulty drivers. As there is no need anymore to clear the clientdata-pointer, remove all current occurrences in the drivers to simplify the code and prevent confusion. Signed-off-by: Wolfram Sang Acked-by: Mark Brown Acked-by: Greg Kroah-Hartman Acked-by: Richard Purdie Acked-by: Dmitry Torokhov Signed-off-by: Jean Delvare --- drivers/i2c/i2c-smbus.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/i2c') diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c index a24e0bfe9201..f61ccc1e5ea3 100644 --- a/drivers/i2c/i2c-smbus.c +++ b/drivers/i2c/i2c-smbus.c @@ -173,7 +173,6 @@ static int smbalert_remove(struct i2c_client *ara) cancel_work_sync(&alert->alert); - i2c_set_clientdata(ara, NULL); kfree(alert); return 0; } -- cgit v1.2.2