diff mbox

[v4,5/5] gpio: pca953x: coding style fixes

Message ID 1473259770-26224-6-git-send-email-bgolaszewski@baylibre.com
State Superseded
Headers show

Commit Message

Bartosz Golaszewski Sept. 7, 2016, 2:49 p.m. UTC
pca953x_gpio_set_multiple() has some coding style issues that make it
harder to read. Tweak the code a bit.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

---
 drivers/gpio/gpio-pca953x.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Bartosz Golaszewski Sept. 7, 2016, 3:19 p.m. UTC | #1
2016-09-07 17:17 GMT+02:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> On Wed, 2016-09-07 at 16:49 +0200, Bartosz Golaszewski wrote:

>> pca953x_gpio_set_multiple() has some coding style issues that make it

>> harder to read. Tweak the code a bit.

>>

>

> Usually give one day for reviewers to have a chance to follow your

> changes.

>


Sure, I re-posted immediately because the changes were minor.

Thanks,
Bartosz

> And one comment below.

>

>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

>> ---

>>  drivers/gpio/gpio-pca953x.c | 22 ++++++++++++----------

>>  1 file changed, 12 insertions(+), 10 deletions(-)

>>

>> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c

>> index b08ed52..079c311 100644

>> --- a/drivers/gpio/gpio-pca953x.c

>> +++ b/drivers/gpio/gpio-pca953x.c

>> @@ -360,25 +360,27 @@ exit:

>>  }

>>

>>  static void pca953x_gpio_set_multiple(struct gpio_chip *gc,

>> -             unsigned long *mask, unsigned long *bits)

>> +                                   unsigned long *mask, unsigned

>> long *bits)

>>  {

>>       struct pca953x_chip *chip = gpiochip_get_data(gc);

>> +     unsigned int bankmask, bankval;

>> +     int bank_shift, bank, ret;

>>       u8 reg_val[MAX_BANK];

>> -     int ret;

>> -     int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);

>> -     int bank;

>

> I meant to keep int ret apart.

>

> struct pca953x_chip *chip = gpiochip_get_data(gc);

> unsigned int bank_mask, bank_val, bank_shift, bank;

> u8 reg_val[MAX_BANK];

> int ret;

>

> And perhaps _ in the names of bank* to be consistent.

>

>

> --

> Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Intel Finland Oy

> --

> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in

> the body of a message to majordomo@vger.kernel.org

> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index b08ed52..079c311 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -360,25 +360,27 @@  exit:
 }
 
 static void pca953x_gpio_set_multiple(struct gpio_chip *gc,
-		unsigned long *mask, unsigned long *bits)
+				      unsigned long *mask, unsigned long *bits)
 {
 	struct pca953x_chip *chip = gpiochip_get_data(gc);
+	unsigned int bankmask, bankval;
+	int bank_shift, bank, ret;
 	u8 reg_val[MAX_BANK];
-	int ret;
-	int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
-	int bank;
+
+	bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
 
 	memcpy(reg_val, chip->reg_output, NBANK(chip));
 	mutex_lock(&chip->i2c_lock);
-	for(bank=0; bank<NBANK(chip); bank++) {
-		unsigned bankmask = mask[bank / sizeof(*mask)] >>
-				    ((bank % sizeof(*mask)) * 8);
-		if(bankmask) {
-			unsigned bankval  = bits[bank / sizeof(*bits)] >>
-					    ((bank % sizeof(*bits)) * 8);
+	for (bank = 0; bank < NBANK(chip); bank++) {
+		bankmask = mask[bank / sizeof(*mask)] >>
+			   ((bank % sizeof(*mask)) * 8);
+		if (bankmask) {
+			bankval = bits[bank / sizeof(*bits)] >>
+				  ((bank % sizeof(*bits)) * 8);
 			reg_val[bank] = (reg_val[bank] & ~bankmask) | bankval;
 		}
 	}
+
 	ret = i2c_smbus_write_i2c_block_data(chip->client,
 					     chip->offset->output << bank_shift,
 					     NBANK(chip), reg_val);