From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Date: Tue, 28 Jan 2020 12:04:34 +0000 (+0000)
Subject: gpio: mpc8xxx: don't do RMW on gpdat register when setting value
X-Git-Tag: v2020.04-rc5~14^2~6
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=dd4cf53f9864362d9e92472298ad60dcbb8b7e72;p=oweals%2Fu-boot.git

gpio: mpc8xxx: don't do RMW on gpdat register when setting value

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@prevas.dk>
---

diff --git a/drivers/gpio/mpc8xxx_gpio.c b/drivers/gpio/mpc8xxx_gpio.c
index d4f3092df0..4b385b8b39 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;
 }