Message ID | 1438795388-22743-3-git-send-email-edubezval@gmail.com |
---|---|
State | New |
Headers | show |
Hi Eduardo, On Wed, Aug 5, 2015 at 2:23 PM, Eduardo Valentin <edubezval@gmail.com> wrote: > In current implementation, reading the value of an output gpio > always return 0. The reason is because when a gpio is configured Have you tried setting the SION bit for the pad? This should fix the problem. Regards, Fabio Estevam -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Fabio, On Wed, Aug 05, 2015 at 02:43:05PM -0300, Fabio Estevam wrote: > Hi Eduardo, > > On Wed, Aug 5, 2015 at 2:23 PM, Eduardo Valentin <edubezval@gmail.com> wrote: > > In current implementation, reading the value of an output gpio > > always return 0. The reason is because when a gpio is configured > > Have you tried setting the SION bit for the pad? No, I haven't. From the bit description, it looks like it does a different thing of what we would achieve with this patch. The SION bit is a overwrite to the pad configuration. That is, the pin will be INPUT always. We don't want to force it to be input. We simply want to be able to read the value of an output GPIO. That is achievable by reading a different register. In case the GPIO is configured as output, the GPIO block documentation states its value can be read from GPIO_PSR. This approach works fine, as the GPIO will be output and we can still read its value. Spotting this is very simple. A quick try on /sys/class/gpio/export on a spare gpio would do it: # echo $SPARE_GPIO > /sys/class/gpio/export # echo out > /sys/class/gpio/gpio$SPARE_GPIO/direction # echo 1 > /sys/class/gpio/gpio$SPARE_GPIO/value # cat /sys/class/gpio/gpio$SPARE_GPIO/value without the patch it would always return 0. With the patch in, you see the read value corresponding the output of that pin (checked on scope, for instance). > > This should fix the problem. > > Regards, > > Fabio Estevam BR, Eduardo Valentin
Hi Eduardo, On Wed, Aug 5, 2015 at 3:02 PM, Eduardo Valentin <edubezval@gmail.com> wrote: > > Hello Fabio, > > On Wed, Aug 05, 2015 at 02:43:05PM -0300, Fabio Estevam wrote: >> Hi Eduardo, >> >> On Wed, Aug 5, 2015 at 2:23 PM, Eduardo Valentin <edubezval@gmail.com> wrote: >> > In current implementation, reading the value of an output gpio >> > always return 0. The reason is because when a gpio is configured >> >> Have you tried setting the SION bit for the pad? > > No, I haven't. From the bit description, it looks like it does a > different thing of what we would achieve with this patch. The SION bit > is a overwrite to the pad configuration. That is, the pin will be INPUT > always. > > We don't want to force it to be input. We simply want to be able to read > the value of an output GPIO. That is achievable by reading a different > register. In case the GPIO is configured as output, the GPIO block > documentation states its value can be read from GPIO_PSR. > This approach works fine, as the GPIO will be output and we can still > read its value. Setting the SION bit allows you to do exactly that. Please see: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/271774.html Regards, Fabio Estevam -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 05, 2015 at 03:11:40PM -0300, Fabio Estevam wrote: > Hi Eduardo, > > On Wed, Aug 5, 2015 at 3:02 PM, Eduardo Valentin <edubezval@gmail.com> wrote: > > > > Hello Fabio, > > > > On Wed, Aug 05, 2015 at 02:43:05PM -0300, Fabio Estevam wrote: > >> Hi Eduardo, > >> > >> On Wed, Aug 5, 2015 at 2:23 PM, Eduardo Valentin <edubezval@gmail.com> wrote: > >> > In current implementation, reading the value of an output gpio > >> > always return 0. The reason is because when a gpio is configured > >> > >> Have you tried setting the SION bit for the pad? > > > > No, I haven't. From the bit description, it looks like it does a > > different thing of what we would achieve with this patch. The SION bit > > is a overwrite to the pad configuration. That is, the pin will be INPUT > > always. > > > > We don't want to force it to be input. We simply want to be able to read > > the value of an output GPIO. That is achievable by reading a different > > register. In case the GPIO is configured as output, the GPIO block > > documentation states its value can be read from GPIO_PSR. > > This approach works fine, as the GPIO will be output and we can still > > read its value. > > Setting the SION bit allows you to do exactly that. Please see: > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/271774.html OK. Then, what is the recommendation? Do we set this by default in the driver code or this is left for DTS pinmux configuration? To me seams like a bug in the gpio driver still, as the possibility to read the value of an output gpio is missing/inconsistent. > > Regards, > > Fabio Estevam
On Wed, Aug 5, 2015 at 4:46 PM, Eduardo Valentin <edubezval@gmail.com> wrote: > OK. Then, what is the recommendation? Do we set this by default in the > driver code or this is left for DTS pinmux configuration? You just need to set the SION bit in the pad you need to read the output. SION is bit 30 according to Documentation/devicetree/bindings/pinctrl/fsl,imx-pinctrl.txt > To me seams like a bug in the gpio driver still, as the possibility to > read the value of an output gpio is missing/inconsistent. No, it is not a bug. The Reference Manual explains the need of setting the SION bit: From the mx6q reference manual: "28.4.3.2 GPIO Write Mode The programming sequence for driving output signals should be as follows: 1. Configure IOMUX to select GPIO mode (Via IOMUXC), also enable SION if need to read loopback pad value through PSR" -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" 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/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c index 9e5bdbd..8822823 100644 --- a/drivers/gpio/gpio-mxc.c +++ b/drivers/gpio/gpio-mxc.c @@ -401,6 +401,18 @@ static int mxc_gpio_to_irq(struct gpio_chip *gc, unsigned offset) return irq_find_mapping(port->domain, offset); } +static int mxc_gpio_get(struct gpio_chip *gc, unsigned int gpio) +{ + struct bgpio_chip *bgc = to_bgpio_chip(gc); + + if (!!(bgc->read_reg(bgc->reg_dir) & bgc->pin2mask(bgc, gpio))) + return !!(bgc->read_reg(bgc->reg_set) & + bgc->pin2mask(bgc, gpio)); + else + return !!(bgc->read_reg(bgc->reg_dat) & + bgc->pin2mask(bgc, gpio)); +} + static int mxc_gpio_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; @@ -455,6 +467,7 @@ static int mxc_gpio_probe(struct platform_device *pdev) if (err) goto out_bgio; + port->bgc.gc.get = mxc_gpio_get; port->bgc.gc.to_irq = mxc_gpio_to_irq; port->bgc.gc.base = (pdev->id < 0) ? of_alias_get_id(np, "gpio") * 32 : pdev->id * 32;