| Message ID | 20231215024022.122022-1-macroalpha82@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] dt-bindings: input: Clarify that abs_min must be less than abs_max | expand |
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. Cheers, Artur PS. cc'd Paul to the conversation. [1] https://github.com/OpenDingux/linux/blob/jz-6.1/arch/mips/boot/dts/ingenic/gcw0.dts#L273C12-L273C12 [2] https://github.com/libsdl-org/SDL-1.2/commit/46806790ad043 > This property is interpreted as two signed 32 bit values. > > abs-fuzz:
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. > > Cheers, > Artur > > PS. cc'd Paul to the conversation. > > [1] https://github.com/OpenDingux/linux/blob/jz-6.1/arch/mips/boot/dts/ingenic/gcw0.dts#L273C12-L273C12 > [2] https://github.com/libsdl-org/SDL-1.2/commit/46806790ad043 Thank you. Okay, I'll update uinput instead to drop checks for min > max, since that's valid/allowed. Chris > > > This property is interpreted as two signed 32 bit values. > > > > abs-fuzz: >
Sorry, meant to add Peter Hutterer to the conversation, but forgot before hitting send... 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 >
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. This property is interpreted as two signed 32 bit values. abs-fuzz: