Re: [RFC] dt-bindings: input: Clarify that abs_min must be less than abs_max

[Peter Hutterer <peter.hutterer@who-t.net>]

On Tue, Dec 19, 2023 at 12:34:51PM -0800, Dmitry Torokhov wrote:
> Sorry, meant to add Peter Hutterer to the conversation, but forgot
> before hitting send...

Thx for the CC, I only saw the other patch and had missed this one. 
 
> On Tue, Dec 19, 2023 at 12:32:44PM -0800, Dmitry Torokhov wrote:
> > On Fri, Dec 15, 2023 at 12:19:51PM +0100, Artur Rojek wrote:
> > > On 2023-12-15 03:40, Chris Morgan wrote:
> > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > 
> > > > uinput refuses to work with abs devices where the min value is greater
> > > > than the max value. uinput_validate_absinfo() returns -EINVAL if this
> > > > is the case and prevents using uinput on such a device. Since uinput
> > > > has worked this way since at least kernel 2.6 (or prior) I presume that
> > > > this is the correct way of doing things, and that this documentation
> > > > needs to be clarified that min must always be less than max.
> > > > 
> > > > uinput is used in my use case to bind together adc-joystick devices
> > > > with gpio-keys devices to create a single unified gamepad for
> > > > userspace.
> > > > 
> > > > Note that there are several boards that will need to be corrected,
> > > > all but a few of them I maintain. Submitting as an RFC for now to get
> > > > comments from the input team and the original author in case there is
> > > > something I am missing.
> > > > 
> > > > Fixes: 7956b0d4694f ("dt-bindings: input: Add docs for ADC driven
> > > > joystick")
> > > > 
> > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/input/adc-joystick.yaml | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > > > b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > > > index 6c244d66f8ce..8f5cdd5ef190 100644
> > > > --- a/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > > > +++ b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > > > @@ -73,8 +73,9 @@ patternProperties:
> > > >          description: >
> > > >            Minimum and maximum values produced by the axis.
> > > >            For an ABS_X axis this will be the left-most and right-most
> > > > -          inclination of the joystick. If min > max, it is left to
> > > > userspace to
> > > > -          treat the axis as inverted.
> > > > +          inclination of the joystick. The axis must always be
> > > > expressed as
> > > > +          min < max, if the axis is inverted it is left to userspace to
> > > > handle
> > > > +          the inversion.
> > > 
> > > Hi Chris,
> > > 
> > > Device Tree is supposed to depict the actual state of the hardware.
> > > I worded the adc-joytick's adc-range property specifically, so that it
> > > covers a case of GCW Zero hardware [1], which has a joystick,  where the
> > > ABS_X axis reports increasing values for the left-wards inclination of
> > > the joystick, and decreasing values for the right-wards inclination. You
> > > are saying that there are even more boards that need to be corrected -
> > > those are all situations, where DT depicts the actual behavior of the
> > > hardware.
> > > What you are trying to do is change hardware description, because of how
> > > a driver in an OS works. You should instead fix behavior of said driver,
> > > even if nobody stumbled upon that issue since 2.6 :) We fixed libSDL [2]
> > > for the same reason.
> > 
> > We have several places in the kernel (such as mousedev and joydev) where
> > we expect that max is greater or equal to min if they are specified. I
> > am sure that at least some userspace components also have this
> > assumption. In general, we expect min to be a minimum value reported and
> > max being maximum value reported, and orientation expressed via
> > different properties (see [1]).
> > 
> > Since we codified min > max as inversion for adc-joystick devices in the
> > bindings, I think we need to handle this *in that driver* and leave the
> > rest alone.
> > 
> > [1] Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml

I quickly checked libevdev and libinput and neither of them have checks
for min < max but the base assumption is there. So we'll get
entertaining results if that stops being the case.

Cheers,
  Peter