Hi there.
Attached is the log of the IRC meeting yesterday, including a lengthy
discussion on the merits/detriments on the calling convention for the PCI
suspend/resume functions.
I have not had a chance to summarize the discussions, and I probably won't
get a chance to do so in the next couple of days.
Pat
-------------- next part --------------
13:02 -!- mochel [^mochel@xxxxxxxxxxxxx] has joined #pm
13:02 [Users #pm]
13:02 [ alan] [ benh] [ jordan] [ lenb] [ mochel] [ nigel]
13:02 -!- Irssi: #pm: Total of 6 nicks [0 ops, 0 halfops, 0 voices, 6 normal]
13:02 -!- Channel #pm created Thu Mar 24 10:15:11 2005
13:02 < benh> hi !
13:02 -!- Irssi: Join to #pm was synced in 0 secs
13:02 < nigel> But I think it will help us see what we need.
13:02 < alan> Hello, Pat.
13:02 < benh> back in a minute ...
13:02 < nigel> Hi Ben. Hi Pat.
13:02 < mochel> Hello.
13:02 -!- pavelm [~pavel@xxxxxxxxxxxxxxxxxxxxx] has joined #pm
13:02 < mochel> I knew I was forgetting something..
13:02 < nigel> Hi Pavel :>
13:02 < alan> Hello, Pavel.
13:02 < pavelm> Heya, everyone.
13:03 < alan> Len has a meeting conflict but he should be back later.
13:03 < pavelm> This time I have reasonable keyboard (not 5cm wide) and reasonable connection (does not fail
half the time)...
13:04 < nigel> :> And your seat doesn't bounce?
13:04 < alan> pavelm: And you're not riding out in the rain while trying to type? :-)
13:04 < pavelm> alan: Not this time. It was close, but I made it to the bus this time.
13:04 < mochel> How is everyone doing?
13:04 < mochel> nigel: thanks for sending the reminder. :)
13:05 * nigel is only here for 3/4hr.
13:05 < nigel> mochel: You're welcome. I'm happy to play organiser :>
13:05 < nigel> (As in Palm)
13:05 < pavelm> ...and we were only driving twice the speed limit ;-)
13:05 < mochel> nigel: heh, good to hear
13:05 < alan> I think we have enough people here to start now. Who wants to go first?
13:05 < mochel> So, what does everyone want to talk about today?
13:06 < pavelm> Well perhaps that me
13:06 < pavelm> pm_message_t changes are *not* done yet.
13:06 < pavelm> Type was changed to pm_message_t, but pm_message_t is still a scalar.
13:06 < pavelm> ...it is going to become struct, probably past 2.6.12.
13:06 < nigel> Pavel: I still have patches.... I can resend what I know still needs changing.
13:06 < mochel> pavelm: did you see my email about the patch?
13:07 < pavelm> If you have changes that look inside pm_message_t, please hold them for a little more time.
13:07 -!- db [~db@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx] has joined #pm
13:07 < pavelm> nigel: Lots of changes are only in -mm tree. There should be very little that needs fixing on
the top of that.
13:07 < benh> heh
13:07 < nigel> This is just patches to complete the switchover from u32 to pm_message_t
13:07 < alan> Hello, David.
13:07 < benh> this time I'm not drunk :)
13:07 < nigel> :>
13:08 < db> Hi ...
13:08 < pavelm> If you know something that need fixing in -mm, just let me know.
13:08 < nigel> Ok. I'll send you what I have off list.
13:08 < mochel> nigel: feel free to cc the list
13:08 < nigel> LKML?
13:08 < mochel> it'd be nice to have some more code flying around :)
13:08 < mochel> nigel: linux-pm
13:08 < nigel> k
13:09 < pavelm> mochel: No, I did not see your mail; I have ~20 unread mails in my inbox.
13:09 < nigel> Only 20?
13:09 < alan> I sent out an email an hour or two ago, has anyone seen it yet?
13:09 < nigel> But then I just woke...
13:09 < mochel> pavelm: ok. it was just a couple of minor comments
13:09 < benh> k, I have about 3/4h before I have to go
13:09 < benh> what is today's topic ? :)
13:09 < nigel> alan: Which list?
13:09 < alan> nigen: linux-pm
13:10 < nigel> "drinking from a fire hose"?
13:10 < mochel> benh: no topic yet
13:10 < nigel> That's the last one I have from you.
13:10 < alan> nigel: Well, isn't it?
13:10 < nigel> :>
13:10 < pavelm> Okay, talking about todays mails... What shall we do with that cpu-specific suspend/resume
routines?
13:10 < db> who set the fire?
13:10 < pavelm> They do not belong to normal trees.
13:10 < alan> nigel: No, there was another one after that.
13:11 < jordan> alan: thats the last one I have too
13:11 < mochel> ditto
13:11 < alan> jordan: It must be sitting on the server.
13:11 < mochel> pavelm: check your inbox :)
13:11 * db hasn't seen anything newer either
13:12 < mochel> pavelm: they should be on a per-cpu list, and iterated over at suspend + resume time
13:12 < db> So Alan, what did the mail say? one-line summary...
13:12 < alan> I was trying to get a consensus on things we could all agree to.
13:12 < pavelm> Seen my inbox *that* far. Perhaps notifier list in struct cpu should be okay....
13:12 < alan> The focus was on "what", not "how".
13:13 < pavelm> ...okay, who codes that "per-cpu suspend/resume lists"?
13:13 < mochel> pavelm: notifier list is about the same as struct type w/ method in it.
13:13 < nigel> pavelm: It has to run with IRQs disabled and not reenable them. That's ok, isn't it?
13:13 < nigel> alan: /me hides
13:13 < mochel> pavelm: but struct w/ method is type safe, and doesn't require modification to
include/linux/notifier.h
13:14 < mochel> pavelm: i think the entire sysdev model needs to be reworked
13:14 < pavelm> not sure... irqs disabled are certainly okay for mtrrs, but I'm not about cpufreq.
13:14 < nigel> Well we can look into that later.
13:14 < mochel> it might not require *that* much work, but it definitely needs to be revisited
13:14 < pavelm> How does cpufreq handle SMP systems, anyway?
13:14 < pavelm> Is it possible to have different cpus running at different speeds? Probably not....
13:14 < mochel> all in all, mtrrs seem like relatively low-priority
13:14 < nigel> Let's put it aside for the mo guys.
13:14 < nigel> we need to cover things everyone can help with
13:15 < pavelm> mochel: mtrrs block smp suspend. I need to solve them somehow.
13:15 < pavelm> mochel: smp suspend is quite important with all that HT machines around.
13:15 < mochel> pavelm: hack it for now, but don't try to merge it in mainline
13:15 < nigel> We have 1/2hr before some of us need to go. What can we cover in that time?
13:16 * mochel thinks we need a -pm tree
13:16 < pavelm> mochel: THat does not fly. I'd like to have working smp swsusp in mainline.
13:16 < pavelm> mochel: We already *have* -pm tree.
13:16 < pavelm> mochel: Only it is -PavelMachek ;-)))))))))))))))))))))))))
13:16 < mochel> pavelm: first, you need it locally
13:17 < mochel> pavelm: so hack it locally, then work on how to create a beautiful solution that everyone can
use.
13:17 < mochel> *then* merge it in mainline
13:17 < nigel> Can we go back to that question you started on earlier Alan?
13:17 < pavelm> mochel: Okay, I think I can put some callbacks to struct cpu.
13:17 < nigel> How do we decide on system states and match them with device capabilities?
13:17 < mochel> the more hacks that go upstream, the longer it will take to implement *all* the changes we need
13:18 < alan> More generally, how can we put helpers in the core while letting drivers do what they want?
13:18 * nigel makes a valiant attempt but doubts it will make a difference :>
13:19 < db> What we had a while back was: suspend() sees system state, driver maps appropriately.
13:19 < nigel> I guess the helpers need to be generic enough, but still be helpful :>
13:19 < pavelm> mochel: We need some kind of per-cpu notification, and list in struct cpu does not seem like a
hack.
13:19 < nigel> (That was a helpful comment, wasn't it!)
13:19 < alan> Haven't we already decided on system states? On, Standby, STR, STD.
13:19 < db> The pm_message_t changed it. Why?
13:19 < mochel> pavelm: ok. just implement and see what it looks like..
13:19 < nigel> I think we have.
13:19 < db> alan: my understanding is that those three states are conventional, but not the entire set.
13:19 < nigel> db: Huh?
13:20 < nigel> Oh.
13:20 < pavelm> db: pm_message_t ... we still want to pass all the info to the drivers, and drivers use helpers.
13:20 < alan> db: I don't follow you.
13:20 < pavelm> db: notice that pm_message_t changes are *not* yet done. pm_message_t is going to become a
struct, with flags saying details.
13:20 < db> nigel: responding to your question "how do we decide..."
13:20 < nigel> alan: db thinks we didn't settle on only STD, STR & standby
13:21 < alan> Should there be other system states too? Platform dependent ones?
13:21 < nigel> The problem we have is, if we provide for arbitrary other system states,
13:21 < pavelm> db: my goal is to have distict flags for each use, so that drivers that *really really* care
can know what is really going on.
13:21 < nigel> how do we manage to map device states to system states.
13:22 < nigel> I think we've agreed that the logic for that needs to be in drivers
13:22 < db> we seem to have three concurrent discussions here ...
13:22 < nigel> with help from helpers
13:22 < mochel> db: such is the fate of irc ;)
13:22 < db> helpers can be platform-specific or bus-specific
13:22 < nigel> But it's still very complex :>
13:22 < db> mochel: but last time it was soo peaceful ... ;)
13:23 < pavelm> db, mochel: Last time I had too small keyboard ;-).
13:23 < db> pavel: oh no!!
13:23 < db> OK, helpers ...
13:23 < alan> How can generic drivers and helpers incorporate platform-specific information about system states?
13:23 < nigel> So definining a new system state would have to also somehow define what devices should do that
may have never heard of this state before.
13:23 < db> helpers will need to tear apart the system state components
13:23 < mochel> alan: #ifdef CONFIG_<arch>
13:24 < pavelm> alan: generic drivers nad generic helpers do not care.
13:24 < db> if a system state is a struct, platform specific helpers can just container_of() to get more details
13:24 < pavelm> but the flag is ...
13:24 < pavelm> db: do you need something more than bitflags?
13:24 < mochel> I don't think we're going to solve this problem today on irc
13:24 < mochel> it would be nice if we had a sample implementation
13:25 < mochel> that way we could comment on specific points
13:25 < db> pavel: bitflags for what? I'm starting with "what's the target system state". Needs to return a
pointer.
13:25 < benh> well, about those states, a few comments
13:25 < mochel> who wants to write it?
13:25 < pavelm> ...I think we do have sample implementation, and I think we are goign to solve it today.
13:25 < db> pavel: no need yet for flags to achieve such a thing.
13:25 < pavelm> target system state is encoded in pm_message_t; it has two fields
13:25 < alan> db: What information do drivers need to know about a new system state?
13:25 < benh> one is, most drivers need to know wether they have to freeze operations, that is, does the state
have freeze semantics
13:25 < pavelm> everyone understands event. If you are doing something strange, you
13:25 < benh> they all do for now of course
13:25 < pavelm> can look inside flags.
13:25 < pavelm> End of story.
13:26 < benh> but we may want to define other kind of system states
13:26 < pavelm> No pointers needed.
13:26 < db> (platform helpers don't need container_of but it's an option to avoid other lookup strategies
13:26 < benh> as far as I know, that's all
13:26 < nigel> Let's go back to basics.
13:26 < benh> that is, drivers who actually care about details like not turning the HD off on STD first pass
can test the explicit state
13:26 < nigel> As far as pm does, what states can a driver be in?
13:26 < db> alan: information needed about target state: will this device lose power? or clock?
13:26 < mochel> benh: that would be dependent on the state, which is dependent on the platform, right?
13:26 < nigel> A system state must just be a paritcular combination of driver states, right?
13:26 < benh> if we go back to what we defined earlier for system state, they could simply be a struct that
contains
13:27 < benh> int state;
13:27 < benh> int action;
13:27 < benh> where state is STD,STR,...
13:27 < alan> db: So a new system state would have to incorporate this information about all power buses and
all clocks?
13:27 < benh> and action is "freeze", "suspend", ....
13:27 < pavelm> benh: No, drivers who actually care about details like not turning the HD off on STD are going
to look into flags...
13:27 < nigel> alan: Or the information would need to be able to be implied from it.
13:28 < db> alan: the mapping logic can embed that info (imply it)
13:28 < pavelm> typedef struct pm_message {
13:28 < pavelm> int event;
13:28 < pavelm> int flags;
13:28 < pavelm> } pm_message_t;
13:28 < db> struct x ... *pm_message_t
13:28 < db> please no struct pass by value
13:28 < alan> db: Something like: extra state for BigSleep and for DeepSleep?
13:28 < pavelm> db: passing two ints is okay.
13:28 < pavelm> db: and it is going to be passed by value
13:28 < pavelm> db: it is too late to change it now.
13:28 < pavelm> db: and it has no downsides.
13:29 < nigel> pavelm: If it's the same state for everyone, why pass it? Just put it in a global and save
stack...
13:29 < pavelm> nigel: global variable was tried before, shot down by akpm or whoever.
13:29 < db> alan: extra state is more relevant for stuff like DPM, where it's platform-specific not cpu-specific
13:29 < pavelm> nigel: because global variables are ugly.
13:30 < db> global variable == evil (how does one make sign of cross in irc) vampires begone!
13:30 < mochel> we can hide a global in a function and call it..
13:30 < nigel> So is recursion :>
13:30 < db> pavel: it's not too late to change, and pass-by-value has downsides. just use a pointer like
everyone else.
13:30 < pavelm> mochel: but why should we do it. pm_message_t is okay. Move on.
13:30 < nigel> pm_get_system_state()
13:30 < alan> db: Still, generic drivers have to figure out how to handle these new states. How can a helper
be made to work?
13:30 < pavelm> db: no.
13:31 < pavelm> db: what downsides?
13:31 < db> alan: platform-specific helper, platform-specific state. what's the confusion? they're all
friendly...
13:31 < nigel> Probably not just platform specific.
13:32 < db> pavel: for one thing, adding more fields to pass-by-value is trouble.
13:32 < nigel> Also bus specific?
13:32 < pavelm> db: Why? So you use little more stack. Who cares.
13:32 < db> nigel: agreed, i just picked on platform-specific because it generalizes well enough
13:32 < pavelm> db: You should not need any more fields anyway.
13:32 < alan> But then each platform needs to add a helper for each driver! That's a lot of helpers.
13:32 < db> pavel: famous last words. 64 KB memory, you shouldn't need more. 4k stack...
13:33 < nigel> A system state might do different things depending on the platform, bus, whether it's busy...
anything else?
13:33 < db> alan: didn't say helper-per-driver, I said helper-per-platform.
13:33 < pavelm> db: if you need something big, you can always add pointer *inside* pm_message_t.
13:33 < nigel> alan: A lot of them will be the same. They could share.
13:33 < db> alan: and any given platform has only a finite number of platform devices -- a handful usually
13:34 < alan> db: I get it. Like: will clock X be turned on in system state Y...
13:34 < db> pavel: see what I said about growing the struct?
13:34 < db> pavel: there you're doing it already.
13:34 < db> pavel: other than you, who would push back on passing pointers?
13:34 < alan> db: Platform devices aren't the problem. Generic devices have to understand the new states as
well.
13:35 < pavelm> db: try implementing that. Pointers mean trouble, lifetime rules, etc, etc.
13:35 < benh> I propose we don't try to solve the system states mess now
13:35 < nigel> Ok. I'm history. Catch you all later.
13:35 < benh> and we stick to the ones we defined so far
13:35 < db> alan: what's "generic" about a specific platform?
13:35 < alan> Bye, Nigel.
13:35 < benh> that is freeze/suspend basically
13:35 < db> see you nigel!
13:35 < pavelm> db: plus it will be tricky to do switch from 2.6.10 u32s to pointers.
13:35 < benh> and focus on the DPM stuff we have been discussinfg
13:35 < pavelm> db: ...and btw this was all decided year or so ago...
13:35 < benh> because I'm convinced that when we have solved _that_, the system state stuff will fit in more
clearly :)
13:36 < db> benh: so how do we recover the distinction between different suspend states?
13:36 < alan> db: A disk driver (like the IDE driver) has to work on many platforms.
13:36 < db> pavelm: clearly, no it was not decided a year or so ago!
13:36 < benh> db: by stopping defining them both to the same numerical value
13:36 < benh> db: that was stupid, I never agreed with pavelm on that one
13:36 < benh> db: we'd rather fix the few drivers that make assumption on the numerical value
13:36 < pavelm> db: read Documentation/power/devices.txt.
13:37 < db> benh: ok, neither did i, and it breaks drivers that do different things based on degree-of-suspend
13:37 < pavelm> db: we had quite bad flames back then, and I do not want to repeat them now.
13:37 < mochel> just because some thing is in the kernel doesn't mean it's a good idea
13:38 < alan> I don't see any reason why passing a structure won't work. There shouldn't be a lot of recursion.
13:38 < pavelm> mochel: Only argument so far was Bill Gates reference.
13:38 < jordan> heh
13:38 < pavelm> benh: two same defines are finally going away.
13:38 < mochel> just because something isn't argued doesn't mean it's a good idea, either.
13:38 < mochel> :)
13:39 < db> alan: re ide "generic", I don't see what you're saying. helper answers "your power goes off" or
not... ?
13:39 < pavelm> benh: They probably will not make it into 2.6.12, but we should have it fixed in 2.6.13.
13:39 < db> just because people got tired of flaming back doesn't mean they agreed either ...
13:39 < alan> db: Here's what I'm getting at...
13:39 < alan> db: A driver gets a message it doesn't understand
13:39 < pavelm> mochel: We had pretty serious flames back then, and current solution is good.
13:39 < alan> db: It asks a helper: Will my clock still be running in
13:40 < alan> db: this new state?
13:40 < alan> db: If that can be made to work, then we should be okay.
13:40 < mochel> pavelm: ...in your opinion :)
13:40 < db> pavelm: you're hearing disagreement about whether current==good ... listen to it!
13:41 < pavelm> mochel, dm: Your best argument was stack consumption. That is not a problem, recursion is not
deep.
13:41 < db> alan: that particular case, "power" is probably a better question. Simple Matter Of Programming
there.
13:41 < pavelm> ...nor it is hot path.
13:41 < mochel> alan: should we have that type of logic in the drivers?
13:41 < pavelm> ...you can try to bring some better argument. Feel free to suggest patch to
13:42 < pavelm> Documentation/power/devices.txt.
13:42 < mochel> pavelm: it's common rule of thumb to not pass structs by value, because they can later grow
without bound, and without realizing it
13:42 < jordan> all the more reason why we should table the system state disucssions, and move on to the policy
stuff
13:42 < alan> mochel: The point is that it's platform dependent. Better to localize it outside of generic
drivers.
13:42 < pavelm> mochel: I'll realize when I'm passing something that big.
13:42 < db> alan: i think "generic" drivers aren't the ones that most need clock info; they need power info.
13:42 < pavelm> mochel: and btw it is stupid to have to define lifetime rules for something that simple.
13:42 < mochel> pavelm: it's also different than 99.99% of all other code in the kernel; it breaks convention
13:43 < db> pavel: you may be wunderkind, but what about everyone else? nobody expects struct-by-value.
13:43 < alan> db: Clock, power ... both are needed, maybe not by all drivers.
13:43 < pavelm> passing pointer to two ints is just plain wrong.
13:43 < db> ... i think Pavel's not keen on listening to anyone on this topic,
13:43 < pavelm> if it is named pm_message_t, yes, they'd better expect struct by value.
13:43 < db> ... maybe we can just agree he's wrong and move on? :)
13:43 < pavelm> db: we had quite long flamewars, see the list archives.
13:44 < db> alan: agreed, driver-specific
13:44 < pavelm> db: it was not me deciding it alone.
13:44 < db> pavelm: what I said before about "getting tired of flamewar" != "agree result was ok" ...?
13:44 -!- gregkh [^gregkh@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx] has joined #pm
13:44 < alan> Greg, you stepped into the middle of a big argument!
13:45 < mochel> alan: agree that it's platform dependent
13:45 * gregkh is just here to observe.
13:45 < pavelm> db: look at those archives...
13:45 < mochel> alan: but, should the drivers care what is going to happen to them, or just do what they're told
13:45 < pavelm> db: if you pass pointers someone will compare the pointers, and someone else will compare the
values...
13:45 * db is amused at the notion of looking at archives of a flamewar
13:45 < benh> pavelm: 2.6.12 is a long way away, we can fairly easily fix that stupid thing now
13:45 < alan> mochel: Both. They can't do what they're told if they don't understand the message...
13:46 < mochel> i.e. should the state they enter be pre-determined by some platform-specific policy, and should
the drivers just obey?
13:46 < pavelm> benh: I was told that 2.6.12 is *not* that far away.
13:46 < benh> anyway, I'm running out of time, I have to head to Sydney rsn and still haven't showered :)
13:46 * jordan tunes into mochel and alan's discussion which sounds much more productive
13:46 < pavelm> benh: only now are u32 vs. pm_message_t patches being merged...
13:46 < benh> pavelm: I think 2.6.12 will need a lot of testing / stabilization
13:46 < alan> mochel: Yes, platform-specific policy maps new special states to device settings. Drivers obey
those mappings.
13:46 < pavelm> benh: and u32 vs. pm_message_t needs to be solved before I can start switching pm_message_t
into struct.
13:47 < benh> pavelm: things like the new pcmcia stuff are nice, but will need a bit of time
13:47 < benh> pavelm: etc....
13:47 < pavelm> benh: Unfortunately we have quite a lot of changes in pm area, too....
13:47 < benh> pavelm: anyway, I don't see why we couldn't have defined two different numbers
13:47 < benh> pavelm: and fixed the drivers that make wrong assumption, it's not like there is a lot ...
13:47 < pavelm> benh: I do not want anyone to test them *now* because I'd have to fix them again once
pm_message_t becomes struct.
13:47 < benh> it would be nice if 2.6.12 has working USB PM :)
13:48 * benh runs
13:48 < db> benh: coward!!
13:48 < benh> hehe
13:48 < alan> benh: Real Soon Now... :-)
13:48 < db> ... working on it.
13:48 < benh> I need to look at this errata thing
13:48 < pavelm> benh: working usb pm does not depend on PMSG_FREEZE vs. PMSG_SUSPEND being different.
13:48 < benh> especially the "suspendable ports" thing
13:48 < benh> I suspect it might explain some of the spurrious wakeups
13:48 < benh> if some ports aren't properly connected to anything and not properly grounded
13:48 < mochel> alan: more generally..
13:48 < db> something broke all kinds of stuff, and I do NOT have time to re-test those dozens of configs any
more
13:49 < benh> we may have to just clamp the off all time
13:49 < mochel> alan: platform-specific policy maps all system states to device states?
13:49 < db> ben: alan and I were talking offline, i think I'll send a patch to teach OHCI and EHCI
13:49 < mochel> or rather.. driver states
13:49 < benh> I think system->device state should be a device matter
13:49 < alan> mochel: I was thinking mainly of the new system states David mentioned,
13:49 < benh> though the device should be able to ask the platform what will happen to it
13:49 < benh> (powered off, unclocked, ...)
13:49 < db> to disable IRQs at the source ** for PCI ONLY ** since both have non-pci implementations
13:49 < alan> mochel: things other than Standby, STR, STD. But it makes sense to
13:49 < benh> but this is a bit dodgy to define
13:50 < alan> mochel: use a platform-specific mapping policy always.
13:50 < mochel> benh: curious, how will affect drivers?
13:50 < pavelm> Len's patch went into 2.6.12-rc1 that basically broke everything.
13:50 < benh> db: ok, you mean MEI ?
13:50 < pavelm> ...it BUG()s in pci_choose_state.
13:50 < mochel> alan: standard states may behave differently on the platform
13:50 < pavelm> ....fix is known.
13:50 < db> benh: yes, MIE on OHCI
13:51 < benh> mochel: well, at least radeonfb has different PM path for "suspend" (D2) and "shutdown" (D3 cold)
13:51 < db> ACPI is the holder of information about whether STR loses OHCI power or not, etc...
13:51 < benh> mochel: though in fact, what ATI does on MacOS is not to bother
13:51 < db> ... so it'd be good to have that behaving ...
13:51 < benh> mochel: they just D2 it on suspend
13:51 < jordan> alan: I think that some bit of that policy would need to be hard coded in the platform, with
the policy able to over ride it at any given time
13:51 < benh> mochel: and on resume, they check the chip state
13:51 < benh> mochel: if it's still in D2 or if it is in power-on reset state
13:51 < db> jordan: yes.
13:51 < pavelm> benh: I do not think that different PM path is taken, btw...
13:51 < benh> mochel: in which case, they take appropriate action
13:51 < benh> so we might not _need_ that info from the system in that case
13:51 < db> ben: not all devices support D2 ! You lucky ATI-owning dog ... ;)
13:52 < benh> USB could do that too, that is just suspend
13:52 < benh> db: D2 is specific in this case
13:52 < alan> mochel and jordan: Are you agreeing with me? Nothing says platform policy can't be hardcoded.
13:52 < benh> db: what I mean here is that it could have been a local PM register on the device that "suspends"
it
13:52 < db> ACPI hard-codes platform policy... once we can get at it!
13:52 < benh> db: D2 by itself means nothing
13:52 < benh> db: it's just that ATI uses it to represent a suspend state
13:52 < benh> anyway
13:53 < benh> I think in most case, the driver can just see what happened "afterward"
13:53 < db> ben, are you talking about weakness of definition for pci states? agreed, but even so d2 may be
illegal.
13:53 < benh> on wakeup
13:53 < alan> benh: D2 means _something_ to the hardware, even though you may not know what!
13:53 < jordan> alan: I do agree with much of what you are saying
13:53 < benh> alan: ok, wait, I'm not talking about D state
13:53 < benh> that was just part of my example, but not my point
13:53 < benh> I wrote about what I think of the bad definition of D states already :)
13:53 < benh> anyway
13:53 < benh> what I mean is
13:54 < benh> most drivers can probably ignore the details of the state
13:54 < benh> like wether they'll lose clock or power
13:54 < benh> and just assume they'll lose clock
13:54 < benh> that is, just unter some kind of suspend state. For example, USB could always just suspend
13:54 < benh> if it loses power ? too bad ... it will notice it at resume anyway and take appropriate actions
13:54 < benh> no need to "know" it in advance
13:54 < benh> in radeonfb, I don't need to know it
13:55 < db> benh: that doesnt work well for USB in all cases
13:55 < benh> the only reason it's useful for me eventually to know it is that it would allow me to say "oh, I
don't know how to resume that card from power on reset" and reject sleep
13:55 < benh> but that's a rare case
13:55 < alan> But some drivers will need to know. Otherwise why have a new type of system state?
13:55 < benh> db: can you give me an example ?
13:55 < db> specifically, usb_suspend implies power is saved
13:55 < benh> alan: some drivers will need to know things like: ok, this is suspend to disk, don't spin down
the disk
13:56 < benh> alan: but then, we already discussed all of that a while ago, and ended up defining a few states
13:56 < db> another example is in HCDs that need to know whether to disable clocks or not, power down ports or
not
13:56 < benh> I really suggest we stick to them for now
13:56 < pavelm> benh: Actually they should get PMSG_FREEZE, so you get "not spining disk down" pretty much free.
13:56 < jordan> benh: I agree
13:56 < benh> db: this is all platform specific, I'm not sure it can be put anywhere but in the drivers
themselves, inside appropriate #ifdef ARCH_*
13:56 < benh> pavelm: I know, that was the point of my example
13:57 < pavelm> benh: ok.
13:57 < alan> Then we are all agreed? :-)
13:57 < db> benh: no, it's specific to suspend level.
13:57 < benh> db: USB may call a platform usb_arch_need_powerdown(blah) or somerthing like that if you want to
make it nicer
13:57 < benh> db: no, becaue if you go this way, you'll have to define 73487256376523 suspend levels
13:58 < benh> db: for all the possible embedded and clock net combinationzs
13:58 < benh> alan: no, db never agrees you know :)
13:58 < db> benh: no, just add platform hooks to the existing levels ... or to platform-specific suspend states
13:58 < db> alan: what are you wanting agreed?
13:59 < benh> we can't define those hooks without a precise idea of what data you need from the platform
13:59 < alan> db: At this point I'm not sure any more... Following these ping-pong conversations isn't easy.
13:59 < benh> which in this case seem fairly specific to the USB controller ...
14:00 < db> benh: I said that ages ago. Specifically, clock and power settings.
14:00 < db> benh: so if you're just proposing those go into the mapping functions, that's pre-agreed. ;)
14:00 < benh> but even clock ...
14:00 < benh> look for example, USUB
14:00 < db> benh: HOWEVER ...
14:00 < benh> it has the PCI clock and the 50Mhz
14:00 < benh> every device has different "clocks"
14:00 < db> benh: we still need to expose things like suspend level to those mapping functions, and
14:00 < benh> same goes for power
14:01 < benh> this is totally platform specific _and_ device implementation specific
14:01 < benh> I don't see how you can expose that on a system suspend level
14:01 < db> we currently discard that in the driver notifications, so we're screwed
14:01 < pavelm> db: driver notfications should get full pm_message_t.
14:01 < db> benh: example, on platform X the ACPI tables say that at S3 device N goes to state (ACPI) D2
14:02 < benh> unless you can come with a concrete (code) example on how properly put those infos in there, I'm
dubious
14:02 < db> benh: but device N+1 goes to D3 (poweroff)
14:02 < db> and on platform Y it's different according to ACPI, and on platform Z there's no ACPI but it's like
X
14:02 < benh> db: are the D states properly defined by ACPI ?
14:02 < benh> db: trhat is, D2 means PCI clock removed and D3 means power goes off ?
14:03 < benh> db: I agree to have a platform-wide function that return that information for a given device
14:03 < benh> db: but it's fairly bus specific
14:03 < db> benh: D states definedin ACPI ... not PCI-Dstates. PCI D3 ~= ACPI D2 (some state lost)
14:04 < db> benh: and PCI D3cold == ACPI D3 (power lost) [ previous meant D3cold]
14:04 < alan> benh: ACPI is _extremely_ specific. The tables are made up specially for each motherboard by the
manufacturer.
14:04 < benh> ok, I don't have the ACPI spec, but it would be interesting to look at these in more details,
maybe we can re-use those definitions
14:04 < db> benh: if the function is takes the struct device, the implementation can use buses or devices or
whatever
14:04 < pavelm> benh: You don't want to look at ACPI spec.
14:04 < pavelm> benh: trust me. 500pages of crap written by comitee.
14:04 < jordan> unless you need lots and lots of toilet paper
14:04 < db> benh: exactly why I mention ACPI on occasion! but just look at the two early chapter overviews
14:05 < benh> pavelm: I don't, which is why I would appreciate if db gave me a quick overview of those states :)
14:05 < benh> db: what is D2 exactly ? "lose some state" is quite vague
14:05 < db> pavelm: just copy the relevant chapters, and use the API specs in the outhouse.
14:06 < db> benh: read the spec. Preserves power. D1 doesn't lose any state.
14:06 < benh> db: ok, but wait
14:06 < benh> db: those "tables" indicate what will happen to the device, not what the driver is supposed to
do, right ?
14:06 < jordan> D2: In general, D2 is expected to save more power and preserve less device context than D1
14:06 < benh> db: so D1 means nothing, or rather, means "won't do anything with this device"
14:06 < alan> ACPI isn't going to be a magic answer. It's not always available and doesn't cover all devices
(just those on the motherboard).
14:06 < jordan> Copied right from the spec - page 21
14:06 < db> benh: that's a question Len would be better at answering, but I think you're right
14:07 < benh> ok
14:07 < db> benh: re D1, that's power reduced compared to D0 fully operational
14:07 < benh> alan: yah, I'm just trying to understand their state definition
14:07 < pavelm> What len did was putting acpi hook in pci_choose_state.
14:07 < benh> db: ah
14:07 < jordan> alan: exactly - plus ACPI won't help us at all with run time power management
14:07 < benh> db: but what does "power reduced" mean ... ?
14:07 < benh> that can be a lot of different things :)
14:07 < benh> BTW
14:07 < pavelm> ... he did it wrong (retyped between pci_power_t, pm_message_t and int), but other than that...
arch hook there makes sense.
14:07 < benh> I looked at Darwin a bit, and Apple does something fun
14:08 < benh> they have an array of states per driver (hehehe)
14:08 < benh> and in there, they actually put the power requirements of each state
14:08 < benh> in mA
14:08 < db> pavelm: agreed, arch hook does make sense there, we'd discussed that last fall ... :)
14:08 < mochel> benh: isn't exposed in the PCI PM capabilities?
14:08 < benh> the system wide suspend maps to driver states in various weird ways tho, I haven't fully
decrypted their stuff, it's a mess
14:08 < benh> mochel: not necessarily
14:08 < pavelm> benh: can we even get the mA information from manufacturers?
14:09 < db> benh: so presumably deeper device suspend states require less power ... ;)
14:09 < pavelm> benh: actually mA is stupid. mW would be a bit better.
14:09 < jordan> benh: so basically, each power state is a given total power consumption value, and the OS picks
the various states that hit that target consumption level?
14:09 < benh> well, a lot of drivers I've seen just put 0 in those tables though :)
14:09 < benh> pavelm: I think it was mW, sorry, typo
14:09 < pavelm> :-)
14:09 < db> pavelm: mW = mA * Volts or similar, yes?
14:09 < mochel> benh: be careful; some companies have legal restrictions on what you can with the method by
which they do things
14:10 < pavelm> db: except that you don't know the Volts ;-)
14:10 < benh> oh, I'm not suggesting we go this way
14:10 < alan> I'm not sure that power consumption rates will be useful except in very specific circumstances
(like USB).
14:10 < benh> anyway
14:10 < db> pavelm: if it's a 3.3V PCI card you know it's not 5V ... :)
14:10 < benh> I have to run
14:10 < pavelm> alan: It certainly gives idea to the user.
14:10 < db> pavelm: why would you want to know specific power usage levels?
14:10 < mochel> to know how long your battery will last
14:10 < pavelm> alan: Like "don't bother with D3 on this card, it only saves 0.001W".
14:11 < mochel> the power consumption is only piece..
14:11 < db> pavelm: when the board takes 0.010 W, that's a lot ... ;)
14:11 < mochel> if a card is generating constant interrupts in D0 and keeping the cpu out of C3, then it would
make sense to put it into D3
14:12 < pavelm> db: well.. I'd like to see that kind of board running linux ;-).
14:12 < jordan> alan: I think people are sort of agreeing that a platform specific manager is a good place to
put these mappings
14:12 < jordan> alan: which I think was your original argument
14:12 < pavelm> benh: bye!
14:12 < alan> jordan: Yes. I was trying to figure out how to make it work.
14:13 < mochel> jordan, alan: there seem to be two ways of approaching that..
14:13 < mochel> 1) put platform-specific information in each driver that runs on that platform
14:13 < mochel> 2) put device-specific information for each device capable of running on a platform.
14:13 < db> mochel: (1) is easy for platform-specific drivers, and very natural ... e.g. driver only works on
this SOC
14:14 < db> mochel: I don't quite follow (2), elaborate please?
14:14 < mochel> db: it's unrealistic, really :)
14:14 < mochel> you have a list of all pci devices that would be present on e.g. x86, and lives in arch/i386/
14:15 < mochel> the driver calls platform_map_me() which accesses that table..
14:15 < db> mochel: I was thinking (3) arch/..../board-foo.c holds the helper function, with all the board's
devices
14:15 < pavelm> mochel: actually platform_map_me() seems to be ACPI solution...
14:16 < db> mochel: or equivalently, platform_data for the devices provides hooks that were initialized there
14:16 < mochel> db: that could be a default for on-board devices
14:16 < jordan> db: and (3a) - bus specific helpers for the more generic devices that are targeted at many
different platforms
14:16 < db> mochel: or equivalently: ACPI does it
14:16 < db> jordan: I might call that a (4) not a (3a). Do you have a (3b)? :)
14:17 < jordan> db: heh
14:17 < mochel> jordan: seems like those would most likely want (1)
14:17 < jordan> db: I was thinking that 3) would be almost impossible on a generic x86 platform
14:17 < pavelm> Actually, there's someting I wanted to make clear.
14:17 < db> yes, bus-specific helpers can work, but they rely on platform hooks I think.
14:17 < db> jordan: isn't that equivalent to the ACPI tables?
14:17 < pavelm> I keep hearing "deep sleep" and "big sleep"... but those are not really system sleep states.
14:18 < pavelm> CPU may be sleeping but system is not.
14:18 < pavelm> It is similar to ACPI C3, but it is very different from PC suspend-to-RAM.
14:18 < jordan> db: I would say for most cases yes
14:18 * mochel wonders if we can ever finish 1 conversation before starting another :)
14:18 < jordan> db: but I'm all for ways for buses, drivers and platforms to control their own destiny
14:19 * db wonders about ever finishing even one ...
14:19 < benh> bah, back
14:19 * alan wonders if we'll ever actually settle on anything at all...
14:19 * gregkh has odds that you all never will :)
14:19 < db> pavelm: you may be right. Does that suggest something?
14:19 < benh> note that this power allowance thing may be more significant/important than I though
14:19 < benh> on laptops, you don't really care... if you eat too much power, you'll battery will run flat
during suspend
14:20 < benh> but desktops are a different matter, at least Apple's are
14:20 < benh> on suspend, the main power supply is off (and the fans are off)
14:20 < benh> so you only get a limited amount of power from the power supply, just enough to keep the RAM up
14:20 < pavelm> db: perhaps it should be handled more like ACPI C3, not like ACPI S3....
14:20 < benh> and a few components, like the USB in suspend state ... and I think they provide some "sleep
current" to PCI as well
14:20 < benh> or there is some room for it at least
14:21 < pavelm> benh: Can you actually select which devices are powered on Apple desktops?
14:21 < benh> pavelm: no
14:21 < pavelm> benh: IIRC on PC desktops you just don't care.
14:21 < benh> the PMU does it all
14:21 < pavelm> benh: you put machine into suspend-to-RAM, and because only voltage on sleeping lines remains
14:21 < pavelm> benh: all devices you forgot about are powered down automagically.
14:21 < benh> yup
14:21 < benh> well, not all
14:21 < db> pavelm: that's one way to take the VST work, yes.
14:21 < benh> Apple ASICs aren't
14:22 < benh> you have to turn cells off yourself in there
14:22 < benh> but I already have code doing that
14:22 < pavelm> benh: ...ouch, ok
14:22 < db> pavelm: nonetheless, folk seem to want to be able to force the system into "deep sleep"
14:23 < pavelm> I wonder how is it possible that radeons eat batteries in thinkpads...
14:23 < pavelm> ...perhaps they don't have separate power and "sleeping power" lines?
14:23 < db> pavelm: for things like cellphone "off"
14:23 < pavelm> db: Yes, that's system state...
14:24 < pavelm> db: but then you just want to turn everything off, and cellphone is not really usefull any
more, right?
14:24 < pavelm> db: BTW are there Linux cellphones already?
14:24 < benh> pavelm: they aren't powered off by ACPI
14:24 < pavelm> db: I know motorola makes something, but I can't buy it here.
14:24 < db> pavelm: I meant "off" as in "not talking" etc. Different degrees of "off". ;)
14:24 < benh> pavelm: but if the thinkpad folks enable my code to put them into D2 state, they eat a lot less
14:25 < benh> db: yah, "off" is typically deep sleep
14:25 * mochel wonders if we could move the whole policy decision to userspace
14:25 < db> pavelm: Linux cell phones ... look in Taiwan, Japan, Korea for most of them (as I understand)
14:25 < mochel> ...make it happen once on startup
14:25 < jordan> mochel: isn't that one of the big reasons why we are here? :)
14:25 < db> pavelm: but not downstairs from where I am. Nope.
14:25 < benh> db: maybe we can get away with modelling system states around a few ones like that: deep sleep,
doze, idle, running ?
14:25 < benh> db: that would cover most needs, and the helpers will sort-of help devices figure out what to do
14:26 < jordan> It doesn't have to be a system state, in my opinion
14:26 < pavelm> db: so one non-existing linux cellphone could not find a way to my table? ;-)
14:27 < db> Well, I've not done that much with them... but the folk building these tend to be using MontaVista
Linux
14:27 < benh> ok, gf not here yet, so I have a bit more time :)
14:27 < benh> db: that means they'll have a 3 yrs outdated kernel at least anyway :)
14:27 < db> which means "DPM", since there's lots of ancillary software that's not (yet) on 2.6
14:27 < benh> db: no need to worry :)
14:28 < benh> oh well, she's here, I'll have to run
14:28 < alan> It sounds like there will have to be a large table somewhere
14:28 < benh> bbl
14:28 -!- benh [^benh@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx] has quit [Quit: Client exiting]
14:28 < jordan> afaik - DPM only does voltage and frequency stepping at this point
14:28 < alan> that summarizes which devices get power and clock in which system states.
14:28 < alan> Or the equivalent information has to be available through, e.g., ACPI.
14:29 < mochel> alan: and that table will have to include information for each state on each platform
14:29 < alan> And it's all going to be platform specific.
14:29 < db> jordan: you may be right, but "operating points" are kinds of system power states (like suspend
states)...
14:29 < alan> Sounds like a mess. How can it be implemented sanely?
14:29 < jordan> db: yeah, its more a pseudo system state
14:29 < db> alan: often rules will let you avoid big tables.
14:30 < mochel> alan: push it to userspace
14:30 < mochel> we only need to figure it out once for each system state
14:30 < db> ACPI manages huge tables for x86 ...
14:30 < mochel> we have a startup script that determines the platform and writes to a sysfs file which state to
enter for each system state the platform supports
14:31 < pavelm> mochel: that's okay for states "on", "standby", "str", "std"; but I guess cellphone-like
devices have 1234712039481723049 or so system states.
14:31 < jordan> mochel: take a step further - the same script would also define psuedo states that would let
you set the system into a "cell phone off" mode if you want
14:31 < mochel> pavelm: they sound like they only have a few
14:31 < mochel> jordan: absolutely
14:31 < jordan> I don't like using the term system state, because on x86 that implies ACPI/APM
14:31 < db> mochel: these not-so-mythical cell phones don't have choice-of-platform. :)
14:31 < mochel> db: ?
14:32 < mochel> db: oh, the devices only operate on that platform
14:32 < mochel> ?
14:32 < db> mochel: why bother with a startup script when there are no choices, that's all. script == overkill.
14:32 < pavelm> mochel: Really? They seem to have 1001 states that are basically 'on' but with various parts
(camera, radio, microphone, ...) turned off, and with different clocks ticking or not.
14:32 < mochel> db: that's fine. the driver, when loaded, can provide a default.
14:32 < mochel> pavelm: i'm not an expert on cell phone states. are you?
14:33 < db> pavel: that seems a fair statement of what the hardware can do. Software may not use it well yet.
14:33 < alan> mochel: Can a startup script make such determinations even for devices that aren't present at
boot time?
14:33 < db> mochel: yes, that's back to what I said earlier.
14:33 < mochel> alan: no, but the hotplug scripts can
14:33 < db> ...
14:33 < db> so what did we accomplish today?
14:33 * lenb exits lurk mode
14:34 < mochel> db: so there's no problem with the default.
14:34 < mochel> db: and, it can be overridden by more device-rich platforms, like x86 and ppc
14:34 < alan> db: Well, we argued a lot about pm_message_t... :-)
14:34 < db> mochel: right, no need to change the default there, platforms can do what they need
14:34 < mochel> lenb: where does one find the acpi spec these days?
14:34 < db> alan: is that accomplishment? :(
14:34 < mochel> db: agree
14:35 < mochel> point is that the drivers can be told "enter this state now" and never have to look anything up
14:35 < mochel> or do any switch statements
14:35 < mochel> etc
14:35 < lenb> mochel: http://www.acpi.info
14:35 < mochel> lenb: it's not responding
14:35 * jordan skips the obvious S3 joke here
14:36 < lenb> moschel: i noticed that too -- shall i e-mail you a copy?
14:36 < lenb> 600 pages, 3MB in PDF format
14:36 < db> mochel: yes, though what state? :)
14:37 < alan> Drivers that don't understand a new state can just assume they won't have power or clock.
14:37 < alan> Is that a reasonable default choice?
14:37 < mochel> lenb: it's ok. i'll wait until it comes back.
14:37 < db> alan: for many drivers it's a reasonable default, not all
14:37 < mochel> lenb: i still have a 2.0 copy holding open a door somewhere :)
14:37 < lenb> pretty good monitor stand, it is:-)
14:38 < jordan> good spider squisher too
14:38 < jordan> gets a good "whack" out of it
14:38 < db> I only have ACPI spec in electronic form. Hard to use when bugsquishing, too bad.
14:38 < mochel> db: for all of them
14:39 < alan> I'd like to see posted an example of an API (one that uses Pat's sysfs
14:39 < alan> idea or something else, doesn't matter) showing how a driver
14:39 < alan> can make these determinations.
14:39 < lenb> pavelm: why is pm_message_t have to wait for post 1.6.12?
14:39 < lenb> 2.6.12
14:40 < db> mochel: it can be done that way, yes
14:40 < pavelm> lenb: u32 vs. pm_message_t needs to be sorted first. Then I can switch pm_message_t to struct.
And akpm wanted 2.6.12 soon.
14:41 < lenb> pavelm: didn't akpm volunteer to stuff all the mm stuff into 2.6.12 immediately on linus' return?
14:41 < pavelm> OTOH if I leave it in current state, *someone* will look inside pm_message_t.
14:41 < lenb> pavelm: yeah, like we just did:-)
14:42 < pavelm> lenb: he did, but in akpm's tree, pm_message_t is typedefed t
14:42 < pavelm> lenb: he did, but in akpm's tree, pm_message_t is typedefed to u32
14:43 < pavelm> so maybe I should switch to struct ASAP, but that will probably delay 2.6.12....
14:43 < jordan> alan: so now you're thinking the platform translation code should be in the drivers?
14:43 < alan> jordan: No, just the opposite. But I'm wondering how the drivers can
14:43 < lenb> pavelm: I think with the number of pm changes we have in the pipe, we need to be aggressive and
selfish WRT the upstream tree.
14:43 < alan> express their platform-specific requirements to a platform-specific
14:44 < alan> helper. Especially while minimizing the amount of platform-specific
14:44 < alan> code in the drivers.
14:44 < lenb> else we'll spend a lot of time waiting, instead of release-early-and-often...
14:44 < jordan> alan: I think that will work itself out, especially since many of the drivers this group cares
about are platform specific to begin with
14:45 < pavelm> lenb: well...
14:45 < db> lenb: AKPM seemed like he might yank the APM patches if the PCI stuff didn't get resolved...
14:45 < pavelm> lenb: benh agrees with you.
14:45 < db> lenb: in email late last night on linux-usb-devel
14:45 < pavelm> lenb: but I don't want to see my head rolling away...
14:45 < alan> jordan: But _all_ drivers have to worry about this, one way or another,
14:45 < mochel> lenb: i agree with you
14:45 < alan> or else they have to default to no power, no clock.
14:45 < jordan> alan: true, but when you get down to it,it seems like they fall into some nice paterns
14:45 < mochel> i think it's more important to have something that a) works and b) we agree on than to have
code merged upstream
14:46 < mochel> the upstream merge will happen eventually, and in a lot smoother fashion if we're all on the
same page
14:46 < jordan> alan: The way I see it, probably about 90% of PCI devices will act the same way
14:46 < alan> mochel: Amen!
14:46 < mochel> and aren't always playing 'catch-up'
14:47 < alan> jordan: David has a better idea than I do of what the requirements will be.
14:47 < lenb> moshel: yes, but that is okay, because we don't have many customers yet
14:47 < pavelm> mochel: struct vs. struct * is really a detail, and current API is *very* easy to missuse.
14:47 < lenb> when will i learn to type your name anyway...
14:47 < mochel> lenb: use tab-completion :)
14:48 < lenb> mochel mochel mochel mochel
14:48 < db> ... we seem about done ...
14:48 < mochel> alan: there will always be platform-specific devices
14:48 < pavelm> is there way to get latest akpm's tree?
14:48 < mochel> alan: those can specify whatever defaults they want
14:49 < alan> mochel: I'm not worried about platform-specific devices.
14:49 < pavelm> not 2.6.12-mm1 but 2.6.12-mmbk?
14:49 < mochel> alan: the problem only lies in devices that can be used on a variety of platforms *and* support
a variety states
14:49 < db> pavelm: 2.6.12-rc1-mm2 is out now, for almost 12 hours
14:49 < alan> mochel: Exactly.
14:49 < db> mochel: which is primarily PCI ...
14:49 < mochel> and usb
14:49 < mochel> for those, we can use a look up table
14:50 < db> mochel: ... and with usb we have complications of MANY different implementations of OHCI, UHCI, EHCI
14:50 < db> mochel: usb doesn't have variety, except at the level of OHCI/UHCI/EHCI
14:50 < mochel> db: ah, ok
14:50 < db> mochel: ... so USB doesn't need tables except those corresponding to what ACPI does
14:51 < db> mochel: ... or eg. the SL811 code that copes with suspend-powered and suspend-unpowered ..
14:51 < alan> db: Would most of those USB tables end up being used by the platform-specific
14:51 < db> mochel: ... and so on. All HCD issues. Alan and I have much pain to testify to, w.r.t. PM and USB
HCDs.
14:51 < alan> part of the drivers, anyway?
14:52 < jordan> alan: I think that we can easily use generic settings to our advantage - the important thing is
making the settings easy to change
14:52 < db> alan: pretty much. That means usb/core/hcd-pci.c , and all the non-PCI ones.
14:52 < jordan> alan: be it by user, driver or platform
14:52 < alan> Okay. I'm convinced the requirements will be small enough
14:52 < alan> and localized enough not to cause any trouble.
14:53 < pavelm> lenb: Okay, so I'll prepare "switch pm_message_t to struct" patches tommorow, against -rc1-mm2.
It is going to be small. And lets see if it is possible to push up.
14:53 < lenb> pavelm: that would be great
14:54 < alan> Does anyone want to volunteer to post a summary of the discussion?
14:54 < mochel> pavelm: you could easily change it to a pointer..
14:54 < pavelm> lenb: btw smp/swsusp now works for me, using cpu hotplug.
14:55 < alan> mochel: Smile when you say that!
14:55 < lenb> pavelm: yeah, i saw your e-mail. Shaohua did the same thing
14:55 < db> pavelm: what mochel said.. :)
14:55 < pavelm> mochel: I could... but then it may only ever be passed pointer to static struct, and thats kind
of ugly.
14:55 < lenb> pavelm: do you think it is kosher to have system suspend on SMP depend on HOTPLUG_CPU?
14:55 < db> pavelm, lenb: hey, that gives me another machine to test PM on! ;)
14:56 < mochel> pavelm: that's not ugly
14:56 < pavelm> lenb: Yes. CPU Hotplug needs testing, and I'm not going to duplicate it.
14:56 < mochel> pavelm: you could also make it const, since you know it will never change
14:56 < mochel> lenb: make it SELECT It
14:56 < db> constness for readonly == good.
14:57 < lenb> mochel: yes, I'd like to see a select also
14:57 < lenb> otherwise people will not understand why their config doesn't work when they asked for eveything
they thought they needed.
14:57 < pavelm> lenb: you could have select if I were not afraid of touching Kconfig.
14:58 < mochel> pavelm: you should post the smp patches to linux-pm :)
14:58 < pavelm> mochel: and that const struct foo should better not be inside module.
14:58 < mochel> pavelm: it won't be; it's coming from the core
14:58 < pavelm> mochel: Heh, explain that in bold letters and someone will *still* get that wrong.
14:59 < lenb> pavelm: select works okay for me -- but somebody got pissed when I made PCI_GOANY select ACPI:-)
14:59 < pavelm> mochel: and you'll get very nasty crash in such case.
14:59 < mochel> pavelm: don't understand
14:59 < mochel> pavelm: how could it come from module?
14:59 < mochel> pavelm: it's a value passed from the core to each driver..
14:59 < pavelm> Why not.
14:59 < pavelm> It is value passed from whoever does the suspend to each driver.
14:59 < mochel> modules don't initiate power transitions
14:59 < alan> mochel: A driver wants to suspend a child device.
14:59 < pavelm> If you do the suspend from apm....
14:59 < mochel> ah
15:00 < mochel> ok
15:00 < pavelm> apm can be modular, iirc.
15:00 < pavelm> I'd rather keep it simple.
15:00 < mochel> so what if it's coming from a module? we can't have const values in modules?
15:00 < pavelm> then the driver stores pointer somewhere and then you rmmod.
15:00 < pavelm> drivers want to store pm_message_t.
15:01 < mochel> they do?
15:01 < mochel> why?
15:01 < pavelm> they use it as a "what a state I'm in".
15:01 < mochel> but in that case, they'd be storing a device state
15:01 < mochel> but what they got passed was the system state
15:01 < pavelm> yes, but the way drivers all over the tree are coded is that they store pm_message_t.
15:02 < mochel> haven't we all agreed that devices shouldn't treat system states == device states?
15:02 < pavelm> and I'd really really like to avoid auditing all the drivers again.
15:02 < mochel> fine, i'll do it :)
15:03 < mochel> pavelm: send your patches to linux-pm and i'll fix them up
15:03 < pavelm> mochel: that does not cut it, I'm afraid.
15:03 < mochel> pavelm: could you please also send those that are in -mm too?
15:03 * alan needs to go home and start cooking dinner. See you next time!
15:03 < jordan> later alan
15:03 < mochel> pavelm: if it's a matter of work, then i'll take the time and fix it up
15:03 < pavelm> mochel: You must get -mm patches, well, from -mm; I did not store them separately.
15:04 < mochel> pavelm: what did you send to akpm?
[16:15] [mochel(+i)] [3:#pm(+n)] -- more --
15:04 -!- alan [~stern@xxxxxxxxxxxxxxx] has quit [Quit: Leaving]
15:04 < pavelm> mochel: I'm not sure if it is "simple matter of work". I think structure by value is actually
right thing to do.
15:04 < pavelm> mochel: I sent no patches in last 12 hours, so -rc1-mm2 should be safe.
15:04 < pavelm> mochel: but you'll get those patches, I need helping hands.
15:05 < mochel> bah, i'd prefer just the single patches
15:05 < mochel> but, oh well
15:06 < pavelm> akpm keeps them separate, and they are trivial. You can just grep for my name...
15:07 < pavelm> Ok, take a look at your inbox, there should be "switch pm_message_t to struct" patch there.
15:08 < pavelm> that's the one I'd like to merge.
15:08 < pavelm> It is important to turn pm_message_t into something that is incompatible with int, or people
will introduce bugs very quickly.
15:08 < mochel> pavelm: ah, that's the one i commented on earlier
15:08 < pavelm> mochel: Yes, its that's one.
15:09 < pavelm> for the record, removing flags for now is completely okay with me.
15:09 < mochel> ok, we've gone over 2 hours
15:10 < mochel> has anyone logged this?
15:10 < pavelm> (not me)
15:11 < pavelm> so... what to do with the patch? You seemed pretty much okay with "switch pm_message_t to
struct"... and it solves some real problems.
15:12 < mochel> pavelm: well, the things i expressed in the email
15:12 < mochel> pavelm: but, i agree that it should be passed by its pointer
15:12 * db still agrees that pointers are better
15:12 < db> gotta go ... bye!
15:13 -!- db [~db@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx] has quit [Quit: using sirc version
2.211+KSIRC/1.3.11]
15:14 < pavelm> mochel: pointer still allows you to do if (!state) ...
15:14 < pavelm> mochel: and some current code does that.
15:14 < pavelm> mochel: anyway if you are willing to go over the drivers... how long do you think it would take?
15:14 < mochel> pavelm: dunno
15:16 < pavelm> mochel: I think it is lot of auditing for a very little gain, and new mess is going to be added
as long as you are working.
15:16 < mochel> pavelm: what new mess?
15:16 < pavelm> mochel: plus struct is really easier to work with, no need to think about modules, no need to
warn people that it needs to be static.
15:17 < pavelm> mochel: "new mess" == drivers are becoming "pm aware" and will do stuff like if (!state) or int
my_state = state [for pm_message_t state].
15:18 < mochel> w/e
15:18 < mochel> it has to be dealt with
15:18 [Users #pm]
15:18 [ gregkh] [ jordan] [ lenb] [ mochel] [ nigel] [ pavelm]
15:18 -!- Irssi: #pm: Total of 6 nicks [0 ops, 0 halfops, 0 voices, 6 normal]
15:19 < mochel> we need to work together and help each other out
15:19 < mochel> not be cynical and keep sabotaging each other
15:19 < pavelm> eh?
15:21 < pavelm> I don't understand your comment. driver authors are not trying to sabotage anyone; but old
interface is just too easy to misuse.
15:21 < mochel> i'm not talking about driver authors
15:21 < mochel> i'm talking about us
15:21 < mochel> creating and propogating interfaces that we don't agree on creates trouble for everyone
15:21 < pavelm> then please explain it.
15:21 < pavelm> we did agree on them half a year ago and difference on what you want is very small.
15:22 < mochel> when someone offers to help, and you tell them that it's pointless and that there will just be
a 'new mess' to deal with, that's just being cynical
15:22 < pavelm> pointer is bad because it is also easy to misuse.
15:22 < mochel> passing a struct by value is bad programming practic
15:22 < pavelm> it is 2 integers. I don't know where you got that mantra, but its wrong in this case.
15:24 < mochel> and if it grows?
15:25 < pavelm> if it grows we are doing something horribly wrong, anyway.
15:25 < pavelm> I could imagine char *name added to it for runtime suspend, but thats about all.
15:25 < mochel> what if something is added that you can't imagine?
15:25 < mochel> you can't claim to imagine all of the possibilities
15:25 < mochel> that's ludicrous
15:26 < mochel> you *will not* know what happens to the size or format of that structure
15:26 < mochel> there is no way to know
15:26 < mochel> we could decide that it's a bad name, or come up with a new interface
15:26 < mochel> you cannot predict the future or make any claims to it
15:26 < pavelm> what kind of argument is that.
15:26 < pavelm> of course I can not predict the future.
15:27 < pavelm> pointer may grow to 128bytes next year.
15:27 < pavelm> or something.
15:27 < mochel> ..so you cannot claim to know what will happen to that structure
15:27 < pavelm> but that does not mean we should not do the right thing *now* and that is to pass it by value.
15:27 < mochel> that is not the obvious right thing
15:28 < mochel> it's actually less efficient to pass by value
15:28 < mochel> you're pushing 8 bytes on x86, instead of just 4
15:28 < pavelm> ...for two ints who cares.
15:28 < mochel> so it's wrong in that sense as well
15:28 < mochel> i know lots of people that care
15:29 < pavelm> you are trading 4 bytes for possibility to get it wrong very easily.
15:29 < pavelm> current code does if (!state), and if you put pointer there, compiler will not stop that.
15:29 < mochel> so, we audit the drivers and make sure they don't get it wrong
15:29 < mochel> so, we fix them up
15:29 < pavelm> yes, but that does not get out-of-tree drivers and that does not get *new* drivers.
15:29 < mochel> we have to fix up the drivers no matter what do deal with the paramters
15:29 < mochel> we do not care about out of tree drivers
15:30 < mochel> and we write sane documents saying how to use it for new drivers
15:30 < pavelm> interface that is hard to mis-use is bonus.
15:30 < mochel> bogus?
15:30 < pavelm> documents does not work, type enforcement does.
15:30 < pavelm> We are esentially trading 4 bytes on stack for better interface.
15:30 < pavelm> I'll go for better interface any day.
15:30 < mochel> no, we are trading N bytes on stack for better interface
15:31 < mochel> how do you prevent people from using the system state as the device state?
15:31 < pavelm> We are trading 4 bytes now.
15:32 < pavelm> I don't think using system state as the device state is *that* wrong, and struct * will not
solve that either.
15:32 < mochel> but, how do drivers know not to do that?
15:32 < pavelm> you can tell them in documentation ;-).
15:32 < mochel> but you said:
15:32 < mochel> 15:30 < pavelm> documents does not work,
15:32 < pavelm> I don't think this can be enforced by type-checking.
15:33 < mochel> right, so you have to explain some things in documents and by setting good examples
15:33 < mochel> at some point, this stuff will be documented in O'Reilly books
15:33 < mochel> and people will refer to as gospel
15:33 < pavelm> Yes, and I'll enforce as much as I can with type checking.
15:33 < pavelm> I believe that's right thing to do.
15:33 < mochel> there are certain rules that people must know
15:34 < mochel> and with proper review of drivers, you catch the bugs when they go into the kernel
15:34 < pavelm> It is still wrong interface. If it can be checked by compiler, it should be checked by
compiler. ANd cost is rather small in this case.
15:34 < pavelm> Documentation is good, but documentation *and* type checking is even better.
15:35 < mochel> we're splitting hairs here
15:35 < pavelm> Okay, what about this one.
15:35 < mochel> ask someone you know and trust their opinion about what to do
15:35 < mochel> in fact, take a survey
15:35 < mochel> passing a struct by value is wrong. period.
15:35 < pavelm> I kill flags field. That makes it ninja mutant structure with one field, 4bytes, very usefull
for type checking.
15:36 < mochel> ..which makes the entire patch moot
15:36 < mochel> we really do have bigger fish to fry
15:36 < lenb> jordan: "alan: exactly - plus ACPI won't help us at all with run time power management"
15:36 < lenb> huh?
15:37 < lenb> The D-states are all run-time.
15:37 < pavelm> mochel: designing interfaces that are trivial to misuse is wrong. period.
15:37 < pavelm> mochel: that makes entire discussion useless.
15:38 < mochel> pavelm: that doesn't mean the first alternative you think of is correct
15:38 < mochel> pavelm: let's try a little harder and come up that a) works and b) we agree on
15:39 < mochel> s/come up/come up with something/
15:39 < jordan> lenb: heh
15:39 < pavelm> page.h:typedef struct { unsigned long long pgd; } pgd_t;
15:39 < pavelm> page.h:typedef struct { unsigned long pgd; } pgd_t;
15:39 < pavelm> mochel: you have to come up with some disadvantage, first. 4 bytes on stack does not cut it,
sorry.
15:40 < lenb> pavelm: how about "common practice within the kernel"?
15:40 < mochel> pgd_t is a cross-platform value that could have different meaning on different platforms
15:40 < pavelm> see pgd_t above ;-)
15:40 < jordan> lenb: I guess I was thinking that when it came to run-time power management, we do it all on
our own without grocking any ACPI registers
15:41 < mochel> pgd_t is the exception, not the rule.
15:41 < lenb> jordan: depends...
15:41 < jordan> But the definitions are still there I guess
15:41 < mochel> just because 1 piece of code does it doesn't mean all pieces of code should do it
15:41 < jordan> lenb: can you use ACPI to put a device in a state without changing the system state? I was
told you can't
15:41 < lenb> jordan: it is actually a _bug_ that we don't evaluate the ACPI device methods, not a feature, at
run-time
15:42 < lenb> jordan: i believe that is false
15:42 < pavelm> you are arguing "common practice", but it is not so common, that seems.
15:42 < jordan> lenb: well crap, now I have to go kill somebody
15:42 < mochel> what is not so common?
15:42 < pavelm> well if pgd_t can do it, there should be no problem for pm_message_t to do it.
15:43 < mochel> i'm sorry, but that's the logic of a 7 year old
15:43 < pavelm> if you are arguing "common practice", that's very valid logic.
15:43 < mochel> common practice is to pass a pointer to a struct
15:43 < mochel> there are a handful of exceptions, for exceptional cases
15:44 < pavelm> if you pass pointer to a struct, you'll not get typechecking, and typechecking matters here.
15:44 < mochel> it is possible to design an easy-to-use, hard-to-misuse interface w/o resorting to an
exceptional case
15:44 < mochel> put the struct inside the pointer
15:44 < pavelm> those exceptions are marked by _t, this one is marked by _t, so noone is going to be surprised
by this.
15:45 < mochel> i fail to see how a typedef is a valid justification
15:45 < mochel> more than anything, you're setting a bad example for people new to the kernel
15:45 < mochel> which is very common in device driver writers
15:46 < mochel> very few people use pgd_t. unless you have a lot of experience, you will never program with it
15:46 < pavelm> you wanted to say "put pointer inside struct", right?
15:46 < mochel> every device driver will see and use pm_message_t, meaning there is a likely chance they will
copy and re-use the concepts
15:46 < mochel> no, i did not
15:46 < mochel> struct pm_message {
15:47 < pavelm> "put struct inside the pointer" does not get you enough typechecking.
15:47 < mochel> struct pm_system_state system_state;
15:47 < mochel> }
15:47 < pavelm> See if (!state).
15:47 < mochel> so what if people do that; we fix them up
15:47 < lenb> jordan: see _PS0, _PS1, _PS2, _PS3 -- they put the device into Device states 0, 1, 2, 3
15:47 < pavelm> you sound like broken record.
15:47 < mochel> then, when people copy the drivers to write a new one, they won't even think that's a
possibility
15:47 < pavelm> who fixes them, you?
15:47 < mochel> so do you
15:47 * jordan pulls up the old ACPI pdf
15:47 < mochel> yes, i volunteered an hour (and much less patience) ago
15:48 * lenb bugs out for dinner
15:48 < pavelm> Having to fix drivers because of bad interface is stupid... Now, what did you want to do with
struct inside struct?
15:48 < mochel> if (!state) just means the device won't do anything on suspend, because it thinks it's a resume
call
15:48 < mochel> if (!state) with a pointer would logically mean that it shouldn't do anything
15:49 < mochel> so, they're equivalent
15:49 < pavelm> yes and you want compiler to error out on if (!state), because it is a bug.
15:49 < pavelm> if pm_message_t is structure, you get the error.
15:49 < mochel> it's a MINOR bug, and one that won't cause problems
15:50 < pavelm> not if you store it and reuse it in resume routine.
15:50 < pavelm> ...and it is bug drivers are making just now. I'd like them to error out so that I can fix it.
15:51 < mochel> you don't want to copy the structure; you only want to copy the device state, which is not in
the structure
15:51 < mochel> so, we audit the drivers now that are doing that and eradicate them.
15:51 < pavelm> yes but drivers are copying it just now. If I knew how, I'd try to stop them.
15:52 < mochel> they must be audited and fixed
15:52 < pavelm> I already did one pass through the drivers, and I'm not looking forward to another one.
15:52 < mochel> that is the only way to fix them
15:52 < pavelm> ...that said, try to come up with a patch that makes it pointer.
15:53 < pavelm> be sure to explain the rules for apm.c-like code (may not be module, must be static const).
15:53 < pavelm> I *think* the result will be more ugly than my current approach, but hey.
15:53 < pavelm> Real work there is auditing the drivers, and my current approach will benefit from that, too.
15:54 < mochel> you must think of what will only benefit pavelm and what will benefit kernel developers and
driver writers
15:54 < pavelm> you are proposing harder interface for driver writers for 4bytes on stack...
the structure
15:51 < mochel> so, we audit the drivers now that are doing that and eradicate them.
15:51 < pavelm> yes but drivers are copying it just now. If I knew how, I'd try to stop them.
15:52 < mochel> they must be audited and fixed
15:52 < pavelm> I already did one pass through the drivers, and I'm not looking forward to another one.
15:52 < mochel> that is the only way to fix them
15:52 < pavelm> ...that said, try to come up with a patch that makes it pointer.
15:53 < pavelm> be sure to explain the rules for apm.c-like code (may not be module, must be static const).
15:53 < pavelm> I *think* the result will be more ugly than my current approach, but hey.
15:53 < pavelm> Real work there is auditing the drivers, and my current approach will benefit from that, too.
15:54 < mochel> you must think of what will only benefit pavelm and what will benefit kernel developers and
driver writers
15:54 < pavelm> you are proposing harder interface for driver writers for 4bytes on stack...
15:54 < mochel> ...and decide what goal you're aiming for, and if you what you're doing *really* does what you
think it will
15:55 < mochel> i'm not saying that i'm right
15:56 < mochel> but, if i were you, i wouldn't be so quick to assert that you are right when met with such
strong resistance among people that are effectively on the same team as you
15:58 < pavelm> I tried to think about something better, but this interface is best I know. I believe that few
bytes on the stack are not really worth the fight and I'd like to have this solved quickly so
that I can do something more interesting.
16:00 < mochel> well, several of us seem to think it's a bad approach, so there are a few things that can
happen..
16:00 < mochel> 1) you keep trying and incorporate input from people until we get to something mutually
palatable
16:00 < mochel> 2) you give up and let someone else try
16:01 < mochel> 3) you ignore the comments and merge it upstream, causing a lot of headaches and creating a lot
of personal grudges
16:01 < mochel> we have tried to help with (1)
16:01 < mochel> i'm will to try w/ (2)
16:02 < mochel> s/will/willing/
16:02 < pavelm> I'm okay with (2) as long as you can give me some reasonable time estimate.
16:02 < mochel> but, please don't just go merge it upstream before there is consensus
16:02 < mochel> hey, i don't work for you :)
16:03 < mochel> it will get done when it gets done
16:03 < pavelm> I know you don't work for me, sorry about that, but I'd like to get this mess solved before
2.6.12
16:03 < gregkh> that might be unreasonable.
16:03 < pavelm> if it is not possible than in 2.6.13-rc1.
16:03 < gregkh> I'm not going to merge any more pm patches.
16:03 < gregkh> for a while...
16:03 < pavelm> greg: okay, good.
16:04 < pavelm> So target is 2.6.13-rc1. (benh and lenb wanted to have it solved for 2.6.12, but...)
16:04 < mochel> we all agree that the sooner the better
16:05 < mochel> but, having the concept of a 'time limit' is rediculous
16:05 < pavelm> My patch seems to do the trick, and only objections I seen are
16:05 < mochel> it's done when it's finished. it's finished when it's something a) works and b) we agree on
16:05 < pavelm> '4 bytes on stack' and 'you don't pass struct by value'
16:05 < pavelm> .
16:06 < mochel> you must realize that what are minor objections to you may be major objections to someone else
16:06 < mochel> there should be no objections
16:06 < pavelm> There will always be some objections, this group is big enough.
16:06 < mochel> besides, don't forget "you're violating common practice" and "you're setting a bad example for
driver writers"
16:06 < mochel> no, there doesn't have to be objections
16:07 < gregkh> don't pass by value is pretty serious to me at least...
16:07 < pavelm> But there's already bad example for driver writers in the tree, its called u32, and more time
it is there, the more damage it does.
16:07 < mochel> that is no justification to create more bad examples
16:08 < pavelm> Yes, but putting struct there now is probably better than putting struct * there in 2.6.29.
16:08 < mochel> i don't agree
16:08 < mochel> and i'm done arguing
16:08 < mochel> it's now been > 3 hours
16:09 < pavelm> I'd like to see this code working and without a mess. If you can come up with something
16:09 < pavelm> ni reasonable timeframe, and it looks better, why not.
16:09 < pavelm> If there's no better solution, and some people don't like it, well, putting struct there
16:09 < pavelm> is certainly better than u32, and I'll have to live with some grunges.
16:11 < pavelm> [Oh and if you give up doing (2), *please* let me know so that I do not hold back the patches
without reason.]
16:11 < pavelm> Bye for now.
16:13 -!- jordan [~nobody@xxxxxxxxxxxxxxxxxxxxxxx] has left #pm [Leaving]
