LKML Archive on lore.kernel.org
 help / Atom feed
* [PATCH 0/5] Updates to staging driver: kpc_i2c
@ 2019-05-18  2:29 Geordan Neukum
  2019-05-18  2:29 ` [PATCH 1/5] staging: kpc2000: kpc_i2c: reindent i2c_driver.c Geordan Neukum
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Geordan Neukum @ 2019-05-18  2:29 UTC (permalink / raw)
  To: gneukum1, Greg Kroah-Hartman, devel, linux-kernel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/5] staging: kpc2000: kpc_i2c: reindent i2c_driver.c
  2019-05-18  2:29 [PATCH 0/5] Updates to staging driver: kpc_i2c Geordan Neukum
@ 2019-05-18  2:29 ` Geordan Neukum
  2019-05-18  2:29 ` [PATCH 2/5] staging: kpc2000: kpc_i2c: reformat copyright for better readability Geordan Neukum
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Geordan Neukum @ 2019-05-18  2:29 UTC (permalink / raw)
  To: gneukum1, Greg Kroah-Hartman, devel, linux-kernel

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 <gneukum1@gmail.com>
---
 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 <matt.sickler@daktronics.com>,
-                             Jordon Hofer <jordon.hofer@daktronics.com>
+			      Matt Sickler <matt.sickler@daktronics.com>,
+			      Jordon Hofer <jordon.hofer@daktronics.com>
     Adapted i2c-i801.c for use with Kadoka hardware.
     Copyright (c) 1998 - 2002  Frodo Looijaard <frodol@dds.nl>,
     Philip Edelbrock <phil@netroedge.com>, and Mark D. Studebaker
     <mdsxyz123@yahoo.com>
     Copyright (C) 2007 - 2012  Jean Delvare <khali@linux-fr.org>
     Copyright (C) 2010         Intel Corporation,
-                               David Woodhouse <dwmw2@infradead.org>
+				David Woodhouse <dwmw2@infradead.org>
 */
 #include <linux/init.h>
 #include <linux/module.h>
@@ -29,11 +29,11 @@ MODULE_AUTHOR("Matt.Sickler@Daktronics.com");
 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/5] staging: kpc2000: kpc_i2c: reformat copyright for better readability
  2019-05-18  2:29 [PATCH 0/5] Updates to staging driver: kpc_i2c Geordan Neukum
  2019-05-18  2:29 ` [PATCH 1/5] staging: kpc2000: kpc_i2c: reindent i2c_driver.c Geordan Neukum
@ 2019-05-18  2:29 ` Geordan Neukum
  2019-05-18  2:29 ` [PATCH 3/5] staging: kpc2000: kpc_i2c: prevent memory leak in probe() error case Geordan Neukum
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Geordan Neukum @ 2019-05-18  2:29 UTC (permalink / raw)
  To: gneukum1, Greg Kroah-Hartman, devel, linux-kernel

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 <gneukum1@gmail.com>
---
 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 <matt.sickler@daktronics.com>,
-			      Jordon Hofer <jordon.hofer@daktronics.com>
-    Adapted i2c-i801.c for use with Kadoka hardware.
-    Copyright (c) 1998 - 2002  Frodo Looijaard <frodol@dds.nl>,
-    Philip Edelbrock <phil@netroedge.com>, and Mark D. Studebaker
-    <mdsxyz123@yahoo.com>
-    Copyright (C) 2007 - 2012  Jean Delvare <khali@linux-fr.org>
-    Copyright (C) 2010         Intel Corporation,
-				David Woodhouse <dwmw2@infradead.org>
-*/
+/*
+ * KPC2000 i2c driver
+ *
+ * Adapted i2c-i801.c for use with Kadoka hardware.
+ *
+ * Copyright (C) 1998 - 2002
+ *	Frodo Looijaard <frodol@dds.nl>,
+ *	Philip Edelbrock <phil@netroedge.com>,
+ *	Mark D. Studebaker <mdsxyz123@yahoo.com>
+ * Copyright (C) 2007 - 2012
+ *	Jean Delvare <khali@linux-fr.org>
+ * Copyright (C) 2010 Intel Corporation
+ *	David Woodhouse <dwmw2@infradead.org>
+ * Copyright (C) 2014-2018 Daktronics
+ *	Matt Sickler <matt.sickler@daktronics.com>,
+ *	Jordon Hofer <jordon.hofer@daktronics.com>
+ */
 #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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/5] staging: kpc2000: kpc_i2c: prevent memory leak in probe() error case
  2019-05-18  2:29 [PATCH 0/5] Updates to staging driver: kpc_i2c Geordan Neukum
  2019-05-18  2:29 ` [PATCH 1/5] staging: kpc2000: kpc_i2c: reindent i2c_driver.c Geordan Neukum
  2019-05-18  2:29 ` [PATCH 2/5] staging: kpc2000: kpc_i2c: reformat copyright for better readability Geordan Neukum
@ 2019-05-18  2:29 ` Geordan Neukum
  2019-05-18  2:29 ` [PATCH 4/5] staging: kpc2000: kpc_i2c: use %s with __func__ identifier in log messages Geordan Neukum
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Geordan Neukum @ 2019-05-18  2:29 UTC (permalink / raw)
  To: gneukum1, Greg Kroah-Hartman, devel, linux-kernel

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 <gneukum1@gmail.com>
---
 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 4/5] staging: kpc2000: kpc_i2c: use %s with __func__ identifier in log messages
  2019-05-18  2:29 [PATCH 0/5] Updates to staging driver: kpc_i2c Geordan Neukum
                   ` (2 preceding siblings ...)
  2019-05-18  2:29 ` [PATCH 3/5] staging: kpc2000: kpc_i2c: prevent memory leak in probe() error case Geordan Neukum
@ 2019-05-18  2:29 ` Geordan Neukum
  2019-05-18  2:58   ` Joe Perches
  2019-05-18  2:30 ` [PATCH 5/5] staging: kpc2000: kpc_i2c: fixup block comment style in i2c_driver.c Geordan Neukum
  2019-05-20  8:30 ` [PATCH 0/5] Updates to staging driver: kpc_i2c Greg Kroah-Hartman
  5 siblings, 1 reply; 12+ messages in thread
From: Geordan Neukum @ 2019-05-18  2:29 UTC (permalink / raw)
  To: gneukum1, Greg Kroah-Hartman, devel, linux-kernel

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 <gneukum1@gmail.com>
---
 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 5/5] staging: kpc2000: kpc_i2c: fixup block comment style in i2c_driver.c
  2019-05-18  2:29 [PATCH 0/5] Updates to staging driver: kpc_i2c Geordan Neukum
                   ` (3 preceding siblings ...)
  2019-05-18  2:29 ` [PATCH 4/5] staging: kpc2000: kpc_i2c: use %s with __func__ identifier in log messages Geordan Neukum
@ 2019-05-18  2:30 ` Geordan Neukum
  2019-05-20  8:30 ` [PATCH 0/5] Updates to staging driver: kpc_i2c Greg Kroah-Hartman
  5 siblings, 0 replies; 12+ messages in thread
From: Geordan Neukum @ 2019-05-18  2:30 UTC (permalink / raw)
  To: gneukum1, Greg Kroah-Hartman, devel, linux-kernel

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 <gneukum1@gmail.com>
---
 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] staging: kpc2000: kpc_i2c: use %s with __func__ identifier in log messages
  2019-05-18  2:29 ` [PATCH 4/5] staging: kpc2000: kpc_i2c: use %s with __func__ identifier in log messages Geordan Neukum
@ 2019-05-18  2:58   ` Joe Perches
  2019-05-18  3:28     ` Geordan Neukum
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2019-05-18  2:58 UTC (permalink / raw)
  To: Geordan Neukum, Greg Kroah-Hartman, devel, linux-kernel

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...


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] staging: kpc2000: kpc_i2c: use %s with __func__ identifier in log messages
  2019-05-18  2:58   ` Joe Perches
@ 2019-05-18  3:28     ` Geordan Neukum
  0 siblings, 0 replies; 12+ messages in thread
From: Geordan Neukum @ 2019-05-18  3:28 UTC (permalink / raw)
  To: Joe Perches; +Cc: Greg Kroah-Hartman, devel, linux-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/5] Updates to staging driver: kpc_i2c
  2019-05-18  2:29 [PATCH 0/5] Updates to staging driver: kpc_i2c Geordan Neukum
                   ` (4 preceding siblings ...)
  2019-05-18  2:30 ` [PATCH 5/5] staging: kpc2000: kpc_i2c: fixup block comment style in i2c_driver.c Geordan Neukum
@ 2019-05-20  8:30 ` Greg Kroah-Hartman
  2019-05-21  8:15   ` Geordan Neukum
  5 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2019-05-20  8:30 UTC (permalink / raw)
  To: Geordan Neukum; +Cc: devel, linux-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/5] Updates to staging driver: kpc_i2c
  2019-05-20  8:30 ` [PATCH 0/5] Updates to staging driver: kpc_i2c Greg Kroah-Hartman
@ 2019-05-21  8:15   ` Geordan Neukum
  2019-05-21  8:34     ` Geordan Neukum
  2019-05-21  8:37     ` Greg Kroah-Hartman
  0 siblings, 2 replies; 12+ messages in thread
From: Geordan Neukum @ 2019-05-21  8:15 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: devel, linux-kernel

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
<gregkh@linuxfoundation.org> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/5] Updates to staging driver: kpc_i2c
  2019-05-21  8:15   ` Geordan Neukum
@ 2019-05-21  8:34     ` Geordan Neukum
  2019-05-21  8:37     ` Greg Kroah-Hartman
  1 sibling, 0 replies; 12+ messages in thread
From: Geordan Neukum @ 2019-05-21  8:34 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: devel, linux-kernel

> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/5] Updates to staging driver: kpc_i2c
  2019-05-21  8:15   ` Geordan Neukum
  2019-05-21  8:34     ` Geordan Neukum
@ 2019-05-21  8:37     ` Greg Kroah-Hartman
  1 sibling, 0 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2019-05-21  8:37 UTC (permalink / raw)
  To: Geordan Neukum; +Cc: devel, linux-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-18  2:29 [PATCH 0/5] Updates to staging driver: kpc_i2c Geordan Neukum
2019-05-18  2:29 ` [PATCH 1/5] staging: kpc2000: kpc_i2c: reindent i2c_driver.c Geordan Neukum
2019-05-18  2:29 ` [PATCH 2/5] staging: kpc2000: kpc_i2c: reformat copyright for better readability Geordan Neukum
2019-05-18  2:29 ` [PATCH 3/5] staging: kpc2000: kpc_i2c: prevent memory leak in probe() error case Geordan Neukum
2019-05-18  2:29 ` [PATCH 4/5] staging: kpc2000: kpc_i2c: use %s with __func__ identifier in log messages Geordan Neukum
2019-05-18  2:58   ` Joe Perches
2019-05-18  3:28     ` Geordan Neukum
2019-05-18  2:30 ` [PATCH 5/5] staging: kpc2000: kpc_i2c: fixup block comment style in i2c_driver.c Geordan Neukum
2019-05-20  8:30 ` [PATCH 0/5] Updates to staging driver: kpc_i2c Greg Kroah-Hartman
2019-05-21  8:15   ` Geordan Neukum
2019-05-21  8:34     ` Geordan Neukum
2019-05-21  8:37     ` Greg Kroah-Hartman

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox