RTEMS_DECONST - Should it be removed? - TMS570

Wed Jan 21 18:44:21 UTC 2015

Sorry for the late answer.

On Wed, Jan 14, 2015 at 6:50 PM, Pavel Pisa <pisa at cmp.felk.cvut.cz> wrote:
> Hello Martin,
>
> On Wednesday 14 of January 2015 18:27:41 Martin Galvan wrote:
>> Hi everyone! We're currently working on improving the TMS570 BSP, and
>> in the process we discovered an important bug caused by a misuse of
>> the RTEMS_DECONST macro. Said macro seems to be used in a few other
>> places throughout the code to bypass const restrictions.
>
> Please, can you provide more details?
> I have contributed to RTEMS_DECONST and TMS570 BSP - directly
> and as GSoC menthor. So bugs can fall to my head.

Sure. We're working with a TMS570 USB Development kit, with both RTEMS and
the HALCogen-generated code placed the board's ROM (I understand you were
loading RTEMS in RAM).

What happened was that, in the SCI driver, driver_context_table was
declared as const and it included a variable called tx_chars_in_hw which is
used by the interrupt handler as a flag to mark that we just wrote a
character. tx_chars_in_hw was supposed to be set to 1 in
tms570_sci_interrupt_write; however because driver_context_table was
declared as const it was being placed inside the board's ROM. Therefore,
tx_chars_in_hw couldn't be written into, and the console got locked after
writing the first character. RTEMS_DECONST was being misused in
console_initialize to cast &driver_context_table.base to
rtems_termios_device_context *, thus masking out the source of the bug. The
only reason it was working for you is that RTEMS was loaded in RAM.

Right now we're facing a different bug where we can't use printf to print a
2-digit integer because the system hangs somewhere inside printf. This
happens in both interrupt-driven and polled mode, but it doesn't happen
with puts or printk (which leads us to think it's either a printf or a
driver issue). Any pointers on this would be welcome.

>> What's the purpose of having something like this in the codebase?
>> Should it be removed to maintain const-correctness throughout the
>> code?
>
> The RTEMS_DECONST is the tool how to suppress warning when
> the target function can cope with const and non-constant
> pointer, can distinguish if it is non-const case from some
> data in from the poited to structure or other parameter
> and it needs to change some data/reference counts in non-const
> case for example. Such cases cannot be solved cleanly with
> C in some specific cases and programmer has to take  responsibility
> into his/her own hands. But there could be bug for sure.

I noticed that. It's unfortunate we have to have something like this in the
codebase; we should at least add a few comments to the macro definition to
avoid having people using it unless it's completely necessary.

> As for TMS570 BSP, Premysl Houdek is due to finish some
> things in the BSP in the next months. He needs to complete
> his master thesis and submit it.
>
> I have spent considerable time with him to negotiate
> and prepare format and way to extend coverage of TMS570 header
> files. I have communicated with Ti representatives about
> releasing files under RTEMS compatible license but they are
> not able to change that for more reasons. So we work on preparation
> of RTEMS license and format friendly header files from PDF manual.

I don't understand what you meant by that. Right now we're using the
HALCogen code for testing purposes only; is there any way to incorporate
them into the BSP code?

> Premek has promised to complete ETHERNET support in frame
> of his thesis. My other colleague finished work on TMS570 ETHERNET
> for Ti provided FreeRTOS to really work. So we have knowledge
> now how the EMAC works. We work on Matlab/Simulink support
> in our other industrial partner paid non-RTEMS TMS570 and RM48
> project based on Ti provided CodeComposer support. So I would be happy
> if we can one day extend TMS570 to support RM48 as well but that
> is far far future without contract. But I hope that TMS570
> will be extended in next months.

That's great. We're not planning to use ethernet with the TMS right now so
we won't be able to test it, but any improvements to the BSP are welcome.

> So please, if you plan to introduce more changes, please, send
> plan of your work in advance to coordinate things.

We're probably gonna have to refactor a few of the existing files and add
CAN support.

> If you want to text TMS570 for real application which is computational
intensive
> then you probably want to try change board configuration files to specify
> hat FPU should be used for math. We have used soft-float
> for initial porting but thanks to Sebastian Huber generic
> ARM work, TMS570 float is supported too.

Right now we're not using the FPU, but we'll definitely need it in the near
future so that's great.

> As for RTEMS_DECONST, its definition should not be removed for sure.
> But it is possible that it is used in incorrect place or even that
> that macro has bug. So please, specify in more detail what is
> the problem and what are your planes.

I'm attaching a patch with the refactored driver. It should work with the
tms570ls3137_hdk configuration. Notice we had to add some calls to
HALCogen-generated functions inside the RTEMS initialization code in order
for everything to work. We also changed the value of the BSP_PLL_OUT_CLOCK
macro from 160 MHz to 180 MHz.

-- 

Martín Galván

Software Engineer

Taller Technologies Argentina

San Lorenzo 47, 3rd Floor, Office 5

Córdoba, Argentina

Phone: 54 351 4217888 / +54 351 4218211
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20150121/a33b3dc8/attachment-0002.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: changes.patch
Type: text/x-diff
Size: 34377 bytes
Desc: not available
URL: <http://lists.rtems.org/pipermail/devel/attachments/20150121/a33b3dc8/attachment-0002.bin>