2024-05-29 07:53:20

by Matthias Schiffer

Subject: [PATCH 2/8] gpio: tqmx86: introduce shadow register for GPIO output value

The TQMx86 GPIO controller uses the same register address for input and
output data. Reading the register will always return current inputs
rather than the previously set outputs (regardless of the current
direction setting). Therefore, using a RMW pattern does not make sense
when setting output values. Instead, the previously set output register
value needs to be stored as a shadow register.

As there is no reliable way to get the current output values from the
hardware, also initialize all channels to 0, to ensure that stored and
actual output values match. This should usually not have any effect in
practise, as the TQMx86 UEFI sets all outputs to 0 during boot.

Also prepare for extension of the driver to more than 8 GPIOs by using
DECLARE_BITMAP.

Fixes: b868db94a6a7 ("gpio: tqmx86: Add GPIO from for this IO controller")
Signed-off-by: Matthias Schiffer <[email protected]>
---
drivers/gpio/gpio-tqmx86.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c
index 3a28c1f273c39..b7e2dbbdc4ebe 100644
--- a/drivers/gpio/gpio-tqmx86.c
+++ b/drivers/gpio/gpio-tqmx86.c
@@ -6,6 +6,7 @@
* Vadim V.Vlasov <[email protected]>
*/

+#include <linux/bitmap.h>
#include <linux/bitops.h>
#include <linux/errno.h>
#include <linux/gpio/driver.h>
@@ -38,6 +39,7 @@ struct tqmx86_gpio_data {
void __iomem *io_base;
int irq;
raw_spinlock_t spinlock;
+ DECLARE_BITMAP(output, TQMX86_NGPIO);
u8 irq_type[TQMX86_NGPI];
};

@@ -64,15 +66,10 @@ static void tqmx86_gpio_set(struct gpio_chip *chip, unsigned int offset,
{
struct tqmx86_gpio_data *gpio = gpiochip_get_data(chip);
unsigned long flags;
- u8 val;

raw_spin_lock_irqsave(&gpio->spinlock, flags);
- val = tqmx86_gpio_read(gpio, TQMX86_GPIOD);
- if (value)
- val |= BIT(offset);
- else
- val &= ~BIT(offset);
- tqmx86_gpio_write(gpio, val, TQMX86_GPIOD);
+ __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);
}

@@ -277,6 +274,13 @@ static int tqmx86_gpio_probe(struct platform_device *pdev)

tqmx86_gpio_write(gpio, (u8)~TQMX86_DIR_INPUT_MASK, TQMX86_GPIODD);

+ /*
+ * Reading the previous output state is not possible with TQMx86 hardware.
+ * Initialize all outputs to 0 to have a defined state that matches the
+ * shadow register.
+ */
+ tqmx86_gpio_write(gpio, 0, TQMX86_GPIOD);
+
chip = &gpio->chip;
chip->label = "gpio-tqmx86";
chip->owner = THIS_MODULE;
--
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/

2024-05-29 12:03:13

by Andrew Lunn

Subject: Re: [PATCH 2/8] gpio: tqmx86: introduce shadow register for GPIO output value

On Wed, May 29, 2024 at 09:45:14AM +0200, Matthias Schiffer wrote:
> The TQMx86 GPIO controller uses the same register address for input and
> output data. Reading the register will always return current inputs
> rather than the previously set outputs (regardless of the current
> direction setting). Therefore, using a RMW pattern does not make sense
> when setting output values. Instead, the previously set output register
> value needs to be stored as a shadow register.
>
> As there is no reliable way to get the current output values from the
> hardware, also initialize all channels to 0, to ensure that stored and
> actual output values match. This should usually not have any effect in
> practise, as the TQMx86 UEFI sets all outputs to 0 during boot.
>
> Also prepare for extension of the driver to more than 8 GPIOs by using
> DECLARE_BITMAP.
>
> Fixes: b868db94a6a7 ("gpio: tqmx86: Add GPIO from for this IO controller")
> Signed-off-by: Matthias Schiffer <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Andrew