TMS570 BSP updates and LwIP support

[Martin Galvan]

On Sun, Nov 8, 2015 at 11:11 AM, Pavel Pisa <pisa at cmp.felk.cvut.cz> wrote:
> Hello Martin and others,

Hi Pavel. I'll comment on the POM and GPIO-related things, since we're
not currently using Ethernet (nor are we familiar with how the TMS
handles it).

>    The complete GPIO API or at least xx_gpio_set(), xx_gpio_clear(),
>    xx_gpio_write(), xx_gpio_get() and lpc24xx_pin_config() would be
>    nice in future. If the actual pin configuration should be reimplemented
>    to support that API or if it API should be wrapper to pinmux functions
>    is a question. The mapping between GPIO and actual pins is quite complex
>    in TMS570 case because pins are mapped by pinmux to peripherals and
>    in case, that given peripheral does not use given pin/terminal for
>    its specific purpose then peripheral includes set of registers to use
>    its available pins as GPIO.
>
>    I think that actual simple code is valuable and should be included
>    in mainline for now even if there is found time to replace it by
>    more complete one later.

If transitioning to the new API would be that problematic, then I
support keeping it as it is right now. However, I think the core
maintainers should also have a say on this. Maybe there are benefits
on supporting the new API that we're not seeing.

>  * there are some changes used for VIM, POM and other parts debugging
>    and issues solving. Most of these is not intended for mainline.
>
>  * set of changes to not use exception vectors replace if it is not required
>    (RTEMS application image starts at address 0). When replacement is necessary,
>    POM is used only to deliver target addresses and not to replace jump
>    instructions now because instruction fetches from POM reveals to be unreliable.
>    All related code is amended by comprehensive comments to understand
>    its function and reasons why used solution has been selected
>
>      https://github.com/AoLaD/rtems/commit/7dab9093bc8211816def58460efef8958b041904

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

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.

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.

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.