Linux Input/HID development
 help / color / mirror / Atom feed
* Re: input question: ambient light sensor button
From: Dmitry Torokhov @ 2013-11-26 19:19 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Benjamin Tissoires, Jiri Kosina, linux-kernel@vger.kernel.org,
	linux-input
In-Reply-To: <201311221213.49580@pali>

On Fri, Nov 22, 2013 at 12:13:49PM +0100, Pali Rohár wrote:
> On Wednesday 20 November 2013 16:59:42 Benjamin Tissoires wrote:
> > Hi,
> > 
> > On Wed, Nov 20, 2013 at 9:50 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> > >> > > I guess we need patch adding
> > >> > > 
> > >> > >   #define KEY_ALS_TOGGLE  0x230
> > >> > > 
> > >> > > Thanks.
> > >> > 
> > >> > So there is no good key for als yet?
> > >> 
> > >> No, but if you send me a patch adding it I will gladly
> > >> apply it.
> > >> 
> > >> Thanks.
> > > 
> > > Ok, here is patch:
> > > 
> > > Add key code for ambient light sensor button
> > > 
> > > More notebooks have special button for enabling/disabling
> > > ambient light sensor. This patch adding new als code to
> > > input.h header file.
> > > 
> > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > 
> > > diff --git a/include/uapi/linux/input.h
> > > b/include/uapi/linux/input.h index a372627..1562f10 100644
> > > --- a/include/uapi/linux/input.h
> > > +++ b/include/uapi/linux/input.h
> > > @@ -719,6 +719,8 @@ struct input_keymap_entry {
> > > 
> > >  #define BTN_DPAD_LEFT          0x222
> > >  #define BTN_DPAD_RIGHT         0x223
> > > 
> > > +#define KEY_ALS_TOGGLE         0x230
> > 
> > Could you just add a comment explaining that ALS is ambiant
> > light sensor? Otherwise, I'm sure someone else will use this
> > event code for an other thing... :)
> > 
> > Cheers,
> > Benjamin
> 
> Ok, here is new diff with comment:

Applied, thank you.

> 
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index a372627..7bacdb5 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -719,6 +719,8 @@ struct input_keymap_entry {
>  #define BTN_DPAD_LEFT		0x222
>  #define BTN_DPAD_RIGHT		0x223
>  
> +#define KEY_ALS_TOGGLE		0x230	/* Ambient light sensor */
> +
>  #define BTN_TRIGGER_HAPPY		0x2c0
>  #define BTN_TRIGGER_HAPPY1		0x2c0
>  #define BTN_TRIGGER_HAPPY2		0x2c1
> 
> 
> -- 
> Pali Rohár
> pali.rohar@gmail.com



-- 
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: [lm-sensors] [PATCH v2] Input: ads7846: Convert to hwmon_device_register_with_groups
From: Jean Delvare @ 2013-11-26 19:36 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Guenter Roeck, lm-sensors, linux-kernel, linux-input
In-Reply-To: <20131126185846.GB22242@core.coreip.homeip.net>

Hi Dmitry,

On Tue, 26 Nov 2013 10:58:46 -0800, Dmitry Torokhov wrote:
> On Mon, Nov 25, 2013 at 08:39:04PM -0800, Guenter Roeck wrote:
> > Simplify the code and create mandatory 'name' attribute by using
> > new hwmon API.
> 
> So this moves hwmon attributes from the parent i2c device to the hwmon
> device, right? Would not that break userspace which expects to find the
> attributes where they were?

libsensors supports that just fine since version 3.0.3 which was
released in September 2008, over 5 years ago.

Applications bypassing libsensors are on their own, and always have
been. But several other drivers have had their attributes in the hwmon
device forever so even these applications are likely to be happy with
other drivers moving in that direction.

-- 
Jean Delvare

^ permalink raw reply

* Re: [PATCH 4/4 v3] Input: wacom - add SW_TOUCH to include/uapi/linux/input.h
From: Ping Cheng @ 2013-11-26 20:38 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, Jason Gerecke, Chris Bagwell, Peter Hutterer
In-Reply-To: <20131126025928.GD31517@core.coreip.homeip.net>

On Mon, Nov 25, 2013 at 6:59 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Ping,
>
> On Fri, Nov 15, 2013 at 12:00:02PM -0800, Ping Cheng wrote:
>>> @@ -856,6 +856,7 @@ struct input_keymap_entry {
>>  #define SW_FRONT_PROXIMITY   0x0b  /* set = front proximity sensor active */
>>  #define SW_ROTATE_LOCK               0x0c  /* set = rotate locked/disabled */
>>  #define SW_LINEIN_INSERT     0x0d  /* set = inserted */
>> +#define SW_TOUCH             0x0e  /* set = touch switch turned on (touch events off) */
>
> I do not think we should be adding this as is as it seems to be very
> wacom-specific. I'd rather call it something else, like SW_MUTE_DEVICE
> or similar.

Thank you Dmitry for reviewing the patch. I can change the event to
SW_MUTE_DEVICE.

> I also wonder if this should really be a switch: can you query it's
> state?

Yes, it is a hardware switch. The state is posted periodically from the device.

> What happen if user plugs in the device, engages the switch and
> then [re]loads the driver?

The state will be updated in the driver, with this patch.

> Will the state be still signalled properly?

Sure.

> What about suspend/resume or hibernation?

Since the state is updated regularly, it won't be lost when system wakes up.

Basically, we are dealing with a hardware switch, which is controlled
by end users. Firmware and driver can not do much about it except
reporting its state.

Does this answer your questions?

Ping

^ permalink raw reply

* [PATCH 00/15] Enable compilation of various Renesas drivers with COMPILE_TEST
From: Laurent Pinchart @ 2013-11-27  1:18 UTC (permalink / raw)
  To: linux-sh-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Thierry Reding,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA, Russell King, Vinod Koul,
	Magnus Damm, Eduardo Valentin, Tomi Valkeinen,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Zhang Rui, Chris Ball,
	Jean-Christophe Plagniol-Villard,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA, Samuel Ortiz,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, Ian Molton, Mark Brown,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	netdev-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-spi-u79uwXL29TY76Z2rM5mHXA

Hello,

This patch series enables driver compilation with COMPILE_TEST for various
Renesas drivers. The approach taken here is to split the Kconfig depends line
in two, with compile dependencies (if any) on one line and runtime
dependencies (as a list of the platforms on which the hardware can be found)
on another line. The runtime dependencies line is extended with
|| COMPILE_TEST to enable test compilation. This clearly identifies compile
dependencies, and allow test compilation while not clobbering the
configuration menu with useless options in the normal case.

The patches are based on top of v3.13-rc1. To avoid build warnings or errors
during bisection the following patches sent to linux-sh and appropriate
mailing lists should be applied first:

serial: sh-sci: Fix warnings due to improper casts and printk formats
DMA: shdma: Fix warnings due to improper casts and printk formats
DMA: shdma: Fix warnings due to declared but unused symbols
DMA: shdma: Make sh_dmae_pm static
v4l: sh_vou: Fix warnings due to improper casts and printk formats
mmc: sh_mmcif: Factorize DMA channel request and configuration code
mmc: sh_mmcif: Fix compilation warning on 64-bit platforms
mtd: sh_flctl: Fix warnings due to improper casts
fbdev: sh_mobile_lcdcfb: Don't use plain 0 as NULL pointer
spi: sh-msiof: Fix warnings due to improper casts
spi: rcar: Fix uninitialized variable warning
spi: rcar: Fix pointer cast in the remove function

For convenience I've pushed the whole series and its prerequisites to my git
tree at

	git://linuxtv.org/pinchartl/fbdev.git clocks/ccf/multiarch-drivers

The patches will need to go through their respective subsystem's trees. To
help me tracking the mainlining state I'd appreciate if the respective
maintainers could notify me when applying the patches to their trees. Before
doing so I'd like to get an ack from Mark Brown and/or Russell King on the
approach.

Cc: dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-pwm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Chris Ball <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org>
Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Eduardo Valentin <eduardo.valentin-l0cyMroinI0@public.gmane.org>
Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Cc: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
Cc: Ian Molton <ian-zdned+2MO1+9FHfhHBbuYA@public.gmane.org>
Cc: Jean-Christophe Plagniol-Villard <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
Cc: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
Cc: Magnus Damm <magnus.damm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Mauro Carvalho Chehab <m.chehab-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Samuel Ortiz <samuel-jcdQHdrhKHMdnm+yROfE0A@public.gmane.org>
Cc: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
Cc: Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
Cc: Zhang Rui <rui.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Laurent Pinchart (15):
  i2c: shmobile/rcar: Restrict non-COMPILE_TEST compilation
  input: sh_keysc: Restrict non-COMPILE_TEST compilation
  serial: sh-sci: Restrict non-COMPILE_TEST compilation
  iommu: shmobile: Enable driver compilation with COMPILE_TEST
  DMA: shdma: Enable driver compilation with COMPILE_TEST
  v4l: sh_vou: Enable driver compilation with COMPILE_TEST
  mmc: sdhi: Enable driver compilation with COMPILE_TEST
  mmc: sh_mmcif: Enable driver compilation with COMPILE_TEST
  mtd: sh_flctl: Enable driver compilation with COMPILE_TEST
  irda: sh_irda: Enable driver compilation with COMPILE_TEST
  pwm: pwm-renesas-tpu: Enable driver compilation with COMPILE_TEST
  thermal: rcar-thermal: Enable driver compilation with COMPILE_TEST
  fbdev: sh-mobile-lcdcfb: Enable driver compilation with COMPILE_TEST
  spi: sh-msiof: Enable driver compilation with COMPILE_TEST
  sh: intc: Enable driver compilation with COMPILE_TEST

 drivers/dma/sh/Kconfig         | 2 +-
 drivers/i2c/busses/Kconfig     | 4 ++--
 drivers/input/keyboard/Kconfig | 2 +-
 drivers/iommu/Kconfig          | 1 +
 drivers/media/platform/Kconfig | 3 ++-
 drivers/mmc/host/Kconfig       | 6 ++++--
 drivers/mtd/nand/Kconfig       | 2 +-
 drivers/net/irda/Kconfig       | 3 ++-
 drivers/pwm/Kconfig            | 2 +-
 drivers/sh/intc/Kconfig        | 2 +-
 drivers/spi/Kconfig            | 3 ++-
 drivers/thermal/Kconfig        | 2 +-
 drivers/tty/serial/Kconfig     | 3 ++-
 drivers/video/Kconfig          | 8 +++++---
 14 files changed, 26 insertions(+), 17 deletions(-)

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* [PATCH 02/15] input: sh_keysc: Restrict non-COMPILE_TEST compilation
From: Laurent Pinchart @ 2013-11-27  1:18 UTC (permalink / raw)
  To: linux-sh; +Cc: linux-arm-kernel, Dmitry Torokhov, linux-input
In-Reply-To: <1385515117-23664-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>

Hardware supported by the driver is only found on SUPERH or
ARCH_SHMOBILE platforms. Restrict non-COMPILE_TEST compilation to them.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/input/keyboard/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index bb174c1..a673c9f 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -525,7 +525,7 @@ config KEYBOARD_SUNKBD
 
 config KEYBOARD_SH_KEYSC
 	tristate "SuperH KEYSC keypad support"
-	depends on SUPERH || ARM || COMPILE_TEST
+	depends on SUPERH || ARCH_SHMOBILE || COMPILE_TEST
 	help
 	  Say Y here if you want to use a keypad attached to the KEYSC block
 	  on SuperH processors such as sh7722 and sh7343.
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH v2] Input: ads7846: Convert to hwmon_device_register_with_groups
From: Guenter Roeck @ 2013-11-27  1:20 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, lm-sensors, linux-kernel
In-Reply-To: <20131126185846.GB22242@core.coreip.homeip.net>

On 11/26/2013 10:58 AM, Dmitry Torokhov wrote:
> Hi Guenter,
>
> On Mon, Nov 25, 2013 at 08:39:04PM -0800, Guenter Roeck wrote:
>> Simplify the code and create mandatory 'name' attribute by using
>> new hwmon API.
>
> So this moves hwmon attributes from the parent i2c device to the hwmon
> device, right? Would not that break userspace which expects to find the
> attributes where they were?
>

In addition to Jean's earlier comments ... s/i2c/spi/, I assume. spi devices
don't create the mandatory name attribute automatically, which means
that the created hwmon device was not recognized by standard user space
applications (eg the sensors command or anything else using libsensors)
in the first place. Which in turn means that only applications which don't
support the standard hwmon ABI - if there are any - would be affected.
What we are more concerned about is to make sure that applications
which _do_ follow the hwmon ABI are working.

Thanks,
Guenter

^ permalink raw reply

* [PATCH] drivers: input: touchscreen: sur40: use static variable instead of stack varialbe for 'packet_id'
From: Chen Gang @ 2013-11-27  2:15 UTC (permalink / raw)
  To: dmitry.torokhov, floe, rydberg, David Herrmann
  Cc: rkuo, linux-kernel@vger.kernel.org, linux-input
In-Reply-To: <20131125011908.GA18921@codeaurora.org>

'packet_id' is used for checking sequence whether in order, it need be
static variable independent from sur40_poll().

The related warning (with allmodconfig under hexagon):

  drivers/input/touchscreen/sur40.c: In function 'sur40_poll':
  drivers/input/touchscreen/sur40.c:297:6: warning: 'packet_id' may be used uninitialized in this function [-Wuninitialized]


Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 drivers/input/touchscreen/sur40.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index cfd1b7e..5dfd01a 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -251,7 +251,7 @@ static void sur40_poll(struct input_polled_dev *polldev)
 	struct sur40_state *sur40 = polldev->private;
 	struct input_dev *input = polldev->input;
 	int result, bulk_read, need_blobs, packet_blobs, i;
-	u32 packet_id;
+	static u32 packet_id;
 
 	struct sur40_header *header = &sur40->bulk_in_buffer->header;
 	struct sur40_blob *inblob = &sur40->bulk_in_buffer->blobs[0];
-- 
1.7.7.6

^ permalink raw reply related

* Re: [PATCH v2] Input:Add support for DualPoint device on Dell XT2 model
From: Dmitry Torokhov @ 2013-11-27  0:18 UTC (permalink / raw)
  To: Yunkang Tang
  Cc: cernekee, dturvene, linux-input, ndevos, jclift, yunkang.tang
In-Reply-To: <1384524702-3411-1-git-send-email-yunkang.tang@cn.alps.com>

Hi Yunkang,

On Fri, Nov 15, 2013 at 10:11:42PM +0800, Yunkang Tang wrote:
> Hi all,
> 
>    Here is the 2nd version for supporting DualPoint device on Dell XT2 model
> 
> Changelist:
>  - Bugfix for trackpoint's behavior was abnormal in v1.
>   [Root Cause]
>    Because of the special MPU controller being used in this DualPoint device,
>   when sending ALPS magic know command, not only touchpad but also trackpoint
>   received the commands. This would cause trackpoint also enter an unexpected
>   raw mode. Unfortunately, trackpoint's raw raw packet has the same check bit
>   as touchpad's but with different packet format. And this would cause 
>   trackpoint's abnormal behavior.
> 
>   [Solution]
>    1. Define a new ALPS_PROTO_V6 macro for this device.
>    2. Add new initialization logic.

It would help greatly if you could document what parameters the new
initialization routine sets.

>    3. Add new packet process logic.

I am curious why we need the new packet processing logic. Apparently the
touchpad can work in the original mode that we already know how to
parse; we just need to make sure the trackpoint is initialized properly.
Or yet another protocol flavor is a must?

Thanks!

>    # Touchpad's dimension is 2047*1535.
> 
> SelfTest:
>  1. Move on touchpad -- OK
>  2. Vertical/Horizontal wheel on touchpad -- OK
>  3. Tap / Tap and drag on touchpad -- OK
>  4. Click with touchpad's L/R button -- OK
>  5. Move on touchpad with pressing touchpad's button -- OK
>  6. Move on trackpoint -- OK
>  7. Click with trackpoint's L/R button -- OK
>  8. Move on trackpoint with pressing trackpoint's button -- OK
>  9. Move cursor with both touchpad and trackpoint -- OK
>  10. Move on touchpad with pressing trackpoint's button -- OK
>  11. Move on trackpoint with pressing touchpad's button -- OK
> 
>  Thanks
> 
> Signed-off-by: Yunkang Tang <yunkang.tang@cn.alps.com>
> ---
>  drivers/input/mouse/alps.c | 206 ++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/input/mouse/alps.h |   1 +
>  2 files changed, 204 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index ca7a26f..5cf62e3 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -70,6 +70,25 @@ static const struct alps_nibble_commands alps_v4_nibble_commands[] = {
>  	{ PSMOUSE_CMD_SETSCALE11,	0x00 }, /* f */
>  };
>  
> +static const struct alps_nibble_commands alps_v6_nibble_commands[] = {
> +	{ PSMOUSE_CMD_ENABLE,		0x00 }, /* 0 */
> +	{ PSMOUSE_CMD_SETRATE,		0x0a }, /* 1 */
> +	{ PSMOUSE_CMD_SETRATE,		0x14 }, /* 2 */
> +	{ PSMOUSE_CMD_SETRATE,		0x28 }, /* 3 */
> +	{ PSMOUSE_CMD_SETRATE,		0x3c }, /* 4 */
> +	{ PSMOUSE_CMD_SETRATE,		0x50 }, /* 5 */
> +	{ PSMOUSE_CMD_SETRATE,		0x64 }, /* 6 */
> +	{ PSMOUSE_CMD_SETRATE,		0xc8 }, /* 7 */
> +	{ PSMOUSE_CMD_GETID,		0x00 }, /* 8 */
> +	{ PSMOUSE_CMD_GETINFO,		0x00 }, /* 9 */
> +	{ PSMOUSE_CMD_SETRES,		0x00 }, /* a */
> +	{ PSMOUSE_CMD_SETRES,		0x01 }, /* b */
> +	{ PSMOUSE_CMD_SETRES,		0x02 }, /* c */
> +	{ PSMOUSE_CMD_SETRES,		0x03 }, /* d */
> +	{ PSMOUSE_CMD_SETSCALE21,	0x00 }, /* e */
> +	{ PSMOUSE_CMD_SETSCALE11,	0x00 }, /* f */
> +};
> +
>  
>  #define ALPS_DUALPOINT		0x02	/* touchpad has trackstick */
>  #define ALPS_PASS		0x04	/* device has a pass-through port */
> @@ -103,6 +122,7 @@ static const struct alps_model_info alps_model_data[] = {
>  	/* Dell Latitude E5500, E6400, E6500, Precision M4400 */
>  	{ { 0x62, 0x02, 0x14 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf,
>  		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },
> +	{ { 0x73, 0x00, 0x14 }, 0x00, ALPS_PROTO_V6, 0xff, 0xff, ALPS_DUALPOINT },		/* Dell XT2 */
>  	{ { 0x73, 0x02, 0x50 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_FOUR_BUTTONS },		/* Dell Vostro 1400 */
>  	{ { 0x52, 0x01, 0x14 }, 0x00, ALPS_PROTO_V2, 0xff, 0xff,
>  		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },				/* Toshiba Tecra A11-11L */
> @@ -645,6 +665,76 @@ static void alps_process_packet_v3(struct psmouse *psmouse)
>  	alps_process_touchpad_packet_v3(psmouse);
>  }
>  
> +static void alps_process_packet_v6(struct psmouse *psmouse)
> +{
> +	struct alps_data *priv = psmouse->private;
> +	unsigned char *packet = psmouse->packet;
> +	struct input_dev *dev = psmouse->dev;
> +	struct input_dev *dev2 = priv->dev2;
> +	int x, y, z, left, right, middle;
> +
> +	/*
> +	 * We can use Byte5 to distinguish if the packet is from Touchpad
> +	 * or Trackpoint.
> +	 * Touchpad:	0 - 0x7E
> +	 * Trackpoint:	0x7F
> +	 */
> +	if (packet[5] == 0x7F) {
> +		/* It should be a DualPoint when received Trackpoint packet */
> +		if (!(priv->flags & ALPS_DUALPOINT))
> +			return;
> +
> +		/* Trackpoint packet */
> +		x = packet[1] | ((packet[3] & 0x20) << 2);
> +		y = packet[2] | ((packet[3] & 0x40) << 1);
> +		z = packet[4];
> +		left = packet[3] & 0x01;
> +		right = packet[3] & 0x02;
> +		middle = packet[3] & 0x04;
> +
> +		/* To prevent the cursor jump when finger lifted */
> +		if (x == 0x7F && y == 0x7F && z == 0x7F)
> +			x = y = z = 0;
> +
> +		/* Divide 4 since trackpoint's speed is too fast */
> +		input_report_rel(dev2, REL_X, (char)x / 4);
> +		input_report_rel(dev2, REL_Y, -((char)y / 4));
> +
> +		input_report_key(dev2, BTN_LEFT, left);
> +		input_report_key(dev2, BTN_RIGHT, right);
> +		input_report_key(dev2, BTN_MIDDLE, middle);
> +
> +		input_sync(dev2);
> +		return;
> +	}
> +
> +	/* Touchpad packet */
> +	x = packet[1] | ((packet[3] & 0x78) << 4);
> +	y = packet[2] | ((packet[4] & 0x78) << 4);
> +	z = packet[5];
> +	left = packet[3] & 0x01;
> +	right = packet[3] & 0x02;
> +
> +	if (z > 30)
> +		input_report_key(dev, BTN_TOUCH, 1);
> +	if (z < 25)
> +		input_report_key(dev, BTN_TOUCH, 0);
> +
> +	if (z > 0) {
> +		input_report_abs(dev, ABS_X, x);
> +		input_report_abs(dev, ABS_Y, y);
> +	}
> +
> +	input_report_abs(dev, ABS_PRESSURE, z);
> +	input_report_key(dev, BTN_TOOL_FINGER, z > 0);
> +
> +	/* v6 touchpad does not have middle button */
> +	input_report_key(dev, BTN_LEFT, left);
> +	input_report_key(dev, BTN_RIGHT, right);
> +
> +	input_sync(dev);
> +}
> +
>  static void alps_process_packet_v4(struct psmouse *psmouse)
>  {
>  	struct alps_data *priv = psmouse->private;
> @@ -897,7 +987,7 @@ static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
>  	}
>  
>  	/* Bytes 2 - pktsize should have 0 in the highest bit */
> -	if (priv->proto_version != ALPS_PROTO_V5 &&
> +	if ((priv->proto_version < ALPS_PROTO_V5) &&
>  	    psmouse->pktcnt >= 2 && psmouse->pktcnt <= psmouse->pktsize &&
>  	    (psmouse->packet[psmouse->pktcnt - 1] & 0x80)) {
>  		psmouse_dbg(psmouse, "refusing packet[%i] = %x\n",
> @@ -1085,6 +1175,80 @@ static int alps_absolute_mode_v1_v2(struct psmouse *psmouse)
>  	return ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETPOLL);
>  }
>  
> +static int alps_monitor_mode_send_word(struct psmouse *psmouse, u16 word)
> +{
> +	int i, nibble;
> +
> +	/*
> +	 * b0-b11 are valid bits, send sequence is inverse.
> +	 * e.g. when word = 0x0123, nibble send sequence is 3, 2, 1
> +	 */
> +	for (i = 0; i <= 8; i += 4) {
> +		nibble = (word >> i) & 0xf;
> +		if (alps_command_mode_send_nibble(psmouse, nibble))
> +			return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int alps_monitor_mode_write_reg(struct psmouse *psmouse,
> +				       u16 addr, u16 value)
> +{
> +	struct ps2dev *ps2dev = &psmouse->ps2dev;
> +
> +	/* 0x0A0 is the command to write the word */
> +	if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE) ||
> +	    alps_monitor_mode_send_word(psmouse, 0x0A0) ||
> +	    alps_monitor_mode_send_word(psmouse, addr) ||
> +	    alps_monitor_mode_send_word(psmouse, value) ||
> +	    ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE))
> +		return -1;
> +
> +	return 0;
> +}
> +
> +static int alps_monitor_mode(struct psmouse *psmouse, bool enable)
> +{
> +	struct ps2dev *ps2dev = &psmouse->ps2dev;
> +
> +	if (enable) {
> +		/* EC E9 F5 F5 E7 E6 E7 E9 to enter monitor mode */
> +		if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_RESET_WRAP) ||
> +		    ps2_command(ps2dev, NULL, PSMOUSE_CMD_GETINFO) ||
> +		    ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
> +		    ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
> +		    ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21) ||
> +		    ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
> +		    ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21) ||
> +		    ps2_command(ps2dev, NULL, PSMOUSE_CMD_GETINFO))
> +			return -1;
> +	} else {
> +		/* EC to exit monitor mode */
> +		if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_RESET_WRAP))
> +			return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int alps_absolute_mode_v6(struct psmouse *psmouse)
> +{
> +	u16 reg_val = 0x181;
> +	int ret = -1;
> +
> +	/* enter monitor mode, to write the register */
> +	if (alps_monitor_mode(psmouse, true))
> +		return -1;
> +
> +	ret = alps_monitor_mode_write_reg(psmouse, 0x000, reg_val);
> +
> +	if (alps_monitor_mode(psmouse, false))
> +		ret = -1;
> +
> +	return ret;
> +}
> +
>  static int alps_get_status(struct psmouse *psmouse, char *param)
>  {
>  	/* Get status: 0xF5 0xF5 0xF5 0xE9 */
> @@ -1189,6 +1353,32 @@ static int alps_hw_init_v1_v2(struct psmouse *psmouse)
>  	return 0;
>  }
>  
> +static int alps_hw_init_v6(struct psmouse *psmouse)
> +{
> +	unsigned char param[2] = {0xC8, 0x14};
> +
> +	/* Enter passthrough mode to let trackpoint enter 6byte raw mode */
> +	if (alps_passthrough_mode_v2(psmouse, true))
> +		return -1;
> +
> +	if (ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
> +	    ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
> +	    ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
> +	    ps2_command(&psmouse->ps2dev, &param[0], PSMOUSE_CMD_SETRATE) ||
> +	    ps2_command(&psmouse->ps2dev, &param[1], PSMOUSE_CMD_SETRATE))
> +		return -1;
> +
> +	if (alps_passthrough_mode_v2(psmouse, false))
> +		return -1;
> +
> +	if (alps_absolute_mode_v6(psmouse)) {
> +		psmouse_err(psmouse, "Failed to enable absolute mode\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * Enable or disable passthrough mode to the trackstick.
>   */
> @@ -1553,6 +1743,8 @@ static void alps_set_defaults(struct alps_data *priv)
>  		priv->hw_init = alps_hw_init_v1_v2;
>  		priv->process_packet = alps_process_packet_v1_v2;
>  		priv->set_abs_params = alps_set_abs_params_st;
> +		priv->x_max = 1023;
> +		priv->y_max = 767;
>  		break;
>  	case ALPS_PROTO_V3:
>  		priv->hw_init = alps_hw_init_v3;
> @@ -1584,6 +1776,14 @@ static void alps_set_defaults(struct alps_data *priv)
>  		priv->x_bits = 23;
>  		priv->y_bits = 12;
>  		break;
> +	case ALPS_PROTO_V6:
> +		priv->hw_init = alps_hw_init_v6;
> +		priv->process_packet = alps_process_packet_v6;
> +		priv->set_abs_params = alps_set_abs_params_st;
> +		priv->nibble_commands = alps_v6_nibble_commands;
> +		priv->x_max = 2047;
> +		priv->y_max = 1535;
> +		break;
>  	}
>  }
>  
> @@ -1705,8 +1905,8 @@ static void alps_disconnect(struct psmouse *psmouse)
>  static void alps_set_abs_params_st(struct alps_data *priv,
>  				   struct input_dev *dev1)
>  {
> -	input_set_abs_params(dev1, ABS_X, 0, 1023, 0, 0);
> -	input_set_abs_params(dev1, ABS_Y, 0, 767, 0, 0);
> +	input_set_abs_params(dev1, ABS_X, 0, priv->x_max, 0, 0);
> +	input_set_abs_params(dev1, ABS_Y, 0, priv->y_max, 0, 0);
>  }
>  
>  static void alps_set_abs_params_mt(struct alps_data *priv,
> diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
> index eee5985..704f0f9 100644
> --- a/drivers/input/mouse/alps.h
> +++ b/drivers/input/mouse/alps.h
> @@ -17,6 +17,7 @@
>  #define ALPS_PROTO_V3	3
>  #define ALPS_PROTO_V4	4
>  #define ALPS_PROTO_V5	5
> +#define ALPS_PROTO_V6	6
>  
>  /**
>   * struct alps_model_info - touchpad ID table
> -- 
> 1.8.1.2
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] input: Add support for MMA7455L/MMA7456L 3-Axis Accelerometer
From: Alexander Shiyan @ 2013-11-27  7:22 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, devicetree, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Grant Likely
In-Reply-To: <20131126191618.GC22242@core.coreip.homeip.net>

Hello.

> On Fri, Nov 22, 2013 at 07:53:35PM +0400, Alexander Shiyan wrote:
> > This patch adds support for Freescale MMA7455L/MMA7456L 3-Axis
> > Accelerometer connected to I2C bus. Driver can be loaded ether
> > with or without DT support. The basic parameters of the driver
> > can be changed through sysfs.
> 
> The driver looks sane but I am hesitant with the sysfs interface. For a
> while I asked accelerometer guys to standardize on sysfs attributes
> applicable to all input-related 3-axis accelerometers, but I have not
> seen a concrete proposal thus far.
> 
> If you remove sysfs portions I can merge it as pure input driver now.

Without sysfs we can not tune device parameters, so only my default
values will be used, ​​that may not be suitable for other users.
What are the alternatives? procfs and ioctl. Not sure it's better ...

...
> > diff --git a/drivers/input/misc/mma745xl.c b/drivers/input/misc/mma745xl.c
...
> > +static int mma745xl_probe(struct i2c_client *client,
> > +			  const struct i2c_device_id *id)
> > +{
...
> > +	input_set_drvdata(input, priv);
> > +	dev_set_drvdata(&client->dev, input);
> 
> 	i2c_set_clientdata()

Device also can be used through SPI.
If anyone will make support for this, we can just put the initialization in a
single function, leaving only the regmap initialization related to bus.
Using dev_set_drvdata is more generic here. Not think so?
Thanks.

---

^ permalink raw reply

* Re: [PATCH v2] Input:Add support for DualPoint device on Dell XT2 model
From: Tommy Will @ 2013-11-27  8:19 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Kevin Cernekee, david turvene, linux-input, Niels de Vos,
	Justin Clift, Yunkang Tang
In-Reply-To: <20131127001822.GB15452@core.coreip.homeip.net>

Hi Dmitry,

Thanks for your reply.

>
> It would help greatly if you could document what parameters the new
> initialization routine sets.
>
Ok, please let me explain what I did in initialization.

>+static int alps_hw_init_v6(struct psmouse *psmouse)
>+{
>+     unsigned char param[2] = {0xC8, 0x14};
>+
>+     /* Enter passthrough mode to let trackpoint enter 6byte raw mode */
>+     if (alps_passthrough_mode_v2(psmouse, true))
>+             return -1;

In order to initialize the trackpoint, we must first send command to
enter passthrough mode so that the following
command can directly be sent to trackpoint device.

+
>+     if (ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
>+         ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
>+         ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
>+         ps2_command(&psmouse->ps2dev, &param[0], PSMOUSE_CMD_SETRATE) ||
>+         ps2_command(&psmouse->ps2dev, &param[1], PSMOUSE_CMD_SETRATE))
>+             return -1;
>+

Here are the commands to let trackpoint device enter 6byte raw mode.
Because of the special MPU being
used in this device, trackpoint's packet would always be encapsulated
to the same packet size as Touchpad's.
So, when touchpad is in 6byte mode, we should also let trackpoint in
6byte mode or trackpoint's output data would be messed.
That's what spec requested.

+     if (alps_passthrough_mode_v2(psmouse, false))
+             return -1;
+

Stop talking with trackpoint.

+     if (alps_absolute_mode_v6(psmouse)) {
+             psmouse_err(psmouse, "Failed to enable absolute mode\n");
+             return -1;
+     }
+

Let touchpad enter 6byte raw mode.

+     return 0;
+}

And in alps_absolute_mode_v6() function. Please notice that there are
two kinds of
6byte mode for this device. Let's temporary define them as RawModeA & RawModeB.
Method to enter RawModeA:   Send ALPS magic knock commad [F5 F5 F5
F5]. (Same as protocol v2)
Method to enter RawModeB:   Write 0x181 to 0x000 register. (New)

I checked spec and found RawModeB was suggested and now being used in
Windows driver.
Maybe the reason is it could provide a more accurate data output.
In RawModeA, data output range is X:[0-1023], Y:[0-767], but
in RawModeB, its range is X:[0-2047], Y:[0-1535].

#Probably have any other reason that I did not know, so, I chose a
safety way that was suggested
by the spec although it needs adding much more source code.


> +static int alps_absolute_mode_v6(struct psmouse *psmouse)
> +{
> +     u16 reg_val = 0x181;
> +     int ret = -1;
> +

What we want to do is write 0x181 to 0x000 register. That would let
device enter RawModeB.

> +     /* enter monitor mode, to write the register */
> +     if (alps_monitor_mode(psmouse, true))
> +             return -1;
> +
> +     ret = alps_monitor_mode_write_reg(psmouse, 0x000, reg_val);
> +
> +     if (alps_monitor_mode(psmouse, false))
> +             ret = -1;
> +
> +     return ret;
> +}


>>    3. Add new packet process logic.
>
> I am curious why we need the new packet processing logic. Apparently the
> touchpad can work in the original mode that we already know how to
> parse; we just need to make sure the trackpoint is initialized properly.
> Or yet another protocol flavor is a must?
>
Yes, you're right, as long as initialized trackpoint to 6byte raw mode
and add decode logic into
alps_process_packet_v1_v2(), both touchpad and trackpoint can work correctly.

However, as I wrote in previous, spec suggested me to use another
protocol to co-work with
trackpoint's 6byte mode. So, I just did follow the spec.

BTW, could you please let me know if you have any concern of adding a
new protocol?
In my opinion, assume that there would be no side-effect by using
original touchpad
protocol + new 6byte trackpoint protocol, although I could reuse the
alps_process_packet_v1_v2(),
I had to add a new flag to adjust the original logic to support
trackpoint's 6byte decoding,
which would also make source code harder to understand.

Please kindly share me your opinion of that. Thanks!

Best Regards,
Tommy Will



> Thanks!
>
>>    # Touchpad's dimension is 2047*1535.
>>
>> SelfTest:
>>  1. Move on touchpad -- OK
>>  2. Vertical/Horizontal wheel on touchpad -- OK
>>  3. Tap / Tap and drag on touchpad -- OK
>>  4. Click with touchpad's L/R button -- OK
>>  5. Move on touchpad with pressing touchpad's button -- OK
>>  6. Move on trackpoint -- OK
>>  7. Click with trackpoint's L/R button -- OK
>>  8. Move on trackpoint with pressing trackpoint's button -- OK
>>  9. Move cursor with both touchpad and trackpoint -- OK
>>  10. Move on touchpad with pressing trackpoint's button -- OK
>>  11. Move on trackpoint with pressing touchpad's button -- OK
>>
>>  Thanks
>>
>> Signed-off-by: Yunkang Tang <yunkang.tang@cn.alps.com>
>> ---
>>  drivers/input/mouse/alps.c | 206 ++++++++++++++++++++++++++++++++++++++++++++-
>>  drivers/input/mouse/alps.h |   1 +
>>  2 files changed, 204 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
>> index ca7a26f..5cf62e3 100644
>> --- a/drivers/input/mouse/alps.c
>> +++ b/drivers/input/mouse/alps.c
>> @@ -70,6 +70,25 @@ static const struct alps_nibble_commands alps_v4_nibble_commands[] = {
>>       { PSMOUSE_CMD_SETSCALE11,       0x00 }, /* f */
>>  };
>>
>> +static const struct alps_nibble_commands alps_v6_nibble_commands[] = {
>> +     { PSMOUSE_CMD_ENABLE,           0x00 }, /* 0 */
>> +     { PSMOUSE_CMD_SETRATE,          0x0a }, /* 1 */
>> +     { PSMOUSE_CMD_SETRATE,          0x14 }, /* 2 */
>> +     { PSMOUSE_CMD_SETRATE,          0x28 }, /* 3 */
>> +     { PSMOUSE_CMD_SETRATE,          0x3c }, /* 4 */
>> +     { PSMOUSE_CMD_SETRATE,          0x50 }, /* 5 */
>> +     { PSMOUSE_CMD_SETRATE,          0x64 }, /* 6 */
>> +     { PSMOUSE_CMD_SETRATE,          0xc8 }, /* 7 */
>> +     { PSMOUSE_CMD_GETID,            0x00 }, /* 8 */
>> +     { PSMOUSE_CMD_GETINFO,          0x00 }, /* 9 */
>> +     { PSMOUSE_CMD_SETRES,           0x00 }, /* a */
>> +     { PSMOUSE_CMD_SETRES,           0x01 }, /* b */
>> +     { PSMOUSE_CMD_SETRES,           0x02 }, /* c */
>> +     { PSMOUSE_CMD_SETRES,           0x03 }, /* d */
>> +     { PSMOUSE_CMD_SETSCALE21,       0x00 }, /* e */
>> +     { PSMOUSE_CMD_SETSCALE11,       0x00 }, /* f */
>> +};
>> +
>>
>>  #define ALPS_DUALPOINT               0x02    /* touchpad has trackstick */
>>  #define ALPS_PASS            0x04    /* device has a pass-through port */
>> @@ -103,6 +122,7 @@ static const struct alps_model_info alps_model_data[] = {
>>       /* Dell Latitude E5500, E6400, E6500, Precision M4400 */
>>       { { 0x62, 0x02, 0x14 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf,
>>               ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },
>> +     { { 0x73, 0x00, 0x14 }, 0x00, ALPS_PROTO_V6, 0xff, 0xff, ALPS_DUALPOINT },              /* Dell XT2 */
>>       { { 0x73, 0x02, 0x50 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_FOUR_BUTTONS },           /* Dell Vostro 1400 */
>>       { { 0x52, 0x01, 0x14 }, 0x00, ALPS_PROTO_V2, 0xff, 0xff,
>>               ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },                            /* Toshiba Tecra A11-11L */
>> @@ -645,6 +665,76 @@ static void alps_process_packet_v3(struct psmouse *psmouse)
>>       alps_process_touchpad_packet_v3(psmouse);
>>  }
>>
>> +static void alps_process_packet_v6(struct psmouse *psmouse)
>> +{
>> +     struct alps_data *priv = psmouse->private;
>> +     unsigned char *packet = psmouse->packet;
>> +     struct input_dev *dev = psmouse->dev;
>> +     struct input_dev *dev2 = priv->dev2;
>> +     int x, y, z, left, right, middle;
>> +
>> +     /*
>> +      * We can use Byte5 to distinguish if the packet is from Touchpad
>> +      * or Trackpoint.
>> +      * Touchpad:    0 - 0x7E
>> +      * Trackpoint:  0x7F
>> +      */
>> +     if (packet[5] == 0x7F) {
>> +             /* It should be a DualPoint when received Trackpoint packet */
>> +             if (!(priv->flags & ALPS_DUALPOINT))
>> +                     return;
>> +
>> +             /* Trackpoint packet */
>> +             x = packet[1] | ((packet[3] & 0x20) << 2);
>> +             y = packet[2] | ((packet[3] & 0x40) << 1);
>> +             z = packet[4];
>> +             left = packet[3] & 0x01;
>> +             right = packet[3] & 0x02;
>> +             middle = packet[3] & 0x04;
>> +
>> +             /* To prevent the cursor jump when finger lifted */
>> +             if (x == 0x7F && y == 0x7F && z == 0x7F)
>> +                     x = y = z = 0;
>> +
>> +             /* Divide 4 since trackpoint's speed is too fast */
>> +             input_report_rel(dev2, REL_X, (char)x / 4);
>> +             input_report_rel(dev2, REL_Y, -((char)y / 4));
>> +
>> +             input_report_key(dev2, BTN_LEFT, left);
>> +             input_report_key(dev2, BTN_RIGHT, right);
>> +             input_report_key(dev2, BTN_MIDDLE, middle);
>> +
>> +             input_sync(dev2);
>> +             return;
>> +     }
>> +
>> +     /* Touchpad packet */
>> +     x = packet[1] | ((packet[3] & 0x78) << 4);
>> +     y = packet[2] | ((packet[4] & 0x78) << 4);
>> +     z = packet[5];
>> +     left = packet[3] & 0x01;
>> +     right = packet[3] & 0x02;
>> +
>> +     if (z > 30)
>> +             input_report_key(dev, BTN_TOUCH, 1);
>> +     if (z < 25)
>> +             input_report_key(dev, BTN_TOUCH, 0);
>> +
>> +     if (z > 0) {
>> +             input_report_abs(dev, ABS_X, x);
>> +             input_report_abs(dev, ABS_Y, y);
>> +     }
>> +
>> +     input_report_abs(dev, ABS_PRESSURE, z);
>> +     input_report_key(dev, BTN_TOOL_FINGER, z > 0);
>> +
>> +     /* v6 touchpad does not have middle button */
>> +     input_report_key(dev, BTN_LEFT, left);
>> +     input_report_key(dev, BTN_RIGHT, right);
>> +
>> +     input_sync(dev);
>> +}
>> +
>>  static void alps_process_packet_v4(struct psmouse *psmouse)
>>  {
>>       struct alps_data *priv = psmouse->private;
>> @@ -897,7 +987,7 @@ static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
>>       }
>>
>>       /* Bytes 2 - pktsize should have 0 in the highest bit */
>> -     if (priv->proto_version != ALPS_PROTO_V5 &&
>> +     if ((priv->proto_version < ALPS_PROTO_V5) &&
>>           psmouse->pktcnt >= 2 && psmouse->pktcnt <= psmouse->pktsize &&
>>           (psmouse->packet[psmouse->pktcnt - 1] & 0x80)) {
>>               psmouse_dbg(psmouse, "refusing packet[%i] = %x\n",
>> @@ -1085,6 +1175,80 @@ static int alps_absolute_mode_v1_v2(struct psmouse *psmouse)
>>       return ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETPOLL);
>>  }
>>
>> +static int alps_monitor_mode_send_word(struct psmouse *psmouse, u16 word)
>> +{
>> +     int i, nibble;
>> +
>> +     /*
>> +      * b0-b11 are valid bits, send sequence is inverse.
>> +      * e.g. when word = 0x0123, nibble send sequence is 3, 2, 1
>> +      */
>> +     for (i = 0; i <= 8; i += 4) {
>> +             nibble = (word >> i) & 0xf;
>> +             if (alps_command_mode_send_nibble(psmouse, nibble))
>> +                     return -1;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static int alps_monitor_mode_write_reg(struct psmouse *psmouse,
>> +                                    u16 addr, u16 value)
>> +{
>> +     struct ps2dev *ps2dev = &psmouse->ps2dev;
>> +
>> +     /* 0x0A0 is the command to write the word */
>> +     if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE) ||
>> +         alps_monitor_mode_send_word(psmouse, 0x0A0) ||
>> +         alps_monitor_mode_send_word(psmouse, addr) ||
>> +         alps_monitor_mode_send_word(psmouse, value) ||
>> +         ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE))
>> +             return -1;
>> +
>> +     return 0;
>> +}
>> +
>> +static int alps_monitor_mode(struct psmouse *psmouse, bool enable)
>> +{
>> +     struct ps2dev *ps2dev = &psmouse->ps2dev;
>> +
>> +     if (enable) {
>> +             /* EC E9 F5 F5 E7 E6 E7 E9 to enter monitor mode */
>> +             if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_RESET_WRAP) ||
>> +                 ps2_command(ps2dev, NULL, PSMOUSE_CMD_GETINFO) ||
>> +                 ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
>> +                 ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
>> +                 ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21) ||
>> +                 ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
>> +                 ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21) ||
>> +                 ps2_command(ps2dev, NULL, PSMOUSE_CMD_GETINFO))
>> +                     return -1;
>> +     } else {
>> +             /* EC to exit monitor mode */
>> +             if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_RESET_WRAP))
>> +                     return -1;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static int alps_absolute_mode_v6(struct psmouse *psmouse)
>> +{
>> +     u16 reg_val = 0x181;
>> +     int ret = -1;
>> +
>> +     /* enter monitor mode, to write the register */
>> +     if (alps_monitor_mode(psmouse, true))
>> +             return -1;
>> +
>> +     ret = alps_monitor_mode_write_reg(psmouse, 0x000, reg_val);
>> +
>> +     if (alps_monitor_mode(psmouse, false))
>> +             ret = -1;
>> +
>> +     return ret;
>> +}
>> +
>>  static int alps_get_status(struct psmouse *psmouse, char *param)
>>  {
>>       /* Get status: 0xF5 0xF5 0xF5 0xE9 */
>> @@ -1189,6 +1353,32 @@ static int alps_hw_init_v1_v2(struct psmouse *psmouse)
>>       return 0;
>>  }
>>
>> +static int alps_hw_init_v6(struct psmouse *psmouse)
>> +{
>> +     unsigned char param[2] = {0xC8, 0x14};
>> +
>> +     /* Enter passthrough mode to let trackpoint enter 6byte raw mode */
>> +     if (alps_passthrough_mode_v2(psmouse, true))
>> +             return -1;
>> +
>> +     if (ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
>> +         ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
>> +         ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
>> +         ps2_command(&psmouse->ps2dev, &param[0], PSMOUSE_CMD_SETRATE) ||
>> +         ps2_command(&psmouse->ps2dev, &param[1], PSMOUSE_CMD_SETRATE))
>> +             return -1;
>> +
>> +     if (alps_passthrough_mode_v2(psmouse, false))
>> +             return -1;
>> +
>> +     if (alps_absolute_mode_v6(psmouse)) {
>> +             psmouse_err(psmouse, "Failed to enable absolute mode\n");
>> +             return -1;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>>  /*
>>   * Enable or disable passthrough mode to the trackstick.
>>   */
>> @@ -1553,6 +1743,8 @@ static void alps_set_defaults(struct alps_data *priv)
>>               priv->hw_init = alps_hw_init_v1_v2;
>>               priv->process_packet = alps_process_packet_v1_v2;
>>               priv->set_abs_params = alps_set_abs_params_st;
>> +             priv->x_max = 1023;
>> +             priv->y_max = 767;
>>               break;
>>       case ALPS_PROTO_V3:
>>               priv->hw_init = alps_hw_init_v3;
>> @@ -1584,6 +1776,14 @@ static void alps_set_defaults(struct alps_data *priv)
>>               priv->x_bits = 23;
>>               priv->y_bits = 12;
>>               break;
>> +     case ALPS_PROTO_V6:
>> +             priv->hw_init = alps_hw_init_v6;
>> +             priv->process_packet = alps_process_packet_v6;
>> +             priv->set_abs_params = alps_set_abs_params_st;
>> +             priv->nibble_commands = alps_v6_nibble_commands;
>> +             priv->x_max = 2047;
>> +             priv->y_max = 1535;
>> +             break;
>>       }
>>  }
>>
>> @@ -1705,8 +1905,8 @@ static void alps_disconnect(struct psmouse *psmouse)
>>  static void alps_set_abs_params_st(struct alps_data *priv,
>>                                  struct input_dev *dev1)
>>  {
>> -     input_set_abs_params(dev1, ABS_X, 0, 1023, 0, 0);
>> -     input_set_abs_params(dev1, ABS_Y, 0, 767, 0, 0);
>> +     input_set_abs_params(dev1, ABS_X, 0, priv->x_max, 0, 0);
>> +     input_set_abs_params(dev1, ABS_Y, 0, priv->y_max, 0, 0);
>>  }
>>
>>  static void alps_set_abs_params_mt(struct alps_data *priv,
>> diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
>> index eee5985..704f0f9 100644
>> --- a/drivers/input/mouse/alps.h
>> +++ b/drivers/input/mouse/alps.h
>> @@ -17,6 +17,7 @@
>>  #define ALPS_PROTO_V3        3
>>  #define ALPS_PROTO_V4        4
>>  #define ALPS_PROTO_V5        5
>> +#define ALPS_PROTO_V6        6
>>
>>  /**
>>   * struct alps_model_info - touchpad ID table
>> --
>> 1.8.1.2
>>
>
> --
> Dmitry

^ permalink raw reply

* Re: [PATCH v2] HID: uhid: fix leak for 64/32 UHID_CREATE
From: Jiri Kosina @ 2013-11-27  9:54 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: David Herrmann, linux-input, stable
In-Reply-To: <1385476197.23855.51.camel@deadeye.wl.decadent.org.uk>

On Tue, 26 Nov 2013, Ben Hutchings wrote:

> On Tue, 2013-11-26 at 13:58 +0100, David Herrmann wrote:
> > UHID allows short writes so user-space can omit unused fields. We
> > automatically set them to 0 in the kernel. However, the 64/32 bit
> > compat-handler didn't do that in the UHID_CREATE fallback. This will
> > reveal random kernel heap data (of random size, even) to user-space.
> > 
> > Reported-by: Ben Hutchings <ben@decadent.org.uk>
> > Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> > Cc: stable@vger.kernel.org
> 
> Fixes: befde0226a59 ('HID: uhid: make creating devices work on 64/32 systems')
> 
> (that should make it clear which versions need the fix)

Thanks a lot, applied with the Fixes: annotation added.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH 00/04] input: RMI4 Synaptics RMI4 Touchscreen Driver
From: Benjamin Tissoires @ 2013-11-27 22:20 UTC (permalink / raw)
  To: Christopher Heiny
  Cc: Dmitry Torokhov, Linux Input, Andrew Duggan, Vincent Huang,
	Vivian Ly, Daniel Rosenberg, Jean Delvare, Joerie de Gram,
	Linus Walleij
In-Reply-To: <1384385972-1686-1-git-send-email-cheiny@synaptics.com>

On Wed, Nov 13, 2013 at 6:39 PM, Christopher Heiny <cheiny@synaptics.com> wrote:
> This patchset implements changes to the synaptics-rmi4 branch of
> Dmitry's input tree.  The base for the patchset is Dmitry's commit
> 4a695a01fba9bf467b3b52e124ccee6cef73b323 from 2013-01-31.
>
>
>
> Overall this patchset implements the following changes with respect to
> the Dmitry's 2013-01-31 commit:
>
> * Refactors the transport layer (rmi_i2c) to be named appropriately.
>
> * Eliminates packed struct bitfields, replacing them with masks
> and shifts.  This should make the various register definitions
> endian-independent.
>
> * Removed most or all of the sysfs and debugfs support from the driver core
> and function drivers.  These features are still critical during platform
> development, particularly on embedded systems, so there are hooks that allow
> custom modules that support these control and debug capabilities.  One result
> of this is that several .c files have a corresponding .h file (for example,
> rmi_f01.c has a corresponding rmi_f01.h).  Also, a rmi_control.h file is
> added to provide general definitions for control/debug modules.
>
> * Fixes a number of bugs in the baseline commit.
>
> * Trivial - added an rmi_version.h file, which lets the version be easily
> tweaked using a script.
>
>
> We've broken this patch into 6 parts, as follows:
>     01 - core sensor and bus implementation
>     02 - I2C physical layer driver
>     03..04 - drivers for individual RMI functions
>
>
> Hopefully this is the last time we'll have wide-ranging structural changes in
> the driver code, and future patchsets can be much smaller and confined to
> one or two areas of interest. (yeah, I've said that before...)
>
>
> Comments and other feedback on this driver are welcomed.
>
> Christopher Heiny and the Synaptics RMI4 driver team
>

I have discussed off list with Chris about this patch series. I was
interested in it because it will be a requirement for the touchpad in
the Dell XPS 12 from 2013.
Basically, I told him that the patches are hard to review because of
the huge 1/4.

a v2 should be on its way :)

Cheers,
Benjamin

^ permalink raw reply

* [PATCH 1/2] HID: hid-sensor-hub: Add logical min and max
From: Srinivas Pandruvada @ 2013-11-27 22:19 UTC (permalink / raw)
  To: jkosina, jic23; +Cc: linux-input, linux-iio, Srinivas Pandruvada

Exporting logical minimum and maximum of HID fields as part of the
hid sensor attribute info. This can be used for range checking and
to calculate enumeration base for NAry fields of HID sensor hub.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/hid-sensor-hub.c   | 20 ++++++++------------
 include/linux/hid-sensor-hub.h |  2 ++
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index a184e19..d87f7cb 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -112,13 +112,15 @@ static int sensor_hub_get_physical_device_count(
 
 static void sensor_hub_fill_attr_info(
 		struct hid_sensor_hub_attribute_info *info,
-		s32 index, s32 report_id, s32 units, s32 unit_expo, s32 size)
+		s32 index, s32 report_id, struct hid_field *field)
 {
 	info->index = index;
 	info->report_id = report_id;
-	info->units = units;
-	info->unit_expo = unit_expo;
-	info->size = size/8;
+	info->units = field->unit;
+	info->unit_expo = field->unit_exponent;
+	info->size = (field->report_size * field->report_count)/8;
+	info->logical_minimum = field->logical_minimum;
+	info->logical_maximum = field->logical_maximum;
 }
 
 static struct hid_sensor_hub_callbacks *sensor_hub_get_callback(
@@ -325,9 +327,7 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
 			if (field->physical == usage_id &&
 				field->logical == attr_usage_id) {
 				sensor_hub_fill_attr_info(info, i, report->id,
-					field->unit, field->unit_exponent,
-					field->report_size *
-							field->report_count);
+							  field);
 				ret = 0;
 			} else {
 				for (j = 0; j < field->maxusage; ++j) {
@@ -336,11 +336,7 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
 					field->usage[j].collection_index ==
 					collection_index) {
 						sensor_hub_fill_attr_info(info,
-							i, report->id,
-							field->unit,
-							field->unit_exponent,
-							field->report_size *
-							field->report_count);
+							  i, report->id, field);
 						ret = 0;
 						break;
 					}
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index a265af2..fd66e45 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -40,6 +40,8 @@ struct hid_sensor_hub_attribute_info {
 	s32 units;
 	s32 unit_expo;
 	s32 size;
+	s32 logical_minimum;
+	s32 logical_maximum;
 };
 
 /**
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 2/2] iio: hid-sensors: Fix power and report state
From: Srinivas Pandruvada @ 2013-11-27 22:19 UTC (permalink / raw)
  To: jkosina, jic23; +Cc: linux-input, linux-iio, Srinivas Pandruvada
In-Reply-To: <1385590783-27604-1-git-send-email-srinivas.pandruvada@linux.intel.com>

In the original HID sensor hub firmwares all Named array enums were
to 0-based. But the most recent hub implemented as 1-based,
because of the implementation by one of the major OS vendor.
Using logical minimum for the field as the base of enum. So we add
logical minimum to the selector values before setting those fields.
Some sensor hub FWs already changed logical minimum from 0 to 1
to reflect this and hope every other vendor will follow.
There is no easy way to add a common HID quirk for NAry elements,
even if the standard specifies these field as NAry, the collection
used to describe selectors is still just "logical".

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/iio/common/hid-sensors/Kconfig              |  9 ---------
 drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 20 +++++++++++++++-----
 include/linux/hid-sensor-ids.h                      | 12 ++++++++++++
 3 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/common/hid-sensors/Kconfig b/drivers/iio/common/hid-sensors/Kconfig
index 1178121..39188b7 100644
--- a/drivers/iio/common/hid-sensors/Kconfig
+++ b/drivers/iio/common/hid-sensors/Kconfig
@@ -25,13 +25,4 @@ config HID_SENSOR_IIO_TRIGGER
 	  If this driver is compiled as a module, it will be named
 	  hid-sensor-trigger.
 
-config HID_SENSOR_ENUM_BASE_QUIRKS
-	bool "ENUM base quirks for HID Sensor IIO drivers"
-	depends on HID_SENSOR_IIO_COMMON
-	help
-	  Say yes here to build support for sensor hub FW using
-	  enumeration, which is using 1 as base instead of 0.
-	  Since logical minimum is still set 0 instead of 1,
-	  there is no easy way to differentiate.
-
 endmenu
diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
index b6e77e0..781d3fa 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -33,24 +33,34 @@ static int hid_sensor_data_rdy_trigger_set_state(struct iio_trigger *trig,
 {
 	struct hid_sensor_common *st = iio_trigger_get_drvdata(trig);
 	int state_val;
+	int report_val;
 
 	if (state) {
 		if (sensor_hub_device_open(st->hsdev))
 			return -EIO;
-	} else
+		state_val =
+		HID_USAGE_SENSOR_PROP_POWER_STATE_D0_FULL_POWER_ENUM;
+		report_val =
+		HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EVENTS_ENUM;
+
+	} else {
 		sensor_hub_device_close(st->hsdev);
+		state_val =
+		HID_USAGE_SENSOR_PROP_POWER_STATE_D4_POWER_OFF_ENUM;
+		report_val =
+		HID_USAGE_SENSOR_PROP_REPORTING_STATE_NO_EVENTS_ENUM;
+	}
 
-	state_val = state ? 1 : 0;
-	if (IS_ENABLED(CONFIG_HID_SENSOR_ENUM_BASE_QUIRKS))
-		++state_val;
 	st->data_ready = state;
+	state_val += st->power_state.logical_minimum;
+	report_val += st->report_state.logical_minimum;
 	sensor_hub_set_feature(st->hsdev, st->power_state.report_id,
 					st->power_state.index,
 					(s32)state_val);
 
 	sensor_hub_set_feature(st->hsdev, st->report_state.report_id,
 					st->report_state.index,
-					(s32)state_val);
+					(s32)report_val);
 
 	return 0;
 }
diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
index 4f945d3..8323775 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -117,4 +117,16 @@
 #define HID_USAGE_SENSOR_PROP_REPORT_STATE			0x200316
 #define HID_USAGE_SENSOR_PROY_POWER_STATE			0x200319
 
+/* Power state enumerations */
+#define HID_USAGE_SENSOR_PROP_POWER_STATE_UNDEFINED_ENUM		0x00
+#define HID_USAGE_SENSOR_PROP_POWER_STATE_D0_FULL_POWER_ENUM		0x01
+#define HID_USAGE_SENSOR_PROP_POWER_STATE_D1_LOW_POWER_ENUM		0x02
+#define HID_USAGE_SENSOR_PROP_POWER_STATE_D2_STANDBY_WITH_WAKE_ENUM	0x03
+#define HID_USAGE_SENSOR_PROP_POWER_STATE_D3_SLEEP_WITH_WAKE_ENUM	0x04
+#define HID_USAGE_SENSOR_PROP_POWER_STATE_D4_POWER_OFF_ENUM		0x05
+
+/* Report State enumerations */
+#define HID_USAGE_SENSOR_PROP_REPORTING_STATE_NO_EVENTS_ENUM		0x00
+#define HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EVENTS_ENUM		0x01
+
 #endif
-- 
1.8.3.2


^ permalink raw reply related

* RE: [PATCH 00/04] input: RMI4 Synaptics RMI4 Touchscreen Driver
From: Christopher Heiny @ 2013-11-28  1:39 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Dmitry Torokhov, Linux Input, Andrew Duggan, Vincent Huang,
	Vivian Ly, Daniel Rosenberg, Jean Delvare, Joerie de Gram,
	Linus Walleij
In-Reply-To: <CAN+gG=Ev12P=p-b8O4Zq9ThJx+TuKXb_1NJAkNj6idZmncz4RQ@mail.gmail.com>

[sorry for top posting - I'm out of the office today, and restricted to the feeble webmail interface.]

Benjamin made some excellent points in our off-line conversation.  Please disregard this particular patchset.  We'll work on doing the same thing in a series of smaller, more manageable steps.

Thanks!
Chris

________________________________________
From: Benjamin Tissoires [benjamin.tissoires@gmail.com]
Sent: Wednesday, November 27, 2013 2:20 PM
To: Christopher Heiny
Cc: Dmitry Torokhov; Linux Input; Andrew Duggan; Vincent Huang; Vivian Ly; Daniel Rosenberg; Jean Delvare; Joerie de Gram; Linus Walleij
Subject: Re: [PATCH 00/04] input: RMI4 Synaptics RMI4 Touchscreen Driver

On Wed, Nov 13, 2013 at 6:39 PM, Christopher Heiny <cheiny@synaptics.com> wrote:
> This patchset implements changes to the synaptics-rmi4 branch of
> Dmitry's input tree.  The base for the patchset is Dmitry's commit
> 4a695a01fba9bf467b3b52e124ccee6cef73b323 from 2013-01-31.
>
>
>
> Overall this patchset implements the following changes with respect to
> the Dmitry's 2013-01-31 commit:
>
> * Refactors the transport layer (rmi_i2c) to be named appropriately.
>
> * Eliminates packed struct bitfields, replacing them with masks
> and shifts.  This should make the various register definitions
> endian-independent.
>
> * Removed most or all of the sysfs and debugfs support from the driver core
> and function drivers.  These features are still critical during platform
> development, particularly on embedded systems, so there are hooks that allow
> custom modules that support these control and debug capabilities.  One result
> of this is that several .c files have a corresponding .h file (for example,
> rmi_f01.c has a corresponding rmi_f01.h).  Also, a rmi_control.h file is
> added to provide general definitions for control/debug modules.
>
> * Fixes a number of bugs in the baseline commit.
>
> * Trivial - added an rmi_version.h file, which lets the version be easily
> tweaked using a script.
>
>
> We've broken this patch into 6 parts, as follows:
>     01 - core sensor and bus implementation
>     02 - I2C physical layer driver
>     03..04 - drivers for individual RMI functions
>
>
> Hopefully this is the last time we'll have wide-ranging structural changes in
> the driver code, and future patchsets can be much smaller and confined to
> one or two areas of interest. (yeah, I've said that before...)
>
>
> Comments and other feedback on this driver are welcomed.
>
> Christopher Heiny and the Synaptics RMI4 driver team
>

I have discussed off list with Chris about this patch series. I was
interested in it because it will be a requirement for the touchpad in
the Dell XPS 12 from 2013.
Basically, I told him that the patches are hard to review because of
the huge 1/4.

a v2 should be on its way :)

Cheers,
Benjamin

^ permalink raw reply

* [PATCH 1/3] Input: emu10k1 - use DEFINE_PCI_DEVICE_TABLE macro
From: Jingoo Han @ 2013-11-28  2:19 UTC (permalink / raw)
  To: 'Dmitry Torokhov'
  Cc: 'Dmitry Torokhov', linux-input, 'Jingoo Han'

This macro is used to create a struct pci_device_id array.

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

diff --git a/drivers/input/gameport/emu10k1-gp.c b/drivers/input/gameport/emu10k1-gp.c
index fa7a95c..72a7213 100644
--- a/drivers/input/gameport/emu10k1-gp.c
+++ b/drivers/input/gameport/emu10k1-gp.c
@@ -46,7 +46,7 @@ struct emu {
 	int size;
 };
 
-static const struct pci_device_id emu_tbl[] = {
+static DEFINE_PCI_DEVICE_TABLE(emu_tbl) = {
 
 	{ 0x1102, 0x7002, PCI_ANY_ID, PCI_ANY_ID }, /* SB Live gameport */
 	{ 0x1102, 0x7003, PCI_ANY_ID, PCI_ANY_ID }, /* Audigy gameport */
-- 
1.7.10.4



^ permalink raw reply related

* [PATCH 2/3] Input: fm801-gp - use DEFINE_PCI_DEVICE_TABLE macro
From: Jingoo Han @ 2013-11-28  2:20 UTC (permalink / raw)
  To: 'Dmitry Torokhov'
  Cc: 'Dmitry Torokhov', linux-input, 'Jingoo Han'
In-Reply-To: <001901ceebe0$46c96b10$d45c4130$%han@samsung.com>

This macro is used to create a struct pci_device_id array.

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

diff --git a/drivers/input/gameport/fm801-gp.c b/drivers/input/gameport/fm801-gp.c
index ae912d3..5ace35bf 100644
--- a/drivers/input/gameport/fm801-gp.c
+++ b/drivers/input/gameport/fm801-gp.c
@@ -140,7 +140,7 @@ static void fm801_gp_remove(struct pci_dev *pci)
 	pci_disable_device(pci);
 }
 
-static const struct pci_device_id fm801_gp_id_table[] = {
+static DEFINE_PCI_DEVICE_TABLE(fm801_gp_id_table) = {
 	{ PCI_VENDOR_ID_FORTEMEDIA, PCI_DEVICE_ID_FM801_GP, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0  },
 	{ 0 }
 };
-- 
1.7.10.4



^ permalink raw reply related

* [PATCH 3/3] Input: pcips2 - use DEFINE_PCI_DEVICE_TABLE macro
From: Jingoo Han @ 2013-11-28  2:21 UTC (permalink / raw)
  To: 'Dmitry Torokhov'
  Cc: 'Dmitry Torokhov', linux-input, 'Jingoo Han'
In-Reply-To: <001901ceebe0$46c96b10$d45c4130$%han@samsung.com>

This macro is used to create a struct pci_device_id array.

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

diff --git a/drivers/input/serio/pcips2.c b/drivers/input/serio/pcips2.c
index 13062f6..2fff5d4 100644
--- a/drivers/input/serio/pcips2.c
+++ b/drivers/input/serio/pcips2.c
@@ -186,7 +186,7 @@ static void pcips2_remove(struct pci_dev *dev)
 	pci_disable_device(dev);
 }
 
-static const struct pci_device_id pcips2_ids[] = {
+static DEFINE_PCI_DEVICE_TABLE(pcips2_ids) = {
 	{
 		.vendor		= 0x14f2,	/* MOBILITY */
 		.device		= 0x0123,	/* Keyboard */
-- 
1.7.10.4



^ permalink raw reply related

* Re: [PATCH 3/3] Input: pcips2 - use DEFINE_PCI_DEVICE_TABLE macro
From: Dmitry Torokhov @ 2013-11-28  3:56 UTC (permalink / raw)
  To: Jingoo Han; +Cc: linux-input
In-Reply-To: <001b01ceebe0$82141940$863c4bc0$%han@samsung.com>

On Thu, Nov 28, 2013 at 11:21:12AM +0900, Jingoo Han wrote:
> This macro is used to create a struct pci_device_id array.

Applied all 3, thank you.

> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>  drivers/input/serio/pcips2.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/serio/pcips2.c b/drivers/input/serio/pcips2.c
> index 13062f6..2fff5d4 100644
> --- a/drivers/input/serio/pcips2.c
> +++ b/drivers/input/serio/pcips2.c
> @@ -186,7 +186,7 @@ static void pcips2_remove(struct pci_dev *dev)
>  	pci_disable_device(dev);
>  }
>  
> -static const struct pci_device_id pcips2_ids[] = {
> +static DEFINE_PCI_DEVICE_TABLE(pcips2_ids) = {
>  	{
>  		.vendor		= 0x14f2,	/* MOBILITY */
>  		.device		= 0x0123,	/* Keyboard */
> -- 
> 1.7.10.4
> 
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] drivers: input: touchscreen: sur40: use static variable instead of stack varialbe for 'packet_id'
From: Dmitry Torokhov @ 2013-11-28  4:07 UTC (permalink / raw)
  To: Chen Gang
  Cc: floe, rydberg, David Herrmann, rkuo, linux-kernel@vger.kernel.org,
	linux-input
In-Reply-To: <529555C5.7030404@gmail.com>

Hi Chen,

On Wed, Nov 27, 2013 at 10:15:33AM +0800, Chen Gang wrote:
> 'packet_id' is used for checking sequence whether in order, it need be
> static variable independent from sur40_poll().
> 
> The related warning (with allmodconfig under hexagon):
> 
>   drivers/input/touchscreen/sur40.c: In function 'sur40_poll':
>   drivers/input/touchscreen/sur40.c:297:6: warning: 'packet_id' may be used uninitialized in this function [-Wuninitialized]
> 
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  drivers/input/touchscreen/sur40.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index cfd1b7e..5dfd01a 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -251,7 +251,7 @@ static void sur40_poll(struct input_polled_dev *polldev)
>  	struct sur40_state *sur40 = polldev->private;
>  	struct input_dev *input = polldev->input;
>  	int result, bulk_read, need_blobs, packet_blobs, i;
> -	u32 packet_id;
> +	static u32 packet_id;

It is usually not a good idea to use statics in device drivers as it
does not work well when you have several devices of the same type
present in a system. Also, we process all blobs in one pass so there is
no need to preserve value of packet_id between calls to sur40_poll().

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2] Input: ads7846: Convert to hwmon_device_register_with_groups
From: Dmitry Torokhov @ 2013-11-28  4:08 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: linux-input, lm-sensors, linux-kernel
In-Reply-To: <529548CD.7060906@roeck-us.net>

On Tue, Nov 26, 2013 at 05:20:13PM -0800, Guenter Roeck wrote:
> On 11/26/2013 10:58 AM, Dmitry Torokhov wrote:
> >Hi Guenter,
> >
> >On Mon, Nov 25, 2013 at 08:39:04PM -0800, Guenter Roeck wrote:
> >>Simplify the code and create mandatory 'name' attribute by using
> >>new hwmon API.
> >
> >So this moves hwmon attributes from the parent i2c device to the hwmon
> >device, right? Would not that break userspace which expects to find the
> >attributes where they were?
> >
> 
> In addition to Jean's earlier comments ... s/i2c/spi/, I assume. spi devices
> don't create the mandatory name attribute automatically, which means
> that the created hwmon device was not recognized by standard user space
> applications (eg the sensors command or anything else using libsensors)
> in the first place. Which in turn means that only applications which don't
> support the standard hwmon ABI - if there are any - would be affected.
> What we are more concerned about is to make sure that applications
> which _do_ follow the hwmon ABI are working.

OK, fair enough, I'll apply this then.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] drivers: input: touchscreen: sur40: use static variable instead of stack varialbe for 'packet_id'
From: Chen Gang @ 2013-11-28  4:24 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: floe, rydberg, David Herrmann, rkuo, linux-kernel@vger.kernel.org,
	linux-input
In-Reply-To: <20131128040741.GD15452@core.coreip.homeip.net>

On 11/28/2013 12:07 PM, Dmitry Torokhov wrote:
> Hi Chen,
> 
> On Wed, Nov 27, 2013 at 10:15:33AM +0800, Chen Gang wrote:
>> > 'packet_id' is used for checking sequence whether in order, it need be
>> > static variable independent from sur40_poll().
>> > 
>> > The related warning (with allmodconfig under hexagon):
>> > 
>> >   drivers/input/touchscreen/sur40.c: In function 'sur40_poll':
>> >   drivers/input/touchscreen/sur40.c:297:6: warning: 'packet_id' may be used uninitialized in this function [-Wuninitialized]
>> > 
>> > 
>> > Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>> > ---
>> >  drivers/input/touchscreen/sur40.c |    2 +-
>> >  1 files changed, 1 insertions(+), 1 deletions(-)
>> > 
>> > diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
>> > index cfd1b7e..5dfd01a 100644
>> > --- a/drivers/input/touchscreen/sur40.c
>> > +++ b/drivers/input/touchscreen/sur40.c
>> > @@ -251,7 +251,7 @@ static void sur40_poll(struct input_polled_dev *polldev)
>> >  	struct sur40_state *sur40 = polldev->private;
>> >  	struct input_dev *input = polldev->input;
>> >  	int result, bulk_read, need_blobs, packet_blobs, i;
>> > -	u32 packet_id;
>> > +	static u32 packet_id;
> It is usually not a good idea to use statics in device drivers as it
> does not work well when you have several devices of the same type
> present in a system. Also, we process all blobs in one pass so there is
> no need to preserve value of packet_id between calls to sur40_poll().

OK, thanks, I will/should send patch v2 for it.

-- 
Chen Gang

^ permalink raw reply

* [PATCH v2] drivers: input: touchscreen: sur40: remove stack variable 'packet_id' from sur40_poll()
From: Chen Gang @ 2013-11-28  4:34 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: floe, rydberg, David Herrmann, rkuo, linux-kernel@vger.kernel.org,
	linux-input
In-Reply-To: <20131128040741.GD15452@core.coreip.homeip.net>

The drivers process all blobs in one pass, so there is no need to
preserve value of packet_id between calls to sur40_poll().

And the original implementation may cause 'packet_id' uninitialized,
the related warning (with allmodconfig under hexagon):

  drivers/input/touchscreen/sur40.c: In function 'sur40_poll':
  drivers/input/touchscreen/sur40.c:297:6: warning: 'packet_id' may be used uninitialized in this function [-Wuninitialized]


Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 drivers/input/touchscreen/sur40.c |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index cfd1b7e..2ca32cb 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -251,7 +251,6 @@ static void sur40_poll(struct input_polled_dev *polldev)
 	struct sur40_state *sur40 = polldev->private;
 	struct input_dev *input = polldev->input;
 	int result, bulk_read, need_blobs, packet_blobs, i;
-	u32 packet_id;
 
 	struct sur40_header *header = &sur40->bulk_in_buffer->header;
 	struct sur40_blob *inblob = &sur40->bulk_in_buffer->blobs[0];
@@ -286,17 +285,8 @@ static void sur40_poll(struct input_polled_dev *polldev)
 		if (need_blobs == -1) {
 			need_blobs = le16_to_cpu(header->count);
 			dev_dbg(sur40->dev, "need %d blobs\n", need_blobs);
-			packet_id = header->packet_id;
 		}
 
-		/*
-		 * Sanity check. when video data is also being retrieved, the
-		 * packet ID will usually increase in the middle of a series
-		 * instead of at the end.
-		 */
-		if (packet_id != header->packet_id)
-			dev_warn(sur40->dev, "packet ID mismatch\n");
-
 		packet_blobs = result / sizeof(struct sur40_blob);
 		dev_dbg(sur40->dev, "received %d blobs\n", packet_blobs);
 
-- 
1.7.7.6

^ permalink raw reply related

* Re: [PATCH 3/3] Input: pcips2 - use DEFINE_PCI_DEVICE_TABLE macro
From: Jingoo Han @ 2013-11-28  4:32 UTC (permalink / raw)
  To: 'Dmitry Torokhov'; +Cc: linux-input, 'Jingoo Han'
In-Reply-To: <20131128035637.GA11139@core.coreip.homeip.net>

On Thursday, November 28, 2013 12:57 PM, Dmitry Torokhov wrote:
> On Thu, Nov 28, 2013 at 11:21:12AM +0900, Jingoo Han wrote:
> > This macro is used to create a struct pci_device_id array.
> 
> Applied all 3, thank you.

Please, drop these patches.
According to the Greg Kroah-Hartman, 

"Yeah, and it's a horrid macro that deserves to be removed, please don't
use it in more places.

Actually, if you could just remove it, that would be best, sorry, I'm
not going to take these patches."

So, I will send the patch to remove 'DEFINE_PCI_DEVICE_TABLE' instead.
Sorry for annoying. :-)

Best regards,
Jingoo Han

> 
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> >  drivers/input/serio/pcips2.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/serio/pcips2.c b/drivers/input/serio/pcips2.c
> > index 13062f6..2fff5d4 100644
> > --- a/drivers/input/serio/pcips2.c
> > +++ b/drivers/input/serio/pcips2.c
> > @@ -186,7 +186,7 @@ static void pcips2_remove(struct pci_dev *dev)
> >  	pci_disable_device(dev);
> >  }
> >
> > -static const struct pci_device_id pcips2_ids[] = {
> > +static DEFINE_PCI_DEVICE_TABLE(pcips2_ids) = {
> >  	{
> >  		.vendor		= 0x14f2,	/* MOBILITY */
> >  		.device		= 0x0123,	/* Keyboard */
> > --
> > 1.7.10.4
> >
> >
> 
> --
> Dmitry


^ permalink raw reply

* Re: [PATCH 3/3] Input: pcips2 - use DEFINE_PCI_DEVICE_TABLE macro
From: Dmitry Torokhov @ 2013-11-28  7:35 UTC (permalink / raw)
  To: Jingoo Han; +Cc: linux-input
In-Reply-To: <005001ceebf2$da2da300$8e88e900$%han@samsung.com>

On Thu, Nov 28, 2013 at 01:32:31PM +0900, Jingoo Han wrote:
> On Thursday, November 28, 2013 12:57 PM, Dmitry Torokhov wrote:
> > On Thu, Nov 28, 2013 at 11:21:12AM +0900, Jingoo Han wrote:
> > > This macro is used to create a struct pci_device_id array.
> > 
> > Applied all 3, thank you.
> 
> Please, drop these patches.
> According to the Greg Kroah-Hartman, 
> 
> "Yeah, and it's a horrid macro that deserves to be removed, please don't
> use it in more places.
> 
> Actually, if you could just remove it, that would be best, sorry, I'm
> not going to take these patches."
> 
> So, I will send the patch to remove 'DEFINE_PCI_DEVICE_TABLE' instead.
> Sorry for annoying. :-)

No worries, I dropped them.

Thanks.

-- 
Dmitry

^ 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