i2c driver fixes for 2.6.0-test3
Subsystems (3)
PATCH 1/1 i2c driver fixes for 2.6.0-test3
Hi,
Here are some i2c driver fixes for 2.6.0-test3. They include a fix for the i2c-dev driver that has gone into 2.4 and a pii4x bugfix from Tom Rini. I've also added adapter and client "name" files in sysfs as the driver core no longer supplies them.
Please pull from: bk://kernel.bkbits.net/gregkh/linux/i2c-2.5
thanks,
greg k-h
drivers/i2c/busses/i2c-nforce2.c | 29 ++++----------------------- drivers/i2c/busses/i2c-piix4.c | 13 ++++++++++-- drivers/i2c/chips/Makefile | 4 ++- drivers/i2c/chips/adm1021.c | 8 +++---- drivers/i2c/i2c-core.c | 28 ++++++++++++++++++++++++++ drivers/i2c/i2c-dev.c | 41 ++++++++++++++++++++++++--------------- 6 files changed, 77 insertions(+), 46 deletions(-)
-----
<khali:linux-fr.org>:
o i2c: user/kernel bug and memory leak in i2c-dev
Greg Kroah-Hartman:
o I2C: add adapter and client name files as the driver core no longer provides them
o I2C: fix up the wording for the pii4x bugfix
o i2c: move w83481d to top of link order due to chip address takeover ability
o i2c: fix up "raw" printk() call
Mark M. Hoffman:
o I2C: i2c nforce2.c fixes
Rusty Lynch:
o I2C: bugfix for initialization bug in adm1021 driver
Tom Rini:
o I2C: Fix for i2c-piix4 with on some boards
PATCH 1/1 i2c driver changes 2.6.0-test3
ChangeSet 1.1123.18.1, 2003/08/11 14:36:10-07:00, [email protected]
[PATCH] I2C: Fix for i2c-piix4 with on some boards
Hello all. On some boards with an Intel PIIX4 the BIOS (such as the one found on a Force CPCI-735) will incorrectly configure the chipset, and leaves a register in an undefined state causing i2c to behave strangley. The following patches (first vs lm_sensors-2.8.0 and then vs 2.6.0-test3) fixes the issue.
drivers/i2c/busses/i2c-piix4.c | 9 +++++++++ 1 files changed, 9 insertions(+)
drivers/i2c/busses/i2c-piix4.c +9 -0
@@ -160,6 +160,15 @@ }pci_read_config_byte (PIIX4_dev,SMBHSTCFG , &temp); + +/* Some BIOS will set up the chipset incorrectly and leave a register +in an undefined state (causing I2C to act very strangely). */ +if (temp &0x02 ) { +printk ("Fixed I2C problem on Force CPCI735\n" ); + temp = temp &0xfd ; +pci_write_config_byte (PIIX4_dev,SMBHSTCFG , temp); + } +/* If force_addr is set, we program the new address here. Just to make sure, we disable the PIIX4 first. */ if (force_addr) {
Re: [PATCH] i2c driver changes 2.6.0-test3
ChangeSet 1.1123.18.2, 2003/08/11 14:36:29-07:00, [email protected]
[PATCH] i2c: user/kernel bug and memory leak in i2c-dev
drivers/i2c/i2c-dev.c | 41 ++++++++++++++++++++++++++--------------- 1 files changed, 26 insertions(+), 15 deletions(-)
drivers/i2c/i2c-dev.c +26 -15
@@ -181,6 +181,7 @@struct i2c_smbus_ioctl_data data_arg;union i2c_smbus_data temp;struct i2c_msg *rdwr_pa; +u8 **data_ptrs;int i,datasize,res;unsigned long funcs;
@@ -214,7 +215,7 @@return (copy_to_user ((unsigned long __user *)arg, &funcs,sizeof (unsigned long )))?-EFAULT :0 ; -case I2C_RDWR : +case I2C_RDWR :if (copy_from_user (&rdwr_arg, (struct i2c_rdwr_ioctl_data __user *)arg,sizeof (rdwr_arg)))
@@ -231,28 +232,37 @@if (rdwr_pa ==NULL )return -ENOMEM ; +if (copy_from_user (rdwr_pa, rdwr_arg.msgs, + rdwr_arg.nmsgs *sizeof (struct i2c_msg ))) { +kfree (rdwr_pa); +return -EFAULT ; + } + + data_ptrs = (u8 **)kmalloc (rdwr_arg.nmsgs *sizeof (u8 *), +GFP_KERNEL ); +if (data_ptrs ==NULL ) { +kfree (rdwr_pa); +return -ENOMEM ; + } + res =0 ;for ( i=0 ; i<rdwr_arg.nmsgs; i++ ) { -if (copy_from_user (&(rdwr_pa[i]), - &(rdwr_arg.msgs[i]), -sizeof (rdwr_pa[i]))) { - res = -EFAULT ; -break ; - }/* Limit the size of the message to a sane amount */ if (rdwr_pa[i].len >8192 ) { res = -EINVAL ;break ; } + data_ptrs[i] = rdwr_pa[i].buf; rdwr_pa[i].buf =kmalloc (rdwr_pa[i].len,GFP_KERNEL );if (rdwr_pa[i].buf ==NULL ) { res = -ENOMEM ;break ; }if (copy_from_user (rdwr_pa[i].buf, - rdwr_arg.msgs[i].buf, + data_ptrs[i], rdwr_pa[i].len)) { - res = -EFAULT ; + ++i;/* Needs to be kfreed too */ + res = -EFAULT ;break ; } }
@@ -260,18 +270,18 @@int j;for (j =0 ; j < i; ++j)kfree (rdwr_pa[j].buf); +kfree (data_ptrs);kfree (rdwr_pa);return res; } -if (!res) { - res =i2c_transfer (client->adapter, - rdwr_pa, - rdwr_arg.nmsgs); - } + + res =i2c_transfer (client->adapter, + rdwr_pa, + rdwr_arg.nmsgs);while (i-- >0 ) {if ( res>=0 && (rdwr_pa[i].flags &I2C_M_RD )) {if (copy_to_user ( - rdwr_arg.msgs[i].buf, + data_ptrs[i], rdwr_pa[i].buf, rdwr_pa[i].len)) { res = -EFAULT ;
@@ -279,6 +289,7 @@ }kfree (rdwr_pa[i].buf); } +kfree (data_ptrs);kfree (rdwr_pa);return res;
Re: [PATCH] i2c driver changes 2.6.0-test3
ChangeSet 1.1123.18.3, 2003/08/11 14:36:57-07:00, [email protected]
[PATCH] i2c: fix up "raw" printk() call.
drivers/i2c/busses/i2c-piix4.c | 2 +- 1 files changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/i2c-piix4.c +1 -1
@@ -164,7 +164,7 @@/* Some BIOS will set up the chipset incorrectly and leave a register in an undefined state (causing I2C to act very strangely). */ if (temp &0x02 ) { -printk ("Fixed I2C problem on Force CPCI735\n" ); +dev_info (&PIIX4_dev->dev,"Fixed I2C problem on Force CPCI735\n" ); temp = temp &0xfd ;pci_write_config_byte (PIIX4_dev,SMBHSTCFG , temp); }
Re: [PATCH] i2c driver changes 2.6.0-test3
ChangeSet 1.1123.18.4, 2003/08/11 15:22:22-07:00, [email protected]
i2c: move w83481d to top of link order due to chip address takeover ability.
Fixes http://bugme.osdl.org/show_bug.cgi?id=1066
drivers/i2c/chips/Makefile | 4 +++- 1 files changed, 3 insertions(+), 1 deletion(-)
drivers/i2c/chips/Makefile +3 -1
@@ -2,10 +2,12 @@ # Makefile for the kernel hardware sensors chip drivers. # +# w83781d goes first, as it can override other driver's addresses. +obj-$(CONFIG_SENSORS_W83781D) += w83781d.o + obj-$(CONFIG_SENSORS_ADM1021) += adm1021.o obj-$(CONFIG_SENSORS_IT87) += it87.o obj-$(CONFIG_SENSORS_LM75) += lm75.o obj-$(CONFIG_SENSORS_LM78) += lm78.o obj-$(CONFIG_SENSORS_LM85) += lm85.o obj-$(CONFIG_SENSORS_VIA686A) += via686a.o -obj-$(CONFIG_SENSORS_W83781D) += w83781d.o
Re: [PATCH] i2c driver changes 2.6.0-test3
ChangeSet 1.1123.18.5, 2003/08/14 14:13:01-07:00, [email protected]
[PATCH] I2C: i2c nforce2.c fixes
This patch restores a line that was wrongly removed. There are also some trivial cleanups. It applies & compiles vs. 2.6.0-test3. It's untested (no hardware here).
drivers/i2c/busses/i2c-nforce2.c | 29 +++++------------------------ 1 files changed, 5 insertions(+), 24 deletions(-)
drivers/i2c/busses/i2c-nforce2.c +5 -24
@@ -51,10 +51,6 @@#ifndef PCI_DEVICE_ID_NVIDIA_NFORCE2_SMBUS #define PCI_DEVICE_ID_NVIDIA_NFORCE2_SMBUS 0x0064#endif -/* TODO: sync with lm-sensors */ -#ifndef I2C_HW_SMBUS_NFORCE2 -#define I2C_HW_SMBUS_NFORCE2 0x0c -#endif struct nforce2_smbus {
@@ -128,20 +124,10 @@ .name = "unset", }; - -#if 0 -/* Internally used pause function */ -static void nforce2_do_pause (unsigned int amount) -{ - current->state =TASK_INTERRUPTIBLE ; -schedule_timeout (amount); -} -#endif -/* Return -1 on error. See smbus.h for more information */ -static s32 nforce2_access (struct i2c_adapter * adap,u16 addr,unsigned short flags, -char read_write,u8 command,int size, -union i2c_smbus_data * data) +static s32 nforce2_access (struct i2c_adapter * adap,u16 addr, +unsigned short flags,char read_write, +u8 command,int size,union i2c_smbus_data * data) {struct nforce2_smbus *smbus = adap->algo_data;unsigned char protocol, pec, temp;
@@ -249,7 +235,7 @@#if 0 do { -nforce2_do_pause (1 ); +i2c_do_pause (1 ); temp =inb_p (NVIDIA_SMB_STS ); }while (((temp &NVIDIA_SMB_STS_DONE ) ==0 ) && (timeout++ <MAX_TIMEOUT ));#endif
@@ -332,13 +318,8 @@ smbus->base, smbus->base+smbus->size-1, name);return -1 ; } -/* -smbus->adapter.owner = THIS_MODULE; -smbus->adapter.id = I2C_ALGO_SMBUS | I2C_HW_SMBUS_NFORCE2; -smbus->adapter.algo = &smbus_algorithm; -smbus->adapter.algo_data = smbus; -*/ smbus->adapter = nforce2_adapter; + smbus->adapter.algo_data = smbus; smbus->adapter.dev.parent = &dev->dev;snprintf (smbus->adapter.name,DEVICE_NAME_SIZE ,"SMBus nForce2 adapter at %04x" , smbus->base);
Re: [PATCH] i2c driver changes 2.6.0-test3
ChangeSet 1.1123.18.6, 2003/08/14 14:31:21-07:00, [email protected]
[PATCH] I2C: bugfix for initialization bug in adm1021 driver
While initializing the adm1021 device, the driver is performing a conversion from fixed point to Celcius on values that were declaired as Celcius. On my Dell Precision 220 this results in a shutdown after a couple of minutes running.
This is a very simple patch against the 2.6.0-test3 tree that just removes the conversion.
drivers/i2c/chips/adm1021.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
drivers/i2c/chips/adm1021.c +4 -4
@@ -356,13 +356,13 @@ {/* Initialize the adm1021 chip */ adm1021_write_value (client,ADM1021_REG_TOS_W , -TEMP_TO_REG (adm1021_INIT_TOS)); + adm1021_INIT_TOS);adm1021_write_value (client,ADM1021_REG_THYST_W , -TEMP_TO_REG (adm1021_INIT_THYST)); + adm1021_INIT_THYST);adm1021_write_value (client,ADM1021_REG_REMOTE_TOS_W , -TEMP_TO_REG (adm1021_INIT_REMOTE_TOS)); + adm1021_INIT_REMOTE_TOS);adm1021_write_value (client,ADM1021_REG_REMOTE_THYST_W , -TEMP_TO_REG (adm1021_INIT_REMOTE_THYST)); + adm1021_INIT_REMOTE_THYST);/* Enable ADC and disable suspend mode */ adm1021_write_value (client,ADM1021_REG_CONFIG_W ,0 );/* Set Conversion rate to 1/sec (this can be tinkered with) */
Re: [PATCH] i2c driver changes 2.6.0-test3
ChangeSet 1.1123.18.7, 2003/08/14 14:46:08-07:00, [email protected]
[PATCH] I2C: fix up the wording for the pii4x bugfix.
drivers/i2c/busses/i2c-piix4.c | 2 +- 1 files changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/i2c-piix4.c +1 -1
@@ -164,7 +164,7 @@/* Some BIOS will set up the chipset incorrectly and leave a register in an undefined state (causing I2C to act very strangely). */ if (temp &0x02 ) { -dev_info (&PIIX4_dev->dev,"Fixed I2C problem on Force CPCI735\n" ); +dev_info (&PIIX4_dev->dev,"Worked around buggy BIOS (I2C)\n" ); temp = temp &0xfd ;pci_write_config_byte (PIIX4_dev,SMBHSTCFG , temp); }
Re: [PATCH] i2c driver changes 2.6.0-test3
ChangeSet 1.1123.18.8, 2003/08/15 11:06:52-07:00, [email protected]
I2C: add adapter and client name files as the driver core no longer provides them.
drivers/i2c/i2c-core.c | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+)
drivers/i2c/i2c-core.c +28 -0
@@ -79,12 +79,36 @@ .release = &i2c_adapter_class_dev_release, }; +static ssize_t show_adapter_name (struct device *dev,char *buf) +{ +struct i2c_adapter *adap =dev_to_i2c_adapter (dev); +return sprintf (buf,"%s\n" , adap->name); +} +static DEVICE_ATTR(name, S_IRUGO, show_adapter_name, NULL); + +static void i2c_client_release (struct device *dev) {struct i2c_client *client =to_i2c_client (dev);complete (&client->released); } +static ssize_t show_client_name (struct device *dev,char *buf) +{ +struct i2c_client *client =to_i2c_client (dev); +return sprintf (buf,"%s\n" , client->name); +} + +/* +* We can't use the DEVICE_ATTR() macro here as we want the same filename for a +* different type of a device. So beware if the DEVICE_ATTR() macro ever +* changes, this definition will also have to change. +*/ +static struct device_attribute dev_attr_client_name = { + .attr = {.name ="name" , .mode =S_IRUGO , .owner =THIS_MODULE }, + .show = &show_client_name, +}; +/* --------------------------------------------------- * registering functions
@@ -120,6 +144,7 @@ adap->dev.driver = &i2c_adapter_driver; adap->dev.release = &i2c_adapter_dev_release;device_register (&adap->dev); +device_create_file (&adap->dev, &dev_attr_name);/* Add this adapter to the i2c_adapter class */ memset (&adap->class_dev,0x00 ,sizeof (struct class_device ));
@@ -184,6 +209,7 @@init_completion (&adap->dev_released);init_completion (&adap->class_dev_released);class_device_unregister (&adap->class_dev); +device_remove_file (&adap->dev, &dev_attr_name);device_unregister (&adap->dev);list_del (&adap->list);
@@ -361,6 +387,7 @@ "%d-%04x", i2c_adapter_id(adapter), client->addr);printk ("registering %s\n" , client->dev.bus_id);device_register (&client->dev); +device_create_file (&client->dev, &dev_attr_client_name);return 0 ; }
@@ -387,6 +414,7 @@down (&adapter->clist_lock);list_del (&client->list);init_completion (&client->released); +device_remove_file (&client->dev, &dev_attr_client_name);device_unregister (&client->dev);up (&adapter->clist_lock);wait_for_completion (&client->released);
PATCH 1/1 i2c driver changes 2.6.0-test3
In reply to Greg KHWhy don't we just make it policy that now and in the future we don't initialize the limits on the sensors?
We don't know in what crazy way a motherboard manufacturer may have wired or scaled the inputs and with the newer sensors, it's leading to shutdowns and fan spinup behavior that is "less than optimal".
If the BIOS knows what reasonable limits are, it will set them and we are better off reading and displaying those values than resetting them. If the BIOS hasn't set any limits, the hardware almost always (does anyone have a counter example?) initializes with the limits at the extreme values. No harm there. We've got (or had...) a user space library that could initialize limits with much more configurability than the drivers ever will have.
I'd really like to see this whole initialization mess go away...
And it would make the drivers blah-blah lines smaller ... :)
:v)
Quoted 41 lines · Greg KH
ChangeSet 1.1123.18.6, 2003/08/14 14:31:21-07:00, [email protected]
[PATCH] I2C: bugfix for initialization bug in adm1021 driver
While initializing the adm1021 device, the driver is performing a conversion from fixed point to Celcius on values that were declaired as Celcius. On my Dell Precision 220 this results in a shutdown after a couple of minutes running.
This is a very simple patch against the 2.6.0-test3 tree that just removes the conversion.
drivers/i2c/chips/adm1021.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)diff -Nru
a/drivers/i2c/chips/adm1021.cb/drivers/i2c/chips/adm1021.cdrivers/i2c/chips/adm1021.c +4
-4@@ -356,13 +356,13 @@ {/* Initialize the adm1021 chip */ adm1021_write_value (client,ADM1021_REG_TOS_W , -TEMP_TO_REG (adm1021_INIT_TOS)); + adm1021_INIT_TOS);adm1021_write_value (client,ADM1021_REG_THYST_W , -TEMP_TO_REG (adm1021_INIT_THYST)); + adm1021_INIT_THYST);adm1021_write_value (client,ADM1021_REG_REMOTE_TOS_W , -TEMP_TO_REG (adm1021_INIT_REMOTE_TOS)); + adm1021_INIT_REMOTE_TOS);adm1021_write_value (client,ADM1021_REG_REMOTE_THYST_W , -TEMP_TO_REG (adm1021_INIT_REMOTE_THYST)); + adm1021_INIT_REMOTE_THYST);/* Enable ADC and disable suspend mode */ adm1021_write_value (client,ADM1021_REG_CONFIG_W ,0 );/* Set Conversion rate to 1/sec (this can be tinkered with) */
PATCH 1/1 i2c driver changes 2.6.0-test3
In reply to Greg KHQuoted 2 lines · Philip Pokorny
Why don't we just make it policy that now and in the future we don't initialize the limits on the sensors?
Sounds good to me. Anyone want to test the existing 2.6 drivers with the init code ripped out from them and send me a patch?
thanks,
greg k-h
PATCH 1/1 i2c driver changes 2.6.0-test3
In reply to Greg KHQuoted 4 lines
Why don't we just make it policy that now and in the future we don't initialize the limits on the sensors?
(...)
I think I agree. As long as it doesn't cause extra problems, I believe the drivers should be kept as simple and "neutral" as possible. If certain drivers really have to intialize limits for any reason, it's OK, but I don't object against the default policy being to not initialize.
(Which doesn't mean I'll rush to remove initializations in "my" drivers".)
Quoted 3 lines
We don't know in what crazy way a motherboard manufacturer may have wired or scaled the inputs and with the newer sensors, it's leading to shutdowns and fan spinup behavior that is "less than optimal".
In this precise case, the reason is not initializing, but initializing with awful values. That's different.
Signature · 2 lines
Jean Delvare http://www.ensicaen.ismra.fr/~delvare/
PATCH 1/1 i2c driver changes 2.6.0-test3
In reply to Greg KHQuoted 3 lines
agreed. it causes lots of problems, e.g. "I loaded your driver and my PC beeped like crazy and then shut down".
Yes, I remember these ones.
Quoted 1 line
should we do this for 2.8.1?
It's not required. To me, 2.8.1 has to be released right after Linux 2.4.22 so that we can have our i2c subsystem go into Linux 2.4. The rest if far less important.
Quoted 26 lines
Quoted 24 lines · Jean Delvare
Quoted 4 lines
Why don't we just make it policy that now and in the future we don't
initialize the limits on the sensors?
(...)I think I agree. As long as it doesn't cause extra problems, I believe the drivers should be kept as simple and "neutral" as possible. If certain drivers really have to intialize limits for any reason, it's OK, but I don't object against the default policy being to not initialize.
(Which doesn't mean I'll rush to remove initializations in "my" drivers".)
Quoted 2 lines
We don't know in what crazy way a motherboard manufacturer may have wired or scaled the inputs and with the newer sensors, it's leading
to>shutdowns and fan spinup behavior that is "less than optimal".
In this precise case, the reason is not initializing, but initializing with awful values. That's different.
Signature · 2 lines
Jean Delvare http://www.ensicaen.ismra.fr/~delvare/
PATCH 1/1 i2c driver changes 2.6.0-test3
In reply to Greg KHagreed. it causes lots of problems, e.g. "I loaded your driver and my PC beeped like crazy and then shut down".
should we do this for 2.8.1?
Quoted 22 lines · Jean Delvare
Quoted 3 lines
Why don't we just make it policy that now and in the future we don't initialize the limits on the sensors?
(...)I think I agree. As long as it doesn't cause extra problems, I believe the drivers should be kept as simple and "neutral" as possible. If certain drivers really have to intialize limits for any reason, it's OK, but I don't object against the default policy being to not initialize.
(Which doesn't mean I'll rush to remove initializations in "my" drivers".)
Quoted 3 lines
We don't know in what crazy way a motherboard manufacturer may have wired or scaled the inputs and with the newer sensors, it's leading to shutdowns and fan spinup behavior that is "less than optimal".
In this precise case, the reason is not initializing, but initializing with awful values. That's different.