Re: [PATCH 10/25] sony-laptop: keyboard backlight support extended to newer Vaios

[Marco Chiappero <marco@absence.it>]

Il 13/06/2011 00:24, Mattia Dongili ha scritto:
> On Fri, Jun 10, 2011 at 02:31:50PM +0200, Marco Chiappero wrote:
>> Il 04/06/2011 09:58, Mattia Dongili ha scritto:
>>
>>>>   struct kbd_backlight {
>>>> -	int mode;
>>>> -	int timeout;
>>>> +	unsigned int base;
>>>> +	unsigned int mode;
>>>> +	unsigned int timeout;
>>>>   	struct device_attribute mode_attr;
>>>>   	struct device_attribute timeout_attr;
>>>>   };
>>>> -
>>>>   static struct kbd_backlight *kbdbl_handle;
>>>> +static int sony_kbd_handle = -1;
>>>
>>> there seems to be no real point initializing this to -1. Also, can it be
>>> made part of the struct above?
>>
>> I'm including these two changes in every patch that provides a new
>> capability using different handles.
>> I need some more time to prepare the new patches, but before
>> resending I'd like to hear some more feedbacks: removing any acpi
>> notification in patch #8, do patches from 1 to 9 look fine? Is it
>> possible to merge them, as they are, as soon as I repost them? If
>
> potentially, if they seem safe yes. I'd rather review what you have than
> a stale version that you're already changing so please send the patches
> over and then we'll see.

Ok, to avoid multiple resends, does this new naming schema look fine to you?

/* Keyboard backlight feature */
static struct sony_kbdbl_data {
	unsigned int handle;
	unsigned int base;
	unsigned int mode;
	unsigned int timeout;
	struct device_attribute mode_attr;
	struct device_attribute timeout_attr;
} *sony_kbdbl;


Another example:

static struct sony_battcare_data {
	unsigned int handle;
	struct device_attribute attrs[2];
} *sony_battcare;


I think that we might use the sony_FEATURE_device namining when we have 
multiple data regarding a physical device rather than just a collection 
of data related to a feature, so sony_kbdbl_data could be named 
sony_kbdbl_device. If you don't like the "sony" prefix, which is not 
much informative, we can change to "snc", which specifies that it's a 
SNC related feature, even though it's already clear enough.
Sorry but I don't really like the current naming schema (using 
kbdbl_handle->handle or kbdbl_handle->snc_handle names doesn't look good 
to me); I'd like to hear comments on this, please.