)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"5e2d54035ff42841f26e7760d3f48f4560b2c2c5","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0988f11c_d2b4dbe0","updated":"2026-04-27 15:19:32.000000000","message":"Thanks for the patch! AFAIU, it should greatly improve the performance (though it seems there is still room for further improving the performance).\nThe main issue I have with this patch concerns the case of busy on SystemBus access (issuing the next access to soon). AFAIU, with the current design:\n1. The whole lot of accesses is run.\n2. All the accesses fail due to SB busy status until the sufficient delay is reached.\n\nIf you were to add a simple sbcs read to each batch you would be able to detect the busy status early and avoid running the extra chunks.","commit_id":"0fe07010ec2f5c1c358e75f2f7d6e69efb33cd4f"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"2c349b28c79ae916d43146ee32c04e7f4ed33ba6","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7ab5ad46_453088b4","in_reply_to":"0988f11c_d2b4dbe0","updated":"2026-04-28 07:20:53.000000000","message":"Thanks for review, Evgeniy!\nI wrote the patch about 6 months ago. I would need some time to get into riscv details again and unfortunately now I\u0027m quite busy to do it now.\nYou wrote alternative code so feel free to take over this patch or submit a better patch for this topic.\nAnyway we should focus to finally tag 1.0 rc. This patch could wait.\nPlease look at 6964: target: base poll frequency on wall time | https://review.openocd.org/c/openocd/+/6964\nwhich is probably the last blocker.","commit_id":"0fe07010ec2f5c1c358e75f2f7d6e69efb33cd4f"}],"src/target/riscv/riscv-013.c":[{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"5e2d54035ff42841f26e7760d3f48f4560b2c2c5","unresolved":true,"context_lines":[{"line_number":3377,"context_line":""},{"line_number":3378,"context_line":"\t/* TODO: Reading all the elements in a single batch will boost the"},{"line_number":3379,"context_line":"\t * performance."},{"line_number":3380,"context_line":"\t */"},{"line_number":3381,"context_line":"\twhile (next_address \u003c end_address) {"},{"line_number":3382,"context_line":"\t\tuint32_t sbcs_write \u003d set_field(0, DM_SBCS_SBREADONADDR, 1);"},{"line_number":3383,"context_line":"\t\tsbcs_write |\u003d sb_sbaccess(size);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"9ccc1722_d34de952","side":"PARENT","line":3380,"updated":"2026-04-27 15:19:32.000000000","message":"TBH, when writing this comment I was imagining a different outlook for this function:\n```\nwhile (next_address \u003c end_address) {\n    struct riscv_batch *batch \u003d riscv_batch_alloc(...);\n    fill_sba_read_prologue(batch, ...) // adds to the batch the writes to sbcs and sbaddr, returns the number of scans used.\n    fill_sba_read_body(batch, ...) // adds to the batch the reads of sbdata*.\n    res \u003d batch_run_timeout(target, batch);\n    if (res !\u003d ERROR_OK) {\n        riscv_batch_free(batch);\n        break;\n    }\n    uint32_t sbcs_read;\n    if (read_sbcs_nonbusy(target, \u0026sbcs_read) !\u003d ERROR_OK) {\n        riscv_batch_free(batch);\n        return ERROR_FAIL;\n    }\n    if (restore_sbcs(target, \u0026sbcs_read) !\u003d ERROR_OK) {\n        riscv_batch_free(batch);\n        return ERROR_FAIL;\n    }\n    ...\n    size_t n_read \u003d 0;\n    switch (get_field(sbcs_read, DM_SBCS_SBERROR)) {\n         ...\n    }\n    report_reads(...)\n    riscv_batch_free(batch);\n}\n```","commit_id":"d56698ae0d208de8c50aa9b0d9f56a409a372ae4"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"5e2d54035ff42841f26e7760d3f48f4560b2c2c5","unresolved":true,"context_lines":[{"line_number":3410,"context_line":"\t\t\t * fetch all the data from the last bus read. */"},{"line_number":3411,"context_line":"\t\t\tfor (uint32_t m \u003d 0; m \u003c chunk; m++) {"},{"line_number":3412,"context_line":"\t\t\t\tfor (uint32_t j \u003d size_in_words - 1; j \u003e 0; --j)"},{"line_number":3413,"context_line":"\t\t\t\t\triscv_batch_add_dm_read(batch, sbdata[j], RISCV_DELAY_BASE);"},{"line_number":3414,"context_line":"\t\t\t\triscv_batch_add_dm_read(batch, sbdata[0], RISCV_DELAY_SYSBUS_READ);"},{"line_number":3415,"context_line":"\t\t\t}"},{"line_number":3416,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"a0c0df04_074b1bb6","line":3413,"updated":"2026-04-27 15:19:32.000000000","message":"[suggestion]\nThis is 4 loops deep. Please, consider factoring this out into something like `fill_sba_read_batch()`.\nThe same is true for the loop of `riscv_batch_get_dmi_read_data()` calls.","commit_id":"0fe07010ec2f5c1c358e75f2f7d6e69efb33cd4f"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"5e2d54035ff42841f26e7760d3f48f4560b2c2c5","unresolved":true,"context_lines":[{"line_number":3430,"context_line":"\t\t\t\t\tk++;"},{"line_number":3431,"context_line":"\t\t\t\t}"},{"line_number":3432,"context_line":"\t\t\t\tconst target_addr_t read_addr \u003d address + (i + m) * increment;"},{"line_number":3433,"context_line":"\t\t\t\tlog_memory_access(read_addr, sbvalue, size, true);"},{"line_number":3434,"context_line":"\t\t\t}"},{"line_number":3435,"context_line":"\t\t\triscv_batch_free(batch);"},{"line_number":3436,"context_line":"\t\t\ti +\u003d chunk;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"b1a76ac1_5bbe2e9f","line":3433,"updated":"2026-04-27 15:19:32.000000000","message":"There is a slight issue -- the access is being reported prematurely. At this point it is not yet known whether the access succeeded or not.\n\nThe issue does not originate from your patch -- it was already there. However, the change does exacerbate the issue -- before the patch a single read was misreported as successful, now there will be a whole bunch of fake reads in the log.\n\nI feel like there should at least be a comment regarding this.","commit_id":"0fe07010ec2f5c1c358e75f2f7d6e69efb33cd4f"}]}