From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vipin Kumar Date: Fri, 21 Jun 2013 10:10:45 +0530 Subject: [U-Boot] [PATCH] gpio: spear_gpio: Fix gpio_set_value() implementation In-Reply-To: References: <1371712419.4200.1.camel@phoenix> <201306201544.30751.marex@denx.de> Message-ID: <51C3D94D.1030507@st.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 6/20/2013 7:26 PM, Axel Lin wrote: > 2013/6/20 Marek Vasut >> >> Dear Axel Lin, >> >>> In current gpio_set_value() implementation, it always sets the gpio control >>> bit no matter the value argument is 0 or 1. Thus the GPIOs never set to >>> low. This patch fixes this bug. >>> >>> Signed-off-by: Axel Lin >>> --- >>> drivers/gpio/spear_gpio.c | 5 ++++- >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpio/spear_gpio.c b/drivers/gpio/spear_gpio.c >>> index d3c728e..8878608 100644 >>> --- a/drivers/gpio/spear_gpio.c >>> +++ b/drivers/gpio/spear_gpio.c >>> @@ -52,7 +52,10 @@ int gpio_set_value(unsigned gpio, int value) >>> { >>> struct gpio_regs *regs = (struct gpio_regs *)CONFIG_GPIO_BASE; >>> >>> - writel(1<< gpio,®s->gpiodata[DATA_REG_ADDR(gpio)]); >>> + if (value) >>> + writel(1<< gpio,®s->gpiodata[DATA_REG_ADDR(gpio)]); >>> + else >>> + writel(0,®s->gpiodata[DATA_REG_ADDR(gpio)]); >> >> How can this possibly work? Writing 0 to the whole bank will unset all the >> GPIOs, no ? > > > Because each GPIO is controlled by a register. > And only one bit will be set when set gpio to high. > > So it's safe to write 0 for clearing the bit. > > Note, the gpio_get_value() implementation also assumes there is only one bit > will be set. ( If this is not true, both gpio_get_value() and gpio_set_value() > need fix.) > > Vipin, can you review this patch and confirm this behavior? > Yes this is right. and the code is fine Regards Vipin > Thanks, > Axel > . >