linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] staging: ste_rmi4: Convert to Type-B support
@ 2012-11-02 17:36 Alexandra Chin
  2012-11-04 21:53 ` Henrik Rydberg
                   ` (4 more replies)
  0 siblings, 5 replies; 32+ messages in thread
From: Alexandra Chin @ 2012-11-02 17:36 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Henrik and all,

This patch converts to MT-B because Synaptics touch devices are
capable of tracking identifiable fingers

This patch was tested on pandaboard, except input_mt_sync_frame(),
which is a quite new function.
I changed to use sylpheed as my mail client. Please let me know
if there is any problem.
Greatly appreciate your time.

Alexandra Chin

Signed-off-by: Alexandra Chin <alexandra.chin@tw.synaptics.com>
---
Changes from v3:
	- Incorporated Henrik's review comments
	  *remove 'else' after an error path return
	  *add input_mt_sync_frame() for pointer emulation effects
	  *correct names of touchscreen
	- Replace printk with dev_err

Changes from v2:
	- Incorporated Henrik's review comments
	  *directly report finger state with Type-B
	- Against 3.7-rcX
	  *call input_mt_init_slots with INPUT_MT_DIRECT flag

 drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c |  122 ++++++++++++-------------
 1 files changed, 57 insertions(+), 65 deletions(-)

diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
index 277491a..7876f6b 100644
--- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
+++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
@@ -1,7 +1,7 @@
 /**
  *
- * Synaptics Register Mapped Interface (RMI4) I2C Physical Layer Driver.
- * Copyright (c) 2007-2010, Synaptics Incorporated
+ * Synaptics Register Mapped Interface (RMI4) I2C Touchscreen Driver.
+ * Copyright (c) 2007-2012, Synaptics Incorporated
  *
  * Author: Js HA <js.ha@stericsson.com> for ST-Ericsson
  * Author: Naveen Kumar G <naveen.gaddipati@stericsson.com> for ST-Ericsson
@@ -31,6 +31,7 @@
 #include <linux/interrupt.h>
 #include <linux/regulator/consumer.h>
 #include <linux/module.h>
+#include <linux/input/mt.h>
 #include "synaptics_i2c_rmi4.h"
 
 /* TODO: for multiple device support will need a per-device mutex */
@@ -63,12 +64,11 @@
 #define MASK_4BIT		0x0F
 #define MASK_3BIT		0x07
 #define MASK_2BIT		0x03
-#define TOUCHPAD_CTRL_INTR	0x8
+#define TOUCHSCREEN_CTRL_INTR	0x8
 #define PDT_START_SCAN_LOCATION (0x00E9)
 #define PDT_END_SCAN_LOCATION	(0x000A)
 #define PDT_ENTRY_SIZE		(0x0006)
-#define RMI4_NUMBER_OF_MAX_FINGERS		(8)
-#define SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM	(0x11)
+#define SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM	(0x11)
 #define SYNAPTICS_RMI4_DEVICE_CONTROL_FUNC_NUM	(0x01)
 
 /**
@@ -164,6 +164,7 @@ struct synaptics_rmi4_device_info {
  * @regulator: pointer to the regulator structure
  * @wait: wait queue structure variable
  * @touch_stopped: flag to stop the thread function
+ * @fingers_supported: maximum supported fingers
  *
  * This structure gives the device data information.
  */
@@ -184,6 +185,7 @@ struct synaptics_rmi4_data {
 	struct regulator	*regulator;
 	wait_queue_head_t	wait;
 	bool			touch_stopped;
+	unsigned char		fingers_supported;
 };
 
 /**
@@ -291,34 +293,33 @@ exit:
 }
 
 /**
- * synpatics_rmi4_touchpad_report() - reports for the rmi4 touchpad device
+ * synpatics_rmi4_touchscreen_report() - reports for the rmi4 touchscreen device
  * @pdata: pointer to synaptics_rmi4_data structure
  * @rfi: pointer to synaptics_rmi4_fn structure
  *
- * This function calls to reports for the rmi4 touchpad device
+ * This function calls to reports for the rmi4 touchscreen device
  */
-static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
+static int synpatics_rmi4_touchscreen_report(struct synaptics_rmi4_data *pdata,
 						struct synaptics_rmi4_fn *rfi)
 {
 	/* number of touch points - fingers down in this case */
 	int	touch_count = 0;
 	int	finger;
-	int	fingers_supported;
 	int	finger_registers;
 	int	reg;
 	int	finger_shift;
 	int	finger_status;
 	int	retval;
+	int	x, y;
+	int	wx, wy;
 	unsigned short	data_base_addr;
 	unsigned short	data_offset;
 	unsigned char	data_reg_blk_size;
 	unsigned char	values[2];
 	unsigned char	data[DATA_LEN];
-	int	x[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	y[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	wx[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	wy[RMI4_NUMBER_OF_MAX_FINGERS];
+	unsigned char	fingers_supported = pdata->fingers_supported;
 	struct	i2c_client *client = pdata->i2c_client;
+	struct	input_dev *input_dev = pdata->input_dev;
 
 	/* get 2D sensor finger data */
 	/*
@@ -333,7 +334,6 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 	 *	10 = finger present but data may not be accurate,
 	 *	11 = reserved for product use.
 	 */
-	fingers_supported	= rfi->num_of_data_points;
 	finger_registers	= (fingers_supported + 3)/4;
 	data_base_addr		= rfi->fn_desc.data_base_addr;
 	retval = synaptics_rmi4_i2c_block_read(pdata, data_base_addr, values,
@@ -353,12 +353,16 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 		reg = finger/4;
 		/* bit shift to get finger's status */
 		finger_shift	= (finger % 4) * 2;
-		finger_status	= (values[reg] >> finger_shift) & 3;
+		finger_status	= (values[reg] >> finger_shift) & MASK_2BIT;
 		/*
 		 * if finger status indicates a finger is present then
 		 * read the finger data and report it
 		 */
-		if (finger_status == 1 || finger_status == 2) {
+		input_mt_slot(input_dev, finger);
+		input_mt_report_slot_state(input_dev, MT_TOOL_FINGER,
+							finger_status != 0);
+
+		if (finger_status) {
 			/* Read the finger data */
 			data_offset = data_base_addr +
 					((finger * data_reg_blk_size) +
@@ -367,50 +371,33 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 						data_offset, data,
 						data_reg_blk_size);
 			if (retval != data_reg_blk_size) {
-				printk(KERN_ERR "%s:read data failed\n",
+				dev_err(&client->dev, "%s:read data failed\n",
 								__func__);
 				return 0;
-			} else {
-				x[touch_count]	=
-					(data[0] << 4) | (data[2] & MASK_4BIT);
-				y[touch_count]	=
-					(data[1] << 4) |
-					((data[2] >> 4) & MASK_4BIT);
-				wy[touch_count]	=
-						(data[3] >> 4) & MASK_4BIT;
-				wx[touch_count]	=
-						(data[3] & MASK_4BIT);
-
-				if (pdata->board->x_flip)
-					x[touch_count] =
-						pdata->sensor_max_x -
-								x[touch_count];
-				if (pdata->board->y_flip)
-					y[touch_count] =
-						pdata->sensor_max_y -
-								y[touch_count];
 			}
+			x = (data[0] << 4) | (data[2] & MASK_4BIT);
+			y = (data[1] << 4) | ((data[2] >> 4) & MASK_4BIT);
+			wy = (data[3] >> 4) & MASK_4BIT;
+			wx = (data[3] & MASK_4BIT);
+
+			if (pdata->board->x_flip)
+				x = pdata->sensor_max_x - x;
+			if (pdata->board->y_flip)
+				y = pdata->sensor_max_y - y;
+
+			input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
+								max(wx, wy));
+			input_report_abs(input_dev, ABS_MT_POSITION_X, x);
+			input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
+
 			/* number of active touch points */
 			touch_count++;
 		}
 	}
 
-	/* report to input subsystem */
-	if (touch_count) {
-		for (finger = 0; finger < touch_count; finger++) {
-			input_report_abs(pdata->input_dev, ABS_MT_TOUCH_MAJOR,
-						max(wx[finger] , wy[finger]));
-			input_report_abs(pdata->input_dev, ABS_MT_POSITION_X,
-								x[finger]);
-			input_report_abs(pdata->input_dev, ABS_MT_POSITION_Y,
-								y[finger]);
-			input_mt_sync(pdata->input_dev);
-		}
-	} else
-		input_mt_sync(pdata->input_dev);
-
 	/* sync after groups of events */
-	input_sync(pdata->input_dev);
+	input_mt_sync_frame(input_dev);
+	input_sync(input_dev);
 	/* return the number of touch points */
 	return touch_count;
 }
@@ -428,13 +415,13 @@ static int synaptics_rmi4_report_device(struct synaptics_rmi4_data *pdata,
 	int touch = 0;
 	struct	i2c_client *client = pdata->i2c_client;
 	static int num_error_reports;
-	if (rfi->fn_number != SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM) {
+	if (rfi->fn_number != SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM) {
 		num_error_reports++;
 		if (num_error_reports < MAX_ERROR_REPORT)
 			dev_err(&client->dev, "%s:report not supported\n",
 								__func__);
 	} else
-		touch = synpatics_rmi4_touchpad_report(pdata, rfi);
+		touch = synpatics_rmi4_touchscreen_report(pdata, rfi);
 	return touch;
 }
 /**
@@ -510,15 +497,15 @@ static irqreturn_t synaptics_rmi4_irq(int irq, void *data)
 }
 
 /**
- * synpatics_rmi4_touchpad_detect() - detects the rmi4 touchpad device
+ * synpatics_rmi4_touchscreen_detect() - detects the rmi4 touchscreen device
  * @pdata: pointer to synaptics_rmi4_data structure
  * @rfi: pointer to synaptics_rmi4_fn structure
  * @fd: pointer to synaptics_rmi4_fn_desc structure
  * @interruptcount: count the number of interrupts
  *
- * This function calls to detects the rmi4 touchpad device
+ * This function calls to detects the rmi4 touchscreen device
  */
-static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
+static int synpatics_rmi4_touchscreen_detect(struct synaptics_rmi4_data *pdata,
 					struct synaptics_rmi4_fn *rfi,
 					struct synaptics_rmi4_fn_desc *fd,
 					unsigned int interruptcount)
@@ -575,6 +562,7 @@ static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
 		if ((queries[1] & MASK_3BIT) == 5)
 			rfi->num_of_data_points = 10;
 	}
+	pdata->fingers_supported = rfi->num_of_data_points;
 	/* Need to get interrupt info for handling interrupts */
 	rfi->index_to_intr_reg = (interruptcount + 7)/8;
 	if (rfi->index_to_intr_reg != 0)
@@ -655,13 +643,13 @@ static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
 }
 
 /**
- * synaptics_rmi4_touchpad_config() - configures the rmi4 touchpad device
+ * synaptics_rmi4_touchscreen_config() - configures the rmi4 touchscreen device
  * @pdata: pointer to synaptics_rmi4_data structure
  * @rfi: pointer to synaptics_rmi4_fn structure
  *
- * This function calls to configures the rmi4 touchpad device
+ * This function calls to configures the rmi4 touchscreen device
  */
-int synaptics_rmi4_touchpad_config(struct synaptics_rmi4_data *pdata,
+int synaptics_rmi4_touchscreen_config(struct synaptics_rmi4_data *pdata,
 						struct synaptics_rmi4_fn *rfi)
 {
 	/*
@@ -751,7 +739,7 @@ static int synaptics_rmi4_i2c_query_device(struct synaptics_rmi4_data *pdata)
 				pdata->fn01_data_base_addr =
 						rmi_fd.data_base_addr;
 				break;
-			case SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM:
+			case SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM:
 				if (rmi_fd.intr_src_count) {
 					rfi = kmalloc(sizeof(*rfi),
 								GFP_KERNEL);
@@ -761,7 +749,8 @@ static int synaptics_rmi4_i2c_query_device(struct synaptics_rmi4_data *pdata)
 								__func__);
 							return -ENOMEM;
 					}
-					retval = synpatics_rmi4_touchpad_detect
+					retval =
+					synpatics_rmi4_touchscreen_detect
 								(pdata,	rfi,
 								&rmi_fd,
 								intr_count);
@@ -854,8 +843,9 @@ static int synaptics_rmi4_i2c_query_device(struct synaptics_rmi4_data *pdata)
 		list_for_each_entry(rfi, &rmi->support_fn_list, link) {
 			if (rfi->num_of_data_sources) {
 				if (rfi->fn_number ==
-					SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM) {
-					retval = synaptics_rmi4_touchpad_config
+					SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM) {
+					retval =
+					synaptics_rmi4_touchscreen_config
 								(pdata, rfi);
 					if (retval < 0)
 						return retval;
@@ -988,6 +978,8 @@ static int __devinit synaptics_rmi4_probe
 					rmi4_data->sensor_max_y, 0, 0);
 	input_set_abs_params(rmi4_data->input_dev, ABS_MT_TOUCH_MAJOR, 0,
 						MAX_TOUCH_MAJOR, 0, 0);
+	input_mt_init_slots(rmi4_data->input_dev,
+				rmi4_data->fingers_supported, INPUT_MT_DIRECT);
 
 	/* Clear interrupts */
 	synaptics_rmi4_i2c_block_read(rmi4_data,
@@ -1076,7 +1068,7 @@ static int synaptics_rmi4_suspend(struct device *dev)
 
 	retval = synaptics_rmi4_i2c_byte_write(rmi4_data,
 					rmi4_data->fn01_ctrl_base_addr + 1,
-					(intr_status & ~TOUCHPAD_CTRL_INTR));
+					(intr_status & ~TOUCHSCREEN_CTRL_INTR));
 	if (retval < 0)
 		return retval;
 
@@ -1112,7 +1104,7 @@ static int synaptics_rmi4_resume(struct device *dev)
 
 	retval = synaptics_rmi4_i2c_byte_write(rmi4_data,
 					rmi4_data->fn01_ctrl_base_addr + 1,
-					(intr_status | TOUCHPAD_CTRL_INTR));
+					(intr_status | TOUCHSCREEN_CTRL_INTR));
 	if (retval < 0)
 		return retval;
 
-- 
1.7.5.4


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

* Re: [PATCH v3] staging: ste_rmi4: Convert to Type-B support
  2012-11-02 17:36 [PATCH v3] staging: ste_rmi4: Convert to Type-B support Alexandra Chin
@ 2012-11-04 21:53 ` Henrik Rydberg
  2012-11-05  6:01   ` Alexandra Chin
  2012-11-12 11:59   ` Alexandra Chin
  2012-11-07  5:49 ` [PATCH v4] " Alexandra Chin
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 32+ messages in thread
From: Henrik Rydberg @ 2012-11-04 21:53 UTC (permalink / raw)
  To: Alexandra Chin
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Alexandra,

Thanks for making changes.

> This patch converts to MT-B because Synaptics touch devices are
> capable of tracking identifiable fingers
> 
> This patch was tested on pandaboard, except input_mt_sync_frame(),
> which is a quite new function.

I am not sure how to interpret this. Is the patch untested, or tested
on something different from a pandaboard?

> I changed to use sylpheed as my mail client. Please let me know
> if there is any problem.
> Greatly appreciate your time.

As mentioned, the commit message of the patch should not contain mail conversation.

>  drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c |  122 ++++++++++++-------------
>  1 files changed, 57 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> index 277491a..7876f6b 100644
> --- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> +++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> @@ -1,7 +1,7 @@
>  /**
>   *
> - * Synaptics Register Mapped Interface (RMI4) I2C Physical Layer Driver.
> - * Copyright (c) 2007-2010, Synaptics Incorporated
> + * Synaptics Register Mapped Interface (RMI4) I2C Touchscreen Driver.
> + * Copyright (c) 2007-2012, Synaptics Incorporated
>   *
>   * Author: Js HA <js.ha@stericsson.com> for ST-Ericsson
>   * Author: Naveen Kumar G <naveen.gaddipati@stericsson.com> for ST-Ericsson
> @@ -31,6 +31,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/module.h>
> +#include <linux/input/mt.h>
>  #include "synaptics_i2c_rmi4.h"
>  
>  /* TODO: for multiple device support will need a per-device mutex */
> @@ -63,12 +64,11 @@
>  #define MASK_4BIT		0x0F
>  #define MASK_3BIT		0x07
>  #define MASK_2BIT		0x03
> -#define TOUCHPAD_CTRL_INTR	0x8
> +#define TOUCHSCREEN_CTRL_INTR	0x8
>  #define PDT_START_SCAN_LOCATION (0x00E9)
>  #define PDT_END_SCAN_LOCATION	(0x000A)
>  #define PDT_ENTRY_SIZE		(0x0006)
> -#define RMI4_NUMBER_OF_MAX_FINGERS		(8)
> -#define SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM	(0x11)
> +#define SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM	(0x11)
>  #define SYNAPTICS_RMI4_DEVICE_CONTROL_FUNC_NUM	(0x01)
>  
>  /**
> @@ -164,6 +164,7 @@ struct synaptics_rmi4_device_info {
>   * @regulator: pointer to the regulator structure
>   * @wait: wait queue structure variable
>   * @touch_stopped: flag to stop the thread function
> + * @fingers_supported: maximum supported fingers
>   *
>   * This structure gives the device data information.
>   */
> @@ -184,6 +185,7 @@ struct synaptics_rmi4_data {
>  	struct regulator	*regulator;
>  	wait_queue_head_t	wait;
>  	bool			touch_stopped;
> +	unsigned char		fingers_supported;
>  };
>  
>  /**
> @@ -291,34 +293,33 @@ exit:
>  }
>  
>  /**
> - * synpatics_rmi4_touchpad_report() - reports for the rmi4 touchpad device
> + * synpatics_rmi4_touchscreen_report() - reports for the rmi4 touchscreen device
>   * @pdata: pointer to synaptics_rmi4_data structure
>   * @rfi: pointer to synaptics_rmi4_fn structure
>   *
> - * This function calls to reports for the rmi4 touchpad device
> + * This function calls to reports for the rmi4 touchscreen device
>   */
> -static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
> +static int synpatics_rmi4_touchscreen_report(struct synaptics_rmi4_data *pdata,
>  						struct synaptics_rmi4_fn *rfi)
>  {
>  	/* number of touch points - fingers down in this case */
>  	int	touch_count = 0;
>  	int	finger;
> -	int	fingers_supported;
>  	int	finger_registers;
>  	int	reg;
>  	int	finger_shift;
>  	int	finger_status;
>  	int	retval;
> +	int	x, y;
> +	int	wx, wy;
>  	unsigned short	data_base_addr;
>  	unsigned short	data_offset;
>  	unsigned char	data_reg_blk_size;
>  	unsigned char	values[2];
>  	unsigned char	data[DATA_LEN];
> -	int	x[RMI4_NUMBER_OF_MAX_FINGERS];
> -	int	y[RMI4_NUMBER_OF_MAX_FINGERS];
> -	int	wx[RMI4_NUMBER_OF_MAX_FINGERS];
> -	int	wy[RMI4_NUMBER_OF_MAX_FINGERS];
> +	unsigned char	fingers_supported = pdata->fingers_supported;
>  	struct	i2c_client *client = pdata->i2c_client;
> +	struct	input_dev *input_dev = pdata->input_dev;

The patch is definitely an improvement over the existing code, but I
would very much recommend a follow-up patch which splits this
function.

>  
>  	/* get 2D sensor finger data */
>  	/*
> @@ -333,7 +334,6 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
>  	 *	10 = finger present but data may not be accurate,
>  	 *	11 = reserved for product use.
>  	 */
> -	fingers_supported	= rfi->num_of_data_points;
>  	finger_registers	= (fingers_supported + 3)/4;
>  	data_base_addr		= rfi->fn_desc.data_base_addr;
>  	retval = synaptics_rmi4_i2c_block_read(pdata, data_base_addr, values,
> @@ -353,12 +353,16 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
>  		reg = finger/4;
>  		/* bit shift to get finger's status */
>  		finger_shift	= (finger % 4) * 2;
> -		finger_status	= (values[reg] >> finger_shift) & 3;
> +		finger_status	= (values[reg] >> finger_shift) & MASK_2BIT;
>  		/*
>  		 * if finger status indicates a finger is present then
>  		 * read the finger data and report it
>  		 */
> -		if (finger_status == 1 || finger_status == 2) {
> +		input_mt_slot(input_dev, finger);
> +		input_mt_report_slot_state(input_dev, MT_TOOL_FINGER,
> +							finger_status != 0);
> +
> +		if (finger_status) {
>  			/* Read the finger data */
>  			data_offset = data_base_addr +
>  					((finger * data_reg_blk_size) +
> @@ -367,50 +371,33 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
>  						data_offset, data,
>  						data_reg_blk_size);
>  			if (retval != data_reg_blk_size) {
> -				printk(KERN_ERR "%s:read data failed\n",
> +				dev_err(&client->dev, "%s:read data failed\n",
>  								__func__);
>  				return 0;
> -			} else {
> -				x[touch_count]	=
> -					(data[0] << 4) | (data[2] & MASK_4BIT);
> -				y[touch_count]	=
> -					(data[1] << 4) |
> -					((data[2] >> 4) & MASK_4BIT);
> -				wy[touch_count]	=
> -						(data[3] >> 4) & MASK_4BIT;
> -				wx[touch_count]	=
> -						(data[3] & MASK_4BIT);
> -
> -				if (pdata->board->x_flip)
> -					x[touch_count] =
> -						pdata->sensor_max_x -
> -								x[touch_count];
> -				if (pdata->board->y_flip)
> -					y[touch_count] =
> -						pdata->sensor_max_y -
> -								y[touch_count];
>  			}
> +			x = (data[0] << 4) | (data[2] & MASK_4BIT);
> +			y = (data[1] << 4) | ((data[2] >> 4) & MASK_4BIT);
> +			wy = (data[3] >> 4) & MASK_4BIT;
> +			wx = (data[3] & MASK_4BIT);
> +
> +			if (pdata->board->x_flip)
> +				x = pdata->sensor_max_x - x;
> +			if (pdata->board->y_flip)
> +				y = pdata->sensor_max_y - y;
> +
> +			input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
> +								max(wx, wy));
> +			input_report_abs(input_dev, ABS_MT_POSITION_X, x);
> +			input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
> +
>  			/* number of active touch points */
>  			touch_count++;
>  		}
>  	}
>  
> -	/* report to input subsystem */
> -	if (touch_count) {
> -		for (finger = 0; finger < touch_count; finger++) {
> -			input_report_abs(pdata->input_dev, ABS_MT_TOUCH_MAJOR,
> -						max(wx[finger] , wy[finger]));
> -			input_report_abs(pdata->input_dev, ABS_MT_POSITION_X,
> -								x[finger]);
> -			input_report_abs(pdata->input_dev, ABS_MT_POSITION_Y,
> -								y[finger]);
> -			input_mt_sync(pdata->input_dev);
> -		}
> -	} else
> -		input_mt_sync(pdata->input_dev);
> -
>  	/* sync after groups of events */
> -	input_sync(pdata->input_dev);
> +	input_mt_sync_frame(input_dev);
> +	input_sync(input_dev);
>  	/* return the number of touch points */
>  	return touch_count;
>  }
> @@ -428,13 +415,13 @@ static int synaptics_rmi4_report_device(struct synaptics_rmi4_data *pdata,
>  	int touch = 0;
>  	struct	i2c_client *client = pdata->i2c_client;
>  	static int num_error_reports;
> -	if (rfi->fn_number != SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM) {
> +	if (rfi->fn_number != SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM) {
>  		num_error_reports++;
>  		if (num_error_reports < MAX_ERROR_REPORT)
>  			dev_err(&client->dev, "%s:report not supported\n",
>  								__func__);
>  	} else
> -		touch = synpatics_rmi4_touchpad_report(pdata, rfi);
> +		touch = synpatics_rmi4_touchscreen_report(pdata, rfi);
>  	return touch;
>  }
>  /**
> @@ -510,15 +497,15 @@ static irqreturn_t synaptics_rmi4_irq(int irq, void *data)
>  }
>  
>  /**
> - * synpatics_rmi4_touchpad_detect() - detects the rmi4 touchpad device
> + * synpatics_rmi4_touchscreen_detect() - detects the rmi4 touchscreen device
>   * @pdata: pointer to synaptics_rmi4_data structure
>   * @rfi: pointer to synaptics_rmi4_fn structure
>   * @fd: pointer to synaptics_rmi4_fn_desc structure
>   * @interruptcount: count the number of interrupts
>   *
> - * This function calls to detects the rmi4 touchpad device
> + * This function calls to detects the rmi4 touchscreen device
>   */
> -static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
> +static int synpatics_rmi4_touchscreen_detect(struct synaptics_rmi4_data *pdata,
>  					struct synaptics_rmi4_fn *rfi,
>  					struct synaptics_rmi4_fn_desc *fd,
>  					unsigned int interruptcount)
> @@ -575,6 +562,7 @@ static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
>  		if ((queries[1] & MASK_3BIT) == 5)
>  			rfi->num_of_data_points = 10;
>  	}
> +	pdata->fingers_supported = rfi->num_of_data_points;
>  	/* Need to get interrupt info for handling interrupts */
>  	rfi->index_to_intr_reg = (interruptcount + 7)/8;
>  	if (rfi->index_to_intr_reg != 0)
> @@ -655,13 +643,13 @@ static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
>  }
>  
>  /**
> - * synaptics_rmi4_touchpad_config() - configures the rmi4 touchpad device
> + * synaptics_rmi4_touchscreen_config() - configures the rmi4 touchscreen device
>   * @pdata: pointer to synaptics_rmi4_data structure
>   * @rfi: pointer to synaptics_rmi4_fn structure
>   *
> - * This function calls to configures the rmi4 touchpad device
> + * This function calls to configures the rmi4 touchscreen device
>   */
> -int synaptics_rmi4_touchpad_config(struct synaptics_rmi4_data *pdata,
> +int synaptics_rmi4_touchscreen_config(struct synaptics_rmi4_data *pdata,
>  						struct synaptics_rmi4_fn *rfi)
>  {
>  	/*
> @@ -751,7 +739,7 @@ static int synaptics_rmi4_i2c_query_device(struct synaptics_rmi4_data *pdata)
>  				pdata->fn01_data_base_addr =
>  						rmi_fd.data_base_addr;
>  				break;
> -			case SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM:
> +			case SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM:
>  				if (rmi_fd.intr_src_count) {
>  					rfi = kmalloc(sizeof(*rfi),
>  								GFP_KERNEL);
> @@ -761,7 +749,8 @@ static int synaptics_rmi4_i2c_query_device(struct synaptics_rmi4_data *pdata)
>  								__func__);
>  							return -ENOMEM;
>  					}
> -					retval = synpatics_rmi4_touchpad_detect
> +					retval =
> +					synpatics_rmi4_touchscreen_detect
>  								(pdata,	rfi,
>  								&rmi_fd,
>  								intr_count);

Odd line break is a clear sign that something could be broken out into its own function.

> @@ -854,8 +843,9 @@ static int synaptics_rmi4_i2c_query_device(struct synaptics_rmi4_data *pdata)
>  		list_for_each_entry(rfi, &rmi->support_fn_list, link) {
>  			if (rfi->num_of_data_sources) {
>  				if (rfi->fn_number ==
> -					SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM) {
> -					retval = synaptics_rmi4_touchpad_config
> +					SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM) {
> +					retval =
> +					synaptics_rmi4_touchscreen_config
>  								(pdata, rfi);
>  					if (retval < 0)
>  						return retval;

Same here.

> @@ -988,6 +978,8 @@ static int __devinit synaptics_rmi4_probe
>  					rmi4_data->sensor_max_y, 0, 0);
>  	input_set_abs_params(rmi4_data->input_dev, ABS_MT_TOUCH_MAJOR, 0,
>  						MAX_TOUCH_MAJOR, 0, 0);
> +	input_mt_init_slots(rmi4_data->input_dev,
> +				rmi4_data->fingers_supported, INPUT_MT_DIRECT);
>  
>  	/* Clear interrupts */
>  	synaptics_rmi4_i2c_block_read(rmi4_data,
> @@ -1076,7 +1068,7 @@ static int synaptics_rmi4_suspend(struct device *dev)
>  
>  	retval = synaptics_rmi4_i2c_byte_write(rmi4_data,
>  					rmi4_data->fn01_ctrl_base_addr + 1,
> -					(intr_status & ~TOUCHPAD_CTRL_INTR));
> +					(intr_status & ~TOUCHSCREEN_CTRL_INTR));
>  	if (retval < 0)
>  		return retval;
>  
> @@ -1112,7 +1104,7 @@ static int synaptics_rmi4_resume(struct device *dev)
>  
>  	retval = synaptics_rmi4_i2c_byte_write(rmi4_data,
>  					rmi4_data->fn01_ctrl_base_addr + 1,
> -					(intr_status | TOUCHPAD_CTRL_INTR));
> +					(intr_status | TOUCHSCREEN_CTRL_INTR));
>  	if (retval < 0)
>  		return retval;
>  
> -- 
> 1.7.5.4
> 

Ok, with a final polish and a proper commit message, this looks like it will work.

Thanks,
Henrik

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

* RE: [PATCH v3] staging: ste_rmi4: Convert to Type-B support
  2012-11-04 21:53 ` Henrik Rydberg
@ 2012-11-05  6:01   ` Alexandra Chin
  2012-11-12 11:59   ` Alexandra Chin
  1 sibling, 0 replies; 32+ messages in thread
From: Alexandra Chin @ 2012-11-05  6:01 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Henrik,

> > This patch converts to MT-B because Synaptics touch devices are
> > capable of tracking identifiable fingers
> >
> > This patch was tested on pandaboard, except input_mt_sync_frame(),
> > which is a quite new function.
> 
> I am not sure how to interpret this. Is the patch untested, or tested
> on something different from a pandaboard?

My validation platform is pandaboard. However kernel running on pandaboard
Have not been updated to 3.7, therefore kernel does not contain function
input_mt_sync_frame(). When I tested my patch on pandaboard, I modified
the patch to call function input_mt_report_pointer_emulation(), instead of
calling function input_mt_sync_frame(). I think calling 
input_mt_report_pointer_emulation() or input_mt_sync_frame()
should have the same results.
Please let me know if my understanding is incorrect.

> > I changed to use sylpheed as my mail client. Please let me know
> > if there is any problem.
> > Greatly appreciate your time.
> 
> As mentioned, the commit message of the patch should not contain mail
> conversation.

Oh, I see! I should move conversation message to the outside of the patch 
content. Thanks for your reminder.

> >  drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c |  122
> ++++++++++++-------------
> >  1 files changed, 57 insertions(+), 65 deletions(-)
> >
> > diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > index 277491a..7876f6b 100644
> > --- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > +++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > @@ -1,7 +1,7 @@
> >  /**
> >   *
> > - * Synaptics Register Mapped Interface (RMI4) I2C Physical Layer Driver.
> > - * Copyright (c) 2007-2010, Synaptics Incorporated
> > + * Synaptics Register Mapped Interface (RMI4) I2C Touchscreen Driver.
> > + * Copyright (c) 2007-2012, Synaptics Incorporated
> >   *
> >   * Author: Js HA <js.ha@stericsson.com> for ST-Ericsson
> >   * Author: Naveen Kumar G <naveen.gaddipati@stericsson.com> for
> ST-Ericsson
> > @@ -31,6 +31,7 @@
> >  #include <linux/interrupt.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/module.h>
> > +#include <linux/input/mt.h>
> >  #include "synaptics_i2c_rmi4.h"
> >
> >  /* TODO: for multiple device support will need a per-device mutex */
> > @@ -63,12 +64,11 @@
> >  #define MASK_4BIT		0x0F
> >  #define MASK_3BIT		0x07
> >  #define MASK_2BIT		0x03
> > -#define TOUCHPAD_CTRL_INTR	0x8
> > +#define TOUCHSCREEN_CTRL_INTR	0x8
> >  #define PDT_START_SCAN_LOCATION (0x00E9)
> >  #define PDT_END_SCAN_LOCATION	(0x000A)
> >  #define PDT_ENTRY_SIZE		(0x0006)
> > -#define RMI4_NUMBER_OF_MAX_FINGERS		(8)
> > -#define SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM	(0x11)
> > +#define SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM	(0x11)
> >  #define SYNAPTICS_RMI4_DEVICE_CONTROL_FUNC_NUM	(0x01)
> >
> >  /**
> > @@ -164,6 +164,7 @@ struct synaptics_rmi4_device_info {
> >   * @regulator: pointer to the regulator structure
> >   * @wait: wait queue structure variable
> >   * @touch_stopped: flag to stop the thread function
> > + * @fingers_supported: maximum supported fingers
> >   *
> >   * This structure gives the device data information.
> >   */
> > @@ -184,6 +185,7 @@ struct synaptics_rmi4_data {
> >  	struct regulator	*regulator;
> >  	wait_queue_head_t	wait;
> >  	bool			touch_stopped;
> > +	unsigned char		fingers_supported;
> >  };
> >
> >  /**
> > @@ -291,34 +293,33 @@ exit:
> >  }
> >
> >  /**
> > - * synpatics_rmi4_touchpad_report() - reports for the rmi4 touchpad device
> > + * synpatics_rmi4_touchscreen_report() - reports for the rmi4 touchscreen
> device
> >   * @pdata: pointer to synaptics_rmi4_data structure
> >   * @rfi: pointer to synaptics_rmi4_fn structure
> >   *
> > - * This function calls to reports for the rmi4 touchpad device
> > + * This function calls to reports for the rmi4 touchscreen device
> >   */
> > -static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
> > +static int synpatics_rmi4_touchscreen_report(struct synaptics_rmi4_data
> *pdata,
> >  						struct synaptics_rmi4_fn *rfi)
> >  {
> >  	/* number of touch points - fingers down in this case */
> >  	int	touch_count = 0;
> >  	int	finger;
> > -	int	fingers_supported;
> >  	int	finger_registers;
> >  	int	reg;
> >  	int	finger_shift;
> >  	int	finger_status;
> >  	int	retval;
> > +	int	x, y;
> > +	int	wx, wy;
> >  	unsigned short	data_base_addr;
> >  	unsigned short	data_offset;
> >  	unsigned char	data_reg_blk_size;
> >  	unsigned char	values[2];
> >  	unsigned char	data[DATA_LEN];
> > -	int	x[RMI4_NUMBER_OF_MAX_FINGERS];
> > -	int	y[RMI4_NUMBER_OF_MAX_FINGERS];
> > -	int	wx[RMI4_NUMBER_OF_MAX_FINGERS];
> > -	int	wy[RMI4_NUMBER_OF_MAX_FINGERS];
> > +	unsigned char	fingers_supported = pdata->fingers_supported;
> >  	struct	i2c_client *client = pdata->i2c_client;
> > +	struct	input_dev *input_dev = pdata->input_dev;
> 
> The patch is definitely an improvement over the existing code, but I
> would very much recommend a follow-up patch which splits this
> function.
> 

I agree.

> >
> >  	/* get 2D sensor finger data */
> >  	/*
> > @@ -333,7 +334,6 @@ static int synpatics_rmi4_touchpad_report(struct
> synaptics_rmi4_data *pdata,
> >  	 *	10 = finger present but data may not be accurate,
> >  	 *	11 = reserved for product use.
> >  	 */
> > -	fingers_supported	= rfi->num_of_data_points;
> >  	finger_registers	= (fingers_supported + 3)/4;
> >  	data_base_addr		= rfi->fn_desc.data_base_addr;
> >  	retval = synaptics_rmi4_i2c_block_read(pdata, data_base_addr, values,
> > @@ -353,12 +353,16 @@ static int synpatics_rmi4_touchpad_report(struct
> synaptics_rmi4_data *pdata,
> >  		reg = finger/4;
> >  		/* bit shift to get finger's status */
> >  		finger_shift	= (finger % 4) * 2;
> > -		finger_status	= (values[reg] >> finger_shift) & 3;
> > +		finger_status	= (values[reg] >> finger_shift) & MASK_2BIT;
> >  		/*
> >  		 * if finger status indicates a finger is present then
> >  		 * read the finger data and report it
> >  		 */
> > -		if (finger_status == 1 || finger_status == 2) {
> > +		input_mt_slot(input_dev, finger);
> > +		input_mt_report_slot_state(input_dev, MT_TOOL_FINGER,
> > +							finger_status != 0);
> > +
> > +		if (finger_status) {
> >  			/* Read the finger data */
> >  			data_offset = data_base_addr +
> >  					((finger * data_reg_blk_size) +
> > @@ -367,50 +371,33 @@ static int synpatics_rmi4_touchpad_report(struct
> synaptics_rmi4_data *pdata,
> >  						data_offset, data,
> >  						data_reg_blk_size);
> >  			if (retval != data_reg_blk_size) {
> > -				printk(KERN_ERR "%s:read data failed\n",
> > +				dev_err(&client->dev, "%s:read data failed\n",
> >  								__func__);
> >  				return 0;
> > -			} else {
> > -				x[touch_count]	=
> > -					(data[0] << 4) | (data[2] & MASK_4BIT);
> > -				y[touch_count]	=
> > -					(data[1] << 4) |
> > -					((data[2] >> 4) & MASK_4BIT);
> > -				wy[touch_count]	=
> > -						(data[3] >> 4) & MASK_4BIT;
> > -				wx[touch_count]	=
> > -						(data[3] & MASK_4BIT);
> > -
> > -				if (pdata->board->x_flip)
> > -					x[touch_count] =
> > -						pdata->sensor_max_x -
> > -								x[touch_count];
> > -				if (pdata->board->y_flip)
> > -					y[touch_count] =
> > -						pdata->sensor_max_y -
> > -								y[touch_count];
> >  			}
> > +			x = (data[0] << 4) | (data[2] & MASK_4BIT);
> > +			y = (data[1] << 4) | ((data[2] >> 4) & MASK_4BIT);
> > +			wy = (data[3] >> 4) & MASK_4BIT;
> > +			wx = (data[3] & MASK_4BIT);
> > +
> > +			if (pdata->board->x_flip)
> > +				x = pdata->sensor_max_x - x;
> > +			if (pdata->board->y_flip)
> > +				y = pdata->sensor_max_y - y;
> > +
> > +			input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
> > +								max(wx, wy));
> > +			input_report_abs(input_dev, ABS_MT_POSITION_X, x);
> > +			input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
> > +
> >  			/* number of active touch points */
> >  			touch_count++;
> >  		}
> >  	}
> >
> > -	/* report to input subsystem */
> > -	if (touch_count) {
> > -		for (finger = 0; finger < touch_count; finger++) {
> > -			input_report_abs(pdata->input_dev, ABS_MT_TOUCH_MAJOR,
> > -						max(wx[finger] , wy[finger]));
> > -			input_report_abs(pdata->input_dev, ABS_MT_POSITION_X,
> > -								x[finger]);
> > -			input_report_abs(pdata->input_dev, ABS_MT_POSITION_Y,
> > -								y[finger]);
> > -			input_mt_sync(pdata->input_dev);
> > -		}
> > -	} else
> > -		input_mt_sync(pdata->input_dev);
> > -
> >  	/* sync after groups of events */
> > -	input_sync(pdata->input_dev);
> > +	input_mt_sync_frame(input_dev);
> > +	input_sync(input_dev);
> >  	/* return the number of touch points */
> >  	return touch_count;
> >  }
> > @@ -428,13 +415,13 @@ static int synaptics_rmi4_report_device(struct
> synaptics_rmi4_data *pdata,
> >  	int touch = 0;
> >  	struct	i2c_client *client = pdata->i2c_client;
> >  	static int num_error_reports;
> > -	if (rfi->fn_number != SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM) {
> > +	if (rfi->fn_number != SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM) {
> >  		num_error_reports++;
> >  		if (num_error_reports < MAX_ERROR_REPORT)
> >  			dev_err(&client->dev, "%s:report not supported\n",
> >  								__func__);
> >  	} else
> > -		touch = synpatics_rmi4_touchpad_report(pdata, rfi);
> > +		touch = synpatics_rmi4_touchscreen_report(pdata, rfi);
> >  	return touch;
> >  }
> >  /**
> > @@ -510,15 +497,15 @@ static irqreturn_t synaptics_rmi4_irq(int irq, void
> *data)
> >  }
> >
> >  /**
> > - * synpatics_rmi4_touchpad_detect() - detects the rmi4 touchpad device
> > + * synpatics_rmi4_touchscreen_detect() - detects the rmi4 touchscreen device
> >   * @pdata: pointer to synaptics_rmi4_data structure
> >   * @rfi: pointer to synaptics_rmi4_fn structure
> >   * @fd: pointer to synaptics_rmi4_fn_desc structure
> >   * @interruptcount: count the number of interrupts
> >   *
> > - * This function calls to detects the rmi4 touchpad device
> > + * This function calls to detects the rmi4 touchscreen device
> >   */
> > -static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
> > +static int synpatics_rmi4_touchscreen_detect(struct synaptics_rmi4_data
> *pdata,
> >  					struct synaptics_rmi4_fn *rfi,
> >  					struct synaptics_rmi4_fn_desc *fd,
> >  					unsigned int interruptcount)
> > @@ -575,6 +562,7 @@ static int synpatics_rmi4_touchpad_detect(struct
> synaptics_rmi4_data *pdata,
> >  		if ((queries[1] & MASK_3BIT) == 5)
> >  			rfi->num_of_data_points = 10;
> >  	}
> > +	pdata->fingers_supported = rfi->num_of_data_points;
> >  	/* Need to get interrupt info for handling interrupts */
> >  	rfi->index_to_intr_reg = (interruptcount + 7)/8;
> >  	if (rfi->index_to_intr_reg != 0)
> > @@ -655,13 +643,13 @@ static int synpatics_rmi4_touchpad_detect(struct
> synaptics_rmi4_data *pdata,
> >  }
> >
> >  /**
> > - * synaptics_rmi4_touchpad_config() - configures the rmi4 touchpad device
> > + * synaptics_rmi4_touchscreen_config() - configures the rmi4 touchscreen
> device
> >   * @pdata: pointer to synaptics_rmi4_data structure
> >   * @rfi: pointer to synaptics_rmi4_fn structure
> >   *
> > - * This function calls to configures the rmi4 touchpad device
> > + * This function calls to configures the rmi4 touchscreen device
> >   */
> > -int synaptics_rmi4_touchpad_config(struct synaptics_rmi4_data *pdata,
> > +int synaptics_rmi4_touchscreen_config(struct synaptics_rmi4_data *pdata,
> >  						struct synaptics_rmi4_fn *rfi)
> >  {
> >  	/*
> > @@ -751,7 +739,7 @@ static int synaptics_rmi4_i2c_query_device(struct
> synaptics_rmi4_data *pdata)
> >  				pdata->fn01_data_base_addr =
> >  						rmi_fd.data_base_addr;
> >  				break;
> > -			case SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM:
> > +			case SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM:
> >  				if (rmi_fd.intr_src_count) {
> >  					rfi = kmalloc(sizeof(*rfi),
> >  								GFP_KERNEL);
> > @@ -761,7 +749,8 @@ static int synaptics_rmi4_i2c_query_device(struct
> synaptics_rmi4_data *pdata)
> >  								__func__);
> >  							return -ENOMEM;
> >  					}
> > -					retval = synpatics_rmi4_touchpad_detect
> > +					retval =
> > +					synpatics_rmi4_touchscreen_detect
> >  								(pdata,	rfi,
> >  								&rmi_fd,
> >  								intr_count);
> 
> Odd line break is a clear sign that something could be broken out into its own
> function.
> 

I totally agree with you.

> > @@ -854,8 +843,9 @@ static int synaptics_rmi4_i2c_query_device(struct
> synaptics_rmi4_data *pdata)
> >  		list_for_each_entry(rfi, &rmi->support_fn_list, link) {
> >  			if (rfi->num_of_data_sources) {
> >  				if (rfi->fn_number ==
> > -					SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM) {
> > -					retval = synaptics_rmi4_touchpad_config
> > +					SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM) {
> > +					retval =
> > +					synaptics_rmi4_touchscreen_config
> >  								(pdata, rfi);
> >  					if (retval < 0)
> >  						return retval;
> 
> Same here.
> 
> > @@ -988,6 +978,8 @@ static int __devinit synaptics_rmi4_probe
> >  					rmi4_data->sensor_max_y, 0, 0);
> >  	input_set_abs_params(rmi4_data->input_dev, ABS_MT_TOUCH_MAJOR, 0,
> >  						MAX_TOUCH_MAJOR, 0, 0);
> > +	input_mt_init_slots(rmi4_data->input_dev,
> > +				rmi4_data->fingers_supported, INPUT_MT_DIRECT);
> >
> >  	/* Clear interrupts */
> >  	synaptics_rmi4_i2c_block_read(rmi4_data,
> > @@ -1076,7 +1068,7 @@ static int synaptics_rmi4_suspend(struct device *dev)
> >
> >  	retval = synaptics_rmi4_i2c_byte_write(rmi4_data,
> >  					rmi4_data->fn01_ctrl_base_addr + 1,
> > -					(intr_status & ~TOUCHPAD_CTRL_INTR));
> > +					(intr_status & ~TOUCHSCREEN_CTRL_INTR));
> >  	if (retval < 0)
> >  		return retval;
> >
> > @@ -1112,7 +1104,7 @@ static int synaptics_rmi4_resume(struct device *dev)
> >
> >  	retval = synaptics_rmi4_i2c_byte_write(rmi4_data,
> >  					rmi4_data->fn01_ctrl_base_addr + 1,
> > -					(intr_status | TOUCHPAD_CTRL_INTR));
> > +					(intr_status | TOUCHSCREEN_CTRL_INTR));
> >  	if (retval < 0)
> >  		return retval;
> >
> > --
> > 1.7.5.4
> >
> 
> Ok, with a final polish and a proper commit message, this looks like it will work.
> 
> Thanks,
> Henrik

I will fix these issues and recommit a patch ASAP.
I really appreciate your recommendation.

Alexandra


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

* [PATCH v4] staging: ste_rmi4: Convert to Type-B support
  2012-11-02 17:36 [PATCH v3] staging: ste_rmi4: Convert to Type-B support Alexandra Chin
  2012-11-04 21:53 ` Henrik Rydberg
@ 2012-11-07  5:49 ` Alexandra Chin
  2012-11-07 19:57   ` Henrik Rydberg
  2012-11-08  7:05 ` [PATCH v5] " Alexandra Chin
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 32+ messages in thread
From: Alexandra Chin @ 2012-11-07  5:49 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Convert to MT-B because Synaptics touch devices are capable of tracking
identifiable fingers.

Signed-off-by: Alexandra Chin <alexandra.chin@tw.synaptics.com>
---
Changes from v4:
        - Incorporated Henrik's review comments
          *split function synpatics_rmi4_touchscreen_report
          *split function synaptics_rmi4_i2c_query_device

Changes from v3:
	- Incorporated Henrik's review comments
	  *remove 'else' after an error path return
	  *add input_mt_sync_frame() for pointer emulation effects
	  *correct names of touchscreen
	- Replace printk with dev_err

Changes from v2:
	- Incorporated Henrik's review comments
	  *directly report finger state with Type-B
	- Against 3.7-rcX
	  *call input_mt_init_slots with INPUT_MT_DIRECT flag
---
 drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c |  375 ++++++++++++++-----------
 1 files changed, 215 insertions(+), 160 deletions(-)

diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
index 277491a..ef3fd0c 100644
--- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
+++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
@@ -1,10 +1,11 @@
 /**
  *
- * Synaptics Register Mapped Interface (RMI4) I2C Physical Layer Driver.
- * Copyright (c) 2007-2010, Synaptics Incorporated
+ * Synaptics Register Mapped Interface (RMI4) I2C Touchscreen Driver.
+ * Copyright (c) 2007-2012, Synaptics Incorporated
  *
  * Author: Js HA <js.ha@stericsson.com> for ST-Ericsson
  * Author: Naveen Kumar G <naveen.gaddipati@stericsson.com> for ST-Ericsson
+ * Author: Alexandra Chin <alexandra.chin@tw.synaptics.com>
  * Copyright 2010 (c) ST-Ericsson AB
  */
 /*
@@ -31,6 +32,7 @@
 #include <linux/interrupt.h>
 #include <linux/regulator/consumer.h>
 #include <linux/module.h>
+#include <linux/input/mt.h>
 #include "synaptics_i2c_rmi4.h"
 
 /* TODO: for multiple device support will need a per-device mutex */
@@ -63,12 +65,11 @@
 #define MASK_4BIT		0x0F
 #define MASK_3BIT		0x07
 #define MASK_2BIT		0x03
-#define TOUCHPAD_CTRL_INTR	0x8
+#define TOUCHSCREEN_CTRL_INTR	0x8
 #define PDT_START_SCAN_LOCATION (0x00E9)
 #define PDT_END_SCAN_LOCATION	(0x000A)
 #define PDT_ENTRY_SIZE		(0x0006)
-#define RMI4_NUMBER_OF_MAX_FINGERS		(8)
-#define SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM	(0x11)
+#define SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM	(0x11)
 #define SYNAPTICS_RMI4_DEVICE_CONTROL_FUNC_NUM	(0x01)
 
 /**
@@ -164,6 +165,7 @@ struct synaptics_rmi4_device_info {
  * @regulator: pointer to the regulator structure
  * @wait: wait queue structure variable
  * @touch_stopped: flag to stop the thread function
+ * @fingers_supported: maximum supported fingers
  *
  * This structure gives the device data information.
  */
@@ -184,6 +186,8 @@ struct synaptics_rmi4_data {
 	struct regulator	*regulator;
 	wait_queue_head_t	wait;
 	bool			touch_stopped;
+	unsigned char		fingers_supported;
+	int			finger_status_register_count;
 };
 
 /**
@@ -291,34 +295,100 @@ exit:
 }
 
 /**
- * synpatics_rmi4_touchpad_report() - reports for the rmi4 touchpad device
+ * synpatics_rmi4_finger_report() - finger reports
  * @pdata: pointer to synaptics_rmi4_data structure
  * @rfi: pointer to synaptics_rmi4_fn structure
+ * @finger: finger index
+ * @values: pointer to buffer of status registers
  *
- * This function calls to reports for the rmi4 touchpad device
+ * This function calls to report multi-finger data to input subsystem
+ * and returns true if finger status is non zero
  */
-static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
+static bool synpatics_rmi4_finger_report(struct synaptics_rmi4_data *pdata,
+						struct synaptics_rmi4_fn *rfi,
+						int finger,
+						unsigned char *values)
+{
+	int	retval;
+	int	x, y;
+	int	wx, wy;
+	int	reg;
+	int	finger_shift;
+	int	finger_status;
+	int	finger_registers = pdata->finger_status_register_count;
+	unsigned char	data[DATA_LEN];
+	unsigned char	data_reg_blk_size = rfi->size_of_data_register_block;
+	unsigned short	data_offset;
+	unsigned short	data_base_addr = rfi->fn_desc.data_base_addr;
+	struct	i2c_client *client = pdata->i2c_client;
+	struct	input_dev *input_dev = pdata->input_dev;
+
+	/* determine which data byte the finger status is in */
+	reg = finger / 4;
+	/* bit shift to get finger's status */
+	finger_shift	= (finger % 4) * 2;
+	finger_status	= (values[reg] >> finger_shift) & MASK_2BIT;
+
+	/*
+	 * if finger status indicates a finger is present then
+	 * read the finger data and report it
+	 */
+	input_mt_slot(input_dev, finger);
+	input_mt_report_slot_state(input_dev, MT_TOOL_FINGER,
+						finger_status != 0);
+	if (finger_status) {
+		/* Read the finger data */
+		data_offset = data_base_addr +
+				((finger * data_reg_blk_size) +
+				finger_registers);
+		retval = synaptics_rmi4_i2c_block_read(pdata,
+					data_offset, data,
+					data_reg_blk_size);
+		if (retval != data_reg_blk_size) {
+			dev_err(&client->dev, "%s:read data failed\n",
+							__func__);
+			return false;
+		}
+		x = (data[0] << 4) | (data[2] & MASK_4BIT);
+		y = (data[1] << 4) | ((data[2] >> 4) & MASK_4BIT);
+		wy = (data[3] >> 4) & MASK_4BIT;
+		wx = (data[3] & MASK_4BIT);
+
+		if (pdata->board->x_flip)
+			x = pdata->sensor_max_x - x;
+		if (pdata->board->y_flip)
+			y = pdata->sensor_max_y - y;
+
+		input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
+							max(wx, wy));
+		input_report_abs(input_dev, ABS_MT_POSITION_X, x);
+		input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
+		return true;
+	}
+	return false;
+}
+
+/**
+ * synpatics_rmi4_touchscreen_report() - reports for the rmi4
+ * touchscreen device
+ * @pdata: pointer to synaptics_rmi4_data structure
+ * @rfi: pointer to synaptics_rmi4_fn structure
+ *
+ * This function calls to reports for the rmi4 touchscreen device
+ */
+static int synpatics_rmi4_touchscreen_report(struct synaptics_rmi4_data *pdata,
 						struct synaptics_rmi4_fn *rfi)
 {
 	/* number of touch points - fingers down in this case */
 	int	touch_count = 0;
 	int	finger;
-	int	fingers_supported;
-	int	finger_registers;
-	int	reg;
-	int	finger_shift;
-	int	finger_status;
+	int	finger_registers = pdata->finger_status_register_count;
 	int	retval;
 	unsigned short	data_base_addr;
-	unsigned short	data_offset;
-	unsigned char	data_reg_blk_size;
 	unsigned char	values[2];
-	unsigned char	data[DATA_LEN];
-	int	x[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	y[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	wx[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	wy[RMI4_NUMBER_OF_MAX_FINGERS];
-	struct	i2c_client *client = pdata->i2c_client;
+	unsigned char	fingers_supported = pdata->fingers_supported;
+	struct i2c_client *client = pdata->i2c_client;
+	struct input_dev *input_dev = pdata->input_dev;
 
 	/* get 2D sensor finger data */
 	/*
@@ -333,8 +403,6 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 	 *	10 = finger present but data may not be accurate,
 	 *	11 = reserved for product use.
 	 */
-	fingers_supported	= rfi->num_of_data_points;
-	finger_registers	= (fingers_supported + 3)/4;
 	data_base_addr		= rfi->fn_desc.data_base_addr;
 	retval = synaptics_rmi4_i2c_block_read(pdata, data_base_addr, values,
 							finger_registers);
@@ -347,70 +415,14 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 	 * For each finger present, read the proper number of registers
 	 * to get absolute data.
 	 */
-	data_reg_blk_size = rfi->size_of_data_register_block;
 	for (finger = 0; finger < fingers_supported; finger++) {
-		/* determine which data byte the finger status is in */
-		reg = finger/4;
-		/* bit shift to get finger's status */
-		finger_shift	= (finger % 4) * 2;
-		finger_status	= (values[reg] >> finger_shift) & 3;
-		/*
-		 * if finger status indicates a finger is present then
-		 * read the finger data and report it
-		 */
-		if (finger_status == 1 || finger_status == 2) {
-			/* Read the finger data */
-			data_offset = data_base_addr +
-					((finger * data_reg_blk_size) +
-					finger_registers);
-			retval = synaptics_rmi4_i2c_block_read(pdata,
-						data_offset, data,
-						data_reg_blk_size);
-			if (retval != data_reg_blk_size) {
-				printk(KERN_ERR "%s:read data failed\n",
-								__func__);
-				return 0;
-			} else {
-				x[touch_count]	=
-					(data[0] << 4) | (data[2] & MASK_4BIT);
-				y[touch_count]	=
-					(data[1] << 4) |
-					((data[2] >> 4) & MASK_4BIT);
-				wy[touch_count]	=
-						(data[3] >> 4) & MASK_4BIT;
-				wx[touch_count]	=
-						(data[3] & MASK_4BIT);
-
-				if (pdata->board->x_flip)
-					x[touch_count] =
-						pdata->sensor_max_x -
-								x[touch_count];
-				if (pdata->board->y_flip)
-					y[touch_count] =
-						pdata->sensor_max_y -
-								y[touch_count];
-			}
-			/* number of active touch points */
-			touch_count++;
-		}
+		touch_count += synpatics_rmi4_finger_report(pdata, rfi, finger,
+								values);
 	}
 
-	/* report to input subsystem */
-	if (touch_count) {
-		for (finger = 0; finger < touch_count; finger++) {
-			input_report_abs(pdata->input_dev, ABS_MT_TOUCH_MAJOR,
-						max(wx[finger] , wy[finger]));
-			input_report_abs(pdata->input_dev, ABS_MT_POSITION_X,
-								x[finger]);
-			input_report_abs(pdata->input_dev, ABS_MT_POSITION_Y,
-								y[finger]);
-			input_mt_sync(pdata->input_dev);
-		}
-	} else
-		input_mt_sync(pdata->input_dev);
-
 	/* sync after groups of events */
-	input_sync(pdata->input_dev);
+	input_mt_sync_frame(input_dev);
+	input_sync(input_dev);
 	/* return the number of touch points */
 	return touch_count;
 }
@@ -428,15 +440,16 @@ static int synaptics_rmi4_report_device(struct synaptics_rmi4_data *pdata,
 	int touch = 0;
 	struct	i2c_client *client = pdata->i2c_client;
 	static int num_error_reports;
-	if (rfi->fn_number != SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM) {
+	if (rfi->fn_number != SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM) {
 		num_error_reports++;
 		if (num_error_reports < MAX_ERROR_REPORT)
 			dev_err(&client->dev, "%s:report not supported\n",
 								__func__);
 	} else
-		touch = synpatics_rmi4_touchpad_report(pdata, rfi);
+		touch = synpatics_rmi4_touchscreen_report(pdata, rfi);
 	return touch;
 }
+
 /**
  * synaptics_rmi4_sensor_report() - reports to input subsystem
  * @pdata: pointer to synaptics_rmi4_data structure
@@ -448,10 +461,10 @@ static int synaptics_rmi4_sensor_report(struct synaptics_rmi4_data *pdata)
 {
 	unsigned char	intr_status[4];
 	/* number of touch points - fingers or buttons */
-	int touch = 0;
-	unsigned int retval;
-	struct synaptics_rmi4_fn		*rfi;
-	struct synaptics_rmi4_device_info	*rmi;
+	int		touch = 0;
+	unsigned int	retval;
+	struct	synaptics_rmi4_fn		*rfi;
+	struct	synaptics_rmi4_device_info	*rmi;
 	struct	i2c_client *client = pdata->i2c_client;
 
 	/*
@@ -510,18 +523,18 @@ static irqreturn_t synaptics_rmi4_irq(int irq, void *data)
 }
 
 /**
- * synpatics_rmi4_touchpad_detect() - detects the rmi4 touchpad device
+ * synpatics_rmi4_touchscreen_detect() - detects the rmi4 touchscreen device
  * @pdata: pointer to synaptics_rmi4_data structure
  * @rfi: pointer to synaptics_rmi4_fn structure
  * @fd: pointer to synaptics_rmi4_fn_desc structure
- * @interruptcount: count the number of interrupts
+ * @intr_count: count the number of interrupts
  *
- * This function calls to detects the rmi4 touchpad device
+ * This function calls to detects the rmi4 touchscreen device
  */
-static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
+static int synpatics_rmi4_touchscreen_detect(struct synaptics_rmi4_data *pdata,
 					struct synaptics_rmi4_fn *rfi,
 					struct synaptics_rmi4_fn_desc *fd,
-					unsigned int interruptcount)
+					unsigned int intr_count)
 {
 	unsigned char	queries[QUERY_LEN];
 	unsigned short	intr_offset;
@@ -575,15 +588,18 @@ static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
 		if ((queries[1] & MASK_3BIT) == 5)
 			rfi->num_of_data_points = 10;
 	}
+	pdata->fingers_supported = rfi->num_of_data_points;
+	pdata->finger_status_register_count = (rfi->num_of_data_points + 3) / 4;
+
 	/* Need to get interrupt info for handling interrupts */
-	rfi->index_to_intr_reg = (interruptcount + 7)/8;
+	rfi->index_to_intr_reg = (intr_count + 7) / 8;
 	if (rfi->index_to_intr_reg != 0)
 		rfi->index_to_intr_reg -= 1;
 	/*
 	 * loop through interrupts for each source in fn $11
 	 * and or in a bit to the interrupt mask for each.
 	 */
-	intr_offset = interruptcount % 8;
+	intr_offset = intr_count % 8;
 	rfi->intr_mask = 0;
 	for (i = intr_offset;
 		i < ((fd->intr_src_count & MASK_3BIT) + intr_offset); i++)
@@ -655,13 +671,13 @@ static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
 }
 
 /**
- * synaptics_rmi4_touchpad_config() - configures the rmi4 touchpad device
+ * synaptics_rmi4_touchscreen_config() - configures the rmi4 touchscreen device
  * @pdata: pointer to synaptics_rmi4_data structure
  * @rfi: pointer to synaptics_rmi4_fn structure
  *
- * This function calls to configures the rmi4 touchpad device
+ * This function calls to configures the rmi4 touchscreen device
  */
-int synaptics_rmi4_touchpad_config(struct synaptics_rmi4_data *pdata,
+int synaptics_rmi4_touchscreen_config(struct synaptics_rmi4_data *pdata,
 						struct synaptics_rmi4_fn *rfi)
 {
 	/*
@@ -702,6 +718,95 @@ int synaptics_rmi4_touchpad_config(struct synaptics_rmi4_data *pdata,
 }
 
 /**
+ * synaptics_rmi4_query_function() - query rmi4 functions
+ * @pdata: pointer to synaptics_rmi4_data structure
+ * @rfi: pointer to synaptics_rmi4_fn structure
+ *
+ * This function is used to query rmi4 functions.
+ */
+static int synaptics_rmi4_query_function(struct synaptics_rmi4_data *pdata,
+						struct synaptics_rmi4_fn *rfi)
+{
+	int	retval;
+	unsigned int	ctrl_offset;
+	struct	synaptics_rmi4_device_info *rmi;
+	struct	i2c_client *client = pdata->i2c_client;
+
+	rmi = &(pdata->rmi4_mod_info);
+	list_for_each_entry(rfi, &rmi->support_fn_list, link) {
+		if (rfi->num_of_data_sources) {
+			if (rfi->fn_number ==
+				SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM) {
+				retval = synaptics_rmi4_touchscreen_config(
+								pdata, rfi);
+				if (retval < 0)
+					return retval;
+			} else
+				dev_err(&client->dev,
+					"%s:fn_number not supported\n",
+							__func__);
+			/*
+			 * Turn on interrupts for this
+			 * function's data sources.
+			 */
+			ctrl_offset = pdata->fn01_ctrl_base_addr + 1 +
+						rfi->index_to_intr_reg;
+			retval = synaptics_rmi4_i2c_byte_write(pdata,
+						ctrl_offset,
+						rfi->intr_mask);
+			if (retval < 0)
+				return retval;
+		}
+	}
+	return 0;
+}
+
+/**
+ * synaptics_rmi4_explore_function() - explore rmi4 functions
+ * @pdata: pointer to synaptics_rmi4_data structure
+ * @rmi_fd: pointer to synaptics_rmi4_fn_desc structure
+ * @rfi: pointer to point of synaptics_rmi4_fn structure
+ * @intr_count: count the number of interrupts
+ *
+ * This function is used to explore rmi4 functions.
+ */
+static int synaptics_rmi4_explore_function(struct synaptics_rmi4_data *pdata,
+					struct synaptics_rmi4_fn_desc *rmi_fd,
+					struct synaptics_rmi4_fn **rfi,
+					unsigned int intr_count)
+{
+	int retval;
+	struct i2c_client *client = pdata->i2c_client;
+
+	switch (rmi_fd->fn_number) {
+	case SYNAPTICS_RMI4_DEVICE_CONTROL_FUNC_NUM:
+		pdata->fn01_query_base_addr =
+				rmi_fd->query_base_addr;
+		pdata->fn01_ctrl_base_addr =
+				rmi_fd->ctrl_base_addr;
+		pdata->fn01_data_base_addr =
+				rmi_fd->data_base_addr;
+		break;
+	case SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM:
+		if (rmi_fd->intr_src_count) {
+			*rfi = kmalloc(sizeof(**rfi), GFP_KERNEL);
+			if (!rfi) {
+				dev_err(&client->dev, "%s:kmalloc failed\n",
+						__func__);
+				return -ENOMEM;
+			}
+			retval = synpatics_rmi4_touchscreen_detect(pdata,
+						*rfi, rmi_fd, intr_count);
+			if (retval < 0) {
+				kfree(*rfi);
+				return retval;
+			}
+		}
+		break;
+	}
+	return 0;
+}
+/**
  * synaptics_rmi4_i2c_query_device() - query the rmi4 device
  * @pdata: pointer to synaptics_rmi4_data structure
  *
@@ -711,13 +816,11 @@ static int synaptics_rmi4_i2c_query_device(struct synaptics_rmi4_data *pdata)
 {
 	int i;
 	int retval;
+	int data_sources = 0;
 	unsigned char std_queries[STD_QUERY_LEN];
 	unsigned char intr_count = 0;
-	int data_sources = 0;
-	unsigned int ctrl_offset;
 	struct synaptics_rmi4_fn *rfi;
 	struct synaptics_rmi4_fn_desc	rmi_fd;
-	struct synaptics_rmi4_device_info *rmi;
 	struct	i2c_client *client = pdata->i2c_client;
 
 	/*
@@ -742,38 +845,13 @@ static int synaptics_rmi4_i2c_query_device(struct synaptics_rmi4_data *pdata)
 		}
 		rfi = NULL;
 		if (rmi_fd.fn_number) {
-			switch (rmi_fd.fn_number & MASK_8BIT) {
-			case SYNAPTICS_RMI4_DEVICE_CONTROL_FUNC_NUM:
-				pdata->fn01_query_base_addr =
-						rmi_fd.query_base_addr;
-				pdata->fn01_ctrl_base_addr =
-						rmi_fd.ctrl_base_addr;
-				pdata->fn01_data_base_addr =
-						rmi_fd.data_base_addr;
-				break;
-			case SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM:
-				if (rmi_fd.intr_src_count) {
-					rfi = kmalloc(sizeof(*rfi),
-								GFP_KERNEL);
-					if (!rfi) {
-						dev_err(&client->dev,
-							"%s:kmalloc failed\n",
-								__func__);
-							return -ENOMEM;
-					}
-					retval = synpatics_rmi4_touchpad_detect
-								(pdata,	rfi,
-								&rmi_fd,
-								intr_count);
-					if (retval < 0) {
-						kfree(rfi);
-						return retval;
-					}
-				}
-				break;
-			}
+			retval = synaptics_rmi4_explore_function(pdata, &rmi_fd,
+							&rfi, intr_count);
+			if (retval < 0)
+				return retval;
 			/* interrupt count for next iteration */
 			intr_count += (rmi_fd.intr_src_count & MASK_3BIT);
+
 			/*
 			 * We only want to add functions to the list
 			 * that have data associated with them.
@@ -850,32 +928,7 @@ static int synaptics_rmi4_i2c_query_device(struct synaptics_rmi4_data *pdata)
 	list_for_each_entry(rfi, &pdata->rmi4_mod_info.support_fn_list, link)
 		data_sources += rfi->num_of_data_sources;
 	if (data_sources) {
-		rmi = &(pdata->rmi4_mod_info);
-		list_for_each_entry(rfi, &rmi->support_fn_list, link) {
-			if (rfi->num_of_data_sources) {
-				if (rfi->fn_number ==
-					SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM) {
-					retval = synaptics_rmi4_touchpad_config
-								(pdata, rfi);
-					if (retval < 0)
-						return retval;
-				} else
-					dev_err(&client->dev,
-						"%s:fn_number not supported\n",
-								__func__);
-				/*
-				 * Turn on interrupts for this
-				 * function's data sources.
-				 */
-				ctrl_offset = pdata->fn01_ctrl_base_addr + 1 +
-							rfi->index_to_intr_reg;
-				retval = synaptics_rmi4_i2c_byte_write(pdata,
-							ctrl_offset,
-							rfi->intr_mask);
-				if (retval < 0)
-					return retval;
-			}
-		}
+		return synaptics_rmi4_query_function(pdata, rfi);
 	}
 	return 0;
 }
@@ -988,6 +1041,8 @@ static int __devinit synaptics_rmi4_probe
 					rmi4_data->sensor_max_y, 0, 0);
 	input_set_abs_params(rmi4_data->input_dev, ABS_MT_TOUCH_MAJOR, 0,
 						MAX_TOUCH_MAJOR, 0, 0);
+	input_mt_init_slots(rmi4_data->input_dev,
+				rmi4_data->fingers_supported, INPUT_MT_DIRECT);
 
 	/* Clear interrupts */
 	synaptics_rmi4_i2c_block_read(rmi4_data,
@@ -1076,7 +1131,7 @@ static int synaptics_rmi4_suspend(struct device *dev)
 
 	retval = synaptics_rmi4_i2c_byte_write(rmi4_data,
 					rmi4_data->fn01_ctrl_base_addr + 1,
-					(intr_status & ~TOUCHPAD_CTRL_INTR));
+					(intr_status & ~TOUCHSCREEN_CTRL_INTR));
 	if (retval < 0)
 		return retval;
 
@@ -1112,7 +1167,7 @@ static int synaptics_rmi4_resume(struct device *dev)
 
 	retval = synaptics_rmi4_i2c_byte_write(rmi4_data,
 					rmi4_data->fn01_ctrl_base_addr + 1,
-					(intr_status | TOUCHPAD_CTRL_INTR));
+					(intr_status | TOUCHSCREEN_CTRL_INTR));
 	if (retval < 0)
 		return retval;
 
-- 
1.7.5.4


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

* Re: [PATCH v4] staging: ste_rmi4: Convert to Type-B support
  2012-11-07  5:49 ` [PATCH v4] " Alexandra Chin
@ 2012-11-07 19:57   ` Henrik Rydberg
  2012-11-08  4:01     ` Alexandra Chin
  0 siblings, 1 reply; 32+ messages in thread
From: Henrik Rydberg @ 2012-11-07 19:57 UTC (permalink / raw)
  To: Alexandra Chin
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Alexandra,

> Convert to MT-B because Synaptics touch devices are capable of tracking
> identifiable fingers.
> 
> Signed-off-by: Alexandra Chin <alexandra.chin@tw.synaptics.com>
> ---
> Changes from v4:
>         - Incorporated Henrik's review comments
>           *split function synpatics_rmi4_touchscreen_report
>           *split function synaptics_rmi4_i2c_query_device

Thanks for the changes. The patch looks cleaner, but also much larger,
containing many changes irrelevant to the commit message (and to what
has hitherto been reviewed). Please resend a minimum patch, there is
plenty of room for additional patches after this one. :-)

Thanks,
Henrik

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

* RE: [PATCH v4] staging: ste_rmi4: Convert to Type-B support
  2012-11-07 19:57   ` Henrik Rydberg
@ 2012-11-08  4:01     ` Alexandra Chin
  0 siblings, 0 replies; 32+ messages in thread
From: Alexandra Chin @ 2012-11-08  4:01 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Henrik.

> > Convert to MT-B because Synaptics touch devices are capable of tracking
> > identifiable fingers.
> >
> > Signed-off-by: Alexandra Chin <alexandra.chin@tw.synaptics.com>
> > ---
> > Changes from v4:
> >         - Incorporated Henrik's review comments
> >           *split function synpatics_rmi4_touchscreen_report
> >           *split function synaptics_rmi4_i2c_query_device
> 
> Thanks for the changes. The patch looks cleaner, but also much larger,
> containing many changes irrelevant to the commit message (and to what
> has hitherto been reviewed). Please resend a minimum patch, there is
> plenty of room for additional patches after this one. :-)
> 
> Thanks,
> Henrik

Appreciate your review!
I think there is not much of change in patch v4, but git diff tool makes it 
look larger.
Since patch v4 is considered too large, let me resubmit a minimum patch,
and update changes of v4 in future patch.
Thank you for your advice.

Alexandra

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

* [PATCH v5] staging: ste_rmi4: Convert to Type-B support
  2012-11-02 17:36 [PATCH v3] staging: ste_rmi4: Convert to Type-B support Alexandra Chin
  2012-11-04 21:53 ` Henrik Rydberg
  2012-11-07  5:49 ` [PATCH v4] " Alexandra Chin
@ 2012-11-08  7:05 ` Alexandra Chin
  2012-11-09 19:26   ` Henrik Rydberg
  2012-11-13 12:12 ` [PATCH v6] " Alexandra Chin
  2012-11-16  8:31 ` [PATCH v6] staging: ste_rmi4: Convert to Type-B support Alexandra Chin
  4 siblings, 1 reply; 32+ messages in thread
From: Alexandra Chin @ 2012-11-08  7:05 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Convert to MT-B because Synaptics touch devices are capable
of tracking identifiable fingers.

Signed-off-by: Alexandra Chin <alexandra.chin@tw.synaptics.com>
---
Changes from v5:
        - Incorporated Henrik's review comments
          *rollback to v3 from v4
          *fix odd line break in v3
        - Include Alexandra in the list of authors

Changes from v4:
        - Incorporated Henrik's review comments
          *split function synpatics_rmi4_touchscreen_report
          *split function synaptics_rmi4_i2c_query_device

Changes from v3:
	- Incorporated Henrik's review comments
	  *remove 'else' after an error path return
	  *add input_mt_sync_frame() for pointer emulation effects
	  *correct names of touchscreen
	- Replace printk with dev_err

Changes from v2:
	- Incorporated Henrik's review comments
	  *directly report finger state with Type-B
	- Against 3.7-rcX
	  *call input_mt_init_slots with INPUT_MT_DIRECT flag
---
 drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c |  245 ++++++++++++-------------
 1 files changed, 114 insertions(+), 131 deletions(-)

diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
index 277491a..7b636c7 100644
--- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
+++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
@@ -1,10 +1,11 @@
 /**
  *
- * Synaptics Register Mapped Interface (RMI4) I2C Physical Layer Driver.
- * Copyright (c) 2007-2010, Synaptics Incorporated
+ * Synaptics Register Mapped Interface (RMI4) I2C Touchscreen Driver.
+ * Copyright (c) 2007-2012, Synaptics Incorporated
  *
  * Author: Js HA <js.ha@stericsson.com> for ST-Ericsson
  * Author: Naveen Kumar G <naveen.gaddipati@stericsson.com> for ST-Ericsson
+ * Author: Alexandra Chin <alexandra.chin@tw.synaptics.com>
  * Copyright 2010 (c) ST-Ericsson AB
  */
 /*
@@ -31,6 +32,7 @@
 #include <linux/interrupt.h>
 #include <linux/regulator/consumer.h>
 #include <linux/module.h>
+#include <linux/input/mt.h>
 #include "synaptics_i2c_rmi4.h"
 
 /* TODO: for multiple device support will need a per-device mutex */
@@ -63,12 +65,11 @@
 #define MASK_4BIT		0x0F
 #define MASK_3BIT		0x07
 #define MASK_2BIT		0x03
-#define TOUCHPAD_CTRL_INTR	0x8
+#define TOUCHSCREEN_CTRL_INTR	0x8
 #define PDT_START_SCAN_LOCATION (0x00E9)
 #define PDT_END_SCAN_LOCATION	(0x000A)
 #define PDT_ENTRY_SIZE		(0x0006)
-#define RMI4_NUMBER_OF_MAX_FINGERS		(8)
-#define SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM	(0x11)
+#define SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM	(0x11)
 #define SYNAPTICS_RMI4_DEVICE_CONTROL_FUNC_NUM	(0x01)
 
 /**
@@ -164,6 +165,7 @@ struct synaptics_rmi4_device_info {
  * @regulator: pointer to the regulator structure
  * @wait: wait queue structure variable
  * @touch_stopped: flag to stop the thread function
+ * @fingers_supported: maximum supported fingers
  *
  * This structure gives the device data information.
  */
@@ -184,6 +186,7 @@ struct synaptics_rmi4_data {
 	struct regulator	*regulator;
 	wait_queue_head_t	wait;
 	bool			touch_stopped;
+	unsigned char		fingers_supported;
 };
 
 /**
@@ -291,34 +294,33 @@ exit:
 }
 
 /**
- * synpatics_rmi4_touchpad_report() - reports for the rmi4 touchpad device
+ * synpatics_rmi4_touchscreen_report() - reports for the rmi4 touchscreen device
  * @pdata: pointer to synaptics_rmi4_data structure
  * @rfi: pointer to synaptics_rmi4_fn structure
  *
- * This function calls to reports for the rmi4 touchpad device
+ * This function calls to reports for the rmi4 touchscreen device
  */
-static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
+static int synpatics_rmi4_touchscreen_report(struct synaptics_rmi4_data *pdata,
 						struct synaptics_rmi4_fn *rfi)
 {
 	/* number of touch points - fingers down in this case */
 	int	touch_count = 0;
 	int	finger;
-	int	fingers_supported;
 	int	finger_registers;
 	int	reg;
 	int	finger_shift;
 	int	finger_status;
 	int	retval;
+	int	x, y;
+	int	wx, wy;
 	unsigned short	data_base_addr;
 	unsigned short	data_offset;
 	unsigned char	data_reg_blk_size;
 	unsigned char	values[2];
 	unsigned char	data[DATA_LEN];
-	int	x[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	y[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	wx[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	wy[RMI4_NUMBER_OF_MAX_FINGERS];
+	unsigned char	fingers_supported = pdata->fingers_supported;
 	struct	i2c_client *client = pdata->i2c_client;
+	struct	input_dev *input_dev = pdata->input_dev;
 
 	/* get 2D sensor finger data */
 	/*
@@ -333,7 +335,6 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 	 *	10 = finger present but data may not be accurate,
 	 *	11 = reserved for product use.
 	 */
-	fingers_supported	= rfi->num_of_data_points;
 	finger_registers	= (fingers_supported + 3)/4;
 	data_base_addr		= rfi->fn_desc.data_base_addr;
 	retval = synaptics_rmi4_i2c_block_read(pdata, data_base_addr, values,
@@ -353,12 +354,16 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 		reg = finger/4;
 		/* bit shift to get finger's status */
 		finger_shift	= (finger % 4) * 2;
-		finger_status	= (values[reg] >> finger_shift) & 3;
+		finger_status	= (values[reg] >> finger_shift) & MASK_2BIT;
 		/*
 		 * if finger status indicates a finger is present then
 		 * read the finger data and report it
 		 */
-		if (finger_status == 1 || finger_status == 2) {
+		input_mt_slot(input_dev, finger);
+		input_mt_report_slot_state(input_dev, MT_TOOL_FINGER,
+							finger_status != 0);
+
+		if (finger_status) {
 			/* Read the finger data */
 			data_offset = data_base_addr +
 					((finger * data_reg_blk_size) +
@@ -367,50 +372,33 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 						data_offset, data,
 						data_reg_blk_size);
 			if (retval != data_reg_blk_size) {
-				printk(KERN_ERR "%s:read data failed\n",
+				dev_err(&client->dev, "%s:read data failed\n",
 								__func__);
 				return 0;
-			} else {
-				x[touch_count]	=
-					(data[0] << 4) | (data[2] & MASK_4BIT);
-				y[touch_count]	=
-					(data[1] << 4) |
-					((data[2] >> 4) & MASK_4BIT);
-				wy[touch_count]	=
-						(data[3] >> 4) & MASK_4BIT;
-				wx[touch_count]	=
-						(data[3] & MASK_4BIT);
-
-				if (pdata->board->x_flip)
-					x[touch_count] =
-						pdata->sensor_max_x -
-								x[touch_count];
-				if (pdata->board->y_flip)
-					y[touch_count] =
-						pdata->sensor_max_y -
-								y[touch_count];
 			}
+			x = (data[0] << 4) | (data[2] & MASK_4BIT);
+			y = (data[1] << 4) | ((data[2] >> 4) & MASK_4BIT);
+			wy = (data[3] >> 4) & MASK_4BIT;
+			wx = (data[3] & MASK_4BIT);
+
+			if (pdata->board->x_flip)
+				x = pdata->sensor_max_x - x;
+			if (pdata->board->y_flip)
+				y = pdata->sensor_max_y - y;
+
+			input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
+								max(wx, wy));
+			input_report_abs(input_dev, ABS_MT_POSITION_X, x);
+			input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
+
 			/* number of active touch points */
 			touch_count++;
 		}
 	}
 
-	/* report to input subsystem */
-	if (touch_count) {
-		for (finger = 0; finger < touch_count; finger++) {
-			input_report_abs(pdata->input_dev, ABS_MT_TOUCH_MAJOR,
-						max(wx[finger] , wy[finger]));
-			input_report_abs(pdata->input_dev, ABS_MT_POSITION_X,
-								x[finger]);
-			input_report_abs(pdata->input_dev, ABS_MT_POSITION_Y,
-								y[finger]);
-			input_mt_sync(pdata->input_dev);
-		}
-	} else
-		input_mt_sync(pdata->input_dev);
-
 	/* sync after groups of events */
-	input_sync(pdata->input_dev);
+	input_mt_sync_frame(input_dev);
+	input_sync(input_dev);
 	/* return the number of touch points */
 	return touch_count;
 }
@@ -428,13 +416,13 @@ static int synaptics_rmi4_report_device(struct synaptics_rmi4_data *pdata,
 	int touch = 0;
 	struct	i2c_client *client = pdata->i2c_client;
 	static int num_error_reports;
-	if (rfi->fn_number != SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM) {
+	if (rfi->fn_number != SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM) {
 		num_error_reports++;
 		if (num_error_reports < MAX_ERROR_REPORT)
 			dev_err(&client->dev, "%s:report not supported\n",
 								__func__);
 	} else
-		touch = synpatics_rmi4_touchpad_report(pdata, rfi);
+		touch = synpatics_rmi4_touchscreen_report(pdata, rfi);
 	return touch;
 }
 /**
@@ -510,15 +498,15 @@ static irqreturn_t synaptics_rmi4_irq(int irq, void *data)
 }
 
 /**
- * synpatics_rmi4_touchpad_detect() - detects the rmi4 touchpad device
+ * synpatics_rmi4_touchscreen_detect() - detects the rmi4 touchscreen device
  * @pdata: pointer to synaptics_rmi4_data structure
  * @rfi: pointer to synaptics_rmi4_fn structure
  * @fd: pointer to synaptics_rmi4_fn_desc structure
  * @interruptcount: count the number of interrupts
  *
- * This function calls to detects the rmi4 touchpad device
+ * This function calls to detects the rmi4 touchscreen device
  */
-static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
+static int synpatics_rmi4_touchscreen_detect(struct synaptics_rmi4_data *pdata,
 					struct synaptics_rmi4_fn *rfi,
 					struct synaptics_rmi4_fn_desc *fd,
 					unsigned int interruptcount)
@@ -575,6 +563,7 @@ static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
 		if ((queries[1] & MASK_3BIT) == 5)
 			rfi->num_of_data_points = 10;
 	}
+	pdata->fingers_supported = rfi->num_of_data_points;
 	/* Need to get interrupt info for handling interrupts */
 	rfi->index_to_intr_reg = (interruptcount + 7)/8;
 	if (rfi->index_to_intr_reg != 0)
@@ -655,13 +644,13 @@ static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
 }
 
 /**
- * synaptics_rmi4_touchpad_config() - configures the rmi4 touchpad device
+ * synaptics_rmi4_touchscreen_config() - configures the rmi4 touchscreen device
  * @pdata: pointer to synaptics_rmi4_data structure
  * @rfi: pointer to synaptics_rmi4_fn structure
  *
- * This function calls to configures the rmi4 touchpad device
+ * This function calls to configures the rmi4 touchscreen device
  */
-int synaptics_rmi4_touchpad_config(struct synaptics_rmi4_data *pdata,
+int synaptics_rmi4_touchscreen_config(struct synaptics_rmi4_data *pdata,
 						struct synaptics_rmi4_fn *rfi)
 {
 	/*
@@ -741,51 +730,7 @@ static int synaptics_rmi4_i2c_query_device(struct synaptics_rmi4_data *pdata)
 			return -EIO;
 		}
 		rfi = NULL;
-		if (rmi_fd.fn_number) {
-			switch (rmi_fd.fn_number & MASK_8BIT) {
-			case SYNAPTICS_RMI4_DEVICE_CONTROL_FUNC_NUM:
-				pdata->fn01_query_base_addr =
-						rmi_fd.query_base_addr;
-				pdata->fn01_ctrl_base_addr =
-						rmi_fd.ctrl_base_addr;
-				pdata->fn01_data_base_addr =
-						rmi_fd.data_base_addr;
-				break;
-			case SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM:
-				if (rmi_fd.intr_src_count) {
-					rfi = kmalloc(sizeof(*rfi),
-								GFP_KERNEL);
-					if (!rfi) {
-						dev_err(&client->dev,
-							"%s:kmalloc failed\n",
-								__func__);
-							return -ENOMEM;
-					}
-					retval = synpatics_rmi4_touchpad_detect
-								(pdata,	rfi,
-								&rmi_fd,
-								intr_count);
-					if (retval < 0) {
-						kfree(rfi);
-						return retval;
-					}
-				}
-				break;
-			}
-			/* interrupt count for next iteration */
-			intr_count += (rmi_fd.intr_src_count & MASK_3BIT);
-			/*
-			 * We only want to add functions to the list
-			 * that have data associated with them.
-			 */
-			if (rfi && rmi_fd.intr_src_count) {
-				/* link this function info to the RMI module */
-				mutex_lock(&(pdata->fn_list_mutex));
-				list_add_tail(&rfi->link,
-					&pdata->rmi4_mod_info.support_fn_list);
-				mutex_unlock(&(pdata->fn_list_mutex));
-			}
-		} else {
+		if (!rmi_fd.fn_number) {
 			/*
 			 * A zero in the function number
 			 * signals the end of the PDT
@@ -794,6 +739,47 @@ static int synaptics_rmi4_i2c_query_device(struct synaptics_rmi4_data *pdata)
 				"%s:end of PDT\n", __func__);
 			break;
 		}
+		switch (rmi_fd.fn_number & MASK_8BIT) {
+		case SYNAPTICS_RMI4_DEVICE_CONTROL_FUNC_NUM:
+			pdata->fn01_query_base_addr =
+					rmi_fd.query_base_addr;
+			pdata->fn01_ctrl_base_addr =
+					rmi_fd.ctrl_base_addr;
+			pdata->fn01_data_base_addr =
+					rmi_fd.data_base_addr;
+			break;
+		case SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM:
+			if (rmi_fd.intr_src_count) {
+				rfi = kmalloc(sizeof(*rfi), GFP_KERNEL);
+				if (!rfi) {
+					dev_err(&client->dev,
+						"%s:kmalloc failed\n",
+							__func__);
+					return -ENOMEM;
+				}
+				retval = synpatics_rmi4_touchscreen_detect(
+							pdata, rfi, &rmi_fd,
+							intr_count);
+				if (retval < 0) {
+					kfree(rfi);
+					return retval;
+				}
+			}
+			break;
+		}
+		/* interrupt count for next iteration */
+		intr_count += (rmi_fd.intr_src_count & MASK_3BIT);
+		/*
+		 * We only want to add functions to the list
+		 * that have data associated with them.
+		 */
+		if (rfi && rmi_fd.intr_src_count) {
+			/* link this function info to the RMI module */
+			mutex_lock(&(pdata->fn_list_mutex));
+			list_add_tail(&rfi->link,
+				&pdata->rmi4_mod_info.support_fn_list);
+			mutex_unlock(&(pdata->fn_list_mutex));
+		}
 	}
 	/*
 	 * calculate the interrupt register count - used in the
@@ -849,33 +835,28 @@ static int synaptics_rmi4_i2c_query_device(struct synaptics_rmi4_data *pdata)
 
 	list_for_each_entry(rfi, &pdata->rmi4_mod_info.support_fn_list, link)
 		data_sources += rfi->num_of_data_sources;
-	if (data_sources) {
-		rmi = &(pdata->rmi4_mod_info);
-		list_for_each_entry(rfi, &rmi->support_fn_list, link) {
-			if (rfi->num_of_data_sources) {
-				if (rfi->fn_number ==
-					SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM) {
-					retval = synaptics_rmi4_touchpad_config
-								(pdata, rfi);
-					if (retval < 0)
-						return retval;
-				} else
-					dev_err(&client->dev,
-						"%s:fn_number not supported\n",
+	if (!data_sources)
+		return 0;
+	rmi = &(pdata->rmi4_mod_info);
+	list_for_each_entry(rfi, &rmi->support_fn_list, link) {
+		if (!rfi->num_of_data_sources)
+			continue;
+		if (rfi->fn_number == SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM) {
+			retval = synaptics_rmi4_touchscreen_config(pdata, rfi);
+			if (retval < 0)
+				return retval;
+		} else
+			dev_err(&client->dev, "%s:fn_number not supported\n",
 								__func__);
-				/*
-				 * Turn on interrupts for this
-				 * function's data sources.
-				 */
-				ctrl_offset = pdata->fn01_ctrl_base_addr + 1 +
-							rfi->index_to_intr_reg;
-				retval = synaptics_rmi4_i2c_byte_write(pdata,
-							ctrl_offset,
+		/*
+		 * Turn on interrupts for this function's data sources.
+		 */
+		ctrl_offset = pdata->fn01_ctrl_base_addr + 1 +
+						rfi->index_to_intr_reg;
+		retval = synaptics_rmi4_i2c_byte_write(pdata, ctrl_offset,
 							rfi->intr_mask);
-				if (retval < 0)
-					return retval;
-			}
-		}
+		if (retval < 0)
+			return retval;
 	}
 	return 0;
 }
@@ -988,6 +969,8 @@ static int __devinit synaptics_rmi4_probe
 					rmi4_data->sensor_max_y, 0, 0);
 	input_set_abs_params(rmi4_data->input_dev, ABS_MT_TOUCH_MAJOR, 0,
 						MAX_TOUCH_MAJOR, 0, 0);
+	input_mt_init_slots(rmi4_data->input_dev,
+				rmi4_data->fingers_supported, INPUT_MT_DIRECT);
 
 	/* Clear interrupts */
 	synaptics_rmi4_i2c_block_read(rmi4_data,
@@ -1076,7 +1059,7 @@ static int synaptics_rmi4_suspend(struct device *dev)
 
 	retval = synaptics_rmi4_i2c_byte_write(rmi4_data,
 					rmi4_data->fn01_ctrl_base_addr + 1,
-					(intr_status & ~TOUCHPAD_CTRL_INTR));
+					(intr_status & ~TOUCHSCREEN_CTRL_INTR));
 	if (retval < 0)
 		return retval;
 
@@ -1112,7 +1095,7 @@ static int synaptics_rmi4_resume(struct device *dev)
 
 	retval = synaptics_rmi4_i2c_byte_write(rmi4_data,
 					rmi4_data->fn01_ctrl_base_addr + 1,
-					(intr_status | TOUCHPAD_CTRL_INTR));
+					(intr_status | TOUCHSCREEN_CTRL_INTR));
 	if (retval < 0)
 		return retval;
 
-- 
1.7.5.4


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

* Re: [PATCH v5] staging: ste_rmi4: Convert to Type-B support
  2012-11-08  7:05 ` [PATCH v5] " Alexandra Chin
@ 2012-11-09 19:26   ` Henrik Rydberg
  2012-11-12 16:40     ` Alexandra Chin
  0 siblings, 1 reply; 32+ messages in thread
From: Henrik Rydberg @ 2012-11-09 19:26 UTC (permalink / raw)
  To: Alexandra Chin
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

On Thu, Nov 08, 2012 at 03:05:29PM +0800, Alexandra Chin wrote:
> Convert to MT-B because Synaptics touch devices are capable
> of tracking identifiable fingers.
> 
> Signed-off-by: Alexandra Chin <alexandra.chin@tw.synaptics.com>
> ---
> Changes from v5:
>         - Incorporated Henrik's review comments
>           *rollback to v3 from v4
>           *fix odd line break in v3
>         - Include Alexandra in the list of authors

I am seeing _new_ irrelevant changes in this version... There is no
need to revert to v3. Just remove the synaptics_rmi4_query_function()
changes and other uneeded cruft from the v4 patch. No new changes,
please, and make sure the final patch is tested.

Thanks.
Henrik

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

* RE: [PATCH v3] staging: ste_rmi4: Convert to Type-B support
  2012-11-04 21:53 ` Henrik Rydberg
  2012-11-05  6:01   ` Alexandra Chin
@ 2012-11-12 11:59   ` Alexandra Chin
  1 sibling, 0 replies; 32+ messages in thread
From: Alexandra Chin @ 2012-11-12 11:59 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Henrik,

Really thanks a lot for all your advice.

> > @@ -751,7 +739,7 @@ static int synaptics_rmi4_i2c_query_device(struct
> synaptics_rmi4_data *pdata)
> >  				pdata->fn01_data_base_addr =
> >  						rmi_fd.data_base_addr;
> >  				break;
> > -			case SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM:
> > +			case SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM:
> >  				if (rmi_fd.intr_src_count) {
> >  					rfi = kmalloc(sizeof(*rfi),
> >  								GFP_KERNEL);
> > @@ -761,7 +749,8 @@ static int synaptics_rmi4_i2c_query_device(struct
> synaptics_rmi4_data *pdata)
> >  								__func__);
> >  							return -ENOMEM;
> >  					}
> > -					retval = synpatics_rmi4_touchpad_detect
> > +					retval =
> > +					synpatics_rmi4_touchscreen_detect
> >  								(pdata,	rfi,
> >  								&rmi_fd,
> >  								intr_count);
> 
> Odd line break is a clear sign that something could be broken out into its own
> function.
> 
> > @@ -854,8 +843,9 @@ static int synaptics_rmi4_i2c_query_device(struct
> synaptics_rmi4_data *pdata)
> >  		list_for_each_entry(rfi, &rmi->support_fn_list, link) {
> >  			if (rfi->num_of_data_sources) {
> >  				if (rfi->fn_number ==
> > -					SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM) {
> > -					retval = synaptics_rmi4_touchpad_config
> > +					SYNAPTICS_RMI4_TOUCHSCREEN_FUNC_NUM) {
> > +					retval =
> > +					synaptics_rmi4_touchscreen_config
> >  								(pdata, rfi);
> >  					if (retval < 0)
> >  						return retval;
> 
> Same here.
> 

As you mentioned, there are odd lines in patch v3.
These odd lines are because that lines are over 80 characters after "touchpad" is
replaced by "touchscreen".

In patch v4, I did the splitting of the functions to fix line over 80 characters issue
(and other irrelevant work).

Somehow I realized that code itself can be optimized, and there is no need
to break out function to fix line over 80 characters issues. Therefore I rollback
patch to v3, and reorganized code flow to fix line over 80 characters issues in
patch v5. Patch v5 only includes odd lines issue fixed, although it looks a little
too large.

And these changes have been verified with Pandaboard.

Greatly appreciate your suggestions, and please let me know if you have any
concerns about v5 (https://lkml.org/lkml/2012/11/8/31).

Yours very truly,
Alexandra

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

* RE: [PATCH v5] staging: ste_rmi4: Convert to Type-B support
  2012-11-09 19:26   ` Henrik Rydberg
@ 2012-11-12 16:40     ` Alexandra Chin
  2012-11-12 17:20       ` Henrik Rydberg
  0 siblings, 1 reply; 32+ messages in thread
From: Alexandra Chin @ 2012-11-12 16:40 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Henrik,

>> Convert to MT-B because Synaptics touch devices are capable
>> of tracking identifiable fingers.
>>
>> Signed-off-by: Alexandra Chin <alexandra.chin@tw.synaptics.com>
>> ---
>> Changes from v5:
>>         - Incorporated Henrik's review comments
>>           *rollback to v3 from v4
>>           *fix odd line break in v3
>>         - Include Alexandra in the list of authors
>
> I am seeing _new_ irrelevant changes in this version... There is no
> need to revert to v3. Just remove the synaptics_rmi4_query_function()
> changes and other uneeded cruft from the v4 patch. No new changes,
> please, and make sure the final patch is tested.

Those changes are intended to fix line over 80 characters in v3.
The code has been optimized and the logic does not change.
Patch v5 has been verified with pandaboard.
Any suggestions are much appreciated.

Best regards,
Alexandra

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

* Re: [PATCH v5] staging: ste_rmi4: Convert to Type-B support
  2012-11-12 16:40     ` Alexandra Chin
@ 2012-11-12 17:20       ` Henrik Rydberg
  2012-11-13  2:04         ` Alexandra Chin
  0 siblings, 1 reply; 32+ messages in thread
From: Henrik Rydberg @ 2012-11-12 17:20 UTC (permalink / raw)
  To: Alexandra Chin
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

On Mon, Nov 12, 2012 at 04:40:34PM +0000, Alexandra Chin wrote:
> Hi Henrik,
> 
> >> Convert to MT-B because Synaptics touch devices are capable
> >> of tracking identifiable fingers.
> >>
> >> Signed-off-by: Alexandra Chin <alexandra.chin@tw.synaptics.com>
> >> ---
> >> Changes from v5:
> >>         - Incorporated Henrik's review comments
> >>           *rollback to v3 from v4
> >>           *fix odd line break in v3
> >>         - Include Alexandra in the list of authors
> >
> > I am seeing _new_ irrelevant changes in this version... There is no
> > need to revert to v3. Just remove the synaptics_rmi4_query_function()
> > changes and other uneeded cruft from the v4 patch. No new changes,
> > please, and make sure the final patch is tested.
> 
> Those changes are intended to fix line over 80 characters in v3.
> The code has been optimized and the logic does not change.

I would still prefer if the trivial non-functional changes were
deferred to later patches. I realize that the change from 'TOUCHPAD'
to 'TOUCHSCREEN' created some problems, but those changes were not
called for in the first place. How about leaving those lines be for
now, and then change to something more generic in a later patch? The
code will obviously work equally well for both touchpad and
touchscreen settings, so the defines could be better named anyways.

> Patch v5 has been verified with pandaboard.

Thank you.

Henrik

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

* RE: [PATCH v5] staging: ste_rmi4: Convert to Type-B support
  2012-11-12 17:20       ` Henrik Rydberg
@ 2012-11-13  2:04         ` Alexandra Chin
  0 siblings, 0 replies; 32+ messages in thread
From: Alexandra Chin @ 2012-11-13  2:04 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Henrik,

> > > I am seeing _new_ irrelevant changes in this version... There is no
> > > need to revert to v3. Just remove the synaptics_rmi4_query_function()
> > > changes and other uneeded cruft from the v4 patch. No new changes,
> > > please, and make sure the final patch is tested.
> >
> > Those changes are intended to fix line over 80 characters in v3.
> > The code has been optimized and the logic does not change.
> 
> I would still prefer if the trivial non-functional changes were
> deferred to later patches. I realize that the change from 'TOUCHPAD'
> to 'TOUCHSCREEN' created some problems, but those changes were not
> called for in the first place. How about leaving those lines be for
> now, and then change to something more generic in a later patch? The
> code will obviously work equally well for both touchpad and
> touchscreen settings, so the defines could be better named anyways.
> 
> > Patch v5 has been verified with pandaboard.
> 
> Thank you.
> 
> Henrik

I see. As your suggestions in patch v5, I will resend patch containing only
functional changes.
Thanks for your advice.

Alexandra

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

* [PATCH v6] staging: ste_rmi4: Convert to Type-B support
  2012-11-02 17:36 [PATCH v3] staging: ste_rmi4: Convert to Type-B support Alexandra Chin
                   ` (2 preceding siblings ...)
  2012-11-08  7:05 ` [PATCH v5] " Alexandra Chin
@ 2012-11-13 12:12 ` Alexandra Chin
  2012-11-13 17:53   ` Henrik Rydberg
  2012-11-16  8:31 ` [PATCH v6] staging: ste_rmi4: Convert to Type-B support Alexandra Chin
  4 siblings, 1 reply; 32+ messages in thread
From: Alexandra Chin @ 2012-11-13 12:12 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Convert to MT-B because Synaptics touch devices are capable
of tracking identifiable fingers.
 
Signed-off-by: Alexandra Chin <alexandra.chin@tw.synaptics.com>
---
This patch was tested on Pandaboard.

Changes from v6:
        - Incorporated Henrik's review comments
          *remove irrelevant changes within the patch

Changes from v5:
        - Incorporated Henrik's review comments
          *rollback to v3 from v4
          *fix odd line break in v3

Changes from v4:
        - Incorporated Henrik's review comments
          *split function synpatics_rmi4_touchscreen_report
          *split function synaptics_rmi4_i2c_query_device

Changes from v3:
	- Incorporated Henrik's review comments
	  *remove 'else' after an error path return
	  *add input_mt_sync_frame() for pointer emulation effects
	  *correct names of touchscreen
	- Replace printk with dev_err

Changes from v2:
	- Incorporated Henrik's review comments
	  *directly report finger state with Type-B
	- Against 3.7-rcX
	  *call input_mt_init_slots with INPUT_MT_DIRECT flag
---
 drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c |   76 +++++++++++--------------
 1 files changed, 33 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
index 277491a..ed304e0 100644
--- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
+++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
@@ -31,6 +31,7 @@
 #include <linux/interrupt.h>
 #include <linux/regulator/consumer.h>
 #include <linux/module.h>
+#include <linux/input/mt.h>
 #include "synaptics_i2c_rmi4.h"
 
 /* TODO: for multiple device support will need a per-device mutex */
@@ -67,7 +68,6 @@
 #define PDT_START_SCAN_LOCATION (0x00E9)
 #define PDT_END_SCAN_LOCATION	(0x000A)
 #define PDT_ENTRY_SIZE		(0x0006)
-#define RMI4_NUMBER_OF_MAX_FINGERS		(8)
 #define SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM	(0x11)
 #define SYNAPTICS_RMI4_DEVICE_CONTROL_FUNC_NUM	(0x01)
 
@@ -164,6 +164,7 @@ struct synaptics_rmi4_device_info {
  * @regulator: pointer to the regulator structure
  * @wait: wait queue structure variable
  * @touch_stopped: flag to stop the thread function
+ * @fingers_supported: maximum supported fingers
  *
  * This structure gives the device data information.
  */
@@ -184,6 +185,7 @@ struct synaptics_rmi4_data {
 	struct regulator	*regulator;
 	wait_queue_head_t	wait;
 	bool			touch_stopped;
+	unsigned char		fingers_supported;
 };
 
 /**
@@ -303,22 +305,21 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 	/* number of touch points - fingers down in this case */
 	int	touch_count = 0;
 	int	finger;
-	int	fingers_supported;
 	int	finger_registers;
 	int	reg;
 	int	finger_shift;
 	int	finger_status;
 	int	retval;
+	int	x, y;
+	int	wx, wy;
 	unsigned short	data_base_addr;
 	unsigned short	data_offset;
 	unsigned char	data_reg_blk_size;
 	unsigned char	values[2];
 	unsigned char	data[DATA_LEN];
-	int	x[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	y[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	wx[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	wy[RMI4_NUMBER_OF_MAX_FINGERS];
+	unsigned char	fingers_supported = pdata->fingers_supported;
 	struct	i2c_client *client = pdata->i2c_client;
+	struct	input_dev *input_dev = pdata->input_dev;
 
 	/* get 2D sensor finger data */
 	/*
@@ -333,7 +334,6 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 	 *	10 = finger present but data may not be accurate,
 	 *	11 = reserved for product use.
 	 */
-	fingers_supported	= rfi->num_of_data_points;
 	finger_registers	= (fingers_supported + 3)/4;
 	data_base_addr		= rfi->fn_desc.data_base_addr;
 	retval = synaptics_rmi4_i2c_block_read(pdata, data_base_addr, values,
@@ -358,7 +358,11 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 		 * if finger status indicates a finger is present then
 		 * read the finger data and report it
 		 */
-		if (finger_status == 1 || finger_status == 2) {
+		input_mt_slot(input_dev, finger);
+		input_mt_report_slot_state(input_dev, MT_TOOL_FINGER,
+							finger_status != 0);
+
+		if (finger_status) {
 			/* Read the finger data */
 			data_offset = data_base_addr +
 					((finger * data_reg_blk_size) +
@@ -367,50 +371,33 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 						data_offset, data,
 						data_reg_blk_size);
 			if (retval != data_reg_blk_size) {
-				printk(KERN_ERR "%s:read data failed\n",
+				dev_err(&client->dev, "%s:read data failed\n",
 								__func__);
 				return 0;
-			} else {
-				x[touch_count]	=
-					(data[0] << 4) | (data[2] & MASK_4BIT);
-				y[touch_count]	=
-					(data[1] << 4) |
-					((data[2] >> 4) & MASK_4BIT);
-				wy[touch_count]	=
-						(data[3] >> 4) & MASK_4BIT;
-				wx[touch_count]	=
-						(data[3] & MASK_4BIT);
-
-				if (pdata->board->x_flip)
-					x[touch_count] =
-						pdata->sensor_max_x -
-								x[touch_count];
-				if (pdata->board->y_flip)
-					y[touch_count] =
-						pdata->sensor_max_y -
-								y[touch_count];
 			}
+			x = (data[0] << 4) | (data[2] & MASK_4BIT);
+			y = (data[1] << 4) | ((data[2] >> 4) & MASK_4BIT);
+			wy = (data[3] >> 4) & MASK_4BIT;
+			wx = (data[3] & MASK_4BIT);
+
+			if (pdata->board->x_flip)
+				x = pdata->sensor_max_x - x;
+			if (pdata->board->y_flip)
+				y = pdata->sensor_max_y - y;
+
+			input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
+								max(wx, wy));
+			input_report_abs(input_dev, ABS_MT_POSITION_X, x);
+			input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
+
 			/* number of active touch points */
 			touch_count++;
 		}
 	}
 
-	/* report to input subsystem */
-	if (touch_count) {
-		for (finger = 0; finger < touch_count; finger++) {
-			input_report_abs(pdata->input_dev, ABS_MT_TOUCH_MAJOR,
-						max(wx[finger] , wy[finger]));
-			input_report_abs(pdata->input_dev, ABS_MT_POSITION_X,
-								x[finger]);
-			input_report_abs(pdata->input_dev, ABS_MT_POSITION_Y,
-								y[finger]);
-			input_mt_sync(pdata->input_dev);
-		}
-	} else
-		input_mt_sync(pdata->input_dev);
-
 	/* sync after groups of events */
-	input_sync(pdata->input_dev);
+	input_mt_sync_frame(input_dev);
+	input_sync(input_dev);
 	/* return the number of touch points */
 	return touch_count;
 }
@@ -575,6 +562,7 @@ static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
 		if ((queries[1] & MASK_3BIT) == 5)
 			rfi->num_of_data_points = 10;
 	}
+	pdata->fingers_supported = rfi->num_of_data_points;
 	/* Need to get interrupt info for handling interrupts */
 	rfi->index_to_intr_reg = (interruptcount + 7)/8;
 	if (rfi->index_to_intr_reg != 0)
@@ -988,6 +976,8 @@ static int __devinit synaptics_rmi4_probe
 					rmi4_data->sensor_max_y, 0, 0);
 	input_set_abs_params(rmi4_data->input_dev, ABS_MT_TOUCH_MAJOR, 0,
 						MAX_TOUCH_MAJOR, 0, 0);
+	input_mt_init_slots(rmi4_data->input_dev,
+				rmi4_data->fingers_supported, 0);
 
 	/* Clear interrupts */
 	synaptics_rmi4_i2c_block_read(rmi4_data,
-- 
1.7.5.4


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

* Re: [PATCH v6] staging: ste_rmi4: Convert to Type-B support
  2012-11-13 12:12 ` [PATCH v6] " Alexandra Chin
@ 2012-11-13 17:53   ` Henrik Rydberg
  2012-11-14 17:33     ` Alexandra Chin
  2012-11-14 17:40     ` [RFC] staging: ste_rmi4: merge into the main kernel tree Alexandra Chin
  0 siblings, 2 replies; 32+ messages in thread
From: Henrik Rydberg @ 2012-11-13 17:53 UTC (permalink / raw)
  To: Alexandra Chin
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Alexandra,

> Convert to MT-B because Synaptics touch devices are capable
> of tracking identifiable fingers.
>  
> Signed-off-by: Alexandra Chin <alexandra.chin@tw.synaptics.com>
> ---
> This patch was tested on Pandaboard.

Thank you for making changes. One minor comment inline, but I am ok
with the patch as is.  Looks like we have finally arrived. :-)

    Reviewed-by: Henrik Rydberg <rydberg@euromail.se>

Thanks,
Henrik

> ---
>  drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c |   76 +++++++++++--------------
>  1 files changed, 33 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> index 277491a..ed304e0 100644
> --- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> +++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> @@ -31,6 +31,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/module.h>
> +#include <linux/input/mt.h>
>  #include "synaptics_i2c_rmi4.h"
>  
>  /* TODO: for multiple device support will need a per-device mutex */
> @@ -67,7 +68,6 @@
>  #define PDT_START_SCAN_LOCATION (0x00E9)
>  #define PDT_END_SCAN_LOCATION	(0x000A)
>  #define PDT_ENTRY_SIZE		(0x0006)
> -#define RMI4_NUMBER_OF_MAX_FINGERS		(8)
>  #define SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM	(0x11)
>  #define SYNAPTICS_RMI4_DEVICE_CONTROL_FUNC_NUM	(0x01)
>  
> @@ -164,6 +164,7 @@ struct synaptics_rmi4_device_info {
>   * @regulator: pointer to the regulator structure
>   * @wait: wait queue structure variable
>   * @touch_stopped: flag to stop the thread function
> + * @fingers_supported: maximum supported fingers
>   *
>   * This structure gives the device data information.
>   */
> @@ -184,6 +185,7 @@ struct synaptics_rmi4_data {
>  	struct regulator	*regulator;
>  	wait_queue_head_t	wait;
>  	bool			touch_stopped;
> +	unsigned char		fingers_supported;
>  };
>  
>  /**
> @@ -303,22 +305,21 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
>  	/* number of touch points - fingers down in this case */
>  	int	touch_count = 0;
>  	int	finger;
> -	int	fingers_supported;
>  	int	finger_registers;
>  	int	reg;
>  	int	finger_shift;
>  	int	finger_status;
>  	int	retval;
> +	int	x, y;
> +	int	wx, wy;
>  	unsigned short	data_base_addr;
>  	unsigned short	data_offset;
>  	unsigned char	data_reg_blk_size;
>  	unsigned char	values[2];
>  	unsigned char	data[DATA_LEN];
> -	int	x[RMI4_NUMBER_OF_MAX_FINGERS];
> -	int	y[RMI4_NUMBER_OF_MAX_FINGERS];
> -	int	wx[RMI4_NUMBER_OF_MAX_FINGERS];
> -	int	wy[RMI4_NUMBER_OF_MAX_FINGERS];
> +	unsigned char	fingers_supported = pdata->fingers_supported;
>  	struct	i2c_client *client = pdata->i2c_client;
> +	struct	input_dev *input_dev = pdata->input_dev;
>  
>  	/* get 2D sensor finger data */
>  	/*
> @@ -333,7 +334,6 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
>  	 *	10 = finger present but data may not be accurate,
>  	 *	11 = reserved for product use.
>  	 */
> -	fingers_supported	= rfi->num_of_data_points;
>  	finger_registers	= (fingers_supported + 3)/4;
>  	data_base_addr		= rfi->fn_desc.data_base_addr;
>  	retval = synaptics_rmi4_i2c_block_read(pdata, data_base_addr, values,
> @@ -358,7 +358,11 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
>  		 * if finger status indicates a finger is present then
>  		 * read the finger data and report it
>  		 */
> -		if (finger_status == 1 || finger_status == 2) {
> +		input_mt_slot(input_dev, finger);
> +		input_mt_report_slot_state(input_dev, MT_TOOL_FINGER,
> +							finger_status != 0);
> +
> +		if (finger_status) {
>  			/* Read the finger data */
>  			data_offset = data_base_addr +
>  					((finger * data_reg_blk_size) +
> @@ -367,50 +371,33 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
>  						data_offset, data,
>  						data_reg_blk_size);
>  			if (retval != data_reg_blk_size) {
> -				printk(KERN_ERR "%s:read data failed\n",
> +				dev_err(&client->dev, "%s:read data failed\n",

Still not needed for this patch. Please try to refrain from making
unnecessary changes in the future.

>  								__func__);
>  				return 0;
> -			} else {
> -				x[touch_count]	=
> -					(data[0] << 4) | (data[2] & MASK_4BIT);
> -				y[touch_count]	=
> -					(data[1] << 4) |
> -					((data[2] >> 4) & MASK_4BIT);
> -				wy[touch_count]	=
> -						(data[3] >> 4) & MASK_4BIT;
> -				wx[touch_count]	=
> -						(data[3] & MASK_4BIT);
> -
> -				if (pdata->board->x_flip)
> -					x[touch_count] =
> -						pdata->sensor_max_x -
> -								x[touch_count];
> -				if (pdata->board->y_flip)
> -					y[touch_count] =
> -						pdata->sensor_max_y -
> -								y[touch_count];
>  			}
> +			x = (data[0] << 4) | (data[2] & MASK_4BIT);
> +			y = (data[1] << 4) | ((data[2] >> 4) & MASK_4BIT);
> +			wy = (data[3] >> 4) & MASK_4BIT;
> +			wx = (data[3] & MASK_4BIT);
> +
> +			if (pdata->board->x_flip)
> +				x = pdata->sensor_max_x - x;
> +			if (pdata->board->y_flip)
> +				y = pdata->sensor_max_y - y;
> +
> +			input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
> +								max(wx, wy));
> +			input_report_abs(input_dev, ABS_MT_POSITION_X, x);
> +			input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
> +
>  			/* number of active touch points */
>  			touch_count++;
>  		}
>  	}
>  
> -	/* report to input subsystem */
> -	if (touch_count) {
> -		for (finger = 0; finger < touch_count; finger++) {
> -			input_report_abs(pdata->input_dev, ABS_MT_TOUCH_MAJOR,
> -						max(wx[finger] , wy[finger]));
> -			input_report_abs(pdata->input_dev, ABS_MT_POSITION_X,
> -								x[finger]);
> -			input_report_abs(pdata->input_dev, ABS_MT_POSITION_Y,
> -								y[finger]);
> -			input_mt_sync(pdata->input_dev);
> -		}
> -	} else
> -		input_mt_sync(pdata->input_dev);
> -
>  	/* sync after groups of events */
> -	input_sync(pdata->input_dev);
> +	input_mt_sync_frame(input_dev);
> +	input_sync(input_dev);
>  	/* return the number of touch points */
>  	return touch_count;
>  }
> @@ -575,6 +562,7 @@ static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
>  		if ((queries[1] & MASK_3BIT) == 5)
>  			rfi->num_of_data_points = 10;
>  	}
> +	pdata->fingers_supported = rfi->num_of_data_points;
>  	/* Need to get interrupt info for handling interrupts */
>  	rfi->index_to_intr_reg = (interruptcount + 7)/8;
>  	if (rfi->index_to_intr_reg != 0)
> @@ -988,6 +976,8 @@ static int __devinit synaptics_rmi4_probe
>  					rmi4_data->sensor_max_y, 0, 0);
>  	input_set_abs_params(rmi4_data->input_dev, ABS_MT_TOUCH_MAJOR, 0,
>  						MAX_TOUCH_MAJOR, 0, 0);
> +	input_mt_init_slots(rmi4_data->input_dev,
> +				rmi4_data->fingers_supported, 0);
>  
>  	/* Clear interrupts */
>  	synaptics_rmi4_i2c_block_read(rmi4_data,
> -- 
> 1.7.5.4
> 

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

* RE: [PATCH v6] staging: ste_rmi4: Convert to Type-B support
  2012-11-13 17:53   ` Henrik Rydberg
@ 2012-11-14 17:33     ` Alexandra Chin
  2012-11-14 17:40     ` [RFC] staging: ste_rmi4: merge into the main kernel tree Alexandra Chin
  1 sibling, 0 replies; 32+ messages in thread
From: Alexandra Chin @ 2012-11-14 17:33 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Henrik,

> > @@ -367,50 +371,33 @@ static int synpatics_rmi4_touchpad_report(struct
> synaptics_rmi4_data *pdata,
> >  						data_offset, data,
> >  						data_reg_blk_size);
> >  			if (retval != data_reg_blk_size) {
> > -				printk(KERN_ERR "%s:read data failed\n",
> > +				dev_err(&client->dev, "%s:read data failed\n",
> 
> Still not needed for this patch. Please try to refrain from making
> unnecessary changes in the future.
> 

Thanks for your acceptance!
And I will remember that do not put irrelevant changes in the same patch.
I will follow the rule to keep updating changes of v4 in future patch.
Thanks you.

Alexandra Chin

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

* [RFC] staging: ste_rmi4: merge into the main kernel tree
  2012-11-13 17:53   ` Henrik Rydberg
  2012-11-14 17:33     ` Alexandra Chin
@ 2012-11-14 17:40     ` Alexandra Chin
  2012-11-14 18:04       ` Dmitry Torokhov
  1 sibling, 1 reply; 32+ messages in thread
From: Alexandra Chin @ 2012-11-14 17:40 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Henrik and all,

As I know, currently there is no synaptics RMI4 touchscreen driver in the
main tree. In order to support our customers effectively, is it able to merge
staging ste_rmi4 driver into the main kernel.org tree? If it is accepted, should
I send a patch for this change?

Any advice is greatly appreciated!

Alexandra Chin

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

* Re: [RFC] staging: ste_rmi4: merge into the main kernel tree
  2012-11-14 17:40     ` [RFC] staging: ste_rmi4: merge into the main kernel tree Alexandra Chin
@ 2012-11-14 18:04       ` Dmitry Torokhov
  2012-11-15  2:56         ` Alexandra Chin
  0 siblings, 1 reply; 32+ messages in thread
From: Dmitry Torokhov @ 2012-11-14 18:04 UTC (permalink / raw)
  To: Alexandra Chin
  Cc: Henrik Rydberg, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Alexandra,

On Wed, Nov 14, 2012 at 05:40:12PM +0000, Alexandra Chin wrote:
> Hi Henrik and all,
> 
> As I know, currently there is no synaptics RMI4 touchscreen driver in the
> main tree. In order to support our customers effectively, is it able to merge
> staging ste_rmi4 driver into the main kernel.org tree? If it is accepted, should
> I send a patch for this change?

I believe that we should keep ste_rmi4 in staging, not as reflection of
its code quality, but rather because it is a temporary solution until
full-fledged RMI4 driver is merged.

Please have Greg commit the patch that Henrik reviewed to staging and
then work with Christopher Heiny group on getting the full featured
driver into mainline.

Thanks.

-- 
Dmitry

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

* RE: [RFC] staging: ste_rmi4: merge into the main kernel tree
  2012-11-14 18:04       ` Dmitry Torokhov
@ 2012-11-15  2:56         ` Alexandra Chin
  2012-11-15  6:43           ` Dmitry Torokhov
  0 siblings, 1 reply; 32+ messages in thread
From: Alexandra Chin @ 2012-11-15  2:56 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Henrik Rydberg, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Dmitry,

> > As I know, currently there is no synaptics RMI4 touchscreen driver in the
> > main tree. In order to support our customers effectively, is it able to merge
> > staging ste_rmi4 driver into the main kernel.org tree? If it is accepted, should
> > I send a patch for this change?
> 
> I believe that we should keep ste_rmi4 in staging, not as reflection of
> its code quality, but rather because it is a temporary solution until
> full-fledged RMI4 driver is merged.
> 
> Please have Greg commit the patch that Henrik reviewed to staging and
> then work with Christopher Heiny group on getting the full featured
> driver into mainline.

Thank you for the prompt reply. The ste_rmi4 driver is actually not intended to be
a temporary solution for the full-fledged driver. We have requests from a number
of our customers and partners (Qualcomm, Samsung, Intel etc.) urging us to have
a driver available in the kernel mainline that is specific to touchscreen with simple
architecture and supporting our S3200 family of Touch Controllers and above. We
are committing resources to continue to maintain this driver and provide updates
to support future touch controller revisions from Synaptics.
It would be greatly appreciated if this driver can be considered for inclusion in the
kernel mainline.

Thank you.

Alexandra Chin

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

* Re: [RFC] staging: ste_rmi4: merge into the main kernel tree
  2012-11-15  2:56         ` Alexandra Chin
@ 2012-11-15  6:43           ` Dmitry Torokhov
  2012-11-15 17:41             ` Linus Walleij
  2012-11-16  9:02             ` Alexandra Chin
  0 siblings, 2 replies; 32+ messages in thread
From: Dmitry Torokhov @ 2012-11-15  6:43 UTC (permalink / raw)
  To: Alexandra Chin
  Cc: Henrik Rydberg, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

On Thu, Nov 15, 2012 at 02:56:45AM +0000, Alexandra Chin wrote:
> Hi Dmitry,
> 
> > > As I know, currently there is no synaptics RMI4 touchscreen driver in the
> > > main tree. In order to support our customers effectively, is it able to merge
> > > staging ste_rmi4 driver into the main kernel.org tree? If it is accepted, should
> > > I send a patch for this change?
> > 
> > I believe that we should keep ste_rmi4 in staging, not as reflection of
> > its code quality, but rather because it is a temporary solution until
> > full-fledged RMI4 driver is merged.
> > 
> > Please have Greg commit the patch that Henrik reviewed to staging and
> > then work with Christopher Heiny group on getting the full featured
> > driver into mainline.
> 
> Thank you for the prompt reply. The ste_rmi4 driver is actually not intended to be
> a temporary solution for the full-fledged driver. We have requests from a number
> of our customers and partners (Qualcomm, Samsung, Intel etc.) urging us to have
> a driver available in the kernel mainline that is specific to touchscreen with simple
> architecture and supporting our S3200 family of Touch Controllers and above. We
> are committing resources to continue to maintain this driver and provide updates
> to support future touch controller revisions from Synaptics.
> It would be greatly appreciated if this driver can be considered for inclusion in the
> kernel mainline.

In this case you need to enumerate the benefits of this driver over
unified driver and show why the unified driver can't be fixed.

Currently the driver is in mainline (even though the directory is
staging) and nobody will remove it until another driver is fully
functional and ready for prime time. But once this happens I do not see
the benefits of maintaining 2 drivers for the same hardware.

Thank you.

-- 
Dmitry

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

* Re: [RFC] staging: ste_rmi4: merge into the main kernel tree
  2012-11-15  6:43           ` Dmitry Torokhov
@ 2012-11-15 17:41             ` Linus Walleij
  2012-11-15 18:55               ` Dmitry Torokhov
  2012-11-16  9:02             ` Alexandra Chin
  1 sibling, 1 reply; 32+ messages in thread
From: Linus Walleij @ 2012-11-15 17:41 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Alexandra Chin, Henrik Rydberg, Linux Kernel, Linux Input,
	Linus Walleij, Naveen Kumar Gaddipati, Mahesh Srinivasan,
	Alex Chang, Scott Lin, Christopher Heiny

On Thu, Nov 15, 2012 at 7:43 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> In this case you need to enumerate the benefits of this driver over
> unified driver and show why the unified driver can't be fixed.

The benefit is that the ugliness in
drivers/staging/ste_rmi4/board-mop500-u8500uib-rmi4.c
can be avoided, as today we're unable to pass the compulsory
platform data in any sane way. I'm being told that this weak symbol
override actually does not really work... especially if there are more
than 1 I2C device on the bus :-P

Another way of fixing the staging driver usable is to add device tree
support, of course, that would decopule it completely from the
platform using it.

Yours,
Linus Walleij

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

* Re: [RFC] staging: ste_rmi4: merge into the main kernel tree
  2012-11-15 17:41             ` Linus Walleij
@ 2012-11-15 18:55               ` Dmitry Torokhov
  2012-11-15 19:05                 ` Linus Walleij
  0 siblings, 1 reply; 32+ messages in thread
From: Dmitry Torokhov @ 2012-11-15 18:55 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexandra Chin, Henrik Rydberg, Linux Kernel, Linux Input,
	Linus Walleij, Naveen Kumar Gaddipati, Mahesh Srinivasan,
	Alex Chang, Scott Lin, Christopher Heiny

Hi Linus,

On Thu, Nov 15, 2012 at 06:41:26PM +0100, Linus Walleij wrote:
> On Thu, Nov 15, 2012 at 7:43 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> 
> > In this case you need to enumerate the benefits of this driver over
> > unified driver and show why the unified driver can't be fixed.
> 
> The benefit is that the ugliness in
> drivers/staging/ste_rmi4/board-mop500-u8500uib-rmi4.c
> can be avoided, as today we're unable to pass the compulsory
> platform data in any sane way. I'm being told that this weak symbol
> override actually does not really work... especially if there are more
> than 1 I2C device on the bus :-P

I am confused as to why this would be a benefit of ste_rmi4 over full
RMI4 implementation from Christopher's group?

I think what we really need is to accelerate work on the full driver so
it is in mainline in 3.9ish.

Thanks.

-- 
Dmitry

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

* Re: [RFC] staging: ste_rmi4: merge into the main kernel tree
  2012-11-15 18:55               ` Dmitry Torokhov
@ 2012-11-15 19:05                 ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2012-11-15 19:05 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Alexandra Chin, Henrik Rydberg, Linux Kernel, Linux Input,
	Linus Walleij, Naveen Kumar Gaddipati, Mahesh Srinivasan,
	Alex Chang, Scott Lin, Christopher Heiny

On Thu, Nov 15, 2012 at 7:55 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, Nov 15, 2012 at 06:41:26PM +0100, Linus Walleij wrote:
>>
>> The benefit is that the ugliness in
>> drivers/staging/ste_rmi4/board-mop500-u8500uib-rmi4.c
>> can be avoided, as today we're unable to pass the compulsory
>> platform data in any sane way. I'm being told that this weak symbol
>> override actually does not really work... especially if there are more
>> than 1 I2C device on the bus :-P
>
> I am confused as to why this would be a benefit of ste_rmi4 over full
> RMI4 implementation from Christopher's group?

So we don't need to keep our real platform data and board files
out-of-tree like we've done for the last 2 years or so.

But I only said it's a benefit, not that I think it's a good idea.

> I think what we really need is to accelerate work on the full driver so
> it is in mainline in 3.9ish.

Yep, agreed.

Yours,
Linus Walleij

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

* [PATCH v6] staging: ste_rmi4: Convert to Type-B support
  2012-11-02 17:36 [PATCH v3] staging: ste_rmi4: Convert to Type-B support Alexandra Chin
                   ` (3 preceding siblings ...)
  2012-11-13 12:12 ` [PATCH v6] " Alexandra Chin
@ 2012-11-16  8:31 ` Alexandra Chin
  2012-11-16 19:54   ` Henrik Rydberg
  4 siblings, 1 reply; 32+ messages in thread
From: Alexandra Chin @ 2012-11-16  8:31 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Henrik Rydberg
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Convert to MT-B because Synaptics touch devices are capable
of tracking identifiable fingers.

Signed-off-by: Alexandra Chin <alexandra.chin@tw.synaptics.com>
---
This patch was tested on Pandaboard.

Changes from v6:
        - Incorporated Henrik's review comments
          *remove irrelevant changes within the patch

Changes from v5:
        - Incorporated Henrik's review comments
          *rollback to v3 from v4
          *fix odd line break in v3

Changes from v4:
        - Incorporated Henrik's review comments
          *split function synpatics_rmi4_touchscreen_report
          *split function synaptics_rmi4_i2c_query_device

Changes from v3:
	- Incorporated Henrik's review comments
	  *remove 'else' after an error path return
	  *add input_mt_sync_frame() for pointer emulation effects
	  *correct names of touchscreen
	- Replace printk with dev_err

Changes from v2:
	- Incorporated Henrik's review comments
	  *directly report finger state with Type-B
	- Against 3.7-rcX
	  *call input_mt_init_slots with INPUT_MT_DIRECT flag
---
 drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c |   76 +++++++++++--------------
 1 files changed, 33 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
index 277491a..ed304e0 100644
--- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
+++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
@@ -31,6 +31,7 @@
 #include <linux/interrupt.h>
 #include <linux/regulator/consumer.h>
 #include <linux/module.h>
+#include <linux/input/mt.h>
 #include "synaptics_i2c_rmi4.h"
 
 /* TODO: for multiple device support will need a per-device mutex */
@@ -67,7 +68,6 @@
 #define PDT_START_SCAN_LOCATION (0x00E9)
 #define PDT_END_SCAN_LOCATION	(0x000A)
 #define PDT_ENTRY_SIZE		(0x0006)
-#define RMI4_NUMBER_OF_MAX_FINGERS		(8)
 #define SYNAPTICS_RMI4_TOUCHPAD_FUNC_NUM	(0x11)
 #define SYNAPTICS_RMI4_DEVICE_CONTROL_FUNC_NUM	(0x01)
 
@@ -164,6 +164,7 @@ struct synaptics_rmi4_device_info {
  * @regulator: pointer to the regulator structure
  * @wait: wait queue structure variable
  * @touch_stopped: flag to stop the thread function
+ * @fingers_supported: maximum supported fingers
  *
  * This structure gives the device data information.
  */
@@ -184,6 +185,7 @@ struct synaptics_rmi4_data {
 	struct regulator	*regulator;
 	wait_queue_head_t	wait;
 	bool			touch_stopped;
+	unsigned char		fingers_supported;
 };
 
 /**
@@ -303,22 +305,21 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 	/* number of touch points - fingers down in this case */
 	int	touch_count = 0;
 	int	finger;
-	int	fingers_supported;
 	int	finger_registers;
 	int	reg;
 	int	finger_shift;
 	int	finger_status;
 	int	retval;
+	int	x, y;
+	int	wx, wy;
 	unsigned short	data_base_addr;
 	unsigned short	data_offset;
 	unsigned char	data_reg_blk_size;
 	unsigned char	values[2];
 	unsigned char	data[DATA_LEN];
-	int	x[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	y[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	wx[RMI4_NUMBER_OF_MAX_FINGERS];
-	int	wy[RMI4_NUMBER_OF_MAX_FINGERS];
+	unsigned char	fingers_supported = pdata->fingers_supported;
 	struct	i2c_client *client = pdata->i2c_client;
+	struct	input_dev *input_dev = pdata->input_dev;
 
 	/* get 2D sensor finger data */
 	/*
@@ -333,7 +334,6 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 	 *	10 = finger present but data may not be accurate,
 	 *	11 = reserved for product use.
 	 */
-	fingers_supported	= rfi->num_of_data_points;
 	finger_registers	= (fingers_supported + 3)/4;
 	data_base_addr		= rfi->fn_desc.data_base_addr;
 	retval = synaptics_rmi4_i2c_block_read(pdata, data_base_addr, values,
@@ -358,7 +358,11 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 		 * if finger status indicates a finger is present then
 		 * read the finger data and report it
 		 */
-		if (finger_status == 1 || finger_status == 2) {
+		input_mt_slot(input_dev, finger);
+		input_mt_report_slot_state(input_dev, MT_TOOL_FINGER,
+							finger_status != 0);
+
+		if (finger_status) {
 			/* Read the finger data */
 			data_offset = data_base_addr +
 					((finger * data_reg_blk_size) +
@@ -367,50 +371,33 @@ static int synpatics_rmi4_touchpad_report(struct synaptics_rmi4_data *pdata,
 						data_offset, data,
 						data_reg_blk_size);
 			if (retval != data_reg_blk_size) {
-				printk(KERN_ERR "%s:read data failed\n",
+				dev_err(&client->dev, "%s:read data failed\n",
 								__func__);
 				return 0;
-			} else {
-				x[touch_count]	=
-					(data[0] << 4) | (data[2] & MASK_4BIT);
-				y[touch_count]	=
-					(data[1] << 4) |
-					((data[2] >> 4) & MASK_4BIT);
-				wy[touch_count]	=
-						(data[3] >> 4) & MASK_4BIT;
-				wx[touch_count]	=
-						(data[3] & MASK_4BIT);
-
-				if (pdata->board->x_flip)
-					x[touch_count] =
-						pdata->sensor_max_x -
-								x[touch_count];
-				if (pdata->board->y_flip)
-					y[touch_count] =
-						pdata->sensor_max_y -
-								y[touch_count];
 			}
+			x = (data[0] << 4) | (data[2] & MASK_4BIT);
+			y = (data[1] << 4) | ((data[2] >> 4) & MASK_4BIT);
+			wy = (data[3] >> 4) & MASK_4BIT;
+			wx = (data[3] & MASK_4BIT);
+
+			if (pdata->board->x_flip)
+				x = pdata->sensor_max_x - x;
+			if (pdata->board->y_flip)
+				y = pdata->sensor_max_y - y;
+
+			input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
+								max(wx, wy));
+			input_report_abs(input_dev, ABS_MT_POSITION_X, x);
+			input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
+
 			/* number of active touch points */
 			touch_count++;
 		}
 	}
 
-	/* report to input subsystem */
-	if (touch_count) {
-		for (finger = 0; finger < touch_count; finger++) {
-			input_report_abs(pdata->input_dev, ABS_MT_TOUCH_MAJOR,
-						max(wx[finger] , wy[finger]));
-			input_report_abs(pdata->input_dev, ABS_MT_POSITION_X,
-								x[finger]);
-			input_report_abs(pdata->input_dev, ABS_MT_POSITION_Y,
-								y[finger]);
-			input_mt_sync(pdata->input_dev);
-		}
-	} else
-		input_mt_sync(pdata->input_dev);
-
 	/* sync after groups of events */
-	input_sync(pdata->input_dev);
+	input_mt_sync_frame(input_dev);
+	input_sync(input_dev);
 	/* return the number of touch points */
 	return touch_count;
 }
@@ -575,6 +562,7 @@ static int synpatics_rmi4_touchpad_detect(struct synaptics_rmi4_data *pdata,
 		if ((queries[1] & MASK_3BIT) == 5)
 			rfi->num_of_data_points = 10;
 	}
+	pdata->fingers_supported = rfi->num_of_data_points;
 	/* Need to get interrupt info for handling interrupts */
 	rfi->index_to_intr_reg = (interruptcount + 7)/8;
 	if (rfi->index_to_intr_reg != 0)
@@ -988,6 +976,8 @@ static int __devinit synaptics_rmi4_probe
 					rmi4_data->sensor_max_y, 0, 0);
 	input_set_abs_params(rmi4_data->input_dev, ABS_MT_TOUCH_MAJOR, 0,
 						MAX_TOUCH_MAJOR, 0, 0);
+	input_mt_init_slots(rmi4_data->input_dev,
+				rmi4_data->fingers_supported, 0);
 
 	/* Clear interrupts */
 	synaptics_rmi4_i2c_block_read(rmi4_data,
-- 
1.7.5.4


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

* RE: [RFC] staging: ste_rmi4: merge into the main kernel tree
  2012-11-15  6:43           ` Dmitry Torokhov
  2012-11-15 17:41             ` Linus Walleij
@ 2012-11-16  9:02             ` Alexandra Chin
  2012-11-16 19:24               ` Dmitry Torokhov
  1 sibling, 1 reply; 32+ messages in thread
From: Alexandra Chin @ 2012-11-16  9:02 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Henrik Rydberg, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Dmitry,

> > > Please have Greg commit the patch that Henrik reviewed to staging and
> > > then work with Christopher Heiny group on getting the full featured
> > > driver into mainline.

Thanks for your reminding, final patch has been re-sent to staging
maintainer.

> In this case you need to enumerate the benefits of this driver over
> unified driver and show why the unified driver can't be fixed.
> 
> Currently the driver is in mainline (even though the directory is
> staging) and nobody will remove it until another driver is fully
> functional and ready for prime time. But once this happens I do not see
> the benefits of maintaining 2 drivers for the same hardware.

I see. Given that the driver will remain in mainline before generic driver
is ready, currently I will continue to maintain the driver in staging.

Appreciate your response.

Alexandra

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

* Re: [RFC] staging: ste_rmi4: merge into the main kernel tree
  2012-11-16  9:02             ` Alexandra Chin
@ 2012-11-16 19:24               ` Dmitry Torokhov
  0 siblings, 0 replies; 32+ messages in thread
From: Dmitry Torokhov @ 2012-11-16 19:24 UTC (permalink / raw)
  To: Alexandra Chin
  Cc: Henrik Rydberg, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

On Friday, November 16, 2012 09:02:30 AM Alexandra Chin wrote:
> Hi Dmitry,
> 
> > > > Please have Greg commit the patch that Henrik reviewed to staging and
> > > > then work with Christopher Heiny group on getting the full featured
> > > > driver into mainline.
> 
> Thanks for your reminding, final patch has been re-sent to staging
> maintainer.
> 
> > In this case you need to enumerate the benefits of this driver over
> > unified driver and show why the unified driver can't be fixed.
> > 
> > Currently the driver is in mainline (even though the directory is
> > staging) and nobody will remove it until another driver is fully
> > functional and ready for prime time. But once this happens I do not see
> > the benefits of maintaining 2 drivers for the same hardware.
> 
> I see. Given that the driver will remain in mainline before generic driver
> is ready, currently I will continue to maintain the driver in staging.

Sounds like a plan.

Thanks!

-- 
Dmitry

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

* Re: [PATCH v6] staging: ste_rmi4: Convert to Type-B support
  2012-11-16 19:54   ` Henrik Rydberg
@ 2012-11-16 19:50     ` Greg Kroah-Hartman
  2012-12-06 11:30       ` Alexandra Chin
  0 siblings, 1 reply; 32+ messages in thread
From: Greg Kroah-Hartman @ 2012-11-16 19:50 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Alexandra Chin, Dmitry Torokhov, Linux Kernel, Linux Input,
	Linus Walleij, Naveen Kumar Gaddipati, Mahesh Srinivasan,
	Alex Chang, Scott Lin, Christopher Heiny

On Fri, Nov 16, 2012 at 08:54:33PM +0100, Henrik Rydberg wrote:
> Hi Alexandra,
> 
> > Convert to MT-B because Synaptics touch devices are capable
> > of tracking identifiable fingers.
> > 
> > Signed-off-by: Alexandra Chin <alexandra.chin@tw.synaptics.com>
> 
> For next time: Once you have received an acked-by or reviewed-by, you
> may add that line under your own signed-off-by in subsequent
> mails. That way, the status of the patch can be tracked properly,
> making life easier for everyone.
> 
> Greg, I think we are ready now. :-)

All applied.

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

* Re: [PATCH v6] staging: ste_rmi4: Convert to Type-B support
  2012-11-16  8:31 ` [PATCH v6] staging: ste_rmi4: Convert to Type-B support Alexandra Chin
@ 2012-11-16 19:54   ` Henrik Rydberg
  2012-11-16 19:50     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 32+ messages in thread
From: Henrik Rydberg @ 2012-11-16 19:54 UTC (permalink / raw)
  To: Alexandra Chin
  Cc: Greg Kroah-Hartman, Dmitry Torokhov, Linux Kernel, Linux Input,
	Linus Walleij, Naveen Kumar Gaddipati, Mahesh Srinivasan,
	Alex Chang, Scott Lin, Christopher Heiny

Hi Alexandra,

> Convert to MT-B because Synaptics touch devices are capable
> of tracking identifiable fingers.
> 
> Signed-off-by: Alexandra Chin <alexandra.chin@tw.synaptics.com>

For next time: Once you have received an acked-by or reviewed-by, you
may add that line under your own signed-off-by in subsequent
mails. That way, the status of the patch can be tracked properly,
making life easier for everyone.

Greg, I think we are ready now. :-)

Thanks,
Henrik

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

* RE: [PATCH v6] staging: ste_rmi4: Convert to Type-B support
  2012-11-16 19:50     ` Greg Kroah-Hartman
@ 2012-12-06 11:30       ` Alexandra Chin
  2012-12-06 16:34         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 32+ messages in thread
From: Alexandra Chin @ 2012-12-06 11:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Henrik Rydberg
  Cc: Dmitry Torokhov, Linux Kernel, Linux Input, Linus Walleij,
	Naveen Kumar Gaddipati, Mahesh Srinivasan, Alex Chang, Scott Lin,
	Christopher Heiny

Hi Greg,

> > For next time: Once you have received an acked-by or reviewed-by, you
> > may add that line under your own signed-off-by in subsequent
> > mails. That way, the status of the patch can be tracked properly,
> > making life easier for everyone.
> >
> > Greg, I think we are ready now. :-)
> 
> All applied.

There are coming patches which needs to be updated and based on this patch,
but I noticed that this patch has not been submitted in recent mainline
archives (3.7-rc7/3.7-rc8).
May I know status of this patch? Please let me know if there is anything I 
should do.
Appreciate your advice.

Best regards,
Alexandra

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

* Re: [PATCH v6] staging: ste_rmi4: Convert to Type-B support
  2012-12-06 11:30       ` Alexandra Chin
@ 2012-12-06 16:34         ` Greg Kroah-Hartman
  2012-12-07  4:01           ` Alexandra Chin
  0 siblings, 1 reply; 32+ messages in thread
From: Greg Kroah-Hartman @ 2012-12-06 16:34 UTC (permalink / raw)
  To: Alexandra Chin
  Cc: Henrik Rydberg, Dmitry Torokhov, Linux Kernel, Linux Input,
	Linus Walleij, Naveen Kumar Gaddipati, Mahesh Srinivasan,
	Alex Chang, Scott Lin, Christopher Heiny

On Thu, Dec 06, 2012 at 11:30:55AM +0000, Alexandra Chin wrote:
> Hi Greg,
> 
> > > For next time: Once you have received an acked-by or reviewed-by, you
> > > may add that line under your own signed-off-by in subsequent
> > > mails. That way, the status of the patch can be tracked properly,
> > > making life easier for everyone.
> > >
> > > Greg, I think we are ready now. :-)
> > 
> > All applied.
> 
> There are coming patches which needs to be updated and based on this patch,
> but I noticed that this patch has not been submitted in recent mainline
> archives (3.7-rc7/3.7-rc8).

That is because they were new features, and not regressions, so they
will be showing up in Linus's tree for 3.8-rc1.

> May I know status of this patch? Please let me know if there is anything I 
> should do.

Didn't you get an email when I applied the patches saying where the
patches could be seen and when they would be merged to Linus?

thanks,

greg k-h

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

* RE: [PATCH v6] staging: ste_rmi4: Convert to Type-B support
  2012-12-06 16:34         ` Greg Kroah-Hartman
@ 2012-12-07  4:01           ` Alexandra Chin
  2012-12-07  5:11             ` Greg Kroah-Hartman
  0 siblings, 1 reply; 32+ messages in thread
From: Alexandra Chin @ 2012-12-07  4:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Henrik Rydberg, Dmitry Torokhov, Linux Kernel, Linux Input,
	Linus Walleij, Naveen Kumar Gaddipati, Mahesh Srinivasan,
	Alex Chang, Scott Lin, Christopher Heiny

Hi Greg,

> That is because they were new features, and not regressions, so they
> will be showing up in Linus's tree for 3.8-rc1.
> 
> > May I know status of this patch? Please let me know if there is anything I
> > should do.
> 
> Didn't you get an email when I applied the patches saying where the
> patches could be seen and when they would be merged to Linus?
> 

Yes, I've received an email saying that the patch will show up in the next
release of the linux-next tree, but I was not aware that linux-next tree
means 3.8-rc1.
Now I understand, and really thank you for your prompt reply.

Alexandra

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

* Re: [PATCH v6] staging: ste_rmi4: Convert to Type-B support
  2012-12-07  4:01           ` Alexandra Chin
@ 2012-12-07  5:11             ` Greg Kroah-Hartman
  2012-12-07  6:45               ` Alexandra Chin
  0 siblings, 1 reply; 32+ messages in thread
From: Greg Kroah-Hartman @ 2012-12-07  5:11 UTC (permalink / raw)
  To: Alexandra Chin
  Cc: Henrik Rydberg, Dmitry Torokhov, Linux Kernel, Linux Input,
	Linus Walleij, Naveen Kumar Gaddipati, Mahesh Srinivasan,
	Alex Chang, Scott Lin, Christopher Heiny

On Fri, Dec 07, 2012 at 04:01:10AM +0000, Alexandra Chin wrote:
> Hi Greg,
> 
> > That is because they were new features, and not regressions, so they
> > will be showing up in Linus's tree for 3.8-rc1.
> > 
> > > May I know status of this patch? Please let me know if there is anything I
> > > should do.
> > 
> > Didn't you get an email when I applied the patches saying where the
> > patches could be seen and when they would be merged to Linus?
> > 
> 
> Yes, I've received an email saying that the patch will show up in the next
> release of the linux-next tree, but I was not aware that linux-next tree
> means 3.8-rc1.

It does not, you didn't read the next sentance which described when the
patch would be sent to Linus...

greg k-h

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

* RE: [PATCH v6] staging: ste_rmi4: Convert to Type-B support
  2012-12-07  5:11             ` Greg Kroah-Hartman
@ 2012-12-07  6:45               ` Alexandra Chin
  0 siblings, 0 replies; 32+ messages in thread
From: Alexandra Chin @ 2012-12-07  6:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Henrik Rydberg, Dmitry Torokhov, Linux Kernel, Linux Input,
	Linus Walleij, Naveen Kumar Gaddipati, Mahesh Srinivasan,
	Alex Chang, Scott Lin, Christopher Heiny

Hi Greg,

> > Yes, I've received an email saying that the patch will show up in the next
> > release of the linux-next tree, but I was not aware that linux-next tree
> > means 3.8-rc1.
> 
> It does not, you didn't read the next sentance which described when the
> patch would be sent to Linus...

Thank you for correcting. Correction to my earlier reply: The patch will
show up in the next release of the linux-next tree, then to Linus Torvalds.
And if it gets accepted at each step along the way, it will be merged in
the mainline :-)

Best regards,
Alexandra

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

end of thread, other threads:[~2012-12-07  6:45 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-02 17:36 [PATCH v3] staging: ste_rmi4: Convert to Type-B support Alexandra Chin
2012-11-04 21:53 ` Henrik Rydberg
2012-11-05  6:01   ` Alexandra Chin
2012-11-12 11:59   ` Alexandra Chin
2012-11-07  5:49 ` [PATCH v4] " Alexandra Chin
2012-11-07 19:57   ` Henrik Rydberg
2012-11-08  4:01     ` Alexandra Chin
2012-11-08  7:05 ` [PATCH v5] " Alexandra Chin
2012-11-09 19:26   ` Henrik Rydberg
2012-11-12 16:40     ` Alexandra Chin
2012-11-12 17:20       ` Henrik Rydberg
2012-11-13  2:04         ` Alexandra Chin
2012-11-13 12:12 ` [PATCH v6] " Alexandra Chin
2012-11-13 17:53   ` Henrik Rydberg
2012-11-14 17:33     ` Alexandra Chin
2012-11-14 17:40     ` [RFC] staging: ste_rmi4: merge into the main kernel tree Alexandra Chin
2012-11-14 18:04       ` Dmitry Torokhov
2012-11-15  2:56         ` Alexandra Chin
2012-11-15  6:43           ` Dmitry Torokhov
2012-11-15 17:41             ` Linus Walleij
2012-11-15 18:55               ` Dmitry Torokhov
2012-11-15 19:05                 ` Linus Walleij
2012-11-16  9:02             ` Alexandra Chin
2012-11-16 19:24               ` Dmitry Torokhov
2012-11-16  8:31 ` [PATCH v6] staging: ste_rmi4: Convert to Type-B support Alexandra Chin
2012-11-16 19:54   ` Henrik Rydberg
2012-11-16 19:50     ` Greg Kroah-Hartman
2012-12-06 11:30       ` Alexandra Chin
2012-12-06 16:34         ` Greg Kroah-Hartman
2012-12-07  4:01           ` Alexandra Chin
2012-12-07  5:11             ` Greg Kroah-Hartman
2012-12-07  6:45               ` Alexandra Chin

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