* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Christopher Heiny @ 2014-01-09 21:38 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: <20140109212949.GA31257@core.coreip.homeip.net>
On 01/09/2014 01:29 PM, Dmitry Torokhov wrote:
> On Thu, Jan 09, 2014 at 01:23:37PM -0800, Christopher Heiny wrote:
>> >On 01/09/2014 12:04 AM, Dmitry Torokhov wrote:
>>> > >On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
>>>> > >>This is a trivial change to replace the sprintf loop with snprintf using
>>>> > >>up-to-date format capability.
>>> > >
>>> > >Hmm, how about we do this instead:
>>> > >
>>> > >Input: synaptics-rmi4 - clean up debug handling in rmi_i2c
>>> > >
>>> > >From: Dmitry Torokhov<dmitry.torokhov@gmail.com>
>>> > >
>>> > >Kernel now has standard facility to format and print binary buffers, let's
>>> > >use it. By doing so we no longer need to allocate memory for debug buffers
>>> > >and we can let debugfs code go as well.
>> >
>> >Not sure where to put this comment, so I'll drop it here.
>> >
>> >I agree the buffers can go. I realized that on the drive home last
>> >night, but was too tired to follow up.
>> >
>> >Talking with some of the folks who use this feature, there's a
>> >desire to keep some sort of finer control on whether the comms
>> >buffers are printed or not - either the existing debugfs setup
>> >(preferred, since it lets them turn on the dmesg clutter only when
>> >needed), or by converting to a config option such as
>> >CONFIG_RMI4_COMMS_DEBUG. It's very useful in new platform
>> >development, since there's a surprising number of ways in which the
>> >reads and writes can go wonky on new hardware.
>
> That is why you have CONFIG_DYNAMIC_DEBUG: you can activate these debug
> statements at will using the common kernel mechanisms.
I'll check this out and get back.
>
> Or we could convert them to dev_vdbg() and then it will be just a tiny
> transport module recompile with DEBUG defined.
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Christopher Heiny @ 2014-01-09 22:11 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: <52CF16B8.7000409@synaptics.com>
On 01/09/2014 01:38 PM, Christopher Heiny wrote:
> On 01/09/2014 01:29 PM, Dmitry Torokhov wrote:
>> On Thu, Jan 09, 2014 at 01:23:37PM -0800, Christopher Heiny wrote:
>>> >On 01/09/2014 12:04 AM, Dmitry Torokhov wrote:
>>>> > >On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
>>>>> > >>This is a trivial change to replace the sprintf loop with
>>>>> snprintf using
>>>>> > >>up-to-date format capability.
>>>> > >
>>>> > >Hmm, how about we do this instead:
>>>> > >
>>>> > >Input: synaptics-rmi4 - clean up debug handling in rmi_i2c
>>>> > >
>>>> > >From: Dmitry Torokhov<dmitry.torokhov@gmail.com>
>>>> > >
>>>> > >Kernel now has standard facility to format and print binary
>>>> buffers, let's
>>>> > >use it. By doing so we no longer need to allocate memory for
>>>> debug buffers
>>>> > >and we can let debugfs code go as well.
>>> >
>>> >Not sure where to put this comment, so I'll drop it here.
>>> >
>>> >I agree the buffers can go. I realized that on the drive home last
>>> >night, but was too tired to follow up.
>>> >
>>> >Talking with some of the folks who use this feature, there's a
>>> >desire to keep some sort of finer control on whether the comms
>>> >buffers are printed or not - either the existing debugfs setup
>>> >(preferred, since it lets them turn on the dmesg clutter only when
>>> >needed), or by converting to a config option such as
>>> >CONFIG_RMI4_COMMS_DEBUG. It's very useful in new platform
>>> >development, since there's a surprising number of ways in which the
>>> >reads and writes can go wonky on new hardware.
> >
>> That is why you have CONFIG_DYNAMIC_DEBUG: you can activate these debug
>> statements at will using the common kernel mechanisms.
>
> I'll check this out and get back.
Looks CONFIG_DYNAMIC_DEBUG will work fine!
Acked-by: Christopher Heiny <cheiny@synaptics.com>
^ permalink raw reply
* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Dmitry Torokhov @ 2014-01-09 22:25 UTC (permalink / raw)
To: Christopher Heiny
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <52CF1E74.6060202@synaptics.com>
On Thu, Jan 09, 2014 at 02:11:00PM -0800, Christopher Heiny wrote:
> On 01/09/2014 01:38 PM, Christopher Heiny wrote:
> >On 01/09/2014 01:29 PM, Dmitry Torokhov wrote:
> >>On Thu, Jan 09, 2014 at 01:23:37PM -0800, Christopher Heiny wrote:
> >>>>On 01/09/2014 12:04 AM, Dmitry Torokhov wrote:
> >>>>> >On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
> >>>>>> >>This is a trivial change to replace the sprintf loop with
> >>>>>snprintf using
> >>>>>> >>up-to-date format capability.
> >>>>> >
> >>>>> >Hmm, how about we do this instead:
> >>>>> >
> >>>>> >Input: synaptics-rmi4 - clean up debug handling in rmi_i2c
> >>>>> >
> >>>>> >From: Dmitry Torokhov<dmitry.torokhov@gmail.com>
> >>>>> >
> >>>>> >Kernel now has standard facility to format and print binary
> >>>>buffers, let's
> >>>>> >use it. By doing so we no longer need to allocate memory for
> >>>>debug buffers
> >>>>> >and we can let debugfs code go as well.
> >>>>
> >>>>Not sure where to put this comment, so I'll drop it here.
> >>>>
> >>>>I agree the buffers can go. I realized that on the drive home last
> >>>>night, but was too tired to follow up.
> >>>>
> >>>>Talking with some of the folks who use this feature, there's a
> >>>>desire to keep some sort of finer control on whether the comms
> >>>>buffers are printed or not - either the existing debugfs setup
> >>>>(preferred, since it lets them turn on the dmesg clutter only when
> >>>>needed), or by converting to a config option such as
> >>>>CONFIG_RMI4_COMMS_DEBUG. It's very useful in new platform
> >>>>development, since there's a surprising number of ways in which the
> >>>>reads and writes can go wonky on new hardware.
> > >
> >>That is why you have CONFIG_DYNAMIC_DEBUG: you can activate these debug
> >>statements at will using the common kernel mechanisms.
> >
> >I'll check this out and get back.
>
> Looks CONFIG_DYNAMIC_DEBUG will work fine!
>
> Acked-by: Christopher Heiny <cheiny@synaptics.com>
Cool, thanks! And another one:
Input: synaptics-rmi4 - fix disabling gpio config in i2c transport
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
We need to pass into pdata->gpio_config() gpio_data which is already a
pointer, and not its address.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/rmi4/rmi_i2c.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index ebe74ec..12aea8c 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -168,7 +168,8 @@ exit:
static int rmi_i2c_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- const struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
+ const struct rmi_device_platform_data *pdata =
+ dev_get_platdata(&client->dev);
struct rmi_transport_dev *xport;
struct rmi_i2c_data *data;
int retval;
@@ -250,12 +251,13 @@ err_gpio:
static int rmi_i2c_remove(struct i2c_client *client)
{
struct rmi_transport_dev *xport = i2c_get_clientdata(client);
- struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
+ const struct rmi_device_platform_data *pdata =
+ dev_get_platdata(&client->dev);
rmi_unregister_transport_device(xport);
if (pdata->gpio_config)
- pdata->gpio_config(&pdata->gpio_data, false);
+ pdata->gpio_config(pdata->gpio_data, false);
return 0;
}
--
Dmitry
^ permalink raw reply related
* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Christopher Heiny @ 2014-01-09 22:47 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: <20140109222546.GA20994@core.coreip.homeip.net>
On 01/09/2014 02:25 PM, Dmitry Torokhov wrote:
> Input: synaptics-rmi4 - fix disabling gpio config in i2c transport
>
> From: Dmitry Torokhov<dmitry.torokhov@gmail.com>
>
> We need to pass into pdata->gpio_config() gpio_data which is already a
> pointer, and not its address.
>
> Signed-off-by: Dmitry Torokhov<dmitry.torokhov@gmail.com>
> ---
> drivers/input/rmi4/rmi_i2c.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
> index ebe74ec..12aea8c 100644
> --- a/drivers/input/rmi4/rmi_i2c.c
> +++ b/drivers/input/rmi4/rmi_i2c.c
> @@ -168,7 +168,8 @@ exit:
> static int rmi_i2c_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> - const struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
> + const struct rmi_device_platform_data *pdata =
> + dev_get_platdata(&client->dev);
> struct rmi_transport_dev *xport;
> struct rmi_i2c_data *data;
> int retval;
> @@ -250,12 +251,13 @@ err_gpio:
> static int rmi_i2c_remove(struct i2c_client *client)
> {
> struct rmi_transport_dev *xport = i2c_get_clientdata(client);
> - struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
> + const struct rmi_device_platform_data *pdata =
> + dev_get_platdata(&client->dev);
>
> rmi_unregister_transport_device(xport);
>
> if (pdata->gpio_config)
> - pdata->gpio_config(&pdata->gpio_data, false);
> + pdata->gpio_config(pdata->gpio_data, false);
>
> return 0;
> }
It's just plain freaky that no problem ever arose from that, but
definitely it's a bug.
Acked-by: Christopher Heiny <cheiny@synaptics.com>
^ permalink raw reply
* [PATCH 0/2] input: appletouch: fixes for jagged/uneven cursor movement
From: Clinton Sprain @ 2014-01-09 23:59 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
From: Clinton Sprain <clintonsprain@gmail.com>
For many devices, the appletouch driver can make the trackpad feel
insensitive and cursor movement will tend to jerk in steps. This is
particularly noticeable when moving the cursor diagonally. This can
greatly impact the trackpad's usability.
The proposed changes address this by dialing back the fuzz and threshold
parameters and by implementing a new cursor movement smoothing algorithm.
User feedback couldn't be found for some older devices that use this driver,
so module parameters have been added to allow users to manually set the
fuzz and threshold settings and/or opt out of the new smoothing algorithm.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
Tested-by: Curtis Rueden <ctrueden@gmail.com>
Clinton Sprain (2):
input: appletouch: parametrize and set saner defaults for fuzz and
threshold
input: appletouch: use better cursor movement smoothing algorithm
drivers/input/mouse/appletouch.c | 184 +++++++++++++++++++++++++++++---------
1 file changed, 142 insertions(+), 42 deletions(-)
--
1.7.9.5
^ permalink raw reply
* [PATCH 2/2] input: appletouch: use better cursor movement smoothing algorithm
From: Clinton Sprain @ 2014-01-10 0:02 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
In-Reply-To: <52CF37E1.3030809@gmail.com>
From: Clinton Sprain <clintonsprain@gmail.com>
Implements a new algorithm used to smooth cursor movement, using the inertia
of the cursor's prior movements to modulate the next cursor movement. This
further mitigates the 'stepping' users see when moving the cursor diagonally.
A 'legacy' module parameter has been added in case the older devices do not
like the new algorithm.
Signed-Off-By: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 118 ++++++++++++++++++++++++++++++++------
1 file changed, 100 insertions(+), 18 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index ba6bbd7..7a33188 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -209,13 +209,16 @@ struct atp {
bool overflow_warned;
int x_old; /* last reported x/y, */
int y_old; /* used for smoothing */
+ int x_older; /* 2nd to last reported x/y,*/
+ int y_older; /* used for smoothing */
+ int x_oldest; /* 3rd to last reported x/y,*/
+ int y_oldest; /* used for smoothing */
signed char xy_cur[ATP_XSENSORS + ATP_YSENSORS];
signed char xy_old[ATP_XSENSORS + ATP_YSENSORS];
int xy_acc[ATP_XSENSORS + ATP_YSENSORS];
int idlecount; /* number of empty packets */
struct work_struct work;
};
-
#define dbg_dump(msg, tab) \
{ \
if (debug > 1) { \
@@ -260,6 +263,13 @@ static int debug;
module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "Activate debugging output");
+static int legacy;
+module_param(legacy, int, 0644);
+MODULE_PARM_DESC(legacy, "1 = Use old cursor movement smoothing."
+ " Older behavior averages current with"
+ " last cursor position. New behavior"
+ " uses cursor movement inertia.");
+
/*
* By default newer Geyser devices send standard USB HID mouse
* packets (Report ID 2). This code changes device mode, so it
@@ -338,6 +348,59 @@ static void atp_reinit(struct work_struct *work)
retval);
}
+static int atp_smooth_legacy(int curr, int old)
+{
+ return ((old * 3) + curr) >> 2;
+}
+
+static int atp_smooth(int curr, int old, int older, int oldest)
+{
+ int diff, projected, olderprojected, smooth;
+
+ diff = old - older;
+ projected = old + diff;
+
+ diff = older - oldest;
+ olderprojected = older + diff;
+
+ smooth = (projected * 7 + olderprojected * 8 + curr) >> 4;
+
+ return smooth;
+}
+
+static void atp_refresh_old_xy(int x, int y, struct atp *dev)
+{
+ if (legacy != true) {
+ dev->x_oldest = dev->x_older;
+ dev->y_oldest = dev->y_older;
+ dev->x_older = dev->x_old;
+ dev->y_older = dev->y_old;
+ }
+ dev->x_old = x;
+ dev->y_old = y;
+}
+
+static void atp_reset_old_xy(struct atp *dev)
+{
+ if (legacy != true) {
+ dev->x_oldest = dev->y_oldest = -1;
+ dev->x_older = dev->y_older = -1;
+ }
+ dev->x_old = dev->y_old = -1;
+}
+
+static void atp_default_old_xy(struct atp *dev)
+{
+ if (dev->x_older == -1)
+ dev->x_older = dev->x_old;
+ if (dev->y_older == -1)
+ dev->y_older = dev->y_old;
+ if (dev->x_oldest == -1)
+ dev->x_oldest = dev->x_older;
+ if (dev->y_oldest == -1)
+ dev->y_oldest = dev->y_older;
+}
+
static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
int *z, int *fingers)
{
@@ -386,8 +449,13 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
* occasionally jump a number of pixels (slowly moving the
* finger makes this issue most apparent.)
*/
- pcum += (xy_sensors[i] - threshold) * i;
- psum += (xy_sensors[i] - threshold);
+ if (legacy == true) {
+ pcum += (xy_sensors[i] - threshold) * i;
+ psum += (xy_sensors[i] - threshold);
+ } else {
+ pcum += (xy_sensors[i]) * i;
+ psum += (xy_sensors[i]);
+ }
}
if (psum > 0) {
@@ -570,10 +638,18 @@ static void atp_complete_geyser_1_2(struct urb *urb)
if (x && y) {
if (dev->x_old != -1) {
- x = (dev->x_old * 3 + x) >> 2;
- y = (dev->y_old * 3 + y) >> 2;
- dev->x_old = x;
- dev->y_old = y;
+ if (legacy == true) {
+ x = atp_smooth_legacy(x, dev->x_old);
+ y = atp_smooth_legacy(y, dev->y_old);
+ } else {
+
+ atp_default_old_xy(dev);
+
+ x = atp_smooth(x, dev->x_old,
+ dev->x_older, dev->x_oldest);
+ y = atp_smooth(y, dev->y_old,
+ dev->y_older, dev->y_oldest);
+ }
if (debug > 1)
printk(KERN_DEBUG "appletouch: "
@@ -587,12 +663,11 @@ static void atp_complete_geyser_1_2(struct urb *urb)
min(ATP_PRESSURE, x_z + y_z));
atp_report_fingers(dev->input, max(x_f, y_f));
}
- dev->x_old = x;
- dev->y_old = y;
+ atp_refresh_old_xy(x, y, dev);
} else if (!x && !y) {
- dev->x_old = dev->y_old = -1;
+ atp_reset_old_xy(dev);
input_report_key(dev->input, BTN_TOUCH, 0);
input_report_abs(dev->input, ABS_PRESSURE, 0);
atp_report_fingers(dev->input, 0);
@@ -682,10 +757,18 @@ static void atp_complete_geyser_3_4(struct urb *urb)
if (x && y) {
if (dev->x_old != -1) {
- x = (dev->x_old * 3 + x) >> 2;
- y = (dev->y_old * 3 + y) >> 2;
- dev->x_old = x;
- dev->y_old = y;
+ if (legacy == true) {
+ x = atp_smooth_legacy(x, dev->x_old);
+ y = atp_smooth_legacy(y, dev->y_old);
+ } else {
+
+ atp_default_old_xy(dev);
+
+ x = atp_smooth(x, dev->x_old,
+ dev->x_older, dev->x_oldest);
+ y = atp_smooth(y, dev->y_old,
+ dev->y_older, dev->y_oldest);
+ }
if (debug > 1)
printk(KERN_DEBUG "appletouch: X: %3d Y: %3d "
@@ -699,12 +782,11 @@ static void atp_complete_geyser_3_4(struct urb *urb)
min(ATP_PRESSURE, x_z + y_z));
atp_report_fingers(dev->input, max(x_f, y_f));
}
- dev->x_old = x;
- dev->y_old = y;
+ atp_refresh_old_xy(x, y, dev);
} else if (!x && !y) {
- dev->x_old = dev->y_old = -1;
+ atp_reset_old_xy(dev);
input_report_key(dev->input, BTN_TOUCH, 0);
input_report_abs(dev->input, ABS_PRESSURE, 0);
atp_report_fingers(dev->input, 0);
@@ -730,7 +812,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
if (!x && !y && !key) {
dev->idlecount++;
if (dev->idlecount == 10) {
- dev->x_old = dev->y_old = -1;
+ atp_reset_old_xy(dev);
dev->idlecount = 0;
schedule_work(&dev->work);
/* Don't resubmit urb here, wait for reinit */
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/2] input: appletouch: parametrize and set saner defaults for fuzz and threshold
From: Clinton Sprain @ 2014-01-10 0:01 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
In-Reply-To: <52CF37E1.3030809@gmail.com>
From: Clinton Sprain <clintonsprain@gmail.com>
Dials back the default fuzz and threshold settings for most devices using
this driver, based on values from user feedback from forums and bug reports.
This increases smoothness and movement granularity. No changes were made for
the older devices that use the driver, as I did not find any user feedback
for them regarding these settings; however, the two settings can also now
both be specified as module parameters in case there is a desire to change
the values for devices that do not have new defaults.
There are also a couple of style cleanups per checkpatch.pl.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 66 ++++++++++++++++++++++++--------------
1 file changed, 42 insertions(+), 24 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index e42f1fa..ba6bbd7 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -43,12 +43,14 @@
*/
struct atp_info {
int xsensors; /* number of X sensors */
- int xsensors_17; /* 17" models have more sensors */
+ int xsensors_17; /* 17" model has more sensors */
int ysensors; /* number of Y sensors */
int xfact; /* X multiplication factor */
int yfact; /* Y multiplication factor */
int datalen; /* size of USB transfers */
void (*callback)(struct urb *); /* callback function */
+ int fuzz; /* fuzz touchpad generates */
+ int threshold; /* sensitivity threshold */
};
static void atp_complete_geyser_1_2(struct urb *urb);
@@ -62,6 +64,8 @@ static const struct atp_info fountain_info = {
.yfact = 43,
.datalen = 81,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 16,
+ .threshold = 5,
};
static const struct atp_info geyser1_info = {
@@ -72,6 +76,8 @@ static const struct atp_info geyser1_info = {
.yfact = 43,
.datalen = 81,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 16,
+ .threshold = 5,
};
static const struct atp_info geyser2_info = {
@@ -82,6 +88,8 @@ static const struct atp_info geyser2_info = {
.yfact = 43,
.datalen = 64,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 0,
+ .threshold = 3,
};
static const struct atp_info geyser3_info = {
@@ -91,6 +99,8 @@ static const struct atp_info geyser3_info = {
.yfact = 64,
.datalen = 64,
.callback = atp_complete_geyser_3_4,
+ .fuzz = 0,
+ .threshold = 3,
};
static const struct atp_info geyser4_info = {
@@ -100,6 +110,8 @@ static const struct atp_info geyser4_info = {
.yfact = 64,
.datalen = 64,
.callback = atp_complete_geyser_3_4,
+ .fuzz = 0,
+ .threshold = 2,
};
#define ATP_DEVICE(prod, info) \
@@ -156,18 +168,9 @@ MODULE_DEVICE_TABLE(usb, atp_table);
#define ATP_XSENSORS 26
#define ATP_YSENSORS 16
-/* amount of fuzz this touchpad generates */
-#define ATP_FUZZ 16
-
/* maximum pressure this driver will report */
#define ATP_PRESSURE 300
-/*
- * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
- * ignored.
- */
-#define ATP_THRESHOLD 5
-
/* Geyser initialization constants */
#define ATP_GEYSER_MODE_READ_REQUEST_ID 1
#define ATP_GEYSER_MODE_WRITE_REQUEST_ID 9
@@ -213,14 +216,16 @@ struct atp {
struct work_struct work;
};
-#define dbg_dump(msg, tab) \
+#define dbg_dump(msg, tab) \
+{ \
if (debug > 1) { \
int __i; \
printk(KERN_DEBUG "appletouch: %s", msg); \
for (__i = 0; __i < ATP_XSENSORS + ATP_YSENSORS; __i++) \
printk(" %02x", tab[__i]); \
printk("\n"); \
- }
+ } \
+}
#define dprintk(format, a...) \
do { \
@@ -236,14 +241,20 @@ MODULE_AUTHOR("Sven Anders");
MODULE_DESCRIPTION("Apple PowerBook and MacBook USB touchpad driver");
MODULE_LICENSE("GPL");
-/*
- * Make the threshold a module parameter
- */
-static int threshold = ATP_THRESHOLD;
+static int threshold = -1;
module_param(threshold, int, 0644);
MODULE_PARM_DESC(threshold, "Discard any change in data from a sensor"
" (the trackpad has many of these sensors)"
- " less than this value.");
+ " less than this value. Devices have defaults;"
+ " this parameter overrides them.");
+static int fuzz = -1;
+
+module_param(fuzz, int, 0644);
+MODULE_PARM_DESC(fuzz, "Fuzz the trackpad generates. The higher"
+ " this is, the less sensitive the trackpad"
+ " will feel, but values too low may generate"
+ " random movement. Devices have defaults;"
+ " this parameter overrides them.");
static int debug;
module_param(debug, int, 0644);
@@ -363,7 +374,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
(!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
(*fingers)++;
is_increasing = 1;
- } else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
+ } else if (i > 0 && (xy_sensors[i - 1] -
+ xy_sensors[i] > threshold)) {
is_increasing = 0;
}
@@ -456,7 +468,7 @@ static void atp_detect_size(struct atp *dev)
input_set_abs_params(dev->input, ABS_X, 0,
(dev->info->xsensors_17 - 1) *
dev->info->xfact - 1,
- ATP_FUZZ, 0);
+ fuzz, 0);
break;
}
}
@@ -513,7 +525,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
/* Y values */
dev->xy_cur[ATP_XSENSORS + i] = dev->data[5 * i + 1];
- dev->xy_cur[ATP_XSENSORS + i + 8] = dev->data[5 * i + 3];
+ dev->xy_cur[ATP_XSENSORS + i + 8] =
+ dev->data[5 * i + 3];
}
}
@@ -809,12 +822,17 @@ static int atp_probe(struct usb_interface *iface,
dev->info = info;
dev->overflow_warned = false;
+ if (fuzz < 0)
+ fuzz = dev->info->fuzz;
+ if (threshold < 0)
+ threshold = dev->info->threshold;
+
dev->urb = usb_alloc_urb(0, GFP_KERNEL);
if (!dev->urb)
goto err_free_devs;
- dev->data = usb_alloc_coherent(dev->udev, dev->info->datalen, GFP_KERNEL,
- &dev->urb->transfer_dma);
+ dev->data = usb_alloc_coherent(dev->udev, dev->info->datalen,
+ GFP_KERNEL, &dev->urb->transfer_dma);
if (!dev->data)
goto err_free_urb;
@@ -844,10 +862,10 @@ static int atp_probe(struct usb_interface *iface,
input_set_abs_params(input_dev, ABS_X, 0,
(dev->info->xsensors - 1) * dev->info->xfact - 1,
- ATP_FUZZ, 0);
+ fuzz, 0);
input_set_abs_params(input_dev, ABS_Y, 0,
(dev->info->ysensors - 1) * dev->info->yfact - 1,
- ATP_FUZZ, 0);
+ fuzz, 0);
input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
set_bit(EV_KEY, input_dev->evbit);
--
1.7.9.5
^ permalink raw reply related
* [PATCH] input: synaptics-rmi4 - rmi_driver.h tidy up
From: Christopher Heiny @ 2014-01-10 1:59 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Linux Input, Christopher Heiny, Andrew Duggan, Vincent Huang,
Vivian Ly, Daniel Rosenberg, Jean Delvare, Joerie de Gram,
Linus Walleij, Benjamin Tissoires
This is a trivial change to remove an unused macro, and change CONFIG_PM to
CONFIG_PM_SLEEP.
Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/input/rmi4/rmi_driver.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 4d82b90..ad3ee33 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -24,8 +24,6 @@
.attrs = _attrs, \
}
-#define attrify(nm) (&dev_attr_##nm.attr)
-
#define PDT_PROPERTIES_LOCATION 0x00EF
#define BSR_LOCATION 0x00FE
@@ -62,7 +60,7 @@ struct rmi_driver_data {
u8 bsr;
bool enabled;
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
bool suspended;
struct mutex suspend_mutex;
^ permalink raw reply related
* [PATCH] add support for ALPS v7 protocol device
From: Qiting Chen @ 2014-01-10 5:53 UTC (permalink / raw)
To: dmitry.torokhov, cernekee, dturvene
Cc: linux-input, ndevos, jclift, Qiting Chen
Here is the patch of supporting ALPS v7 protocol device.
v7 device is a clickpad device.
Device info:
Device ID = 0x73, 0x03, 0x0a
Firmware ID = 0x88, 0xb*, 0x**
Support function of v7 device:
- Cursor
- Tap, double tap, tap drag, 2finger tap
- Pan, pinch
- Button click: v7 device has one physical button under the touchpad. A Button Area covers the bottom of touchpad. Clicking on Button Area produces button acitivities.
Click touchpad with all fingers outside right Button Area --> left click
Click touchpad with at lease 1 finger inside right Button Area --> right click
- Resting finger function: Cursor and gestures won't be influenced with one finger placed still on Button Area.
The resting finger process is in alps.c as it seems the latest xserver-xorg-input-synaptics(1.7.1) doesn't support that part.
We tried registering our device as a clickpad by:
set_bit(INPUT_PROP_BUTTONPAD, dev1->propbit)
But only button click can be correctly recognized. Yet a finger at Button Area won't be ignored while doing cursroing.
Signed-off-by: Qiting Chen <qiting.chen@cn.alps.com>
---
drivers/input/mouse/alps.c | 478 ++++++++++++++++++++++++++++++++++++++++++---
drivers/input/mouse/alps.h | 127 ++++++++++--
2 files changed, 554 insertions(+), 51 deletions(-)
diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index fb15c64..3e8e8f7 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -32,6 +32,11 @@
#define ALPS_REG_BASE_RUSHMORE 0xc2c0
#define ALPS_REG_BASE_PINNACLE 0x0000
+#define LEFT_BUTTON_BIT 0x01
+#define RIGHT_BUTTON_BIT 0x02
+
+#define V7_LARGE_MOVEMENT 130
+
static const struct alps_nibble_commands alps_v3_nibble_commands[] = {
{ PSMOUSE_CMD_SETPOLL, 0x00 }, /* 0 */
{ PSMOUSE_CMD_RESET_DIS, 0x00 }, /* 1 */
@@ -99,6 +104,7 @@ static const struct alps_nibble_commands alps_v6_nibble_commands[] = {
#define ALPS_FOUR_BUTTONS 0x40 /* 4 direction button present */
#define ALPS_PS2_INTERLEAVED 0x80 /* 3-byte PS/2 packet interleaved with
6-byte ALPS packet */
+#define ALPS_BTNLESS 0x100 /* ALPS ClickPad flag */
static const struct alps_model_info alps_model_data[] = {
{ { 0x32, 0x02, 0x14 }, 0x00, ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT }, /* Toshiba Salellite Pro M10 */
@@ -140,6 +146,20 @@ static void alps_set_abs_params_mt(struct alps_data *priv,
* isn't valid per PS/2 spec.
*/
+static unsigned int alps_pt_distance(struct alps_abs_data *pt0,
+ struct alps_abs_data *pt1)
+{
+ int vect_x, vect_y;
+
+ if (!pt0 || !pt1)
+ return 0;
+
+ vect_x = pt0->x - pt1->x;
+ vect_y = pt0->y - pt1->y;
+
+ return int_sqrt(vect_x * vect_x + vect_y * vect_y);
+}
+
/* Packet formats are described in Documentation/input/alps.txt */
static bool alps_is_valid_first_byte(struct alps_data *priv,
@@ -320,8 +340,8 @@ static void alps_process_bitmap_dolphin(struct alps_data *priv,
end_bit = y_msb - 1;
box_middle_y = (priv->y_max * (start_bit + end_bit)) /
(2 * (priv->y_bits - 1));
- *x1 = fields->x;
- *y1 = fields->y;
+ *x1 = fields->pt.x;
+ *y1 = fields->pt.y;
*x2 = 2 * box_middle_x - *x1;
*y2 = 2 * box_middle_y - *y1;
}
@@ -461,6 +481,38 @@ static void alps_report_semi_mt_data(struct input_dev *dev, int num_fingers,
alps_set_slot(dev, 1, num_fingers == 2, x2, y2);
}
+static void alps_report_coord_and_btn(struct psmouse *psmouse,
+ struct alps_fields *f)
+{
+ struct input_dev *dev;
+
+ if (!psmouse || !f)
+ return;
+
+ dev = psmouse->dev;
+
+ if (f->fingers) {
+ input_report_key(dev, BTN_TOUCH, 1);
+ alps_report_semi_mt_data(dev, f->fingers,
+ f->pt_img[0].x, f->pt_img[0].y,
+ f->pt_img[1].x, f->pt_img[1].y);
+ input_mt_report_finger_count(dev, f->fingers);
+
+ input_report_abs(dev, ABS_X, f->pt_img[0].x);
+ input_report_abs(dev, ABS_Y, f->pt_img[0].y);
+ input_report_abs(dev, ABS_PRESSURE, f->pt_img[0].z);
+ } else {
+ input_report_key(dev, BTN_TOUCH, 0);
+ input_mt_report_finger_count(dev, 0);
+ input_report_abs(dev, ABS_PRESSURE, 0);
+ }
+
+ input_report_key(dev, BTN_LEFT, f->btn.left);
+ input_report_key(dev, BTN_RIGHT, f->btn.right);
+
+ input_sync(dev);
+}
+
static void alps_process_trackstick_packet_v3(struct psmouse *psmouse)
{
struct alps_data *priv = psmouse->private;
@@ -523,13 +575,13 @@ static void alps_process_trackstick_packet_v3(struct psmouse *psmouse)
static void alps_decode_buttons_v3(struct alps_fields *f, unsigned char *p)
{
- f->left = !!(p[3] & 0x01);
- f->right = !!(p[3] & 0x02);
- f->middle = !!(p[3] & 0x04);
+ f->btn.left = !!(p[3] & 0x01);
+ f->btn.right = !!(p[3] & 0x02);
+ f->btn.middle = !!(p[3] & 0x04);
- f->ts_left = !!(p[3] & 0x10);
- f->ts_right = !!(p[3] & 0x20);
- f->ts_middle = !!(p[3] & 0x40);
+ f->btn.ts_left = !!(p[3] & 0x10);
+ f->btn.ts_right = !!(p[3] & 0x20);
+ f->btn.ts_middle = !!(p[3] & 0x40);
}
static void alps_decode_pinnacle(struct alps_fields *f, unsigned char *p,
@@ -546,10 +598,10 @@ static void alps_decode_pinnacle(struct alps_fields *f, unsigned char *p,
((p[2] & 0x7f) << 1) |
(p[4] & 0x01);
- f->x = ((p[1] & 0x7f) << 4) | ((p[4] & 0x30) >> 2) |
+ f->pt.x = ((p[1] & 0x7f) << 4) | ((p[4] & 0x30) >> 2) |
((p[0] & 0x30) >> 4);
- f->y = ((p[2] & 0x7f) << 4) | (p[4] & 0x0f);
- f->z = p[5] & 0x7f;
+ f->pt.y = ((p[2] & 0x7f) << 4) | (p[4] & 0x0f);
+ f->pt.z = p[5] & 0x7f;
alps_decode_buttons_v3(f, p);
}
@@ -573,9 +625,9 @@ static void alps_decode_dolphin(struct alps_fields *f, unsigned char *p,
f->is_mp = !!(p[0] & 0x20);
if (!f->is_mp) {
- f->x = ((p[1] & 0x7f) | ((p[4] & 0x0f) << 7));
- f->y = ((p[2] & 0x7f) | ((p[4] & 0xf0) << 3));
- f->z = (p[0] & 4) ? 0 : p[5] & 0x7f;
+ f->pt.x = ((p[1] & 0x7f) | ((p[4] & 0x0f) << 7));
+ f->pt.y = ((p[2] & 0x7f) | ((p[4] & 0xf0) << 3));
+ f->pt.z = (p[0] & 4) ? 0 : p[5] & 0x7f;
alps_decode_buttons_v3(f, p);
} else {
f->fingers = ((p[0] & 0x6) >> 1 |
@@ -687,7 +739,7 @@ static void alps_process_touchpad_packet_v3_v5(struct psmouse *psmouse)
* with x, y, and z all zero, so these seem to be flukes.
* Ignore them.
*/
- if (f.x && f.y && !f.z)
+ if (f.pt.x && f.pt.y && !f.pt.z)
return;
/*
@@ -695,12 +747,12 @@ static void alps_process_touchpad_packet_v3_v5(struct psmouse *psmouse)
* to rely on ST data.
*/
if (!fingers) {
- x1 = f.x;
- y1 = f.y;
- fingers = f.z > 0 ? 1 : 0;
+ x1 = f.pt.x;
+ y1 = f.pt.y;
+ fingers = f.pt.z > 0 ? 1 : 0;
}
- if (f.z >= 64)
+ if (f.pt.z >= 64)
input_report_key(dev, BTN_TOUCH, 1);
else
input_report_key(dev, BTN_TOUCH, 0);
@@ -709,22 +761,22 @@ static void alps_process_touchpad_packet_v3_v5(struct psmouse *psmouse)
input_mt_report_finger_count(dev, fingers);
- input_report_key(dev, BTN_LEFT, f.left);
- input_report_key(dev, BTN_RIGHT, f.right);
- input_report_key(dev, BTN_MIDDLE, f.middle);
+ input_report_key(dev, BTN_LEFT, f.btn.left);
+ input_report_key(dev, BTN_RIGHT, f.btn.right);
+ input_report_key(dev, BTN_MIDDLE, f.btn.middle);
- if (f.z > 0) {
- input_report_abs(dev, ABS_X, f.x);
- input_report_abs(dev, ABS_Y, f.y);
+ if (f.pt.z > 0) {
+ input_report_abs(dev, ABS_X, f.pt.x);
+ input_report_abs(dev, ABS_Y, f.pt.y);
}
- input_report_abs(dev, ABS_PRESSURE, f.z);
+ input_report_abs(dev, ABS_PRESSURE, f.pt.z);
input_sync(dev);
if (!(priv->quirks & ALPS_QUIRK_TRACKSTICK_BUTTONS)) {
- input_report_key(dev2, BTN_LEFT, f.ts_left);
- input_report_key(dev2, BTN_RIGHT, f.ts_right);
- input_report_key(dev2, BTN_MIDDLE, f.ts_middle);
+ input_report_key(dev2, BTN_LEFT, f.btn.ts_left);
+ input_report_key(dev2, BTN_RIGHT, f.btn.ts_right);
+ input_report_key(dev2, BTN_MIDDLE, f.btn.ts_middle);
input_sync(dev2);
}
}
@@ -916,6 +968,294 @@ static void alps_process_packet_v4(struct psmouse *psmouse)
input_sync(dev);
}
+static bool alps_is_valid_package_v7(struct psmouse *psmouse)
+{
+ if ((psmouse->pktcnt == 3) && ((psmouse->packet[2] & 0x40) != 0x40))
+ return false;
+ if ((psmouse->pktcnt == 4) && ((psmouse->packet[3] & 0x48) != 0x48))
+ return false;
+ if ((psmouse->pktcnt == 6) && ((psmouse->packet[5] & 0x40) != 0x0))
+ return false;
+ return true;
+}
+
+static int alps_drop_unsupported_packet_v7(struct psmouse *psmouse)
+{
+ struct alps_data *priv = psmouse->private;
+ int drop = 1;
+
+ if (priv->r.v7.pkt_id == V7_PACKET_ID_NEW ||
+ priv->r.v7.pkt_id == V7_PACKET_ID_TWO ||
+ priv->r.v7.pkt_id == V7_PACKET_ID_MULTI ||
+ priv->r.v7.pkt_id == V7_PACKET_ID_IDLE)
+ drop = 0;
+
+ return drop;
+}
+
+static unsigned char alps_get_packet_id_v7(char *byte)
+{
+ unsigned char packet_id;
+
+ if (byte[4] & 0x40)
+ packet_id = V7_PACKET_ID_TWO;
+ else if (byte[4] & 0x01)
+ packet_id = V7_PACKET_ID_MULTI;
+ else if ((byte[0] & 0x10) && !(byte[4] & 0x43))
+ packet_id = V7_PACKET_ID_NEW;
+ else
+ packet_id = V7_PACKET_ID_IDLE;
+
+ return packet_id;
+}
+
+static void alps_get_finger_coordinate_v7(struct alps_abs_data *pt,
+ unsigned char *pkt,
+ unsigned char pkt_id)
+{
+ if ((pkt_id == V7_PACKET_ID_TWO) ||
+ (pkt_id == V7_PACKET_ID_MULTI) ||
+ (pkt_id == V7_PACKET_ID_NEW)) {
+ pt[0].x = ((pkt[2] & 0x80) << 4);
+ pt[0].x |= ((pkt[2] & 0x3F) << 5);
+ pt[0].x |= ((pkt[3] & 0x30) >> 1);
+ pt[0].x |= (pkt[3] & 0x07);
+ pt[0].y = (pkt[1] << 3) | (pkt[0] & 0x07);
+
+ pt[1].x = ((pkt[3] & 0x80) << 4);
+ pt[1].x |= ((pkt[4] & 0x80) << 3);
+ pt[1].x |= ((pkt[4] & 0x3F) << 4);
+ pt[1].y = ((pkt[5] & 0x80) << 3);
+ pt[1].y |= ((pkt[5] & 0x3F) << 4);
+
+ if (pkt_id == V7_PACKET_ID_TWO) {
+ pt[1].x &= ~0x000F;
+ pt[1].y |= 0x000F;
+ } else if (pkt_id == V7_PACKET_ID_MULTI) {
+ pt[1].x &= ~0x003F;
+ pt[1].y &= ~0x0020;
+ pt[1].y |= ((pkt[4] & 0x02) << 4);
+ pt[1].y |= 0x001F;
+ } else if (pkt_id == V7_PACKET_ID_NEW) {
+ pt[1].x &= ~0x003F;
+ pt[1].x |= (pkt[0] & 0x20);
+ pt[1].y |= 0x000F;
+ }
+
+ pt[0].y = 0x7FF - pt[0].y;
+ pt[1].y = 0x7FF - pt[1].y;
+
+ pt[0].z = (pt[0].x && pt[0].y) ? 62 : 0;
+ pt[1].z = (pt[1].x && pt[1].y) ? 62 : 0;
+ }
+}
+
+static void alps_decode_packet_v7(struct alps_fields *f,
+ unsigned char *p,
+ struct psmouse *psmouse)
+{
+ struct alps_data *priv = psmouse->private;
+
+ priv->r.v7.pkt_id = alps_get_packet_id_v7(p);
+
+ alps_get_finger_coordinate_v7(f->pt_img, p, priv->r.v7.pkt_id);
+
+ priv->r.v7.rest_left = 0;
+ priv->r.v7.rest_right = 0;
+ priv->r.v7.additional_fingers = 0;
+ priv->phy_btn = 0;
+
+ if (priv->r.v7.pkt_id == V7_PACKET_ID_TWO ||
+ priv->r.v7.pkt_id == V7_PACKET_ID_MULTI) {
+ priv->r.v7.rest_left = (p[0] & 0x10) >> 4;
+ priv->r.v7.rest_right = (p[0] & 0x20) >> 5;
+ }
+
+ if (priv->r.v7.pkt_id == V7_PACKET_ID_MULTI)
+ priv->r.v7.additional_fingers = p[5] & 0x03;
+
+ priv->phy_btn = (p[0] & 0x80) >> 7;
+}
+
+static void alps_set_each_pt_attr_v7(struct psmouse *psmouse,
+ struct alps_abs_data *pt,
+ struct alps_bl_pt_attr *pt_attr)
+{
+ struct alps_data *priv = psmouse->private;
+ unsigned int dist;
+
+ if (!pt_attr->is_init_pt_got && pt->z != 0) {
+ pt_attr->is_init_pt_got = 1;
+ pt_attr->is_counted = 0;
+ memcpy(&pt_attr->init_pt, pt, sizeof(pt_attr->init_pt));
+ }
+
+ if (pt->z != 0) {
+ if (pt->y < priv->resting_zone_y_min) {
+ /* A finger is recognized as a non-resting finger
+ if it's position is outside the resting finger zone.*/
+ pt_attr->zone = ZONE_NORMAL;
+ pt_attr->is_counted = 1;
+ } else {
+ /* A finger is recognized as a resting finger if it's
+ position is inside the resting finger zone and there's
+ no large movement from it's touch down position.*/
+ pt_attr->zone = ZONE_RESTING;
+
+ if (pt->x > priv->x_max / 2)
+ pt_attr->zone |= ZONE_RIGHT_BTN;
+ else
+ pt_attr->zone |= ZONE_LEFT_BTN;
+
+ /* A resting finger will turn to be a non-resting
+ finger if it has made large movement from it's touch
+ down position. A non-resting finger will never turn
+ to a resting finger before it leaves the touchpad
+ surface.*/
+ if (pt_attr->is_init_pt_got) {
+ dist = alps_pt_distance(pt, &pt_attr->init_pt);
+
+ if (dist > V7_LARGE_MOVEMENT)
+ pt_attr->is_counted = 1;
+ }
+ }
+ }
+}
+
+static void alps_set_pt_attr_v7(struct psmouse *psmouse,
+ struct alps_fields *f)
+{
+ struct alps_data *priv = psmouse->private;
+ int i;
+
+ switch (priv->r.v7.pkt_id) {
+ case V7_PACKET_ID_TWO:
+ case V7_PACKET_ID_MULTI:
+ for (i = 0; i < V7_IMG_PT_NUM; i++)
+ alps_set_each_pt_attr_v7(psmouse,
+ &f->pt_img[i],
+ &priv->pt_attr[i]);
+ break;
+ default:
+ /*All finger attributes are cleared when packet ID is
+ 'IDLE', 'New'or other unknown IDs. An 'IDLE' packet
+ indicates that there's no finger and no button activity.
+ A 'NEW' packet indicates the finger position in packet
+ is not continues from previous packet. Such as the
+ condition there's finger placed or lifted. In these cases,
+ finger attributes will be reset.*/
+ memset(priv->pt_attr, 0, sizeof(priv->pt_attr[0]) * 2);
+ break;
+ }
+}
+
+static void alps_cal_output_finger_num_v7(struct psmouse *psmouse,
+ struct alps_fields *f)
+{
+ struct alps_data *priv = psmouse->private;
+ unsigned int fn = 0;
+ int i;
+
+ switch (priv->r.v7.pkt_id) {
+ case V7_PACKET_ID_IDLE:
+ case V7_PACKET_ID_NEW:
+ /*No finger is reported when packet ID is 'IDLE' or 'New'.
+ An 'IDLE' packet indicates that there's no finger on touchpad.
+ A 'NEW' packet indicates there's finger placed or lifted.
+ Finger position of 'New' packet is not continues from the
+ previous packet.*/
+ fn = 0;
+ break;
+ case V7_PACKET_ID_TWO:
+ if (f->pt_img[0].z == 0) {
+ /*The first finger slot is zero when a non-resting
+ finger lifted and remaining only one resting finger
+ on touchpad. Hardware report the remaining resting
+ finger in second slot. This resting is ignored*/
+ fn = 0;
+ } else if (f->pt_img[1].z == 0) {
+ /* The second finger slot is zero if there's
+ only one finger*/
+ fn = 1;
+ } else {
+ /*All non-resting fingers will be counted to report*/
+ fn = 0;
+ for (i = 0; i < V7_IMG_PT_NUM; i++) {
+ if (priv->pt_attr[i].is_counted)
+ fn++;
+ }
+
+ /*In the case that both fingers are
+ resting fingers, report the first one*/
+ if (!priv->pt_attr[0].is_counted &&
+ !priv->pt_attr[1].is_counted) {
+ fn = 1;
+ }
+ }
+ break;
+ case V7_PACKET_ID_MULTI:
+ /*A packet ID 'MULTI' indicats that at least 3 non-resting
+ finger exist.*/
+ fn = 3 + priv->r.v7.additional_fingers;
+ break;
+ }
+
+ f->fingers = fn;
+}
+
+static void alps_assign_buttons_v7(struct psmouse *psmouse,
+ struct alps_fields *f)
+{
+ struct alps_data *priv = psmouse->private;
+
+ if (priv->phy_btn) {
+ if (!priv->prev_phy_btn) {
+ /* Report a right click as long as there's finger on
+ right button zone. Othrewise, report a left click.*/
+ if (priv->r.v7.rest_right ||
+ priv->pt_attr[0].zone & ZONE_RIGHT_BTN ||
+ priv->pt_attr[1].zone & ZONE_RIGHT_BTN) {
+ f->btn.right = 1;
+ priv->pressed_btn_bits |= RIGHT_BUTTON_BIT;
+ } else {
+ f->btn.left = 1;
+ priv->pressed_btn_bits |= LEFT_BUTTON_BIT;
+ }
+ } else {
+ if (priv->pressed_btn_bits & RIGHT_BUTTON_BIT)
+ f->btn.right = 1;
+ if (priv->pressed_btn_bits & LEFT_BUTTON_BIT)
+ f->btn.left = 1;
+ }
+ } else {
+ priv->pressed_btn_bits = 0;
+ f->btn.right = 0;
+ f->btn.left = 0;
+ }
+
+ priv->prev_phy_btn = priv->phy_btn;
+}
+
+static void alps_process_packet_v7(struct psmouse *psmouse)
+{
+ struct alps_data *priv = psmouse->private;
+ struct alps_fields f = {0};
+ unsigned char *packet = psmouse->packet;
+
+ priv->decode_fields(&f, packet, psmouse);
+
+ if (alps_drop_unsupported_packet_v7(psmouse))
+ return;
+
+ alps_set_pt_attr_v7(psmouse, &f);
+
+ alps_cal_output_finger_num_v7(psmouse, &f);
+
+ alps_assign_buttons_v7(psmouse, &f);
+
+ alps_report_coord_and_btn(psmouse, &f);
+}
+
static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
unsigned char packet[],
bool report_buttons)
@@ -1080,6 +1420,14 @@ static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
return PSMOUSE_BAD_DATA;
}
+ if ((priv->proto_version == ALPS_PROTO_V7 &&
+ !alps_is_valid_package_v7(psmouse))) {
+ psmouse_dbg(psmouse, "refusing packet[%i] = %x\n",
+ psmouse->pktcnt - 1,
+ psmouse->packet[psmouse->pktcnt - 1]);
+ return PSMOUSE_BAD_DATA;
+ }
+
if (psmouse->pktcnt == psmouse->pktsize) {
priv->process_packet(psmouse);
return PSMOUSE_FULL_PACKET;
@@ -1192,6 +1540,22 @@ static int alps_rpt_cmd(struct psmouse *psmouse, int init_command,
return 0;
}
+static int alps_check_valid_firmware_id(unsigned char id[])
+{
+ int valid = 1;
+
+ if (id[0] == 0x73)
+ valid = 1;
+ else if (id[0] == 0x88) {
+ if ((id[1] == 0x07) ||
+ (id[1] == 0x08) ||
+ ((id[1] & 0xf0) == 0xB0))
+ valid = 1;
+ }
+
+ return valid;
+}
+
static int alps_enter_command_mode(struct psmouse *psmouse)
{
unsigned char param[4];
@@ -1201,8 +1565,7 @@ static int alps_enter_command_mode(struct psmouse *psmouse)
return -1;
}
- if ((param[0] != 0x88 || (param[1] != 0x07 && param[1] != 0x08)) &&
- param[0] != 0x73) {
+ if (!alps_check_valid_firmware_id(param)) {
psmouse_dbg(psmouse,
"unknown response while entering command mode\n");
return -1;
@@ -1704,6 +2067,32 @@ error:
return ret;
}
+static int alps_hw_init_v7(struct psmouse *psmouse)
+{
+ struct ps2dev *ps2dev = &psmouse->ps2dev;
+ int reg_val, ret = -1;
+
+ if (alps_enter_command_mode(psmouse) ||
+ alps_command_mode_read_reg(psmouse, 0xc2d9) == -1)
+ goto error;
+
+ if (alps_command_mode_write_reg(psmouse, 0xc2c9, 0x64))
+ goto error;
+
+ reg_val = alps_command_mode_read_reg(psmouse, 0xc2c4);
+ if (reg_val == -1)
+ goto error;
+ if (__alps_command_mode_write_reg(psmouse, reg_val | 0x02))
+ goto error;
+
+ alps_exit_command_mode(psmouse);
+ return ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
+
+error:
+ alps_exit_command_mode(psmouse);
+ return ret;
+}
+
/* Must be in command mode when calling this function */
static int alps_absolute_mode_v4(struct psmouse *psmouse)
{
@@ -1875,6 +2264,7 @@ static void alps_set_defaults(struct alps_data *priv)
priv->set_abs_params = alps_set_abs_params_st;
priv->x_max = 1023;
priv->y_max = 767;
+ priv->slot_number = 1;
break;
case ALPS_PROTO_V3:
priv->hw_init = alps_hw_init_v3;
@@ -1883,6 +2273,7 @@ static void alps_set_defaults(struct alps_data *priv)
priv->decode_fields = alps_decode_pinnacle;
priv->nibble_commands = alps_v3_nibble_commands;
priv->addr_command = PSMOUSE_CMD_RESET_WRAP;
+ priv->slot_number = 2;
break;
case ALPS_PROTO_V4:
priv->hw_init = alps_hw_init_v4;
@@ -1890,6 +2281,7 @@ static void alps_set_defaults(struct alps_data *priv)
priv->set_abs_params = alps_set_abs_params_mt;
priv->nibble_commands = alps_v4_nibble_commands;
priv->addr_command = PSMOUSE_CMD_DISABLE;
+ priv->slot_number = 2;
break;
case ALPS_PROTO_V5:
priv->hw_init = alps_hw_init_dolphin_v1;
@@ -1905,6 +2297,7 @@ static void alps_set_defaults(struct alps_data *priv)
priv->y_max = 660;
priv->x_bits = 23;
priv->y_bits = 12;
+ priv->slot_number = 2;
break;
case ALPS_PROTO_V6:
priv->hw_init = alps_hw_init_v6;
@@ -1913,6 +2306,22 @@ static void alps_set_defaults(struct alps_data *priv)
priv->nibble_commands = alps_v6_nibble_commands;
priv->x_max = 2047;
priv->y_max = 1535;
+ priv->slot_number = 2;
+ break;
+ case ALPS_PROTO_V7:
+ priv->hw_init = alps_hw_init_v7;
+ priv->process_packet = alps_process_packet_v7;
+ priv->decode_fields = alps_decode_packet_v7;
+ priv->set_abs_params = alps_set_abs_params_mt;
+ priv->nibble_commands = alps_v3_nibble_commands;
+ priv->addr_command = PSMOUSE_CMD_RESET_WRAP;
+ priv->x_max = 0xfff;
+ priv->y_max = 0x7ff;
+ priv->resting_zone_y_min = 0x654;
+ priv->byte0 = 0x48;
+ priv->mask0 = 0x48;
+ priv->flags = 0;
+ priv->slot_number = 2;
break;
}
}
@@ -1982,6 +2391,11 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
return -EIO;
else
return 0;
+ } else if (ec[0] == 0x88 && (ec[1] & 0xf0) == 0xB0) {
+ priv->proto_version = ALPS_PROTO_V7;
+ alps_set_defaults(priv);
+
+ return 0;
} else if (ec[0] == 0x88 && ec[1] == 0x08) {
priv->proto_version = ALPS_PROTO_V3;
alps_set_defaults(priv);
@@ -2045,7 +2459,7 @@ static void alps_set_abs_params_mt(struct alps_data *priv,
struct input_dev *dev1)
{
set_bit(INPUT_PROP_SEMI_MT, dev1->propbit);
- input_mt_init_slots(dev1, 2, 0);
+ input_mt_init_slots(dev1, priv->slot_number, 0);
input_set_abs_params(dev1, ABS_MT_POSITION_X, 0, priv->x_max, 0, 0);
input_set_abs_params(dev1, ABS_MT_POSITION_Y, 0, priv->y_max, 0, 0);
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index 03f88b6..5d2f9ea 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -18,11 +18,36 @@
#define ALPS_PROTO_V4 4
#define ALPS_PROTO_V5 5
#define ALPS_PROTO_V6 6
+#define ALPS_PROTO_V7 7
+
+#define MAX_IMG_PT_NUM 5
+#define V7_IMG_PT_NUM 2
+
+#define ZONE_NORMAL 0x01
+#define ZONE_RESTING 0x02
+#define ZONE_LEFT_BTN 0x04
+#define ZONE_RIGHT_BTN 0x08
#define DOLPHIN_COUNT_PER_ELECTRODE 64
#define DOLPHIN_PROFILE_XOFFSET 8 /* x-electrode offset */
#define DOLPHIN_PROFILE_YOFFSET 1 /* y-electrode offset */
+/*
+ * enum V7_PACKET_ID - defines the packet type for V7
+ * V7_PACKET_ID_IDLE: There's no finger and no button activity.
+ * V7_PACKET_ID_TWO: There's one or two non-resting fingers on touchpad
+ * or there's button activities.
+ * V7_PACKET_ID_MULTI: There are at least three non-resting fingers.
+ * V7_PACKET_ID_NEW: The finger position in slot is not continues from
+ * previous packet.
+*/
+enum V7_PACKET_ID {
+ V7_PACKET_ID_IDLE,
+ V7_PACKET_ID_TWO,
+ V7_PACKET_ID_MULTI,
+ V7_PACKET_ID_NEW,
+};
+
/**
* struct alps_model_info - touchpad ID table
* @signature: E7 response string to match.
@@ -66,15 +91,7 @@ struct alps_nibble_commands {
};
/**
- * struct alps_fields - decoded version of the report packet
- * @x_map: Bitmap of active X positions for MT.
- * @y_map: Bitmap of active Y positions for MT.
- * @fingers: Number of fingers for MT.
- * @x: X position for ST.
- * @y: Y position for ST.
- * @z: Z position for ST.
- * @first_mp: Packet is the first of a multi-packet report.
- * @is_mp: Packet is part of a multi-packet report.
+ * struct alps_btn - decoded version of the button status
* @left: Left touchpad button is active.
* @right: Right touchpad button is active.
* @middle: Middle touchpad button is active.
@@ -82,16 +99,7 @@ struct alps_nibble_commands {
* @ts_right: Right trackstick button is active.
* @ts_middle: Middle trackstick button is active.
*/
-struct alps_fields {
- unsigned int x_map;
- unsigned int y_map;
- unsigned int fingers;
- unsigned int x;
- unsigned int y;
- unsigned int z;
- unsigned int first_mp:1;
- unsigned int is_mp:1;
-
+struct alps_btn {
unsigned int left:1;
unsigned int right:1;
unsigned int middle:1;
@@ -102,6 +110,69 @@ struct alps_fields {
};
/**
+ * struct alps_btn - decoded version of the X Y Z postion for ST.
+ * @x: X position for ST.
+ * @y: Y position for ST.
+ * @z: Z position for ST.
+ */
+struct alps_abs_data {
+ unsigned int x;
+ unsigned int y;
+ unsigned int z;
+};
+
+/**
+ * struct alps_fields - decoded version of the report packet
+ * @fingers: Number of fingers for MT.
+ * @pt: X Y Z postion for ST.
+ * @pt: X Y Z postion for image MT.
+ * @x_map: Bitmap of active X positions for MT.
+ * @y_map: Bitmap of active Y positions for MT.
+ * @first_mp: Packet is the first of a multi-packet report.
+ * @is_mp: Packet is part of a multi-packet report.
+ * @btn: Button activity status
+ */
+struct alps_fields {
+ unsigned int fingers;
+ struct alps_abs_data pt;
+ struct alps_abs_data pt_img[MAX_IMG_PT_NUM];
+ unsigned int x_map;
+ unsigned int y_map;
+ unsigned int first_mp:1;
+ unsigned int is_mp:1;
+ struct alps_btn btn;
+};
+
+/**
+ * struct v7_raw - data decoded from raw packet for V7.
+ * @pkt_id: An id that specifies the type of packet.
+ * @additional_fingers: Number of additional finger that is neighter included
+ * in pt slot nor reflected in rest_left and rest_right flag of data packet.
+ * @rest_left: There are fingers on left resting zone.
+ * @rest_right: There are fingers on right resting zone.
+ */
+struct v7_raw {
+ unsigned char pkt_id;
+ unsigned int additional_fingers;
+ unsigned char rest_left;
+ unsigned char rest_right;
+};
+
+/**
+ * struct alps_bl_pt_attr - generic attributes of touch points for buttonless device
+ * @zone: The part of touchpad that the touch point locates
+ * @is_counted: The touch point is not a resting finger.
+ * @is_init_pt_got: The touch down point is got.
+ * @init_pt: The X Y Z position of the touch down point.
+ */
+struct alps_bl_pt_attr {
+ unsigned char zone;
+ unsigned char is_counted;
+ unsigned char is_init_pt_got;
+ struct alps_abs_data init_pt;
+};
+
+/**
* struct alps_data - private data structure for the ALPS driver
* @dev2: "Relative" device used to report trackstick or mouse activity.
* @phys: Physical path for the relative device.
@@ -116,8 +187,10 @@ struct alps_fields {
* @flags: Additional device capabilities (passthrough port, trackstick, etc.).
* @x_max: Largest possible X position value.
* @y_max: Largest possible Y position value.
+ * @resting_zone_y_min: Smallest Y postion value of the bottom resting zone.
* @x_bits: Number of X bits in the MT bitmap.
* @y_bits: Number of Y bits in the MT bitmap.
+ * @img_fingers: Number of image fingers.
* @hw_init: Protocol-specific hardware init function.
* @process_packet: Protocol-specific function to process a report packet.
* @decode_fields: Protocol-specific function to read packet bitfields.
@@ -132,6 +205,11 @@ struct alps_fields {
* @fingers: Number of fingers from last MT report.
* @quirks: Bitmap of ALPS_QUIRK_*.
* @timer: Timer for flushing out the final report packet in the stream.
+ * @v7: Data decoded from raw packet for V7
+ * @phy_btn: Physical button is active.
+ * @prev_phy_btn: Physical button of previous packet is active.
+ * @pressed_btn_bits: Pressed positon of button zone
+ * @pt_attr: Generic attributes of touch points for buttonless device.
*/
struct alps_data {
struct input_dev *dev2;
@@ -145,8 +223,10 @@ struct alps_data {
unsigned char flags;
int x_max;
int y_max;
+ int resting_zone_y_min;
int x_bits;
int y_bits;
+ unsigned char slot_number;
int (*hw_init)(struct psmouse *psmouse);
void (*process_packet)(struct psmouse *psmouse);
@@ -161,6 +241,15 @@ struct alps_data {
int fingers;
u8 quirks;
struct timer_list timer;
+
+ /* these are used for buttonless touchpad*/
+ union {
+ struct v7_raw v7;
+ } r;
+ unsigned char phy_btn;
+ unsigned char prev_phy_btn;
+ unsigned char pressed_btn_bits;
+ struct alps_bl_pt_attr pt_attr[MAX_IMG_PT_NUM];
};
#define ALPS_QUIRK_TRACKSTICK_BUTTONS 1 /* trakcstick buttons in trackstick packet */
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH] input: synaptics-rmi4 - fix F01 DOM formatting
From: Dmitry Torokhov @ 2014-01-10 6:27 UTC (permalink / raw)
To: Christopher Heiny
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <1389298893-4043-1-git-send-email-cheiny@synaptics.com>
On Thu, Jan 09, 2014 at 12:21:33PM -0800, Christopher Heiny wrote:
> Use a sensible format string for the date of manufacture formatting.
>
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Applied, thank you.
>
> ---
>
> drivers/input/rmi4/rmi_f01.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 1cb11ea..9e6a578 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -50,7 +50,7 @@ struct f01_basic_properties {
> bool has_lts;
> bool has_adjustable_doze;
> bool has_adjustable_doze_holdoff;
> - char dom[9]; /* YYYYMMDD + '\0' */
> + char dom[11]; /* YYYY/MM/DD + '\0' */
> u8 product_id[RMI_PRODUCT_ID_LENGTH + 1];
> u16 productinfo;
> };
> @@ -190,8 +190,7 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
> props->has_adjustable_doze_holdoff =
> basic_query[1] & RMI_F01_QRY1_HAS_ADJ_DOZE_HOFF;
>
> - snprintf(props->dom, sizeof(props->dom),
> - "20%02x%02x%02x",
> + snprintf(props->dom, sizeof(props->dom), "20%02d/%02d/%02d",
> basic_query[5] & RMI_F01_QRY5_YEAR_MASK,
> basic_query[6] & RMI_F01_QRY6_MONTH_MASK,
> basic_query[7] & RMI_F01_QRY7_DAY_MASK);
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: synaptics-rmi4 - rmi_driver.h tidy up
From: Dmitry Torokhov @ 2014-01-10 6:28 UTC (permalink / raw)
To: Christopher Heiny
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <1389319178-27751-1-git-send-email-cheiny@synaptics.com>
On Thu, Jan 09, 2014 at 05:59:38PM -0800, Christopher Heiny wrote:
> This is a trivial change to remove an unused macro, and change CONFIG_PM to
> CONFIG_PM_SLEEP.
>
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Applied, thank you.
>
> ---
>
> drivers/input/rmi4/rmi_driver.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index 4d82b90..ad3ee33 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -24,8 +24,6 @@
> .attrs = _attrs, \
> }
>
> -#define attrify(nm) (&dev_attr_##nm.attr)
> -
> #define PDT_PROPERTIES_LOCATION 0x00EF
> #define BSR_LOCATION 0x00FE
>
> @@ -62,7 +60,7 @@ struct rmi_driver_data {
> u8 bsr;
>
> bool enabled;
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_PM_SLEEP
> bool suspended;
> struct mutex suspend_mutex;
>
--
Dmitry
^ permalink raw reply
* [PATCH 2/4] Input: synaptics-rmi4 - rework transport device allocation
From: Dmitry Torokhov @ 2014-01-10 7:44 UTC (permalink / raw)
To: Christopher Heiny
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Linux Input, Linux Kernel
In-Reply-To: <1389339867-8399-1-git-send-email-dmitry.torokhov@gmail.com>
Instead of allocating common and private part of transport device
separately make private wrap common part and get rid of private data
pointer in the transport device.
Also rename rmi_i2c_data -> rmi_i2c_xport and data -> rmi_i2c.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/rmi4/rmi_bus.h | 3 --
drivers/input/rmi4/rmi_i2c.c | 112 +++++++++++++++++++++----------------------
2 files changed, 56 insertions(+), 59 deletions(-)
diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
index ccf26dc..decb479 100644
--- a/drivers/input/rmi4/rmi_bus.h
+++ b/drivers/input/rmi4/rmi_bus.h
@@ -165,7 +165,6 @@ struct rmi_transport_stats {
* default irq_thread implementation.
* @hard_irq: if not NULL, the sensor driver will use this for the hard IRQ
* handling
- * @data: Private data pointer
* @proto_name: name of the transport protocol (SPI, i2c, etc)
* @ops: pointer to transport operations implementation
* @stats: transport statistics
@@ -181,8 +180,6 @@ struct rmi_transport_dev {
irqreturn_t (*irq_thread)(int irq, void *p);
irqreturn_t (*hard_irq)(int irq, void *p);
- void *data;
-
const char *proto_name;
const struct rmi_transport_ops *ops;
struct rmi_transport_stats stats;
diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index 40badf3..cdc8527 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -17,22 +17,25 @@
#define BUFFER_SIZE_INCREMENT 32
/**
- * struct rmi_i2c_data - stores information for i2c communication
+ * struct rmi_i2c_xport - stores information for i2c communication
+ *
+ * @xport: The transport interface structure
*
* @page_mutex: Locks current page to avoid changing pages in unexpected ways.
* @page: Keeps track of the current virtual page
- * @xport: Pointer to the transport interface
*
* @tx_buf: Buffer used for transmitting data to the sensor over i2c.
* @tx_buf_size: Size of the buffer
*/
-struct rmi_i2c_data {
+struct rmi_i2c_xport {
+ struct rmi_transport_dev xport;
+ struct i2c_client *client;
+
struct mutex page_mutex;
int page;
- struct rmi_transport_dev *xport;
u8 *tx_buf;
- int tx_buf_size;
+ size_t tx_buf_size;
};
#define RMI_PAGE_SELECT_REGISTER 0xff
@@ -52,10 +55,10 @@ struct rmi_i2c_data {
*
* Returns zero on success, non-zero on failure.
*/
-static int rmi_set_page(struct rmi_transport_dev *xport, u8 page)
+static int rmi_set_page(struct rmi_i2c_xport *rmi_i2c, u8 page)
{
- struct i2c_client *client = to_i2c_client(xport->dev);
- struct rmi_i2c_data *data = xport->data;
+ struct rmi_transport_dev *xport = &rmi_i2c->xport;
+ struct i2c_client *client = rmi_i2c->client;
u8 txbuf[2] = {RMI_PAGE_SELECT_REGISTER, page};
int retval;
@@ -70,37 +73,40 @@ static int rmi_set_page(struct rmi_transport_dev *xport, u8 page)
"%s: set page failed: %d.", __func__, retval);
return (retval < 0) ? retval : -EIO;
}
- data->page = page;
+ rmi_i2c->page = page;
return 0;
}
static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
const void *buf, size_t len)
{
- struct i2c_client *client = to_i2c_client(xport->dev);
- struct rmi_i2c_data *data = xport->data;
+ struct rmi_i2c_xport *rmi_i2c =
+ container_of(xport, struct rmi_i2c_xport, xport);
+ struct i2c_client *client = rmi_i2c->client;
size_t tx_size = len + 1;
int retval;
- mutex_lock(&data->page_mutex);
-
- if (!data->tx_buf || data->tx_buf_size < tx_size) {
- if (data->tx_buf)
- devm_kfree(&client->dev, data->tx_buf);
- data->tx_buf_size = tx_size + BUFFER_SIZE_INCREMENT;
- data->tx_buf = devm_kzalloc(&client->dev, data->tx_buf_size,
- GFP_KERNEL);
- if (!data->tx_buf) {
- data->tx_buf_size = 0;
+ mutex_lock(&rmi_i2c->page_mutex);
+
+ if (!rmi_i2c->tx_buf || rmi_i2c->tx_buf_size < tx_size) {
+ if (rmi_i2c->tx_buf)
+ devm_kfree(&client->dev, rmi_i2c->tx_buf);
+ rmi_i2c->tx_buf_size = tx_size + BUFFER_SIZE_INCREMENT;
+ rmi_i2c->tx_buf = devm_kzalloc(&client->dev,
+ rmi_i2c->tx_buf_size,
+ GFP_KERNEL);
+ if (!rmi_i2c->tx_buf) {
+ rmi_i2c->tx_buf_size = 0;
retval = -ENOMEM;
goto exit;
}
}
- data->tx_buf[0] = addr & 0xff;
- memcpy(data->tx_buf + 1, buf, len);
- if (RMI_I2C_PAGE(addr) != data->page) {
- retval = rmi_set_page(xport, RMI_I2C_PAGE(addr));
+ rmi_i2c->tx_buf[0] = addr & 0xff;
+ memcpy(rmi_i2c->tx_buf + 1, buf, len);
+
+ if (RMI_I2C_PAGE(addr) != rmi_i2c->page) {
+ retval = rmi_set_page(rmi_i2c, RMI_I2C_PAGE(addr));
if (retval < 0)
goto exit;
}
@@ -110,29 +116,30 @@ static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
xport->stats.tx_count++;
xport->stats.tx_bytes += tx_size;
- retval = i2c_master_send(client, data->tx_buf, tx_size);
+ retval = i2c_master_send(client, rmi_i2c->tx_buf, tx_size);
if (retval < 0)
xport->stats.tx_errs++;
else
retval--; /* don't count the address byte */
exit:
- mutex_unlock(&data->page_mutex);
+ mutex_unlock(&rmi_i2c->page_mutex);
return retval;
}
static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
void *buf, size_t len)
{
- struct i2c_client *client = to_i2c_client(xport->dev);
- struct rmi_i2c_data *data = xport->data;
+ struct rmi_i2c_xport *rmi_i2c =
+ container_of(xport, struct rmi_i2c_xport, xport);
+ struct i2c_client *client = rmi_i2c->client;
u8 txbuf[1] = {addr & 0xff};
int retval;
- mutex_lock(&data->page_mutex);
+ mutex_lock(&rmi_i2c->page_mutex);
- if (RMI_I2C_PAGE(addr) != data->page) {
- retval = rmi_set_page(xport, RMI_I2C_PAGE(addr));
+ if (RMI_I2C_PAGE(addr) != rmi_i2c->page) {
+ retval = rmi_set_page(rmi_i2c, RMI_I2C_PAGE(addr));
if (retval < 0)
goto exit;
}
@@ -160,7 +167,7 @@ static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
len, addr, (int)len, buf);
exit:
- mutex_unlock(&data->page_mutex);
+ mutex_unlock(&rmi_i2c->page_mutex);
return retval;
}
@@ -174,14 +181,14 @@ static int rmi_i2c_probe(struct i2c_client *client,
{
const struct rmi_device_platform_data *pdata =
dev_get_platdata(&client->dev);
- struct rmi_transport_dev *xport;
- struct rmi_i2c_data *data;
+ struct rmi_i2c_xport *rmi_i2c;
int retval;
if (!pdata) {
dev_err(&client->dev, "no platform data\n");
return -EINVAL;
}
+
dev_dbg(&client->dev, "Probing %s at %#02x (GPIO %d).\n",
pdata->sensor_name ? pdata->sensor_name : "-no name-",
client->addr, pdata->attn_gpio);
@@ -202,44 +209,36 @@ static int rmi_i2c_probe(struct i2c_client *client,
}
}
- xport = devm_kzalloc(&client->dev, sizeof(struct rmi_transport_dev),
+ rmi_i2c = devm_kzalloc(&client->dev, sizeof(struct rmi_i2c_xport),
GFP_KERNEL);
-
- if (!xport)
+ if (!rmi_i2c)
return -ENOMEM;
- data = devm_kzalloc(&client->dev, sizeof(struct rmi_i2c_data),
- GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- data->xport = xport;
-
- xport->data = data;
- xport->dev = &client->dev;
+ rmi_i2c->client = client;
+ mutex_init(&rmi_i2c->page_mutex);
- xport->proto_name = "i2c";
- xport->ops = &rmi_i2c_ops;
-
- mutex_init(&data->page_mutex);
+ rmi_i2c->xport.dev = &client->dev;
+ rmi_i2c->xport.proto_name = "i2c";
+ rmi_i2c->xport.ops = &rmi_i2c_ops;
/*
* Setting the page to zero will (a) make sure the PSR is in a
* known state, and (b) make sure we can talk to the device.
*/
- retval = rmi_set_page(xport, 0);
+ retval = rmi_set_page(rmi_i2c, 0);
if (retval) {
dev_err(&client->dev, "Failed to set page select to 0.\n");
return retval;
}
- retval = rmi_register_transport_device(xport);
+ retval = rmi_register_transport_device(&rmi_i2c->xport);
if (retval) {
dev_err(&client->dev, "Failed to register transport driver at 0x%.2X.\n",
client->addr);
goto err_gpio;
}
- i2c_set_clientdata(client, xport);
+
+ i2c_set_clientdata(client, rmi_i2c);
dev_info(&client->dev, "registered rmi i2c driver at %#04x.\n",
client->addr);
@@ -248,16 +247,17 @@ static int rmi_i2c_probe(struct i2c_client *client,
err_gpio:
if (pdata->gpio_config)
pdata->gpio_config(pdata->gpio_data, false);
+
return retval;
}
static int rmi_i2c_remove(struct i2c_client *client)
{
- struct rmi_transport_dev *xport = i2c_get_clientdata(client);
const struct rmi_device_platform_data *pdata =
dev_get_platdata(&client->dev);
+ struct rmi_i2c_xport *rmi_i2c = i2c_get_clientdata(client);
- rmi_unregister_transport_device(xport);
+ rmi_unregister_transport_device(&rmi_i2c->xport);
if (pdata->gpio_config)
pdata->gpio_config(pdata->gpio_data, false);
--
1.8.4.2
^ permalink raw reply related
* [PATCH 3/4] Input: synaptics-rmi4 - fix I2C functionality check
From: Dmitry Torokhov @ 2014-01-10 7:44 UTC (permalink / raw)
To: Christopher Heiny
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Linux Input, Linux Kernel
In-Reply-To: <1389339867-8399-1-git-send-email-dmitry.torokhov@gmail.com>
When adapter does not support required functionality (I2C_FUNC_I2C) we were
returning 0 to the upper layers, making them believe that device bound
successfully.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/rmi4/rmi_i2c.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index cdc8527..c176218 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -193,11 +193,10 @@ static int rmi_i2c_probe(struct i2c_client *client,
pdata->sensor_name ? pdata->sensor_name : "-no name-",
client->addr, pdata->attn_gpio);
- retval = i2c_check_functionality(client->adapter, I2C_FUNC_I2C);
- if (!retval) {
- dev_err(&client->dev, "i2c_check_functionality error %d.\n",
- retval);
- return retval;
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+ dev_err(&client->dev,
+ "adapter does not support required functionality.\n");
+ return -ENODEV;
}
if (pdata->gpio_config) {
--
1.8.4.2
^ permalink raw reply related
* [PATCH 4/4] Input: synaptics-rmi4 - switch to using i2c_transfer()
From: Dmitry Torokhov @ 2014-01-10 7:44 UTC (permalink / raw)
To: Christopher Heiny
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Linux Input, Linux Kernel
In-Reply-To: <1389339867-8399-1-git-send-email-dmitry.torokhov@gmail.com>
Instead of using 2 separate transactions when reading from the device let's
use i2c_transfer. Because we now have single point of failure I had to
change how we collect statistics. I elected to drop control data from the
stats and only track number of bytes read/written for the device data.
Also, since we are not prepared to deal with short reads and writes change
read_block_data and write_block_data to indicate error if we detect short
transfers.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/rmi4/rmi_i2c.c | 71 ++++++++++++++++++++++++--------------------
1 file changed, 39 insertions(+), 32 deletions(-)
diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index c176218..51f5bc8 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -57,22 +57,17 @@ struct rmi_i2c_xport {
*/
static int rmi_set_page(struct rmi_i2c_xport *rmi_i2c, u8 page)
{
- struct rmi_transport_dev *xport = &rmi_i2c->xport;
struct i2c_client *client = rmi_i2c->client;
u8 txbuf[2] = {RMI_PAGE_SELECT_REGISTER, page};
int retval;
- dev_dbg(&client->dev, "writes 3 bytes: %02x %02x\n",
- txbuf[0], txbuf[1]);
- xport->stats.tx_count++;
- xport->stats.tx_bytes += sizeof(txbuf);
retval = i2c_master_send(client, txbuf, sizeof(txbuf));
if (retval != sizeof(txbuf)) {
- xport->stats.tx_errs++;
dev_err(&client->dev,
"%s: set page failed: %d.", __func__, retval);
return (retval < 0) ? retval : -EIO;
}
+
rmi_i2c->page = page;
return 0;
}
@@ -107,22 +102,27 @@ static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
if (RMI_I2C_PAGE(addr) != rmi_i2c->page) {
retval = rmi_set_page(rmi_i2c, RMI_I2C_PAGE(addr));
- if (retval < 0)
+ if (retval)
goto exit;
}
+ retval = i2c_master_send(client, rmi_i2c->tx_buf, tx_size);
+ if (retval == tx_size)
+ retval = 0;
+ else if (retval >= 0)
+ retval = -EIO;
+
+exit:
dev_dbg(&client->dev,
- "writes %zd bytes at %#06x: %*ph\n", len, addr, (int)len, buf);
+ "write %zd bytes at %#06x: %d (%*ph)\n",
+ len, addr, retval, (int)len, buf);
xport->stats.tx_count++;
- xport->stats.tx_bytes += tx_size;
- retval = i2c_master_send(client, rmi_i2c->tx_buf, tx_size);
- if (retval < 0)
+ if (retval)
xport->stats.tx_errs++;
else
- retval--; /* don't count the address byte */
+ xport->stats.tx_bytes += len;
-exit:
mutex_unlock(&rmi_i2c->page_mutex);
return retval;
}
@@ -133,40 +133,47 @@ static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
struct rmi_i2c_xport *rmi_i2c =
container_of(xport, struct rmi_i2c_xport, xport);
struct i2c_client *client = rmi_i2c->client;
- u8 txbuf[1] = {addr & 0xff};
+ u8 addr_offset = addr & 0xff;
int retval;
+ struct i2c_msg msgs[] = {
+ {
+ .addr = client->addr,
+ .len = sizeof(addr_offset),
+ .buf = &addr_offset,
+ },
+ {
+ .addr = client->addr,
+ .flags = I2C_M_RD,
+ .len = len,
+ .buf = buf,
+ },
+ };
mutex_lock(&rmi_i2c->page_mutex);
if (RMI_I2C_PAGE(addr) != rmi_i2c->page) {
retval = rmi_set_page(rmi_i2c, RMI_I2C_PAGE(addr));
- if (retval < 0)
+ if (retval)
goto exit;
}
- dev_dbg(&client->dev, "writes 1 bytes: %02x\n", txbuf[0]);
+ retval = i2c_transfer(client->adapter, msgs, sizeof(msgs));
+ if (retval == sizeof(msgs))
+ retval = 0; /* success */
+ else if (retval >= 0)
+ retval = -EIO;
- xport->stats.tx_count++;
- xport->stats.tx_bytes += sizeof(txbuf);
- retval = i2c_master_send(client, txbuf, sizeof(txbuf));
- if (retval != sizeof(txbuf)) {
- xport->stats.tx_errs++;
- retval = (retval < 0) ? retval : -EIO;
- goto exit;
- }
+exit:
+ dev_dbg(&client->dev,
+ "read %zd bytes at %#06x: %d (%*ph)\n",
+ len, addr, retval, (int)len, buf);
xport->stats.rx_count++;
- xport->stats.rx_bytes += len;
-
- retval = i2c_master_recv(client, buf, len);
- if (retval < 0)
+ if (retval)
xport->stats.rx_errs++;
else
- dev_dbg(&client->dev,
- "read %zd bytes at %#06x: %*ph\n",
- len, addr, (int)len, buf);
+ xport->stats.rx_bytes += len;
-exit:
mutex_unlock(&rmi_i2c->page_mutex);
return retval;
}
--
1.8.4.2
^ permalink raw reply related
* [PATCH 1/4] Input: synaptics-rmi4 - split of transport ops into a separate structure
From: Dmitry Torokhov @ 2014-01-10 7:44 UTC (permalink / raw)
To: Christopher Heiny
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Linux Input, Linux Kernel
Split off transport operations from rmi_transport_dev into a separate
structure that will be shared between all devices using the same transport
and use const pointer to access it.
Change signature on transport methods so that length is using the proper
tyep - size_t.
Also rename rmi_transport_info to rmi_transport_stats and move protocol
name (which is the only immutable piece of data there) into the transport
device itself.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/rmi4/rmi_bus.h | 64 ++++++++++++++++++++++++-----------------
drivers/input/rmi4/rmi_driver.c | 8 +++---
drivers/input/rmi4/rmi_i2c.c | 49 ++++++++++++++++---------------
3 files changed, 67 insertions(+), 54 deletions(-)
diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
index 3e8b57a..ccf26dc 100644
--- a/drivers/input/rmi4/rmi_bus.h
+++ b/drivers/input/rmi4/rmi_bus.h
@@ -135,26 +135,25 @@ struct rmi_driver {
#define to_rmi_driver(d) \
container_of(d, struct rmi_driver, driver);
-/** struct rmi_transport_info - diagnostic information about the RMI transport
+/**
+ * struct rmi_transport_stats - diagnostic information about the RMI transport
* device, used in the xport_info debugfs file.
*
* @proto String indicating the protocol being used.
* @tx_count Number of transmit operations.
- * @tx_bytes Number of bytes transmitted.
* @tx_errs Number of errors encountered during transmit operations.
+ * @tx_bytes Number of bytes transmitted.
* @rx_count Number of receive operations.
- * @rx_bytes Number of bytes received.
* @rx_errs Number of errors encountered during receive operations.
- * @att_count Number of times ATTN assertions have been handled.
+ * @rx_bytes Number of bytes received.
*/
-struct rmi_transport_info {
- const char *proto;
- long tx_count;
- long tx_bytes;
- long tx_errs;
- long rx_count;
- long rx_bytes;
- long rx_errs;
+struct rmi_transport_stats {
+ unsigned long tx_count;
+ unsigned long tx_errs;
+ size_t tx_bytes;
+ unsigned long rx_count;
+ unsigned long rx_errs;
+ size_t rx_bytes;
};
/**
@@ -162,13 +161,14 @@ struct rmi_transport_info {
*
* @dev: Pointer to the communication device, e.g. i2c or spi
* @rmi_dev: Pointer to the RMI device
- * @write_block: Writing a block of data to the specified address
- * @read_block: Read a block of data from the specified address.
* @irq_thread: if not NULL, the sensor driver will use this instead of the
* default irq_thread implementation.
* @hard_irq: if not NULL, the sensor driver will use this for the hard IRQ
* handling
* @data: Private data pointer
+ * @proto_name: name of the transport protocol (SPI, i2c, etc)
+ * @ops: pointer to transport operations implementation
+ * @stats: transport statistics
*
* The RMI transport device implements the glue between different communication
* buses such as I2C and SPI.
@@ -178,20 +178,30 @@ struct rmi_transport_dev {
struct device *dev;
struct rmi_device *rmi_dev;
- int (*write_block)(struct rmi_transport_dev *xport, u16 addr,
- const void *buf, const int len);
- int (*read_block)(struct rmi_transport_dev *xport, u16 addr,
- void *buf, const int len);
-
- int (*enable_device) (struct rmi_transport_dev *xport);
- void (*disable_device) (struct rmi_transport_dev *xport);
-
irqreturn_t (*irq_thread)(int irq, void *p);
irqreturn_t (*hard_irq)(int irq, void *p);
void *data;
- struct rmi_transport_info info;
+ const char *proto_name;
+ const struct rmi_transport_ops *ops;
+ struct rmi_transport_stats stats;
+};
+
+/**
+ * struct rmi_transport_ops - defines transport protocol operations.
+ *
+ * @write_block: Writing a block of data to the specified address
+ * @read_block: Read a block of data from the specified address.
+ */
+struct rmi_transport_ops {
+ int (*write_block)(struct rmi_transport_dev *xport, u16 addr,
+ const void *buf, size_t len);
+ int (*read_block)(struct rmi_transport_dev *xport, u16 addr,
+ void *buf, size_t len);
+
+ int (*enable_device) (struct rmi_transport_dev *xport);
+ void (*disable_device) (struct rmi_transport_dev *xport);
};
/**
@@ -232,7 +242,7 @@ bool rmi_is_physical_device(struct device *dev);
*/
static inline int rmi_read(struct rmi_device *d, u16 addr, void *buf)
{
- return d->xport->read_block(d->xport, addr, buf, 1);
+ return d->xport->ops->read_block(d->xport, addr, buf, 1);
}
/**
@@ -248,7 +258,7 @@ static inline int rmi_read(struct rmi_device *d, u16 addr, void *buf)
static inline int rmi_read_block(struct rmi_device *d, u16 addr, void *buf,
const int len)
{
- return d->xport->read_block(d->xport, addr, buf, len);
+ return d->xport->ops->read_block(d->xport, addr, buf, len);
}
/**
@@ -262,7 +272,7 @@ static inline int rmi_read_block(struct rmi_device *d, u16 addr, void *buf,
*/
static inline int rmi_write(struct rmi_device *d, u16 addr, const u8 data)
{
- return d->xport->write_block(d->xport, addr, &data, 1);
+ return d->xport->ops->write_block(d->xport, addr, &data, 1);
}
/**
@@ -278,7 +288,7 @@ static inline int rmi_write(struct rmi_device *d, u16 addr, const u8 data)
static inline int rmi_write_block(struct rmi_device *d, u16 addr,
const void *buf, const int len)
{
- return d->xport->write_block(d->xport, addr, buf, len);
+ return d->xport->ops->write_block(d->xport, addr, buf, len);
}
int rmi_register_transport_device(struct rmi_transport_dev *xport);
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index eb790ff..3483e5b 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -126,8 +126,8 @@ static void disable_sensor(struct rmi_device *rmi_dev)
if (!data->irq)
disable_polling(rmi_dev);
- if (rmi_dev->xport->disable_device)
- rmi_dev->xport->disable_device(rmi_dev->xport);
+ if (rmi_dev->xport->ops->disable_device)
+ rmi_dev->xport->ops->disable_device(rmi_dev->xport);
if (data->irq) {
disable_irq(data->irq);
@@ -146,8 +146,8 @@ static int enable_sensor(struct rmi_device *rmi_dev)
if (data->enabled)
return 0;
- if (rmi_dev->xport->enable_device) {
- retval = rmi_dev->xport->enable_device(rmi_dev->xport);
+ if (rmi_dev->xport->ops->enable_device) {
+ retval = rmi_dev->xport->ops->enable_device(rmi_dev->xport);
if (retval)
return retval;
}
diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index 12aea8c..40badf3 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -61,11 +61,11 @@ static int rmi_set_page(struct rmi_transport_dev *xport, u8 page)
dev_dbg(&client->dev, "writes 3 bytes: %02x %02x\n",
txbuf[0], txbuf[1]);
- xport->info.tx_count++;
- xport->info.tx_bytes += sizeof(txbuf);
+ xport->stats.tx_count++;
+ xport->stats.tx_bytes += sizeof(txbuf);
retval = i2c_master_send(client, txbuf, sizeof(txbuf));
if (retval != sizeof(txbuf)) {
- xport->info.tx_errs++;
+ xport->stats.tx_errs++;
dev_err(&client->dev,
"%s: set page failed: %d.", __func__, retval);
return (retval < 0) ? retval : -EIO;
@@ -75,12 +75,12 @@ static int rmi_set_page(struct rmi_transport_dev *xport, u8 page)
}
static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
- const void *buf, const int len)
+ const void *buf, size_t len)
{
struct i2c_client *client = to_i2c_client(xport->dev);
struct rmi_i2c_data *data = xport->data;
+ size_t tx_size = len + 1;
int retval;
- int tx_size = len + 1;
mutex_lock(&data->page_mutex);
@@ -106,13 +106,13 @@ static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
}
dev_dbg(&client->dev,
- "writes %d bytes at %#06x: %*ph\n", len, addr, len, buf);
+ "writes %zd bytes at %#06x: %*ph\n", len, addr, (int)len, buf);
- xport->info.tx_count++;
- xport->info.tx_bytes += tx_size;
+ xport->stats.tx_count++;
+ xport->stats.tx_bytes += tx_size;
retval = i2c_master_send(client, data->tx_buf, tx_size);
if (retval < 0)
- xport->info.tx_errs++;
+ xport->stats.tx_errs++;
else
retval--; /* don't count the address byte */
@@ -121,9 +121,8 @@ exit:
return retval;
}
-
static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
- void *buf, const int len)
+ void *buf, size_t len)
{
struct i2c_client *client = to_i2c_client(xport->dev);
struct rmi_i2c_data *data = xport->data;
@@ -140,31 +139,36 @@ static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
dev_dbg(&client->dev, "writes 1 bytes: %02x\n", txbuf[0]);
- xport->info.tx_count++;
- xport->info.tx_bytes += sizeof(txbuf);
+ xport->stats.tx_count++;
+ xport->stats.tx_bytes += sizeof(txbuf);
retval = i2c_master_send(client, txbuf, sizeof(txbuf));
if (retval != sizeof(txbuf)) {
- xport->info.tx_errs++;
+ xport->stats.tx_errs++;
retval = (retval < 0) ? retval : -EIO;
goto exit;
}
- retval = i2c_master_recv(client, buf, len);
+ xport->stats.rx_count++;
+ xport->stats.rx_bytes += len;
- xport->info.rx_count++;
- xport->info.rx_bytes += len;
+ retval = i2c_master_recv(client, buf, len);
if (retval < 0)
- xport->info.rx_errs++;
+ xport->stats.rx_errs++;
else
dev_dbg(&client->dev,
- "read %d bytes at %#06x: %*ph\n",
- len, addr, len, buf);
+ "read %zd bytes at %#06x: %*ph\n",
+ len, addr, (int)len, buf);
exit:
mutex_unlock(&data->page_mutex);
return retval;
}
+static const struct rmi_transport_ops rmi_i2c_ops = {
+ .write_block = rmi_i2c_write_block,
+ .read_block = rmi_i2c_read_block,
+};
+
static int rmi_i2c_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -214,9 +218,8 @@ static int rmi_i2c_probe(struct i2c_client *client,
xport->data = data;
xport->dev = &client->dev;
- xport->write_block = rmi_i2c_write_block;
- xport->read_block = rmi_i2c_read_block;
- xport->info.proto = "i2c";
+ xport->proto_name = "i2c";
+ xport->ops = &rmi_i2c_ops;
mutex_init(&data->page_mutex);
--
1.8.4.2
^ permalink raw reply related
* Re: [PATCHv6] staging/iio/adc: change the MXS touchscreen driver implementation
From: Jürgen Beisert @ 2014-01-10 8:55 UTC (permalink / raw)
To: Alexandre Belloni
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, marex-ynQEQJNshbs,
fabio.estevam-KZfg59tc24xl57MIdRCFDg,
jic23-KWPb1pKIrIJaa/9Udqfwiw, linux-input-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <52CEA4AA.8050503-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hi Alexandre,
On Thursday 09 January 2014 14:31:22 Alexandre Belloni wrote:
> Sorry to chime in only now but it seems that this series is breaking the
> touchscreen calibration on 3.13 (and -rc7 is out so it might be too
> late).
>
> At first, I though I became a terrible clicker ;) but I found some
> evidences:
>
> xinput_calibrator is complaining about misclicks, debug output:
> DEBUG: Adding click 0 (X=105, Y=59)
> DEBUG: Not adding click 1 (X=100, Y=59): within 7 pixels of previous click
> DEBUG: Adding click 1 (X=696, Y=58)
> DEBUG: Not adding click 2 (X=700, Y=55): within 7 pixels of previous click
> DEBUG: Adding click 2 (X=103, Y=422)
> DEBUG: Mis-click detected, click 3 (X=438, Y=415) not aligned with click 1 (X=696, Y=58) or click 2 (X=103, Y=422) (threshold=15)
> DEBUG: Adding click 0 (X=424, Y=411)
>
> Do you see the confusion ? At some point one of the coordinates is not
> updated. Successful calibration with 3.12.6 gives:
> DEBUG: Adding click 0 (X=126, Y=405)
> DEBUG: Adding click 1 (X=684, Y=399)
> DEBUG: Adding click 2 (X=128, Y=77)
> DEBUG: Adding click 3 (X=683, Y=77)
>
>
> With ts_calibrate:
> xres = 800, yres = 480
> Took 1 samples...
> Top left : X = 435 Y = 3572
> Took 2 samples...
> Top right : X = 2094 Y = 3553
> Took 2 samples...
> Bot right : X = 2939 Y = 2077
> Took 2 samples...
> Bot left : X = 2060 Y = 644
> Took 2 samples...
> Center : X = 1279 Y = 1346
>
> We experience the same thing, when changing position on an axis, we get
> an average between the previous and the new position (probably because
> we got 2 samples):
> - Top Left is ok
> - Top right is almost ok: X should be around 3500/3700
> - Bot right is starting to get really wrong: X is getting better (it
> should still be around 3500/3700) but Y should be quite lower
> - Bot left: Y is ok but X should be lower
> - Center is completely wrong, both values should be around 2000
>
> Last test with evtest which is always difficult because it outputs a lot
> of debug. Two separate touchs, bottom left and then top right (note that
> this is with fsl,ave-ctrl = <8>;):
>
> Event: time 1389210862.451000, type 3 (Absolute), code 0 (X), value 2183
> Event: time 1389210862.451000, type 3 (Absolute), code 1 (Y), value 720
> Event: time 1389210862.451000, type 3 (Absolute), code 24 (Pressure), value 6
> Event: time 1389210862.451000, type 1 (Key), code 330 (Touch), value 1
> Event: time 1389210862.451000, -------------- Report Sync ------------
> Event: time 1389210862.477721, type 3 (Absolute), code 0 (X), value 448
> Event: time 1389210862.477721, type 3 (Absolute), code 1 (Y), value 667
> Event: time 1389210862.477721, type 3 (Absolute), code 24 (Pressure), value 595
> Event: time 1389210862.477721, -------------- Report Sync ------------
> Event: time 1389210862.504718, type 3 (Absolute), code 0 (X), value 434
> Event: time 1389210862.504718, type 3 (Absolute), code 1 (Y), value 704
> Event: time 1389210862.504718, type 3 (Absolute), code 24 (Pressure), value 580
> Event: time 1389210862.504718, -------------- Report Sync ------------
> Event: time 1389210862.536688, type 1 (Key), code 330 (Touch), value 0
> Event: time 1389210862.536688, -------------- Report Sync ------------
> Event: time 1389210863.359697, type 3 (Absolute), code 0 (X), value 432
> Event: time 1389210863.359697, type 3 (Absolute), code 1 (Y), value 726
> Event: time 1389210863.359697, type 3 (Absolute), code 24 (Pressure), value 210
> Event: time 1389210863.359697, type 1 (Key), code 330 (Touch), value 1
> Event: time 1389210863.359697, -------------- Report Sync ------------
> Event: time 1389210863.391687, type 1 (Key), code 330 (Touch), value 0
> Event: time 1389210863.391687, -------------- Report Sync ------------
>
> We get 3 reports for bottom left, then pen up, then one report for top
> right that is almost exactly the same a bottom left !
>
> Output from 3.12:
> Event: time 1389210022.146809, type 3 (Absolute), code 0 (X), value 286
> Event: time 1389210022.146809, type 3 (Absolute), code 1 (Y), value 504
> Event: time 1389210022.146809, type 3 (Absolute), code 24 (Pressure), value 3045
> Event: time 1389210022.146809, type 1 (Key), code 330 (Touch), value 1
> Event: time 1389210022.146809, -------------- Report Sync ------------
> Event: time 1389210022.166830, type 3 (Absolute), code 0 (X), value 256
> Event: time 1389210022.166830, type 3 (Absolute), code 1 (Y), value 489
> Event: time 1389210022.166830, type 3 (Absolute), code 24 (Pressure), value 3033
> Event: time 1389210022.166830, -------------- Report Sync ------------
> Event: time 1389210022.186812, type 3 (Absolute), code 24 (Pressure), value 0
> Event: time 1389210022.186812, type 1 (Key), code 330 (Touch), value 0
> Event: time 1389210022.186812, -------------- Report Sync ------------
> Event: time 1389210025.196808, type 3 (Absolute), code 0 (X), value 3812
> Event: time 1389210025.196808, type 3 (Absolute), code 1 (Y), value 3637
> Event: time 1389210025.196808, type 3 (Absolute), code 24 (Pressure), value 4032
> Event: time 1389210025.196808, type 1 (Key), code 330 (Touch), value 1
> Event: time 1389210025.196808, -------------- Report Sync ------------
> Event: time 1389210025.216819, type 3 (Absolute), code 24 (Pressure), value 0
> Event: time 1389210025.216819, type 1 (Key), code 330 (Touch), value 0
> Event: time 1389210025.216819, -------------- Report Sync ------------
>
>
> While this is not necessarily an issue with ts_calibrate (it is possible
> to click longer so it collects more samples), I don't see that working
> with xinput_calibrator.
>
> While I don't have much experience with the TS part of the code but I
> can investigate if you don't have any idea.
You are right. I have seen the same behaviour here a few times. Currently I'm
short in time to dig deeper into this issue, but I will try to do so.
Juergen
--
Pengutronix e.K. | Juergen Beisert |
Linux Solutions for Science and Industry | http://www.pengutronix.de/ |
^ permalink raw reply
* RE: [PATCH V1] input: use device managed memory in da9052 touchscreen driver
From: Opensource [Anthony Olech] @ 2014-01-10 9:02 UTC (permalink / raw)
To: Dmitry Torokhov, Opensource [Anthony Olech]
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Huqiu Liu, David Dajun Chen
In-Reply-To: <20140109191236.GA28020@core.coreip.homeip.net>
> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: 09 January 2014 19:13
> To: Opensource [Anthony Olech]
> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Huqiu Liu;
> David Dajun Chen
> Subject: Re: [PATCH V1] input: use device managed memory in da9052
> touchscreen driver
>
> Hi Anthony,
>
> On Thu, Jan 09, 2014 at 12:51:37PM +0000, Anthony Olech wrote:
> > The touchscreen component driver for the da9052/3 Dialog PMICs is
> > changed to use device managed memory allocation.
> >
> > This results in simpler error paths as failures in the probe()
> > function do not require explicit calls to free the devm_...
> > allocated memory.
> > The allocation functions used in this driver are:
> > devm_kzalloc()
> > devm_input_allocate_device()
> > devm_request_threaded_irq()
> >
> > Suggested-by: Huqiu Liu <liuhq11@mails.tsinghua.edu.cn>
> > Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com>
> > ---
> > This patch is relative to linux-next repository tag next-20140109
> >
> > Many thanks to Huqiu Liu who instigated this patch.
> >
> > drivers/input/touchscreen/da9052_tsi.c | 62 ++++++++++++++++-----------
> -----
> > 1 file changed, 30 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/input/touchscreen/da9052_tsi.c
> > b/drivers/input/touchscreen/da9052_tsi.c
> > index ab64d58..dcc4cf1 100644
> > --- a/drivers/input/touchscreen/da9052_tsi.c
> > +++ b/drivers/input/touchscreen/da9052_tsi.c
> > @@ -233,18 +233,30 @@ static int da9052_ts_probe(struct
> platform_device *pdev)
> > struct da9052_tsi *tsi;
> > struct input_dev *input_dev;
> > int error;
> > + int pdown_irq;
> > + int ready_irq;
> >
> > da9052 = dev_get_drvdata(pdev->dev.parent);
> > if (!da9052)
> > return -EINVAL;
> >
> > - tsi = kzalloc(sizeof(struct da9052_tsi), GFP_KERNEL);
> > - input_dev = input_allocate_device();
> > - if (!tsi || !input_dev) {
> > - error = -ENOMEM;
> > - goto err_free_mem;
> > - }
> > + pdown_irq = regmap_irq_get_virq(da9052->irq_data,
> DA9052_IRQ_PENDOWN);
> > + if (pdown_irq < 0)
> > + return pdown_irq;
> ...
> >
> > - error = da9052_request_irq(tsi->da9052, DA9052_IRQ_PENDOWN,
> > - "pendown-irq", da9052_ts_pendwn_irq, tsi);
> > + error = devm_request_threaded_irq(&pdev->dev, pdown_irq,
> > + NULL, da9052_ts_pendwn_irq,
> > + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> > + "pendown-irq", tsi);
>
> I am uncomfortable with the touchscreen portion of this driver ignoring the
> framework of it's MFD core and mixing native IRQ management with the
> ones done through the core.
>
> What would happen if somebody changes da9052_request_irq() to do some
> thing more than it is doing now so that your open-coded duplicate of the
> same in da9052_ts_probe() is no longer equivalent? Or
> da9052_disable_irq() no longer works correctly with IRQs allocated by this
> sub-module?
> Thanks.
> --
> Dmitry
Hi Dmitry,
unfortunately the PMIC is a multifunction device and the component drivers come
under different subsystem maintainers. Thus it is not possible to do one patch in one
go to change them all.
Tony Olech
^ permalink raw reply
* DiePresse.com Mein Freund, der Fahrradhändler
From: franklinbuthelezi1 @ 2014-01-10 12:33 UTC (permalink / raw)
To: linux-input
Folgender Artikel auf http://diepresse.com wird Ihnen von Franklin Buthelezi empfohlen:
Mein Freund, der Fahrradhändler
Manche Dinge verändern sich über die langen Wintermonate hinweg.
http://diepresse.com/home/leben/mode/kolumnezumtag/548717/index.do
Notiz:
Dear friend,
I am the international operations manager of ABSA bank South Africa. My name is Franklin Buthelezi. I have a sensitive and private offer from the top executive to seek your partnership in re-profiling some offshore investment funds worth 11.5M U.S.D (Eleven Million five hundred thousand United States Dollars) I am constrained however to withhold most of the details for now. This is a legitimate transaction.
If we agree on the terms you will be given 15% of the total funds, if you are interested please write back via email providing me your personal details and phone number for further directives.
If you are interested and willing to render your assistance please respond via my private email address stated below.
I look forward to your response.
Best regards
Franklin Buthelezi
Email: franklinbuthelezi1@yahoo.com
------------------------------------
DiePresse.com übernimmt keine Haftung für den Inhalt der persönlichen Nachricht.
© DiePresse.com - http://diepresse.com
--
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
* Re: Re: [PATCH] Introduce Naming Convention in Input Subsystem
From: Dmitry Torokhov @ 2014-01-10 16:31 UTC (permalink / raw)
To: Aniroop Mathur
Cc: linux-input@vger.kernel.org, cpgs ., Anurag Aggarwal,
Naveen Kumar, aniroop.mathur@gmail.com, VIKAS KALA,
Poorva Srivastava
In-Reply-To: <FB.45.09100.85A10D25@epcpsbgx2.samsung.com>
Hi Aniroop,
On Fri, Jan 10, 2014 at 04:05:44PM +0000, Aniroop Mathur wrote:
> <HTML><HEAD><TITLE>Samsung Enterprise Portal mySingle</TITLE>
> <META content="text/html; charset=windows-1252" http-equiv=Content-Type>
> <STYLE id=mysingle_style type=text/css>P {
Please no HTML mail as mainling lists drop it.
Thanks.
> MARGIN-TOP: 5px; FONT-FAMILY: Arial, arial; MARGIN-BOTTOM: 5px; FONT-SIZE: 9pt
> }
> TD {
> MARGIN-TOP: 5px; FONT-FAMILY: Arial, arial; MARGIN-BOTTOM: 5px; FONT-SIZE: 9pt
> }
> LI {
> MARGIN-TOP: 5px; FONT-FAMILY: Arial, arial; MARGIN-BOTTOM: 5px; FONT-SIZE: 9pt
> }
> BODY {
> LINE-HEIGHT: 1.4; MARGIN: 10px; FONT-FAMILY: Arial, arial; FONT-SIZE: 9pt
> }
> </STYLE>
>
> <META name=GENERATOR content=ActiveSquare></HEAD>
> <BODY>
> <P>Hello Mr. Torokhov,</P>
> <P>Greetings!</P>
> <P><BR>On Thu, Jan 09, 2014 at 10:27:56AM +0530, Aniroop Mathur wrote:<BR>> This patch allows user(driver) to set sysfs node name of input<BR>> devices. To set sysfs node name, user(driver) just needs to set<BR>> node_name_unique variable. If node_name_unique is not set, default<BR>> name is given(as before). So, this patch is completely<BR>> backward-compatible.<BR>> <BR>> Sysfs Input node name format is: input_<NAME><BR>> Sysfs Event node name format is: event_<NAME><BR>><BR>> This "name" is given by user and automatically, prefix(input and<BR>> event) is added by input core.<BR>> <BR>> This name must be unique among all input devices and driver(user) has<BR>> the responsibility to ensure it. If same
name is used again for other<BR>> input device, registration of that input device will fail because two<BR>> input devices cannot have same name.<BR>> <BR>> Advantages of this patch are:<BR>> <BR>> 1. Reduces Booting Time of HAL/Upper-Layer because now HAL or<BR>> Upper-Layer do not need to search input/event number corresponding to<BR>> each input device in /dev/input/... This searching in /dev/input/ was<BR>> taking too much time. (Especially in mobile devices, where there are<BR>> many input devices (many sensors, touchscreen, etc), it reduces a lot<BR>> of booting time)<BR><BR>I am sorry, how much time does it take to scan a directory of what, 20<BR>devices? If it such a factor have udev create nodes that are easier for<BR>you to
locate, similarly how we already create nodes by-id and by-path.<BR>For example you can encode major:minor in device name.</P>
> <P> </P>
> <P><STRONG><SPAN style="FONT-SIZE: 11pt">Re: (Aniroop Mathur)</SPAN></STRONG></P>
> <P>Its correct that we can set name of a device node using udev. <BR>Yes, this will change the name of device node(/dev/...) but not sysfs node.(/sys/class/input/...)<BR><STRONG>So now, the problem area will shift from dev path to sysfs path, <BR></STRONG>because now we dont know which sysfs node to refer for a particular input device<BR>and hence HAL/Upper-Layer will need to search in /sys/class/input/... instead of /dev/... directory.</P>
> <P> </P>
> <P>Moreover, as i know, udev is mainly for hot-pluggable devices, but my problem is for platform devices,<BR>which are already present on the board during boot up. (Like in Embedded devices)</P>
> <P> </P>
> <P>To avoid confusion and make the problem more clear, <BR>I would like to explain the problem and my suggestion by taking an example:</P>
> <P> </P>
> <P>Suppose in a mobile device, there are 10 embedded input devices as below:<BR><STRONG>Proximity --- /dev/input0 --- /sys/class/input/input0 --- /sys/class/input/event0<BR>Magnetometer --- /dev/input1 --- /sys/class/input/input1 --- /sys/class/input/event1<BR>Accelerometer --- /dev/input2 --- /sys/class/input/input2 --- /sys/class/input/event2<BR>Touchscreen --- /dev/input3 --- /sys/class/input/input3 --- /sys/class/input/event3 </STRONG></P>
> <P><STRONG>... 6 more like this<BR></STRONG>(All these are created during boot up time)</P>
> <P> </P>
> <P>Kernel has created all these nodes, so that HAL/UpperLayer can read or write values from it.<BR>HAL/Upper-Layer needs to do main tasks like:<BR>1. Read raw data - does through /dev/input<num><BR>2. Enable device - does through sys/class/input<num>/enable<BR>3. Set delay - does through sys/class/input<num>/delay<BR>and many more...</P>
> <P> </P>
> <P>Now, Lets suppose we need to do these tasks for Accelerometer.</P>
> <P>If dev node name is set, HAL can directly read value from it (no search required)<BR><STRONG>But for enabling the accelerometer device or set the delay of a hardware chip, <BR>there is no direct way, HAL can know which input node to refer for accelerometer<BR>because the input number is created dynamically as per device probe order,<BR>so this input number can be anything (0,1,2,3...)<BR></STRONG>So HAL will need to search every input node and read its name attribute<BR>and keep on searching until a match is found between the "attribute name" and "name passed as parameter". <BR>Like for accelerometer, this searching needs to be done for all other input devices.<BR>All of this part is done during booting and this takes a lot for time from booting perspective.</P>
> <P> </P>
> <P><STRONG>As I measured, if there are ten devices, it is taking 1 second to do all this searching. (for all devices)<BR>So for 20 devices, i guess, it could take upto 2 seconds.</STRONG></P>
> <P><STRONG></STRONG> </P>
> <P><STRONG>With naming convention, there is no need of search neither for dev path nor for sysfs path<BR>because HAL directly know which node to refer for which input device<BR>and hence this 1 second is reduced to 10ms or even less, therefore saving 990ms.<BR>I believe, this is a very good time saving. (from device booting perspective)</STRONG></P>
> <P> </P>
> <P>(Is there any direct way, without scanning all nodes for every input device ?)</P>
> <P><BR>> <BR>> 2. Improves Readabilty of input and event sysfs node paths because<BR>> names are used instead of numbers.<BR><BR>I do not see why it is that important. If one wants overview<BR>/proc/bus/input/devices gives nice picture.</P>
> <P> </P>
> <P><STRONG><SPAN style="FONT-SIZE: 11pt">Re: (Aniroop Mathur)</SPAN></STRONG></P>
> <P>Its correct, we can get an overview from /proc/bus/input/devices.<BR>And therefore using this, we can know input node number for every input device.<BR>But there are many input devices and input numbers are not fixed, <BR>so its quite difficult to memorize input number for all input devices.<BR><STRONG>Therefore, if a user needs to open some input node from sysfs path,<BR>he needs to check /proc/bus/input/devices before opening because <BR>he does not know the input number. Moreover, this applies for all other<BR>input devices and hence a user need to check this every time.</STRONG><STRONG><BR></P></STRONG>
> <P>It improves readabilty as below</P>
> <P> </P>
> <TABLE style="BACKGROUND-COLOR: #ffffff; WIDTH: 375px" border=1 cellSpacing=0 width=375>
> <TBODY>
> <TR>
> <TD height=26 width=151>
> <P> Before</P></TD>
> <TD height=26 width=214>
> <P> After Patch</P></TD></TR>
> <TR>
> <TD height=26 width=151>
> <P><STRONG>dev node paths:</STRONG></P>
> <P><STRONG></STRONG> </P>
> <P>/dev/input0</P>
> <P>/dev/input1</P>
> <P>... many more </P></TD>
> <TD height=26 width=214>
> <P><STRONG>dev node paths:</STRONG></P>
> <P><STRONG></STRONG> </P>
> <P>/dev/input_proximity</P>
> <P>/dev/input_accelerometer</P>
> <P>... many more </P></TD></TR>
> <TR>
> <TD height=26 width=151>
> <P><STRONG>sysfs node paths:</STRONG></P>
> <P><STRONG></STRONG> </P>
> <P>/sys/class/input/input0</P>
> <P>/sys/class/input/input1 </P>
> <P>... many more</P>
> <P> </P>
> <P>/sys/class/input/event0</P>
> <P>/sys/class/input/event1</P>
> <P>...many more</P></TD>
> <TD height=26 width=214>
> <P><STRONG>sysfs node paths:</STRONG></P>
> <P><STRONG></STRONG> </P>
> <P>/sys/class/input/input_proximity</P>
> <P>/sys/class/input/input_accelerometer</P>
> <P>... many more</P>
> <P> </P>
> <P>/sys/class/input/event_proximity</P>
> <P>/sys/class/input/event_accelerometer</P>
> <P>... many more</P></TD></TR></TBODY></TABLE>
> <P> </P>
> <P><STRONG>So, just by looking, user can directly open or refer any input node.<BR>(no need to refer any other path)</STRONG><STRONG></P>
> <P><BR></STRONG>> <BR>> 3. Removes Input Devices Dependency. If one input device probe fails,<BR>> other input devices still work. Before this patch, if one input<BR>> device probe fails before input_register_device, then input number of<BR>> other input devices changes and due to this permission settings are<BR>> disturbed and hence HAL or upper layer cannot open the required sysfs<BR>> node because permission denied error comes.<BR><BR>I have only one suggestion here: fix your userspace so that does not<BR>depend on device initialization ordering.<BR></P>
> <P><STRONG><SPAN style="FONT-SIZE: 11pt">Re: (Aniroop Mathur)</SPAN></STRONG></P>
> <P><STRONG>We cannot fix userspace because these input/event/dev number are decided/allocated in kernel<BR>as per device initialization ordering during boot up. (userspace has no role in it)<BR></STRONG>So, userspace is not aware, which exact input number corresponds to which input device <BR>so it ends up searching/scanning every input node untill a match is found.</P>
> <P>So, there is input device dependency which needs to be removed.</P>
> <P> </P>
> <P><BR>IOW I am totally unconvinced that this facility is needed.</P>
> <P><STRONG><SPAN style="FONT-SIZE: 11pt">Re: (Aniroop Mathur)</SPAN></STRONG></P>
> <P>I hope my problem and suggestion is more clear and convincing now.</P>
> <P> </P>
> <P> </P>
> <P><STRONG>For reference, copying Patch again:</STRONG></P>
> <P>---<BR>drivers/input/evdev.c | 11 ++++++++++-<BR>drivers/input/input.c | 12 +++++++++++-<BR>include/linux/input.h | 4 ++++<BR>3 files changed, 25 insertions(+), 2 deletions(-)<BR><BR>diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c<BR>index b6ded17..b6a5848 100644<BR>--- a/drivers/input/evdev.c<BR>+++ b/drivers/input/evdev.c<BR>@@ -1131,7 +1131,16 @@ static int evdev_connect(struct input_handler *handler, struct input_dev *dev,<BR>/* Normalize device number if it falls into legacy range */<BR>if (dev_no < EVDEV_MINOR_BASE + EVDEV_MINORS)<BR>dev_no -= EVDEV_MINOR_BASE;<BR>- dev_set_name(&evdev->dev, "event%d", dev_no);<BR>+<BR>+ /*<BR>+ * As per user choice (driver),<BR>+ * name of sysfs node is set, as mentioned in node_nam
e_unique variable.<BR>+ * If node_name_unique is not set, default name is given.<BR>+ */<BR>+ if (dev->node_name_unique)<BR>+ dev_set_name(&evdev->dev, "event_%s", dev->node_name_unique);<BR>+ else<BR>+ dev_set_name(&evdev->dev, "event%d", dev_no);<BR><BR>evdev->handle.dev = input_get_device(dev);<BR>evdev->handle.name = dev_name(&evdev->dev);<BR>diff --git a/drivers/input/input.c b/drivers/input/input.c<BR>index c044699..c8126b3 100644<BR>--- a/drivers/input/input.c<BR>+++ b/drivers/input/input.c<BR>@@ -2073,7 +2073,17 @@ int input_register_device(struct input_dev *dev)<BR>if (!dev->setkeycode)<BR>dev->setkeycode = input_default_setkeycode;<BR><BR>- dev_set_name(&dev->dev, "input%ld",<BR>+ /*<BR>+ * As per user choice (driver),<BR>+ * name o
f sysfs node is set, as mentioned in node_name_unique variable.<BR>+ * If node_name_unique is not set, default name is given.<BR>+ */<BR>+ if (dev->node_name_unique) {<BR>+ atomic_inc_return(&input_no);<BR>+ dev_set_name(&dev->dev, "input_%s",<BR>+ dev->node_name_unique);<BR>+ } else<BR>+ dev_set_name(&dev->dev, "input%ld",<BR> (unsigned long) atomic_inc_return(&input_no) - 1);<BR><BR>error = device_add(&dev->dev);<BR>diff --git a/include/linux/input.h b/include/linux/input.h<BR>index 82ce323..fe44643 100644<BR>--- a/include/linux/input.h<BR>+++ b/include/linux/input.h<BR>@@ -43,6 +43,9 @@ struct input_value {<BR> * @uniq: unique identification code for the device (if device has it)<BR> * @id:
id of the device (struct input_id)<BR> * @propbit: bitmap of device properties and quirks<BR>+ * @node_name_unique: name of input and event sysfs device node (char *).<BR>+ * This name must be unique among all input devices and driver(user)<BR>+ * has the responsibility to ensure it (if using).<BR> * @evbit: bitmap of types of events supported by the device (EV_KEY,<BR> * EV_REL, etc.)<BR> * @keybit: bitmap of keys/buttons this device has<BR>@@ -123,6 +126,7 @@ struct input_dev {<BR>const char *phys;<BR>const char *uniq;<BR>struct input_id id;<BR>+ char *node_name_unique;<BR><BR>unsigned long propbit[BITS_TO_LONGS(INPUT_PROP_CNT)];<BR><BR>-- <BR>1.7.0.4<BR><BR></P>
> <P>Thanks,</P>
> <P>Aniroop Mathur</P>
> <P> </P>
> <TABLE id=confidentialsignimg>
> <TBODY>
> <TR>
> <TD NAMO_LOCK>
> <P><IMG border=0 src="cid:QKNMBDIFBEI0@namo.co.kr" width=520></P></TD></TR></TBODY></TABLE></BODY></HTML><img src='http://ext.samsung.net/mailcheck/SeenTimeChecker?do=166e1c9f408a9352202b65e5c362a372ac21ff698029c65acf20909b63ed3e8c4e627947a285d7d76b91358bc02b4ef9c7b41e955949e5c8a728c55b39cc59eacf878f9a26ce15a0' border=0 width=0 height=0 style='display:none'>
--
Dmitry
^ permalink raw reply
* Re: [PATCH V1] input: use device managed memory in da9052 touchscreen driver
From: Dmitry Torokhov @ 2014-01-10 16:36 UTC (permalink / raw)
To: Opensource [Anthony Olech]
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Huqiu Liu, David Dajun Chen
In-Reply-To: <24DF37198A1E704D9811D8F72B87EB51AC050B52@NB-EX-MBX02.diasemi.com>
On Fri, Jan 10, 2014 at 09:02:10AM +0000, Opensource [Anthony Olech] wrote:
> > -----Original Message-----
> > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> >
> > I am uncomfortable with the touchscreen portion of this driver ignoring the
> > framework of it's MFD core and mixing native IRQ management with the
> > ones done through the core.
> >
> > What would happen if somebody changes da9052_request_irq() to do some
> > thing more than it is doing now so that your open-coded duplicate of the
> > same in da9052_ts_probe() is no longer equivalent? Or
> > da9052_disable_irq() no longer works correctly with IRQs allocated by this
> > sub-module?
> > Thanks.
> > --
> > Dmitry
> Hi Dmitry,
>
> unfortunately the PMIC is a multifunction device and the component
> drivers come under different subsystem maintainers. Thus it is not
> possible to do one patch in one go to change them all.
This could be arranged if you really want to do that. You could post a
patch series and have various maintainers ack it and then Samuel could
take it all through MFD tree.
This is up to you however. The change does not fix any bugs and error
unwinding paths in the input portion of the driver are simple enough, so
I do not see a very strong reason for moving to managed devices. I said
that I would not be opposed to this (given that conversion is solid),
but that was never a request form me.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: Re: [PATCH] Introduce Naming Convention in Input Subsystem
From: Aniroop Mathur @ 2014-01-10 16:49 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input@vger.kernel.org, Aniroop Mathur, cpgs .,
Anurag Aggarwal, Naveen Kumar, aniroop.mathur@gmail.com,
VIKAS KALA, Poorva Srivastava
Hello Mr. Torokhov,
Greetings!
On Thu, Jan 09, 2014 at 10:27:56AM +0530, Aniroop Mathur wrote:
> This patch allows user(driver) to set sysfs node name of input
> devices. To set sysfs node name, user(driver) just needs to set
> node_name_unique variable. If node_name_unique is not set, default
> name is given(as before). So, this patch is completely
> backward-compatible.
>
> Sysfs Input node name format is: input_
> Sysfs Event node name format is: event_
>
> This "name" is given by user and automatically, prefix(input and
> event) is added by input core.
>
> This name must be unique among all input devices and driver(user) has
> the responsibility to ensure it. If same name is used again for other
> input device, registration of that input device will fail because two
> input devices cannot have same name.
>
> Advantages of this patch are:
>
> 1. Reduces Booting Time of HAL/Upper-Layer because now HAL or
> Upper-Layer do not need to search input/event number corresponding to
> each input device in /dev/input/... This searching in /dev/input/ was
> taking too much time. (Especially in mobile devices, where there are
> many input devices (many sensors, touchscreen, etc), it reduces a lot
> of booting time)
I am sorry, how much time does it take to scan a directory of what, 20
devices? If it such a factor have udev create nodes that are easier for
you to locate, similarly how we already create nodes by-id and by-path.
For example you can encode major:minor in device name.
Re: (Aniroop Mathur)
Its correct that we can set name of a device node using udev.
Yes, this will change the name of device node(/dev/...) but not sysfs node.(/sys/class/input/...)
So now, the problem area will shift from dev path to sysfs path,
because now we dont know which sysfs node to refer for a particular input device
and hence HAL/Upper-Layer will need to search in /sys/class/input/... instead of /dev/... directory.
Moreover, as i know, udev is mainly for hot-pluggable devices, but my problem is for platform devices,
which are already present on the board during boot up. (Like in Embedded devices)
To avoid confusion and make the problem more clear,
I would like to explain the problem and my suggestion by taking an example:
Suppose in a mobile device, there are 10 embedded input devices as below:
Proximity --- /dev/input0 --- /sys/class/input/input0 --- /sys/class/input/event0
Magnetometer --- /dev/input1 --- /sys/class/input/input1 --- /sys/class/input/event1
Accelerometer --- /dev/input2 --- /sys/class/input/input2 --- /sys/class/input/event2
Touchscreen --- /dev/input3 --- /sys/class/input/input3 --- /sys/class/input/event3
... 6 more like this
(All these are created during boot up time)
Kernel has created all these nodes, so that HAL/UpperLayer can read or write values from it.
HAL/Upper-Layer needs to do main tasks like:
1. Read raw data - does through /dev/input<num>
2. Enable device - does through sys/class/input<num>/enable
3. Set delay - does through sys/class/input<num>/delay
and many more...
Now, Lets suppose we need to do these tasks for Accelerometer.
If dev node name is set, HAL can directly read value from it (no search required)
But for enabling the accelerometer device or set the delay of a hardware chip,
there is no direct way, HAL can know which input node to refer for accelerometer
because the input number is created dynamically as per device probe order,
so this input number can be anything (0,1,2,3...)
So HAL will need to search every input node and read its name attribute
and keep on searching until a match is found between the "attribute name" and "name passed as parameter".
Like for accelerometer, this searching needs to be done for all other input devices.
All of this part is done during booting and this takes a lot for time from booting perspective.
As I measured, if there are ten devices, it is taking 1 second to do all this searching. (for all devices)
So for 20 devices, i guess, it could take upto 2 seconds.
With naming convention, there is no need of search neither for dev path nor for sysfs path
because HAL directly know which node to refer for which input device
and hence this 1 second is reduced to 10ms or even less, therefore saving 990ms.
I believe, this is a very good time saving. (from device booting perspective)
(Is there any direct way, without scanning all nodes for every input device ?)
>
> 2. Improves Readabilty of input and event sysfs node paths because
> names are used instead of numbers.
I do not see why it is that important. If one wants overview
/proc/bus/input/devices gives nice picture.
Re: (Aniroop Mathur)
Its correct, we can get an overview from /proc/bus/input/devices.
And therefore using this, we can know input node number for every input device.
But there are many input devices and input numbers are not fixed,
so its quite difficult to memorize input number for all input devices.
Therefore, if a user needs to open some input node from sysfs path,
he needs to check /proc/bus/input/devices before opening because
he does not know the input number. Moreover, this applies for all other
input devices and hence a user need to check this every time.
It improves readabilty as below
Before: After patch:
/dev/input0 /dev/input_proximity
/dev/input1 /dev/input_accelerometer
...many more
/sys/class/input/input0 /sys/class/input/input_proximity
/sys/class/input/input1 /sys/class/input/input_accelerometer
...many more
/sys/class/input/event0 /sys/class/input/event_proximity
/sys/class/input/event1 /sys/class/input/event_accelerometer
...many more
So, just by looking, user can directly open or refer any input node.
(no need to refer any other path)
>
> 3. Removes Input Devices Dependency. If one input device probe fails,
> other input devices still work. Before this patch, if one input
> device probe fails before input_register_device, then input number of
> other input devices changes and due to this permission settings are
> disturbed and hence HAL or upper layer cannot open the required sysfs
> node because permission denied error comes.
I have only one suggestion here: fix your userspace so that does not
depend on device initialization ordering.
Re: (Aniroop Mathur)
We cannot fix userspace because these input/event/dev number are decided/allocated in kernel
as per device initialization ordering during boot up. (userspace has no role in it)
So, userspace is not aware, which exact input number corresponds to which input device
so it ends up searching/scanning every input node untill a match is found.
So, there is input device dependency which needs to be removed.
----------------------------
IOW I am totally unconvinced that this facility is needed.
Re: (Aniroop Mathur)
I hope my problem and suggestion is more clear and convincing now.
For reference, copying Patch again:
---
drivers/input/evdev.c | 11 ++++++++++-
drivers/input/input.c | 12 +++++++++++-
include/linux/input.h | 4 ++++
3 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index b6ded17..b6a5848 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -1131,7 +1131,16 @@ static int evdev_connect(struct input_handler *handler, struct input_dev *dev,
/* Normalize device number if it falls into legacy range */
if (dev_no < EVDEV_MINOR_BASE + EVDEV_MINORS)
dev_no -= EVDEV_MINOR_BASE;
- dev_set_name(&evdev->dev, "event%d", dev_no);
+
+ /*
+ * As per user choice (driver),
+ * name of sysfs node is set, as mentioned in node_name_unique variable.
+ * If node_name_unique is not set, default name is given.
+ */
+ if (dev->node_name_unique)
+ dev_set_name(&evdev->dev, "event_%s", dev->node_name_unique);
+ else
+ dev_set_name(&evdev->dev, "event%d", dev_no);
evdev->handle.dev = input_get_device(dev);
evdev->handle.name = dev_name(&evdev->dev);
diff --git a/drivers/input/input.c b/drivers/input/input.c
index c044699..c8126b3 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -2073,7 +2073,17 @@ int input_register_device(struct input_dev *dev)
if (!dev->setkeycode)
dev->setkeycode = input_default_setkeycode;
- dev_set_name(&dev->dev, "input%ld",
+ /*
+ * As per user choice (driver),
+ * name of sysfs node is set, as mentioned in node_name_unique variable.
+ * If node_name_unique is not set, default name is given.
+ */
+ if (dev->node_name_unique) {
+ atomic_inc_return(&input_no);
+ dev_set_name(&dev->dev, "input_%s",
+ dev->node_name_unique);
+ } else
+ dev_set_name(&dev->dev, "input%ld",
(unsigned long) atomic_inc_return(&input_no) - 1);
error = device_add(&dev->dev);
diff --git a/include/linux/input.h b/include/linux/input.h
index 82ce323..fe44643 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -43,6 +43,9 @@ struct input_value {
* @uniq: unique identification code for the device (if device has it)
* @id: id of the device (struct input_id)
* @propbit: bitmap of device properties and quirks
+ * @node_name_unique: name of input and event sysfs device node (char *).
+ * This name must be unique among all input devices and driver(user)
+ * has the responsibility to ensure it (if using).
* @evbit: bitmap of types of events supported by the device (EV_KEY,
* EV_REL, etc.)
* @keybit: bitmap of keys/buttons this device has
@@ -123,6 +126,7 @@ struct input_dev {
const char *phys;
const char *uniq;
struct input_id id;
+ char *node_name_unique;
unsigned long propbit[BITS_TO_LONGS(INPUT_PROP_CNT)];
Thanks,
Aniroop Mathur
^ permalink raw reply related
* Re: Re: [PATCH] Introduce Naming Convention in Input Subsystem
From: Dmitry Torokhov @ 2014-01-10 19:11 UTC (permalink / raw)
To: Aniroop Mathur
Cc: linux-input@vger.kernel.org, cpgs ., Anurag Aggarwal,
Naveen Kumar, aniroop.mathur@gmail.com, VIKAS KALA,
Poorva Srivastava
In-Reply-To: <25201406.444611389372583656.JavaMail.weblogic@epml19>
Hi Aniroop,
On Fri, Jan 10, 2014 at 04:49:43PM +0000, Aniroop Mathur wrote:
> Hello Mr. Torokhov,
> Greetings!
>
> On Thu, Jan 09, 2014 at 10:27:56AM +0530, Aniroop Mathur wrote:
> > This patch allows user(driver) to set sysfs node name of input
> > devices. To set sysfs node name, user(driver) just needs to set
> > node_name_unique variable. If node_name_unique is not set, default
> > name is given(as before). So, this patch is completely
> > backward-compatible.
> >
> > Sysfs Input node name format is: input_
> > Sysfs Event node name format is: event_
> >
> > This "name" is given by user and automatically, prefix(input and
> > event) is added by input core.
> >
> > This name must be unique among all input devices and driver(user) has
> > the responsibility to ensure it. If same name is used again for other
> > input device, registration of that input device will fail because two
> > input devices cannot have same name.
> >
> > Advantages of this patch are:
> >
> > 1. Reduces Booting Time of HAL/Upper-Layer because now HAL or
> > Upper-Layer do not need to search input/event number corresponding to
> > each input device in /dev/input/... This searching in /dev/input/ was
> > taking too much time. (Especially in mobile devices, where there are
> > many input devices (many sensors, touchscreen, etc), it reduces a lot
> > of booting time)
>
> I am sorry, how much time does it take to scan a directory of what, 20
> devices? If it such a factor have udev create nodes that are easier for
> you to locate, similarly how we already create nodes by-id and by-path.
> For example you can encode major:minor in device name.
>
> Re: (Aniroop Mathur)
First of all, it would be great if you could use MUA that can properly
quote and wrap long lines...
> Its correct that we can set name of a device node using udev. Yes,
> this will change the name of device node(/dev/...) but not sysfs
> node.(/sys/class/input/...) So now, the problem area will shift from
> dev path to sysfs path, because now we dont know which sysfs node to
> refer for a particular input device and hence HAL/Upper-Layer will
> need to search in /sys/class/input/... instead of /dev/... directory.
[dtor@dtor-d630 ~]$ mkdir my-sysfs-view
[dtor@dtor-d630 ~]$ ln -s
/sys/devices/platform/i8042/serio1/input/input6
my-sysfs-view/input_touchpad
[dtor@dtor-d630 ~]$ ls my-sysfs-view/input_touchpad/
capabilities/ event6/ modalias name power/
subsystem/ uniq
device/ id/ mouse1/ phys properties
uevent
[dtor@dtor-d630 ~]$ ls my-sysfs-view/input_touchpad/
capabilities device event6 id modalias mouse1 name phys power
properties subsystem uevent uniq
[dtor@dtor-d630 ~]$ ls my-sysfs-view/input_touchpad/event6/
dev device power subsystem uevent
Mmmmkay?
>
> Moreover, as i know, udev is mainly for hot-pluggable devices, but my
> problem is for platform devices, which are already present on the
> board during boot up. (Like in Embedded devices)
No, udev also manages those by requesting to replay all events that
happened dyuring boot.
>
> To avoid confusion and make the problem more clear,
> I would like to explain the problem and my suggestion by taking an example:
>
> Suppose in a mobile device, there are 10 embedded input devices as below:
> Proximity --- /dev/input0 --- /sys/class/input/input0 --- /sys/class/input/event0
> Magnetometer --- /dev/input1 --- /sys/class/input/input1 --- /sys/class/input/event1
> Accelerometer --- /dev/input2 --- /sys/class/input/input2 --- /sys/class/input/event2
> Touchscreen --- /dev/input3 --- /sys/class/input/input3 --- /sys/class/input/event3
> ... 6 more like this
> (All these are created during boot up time)
>
> Kernel has created all these nodes, so that HAL/UpperLayer can read or
> write values from it. HAL/Upper-Layer needs to do main tasks like:
> 1. Read raw data - does through /dev/input<num>
> 2. Enable device - does through sys/class/input<num>/enable
> 3. Set delay - does through sys/class/input<num>/delay
> and many more...
>
> Now, Lets suppose we need to do these tasks for Accelerometer.
>
> If dev node name is set, HAL can directly read value from it (no
> search required) But for enabling the accelerometer device or set the
> delay of a hardware chip, there is no direct way, HAL can know which
> input node to refer for accelerometer because the input number is
> created dynamically as per device probe order, so this input number
> can be anything (0,1,2,3...) So HAL will need to search every input
> node and read its name attribute and keep on searching until a match
> is found between the "attribute name" and "name passed as parameter".
> Like for accelerometer, this searching needs to be done for all other
> input devices. All of this part is done during booting and this takes
> a lot for time from booting perspective.
>
See the above. You can very easily create your own private 'view' of
sysfs, no kernel changes needed.
> As I measured, if there are ten devices, it is taking 1 second to do
> all this searching. (for all devices) So for 20 devices, i guess, it
> could take upto 2 seconds.
That seems _very_ high, maybe you need to profile your code a bit. To
search though 2 directories with less than a hundred files each should
not take 1 second.
>
> With naming convention, there is no need of search neither for dev
> path nor for sysfs path because HAL directly know which node to refer
> for which input device and hence this 1 second is reduced to 10ms or
> even less, therefore saving 990ms. I believe, this is a very good
> time saving. (from device booting perspective)
OK, so create your own sysfs view and use it to do direct lookups.
>
> (Is there any direct way, without scanning all nodes for every input
> device ?)
>
> >
> > 2. Improves Readabilty of input and event sysfs node paths because
> > names are used instead of numbers.
>
> I do not see why it is that important. If one wants overview
> /proc/bus/input/devices gives nice picture.
>
> Re: (Aniroop Mathur)
> Its correct, we can get an overview from /proc/bus/input/devices.
> And therefore using this, we can know input node number for every input device.
> But there are many input devices and input numbers are not fixed,
> so its quite difficult to memorize input number for all input devices.
> Therefore, if a user needs to open some input node from sysfs path,
> he needs to check /proc/bus/input/devices before opening because
> he does not know the input number. Moreover, this applies for all other
> input devices and hence a user need to check this every time.
>
> It improves readabilty as below
>
> Before: After patch:
> /dev/input0 /dev/input_proximity
> /dev/input1 /dev/input_accelerometer
> ...many more
>
> /sys/class/input/input0 /sys/class/input/input_proximity
> /sys/class/input/input1 /sys/class/input/input_accelerometer
> ...many more
>
> /sys/class/input/event0 /sys/class/input/event_proximity
> /sys/class/input/event1 /sys/class/input/event_accelerometer
> ...many more
>
> So, just by looking, user can directly open or refer any input node.
> (no need to refer any other path)
User as in end user or your HAL layer?
>
> >
> > 3. Removes Input Devices Dependency. If one input device probe fails,
> > other input devices still work. Before this patch, if one input
> > device probe fails before input_register_device, then input number of
> > other input devices changes and due to this permission settings are
> > disturbed and hence HAL or upper layer cannot open the required sysfs
> > node because permission denied error comes.
>
> I have only one suggestion here: fix your userspace so that does not
> depend on device initialization ordering.
>
> Re: (Aniroop Mathur)
> We cannot fix userspace because these input/event/dev number are
> decided/allocated in kernel as per device initialization ordering
> during boot up. (userspace has no role in it) So, userspace is not
> aware, which exact input number corresponds to which input device so
> it ends up searching/scanning every input node untill a match is
> found.
>
> So, there is input device dependency which needs to be removed.
Do not use numbers. We emit uevents describing the devices and there a
_lot_ of data there that helps identifying device, such as its path,
subsystem, name, etc.
>
> ----------------------------
>
> IOW I am totally unconvinced that this facility is needed.
>
> Re: (Aniroop Mathur)
> I hope my problem and suggestion is more clear and convincing now.
>
Not in the slightest, I am sorry.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input synaptics-rmi4: PDT scan cleanup
From: Christopher Heiny @ 2014-01-10 20:23 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Linux Input, Christopher Heiny, Andrew Duggan, Vincent Huang,
Vivian Ly, Daniel Rosenberg, Jean Delvare, Joerie de Gram,
Linus Walleij, Benjamin Tissoires
In-Reply-To: <1389126821-4066-1-git-send-email-cheiny@synaptics.com>
On 01/07/2014 12:33 PM, 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).
>
> Updated the copyright dates while we were at it.
Hi Dmitry,
Could you apply this or provide some feedback on it? We've got a
pending patch that depends on it, and that pending work will bring the
driver back to a working (if not necessarily beautiful) state. I don't
want to submit it if this change isn't satisfactory, though.
Thanks,
Chris
>
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> ---
>
> drivers/input/rmi4/rmi_driver.c | 155 ++++++++++++++++++++--------------------
> drivers/input/rmi4/rmi_driver.h | 6 +-
> 2 files changed, 83 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index eb790ff..cbd6485 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.
> @@ -566,85 +566,80 @@ err_free_mem:
> 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)
> +
> +#define RMI_SCAN_CONTINUE 0
> +#define RMI_SCAN_DONE 1
> +
> +static int rmi_initial_reset(struct rmi_device *rmi_dev,
> + void *clbk_ctx, struct pdt_entry *pdt_entry, int page)
> {
> - 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");
> + if (pdt_entry->function_number == 0x01) {
> + u16 cmd_addr = page + pdt_entry->command_base_addr;
> + u8 cmd_buf = RMI_DEVICE_RESET_CMD;
> + const struct rmi_device_platform_data *pdata =
> + to_rmi_platform_data(rmi_dev);
>
> - 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;
> + retval = rmi_write_block(rmi_dev, cmd_addr, &cmd_buf, 1);
> + if (retval < 0) {
> + dev_err(&rmi_dev->dev,
> + "Initial reset failed. Code = %d.\n", retval);
> + return retval;
> + }
> + mdelay(pdata->reset_delay_ms);
>
> - 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;
> + return RMI_SCAN_DONE;
> + }
>
> - if (RMI4_END_OF_PDT(pdt_entry.function_number))
> - break;
> - done = false;
> + /* F01 should always be on page 0. If we don't find it there, fail. */
> + return (!page) ? RMI_SCAN_CONTINUE : -ENODEV;
> +}
>
> - 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;
> - }
> - }
> - }
> +static int rmi_create_functions_clbk(struct rmi_device *rmi_dev,
> + void *clbk_ctx, struct pdt_entry *entry, int page)
> +{
> + int *irq_count = (int *)clbk_ctx;
>
> - if (!has_f01) {
> - dev_warn(dev, "WARNING: Failed to find F01 for initial reset.\n");
> - return -ENODEV;
> - }
> + return create_function(rmi_dev, entry, irq_count,
> + RMI4_PAGE_SIZE * page);
> +}
> +
> +static int rmi_create_functions(struct rmi_device *rmi_dev)
> +{
> + struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> + struct device *dev = &rmi_dev->dev;
> + int irq_count = 0;
> + int retval;
> +
> + // 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_functions_clbk);
> +
> + if (retval)
> + return retval;
> +
> + // 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;
>
> return 0;
> }
>
> -static int rmi_scan_pdt(struct rmi_device *rmi_dev)
> +int rmi_scan_pdt(struct rmi_device *rmi_dev, void *ctx,
> + int (*rmi_pdt_scan_clbk)(struct rmi_device *rmi_dev,
> + void *clbk_ctx, struct pdt_entry *entry, int page))
> {
> - struct rmi_driver_data *data;
> + struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> struct pdt_entry pdt_entry;
> int page;
> - struct device *dev = &rmi_dev->dev;
> - int irq_count = 0;
> - bool done = false;
> int i;
> - int retval;
> -
> - dev_dbg(dev, "Scanning PDT...\n");
> + bool done = false;
> + int retval = 0;
>
> - data = dev_get_drvdata(&rmi_dev->dev);
> + // 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) && !done; page++) {
> @@ -656,27 +651,27 @@ static int rmi_scan_pdt(struct rmi_device *rmi_dev)
> 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;
> + return retval;
>
> if (RMI4_END_OF_PDT(pdt_entry.function_number))
> break;
>
> - dev_dbg(dev, "Found F%02X on page %#04x\n",
> + dev_dbg(&rmi_dev->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);
> -
> - if (retval)
> + retval = rmi_pdt_scan_clbk(rmi_dev, ctx,
> + &pdt_entry, page);
> + if (retval < 0) {
> goto error_exit;
> + } else if (retval == RMI_SCAN_DONE) {
> + done = true;
> + break;
> + }
> }
> done = done || data->f01_bootloader_mode;
> }
> - 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:
> @@ -684,6 +679,7 @@ error_exit:
> return retval;
> }
>
> +
> #ifdef CONFIG_PM_SLEEP
> static int rmi_driver_suspend(struct device *dev)
> {
> @@ -797,10 +793,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 +814,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 df9ddd8..f73be73 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
> @@ -108,6 +108,10 @@ struct pdt_entry {
> int rmi_read_pdt_entry(struct rmi_device *rmi_dev, struct pdt_entry *entry,
> u16 pdt_address);
>
> +int rmi_scan_pdt(struct rmi_device *rmi_dev, void *ctx,
> + int (*rmi_pdt_scan_clbk)(struct rmi_device *rmi_dev,
> + void *clbk_ctx, struct pdt_entry *entry, int page));
> +
> bool rmi_is_physical_driver(struct device_driver *);
> int rmi_register_physical_driver(void);
> void rmi_unregister_physical_driver(void);
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* Re: [PATCH v2] Input: add i2c/smbus driver for elan touchpad
From: Benson Leung @ 2014-01-10 20:30 UTC (permalink / raw)
To: Duson Lin
Cc: linux-kernel@vger.kernel.org, linux-input, Dmitry Torokhov,
agnescheng, phoenix, jeff.chuang
In-Reply-To: <1389064083-11018-1-git-send-email-dusonlin@emc.com.tw>
On Mon, Jan 6, 2014 at 7:08 PM, Duson Lin <dusonlin@emc.com.tw> wrote:
> This driver adds support for elan i2c/smbus touchpad found on some laptops PC
>
> Signed-off-by: Duson Lin <dusonlin@emc.com.tw>
Reviewed-by: Benson Leung <bleung@chromium.org>
--
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org
^ permalink raw reply
* Re: [PATCH] input synaptics-rmi4: PDT scan cleanup
From: Dmitry Torokhov @ 2014-01-10 20:31 UTC (permalink / raw)
To: Christopher Heiny
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <52D056B3.406@synaptics.com>
On Friday, January 10, 2014 12:23:15 PM Christopher Heiny wrote:
> On 01/07/2014 12:33 PM, 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).
> >
> > Updated the copyright dates while we were at it.
>
> Hi Dmitry,
>
> Could you apply this or provide some feedback on it? We've got a
> pending patch that depends on it, and that pending work will bring the
> driver back to a working (if not necessarily beautiful) state. I don't
> want to submit it if this change isn't satisfactory, though.
Speaking of the devil. I was just thinking about it and I wanted to ask
you to send me an example of how it is used as I can;t make my mind about
it.
In general it is OK to submit a few patches at a time even if they are
depend on each other - it gives me better context (as long as there
aren't 80 of those so that I down in them ;) ).
Thanks.
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox