Linux Input/HID development
 help / color / mirror / Atom feed
* Re: EVIOCSFF macro inconsistency
From: Dmitry Torokhov @ 2014-09-08 20:24 UTC (permalink / raw)
  To: Elias Vanderstuyft; +Cc: open list:HID CORE LAYER
In-Reply-To: <CADbOyBT_UJJg3d4VYGi5Xry4L5egqc_T3_PGEL7th=4TO=NPcA@mail.gmail.com>

On Monday, September 08, 2014 09:03:11 PM Elias Vanderstuyft wrote:
> On Mon, Sep 8, 2014 at 8:31 PM, Dmitry Torokhov
> 
> <dmitry.torokhov@gmail.com> wrote:
> > Hi Elias,
> > 
> > On Mon, Sep 08, 2014 at 08:14:13PM +0200, Elias Vanderstuyft wrote:
> >> Hi everyone,
> >> 
> >> After inspecting the <linux/input.h> header file, I found that there
> >> is one single ioctl value macro that is inconsistent w.r.t. the other
> >> 
> >> macros that do an IOC_WRITE :
> >>     EVIOCSFF   _IOC(_IOC_WRITE, 'E', 0x80, sizeof(struct ff_effect))
> >> 
> >> Why not define it as follows? :
> >>     EVIOCSFF   _IOW('E', 0x80, struct ff_effect)
> >> 
> >> Apart from having a more readable definition, it also explicitly
> >> reveals type info ("struct ff_effect").
> > 
> > I think it is just historical.
> > 
> >> Is it worth to create a patch to fix it?
> > 
> > Sure, why not.
> 
> Cool, thanks!
> 
> So probably the same applies to the IOC_READ counter parts? :
>     EVIOCGBIT(ev,len)    _IOC(_IOC_READ, 'E', 0x20 + (ev), len)
>     EVIOCGKEY(len)    _IOC(_IOC_READ, 'E', 0x18, len)
>     EVIOCGLED(len)    _IOC(_IOC_READ, 'E', 0x19, len)
>     EVIOCGMTSLOTS(len)    _IOC(_IOC_READ, 'E', 0x0a, len)
>     EVIOCGNAME(len)    _IOC(_IOC_READ, 'E', 0x06, len)
>     EVIOCGPHYS(len)    _IOC(_IOC_READ, 'E', 0x07, len)
>     EVIOCGPROP(len)    _IOC(_IOC_READ, 'E', 0x09, len)
>     EVIOCGSND(len)    _IOC(_IOC_READ, 'E', 0x1a, len)
>     EVIOCGSW(len)    _IOC(_IOC_READ, 'E', 0x1b, len)
>     EVIOCGUNIQ(len)    _IOC(_IOC_READ, 'E', 0x08, len)
> to be converted to:
>     EVIOCGBIT(ev,len)    _IOR('E', 0x20 + (ev), len)
>     EVIOCGKEY(len)    _IOR('E', 0x18, len)
>     EVIOCGLED(len)    _IOR('E', 0x19, len)
>     EVIOCGMTSLOTS(len)    _IOR('E', 0x0a, len)
>     EVIOCGNAME(len)    _IOR('E', 0x06, len)
>     EVIOCGPHYS(len)    _IOR('E', 0x07, len)
>     EVIOCGPROP(len)    _IOR('E', 0x09, len)
>     EVIOCGSND(len)    _IOR('E', 0x1a, len)
>     EVIOCGSW(len)    _IOR('E', 0x1b, len)
>     EVIOCGUNIQ(len)    _IOR('E', 0x08, len)

No, because 'len' is not a type.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Need help adding support for X1 Carbon 2nd Gen's Keyboard
From: José Díez @ 2014-09-08 20:19 UTC (permalink / raw)
  To: linux-input

Hello linux-input,

This is my first post on this mailing list (or on any other kernel list, 
actually), so sorry if I break unwritten ettiquete. Please let me know.

So I recently got a X1 Carbon Thinkpad, and so far everything has been 
working flawlessly, but the Adaptive Keyboard generates some ACPI HKEY 
events with scancodes that are not currently recognised. They are 
reported to the kernel successfully so one could set up shortcuts by 
using something like `acpid`, but perhaps it would be better if it 
generated normal key presses (i.e XF86Something).

I've read through most of the thinkpad_acpi.c code and understand most 
of it, but so far I've been unable to report normal keyboard events from 
there (and I don't even know if it would be the correct thing to do.)

I must also say that these keys are somewhat odd. In one of the modes, 
there are icons for "snippet tool", "cloud", "man speaking", "hands in 
front of camera". I'm not sure what would be the best way to report 
these, so I seek your advice here.

At the very least, I can list which scancode is generated by each key.

^ permalink raw reply

* Re: EVIOCSFF macro inconsistency
From: Elias Vanderstuyft @ 2014-09-08 20:42 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: open list:HID CORE LAYER
In-Reply-To: <7779930.djVFXGcffV@dtor-glaptop>

On Mon, Sep 8, 2014 at 10:24 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Monday, September 08, 2014 09:03:11 PM Elias Vanderstuyft wrote:
>> On Mon, Sep 8, 2014 at 8:31 PM, Dmitry Torokhov
>>
>> <dmitry.torokhov@gmail.com> wrote:
>> > Hi Elias,
>> >
>> > On Mon, Sep 08, 2014 at 08:14:13PM +0200, Elias Vanderstuyft wrote:
>> >> Hi everyone,
>> >>
>> >> After inspecting the <linux/input.h> header file, I found that there
>> >> is one single ioctl value macro that is inconsistent w.r.t. the other
>> >>
>> >> macros that do an IOC_WRITE :
>> >>     EVIOCSFF   _IOC(_IOC_WRITE, 'E', 0x80, sizeof(struct ff_effect))
>> >>
>> >> Why not define it as follows? :
>> >>     EVIOCSFF   _IOW('E', 0x80, struct ff_effect)
>> >>
>> >> Apart from having a more readable definition, it also explicitly
>> >> reveals type info ("struct ff_effect").
>> >
>> > I think it is just historical.
>> >
>> >> Is it worth to create a patch to fix it?
>> >
>> > Sure, why not.
>>
>> Cool, thanks!
>>
>> So probably the same applies to the IOC_READ counter parts? :
>>     EVIOCGBIT(ev,len)    _IOC(_IOC_READ, 'E', 0x20 + (ev), len)
>>     EVIOCGKEY(len)    _IOC(_IOC_READ, 'E', 0x18, len)
>>     EVIOCGLED(len)    _IOC(_IOC_READ, 'E', 0x19, len)
>>     EVIOCGMTSLOTS(len)    _IOC(_IOC_READ, 'E', 0x0a, len)
>>     EVIOCGNAME(len)    _IOC(_IOC_READ, 'E', 0x06, len)
>>     EVIOCGPHYS(len)    _IOC(_IOC_READ, 'E', 0x07, len)
>>     EVIOCGPROP(len)    _IOC(_IOC_READ, 'E', 0x09, len)
>>     EVIOCGSND(len)    _IOC(_IOC_READ, 'E', 0x1a, len)
>>     EVIOCGSW(len)    _IOC(_IOC_READ, 'E', 0x1b, len)
>>     EVIOCGUNIQ(len)    _IOC(_IOC_READ, 'E', 0x08, len)
>> to be converted to:
>>     EVIOCGBIT(ev,len)    _IOR('E', 0x20 + (ev), len)
>>     EVIOCGKEY(len)    _IOR('E', 0x18, len)
>>     EVIOCGLED(len)    _IOR('E', 0x19, len)
>>     EVIOCGMTSLOTS(len)    _IOR('E', 0x0a, len)
>>     EVIOCGNAME(len)    _IOR('E', 0x06, len)
>>     EVIOCGPHYS(len)    _IOR('E', 0x07, len)
>>     EVIOCGPROP(len)    _IOR('E', 0x09, len)
>>     EVIOCGSND(len)    _IOR('E', 0x1a, len)
>>     EVIOCGSW(len)    _IOR('E', 0x1b, len)
>>     EVIOCGUNIQ(len)    _IOR('E', 0x08, len)
>
> No, because 'len' is not a type.

Indeed, just found out by myself, thank you for the confirmation.

Elias

^ permalink raw reply

* Re: [PATCH] xpad: sync device IDs with xboxdrv
From: Dmitry Torokhov @ 2014-09-08 21:21 UTC (permalink / raw)
  To: Benjamin Valentin
  Cc: linux-input, linux-kernel, Ted Mielczarek, Paul Gortmaker
In-Reply-To: <20140906144727.29b44f5a@rechenknecht2k7>

On Sat, Sep 06, 2014 at 02:47:27PM +0200, Benjamin Valentin wrote:
> The userspace xboxdrv driver knows some more device ids than the kernel.
> This patch adds the missing xbox gamepads from [1] to xpad.c
> 
> [1] https://github.com/Grumbel/xboxdrv/blob/master/src/xpad_device.cpp
> 
> Signed-off-by: Benjamin Valentin <benpicco@zedat.fu-berlin.de>


Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: serport: Add compat_ioctl routine to support 32bit inputtattach in 64bit systems
From: Dmitry Torokhov @ 2014-09-08 21:28 UTC (permalink / raw)
  To: John Sung; +Cc: linux-input, linux-kernel
In-Reply-To: <1409922277-4497-1-git-send-email-penmount.touch@gmail.com>

Hi John,

On Fri, Sep 05, 2014 at 09:04:37PM +0800, John Sung wrote:
> When running a 32-bit inputattach utility in a 64-bit system, there will be error code "inputattach: can't set device type". This is caused by the serport device driver not supporting compat_ioctl, so that SPIOCSTYPE ioctl fails.
> 
> Signed-off-by: John Sung <penmount.touch@gmail.com>
> ---
>  drivers/input/serio/serport.c |   24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
> index 0cb7ef5..4b68c29 100644
> --- a/drivers/input/serio/serport.c
> +++ b/drivers/input/serio/serport.c
> @@ -22,6 +22,8 @@
>  #include <linux/serio.h>
>  #include <linux/tty.h>
>  
> +#include <asm/compat.h>
> +
>  MODULE_AUTHOR("Vojtech Pavlik <vojtech@ucw.cz>");
>  MODULE_DESCRIPTION("Input device TTY line discipline");
>  MODULE_LICENSE("GPL");
> @@ -31,6 +33,8 @@ MODULE_ALIAS_LDISC(N_MOUSE);
>  #define SERPORT_ACTIVE	2
>  #define SERPORT_DEAD	3
>  
> +#define SPIOCSTYPE32 (_IOW('q', 0x01, compat_ulong_t))
> +
>  struct serport {
>  	struct tty_struct *tty;
>  	wait_queue_head_t wait;
> @@ -221,6 +225,25 @@ static int serport_ldisc_ioctl(struct tty_struct * tty, struct file * file, unsi
>  	return -EINVAL;
>  }
>  
> +static long serport_ldisc_compat_ioctl(struct tty_struct *tty, struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	struct serport *serport = (struct serport*) tty->disc_data;
> +	unsigned long type;
> +
> +	if (cmd == SPIOCSTYPE32) {
> +		if (get_user(type, (unsigned long __user *) arg))
> +			return -EFAULT;
> +
> +		serport->id.proto = type & 0x000000ff;
> +		serport->id.id	  = (type & 0x0000ff00) >> 8;
> +		serport->id.extra = (type & 0x00ff0000) >> 16;
> +
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  static void serport_ldisc_write_wakeup(struct tty_struct * tty)
>  {
>  	struct serport *serport = (struct serport *) tty->disc_data;
> @@ -243,6 +266,7 @@ static struct tty_ldisc_ops serport_ldisc = {
>  	.close =	serport_ldisc_close,
>  	.read =		serport_ldisc_read,
>  	.ioctl =	serport_ldisc_ioctl,
> +	.compat_ioctl =	serport_ldisc_compat_ioctl,

I think this, and serport_ldisc_compat_ioctl definition, should be
protected by #ifdef OCNFIG_COMPAT. Also, instea dof replicating the
ioctl definition and parsing, could we combine the code a bit?

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v3 2/4] Input: misc: Add haptic driver on max77693
From: Dmitry Torokhov @ 2014-09-08 21:37 UTC (permalink / raw)
  To: Jaewon Kim
  Cc: Samuel Ortiz, Lee Jones, linux-kernel, linux-input, Chanwoo Choi
In-Reply-To: <1409917702-17009-3-git-send-email-jaewon02.kim@samsung.com>

On Fri, Sep 05, 2014 at 08:48:20PM +0900, Jaewon Kim wrote:
> This patch add max77693-haptic device driver to support the haptic controller
> on MAX77693. The MAX77693 is a Multifunction device with PMIC, CHARGER, LED,
> MUIC, HAPTIC and the patch is haptic device driver in the MAX77693. This driver
> support external pwm and LRA(Linear Resonant Actuator) motor. User can control
> the haptic driver by using force feedback framework.
> 
> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/input/misc/Kconfig           |   12 ++
>  drivers/input/misc/Makefile          |    1 +
>  drivers/input/misc/max77693-haptic.c |  321 ++++++++++++++++++++++++++++++++++
>  include/linux/mfd/max77693-private.h |    9 +
>  4 files changed, 343 insertions(+)
>  create mode 100644 drivers/input/misc/max77693-haptic.c
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 2ff4425..c597c52 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -144,6 +144,18 @@ config INPUT_M68K_BEEP
>  	tristate "M68k Beeper support"
>  	depends on M68K
>  
> +config INPUT_MAX77693_HAPTIC
> +	tristate "MAXIM MAX77693 haptic controller support"
> +	depends on MFD_MAX77693 && PWM
> +	select INPUT_FF_MEMLESS
> +	help
> +	  This option enables device driver support for the haptic controller
> +	  on MAXIM MAX77693 chip. This driver supports ff-memless interface
> +	  from input framework.
> +
> +	  To compile this driver as module, choose M here: the
> +	  module will be called max77693-haptic.
> +
>  config INPUT_MAX8925_ONKEY
>  	tristate "MAX8925 ONKEY support"
>  	depends on MFD_MAX8925
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 4955ad3..b28570c 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -35,6 +35,7 @@ obj-$(CONFIG_INPUT_IXP4XX_BEEPER)	+= ixp4xx-beeper.o
>  obj-$(CONFIG_INPUT_KEYSPAN_REMOTE)	+= keyspan_remote.o
>  obj-$(CONFIG_INPUT_KXTJ9)		+= kxtj9.o
>  obj-$(CONFIG_INPUT_M68K_BEEP)		+= m68kspkr.o
> +obj-$(CONFIG_INPUT_MAX77693_HAPTIC)	+= max77693-haptic.o
>  obj-$(CONFIG_INPUT_MAX8925_ONKEY)	+= max8925_onkey.o
>  obj-$(CONFIG_INPUT_MAX8997_HAPTIC)	+= max8997_haptic.o
>  obj-$(CONFIG_INPUT_MC13783_PWRBUTTON)	+= mc13783-pwrbutton.o
> diff --git a/drivers/input/misc/max77693-haptic.c b/drivers/input/misc/max77693-haptic.c
> new file mode 100644
> index 0000000..d06026b
> --- /dev/null
> +++ b/drivers/input/misc/max77693-haptic.c
> @@ -0,0 +1,321 @@
> +/*
> + * max77693-haptic.c - MAXIM MAX77693 Haptic device driver
> + *
> + * Copyright (C) 2014 Samsung Electronics
> + * Jaewon Kim <jaewon02.kim@samsung.com>
> + *
> + * This program is not provided / owned by Maxim Integrated Products.
> + *
> + * 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/err.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/input.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/mfd/max77693.h>
> +#include <linux/mfd/max77693-private.h>
> +
> +#define MAX_MAGNITUDE_SHIFT	16
> +
> +enum max77693_haptic_motor_type {
> +	MAX77693_HAPTIC_ERM = 0,
> +	MAX77693_HAPTIC_LRA,
> +};
> +
> +enum max77693_haptic_pulse_mode {
> +	MAX77693_HAPTIC_EXTERNAL_MODE = 0,
> +	MAX77693_HAPTIC_INTERNAL_MODE,
> +};
> +
> +enum max77693_haptic_pwm_divisor {
> +	MAX77693_HAPTIC_PWM_DIVISOR_32 = 0,
> +	MAX77693_HAPTIC_PWM_DIVISOR_64,
> +	MAX77693_HAPTIC_PWM_DIVISOR_128,
> +	MAX77693_HAPTIC_PWM_DIVISOR_256,
> +};
> +
> +struct max77693_haptic {
> +	struct regmap *regmap_pmic;
> +	struct regmap *regmap_haptic;
> +	struct device *dev;
> +	struct input_dev *input_dev;
> +	struct pwm_device *pwm_dev;
> +	struct regulator *motor_reg;
> +
> +	bool enabled;
> +	unsigned int magnitude;
> +	unsigned int pwm_duty;
> +	enum max77693_haptic_motor_type type;
> +	enum max77693_haptic_pulse_mode mode;
> +	enum max77693_haptic_pwm_divisor pwm_divisor;
> +
> +	struct work_struct work;
> +};
> +
> +static int max77693_haptic_set_duty_cycle(struct max77693_haptic *haptic)
> +{
> +	int ret;
> +	int delta = (haptic->pwm_dev->period + haptic->pwm_duty)/2;

Spaces around '/'.

> +
> +	ret = pwm_config(haptic->pwm_dev, delta, haptic->pwm_dev->period);
> +	if (ret) {
> +		dev_err(haptic->dev, "cannot configuration pwm\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int max77693_haptic_configure(struct max77693_haptic *haptic,
> +						unsigned int enable)

bool?

> +{
> +	int ret;
> +	unsigned int value = 0;
> +

You do not need to initialize value and assign a brand new value to it
immediately after.

> +	value = ((haptic->type << MAX77693_CONFIG2_MODE) |
> +		(enable << MAX77693_CONFIG2_MEN) |
> +		(haptic->mode << MAX77693_CONFIG2_HTYP) |
> +		(haptic->pwm_divisor));
> +
> +	ret = regmap_write(haptic->regmap_haptic,
> +				MAX77693_HAPTIC_REG_CONFIG2, value);
> +	if (ret) {
> +		dev_err(haptic->dev, "cannot write haptic regmap\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int max77693_haptic_lowsys(struct max77693_haptic *haptic,
> +						unsigned int enable)

bool

> +{
> +	int ret;
> +
> +	ret = regmap_update_bits(haptic->regmap_pmic,
> +			MAX77693_PMIC_REG_LSCNFG,
> +			MAX77693_PMIC_LOW_SYS_MASK,
> +			enable << MAX77693_PMIC_LOW_SYS_SHIFT);
> +	if (ret) {
> +		dev_err(haptic->dev, "cannot update pmic regmap\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void max77693_haptic_enable(struct max77693_haptic *haptic)
> +{
> +	int ret;
> +
> +	if (haptic->enabled)
> +		return;
> +
> +	ret = pwm_enable(haptic->pwm_dev);
> +	if (ret) {
> +		dev_err(haptic->dev, "cannot enable haptic pwm device");
> +		return;
> +	}
> +
> +	ret = max77693_haptic_lowsys(haptic, 1);
> +	if (ret)
> +		goto err_enable_lowsys;
> +
> +	ret = max77693_haptic_configure(haptic, 1);
> +	if (ret)
> +		goto err_enable_config;
> +
> +	haptic->enabled = true;
> +
> +	return;
> +
> +err_enable_config:
> +	max77693_haptic_lowsys(haptic, 0);
> +err_enable_lowsys:
> +	pwm_disable(haptic->pwm_dev);
> +}
> +
> +static void max77693_haptic_disable(struct max77693_haptic *haptic)
> +{
> +	int ret;
> +
> +	if (!haptic->enabled)
> +		return;
> +
> +	ret = max77693_haptic_configure(haptic, 0);
> +	if (ret)
> +		return;
> +
> +	ret = max77693_haptic_lowsys(haptic, 0);
> +	if (ret)
> +		goto err_disable_lowsys;
> +
> +	pwm_disable(haptic->pwm_dev);
> +	haptic->enabled = false;
> +
> +	return;
> +
> +err_disable_lowsys:
> +	max77693_haptic_configure(haptic, 1);
> +}
> +
> +static void max77693_haptic_play_work(struct work_struct *work)
> +{
> +	struct max77693_haptic *haptic =
> +			container_of(work, struct max77693_haptic, work);
> +	int ret;
> +
> +	ret = max77693_haptic_set_duty_cycle(haptic);
> +	if (ret) {
> +		dev_err(haptic->dev, "cannot set duty cycle\n");
> +		return;
> +	}
> +
> +	if (haptic->magnitude)
> +		max77693_haptic_enable(haptic);
> +	else
> +		max77693_haptic_disable(haptic);
> +}
> +
> +static int max77693_haptic_play_effect(struct input_dev *dev, void *data,
> +				struct ff_effect *effect)
> +{
> +	struct max77693_haptic *haptic = input_get_drvdata(dev);
> +	uint64_t period_mag_multi;
> +
> +	haptic->magnitude = effect->u.rumble.strong_magnitude;
> +	if (!haptic->magnitude)
> +		haptic->magnitude = effect->u.rumble.weak_magnitude;
> +
> +	/*
> +	 * The magnitude comes from force-feedback interface.
> +	 * The formula convert magnitude to pwm_duty as following:
> +	 * - pwm_duty = (magnitude * pwm_period) / MAX_MAGNITUDE(0xFFFF)
> +	 */
> +	period_mag_multi = (int64_t)(haptic->pwm_dev->period *
> +						haptic->magnitude);
> +	haptic->pwm_duty = (unsigned int)(period_mag_multi >>
> +						MAX_MAGNITUDE_SHIFT);
> +
> +	schedule_work(&haptic->work);
> +
> +	return 0;
> +}
> +
> +static void max77693_haptic_close(struct input_dev *dev)
> +{
> +	struct max77693_haptic *haptic = input_get_drvdata(dev);
> +
> +	cancel_work_sync(&haptic->work);
> +	max77693_haptic_disable(haptic);
> +}
> +
> +static int max77693_haptic_probe(struct platform_device *pdev)
> +{
> +	struct max77693_dev *max77693 = dev_get_drvdata(pdev->dev.parent);
> +	struct max77693_haptic *haptic;
> +	int ret = 0;
> +
> +	haptic = devm_kzalloc(&pdev->dev, sizeof(*haptic), GFP_KERNEL);
> +	if (!haptic)
> +		return -ENOMEM;
> +
> +	haptic->regmap_pmic = max77693->regmap;
> +	haptic->regmap_haptic = max77693->regmap_haptic;
> +	haptic->dev = &pdev->dev;
> +	haptic->type = MAX77693_HAPTIC_LRA;
> +	haptic->mode = MAX77693_HAPTIC_EXTERNAL_MODE;
> +	haptic->pwm_divisor = MAX77693_HAPTIC_PWM_DIVISOR_128;
> +
> +	/* Get pwm and regulatot for haptic device */
> +	haptic->pwm_dev = devm_pwm_get(&pdev->dev, NULL);
> +	if (IS_ERR(haptic->pwm_dev)) {
> +		dev_err(&pdev->dev, "failed to get pwm device\n");
> +		return PTR_ERR(haptic->pwm_dev);
> +	}
> +
> +	haptic->motor_reg = devm_regulator_get(&pdev->dev, "haptic");
> +	if (IS_ERR(haptic->motor_reg)) {
> +		dev_err(&pdev->dev, "failed to get regulator\n");
> +		return PTR_ERR(haptic->motor_reg);
> +	}
> +
> +	ret = regulator_enable(haptic->motor_reg);
> +	if (ret) {
> +		dev_err(haptic->dev, "failed to enable regulator\n");
> +		return ret;
> +	}
> +
> +	/* Initialize input device for haptic device */
> +	haptic->input_dev = devm_input_allocate_device(&pdev->dev);
> +	if (!haptic->input_dev) {
> +		dev_err(&pdev->dev, "failed to allocate input device\n");

Hmm.. is it OK for the regulator to stay enabled? Actually, why don't
you put regulator enable/disable in open/close?

> +		return -ENOMEM;
> +	}
> +
> +	haptic->input_dev->name = "max77693-haptic";
> +	haptic->input_dev->id.version = 1;
> +	haptic->input_dev->dev.parent = &pdev->dev;
> +	haptic->input_dev->close = max77693_haptic_close;
> +	input_set_drvdata(haptic->input_dev, haptic);
> +	input_set_capability(haptic->input_dev, EV_FF, FF_RUMBLE);
> +
> +	ret = input_ff_create_memless(haptic->input_dev, NULL,
> +				max77693_haptic_play_effect);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to create force-feedback\n");
> +		return ret;
> +	}
> +
> +	ret = input_register_device(haptic->input_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to register input device\n");
> +		return ret;
> +	}
> +
> +	INIT_WORK(&haptic->work, max77693_haptic_play_work);

You need to initialize work before creating input device, otherwise
there is a chance play request would come before work is initialized.

> +
> +	platform_set_drvdata(pdev, haptic);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int max77693_haptic_suspend(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct max77693_haptic *haptic = platform_get_drvdata(pdev);
> +
> +	max77693_haptic_disable(haptic);
> +
> +	return 0;
> +}

I still think you need resume as well. If for some reason you suspended
vibrating you need to resume and continue vibrating. Sleep should be
transparent to users.

> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(max77693_haptic_pm_ops, max77693_haptic_suspend, NULL);
> +
> +static struct platform_driver max77693_haptic_driver = {
> +	.driver		= {
> +		.name	= "max77693-haptic",
> +		.owner	= THIS_MODULE,
> +		.pm	= &max77693_haptic_pm_ops,
> +	},
> +	.probe		= max77693_haptic_probe,
> +};
> +module_platform_driver(max77693_haptic_driver);
> +
> +MODULE_AUTHOR("Jaewon Kim <jaewon02.kim@samsung.com>");
> +MODULE_DESCRIPTION("MAXIM MAX77693 Haptic driver");
> +MODULE_ALIAS("platform:max77693-haptic");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/max77693-private.h b/include/linux/mfd/max77693-private.h
> index c466ff3..d0e578f 100644
> --- a/include/linux/mfd/max77693-private.h
> +++ b/include/linux/mfd/max77693-private.h
> @@ -251,6 +251,15 @@ enum max77693_haptic_reg {
>  	MAX77693_HAPTIC_REG_END,
>  };
>  
> +/* max77693-pmic LSCNFG configuraton register */
> +#define MAX77693_PMIC_LOW_SYS_MASK      0x80
> +#define MAX77693_PMIC_LOW_SYS_SHIFT     7
> +
> +/* max77693-haptic configuration register */
> +#define MAX77693_CONFIG2_MODE           7
> +#define MAX77693_CONFIG2_MEN            6
> +#define MAX77693_CONFIG2_HTYP           5
> +
>  enum max77693_irq_source {
>  	LED_INT = 0,
>  	TOPSYS_INT,
> -- 
> 1.7.9.5
> 

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] elantech: Fix detection of touchpad on ASUS s301l
From: Dmitry Torokhov @ 2014-09-08 21:40 UTC (permalink / raw)
  To: Hans de Goede; +Cc: linux-input, stable
In-Reply-To: <1409754242-10196-1-git-send-email-hdegoede@redhat.com>

On Wed, Sep 03, 2014 at 04:24:02PM +0200, Hans de Goede wrote:
> http://mariusmonton.com/?p=489
> 
> Cc: stable@vger.kernel.org
> Reported-and-tested-by: Màrius Monton <marius.monton@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Applied, thank you.

> ---
>  drivers/input/mouse/elantech.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index daaf82f..fad7367 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -1331,6 +1331,13 @@ static bool elantech_is_signature_valid(const unsigned char *param)
>  	if (param[1] == 0)
>  		return true;
>  
> +	/*
> +	 * Some models have a revision higher then 20. Meaning param[2] may
> +	 * be 10 or 20, skip the rates check for these.
> +	 */
> +	if (param[0] == 0x46 && (param[1] & 0xef) == 0x0f && param[2] < 40)
> +		return true;
> +
>  	for (i = 0; i < ARRAY_SIZE(rates); i++)
>  		if (param[2] == rates[i])
>  			return false;
> -- 
> 2.1.0
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 0/2] input: Add INPUT_PROP_POINTING_STICK property
From: Dmitry Torokhov @ 2014-09-08 21:45 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Peter Hutterer, Benjamin Tissoires, linux-input
In-Reply-To: <1409661804-10489-1-git-send-email-hdegoede@redhat.com>

On Tue, Sep 02, 2014 at 02:43:22PM +0200, Hans de Goede wrote:
> Hi,
> 
> It is useful for userspace to know that they're not dealing with a regular
> mouse but rather with a pointing stick (e.g. a trackpoint) so that userspace
> can e.g. automatically enable middle button scrollwheel emulation.
> 
> It is impossible to tell the difference from the evdev info without resorting
> to putting a list of device / driver names in userspace, this is undesirable.
> 
> These patches add a property which allows userspace to see if a device is a
> pointing stick, and set it on all the pointing stick drivers.
> 
> Note these patch apply on top of the current dtor/input.git for-linus
> branch. Specifically they depend upon a2418fc4a13 (Input: elantech - add
> support for trackpoint found on some v3 models).

Applied both, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: cap1106 - fix register definition
From: Dmitry Torokhov @ 2014-09-08 21:46 UTC (permalink / raw)
  To: Klaus Goger; +Cc: Daniel Mack, linux-input, linux-kernel
In-Reply-To: <53EB1EFA-3BFF-4818-A524-B3ED3CF62D56@theobroma-systems.com>

On Tue, Sep 02, 2014 at 01:01:11PM +0200, Klaus Goger wrote:
> On Sep 2, 2014, at 9:43 AM, Daniel Mack <zonque@gmail.com> wrote:
> > On 09/02/2014 08:32 AM, Klaus Goger wrote:
> >> Use the correct register address for Calibration Active and Interrupt
> >> Enable
> >> 
> >> Signed-off-by: Klaus Goger <klaus.goger@theobroma-systems.com>
> > 
> > These register definitions are currently unused, but your fix is
> > correct. Just curious - are you planning to send patches that make use
> > of them?
> 
> I want to use CAP1106_REG_SENSITIVITY_CONTROL but I have yet to
> decide how to expose it to the user. Probably adding another device tree
> option. But somehow it feels like cluttering up the dts.
> Once I have settled about that I will send patches.

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: Need help adding support for X1 Carbon 2nd Gen's Keyboard
From: Dmitry Torokhov @ 2014-09-08 21:54 UTC (permalink / raw)
  To: José Díez; +Cc: linux-input
In-Reply-To: <540E0F3F.7020101@mediacru.sh>

Hi José,

On Mon, Sep 08, 2014 at 10:19:11PM +0200, José Díez wrote:
> Hello linux-input,
> 
> This is my first post on this mailing list (or on any other kernel
> list, actually), so sorry if I break unwritten ettiquete. Please let
> me know.
> 
> So I recently got a X1 Carbon Thinkpad, and so far everything has
> been working flawlessly, but the Adaptive Keyboard generates some
> ACPI HKEY events with scancodes that are not currently recognised.
> They are reported to the kernel successfully so one could set up
> shortcuts by using something like `acpid`, but perhaps it would be
> better if it generated normal key presses (i.e XF86Something).
> 
> I've read through most of the thinkpad_acpi.c code and understand
> most of it, but so far I've been unable to report normal keyboard
> events from there (and I don't even know if it would be the correct
> thing to do.)
> 
> I must also say that these keys are somewhat odd. In one of the
> modes, there are icons for "snippet tool", "cloud", "man speaking",
> "hands in front of camera". I'm not sure what would be the best way
> to report these, so I seek your advice here.
> 
> At the very least, I can list which scancode is generated by each key.

It would be helpful to know what cations the vendor driver maps for
these keys in Windows (if you still have it available). Then we could
try to come with existing input event definitions for the actions (or
maybe we'd need to add new ones) and map them to the events in
thinkpad_acpi.c.

Thanks.

-- 
Dmitry
--
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 v2 1/2] doc: dt/bindings: input: introduce TI DRV2667 haptic driver description
From: Dmitry Torokhov @ 2014-09-08 23:18 UTC (permalink / raw)
  To: Dan Murphy
  Cc: devicetree, linux-input, linux-kernel, linux-arm-kernel,
	sergei.shtylyov
In-Reply-To: <1408980413-8763-1-git-send-email-dmurphy@ti.com>

On Mon, Aug 25, 2014 at 10:26:51AM -0500, Dan Murphy wrote:
> DRV2667 is a haptic/vibrator driver for Linear Resonant Actuators.
> Adding dt binding for this part
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Applied both, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] HID: rmi: check sanity of the incoming report
From: Andrew Duggan @ 2014-09-09  0:39 UTC (permalink / raw)
  To: Benjamin Tissoires, Jiri Kosina; +Cc: linux-input, linux-kernel
In-Reply-To: <1409925435-6556-1-git-send-email-benjamin.tissoires@redhat.com>

On 09/05/2014 06:57 AM, Benjamin Tissoires wrote:
> In the Dell XPS 13 9333, it appears that sometimes the bus get confused
> and corrupts the incoming data. It fills the input report with the
> sentinel value "ff". Synaptics told us that such behavior does not comes
> from the touchpad itself, so we filter out such reports here.
>
> Unfortunately, we can not simply discard the incoming data because they
> may contain useful information. Most of the time, the misbehavior is
> quite near the end of the report, so we can still use the valid part of
> it.
>
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=1123584
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>   drivers/hid/hid-rmi.c | 31 ++++++++++++++++++++++++++-----
>   1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
> index 8389e81..db92c3b 100644
> --- a/drivers/hid/hid-rmi.c
> +++ b/drivers/hid/hid-rmi.c
> @@ -320,9 +320,6 @@ static int rmi_f11_input_event(struct hid_device *hdev, u8 irq, u8 *data,
>   	int offset;
>   	int i;
>   
> -	if (size < hdata->f11.report_size)
> -		return 0;
> -
>   	if (!(irq & hdata->f11.irq_mask))
>   		return 0;
>   
> @@ -332,9 +329,13 @@ static int rmi_f11_input_event(struct hid_device *hdev, u8 irq, u8 *data,
>   		int fs_bit_position = (i & 0x3) << 1;
>   		int finger_state = (data[fs_byte_position] >> fs_bit_position) &
>   					0x03;
> +		int position = offset + 5 * i;
> +
> +		if (position + 5 > size)
> +			/* partial report, go on with what we received */
> +			break;
>   
> -		rmi_f11_process_touch(hdata, i, finger_state,
> -				&data[offset + 5 * i]);
> +		rmi_f11_process_touch(hdata, i, finger_state, &data[position]);
>   	}
>   	input_mt_sync_frame(hdata->input);
>   	input_sync(hdata->input);
> @@ -412,9 +413,29 @@ static int rmi_read_data_event(struct hid_device *hdev, u8 *data, int size)
>   	return 1;
>   }
>   
> +static int rmi_check_sanity(struct hid_device *hdev, u8 *data, int size)
> +{
> +	int valid_size = size;
> +	/*
> +	 * On the Dell XPS 13 9333, the bus sometimes get confused and fills
> +	 * the report with a sentinel value "ff". Synaptics told us that such
> +	 * behavior does not comes from the touchpad itself, so we filter out
> +	 * such reports here.
> +	 */
> +
> +	while ((data[valid_size - 1] == 0xff) && valid_size > 0)
> +		valid_size--;
> +
> +	return valid_size;
> +}
> +
>   static int rmi_raw_event(struct hid_device *hdev,
>   		struct hid_report *report, u8 *data, int size)
>   {
> +	size = rmi_check_sanity(hdev, data, size);
> +	if (size < 2)
> +		return 0;
> +
>   	switch (data[0]) {
>   	case RMI_READ_DATA_REPORT_ID:
>   		return rmi_read_data_event(hdev, data, size);
I think there should also be a check in rmi_f30_input_event to make sure 
that the F30 data is also valid. The F30 data is at the end of the HID 
report so if the F30 interrupt bit is set, but the value in the report 
is FF then there might be some unintended button events. I think 
checking that size > 0 would be sufficient to make sure the F30 data is 
valid.

Other then that, the sanity check and validation in rmi_f11_input_event 
look good to me.

Andrew

^ permalink raw reply

* Re: [PATCH V5 14/14] input: cyapa: add function to monitor LID close event to off trackpad device
From: Benson Leung @ 2014-09-09  5:09 UTC (permalink / raw)
  To: Dudley Du
  Cc: Dmitry Torokhov, Henrik Rydberg, patrik,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <54003b5a.ac26460a.73fc.ffffe939@mx.google.com>

Hi Dudley,


On Fri, Aug 29, 2014 at 4:35 PM, Dudley Du <dudley.dulixin@gmail.com> wrote:
> Add the function to monitor lid close event to suspend and resume
> trackpad device.
> Because system suspend takes some time to trigger from user space,
> and in that time, the lid panel of the laptop may couple with the
> active trackpad. This may generate stray input events, which may
> in turn cancel the suspend if the drivers use pm_wakup_event(), and
> those input events may trigger something unwanted in the UI.
> So this patch adds the function to do off the trackpad device quickly.
> When the lid is closed, as soon as possible, the trakcpad device must
> be off. And furthermore, the policy on lid close is not always to
> enter suspend (lid closed with external display), and at this time,
> the trackpad device must be disabled as well as again to avoid the
> risk of generating stray events.
> TEST=test on Chromebooks.
>
> Signed-off-by: Dudley Du <dudl@cyrpess.com>

So, I think for the chromeos-kernel, we are moving away from this hack
lid filter solution. Very soon we'll remove this in favor of something
else, so my suggestion would be to drop this from your patch series.



-- 
Benson Leung
Software Engineer, Chrome OS
Google Inc.
bleung@google.com

^ permalink raw reply

* [PATCH v2] Input: serport: Add compat_ioctl routine to support 32bit inputattach in 64bit systems
From: John Sung @ 2014-09-09  5:20 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input, linux-kernel; +Cc: penmount.touch

When running a 32-bit inputattach utility in a 64-bit system, there will be error code "inputattach: can't set device type". This is caused by the serport device driver not supporting compat_ioctl, so that SPIOCSTYPE ioctl fails.

Changes in v2:
(1) Codes of the compat_ioctl are protected by #ifdef CONFIG_COMPAT.
(2) Add a new function serport_set_type() for common codes used by serport_ldisc_ioctl() and serport_ldisc_compat_ioctl().

Signed-off-by: John Sung <penmount.touch@gmail.com>
---
 drivers/input/serio/serport.c |   57 +++++++++++++++++++++++++++++++++--------
 1 file changed, 46 insertions(+), 11 deletions(-)

diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
index 0cb7ef5..1decaa2 100644
--- a/drivers/input/serio/serport.c
+++ b/drivers/input/serio/serport.c
@@ -22,6 +22,10 @@
 #include <linux/serio.h>
 #include <linux/tty.h>
 
+#ifdef CONFIG_COMPAT
+#include <linux/compat.h>
+#endif
+
 MODULE_AUTHOR("Vojtech Pavlik <vojtech@ucw.cz>");
 MODULE_DESCRIPTION("Input device TTY line discipline");
 MODULE_LICENSE("GPL");
@@ -40,6 +44,30 @@ struct serport {
 	unsigned long flags;
 };
 
+#ifdef CONFIG_COMPAT
+#define SPIOCSTYPE32 (_IOW('q', 0x01, compat_ulong_t))
+#endif
+
+/*
+ * serport_set_type() is called by serport_ldisc_ioctl() and
+ * serport_ldisc_compat_ioctl() to set up the serio_device_id values
+ */
+
+static int serport_set_type(struct tty_struct *tty, unsigned long arg)
+{
+	struct serport *serport = (struct serport *) tty->disc_data;
+	unsigned long type;
+
+	if (get_user(type, (unsigned long __user *) arg))
+		return -EFAULT;
+
+	serport->id.proto = type & 0x000000ff;
+	serport->id.id	  = (type & 0x0000ff00) >> 8;
+	serport->id.extra = (type & 0x00ff0000) >> 16;
+
+	return 0;
+}
+
 /*
  * Callback functions from the serio code.
  */
@@ -204,18 +232,8 @@ static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file, u
 
 static int serport_ldisc_ioctl(struct tty_struct * tty, struct file * file, unsigned int cmd, unsigned long arg)
 {
-	struct serport *serport = (struct serport*) tty->disc_data;
-	unsigned long type;
-
 	if (cmd == SPIOCSTYPE) {
-		if (get_user(type, (unsigned long __user *) arg))
-			return -EFAULT;
-
-		serport->id.proto = type & 0x000000ff;
-		serport->id.id	  = (type & 0x0000ff00) >> 8;
-		serport->id.extra = (type & 0x00ff0000) >> 16;
-
-		return 0;
+		return serport_set_type(tty, arg);
 	}
 
 	return -EINVAL;
@@ -232,6 +250,20 @@ static void serport_ldisc_write_wakeup(struct tty_struct * tty)
 	spin_unlock_irqrestore(&serport->lock, flags);
 }
 
+#ifdef CONFIG_COMPAT
+/*
+ * serport_ldisc_compat_ioctl() allows to set the port protocol, and device ID
+ */
+
+static long serport_ldisc_compat_ioctl(struct tty_struct *tty, struct file *file, unsigned int cmd, unsigned long arg)
+{
+	if (cmd == SPIOCSTYPE32)
+		return serport_set_type(tty, arg);
+
+	return -EINVAL;
+}
+#endif
+
 /*
  * The line discipline structure.
  */
@@ -243,6 +275,9 @@ static struct tty_ldisc_ops serport_ldisc = {
 	.close =	serport_ldisc_close,
 	.read =		serport_ldisc_read,
 	.ioctl =	serport_ldisc_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl =	serport_ldisc_compat_ioctl,
+#endif
 	.receive_buf =	serport_ldisc_receive,
 	.write_wakeup =	serport_ldisc_write_wakeup
 };
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH] Input: synaptics - add support for ForcePads
From: Hans de Goede @ 2014-09-09  7:44 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Benjamin Tissoires, Christopher Heiny, linux-kernel,
	Andrew Duggan
In-Reply-To: <20140908165520.GA35051@core.coreip.homeip.net>

Hi,

On 09/08/2014 06:55 PM, Dmitry Torokhov wrote:
> ForcePads are found on HP EliteBook 1040 laptops. They lack any kind of
> physical buttons, instead they generate primary button click when user
> presses somewhat hard on the surface of the touchpad. Unfortunately they
> also report primary button click whenever there are 2 or more contacts
> on the pad, messing up all multi-finger gestures (2-finger scrolling,
> multi-finger tapping, etc). To cope with this behavior we introduce a
> delay (currently 50 msecs) in reporting primary press in case more
> contacts appear.
> 
> For now we are using DMI matching to detect ForcePads, hopefully we'll
> be able to figure a better way down the road.

What about using the pnp-id, in my experience with the recent lenovo
laptops that tends to be more reliable.

Christopher, Andrew (added to the CC), can one of you tell us if there
is a capability bit to detect this, and if not can you perhaps provide
a list of pnp-ids of devices which behave like this ?

> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/mouse/synaptics.c | 80 ++++++++++++++++++++++++++++++++---------
>  drivers/input/mouse/synaptics.h |  5 +++
>  2 files changed, 69 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index e8573c6..5de1bb6 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -618,6 +618,8 @@ static void synaptics_parse_agm(const unsigned char buf[],
>  	priv->agm_pending = true;
>  }
>  
> +static bool is_forcepad;
> +
>  static int synaptics_parse_hw_state(const unsigned char buf[],
>  				    struct synaptics_data *priv,
>  				    struct synaptics_hw_state *hw)
> @@ -629,10 +631,58 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
>  			 ((buf[0] & 0x04) >> 1) |
>  			 ((buf[3] & 0x04) >> 2));
>  
> +		if ((SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) ||
> +			SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) &&
> +		    hw->w == 2) {
> +			synaptics_parse_agm(buf, priv, hw);
> +			return 1;
> +		}
> +
> +		hw->x = (((buf[3] & 0x10) << 8) |
> +			 ((buf[1] & 0x0f) << 8) |
> +			 buf[4]);
> +		hw->y = (((buf[3] & 0x20) << 7) |
> +			 ((buf[1] & 0xf0) << 4) |
> +			 buf[5]);
> +		hw->z = buf[2];
> +
>  		hw->left  = (buf[0] & 0x01) ? 1 : 0;
>  		hw->right = (buf[0] & 0x02) ? 1 : 0;
>  

Moving this up means that on clickpads the button will no longer
be checked + set for hw->w == 2 packets. That seems like an unintended
side effect. If this is intended then this should probably be in its own
patch.

> -		if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) {
> +		if (is_forcepad) {
> +			/* XXX is there a proper capability bit for this? */
> +			/*
> +			 * ForcePads, like Clickpads, use middle button
> +			 * bits to report primary button clicks.
> +			 * Unfortunately they report primary button not only
> +			 * when user presses on the pad above certain threshold,
> +			 * but also when there are more than one finger on the
> +			 * touchpad, which interferes with out multi-finger
> +			 * gestures.
> +			 */
> +			if (hw->z == 0) {
> +				/* No contacts */
> +				priv->press = priv->report_press = false;
> +			} else if (hw->w >= 4 && ((buf[0] ^ buf[3]) & 0x01)) {
> +				/*
> +				 * Single-finger touch with pressure above
> +				 * the threshold.
> +				 */
> +				if  (!priv->press) {
> +					priv->press_start = jiffies;
> +					priv->press = true;
> +				} else if (time_after(jiffies,
> +						priv->press_start +
> +							msecs_to_jiffies(50))) {
> +					priv->report_press = true;
> +				}

You're not setting a timer here, instead relying on there to be further events,
that is probably ok, but maybe put a comment to that extent here ?

> +			} else {
> +				priv->press = false;
> +			}
> +
> +			hw->left = priv->report_press;
> +
> +		} else if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) {
>  			/*
>  			 * Clickpad's button is transmitted as middle button,
>  			 * however, since it is primary button, we will report
> @@ -651,21 +701,6 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
>  			hw->down = ((buf[0] ^ buf[3]) & 0x02) ? 1 : 0;
>  		}
>  
> -		if ((SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) ||
> -			SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) &&
> -		    hw->w == 2) {
> -			synaptics_parse_agm(buf, priv, hw);
> -			return 1;
> -		}
> -
> -		hw->x = (((buf[3] & 0x10) << 8) |
> -			 ((buf[1] & 0x0f) << 8) |
> -			 buf[4]);
> -		hw->y = (((buf[3] & 0x20) << 7) |
> -			 ((buf[1] & 0xf0) << 4) |
> -			 buf[5]);
> -		hw->z = buf[2];
> -
>  		if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) &&
>  		    ((buf[0] ^ buf[3]) & 0x02)) {
>  			switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) & ~0x01) {
> @@ -1642,11 +1677,24 @@ static const struct dmi_system_id __initconst cr48_dmi_table[] = {
>  	{ }
>  };
>  
> +static const struct dmi_system_id forcepad_dmi_table[] __initconst = {
> +#if defined(CONFIG_DMI) && defined(CONFIG_X86)
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook Folio 1040 G1"),
> +		},
> +	},
> +#endif
> +	{ }
> +};
> +
>  void __init synaptics_module_init(void)
>  {
>  	impaired_toshiba_kbc = dmi_check_system(toshiba_dmi_table);
>  	broken_olpc_ec = dmi_check_system(olpc_dmi_table);
>  	cr48_profile_sensor = dmi_check_system(cr48_dmi_table);
> +	is_forcepad = dmi_check_system(forcepad_dmi_table);
>  }
>  
>  static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
> index e594af0..adc80c9 100644
> --- a/drivers/input/mouse/synaptics.h
> +++ b/drivers/input/mouse/synaptics.h
> @@ -177,6 +177,11 @@ struct synaptics_data {
>  	 */
>  	struct synaptics_hw_state agm;
>  	bool agm_pending;			/* new AGM packet received */
> +
> +	/* ForcePad handling */
> +	unsigned long				press_start;
> +	bool					press;
> +	bool					report_press;
>  };
>  
>  void synaptics_module_init(void);
> 

Regards,

Hans

^ permalink raw reply

* [PATCH] Input: atmel_mxt_ts: Add of node type to the i2c table
From: Sjoerd Simons @ 2014-09-09  7:52 UTC (permalink / raw)
  To: Dmitry Torokhov, Nick Dyer
  Cc: linux-input, linux-kernel, linux-samsung-soc,
	Javier Martinez Canillas, Sjoerd Simons

For i2c devices in OF the modalias exposed to userspace is i2c:<node
type>, for the Maxtouch driver this is i2c:maxtouch.

Add maxtouch to the i2c id table such that userspace can correctly
load the module for the device and drop the OF table as it's not
needed for i2c devices.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index db178ed..57ff26d 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2267,16 +2267,11 @@ static int mxt_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(mxt_pm_ops, mxt_suspend, mxt_resume);
 
-static const struct of_device_id mxt_of_match[] = {
-	{ .compatible = "atmel,maxtouch", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, mxt_of_match);
-
 static const struct i2c_device_id mxt_id[] = {
 	{ "qt602240_ts", 0 },
 	{ "atmel_mxt_ts", 0 },
 	{ "atmel_mxt_tp", 0 },
+	{ "maxtouch", 0 },
 	{ "mXT224", 0 },
 	{ }
 };
@@ -2286,7 +2281,6 @@ static struct i2c_driver mxt_driver = {
 	.driver = {
 		.name	= "atmel_mxt_ts",
 		.owner	= THIS_MODULE,
-		.of_match_table = of_match_ptr(mxt_of_match),
 		.pm	= &mxt_pm_ops,
 	},
 	.probe		= mxt_probe,
-- 
2.1.0

^ permalink raw reply related

* Re: [PATCH v3 3/4] mfd: max77693: add haptic of_compatible in mfd_cell
From: Lee Jones @ 2014-09-09  8:15 UTC (permalink / raw)
  To: Jaewon Kim
  Cc: Dmitry Torokhov, Samuel Ortiz, linux-kernel, linux-input,
	Chanwoo Choi
In-Reply-To: <1409917702-17009-4-git-send-email-jaewon02.kim@samsung.com>

On Fri, 05 Sep 2014, Jaewon Kim wrote:

> This patch add haptic of_compatible in order to use the haptic
> device driver using Devicetree.
> 
> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/mfd/max77693.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index fbfed56..03fd431 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -46,7 +46,10 @@ static const struct mfd_cell max77693_devs[] = {
>  	{ .name = "max77693-charger", },
>  	{ .name = "max77693-flash", },
>  	{ .name = "max77693-muic", },
> -	{ .name = "max77693-haptic", },
> +	{
> +	  .name = "max77693-haptic",
> +	  .of_compatible = "maxim,max77693-haptic",
> +	},

Proper tabbing please.

>  };
>  
>  static const struct regmap_config max77693_regmap_config = {

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

* [PATCH] Input: matrix_keypad - make driver useable with GPIO drivers requiring threaded irqs
From: Lothar Waßmann @ 2014-09-09  8:33 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, Dmitry Torokhov, Paul Gortmaker,
	Lothar Waßmann

When trying to use the matrix-keypad driver with GPIO drivers that
require nested irq handlers (e.g. I2C GPIO adapters like PCA9554),
request_irq() fails because the GPIO driver requires a threaded
interrupt handler.

Use request_any_context_irq() to be able to use any GPIO driver as
keypad driver.

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 drivers/input/keyboard/matrix_keypad.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index 8d2e19e..e651fa6 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -332,23 +332,24 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,
 	}
 
 	if (pdata->clustered_irq > 0) {
-		err = request_irq(pdata->clustered_irq,
+		err = request_any_context_irq(pdata->clustered_irq,
 				matrix_keypad_interrupt,
 				pdata->clustered_irq_flags,
 				"matrix-keypad", keypad);
-		if (err) {
+		if (err < 0) {
 			dev_err(&pdev->dev,
 				"Unable to acquire clustered interrupt\n");
 			goto err_free_rows;
 		}
 	} else {
 		for (i = 0; i < pdata->num_row_gpios; i++) {
-			err = request_irq(gpio_to_irq(pdata->row_gpios[i]),
+			err = request_any_context_irq(
+					gpio_to_irq(pdata->row_gpios[i]),
 					matrix_keypad_interrupt,
 					IRQF_TRIGGER_RISING |
 					IRQF_TRIGGER_FALLING,
 					"matrix-keypad", keypad);
-			if (err) {
+			if (err < 0) {
 				dev_err(&pdev->dev,
 					"Unable to acquire interrupt for GPIO line %i\n",
 					pdata->row_gpios[i]);
-- 
1.7.10.4

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

* Re: [PATCH] Handle spurious backslash key repeats on some keyboards
From: Fredrik Hallenberg @ 2014-09-09  9:40 UTC (permalink / raw)
  To: Jiri Kosina, linux-input
In-Reply-To: <1407664566-5303-1-git-send-email-megahallon@gmail.com>

Hi, time for a bump on this one.

On Sun, Aug 10, 2014 at 11:56 AM, Fredrik Hallenberg
<megahallon@gmail.com> wrote:
> Here is my attempt on a fix for bug 70181, please review it. It is
> tested on my nordic Corsair K70, if this solution is deemed acceptable
> I can ask some people with other problematic keyboards to test it.
>
> Signed-off-by: Fredrik Hallenberg <megahallon@gmail.com>
> ---
>  drivers/hid/hid-input.c | 14 ++++++++++++++
>  include/linux/hid.h     |  1 +
>  2 files changed, 15 insertions(+)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 2619f7f..56429c0 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -1085,6 +1085,20 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
>                 return;
>         }
>
> +       /*
> +        * Some keyboards will report both HID keys 0x31 (\ and |) and
> +        * 0x32 (Non-US # and ~) which are both mapped to
> +        * KEY_BACKSLASH. This will cause spurious events causing
> +        * repeated backslash key presses. Handle this by tracking the
> +        * active HID code and ignoring the other one.
> +        */
> +       if (usage->type == EV_KEY && usage->code == KEY_BACKSLASH) {
> +               if (value)
> +                       field->hidinput->backslash_usage = usage->hid;
> +               else if (field->hidinput->backslash_usage != usage->hid)
> +                       return;
> +       }
> +
>         /* report the usage code as scancode if the key status has changed */
>         if (usage->type == EV_KEY && !!test_bit(usage->code, input->key) != value)
>                 input_event(input, EV_MSC, MSC_SCAN, usage->hid);
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index f53c4a9..1c59f8f 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -448,6 +448,7 @@ struct hid_input {
>         struct list_head list;
>         struct hid_report *report;
>         struct input_dev *input;
> +       unsigned backslash_usage;
>  };
>
>  enum hid_type {
> --
> 2.1.0.rc1
>

^ permalink raw reply

* Re: [PATCH RESEND v3] input: Add ROHM BU21023/24 Dual touch support resistive touchscreens
From: Yoichi Yuasa @ 2014-09-09  9:40 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: yuasa, linux-input
In-Reply-To: <20140828114536.a28f06bcd8a7ada3a912e3bc@linux-mips.org>

Hi Dmitry,

I updated the driver according to your comments. 
Furthermore, is there any comment? 

Thanks,

Yoichi

On Thu, 28 Aug 2014 11:45:36 +0900
Yoichi Yuasa <yuasa@linux-mips.org> wrote:

> v3 Changes:
> - fix multi touch slots initialization
> - fix set_bit BTN_TOUCH
> - remove input_unregister_device()
> - switch to __set_bit()
> 
> v2 Changes:
> - remove polling mode
> - switch to threaded interrupt
> - switch to managed resources
> - use MT-B protocol with input_mt_assign_slots
> - provide ST emulation
> - firmware load and device initialization are shifted at opening the device
> - add error handling for IO operations
> 
> Signed-off-by: Yoichi Yuasa <yuasa@linux-mips.org>
> ---
>  drivers/input/touchscreen/Kconfig        |   11 +
>  drivers/input/touchscreen/Makefile       |    1 +
>  drivers/input/touchscreen/rohm_bu21023.c |  931 ++++++++++++++++++++++++++++++
>  drivers/input/touchscreen/rohm_bu21023.h |  258 +++++++++
>  4 files changed, 1201 insertions(+)
> 

^ permalink raw reply

* Re: [PATCH] Handle spurious backslash key repeats on some keyboards
From: Jiri Kosina @ 2014-09-09  9:42 UTC (permalink / raw)
  To: Fredrik Hallenberg; +Cc: linux-input
In-Reply-To: <CAMsZVf_h2fODG2UTs6ddZYZLGfF5hdyY+1PdwXO8N5taDahRwg@mail.gmail.com>

On Tue, 9 Sep 2014, Fredrik Hallenberg wrote:

> Hi, time for a bump on this one.

Do you have any idea how common this pattern is? How many keyboards have 
you encountered doing this?

I hate to be polluting generic code with such strange quirks ... we've 
spent quite some time factoring those out to specific drivers.
So knowing how commong this is could be helpful in deciding how to handle 
this.

Thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] Input: atmel_mxt_ts: Add of node type to the i2c table
From: Javier Martinez Canillas @ 2014-09-09 10:21 UTC (permalink / raw)
  To: Sjoerd Simons, Dmitry Torokhov, Nick Dyer
  Cc: linux-input, linux-kernel, linux-samsung-soc, Lee Jones
In-Reply-To: <1410249158-18192-1-git-send-email-sjoerd.simons@collabora.co.uk>

[adding Lee Jones to cc list since I'm referring on a series he posted]

Hello Sjoerd,

On 09/09/2014 09:52 AM, Sjoerd Simons wrote:
> For i2c devices in OF the modalias exposed to userspace is i2c:<node
> type>, for the Maxtouch driver this is i2c:maxtouch.
> 
> Add maxtouch to the i2c id table such that userspace can correctly
> load the module for the device and drop the OF table as it's not
> needed for i2c devices.
> 
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index db178ed..57ff26d 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -2267,16 +2267,11 @@ static int mxt_resume(struct device *dev)
>  
>  static SIMPLE_DEV_PM_OPS(mxt_pm_ops, mxt_suspend, mxt_resume);
>  
> -static const struct of_device_id mxt_of_match[] = {
> -	{ .compatible = "atmel,maxtouch", },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, mxt_of_match);
> -
>  static const struct i2c_device_id mxt_id[] = {
>  	{ "qt602240_ts", 0 },
>  	{ "atmel_mxt_ts", 0 },
>  	{ "atmel_mxt_tp", 0 },
> +	{ "maxtouch", 0 },
>  	{ "mXT224", 0 },
>  	{ }
>  };
> @@ -2286,7 +2281,6 @@ static struct i2c_driver mxt_driver = {
>  	.driver = {
>  		.name	= "atmel_mxt_ts",
>  		.owner	= THIS_MODULE,
> -		.of_match_table = of_match_ptr(mxt_of_match),
>  		.pm	= &mxt_pm_ops,
>  	},
>  	.probe		= mxt_probe,
> 

I see that Lee is working to allow the I2C subsystem to not need an I2C ID
table to match [0]. I'll let Lee to comment what the future plans are and if
his series are going to solve your issue since I'm not that familiar with the
I2C core.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/6/20/199

^ permalink raw reply

* Re: [PATCH] Input: atmel_mxt_ts: Add of node type to the i2c table
From: Nick Dyer @ 2014-09-09 10:29 UTC (permalink / raw)
  To: Javier Martinez Canillas, Sjoerd Simons, Dmitry Torokhov
  Cc: linux-input, linux-kernel, linux-samsung-soc, Lee Jones
In-Reply-To: <540ED495.20609@collabora.co.uk>

On 09/09/14 11:21, Javier Martinez Canillas wrote:
> On 09/09/2014 09:52 AM, Sjoerd Simons wrote:
>> For i2c devices in OF the modalias exposed to userspace is i2c:<node
>> type>, for the Maxtouch driver this is i2c:maxtouch.
>>
>> Add maxtouch to the i2c id table such that userspace can correctly
>> load the module for the device and drop the OF table as it's not
>> needed for i2c devices.
> I see that Lee is working to allow the I2C subsystem to not need an I2C ID
> table to match [0]. I'll let Lee to comment what the future plans are and if
> his series are going to solve your issue since I'm not that familiar with the
> I2C core.
> 
> Best regards,
> Javier
> 
> [0]: https://lkml.org/lkml/2014/6/20/199

I can see the benefit of not having the duplication. Am I correct that
you're saying that it might make more sense to remove the i2c ids rather
than the OF table, if Lee's changes are accepted?

^ permalink raw reply

* Re: [PATCH] Handle spurious backslash key repeats on some keyboards
From: Fredrik Hallenberg @ 2014-09-09 10:37 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input
In-Reply-To: <alpine.LNX.2.00.1409091141340.5523@pobox.suse.cz>

I have seen reports for Gigabyte Osmium, Corsair Vengeance K-series
and QPAD MK-series. There are probably more. The reason I chose to not
make it a quirk specific to some specific hardware is that it feels
more like a general problem caused by the fact the there are two HID
key codes mapped to a single Linux key code.

On Tue, Sep 9, 2014 at 11:42 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Tue, 9 Sep 2014, Fredrik Hallenberg wrote:
>
>> Hi, time for a bump on this one.
>
> Do you have any idea how common this pattern is? How many keyboards have
> you encountered doing this?
>
> I hate to be polluting generic code with such strange quirks ... we've
> spent quite some time factoring those out to specific drivers.
> So knowing how commong this is could be helpful in deciding how to handle
> this.
>
> Thanks.
>
> --
> Jiri Kosina
> SUSE Labs

^ permalink raw reply

* Re: [PATCH] Input: atmel_mxt_ts: Add of node type to the i2c table
From: Sjoerd Simons @ 2014-09-09 10:54 UTC (permalink / raw)
  To: Nick Dyer
  Cc: Javier Martinez Canillas, Dmitry Torokhov, linux-input,
	linux-kernel, linux-samsung-soc, Lee Jones
In-Reply-To: <540ED671.5020103@itdev.co.uk>

[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]

On Tue, 2014-09-09 at 11:29 +0100, Nick Dyer wrote:
> On 09/09/14 11:21, Javier Martinez Canillas wrote:
> > On 09/09/2014 09:52 AM, Sjoerd Simons wrote:
> >> For i2c devices in OF the modalias exposed to userspace is i2c:<node
> >> type>, for the Maxtouch driver this is i2c:maxtouch.
> >>
> >> Add maxtouch to the i2c id table such that userspace can correctly
> >> load the module for the device and drop the OF table as it's not
> >> needed for i2c devices.
> > I see that Lee is working to allow the I2C subsystem to not need an I2C ID
> > table to match [0]. I'll let Lee to comment what the future plans are and if
> > his series are going to solve your issue since I'm not that familiar with the
> > I2C core.
> 
> I can see the benefit of not having the duplication. Am I correct that
> you're saying that it might make more sense to remove the i2c ids rather
> than the OF table, if Lee's changes are accepted?

You would still need the i2C table for non-OF platforms ofcourse.

I'm not sure what happens with the modalias as exposed to userspace with
Lee's patchset, if that gets changed to prefer an of: type instead of
the current i2c: prefixed ones this patch (at that point) isn't needed.

-- 
Sjoerd Simons <sjoerd.simons@collabora.co.uk>
Collabora Ltd.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6170 bytes --]

^ 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