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

[Pali RohÃr]

On Tuesday 16 December 2014 06:02:34 Dmitry Torokhov wrote:
> 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@xxxxxxxxx>
> > ---
> > 
> >  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.

No without introducing another psmouse_reset call. I want to 
reduce time of loading driver, so I think this is better.

-- 
Pali RohÃr
pali.rohar@xxxxxxxxx

Attachment: signature.asc
Description: This is a digitally signed message part.