Re: [PATCH v2] Input: add 'safe' user switch codes

Quoting Geert Uytterhoeven (2021-11-08 11:00:20)

Hi Jeff,

On Sun, Nov 7, 2021 at 7:17 AM Jeff LaBundy [off-list ref] wrote:

quoted

On Sat, Nov 06, 2021 at 10:13:15AM +0000, Kieran Bingham wrote:

quoted

Quoting Dmitry Torokhov (2021-11-05 23:04:23)

quoted

On Fri, Nov 05, 2021 at 12:00:37PM -0500, Jeff LaBundy wrote:

quoted

On Fri, Nov 05, 2021 at 10:35:07AM +0000, Kieran Bingham wrote:

quoted

All existing SW input codes define an action which can be interpreted by
a user environment to adapt to the condition of the switch.

For example, switches to define the audio mute, will prevent audio
playback, and switches to indicate lid and covers being closed may
disable displays.

Many evaluation platforms provide switches which can be connected to the
input system but associating these to an action incorrectly could
provide inconsistent end user experiences due to unmarked switch
positions.

Define two custom user defined switches allowing hardware descriptions
to be created whereby the position of the switch is not interpreted as
any standard condition that will affect a user experience.

This allows wiring up custom generic switches in a way that will allow
them to be read and processed, without incurring undesired or otherwise
undocumented (by the hardware) 'default' behaviours.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---

Sigh, a compile test might have at least saved the buildbots the trouble
of notifying me I also need to update the INPUT_DEVICE_ID_SW_MAX. But
even so - I'm really looking for a discussion on the best ways to
describe a non-defined switch in device tree.

Here's a compiling v2 ;-) But the real questions are :

 - Should an existing feature switch be used for generic switches?
 - Should we even have a 'user' defined switch?
 - If we add user switches, how many?

This is merely my opinion, but if a hardware switch does not have a defined
purpose, it does not seem necessary to represent it with an input device.

Yes, exactly. For input core we are trying to avoid generic events with
no defined meaning.

That's understandable, particularly as I could then ponder - how do we
even define generic switches, and how many ;-) I wanted to discuss it
because otherwise these switches will be defined in DT as buttons. And
they are not buttons...

quoted

What are these switches? GPIOs? Maybe it would be better to use GPIO
layer to test the state for them?

They are physical slide switches on the board. But they have no defined
purpose by the hardware designer. The purpose would be defined by the
end user, as otherwise they are generic test switches.

These have been previously handled as gpio-key buttons, for instance
key-1 to key-4 at [0] are actually four slides switches.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e3414b8c45afa5cdfb1ffd10f5334da3458c4aa5

What I'm trying to determine/promote is that they are not push buttons,
and shouldn't be described as such. I have posted [1] to add support for
these switches, but I am limited to chosing 'functions' which will have
an impact on the system...

[1] https://lore.kernel.org/all/20211025130457.935122-1-kieran.bingham+renesas@ideasonboard.com/ (local)

Presently in [1] I have chosen SW_LID and SW_DOCK as very arbitrary
functions for the switches. But my concern is that in doing so, the
SW_LID position could for instance suggest to a window environment or
power management system that the lid is closed, and the system should
be suspended (of course depending upon configurations). That would mean
that the board would now be potentially always heading into a suspend
after power up which would not be at all clear from the switch.

I believe a 'switch' is the correct way to define this hardware, so that
both positions can be determined, and read, and events generated on
state change - but that there shouldn't be any artificially imposed side
effects from the description.

If the answer is "no we can't have generic switches" then so be it, but
it feels wrong to further propogate the definition of these test
switches as keys.

I agree that a slide switch tied to a GPIO is indeed a switch in terms of
input core. Note, however, that definitions from your first example (such
as KEY_1) are not any less generic; those have specific meanings too.

But at least the KEY_* events are less likely to cause harmful side
effects than the SW_* events.  I have no idea which daemon in e.g. a
generic Ubuntu userspace would act on the SW_* events.

The fact that they are no-less generic is precisely another reason why I
don't think these switches should be bound to KEY_0/N either.

If the switch is in the 'on' position - then it will cause repeated key
press events... as the key will be permanantly in the active state.

Holding KEY_0 down isn't necessarily as harmful to the user as having
the screen disabled, but I'm sure it would have a painful impact
depending upon various key repeat delays and such of course.

The examples defined in [0] are (IMO) wrong, and have been implemented
incorrectly because of a lack of a generic test switch position.

If we can come up with a solution, I think those definitions should be
fixed up (if we're allowed to modify them now they exist? I expect so?).

quoted

If the concern is that toggling a switch effects undesired behavior, such
as turning a display on or off, then the switch should not be represented
with a gpio-keys node in the first place.

Stated another way, the fact that the GPIO are connected to something does
not necessarily mean they need to be supported. Only once they map to some
function should they be defined, in my opinion.

Following the mantra "DT describes hardware, not software policy", I
think we should describe generic switches in DT, and perhaps have a way
to configure the actual event code from userspace (e.g. using sysfs?).
Without such configuration, perhaps they could emit a SW_* event with
the MSB set, so at least their state can be queried using e.g. evtest?

The hardware exists, and it's permanantly wired this way, we 'just' need
to describe it somehow...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds