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

[Simon Glass]

Hi Wolfgang,

On Wed, Dec 12, 2012 at 2:11 PM, Wolfgang Denk <wd at denx.de> wrote:
> 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?

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