Re: [PATCH] acerhdf: convert to dev_pm_ops

[Peter Feuerer <peter@piie.net>]

Hi,

Borislav Petkov writes:

> Hi,
> 
> On Tue, Jul 28, 2009 at 9:25 AM, Peter Feuerer<peter@piie.net> wrote:
> 
> [..]
> 
>>>> Hmm, looking at the driver I think the only function that actually
>>>> is needed is poweroff() that would turn the fan in automatic mode
>>>> before shutting down. The driver does not perform any actions when
>>>> resuming so why bother?
>>>
>>> Agreed.
>>>
>>> Also, the fan comes out of warm and cold reboot in mode AUTO and
>>> when the driver is enabled, the fan is turned off on the next run of
>>> thermal_zone_device_update() when the read out temperature is within
>>> limits.
>>>
>>> Correct me if I'm wrong, but the only reason I see for setting the
>>> fan to mode AUTO before suspending/hibernating/etc is if it is taking
>>> a really long time to hibernate and write RAM image to disk and the
>>> machine is getting hot during that process. Otherwise, we might just
>>> as well do _nothing_ when suspending and remove all suspend/resume
>>> functionality altogether, no?
>>
>> This is right, currently the only reason for calling the suspend /
>> hibernate functions is to set the fan to auto to ensure it doesn't get
>> too hot while the kernel prepares the machine to suspend / hibernate.
>>
>> I would like to keep the resume function too. It's a nice to see
>> what's happening with verbose=1.
> 
> That's not a reason for keeping code in the kernel and raising bloat
> levels unnecessarily. If the driver doesn't need to do anything on
> resume, then no function is needed.

I don't think the verbose message is useless. If an user has a problem with 
suspend / hibernate I can just ask him to load the module with verbose=1 and 
dmesg tells whether the module is waking up or not. Searching for such an 
error is much easier this way as the user doesn't have to recompile the 
kernelmodule. In my opinion keeping these 4 lines of code is worth it.

kind regards,
--peter