[PATCH v2] Add TOD Hooks to allow BSP to take action when TOD is set

[Sebastian Huber]

On 15/11/2019 15:57, Joel Sherrill wrote:
> 
> 
> On Thu, Nov 14, 2019 at 11:59 PM Sebastian Huber 
> <sebastian.huber at embedded-brains.de 
> <mailto:sebastian.huber at embedded-brains.de>> wrote:
> 
> 
> 
>     On 14/11/2019 21:44, Joel Sherrill wrote:
>      > +bool _TOD_Hook_Register(
>      > +  TOD_Hook *hook
>      > +)
>      > +{
>      > +  ISR_lock_Context lock_context;
>      > +
>      > +  /*
>      > +   * At this time, this method does NOT have a Classic or POSIX API
>      > +   * that exports it. Any use of this method will be a direct call.
>      > +   * It should only be called while NOT holding the TOD lock.
>      > +   */
>      > +  _Assert( !_TOD_Is_owner() );
>      > +
>      > +  if ( hook == NULL ) {
>      > +    return FALSE;
>      > +  }
>      > +
>      > +  _TOD_Lock();
>      > +  _TOD_Acquire( &lock_context );
>      > +  _Chain_Append_unprotected( &_TOD_Hooks, &hook->Node );
>      > +  _TOD_Unlock();
>      > +  return true;
>      > +}
> 
>     Where is the _TOD_Release()? This function should not test for a NULL
>     hook and return void. Such kind of error handling should be done by
>     upper layer functions and not at the lowest level.
> 
> 
> There is no method _TOD_Release in the source tree. I followed the locking
> pattern in clockset.c.
> 
>      _TOD_Lock();
>      _TOD_Acquire( &lock_context );
>      retval = _TOD_Set( &tod_as_timespec, &lock_context );
>      _TOD_Unlock();

In this case you need the _TOD_Acquire() since _TOD_Set() takes care of 
the release and expects an acquired TOD lock and a valid lock context. 
Please remove all the unnecessary _TOD_Acquire() since they break the 
SMP support.

> 
> There is no upper level. Per our earlier discussions, there is not an 
> API above the
> score on this for register and unregister. So the error checking is 
> needed here. If there
> were a layer about this, the locking and error checking would be there.

Testing in _TOD_Hook_Register() that that the hook pointer is non-NULL 
is completely superfluous from my point of view. Users of underscore 
functions are supposed to know what they do. If you call this function, 
you want to register a hook, don't you? If you feel safer, then you can 
add an _Assert( hook != NULL ).

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.