Merge request https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1187 was reviewed by Pavel Pisa
-- Pavel Pisa commented on a discussion on bsps/arm/tms570/start/bspstarthooks-hwinit.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1187#note_148169 > + */ > +#if 0 > + /* Disabled for now, requires specific support in data abort handler */ I can add TODO. But it did not newer worked on RTEMS except for special scenario when some other bootloader on TMS570LS3137 has been used and the bootloader provided data-abort masking during test. There has been much more breakages in TMS570 BSP from the referenced commit introduced during TMS570LC4357 bringup because it flipped the logic by mistake and internal SRAM init and PBIST has never been run when run from Flash or SDRAM. And it has not been run for intram build (which would cause breakage anyway) and SDRAM because `TMS570_USE_HWINIT_STARTUP` has been set only for Flash build. So if somebody used the BSP without some local modifications or introduction/update of safety startup from HalCoGen then system has had been run without ECC on SRAM and without guarantee that errors would be detected even on internal SRAM engine level because its PBIST testing internal logic and functionality of all SRAM cells has been skipped. In the fact, I am not sure if `tms570_pbist_run_and_check` has been good idea. As I have described, it cannot be used for SRAM, because it has to be reinitialized after it and two levels of the functions nesting required to use SRAM for saved registers (i.e. `lr`) when SRAM is not available. The preceding of `tms570_pbist_run`, `tms570_pbist_is_test_passed` and `tms570_pbist_sto` sequences replaced by `tms570_pbist_run_and_check` means that testing is defendant on internal SRAM which is not know to be usable/without errors and damage yet. Option is to move internal SRAM test to assembly part in `bsp_start_hook_0` before all other code. But it has disadvantage, that SRAM would be tested before clock setup so low frequency can mask some problem and the testing could be much slower. Move of all calls of the functions till the comment ``` end of the code skipped for tms570_running_from_tcram() ``` to the assembly part of the hook could help but again there is probably (most sure) dependency on SRAM in `tms570_system_hw_init` and code is quite complex there that its move to some assembly sequences interpreting some list of peripherals registers manipulation without need of SRAM would be really complex. On the other hand HaCoGen code uses similar sequence with SRAM use before full SRAM testing `_c_int00` -> `systemInit()` -> `setupPLL()` The SRAM critical ``` pbistRun(0x08300020U, /* ESRAM Single Port PBIST */ (uint32)PBIST_March13N_SP);` ``` is after this call chain in `_c_int00` so even there it seem that risk of running setup before SRAM is fully tested latter is considered acceptable. If there is some real interest and ideally even funding or some other kind of cooperation from these, who want to build expensive (may it be multi-millions dollars one) missions then there is chance to do deeper analyze for RTEMS. I have colleagues who use Hercules based systems in certified products with certified tools and complete setups, but their code is systemless loop even without interrupts to ensure highest SIL levels. But such analysis is really time consumpting and I am already quite exhausted by my attempt to clean breakages there and support TMS570 and my attempts on OpenOCD side still fails on TMS570LC4357 and on TMS570LS3137 I have working only older version. But I have found more steps forward in newer one. But still not enough. It is frustrating that Ti has library sources and documentation so it would be so easy if there is interest to cooperate instead of hiding, rule and extract maximum short-term profit. I have sent some plea for help after years again so we will see. I have some followup questions for WAF, but I open new thread under this MR. -- Pavel Pisa started a new discussion: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1187#note_148170 I have plea for help with WAF. For testing, it would be much better if the choice of inclusion or skipping full TMS570 system initialization and in depth self-testing (enabled by `TMS570_USE_HWINIT_STARTUP`) can be chosen independently on BSP selection. There is the current list of BSPs and default behavior - `tms570lc4357_hdk` - Flash application full HWINIT including SRAM - `tms570lc4357_hdk_intram` - `tms570lc4357_hdk_sdram` - `tms570ls3137_hdk` - Flash application full HWINIT including SRAM - `tms570ls3137_hdk_intram` - `tms570ls3137_hdk_sdram` `intram` variant can be run with HWINIT when SRAM init and full test are skipped. That is very useful variant for testing. But I do not want to introduce more BSP variants. I would prefer to use modification of the `config.ini` generated by ``` $RTEMS_DIR/waf bspdefaults --rtems-bsps=arm/tms570ls3137_hdk_intram -t "$RTEMS_DIR" -o "$MY_DIR" --prefix "/opt/rtems/7" >config.ini ``` I have solved HWINIT enable for SRAM build by YAM files modification for now ```diff --git a/spec/build/bsps/arm/tms570/objhwinitlc4357hdk.yml b/spec/build/bsps/arm/tms570/objhwinitlc4357hdk.yml index 1b91475804..71ca6b8b47 100644 --- a/spec/build/bsps/arm/tms570/objhwinitlc4357hdk.yml +++ b/spec/build/bsps/arm/tms570/objhwinitlc4357hdk.yml @@ -5,7 +5,9 @@ copyrights: - Copyright (C) 2023 embedded brains GmbH & Co. KG cppflags: [] cxxflags: [] -enabled-by: arm/tms570lc4357_hdk +enabled-by: + - arm/tms570lc4357_hdk + - arm/tms570lc4357_hdk_intram includes: [] install: [] links: [] diff --git a/spec/build/bsps/arm/tms570/objhwinitls3137hdk.yml b/spec/build/bsps/arm/tms570/objhwinitls3137hdk.yml index 9971a0a86f..a7ebcc25f3 100644 --- a/spec/build/bsps/arm/tms570/objhwinitls3137hdk.yml +++ b/spec/build/bsps/arm/tms570/objhwinitls3137hdk.yml @@ -5,7 +5,9 @@ copyrights: - Copyright (C) 2023 embedded brains GmbH & Co. KG cppflags: [] cxxflags: [] -enabled-by: arm/tms570ls3137_hdk +enabled-by: + - arm/tms570ls3137_hdk + - arm/tms570ls3137_hdk_intram includes: [] install: [] links: [] diff --git a/spec/build/bsps/arm/tms570/optlowinit.yml b/spec/build/bsps/arm/tms570/optlowinit.yml index 32b84aa08c..2f2cb3a99e 100644 --- a/spec/build/bsps/arm/tms570/optlowinit.yml +++ b/spec/build/bsps/arm/tms570/optlowinit.yml @@ -9,7 +9,9 @@ copyrights: default: - enabled-by: - arm/tms570ls3137_hdk + - arm/tms570ls3137_hdk_intram - arm/tms570lc4357_hdk + - arm/tms570lc4357_hdk_intram value: true - enabled-by: true value: false ``` I have tried to find how to conventionalize it from `TMS570_USE_HWINIT_STARTUP` in `config.ini` but it did not work for me. I would like to use conditions even for - bsps/arm/tms570/start/hwinit-lc4357-hdk.c - bsps/arm/tms570/start/hwinit-ls3137-hdk.c and more other locations to be based on `TMS570_VARIANT` and or `TMS570_VARIANT_${TMS570_VARIANT}`. But again, I have not found how to use it properly. In the fact, I have removed one such attempt in previous YML set ``` diff --git a/spec/build/bsps/arm/tms570/optmemsramsize.yml b/spec/build/bsps/arm/tms570/optmemsramsize.yml index 515819daad..87536c9364 100644 --- a/spec/build/bsps/arm/tms570/optmemsramsize.yml +++ b/spec/build/bsps/arm/tms570/optmemsramsize.yml @@ -7,7 +7,10 @@ build-type: option copyrights: - Copyright (C) 2023 embedded brains GmbH & Co. KG default: -- enabled-by: TMS570_VARIANT_4357 +- enabled-by: + - arm/tms570lc4357_hdk + - arm/tms570lc4357_hdk_intram + - arm/tms570lc4357_hdk_sdram value: 0x00080000 - enabled-by: true value: 0x00040000 ``` because it seems that use of define setup by `TMS570_VARIANT` does not wok in build scripts and incorrect values has been chosen. There should/could be/work test for for `TMS570_VARIANT` equal to 4357, but I am not sure how to setup it in WAF. -- View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1187 You're receiving this email because of your account on gitlab.rtems.org.
_______________________________________________ bugs mailing list [email protected] http://lists.rtems.org/mailman/listinfo/bugs
