)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1002315,"name":"liangzhen","email":"zhen.liang@spacemit.com","username":"liangzhen"},"change_message_id":"e5919320c764813d147acea286f2e5e43e6dca31","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"65c41a76_28d6341a","updated":"2025-11-10 06:50:10.000000000","message":"@Antonio, @Tomas,\ndo you have any comment on this series?","commit_id":"2870efedea74f1e805c51cbc997716a4dc471f36"},{"author":{"_account_id":1002315,"name":"liangzhen","email":"zhen.liang@spacemit.com","username":"liangzhen"},"change_message_id":"ca362ecb8648e05f25c13979f47bbb9438bc1740","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"23cdb963_ceb81afb","updated":"2025-11-19 08:53:17.000000000","message":"@Antonio, @Tomas, could you please take a look?","commit_id":"2870efedea74f1e805c51cbc997716a4dc471f36"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"29abce858db475155dfc16a57c57da2f2ac8567b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f1b8d913_d410931f","updated":"2025-11-06 15:50:17.000000000","message":"Could you please elaborate what is the reason to implement this as a C code? To me all this looks like a perfect task for .tcl-based scripts.","commit_id":"2870efedea74f1e805c51cbc997716a4dc471f36"},{"author":{"_account_id":1000853,"name":"Marc Schink","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"74c0afb62dac4fc063edb49415b3f2c09cce5dee","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"88cc5fe8_f36781e2","in_reply_to":"1913285c_673220ec","updated":"2025-11-19 10:42:12.000000000","message":"@liangzhen: Thanks for the contribution! Just a few comments from my side as I did not have time for a full review.\n\nAre you aware that there are already some CS trace implementations on Gerrit? I also have at least some implementations (local and not on Gerrit at the moment). We have to examine and merge (pick the best parts of all of them) somehow.\n\n\u003e I\u0027m not sure about that. We\u0027d better to ask for rationale from OpenOCD maintainers regarding the decision to implement it in C. @Antonio, @Tomas maybe you have an option on the matter?\n\nYes, it is correct the some trace mechanisms are already implemented in C. Although I\u0027m a friend and supporter of Tcl, in this case I must clearly speak out in favor of C. The code for all these trace components is definitely better to maintain in C.\n\n@Anatoly, think about all the bit fiddling in Tcl, it\u0027s a total mess. The trace components should be implemented with a simple interface so that they can then be easily used/automated/whatever with Tcl scripts.\n\n@liangzhen: What hardware do you use for testing?","commit_id":"2870efedea74f1e805c51cbc997716a4dc471f36"},{"author":{"_account_id":1002315,"name":"liangzhen","email":"zhen.liang@spacemit.com","username":"liangzhen"},"change_message_id":"6ab9249a9bd1e6ae229e2d91fc3b4d1dd6d10a8d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"38b31bfa_003cf874","in_reply_to":"88cc5fe8_f36781e2","updated":"2025-11-20 05:44:51.000000000","message":"\u003e Are you aware that there are already some CS trace implementations on Gerrit? I also have at least some implementations (local and not on Gerrit at the moment). We have to examine and merge (pick the best parts of all of them) somehow.\n\nYes, I’m aware that. Agree, and current implementation requires independent commands for each component, and I think it would be better to use a single \"cstrace create \u003ctype\u003e ... / rvtrace create \u003ctype\u003e ...\" command.\n\u003e What hardware do you use for testing?\n\nChip under development from Spacemit","commit_id":"2870efedea74f1e805c51cbc997716a4dc471f36"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"2c599385cac91f8e432a327e2c040613de5a1b3c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"1913285c_673220ec","in_reply_to":"d0b43404_50390e95","updated":"2025-11-07 09:44:13.000000000","message":"@liangzhen\n\n\u003e The C language has advantages in terms of performance, maintainability, and extensibility.\n\n- I\u0027m not sure about maintainability and extensibility. From my experience these are false statements. \n- It is also not clear to me which part of your changes requires performance. Could you please point out which part of this specific change requires performance? (I\u0027m asking, because I don\u0027t see it). And even if this is the case - maybe we should limit the scope of what should go to C part?\n\n\u003e While TCL scripts are well-suited for rapid hardware configuration and debugging,\n\nI guess this part of the patches you\u0027ve provided focuses on exactly that. HW configuration.\n\n\n\u003e As far as I known, OpenOCD has basic support for some trace mechanisms (e.g. Arm ETM, CTI, TPIU) based on C. So, I think RSC-V trace should also be like that?\n\nI\u0027m not sure about that. We\u0027d better to ask for rationale from OpenOCD maintainers regarding the decision to implement it in C. @Antonio, @Tomas maybe you have an option on the matter?\n\n\n\u003e Implementing an abstract/generalised RISC-V trace controler using the C language, and employing TCL scripts for hardware configuration of related drivers, I believe this approach is more user-friendly.\n\nMy point is that such tasks are ideal for .tcl scripts. The user won\u0027t notice a difference. The respected .tcl scripts may be easier to maintain","commit_id":"2870efedea74f1e805c51cbc997716a4dc471f36"},{"author":{"_account_id":1002315,"name":"liangzhen","email":"zhen.liang@spacemit.com","username":"liangzhen"},"change_message_id":"5380ff1c61f4cb6eafa050f6a607f0f57b31524b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"d0b43404_50390e95","in_reply_to":"f1b8d913_d410931f","updated":"2025-11-07 01:57:39.000000000","message":"Thanks for your comment!\nI chose to implement these RISC-V trace drivers(e.g. encoder, funnel, ATB) in C language instead of Tcl scripts for the following main reasons:\n1. The C language has advantages in terms of performance, maintainability, and extensibility. \n2. While TCL scripts are well-suited for rapid hardware configuration and debugging, C offers greater flexibility and performance when complex data processing and hardware control are required.\n3. As far as I known, OpenOCD has basic support for some trace mechanisms (e.g. Arm ETM, CTI, TPIU) based on C. So, I think RSC-V trace should also be like that?\n4. Implementing an abstract/generalised RISC-V trace controler using the C language, and employing TCL scripts for hardware configuration of related drivers, I believe this approach is more user-friendly.","commit_id":"2870efedea74f1e805c51cbc997716a4dc471f36"}],"src/target/riscv/trace_encoder.c":[{"author":{"_account_id":1000853,"name":"Marc Schink","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"74c0afb62dac4fc063edb49415b3f2c09cce5dee","unresolved":true,"context_lines":[{"line_number":13,"context_line":"#include \u003chelper/command.h\u003e"},{"line_number":14,"context_line":"#include \u003chelper/jim-nvp.h\u003e"},{"line_number":15,"context_line":"#include \"riscv_trace.h\""},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"#define RVTRACE_ENCODER_CTRL\t\t\t\t    0x000"},{"line_number":18,"context_line":"#define RVTRACE_ENCODER_IMPL\t\t\t\t    0x004"},{"line_number":19,"context_line":"#define RVTRACE_ENCODER_INSTFEAT\t\t\t    0x008"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"8ecb800d_4eec5e14","line":16,"updated":"2025-11-19 10:42:12.000000000","message":"Please add a reference to the trace encoder specification document, see `arm_tpiu_swo.c` as an example.","commit_id":"2870efedea74f1e805c51cbc997716a4dc471f36"},{"author":{"_account_id":1000853,"name":"Marc Schink","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"74c0afb62dac4fc063edb49415b3f2c09cce5dee","unresolved":true,"context_lines":[{"line_number":33,"context_line":"#define RVTRACE_ENCODER_CTRL_INSTSYNCMODE\t\t    0x30000"},{"line_number":34,"context_line":"#define RVTRACE_ENCODER_CTRL_INSTSYNCMAX\t\t    0xf00000"},{"line_number":35,"context_line":"#define RVTRACE_ENCODER_CTRL_FORMAT\t\t\t    0x7000000"},{"line_number":36,"context_line":"/* trTeInstFeatures - 0x008 */"},{"line_number":37,"context_line":"#define RVTRACE_ENCODER_INSTFEAT_NOADDRDIFF\t\t    0x1"},{"line_number":38,"context_line":"#define RVTRACE_ENCODER_INSTFEAT_NOTRAPADDR\t\t    0x2"},{"line_number":39,"context_line":"#define RVTRACE_ENCODER_INSTFEAT_ENSEQUENTIALJUMP\t    0x4"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"6292f432_8083ef39","line":36,"updated":"2025-11-19 10:42:12.000000000","message":"Use `//` for single-line comments here and in the rest of the code.","commit_id":"2870efedea74f1e805c51cbc997716a4dc471f36"},{"author":{"_account_id":1000853,"name":"Marc Schink","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"74c0afb62dac4fc063edb49415b3f2c09cce5dee","unresolved":true,"context_lines":[{"line_number":121,"context_line":"\treturn comp-\u003eprivate_config;"},{"line_number":122,"context_line":"}"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"#define RV_ENCODER_INFO(R) struct rv_encoder_info *(R) \u003d rv_encoder_info(comp)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"static struct rv_encoder_private_config *alloc_default_rv_encoder_private_config(void)"},{"line_number":127,"context_line":"{"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"503786eb_88e50b8b","line":124,"updated":"2025-11-19 10:42:12.000000000","message":"I know that this construct is used frequently in the new RISC-V codebase. Personally, I don\u0027t like it and hope that we can get rid of it, as it doesn\u0027t make the code any more readable or understandable. Please consider removing it.","commit_id":"2870efedea74f1e805c51cbc997716a4dc471f36"}]}
