Thread (45 messages) 45 messages, 4 authors, 2013-06-30

[PATCH v1 04/12] input: matrix-keypad: push/pull, separate polarity

From: Gerhard Sittig <hidden>
Date: 2013-06-30 11:43:24
Also in: linux-devicetree, linux-input

On Fri, Jun 28, 2013 at 09:01 -0600, Stephen Warren wrote:
On 06/28/2013 02:33 AM, Gerhard Sittig wrote:
quoted
On Mon, Jun 24, 2013 at 17:14 -0600, Stephen Warren wrote:
quoted
[ active low pins, individually for sets or even single pins ]

Can't you add this enhancement as follows:

Update the driver to look at the per-pin GPIO flags in all cases.
Presumably in any existing cases, those flags all say "active high"
anyway, since specifying anything else was useless. In the very unlikely
case this isn't true, one could always add a quirk based on the
HW-specific compatible value.
Maybe I missed something, but
- is "the 'active low' property of a GPIO pin" something platform
  specific? (maybe ARM inspired, introduced with pinctrl features)
- is it only available for specific GPIO controller drivers
  ("chips" or "banks"), or is it in some layer on top of gpiolib?
The way this property is represented is technically defined by each
individual GPIO controller's DT binding. However, at least on ARM (and
in fact I'd guess everywhere), many/most controllers do this in the same
way; by adding an additional cell to #gpio-cells, which holds flags
data, with values in that cell containing:

include/linux/of_gpio.h:
quoted
/*
 * This is Linux-specific flags. By default controllers' and Linux' mapping
 * match, but GPIO controllers are free to translate their own flags to
 * Linux-specific in their .xlate callback. Though, 1:1 mapping is recommended.
 */
enum of_gpio_flags {
        OF_GPIO_ACTIVE_LOW = 0x1,
};
include/dt-bindings/gpio/gpio.h:
quoted
#define GPIO_ACTIVE_HIGH 0
#define GPIO_ACTIVE_LOW 1
I'm not sure how new this is; it's possible older bindings don't have
this extra cell. A quick grep of arch/*/boot/dts shows that almost all
values of #gpio-cells are 2, so likely this is almost everywhere though.
Ah, so it's not a feature of gpiolib in itself, but in parallel
to gpiolib.

In addition to the "chip and pin" spec in device tree bindings,
one may specify additional flags, of which currently only
'active-low' exists.  Thank you for the pointer.  Yes I've seen
"phandle plus two cell" specs everywhere, but haven't seen any
non-zero value so far in the second cell.
quoted
- is it in some other tree or branch than mainline?
It's been in mainline for quite a while now; a good number of kernel
releases.
quoted
Documentation/gpio.txt suggests and eyeballing the gpiolib.c
source supports (v3.10-rc7) that the in-kernel API doesn't have
an 'active-low' for pins.  There is one for the sysfs API and
pins that were exported to user space, but not for in-kernel use.

This fits with my observation that application drivers on top of
gpiolib often take care of such a property which actually looks
like it would belong to the physical attachment.

I understand that a chip's driver will hide when a SoC's pin is
inverted, but I cannot see where gpiolib provides a means to hide
an externally connected inverting driver.
With the current gpiolib APIs, gpiolib only provides a way for GPIO
clients to retrieve the GPIO flags from DT. It's then up to the GPIO
client to actually make use of the flag, by inverting the value passed
to gpio_set(), or returned from gpio_get().

Flags are retrieved by calling of_get_named_gpio_flags() rather than
of_get_named_gpio().
Ah, so optionally inverting the level between the logical request
in software and controlling the physical pin remains the job of
the application driver (here: the input driver that manages the
matrix which just happens to be attached to GPIOs), since it's
not transparently done within gpiolib or the gpio chip driver.

But what the matrix driver shall do is not apply one 'invert
everything' flag, but instead apply a flag that's specific to an
individual GPIO pin.  And this pin specific flag either gets
setup from device tree information (from the flags cell of the
GPIO spec), or gets derived from the platform data's 'invert
everything' flag that applies to the non-DT case.  And thus the
individual polarity of rows and columns is a mere "byproduct" of
respecting the individual pins' polarity.  Nice. :)

Thank you, this pointer is much appreciated.

There is a new GPIO API under development which I /think/ will hide this
flag inside the GPIO core, so that GPIO clients don't have to care about
it, and it all works automatically. I'm not sure what the status of this
is though. Search Google for "gpio_get()".
Oh, I understand that this involves a huge sweep over both layers
of GPIO chip drivers as well as GPIO API using drivers.  And is
completely orthogonal to whether and when more drivers are tought
to respect the pin's active-low property.


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help