Re: [PATCH] HID: roccat: Add support for Roccat Kone[+]

Re: [PATCH] HID: roccat: Add support for Roccat Kone[+]

[Jiri Kosina]

On Sun, 24 Oct 2010, Stefan Achatz wrote:

> This patch adds support for Roccat Kone[+] gaming mouse. Kone[+] is an enhanced version
> of the old Kone with more memory for macros, a better sensor and more functionality.
> This driver is conceptual similar to the existing Kone and Pyra drivers.
> Userland tools can soon be found at http://sourceforge.net/projects/roccat

Seems like there is a lot of duplicate code with Roccat Kone.

> +static int koneplus_send_control(struct usb_device *usb_dev, uint value,
> +		enum koneplus_control_requests request)
> +{
> +	int len;
> +	struct koneplus_control *control;
> +
> +	if ((request == KONEPLUS_CONTROL_REQUEST_PROFILE_SETTINGS ||
> +			request == KONEPLUS_CONTROL_REQUEST_PROFILE_BUTTONS) &&
> +			value > 4)
> +		return -EINVAL;
> +
> +	control = kmalloc(sizeof(struct koneplus_control), GFP_KERNEL);
> +	if (!control)
> +		return -ENOMEM;
> +
> +	control->command = KONEPLUS_COMMAND_CONTROL;
> +	control->value = value;
> +	control->request = request;
> +
> +	len = usb_control_msg(usb_dev, usb_sndctrlpipe(usb_dev, 0),
> +			USB_REQ_SET_CONFIGURATION,
> +			USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_OUT,
> +			KONEPLUS_USB_COMMAND_CONTROL, 0, control,
> +			sizeof(struct koneplus_control),
> +			USB_CTRL_SET_TIMEOUT);
> +
> +	kfree(control);
> +
> +	if (len != sizeof(struct koneplus_control))
> +		return len;
> +
> +	return 0;
> +}
> +
> +static int koneplus_receive(struct usb_device *usb_dev, uint usb_command,
> +		void *buf, uint size) {
> +	int len;
> +
> +	len = usb_control_msg(usb_dev, usb_rcvctrlpipe(usb_dev, 0),
> +			USB_REQ_CLEAR_FEATURE,
> +			USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,
> +			usb_command, 0, buf, size, USB_CTRL_SET_TIMEOUT);
> +
> +	return (len != size) ? -EIO : 0;
> +}
> +
> +static int koneplus_receive_control_status(struct usb_device *usb_dev)
> +{
> +	int retval;
> +	struct koneplus_control *control;
> +
> +	control = kmalloc(sizeof(struct koneplus_control), GFP_KERNEL);
> +	if (!control)
> +		return -ENOMEM;
> +
> +	do {
> +		retval = koneplus_receive(usb_dev, KONEPLUS_USB_COMMAND_CONTROL,
> +				control, sizeof(struct koneplus_control));
> +
> +		/* check if we get a completely wrong answer */
> +		if (retval)
> +			goto out;
> +
> +		if (control->value == KONEPLUS_CONTROL_REQUEST_STATUS_OK) {
> +			retval = 0;
> +			goto out;
> +		}
> +
> +		/* indicates that hardware needs some more time to complete action */
> +		if (control->value == KONEPLUS_CONTROL_REQUEST_STATUS_WAIT) {
> +			msleep(500); /* windows driver uses 1000 */
> +			continue;
> +		}
> +
> +		/* seems to be critical - replug necessary */
> +		if (control->value == KONEPLUS_CONTROL_REQUEST_STATUS_OVERLOAD) {
> +			retval = -EINVAL;
> +			goto out;
> +		}
> +
> +		dev_err(&usb_dev->dev, "koneplus_receive_control_status: "
> +				"unknown response value 0x%x\n", control->value);
> +		retval = -EINVAL;
> +		goto out;
> +
> +	} while (1);
> +out:
> +	kfree(control);
> +	return retval;
> +}
> +
> +static int koneplus_send(struct usb_device *usb_dev, uint command,
> +		void *buf, uint size) {
> +	int len;
> +
> +	len = usb_control_msg(usb_dev, usb_sndctrlpipe(usb_dev, 0),
> +			USB_REQ_SET_CONFIGURATION,
> +			USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_OUT,
> +			command, 0, buf, size, USB_CTRL_SET_TIMEOUT);
> +
> +	if (len != size)
> +		return -EIO;
> +
> +	if (koneplus_receive_control_status(usb_dev))
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static int koneplus_select_profile(struct usb_device *usb_dev, uint number,
> +		enum koneplus_control_requests request)
> +{
> +	int retval;
> +
> +	retval = koneplus_send_control(usb_dev, number, request);
> +	if (retval)
> +		return retval;
> +
> +	/* allow time to settle things - windows driver uses 500 */
> +	msleep(100);
> +
> +	retval = koneplus_receive_control_status(usb_dev);
> +	if (retval)
> +		return retval;
> +
> +	return 0;
> +}
> +
> +static int koneplus_get_info(struct usb_device *usb_dev,
> +		struct koneplus_info *buf)
> +{
> +	return koneplus_receive(usb_dev, KONEPLUS_USB_COMMAND_INFO,
> +			buf, sizeof(struct koneplus_info));
> +}
> +
> +static int koneplus_get_profile_settings(struct usb_device *usb_dev,
> +		struct koneplus_profile_settings *buf, uint number)
> +{
> +	int retval;
> +
> +	retval = koneplus_select_profile(usb_dev, number,
> +			KONEPLUS_CONTROL_REQUEST_PROFILE_SETTINGS);
> +	if (retval)
> +		return retval;
> +
> +	return koneplus_receive(usb_dev, KONEPLUS_USB_COMMAND_PROFILE_SETTINGS,
> +			buf, sizeof(struct koneplus_profile_settings));
> +}
> +
> +static int koneplus_set_profile_settings(struct usb_device *usb_dev,
> +		struct koneplus_profile_settings const *settings)
> +{
> +	return koneplus_send(usb_dev, KONEPLUS_USB_COMMAND_PROFILE_SETTINGS,
> +			(void *)settings, sizeof(struct koneplus_profile_settings));
> +}
> +
> +static int koneplus_get_profile_buttons(struct usb_device *usb_dev,
> +		struct koneplus_profile_buttons *buf, int number)
> +{
> +	int retval;
> +
> +	retval = koneplus_select_profile(usb_dev, number,
> +			KONEPLUS_CONTROL_REQUEST_PROFILE_BUTTONS);
> +	if (retval)
> +		return retval;
> +
> +	return koneplus_receive(usb_dev, KONEPLUS_USB_COMMAND_PROFILE_BUTTONS,
> +			buf, sizeof(struct koneplus_profile_buttons));
> +}
> +
> +static int koneplus_set_profile_buttons(struct usb_device *usb_dev,
> +		struct koneplus_profile_buttons const *buttons)
> +{
> +	return koneplus_send(usb_dev, KONEPLUS_USB_COMMAND_PROFILE_BUTTONS,
> +			(void *)buttons, sizeof(struct koneplus_profile_buttons));
> +}
> +
> +/* retval is 0-4 on success, < 0 on error */
> +static int koneplus_get_startup_profile(struct usb_device *usb_dev)
> +{
> +	struct koneplus_startup_profile *buf;
> +	int retval;
> +
> +	buf = kmalloc(sizeof(struct koneplus_startup_profile), GFP_KERNEL);
> +
> +	retval = koneplus_receive(usb_dev, KONEPLUS_USB_COMMAND_STARTUP_PROFILE,
> +			buf, sizeof(struct koneplus_startup_profile));
> +
> +	if (retval)
> +		goto out;
> +
> +	retval = buf->startup_profile;
> +out:
> +	kfree(buf);
> +	return retval;
> +}
> +
> +static int koneplus_set_startup_profile(struct usb_device *usb_dev,
> +		int startup_profile)
> +{
> +	struct koneplus_startup_profile buf;
> +
> +	buf.command = KONEPLUS_COMMAND_STARTUP_PROFILE;
> +	buf.size = sizeof(struct koneplus_startup_profile);
> +	buf.startup_profile = startup_profile;
> +
> +	return koneplus_send(usb_dev, KONEPLUS_USB_COMMAND_STARTUP_PROFILE,
> +			(char *)&buf, sizeof(struct koneplus_profile_buttons));
> +}

Is there any reason this couldn't be merged with the Roccat Kone 
counterparts?

> +static ssize_t koneplus_sysfs_read(struct file *fp, struct kobject *kobj,
> +		char *buf, loff_t off, size_t count,
> +		size_t real_size, uint command)
> +{
> +	struct device *dev = container_of(kobj, struct device, kobj);
> +	struct koneplus_device *koneplus = hid_get_drvdata(dev_get_drvdata(dev));
> +	struct usb_device *usb_dev = interface_to_usbdev(to_usb_interface(dev));
> +	int retval;
> +
> +	if (off != 0 || count != real_size)
> +		return -EINVAL;
> +
> +	mutex_lock(&koneplus->koneplus_lock);
> +	retval = koneplus_receive(usb_dev, command, buf, real_size);
> +	mutex_unlock(&koneplus->koneplus_lock);
> +
> +	if (retval)
> +		return retval;
> +
> +	return real_size;
> +}
> +
> +static ssize_t koneplus_sysfs_write(struct file *fp, struct kobject *kobj,
> +		void const *buf, loff_t off, size_t count,
> +		size_t real_size, uint command)
> +{
> +	struct device *dev = container_of(kobj, struct device, kobj);
> +	struct koneplus_device *koneplus = hid_get_drvdata(dev_get_drvdata(dev));
> +	struct usb_device *usb_dev = interface_to_usbdev(to_usb_interface(dev));
> +	int retval;
> +
> +	if (off != 0 || count != real_size)
> +		return -EINVAL;
> +
> +	mutex_lock(&koneplus->koneplus_lock);
> +	retval = koneplus_send(usb_dev, command, (void *)buf, real_size);
> +	mutex_unlock(&koneplus->koneplus_lock);
> +
> +	if (retval)
> +		return retval;
> +
> +	return real_size;
> +}
> +
> +static ssize_t koneplus_sysfs_write_macro(struct file *fp,
> +		struct kobject *kobj, struct bin_attribute *attr, char *buf,
> +		loff_t off, size_t count)
> +{
> +	return koneplus_sysfs_write(fp, kobj, buf, off, count,
> +			sizeof(struct koneplus_macro), KONEPLUS_USB_COMMAND_MACRO);
> +}
> +
> +static ssize_t koneplus_sysfs_read_sensor(struct file *fp,
> +		struct kobject *kobj, struct bin_attribute *attr, char *buf,
> +		loff_t off, size_t count)
> +{
> +	return koneplus_sysfs_read(fp, kobj, buf, off, count,
> +			sizeof(struct koneplus_sensor), KONEPLUS_USB_COMMAND_SENSOR);
> +}
> +
> +static ssize_t koneplus_sysfs_write_sensor(struct file *fp,
> +		struct kobject *kobj, struct bin_attribute *attr, char *buf,
> +		loff_t off, size_t count)
> +{
> +	return koneplus_sysfs_write(fp, kobj, buf, off, count,
> +			sizeof(struct koneplus_sensor), KONEPLUS_USB_COMMAND_SENSOR);
> +}
> +
> +static ssize_t koneplus_sysfs_write_tcu(struct file *fp,
> +		struct kobject *kobj, struct bin_attribute *attr, char *buf,
> +		loff_t off, size_t count)
> +{
> +	return koneplus_sysfs_write(fp, kobj, buf, off, count,
> +			sizeof(struct koneplus_tcu), KONEPLUS_USB_COMMAND_TCU);
> +}
> +
> +static ssize_t koneplus_sysfs_read_tcu_image(struct file *fp,
> +		struct kobject *kobj, struct bin_attribute *attr, char *buf,
> +		loff_t off, size_t count)
> +{
> +	return koneplus_sysfs_read(fp, kobj, buf, off, count,
> +			sizeof(struct koneplus_tcu_image), KONEPLUS_USB_COMMAND_TCU);
> +}
> +
> +static ssize_t koneplus_sysfs_read_profilex_settings(struct file *fp,
> +		struct kobject *kobj, struct bin_attribute *attr, char *buf,
> +		loff_t off, size_t count, uint number)
> +{
> +	struct device *dev = container_of(kobj, struct device, kobj);
> +	struct koneplus_device *koneplus = hid_get_drvdata(dev_get_drvdata(dev));
> +
> +	if (off >= sizeof(struct koneplus_profile_settings))
> +		return 0;
> +
> +	if (off + count > sizeof(struct koneplus_profile_settings))
> +		count = sizeof(struct koneplus_profile_settings) - off;
> +
> +	mutex_lock(&koneplus->koneplus_lock);
> +	memcpy(buf, ((void const *)&koneplus->profile_settings[number]) + off,
> +			count);
> +	mutex_unlock(&koneplus->koneplus_lock);
> +
> +	return count;
> +}
> +
> +static ssize_t koneplus_sysfs_read_profile1_settings(struct file *fp,
> +		struct kobject *kobj, struct bin_attribute *attr, char *buf,
> +		loff_t off, size_t count)
> +{
> +	return koneplus_sysfs_read_profilex_settings(fp, kobj,
> +			attr, buf, off, count, 0);
> +}
> +
> +static ssize_t koneplus_sysfs_read_profile2_settings(struct file *fp,
> +		struct kobject *kobj, struct bin_attribute *attr, char *buf,
> +		loff_t off, size_t count)
> +{
> +	return koneplus_sysfs_read_profilex_settings(fp, kobj,
> +			attr, buf, off, count, 1);
> +}
> +
> +static ssize_t koneplus_sysfs_read_profile3_settings(struct file *fp,
> +		struct kobject *kobj, struct bin_attribute *attr, char *buf,
> +		loff_t off, size_t count)
> +{
> +	return koneplus_sysfs_read_profilex_settings(fp, kobj,
> +			attr, buf, off, count, 2);
> +}
> +
> +static ssize_t koneplus_sysfs_read_profile4_settings(struct file *fp,
> +		struct kobject *kobj, struct bin_attribute *attr, char *buf,
> +		loff_t off, size_t count)
> +{
> +	return koneplus_sysfs_read_profilex_settings(fp, kobj,
> +			attr, buf, off, count, 3);
> +}
> +
> +static ssize_t koneplus_sysfs_read_profile5_settings(struct file *fp,
> +		struct kobject *kobj, struct bin_attribute *attr, char *buf,
> +		loff_t off, size_t count)
> +{
> +	return koneplus_sysfs_read_profilex_settings(fp, kobj,
> +			attr, buf, off, count, 4);
> +}
> +

This is really ugly. Can't you have just one read() function and 
distuingish according to bin_attribute->private?


> diff --git a/drivers/hid/hid-roccat-koneplus.h b/drivers/hid/hid-roccat-koneplus.h
> new file mode 100644
> index 0000000..bf750fa
> --- /dev/null
> +++ b/drivers/hid/hid-roccat-koneplus.h
> @@ -0,0 +1,229 @@
> +#ifndef __HID_ROCCAT_KONEPLUS_H
> +#define __HID_ROCCAT_KONEPLUS_H
> +
> +/*
> + * Copyright (c) 2010 Stefan Achatz <erazor_de@users.sourceforge.net>
> + */
> +
> +/*
> + * 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.
> + */
> +
> +#include <linux/types.h>
> +
> +#pragma pack(push)
> +#pragma pack(1)

Why?

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH] HID: roccat: Add support for Roccat Kone[+]

[Stefan Achatz]

Hello,

Am Donnerstag, den 04.11.2010, 14:25 -0400 schrieb Jiri Kosina:
> On Sun, 24 Oct 2010, Stefan Achatz wrote:
> 
> > This patch adds support for Roccat Kone[+] gaming mouse. Kone[+] is an enhanced version
> > of the old Kone with more memory for macros, a better sensor and more functionality.
> > This driver is conceptual similar to the existing Kone and Pyra drivers.
> > Userland tools can soon be found at http://sourceforge.net/projects/roccat
> 
> Seems like there is a lot of duplicate code with Roccat Kone.
> Is there any reason this couldn't be merged with the Roccat Kone 
> counterparts?

In fact the Kone[+] seems to be nearer to the Pyra than the old Kone.
Looks like the manufacturer changed the firmware programmer between some
devices. I wanted to wait until I see more devices of this manufacturer
if some kind of genealogy might be visible and remove code duplication
based on that.

> > +static ssize_t koneplus_sysfs_read_profile5_settings(struct file *fp,
> > +		struct kobject *kobj, struct bin_attribute *attr, char *buf,
> > +		loff_t off, size_t count)
> > +{
> > +	return koneplus_sysfs_read_profilex_settings(fp, kobj,
> > +			attr, buf, off, count, 4);
> > +}
> > +
> 
> This is really ugly. Can't you have just one read() function and 
> distuingish according to bin_attribute->private?

The patch comes in a view moments, if you like it I'll do the same for
Kone and Pyra.

> 
> > diff --git a/drivers/hid/hid-roccat-koneplus.h b/drivers/hid/hid-roccat-koneplus.h
> > new file mode 100644
> > index 0000000..bf750fa
> > --- /dev/null
> > +++ b/drivers/hid/hid-roccat-koneplus.h
> > @@ -0,0 +1,229 @@
> > +#ifndef __HID_ROCCAT_KONEPLUS_H
> > +#define __HID_ROCCAT_KONEPLUS_H
> > +
> > +/*
> > + * Copyright (c) 2010 Stefan Achatz <erazor_de@users.sourceforge.net>
> > + */
> > +
> > +/*
> > + * 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.
> > + */
> > +
> > +#include <linux/types.h>
> > +
> > +#pragma pack(push)
> > +#pragma pack(1)
> 
> Why?

What?

Have a nice day,
Stefan

Re: [PATCH] HID: roccat: Add support for Roccat Kone[+]

[Jiri Kosina]

On Fri, 5 Nov 2010, Stefan Achatz wrote:

> > > This patch adds support for Roccat Kone[+] gaming mouse. Kone[+] is an enhanced version
> > > of the old Kone with more memory for macros, a better sensor and more functionality.
> > > This driver is conceptual similar to the existing Kone and Pyra drivers.
> > > Userland tools can soon be found at http://sourceforge.net/projects/roccat
> > 
> > Seems like there is a lot of duplicate code with Roccat Kone.
> > Is there any reason this couldn't be merged with the Roccat Kone 
> > counterparts?
> 
> In fact the Kone[+] seems to be nearer to the Pyra than the old Kone.
> Looks like the manufacturer changed the firmware programmer between some
> devices. I wanted to wait until I see more devices of this manufacturer
> if some kind of genealogy might be visible and remove code duplication
> based on that.

It would be really nice, otherwise this gets into unmaintainable mess very 
quickly.

> > > +static ssize_t koneplus_sysfs_read_profile5_settings(struct file *fp,
> > > +		struct kobject *kobj, struct bin_attribute *attr, char *buf,
> > > +		loff_t off, size_t count)
> > > +{
> > > +	return koneplus_sysfs_read_profilex_settings(fp, kobj,
> > > +			attr, buf, off, count, 4);
> > > +}
> > > +
> > 
> > This is really ugly. Can't you have just one read() function and 
> > distuingish according to bin_attribute->private?
> 
> The patch comes in a view moments, if you like it I'll do the same for
> Kone and Pyra.

Would be nice, thanks for fixing it.

> > > + * Copyright (c) 2010 Stefan Achatz <erazor_de@users.sourceforge.net>
> > > + */
> > > +
> > > +/*
> > > + * 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.
> > > + */
> > > +
> > > +#include <linux/types.h>
> > > +
> > > +#pragma pack(push)
> > > +#pragma pack(1)
> > 
> > Why?
> 
> What?

I would prefer to have comments there, explaining why those pragmas are 
necessary.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

[PATCH 1/3] HID: roccat: Reducing number of functions in kone and pyra drivers

[Stefan Achatz]

The profile number is now passed via bin_attribute->private instead
of function parameter to reduce number of functions.
This patch does for kone and pyra what a previous patch did for
koneplus.

Signed-off-by: Stefan Achatz <erazor_de@users.sourceforge.net>
---
 drivers/hid/hid-roccat-kone.c |  107 ++++++++++---------------------------
 drivers/hid/hid-roccat-pyra.c |  120 +++++++++--------------------------------
 2 files changed, 54 insertions(+), 173 deletions(-)

diff --git a/drivers/hid/hid-roccat-kone.c b/drivers/hid/hid-roccat-kone.c
index f776957..128d1b0 100644
--- a/drivers/hid/hid-roccat-kone.c
+++ b/drivers/hid/hid-roccat-kone.c
@@ -35,6 +35,8 @@
 #include "hid-roccat.h"
 #include "hid-roccat-kone.h"
 
+static uint profile_numbers[5] = {0, 1, 2, 3, 4};
+
 static void kone_set_settings_checksum(struct kone_settings *settings)
 {
 	uint16_t checksum = 0;
@@ -319,9 +321,10 @@ static ssize_t kone_sysfs_write_settings(struct file *fp, struct kobject *kobj,
 	return sizeof(struct kone_settings);
 }
 
-static ssize_t kone_sysfs_read_profilex(struct kobject *kobj,
-		struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count, int number) {
+static ssize_t kone_sysfs_read_profilex(struct file *fp,
+		struct kobject *kobj, struct bin_attribute *attr, char *buf,
+		loff_t off, size_t count)
+{
 	struct device *dev = container_of(kobj, struct device, kobj);
 	struct kone_device *kone = hid_get_drvdata(dev_get_drvdata(dev));
 
@@ -332,46 +335,17 @@ static ssize_t kone_sysfs_read_profilex(struct kobject *kobj,
 		count = sizeof(struct kone_profile) - off;
 
 	mutex_lock(&kone->kone_lock);
-	memcpy(buf, ((char const *)&kone->profiles[number - 1]) + off, count);
+	memcpy(buf, ((char const *)&kone->profiles[*(uint *)(attr->private)]) + off, count);
 	mutex_unlock(&kone->kone_lock);
 
 	return count;
 }
 
-static ssize_t kone_sysfs_read_profile1(struct file *fp, struct kobject *kobj,
-		struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count) {
-	return kone_sysfs_read_profilex(kobj, attr, buf, off, count, 1);
-}
-
-static ssize_t kone_sysfs_read_profile2(struct file *fp, struct kobject *kobj,
-		struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count) {
-	return kone_sysfs_read_profilex(kobj, attr, buf, off, count, 2);
-}
-
-static ssize_t kone_sysfs_read_profile3(struct file *fp, struct kobject *kobj,
-		struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count) {
-	return kone_sysfs_read_profilex(kobj, attr, buf, off, count, 3);
-}
-
-static ssize_t kone_sysfs_read_profile4(struct file *fp, struct kobject *kobj,
-		struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count) {
-	return kone_sysfs_read_profilex(kobj, attr, buf, off, count, 4);
-}
-
-static ssize_t kone_sysfs_read_profile5(struct file *fp, struct kobject *kobj,
-		struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count) {
-	return kone_sysfs_read_profilex(kobj, attr, buf, off, count, 5);
-}
-
 /* Writes data only if different to stored data */
-static ssize_t kone_sysfs_write_profilex(struct kobject *kobj,
-		struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count, int number) {
+static ssize_t kone_sysfs_write_profilex(struct file *fp,
+		struct kobject *kobj, struct bin_attribute *attr, char *buf,
+		loff_t off, size_t count)
+{
 	struct device *dev = container_of(kobj, struct device, kobj);
 	struct kone_device *kone = hid_get_drvdata(dev_get_drvdata(dev));
 	struct usb_device *usb_dev = interface_to_usbdev(to_usb_interface(dev));
@@ -382,13 +356,13 @@ static ssize_t kone_sysfs_write_profilex(struct kobject *kobj,
 	if (off != 0 || count != sizeof(struct kone_profile))
 		return -EINVAL;
 
-	profile = &kone->profiles[number - 1];
+	profile = &kone->profiles[*(uint *)(attr->private)];
 
 	mutex_lock(&kone->kone_lock);
 	difference = memcmp(buf, profile, sizeof(struct kone_profile));
 	if (difference) {
 		retval = kone_set_profile(usb_dev,
-				(struct kone_profile const *)buf, number);
+				(struct kone_profile const *)buf, *(uint *)(attr->private) + 1);
 		if (!retval)
 			memcpy(profile, buf, sizeof(struct kone_profile));
 	}
@@ -400,36 +374,6 @@ static ssize_t kone_sysfs_write_profilex(struct kobject *kobj,
 	return sizeof(struct kone_profile);
 }
 
-static ssize_t kone_sysfs_write_profile1(struct file *fp, struct kobject *kobj,
-		struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count) {
-	return kone_sysfs_write_profilex(kobj, attr, buf, off, count, 1);
-}
-
-static ssize_t kone_sysfs_write_profile2(struct file *fp, struct kobject *kobj,
-		struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count) {
-	return kone_sysfs_write_profilex(kobj, attr, buf, off, count, 2);
-}
-
-static ssize_t kone_sysfs_write_profile3(struct file *fp, struct kobject *kobj,
-		struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count) {
-	return kone_sysfs_write_profilex(kobj, attr, buf, off, count, 3);
-}
-
-static ssize_t kone_sysfs_write_profile4(struct file *fp, struct kobject *kobj,
-		struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count) {
-	return kone_sysfs_write_profilex(kobj, attr, buf, off, count, 4);
-}
-
-static ssize_t kone_sysfs_write_profile5(struct file *fp, struct kobject *kobj,
-		struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count) {
-	return kone_sysfs_write_profilex(kobj, attr, buf, off, count, 5);
-}
-
 static ssize_t kone_sysfs_show_actual_profile(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
@@ -678,36 +622,41 @@ static struct bin_attribute kone_settings_attr = {
 static struct bin_attribute kone_profile1_attr = {
 	.attr = { .name = "profile1", .mode = 0660 },
 	.size = sizeof(struct kone_profile),
-	.read = kone_sysfs_read_profile1,
-	.write = kone_sysfs_write_profile1
+	.read = kone_sysfs_read_profilex,
+	.write = kone_sysfs_write_profilex,
+	.private = &profile_numbers[0]
 };
 
 static struct bin_attribute kone_profile2_attr = {
 	.attr = { .name = "profile2", .mode = 0660 },
 	.size = sizeof(struct kone_profile),
-	.read = kone_sysfs_read_profile2,
-	.write = kone_sysfs_write_profile2
+	.read = kone_sysfs_read_profilex,
+	.write = kone_sysfs_write_profilex,
+	.private = &profile_numbers[1]
 };
 
 static struct bin_attribute kone_profile3_attr = {
 	.attr = { .name = "profile3", .mode = 0660 },
 	.size = sizeof(struct kone_profile),
-	.read = kone_sysfs_read_profile3,
-	.write = kone_sysfs_write_profile3
+	.read = kone_sysfs_read_profilex,
+	.write = kone_sysfs_write_profilex,
+	.private = &profile_numbers[2]
 };
 
 static struct bin_attribute kone_profile4_attr = {
 	.attr = { .name = "profile4", .mode = 0660 },
 	.size = sizeof(struct kone_profile),
-	.read = kone_sysfs_read_profile4,
-	.write = kone_sysfs_write_profile4
+	.read = kone_sysfs_read_profilex,
+	.write = kone_sysfs_write_profilex,
+	.private = &profile_numbers[3]
 };
 
 static struct bin_attribute kone_profile5_attr = {
 	.attr = { .name = "profile5", .mode = 0660 },
 	.size = sizeof(struct kone_profile),
-	.read = kone_sysfs_read_profile5,
-	.write = kone_sysfs_write_profile5
+	.read = kone_sysfs_read_profilex,
+	.write = kone_sysfs_write_profilex,
+	.private = &profile_numbers[4]
 };
 
 static int kone_create_sysfs_attributes(struct usb_interface *intf)
diff --git a/drivers/hid/hid-roccat-pyra.c b/drivers/hid/hid-roccat-pyra.c
index 9bf2304..0dad5c6 100644
--- a/drivers/hid/hid-roccat-pyra.c
+++ b/drivers/hid/hid-roccat-pyra.c
@@ -27,6 +27,8 @@
 #include "hid-roccat.h"
 #include "hid-roccat-pyra.h"
 
+static uint profile_numbers[5] = {0, 1, 2, 3, 4};
+
 static void profile_activated(struct pyra_device *pyra,
 		unsigned int new_profile)
 {
@@ -221,7 +223,7 @@ static int pyra_set_settings(struct usb_device *usb_dev,
 
 static ssize_t pyra_sysfs_read_profilex_settings(struct file *fp,
 		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count, int number)
+		loff_t off, size_t count)
 {
 	struct device *dev = container_of(kobj, struct device, kobj);
 	struct pyra_device *pyra = hid_get_drvdata(dev_get_drvdata(dev));
@@ -233,56 +235,16 @@ static ssize_t pyra_sysfs_read_profilex_settings(struct file *fp,
 		count = sizeof(struct pyra_profile_settings) - off;
 
 	mutex_lock(&pyra->pyra_lock);
-	memcpy(buf, ((char const *)&pyra->profile_settings[number]) + off,
+	memcpy(buf, ((char const *)&pyra->profile_settings[*(uint *)(attr->private)]) + off,
 			count);
 	mutex_unlock(&pyra->pyra_lock);
 
 	return count;
 }
 
-static ssize_t pyra_sysfs_read_profile1_settings(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return pyra_sysfs_read_profilex_settings(fp, kobj,
-			attr, buf, off, count, 0);
-}
-
-static ssize_t pyra_sysfs_read_profile2_settings(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return pyra_sysfs_read_profilex_settings(fp, kobj,
-			attr, buf, off, count, 1);
-}
-
-static ssize_t pyra_sysfs_read_profile3_settings(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return pyra_sysfs_read_profilex_settings(fp, kobj,
-			attr, buf, off, count, 2);
-}
-
-static ssize_t pyra_sysfs_read_profile4_settings(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return pyra_sysfs_read_profilex_settings(fp, kobj,
-			attr, buf, off, count, 3);
-}
-
-static ssize_t pyra_sysfs_read_profile5_settings(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return pyra_sysfs_read_profilex_settings(fp, kobj,
-			attr, buf, off, count, 4);
-}
-
 static ssize_t pyra_sysfs_read_profilex_buttons(struct file *fp,
 		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count, int number)
+		loff_t off, size_t count)
 {
 	struct device *dev = container_of(kobj, struct device, kobj);
 	struct pyra_device *pyra = hid_get_drvdata(dev_get_drvdata(dev));
@@ -294,53 +256,13 @@ static ssize_t pyra_sysfs_read_profilex_buttons(struct file *fp,
 		count = sizeof(struct pyra_profile_buttons) - off;
 
 	mutex_lock(&pyra->pyra_lock);
-	memcpy(buf, ((char const *)&pyra->profile_buttons[number]) + off,
+	memcpy(buf, ((char const *)&pyra->profile_buttons[*(uint *)(attr->private)]) + off,
 			count);
 	mutex_unlock(&pyra->pyra_lock);
 
 	return count;
 }
 
-static ssize_t pyra_sysfs_read_profile1_buttons(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return pyra_sysfs_read_profilex_buttons(fp, kobj,
-			attr, buf, off, count, 0);
-}
-
-static ssize_t pyra_sysfs_read_profile2_buttons(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return pyra_sysfs_read_profilex_buttons(fp, kobj,
-			attr, buf, off, count, 1);
-}
-
-static ssize_t pyra_sysfs_read_profile3_buttons(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return pyra_sysfs_read_profilex_buttons(fp, kobj,
-			attr, buf, off, count, 2);
-}
-
-static ssize_t pyra_sysfs_read_profile4_buttons(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return pyra_sysfs_read_profilex_buttons(fp, kobj,
-			attr, buf, off, count, 3);
-}
-
-static ssize_t pyra_sysfs_read_profile5_buttons(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return pyra_sysfs_read_profilex_buttons(fp, kobj,
-			attr, buf, off, count, 4);
-}
-
 static ssize_t pyra_sysfs_write_profile_settings(struct file *fp,
 		struct kobject *kobj, struct bin_attribute *attr, char *buf,
 		loff_t off, size_t count)
@@ -525,31 +447,36 @@ static struct bin_attribute pyra_profile_settings_attr = {
 static struct bin_attribute pyra_profile1_settings_attr = {
 		.attr = { .name = "profile1_settings", .mode = 0440 },
 		.size = sizeof(struct pyra_profile_settings),
-		.read = pyra_sysfs_read_profile1_settings
+		.read = pyra_sysfs_read_profilex_settings,
+		.private = &profile_numbers[0]
 };
 
 static struct bin_attribute pyra_profile2_settings_attr = {
 		.attr = { .name = "profile2_settings", .mode = 0440 },
 		.size = sizeof(struct pyra_profile_settings),
-		.read = pyra_sysfs_read_profile2_settings
+		.read = pyra_sysfs_read_profilex_settings,
+		.private = &profile_numbers[1]
 };
 
 static struct bin_attribute pyra_profile3_settings_attr = {
 		.attr = { .name = "profile3_settings", .mode = 0440 },
 		.size = sizeof(struct pyra_profile_settings),
-		.read = pyra_sysfs_read_profile3_settings
+		.read = pyra_sysfs_read_profilex_settings,
+		.private = &profile_numbers[2]
 };
 
 static struct bin_attribute pyra_profile4_settings_attr = {
 		.attr = { .name = "profile4_settings", .mode = 0440 },
 		.size = sizeof(struct pyra_profile_settings),
-		.read = pyra_sysfs_read_profile4_settings
+		.read = pyra_sysfs_read_profilex_settings,
+		.private = &profile_numbers[3]
 };
 
 static struct bin_attribute pyra_profile5_settings_attr = {
 		.attr = { .name = "profile5_settings", .mode = 0440 },
 		.size = sizeof(struct pyra_profile_settings),
-		.read = pyra_sysfs_read_profile5_settings
+		.read = pyra_sysfs_read_profilex_settings,
+		.private = &profile_numbers[4]
 };
 
 static struct bin_attribute pyra_profile_buttons_attr = {
@@ -561,31 +488,36 @@ static struct bin_attribute pyra_profile_buttons_attr = {
 static struct bin_attribute pyra_profile1_buttons_attr = {
 		.attr = { .name = "profile1_buttons", .mode = 0440 },
 		.size = sizeof(struct pyra_profile_buttons),
-		.read = pyra_sysfs_read_profile1_buttons
+		.read = pyra_sysfs_read_profilex_buttons,
+		.private = &profile_numbers[0]
 };
 
 static struct bin_attribute pyra_profile2_buttons_attr = {
 		.attr = { .name = "profile2_buttons", .mode = 0440 },
 		.size = sizeof(struct pyra_profile_buttons),
-		.read = pyra_sysfs_read_profile2_buttons
+		.read = pyra_sysfs_read_profilex_buttons,
+		.private = &profile_numbers[1]
 };
 
 static struct bin_attribute pyra_profile3_buttons_attr = {
 		.attr = { .name = "profile3_buttons", .mode = 0440 },
 		.size = sizeof(struct pyra_profile_buttons),
-		.read = pyra_sysfs_read_profile3_buttons
+		.read = pyra_sysfs_read_profilex_buttons,
+		.private = &profile_numbers[2]
 };
 
 static struct bin_attribute pyra_profile4_buttons_attr = {
 		.attr = { .name = "profile4_buttons", .mode = 0440 },
 		.size = sizeof(struct pyra_profile_buttons),
-		.read = pyra_sysfs_read_profile4_buttons
+		.read = pyra_sysfs_read_profilex_buttons,
+		.private = &profile_numbers[3]
 };
 
 static struct bin_attribute pyra_profile5_buttons_attr = {
 		.attr = { .name = "profile5_buttons", .mode = 0440 },
 		.size = sizeof(struct pyra_profile_buttons),
-		.read = pyra_sysfs_read_profile5_buttons
+		.read = pyra_sysfs_read_profilex_buttons,
+		.private = &profile_numbers[4]
 };
 
 static struct bin_attribute pyra_settings_attr = {
-- 
1.7.2.3

[PATCH 2/3] HID: roccat: declaring meaning of pack pragma usage in koneplus driver

[Stefan Achatz]

Using pack pragma to prevent padding bytes in binary data structures
used for hardware communication. Explanation of these pragmas was requested.

Signed-off-by: Stefan Achatz <erazor_de@users.sourceforge.net>
---
 drivers/hid/hid-roccat-koneplus.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-roccat-koneplus.h b/drivers/hid/hid-roccat-koneplus.h
index bf750fa..c25a799 100644
--- a/drivers/hid/hid-roccat-koneplus.h
+++ b/drivers/hid/hid-roccat-koneplus.h
@@ -14,6 +14,9 @@
 
 #include <linux/types.h>
 
+/*
+ * Binary data structures for communication with hardware must have no padding.
+ */
 #pragma pack(push)
 #pragma pack(1)
 
-- 
1.7.2.3

[PATCH 3/3] HID: roccat: declaring meaning of pack pragma usage in kone and pyra driver

[Stefan Achatz]

Using pack pragma to prevent padding bytes in binary data structures
used for hardware communication. Explanation of these pragmas was requested.
This patch does for kone and pyra what another patch does for koneplus.

Signed-off-by: Stefan Achatz <erazor_de@users.sourceforge.net>
---
 drivers/hid/hid-roccat-kone.h |    3 +++
 drivers/hid/hid-roccat-pyra.h |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-roccat-kone.h b/drivers/hid/hid-roccat-kone.h
index 130d656..cb0ed99 100644
--- a/drivers/hid/hid-roccat-kone.h
+++ b/drivers/hid/hid-roccat-kone.h
@@ -14,6 +14,9 @@
 
 #include <linux/types.h>
 
+/*
+ * Binary data structures for communication with hardware must have no padding.
+ */
 #pragma pack(push)
 #pragma pack(1)
 
diff --git a/drivers/hid/hid-roccat-pyra.h b/drivers/hid/hid-roccat-pyra.h
index 22f80a8..31d7b3e 100644
--- a/drivers/hid/hid-roccat-pyra.h
+++ b/drivers/hid/hid-roccat-pyra.h
@@ -14,6 +14,9 @@
 
 #include <linux/types.h>
 
+/*
+ * Binary data structures for communication with hardware must have no padding.
+ */
 #pragma pack(push)
 #pragma pack(1)
 
-- 
1.7.2.3

Re: [PATCH] HID: roccat: Add support for Roccat Kone[+]

[Stefan Achatz]

Am Freitag, den 05.11.2010, 14:01 -0400 schrieb Jiri Kosina:
> On Fri, 5 Nov 2010, Stefan Achatz wrote:
> 
> > > > This patch adds support for Roccat Kone[+] gaming mouse. Kone[+] is an enhanced version
> > > > of the old Kone with more memory for macros, a better sensor and more functionality.
> > > > This driver is conceptual similar to the existing Kone and Pyra drivers.
> > > > Userland tools can soon be found at http://sourceforge.net/projects/roccat
> > > 
> > > Seems like there is a lot of duplicate code with Roccat Kone.
> > > Is there any reason this couldn't be merged with the Roccat Kone 
> > > counterparts?
> > 
> > In fact the Kone[+] seems to be nearer to the Pyra than the old Kone.
> > Looks like the manufacturer changed the firmware programmer between some
> > devices. I wanted to wait until I see more devices of this manufacturer
> > if some kind of genealogy might be visible and remove code duplication
> > based on that.
> 
> It would be really nice, otherwise this gets into unmaintainable mess very 
> quickly.

Just to clarify my words if someones waiting for a patch: That was meant
in a longer term. I'm beginning a new device after I finish Kone[+]
userland tools. That would be at the earliest in january. What I wanted
to say was to please apply these patches in this state, the code
duplication gets handled with the next device.
Thanks,
Stefan

[PATCH] HID: roccat: Using bin-attribute->private to reduce code duplication

[Stefan Achatz]

The profile number is now passed via bin-attribute->private instead
of function parameter to reduce number of functions.

Signed-off-by: Stefan Achatz <erazor_de@users.sourceforge.net>
---
 drivers/hid/hid-roccat-koneplus.c |  120 ++++++++-----------------------------
 1 files changed, 26 insertions(+), 94 deletions(-)

diff --git a/drivers/hid/hid-roccat-koneplus.c b/drivers/hid/hid-roccat-koneplus.c
index 780acb4..7327e1a 100644
--- a/drivers/hid/hid-roccat-koneplus.c
+++ b/drivers/hid/hid-roccat-koneplus.c
@@ -26,6 +26,8 @@
 #include "hid-roccat.h"
 #include "hid-roccat-koneplus.h"
 
+static uint profile_numbers[5] = {0, 1, 2, 3, 4};
+
 static void koneplus_profile_activated(struct koneplus_device *koneplus,
 		uint new_profile)
 {
@@ -328,7 +330,7 @@ static ssize_t koneplus_sysfs_read_tcu_image(struct file *fp,
 
 static ssize_t koneplus_sysfs_read_profilex_settings(struct file *fp,
 		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count, uint number)
+		loff_t off, size_t count)
 {
 	struct device *dev = container_of(kobj, struct device, kobj);
 	struct koneplus_device *koneplus = hid_get_drvdata(dev_get_drvdata(dev));
@@ -340,53 +342,13 @@ static ssize_t koneplus_sysfs_read_profilex_settings(struct file *fp,
 		count = sizeof(struct koneplus_profile_settings) - off;
 
 	mutex_lock(&koneplus->koneplus_lock);
-	memcpy(buf, ((void const *)&koneplus->profile_settings[number]) + off,
+	memcpy(buf, ((void const *)&koneplus->profile_settings[*(uint *)(attr->private)]) + off,
 			count);
 	mutex_unlock(&koneplus->koneplus_lock);
 
 	return count;
 }
 
-static ssize_t koneplus_sysfs_read_profile1_settings(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return koneplus_sysfs_read_profilex_settings(fp, kobj,
-			attr, buf, off, count, 0);
-}
-
-static ssize_t koneplus_sysfs_read_profile2_settings(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return koneplus_sysfs_read_profilex_settings(fp, kobj,
-			attr, buf, off, count, 1);
-}
-
-static ssize_t koneplus_sysfs_read_profile3_settings(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return koneplus_sysfs_read_profilex_settings(fp, kobj,
-			attr, buf, off, count, 2);
-}
-
-static ssize_t koneplus_sysfs_read_profile4_settings(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return koneplus_sysfs_read_profilex_settings(fp, kobj,
-			attr, buf, off, count, 3);
-}
-
-static ssize_t koneplus_sysfs_read_profile5_settings(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return koneplus_sysfs_read_profilex_settings(fp, kobj,
-			attr, buf, off, count, 4);
-}
-
 static ssize_t koneplus_sysfs_write_profile_settings(struct file *fp,
 		struct kobject *kobj, struct bin_attribute *attr, char *buf,
 		loff_t off, size_t count)
@@ -425,7 +387,7 @@ static ssize_t koneplus_sysfs_write_profile_settings(struct file *fp,
 
 static ssize_t koneplus_sysfs_read_profilex_buttons(struct file *fp,
 		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count, uint number)
+		loff_t off, size_t count)
 {
 	struct device *dev = container_of(kobj, struct device, kobj);
 	struct koneplus_device *koneplus = hid_get_drvdata(dev_get_drvdata(dev));
@@ -437,53 +399,13 @@ static ssize_t koneplus_sysfs_read_profilex_buttons(struct file *fp,
 		count = sizeof(struct koneplus_profile_buttons) - off;
 
 	mutex_lock(&koneplus->koneplus_lock);
-	memcpy(buf, ((void const *)&koneplus->profile_buttons[number]) + off,
+	memcpy(buf, ((void const *)&koneplus->profile_buttons[*(uint *)(attr->private)]) + off,
 			count);
 	mutex_unlock(&koneplus->koneplus_lock);
 
 	return count;
 }
 
-static ssize_t koneplus_sysfs_read_profile1_buttons(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return koneplus_sysfs_read_profilex_buttons(fp, kobj,
-			attr, buf, off, count, 0);
-}
-
-static ssize_t koneplus_sysfs_read_profile2_buttons(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return koneplus_sysfs_read_profilex_buttons(fp, kobj,
-			attr, buf, off, count, 1);
-}
-
-static ssize_t koneplus_sysfs_read_profile3_buttons(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return koneplus_sysfs_read_profilex_buttons(fp, kobj,
-			attr, buf, off, count, 2);
-}
-
-static ssize_t koneplus_sysfs_read_profile4_buttons(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return koneplus_sysfs_read_profilex_buttons(fp, kobj,
-			attr, buf, off, count, 3);
-}
-
-static ssize_t koneplus_sysfs_read_profile5_buttons(struct file *fp,
-		struct kobject *kobj, struct bin_attribute *attr, char *buf,
-		loff_t off, size_t count)
-{
-	return koneplus_sysfs_read_profilex_buttons(fp, kobj,
-			attr, buf, off, count, 4);
-}
-
 static ssize_t koneplus_sysfs_write_profile_buttons(struct file *fp,
 		struct kobject *kobj, struct bin_attribute *attr, char *buf,
 		loff_t off, size_t count)
@@ -611,31 +533,36 @@ static struct bin_attribute koneplus_profile_settings_attr = {
 static struct bin_attribute koneplus_profile1_settings_attr = {
 		.attr = { .name = "profile1_settings", .mode = 0440 },
 		.size = sizeof(struct koneplus_profile_settings),
-		.read = koneplus_sysfs_read_profile1_settings
+		.read = koneplus_sysfs_read_profilex_settings,
+		.private = &profile_numbers[0]
 };
 
 static struct bin_attribute koneplus_profile2_settings_attr = {
 		.attr = { .name = "profile2_settings", .mode = 0440 },
 		.size = sizeof(struct koneplus_profile_settings),
-		.read = koneplus_sysfs_read_profile2_settings
+		.read = koneplus_sysfs_read_profilex_settings,
+		.private = &profile_numbers[1]
 };
 
 static struct bin_attribute koneplus_profile3_settings_attr = {
 		.attr = { .name = "profile3_settings", .mode = 0440 },
 		.size = sizeof(struct koneplus_profile_settings),
-		.read = koneplus_sysfs_read_profile3_settings
+		.read = koneplus_sysfs_read_profilex_settings,
+		.private = &profile_numbers[2]
 };
 
 static struct bin_attribute koneplus_profile4_settings_attr = {
 		.attr = { .name = "profile4_settings", .mode = 0440 },
 		.size = sizeof(struct koneplus_profile_settings),
-		.read = koneplus_sysfs_read_profile4_settings
+		.read = koneplus_sysfs_read_profilex_settings,
+		.private = &profile_numbers[3]
 };
 
 static struct bin_attribute koneplus_profile5_settings_attr = {
 		.attr = { .name = "profile5_settings", .mode = 0440 },
 		.size = sizeof(struct koneplus_profile_settings),
-		.read = koneplus_sysfs_read_profile5_settings
+		.read = koneplus_sysfs_read_profilex_settings,
+		.private = &profile_numbers[4]
 };
 
 static struct bin_attribute koneplus_profile_buttons_attr = {
@@ -647,31 +574,36 @@ static struct bin_attribute koneplus_profile_buttons_attr = {
 static struct bin_attribute koneplus_profile1_buttons_attr = {
 		.attr = { .name = "profile1_buttons", .mode = 0440 },
 		.size = sizeof(struct koneplus_profile_buttons),
-		.read = koneplus_sysfs_read_profile1_buttons
+		.read = koneplus_sysfs_read_profilex_buttons,
+		.private = &profile_numbers[0]
 };
 
 static struct bin_attribute koneplus_profile2_buttons_attr = {
 		.attr = { .name = "profile2_buttons", .mode = 0440 },
 		.size = sizeof(struct koneplus_profile_buttons),
-		.read = koneplus_sysfs_read_profile2_buttons
+		.read = koneplus_sysfs_read_profilex_buttons,
+		.private = &profile_numbers[1]
 };
 
 static struct bin_attribute koneplus_profile3_buttons_attr = {
 		.attr = { .name = "profile3_buttons", .mode = 0440 },
 		.size = sizeof(struct koneplus_profile_buttons),
-		.read = koneplus_sysfs_read_profile3_buttons
+		.read = koneplus_sysfs_read_profilex_buttons,
+		.private = &profile_numbers[2]
 };
 
 static struct bin_attribute koneplus_profile4_buttons_attr = {
 		.attr = { .name = "profile4_buttons", .mode = 0440 },
 		.size = sizeof(struct koneplus_profile_buttons),
-		.read = koneplus_sysfs_read_profile4_buttons
+		.read = koneplus_sysfs_read_profilex_buttons,
+		.private = &profile_numbers[3]
 };
 
 static struct bin_attribute koneplus_profile5_buttons_attr = {
 		.attr = { .name = "profile5_buttons", .mode = 0440 },
 		.size = sizeof(struct koneplus_profile_buttons),
-		.read = koneplus_sysfs_read_profile5_buttons
+		.read = koneplus_sysfs_read_profilex_buttons,
+		.private = &profile_numbers[4]
 };
 
 static struct bin_attribute koneplus_macro_attr = {
-- 
1.7.2.3

Re: [PATCH] HID: roccat: Add support for Roccat Kone[+]

[Dmitry Torokhov]

Hi Stefan,

On Sun, Oct 24, 2010 at 07:57:05PM +0200, Stefan Achatz wrote:
> +
> +static int koneplus_create_sysfs_attributes(struct usb_interface *intf)
> +{
> +	int retval;
> +
> +	retval = sysfs_create_group(&intf->dev.kobj, &koneplus_attribute_group);
> +	if (retval)
> +		goto exit_1;
> +
> +	retval = sysfs_create_bin_file(&intf->dev.kobj,
> +			&koneplus_profile_settings_attr);
> +	if (retval)
> +		goto exit_2;
> +
> +	retval = sysfs_create_bin_file(&intf->dev.kobj,
> +			&koneplus_profile1_settings_attr);
> +	if (retval)
> +		goto exit_3;
> +

...

> +
> +	retval = sysfs_create_bin_file(&intf->dev.kobj,
> +			&koneplus_sensor_attr);
> +	if (retval)
> +		goto exit_17;
> +

I think we are ready to sysfs_binary_attribute_group ;) Please add it to
sysfs code and send it to GregKH.

Thanks.

-- 
Dmitry