lkml.org 
[lkml]   [2009]   [Mar]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: PATCH [1/3] drivers/input/xpad.c: Improve Xbox 360 wireless support and add sysfs interface
    On Mon, Mar 02, 2009 at 01:35:51PM -0800, Andrew Morton wrote:
    > On Mon, 2 Mar 2009 13:18:20 -0800
    > Greg KH <greg@kroah.com> wrote:
    >
    > > On Mon, Mar 02, 2009 at 01:04:25PM -0800, Andrew Morton wrote:
    > > > On Sat, 28 Feb 2009 23:53:03 -0500
    > > > Mike Murphy <mamurph@cs.clemson.edu> wrote:
    > > > > ...
    > > > >
    > > > > +static ssize_t xpad_show_int(struct device *dev, struct device_attribute *attr,
    > > > > + char *buf)
    > > > > +{
    > > > > + struct usb_xpad *xpad = to_xpad(dev);
    > > > > + int value;
    > > > > + if (attr == &dev_attr_rumble_enable)
    > > > > + value = xpad->rumble_enable;
    > > > > + else if (attr == &dev_attr_controller_number)
    > > > > + value = xpad->controller_number;
    > > > > + else if (attr == &dev_attr_controller_present)
    > > > > + value = xpad->controller_present;
    > > > > + else if (attr == &dev_attr_controller_type)
    > > > > + value = xpad->controller_type;
    > > > > + else if (attr == &dev_attr_left_trigger_full_axis)
    > > > > + value = xpad->left_trigger_full_axis;
    > > > > + else if (attr == &dev_attr_right_trigger_full_axis)
    > > > > + value = xpad->right_trigger_full_axis;
    > > > > + else
    > > > > + return -EIO;
    > > > > + return snprintf(buf, PAGE_SIZE, "%d\n", value);
    > > > > +}
    > > >
    > > > The code's a bit unusual. Most drivers don't have all that
    > > > if-then-else stuff. They use a separate function for each sysfs file
    > > > and then they wrap it all up into a macro which emits all the data and
    > > > code for each file.
    > >
    > > Yes, most don't, but that is because we used to not pass the attribute
    > > variable to the show/store functions. It was added so that we could
    > > reduce the huge numbers of functions needed, to do stuff like this. The
    > > hwmon drivers are examples of other drivers that do their show/store
    > > like this.
    > >
    > > There's nothing wrong with it that I can see.
    > >
    >
    > OK. I think.
    >
    > - This approach will require a large number of edits each time an
    > attribute is added/removed.

    That's really not a big dea.

    > - otoh, removing nasty macros is always nice.
    >
    > - This approach will generate slower code (which doesn't matter
    > here).

    True.

    > - Was it demonstrated that this approach generates less code?

    Yes it was. I can go dig through the archives if you want real numbers.

    thanks,

    greg k-h


    \
     
     \ /
      Last update: 2009-03-02 23:05    [W:0.024 / U:29.416 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site