[PATCH] score: Split _Terminate and call _Terminate_CPU_Fatal_halt.

[Sebastian Huber]

On 2014-04-28 01:48, Chris Johns wrote:
> On 27/04/2014 10:30 pm, Sebastian Huber wrote:
>> On 04/27/2014 02:13 PM, Chris Johns wrote:
>>> Splitting the call to _CPU_Fatal_halt out into a separate function
>>> allows the rtems-test gdb support the ability to halt once the
>>> _Terminate function has completed it's work.
>>>
>>> This change allows the BeagleBoard xM BSP to pass a number of
>>> important tests.
>>
>> I don't find a BeagleBord BSP in the tree.
>>
>
> Ben has nicely answered this. Thanks Ben.
>
>> In case the BSP has to do fancy things during termination, why don't you
>> do this in bsp_reset() or bsp_fatal_extension()?
>>
>> I guess the bsp.h has no:
>>
>> #include <bsp/default-initial-extension.h>
>>
>
> I do not know and from a testing point of view I do not think it should it matter.
>
> The _Terminate function needs to be split so we can set a break point and have
> it hit in the default case. Setting a break point based on a line number in a
> file is far too fragile. We need the extensions to run so the END marker is
> output in a few tests.

You can set a break point to bsp_fatal_extension() (works with every BSP in the 
tree) or you can set a break point to rtems_test_fatal_extension() (works with 
every test in the tree, if not, then this test is broken).

>
> I added the weak part because it costs nothing to add. Maybe we should consider
> the patch as 2 parts.
>
>> The way to control the termination sequence is via user extensions and
>> not weak functions.
>
> Pros and cons ... If the point is having only one way, that is a valid comment.
> On the other hand weak functions cost little if anything and it lets the BSP
> designer have control over adding the required BSP specialisation. Weak
> functions can be abused and that should be avoided.

I see no point in adding another can of worms for just one BSP if you can 
easily avoid this with the existing solution (see above).

>
> I have now taken a closer look at this default extension approach. The
> <bsp/default-initial-extension.h> fights the cpukit and bsp separation and adds
> new rules to the use of confdefs.h that I do not think are being enforced.

If you don't use confdefs.h, then you should know what you are doing.

> If a
> new user to RTEMS uses a BSP and creates an app using confdefs.h and does not
> first include bsp.h or default-initial-extension.h the resulting BSP code
> linked in is not what the BSP designer intended and as far as I can see there
> is not indication something is wrong.

If you use confdefs.h, then everything is fine:

[...]
#ifdef CONFIGURE_DISABLE_BSP_SETTINGS
   #undef BSP_DEFAULT_UNIFIED_WORK_AREAS
   #undef BSP_IDLE_TASK_BODY
   #undef BSP_IDLE_TASK_STACK_SIZE
   #undef BSP_INITIAL_EXTENSION
   #undef BSP_INTERRUPT_STACK_SIZE
   #undef BSP_MAXIMUM_DEVICES
   #undef BSP_ZERO_WORKSPACE_AUTOMATICALLY
   #undef CONFIGURE_BSP_PREREQUISITE_DRIVERS
   #undef CONFIGURE_MALLOC_BSP_SUPPORTS_SBRK
#else
   #include <bsp.h>
#endif
[...]

This missing include of <bsp.h> was indeed a problem in the past and resulted 
in quite a lot service requests and confused users (e.g. no power saving idle 
thread).

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