Re: Re: [PATCH 35/36] hid: roccat: convert class code to use bin_attrs in groups

On Tue, Aug 20, 2013 at 07:28:46AM +0200, screamingfist@arcor.de wrote:

----- Original Nachricht ----
Von:     Greg Kroah-Hartman [off-list ref]
An:      Stefan Achatz [off-list ref]
Datum:   15.08.2013 18:40
Betreff: Re: [PATCH 35/36] hid: roccat: convert class code to use bin_attrs
 in groups

quoted

On Thu, Aug 15, 2013 at 06:03:46PM +0200, Stefan Achatz wrote:

quoted

Hello Greg,
looking through the patch I found the following:



In hid-roccat-kone.c, PROFILE_ATTR macro:
The line
 .private = &profile_numbers[number],
should read
 .private = &profile_numbers[number-1],

Ah, ok, thanks, will fix up.

quoted

Regarding the compiler warning about unused pyra_sysfs_write_settings:
The attribute is indeed readwrite, but the code to write is not standard
code. The read function was created with the PYRA_SYSFS_R macro and the
write function was written manually. Used were both by
PYRA_BIN_ATTRIBUTE_RW.
In this case I would keep the PYRA_SYSFS_* macros standalone so that the
calling code looks like this:

PYRA_SYSFS_W(control, CONTROL);
PYRA_SYSFS_RW(info, INFO);
PYRA_SYSFS_RW(profile_settings, PROFILE_SETTINGS);
PYRA_SYSFS_RW(profile_buttons, PROFILE_BUTTONS);
PYRA_SYSFS_R(settings, SETTINGS);

PYRA_BIN_ATTRIBUTE_W(control, CONTROL);
PYRA_BIN_ATTRIBUTE_RW(info, INFO);
PYRA_BIN_ATTRIBUTE_RW(profile_settings, PROFILE_SETTINGS);
PYRA_BIN_ATTRIBUTE_RW(profile_buttons, PROFILE_BUTTONS);
PYRA_BIN_ATTRIBUTE_RW(settings, SETTINGS);

Hm, I'll look this over again and see if I can make it better.

But why are you using a "non standard" write function?  What is special
here about this?

The module stores the written value, so that events from the device can be
written to chardev with this extra information.
I made the mistake to put too many functionality in the early kernel modules
and I'm not getting fully rid of it now.

Ok, I've fixed this up now, so it all good, thanks for reviewing this.

greg k-h