[PATCH v2 0/5] gpio-tqmx86: cleanup + changing directions

[PATCH v2 0/5] gpio-tqmx86: cleanup + changing directions

[Matthias Schiffer]

This is the first of two series adding new features to the gpio-tqmx86
driver. The first 4 patches are cleanup/preparation and the last patch
adds support for changing the directions of GPIOs.

Once this is merged, the final series will add support for new TQMx86
variants (SMARC and COM-HPC) that feature up to 14 GPIOs and full IRQ
support on all lines.

# Changelog

v2:
- Improve wording of commit message (patch 1)
- Fix comment format (patch 1)
- Use lock guards

Matthias Schiffer (5):
  gpio: tqmx86: add macros for interrupt configuration
  gpio: tqmx86: consistently refer to IRQs by hwirq numbers
  gpio: tqmx86: use cleanup guards for spinlock
  gpio: tqmx86: introduce tqmx86_gpio_clrsetbits() helper
  gpio: tqmx86: add support for changing GPIO directions

 drivers/gpio/gpio-tqmx86.c | 206 +++++++++++++++++++++----------------
 1 file changed, 119 insertions(+), 87 deletions(-)

-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/

[PATCH v2 1/5] gpio: tqmx86: add macros for interrupt configuration

[Matthias Schiffer]

Consistently use TQMX86_INT_* flags for irq_type values. The
TQMX86_GPII_CONFIG macro is used to convert from TQMX86_INT_TRIG_*
flags to GPII register values. Bit patterns for TQMX86_INT_* are chosen
to make this conversion as simple as possible.

No functional change intended.

Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---

v2:
- improved wording of commit description
- fixed comment format

 drivers/gpio/gpio-tqmx86.c | 44 +++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c
index 5e26eb3adabbf..dda57fc02214b 100644
--- a/drivers/gpio/gpio-tqmx86.c
+++ b/drivers/gpio/gpio-tqmx86.c
@@ -29,18 +29,22 @@
 #define TQMX86_GPIIC	3	/* GPI Interrupt Configuration Register */
 #define TQMX86_GPIIS	4	/* GPI Interrupt Status Register */
 
-#define TQMX86_GPII_NONE	0
-#define TQMX86_GPII_FALLING	BIT(0)
-#define TQMX86_GPII_RISING	BIT(1)
-/* Stored in irq_type as a trigger type, but not actually valid as a register
- * value, so the name doesn't use "GPII"
+/*
+ * NONE, FALLING and RISING use the same bit patterns that can be programmed to
+ * the GPII register (after passing them to the TQMX86_GPII_ macros to shift
+ * them to the right position)
  */
-#define TQMX86_INT_BOTH		(BIT(0) | BIT(1))
-#define TQMX86_GPII_MASK	(BIT(0) | BIT(1))
-#define TQMX86_GPII_BITS	2
+#define TQMX86_INT_TRIG_NONE	0
+#define TQMX86_INT_TRIG_FALLING	BIT(0)
+#define TQMX86_INT_TRIG_RISING	BIT(1)
+#define TQMX86_INT_TRIG_BOTH	(BIT(0) | BIT(1))
+#define TQMX86_INT_TRIG_MASK	(BIT(0) | BIT(1))
 /* Stored in irq_type with GPII bits */
 #define TQMX86_INT_UNMASKED	BIT(2)
 
+#define TQMX86_GPIIC_CONFIG(i, v)	((v) << (2 * (i)))
+#define TQMX86_GPIIC_MASK(i)		TQMX86_GPIIC_CONFIG(i, TQMX86_INT_TRIG_MASK)
+
 struct tqmx86_gpio_data {
 	struct gpio_chip	chip;
 	void __iomem		*io_base;
@@ -115,20 +119,20 @@ static int tqmx86_gpio_get_direction(struct gpio_chip *chip,
 static void tqmx86_gpio_irq_config(struct tqmx86_gpio_data *gpio, int offset)
 	__must_hold(&gpio->spinlock)
 {
-	u8 type = TQMX86_GPII_NONE, gpiic;
+	u8 type = TQMX86_INT_TRIG_NONE, gpiic;
 
 	if (gpio->irq_type[offset] & TQMX86_INT_UNMASKED) {
-		type = gpio->irq_type[offset] & TQMX86_GPII_MASK;
+		type = gpio->irq_type[offset] & TQMX86_INT_TRIG_MASK;
 
-		if (type == TQMX86_INT_BOTH)
+		if (type == TQMX86_INT_TRIG_BOTH)
 			type = tqmx86_gpio_get(&gpio->chip, offset + TQMX86_NGPO)
-				? TQMX86_GPII_FALLING
-				: TQMX86_GPII_RISING;
+				? TQMX86_INT_TRIG_FALLING
+				: TQMX86_INT_TRIG_RISING;
 	}
 
 	gpiic = tqmx86_gpio_read(gpio, TQMX86_GPIIC);
-	gpiic &= ~(TQMX86_GPII_MASK << (offset * TQMX86_GPII_BITS));
-	gpiic |= type << (offset * TQMX86_GPII_BITS);
+	gpiic &= ~TQMX86_GPIIC_MASK(offset);
+	gpiic |= TQMX86_GPIIC_CONFIG(offset, type);
 	tqmx86_gpio_write(gpio, gpiic, TQMX86_GPIIC);
 }
 
@@ -173,20 +177,20 @@ static int tqmx86_gpio_irq_set_type(struct irq_data *data, unsigned int type)
 
 	switch (edge_type) {
 	case IRQ_TYPE_EDGE_RISING:
-		new_type = TQMX86_GPII_RISING;
+		new_type = TQMX86_INT_TRIG_RISING;
 		break;
 	case IRQ_TYPE_EDGE_FALLING:
-		new_type = TQMX86_GPII_FALLING;
+		new_type = TQMX86_INT_TRIG_FALLING;
 		break;
 	case IRQ_TYPE_EDGE_BOTH:
-		new_type = TQMX86_INT_BOTH;
+		new_type = TQMX86_INT_TRIG_BOTH;
 		break;
 	default:
 		return -EINVAL; /* not supported */
 	}
 
 	raw_spin_lock_irqsave(&gpio->spinlock, flags);
-	gpio->irq_type[offset] &= ~TQMX86_GPII_MASK;
+	gpio->irq_type[offset] &= ~TQMX86_INT_TRIG_MASK;
 	gpio->irq_type[offset] |= new_type;
 	tqmx86_gpio_irq_config(gpio, offset);
 	raw_spin_unlock_irqrestore(&gpio->spinlock, flags);
@@ -232,7 +236,7 @@ static void tqmx86_gpio_irq_handler(struct irq_desc *desc)
 		 * reading the input and setting the trigger, we will have a new
 		 * interrupt pending.
 		 */
-		if ((gpio->irq_type[i] & TQMX86_GPII_MASK) == TQMX86_INT_BOTH)
+		if ((gpio->irq_type[i] & TQMX86_INT_TRIG_MASK) == TQMX86_INT_TRIG_BOTH)
 			tqmx86_gpio_irq_config(gpio, i);
 	}
 	raw_spin_unlock_irqrestore(&gpio->spinlock, flags);
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/

[PATCH v2 2/5] gpio: tqmx86: consistently refer to IRQs by hwirq numbers

[Matthias Schiffer]

On currently supported variants of the TQMx86 GPIO controller, only
GPIOs 4-7 have IRQ support; in the interrupt status and config
registers, position 0 therefore corresponds to GPIO 4, position 1 to
GPIO 5, etc. This was made even more confusing by sometimes using the
term "offset" to refer to GPIO numbers (which are equavalent to hwirq
numbers), and sometimes to bit positions in the hardware registers.

With this change, the whole driver consistently uses hwirq numbers (==
GPIO numbers) when referring to the IRQs, and only the two pieces of
code that interact with the hardware registers (tqmx86_gpio_irq_config()
and tqmx86_gpio_irq_handler()) deal with bit positions. Space for hwirq
numbers 0-3 is reserved in the irq_type array, but remains unused for
existing (COM Express) TQMx86 variants; support for TQMx86 variants that
support IRQs on all GPIO lines will be added in the future.

No functional change intended.

Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---

v2: no changes

 drivers/gpio/gpio-tqmx86.c | 40 +++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c
index dda57fc02214b..38208a7dc1e62 100644
--- a/drivers/gpio/gpio-tqmx86.c
+++ b/drivers/gpio/gpio-tqmx86.c
@@ -52,7 +52,7 @@ struct tqmx86_gpio_data {
 	/* Lock must be held for accessing output and irq_type fields */
 	raw_spinlock_t		spinlock;
 	DECLARE_BITMAP(output, TQMX86_NGPIO);
-	u8			irq_type[TQMX86_NGPI];
+	u8			irq_type[TQMX86_NGPIO];
 };
 
 static u8 tqmx86_gpio_read(struct tqmx86_gpio_data *gd, unsigned int reg)
@@ -116,36 +116,36 @@ static int tqmx86_gpio_get_direction(struct gpio_chip *chip,
 	return GPIO_LINE_DIRECTION_OUT;
 }
 
-static void tqmx86_gpio_irq_config(struct tqmx86_gpio_data *gpio, int offset)
+static void tqmx86_gpio_irq_config(struct tqmx86_gpio_data *gpio, int hwirq)
 	__must_hold(&gpio->spinlock)
 {
 	u8 type = TQMX86_INT_TRIG_NONE, gpiic;
+	int gpiic_irq = hwirq - TQMX86_NGPO;
 
-	if (gpio->irq_type[offset] & TQMX86_INT_UNMASKED) {
-		type = gpio->irq_type[offset] & TQMX86_INT_TRIG_MASK;
+	if (gpio->irq_type[hwirq] & TQMX86_INT_UNMASKED) {
+		type = gpio->irq_type[hwirq] & TQMX86_INT_TRIG_MASK;
 
 		if (type == TQMX86_INT_TRIG_BOTH)
-			type = tqmx86_gpio_get(&gpio->chip, offset + TQMX86_NGPO)
+			type = tqmx86_gpio_get(&gpio->chip, hwirq)
 				? TQMX86_INT_TRIG_FALLING
 				: TQMX86_INT_TRIG_RISING;
 	}
 
 	gpiic = tqmx86_gpio_read(gpio, TQMX86_GPIIC);
-	gpiic &= ~TQMX86_GPIIC_MASK(offset);
-	gpiic |= TQMX86_GPIIC_CONFIG(offset, type);
+	gpiic &= ~TQMX86_GPIIC_MASK(gpiic_irq);
+	gpiic |= TQMX86_GPIIC_CONFIG(gpiic_irq, type);
 	tqmx86_gpio_write(gpio, gpiic, TQMX86_GPIIC);
 }
 
 static void tqmx86_gpio_irq_mask(struct irq_data *data)
 {
-	unsigned int offset = (data->hwirq - TQMX86_NGPO);
 	struct tqmx86_gpio_data *gpio = gpiochip_get_data(
 		irq_data_get_irq_chip_data(data));
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&gpio->spinlock, flags);
-	gpio->irq_type[offset] &= ~TQMX86_INT_UNMASKED;
-	tqmx86_gpio_irq_config(gpio, offset);
+	gpio->irq_type[data->hwirq] &= ~TQMX86_INT_UNMASKED;
+	tqmx86_gpio_irq_config(gpio, data->hwirq);
 	raw_spin_unlock_irqrestore(&gpio->spinlock, flags);
 
 	gpiochip_disable_irq(&gpio->chip, irqd_to_hwirq(data));
@@ -153,7 +153,6 @@ static void tqmx86_gpio_irq_mask(struct irq_data *data)
 
 static void tqmx86_gpio_irq_unmask(struct irq_data *data)
 {
-	unsigned int offset = (data->hwirq - TQMX86_NGPO);
 	struct tqmx86_gpio_data *gpio = gpiochip_get_data(
 		irq_data_get_irq_chip_data(data));
 	unsigned long flags;
@@ -161,8 +160,8 @@ static void tqmx86_gpio_irq_unmask(struct irq_data *data)
 	gpiochip_enable_irq(&gpio->chip, irqd_to_hwirq(data));
 
 	raw_spin_lock_irqsave(&gpio->spinlock, flags);
-	gpio->irq_type[offset] |= TQMX86_INT_UNMASKED;
-	tqmx86_gpio_irq_config(gpio, offset);
+	gpio->irq_type[data->hwirq] |= TQMX86_INT_UNMASKED;
+	tqmx86_gpio_irq_config(gpio, data->hwirq);
 	raw_spin_unlock_irqrestore(&gpio->spinlock, flags);
 }
 
@@ -170,7 +169,6 @@ static int tqmx86_gpio_irq_set_type(struct irq_data *data, unsigned int type)
 {
 	struct tqmx86_gpio_data *gpio = gpiochip_get_data(
 		irq_data_get_irq_chip_data(data));
-	unsigned int offset = (data->hwirq - TQMX86_NGPO);
 	unsigned int edge_type = type & IRQF_TRIGGER_MASK;
 	unsigned long flags;
 	u8 new_type;
@@ -190,9 +188,9 @@ static int tqmx86_gpio_irq_set_type(struct irq_data *data, unsigned int type)
 	}
 
 	raw_spin_lock_irqsave(&gpio->spinlock, flags);
-	gpio->irq_type[offset] &= ~TQMX86_INT_TRIG_MASK;
-	gpio->irq_type[offset] |= new_type;
-	tqmx86_gpio_irq_config(gpio, offset);
+	gpio->irq_type[data->hwirq] &= ~TQMX86_INT_TRIG_MASK;
+	gpio->irq_type[data->hwirq] |= new_type;
+	tqmx86_gpio_irq_config(gpio, data->hwirq);
 	raw_spin_unlock_irqrestore(&gpio->spinlock, flags);
 
 	return 0;
@@ -204,7 +202,7 @@ static void tqmx86_gpio_irq_handler(struct irq_desc *desc)
 	struct tqmx86_gpio_data *gpio = gpiochip_get_data(chip);
 	struct irq_chip *irq_chip = irq_desc_get_chip(desc);
 	unsigned long irq_bits, flags;
-	int i;
+	int i, hwirq;
 	u8 irq_status;
 
 	chained_irq_enter(irq_chip, desc);
@@ -216,6 +214,8 @@ static void tqmx86_gpio_irq_handler(struct irq_desc *desc)
 
 	raw_spin_lock_irqsave(&gpio->spinlock, flags);
 	for_each_set_bit(i, &irq_bits, TQMX86_NGPI) {
+		hwirq = i + TQMX86_NGPO;
+
 		/*
 		 * Edge-both triggers are implemented by flipping the edge
 		 * trigger after each interrupt, as the controller only supports
@@ -236,8 +236,8 @@ static void tqmx86_gpio_irq_handler(struct irq_desc *desc)
 		 * reading the input and setting the trigger, we will have a new
 		 * interrupt pending.
 		 */
-		if ((gpio->irq_type[i] & TQMX86_INT_TRIG_MASK) == TQMX86_INT_TRIG_BOTH)
-			tqmx86_gpio_irq_config(gpio, i);
+		if ((gpio->irq_type[hwirq] & TQMX86_INT_TRIG_MASK) == TQMX86_INT_TRIG_BOTH)
+			tqmx86_gpio_irq_config(gpio, hwirq);
 	}
 	raw_spin_unlock_irqrestore(&gpio->spinlock, flags);
 
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/

[PATCH v2 3/5] gpio: tqmx86: use cleanup guards for spinlock

[Matthias Schiffer]

As we're touching this code anyways, go all the way and fully replace
lock/unlock with guard and scoped_guard.

No functional change intended.

Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---

v2: new patch

 drivers/gpio/gpio-tqmx86.c | 84 +++++++++++++++++++-------------------
 1 file changed, 42 insertions(+), 42 deletions(-)

diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c
index 38208a7dc1e62..e55ce4503e70b 100644
--- a/drivers/gpio/gpio-tqmx86.c
+++ b/drivers/gpio/gpio-tqmx86.c
@@ -77,12 +77,11 @@ static void tqmx86_gpio_set(struct gpio_chip *chip, unsigned int offset,
 			    int value)
 {
 	struct tqmx86_gpio_data *gpio = gpiochip_get_data(chip);
-	unsigned long flags;
 
-	raw_spin_lock_irqsave(&gpio->spinlock, flags);
+	guard(raw_spinlock_irqsave)(&gpio->spinlock);
+
 	__assign_bit(offset, gpio->output, value);
 	tqmx86_gpio_write(gpio, bitmap_get_value8(gpio->output, 0), TQMX86_GPIOD);
-	raw_spin_unlock_irqrestore(&gpio->spinlock, flags);
 }
 
 static int tqmx86_gpio_direction_input(struct gpio_chip *chip,
@@ -141,12 +140,11 @@ static void tqmx86_gpio_irq_mask(struct irq_data *data)
 {
 	struct tqmx86_gpio_data *gpio = gpiochip_get_data(
 		irq_data_get_irq_chip_data(data));
-	unsigned long flags;
 
-	raw_spin_lock_irqsave(&gpio->spinlock, flags);
-	gpio->irq_type[data->hwirq] &= ~TQMX86_INT_UNMASKED;
-	tqmx86_gpio_irq_config(gpio, data->hwirq);
-	raw_spin_unlock_irqrestore(&gpio->spinlock, flags);
+	scoped_guard(raw_spinlock_irqsave, &gpio->spinlock) {
+		gpio->irq_type[data->hwirq] &= ~TQMX86_INT_UNMASKED;
+		tqmx86_gpio_irq_config(gpio, data->hwirq);
+	}
 
 	gpiochip_disable_irq(&gpio->chip, irqd_to_hwirq(data));
 }
@@ -155,14 +153,13 @@ static void tqmx86_gpio_irq_unmask(struct irq_data *data)
 {
 	struct tqmx86_gpio_data *gpio = gpiochip_get_data(
 		irq_data_get_irq_chip_data(data));
-	unsigned long flags;
 
 	gpiochip_enable_irq(&gpio->chip, irqd_to_hwirq(data));
 
-	raw_spin_lock_irqsave(&gpio->spinlock, flags);
+	guard(raw_spinlock_irqsave)(&gpio->spinlock);
+
 	gpio->irq_type[data->hwirq] |= TQMX86_INT_UNMASKED;
 	tqmx86_gpio_irq_config(gpio, data->hwirq);
-	raw_spin_unlock_irqrestore(&gpio->spinlock, flags);
 }
 
 static int tqmx86_gpio_irq_set_type(struct irq_data *data, unsigned int type)
@@ -170,7 +167,6 @@ static int tqmx86_gpio_irq_set_type(struct irq_data *data, unsigned int type)
 	struct tqmx86_gpio_data *gpio = gpiochip_get_data(
 		irq_data_get_irq_chip_data(data));
 	unsigned int edge_type = type & IRQF_TRIGGER_MASK;
-	unsigned long flags;
 	u8 new_type;
 
 	switch (edge_type) {
@@ -187,11 +183,11 @@ static int tqmx86_gpio_irq_set_type(struct irq_data *data, unsigned int type)
 		return -EINVAL; /* not supported */
 	}
 
-	raw_spin_lock_irqsave(&gpio->spinlock, flags);
+	guard(raw_spinlock_irqsave)(&gpio->spinlock);
+
 	gpio->irq_type[data->hwirq] &= ~TQMX86_INT_TRIG_MASK;
 	gpio->irq_type[data->hwirq] |= new_type;
 	tqmx86_gpio_irq_config(gpio, data->hwirq);
-	raw_spin_unlock_irqrestore(&gpio->spinlock, flags);
 
 	return 0;
 }
@@ -201,7 +197,7 @@ static void tqmx86_gpio_irq_handler(struct irq_desc *desc)
 	struct gpio_chip *chip = irq_desc_get_handler_data(desc);
 	struct tqmx86_gpio_data *gpio = gpiochip_get_data(chip);
 	struct irq_chip *irq_chip = irq_desc_get_chip(desc);
-	unsigned long irq_bits, flags;
+	unsigned long irq_bits;
 	int i, hwirq;
 	u8 irq_status;
 
@@ -212,34 +208,38 @@ static void tqmx86_gpio_irq_handler(struct irq_desc *desc)
 
 	irq_bits = irq_status;
 
-	raw_spin_lock_irqsave(&gpio->spinlock, flags);
-	for_each_set_bit(i, &irq_bits, TQMX86_NGPI) {
-		hwirq = i + TQMX86_NGPO;
-
-		/*
-		 * Edge-both triggers are implemented by flipping the edge
-		 * trigger after each interrupt, as the controller only supports
-		 * either rising or falling edge triggers, but not both.
-		 *
-		 * Internally, the TQMx86 GPIO controller has separate status
-		 * registers for rising and falling edge interrupts. GPIIC
-		 * configures which bits from which register are visible in the
-		 * interrupt status register GPIIS and defines what triggers the
-		 * parent IRQ line. Writing to GPIIS always clears both rising
-		 * and falling interrupt flags internally, regardless of the
-		 * currently configured trigger.
-		 *
-		 * In consequence, we can cleanly implement the edge-both
-		 * trigger in software by first clearing the interrupt and then
-		 * setting the new trigger based on the current GPIO input in
-		 * tqmx86_gpio_irq_config() - even if an edge arrives between
-		 * reading the input and setting the trigger, we will have a new
-		 * interrupt pending.
-		 */
-		if ((gpio->irq_type[hwirq] & TQMX86_INT_TRIG_MASK) == TQMX86_INT_TRIG_BOTH)
-			tqmx86_gpio_irq_config(gpio, hwirq);
+	scoped_guard(raw_spinlock_irqsave, &gpio->spinlock) {
+		for_each_set_bit(i, &irq_bits, TQMX86_NGPI) {
+			hwirq = i + TQMX86_NGPO;
+
+			/*
+			 * Edge-both triggers are implemented by flipping the
+			 * edge trigger after each interrupt, as the controller
+			 * only supports either rising or falling edge triggers,
+			 * but not both.
+			 *
+			 * Internally, the TQMx86 GPIO controller has separate
+			 * status registers for rising and falling edge
+			 * interrupts. GPIIC configures which bits from which
+			 * register are visible in the interrupt status register
+			 * GPIIS and defines what triggers the parent IRQ line.
+			 * Writing to GPIIS always clears both rising and
+			 * falling interrupt flags internally, regardless of the
+			 * currently configured trigger.
+			 *
+			 * In consequence, we can cleanly implement the
+			 * edge-both trigger in software by first clearing the
+			 * interrupt and then setting the new trigger based on
+			 * the current GPIO input in tqmx86_gpio_irq_config() -
+			 * even if an edge arrives between reading the input and
+			 * setting the trigger, we will have a new interrupt
+			 * pending.
+			 */
+			if ((gpio->irq_type[hwirq] & TQMX86_INT_TRIG_MASK) ==
+			    TQMX86_INT_TRIG_BOTH)
+				tqmx86_gpio_irq_config(gpio, hwirq);
+		}
 	}
-	raw_spin_unlock_irqrestore(&gpio->spinlock, flags);
 
 	for_each_set_bit(i, &irq_bits, TQMX86_NGPI)
 		generic_handle_domain_irq(gpio->chip.irq.domain,
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/

[PATCH v2 4/5] gpio: tqmx86: introduce tqmx86_gpio_clrsetbits() helper

[Matthias Schiffer]

Add a helper for the common read-modify-write pattern (only used in
tqmx86_gpio_irq_config() initially).

No functional change intended.

Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---

v2: no changes

 drivers/gpio/gpio-tqmx86.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c
index e55ce4503e70b..4bef13cad1807 100644
--- a/drivers/gpio/gpio-tqmx86.c
+++ b/drivers/gpio/gpio-tqmx86.c
@@ -66,6 +66,18 @@ static void tqmx86_gpio_write(struct tqmx86_gpio_data *gd, u8 val,
 	iowrite8(val, gd->io_base + reg);
 }
 
+static void tqmx86_gpio_clrsetbits(struct tqmx86_gpio_data *gpio,
+				   u8 clr, u8 set, unsigned int reg)
+	__must_hold(&gpio->spinlock)
+{
+	u8 val = tqmx86_gpio_read(gpio, reg);
+
+	val &= ~clr;
+	val |= set;
+
+	tqmx86_gpio_write(gpio, val, reg);
+}
+
 static int tqmx86_gpio_get(struct gpio_chip *chip, unsigned int offset)
 {
 	struct tqmx86_gpio_data *gpio = gpiochip_get_data(chip);
@@ -118,7 +130,7 @@ static int tqmx86_gpio_get_direction(struct gpio_chip *chip,
 static void tqmx86_gpio_irq_config(struct tqmx86_gpio_data *gpio, int hwirq)
 	__must_hold(&gpio->spinlock)
 {
-	u8 type = TQMX86_INT_TRIG_NONE, gpiic;
+	u8 type = TQMX86_INT_TRIG_NONE;
 	int gpiic_irq = hwirq - TQMX86_NGPO;
 
 	if (gpio->irq_type[hwirq] & TQMX86_INT_UNMASKED) {
@@ -130,10 +142,10 @@ static void tqmx86_gpio_irq_config(struct tqmx86_gpio_data *gpio, int hwirq)
 				: TQMX86_INT_TRIG_RISING;
 	}
 
-	gpiic = tqmx86_gpio_read(gpio, TQMX86_GPIIC);
-	gpiic &= ~TQMX86_GPIIC_MASK(gpiic_irq);
-	gpiic |= TQMX86_GPIIC_CONFIG(gpiic_irq, type);
-	tqmx86_gpio_write(gpio, gpiic, TQMX86_GPIIC);
+	tqmx86_gpio_clrsetbits(gpio,
+			       TQMX86_GPIIC_MASK(gpiic_irq),
+			       TQMX86_GPIIC_CONFIG(gpiic_irq, type),
+			       TQMX86_GPIIC);
 }
 
 static void tqmx86_gpio_irq_mask(struct irq_data *data)
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/

[PATCH v2 5/5] gpio: tqmx86: add support for changing GPIO directions

[Matthias Schiffer]

Only GPIOs 4..7 have IRQ support on the TQMx86 variants currently handled
by the driver, but apart from that, changing directions works fine. The
default directions are left unchanged (0..3 output, 4..7 input) to match
the COM Express specification.

A tqmx86_gpio_set() variant without locking is introduced as a new
helper.

Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---

v2: use lock guards

 drivers/gpio/gpio-tqmx86.c | 44 ++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c
index 4bef13cad1807..18f523a15b3c0 100644
--- a/drivers/gpio/gpio-tqmx86.c
+++ b/drivers/gpio/gpio-tqmx86.c
@@ -85,6 +85,14 @@ static int tqmx86_gpio_get(struct gpio_chip *chip, unsigned int offset)
 	return !!(tqmx86_gpio_read(gpio, TQMX86_GPIOD) & BIT(offset));
 }
 
+static void _tqmx86_gpio_set(struct tqmx86_gpio_data *gpio, unsigned int offset,
+			     int value)
+	__must_hold(&gpio->spinlock)
+{
+	__assign_bit(offset, gpio->output, value);
+	tqmx86_gpio_write(gpio, bitmap_get_value8(gpio->output, 0), TQMX86_GPIOD);
+}
+
 static void tqmx86_gpio_set(struct gpio_chip *chip, unsigned int offset,
 			    int value)
 {
@@ -92,39 +100,47 @@ static void tqmx86_gpio_set(struct gpio_chip *chip, unsigned int offset,
 
 	guard(raw_spinlock_irqsave)(&gpio->spinlock);
 
-	__assign_bit(offset, gpio->output, value);
-	tqmx86_gpio_write(gpio, bitmap_get_value8(gpio->output, 0), TQMX86_GPIOD);
+	_tqmx86_gpio_set(gpio, offset, value);
 }
 
 static int tqmx86_gpio_direction_input(struct gpio_chip *chip,
 				       unsigned int offset)
 {
-	/* Direction cannot be changed. Validate is an input. */
-	if (BIT(offset) & TQMX86_DIR_INPUT_MASK)
-		return 0;
-	else
-		return -EINVAL;
+	struct tqmx86_gpio_data *gpio = gpiochip_get_data(chip);
+
+	guard(raw_spinlock_irqsave)(&gpio->spinlock);
+
+	tqmx86_gpio_clrsetbits(gpio, BIT(offset), 0, TQMX86_GPIODD);
+
+	return 0;
 }
 
 static int tqmx86_gpio_direction_output(struct gpio_chip *chip,
 					unsigned int offset,
 					int value)
 {
-	/* Direction cannot be changed, validate is an output */
-	if (BIT(offset) & TQMX86_DIR_INPUT_MASK)
-		return -EINVAL;
+	struct tqmx86_gpio_data *gpio = gpiochip_get_data(chip);
+
+	guard(raw_spinlock_irqsave)(&gpio->spinlock);
+
+	_tqmx86_gpio_set(gpio, offset, value);
+	tqmx86_gpio_clrsetbits(gpio, 0, BIT(offset), TQMX86_GPIODD);
 
-	tqmx86_gpio_set(chip, offset, value);
 	return 0;
 }
 
 static int tqmx86_gpio_get_direction(struct gpio_chip *chip,
 				     unsigned int offset)
 {
-	if (TQMX86_DIR_INPUT_MASK & BIT(offset))
-		return GPIO_LINE_DIRECTION_IN;
+	struct tqmx86_gpio_data *gpio = gpiochip_get_data(chip);
+	u8 val;
+
+	val = tqmx86_gpio_read(gpio, TQMX86_GPIODD);
+
+	if (val & BIT(offset))
+		return GPIO_LINE_DIRECTION_OUT;
 
-	return GPIO_LINE_DIRECTION_OUT;
+	return GPIO_LINE_DIRECTION_IN;
 }
 
 static void tqmx86_gpio_irq_config(struct tqmx86_gpio_data *gpio, int hwirq)
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/

Re: [PATCH v2 0/5] gpio-tqmx86: cleanup + changing directions

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>


On Thu, 12 Dec 2024 15:08:04 +0100, Matthias Schiffer wrote:
> This is the first of two series adding new features to the gpio-tqmx86
> driver. The first 4 patches are cleanup/preparation and the last patch
> adds support for changing the directions of GPIOs.
> 
> Once this is merged, the final series will add support for new TQMx86
> variants (SMARC and COM-HPC) that feature up to 14 GPIOs and full IRQ
> support on all lines.
> 
> [...]

Applied, thanks!

[1/5] gpio: tqmx86: add macros for interrupt configuration
      commit: 2a485c83787723671b7ad215e4e141315e46b311
[2/5] gpio: tqmx86: consistently refer to IRQs by hwirq numbers
      commit: 0ccf314304ed5b83df7470a8ed0fe1b6ed48fc03
[3/5] gpio: tqmx86: use cleanup guards for spinlock
      commit: 2abb6e53b5b08987265946b258ca29762091930c
[4/5] gpio: tqmx86: introduce tqmx86_gpio_clrsetbits() helper
      commit: a1389f5c128e80c8ad3132bbdc7b5061f3710b7f
[5/5] gpio: tqmx86: add support for changing GPIO directions
      commit: 2251fbd05f2357927fa5c5a8dd955f84da883008

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>