)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"39d79109d4606054c3531cf95e6154ea099aba3d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"cc8cc16c_3fab8a48","updated":"2022-09-21 15:51:35.000000000","message":"I have not read the other patches in the series, no idea if the comment below makes sense in the whole work","commit_id":"660576fd51f5dd9d57e46a79462a1fbeafb90e5f"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"2bfe46dd7d085fdb6d8690051d6bd171350822bc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fffb0d6d_5bb4bead","updated":"2022-09-27 08:31:19.000000000","message":"Let\u0027s leave this change after 0.12 release","commit_id":"57e069c41216b49649e137000eb1b84f0382ade0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"7b2fb62eb83bc25a26426d9771633ee8267a01ec","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a2e9c06a_d75e8208","updated":"2023-01-15 14:06:20.000000000","message":"Tomas, should this be merged as is?","commit_id":"57e069c41216b49649e137000eb1b84f0382ade0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"4121a943ed276ab085e91ab881e97ebad8a5bc26","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"4d11b835_4baaff22","in_reply_to":"a2e9c06a_d75e8208","updated":"2023-01-17 07:39:55.000000000","message":"Give me some time, I\u0027ll rework it according to the proposal.","commit_id":"57e069c41216b49649e137000eb1b84f0382ade0"}],"src/target/adi_v5_swd.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"39d79109d4606054c3531cf95e6154ea099aba3d","unresolved":true,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"\tassert(dap_is_multidrop(dap));"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"\tswd_send_sequence(dap, LINE_RESET);"},{"line_number":180,"context_line":"\t/* From ARM IHI 0074C ADIv6.0, chapter B4.3.3 \"Connection and line reset"},{"line_number":181,"context_line":"\t * sequence\":"},{"line_number":182,"context_line":"\t * - line reset sets DP_SELECT_DPBANK to zero;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"50d5298d_740b8f97","side":"PARENT","line":179,"updated":"2022-09-21 15:51:35.000000000","message":"It gets more difficult to follow the code ... why not passing through another parameter LINE_RESET or DORMANT_TO_SWD ?","commit_id":"97f6adac7c8c40ce7b2ce07df36acb7614098004"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"b7f2c766b1e3c901d318aaf1c92319919468036a","unresolved":true,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"\tassert(dap_is_multidrop(dap));"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"\tswd_send_sequence(dap, LINE_RESET);"},{"line_number":180,"context_line":"\t/* From ARM IHI 0074C ADIv6.0, chapter B4.3.3 \"Connection and line reset"},{"line_number":181,"context_line":"\t * sequence\":"},{"line_number":182,"context_line":"\t * - line reset sets DP_SELECT_DPBANK to zero;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"999875ee_7215d646","side":"PARENT","line":179,"in_reply_to":"50d5298d_740b8f97","updated":"2022-09-21 17:27:29.000000000","message":"First I thought it\u0027s a good idea.\nLooking to the rewritten code I don\u0027t see much improvement. Do you?","commit_id":"97f6adac7c8c40ce7b2ce07df36acb7614098004"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"72e1127fa417ea0537791e628bcfbe8331c34d46","unresolved":false,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"\tassert(dap_is_multidrop(dap));"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"\tswd_send_sequence(dap, LINE_RESET);"},{"line_number":180,"context_line":"\t/* From ARM IHI 0074C ADIv6.0, chapter B4.3.3 \"Connection and line reset"},{"line_number":181,"context_line":"\t * sequence\":"},{"line_number":182,"context_line":"\t * - line reset sets DP_SELECT_DPBANK to zero;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"d7efb92e_30e1f0a0","side":"PARENT","line":179,"in_reply_to":"999875ee_7215d646","updated":"2022-09-23 22:30:31.000000000","message":"For me the improvement is in having the big comment not replicated and not split (see my other review to this patchset2).\n\nApart from readability, do you think we could have some speed benefit by creating a single sequence that merges JTAG_TO_DORMANT + DORMANT_TO_SWD ?\nDo we run often swd_connect_multidrop() or only at connect and re-connect?","commit_id":"97f6adac7c8c40ce7b2ce07df36acb7614098004"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"691c3ab7d71d2ba0dba362dd5cd905bc5a5835c2","unresolved":false,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"\tassert(dap_is_multidrop(dap));"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"\tswd_send_sequence(dap, LINE_RESET);"},{"line_number":180,"context_line":"\t/* From ARM IHI 0074C ADIv6.0, chapter B4.3.3 \"Connection and line reset"},{"line_number":181,"context_line":"\t * sequence\":"},{"line_number":182,"context_line":"\t * - line reset sets DP_SELECT_DPBANK to zero;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"1f2c3919_2a90cf04","side":"PARENT","line":179,"in_reply_to":"d7efb92e_30e1f0a0","updated":"2022-09-25 11:03:10.000000000","message":"No, just at (re-)connect.\nIMO it\u0027s a pity that patchset 2 splited two subsequent swd_send_sequence() apart.","commit_id":"97f6adac7c8c40ce7b2ce07df36acb7614098004"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"72e1127fa417ea0537791e628bcfbe8331c34d46","unresolved":true,"context_lines":[{"line_number":189,"context_line":"\t * - other accesses return protocol error (SWDIO not driven by target)."},{"line_number":190,"context_line":"\t *"},{"line_number":191,"context_line":"\t * Read DP_DPIDR to get out of reset. Initialize dap-\u003eselect to zero to"},{"line_number":192,"context_line":"\t * skip the write to DP_SELECT, avoiding the protocol error."},{"line_number":193,"context_line":"\t */"},{"line_number":194,"context_line":"\tdap-\u003eselect \u003d 0;"},{"line_number":195,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"dd2ea415_dc96cb65","line":192,"updated":"2022-09-23 22:30:31.000000000","message":"I would move back here the comment\n *  ...................       Set again\n * dap-\u003eselect to DP_SELECT_INVALID because the rest of the register is\n * unknown after line reset.\n\nto keep the whole explanation in only one comment. The line that sets DP_SELECT_INVALID is not too far, but splitting the comment we loose the whole story.","commit_id":"57e069c41216b49649e137000eb1b84f0382ade0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"48babcdbf44597898c209c2f7e516bfd3b394341","unresolved":true,"context_lines":[{"line_number":189,"context_line":"\t * - other accesses return protocol error (SWDIO not driven by target)."},{"line_number":190,"context_line":"\t *"},{"line_number":191,"context_line":"\t * Read DP_DPIDR to get out of reset. Initialize dap-\u003eselect to zero to"},{"line_number":192,"context_line":"\t * skip the write to DP_SELECT, avoiding the protocol error."},{"line_number":193,"context_line":"\t */"},{"line_number":194,"context_line":"\tdap-\u003eselect \u003d 0;"},{"line_number":195,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"6ef8b051_fa9b4b0f","line":192,"in_reply_to":"0fc4f34d_6854b9a6","updated":"2023-03-13 18:44:16.000000000","message":"Antonio, the big comment is rather misleading. At least I\u0027ve been fooled that \n - line reset sets DP_SELECT_DPBANK to zero\n\non both ADIv5 and v6. It\u0027s not true! ARM IHI 0031F, the same chapter B4.3.3 \"Connection and line reset sequence\" reads:\n A line reset resets DLCR.\n Note\n Other SW-DP registers are reset only by a powerup reset.\n\n!!!!\nTests with DPv1 and v2 confirm that.\n\nSo we still need DP_SELECT_DPBANK0_APINVALID state used for DPv3 reg 0 banking (there is no banking in DPv012) but must not use it for DP reg 4 banking, which takes place in DPv123\n\nMoreover DP_SELECT_INVALID 0x00FFFF00 seems me a valid ADIv6 address.","commit_id":"57e069c41216b49649e137000eb1b84f0382ade0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"91e30a1a23bb12167a126e78d7706da8cbeb489c","unresolved":false,"context_lines":[{"line_number":189,"context_line":"\t * - other accesses return protocol error (SWDIO not driven by target)."},{"line_number":190,"context_line":"\t *"},{"line_number":191,"context_line":"\t * Read DP_DPIDR to get out of reset. Initialize dap-\u003eselect to zero to"},{"line_number":192,"context_line":"\t * skip the write to DP_SELECT, avoiding the protocol error."},{"line_number":193,"context_line":"\t */"},{"line_number":194,"context_line":"\tdap-\u003eselect \u003d 0;"},{"line_number":195,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"0fc4f34d_6854b9a6","line":192,"in_reply_to":"2dc6d962_27c080b1","updated":"2022-09-27 09:09:47.000000000","message":"agree, but keep in mind that now we support also ADIv6.\nIn ADIv6 register SELECT contains still DPBANKSEL in bits[3:0] and the rest of the register is ADDR in bits[31:4] (plus 64 bits continuation ADDR in reg SELECT1 bits[31:0]). At reset DPBANKSEL is zero, while ADDR is UNKNOWN.\nThe three states should be:\n- DP_SELECT fully UNKNOWN, for openocd attach without reset the DP (e.g. attach in JTAG. Is it really needed even after TLR sequence? to be checked)/\n- after DAP reset, DPBANKSEL is ZERO, other than DPBANKSEL is UNKNOWN.\n- DP_SELECT fully known and cached by openocd.\nTo be studied if can be done as special values in dap-\u003eselect or we need to add an enum dap-\u003eselect_status.","commit_id":"57e069c41216b49649e137000eb1b84f0382ade0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"6dfd21f26dcddefb253386a27f11a0d233dd7ed1","unresolved":true,"context_lines":[{"line_number":189,"context_line":"\t * - other accesses return protocol error (SWDIO not driven by target)."},{"line_number":190,"context_line":"\t *"},{"line_number":191,"context_line":"\t * Read DP_DPIDR to get out of reset. Initialize dap-\u003eselect to zero to"},{"line_number":192,"context_line":"\t * skip the write to DP_SELECT, avoiding the protocol error."},{"line_number":193,"context_line":"\t */"},{"line_number":194,"context_line":"\tdap-\u003eselect \u003d 0;"},{"line_number":195,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"bff73ff4_bdb3ecb6","line":192,"in_reply_to":"6ef8b051_fa9b4b0f","updated":"2023-03-15 18:04:11.000000000","message":"Regarding\n\n\u003e Moreover DP_SELECT_INVALID 0x00FFFF00 seems me a valid ADIv6 address.\n\nHumm, the spec ADIv6 say that register SELECT has all the bits used, with 31-4 for ADDR, and 3-0 for DPBANKSEL, so 0x00FFFF00 looks a valid address.\nUnfortunately SELECT is write-only, so we cannot check if bits 11-4 are really writeble.\nBut bits 11-4 will never be used in ADIv6.\nIn fact we start from BASEPTR0/BASEPTR1 that have valid address bits in 63-12. This provides the AP for the initial ROM table.\nThen, every entry in the ROM table \"ROMENTRY\u003cn\u003e\" also have valid address bits only in 63-12.\nWith ADIv6, every time SELECT will be set with zero in bits 11-0, making 0x00FFFF00 invalid.\nMaybe we need a comment on top of the define of DP_SELECT_INVALID","commit_id":"57e069c41216b49649e137000eb1b84f0382ade0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"f1e2875b626510a90a2713f006d6a7dc1f29a01d","unresolved":false,"context_lines":[{"line_number":189,"context_line":"\t * - other accesses return protocol error (SWDIO not driven by target)."},{"line_number":190,"context_line":"\t *"},{"line_number":191,"context_line":"\t * Read DP_DPIDR to get out of reset. Initialize dap-\u003eselect to zero to"},{"line_number":192,"context_line":"\t * skip the write to DP_SELECT, avoiding the protocol error."},{"line_number":193,"context_line":"\t */"},{"line_number":194,"context_line":"\tdap-\u003eselect \u003d 0;"},{"line_number":195,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"b5f6e83d_b9a59f7a","line":192,"in_reply_to":"bff73ff4_bdb3ecb6","updated":"2023-10-04 15:36:48.000000000","message":"I think the situation is correctly described in\nhttps://review.openocd.org/c/openocd/+/7541/6//COMMIT_MSG#14","commit_id":"57e069c41216b49649e137000eb1b84f0382ade0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"bdc6e5c7ce8ff59cb48f8dc17b86f9aa87256788","unresolved":true,"context_lines":[{"line_number":189,"context_line":"\t * - other accesses return protocol error (SWDIO not driven by target)."},{"line_number":190,"context_line":"\t *"},{"line_number":191,"context_line":"\t * Read DP_DPIDR to get out of reset. Initialize dap-\u003eselect to zero to"},{"line_number":192,"context_line":"\t * skip the write to DP_SELECT, avoiding the protocol error."},{"line_number":193,"context_line":"\t */"},{"line_number":194,"context_line":"\tdap-\u003eselect \u003d 0;"},{"line_number":195,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"2dc6d962_27c080b1","line":192,"in_reply_to":"dd2ea415_dc96cb65","updated":"2022-09-25 10:57:38.000000000","message":"Makes sense.\nOn the other hand, a little hack setting DP_SELECT_INVALID needs its own comment too, as it should stay located on \"magic\" place or it causes an unnecessary transaction or breaks the function completely.\n\nI\u0027m thinking about introducing another dp-\u003eselect pseudo state, let\u0027s say DP_SELECT_DPBANK0_APINVALID to reflect precisely the DP state after LINE_RESET sequence. Having this state would save later setting of DP_SELECT_INVALID.\nIt would require a small change in swd_queue_dp_bankselect() but this code should be changed anyway, as the test for already selected DP bank is suboptimal (we should not include APSEL and APBANK fields to the comparison).\nWe might also use a separate enum variable for select_validity instead of pseudo values with reserved bits set, but I don\u0027t see it\u0027s necessary.\nWhat do you think?","commit_id":"57e069c41216b49649e137000eb1b84f0382ade0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"8cafd0e9004ed1452b51c7225f703e753426b700","unresolved":true,"context_lines":[{"line_number":636,"context_line":""},{"line_number":637,"context_line":"\tdone \u003d true;"},{"line_number":638,"context_line":"\tif (dap_is_multidrop(dap)) {"},{"line_number":639,"context_line":"\t\tswd-\u003eswitch_seq(SWD_TO_DORMANT);"},{"line_number":640,"context_line":"\t\tswd_multidrop_in_swd_state \u003d false;"},{"line_number":641,"context_line":"\t\t/* Revisit!"},{"line_number":642,"context_line":"\t\t * Leaving DPs in dormant state was tested and offers some safety"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"2bc75081_449fb306","line":639,"updated":"2023-04-06 22:20:24.000000000","message":"is it correct to send SWD_TO_DORMANT when swd_multidrop_in_swd_state\u003d\u003dfalse?","commit_id":"1f9fd668affba2acc308479b95952c515d471d5e"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"efa3154bf008fb77769c899c6d8e0c8e9c0d8bf3","unresolved":false,"context_lines":[{"line_number":636,"context_line":""},{"line_number":637,"context_line":"\tdone \u003d true;"},{"line_number":638,"context_line":"\tif (dap_is_multidrop(dap)) {"},{"line_number":639,"context_line":"\t\tswd-\u003eswitch_seq(SWD_TO_DORMANT);"},{"line_number":640,"context_line":"\t\tswd_multidrop_in_swd_state \u003d false;"},{"line_number":641,"context_line":"\t\t/* Revisit!"},{"line_number":642,"context_line":"\t\t * Leaving DPs in dormant state was tested and offers some safety"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"ec4a636f_7c28ee22","line":639,"in_reply_to":"2bc75081_449fb306","updated":"2023-10-04 15:24:03.000000000","message":"Addressed in a comment","commit_id":"1f9fd668affba2acc308479b95952c515d471d5e"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"8cafd0e9004ed1452b51c7225f703e753426b700","unresolved":true,"context_lines":[{"line_number":637,"context_line":"\tdone \u003d true;"},{"line_number":638,"context_line":"\tif (dap_is_multidrop(dap)) {"},{"line_number":639,"context_line":"\t\tswd-\u003eswitch_seq(SWD_TO_DORMANT);"},{"line_number":640,"context_line":"\t\tswd_multidrop_in_swd_state \u003d false;"},{"line_number":641,"context_line":"\t\t/* Revisit!"},{"line_number":642,"context_line":"\t\t * Leaving DPs in dormant state was tested and offers some safety"},{"line_number":643,"context_line":"\t\t * against DPs mismatch in case of unintentional use of non-multidrop SWD."}],"source_content_type":"text/x-csrc","patch_set":4,"id":"0d585f60_ab97cc08","line":640,"updated":"2023-04-06 22:20:24.000000000","message":"should this be set to false in all cases, not only in multidrop?","commit_id":"1f9fd668affba2acc308479b95952c515d471d5e"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"efa3154bf008fb77769c899c6d8e0c8e9c0d8bf3","unresolved":false,"context_lines":[{"line_number":637,"context_line":"\tdone \u003d true;"},{"line_number":638,"context_line":"\tif (dap_is_multidrop(dap)) {"},{"line_number":639,"context_line":"\t\tswd-\u003eswitch_seq(SWD_TO_DORMANT);"},{"line_number":640,"context_line":"\t\tswd_multidrop_in_swd_state \u003d false;"},{"line_number":641,"context_line":"\t\t/* Revisit!"},{"line_number":642,"context_line":"\t\t * Leaving DPs in dormant state was tested and offers some safety"},{"line_number":643,"context_line":"\t\t * against DPs mismatch in case of unintentional use of non-multidrop SWD."}],"source_content_type":"text/x-csrc","patch_set":4,"id":"9faf19ee_55b8e542","line":640,"in_reply_to":"0d585f60_ab97cc08","updated":"2023-10-04 15:24:03.000000000","message":"Seems useless.\nThe variable is not used outside multidrop mode.\nThe variable gets reset before connect\nhttps://review.openocd.org/c/openocd/+/7218/8/src/target/adi_v5_swd.c#411\n\nSo the resetting here is just for good programming style, actually not necessary.\n\nBTW OpenOCD quits very soon after calling dap_cleanup_all() which is the only caller of dap-\u003eops-\u003equit() ;-)","commit_id":"1f9fd668affba2acc308479b95952c515d471d5e"}]}