Re: [PATCH] PM: Hide CONFIG_PM from users
On Tuesday, February 08, 2011, Dmitry Torokhov wrote: On Tue, Feb 08, 2011 at 12:05:40AM +0100, Rafael J. Wysocki wrote: On Monday, February 07, 2011, Dmitry Torokhov wrote: On Mon, Feb 07, 2011 at 11:00:03PM +0100, Rafael J. Wysocki wrote: On Monday, February 07, 2011, Dmitry Torokhov wrote: On Mon, Feb 07, 2011 at 10:15:59PM +0100, Rafael J. Wysocki wrote: On Monday, February 07, 2011, Mark Brown wrote: Yeah, but some people seem very keen on removing the pointers to the PM ops entirely when CONFIG_PM is disabled which means that you end up with varying idioms for what you do with the PM ops as stuff gets ifdefed out. Then again I'm not sure anything would make those people any happier. I really think we should do things that makes sense rather that worry about who's going to like or dislike it (except for Linus maybe, but he tends to like things that make sense anyway). At this point I think the change I suggested makes sense, because it (a) simplifies things and (b) follows the quite common practice which is to make PM callbacks depend on CONFIG_PM. Many people make these callback dependent on PM not because it makes much sense but because it is possible to do so. However, aside of randconfig compile testing, nobody really tests drivers that implement PM in the !CONFIG_PM setting. That I can agree with, but I'm not sure whether it is an argument against the patch I've just posted or for it? More of an observation for your (b) justification. I'd probably force CONFIG_PM to always 'y'w while we weeding references to it from drivers... We simply can't force CONFIG_PM to 'y', because some platforms want it to be 'n'. Again, want or need? It would be nice to know answer to this question. I _think_ the answer is want, which doesn't change a lot, because I'm not going to convince people to stop wanting things. :-) -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Hide CONFIG_PM from users
On Mon, Feb 07, 2011 at 05:17:59PM -0800, Ray Lee wrote: On Mon, Feb 7, 2011 at 7:49 AM, Mark Brown I'm rather hoping that they'll notice the mailing list thread or that someone else who knows what's going on with them does Surely you're joking. I mean, do _you_ scan every message that comes through lkml and its various sister lists? Actually I do at least scan most of the lists. Do a git blame and add them to the CC:. It's the polite thing to do. It's also going to result in the mail not going to the mailing lists as there's a limit on the number of people you can CC enforced by vger which probably isn't constructive. It's moot now but as I said in the text you've helpfully cut I'd have suggested contacting them after the thread had come to a conclusion. -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Hide CONFIG_PM from users
On Mon, Feb 07, 2011 at 06:52:00PM -0800, Frank Rowand wrote: On 02/07/11 04:22, Mark Brown wrote: Since having the configuration option requires non-zero effort to maintain, with ifdefery in most drivers, but it is used with vanishing rarity it is simpler to just remove the option. Proof by assertion that it is used with vanishing rarity. Sure, hopefully if it's incorrect people will come out of the woodwork to correct me :) That is not a good method of getting feedback from users. 1) It immediately removes the ability to have CONFIG_PM undefined, without first giving active users a chance to provide feedback. Note that it's not a terribly difficult change to reverse; if someone urgently does need to do so then I'd be surprised if they were able to build a kernel but not cope with that change. 2) The removal of that ability is not obvious (make oldconfig does not say anything about CONFIG_PM). It is easy to overlook a config change that happens silently. It will expose the sub-options which actually do stuff, though - it's only the top level option for PM. Would it be appropriate to use Documentation/feature-removal-schedule.txt if this truly will be removed? I guess, though I'm a bit pessimistic about anyone actually noticing. With Raphael's version it's not such a big deal as CONFIG_PM is selected by other options that previously depended on it instead of being enabled all the time. -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Hide CONFIG_PM from users
On Tue, Feb 08, 2011 at 12:05:40AM +0100, Rafael J. Wysocki wrote: On Monday, February 07, 2011, Dmitry Torokhov wrote: More of an observation for your (b) justification. I'd probably force CONFIG_PM to always 'y'w while we weeding references to it from drivers... We simply can't force CONFIG_PM to 'y', because some platforms want it to be 'n'. OTOH, if CONFIG_PM = CONFIG_PM_SLEEP||CONFIG_PM_RUNTIME, we can just leave the #ifdefs as they are and simply avoid adding new ones, or use CONFIG_PM for all PM callbacks. For sh at least turning on CONFIG_PM without PM_SLEEP or PM_RUNTIME is largely pointless, so the bulk of the defconfigs have it turned off. The few platforms that do use CONFIG_PM for something also have more comprehensive support implemented. The few times we do have it enabled without one of the others supported is simply for build coverage, mostly due to sharing drivers (whatever isn't already triggered through rand/allyes/modconfigs). I would expect that this is a common scenario for the bulk of the defconfigs that reflect PM being a platform-specific vs architecture-wide property regardless of architecture, however. -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Hide CONFIG_PM from users
* Mark Brown broo...@opensource.wolfsonmicro.com wrote: It is very rare to find a current system which is both sufficiently resource constrained to want to compile out power management support and sufficiently power insensitive to be able to tolerate doing so. Since having the configuration option requires non-zero effort to maintain, with ifdefery in most drivers, but it is used with vanishing rarity it is simpler to just remove the option. Well, either make it dependent on CONFIG_EXPERT or remove the option altogether. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Hide CONFIG_PM from users
On Mon, Feb 07, 2011 at 01:48:46PM +0100, Ingo Molnar wrote: * Mark Brown broo...@opensource.wolfsonmicro.com wrote: Since having the configuration option requires non-zero effort to maintain, with ifdefery in most drivers, but it is used with vanishing rarity it is simpler to just remove the option. Well, either make it dependent on CONFIG_EXPERT or remove the option altogether. The goal is the latter but when I saw the IA64 emulator there I didn't want to make it instabuggy. The current patch means we get the UI effect of the change (especially in terms of avoiding it turning up in randconfigs or whatever). -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Hide CONFIG_PM from users
Hi Mark, On Mon, 7 Feb 2011 12:22:15 + Mark Brown broo...@opensource.wolfsonmicro.com wrote: diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig index 2657299..99e3c52 100644 --- a/kernel/power/Kconfig +++ b/kernel/power/Kconfig @@ -1,23 +1,6 @@ config PM - bool Power Management support - depends on !IA64_HP_SIM - ---help--- - Power Management means that parts of your computer are shut - off or put into a power conserving sleep mode if they are not - being used. There are two competing standards for doing this: APM - and ACPI. If you want to use either one, say Y here and then also - to the requisite support below. - - Power Management is most important for battery powered laptop - computers; if you have a laptop, check out the Linux Laptop home - page on the WWW at http://www.linux-on-laptops.com/ or - Tuxmobil - Linux on Mobile Computers at http://www.tuxmobil.org/ - and the Battery Powered Linux mini-HOWTO, available from - http://www.tldp.org/docs.html#howto. - - Note that, even if you say N here, Linux on the x86 architecture - will issue the hlt instruction if nothing is to be done, thereby - sending the processor to sleep and saving power. + bool + default y if !IA64_HP_SIM Several powerpc configs have CONFIG_PM (implicitly) disabled (e.g. the server configs), so this will unexpectedly turn it on for them. -- Cheers, Stephen Rothwells...@canb.auug.org.au http://www.canb.auug.org.au/~sfr/ pgpkFS5I6VqwG.pgp Description: PGP signature
Re: [PATCH] PM: Hide CONFIG_PM from users
Hi Mark, On Mon, 7 Feb 2011 14:18:29 + Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Tue, Feb 08, 2011 at 01:13:24AM +1100, Stephen Rothwell wrote: On Mon, 7 Feb 2011 12:22:15 + Mark Brown broo...@opensource.wolfsonmicro.com wrote: + bool + default y if !IA64_HP_SIM Several powerpc configs have CONFIG_PM (implicitly) disabled (e.g. the server configs), so this will unexpectedly turn it on for them. Do you mean that these systems require CONFIG_PM be turned off, or just that people tend not to turn it on? If the latter would you expect any ill effects from doing so? I don't know the answer to either question without testing. All I am saying is that currently the default for CONFIG_PM is off and you are changing it to be on and there may not have been any testing done of that in some situations. We don't know where it was explicitly turned off any more since we shrank our defconfig files (which was done automatically) ... since it is off by default, it doesn't need to be mentioned in a defconfig unless it needs to be turned on. -- Cheers, Stephen Rothwells...@canb.auug.org.au http://www.canb.auug.org.au/~sfr/ pgpGzafpyYxuI.pgp Description: PGP signature
Re: [PATCH] PM: Hide CONFIG_PM from users
On Tue, Feb 08, 2011 at 01:44:32AM +1100, Stephen Rothwell wrote: On Mon, 7 Feb 2011 14:18:29 + Mark Brown broo...@opensource.wolfsonmicro.com wrote: Do you mean that these systems require CONFIG_PM be turned off, or just that people tend not to turn it on? If the latter would you expect any ill effects from doing so? I don't know the answer to either question without testing. All I am saying is that currently the default for CONFIG_PM is off and you are changing it to be on and there may not have been any testing done of that in some situations. We don't know where it was explicitly turned off any more since we shrank our defconfig files (which was done automatically) ... since it is off by default, it doesn't need to be mentioned in a defconfig unless it needs to be turned on. My suspicion would be that it'll have been turned off by someone hitting return through a config upgrade rather than through deliberate effort. On the other hand if it is essential for some machines to have it disabled they probably want to have somethnig in Kconfig. -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Hide CONFIG_PM from users
On Mon, Feb 7, 2011 at 15:50, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Tue, Feb 08, 2011 at 01:44:32AM +1100, Stephen Rothwell wrote: On Mon, 7 Feb 2011 14:18:29 + Mark Brown broo...@opensource.wolfsonmicro.com wrote: Do you mean that these systems require CONFIG_PM be turned off, or just that people tend not to turn it on? If the latter would you expect any ill effects from doing so? I don't know the answer to either question without testing. All I am saying is that currently the default for CONFIG_PM is off and you are changing it to be on and there may not have been any testing done of that in some situations. We don't know where it was explicitly turned off any more since we shrank our defconfig files (which was done automatically) ... since it is off by default, it doesn't need to be mentioned in a defconfig unless it needs to be turned on. My suspicion would be that it'll have been turned off by someone hitting return through a config upgrade rather than through deliberate effort. On the other hand if it is essential for some machines to have it disabled they probably want to have somethnig in Kconfig. $ git grep CONFIG_PM is not set 7cf3d73b4360e91b14326632ab1aeda4cb26308d^ -- arch/ | wc -l 256 $ 7cf3d73b4360e91b14326632ab1aeda4cb26308d is the commit that introduced savedefconfig, so that's a safe revision with untrimmed defconfigs. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say programmer or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Hide CONFIG_PM from users
On Tue, Feb 08, 2011 at 02:19:16AM +1100, Stephen Rothwell wrote: At least some of the powerpc defconfigs were added with CONFIG_PM disabled. I assume that was on purpose (though it may not have been). I'd not be so sure - since it's a bool without an explicit default set Kconfig will default to disabling it and if anything enabling it is the option that requires special effort. -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Hide CONFIG_PM from users
On Mon, Feb 07, 2011 at 10:36:31AM -0500, Alan Stern wrote: On Mon, 7 Feb 2011, Mark Brown wrote: I'd not be so sure - since it's a bool without an explicit default set Kconfig will default to disabling it and if anything enabling it is the option that requires special effort. This may be a naive suggestion, but have you considered simply _asking_ the people who added those defconfigs? I'm rather hoping that they'll notice the mailing list thread or that someone else who knows what's going on with them does - as Geert pointed out there's a considerable number of defconfigs that have this turned off. It seems more sensible to get some idea if this seems sane to people in the general case before going trying to identify and contact so many individuals. If there are systems that really require disabling CONFIG_PM we probably need to add stuff to Kconfig to make sure it can't be enabled anyway; this shouldn't enable any new configurations. -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Hide CONFIG_PM from users
On Monday, February 07, 2011, Mark Brown wrote: It is very rare to find a current system which is both sufficiently resource constrained to want to compile out power management support and sufficiently power insensitive to be able to tolerate doing so. Since having the configuration option requires non-zero effort to maintain, with ifdefery in most drivers, but it is used with vanishing rarity it is simpler to just remove the option. Begin doing so by hiding it from users - this should attract complaints from any active users. The option is left disabled for the IA64 Ski simulator which is a partial simulator for IA64 systems mostly missing device support. This is a very limited use case which is unlikely to ever want to enable most drivers. Signed-off-by: Mark Brown broo...@opensource.wolfsonmicro.com --- kernel/power/Kconfig | 21 ++--- 1 files changed, 2 insertions(+), 19 deletions(-) diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig index 2657299..99e3c52 100644 --- a/kernel/power/Kconfig +++ b/kernel/power/Kconfig @@ -1,23 +1,6 @@ config PM - bool Power Management support - depends on !IA64_HP_SIM - ---help--- - Power Management means that parts of your computer are shut - off or put into a power conserving sleep mode if they are not - being used. There are two competing standards for doing this: APM - and ACPI. If you want to use either one, say Y here and then also - to the requisite support below. - - Power Management is most important for battery powered laptop - computers; if you have a laptop, check out the Linux Laptop home - page on the WWW at http://www.linux-on-laptops.com/ or - Tuxmobil - Linux on Mobile Computers at http://www.tuxmobil.org/ - and the Battery Powered Linux mini-HOWTO, available from - http://www.tldp.org/docs.html#howto. - - Note that, even if you say N here, Linux on the x86 architecture - will issue the hlt instruction if nothing is to be done, thereby - sending the processor to sleep and saving power. + bool + default y if !IA64_HP_SIM config PM_DEBUG bool Power Management Debug Support I think it would be better to simply rename CONFIG_PM_OPS into CONFIG_PM. However, there's a number of things that I'm afraid wouldn't build correctly if none of CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME were set in that case. Anyway, I'll try to prepare a patch doing that and see what happens. Stay tuned. Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Hide CONFIG_PM from users
On Mon, Feb 07, 2011 at 08:14:03PM +0100, Rafael J. Wysocki wrote: On Monday, February 07, 2011, Mark Brown wrote: config PM_DEBUG bool Power Management Debug Support I think it would be better to simply rename CONFIG_PM_OPS into CONFIG_PM. That still leaves the IA64 emulator to worry about but I'm not fundamentally opposed to that, it achieves a similar effect. The main thing I'm looking for here is to cut down on the configuration options we have to maintain. However, there's a number of things that I'm afraid wouldn't build correctly if none of CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME were set in that case. Actually CONFIG_PM_OPS probably also wants to be on independantly of those two sometimes for .poweroff() which I'd expect to run even if we can't suspend. -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Hide CONFIG_PM from users
On Mon, Feb 07, 2011 at 08:46:48PM +0100, Rafael J. Wysocki wrote: On Monday, February 07, 2011, Mark Brown wrote: On Mon, Feb 07, 2011 at 08:14:03PM +0100, Rafael J. Wysocki wrote: I think it would be better to simply rename CONFIG_PM_OPS into CONFIG_PM. That still leaves the IA64 emulator to worry about Why exactly? Actually not so much the IA64 emulator (which does have the !PM dependency declared already - I expect that'd just be moved) as any other platforms with an undeclared dependency on !PM. but I'm not fundamentally opposed to that, it achieves a similar effect. The main thing I'm looking for here is to cut down on the configuration options we have to maintain. But I must say you chose a particularly bad time for that from my point of view. This doesn't seem like it's a worse time than any other? However, there's a number of things that I'm afraid wouldn't build correctly if none of CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME were set in that case. Actually CONFIG_PM_OPS probably also wants to be on independantly of those two sometimes for .poweroff() which I'd expect to run even if we can't suspend. If you worry about that, then add CONFIG_PM_POWEROFF and make CONFIG_PM(_OPS) depend on it, but I don't think it really is worth it, because people generally don't make the poweroff code depend on CONFIG_PM. Yeah, but some people seem very keen on removing the pointers to the PM ops entirely when CONFIG_PM is disabled which means that you end up with varying idioms for what you do with the PM ops as stuff gets ifdefed out. Then again I'm not sure anything would make those people any happier. -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Hide CONFIG_PM from users
On Mon, Feb 07, 2011 at 11:00:03PM +0100, Rafael J. Wysocki wrote: On Monday, February 07, 2011, Dmitry Torokhov wrote: On Mon, Feb 07, 2011 at 10:15:59PM +0100, Rafael J. Wysocki wrote: On Monday, February 07, 2011, Mark Brown wrote: Yeah, but some people seem very keen on removing the pointers to the PM ops entirely when CONFIG_PM is disabled which means that you end up with varying idioms for what you do with the PM ops as stuff gets ifdefed out. Then again I'm not sure anything would make those people any happier. I really think we should do things that makes sense rather that worry about who's going to like or dislike it (except for Linus maybe, but he tends to like things that make sense anyway). At this point I think the change I suggested makes sense, because it (a) simplifies things and (b) follows the quite common practice which is to make PM callbacks depend on CONFIG_PM. Many people make these callback dependent on PM not because it makes much sense but because it is possible to do so. However, aside of randconfig compile testing, nobody really tests drivers that implement PM in the !CONFIG_PM setting. That I can agree with, but I'm not sure whether it is an argument against the patch I've just posted or for it? More of an observation for your (b) justification. I'd probably force CONFIG_PM to always 'y'w while we weeding references to it from drivers... -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Hide CONFIG_PM from users
On Monday, February 07, 2011, Dmitry Torokhov wrote: On Mon, Feb 07, 2011 at 11:00:03PM +0100, Rafael J. Wysocki wrote: On Monday, February 07, 2011, Dmitry Torokhov wrote: On Mon, Feb 07, 2011 at 10:15:59PM +0100, Rafael J. Wysocki wrote: On Monday, February 07, 2011, Mark Brown wrote: Yeah, but some people seem very keen on removing the pointers to the PM ops entirely when CONFIG_PM is disabled which means that you end up with varying idioms for what you do with the PM ops as stuff gets ifdefed out. Then again I'm not sure anything would make those people any happier. I really think we should do things that makes sense rather that worry about who's going to like or dislike it (except for Linus maybe, but he tends to like things that make sense anyway). At this point I think the change I suggested makes sense, because it (a) simplifies things and (b) follows the quite common practice which is to make PM callbacks depend on CONFIG_PM. Many people make these callback dependent on PM not because it makes much sense but because it is possible to do so. However, aside of randconfig compile testing, nobody really tests drivers that implement PM in the !CONFIG_PM setting. That I can agree with, but I'm not sure whether it is an argument against the patch I've just posted or for it? More of an observation for your (b) justification. I'd probably force CONFIG_PM to always 'y'w while we weeding references to it from drivers... We simply can't force CONFIG_PM to 'y', because some platforms want it to be 'n'. OTOH, if CONFIG_PM = CONFIG_PM_SLEEP||CONFIG_PM_RUNTIME, we can just leave the #ifdefs as they are and simply avoid adding new ones, or use CONFIG_PM for all PM callbacks. Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-embedded in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html