2019-05-18 02:32:39

by Geordan Neukum

Subject: [PATCH 0/5] Updates to staging driver: kpc_i2c

Attached are an assortment of updates to the kpc_i2c driver in the
staging subtree.

As a quick summary:

Patches 1, 4, and 5 address style concerns raised by the checkpatch
script. Patch 1 (a reindentation fix) likely added additional 'line
length' warnings, but given the fact that they were only hidden due to a
nonstandard indentation style, I elected to defer that work to a future
patchset. If the reviewers should disagree with that choice, I'd be happy
to fix up those issues in a v2 upload.

Patch 2 is a reformatting / reorganization of the copyright header at the
top of the file. I attempted to look at some other drivers in the i2c
subsystem for inspiration, but I'd be happy to drop this isn't as simple
an update as it seems.

Patch 3 addresses a potential bug in the cleanup of memory allocated in
the probe method.

Geordan Neukum (5):
staging: kpc2000: kpc_i2c: reindent i2c_driver.c
staging: kpc2000: kpc_i2c: reformat copyright for better readability
staging: kpc2000: kpc_i2c: prevent memory leak in probe() error case
staging: kpc2000: kpc_i2c: use %s with __func__ identifier in log
messages
staging: kpc2000: kpc_i2c: fixup block comment style in i2c_driver.c

drivers/staging/kpc2000/kpc_i2c/i2c_driver.c | 1043 +++++++++---------
1 file changed, 527 insertions(+), 516 deletions(-)

--
2.21.0

2019-05-18 02:33:08

by Geordan Neukum

Subject: [PATCH 5/5] staging: kpc2000: kpc_i2c: fixup block comment style in i2c_driver.c

Throughout i2c_driver.c, there are numerous deviations from the two
standards of:
- placing a '*' at the beginning of every line containing a
block comment.
- placing the closing comment marker '*/' on a new line.

Instead, use a block comment style that is more consistent with the
prescribed guidelines.

Signed-off-by: Geordan Neukum <[email protected]>
---
drivers/staging/kpc2000/kpc_i2c/i2c_driver.c | 36 ++++++++++++--------
1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
index 03e401322a18..986148c72185 100644
--- a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
+++ b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
@@ -137,7 +137,8 @@ MODULE_PARM_DESC(disable_features, "Disable selected driver features");
#define outb_p(d,a) writeq(d,(void*)a)

/* Make sure the SMBus host is ready to start transmitting.
- Return 0 if it is, -EBUSY if it is not. */
+ * Return 0 if it is, -EBUSY if it is not.
+ */
static int i801_check_pre(struct i2c_device *priv)
{
int status;
@@ -226,7 +227,8 @@ static int i801_transaction(struct i2c_device *priv, int xact)
return result;
}
/* the current contents of SMBHSTCNT can be overwritten, since PEC,
- * INTREN, SMBSCMD are passed in xact */
+ * INTREN, SMBSCMD are passed in xact
+ */
outb_p(xact | I801_START, SMBHSTCNT(priv));

/* We will always wait for a fraction of a second! */
@@ -424,8 +426,9 @@ static int i801_block_transaction(struct i2c_device *priv, union i2c_smbus_data
}

/* Experience has shown that the block buffer can only be used for
- SMBus (not I2C) block transactions, even though the datasheet
- doesn't mention this limitation. */
+ * SMBus (not I2C) block transactions, even though the datasheet
+ * doesn't mention this limitation.
+ */
if ((priv->features & FEATURE_BLOCK_BUFFER) && command != I2C_SMBUS_I2C_BLOCK_DATA && i801_set_block_buffer_mode(priv) == 0) {
result = i801_block_transaction_by_block(priv, data, read_write, hwpec);
} else {
@@ -499,11 +502,13 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
case I2C_SMBUS_I2C_BLOCK_DATA:
dev_dbg(&priv->adapter.dev, " [acc] SMBUS_I2C_BLOCK_DATA\n");
/* NB: page 240 of ICH5 datasheet shows that the R/#W
- * bit should be cleared here, even when reading */
+ * bit should be cleared here, even when reading
+ */
outb_p((addr & 0x7f) << 1, SMBHSTADD(priv));
if (read_write == I2C_SMBUS_READ) {
/* NB: page 240 of ICH5 datasheet also shows
- * that DATA1 is the cmd field when reading */
+ * that DATA1 is the cmd field when reading
+ */
outb_p(command, SMBHSTDAT1(priv));
} else {
outb_p(command, SMBHSTCMD(priv));
@@ -533,8 +538,9 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
}

/* Some BIOSes don't like it when PEC is enabled at reboot or resume
- time, so we forcibly disable it after every transaction. Turn off
- E32B for the same reason. */
+ * time, so we forcibly disable it after every transaction. Turn off
+ * E32B for the same reason.
+ */
if (hwpec || block) {
dev_dbg(&priv->adapter.dev, " [acc] hwpec || block\n");
outb_p(inb_p(SMBAUXCTL(priv)) & ~(SMBAUXCTL_CRC | SMBAUXCTL_E32B), SMBAUXCTL(priv));
@@ -573,13 +579,13 @@ static u32 i801_func(struct i2c_adapter *adapter)
struct i2c_device *priv = i2c_get_adapdata(adapter);

/* original settings
- u32 f = I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
- I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
- I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_WRITE_I2C_BLOCK |
- ((priv->features & FEATURE_SMBUS_PEC) ? I2C_FUNC_SMBUS_PEC : 0) |
- ((priv->features & FEATURE_I2C_BLOCK_READ) ?
- I2C_FUNC_SMBUS_READ_I2C_BLOCK : 0);
- */
+ * u32 f = I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
+ * I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
+ * I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_WRITE_I2C_BLOCK |
+ * ((priv->features & FEATURE_SMBUS_PEC) ? I2C_FUNC_SMBUS_PEC : 0) |
+ * ((priv->features & FEATURE_I2C_BLOCK_READ) ?
+ * I2C_FUNC_SMBUS_READ_I2C_BLOCK : 0);
+ */

// http://lxr.free-electrons.com/source/include/uapi/linux/i2c.h#L85

--
2.21.0

2019-05-18 02:33:39

by Geordan Neukum

Subject: [PATCH 4/5] staging: kpc2000: kpc_i2c: use %s with __func__ identifier in log messages

Throughout i2c_driver.c, there are instances where the log strings
contain the function's name hardcoded into the string. Instead, use the
printk conversion specifier '%s' with the __func__ preprocessor
identifier to more maintainably print the function's name.

Signed-off-by: Geordan Neukum <[email protected]>
---
drivers/staging/kpc2000/kpc_i2c/i2c_driver.c | 27 +++++++++++---------
1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
index 9b9de81c8548..03e401322a18 100644
--- a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
+++ b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
@@ -142,7 +142,7 @@ static int i801_check_pre(struct i2c_device *priv)
{
int status;

- dev_dbg(&priv->adapter.dev, "i801_check_pre\n");
+ dev_dbg(&priv->adapter.dev, "%s\n", __func__);

status = inb_p(SMBHSTSTS(priv));
if (status & SMBHSTSTS_HOST_BUSY) {
@@ -168,7 +168,7 @@ static int i801_check_post(struct i2c_device *priv, int status, int timeout)
{
int result = 0;

- dev_dbg(&priv->adapter.dev, "i801_check_post\n");
+ dev_dbg(&priv->adapter.dev, "%s\n", __func__);

/* If the SMBus is still busy, we give up */
if (timeout) {
@@ -219,7 +219,7 @@ static int i801_transaction(struct i2c_device *priv, int xact)
int result;
int timeout = 0;

- dev_dbg(&priv->adapter.dev, "i801_transaction\n");
+ dev_dbg(&priv->adapter.dev, "%s\n", __func__);

result = i801_check_pre(priv);
if (result < 0) {
@@ -250,7 +250,7 @@ static void i801_wait_hwpec(struct i2c_device *priv)
int timeout = 0;
int status;

- dev_dbg(&priv->adapter.dev, "i801_wait_hwpec\n");
+ dev_dbg(&priv->adapter.dev, "%s\n", __func__);

do {
usleep_range(250, 500);
@@ -269,7 +269,7 @@ static int i801_block_transaction_by_block(struct i2c_device *priv, union i2c_sm
int i, len;
int status;

- dev_dbg(&priv->adapter.dev, "i801_block_transaction_by_block\n");
+ dev_dbg(&priv->adapter.dev, "%s\n", __func__);

inb_p(SMBHSTCNT(priv)); /* reset the data buffer index */

@@ -309,7 +309,7 @@ static int i801_block_transaction_byte_by_byte(struct i2c_device *priv, union i2
int result;
int timeout;

- dev_dbg(&priv->adapter.dev, "i801_block_transaction_byte_by_byte\n");
+ dev_dbg(&priv->adapter.dev, "%s\n", __func__);

result = i801_check_pre(priv);
if (result < 0) {
@@ -383,7 +383,7 @@ static int i801_block_transaction_byte_by_byte(struct i2c_device *priv, union i2

static int i801_set_block_buffer_mode(struct i2c_device *priv)
{
- dev_dbg(&priv->adapter.dev, "i801_set_block_buffer_mode\n");
+ dev_dbg(&priv->adapter.dev, "%s\n", __func__);

outb_p(inb_p(SMBAUXCTL(priv)) | SMBAUXCTL_E32B, SMBAUXCTL(priv));
if ((inb_p(SMBAUXCTL(priv)) & SMBAUXCTL_E32B) == 0) {
@@ -398,7 +398,7 @@ static int i801_block_transaction(struct i2c_device *priv, union i2c_smbus_data
int result = 0;
//unsigned char hostc;

- dev_dbg(&priv->adapter.dev, "i801_block_transaction\n");
+ dev_dbg(&priv->adapter.dev, "%s\n", __func__);

if (command == I2C_SMBUS_I2C_BLOCK_DATA) {
if (read_write == I2C_SMBUS_WRITE) {
@@ -450,8 +450,9 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
int ret, xact = 0;
struct i2c_device *priv = i2c_get_adapdata(adap);

- dev_dbg(&priv->adapter.dev, "i801_access (addr=%0d) flags=%x read_write=%x command=%x size=%x",
- addr, flags, read_write, command, size );
+ dev_dbg(&priv->adapter.dev,
+ "%s (addr=%0d) flags=%x read_write=%x command=%x size=%x",
+ __func__, addr, flags, read_write, command, size);

hwpec = (priv->features & FEATURE_SMBUS_PEC) && (flags & I2C_CLIENT_PEC) && size != I2C_SMBUS_QUICK && size != I2C_SMBUS_I2C_BLOCK_DATA;

@@ -626,7 +627,8 @@ int pi2c_probe(struct platform_device *pldev)
struct i2c_device *priv;
struct resource *res;

- dev_dbg(&pldev->dev, "pi2c_probe(pldev = %p '%s')\n", pldev, pldev->name);
+ dev_dbg(&pldev->dev, "%s(pldev = %p '%s')\n", __func__, pldev,
+ pldev->name);

priv = devm_kzalloc(&pldev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv) {
@@ -673,7 +675,8 @@ int pi2c_probe(struct platform_device *pldev)
int pi2c_remove(struct platform_device *pldev)
{
struct i2c_device *lddev;
- dev_dbg(&pldev->dev, "pi2c_remove(pldev = %p '%s')\n", pldev, pldev->name);
+ dev_dbg(&pldev->dev, "%s(pldev = %p '%s')\n", __func__, pldev,
+ pldev->name);

lddev = (struct i2c_device *)pldev->dev.platform_data;

--
2.21.0

2019-05-18 02:33:48

by Geordan Neukum

Subject: [PATCH 2/5] staging: kpc2000: kpc_i2c: reformat copyright for better readability

The copyright header in i2c_driver.c is difficult to read and not
chronologically ordered. Reformat and reorganize the copyright header
to be similar to other drivers in the i2c subsystem.

Signed-off-by: Geordan Neukum <[email protected]>
---
drivers/staging/kpc2000/kpc_i2c/i2c_driver.c | 30 ++++++++++++--------
1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
index 6dda4eb6de75..6cb63d20b00f 100644
--- a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
+++ b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
@@ -1,15 +1,21 @@
// SPDX-License-Identifier: GPL-2.0+
-/* Copyright (c) 2014-2018 Daktronics,
- Matt Sickler <[email protected]>,
- Jordon Hofer <[email protected]>
- Adapted i2c-i801.c for use with Kadoka hardware.
- Copyright (c) 1998 - 2002 Frodo Looijaard <[email protected]>,
- Philip Edelbrock <[email protected]>, and Mark D. Studebaker
- <[email protected]>
- Copyright (C) 2007 - 2012 Jean Delvare <[email protected]>
- Copyright (C) 2010 Intel Corporation,
- David Woodhouse <[email protected]>
-*/
+/*
+ * KPC2000 i2c driver
+ *
+ * Adapted i2c-i801.c for use with Kadoka hardware.
+ *
+ * Copyright (C) 1998 - 2002
+ * Frodo Looijaard <[email protected]>,
+ * Philip Edelbrock <[email protected]>,
+ * Mark D. Studebaker <[email protected]>
+ * Copyright (C) 2007 - 2012
+ * Jean Delvare <[email protected]>
+ * Copyright (C) 2010 Intel Corporation
+ * David Woodhouse <[email protected]>
+ * Copyright (C) 2014-2018 Daktronics
+ * Matt Sickler <[email protected]>,
+ * Jordon Hofer <[email protected]>
+ */
#include <linux/init.h>
#include <linux/module.h>
#include <linux/types.h>
@@ -445,7 +451,7 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
struct i2c_device *priv = i2c_get_adapdata(adap);

dev_dbg(&priv->adapter.dev, "i801_access (addr=%0d) flags=%x read_write=%x command=%x size=%x",
- addr, flags, read_write, command, size );
+ addr, flags, read_write, command, size );

hwpec = (priv->features & FEATURE_SMBUS_PEC) && (flags & I2C_CLIENT_PEC) && size != I2C_SMBUS_QUICK && size != I2C_SMBUS_I2C_BLOCK_DATA;

--
2.21.0

2019-05-18 02:34:57

by Geordan Neukum

Subject: [PATCH 3/5] staging: kpc2000: kpc_i2c: prevent memory leak in probe() error case

The probe() function performs a kzalloc to dynamically allocate memory
at runtime. If the allocation succeeds, yet invoking the function
i2c_add_adapter fails, the dynamically allocated memory is never freed.
Change the allocation to use the managed allocation API instead and
remove the manual freeing of the memory in the remove() function.

Signed-off-by: Geordan Neukum <[email protected]>
---
drivers/staging/kpc2000/kpc_i2c/i2c_driver.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
index 6cb63d20b00f..9b9de81c8548 100644
--- a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
+++ b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
@@ -628,7 +628,7 @@ int pi2c_probe(struct platform_device *pldev)

dev_dbg(&pldev->dev, "pi2c_probe(pldev = %p '%s')\n", pldev, pldev->name);

- priv = kzalloc(sizeof(struct i2c_device), GFP_KERNEL);
+ priv = devm_kzalloc(&pldev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv) {
return -ENOMEM;
}
@@ -685,10 +685,6 @@ int pi2c_remove(struct platform_device *pldev)
//pci_set_drvdata(dev, NULL);

//cdev_del(&lddev->cdev);
- if(lddev != 0) {
- kfree(lddev);
- pldev->dev.platform_data = 0;
- }

return 0;
}
--
2.21.0

2019-05-18 03:49:49

by Geordan Neukum

Subject: [PATCH 1/5] staging: kpc2000: kpc_i2c: reindent i2c_driver.c

i2c_driver.c uses a mixture of space and tab indentations which
conflicts with the kernel coding style guide. Reindent i2c_driver.c.

Signed-off-by: Geordan Neukum <[email protected]>
---
drivers/staging/kpc2000/kpc_i2c/i2c_driver.c | 1014 +++++++++---------
1 file changed, 507 insertions(+), 507 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
index 0fb068b2408d..6dda4eb6de75 100644
--- a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
+++ b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
@@ -1,14 +1,14 @@
// SPDX-License-Identifier: GPL-2.0+
/* Copyright (c) 2014-2018 Daktronics,
- Matt Sickler <[email protected]>,
- Jordon Hofer <[email protected]>
+ Matt Sickler <[email protected]>,
+ Jordon Hofer <[email protected]>
Adapted i2c-i801.c for use with Kadoka hardware.
Copyright (c) 1998 - 2002 Frodo Looijaard <[email protected]>,
Philip Edelbrock <[email protected]>, and Mark D. Studebaker
<[email protected]>
Copyright (C) 2007 - 2012 Jean Delvare <[email protected]>
Copyright (C) 2010 Intel Corporation,
- David Woodhouse <[email protected]>
+ David Woodhouse <[email protected]>
*/
#include <linux/init.h>
#include <linux/module.h>
@@ -29,11 +29,11 @@ MODULE_AUTHOR("[email protected]");
MODULE_SOFTDEP("pre: i2c-dev");

struct i2c_device {
- unsigned long smba;
- struct i2c_adapter adapter;
- struct platform_device *pldev;
- struct rw_semaphore rw_sem;
- unsigned int features;
+ unsigned long smba;
+ struct i2c_adapter adapter;
+ struct platform_device *pldev;
+ struct rw_semaphore rw_sem;
+ unsigned int features;
};

/*****************************
@@ -134,479 +134,479 @@ MODULE_PARM_DESC(disable_features, "Disable selected driver features");
Return 0 if it is, -EBUSY if it is not. */
static int i801_check_pre(struct i2c_device *priv)
{
- int status;
-
- dev_dbg(&priv->adapter.dev, "i801_check_pre\n");
-
- status = inb_p(SMBHSTSTS(priv));
- if (status & SMBHSTSTS_HOST_BUSY) {
- dev_err(&priv->adapter.dev, "SMBus is busy, can't use it! (status=%x)\n", status);
- return -EBUSY;
- }
-
- status &= STATUS_FLAGS;
- if (status) {
- //dev_dbg(&priv->adapter.dev, "Clearing status flags (%02x)\n", status);
- outb_p(status, SMBHSTSTS(priv));
- status = inb_p(SMBHSTSTS(priv)) & STATUS_FLAGS;
- if (status) {
- dev_err(&priv->adapter.dev, "Failed clearing status flags (%02x)\n", status);
- return -EBUSY;
- }
- }
- return 0;
+ int status;
+
+ dev_dbg(&priv->adapter.dev, "i801_check_pre\n");
+
+ status = inb_p(SMBHSTSTS(priv));
+ if (status & SMBHSTSTS_HOST_BUSY) {
+ dev_err(&priv->adapter.dev, "SMBus is busy, can't use it! (status=%x)\n", status);
+ return -EBUSY;
+ }
+
+ status &= STATUS_FLAGS;
+ if (status) {
+ //dev_dbg(&priv->adapter.dev, "Clearing status flags (%02x)\n", status);
+ outb_p(status, SMBHSTSTS(priv));
+ status = inb_p(SMBHSTSTS(priv)) & STATUS_FLAGS;
+ if (status) {
+ dev_err(&priv->adapter.dev, "Failed clearing status flags (%02x)\n", status);
+ return -EBUSY;
+ }
+ }
+ return 0;
}

/* Convert the status register to an error code, and clear it. */
static int i801_check_post(struct i2c_device *priv, int status, int timeout)
{
- int result = 0;
-
- dev_dbg(&priv->adapter.dev, "i801_check_post\n");
-
- /* If the SMBus is still busy, we give up */
- if (timeout) {
- dev_err(&priv->adapter.dev, "Transaction timeout\n");
- /* try to stop the current command */
- dev_dbg(&priv->adapter.dev, "Terminating the current operation\n");
- outb_p(inb_p(SMBHSTCNT(priv)) | SMBHSTCNT_KILL, SMBHSTCNT(priv));
- usleep_range(1000, 2000);
- outb_p(inb_p(SMBHSTCNT(priv)) & (~SMBHSTCNT_KILL), SMBHSTCNT(priv));
-
- /* Check if it worked */
- status = inb_p(SMBHSTSTS(priv));
- if ((status & SMBHSTSTS_HOST_BUSY) || !(status & SMBHSTSTS_FAILED)) {
- dev_err(&priv->adapter.dev, "Failed terminating the transaction\n");
- }
- outb_p(STATUS_FLAGS, SMBHSTSTS(priv));
- return -ETIMEDOUT;
- }
-
- if (status & SMBHSTSTS_FAILED) {
- result = -EIO;
- dev_err(&priv->adapter.dev, "Transaction failed\n");
- }
- if (status & SMBHSTSTS_DEV_ERR) {
- result = -ENXIO;
- dev_dbg(&priv->adapter.dev, "No response\n");
- }
- if (status & SMBHSTSTS_BUS_ERR) {
- result = -EAGAIN;
- dev_dbg(&priv->adapter.dev, "Lost arbitration\n");
- }
-
- if (result) {
- /* Clear error flags */
- outb_p(status & STATUS_FLAGS, SMBHSTSTS(priv));
- status = inb_p(SMBHSTSTS(priv)) & STATUS_FLAGS;
- if (status) {
- dev_warn(&priv->adapter.dev, "Failed clearing status flags at end of transaction (%02x)\n", status);
- }
- }
-
- return result;
+ int result = 0;
+
+ dev_dbg(&priv->adapter.dev, "i801_check_post\n");
+
+ /* If the SMBus is still busy, we give up */
+ if (timeout) {
+ dev_err(&priv->adapter.dev, "Transaction timeout\n");
+ /* try to stop the current command */
+ dev_dbg(&priv->adapter.dev, "Terminating the current operation\n");
+ outb_p(inb_p(SMBHSTCNT(priv)) | SMBHSTCNT_KILL, SMBHSTCNT(priv));
+ usleep_range(1000, 2000);
+ outb_p(inb_p(SMBHSTCNT(priv)) & (~SMBHSTCNT_KILL), SMBHSTCNT(priv));
+
+ /* Check if it worked */
+ status = inb_p(SMBHSTSTS(priv));
+ if ((status & SMBHSTSTS_HOST_BUSY) || !(status & SMBHSTSTS_FAILED)) {
+ dev_err(&priv->adapter.dev, "Failed terminating the transaction\n");
+ }
+ outb_p(STATUS_FLAGS, SMBHSTSTS(priv));
+ return -ETIMEDOUT;
+ }
+
+ if (status & SMBHSTSTS_FAILED) {
+ result = -EIO;
+ dev_err(&priv->adapter.dev, "Transaction failed\n");
+ }
+ if (status & SMBHSTSTS_DEV_ERR) {
+ result = -ENXIO;
+ dev_dbg(&priv->adapter.dev, "No response\n");
+ }
+ if (status & SMBHSTSTS_BUS_ERR) {
+ result = -EAGAIN;
+ dev_dbg(&priv->adapter.dev, "Lost arbitration\n");
+ }
+
+ if (result) {
+ /* Clear error flags */
+ outb_p(status & STATUS_FLAGS, SMBHSTSTS(priv));
+ status = inb_p(SMBHSTSTS(priv)) & STATUS_FLAGS;
+ if (status) {
+ dev_warn(&priv->adapter.dev, "Failed clearing status flags at end of transaction (%02x)\n", status);
+ }
+ }
+
+ return result;
}

static int i801_transaction(struct i2c_device *priv, int xact)
{
- int status;
- int result;
- int timeout = 0;
-
- dev_dbg(&priv->adapter.dev, "i801_transaction\n");
-
- result = i801_check_pre(priv);
- if (result < 0) {
- return result;
- }
- /* the current contents of SMBHSTCNT can be overwritten, since PEC,
- * INTREN, SMBSCMD are passed in xact */
- outb_p(xact | I801_START, SMBHSTCNT(priv));
-
- /* We will always wait for a fraction of a second! */
- do {
- usleep_range(250, 500);
- status = inb_p(SMBHSTSTS(priv));
- } while ((status & SMBHSTSTS_HOST_BUSY) && (timeout++ < MAX_RETRIES));
-
- result = i801_check_post(priv, status, timeout > MAX_RETRIES);
- if (result < 0) {
- return result;
- }
-
- outb_p(SMBHSTSTS_INTR, SMBHSTSTS(priv));
- return 0;
+ int status;
+ int result;
+ int timeout = 0;
+
+ dev_dbg(&priv->adapter.dev, "i801_transaction\n");
+
+ result = i801_check_pre(priv);
+ if (result < 0) {
+ return result;
+ }
+ /* the current contents of SMBHSTCNT can be overwritten, since PEC,
+ * INTREN, SMBSCMD are passed in xact */
+ outb_p(xact | I801_START, SMBHSTCNT(priv));
+
+ /* We will always wait for a fraction of a second! */
+ do {
+ usleep_range(250, 500);
+ status = inb_p(SMBHSTSTS(priv));
+ } while ((status & SMBHSTSTS_HOST_BUSY) && (timeout++ < MAX_RETRIES));
+
+ result = i801_check_post(priv, status, timeout > MAX_RETRIES);
+ if (result < 0) {
+ return result;
+ }
+
+ outb_p(SMBHSTSTS_INTR, SMBHSTSTS(priv));
+ return 0;
}

/* wait for INTR bit as advised by Intel */
static void i801_wait_hwpec(struct i2c_device *priv)
{
- int timeout = 0;
- int status;
-
- dev_dbg(&priv->adapter.dev, "i801_wait_hwpec\n");
-
- do {
- usleep_range(250, 500);
- status = inb_p(SMBHSTSTS(priv));
- } while ((!(status & SMBHSTSTS_INTR)) && (timeout++ < MAX_RETRIES));
-
- if (timeout > MAX_RETRIES) {
- dev_dbg(&priv->adapter.dev, "PEC Timeout!\n");
- }
-
- outb_p(status, SMBHSTSTS(priv));
+ int timeout = 0;
+ int status;
+
+ dev_dbg(&priv->adapter.dev, "i801_wait_hwpec\n");
+
+ do {
+ usleep_range(250, 500);
+ status = inb_p(SMBHSTSTS(priv));
+ } while ((!(status & SMBHSTSTS_INTR)) && (timeout++ < MAX_RETRIES));
+
+ if (timeout > MAX_RETRIES) {
+ dev_dbg(&priv->adapter.dev, "PEC Timeout!\n");
+ }
+
+ outb_p(status, SMBHSTSTS(priv));
}

static int i801_block_transaction_by_block(struct i2c_device *priv, union i2c_smbus_data *data, char read_write, int hwpec)
{
- int i, len;
- int status;
-
- dev_dbg(&priv->adapter.dev, "i801_block_transaction_by_block\n");
-
- inb_p(SMBHSTCNT(priv)); /* reset the data buffer index */
-
- /* Use 32-byte buffer to process this transaction */
- if (read_write == I2C_SMBUS_WRITE) {
- len = data->block[0];
- outb_p(len, SMBHSTDAT0(priv));
- for (i = 0; i < len; i++) {
- outb_p(data->block[i+1], SMBBLKDAT(priv));
- }
- }
-
- status = i801_transaction(priv, I801_BLOCK_DATA | ENABLE_INT9 | I801_PEC_EN * hwpec);
- if (status) {
- return status;
- }
-
- if (read_write == I2C_SMBUS_READ) {
- len = inb_p(SMBHSTDAT0(priv));
- if (len < 1 || len > I2C_SMBUS_BLOCK_MAX) {
- return -EPROTO;
- }
-
- data->block[0] = len;
- for (i = 0; i < len; i++) {
- data->block[i + 1] = inb_p(SMBBLKDAT(priv));
- }
- }
- return 0;
+ int i, len;
+ int status;
+
+ dev_dbg(&priv->adapter.dev, "i801_block_transaction_by_block\n");
+
+ inb_p(SMBHSTCNT(priv)); /* reset the data buffer index */
+
+ /* Use 32-byte buffer to process this transaction */
+ if (read_write == I2C_SMBUS_WRITE) {
+ len = data->block[0];
+ outb_p(len, SMBHSTDAT0(priv));
+ for (i = 0; i < len; i++) {
+ outb_p(data->block[i+1], SMBBLKDAT(priv));
+ }
+ }
+
+ status = i801_transaction(priv, I801_BLOCK_DATA | ENABLE_INT9 | I801_PEC_EN * hwpec);
+ if (status) {
+ return status;
+ }
+
+ if (read_write == I2C_SMBUS_READ) {
+ len = inb_p(SMBHSTDAT0(priv));
+ if (len < 1 || len > I2C_SMBUS_BLOCK_MAX) {
+ return -EPROTO;
+ }
+
+ data->block[0] = len;
+ for (i = 0; i < len; i++) {
+ data->block[i + 1] = inb_p(SMBBLKDAT(priv));
+ }
+ }
+ return 0;
}

static int i801_block_transaction_byte_by_byte(struct i2c_device *priv, union i2c_smbus_data *data, char read_write, int command, int hwpec)
{
- int i, len;
- int smbcmd;
- int status;
- int result;
- int timeout;
-
- dev_dbg(&priv->adapter.dev, "i801_block_transaction_byte_by_byte\n");
-
- result = i801_check_pre(priv);
- if (result < 0) {
- return result;
- }
-
- len = data->block[0];
-
- if (read_write == I2C_SMBUS_WRITE) {
- outb_p(len, SMBHSTDAT0(priv));
- outb_p(data->block[1], SMBBLKDAT(priv));
- }
-
- for (i = 1; i <= len; i++) {
- if (i == len && read_write == I2C_SMBUS_READ) {
- if (command == I2C_SMBUS_I2C_BLOCK_DATA) {
- smbcmd = I801_I2C_BLOCK_LAST;
- } else {
- smbcmd = I801_BLOCK_LAST;
- }
- } else {
- if (command == I2C_SMBUS_I2C_BLOCK_DATA && read_write == I2C_SMBUS_READ) {
- smbcmd = I801_I2C_BLOCK_DATA;
- } else {
- smbcmd = I801_BLOCK_DATA;
- }
- }
- outb_p(smbcmd | ENABLE_INT9, SMBHSTCNT(priv));
-
- if (i == 1) {
- outb_p(inb(SMBHSTCNT(priv)) | I801_START, SMBHSTCNT(priv));
- }
- /* We will always wait for a fraction of a second! */
- timeout = 0;
- do {
- usleep_range(250, 500);
- status = inb_p(SMBHSTSTS(priv));
- } while ((!(status & SMBHSTSTS_BYTE_DONE)) && (timeout++ < MAX_RETRIES));
-
- result = i801_check_post(priv, status, timeout > MAX_RETRIES);
- if (result < 0) {
- return result;
- }
- if (i == 1 && read_write == I2C_SMBUS_READ && command != I2C_SMBUS_I2C_BLOCK_DATA) {
- len = inb_p(SMBHSTDAT0(priv));
- if (len < 1 || len > I2C_SMBUS_BLOCK_MAX) {
- dev_err(&priv->adapter.dev, "Illegal SMBus block read size %d\n", len);
- /* Recover */
- while (inb_p(SMBHSTSTS(priv)) & SMBHSTSTS_HOST_BUSY) {
- outb_p(SMBHSTSTS_BYTE_DONE, SMBHSTSTS(priv));
- }
- outb_p(SMBHSTSTS_INTR, SMBHSTSTS(priv));
- return -EPROTO;
- }
- data->block[0] = len;
- }
-
- /* Retrieve/store value in SMBBLKDAT */
- if (read_write == I2C_SMBUS_READ) {
- data->block[i] = inb_p(SMBBLKDAT(priv));
- }
- if (read_write == I2C_SMBUS_WRITE && i+1 <= len) {
- outb_p(data->block[i+1], SMBBLKDAT(priv));
- }
- /* signals SMBBLKDAT ready */
- outb_p(SMBHSTSTS_BYTE_DONE | SMBHSTSTS_INTR, SMBHSTSTS(priv));
- }
-
- return 0;
+ int i, len;
+ int smbcmd;
+ int status;
+ int result;
+ int timeout;
+
+ dev_dbg(&priv->adapter.dev, "i801_block_transaction_byte_by_byte\n");
+
+ result = i801_check_pre(priv);
+ if (result < 0) {
+ return result;
+ }
+
+ len = data->block[0];
+
+ if (read_write == I2C_SMBUS_WRITE) {
+ outb_p(len, SMBHSTDAT0(priv));
+ outb_p(data->block[1], SMBBLKDAT(priv));
+ }
+
+ for (i = 1; i <= len; i++) {
+ if (i == len && read_write == I2C_SMBUS_READ) {
+ if (command == I2C_SMBUS_I2C_BLOCK_DATA) {
+ smbcmd = I801_I2C_BLOCK_LAST;
+ } else {
+ smbcmd = I801_BLOCK_LAST;
+ }
+ } else {
+ if (command == I2C_SMBUS_I2C_BLOCK_DATA && read_write == I2C_SMBUS_READ) {
+ smbcmd = I801_I2C_BLOCK_DATA;
+ } else {
+ smbcmd = I801_BLOCK_DATA;
+ }
+ }
+ outb_p(smbcmd | ENABLE_INT9, SMBHSTCNT(priv));
+
+ if (i == 1) {
+ outb_p(inb(SMBHSTCNT(priv)) | I801_START, SMBHSTCNT(priv));
+ }
+ /* We will always wait for a fraction of a second! */
+ timeout = 0;
+ do {
+ usleep_range(250, 500);
+ status = inb_p(SMBHSTSTS(priv));
+ } while ((!(status & SMBHSTSTS_BYTE_DONE)) && (timeout++ < MAX_RETRIES));
+
+ result = i801_check_post(priv, status, timeout > MAX_RETRIES);
+ if (result < 0) {
+ return result;
+ }
+ if (i == 1 && read_write == I2C_SMBUS_READ && command != I2C_SMBUS_I2C_BLOCK_DATA) {
+ len = inb_p(SMBHSTDAT0(priv));
+ if (len < 1 || len > I2C_SMBUS_BLOCK_MAX) {
+ dev_err(&priv->adapter.dev, "Illegal SMBus block read size %d\n", len);
+ /* Recover */
+ while (inb_p(SMBHSTSTS(priv)) & SMBHSTSTS_HOST_BUSY) {
+ outb_p(SMBHSTSTS_BYTE_DONE, SMBHSTSTS(priv));
+ }
+ outb_p(SMBHSTSTS_INTR, SMBHSTSTS(priv));
+ return -EPROTO;
+ }
+ data->block[0] = len;
+ }
+
+ /* Retrieve/store value in SMBBLKDAT */
+ if (read_write == I2C_SMBUS_READ) {
+ data->block[i] = inb_p(SMBBLKDAT(priv));
+ }
+ if (read_write == I2C_SMBUS_WRITE && i+1 <= len) {
+ outb_p(data->block[i+1], SMBBLKDAT(priv));
+ }
+ /* signals SMBBLKDAT ready */
+ outb_p(SMBHSTSTS_BYTE_DONE | SMBHSTSTS_INTR, SMBHSTSTS(priv));
+ }
+
+ return 0;
}

static int i801_set_block_buffer_mode(struct i2c_device *priv)
{
- dev_dbg(&priv->adapter.dev, "i801_set_block_buffer_mode\n");
-
- outb_p(inb_p(SMBAUXCTL(priv)) | SMBAUXCTL_E32B, SMBAUXCTL(priv));
- if ((inb_p(SMBAUXCTL(priv)) & SMBAUXCTL_E32B) == 0) {
- return -EIO;
- }
- return 0;
+ dev_dbg(&priv->adapter.dev, "i801_set_block_buffer_mode\n");
+
+ outb_p(inb_p(SMBAUXCTL(priv)) | SMBAUXCTL_E32B, SMBAUXCTL(priv));
+ if ((inb_p(SMBAUXCTL(priv)) & SMBAUXCTL_E32B) == 0) {
+ return -EIO;
+ }
+ return 0;
}

/* Block transaction function */
static int i801_block_transaction(struct i2c_device *priv, union i2c_smbus_data *data, char read_write, int command, int hwpec)
{
- int result = 0;
- //unsigned char hostc;
-
- dev_dbg(&priv->adapter.dev, "i801_block_transaction\n");
-
- if (command == I2C_SMBUS_I2C_BLOCK_DATA) {
- if (read_write == I2C_SMBUS_WRITE) {
- /* set I2C_EN bit in configuration register */
- //TODO: Figure out the right thing to do here...
- //pci_read_config_byte(priv->pci_dev, SMBHSTCFG, &hostc);
- //pci_write_config_byte(priv->pci_dev, SMBHSTCFG, hostc | SMBHSTCFG_I2C_EN);
- } else if (!(priv->features & FEATURE_I2C_BLOCK_READ)) {
- dev_err(&priv->adapter.dev, "I2C block read is unsupported!\n");
- return -EOPNOTSUPP;
- }
- }
-
- if (read_write == I2C_SMBUS_WRITE || command == I2C_SMBUS_I2C_BLOCK_DATA) {
- if (data->block[0] < 1) {
- data->block[0] = 1;
- }
- if (data->block[0] > I2C_SMBUS_BLOCK_MAX) {
- data->block[0] = I2C_SMBUS_BLOCK_MAX;
- }
- } else {
- data->block[0] = 32; /* max for SMBus block reads */
- }
-
- /* Experience has shown that the block buffer can only be used for
- SMBus (not I2C) block transactions, even though the datasheet
- doesn't mention this limitation. */
- if ((priv->features & FEATURE_BLOCK_BUFFER) && command != I2C_SMBUS_I2C_BLOCK_DATA && i801_set_block_buffer_mode(priv) == 0) {
- result = i801_block_transaction_by_block(priv, data, read_write, hwpec);
- } else {
- result = i801_block_transaction_byte_by_byte(priv, data, read_write, command, hwpec);
- }
- if (result == 0 && hwpec) {
- i801_wait_hwpec(priv);
- }
- if (command == I2C_SMBUS_I2C_BLOCK_DATA && read_write == I2C_SMBUS_WRITE) {
- /* restore saved configuration register value */
- //TODO: Figure out the right thing to do here...
- //pci_write_config_byte(priv->pci_dev, SMBHSTCFG, hostc);
- }
- return result;
+ int result = 0;
+ //unsigned char hostc;
+
+ dev_dbg(&priv->adapter.dev, "i801_block_transaction\n");
+
+ if (command == I2C_SMBUS_I2C_BLOCK_DATA) {
+ if (read_write == I2C_SMBUS_WRITE) {
+ /* set I2C_EN bit in configuration register */
+ //TODO: Figure out the right thing to do here...
+ //pci_read_config_byte(priv->pci_dev, SMBHSTCFG, &hostc);
+ //pci_write_config_byte(priv->pci_dev, SMBHSTCFG, hostc | SMBHSTCFG_I2C_EN);
+ } else if (!(priv->features & FEATURE_I2C_BLOCK_READ)) {
+ dev_err(&priv->adapter.dev, "I2C block read is unsupported!\n");
+ return -EOPNOTSUPP;
+ }
+ }
+
+ if (read_write == I2C_SMBUS_WRITE || command == I2C_SMBUS_I2C_BLOCK_DATA) {
+ if (data->block[0] < 1) {
+ data->block[0] = 1;
+ }
+ if (data->block[0] > I2C_SMBUS_BLOCK_MAX) {
+ data->block[0] = I2C_SMBUS_BLOCK_MAX;
+ }
+ } else {
+ data->block[0] = 32; /* max for SMBus block reads */
+ }
+
+ /* Experience has shown that the block buffer can only be used for
+ SMBus (not I2C) block transactions, even though the datasheet
+ doesn't mention this limitation. */
+ if ((priv->features & FEATURE_BLOCK_BUFFER) && command != I2C_SMBUS_I2C_BLOCK_DATA && i801_set_block_buffer_mode(priv) == 0) {
+ result = i801_block_transaction_by_block(priv, data, read_write, hwpec);
+ } else {
+ result = i801_block_transaction_byte_by_byte(priv, data, read_write, command, hwpec);
+ }
+ if (result == 0 && hwpec) {
+ i801_wait_hwpec(priv);
+ }
+ if (command == I2C_SMBUS_I2C_BLOCK_DATA && read_write == I2C_SMBUS_WRITE) {
+ /* restore saved configuration register value */
+ //TODO: Figure out the right thing to do here...
+ //pci_write_config_byte(priv->pci_dev, SMBHSTCFG, hostc);
+ }
+ return result;
}

/* Return negative errno on error. */
static s32 i801_access(struct i2c_adapter *adap, u16 addr, unsigned short flags, char read_write, u8 command, int size, union i2c_smbus_data *data)
{
- int hwpec;
- int block = 0;
- int ret, xact = 0;
- struct i2c_device *priv = i2c_get_adapdata(adap);
-
- dev_dbg(&priv->adapter.dev, "i801_access (addr=%0d) flags=%x read_write=%x command=%x size=%x",
- addr, flags, read_write, command, size );
-
- hwpec = (priv->features & FEATURE_SMBUS_PEC) && (flags & I2C_CLIENT_PEC) && size != I2C_SMBUS_QUICK && size != I2C_SMBUS_I2C_BLOCK_DATA;
-
- switch (size) {
- case I2C_SMBUS_QUICK:
- dev_dbg(&priv->adapter.dev, " [acc] SMBUS_QUICK\n");
- outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), SMBHSTADD(priv));
- xact = I801_QUICK;
- break;
- case I2C_SMBUS_BYTE:
- dev_dbg(&priv->adapter.dev, " [acc] SMBUS_BYTE\n");
-
- outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), SMBHSTADD(priv));
- if (read_write == I2C_SMBUS_WRITE) {
- outb_p(command, SMBHSTCMD(priv));
- }
- xact = I801_BYTE;
- break;
- case I2C_SMBUS_BYTE_DATA:
- dev_dbg(&priv->adapter.dev, " [acc] SMBUS_BYTE_DATA\n");
- outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), SMBHSTADD(priv));
- outb_p(command, SMBHSTCMD(priv));
- if (read_write == I2C_SMBUS_WRITE) {
- outb_p(data->byte, SMBHSTDAT0(priv));
- }
- xact = I801_BYTE_DATA;
- break;
- case I2C_SMBUS_WORD_DATA:
- dev_dbg(&priv->adapter.dev, " [acc] SMBUS_WORD_DATA\n");
- outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), SMBHSTADD(priv));
- outb_p(command, SMBHSTCMD(priv));
- if (read_write == I2C_SMBUS_WRITE) {
- outb_p(data->word & 0xff, SMBHSTDAT0(priv));
- outb_p((data->word & 0xff00) >> 8, SMBHSTDAT1(priv));
- }
- xact = I801_WORD_DATA;
- break;
- case I2C_SMBUS_BLOCK_DATA:
- dev_dbg(&priv->adapter.dev, " [acc] SMBUS_BLOCK_DATA\n");
- outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), SMBHSTADD(priv));
- outb_p(command, SMBHSTCMD(priv));
- block = 1;
- break;
- case I2C_SMBUS_I2C_BLOCK_DATA:
- dev_dbg(&priv->adapter.dev, " [acc] SMBUS_I2C_BLOCK_DATA\n");
- /* NB: page 240 of ICH5 datasheet shows that the R/#W
- * bit should be cleared here, even when reading */
- outb_p((addr & 0x7f) << 1, SMBHSTADD(priv));
- if (read_write == I2C_SMBUS_READ) {
- /* NB: page 240 of ICH5 datasheet also shows
- * that DATA1 is the cmd field when reading */
- outb_p(command, SMBHSTDAT1(priv));
- } else {
- outb_p(command, SMBHSTCMD(priv));
- }
- block = 1;
- break;
- default:
- dev_dbg(&priv->adapter.dev, " [acc] Unsupported transaction %d\n", size);
- return -EOPNOTSUPP;
- }
-
- if (hwpec) { /* enable/disable hardware PEC */
- dev_dbg(&priv->adapter.dev, " [acc] hwpec: yes\n");
- outb_p(inb_p(SMBAUXCTL(priv)) | SMBAUXCTL_CRC, SMBAUXCTL(priv));
- } else {
- dev_dbg(&priv->adapter.dev, " [acc] hwpec: no\n");
- outb_p(inb_p(SMBAUXCTL(priv)) & (~SMBAUXCTL_CRC), SMBAUXCTL(priv));
- }
-
- if (block) {
- //ret = 0;
- dev_dbg(&priv->adapter.dev, " [acc] block: yes\n");
- ret = i801_block_transaction(priv, data, read_write, size, hwpec);
- } else {
- dev_dbg(&priv->adapter.dev, " [acc] block: no\n");
- ret = i801_transaction(priv, xact | ENABLE_INT9);
- }
-
- /* Some BIOSes don't like it when PEC is enabled at reboot or resume
- time, so we forcibly disable it after every transaction. Turn off
- E32B for the same reason. */
- if (hwpec || block) {
- dev_dbg(&priv->adapter.dev, " [acc] hwpec || block\n");
- outb_p(inb_p(SMBAUXCTL(priv)) & ~(SMBAUXCTL_CRC | SMBAUXCTL_E32B), SMBAUXCTL(priv));
- }
- if (block) {
- dev_dbg(&priv->adapter.dev, " [acc] block\n");
- return ret;
- }
- if (ret) {
- dev_dbg(&priv->adapter.dev, " [acc] ret %d\n", ret);
- return ret;
- }
- if ((read_write == I2C_SMBUS_WRITE) || (xact == I801_QUICK)) {
- dev_dbg(&priv->adapter.dev, " [acc] I2C_SMBUS_WRITE || I801_QUICK -> ret 0\n");
- return 0;
- }
-
- switch (xact & 0x7f) {
- case I801_BYTE: /* Result put in SMBHSTDAT0 */
- case I801_BYTE_DATA:
- dev_dbg(&priv->adapter.dev, " [acc] I801_BYTE or I801_BYTE_DATA\n");
- data->byte = inb_p(SMBHSTDAT0(priv));
- break;
- case I801_WORD_DATA:
- dev_dbg(&priv->adapter.dev, " [acc] I801_WORD_DATA\n");
- data->word = inb_p(SMBHSTDAT0(priv)) + (inb_p(SMBHSTDAT1(priv)) << 8);
- break;
- }
- return 0;
+ int hwpec;
+ int block = 0;
+ int ret, xact = 0;
+ struct i2c_device *priv = i2c_get_adapdata(adap);
+
+ dev_dbg(&priv->adapter.dev, "i801_access (addr=%0d) flags=%x read_write=%x command=%x size=%x",
+ addr, flags, read_write, command, size );
+
+ hwpec = (priv->features & FEATURE_SMBUS_PEC) && (flags & I2C_CLIENT_PEC) && size != I2C_SMBUS_QUICK && size != I2C_SMBUS_I2C_BLOCK_DATA;
+
+ switch (size) {
+ case I2C_SMBUS_QUICK:
+ dev_dbg(&priv->adapter.dev, " [acc] SMBUS_QUICK\n");
+ outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), SMBHSTADD(priv));
+ xact = I801_QUICK;
+ break;
+ case I2C_SMBUS_BYTE:
+ dev_dbg(&priv->adapter.dev, " [acc] SMBUS_BYTE\n");
+
+ outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), SMBHSTADD(priv));
+ if (read_write == I2C_SMBUS_WRITE) {
+ outb_p(command, SMBHSTCMD(priv));
+ }
+ xact = I801_BYTE;
+ break;
+ case I2C_SMBUS_BYTE_DATA:
+ dev_dbg(&priv->adapter.dev, " [acc] SMBUS_BYTE_DATA\n");
+ outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), SMBHSTADD(priv));
+ outb_p(command, SMBHSTCMD(priv));
+ if (read_write == I2C_SMBUS_WRITE) {
+ outb_p(data->byte, SMBHSTDAT0(priv));
+ }
+ xact = I801_BYTE_DATA;
+ break;
+ case I2C_SMBUS_WORD_DATA:
+ dev_dbg(&priv->adapter.dev, " [acc] SMBUS_WORD_DATA\n");
+ outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), SMBHSTADD(priv));
+ outb_p(command, SMBHSTCMD(priv));
+ if (read_write == I2C_SMBUS_WRITE) {
+ outb_p(data->word & 0xff, SMBHSTDAT0(priv));
+ outb_p((data->word & 0xff00) >> 8, SMBHSTDAT1(priv));
+ }
+ xact = I801_WORD_DATA;
+ break;
+ case I2C_SMBUS_BLOCK_DATA:
+ dev_dbg(&priv->adapter.dev, " [acc] SMBUS_BLOCK_DATA\n");
+ outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), SMBHSTADD(priv));
+ outb_p(command, SMBHSTCMD(priv));
+ block = 1;
+ break;
+ case I2C_SMBUS_I2C_BLOCK_DATA:
+ dev_dbg(&priv->adapter.dev, " [acc] SMBUS_I2C_BLOCK_DATA\n");
+ /* NB: page 240 of ICH5 datasheet shows that the R/#W
+ * bit should be cleared here, even when reading */
+ outb_p((addr & 0x7f) << 1, SMBHSTADD(priv));
+ if (read_write == I2C_SMBUS_READ) {
+ /* NB: page 240 of ICH5 datasheet also shows
+ * that DATA1 is the cmd field when reading */
+ outb_p(command, SMBHSTDAT1(priv));
+ } else {
+ outb_p(command, SMBHSTCMD(priv));
+ }
+ block = 1;
+ break;
+ default:
+ dev_dbg(&priv->adapter.dev, " [acc] Unsupported transaction %d\n", size);
+ return -EOPNOTSUPP;
+ }
+
+ if (hwpec) { /* enable/disable hardware PEC */
+ dev_dbg(&priv->adapter.dev, " [acc] hwpec: yes\n");
+ outb_p(inb_p(SMBAUXCTL(priv)) | SMBAUXCTL_CRC, SMBAUXCTL(priv));
+ } else {
+ dev_dbg(&priv->adapter.dev, " [acc] hwpec: no\n");
+ outb_p(inb_p(SMBAUXCTL(priv)) & (~SMBAUXCTL_CRC), SMBAUXCTL(priv));
+ }
+
+ if (block) {
+ //ret = 0;
+ dev_dbg(&priv->adapter.dev, " [acc] block: yes\n");
+ ret = i801_block_transaction(priv, data, read_write, size, hwpec);
+ } else {
+ dev_dbg(&priv->adapter.dev, " [acc] block: no\n");
+ ret = i801_transaction(priv, xact | ENABLE_INT9);
+ }
+
+ /* Some BIOSes don't like it when PEC is enabled at reboot or resume
+ time, so we forcibly disable it after every transaction. Turn off
+ E32B for the same reason. */
+ if (hwpec || block) {
+ dev_dbg(&priv->adapter.dev, " [acc] hwpec || block\n");
+ outb_p(inb_p(SMBAUXCTL(priv)) & ~(SMBAUXCTL_CRC | SMBAUXCTL_E32B), SMBAUXCTL(priv));
+ }
+ if (block) {
+ dev_dbg(&priv->adapter.dev, " [acc] block\n");
+ return ret;
+ }
+ if (ret) {
+ dev_dbg(&priv->adapter.dev, " [acc] ret %d\n", ret);
+ return ret;
+ }
+ if ((read_write == I2C_SMBUS_WRITE) || (xact == I801_QUICK)) {
+ dev_dbg(&priv->adapter.dev, " [acc] I2C_SMBUS_WRITE || I801_QUICK -> ret 0\n");
+ return 0;
+ }
+
+ switch (xact & 0x7f) {
+ case I801_BYTE: /* Result put in SMBHSTDAT0 */
+ case I801_BYTE_DATA:
+ dev_dbg(&priv->adapter.dev, " [acc] I801_BYTE or I801_BYTE_DATA\n");
+ data->byte = inb_p(SMBHSTDAT0(priv));
+ break;
+ case I801_WORD_DATA:
+ dev_dbg(&priv->adapter.dev, " [acc] I801_WORD_DATA\n");
+ data->word = inb_p(SMBHSTDAT0(priv)) + (inb_p(SMBHSTDAT1(priv)) << 8);
+ break;
+ }
+ return 0;
}



static u32 i801_func(struct i2c_adapter *adapter)
{
- struct i2c_device *priv = i2c_get_adapdata(adapter);
-
- /* original settings
- u32 f = I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
- I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
- I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_WRITE_I2C_BLOCK |
- ((priv->features & FEATURE_SMBUS_PEC) ? I2C_FUNC_SMBUS_PEC : 0) |
- ((priv->features & FEATURE_I2C_BLOCK_READ) ?
- I2C_FUNC_SMBUS_READ_I2C_BLOCK : 0);
- */
-
- // http://lxr.free-electrons.com/source/include/uapi/linux/i2c.h#L85
-
- u32 f =
- I2C_FUNC_I2C | /* 0x00000001 (I enabled this one) */
- !I2C_FUNC_10BIT_ADDR | /* 0x00000002 */
- !I2C_FUNC_PROTOCOL_MANGLING | /* 0x00000004 */
- ((priv->features & FEATURE_SMBUS_PEC) ? I2C_FUNC_SMBUS_PEC : 0) | /* 0x00000008 */
- !I2C_FUNC_SMBUS_BLOCK_PROC_CALL | /* 0x00008000 */
- I2C_FUNC_SMBUS_QUICK | /* 0x00010000 */
- !I2C_FUNC_SMBUS_READ_BYTE | /* 0x00020000 */
- !I2C_FUNC_SMBUS_WRITE_BYTE | /* 0x00040000 */
- !I2C_FUNC_SMBUS_READ_BYTE_DATA | /* 0x00080000 */
- !I2C_FUNC_SMBUS_WRITE_BYTE_DATA | /* 0x00100000 */
- !I2C_FUNC_SMBUS_READ_WORD_DATA | /* 0x00200000 */
- !I2C_FUNC_SMBUS_WRITE_WORD_DATA | /* 0x00400000 */
- !I2C_FUNC_SMBUS_PROC_CALL | /* 0x00800000 */
- !I2C_FUNC_SMBUS_READ_BLOCK_DATA | /* 0x01000000 */
- !I2C_FUNC_SMBUS_WRITE_BLOCK_DATA | /* 0x02000000 */
- ((priv->features & FEATURE_I2C_BLOCK_READ) ? I2C_FUNC_SMBUS_READ_I2C_BLOCK : 0) | /* 0x04000000 */
- I2C_FUNC_SMBUS_WRITE_I2C_BLOCK | /* 0x08000000 */
-
- I2C_FUNC_SMBUS_BYTE | /* _READ_BYTE _WRITE_BYTE */
- I2C_FUNC_SMBUS_BYTE_DATA | /* _READ_BYTE_DATA _WRITE_BYTE_DATA */
- I2C_FUNC_SMBUS_WORD_DATA | /* _READ_WORD_DATA _WRITE_WORD_DATA */
- I2C_FUNC_SMBUS_BLOCK_DATA | /* _READ_BLOCK_DATA _WRITE_BLOCK_DATA */
- !I2C_FUNC_SMBUS_I2C_BLOCK | /* _READ_I2C_BLOCK _WRITE_I2C_BLOCK */
- !I2C_FUNC_SMBUS_EMUL; /* _QUICK _BYTE _BYTE_DATA _WORD_DATA _PROC_CALL _WRITE_BLOCK_DATA _I2C_BLOCK _PEC */
- return f;
+ struct i2c_device *priv = i2c_get_adapdata(adapter);
+
+ /* original settings
+ u32 f = I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
+ I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
+ I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_WRITE_I2C_BLOCK |
+ ((priv->features & FEATURE_SMBUS_PEC) ? I2C_FUNC_SMBUS_PEC : 0) |
+ ((priv->features & FEATURE_I2C_BLOCK_READ) ?
+ I2C_FUNC_SMBUS_READ_I2C_BLOCK : 0);
+ */
+
+ // http://lxr.free-electrons.com/source/include/uapi/linux/i2c.h#L85
+
+ u32 f =
+ I2C_FUNC_I2C | /* 0x00000001 (I enabled this one) */
+ !I2C_FUNC_10BIT_ADDR | /* 0x00000002 */
+ !I2C_FUNC_PROTOCOL_MANGLING | /* 0x00000004 */
+ ((priv->features & FEATURE_SMBUS_PEC) ? I2C_FUNC_SMBUS_PEC : 0) | /* 0x00000008 */
+ !I2C_FUNC_SMBUS_BLOCK_PROC_CALL | /* 0x00008000 */
+ I2C_FUNC_SMBUS_QUICK | /* 0x00010000 */
+ !I2C_FUNC_SMBUS_READ_BYTE | /* 0x00020000 */
+ !I2C_FUNC_SMBUS_WRITE_BYTE | /* 0x00040000 */
+ !I2C_FUNC_SMBUS_READ_BYTE_DATA | /* 0x00080000 */
+ !I2C_FUNC_SMBUS_WRITE_BYTE_DATA | /* 0x00100000 */
+ !I2C_FUNC_SMBUS_READ_WORD_DATA | /* 0x00200000 */
+ !I2C_FUNC_SMBUS_WRITE_WORD_DATA | /* 0x00400000 */
+ !I2C_FUNC_SMBUS_PROC_CALL | /* 0x00800000 */
+ !I2C_FUNC_SMBUS_READ_BLOCK_DATA | /* 0x01000000 */
+ !I2C_FUNC_SMBUS_WRITE_BLOCK_DATA | /* 0x02000000 */
+ ((priv->features & FEATURE_I2C_BLOCK_READ) ? I2C_FUNC_SMBUS_READ_I2C_BLOCK : 0) | /* 0x04000000 */
+ I2C_FUNC_SMBUS_WRITE_I2C_BLOCK | /* 0x08000000 */
+
+ I2C_FUNC_SMBUS_BYTE | /* _READ_BYTE _WRITE_BYTE */
+ I2C_FUNC_SMBUS_BYTE_DATA | /* _READ_BYTE_DATA _WRITE_BYTE_DATA */
+ I2C_FUNC_SMBUS_WORD_DATA | /* _READ_WORD_DATA _WRITE_WORD_DATA */
+ I2C_FUNC_SMBUS_BLOCK_DATA | /* _READ_BLOCK_DATA _WRITE_BLOCK_DATA */
+ !I2C_FUNC_SMBUS_I2C_BLOCK | /* _READ_I2C_BLOCK _WRITE_I2C_BLOCK */
+ !I2C_FUNC_SMBUS_EMUL; /* _QUICK _BYTE _BYTE_DATA _WORD_DATA _PROC_CALL _WRITE_BLOCK_DATA _I2C_BLOCK _PEC */
+ return f;
}

static const struct i2c_algorithm smbus_algorithm = {
- .smbus_xfer = i801_access,
- .functionality = i801_func,
+ .smbus_xfer = i801_access,
+ .functionality = i801_func,
};


@@ -616,84 +616,84 @@ static const struct i2c_algorithm smbus_algorithm = {
********************************/
int pi2c_probe(struct platform_device *pldev)
{
- int err;
- struct i2c_device *priv;
- struct resource *res;
-
- dev_dbg(&pldev->dev, "pi2c_probe(pldev = %p '%s')\n", pldev, pldev->name);
-
- priv = kzalloc(sizeof(struct i2c_device), GFP_KERNEL);
- if (!priv) {
- return -ENOMEM;
- }
-
- i2c_set_adapdata(&priv->adapter, priv);
- priv->adapter.owner = THIS_MODULE;
- priv->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
- priv->adapter.algo = &smbus_algorithm;
-
- res = platform_get_resource(pldev, IORESOURCE_MEM, 0);
- priv->smba = (unsigned long)ioremap_nocache(res->start, resource_size(res));
-
- priv->pldev = pldev;
- pldev->dev.platform_data = priv;
-
- priv->features |= FEATURE_IDF;
- priv->features |= FEATURE_I2C_BLOCK_READ;
- priv->features |= FEATURE_SMBUS_PEC;
- priv->features |= FEATURE_BLOCK_BUFFER;
-
- //init_MUTEX(&lddata->sem);
- init_rwsem(&priv->rw_sem);
-
- /* set up the sysfs linkage to our parent device */
- priv->adapter.dev.parent = &pldev->dev;
-
- /* Retry up to 3 times on lost arbitration */
- priv->adapter.retries = 3;
-
- //snprintf(priv->adapter.name, sizeof(priv->adapter.name), "Fake SMBus I801 adapter at %04lx", priv->smba);
- snprintf(priv->adapter.name, sizeof(priv->adapter.name), "Fake SMBus I801 adapter");
-
- err = i2c_add_adapter(&priv->adapter);
- if (err) {
- dev_err(&priv->adapter.dev, "Failed to add SMBus adapter\n");
- return err;
- }
-
- return 0;
+ int err;
+ struct i2c_device *priv;
+ struct resource *res;
+
+ dev_dbg(&pldev->dev, "pi2c_probe(pldev = %p '%s')\n", pldev, pldev->name);
+
+ priv = kzalloc(sizeof(struct i2c_device), GFP_KERNEL);
+ if (!priv) {
+ return -ENOMEM;
+ }
+
+ i2c_set_adapdata(&priv->adapter, priv);
+ priv->adapter.owner = THIS_MODULE;
+ priv->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
+ priv->adapter.algo = &smbus_algorithm;
+
+ res = platform_get_resource(pldev, IORESOURCE_MEM, 0);
+ priv->smba = (unsigned long)ioremap_nocache(res->start, resource_size(res));
+
+ priv->pldev = pldev;
+ pldev->dev.platform_data = priv;
+
+ priv->features |= FEATURE_IDF;
+ priv->features |= FEATURE_I2C_BLOCK_READ;
+ priv->features |= FEATURE_SMBUS_PEC;
+ priv->features |= FEATURE_BLOCK_BUFFER;
+
+ //init_MUTEX(&lddata->sem);
+ init_rwsem(&priv->rw_sem);
+
+ /* set up the sysfs linkage to our parent device */
+ priv->adapter.dev.parent = &pldev->dev;
+
+ /* Retry up to 3 times on lost arbitration */
+ priv->adapter.retries = 3;
+
+ //snprintf(priv->adapter.name, sizeof(priv->adapter.name), "Fake SMBus I801 adapter at %04lx", priv->smba);
+ snprintf(priv->adapter.name, sizeof(priv->adapter.name), "Fake SMBus I801 adapter");
+
+ err = i2c_add_adapter(&priv->adapter);
+ if (err) {
+ dev_err(&priv->adapter.dev, "Failed to add SMBus adapter\n");
+ return err;
+ }
+
+ return 0;
}

int pi2c_remove(struct platform_device *pldev)
{
- struct i2c_device *lddev;
- dev_dbg(&pldev->dev, "pi2c_remove(pldev = %p '%s')\n", pldev, pldev->name);
-
- lddev = (struct i2c_device *)pldev->dev.platform_data;
-
- i2c_del_adapter(&lddev->adapter);
-
- //TODO: Figure out the right thing to do here...
- //pci_write_config_byte(dev, SMBHSTCFG, priv->original_hstcfg);
- //pci_release_region(dev, SMBBAR);
- //pci_set_drvdata(dev, NULL);
-
- //cdev_del(&lddev->cdev);
- if(lddev != 0) {
- kfree(lddev);
- pldev->dev.platform_data = 0;
- }
-
- return 0;
+ struct i2c_device *lddev;
+ dev_dbg(&pldev->dev, "pi2c_remove(pldev = %p '%s')\n", pldev, pldev->name);
+
+ lddev = (struct i2c_device *)pldev->dev.platform_data;
+
+ i2c_del_adapter(&lddev->adapter);
+
+ //TODO: Figure out the right thing to do here...
+ //pci_write_config_byte(dev, SMBHSTCFG, priv->original_hstcfg);
+ //pci_release_region(dev, SMBBAR);
+ //pci_set_drvdata(dev, NULL);
+
+ //cdev_del(&lddev->cdev);
+ if(lddev != 0) {
+ kfree(lddev);
+ pldev->dev.platform_data = 0;
+ }
+
+ return 0;
}

struct platform_driver i2c_plat_driver_i = {
- .probe = pi2c_probe,
- .remove = pi2c_remove,
- .driver = {
- .name = KP_DRIVER_NAME_I2C,
- .owner = THIS_MODULE,
- },
+ .probe = pi2c_probe,
+ .remove = pi2c_remove,
+ .driver = {
+ .name = KP_DRIVER_NAME_I2C,
+ .owner = THIS_MODULE,
+ },
};

module_platform_driver(i2c_plat_driver_i);
--
2.21.0

2019-05-18 04:19:58

by Geordan Neukum

Subject: Re: [PATCH 4/5] staging: kpc2000: kpc_i2c: use %s with __func__ identifier in log messages

On Fri, May 17, 2019 at 07:58:19PM -0700, Joe Perches wrote:
> On Sat, 2019-05-18 at 02:29 +0000, Geordan Neukum wrote:
> > Throughout i2c_driver.c, there are instances where the log strings
> > contain the function's name hardcoded into the string. Instead, use the
> > printk conversion specifier '%s' with the __func__ preprocessor
> > identifier to more maintainably print the function's name.
>
> Might as well remove all of these and use the
> builtin ftrace function tracing mechanism instead.
>
> > diff --git a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
> []
> > @@ -142,7 +142,7 @@ static int i801_check_pre(struct i2c_device *priv)
> > {
> > int status;
> >
> > - dev_dbg(&priv->adapter.dev, "i801_check_pre\n");
> > + dev_dbg(&priv->adapter.dev, "%s\n", __func__);
>
> etc...
>
Joe/All,

Acknowledged. I apologize for the inconvenience there -- I was
unfamiliar with that API until receiving your email. I'll hold on
additional uploads until other reviewers have had time to take a
look, but I do plan on leveraging the ftrace API instead of just
using __func__ and %s in my printk strings in the upcoming 'v2'
patchset.

Thanks for your feedback,
Geordan

2019-05-18 04:45:57

by Joe Perches

Subject: Re: [PATCH 4/5] staging: kpc2000: kpc_i2c: use %s with __func__ identifier in log messages

On Sat, 2019-05-18 at 02:29 +0000, Geordan Neukum wrote:
> Throughout i2c_driver.c, there are instances where the log strings
> contain the function's name hardcoded into the string. Instead, use the
> printk conversion specifier '%s' with the __func__ preprocessor
> identifier to more maintainably print the function's name.

Might as well remove all of these and use the
builtin ftrace function tracing mechanism instead.

> diff --git a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
[]
> @@ -142,7 +142,7 @@ static int i801_check_pre(struct i2c_device *priv)
> {
> int status;
>
> - dev_dbg(&priv->adapter.dev, "i801_check_pre\n");
> + dev_dbg(&priv->adapter.dev, "%s\n", __func__);

etc...

2019-05-20 09:46:33

by Greg Kroah-Hartman

Subject: Re: [PATCH 0/5] Updates to staging driver: kpc_i2c

On Sat, May 18, 2019 at 02:29:55AM +0000, Geordan Neukum wrote:
> Attached are an assortment of updates to the kpc_i2c driver in the
> staging subtree.

All now queued up. I'll rebase my patches that move this file on top of
yours, as kbuild found some problems with mine, and I'll resend them to
the list.

thanks,

greg k-h

2019-05-21 08:17:30

by Geordan Neukum

Subject: Re: [PATCH 0/5] Updates to staging driver: kpc_i2c

On Mon, May 20, 2019 at 10:30:26AM +0200, Greg Kroah-Hartman wrote:
> On Sat, May 18, 2019 at 02:29:55AM +0000, Geordan Neukum wrote:
> > Attached are an assortment of updates to the kpc_i2c driver in the
> > staging subtree.
>
> All now queued up. I'll rebase my patches that move this file on top of
> yours, as kbuild found some problems with mine, and I'll resend them to
> the list.
>
Thanks.

Additionally, I plan on trying to clean up that driver a bit more. Should I
base my future patches off of the staging tree so that I'll have the
"latest" driver as my basepoint? I don't want to cause any headaches
for anyone in the future.

Apologies, if I missed something obvious on the newbies wiki.
Assuming that I did not, I will certainly go ahead and try to document
this case either on or as a link from the "sending your first patch"
page.

Cheers,
Geordan

On Mon, May 20, 2019 at 8:30 AM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Sat, May 18, 2019 at 02:29:55AM +0000, Geordan Neukum wrote:
> > Attached are an assortment of updates to the kpc_i2c driver in the
> > staging subtree.
>
> All now queued up. I'll rebase my patches that move this file on top of
> yours, as kbuild found some problems with mine, and I'll resend them to
> the list.
>
> thanks,
>
> greg k-h

2019-05-21 08:36:47

by Geordan Neukum

Subject: Re: [PATCH 0/5] Updates to staging driver: kpc_i2c

> On Mon, May 20, 2019 at 10:30:26AM +0200, Greg Kroah-Hartman wrote:
>
> All now queued up. I'll rebase my patches that move this file on top of
> yours, as kbuild found some problems with mine, and I'll resend them to
> the list.
>
> Thanks.

** Same content as last reply, just realized that I had configured my
** email client to do something wrong. Resend for readability's sake.

Additionally, I plan on trying to clean up that driver a bit more. Should I
base my future patches off of the staging tree so that I'll have the
"latest" driver as my basepoint? I don't want to cause any headaches
for anyone in the future.

Apologies, if I missed something obvious on the newbies wiki.
Assuming that I did not, I will certainly go ahead and try to document
this case either on or as a link from the "sending your first patch"
page.

Cheers,
Geordan

2019-05-21 08:40:23

by Greg Kroah-Hartman

Subject: Re: [PATCH 0/5] Updates to staging driver: kpc_i2c

On Tue, May 21, 2019 at 08:15:52AM +0000, Geordan Neukum wrote:
> On Mon, May 20, 2019 at 10:30:26AM +0200, Greg Kroah-Hartman wrote:
> > On Sat, May 18, 2019 at 02:29:55AM +0000, Geordan Neukum wrote:
> > > Attached are an assortment of updates to the kpc_i2c driver in the
> > > staging subtree.
> >
> > All now queued up. I'll rebase my patches that move this file on top of
> > yours, as kbuild found some problems with mine, and I'll resend them to
> > the list.
> >
> Thanks.
>
> Additionally, I plan on trying to clean up that driver a bit more. Should I
> base my future patches off of the staging tree so that I'll have the
> "latest" driver as my basepoint? I don't want to cause any headaches
> for anyone in the future.

Yes, please do so. Please work off of either the staging-next or even
better, staging-testing as that contains the latest patches. I apply
patches to the -testing branch first, and if that passes the 0-day bot,
I then merge them to -next.

Given that there are a lot of people working on this codebase right now
(as it needs so much obvious work), I would recommend using -next and
getting used to rebasing your changes :)

I take patches as they are submitted, so sometimes people do step on
each other's toes, that's normal. But I think you are the only one
touching the i2c driver at the moment so it shouldn't be that bad.

> Apologies, if I missed something obvious on the newbies wiki.
> Assuming that I did not, I will certainly go ahead and try to document
> this case either on or as a link from the "sending your first patch"
> page.

This is beyond the "first patch" work, this is now in the "being an
active developer" workflow :)

thanks,

greg k-h