On Sun, Nov 8, 2015 at 11:11 AM, Pavel Pisa <p...@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. _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
