[PATCH 0/5] i2c: i2c-piix4: set of coding style fixes

[PATCH 0/5] i2c: i2c-piix4: set of coding style fixes

[Carlos Palminha]

This patchset fixes several coding style issues.

Carlos Palminha (5):
  i2c: i2c-piix4: coding style fix - '*' adjacent to data name
  i2c: i2c-piix4: coding style fix - assignment in if condition
  i2c: i2c-piix4: coding style fix - long comments blocks
  i2c: i2c-piix4: coding style fix - spaces with open parenthesis
  i2c: i2c-piix4: coding style fix - quoted string split across lines

 drivers/i2c/busses/i2c-piix4.c | 144 +++++++++++++++++++++++------------------
 1 file changed, 82 insertions(+), 62 deletions(-)

-- 
2.9.3

[PATCH 1/5] i2c: i2c-piix4: coding style fix - '*' adjacent to data name

[Carlos Palminha]

Signed-off-by: Carlos Palminha <palminha@synopsys.com>
---
 drivers/i2c/busses/i2c-piix4.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index c2268cd..6b55a8e 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -482,9 +482,9 @@ static int piix4_transaction(struct i2c_adapter *piix4_adapter)
 }
 
 /* Return negative errno on error. */
-static s32 piix4_access(struct i2c_adapter * adap, u16 addr,
+static s32 piix4_access(struct i2c_adapter *adap, u16 addr,
 		 unsigned short flags, char read_write,
-		 u8 command, int size, union i2c_smbus_data * data)
+		 u8 command, int size, union i2c_smbus_data *data)
 {
 	struct i2c_piix4_adapdata *adapdata = i2c_get_adapdata(adap);
 	unsigned short piix4_smba = adapdata->smba;
-- 
2.9.3

[PATCH 2/5] i2c: i2c-piix4: coding style fix - assignment in if condition

[Carlos Palminha]

Signed-off-by: Carlos Palminha <palminha@synopsys.com>
---
 drivers/i2c/busses/i2c-piix4.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 6b55a8e..5d6f637 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -419,11 +419,13 @@ static int piix4_transaction(struct i2c_adapter *piix4_adapter)
 		inb_p(SMBHSTDAT1));
 
 	/* Make sure the SMBus host is ready to start transmitting */
-	if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
+	temp = inb_p(SMBHSTSTS);
+	if (temp != 0x00) {
 		dev_dbg(&piix4_adapter->dev, "SMBus busy (%02x). "
 			"Resetting...\n", temp);
 		outb_p(temp, SMBHSTSTS);
-		if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
+		temp = inb_p(SMBHSTSTS);
+		if (temp != 0x00) {
 			dev_err(&piix4_adapter->dev, "Failed! (%02x)\n", temp);
 			return -EBUSY;
 		} else {
@@ -470,7 +472,8 @@ static int piix4_transaction(struct i2c_adapter *piix4_adapter)
 	if (inb_p(SMBHSTSTS) != 0x00)
 		outb_p(inb(SMBHSTSTS), SMBHSTSTS);
 
-	if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
+	temp = inb_p(SMBHSTSTS);
+	if (temp != 0x00) {
 		dev_err(&piix4_adapter->dev, "Failed reset at end of "
 			"transaction (%02x)\n", temp);
 	}
-- 
2.9.3

[PATCH 3/5] i2c: i2c-piix4: coding style fix - long comments blocks

[Carlos Palminha]

Signed-off-by: Carlos Palminha <palminha@synopsys.com>
---
 drivers/i2c/busses/i2c-piix4.c | 94 ++++++++++++++++++++++++------------------
 1 file changed, 54 insertions(+), 40 deletions(-)

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 5d6f637..85b1052 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -1,32 +1,32 @@
 /*
-    Copyright (c) 1998 - 2002 Frodo Looijaard <frodol@dds.nl> and
-    Philip Edelbrock <phil@netroedge.com>
-
-    This program is free software; you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation; either version 2 of the License, or
-    (at your option) any later version.
-
-    This program is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-*/
+ *  Copyright (c) 1998 - 2002 Frodo Looijaard <frodol@dds.nl> and
+ *  Philip Edelbrock <phil@netroedge.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ */
 
 /*
-   Supports:
-	Intel PIIX4, 440MX
-	Serverworks OSB4, CSB5, CSB6, HT-1000, HT-1100
-	ATI IXP200, IXP300, IXP400, SB600, SB700/SP5100, SB800
-	AMD Hudson-2, ML, CZ
-	SMSC Victory66
-
-   Note: we assume there can only be one device, with one or more
-   SMBus interfaces.
-   The device can register multiple i2c_adapters (up to PIIX4_MAX_ADAPTERS).
-   For devices supporting multiple ports the i2c_adapter should provide
-   an i2c_algorithm to access them.
-*/
+ * Supports:
+ *	Intel PIIX4, 440MX
+ *	Serverworks OSB4, CSB5, CSB6, HT-1000, HT-1100
+ *	ATI IXP200, IXP300, IXP400, SB600, SB700/SP5100, SB800
+ *	AMD Hudson-2, ML, CZ
+ *	SMSC Victory66
+ *
+ *  Note: we assume there can only be one device, with one or more
+ * SMBus interfaces.
+ * The device can register multiple i2c_adapters (up to PIIX4_MAX_ADAPTERS).
+ * For devices supporting multiple ports the i2c_adapter should provide
+ * an i2c_algorithm to access them.
+ */
 
 #include <linux/module.h>
 #include <linux/moduleparam.h>
@@ -97,14 +97,18 @@
 
 /* insmod parameters */
 
-/* If force is set to anything different from 0, we forcibly enable the
-   PIIX4. DANGEROUS! */
+/*
+ * If force is set to anything different from 0, we forcibly enable the
+ * PIIX4. DANGEROUS!
+ */
 static int force;
 module_param (force, int, 0);
 MODULE_PARM_DESC(force, "Forcibly enable the PIIX4. DANGEROUS!");
 
-/* If force_addr is set to anything different from 0, we forcibly enable
-   the PIIX4 at the given address. VERY DANGEROUS! */
+/*
+ * If force_addr is set to anything different from 0, we forcibly enable
+ * the PIIX4 at the given address. VERY DANGEROUS!
+ */
 static int force_addr;
 module_param (force_addr, int, 0);
 MODULE_PARM_DESC(force_addr,
@@ -132,8 +136,10 @@ static const struct dmi_system_id piix4_dmi_blacklist[] = {
 	{ }
 };
 
-/* The IBM entry is in a separate table because we only check it
-   on Intel-based systems */
+/*
+ * The IBM entry is in a separate table because we only check it
+ * on Intel-based systems
+ */
 static const struct dmi_system_id piix4_dmi_ibm[] = {
 	{
 		.ident = "IBM",
@@ -172,8 +178,10 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
 	    (PIIX4_dev->device == PCI_DEVICE_ID_SERVERWORKS_CSB5))
 		srvrworks_csb5_delay = 1;
 
-	/* On some motherboards, it was reported that accessing the SMBus
-	   caused severe hardware problems */
+	/*
+	 * On some motherboards, it was reported that accessing the SMBus
+	 * caused severe hardware problems
+	 */
 	if (dmi_check_system(piix4_dmi_blacklist)) {
 		dev_err(&PIIX4_dev->dev,
 			"Accessing the SMBus on this system is unsafe!\n");
@@ -215,8 +223,10 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
 
 	pci_read_config_byte(PIIX4_dev, SMBHSTCFG, &temp);
 
-	/* If force_addr is set, we program the new address here. Just to make
-	   sure, we disable the PIIX4 first. */
+	/*
+	 * If force_addr is set, we program the new address here. Just to make
+	 * sure, we disable the PIIX4 first.
+	 */
 	if (force_addr) {
 		pci_write_config_byte(PIIX4_dev, SMBHSTCFG, temp & 0xfe);
 		pci_write_config_word(PIIX4_dev, SMBBA, piix4_smba);
@@ -369,8 +379,10 @@ static int piix4_setup_aux(struct pci_dev *PIIX4_dev,
 			   const struct pci_device_id *id,
 			   unsigned short base_reg_addr)
 {
-	/* Set up auxiliary SMBus controllers found on some
-	 * AMD chipsets e.g. SP5100 (SB700 derivative) */
+	/*
+	 * Set up auxiliary SMBus controllers found on some
+	 * AMD chipsets e.g. SP5100 (SB700 derivative)
+	 */
 
 	unsigned short piix4_smba;
 
@@ -804,8 +816,10 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	}
 
 	if (retval > 0) {
-		/* Try to add the aux adapter if it exists,
-		 * piix4_add_adapter will clean up if this fails */
+		/*
+		 * Try to add the aux adapter if it exists,
+		 * piix4_add_adapter will clean up if this fails
+		 */
 		piix4_add_adapter(dev, retval, false, 0,
 				  is_sb800 ? piix4_aux_port_name_sb800 : "",
 				  &piix4_aux_adapter);
-- 
2.9.3

Re: [PATCH 3/5] i2c: i2c-piix4: coding style fix - long comments blocks

[Jean Delvare]

Hi Carlos,

On Tue, 11 Oct 2016 18:24:39 +0100, Carlos Palminha wrote:
> Signed-off-by: Carlos Palminha <palminha@synopsys.com>
> ---
>  drivers/i2c/busses/i2c-piix4.c | 94 ++++++++++++++++++++++++------------------
>  1 file changed, 54 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> index 5d6f637..85b1052 100644
> --- a/drivers/i2c/busses/i2c-piix4.c
> +++ b/drivers/i2c/busses/i2c-piix4.c
> (...)
>  /*
> -   Supports:
> -	Intel PIIX4, 440MX
> -	Serverworks OSB4, CSB5, CSB6, HT-1000, HT-1100
> -	ATI IXP200, IXP300, IXP400, SB600, SB700/SP5100, SB800
> -	AMD Hudson-2, ML, CZ
> -	SMSC Victory66
> -
> -   Note: we assume there can only be one device, with one or more
> -   SMBus interfaces.
> -   The device can register multiple i2c_adapters (up to PIIX4_MAX_ADAPTERS).
> -   For devices supporting multiple ports the i2c_adapter should provide
> -   an i2c_algorithm to access them.
> -*/
> + * Supports:
> + *	Intel PIIX4, 440MX
> + *	Serverworks OSB4, CSB5, CSB6, HT-1000, HT-1100
> + *	ATI IXP200, IXP300, IXP400, SB600, SB700/SP5100, SB800
> + *	AMD Hudson-2, ML, CZ
> + *	SMSC Victory66
> + *
> + *  Note: we assume there can only be one device, with one or more

There's an extra space before "Note:", please remove.

> + * SMBus interfaces.
> + * The device can register multiple i2c_adapters (up to PIIX4_MAX_ADAPTERS).
> + * For devices supporting multiple ports the i2c_adapter should provide
> + * an i2c_algorithm to access them.
> + */

-- 
Jean Delvare
SUSE L3 Support

[PATCH 4/5] i2c: i2c-piix4: coding style fix - spaces with open parenthesis

[Carlos Palminha]

Signed-off-by: Carlos Palminha <palminha@synopsys.com>
---
 drivers/i2c/busses/i2c-piix4.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 85b1052..5cc460a 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -102,7 +102,7 @@
  * PIIX4. DANGEROUS!
  */
 static int force;
-module_param (force, int, 0);
+module_param(force, int, 0);
 MODULE_PARM_DESC(force, "Forcibly enable the PIIX4. DANGEROUS!");
 
 /*
@@ -110,7 +110,7 @@ MODULE_PARM_DESC(force, "Forcibly enable the PIIX4. DANGEROUS!");
  * the PIIX4 at the given address. VERY DANGEROUS!
  */
 static int force_addr;
-module_param (force_addr, int, 0);
+module_param(force_addr, int, 0);
 MODULE_PARM_DESC(force_addr,
 		 "Forcibly enable the PIIX4 at the given address. "
 		 "EXTREMELY DANGEROUS!");
@@ -204,7 +204,7 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
 	} else {
 		pci_read_config_word(PIIX4_dev, SMBBA, &piix4_smba);
 		piix4_smba &= 0xfff0;
-		if(piix4_smba == 0) {
+		if (piix4_smba == 0) {
 			dev_err(&PIIX4_dev->dev, "SMBus base address "
 				"uninitialized - upgrade BIOS or use "
 				"force_addr=0xaddr\n");
@@ -664,7 +664,7 @@ static const struct pci_device_id piix4_ids[] = {
 	{ 0, }
 };
 
-MODULE_DEVICE_TABLE (pci, piix4_ids);
+MODULE_DEVICE_TABLE(pci, piix4_ids);
 
 static struct i2c_adapter *piix4_main_adapters[PIIX4_MAX_ADAPTERS];
 static struct i2c_adapter *piix4_aux_adapter;
-- 
2.9.3

[PATCH 5/5] i2c: i2c-piix4: coding style fix - quoted string split across lines

[Carlos Palminha]

Signed-off-by: Carlos Palminha <palminha@synopsys.com>
---
 drivers/i2c/busses/i2c-piix4.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 5cc460a..235a406 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -260,7 +260,8 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
 	else if ((temp & 0x0E) == 0)
 		dev_dbg(&PIIX4_dev->dev, "Using SMI# for SMBus\n");
 	else
-		dev_err(&PIIX4_dev->dev, "Illegal Interrupt configuration "
+		dev_err(&PIIX4_dev->dev,
+			"Illegal Interrupt configuration "
 			"(or code out of date)!\n");
 
 	pci_read_config_byte(PIIX4_dev, SMBREV, &temp);
@@ -280,8 +281,8 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 
 	/* SB800 and later SMBus does not support forcing address */
 	if (force || force_addr) {
-		dev_err(&PIIX4_dev->dev, "SMBus does not support "
-			"forcing address!\n");
+		dev_err(&PIIX4_dev->dev,
+			"SMBus does not support forcing address!\n");
 		return -EINVAL;
 	}
 
@@ -338,8 +339,9 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 
 	/* Request the SMBus I2C bus config region */
 	if (!request_region(piix4_smba + i2ccfg_offset, 1, "i2ccfg")) {
-		dev_err(&PIIX4_dev->dev, "SMBus I2C bus config region "
-			"0x%x already in use!\n", piix4_smba + i2ccfg_offset);
+		dev_err(&PIIX4_dev->dev,
+			"SMBus I2C bus config region 0x%x already in use!\n",
+			piix4_smba + i2ccfg_offset);
 		release_region(piix4_smba, SMBIOSIZE);
 		return -EBUSY;
 	}
@@ -405,8 +407,9 @@ static int piix4_setup_aux(struct pci_dev *PIIX4_dev,
 		return -ENODEV;
 
 	if (!request_region(piix4_smba, SMBIOSIZE, piix4_driver.name)) {
-		dev_err(&PIIX4_dev->dev, "Auxiliary SMBus region 0x%x "
-			"already in use!\n", piix4_smba);
+		dev_err(&PIIX4_dev->dev,
+			"Auxiliary SMBus region 0x%x already in use!\n",
+			piix4_smba);
 		return -EBUSY;
 	}
 
@@ -433,8 +436,8 @@ static int piix4_transaction(struct i2c_adapter *piix4_adapter)
 	/* Make sure the SMBus host is ready to start transmitting */
 	temp = inb_p(SMBHSTSTS);
 	if (temp != 0x00) {
-		dev_dbg(&piix4_adapter->dev, "SMBus busy (%02x). "
-			"Resetting...\n", temp);
+		dev_dbg(&piix4_adapter->dev,
+			"SMBus busy (%02x). Resetting...\n", temp);
 		outb_p(temp, SMBHSTSTS);
 		temp = inb_p(SMBHSTSTS);
 		if (temp != 0x00) {
@@ -486,8 +489,8 @@ static int piix4_transaction(struct i2c_adapter *piix4_adapter)
 
 	temp = inb_p(SMBHSTSTS);
 	if (temp != 0x00) {
-		dev_err(&piix4_adapter->dev, "Failed reset at end of "
-			"transaction (%02x)\n", temp);
+		dev_err(&piix4_adapter->dev,
+			"Failed reset at end of transaction (%02x)\n", temp);
 	}
 	dev_dbg(&piix4_adapter->dev, "Transaction (post): CNT=%02x, CMD=%02x, "
 		"ADD=%02x, DAT0=%02x, DAT1=%02x\n", inb_p(SMBHSTCNT),
@@ -870,7 +873,7 @@ static struct pci_driver piix4_driver = {
 
 module_pci_driver(piix4_driver);
 
-MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl> and "
-		"Philip Edelbrock <phil@netroedge.com>");
+MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl>");
+MODULE_AUTHOR("Philip Edelbrock <phil@netroedge.com>");
 MODULE_DESCRIPTION("PIIX4 SMBus driver");
 MODULE_LICENSE("GPL");
-- 
2.9.3

Re: [PATCH 5/5] i2c: i2c-piix4: coding style fix - quoted string split across lines

[Joe Perches]

On Tue, 2016-10-11 at 18:34 +0100, Carlos Palminha wrote:
[]
> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
[]
> @@ -260,7 +260,8 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
>  	else if ((temp & 0x0E) == 0)
>  		dev_dbg(&PIIX4_dev->dev, "Using SMI# for SMBus\n");
>  	else
> -		dev_err(&PIIX4_dev->dev, "Illegal Interrupt configuration "
> +		dev_err(&PIIX4_dev->dev,
> +			"Illegal Interrupt configuration "
>  			"(or code out of date)!\n");

typically, this format would be coalesced into a single line.

Re: [PATCH 5/5] i2c: i2c-piix4: coding style fix - quoted string split across lines

[Jean Delvare]

On Tue, 11 Oct 2016 19:03:56 -0700, Joe Perches wrote:
> On Tue, 2016-10-11 at 18:34 +0100, Carlos Palminha wrote:
> []
> > diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> []
> > @@ -260,7 +260,8 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
> >  	else if ((temp & 0x0E) == 0)
> >  		dev_dbg(&PIIX4_dev->dev, "Using SMI# for SMBus\n");
> >  	else
> > -		dev_err(&PIIX4_dev->dev, "Illegal Interrupt configuration "
> > +		dev_err(&PIIX4_dev->dev,
> > +			"Illegal Interrupt configuration "
> >  			"(or code out of date)!\n");
> 
> typically, this format would be coalesced into a single line.

Especially when the patch claims to "fix quoted string split across
lines" :-D

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH 0/5] i2c: i2c-piix4: set of coding style fixes

[Jean Delvare]

Hi Carlos,

On Tue, 11 Oct 2016 18:24:36 +0100, Carlos Palminha wrote:
> This patchset fixes several coding style issues.
> 
> Carlos Palminha (5):
>   i2c: i2c-piix4: coding style fix - '*' adjacent to data name
>   i2c: i2c-piix4: coding style fix - assignment in if condition
>   i2c: i2c-piix4: coding style fix - long comments blocks
>   i2c: i2c-piix4: coding style fix - spaces with open parenthesis
>   i2c: i2c-piix4: coding style fix - quoted string split across lines
> 
>  drivers/i2c/busses/i2c-piix4.c | 144 +++++++++++++++++++++++------------------
>  1 file changed, 82 insertions(+), 62 deletions(-)

Except for the two issues which were already pointed out, I'm fine with
these changes, however there is no good reason to split them into 5
separate patches. Fixing the coding style is one goal, it should be all
done with a single patch. So, when you resubmit, please do so as a
single patch.

Thanks,
-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH 0/5] i2c: i2c-piix4: set of coding style fixes

[Carlos Palminha]

Hi Jean,

On 13-10-2016 12:40, Jean Delvare wrote:
> Hi Carlos,
> 
> On Tue, 11 Oct 2016 18:24:36 +0100, Carlos Palminha wrote:
>> This patchset fixes several coding style issues.
>>
>> Carlos Palminha (5):
>>   i2c: i2c-piix4: coding style fix - '*' adjacent to data name
>>   i2c: i2c-piix4: coding style fix - assignment in if condition
>>   i2c: i2c-piix4: coding style fix - long comments blocks
>>   i2c: i2c-piix4: coding style fix - spaces with open parenthesis
>>   i2c: i2c-piix4: coding style fix - quoted string split across lines
>>
>>  drivers/i2c/busses/i2c-piix4.c | 144 +++++++++++++++++++++++------------------
>>  1 file changed, 82 insertions(+), 62 deletions(-)
> 
> Except for the two issues which were already pointed out, I'm fine with
> these changes, however there is no good reason to split them into 5
> separate patches. Fixing the coding style is one goal, it should be all
> done with a single patch. So, when you resubmit, please do so as a
> single patch.
> 
> Thanks,
> 

Sure... I'll resubmit as one patch.

Regards,
C.Palminha