)]}'
{"src/flash/nor/rp2xxx.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"b58a3c5a53bc9d126cb982483dc1045aa888f93f","unresolved":true,"context_lines":[{"line_number":613,"context_line":""},{"line_number":614,"context_line":"static int rp2350_save_accessctrl(struct target *target, struct rp2xxx_flash_bank *priv)"},{"line_number":615,"context_line":"{"},{"line_number":616,"context_line":"\treturn target_read_memory(target, ACCESSCTRL_SAVE_BASE, 4, ACCESSCTRL_SAVE_SIZE / 4,"},{"line_number":617,"context_line":"\t\t\t\t\t\t\t  priv-\u003esaved_accessctrl);"},{"line_number":618,"context_line":"}"},{"line_number":619,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"515a0c9c_fec73fe5","line":616,"updated":"2025-06-20 16:43:23.000000000","message":"I\u0027m not used to these rp2350 accessctrl, but it looks strange that reading the target from ACCESSCTRL_SAVE_BASE to host memory triggers below\n`priv-\u003eaccessctrl_dirty \u003d true;`\nwhile writing it back with the bit `ACCESSCTRL_WRITE_PASSWORD` set triggers\n`priv-\u003eaccessctrl_dirty \u003d false;`.\n\nIntuitively I would have said that it\u0027s the opposite!\n\nI see you explain it in the commit message.\nRepeating it in a short comment here could be useful for readers of this code.","commit_id":"770a06534392ac66134581eff0fe6f683fdcf306"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"229e49ad33549f56dc214c1457926fc813d1c95c","unresolved":false,"context_lines":[{"line_number":613,"context_line":""},{"line_number":614,"context_line":"static int rp2350_save_accessctrl(struct target *target, struct rp2xxx_flash_bank *priv)"},{"line_number":615,"context_line":"{"},{"line_number":616,"context_line":"\treturn target_read_memory(target, ACCESSCTRL_SAVE_BASE, 4, ACCESSCTRL_SAVE_SIZE / 4,"},{"line_number":617,"context_line":"\t\t\t\t\t\t\t  priv-\u003esaved_accessctrl);"},{"line_number":618,"context_line":"}"},{"line_number":619,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"867c21ba_30949daa","line":616,"in_reply_to":"515a0c9c_fec73fe5","updated":"2025-06-22 11:34:03.000000000","message":"Hopefully the added comment explains the shortcut `dirty \u003d valid` enough","commit_id":"770a06534392ac66134581eff0fe6f683fdcf306"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"b58a3c5a53bc9d126cb982483dc1045aa888f93f","unresolved":true,"context_lines":[{"line_number":652,"context_line":"\t} else {"},{"line_number":653,"context_line":"\t\tint retval \u003d rp2350_save_accessctrl(target, priv);"},{"line_number":654,"context_line":"\t\tif (retval \u003d\u003d ERROR_OK)"},{"line_number":655,"context_line":"\t\t\tpriv-\u003eaccessctrl_dirty \u003d true;"},{"line_number":656,"context_line":"\t\t// Don\u0027t fail on save ACCESSCTRL error, not vital for ROM API call"},{"line_number":657,"context_line":""},{"line_number":658,"context_line":"\t\tLOG_DEBUG(\"Reset ACCESSCTRL permissions via CFGRESET\");"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"4e5ec914_2831ae06","line":655,"updated":"2025-06-20 16:43:23.000000000","message":"I think this can improve the readability, please give it a try:\n\nApart from the initialization of `priv-\u003eaccessctrl_dirty \u003d false;` at the beginning of this function, I think that this assignment to true could be moved inside `rp2350_save_accessctrl()`.\nThen below ...","commit_id":"770a06534392ac66134581eff0fe6f683fdcf306"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"229e49ad33549f56dc214c1457926fc813d1c95c","unresolved":false,"context_lines":[{"line_number":652,"context_line":"\t} else {"},{"line_number":653,"context_line":"\t\tint retval \u003d rp2350_save_accessctrl(target, priv);"},{"line_number":654,"context_line":"\t\tif (retval \u003d\u003d ERROR_OK)"},{"line_number":655,"context_line":"\t\t\tpriv-\u003eaccessctrl_dirty \u003d true;"},{"line_number":656,"context_line":"\t\t// Don\u0027t fail on save ACCESSCTRL error, not vital for ROM API call"},{"line_number":657,"context_line":""},{"line_number":658,"context_line":"\t\tLOG_DEBUG(\"Reset ACCESSCTRL permissions via CFGRESET\");"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"952caed8_52beeccb","line":655,"in_reply_to":"4e5ec914_2831ae06","updated":"2025-06-22 11:34:03.000000000","message":"IMO refactoring `accessctrl_dirty` handling into `rp2350_save_accessctrl()` and `rp2350_restore_accessctrl()` gains nothing as both functions are called just by one caller. In such case I prefer to have all `accessctrl_dirty` handling at the same level.\n\nThe fix was already tested and accepted at RPi branch, so I\u0027d better keep the code as is.","commit_id":"770a06534392ac66134581eff0fe6f683fdcf306"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"b58a3c5a53bc9d126cb982483dc1045aa888f93f","unresolved":true,"context_lines":[{"line_number":870,"context_line":"\tif (IS_RP2350(priv-\u003eid)) {"},{"line_number":871,"context_line":"\t\tint retval1 \u003d ERROR_OK;"},{"line_number":872,"context_line":"\t\tif (priv-\u003eaccessctrl_dirty) {"},{"line_number":873,"context_line":"\t\t\tretval1 \u003d rp2350_restore_accessctrl(target, priv);"},{"line_number":874,"context_line":"\t\t\tpriv-\u003eaccessctrl_dirty \u003d false;"},{"line_number":875,"context_line":"\t\t}"},{"line_number":876,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c563ebfc_b3655b3a","line":873,"updated":"2025-06-20 16:43:23.000000000","message":"... the test `(priv-\u003eaccessctrl_dirty)` and the assignment to false below can be moved in `rp2350_restore_accessctrl()`","commit_id":"770a06534392ac66134581eff0fe6f683fdcf306"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"229e49ad33549f56dc214c1457926fc813d1c95c","unresolved":false,"context_lines":[{"line_number":870,"context_line":"\tif (IS_RP2350(priv-\u003eid)) {"},{"line_number":871,"context_line":"\t\tint retval1 \u003d ERROR_OK;"},{"line_number":872,"context_line":"\t\tif (priv-\u003eaccessctrl_dirty) {"},{"line_number":873,"context_line":"\t\t\tretval1 \u003d rp2350_restore_accessctrl(target, priv);"},{"line_number":874,"context_line":"\t\t\tpriv-\u003eaccessctrl_dirty \u003d false;"},{"line_number":875,"context_line":"\t\t}"},{"line_number":876,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"5b21fdc4_ca492e4e","line":873,"in_reply_to":"c563ebfc_b3655b3a","updated":"2025-06-22 11:34:03.000000000","message":"Commented above","commit_id":"770a06534392ac66134581eff0fe6f683fdcf306"}]}