amd756 and amd8111 sensors support

cool! lm_sensors finally in 2.5!

It would be cool if the code wasn't that obviously broken.

obj-$(CONFIG_I2C) += i2c/
+obj-$(CONFIG_I2C_MAINBOARD) += i2c/busses/
+obj-$(CONFIG_SENSORS) += i2c/chips/

Please add them inside drivers/i2c/, not at the toplevel.

obj-$(CONFIG_PHONE) += telephony/
obj-$(CONFIG_MD) += md/
obj-$(CONFIG_BT) += bluetooth/

diff -Nru a/drivers/char/mem.c b/drivers/char/mem.c
@@ -27,6 +27,12 @@
 #include <asm/io.h>
 #include <asm/pgalloc.h>
 
+#ifdef CONFIG_I2C_MAINBOARD
+extern void i2c_mainboard_init_all(void);
+#endif
+#ifdef CONFIG_SENSORS
+extern void sensors_init_all(void);
+#endif
 #ifdef CONFIG_I2C
 extern int i2c_init_all(void);
 #endif

@@ -705,6 +711,9 @@
 #ifdef CONFIG_I2C
 	i2c_init_all();
 #endif
+#ifdef CONFIG_I2C_MAINBOARD
+	i2c_mainboard_init_all();
+#endif
 #if defined (CONFIG_FB)
 	fbmem_init();
 #endif

@@ -719,6 +728,10 @@
 #if defined(CONFIG_S390_TAPE) && defined(CONFIG_S390_TAPE_CHAR)
 	tapechar_init();
 #endif
+#ifdef CONFIG_SENSORS
+	sensors_init_all();
+#endif
 

Where have you been the last years? Plese use initcalls instead of adding explicit junk in mem.c.

+#include <linux/version.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <asm/io.h>
+#include <linux/kernel.h>
+#include <linux/stddef.h>
+#include <linux/sched.h>
+#include <linux/ioport.h>
+#include <linux/i2c.h>
+#include <linux/init.h>

<asm/*.h> after <linux/*.h>, please

+
+struct sd {

+    const unsigned short vendor;
+    const unsigned short device;
+    const unsigned short function;
+    const char* name;
+    int amdsetup:1;
+};
+
+static struct sd supported[] = {
+    {PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_VIPER_740B, 3, "AMD756", 1},
+    {PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_VIPER_7413, 3, "AMD766", 1},
+    {PCI_VENDOR_ID_AMD, 0x7443, 3, "AMD768", 1},
+    {PCI_VENDOR_ID_NVIDIA, 0x01B4, 1, "nVidia nForce", 0},
+    {0, 0, 0}

+};

This should really use a proper pci ID table from the generic code.

+ /* id */ I2C_ALGO_SMBUS,
+ /* master_xfer */ NULL,
+ /* smbus_access */ amd756_access,
+ /* slave;_send */ NULL,
+ /* slave_rcv */ NULL,
+ /* algo_control */ NULL,
+ /* functionality */ amd756_func,
+};
+
+static struct i2c_adapter amd756_adapter = {
+ "unset",
+ I2C_ALGO_SMBUS | I2C_HW_SMBUS_AMD756,
+ &smbus_algorithm,
+ NULL,
+ amd756_inc,
+ amd756_dec,
+ NULL,
+ NULL,
+};

Please use named initializers for such sparse method vectors.

+
+static int __initdata amd756_initialized;

__initdata for function used in cleanup code is bogus. But this whole flag is crap, see below.

+static struct sd *amd756_sd = NULL;

Just properly organize your setup code and you don't need this one.

+static unsigned short amd756_smba = 0;

This is in .bss, no need to initialize to zero.

+ struct pci_dev *AMD756_dev = NULL;
+
+ if (pci_present() == 0) {
+ return -ENODEV;
+ }
+
+ /* Look for a supported chip */
+ for(currdev = supported; currdev->vendor; ) {
+ AMD756_dev = pci_find_device(currdev->vendor,
+ currdev->device, AMD756_dev);
+ if (AMD756_dev != NULL) {
+ if (PCI_FUNC(AMD756_dev->devfn) == currdev->function)
+ break;
+ } else {
+ currdev++;
+ }
+ }

I think you really just want to use a proper new-style pci driver..

+ if (check_region(amd756_smba, SMB_IOSIZE)) {
+ printk
+ ("i2c-amd756.o: SMB region 0x%x already in use!\n",
+ amd756_smba);
+ return -ENODEV;
+ }
+
+ /* Everything is happy, let's grab the memory and set things up. */
+ request_region(amd756_smba, SMB_IOSIZE, "amd756-smbus");

Don't use check_region.

+
+void amd756_inc(struct i2c_adapter *adapter)
+{
+ MOD_INC_USE_COUNT;
+}
+
+void amd756_dec(struct i2c_adapter *adapter)
+{
+
+ MOD_DEC_USE_COUNT;
+}

Your module-refcounting is bogus.

+int __init i2c_amd756_init(void)
+{
+ int res;
+#ifdef DEBUG
+/* PE- It might be good to make this a permanent part of the code! */
+ if (amd756_initialized) {
+ printk
+ ("i2c-amd756.o: Oops, amd756_init called a second time!\n");
+ return -EBUSY;
+ }
+#endif

This assert can't ever be triggered.

+ amd756_initialized = 0;

It's in .bss and thus already zero.

+ if ((res = amd756_setup())) {
+ printk
+ ("i2c-amd756.o: AMD756 or compatible device not detected, module not inserted.\n");
+ amd756_cleanup();
+ return res;
+ }

Just merge amd756_setup into it's only caller and your init code will get a lot nicer.

+void __exit i2c_amd756_exit(void)
+{
+ amd756_cleanup();
+}
+
+static int amd756_cleanup(void)
+{
+ int res;
+ if (amd756_initialized >= 2) {
+ if ((res = i2c_del_adapter(&amd756_adapter))) {
+ printk
+ ("i2c-amd756.o: i2c_del_adapter failed, module not removed\n");
+ return res;
+ } else
+ amd756_initialized--;
+ }
+ if (amd756_initialized >= 1) {
+ release_region(amd756_smba, SMB_IOSIZE);
+ amd756_initialized--;
+ }
+ return 0;
+}

Please merge i2c_amd756_exit into amd756_cleanup and do proper error handling inside the int function instead of the gliobal state mess.

+
+EXPORT_NO_SYMBOLS;

Remove it, it's a noop for 2.5.

+#ifdef MODULE
+
+MODULE_AUTHOR("Merlin Hughes <[email protected]>"); +MODULE_DESCRIPTION("AMD756/766/768/nVidia nForce SMBus driver"); +
+#ifdef MODULE_LICENSE
+MODULE_LICENSE("GPL");
+#endif
+
+#endif /* MODULE */

Please get rid of that ifdef mess, you can just always use it.

And please backout the init/exit changes in i2c-dev.c, i2c-core.c and i2c-proc.c, they're just bogus.

This patch adds the i2c-isa pseudo ISA adapter and the w8387* series sensors support to 2.5.54 (tested on bk1 and bk3).

I've just took the code from the lm_sensor package and changed it to be more or less consistent with the lm75 and amd* patches Christoph has done.
Christoph, maybe you could check this because I don't really understand all of the code and maybe it needs some more changes.

GertJan

diff -Nru linux-2.5.54-bk1/drivers/i2c/busses/Kconfig linux-2.5.54/drivers/i2c/busses/Kconfig
@@ -39,5 +39,23 @@
 	  in the lm_sensors package, which you can download at
 	  http://www.lm-sensors.nu
 
+config I2C_ISA
+	tristate "  Pseudo ISA adapter (for some hardware sensors)"
+	depends on I2C && I2C_PROC
+	help
+	  This provide support for accessing some hardware sensors chips over
+	  the ISA bus rather than the I2C or SMBus. If you want to do this,
+	  say yes here.
+
+	  This can also be built as a module which can be inserted and removed
+	  while the kernel is running.  If you want to compile it as a module,
+	  say M here and read <file:Documentation/modules.txt>.
+
+	  The module will be called i2c-isa.o.
+
+	  You will also need the latest user-space utilties: you can find them
+	  in the lm_sensors package, which you can download at
+	  http://www.lm-sensors.nu
+
 endmenu
 

diff -Nru linux-2.5.54-bk1/drivers/i2c/busses/Makefile linux-2.5.54/drivers/i2c/busses/Makefile
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_I2C_AMD756)	+= i2c-amd756.o
 obj-$(CONFIG_I2C_AMD8111)	+= i2c-amd8111.o
+obj-$(CONFIG_I2C_ISA)		+= i2c-isa.o

diff -Nru linux-2.5.54-bk1/drivers/i2c/chips/Kconfig linux-2.5.54/drivers/i2c/chips/Kconfig
@@ -37,4 +37,19 @@
 	  in the lm_sensors package, which you can download at
 	  http://www.lm-sensors.nu
 
+config SENSORS_W83781D
+	tristate "  Winbond W83781D, W83782D, W83783S, W83627HF and Asus AS99127F"
+	depends on I2C && I2C_PROC && I2C_ISA
+	help
+	  If you say yes here you get support for the Winbond W8378x series
+	  of sensor chips: the W83781D, W83782D, W83783S, W83627HF and the
+	  similar Asus AS99127F. This can also be built as a module
+	  which can be inserted and removed while the kernel is running.
+
+	  The module will be called w83781d.o.
+
+	  You will also need the latest user-space utilties: you can find them
+	  in the lm_sensors package, which you can download at 
+	  http://www.lm-sensors.nu
+
 endmenu

diff -Nru linux-2.5.54-bk1/drivers/i2c/chips/Makefile linux-2.5.54/drivers/i2c/chips/Makefile
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_SENSORS_ADM1021)	+= adm1021.o
 obj-$(CONFIG_SENSORS_LM75)	+= lm75.o
+obj-$(CONFIG_SENSORS_W83781D)	+= w83781d.o

Looks fine in principle. A few nitpicks:

Hi!

Done.

Fixed.

Fixed.

Killed.

Ok.

Fixed.

Killed.

Killed.

Okay, I see lot more cleaning up is needed. I actually saved it as a "TODO" file... Thanks for your comments.

Pavel

@@ -38,8 +38,6 @@
 obj-$(CONFIG_SERIO)		+= input/serio/
 obj-$(CONFIG_I2O)		+= message/
 obj-$(CONFIG_I2C)		+= i2c/
-obj-$(CONFIG_I2C_MAINBOARD)	+= i2c/busses/
-obj-$(CONFIG_SENSORS)		+= i2c/chips/
 obj-$(CONFIG_PHONE)		+= telephony/
 obj-$(CONFIG_MD)		+= md/
 obj-$(CONFIG_BT)		+= bluetooth/

@@ -6,6 +6,8 @@
 		   i2c-algo-ite.o i2c-proc.o i2c-algo-ibm_ocp.o
 
 obj-$(CONFIG_I2C)		+= i2c-core.o
+obj-$(CONFIG_I2C_MAINBOARD)	+= busses/
+obj-$(CONFIG_SENSORS)		+= chips/
 obj-$(CONFIG_I2C_CHARDEV)	+= i2c-dev.o
 obj-$(CONFIG_I2C_ALGOBIT)	+= i2c-algo-bit.o
 obj-$(CONFIG_I2C_PHILIPSPAR)	+= i2c-philips-par.o

@@ -223,10 +223,6 @@
 	u32 (*functionality) (struct i2c_adapter *);
 };
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,1,29)
-struct proc_dir_entry;
-#endif
-
 /*
  * i2c_adapter is the structure used to identify a physical i2c bus along
  * with the access algorithms necessary to access it.

Worst form of spam? Adding advertisment signatures ala sourceforge.net.
What goes next? Inserting advertisment *into* email?