From: Dmitry Torokhov <dtor_core@ameritech.net>
To: Vojtech Pavlik <vojtech@suse.cz>
Subject: Re: [PATCH 2.6.14.2] Updated itmtouch kernel usb input driver (1/1)
Date: Wed, 23 Nov 2005 21:01:33 -0500
User-Agent: KMail/1.8.3
Cc: Hans-Christian Egtvedt <hc@mivu.no>, LKML <linux-kernel@vger.kernel.org>,
       linux-input@atrey.karlin.mff.cuni.cz
References: <1132764764.6394.14.camel@charlie.egtvedt.no> <20051123165813.GA3201@ucw.cz>
In-Reply-To: <20051123165813.GA3201@ucw.cz>
MIME-Version: 1.0
Content-Type: text/plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 8BIT
Content-Disposition: inline
Message-Id: <200511232101.33783.dtor_core@ameritech.net>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1391
Lines: 43

On Wednesday 23 November 2005 11:58, Vojtech Pavlik wrote:
> > ?static int itmtouch_open(struct input_dev *input)
> > ?{
> > ??????struct itmtouch_dev *itmtouch = input->private;
> > ?
> > +?????if (itmtouch->users++)
> > +?????????????return 0;
> > +

Why are you adding this? input_open/close are serialized and called
only once when needed.

> > ??????itmtouch->readurb->dev = itmtouch->usbdev;
> > ?
> > ??????if (usb_submit_urb(itmtouch->readurb, GFP_KERNEL))
> > +?????{
> > +?????????????itmtouch->users--;
> > ??????????????return -EIO;
> > +?????}
> > ?

Brace should go on the same line with "if".

> > -?????usb_to_input_id(udev, &itmtouch->inputdev.id);
> > +?????itmtouch->inputdev.id.bustype = BUS_USB;
> > +?????itmtouch->inputdev.id.vendor = udev->descriptor.idVendor;
> > +?????itmtouch->inputdev.id.product = udev->descriptor.idProduct;
> > +?????itmtouch->inputdev.id.version = udev->descriptor.bcdDevice;
> > ??????itmtouch->inputdev.dev = &intf->dev;

Why are you replacing perfectly good code with incorrect one (endianess
issues)?

Plus you need to convert it to dynamic input_dev allocation for newer
kernels. 

-- 
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/