On Tue, Sep 13, 2016 at 5:11 PM, Pavel Pisa <p...@cmp.felk.cvut.cz> 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