From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Trimarchi Date: Fri, 21 Jun 2013 08:42:50 +0200 Subject: [U-Boot] [PATCH] gpio: spear_gpio: Fix gpio_set_value() implementation In-Reply-To: <51C3D94D.1030507@st.com> References: <1371712419.4200.1.camel@phoenix> <201306201544.30751.marex@denx.de> <51C3D94D.1030507@st.com> Message-ID: <51C3F5EA.2050506@amarulasolutions.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 06/21/2013 06:40 AM, Vipin Kumar wrote: > 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 > The problem is not in set one bit but in reset one bit. Can you check the else path? Michael > Regards > Vipin > >> Thanks, >> Axel >> . >> > > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot