| Message ID | 4e1455be0902062015i1ff94b28p93588e61b7860618@mail.gmail.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
On Friday 06 February 2009, Joonyoung Shim wrote: > If the GPIO pin is output, must read the value from OMAP24XX_GPIO_DATAOUT > register in __omap_get_gpio_datain(). Same comment as with the similar twl4030 patch you sent: the API specifies that the value reported for input should be the actual value AT THE PIN, even for outputs ... this patch would change the behavior to report the value the pin is trying to drive, which isn't necessarily going to reflect the signal value at the pin. So, NAK. In a few rare cases on OMAP3 chips -- not currently used by Linux -- GPIOs can be output-only (input drivers not enabled). Such a change might be appropriate for GPIOs configured that way ... were it not for the fact that in those cases, a value of zero should be returned. - Dave > Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com> > --- > > diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c > index f856a90..296773a 100644 > --- a/arch/arm/plat-omap/gpio.c > +++ b/arch/arm/plat-omap/gpio.c > @@ -428,7 +428,11 @@ static int __omap_get_gpio_datain(int gpio) > #endif > #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX) > case METHOD_GPIO_24XX: > - reg += OMAP24XX_GPIO_DATAIN; > + if (__raw_readl(reg + OMAP24XX_GPIO_OE) > + & (1 << get_gpio_index(gpio))) > + reg += OMAP24XX_GPIO_DATAIN; > + else > + reg += OMAP24XX_GPIO_DATAOUT; > break; > #endif > default: > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sorry, I added CC. 2009/2/9 David Brownell <david-b@pacbell.net>: > On Sunday 08 February 2009, Joonyoung Shim wrote: >> I have knew if gpio pin is configured as an output, the value of the >> corresponding bit >> in the GPIO_DATAOUT register is driven on the corresponding gpio pin. >> But the implemented API only reports the value of the corresponding bit in the >> GPIO_DATAIN register. This can report a wrong value. >> >> For example, even though the gpio 141pin is configured as an output and assigns >> the gpio's value to 1 through gpio_set_value(), the result of >> "# cat /sys/kernel/debug/gpio" reports to me as follows. >> >> GPIOs 128-159, gpio: >> gpio-141 (motor enable ) out lo >> >> but it should have reported "hi" instead of "lo". > > I happen to observe that mach-omap2/mux.c there are a bunch > of pins that are wrongly multiplexed. Like: > > MUX_CFG_34XX("AE6_34XX_GPIO141", 0x16e, > OMAP34XX_MUX_MODE4 | OMAP34XX_PIN_OUTPUT) I think that it is right because i want to use gpio141 pin as output, > > Which should clearly be AE6_34XX_GPIO141_OUT since it does > not enable the input driver. Try making that ...PIN_OUTPUT > read ...PIN_INPUT and see if things behave. Of course, it will be better if the name is clear as AE6_34XX_GPIO141_OUT. but, the core of this problem is not it, the chip->get() called in gpiolib_dbg_show() of drivers/gpio/gpiolib.c or dbg_gpio_show() of arch/arm/plat-omap/gpio.c can return the wrong value when gpio pin is configured as an output. the reason as i say at former email, is because __omap_get_gpio_datain() called by gpio_get() of arch/arm/plat-omap/gpio.c reports only value of OMAP24XX_GPIO_DATAIN register. - Joonyoung > > - Dave > > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sunday 08 February 2009, Joonyoung Shim wrote: > > Which should clearly be AE6_34XX_GPIO141_OUT since it does > > not enable the input driver. Â Try making that ...PIN_OUTPUT > > read ...PIN_INPUT and see if things behave. > > Of course, it will be better if the name is clear as AE6_34XX_GPIO141_OUT. > but, the core of this problem is not it, the chip->get() called in > gpiolib_dbg_show() > of drivers/gpio/gpiolib.c or dbg_gpio_show() of arch/arm/plat-omap/gpio.c > can return the wrong value when gpio pin is configured as an output. No, the core of the problem is that you're using a GPIO which is explicitly configured as output-only ... but are treating it as if it were a normal bi-directional GPIO. It's acting *exactly* right for an output-only GPIO. But that's not the behavior you expect, or want. If you weren't disabling the input driver, then when you ask for the input value it would work as you expect. Which is why I suggested you change how that ball is configured. (That's actually a bugfix to those muxing tables ...) - Dave p.s. This specific confusion is new on OMAP3; previous OMAPs didn't have the extra byte of options to configure. The PIN_INPUT flag was, on previous chips, effectively hard-wired on for GPIOs. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2009/2/9 David Brownell <david-b@pacbell.net>: > On Sunday 08 February 2009, Joonyoung Shim wrote: >> > Which should clearly be AE6_34XX_GPIO141_OUT since it does >> > not enable the input driver. Try making that ...PIN_OUTPUT >> > read ...PIN_INPUT and see if things behave. >> >> Of course, it will be better if the name is clear as AE6_34XX_GPIO141_OUT. >> but, the core of this problem is not it, the chip->get() called in >> gpiolib_dbg_show() >> of drivers/gpio/gpiolib.c or dbg_gpio_show() of arch/arm/plat-omap/gpio.c >> can return the wrong value when gpio pin is configured as an output. > > No, the core of the problem is that you're using a GPIO > which is explicitly configured as output-only ... but are > treating it as if it were a normal bi-directional GPIO. > > It's acting *exactly* right for an output-only GPIO. But > that's not the behavior you expect, or want. > > If you weren't disabling the input driver, then when you > ask for the input value it would work as you expect. Which > is why I suggested you change how that ball is configured. > (That's actually a bugfix to those muxing tables ...) I understood that you say, ths PIN_OUTPUT flag must use when GPIO pin is output-only, and i got the result i want after GPIO pin is configured as bi-diretional GPIO. But i wonder why don't control in gpio_get() of arch/arm/plat-omap/gpio.c when the GPIO pin is output-only? thank you. - Joonyoung > > - Dave > > p.s. This specific confusion is new on OMAP3; previous > OMAPs didn't have the extra byte of options to > configure. The PIN_INPUT flag was, on previous > chips, effectively hard-wired on for GPIOs. > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c index f856a90..296773a 100644 --- a/arch/arm/plat-omap/gpio.c +++ b/arch/arm/plat-omap/gpio.c @@ -428,7 +428,11 @@ static int __omap_get_gpio_datain(int gpio) #endif #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX) case METHOD_GPIO_24XX: - reg += OMAP24XX_GPIO_DATAIN; + if (__raw_readl(reg + OMAP24XX_GPIO_OE) + & (1 << get_gpio_index(gpio))) + reg += OMAP24XX_GPIO_DATAIN; + else + reg += OMAP24XX_GPIO_DATAOUT; break; #endif default:
If the GPIO pin is output, must read the value from OMAP24XX_GPIO_DATAOUT register in __omap_get_gpio_datain(). Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com> --- -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html