Re: [PATCH 1/4] input: touchscreen: Add generic touchscreen softbutton handling code

[Hans de Goede <hdegoede@redhat.com>]

On 02-08-16 10:33, Hans de Goede wrote:
> Hi,
>
> On 01-08-16 19:41, Dmitry Torokhov wrote:
>> On Mon, Aug 01, 2016 at 11:54:30AM -0500, Rob Herring wrote:
>>> On Sun, Jul 31, 2016 at 05:23:07PM +0200, Hans de Goede wrote:
>>>> Some touchscreens extend over the display they cover and have a number
>>>> of capacative softbuttons outside of the display the cover.
>>>>
>>>> With some hardware these softbuttons simply report touches with
>>>> coordinates outside of the normal coordinate space for touches on the
>>>> display.
>>>>
>>>> This commit adds a devicetree binding for describing such buttons in
>>>> devicetree and a bunch of helper functions to easily add support for
>>>> these to existing touchscreen drivers.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>  .../bindings/input/touchscreen/softbuttons.txt     |  58 +++++++++
>>>>  drivers/input/touchscreen/Makefile                 |   2 +-
>>>>  drivers/input/touchscreen/softbuttons.c            | 145 +++++++++++++++++++++
>>>>  include/linux/input/touchscreen.h                  |   9 ++
>>>>  4 files changed, 213 insertions(+), 1 deletion(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/softbuttons.txt
>>>>  create mode 100644 drivers/input/touchscreen/softbuttons.c
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/softbuttons.txt b/Documentation/devicetree/bindings/input/touchscreen/softbuttons.txt
>>>> new file mode 100644
>>>> index 0000000..3eb6f4c
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/input/touchscreen/softbuttons.txt
>>>> @@ -0,0 +1,58 @@
>>>> +General Touchscreen Softbutton Properties:
>>>> +
>>>> +Some touchscreens extend over the display they cover and have a number
>>>> +of capacative softbuttons outside of the display the cover.
>>>> +
>>>> +Some of these softbuttons simply report touches with coordinates outside of
>>>> +the normal coordinate space for touches on the display. This binding is for
>>>> +describing such buttons in devicetree.
>>>> +
>>>> +Each softkey is represented as a sub-node of the touchscreen node.
>>>> +
>>>> +Required subnode-properties:
>>>> + - label            : Descriptive name of the key.
>>>> + - linux,code            : Keycode to emit.
>>>> + - softbutton-min-x        : X start of the area the softbutton area covers
>>>> + - softbutton-max-x        : X end of the area the softbutton area covers
>>>> + - softbutton-min-y        : Y start of the area the softbutton area covers
>>>> + - softbutton-max-y        : Y end of the area the softbutton area covers
>>>
>>> This generally looks fine to me, but I am wondering one thing. If the
>>> buttons are located at the origin of an axis, can we handle that case? I
>>> don't think you can unless you assume softbutton-max-? is 0 for the
>>> touchscreen. To put it another way, you have a gap from 1024 to 1084
>>> which you can't express for buttons at the origin unless you do negative
>>> numbers.
>>
>> I do not this this should be done in kernel: I do not see nay difference
>> in softbuttons or sliders or circular controls or... They are not
>> controller-specific and I think are better handled in userspace. We do
>> that for Synaptics touchpads with softbuttons, we can do that for other
>> controllers.
>>
>> Also, what is or stance when there is no bezel and we sill want to have
>> the softbutons (i.e. all Nexus phones and tablets)?
>
> Maybe softbuttons is not the best name (I googled and it seemed to match),
> so first lets make clear what I'm talking about here, I wrote this patch-set
> for this tablet:
>
> https://content.hwigroup.net/images/products_xl/157078/dserve-dsrv-9703c.jpg
>
> I decided to make it generic, since it seemed to me that there will likely
> be other hardware which is similar (but uses a different touch screen
> controller) out there, so keeping this generic seemed best.
>
> Now back to the kernelspace vs userspace solution question, if you look at
> the picture you will see 3 clearly marked buttons on the front of the tablet,
> outside of the display: menu, home and back. These are not buttons which get
> generated / drawn on the display when you touch the bottom of the screen, these
> are separate single-purpose buttons which happen to report presses via the
> touchscreen controller then via a separate hw mechanism.
>
> If these buttons used a separate capacative button controller as used in
> e.g. capacitive numpads, say something like this:
>
> http://www.ebay.com/itm/2PCS-TTP223-Capacitive-Touch-Switch-Button-Self-Lock-Module-for-Arduino-/131662428748
>
> Then there would be no question that this belongs in the kernel. I do not
> see how these are any different really. These cannot be runtime re-configured,
> they are separate dedicated buttons which happens to report presses via
> the touchscreen controllers.
>
> There also is the question of hardware description, no matter where we
> put support for this, we need some place were we describe the presence
> of these buttons to the software bits which end up dealing with them.
>
> And we already have a mechanism for describing "fixed" hardware for a
> certain model machine / board in the form of devicetree. To me putting
> the hardware description for this anywhere but in devicetree makes
> no sense because then we start scattering the description of a single
> device-type over multiple places which seems like a bad idea.
>
> And once we decided to put the description in devicetree, then dealing
> with this in the kernel becomes the logical thing to do.
>
> Regards,
>
> Hans
>
>
> p.s.
>
> One other example of buttons like these are those on the nexus one:
>
> http://www.informatica.com.do/7BZ-PortalImageUpload/image/2010151059110.google-nexus-one-01.jpg
>
> Note I'm not saying this binding will work for those, I've no idea how
> they are hooked up, but they are the same in that they are really
> 4 dedicated separate buttons which happen to be part of the main digitizer.

Dmitry, ping? Any reaction to the above ?

Regards,

Hans


p.s.

I've been thinking a bit more about this, trying to envision a reasonable
user-space solution and I do not really see any. IMHO this really belongs
in the kernel, and it is not a lot of code, the basic softbutton core is
quite small, and the actually needed touchscreen driver changer are
only a couple of line, so it is not like doing this in the kernel is
a hassle, where as doing this in userspace will be a hassle.