Hi Wolfgang,
On Wed, Dec 12, 2012 at 2:11 PM, Wolfgang Denk wd@denx.de wrote:
Dear Simon Glass,
In message CAPnjgZ0bmsJAx4MAk-+GLo+QLC-6zbgp24N=BNqO=2fsUikjDw@mail.gmail.com you wrote:
Also, this should not be considered as somethin board specific as the name "board polling function" might suggest. What is being discussed here is not more and not less than support for periodic, asynchronous services. So let's call it that, so everybody understand what it's about.
Well that means interrupts. Are you suggesting that we implement generic timers and allow drivers to register a timer callback function to be called periodically? That seems like a bold move, but we could do it.
Which architectures do not use interrupts for the timer services?
For example some ARM implementations just have a hardware counter that we can read. It doesn't roll over so no need for interrupts.
How would one recover from that?
Firstly we can detect that the system is idle (by noticing that it has been long time since a keypress, for example), and reduce the CPU clock speed until we next see a key. This might be enough to remove the problem. Perhaps we should add some sort of feature to console to
No, this is just a crappy design.
Assume you load some big image and write it to NAND (or you do other operations that take some time; eventually in a loop). Your keyboard dependet code would not trigger here, and you would just overheat the system.
I mean, what good is such protection when a simple "while : ; do : ; done" will just toast your box?
Well it will force a hard power off - the hardware has a hard limit at which it will power off. Don't do that! The more common case is the user sitting at a prompt doing nothing, and that's the case which prompted this patch I think.
record the time of last key input to facilitate that. There are lots of ways to do it.
Sorry, but keyboard activity has _nothing_ to do ith it and is the totally wrong place to attach such functionality to.
Can you suggest an alternative place which gives us an indicator of user activity?
Secondly, when the system warms up we can display a warning on the console, and perhaps reduce CPU speed further.
Thirdly, when the system starts to melt, we can power it off.
Would you ever buy such a design? I wouldn't. And any competing company would probably have excellent arguments for their own marketing material.
So long as it had some fail safe power off when things get hot it is ok. But we need to try to avoid getting into that condition. Running flat out at 1.7GHz waiting for keyboard input that may never come is not good design.
So it seems that some sort of timer hook might be good for the patch under discussion, but for the watchdog we need an idle loop or
I don;t understand your arguments here. You are aware that we don;t really have watchdog support in U-Boot (not in the sense that we monitor U-Boot's operation) - we only make sure to trigger it periodically.
Yes I was responding to your complaint about how watchdogs currently work. It is ugly that things like hashing functions need a parameter telling them when to reset, and have watchdog calls in the algorithm.
similar. The timer hook does open a bit of a can of worms for other reasons - e.g. you are in the middle of an i2c transaction, the timer fires, you jump to the temperature monitor which wants to use i2c to find out the temperature. I thought we wanted to avoid this sort of thing in U-Boot so long as possible.
Yes, we definitely do want to avoid this, which is the reson why I insist on asking of we actually need such a feature in U-Boot. If the hardware cannot protect itself sufficiently and relies on software support, you are doomed anyway, because somebody will always just do some stupid things and find out that he detected a HCF macro...
Hopefully we have covered this now. HCF will cause a hard power off eventually, but we don't want sitting idle at the prompt to be equivalent to HCF.
We could perhaps add an 'idle hook' so that callers can register themselves:
Where exactly would you hook that to, if not to some timer interrupt?
Console input, places where the watchdog is currently reset, for example.
ANd please tink about this again - if you talk about an 'idle hook', you actually are talking about an "idle task", i. e. about introducing a multi-task concept (even if it's just a simple one).
This is not exactly a small can of worms, and I guess these worms have a smell ...
and then change all the WATCHDOG_RESET() calls to call idle_poll() instead.
Oh. Cooperative multitasking...
Are you really, really sure we want to do that?
Actually I was happy enough with a simple patch to indicate idle in the console code :-)
We could perhaps have a flags parameter to permit selection of what sort of things to poll - e.g. IDLE_POLL_WATCHDOG for a quick watchdog reset, IDLE_POLL_ALL to call all the registered handlers).
The hooks could then be generalised to support timers, with the proviso that timers are only ever invoked from the idle loop so that
Yes, we can do all that. We can actually implement a full-blown OS.
Are you really, really sure we want to do that?
<1> <2> <3> <4> this patch idle support co-op multitasking full-blown OS
Please choose :/) I think you are saying that <1> is too much of a hack. Clearly <4> is not why we are here. I suggest <1>, or failing that, <2>. I think <3> is scary and I think there is clear daylight between <2> and <3>.
But if you can accept that this feature is useful, how would you implement it?
Any of the above is much more radical than this patch.
But the patch as submitted here is not even functional...
It seems to work as intended, albeit I'm sure there are flaws.
Best regards,
Wolfgang Denk
--
Regards, Simon
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de Weekends were made for programming. - Karl Lehenbauer