[PATCH v2 0/2] I2C: SIS964: Bus driver

[PATCH v2 0/2] I2C: SIS964: Bus driver

[Amaury Decrême]

This series of patches are a I2C bus driver for the SIS964.


The SIS964 is nearly the same as SIS630 for the SMBUS :
	- Most of the SMBus registers (Status, Enable, Control,
	  Host Control, etc.) are at the same adresses + 0x60
	- The register count is 21 instead of 20

The SIS964 name is ambiguous as this chip isn't part of the SIS96X
family and behaves differently.

The datasheets have been used to develop the driver based on the
exsiting driver for I2C SIS630 . 


Content of the patches:
	- 1/2 The driver itself
	- 2/2 Documentation, Kconfig, Makefile, Maintainers


Changes:
	V2
	- Remove of the insertion on include/linux/pci_ids.h
	and no quirk needed
	- Cosmetics on the source code + patch split

Amaury Decrême (2):
  I2C: SIS964: Bus driver - Driver
  I2C: SIS964: Bus driver - other files

 Documentation/i2c/busses/i2c-sis964 |   36 +++
 MAINTAINERS                         |    7 +
 drivers/i2c/busses/Kconfig          |   12 +-
 drivers/i2c/busses/Makefile         |    1 +
 drivers/i2c/busses/i2c-sis964.c     |  578 +++++++++++++++++++++++++++++++++++
 5 files changed, 633 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/i2c/busses/i2c-sis964
 create mode 100644 drivers/i2c/busses/i2c-sis964.c

-- 
1.7.8.6

[PATCH v2 1/2] I2C: SIS964: Bus driver - Driver

[Amaury Decrême]

This patchs contains the SMBus driver for SIS964.
Built with Sis datasheets and by forking the SIS630 driver.

Signed-off-by: Amaury Decrême <amaury.decreme@gmail.com>
---
 drivers/i2c/busses/i2c-sis964.c |  578 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 578 insertions(+), 0 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-sis964.c

diff --git a/drivers/i2c/busses/i2c-sis964.c b/drivers/i2c/busses/i2c-sis964.c
new file mode 100644
index 0000000..91b6ec9
--- /dev/null
+++ b/drivers/i2c/busses/i2c-sis964.c
@@ -0,0 +1,578 @@
+/*
+    Copyright (c) 2012 Amaury Decrême <amaury.decreme@gmail.com>
+
+    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.
+*/
+
+/*
+   Changes:
+   11.08.2011
+	Fork of original i2c-sis630 - Alexander Malysh <amalysh@web.de>
+	Adapted for SiS964 with datasheets
+			- Amaury Decrême <amaury.decreme@gmail.com>
+*/
+
+/*
+   Supports:
+	SIS 964
+
+   Note: we assume there can only be one device, with one SMBus interface.
+*/
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/pci.h>
+#include <linux/ioport.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/acpi.h>
+#include <linux/io.h>
+
+#define PCI_DEVICE_ID_SI_964	0x0964
+
+
+/* SIS964 SMBus registers */
+#define SMB_STS			0xE0	/* status */
+#define SMB_EN			0xE1	/* status enable */
+#define SMB_CNT			0xE2	/* Control */
+#define SMBHOST_CNT		0xE3	/* Host Control */
+#define SMB_ADDR		0xE4	/* Address */
+#define SMB_CMD			0xE5	/* Command */
+#define SMB_PERRCHK		0xE6	/* Packet Error Check */
+#define SMB_COUNT		0xE7	/* Byte Count */
+#define SMB_BYTE		0xE8	/* ~0x8F data byte field */
+#define SMBDEV_ADDR		0xF0	/* Device Address */
+#define SMB_DB0			0xF1	/* Device byte0 */
+#define SMB_DB1			0xF2	/* Device byte1 */
+#define SMB_SAA			0xF3	/* Host slave alias address */
+#define SMB_PCOUNT		0xF4	/* processed byte count */
+
+
+/* SMB_STS register */
+#define SMBALT_STS		0x80	/* Slave alert */
+#define BYTE_DONE_STS		0x10	/* Byte Done Status / Block Array */
+#define SMBMAS_STS		0x08	/* Host Master */
+#define SMBCOL_STS		0x04	/* Collision */
+#define SMBERR_STS		0x02	/* Device error */
+
+/* SMB_CNT register */
+#define SMBCLK_SEL		0x20	/* Host master clock selection */
+#define SMB_PROBE		0x02	/* Bus Probe */
+#define SMB_HOSTBUSY		0x01	/* Host Busy */
+
+/* SMBHOST_CNT register */
+#define SMB_KILL		0x20	/* Kill */
+#define SMB_START		0x10	/* Start */
+#define SMB_PTL			0x07	/* Command Protocol */
+
+
+/* SMB_ADDR register */
+#define SMB_ADDRESS		0xFE	/* Adress */
+#define SMB_RW			0x01	/* Read/Write */
+
+
+/* SMB_BYTE register */
+#define SMB_BYTE0		0xFF	/* Byte 0 */
+#define SMB_BYTE1		0xFF00	/* Byte 1 */
+
+/* register count for request_region */
+#define SIS964_SMB_IOREGION	21
+
+/* PCI address constants */
+/* acpi base address register  */
+#define SIS964_ACPI_BASE_REG	0x74
+/* bios control register */
+#define SIS964_BIOS_CTL_REG	0x40
+
+/* Other settings */
+#define MAX_TIMEOUT		500
+
+/* SIS964 constants */
+#define SIS964_QUICK		0x00
+#define SIS964_BYTE		0x01
+#define SIS964_BYTE_DATA	0x02
+#define SIS964_WORD_DATA	0x03
+#define SIS964_PCALL		0x04
+#define SIS964_BLOCK_DATA	0x05
+
+static struct pci_driver sis964_driver;
+
+/* insmod parameters */
+static bool low_clock;
+static bool force;
+module_param(low_clock, bool, 0);
+MODULE_PARM_DESC(low_clock, "Set Host Master Clock to 28KHz (default 56KHz).");
+module_param(force, bool, 0);
+MODULE_PARM_DESC(force, "Forcibly enable the SIS964. DANGEROUS!");
+
+/* acpi base address */
+static unsigned short acpi_base;
+
+/* supported chips */
+static int supported[] = {
+	PCI_DEVICE_ID_SI_964,
+	0 /* terminates the list */
+};
+
+static inline u8 sis964_read(u8 reg)
+{
+	return inb(acpi_base + reg);
+}
+
+static inline void sis964_write(u8 reg, u8 data)
+{
+	outb(data, acpi_base + reg);
+}
+
+static int sis964_transaction_start(struct i2c_adapter *adap,
+					int ptl, u8 *oldclock)
+{
+	int tmp = 0;
+
+	/* Clear status register */
+	sis964_write(SMB_STS, 0xFF);
+
+	/* Make sure the SMBus host is ready to start transmitting. */
+	tmp = sis964_read(SMB_CNT);
+	if (tmp & (SMB_PROBE | SMB_HOSTBUSY)) {
+		dev_dbg(&adap->dev,
+			"Bus busy (status 0x%02x). Killing transaction.\n",
+			tmp);
+
+		sis964_write(SMBHOST_CNT, SMB_KILL);
+
+		return -EBUSY;
+	}
+
+	/* Set Host Master Clock to 28KHz if requested */
+	if (low_clock) {
+		*oldclock = sis964_read(SMB_CNT);
+		sis964_write(SMB_CNT, SMBCLK_SEL);
+	}
+
+	/* start the transaction by setting bit start and protocol */
+	sis964_write(SMBHOST_CNT, SMB_START | (ptl & SMB_PTL));
+
+	return 0;
+}
+
+static int sis964_transaction_wait(struct i2c_adapter *adap, int ptl)
+{
+	int tmp = 0, timeout = 0;
+
+	/* Wait 30us, valid for 28Khz and 56Khz */
+	udelay(30);
+
+	tmp = sis964_read(SMB_STS);
+	if (!(tmp & SMB_PROBE) && (tmp & SMB_HOSTBUSY)) {
+		dev_dbg(&adap->dev,
+			"Host busy (status 0x%02x). Restarting transaction.\n",
+			tmp);
+		sis964_write(SMBHOST_CNT, SMB_KILL);
+		return -EAGAIN;
+	}
+
+	while (!(ptl == SIS964_BLOCK_DATA && (tmp & BYTE_DONE_STS))
+		&& !(tmp & (SMBMAS_STS | SMBCOL_STS | SMBERR_STS))
+		&& (timeout++ < MAX_TIMEOUT)) {
+
+		/* Datasheets: wait 4ms max at 28Khz and
+		 * 2ms max at 56Khz for 8 bytes */
+		if (low_clock)
+			udelay(4000);
+		else
+			udelay(2000);
+		tmp = sis964_read(SMB_STS);
+	}
+
+	/* If the SMBus is still busy, we give up */
+	if (timeout > MAX_TIMEOUT) {
+		dev_dbg(&adap->dev,
+			"Bus Timeout (status 0x%02x)!\n", tmp);
+		return -ETIMEDOUT;
+	}
+
+	if (tmp & SMBERR_STS) {
+		dev_dbg(&adap->dev,
+			"Failed bus transaction (status 0x%02x)!\n", tmp);
+		return -ENXIO;
+	}
+
+	if (tmp & SMBCOL_STS) {
+		dev_err(&adap->dev,
+			"Bus collision (status 0x%02x)!\n", tmp);
+		sis964_write(SMB_STS, tmp & ~SMBCOL_STS);
+		return -EAGAIN;
+	}
+
+	return 0;
+}
+
+static void sis964_transaction_end(u8 oldclock)
+{
+	/* clear all status "sticky" bits */
+	sis964_write(SMB_STS, 0xFF);
+
+	/* restore old Host Master Clock if low_clock is set */
+	if (low_clock)
+		sis964_write(SMB_CNT, oldclock & SMBCLK_SEL);
+}
+
+static int sis964_transaction(struct i2c_adapter *adap, int ptl)
+{
+	int tmp = 0, timeout = 0;
+	u8 oldclock = 0;
+
+	do {
+		tmp = sis964_transaction_start(adap, ptl, &oldclock);
+		if (tmp)
+			return tmp;
+
+		tmp = sis964_transaction_wait(adap, ptl);
+		sis964_transaction_end(oldclock);
+	} while (tmp == -EAGAIN && timeout++ < MAX_TIMEOUT);
+
+	if (timeout > MAX_TIMEOUT) {
+		dev_dbg(&adap->dev, "Bus timeout !\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int sis964_block_data_read(struct i2c_adapter *adap,
+				union i2c_smbus_data *data)
+{
+	int i, len = 0, tmp = 0;
+	u8 oldclock = 0;
+
+	data->block[0] = len = 0;
+
+	tmp = sis964_transaction_start(adap, SIS964_BLOCK_DATA, &oldclock);
+	if (tmp)
+		return tmp;
+
+	do {
+		tmp = sis964_transaction_wait(adap, SIS964_BLOCK_DATA);
+		if (tmp) {
+			dev_dbg(&adap->dev, "Transaction wait failed\n");
+			break;
+		}
+
+		/* if this first transaction then read byte count */
+		if (len == 0)
+			data->block[0] = sis964_read(SMB_COUNT);
+
+		if (data->block[0] > 32)
+			data->block[0] = 32;
+
+		dev_dbg(&adap->dev, "Block data read len=0x%x\n",
+			data->block[0]);
+
+		for (i = 0; i < 8 && len < data->block[0]; i++, len++) {
+			dev_dbg(&adap->dev, "Read i=%d len=%d\n", i, len);
+			data->block[len+1] = sis964_read(SMB_BYTE+i);
+		}
+
+		/* clear BYTE_DONE_STS */
+		sis964_write(SMB_STS, BYTE_DONE_STS);
+	} while (len < data->block[0]);
+
+	sis964_transaction_end(oldclock);
+
+	return 0;
+}
+
+
+static int sis964_block_data_write(struct i2c_adapter *adap,
+				union i2c_smbus_data *data)
+{
+
+	int i, len = 0, tmp = 0;
+	u8 oldclock = 0;
+
+	len = data->block[0];
+	if (len < 0)
+		len = 0;
+	else if (len > 32)
+		len = 32;
+
+	sis964_write(SMB_COUNT, len);
+
+	for (i = 1; i <= len; i++) {
+		dev_dbg(&adap->dev, "Set data 0x%02x\n", data->block[i]);
+
+		/* set data */
+		sis964_write(SMB_BYTE+(i-1)%8, data->block[i]);
+		if (i == 8 || (len < 8 && i == len)) {
+
+			/* first transaction */
+			tmp = sis964_transaction_start(adap, SIS964_BLOCK_DATA,
+					&oldclock);
+			if (tmp)
+				return tmp;
+
+		} else if ((i-1)%8 == 7 || i == len) {
+			if (i > 8) {
+				dev_dbg(&adap->dev,
+				"Clear smbary_sts len=%d i=%d\n", len, i);
+
+				/*
+				   If this is not first transaction,
+				   we must clear sticky bit.
+				   clear BYTE_DONE-STS
+				*/
+				sis964_write(SMB_STS, BYTE_DONE_STS);
+			}
+			tmp = sis964_transaction_wait(adap,
+					SIS964_BLOCK_DATA);
+			if (tmp) {
+				dev_dbg(&adap->dev,
+					"Transaction wait failed\n");
+				break;
+			}
+		}
+	}
+
+	sis964_transaction_end(oldclock);
+
+	return 0;
+}
+
+static int sis964_block_data(struct i2c_adapter *adap,
+				union i2c_smbus_data *data, int read_write)
+{
+	if (read_write == I2C_SMBUS_WRITE)
+		return sis964_block_data_write(adap, data);
+	else
+		return sis964_block_data_read(adap, data);
+}
+
+/* Return negative errno on error. */
+static s32 sis964_access(struct i2c_adapter *adap, u16 addr,
+			 unsigned short flags, char read_write,
+			 u8 command, int ptl, union i2c_smbus_data *data)
+{
+	int tmp = 0;
+
+	switch (ptl) {
+	case I2C_SMBUS_QUICK:
+		sis964_write(SMB_ADDR, ((addr << 1) & SMB_ADDRESS) |
+					(read_write & SMB_RW));
+		ptl = SIS964_QUICK;
+		break;
+	case I2C_SMBUS_BYTE:
+		sis964_write(SMB_ADDR, ((addr << 1) & SMB_ADDRESS) |
+					(read_write & SMB_RW));
+		if (read_write == I2C_SMBUS_WRITE)
+			sis964_write(SMB_CMD, command);
+		ptl = SIS964_BYTE;
+		break;
+	case I2C_SMBUS_BYTE_DATA:
+		sis964_write(SMB_ADDR, ((addr << 1) & SMB_ADDRESS) |
+					(read_write & SMB_RW));
+		sis964_write(SMB_CMD, command);
+		if (read_write == I2C_SMBUS_WRITE)
+			sis964_write(SMB_BYTE, data->byte);
+		ptl = SIS964_BYTE_DATA;
+		break;
+	case I2C_SMBUS_PROC_CALL:
+	case I2C_SMBUS_WORD_DATA:
+		sis964_write(SMB_ADDR, ((addr << 1) & SMB_ADDRESS) |
+					(read_write & SMB_RW));
+		sis964_write(SMB_CMD, command);
+		if (read_write == I2C_SMBUS_WRITE) {
+			sis964_write(SMB_BYTE, data->word & SMB_BYTE0);
+			sis964_write(SMB_BYTE + 1,
+					(data->word & SMB_BYTE1) >> 8);
+		}
+		ptl = (ptl == I2C_SMBUS_PROC_CALL ?
+				SIS964_PCALL : SIS964_WORD_DATA);
+		break;
+	case I2C_SMBUS_BLOCK_DATA:
+		sis964_write(SMB_ADDR, ((addr << 1) & SMB_ADDRESS) |
+					(read_write & SMB_RW));
+		sis964_write(SMB_CMD, command);
+		ptl = SIS964_BLOCK_DATA;
+		return sis964_block_data(adap, data, read_write);
+	default:
+		dev_warn(&adap->dev, "Unsupported transaction %d\n",
+			 ptl);
+		return -EOPNOTSUPP;
+	}
+
+	tmp = sis964_transaction(adap, ptl);
+	if (tmp)
+		return tmp;
+
+	if (ptl != SIS964_PCALL &&
+		(read_write == I2C_SMBUS_WRITE || ptl == SIS964_QUICK)) {
+		return 0;
+	}
+
+	switch (ptl) {
+	case SIS964_BYTE:
+	case SIS964_BYTE_DATA:
+		data->byte = sis964_read(SMB_BYTE);
+		break;
+	case SIS964_PCALL:
+	case SIS964_WORD_DATA:
+		data->word = sis964_read(SMB_BYTE) +
+				(sis964_read(SMB_BYTE + 1) << 8);
+		break;
+	}
+
+	return 0;
+}
+
+static u32 sis964_func(struct i2c_adapter *adapter)
+{
+	/* SMBus Command protocol supported */
+	return I2C_FUNC_SMBUS_QUICK |		/* Quick command */
+		I2C_FUNC_SMBUS_BYTE |		/* Send/Receive Byte */
+		I2C_FUNC_SMBUS_BYTE_DATA |	/* Read/Write Byte Data */
+		I2C_FUNC_SMBUS_WORD_DATA |	/* Read/Write Word Data */
+		I2C_FUNC_SMBUS_PROC_CALL |	/* Process Call */
+		I2C_FUNC_SMBUS_BLOCK_DATA;	/* Read/Write Block Data */
+}
+
+static int __devinit sis964_setup(struct pci_dev *sis964_dev)
+{
+	unsigned char b;
+	struct pci_dev *dummy = NULL;
+	int tmp = 0, i;
+
+	/* check for supported SiS devices */
+	for (i = 0; supported[i] > 0 && dummy == NULL; i++)
+		dummy = pci_get_device(PCI_VENDOR_ID_SI, supported[i], dummy);
+
+	if (dummy) {
+		pci_dev_put(dummy);
+	} else if (force) {
+		dev_err(&sis964_dev->dev,
+			"WARNING: Can't detect SIS964 compatible device, but "
+			"loading because of force option enabled\n");
+	} else {
+		dev_err(&sis964_dev->dev,
+			"WARNING: Can't detect SIS964 compatible device!\n");
+		return -ENODEV;
+	}
+
+
+	/*
+	   Enable ACPI first , so we can accsess reg 74-75
+	   in acpi io space and read acpi base addr
+	*/
+	if (pci_read_config_byte(sis964_dev, SIS964_BIOS_CTL_REG, &b)) {
+		dev_err(&sis964_dev->dev, "Error: Can't read bios ctl reg\n");
+		return -ENODEV;
+	}
+	/* if ACPI already enabled , do nothing */
+	if (!(b & 0x80) &&
+	    pci_write_config_byte(sis964_dev, SIS964_BIOS_CTL_REG, b | 0x80)) {
+		dev_err(&sis964_dev->dev, "Error: Can't enable ACPI\n");
+		return -ENODEV;
+	}
+
+	/* Determine the ACPI base address */
+	if (pci_read_config_word(sis964_dev, SIS964_ACPI_BASE_REG,
+				&acpi_base)) {
+		dev_err(&sis964_dev->dev,
+				"Error: Can't determine ACPI base address\n");
+		return -ENODEV;
+	}
+
+	dev_dbg(&sis964_dev->dev, "ACPI base at 0x%04x\n", acpi_base);
+
+	tmp = acpi_check_region(acpi_base + SMB_STS, SIS964_SMB_IOREGION,
+				   sis964_driver.name);
+	if (tmp) {
+		acpi_base = 0;
+		return -ENODEV;
+	}
+
+	/* Everything is happy, let's grab the memory and set things up. */
+	if (!request_region(acpi_base + SMB_STS, SIS964_SMB_IOREGION,
+			    sis964_driver.name)) {
+		dev_err(&sis964_dev->dev,
+			"SMBus registers 0x%04x-0x%04x already in use!\n",
+			acpi_base + SMB_STS, acpi_base + SMB_SAA);
+		acpi_base = 0;
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+
+static const struct i2c_algorithm smbus_algorithm = {
+	.smbus_xfer	= sis964_access,
+	.functionality	= sis964_func,
+};
+
+static struct i2c_adapter sis964_adapter = {
+	.owner		= THIS_MODULE,
+	.class		= I2C_CLASS_HWMON | I2C_CLASS_SPD,
+	.algo		= &smbus_algorithm,
+};
+
+static DEFINE_PCI_DEVICE_TABLE(sis964_ids) = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_964) },
+	{ 0 }
+};
+
+MODULE_DEVICE_TABLE(pci, sis964_ids);
+
+static int __devinit sis964_probe(struct pci_dev *dev,
+					const struct pci_device_id *id)
+{
+	if (sis964_setup(dev)) {
+		dev_err(&dev->dev,
+		       "SIS964 comp. bus not detected, module not inserted.\n");
+		return -ENODEV;
+	}
+
+	/* set up the sysfs linkage to our parent device */
+	sis964_adapter.dev.parent = &dev->dev;
+
+	snprintf(sis964_adapter.name, sizeof(sis964_adapter.name),
+		 "SMBus SIS964 adapter at %04xh", acpi_base + SMB_STS);
+
+	return i2c_add_adapter(&sis964_adapter);
+}
+
+static void __devexit sis964_remove(struct pci_dev *dev)
+{
+	if (acpi_base) {
+		i2c_del_adapter(&sis964_adapter);
+		release_region(acpi_base + SMB_STS, SIS964_SMB_IOREGION);
+		acpi_base = 0;
+	}
+}
+
+
+static struct pci_driver sis964_driver = {
+	.name		= "sis964_smbus",
+	.id_table	= sis964_ids,
+	.probe		= sis964_probe,
+	.remove		= __devexit_p(sis964_remove),
+};
+
+module_pci_driver(sis964_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Amaury Decrême <amaury.decreme@gmail.com>");
+MODULE_DESCRIPTION("SiS964 SMBus driver");
-- 
1.7.8.6

[PATCH v2 2/2] I2C: SIS964: Bus driver - other files

[Amaury Decrême]

This patch contains:
	- Documentation for i2c-sis964
	- Kconfig, Makefile, Maintainers

Signed-off-by: Amaury Decrême <amaury.decreme@gmail.com>
---
 Documentation/i2c/busses/i2c-sis964 |   36 +++++++++++++++++++++++++++++++++++
 MAINTAINERS                         |    7 ++++++
 drivers/i2c/busses/Kconfig          |   12 ++++++++++-
 drivers/i2c/busses/Makefile         |    1 +
 4 files changed, 55 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/i2c/busses/i2c-sis964

diff --git a/Documentation/i2c/busses/i2c-sis964 b/Documentation/i2c/busses/i2c-sis964
new file mode 100644
index 0000000..9cc9846
--- /dev/null
+++ b/Documentation/i2c/busses/i2c-sis964
@@ -0,0 +1,36 @@
+Kernel driver i2c-sis964
+
+Supported adapters:
+  * Silicon Integrated Systems Corp (SiS)
+	964 chipset (Datasheet by SiS)
+  * Possible other SiS chipsets with the same registers and clocks
+
+Author:	Amaury Decrême <amaury.decreme@gmail.com>
+
+Module Parameters
+-----------------
+
+* force = [1|0] 	Forcibly enable the SIS964. DANGEROUS!
+			This can be interesting for chipsets not named
+			above to check if it works for you chipset.
+
+* low_clock = [1|0] 	Set Host Master Clock to 28KHz (defaut 56Khz)
+
+Description
+-----------
+
+This SMBus driver works for motherboards with the SiS964 chipset.
+Despite its name, SIS964 is not compatible with the SIS96X family.
+
+If you see something like this:
+
+00:02.0 ISA bridge: Silicon Integrated Systems [SiS] SiS964 [MuTIOL Media IO]
+
+in your 'lspci' output , then this driver is for your chipset.
+
+Thank You
+---------
+Alexander Malysh <amalysh@web.de>
+Mark M. Hoffman <mhoffman@lightlink.com>
+Who have worked on i2c-sis630 used to fork i2c-sis964
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 94b823f..9a26e94 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6238,6 +6238,13 @@ S:	Maintained
 F:	Documentation/i2c/busses/i2c-sis96x
 F:	drivers/i2c/busses/i2c-sis96x.c
 
+SIS 964 I2C/SMBUS DRIVER
+M:     "Amaury Decrême" <amaury.decreme@gmail.com>
+L:     linux-i2c@vger.kernel.org
+S:     Maintained
+F:     Documentation/i2c/busses/i2c-sis96i4
+F:     drivers/i2c/busses/i2c-sis964.c
+
 SIS FRAMEBUFFER DRIVER
 M:	Thomas Winischhofer <thomas@winischhofer.net>
 W:	http://www.winischhofer.net/linuxsisvga.shtml
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index b4aaa1b..4fc68a2 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -194,7 +194,7 @@ config I2C_SIS630
 	  will be called i2c-sis630.
 
 config I2C_SIS96X
-	tristate "SiS 96x"
+	tristate "SiS 96x (except SiS964)"
 	depends on PCI
 	help
 	  If you say yes to this option, support will be included for the SiS
@@ -211,6 +211,16 @@ config I2C_SIS96X
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-sis96x.
 
+config I2C_SIS964
+	tristate "SiS 964"
+	depends on PCI && EXPERIMENTAL
+	help
+	  If you say yes to this option, support will be included for the SiS
+	  964 SMBus (a subset of I2C) interfaces.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-sis964.
+
 config I2C_VIA
 	tristate "VIA VT82C586B"
 	depends on PCI && EXPERIMENTAL
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index ce3c2be..b985bc8 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_I2C_NFORCE2_S4985)	+= i2c-nforce2-s4985.o
 obj-$(CONFIG_I2C_PIIX4)		+= i2c-piix4.o
 obj-$(CONFIG_I2C_SIS5595)	+= i2c-sis5595.o
 obj-$(CONFIG_I2C_SIS630)	+= i2c-sis630.o
+obj-$(CONFIG_I2C_SIS964)	+= i2c-sis964.o
 obj-$(CONFIG_I2C_SIS96X)	+= i2c-sis96x.o
 obj-$(CONFIG_I2C_VIA)		+= i2c-via.o
 obj-$(CONFIG_I2C_VIAPRO)	+= i2c-viapro.o
-- 
1.7.8.6

Re: [PATCH v2 0/2] I2C: SIS964: Bus driver

[Jean Delvare]

Hi Amaury,

On Fri,  3 Aug 2012 17:35:43 +0200, Amaury Decrême wrote:
> This series of patches are a I2C bus driver for the SIS964.
> 
> 
> The SIS964 is nearly the same as SIS630 for the SMBUS :
> 	- Most of the SMBus registers (Status, Enable, Control,
> 	  Host Control, etc.) are at the same adresses + 0x60
> 	- The register count is 21 instead of 20
> 
> The SIS964 name is ambiguous as this chip isn't part of the SIS96X
> family and behaves differently.
> 
> The datasheets have been used to develop the driver based on the
> exsiting driver for I2C SIS630 . 

What is the rationale for not adding support for the new chip to
i2c-sis630 then? That would probably be a lot easier to review.

> Content of the patches:
> 	- 1/2 The driver itself
> 	- 2/2 Documentation, Kconfig, Makefile, Maintainers

This split makes little sense. Integration should come with the driver,
otherwise even build-testing is impossible.


-- 
Jean Delvare

Re: [PATCH v2 0/2] I2C: SIS964: Bus driver

[Amaury Decrême]

Hi Jean,

>
> What is the rationale for not adding support for the new chip to
> i2c-sis630 then? That would probably be a lot easier to review.
>

I was afraid that adding SIS964 support to i2c-sis630 would lead
to confusion.

I can try to submit a patch for i2c-sis630. In this case, do you think
we should keep the 630 name or change it to something else to
avoid misunderstanding ?


>
> This split makes little sense. Integration should come with the driver,
> otherwise even build-testing is impossible.
>

Sorry for that. As the patch V1 was rather big, I was hesitating for
the same reason but posted it split. I'll "unsplit" for the V3 (unless
i2c-sis630 patch).


Amaury Decrême

Re: [PATCH v2 0/2] I2C: SIS964: Bus driver

[Jean Delvare]

On Fri, 3 Aug 2012 18:36:36 +0200, Amaury Decrême wrote:
> > What is the rationale for not adding support for the new chip to
> > i2c-sis630 then? That would probably be a lot easier to review.
> >
> 
> I was afraid that adding SIS964 support to i2c-sis630 would lead
> to confusion.

There's nothing confusing, drivers supporting several devices are
legion. If the devices are really almost compatible, reusing an
existing driver is the way to go.

> I can try to submit a patch for i2c-sis630. In this case, do you think
> we should keep the 630 name or change it to something else to
> avoid misunderstanding ?

Keep the name. It is very common to name drivers by the name of the
first supported device, and changing a driver name is always a source
of trouble.

-- 
Jean Delvare

Re: [PATCH v2 0/2] I2C: SIS964: Bus driver

[Amaury Decrême]

> There's nothing confusing, drivers supporting several devices are
> legion. If the devices are really almost compatible, reusing an
> existing driver is the way to go.

With that in mind, here is an alpha preview of what the patch will
look like if SIS964 support is added in i2c-sis630.


diff --git a/drivers/i2c/busses/i2c-sis630.c b/drivers/i2c/busses/i2c-sis630.c
index 5d6723b..861d58b 100644
--- a/drivers/i2c/busses/i2c-sis630.c
+++ b/drivers/i2c/busses/i2c-sis630.c
@@ -33,6 +33,8 @@
        Fixed logical error by restoring of Host Master Clock
    31.07.2003
        Added block data read/write support.
+   03.08.2012
+       Added support of SiS964 - Amaury Decrême <amaury.decreme@gmail.com>
 */

 /*
@@ -41,6 +43,7 @@
    Supports:
        SIS 630
        SIS 730
+       SIS 964

    Note: we assume there can only be one device, with one SMBus interface.
 */
@@ -55,22 +58,22 @@
 #include <linux/acpi.h>
 #include <linux/io.h>

+/* SIS964 id, defined here as we are the only file using it */
+#define PCI_DEVICE_ID_SI_964   0x0964
+
 /* SIS630 SMBus registers */
-#define SMB_STS                        0x80    /* status */
-#define SMB_EN                 0x81    /* status enable */
-#define SMB_CNT                        0x82
-#define SMBHOST_CNT            0x83
-#define SMB_ADDR               0x84
-#define SMB_CMD                        0x85
-#define SMB_PCOUNT             0x86    /* processed count */
-#define SMB_COUNT              0x87
-#define SMB_BYTE               0x88    /* ~0x8F data byte field */
-#define SMBDEV_ADDR            0x90
-#define SMB_DB0                        0x91
-#define SMB_DB1                        0x92
-#define SMB_SAA                        0x93
-
-/* register count for request_region */
+#define SMB_STS                        0x00 + offset   /* status */
+#define SMB_CNT                        0x02 + offset   /* control */
+#define SMBHOST_CNT            0x03 + offset   /* host control */
+#define SMB_ADDR               0x04 + offset   /* address */
+#define SMB_CMD                        0x05 + offset   /* command */
+#define SMB_COUNT              0x07 + offset   /* byte count */
+#define SMB_BYTE               0x08 + offset   /* ~0x8F data byte field */
+#define SMB_SAA                        0x13 + offset   /* host slave
alias address */
+
+/* register count for request_region
+ * As we don't use SMB_PCOUNT 20 is ok for SiS630 and SiS964
+ */
 #define SIS630_SMB_IOREGION    20

 /* PCI address constants */
@@ -107,9 +110,13 @@ static unsigned short acpi_base;
 static int supported[] = {
        PCI_DEVICE_ID_SI_630,
        PCI_DEVICE_ID_SI_730,
+       PCI_DEVICE_ID_SI_964,
        0 /* terminates the list */
 };

+/* SMB registers offset */
+static int offset;
+
 static inline u8 sis630_read(u8 reg)
 {
        return inb(acpi_base + reg);
@@ -412,6 +419,10 @@ static int __devinit sis630_setup(struct pci_dev
*sis630_dev)
                return -ENODEV;
        }

+       if (supported[i] == PCI_DEVICE_ID_SI_964)
+               offset = 0xE0;
+       else
+               offset = 0x80;
        /*
           Enable ACPI first , so we can accsess reg 74-75
           in acpi io space and read acpi base addr
@@ -474,6 +485,7 @@ static struct i2c_adapter sis630_adapter = {

 static DEFINE_PCI_DEVICE_TABLE(sis630_ids) = {
        { PCI_DEVICE(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503) },
+       { PCI_DEVICE(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_964) },
        { PCI_DEVICE(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC) },
        { 0, }
 };
@@ -482,6 +494,7 @@ MODULE_DEVICE_TABLE (pci, sis630_ids);

 static int __devinit sis630_probe(struct pci_dev *dev, const struct
pci_device_id *id)
 {
+       dev_dbg(&dev->dev, "salut");
        if (sis630_setup(dev)) {
                dev_err(&dev->dev, "SIS630 comp. bus not detected,
module not inserted.\n");
                return -ENODEV;


--
Amaury Decrême

Re: [PATCH v2 0/2] I2C: SIS964: Bus driver

[Jean Delvare]

Salut Amaury,

On Sat, 4 Aug 2012 00:38:29 +0000, Amaury Decrême wrote:
> > There's nothing confusing, drivers supporting several devices are
> > legion. If the devices are really almost compatible, reusing an
> > existing driver is the way to go.
> 
> With that in mind, here is an alpha preview of what the patch will
> look like if SIS964 support is added in i2c-sis630.

If that's all that is needed to get the SIS964 supported, then we
definitely don't want a separate driver.

> diff --git a/drivers/i2c/busses/i2c-sis630.c b/drivers/i2c/busses/i2c-sis630.c
> index 5d6723b..861d58b 100644
> --- a/drivers/i2c/busses/i2c-sis630.c
> +++ b/drivers/i2c/busses/i2c-sis630.c
> @@ -33,6 +33,8 @@
>         Fixed logical error by restoring of Host Master Clock
>     31.07.2003
>         Added block data read/write support.
> +   03.08.2012
> +       Added support of SiS964 - Amaury Decrême <amaury.decreme@gmail.com>
>  */
> 
>  /*
> @@ -41,6 +43,7 @@
>     Supports:
>         SIS 630
>         SIS 730
> +       SIS 964
> 
>     Note: we assume there can only be one device, with one SMBus interface.
>  */
> @@ -55,22 +58,22 @@
>  #include <linux/acpi.h>
>  #include <linux/io.h>
> 
> +/* SIS964 id, defined here as we are the only file using it */
> +#define PCI_DEVICE_ID_SI_964   0x0964
> +
>  /* SIS630 SMBus registers */
> -#define SMB_STS                        0x80    /* status */
> -#define SMB_EN                 0x81    /* status enable */
> -#define SMB_CNT                        0x82
> -#define SMBHOST_CNT            0x83
> -#define SMB_ADDR               0x84
> -#define SMB_CMD                        0x85
> -#define SMB_PCOUNT             0x86    /* processed count */
> -#define SMB_COUNT              0x87
> -#define SMB_BYTE               0x88    /* ~0x8F data byte field */
> -#define SMBDEV_ADDR            0x90
> -#define SMB_DB0                        0x91
> -#define SMB_DB1                        0x92
> -#define SMB_SAA                        0x93
> -
> -/* register count for request_region */
> +#define SMB_STS                        0x00 + offset   /* status */
> +#define SMB_CNT                        0x02 + offset   /* control */
> +#define SMBHOST_CNT            0x03 + offset   /* host control */
> +#define SMB_ADDR               0x04 + offset   /* address */
> +#define SMB_CMD                        0x05 + offset   /* command */
> +#define SMB_COUNT              0x07 + offset   /* byte count */
> +#define SMB_BYTE               0x08 + offset   /* ~0x8F data byte field */
> +#define SMB_SAA                        0x13 + offset   /* host slave
> alias address */

Your email client apparently folds long lines, you'll have to fix that
when you resend.

The above definitions are dangerous. At the very least you need
parentheses around. Better would be to pass the offset as a parameter.
Best would be to only define the constants and add the offset in
the calling code. There are only 8 locations so it should be easy.

> +
> +/* register count for request_region
> + * As we don't use SMB_PCOUNT 20 is ok for SiS630 and SiS964
> + */
>  #define SIS630_SMB_IOREGION    20
> 
>  /* PCI address constants */
> @@ -107,9 +110,13 @@ static unsigned short acpi_base;
>  static int supported[] = {
>         PCI_DEVICE_ID_SI_630,
>         PCI_DEVICE_ID_SI_730,
> +       PCI_DEVICE_ID_SI_964,
>         0 /* terminates the list */
>  };
> 
> +/* SMB registers offset */
> +static int offset;

Please move this declaration close to acpi_base.

It would be more efficient to not introduce an offset variable but
rather smbus_base = acpi_base + offset. That way you save an addition
each time you access a register.

> +
>  static inline u8 sis630_read(u8 reg)
>  {
>         return inb(acpi_base + reg);
> @@ -412,6 +419,10 @@ static int __devinit sis630_setup(struct pci_dev
> *sis630_dev)
>                 return -ENODEV;
>         }
> 
> +       if (supported[i] == PCI_DEVICE_ID_SI_964)
> +               offset = 0xE0;
> +       else
> +               offset = 0x80;
>         /*
>            Enable ACPI first , so we can accsess reg 74-75
>            in acpi io space and read acpi base addr
> @@ -474,6 +485,7 @@ static struct i2c_adapter sis630_adapter = {
> 
>  static DEFINE_PCI_DEVICE_TABLE(sis630_ids) = {
>         { PCI_DEVICE(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503) },
> +       { PCI_DEVICE(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_964) },
>         { PCI_DEVICE(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC) },
>         { 0, }
>  };
> @@ -482,6 +494,7 @@ MODULE_DEVICE_TABLE (pci, sis630_ids);
> 
>  static int __devinit sis630_probe(struct pci_dev *dev, const struct
> pci_device_id *id)
>  {
> +       dev_dbg(&dev->dev, "salut");

Bien le bonjour à toi aussi :)

>         if (sis630_setup(dev)) {
>                 dev_err(&dev->dev, "SIS630 comp. bus not detected,
> module not inserted.\n");
>                 return -ENODEV;
> 

-- 
Jean Delvare

Re: [PATCH v2 0/2] I2C: SIS964: Bus driver

[Amaury Decrême]

Salut Jean,

Thanks for your comments.
I will post now a new serie of patches for SIS630.

--
Amaury Decrême

[PATCH 0/2] I2C: SIS630: Add SIS964 support

[Amaury Decrême]

This series of patches add SIS964 support to i2c-sis630.

The SIS964 is very similar to SIS630 for the SMBus.
	+------------------------+--------------------+-------------------+
	|                        |     SIS630/730     |      SIS964       |
	+------------------------+--------------------+-------------------+
	| Clock                  | 14kHz/56kHz        | 55.56kHz/27.78kHz |
	| SMBus registers offset | 0x80               | 0xE0              |
	| SMB_CNT                | Bit 1 = Slave Busy | Bit 1 = Bus probe |
	| SMB_COUNT              | 4:0 bits           | 5:0 bits          |
	+------------------------+--------------------+-------------------+

The other differences doesn't affect the functions provided by the original
driver.

The SIS964 name is ambiguous as this chip isn't part of the SIS96X family and
behaves differently.

Done with SiS 630 and 964 datasheets.

Amaury Decrême (2):
  I2C: SIS630: Add SiS964 support
  I2C: SIS630: Cosmetics

 Documentation/i2c/busses/i2c-sis630 |   17 +-
 drivers/i2c/busses/Kconfig          |    4 +-
 drivers/i2c/busses/i2c-sis630.c     |  445 +++++++++++++++++++++--------------
 3 files changed, 278 insertions(+), 188 deletions(-)

-- 
1.7.8.6

[PATCH 1/2] I2C: SIS630: Add SiS964 support

[Amaury Decrême]

This patch add SiS964 support to i2c-sis630.

Signed-off-by: Amaury Decrême <amaury.decreme@gmail.com>
---
 Documentation/i2c/busses/i2c-sis630 |   17 +++-
 drivers/i2c/busses/Kconfig          |    4 +-
 drivers/i2c/busses/i2c-sis630.c     |  148 ++++++++++++++++++++++-------------
 3 files changed, 107 insertions(+), 62 deletions(-)

diff --git a/Documentation/i2c/busses/i2c-sis630 b/Documentation/i2c/busses/i2c-sis630
index 0b96973..46b62e7 100644
--- a/Documentation/i2c/busses/i2c-sis630
+++ b/Documentation/i2c/busses/i2c-sis630
@@ -4,20 +4,23 @@ Supported adapters:
   * Silicon Integrated Systems Corp (SiS)
 	630 chipset (Datasheet: available at http://www.sfr-fresh.com/linux)
 	730 chipset
+	964 chipset
   * Possible other SiS chipsets ?
 
 Author: Alexander Malysh <amalysh@web.de>
+	Amaury Decrême <amaury.decreme@gmail.com> - SiS964 patch
 
 Module Parameters
 -----------------
 
-* force = [1|0] Forcibly enable the SIS630. DANGEROUS!
+* force = [1|0] Forcibly enable the driver. DANGEROUS!
 		This can be interesting for chipsets not named
 		above to check if it works for you chipset, but DANGEROUS!
 
-* high_clock = [1|0] Forcibly set Host Master Clock to 56KHz (default,
-			what your BIOS use). DANGEROUS! This should be a bit
-			faster, but freeze some systems (i.e. my Laptop).
+* clock_sel = [1|0] Forcibly set Host Master Clock.
+			SiS630/730	56kHz instead of 14kHz
+			SiS964		27.78kHz instead of 55.56 kHz
+			DANGEROUS! It can freeze some systems.
 
 
 Description
@@ -36,6 +39,12 @@ or like this:
 00:00.0 Host bridge: Silicon Integrated Systems [SiS] 730 Host (rev 02)
 00:01.0 ISA bridge: Silicon Integrated Systems [SiS] 85C503/5513
 
+or like this:
+
+00:00.0 Host bridge: Silicon Integrated Systems [SiS] 760/M760 Host (rev 02)
+00:02.0 ISA bridge: Silicon Integrated Systems [SiS] SiS964 [MuTIOL Media IO]
+							LPC Controller (rev 36)
+
 in your 'lspci' output , then this driver is for your chipset.
 
 Thank You
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index b4aaa1b..ee9ca06 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -184,11 +184,11 @@ config I2C_SIS5595
 	  will be called i2c-sis5595.
 
 config I2C_SIS630
-	tristate "SiS 630/730"
+	tristate "SiS 630/730/964"
 	depends on PCI
 	help
 	  If you say yes to this option, support will be included for the
-	  SiS630 and SiS730 SMBus (a subset of I2C) interface.
+	  SiS630, SiS730 and SiS964 SMBus (a subset of I2C) interface.
 
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-sis630.
diff --git a/drivers/i2c/busses/i2c-sis630.c b/drivers/i2c/busses/i2c-sis630.c
index 5d6723b..c950397 100644
--- a/drivers/i2c/busses/i2c-sis630.c
+++ b/drivers/i2c/busses/i2c-sis630.c
@@ -24,7 +24,7 @@
    18.09.2002
 	Added SIS730 as supported.
    21.09.2002
-	Added high_clock module option.If this option is set
+	Added clock_sel module option.If this option is set
 	used Host Master Clock 56KHz (default 14KHz).For now we save old Host
 	Master Clock and after transaction completed restore (otherwise
 	it's confuse BIOS and hung Machine).
@@ -32,7 +32,9 @@
 	Fixed typo in sis630_access
 	Fixed logical error by restoring of Host Master Clock
    31.07.2003
-   	Added block data read/write support.
+	Added block data read/write support.
+   03.08.2012
+	Added support of SiS964 - Amaury Decrême <amaury.decreme@gmail.com>
 */
 
 /*
@@ -41,6 +43,18 @@
    Supports:
 	SIS 630
 	SIS 730
+	SIS 964
+
+   Notable differences between chips:
+	+------------------------+--------------------+-------------------+
+	|                        |     SIS630/730     |      SIS964       |
+	+------------------------+--------------------+-------------------+
+	| Clock                  | 14kHz/56kHz        | 55.56kHz/27.78kHz |
+	| SMBus registers offset | 0x80               | 0xE0              |
+	| SMB_CNT                | Bit 1 = Slave Busy | Bit 1 = Bus probe |
+	| SMB_COUNT              | 4:0 bits           | 5:0 bits          |
+	+------------------------+--------------------+-------------------+
+	(Other differences doesn't affect the functions provided by the driver)
 
    Note: we assume there can only be one device, with one SMBus interface.
 */
@@ -55,22 +69,22 @@
 #include <linux/acpi.h>
 #include <linux/io.h>
 
-/* SIS630 SMBus registers */
-#define SMB_STS			0x80	/* status */
-#define SMB_EN			0x81	/* status enable */
-#define SMB_CNT			0x82
-#define SMBHOST_CNT		0x83
-#define SMB_ADDR		0x84
-#define SMB_CMD			0x85
-#define SMB_PCOUNT		0x86	/* processed count */
-#define SMB_COUNT		0x87
-#define SMB_BYTE		0x88	/* ~0x8F data byte field */
-#define SMBDEV_ADDR		0x90
-#define SMB_DB0			0x91
-#define SMB_DB1			0x92
-#define SMB_SAA			0x93
-
-/* register count for request_region */
+/* SIS964 id, defined here as we are the only file using it */
+#define PCI_DEVICE_ID_SI_964	0x0964
+
+/* SIS630/730/964 SMBus registers */
+#define SMB_STS			0x00	/* status */
+#define SMB_CNT			0x02	/* control */
+#define SMBHOST_CNT		0x03	/* host control */
+#define SMB_ADDR		0x04	/* address */
+#define SMB_CMD			0x05	/* command */
+#define SMB_COUNT		0x07	/* byte count */
+#define SMB_BYTE		0x08	/* ~0x8F data byte field */
+#define SMB_SAA			0x13	/* host slave alias address */
+
+/* register count for request_region
+ * As we don't use SMB_PCOUNT, 20 is ok for SiS630 and SiS964
+ */
 #define SIS630_SMB_IOREGION	20
 
 /* PCI address constants */
@@ -93,31 +107,33 @@
 static struct pci_driver sis630_driver;
 
 /* insmod parameters */
-static bool high_clock;
+static bool clock_sel;
 static bool force;
-module_param(high_clock, bool, 0);
-MODULE_PARM_DESC(high_clock, "Set Host Master Clock to 56KHz (default 14KHz).");
+module_param(clock_sel, bool, 0);
+MODULE_PARM_DESC(clock_sel,
+"Set Host Master Clock to 56kHz for SIS630/730 and to 27.78kHz for SIS964.");
 module_param(force, bool, 0);
-MODULE_PARM_DESC(force, "Forcibly enable the SIS630. DANGEROUS!");
+MODULE_PARM_DESC(force, "Forcibly enable the driver. DANGEROUS!");
 
-/* acpi base address */
-static unsigned short acpi_base;
+/* SMBus base adress */
+static unsigned short smbus_base;
 
 /* supported chips */
 static int supported[] = {
 	PCI_DEVICE_ID_SI_630,
 	PCI_DEVICE_ID_SI_730,
+	PCI_DEVICE_ID_SI_964,
 	0 /* terminates the list */
 };
 
 static inline u8 sis630_read(u8 reg)
 {
-	return inb(acpi_base + reg);
+	return inb(smbus_base + reg);
 }
 
 static inline void sis630_write(u8 reg, u8 data)
 {
-	outb(data, acpi_base + reg);
+	outb(data, smbus_base + reg);
 }
 
 static int sis630_transaction_start(struct i2c_adapter *adap, int size, u8 *oldclock)
@@ -143,8 +159,7 @@ static int sis630_transaction_start(struct i2c_adapter *adap, int size, u8 *oldc
 
 	dev_dbg(&adap->dev, "saved clock 0x%02x\n", *oldclock);
 
-	/* disable timeout interrupt , set Host Master Clock to 56KHz if requested */
-	if (high_clock)
+	if (clock_sel)
 		sis630_write(SMB_CNT, 0x20);
 	else
 		sis630_write(SMB_CNT, (*oldclock & ~0x40));
@@ -185,12 +200,14 @@ static int sis630_transaction_wait(struct i2c_adapter *adap, int size)
 
 	if (temp & 0x04) {
 		dev_err(&adap->dev, "Bus collision!\n");
-		result = -EIO;
-		/*
-		  TBD: Datasheet say:
-		  the software should clear this bit and restart SMBUS operation.
-		  Should we do it or user start request again?
-		*/
+		/* Datasheet:
+		 * SMBus Collision (SMBCOL_STS)
+		 * This bit is set when a SMBus Collision condition occurs and
+		 * SMBus Host loses in the bus arbitration. The software should
+		 * clear this bit and re-start SMBus operation.
+		 */
+		sis630_write(SMB_STS, temp & ~0x04);
+		return -EAGAIN;
 	}
 
 	return result;
@@ -198,18 +215,17 @@ static int sis630_transaction_wait(struct i2c_adapter *adap, int size)
 
 static void sis630_transaction_end(struct i2c_adapter *adap, u8 oldclock)
 {
-	int temp = 0;
-
-	/* clear all status "sticky" bits */
-	sis630_write(SMB_STS, temp);
+	/* clear all status "sticky" bits
+	 * Datasheet:
+	 * SMBus Status (SMB_STS)
+	 * The following registers are all sticky bits and only can be
+	 * cleared by writing a one to their corresponding fields.
+	 */
+	sis630_write(SMB_STS, 0xFF);
 
 	dev_dbg(&adap->dev, "SMB_CNT before clock restore 0x%02x\n", sis630_read(SMB_CNT));
 
-	/*
-	 * restore old Host Master Clock if high_clock is set
-	 * and oldclock was not 56KHz
-	 */
-	if (high_clock && !(oldclock & 0x20))
+	if (clock_sel && !(oldclock & 0x20))
 		sis630_write(SMB_CNT,(sis630_read(SMB_CNT) & ~0x20));
 
 	dev_dbg(&adap->dev, "SMB_CNT after clock restore 0x%02x\n", sis630_read(SMB_CNT));
@@ -218,12 +234,21 @@ static void sis630_transaction_end(struct i2c_adapter *adap, u8 oldclock)
 static int sis630_transaction(struct i2c_adapter *adap, int size)
 {
 	int result = 0;
+	int timeout = 0;
 	u8 oldclock = 0;
 
-	result = sis630_transaction_start(adap, size, &oldclock);
-	if (!result) {
-		result = sis630_transaction_wait(adap, size);
-		sis630_transaction_end(adap, oldclock);
+	/* We loop in case of collisions */
+	do {
+		result = sis630_transaction_start(adap, size, &oldclock);
+		if (!result) {
+			result = sis630_transaction_wait(adap, size);
+			sis630_transaction_end(adap, oldclock);
+		}
+	} while (result == -EAGAIN && timeout++ < MAX_TIMEOUT);
+
+	if (timeout > MAX_TIMEOUT) {
+		dev_dbg(&adap->dev, "Too many collisions !\n");
+		return -ETIMEDOUT;
 	}
 
 	return result;
@@ -394,6 +419,8 @@ static int __devinit sis630_setup(struct pci_dev *sis630_dev)
 	unsigned char b;
 	struct pci_dev *dummy = NULL;
 	int retval, i;
+	/* acpi base address */
+	static unsigned short acpi_base;
 
 	/* check for supported SiS devices */
 	for (i=0; supported[i] > 0 ; i++) {
@@ -438,16 +465,24 @@ static int __devinit sis630_setup(struct pci_dev *sis630_dev)
 
 	dev_dbg(&sis630_dev->dev, "ACPI base at 0x%04x\n", acpi_base);
 
-	retval = acpi_check_region(acpi_base + SMB_STS, SIS630_SMB_IOREGION,
+	if (supported[i] == PCI_DEVICE_ID_SI_964)
+		smbus_base = acpi_base + 0xE0;
+	else
+		smbus_base = acpi_base + 0x80;
+
+	dev_dbg(&sis630_dev->dev, "SMBus base at 0x%04x\n", smbus_base);
+
+	retval = acpi_check_region(smbus_base + SMB_STS, SIS630_SMB_IOREGION,
 				   sis630_driver.name);
 	if (retval)
 		goto exit;
 
 	/* Everything is happy, let's grab the memory and set things up. */
-	if (!request_region(acpi_base + SMB_STS, SIS630_SMB_IOREGION,
+	if (!request_region(smbus_base + SMB_STS, SIS630_SMB_IOREGION,
 			    sis630_driver.name)) {
-		dev_err(&sis630_dev->dev, "SMBus registers 0x%04x-0x%04x already "
-			"in use!\n", acpi_base + SMB_STS, acpi_base + SMB_SAA);
+		dev_err(&sis630_dev->dev,
+			"SMBus registers 0x%04x-0x%04x already in use!\n",
+			smbus_base + SMB_STS, smbus_base + SMB_SAA);
 		retval = -EBUSY;
 		goto exit;
 	}
@@ -456,7 +491,7 @@ static int __devinit sis630_setup(struct pci_dev *sis630_dev)
 
 exit:
 	if (retval)
-		acpi_base = 0;
+		smbus_base = 0;
 	return retval;
 }
 
@@ -474,6 +509,7 @@ static struct i2c_adapter sis630_adapter = {
 
 static DEFINE_PCI_DEVICE_TABLE(sis630_ids) = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_964) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC) },
 	{ 0, }
 };
@@ -491,17 +527,17 @@ static int __devinit sis630_probe(struct pci_dev *dev, const struct pci_device_i
 	sis630_adapter.dev.parent = &dev->dev;
 
 	snprintf(sis630_adapter.name, sizeof(sis630_adapter.name),
-		 "SMBus SIS630 adapter at %04x", acpi_base + SMB_STS);
+		 "SMBus SIS630 adapter at %04x", smbus_base + SMB_STS);
 
 	return i2c_add_adapter(&sis630_adapter);
 }
 
 static void __devexit sis630_remove(struct pci_dev *dev)
 {
-	if (acpi_base) {
+	if (smbus_base) {
 		i2c_del_adapter(&sis630_adapter);
-		release_region(acpi_base + SMB_STS, SIS630_SMB_IOREGION);
-		acpi_base = 0;
+		release_region(smbus_base + SMB_STS, SIS630_SMB_IOREGION);
+		smbus_base = 0;
 	}
 }
 
-- 
1.7.8.6

[PATCH 2/2] I2C: SIS630: Cosmetics

[Amaury Decrême]

This patch replaces SMBus hex values by human readable constants and
correct some checkpatch -f errors.

Signed-off-by: Amaury Decrême <amaury.decreme@gmail.com>
---
 drivers/i2c/busses/i2c-sis630.c |  311 ++++++++++++++++++++++-----------------
 1 files changed, 178 insertions(+), 133 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sis630.c b/drivers/i2c/busses/i2c-sis630.c
index c950397..8dff4b9 100644
--- a/drivers/i2c/busses/i2c-sis630.c
+++ b/drivers/i2c/busses/i2c-sis630.c
@@ -19,7 +19,7 @@
 /*
    Changes:
    24.08.2002
-   	Fixed the typo in sis630_access (Thanks to Mark M. Hoffman)
+	Fixed the typo in sis630_access (Thanks to Mark M. Hoffman)
 	Changed sis630_transaction.(Thanks to Mark M. Hoffman)
    18.09.2002
 	Added SIS730 as supported.
@@ -82,6 +82,32 @@
 #define SMB_BYTE		0x08	/* ~0x8F data byte field */
 #define SMB_SAA			0x13	/* host slave alias address */
 
+/* SMB_STS register */
+#define SMBALT_STS		0x80	/* Slave alert */
+#define BYTE_DONE_STS		0x10	/* Byte Done Status / Block Array */
+#define SMBMAS_STS		0x08	/* Host Master */
+#define SMBCOL_STS		0x04	/* Collision */
+#define SMBERR_STS		0x02	/* Device error */
+
+/* SMB_CNT register */
+#define MSTO_EN			0x40	/* Host Master Timeout Enable */
+#define SMBCLK_SEL		0x20	/* Host master clock selection */
+#define SMB_PROBE		0x02	/* Bus Probe */
+#define SMB_HOSTBUSY		0x01	/* Host Busy */
+
+/* SMBHOST_CNT register */
+#define SMB_KILL		0x20	/* Kill */
+#define SMB_START		0x10	/* Start */
+#define SMB_PTL			0x07	/* Command Protocol */
+
+/* SMB_ADDR register */
+#define SMB_ADDRESS		0xFE	/* Adress */
+#define SMB_RW			0x01	/* Read/Write */
+
+/* SMB_BYTE register */
+#define SMB_BYTE0		0xFF	/* Byte 0 */
+#define SMB_BYTE1		0xFF00	/* Byte 1 */
+
 /* register count for request_region
  * As we don't use SMB_PCOUNT, 20 is ok for SiS630 and SiS964
  */
@@ -136,23 +162,26 @@ static inline void sis630_write(u8 reg, u8 data)
 	outb(data, smbus_base + reg);
 }
 
-static int sis630_transaction_start(struct i2c_adapter *adap, int size, u8 *oldclock)
+static int sis630_transaction_start(struct i2c_adapter *adap, int size,
+					u8 *oldclock)
 {
-        int temp;
+	int tmp;
 
 	/* Make sure the SMBus host is ready to start transmitting. */
-	if ((temp = sis630_read(SMB_CNT) & 0x03) != 0x00) {
-		dev_dbg(&adap->dev, "SMBus busy (%02x).Resetting...\n",temp);
+	tmp = sis630_read(SMB_CNT);
+	if ((tmp & (SMB_PROBE | SMB_HOSTBUSY)) != 0x00) {
+		dev_dbg(&adap->dev, "SMBus busy (%02x). Resetting...\n", tmp);
 		/* kill smbus transaction */
-		sis630_write(SMBHOST_CNT, 0x20);
+		sis630_write(SMBHOST_CNT, SMB_KILL);
 
-		if ((temp = sis630_read(SMB_CNT) & 0x03) != 0x00) {
-			dev_dbg(&adap->dev, "Failed! (%02x)\n", temp);
+		tmp = sis630_read(SMB_CNT);
+		if (tmp & (SMB_PROBE | SMB_HOSTBUSY)) {
+			dev_dbg(&adap->dev, "Failed! (%02x)\n", tmp);
 			return -EBUSY;
-                } else {
+		} else {
 			dev_dbg(&adap->dev, "Successful!\n");
 		}
-        }
+	}
 
 	/* save old clock, so we can prevent machine for hung */
 	*oldclock = sis630_read(SMB_CNT);
@@ -160,45 +189,46 @@ static int sis630_transaction_start(struct i2c_adapter *adap, int size, u8 *oldc
 	dev_dbg(&adap->dev, "saved clock 0x%02x\n", *oldclock);
 
 	if (clock_sel)
-		sis630_write(SMB_CNT, 0x20);
+		sis630_write(SMB_CNT, SMBCLK_SEL);
 	else
-		sis630_write(SMB_CNT, (*oldclock & ~0x40));
+		sis630_write(SMB_CNT, (*oldclock & ~MSTO_EN));
 
 	/* clear all sticky bits */
-	temp = sis630_read(SMB_STS);
-	sis630_write(SMB_STS, temp & 0x1e);
+	tmp = sis630_read(SMB_STS);
+	sis630_write(SMB_STS, tmp & (BYTE_DONE_STS | SMBMAS_STS
+					| SMBCOL_STS | SMBERR_STS));
 
 	/* start the transaction by setting bit 4 and size */
-	sis630_write(SMBHOST_CNT,0x10 | (size & 0x07));
+	sis630_write(SMBHOST_CNT, SMB_START | (size & SMB_PTL));
 
 	return 0;
 }
 
 static int sis630_transaction_wait(struct i2c_adapter *adap, int size)
 {
-	int temp, result = 0, timeout = 0;
+	int tmp, timeout = 0;
 
 	/* We will always wait for a fraction of a second! */
 	do {
 		msleep(1);
-		temp = sis630_read(SMB_STS);
+		tmp = sis630_read(SMB_STS);
 		/* check if block transmitted */
-		if (size == SIS630_BLOCK_DATA && (temp & 0x10))
-			break;
-	} while (!(temp & 0x0e) && (timeout++ < MAX_TIMEOUT));
+	} while (!(size == SIS630_BLOCK_DATA && (tmp & BYTE_DONE_STS))
+		&& !(tmp & (SMBMAS_STS | SMBCOL_STS | SMBERR_STS))
+		&& (timeout++ < MAX_TIMEOUT));
 
 	/* If the SMBus is still busy, we give up */
 	if (timeout > MAX_TIMEOUT) {
 		dev_dbg(&adap->dev, "SMBus Timeout!\n");
-		result = -ETIMEDOUT;
+		return -ETIMEDOUT;
 	}
 
-	if (temp & 0x02) {
+	if (tmp & SMBERR_STS) {
 		dev_dbg(&adap->dev, "Error: Failed bus transaction\n");
-		result = -ENXIO;
+		return -ENXIO;
 	}
 
-	if (temp & 0x04) {
+	if (tmp & SMBCOL_STS) {
 		dev_err(&adap->dev, "Bus collision!\n");
 		/* Datasheet:
 		 * SMBus Collision (SMBCOL_STS)
@@ -206,11 +236,11 @@ static int sis630_transaction_wait(struct i2c_adapter *adap, int size)
 		 * SMBus Host loses in the bus arbitration. The software should
 		 * clear this bit and re-start SMBus operation.
 		 */
-		sis630_write(SMB_STS, temp & ~0x04);
+		sis630_write(SMB_STS, tmp & ~SMBCOL_STS);
 		return -EAGAIN;
 	}
 
-	return result;
+	return 0;
 }
 
 static void sis630_transaction_end(struct i2c_adapter *adap, u8 oldclock)
@@ -223,38 +253,41 @@ static void sis630_transaction_end(struct i2c_adapter *adap, u8 oldclock)
 	 */
 	sis630_write(SMB_STS, 0xFF);
 
-	dev_dbg(&adap->dev, "SMB_CNT before clock restore 0x%02x\n", sis630_read(SMB_CNT));
+	dev_dbg(&adap->dev, "SMB_CNT before clock restore 0x%02x\n",
+		sis630_read(SMB_CNT));
 
-	if (clock_sel && !(oldclock & 0x20))
-		sis630_write(SMB_CNT,(sis630_read(SMB_CNT) & ~0x20));
+	if (clock_sel && !(oldclock & SMBCLK_SEL))
+		sis630_write(SMB_CNT, sis630_read(SMB_CNT) & ~SMBCLK_SEL);
 
-	dev_dbg(&adap->dev, "SMB_CNT after clock restore 0x%02x\n", sis630_read(SMB_CNT));
+	dev_dbg(&adap->dev, "SMB_CNT after clock restore 0x%02x\n",
+		sis630_read(SMB_CNT));
 }
 
 static int sis630_transaction(struct i2c_adapter *adap, int size)
 {
-	int result = 0;
+	int tmp;
 	int timeout = 0;
 	u8 oldclock = 0;
 
 	/* We loop in case of collisions */
 	do {
-		result = sis630_transaction_start(adap, size, &oldclock);
-		if (!result) {
-			result = sis630_transaction_wait(adap, size);
+		tmp = sis630_transaction_start(adap, size, &oldclock);
+		if (!tmp) {
+			tmp = sis630_transaction_wait(adap, size);
 			sis630_transaction_end(adap, oldclock);
 		}
-	} while (result == -EAGAIN && timeout++ < MAX_TIMEOUT);
+	} while (tmp == -EAGAIN && timeout++ < MAX_TIMEOUT);
 
 	if (timeout > MAX_TIMEOUT) {
 		dev_dbg(&adap->dev, "Too many collisions !\n");
 		return -ETIMEDOUT;
 	}
 
-	return result;
+	return 0;
 }
 
-static int sis630_block_data(struct i2c_adapter *adap, union i2c_smbus_data *data, int read_write)
+static int sis630_block_data(struct i2c_adapter *adap,
+				union i2c_smbus_data *data, int read_write)
 {
 	int i, len = 0, rc = 0;
 	u8 oldclock = 0;
@@ -266,39 +299,43 @@ static int sis630_block_data(struct i2c_adapter *adap, union i2c_smbus_data *dat
 		else if (len > 32)
 			len = 32;
 		sis630_write(SMB_COUNT, len);
-		for (i=1; i <= len; i++) {
-			dev_dbg(&adap->dev, "set data 0x%02x\n", data->block[i]);
+		for (i = 1; i <= len; i++) {
+			dev_dbg(&adap->dev, "set data 0x%02x\n",
+				data->block[i]);
 			/* set data */
 			sis630_write(SMB_BYTE+(i-1)%8, data->block[i]);
-			if (i==8 || (len<8 && i==len)) {
-				dev_dbg(&adap->dev, "start trans len=%d i=%d\n",len ,i);
+			if (i == 8 || (len < 8 && i == len)) {
+				dev_dbg(&adap->dev, "start trans len=%d i=%d\n",
+					len, i);
 				/* first transaction */
 				rc = sis630_transaction_start(adap,
 						SIS630_BLOCK_DATA, &oldclock);
 				if (rc)
 					return rc;
-			}
-			else if ((i-1)%8 == 7 || i==len) {
-				dev_dbg(&adap->dev, "trans_wait len=%d i=%d\n",len,i);
-				if (i>8) {
-					dev_dbg(&adap->dev, "clear smbary_sts len=%d i=%d\n",len,i);
+			} else if ((i-1)%8 == 7 || i == len) {
+				dev_dbg(&adap->dev, "trans_wait len=%d i=%d\n",
+					len, i);
+				if (i > 8) {
+					dev_dbg(&adap->dev,
+						"clr smbary_sts len=%d i=%d\n",
+						len, i);
 					/*
 					   If this is not first transaction,
 					   we must clear sticky bit.
 					   clear SMBARY_STS
 					*/
-					sis630_write(SMB_STS,0x10);
+					sis630_write(SMB_STS, BYTE_DONE_STS);
 				}
 				rc = sis630_transaction_wait(adap,
 						SIS630_BLOCK_DATA);
 				if (rc) {
-					dev_dbg(&adap->dev, "trans_wait failed\n");
+					dev_dbg(&adap->dev,
+						"trans_wait failed\n");
 					break;
 				}
 			}
 		}
-	}
-	else {
+	} else {
 		/* read request */
 		data->block[0] = len = 0;
 		rc = sis630_transaction_start(adap,
@@ -319,18 +356,21 @@ static int sis630_block_data(struct i2c_adapter *adap, union i2c_smbus_data *dat
 			if (data->block[0] > 32)
 				data->block[0] = 32;
 
-			dev_dbg(&adap->dev, "block data read len=0x%x\n", data->block[0]);
+			dev_dbg(&adap->dev, "block data read len=0x%x\n",
+				data->block[0]);
 
-			for (i=0; i < 8 && len < data->block[0]; i++,len++) {
-				dev_dbg(&adap->dev, "read i=%d len=%d\n", i, len);
+			for (i = 0; i < 8 && len < data->block[0]; i++, len++) {
+				dev_dbg(&adap->dev, "read i=%d len=%d\n", i,
+					len);
 				data->block[len+1] = sis630_read(SMB_BYTE+i);
 			}
 
-			dev_dbg(&adap->dev, "clear smbary_sts len=%d i=%d\n",len,i);
+			dev_dbg(&adap->dev, "clear smbary_sts len=%d i=%d\n",
+				len, i);
 
 			/* clear SMBARY_STS */
-			sis630_write(SMB_STS,0x10);
-		} while(len < data->block[0]);
+			sis630_write(SMB_STS, BYTE_DONE_STS);
+		} while (len < data->block[0]);
 	}
 
 	sis630_transaction_end(adap, oldclock);
@@ -346,42 +386,48 @@ static s32 sis630_access(struct i2c_adapter *adap, u16 addr,
 	int status;
 
 	switch (size) {
-		case I2C_SMBUS_QUICK:
-			sis630_write(SMB_ADDR, ((addr & 0x7f) << 1) | (read_write & 0x01));
-			size = SIS630_QUICK;
-			break;
-		case I2C_SMBUS_BYTE:
-			sis630_write(SMB_ADDR, ((addr & 0x7f) << 1) | (read_write & 0x01));
-			if (read_write == I2C_SMBUS_WRITE)
-				sis630_write(SMB_CMD, command);
-			size = SIS630_BYTE;
-			break;
-		case I2C_SMBUS_BYTE_DATA:
-			sis630_write(SMB_ADDR, ((addr & 0x7f) << 1) | (read_write & 0x01));
-			sis630_write(SMB_CMD, command);
-			if (read_write == I2C_SMBUS_WRITE)
-				sis630_write(SMB_BYTE, data->byte);
-			size = SIS630_BYTE_DATA;
-			break;
-		case I2C_SMBUS_PROC_CALL:
-		case I2C_SMBUS_WORD_DATA:
-			sis630_write(SMB_ADDR,((addr & 0x7f) << 1) | (read_write & 0x01));
-			sis630_write(SMB_CMD, command);
-			if (read_write == I2C_SMBUS_WRITE) {
-				sis630_write(SMB_BYTE, data->word & 0xff);
-				sis630_write(SMB_BYTE + 1,(data->word & 0xff00) >> 8);
-			}
-			size = (size == I2C_SMBUS_PROC_CALL ? SIS630_PCALL : SIS630_WORD_DATA);
-			break;
-		case I2C_SMBUS_BLOCK_DATA:
-			sis630_write(SMB_ADDR,((addr & 0x7f) << 1) | (read_write & 0x01));
+	case I2C_SMBUS_QUICK:
+		sis630_write(SMB_ADDR, ((addr << 1) & SMB_ADDRESS) |
+							(read_write & SMB_RW));
+		size = SIS630_QUICK;
+		break;
+	case I2C_SMBUS_BYTE:
+		sis630_write(SMB_ADDR, ((addr << 1) & SMB_ADDRESS) |
+							(read_write & SMB_RW));
+		if (read_write == I2C_SMBUS_WRITE)
 			sis630_write(SMB_CMD, command);
-			size = SIS630_BLOCK_DATA;
-			return sis630_block_data(adap, data, read_write);
-		default:
-			dev_warn(&adap->dev, "Unsupported transaction %d\n",
-				 size);
-			return -EOPNOTSUPP;
+		size = SIS630_BYTE;
+		break;
+	case I2C_SMBUS_BYTE_DATA:
+		sis630_write(SMB_ADDR, ((addr << 1) & SMB_ADDRESS) |
+							(read_write & SMB_RW));
+		sis630_write(SMB_CMD, command);
+		if (read_write == I2C_SMBUS_WRITE)
+			sis630_write(SMB_BYTE, data->byte);
+		size = SIS630_BYTE_DATA;
+		break;
+	case I2C_SMBUS_PROC_CALL:
+	case I2C_SMBUS_WORD_DATA:
+		sis630_write(SMB_ADDR, ((addr << 1) & SMB_ADDRESS) |
+							(read_write & SMB_RW));
+		sis630_write(SMB_CMD, command);
+		if (read_write == I2C_SMBUS_WRITE) {
+			sis630_write(SMB_BYTE, data->word & SMB_BYTE0);
+			sis630_write(SMB_BYTE + 1,
+						(data->word & SMB_BYTE1) >> 8);
+		}
+		size = (size == I2C_SMBUS_PROC_CALL ?
+				SIS630_PCALL : SIS630_WORD_DATA);
+		break;
+	case I2C_SMBUS_BLOCK_DATA:
+		sis630_write(SMB_ADDR, ((addr << 1) & SMB_ADDRESS) |
+							(read_write & SMB_RW));
+		sis630_write(SMB_CMD, command);
+		size = SIS630_BLOCK_DATA;
+		return sis630_block_data(adap, data, read_write);
+	default:
+		dev_warn(&adap->dev, "Unsupported transaction %d\n", size);
+		return -EOPNOTSUPP;
 	}
 
 	status = sis630_transaction(adap, size);
@@ -393,15 +439,16 @@ static s32 sis630_access(struct i2c_adapter *adap, u16 addr,
 		return 0;
 	}
 
-	switch(size) {
-		case SIS630_BYTE:
-		case SIS630_BYTE_DATA:
-			data->byte = sis630_read(SMB_BYTE);
-			break;
-		case SIS630_PCALL:
-		case SIS630_WORD_DATA:
-			data->word = sis630_read(SMB_BYTE) + (sis630_read(SMB_BYTE + 1) << 8);
-			break;
+	switch (size) {
+	case SIS630_BYTE:
+	case SIS630_BYTE_DATA:
+		data->byte = sis630_read(SMB_BYTE);
+		break;
+	case SIS630_PCALL:
+	case SIS630_WORD_DATA:
+		data->word = sis630_read(SMB_BYTE) +
+				(sis630_read(SMB_BYTE + 1) << 8);
+		break;
 	}
 
 	return 0;
@@ -409,9 +456,9 @@ static s32 sis630_access(struct i2c_adapter *adap, u16 addr,
 
 static u32 sis630_func(struct i2c_adapter *adapter)
 {
-	return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_BYTE_DATA |
-		I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_PROC_CALL |
-		I2C_FUNC_SMBUS_BLOCK_DATA;
+	return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
+		I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
+		I2C_FUNC_SMBUS_PROC_CALL | I2C_FUNC_SMBUS_BLOCK_DATA;
 }
 
 static int __devinit sis630_setup(struct pci_dev *sis630_dev)
@@ -423,19 +470,19 @@ static int __devinit sis630_setup(struct pci_dev *sis630_dev)
 	static unsigned short acpi_base;
 
 	/* check for supported SiS devices */
-	for (i=0; supported[i] > 0 ; i++) {
-		if ((dummy = pci_get_device(PCI_VENDOR_ID_SI, supported[i], dummy)))
+	for (i = 0; supported[i] > 0; i++) {
+		dummy = pci_get_device(PCI_VENDOR_ID_SI, supported[i], dummy);
+		if (dummy)
 			break; /* found */
 	}
 
 	if (dummy) {
 		pci_dev_put(dummy);
-	}
-        else if (force) {
-		dev_err(&sis630_dev->dev, "WARNING: Can't detect SIS630 compatible device, but "
+	} else if (force) {
+		dev_err(&sis630_dev->dev,
+			"WARNING: Can't detect SIS630 compatible device, but "
 			"loading because of force option enabled\n");
- 	}
-	else {
+	} else {
 		return -ENODEV;
 	}
 
@@ -443,24 +490,23 @@ static int __devinit sis630_setup(struct pci_dev *sis630_dev)
 	   Enable ACPI first , so we can accsess reg 74-75
 	   in acpi io space and read acpi base addr
 	*/
-	if (pci_read_config_byte(sis630_dev, SIS630_BIOS_CTL_REG,&b)) {
+	if (pci_read_config_byte(sis630_dev, SIS630_BIOS_CTL_REG, &b)) {
 		dev_err(&sis630_dev->dev, "Error: Can't read bios ctl reg\n");
-		retval = -ENODEV;
-		goto exit;
+		return -ENODEV;
 	}
 	/* if ACPI already enabled , do nothing */
 	if (!(b & 0x80) &&
 	    pci_write_config_byte(sis630_dev, SIS630_BIOS_CTL_REG, b | 0x80)) {
 		dev_err(&sis630_dev->dev, "Error: Can't enable ACPI\n");
-		retval = -ENODEV;
-		goto exit;
+		return -ENODEV;
 	}
 
 	/* Determine the ACPI base address */
-	if (pci_read_config_word(sis630_dev,SIS630_ACPI_BASE_REG,&acpi_base)) {
-		dev_err(&sis630_dev->dev, "Error: Can't determine ACPI base address\n");
-		retval = -ENODEV;
-		goto exit;
+	if (pci_read_config_word(sis630_dev, SIS630_ACPI_BASE_REG,
+								&acpi_base)) {
+		dev_err(&sis630_dev->dev,
+				"Error: Can't determine ACPI base address\n");
+		return -ENODEV;
 	}
 
 	dev_dbg(&sis630_dev->dev, "ACPI base at 0x%04x\n", acpi_base);
@@ -474,8 +520,10 @@ static int __devinit sis630_setup(struct pci_dev *sis630_dev)
 
 	retval = acpi_check_region(smbus_base + SMB_STS, SIS630_SMB_IOREGION,
 				   sis630_driver.name);
-	if (retval)
-		goto exit;
+	if (retval) {
+		smbus_base = 0;
+		return retval;
+	}
 
 	/* Everything is happy, let's grab the memory and set things up. */
 	if (!request_region(smbus_base + SMB_STS, SIS630_SMB_IOREGION,
@@ -483,16 +531,10 @@ static int __devinit sis630_setup(struct pci_dev *sis630_dev)
 		dev_err(&sis630_dev->dev,
 			"SMBus registers 0x%04x-0x%04x already in use!\n",
 			smbus_base + SMB_STS, smbus_base + SMB_SAA);
-		retval = -EBUSY;
-		goto exit;
+		return -EBUSY;
 	}
 
-	retval = 0;
-
-exit:
-	if (retval)
-		smbus_base = 0;
-	return retval;
+	return 0;
 }
 
 
@@ -511,15 +553,18 @@ static DEFINE_PCI_DEVICE_TABLE(sis630_ids) = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_964) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC) },
-	{ 0, }
+	{ 0 }
 };
 
-MODULE_DEVICE_TABLE (pci, sis630_ids);
+MODULE_DEVICE_TABLE(pci, sis630_ids);
 
-static int __devinit sis630_probe(struct pci_dev *dev, const struct pci_device_id *id)
+static int __devinit sis630_probe(struct pci_dev *dev,
+					const struct pci_device_id *id)
 {
 	if (sis630_setup(dev)) {
-		dev_err(&dev->dev, "SIS630 comp. bus not detected, module not inserted.\n");
+		dev_err(&dev->dev,
+			"SIS630 compatible bus not detected, "
+			"module not inserted.\n");
 		return -ENODEV;
 	}
 
-- 
1.7.8.6