)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"dd1a0db8444e47ddfd6c7b96f213a4edabc6b965","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e60b8962_f49630cf","updated":"2025-03-23 16:16:03.000000000","message":"Another point.\nFor `read-buffer` and `write-buffer` I choose to format the data as a single long string of hex values, e.g.:\n```\n$target_name read_buffer $address 16\n00112233445566778899aabbccddeeff\n```\nThis to reduce the overhead of handling multiple arguments, one per byte or per word, and of stripping `0x` in front of each argument.\nThis is also the data format exchanged on the GDB remote protocol, of course with header and trailer. It should be easy to move this code in `gdb_server.c`.\nWhile it\u0027s probably too odd and not compatible with the other OpenOCD commands, I think that due to this specific use case we can ignore the compatibility and stick with it.","commit_id":"58969f71c7f2792a6972d7e550bcbc19a464d828"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"d1d68a1829529f35bd6132a4621fd0b5cae17ded","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"45407692_1cc2985e","updated":"2025-03-30 13:05:22.000000000","message":"Different types of events introduces inconsistency and confusion for a little benefit of only having the new \"read-buffer\" / \"write-buffer\" events. Looking to the future, especially that we are moving towards version 1.0, wouldn\u0027t it make sense to rebuild the events in general so that we can pass parameters - especially the target that generates the event? For backward compability we could check if the passed \"body\" is a proc and if not raise a deprecation warning. What do you think?","commit_id":"58969f71c7f2792a6972d7e550bcbc19a464d828"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"7da409abac15407bcdcd1c44be8f5630d51ce9a2","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"04cf4d8b_aadae367","updated":"2025-03-23 16:05:04.000000000","message":"This is actually a POC, probably not to be merged as is.\nWhile it addresses some odd GDB request, it is mainly supposed to help reworking 7149.\n\nBut 7149 overrides `.write_memory`, not `.write_buffer`. Is it because most of the targets use the default `target_write_buffer_default()` ? But it\u0027s not the case for `cortex_a` and `esp32`!!!\n\nAlso, `target_write_buffer()` and `target_read_buffer()` are used in other contexts, outside GDB server code. Probably this patch should override inside `gdb_server.c` only and avoid breaking other use cases.\n\nAlso the way to set these new events is odd:\n```\nproc new_read_buffer {address count} {... proc body ...}\n$target_name -event read-buffer new_read_buffer\n```\nI choose this way to be less invasive as possible. It would have been more readable adding the parameters while setting the event, e.g.:\n```\n$target_name -event read-buffer {address count} {... event body ...}\n```\nand, if the procedure is preferred, writing:\n```\n$target_name -event read-buffer {address count} {new_read_buffer $address $count}\n```\n\nYour comments are welcome.\nNext patch in this series is just an example on how to use it. I don\u0027t plan to merge it.","commit_id":"58969f71c7f2792a6972d7e550bcbc19a464d828"},{"author":{"_account_id":1001872,"name":"Jacek Wuwer","email":"jacekmw8@gmail.com","username":"jacekmw8"},"change_message_id":"24d47110730b43b5998f0f4aca0dc6a870fda375","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3e6d6db9_77f8e652","updated":"2025-04-09 18:04:54.000000000","message":"hi Antonio,\nNot sure if these events can be used for the purpose the patch 7149 has been implemented. Pls see my elaboration below.\nJacek","commit_id":"58969f71c7f2792a6972d7e550bcbc19a464d828"},{"author":{"_account_id":1001872,"name":"Jacek Wuwer","email":"jacekmw8@gmail.com","username":"jacekmw8"},"change_message_id":"24d47110730b43b5998f0f4aca0dc6a870fda375","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2f02d030_4057ef0f","in_reply_to":"04cf4d8b_aadae367","updated":"2025-04-09 18:04:54.000000000","message":"Thank you for the patch. I actually do not know if/how it can be useful for the patch 7149. The purpose of this patch is to access the memories directly when gdb needs it, avoiding costly front end access through serial JTAG, registers or even bus.\nThat is the reason why 7149 overrides the target lowest level write memory and not write_buffer, which by default (when not overridden by a specific target) is target_write_buffer(). And as you said, it is used on other, non gdb contexts.\n\nAnother problem I see is the way to handle this events at the TCL level without being able to pass the arguments properly. I need a way to intercept those GDB binary packets that are destined for any of the memories in the design, identified by the vdebug driver settings and write them directly instead of using target write_memory. The data itself, the address and size are in the GDB packet, so it would be cumbersome to say the least to pass it through TCL and back.\n\nAnother point is - 7149 now implements now a single event to \u0027register\u0027 the current target through the handler. The actual backdoor access happens (or not) whenever gdb needs it directly in the driver without generating potentially large number of events.","commit_id":"58969f71c7f2792a6972d7e550bcbc19a464d828"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"fb61cb14a9ca6848bf659e7e12763446cd2091cb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"150b830c_acac11eb","in_reply_to":"126aab5b_52e00655","updated":"2025-06-10 08:13:56.000000000","message":"Marc,\ntoo much activity for the 1.0.0-rc1, so I will work on this after 1.0.0.\nI will also give priority to the rework of jimtcl code, because the event\u0027s code rely on it and I don\u0027t want to do it twice.\n\nMy suggestion to rename \u0027events\u0027 as \u0027callbacks\u0027 is due to a clash with the topic \u0027event\u0027 in TCL language\nhttps://wiki.tcl-lang.org/page/event\nhttps://wiki.tcl-lang.org/page/Event+tutorial\nI find term \u0027callback\u0027 as more appropriate, but I\u0027m not going to enforce it.\n\nI\u0027m currently reconsidering the way to define the events, even if I will keep the backward compatibility for some time. I\u0027m checking using a TCL namespace per target, where an events is a proc with predefined name in that target namespace:\n```\nnamespace eval ::$_TARGETNAME {\n    proc event_examine { target } { ... }\n\n    # default events, no need for user to declared them\n    proc event_gdb_attach { target } { halt 1000 }\n    proc event_gdb_flash_erase_start { target } { reset init }\n    proc event_gdb_flash_write_end { target } { reset halt }\n}\n```\nMissing proc means no event assigned.\nOld way to add events through `target create` or `configure` simply generate the proc in the namespace, but could be deprecated. Events can be simply added as I wrote in the example above.\nCommand `eventlist` give a status of the target\u0027s namespace as today.\nI think this would make more readable the whole stuff.\n\nActually the parameter `target` can also be assigned through TCL default value for optional arguments:\n`proc event_examine { {target $_TARGETNAME} } { ... }`\nand no need to pass the parameter from the OpenOCD code, but for me this makes writing the script more difficult and error prone.","commit_id":"58969f71c7f2792a6972d7e550bcbc19a464d828"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"17691b176405dc6149e36e33a6c41cca0213f31f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b500bb68_c34c9f06","in_reply_to":"45407692_1cc2985e","updated":"2025-03-30 18:00:51.000000000","message":"Thanks for the review Marc.\n\n\u003e Different types of events introduces inconsistency and confusion for a little benefit of only having the new \"read-buffer\" / \"write-buffer\" events. Looking to the future, especially that we are moving towards version 1.0, wouldn\u0027t it make sense to rebuild the events in general so that we can pass parameters - especially the target that generates the event?\n\nThe target that generates the event is already set as \u0027current target\u0027 before entering in the event\u0027s code. No need to pass the target as parameter.\nBut events like `reset-deassert-pre` can benefit in readability by having the parameter `halt` explicitly listed.\n\nWhat about the proposal of having a proc-like syntax\n`$target_name -event event_name {parameter list} {event body}`\nwhere the two elements after `event_name` begin both with the character `{` ?\nIt would be easy to handle the backward compatibility with simple checks.\n\n\u003e For backward compatibility we could check if the passed \"body\" is a proc and if not raise a deprecation warning. What do you think?\n\nFor simple event\u0027s code I find overly complex to pass through a proc. Think about current default:\n`$target_name -event gdb-attach {halt 1000}`\nthat should be rewritten as:\n```\nproc default_gdb_attach {}\n{\n   halt 1000\n}\n$target_name -event gdb-attach default_gdb_attach\n```\nI prefer not forcing the use of procs.\n\nI\u0027m also thinking if we should still call them \u0027events\u0027 or renaming them as \u0027callbacks\u0027!","commit_id":"58969f71c7f2792a6972d7e550bcbc19a464d828"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"00afab0178a1e95b293513756fd00e5d9a642274","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"126aab5b_52e00655","in_reply_to":"b500bb68_c34c9f06","updated":"2025-06-09 19:55:04.000000000","message":"\u003e Thanks for the review Marc.\n\u003e \n\u003e \u003e Different types of events introduces inconsistency and confusion for a little benefit of only having the new \"read-buffer\" / \"write-buffer\" events. Looking to the future, especially that we are moving towards version 1.0, wouldn\u0027t it make sense to rebuild the events in general so that we can pass parameters - especially the target that generates the event?\n\u003e \n\u003e The target that generates the event is already set as \u0027current target\u0027 before entering in the event\u0027s code. No need to pass the target as parameter.\n\nThat\u0027s a good point. However, we will have many \u0027objects\u0027 in the future that will generate events and can have multiple instances. One example is RTT with multi-target support. Another example is ETM and other CoreSight components. For all these objects it would be very helpful to pass the event generating object as first argument. For consistency we could/should also pass this argument in target events.\n\n\u003e What about the proposal of having a proc-like syntax\n\u003e `$target_name -event event_name {parameter list} {event body}`\n\u003e where the two elements after `event_name` begin both with the character `{` ?\n\u003e It would be easy to handle the backward compatibility with simple checks.\n\nLooks like a reasonable approach.\n\n\u003e \u003e For backward compatibility we could check if the passed \"body\" is a proc and if not raise a deprecation warning. What do you think?\n\u003e \n\u003e For simple event\u0027s code I find overly complex to pass through a proc. Think about current default:\n\u003e `$target_name -event gdb-attach {halt 1000}`\n\u003e that should be rewritten as:\n\u003e ```\n\u003e proc default_gdb_attach {}\n\u003e {\n\u003e    halt 1000\n\u003e }\n\u003e $target_name -event gdb-attach default_gdb_attach\n\u003e ```\n\u003e I prefer not forcing the use of procs.\n\nYep, that\u0027s true. For simple events that\u0027s too much overhead.\n\n\u003e I\u0027m also thinking if we should still call them \u0027events\u0027 or renaming them as \u0027callbacks\u0027!\n\nPersonally, I like the name \u0027event\u0027.\n\nIs there any progress with the event handling Antonio?","commit_id":"58969f71c7f2792a6972d7e550bcbc19a464d828"},{"author":{"_account_id":1001872,"name":"Jacek Wuwer","email":"jacekmw8@gmail.com","username":"jacekmw8"},"change_message_id":"24d47110730b43b5998f0f4aca0dc6a870fda375","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8e8275f6_d37bd700","in_reply_to":"e60b8962_f49630cf","updated":"2025-04-09 18:04:54.000000000","message":"As I pointed out above, we do not have the data to write at the TCL level. At the point when write_buffer is called its already converted from GDB remote protocol to binary format.","commit_id":"58969f71c7f2792a6972d7e550bcbc19a464d828"}]}