diff mbox series

[2/2] gpio: mpc8xxx: don't do RMW on gpdat register when setting value

Message ID 20200128111020.15136-3-rasmus.villemoes@prevas.dk
State Superseded
Headers show
Series gpio: mpc8xxx: honour shadow register when writing gpdat | expand

Commit Message

Rasmus Villemoes Jan. 28, 2020, 11:10 a.m. UTC
The driver correctly handles reading back the value of an output gpio
by reading from the shadow register for output, and from gpdat for
inputs.

Unfortunately, when setting the value of some gpio, we do a RMW cycle
on the gpdat register without taking the shadow register into account,
thus accidentally setting other output gpios (at least those whose
value cannot be read back) to 0 at the same time.

When changing a gpio from input to output, we still need to make sure
it initially has the requested value. So, the procedure is

- update the shadow register
- compute the new gpdir register
- write the bitwise and of the shadow and new gpdir register to gpdat
- write the new gpdir register

Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
---
 drivers/gpio/mpc8xxx_gpio.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)
diff mbox series

Patch

diff --git a/drivers/gpio/mpc8xxx_gpio.c b/drivers/gpio/mpc8xxx_gpio.c
index 5438196fe0..c5be8e673b 100644
--- a/drivers/gpio/mpc8xxx_gpio.c
+++ b/drivers/gpio/mpc8xxx_gpio.c
@@ -57,20 +57,6 @@  static inline u32 mpc8xxx_gpio_get_dir(struct ccsr_gpio *base, u32 mask)
 	return in_be32(&base->gpdir) & mask;
 }
 
-static inline void mpc8xxx_gpio_set_low(struct ccsr_gpio *base, u32 gpios)
-{
-	clrbits_be32(&base->gpdat, gpios);
-	/* GPDIR register 1 -> output */
-	setbits_be32(&base->gpdir, gpios);
-}
-
-static inline void mpc8xxx_gpio_set_high(struct ccsr_gpio *base, u32 gpios)
-{
-	setbits_be32(&base->gpdat, gpios);
-	/* GPDIR register 1 -> output */
-	setbits_be32(&base->gpdir, gpios);
-}
-
 static inline int mpc8xxx_gpio_open_drain_val(struct ccsr_gpio *base, u32 mask)
 {
 	return in_be32(&base->gpodr) & mask;
@@ -104,14 +90,21 @@  static int mpc8xxx_gpio_direction_input(struct udevice *dev, uint gpio)
 static int mpc8xxx_gpio_set_value(struct udevice *dev, uint gpio, int value)
 {
 	struct mpc8xxx_gpio_data *data = dev_get_priv(dev);
+	struct ccsr_gpio *base = data->base;
+	u32 mask = gpio_mask(gpio);
+	u32 gpdir;
 
 	if (value) {
-		data->dat_shadow |= gpio_mask(gpio);
-		mpc8xxx_gpio_set_high(data->base, gpio_mask(gpio));
+		data->dat_shadow |= mask;
 	} else {
-		data->dat_shadow &= ~gpio_mask(gpio);
-		mpc8xxx_gpio_set_low(data->base, gpio_mask(gpio));
+		data->dat_shadow &= ~mask;
 	}
+
+	gpdir = in_be32(&base->gpdir);
+	gpdir |= gpio_mask(gpio);
+	out_be32(&base->gpdat, gpdir & data->dat_shadow);
+	out_be32(&base->gpdir, gpdir);
+
 	return 0;
 }