)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"07f46f0b135a113230baac02e82925e7f59da71d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"5943beb3_775e6b9b","updated":"2025-09-24 08:49:36.000000000","message":"Thanks for this patch.\nI didn\u0027t run a full review as I want to address, at first some legal concern.\nSome of the files in `contrib` folder, if I correctly searched, are generic files taken from the \u0027Microsemi Libero Software\u0027 distribution library, but I cannot find a public and complete version of the original SW.\n\n1) all the files I have found in github have the original copyright of Microsemi. This should be kept in the files in OpenOCD\n\n2) I cannot identify under which license the files are distributed. You have added the GPL tag in the SPDX. That\u0027s great! But I want to be sure this is allowed by the original license of these files.\n\n3) For future maintenance of this code, it could be great to have in a file `contrib/loaders/flash/microchip/pic64gx/readme.txt` the URL to recover the whole library and the version of the library used in the current code.","commit_id":"185e17fe34022497952d644030ef326f7c928415"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"30bbf70c081d4aed3adbfaf1c11c27bd23f2a96f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":12,"id":"20f2fa01_bce44188","in_reply_to":"5943beb3_775e6b9b","updated":"2025-09-24 19:28:08.000000000","message":"Uhh, I missed this comment. Probably should not be marked as resolved.\n\nSome files are in\nhttps://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/614a67abb3023ba47ea6d1b8d7b9a9997353e007/src/platform/drivers/mss/mss_sys_services/\nand they are licenced under MIT. This permissive licence is compatible so there is no need to re-licencing them to GPL2.","commit_id":"185e17fe34022497952d644030ef326f7c928415"}],"contrib/loaders/flash/mchp_flash/e51.c":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"22471fdcf6fd761a39963df9bda7867ab75054ea","unresolved":true,"context_lines":[{"line_number":44,"context_line":"\tenvm_clear_page_data();"},{"line_number":45,"context_line":"\tenvm_set_page_data(payload_addr);"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"\tuint_fast8_t ret_val \u003d envm_program_page(sector, page);"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"\treturn ret_val;"},{"line_number":50,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":6,"id":"d46264be_0c974aed","line":47,"range":{"start_line":47,"start_character":14,"end_line":47,"end_character":22},"updated":"2025-09-24 17:43:04.000000000","message":"The returned status may indicate a failure important to user. E.g. verify error.\nSo it should be properly propagated to OpenOCD.","commit_id":"64dfa543733736414ec410372113807436e0586b"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"22471fdcf6fd761a39963df9bda7867ab75054ea","unresolved":true,"context_lines":[{"line_number":65,"context_line":"\tuint32_t offset \u003d 0;"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"\tfor (; i \u003c range; i++) {"},{"line_number":68,"context_line":"\t\t_write_page(sector, i, (write_addr + offset));"},{"line_number":69,"context_line":"\t\toffset +\u003d PAGE_SIZE_U32;"},{"line_number":70,"context_line":"\t\tpages_written++;"},{"line_number":71,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":6,"id":"cfabe4c3_8589f689","line":68,"updated":"2025-09-24 17:43:04.000000000","message":"And what you do with the returned status? Simply ignore. Errors never happen anyway?!","commit_id":"64dfa543733736414ec410372113807436e0586b"}],"src/flash/nor/pic64gx.c":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"22471fdcf6fd761a39963df9bda7867ab75054ea","unresolved":true,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"#define LOADER_BASE_ADDRESS 0x8000000"},{"line_number":17,"context_line":"#define PAYLOAD_SIZE_ADDR 0x8020000"},{"line_number":18,"context_line":"#define PAYLOAD_WORKING_AREA 0x8021000"},{"line_number":19,"context_line":"#define ENVM_BASE_ADDR  0x20220000U"},{"line_number":20,"context_line":"#define ENVM_SIZE  0x20000U"}],"source_content_type":"text/x-csrc","patch_set":12,"id":"9b952b17_30716640","line":17,"range":{"start_line":17,"start_character":8,"end_line":17,"end_character":25},"updated":"2025-09-24 17:43:04.000000000","message":"No more used","commit_id":"185e17fe34022497952d644030ef326f7c928415"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"22471fdcf6fd761a39963df9bda7867ab75054ea","unresolved":true,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"static int pic64gx_flash_probe(struct flash_bank *bank)"},{"line_number":38,"context_line":"{"},{"line_number":39,"context_line":"\tstruct pic64gx_flash_bank *pic64gx_flash_bank \u003d bank-\u003edriver_priv;"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"\tint retval \u003d ERROR_OK;"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"\tif (!pic64gx_flash_bank-\u003eprobed)"},{"line_number":44,"context_line":"\t\tretval \u003d pic64gx_flash_auto_probe(bank);"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"\treturn retval;"},{"line_number":47,"context_line":"}"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"static int pic64gx_flash_auto_probe(struct flash_bank *bank)"}],"source_content_type":"text/x-csrc","patch_set":12,"id":"6558bc1a_b39269a0","line":46,"range":{"start_line":39,"start_character":1,"end_line":46,"end_character":15},"updated":"2025-09-24 17:43:04.000000000","message":"There is nothing to probe, so both `pic64gx_flash_probe()` and `pic64gx_flash_auto_probe()` could simply return ERROR_OK and `probed` could be dropped from `pic64gx_flash_bank`.\n\nOtherwise if you want to keep place for some future probing code, `pic64gx_flash_probe()` should do the real and unconditional probing and if successful set `probed` flag. Copy `xxx_auto_probe()` from almost any flash driver, e.g. flash/nor/atsamv.c","commit_id":"185e17fe34022497952d644030ef326f7c928415"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"22471fdcf6fd761a39963df9bda7867ab75054ea","unresolved":true,"context_lines":[{"line_number":50,"context_line":"{"},{"line_number":51,"context_line":"\tstruct pic64gx_flash_bank *pic64gx_flash_bank \u003d bank-\u003edriver_priv;"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"\tif (bank-\u003etarget-\u003estate !\u003d TARGET_HALTED) {"},{"line_number":54,"context_line":"\t\tLOG_ERROR(\"Target not halted\");"},{"line_number":55,"context_line":"\t\treturn ERROR_TARGET_NOT_HALTED;"},{"line_number":56,"context_line":"\t}"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"\tif (!pic64gx_flash_bank-\u003eprobed)"},{"line_number":59,"context_line":"\t\tpic64gx_flash_bank-\u003eprobed \u003d true;"}],"source_content_type":"text/x-csrc","patch_set":12,"id":"7b0ce3f3_3d263468","line":56,"range":{"start_line":53,"start_character":1,"end_line":56,"end_character":2},"updated":"2025-09-24 17:43:04.000000000","message":"Should be in `pic64gx_flash_write()`, not here","commit_id":"185e17fe34022497952d644030ef326f7c928415"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"22471fdcf6fd761a39963df9bda7867ab75054ea","unresolved":true,"context_lines":[{"line_number":67,"context_line":"\t\t\t\t\t\t\t\t\tuint32_t first, uint32_t last)"},{"line_number":68,"context_line":"{"},{"line_number":69,"context_line":"\tLOG_INFO(\"Not implemented\");"},{"line_number":70,"context_line":"\treturn ERROR_OK;"},{"line_number":71,"context_line":"}"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"static int pic64gx_flash_write(struct flash_bank *bank,"}],"source_content_type":"text/x-csrc","patch_set":12,"id":"5a10382d_d82e0346","line":70,"updated":"2025-09-24 17:43:04.000000000","message":"The standard flash memory has besides reading two basic operation erase and write. Not excluding Microchip eNVM.\n\nOpenOCD follows that model and has commands `flash erase...` and `flash write...`.\n\nBut your target loader breaks that model squashing erase and write to one operation for unknown reason. Yes, it\u0027s possible to program an image even with \"induced\" erase. However an experienced OpenOCD user may gets confused. How e.g. `flash erase_check` could be used? How to write two images with boundary not aligned to flash sector boundary? It wouldn\u0027t work on such crippled flash model.\n\nIt\u0027s not strict requirement for accepting the flash driver. Anyway please consider to go more standard way and make accessible both erase and write.","commit_id":"185e17fe34022497952d644030ef326f7c928415"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"22471fdcf6fd761a39963df9bda7867ab75054ea","unresolved":true,"context_lines":[{"line_number":76,"context_line":"\tstruct target *target \u003d bank-\u003etarget;"},{"line_number":77,"context_line":"\tuint32_t retval;"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"\tLOG_INFO(\"offset value is %d\", offset);"},{"line_number":80,"context_line":"\tLOG_INFO(\"count value is %d\", count);"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"\tretval \u003d target_write_buffer(target, LOADER_BASE_ADDRESS,"},{"line_number":83,"context_line":"\t\t\t\tsizeof(pic64gx_algo), pic64gx_algo);"}],"source_content_type":"text/x-csrc","patch_set":12,"id":"96781be6_1ba5bfa9","line":80,"range":{"start_line":79,"start_character":1,"end_line":80,"end_character":38},"updated":"2025-09-24 17:43:04.000000000","message":"Too verbose.\nBTW What happens if offset is non zero? IMO the fragment gets written from the beginning of eNVM","commit_id":"185e17fe34022497952d644030ef326f7c928415"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"22471fdcf6fd761a39963df9bda7867ab75054ea","unresolved":true,"context_lines":[{"line_number":102,"context_line":"\tbuf_set_u64(reg_params[0].value, 0, xlen, address);"},{"line_number":103,"context_line":"\tbuf_set_u64(reg_params[1].value, 0, xlen, count);"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"\tuint64_t get_a0 \u003d buf_get_u64(reg_params[0].value, 0, xlen);"},{"line_number":106,"context_line":"\tuint64_t get_a1 \u003d buf_get_u64(reg_params[1].value, 0, xlen);"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"\tif (get_a0 !\u003d PAYLOAD_WORKING_AREA)"},{"line_number":109,"context_line":"\t\tLOG_ERROR(\"read back from a0, value does not match\");"},{"line_number":110,"context_line":"\telse"},{"line_number":111,"context_line":"\t\tLOG_DEBUG(\"read back from a0, value matches payload working area\");"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"\tif (get_a1 !\u003d count)"},{"line_number":114,"context_line":"\t\tLOG_ERROR(\"read back from a1, value does not match\");"},{"line_number":115,"context_line":"\telse"},{"line_number":116,"context_line":"\t\tLOG_DEBUG(\"read back from a1, value matches count\");"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":119,"context_line":"\t\tLOG_ERROR(\"could not write the size into memory\");"}],"source_content_type":"text/x-csrc","patch_set":12,"id":"a6258777_01e2b8bb","line":116,"range":{"start_line":105,"start_character":1,"end_line":116,"end_character":54},"updated":"2025-09-24 17:43:04.000000000","message":"What is this? (code from line 105) Test of `buf_set_u64()`/`buf_get_u64()` operation? How it is related to the flash driver?","commit_id":"185e17fe34022497952d644030ef326f7c928415"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"22471fdcf6fd761a39963df9bda7867ab75054ea","unresolved":true,"context_lines":[{"line_number":115,"context_line":"\telse"},{"line_number":116,"context_line":"\t\tLOG_DEBUG(\"read back from a1, value matches count\");"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":119,"context_line":"\t\tLOG_ERROR(\"could not write the size into memory\");"},{"line_number":120,"context_line":"\t\treturn ERROR_FLASH_OPERATION_FAILED;"},{"line_number":121,"context_line":"\t}"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"\tretval \u003d target_run_algorithm(target, 0, NULL, 2, reg_params,"},{"line_number":124,"context_line":"\t\t\t\tLOADER_BASE_ADDRESS, 0, 40000, NULL);"}],"source_content_type":"text/x-csrc","patch_set":12,"id":"d214a023_c9c36712","line":121,"range":{"start_line":118,"start_character":1,"end_line":121,"end_character":2},"updated":"2025-09-24 17:43:04.000000000","message":"And this?!! Leftover after removed\n`retval \u003d target_write_u32(target, PAYLOAD_SIZE_ADDR, count);`","commit_id":"185e17fe34022497952d644030ef326f7c928415"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"22471fdcf6fd761a39963df9bda7867ab75054ea","unresolved":true,"context_lines":[{"line_number":133,"context_line":""},{"line_number":134,"context_line":"FLASH_BANK_COMMAND_HANDLER(pic64gx_flash_bank_command)"},{"line_number":135,"context_line":"{"},{"line_number":136,"context_line":"\tstruct flash_bank *t_bank \u003d bank;"},{"line_number":137,"context_line":"\tstruct pic64gx_flash_bank *pic64gx_flash_bank;"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"\tif (CMD_ARGC \u003c 6)"}],"source_content_type":"text/x-csrc","patch_set":12,"id":"a9b324bf_8c05f2df","line":136,"updated":"2025-09-24 17:43:04.000000000","message":"Why not use `bank` directly? I see no point in creating the t_bank alias","commit_id":"185e17fe34022497952d644030ef326f7c928415"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"22471fdcf6fd761a39963df9bda7867ab75054ea","unresolved":true,"context_lines":[{"line_number":151,"context_line":"\tpic64gx_flash_bank-\u003efamily_name \u003d \"Microchip_eNVM\";"},{"line_number":152,"context_line":"\tpic64gx_flash_bank-\u003eenvm_size \u003d ENVM_SIZE;"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"\tt_bank-\u003ebank_number \u003d 0;"},{"line_number":155,"context_line":"\tt_bank-\u003ebase \u003d ENVM_BASE_ADDR;"},{"line_number":156,"context_line":"\tt_bank-\u003esize \u003d ENVM_SIZE;"},{"line_number":157,"context_line":"\tt_bank-\u003enum_sectors \u003d 4;"}],"source_content_type":"text/x-csrc","patch_set":12,"id":"66b53425_ba0eee90","line":154,"range":{"start_line":154,"start_character":1,"end_line":154,"end_character":25},"updated":"2025-09-24 17:43:04.000000000","message":"Don\u0027t touch `bank_number`, it\u0027s handled by flash infrastructure","commit_id":"185e17fe34022497952d644030ef326f7c928415"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"22471fdcf6fd761a39963df9bda7867ab75054ea","unresolved":true,"context_lines":[{"line_number":155,"context_line":"\tt_bank-\u003ebase \u003d ENVM_BASE_ADDR;"},{"line_number":156,"context_line":"\tt_bank-\u003esize \u003d ENVM_SIZE;"},{"line_number":157,"context_line":"\tt_bank-\u003enum_sectors \u003d 4;"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"\tt_bank-\u003esectors \u003d malloc(t_bank-\u003enum_sectors * sizeof(struct flash_sector));"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"\tt_bank-\u003esectors[0].offset \u003d 0;"}],"source_content_type":"text/x-csrc","patch_set":12,"id":"7b872b96_6657dafe","line":158,"updated":"2025-09-24 17:43:04.000000000","message":"Do\n`free(bank-\u003esectors)`\nbefore allocation to avoid memory leak in case of re-probing.","commit_id":"185e17fe34022497952d644030ef326f7c928415"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"22471fdcf6fd761a39963df9bda7867ab75054ea","unresolved":true,"context_lines":[{"line_number":156,"context_line":"\tt_bank-\u003esize \u003d ENVM_SIZE;"},{"line_number":157,"context_line":"\tt_bank-\u003enum_sectors \u003d 4;"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"\tt_bank-\u003esectors \u003d malloc(t_bank-\u003enum_sectors * sizeof(struct flash_sector));"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"\tt_bank-\u003esectors[0].offset \u003d 0;"},{"line_number":162,"context_line":"\tt_bank-\u003esectors[0].size \u003d 0x2000;"}],"source_content_type":"text/x-csrc","patch_set":12,"id":"f4c591b0_f582756a","line":159,"updated":"2025-09-24 17:43:04.000000000","message":"What if `malloc()` fails?","commit_id":"185e17fe34022497952d644030ef326f7c928415"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"22471fdcf6fd761a39963df9bda7867ab75054ea","unresolved":true,"context_lines":[{"line_number":178,"context_line":"\tt_bank-\u003esectors[3].is_erased \u003d -1;"},{"line_number":179,"context_line":"\tt_bank-\u003esectors[3].is_protected \u003d -1;"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"\tbank-\u003edriver_priv \u003d pic64gx_flash_bank;"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"\treturn ERROR_OK;"},{"line_number":184,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":12,"id":"424532ad_31288ad6","line":181,"range":{"start_line":181,"start_character":1,"end_line":181,"end_character":40},"updated":"2025-09-24 17:43:04.000000000","message":"Duplicate of line 143","commit_id":"185e17fe34022497952d644030ef326f7c928415"}]}