[PATCH 1/3] Input: synaptics-rmi4 - add capabilities for touchpads

[PATCH 1/3] Input: synaptics-rmi4 - add capabilities for touchpads

[Christopher Heiny]

When the device is a touchpad additional capabilities need to
be set and reported.

Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
Acked-by: Christopher Heiny <cheiny@synaptics.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Linux Walleij <linus.walleij@linaro.org>
Cc: David Herrmann <dh.herrmann@gmail.com>
Cc: Jiri Kosina <jkosina@suse.cz>

---
 drivers/input/rmi4/rmi_f11.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 8709abe..07044d79 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -688,6 +688,9 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11,
 	/* MT sync between fingers */
 	if (sensor->type_a)
 		input_mt_sync(sensor->input);
+
+	if (sensor->sensor_type == rmi_f11_sensor_touchpad)
+		input_mt_report_pointer_emulation(sensor->input, true);
 }
 
 static void rmi_f11_finger_handler(struct f11_data *f11,
@@ -717,7 +720,7 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
 		if (sensor->data.rel_pos)
 			rmi_f11_rel_pos_report(sensor, i);
 	}
-	input_mt_sync(sensor->input);
+	input_report_key(sensor->input, BTN_TOUCH, finger_pressed_count);
 	input_sync(sensor->input);
 }
 
@@ -1137,6 +1140,9 @@ static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
 	dev_dbg(&fn->dev, "Set ranges X=[%d..%d] Y=[%d..%d].",
 			x_min, x_max, y_min, y_max);
 
+	input_set_abs_params(input, ABS_X, x_min, x_max, 0, 0);
+	input_set_abs_params(input, ABS_Y, y_min, y_max, 0, 0);
+
 	input_set_abs_params(input, ABS_MT_PRESSURE, 0,
 			DEFAULT_MAX_ABS_MT_PRESSURE, 0, 0);
 	input_set_abs_params(input, ABS_MT_TOUCH_MAJOR,
@@ -1374,6 +1380,15 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
 		set_bit(BTN_RIGHT, input_dev_mouse->keybit);
 	}
 
+	if (sensor->sensor_type == rmi_f11_sensor_touchpad) {
+		set_bit(BTN_TOOL_FINGER, input_dev->keybit);
+		set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
+		set_bit(BTN_TOOL_TRIPLETAP, input_dev->keybit);
+		set_bit(BTN_TOOL_QUADTAP, input_dev->keybit);
+		set_bit(BTN_TOOL_QUINTTAP, input_dev->keybit);
+	}
+
+
 	return 0;
 
 error_unregister:
-- 
1.8.3.2

[PATCH 2/3] Input: synaptics-rmi4 - ability disable abs or rel reporting

[Christopher Heiny]

Even if the RMI4 touchscreen/touchpad provides reporting both
relative and absolute coordinates, reporting both to userspace
could be confusing. Allow the platform data to disable either
absolute or relative coordinates.

Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
Acked-by: Christopher Heiny <cheiny@synaptics.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Linux Walleij <linus.walleij@linaro.org>
Cc: David Herrmann <dh.herrmann@gmail.com>
Cc: Jiri Kosina <jkosina@suse.cz>

---
 drivers/input/rmi4/rmi_f11.c | 78 +++++++++++++++++++++++++++++++++++++-------
 include/linux/rmi.h          |  6 ++++
 2 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 07044d79..c87c6cc3 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -520,6 +520,8 @@ struct f11_2d_sensor {
 	struct rmi_function *fn;
 	char input_phys[NAME_BUFFER_SIZE];
 	char input_phys_mouse[NAME_BUFFER_SIZE];
+	u8 report_abs;
+	u8 report_rel;
 };
 
 /** Data pertaining to F11 in general.  For per-sensor data, see struct
@@ -544,6 +546,10 @@ struct f11_data {
 	struct mutex dev_controls_mutex;
 	u16 rezero_wait_ms;
 	struct f11_2d_sensor sensor;
+	unsigned long *abs_mask;
+	unsigned long *rel_mask;
+	unsigned long *result_bits;
+	unsigned long mask_memory[];
 };
 
 enum finger_state_values {
@@ -591,10 +597,14 @@ static void rmi_f11_rel_pos_report(struct f11_2d_sensor *sensor, u8 n_finger)
 	if (x || y) {
 		input_report_rel(sensor->input, REL_X, x);
 		input_report_rel(sensor->input, REL_Y, y);
-		input_report_rel(sensor->mouse_input, REL_X, x);
-		input_report_rel(sensor->mouse_input, REL_Y, y);
+
+		if (sensor->mouse_input) {
+			input_report_rel(sensor->mouse_input, REL_X, x);
+			input_report_rel(sensor->mouse_input, REL_Y, y);
+		}
 	}
-	input_sync(sensor->mouse_input);
+	if (sensor->mouse_input)
+		input_sync(sensor->mouse_input);
 }
 
 static void rmi_f11_abs_pos_report(struct f11_data *f11,
@@ -694,13 +704,17 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11,
 }
 
 static void rmi_f11_finger_handler(struct f11_data *f11,
-				   struct f11_2d_sensor *sensor)
+				   struct f11_2d_sensor *sensor,
+				   unsigned long *irq_bits, int num_irq_regs)
 {
 	const u8 *f_state = sensor->data.f_state;
 	u8 finger_state;
 	u8 finger_pressed_count;
 	u8 i;
 
+	int rel_bits;
+	int abs_bits;
+
 	for (i = 0, finger_pressed_count = 0; i < sensor->nbr_fingers; i++) {
 		/* Possible of having 4 fingers per f_statet register */
 		finger_state = (f_state[i / 4] >> (2 * (i % 4))) &
@@ -714,13 +728,19 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
 			finger_pressed_count++;
 		}
 
-		if (sensor->data.abs_pos)
+		abs_bits = bitmap_and(f11->result_bits, irq_bits, f11->abs_mask,
+				num_irq_regs);
+		if (abs_bits)
 			rmi_f11_abs_pos_report(f11, sensor, finger_state, i);
 
-		if (sensor->data.rel_pos)
+		rel_bits = bitmap_and(f11->result_bits, irq_bits, f11->rel_mask,
+				num_irq_regs);
+		if (rel_bits)
 			rmi_f11_rel_pos_report(sensor, i);
 	}
+
 	input_report_key(sensor->input, BTN_TOUCH, finger_pressed_count);
+
 	input_sync(sensor->input);
 }
 
@@ -1180,21 +1200,33 @@ static int rmi_f11_initialize(struct rmi_function *fn)
 	u16 max_x_pos, max_y_pos, temp;
 	int rc;
 	const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
+	struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
 	struct f11_2d_sensor *sensor;
 	u8 buf;
+	int mask_size;
 
 	dev_dbg(&fn->dev, "Initializing F11 values for %s.\n",
 		 pdata->sensor_name);
 
+	mask_size = BITS_TO_LONGS(drvdata->irq_count) * sizeof(unsigned long);
+
 	/*
 	** init instance data, fill in values and create any sysfs files
 	*/
-	f11 = devm_kzalloc(&fn->dev, sizeof(struct f11_data), GFP_KERNEL);
+	f11 = devm_kzalloc(&fn->dev, sizeof(struct f11_data) + mask_size * 3,
+			GFP_KERNEL);
 	if (!f11)
 		return -ENOMEM;
 
 	f11->rezero_wait_ms = pdata->f11_rezero_wait;
 
+	f11->abs_mask = f11->mask_memory + mask_size * 0;
+	f11->rel_mask = f11->mask_memory + mask_size * 1;
+	f11->result_bits = f11->mask_memory + mask_size * 2;
+
+	set_bit(fn->irq_pos, f11->abs_mask);
+	set_bit(fn->irq_pos + 1, f11->rel_mask);
+
 	query_base_addr = fn->fd.query_base_addr;
 	control_base_addr = fn->fd.control_base_addr;
 
@@ -1226,12 +1258,25 @@ static int rmi_f11_initialize(struct rmi_function *fn)
 		return rc;
 	}
 
+	sensor->report_rel = sensor->sens_query.has_rel;
+	sensor->report_abs = sensor->sens_query.has_abs;
+
 	if (pdata->f11_sensor_data) {
 		sensor->axis_align =
 			pdata->f11_sensor_data->axis_align;
 		sensor->type_a = pdata->f11_sensor_data->type_a;
 		sensor->sensor_type =
 				pdata->f11_sensor_data->sensor_type;
+
+		if (sensor->sens_query.has_abs)
+			sensor->report_abs = sensor->report_abs
+				&& !(pdata->f11_sensor_data->disable_report_mask
+					& RMI_F11_DISABLE_ABS_REPORT);
+
+		if (sensor->sens_query.has_rel)
+			sensor->report_rel = sensor->report_rel
+				&& !(pdata->f11_sensor_data->disable_report_mask
+					& RMI_F11_DISABLE_REL_REPORT);
 	}
 
 	rc = rmi_read_block(rmi_dev,
@@ -1324,9 +1369,10 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
 	set_bit(EV_ABS, input_dev->evbit);
 	input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
 
-	f11_set_abs_params(fn, f11);
+	if (sensor->report_abs)
+		f11_set_abs_params(fn, f11);
 
-	if (sensor->sens_query.has_rel) {
+	if (sensor->report_rel) {
 		set_bit(EV_REL, input_dev->evbit);
 		set_bit(REL_X, input_dev->relbit);
 		set_bit(REL_Y, input_dev->relbit);
@@ -1338,7 +1384,7 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
 		goto error_unregister;
 	}
 
-	if (sensor->sens_query.has_rel) {
+	if (sensor->report_rel) {
 		/*create input device for mouse events  */
 		input_dev_mouse = input_allocate_device();
 		if (!input_dev_mouse) {
@@ -1407,8 +1453,16 @@ error_unregister:
 static int rmi_f11_config(struct rmi_function *fn)
 {
 	struct f11_data *f11 = dev_get_drvdata(&fn->dev);
+	struct rmi_driver *drv = fn->rmi_dev->driver;
+	struct f11_2d_sensor *sensor = &f11->sensor;
 	int rc;
 
+	if (!sensor->report_abs)
+		drv->clear_irq_bits(fn->rmi_dev, f11->abs_mask);
+
+	if (!sensor->report_rel)
+		drv->clear_irq_bits(fn->rmi_dev, f11->rel_mask);
+
 	rc = f11_write_control_regs(fn, &f11->sensor.sens_query,
 			   &f11->dev_controls, fn->fd.query_base_addr);
 	if (rc < 0)
@@ -1420,6 +1474,7 @@ static int rmi_f11_config(struct rmi_function *fn)
 static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
 {
 	struct rmi_device *rmi_dev = fn->rmi_dev;
+	struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
 	struct f11_data *f11 = dev_get_drvdata(&fn->dev);
 	u16 data_base_addr = fn->fd.data_base_addr;
 	u16 data_base_addr_offset = 0;
@@ -1432,7 +1487,8 @@ static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
 	if (error)
 		return error;
 
-	rmi_f11_finger_handler(f11, &f11->sensor);
+	rmi_f11_finger_handler(f11, &f11->sensor, irq_bits,
+				drvdata->num_of_irq_regs);
 	data_base_addr_offset += f11->sensor.pkt_size;
 
 	return 0;
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index 735e978..a0d0187 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -76,6 +76,9 @@ enum rmi_f11_sensor_type {
 	rmi_f11_sensor_touchpad
 };
 
+#define RMI_F11_DISABLE_ABS_REPORT      (1 << 0)
+#define RMI_F11_DISABLE_REL_REPORT      (1 << 1)
+
 /**
  * struct rmi_f11_sensor_data - overrides defaults for a single F11 2D sensor.
  * @axis_align - provides axis alignment overrides (see above).
@@ -86,11 +89,14 @@ enum rmi_f11_sensor_type {
  * pointing device (touchpad) rather than a direct pointing device
  * (touchscreen).  This is useful when F11_2D_QUERY14 register is not
  * available.
+ * @disable_report_mask - Force data to not be reported even if it is supported
+ * by the firware.
  */
 struct rmi_f11_sensor_data {
 	struct rmi_f11_2d_axis_alignment axis_align;
 	bool type_a;
 	enum rmi_f11_sensor_type sensor_type;
+	int disable_report_mask;
 };
 
 /**
-- 
1.8.3.2

Re: [PATCH 2/3] Input: synaptics-rmi4 - ability disable abs or rel reporting

[Benjamin Tissoires]

On 03/18/2014 09:03 PM, Christopher Heiny wrote:
> Even if the RMI4 touchscreen/touchpad provides reporting both
> relative and absolute coordinates, reporting both to userspace
> could be confusing. Allow the platform data to disable either
> absolute or relative coordinates.

General comments on the patch:
Is there really a need to export the rel axis when there is already an 
abs collection?
I mean, with the RMI4 over HID over I2C found on the XPS Haswell series, 
RMI4 will be bound automatically, and the sensor may (will) pretend that 
it can do both abs and rel. However, we are not using a platform_data 
for them (I think we should not), and we will get the two collections.

I would personally be in favor of having a priority mechanism: if abs is 
here, skip rel, otherwise use rel. But I have no clue if you will ship 
devices which will require both. So you make the call.

>
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
> Acked-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: Linux Walleij <linus.walleij@linaro.org>
> Cc: David Herrmann <dh.herrmann@gmail.com>
> Cc: Jiri Kosina <jkosina@suse.cz>
>
> ---
>   drivers/input/rmi4/rmi_f11.c | 78 +++++++++++++++++++++++++++++++++++++-------
>   include/linux/rmi.h          |  6 ++++
>   2 files changed, 73 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 07044d79..c87c6cc3 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -520,6 +520,8 @@ struct f11_2d_sensor {
>   	struct rmi_function *fn;
>   	char input_phys[NAME_BUFFER_SIZE];
>   	char input_phys_mouse[NAME_BUFFER_SIZE];
> +	u8 report_abs;
> +	u8 report_rel;
>   };
>
>   /** Data pertaining to F11 in general.  For per-sensor data, see struct
> @@ -544,6 +546,10 @@ struct f11_data {
>   	struct mutex dev_controls_mutex;
>   	u16 rezero_wait_ms;
>   	struct f11_2d_sensor sensor;
> +	unsigned long *abs_mask;
> +	unsigned long *rel_mask;
> +	unsigned long *result_bits;
> +	unsigned long mask_memory[];
>   };
>
>   enum finger_state_values {
> @@ -591,10 +597,14 @@ static void rmi_f11_rel_pos_report(struct f11_2d_sensor *sensor, u8 n_finger)
>   	if (x || y) {
>   		input_report_rel(sensor->input, REL_X, x);
>   		input_report_rel(sensor->input, REL_Y, y);
> -		input_report_rel(sensor->mouse_input, REL_X, x);
> -		input_report_rel(sensor->mouse_input, REL_Y, y);
> +
> +		if (sensor->mouse_input) {
> +			input_report_rel(sensor->mouse_input, REL_X, x);
> +			input_report_rel(sensor->mouse_input, REL_Y, y);
> +		}
>   	}
> -	input_sync(sensor->mouse_input);
> +	if (sensor->mouse_input)
> +		input_sync(sensor->mouse_input);
>   }
>
>   static void rmi_f11_abs_pos_report(struct f11_data *f11,
> @@ -694,13 +704,17 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11,
>   }
>
>   static void rmi_f11_finger_handler(struct f11_data *f11,
> -				   struct f11_2d_sensor *sensor)
> +				   struct f11_2d_sensor *sensor,
> +				   unsigned long *irq_bits, int num_irq_regs)
>   {
>   	const u8 *f_state = sensor->data.f_state;
>   	u8 finger_state;
>   	u8 finger_pressed_count;
>   	u8 i;
>
> +	int rel_bits;
> +	int abs_bits;
> +
>   	for (i = 0, finger_pressed_count = 0; i < sensor->nbr_fingers; i++) {
>   		/* Possible of having 4 fingers per f_statet register */
>   		finger_state = (f_state[i / 4] >> (2 * (i % 4))) &
> @@ -714,13 +728,19 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
>   			finger_pressed_count++;
>   		}
>
> -		if (sensor->data.abs_pos)
> +		abs_bits = bitmap_and(f11->result_bits, irq_bits, f11->abs_mask,
> +				num_irq_regs);
> +		if (abs_bits)
>   			rmi_f11_abs_pos_report(f11, sensor, finger_state, i);

rmi_driver.c uses bitmap_and this way:
bitmap_and(data->fn_irq_bits, data->irq_status, fn->irq_mask, 
data->irq_count);
if (!bitmap_empty(data->fn_irq_bits, data->irq_count))
	fh->attention(fn, data->fn_irq_bits);

Not sure which way is the best.

>
> -		if (sensor->data.rel_pos)
> +		rel_bits = bitmap_and(f11->result_bits, irq_bits, f11->rel_mask,
> +				num_irq_regs);
> +		if (rel_bits)
>   			rmi_f11_rel_pos_report(sensor, i);
>   	}
> +
>   	input_report_key(sensor->input, BTN_TOUCH, finger_pressed_count);
> +

those two blank lines are unrelated to the commit.

>   	input_sync(sensor->input);
>   }
>
> @@ -1180,21 +1200,33 @@ static int rmi_f11_initialize(struct rmi_function *fn)
>   	u16 max_x_pos, max_y_pos, temp;
>   	int rc;
>   	const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
> +	struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
>   	struct f11_2d_sensor *sensor;
>   	u8 buf;
> +	int mask_size;
>
>   	dev_dbg(&fn->dev, "Initializing F11 values for %s.\n",
>   		 pdata->sensor_name);
>
> +	mask_size = BITS_TO_LONGS(drvdata->irq_count) * sizeof(unsigned long);
> +
>   	/*
>   	** init instance data, fill in values and create any sysfs files
>   	*/
> -	f11 = devm_kzalloc(&fn->dev, sizeof(struct f11_data), GFP_KERNEL);
> +	f11 = devm_kzalloc(&fn->dev, sizeof(struct f11_data) + mask_size * 3,
> +			GFP_KERNEL);
>   	if (!f11)
>   		return -ENOMEM;
>
>   	f11->rezero_wait_ms = pdata->f11_rezero_wait;
>
> +	f11->abs_mask = f11->mask_memory + mask_size * 0;

I personally don't like the " + mask_size * 0"

Can't you just also remove the mask_memory field and use sizeof(struct 
f11_data)?

> +	f11->rel_mask = f11->mask_memory + mask_size * 1;
> +	f11->result_bits = f11->mask_memory + mask_size * 2;
> +
> +	set_bit(fn->irq_pos, f11->abs_mask);
> +	set_bit(fn->irq_pos + 1, f11->rel_mask);
> +
>   	query_base_addr = fn->fd.query_base_addr;
>   	control_base_addr = fn->fd.control_base_addr;
>
> @@ -1226,12 +1258,25 @@ static int rmi_f11_initialize(struct rmi_function *fn)
>   		return rc;
>   	}
>
> +	sensor->report_rel = sensor->sens_query.has_rel;
> +	sensor->report_abs = sensor->sens_query.has_abs;
> +
>   	if (pdata->f11_sensor_data) {
>   		sensor->axis_align =
>   			pdata->f11_sensor_data->axis_align;
>   		sensor->type_a = pdata->f11_sensor_data->type_a;
>   		sensor->sensor_type =
>   				pdata->f11_sensor_data->sensor_type;
> +
> +		if (sensor->sens_query.has_abs)
> +			sensor->report_abs = sensor->report_abs
> +				&& !(pdata->f11_sensor_data->disable_report_mask
> +					& RMI_F11_DISABLE_ABS_REPORT);

sensor->report_abs already contains sensor->sens_query.has_abs (set few 
lines above)...

so you can just skip the if test here.


> +
> +		if (sensor->sens_query.has_rel)
> +			sensor->report_rel = sensor->report_rel
> +				&& !(pdata->f11_sensor_data->disable_report_mask
> +					& RMI_F11_DISABLE_REL_REPORT);

same here.

>   	}
>
>   	rc = rmi_read_block(rmi_dev,
> @@ -1324,9 +1369,10 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
>   	set_bit(EV_ABS, input_dev->evbit);
>   	input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
>
> -	f11_set_abs_params(fn, f11);
> +	if (sensor->report_abs)
> +		f11_set_abs_params(fn, f11);
>
> -	if (sensor->sens_query.has_rel) {
> +	if (sensor->report_rel) {
>   		set_bit(EV_REL, input_dev->evbit);
>   		set_bit(REL_X, input_dev->relbit);
>   		set_bit(REL_Y, input_dev->relbit);
> @@ -1338,7 +1384,7 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
>   		goto error_unregister;
>   	}
>
> -	if (sensor->sens_query.has_rel) {
> +	if (sensor->report_rel) {
>   		/*create input device for mouse events  */
>   		input_dev_mouse = input_allocate_device();
>   		if (!input_dev_mouse) {
> @@ -1407,8 +1453,16 @@ error_unregister:
>   static int rmi_f11_config(struct rmi_function *fn)
>   {
>   	struct f11_data *f11 = dev_get_drvdata(&fn->dev);
> +	struct rmi_driver *drv = fn->rmi_dev->driver;
> +	struct f11_2d_sensor *sensor = &f11->sensor;
>   	int rc;
>
> +	if (!sensor->report_abs)
> +		drv->clear_irq_bits(fn->rmi_dev, f11->abs_mask);
> +
> +	if (!sensor->report_rel)
> +		drv->clear_irq_bits(fn->rmi_dev, f11->rel_mask);
> +
>   	rc = f11_write_control_regs(fn, &f11->sensor.sens_query,
>   			   &f11->dev_controls, fn->fd.query_base_addr);
>   	if (rc < 0)
> @@ -1420,6 +1474,7 @@ static int rmi_f11_config(struct rmi_function *fn)
>   static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
>   {
>   	struct rmi_device *rmi_dev = fn->rmi_dev;
> +	struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
>   	struct f11_data *f11 = dev_get_drvdata(&fn->dev);
>   	u16 data_base_addr = fn->fd.data_base_addr;
>   	u16 data_base_addr_offset = 0;
> @@ -1432,7 +1487,8 @@ static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
>   	if (error)
>   		return error;
>
> -	rmi_f11_finger_handler(f11, &f11->sensor);
> +	rmi_f11_finger_handler(f11, &f11->sensor, irq_bits,
> +				drvdata->num_of_irq_regs);
>   	data_base_addr_offset += f11->sensor.pkt_size;
>
>   	return 0;
> diff --git a/include/linux/rmi.h b/include/linux/rmi.h
> index 735e978..a0d0187 100644
> --- a/include/linux/rmi.h
> +++ b/include/linux/rmi.h
> @@ -76,6 +76,9 @@ enum rmi_f11_sensor_type {
>   	rmi_f11_sensor_touchpad
>   };
>
> +#define RMI_F11_DISABLE_ABS_REPORT      (1 << 0)
> +#define RMI_F11_DISABLE_REL_REPORT      (1 << 1)

We have BIT() macro in the kernel for this (I know, I do not use it that 
much either... :-P )

Cheers,
Benjamin

> +
>   /**
>    * struct rmi_f11_sensor_data - overrides defaults for a single F11 2D sensor.
>    * @axis_align - provides axis alignment overrides (see above).
> @@ -86,11 +89,14 @@ enum rmi_f11_sensor_type {
>    * pointing device (touchpad) rather than a direct pointing device
>    * (touchscreen).  This is useful when F11_2D_QUERY14 register is not
>    * available.
> + * @disable_report_mask - Force data to not be reported even if it is supported
> + * by the firware.
>    */
>   struct rmi_f11_sensor_data {
>   	struct rmi_f11_2d_axis_alignment axis_align;
>   	bool type_a;
>   	enum rmi_f11_sensor_type sensor_type;
> +	int disable_report_mask;
>   };
>
>   /**
>

Re: [PATCH 2/3] Input: synaptics-rmi4 - ability disable abs or rel reporting

[Christopher Heiny]

On 03/19/2014 08:02 AM, Benjamin Tissoires wrote:
>
>
> On 03/18/2014 09:03 PM, Christopher Heiny wrote:
>> Even if the RMI4 touchscreen/touchpad provides reporting both
>> relative and absolute coordinates, reporting both to userspace
>> could be confusing. Allow the platform data to disable either
>> absolute or relative coordinates.
>
> General comments on the patch:
> Is there really a need to export the rel axis when there is already an
> abs collection?
> I mean, with the RMI4 over HID over I2C found on the XPS Haswell series,
> RMI4 will be bound automatically, and the sensor may (will) pretend that
> it can do both abs and rel. However, we are not using a platform_data
> for them (I think we should not), and we will get the two collections.
>
> I would personally be in favor of having a priority mechanism: if abs is
> here, skip rel, otherwise use rel. But I have no clue if you will ship
> devices which will require both. So you make the call.
>

Hi Benjamin,

That's a great idea.  I'm a tad embarrassed we didn't think of that 
approach.

We do ship products with both abs and rel enabled.  We'll implement the 
abs-priority approach, but also keep the platform data (or device tree, 
once that's implemented) top optionally disable abs reporting, because 
it's very handy to have during new system bring up and certain 
prototyping use cases.

As before, I'll let Andrew handle the code specific comments.

					Cheers,
						Chris

[snip]

Re: [PATCH 2/3] Input: synaptics-rmi4 - ability disable abs or rel reporting

[Andrew Duggan]

Hi Benjamin,

Thanks for reviewing our patches. We are putting together a v2 set based 
on your comments. Everything looks straight forward, I just have one 
comment in line.

On 03/19/2014 08:02 AM, Benjamin Tissoires wrote:
>
>
> On 03/18/2014 09:03 PM, Christopher Heiny wrote:
>> Even if the RMI4 touchscreen/touchpad provides reporting both
>> relative and absolute coordinates, reporting both to userspace
>> could be confusing. Allow the platform data to disable either
>> absolute or relative coordinates.
>
> General comments on the patch:
> Is there really a need to export the rel axis when there is already an 
> abs collection?
> I mean, with the RMI4 over HID over I2C found on the XPS Haswell 
> series, RMI4 will be bound automatically, and the sensor may (will) 
> pretend that it can do both abs and rel. However, we are not using a 
> platform_data for them (I think we should not), and we will get the 
> two collections.
>
> I would personally be in favor of having a priority mechanism: if abs 
> is here, skip rel, otherwise use rel. But I have no clue if you will 
> ship devices which will require both. So you make the call.
>
>>
>> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
>> Acked-by: Christopher Heiny <cheiny@synaptics.com>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> Cc: Linux Walleij <linus.walleij@linaro.org>
>> Cc: David Herrmann <dh.herrmann@gmail.com>
>> Cc: Jiri Kosina <jkosina@suse.cz>
>>
>> ---
>>   drivers/input/rmi4/rmi_f11.c | 78 
>> +++++++++++++++++++++++++++++++++++++-------
>>   include/linux/rmi.h          |  6 ++++
>>   2 files changed, 73 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
>> index 07044d79..c87c6cc3 100644
>> --- a/drivers/input/rmi4/rmi_f11.c
>> +++ b/drivers/input/rmi4/rmi_f11.c
>> @@ -520,6 +520,8 @@ struct f11_2d_sensor {
>>       struct rmi_function *fn;
>>       char input_phys[NAME_BUFFER_SIZE];
>>       char input_phys_mouse[NAME_BUFFER_SIZE];
>> +    u8 report_abs;
>> +    u8 report_rel;
>>   };
>>
>>   /** Data pertaining to F11 in general.  For per-sensor data, see 
>> struct
>> @@ -544,6 +546,10 @@ struct f11_data {
>>       struct mutex dev_controls_mutex;
>>       u16 rezero_wait_ms;
>>       struct f11_2d_sensor sensor;
>> +    unsigned long *abs_mask;
>> +    unsigned long *rel_mask;
>> +    unsigned long *result_bits;
>> +    unsigned long mask_memory[];
>>   };
>>
>>   enum finger_state_values {
>> @@ -591,10 +597,14 @@ static void rmi_f11_rel_pos_report(struct 
>> f11_2d_sensor *sensor, u8 n_finger)
>>       if (x || y) {
>>           input_report_rel(sensor->input, REL_X, x);
>>           input_report_rel(sensor->input, REL_Y, y);
>> -        input_report_rel(sensor->mouse_input, REL_X, x);
>> -        input_report_rel(sensor->mouse_input, REL_Y, y);
>> +
>> +        if (sensor->mouse_input) {
>> +            input_report_rel(sensor->mouse_input, REL_X, x);
>> +            input_report_rel(sensor->mouse_input, REL_Y, y);
>> +        }
>>       }
>> -    input_sync(sensor->mouse_input);
>> +    if (sensor->mouse_input)
>> +        input_sync(sensor->mouse_input);
>>   }
>>
>>   static void rmi_f11_abs_pos_report(struct f11_data *f11,
>> @@ -694,13 +704,17 @@ static void rmi_f11_abs_pos_report(struct 
>> f11_data *f11,
>>   }
>>
>>   static void rmi_f11_finger_handler(struct f11_data *f11,
>> -                   struct f11_2d_sensor *sensor)
>> +                   struct f11_2d_sensor *sensor,
>> +                   unsigned long *irq_bits, int num_irq_regs)
>>   {
>>       const u8 *f_state = sensor->data.f_state;
>>       u8 finger_state;
>>       u8 finger_pressed_count;
>>       u8 i;
>>
>> +    int rel_bits;
>> +    int abs_bits;
>> +
>>       for (i = 0, finger_pressed_count = 0; i < sensor->nbr_fingers; 
>> i++) {
>>           /* Possible of having 4 fingers per f_statet register */
>>           finger_state = (f_state[i / 4] >> (2 * (i % 4))) &
>> @@ -714,13 +728,19 @@ static void rmi_f11_finger_handler(struct 
>> f11_data *f11,
>>               finger_pressed_count++;
>>           }
>>
>> -        if (sensor->data.abs_pos)
>> +        abs_bits = bitmap_and(f11->result_bits, irq_bits, 
>> f11->abs_mask,
>> +                num_irq_regs);
>> +        if (abs_bits)
>>               rmi_f11_abs_pos_report(f11, sensor, finger_state, i);
>
> rmi_driver.c uses bitmap_and this way:
> bitmap_and(data->fn_irq_bits, data->irq_status, fn->irq_mask, 
> data->irq_count);
> if (!bitmap_empty(data->fn_irq_bits, data->irq_count))
>     fh->attention(fn, data->fn_irq_bits);
>
> Not sure which way is the best.
Looks like based on commit f4b0373b26567cafd421d91101852ed7a34e9e94 the 
call to bitmap_empty is redundant so I am going to leave it the way it 
is. We should clean up rmi_driver.c in a future patch.
>
>>
>> -        if (sensor->data.rel_pos)
>> +        rel_bits = bitmap_and(f11->result_bits, irq_bits, 
>> f11->rel_mask,
>> +                num_irq_regs);
>> +        if (rel_bits)
>>               rmi_f11_rel_pos_report(sensor, i);
>>       }
>> +
>>       input_report_key(sensor->input, BTN_TOUCH, finger_pressed_count);
>> +
>
> those two blank lines are unrelated to the commit.
>
>>       input_sync(sensor->input);
>>   }
>>
>> @@ -1180,21 +1200,33 @@ static int rmi_f11_initialize(struct 
>> rmi_function *fn)
>>       u16 max_x_pos, max_y_pos, temp;
>>       int rc;
>>       const struct rmi_device_platform_data *pdata = 
>> rmi_get_platform_data(rmi_dev);
>> +    struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
>>       struct f11_2d_sensor *sensor;
>>       u8 buf;
>> +    int mask_size;
>>
>>       dev_dbg(&fn->dev, "Initializing F11 values for %s.\n",
>>            pdata->sensor_name);
>>
>> +    mask_size = BITS_TO_LONGS(drvdata->irq_count) * sizeof(unsigned 
>> long);
>> +
>>       /*
>>       ** init instance data, fill in values and create any sysfs files
>>       */
>> -    f11 = devm_kzalloc(&fn->dev, sizeof(struct f11_data), GFP_KERNEL);
>> +    f11 = devm_kzalloc(&fn->dev, sizeof(struct f11_data) + mask_size 
>> * 3,
>> +            GFP_KERNEL);
>>       if (!f11)
>>           return -ENOMEM;
>>
>>       f11->rezero_wait_ms = pdata->f11_rezero_wait;
>>
>> +    f11->abs_mask = f11->mask_memory + mask_size * 0;
>
> I personally don't like the " + mask_size * 0"
>
> Can't you just also remove the mask_memory field and use sizeof(struct 
> f11_data)?
>
>> +    f11->rel_mask = f11->mask_memory + mask_size * 1;
>> +    f11->result_bits = f11->mask_memory + mask_size * 2;
>> +
>> +    set_bit(fn->irq_pos, f11->abs_mask);
>> +    set_bit(fn->irq_pos + 1, f11->rel_mask);
>> +
>>       query_base_addr = fn->fd.query_base_addr;
>>       control_base_addr = fn->fd.control_base_addr;
>>
>> @@ -1226,12 +1258,25 @@ static int rmi_f11_initialize(struct 
>> rmi_function *fn)
>>           return rc;
>>       }
>>
>> +    sensor->report_rel = sensor->sens_query.has_rel;
>> +    sensor->report_abs = sensor->sens_query.has_abs;
>> +
>>       if (pdata->f11_sensor_data) {
>>           sensor->axis_align =
>>               pdata->f11_sensor_data->axis_align;
>>           sensor->type_a = pdata->f11_sensor_data->type_a;
>>           sensor->sensor_type =
>>                   pdata->f11_sensor_data->sensor_type;
>> +
>> +        if (sensor->sens_query.has_abs)
>> +            sensor->report_abs = sensor->report_abs
>> +                && !(pdata->f11_sensor_data->disable_report_mask
>> +                    & RMI_F11_DISABLE_ABS_REPORT);
>
> sensor->report_abs already contains sensor->sens_query.has_abs (set 
> few lines above)...
>
> so you can just skip the if test here.
>
>
>> +
>> +        if (sensor->sens_query.has_rel)
>> +            sensor->report_rel = sensor->report_rel
>> +                && !(pdata->f11_sensor_data->disable_report_mask
>> +                    & RMI_F11_DISABLE_REL_REPORT);
>
> same here.
>
>>       }
>>
>>       rc = rmi_read_block(rmi_dev,
>> @@ -1324,9 +1369,10 @@ static int rmi_f11_register_devices(struct 
>> rmi_function *fn)
>>       set_bit(EV_ABS, input_dev->evbit);
>>       input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
>>
>> -    f11_set_abs_params(fn, f11);
>> +    if (sensor->report_abs)
>> +        f11_set_abs_params(fn, f11);
>>
>> -    if (sensor->sens_query.has_rel) {
>> +    if (sensor->report_rel) {
>>           set_bit(EV_REL, input_dev->evbit);
>>           set_bit(REL_X, input_dev->relbit);
>>           set_bit(REL_Y, input_dev->relbit);
>> @@ -1338,7 +1384,7 @@ static int rmi_f11_register_devices(struct 
>> rmi_function *fn)
>>           goto error_unregister;
>>       }
>>
>> -    if (sensor->sens_query.has_rel) {
>> +    if (sensor->report_rel) {
>>           /*create input device for mouse events  */
>>           input_dev_mouse = input_allocate_device();
>>           if (!input_dev_mouse) {
>> @@ -1407,8 +1453,16 @@ error_unregister:
>>   static int rmi_f11_config(struct rmi_function *fn)
>>   {
>>       struct f11_data *f11 = dev_get_drvdata(&fn->dev);
>> +    struct rmi_driver *drv = fn->rmi_dev->driver;
>> +    struct f11_2d_sensor *sensor = &f11->sensor;
>>       int rc;
>>
>> +    if (!sensor->report_abs)
>> +        drv->clear_irq_bits(fn->rmi_dev, f11->abs_mask);
>> +
>> +    if (!sensor->report_rel)
>> +        drv->clear_irq_bits(fn->rmi_dev, f11->rel_mask);
>> +
>>       rc = f11_write_control_regs(fn, &f11->sensor.sens_query,
>>                  &f11->dev_controls, fn->fd.query_base_addr);
>>       if (rc < 0)
>> @@ -1420,6 +1474,7 @@ static int rmi_f11_config(struct rmi_function *fn)
>>   static int rmi_f11_attention(struct rmi_function *fn, unsigned long 
>> *irq_bits)
>>   {
>>       struct rmi_device *rmi_dev = fn->rmi_dev;
>> +    struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
>>       struct f11_data *f11 = dev_get_drvdata(&fn->dev);
>>       u16 data_base_addr = fn->fd.data_base_addr;
>>       u16 data_base_addr_offset = 0;
>> @@ -1432,7 +1487,8 @@ static int rmi_f11_attention(struct 
>> rmi_function *fn, unsigned long *irq_bits)
>>       if (error)
>>           return error;
>>
>> -    rmi_f11_finger_handler(f11, &f11->sensor);
>> +    rmi_f11_finger_handler(f11, &f11->sensor, irq_bits,
>> +                drvdata->num_of_irq_regs);
>>       data_base_addr_offset += f11->sensor.pkt_size;
>>
>>       return 0;
>> diff --git a/include/linux/rmi.h b/include/linux/rmi.h
>> index 735e978..a0d0187 100644
>> --- a/include/linux/rmi.h
>> +++ b/include/linux/rmi.h
>> @@ -76,6 +76,9 @@ enum rmi_f11_sensor_type {
>>       rmi_f11_sensor_touchpad
>>   };
>>
>> +#define RMI_F11_DISABLE_ABS_REPORT      (1 << 0)
>> +#define RMI_F11_DISABLE_REL_REPORT      (1 << 1)
>
> We have BIT() macro in the kernel for this (I know, I do not use it 
> that much either... :-P )
>
> Cheers,
> Benjamin
>
>> +
>>   /**
>>    * struct rmi_f11_sensor_data - overrides defaults for a single F11 
>> 2D sensor.
>>    * @axis_align - provides axis alignment overrides (see above).
>> @@ -86,11 +89,14 @@ enum rmi_f11_sensor_type {
>>    * pointing device (touchpad) rather than a direct pointing device
>>    * (touchscreen).  This is useful when F11_2D_QUERY14 register is not
>>    * available.
>> + * @disable_report_mask - Force data to not be reported even if it 
>> is supported
>> + * by the firware.
>>    */
>>   struct rmi_f11_sensor_data {
>>       struct rmi_f11_2d_axis_alignment axis_align;
>>       bool type_a;
>>       enum rmi_f11_sensor_type sensor_type;
>> +    int disable_report_mask;
>>   };
>>
>>   /**
>>
Andrew

[PATCH 3/3] Input: synaptics-rmi4 - report sensor resolution

[Christopher Heiny]

Reports the sensor resolution by reading the size of the sensor
from F11 query registers or from the platform data if the firmware
does not contain the appropriate query registers.

Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
Acked-by: Christopher Heiny <cheiny@synaptics.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Linux Walleij <linus.walleij@linaro.org>
Cc: David Herrmann <dh.herrmann@gmail.com>
Cc: Jiri Kosina <jkosina@suse.cz>

---
 drivers/input/rmi4/rmi_f11.c | 53 +++++++++++++++++++++++++++++++++++++++++++-
 include/linux/rmi.h          |  2 ++
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index f5b8b71..f2a6f5f 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -408,6 +408,10 @@ struct f11_2d_sensor_queries {
 	u8 clickpad_props;
 	u8 mouse_buttons;
 	bool has_advanced_gestures;
+
+	/* Query 15 - 18 */
+	u16 x_sensor_size_mm;
+	u16 y_sensor_size_mm;
 };
 
 /* Defs for Ctrl0. */
@@ -520,6 +524,8 @@ struct f11_2d_sensor {
 	struct rmi_function *fn;
 	char input_phys[NAME_BUFFER_SIZE];
 	char input_phys_mouse[NAME_BUFFER_SIZE];
+	u8 x_mm;
+	u8 y_mm;
 	u8 report_abs;
 	u8 report_rel;
 };
@@ -1098,6 +1104,20 @@ static int rmi_f11_get_query_parameters(struct rmi_device *rmi_dev,
 		query_size++;
 	}
 
+	if (f11->has_query12 && sensor_query->has_physical_props) {
+		rc = rmi_read_block(rmi_dev, query_base_addr
+			+ query_size, query_buf, ARRAY_SIZE(query_buf));
+		if (rc < 0)
+			return rc;
+
+		sensor_query->x_sensor_size_mm =
+			(query_buf[0] | (query_buf[1] << 8)) / 10;
+		sensor_query->y_sensor_size_mm =
+			(query_buf[2] | (query_buf[3] << 8)) / 10;
+
+		query_size += 4;
+	}
+
 	return query_size;
 }
 
@@ -1119,6 +1139,7 @@ static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
 			((f11->dev_controls.ctrl0_9[9] & 0x0F) << 8);
 	u16 x_min, x_max, y_min, y_max;
 	unsigned int input_flags;
+	int res_x, res_y;
 
 	/* We assume touchscreen unless demonstrably a touchpad or specified
 	 * as a touchpad in the platform data
@@ -1175,6 +1196,18 @@ static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
 			x_min, x_max, 0, 0);
 	input_set_abs_params(input, ABS_MT_POSITION_Y,
 			y_min, y_max, 0, 0);
+
+	if (sensor->x_mm && sensor->y_mm) {
+		res_x = (x_max - x_min) / sensor->x_mm;
+		res_y = (y_max - y_min) / sensor->y_mm;
+
+		input_abs_set_res(input, ABS_X, res_x);
+		input_abs_set_res(input, ABS_Y, res_y);
+
+		input_abs_set_res(input, ABS_MT_POSITION_X, res_x);
+		input_abs_set_res(input, ABS_MT_POSITION_Y, res_y);
+	}
+
 	if (!sensor->type_a)
 		input_mt_init_slots(input, sensor->nbr_fingers, input_flags);
 	if (IS_ENABLED(CONFIG_RMI4_F11_PEN) && sensor->sens_query.has_pen)
@@ -1261,8 +1294,26 @@ static int rmi_f11_initialize(struct rmi_function *fn)
 		sensor->axis_align =
 			pdata->f11_sensor_data->axis_align;
 		sensor->type_a = pdata->f11_sensor_data->type_a;
-		sensor->sensor_type =
+
+		if (sensor->sens_query.has_info2) {
+			if (sensor->sens_query.is_clear)
+				sensor->sensor_type =
+					rmi_f11_sensor_touchscreen;
+			else
+				sensor->sensor_type = rmi_f11_sensor_touchpad;
+		} else {
+			sensor->sensor_type =
 				pdata->f11_sensor_data->sensor_type;
+		}
+
+		if (f11->has_query12
+			&& sensor->sens_query.has_physical_props) {
+			sensor->x_mm = sensor->sens_query.x_sensor_size_mm;
+			sensor->y_mm = sensor->sens_query.y_sensor_size_mm;
+		} else {
+			sensor->x_mm = pdata->f11_sensor_data->x_mm;
+			sensor->y_mm = pdata->f11_sensor_data->y_mm;
+		}
 
 		if (sensor->sens_query.has_abs)
 			sensor->report_abs = sensor->report_abs
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index a0d0187..9139873 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -96,6 +96,8 @@ struct rmi_f11_sensor_data {
 	struct rmi_f11_2d_axis_alignment axis_align;
 	bool type_a;
 	enum rmi_f11_sensor_type sensor_type;
+	int x_mm;
+	int y_mm;
 	int disable_report_mask;
 };
 
-- 
1.8.3.2

Re: [PATCH 3/3] Input: synaptics-rmi4 - report sensor resolution

[Benjamin Tissoires]

On 03/18/2014 09:03 PM, Christopher Heiny wrote:
> Reports the sensor resolution by reading the size of the sensor
> from F11 query registers or from the platform data if the firmware
> does not contain the appropriate query registers.

Hehe, nice, I was just wondering if it was possible to retrieve this 
info from the FW. :)

>
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
> Acked-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: Linux Walleij <linus.walleij@linaro.org>
> Cc: David Herrmann <dh.herrmann@gmail.com>
> Cc: Jiri Kosina <jkosina@suse.cz>
>
> ---
>   drivers/input/rmi4/rmi_f11.c | 53 +++++++++++++++++++++++++++++++++++++++++++-
>   include/linux/rmi.h          |  2 ++
>   2 files changed, 54 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index f5b8b71..f2a6f5f 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -408,6 +408,10 @@ struct f11_2d_sensor_queries {
>   	u8 clickpad_props;
>   	u8 mouse_buttons;
>   	bool has_advanced_gestures;
> +
> +	/* Query 15 - 18 */
> +	u16 x_sensor_size_mm;
> +	u16 y_sensor_size_mm;
>   };
>
>   /* Defs for Ctrl0. */
> @@ -520,6 +524,8 @@ struct f11_2d_sensor {
>   	struct rmi_function *fn;
>   	char input_phys[NAME_BUFFER_SIZE];
>   	char input_phys_mouse[NAME_BUFFER_SIZE];
> +	u8 x_mm;
> +	u8 y_mm;
>   	u8 report_abs;
>   	u8 report_rel;
>   };
> @@ -1098,6 +1104,20 @@ static int rmi_f11_get_query_parameters(struct rmi_device *rmi_dev,
>   		query_size++;
>   	}
>
> +	if (f11->has_query12 && sensor_query->has_physical_props) {

sensor->has_physical_props is only set to true if f11->has_query12 is 
true (the initial struct is zero-allocated).

So we can safely skip the test against f11->has_query12

> +		rc = rmi_read_block(rmi_dev, query_base_addr
> +			+ query_size, query_buf, ARRAY_SIZE(query_buf));
> +		if (rc < 0)
> +			return rc;
> +
> +		sensor_query->x_sensor_size_mm =
> +			(query_buf[0] | (query_buf[1] << 8)) / 10;
> +		sensor_query->y_sensor_size_mm =
> +			(query_buf[2] | (query_buf[3] << 8)) / 10;
> +
> +		query_size += 4;

should be ARRAY_SIZE(query_buf). Or maybe we should use 4 in the 
rmi_read_block call (jsut in case someone changes the size of buf).

> +	}
> +
>   	return query_size;
>   }
>
> @@ -1119,6 +1139,7 @@ static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
>   			((f11->dev_controls.ctrl0_9[9] & 0x0F) << 8);
>   	u16 x_min, x_max, y_min, y_max;
>   	unsigned int input_flags;
> +	int res_x, res_y;
>
>   	/* We assume touchscreen unless demonstrably a touchpad or specified
>   	 * as a touchpad in the platform data
> @@ -1175,6 +1196,18 @@ static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
>   			x_min, x_max, 0, 0);
>   	input_set_abs_params(input, ABS_MT_POSITION_Y,
>   			y_min, y_max, 0, 0);
> +
> +	if (sensor->x_mm && sensor->y_mm) {
> +		res_x = (x_max - x_min) / sensor->x_mm;
> +		res_y = (y_max - y_min) / sensor->y_mm;
> +
> +		input_abs_set_res(input, ABS_X, res_x);
> +		input_abs_set_res(input, ABS_Y, res_y);
> +
> +		input_abs_set_res(input, ABS_MT_POSITION_X, res_x);
> +		input_abs_set_res(input, ABS_MT_POSITION_Y, res_y);
> +	}
> +
>   	if (!sensor->type_a)
>   		input_mt_init_slots(input, sensor->nbr_fingers, input_flags);
>   	if (IS_ENABLED(CONFIG_RMI4_F11_PEN) && sensor->sens_query.has_pen)
> @@ -1261,8 +1294,26 @@ static int rmi_f11_initialize(struct rmi_function *fn)
>   		sensor->axis_align =
>   			pdata->f11_sensor_data->axis_align;
>   		sensor->type_a = pdata->f11_sensor_data->type_a;
> -		sensor->sensor_type =
> +
> +		if (sensor->sens_query.has_info2) {
> +			if (sensor->sens_query.is_clear)
> +				sensor->sensor_type =
> +					rmi_f11_sensor_touchscreen;
> +			else
> +				sensor->sensor_type = rmi_f11_sensor_touchpad;
> +		} else {
> +			sensor->sensor_type =
>   				pdata->f11_sensor_data->sensor_type;

These few lines above are not related to the current commit. Please 
split this.

> +		}
> +
> +		if (f11->has_query12
> +			&& sensor->sens_query.has_physical_props) {

again, f11->has_query12 can be skipped

> +			sensor->x_mm = sensor->sens_query.x_sensor_size_mm;
> +			sensor->y_mm = sensor->sens_query.y_sensor_size_mm;
> +		} else {
> +			sensor->x_mm = pdata->f11_sensor_data->x_mm;
> +			sensor->y_mm = pdata->f11_sensor_data->y_mm;

there is a test regarding pdata->f11_sensor_data in 
rmi_f11_initialize(). So I guess this pointer might be null, and you 
will get an oops.

Cheers,
Benjamin

> +		}
>
>   		if (sensor->sens_query.has_abs)
>   			sensor->report_abs = sensor->report_abs
> diff --git a/include/linux/rmi.h b/include/linux/rmi.h
> index a0d0187..9139873 100644
> --- a/include/linux/rmi.h
> +++ b/include/linux/rmi.h
> @@ -96,6 +96,8 @@ struct rmi_f11_sensor_data {
>   	struct rmi_f11_2d_axis_alignment axis_align;
>   	bool type_a;
>   	enum rmi_f11_sensor_type sensor_type;
> +	int x_mm;
> +	int y_mm;
>   	int disable_report_mask;
>   };
>
>

Re: [PATCH 1/3] Input: synaptics-rmi4 - add capabilities for touchpads

[Benjamin Tissoires]

Hi Chris,

On 03/18/2014 09:03 PM, Christopher Heiny wrote:
> When the device is a touchpad additional capabilities need to
> be set and reported.
>

We have a problem here. While this patch would have been fine in the 
pre-v3.8 era, it is not true anymore.
However, the current branch where synaptics-rmi4 is attached is v3.4.

So if you use the right API (the current one), it will not compile :(

Dmitry, would it be possible to update the branch to at least v3.8 to 
get the new input-mt API? (if the Synaptics guys are ok).

> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
> Acked-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: Linux Walleij <linus.walleij@linaro.org>
> Cc: David Herrmann <dh.herrmann@gmail.com>
> Cc: Jiri Kosina <jkosina@suse.cz>
>
> ---
>   drivers/input/rmi4/rmi_f11.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 8709abe..07044d79 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -688,6 +688,9 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11,
>   	/* MT sync between fingers */
>   	if (sensor->type_a)
>   		input_mt_sync(sensor->input);
> +
> +	if (sensor->sensor_type == rmi_f11_sensor_touchpad)
> +		input_mt_report_pointer_emulation(sensor->input, true);

In recent kernels, you just need to set the input mt flags 
INPUT_MT_POINTER when initializing the slots, and you just need to call 
input_mt_sync_frame at the end of the report. The mt lib will take care 
of the pointer emulation, finger count, etc...

(see commit 55e49089f4589908eb688742d2d7eff33b74ac78)

>   }
>
>   static void rmi_f11_finger_handler(struct f11_data *f11,
> @@ -717,7 +720,7 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
>   		if (sensor->data.rel_pos)
>   			rmi_f11_rel_pos_report(sensor, i);
>   	}
> -	input_mt_sync(sensor->input);

This particular line is a bug in the current implementation. Only 
multitouch protocol A devices should use input_mt_sync.

> +	input_report_key(sensor->input, BTN_TOUCH, finger_pressed_count);

This is already handled by input_mt_sync_frame.

>   	input_sync(sensor->input);
>   }
>
> @@ -1137,6 +1140,9 @@ static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
>   	dev_dbg(&fn->dev, "Set ranges X=[%d..%d] Y=[%d..%d].",
>   			x_min, x_max, y_min, y_max);
>
> +	input_set_abs_params(input, ABS_X, x_min, x_max, 0, 0);
> +	input_set_abs_params(input, ABS_Y, y_min, y_max, 0, 0);
> +

There is no need (and it's not the way you should do) to setup the ABS_X 
and ABS_Y (and ABS_PRESSURE) axis if you call input_mt_init_slot after 
having set all the input mt axis.

As a general rule, set all the mt axis, then call input_mt_init_slot. It 
will handle the single touch emulation for you in a better way (like 
fuzz should not be set for ABS_X|Y otherwise it will be called twice).

>   	input_set_abs_params(input, ABS_MT_PRESSURE, 0,
>   			DEFAULT_MAX_ABS_MT_PRESSURE, 0, 0);
>   	input_set_abs_params(input, ABS_MT_TOUCH_MAJOR,
> @@ -1374,6 +1380,15 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
>   		set_bit(BTN_RIGHT, input_dev_mouse->keybit);
>   	}
>
> +	if (sensor->sensor_type == rmi_f11_sensor_touchpad) {
> +		set_bit(BTN_TOOL_FINGER, input_dev->keybit);
> +		set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
> +		set_bit(BTN_TOOL_TRIPLETAP, input_dev->keybit);
> +		set_bit(BTN_TOOL_QUADTAP, input_dev->keybit);
> +		set_bit(BTN_TOOL_QUINTTAP, input_dev->keybit);
> +	}
> +

This is already handled by input_mt_init_slot with the flag 
INPUT_MT_POINTER.

Cheers,
Benjamin

> +
>   	return 0;
>
>   error_unregister:
>

Re: [PATCH 1/3] Input: synaptics-rmi4 - add capabilities for touchpads

[Christopher Heiny]

On 03/19/2014 07:29 AM, Benjamin Tissoires wrote:
> Hi Chris,
>
> On 03/18/2014 09:03 PM, Christopher Heiny wrote:
>> When the device is a touchpad additional capabilities need to
>> be set and reported.
>>
>
> We have a problem here. While this patch would have been fine in the
> pre-v3.8 era, it is not true anymore.
> However, the current branch where synaptics-rmi4 is attached is v3.4.
>
> So if you use the right API (the current one), it will not compile :(
>
> Dmitry, would it be possible to update the branch to at least v3.8 to
> get the new input-mt API? (if the Synaptics guys are ok).

Hi Benjamin,

Yes, v3.8 migration is OK with us.  Touchpad related development is 
already there, and we're in the process of migrating to v3.8 for the 
touchscreen development work.

>
>> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
>> Acked-by: Christopher Heiny <cheiny@synaptics.com>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> Cc: Linux Walleij <linus.walleij@linaro.org>
>> Cc: David Herrmann <dh.herrmann@gmail.com>
>> Cc: Jiri Kosina <jkosina@suse.cz>

[snip]

Since Andrew is the most familiar with the code in this patchset, I've 
asked him to reply to most of the direct code-related comments.

					Cheers,
						Chris

Re: [PATCH 1/3] Input: synaptics-rmi4 - add capabilities for touchpads

[Dmitry Torokhov]

On Wed, Mar 19, 2014 at 10:29:34AM -0400, Benjamin Tissoires wrote:
> Hi Chris,
> 
> On 03/18/2014 09:03 PM, Christopher Heiny wrote:
> >When the device is a touchpad additional capabilities need to
> >be set and reported.
> >
> 
> We have a problem here. While this patch would have been fine in the
> pre-v3.8 era, it is not true anymore.
> However, the current branch where synaptics-rmi4 is attached is v3.4.
> 
> So if you use the right API (the current one), it will not compile :(
> 
> Dmitry, would it be possible to update the branch to at least v3.8
> to get the new input-mt API? (if the Synaptics guys are ok).

If we are getting ready to pull it into mainline (and I think we are
for F01 and F11 support) then I think I should simply uprev to the
latest released kernel.

Thanks.

-- 
Dmitry

Re: [PATCH 1/3] Input: synaptics-rmi4 - add capabilities for touchpads

[Christopher Heiny]

On 03/28/2014 09:15 AM, Dmitry Torokhov wrote:
> On Wed, Mar 19, 2014 at 10:29:34AM -0400, Benjamin Tissoires wrote:
>> >Hi Chris,
>> >
>> >On 03/18/2014 09:03 PM, Christopher Heiny wrote:
>>> > >When the device is a touchpad additional capabilities need to
>>> > >be set and reported.
>>> > >
>> >
>> >We have a problem here. While this patch would have been fine in the
>> >pre-v3.8 era, it is not true anymore.
>> >However, the current branch where synaptics-rmi4 is attached is v3.4.
>> >
>> >So if you use the right API (the current one), it will not compile :(
>> >
>> >Dmitry, would it be possible to update the branch to at least v3.8
>> >to get the new input-mt API? (if the Synaptics guys are ok).
 >
> If we are getting ready to pull it into mainline (and I think we are
> for F01 and F11 support) then I think I should simply uprev to the
> latest released kernel.

That works for us.

Re: [PATCH 1/3] Input: synaptics-rmi4 - add capabilities for touchpads

[Christopher Heiny]

On 03/28/2014 09:15 AM, Dmitry Torokhov wrote:
> On Wed, Mar 19, 2014 at 10:29:34AM -0400, Benjamin Tissoires wrote:
>> Hi Chris,
>>
>> On 03/18/2014 09:03 PM, Christopher Heiny wrote:
>>> When the device is a touchpad additional capabilities need to
>>> be set and reported.
>>>
>>
>> We have a problem here. While this patch would have been fine in the
>> pre-v3.8 era, it is not true anymore.
>> However, the current branch where synaptics-rmi4 is attached is v3.4.
>>
>> So if you use the right API (the current one), it will not compile :(
>>
>> Dmitry, would it be possible to update the branch to at least v3.8
>> to get the new input-mt API? (if the Synaptics guys are ok).
>
> If we are getting ready to pull it into mainline (and I think we are
> for F01 and F11 support) then I think I should simply uprev to the
> latest released kernel.

Looking at this further, we can rebase pretty easily to 3.9 or 3.10.

New kernels would be more work, as we don't currently have a dev 
platform available for those.  A quick nose around the intertubes shows 
some 3.14 implementations for at least one of our dev platforms 
(BeagleBone Black), so it's not like it would be painful - it'd just 
take soemwhat longer.

In either case, I'd like to get the current patch backlog addressed 
before rebasing.  That way the change set can be limited to those needed 
for the rebase.

Dmitry - how do you want to approach this?

					Chris