Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v2 2/3] input: appletouch: use better cursor movement smoothing algorithm
From: Clinton Sprain @ 2014-02-07  0:43 UTC (permalink / raw)
  To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
In-Reply-To: <52F42AEE.3040505@gmail.com>

Use smoothed version of sensor array data to calculate movement and add weight to prior values when calculating average. This gives more granular and more predictable movement.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   60 ++++++++++++++++++++++++++------------
 1 file changed, 42 insertions(+), 18 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index d48181b..edbdd95 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -338,21 +338,51 @@ static void atp_reinit(struct work_struct *work)
 static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 			     int *z, int *fingers)
 {
-	int i;
+	int i, k;
+	int smooth[nb_sensors + 2];
+	int smooth_tmp[nb_sensors + 2];
+
 	/* values to calculate mean */
 	int pcum = 0, psum = 0;
 	int is_increasing = 0;
 
 	*fingers = 0;
 
+	/*
+	 * Use a smoothed version of sensor data for movement calculations, to
+	 * combat noise without needing to toss out values based on a threshold.
+	 * This improves tracking. Finger count is calculated separately based
+	 * on raw data.
+	 */
+
+	for (i = 0; i < nb_sensors; i++) {           /* Scale up to minimize */
+		smooth[i] = xy_sensors[i] << 12;     /* rounding/truncation. */
+	}
+
+	/* Mitigate some of the data loss from smoothing on the edge sensors. */
+	smooth[-1] = smooth[0] >> 2;
+	smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;
+
+	for (k = 0; k < 4; k++) {
+		for (i = 0; i < nb_sensors; i++) {   /* Blend with neighbors. */
+			smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
+		}
+
+		for (i = 0; i < nb_sensors; i++) {
+			smooth[i] = smooth_tmp[i];
+			if (k == 3)     /* Last go-round, so scale back down. */
+				smooth[i] = smooth[i] >> 12;
+		}
+
+		smooth[-1] = smooth[0] >> 2;
+		smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;
+	}
+
 	for (i = 0; i < nb_sensors; i++) {
 		if (xy_sensors[i] < threshold) {
 			if (is_increasing)
 				is_increasing = 0;
 
-			continue;
-		}
-
 		/*
 		 * Makes the finger detection more versatile.  For example,
 		 * two fingers with no gap will be detected.  Also, my
@@ -367,7 +397,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 		 *
 		 * - Jason Parekh <jasonparekh@gmail.com>
 		 */
-		if (i < 1 ||
+
+		} else if (i < 1 ||
 		    (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
 			(*fingers)++;
 			is_increasing = 1;
@@ -375,15 +406,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 			is_increasing = 0;
 		}
 
-		/*
-		 * Subtracts threshold so a high sensor that just passes the
-		 * threshold won't skew the calculated absolute coordinate.
-		 * Fixes an issue where slowly moving the mouse would
-		 * 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);
+		pcum += (smooth[i]) * i;
+		psum += (smooth[i]);
 	}
 
 	if (psum > 0) {
@@ -565,8 +589,8 @@ 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;
+			x = (dev->x_old * 7 + x) >> 3;
+			y = (dev->y_old * 7 + y) >> 3;
 			dev->x_old = x;
 			dev->y_old = y;
 
@@ -677,8 +701,8 @@ 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;
+			x = (dev->x_old * 7 + x) >> 3;
+			y = (dev->y_old * 7 + y) >> 3;
 			dev->x_old = x;
 			dev->y_old = y;
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 3/3] input: appletouch: fix jumps when additional fingers are detected
From: Clinton Sprain @ 2014-02-07  0:46 UTC (permalink / raw)
  To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
In-Reply-To: <52F42AEE.3040505@gmail.com>

Discard cursor movements if they directly coincide with a change in the number of fingers detected. This helps with two issues - a sudden jump of the cursor when a second finger enters or leaves the touchpad, and an unexpected jump in page scrolling under the same scenario. The fix doesn't completely eliminate the problem but does greatly reduce its frequency and severity.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index edbdd95..370d0e9 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -212,6 +212,7 @@ struct atp {
 	signed char		xy_old[ATP_XSENSORS + ATP_YSENSORS];
 	int			xy_acc[ATP_XSENSORS + ATP_YSENSORS];
 	int			idlecount;	/* number of empty packets */
+	int			fingers_old;	/* last reported finger count */
 	struct work_struct	work;
 };
 
@@ -505,6 +506,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 	int x, y, x_z, y_z, x_f, y_f;
 	int retval, i, j;
 	int key;
+	int fingers;
 	struct atp *dev = urb->context;
 	int status = atp_status_check(urb);
 
@@ -587,7 +589,9 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 			      dev->info->yfact, &y_z, &y_f);
 	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
 
-	if (x && y) {
+	fingers = max(x_f, y_f);
+
+	if (x && y && (fingers == dev->fingers_old)) {
 		if (dev->x_old != -1) {
 			x = (dev->x_old * 7 + x) >> 3;
 			y = (dev->y_old * 7 + y) >> 3;
@@ -604,7 +608,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 			input_report_abs(dev->input, ABS_Y, y);
 			input_report_abs(dev->input, ABS_PRESSURE,
 					 min(ATP_PRESSURE, x_z + y_z));
-			atp_report_fingers(dev->input, max(x_f, y_f));
+			atp_report_fingers(dev->input, fingers);
 		}
 		dev->x_old = x;
 		dev->y_old = y;
@@ -620,6 +624,10 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
 	}
 
+	if (fingers != dev->fingers_old)
+		dev->x_old = dev->y_old = -1;
+	dev->fingers_old = fingers;
+
 	input_report_key(dev->input, BTN_LEFT, key);
 	input_sync(dev->input);
 
@@ -638,6 +646,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 	int x, y, x_z, y_z, x_f, y_f;
 	int retval, i, j;
 	int key;
+	int fingers;
 	struct atp *dev = urb->context;
 	int status = atp_status_check(urb);
 
@@ -699,7 +708,9 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 			      dev->info->yfact, &y_z, &y_f);
 	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
 
-	if (x && y) {
+	fingers = max(x_f, y_f);
+
+	if (x && y && (fingers == dev->fingers_old)) {
 		if (dev->x_old != -1) {
 			x = (dev->x_old * 7 + x) >> 3;
 			y = (dev->y_old * 7 + y) >> 3;
@@ -716,7 +727,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 			input_report_abs(dev->input, ABS_Y, y);
 			input_report_abs(dev->input, ABS_PRESSURE,
 					 min(ATP_PRESSURE, x_z + y_z));
-			atp_report_fingers(dev->input, max(x_f, y_f));
+			atp_report_fingers(dev->input, fingers);
 		}
 		dev->x_old = x;
 		dev->y_old = y;
@@ -732,6 +743,10 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
 	}
 
+	if (fingers != dev->fingers_old)
+		dev->x_old = dev->y_old = -1;
+	dev->fingers_old = fingers;
+
 	input_report_key(dev->input, BTN_LEFT, key);
 	input_sync(dev->input);
 
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH 05/15] Input: synaptics-rmi4 - remove gpio handling and polling
From: Courtney Cavin @ 2014-02-07  1:45 UTC (permalink / raw)
  To: Christopher Heiny
  Cc: Linus Walleij, linux-input@vger.kernel.org,
	dmitry.torokhov@gmail.com, Linus WALLEIJ
In-Reply-To: <52F3EAF5.7030203@synaptics.com>

On Thu, Feb 06, 2014 at 09:05:09PM +0100, Christopher Heiny wrote:
> On 02/06/2014 01:28 AM, Linus Walleij wrote:
> > On Wed, Feb 5, 2014 at 3:31 AM, Courtney Cavin
> > <courtney.cavin@sonymobile.com>  wrote:
> >> >On Wed, Feb 05, 2014 at 12:08:35AM +0100, Christopher Heiny wrote:
> >>> >>On 01/23/2014 04:00 PM, Courtney Cavin wrote:
> >>>> >> >Since all the configuration needed for an irq can be provided in other
> >>>> >> >ways, remove all gpio->irq functionality. This cleans up the code quite
> >>>> >> >a bit.
> >>> >>
> >>> >>In certain diagnostic modes, we need to be able to release the IRQ so
> >>> >>the GPIO can be monitored from userspace.  This patch removes that
> >>> >>capability.
> >> >
> >> >Polling a GPIO from userspace is poor design regardless of the use-case, if
> >> >you ask me.  It certainly doesn't motivate the extra gpio<->IRQ code.
>  >
> > I think Courtney is right here, if you want this kind of stuff for debugging
> > it should be debug-add-on-patches, not part of the normal execution
> > path for the driver.
> 
> I'm OK with replacing the current polling implementation with an 
> add-on-patch, as long as the support for the debug-add-on-patches 
> doesn't get torn out of the driver because "hey, I don't see what this 
> is used for in the core driver code, so it must be unnecessary".

An add-on patch shouldn't force dependencies on the code which it
patches.  Thus, the code shouldn't need support for debug add-ons.

Besides, try "hey, this isn't used in the core driver code, so it is
unnecessary."  This whole "there's another tree which depends on things
staying the way they are," isn't conducive to actually making changes to
improve the driver.

> >>> >>As mentioned in previous patch discussions, the polling functionality is
> >>> >>quite useful during new system integration, so we need to retain that.
> >>> >>If you have a proposal for where else it could be done, we'd certainly
> >>> >>entertain a patch that implements that.
> >> >
> >> >Do you actually have systems that have these hooked up to GPIOs without
> >> >IRQ support?
> >> >
> >> >Regardless, if this is the case, implementing a GPIO polling IRQ chip
> >> >should be possible, if extremely ugly.
> > OMG that would be quite horrible.
> >
> >> >Linus may have some comments in this area, though.  Linus?
>  >
> > A driver may rely on polling but a driver may not rely on polling
> > done from userspace.
> 
> Fortunately, the driver doesn't rely on polling from user space.
> --
> 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: [PATCH 05/15] Input: synaptics-rmi4 - remove gpio handling and polling
From: Courtney Cavin @ 2014-02-07  1:47 UTC (permalink / raw)
  To: Christopher Heiny
  Cc: linux-input@vger.kernel.org, dmitry.torokhov@gmail.com,
	linus.walleij@stericsson.com, linus.walleij@linaro.org
In-Reply-To: <52F3EAF1.9000505@synaptics.com>

On Thu, Feb 06, 2014 at 09:05:05PM +0100, Christopher Heiny wrote:
> On 02/04/2014 06:31 PM, Courtney Cavin wrote:
> > On Wed, Feb 05, 2014 at 12:08:35AM +0100, Christopher Heiny wrote:
> >> On 01/23/2014 04:00 PM, Courtney Cavin wrote:
> >>> Since all the configuration needed for an irq can be provided in other
> >>> ways, remove all gpio->irq functionality. This cleans up the code quite
> >>> a bit.
> >>
> >> In certain diagnostic modes, we need to be able to release the IRQ so
> >> the GPIO can be monitored from userspace.  This patch removes that
> >> capability.
> >>
> >
> > Polling a GPIO from userspace is poor design regardless of the use-case, if
> > you ask me.  It certainly doesn't motivate the extra gpio<->IRQ code.
> 
> Fortunately, neither the in-driver polling implementation nor the user
> space diagnostic applications are implemented that way.

I'm missing something here then.  If the userspace diagnostic
applications do not poll the GPIO, then what exactly is the purpose of
them accessing the GPIO at all?

^ permalink raw reply

* Fwd: FW: Dell Latitude E7440 - ALPS touchpad keeps having state reset
From: Tommy Will @ 2014-02-07  7:46 UTC (permalink / raw)
  To: allan; +Cc: linux-input@vger.kernel.org
In-Reply-To: <CA+F6ckM4gTBAE5Z3ymc=V8Oo73CV4p5tRTCZYMMVi2+3SK+U6g@mail.gmail.com>

Hi Allan,

This is Tommy from ALPS. Thanks for your contact and so sorry for my
overlooking your last week's mail.

> I've reported the problem I'm having in more detail here:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1258837
>
> In summary - I keep suffering from the mouse pointer jumping around erratically, with the following messages output to syslog:
>  psmouse serio1: DualPoint TouchPad at isa0060/serio1/input0 lost sync at byte 1  psmouse serio1: DualPoint TouchPad at isa0060/serio1/input0 - driver resynced.
>
> Other people have reported the same problem in that bug report.
> Bisecting the kernel, the problem seems to have always been in place as soon as the touchpad was correctly identified; according to xinput
> - it's a "AlpsPS/2 ALPS DualPoint TouchPad". When it was being identified as a generic PS/2 mouse, there's no problem (though of course, there's also no touchpad functionality available either).
>

It seems that with the latest kernel, your touchpad became crazy
erratically and almost cannot use?
It looks like that our kernel driver did not let touchpad enter the
correct mode or did not use the correct way to decode the raw data.

>From another mail sent by Vasily, he met the same issue with touchpad
on Dell E7440 as yours.
According to his touchpad info "E7: 73-03-0A EC: 88-08-22", I think we
can start the investigation.
Please give me some time and I'll feedback you later. Thanks!

--
Best Regards,
Tommy

^ permalink raw reply

* ShuttlePROv2 jog dial reports EV_REL events with absolute position event values; pos 0 events get dropped
From: Harald Albrecht @ 2014-02-07  7:51 UTC (permalink / raw)
  To: linux-input

I have a ShuttlePRO v2 (Contour Design Inc) that identifies itself as a 
USB HID. The kernel properly detects its and I can receive events from 
the device. The ShuttlePRO v2 is a media controller for use with video 
editing software, as well as for many other applications. The controller 
itself has a jog dial for frame-by-frame movement. Next, it has a 
so-called shuttle, which is ring around the dial, but separate from it. 
The shuttle can be turned CW and CCW up to 7 positions and a spring 
ensures it returns to its center position when you take your hand off of 
it. The shuttle is used to play video or audio forwards or backwards, 
with varying speed. Finally, there are 15 buttons on the ShuttlePRO v2.

However, there is an annoying quirk of which I don't at this time where 
it comes from. The Linux event/input system does not report when the 
shuttle returns to its home/center position. It seems as if the event 
sent by the device gets swallowed. I know that the device sends such an 
event, as in Windows there is no such problem. Similar, when turning the 
jog dial, every 256th position gets swallowed as well.

I suspect the device manufacturer to have messed up this, but I may be 
wrong. Anyway, I'm looking for help to get around this annoying quirk.

When I point evtest at the ShuttlePro v2, it reports the following 
information:

Input driver version is 1.0.1
Input device ID: bus 0x3 vendor 0xb33 product 0x30 version 0x110
Input device name: "Contour Design ShuttlePRO v2"
Supported events:
   Event type 0 (EV_SYN)
   Event type 1 (EV_KEY)
     Event code 256 (BTN_0)
     Event code 257 (BTN_1)
     Event code 258 (BTN_2)
     Event code 259 (BTN_3)
     Event code 260 (BTN_4)
     Event code 261 (BTN_5)
     Event code 262 (BTN_6)
     Event code 263 (BTN_7)
     Event code 264 (BTN_8)
     Event code 265 (BTN_9)
     Event code 266 (?)
     Event code 267 (?)
     Event code 268 (?)
     Event code 269 (?)
     Event code 270 (?)
   Event type 2 (EV_REL)
     Event code 7 (REL_DIAL)
     Event code 8 (REL_WHEEL)
   Event type 4 (EV_MSC)
     Event code 4 (MSC_SCAN)

Please notice that for whatever reason, the ShuttlePRO v2 uses relative 
events for the dial and shuttle (wheel). Unfortunately, the device sends 
events that contain the dial and shuttle absolute positions.

Please notice that for whatever reason, the ShuttlePRO v2 uses relative 
events for the dial and shuttle (wheel). At least that is my 
understanding of the output from evtest. Unfortunately, the ShuttlePRO 
device sends events that contain the dial and shuttle absolute 
positions. This can be clearly seen from evtest output:

Event: type 2 (EV_REL), code 8 (REL_WHEEL), value 1
Event: -------------- SYN_REPORT ------------
Event: type 2 (EV_REL), code 8 (REL_WHEEL), value 2
Event: -------------- SYN_REPORT ------------
Event: type 2 (EV_REL), code 8 (REL_WHEEL), value 1
Event: -------------- SYN_REPORT ------------
Event: type 2 (EV_REL), code 8 (REL_WHEEL), value -1
Event: -------------- SYN_REPORT ------------

My impression is that the Linux HID module correctly assumes that 
relative events without any position movement can be dropped. Can 
someone please confirm, if this actually is the case?

The issue for the jog dial is similar, here whenever the position 
counter which is in the range of 0 to 255 doesn't get reported when it 
reaches pos 0.

Event: type 2 (EV_REL), code 7 (REL_DIAL), value 2
Event: -------------- SYN_REPORT ------------
Event: type 2 (EV_REL), code 7 (REL_DIAL), value 1
Event: -------------- SYN_REPORT ------------
Event: type 2 (EV_REL), code 7 (REL_DIAL), value 255
Event: -------------- SYN_REPORT ------------

Is there any change of getting the handling of the ShuttlePRO v2 fixed 
i? I asked on the linux-usb mailing list and someone pointed me in the 
direction of this mailing list. As I see the situation, it would be 
preferable if dropping the events could be fixed because then existing 
software does not need to be changed, as it currently expects REL_DIAL 
and REL_WHEEL events. In contrast, trying to fix the issue by mapping 
the EV_REL to EV_ABS events, would cause all existing software that 
works with the ShuttlePRO v2 to break completely.

Any help and points greatly appreciated!
-- TheDiveO

^ permalink raw reply

* Re: PROBLEM: [Dell LATITUDE E7440] ALPS touchpad events are not routed into /dev/input/mice and /dev/input/mouseX
From: Tommy Will @ 2014-02-07  8:18 UTC (permalink / raw)
  To: qehgt0; +Cc: linux-input@vger.kernel.org

Hi Vasily,

Thanks for your mail & information. We would start the investigation
and give you feedback later.
Please wait.

> The issues occurs on every kernel after
> 1302bac33d9e88cd43e482191a806998f3ed43cc commit. Before the commit, this touchpad device was detected as PS/2 mouse and "cat /dev/input/mice" shows data when the touchpad is used.
> After the commit, there are no events from the touchpad in /dev/input/mice or /dev/input/mouseX files anymore.
>
> According to drivers/input/mouse/alps.c source file, all devices with signatures "ec[0] == 0x88 && ec[1] == 0x08" are treated as "Rushmore touchpads". This touchpad has E7 signature of "73-03-0A" and EC signature of "88-08-22", and it looks like that V5 protocol (not
> "Rushmore") should be used here. If V5 protocol is forced to use for this device then everything works fine.
>
> The patch is trivial, but I'm not sure how to properly separate "Rushmore" and "V5" devices. Are there any datasheets or experts to help me with it?
>

I checked the E7 & EC information and think it should be a Rushmore device.
Could you please let me know after you use V5 protocol, if touchpad
still be recognized as "AlpsPS/2 ALPS DualPoint TouchPad" or returns
to "PS/2 Generic Mouse" ?

Thanks
-- 
Best Regards,
Tommy

^ permalink raw reply

* Re: PROBLEM: [Dell LATITUDE E7440] ALPS touchpad events are not routed into /dev/input/mice and /dev/input/mouseX
From: Vasily Titskiy @ 2014-02-07 15:18 UTC (permalink / raw)
  To: Tommy Will; +Cc: linux-input@vger.kernel.org
In-Reply-To: <CA+F6ckO02oxrgx3-9Fhu3XQFX_8WrUFL5_LTjA8-mUpZMy7QaQ@mail.gmail.com>

Hi Tommy,

This is the output of `dmesg | grep -i 'input\|alps'` command after V5 enforced:
[    1.592880] input: Lid Switch as
/devices/LNXSYSTM:00/device:00/PNP0C0D:00/input/input0
[    1.593928] input: Power Button as
/devices/LNXSYSTM:00/device:00/PNP0C0C:00/input/input1
[    1.593964] input: Sleep Button as
/devices/LNXSYSTM:00/device:00/PNP0C0E:00/input/input2
[    1.593999] input: Power Button as
/devices/LNXSYSTM:00/LNXPWRBN:00/input/input3
[    1.788587] input: AT Translated Set 2 keyboard as
/devices/platform/i8042/serio0/input/input4
[    2.661783] input: PS/2 Mouse as /devices/platform/i8042/serio1/input/input5
[    2.674857] input: AlpsPS/2 ALPS GlidePoint as
/devices/platform/i8042/serio1/input/input6

...and before my patch (current kernel behavior):
[    1.604029] input: Lid Switch as
/devices/LNXSYSTM:00/device:00/PNP0C0D:00/input/input0
[    1.606813] input: Power Button as
/devices/LNXSYSTM:00/device:00/PNP0C0C:00/input/input1
[    1.608991] input: Sleep Button as
/devices/LNXSYSTM:00/device:00/PNP0C0E:00/input/input2
[    1.610997] input: Power Button as
/devices/LNXSYSTM:00/LNXPWRBN:00/input/input3
[    3.636959] input: AT Translated Set 2 keyboard as
/devices/platform/i8042/serio0/input/input4
[    8.377859] input: DualPoint Stick as
/devices/platform/i8042/serio1/input/input5
[    8.542194] input: AlpsPS/2 ALPS DualPoint TouchPad as
/devices/platform/i8042/serio1/input/input6

  Vasily Titskiy


On Fri, Feb 7, 2014 at 3:18 AM, Tommy Will <tommywill2011@gmail.com> wrote:
> Hi Vasily,
>
> Thanks for your mail & information. We would start the investigation
> and give you feedback later.
> Please wait.
>
>> The issues occurs on every kernel after
>> 1302bac33d9e88cd43e482191a806998f3ed43cc commit. Before the commit, this touchpad device was detected as PS/2 mouse and "cat /dev/input/mice" shows data when the touchpad is used.
>> After the commit, there are no events from the touchpad in /dev/input/mice or /dev/input/mouseX files anymore.
>>
>> According to drivers/input/mouse/alps.c source file, all devices with signatures "ec[0] == 0x88 && ec[1] == 0x08" are treated as "Rushmore touchpads". This touchpad has E7 signature of "73-03-0A" and EC signature of "88-08-22", and it looks like that V5 protocol (not
>> "Rushmore") should be used here. If V5 protocol is forced to use for this device then everything works fine.
>>
>> The patch is trivial, but I'm not sure how to properly separate "Rushmore" and "V5" devices. Are there any datasheets or experts to help me with it?
>>
>
> I checked the E7 & EC information and think it should be a Rushmore device.
> Could you please let me know after you use V5 protocol, if touchpad
> still be recognized as "AlpsPS/2 ALPS DualPoint TouchPad" or returns
> to "PS/2 Generic Mouse" ?
>
> Thanks
> --
> Best Regards,
> Tommy

^ permalink raw reply

* Re: PROBLEM: [Dell LATITUDE E7440] ALPS touchpad events are not routed into /dev/input/mice and /dev/input/mouseX
From: Kevin Cernekee @ 2014-02-07 16:46 UTC (permalink / raw)
  To: Vasily Titskiy; +Cc: Tommy Will, linux-input@vger.kernel.org
In-Reply-To: <CABSveYhh-3WVnEUKDaYjdbTBQLwPJ1YnJ3toT0kxXd=9aECHyA@mail.gmail.com>

On Fri, Feb 7, 2014 at 7:18 AM, Vasily Titskiy <qehgt0@gmail.com> wrote:
> Hi Tommy,
>
> This is the output of `dmesg | grep -i 'input\|alps'` command after V5 enforced:
> [    1.592880] input: Lid Switch as
> /devices/LNXSYSTM:00/device:00/PNP0C0D:00/input/input0
> [    1.593928] input: Power Button as
> /devices/LNXSYSTM:00/device:00/PNP0C0C:00/input/input1
> [    1.593964] input: Sleep Button as
> /devices/LNXSYSTM:00/device:00/PNP0C0E:00/input/input2
> [    1.593999] input: Power Button as
> /devices/LNXSYSTM:00/LNXPWRBN:00/input/input3
> [    1.788587] input: AT Translated Set 2 keyboard as
> /devices/platform/i8042/serio0/input/input4
> [    2.661783] input: PS/2 Mouse as /devices/platform/i8042/serio1/input/input5
> [    2.674857] input: AlpsPS/2 ALPS GlidePoint as
> /devices/platform/i8042/serio1/input/input6

Could you confirm that you're seeing actual ALPS V5 absolute reports
with the patched driver, not just emulated PS/2 mouse reports?

Do edge scrolling and multitouch work correctly on the patched driver?

> ...and before my patch (current kernel behavior):
> [    1.604029] input: Lid Switch as
> /devices/LNXSYSTM:00/device:00/PNP0C0D:00/input/input0
> [    1.606813] input: Power Button as
> /devices/LNXSYSTM:00/device:00/PNP0C0C:00/input/input1
> [    1.608991] input: Sleep Button as
> /devices/LNXSYSTM:00/device:00/PNP0C0E:00/input/input2
> [    1.610997] input: Power Button as
> /devices/LNXSYSTM:00/LNXPWRBN:00/input/input3
> [    3.636959] input: AT Translated Set 2 keyboard as
> /devices/platform/i8042/serio0/input/input4
> [    8.377859] input: DualPoint Stick as
> /devices/platform/i8042/serio1/input/input5
> [    8.542194] input: AlpsPS/2 ALPS DualPoint TouchPad as
> /devices/platform/i8042/serio1/input/input6

This laptop does have a trackstick, correct?

(I see it on the Dell spec sheet but I'd like to confirm your unit has it.)

Thanks.

^ permalink raw reply

* Re: PROBLEM: [Dell LATITUDE E7440] ALPS touchpad events are not routed into /dev/input/mice and /dev/input/mouseX
From: Vasily Titskiy @ 2014-02-07 17:44 UTC (permalink / raw)
  To: Kevin Cernekee; +Cc: Tommy Will, linux-input@vger.kernel.org
In-Reply-To: <CAJiQ=7BgrY2aASfv0N4xx6TnZa1DSgvZdAzcg5XNYiMcpeFkjA@mail.gmail.com>

Hi all,

On Fri, Feb 07, 2014 at 08:46:08AM -0800, Kevin Cernekee wrote:
> On Fri, Feb 7, 2014 at 7:18 AM, Vasily Titskiy <qehgt0@gmail.com> wrote:
> > Hi Tommy,
> >
> > [    2.674857] input: AlpsPS/2 ALPS GlidePoint as
> > /devices/platform/i8042/serio1/input/input6
> 
> Could you confirm that you're seeing actual ALPS V5 absolute reports
> with the patched driver, not just emulated PS/2 mouse reports?
> 
> Do edge scrolling and multitouch work correctly on the patched driver?
What is the easiest way to do it?

> 
> > ...and before my patch (current kernel behavior):
> > [    1.604029] input: Lid Switch as
> > /devices/LNXSYSTM:00/device:00/PNP0C0D:00/input/input0
> > [    1.606813] input: Power Button as
> > /devices/LNXSYSTM:00/device:00/PNP0C0C:00/input/input1
> > [    1.608991] input: Sleep Button as
> > /devices/LNXSYSTM:00/device:00/PNP0C0E:00/input/input2
> > [    1.610997] input: Power Button as
> > /devices/LNXSYSTM:00/LNXPWRBN:00/input/input3
> > [    3.636959] input: AT Translated Set 2 keyboard as
> > /devices/platform/i8042/serio0/input/input4
> > [    8.377859] input: DualPoint Stick as
> > /devices/platform/i8042/serio1/input/input5
> > [    8.542194] input: AlpsPS/2 ALPS DualPoint TouchPad as
> > /devices/platform/i8042/serio1/input/input6
> 
> This laptop does have a trackstick, correct?
> 
> (I see it on the Dell spec sheet but I'd like to confirm your unit has it.)
Yes, it does. There is a trackstick beetween G,H,B keys.

--
  Vasily

^ permalink raw reply

* Re: [PATCH v2 02/10] genirq: Add devm_request_any_context_irq()
From: Stephen Boyd @ 2014-02-07 21:08 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-arm-msm, Dmitry Torokhov, linux-kernel, linux-arm-kernel,
	linux-input
In-Reply-To: <52D5A266.5000504@codeaurora.org>

On 01/14, Stephen Boyd wrote:
> On 01/06/14 14:24, Stephen Boyd wrote:
> > On 01/02/14 16:37, Stephen Boyd wrote:
> >> Some drivers use request_any_context_irq() but there isn't a
> >> devm_* function for it. Add one so that these drivers don't need
> >> to explicitly free the irq on driver detach.
> >>
> >> Cc: Thomas Gleixner <tglx@linutronix.de>
> >> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> > Thomas, can you please review this patch?
> 
> ping?

ping?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply

* Re: [PATCH 2/2] iio: hid-sensor-hub: Remove hard coded indexes
From: Jonathan Cameron @ 2014-02-08 11:49 UTC (permalink / raw)
  To: Jiri Kosina, Srinivas Pandruvada
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <alpine.LNX.2.00.1401291412230.21385-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>

On 29/01/14 13:13, Jiri Kosina wrote:
> On Thu, 23 Jan 2014, Srinivas Pandruvada wrote:
>
>> Remove the hard coded indexes, instead search for usage id and
>> use the index to set the power and report state.
>> This will fix issue, where the report descriptor doesn't contain
>> the full list of possible selector for power and report state.
>>
>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> Jonathan, I'd probably take this together with the patch for HID API
> change. Could you please provide your Ack/Signoff, and I'll take it
> through my tree, if you agree?
Agreed. Help yourself ;)

Thanks and sorry for the slow response.  Amazing the backlog that builds
up if you take about 3 weeks out (more or less)
>
> Thanks.
>

^ permalink raw reply

* Re: [PATCH v2 2/3] input: appletouch: use better cursor movement smoothing algorithm
From: Henrik Rydberg @ 2014-02-09 12:15 UTC (permalink / raw)
  To: Clinton Sprain, linux-input; +Cc: Dmitry Torokhov
In-Reply-To: <52F42C32.8080408@gmail.com>

Hi Clinton,

> Use smoothed version of sensor array data to calculate movement and add weight
to prior values when calculating average. This gives more granular and more
predictable movement.
> 
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
>  drivers/input/mouse/appletouch.c |   60 ++++++++++++++++++++++++++------------
>  1 file changed, 42 insertions(+), 18 deletions(-)

I like this patch, but there are some technical glitches, comments below.

> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index d48181b..edbdd95 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -338,21 +338,51 @@ static void atp_reinit(struct work_struct *work)
>  static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>  			     int *z, int *fingers)
>  {
> -	int i;
> +	int i, k;
> +	int smooth[nb_sensors + 2];
> +	int smooth_tmp[nb_sensors + 2];
> +
>  	/* values to calculate mean */
>  	int pcum = 0, psum = 0;
>  	int is_increasing = 0;
>  
>  	*fingers = 0;
>  
> +	/*
> +	 * Use a smoothed version of sensor data for movement calculations, to
> +	 * combat noise without needing to toss out values based on a threshold.
> +	 * This improves tracking. Finger count is calculated separately based
> +	 * on raw data.
> +	 */
> +
> +	for (i = 0; i < nb_sensors; i++) {           /* Scale up to minimize */
> +		smooth[i] = xy_sensors[i] << 12;     /* rounding/truncation. */
> +	}
> +
> +	/* Mitigate some of the data loss from smoothing on the edge sensors. */
> +	smooth[-1] = smooth[0] >> 2;
> +	smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;

Out of bounds array... also, the boundary condition seems odd. If you want to
extrapolate the edge velocity, and assuming smooth[0] is the first sensor, you
would get something like (N = nb_sensors)

	smooth_tmp[0] = 2 * smooth[1] - smooth[2];
	smooth_tmp[N-1] = 2 * smooth[N-2] - smooth[N-3];

> +	for (k = 0; k < 4; k++) {
> +		for (i = 0; i < nb_sensors; i++) {   /* Blend with neighbors. */

And here would be for (i = 1; i < nb_sensors - 1; i++)

> +			smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
> +		}
> +
> +		for (i = 0; i < nb_sensors; i++) {
> +			smooth[i] = smooth_tmp[i];
> +			if (k == 3)     /* Last go-round, so scale back down. */
> +				smooth[i] = smooth[i] >> 12;

The scale-up is done separately, so why not make the scale-down separately too?

> +		}
> +
> +		smooth[-1] = smooth[0] >> 2;
> +		smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;

And these would be dropped.

> +	}
> +
>  	for (i = 0; i < nb_sensors; i++) {
>  		if (xy_sensors[i] < threshold) {
>  			if (is_increasing)
>  				is_increasing = 0;
>  
> -			continue;
> -		}
> -
>  		/*
>  		 * Makes the finger detection more versatile.  For example,
>  		 * two fingers with no gap will be detected.  Also, my
> @@ -367,7 +397,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>  		 *
>  		 * - Jason Parekh <jasonparekh@gmail.com>
>  		 */
> -		if (i < 1 ||
> +
> +		} else if (i < 1 ||
>  		    (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
>  			(*fingers)++;
>  			is_increasing = 1;
> @@ -375,15 +406,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>  			is_increasing = 0;
>  		}
>  
> -		/*
> -		 * Subtracts threshold so a high sensor that just passes the
> -		 * threshold won't skew the calculated absolute coordinate.
> -		 * Fixes an issue where slowly moving the mouse would
> -		 * 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);
> +		pcum += (smooth[i]) * i;
> +		psum += (smooth[i]);

Great, this makes so much more sense.

>  	}
>  
>  	if (psum > 0) {
> @@ -565,8 +589,8 @@ 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;
> +			x = (dev->x_old * 7 + x) >> 3;
> +			y = (dev->y_old * 7 + y) >> 3;
>  			dev->x_old = x;
>  			dev->y_old = y;
>  
> @@ -677,8 +701,8 @@ 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;
> +			x = (dev->x_old * 7 + x) >> 3;
> +			y = (dev->y_old * 7 + y) >> 3;
>  			dev->x_old = x;
>  			dev->y_old = y;

Would you say that the cursor slow-down here is noticeable, or are there enough
samples per second that it does not matter?

Thanks,
Henrik


^ permalink raw reply

* Re: [PATCH v2 3/3] input: appletouch: fix jumps when additional fingers are detected
From: Henrik Rydberg @ 2014-02-09 12:16 UTC (permalink / raw)
  To: Clinton Sprain, linux-input; +Cc: Dmitry Torokhov
In-Reply-To: <52F42CDD.30703@gmail.com>

On 02/07/2014 01:46 AM, Clinton Sprain wrote:
> Discard cursor movements if they directly coincide with a change in the number of fingers detected. This helps with two issues - a sudden jump of the cursor when a second finger enters or leaves the touchpad, and an unexpected jump in page scrolling under the same scenario. The fix doesn't completely eliminate the problem but does greatly reduce its frequency and severity.
> 
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
>  drivers/input/mouse/appletouch.c |   23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index edbdd95..370d0e9 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -212,6 +212,7 @@ struct atp {
>  	signed char		xy_old[ATP_XSENSORS + ATP_YSENSORS];
>  	int			xy_acc[ATP_XSENSORS + ATP_YSENSORS];
>  	int			idlecount;	/* number of empty packets */
> +	int			fingers_old;	/* last reported finger count */
>  	struct work_struct	work;
>  };
>  
> @@ -505,6 +506,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>  	int x, y, x_z, y_z, x_f, y_f;
>  	int retval, i, j;
>  	int key;
> +	int fingers;
>  	struct atp *dev = urb->context;
>  	int status = atp_status_check(urb);
>  
> @@ -587,7 +589,9 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>  			      dev->info->yfact, &y_z, &y_f);
>  	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
>  
> -	if (x && y) {
> +	fingers = max(x_f, y_f);
> +
> +	if (x && y && (fingers == dev->fingers_old)) {
>  		if (dev->x_old != -1) {
>  			x = (dev->x_old * 7 + x) >> 3;
>  			y = (dev->y_old * 7 + y) >> 3;
> @@ -604,7 +608,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>  			input_report_abs(dev->input, ABS_Y, y);
>  			input_report_abs(dev->input, ABS_PRESSURE,
>  					 min(ATP_PRESSURE, x_z + y_z));
> -			atp_report_fingers(dev->input, max(x_f, y_f));
> +			atp_report_fingers(dev->input, fingers);
>  		}
>  		dev->x_old = x;
>  		dev->y_old = y;
> @@ -620,6 +624,10 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>  		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
>  	}
>  
> +	if (fingers != dev->fingers_old)
> +		dev->x_old = dev->y_old = -1;
> +	dev->fingers_old = fingers;
> +
>  	input_report_key(dev->input, BTN_LEFT, key);
>  	input_sync(dev->input);
>  
> @@ -638,6 +646,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>  	int x, y, x_z, y_z, x_f, y_f;
>  	int retval, i, j;
>  	int key;
> +	int fingers;
>  	struct atp *dev = urb->context;
>  	int status = atp_status_check(urb);
>  
> @@ -699,7 +708,9 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>  			      dev->info->yfact, &y_z, &y_f);
>  	key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
>  
> -	if (x && y) {
> +	fingers = max(x_f, y_f);
> +
> +	if (x && y && (fingers == dev->fingers_old)) {
>  		if (dev->x_old != -1) {
>  			x = (dev->x_old * 7 + x) >> 3;
>  			y = (dev->y_old * 7 + y) >> 3;
> @@ -716,7 +727,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>  			input_report_abs(dev->input, ABS_Y, y);
>  			input_report_abs(dev->input, ABS_PRESSURE,
>  					 min(ATP_PRESSURE, x_z + y_z));
> -			atp_report_fingers(dev->input, max(x_f, y_f));
> +			atp_report_fingers(dev->input, fingers);
>  		}
>  		dev->x_old = x;
>  		dev->y_old = y;
> @@ -732,6 +743,10 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>  		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
>  	}
>  
> +	if (fingers != dev->fingers_old)
> +		dev->x_old = dev->y_old = -1;
> +	dev->fingers_old = fingers;
> +
>  	input_report_key(dev->input, BTN_LEFT, key);
>  	input_sync(dev->input);
>  
> 

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

Thanks,
Henrik


^ permalink raw reply

* Re: [PATCH v2 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold
From: Henrik Rydberg @ 2014-02-09 12:01 UTC (permalink / raw)
  To: Clinton Sprain, linux-input; +Cc: Dmitry Torokhov
In-Reply-To: <52F42B68.9010905@gmail.com>

Hi Clinton,

On 02/07/2014 01:40 AM, Clinton Sprain wrote:
> Dials back the default fuzz and threshold settings for most devices using this driver, based on user input from forums and bug reports, increasing sensitivity. These two settings can also now both be overridden as module parameters in the event that users have hardware that does not respond well to the new defaults, or there is a desire to change the values for devices that do not have new defaults.
> 
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
>  drivers/input/mouse/appletouch.c |   48 ++++++++++++++++++++++++--------------
>  1 file changed, 31 insertions(+), 17 deletions(-)

Thanks for the patches, they seem like a good improvement. There are still some
comments, though, you will find them inline.

> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index 800ca7d..d48181b 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -48,6 +48,8 @@ struct atp_info {
>  	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);
> @@ -61,6 +63,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 = {
> @@ -71,6 +75,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 = {
> @@ -81,6 +87,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 = {
> @@ -90,6 +98,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 = {
> @@ -99,6 +109,8 @@ static const struct atp_info geyser4_info = {
>  	.yfact		= 64,
>  	.datalen	= 64,
>  	.callback	= atp_complete_geyser_3_4,
> +	.fuzz		= 0,
> +	.threshold	= 3,
>  };
>  
>  #define ATP_DEVICE(prod, info)					\
> @@ -155,18 +167,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
> @@ -235,14 +238,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.");

Given that there already is a userland interface for the fuzz parameter - and it
is indeed in frequent use - this one does not seem warranted. If we are unsure
if the changes to the default are not all good, perhaps they should not be
changed at all?

>  
>  static int debug;
>  module_param(debug, int, 0644);
> @@ -455,7 +464,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;
>  		}
>  	}
> @@ -808,6 +817,11 @@ 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;
> @@ -843,10 +857,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);
> 

Thanks,
Henrik


^ permalink raw reply

* [PATCH 0/7] Remove HAVE_PWM config option
From: Jingoo Han @ 2014-02-10  1:07 UTC (permalink / raw)
  To: 'Arnd Bergmann', 'Ralf Baechle',
	'Dmitry Torokhov', 'Thierry Reding'
  Cc: 'Linus Walleij', 'Russell King - ARM Linux',
	'Eric Miao', linux-kernel, linux-arm-kernel, linux-mips,
	linux-input, linux-pwm, 'Jingoo Han'

The HAVE_PWM symbol is only for legacy platforms that provide
the PWM API without using the generic framework, while PWM symbol
is used for PWM drivers using the generic PWM framework.

I looked at all HAVE_PWMs in the latest mainline kernel 3.14-rc1.
Three platforms are still using HAVE_PWM as below:

1. ARM - PXA
  ./arch/arm/mach-pxa/Kconfig

2. ARM - NXP LPC32XX
  ./arch/arm/Kconfig
  config ARCH_LPC32XX
  	select HAVE_PWM

3. MIPS - Ingenic JZ4740 based machines
  ./arch/mips/Kconfig
  config MACH_JZ4740
  	select HAVE_PWM

However, the legacy PWM drivers for PXA, LPC32XX, and JZ474 were
already moved to the generic PWM framework.
  ./drivers/pwm/pwm-pxa.c
  ./drivers/pwm/pwm-lpc32xx.c
  ./drivers/pwm/pwm-jz4740.c

In conclusion, HAVE_PWM should be removed, because HAVE_PWM is
NOT required anymore.

Jingoo Han (7):
      ARM: pxa: don't select HAVE_PWM
      ARM: lpc32xx: don't select HAVE_PWM
      ARM: remove HAVE_PWM config option
      MIPS: jz4740: don't select HAVE_PWM
      Input: max8997_haptic: remove HAVE_PWM dependencies
      Input: pwm-beepe: remove HAVE_PWM dependencies
      pwm: don't use IS_ENABLED(CONFIG_HAVE_PWM)

 arch/arm/Kconfig           |    4 ----
 arch/arm/mach-pxa/Kconfig  |   15 ---------------
 arch/mips/Kconfig          |    1 -
 drivers/input/misc/Kconfig |    4 ++--
 include/linux/pwm.h        |    2 +-
 5 files changed, 3 insertions(+), 23 deletions(-)

I would like to merge these patches as below:

1. Through arm-soc tree
  [PATCH 1/7] ARM: pxa: don't select HAVE_PWM
  [PATCH 2/7] ARM: lpc32xx: don't select HAVE_PWM
  [PATCH 3/7] ARM: remove HAVE_PWM config option

2. Through MIPS tree
  [PATCH 4/7] MIPS: jz4740: don't select HAVE_PWM

3. Through Input tree
  [PATCH 5/7] Input: max8997_haptic: remove HAVE_PWM dependencies
  [PATCH 6/7] Input: pwm-beepe: remove HAVE_PWM dependencies

4. Through PWM tree
  [PATCH 7/7] pwm: don't use IS_ENABLED(CONFIG_HAVE_PWM)

After merging these patches, all HAVE_PWM will be removed from
the mainline kernel. Thank you. :-)

Best regards,
Jingoo Han


^ permalink raw reply

* [PATCH 5/7] Input: max8997_haptic: remove HAVE_PWM dependencies
From: Jingoo Han @ 2014-02-10  1:15 UTC (permalink / raw)
  To: 'Dmitry Torokhov'; +Cc: linux-kernel, linux-input
In-Reply-To: <003901cf25fc$73002790$590076b0$%han@samsung.com>

The HAVE_PWM symbol is only for legacy platforms that provide
the PWM API without using the generic framework. However, legacy
PWM drivers were already moved to the generic PWM framework.
Thus, HAVE_PWM should be removed, because HAVE_PWM is not
required anymore.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/input/misc/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 7904ab0..ae74df7 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -156,7 +156,7 @@ config INPUT_MAX8925_ONKEY
 
 config INPUT_MAX8997_HAPTIC
 	tristate "MAXIM MAX8997 haptic controller support"
-	depends on PWM && HAVE_PWM && MFD_MAX8997
+	depends on PWM && MFD_MAX8997
 	select INPUT_FF_MEMLESS
 	help
 	  This option enables device driver support for the haptic controller
-- 
1.7.10.4



^ permalink raw reply related

* [PATCH 6/7] Input: pwm-beepe: remove HAVE_PWM dependencies
From: Jingoo Han @ 2014-02-10  1:16 UTC (permalink / raw)
  To: 'Dmitry Torokhov'
  Cc: linux-kernel, linux-input, 'Jingoo Han',
	'Lars-Peter Clausen'
In-Reply-To: <003901cf25fc$73002790$590076b0$%han@samsung.com>

The HAVE_PWM symbol is only for legacy platforms that provide
the PWM API without using the generic framework. However, legacy
PWM drivers were already moved to the generic PWM framework.
Thus, HAVE_PWM should be removed, because HAVE_PWM is not
required anymore.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/input/misc/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index ae74df7..762e6d2 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -470,7 +470,7 @@ config INPUT_PCF8574
 
 config INPUT_PWM_BEEPER
 	tristate "PWM beeper support"
-	depends on PWM && HAVE_PWM
+	depends on PWM
 	help
 	  Say Y here to get support for PWM based beeper devices.
 
-- 
1.7.10.4



^ permalink raw reply related

* [PATCH RESEND] input: Add commonly used event types
From: Alexander Shiyan @ 2014-02-10  4:21 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, devicetree, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
	Alexander Shiyan

Patch adds commonly used event types (EV_KEY and EV_SW) to
devicetree bindings header for input subsystem.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 include/dt-bindings/input/input.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/dt-bindings/input/input.h b/include/dt-bindings/input/input.h
index 042e7b3..3a141d92 100644
--- a/include/dt-bindings/input/input.h
+++ b/include/dt-bindings/input/input.h
@@ -9,6 +9,9 @@
 #ifndef _DT_BINDINGS_INPUT_INPUT_H
 #define _DT_BINDINGS_INPUT_INPUT_H
 
+#define EV_KEY			0x01
+#define EV_SW			0x05
+
 #define KEY_RESERVED		0
 #define KEY_ESC			1
 #define KEY_1			2
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH v2 1/2] input: Add new driver for ARM CLPS711X keypad
From: Alexander Shiyan @ 2014-02-10  4:22 UTC (permalink / raw)
  To: linux-input; +Cc: Dmitry Torokhov, Alexander Shiyan

This patch adds a new driver for keypad for Cirrus Logic CLPS711X CPUs.
Target CPU contain keyboard interface which can scan 8 column lines,
so we can read row GPIOs to read status and determine asserted state.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/input/keyboard/Kconfig           |  12 ++
 drivers/input/keyboard/Makefile          |   1 +
 drivers/input/keyboard/clps711x-keypad.c | 209 +++++++++++++++++++++++++++++++
 3 files changed, 222 insertions(+)
 create mode 100644 drivers/input/keyboard/clps711x-keypad.c

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index a673c9f..45274e3 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -151,6 +151,18 @@ config KEYBOARD_BFIN
 	  To compile this driver as a module, choose M here: the
 	  module will be called bf54x-keys.
 
+config KEYBOARD_CLPS711X
+	tristate "CLPS711X Keypad support"
+	depends on OF_GPIO && (ARCH_CLPS711X || COMPILE_TEST)
+	select INPUT_MATRIXKMAP
+	select INPUT_POLLDEV
+	help
+	  Say Y here to enable the matrix keypad on the Cirrus Logic
+	  CLPS711X CPUs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called clps711x-keypad.
+
 config KEYBOARD_LKKBD
 	tristate "DECstation/VAXstation LK201/LK401 keyboard"
 	select SERIO
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index a699b61..f8589fe 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_KEYBOARD_AMIGA)		+= amikbd.o
 obj-$(CONFIG_KEYBOARD_ATARI)		+= atakbd.o
 obj-$(CONFIG_KEYBOARD_ATKBD)		+= atkbd.o
 obj-$(CONFIG_KEYBOARD_BFIN)		+= bf54x-keys.o
+obj-$(CONFIG_KEYBOARD_CLPS711X)		+= clps711x-keypad.o
 obj-$(CONFIG_KEYBOARD_CROS_EC)		+= cros_ec_keyb.o
 obj-$(CONFIG_KEYBOARD_DAVINCI)		+= davinci_keyscan.o
 obj-$(CONFIG_KEYBOARD_EP93XX)		+= ep93xx_keypad.o
diff --git a/drivers/input/keyboard/clps711x-keypad.c b/drivers/input/keyboard/clps711x-keypad.c
new file mode 100644
index 0000000..1e2c93f
--- /dev/null
+++ b/drivers/input/keyboard/clps711x-keypad.c
@@ -0,0 +1,209 @@
+/*
+ * Cirrus Logic CLPS711X Keypad driver
+ *
+ * Copyright (C) 2014 Alexander Shiyan <shc_work@mail.ru>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/delay.h>
+#include <linux/input.h>
+#include <linux/input-polldev.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mfd/syscon/clps711x.h>
+
+#define CLPS711X_KEYPAD_COL_COUNT	8
+
+struct clps711x_gpio_data {
+	int	gpio;
+	bool	active_low;
+	DECLARE_BITMAP(last_state, CLPS711X_KEYPAD_COL_COUNT);
+};
+
+struct clps711x_keypad_data {
+	struct regmap			*syscon;
+	int				row_count;
+	unsigned int			row_shift;
+	struct clps711x_gpio_data	*gpio_data;
+};
+
+static void clps711x_keypad_poll(struct input_polled_dev *dev)
+{
+	const unsigned short *keycodes = dev->input->keycode;
+	struct clps711x_keypad_data *priv = dev->private;
+	int col, row;
+
+	for (col = 0; col < CLPS711X_KEYPAD_COL_COUNT; col++) {
+		/* Assert column */
+		regmap_update_bits(priv->syscon, SYSCON_OFFSET,
+				   SYSCON1_KBDSCAN_MASK,
+				   SYSCON1_KBDSCAN(8 + col));
+
+		/* Scan rows */
+		for (row = 0; row < priv->row_count; row++) {
+			struct clps711x_gpio_data *data = &priv->gpio_data[row];
+			bool state, state1;
+
+			/* Protection against fluctuations */
+			do {
+				state = gpio_get_value_cansleep(data->gpio);
+				udelay(1);
+				state1 = gpio_get_value_cansleep(data->gpio);
+			} while (state != state1);
+
+			state ^= data->active_low;
+
+			if (test_bit(col, data->last_state) != state) {
+				int code = MATRIX_SCAN_CODE(row, col,
+							    priv->row_shift);
+
+				if (state)
+					set_bit(col, data->last_state);
+				else
+					clear_bit(col, data->last_state);
+
+				input_event(dev->input, EV_MSC, MSC_SCAN, code);
+				if (keycodes[code])
+					input_report_key(dev->input,
+							 keycodes[code], state);
+			}
+		}
+
+		/* Set all columns to low */
+		regmap_update_bits(priv->syscon, SYSCON_OFFSET,
+				   SYSCON1_KBDSCAN_MASK, SYSCON1_KBDSCAN(1));
+	}
+
+	input_sync(dev->input);
+}
+
+static int clps711x_keypad_probe(struct platform_device *pdev)
+{
+	struct clps711x_keypad_data *priv;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct input_polled_dev *poll_dev;
+	u32 poll_interval;
+	int i, err;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->syscon = syscon_regmap_lookup_by_phandle(np, "syscon");
+	if (IS_ERR(priv->syscon))
+		return PTR_ERR(priv->syscon);
+
+	priv->row_count = of_gpio_named_count(np, "row-gpios");
+	if (priv->row_count < 1)
+		return -EINVAL;
+
+	priv->gpio_data = devm_kzalloc(dev, sizeof(*priv->gpio_data) *
+				       priv->row_count, GFP_KERNEL);
+	if (!priv->gpio_data)
+		return -ENOMEM;
+
+	priv->row_shift = get_count_order(CLPS711X_KEYPAD_COL_COUNT);
+
+	for (i = 0; i < priv->row_count; i++) {
+		struct clps711x_gpio_data *data = &priv->gpio_data[i];
+		enum of_gpio_flags flg;
+
+		data->gpio = of_get_named_gpio_flags(np, "row-gpios", i, &flg);
+		if (data->gpio < 0)
+			return data->gpio;
+
+		err = devm_gpio_request_one(dev, data->gpio, GPIOF_IN, NULL);
+		if (err)
+			return err;
+
+		data->active_low = flg & OF_GPIO_ACTIVE_LOW;
+	}
+
+	err = of_property_read_u32(np, "poll-interval", &poll_interval);
+	if (err)
+		return err;
+
+	poll_dev = input_allocate_polled_device();
+	if (!poll_dev)
+		return -ENOMEM;
+
+	poll_dev->private		= priv;
+	poll_dev->poll			= clps711x_keypad_poll;
+	poll_dev->poll_interval		= poll_interval;
+	poll_dev->input->name		= pdev->name;
+	poll_dev->input->dev.parent	= &pdev->dev;
+	poll_dev->input->id.bustype	= BUS_HOST;
+	poll_dev->input->id.vendor	= 0x0001;
+	poll_dev->input->id.product	= 0x0001;
+	poll_dev->input->id.version	= 0x0100;
+
+	err = matrix_keypad_build_keymap(NULL, NULL, priv->row_count,
+					 CLPS711X_KEYPAD_COL_COUNT,
+					 NULL, poll_dev->input);
+	if (err)
+		goto out_err;
+
+	input_set_capability(poll_dev->input, EV_MSC, MSC_SCAN);
+	if (of_property_read_bool(np, "autorepeat"))
+		__set_bit(EV_REP, poll_dev->input->evbit);
+
+	platform_set_drvdata(pdev, poll_dev);
+
+	/* Set all columns to low */
+	regmap_update_bits(priv->syscon, SYSCON_OFFSET, SYSCON1_KBDSCAN_MASK,
+			   SYSCON1_KBDSCAN(1));
+
+	err = input_register_polled_device(poll_dev);
+	if (err)
+		goto out_err;
+
+	/* Report initial state */
+	clps711x_keypad_poll(poll_dev);
+
+	return 0;
+
+out_err:
+	input_free_polled_device(poll_dev);
+
+	return err;
+}
+
+static int clps711x_keypad_remove(struct platform_device *pdev)
+{
+	struct input_polled_dev *poll_dev = platform_get_drvdata(pdev);
+
+	input_unregister_polled_device(poll_dev);
+	input_free_polled_device(poll_dev);
+
+	return 0;
+}
+
+static struct of_device_id clps711x_keypad_of_match[] = {
+	{ .compatible = "cirrus,clps711x-keypad", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, clps711x_keypad_of_match);
+
+static struct platform_driver clps711x_keypad_driver = {
+	.driver	= {
+		.name		= "clps711x-keypad",
+		.owner		= THIS_MODULE,
+		.of_match_table	= clps711x_keypad_of_match,
+	},
+	.probe	= clps711x_keypad_probe,
+	.remove	= clps711x_keypad_remove,
+};
+module_platform_driver(clps711x_keypad_driver);
+
+MODULE_AUTHOR("Alexander Shiyan <shc_work@mail.ru>");
+MODULE_DESCRIPTION("Cirrus Logic CLPS711X Keypad driver");
+MODULE_LICENSE("GPL");
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 1/2] input: gpio_keys_polled: Convert to devm-* API
From: Alexander Shiyan @ 2014-02-10  4:22 UTC (permalink / raw)
  To: linux-input; +Cc: Dmitry Torokhov, Alexander Shiyan

Replace existing resource handling in the driver with managed
device resource, this ensures more consistent error values and
simplifies error paths.
kzalloc -> devm_kzalloc
gpio_request_one -> devm_gpio_request_one

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/input/keyboard/gpio_keys_polled.c | 87 ++++++++-----------------------
 1 file changed, 23 insertions(+), 64 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index e571e19..3a5e49b 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -108,9 +108,7 @@ static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct
 	struct device_node *node, *pp;
 	struct gpio_keys_platform_data *pdata;
 	struct gpio_keys_button *button;
-	int error;
-	int nbuttons;
-	int i;
+	int i, nbuttons;
 
 	node = dev->of_node;
 	if (!node)
@@ -120,12 +118,10 @@ static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct
 	if (nbuttons == 0)
 		return NULL;
 
-	pdata = kzalloc(sizeof(*pdata) + nbuttons * (sizeof *button),
-			GFP_KERNEL);
-	if (!pdata) {
-		error = -ENOMEM;
-		goto err_out;
-	}
+	pdata = devm_kzalloc(dev, sizeof(*pdata) + nbuttons * sizeof(*button),
+			     GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
 
 	pdata->buttons = (struct gpio_keys_button *)(pdata + 1);
 	pdata->nbuttons = nbuttons;
@@ -146,12 +142,11 @@ static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct
 
 		gpio = of_get_gpio_flags(pp, 0, &flags);
 		if (gpio < 0) {
-			error = gpio;
-			if (error != -EPROBE_DEFER)
+			if (gpio != -EPROBE_DEFER)
 				dev_err(dev,
 					"Failed to get gpio flags, error: %d\n",
-					error);
-			goto err_free_pdata;
+					gpio);
+			return ERR_PTR(gpio);
 		}
 
 		button = &pdata->buttons[i++];
@@ -162,8 +157,7 @@ static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct
 		if (of_property_read_u32(pp, "linux,code", &button->code)) {
 			dev_err(dev, "Button without keycode: 0x%x\n",
 				button->gpio);
-			error = -EINVAL;
-			goto err_free_pdata;
+			return ERR_PTR(-EINVAL);
 		}
 
 		button->desc = of_get_property(pp, "label", NULL);
@@ -178,17 +172,10 @@ static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct
 			button->debounce_interval = 5;
 	}
 
-	if (pdata->nbuttons == 0) {
-		error = -EINVAL;
-		goto err_free_pdata;
-	}
+	if (!pdata->nbuttons)
+		return ERR_PTR(-EINVAL);
 
 	return pdata;
-
-err_free_pdata:
-	kfree(pdata);
-err_out:
-	return ERR_PTR(error);
 }
 
 static struct of_device_id gpio_keys_polled_of_match[] = {
@@ -228,24 +215,21 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 
 	if (!pdata->poll_interval) {
 		dev_err(dev, "missing poll_interval value\n");
-		error = -EINVAL;
-		goto err_free_pdata;
+		return -EINVAL;
 	}
 
-	bdev = kzalloc(sizeof(struct gpio_keys_polled_dev) +
-		       pdata->nbuttons * sizeof(struct gpio_keys_button_data),
-		       GFP_KERNEL);
+	bdev = devm_kzalloc(&pdev->dev, sizeof(struct gpio_keys_polled_dev) +
+		pdata->nbuttons * sizeof(struct gpio_keys_button_data),
+		GFP_KERNEL);
 	if (!bdev) {
 		dev_err(dev, "no memory for private data\n");
-		error = -ENOMEM;
-		goto err_free_pdata;
+		return -ENOMEM;
 	}
 
 	poll_dev = input_allocate_polled_device();
 	if (!poll_dev) {
 		dev_err(dev, "no memory for polled device\n");
-		error = -ENOMEM;
-		goto err_free_bdev;
+		return -ENOMEM;
 	}
 
 	poll_dev->private = bdev;
@@ -278,15 +262,15 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 		if (button->wakeup) {
 			dev_err(dev, DRV_NAME " does not support wakeup\n");
 			error = -EINVAL;
-			goto err_free_gpio;
+			goto err_out;
 		}
 
-		error = gpio_request_one(gpio, GPIOF_IN,
-					 button->desc ?: DRV_NAME);
+		error = devm_gpio_request_one(&pdev->dev, gpio, GPIOF_IN,
+					      button->desc ? : DRV_NAME);
 		if (error) {
 			dev_err(dev, "unable to claim gpio %u, err=%d\n",
 				gpio, error);
-			goto err_free_gpio;
+			goto err_out;
 		}
 
 		bdata->can_sleep = gpio_cansleep(gpio);
@@ -306,7 +290,7 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 	if (error) {
 		dev_err(dev, "unable to register polled device, err=%d\n",
 			error);
-		goto err_free_gpio;
+		goto err_out;
 	}
 
 	/* report initial state of the buttons */
@@ -316,45 +300,20 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_free_gpio:
-	while (--i >= 0)
-		gpio_free(pdata->buttons[i].gpio);
-
+err_out:
 	input_free_polled_device(poll_dev);
 
-err_free_bdev:
-	kfree(bdev);
-
-err_free_pdata:
-	/* If we have no platform_data, we allocated pdata dynamically.  */
-	if (!dev_get_platdata(&pdev->dev))
-		kfree(pdata);
-
 	return error;
 }
 
 static int gpio_keys_polled_remove(struct platform_device *pdev)
 {
 	struct gpio_keys_polled_dev *bdev = platform_get_drvdata(pdev);
-	const struct gpio_keys_platform_data *pdata = bdev->pdata;
-	int i;
 
 	input_unregister_polled_device(bdev->poll_dev);
 
-	for (i = 0; i < pdata->nbuttons; i++)
-		gpio_free(pdata->buttons[i].gpio);
-
 	input_free_polled_device(bdev->poll_dev);
 
-	/*
-	 * If we had no platform_data, we allocated pdata dynamically and
-	 * must free it here.
-	 */
-	if (!dev_get_platdata(&pdev->dev))
-		kfree(pdata);
-
-	kfree(bdev);
-
 	return 0;
 }
 
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 1/4] input: mc13783: Make mc13xxx_buttons_platform_data more flexible
From: Alexander Shiyan @ 2014-02-10  4:22 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Shawn Guo, Sascha Hauer, Samuel Ortiz, Lee Jones,
	Alexander Shiyan

Instead of define each button in separate variable, make an array
for storing all buttons. This change will help to add support for
other types of PMIC and add support for probing with devicetree
in the future.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 arch/arm/mach-imx/mach-mx31moboard.c   |  9 ++++--
 drivers/input/misc/mc13783-pwrbutton.c | 56 +++++++++++++++++-----------------
 include/linux/mfd/mc13xxx.h            | 28 +++++++++--------
 3 files changed, 49 insertions(+), 44 deletions(-)

diff --git a/arch/arm/mach-imx/mach-mx31moboard.c b/arch/arm/mach-imx/mach-mx31moboard.c
index b3738e6..7f13c47 100644
--- a/arch/arm/mach-imx/mach-mx31moboard.c
+++ b/arch/arm/mach-imx/mach-mx31moboard.c
@@ -276,9 +276,12 @@ static struct mc13xxx_leds_platform_data moboard_leds = {
 };
 
 static struct mc13xxx_buttons_platform_data moboard_buttons = {
-	.b1on_flags = MC13783_BUTTON_DBNC_750MS | MC13783_BUTTON_ENABLE |
-			MC13783_BUTTON_POL_INVERT,
-	.b1on_key = KEY_POWER,
+	.buttons[0] = {
+		.keycode	= KEY_POWER,
+		.flags		= MC13XXX_BUTTON_ENABLE |
+				  MC13XXX_BUTTON_DBNC_750MS |
+				  MC13XXX_BUTTON_POL_INVERT,
+	},
 };
 
 static struct mc13xxx_codec_platform_data moboard_codec = {
diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
index 0df6e8d..60be67a 100644
--- a/drivers/input/misc/mc13783-pwrbutton.c
+++ b/drivers/input/misc/mc13783-pwrbutton.c
@@ -36,7 +36,7 @@ struct mc13783_pwrb {
 #define MC13783_PWRB_B2_POL_INVERT	(1 << 1)
 #define MC13783_PWRB_B3_POL_INVERT	(1 << 2)
 	int flags;
-	unsigned short keymap[3];
+	unsigned short keymap[MAX13XXX_NUM_BUTTONS];
 };
 
 #define MC13783_REG_INTERRUPT_SENSE_1		5
@@ -116,24 +116,24 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
 		goto free_input_dev;
 	}
 
-	reg |= (pdata->b1on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON1BDBNC;
-	reg |= (pdata->b2on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON2BDBNC;
-	reg |= (pdata->b3on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON3BDBNC;
+	reg |= (pdata->buttons[0].flags & 0x3) << MC13783_POWER_CONTROL_2_ON1BDBNC;
+	reg |= (pdata->buttons[1].flags & 0x3) << MC13783_POWER_CONTROL_2_ON2BDBNC;
+	reg |= (pdata->buttons[2].flags & 0x3) << MC13783_POWER_CONTROL_2_ON3BDBNC;
 
 	priv->pwr = pwr;
 	priv->mc13783 = mc13783;
 
 	mc13xxx_lock(mc13783);
 
-	if (pdata->b1on_flags & MC13783_BUTTON_ENABLE) {
-		priv->keymap[0] = pdata->b1on_key;
-		if (pdata->b1on_key != KEY_RESERVED)
-			__set_bit(pdata->b1on_key, pwr->keybit);
+	if (pdata->buttons[0].flags & MC13XXX_BUTTON_ENABLE) {
+		priv->keymap[0] = pdata->buttons[0].keycode;
+		if (priv->keymap[0] != KEY_RESERVED)
+			__set_bit(priv->keymap[0], pwr->keybit);
 
-		if (pdata->b1on_flags & MC13783_BUTTON_POL_INVERT)
+		if (pdata->buttons[0].flags & MC13XXX_BUTTON_POL_INVERT)
 			priv->flags |= MC13783_PWRB_B1_POL_INVERT;
 
-		if (pdata->b1on_flags & MC13783_BUTTON_RESET_EN)
+		if (pdata->buttons[0].flags & MC13XXX_BUTTON_RESET_EN)
 			reg |= MC13783_POWER_CONTROL_2_ON1BRSTEN;
 
 		err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD1,
@@ -144,15 +144,15 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (pdata->b2on_flags & MC13783_BUTTON_ENABLE) {
-		priv->keymap[1] = pdata->b2on_key;
-		if (pdata->b2on_key != KEY_RESERVED)
-			__set_bit(pdata->b2on_key, pwr->keybit);
+	if (pdata->buttons[1].flags & MC13XXX_BUTTON_ENABLE) {
+		priv->keymap[1] = pdata->buttons[1].keycode;
+		if (priv->keymap[1] != KEY_RESERVED)
+			__set_bit(priv->keymap[1], pwr->keybit);
 
-		if (pdata->b2on_flags & MC13783_BUTTON_POL_INVERT)
+		if (pdata->buttons[1].flags & MC13XXX_BUTTON_POL_INVERT)
 			priv->flags |= MC13783_PWRB_B2_POL_INVERT;
 
-		if (pdata->b2on_flags & MC13783_BUTTON_RESET_EN)
+		if (pdata->buttons[1].flags & MC13XXX_BUTTON_RESET_EN)
 			reg |= MC13783_POWER_CONTROL_2_ON2BRSTEN;
 
 		err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD2,
@@ -163,15 +163,15 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (pdata->b3on_flags & MC13783_BUTTON_ENABLE) {
-		priv->keymap[2] = pdata->b3on_key;
-		if (pdata->b3on_key != KEY_RESERVED)
-			__set_bit(pdata->b3on_key, pwr->keybit);
+	if (pdata->buttons[2].flags & MC13XXX_BUTTON_ENABLE) {
+		priv->keymap[2] = pdata->buttons[2].keycode;
+		if (priv->keymap[2] != KEY_RESERVED)
+			__set_bit(priv->keymap[2], pwr->keybit);
 
-		if (pdata->b3on_flags & MC13783_BUTTON_POL_INVERT)
+		if (pdata->buttons[2].flags & MC13XXX_BUTTON_POL_INVERT)
 			priv->flags |= MC13783_PWRB_B3_POL_INVERT;
 
-		if (pdata->b3on_flags & MC13783_BUTTON_RESET_EN)
+		if (pdata->buttons[2].flags & MC13XXX_BUTTON_RESET_EN)
 			reg |= MC13783_POWER_CONTROL_2_ON3BRSTEN;
 
 		err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD3,
@@ -208,15 +208,15 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
 free_irq:
 	mc13xxx_lock(mc13783);
 
-	if (pdata->b3on_flags & MC13783_BUTTON_ENABLE)
+	if (pdata->buttons[2].flags & MC13XXX_BUTTON_ENABLE)
 		mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD3, priv);
 
 free_irq_b2:
-	if (pdata->b2on_flags & MC13783_BUTTON_ENABLE)
+	if (pdata->buttons[1].flags & MC13XXX_BUTTON_ENABLE)
 		mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD2, priv);
 
 free_irq_b1:
-	if (pdata->b1on_flags & MC13783_BUTTON_ENABLE)
+	if (pdata->buttons[0].flags & MC13XXX_BUTTON_ENABLE)
 		mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD1, priv);
 
 free_priv:
@@ -238,11 +238,11 @@ static int mc13783_pwrbutton_remove(struct platform_device *pdev)
 
 	mc13xxx_lock(priv->mc13783);
 
-	if (pdata->b3on_flags & MC13783_BUTTON_ENABLE)
+	if (pdata->buttons[2].flags & MC13XXX_BUTTON_ENABLE)
 		mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD3, priv);
-	if (pdata->b2on_flags & MC13783_BUTTON_ENABLE)
+	if (pdata->buttons[1].flags & MC13XXX_BUTTON_ENABLE)
 		mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD2, priv);
-	if (pdata->b1on_flags & MC13783_BUTTON_ENABLE)
+	if (pdata->buttons[0].flags & MC13XXX_BUTTON_ENABLE)
 		mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD1, priv);
 
 	mc13xxx_unlock(priv->mc13783);
diff --git a/include/linux/mfd/mc13xxx.h b/include/linux/mfd/mc13xxx.h
index a326c85..4cedec4 100644
--- a/include/linux/mfd/mc13xxx.h
+++ b/include/linux/mfd/mc13xxx.h
@@ -172,20 +172,22 @@ struct mc13xxx_leds_platform_data {
 	u32 led_control[MAX_LED_CONTROL_REGS];
 };
 
+#define MAX13XXX_NUM_BUTTONS	3
+
+struct mc13xxx_button {
+	u16		keycode;
+	unsigned int	flags;
+#define MC13XXX_BUTTON_DBNC_0MS		0
+#define MC13XXX_BUTTON_DBNC_30MS	1
+#define MC13XXX_BUTTON_DBNC_150MS	2
+#define MC13XXX_BUTTON_DBNC_750MS	3
+#define MC13XXX_BUTTON_ENABLE		(1 << 2)
+#define MC13XXX_BUTTON_POL_INVERT	(1 << 3)
+#define MC13XXX_BUTTON_RESET_EN		(1 << 4)
+};
+
 struct mc13xxx_buttons_platform_data {
-#define MC13783_BUTTON_DBNC_0MS		0
-#define MC13783_BUTTON_DBNC_30MS	1
-#define MC13783_BUTTON_DBNC_150MS	2
-#define MC13783_BUTTON_DBNC_750MS	3
-#define MC13783_BUTTON_ENABLE		(1 << 2)
-#define MC13783_BUTTON_POL_INVERT	(1 << 3)
-#define MC13783_BUTTON_RESET_EN		(1 << 4)
-	int b1on_flags;
-	unsigned short b1on_key;
-	int b2on_flags;
-	unsigned short b2on_key;
-	int b3on_flags;
-	unsigned short b3on_key;
+	struct mc13xxx_button	buttons[MAX13XXX_NUM_BUTTONS];
 };
 
 struct mc13xxx_ts_platform_data {
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH v2 2/2] input: clps711x-keypad: dts: Add bindings documentation
From: Alexander Shiyan @ 2014-02-10  4:22 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, devicetree, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
	Alexander Shiyan

This patch adds the devicetree documentation for the Cirrus Logic
CLPS711X keypad.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 .../devicetree/bindings/input/clps711x-keypad.txt  | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/clps711x-keypad.txt

diff --git a/Documentation/devicetree/bindings/input/clps711x-keypad.txt b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
new file mode 100644
index 0000000..bcf5f02
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
@@ -0,0 +1,29 @@
+* Cirrus Logic CLPS711X matrix keypad device tree bindings
+
+Required Properties:
+- compatible:    Should be "cirrus,clps711x-keypad".
+- syscon:        phandle to syscon device node which control keypad (SYSCON1).
+- row-gpios:     List of GPIOs used as row lines.
+- poll-interval: Poll interval time in milliseconds.
+- linux,keymap:  The definition can be found at
+                 bindings/input/matrix-keymap.txt.
+
+Optional Properties:
+- autorepeat:    Enable autorepeat feature.
+
+Example:
+	keypad {
+		compatible = "cirrus,clps711x-keypad";
+		syscon = <&syscon1>;
+		autorepeat;
+		poll-interval = <120>;
+		row-gpios = <&porta 0 0>,
+			    <&porta 1 0>;
+
+		linux,keymap = <
+			MATRIX_KEY(0, 0, KEY_UP)
+			MATRIX_KEY(0, 1, KEY_DOWN)
+			MATRIX_KEY(1, 0, KEY_LEFT)
+			MATRIX_KEY(1, 1, KEY_RIGHT)
+		>;
+	};
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 2/2] input: gpio_keys: Convert to devm-* API
From: Alexander Shiyan @ 2014-02-10  4:22 UTC (permalink / raw)
  To: linux-input; +Cc: Dmitry Torokhov, Alexander Shiyan

Replace existing resource handling in the driver with managed
device resource, this ensures more consistent error values and
simplifies error paths.
kzalloc -> devm_kzalloc
gpio_request_one -> devm_gpio_request_one
input_allocate_device -> devm_input_allocate_device

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/input/keyboard/gpio_keys.c | 96 +++++++++++---------------------------
 1 file changed, 28 insertions(+), 68 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 2db1324..50c2746 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -433,7 +433,7 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 	struct device *dev = &pdev->dev;
 	irq_handler_t isr;
 	unsigned long irqflags;
-	int irq, error;
+	int error;
 
 	bdata->input = input;
 	bdata->button = button;
@@ -441,7 +441,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 
 	if (gpio_is_valid(button->gpio)) {
 
-		error = gpio_request_one(button->gpio, GPIOF_IN, desc);
+		error = devm_gpio_request_one(&pdev->dev, button->gpio,
+					      GPIOF_IN, desc);
 		if (error < 0) {
 			dev_err(dev, "Failed to request GPIO %d, error %d\n",
 				button->gpio, error);
@@ -457,15 +458,13 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 						button->debounce_interval;
 		}
 
-		irq = gpio_to_irq(button->gpio);
-		if (irq < 0) {
-			error = irq;
+		bdata->irq = gpio_to_irq(button->gpio);
+		if (bdata->irq < 0) {
 			dev_err(dev,
 				"Unable to get irq number for GPIO %d, error %d\n",
-				button->gpio, error);
-			goto fail;
+				button->gpio, bdata->irq);
+			return bdata->irq;
 		}
-		bdata->irq = irq;
 
 		INIT_WORK(&bdata->work, gpio_keys_gpio_work_func);
 		setup_timer(&bdata->timer,
@@ -507,16 +506,10 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 	if (error < 0) {
 		dev_err(dev, "Unable to claim irq %d; error %d\n",
 			bdata->irq, error);
-		goto fail;
+		return error;
 	}
 
 	return 0;
-
-fail:
-	if (gpio_is_valid(button->gpio))
-		gpio_free(button->gpio);
-
-	return error;
 }
 
 static void gpio_keys_report_state(struct gpio_keys_drvdata *ddata)
@@ -573,28 +566,20 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 	struct device_node *node, *pp;
 	struct gpio_keys_platform_data *pdata;
 	struct gpio_keys_button *button;
-	int error;
-	int nbuttons;
-	int i;
+	int i, nbuttons;
 
 	node = dev->of_node;
-	if (!node) {
-		error = -ENODEV;
-		goto err_out;
-	}
+	if (!node)
+		return ERR_PTR(-ENODEV);
 
 	nbuttons = of_get_child_count(node);
-	if (nbuttons == 0) {
-		error = -ENODEV;
-		goto err_out;
-	}
+	if (nbuttons == 0)
+		return ERR_PTR(-ENODEV);
 
-	pdata = kzalloc(sizeof(*pdata) + nbuttons * (sizeof *button),
-			GFP_KERNEL);
-	if (!pdata) {
-		error = -ENOMEM;
-		goto err_out;
-	}
+	pdata = devm_kzalloc(dev, sizeof(*pdata) + nbuttons * sizeof(*button),
+			     GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
 
 	pdata->buttons = (struct gpio_keys_button *)(pdata + 1);
 	pdata->nbuttons = nbuttons;
@@ -614,12 +599,11 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 
 		gpio = of_get_gpio_flags(pp, 0, &flags);
 		if (gpio < 0) {
-			error = gpio;
-			if (error != -EPROBE_DEFER)
+			if (gpio != -EPROBE_DEFER)
 				dev_err(dev,
 					"Failed to get gpio flags, error: %d\n",
-					error);
-			goto err_free_pdata;
+					gpio);
+			return ERR_PTR(gpio);
 		}
 
 		button = &pdata->buttons[i++];
@@ -630,8 +614,7 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 		if (of_property_read_u32(pp, "linux,code", &button->code)) {
 			dev_err(dev, "Button without keycode: 0x%x\n",
 				button->gpio);
-			error = -EINVAL;
-			goto err_free_pdata;
+			return ERR_PTR(-EINVAL);
 		}
 
 		button->desc = of_get_property(pp, "label", NULL);
@@ -646,17 +629,10 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 			button->debounce_interval = 5;
 	}
 
-	if (pdata->nbuttons == 0) {
-		error = -EINVAL;
-		goto err_free_pdata;
-	}
+	if (!pdata->nbuttons)
+		return ERR_PTR(-EINVAL);
 
 	return pdata;
-
-err_free_pdata:
-	kfree(pdata);
-err_out:
-	return ERR_PTR(error);
 }
 
 static struct of_device_id gpio_keys_of_match[] = {
@@ -681,8 +657,6 @@ static void gpio_remove_key(struct gpio_button_data *bdata)
 	if (bdata->timer_debounce)
 		del_timer_sync(&bdata->timer);
 	cancel_work_sync(&bdata->work);
-	if (gpio_is_valid(bdata->button->gpio))
-		gpio_free(bdata->button->gpio);
 }
 
 static int gpio_keys_probe(struct platform_device *pdev)
@@ -700,14 +674,13 @@ static int gpio_keys_probe(struct platform_device *pdev)
 			return PTR_ERR(pdata);
 	}
 
-	ddata = kzalloc(sizeof(struct gpio_keys_drvdata) +
-			pdata->nbuttons * sizeof(struct gpio_button_data),
-			GFP_KERNEL);
-	input = input_allocate_device();
+	ddata = devm_kzalloc(&pdev->dev, sizeof(struct gpio_keys_drvdata) +
+			     pdata->nbuttons * sizeof(struct gpio_button_data),
+			     GFP_KERNEL);
+	input = devm_input_allocate_device(&pdev->dev);
 	if (!ddata || !input) {
 		dev_err(dev, "failed to allocate state\n");
-		error = -ENOMEM;
-		goto fail1;
+		return -ENOMEM;
 	}
 
 	ddata->pdata = pdata;
@@ -768,13 +741,6 @@ static int gpio_keys_probe(struct platform_device *pdev)
 	while (--i >= 0)
 		gpio_remove_key(&ddata->data[i]);
 
- fail1:
-	input_free_device(input);
-	kfree(ddata);
-	/* If we have no platform data, we allocated pdata dynamically. */
-	if (!dev_get_platdata(&pdev->dev))
-		kfree(pdata);
-
 	return error;
 }
 
@@ -793,12 +759,6 @@ static int gpio_keys_remove(struct platform_device *pdev)
 
 	input_unregister_device(input);
 
-	/* If we have no platform data, we allocated pdata dynamically. */
-	if (!dev_get_platdata(&pdev->dev))
-		kfree(ddata->pdata);
-
-	kfree(ddata);
-
 	return 0;
 }
 
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 2/4] input: mc13783: Use module_platform_driver_probe()
From: Alexander Shiyan @ 2014-02-10  4:22 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Shawn Guo, Sascha Hauer, Samuel Ortiz, Lee Jones,
	Alexander Shiyan

Driver should be probed from MC13XXX MFD core only, so change
driver declaration to use module_platform_driver_probe().

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/input/misc/mc13783-pwrbutton.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
index 60be67a..a1e45235 100644
--- a/drivers/input/misc/mc13783-pwrbutton.c
+++ b/drivers/input/misc/mc13783-pwrbutton.c
@@ -88,7 +88,7 @@ static irqreturn_t button_irq(int irq, void *_priv)
 	return IRQ_HANDLED;
 }
 
-static int mc13783_pwrbutton_probe(struct platform_device *pdev)
+static int __init mc13xxx_pwrbutton_probe(struct platform_device *pdev)
 {
 	const struct mc13xxx_buttons_platform_data *pdata;
 	struct mc13xxx *mc13783 = dev_get_drvdata(pdev->dev.parent);
@@ -229,7 +229,7 @@ free_input_dev:
 	return err;
 }
 
-static int mc13783_pwrbutton_remove(struct platform_device *pdev)
+static int mc13xxx_pwrbutton_remove(struct platform_device *pdev)
 {
 	struct mc13783_pwrb *priv = platform_get_drvdata(pdev);
 	const struct mc13xxx_buttons_platform_data *pdata;
@@ -253,18 +253,15 @@ static int mc13783_pwrbutton_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver mc13783_pwrbutton_driver = {
-	.probe		= mc13783_pwrbutton_probe,
-	.remove		= mc13783_pwrbutton_remove,
+static struct platform_driver mc13xxx_pwrbutton_driver = {
 	.driver		= {
 		.name	= "mc13783-pwrbutton",
 		.owner	= THIS_MODULE,
 	},
+	.remove		= mc13xxx_pwrbutton_remove,
 };
+module_platform_driver_probe(mc13xxx_pwrbutton_driver, mc13xxx_pwrbutton_probe);
 
-module_platform_driver(mc13783_pwrbutton_driver);
-
-MODULE_ALIAS("platform:mc13783-pwrbutton");
 MODULE_DESCRIPTION("MC13783 Power Button");
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Philippe Retornaz");
-- 
1.8.3.2


^ permalink raw reply related


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