linux-kernel - Re: [PATCH 22/26] Input: synaptics-rmi4

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20151109232557.GG9155@dtor-ws>
Date:	Mon, 9 Nov 2015 15:25:57 -0800
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	Andrew Duggan <aduggan@...aptics.com>
Cc:	Benjamin Tissoires <benjamin.tissoires@...il.com>,
	Linus Walleij <linus.walleij@...aro.org>,
	Linux Input <linux-input@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Benjamin Tissoires <benjamin.tissoires@...hat.com>,
	Christopher Heiny <cheiny@...aptics.com>,
	Stephen Chandler Paul <cpaul@...hat.com>,
	Allie Xiong <axiong@...aptics.com>
Subject: Re: [PATCH 22/26] Input: synaptics-rmi4 - Add F30 support

On Mon, Nov 09, 2015 at 02:54:08PM -0800, Andrew Duggan wrote:
> On 11/09/2015 06:06 AM, Benjamin Tissoires wrote:
> >Hey Linus,
> >
> >first thanks for the reviews. Much appreciated.
> >
> >On Mon, Nov 9, 2015 at 2:32 PM, Linus Walleij <linus.walleij@...aro.org> wrote:
> >>On Fri, Nov 6, 2015 at 12:42 AM, Andrew Duggan <aduggan@...aptics.com> wrote:
> >>
> >>>From: Benjamin Tissoires <benjamin.tissoires@...hat.com>
> >>>
> >>>RMI4 Function 0x30 provides support for GPIOs, LEDs and mechanical
> >>>buttons.  In particular, the mechanical button support is used in
> >>>an increasing number of touchpads.
> >>>
> >>>[BT] cured the code to rely only on the unified input node created
> >>>by rmi_driver.
> >>>
> >>>Signed-off-by: Andrew Duggan <aduggan@...aptics.com>
> >>>Signed-off-by: Allie Xiong <axiong@...aptics.com>
> >>>Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
> >>I see this function driver is not yet adding any gpio_chip or
> >>LEDs class devices, which is fine, we can add that later when
> >>we have something to test. Or is iit using that LED feature
> >>in the input layer that corresponds to caps lock etc?
> 
> F30 is currently only used in touchpads and mostly used to report
> the the click of the tact switch on clickpads. When the GPIO
> interrupts we report the appropriate button event to the host.
> Because the GPIOs are wired to our ASIC and not to the host I'm not
> sure there is any benefit to exposing it to the host using
> gpio_chip.
> 
> Dmitry asked a similar question and here is my response to him:
> http://www.spinics.net/lists/linux-input/msg40458.html
> 
> There are a few rmi touchpads which have LEDs in the top left corner
> to indicate if the touchpad has been enabled / disabled. Writing to
> a register in F30 would turn on and off the LED. I don't think they
> are being made anymore so I haven't really looked into how we would
> implement this. If there is something in the input layer for
> controlling LEDs that might be the way to do it.

No, it should be eventually exported as a generic LED (input core itself
now registers its capslock, etc leds with LED core and I do not plan on
adding new input LEDs).

> 
> >Do not take my words as the official ones, but when we discussed with
> >Synaptics about F30 (and unified input), they told us that they
> >designed the driver based on the phone use case. In such use case, the
> >power (and maybe LEDs) are handled through F30, and the touchscreen
> >through F11/12. Problem is, I am not even sure there are phones around
> >with such F30/F11 combination.
> >
> >So in the end, from what I can see, F30 is used for buttons on
> >touchpads/clickpads, and LEDs when there are some on these touchpads.
> >
> >I don't know if the keyboards would use F30 for their LEDs though.
> >
> >That being said. Unless Synaptics tells us that there are uses of a
> >non "unified" input device somewhere, I would also agree to only keep
> >the "unified" input node, which would simplify both F11/12 and F30.
> 
> The original architecture of the driver was to have each function
> have it's own input device. The reasoning was that you would have a
> phone with a touchscreen (F11) and maybe some capacitive buttons
> (F19 or F1A or something) and that the system should see these as
> two separate devices. I'm wondering if this is needed though. Would
> userspace care if a touchscreen also reported KEY_HOME, KEY_BACK, or
> KEY_MENU?
> 
> At this point I agree, it's probably time to just have F11, F12, and
> F30 all report from the input device created in rmi_driver and
> remove the non unified option. If someday there is a function which
> needs to report data from a different input device it can just
> create it in that function.

Hmm.. if F30 is used for clickpads I agree that we'd want to mix it in
with the F11 data. But for generic capacitive buttons I think it would
be better if they are reported separately. Whether we want to do that
form F30 or have F30 actually export gpios and use gpio-keys to actually
create input device - I am open to discussing.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/