Linux Input/HID development
 help / color / mirror / Atom feed
* 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

* [PATCH 3/4] input: mc13783: Prepare driver to support MC13892
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

This patch rewrite driver code to be ready to add support for
MC13892 buttons and probe from devicetree.

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

diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
index a1e45235..f9e2bfb 100644
--- a/drivers/input/misc/mc13783-pwrbutton.c
+++ b/drivers/input/misc/mc13783-pwrbutton.c
@@ -23,245 +23,174 @@
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/input.h>
-#include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/mc13783.h>
-#include <linux/sched.h>
-#include <linux/slab.h>
-
-struct mc13783_pwrb {
-	struct input_dev *pwr;
-	struct mc13xxx *mc13783;
-#define MC13783_PWRB_B1_POL_INVERT	(1 << 0)
-#define MC13783_PWRB_B2_POL_INVERT	(1 << 1)
-#define MC13783_PWRB_B3_POL_INVERT	(1 << 2)
-	int flags;
-	unsigned short keymap[MAX13XXX_NUM_BUTTONS];
+
+struct mc13xxx_button_def {
+	unsigned int	irq;
+	unsigned int	sense_bit;
+};
+
+struct mc13xxx_pwrb_devtype {
+	struct mc13xxx_button_def	btn_def[MAX13XXX_NUM_BUTTONS];
 };
 
-#define MC13783_REG_INTERRUPT_SENSE_1		5
-#define MC13783_IRQSENSE1_ONOFD1S		(1 << 3)
-#define MC13783_IRQSENSE1_ONOFD2S		(1 << 4)
-#define MC13783_IRQSENSE1_ONOFD3S		(1 << 5)
+struct mc13xxx_pwrb {
+	struct mc13xxx_pwrb_devtype	*devtype;
+	unsigned int			enabled;
+	unsigned int			inverted;
+	u16				btn_code[MAX13XXX_NUM_BUTTONS];
+	struct input_dev		*input;
+	struct mc13xxx			*mc13xxx;
+};
 
-#define MC13783_REG_POWER_CONTROL_2		15
-#define MC13783_POWER_CONTROL_2_ON1BDBNC	4
-#define MC13783_POWER_CONTROL_2_ON2BDBNC	6
-#define MC13783_POWER_CONTROL_2_ON3BDBNC	8
-#define MC13783_POWER_CONTROL_2_ON1BRSTEN	(1 << 1)
-#define MC13783_POWER_CONTROL_2_ON2BRSTEN	(1 << 2)
-#define MC13783_POWER_CONTROL_2_ON3BRSTEN	(1 << 3)
+#define MC13XXX_REG_INTERRUPT_SENSE_1	5
+#define MC13XXX_REG_POWER_CONTROL_2	15
 
-static irqreturn_t button_irq(int irq, void *_priv)
+static irqreturn_t mc13xxx_pwrbutton_irq(int irq, void *data)
 {
-	struct mc13783_pwrb *priv = _priv;
-	int val;
-
-	mc13xxx_irq_ack(priv->mc13783, irq);
-	mc13xxx_reg_read(priv->mc13783, MC13783_REG_INTERRUPT_SENSE_1, &val);
-
-	switch (irq) {
-	case MC13783_IRQ_ONOFD1:
-		val = val & MC13783_IRQSENSE1_ONOFD1S ? 1 : 0;
-		if (priv->flags & MC13783_PWRB_B1_POL_INVERT)
-			val ^= 1;
-		input_report_key(priv->pwr, priv->keymap[0], val);
-		break;
-
-	case MC13783_IRQ_ONOFD2:
-		val = val & MC13783_IRQSENSE1_ONOFD2S ? 1 : 0;
-		if (priv->flags & MC13783_PWRB_B2_POL_INVERT)
-			val ^= 1;
-		input_report_key(priv->pwr, priv->keymap[1], val);
-		break;
-
-	case MC13783_IRQ_ONOFD3:
-		val = val & MC13783_IRQSENSE1_ONOFD3S ? 1 : 0;
-		if (priv->flags & MC13783_PWRB_B3_POL_INVERT)
-			val ^= 1;
-		input_report_key(priv->pwr, priv->keymap[2], val);
-		break;
-	}
-
-	input_sync(priv->pwr);
+	struct mc13xxx_pwrb *priv = data;
+	unsigned int i, val;
+
+	mc13xxx_irq_ack(priv->mc13xxx, irq);
+	mc13xxx_reg_read(priv->mc13xxx, MC13XXX_REG_INTERRUPT_SENSE_1, &val);
+
+	for (i = 0; i < MAX13XXX_NUM_BUTTONS; i++)
+		if (irq == priv->devtype->btn_def[i].irq) {
+			val = !!(val & priv->devtype->btn_def[i].sense_bit);
+			if (priv->inverted & BIT(i))
+				val = !val;
+			input_report_key(priv->input, priv->btn_code[i], val);
+			input_sync(priv->input);
+			break;
+		}
 
 	return IRQ_HANDLED;
 }
 
 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);
-	struct input_dev *pwr;
-	struct mc13783_pwrb *priv;
-	int err = 0;
-	int reg = 0;
-
-	pdata = dev_get_platdata(&pdev->dev);
+	struct mc13xxx_buttons_platform_data *pdata =
+		dev_get_platdata(&pdev->dev);
+	struct mc13xxx *mc13xxx = dev_get_drvdata(pdev->dev.parent);
+	struct mc13xxx_pwrb_devtype *devtype =
+		(struct mc13xxx_pwrb_devtype *)pdev->id_entry->driver_data;
+	struct mc13xxx_pwrb *priv;
+	int i, reg = 0;
+
 	if (!pdata) {
 		dev_err(&pdev->dev, "missing platform data\n");
 		return -ENODEV;
 	}
 
-	pwr = input_allocate_device();
-	if (!pwr) {
-		dev_dbg(&pdev->dev, "Can't allocate power button\n");
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
 		return -ENOMEM;
-	}
-
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv) {
-		err = -ENOMEM;
-		dev_dbg(&pdev->dev, "Can't allocate power button\n");
-		goto free_input_dev;
-	}
-
-	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->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->buttons[0].flags & MC13XXX_BUTTON_POL_INVERT)
-			priv->flags |= MC13783_PWRB_B1_POL_INVERT;
-
-		if (pdata->buttons[0].flags & MC13XXX_BUTTON_RESET_EN)
-			reg |= MC13783_POWER_CONTROL_2_ON1BRSTEN;
-
-		err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD1,
-					  button_irq, "b1on", priv);
-		if (err) {
-			dev_dbg(&pdev->dev, "Can't request irq\n");
-			goto free_priv;
-		}
-	}
-
-	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->buttons[1].flags & MC13XXX_BUTTON_POL_INVERT)
-			priv->flags |= MC13783_PWRB_B2_POL_INVERT;
+	priv->input = devm_input_allocate_device(&pdev->dev);
+	if (!priv->input)
+		return -ENOMEM;
 
-		if (pdata->buttons[1].flags & MC13XXX_BUTTON_RESET_EN)
-			reg |= MC13783_POWER_CONTROL_2_ON2BRSTEN;
+	priv->mc13xxx = mc13xxx;
+	priv->devtype = devtype;
+	platform_set_drvdata(pdev, priv);
 
-		err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD2,
-					  button_irq, "b2on", priv);
-		if (err) {
-			dev_dbg(&pdev->dev, "Can't request irq\n");
-			goto free_irq_b1;
-		}
+	for (i = 0; i < MAX13XXX_NUM_BUTTONS; i++) {
+		u16 code, invert, reset, debounce;
+
+		if (!(pdata->buttons[i].flags & MC13XXX_BUTTON_ENABLE))
+			continue;
+		code = pdata->buttons[i].keycode;
+		invert = !!(pdata->buttons[i].flags &
+			    MC13XXX_BUTTON_POL_INVERT);
+		reset = !!(pdata->buttons[i].flags &
+			   MC13XXX_BUTTON_RESET_EN);
+		debounce = pdata->buttons[i].flags;
+
+		priv->btn_code[i] = code;
+		if (code != KEY_RESERVED)
+			__set_bit(code, priv->input->keybit);
+
+		priv->enabled |= BIT(i);
+		priv->inverted |= invert << i;
+		reg |= reset << (i + 1);
+		reg |= (debounce & 0x03) << (4 + i * 2);
 	}
 
-	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);
+	mc13xxx_lock(mc13xxx);
 
-		if (pdata->buttons[2].flags & MC13XXX_BUTTON_POL_INVERT)
-			priv->flags |= MC13783_PWRB_B3_POL_INVERT;
+	mc13xxx_reg_rmw(mc13xxx, MC13XXX_REG_POWER_CONTROL_2, 0x3fe, reg);
 
-		if (pdata->buttons[2].flags & MC13XXX_BUTTON_RESET_EN)
-			reg |= MC13783_POWER_CONTROL_2_ON3BRSTEN;
+	for (i = 0; i < MAX13XXX_NUM_BUTTONS; i++)
+		if (priv->enabled & BIT(i)) {
+			int ret;
 
-		err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD3,
-					  button_irq, "b3on", priv);
-		if (err) {
-			dev_dbg(&pdev->dev, "Can't request irq: %d\n", err);
-			goto free_irq_b2;
+			ret = mc13xxx_irq_request(mc13xxx,
+						  devtype->btn_def[i].irq,
+						  mc13xxx_pwrbutton_irq, NULL,
+						  priv);
+			if (ret) {
+				dev_err(&pdev->dev, "Can't request IRQ%i: %i\n",
+					devtype->btn_def[i].irq, ret);
+				priv->enabled &= ~BIT(i);
+			}
 		}
-	}
-
-	mc13xxx_reg_rmw(mc13783, MC13783_REG_POWER_CONTROL_2, 0x3FE, reg);
-
-	mc13xxx_unlock(mc13783);
-
-	pwr->name = "mc13783_pwrbutton";
-	pwr->phys = "mc13783_pwrbutton/input0";
-	pwr->dev.parent = &pdev->dev;
-
-	pwr->keycode = priv->keymap;
-	pwr->keycodemax = ARRAY_SIZE(priv->keymap);
-	pwr->keycodesize = sizeof(priv->keymap[0]);
-	__set_bit(EV_KEY, pwr->evbit);
-
-	err = input_register_device(pwr);
-	if (err) {
-		dev_dbg(&pdev->dev, "Can't register power button: %d\n", err);
-		goto free_irq;
-	}
-
-	platform_set_drvdata(pdev, priv);
-
-	return 0;
 
-free_irq:
-	mc13xxx_lock(mc13783);
+	mc13xxx_unlock(mc13xxx);
 
-	if (pdata->buttons[2].flags & MC13XXX_BUTTON_ENABLE)
-		mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD3, priv);
+	priv->input->name		= pdev->name;
+	priv->input->dev.parent		= &pdev->dev;
+	priv->input->id.bustype		= BUS_HOST;
+	priv->input->id.vendor		= 0x0001;
+	priv->input->id.product		= 0x0001;
+	priv->input->id.version		= 0x0100;
+	priv->input->keycode		= priv->btn_code;
+	priv->input->keycodemax		= ARRAY_SIZE(priv->btn_code);
+	priv->input->keycodesize	= sizeof(priv->btn_code[0]);
+	__set_bit(EV_KEY, priv->input->evbit);
 
-free_irq_b2:
-	if (pdata->buttons[1].flags & MC13XXX_BUTTON_ENABLE)
-		mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD2, priv);
+	input_set_drvdata(priv->input, priv);
 
-free_irq_b1:
-	if (pdata->buttons[0].flags & MC13XXX_BUTTON_ENABLE)
-		mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD1, priv);
-
-free_priv:
-	mc13xxx_unlock(mc13783);
-	kfree(priv);
-
-free_input_dev:
-	input_free_device(pwr);
-
-	return err;
+	return input_register_device(priv->input);
 }
 
 static int mc13xxx_pwrbutton_remove(struct platform_device *pdev)
 {
-	struct mc13783_pwrb *priv = platform_get_drvdata(pdev);
-	const struct mc13xxx_buttons_platform_data *pdata;
-
-	pdata = dev_get_platdata(&pdev->dev);
-
-	mc13xxx_lock(priv->mc13783);
-
-	if (pdata->buttons[2].flags & MC13XXX_BUTTON_ENABLE)
-		mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD3, priv);
-	if (pdata->buttons[1].flags & MC13XXX_BUTTON_ENABLE)
-		mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD2, priv);
-	if (pdata->buttons[0].flags & MC13XXX_BUTTON_ENABLE)
-		mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD1, priv);
+	struct mc13xxx_pwrb *priv = platform_get_drvdata(pdev);
+	int i;
 
-	mc13xxx_unlock(priv->mc13783);
-
-	input_unregister_device(priv->pwr);
-	kfree(priv);
+	mc13xxx_lock(priv->mc13xxx);
+	for (i = 0; i < MAX13XXX_NUM_BUTTONS; i++)
+		if (priv->enabled & BIT(i))
+			mc13xxx_irq_free(priv->mc13xxx,
+					 priv->devtype->btn_def[i].irq, priv);
+	mc13xxx_unlock(priv->mc13xxx);
 
 	return 0;
 }
 
+static const struct mc13xxx_pwrb_devtype mc13783_pwrb_devtype = {
+	.btn_def[0] = { MC13783_IRQ_ONOFD1, BIT(3), },
+	.btn_def[1] = { MC13783_IRQ_ONOFD2, BIT(4), },
+	.btn_def[2] = { MC13783_IRQ_ONOFD3, BIT(5), },
+};
+
+static const struct platform_device_id mc13xxx_pwrbutton_id_table[] = {
+	{ "mc13783-pwrbutton", (kernel_ulong_t)&mc13783_pwrb_devtype },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, mc13xxx_pwrbutton_id_table);
+
 static struct platform_driver mc13xxx_pwrbutton_driver = {
 	.driver		= {
-		.name	= "mc13783-pwrbutton",
+		.name	= "mc13xxx-pwrbutton",
 		.owner	= THIS_MODULE,
 	},
+	.id_table	= mc13xxx_pwrbutton_id_table,
 	.remove		= mc13xxx_pwrbutton_remove,
 };
 module_platform_driver_probe(mc13xxx_pwrbutton_driver, mc13xxx_pwrbutton_probe);
 
-MODULE_DESCRIPTION("MC13783 Power Button");
+MODULE_DESCRIPTION("MC13XXX Power Button");
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Philippe Retornaz");
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 4/4] input: mc13783: Add MC13892 support
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

This patch adds support for MC13892 PMIC to mc13783-pwrbutton driver.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/input/misc/Kconfig             | 4 ++--
 drivers/input/misc/mc13783-pwrbutton.c | 8 ++++++++
 include/linux/mfd/mc13892.h            | 4 ++++
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 7904ab0..3d91660 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -167,10 +167,10 @@ config INPUT_MAX8997_HAPTIC
 	  module will be called max8997-haptic.
 
 config INPUT_MC13783_PWRBUTTON
-	tristate "MC13783 ON buttons"
+	tristate "MC13783/MC13892 ON buttons"
 	depends on MFD_MC13XXX
 	help
-	  Support the ON buttons of MC13783 PMIC as an input device
+	  Support the ON buttons of MC13783/MC13892 PMIC as an input device
 	  reporting power button status.
 
 	  To compile this driver as a module, choose M here: the module
diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
index f9e2bfb..03308b0 100644
--- a/drivers/input/misc/mc13783-pwrbutton.c
+++ b/drivers/input/misc/mc13783-pwrbutton.c
@@ -25,6 +25,7 @@
 #include <linux/input.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/mc13783.h>
+#include <linux/mfd/mc13892.h>
 
 struct mc13xxx_button_def {
 	unsigned int	irq;
@@ -175,8 +176,15 @@ static const struct mc13xxx_pwrb_devtype mc13783_pwrb_devtype = {
 	.btn_def[2] = { MC13783_IRQ_ONOFD3, BIT(5), },
 };
 
+static const struct mc13xxx_pwrb_devtype mc13892_pwrb_devtype = {
+	.btn_def[0] = { MC13892_IRQ_ONOFD1, BIT(3), },
+	.btn_def[1] = { MC13892_IRQ_ONOFD2, BIT(4), },
+	.btn_def[2] = { MC13892_IRQ_ONOFD3, BIT(2), },
+};
+
 static const struct platform_device_id mc13xxx_pwrbutton_id_table[] = {
 	{ "mc13783-pwrbutton", (kernel_ulong_t)&mc13783_pwrb_devtype },
+	{ "mc13892-pwrbutton", (kernel_ulong_t)&mc13892_pwrb_devtype },
 	{ }
 };
 MODULE_DEVICE_TABLE(platform, mc13xxx_pwrbutton_id_table);
diff --git a/include/linux/mfd/mc13892.h b/include/linux/mfd/mc13892.h
index a00f2be..bdc3baf 100644
--- a/include/linux/mfd/mc13892.h
+++ b/include/linux/mfd/mc13892.h
@@ -36,4 +36,8 @@
 #define MC13892_PWGT2SPI	22
 #define MC13892_VCOINCELL	23
 
+#define MC13892_IRQ_ONOFD3	26
+#define MC13892_IRQ_ONOFD1	27
+#define MC13892_IRQ_ONOFD2	28
+
 #endif
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH 2/4] input: mc13783: Use module_platform_driver_probe()
From: Uwe Kleine-König @ 2014-02-10  7:18 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: linux-input, Dmitry Torokhov, Shawn Guo, Sascha Hauer,
	Samuel Ortiz, Lee Jones
In-Reply-To: <1392006138-10341-1-git-send-email-shc_work@mail.ru>

On Mon, Feb 10, 2014 at 08:22:18AM +0400, Alexander Shiyan wrote:
> Driver should be probed from MC13XXX MFD core only, so change
> driver declaration to use module_platform_driver_probe().
That reasoning is broken I think. At least it doesn't mean that a
button-device is only created at boot or module load time. I didn't test
it, but I'd expect that you can unbind/bind the mc13xxx driver via sysfs
and that makes the button support disappear.

(That doesn't necessarily means your change is wrong, just the reason
you provided to justify it is invalid.)

Best regards
Uwe

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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
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

* [PATCH] input: sirfsoc-onkey - report onkey untouch event by detecting pin status
From: Barry Song @ 2014-02-10 10:07 UTC (permalink / raw)
  To: dmitry.torokhov, linux-input
  Cc: linux-arm-kernel, workgroup.linux, Xianglong Du, Rongjun Ying,
	Barry Song

From: Xianglong Du <Xianglong.Du@csr.com>

this patch adds a delayed_work to detect the untouch of onkey since HW will
not generate interrupt for it.

at the same time, we move the KEY event to POWER instead of SUSPEND, which
will be suitable for both Android and Linux. Userspace PowerManager Daemon
will decide to suspend or shutdown based on how long we have touched onkey

Signed-off-by: Xianglong Du <Xianglong.Du@csr.com>
Signed-off-by: Rongjun Ying <Rongjun.Ying@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/input/misc/sirfsoc-onkey.c |   48 ++++++++++++++++++++++++++++-------
 1 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/drivers/input/misc/sirfsoc-onkey.c b/drivers/input/misc/sirfsoc-onkey.c
index e8897c3..9cd810b 100644
--- a/drivers/input/misc/sirfsoc-onkey.c
+++ b/drivers/input/misc/sirfsoc-onkey.c
@@ -13,16 +13,44 @@
 #include <linux/input.h>
 #include <linux/rtc/sirfsoc_rtciobrg.h>
 #include <linux/of.h>
+#include <linux/workqueue.h>
 
 struct sirfsoc_pwrc_drvdata {
 	u32			pwrc_base;
 	struct input_dev	*input;
+	struct delayed_work	work;
 };
 
 #define PWRC_ON_KEY_BIT			(1 << 0)
 
 #define PWRC_INT_STATUS			0xc
 #define PWRC_INT_MASK			0x10
+#define PWRC_PIN_STATUS			0x14
+#define PWRC_KEY_DETECT_UP_TIME		20	/* ms*/
+
+static inline int sirfsoc_pwrc_is_on_key_down(
+		struct sirfsoc_pwrc_drvdata *pwrcdrv)
+{
+	int state = sirfsoc_rtc_iobrg_readl(
+				pwrcdrv->pwrc_base + PWRC_PIN_STATUS)
+				& PWRC_ON_KEY_BIT;
+	return !state; /* ON_KEY is active low */
+}
+
+static void sirfsoc_pwrc_report_event(struct work_struct *work)
+{
+	struct sirfsoc_pwrc_drvdata *pwrcdrv =
+				container_of((struct delayed_work *)work,
+				struct sirfsoc_pwrc_drvdata, work);
+
+	if (!sirfsoc_pwrc_is_on_key_down(pwrcdrv)) {
+		input_event(pwrcdrv->input, EV_PWR, KEY_POWER, 0);
+		input_sync(pwrcdrv->input);
+	} else {
+		schedule_delayed_work(&pwrcdrv->work,
+			msecs_to_jiffies(PWRC_KEY_DETECT_UP_TIME));
+	}
+}
 
 static irqreturn_t sirfsoc_pwrc_isr(int irq, void *dev_id)
 {
@@ -34,17 +62,11 @@ static irqreturn_t sirfsoc_pwrc_isr(int irq, void *dev_id)
 	sirfsoc_rtc_iobrg_writel(int_status & ~PWRC_ON_KEY_BIT,
 				 pwrcdrv->pwrc_base + PWRC_INT_STATUS);
 
-	/*
-	 * For a typical Linux system, we report KEY_SUSPEND to trigger apm-power.c
-	 * to queue a SUSPEND APM event
-	 */
-	input_event(pwrcdrv->input, EV_PWR, KEY_SUSPEND, 1);
-	input_sync(pwrcdrv->input);
 
-	/*
-	 * Todo: report KEY_POWER event for Android platforms, Android PowerManager
-	 * will handle the suspend and powerdown/hibernation
-	 */
+	input_event(pwrcdrv->input, EV_PWR, KEY_POWER, 1);
+	input_sync(pwrcdrv->input);
+	schedule_delayed_work(&pwrcdrv->work,
+		msecs_to_jiffies(PWRC_KEY_DETECT_UP_TIME));
 
 	return IRQ_HANDLED;
 }
@@ -88,6 +110,8 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
 	pwrcdrv->input->phys = "pwrc/input0";
 	pwrcdrv->input->evbit[0] = BIT_MASK(EV_PWR);
 
+	INIT_DELAYED_WORK(&pwrcdrv->work, sirfsoc_pwrc_report_event);
+
 	irq = platform_get_irq(pdev, 0);
 	error = devm_request_irq(&pdev->dev, irq,
 				 sirfsoc_pwrc_isr, IRQF_SHARED,
@@ -119,8 +143,12 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
 
 static int sirfsoc_pwrc_remove(struct platform_device *pdev)
 {
+	struct sirfsoc_pwrc_drvdata *pwrcdrv = dev_get_drvdata(&pdev->dev);
+
 	device_init_wakeup(&pdev->dev, 0);
 
+	cancel_delayed_work_sync(&pwrcdrv->work);
+
 	return 0;
 }
 
-- 
1.7.5.4


^ permalink raw reply related

* Re: [PATCH 4/4] input: mc13783: Add MC13892 support
From: Lee Jones @ 2014-02-10 11:23 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: linux-input, Dmitry Torokhov, Shawn Guo, Sascha Hauer,
	Samuel Ortiz
In-Reply-To: <1392006148-10417-1-git-send-email-shc_work@mail.ru>

> This patch adds support for MC13892 PMIC to mc13783-pwrbutton driver.
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  drivers/input/misc/Kconfig             | 4 ++--
>  drivers/input/misc/mc13783-pwrbutton.c | 8 ++++++++
>  include/linux/mfd/mc13892.h            | 4 ++++

For the MFD changes:
  Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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 1/4] input: mc13783: Make mc13xxx_buttons_platform_data more flexible
From: Lee Jones @ 2014-02-10 11:29 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: linux-input, Dmitry Torokhov, Shawn Guo, Sascha Hauer,
	Samuel Ortiz
In-Reply-To: <1392006129-10227-1-git-send-email-shc_work@mail.ru>

> 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 +++++++++--------

Are all the changes in these files entangled? If there is any way to
make them orthogonal, then all the better.

<snip>

> --- 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)
> +};
> +

Please take the opportunity to remove this slab list of #defines from
the centre of the struct definition. Just above will be fine.

>  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 {

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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


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