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

[Greg KH]

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@xxxxxxxxxxxxxx> 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.

thanks,

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