Re: [PATCH 0/5] arm/tms570: include hardware initialization and selftests to run RTEMS directly from Flash without loader on TMS570LS3137 HDK.
I'm slowly reading through these. Is there a doc or is it worth it to generate one that maps the HalcoGen functions to their RTEMS renamed versions? I'm not sure if it is of much value. On Mon, Sep 12, 2016 at 5:47 PM, wrote: > From: Pavel Pisa > > The code introduces initialization algorithms bases on Ti HalCoGen > generated files. The most of the code has been rewritten to use > RTEMS much more complete header files. This allowed to replace most > of the hardcoded hexadecimal values by appropriate fields macros documenting > how values are build. > > The parity self-tests have been rewritten completely to minimize > code duplication. The solution is much better prepared to cover > variations of integrated peripherals between different Hercules > family chips. > > The pin multiplexer code is written from scratch and complete > pin alternative functions "database" is included for easy > preparation of configurations for different chips and even > runtime pin functions changes. > > Original Ti HalCoGen generated files include a BSD-like license. > It has been preserved where the file is mostly based Ti code. > Some other smaller (mostly rewritten) fragments are without > copyright header for now. > > The code is reformatted to conform RTEMS coding style mostly. > There are left some original functions names and comments > using C++ delimiters "//". I expect to remove them in long > term perspective. But I would like to use them for matching > code and algorithms with other chip family members code > generated by HalCoGen. When two or three variants are fully > supported then we learn how to write support portable > and configurable way in the C code and these remarks would > not be required anymore. > > The code taking care about internal SRAM parity self-test > is disabled for now because it clashes with code testing > for internal RAM linking variant. The MPU initialization > is missing as well. It would be required to use external > SDRAM for code. > > Generally, there is much work required still to make BSP > full industry grade one. But I think that actual state > is the great step forward and work worth to be included > in RTEMS mainline in the actual state to simplify testing > and be base for new projects. > > The second patch is quite huge so if it does not pass > through list then it can be accessed on GitHub > > The branch: https://github.com/AoLaD/rtems/commits/tms570-bsp > Patch 2/5: > https://github.com/AoLaD/rtems/commit/9d06f3be64f9b5e9e1988163762b613daba0963c > > Pavel Pisa (5): > arm/tms570: define base addresses of all TMS570LS3137 SPI interfaces. > arm/tms570: include hardware initialization and selftest based on Ti > HalCoGen generated files. > arm/tms570: include TMS570_USE_HWINIT_STARTUP option to select bare > metal startup and selftest. > arm/tms570: document BSP setup with included hardware initialization. > arm/tms570: update bootstrap generated preinstall.am > > c/src/lib/libbsp/arm/tms570/Makefile.am| 21 + > c/src/lib/libbsp/arm/tms570/README | 104 +++- > c/src/lib/libbsp/arm/tms570/configure.ac | 4 + > .../arm/tms570/hwinit/bspstarthooks-hwinit.c | 393 ++ > .../libbsp/arm/tms570/hwinit/fail_notification.c | 17 + > .../lib/libbsp/arm/tms570/hwinit/init_emif_sdram.c | 59 +++ > c/src/lib/libbsp/arm/tms570/hwinit/init_esm.c | 50 ++ > c/src/lib/libbsp/arm/tms570/hwinit/init_pinmux.c | 195 +++ > c/src/lib/libbsp/arm/tms570/hwinit/init_system.c | 380 + > c/src/lib/libbsp/arm/tms570/hwinit/tms570_hwinit.h | 23 + > .../libbsp/arm/tms570/hwinit/tms570_parity_can.c | 60 +++ > .../arm/tms570/hwinit/tms570_parity_mibspi.c | 72 +++ > .../libbsp/arm/tms570/hwinit/tms570_parity_std.c | 55 ++ > .../libbsp/arm/tms570/hwinit/tms570_parity_tests.c | 273 ++ > .../libbsp/arm/tms570/hwinit/tms570_parity_tests.h | 75 +++ > .../lib/libbsp/arm/tms570/hwinit/tms570_selftest.c | 589 > + > .../lib/libbsp/arm/tms570/hwinit/tms570_selftest.h | 156 ++ > .../lib/libbsp/arm/tms570/hwinit/tms570_sys_core.S | 575 > c/src/lib/libbsp/arm/tms570/include/tms570.h | 6 +- > c/src/lib/libbsp/arm/tms570/preinstall.am | 13 + > 20 files changed, 3097 insertions(+), 23 deletions(-) > create mode 100644 c/src/lib/libbsp/arm/tms570/hwinit/bspstarthooks-hwinit.c > create mode 100644 c/src/lib/libbsp/arm/tms570/hwinit/fail_notification.c > create mode 100644 c/src/lib/libbsp/arm/tms570/hwinit/init_emif_sdram.c > create mode 100644 c/src/lib/libbsp/arm/tms570/hwinit/init_esm.c > create mode 100644 c/src/lib/libbsp/arm/tms570/hwinit/init_pinmux.c > create mode 100644 c/src/lib/libbsp/arm/tms570/hwinit/init_system.c > create mode 100644 c/src/lib/libbsp/arm/tms570/hwinit/tms570_hwinit.h > create mode 100644 c/src/lib/libbsp/arm/tms570/hwinit/tms570_parity_can.c > create mode 100644 c/src/lib/libbsp/arm/tm
Re: [PATCH 0/5] arm/tms570: include hardware initialization and selftests to run RTEMS directly from Flash without loader on TMS570LS3137 HDK.
Hello Gedare, thanks for the review. It is huge piece and not in a state as nice as I would like it. On Tuesday 13 of September 2016 21:42:48 Gedare Bloom wrote: > I'm slowly reading through these. Is there a doc or is it worth it to > generate one that maps the HalcoGen functions to their RTEMS renamed > versions? I'm not sure if it is of much value. I am not sure at the end. I have tried to change names to RTEMS, Linux, POSIX style. The rest of BSP (which is written mostly from scratch) already follows that. Other option is to use Ti CammelCase and add tms570_ prefix. Genrally I am not sure. I have not renamed functions in the ARM core support tms570_sys_core.S . But if we agree that all should follow single naming convention I take a while for that. But there could be value in preserving Ti names either. > I'm slowly going through this big one. One quick note I wanted to make > is to ask you to make comments where you have used #if 0 ... #endif to > comment out code. Yes, I should comment that more. Some small parts are only comments by code sections. But more text would help there. But the biggest commented out blocks are in bspstarthooks-hwinit.c. I probably try to move this code to subroutines. The two bigger parts are internal main SRAM selftest and test of reaction to SRAM content protection against bitflip errors. I plan to try to reenable at least selftest for cases where it does not lead to crash. But these testes has to be skipped for case when RTEMS application is linked and run from SRAM and even for case when SRAM is used as overlay for FLASH when RTEMS application is not starting from address zero. Or at least I have problems with these cases. When we have already finally link mode where RTEMS can be the first (and only) Flash content starting directly then for this mode code should work. But requires more experiments. Masks for tms570_pbist_run should be converted to some defines as well -> to my TODO. Complete RAM ECC reporting checking then triggers real uncorrectable error fault which leads to ARM level exception which is catch and only if that all happens then selftest continues, else fault is reported. All that requires that code runs from Flash and does not touch RAM except for precise triggering events. So again quite huge task and requiring modification of RTEMS ARM exception table. > Is "partest" halcogen term, or you chose it? I might prefer > "parity_check" instead (and remove any redundant "parity_check_check". I looked for some shorthand to have sane length names. But I agree that they could be misleading. May it be parck_ to be short? On the other hand parity_check_ is more clear. But in the fact it is "selftest of parity check reporting mechanism health". Best wishes, Pavel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 0/5] arm/tms570: include hardware initialization and selftests to run RTEMS directly from Flash without loader on TMS570LS3137 HDK.
On Tue, Sep 13, 2016 at 5:11 PM, Pavel Pisa wrote: > Hello Gedare, > > thanks for the review. It is huge piece and not in a state > as nice as I would like it. > > On Tuesday 13 of September 2016 21:42:48 Gedare Bloom wrote: >> I'm slowly reading through these. Is there a doc or is it worth it to >> generate one that maps the HalcoGen functions to their RTEMS renamed >> versions? I'm not sure if it is of much value. > > I am not sure at the end. I have tried to change names > to RTEMS, Linux, POSIX style. The rest of BSP (which > is written mostly from scratch) already follows that. > Other option is to use Ti CammelCase and add tms570_ prefix. > Genrally I am not sure. I have not renamed functions > in the ARM core support tms570_sys_core.S . > But if we agree that all should follow single naming > convention I take a while for that. > But there could be value in preserving Ti names either. > I wouldn't mind simple tms570_preserveCamelCase() as an easy rule to follow and to track back clearly to what came from halcogen versus own code that should use bsp style (tms570_xxx_yyy_zzz). >> I'm slowly going through this big one. One quick note I wanted to make >> is to ask you to make comments where you have used #if 0 ... #endif to >> comment out code. > > Yes, I should comment that more. > Some small parts are only comments by code sections. > But more text would help there. > > But the biggest commented out blocks are in bspstarthooks-hwinit.c. > I probably try to move this code to subroutines. > The two bigger parts are internal main SRAM selftest > and test of reaction to SRAM content protection against bitflip > errors. > > I plan to try to reenable at least selftest for cases where > it does not lead to crash. But these testes has to be skipped > for case when RTEMS application is linked and run from SRAM > and even for case when SRAM is used as overlay for FLASH > when RTEMS application is not starting from address zero. > Or at least I have problems with these cases. > When we have already finally link mode where RTEMS > can be the first (and only) Flash content starting > directly then for this mode code should work. > But requires more experiments. > > Masks for tms570_pbist_run should be converted to > some defines as well -> to my TODO. > > Complete RAM ECC reporting checking then triggers > real uncorrectable error fault which leads to ARM level > exception which is catch and only if that all happens > then selftest continues, else fault is reported. > All that requires that code runs from Flash and does > not touch RAM except for precise triggering events. > So again quite huge task and requiring modification > of RTEMS ARM exception table. > >> Is "partest" halcogen term, or you chose it? I might prefer >> "parity_check" instead (and remove any redundant "parity_check_check". > > I looked for some shorthand to have sane length names. > But I agree that they could be misleading. May it be parck_ > to be short? On the other hand parity_check_ is more clear. > But in the fact it is "selftest of parity check reporting > mechanism health". > maybe for this selftest code it should all be prefixed with selftest_, then this is selftest_par_xxx? Makes it a bit more clear when casual reading I guess. I'm still making slow progress. > Best wishes, > >Pavel > > ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
