* [PATCH 0/5] I8K driver facelift
@ 2005-02-24 6:10 Dmitry Torokhov
2005-02-24 6:11 ` [PATCH 1/5] I8K - pass though Lindent Dmitry Torokhov
2005-03-13 3:41 ` [PATCH 0/5] I8K driver facelift Frank Sorenson
0 siblings, 2 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2005-02-24 6:10 UTC (permalink / raw)
To: LKML; +Cc: Massimo Dal Zotto
Hi,
here are some changes that freshen I8K driver (Dell Inspiron/Latitude
platform driver). The patches have been tested on Inspiron 8100.
i8k-lindent.patch
- pass the driver through Lindent to comply with CondingStyle requirements
(4 spaces vs. TAB indentation)
i8k-use-dmi.patch
- use standard DMI handling functions instead of homemade ones. The driver
now requires DMI data to match list of supported models - this way driver
can be safely enabled without fear of it poking into SMM code on wrong
box. DMI checks can be ignored with i8k.ignore_dmi option.
i8k-seqfile.patch
- switch proc handlig code to seq_file instead of having custom read
function splitting output to fit into user's buffer.
i8k-cleanup.patch
- use module_{init|exit} instead of old-style module intialization code,
some formatting changes.
i8k-sysfs.patch
- make i8k a platform device and export temperatiure and both fan states
as sysfs attributes. Wringing into fan1_state and fan2_state attributes
allows switching fans on and off without need for special utility.
Please consider for inclusion.
Thanks!
--
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH 1/5] I8K - pass though Lindent 2005-02-24 6:10 [PATCH 0/5] I8K driver facelift Dmitry Torokhov @ 2005-02-24 6:11 ` Dmitry Torokhov 2005-02-24 6:12 ` [PATCH 2/5] I8K - use standard DMI functions Dmitry Torokhov 2005-03-13 3:41 ` [PATCH 0/5] I8K driver facelift Frank Sorenson 1 sibling, 1 reply; 28+ messages in thread From: Dmitry Torokhov @ 2005-02-24 6:11 UTC (permalink / raw) To: LKML; +Cc: Massimo Dal Zotto =================================================================== I8K: pass through Lindent to change 4 spaces identation to TABs Signed-off-by: Dmitry Torokhov <dtor@mail.ru> i8k.c | 954 +++++++++++++++++++++++++++++++++--------------------------------- 1 files changed, 477 insertions(+), 477 deletions(-) Index: dtor/drivers/char/i8k.c =================================================================== --- dtor.orig/drivers/char/i8k.c +++ dtor/drivers/char/i8k.c @@ -55,14 +55,14 @@ #define DELL_SIGNATURE "Dell Computer" static char *supported_models[] = { - "Inspiron", - "Latitude", - NULL + "Inspiron", + "Latitude", + NULL }; static char system_vendor[48] = "?"; -static char product_name [48] = "?"; -static char bios_version [4] = "?"; +static char product_name[48] = "?"; +static char bios_version[4] = "?"; static char serial_number[16] = "?"; MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)"); @@ -86,64 +86,63 @@ static int i8k_ioctl(struct inode *, str unsigned long); static struct file_operations i8k_fops = { - .read = i8k_read, - .ioctl = i8k_ioctl, + .read = i8k_read, + .ioctl = i8k_ioctl, }; typedef struct { - unsigned int eax; - unsigned int ebx __attribute__ ((packed)); - unsigned int ecx __attribute__ ((packed)); - unsigned int edx __attribute__ ((packed)); - unsigned int esi __attribute__ ((packed)); - unsigned int edi __attribute__ ((packed)); + unsigned int eax; + unsigned int ebx __attribute__ ((packed)); + unsigned int ecx __attribute__ ((packed)); + unsigned int edx __attribute__ ((packed)); + unsigned int esi __attribute__ ((packed)); + unsigned int edi __attribute__ ((packed)); } SMMRegisters; typedef struct { - u8 type; - u8 length; - u16 handle; + u8 type; + u8 length; + u16 handle; } DMIHeader; /* * Call the System Management Mode BIOS. Code provided by Jonathan Buzzard. */ -static int i8k_smm(SMMRegisters *regs) +static int i8k_smm(SMMRegisters * regs) { - int rc; - int eax = regs->eax; + int rc; + int eax = regs->eax; - asm("pushl %%eax\n\t" \ - "movl 0(%%eax),%%edx\n\t" \ - "push %%edx\n\t" \ - "movl 4(%%eax),%%ebx\n\t" \ - "movl 8(%%eax),%%ecx\n\t" \ - "movl 12(%%eax),%%edx\n\t" \ - "movl 16(%%eax),%%esi\n\t" \ - "movl 20(%%eax),%%edi\n\t" \ - "popl %%eax\n\t" \ - "out %%al,$0xb2\n\t" \ - "out %%al,$0x84\n\t" \ - "xchgl %%eax,(%%esp)\n\t" - "movl %%ebx,4(%%eax)\n\t" \ - "movl %%ecx,8(%%eax)\n\t" \ - "movl %%edx,12(%%eax)\n\t" \ - "movl %%esi,16(%%eax)\n\t" \ - "movl %%edi,20(%%eax)\n\t" \ - "popl %%edx\n\t" \ - "movl %%edx,0(%%eax)\n\t" \ - "lahf\n\t" \ - "shrl $8,%%eax\n\t" \ - "andl $1,%%eax\n" \ - : "=a" (rc) - : "a" (regs) - : "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory"); - - if ((rc != 0) || ((regs->eax & 0xffff) == 0xffff) || (regs->eax == eax)) { - return -EINVAL; - } + asm("pushl %%eax\n\t" + "movl 0(%%eax),%%edx\n\t" + "push %%edx\n\t" + "movl 4(%%eax),%%ebx\n\t" + "movl 8(%%eax),%%ecx\n\t" + "movl 12(%%eax),%%edx\n\t" + "movl 16(%%eax),%%esi\n\t" + "movl 20(%%eax),%%edi\n\t" + "popl %%eax\n\t" + "out %%al,$0xb2\n\t" + "out %%al,$0x84\n\t" + "xchgl %%eax,(%%esp)\n\t" + "movl %%ebx,4(%%eax)\n\t" + "movl %%ecx,8(%%eax)\n\t" + "movl %%edx,12(%%eax)\n\t" + "movl %%esi,16(%%eax)\n\t" + "movl %%edi,20(%%eax)\n\t" + "popl %%edx\n\t" + "movl %%edx,0(%%eax)\n\t" + "lahf\n\t" + "shrl $8,%%eax\n\t" + "andl $1,%%eax\n":"=a"(rc) + : "a"(regs) + : "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory"); - return 0; + if ((rc != 0) || ((regs->eax & 0xffff) == 0xffff) || (regs->eax == eax)) { + return -EINVAL; + } + + return 0; } /* @@ -152,15 +151,15 @@ static int i8k_smm(SMMRegisters *regs) */ static int i8k_get_bios_version(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; + int rc; - regs.eax = I8K_SMM_BIOS_VERSION; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } + regs.eax = I8K_SMM_BIOS_VERSION; + if ((rc = i8k_smm(®s)) < 0) { + return rc; + } - return regs.eax; + return regs.eax; } /* @@ -168,8 +167,8 @@ static int i8k_get_bios_version(void) */ static int i8k_get_serial_number(unsigned char *buff) { - strlcpy(buff, serial_number, sizeof(serial_number)); - return 0; + strlcpy(buff, serial_number, sizeof(serial_number)); + return 0; } /* @@ -177,24 +176,24 @@ static int i8k_get_serial_number(unsigne */ static int i8k_get_fn_status(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; + int rc; - regs.eax = I8K_SMM_FN_STATUS; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - - switch ((regs.eax >> I8K_FN_SHIFT) & I8K_FN_MASK) { - case I8K_FN_UP: - return I8K_VOL_UP; - case I8K_FN_DOWN: - return I8K_VOL_DOWN; - case I8K_FN_MUTE: - return I8K_VOL_MUTE; - default: - return 0; - } + regs.eax = I8K_SMM_FN_STATUS; + if ((rc = i8k_smm(®s)) < 0) { + return rc; + } + + switch ((regs.eax >> I8K_FN_SHIFT) & I8K_FN_MASK) { + case I8K_FN_UP: + return I8K_VOL_UP; + case I8K_FN_DOWN: + return I8K_VOL_DOWN; + case I8K_FN_MUTE: + return I8K_VOL_MUTE; + default: + return 0; + } } /* @@ -202,20 +201,20 @@ static int i8k_get_fn_status(void) */ static int i8k_get_power_status(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; + int rc; + + regs.eax = I8K_SMM_POWER_STATUS; + if ((rc = i8k_smm(®s)) < 0) { + return rc; + } - regs.eax = I8K_SMM_POWER_STATUS; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - - switch (regs.eax & 0xff) { - case I8K_POWER_AC: - return I8K_AC; - default: - return I8K_BATTERY; - } + switch (regs.eax & 0xff) { + case I8K_POWER_AC: + return I8K_AC; + default: + return I8K_BATTERY; + } } /* @@ -223,16 +222,16 @@ static int i8k_get_power_status(void) */ static int i8k_get_fan_status(int fan) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; + int rc; - regs.eax = I8K_SMM_GET_FAN; - regs.ebx = fan & 0xff; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } + regs.eax = I8K_SMM_GET_FAN; + regs.ebx = fan & 0xff; + if ((rc = i8k_smm(®s)) < 0) { + return rc; + } - return (regs.eax & 0xff); + return (regs.eax & 0xff); } /* @@ -240,16 +239,16 @@ static int i8k_get_fan_status(int fan) */ static int i8k_get_fan_speed(int fan) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; + int rc; - regs.eax = I8K_SMM_GET_SPEED; - regs.ebx = fan & 0xff; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } + regs.eax = I8K_SMM_GET_SPEED; + regs.ebx = fan & 0xff; + if ((rc = i8k_smm(®s)) < 0) { + return rc; + } - return (regs.eax & 0xffff) * I8K_FAN_MULT; + return (regs.eax & 0xffff) * I8K_FAN_MULT; } /* @@ -257,18 +256,18 @@ static int i8k_get_fan_speed(int fan) */ static int i8k_set_fan(int fan, int speed) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; + int rc; - speed = (speed < 0) ? 0 : ((speed > I8K_FAN_MAX) ? I8K_FAN_MAX : speed); + speed = (speed < 0) ? 0 : ((speed > I8K_FAN_MAX) ? I8K_FAN_MAX : speed); - regs.eax = I8K_SMM_SET_FAN; - regs.ebx = (fan & 0xff) | (speed << 8); - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } + regs.eax = I8K_SMM_SET_FAN; + regs.ebx = (fan & 0xff) | (speed << 8); + if ((rc = i8k_smm(®s)) < 0) { + return rc; + } - return (i8k_get_fan_status(fan)); + return (i8k_get_fan_status(fan)); } /* @@ -276,143 +275,143 @@ static int i8k_set_fan(int fan, int spee */ static int i8k_get_cpu_temp(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; - int temp; + SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; + int rc; + int temp; #ifdef I8K_TEMPERATURE_BUG - static int prev = 0; + static int prev = 0; #endif - regs.eax = I8K_SMM_GET_TEMP; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - temp = regs.eax & 0xff; + regs.eax = I8K_SMM_GET_TEMP; + if ((rc = i8k_smm(®s)) < 0) { + return rc; + } + temp = regs.eax & 0xff; #ifdef I8K_TEMPERATURE_BUG - /* - * Sometimes the temperature sensor returns 0x99, which is out of range. - * In this case we return (once) the previous cached value. For example: - # 1003655137 00000058 00005a4b - # 1003655138 00000099 00003a80 <--- 0x99 = 153 degrees - # 1003655139 00000054 00005c52 - */ - if (temp > I8K_MAX_TEMP) { - temp = prev; - prev = I8K_MAX_TEMP; - } else { - prev = temp; - } + /* + * Sometimes the temperature sensor returns 0x99, which is out of range. + * In this case we return (once) the previous cached value. For example: + # 1003655137 00000058 00005a4b + # 1003655138 00000099 00003a80 <--- 0x99 = 153 degrees + # 1003655139 00000054 00005c52 + */ + if (temp > I8K_MAX_TEMP) { + temp = prev; + prev = I8K_MAX_TEMP; + } else { + prev = temp; + } #endif - return temp; + return temp; } static int i8k_get_dell_signature(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; + int rc; - regs.eax = I8K_SMM_GET_DELL_SIG; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } + regs.eax = I8K_SMM_GET_DELL_SIG; + if ((rc = i8k_smm(®s)) < 0) { + return rc; + } - if ((regs.eax == 1145651527) && (regs.edx == 1145392204)) { - return 0; - } else { - return -1; - } + if ((regs.eax == 1145651527) && (regs.edx == 1145392204)) { + return 0; + } else { + return -1; + } } static int i8k_ioctl(struct inode *ip, struct file *fp, unsigned int cmd, unsigned long arg) { - int val; - int speed; - unsigned char buff[16]; - int __user *argp = (int __user *)arg; - - if (!argp) - return -EINVAL; - - switch (cmd) { - case I8K_BIOS_VERSION: - val = i8k_get_bios_version(); - break; - - case I8K_MACHINE_ID: - memset(buff, 0, 16); - val = i8k_get_serial_number(buff); - break; - - case I8K_FN_STATUS: - val = i8k_get_fn_status(); - break; - - case I8K_POWER_STATUS: - val = i8k_get_power_status(); - break; - - case I8K_GET_TEMP: - val = i8k_get_cpu_temp(); - break; - - case I8K_GET_SPEED: - if (copy_from_user(&val, argp, sizeof(int))) { - return -EFAULT; - } - val = i8k_get_fan_speed(val); - break; - - case I8K_GET_FAN: - if (copy_from_user(&val, argp, sizeof(int))) { - return -EFAULT; - } - val = i8k_get_fan_status(val); - break; - - case I8K_SET_FAN: - if (restricted && !capable(CAP_SYS_ADMIN)) { - return -EPERM; - } - if (copy_from_user(&val, argp, sizeof(int))) { - return -EFAULT; - } - if (copy_from_user(&speed, argp+1, sizeof(int))) { - return -EFAULT; - } - val = i8k_set_fan(val, speed); - break; - - default: - return -EINVAL; - } - - if (val < 0) { - return val; - } - - switch (cmd) { - case I8K_BIOS_VERSION: - if (copy_to_user(argp, &val, 4)) { - return -EFAULT; - } - break; - case I8K_MACHINE_ID: - if (copy_to_user(argp, buff, 16)) { - return -EFAULT; - } - break; - default: - if (copy_to_user(argp, &val, sizeof(int))) { - return -EFAULT; + int val; + int speed; + unsigned char buff[16]; + int __user *argp = (int __user *)arg; + + if (!argp) + return -EINVAL; + + switch (cmd) { + case I8K_BIOS_VERSION: + val = i8k_get_bios_version(); + break; + + case I8K_MACHINE_ID: + memset(buff, 0, 16); + val = i8k_get_serial_number(buff); + break; + + case I8K_FN_STATUS: + val = i8k_get_fn_status(); + break; + + case I8K_POWER_STATUS: + val = i8k_get_power_status(); + break; + + case I8K_GET_TEMP: + val = i8k_get_cpu_temp(); + break; + + case I8K_GET_SPEED: + if (copy_from_user(&val, argp, sizeof(int))) { + return -EFAULT; + } + val = i8k_get_fan_speed(val); + break; + + case I8K_GET_FAN: + if (copy_from_user(&val, argp, sizeof(int))) { + return -EFAULT; + } + val = i8k_get_fan_status(val); + break; + + case I8K_SET_FAN: + if (restricted && !capable(CAP_SYS_ADMIN)) { + return -EPERM; + } + if (copy_from_user(&val, argp, sizeof(int))) { + return -EFAULT; + } + if (copy_from_user(&speed, argp + 1, sizeof(int))) { + return -EFAULT; + } + val = i8k_set_fan(val, speed); + break; + + default: + return -EINVAL; } - break; - } - return 0; + if (val < 0) { + return val; + } + + switch (cmd) { + case I8K_BIOS_VERSION: + if (copy_to_user(argp, &val, 4)) { + return -EFAULT; + } + break; + case I8K_MACHINE_ID: + if (copy_to_user(argp, buff, 16)) { + return -EFAULT; + } + break; + default: + if (copy_to_user(argp, &val, sizeof(int))) { + return -EFAULT; + } + break; + } + + return 0; } /* @@ -420,90 +419,87 @@ static int i8k_ioctl(struct inode *ip, s */ static int i8k_get_info(char *buffer, char **start, off_t fpos, int length) { - int n, fn_key, cpu_temp, ac_power; - int left_fan, right_fan, left_speed, right_speed; + int n, fn_key, cpu_temp, ac_power; + int left_fan, right_fan, left_speed, right_speed; + + cpu_temp = i8k_get_cpu_temp(); /* 11100 ??s */ + left_fan = i8k_get_fan_status(I8K_FAN_LEFT); /* 580 ??s */ + right_fan = i8k_get_fan_status(I8K_FAN_RIGHT); /* 580 ??s */ + left_speed = i8k_get_fan_speed(I8K_FAN_LEFT); /* 580 ??s */ + right_speed = i8k_get_fan_speed(I8K_FAN_RIGHT); /* 580 ??s */ + fn_key = i8k_get_fn_status(); /* 750 ??s */ + if (power_status) { + ac_power = i8k_get_power_status(); /* 14700 ??s */ + } else { + ac_power = -1; + } - cpu_temp = i8k_get_cpu_temp(); /* 11100 ??s */ - left_fan = i8k_get_fan_status(I8K_FAN_LEFT); /* 580 ??s */ - right_fan = i8k_get_fan_status(I8K_FAN_RIGHT); /* 580 ??s */ - left_speed = i8k_get_fan_speed(I8K_FAN_LEFT); /* 580 ??s */ - right_speed = i8k_get_fan_speed(I8K_FAN_RIGHT); /* 580 ??s */ - fn_key = i8k_get_fn_status(); /* 750 ??s */ - if (power_status) { - ac_power = i8k_get_power_status(); /* 14700 ??s */ - } else { - ac_power = -1; - } - - /* - * Info: - * - * 1) Format version (this will change if format changes) - * 2) BIOS version - * 3) BIOS machine ID - * 4) Cpu temperature - * 5) Left fan status - * 6) Right fan status - * 7) Left fan speed - * 8) Right fan speed - * 9) AC power - * 10) Fn Key status - */ - n = sprintf(buffer, "%s %s %s %d %d %d %d %d %d %d\n", - I8K_PROC_FMT, - bios_version, - serial_number, - cpu_temp, - left_fan, - right_fan, - left_speed, - right_speed, - ac_power, - fn_key); - - return n; + /* + * Info: + * + * 1) Format version (this will change if format changes) + * 2) BIOS version + * 3) BIOS machine ID + * 4) Cpu temperature + * 5) Left fan status + * 6) Right fan status + * 7) Left fan speed + * 8) Right fan speed + * 9) AC power + * 10) Fn Key status + */ + n = sprintf(buffer, "%s %s %s %d %d %d %d %d %d %d\n", + I8K_PROC_FMT, + bios_version, + serial_number, + cpu_temp, + left_fan, + right_fan, left_speed, right_speed, ac_power, fn_key); + + return n; } -static ssize_t i8k_read(struct file *f, char __user *buffer, size_t len, loff_t *fpos) +static ssize_t i8k_read(struct file *f, char __user * buffer, size_t len, + loff_t * fpos) { - int n; - char info[128]; + int n; + char info[128]; - n = i8k_get_info(info, NULL, 0, 128); - if (n <= 0) { - return n; - } + n = i8k_get_info(info, NULL, 0, 128); + if (n <= 0) { + return n; + } - if (*fpos >= n) { - return 0; - } + if (*fpos >= n) { + return 0; + } - if ((*fpos + len) >= n) { - len = n - *fpos; - } + if ((*fpos + len) >= n) { + len = n - *fpos; + } - if (copy_to_user(buffer, info, len) != 0) { - return -EFAULT; - } + if (copy_to_user(buffer, info, len) != 0) { + return -EFAULT; + } - *fpos += len; - return len; + *fpos += len; + return len; } -static char* __init string_trim(char *s, int size) +static char *__init string_trim(char *s, int size) { - int len; - char *p; + int len; + char *p; - if ((len = strlen(s)) > size) { - len = size; - } + if ((len = strlen(s)) > size) { + len = size; + } - for (p=s+len-1; len && (*p==' '); len--,p--) { - *p = '\0'; - } + for (p = s + len - 1; len && (*p == ' '); len--, p--) { + *p = '\0'; + } - return s; + return s; } /* DMI code, stolen from arch/i386/kernel/dmi_scan.c */ @@ -515,111 +511,112 @@ static char* __init string_trim(char *s, * | | * +-----------------------+ */ -static char* __init dmi_string(DMIHeader *dmi, u8 s) +static char *__init dmi_string(DMIHeader * dmi, u8 s) { - u8 *p; + u8 *p; - if (!s) { - return ""; - } - s--; - - p = (u8 *)dmi + dmi->length; - while (s > 0) { - p += strlen(p); - p++; + if (!s) { + return ""; + } s--; - } - return p; + p = (u8 *) dmi + dmi->length; + while (s > 0) { + p += strlen(p); + p++; + s--; + } + + return p; } -static void __init dmi_decode(DMIHeader *dmi) +static void __init dmi_decode(DMIHeader * dmi) { - u8 *data = (u8 *) dmi; - char *p; + u8 *data = (u8 *) dmi; + char *p; #ifdef I8K_DEBUG - int i; - printk("%08x ", (int)data); - for (i=0; i<data[1] && i<64; i++) { - printk("%02x ", data[i]); - } - printk("\n"); + int i; + printk("%08x ", (int)data); + for (i = 0; i < data[1] && i < 64; i++) { + printk("%02x ", data[i]); + } + printk("\n"); #endif - switch (dmi->type) { - case 0: /* BIOS Information */ - p = dmi_string(dmi,data[5]); - if (*p) { - strlcpy(bios_version, p, sizeof(bios_version)); - string_trim(bios_version, sizeof(bios_version)); - } - break; - case 1: /* System Information */ - p = dmi_string(dmi,data[4]); - if (*p) { - strlcpy(system_vendor, p, sizeof(system_vendor)); - string_trim(system_vendor, sizeof(system_vendor)); - } - p = dmi_string(dmi,data[5]); - if (*p) { - strlcpy(product_name, p, sizeof(product_name)); - string_trim(product_name, sizeof(product_name)); - } - p = dmi_string(dmi,data[7]); - if (*p) { - strlcpy(serial_number, p, sizeof(serial_number)); - string_trim(serial_number, sizeof(serial_number)); - } - break; - } -} - -static int __init dmi_table(u32 base, int len, int num, void (*fn)(DMIHeader*)) -{ - u8 *buf; - u8 *data; - DMIHeader *dmi; - int i = 1; + switch (dmi->type) { + case 0: /* BIOS Information */ + p = dmi_string(dmi, data[5]); + if (*p) { + strlcpy(bios_version, p, sizeof(bios_version)); + string_trim(bios_version, sizeof(bios_version)); + } + break; + case 1: /* System Information */ + p = dmi_string(dmi, data[4]); + if (*p) { + strlcpy(system_vendor, p, sizeof(system_vendor)); + string_trim(system_vendor, sizeof(system_vendor)); + } + p = dmi_string(dmi, data[5]); + if (*p) { + strlcpy(product_name, p, sizeof(product_name)); + string_trim(product_name, sizeof(product_name)); + } + p = dmi_string(dmi, data[7]); + if (*p) { + strlcpy(serial_number, p, sizeof(serial_number)); + string_trim(serial_number, sizeof(serial_number)); + } + break; + } +} - buf = ioremap(base, len); - if (buf == NULL) { - return -1; - } - data = buf; +static int __init dmi_table(u32 base, int len, int num, + void (*fn) (DMIHeader *)) +{ + u8 *buf; + u8 *data; + DMIHeader *dmi; + int i = 1; - /* - * Stop when we see al the items the table claimed to have - * or we run off the end of the table (also happens) - */ - while ((i<num) && ((data-buf) < len)) { - dmi = (DMIHeader *)data; - /* - * Avoid misparsing crud if the length of the last - * record is crap - */ - if ((data-buf+dmi->length) >= len) { - break; + buf = ioremap(base, len); + if (buf == NULL) { + return -1; } - fn(dmi); - data += dmi->length; + data = buf; + /* - * Don't go off the end of the data if there is - * stuff looking like string fill past the end + * Stop when we see al the items the table claimed to have + * or we run off the end of the table (also happens) */ - while (((data-buf) < len) && (*data || data[1])) { - data++; + while ((i < num) && ((data - buf) < len)) { + dmi = (DMIHeader *) data; + /* + * Avoid misparsing crud if the length of the last + * record is crap + */ + if ((data - buf + dmi->length) >= len) { + break; + } + fn(dmi); + data += dmi->length; + /* + * Don't go off the end of the data if there is + * stuff looking like string fill past the end + */ + while (((data - buf) < len) && (*data || data[1])) { + data++; + } + data += 2; + i++; } - data += 2; - i++; - } - iounmap(buf); + iounmap(buf); - return 0; + return 0; } -static int __init dmi_iterate(void (*decode)(DMIHeader *)) +static int __init dmi_iterate(void (*decode) (DMIHeader *)) { unsigned char buf[20]; void __iomem *p = ioremap(0xe0000, 0x20000), *q; @@ -629,20 +626,20 @@ static int __init dmi_iterate(void (*dec for (q = p; q < p + 0x20000; q += 16) { memcpy_fromio(buf, q, 20); - if (memcmp(buf, "_DMI_", 5)==0) { - u16 num = buf[13]<<8 | buf[12]; - u16 len = buf [7]<<8 | buf [6]; - u32 base = buf[11]<<24 | buf[10]<<16 | buf[9]<<8 | buf[8]; + if (memcmp(buf, "_DMI_", 5) == 0) { + u16 num = buf[13] << 8 | buf[12]; + u16 len = buf[7] << 8 | buf[6]; + u32 base = buf[11] << 24 | buf[10] << 16 | buf[9] << 8 | buf[8]; #ifdef I8K_DEBUG printk(KERN_INFO "DMI %d.%d present.\n", - buf[14]>>4, buf[14]&0x0F); + buf[14] >> 4, buf[14] & 0x0F); printk(KERN_INFO "%d structures occupying %d bytes.\n", - buf[13]<<8 | buf[12], - buf [7]<<8 | buf[6]); + buf[13] << 8 | buf[12], buf[7] << 8 | buf[6]); printk(KERN_INFO "DMI table at 0x%08X.\n", - buf[11]<<24 | buf[10]<<16 | buf[9]<<8 | buf[8]); + buf[11] << 24 | buf[10] << 16 | buf[9] << 8 | + buf[8]); #endif - if (dmi_table(base, len, num, decode)==0) { + if (dmi_table(base, len, num, decode) == 0) { iounmap(p); return 0; } @@ -651,6 +648,7 @@ static int __init dmi_iterate(void (*dec iounmap(p); return -1; } + /* end of DMI code */ /* @@ -658,29 +656,30 @@ static int __init dmi_iterate(void (*dec */ static int __init i8k_dmi_probe(void) { - char **p; - - if (dmi_iterate(dmi_decode) != 0) { - printk(KERN_INFO "i8k: unable to get DMI information\n"); - return -ENODEV; - } + char **p; - if (strncmp(system_vendor,DELL_SIGNATURE,strlen(DELL_SIGNATURE)) != 0) { - printk(KERN_INFO "i8k: not running on a Dell system\n"); - return -ENODEV; - } + if (dmi_iterate(dmi_decode) != 0) { + printk(KERN_INFO "i8k: unable to get DMI information\n"); + return -ENODEV; + } - for (p=supported_models; ; p++) { - if (!*p) { - printk(KERN_INFO "i8k: unsupported model: %s\n", product_name); - return -ENODEV; + if (strncmp(system_vendor, DELL_SIGNATURE, strlen(DELL_SIGNATURE)) != 0) { + printk(KERN_INFO "i8k: not running on a Dell system\n"); + return -ENODEV; } - if (strncmp(product_name,*p,strlen(*p)) == 0) { - break; + + for (p = supported_models;; p++) { + if (!*p) { + printk(KERN_INFO "i8k: unsupported model: %s\n", + product_name); + return -ENODEV; + } + if (strncmp(product_name, *p, strlen(*p)) == 0) { + break; + } } - } - return 0; + return 0; } /* @@ -688,59 +687,60 @@ static int __init i8k_dmi_probe(void) */ static int __init i8k_probe(void) { - char buff[4]; - int version; - int smm_found = 0; - - /* - * Get DMI information - */ - if (i8k_dmi_probe() != 0) { - printk(KERN_INFO "i8k: vendor=%s, model=%s, version=%s\n", - system_vendor, product_name, bios_version); - } - - /* - * Get SMM Dell signature - */ - if (i8k_get_dell_signature() != 0) { - printk(KERN_INFO "i8k: unable to get SMM Dell signature\n"); - } else { - smm_found = 1; - } - - /* - * Get SMM BIOS version. - */ - version = i8k_get_bios_version(); - if (version <= 0) { - printk(KERN_INFO "i8k: unable to get SMM BIOS version\n"); - } else { - smm_found = 1; - buff[0] = (version >> 16) & 0xff; - buff[1] = (version >> 8) & 0xff; - buff[2] = (version) & 0xff; - buff[3] = '\0'; + char buff[4]; + int version; + int smm_found = 0; + /* - * If DMI BIOS version is unknown use SMM BIOS version. + * Get DMI information */ - if (bios_version[0] == '?') { - strcpy(bios_version, buff); + if (i8k_dmi_probe() != 0) { + printk(KERN_INFO "i8k: vendor=%s, model=%s, version=%s\n", + system_vendor, product_name, bios_version); } + /* - * Check if the two versions match. + * Get SMM Dell signature */ - if (strncmp(buff,bios_version,sizeof(bios_version)) != 0) { - printk(KERN_INFO "i8k: BIOS version mismatch: %s != %s\n", - buff, bios_version); + if (i8k_get_dell_signature() != 0) { + printk(KERN_INFO "i8k: unable to get SMM Dell signature\n"); + } else { + smm_found = 1; } - } - if (!smm_found && !force) { - return -ENODEV; - } + /* + * Get SMM BIOS version. + */ + version = i8k_get_bios_version(); + if (version <= 0) { + printk(KERN_INFO "i8k: unable to get SMM BIOS version\n"); + } else { + smm_found = 1; + buff[0] = (version >> 16) & 0xff; + buff[1] = (version >> 8) & 0xff; + buff[2] = (version) & 0xff; + buff[3] = '\0'; + /* + * If DMI BIOS version is unknown use SMM BIOS version. + */ + if (bios_version[0] == '?') { + strcpy(bios_version, buff); + } + /* + * Check if the two versions match. + */ + if (strncmp(buff, bios_version, sizeof(bios_version)) != 0) { + printk(KERN_INFO + "i8k: BIOS version mismatch: %s != %s\n", buff, + bios_version); + } + } - return 0; + if (!smm_found && !force) { + return -ENODEV; + } + + return 0; } #ifdef MODULE @@ -748,40 +748,40 @@ static #endif int __init i8k_init(void) { - struct proc_dir_entry *proc_i8k; + struct proc_dir_entry *proc_i8k; - /* Are we running on an supported laptop? */ - if (i8k_probe() != 0) { - return -ENODEV; - } - - /* Register the proc entry */ - proc_i8k = create_proc_info_entry("i8k", 0, NULL, i8k_get_info); - if (!proc_i8k) { - return -ENOENT; - } - proc_i8k->proc_fops = &i8k_fops; - proc_i8k->owner = THIS_MODULE; - - printk(KERN_INFO - "Dell laptop SMM driver v%s Massimo Dal Zotto (dz@debian.org)\n", - I8K_VERSION); + /* Are we running on an supported laptop? */ + if (i8k_probe() != 0) { + return -ENODEV; + } + + /* Register the proc entry */ + proc_i8k = create_proc_info_entry("i8k", 0, NULL, i8k_get_info); + if (!proc_i8k) { + return -ENOENT; + } + proc_i8k->proc_fops = &i8k_fops; + proc_i8k->owner = THIS_MODULE; - return 0; + printk(KERN_INFO + "Dell laptop SMM driver v%s Massimo Dal Zotto (dz@debian.org)\n", + I8K_VERSION); + + return 0; } #ifdef MODULE int init_module(void) { - return i8k_init(); + return i8k_init(); } void cleanup_module(void) { - /* Remove the proc entry */ - remove_proc_entry("i8k", NULL); + /* Remove the proc entry */ + remove_proc_entry("i8k", NULL); - printk(KERN_INFO "i8k: module unloaded\n"); + printk(KERN_INFO "i8k: module unloaded\n"); } #endif ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/5] I8K - use standard DMI functions 2005-02-24 6:11 ` [PATCH 1/5] I8K - pass though Lindent Dmitry Torokhov @ 2005-02-24 6:12 ` Dmitry Torokhov 2005-02-24 6:12 ` [PATCH 3/5] I8K - switch to seq_file Dmitry Torokhov 0 siblings, 1 reply; 28+ messages in thread From: Dmitry Torokhov @ 2005-02-24 6:12 UTC (permalink / raw) To: LKML; +Cc: Massimo Dal Zotto =================================================================== I8K: Change to use stock dmi infrastructure instead of homegrown parsing code. The driver now requres box's DMI data to match list of supported models so driver can be safely compiled-in by default without fear of it poking into random SMM BIOS code. DMI checks can be ignored with i8k.ignore_dmi option. Signed-off-by: Dmitry Torokhov <dtor@mail.ru> Documentation/kernel-parameters.txt | 3 arch/i386/kernel/dmi_scan.c | 1 drivers/char/i8k.c | 304 ++++++------------------------------ include/linux/dmi.h | 1 4 files changed, 60 insertions(+), 249 deletions(-) Index: dtor/arch/i386/kernel/dmi_scan.c =================================================================== --- dtor.orig/arch/i386/kernel/dmi_scan.c +++ dtor/arch/i386/kernel/dmi_scan.c @@ -416,6 +416,7 @@ static void __init dmi_decode(struct dmi dmi_save_ident(dm, DMI_PRODUCT_VERSION, 6); dmi_printk(("Serial Number: %s\n", dmi_string(dm, data[7]))); + dmi_save_ident(dm, DMI_PRODUCT_SERIAL, 7); break; case 2: dmi_printk(("Board Vendor: %s\n", Index: dtor/include/linux/dmi.h =================================================================== --- dtor.orig/include/linux/dmi.h +++ dtor/include/linux/dmi.h @@ -9,6 +9,7 @@ enum dmi_field { DMI_SYS_VENDOR, DMI_PRODUCT_NAME, DMI_PRODUCT_VERSION, + DMI_PRODUCT_SERIAL, DMI_BOARD_VENDOR, DMI_BOARD_NAME, DMI_BOARD_VERSION, Index: dtor/drivers/char/i8k.c =================================================================== --- dtor.orig/drivers/char/i8k.c +++ dtor/drivers/char/i8k.c @@ -20,7 +20,7 @@ #include <linux/types.h> #include <linux/init.h> #include <linux/proc_fs.h> -#include <linux/apm_bios.h> +#include <linux/dmi.h> #include <asm/uaccess.h> #include <asm/io.h> @@ -52,18 +52,7 @@ #define I8K_TEMPERATURE_BUG 1 -#define DELL_SIGNATURE "Dell Computer" - -static char *supported_models[] = { - "Inspiron", - "Latitude", - NULL -}; - -static char system_vendor[48] = "?"; -static char product_name[48] = "?"; -static char bios_version[4] = "?"; -static char serial_number[16] = "?"; +static char bios_version[4]; MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)"); MODULE_DESCRIPTION("Driver for accessing SMM BIOS on Dell laptops"); @@ -73,6 +62,10 @@ static int force; module_param(force, bool, 0); MODULE_PARM_DESC(force, "Force loading without checking for supported models"); +static int ignore_dmi; +module_param(ignore_dmi, bool, 0); +MODULE_PARM_DESC(ignore_dmi, "Continue probing hardware even if DMI data does not match"); + static int restricted; module_param(restricted, bool, 0); MODULE_PARM_DESC(restricted, "Allow fan control if SYS_ADMIN capability set"); @@ -99,11 +92,10 @@ typedef struct { unsigned int edi __attribute__ ((packed)); } SMMRegisters; -typedef struct { - u8 type; - u8 length; - u16 handle; -} DMIHeader; +static inline char *i8k_get_dmi_data(int field) +{ + return dmi_get_system_info(field) ? : "N/A"; +} /* * Call the System Management Mode BIOS. Code provided by Jonathan Buzzard. @@ -163,15 +155,6 @@ static int i8k_get_bios_version(void) } /* - * Read the machine id. - */ -static int i8k_get_serial_number(unsigned char *buff) -{ - strlcpy(buff, serial_number, sizeof(serial_number)); - return 0; -} - -/* * Read the Fn key status. */ static int i8k_get_fn_status(void) @@ -328,7 +311,7 @@ static int i8k_get_dell_signature(void) static int i8k_ioctl(struct inode *ip, struct file *fp, unsigned int cmd, unsigned long arg) { - int val; + int val = 0; int speed; unsigned char buff[16]; int __user *argp = (int __user *)arg; @@ -343,7 +326,7 @@ static int i8k_ioctl(struct inode *ip, s case I8K_MACHINE_ID: memset(buff, 0, 16); - val = i8k_get_serial_number(buff); + strlcpy(buff, i8k_get_dmi_data(DMI_PRODUCT_SERIAL), sizeof(buff)); break; case I8K_FN_STATUS: @@ -451,10 +434,10 @@ static int i8k_get_info(char *buffer, ch n = sprintf(buffer, "%s %s %s %d %d %d %d %d %d %d\n", I8K_PROC_FMT, bios_version, - serial_number, + dmi_get_system_info(DMI_PRODUCT_SERIAL) ? : "N/A", cpu_temp, - left_fan, - right_fan, left_speed, right_speed, ac_power, fn_key); + left_fan, right_fan, left_speed, right_speed, + ac_power, fn_key); return n; } @@ -486,201 +469,23 @@ static ssize_t i8k_read(struct file *f, return len; } -static char *__init string_trim(char *s, int size) -{ - int len; - char *p; - - if ((len = strlen(s)) > size) { - len = size; - } - - for (p = s + len - 1; len && (*p == ' '); len--, p--) { - *p = '\0'; - } - - return s; -} - -/* DMI code, stolen from arch/i386/kernel/dmi_scan.c */ - -/* - * |<-- dmi->length -->| - * | | - * |dmi header s=N | string1,\0, ..., stringN,\0, ..., \0 - * | | - * +-----------------------+ - */ -static char *__init dmi_string(DMIHeader * dmi, u8 s) -{ - u8 *p; - - if (!s) { - return ""; - } - s--; - - p = (u8 *) dmi + dmi->length; - while (s > 0) { - p += strlen(p); - p++; - s--; - } - - return p; -} - -static void __init dmi_decode(DMIHeader * dmi) -{ - u8 *data = (u8 *) dmi; - char *p; - -#ifdef I8K_DEBUG - int i; - printk("%08x ", (int)data); - for (i = 0; i < data[1] && i < 64; i++) { - printk("%02x ", data[i]); - } - printk("\n"); -#endif - - switch (dmi->type) { - case 0: /* BIOS Information */ - p = dmi_string(dmi, data[5]); - if (*p) { - strlcpy(bios_version, p, sizeof(bios_version)); - string_trim(bios_version, sizeof(bios_version)); - } - break; - case 1: /* System Information */ - p = dmi_string(dmi, data[4]); - if (*p) { - strlcpy(system_vendor, p, sizeof(system_vendor)); - string_trim(system_vendor, sizeof(system_vendor)); - } - p = dmi_string(dmi, data[5]); - if (*p) { - strlcpy(product_name, p, sizeof(product_name)); - string_trim(product_name, sizeof(product_name)); - } - p = dmi_string(dmi, data[7]); - if (*p) { - strlcpy(serial_number, p, sizeof(serial_number)); - string_trim(serial_number, sizeof(serial_number)); - } - break; - } -} - -static int __init dmi_table(u32 base, int len, int num, - void (*fn) (DMIHeader *)) -{ - u8 *buf; - u8 *data; - DMIHeader *dmi; - int i = 1; - - buf = ioremap(base, len); - if (buf == NULL) { - return -1; - } - data = buf; - - /* - * Stop when we see al the items the table claimed to have - * or we run off the end of the table (also happens) - */ - while ((i < num) && ((data - buf) < len)) { - dmi = (DMIHeader *) data; - /* - * Avoid misparsing crud if the length of the last - * record is crap - */ - if ((data - buf + dmi->length) >= len) { - break; - } - fn(dmi); - data += dmi->length; - /* - * Don't go off the end of the data if there is - * stuff looking like string fill past the end - */ - while (((data - buf) < len) && (*data || data[1])) { - data++; - } - data += 2; - i++; - } - iounmap(buf); - - return 0; -} - -static int __init dmi_iterate(void (*decode) (DMIHeader *)) -{ - unsigned char buf[20]; - void __iomem *p = ioremap(0xe0000, 0x20000), *q; - - if (!p) - return -1; - - for (q = p; q < p + 0x20000; q += 16) { - memcpy_fromio(buf, q, 20); - if (memcmp(buf, "_DMI_", 5) == 0) { - u16 num = buf[13] << 8 | buf[12]; - u16 len = buf[7] << 8 | buf[6]; - u32 base = buf[11] << 24 | buf[10] << 16 | buf[9] << 8 | buf[8]; -#ifdef I8K_DEBUG - printk(KERN_INFO "DMI %d.%d present.\n", - buf[14] >> 4, buf[14] & 0x0F); - printk(KERN_INFO "%d structures occupying %d bytes.\n", - buf[13] << 8 | buf[12], buf[7] << 8 | buf[6]); - printk(KERN_INFO "DMI table at 0x%08X.\n", - buf[11] << 24 | buf[10] << 16 | buf[9] << 8 | - buf[8]); -#endif - if (dmi_table(base, len, num, decode) == 0) { - iounmap(p); - return 0; - } - } - } - iounmap(p); - return -1; -} - -/* end of DMI code */ - -/* - * Get DMI information. - */ -static int __init i8k_dmi_probe(void) -{ - char **p; - - if (dmi_iterate(dmi_decode) != 0) { - printk(KERN_INFO "i8k: unable to get DMI information\n"); - return -ENODEV; - } - - if (strncmp(system_vendor, DELL_SIGNATURE, strlen(DELL_SIGNATURE)) != 0) { - printk(KERN_INFO "i8k: not running on a Dell system\n"); - return -ENODEV; - } - - for (p = supported_models;; p++) { - if (!*p) { - printk(KERN_INFO "i8k: unsupported model: %s\n", - product_name); - return -ENODEV; - } - if (strncmp(product_name, *p, strlen(*p)) == 0) { - break; - } - } - - return 0; -} +static struct dmi_system_id __initdata i8k_dmi_table[] = { + { + .ident = "Dell Inspiron", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer"), + DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron"), + }, + }, + { + .ident = "Dell Latitude", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer"), + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude"), + }, + }, + { } +}; /* * Probe for the presence of a supported laptop. @@ -689,23 +494,30 @@ static int __init i8k_probe(void) { char buff[4]; int version; - int smm_found = 0; /* * Get DMI information */ - if (i8k_dmi_probe() != 0) { + if (!dmi_check_system(i8k_dmi_table)) { + if (!ignore_dmi && !force) + return -ENODEV; + + printk(KERN_INFO "i8k: not running on a supported Dell system.\n"); printk(KERN_INFO "i8k: vendor=%s, model=%s, version=%s\n", - system_vendor, product_name, bios_version); + i8k_get_dmi_data(DMI_SYS_VENDOR), + i8k_get_dmi_data(DMI_PRODUCT_NAME), + i8k_get_dmi_data(DMI_BIOS_VERSION)); } + strlcpy(bios_version, i8k_get_dmi_data(DMI_BIOS_VERSION), sizeof(bios_version)); + /* * Get SMM Dell signature */ if (i8k_get_dell_signature() != 0) { - printk(KERN_INFO "i8k: unable to get SMM Dell signature\n"); - } else { - smm_found = 1; + printk(KERN_ERR "i8k: unable to get SMM Dell signature\n"); + if (!force) + return -ENODEV; } /* @@ -713,9 +525,10 @@ static int __init i8k_probe(void) */ version = i8k_get_bios_version(); if (version <= 0) { - printk(KERN_INFO "i8k: unable to get SMM BIOS version\n"); + printk(KERN_ERR "i8k: unable to get SMM BIOS version\n"); + if (!force) + return -ENODEV; } else { - smm_found = 1; buff[0] = (version >> 16) & 0xff; buff[1] = (version >> 8) & 0xff; buff[2] = (version) & 0xff; @@ -723,21 +536,15 @@ static int __init i8k_probe(void) /* * If DMI BIOS version is unknown use SMM BIOS version. */ - if (bios_version[0] == '?') { - strcpy(bios_version, buff); - } + if (!dmi_get_system_info(DMI_BIOS_VERSION)) + strlcpy(bios_version, buff, sizeof(bios_version)); + /* * Check if the two versions match. */ - if (strncmp(buff, bios_version, sizeof(bios_version)) != 0) { - printk(KERN_INFO - "i8k: BIOS version mismatch: %s != %s\n", buff, - bios_version); - } - } - - if (!smm_found && !force) { - return -ENODEV; + if (strncmp(buff, bios_version, sizeof(bios_version)) != 0) + printk(KERN_WARNING "i8k: BIOS version mismatch: %s != %s\n", + buff, bios_version); } return 0; @@ -751,9 +558,8 @@ int __init i8k_init(void) struct proc_dir_entry *proc_i8k; /* Are we running on an supported laptop? */ - if (i8k_probe() != 0) { + if (i8k_probe()) return -ENODEV; - } /* Register the proc entry */ proc_i8k = create_proc_info_entry("i8k", 0, NULL, i8k_get_info); Index: dtor/Documentation/kernel-parameters.txt =================================================================== --- dtor.orig/Documentation/kernel-parameters.txt +++ dtor/Documentation/kernel-parameters.txt @@ -528,6 +528,9 @@ running once the system is up. i810= [HW,DRM] + i8k.ignore_dmi [HW] Continue probing hardware even if DMI data + indicates that the driver is running on unsupported + hardware. i8k.force [HW] Activate i8k driver even if SMM BIOS signature does not match list of supported models. i8k.power_status ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/5] I8K - switch to seq_file 2005-02-24 6:12 ` [PATCH 2/5] I8K - use standard DMI functions Dmitry Torokhov @ 2005-02-24 6:12 ` Dmitry Torokhov 2005-02-24 6:14 ` [PATCH 4/5] I8K - switch to module_{init|exit} Dmitry Torokhov 0 siblings, 1 reply; 28+ messages in thread From: Dmitry Torokhov @ 2005-02-24 6:12 UTC (permalink / raw) To: LKML; +Cc: Massimo Dal Zotto =================================================================== I8K: Change proc code to use seq_file. Signed-off-by: Dmitry Torokhov <dtor@mail.ru> i8k.c | 64 ++++++++++++++++++++++------------------------------------------ 1 files changed, 22 insertions(+), 42 deletions(-) Index: dtor/drivers/char/i8k.c =================================================================== --- dtor.orig/drivers/char/i8k.c +++ dtor/drivers/char/i8k.c @@ -20,13 +20,14 @@ #include <linux/types.h> #include <linux/init.h> #include <linux/proc_fs.h> +#include <linux/seq_file.h> #include <linux/dmi.h> #include <asm/uaccess.h> #include <asm/io.h> #include <linux/i8k.h> -#define I8K_VERSION "1.13 14/05/2002" +#define I8K_VERSION "1.14 21/02/2005" #define I8K_SMM_FN_STATUS 0x0025 #define I8K_SMM_POWER_STATUS 0x0069 @@ -74,13 +75,16 @@ static int power_status; module_param(power_status, bool, 0600); MODULE_PARM_DESC(power_status, "Report power status in /proc/i8k"); -static ssize_t i8k_read(struct file *, char __user *, size_t, loff_t *); +static int i8k_open_fs(struct inode *inode, struct file *file); static int i8k_ioctl(struct inode *, struct file *, unsigned int, unsigned long); static struct file_operations i8k_fops = { - .read = i8k_read, - .ioctl = i8k_ioctl, + .open = i8k_open_fs, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, + .ioctl = i8k_ioctl, }; typedef struct { @@ -400,9 +404,9 @@ static int i8k_ioctl(struct inode *ip, s /* * Print the information for /proc/i8k. */ -static int i8k_get_info(char *buffer, char **start, off_t fpos, int length) +static int i8k_proc_show(struct seq_file *seq, void *offset) { - int n, fn_key, cpu_temp, ac_power; + int fn_key, cpu_temp, ac_power; int left_fan, right_fan, left_speed, right_speed; cpu_temp = i8k_get_cpu_temp(); /* 11100 ??s */ @@ -431,42 +435,18 @@ static int i8k_get_info(char *buffer, ch * 9) AC power * 10) Fn Key status */ - n = sprintf(buffer, "%s %s %s %d %d %d %d %d %d %d\n", - I8K_PROC_FMT, - bios_version, - dmi_get_system_info(DMI_PRODUCT_SERIAL) ? : "N/A", - cpu_temp, - left_fan, right_fan, left_speed, right_speed, - ac_power, fn_key); - - return n; + return seq_printf(seq, "%s %s %s %d %d %d %d %d %d %d\n", + I8K_PROC_FMT, + bios_version, + dmi_get_system_info(DMI_PRODUCT_SERIAL) ? : "N/A", + cpu_temp, + left_fan, right_fan, left_speed, right_speed, + ac_power, fn_key); } -static ssize_t i8k_read(struct file *f, char __user * buffer, size_t len, - loff_t * fpos) +static int i8k_open_fs(struct inode *inode, struct file *file) { - int n; - char info[128]; - - n = i8k_get_info(info, NULL, 0, 128); - if (n <= 0) { - return n; - } - - if (*fpos >= n) { - return 0; - } - - if ((*fpos + len) >= n) { - len = n - *fpos; - } - - if (copy_to_user(buffer, info, len) != 0) { - return -EFAULT; - } - - *fpos += len; - return len; + return single_open(file, i8k_proc_show, NULL); } static struct dmi_system_id __initdata i8k_dmi_table[] = { @@ -562,10 +542,10 @@ int __init i8k_init(void) return -ENODEV; /* Register the proc entry */ - proc_i8k = create_proc_info_entry("i8k", 0, NULL, i8k_get_info); - if (!proc_i8k) { + proc_i8k = create_proc_entry("i8k", 0, NULL); + if (!proc_i8k) return -ENOENT; - } + proc_i8k->proc_fops = &i8k_fops; proc_i8k->owner = THIS_MODULE; ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 4/5] I8K - switch to module_{init|exit} 2005-02-24 6:12 ` [PATCH 3/5] I8K - switch to seq_file Dmitry Torokhov @ 2005-02-24 6:14 ` Dmitry Torokhov 2005-02-24 6:14 ` [PATCH 5/5] I8K - convert to platform device (sysfs) Dmitry Torokhov 0 siblings, 1 reply; 28+ messages in thread From: Dmitry Torokhov @ 2005-02-24 6:14 UTC (permalink / raw) To: LKML; +Cc: Massimo Dal Zotto =================================================================== I8K: use module_{init|exit} instead of old style #ifdef MODULE code, some formatting changes. Signed-off-by: Dmitry Torokhov <dtor@mail.ru> i8k.c | 149 ++++++++++++++++++++--------------------------------------------- misc.c | 4 - 2 files changed, 47 insertions(+), 106 deletions(-) Index: dtor/drivers/char/misc.c =================================================================== --- dtor.orig/drivers/char/misc.c +++ dtor/drivers/char/misc.c @@ -67,7 +67,6 @@ extern int rtc_DP8570A_init(void); extern int rtc_MK48T08_init(void); extern int pmu_device_init(void); extern int tosh_init(void); -extern int i8k_init(void); #ifdef CONFIG_PROC_FS static void *misc_seq_start(struct seq_file *seq, loff_t *pos) @@ -317,9 +316,6 @@ static int __init misc_init(void) #ifdef CONFIG_TOSHIBA tosh_init(); #endif -#ifdef CONFIG_I8K - i8k_init(); -#endif if (register_chrdev(MISC_MAJOR,"misc",&misc_fops)) { printk("unable to get major %d for misc devices\n", MISC_MAJOR); Index: dtor/drivers/char/i8k.c =================================================================== --- dtor.orig/drivers/char/i8k.c +++ dtor/drivers/char/i8k.c @@ -87,14 +87,14 @@ static struct file_operations i8k_fops = .ioctl = i8k_ioctl, }; -typedef struct { +struct smm_regs { unsigned int eax; unsigned int ebx __attribute__ ((packed)); unsigned int ecx __attribute__ ((packed)); unsigned int edx __attribute__ ((packed)); unsigned int esi __attribute__ ((packed)); unsigned int edi __attribute__ ((packed)); -} SMMRegisters; +}; static inline char *i8k_get_dmi_data(int field) { @@ -104,7 +104,7 @@ static inline char *i8k_get_dmi_data(int /* * Call the System Management Mode BIOS. Code provided by Jonathan Buzzard. */ -static int i8k_smm(SMMRegisters * regs) +static int i8k_smm(struct smm_regs *regs) { int rc; int eax = regs->eax; @@ -134,9 +134,8 @@ static int i8k_smm(SMMRegisters * regs) : "a"(regs) : "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory"); - if ((rc != 0) || ((regs->eax & 0xffff) == 0xffff) || (regs->eax == eax)) { + if (rc != 0 || (regs->eax & 0xffff) == 0xffff || regs->eax == eax) return -EINVAL; - } return 0; } @@ -147,15 +146,9 @@ static int i8k_smm(SMMRegisters * regs) */ static int i8k_get_bios_version(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; - - regs.eax = I8K_SMM_BIOS_VERSION; - if ((rc = i8k_smm(®s)) < 0) { - return rc; - } + struct smm_regs regs = { .eax = I8K_SMM_BIOS_VERSION, }; - return regs.eax; + return i8k_smm(®s) < 0 ? : regs.eax; } /* @@ -163,13 +156,11 @@ static int i8k_get_bios_version(void) */ static int i8k_get_fn_status(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; + struct smm_regs regs = { .eax = I8K_SMM_FN_STATUS, }; int rc; - regs.eax = I8K_SMM_FN_STATUS; - if ((rc = i8k_smm(®s)) < 0) { + if ((rc = i8k_smm(®s)) < 0) return rc; - } switch ((regs.eax >> I8K_FN_SHIFT) & I8K_FN_MASK) { case I8K_FN_UP: @@ -188,20 +179,13 @@ static int i8k_get_fn_status(void) */ static int i8k_get_power_status(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; + struct smm_regs regs = { .eax = I8K_SMM_POWER_STATUS, }; int rc; - regs.eax = I8K_SMM_POWER_STATUS; - if ((rc = i8k_smm(®s)) < 0) { + if ((rc = i8k_smm(®s)) < 0) return rc; - } - switch (regs.eax & 0xff) { - case I8K_POWER_AC: - return I8K_AC; - default: - return I8K_BATTERY; - } + return (regs.eax & 0xff) == I8K_POWER_AC ? I8K_AC : I8K_BATTERY; } /* @@ -209,16 +193,10 @@ static int i8k_get_power_status(void) */ static int i8k_get_fan_status(int fan) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + struct smm_regs regs = { .eax = I8K_SMM_GET_FAN, }; - regs.eax = I8K_SMM_GET_FAN; regs.ebx = fan & 0xff; - if ((rc = i8k_smm(®s)) < 0) { - return rc; - } - - return (regs.eax & 0xff); + return i8k_smm(®s) < 0 ? : regs.eax & 0xff; } /* @@ -226,16 +204,10 @@ static int i8k_get_fan_status(int fan) */ static int i8k_get_fan_speed(int fan) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + struct smm_regs regs = { .eax = I8K_SMM_GET_SPEED, }; - regs.eax = I8K_SMM_GET_SPEED; regs.ebx = fan & 0xff; - if ((rc = i8k_smm(®s)) < 0) { - return rc; - } - - return (regs.eax & 0xffff) * I8K_FAN_MULT; + return i8k_smm(®s) < 0 ? : (regs.eax & 0xffff) * I8K_FAN_MULT; } /* @@ -243,18 +215,12 @@ static int i8k_get_fan_speed(int fan) */ static int i8k_set_fan(int fan, int speed) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + struct smm_regs regs = { .eax = I8K_SMM_SET_FAN, }; speed = (speed < 0) ? 0 : ((speed > I8K_FAN_MAX) ? I8K_FAN_MAX : speed); - - regs.eax = I8K_SMM_SET_FAN; regs.ebx = (fan & 0xff) | (speed << 8); - if ((rc = i8k_smm(®s)) < 0) { - return rc; - } - return (i8k_get_fan_status(fan)); + return i8k_smm(®s) < 0 ? : i8k_get_fan_status(fan); } /* @@ -262,18 +228,17 @@ static int i8k_set_fan(int fan, int spee */ static int i8k_get_cpu_temp(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; + struct smm_regs regs = { .eax = I8K_SMM_GET_TEMP, }; int rc; int temp; #ifdef I8K_TEMPERATURE_BUG - static int prev = 0; + static int prev; #endif - regs.eax = I8K_SMM_GET_TEMP; - if ((rc = i8k_smm(®s)) < 0) { + if ((rc = i8k_smm(®s)) < 0) return rc; - } + temp = regs.eax & 0xff; #ifdef I8K_TEMPERATURE_BUG @@ -297,19 +262,13 @@ static int i8k_get_cpu_temp(void) static int i8k_get_dell_signature(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; + struct smm_regs regs = { .eax = I8K_SMM_GET_DELL_SIG, }; int rc; - regs.eax = I8K_SMM_GET_DELL_SIG; - if ((rc = i8k_smm(®s)) < 0) { + if ((rc = i8k_smm(®s)) < 0) return rc; - } - if ((regs.eax == 1145651527) && (regs.edx == 1145392204)) { - return 0; - } else { - return -1; - } + return regs.eax == 1145651527 && regs.edx == 1145392204 ? 0 : -1; } static int i8k_ioctl(struct inode *ip, struct file *fp, unsigned int cmd, @@ -346,29 +305,29 @@ static int i8k_ioctl(struct inode *ip, s break; case I8K_GET_SPEED: - if (copy_from_user(&val, argp, sizeof(int))) { + if (copy_from_user(&val, argp, sizeof(int))) return -EFAULT; - } + val = i8k_get_fan_speed(val); break; case I8K_GET_FAN: - if (copy_from_user(&val, argp, sizeof(int))) { + if (copy_from_user(&val, argp, sizeof(int))) return -EFAULT; - } + val = i8k_get_fan_status(val); break; case I8K_SET_FAN: - if (restricted && !capable(CAP_SYS_ADMIN)) { + if (restricted && !capable(CAP_SYS_ADMIN)) return -EPERM; - } - if (copy_from_user(&val, argp, sizeof(int))) { + + if (copy_from_user(&val, argp, sizeof(int))) return -EFAULT; - } - if (copy_from_user(&speed, argp + 1, sizeof(int))) { + + if (copy_from_user(&speed, argp + 1, sizeof(int))) return -EFAULT; - } + val = i8k_set_fan(val, speed); break; @@ -376,25 +335,24 @@ static int i8k_ioctl(struct inode *ip, s return -EINVAL; } - if (val < 0) { + if (val < 0) return val; - } switch (cmd) { case I8K_BIOS_VERSION: - if (copy_to_user(argp, &val, 4)) { + if (copy_to_user(argp, &val, 4)) return -EFAULT; - } + break; case I8K_MACHINE_ID: - if (copy_to_user(argp, buff, 16)) { + if (copy_to_user(argp, buff, 16)) return -EFAULT; - } + break; default: - if (copy_to_user(argp, &val, sizeof(int))) { + if (copy_to_user(argp, &val, sizeof(int))) return -EFAULT; - } + break; } @@ -415,11 +373,10 @@ static int i8k_proc_show(struct seq_file left_speed = i8k_get_fan_speed(I8K_FAN_LEFT); /* 580 ??s */ right_speed = i8k_get_fan_speed(I8K_FAN_RIGHT); /* 580 ??s */ fn_key = i8k_get_fn_status(); /* 750 ??s */ - if (power_status) { + if (power_status) ac_power = i8k_get_power_status(); /* 14700 ??s */ - } else { + else ac_power = -1; - } /* * Info: @@ -530,10 +487,7 @@ static int __init i8k_probe(void) return 0; } -#ifdef MODULE -static -#endif -int __init i8k_init(void) +static int __init i8k_init(void) { struct proc_dir_entry *proc_i8k; @@ -556,19 +510,10 @@ int __init i8k_init(void) return 0; } -#ifdef MODULE -int init_module(void) +static void __exit i8k_exit(void) { - return i8k_init(); -} - -void cleanup_module(void) -{ - /* Remove the proc entry */ remove_proc_entry("i8k", NULL); - - printk(KERN_INFO "i8k: module unloaded\n"); } -#endif -/* end of file */ +module_init(i8k_init); +module_exit(i8k_exit); ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/5] I8K - convert to platform device (sysfs) 2005-02-24 6:14 ` [PATCH 4/5] I8K - switch to module_{init|exit} Dmitry Torokhov @ 2005-02-24 6:14 ` Dmitry Torokhov 0 siblings, 0 replies; 28+ messages in thread From: Dmitry Torokhov @ 2005-02-24 6:14 UTC (permalink / raw) To: LKML; +Cc: Massimo Dal Zotto i8k.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 117 insertions(+) Index: dtor/drivers/char/i8k.c =================================================================== --- dtor.orig/drivers/char/i8k.c +++ dtor/drivers/char/i8k.c @@ -22,6 +22,7 @@ #include <linux/proc_fs.h> #include <linux/seq_file.h> #include <linux/dmi.h> +#include <linux/device.h> #include <asm/uaccess.h> #include <asm/io.h> @@ -87,6 +88,13 @@ static struct file_operations i8k_fops = .ioctl = i8k_ioctl, }; +static struct device_driver i8k_driver = { + .name = "i8k", + .bus = &platform_bus_type, +}; + +static struct platform_device *i8k_device; + struct smm_regs { unsigned int eax; unsigned int ebx __attribute__ ((packed)); @@ -406,6 +414,89 @@ static int i8k_open_fs(struct inode *ino return single_open(file, i8k_proc_show, NULL); } +static ssize_t i8k_sysfs_cpu_temp_show(struct device *dev, char *buf) +{ + int temp = i8k_get_cpu_temp(); + + return temp < 0 ? -EIO : sprintf(buf, "%d\n", temp); +} + +static ssize_t i8k_sysfs_fan1_show(struct device *dev, char *buf) +{ + int status = i8k_get_fan_status(0); + return status < 0 ? -EIO : sprintf(buf, "%d\n", status); +} + +static ssize_t i8k_sysfs_fan1_set(struct device *dev, const char *buf, size_t count) +{ + unsigned long state; + char *rest; + + if (restricted && !capable(CAP_SYS_ADMIN)) + return -EPERM; + + state = simple_strtoul(buf, &rest, 10); + if (*rest || state > I8K_FAN_MAX) + return -EINVAL; + + if (i8k_set_fan(0, state) < 0) + return -EIO; + + return count; +} + +static ssize_t i8k_sysfs_fan2_show(struct device *dev, char *buf) +{ + int status = i8k_get_fan_status(1); + return status < 0 ? -EIO : sprintf(buf, "%d\n", status); +} + +static ssize_t i8k_sysfs_fan2_set(struct device *dev, const char *buf, size_t count) +{ + unsigned long state; + char *rest; + + if (restricted && !capable(CAP_SYS_ADMIN)) + return -EPERM; + + state = simple_strtoul(buf, &rest, 10); + if (*rest || state > I8K_FAN_MAX) + return -EINVAL; + + if (i8k_set_fan(1, state) < 0) + return -EIO; + + return count; +} + +static ssize_t i8k_sysfs_fan1_speed_show(struct device *dev, char *buf) +{ + int speed = i8k_get_fan_speed(0); + return speed < 0 ? -EIO : sprintf(buf, "%d\n", speed); +} + +static ssize_t i8k_sysfs_fan2_speed_show(struct device *dev, char *buf) +{ + int speed = i8k_get_fan_speed(1); + return speed < 0 ? -EIO : sprintf(buf, "%d\n", speed); +} + +static ssize_t i8k_sysfs_power_status_show(struct device *dev, char *buf) +{ + int status = power_status ? i8k_get_power_status() : -1; + return status < 0 ? -EIO : sprintf(buf, "%d\n", status); +} + +static struct device_attribute i8k_device_attrs[] = { + __ATTR(cpu_temp, 0444, i8k_sysfs_cpu_temp_show, NULL), + __ATTR(fan1_state, 0644, i8k_sysfs_fan1_show, i8k_sysfs_fan1_set), + __ATTR(fan2_state, 0644, i8k_sysfs_fan2_show, i8k_sysfs_fan2_set), + __ATTR(fan1_speed, 0444, i8k_sysfs_fan1_speed_show, NULL), + __ATTR(fan2_speed, 0444, i8k_sysfs_fan2_speed_show, NULL), + __ATTR(power_status, 0444, i8k_sysfs_power_status_show, NULL), + __ATTR_NULL +}; + static struct dmi_system_id __initdata i8k_dmi_table[] = { { .ident = "Dell Inspiron", @@ -490,6 +581,7 @@ static int __init i8k_probe(void) static int __init i8k_init(void) { struct proc_dir_entry *proc_i8k; + int err, i; /* Are we running on an supported laptop? */ if (i8k_probe()) @@ -503,15 +595,40 @@ static int __init i8k_init(void) proc_i8k->proc_fops = &i8k_fops; proc_i8k->owner = THIS_MODULE; + err = driver_register(&i8k_driver); + if (err) + goto fail1; + + i8k_device = platform_device_register_simple("i8k", -1, NULL, 0); + if (IS_ERR(i8k_device)) { + err = PTR_ERR(i8k_device); + goto fail2; + } + + for (i = 0; attr_name(i8k_device_attrs[i]); i++) { + err = device_create_file(&i8k_device->dev, &i8k_device_attrs[i]); + if (err) + goto fail3; + } + printk(KERN_INFO "Dell laptop SMM driver v%s Massimo Dal Zotto (dz@debian.org)\n", I8K_VERSION); return 0; + +fail3: while (--i >= 0) + device_remove_file(&i8k_device->dev, &i8k_device_attrs[i]); + platform_device_unregister(i8k_device); +fail2: driver_unregister(&i8k_driver); +fail1: remove_proc_entry("i8k", NULL); + return err; } static void __exit i8k_exit(void) { + platform_device_unregister(i8k_device); + driver_unregister(&i8k_driver); remove_proc_entry("i8k", NULL); } ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-02-24 6:10 [PATCH 0/5] I8K driver facelift Dmitry Torokhov 2005-02-24 6:11 ` [PATCH 1/5] I8K - pass though Lindent Dmitry Torokhov @ 2005-03-13 3:41 ` Frank Sorenson 2005-03-13 3:59 ` Dmitry Torokhov ` (2 more replies) 1 sibling, 3 replies; 28+ messages in thread From: Frank Sorenson @ 2005-03-13 3:41 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: LKML, Massimo Dal Zotto -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Dmitry Torokhov wrote: | Hi, | | here are some changes that freshen I8K driver (Dell Inspiron/Latitude | platform driver). The patches have been tested on Inspiron 8100. <snip> | Please consider for inclusion. | | Thanks! These patches look pretty good. A few comments (with a patch--tested on my Inspiron 9200): - - The "return i8k_smm(®s) < 0 ? : regs.eax;" construction is nice and tidy, but it isn't passing on the return value of the called function, and is returning TRUE or 1 on failure. This makes it difficult to check the return value for valid data. Old behavior returned negative, so I'll return -1. - - The I8K_VERSION string should probably be changed to something more unique. The version maintained outside the kernel tree by Massimo Dal Zotto (http://www.debian.org/~dz/i8k/) is up to 1.25, so 1.14 isn't very distinguishing. Maybe just use the date? Not sure here... - - Also, some newer Dell laptops require a different function to get the Dell signature, so the i8k_get_dell_signature test should check both (borrowing some code from the out-of-kernel version). - - Some newer Dell laptops report DMI_SYS_VENDOR as "Dell Inc." rather than "Dell Computer" - - Some of the Dell motherboards provide more than 1 temperature sensor. ~ How about a generic i8k_get_temp function, and i8k_get_cpu_temp just calls that with sensor 0. - - Also, I've added detection of the number of temperature sensors and fans at init time. This way, we aren't hardcoded to 1 sensor and 2 fans. I couldn't figure out how to set up the sysfs entries dynamically, but that probably should happen too. - - Some boards don't need the I8K_FAN_MULT to get the right fan RPM (I don't think my fans are rotating at over 90,000 RPM)! I guess we'll need to address this sometime, but I have not done so. Patch follows: Signed-off-by: Frank Sorenson <frank@tuxrocks.com> - --- 2.6.11-a/drivers/char/i8k.c 2005-03-12 18:47:55.000000000 -0700 +++ 2.6.11-b/drivers/char/i8k.c 2005-03-12 20:29:01.000000000 -0700 @@ -28,7 +28,7 @@ ~ #include <linux/i8k.h> - -#define I8K_VERSION "1.14 21/02/2005" +#define I8K_VERSION "2005-03-12" ~ #define I8K_SMM_FN_STATUS 0x0025 ~ #define I8K_SMM_POWER_STATUS 0x0069 @@ -36,7 +36,8 @@ ~ #define I8K_SMM_GET_FAN 0x00a3 ~ #define I8K_SMM_GET_SPEED 0x02a3 ~ #define I8K_SMM_GET_TEMP 0x10a3 - -#define I8K_SMM_GET_DELL_SIG 0xffa3 +#define I8K_SMM_GET_DELL_SIG1 0xfea3 +#define I8K_SMM_GET_DELL_SIG2 0xffa3 ~ #define I8K_SMM_BIOS_VERSION 0x00a6 ~ #define I8K_FAN_MULT 30 @@ -55,6 +56,8 @@ ~ #define I8K_TEMPERATURE_BUG 1 ~ static char bios_version[4]; +static int num_temps = 0; +static int num_fans = 0; ~ MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)"); ~ MODULE_DESCRIPTION("Driver for accessing SMM BIOS on Dell laptops"); @@ -201,10 +204,10 @@ ~ */ ~ static int i8k_get_fan_status(int fan) ~ { - - struct smm_regs regs = { .eax = I8K_SMM_GET_FAN, }; + struct smm_regs regs = { .eax = I8K_SMM_GET_FAN, .ebx = (fan & 0xff)}; ~ regs.ebx = fan & 0xff; - - return i8k_smm(®s) < 0 ? : regs.eax & 0xff; + return i8k_smm(®s) < 0 ? -1 : regs.eax & 0xff; ~ } ~ /* @@ -215,7 +218,7 @@ ~ struct smm_regs regs = { .eax = I8K_SMM_GET_SPEED, }; ~ regs.ebx = fan & 0xff; - - return i8k_smm(®s) < 0 ? : (regs.eax & 0xffff) * I8K_FAN_MULT; + return i8k_smm(®s) < 0 ? -1 : (regs.eax & 0xffff) * I8K_FAN_MULT; ~ } ~ /* @@ -228,15 +231,15 @@ ~ speed = (speed < 0) ? 0 : ((speed > I8K_FAN_MAX) ? I8K_FAN_MAX : speed); ~ regs.ebx = (fan & 0xff) | (speed << 8); - - return i8k_smm(®s) < 0 ? : i8k_get_fan_status(fan); + return i8k_smm(®s) < 0 ? -1 : i8k_get_fan_status(fan); ~ } ~ /* ~ * Read the cpu temperature. ~ */ - -static int i8k_get_cpu_temp(void) +static int i8k_get_temp(int sensor) ~ { - - struct smm_regs regs = { .eax = I8K_SMM_GET_TEMP, }; + struct smm_regs regs = { .eax = I8K_SMM_GET_TEMP, .ebx = sensor }; ~ int rc; ~ int temp; @@ -268,9 +271,14 @@ ~ return temp; ~ } - -static int i8k_get_dell_signature(void) +static int i8k_get_cpu_temp(void) +{ + return i8k_get_temp(0); +} + +static int i8k_get_dell_sig_aux(int fn) ~ { - - struct smm_regs regs = { .eax = I8K_SMM_GET_DELL_SIG, }; + struct smm_regs regs = { .eax = fn, }; ~ int rc; ~ if ((rc = i8k_smm(®s)) < 0) @@ -279,6 +287,17 @@ ~ return regs.eax == 1145651527 && regs.edx == 1145392204 ? 0 : -1; ~ } +static int i8k_get_dell_signature(void) +{ + int rc; + + if (((rc=i8k_get_dell_sig_aux(I8K_SMM_GET_DELL_SIG1)) < 0) && + ((rc=i8k_get_dell_sig_aux(I8K_SMM_GET_DELL_SIG2)) < 0)) { + return rc; + } + return 0; +} + ~ static int i8k_ioctl(struct inode *ip, struct file *fp, unsigned int cmd, ~ unsigned long arg) ~ { @@ -506,6 +525,13 @@ ~ }, ~ }, ~ { + .ident = "Dell Inspiron", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron"), + }, + }, + { ~ .ident = "Dell Latitude", ~ .matches = { ~ DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer"), @@ -587,6 +613,11 @@ ~ if (i8k_probe()) ~ return -ENODEV; + while (i8k_get_temp(num_temps) >= 0) num_temps ++; + printk(KERN_INFO "i8k: found %d temperature sensors\n", num_temps); + while (i8k_get_fan_status(num_fans) >= 0) num_fans ++; + printk(KERN_INFO "i8k: found %d fans\n", num_fans); + ~ /* Register the proc entry */ ~ proc_i8k = create_proc_entry("i8k", 0, NULL); ~ if (!proc_i8k) Frank - -- Frank Sorenson - KD7TZK Systems Manager, Computer Science Department Brigham Young University frank@tuxrocks.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.6 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCM7ZZaI0dwg4A47wRAok1AKDLYIrMXLphYAeAq98OXYqxk6U1vACfQpld qczdJm2992BjeQ/iU9RP+k4= =nNut -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-13 3:41 ` [PATCH 0/5] I8K driver facelift Frank Sorenson @ 2005-03-13 3:59 ` Dmitry Torokhov 2005-03-15 8:12 ` Valdis.Kletnieks 2005-03-16 21:38 ` Frank Sorenson 2 siblings, 0 replies; 28+ messages in thread From: Dmitry Torokhov @ 2005-03-13 3:59 UTC (permalink / raw) To: Frank Sorenson; +Cc: LKML, Massimo Dal Zotto On Saturday 12 March 2005 22:41, Frank Sorenson wrote: > Dmitry Torokhov wrote: > | Hi, > | > | here are some changes that freshen I8K driver (Dell Inspiron/Latitude > | platform driver). The patches have been tested on Inspiron 8100. > <snip> > | Please consider for inclusion. > | > | Thanks! > > These patches look pretty good. A few comments (with a patch--tested on > my Inspiron 9200): > > - The "return i8k_smm(®s) < 0 ? : regs.eax;" construction is nice and > tidy, but it isn't passing on the return value of the called function, > and is returning TRUE or 1 on failure. This makes it difficult to check > the return value for valid data. Old behavior returned negative, so > I'll return -1. Hi, Actually I am not sure what I was thinkinhg when I wrtote it, the correct version should be "return i8k_smm(®s) ? : regs.eax;" since i8k_smm return 0 on success. I will think about dynamically adding attributes... -- Dmitry ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-13 3:41 ` [PATCH 0/5] I8K driver facelift Frank Sorenson 2005-03-13 3:59 ` Dmitry Torokhov @ 2005-03-15 8:12 ` Valdis.Kletnieks 2005-03-15 10:59 ` Giuseppe Bilotta 2005-03-16 21:38 ` Frank Sorenson 2 siblings, 1 reply; 28+ messages in thread From: Valdis.Kletnieks @ 2005-03-15 8:12 UTC (permalink / raw) To: Frank Sorenson; +Cc: Dmitry Torokhov, LKML, Massimo Dal Zotto [-- Attachment #1: Type: text/plain, Size: 815 bytes --] On Sat, 12 Mar 2005 20:41:14 MST, Frank Sorenson said: > These patches look pretty good. A few comments (with a patch--tested on > my Inspiron 9200): I tested your patch on top of Dmitry's on a Dell Latitude C840, seems to work. > - - Some of the Dell motherboards provide more than 1 temperature sensor. > ~ How about a generic i8k_get_temp function, and i8k_get_cpu_temp just > calls that with sensor 0. > > - - Also, I've added detection of the number of temperature sensors and > fans at init time. This way, we aren't hardcoded to 1 sensor and 2 > fans. I couldn't figure out how to set up the sysfs entries > dynamically, but that probably should happen too. According to your patch, the C840 has 2 temp sensors. I'll have to figure out what the second one is (prob either the GPU or the disk drive?) [-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --] ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-15 8:12 ` Valdis.Kletnieks @ 2005-03-15 10:59 ` Giuseppe Bilotta 2005-03-15 17:30 ` Valdis.Kletnieks 0 siblings, 1 reply; 28+ messages in thread From: Giuseppe Bilotta @ 2005-03-15 10:59 UTC (permalink / raw) To: linux-kernel > According to your patch, the C840 has 2 temp sensors. I'll have to figure > out what the second one is (prob either the GPU or the disk drive?) If it runs over 40 C easily it's probably the GPU :) -- Giuseppe "Oblomov" Bilotta Can't you see It all makes perfect sense Expressed in dollar and cents Pounds shillings and pence (Roger Waters) ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-15 10:59 ` Giuseppe Bilotta @ 2005-03-15 17:30 ` Valdis.Kletnieks 2005-03-15 22:34 ` Frank Sorenson 0 siblings, 1 reply; 28+ messages in thread From: Valdis.Kletnieks @ 2005-03-15 17:30 UTC (permalink / raw) To: Giuseppe Bilotta; +Cc: linux-kernel [-- Attachment #1: Type: text/plain, Size: 707 bytes --] On Tue, 15 Mar 2005 11:59:22 +0100, Giuseppe Bilotta said: > > According to your patch, the C840 has 2 temp sensors. I'll have to figure > > out what the second one is (prob either the GPU or the disk drive?) > > If it runs over 40 C easily it's probably the GPU :) Well, (a) the next rev of the patch will hopefully provide more access to the second thermal probe than just detecting its existence (it still doesn't do the sysfs or whatever magic to make the actual value accessible), and (b) the probe I *know* about is on the CPU, and runs over 40C easily as well (it's sitting at 49C right now). Remember we're talking about a laptop here, there's not a lot of room for a big heat sink in there.. ;) [-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --] ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-15 17:30 ` Valdis.Kletnieks @ 2005-03-15 22:34 ` Frank Sorenson 0 siblings, 0 replies; 28+ messages in thread From: Frank Sorenson @ 2005-03-15 22:34 UTC (permalink / raw) To: linux-kernel; +Cc: Valdis.Kletnieks, Giuseppe Bilotta -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Valdis.Kletnieks@vt.edu wrote: > Well, (a) the next rev of the patch will hopefully provide more access to the > second thermal probe than just detecting its existence (it still doesn't do > the sysfs or whatever magic to make the actual value accessible), and (b) the > probe I *know* about is on the CPU, and runs over 40C easily as well (it's sitting > at 49C right now). Remember we're talking about a laptop here, there's not > a lot of room for a big heat sink in there.. ;) I've been trying to work out how to do this through dynamic sysfs attributes, but I have not found a way to create arbitrary attributes like this. It's not hard to define them at kernel compile time, but selecting the right number of sensors to compile in seems arbitrary. My Inspiron 9200 has 4 sensors, and who knows how many next year's model will have. It just doesn't seem like the Linux Kernel way of doing things to arbitrarily limit it like this. I've looked into several ways of creating sysfs attributes, but haven't found anything that works right/well. One of the most interesting was in this past LKML thread - http://lkml.org/lkml/2004/8/20/287 If I could replace the sysfs_attr_show() with my own, I believe that might work (the attribute is passed into the function, so the name should be available). It's odd that it's so easy to compile sysfs attributes into the kernel, but nobody seems to know how to generate them dynamically. Thoughts? Suggestions? Thanks, Frank - -- Frank Sorenson - KD7TZK Systems Manager, Computer Science Department Brigham Young University frank@tuxrocks.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.6 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCN2LeaI0dwg4A47wRAhWEAKC+CcoLmoyvS6RXy7n7gtTnKjPXsACgtCbE zofgMMEmc5mAzrQKdKwpIMQ= =xNOU -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-13 3:41 ` [PATCH 0/5] I8K driver facelift Frank Sorenson 2005-03-13 3:59 ` Dmitry Torokhov 2005-03-15 8:12 ` Valdis.Kletnieks @ 2005-03-16 21:38 ` Frank Sorenson 2005-03-17 6:40 ` Dmitry Torokhov 2005-03-17 8:16 ` Valdis.Kletnieks 2 siblings, 2 replies; 28+ messages in thread From: Frank Sorenson @ 2005-03-16 21:38 UTC (permalink / raw) To: Frank Sorenson; +Cc: Dmitry Torokhov, LKML, Valdis.Kletnieks Okay, I replaced the sysfs_ops with ops of my own, and now all the show and store functions also accept the name of the attribute as a parameter. This lets the functions know what attribute is being accessed, and allows us to create attributes that share show and store functions, so things don't need to be defined at compile time (I feel slightly evil!). This patch puts the correct number of temp sensors and fans into sysfs, and only exposes power_status if enabled by the power_status module parameter. This patch applies on top of Dmitry Torokov's patches. Comments welcome! Thanks, Frank Signed-off-by: Frank Sorenson <frank@tuxrocks.com> --- 2.6.11-a/drivers/char/i8k.c 2005-03-12 18:47:55.000000000 -0700 +++ 2.6.11-b/drivers/char/i8k.c 2005-03-16 14:23:40.000000000 -0700 @@ -23,12 +23,14 @@ #include <linux/seq_file.h> #include <linux/dmi.h> #include <linux/device.h> +#include <linux/sysfs.h> +#include <linux/kobject.h> #include <asm/uaccess.h> #include <asm/io.h> #include <linux/i8k.h> -#define I8K_VERSION "1.14 21/02/2005" +#define I8K_VERSION "2005-03-16" #define I8K_SMM_FN_STATUS 0x0025 #define I8K_SMM_POWER_STATUS 0x0069 @@ -36,7 +38,8 @@ #define I8K_SMM_GET_FAN 0x00a3 #define I8K_SMM_GET_SPEED 0x02a3 #define I8K_SMM_GET_TEMP 0x10a3 -#define I8K_SMM_GET_DELL_SIG 0xffa3 +#define I8K_SMM_GET_DELL_SIG1 0xfea3 +#define I8K_SMM_GET_DELL_SIG2 0xffa3 #define I8K_SMM_BIOS_VERSION 0x00a6 #define I8K_FAN_MULT 30 @@ -54,7 +57,12 @@ #define I8K_TEMPERATURE_BUG 1 +#define to_dev(d) container_of(d, struct device, kobj) +#define to_dev_attr(_attr) container_of(_attr,struct device_attribute,attr) + static char bios_version[4]; +static int num_temps = 0; +static int num_fans = 0; MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)"); MODULE_DESCRIPTION("Driver for accessing SMM BIOS on Dell laptops"); @@ -80,6 +88,8 @@ static int i8k_ioctl(struct inode *, struct file *, unsigned int, unsigned long); +static struct kobj_type *backup_ktype; + static struct file_operations i8k_fops = { .open = i8k_open_fs, .read = seq_read, @@ -94,7 +104,7 @@ }; static struct platform_device *i8k_device; - + struct smm_regs { unsigned int eax; unsigned int ebx __attribute__ ((packed)); @@ -156,7 +166,7 @@ { struct smm_regs regs = { .eax = I8K_SMM_BIOS_VERSION, }; - return i8k_smm(®s) < 0 ? : regs.eax; + return i8k_smm(®s) ? : regs.eax; } /* @@ -201,10 +211,10 @@ */ static int i8k_get_fan_status(int fan) { - struct smm_regs regs = { .eax = I8K_SMM_GET_FAN, }; + struct smm_regs regs = { .eax = I8K_SMM_GET_FAN, .ebx = (fan & 0xff)}; regs.ebx = fan & 0xff; - return i8k_smm(®s) < 0 ? : regs.eax & 0xff; + return i8k_smm(®s) ? : regs.eax & 0xff; } /* @@ -215,7 +225,7 @@ struct smm_regs regs = { .eax = I8K_SMM_GET_SPEED, }; regs.ebx = fan & 0xff; - return i8k_smm(®s) < 0 ? : (regs.eax & 0xffff) * I8K_FAN_MULT; + return i8k_smm(®s) ? : (regs.eax & 0xffff) * I8K_FAN_MULT; } /* @@ -228,15 +238,15 @@ speed = (speed < 0) ? 0 : ((speed > I8K_FAN_MAX) ? I8K_FAN_MAX : speed); regs.ebx = (fan & 0xff) | (speed << 8); - return i8k_smm(®s) < 0 ? : i8k_get_fan_status(fan); + return i8k_smm(®s) ? : i8k_get_fan_status(fan); } /* * Read the cpu temperature. */ -static int i8k_get_cpu_temp(void) +static int i8k_get_temp(int sensor) { - struct smm_regs regs = { .eax = I8K_SMM_GET_TEMP, }; + struct smm_regs regs = { .eax = I8K_SMM_GET_TEMP, .ebx = sensor }; int rc; int temp; @@ -268,9 +278,14 @@ return temp; } -static int i8k_get_dell_signature(void) +static int i8k_get_cpu_temp(void) { - struct smm_regs regs = { .eax = I8K_SMM_GET_DELL_SIG, }; + return i8k_get_temp(0); +} + +static int i8k_get_dell_sig_aux(int fn) +{ + struct smm_regs regs = { .eax = fn, }; int rc; if ((rc = i8k_smm(®s)) < 0) @@ -279,6 +294,17 @@ return regs.eax == 1145651527 && regs.edx == 1145392204 ? 0 : -1; } +static int i8k_get_dell_signature(void) +{ + int rc; + + if (((rc=i8k_get_dell_sig_aux(I8K_SMM_GET_DELL_SIG1)) < 0) && + ((rc=i8k_get_dell_sig_aux(I8K_SMM_GET_DELL_SIG2)) < 0)) { + return rc; + } + return 0; +} + static int i8k_ioctl(struct inode *ip, struct file *fp, unsigned int cmd, unsigned long arg) { @@ -414,87 +440,112 @@ return single_open(file, i8k_proc_show, NULL); } -static ssize_t i8k_sysfs_cpu_temp_show(struct device *dev, char *buf) +static ssize_t i8k_sysfs_cpu_temp_show(struct device *dev, char *buf, + const char *name) { int temp = i8k_get_cpu_temp(); return temp < 0 ? -EIO : sprintf(buf, "%d\n", temp); } -static ssize_t i8k_sysfs_fan1_show(struct device *dev, char *buf) +static ssize_t i8k_sysfs_temp_show(struct device *dev, char *buf, + const char *name) { - int status = i8k_get_fan_status(0); - return status < 0 ? -EIO : sprintf(buf, "%d\n", status); -} - -static ssize_t i8k_sysfs_fan1_set(struct device *dev, const char *buf, size_t count) -{ - unsigned long state; + unsigned long temp_num; + int temp; char *rest; - if (restricted && !capable(CAP_SYS_ADMIN)) - return -EPERM; - - state = simple_strtoul(buf, &rest, 10); - if (*rest || state > I8K_FAN_MAX) - return -EINVAL; + temp_num = simple_strtoul(name + 4, &rest, 10); + temp = i8k_get_temp(temp_num - 1); - if (i8k_set_fan(0, state) < 0) - return -EIO; - - return count; + return temp < 0 ? -EIO : sprintf(buf, "%d\n", temp); } -static ssize_t i8k_sysfs_fan2_show(struct device *dev, char *buf) +static ssize_t i8k_sysfs_fan_show(struct device *dev, char *buf, + const char *name) { - int status = i8k_get_fan_status(1); + unsigned long fan_num; + int status; + char *rest; + + fan_num = simple_strtoul(name + 3, &rest, 10); + status = i8k_get_fan_status(fan_num - 1); return status < 0 ? -EIO : sprintf(buf, "%d\n", status); } -static ssize_t i8k_sysfs_fan2_set(struct device *dev, const char *buf, size_t count) +static ssize_t i8k_sysfs_fan_set(struct device *dev, const char *buf, + size_t count, const char *name) { + unsigned long fan_num; unsigned long state; char *rest; if (restricted && !capable(CAP_SYS_ADMIN)) return -EPERM; + fan_num = simple_strtoul(name + 3, &rest, 10); + state = simple_strtoul(buf, &rest, 10); if (*rest || state > I8K_FAN_MAX) return -EINVAL; - if (i8k_set_fan(1, state) < 0) + if (i8k_set_fan(fan_num - 1, state) < 0) return -EIO; return count; } -static ssize_t i8k_sysfs_fan1_speed_show(struct device *dev, char *buf) +static ssize_t i8k_sysfs_fan_speed_show(struct device *dev, char *buf, + const char *name) { - int speed = i8k_get_fan_speed(0); - return speed < 0 ? -EIO : sprintf(buf, "%d\n", speed); -} + unsigned long fan_num; + char *rest; + int speed; -static ssize_t i8k_sysfs_fan2_speed_show(struct device *dev, char *buf) -{ - int speed = i8k_get_fan_speed(1); + fan_num = simple_strtoul(name + 3, &rest, 10); + speed = i8k_get_fan_speed(fan_num - 1); return speed < 0 ? -EIO : sprintf(buf, "%d\n", speed); } -static ssize_t i8k_sysfs_power_status_show(struct device *dev, char *buf) +static ssize_t i8k_sysfs_power_status_show(struct device *dev, char *buf, + char *name) { int status = power_status ? i8k_get_power_status() : -1; return status < 0 ? -EIO : sprintf(buf, "%d\n", status); } -static struct device_attribute i8k_device_attrs[] = { - __ATTR(cpu_temp, 0444, i8k_sysfs_cpu_temp_show, NULL), - __ATTR(fan1_state, 0644, i8k_sysfs_fan1_show, i8k_sysfs_fan1_set), - __ATTR(fan2_state, 0644, i8k_sysfs_fan2_show, i8k_sysfs_fan2_set), - __ATTR(fan1_speed, 0444, i8k_sysfs_fan1_speed_show, NULL), - __ATTR(fan2_speed, 0444, i8k_sysfs_fan2_speed_show, NULL), - __ATTR(power_status, 0444, i8k_sysfs_power_status_show, NULL), - __ATTR_NULL +static ssize_t i8k_sysfs_show(struct kobject *kobj, struct attribute *attr, + char * buf) +{ + struct device_attribute * dev_attr = to_dev_attr(attr); + struct device * dev = to_dev(kobj); + int (*func3)(struct device *, char *, char *); + + if (dev_attr->show) { + func3 = dev_attr->show; + return (*func3)(dev, buf, dev_attr->attr.name); + } + return -EINVAL; +} + +static ssize_t i8k_sysfs_store(struct kobject *kobj, struct attribute *attr, + const char *buf, size_t count) +{ + struct device_attribute * dev_attr = to_dev_attr(attr); + struct device * dev = to_dev(kobj); + int (*func4)(struct device *, const char *, size_t, char *); + + if (dev_attr->store) + { + func4 = dev_attr->store; + return (*func4)(dev, buf, count, dev_attr->attr.name); + } + return -EINVAL; +} + +static struct sysfs_ops i8k_sysfs_ops = { + .show = i8k_sysfs_show, + .store = i8k_sysfs_store, }; static struct dmi_system_id __initdata i8k_dmi_table[] = { @@ -506,6 +557,13 @@ }, }, { + .ident = "Dell Inspiron", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron"), + }, + }, + { .ident = "Dell Latitude", .matches = { DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer"), @@ -578,6 +636,105 @@ return 0; } +static int sysfs_add_generic(char *dev_name, int mode, void *show, void *store) +{ + char *name; + struct device_attribute *attr; + int err; + + name = kmalloc(strlen(dev_name) + 1, GFP_KERNEL); + if (name == NULL) + return -ENOMEM; + strncpy(name, dev_name, strlen(dev_name)); + name[strlen(dev_name)] = '\0'; + attr = kmalloc(sizeof *attr, GFP_KERNEL); + if (attr == NULL) + return -ENOMEM; + *attr = ((struct device_attribute) { + .attr = { + .name = name, + .mode = mode, + }, + .show = show, + .store = store, + }); + err = device_create_file(&i8k_device->dev, attr); + if (err) + { + device_remove_file(&i8k_device->dev, attr); + kfree(attr); + kfree(name); + return -1; + } + return 0; +} + +static int sysfs_add_temp(int temp_num) +{ + char temp_name[30]; + + sprintf(temp_name, "temp%d", temp_num + 1); + return sysfs_add_generic(temp_name, 0444, i8k_sysfs_temp_show, NULL); +} + +static int sysfs_add_fan(int fan_num) +{ + char fan_state_name[30]; + char fan_speed_name[30]; + + sprintf(fan_state_name, "fan%d_state", fan_num + 1); + sprintf(fan_speed_name, "fan%d_speed", fan_num + 1); + + sysfs_add_generic(fan_state_name, 0644, + i8k_sysfs_fan_show, i8k_sysfs_fan_set); + sysfs_add_generic(fan_speed_name, 0444, + i8k_sysfs_fan_speed_show, NULL); + return 0; +} + +static int i8k_redirect_sysfs_ops(void) +{ + struct device *dev; + struct kobject *kobj; + struct kobj_type *ktype; + struct kobj_type *temp_ktype; + struct kset *kset; + + dev = &i8k_device->dev; + kobj = &dev->kobj; + kset = kobj->kset; + ktype = kset->ktype; + + temp_ktype = kmalloc(sizeof *temp_ktype, GFP_KERNEL); + + temp_ktype->release = ktype->release; + temp_ktype->sysfs_ops = &i8k_sysfs_ops; + temp_ktype->default_attrs = ktype->default_attrs; + + backup_ktype = ktype; + kset->ktype = temp_ktype; + + return 0; +} + +static int i8k_restore_sysfs_ops(void) +{ + struct device *dev; + struct kobject *kobj; + struct kset *kset; + struct kobj_type *temp_ktype; + + dev = &i8k_device->dev; + kobj = &dev->kobj; + kset = kobj->kset; + temp_ktype = kset->ktype; + + kset->ktype = backup_ktype; + kfree(temp_ktype); + + return 0; +} + static int __init i8k_init(void) { struct proc_dir_entry *proc_i8k; @@ -587,6 +744,11 @@ if (i8k_probe()) return -ENODEV; + while (i8k_get_temp(num_temps) >= 0) num_temps ++; + printk(KERN_INFO "i8k: found %d temperature sensors\n", num_temps); + while (i8k_get_fan_status(num_fans) >= 0) num_fans ++; + printk(KERN_INFO "i8k: found %d fans\n", num_fans); + /* Register the proc entry */ proc_i8k = create_proc_entry("i8k", 0, NULL); if (!proc_i8k) @@ -605,21 +767,31 @@ goto fail2; } - for (i = 0; attr_name(i8k_device_attrs[i]); i++) { - err = device_create_file(&i8k_device->dev, &i8k_device_attrs[i]); + i8k_redirect_sysfs_ops(); + + /* register the temp sensors */ + for (i = 0; i < num_temps; i++) { + err = sysfs_add_temp(i); + if (err) + goto fail2; + } + + /* register the fans */ + for (i = 0; i < num_fans; i++) { + err = sysfs_add_fan(i); if (err) - goto fail3; + goto fail2; } + sysfs_add_generic("cpu_temp", 0444, i8k_sysfs_cpu_temp_show, NULL); + if (power_status) + sysfs_add_generic("power_status", 0444, + i8k_sysfs_power_status_show, NULL); printk(KERN_INFO "Dell laptop SMM driver v%s Massimo Dal Zotto (dz@debian.org)\n", I8K_VERSION); return 0; - -fail3: while (--i >= 0) - device_remove_file(&i8k_device->dev, &i8k_device_attrs[i]); - platform_device_unregister(i8k_device); fail2: driver_unregister(&i8k_driver); fail1: remove_proc_entry("i8k", NULL); return err; @@ -627,6 +799,7 @@ static void __exit i8k_exit(void) { + i8k_restore_sysfs_ops(); platform_device_unregister(i8k_device); driver_unregister(&i8k_driver); remove_proc_entry("i8k", NULL); ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-16 21:38 ` Frank Sorenson @ 2005-03-17 6:40 ` Dmitry Torokhov 2005-03-17 9:37 ` Frank Sorenson ` (2 more replies) 2005-03-17 8:16 ` Valdis.Kletnieks 1 sibling, 3 replies; 28+ messages in thread From: Dmitry Torokhov @ 2005-03-17 6:40 UTC (permalink / raw) To: Frank Sorenson; +Cc: Greg KH, LKML, Valdis.Kletnieks [-- Attachment #1: Type: text/plain, Size: 7207 bytes --] On Wednesday 16 March 2005 16:38, Frank Sorenson wrote: > Okay, I replaced the sysfs_ops with ops of my own, and now all the show > and store functions also accept the name of the attribute as a parameter. > This lets the functions know what attribute is being accessed, and allows > us to create attributes that share show and store functions, so things > don't need to be defined at compile time (I feel slightly evil!). Hrm, can we be a little more explicit and not poke in the sysfs guts right in the driver? What do you think about the patch below athat implements "attribute arrays"? And I am attaching cumulative i8k patch using these arrays so they can be tested. I am CC-ing Greg to see what he thinks about it. -- Dmitry =================================================================== sysfs: add support for attribure arrays so it is possible to create a (variable) number of similar attributes all sharing the same show and store handlers: ../<attr_name>/0 ../<attr_name>/1 ... ../<attr_name>/<n> Signed-off-by: Dmitry Torokhov <dtor@mail.ru> fs/sysfs/Makefile | 2 fs/sysfs/array.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/sysfs.h | 20 +++++ 3 files changed, 187 insertions(+), 2 deletions(-) Index: dtor/fs/sysfs/array.c =================================================================== --- /dev/null +++ dtor/fs/sysfs/array.c @@ -0,0 +1,167 @@ +/* + * fs/sysfs/array.c - Operations for adding/removing arrays of attributes. + * + * Copyright (c) 2005 Dmitry Torokhov <dtor@mail.ru> + * + * This file is released under the GPL v2. + * + */ + +#include <linux/kobject.h> +#include <linux/module.h> +#include <linux/dcache.h> +#include <linux/err.h> +#include "sysfs.h" + +struct attr_element { + struct attribute attr; + unsigned int idx; + char name[4]; +}; +#define to_attr_element(e) container_of(e, struct attr_element, attr) + +struct array_object { + const struct attribute_array *array; + unsigned int n_elements; + struct kobject kobj; + struct attr_element elements[0]; +}; +#define to_array_object(obj) container_of(obj, struct array_object, kobj) + +static ssize_t +attr_array_show(struct kobject *kobj, struct attribute *attr, char *buf) +{ + struct attr_element *element = to_attr_element(attr); + struct array_object *obj = to_array_object(kobj); + ssize_t ret = 0; + + if (obj->array->show) { + struct kobject *parent = kobject_get(kobj->parent); + ret = obj->array->show(parent, element->idx, buf); + kobject_put(parent); + } + + return ret; +} + +static ssize_t +attr_array_store(struct kobject *kobj, struct attribute *attr, + const char *buf, size_t count) +{ + struct attr_element *element = to_attr_element(attr); + struct array_object *obj = to_array_object(kobj); + ssize_t ret = 0; + + if (obj->array->store) { + struct kobject *parent = kobject_get(kobj->parent); + ret = obj->array->store(parent, element->idx, buf, count); + kobject_put(parent); + } + + return ret; +} + +static struct sysfs_ops attr_array_sysfs_ops = { + .show = attr_array_show, + .store = attr_array_store, +}; + + +static void attr_array_release(struct kobject *kobj) +{ + kfree(to_array_object(kobj)); +} + +static struct kobj_type ktype_attr_array = { + .release = attr_array_release, + .sysfs_ops = &attr_array_sysfs_ops, +}; + + +static int create_array_element(struct array_object *obj, + unsigned int idx) +{ + struct attr_element *element = &obj->elements[idx]; + + snprintf(element->name, sizeof(element->name), "%d", idx); + element->idx = idx; + element->attr = obj->array->attr; + element->attr.name = element->name; + + return sysfs_create_file(&obj->kobj, &element->attr); +} + +static inline void remove_array_element(struct array_object *obj, + unsigned int idx) +{ + sysfs_remove_file(&obj->kobj, &obj->elements[idx].attr); +} + +int sysfs_create_array(struct kobject *kobj, + const struct attribute_array *array, + unsigned int n_elements) +{ + struct array_object *obj; + size_t size; + unsigned int i; + int error; + + BUG_ON(!kobj || !kobj->dentry); + BUG_ON(!array->attr.name); + + size = sizeof(struct array_object) + + sizeof(struct attr_element) * n_elements; + + obj = kmalloc(size, GFP_KERNEL); + if (!obj) + return -ENOMEM; + + memset(obj, 0, size); + obj->array = array; + obj->n_elements = n_elements; + + obj->kobj.ktype = &ktype_attr_array; + obj->kobj.parent = kobj; + kobject_set_name(&obj->kobj, array->attr.name); + + error = kobject_register(&obj->kobj); + if (error) + goto fail; + + for (i = 0; i < n_elements; i++) { + error = create_array_element(obj, i); + if (error) { + while (--i) + remove_array_element(obj, i); + goto fail; + } + } + + return 0; + + fail: + kfree(obj); + return error; +} + +void sysfs_remove_array(struct kobject *kobj, + const struct attribute_array *array) +{ + struct array_object *obj; + struct dentry *dir; + unsigned int i; + + dir = sysfs_get_dentry(kobj->dentry, array->attr.name); + + if (dir) { + obj = to_array_object(to_kobj(dir)); + for (i = 0; i < obj->n_elements; i++) + remove_array_element(obj, i); + kobject_unregister(&obj->kobj); + } + + dput(dir); +} + +EXPORT_SYMBOL_GPL(sysfs_create_array); +EXPORT_SYMBOL_GPL(sysfs_remove_array); Index: dtor/include/linux/sysfs.h =================================================================== --- dtor.orig/include/linux/sysfs.h +++ dtor/include/linux/sysfs.h @@ -26,7 +26,12 @@ struct attribute_group { struct attribute ** attrs; }; +struct attribute_array { + struct attribute attr; + ssize_t (*show)(struct kobject *, unsigned int index, char *); + ssize_t (*store)(struct kobject *, unsigned int index, const char *, size_t); +}; /** * Use these macros to make defining attributes easier. See include/linux/device.h @@ -102,7 +107,7 @@ sysfs_update_file(struct kobject *, cons extern void sysfs_remove_file(struct kobject *, const struct attribute *); -extern int +extern int sysfs_create_link(struct kobject * kobj, struct kobject * target, char * name); extern void @@ -114,6 +119,9 @@ int sysfs_remove_bin_file(struct kobject int sysfs_create_group(struct kobject *, const struct attribute_group *); void sysfs_remove_group(struct kobject *, const struct attribute_group *); +int sysfs_create_array(struct kobject *, const struct attribute_array *, unsigned int); +void sysfs_remove_array(struct kobject *, const struct attribute_array *); + #else /* CONFIG_SYSFS */ static inline int sysfs_create_dir(struct kobject * k) @@ -177,6 +185,16 @@ static inline void sysfs_remove_group(st ; } +static inline int sysfs_create_array(struct kobject * k, const struct attribute_array *a, unsigned int n) +{ + return 0; +} + +static inline void sysfs_remove_array(struct kobject * k, const struct attribute_array * a) +{ + ; +} + #endif /* CONFIG_SYSFS */ #endif /* _SYSFS_H_ */ Index: dtor/fs/sysfs/Makefile =================================================================== --- dtor.orig/fs/sysfs/Makefile +++ dtor/fs/sysfs/Makefile @@ -3,4 +3,4 @@ # obj-y := inode.o file.o dir.o symlink.o mount.o bin.o \ - group.o + group.o array.o [-- Attachment #2: i8k-cumulative.patch --] [-- Type: text/x-diff, Size: 30736 bytes --] --- dtor/drivers/char/i8k.c.orig 2005-03-17 01:20:04.000000000 -0500 +++ dtor/drivers/char/i8k.c 2005-03-17 01:19:26.000000000 -0500 @@ -20,13 +20,15 @@ #include <linux/types.h> #include <linux/init.h> #include <linux/proc_fs.h> -#include <linux/apm_bios.h> +#include <linux/seq_file.h> +#include <linux/dmi.h> +#include <linux/device.h> #include <asm/uaccess.h> #include <asm/io.h> #include <linux/i8k.h> -#define I8K_VERSION "1.13 14/05/2002" +#define I8K_VERSION "1.14 21/02/2005" #define I8K_SMM_FN_STATUS 0x0025 #define I8K_SMM_POWER_STATUS 0x0069 @@ -34,7 +36,8 @@ #define I8K_SMM_GET_FAN 0x00a3 #define I8K_SMM_GET_SPEED 0x02a3 #define I8K_SMM_GET_TEMP 0x10a3 -#define I8K_SMM_GET_DELL_SIG 0xffa3 +#define I8K_SMM_GET_DELL_SIG1 0xfea3 +#define I8K_SMM_GET_DELL_SIG2 0xffa3 #define I8K_SMM_BIOS_VERSION 0x00a6 #define I8K_FAN_MULT 30 @@ -52,18 +55,7 @@ #define I8K_TEMPERATURE_BUG 1 -#define DELL_SIGNATURE "Dell Computer" - -static char *supported_models[] = { - "Inspiron", - "Latitude", - NULL -}; - -static char system_vendor[48] = "?"; -static char product_name [48] = "?"; -static char bios_version [4] = "?"; -static char serial_number[16] = "?"; +static char bios_version[4]; MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)"); MODULE_DESCRIPTION("Driver for accessing SMM BIOS on Dell laptops"); @@ -73,6 +65,10 @@ module_param(force, bool, 0); MODULE_PARM_DESC(force, "Force loading without checking for supported models"); +static int ignore_dmi; +module_param(ignore_dmi, bool, 0); +MODULE_PARM_DESC(ignore_dmi, "Continue probing hardware even if DMI data does not match"); + static int restricted; module_param(restricted, bool, 0); MODULE_PARM_DESC(restricted, "Allow fan control if SYS_ADMIN capability set"); @@ -81,69 +77,76 @@ module_param(power_status, bool, 0600); MODULE_PARM_DESC(power_status, "Report power status in /proc/i8k"); -static ssize_t i8k_read(struct file *, char __user *, size_t, loff_t *); +static int i8k_open_fs(struct inode *inode, struct file *file); static int i8k_ioctl(struct inode *, struct file *, unsigned int, unsigned long); static struct file_operations i8k_fops = { - .read = i8k_read, - .ioctl = i8k_ioctl, + .open = i8k_open_fs, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, + .ioctl = i8k_ioctl, }; -typedef struct { - unsigned int eax; - unsigned int ebx __attribute__ ((packed)); - unsigned int ecx __attribute__ ((packed)); - unsigned int edx __attribute__ ((packed)); - unsigned int esi __attribute__ ((packed)); - unsigned int edi __attribute__ ((packed)); -} SMMRegisters; - -typedef struct { - u8 type; - u8 length; - u16 handle; -} DMIHeader; +static struct device_driver i8k_driver = { + .name = "i8k", + .bus = &platform_bus_type, +}; + +static struct platform_device *i8k_device; + +struct smm_regs { + unsigned int eax; + unsigned int ebx __attribute__ ((packed)); + unsigned int ecx __attribute__ ((packed)); + unsigned int edx __attribute__ ((packed)); + unsigned int esi __attribute__ ((packed)); + unsigned int edi __attribute__ ((packed)); +}; + +static inline char *i8k_get_dmi_data(int field) +{ + return dmi_get_system_info(field) ? : "N/A"; +} /* * Call the System Management Mode BIOS. Code provided by Jonathan Buzzard. */ -static int i8k_smm(SMMRegisters *regs) +static int i8k_smm(struct smm_regs *regs) { - int rc; - int eax = regs->eax; + int rc; + int eax = regs->eax; + + asm("pushl %%eax\n\t" + "movl 0(%%eax),%%edx\n\t" + "push %%edx\n\t" + "movl 4(%%eax),%%ebx\n\t" + "movl 8(%%eax),%%ecx\n\t" + "movl 12(%%eax),%%edx\n\t" + "movl 16(%%eax),%%esi\n\t" + "movl 20(%%eax),%%edi\n\t" + "popl %%eax\n\t" + "out %%al,$0xb2\n\t" + "out %%al,$0x84\n\t" + "xchgl %%eax,(%%esp)\n\t" + "movl %%ebx,4(%%eax)\n\t" + "movl %%ecx,8(%%eax)\n\t" + "movl %%edx,12(%%eax)\n\t" + "movl %%esi,16(%%eax)\n\t" + "movl %%edi,20(%%eax)\n\t" + "popl %%edx\n\t" + "movl %%edx,0(%%eax)\n\t" + "lahf\n\t" + "shrl $8,%%eax\n\t" + "andl $1,%%eax\n":"=a"(rc) + : "a"(regs) + : "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory"); - asm("pushl %%eax\n\t" \ - "movl 0(%%eax),%%edx\n\t" \ - "push %%edx\n\t" \ - "movl 4(%%eax),%%ebx\n\t" \ - "movl 8(%%eax),%%ecx\n\t" \ - "movl 12(%%eax),%%edx\n\t" \ - "movl 16(%%eax),%%esi\n\t" \ - "movl 20(%%eax),%%edi\n\t" \ - "popl %%eax\n\t" \ - "out %%al,$0xb2\n\t" \ - "out %%al,$0x84\n\t" \ - "xchgl %%eax,(%%esp)\n\t" - "movl %%ebx,4(%%eax)\n\t" \ - "movl %%ecx,8(%%eax)\n\t" \ - "movl %%edx,12(%%eax)\n\t" \ - "movl %%esi,16(%%eax)\n\t" \ - "movl %%edi,20(%%eax)\n\t" \ - "popl %%edx\n\t" \ - "movl %%edx,0(%%eax)\n\t" \ - "lahf\n\t" \ - "shrl $8,%%eax\n\t" \ - "andl $1,%%eax\n" \ - : "=a" (rc) - : "a" (regs) - : "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory"); - - if ((rc != 0) || ((regs->eax & 0xffff) == 0xffff) || (regs->eax == eax)) { - return -EINVAL; - } + if (rc != 0 || (regs->eax & 0xffff) == 0xffff || regs->eax == eax) + return -EINVAL; - return 0; + return 0; } /* @@ -152,24 +155,9 @@ */ static int i8k_get_bios_version(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; - - regs.eax = I8K_SMM_BIOS_VERSION; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - - return regs.eax; -} + struct smm_regs regs = { .eax = I8K_SMM_BIOS_VERSION, }; -/* - * Read the machine id. - */ -static int i8k_get_serial_number(unsigned char *buff) -{ - strlcpy(buff, serial_number, sizeof(serial_number)); - return 0; + return i8k_smm(®s) ? : regs.eax; } /* @@ -177,24 +165,22 @@ */ static int i8k_get_fn_status(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + struct smm_regs regs = { .eax = I8K_SMM_FN_STATUS, }; + int rc; - regs.eax = I8K_SMM_FN_STATUS; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - - switch ((regs.eax >> I8K_FN_SHIFT) & I8K_FN_MASK) { - case I8K_FN_UP: - return I8K_VOL_UP; - case I8K_FN_DOWN: - return I8K_VOL_DOWN; - case I8K_FN_MUTE: - return I8K_VOL_MUTE; - default: - return 0; - } + if ((rc = i8k_smm(®s)) < 0) + return rc; + + switch ((regs.eax >> I8K_FN_SHIFT) & I8K_FN_MASK) { + case I8K_FN_UP: + return I8K_VOL_UP; + case I8K_FN_DOWN: + return I8K_VOL_DOWN; + case I8K_FN_MUTE: + return I8K_VOL_MUTE; + default: + return 0; + } } /* @@ -202,20 +188,13 @@ */ static int i8k_get_power_status(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + struct smm_regs regs = { .eax = I8K_SMM_POWER_STATUS, }; + int rc; + + if ((rc = i8k_smm(®s)) < 0) + return rc; - regs.eax = I8K_SMM_POWER_STATUS; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - - switch (regs.eax & 0xff) { - case I8K_POWER_AC: - return I8K_AC; - default: - return I8K_BATTERY; - } + return (regs.eax & 0xff) == I8K_POWER_AC ? I8K_AC : I8K_BATTERY; } /* @@ -223,16 +202,10 @@ */ static int i8k_get_fan_status(int fan) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + struct smm_regs regs = { .eax = I8K_SMM_GET_FAN, }; - regs.eax = I8K_SMM_GET_FAN; - regs.ebx = fan & 0xff; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - - return (regs.eax & 0xff); + regs.ebx = fan & 0xff; + return i8k_smm(®s) ? : regs.eax & 0xff; } /* @@ -240,16 +213,10 @@ */ static int i8k_get_fan_speed(int fan) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; - - regs.eax = I8K_SMM_GET_SPEED; - regs.ebx = fan & 0xff; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } + struct smm_regs regs = { .eax = I8K_SMM_GET_SPEED, }; - return (regs.eax & 0xffff) * I8K_FAN_MULT; + regs.ebx = fan & 0xff; + return i8k_smm(®s) ? : (regs.eax & 0xffff) * I8K_FAN_MULT; } /* @@ -257,532 +224,441 @@ */ static int i8k_set_fan(int fan, int speed) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + struct smm_regs regs = { .eax = I8K_SMM_SET_FAN, }; - speed = (speed < 0) ? 0 : ((speed > I8K_FAN_MAX) ? I8K_FAN_MAX : speed); + speed = (speed < 0) ? 0 : ((speed > I8K_FAN_MAX) ? I8K_FAN_MAX : speed); + regs.ebx = (fan & 0xff) | (speed << 8); - regs.eax = I8K_SMM_SET_FAN; - regs.ebx = (fan & 0xff) | (speed << 8); - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - - return (i8k_get_fan_status(fan)); + return i8k_smm(®s) ? : i8k_get_fan_status(fan); } /* * Read the cpu temperature. */ -static int i8k_get_cpu_temp(void) +static int i8k_get_temp(int sensor) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; - int temp; + struct smm_regs regs = { .eax = I8K_SMM_GET_TEMP, }; + int rc; + int temp; #ifdef I8K_TEMPERATURE_BUG - static int prev = 0; + static int prev; #endif + regs.ebx = sensor & 0xff; + if ((rc = i8k_smm(®s)) < 0) + return rc; - regs.eax = I8K_SMM_GET_TEMP; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - temp = regs.eax & 0xff; + temp = regs.eax & 0xff; #ifdef I8K_TEMPERATURE_BUG - /* - * Sometimes the temperature sensor returns 0x99, which is out of range. - * In this case we return (once) the previous cached value. For example: - # 1003655137 00000058 00005a4b - # 1003655138 00000099 00003a80 <--- 0x99 = 153 degrees - # 1003655139 00000054 00005c52 - */ - if (temp > I8K_MAX_TEMP) { - temp = prev; - prev = I8K_MAX_TEMP; - } else { - prev = temp; - } + /* + * Sometimes the temperature sensor returns 0x99, which is out of range. + * In this case we return (once) the previous cached value. For example: + # 1003655137 00000058 00005a4b + # 1003655138 00000099 00003a80 <--- 0x99 = 153 degrees + # 1003655139 00000054 00005c52 + */ + if (temp > I8K_MAX_TEMP) { + temp = prev; + prev = I8K_MAX_TEMP; + } else { + prev = temp; + } #endif - return temp; + return temp; } -static int i8k_get_dell_signature(void) +static int i8k_get_dell_signature(int req_fn) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + struct smm_regs regs = { .eax = req_fn, }; + int rc; - regs.eax = I8K_SMM_GET_DELL_SIG; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } + if ((rc = i8k_smm(®s)) < 0) + return rc; - if ((regs.eax == 1145651527) && (regs.edx == 1145392204)) { - return 0; - } else { - return -1; - } + return regs.eax == 1145651527 && regs.edx == 1145392204 ? 0 : -1; } static int i8k_ioctl(struct inode *ip, struct file *fp, unsigned int cmd, unsigned long arg) { - int val; - int speed; - unsigned char buff[16]; - int __user *argp = (int __user *)arg; - - if (!argp) - return -EINVAL; - - switch (cmd) { - case I8K_BIOS_VERSION: - val = i8k_get_bios_version(); - break; - - case I8K_MACHINE_ID: - memset(buff, 0, 16); - val = i8k_get_serial_number(buff); - break; - - case I8K_FN_STATUS: - val = i8k_get_fn_status(); - break; - - case I8K_POWER_STATUS: - val = i8k_get_power_status(); - break; - - case I8K_GET_TEMP: - val = i8k_get_cpu_temp(); - break; - - case I8K_GET_SPEED: - if (copy_from_user(&val, argp, sizeof(int))) { - return -EFAULT; - } - val = i8k_get_fan_speed(val); - break; + int val = 0; + int speed; + unsigned char buff[16]; + int __user *argp = (int __user *)arg; - case I8K_GET_FAN: - if (copy_from_user(&val, argp, sizeof(int))) { - return -EFAULT; - } - val = i8k_get_fan_status(val); - break; + if (!argp) + return -EINVAL; - case I8K_SET_FAN: - if (restricted && !capable(CAP_SYS_ADMIN)) { - return -EPERM; - } - if (copy_from_user(&val, argp, sizeof(int))) { - return -EFAULT; - } - if (copy_from_user(&speed, argp+1, sizeof(int))) { - return -EFAULT; - } - val = i8k_set_fan(val, speed); - break; + switch (cmd) { + case I8K_BIOS_VERSION: + val = i8k_get_bios_version(); + break; - default: - return -EINVAL; - } - - if (val < 0) { - return val; - } - - switch (cmd) { - case I8K_BIOS_VERSION: - if (copy_to_user(argp, &val, 4)) { - return -EFAULT; - } - break; - case I8K_MACHINE_ID: - if (copy_to_user(argp, buff, 16)) { - return -EFAULT; - } - break; - default: - if (copy_to_user(argp, &val, sizeof(int))) { - return -EFAULT; - } - break; - } + case I8K_MACHINE_ID: + memset(buff, 0, 16); + strlcpy(buff, i8k_get_dmi_data(DMI_PRODUCT_SERIAL), sizeof(buff)); + break; - return 0; -} + case I8K_FN_STATUS: + val = i8k_get_fn_status(); + break; -/* - * Print the information for /proc/i8k. - */ -static int i8k_get_info(char *buffer, char **start, off_t fpos, int length) -{ - int n, fn_key, cpu_temp, ac_power; - int left_fan, right_fan, left_speed, right_speed; + case I8K_POWER_STATUS: + val = i8k_get_power_status(); + break; - cpu_temp = i8k_get_cpu_temp(); /* 11100 µs */ - left_fan = i8k_get_fan_status(I8K_FAN_LEFT); /* 580 µs */ - right_fan = i8k_get_fan_status(I8K_FAN_RIGHT); /* 580 µs */ - left_speed = i8k_get_fan_speed(I8K_FAN_LEFT); /* 580 µs */ - right_speed = i8k_get_fan_speed(I8K_FAN_RIGHT); /* 580 µs */ - fn_key = i8k_get_fn_status(); /* 750 µs */ - if (power_status) { - ac_power = i8k_get_power_status(); /* 14700 µs */ - } else { - ac_power = -1; - } - - /* - * Info: - * - * 1) Format version (this will change if format changes) - * 2) BIOS version - * 3) BIOS machine ID - * 4) Cpu temperature - * 5) Left fan status - * 6) Right fan status - * 7) Left fan speed - * 8) Right fan speed - * 9) AC power - * 10) Fn Key status - */ - n = sprintf(buffer, "%s %s %s %d %d %d %d %d %d %d\n", - I8K_PROC_FMT, - bios_version, - serial_number, - cpu_temp, - left_fan, - right_fan, - left_speed, - right_speed, - ac_power, - fn_key); - - return n; -} - -static ssize_t i8k_read(struct file *f, char __user *buffer, size_t len, loff_t *fpos) -{ - int n; - char info[128]; - - n = i8k_get_info(info, NULL, 0, 128); - if (n <= 0) { - return n; - } + case I8K_GET_TEMP: + val = i8k_get_temp(0); + break; - if (*fpos >= n) { - return 0; - } + case I8K_GET_SPEED: + if (copy_from_user(&val, argp, sizeof(int))) + return -EFAULT; - if ((*fpos + len) >= n) { - len = n - *fpos; - } + val = i8k_get_fan_speed(val); + break; - if (copy_to_user(buffer, info, len) != 0) { - return -EFAULT; - } + case I8K_GET_FAN: + if (copy_from_user(&val, argp, sizeof(int))) + return -EFAULT; - *fpos += len; - return len; -} + val = i8k_get_fan_status(val); + break; -static char* __init string_trim(char *s, int size) -{ - int len; - char *p; + case I8K_SET_FAN: + if (restricted && !capable(CAP_SYS_ADMIN)) + return -EPERM; - if ((len = strlen(s)) > size) { - len = size; - } + if (copy_from_user(&val, argp, sizeof(int))) + return -EFAULT; - for (p=s+len-1; len && (*p==' '); len--,p--) { - *p = '\0'; - } + if (copy_from_user(&speed, argp + 1, sizeof(int))) + return -EFAULT; - return s; -} + val = i8k_set_fan(val, speed); + break; + + default: + return -EINVAL; + } -/* DMI code, stolen from arch/i386/kernel/dmi_scan.c */ + if (val < 0) + return val; + + switch (cmd) { + case I8K_BIOS_VERSION: + if (copy_to_user(argp, &val, 4)) + return -EFAULT; + + break; + case I8K_MACHINE_ID: + if (copy_to_user(argp, buff, 16)) + return -EFAULT; + + break; + default: + if (copy_to_user(argp, &val, sizeof(int))) + return -EFAULT; + + break; + } + + return 0; +} /* - * |<-- dmi->length -->| - * | | - * |dmi header s=N | string1,\0, ..., stringN,\0, ..., \0 - * | | - * +-----------------------+ + * Print the information for /proc/i8k. */ -static char* __init dmi_string(DMIHeader *dmi, u8 s) +static int i8k_proc_show(struct seq_file *seq, void *offset) { - u8 *p; + int fn_key, cpu_temp, ac_power; + int left_fan, right_fan, left_speed, right_speed; - if (!s) { - return ""; - } - s--; + cpu_temp = i8k_get_temp(0); /* 11100 µs */ + left_fan = i8k_get_fan_status(I8K_FAN_LEFT); /* 580 µs */ + right_fan = i8k_get_fan_status(I8K_FAN_RIGHT); /* 580 µs */ + left_speed = i8k_get_fan_speed(I8K_FAN_LEFT); /* 580 µs */ + right_speed = i8k_get_fan_speed(I8K_FAN_RIGHT); /* 580 µs */ + fn_key = i8k_get_fn_status(); /* 750 µs */ + if (power_status) + ac_power = i8k_get_power_status(); /* 14700 µs */ + else + ac_power = -1; - p = (u8 *)dmi + dmi->length; - while (s > 0) { - p += strlen(p); - p++; - s--; - } + /* + * Info: + * + * 1) Format version (this will change if format changes) + * 2) BIOS version + * 3) BIOS machine ID + * 4) Cpu temperature + * 5) Left fan status + * 6) Right fan status + * 7) Left fan speed + * 8) Right fan speed + * 9) AC power + * 10) Fn Key status + */ + return seq_printf(seq, "%s %s %s %d %d %d %d %d %d %d\n", + I8K_PROC_FMT, + bios_version, + dmi_get_system_info(DMI_PRODUCT_SERIAL) ? : "N/A", + cpu_temp, + left_fan, right_fan, left_speed, right_speed, + ac_power, fn_key); +} - return p; +static int i8k_open_fs(struct inode *inode, struct file *file) +{ + return single_open(file, i8k_proc_show, NULL); } -static void __init dmi_decode(DMIHeader *dmi) + +static ssize_t i8k_sysfs_temp_show(struct kobject *kobj, unsigned int idx, char *buf) { - u8 *data = (u8 *) dmi; - char *p; + int temp = i8k_get_temp(idx); + return temp < 0 ? -EIO : sprintf(buf, "%d\n", temp); +} -#ifdef I8K_DEBUG - int i; - printk("%08x ", (int)data); - for (i=0; i<data[1] && i<64; i++) { - printk("%02x ", data[i]); - } - printk("\n"); -#endif +static struct attribute_array i8k_temp_attr = + __ATTR(temp, 0444, i8k_sysfs_temp_show, NULL); - switch (dmi->type) { - case 0: /* BIOS Information */ - p = dmi_string(dmi,data[5]); - if (*p) { - strlcpy(bios_version, p, sizeof(bios_version)); - string_trim(bios_version, sizeof(bios_version)); - } - break; - case 1: /* System Information */ - p = dmi_string(dmi,data[4]); - if (*p) { - strlcpy(system_vendor, p, sizeof(system_vendor)); - string_trim(system_vendor, sizeof(system_vendor)); - } - p = dmi_string(dmi,data[5]); - if (*p) { - strlcpy(product_name, p, sizeof(product_name)); - string_trim(product_name, sizeof(product_name)); - } - p = dmi_string(dmi,data[7]); - if (*p) { - strlcpy(serial_number, p, sizeof(serial_number)); - string_trim(serial_number, sizeof(serial_number)); - } - break; - } +static ssize_t i8k_sysfs_fan_show(struct kobject *kobj, unsigned int idx, char *buf) +{ + int status = i8k_get_fan_status(idx); + return status < 0 ? -EIO : sprintf(buf, "%d\n", status); } -static int __init dmi_table(u32 base, int len, int num, void (*fn)(DMIHeader*)) +static ssize_t i8k_sysfs_fan_set(struct kobject *kobj, unsigned int idx, const char *buf, size_t count) { - u8 *buf; - u8 *data; - DMIHeader *dmi; - int i = 1; - - buf = ioremap(base, len); - if (buf == NULL) { - return -1; - } - data = buf; - - /* - * Stop when we see al the items the table claimed to have - * or we run off the end of the table (also happens) - */ - while ((i<num) && ((data-buf) < len)) { - dmi = (DMIHeader *)data; - /* - * Avoid misparsing crud if the length of the last - * record is crap - */ - if ((data-buf+dmi->length) >= len) { - break; - } - fn(dmi); - data += dmi->length; - /* - * Don't go off the end of the data if there is - * stuff looking like string fill past the end - */ - while (((data-buf) < len) && (*data || data[1])) { - data++; - } - data += 2; - i++; - } - iounmap(buf); - - return 0; -} - -static int __init dmi_iterate(void (*decode)(DMIHeader *)) -{ - unsigned char buf[20]; - void __iomem *p = ioremap(0xe0000, 0x20000), *q; - - if (!p) - return -1; - - for (q = p; q < p + 0x20000; q += 16) { - memcpy_fromio(buf, q, 20); - if (memcmp(buf, "_DMI_", 5)==0) { - u16 num = buf[13]<<8 | buf[12]; - u16 len = buf [7]<<8 | buf [6]; - u32 base = buf[11]<<24 | buf[10]<<16 | buf[9]<<8 | buf[8]; -#ifdef I8K_DEBUG - printk(KERN_INFO "DMI %d.%d present.\n", - buf[14]>>4, buf[14]&0x0F); - printk(KERN_INFO "%d structures occupying %d bytes.\n", - buf[13]<<8 | buf[12], - buf [7]<<8 | buf[6]); - printk(KERN_INFO "DMI table at 0x%08X.\n", - buf[11]<<24 | buf[10]<<16 | buf[9]<<8 | buf[8]); -#endif - if (dmi_table(base, len, num, decode)==0) { - iounmap(p); - return 0; - } - } - } - iounmap(p); - return -1; + unsigned long state; + char *rest; + + if (restricted && !capable(CAP_SYS_ADMIN)) + return -EPERM; + + state = simple_strtoul(buf, &rest, 10); + if (*rest || state > I8K_FAN_MAX) + return -EINVAL; + + if (i8k_set_fan(idx, state) < 0) + return -EIO; + + return count; } -/* end of DMI code */ -/* - * Get DMI information. - */ -static int __init i8k_dmi_probe(void) +static struct attribute_array i8k_fan_state_attr = + __ATTR(fan_state, 0644, i8k_sysfs_fan_show, i8k_sysfs_fan_set); + +static ssize_t i8k_sysfs_fan_speed_show(struct kobject *kobj, unsigned int idx, char *buf) { - char **p; + int speed = i8k_get_fan_speed(idx); + return speed < 0 ? -EIO : sprintf(buf, "%d\n", speed); +} - if (dmi_iterate(dmi_decode) != 0) { - printk(KERN_INFO "i8k: unable to get DMI information\n"); - return -ENODEV; - } - - if (strncmp(system_vendor,DELL_SIGNATURE,strlen(DELL_SIGNATURE)) != 0) { - printk(KERN_INFO "i8k: not running on a Dell system\n"); - return -ENODEV; - } - - for (p=supported_models; ; p++) { - if (!*p) { - printk(KERN_INFO "i8k: unsupported model: %s\n", product_name); - return -ENODEV; - } - if (strncmp(product_name,*p,strlen(*p)) == 0) { - break; - } - } +static struct attribute_array i8k_fan_speed_attr = + __ATTR(fan_speed, 0644, i8k_sysfs_fan_speed_show, NULL); - return 0; +static ssize_t i8k_sysfs_power_status_show(struct device *dev, char *buf) +{ + int status = power_status ? i8k_get_power_status() : -1; + return status < 0 ? -EIO : sprintf(buf, "%d\n", status); } +static struct device_attribute i8k_power_status_attr = + __ATTR(power_status, 0444, i8k_sysfs_power_status_show, NULL); + + +static struct dmi_system_id __initdata i8k_dmi_table[] = { + { + .ident = "Dell Inspiron", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer"), + DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron"), + }, + }, + { + .ident = "Dell Latitude", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer"), + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude"), + }, + }, + { + .ident = "Dell Inspiron 2", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron"), + }, + }, + { + .ident = "Dell Latitude 2", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude"), + }, + }, + { } +}; + /* * Probe for the presence of a supported laptop. */ static int __init i8k_probe(void) { - char buff[4]; - int version; - int smm_found = 0; - - /* - * Get DMI information - */ - if (i8k_dmi_probe() != 0) { - printk(KERN_INFO "i8k: vendor=%s, model=%s, version=%s\n", - system_vendor, product_name, bios_version); - } - - /* - * Get SMM Dell signature - */ - if (i8k_get_dell_signature() != 0) { - printk(KERN_INFO "i8k: unable to get SMM Dell signature\n"); - } else { - smm_found = 1; - } - - /* - * Get SMM BIOS version. - */ - version = i8k_get_bios_version(); - if (version <= 0) { - printk(KERN_INFO "i8k: unable to get SMM BIOS version\n"); - } else { - smm_found = 1; - buff[0] = (version >> 16) & 0xff; - buff[1] = (version >> 8) & 0xff; - buff[2] = (version) & 0xff; - buff[3] = '\0'; + char buff[4]; + int version; + /* - * If DMI BIOS version is unknown use SMM BIOS version. + * Get DMI information */ - if (bios_version[0] == '?') { - strcpy(bios_version, buff); + if (!dmi_check_system(i8k_dmi_table)) { + if (!ignore_dmi && !force) + return -ENODEV; + + printk(KERN_INFO "i8k: not running on a supported Dell system.\n"); + printk(KERN_INFO "i8k: vendor=%s, model=%s, version=%s\n", + i8k_get_dmi_data(DMI_SYS_VENDOR), + i8k_get_dmi_data(DMI_PRODUCT_NAME), + i8k_get_dmi_data(DMI_BIOS_VERSION)); } + + strlcpy(bios_version, i8k_get_dmi_data(DMI_BIOS_VERSION), sizeof(bios_version)); + /* - * Check if the two versions match. + * Get SMM Dell signature */ - if (strncmp(buff,bios_version,sizeof(bios_version)) != 0) { - printk(KERN_INFO "i8k: BIOS version mismatch: %s != %s\n", - buff, bios_version); + if (i8k_get_dell_signature(I8K_SMM_GET_DELL_SIG1) && + i8k_get_dell_signature(I8K_SMM_GET_DELL_SIG2)) { + printk(KERN_ERR "i8k: unable to get SMM Dell signature\n"); + if (!force) + return -ENODEV; } - } - if (!smm_found && !force) { - return -ENODEV; - } + /* + * Get SMM BIOS version. + */ + version = i8k_get_bios_version(); + if (version <= 0) { + printk(KERN_ERR "i8k: unable to get SMM BIOS version\n"); + if (!force) + return -ENODEV; + } else { + buff[0] = (version >> 16) & 0xff; + buff[1] = (version >> 8) & 0xff; + buff[2] = (version) & 0xff; + buff[3] = '\0'; + /* + * If DMI BIOS version is unknown use SMM BIOS version. + */ + if (!dmi_get_system_info(DMI_BIOS_VERSION)) + strlcpy(bios_version, buff, sizeof(bios_version)); + + /* + * Check if the two versions match. + */ + if (strncmp(buff, bios_version, sizeof(bios_version)) != 0) + printk(KERN_WARNING "i8k: BIOS version mismatch: %s != %s\n", + buff, bios_version); + } - return 0; + return 0; } -#ifdef MODULE -static -#endif -int __init i8k_init(void) +static int __init i8k_create_sysfs_files(void) { - struct proc_dir_entry *proc_i8k; + int err, num; + + err = device_create_file(&i8k_device->dev, &i8k_power_status_attr); + if (err) + return err; + + for (num = 0; i8k_get_fan_status(num) >= 0; num++) + /* empty */; + + err = sysfs_create_array(&i8k_device->dev.kobj, &i8k_fan_state_attr, num); + if (err) + goto fail1; + + err = sysfs_create_array(&i8k_device->dev.kobj, &i8k_fan_speed_attr, num); + if (err) + goto fail2; - /* Are we running on an supported laptop? */ - if (i8k_probe() != 0) { - return -ENODEV; - } + for (num = 0; i8k_get_temp(num) >= 0; num++) + /* empty */; - /* Register the proc entry */ - proc_i8k = create_proc_info_entry("i8k", 0, NULL, i8k_get_info); - if (!proc_i8k) { - return -ENOENT; - } - proc_i8k->proc_fops = &i8k_fops; - proc_i8k->owner = THIS_MODULE; + err = sysfs_create_array(&i8k_device->dev.kobj, &i8k_temp_attr, num); + if (err) + goto fail3; - printk(KERN_INFO - "Dell laptop SMM driver v%s Massimo Dal Zotto (dz@debian.org)\n", - I8K_VERSION); + return 0; - return 0; +fail3: sysfs_remove_array(&i8k_device->dev.kobj, &i8k_fan_speed_attr); +fail2: sysfs_remove_array(&i8k_device->dev.kobj, &i8k_fan_state_attr); +fail1: device_remove_file(&i8k_device->dev, &i8k_power_status_attr); + return err; } -#ifdef MODULE -int init_module(void) +static void __exit i8k_remove_sysfs_files(void) { - return i8k_init(); + sysfs_remove_array(&i8k_device->dev.kobj, &i8k_temp_attr); + sysfs_remove_array(&i8k_device->dev.kobj, &i8k_fan_state_attr); + sysfs_remove_array(&i8k_device->dev.kobj, &i8k_fan_speed_attr); + device_remove_file(&i8k_device->dev, &i8k_power_status_attr); } -void cleanup_module(void) +static int __init i8k_init(void) { - /* Remove the proc entry */ - remove_proc_entry("i8k", NULL); + struct proc_dir_entry *proc_i8k; + int err; + + /* Are we running on an supported laptop? */ + if (i8k_probe()) + return -ENODEV; + + /* Register the proc entry */ + proc_i8k = create_proc_entry("i8k", 0, NULL); + if (!proc_i8k) + return -ENOENT; + + proc_i8k->proc_fops = &i8k_fops; + proc_i8k->owner = THIS_MODULE; + + err = driver_register(&i8k_driver); + if (err) + goto fail1; + + i8k_device = platform_device_register_simple("i8k", -1, NULL, 0); + if (IS_ERR(i8k_device)) { + err = PTR_ERR(i8k_device); + goto fail2; + } + + err = i8k_create_sysfs_files(); + if (err) + goto fail3; - printk(KERN_INFO "i8k: module unloaded\n"); + printk(KERN_INFO + "Dell laptop SMM driver v%s Massimo Dal Zotto (dz@debian.org)\n", + I8K_VERSION); + + return 0; + +fail3: platform_device_unregister(i8k_device); +fail2: driver_unregister(&i8k_driver); +fail1: remove_proc_entry("i8k", NULL); + return err; +} + +static void __exit i8k_exit(void) +{ + i8k_remove_sysfs_files(); + platform_device_unregister(i8k_device); + driver_unregister(&i8k_driver); + remove_proc_entry("i8k", NULL); } -#endif -/* end of file */ +module_init(i8k_init); +module_exit(i8k_exit); --- dtor/drivers/char/misc.c.orig 2005-03-17 01:24:32.000000000 -0500 +++ dtor/drivers/char/misc.c 2005-03-17 01:24:13.000000000 -0500 @@ -67,7 +67,6 @@ extern int rtc_MK48T08_init(void); extern int pmu_device_init(void); extern int tosh_init(void); -extern int i8k_init(void); #ifdef CONFIG_PROC_FS static void *misc_seq_start(struct seq_file *seq, loff_t *pos) @@ -317,9 +316,6 @@ #ifdef CONFIG_TOSHIBA tosh_init(); #endif -#ifdef CONFIG_I8K - i8k_init(); -#endif if (register_chrdev(MISC_MAJOR,"misc",&misc_fops)) { printk("unable to get major %d for misc devices\n", MISC_MAJOR); ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-17 6:40 ` Dmitry Torokhov @ 2005-03-17 9:37 ` Frank Sorenson 2005-03-17 15:05 ` Dmitry Torokhov 2005-03-17 9:46 ` Frank Sorenson 2005-03-24 7:25 ` Greg KH 2 siblings, 1 reply; 28+ messages in thread From: Frank Sorenson @ 2005-03-17 9:37 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Greg KH, LKML, Valdis.Kletnieks -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Dmitry Torokhov wrote: | Hrm, can we be a little more explicit and not poke in the sysfs guts right | in the driver? What do you think about the patch below athat implements | "attribute arrays"? And I am attaching cumulative i8k patch using these | arrays so they can be tested. | | I am CC-ing Greg to see what he thinks about it. Well, yes. That would probably be the better way to go about it, though I have to admit I was somewhat pleased with myself that I came up with something that worked. :) Your patches work well, with a few minor notes: 1: My Inspiron 9200 (and perhaps others) doesn't seem to respond to the I8K_SMM_BIOS_VERSION function call, so it fails the check in i8k_probe. ~ The check of i8k_get_bios_version doesn't seem critical, and removing the return -ENODEV makes it work again for me. That's the current behavior, so perhaps the printk level should just be changed to KERN_WARNING rather than KERN_ALERT. 2: To compile 2.6.11 cleanly, I needed two hunks from your original patch 2 (perhaps you're working from a more up-to-date tree than I am? If so, these are probably already addressed.): - --- dtor.orig/arch/i386/kernel/dmi_scan.c +++ dtor/arch/i386/kernel/dmi_scan.c @@ -416,6 +416,7 @@ static void __init dmi_decode(struct dmi ~ dmi_save_ident(dm, DMI_PRODUCT_VERSION, 6); ~ dmi_printk(("Serial Number: %s\n", ~ dmi_string(dm, data[7]))); + dmi_save_ident(dm, DMI_PRODUCT_SERIAL, 7); ~ break; ~ case 2: ~ dmi_printk(("Board Vendor: %s\n", - --- dtor.orig/include/linux/dmi.h +++ dtor/include/linux/dmi.h @@ -9,6 +9,7 @@ enum dmi_field { ~ DMI_SYS_VENDOR, ~ DMI_PRODUCT_NAME, ~ DMI_PRODUCT_VERSION, + DMI_PRODUCT_SERIAL, ~ DMI_BOARD_VENDOR, ~ DMI_BOARD_NAME, ~ DMI_BOARD_VERSION, I also have a question about the structure created using sysfs attribute arrays. Applying it in this case, I get: ./temp ./temp/3 ./temp/2 ./temp/1 ./temp/0 ./fan_speed ./fan_speed/1 ./fan_speed/0 ./fan_state ./fan_state/1 ./fan_state/0 The 'temp' entries make sense, however I'm not sure about the fan_speed and fan_state entries. From the perspective of how the objects are ordered, a fan would have 'speed' and 'state' attributes, but a 'fan_state' attribute wouldn't normally have a fan. Maybe something along these lines would make more sense from that perspective: ./fan/0 ./fan/0/speed ./fan/0/state ./fan/1 ./fan/1/speed ./fan/1/state I'm not certain about the best way to do this, so this may just be a thought. It would certainly be more complex to reorder it, and it appears usable in its current form. Frank - -- Frank Sorenson - KD7TZK Systems Manager, Computer Science Department Brigham Young University frank@tuxrocks.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.6 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD4DBQFCOU/0aI0dwg4A47wRAjgDAJwLsvd14J/qAmgv7JzkXG2xgAmTGwCY6RUc Nomk0pwTSfymHtIuF7ylzQ== =85eA -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-17 9:37 ` Frank Sorenson @ 2005-03-17 15:05 ` Dmitry Torokhov 0 siblings, 0 replies; 28+ messages in thread From: Dmitry Torokhov @ 2005-03-17 15:05 UTC (permalink / raw) To: Frank Sorenson; +Cc: Greg KH, LKML, Valdis.Kletnieks On Thu, 17 Mar 2005 02:37:56 -0700, Frank Sorenson <frank@tuxrocks.com> wrote: > 1: My Inspiron 9200 (and perhaps others) doesn't seem to respond to the > I8K_SMM_BIOS_VERSION function call, so it fails the check in i8k_probe. > ~ The check of i8k_get_bios_version doesn't seem critical, and removing > the return -ENODEV makes it work again for me. That's the current > behavior, so perhaps the printk level should just be changed to > KERN_WARNING rather than KERN_ALERT. You are probably right, I shoudl change that. > 2: To compile 2.6.11 cleanly, I needed two hunks from your original > patch 2 (perhaps you're working from a more up-to-date tree than I am? > If so, these are probably already addressed.): > Oh, sorry - when I was pereparing cumulative patch I simply missed this bit. It is still nowhere near the official tree. > > The 'temp' entries make sense, however I'm not sure about the fan_speed > and fan_state entries. From the perspective of how the objects are > ordered, a fan would have 'speed' and 'state' attributes, but a > 'fan_state' attribute wouldn't normally have a fan. Maybe something > along these lines would make more sense from that perspective: > > ./fan/0 > ./fan/0/speed > ./fan/0/state > ./fan/1 > ./fan/1/speed > ./fan/1/state > Yes, as soon as I did attribute array I realized that something like attr_array_group would reflect the structure better... We'll see what can be done. Thank you for your comments and suggestions! -- Dmitry ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-17 6:40 ` Dmitry Torokhov 2005-03-17 9:37 ` Frank Sorenson @ 2005-03-17 9:46 ` Frank Sorenson 2005-03-21 5:12 ` Dmitry Torokhov 2005-03-24 7:25 ` Greg KH 2 siblings, 1 reply; 28+ messages in thread From: Frank Sorenson @ 2005-03-17 9:46 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: LKML -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Dmitry Torokhov wrote: | Hrm, can we be a little more explicit and not poke in the sysfs guts right | in the driver? What do you think about the patch below athat implements | "attribute arrays"? And I am attaching cumulative i8k patch using these | arrays so they can be tested. Also, with power_status being a module parameter defaulting to 0/off, we can leave out the device_create_file for the i8k_power_status_attr. No need to expose it in sysfs if it will always return -EIO. Frank - -- Frank Sorenson - KD7TZK Systems Manager, Computer Science Department Brigham Young University frank@tuxrocks.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.6 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCOVHjaI0dwg4A47wRAgquAJ49qf5qFZX9twSbLetJiliEnES5GwCg41z2 r6AWC22/zAcz54xAIfNQJ4I= =0BtE -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-17 9:46 ` Frank Sorenson @ 2005-03-21 5:12 ` Dmitry Torokhov 2005-03-21 22:53 ` Frank Sorenson 0 siblings, 1 reply; 28+ messages in thread From: Dmitry Torokhov @ 2005-03-21 5:12 UTC (permalink / raw) To: Frank Sorenson; +Cc: LKML, Valdis.Kletnieks [-- Attachment #1: Type: text/plain, Size: 1043 bytes --] Hi, On Thursday 17 March 2005 04:46, Frank Sorenson wrote: > Dmitry Torokhov wrote: > | Hrm, can we be a little more explicit and not poke in the sysfs guts right > | in the driver? What do you think about the patch below athat implements > | "attribute arrays"? And I am attaching cumulative i8k patch using these > | arrays so they can be tested. > > Also, with power_status being a module parameter defaulting to 0/off, we > can leave out the device_create_file for the i8k_power_status_attr. No > need to expose it in sysfs if it will always return -EIO. > Since root can toggle power_status through sysfs at runtime (see /sys/modules/i8k/parameters/power_status) I think it is simplier to have the file always created. I have implemented arrays of groups of attributes: [dtor@core ~]$ ls -R /sys/bus/platform/devices/i8k/fan/ /sys/bus/platform/devices/i8k/fan/: 0 1 /sys/bus/platform/devices/i8k/fan/0: speed state /sys/bus/platform/devices/i8k/fan/1: speed state Please give it a try when you have time. Thanks! -- Dmitry [-- Attachment #2: attr-array.patch --] [-- Type: text/x-diff, Size: 6439 bytes --] =================================================================== sysfs: add support for attribute arrays so it is possible to create a number of similar attributes all sharing the same show and store handlers: ../<attr_name>/0 ../<attr_name>/1 ... ../<attr_name>/<n> Number of attributes can be specified at run-time. Signed-off-by: Dmitry Torokhov <dtor@mail.ru> fs/sysfs/Makefile | 2 fs/sysfs/array.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/sysfs.h | 20 +++++- 3 files changed, 182 insertions(+), 2 deletions(-) Index: dtor/fs/sysfs/array.c =================================================================== --- /dev/null +++ dtor/fs/sysfs/array.c @@ -0,0 +1,162 @@ +/* + * fs/sysfs/array.c - Operations for adding/removing arrays of attributes. + * + * Copyright (c) 2005 Dmitry Torokhov <dtor@mail.ru> + * + * This file is released under the GPL v2. + * + */ + +#include <linux/kobject.h> +#include <linux/module.h> +#include <linux/dcache.h> +#include "sysfs.h" + +struct attr_element { + struct attribute attr; + unsigned int idx; + char name[4]; +}; +#define to_attr_element(e) container_of(e, struct attr_element, attr) + +struct attr_array { + const struct enumerated_attr *base_attr; + unsigned int n_elements; + struct kobject kobj; + struct attr_element elements[0]; +}; +#define to_attr_array(obj) container_of(obj, struct attr_array, kobj) + +static ssize_t +attr_array_show(struct kobject *kobj, struct attribute *attr, char *buf) +{ + struct attr_element *element = to_attr_element(attr); + struct attr_array *array = to_attr_array(kobj); + ssize_t ret = 0; + + if (array->base_attr->show) + ret = array->base_attr->show(kobj->parent, element->idx, buf); + + return ret; +} + +static ssize_t +attr_array_store(struct kobject *kobj, struct attribute *attr, + const char *buf, size_t count) +{ + struct attr_element *element = to_attr_element(attr); + struct attr_array *array = to_attr_array(kobj); + ssize_t ret = 0; + + if (array->base_attr->store) + ret = array->base_attr->store(kobj->parent, element->idx, buf, count); + + return ret; +} + +static struct sysfs_ops attr_array_sysfs_ops = { + .show = attr_array_show, + .store = attr_array_store, +}; + +static void attr_array_release(struct kobject *kobj) +{ + kfree(to_attr_array(kobj)); +} + +static struct kobj_type ktype_attr_array = { + .release = attr_array_release, + .sysfs_ops = &attr_array_sysfs_ops, +}; + + +static int create_array_element(struct attr_array *array, + unsigned int idx) +{ + struct attr_element *element = &array->elements[idx]; + + snprintf(element->name, sizeof(element->name), "%d", idx); + element->idx = idx; + element->attr = array->base_attr->attr; + element->attr.name = element->name; + + return sysfs_create_file(&array->kobj, &element->attr); +} + +static inline void remove_array_element(struct attr_array *array, + unsigned int idx) +{ + sysfs_remove_file(&array->kobj, &array->elements[idx].attr); +} + +int sysfs_create_array(struct kobject *kobj, + const struct enumerated_attr *attr, + unsigned int n_elements) +{ + struct attr_array *array; + size_t size; + unsigned int i; + int error; + + BUG_ON(!kobj || !kobj->dentry); + BUG_ON(!attr_name(*attr)); + + if (n_elements > 999) + return -EINVAL; + + size = sizeof(struct attr_array) + + sizeof(struct attr_element) * n_elements; + + array = kmalloc(size, GFP_KERNEL); + if (!array) + return -ENOMEM; + + memset(array, 0, size); + array->base_attr = attr; + array->n_elements = n_elements; + + array->kobj.ktype = &ktype_attr_array; + array->kobj.parent = kobj; + kobject_set_name(&array->kobj, attr_name(*attr)); + + error = kobject_register(&array->kobj); + if (error) + goto fail; + + for (i = 0; i < n_elements; i++) { + error = create_array_element(array, i); + if (error) { + while (--i) + remove_array_element(array, i); + goto fail; + } + } + + return 0; + + fail: + kfree(array); + return error; +} + +void sysfs_remove_array(struct kobject *kobj, + const struct enumerated_attr *attr) +{ + struct attr_array *array; + struct dentry *dir; + unsigned int i; + + dir = sysfs_get_dentry(kobj->dentry, attr_name(*attr)); + + if (dir) { + array = to_attr_array(to_kobj(dir)); + for (i = 0; i < array->n_elements; i++) + remove_array_element(array, i); + kobject_unregister(&array->kobj); + } + + dput(dir); +} + +EXPORT_SYMBOL_GPL(sysfs_create_array); +EXPORT_SYMBOL_GPL(sysfs_remove_array); Index: dtor/include/linux/sysfs.h =================================================================== --- dtor.orig/include/linux/sysfs.h +++ dtor/include/linux/sysfs.h @@ -26,7 +26,12 @@ struct attribute_group { struct attribute ** attrs; }; +struct enumerated_attr { + struct attribute attr; + ssize_t (*show)(struct kobject *, unsigned int index, char *); + ssize_t (*store)(struct kobject *, unsigned int index, const char *, size_t); +}; /** * Use these macros to make defining attributes easier. See include/linux/device.h @@ -102,7 +107,7 @@ sysfs_update_file(struct kobject *, cons extern void sysfs_remove_file(struct kobject *, const struct attribute *); -extern int +extern int sysfs_create_link(struct kobject * kobj, struct kobject * target, char * name); extern void @@ -114,6 +119,9 @@ int sysfs_remove_bin_file(struct kobject int sysfs_create_group(struct kobject *, const struct attribute_group *); void sysfs_remove_group(struct kobject *, const struct attribute_group *); +int sysfs_create_array(struct kobject *, const struct enumerated_attr *, unsigned int); +void sysfs_remove_array(struct kobject *, const struct enumerated_attr *); + #else /* CONFIG_SYSFS */ static inline int sysfs_create_dir(struct kobject * k) @@ -177,6 +185,16 @@ static inline void sysfs_remove_group(st ; } +static inline int sysfs_create_array(struct kobject * k, const struct enumerated_attr * a, unsigned int n) +{ + return 0; +} + +static inline void sysfs_remove_array(struct kobject * k, const struct enumerated_attr * a) +{ + ; +} + #endif /* CONFIG_SYSFS */ #endif /* _SYSFS_H_ */ Index: dtor/fs/sysfs/Makefile =================================================================== --- dtor.orig/fs/sysfs/Makefile +++ dtor/fs/sysfs/Makefile @@ -3,4 +3,4 @@ # obj-y := inode.o file.o dir.o symlink.o mount.o bin.o \ - group.o + group.o array.o [-- Attachment #3: attr-group-array.patch --] [-- Type: text/x-diff, Size: 7263 bytes --] =================================================================== sysfs: add support for arrays of groups of attributes: ../<group_name>/0/<attr_a> ../<group_name>/0/<attr_b> ../<group_name>/1/<attr_a> ../<group_name>/1/<attr_b> ... ../<group_name>/<n>/<attr_a> ../<group_name>/<n>/<attr_b> Signed-off-by: Dmitry Torokhov <dtor@mail.ru> fs/sysfs/Makefile | 2 fs/sysfs/group-array.c | 206 +++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/sysfs.h | 18 ++++ 3 files changed, 225 insertions(+), 1 deletion(-) Index: dtor/fs/sysfs/Makefile =================================================================== --- dtor.orig/fs/sysfs/Makefile +++ dtor/fs/sysfs/Makefile @@ -3,4 +3,4 @@ # obj-y := inode.o file.o dir.o symlink.o mount.o bin.o \ - group.o array.o + group.o array.o group-array.o Index: dtor/include/linux/sysfs.h =================================================================== --- dtor.orig/include/linux/sysfs.h +++ dtor/include/linux/sysfs.h @@ -33,6 +33,11 @@ struct enumerated_attr { ssize_t (*store)(struct kobject *, unsigned int index, const char *, size_t); }; +struct attribute_group_array { + char * name; + struct enumerated_attr * attrs; +}; + /** * Use these macros to make defining attributes easier. See include/linux/device.h * for examples.. @@ -122,6 +127,9 @@ void sysfs_remove_group(struct kobject * int sysfs_create_array(struct kobject *, const struct enumerated_attr *, unsigned int); void sysfs_remove_array(struct kobject *, const struct enumerated_attr *); +int sysfs_create_group_array(struct kobject *, const struct attribute_group_array *, unsigned int); +void sysfs_remove_group_array(struct kobject *, const struct attribute_group_array *); + #else /* CONFIG_SYSFS */ static inline int sysfs_create_dir(struct kobject * k) @@ -195,6 +203,16 @@ static inline void sysfs_remove_array(st ; } +static inline int sysfs_create_group_array(struct kobject * k, const struct attribute_group_array * a, unsigned int n) +{ + return 0; +} + +static inline void sysfs_remove_group_array(struct kobject * k, const struct attribute_group_array * a) +{ + ; +} + #endif /* CONFIG_SYSFS */ #endif /* _SYSFS_H_ */ Index: dtor/fs/sysfs/group-array.c =================================================================== --- /dev/null +++ dtor/fs/sysfs/group-array.c @@ -0,0 +1,206 @@ +/* + * fs/sysfs/group-array.c - Operations for adding/removing arrays of groups + * of attributes. + * + * Copyright (c) 2005 Dmitry Torokhov <dtor@mail.ru> + * + * This file is released under the GPL v2. + * + */ + +#include <linux/kobject.h> +#include <linux/module.h> +#include <linux/dcache.h> +#include "sysfs.h" + +#define to_enumerated_attr(obj) container_of(obj, struct enumerated_attr, attr) + +struct attr_group { + struct kobject kobj; + struct kobject *attr_owner; + unsigned int idx; +}; +#define to_attr_group(obj) container_of(obj, struct attr_group, kobj) + +struct attr_array { + struct kobject kobj; + unsigned int n_elements; + struct enumerated_attr *attrs; +}; +#define to_attr_array(obj) container_of(obj, struct attr_array, kobj) + +static ssize_t +attr_group_show(struct kobject *kobj, struct attribute *attr, char *buf) +{ + struct enumerated_attr *eattr = to_enumerated_attr(attr); + struct attr_group *group = to_attr_group(kobj); + ssize_t ret = 0; + + if (eattr->show) + ret = eattr->show(group->attr_owner, group->idx, buf); + + return ret; +} + +static ssize_t +attr_group_store(struct kobject *kobj, struct attribute *attr, + const char *buf, size_t count) +{ + struct enumerated_attr *eattr = to_enumerated_attr(attr); + struct attr_group *group = to_attr_group(kobj); + ssize_t ret = 0; + + if (eattr->store) + ret = eattr->store(group->attr_owner, group->idx, buf, count); + + return ret; +} + +static struct sysfs_ops attr_group_sysfs_ops = { + .show = attr_group_show, + .store = attr_group_store, +}; + +static void attr_group_release(struct kobject *kobj) +{ + struct attr_group *group = to_attr_group(kobj); + + kobject_put(group->attr_owner); + kfree(group); +} + +static struct kobj_type ktype_attr_group = { + .release = attr_group_release, + .sysfs_ops = &attr_group_sysfs_ops, +}; + +static void attr_array_release(struct kobject *kobj) +{ + kfree(to_attr_array(kobj)); +} + +static struct kobj_type ktype_attr_array = { + .release = attr_array_release, +}; + +static int create_group(struct attr_array *array, unsigned int idx) +{ + struct attr_group *group; + struct enumerated_attr *eattr, *eattr2; + int err; + + group = kmalloc(sizeof(struct attr_group), GFP_KERNEL); + if (!group) + return -ENOMEM; + + memset(group, 0, sizeof(struct attr_group)); + kobject_set_name(&group->kobj, "%d", idx); + group->kobj.ktype = &ktype_attr_group; + group->kobj.parent = &array->kobj; + group->attr_owner = kobject_get(array->kobj.parent); + group->idx = idx; + + err = kobject_register(&group->kobj); + if (err) + goto fail1; + + for (eattr = array->attrs; attr_name(*eattr); eattr++) { + err = sysfs_create_file(&group->kobj, &eattr->attr); + if (err) + goto fail2; + } + + return 0; + + fail2: for (eattr2 = array->attrs; eattr2 != eattr; eattr2++) + sysfs_remove_file(&group->kobj, &eattr2->attr); + fail1: kfree(group); + return err; +} + +static void remove_group(struct attr_array *array, unsigned int idx) +{ + struct attr_group *group; + struct enumerated_attr *eattr; + struct dentry *dir; + char name[4]; + + snprintf(name, sizeof(name), "%d", idx); + dir = sysfs_get_dentry(array->kobj.dentry, name); + if (dir) { + group = to_attr_group(to_kobj(dir)); + for (eattr = array->attrs; attr_name(*eattr); eattr++) + sysfs_remove_file(&group->kobj, &eattr->attr); + kobject_unregister(&group->kobj); + } + + dput(dir); +} + +int sysfs_create_group_array(struct kobject *kobj, + const struct attribute_group_array *a, + unsigned int n_elements) +{ + struct attr_array *array; + unsigned int i; + int error; + + BUG_ON(!kobj || !kobj->dentry); + BUG_ON(!a->name); + + if (n_elements > 999) + return -EINVAL; + + array = kmalloc(sizeof(struct attr_array), GFP_KERNEL); + if (!array) + return -ENOMEM; + + memset(array, 0, sizeof(struct attr_array)); + array->attrs = a->attrs; + array->n_elements = n_elements; + + array->kobj.ktype = &ktype_attr_array; + array->kobj.parent = kobj; + kobject_set_name(&array->kobj, a->name); + + error = kobject_register(&array->kobj); + if (error) + goto fail; + + for (i = 0; i < n_elements; i++) { + error = create_group(array, i); + if (error) { + while (--i) + remove_group(array, i); + goto fail; + } + } + + return 0; + + fail: + kfree(array); + return error; +} + +void sysfs_remove_group_array(struct kobject *kobj, + const struct attribute_group_array *a) +{ + struct attr_array *array; + struct dentry *dir; + unsigned int i; + + dir = sysfs_get_dentry(kobj->dentry, a->name); + + if (dir) { + array = to_attr_array(to_kobj(dir)); + for (i = 0; i < array->n_elements; i++) + remove_group(array, i); + kobject_unregister(&array->kobj); + } + + dput(dir); +} + +EXPORT_SYMBOL_GPL(sysfs_create_group_array); +EXPORT_SYMBOL_GPL(sysfs_remove_group_array); [-- Attachment #4: i8k-cumulative.patch --] [-- Type: text/x-diff, Size: 32614 bytes --] Documentation/kernel-parameters.txt | 3 arch/i386/kernel/dmi_scan.c | 1 drivers/char/i8k.c | 1005 +++++++++++++++--------------------- drivers/char/misc.c | 4 include/linux/dmi.h | 1 5 files changed, 444 insertions(+), 570 deletions(-) Index: dtor/arch/i386/kernel/dmi_scan.c =================================================================== --- dtor.orig/arch/i386/kernel/dmi_scan.c +++ dtor/arch/i386/kernel/dmi_scan.c @@ -416,6 +416,7 @@ static void __init dmi_decode(struct dmi dmi_save_ident(dm, DMI_PRODUCT_VERSION, 6); dmi_printk(("Serial Number: %s\n", dmi_string(dm, data[7]))); + dmi_save_ident(dm, DMI_PRODUCT_SERIAL, 7); break; case 2: dmi_printk(("Board Vendor: %s\n", Index: dtor/Documentation/kernel-parameters.txt =================================================================== --- dtor.orig/Documentation/kernel-parameters.txt +++ dtor/Documentation/kernel-parameters.txt @@ -545,6 +545,9 @@ running once the system is up. i810= [HW,DRM] + i8k.ignore_dmi [HW] Continue probing hardware even if DMI data + indicates that the driver is running on unsupported + hardware. i8k.force [HW] Activate i8k driver even if SMM BIOS signature does not match list of supported models. i8k.power_status Index: dtor/drivers/char/misc.c =================================================================== --- dtor.orig/drivers/char/misc.c +++ dtor/drivers/char/misc.c @@ -67,7 +67,6 @@ extern int rtc_DP8570A_init(void); extern int rtc_MK48T08_init(void); extern int pmu_device_init(void); extern int tosh_init(void); -extern int i8k_init(void); #ifdef CONFIG_PROC_FS static void *misc_seq_start(struct seq_file *seq, loff_t *pos) @@ -317,9 +316,6 @@ static int __init misc_init(void) #ifdef CONFIG_TOSHIBA tosh_init(); #endif -#ifdef CONFIG_I8K - i8k_init(); -#endif if (register_chrdev(MISC_MAJOR,"misc",&misc_fops)) { printk("unable to get major %d for misc devices\n", MISC_MAJOR); Index: dtor/include/linux/dmi.h =================================================================== --- dtor.orig/include/linux/dmi.h +++ dtor/include/linux/dmi.h @@ -9,6 +9,7 @@ enum dmi_field { DMI_SYS_VENDOR, DMI_PRODUCT_NAME, DMI_PRODUCT_VERSION, + DMI_PRODUCT_SERIAL, DMI_BOARD_VENDOR, DMI_BOARD_NAME, DMI_BOARD_VERSION, Index: dtor/drivers/char/i8k.c =================================================================== --- dtor.orig/drivers/char/i8k.c +++ dtor/drivers/char/i8k.c @@ -20,13 +20,15 @@ #include <linux/types.h> #include <linux/init.h> #include <linux/proc_fs.h> -#include <linux/apm_bios.h> +#include <linux/seq_file.h> +#include <linux/dmi.h> +#include <linux/device.h> #include <asm/uaccess.h> #include <asm/io.h> #include <linux/i8k.h> -#define I8K_VERSION "1.13 14/05/2002" +#define I8K_VERSION "1.14 21/02/2005" #define I8K_SMM_FN_STATUS 0x0025 #define I8K_SMM_POWER_STATUS 0x0069 @@ -34,7 +36,8 @@ #define I8K_SMM_GET_FAN 0x00a3 #define I8K_SMM_GET_SPEED 0x02a3 #define I8K_SMM_GET_TEMP 0x10a3 -#define I8K_SMM_GET_DELL_SIG 0xffa3 +#define I8K_SMM_GET_DELL_SIG1 0xfea3 +#define I8K_SMM_GET_DELL_SIG2 0xffa3 #define I8K_SMM_BIOS_VERSION 0x00a6 #define I8K_FAN_MULT 30 @@ -52,18 +55,7 @@ #define I8K_TEMPERATURE_BUG 1 -#define DELL_SIGNATURE "Dell Computer" - -static char *supported_models[] = { - "Inspiron", - "Latitude", - NULL -}; - -static char system_vendor[48] = "?"; -static char product_name [48] = "?"; -static char bios_version [4] = "?"; -static char serial_number[16] = "?"; +static char bios_version[4]; MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)"); MODULE_DESCRIPTION("Driver for accessing SMM BIOS on Dell laptops"); @@ -73,6 +65,10 @@ static int force; module_param(force, bool, 0); MODULE_PARM_DESC(force, "Force loading without checking for supported models"); +static int ignore_dmi; +module_param(ignore_dmi, bool, 0); +MODULE_PARM_DESC(ignore_dmi, "Continue probing hardware even if DMI data does not match"); + static int restricted; module_param(restricted, bool, 0); MODULE_PARM_DESC(restricted, "Allow fan control if SYS_ADMIN capability set"); @@ -81,69 +77,76 @@ static int power_status; module_param(power_status, bool, 0600); MODULE_PARM_DESC(power_status, "Report power status in /proc/i8k"); -static ssize_t i8k_read(struct file *, char __user *, size_t, loff_t *); +static int i8k_open_fs(struct inode *inode, struct file *file); static int i8k_ioctl(struct inode *, struct file *, unsigned int, unsigned long); static struct file_operations i8k_fops = { - .read = i8k_read, - .ioctl = i8k_ioctl, + .open = i8k_open_fs, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, + .ioctl = i8k_ioctl, }; -typedef struct { - unsigned int eax; - unsigned int ebx __attribute__ ((packed)); - unsigned int ecx __attribute__ ((packed)); - unsigned int edx __attribute__ ((packed)); - unsigned int esi __attribute__ ((packed)); - unsigned int edi __attribute__ ((packed)); -} SMMRegisters; - -typedef struct { - u8 type; - u8 length; - u16 handle; -} DMIHeader; +static struct device_driver i8k_driver = { + .name = "i8k", + .bus = &platform_bus_type, +}; + +static struct platform_device *i8k_device; + +struct smm_regs { + unsigned int eax; + unsigned int ebx __attribute__ ((packed)); + unsigned int ecx __attribute__ ((packed)); + unsigned int edx __attribute__ ((packed)); + unsigned int esi __attribute__ ((packed)); + unsigned int edi __attribute__ ((packed)); +}; + +static inline char *i8k_get_dmi_data(int field) +{ + return dmi_get_system_info(field) ? : "N/A"; +} /* * Call the System Management Mode BIOS. Code provided by Jonathan Buzzard. */ -static int i8k_smm(SMMRegisters *regs) +static int i8k_smm(struct smm_regs *regs) { - int rc; - int eax = regs->eax; + int rc; + int eax = regs->eax; + + asm("pushl %%eax\n\t" + "movl 0(%%eax),%%edx\n\t" + "push %%edx\n\t" + "movl 4(%%eax),%%ebx\n\t" + "movl 8(%%eax),%%ecx\n\t" + "movl 12(%%eax),%%edx\n\t" + "movl 16(%%eax),%%esi\n\t" + "movl 20(%%eax),%%edi\n\t" + "popl %%eax\n\t" + "out %%al,$0xb2\n\t" + "out %%al,$0x84\n\t" + "xchgl %%eax,(%%esp)\n\t" + "movl %%ebx,4(%%eax)\n\t" + "movl %%ecx,8(%%eax)\n\t" + "movl %%edx,12(%%eax)\n\t" + "movl %%esi,16(%%eax)\n\t" + "movl %%edi,20(%%eax)\n\t" + "popl %%edx\n\t" + "movl %%edx,0(%%eax)\n\t" + "lahf\n\t" + "shrl $8,%%eax\n\t" + "andl $1,%%eax\n":"=a"(rc) + : "a"(regs) + : "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory"); - asm("pushl %%eax\n\t" \ - "movl 0(%%eax),%%edx\n\t" \ - "push %%edx\n\t" \ - "movl 4(%%eax),%%ebx\n\t" \ - "movl 8(%%eax),%%ecx\n\t" \ - "movl 12(%%eax),%%edx\n\t" \ - "movl 16(%%eax),%%esi\n\t" \ - "movl 20(%%eax),%%edi\n\t" \ - "popl %%eax\n\t" \ - "out %%al,$0xb2\n\t" \ - "out %%al,$0x84\n\t" \ - "xchgl %%eax,(%%esp)\n\t" - "movl %%ebx,4(%%eax)\n\t" \ - "movl %%ecx,8(%%eax)\n\t" \ - "movl %%edx,12(%%eax)\n\t" \ - "movl %%esi,16(%%eax)\n\t" \ - "movl %%edi,20(%%eax)\n\t" \ - "popl %%edx\n\t" \ - "movl %%edx,0(%%eax)\n\t" \ - "lahf\n\t" \ - "shrl $8,%%eax\n\t" \ - "andl $1,%%eax\n" \ - : "=a" (rc) - : "a" (regs) - : "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory"); - - if ((rc != 0) || ((regs->eax & 0xffff) == 0xffff) || (regs->eax == eax)) { - return -EINVAL; - } + if (rc != 0 || (regs->eax & 0xffff) == 0xffff || regs->eax == eax) + return -EINVAL; - return 0; + return 0; } /* @@ -152,24 +155,9 @@ static int i8k_smm(SMMRegisters *regs) */ static int i8k_get_bios_version(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; - - regs.eax = I8K_SMM_BIOS_VERSION; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - - return regs.eax; -} + struct smm_regs regs = { .eax = I8K_SMM_BIOS_VERSION, }; -/* - * Read the machine id. - */ -static int i8k_get_serial_number(unsigned char *buff) -{ - strlcpy(buff, serial_number, sizeof(serial_number)); - return 0; + return i8k_smm(®s) ? : regs.eax; } /* @@ -177,24 +165,22 @@ static int i8k_get_serial_number(unsigne */ static int i8k_get_fn_status(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + struct smm_regs regs = { .eax = I8K_SMM_FN_STATUS, }; + int rc; - regs.eax = I8K_SMM_FN_STATUS; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - - switch ((regs.eax >> I8K_FN_SHIFT) & I8K_FN_MASK) { - case I8K_FN_UP: - return I8K_VOL_UP; - case I8K_FN_DOWN: - return I8K_VOL_DOWN; - case I8K_FN_MUTE: - return I8K_VOL_MUTE; - default: - return 0; - } + if ((rc = i8k_smm(®s)) < 0) + return rc; + + switch ((regs.eax >> I8K_FN_SHIFT) & I8K_FN_MASK) { + case I8K_FN_UP: + return I8K_VOL_UP; + case I8K_FN_DOWN: + return I8K_VOL_DOWN; + case I8K_FN_MUTE: + return I8K_VOL_MUTE; + default: + return 0; + } } /* @@ -202,20 +188,13 @@ static int i8k_get_fn_status(void) */ static int i8k_get_power_status(void) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + struct smm_regs regs = { .eax = I8K_SMM_POWER_STATUS, }; + int rc; + + if ((rc = i8k_smm(®s)) < 0) + return rc; - regs.eax = I8K_SMM_POWER_STATUS; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - - switch (regs.eax & 0xff) { - case I8K_POWER_AC: - return I8K_AC; - default: - return I8K_BATTERY; - } + return (regs.eax & 0xff) == I8K_POWER_AC ? I8K_AC : I8K_BATTERY; } /* @@ -223,16 +202,10 @@ static int i8k_get_power_status(void) */ static int i8k_get_fan_status(int fan) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + struct smm_regs regs = { .eax = I8K_SMM_GET_FAN, }; - regs.eax = I8K_SMM_GET_FAN; - regs.ebx = fan & 0xff; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - - return (regs.eax & 0xff); + regs.ebx = fan & 0xff; + return i8k_smm(®s) ? : regs.eax & 0xff; } /* @@ -240,16 +213,10 @@ static int i8k_get_fan_status(int fan) */ static int i8k_get_fan_speed(int fan) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; - - regs.eax = I8K_SMM_GET_SPEED; - regs.ebx = fan & 0xff; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } + struct smm_regs regs = { .eax = I8K_SMM_GET_SPEED, }; - return (regs.eax & 0xffff) * I8K_FAN_MULT; + regs.ebx = fan & 0xff; + return i8k_smm(®s) ? : (regs.eax & 0xffff) * I8K_FAN_MULT; } /* @@ -257,532 +224,438 @@ static int i8k_get_fan_speed(int fan) */ static int i8k_set_fan(int fan, int speed) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + struct smm_regs regs = { .eax = I8K_SMM_SET_FAN, }; - speed = (speed < 0) ? 0 : ((speed > I8K_FAN_MAX) ? I8K_FAN_MAX : speed); + speed = (speed < 0) ? 0 : ((speed > I8K_FAN_MAX) ? I8K_FAN_MAX : speed); + regs.ebx = (fan & 0xff) | (speed << 8); - regs.eax = I8K_SMM_SET_FAN; - regs.ebx = (fan & 0xff) | (speed << 8); - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - - return (i8k_get_fan_status(fan)); + return i8k_smm(®s) ? : i8k_get_fan_status(fan); } /* * Read the cpu temperature. */ -static int i8k_get_cpu_temp(void) +static int i8k_get_temp(int sensor) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; - int temp; + struct smm_regs regs = { .eax = I8K_SMM_GET_TEMP, }; + int rc; + int temp; #ifdef I8K_TEMPERATURE_BUG - static int prev = 0; + static int prev; #endif + regs.ebx = sensor & 0xff; + if ((rc = i8k_smm(®s)) < 0) + return rc; - regs.eax = I8K_SMM_GET_TEMP; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } - temp = regs.eax & 0xff; + temp = regs.eax & 0xff; #ifdef I8K_TEMPERATURE_BUG - /* - * Sometimes the temperature sensor returns 0x99, which is out of range. - * In this case we return (once) the previous cached value. For example: - # 1003655137 00000058 00005a4b - # 1003655138 00000099 00003a80 <--- 0x99 = 153 degrees - # 1003655139 00000054 00005c52 - */ - if (temp > I8K_MAX_TEMP) { - temp = prev; - prev = I8K_MAX_TEMP; - } else { - prev = temp; - } + /* + * Sometimes the temperature sensor returns 0x99, which is out of range. + * In this case we return (once) the previous cached value. For example: + # 1003655137 00000058 00005a4b + # 1003655138 00000099 00003a80 <--- 0x99 = 153 degrees + # 1003655139 00000054 00005c52 + */ + if (temp > I8K_MAX_TEMP) { + temp = prev; + prev = I8K_MAX_TEMP; + } else { + prev = temp; + } #endif - return temp; + return temp; } -static int i8k_get_dell_signature(void) +static int i8k_get_dell_signature(int req_fn) { - SMMRegisters regs = { 0, 0, 0, 0, 0, 0 }; - int rc; + struct smm_regs regs = { .eax = req_fn, }; + int rc; - regs.eax = I8K_SMM_GET_DELL_SIG; - if ((rc=i8k_smm(®s)) < 0) { - return rc; - } + if ((rc = i8k_smm(®s)) < 0) + return rc; - if ((regs.eax == 1145651527) && (regs.edx == 1145392204)) { - return 0; - } else { - return -1; - } + return regs.eax == 1145651527 && regs.edx == 1145392204 ? 0 : -1; } static int i8k_ioctl(struct inode *ip, struct file *fp, unsigned int cmd, unsigned long arg) { - int val; - int speed; - unsigned char buff[16]; - int __user *argp = (int __user *)arg; - - if (!argp) - return -EINVAL; - - switch (cmd) { - case I8K_BIOS_VERSION: - val = i8k_get_bios_version(); - break; - - case I8K_MACHINE_ID: - memset(buff, 0, 16); - val = i8k_get_serial_number(buff); - break; - - case I8K_FN_STATUS: - val = i8k_get_fn_status(); - break; - - case I8K_POWER_STATUS: - val = i8k_get_power_status(); - break; - - case I8K_GET_TEMP: - val = i8k_get_cpu_temp(); - break; - - case I8K_GET_SPEED: - if (copy_from_user(&val, argp, sizeof(int))) { - return -EFAULT; - } - val = i8k_get_fan_speed(val); - break; + int val = 0; + int speed; + unsigned char buff[16]; + int __user *argp = (int __user *)arg; - case I8K_GET_FAN: - if (copy_from_user(&val, argp, sizeof(int))) { - return -EFAULT; - } - val = i8k_get_fan_status(val); - break; + if (!argp) + return -EINVAL; - case I8K_SET_FAN: - if (restricted && !capable(CAP_SYS_ADMIN)) { - return -EPERM; - } - if (copy_from_user(&val, argp, sizeof(int))) { - return -EFAULT; - } - if (copy_from_user(&speed, argp+1, sizeof(int))) { - return -EFAULT; - } - val = i8k_set_fan(val, speed); - break; + switch (cmd) { + case I8K_BIOS_VERSION: + val = i8k_get_bios_version(); + break; - default: - return -EINVAL; - } - - if (val < 0) { - return val; - } - - switch (cmd) { - case I8K_BIOS_VERSION: - if (copy_to_user(argp, &val, 4)) { - return -EFAULT; - } - break; - case I8K_MACHINE_ID: - if (copy_to_user(argp, buff, 16)) { - return -EFAULT; + case I8K_MACHINE_ID: + memset(buff, 0, 16); + strlcpy(buff, i8k_get_dmi_data(DMI_PRODUCT_SERIAL), sizeof(buff)); + break; + + case I8K_FN_STATUS: + val = i8k_get_fn_status(); + break; + + case I8K_POWER_STATUS: + val = i8k_get_power_status(); + break; + + case I8K_GET_TEMP: + val = i8k_get_temp(0); + break; + + case I8K_GET_SPEED: + if (copy_from_user(&val, argp, sizeof(int))) + return -EFAULT; + + val = i8k_get_fan_speed(val); + break; + + case I8K_GET_FAN: + if (copy_from_user(&val, argp, sizeof(int))) + return -EFAULT; + + val = i8k_get_fan_status(val); + break; + + case I8K_SET_FAN: + if (restricted && !capable(CAP_SYS_ADMIN)) + return -EPERM; + + if (copy_from_user(&val, argp, sizeof(int))) + return -EFAULT; + + if (copy_from_user(&speed, argp + 1, sizeof(int))) + return -EFAULT; + + val = i8k_set_fan(val, speed); + break; + + default: + return -EINVAL; } - break; - default: - if (copy_to_user(argp, &val, sizeof(int))) { - return -EFAULT; + + if (val < 0) + return val; + + switch (cmd) { + case I8K_BIOS_VERSION: + if (copy_to_user(argp, &val, 4)) + return -EFAULT; + + break; + case I8K_MACHINE_ID: + if (copy_to_user(argp, buff, 16)) + return -EFAULT; + + break; + default: + if (copy_to_user(argp, &val, sizeof(int))) + return -EFAULT; + + break; } - break; - } - return 0; + return 0; } /* * Print the information for /proc/i8k. */ -static int i8k_get_info(char *buffer, char **start, off_t fpos, int length) +static int i8k_proc_show(struct seq_file *seq, void *offset) { - int n, fn_key, cpu_temp, ac_power; - int left_fan, right_fan, left_speed, right_speed; - - cpu_temp = i8k_get_cpu_temp(); /* 11100 µs */ - left_fan = i8k_get_fan_status(I8K_FAN_LEFT); /* 580 µs */ - right_fan = i8k_get_fan_status(I8K_FAN_RIGHT); /* 580 µs */ - left_speed = i8k_get_fan_speed(I8K_FAN_LEFT); /* 580 µs */ - right_speed = i8k_get_fan_speed(I8K_FAN_RIGHT); /* 580 µs */ - fn_key = i8k_get_fn_status(); /* 750 µs */ - if (power_status) { - ac_power = i8k_get_power_status(); /* 14700 µs */ - } else { - ac_power = -1; - } - - /* - * Info: - * - * 1) Format version (this will change if format changes) - * 2) BIOS version - * 3) BIOS machine ID - * 4) Cpu temperature - * 5) Left fan status - * 6) Right fan status - * 7) Left fan speed - * 8) Right fan speed - * 9) AC power - * 10) Fn Key status - */ - n = sprintf(buffer, "%s %s %s %d %d %d %d %d %d %d\n", - I8K_PROC_FMT, - bios_version, - serial_number, - cpu_temp, - left_fan, - right_fan, - left_speed, - right_speed, - ac_power, - fn_key); - - return n; -} - -static ssize_t i8k_read(struct file *f, char __user *buffer, size_t len, loff_t *fpos) -{ - int n; - char info[128]; - - n = i8k_get_info(info, NULL, 0, 128); - if (n <= 0) { - return n; - } - - if (*fpos >= n) { - return 0; - } + int fn_key, cpu_temp, ac_power; + int left_fan, right_fan, left_speed, right_speed; - if ((*fpos + len) >= n) { - len = n - *fpos; - } + cpu_temp = i8k_get_temp(0); /* 11100 µs */ + left_fan = i8k_get_fan_status(I8K_FAN_LEFT); /* 580 µs */ + right_fan = i8k_get_fan_status(I8K_FAN_RIGHT); /* 580 µs */ + left_speed = i8k_get_fan_speed(I8K_FAN_LEFT); /* 580 µs */ + right_speed = i8k_get_fan_speed(I8K_FAN_RIGHT); /* 580 µs */ + fn_key = i8k_get_fn_status(); /* 750 µs */ + if (power_status) + ac_power = i8k_get_power_status(); /* 14700 µs */ + else + ac_power = -1; - if (copy_to_user(buffer, info, len) != 0) { - return -EFAULT; - } - - *fpos += len; - return len; + /* + * Info: + * + * 1) Format version (this will change if format changes) + * 2) BIOS version + * 3) BIOS machine ID + * 4) Cpu temperature + * 5) Left fan status + * 6) Right fan status + * 7) Left fan speed + * 8) Right fan speed + * 9) AC power + * 10) Fn Key status + */ + return seq_printf(seq, "%s %s %s %d %d %d %d %d %d %d\n", + I8K_PROC_FMT, + bios_version, + dmi_get_system_info(DMI_PRODUCT_SERIAL) ? : "N/A", + cpu_temp, + left_fan, right_fan, left_speed, right_speed, + ac_power, fn_key); } -static char* __init string_trim(char *s, int size) +static int i8k_open_fs(struct inode *inode, struct file *file) { - int len; - char *p; - - if ((len = strlen(s)) > size) { - len = size; - } + return single_open(file, i8k_proc_show, NULL); +} - for (p=s+len-1; len && (*p==' '); len--,p--) { - *p = '\0'; - } - return s; +static ssize_t i8k_sysfs_temp_show(struct kobject *kobj, unsigned int idx, char *buf) +{ + int temp = i8k_get_temp(idx); + return temp < 0 ? -EIO : sprintf(buf, "%d\n", temp); } -/* DMI code, stolen from arch/i386/kernel/dmi_scan.c */ +static struct enumerated_attr i8k_temp_attr = + __ATTR(temp, 0444, i8k_sysfs_temp_show, NULL); -/* - * |<-- dmi->length -->| - * | | - * |dmi header s=N | string1,\0, ..., stringN,\0, ..., \0 - * | | - * +-----------------------+ - */ -static char* __init dmi_string(DMIHeader *dmi, u8 s) +static ssize_t i8k_sysfs_fan_show(struct kobject *kobj, unsigned int idx, char *buf) +{ + int status = i8k_get_fan_status(idx); + return status < 0 ? -EIO : sprintf(buf, "%d\n", status); +} + +static ssize_t i8k_sysfs_fan_set(struct kobject *kobj, unsigned int idx, const char *buf, size_t count) { - u8 *p; + unsigned long state; + char *rest; - if (!s) { - return ""; - } - s--; + if (restricted && !capable(CAP_SYS_ADMIN)) + return -EPERM; - p = (u8 *)dmi + dmi->length; - while (s > 0) { - p += strlen(p); - p++; - s--; - } + state = simple_strtoul(buf, &rest, 10); + if (*rest || state > I8K_FAN_MAX) + return -EINVAL; - return p; + if (i8k_set_fan(idx, state) < 0) + return -EIO; + + return count; } -static void __init dmi_decode(DMIHeader *dmi) +static ssize_t i8k_sysfs_fan_speed_show(struct kobject *kobj, unsigned int idx, char *buf) { - u8 *data = (u8 *) dmi; - char *p; + int speed = i8k_get_fan_speed(idx); + return speed < 0 ? -EIO : sprintf(buf, "%d\n", speed); +} -#ifdef I8K_DEBUG - int i; - printk("%08x ", (int)data); - for (i=0; i<data[1] && i<64; i++) { - printk("%02x ", data[i]); - } - printk("\n"); -#endif +static struct enumerated_attr i8k_fan_attrs[] = { + __ATTR(state, 0644, i8k_sysfs_fan_show, i8k_sysfs_fan_set), + __ATTR(speed, 0444, i8k_sysfs_fan_speed_show, NULL), + __ATTR_NULL +}; - switch (dmi->type) { - case 0: /* BIOS Information */ - p = dmi_string(dmi,data[5]); - if (*p) { - strlcpy(bios_version, p, sizeof(bios_version)); - string_trim(bios_version, sizeof(bios_version)); - } - break; - case 1: /* System Information */ - p = dmi_string(dmi,data[4]); - if (*p) { - strlcpy(system_vendor, p, sizeof(system_vendor)); - string_trim(system_vendor, sizeof(system_vendor)); - } - p = dmi_string(dmi,data[5]); - if (*p) { - strlcpy(product_name, p, sizeof(product_name)); - string_trim(product_name, sizeof(product_name)); - } - p = dmi_string(dmi,data[7]); - if (*p) { - strlcpy(serial_number, p, sizeof(serial_number)); - string_trim(serial_number, sizeof(serial_number)); - } - break; - } -} +static struct attribute_group_array i8k_fan_attr_array = { + .name = "fan", + .attrs = i8k_fan_attrs, +}; -static int __init dmi_table(u32 base, int len, int num, void (*fn)(DMIHeader*)) +static ssize_t i8k_sysfs_power_status_show(struct device *dev, char *buf) { - u8 *buf; - u8 *data; - DMIHeader *dmi; - int i = 1; - - buf = ioremap(base, len); - if (buf == NULL) { - return -1; - } - data = buf; - - /* - * Stop when we see al the items the table claimed to have - * or we run off the end of the table (also happens) - */ - while ((i<num) && ((data-buf) < len)) { - dmi = (DMIHeader *)data; - /* - * Avoid misparsing crud if the length of the last - * record is crap - */ - if ((data-buf+dmi->length) >= len) { - break; - } - fn(dmi); - data += dmi->length; - /* - * Don't go off the end of the data if there is - * stuff looking like string fill past the end - */ - while (((data-buf) < len) && (*data || data[1])) { - data++; - } - data += 2; - i++; - } - iounmap(buf); - - return 0; -} - -static int __init dmi_iterate(void (*decode)(DMIHeader *)) -{ - unsigned char buf[20]; - void __iomem *p = ioremap(0xe0000, 0x20000), *q; - - if (!p) - return -1; - - for (q = p; q < p + 0x20000; q += 16) { - memcpy_fromio(buf, q, 20); - if (memcmp(buf, "_DMI_", 5)==0) { - u16 num = buf[13]<<8 | buf[12]; - u16 len = buf [7]<<8 | buf [6]; - u32 base = buf[11]<<24 | buf[10]<<16 | buf[9]<<8 | buf[8]; -#ifdef I8K_DEBUG - printk(KERN_INFO "DMI %d.%d present.\n", - buf[14]>>4, buf[14]&0x0F); - printk(KERN_INFO "%d structures occupying %d bytes.\n", - buf[13]<<8 | buf[12], - buf [7]<<8 | buf[6]); - printk(KERN_INFO "DMI table at 0x%08X.\n", - buf[11]<<24 | buf[10]<<16 | buf[9]<<8 | buf[8]); -#endif - if (dmi_table(base, len, num, decode)==0) { - iounmap(p); - return 0; - } - } - } - iounmap(p); - return -1; + int status = power_status ? i8k_get_power_status() : -1; + return status < 0 ? -EIO : sprintf(buf, "%d\n", status); } -/* end of DMI code */ -/* - * Get DMI information. - */ -static int __init i8k_dmi_probe(void) -{ - char **p; +static struct device_attribute i8k_power_status_attr = + __ATTR(power_status, 0444, i8k_sysfs_power_status_show, NULL); - if (dmi_iterate(dmi_decode) != 0) { - printk(KERN_INFO "i8k: unable to get DMI information\n"); - return -ENODEV; - } - - if (strncmp(system_vendor,DELL_SIGNATURE,strlen(DELL_SIGNATURE)) != 0) { - printk(KERN_INFO "i8k: not running on a Dell system\n"); - return -ENODEV; - } - - for (p=supported_models; ; p++) { - if (!*p) { - printk(KERN_INFO "i8k: unsupported model: %s\n", product_name); - return -ENODEV; - } - if (strncmp(product_name,*p,strlen(*p)) == 0) { - break; - } - } - return 0; -} +static struct dmi_system_id __initdata i8k_dmi_table[] = { + { + .ident = "Dell Inspiron", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer"), + DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron"), + }, + }, + { + .ident = "Dell Latitude", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer"), + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude"), + }, + }, + { + .ident = "Dell Inspiron 2", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron"), + }, + }, + { + .ident = "Dell Latitude 2", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude"), + }, + }, + { } +}; /* * Probe for the presence of a supported laptop. */ static int __init i8k_probe(void) { - char buff[4]; - int version; - int smm_found = 0; - - /* - * Get DMI information - */ - if (i8k_dmi_probe() != 0) { - printk(KERN_INFO "i8k: vendor=%s, model=%s, version=%s\n", - system_vendor, product_name, bios_version); - } - - /* - * Get SMM Dell signature - */ - if (i8k_get_dell_signature() != 0) { - printk(KERN_INFO "i8k: unable to get SMM Dell signature\n"); - } else { - smm_found = 1; - } - - /* - * Get SMM BIOS version. - */ - version = i8k_get_bios_version(); - if (version <= 0) { - printk(KERN_INFO "i8k: unable to get SMM BIOS version\n"); - } else { - smm_found = 1; - buff[0] = (version >> 16) & 0xff; - buff[1] = (version >> 8) & 0xff; - buff[2] = (version) & 0xff; - buff[3] = '\0'; + char buff[4]; + int version; + /* - * If DMI BIOS version is unknown use SMM BIOS version. + * Get DMI information */ - if (bios_version[0] == '?') { - strcpy(bios_version, buff); + if (!dmi_check_system(i8k_dmi_table)) { + if (!ignore_dmi && !force) + return -ENODEV; + + printk(KERN_INFO "i8k: not running on a supported Dell system.\n"); + printk(KERN_INFO "i8k: vendor=%s, model=%s, version=%s\n", + i8k_get_dmi_data(DMI_SYS_VENDOR), + i8k_get_dmi_data(DMI_PRODUCT_NAME), + i8k_get_dmi_data(DMI_BIOS_VERSION)); } + + strlcpy(bios_version, i8k_get_dmi_data(DMI_BIOS_VERSION), sizeof(bios_version)); + /* - * Check if the two versions match. + * Get SMM Dell signature */ - if (strncmp(buff,bios_version,sizeof(bios_version)) != 0) { - printk(KERN_INFO "i8k: BIOS version mismatch: %s != %s\n", - buff, bios_version); + if (i8k_get_dell_signature(I8K_SMM_GET_DELL_SIG1) && + i8k_get_dell_signature(I8K_SMM_GET_DELL_SIG2)) { + printk(KERN_ERR "i8k: unable to get SMM Dell signature\n"); + if (!force) + return -ENODEV; } - } - if (!smm_found && !force) { - return -ENODEV; - } + /* + * Get SMM BIOS version. + */ + version = i8k_get_bios_version(); + if (version <= 0) { + printk(KERN_WARNING "i8k: unable to get SMM BIOS version\n"); + } else { + buff[0] = (version >> 16) & 0xff; + buff[1] = (version >> 8) & 0xff; + buff[2] = (version) & 0xff; + buff[3] = '\0'; + /* + * If DMI BIOS version is unknown use SMM BIOS version. + */ + if (!dmi_get_system_info(DMI_BIOS_VERSION)) + strlcpy(bios_version, buff, sizeof(bios_version)); + + /* + * Check if the two versions match. + */ + if (strncmp(buff, bios_version, sizeof(bios_version)) != 0) + printk(KERN_WARNING "i8k: BIOS version mismatch: %s != %s\n", + buff, bios_version); + } - return 0; + return 0; } -#ifdef MODULE -static -#endif -int __init i8k_init(void) +static int __init i8k_create_sysfs_files(void) { - struct proc_dir_entry *proc_i8k; + int err, num; + + err = device_create_file(&i8k_device->dev, &i8k_power_status_attr); + if (err) + return err; - /* Are we running on an supported laptop? */ - if (i8k_probe() != 0) { - return -ENODEV; - } + for (num = 0; i8k_get_fan_status(num) >= 0; num++) + /* empty */; - /* Register the proc entry */ - proc_i8k = create_proc_info_entry("i8k", 0, NULL, i8k_get_info); - if (!proc_i8k) { - return -ENOENT; - } - proc_i8k->proc_fops = &i8k_fops; - proc_i8k->owner = THIS_MODULE; + err = sysfs_create_group_array(&i8k_device->dev.kobj, &i8k_fan_attr_array, num); + if (err) + goto fail1; - printk(KERN_INFO - "Dell laptop SMM driver v%s Massimo Dal Zotto (dz@debian.org)\n", - I8K_VERSION); + for (num = 0; i8k_get_temp(num) >= 0; num++) + /* empty */; + + err = sysfs_create_array(&i8k_device->dev.kobj, &i8k_temp_attr, num); + if (err) + goto fail2; + + return 0; - return 0; +fail2: sysfs_remove_group_array(&i8k_device->dev.kobj, &i8k_fan_attr_array); +fail1: device_remove_file(&i8k_device->dev, &i8k_power_status_attr); + return err; } -#ifdef MODULE -int init_module(void) +static void __exit i8k_remove_sysfs_files(void) { - return i8k_init(); + sysfs_remove_array(&i8k_device->dev.kobj, &i8k_temp_attr); + sysfs_remove_group_array(&i8k_device->dev.kobj, &i8k_fan_attr_array); + device_remove_file(&i8k_device->dev, &i8k_power_status_attr); } -void cleanup_module(void) +static int __init i8k_init(void) { - /* Remove the proc entry */ - remove_proc_entry("i8k", NULL); + struct proc_dir_entry *proc_i8k; + int err; - printk(KERN_INFO "i8k: module unloaded\n"); + /* Are we running on an supported laptop? */ + if (i8k_probe()) + return -ENODEV; + + /* Register the proc entry */ + proc_i8k = create_proc_entry("i8k", 0, NULL); + if (!proc_i8k) + return -ENOENT; + + proc_i8k->proc_fops = &i8k_fops; + proc_i8k->owner = THIS_MODULE; + + err = driver_register(&i8k_driver); + if (err) + goto fail1; + + i8k_device = platform_device_register_simple("i8k", -1, NULL, 0); + if (IS_ERR(i8k_device)) { + err = PTR_ERR(i8k_device); + goto fail2; + } + + err = i8k_create_sysfs_files(); + if (err) + goto fail3; + + printk(KERN_INFO + "Dell laptop SMM driver v%s Massimo Dal Zotto (dz@debian.org)\n", + I8K_VERSION); + + return 0; + +fail3: platform_device_unregister(i8k_device); +fail2: driver_unregister(&i8k_driver); +fail1: remove_proc_entry("i8k", NULL); + return err; +} + +static void __exit i8k_exit(void) +{ + i8k_remove_sysfs_files(); + platform_device_unregister(i8k_device); + driver_unregister(&i8k_driver); + remove_proc_entry("i8k", NULL); } -#endif -/* end of file */ +module_init(i8k_init); +module_exit(i8k_exit); ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-21 5:12 ` Dmitry Torokhov @ 2005-03-21 22:53 ` Frank Sorenson 2005-03-21 23:55 ` Dmitry Torokhov 0 siblings, 1 reply; 28+ messages in thread From: Frank Sorenson @ 2005-03-21 22:53 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: LKML, Valdis.Kletnieks -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Dmitry Torokhov wrote: > I have implemented arrays of groups of attributes: Works great here. The i8k-cumulative patch claimed to be malformed, but I merged it in just fine by hand. In arch/i386/kernel/dmi_scan.c, I had to EXPORT dmi_get_system_info in order to get the i8k module to load. That may have been a mistake on my end (lots of odd patches in my tree right now). I'm a little curious to see how many people are going to find they need the ignore_dmi flag versus working without it. Everything works great, and it is a big step up from the existing code. I say lets go with it. Frank - -- Frank Sorenson - KD7TZK Systems Manager, Computer Science Department Brigham Young University frank@tuxrocks.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.6 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCP1B7aI0dwg4A47wRAufNAJ9tJODed2rcndtytGCZbJHr5AQJPgCgqbA1 zWnRrePRdJ/+5K1yu5HM3jg= =OzaL -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-21 22:53 ` Frank Sorenson @ 2005-03-21 23:55 ` Dmitry Torokhov 0 siblings, 0 replies; 28+ messages in thread From: Dmitry Torokhov @ 2005-03-21 23:55 UTC (permalink / raw) To: Frank Sorenson; +Cc: LKML, Valdis.Kletnieks On Monday 21 March 2005 17:53, Frank Sorenson wrote: > Dmitry Torokhov wrote: > > I have implemented arrays of groups of attributes: > > Works great here. The i8k-cumulative patch claimed to be malformed, but > I merged it in just fine by hand. In arch/i386/kernel/dmi_scan.c, I had > to EXPORT dmi_get_system_info in order to get the i8k module to load. > That may have been a mistake on my end (lots of odd patches in my tree > right now). No, I bet I forgot to export it - I have i8k compiled in and would not notice missing export. > I'm a little curious to see how many people are going to > find they need the ignore_dmi flag versus working without it. I want this driver to be first of all safe for loading on a random box so it can be enabled by default. > > Everything works great, and it is a big step up from the existing code. > I say lets go with it. > Yep, I will add missing export and forward it to Andrew - it could use some time in -mm. Thank you very much for testing it. -- Dmitry ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-17 6:40 ` Dmitry Torokhov 2005-03-17 9:37 ` Frank Sorenson 2005-03-17 9:46 ` Frank Sorenson @ 2005-03-24 7:25 ` Greg KH 2005-03-24 7:39 ` Dmitry Torokhov 2 siblings, 1 reply; 28+ messages in thread From: Greg KH @ 2005-03-24 7:25 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Frank Sorenson, LKML, Valdis.Kletnieks On Thu, Mar 17, 2005 at 01:40:48AM -0500, Dmitry Torokhov wrote: > On Wednesday 16 March 2005 16:38, Frank Sorenson wrote: > > Okay, I replaced the sysfs_ops with ops of my own, and now all the show > > and store functions also accept the name of the attribute as a parameter. > > This lets the functions know what attribute is being accessed, and allows > > us to create attributes that share show and store functions, so things > > don't need to be defined at compile time (I feel slightly evil!). > > Hrm, can we be a little more explicit and not poke in the sysfs guts right > in the driver? What do you think about the patch below athat implements > "attribute arrays"? And I am attaching cumulative i8k patch using these > arrays so they can be tested. > > I am CC-ing Greg to see what he thinks about it. Hm, I think it's proably of limited use, right? What other code would want this (the i2c sensor code doesn't, as it's naming scheme is different.) What drivers _do_ want is a way to create attributes on the fly easily, and be able to have a "private" pointer to determine easier what file was opened (to allow a single file handler, instead of the current one-per-attribute type). thanks, greg k-h ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-24 7:25 ` Greg KH @ 2005-03-24 7:39 ` Dmitry Torokhov 2005-03-24 8:00 ` Greg KH 0 siblings, 1 reply; 28+ messages in thread From: Dmitry Torokhov @ 2005-03-24 7:39 UTC (permalink / raw) To: Greg KH; +Cc: Frank Sorenson, LKML, Valdis.Kletnieks On Thursday 24 March 2005 02:25, Greg KH wrote: > On Thu, Mar 17, 2005 at 01:40:48AM -0500, Dmitry Torokhov wrote: > > On Wednesday 16 March 2005 16:38, Frank Sorenson wrote: > > > Okay, I replaced the sysfs_ops with ops of my own, and now all the show > > > and store functions also accept the name of the attribute as a parameter. > > > This lets the functions know what attribute is being accessed, and allows > > > us to create attributes that share show and store functions, so things > > > don't need to be defined at compile time (I feel slightly evil!). > > > > Hrm, can we be a little more explicit and not poke in the sysfs guts right > > in the driver? What do you think about the patch below athat implements > > "attribute arrays"? And I am attaching cumulative i8k patch using these > > arrays so they can be tested. > > > > I am CC-ing Greg to see what he thinks about it. > > Hm, I think it's proably of limited use, right? What other code would > want this (the i2c sensor code doesn't, as it's naming scheme is > different.) > Looking at their attributes they would benefit from arrays of goups of attributes... They have: temp[1-4]_max temp[1-3]_min temp[1-3]_max_hyst It could be: temp/<n>/max min max_hyst -- Dmitry ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-24 7:39 ` Dmitry Torokhov @ 2005-03-24 8:00 ` Greg KH 2005-03-24 14:44 ` Dmitry Torokhov 0 siblings, 1 reply; 28+ messages in thread From: Greg KH @ 2005-03-24 8:00 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Frank Sorenson, LKML, Valdis.Kletnieks On Thu, Mar 24, 2005 at 02:39:32AM -0500, Dmitry Torokhov wrote: > On Thursday 24 March 2005 02:25, Greg KH wrote: > > On Thu, Mar 17, 2005 at 01:40:48AM -0500, Dmitry Torokhov wrote: > > > On Wednesday 16 March 2005 16:38, Frank Sorenson wrote: > > > > Okay, I replaced the sysfs_ops with ops of my own, and now all the show > > > > and store functions also accept the name of the attribute as a parameter. > > > > This lets the functions know what attribute is being accessed, and allows > > > > us to create attributes that share show and store functions, so things > > > > don't need to be defined at compile time (I feel slightly evil!). > > > > > > Hrm, can we be a little more explicit and not poke in the sysfs guts right > > > in the driver? What do you think about the patch below athat implements > > > "attribute arrays"? And I am attaching cumulative i8k patch using these > > > arrays so they can be tested. > > > > > > I am CC-ing Greg to see what he thinks about it. > > > > Hm, I think it's proably of limited use, right? What other code would > > want this (the i2c sensor code doesn't, as it's naming scheme is > > different.) > > > > Looking at their attributes they would benefit from arrays of goups of > attributes... They have: > > temp[1-4]_max > temp[1-3]_min > temp[1-3]_max_hyst > > It could be: > > temp/<n>/max > min > max_hyst > Yeah, but then you break the userspace api that tools are already expecting to see :( thanks, greg k-h ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-24 8:00 ` Greg KH @ 2005-03-24 14:44 ` Dmitry Torokhov 0 siblings, 0 replies; 28+ messages in thread From: Dmitry Torokhov @ 2005-03-24 14:44 UTC (permalink / raw) To: Greg KH; +Cc: Frank Sorenson, LKML, Valdis.Kletnieks On Thu, 24 Mar 2005 00:00:48 -0800, Greg KH <greg@kroah.com> wrote: > On Thu, Mar 24, 2005 at 02:39:32AM -0500, Dmitry Torokhov wrote: > > On Thursday 24 March 2005 02:25, Greg KH wrote: > > > On Thu, Mar 17, 2005 at 01:40:48AM -0500, Dmitry Torokhov wrote: > > > > On Wednesday 16 March 2005 16:38, Frank Sorenson wrote: > > > > > Okay, I replaced the sysfs_ops with ops of my own, and now all the show > > > > > and store functions also accept the name of the attribute as a parameter. > > > > > This lets the functions know what attribute is being accessed, and allows > > > > > us to create attributes that share show and store functions, so things > > > > > don't need to be defined at compile time (I feel slightly evil!). > > > > > > > > Hrm, can we be a little more explicit and not poke in the sysfs guts right > > > > in the driver? What do you think about the patch below athat implements > > > > "attribute arrays"? And I am attaching cumulative i8k patch using these > > > > arrays so they can be tested. > > > > > > > > I am CC-ing Greg to see what he thinks about it. > > > > > > Hm, I think it's proably of limited use, right? What other code would > > > want this (the i2c sensor code doesn't, as it's naming scheme is > > > different.) > > > > > > > Looking at their attributes they would benefit from arrays of goups of > > attributes... They have: > > ... > Yeah, but then you break the userspace api that tools are already > expecting to see :( > Yes, although i2c did change inreface somewhat recently - I remember I had to upgrade sensor package couple of month ago to get sensors output, so we have some precedent. Also, even if i2c decides not to use it it does not mean that at some point nobody else would use attribute arrays and arrays of groups. I am willing to bet if these would be available they could be used: /drivers/macintosh/therm_pm72.c /drivers/usb/misc/cytherm.c Attributes with a private pointer passed in would be very useful too. -- Dmitry ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-16 21:38 ` Frank Sorenson 2005-03-17 6:40 ` Dmitry Torokhov @ 2005-03-17 8:16 ` Valdis.Kletnieks 2005-03-24 7:24 ` Greg KH 1 sibling, 1 reply; 28+ messages in thread From: Valdis.Kletnieks @ 2005-03-17 8:16 UTC (permalink / raw) To: Frank Sorenson; +Cc: Dmitry Torokhov, Greg KH, LKML [-- Attachment #1: Type: text/plain, Size: 1629 bytes --] On Wed, 16 Mar 2005 14:38:50 MST, Frank Sorenson said: > Okay, I replaced the sysfs_ops with ops of my own, and now all the show > and store functions also accept the name of the attribute as a parameter. > This lets the functions know what attribute is being accessed, and allows > us to create attributes that share show and store functions, so things > don't need to be defined at compile time (I feel slightly evil!). > > This patch puts the correct number of temp sensors and fans into sysfs, > and only exposes power_status if enabled by the power_status module > parameter. Works for me: [/sys/bus/platform/drivers/i8k/i8k]2 ls -l total 0 lrwxrwxrwx 1 root root 0 Mar 17 03:02 bus -> ../../../bus/platform -r--r--r-- 1 root root 4096 Mar 17 03:02 cpu_temp -rw-r--r-- 1 root root 4096 Mar 17 03:01 detach_state lrwxrwxrwx 1 root root 0 Mar 17 03:02 driver -> ../../../bus/platform/drivers/i8k -r--r--r-- 1 root root 4096 Mar 17 03:02 fan1_speed -rw-r--r-- 1 root root 4096 Mar 17 03:02 fan1_state -r--r--r-- 1 root root 4096 Mar 17 03:02 fan2_speed -rw-r--r-- 1 root root 4096 Mar 17 03:02 fan2_state drwxr-xr-x 2 root root 0 Mar 17 03:02 power -r--r--r-- 1 root root 4096 Mar 17 03:02 power_status -r--r--r-- 1 root root 4096 Mar 17 03:02 temp1 -r--r--r-- 1 root root 4096 Mar 17 03:02 temp2 The valyes of the fan* settings, and cpu_temp match what's reported in /proc/i8k. temp1 is the same as cpu_temp. temp2 is running about 7C higher and is still unidentified (though probably the NVidia chip). I'll give Dmitry's sysfs/array stuff a test tomorrow-ish unless Greg has comments before then. [-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --] ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-17 8:16 ` Valdis.Kletnieks @ 2005-03-24 7:24 ` Greg KH 2005-04-13 6:33 ` Dmitry Torokhov 0 siblings, 1 reply; 28+ messages in thread From: Greg KH @ 2005-03-24 7:24 UTC (permalink / raw) To: Valdis.Kletnieks; +Cc: Frank Sorenson, Dmitry Torokhov, LKML On Thu, Mar 17, 2005 at 03:16:24AM -0500, Valdis.Kletnieks@vt.edu wrote: > On Wed, 16 Mar 2005 14:38:50 MST, Frank Sorenson said: > > Okay, I replaced the sysfs_ops with ops of my own, and now all the show > > and store functions also accept the name of the attribute as a parameter. > > This lets the functions know what attribute is being accessed, and allows > > us to create attributes that share show and store functions, so things > > don't need to be defined at compile time (I feel slightly evil!). > > > > This patch puts the correct number of temp sensors and fans into sysfs, > > and only exposes power_status if enabled by the power_status module > > parameter. > > Works for me: > > [/sys/bus/platform/drivers/i8k/i8k]2 ls -l > total 0 > lrwxrwxrwx 1 root root 0 Mar 17 03:02 bus -> ../../../bus/platform > -r--r--r-- 1 root root 4096 Mar 17 03:02 cpu_temp > -rw-r--r-- 1 root root 4096 Mar 17 03:01 detach_state > lrwxrwxrwx 1 root root 0 Mar 17 03:02 driver -> ../../../bus/platform/drivers/i8k > -r--r--r-- 1 root root 4096 Mar 17 03:02 fan1_speed > -rw-r--r-- 1 root root 4096 Mar 17 03:02 fan1_state > -r--r--r-- 1 root root 4096 Mar 17 03:02 fan2_speed > -rw-r--r-- 1 root root 4096 Mar 17 03:02 fan2_state > drwxr-xr-x 2 root root 0 Mar 17 03:02 power > -r--r--r-- 1 root root 4096 Mar 17 03:02 power_status > -r--r--r-- 1 root root 4096 Mar 17 03:02 temp1 > -r--r--r-- 1 root root 4096 Mar 17 03:02 temp2 > > The valyes of the fan* settings, and cpu_temp match what's reported in /proc/i8k. Please match the same units and filename as the other i2c sensors. See the documentation in the Documentation/i2c/ directory for what that standard is, so userspace programs will "just work" with your devices. thanks, greg k-h ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-03-24 7:24 ` Greg KH @ 2005-04-13 6:33 ` Dmitry Torokhov 2005-04-13 8:00 ` Greg KH 0 siblings, 1 reply; 28+ messages in thread From: Dmitry Torokhov @ 2005-04-13 6:33 UTC (permalink / raw) To: Greg KH; +Cc: Valdis.Kletnieks, Frank Sorenson, LKML On Thursday 24 March 2005 02:24, Greg KH wrote: > On Thu, Mar 17, 2005 at 03:16:24AM -0500, Valdis.Kletnieks@vt.edu wrote: > > On Wed, 16 Mar 2005 14:38:50 MST, Frank Sorenson said: > > > Okay, I replaced the sysfs_ops with ops of my own, and now all the show > > > and store functions also accept the name of the attribute as a parameter. > > > This lets the functions know what attribute is being accessed, and allows > > > us to create attributes that share show and store functions, so things > > > don't need to be defined at compile time (I feel slightly evil!). > > > > > > This patch puts the correct number of temp sensors and fans into sysfs, > > > and only exposes power_status if enabled by the power_status module > > > parameter. > > > > Works for me: > > > > [/sys/bus/platform/drivers/i8k/i8k]2 ls -l > > total 0 > > lrwxrwxrwx 1 root root 0 Mar 17 03:02 bus -> ../../../bus/platform > > -r--r--r-- 1 root root 4096 Mar 17 03:02 cpu_temp > > -rw-r--r-- 1 root root 4096 Mar 17 03:01 detach_state > > lrwxrwxrwx 1 root root 0 Mar 17 03:02 driver -> ../../../bus/platform/drivers/i8k > > -r--r--r-- 1 root root 4096 Mar 17 03:02 fan1_speed > > -rw-r--r-- 1 root root 4096 Mar 17 03:02 fan1_state > > -r--r--r-- 1 root root 4096 Mar 17 03:02 fan2_speed > > -rw-r--r-- 1 root root 4096 Mar 17 03:02 fan2_state > > drwxr-xr-x 2 root root 0 Mar 17 03:02 power > > -r--r--r-- 1 root root 4096 Mar 17 03:02 power_status > > -r--r--r-- 1 root root 4096 Mar 17 03:02 temp1 > > -r--r--r-- 1 root root 4096 Mar 17 03:02 temp2 > > > > The valyes of the fan* settings, and cpu_temp match what's reported in /proc/i8k. > > Please match the same units and filename as the other i2c sensors. See > the documentation in the Documentation/i2c/ directory for what that > standard is, so userspace programs will "just work" with your devices. > Greg, I almost started doing what you just said but then I realized that none of the programs will "just work" because all of them will look into /sys/bus/i2c instead of /sys/bus/platform/i8k. For userspace tools to work transparently we would need something like /sys/class/sensor/{fan|temp|current}, but it is something I am not ready to do now - I need to finish input layer first. So given above I think having private scheme for now is ok. Sooo... Can I get my attributes goups patch in so I can use it in i8k, please? Thanks! -- Dmitry ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/5] I8K driver facelift 2005-04-13 6:33 ` Dmitry Torokhov @ 2005-04-13 8:00 ` Greg KH 0 siblings, 0 replies; 28+ messages in thread From: Greg KH @ 2005-04-13 8:00 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Valdis.Kletnieks, Frank Sorenson, LKML On Wed, Apr 13, 2005 at 01:33:31AM -0500, Dmitry Torokhov wrote: > On Thursday 24 March 2005 02:24, Greg KH wrote: > > On Thu, Mar 17, 2005 at 03:16:24AM -0500, Valdis.Kletnieks@vt.edu wrote: > > > On Wed, 16 Mar 2005 14:38:50 MST, Frank Sorenson said: > > > > Okay, I replaced the sysfs_ops with ops of my own, and now all the show > > > > and store functions also accept the name of the attribute as a parameter. > > > > This lets the functions know what attribute is being accessed, and allows > > > > us to create attributes that share show and store functions, so things > > > > don't need to be defined at compile time (I feel slightly evil!). > > > > > > > > This patch puts the correct number of temp sensors and fans into sysfs, > > > > and only exposes power_status if enabled by the power_status module > > > > parameter. > > > > > > Works for me: > > > > > > [/sys/bus/platform/drivers/i8k/i8k]2 ls -l > > > total 0 > > > lrwxrwxrwx 1 root root 0 Mar 17 03:02 bus -> ../../../bus/platform > > > -r--r--r-- 1 root root 4096 Mar 17 03:02 cpu_temp > > > -rw-r--r-- 1 root root 4096 Mar 17 03:01 detach_state > > > lrwxrwxrwx 1 root root 0 Mar 17 03:02 driver -> ../../../bus/platform/drivers/i8k > > > -r--r--r-- 1 root root 4096 Mar 17 03:02 fan1_speed > > > -rw-r--r-- 1 root root 4096 Mar 17 03:02 fan1_state > > > -r--r--r-- 1 root root 4096 Mar 17 03:02 fan2_speed > > > -rw-r--r-- 1 root root 4096 Mar 17 03:02 fan2_state > > > drwxr-xr-x 2 root root 0 Mar 17 03:02 power > > > -r--r--r-- 1 root root 4096 Mar 17 03:02 power_status > > > -r--r--r-- 1 root root 4096 Mar 17 03:02 temp1 > > > -r--r--r-- 1 root root 4096 Mar 17 03:02 temp2 > > > > > > The valyes of the fan* settings, and cpu_temp match what's reported in /proc/i8k. > > > > Please match the same units and filename as the other i2c sensors. See > > the documentation in the Documentation/i2c/ directory for what that > > standard is, so userspace programs will "just work" with your devices. > > > > Greg, > > I almost started doing what you just said but then I realized that none of > the programs will "just work" because all of them will look into /sys/bus/i2c > instead of /sys/bus/platform/i8k. > > For userspace tools to work transparently we would need something like > /sys/class/sensor/{fan|temp|current}, but it is something I am not ready to do > now - I need to finish input layer first. The sensors developers are creating just such a class, see their hwmon patch in their email archive if you are curious. > So given above I think having private scheme for now is ok. Sooo... Can I get > my attributes goups patch in so I can use it in i8k, please? Ick, no, use the upcoming hwmon stuff instead :) thanks, greg k-h ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2005-04-13 8:01 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-02-24 6:10 [PATCH 0/5] I8K driver facelift Dmitry Torokhov
2005-02-24 6:11 ` [PATCH 1/5] I8K - pass though Lindent Dmitry Torokhov
2005-02-24 6:12 ` [PATCH 2/5] I8K - use standard DMI functions Dmitry Torokhov
2005-02-24 6:12 ` [PATCH 3/5] I8K - switch to seq_file Dmitry Torokhov
2005-02-24 6:14 ` [PATCH 4/5] I8K - switch to module_{init|exit} Dmitry Torokhov
2005-02-24 6:14 ` [PATCH 5/5] I8K - convert to platform device (sysfs) Dmitry Torokhov
2005-03-13 3:41 ` [PATCH 0/5] I8K driver facelift Frank Sorenson
2005-03-13 3:59 ` Dmitry Torokhov
2005-03-15 8:12 ` Valdis.Kletnieks
2005-03-15 10:59 ` Giuseppe Bilotta
2005-03-15 17:30 ` Valdis.Kletnieks
2005-03-15 22:34 ` Frank Sorenson
2005-03-16 21:38 ` Frank Sorenson
2005-03-17 6:40 ` Dmitry Torokhov
2005-03-17 9:37 ` Frank Sorenson
2005-03-17 15:05 ` Dmitry Torokhov
2005-03-17 9:46 ` Frank Sorenson
2005-03-21 5:12 ` Dmitry Torokhov
2005-03-21 22:53 ` Frank Sorenson
2005-03-21 23:55 ` Dmitry Torokhov
2005-03-24 7:25 ` Greg KH
2005-03-24 7:39 ` Dmitry Torokhov
2005-03-24 8:00 ` Greg KH
2005-03-24 14:44 ` Dmitry Torokhov
2005-03-17 8:16 ` Valdis.Kletnieks
2005-03-24 7:24 ` Greg KH
2005-04-13 6:33 ` Dmitry Torokhov
2005-04-13 8:00 ` Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox