TMS570 BSP updates and LwIP support

[Pavel Pisa]

Hello Martin,

On Monday 09 of November 2015 16:29:09 Martin Galvan wrote:
> On Sun, Nov 8, 2015 at 11:11 AM, Pavel Pisa <pisa at cmp.felk.cvut.cz> wrote:
...
> > https://github.com/AoLaD/rtems/commit/7dab9093bc8211816def58460efef8958b0
> >41904
>
> I saw you added this function before tms570_pom_remap:
>
> +void tms570_initialize_and_clear(void)
> +{
> +  int i;
> +
> +  TMS570_POM.GLBCTRL = 0 | (TMS570_POM_GLBCTRL_OTADDR(~0) &
> +                            pom_global_overlay_target_address_start);
> +
> +  for ( i = 0; i < TMS570_POM_REGIONS; ++i ) {
> +    TMS570_POM.REG[i].REGSIZE =
> TMS570_POM_REGSIZE_SIZE(TMS570_POM_REGSIZE_DISABLED);
> +  }
> +}
>
> I don't see the point of doing (0 | something).

This is to highlight that register value is not only the base
of overlay target address. The following expression
contains all information based on our headers using
RTEMS standard for fields values.

  TMS570_POM.GLBCTRL = TMS570_POM_GLBCTRL_ON_OFF(0) |
                       TMS570_POM_GLBCTRL_ETO(0) |
                       (TMS570_POM_GLBCTRL_OTADDR(~0) &
                        pom_global_overlay_target_address_start);

It has same value and effect as simplified one.
But if you prefer this form then I change code.

> I see you'll only be calling tms570_pom_remap if
> bsp_start_vector_table_begin isn't zero. This looks good enough for
> our use case, though I'd prefer if you didn't use the hardcoded zero.
> This is merely a stylistic suggestion though.
>
> In any case, is there a reason why you're seting the OTADDR in
> tms570_initialize_and_clear if you're gonna overwrite it later in
> tms570_pom_remap (or, in our case, not use it at all)? I think simply
> setting TMS570_POM.GLBCTRL to zero would be clearer.

The OT base address is global decision so even if following
region setup code is implemented such way, that it looks for
the first available POM region it has to allocate overlay
target in memory foolowing OT base address. So code reflect
this information that decision is global. If there more SRAM
memory reserved than even overlay target can be allocated
dynamically. I do not expect that we need such mechanism
soon or in middle time horizon. But I want to document hat.

> I also don't see the point of doing bsp_int_vec_overlay_start =
> ORIGIN(RAM_INT_VEC) in the flash-based linker script, though since
> it's not gonna be used anyway it shouldn't do any harm. A comment
> explaining this would be nice.

Can be omitted. But there is even chance that RTEMS can be used as boot
loader and if mechanism with jump over SRAM trampolines is used
then even code placed from address 0 needs space for SRAM trampolines.
I am not sure which of POM, SRAM or SWAP mechanism is bet option for now.
So we try to keep decision open and area reserved. But I agree that
it is a little waste of memory.

> Just to be sure, I checked the Technical Reference Manual again and
> noticed the ON/OFF bits of the POM GLBCTRL must be 0x5 to activate the
> POM (as opposed to the 0xA it's being passed). I know it works with
> 0xA (since you've been testing it like this), so perhaps it's a
> documentation error? I also noticed that the 'OFF' setting says: "The
> key should be written to 5h, to avoid single bit flips inadvertently
> turning on the module", which seems to reinforce the 0xA value.

The older version of Technical Refence Manual is unclear.
It take me some time to find the problem by comparison with
manuals for newer TMS570 family members when we introduced
TMS570 BSP. Manual version SPNU499B from November 2012
(Revised August 2013) is already clear

Value   Ah ... POM is functional.
All other values ... POM is held in reset.
NOTE: The key should be written to 5h, to avoid single bit flips
inadvertently turning on the module.

May it be we should use 0x5 for disable. But even 0 differ in 2 bits.

Have you some plans to integrate or cooperate on bare (no loader)
startup code integration to mainline RTEMS BSP? It seems that
HalCoGen licensing has dramatically changed for latest versions
(so after review by others) it could be possible to reuse parts
of the generated code. Other option (may it be better fitting RTEMS
code style) is incrementally rewrite/write own code.

Have you some plans with loader/firmware updates support
in longer term perspective.

If you intend to use 4.11 for production, please, test our changes
and suggest if they are appropriate for 4.11.

We have rebased tme570-bsp branch to today 4.11 base and cleaned
up commits sequence. We send patches to mailing list for general
review after some more remarks collection.


Best wishes,

              Pavel