)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"3ec978f9a85e852ae348d1abf952621280d700d4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"82b659b8_4f4d0952","updated":"2026-05-20 11:57:15.000000000","message":"Tomas, please take a look if you have the time.","commit_id":"756b8d7e838111d62413f602876ec7dc397018f2"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"907f9a9ac46cbce87d93d9f207fae14fa5fddd14","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3e4f7229_b822c503","updated":"2026-06-15 13:40:09.000000000","message":"Hi Evgeniy,\n\nthanks for working to eliminate the fake target_types \"riscv013_target\" and \"riscv011_target\". BTW, I think the function pointers in struct riscv_info are the same form of antipattern.\n\nCould you please consider this different, simpler approach?\n\n1) Remove riscv011_target and riscv013_target structures.\n2) Remove function pointers from struct riscv_info.\n3) Expose the the version specific functions in the header files: riscv-013.h or riscv-011.h.\n4) Whenever it is necessary to pick one of the implementations (0.11 or 0.13), use a standard switch (or a macro):\n\n```\nswitch (info-\u003edtm_version) {\ncase DTM_DTMCS_VERSION_0_11:\n    return riscv011_foo(arg1, arg2);\ncase DTM_DTMCS_VERSION_1_0:\n    return riscv013_foo(arg1, arg2);\ndefault:\n    assert(false \u0026\u0026 \"...\");\n    return ERROR_FAIL;\n}\n\n// or with a helper macro\n\nRISCV_CHOOSE_IMPLEM(info, riscv011_foo, riscv013_foo, arg1, arg2);\n\n```\n\nThis way, all calls will be static, no need for function pointers. This should help both readers and static analysis tools.\n\nWould you like it?","commit_id":"cc3a388e555d319b978c5c4ea2099aac5525934d"}],"src/target/riscv/riscv-013.c":[{"author":{"_account_id":1002135,"name":"Mark Zhuang","email":"mark.zhuang@spacemit.com","username":"zqb-all"},"change_message_id":"c89d1754407fc868dadc38955aff72b499807fd4","unresolved":true,"context_lines":[{"line_number":1773,"context_line":""},{"line_number":1774,"context_line":"static void riscv013_deinit_version_specific(struct target *target)"},{"line_number":1775,"context_line":"{"},{"line_number":1776,"context_line":"\tLOG_TARGET_DEBUG(target, \"Deinitializing verison-specific info\");"},{"line_number":1777,"context_line":""},{"line_number":1778,"context_line":"\tstruct riscv_info *info \u003d target-\u003earch_info;"},{"line_number":1779,"context_line":"\tif (!info)"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"7666daab_6aae0fc6","line":1776,"updated":"2026-06-14 15:09:32.000000000","message":"verison -\u003e version","commit_id":"cc3a388e555d319b978c5c4ea2099aac5525934d"}],"src/target/riscv/riscv.c":[{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"65a0bf02f9da3a1470315b84c05fc5d0dd1f3328","unresolved":true,"context_lines":[{"line_number":5982,"context_line":""},{"line_number":5983,"context_line":"\tr-\u003ecommon_magic \u003d RISCV_COMMON_MAGIC;"},{"line_number":5984,"context_line":""},{"line_number":5985,"context_line":"\tr-\u003edtm_version \u003d DTM_DTMCS_VERSION_UNKNOWN;"},{"line_number":5986,"context_line":"\tr-\u003eversion_specific \u003d NULL;"},{"line_number":5987,"context_line":""},{"line_number":5988,"context_line":"\tmemset(r-\u003etrigger_unique_id, 0xff, sizeof(r-\u003etrigger_unique_id));"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"f00f52f1_20083f23","side":"PARENT","line":5985,"updated":"2026-06-15 13:58:16.000000000","message":"As mentioned in [a previous comment](https://review.openocd.org/c/openocd/+/9668/comments/3e4f7229_b822c503), I would prefer to keep the version number in the structure. And based on the value, simply call one function or another.","commit_id":"103cde964855ab5693e15f3b0cefbdb10027ba0d"},{"author":{"_account_id":1002135,"name":"Mark Zhuang","email":"mark.zhuang@spacemit.com","username":"zqb-all"},"change_message_id":"c89d1754407fc868dadc38955aff72b499807fd4","unresolved":true,"context_lines":[{"line_number":2478,"context_line":""},{"line_number":2479,"context_line":"static int init_dtm_version_specific_type(struct target *target, uint32_t dtm_version)"},{"line_number":2480,"context_line":"{"},{"line_number":2481,"context_line":"\tLOG_TARGET_DEBUG(target, \"DTM version\u003d0x%\" PRIu32, dtm_version);"},{"line_number":2482,"context_line":"\tstruct riscv_info *r \u003d riscv_info(target);"},{"line_number":2483,"context_line":"\tassert(!r-\u003eversion_specific_type);"},{"line_number":2484,"context_line":"\tr-\u003eversion_specific_type \u003d dtm_version_to_type(dtm_version);"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"7f5b94f6_3374d4a4","line":2481,"updated":"2026-06-14 15:09:32.000000000","message":"\"0x%\" PRIu32 is inconsistent, maybe we should drop \"0x“","commit_id":"cc3a388e555d319b978c5c4ea2099aac5525934d"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"dc23bff56fcb6d9936f6bcba10ad29b0c6e453b4","unresolved":true,"context_lines":[{"line_number":2516,"context_line":"\tconst struct riscv_info *r \u003d riscv_info(target);"},{"line_number":2517,"context_line":"\tassert(r);"},{"line_number":2518,"context_line":"\tuint32_t dtm_version \u003d get_field32(dtmcontrol, DTMCONTROL_VERSION);"},{"line_number":2519,"context_line":"\tint res \u003d r-\u003eversion_specific_type ?"},{"line_number":2520,"context_line":"\t\tcheck_version_specific_type(target, dtm_version) :"},{"line_number":2521,"context_line":"\t\tinit_dtm_version_specific_type(target, dtm_version);"},{"line_number":2522,"context_line":"\tif (res !\u003d ERROR_OK)"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"8c3057e8_9f611d23","line":2519,"updated":"2026-06-15 14:06:51.000000000","message":"Did I get the meaning of this part of the code correctly, please?\n\n```\nif (r-\u003edtm_version \u003d\u003d DTM_DTMCS_VERSION_UNKNOWN) {\n    /* First time examination examination */\n    if (!check_supported_dtm_version(found_dtm_verson))  /* Check \u0026 print error */\n        return ERROR_FAIL;\n    \n    r-\u003edtm_version \u003d found_dtm_version;\n} else {\n    /* Re-examination */\n    if (r-\u003edtm_version !\u003d found_dtm_version) {\n        LOG_TARGET_ERROR(target, \"The DTM version of the target \"\n            \"unexpectedly changed (0x%\" PRIx32 \" --\u003e 0x%\" PRIx32 \"). \"\n            \"Check the target and adapter connection\",\n            r-\u003edtm_version,\n            found_dtm_version);\n    }\n    return ERROR_FAIL;\n}\n\n```\n\nIf so, I think the variant with keeping `r-\u003edtm_version` stored, and deriving everything else from it, would lead to clearer code.","commit_id":"cc3a388e555d319b978c5c4ea2099aac5525934d"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"65a0bf02f9da3a1470315b84c05fc5d0dd1f3328","unresolved":true,"context_lines":[{"line_number":5452,"context_line":"\t}"},{"line_number":5453,"context_line":""},{"line_number":5454,"context_line":"\tRISCV_INFO(r);"},{"line_number":5455,"context_line":"\tif (r-\u003eversion_specific_type !\u003d \u0026riscv_dtm_v1_0_type) {"},{"line_number":5456,"context_line":"\t\tLOG_TARGET_ERROR(target, \"exec_progbuf: Program buffer is \""},{"line_number":5457,"context_line":"\t\t\t\t\"only supported on v0.13 or v1.0 targets.\");"},{"line_number":5458,"context_line":"\t\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"71705d31_bc50a8a3","line":5455,"updated":"2026-06-15 13:58:16.000000000","message":"(If you decide to stick with this approach)\n\nI think `version_specific_type` is an awkward name (perhaps a merger of \"version_specific\" and \"target_type\"?).  \n\nHow about something like this? \n\n`version_specific_callbacks`\n`version_specific_api`\n`version_specific_impl`\n(or similar)","commit_id":"cc3a388e555d319b978c5c4ea2099aac5525934d"}],"src/target/riscv/riscv.h":[{"author":{"_account_id":1002135,"name":"Mark Zhuang","email":"mark.zhuang@spacemit.com","username":"zqb-all"},"change_message_id":"c89d1754407fc868dadc38955aff72b499807fd4","unresolved":true,"context_lines":[{"line_number":122,"context_line":"\tchar *name;"},{"line_number":123,"context_line":"} range_list_t;"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"#define DTM_DTMCS_VERSION_UNKNOWN ((unsigned int)-1)"},{"line_number":126,"context_line":"#define RISCV_TINFO_VERSION_UNKNOWN (-1)"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"#define RISCV013_DTMCS_ABITS_MIN 7"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"2e90f977_6fdeb4b9","line":125,"updated":"2026-06-14 15:09:32.000000000","message":"DTM_DTMCS_VERSION_UNKNOWN becomes unused after this patch and should be removed","commit_id":"cc3a388e555d319b978c5c4ea2099aac5525934d"}]}
