Skip to contentSkip to conversation

SuperIO locks coordinator - use in other hwmon/*.c

PATCH In review Jim Cromie 4 participants 18 messages Oct 15, 2007 – Dec 13, 2007

Subsystems (8)
lore
Share & tools
lore

Jim Cromie

author maintainer
#0 lore raw

PATCH 5/5 SuperIO locks coordinator - use in other hwmon/*.c

05 - use superio-locks in rest of drivers/hwmon/*.c

this patch is compile-tested only, please review for sanity before you try running them. Things to look for - missing superio_release(), opportunities to use superio_devid(), superio_inw(), etc.

---
hwmon-superio-others

 Kconfig      |    6 +++
 f71805f.c    |   86 +++++++++++++++-------------------------------
 it87.c       |   80 +++++++++++++++---------------------------
 smsc47b397.c |   63 ++++++++++++---------------------
 smsc47m1.c   |   69 +++++++++++++------------------------
 vt1211.c     |   70 +++++++++++++------------------------
 w83627ehf.c  |  110 +++++++++++++++++++++++++----------------------------------
 7 files changed, 188 insertions(+), 296 deletions(-)
drivers/hwmon/f71805f.c +29 -57
diff -ruNp -X dontdiff -X exclude-diffs hwmon-fan-push-offset/drivers/hwmon/f71805f.c hwmon-superio.old/drivers/hwmon/f71805f.c
@@ -39,6 +39,7 @@
 #include <linux/mutex.h>
 #include <linux/sysfs.h>
 #include <linux/ioport.h>
+#include <linux/superio-locks.h>
 #include <asm/io.h>
 
 static struct platform_device *pdev;
@@ -52,8 +53,6 @@ enum kinds { f71805f, f71872f };
 
 #define F71805F_LD_HWM		0x04
 
-#define SIO_REG_LDSEL		0x07	/* Logical device select */
-#define SIO_REG_DEVID		0x20	/* Device ID (2 bytes) */
 #define SIO_REG_DEVREV		0x22	/* Device revision */
 #define SIO_REG_MANID		0x23	/* Fintek ID (2 bytes) */
 #define SIO_REG_FNSEL1		0x29	/* Multi Function Select 1 (F71872F) */
@@ -64,43 +63,15 @@ enum kinds { f71805f, f71872f };
 #define SIO_F71805F_ID		0x0406
 #define SIO_F71872F_ID		0x0341
 
-static inline int
-superio_inb(int base, int reg)
-{
-	outb(reg, base);
-	return inb(base + 1);
-}
-
-static int
-superio_inw(int base, int reg)
-{
-	int val;
-	outb(reg++, base);
-	val = inb(base + 1) << 8;
-	outb(reg, base);
-	val |= inb(base + 1);
-	return val;
-}
+static struct superio* gate;
 
-static inline void
-superio_select(int base, int ld)
-{
-	outb(SIO_REG_LDSEL, base);
-	outb(ld, base + 1);
-}
-
-static inline void
-superio_enter(int base)
-{
-	outb(0x87, base);
-	outb(0x87, base);
-}
-
-static inline void
-superio_exit(int base)
-{
-	outb(0xaa, base);
-}
+static __devinit struct superio_search where = {
+	.cmdreg_addrs = { 0x2e, 0x4e },
+	.device_ids   = { SIO_F71805F_ID, SIO_F71872F_ID, 0 },
+	.devid_word   = 1,
+	.enter_seq    = { 0x87, 0x87 },
+	.exit_seq     = { 0xAA }
+};
 
 /*
  * ISA constants
@@ -1480,31 +1451,33 @@ exit:
 	return err;
 }
 
-static int __init f71805f_find(int sioaddr, unsigned short *address,
-			       struct f71805f_sio_data *sio_data)
+static int __init f71805f_find(struct f71805f_sio_data *sio_data)
 {
 	int err = -ENODEV;
-	u16 devid;
+	u16 devid, address;
 
 	static const char *names[] = {
 		"F71805F/FG",
 		"F71872F/FG or F71806F/FG",
 	};
-
-	superio_enter(sioaddr);
-
-	devid = superio_inw(sioaddr, SIO_REG_MANID);
+	gate = superio_find(&where);
+	if (!gate) {
+		printk(KERN_WARNING "pc87360: superio port not detected, "
+		       "module not intalled.\n");
+		return -ENODEV;
+	}
+	superio_enter(gate);
+	devid = superio_inw(gate, SIO_REG_MANID);
 	if (devid != SIO_FINTEK_ID)
 		goto exit;
 
-	devid = superio_inw(sioaddr, SIO_REG_DEVID);
 	switch (devid) {
 	case SIO_F71805F_ID:
 		sio_data->kind = f71805f;
 		break;
 	case SIO_F71872F_ID:
 		sio_data->kind = f71872f;
-		sio_data->fnsel1 = superio_inb(sioaddr, SIO_REG_FNSEL1);
+		sio_data->fnsel1 = superio_inb(gate, SIO_REG_FNSEL1);
 		break;
 	default:
 		printk(KERN_INFO DRVNAME ": Unsupported Fintek device, "
@@ -1512,28 +1485,28 @@ static int __init f71805f_find(int sioad
 		goto exit;
 	}
 
-	superio_select(sioaddr, F71805F_LD_HWM);
-	if (!(superio_inb(sioaddr, SIO_REG_ENABLE) & 0x01)) {
+	superio_select(gate, F71805F_LD_HWM);
+	if (!(superio_inb(gate, SIO_REG_ENABLE) & 0x01)) {
 		printk(KERN_WARNING DRVNAME ": Device not activated, "
 		       "skipping\n");
 		goto exit;
 	}
 
-	*address = superio_inw(sioaddr, SIO_REG_ADDR);
-	if (*address = 0) {
+	address = superio_inw(gate, SIO_REG_ADDR);
+	if (address = 0) {
 		printk(KERN_WARNING DRVNAME ": Base address not set, "
 		       "skipping\n");
 		goto exit;
 	}
-	*address &= ~(REGION_LENGTH - 1);	/* Ignore 3 LSB */
+	address &= ~(REGION_LENGTH - 1);	/* Ignore 3 LSB */
 
 	err = 0;
 	printk(KERN_INFO DRVNAME ": Found %s chip at %#x, revision %u\n",
-	       names[sio_data->kind], *address,
-	       superio_inb(sioaddr, SIO_REG_DEVREV));
+	       names[sio_data->kind], address,
+	       superio_inb(gate, SIO_REG_DEVREV));
 
 exit:
-	superio_exit(sioaddr);
+	superio_exit(gate);
 	return err;
 }
 
@@ -1543,8 +1516,7 @@ static int __init f71805f_init(void)
 	unsigned short address;
 	struct f71805f_sio_data sio_data;
 
-	if (f71805f_find(0x2e, &address, &sio_data)
-	 && f71805f_find(0x4e, &address, &sio_data))
+	if (f71805f_find(&sio_data))
 		return -ENODEV;
 
 	err = platform_driver_register(&f71805f_driver);
drivers/hwmon/it87.c +29 -51

134 lines not on this page · raw message

drivers/hwmon/Kconfig +6 -0
diff -ruNp -X dontdiff -X exclude-diffs hwmon-fan-push-offset/drivers/hwmon/Kconfig hwmon-superio.old/drivers/hwmon/Kconfig
@@ -218,6 +218,7 @@ config SENSORS_DS1621
 config SENSORS_F71805F
 	tristate "Fintek F71805F/FG, F71806F/FG and F71872F/FG"
 	depends on EXPERIMENTAL
+	select SUPERIO_LOCKS
 	help
 	  If you say yes here you get support for hardware monitoring
 	  features of the Fintek F71805F/FG, F71806F/FG and F71872F/FG
@@ -325,6 +328,7 @@ config SENSORS_IBMPEX
 config SENSORS_IT87
 	tristate "ITE IT87xx and compatibles"
 	select HWMON_VID
+	select SUPERIO_LOCKS
 	help
 	  If you say yes here you get support for ITE IT8705F, IT8712F,
 	  IT8716F, IT8718F and IT8726F sensor chips, and the SiS960 clone.
@@ -554,6 +564,7 @@ config SENSORS_SMSC47M192
 	tristate "SMSC LPC47M192 and compatibles"
 	depends on I2C && EXPERIMENTAL
 	select HWMON_VID
+	select SUPERIO_LOCKS
 	help
 	  If you say yes here you get support for the temperature and
 	  voltage sensors of the SMSC LPC47M192, LPC47M15x, LPC47M292
@@ -570,6 +581,7 @@ config SENSORS_SMSC47M192
 config SENSORS_SMSC47B397
 	tristate "SMSC LPC47B397-NC"
 	depends on EXPERIMENTAL
+	select SUPERIO_LOCKS
 	help
 	  If you say yes here you get support for the SMSC LPC47B397-NC
 	  sensor chip.
@@ -601,6 +613,7 @@ config SENSORS_VT1211
 	tristate "VIA VT1211"
 	depends on EXPERIMENTAL
 	select HWMON_VID
+	select SUPERIO_LOCKS
 	help
 	  If you say yes here then you get support for hardware monitoring
 	  features of the VIA VT1211 Super-I/O chip.
@@ -686,6 +700,7 @@ config SENSORS_W83627HF
 config SENSORS_W83627EHF
 	tristate "Winbond W83627EHF/DHG"
 	select HWMON_VID
+	select SUPERIO_LOCKS
 	help
 	  If you say yes here you get support for the hardware
 	  monitoring functionality of the Winbond W83627EHF Super-I/O chip.
drivers/hwmon/smsc47b397.c +24 -39

99 lines not on this page · raw message

drivers/hwmon/smsc47m1.c +26 -43

102 lines not on this page · raw message

drivers/hwmon/vt1211.c +26 -44

114 lines not on this page · raw message

drivers/hwmon/w83627ehf.c +48 -62

193 lines not on this page · raw message

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#1 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

Hi Jim,

Quoted 10 lines · Jim Cromie

05 - use superio-locks in rest of drivers/hwmon/*.c

this patch is compile-tested only, please review for sanity before you try running them. Things to look for - missing superio_release(), opportunities to use superio_devid(), superio_inw(), etc.

---
hwmon-superio-others

Quoted 7 lines
 Kconfig      |    6 +++
 f71805f.c    |   86 +++++++++++++++-------------------------------
 it87.c       |   80 +++++++++++++++---------------------------
 smsc47b397.c |   63 ++++++++++++---------------------
 smsc47m1.c   |   69 +++++++++++++------------------------
 vt1211.c     |   70 +++++++++++++------------------------
 w83627ehf.c  |  110 +++++++++++++++++++++++++----------------------------------

I'll test on w83627ehf hardware and report.

David

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#2 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie

Hi Jim, Axel,

Quoted 21 lines · David Hubbard

Hi Jim,

Quoted 10 lines · Jim Cromie

05 - use superio-locks in rest of drivers/hwmon/*.c

this patch is compile-tested only, please review for sanity before you try running them. Things to look for - missing superio_release(), opportunities to use superio_devid(), superio_inw(), etc.

---
hwmon-superio-others

Quoted 7 lines
 Kconfig      |    6 +++
 f71805f.c    |   86 +++++++++++++++-------------------------------
 it87.c       |   80 +++++++++++++++---------------------------
 smsc47b397.c |   63 ++++++++++++---------------------
 smsc47m1.c   |   69 +++++++++++++------------------------
 vt1211.c     |   70 +++++++++++++------------------------
 w83627ehf.c  |  110 +++++++++++++++++++++++++----------------------------------
Quoted 3 lines

I'll test on w83627ehf hardware and report.

David

This appears to be pasted in and needs to be updated:
printk(KERN_WARNING "pc87360: superio port not detected, "

I am unable to clone the hwmon.git repository. I believe the git port is blocked by my ISP, and I may be able to get it opened, but I tried cloning this way, and failed:

$ git clone http://lm-sensors.org/kernel?p=kernel/mhoffman/hwmon-2.6.git
hwmon.git
Initialized empty Git repository in hwmon.git/.git/
Cannot get remote repository information.
Perhaps git-update-server-info needs to be run there?

Axel, is this something easy to fix on the server side?

Thanks,
David

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#3 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie
Quoted 35 lines · David Hubbard

Hi Jim, Axel,

Quoted 21 lines · David Hubbard

Hi Jim,

Quoted 10 lines · Jim Cromie

05 - use superio-locks in rest of drivers/hwmon/*.c

this patch is compile-tested only, please review for sanity before you try running them. Things to look for - missing superio_release(), opportunities to use superio_devid(), superio_inw(), etc.

---
hwmon-superio-others

Quoted 7 lines
 Kconfig      |    6 +++
 f71805f.c    |   86 +++++++++++++++-------------------------------
 it87.c       |   80 +++++++++++++++---------------------------
 smsc47b397.c |   63 ++++++++++++---------------------
 smsc47m1.c   |   69 +++++++++++++------------------------
 vt1211.c     |   70 +++++++++++++------------------------
 w83627ehf.c  |  110 +++++++++++++++++++++++++----------------------------------
Quoted 3 lines

I'll test on w83627ehf hardware and report.

David

This appears to be pasted in and needs to be updated:
printk(KERN_WARNING "pc87360: superio port not detected, "

I am unable to clone the hwmon.git repository. I believe the git port is blocked by my ISP, and I may be able to get it opened, but I tried cloning this way, and failed:

Quoted 7 lines
$ git clone http://lm-sensors.org/kernel?p=kernel/mhoffman/hwmon-2.6.git
hwmon.git
Initialized empty Git repository in hwmon.git/.git/
Cannot get remote repository information.
Perhaps git-update-server-info needs to be run there?

Axel, is this something easy to fix on the server side?

Looks more like a broken URL, where did you get the above? The correct one is:
git://lm-sensors.org/kernel/mhoffman/hwmon-2.6.git

HTH

Signature · 1 line
Axel.Thimm at ATrpms.net

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#4 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie

Hi Axel,

Quoted 44 lines · Axel Thimm
Quoted 35 lines · David Hubbard

Hi Jim, Axel,

Quoted 21 lines · David Hubbard

Hi Jim,

Quoted 10 lines · Jim Cromie

05 - use superio-locks in rest of drivers/hwmon/*.c

this patch is compile-tested only, please review for sanity before you try running them. Things to look for - missing superio_release(), opportunities to use superio_devid(), superio_inw(), etc.

---
hwmon-superio-others

Quoted 7 lines
 Kconfig      |    6 +++
 f71805f.c    |   86 +++++++++++++++-------------------------------
 it87.c       |   80 +++++++++++++++---------------------------
 smsc47b397.c |   63 ++++++++++++---------------------
 smsc47m1.c   |   69 +++++++++++++------------------------
 vt1211.c     |   70 +++++++++++++------------------------
 w83627ehf.c  |  110 +++++++++++++++++++++++++----------------------------------
Quoted 3 lines

I'll test on w83627ehf hardware and report.

David

This appears to be pasted in and needs to be updated:
printk(KERN_WARNING "pc87360: superio port not detected, "

I am unable to clone the hwmon.git repository. I believe the git port is blocked by my ISP, and I may be able to get it opened, but I tried cloning this way, and failed:

Quoted 7 lines
$ git clone http://lm-sensors.org/kernel?p=kernel/mhoffman/hwmon-2.6.git
hwmon.git
Initialized empty Git repository in hwmon.git/.git/
Cannot get remote repository information.
Perhaps git-update-server-info needs to be run there?

Axel, is this something easy to fix on the server side?

Quoted 7 lines

Looks more like a broken URL, where did you get the above? The correct one is:
git://lm-sensors.org/kernel/mhoffman/hwmon-2.6.git

HTH
--
Axel.Thimm at ATrpms.net

I can't connect to any git servers on the git port. Can you please look at the http url?

David

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#5 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie

Hi Axel,

Quoted 42 lines · Axel Thimm
Quoted 41 lines · David Hubbard

Hi Jim, Axel,

Quoted 21 lines · David Hubbard

Hi Jim,

Quoted 10 lines · Jim Cromie

05 - use superio-locks in rest of drivers/hwmon/*.c

this patch is compile-tested only, please review for sanity before you try running them. Things to look for - missing superio_release(), opportunities to use superio_devid(), superio_inw(), etc.

---
hwmon-superio-others

Quoted 7 lines
 Kconfig      |    6 +++
 f71805f.c    |   86 +++++++++++++++-------------------------------
 it87.c       |   80 +++++++++++++++---------------------------
 smsc47b397.c |   63 ++++++++++++---------------------
 smsc47m1.c   |   69 +++++++++++++------------------------
 vt1211.c     |   70 +++++++++++++------------------------
 w83627ehf.c  |  110 +++++++++++++++++++++++++----------------------------------
Quoted 3 lines

I'll test on w83627ehf hardware and report.

David

This appears to be pasted in and needs to be updated:
printk(KERN_WARNING "pc87360: superio port not detected, "

I am unable to clone the hwmon.git repository. I believe the git port is blocked by my ISP, and I may be able to get it opened, but I tried cloning this way, and failed:

$ git clone http://lm-sensors.org/kernel?p=kernel/mhoffman/hwmon-2.6.git
hwmon.git
Initialized empty Git repository in hwmon.git/.git/
Cannot get remote repository information.
Perhaps git-update-server-info needs to be run there?
Quoted 7 lines
Quoted 1 line

Axel, is this something easy to fix on the server side?

Looks more like a broken URL, where did you get the above? The correct one is:
git://lm-sensors.org/kernel/mhoffman/hwmon-2.6.git

HTH

I can't connect to any git servers on the git port. Can you please look at the http url?

David

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#6 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie
Quoted 11 lines · David Hubbard
Quoted 8 lines
Quoted 2 lines
$ git clone http://lm-sensors.org/kernel?p=kernel/mhoffman/hwmon-2.6.git
hwmon.git

Looks more like a broken URL, where did you get the above? The correct one is:
git://lm-sensors.org/kernel/mhoffman/hwmon-2.6.git

HTH

I can't connect to any git servers on the git port. Can you please look at the http url?

There is no http URL and the git server/port is working - I just checked again. Try

nmap -sT -p9418 lm-sensors.org

Signature · 1 line
Axel.Thimm at ATrpms.net

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#7 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie

Hi Axel,

Thank you for checking the server, but would you be so kind as to enable http access? I am trying to convince my ISP to unblock the git port, but I may not be able to convince them.

$ sudo nmap -sT -p9418 lm-sensors.org

Starting Nmap 4.20 ( http://insecure.org ) at 2007-10-15 14:21 PDT Interesting ports on teufli.physik.fu-berlin.de (130.133.35.29):
PORT STATE SERVICE
9418/tcp filtered unknown

Nmap finished: 1 IP address (1 host up) scanned in 13.709 seconds

$ git clone git://lm-sensors.org/kernel/mhoffman/hwmon-2.6.git
 hwmon.git
Initialized empty Git repository in /usr/src/patches/hwmon.git/.git/
lm-sensors.org[0: 130.133.35.29]: errno=Connection timed out
fatal: unable to connect a socket (Connection timed out)
fetch-pack from 'git://lm-sensors.org/kernel/mhoffman/hwmon-2.6.git' failed.

I'm fairly certain it's my ISP. They block a large number of ports, "to prevent file sharing."

David

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#8 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie

Hi David:

Quoted 8 lines
Quoted 8 lines · David Hubbard
Quoted 7 lines
$ git clone http://lm-sensors.org/kernel?p=kernel/mhoffman/hwmon-2.6.git
hwmon.git
Initialized empty Git repository in hwmon.git/.git/
Cannot get remote repository information.
Perhaps git-update-server-info needs to be run there?

Axel, is this something easy to fix on the server side?

That's a gitweb URL, for browsers. You can't 'git clone' from that.

Quoted 4 lines · Axel Thimm
Quoted 3 lines

Looks more like a broken URL, where did you get the above? The correct one is:
git://lm-sensors.org/kernel/mhoffman/hwmon-2.6.git

This is correct. Also, I gave some step-by-step instructions in an earlier message to Hans:

http://lists.lm-sensors.org/pipermail/lm-sensors/2007-October/021440.html

Quoted 2 lines · David Hubbard

I can't connect to any git servers on the git port. Can you please look at the http url?

At the moment, you can get by without git by grabbing the 2.6.23-git8 patch from kernel.org. That includes all the patches I've accepted up to now.

Regards,

Signature · 8 lines
Mark M. Hoffman
[email protected]


_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
#9 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie

Hi Mark,

Quoted 12 lines · Mark M. Hoffman

Hi David:

That's a gitweb URL, for browsers. You can't 'git clone' from that.

Quoted 4 lines · Axel Thimm
Quoted 3 lines

Looks more like a broken URL, where did you get the above? The correct one is:
git://lm-sensors.org/kernel/mhoffman/hwmon-2.6.git

This is correct. Also, I gave some step-by-step instructions in an earlier message to Hans:

Quoted 8 lines

http://lists.lm-sensors.org/pipermail/lm-sensors/2007-October/021440.html

Quoted 2 lines · David Hubbard

I can't connect to any git servers on the git port. Can you please look at the http url?

At the moment, you can get by without git by grabbing the 2.6.23-git8 patch from kernel.org. That includes all the patches I've accepted up to now.

Thank you. I did that.

David

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#10 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie

Hi Jim,

Three things:

1) This appears to be pasted in and needs to be updated for each driver:
printk(KERN_WARNING "pc87360: superio port not detected, "

I found it in the following places in patch 5/5:

$ grep -n pc87360 jim_05.patch
107:+           printk(KERN_WARNING "pc87360: superio port not detected, "
257:+           printk(KERN_WARNING "pc87360: superio port not detected, "
443:+           printk(KERN_WARNING "pc87360: superio port not detected, "
547:+           printk(KERN_WARNING "pc87360: superio port not detected, "
834:+           printk(KERN_WARNING "pc87360: superio port not detected, "

2) There's a compile warning in drivers/hwmon/superio_locks.c:
CC [M] drivers/hwmon/superio_locks.o
drivers/hwmon/superio_locks.c: In function 'superio_probe_reserve':
drivers/hwmon/superio_locks.c:44: warning: unused variable 'rc'

3) superio_find() does not correctly find the w83627ehf:

# modprobe w83627ehf
[   84.162869] initializing with 3 reservation slots
[   84.188669] got devid 3, want 8850
[   84.188671] no devid:8850 at port:2e
[   84.188684] got devid 3, want 8860
[   84.188685] no devid:8860 at port:2e
[   84.188698] got devid 3, want a020
[   84.188700] no devid:a020 at port:2e
[   84.188712] got devid f, want 8850
[   84.188713] no devid:8850 at port:4e
[   84.188726] got devid f, want 8860
[   84.188727] no devid:8860 at port:4e
[   84.188740] got devid f, want a020
[   84.188741] no devid:a020 at port:4e
[   84.188742] pc87360: superio port not detected, module not intalled.
FATAL: Error inserting w83627ehf
(/lib/modules/2.6.23-git8/kernel/drivers/hwmon/w83627ehf.ko): No such
device

Your suggestion about sending the superio-enter sequence seems right:
it sees the port, but it doesn't see the devid. I added a whole bunch of printks and I think the mask value is wrong (it gets a devid of 8863 on my w83627ehf, and what we want is the 8860, not the 3, on port 0x2e; 0x4e gives 0xffff.)

So what I changed:
Index: linux-2.6.23-git8/drivers/hwmon/w83627ehf.c
=================================--- linux-2.6.23-git8/drivers/hwmon/w83627ehf.c 2007-10-18
12:05:02.000000000 -0700
+++ linux-2.6.23-git8/drivers/hwmon/w83627ehf.c 2007-10-18
12:36:21.000000000 -0700
@@ -79,7 +79,7 @@
#define SIO_W83627EHF_ID 0x8850
#define SIO_W83627EHG_ID 0x8860
#define SIO_W83627DHG_ID 0xa020
-#define SIO_ID_MASK 0xFFF0
+#define SIO_ID_MASK 0xF

static struct superio* gate;

@@ -1426,7 +1426,7 @@
superio_enter(gate);
val = superio_devid(gate);

- switch (val & SIO_ID_MASK) {
+ switch (val & (~SIO_ID_MASK)) {
case SIO_W83627EHF_ID:
sio_data->kind = w83627ehf;
sio_name = sio_name_W83627EHF;
I didn't look at how other drivers do it, but I suspect if I had this problem, w83627hf will have it too. Is this line in superio_locks.c (around line 80, except I've patched up mine):
mydevid &= ~ where->devid_mask;

Is it correct to invert the mask? Is every other driver inverting the mask first? It seems you would want to flip the bits in the mask constants, rather than invert every mask before using it. I didn't find anywhere else that devid_mask was used.

OK, so enough rambling. With the mask changed, I get:

[ 1246.391103] initializing with 3 reservation slots
[ 1246.392371] superio_locks init: enter seq 87 port 2e
[ 1246.392374] superio_locks init: enter seq 87 port 2e
[ 1246.392376] superio_locks init: select devid: 20 port 2e
[ 1246.392379] superio_locks init: high devid: 8800 from port 2f
[ 1246.392380] superio_locks init: select devid+1: 21 port 2e
[ 1246.392383] superio_locks init: whole devid: 8863 from port 2f
[ 1246.392384] superio_locks init: mask devid: 8863 & ~ f = 8860
[ 1246.392386] got devid 8860, want 8850
[ 1246.392387] no devid:8850 at port:2e
[ 1246.392388] superio_locks init: enter seq 87 port 2e
[ 1246.392390] superio_locks init: enter seq 87 port 2e
[ 1246.392392] superio_locks init: select devid: 20 port 2e
[ 1246.392394] superio_locks init: high devid: 8800 from port 2f
[ 1246.392396] superio_locks init: select devid+1: 21 port 2e
[ 1246.392398] superio_locks init: whole devid: 8863 from port 2f
[ 1246.392400] superio_locks init: mask devid: 8863 & ~ f = 8860
[ 1246.392402] superio_locks: superio port region already claimed
[ 1246.392403] allocating slot 0, addr 2e for device 8860
[ 1246.392404] found devid:8860 port:2e users:1
[ 1246.392413] w83627ehf: Found W83627EHG chip at 0x290
[ 1246.392455] w83627ehf w83627ehf.0: Failed to request region 0x5-0x6
[ 1246.392460] w83627ehf: probe of w83627ehf.0 failed with error -16

Looking at that, I noticed this in your patch:
/* w83627ehf_find() looks for a '627 in the Super-I/O config space */
-static int __init w83627ehf_find(int sioaddr, unsigned short *addr,
- struct w83627ehf_sio_data *sio_data)
+static int __init w83627ehf_find(struct w83627ehf_sio_data *sio_data)

 {
        static const char __initdata sio_name_W83627EHF[] = "W83627EHF";
        static const char __initdata sio_name_W83627EHG[] = "W83627EHG";
        static const char __initdata sio_name_W83627DHG[] = "W83627DHG";

- u16 val;

+       u16 val, addr;
        const char *sio_name;

I don't think that is correct. We should return addr from w83627ehf_find into sensors_w83627ehf_init where you patched:
@@ -1496,7 +1483,7 @@ static struct platform_device *pdev;
static int __init sensors_w83627ehf_init(void)

 {
        int err;
-       unsigned short address;
+       unsigned short address = 0;
        struct resource res;
        struct w83627ehf_sio_data sio_data;

This line:

	if (!(pdev = platform_device_alloc(DRVNAME, address))) {

uses the address to reserve the region (which should be 0x290 on my machine) where the ISA interface to the w83627ehf is found. Then the w83627ehf is used like some normal PnP device, with ports in the ISA space.

Hmm... so I hope this is valuable feedback. I could put together a patch that undoes the change in the handling of addr, but I thought I'd just shoot you back an email instead. So I'm lazy. :-)

David

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

author maintainer
#11 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie

missed cc to list

Quoted 120 lines · Jim Cromie
Quoted 2 lines · David Hubbard

Hi Jim,

hi David, thanks for the very helpful reply.
sorry for delay responding

Quoted 5 lines

Three things:

1) This appears to be pasted in and needs to be updated for each driver:
printk(KERN_WARNING "pc87360: superio port not detected, "

fixed here, thanks

Quoted 4 lines

2) There's a compile warning in drivers/hwmon/superio_locks.c:
CC [M] drivers/hwmon/superio_locks.o
drivers/hwmon/superio_locks.c: In function 'superio_probe_reserve':
drivers/hwmon/superio_locks.c:44: warning: unused variable 'rc'

rc is used in commented out code - placemarker for question - pc87360 spec says that the sio-cmd-port is readable as well as writable, so I figured a write,read-back test was minimally risky (low false positive). Turns out the test is nogood (false negative) on w83627hf. Ive dropped test here, but am soliciting input/viewpoint/thoughts on it.

Quoted 41 lines

3) superio_find() does not correctly find the w83627ehf:

# modprobe w83627ehf
[   84.162869] initializing with 3 reservation slots
[   84.188669] got devid 3, want 8850

it sees the port, but it doesn't see the devid. I added a whole bunch of printks and I think the mask value is wrong (it gets a devid of 8863 on my w83627ehf, and what we want is the 8860, not the 3, on port 0x2e; 0x4e gives 0xffff.)

So what I changed:
Index: linux-2.6.23-git8/drivers/hwmon/w83627ehf.c =================================> > --- linux-2.6.23-git8/drivers/hwmon/w83627ehf.c 2007-10-18 12:05:02.000000000 -0700
+++ linux-2.6.23-git8/drivers/hwmon/w83627ehf.c 2007-10-18
12:36:21.000000000 -0700

(quoted diff) +2 -2
@@ -79,7 +79,7 @@
 #define SIO_W83627EHF_ID       0x8850
 #define SIO_W83627EHG_ID       0x8860
 #define SIO_W83627DHG_ID       0xa020
-#define SIO_ID_MASK            0xFFF0
+#define SIO_ID_MASK            0xF
 
 static struct superio* gate;
 
@@ -1426,7 +1426,7 @@
        superio_enter(gate);
        val = superio_devid(gate);
 
-       switch (val & SIO_ID_MASK) {
+       switch (val & (~SIO_ID_MASK)) {
        case SIO_W83627EHF_ID:
                sio_data->kind = w83627ehf;
                sio_name = sio_name_W83627EHF;

I didn't look at how other drivers do it, but I suspect if I had this problem, w83627hf will have it too. Is this line in superio_locks.c (around line 80, except I've patched up mine):
mydevid &= ~ where->devid_mask;

Quoted 4 lines

Is it correct to invert the mask? Is every other driver inverting the mask first? It seems you would want to flip the bits in the mask constants, rather than invert every mask before using it. I didn't find anywhere else that devid_mask was used.

nice catch & analysis - ehf is only driver using devid-mask, so its the model :-)
If I drop the tilde, things should work w/o the above change

Quoted 14 lines

Looking at that, I noticed this in your patch:
/* w83627ehf_find() looks for a '627 in the Super-I/O config space */

drivers/hwmon/w83627ehf.c +2 -3
-static int __init w83627ehf_find(int sioaddr, unsigned short *addr,
-                                struct w83627ehf_sio_data *sio_data)
+static int __init w83627ehf_find(struct w83627ehf_sio_data *sio_data)
 {
        static const char __initdata sio_name_W83627EHF[] = "W83627EHF";
        static const char __initdata sio_name_W83627EHG[] = "W83627EHG";
        static const char __initdata sio_name_W83627DHG[] = "W83627DHG";
 
-       u16 val;
+       u16 val, addr;
        const char *sio_name;
Quoted 1 line

I don't think that is correct. We should return addr from

ie diff-chunks doing a half-job on a fn-interface-change

They do a 1/2-baked job changing ehf-find() sig and caller interface. The part not done is actually returning and assigning the addr. Im reverting them for now - maybe redo later as another patch

Im pleased to note your wording above: "return addr from", that thinking is how the interface-change crept into the code..

Quoted 18 lines

w83627ehf_find into sensors_w83627ehf_init where you patched:

drivers/hwmon/w83627ehf.c +1 -1
@@ -1496,7 +1483,7 @@ static struct platform_device *pdev;
 static int __init sensors_w83627ehf_init(void)
 {
        int err;
-       unsigned short address;
+       unsigned short address = 0;
        struct resource res;
        struct w83627ehf_sio_data sio_data;
 

This line:

        if (!(pdev = platform_device_alloc(DRVNAME, address))) {

uses the address to reserve the region (which should be 0x290 on my machine) where the ISA interface to the w83627ehf is found. Then the w83627ehf is used like some normal PnP device, with ports in the ISA space.

Quoted 12 lines
Quoted 4 lines

Hmm... so I hope this is valuable feedback. I could put together a patch that undoes the change in the handling of addr, but I thought I'd just shoot you back an email instead. So I'm lazy. :-)

hey, youre just doing some well-earned laurel-resting ;-) Your feedback was hugely helpful.
new patchset forthcoming.

Quoted 2 lines

David

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#12 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie

Hi Jim,

Quoted 2 lines

hi David, thanks for the very helpful reply.
sorry for delay responding

Had a laugh about the "laurel resting." Maybe I'm "pushing up daisies." It's been a while since I posted on LM-Sensors.

Quoted 1 line

fixed here, thanks

I realized afterward that the way w83627ehf does it is not "pc87360" or "w83627ehf" but DEVNAME as a macro. FYI.

Quoted 5 lines

rc is used in commented out code - placemarker for question - pc87360 spec says that the sio-cmd-port is readable as well as writable, so I figured a write,read-back test was minimally risky (low false positive). Turns out the test is nogood (false negative) on w83627hf. Ive dropped test here, but am soliciting input/viewpoint/thoughts on it.

Okay, here's my viewpoint: I wish every register supported read/write. It makes multiplexing access to the registers easy.

I should have (but haven't yet) tested the commented out code in superio_probe_reserve and figured out what works best for the EHF. I'll get to it.

Probing and initializing the device is always the hardest part. The rest of superio_locks.c should be a piece of cake. So it bugs me that w83627ehf needs a custom superio_exit sequence. I'll fiddle around and try to scratch that itch.

Quoted 3 lines

nice catch & analysis - ehf is only driver using devid-mask, so its the model :-)
If I drop the tilde, things should work w/o the above change

OK, I'll revert that and continue testing.

Quoted 10 lines
Quoted 1 line

I don't think that is correct. We should return addr from

ie diff-chunks doing a half-job on a fn-interface-change

They do a 1/2-baked job changing ehf-find() sig and caller interface. The part not done is actually returning and assigning the addr. Im reverting them for now - maybe redo later as another patch

Im pleased to note your wording above: "return addr from", that thinking is how the interface-change crept into the code..

So maybe using addr as a pointer to an int is not the best fn interface. Returning an address from the function would be OK, since negative values are obviously error return codes.

I'll wait for an updated patch from you, and work on the superio_probe_reserve and superio_exit sequences, and maybe change that function interface for w83627ehf_find(), then send back a patch.

Cheers,
David

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#13 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie

Hi Jim,

I pulled the w83627ehf.c patch out of patch #5, so patch #5 is (mostly) unchanged, and then put some of the superio-locks.c changes in patch #1 to support changes to w83627ehf. So w83627ehf changes are in my patch #6, where I did the things we have talked about.

Quoted 5 lines
Quoted 3 lines

1) This appears to be pasted in and needs to be updated for each driver:
printk(KERN_WARNING "pc87360: superio port not detected, "

fixed here, thanks

Full changelog for patches 1-5:
1. Patches are now based on 2.6.24-rc2-git5. Since this patchset touches so many drivers, it needs some heavy testing. So I also made a patchset for 2.6.24-rc2-mm1, and I can post that set if I've based off the wrong kernel version. (Both patchsets are compile-tested.)

1. I renamed drivers/hwmon/superio_locks.c to drivers/hwmon/superio-locks.c (it seems to be the convention)

2. I updated patch #3 and #5 to use DRVNAME

3. I trimmed down patch #5 so that new stuff is in patch #6

4. I added a parameter enter_exit_method to struct superio_search & struct superio which specifies what enter method to use. 0 = default, the exit_seq is sent to the superio-port in a sequence. 1 = register offset is first byte, followed by value. So W83627EHF has an exit_seq of [ 0, 2, 1, 2 ]. Other methods can be added, if there are other chips as weird as the W83627EHF.

5. I looked around superio-locks.c and I couldn't find code to copy enter_seq, exit_seq, devid_word, and ldn_addr from struct superio_search to struct superio. I added something to do this at the end of superio_probe_reserve().

6. I renamed struct superio_search devid_mask to devid_mask_inverse

7. I fixed a bug in superio-locks.h:
for (i = 0; i < SEQ_SZ && gate->enter_seq[i]; i++) outb(gate->exit_seq[i], gate->sioaddr);

The termination condition in the for loop in superio_exit() checks enter_seq for zeros. It should check exit_seq, like this:
for (i = 0; i < SEQ_SZ && gate->exit_seq[i]; i++) outb(gate->exit_seq[i], gate->sioaddr);

Patch #6 is the stuff in w83627ehf.c:
8. I removed SIO_ID_MASK and used superio_devid() instead in w83627ehf_find(), because the devid is saved after having been masked by struct superio_search devid_mask_inverse. I initialize struct superio_search with 0xF, instead of ~SIO_ID_MASK where SIO_ID_MASK was 0xFFF0.

9. I removed my_superio_exit(), since superio-locks.c can now do the superio_exit() for the w83627ehf.

10. I removed SIO_REG_LDSEL and SIO_REG_DEVID, since they are part of superio-locks.c now. Other drivers might be able to do this too.

11. I moved struct superio * gate into struct w83627ehf_sio_data, where it replaces int sioreg there. int sioreg was where the probed superio address was stored, so that's where the gate should be stored now.

12. I use superio_inw to get the ISA address

13. I release the superio slot reservation after successfully probing the device and finding its ISA address. The code that releases it is at the end of sensors_w83627ehf_init().

14. w83627ehf_find() return value is the ISA address or a negative error code, as we discussed.

I tested everything on a w83627ehf and it works ok. I can test on a w83627dhg in a few days.

Jim, when you have some time, please test on the motherboards you have.

Jean, if you have a chance, can you please review these patches?

Regards,
David

hwmon_superio_module
Applies on top of linux-2.6.24-rc2

drivers/hwmon/Kconfig +9 -0
diff -u linux-2.6.24-rc2-git5/drivers/hwmon/Kconfig linux-2.6.24-rc2-git5/drivers/hwmon/Kconfig
@@ -763,4 +763,13 @@
 	  a problem with I2C support and want to see more of what is going
 	  on.
 
+config SUPERIO_LOCKS
+	tristate "Super-IO port sharing"
+	default n
+	help
+	  This module provides a shared reservation for use by drivers
+	  which need to share access to a multi-function device via
+	  its superio port, and which register that port.
+
 endif # HWMON
+
drivers/hwmon/Makefile +1 -0
diff -u linux-2.6.24-rc2-git5/drivers/hwmon/Makefile linux-2.6.24-rc2-git5/drivers/hwmon/Makefile
@@ -73,3 +73,4 @@
 EXTRA_CFLAGS += -DDEBUG
 endif
 
+obj-$(CONFIG_SUPERIO_LOCKS)	+= superio-locks.o
drivers/hwmon/superio-locks.c +240 -0

240 lines not on this page · raw message

include/linux/superio-locks.h +139 -0

139 lines not on this page · raw message

hwmon-superio-w83627hf

drivers/hwmon/Kconfig +1 -0
diff -u linux-2.6.24-rc2-git5/drivers/hwmon/Kconfig linux-2.6.24-rc2-git5/drivers/hwmon/Kconfig
@@ -686,6 +686,7 @@
 config SENSORS_W83627HF
 	tristate "Winbond W83627HF, W83627THF, W83637HF, W83687THF, W83697HF"
 	select HWMON_VID
+	select SUPERIO_LOCKS
 	help
 	  If you say yes here you get support for the Winbond W836X7 series
 	  of sensor chips: the W83627HF, W83627THF, W83637HF, W83687THF and
drivers/hwmon/w83627hf.c +58 -83

257 lines not on this page · raw message

hwmon-superio-pc87360

drivers/hwmon/Kconfig +1 -0
diff -u linux-2.6.24-rc2-git5/drivers/hwmon/Kconfig linux-2.6.24-rc2-git5/drivers/hwmon/Kconfig
@@ -500,6 +500,7 @@
 config SENSORS_PC87360
 	tristate "National Semiconductor PC87360 family"
 	select HWMON_VID
+	select SUPERIO_LOCKS
 	help
 	  If you say yes here you get access to the hardware monitoring
 	  functions of the National Semiconductor PC8736x Super-I/O chips.
drivers/hwmon/pc87360.c +46 -57

236 lines not on this page · raw message

hwmon-superio-pc8736x-gpio

drivers/char/Kconfig +1 -0
diff -u linux-2.6.24-rc2-git5/drivers/char/Kconfig linux-2.6.24-rc2-git5/drivers/char/Kconfig
@@ -941,6 +941,7 @@
 	depends on X86
 	default SCx200_GPIO	# mostly N
 	select NSC_GPIO		# needed for support routines
+	select SUPERIO_LOCKS
 	help
 	  Give userspace access to the GPIO pins on the National
 	  Semiconductor PC-8736x (x=[03456]) SuperIO chip.  The chip
drivers/char/pc8736x_gpio.c +33 -50

182 lines not on this page · raw message

hwmon-superio-others

drivers/hwmon/Kconfig +6 -0
diff -u linux-2.6.24-rc2-git5/drivers/hwmon/Kconfig linux-2.6.24-rc2-git5/drivers/hwmon/Kconfig
@@ -229,6 +229,7 @@
 config SENSORS_F71805F
 	tristate "Fintek F71805F/FG, F71806F/FG and F71872F/FG"
 	depends on EXPERIMENTAL
+	select SUPERIO_LOCKS
 	help
 	  If you say yes here you get support for hardware monitoring
 	  features of the Fintek F71805F/FG, F71806F/FG and F71872F/FG
@@ -336,6 +337,7 @@
 config SENSORS_IT87
 	tristate "ITE IT87xx and compatibles"
 	select HWMON_VID
+	select SUPERIO_LOCKS
 	help
 	  If you say yes here you get support for ITE IT8705F, IT8712F,
 	  IT8716F, IT8718F and IT8726F sensor chips, and the SiS960 clone.
@@ -566,6 +568,7 @@
 	tristate "SMSC LPC47M192 and compatibles"
 	depends on I2C && EXPERIMENTAL
 	select HWMON_VID
+	select SUPERIO_LOCKS
 	help
 	  If you say yes here you get support for the temperature and
 	  voltage sensors of the SMSC LPC47M192, LPC47M15x, LPC47M292
@@ -582,6 +585,7 @@
 config SENSORS_SMSC47B397
 	tristate "SMSC LPC47B397-NC"
 	depends on EXPERIMENTAL
+	select SUPERIO_LOCKS
 	help
 	  If you say yes here you get support for the SMSC LPC47B397-NC
 	  sensor chip.
@@ -613,6 +617,7 @@
 	tristate "VIA VT1211"
 	depends on EXPERIMENTAL
 	select HWMON_VID
+	select SUPERIO_LOCKS
 	help
 	  If you say yes here then you get support for hardware monitoring
 	  features of the VIA VT1211 Super-I/O chip.
@@ -699,6 +704,7 @@
 config SENSORS_W83627EHF
 	tristate "Winbond W83627EHF/DHG"
 	select HWMON_VID
+	select SUPERIO_LOCKS
 	help
 	  If you say yes here you get support for the hardware
 	  monitoring functionality of the Winbond W83627EHF Super-I/O chip.
drivers/hwmon/f71805f.c +30 -57

154 lines not on this page · raw message

drivers/hwmon/it87.c +30 -51

135 lines not on this page · raw message

drivers/hwmon/smsc47b397.c +25 -39

100 lines not on this page · raw message

drivers/hwmon/smsc47m1.c +27 -43

103 lines not on this page · raw message

drivers/hwmon/vt1211.c +27 -44

115 lines not on this page · raw message

hwmon-superio-w83627ehf

drivers/hwmon/w83627ehf.c +63 -82

266 lines not on this page · raw message

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

author maintainer
#14 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie
Quoted 8 lines · David Hubbard

Hi Jim,

I pulled the w83627ehf.c patch out of patch #5, so patch #5 is (mostly) unchanged, and then put some of the superio-locks.c changes in patch #1 to support changes to w83627ehf. So w83627ehf changes are in my patch #6, where I did the things we have talked about.

Hi David,

thanks so much for pushing forward with this.

Sorry for the long delay - I was trying to get facile with git, then I moved, and things (hw) are still in cartons, scattered to the winds, etc,
and internet access is rare/inconvenient.

I hope to be able to make some progress in Dec, despite travel interruptions,
holidays etc. I hope that this slowness is only a little annoying.

And now that you've reworked the patches on top of something fresh, that simplifies things greatly :-) and I'll try them out asap.

thanks again.

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#15 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie

Hi Jim,

Good to hear from you! Merry Christmas, Joyous Noel, whatever ... you get patches for your holidays this year.

I'm excited for the possibilities these patches open up ... especially for w83627ehf.

Take care,
David

Quoted 19 lines · Jim Cromie
Quoted 8 lines · David Hubbard

Hi Jim,

I pulled the w83627ehf.c patch out of patch #5, so patch #5 is (mostly) unchanged, and then put some of the superio-locks.c changes in patch #1 to support changes to w83627ehf. So w83627ehf changes are in my patch #6, where I did the things we have talked about.

Hi David,

thanks so much for pushing forward with this.

Sorry for the long delay - I was trying to get facile with git, then I moved, and things (hw) are still in cartons, scattered to the winds, etc,
and internet access is rare/inconvenient.

Quoted 9 lines

I hope to be able to make some progress in Dec, despite travel interruptions,
holidays etc. I hope that this slowness is only a little annoying.

And now that you've reworked the patches on top of something fresh, that simplifies things greatly :-) and I'll try them out asap.

thanks again.

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

author maintainer
#16 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie
Quoted 5 lines · David Hubbard

Hi Jim,

Good to hear from you! Merry Christmas, Joyous Noel, whatever ... you get patches for your holidays this year.

Quoted 17 lines

I'm excited for the possibilities these patches open up ... especially for w83627ehf.

Take care,
David

Quoted 9 lines · Jim Cromie
Quoted 8 lines · David Hubbard

Hi Jim,

I pulled the w83627ehf.c patch out of patch #5, so patch #5 is (mostly) unchanged, and then put some of the superio-locks.c changes in patch #1 to support changes to w83627ehf. So w83627ehf changes are in my patch #6, where I did the things we have talked about.

Hi David,

happy 0th day of christmas - on this day I give you 0 patches. (not that I can give you 12 by christmas day, I just couldnt leave it alone.) .
A few quick notes.

we dont need 2 superio_inw*'s - mine is wrong (missing a +1), so Ive removed it, and renamed yours.

Wrt superio_exit, I suspect more critical reviewers will dislike the use of exit_seq[] for both values to write, and addresses to write to.
Something like it is obviously needed for your ehf chip (or you
wouldnt have added it). An exit_addr_seq[] (with offsets from gate->sioaddr) would be clearer, and would like method-0, but would also be a bit bigger.

Perhaps a callback is better, maybe in a union with enter/exit_seq[]s. I wonder what the next device will require. Im also not thrilled by enter-exit-method, it influences only the exit method now, and maybe should be a single bit, allowing another for enter-method (if needed later).
Lastly, I think exit_addr_seq[] may obsolete enter-exit-method.

Chances are poor that I'll locate hardware here that I can test upon, so Im somewhat limited till mid Jan.

thanks again David,
happy kwanzaa, etc

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#17 lore raw

Re: [lm-sensors] [ patch .24-rc0 5/5 ] SuperIO locks coordinator -

In reply to Jim Cromie

Hi Jim,

Quoted 9 lines

Hi David,

happy 0th day of christmas - on this day I give you 0 patches. (not that I can give you 12 by christmas day, I just couldnt leave it alone.) .
A few quick notes.

we dont need 2 superio_inw*'s - mine is wrong (missing a +1), so Ive removed it, and renamed yours.

Okay, sounds good.

Quoted 7 lines

Wrt superio_exit, I suspect more critical reviewers will dislike the use of exit_seq[] for both values to write, and addresses to write to.
Something like it is obviously needed for your ehf chip (or you
wouldnt have added it). An exit_addr_seq[] (with offsets from gate->sioaddr) would be clearer, and would like method-0, but would also be a bit bigger.

Quoted 6 lines

Perhaps a callback is better, maybe in a union with enter/exit_seq[]s. I wonder what the next device will require. Im also not thrilled by enter-exit-method, it influences only the exit method now, and maybe should be a single bit, allowing another for enter-method (if needed later).
Lastly, I think exit_addr_seq[] may obsolete enter-exit-method.

Okay, agreed. A callback would make the most sense, and that's similar to the way other bus subsystems handle driver-specific code. So I actually like exit_addr_seq[] except that it is unnecessary for most of the drivers. I'll think about how to do a union, but I don't know if that is very common in kernel code, and if it's not it might be criticized...

Quoted 2 lines

Chances are poor that I'll locate hardware here that I can test upon, so Im somewhat limited till mid Jan.

Ok, I'm not rushed... :-)

Quoted 2 lines

thanks again David,
happy kwanzaa, etc

Happy Hannukah,
David

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors