Re: PATCH [1/3] drivers/input/xpad.c: Improve Xbox 360 wireless support and add sysfs interface

[Mike Murphy <mamurph@cs.clemson.edu>]

On Mon, Mar 2, 2009 at 4:35 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
>
> OK.  I think.
>
> - This approach will require a large number of edits each time an
>  attribute is added/removed.
>
> - otoh, removing nasty macros is always nice.
>
> - This approach will generate slower code (which doesn't matter
>  here).
>
> - Was it demonstrated that this approach generates less code?
>

It should generate less code, as compared to having two functions
(show and store) per attribute, in which each function has about 4
lines and a local variable for the store functions (which is there for
type validation). For both show and store functions, there still has
to be code to get to the struct usb_xpad instance.

As I was coding this interface, I found myself wishing for a
macro-based system like module parameters have. For example, it would
be nice to be able to do something like the following:

DECLARE_SYSFS_INTERFACE(&xpad->dead_zone, uint, &check_dead_zone);

which would create the needed show/store functions that worked with an
unsigned int. The check_dead_zone function would be a validator
function inside the module that would check the validity on a
user-supplied value on store (in this case, to be sure it was at least
1024 below the stick limit) and return an error code if the value were
out of range. Yes, the validator would be an extra function call, but
this code doesn't run in an interrupt context, so the benefit of being
able to centralize the functionality and simplify drivers might be
worth the extra overhead.

Mike
-- 
Mike Murphy
Ph.D. Candidate and NSF Graduate Research Fellow
Clemson University School of Computing
120 McAdams Hall
Clemson, SC 29634-0974 USA
Tel: +1 864.656.2838   Fax: +1 864.656.0145
http://cirg.cs.clemson.edu/~mamurph
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html