Linux Input/HID development
 help / color / mirror / Atom feed
* Dell Latitude E7440 - ALPS touchpad keeps having state reset
From: Allan Crooks @ 2014-01-27 15:23 UTC (permalink / raw)
  To: linux-input

Hi,

I've reported the problem I'm having in more detail here:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1258837

In summary - I keep suffering from the mouse pointer jumping around
erratically, with the following messages output to syslog:
 psmouse serio1: DualPoint TouchPad at isa0060/serio1/input0 lost sync at byte 1
 psmouse serio1: DualPoint TouchPad at isa0060/serio1/input0 - driver resynced.

Other people have reported the same problem in that bug report.
Bisecting the kernel, the problem seems to have always been in place
as soon as the touchpad was correctly identified; according to xinput
- it's a "AlpsPS/2 ALPS DualPoint TouchPad". When it was being
identified as a generic PS/2 mouse, there's no problem (though of
course, there's also no touchpad functionality available either).

Is there anything I can do to help provide information to get this fixed?

Thanks,
Allan.

^ permalink raw reply

* Input: zforce - fix various small issues
From: Luis Ortega @ 2014-01-27 18:46 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: heiko, yongjun_wei, linux-input, linux-kernel

As a kernel newbie and owner of a Barnes & Noble e-reader I was
curious to review this driver to learn more about the touchscreen.

The first two patches are fairly innocuous whereas the last two
slightly modify the code to fix two small issues I discovered.
I don't have the setup to test them but they look logically correct
to me.

[PATCH 1/4] Input: zforce - fix spelling errors
[PATCH 2/4] Input: zforce - fix lines exceeding 80 columns
[PATCH 3/4] Input: zforce - Remove unnecessary payload data checks
[PATCH 4/4] Input: zforce - reduce stack memory allocated to frames

^ permalink raw reply

* [PATCH 4/4] Input: zforce - reduce stack memory allocated to frames
From: Luis Ortega @ 2014-01-27 18:46 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: heiko, yongjun_wei, linux-input, linux-kernel, Luis Ortega
In-Reply-To: <1390848373-7723-1-git-send-email-luiorpe1@upv.es>

A frame is a u8 array with the following structure:
[PAYLOAD_HEADER, PAYLOAD_LENGTH, ...PAYLOAD_BODY...]

PAYLOAD_BODY can be at most 255 bytes long, as it's size is represented
by PAYLOAD_LENGTH. Therefore we can reduce the stack memory allocated to
payload_buffer[] roughly by half, from 512 to 257 bytes.

Signed-off-by: Luis Ortega <luiorpe1@upv.es>
---
 drivers/input/touchscreen/zforce_ts.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index e082d5c..afb2492 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -33,6 +33,7 @@
 #define WAIT_TIMEOUT		msecs_to_jiffies(1000)
 
 #define FRAME_START		0xee
+#define FRAME_MAXSIZE		257
 
 /* Offsets of the different parts of the payload the controller sends */
 #define PAYLOAD_HEADER		0
@@ -475,7 +476,7 @@ static irqreturn_t zforce_irq_thread(int irq, void *dev_id)
 	struct i2c_client *client = ts->client;
 	const struct zforce_ts_platdata *pdata = dev_get_platdata(&client->dev);
 	int ret;
-	u8 payload_buffer[512];
+	u8 payload_buffer[FRAME_MAXSIZE];
 	u8 *payload;
 
 	/*
-- 
1.8.5.3


^ permalink raw reply related

* Re: [PATCH v2 2/2] HID: sony: Add output events for the multi-touch pad on the Dualshock 4.
From: simon @ 2014-01-27 17:47 UTC (permalink / raw)
  Cc: linux-input, jkosina, Frank Praznik
In-Reply-To: <1390835857-4523-2-git-send-email-frank.praznik@oh.rr.com>


> +
> +	/* The Dualshock 4 multi-touch trackpad data starts at offset 35 on USB.
> +	 * The first 7 bits of the first byte is a counter and bit 8 is a touch
> +	 * indicator that is 0 when pressed and 1 when not pressed.
> +	 * The next 3 bytes are two 12 bit touch coordinates, X and Y.
> +	 * The data for the second touch is in the same format and immediatly
> +	 * follows the data for the first.
> +	 */

Hi Frank,
I don't know if it helps multitouch, but there is a second set of data in
the HID stream, that is present when a figure is moving/swiping on the
touchpad. This gives us a 'vector' rather than a 'point'.

I previously attempted to describe the format here:
http://www.spinics.net/lists/linux-input/msg28525.html

Ahead of the 'event counter' is a 'point or vector' byte and what might be
a 'pressure' measurement (couldn't really tell).

Interspersed with the data you mention is a second set of X1/Y1/X2/Y2 
containing the starting position of the swipe/vector. These are
differentiated with '02' (rather than '01') in the 34'th byte.

1 finger swipe left to right
--
# cat /dev/hidraw0 | hexdump -v -e '64/1 "%02x " "\n"' | cut -d ' ' -f
34-52 | grep -e '^02'
02 01 34 93 90 1d 9e bd e0 23 06 34 94 90 1d 9e bd e0 23
02 1f 34 a5 90 1d 9e bd e0 23 22 34 aa 90 1d 9e bd e0 23
02 3c 34 c3 90 1d 9e bd e0 23 41 34 ca 90 1d 9e bd e0 23
02 7c 34 37 c1 1c 9e bd e0 23 81 34 47 c1 1c 9e bd e0 23
02 bd 34 23 e2 1b 9e bd e0 23 c2 34 3a e2 1b 9e bd e0 23
02 d4 34 8e 82 1b 9e bd e0 23 d9 34 ad 72 1b 9e bd e0 23
02 f8 34 3d 23 1b 9e bd e0 23 fb 34 55 13 1b 9e bd e0 23
02 14 34 a5 e3 1a 9e bd e0 23 1a 34 b6 d3 1a 9e bd e0 23
02 2e 34 f3 b3 1a 9e bd e0 23 31 34 01 a4 1a 9e bd e0 23
02 55 34 40 a4 1a 9e bd e0 23 5a 34 47 a4 1a 9e bd e0 23
02 95 34 71 a4 1a 9e bd e0 23 9a 34 73 a4 1a 9e bd e0 23
02 ad 34 78 a4 1a 9e bd e0 23 b2 34 7a a4 1a 9e bd e0 23
      [ data you use....... ]    [ start of swipe....  ]
--

2 finger swipe top to bottom
--
# cat /dev/hidraw0 | hexdump -v -e '64/1 "%02x " "\n"' | cut -d ' ' -f
34-52 | grep -e '^02'
02 26 37 4a b6 04 9e bd e0 23 2b 37 4a c6 04 9e bd e0 23
02 5b 37 43 96 05 38 64 80 0b 60 37 43 d6 05 38 64 80 0b
02 84 37 3f 56 08 38 6c b0 0c 89 37 3f e6 08 38 6e 00 0d
02 a7 37 38 86 0d 38 80 c0 0f ad 37 37 b6 0e 38 85 80 10
02 d0 37 32 b6 16 38 bc d0 16 d5 37 32 a6 17 38 c5 c0 17
02 f4 37 32 16 1b 38 e7 00 1b f9 37 32 86 1b 38 eb 50 1b
02 17 37 32 f6 1c 38 f5 70 1c 1c 37 32 26 1d 38 f6 90 1c
02 40 37 32 36 1e 38 fd 40 1d 44 37 32 56 1e 38 fd 50 1d
02 63 37 32 c6 1e 38 00 91 1d 68 37 32 e6 1e 38 00 a1 1d
02 86 37 32 46 1f 38 03 d1 1d 8b 37 32 56 1f 38 03 e1 1d
02 af 37 32 b6 1f 38 06 11 1e b3 37 32 c6 1f 38 06 11 1e
02 3b 37 32 96 20 38 0b 81 1e 41 37 32 96 20 38 0b 91 1e
02 82 37 32 e6 20 38 0c c1 1e 87 37 32 e6 20 38 0c c1 1e
02 a5 37 32 16 21 38 0c 01 1f aa 37 32 26 21 38 0c 01 1f
--

Also note that there are seperate event counters for 1st and 2nd fingers,
which mean that the 2nd finger can remain pressed whilst the 1st is
pressed/released.
--
# cat /dev/hidraw0 | hexdump -v -e '64/1 "%02x " "\n"' | cut -d ' ' -f
34-52 | grep -e '^02'
02 a6>42<16 b1 19 c1 58 21 15 ab 42 16 b1 19 c1 58 21 15 <- 1st press
02 15 42 26 b1 19 c1 58 21 15 1b 42 27 b1 19 c1 58 21 15
02 50 42 2d b1 19 c1 58 21 15 55 42 2e b1 19 c1 58 21 15
02 b6 42 41 b1 19 c3 e2 c6 12 bb 42 41 b1 19 44 b5 96 14
02 01 42 43 b1 19>44<a4 96 14 06 42 43 b1 19 44 a3 96 14 <- 2nd press
02 1f 42 43 b1 19 44 9d 96 14 24 42 43 b1 19 44 9c 96 14
02 6b 42 44 b1 19 44 8e 96 14 70 42 45 b1 19 44 8d 96 14
02 cf 42 46 b1 19 44 81 c6 13 d4 42 46 b1 19 44 80 c6 13
02 38 42 47 b1 19 44 78 a6 13 3d 42 47 b1 19 44 77 a6 13
02 3a 42 48 b1 19 44 6b 56 13 3f 42 48 b1 19 44 6b 56 13
02 f5 42 48 d1 1a 44 67 f6 12 fa 42 48 11 1b 44 67 e6 12
02 18 42 49 d1 1c 44 67 d6 12 1d 42 4a 41 1d 44 67 d6 12
02 47 42 53 81 21 44 66 c6 12 4c 42 54 41 22 44 66 c6 12
02 70 42 5d 01 28 44 66 b6 12 75 42 5f e1 28 44 66 b6 12
02 99 42 65 e1 2d 44 66 a6 12 9e 42 66 91 2e 44 66 a6 12
02 bd 42 68 61 31 44 65 96 12 c2 42 68 d1 31 44 65 96 12
02 0e>c2<6d d1 34 44 64 86 12 13 c2 6d d1 34 44 64 86 12 <- 1st release
02 26 c2 6d d1 34 44 64 86 12 2b c2 6d d1 34 44 64 86 12
02 68 c2 6d d1 34 44 62 76 12 6d c2 6d d1 34 44 62 76 12
02 07>45<52 71 08 44 62 76 12 0c 45 52 d1 08 44 62 76 12 <- 1st press
02 30 45 56 41 0c 44 62 76 12 34 45 57 d1 0c 44 62 76 12
02 58 45 5a 51 10 44 62 76 12 5d 45 5a d1 10 44 62 76 12
02 7b 45 5c f1 12 44 62 76 12 80 45 5d 51 13 44 62 76 12
02 ce 45 64 71 1a 44 62 76 12 d3 45 65 01 1b 44 62 76 12
02 f1 45 67 91 1d 44 62 66 12 f6 45 68 21 1e 44 62 66 12
02 3d 45 6f e1 22 44 62 66 12 42 45 6f 01 23 44 62 66 12
02 60 45 70 b1 23 44 62 66 12 65 45 70 c1 23 44 62 66 12
02 3a>c5<73 41 25 44 7a 56 12 3d c5 73 41 25 44 7d b6 12 <- 1st release
02 57 c5 73 41 25 44 87 f6 12 5c c5 73 41 25 44 8a 06 13
--

Simon.



^ permalink raw reply

* Re: [PATCH 1/4] Input: zforce - fix spelling errors
From: Heiko Stübner @ 2014-01-27 18:41 UTC (permalink / raw)
  To: Luis Ortega; +Cc: dmitry.torokhov, yongjun_wei, linux-input, linux-kernel
In-Reply-To: <1390848373-7723-2-git-send-email-luiorpe1@upv.es>

On Monday, 27. January 2014 19:46:10 Luis Ortega wrote:
> Fixed a few spelling errors.
> 
> Signed-off-by: Luis Ortega <luiorpe1@upv.es>

Acked-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  drivers/input/touchscreen/zforce_ts.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/zforce_ts.c
> b/drivers/input/touchscreen/zforce_ts.c index 2175f34..c145c1d 100644
> --- a/drivers/input/touchscreen/zforce_ts.c
> +++ b/drivers/input/touchscreen/zforce_ts.c
> @@ -64,7 +64,7 @@
>  #define RESPONSE_STATUS		0X1e
> 
>  /*
> - * Notifications are send by the touch controller without
> + * Notifications are sent by the touch controller without
>   * being requested by the driver and include for example
>   * touch indications
>   */
> @@ -103,8 +103,8 @@ struct zforce_point {
>   * @suspended		device suspended
>   * @access_mutex	serialize i2c-access, to keep multipart reads together
>   * @command_done	completion to wait for the command result
> - * @command_mutex	serialize commands send to the ic
> - * @command_waiting	the id of the command that that is currently waiting
> + * @command_mutex	serialize commands sent to the ic
> + * @command_waiting	the id of the command that is currently waiting
>   *			for a result
>   * @command_result	returned result of the command
>   */
> @@ -332,7 +332,7 @@ static int zforce_touch_event(struct zforce_ts *ts, u8
> *payload)
> 
>  	count = payload[0];
>  	if (count > ZFORCE_REPORT_POINTS) {
> -		dev_warn(&client->dev, "to many coordinates %d, expected max %d\n",
> +		dev_warn(&client->dev, "too many coordinates %d, expected max %d\n",
>  			 count, ZFORCE_REPORT_POINTS);
>  		count = ZFORCE_REPORT_POINTS;
>  	}
> @@ -798,7 +798,7 @@ static int zforce_probe(struct i2c_client *client,
>  		return ret;
>  	}
> 
> -	/* this gets the firmware version among other informations */
> +	/* this gets the firmware version among other information */
>  	ret = zforce_command_wait(ts, COMMAND_STATUS);
>  	if (ret < 0) {
>  		dev_err(&client->dev, "couldn't get status, %d\n", ret);


^ permalink raw reply

* [PATCH 1/4] Input: zforce - fix spelling errors
From: Luis Ortega @ 2014-01-27 18:46 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: heiko, yongjun_wei, linux-input, linux-kernel, Luis Ortega
In-Reply-To: <1390848373-7723-1-git-send-email-luiorpe1@upv.es>

Fixed a few spelling errors.

Signed-off-by: Luis Ortega <luiorpe1@upv.es>
---
 drivers/input/touchscreen/zforce_ts.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index 2175f34..c145c1d 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -64,7 +64,7 @@
 #define RESPONSE_STATUS		0X1e
 
 /*
- * Notifications are send by the touch controller without
+ * Notifications are sent by the touch controller without
  * being requested by the driver and include for example
  * touch indications
  */
@@ -103,8 +103,8 @@ struct zforce_point {
  * @suspended		device suspended
  * @access_mutex	serialize i2c-access, to keep multipart reads together
  * @command_done	completion to wait for the command result
- * @command_mutex	serialize commands send to the ic
- * @command_waiting	the id of the command that that is currently waiting
+ * @command_mutex	serialize commands sent to the ic
+ * @command_waiting	the id of the command that is currently waiting
  *			for a result
  * @command_result	returned result of the command
  */
@@ -332,7 +332,7 @@ static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
 
 	count = payload[0];
 	if (count > ZFORCE_REPORT_POINTS) {
-		dev_warn(&client->dev, "to many coordinates %d, expected max %d\n",
+		dev_warn(&client->dev, "too many coordinates %d, expected max %d\n",
 			 count, ZFORCE_REPORT_POINTS);
 		count = ZFORCE_REPORT_POINTS;
 	}
@@ -798,7 +798,7 @@ static int zforce_probe(struct i2c_client *client,
 		return ret;
 	}
 
-	/* this gets the firmware version among other informations */
+	/* this gets the firmware version among other information */
 	ret = zforce_command_wait(ts, COMMAND_STATUS);
 	if (ret < 0) {
 		dev_err(&client->dev, "couldn't get status, %d\n", ret);
-- 
1.8.5.3

^ permalink raw reply related

* [PATCH 2/4] Input: zforce - fix lines exceeding 80 columns
From: Luis Ortega @ 2014-01-27 18:46 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: heiko, yongjun_wei, linux-input, linux-kernel, Luis Ortega
In-Reply-To: <1390848373-7723-1-git-send-email-luiorpe1@upv.es>

Fixed lines exceeding 80 characters long wherever possible,
as per the coding style.

Signed-off-by: Luis Ortega <luiorpe1@upv.es>
---
 drivers/input/touchscreen/zforce_ts.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index c145c1d..c1b6b82 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -235,7 +235,8 @@ static int zforce_scan_frequency(struct zforce_ts *ts, u16 idle, u16 finger,
 			(finger & 0xff), ((finger >> 8) & 0xff),
 			(stylus & 0xff), ((stylus >> 8) & 0xff) };
 
-	dev_dbg(&client->dev, "set scan frequency to (idle: %d, finger: %d, stylus: %d)\n",
+	dev_dbg(&client->dev,
+		"set scan frequency to (idle: %d, finger: %d, stylus: %d)\n",
 		idle, finger, stylus);
 
 	return zforce_send_wait(ts, &buf[0], ARRAY_SIZE(buf));
@@ -332,7 +333,8 @@ static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
 
 	count = payload[0];
 	if (count > ZFORCE_REPORT_POINTS) {
-		dev_warn(&client->dev, "too many coordinates %d, expected max %d\n",
+		dev_warn(&client->dev,
+			 "too many coordinates %d, expected max %d\n",
 			 count, ZFORCE_REPORT_POINTS);
 		count = ZFORCE_REPORT_POINTS;
 	}
@@ -494,8 +496,8 @@ static irqreturn_t zforce_irq_thread(int irq, void *dev_id)
 	while (!gpio_get_value(pdata->gpio_int)) {
 		ret = zforce_read_packet(ts, payload_buffer);
 		if (ret < 0) {
-			dev_err(&client->dev, "could not read packet, ret: %d\n",
-				ret);
+			dev_err(&client->dev,
+				"could not read packet, ret: %d\n", ret);
 			break;
 		}
 
@@ -539,7 +541,8 @@ static irqreturn_t zforce_irq_thread(int irq, void *dev_id)
 						payload[RESPONSE_DATA + 4];
 			ts->version_rev   = (payload[RESPONSE_DATA + 7] << 8) |
 						payload[RESPONSE_DATA + 6];
-			dev_dbg(&ts->client->dev, "Firmware Version %04x:%04x %04x:%04x\n",
+			dev_dbg(&ts->client->dev,
+				"Firmware Version %04x:%04x %04x:%04x\n",
 				ts->version_major, ts->version_minor,
 				ts->version_build, ts->version_rev);
 
@@ -552,7 +555,8 @@ static irqreturn_t zforce_irq_thread(int irq, void *dev_id)
 			break;
 
 		default:
-			dev_err(&ts->client->dev, "unrecognized response id: 0x%x\n",
+			dev_err(&ts->client->dev,
+				"unrecognized response id: 0x%x\n",
 				payload[RESPONSE_ID]);
 			break;
 		}
@@ -618,7 +622,8 @@ static int zforce_suspend(struct device *dev)
 
 		enable_irq_wake(client->irq);
 	} else if (input->users) {
-		dev_dbg(&client->dev, "suspend without being a wakeup source\n");
+		dev_dbg(&client->dev,
+			"suspend without being a wakeup source\n");
 
 		ret = zforce_stop(ts);
 		if (ret)
-- 
1.8.5.3

^ permalink raw reply related

* [PATCH 3/4] Input: zforce - Remove unnecessary payload data checks
From: Luis Ortega @ 2014-01-27 18:46 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: heiko, yongjun_wei, linux-input, linux-kernel, Luis Ortega
In-Reply-To: <1390848373-7723-1-git-send-email-luiorpe1@upv.es>

The function zforce_read_packet() reads 2 values (bytes) of payload
header, validates them and then proceeds to read the payload body.
The function stores all these in a u8 buffer.

The PAYLOAD_LENGTH check seems to be trying to detect an overflow error.
However, since we are just reading a u8 value from the buffer, these
checks are unnecessary and we should simply compare against zero.

Signed-off-by: Luis Ortega <luiorpe1@upv.es>
---
 drivers/input/touchscreen/zforce_ts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index c1b6b82..e082d5c 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -423,7 +423,7 @@ static int zforce_read_packet(struct zforce_ts *ts, u8 *buf)
 		goto unlock;
 	}
 
-	if (buf[PAYLOAD_LENGTH] <= 0 || buf[PAYLOAD_LENGTH] > 255) {
+	if (buf[PAYLOAD_LENGTH] == 0) {
 		dev_err(&client->dev, "invalid payload length: %d\n",
 			buf[PAYLOAD_LENGTH]);
 		ret = -EIO;
-- 
1.8.5.3

^ permalink raw reply related

* [RFC][PATCH] module: allow multiple calls to MODULE_DEVICE_TABLE() per module
From: Tom Gundersen @ 2014-01-27 19:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-input, kay, Tom Gundersen, Dmitry Torokhov,
	Greg Kroah-Hartman, Rusty Russell

Commit 78551277e4df5: "Input: i8042 - add PNP modaliases" had a bug, where the
second call to MODULE_DEVICE_TABLE() overrode the first resulting in not all
the modaliases being exposed.

This fixes the problem by including the name of the device_id table in the
__mod_*_device_table alias, allowing us to export several device_id tables
per module.

Suggested-by: Kay Sievers <kay@vrfy.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>
---
 include/linux/module.h   |  2 +-
 scripts/mod/file2alias.c | 14 +++++++++-----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 15cd6b1..7732d76 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -143,7 +143,7 @@ extern const struct gtype##_id __mod_##gtype##_table		\
 #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
 
 #define MODULE_DEVICE_TABLE(type,name)		\
-  MODULE_GENERIC_TABLE(type##_device,name)
+  MODULE_GENERIC_TABLE(type##__##name##_device,name)
 
 /* Version of form [<epoch>:]<version>[-<extra-version>].
    Or for CVS/RCS ID version, everything but the number is stripped.
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 2370863..6778381 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -42,7 +42,7 @@ typedef unsigned char	__u8;
 
 /* This array collects all instances that use the generic do_table */
 struct devtable {
-	const char *device_id; /* name of table, __mod_<name>_device_table. */
+	const char *device_id; /* name of table, __mod_<name>__*_device_table. */
 	unsigned long id_size;
 	void *function;
 };
@@ -146,7 +146,8 @@ static void device_id_check(const char *modname, const char *device_id,
 
 	if (size % id_size || size < id_size) {
 		fatal("%s: sizeof(struct %s_device_id)=%lu is not a modulo "
-		      "of the size of section __mod_%s_device_table=%lu.\n"
+		      "of the size of "
+		      "section __mod_%s__<identifier>_device_table=%lu.\n"
 		      "Fix definition of struct %s_device_id "
 		      "in mod_devicetable.h\n",
 		      modname, device_id, id_size, device_id, size, device_id);
@@ -1206,7 +1207,7 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
 {
 	void *symval;
 	char *zeros = NULL;
-	const char *name;
+	const char *name, *identifier;
 	unsigned int namelen;
 
 	/* We're looking for a section relative symbol */
@@ -1217,7 +1218,7 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
 	if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
 		return;
 
-	/* All our symbols are of form <prefix>__mod_XXX_device_table. */
+	/* All our symbols are of form <prefix>__mod_<name>__<identifier>_device_table. */
 	name = strstr(symname, "__mod_");
 	if (!name)
 		return;
@@ -1227,7 +1228,10 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
 		return;
 	if (strcmp(name + namelen - strlen("_device_table"), "_device_table"))
 		return;
-	namelen -= strlen("_device_table");
+	identifier = strstr(name, "__");
+	if (!identifier)
+		return;
+	namelen = identifier - name;
 
 	/* Handle all-NULL symbols allocated into .bss */
 	if (info->sechdrs[get_secindex(info, sym)].sh_type & SHT_NOBITS) {
-- 
1.8.5.3


^ permalink raw reply related

* Re: [PATCH 2/4] Input: zforce - fix lines exceeding 80 columns
From: Heiko Stübner @ 2014-01-27 19:12 UTC (permalink / raw)
  To: Luis Ortega; +Cc: dmitry.torokhov, yongjun_wei, linux-input, linux-kernel
In-Reply-To: <1390848373-7723-3-git-send-email-luiorpe1@upv.es>

On Monday, 27. January 2014 19:46:11 Luis Ortega wrote:
> Fixed lines exceeding 80 characters long wherever possible,
> as per the coding style.
> 
> Signed-off-by: Luis Ortega <luiorpe1@upv.es>

checkpatch did not complain on the initial submission, so I guess it's more an 
issue of taste. Nevertheless I don't have a preference for one way or the 
other, so

Acked-by: Heiko Stuebner <heiko@sntech.de>


> ---
>  drivers/input/touchscreen/zforce_ts.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/zforce_ts.c
> b/drivers/input/touchscreen/zforce_ts.c index c145c1d..c1b6b82 100644
> --- a/drivers/input/touchscreen/zforce_ts.c
> +++ b/drivers/input/touchscreen/zforce_ts.c
> @@ -235,7 +235,8 @@ static int zforce_scan_frequency(struct zforce_ts *ts,
> u16 idle, u16 finger, (finger & 0xff), ((finger >> 8) & 0xff),
>  			(stylus & 0xff), ((stylus >> 8) & 0xff) };
> 
> -	dev_dbg(&client->dev, "set scan frequency to (idle: %d, finger: %d,
> stylus: %d)\n", +	dev_dbg(&client->dev,
> +		"set scan frequency to (idle: %d, finger: %d, stylus: %d)\n",
>  		idle, finger, stylus);
> 
>  	return zforce_send_wait(ts, &buf[0], ARRAY_SIZE(buf));
> @@ -332,7 +333,8 @@ static int zforce_touch_event(struct zforce_ts *ts, u8
> *payload)
> 
>  	count = payload[0];
>  	if (count > ZFORCE_REPORT_POINTS) {
> -		dev_warn(&client->dev, "too many coordinates %d, expected max %d\n",
> +		dev_warn(&client->dev,
> +			 "too many coordinates %d, expected max %d\n",
>  			 count, ZFORCE_REPORT_POINTS);
>  		count = ZFORCE_REPORT_POINTS;
>  	}
> @@ -494,8 +496,8 @@ static irqreturn_t zforce_irq_thread(int irq, void
> *dev_id) while (!gpio_get_value(pdata->gpio_int)) {
>  		ret = zforce_read_packet(ts, payload_buffer);
>  		if (ret < 0) {
> -			dev_err(&client->dev, "could not read packet, ret: %d\n",
> -				ret);
> +			dev_err(&client->dev,
> +				"could not read packet, ret: %d\n", ret);
>  			break;
>  		}
> 
> @@ -539,7 +541,8 @@ static irqreturn_t zforce_irq_thread(int irq, void
> *dev_id) payload[RESPONSE_DATA + 4];
>  			ts->version_rev   = (payload[RESPONSE_DATA + 7] << 8) |
>  						payload[RESPONSE_DATA + 6];
> -			dev_dbg(&ts->client->dev, "Firmware Version %04x:%04x %04x:
%04x\n",
> +			dev_dbg(&ts->client->dev,
> +				"Firmware Version %04x:%04x %04x:%04x\n",
>  				ts->version_major, ts->version_minor,
>  				ts->version_build, ts->version_rev);
> 
> @@ -552,7 +555,8 @@ static irqreturn_t zforce_irq_thread(int irq, void
> *dev_id) break;
> 
>  		default:
> -			dev_err(&ts->client->dev, "unrecognized response id: 0x%x\n",
> +			dev_err(&ts->client->dev,
> +				"unrecognized response id: 0x%x\n",
>  				payload[RESPONSE_ID]);
>  			break;
>  		}
> @@ -618,7 +622,8 @@ static int zforce_suspend(struct device *dev)
> 
>  		enable_irq_wake(client->irq);
>  	} else if (input->users) {
> -		dev_dbg(&client->dev, "suspend without being a wakeup source\n");
> +		dev_dbg(&client->dev,
> +			"suspend without being a wakeup source\n");
> 
>  		ret = zforce_stop(ts);
>  		if (ret)


^ permalink raw reply

* Re: [PATCH 2/4] Input: zforce - fix lines exceeding 80 columns
From: Luis Ortega @ 2014-01-27 19:29 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: Luis Ortega, Dmitry Torokhov, yongjun_wei, linux-input,
	linux-kernel
In-Reply-To: <1864335.qYXGSfrppl@phil>

Yeah, I also ran checkpatch on the file and it had nothing to report.
Unfortunately those lines are slightly long and got word wrapped in my
editor, disrupting the flow of text. I thought fixing them wouldn't hurt.

On Mon, Jan 27, 2014 at 8:12 PM, Heiko Stübner <heiko@sntech.de> wrote:
> On Monday, 27. January 2014 19:46:11 Luis Ortega wrote:
>> Fixed lines exceeding 80 characters long wherever possible,
>> as per the coding style.
>>
>> Signed-off-by: Luis Ortega <luiorpe1@upv.es>
>
> checkpatch did not complain on the initial submission, so I guess it's more an
> issue of taste. Nevertheless I don't have a preference for one way or the
> other, so
>
> Acked-by: Heiko Stuebner <heiko@sntech.de>
>
>
>> ---
>>  drivers/input/touchscreen/zforce_ts.c | 19 ++++++++++++-------
>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/zforce_ts.c
>> b/drivers/input/touchscreen/zforce_ts.c index c145c1d..c1b6b82 100644
>> --- a/drivers/input/touchscreen/zforce_ts.c
>> +++ b/drivers/input/touchscreen/zforce_ts.c
>> @@ -235,7 +235,8 @@ static int zforce_scan_frequency(struct zforce_ts *ts,
>> u16 idle, u16 finger, (finger & 0xff), ((finger >> 8) & 0xff),
>>                       (stylus & 0xff), ((stylus >> 8) & 0xff) };
>>
>> -     dev_dbg(&client->dev, "set scan frequency to (idle: %d, finger: %d,
>> stylus: %d)\n", +     dev_dbg(&client->dev,
>> +             "set scan frequency to (idle: %d, finger: %d, stylus: %d)\n",
>>               idle, finger, stylus);
>>
>>       return zforce_send_wait(ts, &buf[0], ARRAY_SIZE(buf));
>> @@ -332,7 +333,8 @@ static int zforce_touch_event(struct zforce_ts *ts, u8
>> *payload)
>>
>>       count = payload[0];
>>       if (count > ZFORCE_REPORT_POINTS) {
>> -             dev_warn(&client->dev, "too many coordinates %d, expected max %d\n",
>> +             dev_warn(&client->dev,
>> +                      "too many coordinates %d, expected max %d\n",
>>                        count, ZFORCE_REPORT_POINTS);
>>               count = ZFORCE_REPORT_POINTS;
>>       }
>> @@ -494,8 +496,8 @@ static irqreturn_t zforce_irq_thread(int irq, void
>> *dev_id) while (!gpio_get_value(pdata->gpio_int)) {
>>               ret = zforce_read_packet(ts, payload_buffer);
>>               if (ret < 0) {
>> -                     dev_err(&client->dev, "could not read packet, ret: %d\n",
>> -                             ret);
>> +                     dev_err(&client->dev,
>> +                             "could not read packet, ret: %d\n", ret);
>>                       break;
>>               }
>>
>> @@ -539,7 +541,8 @@ static irqreturn_t zforce_irq_thread(int irq, void
>> *dev_id) payload[RESPONSE_DATA + 4];
>>                       ts->version_rev   = (payload[RESPONSE_DATA + 7] << 8) |
>>                                               payload[RESPONSE_DATA + 6];
>> -                     dev_dbg(&ts->client->dev, "Firmware Version %04x:%04x %04x:
> %04x\n",
>> +                     dev_dbg(&ts->client->dev,
>> +                             "Firmware Version %04x:%04x %04x:%04x\n",
>>                               ts->version_major, ts->version_minor,
>>                               ts->version_build, ts->version_rev);
>>
>> @@ -552,7 +555,8 @@ static irqreturn_t zforce_irq_thread(int irq, void
>> *dev_id) break;
>>
>>               default:
>> -                     dev_err(&ts->client->dev, "unrecognized response id: 0x%x\n",
>> +                     dev_err(&ts->client->dev,
>> +                             "unrecognized response id: 0x%x\n",
>>                               payload[RESPONSE_ID]);
>>                       break;
>>               }
>> @@ -618,7 +622,8 @@ static int zforce_suspend(struct device *dev)
>>
>>               enable_irq_wake(client->irq);
>>       } else if (input->users) {
>> -             dev_dbg(&client->dev, "suspend without being a wakeup source\n");
>> +             dev_dbg(&client->dev,
>> +                     "suspend without being a wakeup source\n");
>>
>>               ret = zforce_stop(ts);
>>               if (ret)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* HID_REQ_GET_REPORT and report not filled during hid_pidff_init
From: Lauri Peltonen @ 2014-01-27 19:31 UTC (permalink / raw)
  To: linux-input; +Cc: jkosina

Hi.

I'm making some enhancements to hid/usbhid/hid-pidff.c to support more
force feedback devices, but I've encountered some weird behaviour. I'm
not sure whether it's a bug or something else.

My changes depend on knowing some device information (like memory) in
the initialization phase, and luckily hid-pidff reads the info from
PID_POOL report in hid_pidff_init -> pidff_reset. [1] and [2]

However, I noticed that the report struct never gets filled (all
zeros), even though with usbmon I can see the ctrl message out and the
report coming in with correct data (300 bytes of memory etc). I
managed to locate the line which causes the report to be dropped in
hid/hid-core.c , in hid_input_repor(..)t checking for
driver_input_lock. [3] Commenting out the following lines "fixes" the
issue, and the struct gets filled correctly. Anyways, I think that is
not a real solution.

if (down_trylock(&hid->driver_input_lock))
 return -EBUSY;

Now, I wonder if there is a reason why the locking fails? Is the
driver locked during the initialization and hid_hw_request(...
HID_REQ_GET_REPORT) cannot be used? If that is so, what is the correct
way to read device reports during the init? And does the hid-pidff
driver ever read the pool report correctly, or is this just with my
Saitek device?

I'm using kernel 3.13.

Best regards, Lauri Peltonen

[1] http://lxr.free-electrons.com/source/drivers/hid/usbhid/hid-pidff.c#L1268
[2] http://lxr.free-electrons.com/source/drivers/hid/usbhid/hid-pidff.c#L1172
[3] http://lxr.free-electrons.com/source/drivers/hid/hid-core.c#L1392

-- 
Lauri Peltonen
lauri.peltonen@gmail.com

^ permalink raw reply

* Re: [RFC][PATCH] module: allow multiple calls to MODULE_DEVICE_TABLE() per module
From: Greg Kroah-Hartman @ 2014-01-27 19:54 UTC (permalink / raw)
  To: Tom Gundersen
  Cc: linux-kernel, linux-input, kay, Dmitry Torokhov, Rusty Russell
In-Reply-To: <1390849795-2155-1-git-send-email-teg@jklm.no>

On Mon, Jan 27, 2014 at 08:09:55PM +0100, Tom Gundersen wrote:
> Commit 78551277e4df5: "Input: i8042 - add PNP modaliases" had a bug, where the
> second call to MODULE_DEVICE_TABLE() overrode the first resulting in not all
> the modaliases being exposed.
> 
> This fixes the problem by including the name of the device_id table in the
> __mod_*_device_table alias, allowing us to export several device_id tables
> per module.
> 
> Suggested-by: Kay Sievers <kay@vrfy.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> ---
>  include/linux/module.h   |  2 +-
>  scripts/mod/file2alias.c | 14 +++++++++-----
>  2 files changed, 10 insertions(+), 6 deletions(-)

Ah, very nice, I've wanted this for a while now, it would make a number
of different drivers much smaller and simpler to add new device ids to
(no multiple lists of ids, one for the module loader and one for the
sub-driver that is in the single file.)

If this doesn't break any userspace tools, I have no objection to it at
all:

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply

* [PATCH] USB: input: gtco.c: fix usb_dev leak
From: Alexey Khoroshilov @ 2014-01-27 20:10 UTC (permalink / raw)
  To: Dmitry Torokhov, Greg Kroah-Hartman
  Cc: Alexey Khoroshilov, linux-input, linux-kernel, ldv-project

There is usb_get_dev() in gtco_probe(), but there is no usb_put_dev()
anywhere in the driver.

As pointed out by Dmitry Torokhov:
The lifetime of gtco structure is already directly tied to lifetime of
usb_dev: when destroying usb_dev driver core will call remove() function
of currently bound driver (in our case gtco) which will destroy gtco
memory. Taking additional reference is not needed here.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/input/tablet/gtco.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/tablet/gtco.c b/drivers/input/tablet/gtco.c
index caecffe8caff..858045694e9d 100644
--- a/drivers/input/tablet/gtco.c
+++ b/drivers/input/tablet/gtco.c
@@ -848,7 +848,7 @@ static int gtco_probe(struct usb_interface *usbinterface,
 	gtco->inputdevice = input_dev;
 
 	/* Save interface information */
-	gtco->usbdev = usb_get_dev(interface_to_usbdev(usbinterface));
+	gtco->usbdev = interface_to_usbdev(usbinterface);
 	gtco->intf = usbinterface;
 
 	/* Allocate some data for incoming reports */
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH 3/4] Input: zforce - Remove unnecessary payload data checks
From: Heiko Stübner @ 2014-01-27 20:16 UTC (permalink / raw)
  To: Luis Ortega; +Cc: dmitry.torokhov, yongjun_wei, linux-input, linux-kernel
In-Reply-To: <1390848373-7723-4-git-send-email-luiorpe1@upv.es>

On Monday, 27. January 2014 19:46:12 Luis Ortega wrote:
> The function zforce_read_packet() reads 2 values (bytes) of payload
> header, validates them and then proceeds to read the payload body.
> The function stores all these in a u8 buffer.
> 
> The PAYLOAD_LENGTH check seems to be trying to detect an overflow error.
> However, since we are just reading a u8 value from the buffer, these
> checks are unnecessary and we should simply compare against zero.
> 
> Signed-off-by: Luis Ortega <luiorpe1@upv.es>

Acked-by: Heiko Stuebner <heiko@sntech.de>

on a bq Cervantes (imx6sl)
Tested-by: Heiko Stuebner <heiko@sntech.de>


> ---
>  drivers/input/touchscreen/zforce_ts.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/zforce_ts.c
> b/drivers/input/touchscreen/zforce_ts.c index c1b6b82..e082d5c 100644
> --- a/drivers/input/touchscreen/zforce_ts.c
> +++ b/drivers/input/touchscreen/zforce_ts.c
> @@ -423,7 +423,7 @@ static int zforce_read_packet(struct zforce_ts *ts, u8
> *buf) goto unlock;
>  	}
> 
> -	if (buf[PAYLOAD_LENGTH] <= 0 || buf[PAYLOAD_LENGTH] > 255) {
> +	if (buf[PAYLOAD_LENGTH] == 0) {
>  		dev_err(&client->dev, "invalid payload length: %d\n",
>  			buf[PAYLOAD_LENGTH]);
>  		ret = -EIO;


^ permalink raw reply

* Re: [PATCH 4/4] Input: zforce - reduce stack memory allocated to frames
From: Heiko Stübner @ 2014-01-27 20:22 UTC (permalink / raw)
  To: Luis Ortega; +Cc: dmitry.torokhov, yongjun_wei, linux-input, linux-kernel
In-Reply-To: <1390848373-7723-5-git-send-email-luiorpe1@upv.es>

On Monday, 27. January 2014 19:46:13 Luis Ortega wrote:
> A frame is a u8 array with the following structure:
> [PAYLOAD_HEADER, PAYLOAD_LENGTH, ...PAYLOAD_BODY...]
> 
> PAYLOAD_BODY can be at most 255 bytes long, as it's size is represented
> by PAYLOAD_LENGTH. Therefore we can reduce the stack memory allocated to
> payload_buffer[] roughly by half, from 512 to 257 bytes.

Nice catch, thanks

Acked-by: Heiko Stuebner <heiko@sntech.de>

on a bq Cervantes (imx6sl)
Tested-by: Heiko Stuebner <heiko@sntech.de>


As a side-note, this change conflicts with one of my patches adding devicetree 
support to the driver [0], which Dmitry will hopefully also look at soon.

So one of us might need to respin his series depending on the ordering.



[0] http://permalink.gmane.org/gmane.linux.kernel.input/33587




> 
> Signed-off-by: Luis Ortega <luiorpe1@upv.es>
> ---
>  drivers/input/touchscreen/zforce_ts.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/zforce_ts.c
> b/drivers/input/touchscreen/zforce_ts.c index e082d5c..afb2492 100644
> --- a/drivers/input/touchscreen/zforce_ts.c
> +++ b/drivers/input/touchscreen/zforce_ts.c
> @@ -33,6 +33,7 @@
>  #define WAIT_TIMEOUT		msecs_to_jiffies(1000)
> 
>  #define FRAME_START		0xee
> +#define FRAME_MAXSIZE		257
> 
>  /* Offsets of the different parts of the payload the controller sends */
>  #define PAYLOAD_HEADER		0
> @@ -475,7 +476,7 @@ static irqreturn_t zforce_irq_thread(int irq, void
> *dev_id) struct i2c_client *client = ts->client;
>  	const struct zforce_ts_platdata *pdata = dev_get_platdata(&client->dev);
>  	int ret;
> -	u8 payload_buffer[512];
> +	u8 payload_buffer[FRAME_MAXSIZE];
>  	u8 *payload;
> 
>  	/*


^ permalink raw reply

* Re: [PATCH] USB: input: gtco.c: fix usb_dev leak
From: Dmitry Torokhov @ 2014-01-27 20:24 UTC (permalink / raw)
  To: Alexey Khoroshilov
  Cc: Greg Kroah-Hartman, linux-input, linux-kernel, ldv-project
In-Reply-To: <1390853454-31749-1-git-send-email-khoroshilov@ispras.ru>

On Tue, Jan 28, 2014 at 12:10:54AM +0400, Alexey Khoroshilov wrote:
> There is usb_get_dev() in gtco_probe(), but there is no usb_put_dev()
> anywhere in the driver.
> 
> As pointed out by Dmitry Torokhov:
> The lifetime of gtco structure is already directly tied to lifetime of
> usb_dev: when destroying usb_dev driver core will call remove() function
> of currently bound driver (in our case gtco) which will destroy gtco
> memory. Taking additional reference is not needed here.
> 
> Found by Linux Driver Verification project (linuxtesting.org).
> 
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>

Applied, thank you.

> ---
>  drivers/input/tablet/gtco.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/tablet/gtco.c b/drivers/input/tablet/gtco.c
> index caecffe8caff..858045694e9d 100644
> --- a/drivers/input/tablet/gtco.c
> +++ b/drivers/input/tablet/gtco.c
> @@ -848,7 +848,7 @@ static int gtco_probe(struct usb_interface *usbinterface,
>  	gtco->inputdevice = input_dev;
>  
>  	/* Save interface information */
> -	gtco->usbdev = usb_get_dev(interface_to_usbdev(usbinterface));
> +	gtco->usbdev = interface_to_usbdev(usbinterface);
>  	gtco->intf = usbinterface;
>  
>  	/* Allocate some data for incoming reports */
> -- 
> 1.8.3.2
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: i8042-io - Exclude mips platforms when allocating/deallocating IO regions.
From: Aaro Koskinen @ 2014-01-27 20:24 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Raghu Gandham, linux-input@vger.kernel.org,
	linux-mips@linux-mips.org, linux-kernel@vger.kernel.org
In-Reply-To: <20140127065638.GB11945@core.coreip.homeip.net>

Hi,

On Sun, Jan 26, 2014 at 10:56:38PM -0800, Dmitry Torokhov wrote:
> On Mon, Jan 27, 2014 at 12:32:36AM +0000, Raghu Gandham wrote:
> > > On Sat, Jan 25, 2014 at 11:01:54AM -0800, Raghu Gandham wrote:
> > > > The standard IO regions are already reserved by the platform code on
> > > > most MIPS devices(malta, cobalt, sni). The Commit
> > > > 197a1e96c8be5b6005145af3a4c0e45e2d651444
> > > > ("Input: i8042-io - fix up region handling on MIPS") introduced a bug
> > > > on these MIPS platforms causing i8042 driver to fail when trying to
> > > > reserve IO ports.
> > > > Prior to the above mentioned commit request_region is skipped on MIPS
> > > > but release_region is called.
> > > >
> > > > This patch reverts commit 197a1e96c8be5b6005145af3a4c0e45e2d651444
> > > > and also avoids calling release_region for MIPS.
> > > 
> > > The problem is that IO regions are reserved on _most_, but not _all_
> > > devices.
> > > MIPS should figure out what they want to do with i8042 registers and be
> > > consistent on all devices.
> > 
> > Please examine the attached patch which went upstream in April of 2004.
> > Since then it had been a convention not to call request_region routine in
> > i8042 for MIPS. The attached patch had a glitch that it guarded
> > request_region in i8042-io.h but skipped guarding release_region in
> > i8042-io.h. I believe that the issue Aaro saw was due to this 
> > glitch. Below is the error quoted in Aaro's commit message.
> > 
> >     [    2.112000] Trying to free nonexistent resource <0000000000000060-000000000000006f>
> > 
> > My patch reinstates the convention followed on MIPS devices along with
> > fixing Aaro's issue.
> 
> I assume that Aaro did test his patch and on his box request_region()
> succeeds. That would indicate that various MIPS sub-arches still not
> settled on the topic.

request_region() succeeds on Loongson and OCTEON.

On OCTEONs without PCI, request_region() will fail which is correct as
there is no I/O space.

I wasn't aware of that 2004 patch (it pre-dates GIT history of mainline
Linux). Why the regions are already reserved by the platform code?

A.

^ permalink raw reply

* Re: [RFC][PATCH] module: allow multiple calls to MODULE_DEVICE_TABLE() per module
From: Dmitry Torokhov @ 2014-01-27 20:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Tom Gundersen, linux-kernel, linux-input, kay, Rusty Russell
In-Reply-To: <20140127195422.GB21018@kroah.com>

On Mon, Jan 27, 2014 at 11:54:22AM -0800, Greg Kroah-Hartman wrote:
> On Mon, Jan 27, 2014 at 08:09:55PM +0100, Tom Gundersen wrote:
> > Commit 78551277e4df5: "Input: i8042 - add PNP modaliases" had a bug, where the
> > second call to MODULE_DEVICE_TABLE() overrode the first resulting in not all
> > the modaliases being exposed.
> > 
> > This fixes the problem by including the name of the device_id table in the
> > __mod_*_device_table alias, allowing us to export several device_id tables
> > per module.
> > 
> > Suggested-by: Kay Sievers <kay@vrfy.org>
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > ---
> >  include/linux/module.h   |  2 +-
> >  scripts/mod/file2alias.c | 14 +++++++++-----
> >  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> Ah, very nice, I've wanted this for a while now, it would make a number
> of different drivers much smaller and simpler to add new device ids to
> (no multiple lists of ids, one for the module loader and one for the
> sub-driver that is in the single file.)
> 
> If this doesn't break any userspace tools, I have no objection to it at
> all:
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Looks great to me as long as module tools keep working.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 2/2] HID: sony: Add output events for the multi-touch pad on the Dualshock 4.
From: simon @ 2014-01-27 20:40 UTC (permalink / raw)
  Cc: linux-input, jkosina, Frank Praznik
In-Reply-To: <1390835857-4523-2-git-send-email-frank.praznik@oh.rr.com>


> +	for (n = 0; n < 2; n++) {
> +		__u16 x, y;
> +
> +		x = rd[offset+1] | ((rd[offset+2] & 0xF) << 8);
> +		y = ((rd[offset+2] & 0xF0) >> 4) | (rd[offset+3] << 4);
> +
> +		input_mt_slot(input_dev, n);
> +		input_mt_report_slot_state(input_dev, MT_TOOL_FINGER,
> +					!(rd[offset] >> 7));
> +		input_report_abs(input_dev, ABS_MT_POSITION_X, x);
> +		input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
> +
> +		offset += 4;
> +	}

Sorry I have another point/question....

Doesn't this 'spam' the Multitouch interface every incoming report,
regardless on whether anything has actually change? ie. Should HID-Sony
track the values, and only send when it detects a change...

Simon


^ permalink raw reply

* Re: [PATCH v2 2/2] HID: sony: Add output events for the multi-touch pad on the Dualshock 4.
From: Frank Praznik @ 2014-01-27 21:02 UTC (permalink / raw)
  To: simon, Frank Praznik; +Cc: linux-input, jkosina
In-Reply-To: <d3172725c00c3ddcc924d036820b7f5c.squirrel@mungewell.org>

On 1/27/2014 15:40, simon@mungewell.org wrote:
>> +	for (n = 0; n < 2; n++) {
>> +		__u16 x, y;
>> +
>> +		x = rd[offset+1] | ((rd[offset+2] & 0xF) << 8);
>> +		y = ((rd[offset+2] & 0xF0) >> 4) | (rd[offset+3] << 4);
>> +
>> +		input_mt_slot(input_dev, n);
>> +		input_mt_report_slot_state(input_dev, MT_TOOL_FINGER,
>> +					!(rd[offset] >> 7));
>> +		input_report_abs(input_dev, ABS_MT_POSITION_X, x);
>> +		input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
>> +
>> +		offset += 4;
>> +	}
> Sorry I have another point/question....
>
> Doesn't this 'spam' the Multitouch interface every incoming report,
> regardless on whether anything has actually change? ie. Should HID-Sony
> track the values, and only send when it detects a change...
>
> Simon
>
No, the HID layer tracks the current state and only sends events when 
something changes so there is no need to shadow it again.  If you watch 
the output in evtest, events are only sent when something changes.  All 
this does is manually parse the touch data for the HID layer.

^ permalink raw reply

* Re: [PATCH v2 2/2] HID: sony: Add output events for the multi-touch pad on the Dualshock 4.
From: Frank Praznik @ 2014-01-27 21:28 UTC (permalink / raw)
  To: simon, Frank Praznik; +Cc: linux-input, jkosina
In-Reply-To: <ddf2398947f6323430699b45bc509091.squirrel@mungewell.org>

On 1/27/2014 12:47, simon@mungewell.org wrote:
>> +
>> +	/* The Dualshock 4 multi-touch trackpad data starts at offset 35 on USB.
>> +	 * The first 7 bits of the first byte is a counter and bit 8 is a touch
>> +	 * indicator that is 0 when pressed and 1 when not pressed.
>> +	 * The next 3 bytes are two 12 bit touch coordinates, X and Y.
>> +	 * The data for the second touch is in the same format and immediatly
>> +	 * follows the data for the first.
>> +	 */
> Hi Frank,
> I don't know if it helps multitouch, but there is a second set of data in
> the HID stream, that is present when a figure is moving/swiping on the
> touchpad. This gives us a 'vector' rather than a 'point'.
>
> I previously attempted to describe the format here:
> http://www.spinics.net/lists/linux-input/msg28525.html
>
> Ahead of the 'event counter' is a 'point or vector' byte and what might be
> a 'pressure' measurement (couldn't really tell).
>
> Interspersed with the data you mention is a second set of X1/Y1/X2/Y2
> containing the starting position of the swipe/vector. These are
> differentiated with '02' (rather than '01') in the 34'th byte.
>
> Simon.
>
All the device driver is concerned with is the sending the 'raw' touch 
points to the HID layer (ie. point 1/2 is up/down at position X and Y).  
The HID layer then tracks the IDs for each contact point and sends the 
appropriate events to the user application.  It's then up to the 
application software to track and interpret it.

See this document for information on the multi-touch protocol:
https://www.kernel.org/doc/Documentation/input/multi-touch-protocol.txt

^ permalink raw reply

* [PATCH v2] Input: xpad - do not map the DPAD to buttons with xbox 360 wireless controllers
From: Petr Sebor @ 2014-01-27 22:53 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, Petr Sebor
In-Reply-To: <10f1d8a1b4e46e767197c00c77719f77@mail.scs>

Having the DPAD mapped to buttons makes the wireless gamepad behave
differently from the wired counterpart. Given the MAP_DPAD_TO_BUTTONS
flag is typically used for dance pads, this was probably added by
a mistake. Not specifying the flag makes the controller's hat switch
behave as expected.

Signed-off-by: Petr Sebor <petr@scssoft.com>
---
 drivers/input/joystick/xpad.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 603fe0d..a433636 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -121,8 +121,8 @@ static const struct xpad_device {
 	{ 0x045e, 0x0287, "Microsoft Xbox Controller S", 0, XTYPE_XBOX },
 	{ 0x045e, 0x0289, "Microsoft X-Box pad v2 (US)", 0, XTYPE_XBOX },
 	{ 0x045e, 0x028e, "Microsoft X-Box 360 pad", 0, XTYPE_XBOX360 },
-	{ 0x045e, 0x0291, "Xbox 360 Wireless Receiver (XBOX)", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
-	{ 0x045e, 0x0719, "Xbox 360 Wireless Receiver", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
+	{ 0x045e, 0x0291, "Xbox 360 Wireless Receiver (XBOX)", 0, XTYPE_XBOX360W },
+	{ 0x045e, 0x0719, "Xbox 360 Wireless Receiver", 0, XTYPE_XBOX360W },
 	{ 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
 	{ 0x046d, 0xc21d, "Logitech Gamepad F310", 0, XTYPE_XBOX360 },
 	{ 0x046d, 0xc21f, "Logitech Gamepad F710", 0, XTYPE_XBOX360 },
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH] HID: usbhid: quirk for CY-TM75 75 inch Touch Overlay
From: Yufeng Shen @ 2014-01-27 23:02 UTC (permalink / raw)
  To: linux-input; +Cc: Jiri Kosina, linux-kernel, linux-usb, miletus, adlr

There is timeout error during initialization:
kernel: [   11.733104] hid-multitouch 0003:1870:0110.0001: usb_submit_urb(ctrl) failed: -1
kernel: [   11.734093] hid-multitouch 0003:1870:0110.0001: timeout initializing reports

Adding quirk HID_QUIRK_NO_INIT_REPORTS can solve the problem.

Signed-off-by: Yufeng Shen <miletus@chromium.org>
---
 drivers/hid/hid-ids.h           | 1 +
 drivers/hid/usbhid/hid-quirks.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index f9304cb..ece2997 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -636,6 +636,7 @@
 
 #define USB_VENDOR_ID_NEXIO		0x1870
 #define USB_DEVICE_ID_NEXIO_MULTITOUCH_420	0x010d
+#define USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750	0x0110
 
 #define USB_VENDOR_ID_NEXTWINDOW	0x1926
 #define USB_DEVICE_ID_NEXTWINDOW_TOUCHSCREEN	0x0003
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 0db9a67..be5270a 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -74,6 +74,7 @@ static const struct hid_blacklist {
 	{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
+	{ USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, HID_QUIRK_NO_INIT_REPORTS },
 	{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, HID_QUIRK_NO_INIT_REPORTS },
 	{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN, HID_QUIRK_NO_INIT_REPORTS },
 	{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN1, HID_QUIRK_NO_INIT_REPORTS },
-- 
1.8.5.3

^ permalink raw reply related

* Re: [PATCH v2] input synaptics-rmi4: PDT scan cleanup
From: Christopher Heiny @ 2014-01-28  2:28 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
	Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
	Benjamin Tissoires
In-Reply-To: <20140127063038.GA32605@core.coreip.homeip.net>

On 01/26/2014 10:30 PM, Dmitry Torokhov wrote:> Hi Christopher,

Hi Dmitry,

Sorry if this reply is oddly formatted - I'm having some trouble with my 
Thunderbird settings today.

 >
 > On Wed, Jan 22, 2014 at 04:56:09PM -0800, Christopher Heiny wrote:
 >> Eliminates copy-paste code that handled scans of the Page Descriptor
 >> Table, replacing it with a single PDT scan routine that invokes a
 >> callback function.  The scan routine is not static so that it can be
 >> used by the firmware update code (under development, not yet submitted).
 >>
 >> Symbols that no longer needed to be public were moved into rmi_driver.c.
 >>
 >> Updated the copyright dates and eliminate C++ style comments while we
 >> were at it.
 >>
 >> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
 >
 > I compared the 2 versions that you posted and I realized that losing
 > error codes from lower levels by reducing them to RMI_SCAN_ERROR is not
 > the best option, so I took your first patch as a basis and tried to
 > rework it a bit so that we:
 >
 > - do not have special logic in generic scan routine regarding bootloader
 >    mode,

If an RMI4 device reaches the point of calling rmi_create_functions() 
while still in bootloader mode, it means the main firmware is so broken 
that a reflash is required in order to recover.  To support the reflash, 
you need to scan all of Page 0 and create the associated functions for 
F34 and F01.  But the patch as written will stop creating functions 
after the first one it finds on Page 0, creating one of those, but not 
the other.

Additionally, anything scanning the PDT when in bootloader mode (whether 
it's the IRQ counter, the rmi_create_functions(), or the reflash module) 
needs to stop after scanning Page 0.  I think it's tidier to put that 
check in rmi_scan_pdt_page(), rather than include it in every callback 
implementation.  See annotations below.


 > and
 >
 > - do not leave with mutex locked in case of PDT read error.
 >
 > I also added start page address to PDT structure so we do not have to
 > pass it around so much.

Hmmmm.  Since we're no longer treating struct pdt_entry as a bitmapped 
struct and doing reads directly into it, we should look into whether it 
makes sense to merge struct pdt_entry and struct 
rmi_function_descriptor.  But that's something for later.

 > Please let me know if the patch below makes sense to you (only compile
 > tested, as always).

I had to apply the patch manually to my test tree because that contains 
some other pending changes.  I think that was done correctly, and with 
the changes described below, it works.

					Cheers,
						Chris

 >
 > Thanks!
 >
 > Dmitry
 >
 > Input: input synaptics-rmi4 - PDT scan cleanup
 >
 > From: Christopher Heiny <cheiny@synaptics.com>
 >
 > Eliminates copy-paste code that handled scans of the Page Descriptor 
Table,
 > replacing it with a single PDT scan routine that invokes a callback
 > function.
 > The scan routine is not static so that it can be used by the firmware
 > update
 > code (under development, not yet submitted).
 >
 > Updated the copyright dates while we were at it.
 >
 > Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
 > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
 > --
 >   drivers/input/rmi4/rmi_driver.c |  258
 > ++++++++++++++++++++-------------------
 >   drivers/input/rmi4/rmi_driver.h |    3  2 files changed, 132
 > insertions(+), 129 deletions(-)
 >
 > diff --git a/drivers/input/rmi4/rmi_driver.c
 > b/drivers/input/rmi4/rmi_driver.c
 > index c01a6b8..b9eb8a5 100644
 > --- a/drivers/input/rmi4/rmi_driver.c
 > +++ b/drivers/input/rmi4/rmi_driver.c
 > @@ -1,5 +1,5 @@
 >   /*
 > - * Copyright (c) 2011-2013 Synaptics Incorporated
 > + * Copyright (c) 2011-2014 Synaptics Incorporated
 >    * Copyright (c) 2011 Unixphere
 >    *
 >    * This driver provides the core support for a single RMI4-based 
device.
 > @@ -467,7 +467,8 @@ static int rmi_driver_reset_handler(struct
 > rmi_device *rmi_dev)
 >    * Construct a function's IRQ mask. This should be called once and
 > stored.
 >    */
 >   int rmi_driver_irq_get_mask(struct rmi_device *rmi_dev,
 > -        struct rmi_function *fn) {
 > +                struct rmi_function *fn)
 > +{
 >       int i;
 >       struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
 >   @@ -491,12 +492,13 @@ int rmi_read_pdt_entry(struct rmi_device
 > *rmi_dev, struct pdt_entry *entry,
 >       int error;
 >        error = rmi_read_block(rmi_dev, pdt_address, buf,
 > RMI_PDT_ENTRY_SIZE);
 > -    if (error < 0) {
 > +    if (error) {
 >           dev_err(&rmi_dev->dev, "Read PDT entry at %#06x failed, code:
 > %d.\n",
 >                   pdt_address, error);
 >           return error;
 >       }
 >   +    entry->page_start = pdt_address / RMI4_PAGE_SIZE;
 >       entry->query_base_addr = buf[0];
 >       entry->command_base_addr = buf[1];
 >       entry->control_base_addr = buf[2];
 > @@ -509,27 +511,115 @@ int rmi_read_pdt_entry(struct rmi_device
 > *rmi_dev, struct pdt_entry *entry,
 >   }
 >   EXPORT_SYMBOL_GPL(rmi_read_pdt_entry);
 >   -static void rmi_driver_copy_pdt_to_fd(struct pdt_entry *pdt,
 > -                      struct rmi_function_descriptor *fd,
 > -                      u16 page_start)
 > +static void rmi_driver_copy_pdt_to_fd(const struct pdt_entry *pdt,
 > +                      struct rmi_function_descriptor *fd)
 >   {
 > -    fd->query_base_addr = pdt->query_base_addr + page_start;
 > -    fd->command_base_addr = pdt->command_base_addr + page_start;
 > -    fd->control_base_addr = pdt->control_base_addr + page_start;
 > -    fd->data_base_addr = pdt->data_base_addr + page_start;
 > +    fd->query_base_addr = pdt->query_base_addr + pdt->page_start;
 > +    fd->command_base_addr = pdt->command_base_addr + pdt->page_start;
 > +    fd->control_base_addr = pdt->control_base_addr + pdt->page_start;
 > +    fd->data_base_addr = pdt->data_base_addr + pdt->page_start;
 >       fd->function_number = pdt->function_number;
 >       fd->interrupt_source_count = pdt->interrupt_source_count;
 >       fd->function_version = pdt->function_version;
 >   }
 >   -static int create_function(struct rmi_device *rmi_dev,
 > -                     struct pdt_entry *pdt,
 > -                     int *current_irq_count,
 > -                     u16 page_start)
 > +#define RMI_SCAN_CONTINUE    0
 > +#define RMI_SCAN_DONE        1
 > +
 > +static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
 > +                 int page,
 > +                 void *ctx,
 > +                 int (*callback)(struct rmi_device *rmi_dev,
 > +                         void *ctx,
 > +                         const struct pdt_entry *entry))
 > +{

Add this for use below
	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);

 > +    struct pdt_entry pdt_entry;
 > +    u16 page_start = RMI4_PAGE_SIZE * page;
 > +    u16 pdt_start = page_start + PDT_START_SCAN_LOCATION;
 > +    u16 pdt_end = page_start + PDT_END_SCAN_LOCATION;
 > +    u16 addr;
 > +    int error;
 > +    int retval;
 > +
 > +    for (addr = pdt_start; addr >= pdt_end; addr -= 
RMI_PDT_ENTRY_SIZE) {
 > +        error = rmi_read_pdt_entry(rmi_dev, &pdt_entry, addr);
 > +        if (error)
 > +            return error;
 > +
 > +        if (RMI4_END_OF_PDT(pdt_entry.function_number))
 > +            break;
 > +
 > +        dev_dbg(&rmi_dev->dev, "Found F%02X on page %#04x\n",
 > +            pdt_entry.function_number, page);
 > +
 > +        retval = callback(rmi_dev, ctx, &pdt_entry);
 > +        if (retval != RMI_SCAN_CONTINUE)
 > +            return retval;
 > +    }
 > +
 > +    return RMI_SCAN_CONTINUE;

Suggest instead to do:

	return data->f01_bootloader_mode ? RMI_SCAN_DONE : RMI_SCAN_CONTINUE;




 > +}
 > +
 > +static int rmi_scan_pdt(struct rmi_device *rmi_dev, void *ctx,
 > +            int (*callback)(struct rmi_device *rmi_dev,
 > +                    void *ctx,
 > +                    const struct pdt_entry *entry))
 > +{
 > +    struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
 > +    int page;
 > +    int retval = RMI_SCAN_DONE;
 > +
 > +    /*
 > +     * TODO: With F01 and reflash as part of the core now, is this
 > +     * lock still required?
 > +     */
 > +    mutex_lock(&data->pdt_mutex);
 > +
 > +    for (page = 0; page <= RMI4_MAX_PAGE; page++) {
 > +        retval = rmi_scan_pdt_page(rmi_dev, page, ctx, callback);
 > +        if (retval != RMI_SCAN_CONTINUE)
 > +            break;
 > +    }
 > +
 > +    mutex_unlock(&data->pdt_mutex);
 > +
 > +    return retval < 0 ? retval : 0;
 > +}
 > +
 > +static int rmi_initial_reset(struct rmi_device *rmi_dev,
 > +                 void *ctx, const struct pdt_entry *pdt)
 > +{
 > +    int error;
 > +
 > +    if (pdt->function_number == 0x01) {
 > +        u16 cmd_addr = pdt->page_start + pdt->command_base_addr;
 > +        u8 cmd_buf = RMI_DEVICE_RESET_CMD;
 > +        const struct rmi_device_platform_data *pdata =
 > +                to_rmi_platform_data(rmi_dev);
 > +
 > +        error = rmi_write_block(rmi_dev, cmd_addr, &cmd_buf, 1);
 > +        if (error) {
 > +            dev_err(&rmi_dev->dev,
 > +                "Initial reset failed. Code = %d.\n", error);
 > +            return error;
 > +        }
 > +
 > +        mdelay(pdata->reset_delay_ms);
 > +
 > +        return RMI_SCAN_DONE;
 > +    }
 > +
 > +    /* F01 should always be on page 0. If we don't find it there, 
fail. */
 > +    return pdt->page_start == 0 ? RMI_SCAN_CONTINUE : -ENODEV;
 > +}
 > +
 > +static int rmi_create_function(struct rmi_device *rmi_dev,
 > +                   void *ctx, const struct pdt_entry *pdt)
 >   {
 >       struct device *dev = &rmi_dev->dev;
 >       struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
 >       struct rmi_device_platform_data *pdata =
 > to_rmi_platform_data(rmi_dev);
 > +    int *current_irq_count = ctx;
 >       struct rmi_function *fn;
 >       int error;
 >   @@ -551,7 +641,7 @@ static int create_function(struct rmi_device
 > *rmi_dev,
 >       fn->irq_pos = *current_irq_count;
 >       *current_irq_count += fn->num_of_irqs;
 >   -    rmi_driver_copy_pdt_to_fd(pdt, &fn->fd, page_start);
 > +    rmi_driver_copy_pdt_to_fd(pdt, &fn->fd);
 >        error = rmi_register_function(fn);
 >       if (error)
 > @@ -559,131 +649,38 @@ static int create_function(struct rmi_device
 > *rmi_dev,
 >        list_add_tail(&fn->node, &data->function_list);
 >   -    return 0;
 > +    return data->f01_bootloader_mode ? RMI_SCAN_DONE : 
RMI_SCAN_CONTINUE;

As described previously, this is a bug.

 >    err_free_mem:
 >       kfree(fn);
 >       return error;
 >   }
 >   -/*
 > - * Scan the PDT for F01 so we can force a reset before anything else
 > - * is done.  This forces the sensor into a known state, and also
 > - * forces application of any pending updates from reflashing the
 > - * firmware or configuration.
 > - *
 > - * We have to do this before actually building the PDT because the 
reflash
 > - * updates (if any) might cause various registers to move around.
 > - */
 > -static int rmi_initial_reset(struct rmi_device *rmi_dev)
 > +static int rmi_create_functions(struct rmi_device *rmi_dev)
 >   {
 > -    struct pdt_entry pdt_entry;
 > -    int page;
 > -    struct device *dev = &rmi_dev->dev;
 > -    bool done = false;
 > -    bool has_f01 = false;
 > -    int i;
 > -    int retval;
 > -    const struct rmi_device_platform_data *pdata =
 > -            to_rmi_platform_data(rmi_dev);
 > -
 > -    dev_dbg(dev, "Initial reset.\n");
 > -
 > -    for (page = 0; (page <= RMI4_MAX_PAGE) && !done; page++) {
 > -        u16 page_start = RMI4_PAGE_SIZE * page;
 > -        u16 pdt_start = page_start + PDT_START_SCAN_LOCATION;
 > -        u16 pdt_end = page_start + PDT_END_SCAN_LOCATION;
 > -
 > -        done = true;
 > -        for (i = pdt_start; i >= pdt_end; i -= RMI_PDT_ENTRY_SIZE) {
 > -            retval = rmi_read_pdt_entry(rmi_dev, &pdt_entry, i);
 > -            if (retval < 0)
 > -                return retval;
 > -
 > -            if (RMI4_END_OF_PDT(pdt_entry.function_number))
 > -                break;
 > -            done = false;
 > -
 > -            if (pdt_entry.function_number == 0x01) {
 > -                u16 cmd_addr = page_start +
 > -                    pdt_entry.command_base_addr;
 > -                u8 cmd_buf = RMI_DEVICE_RESET_CMD;
 > -                retval = rmi_write_block(rmi_dev, cmd_addr,
 > -                        &cmd_buf, 1);
 > -                if (retval < 0) {
 > -                    dev_err(dev, "Initial reset failed. Code = %d.\n",
 > -                        retval);
 > -                    return retval;
 > -                }
 > -                mdelay(pdata->reset_delay_ms);
 > -                done = true;
 > -                has_f01 = true;
 > -                break;
 > -            }
 > -        }
 > -    }
 > -
 > -    if (!has_f01) {
 > -        dev_warn(dev, "WARNING: Failed to find F01 for initial 
reset.\n");
 > -        return -ENODEV;
 > -    }
 > -
 > -    return 0;
 > -}
 > -
 > -static int rmi_scan_pdt(struct rmi_device *rmi_dev)
 > -{
 > -    struct rmi_driver_data *data;
 > -    struct pdt_entry pdt_entry;
 > -    int page;
 > -    struct device *dev = &rmi_dev->dev;
 > +    struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
 >       int irq_count = 0;
 > -    bool done = false;
 > -    int i;
 >       int retval;
 >   -    dev_dbg(dev, "Scanning PDT...\n");
 > -
 > -    data = dev_get_drvdata(&rmi_dev->dev);
 > -    mutex_lock(&data->pdt_mutex);
 > -
 > -    for (page = 0; (page <= RMI4_MAX_PAGE) && !done; page++) {
 > -        u16 page_start = RMI4_PAGE_SIZE * page;
 > -        u16 pdt_start = page_start + PDT_START_SCAN_LOCATION;
 > -        u16 pdt_end = page_start + PDT_END_SCAN_LOCATION;
 > -
 > -        done = true;
 > -        for (i = pdt_start; i >= pdt_end; i -= RMI_PDT_ENTRY_SIZE) {
 > -            retval = rmi_read_pdt_entry(rmi_dev, &pdt_entry, i);
 > -            if (retval < 0)
 > -                goto error_exit;
 > -
 > -            if (RMI4_END_OF_PDT(pdt_entry.function_number))
 > -                break;
 > -
 > -            dev_dbg(dev, "Found F%02X on page %#04x\n",
 > -                    pdt_entry.function_number, page);
 > -            done = false;
 > -
 > -            // XXX need to make sure we create F01 first...
 > -            retval = create_function(rmi_dev,
 > -                    &pdt_entry, &irq_count, page_start);
 > +    /*
 > +     * XXX need to make sure we create F01 first...
 > +     * XXX or do we?  It might not be required in the updated structure.
 > +     */
 > +    retval = rmi_scan_pdt(rmi_dev, &irq_count, rmi_create_function);
 > +    if (retval < 0)
 > +        return retval;
 >   -            if (retval)
 > -                goto error_exit;
 > -        }
 > -        done = done || data->f01_bootloader_mode;
 > -    }
 > +    /*
 > +     * TODO: I think we need to count the IRQs before creating the
 > +     * functions.
 > +     */
 >       data->irq_count = irq_count;
 >       data->num_of_irq_regs = (irq_count + 7) / 8;
 > -    dev_dbg(dev, "%s: Done with PDT scan.\n", __func__);
 > -    retval = 0;
 >   -error_exit:
 > -    mutex_unlock(&data->pdt_mutex);
 > -    return retval;
 > +    return 0;
 >   }
 >   +
 >   #ifdef CONFIG_PM_SLEEP
 >   static int rmi_driver_suspend(struct device *dev)
 >   {
 > @@ -797,10 +794,15 @@ static int rmi_driver_probe(struct device *dev)
 >        /*
 >        * Right before a warm boot, the sensor might be in some unusual
 > state,
 > -     * such as F54 diagnostics, or F34 bootloader mode.  In order to 
clear
 > -     * the sensor to a known state, we issue a initial reset to 
clear any
 > +     * such as F54 diagnostics, or F34 bootloader mode after a firmware
 > +     * or configuration update.  In order to clear the sensor to a known
 > +     * state and/or apply any updates, we issue a initial reset to
 > clear any
 >        * previous settings and force it into normal operation.
 >        *
 > +     * We have to do this before actually building the PDT because
 > +     * the reflash updates (if any) might cause various registers to 
move
 > +     * around.
 > +     *
 >        * For a number of reasons, this initial reset may fail to return
 >        * within the specified time, but we'll still be able to bring 
up the
 >        * driver normally after that failure.  This occurs most 
commonly in
 > @@ -813,11 +815,11 @@ static int rmi_driver_probe(struct device *dev)
 >        */
 >       if (!pdata->reset_delay_ms)
 >           pdata->reset_delay_ms = DEFAULT_RESET_DELAY_MS;
 > -    retval = rmi_initial_reset(rmi_dev);
 > +    retval = rmi_scan_pdt(rmi_dev, NULL, rmi_initial_reset);
 >       if (retval)
 >           dev_warn(dev, "RMI initial reset failed! Continuing in spite
 > of this.\n");
 >   -    retval = rmi_scan_pdt(rmi_dev);
 > +    retval = rmi_create_functions(rmi_dev);
 >       if (retval) {
 >           dev_err(dev, "PDT scan for %s failed with code %d.\n",
 >               pdata->sensor_name, retval);
 > diff --git a/drivers/input/rmi4/rmi_driver.h
 > b/drivers/input/rmi4/rmi_driver.h
 > index 4f44a54..9ecd31b 100644
 > --- a/drivers/input/rmi4/rmi_driver.h
 > +++ b/drivers/input/rmi4/rmi_driver.h
 > @@ -1,5 +1,5 @@
 >   /*
 > - * Copyright (c) 2011-2013 Synaptics Incorporated
 > + * Copyright (c) 2011-2014 Synaptics Incorporated
 >    * Copyright (c) 2011 Unixphere
 >    *
 >    * This program is free software; you can redistribute it and/or
 > modify it
 > @@ -94,6 +94,7 @@ struct rmi_driver_data {
 >   #define RMI4_END_OF_PDT(id) ((id) == 0x00 || (id) == 0xff)
 >    struct pdt_entry {
 > +    u16 page_start;
 >       u8 query_base_addr;
 >       u8 command_base_addr;
 >       u8 control_base_addr;

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox