feedback on Re: [PATCH v3] BSP for TMS570LS31x Hercules Development Kit from TI (TMS570LS3137)

[Pavel Pisa]

Hello Sebastian,

On Thursday 21 of August 2014 08:59:15 Sebastian Huber wrote:
> On 20/08/14 20:17, Joel Sherrill wrote:
> > Hi
> >
> > Initial quick feedback.  I build normally with --enable-rtems-debug
> > and the tms570ls3137_hdk variant failed to build with this
> > failure:
> >
> > arm-rtems4.11-gcc --pipe -DHAVE_CONFIG_H   -I../../..
> > -I../../../../cpukit/../../../tms570ls3137_hdk/lib/include  -DASM
> > -march=armv7-r -mthumb -mbig-endian -O2 -ggdb3 -DNDEBUG -Wall
> > -Wmissing-prototypes -Wimplicit-function-declaration -Wstrict-prototypes
> > -Wnested-externs -march=armv7-r -mthumb -mbig-endian -O2 -ggdb3 -DNDEBUG
>
> Why do we have this -ggdb3 -NDEBUG set?

May be, we have not understood right flags intended use.
I my understanding is, that CFLAGS_OPTIMIZE_V are used for
build without "--enable-rtems-debug" and flags "CFLAGS_DEBUG_V"
when "--enable-rtems-debug" is present.

Then BSP defines in preferred way. We have build with default
options (without internal debug). This means that RTEMS
binary should be optimized and without internal self debug
code (NDEBUG). But want to have as much debug information
for external debugger as possible. This is -ggdb/-ggdb3.

In the fact the settings has been taken from other boards.
Actually from
  arm/lpc176x/make/custom/lpc1768_mbed.cfg
but rtl22xx.cfg is the same and -g is in almost every other ARM *.cfg.

And there is no problem with build.

But when Joel enabled --enable-rtems-debug (which we forgot to test),
then I expect that NDEBUG is not set, i.e. RTEMS uses its defaults
for debug build variant and something is broken. I would expect
that it should be case for more boards because it seems
to be in the core code. Other option is that some other BSP
have some define which in headerfiles or include some variant of
assert.h that hide the problem.

> > -Wall -Wmissing-prototypes -Wimplicit-function-declaration
> > -Wstrict-prototypes -Wnested-externs -MT
> > libscorecpu_a-arm-context-volatile-clobber.o -MD -MP -MF
> > .deps/libscorecpu_a-arm-context-volatile-clobber.Tpo -c -o
> > libscorecpu_a-arm-context-volatile-clobber.o `test -f
> > 'arm-context-volatile-clobber.S' || echo
> > '../../../../../../../../rtems/c/src/../../cpukit/score/cpu/arm/'`arm-con
> >text-volatile-clobber.S mv -f
> > .deps/libscorecpu_a-arm-context-volatile-clobber.Tpo
> > .deps/libscorecpu_a-arm-context-volatile-clobber.Po
> > In file included from
> > ../../../../cpukit/../../../tms570ls3137_hdk/lib/include/rtems/score/perc
> >pu.h:25:0, from
> > ../../../../cpukit/../../../tms570ls3137_hdk/lib/include/rtems/system.h:2
> >3, from
> > ../../../../../../../../rtems/c/src/../../cpukit/score/cpu/arm/cpu.c:29:
> > ../../../../cpukit/../../../tms570ls3137_hdk/lib/include/rtems/score/isrl
> >ock.h: In function '_ISR_lock_ISR_disable_and_acquire':
> > ../../../../cpukit/../../../tms570ls3137_hdk/lib/include/rtems/score/asse
> >rt.h:38:46: error: '__ASSERT_FUNC' undeclared (first use in this function)
> >             __assert_func( __FILE__, __LINE__, __ASSERT_FUNC, #_e ) )
>
> It seems <rtems/score/assert.h> is broken in case NDEBUG is defined.

My first glimpse looks as it would work if it really pulls in NewLib's
assert.h, but what I can imagine as possible problem cause is that build in 
cpukit/score/cpu/arm somehow preferences includes from  include/rtems/score 
(-I option ???) and recurses to itself where blocks on standard ifdef
and __ASSERT_FUNC is not defined ???? I need more look at that.

I have found some other missing support functions which are expected
by full tests build so I am looking at that.

It seems that we need pull

# Cache
libbsp_a_SOURCES += ../../../libcpu/shared/src/cache_manager.c
libbsp_a_SOURCES += ../../../libcpu/arm/shared/include/cache_.h
libbsp_a_CPPFLAGS += -I$(srcdir)/../../../libcpu/arm/shared/include

even that there is no cache on our target.

Is that right way to make core functions happy?

Other two are _CPU_Counter_read and _CPU_Counter_read.
I need to check if there is some easily accessible
performance counter on Cortex-R.

If not, we use TMS570_RTI.RTIFRC0 used in clock.c which we have
configured exactly for 1 MHz.

Should be _CPU_Counter_read included directly in tms570/clock/clock.c
or separate source files is preferred?

I (personally) would tend to put it in clock.c to keep all things which
are based on our presumption/selection of 1 usec base together.


Best wishes,

               Pavel