On 28/04/2014 4:18 pm, Sebastian Huber wrote:
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).


The split lets me execute as much code as possible and it is something we should encourage with testing.


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


Happy to drop the weak attribute from the change.


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


I missed this and Joel pointed it out to me. This is fine.

Chris
_______________________________________________
rtems-devel mailing list
rtems-devel@rtems.org
http://www.rtems.org/mailman/listinfo/rtems-devel

Reply via email to