)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"560bf58af9b40aaa4b38118b0e4eb4d311c1d3d8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"e58965ed_44365bb8","updated":"2025-02-16 18:23:22.000000000","message":"Some minor comments. I\u0027m not comfortable to review the code in the flash framework.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"8c602a1aa4237507a835fe2e8aee28d3e348d6f4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"c01d2d0e_728d03b8","updated":"2025-03-01 18:56:30.000000000","message":"Additionally, I fixed the sort of macros, accidentally I sorted it with `SFLASH_CFG_SIZE` macro, so I put it again on top of the definitions of other struct fields.","commit_id":"499200c8541036b9927798e91553a98b175bc35f"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d760ca6b9d39c8d3ff27aba560f1b27810c59dec","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"752efb3a_2ddadb16","updated":"2025-02-26 17:04:02.000000000","message":"Hi Antonio and Tomas. Thank you for your review.\nI tried to implement all comments and also took a look over the code for other issues, hopefully it\u0027s better now. I tested this patch with both slow and fast adapter, on BL702 and BL702L (BL602 is similar as BL702, so that will be working for sure). Additionally, when the parent PR #8593 will be merged, I will rebase it on master.","commit_id":"499200c8541036b9927798e91553a98b175bc35f"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d0bdaf5414547e627c3f77d4de5e0881c9f031a4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"0c5be6f1_ba135451","updated":"2025-09-18 09:28:06.000000000","message":"I am bumping this patch. I checked out the history, but it seems everything got resolved. I remember there were some stuff I wanted to update, but since it\u0027s some time since I worked on this, I do not remember what exactly I wanted to change. Anyway, from my tests, the driver was working really nice, so I think this is sufficient for beginning, and if something will pop up, I will fix it right away.","commit_id":"90d5ccc750532ecf4882e53a511f40e0eb34324d"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"dff947329eb197c68457ac2d0b87f6315212fe1d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"75332699_7108e5ce","updated":"2025-09-18 09:56:52.000000000","message":"Looks good, just minor comments.\nBecause it have been half year since the last jenkins compilation,\nplease rebase to the current git master to check with latest source.\nThanks!","commit_id":"90d5ccc750532ecf4882e53a511f40e0eb34324d"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"6bdb38c011ce6274c80488ee0b87f0b91bf1a845","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"0cdbffff_cb7632a0","updated":"2025-09-22 15:07:53.000000000","message":"Thanks!","commit_id":"ba757faba672d82f4c4850eacb8bd7e582f9658a"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"eac6940f57b08d6f4771f60e6353740b2470ae90","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"b7f6c764_da6458cf","in_reply_to":"0cdbffff_cb7632a0","updated":"2025-09-22 16:42:57.000000000","message":"Thank you for fast review 😊","commit_id":"ba757faba672d82f4c4850eacb8bd7e582f9658a"}],"src/flash/nor/bl602.c":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"a49ae3cce766c83d11c624d4f53f118c6bd839b3","unresolved":true,"context_lines":[{"line_number":208,"context_line":"\tunsigned int avail_pages \u003d target_get_working_area_avail(target) / priv-\u003edev-\u003epagesize;"},{"line_number":209,"context_line":"\t/* We try to allocate working area rounded down to device page size,"},{"line_number":210,"context_line":"\t * at least 1 page, at most the write data size */"},{"line_number":211,"context_line":"\tunsigned int chunk_size \u003d MIN(MAX(avail_pages - 1, 1) * priv-\u003edev-\u003epagesize, count);"},{"line_number":212,"context_line":"\tretval \u003d target_alloc_working_area(target, chunk_size, working_area);"},{"line_number":213,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":214,"context_line":"\t\tLOG_ERROR(\"Could not allocate bounce buffer for flash manipulation. Can\u0027t continue\");"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"d5dc60e0_e0103bea","line":211,"range":{"start_line":211,"start_character":46,"end_line":211,"end_character":50},"updated":"2024-12-12 13:04:56.000000000","message":"Does it need to be decrement?\n`avail_pages` * pagesize \u003c\u003d working_area_avail","commit_id":"ef3baf4b868a831c80933028c82a86af40910f2a"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"57e865ff6be01614ff1083ed2ad35c0602a4bbbd","unresolved":false,"context_lines":[{"line_number":208,"context_line":"\tunsigned int avail_pages \u003d target_get_working_area_avail(target) / priv-\u003edev-\u003epagesize;"},{"line_number":209,"context_line":"\t/* We try to allocate working area rounded down to device page size,"},{"line_number":210,"context_line":"\t * at least 1 page, at most the write data size */"},{"line_number":211,"context_line":"\tunsigned int chunk_size \u003d MIN(MAX(avail_pages - 1, 1) * priv-\u003edev-\u003epagesize, count);"},{"line_number":212,"context_line":"\tretval \u003d target_alloc_working_area(target, chunk_size, working_area);"},{"line_number":213,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":214,"context_line":"\t\tLOG_ERROR(\"Could not allocate bounce buffer for flash manipulation. Can\u0027t continue\");"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"117d62fc_95bf706b","line":211,"range":{"start_line":211,"start_character":46,"end_line":211,"end_character":50},"in_reply_to":"412b3c0a_42b8186f","updated":"2024-12-17 14:53:22.000000000","message":"Ok then","commit_id":"ef3baf4b868a831c80933028c82a86af40910f2a"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"8f5f78858b0a56d36ba57289edb615c3fa68341b","unresolved":true,"context_lines":[{"line_number":208,"context_line":"\tunsigned int avail_pages \u003d target_get_working_area_avail(target) / priv-\u003edev-\u003epagesize;"},{"line_number":209,"context_line":"\t/* We try to allocate working area rounded down to device page size,"},{"line_number":210,"context_line":"\t * at least 1 page, at most the write data size */"},{"line_number":211,"context_line":"\tunsigned int chunk_size \u003d MIN(MAX(avail_pages - 1, 1) * priv-\u003edev-\u003epagesize, count);"},{"line_number":212,"context_line":"\tretval \u003d target_alloc_working_area(target, chunk_size, working_area);"},{"line_number":213,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":214,"context_line":"\t\tLOG_ERROR(\"Could not allocate bounce buffer for flash manipulation. Can\u0027t continue\");"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"412b3c0a_42b8186f","line":211,"range":{"start_line":211,"start_character":46,"end_line":211,"end_character":50},"in_reply_to":"d5dc60e0_e0103bea","updated":"2024-12-16 22:31:53.000000000","message":"My intention with this was, that we keep at least one page free, in case some function after allocating bounce buffer will need some working area as well.","commit_id":"ef3baf4b868a831c80933028c82a86af40910f2a"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"a49ae3cce766c83d11c624d4f53f118c6bd839b3","unresolved":true,"context_lines":[{"line_number":700,"context_line":"\treturn bl602_probe(bank);"},{"line_number":701,"context_line":"}"},{"line_number":702,"context_line":""},{"line_number":703,"context_line":"static void bl602_free_driver_priv(struct flash_bank *bank)"},{"line_number":704,"context_line":"{"},{"line_number":705,"context_line":"\tfree(bank-\u003edriver_priv);"},{"line_number":706,"context_line":"\tbank-\u003edriver_priv \u003d NULL;"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"c154e10d_54bd854d","line":703,"range":{"start_line":703,"start_character":0,"end_line":703,"end_character":59},"updated":"2024-12-12 13:04:56.000000000","message":"Use default_flash_free_driver_priv() instead.","commit_id":"ef3baf4b868a831c80933028c82a86af40910f2a"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"8f5f78858b0a56d36ba57289edb615c3fa68341b","unresolved":false,"context_lines":[{"line_number":700,"context_line":"\treturn bl602_probe(bank);"},{"line_number":701,"context_line":"}"},{"line_number":702,"context_line":""},{"line_number":703,"context_line":"static void bl602_free_driver_priv(struct flash_bank *bank)"},{"line_number":704,"context_line":"{"},{"line_number":705,"context_line":"\tfree(bank-\u003edriver_priv);"},{"line_number":706,"context_line":"\tbank-\u003edriver_priv \u003d NULL;"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"c9b00a21_846d05bb","line":703,"range":{"start_line":703,"start_character":0,"end_line":703,"end_character":59},"in_reply_to":"c154e10d_54bd854d","updated":"2024-12-16 22:31:53.000000000","message":"Done","commit_id":"ef3baf4b868a831c80933028c82a86af40910f2a"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"560bf58af9b40aaa4b38118b0e4eb4d311c1d3d8","unresolved":true,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"// SFlash CFG definitions"},{"line_number":36,"context_line":"#define SFLASH_CFG_SIZE 84"},{"line_number":37,"context_line":"#define SFLASH_CFG_JEDEC_ID_CMD_POS 0x08"},{"line_number":38,"context_line":"#define SFLASH_CFG_JEDEC_ID_CMD_DMY_CLK_POS 0x09"},{"line_number":39,"context_line":"#define SFLASH_CFG_PAGE_SIZE_POS 0x0E"},{"line_number":40,"context_line":"#define SFLASH_CFG_PAGE_PROGRAM_CMD_POS 0x15"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"17d631fe_82fd3db1","line":37,"updated":"2025-02-16 18:23:22.000000000","message":"What about sorting these `SFLASH_CFG_*_POS` by value and add some space to align the values in the same column?\nThis helps verifying there are no duplications","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d760ca6b9d39c8d3ff27aba560f1b27810c59dec","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"// SFlash CFG definitions"},{"line_number":36,"context_line":"#define SFLASH_CFG_SIZE 84"},{"line_number":37,"context_line":"#define SFLASH_CFG_JEDEC_ID_CMD_POS 0x08"},{"line_number":38,"context_line":"#define SFLASH_CFG_JEDEC_ID_CMD_DMY_CLK_POS 0x09"},{"line_number":39,"context_line":"#define SFLASH_CFG_PAGE_SIZE_POS 0x0E"},{"line_number":40,"context_line":"#define SFLASH_CFG_PAGE_PROGRAM_CMD_POS 0x15"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"ee2fc712_e5c09ff0","line":37,"in_reply_to":"17d631fe_82fd3db1","updated":"2025-02-26 17:04:02.000000000","message":"This is good idea, done.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"560bf58af9b40aaa4b38118b0e4eb4d311c1d3d8","unresolved":true,"context_lines":[{"line_number":59,"context_line":"};"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"struct bl602_part_info {"},{"line_number":62,"context_line":"\tconst uint32_t idcode;"},{"line_number":63,"context_line":"\tconst enum bflb_series series;"},{"line_number":64,"context_line":"\tconst uint32_t romapi_get_jedec_id;"},{"line_number":65,"context_line":"\tconst uint32_t romapi_sflash_init_gpio;"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"0da245fb_5b58b260","line":62,"updated":"2025-02-16 18:23:22.000000000","message":"I don\u0027t think you need to specify `const` for each member of the struct, as below you are using `const` for the whole struct.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d760ca6b9d39c8d3ff27aba560f1b27810c59dec","unresolved":false,"context_lines":[{"line_number":59,"context_line":"};"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"struct bl602_part_info {"},{"line_number":62,"context_line":"\tconst uint32_t idcode;"},{"line_number":63,"context_line":"\tconst enum bflb_series series;"},{"line_number":64,"context_line":"\tconst uint32_t romapi_get_jedec_id;"},{"line_number":65,"context_line":"\tconst uint32_t romapi_sflash_init_gpio;"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"41245e10_7f75d1ed","line":62,"in_reply_to":"0da245fb_5b58b260","updated":"2025-02-26 17:04:02.000000000","message":"Done","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"560bf58af9b40aaa4b38118b0e4eb4d311c1d3d8","unresolved":true,"context_lines":[{"line_number":116,"context_line":"\tuint32_t arg_data[], unsigned int n_args, uint32_t *return_data, unsigned int timeout_ms)"},{"line_number":117,"context_line":"{"},{"line_number":118,"context_line":"\tint retval;"},{"line_number":119,"context_line":"\tstatic char * const reg_names[] \u003d { \"a0\", \"a1\", \"a2\", \"a3\", \"a4\", \"a5\" };"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"\tassert(n_args \u003c\u003d ARRAY_SIZE(reg_names)); // only allow register arguments"},{"line_number":122,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":13,"id":"06696382_db0a6b66","line":119,"updated":"2025-02-16 18:23:22.000000000","message":"You can drop `static` and `const`. This array is declared and used inside this function within the next few lines","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c78cf095c9f6246b89a33ba493c3a5638991eddf","unresolved":true,"context_lines":[{"line_number":116,"context_line":"\tuint32_t arg_data[], unsigned int n_args, uint32_t *return_data, unsigned int timeout_ms)"},{"line_number":117,"context_line":"{"},{"line_number":118,"context_line":"\tint retval;"},{"line_number":119,"context_line":"\tstatic char * const reg_names[] \u003d { \"a0\", \"a1\", \"a2\", \"a3\", \"a4\", \"a5\" };"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"\tassert(n_args \u003c\u003d ARRAY_SIZE(reg_names)); // only allow register arguments"},{"line_number":122,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":13,"id":"70122d69_a3ced6a7","line":119,"in_reply_to":"0140e3f8_005899b3","updated":"2025-02-26 22:35:18.000000000","message":"Checkpatch is just a tool, and it also has false positive, so always double check its result. We have ways to silent it when it fails, see in HACKING file.\n\nAnyway here:\n`char *reg_names[] \u003d { \"a0\", \"a1\", \"a2\", \"a3\", \"a4\", \"a5\" };`\nis an array of `char *`, initialized with some content.\n\n`char * const reg_names[] \u003d ...`\nmeans that the array of pointers is `const`, so you cannot run\n`reg_names[1] \u003d \"hello\";`\nbut you can still change the bytes in the strings pointed by the array\n`reg_name[1][0] \u003d \u0027b\u0027;`\n\n`const char *reg_names[] \u003d ...`\ninstead means you can now change the pointer, but not the bytes in the strings\n\nCheckpatch suggest that you probably want to have all const, both the pointers and the strings.\nI think it\u0027s right, that\u0027s the way to assure it is not allocated nor copied at runtime.\n\nBut, as you write, this is passed as argument to\n```\nvoid init_reg_param(struct reg_param *param,\n                char *reg_name, uint32_t size, enum param_direction dir);\n```\nthat (crap!) wants `char *reg_name` ! This must be fixed! \nPlease consider https://review.openocd.org/c/openocd/+/8797\nThanks for reporting it","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"b6616365ff235e01621f124c64cd3812a2ee9369","unresolved":true,"context_lines":[{"line_number":116,"context_line":"\tuint32_t arg_data[], unsigned int n_args, uint32_t *return_data, unsigned int timeout_ms)"},{"line_number":117,"context_line":"{"},{"line_number":118,"context_line":"\tint retval;"},{"line_number":119,"context_line":"\tstatic char * const reg_names[] \u003d { \"a0\", \"a1\", \"a2\", \"a3\", \"a4\", \"a5\" };"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"\tassert(n_args \u003c\u003d ARRAY_SIZE(reg_names)); // only allow register arguments"},{"line_number":122,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":13,"id":"9fe8ad1d_d06926e7","line":119,"in_reply_to":"06696382_db0a6b66","updated":"2025-02-16 20:31:55.000000000","message":"Or use `static const char *reg_names` because the array will be placed in a loaded segment and not allocated on stack and then initialized - therefore produces shorter code.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"8c602a1aa4237507a835fe2e8aee28d3e348d6f4","unresolved":false,"context_lines":[{"line_number":116,"context_line":"\tuint32_t arg_data[], unsigned int n_args, uint32_t *return_data, unsigned int timeout_ms)"},{"line_number":117,"context_line":"{"},{"line_number":118,"context_line":"\tint retval;"},{"line_number":119,"context_line":"\tstatic char * const reg_names[] \u003d { \"a0\", \"a1\", \"a2\", \"a3\", \"a4\", \"a5\" };"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"\tassert(n_args \u003c\u003d ARRAY_SIZE(reg_names)); // only allow register arguments"},{"line_number":122,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":13,"id":"4585b39c_d4cfbb71","line":119,"in_reply_to":"70122d69_a3ced6a7","updated":"2025-03-01 18:56:30.000000000","message":"Thanks for HACKING file tip and great explanation of array allocation.\nAbout the issue: Thank you for creating the `init_reg_param` patch and merging it so fast! I rebased this PR on it, and changed it to `static const char * const reg_names[]`, based on your suggestion.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d760ca6b9d39c8d3ff27aba560f1b27810c59dec","unresolved":true,"context_lines":[{"line_number":116,"context_line":"\tuint32_t arg_data[], unsigned int n_args, uint32_t *return_data, unsigned int timeout_ms)"},{"line_number":117,"context_line":"{"},{"line_number":118,"context_line":"\tint retval;"},{"line_number":119,"context_line":"\tstatic char * const reg_names[] \u003d { \"a0\", \"a1\", \"a2\", \"a3\", \"a4\", \"a5\" };"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"\tassert(n_args \u003c\u003d ARRAY_SIZE(reg_names)); // only allow register arguments"},{"line_number":122,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":13,"id":"0140e3f8_005899b3","line":119,"in_reply_to":"9fe8ad1d_d06926e7","updated":"2025-02-26 17:04:02.000000000","message":"I went with Antonio\u0027s suggestion, because `static const char *reg_names` was causing troubles passing `const char*` into `char*` in `init_reg_param` function (I still can recast it, but is this good to do?). \n\nBut dropping `static` and `const` wasn\u0027t passing checkpatch.sh rules: `ERROR:STATIC_CONST_CHAR_ARRAY: char * array declaration might be better as static const`, so I kept it as is. What would be best to do in this case?","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"560bf58af9b40aaa4b38118b0e4eb4d311c1d3d8","unresolved":true,"context_lines":[{"line_number":126,"context_line":"\t\tebreak(),"},{"line_number":127,"context_line":"\t};"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"\tretval \u003d target_alloc_working_area(target, sizeof(trampoline_code),"},{"line_number":130,"context_line":"\t\t\t\u0026trampoline_algorithm);"},{"line_number":131,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":132,"context_line":"\t\tLOG_WARNING(\"No working area available, can\u0027t do trampoline\");"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"ef028c05_e258c090","line":129,"updated":"2025-02-16 18:23:22.000000000","message":"The coding style requires to declare the variable at the first use, when it makes sense. Here:\n`int retval \u003d target_alloc_working_area(...);`\nmore case below.\nCheck `doc/manual/style.txt`","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d760ca6b9d39c8d3ff27aba560f1b27810c59dec","unresolved":false,"context_lines":[{"line_number":126,"context_line":"\t\tebreak(),"},{"line_number":127,"context_line":"\t};"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"\tretval \u003d target_alloc_working_area(target, sizeof(trampoline_code),"},{"line_number":130,"context_line":"\t\t\t\u0026trampoline_algorithm);"},{"line_number":131,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":132,"context_line":"\t\tLOG_WARNING(\"No working area available, can\u0027t do trampoline\");"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"ef719495_0821a059","line":129,"in_reply_to":"0e28ed7c_92aabec9","updated":"2025-02-26 17:04:02.000000000","message":"Oh, sorry, I missed this from the code style rules.\nI am used to what Tomas is suggesting, although, since this is stated in the code style guidelines, I decided to follow it.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"48118ce62a66bd11b6be6372729c61d6522de89d","unresolved":true,"context_lines":[{"line_number":126,"context_line":"\t\tebreak(),"},{"line_number":127,"context_line":"\t};"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"\tretval \u003d target_alloc_working_area(target, sizeof(trampoline_code),"},{"line_number":130,"context_line":"\t\t\t\u0026trampoline_algorithm);"},{"line_number":131,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":132,"context_line":"\t\tLOG_WARNING(\"No working area available, can\u0027t do trampoline\");"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"fb6151aa_ebd5e2f2","line":129,"in_reply_to":"ef028c05_e258c090","updated":"2025-02-16 19:41:25.000000000","message":"Antonio, I would vote here for the exception from the rule:\n`retval` is used so frequently that it pays off to declare at the beginning of procedure than move the declaration point several times during code development (every time you add some call before the `retval` declaration). And there is no doubt what the variable is used for. What do you think?","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"8ee7d5dbcd586bbf2d79e5057ff1a96af302ae9e","unresolved":true,"context_lines":[{"line_number":126,"context_line":"\t\tebreak(),"},{"line_number":127,"context_line":"\t};"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"\tretval \u003d target_alloc_working_area(target, sizeof(trampoline_code),"},{"line_number":130,"context_line":"\t\t\t\u0026trampoline_algorithm);"},{"line_number":131,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":132,"context_line":"\t\tLOG_WARNING(\"No working area available, can\u0027t do trampoline\");"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"0e28ed7c_92aabec9","line":129,"in_reply_to":"fb6151aa_ebd5e2f2","updated":"2025-02-16 19:55:33.000000000","message":"Actually I don\u0027t rally mind, but I\u0027m getting used at this from the coding style.\nThis notation makes the function shorter and easier to read, so why not!\n\nIn other functions below, e.g. bl602_flash_init, retval is initialized to ERROR_OK which makes the reader think about some special return path in the function. Instead it is assigned with a new value few lines after! By declaring retval at the first assignment it makes the code cleaner. I did not highlighted it in that function because I already put this comment here.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"560bf58af9b40aaa4b38118b0e4eb4d311c1d3d8","unresolved":true,"context_lines":[{"line_number":130,"context_line":"\t\t\t\u0026trampoline_algorithm);"},{"line_number":131,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":132,"context_line":"\t\tLOG_WARNING(\"No working area available, can\u0027t do trampoline\");"},{"line_number":133,"context_line":"\t\treturn ERROR_TARGET_RESOURCE_NOT_AVAILABLE;"},{"line_number":134,"context_line":"\t}"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"\tretval \u003d target_write_buffer(target, trampoline_algorithm-\u003eaddress,"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"b6d90f69_08fd05e4","line":133,"updated":"2025-02-16 18:23:22.000000000","message":"why you don\u0027t `return retval;` ?\nAny reason to overwrite the returned value?\nThere are other 3 cases below.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d760ca6b9d39c8d3ff27aba560f1b27810c59dec","unresolved":false,"context_lines":[{"line_number":130,"context_line":"\t\t\t\u0026trampoline_algorithm);"},{"line_number":131,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":132,"context_line":"\t\tLOG_WARNING(\"No working area available, can\u0027t do trampoline\");"},{"line_number":133,"context_line":"\t\treturn ERROR_TARGET_RESOURCE_NOT_AVAILABLE;"},{"line_number":134,"context_line":"\t}"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"\tretval \u003d target_write_buffer(target, trampoline_algorithm-\u003eaddress,"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"4663a18e_cf3c26e7","line":133,"in_reply_to":"b6d90f69_08fd05e4","updated":"2025-02-26 17:04:02.000000000","message":"Honestly, no reason, as this part of code was inspired from another driver, where it was done this way. Additionally, I noticed LOG_WARNING is used, what according to code style rules, isn\u0027t appropriate log level for such kind of failure.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"560bf58af9b40aaa4b38118b0e4eb4d311c1d3d8","unresolved":true,"context_lines":[{"line_number":201,"context_line":"static int bl602_alloc_bounce_buffer(struct flash_bank *bank,"},{"line_number":202,"context_line":"\t\tstruct working_area **working_area, uint32_t count)"},{"line_number":203,"context_line":"{"},{"line_number":204,"context_line":"\tint retval \u003d ERROR_OK;"},{"line_number":205,"context_line":"\tstruct bl602_flash_bank *priv \u003d bank-\u003edriver_priv;"},{"line_number":206,"context_line":"\tstruct target *target \u003d bank-\u003etarget;"},{"line_number":207,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":13,"id":"cd63003b_2debb920","line":204,"updated":"2025-02-16 18:23:22.000000000","message":"No need for this initialization.\nDirectly below:\n`int retval \u003d ...`\nOther similar cases below","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d760ca6b9d39c8d3ff27aba560f1b27810c59dec","unresolved":false,"context_lines":[{"line_number":201,"context_line":"static int bl602_alloc_bounce_buffer(struct flash_bank *bank,"},{"line_number":202,"context_line":"\t\tstruct working_area **working_area, uint32_t count)"},{"line_number":203,"context_line":"{"},{"line_number":204,"context_line":"\tint retval \u003d ERROR_OK;"},{"line_number":205,"context_line":"\tstruct bl602_flash_bank *priv \u003d bank-\u003edriver_priv;"},{"line_number":206,"context_line":"\tstruct target *target \u003d bank-\u003etarget;"},{"line_number":207,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":13,"id":"1628550a_e42148c3","line":204,"in_reply_to":"cd63003b_2debb920","updated":"2025-02-26 17:04:02.000000000","message":"Done","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"560bf58af9b40aaa4b38118b0e4eb4d311c1d3d8","unresolved":true,"context_lines":[{"line_number":208,"context_line":"\tunsigned int avail_pages \u003d target_get_working_area_avail(target) / priv-\u003edev-\u003epagesize;"},{"line_number":209,"context_line":"\t/* We try to allocate working area rounded down to device page size,"},{"line_number":210,"context_line":"\t * at least 1 page, at most the write data size */"},{"line_number":211,"context_line":"\tunsigned int chunk_size \u003d MIN(MAX(avail_pages - 1, 1) * priv-\u003edev-\u003epagesize, count);"},{"line_number":212,"context_line":"\tretval \u003d target_alloc_working_area(target, chunk_size, working_area);"},{"line_number":213,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":214,"context_line":"\t\tLOG_ERROR(\"Could not allocate bounce buffer for flash manipulation. Can\u0027t continue\");"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"7e77363b_5729750e","line":211,"updated":"2025-02-16 18:23:22.000000000","message":"What if there is no working area and `avail_pages \u003d\u003d 0` ?\n`avail_pages - 1` becomes UINT_MAX.\n\nBut the real question is why you need `avail_pages - 1` ? Do you need to allocate another page somewhere else?\nPlease notice that the unsigned integer division above is already operating the rounding-down of the result.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d760ca6b9d39c8d3ff27aba560f1b27810c59dec","unresolved":false,"context_lines":[{"line_number":208,"context_line":"\tunsigned int avail_pages \u003d target_get_working_area_avail(target) / priv-\u003edev-\u003epagesize;"},{"line_number":209,"context_line":"\t/* We try to allocate working area rounded down to device page size,"},{"line_number":210,"context_line":"\t * at least 1 page, at most the write data size */"},{"line_number":211,"context_line":"\tunsigned int chunk_size \u003d MIN(MAX(avail_pages - 1, 1) * priv-\u003edev-\u003epagesize, count);"},{"line_number":212,"context_line":"\tretval \u003d target_alloc_working_area(target, chunk_size, working_area);"},{"line_number":213,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":214,"context_line":"\t\tLOG_ERROR(\"Could not allocate bounce buffer for flash manipulation. Can\u0027t continue\");"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"489bf7eb_724dd238","line":211,"in_reply_to":"7e77363b_5729750e","updated":"2025-02-26 17:04:02.000000000","message":"In terms of my code, there is no need for allocating another page. Although, since I do not know how exactly other parts work (such as algorithms), I wanted to always allocate one page less than maximum available, just to be sure anything else have enough space. I added check for `available_pages \u003d\u003d 0`. Although, if this is bad practice, let me know, will change it.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"560bf58af9b40aaa4b38118b0e4eb4d311c1d3d8","unresolved":true,"context_lines":[{"line_number":287,"context_line":"\t}"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"\t// initialize flash GPIOs"},{"line_number":290,"context_line":"\t{"},{"line_number":291,"context_line":"\t\tuint32_t args[] \u003d {"},{"line_number":292,"context_line":"\t\t\tflash_pin_cfg,"},{"line_number":293,"context_line":"\t\t\t1,\t// restoreDefault"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"541f3495_953789e1","line":290,"updated":"2025-02-16 18:23:22.000000000","message":"I don\u0027t think you need to open a new block of code.\nYou should be able to declare the array `args` in the middle of the function.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d760ca6b9d39c8d3ff27aba560f1b27810c59dec","unresolved":false,"context_lines":[{"line_number":287,"context_line":"\t}"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"\t// initialize flash GPIOs"},{"line_number":290,"context_line":"\t{"},{"line_number":291,"context_line":"\t\tuint32_t args[] \u003d {"},{"line_number":292,"context_line":"\t\t\tflash_pin_cfg,"},{"line_number":293,"context_line":"\t\t\t1,\t// restoreDefault"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"4a77237a_bb7c4362","line":290,"in_reply_to":"541f3495_953789e1","updated":"2025-02-26 17:04:02.000000000","message":"Done","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"560bf58af9b40aaa4b38118b0e4eb4d311c1d3d8","unresolved":true,"context_lines":[{"line_number":294,"context_line":"\t\t};"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"\t\tretval \u003d bl602_call_romapi_func(target, part_info-\u003eromapi_sflash_init_gpio,"},{"line_number":297,"context_line":"\t\t\t\targs, ARRAY_SIZE(args), NULL, 3000);"},{"line_number":298,"context_line":"\t\tif (retval !\u003d ERROR_OK) {"},{"line_number":299,"context_line":"\t\t\tLOG_ERROR(\"Failed to invoke spi flash gpio init function\");"},{"line_number":300,"context_line":"\t\t\treturn retval;"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"a3b180d6_cdf4e1be","line":297,"updated":"2025-02-16 18:23:22.000000000","message":"Use macros for the timeout values. You have `3000` and in one case `20000`, but I don\u0027t know if it makes sense to use a single macro for `3000` or multiple macros with the same value.\nUse reasonable names for the macros, with `TIMEOUT` in the name and suffix `_MS`.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d760ca6b9d39c8d3ff27aba560f1b27810c59dec","unresolved":false,"context_lines":[{"line_number":294,"context_line":"\t\t};"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"\t\tretval \u003d bl602_call_romapi_func(target, part_info-\u003eromapi_sflash_init_gpio,"},{"line_number":297,"context_line":"\t\t\t\targs, ARRAY_SIZE(args), NULL, 3000);"},{"line_number":298,"context_line":"\t\tif (retval !\u003d ERROR_OK) {"},{"line_number":299,"context_line":"\t\t\tLOG_ERROR(\"Failed to invoke spi flash gpio init function\");"},{"line_number":300,"context_line":"\t\t\treturn retval;"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"b44d09a8_c35900d7","line":297,"in_reply_to":"a3b180d6_cdf4e1be","updated":"2025-02-26 17:04:02.000000000","message":"Oh, I missed that `20000` ms timeout, it was left-over from debugging. I chose 5000ms default timeout for every action, as in some scenarios communication with Flash could be slow.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"560bf58af9b40aaa4b38118b0e4eb4d311c1d3d8","unresolved":true,"context_lines":[{"line_number":322,"context_line":""},{"line_number":323,"context_line":"\tif (part_info-\u003eseries \u003d\u003d BFLB_SERIES_BL602 ||"},{"line_number":324,"context_line":"\t\tpart_info-\u003eseries \u003d\u003d BFLB_SERIES_BL702) {"},{"line_number":325,"context_line":"\t\tstatic const uint8_t bl602_sflash_ctrl_cfg[] \u003d {"},{"line_number":326,"context_line":"\t\t\t0x00, 0x00, 0x00, 0x00, 0x01, 0x01, 0x00, 0x00, 0x00"},{"line_number":327,"context_line":"\t\t};"},{"line_number":328,"context_line":"\t\tsflash_ctrl_cfg \u003d bl602_sflash_ctrl_cfg;"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"ce837d66_5a820578","line":325,"updated":"2025-02-16 18:23:22.000000000","message":"Move this array and `bl702l_sflash_ctrl_cfg[]` outside the if/then/else.\nThis will improve the code readability and make useless the `static` attribute.\nYou could even move them outside this function (but keep them just before the function to help readability), but in this case you should keep them `static`.\n\nIn this function it could be more readable using switch/case instead of if/then/else. Not sure it can work in all the other functions","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d760ca6b9d39c8d3ff27aba560f1b27810c59dec","unresolved":false,"context_lines":[{"line_number":322,"context_line":""},{"line_number":323,"context_line":"\tif (part_info-\u003eseries \u003d\u003d BFLB_SERIES_BL602 ||"},{"line_number":324,"context_line":"\t\tpart_info-\u003eseries \u003d\u003d BFLB_SERIES_BL702) {"},{"line_number":325,"context_line":"\t\tstatic const uint8_t bl602_sflash_ctrl_cfg[] \u003d {"},{"line_number":326,"context_line":"\t\t\t0x00, 0x00, 0x00, 0x00, 0x01, 0x01, 0x00, 0x00, 0x00"},{"line_number":327,"context_line":"\t\t};"},{"line_number":328,"context_line":"\t\tsflash_ctrl_cfg \u003d bl602_sflash_ctrl_cfg;"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"9a50bf87_7cba3b12","line":325,"in_reply_to":"c66d574d_2023dc15","updated":"2025-02-26 17:04:02.000000000","message":"I moved the configs outside of the function.\n\nAbout switch/case, I changed it there as well, but if/then/else in `bl602_flash_gpio_init`, as here it looks better when using ifs by my opinion.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"b6616365ff235e01621f124c64cd3812a2ee9369","unresolved":true,"context_lines":[{"line_number":322,"context_line":""},{"line_number":323,"context_line":"\tif (part_info-\u003eseries \u003d\u003d BFLB_SERIES_BL602 ||"},{"line_number":324,"context_line":"\t\tpart_info-\u003eseries \u003d\u003d BFLB_SERIES_BL702) {"},{"line_number":325,"context_line":"\t\tstatic const uint8_t bl602_sflash_ctrl_cfg[] \u003d {"},{"line_number":326,"context_line":"\t\t\t0x00, 0x00, 0x00, 0x00, 0x01, 0x01, 0x00, 0x00, 0x00"},{"line_number":327,"context_line":"\t\t};"},{"line_number":328,"context_line":"\t\tsflash_ctrl_cfg \u003d bl602_sflash_ctrl_cfg;"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"c66d574d_2023dc15","line":325,"in_reply_to":"ce837d66_5a820578","updated":"2025-02-16 20:31:55.000000000","message":"BTW local `static const` has a good reason even if factored out of conditional block: the data resides in a loaded segment unlike the local \u0027const\u0027 which is allocated on the stack and initialized.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"560bf58af9b40aaa4b38118b0e4eb4d311c1d3d8","unresolved":true,"context_lines":[{"line_number":374,"context_line":"\treturn retval;"},{"line_number":375,"context_line":"}"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"static int bl602_flash_read_id(struct flash_bank *bank,"},{"line_number":378,"context_line":"\t\tuint32_t *jedec_id)"},{"line_number":379,"context_line":"{"},{"line_number":380,"context_line":"\tstruct bl602_flash_bank *priv \u003d bank-\u003edriver_priv;"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"1151f5da_b55ba931","line":377,"updated":"2025-02-16 18:23:22.000000000","message":"can fit in a single line","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d760ca6b9d39c8d3ff27aba560f1b27810c59dec","unresolved":false,"context_lines":[{"line_number":374,"context_line":"\treturn retval;"},{"line_number":375,"context_line":"}"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"static int bl602_flash_read_id(struct flash_bank *bank,"},{"line_number":378,"context_line":"\t\tuint32_t *jedec_id)"},{"line_number":379,"context_line":"{"},{"line_number":380,"context_line":"\tstruct bl602_flash_bank *priv \u003d bank-\u003edriver_priv;"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"da556916_fad57792","line":377,"in_reply_to":"1151f5da_b55ba931","updated":"2025-02-26 17:04:02.000000000","message":"Done","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"560bf58af9b40aaa4b38118b0e4eb4d311c1d3d8","unresolved":true,"context_lines":[{"line_number":383,"context_line":"\tint retval \u003d ERROR_OK;"},{"line_number":384,"context_line":""},{"line_number":385,"context_line":"\tstruct working_area *data_area;"},{"line_number":386,"context_line":"\tretval \u003d target_alloc_working_area(target, sizeof(uint32_t) + 84, \u0026data_area);"},{"line_number":387,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":388,"context_line":"\t\tLOG_WARNING(\"No working area available, can\u0027t read flash id\");"},{"line_number":389,"context_line":"\t\treturn ERROR_TARGET_RESOURCE_NOT_AVAILABLE;"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"030404b1_74c754a4","line":386,"updated":"2025-02-16 18:23:22.000000000","message":"From where this `84` comes from?\nIs it `SFLASH_CFG_SIZE`?\nOtherwise please define a macro with a reasonable name for this value or directly for the value `sizeof(uint32_t) + 84`","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d760ca6b9d39c8d3ff27aba560f1b27810c59dec","unresolved":false,"context_lines":[{"line_number":383,"context_line":"\tint retval \u003d ERROR_OK;"},{"line_number":384,"context_line":""},{"line_number":385,"context_line":"\tstruct working_area *data_area;"},{"line_number":386,"context_line":"\tretval \u003d target_alloc_working_area(target, sizeof(uint32_t) + 84, \u0026data_area);"},{"line_number":387,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":388,"context_line":"\t\tLOG_WARNING(\"No working area available, can\u0027t read flash id\");"},{"line_number":389,"context_line":"\t\treturn ERROR_TARGET_RESOURCE_NOT_AVAILABLE;"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"260b9aca_307864c7","line":386,"in_reply_to":"030404b1_74c754a4","updated":"2025-02-26 17:04:02.000000000","message":"Yes, it is SFLASH_CFG_SIZE. Changed, thanks.","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"560bf58af9b40aaa4b38118b0e4eb4d311c1d3d8","unresolved":true,"context_lines":[{"line_number":679,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":680,"context_line":""},{"line_number":681,"context_line":"\tpriv-\u003esflash_cfg[SFLASH_CFG_PAGE_PROGRAM_CMD_POS] \u003d priv-\u003edev-\u003epprog_cmd;"},{"line_number":682,"context_line":"\tpriv-\u003esflash_cfg[SFLASH_CFG_PAGE_SIZE_POS] \u003d (priv-\u003edev-\u003epagesize \u0026 0xFF);"},{"line_number":683,"context_line":"\tpriv-\u003esflash_cfg[SFLASH_CFG_PAGE_SIZE_POS + 1] \u003d ((priv-\u003edev-\u003epagesize \u003e\u003e 8) \u0026 0xFF);"},{"line_number":684,"context_line":"\tpriv-\u003esflash_cfg[SFLASH_CFG_FAST_READ_CMD_POS] \u003d priv-\u003edev-\u003eread_cmd;"},{"line_number":685,"context_line":"\tpriv-\u003esflash_cfg[SFLASH_CFG_SECTOR_SIZE_POS] \u003d priv-\u003edev-\u003esectorsize / 1024;"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"b766f877_27e797ca","line":682,"updated":"2025-02-16 18:23:22.000000000","message":"no need for the parenthesis `(` and `)`.\nSame in next line","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"d760ca6b9d39c8d3ff27aba560f1b27810c59dec","unresolved":false,"context_lines":[{"line_number":679,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":680,"context_line":""},{"line_number":681,"context_line":"\tpriv-\u003esflash_cfg[SFLASH_CFG_PAGE_PROGRAM_CMD_POS] \u003d priv-\u003edev-\u003epprog_cmd;"},{"line_number":682,"context_line":"\tpriv-\u003esflash_cfg[SFLASH_CFG_PAGE_SIZE_POS] \u003d (priv-\u003edev-\u003epagesize \u0026 0xFF);"},{"line_number":683,"context_line":"\tpriv-\u003esflash_cfg[SFLASH_CFG_PAGE_SIZE_POS + 1] \u003d ((priv-\u003edev-\u003epagesize \u003e\u003e 8) \u0026 0xFF);"},{"line_number":684,"context_line":"\tpriv-\u003esflash_cfg[SFLASH_CFG_FAST_READ_CMD_POS] \u003d priv-\u003edev-\u003eread_cmd;"},{"line_number":685,"context_line":"\tpriv-\u003esflash_cfg[SFLASH_CFG_SECTOR_SIZE_POS] \u003d priv-\u003edev-\u003esectorsize / 1024;"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"2f7ab16b_12c94fff","line":682,"in_reply_to":"b766f877_27e797ca","updated":"2025-02-26 17:04:02.000000000","message":"Done","commit_id":"e7a34833a0e24eabbebd67713c23097754db7be4"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"dff947329eb197c68457ac2d0b87f6315212fe1d","unresolved":true,"context_lines":[{"line_number":13,"context_line":"#include \u003cjtag/jtag.h\u003e"},{"line_number":14,"context_line":"#include \u003ctarget/algorithm.h\u003e"},{"line_number":15,"context_line":"#include \"spi.h\""},{"line_number":16,"context_line":"#include \"../../target/riscv/opcodes.h\""},{"line_number":17,"context_line":"#include \"../../target/riscv/gdb_regs.h\""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"/*"}],"source_content_type":"text/x-csrc","patch_set":15,"id":"d5a8af71_229da8bb","line":16,"range":{"start_line":16,"start_character":10,"end_line":16,"end_character":16},"updated":"2025-09-18 09:56:52.000000000","message":"All is compiled with -I path to src/ dir, so please remove two steps to parent dir.","commit_id":"90d5ccc750532ecf4882e53a511f40e0eb34324d"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"6bdb38c011ce6274c80488ee0b87f0b91bf1a845","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#include \u003cjtag/jtag.h\u003e"},{"line_number":14,"context_line":"#include \u003ctarget/algorithm.h\u003e"},{"line_number":15,"context_line":"#include \"spi.h\""},{"line_number":16,"context_line":"#include \"../../target/riscv/opcodes.h\""},{"line_number":17,"context_line":"#include \"../../target/riscv/gdb_regs.h\""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"/*"}],"source_content_type":"text/x-csrc","patch_set":15,"id":"3e6b3630_c55133fe","line":16,"range":{"start_line":16,"start_character":10,"end_line":16,"end_character":16},"in_reply_to":"d5a8af71_229da8bb","updated":"2025-09-22 15:07:53.000000000","message":"Done","commit_id":"90d5ccc750532ecf4882e53a511f40e0eb34324d"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"dff947329eb197c68457ac2d0b87f6315212fe1d","unresolved":true,"context_lines":[{"line_number":14,"context_line":"#include \u003ctarget/algorithm.h\u003e"},{"line_number":15,"context_line":"#include \"spi.h\""},{"line_number":16,"context_line":"#include \"../../target/riscv/opcodes.h\""},{"line_number":17,"context_line":"#include \"../../target/riscv/gdb_regs.h\""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"/*"},{"line_number":20,"context_line":" * Flash bank driver for Bouffalo chips with BL602-like flash controller"}],"source_content_type":"text/x-csrc","patch_set":15,"id":"39e6cb2c_dd7eb5d5","line":17,"updated":"2025-09-18 09:56:52.000000000","message":"As above","commit_id":"90d5ccc750532ecf4882e53a511f40e0eb34324d"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"6bdb38c011ce6274c80488ee0b87f0b91bf1a845","unresolved":false,"context_lines":[{"line_number":14,"context_line":"#include \u003ctarget/algorithm.h\u003e"},{"line_number":15,"context_line":"#include \"spi.h\""},{"line_number":16,"context_line":"#include \"../../target/riscv/opcodes.h\""},{"line_number":17,"context_line":"#include \"../../target/riscv/gdb_regs.h\""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"/*"},{"line_number":20,"context_line":" * Flash bank driver for Bouffalo chips with BL602-like flash controller"}],"source_content_type":"text/x-csrc","patch_set":15,"id":"6259e141_925b1743","line":17,"in_reply_to":"39e6cb2c_dd7eb5d5","updated":"2025-09-22 15:07:53.000000000","message":"Done","commit_id":"90d5ccc750532ecf4882e53a511f40e0eb34324d"}],"tcl/target/bl602_common.cfg":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"a49ae3cce766c83d11c624d4f53f118c6bd839b3","unresolved":true,"context_lines":[{"line_number":84,"context_line":"\t# memory access, so when this is done by progbuf or sysbus, OpenOCD will fail to read"},{"line_number":85,"context_line":"\t# if write was successful or not, and will print error about that. Since receiving of"},{"line_number":86,"context_line":"\t# this error is expected, we will turn off log printing for a moment,"},{"line_number":87,"context_line":"\tset lvl [lindex [debug_level] 1]"},{"line_number":88,"context_line":"\tdebug_level -1"},{"line_number":89,"context_line":"\t# In GLB_SWRST_CFG2, set CTRL_SYS_RESET, CTRL_CPU_RESET and CTRL_PWRON_RESET to 1"},{"line_number":90,"context_line":"\tcatch {mmw 0x40000018 0x7 0x0}"},{"line_number":91,"context_line":"\tdebug_level $lvl"}],"source_content_type":"text/x-ttcn-cfg","patch_set":10,"id":"bf7e3d6e_909a9608","line":88,"range":{"start_line":87,"start_character":1,"end_line":88,"end_character":15},"updated":"2024-12-12 13:04:56.000000000","message":"Please remove debug_level handling from Tcl script.\n\nI know OpenOCD misses functionality to conceal an expected error.\nHowever changing the debug_level is way to hell. If we suspect some problem in OpenOCD or just giving support we often ask user for **complete -d3 log**. But what if an important problem happens when log is blocked by level -1?\n\nThe script may `echo` text that the following error is expected (until somebody contributes better error handling mechanism).","commit_id":"ef3baf4b868a831c80933028c82a86af40910f2a"},{"author":{"_account_id":1002298,"name":"Marek Kraus","email":"gamelaster@outlook.com","username":"gamelaster"},"change_message_id":"8f5f78858b0a56d36ba57289edb615c3fa68341b","unresolved":false,"context_lines":[{"line_number":84,"context_line":"\t# memory access, so when this is done by progbuf or sysbus, OpenOCD will fail to read"},{"line_number":85,"context_line":"\t# if write was successful or not, and will print error about that. Since receiving of"},{"line_number":86,"context_line":"\t# this error is expected, we will turn off log printing for a moment,"},{"line_number":87,"context_line":"\tset lvl [lindex [debug_level] 1]"},{"line_number":88,"context_line":"\tdebug_level -1"},{"line_number":89,"context_line":"\t# In GLB_SWRST_CFG2, set CTRL_SYS_RESET, CTRL_CPU_RESET and CTRL_PWRON_RESET to 1"},{"line_number":90,"context_line":"\tcatch {mmw 0x40000018 0x7 0x0}"},{"line_number":91,"context_line":"\tdebug_level $lvl"}],"source_content_type":"text/x-ttcn-cfg","patch_set":10,"id":"9fd40733_458ea156","line":88,"range":{"start_line":87,"start_character":1,"end_line":88,"end_character":15},"in_reply_to":"19bee1d9_257a6913","updated":"2024-12-16 22:31:53.000000000","message":"Thank you Tomas and Antonio for feedback. In terms of solving the error message issue, I decided to do second approach Antonio suggest. Thus, directly with RISC-V DMI commands, I am triggering the SW reset via system bus memory access. Thanks to this, no error messages are reported by OpenOCD at all.\n\nAdditionally, as I reported on OpenOCD IRC, reset mechanism is not reliable on slow JTAG adapters. So I reworked that as well, and found better, faster, and very reliable way of doing software reset. I tested it on all chips with various adapters, and it works great. I made the changes to TCL script in parent commit, change #8593 , and rebased this commit/change upon it.","commit_id":"ef3baf4b868a831c80933028c82a86af40910f2a"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"09430a689f9db672c9bb1945b9beea0f9315ed41","unresolved":true,"context_lines":[{"line_number":84,"context_line":"\t# memory access, so when this is done by progbuf or sysbus, OpenOCD will fail to read"},{"line_number":85,"context_line":"\t# if write was successful or not, and will print error about that. Since receiving of"},{"line_number":86,"context_line":"\t# this error is expected, we will turn off log printing for a moment,"},{"line_number":87,"context_line":"\tset lvl [lindex [debug_level] 1]"},{"line_number":88,"context_line":"\tdebug_level -1"},{"line_number":89,"context_line":"\t# In GLB_SWRST_CFG2, set CTRL_SYS_RESET, CTRL_CPU_RESET and CTRL_PWRON_RESET to 1"},{"line_number":90,"context_line":"\tcatch {mmw 0x40000018 0x7 0x0}"},{"line_number":91,"context_line":"\tdebug_level $lvl"}],"source_content_type":"text/x-ttcn-cfg","patch_set":10,"id":"19bee1d9_257a6913","line":88,"range":{"start_line":87,"start_character":1,"end_line":88,"end_character":15},"in_reply_to":"bf7e3d6e_909a9608","updated":"2024-12-15 19:46:32.000000000","message":"What Tomas say is true; a log run with -d3 would be incomplete.\nTo avoid hiding important logs at -d3, it could be better using here:\n```\n\tset lvl [lindex [debug_level] 1]\n\tif {$lvl \u003c\u003d 2} {debug_level -1}\n\t...\n\tdebug_level $lvl\n```\n\nAnother possible approach...\nChecking the code for `write_memory()` in risc-v. Actually there are two implementations, in riscv-011.c and riscv-013.c.\n\nIf your SoC uses riscv-011.c, there should be no error for JTAG not available after the write; eventually a `sleep 100` after the command `mmw` below could be needed to prevent OpenOCD to run a `poll` too early and getting error by the `poll`.\n\nIf your SoC uses riscv-013.c, instead, the code looks like it issues a JTAG read at the end of `write_memory()` to check if the write has completed. This read will get the error of JTAG not available. In this case you could implement here this write without the check by using JTAG low-level primitives `irscan` and `drscan`. It\u0027s not straightforward, but if you dump the JTAG low-level communication you can get most of the info you need.","commit_id":"ef3baf4b868a831c80933028c82a86af40910f2a"}]}