Re: [PATCH 4/25] sony-laptop: new SNC setup and cleanup functions

[Mattia Dongili <malattia@linux.it>]

On Mon, Jul 18, 2011 at 04:49:08PM +0200, Marco Chiappero wrote:
> Il 15/07/2011 00:05, Mattia Dongili ha scritto:
> 
> >>>I recently uploaded the first part (new features excluded) of my v2
> >>>patches at http://www.absence.it/vaio-acpi/source/patches/patchset-v2/
> >>>to help you and receive new feedbacks.
> >>
> >>Still no feedbacks...
> >
> >still not much time. what I noticed so far:
> >- patches are 14 instead of 25, any reason?
> 
> Because writing patches is time consuming. The first 14 patches
> include code cleaning/reorganization and improvements that can be
> merged immediately (if they look fine now), while the new features
> are a bit more problematic. Please, just review these 14 patches at
> the moment.

send the patches to the mailing list please. There are more eyes
watching, they are archived and they can be quoted in emails.

> >- still stopping handlers evaluation if the magi SncSupported string is
> >   not present
> 
> I do believe it's a good thing to have it. Do you prefer to drop the
> while patch #14 or just to change that check?

I thought I made it clear already. Stopping evaluation of handlers will
break things and make the driver less maintainable. If you want to print
a debug message about SncSupported is up to you. I don't mind.

> >- the function naming in patch 5 looks ok.
> 
> Do you wish to include the change from
> 
> 	struct sony_nc_handles {
> 		u16 cap[0x10];
> 		struct device_attribute devattr;
> 	};
> 
> 	static struct sony_nc_handles *handles;
> 
> to
> 
> 	struct sony_nc_handles_list {
> 		u16 cap[0x10];
> 		struct device_attribute devattr;
> 	};
> 
> 	static struct sony_nc_handles_list *handles;
> 
> as well?
> 
> >Once again, post the patches here with proper description, it's easier
> >for everyone to review and comment the changes.
> 
> I'm still waiting for a few replies [1], I'd rather avoid spending

You've got your replies now, for the rest propose your solutions.
-- 
mattia
:wq!