Re: [RPC] OLPC tablet input driver.

["Zephaniah E. Hull" <warp@aehallh.com>]

[-- Attachment #1: Type: text/plain, Size: 2485 bytes --]

On Tue, Aug 29, 2006 at 01:55:37AM -0700, Komal Shah wrote:
> --- "Zephaniah E. Hull" <warp@aehallh.com> wrote:
> 
> > 
> > 
> > That said, here the patch is for comments.
> > (And possibly for the OLPC kernel tree for others with samples to
> > play
> > with.)
> > 
> > 
> > Signed-off-by: Zephaniah E. Hull <warp@aehallh.com>
> > 
> > diff --git a/drivers/input/mouse/Makefile
> > b/drivers/input/mouse/Makefile
> > index 21a1de6..6218e5a 100644
> > --- a/drivers/input/mouse/Makefile
> > +++ b/drivers/input/mouse/Makefile
> > @@ -14,4 +14,4 @@ obj-$(CONFIG_MOUSE_SERIAL)	+= sermouse.o
> >  obj-$(CONFIG_MOUSE_HIL)		+= hil_ptr.o
> >  obj-$(CONFIG_MOUSE_VSXXXAA)	+= vsxxxaa.o
> >  
> > -psmouse-objs  := psmouse-base.o alps.o logips2pp.o synaptics.o
> > lifebook.o trackpoint.o
> > +psmouse-objs  := psmouse-base.o alps.o logips2pp.o synaptics.o
> > lifebook.o trackpoint.o olpc.o
> 
> Where is KConfigurable entry ?

It is a component of psmouse.o, which is a few lines up.

Breaking out the components of psmouse.o into separate configuration
items might be interesting, but it is quite a bit beyond the scope of
this patch.
> 
> > diff --git a/drivers/input/mouse/olpc.c b/drivers/input/mouse/olpc.c
> > new file mode 100644
> > index 0000000..245f29e
> > --- /dev/null
> > +++ b/drivers/input/mouse/olpc.c
> > @@ -0,0 +1,327 @@
> 
> 
> > +/*
> > + * OLPC touchpad PS/2 mouse driver
> > + *
> > +int olpc_init(struct psmouse *psmouse)
> > +{
> > +	struct olpc_data *priv;
> > +	struct input_dev *dev = psmouse->dev;
> > +	struct input_dev *dev2;
> > +
> > +	psmouse->private = priv = kzalloc(sizeof(struct olpc_data),
> > GFP_KERNEL);
> 
> I think you should assign priv to private only if !NULL.

Fixed.

It should not actually matter, as a failure to get a !NULL value causes
us to return false, which will fall over to other psmouse drivers which
will either set it themselves, or not use it at all, however.

It should be noted that alps.c contains the same issue.

> > +	input_register_device(priv->dev2);
> 
> Please check the return value of input_register_device and its friends.

Alright, added to my todo, should have it done by the next patch
revision.


Thank you very much.
Zephaniah E. Hull.

-- 
	  1024D/E65A7801 Zephaniah E. Hull <warp@aehallh.com>
	   92ED 94E4 B1E6 3624 226D  5727 4453 008B E65A 7801
	    CCs of replies from mailing lists are requested.

"And now, little kittens, we're going to run across red-hot
motherboards, with our bare feet." -- Buzh.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]