linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V1] Input: synaptics-rmi4- Add a new feature for Forcepad.
@ 2025-06-19 11:25 Marge Yang
  2025-06-29  5:00 ` Dmitry Torokhov
  0 siblings, 1 reply; 3+ messages in thread
From: Marge Yang @ 2025-06-19 11:25 UTC (permalink / raw)
  To: dmitry.torokhov, linux-input, linux-kernel, vincent.huang,
	marge.yang
  Cc: david.chiu, derek.cheng, sam.tsai, Marge Yang

From: Marge Yang <Marge.Yang@tw.synaptics.com>

Forcepad devices will use F21, for click simulation
due to lack of a metal button, so we add F21 support
to make forcepad support click function.

Signed-off-by: Marge Yang <Marge.Yang@tw.synaptics.com>
---
 drivers/input/rmi4/Kconfig      |   8 ++
 drivers/input/rmi4/Makefile     |   1 +
 drivers/input/rmi4/rmi_bus.c    |   3 +
 drivers/input/rmi4/rmi_driver.h |   5 ++
 drivers/input/rmi4/rmi_f21.c    | 126 ++++++++++++++++++++++++++++++++
 5 files changed, 143 insertions(+)
 create mode 100644 drivers/input/rmi4/rmi_f21.c

diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index c0163b983ce6..086013be6a64 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -82,6 +82,14 @@ config RMI4_F12
 	  touchpads. For sensors that support relative pointing, F12 also
 	  provides mouse input.
 
+config RMI4_F21
+	bool "RMI4 Function 21 (PRESSURE)"
+	help
+	  Say Y here if you want to add support for RMI4 function 21.
+
+	  Function 21 provides buttons/pressure for RMI4 devices. This includes
+	  support for buttons/pressure on PressurePad.
+
 config RMI4_F30
 	bool "RMI4 Function 30 (GPIO LED)"
 	help
diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
index 02f14c846861..484b97eca025 100644
--- a/drivers/input/rmi4/Makefile
+++ b/drivers/input/rmi4/Makefile
@@ -8,6 +8,7 @@ rmi_core-$(CONFIG_RMI4_2D_SENSOR) += rmi_2d_sensor.o
 rmi_core-$(CONFIG_RMI4_F03) += rmi_f03.o
 rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o
 rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o
+rmi_core-$(CONFIG_RMI4_F21) += rmi_f21.o
 rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
 rmi_core-$(CONFIG_RMI4_F34) += rmi_f34.o rmi_f34v7.o
 rmi_core-$(CONFIG_RMI4_F3A) += rmi_f3a.o
diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
index 3aee04837205..47fe7a88c92b 100644
--- a/drivers/input/rmi4/rmi_bus.c
+++ b/drivers/input/rmi4/rmi_bus.c
@@ -360,6 +360,9 @@ static struct rmi_function_handler *fn_handlers[] = {
 #ifdef CONFIG_RMI4_F12
 	&rmi_f12_handler,
 #endif
+#ifdef CONFIG_RMI4_F21
+	&rmi_f21_handler,
+#endif
 #ifdef CONFIG_RMI4_F30
 	&rmi_f30_handler,
 #endif
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 3bfe9013043e..18fdf2a166d5 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -115,6 +115,10 @@ static inline int rmi_f03_overwrite_button(struct rmi_function *fn,
 static inline void rmi_f03_commit_buttons(struct rmi_function *fn) {}
 #endif
 
+#ifdef CONFIG_RMI4_F21
+int rmi_f21_report_pressure(struct rmi_function *fn, int i);
+#endif
+
 #ifdef CONFIG_RMI4_F34
 int rmi_f34_create_sysfs(struct rmi_device *rmi_dev);
 void rmi_f34_remove_sysfs(struct rmi_device *rmi_dev);
@@ -133,6 +137,7 @@ extern struct rmi_function_handler rmi_f01_handler;
 extern struct rmi_function_handler rmi_f03_handler;
 extern struct rmi_function_handler rmi_f11_handler;
 extern struct rmi_function_handler rmi_f12_handler;
+extern struct rmi_function_handler rmi_f21_handler;
 extern struct rmi_function_handler rmi_f30_handler;
 extern struct rmi_function_handler rmi_f34_handler;
 extern struct rmi_function_handler rmi_f3a_handler;
diff --git a/drivers/input/rmi4/rmi_f21.c b/drivers/input/rmi4/rmi_f21.c
new file mode 100644
index 000000000000..93ef2331ed16
--- /dev/null
+++ b/drivers/input/rmi4/rmi_f21.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2012-2025 Synaptics Incorporated
+ */
+
+#include <linux/kernel.h>
+#include <linux/rmi.h>
+#include <linux/input.h>
+#include <linux/slab.h>
+#include "rmi_driver.h"
+
+#define RMI_f21_INPUT_REPORT_DATA_SIZE	6
+#define RMI_F21_INPUT_REPORT_FORCE_CLICK_OFFSET	5
+#define RMI_F21_TABLE_FORCE_CLICK_OFFSET	8
+#define RMI_f21_FORCE_CLICK			0x01
+#define RMI_f21_DATA_REGS_MAX_SIZE	19
+#define RMI_f21_FORCEPAD_BUTTON_COUNT	1
+
+struct f21_data {
+	/* Query Data */
+	u8 data_regs[RMI_f21_DATA_REGS_MAX_SIZE];
+	u8 input_report_data[RMI_f21_INPUT_REPORT_DATA_SIZE];
+	struct input_dev *input;
+	u16 key_code;
+};
+
+static irqreturn_t rmi_f21_attention(int irq, void *ctx)
+{
+	struct rmi_function *fn = ctx;
+	struct f21_data *f21 = dev_get_drvdata(&fn->dev);
+	struct rmi_driver_data *drvdata = dev_get_drvdata(&fn->rmi_dev->dev);
+	int error;
+	bool pressed;
+
+	if (drvdata->attn_data.data) {
+		if (drvdata->attn_data.size < RMI_f21_INPUT_REPORT_DATA_SIZE) {
+			dev_warn(&fn->dev, "f21 interrupted, but data is missing\n");
+			return IRQ_HANDLED;
+		}
+		memcpy(f21->input_report_data, drvdata->attn_data.data, RMI_f21_INPUT_REPORT_DATA_SIZE);
+		drvdata->attn_data.data += RMI_f21_INPUT_REPORT_DATA_SIZE;
+		drvdata->attn_data.size -= RMI_f21_INPUT_REPORT_DATA_SIZE;
+
+		pressed = !!(f21->input_report_data[RMI_F21_INPUT_REPORT_FORCE_CLICK_OFFSET] &
+					RMI_f21_FORCE_CLICK);
+	} else {
+		error = rmi_read_block(fn->rmi_dev, fn->fd.data_base_addr,
+					f21->data_regs, RMI_f21_DATA_REGS_MAX_SIZE);
+		if (error) {
+			dev_err(&fn->dev, "%s: Failed to read f21 data registers: %d\n",
+				__func__, error);
+			return IRQ_RETVAL(error);
+		}
+		pressed = !!(f21->data_regs[RMI_F21_TABLE_FORCE_CLICK_OFFSET] &
+					RMI_f21_FORCE_CLICK);
+	}
+
+	input_report_key(f21->input, f21->key_code, pressed);
+
+	return IRQ_HANDLED;
+}
+
+static int rmi_f21_config(struct rmi_function *fn)
+{
+	struct f21_data *f21 = dev_get_drvdata(&fn->dev);
+	struct rmi_driver *drv = fn->rmi_dev->driver;
+
+	if (!f21)
+		return 0;
+
+	drv->set_irq_bits(fn->rmi_dev, fn->irq_mask);
+
+	return 0;
+}
+
+static int rmi_f21_initialize(struct rmi_function *fn, struct f21_data *f21)
+{
+	struct input_dev *input = f21->input;
+	unsigned int button = BTN_LEFT;
+
+	f21->key_code = button;
+	input_set_capability(input, EV_KEY, f21->key_code);
+	input->keycode = &(f21->key_code);
+	input->keycodesize = sizeof(f21->key_code);
+	input->keycodemax = RMI_f21_FORCEPAD_BUTTON_COUNT;
+
+	__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
+
+	return 0;
+}
+
+static int rmi_f21_probe(struct rmi_function *fn)
+{
+	struct rmi_device *rmi_dev = fn->rmi_dev;
+	struct rmi_driver_data *drv_data = dev_get_drvdata(&rmi_dev->dev);
+	struct f21_data *f21;
+	int error;
+
+	if (!drv_data->input) {
+		dev_info(&fn->dev, "f21: no input device found, ignoring\n");
+		return -ENXIO;
+	}
+
+	f21 = devm_kzalloc(&fn->dev, sizeof(*f21), GFP_KERNEL);
+	if (!f21)
+		return -ENOMEM;
+
+	f21->input = drv_data->input;
+
+	error = rmi_f21_initialize(fn, f21);
+	if (error)
+		return error;
+
+	dev_set_drvdata(&fn->dev, f21);
+	return 0;
+}
+
+struct rmi_function_handler rmi_f21_handler = {
+	.driver = {
+		.name = "rmi4_f21",
+	},
+	.func = 0x21,
+	.probe = rmi_f21_probe,
+	.config = rmi_f21_config,
+	.attention = rmi_f21_attention,
+};
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH V1] Input: synaptics-rmi4- Add a new feature for Forcepad.
  2025-06-19 11:25 [PATCH V1] Input: synaptics-rmi4- Add a new feature for Forcepad Marge Yang
@ 2025-06-29  5:00 ` Dmitry Torokhov
  2025-07-14  3:17   ` Marge Yang
  0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Torokhov @ 2025-06-29  5:00 UTC (permalink / raw)
  To: Marge Yang
  Cc: linux-input, linux-kernel, vincent.huang, david.chiu, derek.cheng,
	sam.tsai

Hi Marge,

On Thu, Jun 19, 2025 at 11:25:00AM +0000, Marge Yang wrote:
> From: Marge Yang <Marge.Yang@tw.synaptics.com>
> 
> Forcepad devices will use F21, for click simulation
> due to lack of a metal button, so we add F21 support
> to make forcepad support click function.

I guess with this we can remove the blacklist we have for forcepads in
drivers/input/mouse/synaptics.c?

> 
> Signed-off-by: Marge Yang <Marge.Yang@tw.synaptics.com>
> ---
>  drivers/input/rmi4/Kconfig      |   8 ++
>  drivers/input/rmi4/Makefile     |   1 +
>  drivers/input/rmi4/rmi_bus.c    |   3 +
>  drivers/input/rmi4/rmi_driver.h |   5 ++
>  drivers/input/rmi4/rmi_f21.c    | 126 ++++++++++++++++++++++++++++++++
>  5 files changed, 143 insertions(+)
>  create mode 100644 drivers/input/rmi4/rmi_f21.c
> 
> diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
> index c0163b983ce6..086013be6a64 100644
> --- a/drivers/input/rmi4/Kconfig
> +++ b/drivers/input/rmi4/Kconfig
> @@ -82,6 +82,14 @@ config RMI4_F12
>  	  touchpads. For sensors that support relative pointing, F12 also
>  	  provides mouse input.
>  
> +config RMI4_F21
> +	bool "RMI4 Function 21 (PRESSURE)"
> +	help
> +	  Say Y here if you want to add support for RMI4 function 21.
> +
> +	  Function 21 provides buttons/pressure for RMI4 devices. This includes
> +	  support for buttons/pressure on PressurePad.
> +
>  config RMI4_F30
>  	bool "RMI4 Function 30 (GPIO LED)"
>  	help
> diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
> index 02f14c846861..484b97eca025 100644
> --- a/drivers/input/rmi4/Makefile
> +++ b/drivers/input/rmi4/Makefile
> @@ -8,6 +8,7 @@ rmi_core-$(CONFIG_RMI4_2D_SENSOR) += rmi_2d_sensor.o
>  rmi_core-$(CONFIG_RMI4_F03) += rmi_f03.o
>  rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o
>  rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o
> +rmi_core-$(CONFIG_RMI4_F21) += rmi_f21.o
>  rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
>  rmi_core-$(CONFIG_RMI4_F34) += rmi_f34.o rmi_f34v7.o
>  rmi_core-$(CONFIG_RMI4_F3A) += rmi_f3a.o
> diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
> index 3aee04837205..47fe7a88c92b 100644
> --- a/drivers/input/rmi4/rmi_bus.c
> +++ b/drivers/input/rmi4/rmi_bus.c
> @@ -360,6 +360,9 @@ static struct rmi_function_handler *fn_handlers[] = {
>  #ifdef CONFIG_RMI4_F12
>  	&rmi_f12_handler,
>  #endif
> +#ifdef CONFIG_RMI4_F21
> +	&rmi_f21_handler,
> +#endif
>  #ifdef CONFIG_RMI4_F30
>  	&rmi_f30_handler,
>  #endif
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index 3bfe9013043e..18fdf2a166d5 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -115,6 +115,10 @@ static inline int rmi_f03_overwrite_button(struct rmi_function *fn,
>  static inline void rmi_f03_commit_buttons(struct rmi_function *fn) {}
>  #endif
>  
> +#ifdef CONFIG_RMI4_F21
> +int rmi_f21_report_pressure(struct rmi_function *fn, int i);
> +#endif
> +

I do not see definition for this anywhere in the patch.

>  #ifdef CONFIG_RMI4_F34
>  int rmi_f34_create_sysfs(struct rmi_device *rmi_dev);
>  void rmi_f34_remove_sysfs(struct rmi_device *rmi_dev);
> @@ -133,6 +137,7 @@ extern struct rmi_function_handler rmi_f01_handler;
>  extern struct rmi_function_handler rmi_f03_handler;
>  extern struct rmi_function_handler rmi_f11_handler;
>  extern struct rmi_function_handler rmi_f12_handler;
> +extern struct rmi_function_handler rmi_f21_handler;
>  extern struct rmi_function_handler rmi_f30_handler;
>  extern struct rmi_function_handler rmi_f34_handler;
>  extern struct rmi_function_handler rmi_f3a_handler;
> diff --git a/drivers/input/rmi4/rmi_f21.c b/drivers/input/rmi4/rmi_f21.c
> new file mode 100644
> index 000000000000..93ef2331ed16
> --- /dev/null
> +++ b/drivers/input/rmi4/rmi_f21.c
> @@ -0,0 +1,126 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2012-2025 Synaptics Incorporated
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/rmi.h>
> +#include <linux/input.h>
> +#include <linux/slab.h>
> +#include "rmi_driver.h"
> +
> +#define RMI_f21_INPUT_REPORT_DATA_SIZE	6
> +#define RMI_F21_INPUT_REPORT_FORCE_CLICK_OFFSET	5
> +#define RMI_F21_TABLE_FORCE_CLICK_OFFSET	8
> +#define RMI_f21_FORCE_CLICK			0x01

Use BIT(0) here please.

> +#define RMI_f21_DATA_REGS_MAX_SIZE	19

Why such a big difference in F21 data size between HID and other
transports?

> +#define RMI_f21_FORCEPAD_BUTTON_COUNT	1
> +
> +struct f21_data {
> +	/* Query Data */
> +	u8 data_regs[RMI_f21_DATA_REGS_MAX_SIZE];
> +	u8 input_report_data[RMI_f21_INPUT_REPORT_DATA_SIZE];
> +	struct input_dev *input;
> +	u16 key_code;
> +};
> +
> +static irqreturn_t rmi_f21_attention(int irq, void *ctx)
> +{
> +	struct rmi_function *fn = ctx;
> +	struct f21_data *f21 = dev_get_drvdata(&fn->dev);
> +	struct rmi_driver_data *drvdata = dev_get_drvdata(&fn->rmi_dev->dev);
> +	int error;
> +	bool pressed;
> +
> +	if (drvdata->attn_data.data) {
> +		if (drvdata->attn_data.size < RMI_f21_INPUT_REPORT_DATA_SIZE) {
> +			dev_warn(&fn->dev, "f21 interrupted, but data is missing\n");
> +			return IRQ_HANDLED;
> +		}
> +		memcpy(f21->input_report_data, drvdata->attn_data.data, RMI_f21_INPUT_REPORT_DATA_SIZE);

I do not think you need to do the copy, you can test the bit directly in
drvdata->attn_data.data buffer.

> +		drvdata->attn_data.data += RMI_f21_INPUT_REPORT_DATA_SIZE;
> +		drvdata->attn_data.size -= RMI_f21_INPUT_REPORT_DATA_SIZE;
> +
> +		pressed = !!(f21->input_report_data[RMI_F21_INPUT_REPORT_FORCE_CLICK_OFFSET] &
> +					RMI_f21_FORCE_CLICK);

No need to double negation here, converting to bool will do the right
thing.

> +	} else {
> +		error = rmi_read_block(fn->rmi_dev, fn->fd.data_base_addr,
> +					f21->data_regs, RMI_f21_DATA_REGS_MAX_SIZE);
> +		if (error) {
> +			dev_err(&fn->dev, "%s: Failed to read f21 data registers: %d\n",
> +				__func__, error);
> +			return IRQ_RETVAL(error);
> +		}
> +		pressed = !!(f21->data_regs[RMI_F21_TABLE_FORCE_CLICK_OFFSET] &
> +					RMI_f21_FORCE_CLICK);

Same here.

> +	}
> +
> +	input_report_key(f21->input, f21->key_code, pressed);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int rmi_f21_config(struct rmi_function *fn)
> +{
> +	struct f21_data *f21 = dev_get_drvdata(&fn->dev);
> +	struct rmi_driver *drv = fn->rmi_dev->driver;
> +
> +	if (!f21)
> +		return 0;

Is this actually possible for f21 to be NULL here?

> +
> +	drv->set_irq_bits(fn->rmi_dev, fn->irq_mask);
> +
> +	return 0;
> +}
> +
> +static int rmi_f21_initialize(struct rmi_function *fn, struct f21_data *f21)
> +{
> +	struct input_dev *input = f21->input;
> +	unsigned int button = BTN_LEFT;

This variable is not needed.

> +
> +	f21->key_code = button;

	f21->key_code = BTN_LEFT;

> +	input_set_capability(input, EV_KEY, f21->key_code);
> +	input->keycode = &(f21->key_code);

Drop extra parenthesis.

> +	input->keycodesize = sizeof(f21->key_code);
> +	input->keycodemax = RMI_f21_FORCEPAD_BUTTON_COUNT;
> +
> +	__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
> +
> +	return 0;
> +}
> +
> +static int rmi_f21_probe(struct rmi_function *fn)
> +{
> +	struct rmi_device *rmi_dev = fn->rmi_dev;
> +	struct rmi_driver_data *drv_data = dev_get_drvdata(&rmi_dev->dev);
> +	struct f21_data *f21;
> +	int error;
> +
> +	if (!drv_data->input) {
> +		dev_info(&fn->dev, "f21: no input device found, ignoring\n");
> +		return -ENXIO;
> +	}
> +
> +	f21 = devm_kzalloc(&fn->dev, sizeof(*f21), GFP_KERNEL);
> +	if (!f21)
> +		return -ENOMEM;
> +
> +	f21->input = drv_data->input;
> +
> +	error = rmi_f21_initialize(fn, f21);
> +	if (error)
> +		return error;
> +
> +	dev_set_drvdata(&fn->dev, f21);
> +	return 0;
> +}
> +
> +struct rmi_function_handler rmi_f21_handler = {
> +	.driver = {
> +		.name = "rmi4_f21",
> +	},
> +	.func = 0x21,
> +	.probe = rmi_f21_probe,
> +	.config = rmi_f21_config,
> +	.attention = rmi_f21_attention,
> +};

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH V1] Input: synaptics-rmi4- Add a new feature for Forcepad.
  2025-06-29  5:00 ` Dmitry Torokhov
@ 2025-07-14  3:17   ` Marge Yang
  0 siblings, 0 replies; 3+ messages in thread
From: Marge Yang @ 2025-07-14  3:17 UTC (permalink / raw)
  To: Dmitry Torokhov, Marge Yang
  Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Vincent Huang, David Chiu, Derek Cheng, Sam Tsai

Hi Dmitry,
	Update the status.

-----Original Message-----
From: Dmitry Torokhov <dmitry.torokhov@gmail.com> 
Sent: Sunday, June 29, 2025 1:01 PM
To: Marge Yang <Marge.Yang@tw.synaptics.com>
Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Vincent Huang <Vincent.huang@tw.synaptics.com>; David Chiu <David.Chiu@tw.synaptics.com>; Derek Cheng <derek.cheng@tw.synaptics.com>; Sam Tsai <Sam.Tsai@synaptics.com>
Subject: Re: [PATCH V1] Input: synaptics-rmi4- Add a new feature for Forcepad.

CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.


Hi Marge,

On Thu, Jun 19, 2025 at 11:25:00AM +0000, Marge Yang wrote:
> From: Marge Yang <Marge.Yang@tw.synaptics.com>
>
> Forcepad devices will use F21, for click simulation due to lack of a 
> metal button, so we add F21 support to make forcepad support click 
> function.

I guess with this we can remove the blacklist we have for forcepads in drivers/input/mouse/synaptics.c?


[Marge 07/14]
The current version's detection method does not support the F21 design from 12 years ago. 
I will discuss with the Synaptics firmware team member how to add support, and it will be implemented in a future release.

>
> Signed-off-by: Marge Yang <Marge.Yang@tw.synaptics.com>
> ---
>  drivers/input/rmi4/Kconfig      |   8 ++
>  drivers/input/rmi4/Makefile     |   1 +
>  drivers/input/rmi4/rmi_bus.c    |   3 +
>  drivers/input/rmi4/rmi_driver.h |   5 ++
>  drivers/input/rmi4/rmi_f21.c    | 126 ++++++++++++++++++++++++++++++++
>  5 files changed, 143 insertions(+)
>  create mode 100644 drivers/input/rmi4/rmi_f21.c
>
> diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig 
> index c0163b983ce6..086013be6a64 100644
> --- a/drivers/input/rmi4/Kconfig
> +++ b/drivers/input/rmi4/Kconfig
> @@ -82,6 +82,14 @@ config RMI4_F12
>         touchpads. For sensors that support relative pointing, F12 also
>         provides mouse input.
>
> +config RMI4_F21
> +     bool "RMI4 Function 21 (PRESSURE)"
> +     help
> +       Say Y here if you want to add support for RMI4 function 21.
> +
> +       Function 21 provides buttons/pressure for RMI4 devices. This includes
> +       support for buttons/pressure on PressurePad.
> +
>  config RMI4_F30
>       bool "RMI4 Function 30 (GPIO LED)"
>       help
> diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile 
> index 02f14c846861..484b97eca025 100644
> --- a/drivers/input/rmi4/Makefile
> +++ b/drivers/input/rmi4/Makefile
> @@ -8,6 +8,7 @@ rmi_core-$(CONFIG_RMI4_2D_SENSOR) += rmi_2d_sensor.o
>  rmi_core-$(CONFIG_RMI4_F03) += rmi_f03.o
>  rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o
>  rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o
> +rmi_core-$(CONFIG_RMI4_F21) += rmi_f21.o
>  rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
>  rmi_core-$(CONFIG_RMI4_F34) += rmi_f34.o rmi_f34v7.o
>  rmi_core-$(CONFIG_RMI4_F3A) += rmi_f3a.o diff --git 
> a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c index 
> 3aee04837205..47fe7a88c92b 100644
> --- a/drivers/input/rmi4/rmi_bus.c
> +++ b/drivers/input/rmi4/rmi_bus.c
> @@ -360,6 +360,9 @@ static struct rmi_function_handler *fn_handlers[] 
> = {  #ifdef CONFIG_RMI4_F12
>       &rmi_f12_handler,
>  #endif
> +#ifdef CONFIG_RMI4_F21
> +     &rmi_f21_handler,
> +#endif
>  #ifdef CONFIG_RMI4_F30
>       &rmi_f30_handler,
>  #endif
> diff --git a/drivers/input/rmi4/rmi_driver.h 
> b/drivers/input/rmi4/rmi_driver.h index 3bfe9013043e..18fdf2a166d5 
> 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -115,6 +115,10 @@ static inline int rmi_f03_overwrite_button(struct 
> rmi_function *fn,  static inline void rmi_f03_commit_buttons(struct 
> rmi_function *fn) {}  #endif
>
> +#ifdef CONFIG_RMI4_F21
> +int rmi_f21_report_pressure(struct rmi_function *fn, int i); #endif
> +

I do not see definition for this anywhere in the patch.

[Marge 07/14]
I will remove it in next patch.

>  #ifdef CONFIG_RMI4_F34
>  int rmi_f34_create_sysfs(struct rmi_device *rmi_dev);  void 
> rmi_f34_remove_sysfs(struct rmi_device *rmi_dev); @@ -133,6 +137,7 @@ 
> extern struct rmi_function_handler rmi_f01_handler;  extern struct 
> rmi_function_handler rmi_f03_handler;  extern struct 
> rmi_function_handler rmi_f11_handler;  extern struct 
> rmi_function_handler rmi_f12_handler;
> +extern struct rmi_function_handler rmi_f21_handler;
>  extern struct rmi_function_handler rmi_f30_handler;  extern struct 
> rmi_function_handler rmi_f34_handler;  extern struct 
> rmi_function_handler rmi_f3a_handler; diff --git 
> a/drivers/input/rmi4/rmi_f21.c b/drivers/input/rmi4/rmi_f21.c new file 
> mode 100644 index 000000000000..93ef2331ed16
> --- /dev/null
> +++ b/drivers/input/rmi4/rmi_f21.c
> @@ -0,0 +1,126 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2012-2025 Synaptics Incorporated  */
> +
> +#include <linux/kernel.h>
> +#include <linux/rmi.h>
> +#include <linux/input.h>
> +#include <linux/slab.h>
> +#include "rmi_driver.h"
> +
> +#define RMI_f21_INPUT_REPORT_DATA_SIZE       6
> +#define RMI_F21_INPUT_REPORT_FORCE_CLICK_OFFSET      5
> +#define RMI_F21_TABLE_FORCE_CLICK_OFFSET     8
> +#define RMI_f21_FORCE_CLICK                  0x01

Use BIT(0) here please.

[Marge 07/14]
I will use BIT(0) in next patch.

> +#define RMI_f21_DATA_REGS_MAX_SIZE   19

Why such a big difference in F21 data size between HID and other transports?

[Marge 07/14]
F21 Data Structure:
Data size = (number of Forcepad sensors) × 2 bytes (Currently there are 4 Forcepad sensors) 
+ 1 byte for the Forcepad button state
+ (5 fingers × 2 bytes) for finger pressure values (Supports five fingers, each finger's pressure value occupies two bytes) 

HID Attention: 6 bytes
The pressure values for the five fingers are each represented by one byte, so that's five bytes total, plus one byte for the Forcepad button state.

I will discuss with the Synaptics firmware team member how to add support by representing it through a lookup table.
Update it in next patch

> +#define RMI_f21_FORCEPAD_BUTTON_COUNT        1
> +
> +struct f21_data {
> +     /* Query Data */
> +     u8 data_regs[RMI_f21_DATA_REGS_MAX_SIZE];
> +     u8 input_report_data[RMI_f21_INPUT_REPORT_DATA_SIZE];
> +     struct input_dev *input;
> +     u16 key_code;
> +};
> +
> +static irqreturn_t rmi_f21_attention(int irq, void *ctx) {
> +     struct rmi_function *fn = ctx;
> +     struct f21_data *f21 = dev_get_drvdata(&fn->dev);
> +     struct rmi_driver_data *drvdata = dev_get_drvdata(&fn->rmi_dev->dev);
> +     int error;
> +     bool pressed;
> +
> +     if (drvdata->attn_data.data) {
> +             if (drvdata->attn_data.size < RMI_f21_INPUT_REPORT_DATA_SIZE) {
> +                     dev_warn(&fn->dev, "f21 interrupted, but data is missing\n");
> +                     return IRQ_HANDLED;
> +             }
> +             memcpy(f21->input_report_data, drvdata->attn_data.data, 
> + RMI_f21_INPUT_REPORT_DATA_SIZE);

I do not think you need to do the copy, you can test the bit directly in
drvdata->attn_data.data buffer.

[Marge 07/14]
I will use drvdata->attn_data.data buffer directly in next patch.

> +             drvdata->attn_data.data += RMI_f21_INPUT_REPORT_DATA_SIZE;
> +             drvdata->attn_data.size -= 
> + RMI_f21_INPUT_REPORT_DATA_SIZE;
> +
> +             pressed = !!(f21->input_report_data[RMI_F21_INPUT_REPORT_FORCE_CLICK_OFFSET] &
> +                                     RMI_f21_FORCE_CLICK);

No need to double negation here, converting to bool will do the right thing.

[Marge 07/14]
I will correct it in next patch.

> +     } else {
> +             error = rmi_read_block(fn->rmi_dev, fn->fd.data_base_addr,
> +                                     f21->data_regs, RMI_f21_DATA_REGS_MAX_SIZE);
> +             if (error) {
> +                     dev_err(&fn->dev, "%s: Failed to read f21 data registers: %d\n",
> +                             __func__, error);
> +                     return IRQ_RETVAL(error);
> +             }
> +             pressed = !!(f21->data_regs[RMI_F21_TABLE_FORCE_CLICK_OFFSET] &
> +                                     RMI_f21_FORCE_CLICK);

Same here.

[Marge 07/14]
I will correct it in next patch.

> +     }
> +
> +     input_report_key(f21->input, f21->key_code, pressed);
> +
> +     return IRQ_HANDLED;
> +}
> +
> +static int rmi_f21_config(struct rmi_function *fn) {
> +     struct f21_data *f21 = dev_get_drvdata(&fn->dev);
> +     struct rmi_driver *drv = fn->rmi_dev->driver;
> +
> +     if (!f21)
> +             return 0;

Is this actually possible for f21 to be NULL here?

[Marge 07/14]
I will remove it in next patch.

> +
> +     drv->set_irq_bits(fn->rmi_dev, fn->irq_mask);
> +
> +     return 0;
> +}
> +
> +static int rmi_f21_initialize(struct rmi_function *fn, struct 
> +f21_data *f21) {
> +     struct input_dev *input = f21->input;
> +     unsigned int button = BTN_LEFT;

This variable is not needed.

[Marge 07/14]
I will correct it in next patch.

> +
> +     f21->key_code = button;

        f21->key_code = BTN_LEFT;

> +     input_set_capability(input, EV_KEY, f21->key_code);
> +     input->keycode = &(f21->key_code);

Drop extra parenthesis.

[Marge 07/14]
I will correct it in next patch.

> +     input->keycodesize = sizeof(f21->key_code);
> +     input->keycodemax = RMI_f21_FORCEPAD_BUTTON_COUNT;
> +
> +     __set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
> +
> +     return 0;
> +}
> +
> +static int rmi_f21_probe(struct rmi_function *fn) {
> +     struct rmi_device *rmi_dev = fn->rmi_dev;
> +     struct rmi_driver_data *drv_data = dev_get_drvdata(&rmi_dev->dev);
> +     struct f21_data *f21;
> +     int error;
> +
> +     if (!drv_data->input) {
> +             dev_info(&fn->dev, "f21: no input device found, ignoring\n");
> +             return -ENXIO;
> +     }
> +
> +     f21 = devm_kzalloc(&fn->dev, sizeof(*f21), GFP_KERNEL);
> +     if (!f21)
> +             return -ENOMEM;
> +
> +     f21->input = drv_data->input;
> +
> +     error = rmi_f21_initialize(fn, f21);
> +     if (error)
> +             return error;
> +
> +     dev_set_drvdata(&fn->dev, f21);
> +     return 0;
> +}
> +
> +struct rmi_function_handler rmi_f21_handler = {
> +     .driver = {
> +             .name = "rmi4_f21",
> +     },
> +     .func = 0x21,
> +     .probe = rmi_f21_probe,
> +     .config = rmi_f21_config,
> +     .attention = rmi_f21_attention,
> +};

Thanks.

--
Dmitry

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-14  3:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-19 11:25 [PATCH V1] Input: synaptics-rmi4- Add a new feature for Forcepad Marge Yang
2025-06-29  5:00 ` Dmitry Torokhov
2025-07-14  3:17   ` Marge Yang

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).