Skip to contentSkip to conversation

gpio: thunderx: Utilize for_each_set_clump macro

PATCH Superseded Syed Nayyar Waris 2 participants 3 messages Apr 24, 2020

Subsystems (2)
GPIO Thunderx GPIO driver

v1 of 12 Superseded Latest is v12 →

lore
Share & tools
lore

Syed Nayyar Waris

author
#0 lore raw

PATCH 5/6 gpio: thunderx: Utilize for_each_set_clump macro

This patch reimplements the thunderx_gpio_set_multiple function in drivers/gpio/gpio-thunderx.c to use the new for_each_set_clump macro. Instead of looping for each bank in thunderx_gpio_set_multiple function, now we can skip bank which is not set and save cycles.

Cc Robert Richter <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Bartosz Golaszewski <[email protected]>
Signed-off-by Syed Nayyar Waris <[email protected]>
Signed-off-by William Breathitt Gray <[email protected]>
---
 drivers/gpio/gpio-thunderx.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/gpio/gpio-thunderx.c +8 -4
index 9f66dea..74aea25 100644
@@ -275,12 +275,16 @@ static void thunderx_gpio_set_multiple(struct gpio_chip *chip,
 				       unsigned long *bits)
 {
 	int bank;
-	u64 set_bits, clear_bits;
+	u64 set_bits, clear_bits, gpio_mask;
+	const unsigned long bank_size = 64;
+	unsigned long offset;
+
 	struct thunderx_gpio *txgpio = gpiochip_get_data(chip);
 
-	for (bank = 0; bank <= chip->ngpio / 64; bank++) {
-		set_bits = bits[bank] & mask[bank];
-		clear_bits = ~bits[bank] & mask[bank];
+	for_each_set_clump(offset, gpio_mask, mask, chip->ngpio, bank_size) {
+		bank = offset / bank_size;
+		set_bits = bits[bank] & gpio_mask;
+		clear_bits = ~bits[bank] & gpio_mask;
 		writeq(set_bits, txgpio->register_base + (bank * GPIO_2ND_BANK) + GPIO_TX_SET);
 		writeq(clear_bits, txgpio->register_base + (bank * GPIO_2ND_BANK) + GPIO_TX_CLR);
 	}
Signature · 1 line
2.7.4
#1 lore raw
Quoted 41 lines · Syed Nayyar Waris

This patch reimplements the thunderx_gpio_set_multiple function in drivers/gpio/gpio-thunderx.c to use the new for_each_set_clump macro. Instead of looping for each bank in thunderx_gpio_set_multiple function, now we can skip bank which is not set and save cycles.

Cc Robert Richter <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Bartosz Golaszewski <[email protected]>
Signed-off-by Syed Nayyar Waris <[email protected]>
Signed-off-by William Breathitt Gray <[email protected]>
---
 drivers/gpio/gpio-thunderx.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/gpio/gpio-thunderx.c +8 -4
index 9f66dea..74aea25 100644
@@ -275,12 +275,16 @@ static void thunderx_gpio_set_multiple(struct gpio_chip *chip,
 				       unsigned long *bits)
 {
 	int bank;
-	u64 set_bits, clear_bits;
+	u64 set_bits, clear_bits, gpio_mask;
+	const unsigned long bank_size = 64;
+	unsigned long offset;
+
 	struct thunderx_gpio *txgpio = gpiochip_get_data(chip);
 
-	for (bank = 0; bank <= chip->ngpio / 64; bank++) {
-		set_bits = bits[bank] & mask[bank];
-		clear_bits = ~bits[bank] & mask[bank];
+	for_each_set_clump(offset, gpio_mask, mask, chip->ngpio, bank_size) {
+		bank = offset / bank_size;
+		set_bits = bits[bank] & gpio_mask;
+		clear_bits = ~bits[bank] & gpio_mask;
 		writeq(set_bits, txgpio->register_base + (bank * GPIO_2ND_BANK) + GPIO_TX_SET);
 		writeq(clear_bits, txgpio->register_base + (bank * GPIO_2ND_BANK) + GPIO_TX_CLR);
 	}
Signature · 1 line
2.7.4

We noticed in the original code that this set_multiple callback does not appear to work correctly on systems where BITS_PER_LONG == 32. On those systems, the bits and mask values are 32-bit, but the for loop jumps 64 bits at a time -- that means the loop is skipping the upper 32 gpio lines of every iteration.

Is the gpio-thunderx driver only intended for 64-bit systems? Or this behavior a bug?

William Breathitt Gray

#2 lore raw
Quoted 49 lines · William Breathitt Gray
Quoted 41 lines · Syed Nayyar Waris

This patch reimplements the thunderx_gpio_set_multiple function in drivers/gpio/gpio-thunderx.c to use the new for_each_set_clump macro. Instead of looping for each bank in thunderx_gpio_set_multiple function, now we can skip bank which is not set and save cycles.

Cc Robert Richter <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Bartosz Golaszewski <[email protected]>
Signed-off-by Syed Nayyar Waris <[email protected]>
Signed-off-by William Breathitt Gray <[email protected]>
---
 drivers/gpio/gpio-thunderx.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/gpio/gpio-thunderx.c +8 -4
index 9f66dea..74aea25 100644
@@ -275,12 +275,16 @@ static void thunderx_gpio_set_multiple(struct gpio_chip *chip,
 				       unsigned long *bits)
 {
 	int bank;
-	u64 set_bits, clear_bits;
+	u64 set_bits, clear_bits, gpio_mask;
+	const unsigned long bank_size = 64;
+	unsigned long offset;
+
 	struct thunderx_gpio *txgpio = gpiochip_get_data(chip);
 
-	for (bank = 0; bank <= chip->ngpio / 64; bank++) {
-		set_bits = bits[bank] & mask[bank];
-		clear_bits = ~bits[bank] & mask[bank];
+	for_each_set_clump(offset, gpio_mask, mask, chip->ngpio, bank_size) {
+		bank = offset / bank_size;
+		set_bits = bits[bank] & gpio_mask;
+		clear_bits = ~bits[bank] & gpio_mask;
 		writeq(set_bits, txgpio->register_base + (bank * GPIO_2ND_BANK) + GPIO_TX_SET);
 		writeq(clear_bits, txgpio->register_base + (bank * GPIO_2ND_BANK) + GPIO_TX_CLR);
 	}
Signature · 1 line
2.7.4

We noticed in the original code that this set_multiple callback does not appear to work correctly on systems where BITS_PER_LONG == 32. On those systems, the bits and mask values are 32-bit, but the for loop jumps 64 bits at a time -- that means the loop is skipping the upper 32 gpio lines of every iteration.

Quoted 4 lines

Is the gpio-thunderx driver only intended for 64-bit systems? Or this behavior a bug?

William Breathitt Gray

Nevermind, I spoke too soon; I see there's a depends on 64BIT line in the Kconfig for GPIO_THUNDERX.

William Breathitt Gray