[PATCH v2 0/3] adc-joystick: Add polled support

[PATCH v2 0/3] adc-joystick: Add polled support

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add support to the existing adc-joystick driver to support polling
rather than relying on triggered buffers. This is useful for devices
that do not offer triggered buffers in hardware. Code adapted from
changes made by Maya Matuszczyk <maccraft123mc@gmail.com>.

Changes from V1:
 - Removed driver compatible string of "adc-joystick-polled".
 - Added new optional boolean value of "adc-joystick,polled".
 - Cleaned up if statements regarding polling behavior.

Chris Morgan (3):
  dt-bindings: adc-joystick: add adc-joystick,polled option
  Input: adc-joystick - Add polled input device support
  arm64: dts: rockchip: Update joystick to polled for Odroid-Go2

 .../bindings/input/adc-joystick.yaml          |  9 +++-
 .../boot/dts/rockchip/rk3326-odroid-go2.dts   |  1 +
 drivers/input/joystick/adc-joystick.c         | 52 +++++++++++++++----
 3 files changed, 50 insertions(+), 12 deletions(-)

-- 
2.25.1

[PATCH v2 1/3] dt-bindings: adc-joystick: add adc-joystick,polled option

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add documentation for adc-joystick,polled. New device-tree properties
have been added.

- adc-joystick,polled: A boolean value noting the joystick device
		       should be polled rather than use a triggered
		       buffer.

Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 .../devicetree/bindings/input/adc-joystick.yaml          | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/adc-joystick.yaml b/Documentation/devicetree/bindings/input/adc-joystick.yaml
index 2ee04e03bc22..4f49a1a5772e 100644
--- a/Documentation/devicetree/bindings/input/adc-joystick.yaml
+++ b/Documentation/devicetree/bindings/input/adc-joystick.yaml
@@ -12,12 +12,19 @@ maintainers:
 
 description: >
   Bindings for joystick devices connected to ADC controllers supporting
-  the Industrial I/O subsystem.
+  the Industrial I/O subsystem. Supports both polled devices where no
+  iio trigger is available and non-polled devices which are triggered
+  by iio.
 
 properties:
   compatible:
     const: adc-joystick
 
+  adc-joystick,polled:
+    type: boolean
+    description:
+      If the device does not support triggered buffers and needs to be polled.
+
   io-channels:
     minItems: 1
     maxItems: 1024
-- 
2.25.1

[PATCH v2 2/3] Input: adc-joystick - Add polled input device support

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add polled input device support to the adc-joystick driver. This is
useful for devices which do not have hardware capable triggers on
their SARADC. Code modified from adc-joystick.c changes made by Maya
Matuszczyk.

Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/input/joystick/adc-joystick.c | 52 +++++++++++++++++++++------
 1 file changed, 41 insertions(+), 11 deletions(-)

diff --git a/drivers/input/joystick/adc-joystick.c b/drivers/input/joystick/adc-joystick.c
index 78ebca7d400a..5a28fe7b8ebc 100644
--- a/drivers/input/joystick/adc-joystick.c
+++ b/drivers/input/joystick/adc-joystick.c
@@ -13,6 +13,10 @@
 
 #include <asm/unaligned.h>
 
+#define ADC_JSK_POLL_INTERVAL	16
+#define ADC_JSK_POLL_MIN	8
+#define ADC_JSK_POLL_MAX	32
+
 struct adc_joystick_axis {
 	u32 code;
 	s32 range[2];
@@ -26,8 +30,21 @@ struct adc_joystick {
 	struct adc_joystick_axis *axes;
 	struct iio_channel *chans;
 	int num_chans;
+	bool polled;
 };
 
+static void adc_joystick_poll(struct input_dev *input)
+{
+	struct adc_joystick *joy = input_get_drvdata(input);
+	int i, val;
+
+	for (i = 0; i < joy->num_chans; i++) {
+		iio_read_channel_raw(&joy->chans[i], &val);
+		input_report_abs(input, joy->axes[i].code, val);
+	}
+	input_sync(input);
+}
+
 static int adc_joystick_handle(const void *data, void *private)
 {
 	struct adc_joystick *joy = private;
@@ -215,8 +232,19 @@ static int adc_joystick_probe(struct platform_device *pdev)
 	joy->input = input;
 	input->name = pdev->name;
 	input->id.bustype = BUS_HOST;
-	input->open = adc_joystick_open;
-	input->close = adc_joystick_close;
+
+	if (device_property_read_bool(dev, "adc-joystick,polled"))
+		joy->polled = 1;
+
+	if (joy->polled) {
+		input_setup_polling(input, adc_joystick_poll);
+		input_set_poll_interval(input, ADC_JSK_POLL_INTERVAL);
+		input_set_min_poll_interval(input, ADC_JSK_POLL_MIN);
+		input_set_max_poll_interval(input, ADC_JSK_POLL_MAX);
+	} else {
+		input->open = adc_joystick_open;
+		input->close = adc_joystick_close;
+	}
 
 	error = adc_joystick_set_axes(dev, joy);
 	if (error)
@@ -229,16 +257,18 @@ static int adc_joystick_probe(struct platform_device *pdev)
 		return error;
 	}
 
-	joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, joy);
-	if (IS_ERR(joy->buffer)) {
-		dev_err(dev, "Unable to allocate callback buffer\n");
-		return PTR_ERR(joy->buffer);
-	}
+	if (!joy->polled) {
+		joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, joy);
+		if (IS_ERR(joy->buffer)) {
+			dev_err(dev, "Unable to allocate callback buffer\n");
+			return PTR_ERR(joy->buffer);
+		}
 
-	error = devm_add_action_or_reset(dev, adc_joystick_cleanup, joy->buffer);
-	if (error)  {
-		dev_err(dev, "Unable to add action\n");
-		return error;
+		error = devm_add_action_or_reset(dev, adc_joystick_cleanup, joy->buffer);
+		if (error)  {
+			dev_err(dev, "Unable to add action\n");
+			return error;
+		}
 	}
 
 	return 0;
-- 
2.25.1

[PATCH v2 3/3] arm64: dts: rockchip: Update joystick to polled for Odroid-Go2

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Update the Odroid Go Advance to use the polled method from the
adc-joystick driver.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts b/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
index ea0695b51ecd..ae714ff71e84 100644
--- a/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
@@ -24,6 +24,7 @@ chosen {
 
 	adc-joystick {
 		compatible = "adc-joystick";
+		adc-joystick,polled;
 		io-channels = <&saradc 1>,
 			      <&saradc 2>;
 		#address-cells = <1>;
-- 
2.25.1

Re: [PATCH v2 2/3] Input: adc-joystick - Add polled input device support

[Dmitry Torokhov]

On Wed, Jun 01, 2022 at 03:49:26PM -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add polled input device support to the adc-joystick driver. This is
> useful for devices which do not have hardware capable triggers on
> their SARADC. Code modified from adc-joystick.c changes made by Maya
> Matuszczyk.

I'd like to hear Jonathan's take on this...

> 
> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  drivers/input/joystick/adc-joystick.c | 52 +++++++++++++++++++++------
>  1 file changed, 41 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/input/joystick/adc-joystick.c b/drivers/input/joystick/adc-joystick.c
> index 78ebca7d400a..5a28fe7b8ebc 100644
> --- a/drivers/input/joystick/adc-joystick.c
> +++ b/drivers/input/joystick/adc-joystick.c
> @@ -13,6 +13,10 @@
>  
>  #include <asm/unaligned.h>
>  
> +#define ADC_JSK_POLL_INTERVAL	16
> +#define ADC_JSK_POLL_MIN	8
> +#define ADC_JSK_POLL_MAX	32
> +
>  struct adc_joystick_axis {
>  	u32 code;
>  	s32 range[2];
> @@ -26,8 +30,21 @@ struct adc_joystick {
>  	struct adc_joystick_axis *axes;
>  	struct iio_channel *chans;
>  	int num_chans;
> +	bool polled;
>  };
>  
> +static void adc_joystick_poll(struct input_dev *input)
> +{
> +	struct adc_joystick *joy = input_get_drvdata(input);
> +	int i, val;
> +
> +	for (i = 0; i < joy->num_chans; i++) {
> +		iio_read_channel_raw(&joy->chans[i], &val);
> +		input_report_abs(input, joy->axes[i].code, val);
> +	}
> +	input_sync(input);
> +}
> +
>  static int adc_joystick_handle(const void *data, void *private)
>  {
>  	struct adc_joystick *joy = private;
> @@ -215,8 +232,19 @@ static int adc_joystick_probe(struct platform_device *pdev)
>  	joy->input = input;
>  	input->name = pdev->name;
>  	input->id.bustype = BUS_HOST;
> -	input->open = adc_joystick_open;
> -	input->close = adc_joystick_close;
> +
> +	if (device_property_read_bool(dev, "adc-joystick,polled"))
> +		joy->polled = 1;
> +
> +	if (joy->polled) {
> +		input_setup_polling(input, adc_joystick_poll);
> +		input_set_poll_interval(input, ADC_JSK_POLL_INTERVAL);
> +		input_set_min_poll_interval(input, ADC_JSK_POLL_MIN);
> +		input_set_max_poll_interval(input, ADC_JSK_POLL_MAX);
> +	} else {
> +		input->open = adc_joystick_open;
> +		input->close = adc_joystick_close;
> +	}
>  
>  	error = adc_joystick_set_axes(dev, joy);
>  	if (error)
> @@ -229,16 +257,18 @@ static int adc_joystick_probe(struct platform_device *pdev)
>  		return error;
>  	}
>  
> -	joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, joy);
> -	if (IS_ERR(joy->buffer)) {
> -		dev_err(dev, "Unable to allocate callback buffer\n");
> -		return PTR_ERR(joy->buffer);
> -	}
> +	if (!joy->polled) {
> +		joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, joy);
> +		if (IS_ERR(joy->buffer)) {
> +			dev_err(dev, "Unable to allocate callback buffer\n");
> +			return PTR_ERR(joy->buffer);
> +		}
>  
> -	error = devm_add_action_or_reset(dev, adc_joystick_cleanup, joy->buffer);
> -	if (error)  {
> -		dev_err(dev, "Unable to add action\n");
> -		return error;
> +		error = devm_add_action_or_reset(dev, adc_joystick_cleanup, joy->buffer);
> +		if (error)  {
> +			dev_err(dev, "Unable to add action\n");
> +			return error;
> +		}
>  	}
>  
>  	return 0;
> -- 
> 2.25.1
> 

-- 
Dmitry

Re: [PATCH v2 2/3] Input: adc-joystick - Add polled input device support

[Artur Rojek]

On 2022-06-01 22:49, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add polled input device support to the adc-joystick driver. This is
> useful for devices which do not have hardware capable triggers on
> their SARADC. Code modified from adc-joystick.c changes made by Maya
> Matuszczyk.
> 
> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  drivers/input/joystick/adc-joystick.c | 52 +++++++++++++++++++++------
>  1 file changed, 41 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/input/joystick/adc-joystick.c
> b/drivers/input/joystick/adc-joystick.c
> index 78ebca7d400a..5a28fe7b8ebc 100644
> --- a/drivers/input/joystick/adc-joystick.c
> +++ b/drivers/input/joystick/adc-joystick.c
> @@ -13,6 +13,10 @@
> 
>  #include <asm/unaligned.h>
> 
> +#define ADC_JSK_POLL_INTERVAL	16
> +#define ADC_JSK_POLL_MIN	8
> +#define ADC_JSK_POLL_MAX	32
> +
>  struct adc_joystick_axis {
>  	u32 code;
>  	s32 range[2];
> @@ -26,8 +30,21 @@ struct adc_joystick {
>  	struct adc_joystick_axis *axes;
>  	struct iio_channel *chans;
>  	int num_chans;
> +	bool polled;
>  };
> 
> +static void adc_joystick_poll(struct input_dev *input)
> +{
> +	struct adc_joystick *joy = input_get_drvdata(input);
> +	int i, val;
> +
> +	for (i = 0; i < joy->num_chans; i++) {
> +		iio_read_channel_raw(&joy->chans[i], &val);
> +		input_report_abs(input, joy->axes[i].code, val);
> +	}
> +	input_sync(input);
> +}
> +
>  static int adc_joystick_handle(const void *data, void *private)
>  {
>  	struct adc_joystick *joy = private;
> @@ -215,8 +232,19 @@ static int adc_joystick_probe(struct 
> platform_device *pdev)
>  	joy->input = input;
>  	input->name = pdev->name;
>  	input->id.bustype = BUS_HOST;
> -	input->open = adc_joystick_open;
> -	input->close = adc_joystick_close;
> +
> +	if (device_property_read_bool(dev, "adc-joystick,polled"))
> +		joy->polled = 1;
No need for branching, you can simply do:
	joy->polled = device_property_read_bool(dev, "adc-joystick,polled");

> +
> +	if (joy->polled) {
> +		input_setup_polling(input, adc_joystick_poll);
> +		input_set_poll_interval(input, ADC_JSK_POLL_INTERVAL);
> +		input_set_min_poll_interval(input, ADC_JSK_POLL_MIN);
> +		input_set_max_poll_interval(input, ADC_JSK_POLL_MAX);
> +	} else {
> +		input->open = adc_joystick_open;
> +		input->close = adc_joystick_close;
> +	}
> 
>  	error = adc_joystick_set_axes(dev, joy);
>  	if (error)
> @@ -229,16 +257,18 @@ static int adc_joystick_probe(struct
> platform_device *pdev)
>  		return error;
>  	}
> 
> -	joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, joy);
> -	if (IS_ERR(joy->buffer)) {
> -		dev_err(dev, "Unable to allocate callback buffer\n");
> -		return PTR_ERR(joy->buffer);
> -	}
> +	if (!joy->polled) {
> +		joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, joy);
> +		if (IS_ERR(joy->buffer)) {
> +			dev_err(dev, "Unable to allocate callback buffer\n");
> +			return PTR_ERR(joy->buffer);
> +		}
> 
> -	error = devm_add_action_or_reset(dev, adc_joystick_cleanup, 
> joy->buffer);
> -	if (error)  {
> -		dev_err(dev, "Unable to add action\n");
> -		return error;
> +		error = devm_add_action_or_reset(dev, adc_joystick_cleanup, 
> joy->buffer);
> +		if (error)  {
> +			dev_err(dev, "Unable to add action\n");
> +			return error;
> +		}
>  	}
> 
>  	return 0;

Re: [PATCH v2 2/3] Input: adc-joystick - Add polled input device support

[Paul Cercueil]

Hi Chris,

Le mer., juin 1 2022 at 15:49:26 -0500, Chris Morgan 
<macroalpha82@gmail.com> a écrit :
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add polled input device support to the adc-joystick driver. This is
> useful for devices which do not have hardware capable triggers on
> their SARADC. Code modified from adc-joystick.c changes made by Maya
> Matuszczyk.

I understand what you're trying to do, but maybe there's a better way.

It would be better if pollable IIO devices would also be able to be 
used with a buffer, with the client driver manually triggering the IIO 
device through a function call.

Jonathan (Cc'd) might have his reasons why this isn't done this way, 
and in this case your patch makes sense, but I'd like to hear 
Jonathan's thoughts first.

As for "adc-joystick,polled"... it doesn't describe the hardware but 
rather a specific configuration, so it shouldn't appear in the device 
tree.

Cheers,
-Paul

> 
> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  drivers/input/joystick/adc-joystick.c | 52 
> +++++++++++++++++++++------
>  1 file changed, 41 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/input/joystick/adc-joystick.c 
> b/drivers/input/joystick/adc-joystick.c
> index 78ebca7d400a..5a28fe7b8ebc 100644
> --- a/drivers/input/joystick/adc-joystick.c
> +++ b/drivers/input/joystick/adc-joystick.c
> @@ -13,6 +13,10 @@
> 
>  #include <asm/unaligned.h>
> 
> +#define ADC_JSK_POLL_INTERVAL	16
> +#define ADC_JSK_POLL_MIN	8
> +#define ADC_JSK_POLL_MAX	32
> +
>  struct adc_joystick_axis {
>  	u32 code;
>  	s32 range[2];
> @@ -26,8 +30,21 @@ struct adc_joystick {
>  	struct adc_joystick_axis *axes;
>  	struct iio_channel *chans;
>  	int num_chans;
> +	bool polled;
>  };
> 
> +static void adc_joystick_poll(struct input_dev *input)
> +{
> +	struct adc_joystick *joy = input_get_drvdata(input);
> +	int i, val;
> +
> +	for (i = 0; i < joy->num_chans; i++) {
> +		iio_read_channel_raw(&joy->chans[i], &val);
> +		input_report_abs(input, joy->axes[i].code, val);
> +	}
> +	input_sync(input);
> +}
> +
>  static int adc_joystick_handle(const void *data, void *private)
>  {
>  	struct adc_joystick *joy = private;
> @@ -215,8 +232,19 @@ static int adc_joystick_probe(struct 
> platform_device *pdev)
>  	joy->input = input;
>  	input->name = pdev->name;
>  	input->id.bustype = BUS_HOST;
> -	input->open = adc_joystick_open;
> -	input->close = adc_joystick_close;
> +
> +	if (device_property_read_bool(dev, "adc-joystick,polled"))
> +		joy->polled = 1;
> +
> +	if (joy->polled) {
> +		input_setup_polling(input, adc_joystick_poll);
> +		input_set_poll_interval(input, ADC_JSK_POLL_INTERVAL);
> +		input_set_min_poll_interval(input, ADC_JSK_POLL_MIN);
> +		input_set_max_poll_interval(input, ADC_JSK_POLL_MAX);
> +	} else {
> +		input->open = adc_joystick_open;
> +		input->close = adc_joystick_close;
> +	}
> 
>  	error = adc_joystick_set_axes(dev, joy);
>  	if (error)
> @@ -229,16 +257,18 @@ static int adc_joystick_probe(struct 
> platform_device *pdev)
>  		return error;
>  	}
> 
> -	joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, joy);
> -	if (IS_ERR(joy->buffer)) {
> -		dev_err(dev, "Unable to allocate callback buffer\n");
> -		return PTR_ERR(joy->buffer);
> -	}
> +	if (!joy->polled) {
> +		joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, 
> joy);
> +		if (IS_ERR(joy->buffer)) {
> +			dev_err(dev, "Unable to allocate callback buffer\n");
> +			return PTR_ERR(joy->buffer);
> +		}
> 
> -	error = devm_add_action_or_reset(dev, adc_joystick_cleanup, 
> joy->buffer);
> -	if (error)  {
> -		dev_err(dev, "Unable to add action\n");
> -		return error;
> +		error = devm_add_action_or_reset(dev, adc_joystick_cleanup, 
> joy->buffer);
> +		if (error)  {
> +			dev_err(dev, "Unable to add action\n");
> +			return error;
> +		}
>  	}
> 
>  	return 0;

Re: [PATCH v2 1/3] dt-bindings: adc-joystick: add adc-joystick,polled option

[Krzysztof Kozlowski]

On 01/06/2022 22:49, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add documentation for adc-joystick,polled. New device-tree properties
> have been added.
> 
> - adc-joystick,polled: A boolean value noting the joystick device
> 		       should be polled rather than use a triggered
> 		       buffer.
> 
> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  .../devicetree/bindings/input/adc-joystick.yaml          | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/adc-joystick.yaml b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> index 2ee04e03bc22..4f49a1a5772e 100644
> --- a/Documentation/devicetree/bindings/input/adc-joystick.yaml
> +++ b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> @@ -12,12 +12,19 @@ maintainers:
>  
>  description: >
>    Bindings for joystick devices connected to ADC controllers supporting
> -  the Industrial I/O subsystem.
> +  the Industrial I/O subsystem. Supports both polled devices where no
> +  iio trigger is available and non-polled devices which are triggered
> +  by iio.
>  
>  properties:
>    compatible:
>      const: adc-joystick
>  
> +  adc-joystick,polled:
> +    type: boolean
> +    description:
> +      If the device does not support triggered buffers and needs to be polled.

You described desired Linux feature or behavior, not the actual
hardware. The bindings are about the latter, so instead you need to
rephrase the property and it's description to match actual hardware
capabilities/features/configuration etc.


Best regards,
Krzysztof

Re: [PATCH v2 2/3] Input: adc-joystick - Add polled input device support

[Heiko Stübner]

Am Mittwoch, 1. Juni 2022, 22:49:26 CEST schrieb Chris Morgan:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add polled input device support to the adc-joystick driver. This is
> useful for devices which do not have hardware capable triggers on
> their SARADC. Code modified from adc-joystick.c changes made by Maya
> Matuszczyk.

as the dts patch mentiones the odroid go2, this probably means the
Rockchip-saradc?

I very much remember that we converted the saradc to also support
triggered buffers [0] two years ago, and as I remember that worked
rather nicely on the Go-1 at least - similar hardware I think.

So it would be really helpful to provide some more explanation on
why this isn't enough for your use-case.


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


> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  drivers/input/joystick/adc-joystick.c | 52 +++++++++++++++++++++------
>  1 file changed, 41 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/input/joystick/adc-joystick.c b/drivers/input/joystick/adc-joystick.c
> index 78ebca7d400a..5a28fe7b8ebc 100644
> --- a/drivers/input/joystick/adc-joystick.c
> +++ b/drivers/input/joystick/adc-joystick.c
> @@ -13,6 +13,10 @@
>  
>  #include <asm/unaligned.h>
>  
> +#define ADC_JSK_POLL_INTERVAL	16
> +#define ADC_JSK_POLL_MIN	8
> +#define ADC_JSK_POLL_MAX	32
> +
>  struct adc_joystick_axis {
>  	u32 code;
>  	s32 range[2];
> @@ -26,8 +30,21 @@ struct adc_joystick {
>  	struct adc_joystick_axis *axes;
>  	struct iio_channel *chans;
>  	int num_chans;
> +	bool polled;
>  };
>  
> +static void adc_joystick_poll(struct input_dev *input)
> +{
> +	struct adc_joystick *joy = input_get_drvdata(input);
> +	int i, val;
> +
> +	for (i = 0; i < joy->num_chans; i++) {
> +		iio_read_channel_raw(&joy->chans[i], &val);
> +		input_report_abs(input, joy->axes[i].code, val);
> +	}
> +	input_sync(input);
> +}
> +
>  static int adc_joystick_handle(const void *data, void *private)
>  {
>  	struct adc_joystick *joy = private;
> @@ -215,8 +232,19 @@ static int adc_joystick_probe(struct platform_device *pdev)
>  	joy->input = input;
>  	input->name = pdev->name;
>  	input->id.bustype = BUS_HOST;
> -	input->open = adc_joystick_open;
> -	input->close = adc_joystick_close;
> +
> +	if (device_property_read_bool(dev, "adc-joystick,polled"))
> +		joy->polled = 1;
> +
> +	if (joy->polled) {
> +		input_setup_polling(input, adc_joystick_poll);
> +		input_set_poll_interval(input, ADC_JSK_POLL_INTERVAL);
> +		input_set_min_poll_interval(input, ADC_JSK_POLL_MIN);
> +		input_set_max_poll_interval(input, ADC_JSK_POLL_MAX);
> +	} else {
> +		input->open = adc_joystick_open;
> +		input->close = adc_joystick_close;
> +	}
>  
>  	error = adc_joystick_set_axes(dev, joy);
>  	if (error)
> @@ -229,16 +257,18 @@ static int adc_joystick_probe(struct platform_device *pdev)
>  		return error;
>  	}
>  
> -	joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, joy);
> -	if (IS_ERR(joy->buffer)) {
> -		dev_err(dev, "Unable to allocate callback buffer\n");
> -		return PTR_ERR(joy->buffer);
> -	}
> +	if (!joy->polled) {
> +		joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, joy);
> +		if (IS_ERR(joy->buffer)) {
> +			dev_err(dev, "Unable to allocate callback buffer\n");
> +			return PTR_ERR(joy->buffer);
> +		}
>  
> -	error = devm_add_action_or_reset(dev, adc_joystick_cleanup, joy->buffer);
> -	if (error)  {
> -		dev_err(dev, "Unable to add action\n");
> -		return error;
> +		error = devm_add_action_or_reset(dev, adc_joystick_cleanup, joy->buffer);
> +		if (error)  {
> +			dev_err(dev, "Unable to add action\n");
> +			return error;
> +		}
>  	}
>  
>  	return 0;
> 

Re: [PATCH v2 1/3] dt-bindings: adc-joystick: add adc-joystick,polled option

[Chris Morgan]

On Thu, Jun 02, 2022 at 11:56:10AM +0200, Krzysztof Kozlowski wrote:
> On 01/06/2022 22:49, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Add documentation for adc-joystick,polled. New device-tree properties
> > have been added.
> > 
> > - adc-joystick,polled: A boolean value noting the joystick device
> > 		       should be polled rather than use a triggered
> > 		       buffer.
> > 
> > Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> >  .../devicetree/bindings/input/adc-joystick.yaml          | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/input/adc-joystick.yaml b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > index 2ee04e03bc22..4f49a1a5772e 100644
> > --- a/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > +++ b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > @@ -12,12 +12,19 @@ maintainers:
> >  
> >  description: >
> >    Bindings for joystick devices connected to ADC controllers supporting
> > -  the Industrial I/O subsystem.
> > +  the Industrial I/O subsystem. Supports both polled devices where no
> > +  iio trigger is available and non-polled devices which are triggered
> > +  by iio.
> >  
> >  properties:
> >    compatible:
> >      const: adc-joystick
> >  
> > +  adc-joystick,polled:
> > +    type: boolean
> > +    description:
> > +      If the device does not support triggered buffers and needs to be polled.
> 

What would your recommendation be? I'm basically proposing polling
instead of utilizing a triggered buffer, so instead of the driver
relying on a hardware trigger or a software trigger being set up to
fill the buffer that is read by the input subsystem we use the input
subsystem polling function directly (this is useful for my use-case
where the Rockchip SARADC doesn't have a hardware trigger).

Thank you for your help.

> You described desired Linux feature or behavior, not the actual
> hardware. The bindings are about the latter, so instead you need to
> rephrase the property and it's description to match actual hardware
> capabilities/features/configuration etc.
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH v2 1/3] dt-bindings: adc-joystick: add adc-joystick,polled option

[Krzysztof Kozlowski]

On 02/06/2022 21:44, Chris Morgan wrote:
> On Thu, Jun 02, 2022 at 11:56:10AM +0200, Krzysztof Kozlowski wrote:
>> On 01/06/2022 22:49, Chris Morgan wrote:
>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>
>>> Add documentation for adc-joystick,polled. New device-tree properties
>>> have been added.
>>>
>>> - adc-joystick,polled: A boolean value noting the joystick device
>>> 		       should be polled rather than use a triggered
>>> 		       buffer.
>>>
>>> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
>>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>>> ---
>>>  .../devicetree/bindings/input/adc-joystick.yaml          | 9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/adc-joystick.yaml b/Documentation/devicetree/bindings/input/adc-joystick.yaml
>>> index 2ee04e03bc22..4f49a1a5772e 100644
>>> --- a/Documentation/devicetree/bindings/input/adc-joystick.yaml
>>> +++ b/Documentation/devicetree/bindings/input/adc-joystick.yaml
>>> @@ -12,12 +12,19 @@ maintainers:
>>>  
>>>  description: >
>>>    Bindings for joystick devices connected to ADC controllers supporting
>>> -  the Industrial I/O subsystem.
>>> +  the Industrial I/O subsystem. Supports both polled devices where no
>>> +  iio trigger is available and non-polled devices which are triggered
>>> +  by iio.
>>>  
>>>  properties:
>>>    compatible:
>>>      const: adc-joystick
>>>  
>>> +  adc-joystick,polled:
>>> +    type: boolean
>>> +    description:
>>> +      If the device does not support triggered buffers and needs to be polled.
>>
> 
> What would your recommendation be? I'm basically proposing polling
> instead of utilizing a triggered buffer, so instead of the driver
> relying on a hardware trigger or a software trigger being set up to
> fill the buffer that is read by the input subsystem we use the input
> subsystem polling function directly (this is useful for my use-case
> where the Rockchip SARADC doesn't have a hardware trigger).

Then it could be: "adc-joystik,no-hardware-trigger". The description is
I think accurate, so just the property name.



Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: adc-joystick: add adc-joystick,polled option

[Artur Rojek]

On 2022-06-03 07:33, Krzysztof Kozlowski wrote:
> On 02/06/2022 21:44, Chris Morgan wrote:
>> On Thu, Jun 02, 2022 at 11:56:10AM +0200, Krzysztof Kozlowski wrote:
>>> On 01/06/2022 22:49, Chris Morgan wrote:
>>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>> 
>>>> Add documentation for adc-joystick,polled. New device-tree 
>>>> properties
>>>> have been added.
>>>> 
>>>> - adc-joystick,polled: A boolean value noting the joystick device
>>>> 		       should be polled rather than use a triggered
>>>> 		       buffer.
>>>> 
>>>> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
>>>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>>>> ---
>>>>  .../devicetree/bindings/input/adc-joystick.yaml          | 9 
>>>> ++++++++-
>>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git 
>>>> a/Documentation/devicetree/bindings/input/adc-joystick.yaml 
>>>> b/Documentation/devicetree/bindings/input/adc-joystick.yaml
>>>> index 2ee04e03bc22..4f49a1a5772e 100644
>>>> --- a/Documentation/devicetree/bindings/input/adc-joystick.yaml
>>>> +++ b/Documentation/devicetree/bindings/input/adc-joystick.yaml
>>>> @@ -12,12 +12,19 @@ maintainers:
>>>> 
>>>>  description: >
>>>>    Bindings for joystick devices connected to ADC controllers 
>>>> supporting
>>>> -  the Industrial I/O subsystem.
>>>> +  the Industrial I/O subsystem. Supports both polled devices where 
>>>> no
>>>> +  iio trigger is available and non-polled devices which are 
>>>> triggered
>>>> +  by iio.
>>>> 
>>>>  properties:
>>>>    compatible:
>>>>      const: adc-joystick
>>>> 
>>>> +  adc-joystick,polled:
>>>> +    type: boolean
>>>> +    description:
>>>> +      If the device does not support triggered buffers and needs to 
>>>> be polled.
>>> 
>> 
>> What would your recommendation be? I'm basically proposing polling
>> instead of utilizing a triggered buffer, so instead of the driver
>> relying on a hardware trigger or a software trigger being set up to
>> fill the buffer that is read by the input subsystem we use the input
>> subsystem polling function directly (this is useful for my use-case
>> where the Rockchip SARADC doesn't have a hardware trigger).
> 
> Then it could be: "adc-joystik,no-hardware-trigger". The description is
> I think accurate, so just the property name.

Hmm... I don't think DT involvement is necessary here, as this 
information can be derived from iio at runtime:
`if (iio_dev->modes & INDIO_BUFFER_SOFTWARE)` - a buffer can be used,
`if (iio_dev->modes & INDIO_DIRECT_MODE)` - device can be polled.

Cheers,
Artur
> 
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH v2 2/3] Input: adc-joystick - Add polled input device support

[Jonathan Cameron]

On Thu, 02 Jun 2022 10:22:11 +0100
Paul Cercueil <paul@crapouillou.net> wrote:

> Hi Chris,
> 
> Le mer., juin 1 2022 at 15:49:26 -0500, Chris Morgan 
> <macroalpha82@gmail.com> a écrit :
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Add polled input device support to the adc-joystick driver. This is
> > useful for devices which do not have hardware capable triggers on
> > their SARADC. Code modified from adc-joystick.c changes made by Maya
> > Matuszczyk.  
> 
> I understand what you're trying to do, but maybe there's a better way.
> 
> It would be better if pollable IIO devices would also be able to be 
> used with a buffer, with the client driver manually triggering the IIO 
> device through a function call.

We've done exactly that in the past for some of the weirder consumer
drivers.  Though it's done by hooking up a trigger that is provided
by the channel consumer to the supplier device.

> 
> Jonathan (Cc'd) might have his reasons why this isn't done this way, 
> and in this case your patch makes sense, but I'd like to hear 
> Jonathan's thoughts first.

It rarely makes sense to have a sysfs only ADC and it's usually trivial
to add basic support for buffered mode to use with a trigger from elsewhere.
It's rare that their aren't optimizations around batching etc that
make sense though in this path and not the sysfs polling path where only
one channel is being read.   Hence it's not worth a generic route to
hooking them together.

> 
> As for "adc-joystick,polled"... it doesn't describe the hardware but 
> rather a specific configuration, so it shouldn't appear in the device 
> tree.

Sorry, got lost in my email.

I'll take a look at v3.

Jonathan

> 
> Cheers,
> -Paul
> 
> > 
> > Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> >  drivers/input/joystick/adc-joystick.c | 52 
> > +++++++++++++++++++++------
> >  1 file changed, 41 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/input/joystick/adc-joystick.c 
> > b/drivers/input/joystick/adc-joystick.c
> > index 78ebca7d400a..5a28fe7b8ebc 100644
> > --- a/drivers/input/joystick/adc-joystick.c
> > +++ b/drivers/input/joystick/adc-joystick.c
> > @@ -13,6 +13,10 @@
> > 
> >  #include <asm/unaligned.h>
> > 
> > +#define ADC_JSK_POLL_INTERVAL	16
> > +#define ADC_JSK_POLL_MIN	8
> > +#define ADC_JSK_POLL_MAX	32
> > +
> >  struct adc_joystick_axis {
> >  	u32 code;
> >  	s32 range[2];
> > @@ -26,8 +30,21 @@ struct adc_joystick {
> >  	struct adc_joystick_axis *axes;
> >  	struct iio_channel *chans;
> >  	int num_chans;
> > +	bool polled;
> >  };
> > 
> > +static void adc_joystick_poll(struct input_dev *input)
> > +{
> > +	struct adc_joystick *joy = input_get_drvdata(input);
> > +	int i, val;
> > +
> > +	for (i = 0; i < joy->num_chans; i++) {
> > +		iio_read_channel_raw(&joy->chans[i], &val);
> > +		input_report_abs(input, joy->axes[i].code, val);
> > +	}
> > +	input_sync(input);
> > +}
> > +
> >  static int adc_joystick_handle(const void *data, void *private)
> >  {
> >  	struct adc_joystick *joy = private;
> > @@ -215,8 +232,19 @@ static int adc_joystick_probe(struct 
> > platform_device *pdev)
> >  	joy->input = input;
> >  	input->name = pdev->name;
> >  	input->id.bustype = BUS_HOST;
> > -	input->open = adc_joystick_open;
> > -	input->close = adc_joystick_close;
> > +
> > +	if (device_property_read_bool(dev, "adc-joystick,polled"))
> > +		joy->polled = 1;
> > +
> > +	if (joy->polled) {
> > +		input_setup_polling(input, adc_joystick_poll);
> > +		input_set_poll_interval(input, ADC_JSK_POLL_INTERVAL);
> > +		input_set_min_poll_interval(input, ADC_JSK_POLL_MIN);
> > +		input_set_max_poll_interval(input, ADC_JSK_POLL_MAX);
> > +	} else {
> > +		input->open = adc_joystick_open;
> > +		input->close = adc_joystick_close;
> > +	}
> > 
> >  	error = adc_joystick_set_axes(dev, joy);
> >  	if (error)
> > @@ -229,16 +257,18 @@ static int adc_joystick_probe(struct 
> > platform_device *pdev)
> >  		return error;
> >  	}
> > 
> > -	joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, joy);
> > -	if (IS_ERR(joy->buffer)) {
> > -		dev_err(dev, "Unable to allocate callback buffer\n");
> > -		return PTR_ERR(joy->buffer);
> > -	}
> > +	if (!joy->polled) {
> > +		joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, 
> > joy);
> > +		if (IS_ERR(joy->buffer)) {
> > +			dev_err(dev, "Unable to allocate callback buffer\n");
> > +			return PTR_ERR(joy->buffer);
> > +		}
> > 
> > -	error = devm_add_action_or_reset(dev, adc_joystick_cleanup, 
> > joy->buffer);
> > -	if (error)  {
> > -		dev_err(dev, "Unable to add action\n");
> > -		return error;
> > +		error = devm_add_action_or_reset(dev, adc_joystick_cleanup, 
> > joy->buffer);
> > +		if (error)  {
> > +			dev_err(dev, "Unable to add action\n");
> > +			return error;
> > +		}
> >  	}
> > 
> >  	return 0;  
> 
>