[U-Boot] [PATCH 4/8] Add a poll function to monitor events

[Wolfgang Denk]

Dear Simon Glass,

In message <CAPnjgZ0bmsJAx4MAk-+GLo+QLC-6zbgp24N=BNqO=2fsUikjDw at 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?

> > 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?

> 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.

> 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 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.

> 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...

> 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?

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?

> 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?

> Any of the above is much more radical than this patch.

But the patch as submitted here is not even functional...

Best regards,

Wolfgang Denk

-- 
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 at denx.de
Weekends were made for programming.                 - Karl Lehenbauer