Re: [PATCH 1/7] input: alps: Set correct name of psmouse device in alps_init()

Dmitry Torokhov
2014-12-16T05:02:34Z

Hi Pali,

On Fri, Nov 14, 2014 at 08:38:20PM +0100, Pali Rohár wrote:
> On some laptops after starting them from off state (not after reboot), function
> alps_probe_trackstick_v3() (called from function alps_identify()) does not
> detect trackstick. To fix this problem we need to reset device. But function
> alps_identify() is called also from alps_detect() and we do not want to reset
> device in detect function because it will slow down initialization of all other
> non alps devices.
>
> This patch moves code for setting correct device name & protocol from function
> alps_detect() to alps_init() which already doing full device reset.
>
> So this patch removes need to do trackstick detection in alps_detect() function.
>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
> drivers/input/mouse/alps.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 8d85c79..9ffa98d 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2392,6 +2392,10 @@ int alps_init(struct psmouse *psmouse)
> if (input_register_device(priv->dev2))
> goto init_fail;
>
> + if (!(priv->flags & ALPS_DUALPOINT))
> + psmouse->name = "GlidePoint TouchPad";
> + psmouse->model = priv->proto_version;
> +
> psmouse->protocol_handler = alps_process_byte;
> psmouse->poll = alps_poll;
> psmouse->disconnect = alps_disconnect;
> @@ -2422,11 +2426,18 @@ int alps_detect(struct psmouse *psmouse, bool set_properties)
> return -1;
>
> if (set_properties) {
> + /*
> + * NOTE: To detect model and trackstick presence we need to do
> + * full device reset. To speed up detection and prevent
> + * calling duplicate initialization sequence (both in
> + * alps_detect() and alps_init()) we set model/protocol
> + * version and correct name in alps_init() (which will
> + * do full device reset). For now set name to DualPoint.
> + */
> psmouse->vendor = "ALPS";
> - psmouse->name = dummy.flags & ALPS_DUALPOINT ?
> - "DualPoint TouchPad" : "GlidePoint";
> - psmouse->model = dummy.proto_version << 8;
> + psmouse->name = "DualPoint TouchPad";
> }
> +

I do not quite like the way we change the device description back and
forth. Do you think we could allocate the "real" priv structure in
alps_detect() and have alps_init() expect to find it (and free it if
set_properties is false). This way we'd go through initialization once
in detect, it will be authoritative, and we would set the name of the
device properly from the beginning.

Thanks.

--
Dmitry