[prev in list] [next in list] [prev in thread] [next in thread]
List: linux-i2c
Subject: Re: [PATCH] i2c multiplexer driver for Proliant microserver N36L
From: Thomas Brandon <tbrandonau () gmail ! com>
Date: 2012-02-27 8:42:53
Message-ID: CAM5MpD46pT1N2f5yZb0yW1O8CjH7B969TmvxDZztMfwBoyom3g () mail ! gmail ! com
[Download RAW message or body]
On Sun, Feb 26, 2012 at 10:30 PM, Mike Shestyrev wrote:
> Hi Thomas,
>
> I've applied you patch to i2c-piix4.c, kernel 2.26.33, the machine is N40L.
> It works!! Thank you.
> When probing SDA4 panel LEDs are turned off ant the power button stops
> to work.
> SMBus pins can be programmed as GPIO optionally, and the guess is that
> SMBus pins from some channel(s) are used to drive LEDs and to read buttons,
> and were programmed as GPIO by BIOS .
>
> Is it possible to read already programmed state for each channel
> before initialization?
> If it's GPIO - to skip it, if SMBus mode - to register.
>
> Regards,
> /Mike.
Thanks for testing Mike and glad to hear you had some success. Could
you use Reply to all in Gmail to cc the mailing list.
It is true that the pins are multiplexed and it looks like SDA4 is not
being used for SMBus, though it is set to the default which is part of
the PS2 interface (not present on the N40L) not GPIO. However I don't
think this is a problem, I would assume that these settings would
disconnect the SMBus module from those pins, though I could be wrong.
Further testing shows that the problem was that the port selection was
being changed by the driver and having the port set to anything other
than port 0 was causing the problems when restarting. I have now
modified the driver to restore the default value after each
transaction which appears to have fixed the problem.
There remains the problem that running sensors-detect on SDA3 causes a
bus collision and locks up the bus. This also means you see the same
reset issues and must remove the power cable to recover. I have
tracked this down to the ds1621_detect routine performing a word write
which hangs the chip at 0x4c. Removing this detection routine fixes
the issue (whatever chip is there is not identified). I gather this is
not a problem with the driver per se but rather the inherently unsafe
nature of sensors-detect and it is sensors-detect, if anything, that
should be modified to fix the issue. I will look into that further and
move that to the lm_sensors mailing list. For now I have attached a
modified version of sensors-detect which removes this chip.
With the updated patch and the modified sensors-detect I have been
able to probe all ports and have not encountered the power issue
again. If no further issues are identified I will seperate the patch
into stages and submit for review.
Tom.
--- linux-3.2.1-gentoo-r2/drivers/i2c/busses/i2c-piix4.c 2012-01-05
10:55:44.000000000 +1100
+++ dev/drivers/i2c/busses/i2c-piix4.c 2012-02-27 04:35:11.000000000 +1100
@@ -25,7 +25,10 @@
AMD Hudson-2
SMSC Victory66
- Note: we assume there can only be one device, with one SMBus interface.
+ Note: we assume there can only be one device.
+ The device can register multiple i2c_adapters (up to PIIX4_MAX_ADAPTERS).
+ For devices supporting multiple ports the i2c_adapter should provide
+ an i2c_algorithm to access them.
*/
#include <linux/module.h>
@@ -40,6 +43,7 @@
#include <linux/dmi.h>
#include <linux/acpi.h>
#include <linux/io.h>
+#include <linux/mutex.h>
/* PIIX4 SMBus address offsets */
@@ -78,6 +82,9 @@
#define PIIX4_WORD_DATA 0x0C
#define PIIX4_BLOCK_DATA 0x14
+/* Multi-port constants */
+#define PIIX4_MAX_ADAPTERS 4
+
/* insmod parameters */
/* If force is set to anything different from 0, we forcibly enable the
@@ -97,7 +104,14 @@ MODULE_PARM_DESC(force_addr,
static unsigned short piix4_smba;
static int srvrworks_csb5_delay;
static struct pci_driver piix4_driver;
-static struct i2c_adapter piix4_adapter;
+static struct i2c_adapter *piix4_adapters[PIIX4_MAX_ADAPTERS];
+
+/* SB800 globals */
+DEFINE_MUTEX(piix4_mutex_sb800);
+static u8 piix4_adapter_ports_sb800[4];
+static const char *piix4_port_names_sb800[4] = {
+ "SDA0", "SDA2", "SDA3", "SDA4"
+};
static struct dmi_system_id __devinitdata piix4_dmi_blacklist[] = {
{
@@ -284,6 +298,8 @@ static int __devinit piix4_setup_sb800(s
else
dev_dbg(&PIIX4_dev->dev, "Using SMI# for SMBus.\n");
+ mutex_init(&piix4_mutex_sb800);
+
dev_info(&PIIX4_dev->dev,
"SMBus Host Controller at 0x%x, revision %d\n",
piix4_smba, i2ccfg >> 4);
@@ -291,27 +307,27 @@ static int __devinit piix4_setup_sb800(s
return 0;
}
-static int piix4_transaction(void)
+static int piix4_transaction(struct i2c_adapter *adap)
{
int temp;
int result = 0;
int timeout = 0;
- dev_dbg(&piix4_adapter.dev, "Transaction (pre): CNT=%02x, CMD=%02x, "
+ dev_dbg(&adap->dev, "Transaction (pre): CNT=%02x, CMD=%02x, "
"ADD=%02x, DAT0=%02x, DAT1=%02x\n", inb_p(SMBHSTCNT),
inb_p(SMBHSTCMD), inb_p(SMBHSTADD), inb_p(SMBHSTDAT0),
inb_p(SMBHSTDAT1));
/* Make sure the SMBus host is ready to start transmitting */
if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
- dev_dbg(&piix4_adapter.dev, "SMBus busy (%02x). "
+ dev_dbg(&adap->dev, "SMBus busy (%02x). "
"Resetting...\n", temp);
outb_p(temp, SMBHSTSTS);
if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
- dev_err(&piix4_adapter.dev, "Failed! (%02x)\n", temp);
+ dev_err(&adap->dev, "Failed! (%02x)\n", temp);
return -EBUSY;
} else {
- dev_dbg(&piix4_adapter.dev, "Successful!\n");
+ dev_dbg(&adap->dev, "Successful!\n");
}
}
@@ -330,35 +346,35 @@ static int piix4_transaction(void)
/* If the SMBus is still busy, we give up */
if (timeout == MAX_TIMEOUT) {
- dev_err(&piix4_adapter.dev, "SMBus Timeout!\n");
+ dev_err(&adap->dev, "SMBus Timeout!\n");
result = -ETIMEDOUT;
}
if (temp & 0x10) {
result = -EIO;
- dev_err(&piix4_adapter.dev, "Error: Failed bus transaction\n");
+ dev_err(&adap->dev, "Error: Failed bus transaction\n");
}
if (temp & 0x08) {
result = -EIO;
- dev_dbg(&piix4_adapter.dev, "Bus collision! SMBus may be "
+ dev_dbg(&adap->dev, "Bus collision! SMBus may be "
"locked until next hard reset. (sorry!)\n");
/* Clock stops and slave is stuck in mid-transmission */
}
if (temp & 0x04) {
result = -ENXIO;
- dev_dbg(&piix4_adapter.dev, "Error: no response!\n");
+ dev_dbg(&adap->dev, "Error: no response!\n");
}
if (inb_p(SMBHSTSTS) != 0x00)
outb_p(inb(SMBHSTSTS), SMBHSTSTS);
if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
- dev_err(&piix4_adapter.dev, "Failed reset at end of "
+ dev_err(&adap->dev, "Failed reset at end of "
"transaction (%02x)\n", temp);
}
- dev_dbg(&piix4_adapter.dev, "Transaction (post): CNT=%02x, CMD=%02x, "
+ dev_dbg(&adap->dev, "Transaction (post): CNT=%02x, CMD=%02x, "
"ADD=%02x, DAT0=%02x, DAT1=%02x\n", inb_p(SMBHSTCNT),
inb_p(SMBHSTCMD), inb_p(SMBHSTADD), inb_p(SMBHSTDAT0),
inb_p(SMBHSTDAT1));
@@ -426,7 +442,7 @@ static s32 piix4_access(struct i2c_adapt
outb_p((size & 0x1C) + (ENABLE_INT9 & 1), SMBHSTCNT);
- status = piix4_transaction();
+ status = piix4_transaction(adap);
if (status)
return status;
@@ -454,6 +470,47 @@ static s32 piix4_access(struct i2c_adapt
return 0;
}
+/* Handles access to multiple SMBus ports on the SB800.
+ * The port is selected by bits 2:1 of the smb_en register (0x2C).
+ * NOTE: The selected port must be returned to the initial selection to
+ * avoid problems on certain systems.
+ * Return negative errno on error.
+ */
+static s32 piix4_access_sb800(struct i2c_adapter * adap, u16 addr,
+ unsigned short flags, char read_write,
+ u8 command, int size, union i2c_smbus_data * data)
+{
+ unsigned short smba_idx = 0xcd6;
+ u8 smba_en_lo, smb_en = 0x2c;
+ u8 port;
+ int retval;
+
+ mutex_lock(&piix4_mutex_sb800);
+
+ if (!request_region(smba_idx, 2, "smba_idx")) {
+ dev_err(&adap->dev, "SMBus base address index region "
+ "0x%x already in use!\n", smba_idx);
+ retval = -EBUSY;
+ goto ERROR;
+ }
+ outb_p(smb_en, smba_idx);
+ smba_en_lo = inb_p(smba_idx + 1);
+
+ port = *((u8*)adap->algo_data);
+ if ((smba_en_lo & 6) != (port << 1))
+ outb_p((smba_en_lo & ~6) | (port << 1), smba_idx + 1);
+
+ retval = piix4_access(adap, addr, flags, read_write, command, size, data);
+
+ outb_p(smba_en_lo, smba_idx + 1);
+ release_region(smba_idx, 2);
+
+ ERROR:
+ mutex_unlock(&piix4_mutex_sb800);
+
+ return retval;
+}
+
static u32 piix4_func(struct i2c_adapter *adapter)
{
return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
@@ -461,18 +518,91 @@ static u32 piix4_func(struct i2c_adapter
I2C_FUNC_SMBUS_BLOCK_DATA;
}
-static const struct i2c_algorithm smbus_algorithm = {
+static const struct i2c_algorithm piix4_smbus_algorithm = {
.smbus_xfer = piix4_access,
.functionality = piix4_func,
};
-static struct i2c_adapter piix4_adapter = {
- .owner = THIS_MODULE,
- .class = I2C_CLASS_HWMON | I2C_CLASS_SPD,
- .algo = &smbus_algorithm,
+static const struct i2c_algorithm piix4_smbus_algorithm_sb800 = {
+ .smbus_xfer = piix4_access_sb800,
+ .functionality = piix4_func,
};
-static const struct pci_device_id piix4_ids[] = {
+static int __devinit piix4_setup_adapters(struct pci_dev *dev)
+{
+ int retval;
+
+ if (!(piix4_adapters[0] = kzalloc(sizeof(**piix4_adapters), GFP_KERNEL)))
+ return -ENOMEM;
+
+ piix4_adapters[0]->owner = THIS_MODULE;
+ piix4_adapters[0]->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
+ piix4_adapters[0]->algo = &piix4_smbus_algorithm;
+
+ /* set up the sysfs linkage to our parent device */
+ piix4_adapters[0]->dev.parent = &dev->dev;
+
+ snprintf(piix4_adapters[0]->name, sizeof(piix4_adapters[0]->name),
+ "SMBus PIIX4 adapter at %04x", piix4_smba);
+
+ if ((retval = i2c_add_adapter(piix4_adapters[0]))) {
+ dev_err(&dev->dev, "Couldn't register adapter!\n");
+ kfree(piix4_adapters[0]);
+ piix4_adapters[0] = NULL;
+ }
+
+ return retval;
+}
+
+static int __devinit piix4_setup_adapters_sb800(struct pci_dev *dev)
+{
+ int i, retval;
+
+ /* Register 4 adapters */
+ for (i = 0; i < 4; i++) {
+ if (!(piix4_adapters[i] = kzalloc(sizeof(**piix4_adapters), GFP_KERNEL))) {
+ dev_err(&dev->dev, "Out of memory!\n");
+ retval = -ENOMEM;
+ goto ERROR;
+
+ }
+ piix4_adapters[i]->owner = THIS_MODULE;
+ piix4_adapters[i]->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
+ piix4_adapters[i]->algo = &piix4_smbus_algorithm_sb800;
+
+ piix4_adapter_ports_sb800[i] = i;
+ piix4_adapters[i]->algo_data = &(piix4_adapter_ports_sb800[i]);
+
+ /* set up the sysfs linkage to our parent device */
+ piix4_adapters[i]->dev.parent = &dev->dev;
+
+ snprintf(piix4_adapters[i]->name, sizeof(piix4_adapters[i]->name),
+ "SB800 SMBus adapter %s at %04x",
+ piix4_port_names_sb800[i], piix4_smba);
+
+ if ((retval = i2c_add_adapter(piix4_adapters[i]))) {
+ dev_err(&dev->dev, "Couldn't register SB800 adapter %s (%d)!\n",
+ piix4_port_names_sb800[i], retval);
+ kfree(piix4_adapters[i]);
+ piix4_adapters[i] = NULL;
+ goto ERROR;
+ }
+ }
+
+ return retval;
+
+ ERROR:
+ dev_err(&dev->dev,
+ "Error setting up SB800 adapters. Unregistering all adapters!\n");
+ for (i--; i >= 0; i--) {
+ i2c_del_adapter(piix4_adapters[i]);
+ kfree(piix4_adapters[i]);
+ piix4_adapters[i] = NULL;
+ }
+ return retval;
+}
+
+static DEFINE_PCI_DEVICE_TABLE(piix4_ids) = {
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371AB_3) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82443MX_3) },
{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_SLC90E66_3) },
@@ -504,23 +634,18 @@ static int __devinit piix4_probe(struct
if ((dev->vendor == PCI_VENDOR_ID_ATI &&
dev->device == PCI_DEVICE_ID_ATI_SBX00_SMBUS &&
dev->revision >= 0x40) ||
- dev->vendor == PCI_VENDOR_ID_AMD)
- /* base address location etc changed in SB800 */
- retval = piix4_setup_sb800(dev, id);
- else
- retval = piix4_setup(dev, id);
-
- if (retval)
- return retval;
-
- /* set up the sysfs linkage to our parent device */
- piix4_adapter.dev.parent = &dev->dev;
-
- snprintf(piix4_adapter.name, sizeof(piix4_adapter.name),
- "SMBus PIIX4 adapter at %04x", piix4_smba);
+ dev->vendor == PCI_VENDOR_ID_AMD) {
+ /* SB800 uses a different base address and has 4 SMBus ports */
+ if ((retval = piix4_setup_sb800(dev, id)))
+ return retval;
+ retval = piix4_setup_adapters_sb800(dev);
+ } else {
+ if ((retval = piix4_setup(dev, id)))
+ return retval;
+ retval = piix4_setup_adapters(dev);
+ }
- if ((retval = i2c_add_adapter(&piix4_adapter))) {
- dev_err(&dev->dev, "Couldn't register adapter!\n");
+ if (retval) {
release_region(piix4_smba, SMBIOSIZE);
piix4_smba = 0;
}
@@ -530,8 +655,16 @@ static int __devinit piix4_probe(struct
static void __devexit piix4_remove(struct pci_dev *dev)
{
+ int i;
+
+ for (i = 0; i < PIIX4_MAX_ADAPTERS; i++) {
+ if (piix4_adapters[i]) {
+ i2c_del_adapter(piix4_adapters[i]);
+ kfree(piix4_adapters[i]);
+ piix4_adapters[i] = NULL;
+ }
+ }
if (piix4_smba) {
- i2c_del_adapter(&piix4_adapter);
release_region(piix4_smba, SMBIOSIZE);
piix4_smba = 0;
}
["sensors-detect-n40l" (application/octet-stream)]
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic