From dd4cf53f9864362d9e92472298ad60dcbb8b7e72 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 28 Jan 2020 12:04:34 +0000 Subject: [PATCH] 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 --- drivers/gpio/mpc8xxx_gpio.c | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) 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; } -- 2.25.1