Skip to contentSkip to conversation

gpio: tpic2810: Make sure cached buffer has consistent status with h/w status

PATCH In review Axel Lin 3 participants 3 messages Mar 23, 2016 – Mar 31, 2016 GPIO

lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH 1/1 gpio: tpic2810: Make sure cached buffer has consistent status with h/w status

i2c_smbus_write_byte_data() can fail. To ensure the cached buffer has consistent status with h/w status, don't update the cached gpio->buffer if write fails.
Also refactor the code a bit by adding a tpic2810_set_mask_bits() helper and use it to simplify the code.

Signed-off-by Axel Lin <[email protected]>
---
 drivers/gpio/gpio-tpic2810.c | 35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)
drivers/gpio/gpio-tpic2810.c +15 -20
index 9f020aa..cace79c 100644
@@ -57,39 +57,34 @@ static int tpic2810_direction_output(struct gpio_chip *chip,
 	return 0;
 }
 
-static void tpic2810_set(struct gpio_chip *chip, unsigned offset, int value)
+static void tpic2810_set_mask_bits(struct gpio_chip *chip, u8 mask, u8 bits)
 {
 	struct tpic2810 *gpio = gpiochip_get_data(chip);
+	u8 buffer;
+	int err;
 
 	mutex_lock(&gpio->lock);
 
-	if (value)
-		gpio->buffer |= BIT(offset);
-	else
-		gpio->buffer &= ~BIT(offset);
+	buffer = gpio->buffer & ~mask;
+	buffer |= (mask & bits);
 
-	i2c_smbus_write_byte_data(gpio->client, TPIC2810_WS_COMMAND,
-				  gpio->buffer);
+	err = i2c_smbus_write_byte_data(gpio->client, TPIC2810_WS_COMMAND,
+					buffer);
+	if (!err)
+		gpio->buffer = buffer;
 
 	mutex_unlock(&gpio->lock);
 }
 
+static void tpic2810_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+	tpic2810_set_mask_bits(chip, BIT(offset), value ? BIT(offset) : 0);
+}
+
 static void tpic2810_set_multiple(struct gpio_chip *chip, unsigned long *mask,
 				  unsigned long *bits)
 {
-	struct tpic2810 *gpio = gpiochip_get_data(chip);
-
-	mutex_lock(&gpio->lock);
-
-	/* clear bits under mask */
-	gpio->buffer &= ~(*mask);
-	/* set bits under mask */
-	gpio->buffer |= ((*mask) & (*bits));
-
-	i2c_smbus_write_byte_data(gpio->client, TPIC2810_WS_COMMAND,
-				  gpio->buffer);
-
-	mutex_unlock(&gpio->lock);
+	tpic2810_set_mask_bits(chip, *mask, *bits);
 }
 
 static struct gpio_chip template_chip = {
Signature · 1 line
2.1.4
Reviewed-by on v1 1/1
#1 lore raw
Quoted 5 lines · Axel Lin

i2c_smbus_write_byte_data() can fail. To ensure the cached buffer has consistent status with h/w status, don't update the cached gpio->buffer if write fails.
Also refactor the code a bit by adding a tpic2810_set_mask_bits() helper and use it to simplify the code.

Looks pretty good!

Reviewed-by Alexandre Courbot <[email protected]>
#2 lore raw
In reply to Axel Lin
Quoted 7 lines · Axel Lin

i2c_smbus_write_byte_data() can fail. To ensure the cached buffer has consistent status with h/w status, don't update the cached gpio->buffer if write fails.
Also refactor the code a bit by adding a tpic2810_set_mask_bits() helper and use it to simplify the code.

Signed-off-by Axel Lin <[email protected]>

Patch applied with Alexandre's review tag.

Yours,
Linus Walleij