Re: [PATCH] power management: no valid states w/o pm_ops + docs

[Randy Dunlap <randy.dunlap@oracle.com>]

Johannes Berg wrote:
> On Tue, 2007-02-13 at 09:23 -0800, Randy Dunlap wrote:
>> On Tue, 13 Feb 2007 17:26:20 +0100 Johannes Berg wrote:
>>
>>> This patch changes /sys/power/state to not advertise any valid states
>>> (except for disk if SOFTWARE_SUSPEND is enabled) when no pm_ops have been
>>> set so userspace can easily discover what states should be available.
>>>
>>> Also, because the pm ops in powermac are obviously not using them as
>>> intended, I added documentation for it in kernel-doc format.
>> Thanks... but it's not quite in kernel-doc format.
> 
> Hmm ok :)
> 
>> Did you test it?
> 
> Yes! The output (in html) looked fine to me.

Great.  Some people don't even test.  :(

>>> +/**
>>> + * struct pm_ops
>>> + *
>>> + * Callbacks for managing platform dependent suspend states.
>>> + *
>>  * struct pm_ops - callbacks for managing platform-dependent suspend states
> 
> But then the description "callbacks for ..." disappears completely in
> the HTML output! Should I duplicate it or something?

No, it's right there by the struct name (top line).
As you have it, it shows up at the bottom, after the struct members,
doesn't it (i.e., out of order)?

>>> + * @valid: Callback to determine whether the given state can be entered.
>>> + *         If %CONFIG_SOFTWARE_SUSPEND is set then %PM_SUSPEND_DISK is
>>> + *         always valid and never passed to this call.
>>> + *         If not assigned, all suspend states are advertised as valid
>>> + *         in /sys/power/state (but can still be rejected by prepare or enter.)
>>> + * @prepare: Prepare the platform for the given suspend state. Can return a
>>> + *           negative error code if necessary.
>>> + * @enter: Enter the given suspend state, must be assigned. Can return a
>>> + *         negative error code if necessary.
>>> + * @finish: Called when the system has left the given state and all devices
>>> + *          are resumed. The return value is ignored.
>>> + * @pm_disk_mode: Set to the disk method that the user should be able to
>>> + *                configure for suspend-to-disk. Since %PM_DISK_SHUTDOWN,
>>> + *                %PM_DISK_REBOOT, %PM_DISK_TEST and %PM_DISK_TESTPROC
>>> + *                are always allowed, currently only %PM_DISK_PLATFORM
>>> + *                makes sense. If the user then choses %PM_DISK_PLATFORM,
>>> + *                the @prepare call will be called before suspending to disk
>>> + *                (if present), the @enter call should be present and will
>>> + *                be called after all state has been saved and the machine
>>> + *                is ready to be shut down/suspended/..., and the @finish
>>> + *                callback is called after state has been restored. All
>>> + *                these calls are called with %PM_SUSPEND_DISK as the state.
>> We usually just indent the following lines the same for all function
>> parameters or struct members.
> 
> So I should indent them to the deepest indent I have like this?
> 
>  @x:         description for x
>              in two lines
>  @verylong:  description for verylong
>              in two lines

I would probably just indent all of them by one tab.  Don't worry about
the @verylong one.

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***