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

[Wolfgang Denk]

Dear Simon Glass,

In message <CAPnjgZ05L+CpbQphaD6Of3zR1hst2YL3pK86=yMC79UtRxvjXQ at mail.gmail.com> you wrote:
> 
> > 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.

Let me summarize the facts and arguments as far as I understand these
so far:

- There is built-in hardware protection; if we don't apply this patch,
  nothing gets broken, but the system will power off when overheating.

- The normal mode of operation s to boot an OS as quickly as possible,
  without any user interaction, so this patch is not needed, nor would
  the code be used at all.

- In the few cases where we don't boot an OS quickly, we assume that
  the only long-running activity that needs protection is waiting for
  user input.

- If we spend too much time in U-Boot and te system overheats, then we
  power it down.


I have these comments:

* The assumption, that only waiting for keyboard input is a critical
  operation, is obviously broken.

* If we just power down, we can as well use the hardware-triggered
  power down; the effect would be the same?

I can only repeat myself:
- what would be needed here is an asynchronous (periodic) service
- hooking this onto keyboard activity is broken by design
- we don't have any other generic mechanism to implement such a
  feature yet.

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

Please stop thinking about user activity.  It has _nothing_ to do with
it.  What you are looking for is a time triggered service - you want
to make sure to run your temperature control code at least every N
milliseconds.

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

Why are we running at such a high clock anyway in U-Boot?  Most
activities in U-Boot are limited by I/O troughput and/or memory
bandwidth.  There are very few exceptions: uncompressing and
checksumming an OS image when boting it are such; at the moment I
cannot think of any other that might be significant in the sense that
they would play any role for boot times in the majority of use cases
(just booting an OS).

So why not simply run U-Boot at a much lower clock?  Has anybody
measured how many milliseconds of boot time that would cost?

And, if needed, you could still rev up the clock rate when actually
running the bootm code where speed actually plays a role...


> Actually I was happy enough with a simple patch to indicate idle in
> the console code :-)

I fail to understand why you don't accept the fact that this does not
even work?

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

My vote is to none of this.

> But if you can accept that this feature is useful, how would you implement it?

I am pretty much convinced that the chosen approach is wrong, and that
we do no actually need any such feature here.

I am very sure that hooking this into console code is totally wrong,
as the heating up takes place independent of any user interaction.

I also challenge the assumption that periodic polling is needed.
I don't know the hardware inplace, but most likely it is capable of
actively signalling that overheating takes place - I guess you can
program a maximum temperatrue and it will raise an interrupt if this
is reached?  If so, temperature control should be implemented in an
event-driven way, and not using polling.

If in the end we still should find we need to implement an
asynchronous, time-triggered service, then this should be implemented
based on a periodic timer interrupt.

> > But the patch as submitted here is not even functional...
> 
> It seems to work as intended, albeit I'm sure there are flaws.

It works only as long as you do not anything which cases  U-Boot to
run for too long before returning to the console - and run time is
totally out of your control.  In other words: it is just good for a
demo and to collect brownie points from some bosses.  Would you drive
a car where the operation of the safety belt was similarly reliable?

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
I mean, I . . . think to understand you, I just don't know  what  you
are saying ...                        - Terry Pratchett, _Soul Music_