On Tue, Jan 02, 2007 at 01:06:07PM -0700, Roberson, Jeremy wrote:
> From: Jeremy Roberson <[EMAIL PROTECTED]>
>
> Added a kernel module (gtco) to the USB Input subsystem. This kernel
> module adds support for all GTCO CalComp USB InterWrite School products.
>
> Signed-off-by: Jeremy A. Roberson <[EMAIL PROTECTED]>
>
> ---
> diff -uprN a/drivers/usb/input/gtco.c b/drivers/usb/input/gtco.c
> --- a/drivers/usb/input/gtco.c 1969-12-31 17:00:00.000000000 -0700
> +++ b/drivers/usb/input/gtco.c 2006-10-08 22:09:02.000000000 -0700
Hm, the patch is line-wrapped and can't be applied, care to try it again
with at least the changes mentioned below?
> @@ -0,0 +1,1164 @@
> +/*
> [EMAIL PROTECTED] gtco.c
What is this for?
> +GTCO digitizer USB driver
> +
> +Use the err(), dbg() and info() macros from usb.h for system logging
> +
> +TO CHECK: Is pressure done right on report 5?
> +
> +Copyright (C) 2006 GTCO CalComp
> +
> +This program is free software; you can redistribute it and/or
> +modify it under the terms of the GNU General Public License
> +as published by the Free Software Foundation; either version 2
> +of the License, or (at your option) any later version.
Are you sure of the "any later version" option?
> +#include <linux/version.h>
> +#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,14))
> +/** This define is for kernels 2.6.15 and up where the input driver
> + * interface has changed
> + */
> +#define USE_INPUT_ALLOCATE
> +#include <linux/usb_input.h>
> +#endif
Any version check is not needed when the driver is in the main kernel
tree. Care to remove them?
> +#ifdef USE_INPUT_ALLOCATE
> +#include <linux/usb_input.h>
> +#endif
Is this check really needed?
> +/**
> + In kernel 2.6.15+ the input device must be explicitedly allocated,
> + not part of our own structure. We use this macro to get the pointer
> + to the input device from our structure, which is allocated
> + differently depending on the kernel version
> +*/
> +#ifdef USE_INPUT_ALLOCATE
> +#define GET_INPUT_DEV_PTR(x) (x)->pInputDev
> +#define FREE_INPUT_DEV(x) input_free_device((x)->pInputDev);
> +#else
> +#define GET_INPUT_DEV_PTR(x) &((x)->inputDev)
> +#define FREE_INPUT_DEV(x)
> +#endif
I think these can go away now too, right? Just put the proper logic in
the driver, no macro needed.
> +/* Structure to hold all of our device specific stuff */
> +typedef struct gtco_dev {
Please do not use typedefs. See the Documentation/CodingStyle file for
more details about this, and other coding style issues that need to be
fixed up in this file before it can be accepted (tab indentation issues,
etc.)
> +/**
> + Setup input device capabilities. Tell the input system what this
> + device is capable of generating.
> +
> + This information is based on what is read from the HID report and
> + placed in the GTCO_DEVICE_T structure
> +
> [EMAIL PROTECTED] apInputDev pointer to input device
> +*/
This is not the proper kerneldoc commenting style. @param will probably
break the tools :)
> +static void GTCO_SetupInputCaps(struct input_dev *apInputDev)
function naming issue (as per the CodingStyle file).
> +static void gtcoUrbCallback(struct urb *apUrb, struct pt_regs *apRegs)
Are you sure you built this file on the latest kernel version? PLease
fix up all warnings before resubmitting it.
> + //
> + // Good URB, now process
> + //
// comments are frowned apon :(
> +#ifdef USE_BUTTONS
What sets this value? Why have it at all?
> + for ( i=0;i<5;i++){
> + input_report_key(pInput, BTN_DIGI+i,val&(1<<i));
> + }
Braces not needed for one line for statements.
> + // Allocate memory for device structure
> + pDevice = kmalloc(sizeof(GTCO_DEVICE_T), GFP_KERNEL);
> + if (pDevice == NULL) {
> + err("No more memory");
> + return -ENOMEM;
> + }
> + memset(pDevice, 0x00, sizeof (GTCO_DEVICE_T));
kzalloc() can be used instead of kmalloc + memset.
> +//MODULE_LICENSE("Proprietary");
Why did you forget to remove this commented out line? :)
thanks,
greg k-h
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
