[PATCH 0/6] Minor updates to kpc_i2c driver and kpc2000 core

[PATCH 0/6] Minor updates to kpc_i2c driver and kpc2000 core

[Geordan Neukum]

Attached are an assortment of minor updates to the kpc_i2c driver as
well as a build fix for all of those who will need the KPC2000 core.

Thanks,
Geordan

Geordan Neukum (6):
  staging: kpc2000: make kconfig symbol 'KPC2000' select dependencies
  staging: kpc2000: kpc_i2c: remove unused module param disable_features
  staging: kpc2000: kpc_i2c: newline fixups to meet linux style guide
  staging: kpc2000: kpc_i2c: use <linux/io.h> instead of <asm/io.h>
  staging: kpc2000: kpc_i2c: Remove unnecessary function tracing prints
  staging: kpc2000: kpc_i2c: add static qual to local symbols in
    kpc_i2c.c

 drivers/staging/kpc2000/Kconfig       |   2 +
 drivers/staging/kpc2000/kpc2000_i2c.c | 118 +++++++-------------------
 2 files changed, 34 insertions(+), 86 deletions(-)

-- 
2.21.0

[PATCH 1/6] staging: kpc2000: make kconfig symbol 'KPC2000' select dependencies

[Geordan Neukum]

The kpc2000 core makes calls against functions which are conditionally
exported upon the kconfig symbols 'MFD_CORE' and 'UIO' being selected
Therefore, in order to guarantee correct compilation, the 'KPC2000'
kconfig symbol (which brings in that code) must explicitly select its
hard dependencies.

Signed-off-by: Geordan Neukum <gneukum1@gmail.com>
---
 drivers/staging/kpc2000/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/kpc2000/Kconfig b/drivers/staging/kpc2000/Kconfig
index fb5922928f47..8992dc67ff37 100644
--- a/drivers/staging/kpc2000/Kconfig
+++ b/drivers/staging/kpc2000/Kconfig
@@ -3,6 +3,8 @@
 config KPC2000
 	bool "Daktronics KPC Device support"
 	depends on PCI
+	select MFD_CORE
+	select UIO
 	help
 	  Select this if you wish to use the Daktronics KPC PCI devices
 
-- 
2.21.0

Re: [PATCH 1/6] staging: kpc2000: make kconfig symbol 'KPC2000' select dependencies

[Greg Kroah-Hartman]

On Wed, May 22, 2019 at 12:13:57PM +0000, Geordan Neukum wrote:
> The kpc2000 core makes calls against functions which are conditionally
> exported upon the kconfig symbols 'MFD_CORE' and 'UIO' being selected
> Therefore, in order to guarantee correct compilation, the 'KPC2000'
> kconfig symbol (which brings in that code) must explicitly select its
> hard dependencies.
> 
> Signed-off-by: Geordan Neukum <gneukum1@gmail.com>
> ---
>  drivers/staging/kpc2000/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/kpc2000/Kconfig b/drivers/staging/kpc2000/Kconfig
> index fb5922928f47..8992dc67ff37 100644
> --- a/drivers/staging/kpc2000/Kconfig
> +++ b/drivers/staging/kpc2000/Kconfig
> @@ -3,6 +3,8 @@
>  config KPC2000
>  	bool "Daktronics KPC Device support"
>  	depends on PCI
> +	select MFD_CORE
> +	select UIO

depends on is better than select.  There's a change to depend on UIO for
this code already in my -linus branch which will show up in Linus's tree
in a week or so.

Are you sure we need MFD_CORE as well for this code?  Why hasn't that
been seen on any build errors?

thanks,

greg k-h

Re: [PATCH 1/6] staging: kpc2000: make kconfig symbol 'KPC2000' select dependencies

[Geordan Neukum]

On Wed, May 22, 2019 at 12:27 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> depends on is better than select.  There's a change to depend on UIO for
> this code already in my -linus branch which will show up in Linus's tree
> in a week or so.

Noted on both accounts. Thanks for the feedback and sorry for the
inconvenience on the latter.

> Are you sure we need MFD_CORE as well for this code?

I noticed the build issue when working locally. I was doing
something along the lines of: 'make distclean && make x86_64_defconfig',
selecting 'CONFIG_KPC2000' and 'CONFIG_UIO' via menuconfig, then
running a good old 'make'. From make, I received an error along the
lines of:

ERROR: "mfd_remove_devices"
[drivers/staging/kpc2000/kpc2000/kpc2000.ko] undefined!
ERROR: "mfd_add_devices" [drivers/staging/kpc2000/kpc2000/kpc2000.ko] undefined!
make[1]: *** [scripts/Makefile.modpost:91: __modpost] Error 1
make: *** [Makefile:1290: modules] Error 2

which appears to indicate that those two symbols are undefined. When
I looked, it appeared that those symbols were exported from the
mfd-core which is why I also threw in a select for that Kconfig
symbol. Assuming that I didn't do something silly above, I'd be happy
to submit a new patch (with only a depends on for MFD_CORE) as I
continue trying to fix up the i2c driver.

>Why hasn't that been seen on any build errors?

To be honest, I can't say that I'm familiar with all of the different
build bots out there so I can't even begin to speculate on that one.
If someone could point me in the right direction there, I'd be happy
to investigate further.

Thanks again for your feedback all,
Geordan Neukum

Re: [PATCH 1/6] staging: kpc2000: make kconfig symbol 'KPC2000' select dependencies

[Greg Kroah-Hartman]

On Thu, May 23, 2019 at 01:35:02AM +0000, Geordan Neukum wrote:
> On Wed, May 22, 2019 at 12:27 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > depends on is better than select.  There's a change to depend on UIO for
> > this code already in my -linus branch which will show up in Linus's tree
> > in a week or so.
> 
> Noted on both accounts. Thanks for the feedback and sorry for the
> inconvenience on the latter.
> 
> > Are you sure we need MFD_CORE as well for this code?
> 
> I noticed the build issue when working locally. I was doing
> something along the lines of: 'make distclean && make x86_64_defconfig',
> selecting 'CONFIG_KPC2000' and 'CONFIG_UIO' via menuconfig, then
> running a good old 'make'. From make, I received an error along the
> lines of:
> 
> ERROR: "mfd_remove_devices"
> [drivers/staging/kpc2000/kpc2000/kpc2000.ko] undefined!
> ERROR: "mfd_add_devices" [drivers/staging/kpc2000/kpc2000/kpc2000.ko] undefined!
> make[1]: *** [scripts/Makefile.modpost:91: __modpost] Error 1
> make: *** [Makefile:1290: modules] Error 2
> 
> which appears to indicate that those two symbols are undefined. When
> I looked, it appeared that those symbols were exported from the
> mfd-core which is why I also threw in a select for that Kconfig
> symbol. Assuming that I didn't do something silly above, I'd be happy
> to submit a new patch (with only a depends on for MFD_CORE) as I
> continue trying to fix up the i2c driver.

Yes, a depends for MFD_CORE would be good, can you base it against my
staging-linus branch so that fix can go to Linus for this release?

thanks,

greg k-h

[PATCH v2] staging: kpc2000: Add dependency on MFD_CORE to kconfig symbol 'KPC2000'

[Geordan Neukum]

The kpc2000 core makes calls against functions conditionally exported
upon selection of the kconfig symbol MFD_CORE. Therefore, the kpc2000
core depends upon the mfd_core, and that dependency must be tracked in
Kconfig to avoid potential build issues.

Signed-off-by: Geordan Neukum <gneukum1@gmail.com>
---
v2 changes
  - base patch on staging-linus
  - only add MFD_CORE dependency as the UIO dependency has already been
    handled by YueHaibing

 drivers/staging/kpc2000/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/kpc2000/Kconfig b/drivers/staging/kpc2000/Kconfig
index febe4f8b30e5..ef0f4abe894a 100644
--- a/drivers/staging/kpc2000/Kconfig
+++ b/drivers/staging/kpc2000/Kconfig
@@ -2,6 +2,7 @@

 config KPC2000
 	bool "Daktronics KPC Device support"
+	depends on MFD_CORE
 	depends on PCI
 	depends on UIO
 	help
--
2.21.0

Re: [PATCH v2] staging: kpc2000: Add dependency on MFD_CORE to kconfig symbol 'KPC2000'

[Geordan Neukum]

On Fri, May 24, 2019 at 2:38 AM Geordan Neukum <gneukum1@gmail.com> wrote:
> +       depends on MFD_CORE

In order for this to work in menuconfig, this either needs to be a
select or I need to
add a prompt to MFD_CORE. I don't have strong feelings either way, but all other
Kconfig options which are related to the MFD_CORE appear to do a straight
selection. Let me know what you think and I'll go that route.

Thanks,
Geordan

Re: [PATCH v2] staging: kpc2000: Add dependency on MFD_CORE to kconfig symbol 'KPC2000'

[Greg Kroah-Hartman]

On Fri, May 24, 2019 at 03:02:48AM +0000, Geordan Neukum wrote:
> On Fri, May 24, 2019 at 2:38 AM Geordan Neukum <gneukum1@gmail.com> wrote:
> > +       depends on MFD_CORE
> 
> In order for this to work in menuconfig, this either needs to be a
> select or I need to
> add a prompt to MFD_CORE. I don't have strong feelings either way, but all other
> Kconfig options which are related to the MFD_CORE appear to do a straight
> selection. Let me know what you think and I'll go that route.

Yeah, you are right, sorry about that.  Let me just go hand edit-this
patch and queue it up as this was my fault...

thanks,

greg k-h

[PATCH 2/6] staging: kpc2000: kpc_i2c: remove unused module param disable_features

[Geordan Neukum]

The module parameter 'disable_features' is currently unused. Therefore,
it should be removed.

Signed-off-by: Geordan Neukum <gneukum1@gmail.com>
---
 drivers/staging/kpc2000/kpc2000_i2c.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c b/drivers/staging/kpc2000/kpc2000_i2c.c
index 42061318d2d4..40a89998726e 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -126,10 +126,6 @@ struct i2c_device {
 /* Not really a feature, but it's convenient to handle it as such */
 #define FEATURE_IDF             (1 << 15)
 
-static unsigned int disable_features;
-module_param(disable_features, uint, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(disable_features, "Disable selected driver features");
-
 // FIXME!
 #undef inb_p
 #define inb_p(a) readq((void*)a)
-- 
2.21.0

[PATCH 3/6] staging: kpc2000: kpc_i2c: newline fixups to meet linux style guide

[Geordan Neukum]

The linux coding style document states:

  1) That braces should not be used where a single single statement
     will do. Therefore all instances of single block statements
     wrapped in braces that do not meet the qualifications of any
     of the exceptions to the rule should be fixed up.

  2) That the declaration of variables local to a given function
     should be immediately followed by a blank newline. Therefore,
     the single instance of this in kpc2000_i2c.c should be fixed
     up.

Signed-off-by: Geordan Neukum <gneukum1@gmail.com>
---
 drivers/staging/kpc2000/kpc2000_i2c.c | 82 ++++++++++-----------------
 1 file changed, 29 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c b/drivers/staging/kpc2000/kpc2000_i2c.c
index 40a89998726e..a1ebc2386d70 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -178,9 +178,8 @@ static int i801_check_post(struct i2c_device *priv, int status, int timeout)
 
 		/* Check if it worked */
 		status = inb_p(SMBHSTSTS(priv));
-		if ((status & SMBHSTSTS_HOST_BUSY) || !(status & SMBHSTSTS_FAILED)) {
+		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;
 	}
@@ -202,9 +201,8 @@ static int i801_check_post(struct i2c_device *priv, int status, int timeout)
 		/* Clear error flags */
 		outb_p(status & STATUS_FLAGS, SMBHSTSTS(priv));
 		status = inb_p(SMBHSTSTS(priv)) & STATUS_FLAGS;
-		if (status) {
+		if (status)
 			dev_warn(&priv->adapter.dev, "Failed clearing status flags at end of transaction (%02x)\n", status);
-		}
 	}
 
 	return result;
@@ -219,9 +217,8 @@ static int i801_transaction(struct i2c_device *priv, int xact)
 	dev_dbg(&priv->adapter.dev, "%s\n", __func__);
 
 	result = i801_check_pre(priv);
-	if (result < 0) {
+	if (result < 0)
 		return result;
-	}
 	/* the current contents of SMBHSTCNT can be overwritten, since PEC,
 	 * INTREN, SMBSCMD are passed in xact
 	 */
@@ -234,9 +231,8 @@ static int i801_transaction(struct i2c_device *priv, int xact)
 	} while ((status & SMBHSTSTS_HOST_BUSY) && (timeout++ < MAX_RETRIES));
 
 	result = i801_check_post(priv, status, timeout > MAX_RETRIES);
-	if (result < 0) {
+	if (result < 0)
 		return result;
-	}
 
 	outb_p(SMBHSTSTS_INTR, SMBHSTSTS(priv));
 	return 0;
@@ -255,9 +251,8 @@ static void i801_wait_hwpec(struct i2c_device *priv)
 		status = inb_p(SMBHSTSTS(priv));
 	} while ((!(status & SMBHSTSTS_INTR)) && (timeout++ < MAX_RETRIES));
 
-	if (timeout > MAX_RETRIES) {
+	if (timeout > MAX_RETRIES)
 		dev_dbg(&priv->adapter.dev, "PEC Timeout!\n");
-	}
 
 	outb_p(status, SMBHSTSTS(priv));
 }
@@ -275,26 +270,22 @@ static int i801_block_transaction_by_block(struct i2c_device *priv, union i2c_sm
 	if (read_write == I2C_SMBUS_WRITE) {
 		len = data->block[0];
 		outb_p(len, SMBHSTDAT0(priv));
-		for (i = 0; i < len; i++) {
+		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) {
+	if (status)
 		return status;
-	}
 
 	if (read_write == I2C_SMBUS_READ) {
 		len = inb_p(SMBHSTDAT0(priv));
-		if (len < 1 || len > I2C_SMBUS_BLOCK_MAX) {
+		if (len < 1 || len > I2C_SMBUS_BLOCK_MAX)
 			return -EPROTO;
-		}
 
 		data->block[0] = len;
-		for (i = 0; i < len; i++) {
+		for (i = 0; i < len; i++)
 			data->block[i + 1] = inb_p(SMBBLKDAT(priv));
-		}
 	}
 	return 0;
 }
@@ -310,9 +301,8 @@ static int i801_block_transaction_byte_by_byte(struct i2c_device *priv, union i2
 	dev_dbg(&priv->adapter.dev, "%s\n", __func__);
 
 	result = i801_check_pre(priv);
-	if (result < 0) {
+	if (result < 0)
 		return result;
-	}
 
 	len = data->block[0];
 
@@ -323,23 +313,20 @@ static int i801_block_transaction_byte_by_byte(struct i2c_device *priv, union i2
 
 	for (i = 1; i <= len; i++) {
 		if (i == len && read_write == I2C_SMBUS_READ) {
-			if (command == I2C_SMBUS_I2C_BLOCK_DATA) {
+			if (command == I2C_SMBUS_I2C_BLOCK_DATA)
 				smbcmd = I801_I2C_BLOCK_LAST;
-			} else {
+			else
 				smbcmd = I801_BLOCK_LAST;
-			}
 		} else {
-			if (command == I2C_SMBUS_I2C_BLOCK_DATA && read_write == I2C_SMBUS_READ) {
+			if (command == I2C_SMBUS_I2C_BLOCK_DATA && read_write == I2C_SMBUS_READ)
 				smbcmd = I801_I2C_BLOCK_DATA;
-			} else {
+			else
 				smbcmd = I801_BLOCK_DATA;
-			}
 		}
 		outb_p(smbcmd | ENABLE_INT9, SMBHSTCNT(priv));
 
-		if (i == 1) {
+		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 {
@@ -348,17 +335,15 @@ static int i801_block_transaction_byte_by_byte(struct i2c_device *priv, union i2
 		} while ((!(status & SMBHSTSTS_BYTE_DONE)) && (timeout++ < MAX_RETRIES));
 
 		result = i801_check_post(priv, status, timeout > MAX_RETRIES);
-		if (result < 0) {
+		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) {
+				while (inb_p(SMBHSTSTS(priv)) & SMBHSTSTS_HOST_BUSY)
 					outb_p(SMBHSTSTS_BYTE_DONE, SMBHSTSTS(priv));
-				}
 				outb_p(SMBHSTSTS_INTR, SMBHSTSTS(priv));
 				return -EPROTO;
 			}
@@ -366,12 +351,10 @@ static int i801_block_transaction_byte_by_byte(struct i2c_device *priv, union i2
 		}
 
 		/* Retrieve/store value in SMBBLKDAT */
-		if (read_write == I2C_SMBUS_READ) {
+		if (read_write == I2C_SMBUS_READ)
 			data->block[i] = inb_p(SMBBLKDAT(priv));
-		}
-		if (read_write == I2C_SMBUS_WRITE && i+1 <= len) {
+		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));
 	}
@@ -384,9 +367,8 @@ static int i801_set_block_buffer_mode(struct i2c_device *priv)
 	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) {
+	if ((inb_p(SMBAUXCTL(priv)) & SMBAUXCTL_E32B) == 0)
 		return -EIO;
-	}
 	return 0;
 }
 
@@ -411,12 +393,10 @@ static int i801_block_transaction(struct i2c_device *priv, union i2c_smbus_data
 	}
 
 	if (read_write == I2C_SMBUS_WRITE || command == I2C_SMBUS_I2C_BLOCK_DATA) {
-		if (data->block[0] < 1) {
+		if (data->block[0] < 1)
 			data->block[0] = 1;
-		}
-		if (data->block[0] > I2C_SMBUS_BLOCK_MAX) {
+		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 */
 	}
@@ -425,14 +405,12 @@ static int i801_block_transaction(struct i2c_device *priv, union i2c_smbus_data
 	 * 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) {
+	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 {
+	else
 		result = i801_block_transaction_byte_by_byte(priv, data, read_write, command, hwpec);
-	}
-	if (result == 0 && 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...
@@ -465,18 +443,16 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 		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) {
+		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) {
+		if (read_write == I2C_SMBUS_WRITE)
 			outb_p(data->byte, SMBHSTDAT0(priv));
-		}
 		xact = I801_BYTE_DATA;
 		break;
 	case I2C_SMBUS_WORD_DATA:
@@ -633,9 +609,8 @@ int pi2c_probe(struct platform_device *pldev)
 		pldev->name);
 
 	priv = devm_kzalloc(&pldev->dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv) {
+	if (!priv)
 		return -ENOMEM;
-	}
 
 	i2c_set_adapdata(&priv->adapter, priv);
 	priv->adapter.owner = THIS_MODULE;
@@ -677,6 +652,7 @@ int pi2c_probe(struct platform_device *pldev)
 int pi2c_remove(struct platform_device *pldev)
 {
 	struct i2c_device *lddev;
+
 	dev_dbg(&pldev->dev, "%s(pldev = %p '%s')\n", __func__, pldev,
 		pldev->name);
 
-- 
2.21.0

Re: [PATCH 3/6] staging: kpc2000: kpc_i2c: newline fixups to meet linux style guide

[Greg Kroah-Hartman]

On Wed, May 22, 2019 at 12:13:59PM +0000, Geordan Neukum wrote:
> The linux coding style document states:
> 
>   1) That braces should not be used where a single single statement
>      will do. Therefore all instances of single block statements
>      wrapped in braces that do not meet the qualifications of any
>      of the exceptions to the rule should be fixed up.
> 
>   2) That the declaration of variables local to a given function
>      should be immediately followed by a blank newline. Therefore,
>      the single instance of this in kpc2000_i2c.c should be fixed
>      up.

This really should be 2 different patches, but given that this file is
so messy, I'll take it for now :)

thanks,

greg k-h

[PATCH 4/6] staging: kpc2000: kpc_i2c: use <linux/io.h> instead of <asm/io.h>

[Geordan Neukum]

Rather than include asm/io.h, include linux/io.h. Issue reported
by the script checkpatch.pl.

Signed-off-by: Geordan Neukum <gneukum1@gmail.com>
---
 drivers/staging/kpc2000/kpc2000_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c b/drivers/staging/kpc2000/kpc2000_i2c.c
index a1ebc2386d70..5d98ed54c05c 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -19,7 +19,7 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/types.h>
-#include <asm/io.h>
+#include <linux/io.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/export.h>
 #include <linux/slab.h>
-- 
2.21.0

[PATCH 5/6] staging: kpc2000: kpc_i2c: Remove unnecessary function tracing prints

[Geordan Neukum]

Many of the functions in kpc_i2c log debug-level messages to the
kernel log message buffer upon invocation. This is unnecessary, as
debugging tools like kgdb, kdb, etc. or the tracing tool ftrace
should be able to provide this same information. Therefore, remove
these print statements.

Signed-off-by: Geordan Neukum <gneukum1@gmail.com>
---
 drivers/staging/kpc2000/kpc2000_i2c.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c b/drivers/staging/kpc2000/kpc2000_i2c.c
index 5d98ed54c05c..f9259c06b605 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -139,8 +139,6 @@ static int i801_check_pre(struct i2c_device *priv)
 {
 	int status;
 
-	dev_dbg(&priv->adapter.dev, "%s\n", __func__);
-
 	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);
@@ -165,8 +163,6 @@ static int i801_check_post(struct i2c_device *priv, int status, int timeout)
 {
 	int result = 0;
 
-	dev_dbg(&priv->adapter.dev, "%s\n", __func__);
-
 	/* If the SMBus is still busy, we give up */
 	if (timeout) {
 		dev_err(&priv->adapter.dev, "Transaction timeout\n");
@@ -214,8 +210,6 @@ static int i801_transaction(struct i2c_device *priv, int xact)
 	int result;
 	int timeout = 0;
 
-	dev_dbg(&priv->adapter.dev, "%s\n", __func__);
-
 	result = i801_check_pre(priv);
 	if (result < 0)
 		return result;
@@ -244,8 +238,6 @@ static void i801_wait_hwpec(struct i2c_device *priv)
 	int timeout = 0;
 	int status;
 
-	dev_dbg(&priv->adapter.dev, "%s\n", __func__);
-
 	do {
 		usleep_range(250, 500);
 		status = inb_p(SMBHSTSTS(priv));
@@ -262,8 +254,6 @@ static int i801_block_transaction_by_block(struct i2c_device *priv, union i2c_sm
 	int i, len;
 	int status;
 
-	dev_dbg(&priv->adapter.dev, "%s\n", __func__);
-
 	inb_p(SMBHSTCNT(priv)); /* reset the data buffer index */
 
 	/* Use 32-byte buffer to process this transaction */
@@ -298,8 +288,6 @@ static int i801_block_transaction_byte_by_byte(struct i2c_device *priv, union i2
 	int result;
 	int timeout;
 
-	dev_dbg(&priv->adapter.dev, "%s\n", __func__);
-
 	result = i801_check_pre(priv);
 	if (result < 0)
 		return result;
@@ -364,8 +352,6 @@ 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, "%s\n", __func__);
-
 	outb_p(inb_p(SMBAUXCTL(priv)) | SMBAUXCTL_E32B, SMBAUXCTL(priv));
 	if ((inb_p(SMBAUXCTL(priv)) & SMBAUXCTL_E32B) == 0)
 		return -EIO;
@@ -378,8 +364,6 @@ static int i801_block_transaction(struct i2c_device *priv, union i2c_smbus_data
 	int result = 0;
 	//unsigned char hostc;
 
-	dev_dbg(&priv->adapter.dev, "%s\n", __func__);
-
 	if (command == I2C_SMBUS_I2C_BLOCK_DATA) {
 		if (read_write == I2C_SMBUS_WRITE) {
 			/* set I2C_EN bit in configuration register */
@@ -427,10 +411,6 @@ 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,
-		"%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;
 
 	switch (size) {
@@ -605,9 +585,6 @@ int pi2c_probe(struct platform_device *pldev)
 	struct i2c_device *priv;
 	struct resource *res;
 
-	dev_dbg(&pldev->dev, "%s(pldev = %p '%s')\n", __func__, pldev,
-		pldev->name);
-
 	priv = devm_kzalloc(&pldev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
@@ -653,9 +630,6 @@ int pi2c_remove(struct platform_device *pldev)
 {
 	struct i2c_device *lddev;
 
-	dev_dbg(&pldev->dev, "%s(pldev = %p '%s')\n", __func__, pldev,
-		pldev->name);
-
 	lddev = (struct i2c_device *)pldev->dev.platform_data;
 
 	i2c_del_adapter(&lddev->adapter);
-- 
2.21.0

[PATCH 6/6] staging: kpc2000: kpc_i2c: add static qual to local symbols in kpc_i2c.c

[Geordan Neukum]

kpc_i2c.c declares:
  - two functions
    - pi2c_probe()
    - pi2c_remove()
  - one struct
    - i2c_plat_driver_i
which are local to the file, yet missing the static qualifier. Add the
static qualifier to these symbols.

Signed-off-by: Geordan Neukum <gneukum1@gmail.com>
---
 drivers/staging/kpc2000/kpc2000_i2c.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c b/drivers/staging/kpc2000/kpc2000_i2c.c
index f9259c06b605..97e738349ba2 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -579,7 +579,7 @@ static const struct i2c_algorithm smbus_algorithm = {
 /********************************
  *** Part 2 - Driver Handlers ***
  ********************************/
-int pi2c_probe(struct platform_device *pldev)
+static int pi2c_probe(struct platform_device *pldev)
 {
 	int err;
 	struct i2c_device *priv;
@@ -626,7 +626,7 @@ int pi2c_probe(struct platform_device *pldev)
 	return 0;
 }
 
-int pi2c_remove(struct platform_device *pldev)
+static int pi2c_remove(struct platform_device *pldev)
 {
 	struct i2c_device *lddev;
 
@@ -644,7 +644,7 @@ int pi2c_remove(struct platform_device *pldev)
 	return 0;
 }
 
-struct platform_driver i2c_plat_driver_i = {
+static struct platform_driver i2c_plat_driver_i = {
 	.probe      = pi2c_probe,
 	.remove     = pi2c_remove,
 	.driver     = {
-- 
2.21.0

Re: [PATCH 0/6] Minor updates to kpc_i2c driver and kpc2000 core

[Greg Kroah-Hartman]

On Wed, May 22, 2019 at 12:13:56PM +0000, Geordan Neukum wrote:
> Attached are an assortment of minor updates to the kpc_i2c driver as
> well as a build fix for all of those who will need the KPC2000 core.

Nit, please put "staging" in your 0/6 patch to make it easier for
scripts to pick this up properly.  For next time please.

thanks,

greg k-h