)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"b00f080e2c8afa58f3ab678a2ffcc2ac1a7989b3","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1e3026b0_4e9670f4","updated":"2024-08-15 07:43:47.000000000","message":"Graham, could you please sign the patch - actually it is rp2040 flash driver part of your\nhttps://github.com/raspberrypi/openocd-rp2350/commit/ba79cb8a6561c3f96062f3f553584689b77fe859\n\nJenkins insists on signing by you\n`checking patch ...\nERROR:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author \u0027graham sanderson \u003cgraham.sanderson@raspberrypi.com\u003e\u0027\n`\n\nThanks","commit_id":"9e9e8f136d0d04fbfd38a4daef8fb08a63a71a4a"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"240df4f4f186b01a609d83d404bdf5e37f194c39","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"05c540e6_3a51c00b","in_reply_to":"1e3026b0_4e9670f4","updated":"2025-01-29 09:53:54.000000000","message":"Done","commit_id":"9e9e8f136d0d04fbfd38a4daef8fb08a63a71a4a"},{"author":{"_account_id":1001632,"name":"Graham Sanderson","email":"graham.sanderson@gmail.com","username":"graham.sanderson"},"change_message_id":"3ca3b1ac22b54250e2eec354060322b1df72e56d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"a3c0acc2_e11b7dd6","updated":"2025-02-17 18:24:42.000000000","message":"- \"LOG_WARNING(\"GOT FLASH ERASE AT %08x\\n\", (int)priv-\u003ejump_flash_range_erase);\" is probably left over debugging, though i don\u0027t think this a reason not to commit\n- I\u0027m not sure why various configuration values were changed to hardcoded values?","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"95751e2cee556d1e3b337ca4724cd8f4d552f2fe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"028ac064_2fe08385","updated":"2025-03-16 18:41:21.000000000","message":"Antonio, thanks a lot for taking time on this long patch.\nUnfortunately you probably missed that I intentionally submitted Raspberry Pi guys authored code as close as possible to the original patches.\nTherefore I made just the style changes forced by jenkins check and *all other changes* submitted as the follow up patches. I want to be clear who authored what part of code. It would be *total waste of time and effort* for both you and me to start over and make updates you requested and propagate them though 28 or so following patches. I doubt that Graham or Luke are going to work on this.\nAlso there are 3 original Rpi patches and 3 following RPi fixes. I simply did not care about the code between them, I started at the level of\n8446: flash/nor/rp2040: allow flash size override from cfg | https://review.openocd.org/c/openocd/+/8446\n\nI marked DONE your comments already addressed in the follow ups.\n\nI\u0027ll address the rest in new patches.","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"a107ffe351883203e94c0744cf9298f310bc8402","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9718417a_87fbb41a","in_reply_to":"028ac064_2fe08385","updated":"2025-03-16 21:18:58.000000000","message":"thanks for the explanation.\nOk for me!","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"cb1d9e35bdb76bd18ab001e7c41ca8234f5a8e03","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"2e9e8229_42996f0c","in_reply_to":"a3c0acc2_e11b7dd6","updated":"2025-02-17 19:55:58.000000000","message":"Thanks for reviewing, Graham.\n\n\u003e - \"LOG_WARNING(\"GOT FLASH ERASE AT %08x\\n\", (int)priv-\u003ejump_flash_range_erase);\" is probably left over debugging, though i don\u0027t think this a reason not to commit\n\nThis message gets removed in the following commit\n8441: flash/nor/rp2040: Add RISC-V ROM algorithm batch call support | https://review.openocd.org/c/openocd/+/8441\n\nThis patch is yours\nhttps://github.com/raspberrypi/openocd-rp2350/commit/ba79cb8a6561c3f96062f3f553584689b77fe859\nwith minimal changes to pass jenkins check.\n\n\u003e - I\u0027m not sure why various configuration values were changed to hardcoded values?\n\nCan you give more details? I didn\u0027t make any substantial changes. Don\u0027t you miss something from following patches?","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"}],"src/flash/nor/rp2040.c":[{"author":{"_account_id":1002337,"name":"Luke Wren","username":"Wren6991"},"change_message_id":"3bc958e0275f4147af4fd0629f9f4d9a27cecd0f","unresolved":true,"context_lines":[{"line_number":448,"context_line":"\tuint32_t args[4] \u003d {"},{"line_number":449,"context_line":"\t\tbank-\u003esectors[first].offset, /* addr */"},{"line_number":450,"context_line":"\t\tbank-\u003esectors[last].offset + bank-\u003esectors[last].size - bank-\u003esectors[first].offset, /* count */"},{"line_number":451,"context_line":"\t\t65536, /* block_size */"},{"line_number":452,"context_line":"\t\t0xd8   /* block_cmd */"},{"line_number":453,"context_line":"\t};"},{"line_number":454,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"836a6c23_eae4e055","line":451,"updated":"2025-03-14 10:21:02.000000000","message":"Maybe these are the constants Graham was referring to. The values used here are (almost) universal. I imagine the constants were used to work around early trouble with not being able to probe the flash device in some states. We could revert these lines.","commit_id":"4f332cd23aea3d0fef0652058e89cafcc2940dc8"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"bf699859adc7c1bb6c8a11b5f419a9475152f719","unresolved":false,"context_lines":[{"line_number":448,"context_line":"\tuint32_t args[4] \u003d {"},{"line_number":449,"context_line":"\t\tbank-\u003esectors[first].offset, /* addr */"},{"line_number":450,"context_line":"\t\tbank-\u003esectors[last].offset + bank-\u003esectors[last].size - bank-\u003esectors[first].offset, /* count */"},{"line_number":451,"context_line":"\t\t65536, /* block_size */"},{"line_number":452,"context_line":"\t\t0xd8   /* block_cmd */"},{"line_number":453,"context_line":"\t};"},{"line_number":454,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"eb01fba2_7bb89bff","line":451,"in_reply_to":"836a6c23_eae4e055","updated":"2025-03-14 12:12:44.000000000","message":"In the next patch a modified comment\nhttps://review.openocd.org/c/openocd/+/8441/3/src/flash/nor/rp2040.c#835\nexplains why the block erase is hardcoded.\nWell it\u0027s not a good practice to combine flash detection with hardcoded erase commands, but I believe the main reason is flash erase speed.\nOpenOCD currently has no mechanism to select from 2 or 3 erase block sizes and spi.c flash definitions tend to erase by sectors (the slowest erase).\nI think we can leave it as it until OpenOCD implements multiple erase sizes in the flash infrastructure.","commit_id":"4f332cd23aea3d0fef0652058e89cafcc2940dc8"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"68c6993df5f9611f46a60210d4cd2aafd2dac33b","unresolved":true,"context_lines":[{"line_number":100,"context_line":"{"},{"line_number":101,"context_line":"\tuint32_t magic, magic_addr;"},{"line_number":102,"context_line":"\tbool found_rp2040_magic, found_rp2350_magic;"},{"line_number":103,"context_line":"\tmagic_addr \u003d BOOTROM_MAGIC_ADDR;"},{"line_number":104,"context_line":"\tint err \u003d target_read_u32(target, BOOTROM_MAGIC_ADDR, \u0026magic);"},{"line_number":105,"context_line":"\tif (err !\u003d ERROR_OK)"},{"line_number":106,"context_line":"\t\treturn err;"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"e621bef2_489f9c2c","line":103,"updated":"2025-03-16 10:38:20.000000000","message":"I would have not complain for this alone, but since there are other comments below, please also fix this\n`uint32_t magic_addr \u003d BOOTROM_MAGIC_ADDR;`","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"95751e2cee556d1e3b337ca4724cd8f4d552f2fe","unresolved":false,"context_lines":[{"line_number":100,"context_line":"{"},{"line_number":101,"context_line":"\tuint32_t magic, magic_addr;"},{"line_number":102,"context_line":"\tbool found_rp2040_magic, found_rp2350_magic;"},{"line_number":103,"context_line":"\tmagic_addr \u003d BOOTROM_MAGIC_ADDR;"},{"line_number":104,"context_line":"\tint err \u003d target_read_u32(target, BOOTROM_MAGIC_ADDR, \u0026magic);"},{"line_number":105,"context_line":"\tif (err !\u003d ERROR_OK)"},{"line_number":106,"context_line":"\t\treturn err;"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"d7e26cb9_06e2c69d","line":103,"in_reply_to":"e621bef2_489f9c2c","updated":"2025-03-16 18:41:21.000000000","message":"Done","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"68c6993df5f9611f46a60210d4cd2aafd2dac33b","unresolved":true,"context_lines":[{"line_number":105,"context_line":"\tif (err !\u003d ERROR_OK)"},{"line_number":106,"context_line":"\t\treturn err;"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"\tmagic \u0026\u003d 0xffffff; /* ignore bootrom version */"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"\tfound_rp2040_magic \u003d magic \u003d\u003d BOOTROM_RP2040_MAGIC;"},{"line_number":111,"context_line":"\tfound_rp2350_magic \u003d magic \u003d\u003d BOOTROM_RP2350_MAGIC;"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"bb723158_3adcfc17","line":108,"updated":"2025-03-16 10:38:20.000000000","message":"Use a macro for this value, e.g. `BOOTROM_MAGIC_MASK`","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"b4c531179d5c2cd66721a8194a2dfdba7f6a8321","unresolved":false,"context_lines":[{"line_number":105,"context_line":"\tif (err !\u003d ERROR_OK)"},{"line_number":106,"context_line":"\t\treturn err;"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"\tmagic \u0026\u003d 0xffffff; /* ignore bootrom version */"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"\tfound_rp2040_magic \u003d magic \u003d\u003d BOOTROM_RP2040_MAGIC;"},{"line_number":111,"context_line":"\tfound_rp2350_magic \u003d magic \u003d\u003d BOOTROM_RP2350_MAGIC;"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"4348bd92_627627a5","line":108,"in_reply_to":"bb723158_3adcfc17","updated":"2025-03-23 09:45:28.000000000","message":"8810: flash/nor/rp2xxx: define macro BOOTROM_MAGIC_MASK | https://review.openocd.org/c/openocd/+/8810","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"68c6993df5f9611f46a60210d4cd2aafd2dac33b","unresolved":true,"context_lines":[{"line_number":133,"context_line":"\t\t\t\terr \u003d target_read_u16(target, table_entry + 4, \u0026flags);"},{"line_number":134,"context_line":"\t\t\t\tif (err !\u003d ERROR_OK)"},{"line_number":135,"context_line":"\t\t\t\t\treturn err;"},{"line_number":136,"context_line":"\t\t\t\t//"},{"line_number":137,"context_line":"\t\t\t\tif (flags \u0026 RT_ARM_FUNC) {"},{"line_number":138,"context_line":"\t\t\t\t\t/* 16 bit symbol */"},{"line_number":139,"context_line":"\t\t\t\t\treturn target_read_u16(target, table_entry + 2, symbol);"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"d544c017_810163fc","line":136,"updated":"2025-03-16 10:38:20.000000000","message":"what\u0027s this line for?","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"95751e2cee556d1e3b337ca4724cd8f4d552f2fe","unresolved":false,"context_lines":[{"line_number":133,"context_line":"\t\t\t\terr \u003d target_read_u16(target, table_entry + 4, \u0026flags);"},{"line_number":134,"context_line":"\t\t\t\tif (err !\u003d ERROR_OK)"},{"line_number":135,"context_line":"\t\t\t\t\treturn err;"},{"line_number":136,"context_line":"\t\t\t\t//"},{"line_number":137,"context_line":"\t\t\t\tif (flags \u0026 RT_ARM_FUNC) {"},{"line_number":138,"context_line":"\t\t\t\t\t/* 16 bit symbol */"},{"line_number":139,"context_line":"\t\t\t\t\treturn target_read_u16(target, table_entry + 2, symbol);"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"6d4f9236_4dce9206","line":136,"in_reply_to":"d544c017_810163fc","updated":"2025-03-16 18:41:21.000000000","message":"Done","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"68c6993df5f9611f46a60210d4cd2aafd2dac33b","unresolved":true,"context_lines":[{"line_number":144,"context_line":"\t\t\t}"},{"line_number":145,"context_line":"\t\t}"},{"line_number":146,"context_line":"\t\ttable_entry +\u003d found_rp2350_magic ? 6 : 4;"},{"line_number":147,"context_line":"\t} while (entry_tag);"},{"line_number":148,"context_line":"\treturn ERROR_FAIL;"},{"line_number":149,"context_line":"}"},{"line_number":150,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":3,"id":"b60aedda_ac884d61","line":147,"updated":"2025-03-16 10:38:20.000000000","message":"Just personal preference for readability and reduced indentation, not a strict requirement.\n```\n\tdo {\n\t\terr \u003d target_read_u16(target, table_entry, \u0026entry_tag);\n\t\tif (err !\u003d ERROR_OK)\n\t\t\treturn err;\n\t\tif (entry_tag \u003d\u003d tag)\n\t\t\tbreak;\n\t\tif (entry_tag !\u003d 0)\n\t\t\treturn ERROR_FAIL;\n\t\ttable_entry +\u003d found_rp2350_magic ? 6 : 4;\n\t} while (1);\n\n\tif (found_rp2350_magic) {\n\t\t... same as above ...\n\t\treturn target_read_u16(target, table_entry + 2, symbol);\n\t}\n\n\t/* 16 bit symbol is next */\n\treturn target_read_u16(target, table_entry + 2, symbol);\n}","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"68c6993df5f9611f46a60210d4cd2aafd2dac33b","unresolved":true,"context_lines":[{"line_number":162,"context_line":"\ttarget_addr_t stacktop \u003d priv-\u003estack-\u003eaddress + priv-\u003estack-\u003esize;"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"\tLOG_DEBUG(\"Calling ROM func @0x%\" PRIx16 \" with %d arguments\", func_offset, n_args);"},{"line_number":165,"context_line":"\tLOG_DEBUG(\"Calling on core \\\"%s\\\"\", target-\u003ecmd_name);"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"\tstruct reg_param args[ARRAY_SIZE(regnames) + 12];"},{"line_number":168,"context_line":"\tstruct armv7m_algorithm alg_info;"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"d618fb69_38de370d","line":165,"updated":"2025-03-16 10:38:20.000000000","message":"Never use directly `target-\u003ecmd_name` but use the helper `target_name(target)`.\nI was sure I have already dropped all these uses, but I see some again...\nWhat\u0027s wrong with using `LOG_TARGET_DEBUG(target, \"..\")`?","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"95751e2cee556d1e3b337ca4724cd8f4d552f2fe","unresolved":false,"context_lines":[{"line_number":162,"context_line":"\ttarget_addr_t stacktop \u003d priv-\u003estack-\u003eaddress + priv-\u003estack-\u003esize;"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"\tLOG_DEBUG(\"Calling ROM func @0x%\" PRIx16 \" with %d arguments\", func_offset, n_args);"},{"line_number":165,"context_line":"\tLOG_DEBUG(\"Calling on core \\\"%s\\\"\", target-\u003ecmd_name);"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"\tstruct reg_param args[ARRAY_SIZE(regnames) + 12];"},{"line_number":168,"context_line":"\tstruct armv7m_algorithm alg_info;"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"7eea012b_1c3ac3e4","line":165,"in_reply_to":"d618fb69_38de370d","updated":"2025-03-16 18:41:21.000000000","message":"Done","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"68c6993df5f9611f46a60210d4cd2aafd2dac33b","unresolved":true,"context_lines":[{"line_number":212,"context_line":"\t\t0, NULL,          /* No memory arguments */"},{"line_number":213,"context_line":"\t\tn_args + extra_args, args, /* User arguments + r7 + SPs */"},{"line_number":214,"context_line":"\t\tpriv-\u003ejump_debug_trampoline, priv-\u003ejump_debug_trampoline_end,"},{"line_number":215,"context_line":"\t\t3000, /* 3s timeout */"},{"line_number":216,"context_line":"\t\t\u0026alg_info"},{"line_number":217,"context_line":"\t);"},{"line_number":218,"context_line":"\tfor (unsigned int i \u003d 0; i \u003c n_args + extra_args; ++i)"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"6c78b413_ba97a79d","line":215,"updated":"2025-03-16 10:38:20.000000000","message":"In general, all these timeouts would be better written as macros that embed in the name the time unit, e.g.\n`#define TIMEOUT_ROM_FUNC_MS 3000`","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"b4c531179d5c2cd66721a8194a2dfdba7f6a8321","unresolved":false,"context_lines":[{"line_number":212,"context_line":"\t\t0, NULL,          /* No memory arguments */"},{"line_number":213,"context_line":"\t\tn_args + extra_args, args, /* User arguments + r7 + SPs */"},{"line_number":214,"context_line":"\t\tpriv-\u003ejump_debug_trampoline, priv-\u003ejump_debug_trampoline_end,"},{"line_number":215,"context_line":"\t\t3000, /* 3s timeout */"},{"line_number":216,"context_line":"\t\t\u0026alg_info"},{"line_number":217,"context_line":"\t);"},{"line_number":218,"context_line":"\tfor (unsigned int i \u003d 0; i \u003c n_args + extra_args; ++i)"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"105a30c2_56d0ad5a","line":215,"in_reply_to":"6c78b413_ba97a79d","updated":"2025-03-23 09:45:28.000000000","message":"I decided to leave this as is: there is just one use of 3s timeout and one use of 1s timeout, always with decent comment. I personally prefer this compact solution over splitted definitions from the code.","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"68c6993df5f9611f46a60210d4cd2aafd2dac33b","unresolved":true,"context_lines":[{"line_number":268,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":269,"context_line":"\t}"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"\tLOG_DEBUG(\"Calling rcp_init core \\\"%s\\\" code at 0x%\" PRIx16 \"\\n\", target-\u003ecmd_name, (uint32_t)priv-\u003estack-\u003eaddress);"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"\t/* Actually call the function */"},{"line_number":274,"context_line":"\talg_info.common_magic \u003d ARMV7M_COMMON_MAGIC;"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"31f01ac7_6dec00ba","line":271,"updated":"2025-03-16 10:38:20.000000000","message":"no `target-\u003ecmd_name`, but `target_name(target)` or better `LOG_TARGET_DEBUG(target, \"\")`\n\nWhat\u0027s happening here?\n`struct working_area::address` is of type `target_addr_t`.\nYou cast it to `uint32_t` and then you use `PRIx16` in the format string?","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"b4c531179d5c2cd66721a8194a2dfdba7f6a8321","unresolved":false,"context_lines":[{"line_number":268,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":269,"context_line":"\t}"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"\tLOG_DEBUG(\"Calling rcp_init core \\\"%s\\\" code at 0x%\" PRIx16 \"\\n\", target-\u003ecmd_name, (uint32_t)priv-\u003estack-\u003eaddress);"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"\t/* Actually call the function */"},{"line_number":274,"context_line":"\talg_info.common_magic \u003d ARMV7M_COMMON_MAGIC;"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"bc2ce0f2_398fd763","line":271,"in_reply_to":"31f01ac7_6dec00ba","updated":"2025-03-23 09:45:28.000000000","message":"8809: flash/nor/rp2xxx: fix LOG_xxx messages | https://review.openocd.org/c/openocd/+/8809","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"68c6993df5f9611f46a60210d4cd2aafd2dac33b","unresolved":true,"context_lines":[{"line_number":278,"context_line":"\t\t\t0, NULL,          /* No register arguments */"},{"line_number":279,"context_line":"\t\t\tpriv-\u003estack-\u003eaddress,"},{"line_number":280,"context_line":"\t\t\tpriv-\u003estack-\u003eaddress + rcp_init_code_bkpt_offset,"},{"line_number":281,"context_line":"\t\t\t1000, /* 1s timeout */"},{"line_number":282,"context_line":"\t\t\t\u0026alg_info"},{"line_number":283,"context_line":"\t);"},{"line_number":284,"context_line":"\tif (err !\u003d ERROR_OK) {"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"c7f46958_a8f10359","line":281,"updated":"2025-03-16 10:38:20.000000000","message":"another timeout that should be a macro","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"b4c531179d5c2cd66721a8194a2dfdba7f6a8321","unresolved":false,"context_lines":[{"line_number":278,"context_line":"\t\t\t0, NULL,          /* No register arguments */"},{"line_number":279,"context_line":"\t\t\tpriv-\u003estack-\u003eaddress,"},{"line_number":280,"context_line":"\t\t\tpriv-\u003estack-\u003eaddress + rcp_init_code_bkpt_offset,"},{"line_number":281,"context_line":"\t\t\t1000, /* 1s timeout */"},{"line_number":282,"context_line":"\t\t\t\u0026alg_info"},{"line_number":283,"context_line":"\t);"},{"line_number":284,"context_line":"\tif (err !\u003d ERROR_OK) {"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"4424b4ca_a747f451","line":281,"in_reply_to":"c7f46958_a8f10359","updated":"2025-03-23 09:45:28.000000000","message":"Same as above https://review.openocd.org/c/openocd/+/8440/comment/6c78b413_ba97a79d/","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"68c6993df5f9611f46a60210d4cd2aafd2dac33b","unresolved":true,"context_lines":[{"line_number":322,"context_line":"\t// Flash algorithms must run in Secure state"},{"line_number":323,"context_line":"\tuint32_t dscsr;"},{"line_number":324,"context_line":"\t(void)target_read_u32(target, DCB_DSCSR, \u0026dscsr);"},{"line_number":325,"context_line":"\tLOG_DEBUG(\"DSCSR:  %08x\\n\", dscsr);"},{"line_number":326,"context_line":"\tif (!(dscsr \u0026 DSCSR_CDS)) {"},{"line_number":327,"context_line":"\t\tLOG_DEBUG(\"Setting Current Domain Secure in DSCSR\\n\");"},{"line_number":328,"context_line":"\t\t(void)target_write_u32(target, DCB_DSCSR, (dscsr \u0026 ~DSCSR_CDSKEY) | DSCSR_CDS);"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"255cc207_ec032fc6","line":325,"updated":"2025-03-16 10:38:20.000000000","message":"`uint32_t` deserves `PRIx32`\nNo newline at the end of LOG_XXX messages. Same issue below","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"b4c531179d5c2cd66721a8194a2dfdba7f6a8321","unresolved":false,"context_lines":[{"line_number":322,"context_line":"\t// Flash algorithms must run in Secure state"},{"line_number":323,"context_line":"\tuint32_t dscsr;"},{"line_number":324,"context_line":"\t(void)target_read_u32(target, DCB_DSCSR, \u0026dscsr);"},{"line_number":325,"context_line":"\tLOG_DEBUG(\"DSCSR:  %08x\\n\", dscsr);"},{"line_number":326,"context_line":"\tif (!(dscsr \u0026 DSCSR_CDS)) {"},{"line_number":327,"context_line":"\t\tLOG_DEBUG(\"Setting Current Domain Secure in DSCSR\\n\");"},{"line_number":328,"context_line":"\t\t(void)target_write_u32(target, DCB_DSCSR, (dscsr \u0026 ~DSCSR_CDSKEY) | DSCSR_CDS);"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"561679e3_83cd226c","line":325,"in_reply_to":"255cc207_ec032fc6","updated":"2025-03-23 09:45:28.000000000","message":"8809: flash/nor/rp2xxx: fix LOG_xxx messages | https://review.openocd.org/c/openocd/+/8809","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"68c6993df5f9611f46a60210d4cd2aafd2dac33b","unresolved":true,"context_lines":[{"line_number":381,"context_line":"\t\treturn err;"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"\t// Allocate as much memory as possible, rounded down to a whole number of flash pages"},{"line_number":384,"context_line":"\tconst unsigned int chunk_size \u003d target_get_working_area_avail(target) \u0026 ~0xffu;"},{"line_number":385,"context_line":"\tif (chunk_size \u003d\u003d 0 || target_alloc_working_area(target, chunk_size, \u0026bounce) !\u003d ERROR_OK) {"},{"line_number":386,"context_line":"\t\tLOG_ERROR(\"Could not allocate bounce buffer for flash programming. Can\u0027t continue\");"},{"line_number":387,"context_line":"\t\treturn ERROR_TARGET_RESOURCE_NOT_AVAILABLE;"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"97c52877_18244581","line":384,"updated":"2025-03-16 10:38:20.000000000","message":"`chunk_size \u003d ALIGN_DOWN(target_get_working_area_avail(target), 0x100);`\nbetter using a macro for the value `0x100` or `256`. Is it a page size?\n\nActually much readable as:\n```\nunsigned int chunk_size \u003d target_get_working_area_avail(target);\nif (chunk_size \u003c PAGE_SIZE) {\n  LOG_ERROR(\"Not enough memory for ..\");\n  return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;\n}\nchunk_size \u003d ALIGN_DOWN(chunk_size, PAGE_SIZE);\nerr \u003d target_alloc_working_area(target, chunk_size, \u0026bounce);\nif (err !\u003d ERROR_OK) {\n```","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"95751e2cee556d1e3b337ca4724cd8f4d552f2fe","unresolved":false,"context_lines":[{"line_number":381,"context_line":"\t\treturn err;"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"\t// Allocate as much memory as possible, rounded down to a whole number of flash pages"},{"line_number":384,"context_line":"\tconst unsigned int chunk_size \u003d target_get_working_area_avail(target) \u0026 ~0xffu;"},{"line_number":385,"context_line":"\tif (chunk_size \u003d\u003d 0 || target_alloc_working_area(target, chunk_size, \u0026bounce) !\u003d ERROR_OK) {"},{"line_number":386,"context_line":"\t\tLOG_ERROR(\"Could not allocate bounce buffer for flash programming. Can\u0027t continue\");"},{"line_number":387,"context_line":"\t\treturn ERROR_TARGET_RESOURCE_NOT_AVAILABLE;"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"697460b9_34bdaa6a","line":384,"in_reply_to":"97c52877_18244581","updated":"2025-03-16 18:41:21.000000000","message":"Done","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"68c6993df5f9611f46a60210d4cd2aafd2dac33b","unresolved":true,"context_lines":[{"line_number":448,"context_line":"\tuint32_t args[4] \u003d {"},{"line_number":449,"context_line":"\t\tbank-\u003esectors[first].offset, /* addr */"},{"line_number":450,"context_line":"\t\tbank-\u003esectors[last].offset + bank-\u003esectors[last].size - bank-\u003esectors[first].offset, /* count */"},{"line_number":451,"context_line":"\t\t65536, /* block_size */"},{"line_number":452,"context_line":"\t\t0xd8   /* block_cmd */"},{"line_number":453,"context_line":"\t};"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"\t/*"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"6e104537_3367a6e0","line":452,"range":{"start_line":451,"start_character":2,"end_line":452,"end_character":24},"updated":"2025-03-16 10:38:20.000000000","message":"these and probably other in the code looks the result of dropping `rp2040_default_spi_device`\nShould macros be used for these values?","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"b4c531179d5c2cd66721a8194a2dfdba7f6a8321","unresolved":false,"context_lines":[{"line_number":448,"context_line":"\tuint32_t args[4] \u003d {"},{"line_number":449,"context_line":"\t\tbank-\u003esectors[first].offset, /* addr */"},{"line_number":450,"context_line":"\t\tbank-\u003esectors[last].offset + bank-\u003esectors[last].size - bank-\u003esectors[first].offset, /* count */"},{"line_number":451,"context_line":"\t\t65536, /* block_size */"},{"line_number":452,"context_line":"\t\t0xd8   /* block_cmd */"},{"line_number":453,"context_line":"\t};"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"\t/*"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"721e7c6a_53d6be2d","line":452,"range":{"start_line":451,"start_character":2,"end_line":452,"end_character":24},"in_reply_to":"6e104537_3367a6e0","updated":"2025-03-23 09:45:28.000000000","message":"We may consider adding the block and half-block erase commands with the usual erase sizes to spi.h, I\u0027m not fan of defining this privately in any flash driver. Also `struct flash_device flash_devices[]` table in spi.c has lot of the 0xd8 without a macro...","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"68c6993df5f9611f46a60210d4cd2aafd2dac33b","unresolved":true,"context_lines":[{"line_number":506,"context_line":"\t\tLOG_ERROR(\"Function FUNC_FLASH_RANGE_ERASE not found in RP2040 ROM.\");"},{"line_number":507,"context_line":"\t\treturn err;"},{"line_number":508,"context_line":"\t}"},{"line_number":509,"context_line":"\tLOG_WARNING(\"GOT FLASH ERASE AT %08x\\n\", (int)priv-\u003ejump_flash_range_erase);"},{"line_number":510,"context_line":""},{"line_number":511,"context_line":"\terr \u003d rp2040_lookup_symbol(target, FUNC_FLASH_RANGE_PROGRAM, \u0026priv-\u003ejump_flash_range_program);"},{"line_number":512,"context_line":"\tif (err !\u003d ERROR_OK) {"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"6a1010f5_086212cf","line":509,"updated":"2025-03-16 10:38:20.000000000","message":"`jump_flash_range_erase` is of type uint16_t\nDrop the cast and use `PRIx16`","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"95751e2cee556d1e3b337ca4724cd8f4d552f2fe","unresolved":false,"context_lines":[{"line_number":506,"context_line":"\t\tLOG_ERROR(\"Function FUNC_FLASH_RANGE_ERASE not found in RP2040 ROM.\");"},{"line_number":507,"context_line":"\t\treturn err;"},{"line_number":508,"context_line":"\t}"},{"line_number":509,"context_line":"\tLOG_WARNING(\"GOT FLASH ERASE AT %08x\\n\", (int)priv-\u003ejump_flash_range_erase);"},{"line_number":510,"context_line":""},{"line_number":511,"context_line":"\terr \u003d rp2040_lookup_symbol(target, FUNC_FLASH_RANGE_PROGRAM, \u0026priv-\u003ejump_flash_range_program);"},{"line_number":512,"context_line":"\tif (err !\u003d ERROR_OK) {"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"08cd42fa_01535896","line":509,"in_reply_to":"6a1010f5_086212cf","updated":"2025-03-16 18:41:21.000000000","message":"Done","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"68c6993df5f9611f46a60210d4cd2aafd2dac33b","unresolved":true,"context_lines":[{"line_number":529,"context_line":"\terr \u003d rp2040_lookup_symbol(target, FUNC_BOOTROM_STATE_RESET, \u0026priv-\u003ejump_bootrom_reset_state);"},{"line_number":530,"context_line":"\tif (err !\u003d ERROR_OK) {"},{"line_number":531,"context_line":"\t\tpriv-\u003ejump_bootrom_reset_state \u003d 0;"},{"line_number":532,"context_line":"//        LOG_ERROR(\"Function FUNC_BOOTROM_STATE_RESET not found in RP2040 ROM.\");"},{"line_number":533,"context_line":"//        return err;"},{"line_number":534,"context_line":"\t}"},{"line_number":535,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":3,"id":"73a15611_81bf0730","line":532,"updated":"2025-03-16 10:38:20.000000000","message":"useless code? To be dropped?","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"95751e2cee556d1e3b337ca4724cd8f4d552f2fe","unresolved":false,"context_lines":[{"line_number":529,"context_line":"\terr \u003d rp2040_lookup_symbol(target, FUNC_BOOTROM_STATE_RESET, \u0026priv-\u003ejump_bootrom_reset_state);"},{"line_number":530,"context_line":"\tif (err !\u003d ERROR_OK) {"},{"line_number":531,"context_line":"\t\tpriv-\u003ejump_bootrom_reset_state \u003d 0;"},{"line_number":532,"context_line":"//        LOG_ERROR(\"Function FUNC_BOOTROM_STATE_RESET not found in RP2040 ROM.\");"},{"line_number":533,"context_line":"//        return err;"},{"line_number":534,"context_line":"\t}"},{"line_number":535,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":3,"id":"fa8414a6_c5c37239","line":532,"in_reply_to":"73a15611_81bf0730","updated":"2025-03-16 18:41:21.000000000","message":"Done","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"68c6993df5f9611f46a60210d4cd2aafd2dac33b","unresolved":true,"context_lines":[{"line_number":540,"context_line":"\t// Max size -- up to two devices (two chip selects) in adjacent 24-bit address windows"},{"line_number":541,"context_line":"\tbank-\u003esize \u003d 32 * 1024 * 1024;"},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"\tbank-\u003enum_sectors \u003d bank-\u003esize / 4096;"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"\tLOG_INFO(\"RP2040 Flash Probe: %d bytes @\" TARGET_ADDR_FMT \", in %d sectors\\n\","},{"line_number":546,"context_line":"\t\tbank-\u003esize, bank-\u003ebase, bank-\u003enum_sectors);"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"dd3186a7_f36c2217","line":543,"updated":"2025-03-16 10:38:20.000000000","message":"still magic numbers that should go as macros","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"95751e2cee556d1e3b337ca4724cd8f4d552f2fe","unresolved":false,"context_lines":[{"line_number":540,"context_line":"\t// Max size -- up to two devices (two chip selects) in adjacent 24-bit address windows"},{"line_number":541,"context_line":"\tbank-\u003esize \u003d 32 * 1024 * 1024;"},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"\tbank-\u003enum_sectors \u003d bank-\u003esize / 4096;"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"\tLOG_INFO(\"RP2040 Flash Probe: %d bytes @\" TARGET_ADDR_FMT \", in %d sectors\\n\","},{"line_number":546,"context_line":"\t\tbank-\u003esize, bank-\u003ebase, bank-\u003enum_sectors);"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"1a8cddec_651dfd7f","line":543,"in_reply_to":"dd3186a7_f36c2217","updated":"2025-03-16 18:41:21.000000000","message":"Done","commit_id":"98eebf156cc23516d17d6036bc4cd8d91e3646b0"}]}