Re: [GIT PULL] Power management Kconfig modification for 2.6.39

[Linus Torvalds]

On Tue, Mar 29, 2011 at 12:43 PM, Rafael J. Wysocki <rjw@xxxxxxx> wrote:
>
> Please pull a power management Kconfig modification for 2.6.39 from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/suspend-2.6.git for-linus

I absolutely detest this.

The way you changed the Kconfig file makes no sense what-so-ever, and
now the Kconfig options are exactly the wrong way around.

We should NOT ask users "do you want the hibernation interface?".
That's just a totally idiotic question to ask, doubly so when it's not
what we ask for the STR "interfaces" either.

>From a user standpoint, the fact that Xen may want to use the
low-level code for hibernation should NOT mean that the user should be
asked about some interface. That's not what they care about, and it's
not what they _should_ care about.

They want hibernation support. You shouldn't ask whether they want the
"interface". That's just crazy.

Now, I realize that you did it the way you did to make the diff
smaller, but I really don't think that's a good reason to make the
config options be the wrong way around, and ask questions (and show a
config option name) of users that are illogical and doesn't match the
other related config options they see. That's the wrong choice to make
just to keep a patch smaller.

I could imagine using just "CONFIG_HIBERNATE" instead of your
suggested "CONFIG_HIBERNATION_INTERFACE", as that would at least match
the current CONFIG_SUSPEND that we ask users about - ie at least the
config option would not be visibly strange to the user. Still, I think
that would be prone to confusion at a code level ("What's the
difference between CONFIG_HIBERNATE and CONFIG_HIBERNATION?"), so I
think that's a bad idea too.

So I'd suggest just making the patch bigger, and make the new
(non-asked) config option be called CONFIG_HIBERNATION_SUPPORT or
something like that. Make Xen select that option instead.

(I also think that your patch was actually wrong. Why does the x86
code to actually save/restore registers depend on that "INTERFACE"
thing, but not on any other architecture? In general, I just think the
thing is confusing. Maybe it's confusing exactly because the config
option name doesn't actually describe _what_ part of the hibernation
code it is all about?)

                     Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/