* [PATCH v2] Input: add 'safe' user switch codes @ 2021-11-05 10:35 Kieran Bingham 2021-11-05 17:00 ` Jeff LaBundy 0 siblings, 1 reply; 8+ messages in thread From: Kieran Bingham @ 2021-11-05 10:35 UTC (permalink / raw) To: linux-input, Geert Uytterhoeven, linux-renesas-soc Cc: Kieran Bingham, Dmitry Torokhov, Max Gurtovoy, Hans de Goede, Wu Hao, Bjorn Helgaas, Dan Williams, Dave Ertman, Maximilian Luz, Stephan Gerhold, Xu Yilun, open list 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? include/linux/mod_devicetable.h | 2 +- include/uapi/linux/input-event-codes.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h index ae2e75d15b21..dfa1e4f41cd8 100644 --- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -326,7 +326,7 @@ struct pcmcia_device_id { #define INPUT_DEVICE_ID_LED_MAX 0x0f #define INPUT_DEVICE_ID_SND_MAX 0x07 #define INPUT_DEVICE_ID_FF_MAX 0x7f -#define INPUT_DEVICE_ID_SW_MAX 0x10 +#define INPUT_DEVICE_ID_SW_MAX 0x12 #define INPUT_DEVICE_ID_PROP_MAX 0x1f #define INPUT_DEVICE_ID_MATCH_BUS 1 diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h index 225ec87d4f22..84a7b3debcb3 100644 --- a/include/uapi/linux/input-event-codes.h +++ b/include/uapi/linux/input-event-codes.h @@ -894,7 +894,9 @@ #define SW_MUTE_DEVICE 0x0e /* set = device disabled */ #define SW_PEN_INSERTED 0x0f /* set = pen inserted */ #define SW_MACHINE_COVER 0x10 /* set = cover closed */ -#define SW_MAX 0x10 +#define SW_1 0x11 /* set = user defined */ +#define SW_2 0x12 /* set = user defined */ +#define SW_MAX 0x12 #define SW_CNT (SW_MAX+1) /* -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Input: add 'safe' user switch codes 2021-11-05 10:35 [PATCH v2] Input: add 'safe' user switch codes Kieran Bingham @ 2021-11-05 17:00 ` Jeff LaBundy 2021-11-05 23:04 ` Dmitry Torokhov 0 siblings, 1 reply; 8+ messages in thread From: Jeff LaBundy @ 2021-11-05 17:00 UTC (permalink / raw) To: Kieran Bingham Cc: linux-input, Geert Uytterhoeven, linux-renesas-soc, Dmitry Torokhov, Max Gurtovoy, Hans de Goede, Wu Hao, Bjorn Helgaas, Dan Williams, Dave Ertman, Maximilian Luz, Stephan Gerhold, Xu Yilun, open list Hi Kieran, On Fri, Nov 05, 2021 at 10:35:07AM +0000, Kieran Bingham wrote: > 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. If the goal is to simply perform validation of a driver and/or hardware, is it not sufficient to map the switch to an existing code? Similarly, there is no way to know if two are enough. What if we have a new board tomorrow and need four? > > include/linux/mod_devicetable.h | 2 +- > include/uapi/linux/input-event-codes.h | 4 +++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h > index ae2e75d15b21..dfa1e4f41cd8 100644 > --- a/include/linux/mod_devicetable.h > +++ b/include/linux/mod_devicetable.h > @@ -326,7 +326,7 @@ struct pcmcia_device_id { > #define INPUT_DEVICE_ID_LED_MAX 0x0f > #define INPUT_DEVICE_ID_SND_MAX 0x07 > #define INPUT_DEVICE_ID_FF_MAX 0x7f > -#define INPUT_DEVICE_ID_SW_MAX 0x10 > +#define INPUT_DEVICE_ID_SW_MAX 0x12 > #define INPUT_DEVICE_ID_PROP_MAX 0x1f > > #define INPUT_DEVICE_ID_MATCH_BUS 1 > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h > index 225ec87d4f22..84a7b3debcb3 100644 > --- a/include/uapi/linux/input-event-codes.h > +++ b/include/uapi/linux/input-event-codes.h > @@ -894,7 +894,9 @@ > #define SW_MUTE_DEVICE 0x0e /* set = device disabled */ > #define SW_PEN_INSERTED 0x0f /* set = pen inserted */ > #define SW_MACHINE_COVER 0x10 /* set = cover closed */ > -#define SW_MAX 0x10 > +#define SW_1 0x11 /* set = user defined */ > +#define SW_2 0x12 /* set = user defined */ > +#define SW_MAX 0x12 > #define SW_CNT (SW_MAX+1) > > /* > -- > 2.30.2 > Kind regards, Jeff LaBundy ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Input: add 'safe' user switch codes 2021-11-05 17:00 ` Jeff LaBundy @ 2021-11-05 23:04 ` Dmitry Torokhov 2021-11-06 10:13 ` Kieran Bingham 0 siblings, 1 reply; 8+ messages in thread From: Dmitry Torokhov @ 2021-11-05 23:04 UTC (permalink / raw) To: Jeff LaBundy Cc: Kieran Bingham, linux-input, Geert Uytterhoeven, linux-renesas-soc, Max Gurtovoy, Hans de Goede, Wu Hao, Bjorn Helgaas, Dan Williams, Dave Ertman, Maximilian Luz, Stephan Gerhold, Xu Yilun, open list Hi Jeff, Kieran, On Fri, Nov 05, 2021 at 12:00:37PM -0500, Jeff LaBundy wrote: > Hi Kieran, > > On Fri, Nov 05, 2021 at 10:35:07AM +0000, Kieran Bingham wrote: > > 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. What are these switches? GPIOs? Maybe it would be better to use GPIO layer to test the state for them? Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Input: add 'safe' user switch codes 2021-11-05 23:04 ` Dmitry Torokhov @ 2021-11-06 10:13 ` Kieran Bingham 2021-11-07 6:17 ` Jeff LaBundy 0 siblings, 1 reply; 8+ messages in thread From: Kieran Bingham @ 2021-11-06 10:13 UTC (permalink / raw) To: Dmitry Torokhov, Jeff LaBundy Cc: linux-input, Geert Uytterhoeven, linux-renesas-soc, Max Gurtovoy, Hans de Goede, Wu Hao, Bjorn Helgaas, Dan Williams, Dave Ertman, Maximilian Luz, Stephan Gerhold, Xu Yilun, open list Hi Dmitry, Jeff, Thanks for looking into this, Quoting Dmitry Torokhov (2021-11-05 23:04:23) > Hi Jeff, Kieran, > > On Fri, Nov 05, 2021 at 12:00:37PM -0500, Jeff LaBundy wrote: > > Hi Kieran, > > > > On Fri, Nov 05, 2021 at 10:35:07AM +0000, Kieran Bingham wrote: > > > 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... > 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/ 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. -- Regards Kieran ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Input: add 'safe' user switch codes 2021-11-06 10:13 ` Kieran Bingham @ 2021-11-07 6:17 ` Jeff LaBundy 2021-11-08 11:00 ` Geert Uytterhoeven 0 siblings, 1 reply; 8+ messages in thread From: Jeff LaBundy @ 2021-11-07 6:17 UTC (permalink / raw) To: Kieran Bingham Cc: Dmitry Torokhov, linux-input, Geert Uytterhoeven, linux-renesas-soc, Max Gurtovoy, Hans de Goede, Wu Hao, Bjorn Helgaas, Dan Williams, Dave Ertman, Maximilian Luz, Stephan Gerhold, Xu Yilun, open list Hi Kieran, On Sat, Nov 06, 2021 at 10:13:15AM +0000, Kieran Bingham wrote: > Hi Dmitry, Jeff, > > Thanks for looking into this, > > Quoting Dmitry Torokhov (2021-11-05 23:04:23) > > Hi Jeff, Kieran, > > > > On Fri, Nov 05, 2021 at 12:00:37PM -0500, Jeff LaBundy wrote: > > > Hi Kieran, > > > > > > On Fri, Nov 05, 2021 at 10:35:07AM +0000, Kieran Bingham wrote: > > > > 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... > > > > 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/ > > 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. 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. > > -- > Regards > > Kieran Kind regards, Jeff LaBundy ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Input: add 'safe' user switch codes 2021-11-07 6:17 ` Jeff LaBundy @ 2021-11-08 11:00 ` Geert Uytterhoeven 2021-11-08 12:35 ` Kieran Bingham 0 siblings, 1 reply; 8+ messages in thread From: Geert Uytterhoeven @ 2021-11-08 11:00 UTC (permalink / raw) To: Jeff LaBundy Cc: Kieran Bingham, Dmitry Torokhov, linux-input, Linux-Renesas, Max Gurtovoy, Hans de Goede, Wu Hao, Bjorn Helgaas, Dan Williams, Dave Ertman, Maximilian Luz, Stephan Gerhold, Xu Yilun, open list Hi Jeff, On Sun, Nov 7, 2021 at 7:17 AM Jeff LaBundy <jeff@labundy.com> wrote: > On Sat, Nov 06, 2021 at 10:13:15AM +0000, Kieran Bingham wrote: > > Quoting Dmitry Torokhov (2021-11-05 23:04:23) > > > On Fri, Nov 05, 2021 at 12:00:37PM -0500, Jeff LaBundy wrote: > > > > On Fri, Nov 05, 2021 at 10:35:07AM +0000, Kieran Bingham wrote: > > > > > 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... > > > > > 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/ > > > > 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. > 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? 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Input: add 'safe' user switch codes 2021-11-08 11:00 ` Geert Uytterhoeven @ 2021-11-08 12:35 ` Kieran Bingham 2021-11-08 12:41 ` Geert Uytterhoeven 0 siblings, 1 reply; 8+ messages in thread From: Kieran Bingham @ 2021-11-08 12:35 UTC (permalink / raw) To: Geert Uytterhoeven, Jeff LaBundy Cc: Dmitry Torokhov, linux-input, Linux-Renesas, Max Gurtovoy, Hans de Goede, Wu Hao, Bjorn Helgaas, Dan Williams, Dave Ertman, Maximilian Luz, Stephan Gerhold, Xu Yilun, open list Quoting Geert Uytterhoeven (2021-11-08 11:00:20) > Hi Jeff, > > On Sun, Nov 7, 2021 at 7:17 AM Jeff LaBundy <jeff@labundy.com> wrote: > > On Sat, Nov 06, 2021 at 10:13:15AM +0000, Kieran Bingham wrote: > > > Quoting Dmitry Torokhov (2021-11-05 23:04:23) > > > > On Fri, Nov 05, 2021 at 12:00:37PM -0500, Jeff LaBundy wrote: > > > > > On Fri, Nov 05, 2021 at 10:35:07AM +0000, Kieran Bingham wrote: > > > > > > 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... > > > > > > > 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/ > > > > > > 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?). > > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Input: add 'safe' user switch codes 2021-11-08 12:35 ` Kieran Bingham @ 2021-11-08 12:41 ` Geert Uytterhoeven 0 siblings, 0 replies; 8+ messages in thread From: Geert Uytterhoeven @ 2021-11-08 12:41 UTC (permalink / raw) To: Kieran Bingham Cc: Jeff LaBundy, Dmitry Torokhov, linux-input, Linux-Renesas, Max Gurtovoy, Hans de Goede, Wu Hao, Bjorn Helgaas, Dan Williams, Dave Ertman, Maximilian Luz, Stephan Gerhold, Xu Yilun, open list Hi Kieran, On Mon, Nov 8, 2021 at 1:35 PM Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> wrote: > Quoting Geert Uytterhoeven (2021-11-08 11:00:20) > > On Sun, Nov 7, 2021 at 7:17 AM Jeff LaBundy <jeff@labundy.com> wrote: > > > On Sat, Nov 06, 2021 at 10:13:15AM +0000, Kieran Bingham wrote: > > > > Quoting Dmitry Torokhov (2021-11-05 23:04:23) > > > > > On Fri, Nov 05, 2021 at 12:00:37PM -0500, Jeff LaBundy wrote: > > > > > > On Fri, Nov 05, 2021 at 10:35:07AM +0000, Kieran Bingham wrote: > > > > > > > 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... > > > > > > > > > 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/ > > > > > > > > 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. Switches behaving like stuck keys is indeed annoying. I was mainly referring to using KEY_* being innocent for push buttons. > The examples defined in [0] are (IMO) wrong, and have been implemented > incorrectly because of a lack of a generic test switch position. Sure. I do agree the switches should be described as switches using SW_*, not KEY_* events. > 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?). I do think we can change them. 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-11-08 12:42 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-05 10:35 [PATCH v2] Input: add 'safe' user switch codes Kieran Bingham 2021-11-05 17:00 ` Jeff LaBundy 2021-11-05 23:04 ` Dmitry Torokhov 2021-11-06 10:13 ` Kieran Bingham 2021-11-07 6:17 ` Jeff LaBundy 2021-11-08 11:00 ` Geert Uytterhoeven 2021-11-08 12:35 ` Kieran Bingham 2021-11-08 12:41 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).