Is gpio_get_value() working correctly?

[EdSutter]

I'm working with u-boot-2013.10 with an iMX6D based board and just noticed something odd about the gpio_get_value() function in drivers/gpio/mxc_gpio.c (or maybe its just that I don't fully understand the difference between the PSR and DR GPIO control registers)...

I have a couple of LEDs on my board connected to GPIO pins.  I just wrote a test function to blink those LEDs.  I first retrieve the current state using gpio_get_value(), then I change the state for the test using gpio_set_value()  and finally I restore the state back to each pin that I originally read.  Turns out that didn't work (it doesn't  restore the state).  Looking closer at the function, I see that it retrieves the bit from the PSR (pad status register); but it appears that this register should only be used on GPIO bits that are programmed as inputs.  When I changed this function to use the DR (data register), then it properly returned the status of the GPIO bit I was interested in.

So, it seems to me that the gpio_get_value() function should actually use DR for requests of bits that are programmed as

GPIO output, and PSR for requests of bits that are programmed as GPIO input. 

Does that make sense?

Ed

[fabio_estevam · Accepted solution]

Hi Ed and Kursad,

We discussed about this issue recently in the U-boot list and the conclusion was that the proper way to handle it would be passing the SION flag to the GPIO connected to your LED.

Please check this patch from Otavio:

[U-Boot,v2,5/9] imx: Easy enabling of SION per-pin using MUX_MODE_SION helper macro - Patchwork

Regards,

Fabio Estevam

[EdSutter]

Ok, I see...

Kursad and Fabio, thanks much for the quick help.

Ed

[KursadOney]

Ed,

I believe you're correct. In fact, it sounds like there is no need to use the DR PSR registers at all. The reference manual says:

While the GPIO direction is set to input (GPIO_GDIR = 0), a

read access to GPIO_DR does not return GPIO_DR data.

Instead, it returns the GPIO_PSR data, which is the

corresponding input signal value.

I checked the denx u-boot tree and this issue seems to be there too. The following line:

val = (readl(&regs->gpio_psr) >> gpio) & 0x01;

should be:

val = (readl(&regs->gpio_dr) >> gpio) & 0x01;

FabioEstevam - can you verify this? Can Ed submit a patch to your tree that can be propogated upstream?

[EdSutter]

Kursad,

Thanks for the quick reply.

Just to clarify, I assume you meant...

... no need to use the PSR registers at all...

correct?

And, by the way, I'm using the source from the denx uboot tree.

Ed

[KursadOney]

Ah yes, that's what I meant! I'll correct my reply. Thanks!