Hi Javier,
Am 27.08.2014 09:13, schrieb Javier Martinez Canillas:
> On 08/27/2014 12:53 AM, Andreas Färber wrote:
>>>
>>> +&hsi2c_8 {
>>> + status = "okay";
>>> + clock-frequency = <333000>;
>>> +
>>> + trackpad@4b {
>>> + compatible="atmel,maxtouch";
>>> + reg=<0x4b>;
>>> + interrupt-parent=<&gpx1>;
>>> + interrupts=<1 IRQ_TYPE_EDGE_FALLING>;
>>
>> Nit: Here's a style break (4x spaces around '=' missing).
>>
>
> True, these bits were copied from the downstream Chrome OS verbatim and
> the missing space around '=' was there, I missed that when reviewing.
>
> I'll re-spin and fix those style issues.
>
>>> + wakeup-source;
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&trackpad_irq>;
>>> + linux,gpio-keymap = < KEY_RESERVED
>>> + KEY_RESERVED
>>> + 0 /* GPIO 0 */
>>> + 0 /* GPIO 1 */
>>> + 0 /* GPIO 2 */
>>> + BTN_LEFT /* GPIO 3 */
>>> + KEY_RESERVED
>>> + KEY_RESERVED >;
>>> + };
>>
>> Coincidentally, I experimentally came up with a very similar DT node for
>> Spring the weekend:
>>
>> + trackpad@4b {
>> + compatible = "atmel,maxtouch";
>> + reg = <0x4b>;
>> + interrupt-parent = <&gpx1>;
>> + interrupts = <2 IRQ_TYPE_NONE>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&trackpad_irq>;
>> + linux,gpio-keymap = <KEY_RESERVED
>> + KEY_RESERVED
>> + KEY_RESERVED
>> + KEY_RESERVED
>> + KEY_RESERVED
>> + BTN_LEFT>;
>> + wakeup-source;
>> + };
>>
>> 0 == KEY_RESERVED, so you can consistently use it for GPIO 0-2, too. :)
>>
>
> I know that the value of KEY_RESERVED is 0 but I didn't use KEY_RESERVED
> for the GPIO on purpose.
>
> What I understood is that the SPT_GPIOPWN_T19 object sends messages using
> a status byte so you have a maximum of 8 GPIO but not every maXTouch
> devices use all of them. So in the particular case of the device in the
> Peach Pit, from the 8 possible GPIO only 4 can be used and these are pins
> 2-5. So in theory you could connect 3 more GPIO in case you had more
> buttons (e.g: BTN_RIGHT, BTN_MIDDLE) but only 1 is used since the
> Chromebook just have BTN_LEFT.
FWIW when I press to the bottom right of my touchpad, I do get
right-click functionality even with just BTN_LEFT specified in the
keymap. Magic. :)
> Nick sent a patch [0] that extend the atmel touchpad DT binding and the
> doc says "Use KEY_RESERVED for unused padding values". But is not clear
> what value you should use for GPIO that are actually supported by the
> device but have no keycode associated.
>
> So by using 0 instead of KEY_RESERVED I wanted to document that pins 2-4
> are actually supported and not reserved by the device but there is no
> keycode associated with that GPIO.
You already documented that via comments though.
> If there was a BTN_NONE or KEY_UNUSED it would had been better but I think
> that making a distinction between these two cases (reserved pin vs GPIO
> available but not used) is useful.
Maybe Nick can comment here.
>> I probably should add the two trailing _RESERVEDs, too?
>>
>
> I see that is used for properties that are arrays, for example
> "linux,keymap" in Documentation/devicetree/bindings/input/matrix-keymap.txt.
That does not answer my question: Do all maxTouch touchpads (or
specifically that in Spring) need eight entries, padded with said
KEY_RESERVED? In my experiments using just six entries (i.e., until the
non-zero entry) worked okay - so does this T19 message have specifically
eight bits? Tegra used just four entries iirc.
>> With my above snippet I got an awful lot of "Interrupt triggered but
>> zero messages" warnings (which I simply commented out as quickfix).
>> Is that why you are using _EDGE_FALLING? Or pin-function 0xf?
>> (In my case the ChromeOS DT had IRQ_TYPE_NONE and pin-function 0x0.)
>>
>
> These are two separate but related things:
>
> a) IRQF_TRIGGER_FALLING:
>
> Yes, the Chrome OS DT for Peach Pit also has IRQ_TYPE_NONE but the DTS is
> not correct.
>
> If you look at the Chrome OS atmel driver
> (drivers/input/touchscreen/atmel_mxt_ts.c), it passes IRQF_TRIGGER_FALLING
> to request_threaded_irq():
>
> /* Default to falling edge if no platform data provided */
> irqflags = data->pdata ? data->pdata->irqflags : IRQF_TRIGGER_FALLING;
> error = request_threaded_irq(client->irq, NULL, mxt_interrupt,
> irqflags | IRQF_ONESHOT,
> client->name, data);
>
> The above code is wrong since is overwriting the edge/level type flags set
> by OF when parsing the "interrupts" property so you have to use the
> expected IRQ flags in your DTS.
>
> b) pin-function 0xf instead of 0x0:
>
> The pin-function 0x0 is GPIO input while 0xf is GPIO IRQ. Usually on other
> SoCs to use a GPIO IRQ you just configure the pad as GPIO input and then
> enable the pin as an interrupt but on Exynos SoC these are really two
> different functions. So if you configure the pin as GPIO input and this
> happens after the pin is configured as GPIO IRQ, interrupts are not triggered.
>
> I faced that issue before [1] and was solved with Tomasz's commit:
>
> f6a8249 pinctrl: exynos: Lock GPIOs as interrupts when used as EINTs
>
> which changes the pinctrl-exynos driver to setup a pin as GPIO IRQ on
> .irq_request_resources instead of .irq_set_type. So, with that patch even
> when pin-function re-configures the function to GPIO input, is then
> configured as GPIO IRQ when request_threaded_irq() is called.
>
> So probably is working for you just because you tested on linux-next that
> already has Tomasz's changes but still the correct thing to do is to setup
> the pin as 0xf. This change probably is needed on other pins used as GPIO
> IRQ that are using 0x0 now.
>
> Sorry, the email became longer than I wanted but I hope is helpful to you.
Thanks for the explanations, I'll test those settings on Spring then.
Could you point me to what ChromeOS tree and branch I should be looking
at? For instance, the linux-next.git chromeos-3.8 branch did not have
any DT for Spring. Therefore my series is based on /proc/device-tree
rather than any ChromeOS source code.
Thanks,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
