From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Dmitry Torokhov" Subject: Re: [PATCH 1/1] gpio_mouse driver Date: Tue, 29 May 2007 11:36:38 -0400 Message-ID: References: <1180444046354-git-send-email-hcegtvedt@norway.atmel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1180444046354-git-send-email-hcegtvedt@norway.atmel.com> Content-Disposition: inline Sender: owner-linux-input@atrey.karlin.mff.cuni.cz List-Help: List-Owner: List-Post: List-Unsubscribe: To: Hans-Christian Egtvedt Cc: linux-input@atrey.karlin.mff.cuni.cz, Hans-Christian Egtvedt List-Id: linux-input@vger.kernel.org Hi, On 5/29/07, Hans-Christian Egtvedt 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. > + > + 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. > + 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. > + > + /* 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. -- Dmitry