Re: [PATCH v6] Cypress PS/2 Trackpad driver
On Mon, Dec 10, 2012 at 06:13:59PM +0100, Henrik Rydberg wrote: > > > > +config MOUSE_PS2_CYPRESS > > > > + bool "Cypress PS/2 mouse protocol extension" if EXPERT > > > > > > Why EXPERT here? > > > > > > > + default y > > > > > > Should it really be default y here? > > > > > > This config entry (with phrases "if EXPERT" and "default y") was simply > > cloned from similar devices (e.g. SYNAPTICS, ALPS, LIFEBOOK). > > > > If your preference is that CYPRESS should be disabled by default, I'll > > omit those phrases. Please advise. > > Dmitry, which default do you prefer here? Given that psmouse is monolithic and can't be [easily] split into separate drivers (because they each protocol has to be actively probed), I believe that allprotocols should be enabled by default. People who really know what they are doing can reduce the size by compiling parts of it out. So copying what other protocols did was right ;) Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6] Cypress PS/2 Trackpad driver
> > > +config MOUSE_PS2_CYPRESS > > > + bool "Cypress PS/2 mouse protocol extension" if EXPERT > > > > Why EXPERT here? > > > > > + default y > > > > Should it really be default y here? > > > This config entry (with phrases "if EXPERT" and "default y") was simply > cloned from similar devices (e.g. SYNAPTICS, ALPS, LIFEBOOK). > > If your preference is that CYPRESS should be disabled by default, I'll > omit those phrases. Please advise. Dmitry, which default do you prefer here? Henrik -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v6] Cypress PS/2 Trackpad driver
Hi Kamal, I think in default branch in cypress_get_finger_count() function return 0 is not suitable, We should return a value bigger than 5 to indicate invalid package to avoid unable to distinguish really 0 contact_cnt packages. 0 contact_cnt packages may still have valid left/right mechanical button events data, so cannot be mixed together. otherwise cypress_validate_byte() function cannot filter out invalid package data. Could you help double check with it, Thanks. > > This driver, submitted on behalf of Cypress Semiconductor Corporation > > and additional contributors, provides support for the Cypress PS/2 Trackpad. > > > > Note that the patch "increase struct ps2dev cmdbuf[] to 8 bytes" [5] > > is a PREREQUISITE for this patch. > > > > This [PATCH v6] version differs from my previous submitted version[4]: > > > > Changes as recommended by Dmitry Torokhov: > > > > - patches #2 (main driver) and #3 (link in driver) have been merged. > > - eliminated cytp_dbg(), is_cypress_key(), and cypress_set_abs_rel_mode(). > > - restructured cypress_validate_byte() and cypress_reset(). > > - stopped unnecessary fiddling with psmouse->pktsize. > > - fixed cypress_read_cmd_status() printf. > > - removed unused members from struct cytp_data. > > - misc code and comment style cleanups. > > > > Known problems: None. > > > > -Kamal Mostafa > > > > [0] PATCH v1: http://www.spinics.net/lists/linux-input/msg23690.html > > [1] PATCH v2: http://www.spinics.net/lists/linux-input/msg23718.html > > [2] PATCH v3: http://www.spinics.net/lists/linux-input/msg23943.html > > [3] PATCH v4: http://www.spinics.net/lists/linux-input/msg24047.html > > [4] PATCH v5: http://www.spinics.net/lists/linux-input/msg24096.html > > [5] cmdbuf patch: > > http://www.spinics.net/lists/linux-input/msg24095.html > > > > -- >8 -- > > From: Dudley Du > > Subject: [PATCH v6] input: Cypress PS/2 Trackpad psmouse driver > > > > Input/mouse driver for Cypress PS/2 Trackpad. > > > > Original code contributed by Dudley Du (Cypress Semiconductor > > Corporation), modified by Kamal Mostafa and Kyle Fazzari. > > > > BugLink: http://launchpad.net/bugs/978807 > > > > Signed-off-by: Dudley Du > > Signed-off-by: Kamal Mostafa > > Signed-off-by: Kyle Fazzari > > Signed-off-by: Mario Limonciello > > Signed-off-by: Tim Gardner > > Acked-by: Herton Krzesinski > > --- > > drivers/input/mouse/Kconfig| 10 + > > drivers/input/mouse/Makefile |1 + > > drivers/input/mouse/cypress_ps2.c | 719 > > > > drivers/input/mouse/cypress_ps2.h | 191 ++ > > drivers/input/mouse/psmouse-base.c | 32 ++ > > drivers/input/mouse/psmouse.h |1 + > > 6 files changed, 954 insertions(+) > > create mode 100644 drivers/input/mouse/cypress_ps2.c create mode > > 100644 drivers/input/mouse/cypress_ps2.h > > > > diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig > > index cd6268c..88954dd 100644 > > --- a/drivers/input/mouse/Kconfig > > +++ b/drivers/input/mouse/Kconfig > > @@ -68,6 +68,16 @@ config MOUSE_PS2_SYNAPTICS > > > > If unsure, say Y. > > > > +config MOUSE_PS2_CYPRESS > > + bool "Cypress PS/2 mouse protocol extension" if EXPERT > > Why EXPERT here? > > > + default y > > Should it really be default y here? > > > + depends on MOUSE_PS2 > > + help > > + Say Y here if you have a Cypress PS/2 Trackpad connected to > > + your system. > > + > > + If unsure, say Y. > > + > > config MOUSE_PS2_LIFEBOOK > > bool "Fujitsu Lifebook PS/2 mouse protocol extension" if EXPERT > > default y > > diff --git a/drivers/input/mouse/Makefile > > b/drivers/input/mouse/Makefile index 46ba755..323e352 100644 > > --- a/drivers/input/mouse/Makefile > > +++ b/drivers/input/mouse/Makefile > > @@ -32,3 +32,4 @@ psmouse-$(CONFIG_MOUSE_PS2_LIFEBOOK) += lifebook.o > > psmouse-$(CONFIG_MOUSE_PS2_SENTELIC) += sentelic.o > > psmouse-$(CONFIG_MOUSE_PS2_TRACKPOINT) += trackpoint.o > > psmouse-$(CONFIG_MOUSE_PS2_TOUCHKIT) += touchkit_ps2.o > > +psmouse-$(CONFIG_MOUSE_PS2_CYPRESS)+= cypress_ps2.o > > diff --git a/drivers/input/mouse/cypress_ps2.c > > b/drivers/input/mouse/cypress_ps2.c > > new file mode 100644 > > index 000..310cb49 > > --- /dev/null > > +++ b/drivers/input/mouse/cypress_ps2.c > > @@ -0,0 +1,719 @@ > > +/* > > + * Cypress Trackpad PS/2 mouse driver > > + * > > + * Copyright (c) 2012 Cypress Semiconductor Corporation. > > + * > > + * Author: > > + * Dudley Du > > + * > > + * Additional contributors include: > > + * Kamal Mostafa > > + * Kyle Fazzari > > + * > > + * This program is free software; you can redistribute it and/or > > +modify it > > + * under the terms of the GNU General Public License version 2 as > > +published by > > + * the Free Software Foundation. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#incl
Re: [PATCH v6] Cypress PS/2 Trackpad driver
Hi Henrik- On Sat, 2012-12-08 at 13:54 +0100, Henrik Rydberg wrote: > [...] > > --- a/drivers/input/mouse/Kconfig > > +++ b/drivers/input/mouse/Kconfig > > @@ -68,6 +68,16 @@ config MOUSE_PS2_SYNAPTICS > > > > If unsure, say Y. > > > > +config MOUSE_PS2_CYPRESS > > + bool "Cypress PS/2 mouse protocol extension" if EXPERT > > Why EXPERT here? > > > + default y > > Should it really be default y here? This config entry (with phrases "if EXPERT" and "default y") was simply cloned from similar devices (e.g. SYNAPTICS, ALPS, LIFEBOOK). If your preference is that CYPRESS should be disabled by default, I'll omit those phrases. Please advise. > > + case 0: return(4); > > No parenthesis on return. I'll fix that. > Reviewed-by: Henrik Rydberg Thanks! -Kamal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6] Cypress PS/2 Trackpad driver
Hi Kamal,
> This driver, submitted on behalf of Cypress Semiconductor Corporation and
> additional contributors, provides support for the Cypress PS/2 Trackpad.
>
> Note that the patch "increase struct ps2dev cmdbuf[] to 8 bytes" [5] is a
> PREREQUISITE for this patch.
>
> This [PATCH v6] version differs from my previous submitted version[4]:
>
> Changes as recommended by Dmitry Torokhov:
>
> - patches #2 (main driver) and #3 (link in driver) have been merged.
> - eliminated cytp_dbg(), is_cypress_key(), and cypress_set_abs_rel_mode().
> - restructured cypress_validate_byte() and cypress_reset().
> - stopped unnecessary fiddling with psmouse->pktsize.
> - fixed cypress_read_cmd_status() printf.
> - removed unused members from struct cytp_data.
> - misc code and comment style cleanups.
>
> Known problems: None.
>
> -Kamal Mostafa
>
> [0] PATCH v1: http://www.spinics.net/lists/linux-input/msg23690.html
> [1] PATCH v2: http://www.spinics.net/lists/linux-input/msg23718.html
> [2] PATCH v3: http://www.spinics.net/lists/linux-input/msg23943.html
> [3] PATCH v4: http://www.spinics.net/lists/linux-input/msg24047.html
> [4] PATCH v5: http://www.spinics.net/lists/linux-input/msg24096.html
> [5] cmdbuf patch: http://www.spinics.net/lists/linux-input/msg24095.html
>
> -- >8 --
> From: Dudley Du
> Subject: [PATCH v6] input: Cypress PS/2 Trackpad psmouse driver
>
> Input/mouse driver for Cypress PS/2 Trackpad.
>
> Original code contributed by Dudley Du (Cypress Semiconductor Corporation),
> modified by Kamal Mostafa and Kyle Fazzari.
>
> BugLink: http://launchpad.net/bugs/978807
>
> Signed-off-by: Dudley Du
> Signed-off-by: Kamal Mostafa
> Signed-off-by: Kyle Fazzari
> Signed-off-by: Mario Limonciello
> Signed-off-by: Tim Gardner
> Acked-by: Herton Krzesinski
> ---
> drivers/input/mouse/Kconfig| 10 +
> drivers/input/mouse/Makefile |1 +
> drivers/input/mouse/cypress_ps2.c | 719
>
> drivers/input/mouse/cypress_ps2.h | 191 ++
> drivers/input/mouse/psmouse-base.c | 32 ++
> drivers/input/mouse/psmouse.h |1 +
> 6 files changed, 954 insertions(+)
> create mode 100644 drivers/input/mouse/cypress_ps2.c
> create mode 100644 drivers/input/mouse/cypress_ps2.h
>
> diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
> index cd6268c..88954dd 100644
> --- a/drivers/input/mouse/Kconfig
> +++ b/drivers/input/mouse/Kconfig
> @@ -68,6 +68,16 @@ config MOUSE_PS2_SYNAPTICS
>
> If unsure, say Y.
>
> +config MOUSE_PS2_CYPRESS
> + bool "Cypress PS/2 mouse protocol extension" if EXPERT
Why EXPERT here?
> + default y
Should it really be default y here?
> + depends on MOUSE_PS2
> + help
> + Say Y here if you have a Cypress PS/2 Trackpad connected to
> + your system.
> +
> + If unsure, say Y.
> +
> config MOUSE_PS2_LIFEBOOK
> bool "Fujitsu Lifebook PS/2 mouse protocol extension" if EXPERT
> default y
> diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
> index 46ba755..323e352 100644
> --- a/drivers/input/mouse/Makefile
> +++ b/drivers/input/mouse/Makefile
> @@ -32,3 +32,4 @@ psmouse-$(CONFIG_MOUSE_PS2_LIFEBOOK)+= lifebook.o
> psmouse-$(CONFIG_MOUSE_PS2_SENTELIC) += sentelic.o
> psmouse-$(CONFIG_MOUSE_PS2_TRACKPOINT) += trackpoint.o
> psmouse-$(CONFIG_MOUSE_PS2_TOUCHKIT) += touchkit_ps2.o
> +psmouse-$(CONFIG_MOUSE_PS2_CYPRESS) += cypress_ps2.o
> diff --git a/drivers/input/mouse/cypress_ps2.c
> b/drivers/input/mouse/cypress_ps2.c
> new file mode 100644
> index 000..310cb49
> --- /dev/null
> +++ b/drivers/input/mouse/cypress_ps2.c
> @@ -0,0 +1,719 @@
> +/*
> + * Cypress Trackpad PS/2 mouse driver
> + *
> + * Copyright (c) 2012 Cypress Semiconductor Corporation.
> + *
> + * Author:
> + * Dudley Du
> + *
> + * Additional contributors include:
> + * Kamal Mostafa
> + * Kyle Fazzari
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> by
> + * the Free Software Foundation.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "cypress_ps2.h"
> +
> +#undef CYTP_DEBUG_VERBOSE /* define this and DEBUG for more verbose dump */
> +
> +static void cypress_set_packet_size(struct psmouse *psmouse, unsigned int n)
> +{
> + struct cytp_data *cytp = psmouse->private;
> + cytp->pkt_size = n;
> +}
> +
> +static const unsigned char cytp_rate[] = {10, 20, 40, 60, 100, 200};
> +static const unsigned char cytp_resolution[] = {0x00, 0x01, 0x02, 0x03};
> +
> +static int cypress_ps2_sendbyte(struct psmouse *psmouse, int value)
> +{
> + struct ps2dev *ps2dev = &psmouse->ps2dev;
> +
> + if (ps2_sendbyte(ps2dev, value & 0xff, CYTP_CMD_TIMEOUT) < 0) {
> + psmouse
