More on SMBus multiplexing

More on SMBus multiplexing

[Jean Delvare]

Hi all,

The more I think of Mark Hoffman's proposal to handle SMBus
multiplexing, the more I think he pointed us to the right direction.
However, I now wonder if things aren't even more simple than we first
thought.

The main idea is to remove the physical adapter from the i2c adapters
list, and add virtual busses instead. We already have the possibility to
do that right now. The second thing Mark proposed was to lock the
physical SMBus. While it may sound safer, I don't think it is really
necessary. The sole fact of removing it from the main list will
disconnect all chip clients, and clients loaded later won't see the
physical bus anymore. The only way to attach a client to the SMBus would
be to do it manually. Certainly it could hurt if someone would do that
(because it would interfer with the virtual busses) but it happens that
nobody does this (the whole thing is based on the idea that the core
will automatically connect all clients from the client lists with all
adapters from the adapters list).

What are we trying to protect us from with this "exclusive client" idea
exactly? It will always be possible to run raw commands on the physical
bus anyway, just call adapter.algo->smbus_xfer. There is no way we can
protect us against that. So I think we are trying to protect us from an
event that will never happen and that the protection can be circumvented
by someone with enough motivation anyway. Much work for nothing, IMHO.

If we forget about that bus locking and exclusive client access, I don't
think that anything is missing from the i2c core. All we need is
i2c_{add,del}_adapter, and possibly a way to find an adapter from its id
(I didn't find something like i2c_find_adapter, did I miss it?). I'm not
even sure that this is needed though. A possible approach is to have the
main smbus driver export its i2c_adapter structure, so that the virtual
smbus driver can access it. This also automagically creates a dependancy
between both modules, and ensures that loading the vitual bus driver
will load the main bus driver first.

Note that this approach (as Mark's original one, but contrary to my
original one) would require a couple changes to sensors-detect. It
should really only be a matter of adding the correct entries in the pci
devices list though. However, this could be made unnecessary by
requesting the second module after loading the first one, using
request_module(). I tried, it works, I just don't know if we want to do
it or not. The drawback is that it makes a few board-specific changes to
the main module.

As a side note, the concept of "exclusive access" for a multiplexer
client also raises issues in the case one SMBus would host more that one
multiplexer chip directly at its "root" level. I see no reason why this
would be technically impossible, yet Mark's original model wouldn't
support it. I guess it would be possible to emulate it as if the second
multiplexer was located behind the first one, but this would certainly
make things more complex.

Of course, as with Mark's approach, we have the benefit of having two
separated module, no code duplication and little to very little change
to the original driver, depending on whether we implement the autoload
mechanism or not, for a maximum maintainabilty (so that's better than
my proposal for the amd756-s4882 driver in 2.4/CVS, even after taking
Mark's comment into account).

I was about to commit my i2c-amd756-s4882 module to the lm_sensors CVS
repository, but now I think that I'll try that new approach instead of
my brutal code inclusion, which works but isn't really clean, to say the
least.

As a kind of proof of concept, I did a fake i2c-i801-vaio module to
virtualize the SMBus on my laptop (although it doesn't have a mux chip).
It works just OK as far as I can tell. Of course the code is stupidly
useless (the virtual adapter doesn't do anything more than dumbly
redirect the calls to the physical bus), and lacks the mux client
registration part, since there is no such chip. I think that the idea is
clear though, and at least now we have code to comment on ;)

Thanks.

Index: kernel/busses/Module.mk
===================================================================
RCS file: /home/cvs/lm_sensors2/kernel/busses/Module.mk,v
retrieving revision 1.48
diff -u -r1.48 Module.mk
--- kernel/busses/Module.mk	16 Apr 2004 20:56:53 -0000	1.48
+++ kernel/busses/Module.mk	23 Oct 2004 18:24:17 -0000
@@ -48,6 +48,7 @@
 endif
 ifneq ($(shell if grep -q '^CONFIG_I2C_I801=y' $(LINUX)/.config; then echo 1; fi),1)
 KERNELBUSSESTARGETS += $(MODULE_DIR)/i2c-i801.o
+KERNELBUSSESTARGETS += $(MODULE_DIR)/i2c-i801-vaio.o
 endif
 ifneq ($(shell if grep -q '^CONFIG_I2C_I810=y' $(LINUX)/.config; then echo 1; fi),1)
 KERNELBUSSESTARGETS += $(MODULE_DIR)/i2c-i810.o
Index: kernel/busses/i2c-i801-vaio.c
===================================================================
RCS file: kernel/busses/i2c-i801-vaio.c
diff -N kernel/busses/i2c-i801-vaio.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ kernel/busses/i2c-i801-vaio.c	23 Oct 2004 18:24:17 -0000
@@ -0,0 +1,93 @@
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include "version.h"
+
+extern struct i2c_adapter i801_adapter;
+
+static s32 i801_vaio_access(struct i2c_adapter * adap, u16 addr,
+		       unsigned short flags, char read_write, u8 command,
+		       int size, union i2c_smbus_data * data)
+{
+	return i801_adapter.algo->smbus_xfer(adap, addr, flags,
+		read_write, command, size, data);
+}
+
+static struct i2c_algorithm smbus_algorithm = {
+	.name		= "Non-I2C SMBus adapter",
+	.id		= I2C_ALGO_SMBUS,
+	.smbus_xfer	= i801_vaio_access,
+};
+
+struct i2c_adapter i801_vaio_adapter = {
+	.owner		= THIS_MODULE,
+	.id		= I2C_ALGO_SMBUS | I2C_HW_SMBUS_I801,
+	.algo		= &smbus_algorithm,
+	.name		= "unset",
+};
+
+static int __init i2c_i801_vaio_init(void)
+{
+	int err;
+
+	printk(KERN_INFO "i2c-i801-vaio version %s (%s)\n", LM_VERSION,
+	       LM_DATE);
+
+	if (pci_find_subsys(PCI_VENDOR_ID_INTEL,
+			    PCI_DEVICE_ID_INTEL_82801CA_3,
+			    PCI_VENDOR_ID_SONY, 0x80e7, NULL) == NULL) {
+		return -ENODEV;
+	}
+
+	err = i2c_del_adapter(&i801_adapter);
+	if (err) {
+		printk(KERN_ERR "i2c-i801-vaio: Failed to delete physical "
+		       "adapter\n");
+		return err;
+	}
+
+	/* Attach mux chip to main adapter here */
+
+	smbus_algorithm.functionality = i801_adapter.algo->functionality;
+	snprintf(i801_vaio_adapter.name, 32, "SMBus I801 virtual adapter");
+
+	err = i2c_add_adapter(&i801_vaio_adapter);
+	if (err) {
+		printk(KERN_ERR "i2c-i801-vaio: Failed to add virtual "
+		       "adapter\n");
+		i2c_add_adapter(&i801_adapter);
+		return err;
+	}
+
+	return 0;
+}
+
+static void __exit i2c_i801_vaio_exit(void)
+{
+	int err;
+	
+	err = i2c_del_adapter(&i801_vaio_adapter);
+	if (err) {
+		printk(KERN_ERR "i2c-i801-vaio: Failed to delete virtual "
+		       "adapter\n");
+		return;
+	}
+
+	/* Detach mux chip from main adapter here */
+
+	i2c_add_adapter(&i801_adapter);
+	if (err) {
+		printk(KERN_ERR "i2c-i801-vaio: Failed to add physical "
+		       "adapter back\n");
+		return;
+	}
+}
+
+MODULE_AUTHOR("Jean Delvare <khali@linux-fr.org");
+MODULE_DESCRIPTION("I801 SMBus multiplexing");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_i801_vaio_init);
+module_exit(i2c_i801_vaio_exit);
Index: kernel/busses/i2c-i801.c
===================================================================
RCS file: /home/cvs/lm_sensors2/kernel/busses/i2c-i801.c,v
retrieving revision 1.36
diff -u -r1.36 i2c-i801.c
--- kernel/busses/i2c-i801.c	22 May 2004 04:02:19 -0000	1.36
+++ kernel/busses/i2c-i801.c	23 Oct 2004 18:24:18 -0000
@@ -49,6 +49,7 @@
 #include <linux/ioport.h>
 #include <linux/init.h>
 #include <linux/i2c.h>
+#include <linux/kmod.h>
 #include <asm/io.h>
 #include "version.h"
 #include "sensors_compat.h"
@@ -548,7 +549,7 @@
 	.functionality	= i801_func,
 };
 
-static struct i2c_adapter i801_adapter = {
+struct i2c_adapter i801_adapter = {
 	.owner		= THIS_MODULE,
 	.id		= I2C_ALGO_SMBUS | I2C_HW_SMBUS_I801,
 	.algo		= &smbus_algorithm,
@@ -558,6 +559,12 @@
 static struct pci_device_id i801_ids[] __devinitdata = {
 	{
 		.vendor =	PCI_VENDOR_ID_INTEL,
+		.device =	PCI_DEVICE_ID_INTEL_82801CA_3,
+		.subvendor =	PCI_VENDOR_ID_SONY,
+		.subdevice =	0x80e7,
+	},
+	{
+		.vendor =	PCI_VENDOR_ID_INTEL,
 		.device =	PCI_DEVICE_ID_INTEL_82801AA_3,
 		.subvendor =	PCI_ANY_ID,
 		.subdevice =	PCI_ANY_ID,
@@ -609,6 +616,7 @@
 
 static int __devinit i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
+	int err;
 
 	if (i801_setup(dev)) {
 		dev_warn(dev,
@@ -618,7 +626,14 @@
 
 	snprintf(i801_adapter.name, 32,
 		"SMBus I801 adapter at %04x", i801_smba);
-	return i2c_add_adapter(&i801_adapter);
+	if ((err = i2c_add_adapter(&i801_adapter)))
+		return err;
+
+	if (dev->subsystem_vendor == PCI_VENDOR_ID_SONY
+	 && dev->subsystem_device == 0x80e7)
+	 	request_module("i2c-i801-vaio");
+
+	return 0;
 }
 
 static void __devexit i801_remove(struct pci_dev *dev)
@@ -651,5 +666,7 @@
 MODULE_DESCRIPTION("I801 SMBus driver");
 MODULE_LICENSE("GPL");
 
+EXPORT_SYMBOL(i801_adapter);
+
 module_init(i2c_i801_init);
 module_exit(i2c_i801_exit);



-- 
Jean Delvare
http://khali.linux-fr.org/

Re: More on SMBus multiplexing

[Jean Delvare]

Replying to myself:

> As a kind of proof of concept, I did a fake i2c-i801-vaio module to
> virtualize the SMBus on my laptop (although it doesn't have a mux
> chip). It works just OK as far as I can tell. Of course the code is
> stupidly useless (the virtual adapter doesn't do anything more than
> dumbly redirect the calls to the physical bus), and lacks the mux
> client registration part, since there is no such chip. I think that
> the idea is clear though, and at least now we have code to comment on
> ;)

I find that I am unable to actually register the mux client. Odd, since
it worked OK on a 2.4 kernel, and several tries led me nowhere on 2.6
kernels. If anyone has sample code to just occupy a given I2C address on
a given bus, please share it with me.

However, why do we even need this? Looks far easier to simply exclude
the multiplexer address from the virtual busses (which we need to do
anyway). Nobody is supposed to access the physical bus directly (it's
not in the main adapters list anyway). Again, I see no reason to protect
us from something that is just never going to happen. This makes the
whole thing even more simple, exactly as in my demo code.

Thanks.

-- 
Jean Delvare
http://khali.linux-fr.org/

Re: More on SMBus multiplexing

[Jean Delvare]

Hi Greg,

Would you accept such a patch? This adds Tyan S4882-specific SMBus
multiplexing support, in the form of a separate driver, as per Mark
Hoffman's suggestion.

Changes to the original driver (i2c-amd756) are kept to a minimum:

1* The i2c_adapter structure is exported (some noise due to a struct
name change, I didn't want to export something as vague as
"amd756_adapter").

2* Autoload of the new driver (i2c-amd756-s4882) when needed. This can
go away if you don't like it, but I thought it'd be more convenient for
the end user. BTW, what will happen if the i2c-amd756-s4882 module
doesn't exist? Should I add some #ifdef magic in the i2c-amd756 driver
so that the autoload will not happen if the i2c-amd756-s4882 module was
not selected?

The i2c-amd756-s4882 driver follows Mark Hoffman's suggestion, but in a
simplified way. On load:

1* Remove the physical adapter from the main i2c adapters list. As a
side effect, any client gets kicked off.

2* Register 5 virtual busses using to wrappers pointing to the original
SMBus code. Clients can connect to the new adapters where it makes
sense.

On unload, same thing but the other way around.

This approach has a drawback in the case several client drivers are
already loaded (they will get attached, detached and attached again in a
raw), but I don't think this is really a problem:

1* In most cases bus drivers are loaded before clients anyway.

2* SMBus multiplexing is quite rare, and I don't care if people with
such specific boards experience drawbacks at init time in uncommon
cases.

Advantages of the approach, however, are great. Multiplexing code
belongs to a different driver, which improves maintainability as Mark
pointed out, and means no impact on module size nor performance for
regular users. The whole thing is also almost transparent for userspace
and client drivers, requiring very very little change, if any at all.

Greg, anyone, comments on the approach and code are welcome. If
everyone's OK, we'll go that way for both the 2.6 kernel and lm_sensors
(for 2.4).

Thanks.

diff -ruN linux-2.6.9-rc4-bk1/drivers/i2c/busses.orig/Kconfig linux-2.6.9-rc4-bk1/drivers/i2c/busses/Kconfig
--- linux-2.6.9-rc4-bk1/drivers/i2c/busses.orig/Kconfig	2004-10-16 19:37:33.000000000 +0200
+++ linux-2.6.9-rc4-bk1/drivers/i2c/busses/Kconfig	2004-10-24 18:46:53.000000000 +0200
@@ -51,6 +51,17 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-amd756.
 
+config I2C_AMD756_S4882
+	tristate "SMBus multiplexing on the Tyan S4882"
+	depends on I2C_AMD756 && EXPERIMENTAL
+	default y
+	help
+	  Enabling this option will add specific SMBus support for the Tyan
+	  S4882 motherboard.  On this 4-CPU board, the SMBus is multiplexed
+	  over 8 different channels, where the various memory module EEPROMs
+	  and temperature sensors live.  Saying yes here will give you access
+	  to these in addition to the trunk.
+
 config I2C_AMD8111
 	tristate "AMD 8111"
 	depends on I2C && PCI && EXPERIMENTAL
diff -ruN linux-2.6.9-rc4-bk1/drivers/i2c/busses.orig/Makefile linux-2.6.9-rc4-bk1/drivers/i2c/busses/Makefile
--- linux-2.6.9-rc4-bk1/drivers/i2c/busses.orig/Makefile	2004-10-12 19:44:25.000000000 +0200
+++ linux-2.6.9-rc4-bk1/drivers/i2c/busses/Makefile	2004-10-23 22:55:45.000000000 +0200
@@ -6,6 +6,7 @@
 obj-$(CONFIG_I2C_ALI1563)	+= i2c-ali1563.o
 obj-$(CONFIG_I2C_ALI15X3)	+= i2c-ali15x3.o
 obj-$(CONFIG_I2C_AMD756)	+= i2c-amd756.o
+obj-$(CONFIG_I2C_AMD756_S4882)	+= i2c-amd756-s4882.o
 obj-$(CONFIG_I2C_AMD8111)	+= i2c-amd8111.o
 obj-$(CONFIG_I2C_ELEKTOR)	+= i2c-elektor.o
 obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
diff -ruN linux-2.6.9-rc4-bk1/drivers/i2c/busses.orig/i2c-amd756-s4882.c linux-2.6.9-rc4-bk1/drivers/i2c/busses/i2c-amd756-s4882.c
--- linux-2.6.9-rc4-bk1/drivers/i2c/busses.orig/i2c-amd756-s4882.c	1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.9-rc4-bk1/drivers/i2c/busses/i2c-amd756-s4882.c	2004-10-24 18:42:12.000000000 +0200
@@ -0,0 +1,266 @@
+/*
+ * i2c-amd756-s4882.c - i2c-amd756 extras for the Tyan S4882 motherboard
+ *
+ * Copyright (C) 2004 Jean Delvare <khali@linux-fr.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+ 
+/*
+ * We select the channels by sending commands to the Philips
+ * PCA9556 chip at I2C address 0x18. The main adapter is used for
+ * the non-multiplexed part of the bus, and 4 virtual adapters
+ * are defined for the multiplexed addresses: 0x50-0x53 (memory
+ * module EEPROM) located on channels 1-4, and 0x4c (LM63)
+ * located on multiplexed channels 0 and 5-7. We define one
+ * virtual adapter per CPU, which corresponds to two multiplexed
+ * channels:
+ *   CPU0: virtual adapter 1, channels 1 and 0
+ *   CPU1: virtual adapter 2, channels 2 and 5
+ *   CPU2: virtual adapter 3, channels 3 and 6
+ *   CPU3: virtual adapter 4, channels 4 and 7
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+
+extern struct i2c_adapter amd756_smbus;
+
+static struct i2c_adapter *s4882_adapter;
+static struct i2c_algorithm *s4882_algo;
+
+/* Wrapper access functions for multiplexed SMBus */
+static struct semaphore amd756_lock;
+
+static s32 amd756_access_virt0(struct i2c_adapter * adap, u16 addr,
+			       unsigned short flags, char read_write,
+			       u8 command, int size,
+			       union i2c_smbus_data * data)
+{
+	int error;
+
+	/* We exclude the multiplexed addresses */
+	if (addr == 0x4c || (addr & 0xfc) == 0x50 || (addr & 0xfc) == 0x30
+	 || addr == 0x18)
+		return -1;
+
+	down(&amd756_lock);
+
+	error = amd756_smbus.algo->smbus_xfer(adap, addr, flags, read_write,
+					      command, size, data);
+
+	up(&amd756_lock);
+
+	return error;
+}
+
+/* We remember the last used channels combination so as to only switch
+   channels when it is really needed. This greatly reduces the SMBus
+   overhead, but also assumes that nobody will be writing to the PCA9556
+   in our back. */
+static u8 last_channels;
+
+static inline s32 amd756_access_channel(struct i2c_adapter * adap, u16 addr,
+					unsigned short flags, char read_write,
+					u8 command, int size,
+					union i2c_smbus_data * data,
+					u8 channels)
+{
+	int error;
+
+	/* We exclude the non-multiplexed addresses */
+	if (addr != 0x4c && (addr & 0xfc) != 0x50 && (addr & 0xfc) != 0x30)
+		return -1;
+
+	down(&amd756_lock);
+
+	if (last_channels != channels) {
+		union i2c_smbus_data mplxdata;
+		mplxdata.byte = channels;
+
+		error = amd756_smbus.algo->smbus_xfer(adap, 0x18, 0,
+						      I2C_SMBUS_WRITE, 0x01,
+						      I2C_SMBUS_BYTE_DATA,
+						      &mplxdata);
+		if (error)
+			goto UNLOCK;
+		last_channels = channels;
+	}
+	error = amd756_smbus.algo->smbus_xfer(adap, addr, flags, read_write,
+					      command, size, data);
+
+UNLOCK:
+	up(&amd756_lock);
+	return error;
+}
+
+static s32 amd756_access_virt1(struct i2c_adapter * adap, u16 addr,
+			       unsigned short flags, char read_write,
+			       u8 command, int size,
+			       union i2c_smbus_data * data)
+{
+	/* CPU0: channels 1 and 0 enabled */
+	return amd756_access_channel(adap, addr, flags, read_write, command,
+				     size, data, 0x03);
+}
+
+static s32 amd756_access_virt2(struct i2c_adapter * adap, u16 addr,
+			       unsigned short flags, char read_write,
+			       u8 command, int size,
+			       union i2c_smbus_data * data)
+{
+	/* CPU1: channels 2 and 5 enabled */
+	return amd756_access_channel(adap, addr, flags, read_write, command,
+				     size, data, 0x24);
+}
+
+static s32 amd756_access_virt3(struct i2c_adapter * adap, u16 addr,
+			       unsigned short flags, char read_write,
+			       u8 command, int size,
+			       union i2c_smbus_data * data)
+{
+	/* CPU2: channels 3 and 6 enabled */
+	return amd756_access_channel(adap, addr, flags, read_write, command,
+				     size, data, 0x48);
+}
+
+static s32 amd756_access_virt4(struct i2c_adapter * adap, u16 addr,
+			       unsigned short flags, char read_write,
+			       u8 command, int size,
+			       union i2c_smbus_data * data)
+{
+	/* CPU3: channels 4 and 7 enabled */
+	return amd756_access_channel(adap, addr, flags, read_write, command,
+				     size, data, 0x90);
+}
+
+static int __init amd756_s4882_init(void)
+{
+	int i, error;
+	union i2c_smbus_data ioconfig;
+
+	if (pci_find_subsys(PCI_VENDOR_ID_AMD, 0x746B, PCI_VENDOR_ID_AMD,
+			    0x36C0, NULL) == NULL)
+		return -ENODEV;
+
+	printk(KERN_INFO "Enabling SMBus multiplexing for Tyan S4882\n");
+
+	init_MUTEX(&amd756_lock);
+
+	/* Unregister physical bus */
+	error = i2c_del_adapter(&amd756_smbus);
+	if (error) {
+		dev_err(&amd756_smbus.dev, "Physical bus removal "
+			"failed\n");
+		goto ERROR0;
+	}
+
+	/* Define the 5 virtual adapters and algorithms structures */
+	if (!(s4882_adapter = kmalloc(5 * sizeof(struct i2c_adapter),
+				      GFP_KERNEL))) {
+		error = -ENOMEM;
+		goto ERROR1;
+	}
+	if (!(s4882_algo = kmalloc(5 * sizeof(struct i2c_algorithm),
+				   GFP_KERNEL))) {
+		error = -ENOMEM;
+		goto ERROR2;
+	}
+
+	/* Fill in the new structures */
+	s4882_algo[0] = *(amd756_smbus.algo);
+	s4882_algo[0].smbus_xfer = amd756_access_virt0;
+	s4882_adapter[0] = amd756_smbus;
+	s4882_adapter[0].algo = s4882_algo;
+	for (i = 1; i < 5; i++) {
+		s4882_algo[i] = *(amd756_smbus.algo);
+		s4882_adapter[i] = amd756_smbus;
+		sprintf(s4882_adapter[i].name,
+			"SMBus 8111 adapter (CPU%d)", i-1);
+		s4882_adapter[i].algo = s4882_algo+i;
+	}
+	s4882_algo[1].smbus_xfer = amd756_access_virt1;
+	s4882_algo[2].smbus_xfer = amd756_access_virt2;
+	s4882_algo[3].smbus_xfer = amd756_access_virt3;
+	s4882_algo[4].smbus_xfer = amd756_access_virt4;
+
+	/* Configure the PCA9556 multiplexer */
+	ioconfig.byte = 0x00; /* All I/O to output mode */
+	error = amd756_smbus.algo->smbus_xfer(&amd756_smbus, 0x18, 0,
+					      I2C_SMBUS_WRITE, 0x03,
+					      I2C_SMBUS_BYTE_DATA, &ioconfig);
+	if (error) {
+		dev_dbg(&amd756_smbus.dev, "PCA9556 configuration failed\n");
+		error = -EIO;
+		goto ERROR3;
+	}
+
+	/* Register virtual adapters */
+	for (i = 0; i < 5; i++) {
+		error = i2c_add_adapter(s4882_adapter+i);
+		if (error) {
+			dev_err(&amd756_smbus.dev,
+			       "Virtual adapter %d registration "
+			       "failed, module not inserted\n", i);
+			for (i--; i >= 0; i--)
+				i2c_del_adapter(s4882_adapter+i);
+			goto ERROR3;
+		}
+	}
+
+	return 0;
+
+ERROR3:
+	kfree(s4882_algo);
+	s4882_algo = NULL;
+ERROR2:
+	kfree(s4882_adapter);
+	s4882_adapter = NULL;
+ERROR1:
+	i2c_del_adapter(&amd756_smbus);
+ERROR0:
+	return error;
+}
+
+static void __exit amd756_s4882_exit(void)
+{
+	if (s4882_adapter) {
+		int i;
+
+		for (i = 0; i < 5; i++)
+			i2c_del_adapter(s4882_adapter+i);
+		kfree(s4882_adapter);
+		s4882_adapter = NULL;
+	}
+	if (s4882_algo) {
+		kfree(s4882_algo);
+		s4882_algo = NULL;
+	}
+
+	/* Restore physical bus */
+	if (i2c_add_adapter(&amd756_smbus))
+		dev_err(&amd756_smbus.dev, "Physical bus restoration "
+			"failed\n");
+}
+
+MODULE_AUTHOR("Jean Delvare <khali@linux-fr.org>");
+MODULE_DESCRIPTION("S4882 SMBus multiplexing");
+MODULE_LICENSE("GPL");
+
+module_init(amd756_s4882_init);
+module_exit(amd756_s4882_exit);
diff -ruN linux-2.6.9-rc4-bk1/drivers/i2c/busses.orig/i2c-amd756.c linux-2.6.9-rc4-bk1/drivers/i2c/busses/i2c-amd756.c
--- linux-2.6.9-rc4-bk1/drivers/i2c/busses.orig/i2c-amd756.c	2004-10-16 21:13:45.000000000 +0200
+++ linux-2.6.9-rc4-bk1/drivers/i2c/busses/i2c-amd756.c	2004-10-24 18:51:52.000000000 +0200
@@ -47,6 +47,7 @@
 #include <linux/ioport.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
+#include <linux/kmod.h>
 #include <asm/io.h>
 
 /* AMD756 SMBus address offsets */
@@ -302,23 +303,24 @@
 	.functionality	= amd756_func,
 };
 
-static struct i2c_adapter amd756_adapter = {
+struct i2c_adapter amd756_smbus = {
 	.owner		= THIS_MODULE,
 	.class          = I2C_CLASS_HWMON,
 	.algo		= &smbus_algorithm,
 	.name		= "unset",
 };
 
-enum chiptype { AMD756, AMD766, AMD768, NFORCE, AMD8111 };
+enum chiptype { AMD756, AMD766, AMD768, NFORCE, AMD8111, S4882 };
 static const char* chipname[] = {
 	"AMD756", "AMD766", "AMD768",
-	"nVidia nForce", "AMD8111",
+	"nVidia nForce", "AMD8111", "AMD8111",
 };
 
 static struct pci_device_id amd756_ids[] = {
 	{PCI_VENDOR_ID_AMD, 0x740B, PCI_ANY_ID, PCI_ANY_ID, 0, 0, AMD756 },
 	{PCI_VENDOR_ID_AMD, 0x7413, PCI_ANY_ID, PCI_ANY_ID, 0, 0, AMD766 },
 	{PCI_VENDOR_ID_AMD, 0x7443, PCI_ANY_ID, PCI_ANY_ID, 0, 0, AMD768 },
+	{PCI_VENDOR_ID_AMD, 0x746B, PCI_VENDOR_ID_AMD, 0x36C0, 0, 0, S4882 },
 	{PCI_VENDOR_ID_AMD, 0x746B, PCI_ANY_ID, PCI_ANY_ID, 0, 0, AMD8111 },
 	{PCI_VENDOR_ID_NVIDIA, 0x01B4, PCI_ANY_ID, PCI_ANY_ID, 0, 0, NFORCE },
 	{ 0, }
@@ -330,6 +332,7 @@
 				  const struct pci_device_id *id)
 {
 	int nforce = (id->driver_data == NFORCE);
+	int s4882 = (id->driver_data == S4882);
 	int error;
 	u8 temp;
 	
@@ -374,18 +377,21 @@
 	dev_dbg(&pdev->dev, "AMD756_smba = 0x%X\n", amd756_ioport);
 
 	/* set up the driverfs linkage to our parent device */
-	amd756_adapter.dev.parent = &pdev->dev;
+	amd756_smbus.dev.parent = &pdev->dev;
 
-	sprintf(amd756_adapter.name, "SMBus %s adapter at %04x",
+	sprintf(amd756_smbus.name, "SMBus %s adapter at %04x",
 		chipname[id->driver_data], amd756_ioport);
 
-	error = i2c_add_adapter(&amd756_adapter);
+	error = i2c_add_adapter(&amd756_smbus);
 	if (error) {
 		dev_err(&pdev->dev,
 			"Adapter registration failed, module not inserted\n");
 		goto out_err;
 	}
 
+	if (s4882)
+		request_module("i2c-amd756-s4882");
+
 	return 0;
 
  out_err:
@@ -395,7 +401,7 @@
 
 static void __devexit amd756_remove(struct pci_dev *dev)
 {
-	i2c_del_adapter(&amd756_adapter);
+	i2c_del_adapter(&amd756_smbus);
 	release_region(amd756_ioport, SMB_IOSIZE);
 }
 
@@ -420,5 +426,7 @@
 MODULE_DESCRIPTION("AMD756/766/768/8111 and nVidia nForce SMBus driver");
 MODULE_LICENSE("GPL");
 
+EXPORT_SYMBOL(amd756_smbus);
+
 module_init(amd756_init)
 module_exit(amd756_exit)


-- 
Jean Delvare
http://khali.linux-fr.org/