gpio: Fix bit masking in Kontron PLD GPIO driver

PATCH In review Michael Brunner 4 participants 7 messages Aug 9, 2013 – Aug 16, 2013 GPIO

Share & tools

Michael Brunner

author

2013-08-09 15:33 UTC #0 lore raw

PATCH 1/1 gpio: Fix bit masking in Kontron PLD GPIO driver

This patch fixes the bit masking within the GPIO driver. The masking is basically done twice which causes the wrong GPIOs to be addressed.

Signed-off-by Michael Brunner <[email protected]>

---
 drivers/gpio/gpio-kempld.c |   24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

drivers/gpio/gpio-kempld.c +9 ~~-15~~

index d3ed563..6118b66 100644
@@ -46,9 +46,9 @@ static void kempld_gpio_bitop(struct kempld_device_data *pld,
 
        status = kempld_read8(pld, reg);
        if (val)
-               status |= (1 << bit);
+               status |= KEMPLD_GPIO_MASK(bit);
        else
-               status &= ~(1 << bit);
+               status &= ~KEMPLD_GPIO_MASK(bit);
        kempld_write8(pld, reg, status);
 }

@@ -60,7 +60,7 @@ static int kempld_gpio_get_bit(struct kempld_device_data *pld, u8 reg, u8 bit)
        status = kempld_read8(pld, reg);
        kempld_release_mutex(pld);
 
-       return !!(status & (1 << bit));
+       return !!(status & KEMPLD_GPIO_MASK(bit));
 }
 
 static int kempld_gpio_get(struct gpio_chip *chip, unsigned offset)

@@ -69,8 +69,7 @@ static int kempld_gpio_get(struct gpio_chip *chip, unsigned offset)
                = container_of(chip, struct kempld_gpio_data, chip);
        struct kempld_device_data *pld = gpio->pld;
 
-       return kempld_gpio_get_bit(pld, KEMPLD_GPIO_LVL_NUM(offset),
-                                  KEMPLD_GPIO_MASK(offset));
+       return kempld_gpio_get_bit(pld, KEMPLD_GPIO_LVL_NUM(offset), offset);
 }
 
 static void kempld_gpio_set(struct gpio_chip *chip, unsigned offset, int value)

@@ -80,8 +79,7 @@ static void kempld_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
        struct kempld_device_data *pld = gpio->pld;
 
        kempld_get_mutex(pld);
-       kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset),
-                         KEMPLD_GPIO_MASK(offset), value);
+       kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset), offset, value);
        kempld_release_mutex(pld);
 }

@@ -92,8 +90,7 @@ static int kempld_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
        struct kempld_device_data *pld = gpio->pld;
 
        kempld_get_mutex(pld);
-       kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset),
-                         KEMPLD_GPIO_MASK(offset), 0);
+       kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset), offset, 0);
        kempld_release_mutex(pld);
 
        return 0;

@@ -107,10 +104,8 @@ static int kempld_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
        struct kempld_device_data *pld = gpio->pld;
 
        kempld_get_mutex(pld);
-       kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset),
-                         KEMPLD_GPIO_MASK(offset), value);
-       kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset),
-                         KEMPLD_GPIO_MASK(offset), 1);
+       kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset), offset, value);
+       kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset), offset, 1);
        kempld_release_mutex(pld);
 
        return 0;

@@ -122,8 +117,7 @@ static int kempld_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
                = container_of(chip, struct kempld_gpio_data, chip);
        struct kempld_device_data *pld = gpio->pld;
 
-       return kempld_gpio_get_bit(pld, KEMPLD_GPIO_DIR_NUM(offset),
-                                  KEMPLD_GPIO_MASK(offset));
+       return kempld_gpio_get_bit(pld, KEMPLD_GPIO_DIR_NUM(offset), offset);
 }
 
 static int kempld_gpio_pincount(struct kempld_device_data *pld)

Guenter Roeck

Reviewed-by on v1 1/1

2013-08-09 19:25 UTC #1 lore raw

Quoted 4 lines · Michael Brunner

This patch fixes the bit masking within the GPIO driver. The masking is basically done twice which causes the wrong GPIOs to be addressed.
Signed-off-by Michael Brunner <[email protected]>

Reviewed-by Guenter Roeck <[email protected]>

Quoted 83 lines

---
 drivers/gpio/gpio-kempld.c |   24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

drivers/gpio/gpio-kempld.c +9 ~~-15~~

index d3ed563..6118b66 100644
@@ -46,9 +46,9 @@ static void kempld_gpio_bitop(struct kempld_device_data *pld,
 
        status = kempld_read8(pld, reg);
        if (val)
-               status |= (1 << bit);
+               status |= KEMPLD_GPIO_MASK(bit);
        else
-               status &= ~(1 << bit);
+               status &= ~KEMPLD_GPIO_MASK(bit);
        kempld_write8(pld, reg, status);
 }

@@ -60,7 +60,7 @@ static int kempld_gpio_get_bit(struct kempld_device_data *pld, u8 reg, u8 bit)
        status = kempld_read8(pld, reg);
        kempld_release_mutex(pld);
 
-       return !!(status & (1 << bit));
+       return !!(status & KEMPLD_GPIO_MASK(bit));
 }
 
 static int kempld_gpio_get(struct gpio_chip *chip, unsigned offset)

@@ -69,8 +69,7 @@ static int kempld_gpio_get(struct gpio_chip *chip, unsigned offset)
                = container_of(chip, struct kempld_gpio_data, chip);
        struct kempld_device_data *pld = gpio->pld;
 
-       return kempld_gpio_get_bit(pld, KEMPLD_GPIO_LVL_NUM(offset),
-                                  KEMPLD_GPIO_MASK(offset));
+       return kempld_gpio_get_bit(pld, KEMPLD_GPIO_LVL_NUM(offset), offset);
 }
 
 static void kempld_gpio_set(struct gpio_chip *chip, unsigned offset, int value)

@@ -80,8 +79,7 @@ static void kempld_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
        struct kempld_device_data *pld = gpio->pld;
 
        kempld_get_mutex(pld);
-       kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset),
-                         KEMPLD_GPIO_MASK(offset), value);
+       kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset), offset, value);
        kempld_release_mutex(pld);
 }

@@ -92,8 +90,7 @@ static int kempld_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
        struct kempld_device_data *pld = gpio->pld;
 
        kempld_get_mutex(pld);
-       kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset),
-                         KEMPLD_GPIO_MASK(offset), 0);
+       kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset), offset, 0);
        kempld_release_mutex(pld);
 
        return 0;

@@ -107,10 +104,8 @@ static int kempld_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
        struct kempld_device_data *pld = gpio->pld;
 
        kempld_get_mutex(pld);
-       kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset),
-                         KEMPLD_GPIO_MASK(offset), value);
-       kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset),
-                         KEMPLD_GPIO_MASK(offset), 1);
+       kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset), offset, value);
+       kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset), offset, 1);
        kempld_release_mutex(pld);
 
        return 0;

@@ -122,8 +117,7 @@ static int kempld_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
                = container_of(chip, struct kempld_gpio_data, chip);
        struct kempld_device_data *pld = gpio->pld;
 
-       return kempld_gpio_get_bit(pld, KEMPLD_GPIO_DIR_NUM(offset),
-                                  KEMPLD_GPIO_MASK(offset));
+       return kempld_gpio_get_bit(pld, KEMPLD_GPIO_DIR_NUM(offset), offset);
 }
 
 static int kempld_gpio_pincount(struct kempld_device_data *pld)

Kevin Strasser

Acked-by on v1 1/1

2013-08-10 22:12 UTC #2 lore raw

In reply to Michael Brunner

Quoted 4 lines · Michael Brunner

This patch fixes the bit masking within the GPIO driver. The masking is basically done twice which causes the wrong GPIOs to be addressed.
Signed-off-by Michael Brunner <[email protected]>

Acked-by Kevin Strasser <[email protected]>

Quoted 82 lines

---
 drivers/gpio/gpio-kempld.c |   24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

drivers/gpio/gpio-kempld.c +9 ~~-15~~

index d3ed563..6118b66 100644
@@ -46,9 +46,9 @@ static void kempld_gpio_bitop(struct kempld_device_data *pld,
 
        status = kempld_read8(pld, reg);
        if (val)
-               status |= (1 << bit);
+               status |= KEMPLD_GPIO_MASK(bit);
        else
-               status &= ~(1 << bit);
+               status &= ~KEMPLD_GPIO_MASK(bit);
        kempld_write8(pld, reg, status);
 }

@@ -60,7 +60,7 @@ static int kempld_gpio_get_bit(struct kempld_device_data *pld, u8 reg, u8 bit)
        status = kempld_read8(pld, reg);
        kempld_release_mutex(pld);
 
-       return !!(status & (1 << bit));
+       return !!(status & KEMPLD_GPIO_MASK(bit));
 }
 
 static int kempld_gpio_get(struct gpio_chip *chip, unsigned offset)

@@ -69,8 +69,7 @@ static int kempld_gpio_get(struct gpio_chip *chip, unsigned offset)
                = container_of(chip, struct kempld_gpio_data, chip);
        struct kempld_device_data *pld = gpio->pld;
 
-       return kempld_gpio_get_bit(pld, KEMPLD_GPIO_LVL_NUM(offset),
-                                  KEMPLD_GPIO_MASK(offset));
+       return kempld_gpio_get_bit(pld, KEMPLD_GPIO_LVL_NUM(offset), offset);
 }
 
 static void kempld_gpio_set(struct gpio_chip *chip, unsigned offset, int value)

@@ -80,8 +79,7 @@ static void kempld_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
        struct kempld_device_data *pld = gpio->pld;
 
        kempld_get_mutex(pld);
-       kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset),
-                         KEMPLD_GPIO_MASK(offset), value);
+       kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset), offset, value);
        kempld_release_mutex(pld);
 }

@@ -92,8 +90,7 @@ static int kempld_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
        struct kempld_device_data *pld = gpio->pld;
 
        kempld_get_mutex(pld);
-       kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset),
-                         KEMPLD_GPIO_MASK(offset), 0);
+       kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset), offset, 0);
        kempld_release_mutex(pld);
 
        return 0;

@@ -107,10 +104,8 @@ static int kempld_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
        struct kempld_device_data *pld = gpio->pld;
 
        kempld_get_mutex(pld);
-       kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset),
-                         KEMPLD_GPIO_MASK(offset), value);
-       kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset),
-                         KEMPLD_GPIO_MASK(offset), 1);
+       kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset), offset, value);
+       kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset), offset, 1);
        kempld_release_mutex(pld);
 
        return 0;

@@ -122,8 +117,7 @@ static int kempld_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
                = container_of(chip, struct kempld_gpio_data, chip);
        struct kempld_device_data *pld = gpio->pld;
 
-       return kempld_gpio_get_bit(pld, KEMPLD_GPIO_DIR_NUM(offset),
-                                  KEMPLD_GPIO_MASK(offset));
+       return kempld_gpio_get_bit(pld, KEMPLD_GPIO_DIR_NUM(offset), offset);
 }
 
 static int kempld_gpio_pincount(struct kempld_device_data *pld)

Linus Walleij

2013-08-16 15:09 UTC #3 lore raw

In reply to Michael Brunner

Quoted 4 lines · Michael Brunner

This patch fixes the bit masking within the GPIO driver. The masking is basically done twice which causes the wrong GPIOs to be addressed.
Signed-off-by Michael Brunner <[email protected]>

This looks good but just doesn't apply. Look:

git am --signoff brunner2.patch
Tillämpar: gpio: Fix bit masking in Kontron PLD GPIO driver
error: patch misslyckades: drivers/gpio/gpio-kempld.c:46
error: drivers/gpio/gpio-kempld.c: patchen kan inte tillämpas

Then:
patch -p1 < brunner2.patch
patching file drivers/gpio/gpio-kempld.c
Hunk #1 FAILED at 46.
Hunk #2 FAILED at 60.
Hunk #3 FAILED at 69.
Hunk #4 FAILED at 80.
Hunk #5 FAILED at 92.
Hunk #6 FAILED at 107.
Hunk #7 FAILED at 122.
7 out of 7 hunks FAILED -- saving rejects to file drivers/gpio/gpio-kempld.c.rej

I think there may be some whitespace mangling in your client or mail server or whatever.

I had the same problem with the previous patch but hand-coded it right since it was just a oneline.

Please try to figure out what is causing this.
Consult:
Documentation/email-clients.txt

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Guenter Roeck

2013-08-16 15:52 UTC #4 lore raw

Quoted 29 lines · Linus Walleij

Quoted 4 lines · Michael Brunner
This patch fixes the bit masking within the GPIO driver. The masking is basically done twice which causes the wrong GPIOs to be addressed.
Signed-off-by Michael Brunner <[email protected]>
This looks good but just doesn't apply. Look:
git am --signoff brunner2.patch
Tillämpar: gpio: Fix bit masking in Kontron PLD GPIO driver
error: patch misslyckades: drivers/gpio/gpio-kempld.c:46
error: drivers/gpio/gpio-kempld.c: patchen kan inte tillämpas
Then:
patch -p1 < brunner2.patch
patching file drivers/gpio/gpio-kempld.c
Hunk #1 FAILED at 46.
Hunk #2 FAILED at 60.
Hunk #3 FAILED at 69.
Hunk #4 FAILED at 80.
Hunk #5 FAILED at 92.
Hunk #6 FAILED at 107.
Hunk #7 FAILED at 122.
7 out of 7 hunks FAILED -- saving rejects to file drivers/gpio/gpio-kempld.c.rej
I think there may be some whitespace mangling in your client or mail server or whatever.

Quoted 7 lines

I had the same problem with the previous patch but hand-coded it right since it was just a oneline.
Please try to figure out what is causing this.
Consult:
Documentation/email-clients.txt

I think Michael just went on vacation.

Clean version is below. He must have sent it to me separately because it worked for me.

Guenter

---
From: Brunner Michael <[email protected]>
Date: Wed, 31 Jul 2013 20:55:39 +0200
Subject: [PATCH] gpio: Fix bit masking in Kontron PLD GPIO driver

This patch fixes the bit masking within the GPIO driver. The masking is basically done twice which causes the wrong GPIOs to be addressed.

Signed-off-by Michael Brunner <[email protected]>

Signed-off-by Guenter Roeck <[email protected]>

---
 drivers/gpio/gpio-kempld.c |   24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

drivers/gpio/gpio-kempld.c +9 ~~-15~~

index d3ed563..6118b66 100644
@@ -46,9 +46,9 @@ static void kempld_gpio_bitop(struct kempld_device_data *pld,
 
 	status = kempld_read8(pld, reg);
 	if (val)
-		status |= (1 << bit);
+		status |= KEMPLD_GPIO_MASK(bit);
 	else
-		status &= ~(1 << bit);
+		status &= ~KEMPLD_GPIO_MASK(bit);
 	kempld_write8(pld, reg, status);
 }

@@ -60,7 +60,7 @@ static int kempld_gpio_get_bit(struct kempld_device_data *pld, u8 reg, u8 bit)
 	status = kempld_read8(pld, reg);
 	kempld_release_mutex(pld);
 
-	return !!(status & (1 << bit));
+	return !!(status & KEMPLD_GPIO_MASK(bit));
 }
 
 static int kempld_gpio_get(struct gpio_chip *chip, unsigned offset)

@@ -69,8 +69,7 @@ static int kempld_gpio_get(struct gpio_chip *chip, unsigned offset)
 		= container_of(chip, struct kempld_gpio_data, chip);
 	struct kempld_device_data *pld = gpio->pld;
 
-	return kempld_gpio_get_bit(pld, KEMPLD_GPIO_LVL_NUM(offset),
-				   KEMPLD_GPIO_MASK(offset));
+	return kempld_gpio_get_bit(pld, KEMPLD_GPIO_LVL_NUM(offset), offset);
 }
 
 static void kempld_gpio_set(struct gpio_chip *chip, unsigned offset, int value)

@@ -80,8 +79,7 @@ static void kempld_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 	struct kempld_device_data *pld = gpio->pld;
 
 	kempld_get_mutex(pld);
-	kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset),
-			  KEMPLD_GPIO_MASK(offset), value);
+	kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset), offset, value);
 	kempld_release_mutex(pld);
 }

@@ -92,8 +90,7 @@ static int kempld_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
 	struct kempld_device_data *pld = gpio->pld;
 
 	kempld_get_mutex(pld);
-	kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset),
-			  KEMPLD_GPIO_MASK(offset), 0);
+	kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset), offset, 0);
 	kempld_release_mutex(pld);
 
 	return 0;

@@ -107,10 +104,8 @@ static int kempld_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
 	struct kempld_device_data *pld = gpio->pld;
 
 	kempld_get_mutex(pld);
-	kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset),
-			  KEMPLD_GPIO_MASK(offset), value);
-	kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset),
-			  KEMPLD_GPIO_MASK(offset), 1);
+	kempld_gpio_bitop(pld, KEMPLD_GPIO_LVL_NUM(offset), offset, value);
+	kempld_gpio_bitop(pld, KEMPLD_GPIO_DIR_NUM(offset), offset, 1);
 	kempld_release_mutex(pld);
 
 	return 0;

@@ -122,8 +117,7 @@ static int kempld_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
 		= container_of(chip, struct kempld_gpio_data, chip);
 	struct kempld_device_data *pld = gpio->pld;
 
-	return kempld_gpio_get_bit(pld, KEMPLD_GPIO_DIR_NUM(offset),
-				   KEMPLD_GPIO_MASK(offset));
+	return kempld_gpio_get_bit(pld, KEMPLD_GPIO_DIR_NUM(offset), offset);
 }
 
 static int kempld_gpio_pincount(struct kempld_device_data *pld)

Signature · 6 lines

1.7.9.7

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

Linus Walleij

2013-08-16 15:58 UTC #5 lore raw

Quoted 4 lines · Guenter Roeck

I think Michael just went on vacation.
Clean version is below. He must have sent it to me separately because it worked for me.

Yeah this worked fine ... strange.

Patch applied.

Yours,
Linus Walleij

Guenter Roeck

2013-08-16 15:59 UTC #6 lore raw

Quoted 9 lines · Linus Walleij

Quoted 4 lines · Guenter Roeck
I think Michael just went on vacation.
Clean version is below. He must have sent it to me separately because it worked for me.
Yeah this worked fine ... strange.

Probably there is some outlook server in the path :(

Guenter