Re: [PATCH 1/1] gpio_mouse driver

[Hans-Christian Egtvedt <hcegtvedt@norway.atmel.com>]

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

On Tue, 2007-05-29 at 11:36 -0400, Dmitry Torokhov wrote:
> Hi,
> 
> On 5/29/07, Hans-Christian Egtvedt <hcegtvedt@norway.atmel.com> wrote:
> > This patch adds support for simulating a mouse using GPIO lines.
> >
> > The driver needs a platform_data struct to be defined and registered with the
> > appropriate platform_device.
> >
> > The driver has been tested on AT32AP7000 microprocessor using the ATSTK1000
> > development board.
> >
> 
> It looks sane although I would recommend switching to input-polldev
> when implementing a polled input device.

Oh, I was not aware about this, it seems like just the thing I need.

Could it be scheduled for after the official kernel has this included?
AFAICT it will be released with 2.6.22?

I have attached a new patch with some minor modifications.

> > +
> > +       input->name = pdev->name;
> > +       input->cdev.dev = &pdev->dev;
> 
> Please use input->dev.parent = &pdev->dev. Input devices are being
> moved from class_device to struct device.

This is not present in 2.6.21.1, could I schedule it for the next update
of the driver as above?

> > +       input->private = pdata;
> > +
> > +       /*
> > +        * Revisit: is bustype, vendor, product and version needed to
> > +        * input->id? And if they should be present, what values should they
> > +        * have?
> > +        */
> 
> BUS_HOST seems to be most suitable here. The rest may stay 0.

Thanks, applied.

> > +
> > +       /* private */
> > +       struct timer_list timer;
> > +};
> 
> I don't think it is a good idea to have timer structure in platform
> data which should really be constant. Timer shoudl be part of the
> stucture created when driver binds to a device. I can see you may not
> want to introduce extra complexity in the driver; however if you use
> input-polldev it will handle timer for you.

I was a bit unsure about this before submitting. I have moved the
timer_list into a struct in the driver and added the usual
kmalloc/kfree.

-- 
Best regards
Hans-Christian Egtvedt

[-- Attachment #2: 0001-gpio_mouse-driver.txt --]
[-- Type: application/mbox, Size: 11812 bytes --]