Merge request https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/823 was reviewed by Gedare Bloom
-- Gedare Bloom started a new discussion on bsps/include/dev/nor/config-parser.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/823#note_135697 > + * POSSIBILITY OF SUCH DAMAGE. > + */ > + I think this header file should have doxygen. -- Gedare Bloom started a new discussion on bsps/include/dev/nor/config-parser.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/823#note_135698 > + * Information (CFI) read from a flash chip. > + */ > +typedef struct NOR_Config_Data { naming should be in a consistent way. This is creating an API. Put all global symbols and typedefs in a "namespace" -- Gedare Bloom started a new discussion on bsps/include/dev/nor/config-parser.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/823#note_135699 > + /* Sector size in bytes */ > + uint32_t SectorSize; > + /* Number of sectors */ these are not a consistent style. I think we usually have snake_case with lower_case -- Gedare Bloom started a new discussion on bsps/shared/dev/nor/config-parser.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/823#note_135700 > + > + data->jedec_id = ((cfi_raw[0] << 16) | (cfi_raw[1] << 8) | cfi_raw[2]); > + int datalen = cfi_raw[3]; declare all variables at the start of their block. -- Gedare Bloom started a new discussion on bsps/shared/dev/nor/config-parser.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/823#note_135701 > + /* Apply local parse data now that parsing has succeeded */ > + *data = local; > + return 0; these functions are quite long. it would be worth thinking about how to refactor them. -- View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/823 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
