On 06/22/2013 03:36 AM, Gerhard Sittig wrote:

On Fri, Jun 21, 2013 at 15:34 -0600, Stephen Warren wrote:

quoted

On 06/21/2013 12:09 PM, Gerhard Sittig wrote:

quoted

extend the device tree adjustable hardware configuration:
- allow for differing polarity of the row and column GPIO pins
- optionally fully drive column output pins instead of the former
  unconditional open collector emulation approach

quoted

diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt

quoted

+- row-gpios-activelow:	row GPIO pins are active low
+- col-gpios-activelow:	column GPIO pins are active low
 - gpio-activelow:	row pins as well as column pins are active low
+			(provided for backward compatibility, and useful
+			for matrix layouts of identical polarity for
+			rows and columns)

Those should all come from the existing GPIO flags, and may even differ
for each GPIO.

quoted

+- col-gpios-pushpull:	fully drive the column selection pins in either
+			direction (high and low signals), the default
+			behaviour is to actively drive low signals and
+			be passive otherwise (emulates an open collector
+			output driver)

We don't actually have GPIO flags defined for pushpull-vs-open-collector
etc. Perhaps we should do so. Then, we wouldn't need to invent custom
properties to represent that in this binding.

I see your concerns and understand, but chose the above way to
just extend the existing implementation in the least intrusive
way without changing its nature.  See the reply on the first
patch for more details on the motivation.

Of course I agree that pin properties belong to the GPIO layer
and that "application drivers" on top should not worry about
that.  Were I allowed to break backwards compatibility, then I
could have done more shuffling.  But lacking the capability to
test the affected setups puts more burdon on others, which I
wanted to avoid.

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.

If gpio-activelow is set, mark all pins as active-low.

This way, any future extensions (i.e. what this patch implements: just
rows active low, just columns active low) can be implemented using
purely the standard GPIO flags, but the existing behaviour of the
existing gpio-activelow property is maintained.