[RFC PATCH 0/2] Input: adc-joystick: add detachable devices support

[RFC PATCH 0/2] Input: adc-joystick: add detachable devices support

[Siarhei Volkau]

For detachable or lock-able joysticks the ADC lanes might
be biased to GND or AVDD when the joystick is detached/locked.

One such kind of joystick is found in the Ritmix RZX-50 handheld.
The joystick is non-detachable, although ADC lane biased to power
supply when the "Hold" switch is activated.

To avoid reporting old/broken measurements valid-range is introduced.
When measured value is outside valid-range the driver reports
safe center position for corresponding axis.

Siarhei Volkau (2):
  dt-bindings: adc-joystick: add valid-range
  Input: adc-joystick - add detachable devices support

 .../bindings/input/adc-joystick.yaml          | 62 +++++++++++++++++++
 drivers/input/joystick/adc-joystick.c         | 58 ++++++++++++++++-
 2 files changed, 119 insertions(+), 1 deletion(-)

-- 
2.36.1

[RFC PATCH 1/2] dt-bindings: adc-joystick: add valid-range

[Siarhei Volkau]

The valid-range property aims to cover detachable/lock-able
joysticks where measured value goes outside valid-range.

Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
---
 .../bindings/input/adc-joystick.yaml          | 62 +++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/adc-joystick.yaml b/Documentation/devicetree/bindings/input/adc-joystick.yaml
index da0f8dfca..0483f0afe 100644
--- a/Documentation/devicetree/bindings/input/adc-joystick.yaml
+++ b/Documentation/devicetree/bindings/input/adc-joystick.yaml
@@ -91,6 +91,20 @@ patternProperties:
           Omitting this property indicates the axis always returns to exactly
           the center position.
 
+      valid-range:
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+        items:
+          - description: minimum value
+          - description: maximum value
+        description: >
+          Minimum and maximum values which are still valid for the axis.
+          If the value is outside the range an event won't emitted.
+          Shall fully cover the abs-range with some space added,
+          to take into account imprecise analog nature of the joystick.
+          When value goes out of valid range - corresponding axis is
+          centered. The center point calculated on abs-range basis.
+          This property is interpreted as two signed 32 bit values.
+
     required:
       - reg
       - linux,code
@@ -125,3 +139,51 @@ examples:
               abs-flat = <200>;
       };
     };
+  # A rather complex joystick hardware definition
+  # with two axes muxed into one ADC channel.
+  - |
+    #include <dt-bindings/iio/adc/ingenic,adc.h>
+    #include <dt-bindings/input/input.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    gpio_mux: mux-controller {
+      compatible = "gpio-mux";
+      #mux-control-cells = <0>;
+
+      mux-gpios = <&gpe 2 GPIO_ACTIVE_HIGH>;
+    };
+
+    adcmux: adc-mux {
+      compatible = "io-channel-mux";
+      io-channels = <&adc INGENIC_ADC_AUX>;
+      io-channel-names = "parent";
+      #io-channel-cells = <1>;
+
+      mux-controls = <&gpio_mux>;
+
+      channels = "joystick-axis-x", "joystick-axis-y";
+    };
+
+    joystick: joystick {
+      compatible = "adc-joystick";
+      io-channels = <&adcmux 0>, <&adcmux 1>;
+      io-channel-names = "joystick-axis-x", "joystick-axis-y";
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      poll-interval = <16>;
+
+      axis@0 {
+        reg = <0>;
+        linux,code = <ABS_X>;
+        abs-range = <3400 670>;
+        valid-range = <300 3700>;
+      };
+
+      axis@1 {
+        reg = <1>;
+        linux,code = <ABS_Y>;
+        abs-range = <670 3400>;
+        valid-range = <300 3700>;
+      };
+    };
-- 
2.36.1

Re: [RFC PATCH 1/2] dt-bindings: adc-joystick: add valid-range

[Rob Herring]

On Mon, 31 Oct 2022 22:01:58 +0300, Siarhei Volkau wrote:
> The valid-range property aims to cover detachable/lock-able
> joysticks where measured value goes outside valid-range.
> 
> Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
> ---
>  .../bindings/input/adc-joystick.yaml          | 62 +++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/input/adc-joystick.example.dts:82.28-104.11: ERROR (duplicate_label): /example-1/joystick: Duplicate label 'joystick' on /example-1/joystick and /example-0/adc-joystick
ERROR: Input tree has errors, aborting (use -f to force output)
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/input/adc-joystick.example.dtb] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [RFC PATCH 1/2] dt-bindings: adc-joystick: add valid-range

[Rob Herring]

On Mon, Oct 31, 2022 at 05:05:54PM -0500, Rob Herring wrote:
> 
> On Mon, 31 Oct 2022 22:01:58 +0300, Siarhei Volkau wrote:
> > The valid-range property aims to cover detachable/lock-able
> > joysticks where measured value goes outside valid-range.
> > 
> > Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
> > ---
> >  .../bindings/input/adc-joystick.yaml          | 62 +++++++++++++++++++
> >  1 file changed, 62 insertions(+)
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/input/adc-joystick.example.dts:82.28-104.11: ERROR (duplicate_label): /example-1/joystick: Duplicate label 'joystick' on /example-1/joystick and /example-0/adc-joystick
> ERROR: Input tree has errors, aborting (use -f to force output)
> make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/input/adc-joystick.example.dtb] Error 2
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1492: dt_binding_check] Error 2

The examples aren't completely independent, so you can't use the same 
labels.

Rob

Re: [RFC PATCH 1/2] dt-bindings: adc-joystick: add valid-range

[Siarhei Volkau]

вт, 1 нояб. 2022 г. в 04:08, Rob Herring <robh@kernel.org>:
>
> On Mon, Oct 31, 2022 at 05:05:54PM -0500, Rob Herring wrote:
> >
> > On Mon, 31 Oct 2022 22:01:58 +0300, Siarhei Volkau wrote:
> > > The valid-range property aims to cover detachable/lock-able
> > > joysticks where measured value goes outside valid-range.
> > >
> > > Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
> > > ---
> > >  .../bindings/input/adc-joystick.yaml          | 62 +++++++++++++++++++
> > >  1 file changed, 62 insertions(+)
> > >
> >
> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > Documentation/devicetree/bindings/input/adc-joystick.example.dts:82.28-104.11: ERROR (duplicate_label): /example-1/joystick: Duplicate label 'joystick' on /example-1/joystick and /example-0/adc-joystick
> > ERROR: Input tree has errors, aborting (use -f to force output)
> > make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/input/adc-joystick.example.dtb] Error 2
> > make[1]: *** Waiting for unfinished jobs....
> > make: *** [Makefile:1492: dt_binding_check] Error 2
>
> The examples aren't completely independent, so you can't use the same
> labels.

Good explanation, thank you.

[RFC PATCH 2/2] Input: adc-joystick - add detachable devices support

[Siarhei Volkau]

For detachable or lock-able joysticks the ADC lanes might
be biased to GND or AVDD when the joystick is detached/locked.

One such kind of joystick is found in the Ritmix RZX-50 device.
The joystick is non-detachable, although ADC lane biased to power
supply when the "Hold" switch is activated.

To avoid reporting old/broken measurements valid-range is used.
When measured value is outside valid-range the driver reports
safe center position for corresponding axis.

Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
---
 drivers/input/joystick/adc-joystick.c | 58 ++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/input/joystick/adc-joystick.c b/drivers/input/joystick/adc-joystick.c
index c0deff5d4..6a143dc38 100644
--- a/drivers/input/joystick/adc-joystick.c
+++ b/drivers/input/joystick/adc-joystick.c
@@ -18,6 +18,8 @@ struct adc_joystick_axis {
 	s32 range[2];
 	s32 fuzz;
 	s32 flat;
+	s32 valid[2];
+	s32 center;
 };
 
 struct adc_joystick {
@@ -29,6 +31,14 @@ struct adc_joystick {
 	bool polled;
 };
 
+static inline bool is_in_range(const s32 *range, int val)
+{
+	s32 min = range[0];
+	s32 max = range[1];
+
+	return (min == 0 && max == 0) || (val >= min && val <= max);
+}
+
 static void adc_joystick_poll(struct input_dev *input)
 {
 	struct adc_joystick *joy = input_get_drvdata(input);
@@ -38,6 +48,10 @@ static void adc_joystick_poll(struct input_dev *input)
 		ret = iio_read_channel_raw(&joy->chans[i], &val);
 		if (ret < 0)
 			return;
+
+		if (!is_in_range(joy->axes[i].valid, val))
+			val = joy->axes[i].center;
+
 		input_report_abs(input, joy->axes[i].code, val);
 	}
 	input_sync(input);
@@ -86,6 +100,10 @@ static int adc_joystick_handle(const void *data, void *private)
 			val = sign_extend32(val, msb);
 		else
 			val &= GENMASK(msb, 0);
+
+		if (!is_in_range(joy->axes[i].valid, val))
+			val = joy->axes[i].center;
+
 		input_report_abs(joy->input, joy->axes[i].code, val);
 	}
 
@@ -119,6 +137,21 @@ static void adc_joystick_cleanup(void *data)
 	iio_channel_release_all_cb(data);
 }
 
+static bool valid_range_covers_abs_range(struct adc_joystick_axis *axis)
+{
+	s32 abs_min, abs_max;
+
+	if (axis->range[0] > axis->range[1]) {
+		abs_min = axis->range[1];
+		abs_max = axis->range[0];
+	} else {
+		abs_min = axis->range[0];
+		abs_max = axis->range[1];
+	}
+
+	return axis->valid[0] <= abs_min && axis->valid[1] >= abs_max;
+}
+
 static int adc_joystick_set_axes(struct device *dev, struct adc_joystick *joy)
 {
 	struct adc_joystick_axis *axes;
@@ -137,7 +170,7 @@ static int adc_joystick_set_axes(struct device *dev, struct adc_joystick *joy)
 		return -EINVAL;
 	}
 
-	axes = devm_kmalloc_array(dev, num_axes, sizeof(*axes), GFP_KERNEL);
+	axes = devm_kcalloc(dev, num_axes, sizeof(*axes), GFP_KERNEL);
 	if (!axes)
 		return -ENOMEM;
 
@@ -167,10 +200,33 @@ static int adc_joystick_set_axes(struct device *dev, struct adc_joystick *joy)
 			dev_err(dev, "abs-range invalid or missing\n");
 			goto err_fwnode_put;
 		}
+		axes[i].center = (axes[i].range[0] + axes[i].range[1]) / 2;
 
 		fwnode_property_read_u32(child, "abs-fuzz", &axes[i].fuzz);
 		fwnode_property_read_u32(child, "abs-flat", &axes[i].flat);
 
+		if (fwnode_property_present(child, "valid-range")) {
+			error = fwnode_property_read_u32_array(child,
+						       "valid-range",
+						       axes[i].valid, 2);
+			if (error) {
+				dev_err(dev, "valid-range invalid\n");
+				goto err_fwnode_put;
+			}
+
+			if (axes[i].valid[0] > axes[i].valid[1]) {
+				dev_err(dev,
+					"valid-range invalid (min > max)\n");
+				goto err_fwnode_put;
+			}
+
+			if (!valid_range_covers_abs_range(&axes[i])) {
+				dev_err(dev,
+					"valid-range must cover abs-range\n");
+				goto err_fwnode_put;
+			}
+		}
+
 		input_set_abs_params(joy->input, axes[i].code,
 				     axes[i].range[0], axes[i].range[1],
 				     axes[i].fuzz, axes[i].flat);
-- 
2.36.1

Re: [RFC PATCH 0/2] Input: adc-joystick: add detachable devices support

[Paul Cercueil]

Hi Siarhei,

Le lun. 31 oct. 2022 à 22:01:57 +0300, Siarhei Volkau 
<lis8215@gmail.com> a écrit :
> For detachable or lock-able joysticks the ADC lanes might
> be biased to GND or AVDD when the joystick is detached/locked.
> 
> One such kind of joystick is found in the Ritmix RZX-50 handheld.
> The joystick is non-detachable, although ADC lane biased to power
> supply when the "Hold" switch is activated.

But the RZX-50 has no joystick...
Or is the d-pad actually wired to the ADC instead of GPIOs?

> To avoid reporting old/broken measurements valid-range is introduced.
> When measured value is outside valid-range the driver reports
> safe center position for corresponding axis.

First of all, you already have a "valid range", it is called 
"abs-range"; no need for a new one.

Then, the driver has no business doing events filtering. Notice that 
when you activate the "hold" button and your joystick values go way 
off-range, you still get input events in userspace: that's because the 
kernel is not responsible for enforcing the deadzone, the userspace is.

In your case, you need to update your userspace applications/libraries 
so that when the joystick values are way off-range, the assumed 
position is the center.

Cheers,
-Paul

Re: [RFC PATCH 0/2] Input: adc-joystick: add detachable devices support

[Siarhei Volkau]

вс, 6 нояб. 2022 г. в 02:39, Paul Cercueil <paul@crapouillou.net>:
>
> Hi Siarhei,
>
> Le lun. 31 oct. 2022 à 22:01:57 +0300, Siarhei Volkau
> <lis8215@gmail.com> a écrit :
> > For detachable or lock-able joysticks the ADC lanes might
> > be biased to GND or AVDD when the joystick is detached/locked.
> >
> > One such kind of joystick is found in the Ritmix RZX-50 handheld.
> > The joystick is non-detachable, although ADC lane biased to power
> > supply when the "Hold" switch is activated.
>
> But the RZX-50 has no joystick...

Well, actually there's two versions in the wild (google "RZX-50 pictures"):
 - with analog joystick and speakers on the back side
 - without the joystick and speakers on the front side
I have only the first one at the moment, but I'm looking for another one.

> Or is the d-pad actually wired to the ADC instead of GPIOs?

The D-Pad is another kind of pain there - it's a part of the matrix-keypad
but the pad's row line is shared with the LCD HSYNC signal.

> > To avoid reporting old/broken measurements valid-range is introduced.
> > When measured value is outside valid-range the driver reports
> > safe center position for corresponding axis.
>
> First of all, you already have a "valid range", it is called
> "abs-range"; no need for a new one.
>
> Then, the driver has no business doing events filtering. Notice that
> when you activate the "hold" button and your joystick values go way
> off-range, you still get input events in userspace: that's because the
> kernel is not responsible for enforcing the deadzone, the userspace is.
>
> In your case, you need to update your userspace applications/libraries
> so that when the joystick values are way off-range, the assumed
> position is the center.

Many userspace apps use SDL library to handle joystick input, the SDL
unfortunately hides the fact that the joystick position is out of range -
it does normalization and clamping of the abs-range to an int16_t range.

I see two approaches to do that in userspace:
1. Make a quirk in the SDL library, which affects many types of joysticks
and lead to a fragmentation (special userspace apps for every hardware
isn't the right way in my opinion).
2. Make a special filter app which will filter out invalid joystick events and
route rest to the /dev/input/uinput. Not sure how to hide the real joystick
device from libraries like SDL then.
Both of them don't look like a "silver bullet", however feasible.

>
> Cheers,
> -Paul
>
>

Thank you.

Re: [RFC PATCH 0/2] Input: adc-joystick: add detachable devices support

[Siarhei Volkau]

> Not sure how to hide the real joystick device from libraries like SDL

Got it, EVIOCGRAB ioctl shall do the job.

Thank you all !

BR,
Siarhei