It works for me too
What about including this patch in the mainline?
Regards....
Eddi
Tested-by: Eddi De Pieri <eddi@xxxxxxxxxxx>
On Mon, Feb 27, 2012 at 9:42 AM, Thomas Brandon <tbrandonau@xxxxxxxxx> wrote:
>
> 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;
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
