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

[Simon Glass]

Hi Wolfgang,

On Wed, Dec 12, 2012 at 12:44 PM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Simon Glass,
>
> In message <CAPnjgZ3VedAoM9tkzHoHWPi9cAJ=6WQ4CDOb0zQMgi7XwGwWBQ at mail.gmail.com> you wrote:
>>
>> >    If anything like this gets implemented, it has to be done in a way
>> >    that will be general enough to be useful to others as well.
>>
>> Yes. It isn't really clear how this sort of thing should be done, but
>> creating a board polling function seems like a reasonable idea. It's
>> then just a question of when to call it. Since there is no particular
>> idle or event loop in U-Boot, perhaps we need to create one, and the
>> console code is probably the only sensible place since it is waiting
>> for user input.
>
> No, using the console driver for such a hook is not sensible at all.
>
> Do we really have to re-invent the wheel each time we need one?
>
> If we need a periodic service, then this should be obviously (at least
> this seems obvious to me) be bound to the timer services.  On PPC, we
> could for example simply hook it in the timer interrupt.
>
> 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.

>
>> I am not sure about the general issue. Obviously some sort of
>> background activity is going on in the chip all the time, and
>> monitoring is sometimes necessary. I am not sure about the best
>> approach for this.
>
> I agree about the "sometimes necessary".  In this specific case, I
> doubt is this is one of these "some" cases.  But I asked this before.
>
>> By monitoring temperature we can be sure that the system does not
>> overheat - it does depend on the hardware of course (power output,
>> heatsinks) but it can happen very quickly, certainly within a few 10s
>> of seconds.
>
> Is this some theoretical consideration, or does it refer to the actual
> state of a specific piece of hardware?
>
> Assume we have such a system - how would it be going to be used?
> Example:
>
>         - power on
>         - U-Boot enters (either directly or as a result of some boot
>           error or similar) the interactive command line interface and
>           waits for input from the user
>         - temperature monitoring detects overheating
>         - system powers down
>
> 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
record the time of last key input to facilitate that. There are lots
of ways to do it.

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 it not make much more sense to bring up the system in such a
> mode of operation that no overheating will happen?

Yes, but perhaps only by running very slowly. In the normal case we
are only in U-Boot for a short period so want to run at full speed.

>
>> > This needs a lot of additional thought, and major changes to the
>> > implementation.
>>
>> Perhaps add a new idle function in common code which can be called
>> from various places (including console), and itself calls
>> board_poll_devices()? That is cosmetic, but does at least detach the
>> code from the console.
>
> No.  This is crap.  If we need a periodic service, we should implement
> one, and not add hooks here and there at random.  We already did this
> once (for the watchdog triggering), and look what a crap it is. We
> should not repeat this.

Yes I'm not a big fan of putting calls everywhere, but in a sense this
is pointing us towards an idle loop, something that we declare must be
called periodically for U-Boot to function. A timer doesn't
necessarily suit the watchdog, since we may have locked up in an
infinite loop somewhere due to a s/w or h/w problem - in that case we
don't want to kick the watchdog on an interval, only when we know that
the system is happy.

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

We could perhaps add an 'idle hook' so that callers can register themselves:

int idle_register_handler(int (*callback_fn)(void), void *priv);
int idle_free_handler(int (*callback_fn)(void), void *priv);

define an idle handler:

void idle_poll(void)
{
         WATCHDOG_RESET();
         (loop through registered handlers and call them)
}

and then change all the WATCHDOG_RESET() calls to call idle_poll() instead.

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
there is little possibility of strange things happening. That might
get around the timer interrupt problem I mentioned above.

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

Regards,
Simon

>
> 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 often quote myself; it adds spice to my conversation.  - G. B. Shaw