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

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

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