)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"305242575ede51aea517ff99722c1834e22ca94e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"195d45d0_591304da","updated":"2024-02-01 06:49:52.000000000","message":"Thanks Vincent!\n\nLooks good, just minor comments.","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1002139,"name":"Vincent Fazio","email":"vfazio@gmail.com","username":"vfazio"},"change_message_id":"e77c5c34f3b791739dade74d15dccd2311334265","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c5db2e16_a2d83f67","updated":"2024-02-01 18:28:18.000000000","message":"Thanks again for the quick review! I made some changes, please let me know if there is anything else you\u0027d like updated in this changeset.","commit_id":"1ff115e60ed926a2f9fce431cf16d075345e7d2b"},{"author":{"_account_id":1002139,"name":"Vincent Fazio","email":"vfazio@gmail.com","username":"vfazio"},"change_message_id":"bde99dd44a21c9cf09d8f92023eac0ac8c762d05","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6ec4de6e_8591735b","updated":"2024-02-01 21:06:42.000000000","message":"Added missing casts, thanks for catching those.","commit_id":"d754a5a96a35d8559f16c5b08399133dd2cd8d2f"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"cbe784bb4c01db162cf1c1fd9354ec289ce7210a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"479ecc4c_189a58c8","updated":"2024-02-01 21:48:12.000000000","message":"Thanks!","commit_id":"248d2efaea3a6478927e3a4fad2247960ab6aef9"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"0fa8f5a303d93e0f3ba0d0c157732a0edd15901a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"7b46d889_008ad2dc","updated":"2024-02-08 22:23:14.000000000","message":"Frankly speaking I would have put -2 to this patch if I ever had the opportunity to see it before.\n\nFirst of all, WHY this change? What are you trying to address or to improve? The commit message does not say anything about WHY!\n\nThis patch introduces additional casts to hack the print of the unsigned value UINT_MAX as the signed -1.\nSince long time I\u0027m trying to clean-up the OpenOCD code dropping most of cast and converting the format strings to the proper one matching the variable type.\nNext time that I will run again my scripts, they will trigger at the changes in this patch saying: an unsigned is first cast to signed to match the %d, suggest for removing the cast and use %u.\nThere is even not a single comment in the code that reports that there is this signed/unsigned hack, nor why the hack has been introduced.\n\nThen there is the extra change that makes the error message less clear. See below.\n\nThinking about the WHY, the only think that looks like an improvement in this patch is the content of the function is_gpio_config_valid(). But that function was there to hide the eventual complexity of the check. So also this is not a real improvement.\n\nWhile waiting for an explanation about WHY, I vote for a revert!","commit_id":"0d3d4c981ac77b600ce95c9ea6f1cdb280127342"},{"author":{"_account_id":1002139,"name":"Vincent Fazio","email":"vfazio@gmail.com","username":"vfazio"},"change_message_id":"6cfd44a7495b14c28b3cedcdc6c6ed891e4e42ef","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"33d9f5fa_d7261a5a","in_reply_to":"0e52668a_aa501a79","updated":"2024-02-25 00:11:02.000000000","message":"I can certainly modify all of the log message to drop this information, if we want people to only reference the initial output based on the configuration. \n\nI think the _why_ is two-fold:\n\n1. GPIO numbers should generally never be negative so the signed int type is, IMO, inappropriate for 99% of legitimate use cases. As the commit message stated, we were using the signed -1 value as a sentinel to show an unconfigured pin value. This has been updated to UINT_MAX, which has the same decimal representation and is, for all intents and purposes, unlikely to be a \"legitimate\" pin number, much like -1 was not a legitimate number. All we\u0027re doing is substituting the sentinel value for the new type.\n\n2. As Tomas alluded to, changing the type to unsigned int makes a difference in drivers that utilize the pin value for performing math, such as the bcm2835 driver. Performing math on an signed int can generate extra instructions to handle signed values, which we should never encounter because pins should only ever be positive unsigned ints.\n\nAn unsigned int division and modulo by 32, which is common in the driver, boils down to:\n\nunsigned div\n```\nlsr     w0, w0, 5\n```\n\nThe signed variant:\n\n```\n        add     w1, w0, 31\n        cmp     w0, 0\n        csel    w0, w1, w0, lt\n        asr     w0, w0, 5\n```\nand\n\nunsigned mod\n```\n        and     w0, w0, 31\n```\n\nsigned\n```\n        negs    w1, w0\n        and     w0, w0, 31\n        and     w1, w1, 31\n        csneg   w0, w0, w1, mi\n```\n\nI think this may have been a much deal before the code was updated to use inline functions and a pointer registry for toggling pins in the hot path (for bcm2835 at least) but still has benefits in other paths and other drivers that may need to perform math on pins get to reap the benefits as well. At some point, it may make sense to update the imx driver to use the adapter framework where this will also be leveraged. I or someone on my team may help with that port as we\u0027re migrating to an IMX chip over the course of the next few months.","commit_id":"0d3d4c981ac77b600ce95c9ea6f1cdb280127342"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"9283946a148025eb52d28b1593711df96cda5852","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"fc1dc802_526d0c09","in_reply_to":"3a846a81_b5192bca","updated":"2024-02-17 20:41:07.000000000","message":"\u003e The concern I have with the above helper function is that the `static char` array is going to be shared, so you can\u0027t do something like:\n\nYou\u0027re right, we missed this.\nConsidering it I think that solution could be one text helper with output of both pin and chip. It could pass the output text in a static variable.\n\n\u003e My intent is to make valuable contributions and I\u0027m sorry this patch was not up to standards.\n\nNo need for excuses, we appreciate your contribution","commit_id":"0d3d4c981ac77b600ce95c9ea6f1cdb280127342"},{"author":{"_account_id":1002139,"name":"Vincent Fazio","email":"vfazio@gmail.com","username":"vfazio"},"change_message_id":"c4d142fc92d0acc282d26a77fec3f4651cd50275","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3a846a81_b5192bca","in_reply_to":"52d99421_954c8b86","updated":"2024-02-12 18:10:02.000000000","message":"I\u0027m a bit busy at work for the next few weeks but I\u0027m willing to discuss and write up a patch as I have time.\n\nThe concern I have with the above helper function is that the `static char` array is going to be shared, so you can\u0027t do something like:\n\n```\n\tcommand_print(CMD, \"adapter gpio %s (%s): num %s, chip %s, active-%s%s%s%s\",\n\t\tgpio_map[gpio_idx].name, dir,\n\t\thelper_name(gpio_config-\u003egpio_num),\n\t\thelper_name(gpio_config-\u003echip_num),\n\t\tactive_state, drive, pull, init_state);\n```\n\nBecause that static buffer gets reused and will only return the value of the last call, in this case `helper_name(gpio_config-\u003egpio_num)` due to the order of argument evaluation.\n\nWe\u0027d need a static buffer for each invocation or at least a unique one per call. It sounds like we want to avoid `calloc/free` because it may be non-obvious to callers when to free memory, so one option is to instead require a buffer and length which can be stack allocated:\n\n\n```\nint helper_name(unsigned int value, char *ary, size_t len)\n{\n  int ret \u003d 0;\n  if (value \u003d\u003d ADAPTER_GPIO_NOT_SET)\n    ret \u003d snprintf(ary, len, \"%s\", \"not set\");\n  else\n    ret \u003d snprintf(ary, len, \"%u\", value);\n\n  return ret;\n}\n\n\nchar gpio_num[11] \u003d {};\nchar chip_num[11] \u003d {};\nint y \u003d 0;\ny \u003d helper_name(2, gpio_num, sizeof(gpio_num));\ny \u003d helper_name(ADAPTER_GPIO_NOT_SET, chip_num, sizeof(chip_num));\nprintf(\"%s %s\\n\", gpio_num, chip_num);\n\n```\n\nFor DEBUG callers, this could be gated by:\n\n```\n\tif (LOG_LEVEL_IS(LOG_LVL_DEBUG)) {\n\t\tchar gpio_num[11] \u003d {};\n\t\tchar chip_num[11] \u003d {};\n\t\thelper_name(adapter_gpio_config[idx].gpio_num, gpio_num, sizeof(gpio_num));\n\t\thelper_name(adapter_gpio_config[idx].chip_num, chip_num, sizeof(chip_num));\n\t\tLOG_DEBUG(\"saved GPIO mode for %s (GPIO %s %s): %s\",\n\t\t\t\tadapter_gpio_get_name(idx), chip_num, gpio_num,\n\t\t\t\tget_gpio_mode_name(initial_gpio_mode[idx]));\n\t}\n```\n\nOr, there\u0027s another member in `adapter_gpio_config` specifically for the string representation of the value and it gets set after parsing the number as part of `COMMAND_HANDLER(adapter_gpio_config_handler)` and it\u0027s only purpose for living is for output strings.\n\n\nIf i\u0027m misunderstanding the problem or the proposed solution, please let me know. \n\nMy intent is to make valuable contributions and I\u0027m sorry this patch was not up to standards.","commit_id":"0d3d4c981ac77b600ce95c9ea6f1cdb280127342"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"a1f6b408f9864468fe79e185745a2c02878fe52d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"52d99421_954c8b86","in_reply_to":"57089a7a_2705d0b1","updated":"2024-02-10 10:20:15.000000000","message":"Yes, the old good C and static variables. I thought about alloc_printf() however it would be memory leak prone. Your solution is better.\nBTW don\u0027t we need a similar helper in other parts? Combining a numeric value with a special one for undef/off is likely to be used quite often...\n\nVincent, could you please address these additional comments in a new patch?","commit_id":"0d3d4c981ac77b600ce95c9ea6f1cdb280127342"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"95ebfc965a54b92c93995c4dba7f4467be9854ae","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"c4b6d155_7547a0f1","in_reply_to":"7b46d889_008ad2dc","updated":"2024-02-09 08:21:33.000000000","message":"\u003e Frankly speaking I would have put -2 to this patch if I ever had the opportunity to see it before.\n\nPlease note that I put score +2 and then had been waiting for exactly one week before I merged the change. I think that 1 week is a good chill out period and I mostly comply with. And I\u0027d like other maintainers do so. Let\u0027s discuss it on the mail list if you want some change.\n\n\u003e This patch introduces additional casts to hack the print of the unsigned value UINT_MAX as the signed -1.\n\nUnfortunately it does. To be perfect we should introduce a print helper to print either %u or `not set` text or something similar. Showing -1 (regardless of it comes from -1 int or UINT_MAX) is readable for most of us but not the best. But we\u0027re speaking about a couple of LOG_DEBUG messages and yes, one command print. Is the helper worth of such narrow usage?\n\n\u003e Next time that I will run again my scripts, they will trigger at the changes in this patch saying: an unsigned is first cast to signed to match the %d, suggest for removing the cast and use %u.\n\nThere are thousand of casts in oocd code, many of them much much worse in terms of code quality. I\u0027m not defending them, I just think that anybody\u0027s script should not be considered as a rule.\n\n\u003e There is even not a single comment in the code that reports that there is this signed/unsigned hack, nor why the hack has been introduced.\n\nAgree. But each review is a compromise between what you want and what you really get...\n\n\u003e Thinking about the WHY, the only think that looks like an improvement in this patch is the content of the function is_gpio_config_valid().\n\nI see remarkable improvement in simplifying the machine code generated for time critical bitbang operations. Discussed in\nhttps://review.openocd.org/c/openocd/+/7732","commit_id":"0d3d4c981ac77b600ce95c9ea6f1cdb280127342"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"8c8618e5dc0ca5aa0d01d43819e7c6ee01cd5e48","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"0e52668a_aa501a79","in_reply_to":"8dc26f1a_9cd8a944","updated":"2024-02-24 23:06:05.000000000","message":"Quickly read 7732 and I don\u0027t see the WHY there.\n```gpio_num``` is known to be valid when passed as parameter to functions that can handle the conversion.\nSo my proposal for reverting this is still valid.\n\nRegarding this patch, there is actually no reason to print ```gpio_num``` and ```chip_num``` in the LOG_DEBUG() about toggling ```nrst``` and ```trst```.\nThe message is about the signals, the gpio should be already known or should be known by other means.\nThe message could be simplified as\n```\nLOG_DEBUG(\"trst %d, srst %d\", trst, srst);\n```\nremoving most of the ugly cast.\nLast cast is in the command_print() in src/jtag/adapter.c, and it should be handled properly.","commit_id":"0d3d4c981ac77b600ce95c9ea6f1cdb280127342"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"693a3e699611320bd375e31d2e394ac164c748eb","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"8dc26f1a_9cd8a944","in_reply_to":"aa9278f6_cc626cc3","updated":"2024-02-18 06:02:45.000000000","message":"You\u0027re right again. Moreover there would always be a danger that somebody will use 2 gpio_get_config_xx_as_str() calls in one LOG function in a new code without noticing the problem of static vars.\n\nAntonio, what if we return back to printing -1 and just hide (from checking script)/wrap the type cast into an inline function, like\n```\nstatic inline int adapter_gpio_pin_to_printable_num(const struct adapter_gpio_config cfg)\n{\n   /* a comment explaining why return signed */\n   return (int)cfg-\u003egpio_num;\n}\n```","commit_id":"0d3d4c981ac77b600ce95c9ea6f1cdb280127342"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"a92a6c20be56aa878732926de281b66aa01718f3","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"57089a7a_2705d0b1","in_reply_to":"c4b6d155_7547a0f1","updated":"2024-02-09 09:37:38.000000000","message":"I\u0027m complaining with myself, not with you. The amount of patches queuing in gerrit is by far above what we can handle.\nThe handler for \"not set\" would be the only improvement related to this patch. I have not checked 7732 yet.\nBut has some constraint on the allocation and free(). Maybe using a static container:\n```\nconst char *helper_name(unsigned int value)\n{\n  static returned_string[11]; /* to contain till UINT_MAX 4,294,967,295 */\n  if (value \u003d\u003d ADAPTER_GPIO_NOT_SET)\n    return \"not set\";\n  snprintf(returned_string, sizeof(returned_string), \"%u\", value);\n  return returned_string;\n}\n```\n\nAnd another point: ```ADAPTER_GPIO_NOT_SET``` is used for both gpio and chip.\nShould have been named ```ADAPTER_GPIO_OR_CHIP_NOT_SET```","commit_id":"0d3d4c981ac77b600ce95c9ea6f1cdb280127342"},{"author":{"_account_id":1002139,"name":"Vincent Fazio","email":"vfazio@gmail.com","username":"vfazio"},"change_message_id":"80b4bec663ae989e8a624473f4a7d131860bee19","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"aa9278f6_cc626cc3","in_reply_to":"fc1dc802_526d0c09","updated":"2024-02-17 23:53:05.000000000","message":"I\u0027m not exactly sure what that would look like, but open to trying it with a bit more guidance. Would this be some new struct we\u0027d define in adapter.h that contains two char [11] fields?\n\nIs there an advantage to this approach instead of just adding two new fields to `struct adapter_gpio_config` that we would just initialize as part of `COMMAND_HANDLER(adapter_gpio_config_handler)`? \n\nIt bloats the struct a bit, but:\n\n1. Allows the conversion to happen exactly once (as part of initialization) and not have to do multiple conversions for a value that should not change during an execution.\n2. Affords us the luxury of not having to restructure debug messages other than changing the struct member being referenced and the format string to `%s`.\n3. Doesn\u0027t require additional functions that consumers need to be made aware of.\n\nIt looks like the `adapter_config` struct is already statically allocated, so increasing the size of the `adapter_gpio_config` member to include these char arrays would just make that static allocation larger by 22 bytes.\n\nAlternatively, we could maybe do something like this in `adapter.c` (untested pseudo-code):\n\n```\nstatic int _chip_or_pin_val_to_str(unsigned int value, char ary[], size_t len)\n{\n\tint ret \u003d 0;\n\tif (value \u003d\u003d ADAPTER_GPIO_NOT_SET)\n\t\tret \u003d snprintf(ary, len, \"[not set]\");\n\telse\n\t\tret \u003d snprintf(ary, len, \"%u\", value);\n\treturn ret;\n}\n\nconst char *adapter_gpio_get_config_pin_as_str(struct adapter_gpio_config *config)\n{\n\tstatic char pin[11];\n\t_chip_or_pin_val_to_str(config-\u003egpio_num, pin, sizeof(pin));\n\treturn pin;\n}\n\nconst char *adapter_gpio_get_config_chip_as_str(struct adapter_gpio_config *config)\n{\n\tstatic char chip[11];\n\t_chip_or_pin_val_to_str(config-\u003echip_num, chip, sizeof(chip));\n\treturn chip;\n}\n```\n\nHowever, any static buffer that we use cannot be used successively as an argument to a function, so:\n\n```\n\tLOG_DEBUG(\"trst %d gpio: %d %d, srst %d gpio: %d %d\",\n\t\ttrst,\n\t\t(int)adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].chip_num,\n\t\t(int)adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].gpio_num,\n\t\tsrst,\n\t\t(int)adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].chip_num,\n\t\t(int)adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].gpio_num);\n```\n\nwould have to be restructured to:\n\n\n```\n\tLOG_DEBUG(\"trst %d gpio: %s %s\",\n\t\ttrst,\n\t\tadapter_gpio_get_config_chip_as_str(adapter_gpio_config);\n\t\tadapter_gpio_get_config_pin_as_str(adapter_gpio_config);\n\tLOG_DEBUG(\"srst %d gpio: %s %s\",\n\t\tsrst,\n\t\tadapter_gpio_get_config_chip_as_str(adapter_gpio_config);\n\t\tadapter_gpio_get_config_pin_as_str(adapter_gpio_config);\n```\n\nThis would be true even if a single helper returned both values in some static buffer.","commit_id":"0d3d4c981ac77b600ce95c9ea6f1cdb280127342"}],"src/jtag/adapter.c":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"305242575ede51aea517ff99722c1834e22ca94e","unresolved":true,"context_lines":[{"line_number":96,"context_line":"\tfor (int i \u003d 0; i \u003c ADAPTER_GPIO_IDX_NUM; ++i) {"},{"line_number":97,"context_line":"\t\t/* Use UINT_MAX as the sentinel \u0027unset\u0027 value."},{"line_number":98,"context_line":"\t\t * All adapters have an upper bound less than UINT_MAX. */"},{"line_number":99,"context_line":"\t\tadapter_config.gpios[i].gpio_num \u003d UINT_MAX;"},{"line_number":100,"context_line":"\t\tadapter_config.gpios[i].chip_num \u003d UINT_MAX;"},{"line_number":101,"context_line":"\t\tif (gpio_map[i].direction \u003d\u003d ADAPTER_GPIO_DIRECTION_INPUT)"},{"line_number":102,"context_line":"\t\t\tadapter_config.gpios[i].init_state \u003d ADAPTER_GPIO_INIT_STATE_INPUT;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"0ce0bafb_e1a297f1","line":99,"range":{"start_line":99,"start_character":37,"end_line":99,"end_character":45},"updated":"2024-02-01 06:49:52.000000000","message":"Please consider defining a macro like ADAPTER_GPIO_NOT_SET with UINT_MAX value in the header file. Not sure if it isn\u0027t an overkill for 2 usages only, perhaps we might need to explicitly test for GPIO_NOT_SET in future.\nOkay with me if you decide to leave it as is.","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1002139,"name":"Vincent Fazio","email":"vfazio@gmail.com","username":"vfazio"},"change_message_id":"e77c5c34f3b791739dade74d15dccd2311334265","unresolved":true,"context_lines":[{"line_number":96,"context_line":"\tfor (int i \u003d 0; i \u003c ADAPTER_GPIO_IDX_NUM; ++i) {"},{"line_number":97,"context_line":"\t\t/* Use UINT_MAX as the sentinel \u0027unset\u0027 value."},{"line_number":98,"context_line":"\t\t * All adapters have an upper bound less than UINT_MAX. */"},{"line_number":99,"context_line":"\t\tadapter_config.gpios[i].gpio_num \u003d UINT_MAX;"},{"line_number":100,"context_line":"\t\tadapter_config.gpios[i].chip_num \u003d UINT_MAX;"},{"line_number":101,"context_line":"\t\tif (gpio_map[i].direction \u003d\u003d ADAPTER_GPIO_DIRECTION_INPUT)"},{"line_number":102,"context_line":"\t\t\tadapter_config.gpios[i].init_state \u003d ADAPTER_GPIO_INIT_STATE_INPUT;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"10fecf7d_73b6545c","line":99,"range":{"start_line":99,"start_character":37,"end_line":99,"end_character":45},"in_reply_to":"0ce0bafb_e1a297f1","updated":"2024-02-01 18:28:18.000000000","message":"I\u0027ve defined this as ADAPTER_GPIO_NOT_SET in adapter.h","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"a58394901dfa22872282b2de460e7a4e8c70fbca","unresolved":false,"context_lines":[{"line_number":96,"context_line":"\tfor (int i \u003d 0; i \u003c ADAPTER_GPIO_IDX_NUM; ++i) {"},{"line_number":97,"context_line":"\t\t/* Use UINT_MAX as the sentinel \u0027unset\u0027 value."},{"line_number":98,"context_line":"\t\t * All adapters have an upper bound less than UINT_MAX. */"},{"line_number":99,"context_line":"\t\tadapter_config.gpios[i].gpio_num \u003d UINT_MAX;"},{"line_number":100,"context_line":"\t\tadapter_config.gpios[i].chip_num \u003d UINT_MAX;"},{"line_number":101,"context_line":"\t\tif (gpio_map[i].direction \u003d\u003d ADAPTER_GPIO_DIRECTION_INPUT)"},{"line_number":102,"context_line":"\t\t\tadapter_config.gpios[i].init_state \u003d ADAPTER_GPIO_INIT_STATE_INPUT;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"f2659664_96b9c0e2","line":99,"range":{"start_line":99,"start_character":37,"end_line":99,"end_character":45},"in_reply_to":"10fecf7d_73b6545c","updated":"2024-02-01 18:57:03.000000000","message":"Done","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"305242575ede51aea517ff99722c1834e22ca94e","unresolved":true,"context_lines":[{"line_number":902,"context_line":"\t\t}"},{"line_number":903,"context_line":"\t}"},{"line_number":904,"context_line":""},{"line_number":905,"context_line":"\tcommand_print(CMD, \"adapter gpio %s (%s): num %u, chip %u, active-%s%s%s%s\","},{"line_number":906,"context_line":"\t\tgpio_map[gpio_idx].name, dir, gpio_config-\u003egpio_num, gpio_config-\u003echip_num, active_state,"},{"line_number":907,"context_line":"\t\tdrive, pull, init_state);"},{"line_number":908,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"4fb6731f_120d4613","line":905,"range":{"start_line":905,"start_character":48,"end_line":905,"end_character":58},"updated":"2024-02-01 06:49:52.000000000","message":"AFAIK the helper can be used on not defined gpios as well.\nNot sure if a message like\n`adapter gpio xx (xx): num 4294967295, chip 4294967295`\nis the best way how to tell that gpio is not set. Either use %d format and cast the parameter or better add a separate branch to handle unset gpios and chips (IMO in this case we don\u0027t need other info like direction etc...)","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1002139,"name":"Vincent Fazio","email":"vfazio@gmail.com","username":"vfazio"},"change_message_id":"e77c5c34f3b791739dade74d15dccd2311334265","unresolved":true,"context_lines":[{"line_number":902,"context_line":"\t\t}"},{"line_number":903,"context_line":"\t}"},{"line_number":904,"context_line":""},{"line_number":905,"context_line":"\tcommand_print(CMD, \"adapter gpio %s (%s): num %u, chip %u, active-%s%s%s%s\","},{"line_number":906,"context_line":"\t\tgpio_map[gpio_idx].name, dir, gpio_config-\u003egpio_num, gpio_config-\u003echip_num, active_state,"},{"line_number":907,"context_line":"\t\tdrive, pull, init_state);"},{"line_number":908,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"d80c64ad_63969950","line":905,"range":{"start_line":905,"start_character":48,"end_line":905,"end_character":58},"in_reply_to":"4fb6731f_120d4613","updated":"2024-02-01 18:28:18.000000000","message":"I decided to add an earlier branch for unconfigured pins. There are times when an unconfigured chip is OK (bcm2835) and we can still print the pin information. I changed the format specifiers back to %d however (explanation in commit message).","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"bb2ec1db8c77deadc9e0ee46a2ece6503aabf9af","unresolved":false,"context_lines":[{"line_number":902,"context_line":"\t\t}"},{"line_number":903,"context_line":"\t}"},{"line_number":904,"context_line":""},{"line_number":905,"context_line":"\tcommand_print(CMD, \"adapter gpio %s (%s): num %u, chip %u, active-%s%s%s%s\","},{"line_number":906,"context_line":"\t\tgpio_map[gpio_idx].name, dir, gpio_config-\u003egpio_num, gpio_config-\u003echip_num, active_state,"},{"line_number":907,"context_line":"\t\tdrive, pull, init_state);"},{"line_number":908,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"a16f25b0_0e53d208","line":905,"range":{"start_line":905,"start_character":48,"end_line":905,"end_character":58},"in_reply_to":"d80c64ad_63969950","updated":"2024-02-01 20:47:14.000000000","message":"Done","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"305242575ede51aea517ff99722c1834e22ca94e","unresolved":true,"context_lines":[{"line_number":944,"context_line":"\t\tLOG_DEBUG(\"Processing %s\", CMD_ARGV[i]);"},{"line_number":945,"context_line":""},{"line_number":946,"context_line":"\t\tif (isdigit(*CMD_ARGV[i])) {"},{"line_number":947,"context_line":"\t\t\tunsigned int gpio_num; /* Use a meaningful output parameter for more helpful error messages */"},{"line_number":948,"context_line":"\t\t\tCOMMAND_PARSE_NUMBER(uint, CMD_ARGV[i], gpio_num);"},{"line_number":949,"context_line":"\t\t\tgpio_config-\u003egpio_num \u003d gpio_num;"},{"line_number":950,"context_line":"\t\t\t++i;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"353b8350_9de45727","line":947,"range":{"start_line":947,"start_character":3,"end_line":947,"end_character":97},"updated":"2024-02-01 06:49:52.000000000","message":"I don\u0027t grasp what the original author mentioned by this comment neither why a temporary variable is used. Feel free to simplify to\n`COMMAND_PARSE_NUMBER(uint, CMD_ARGV[i], gpio_config-\u003egpio_num);`","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"a1f6b408f9864468fe79e185745a2c02878fe52d","unresolved":true,"context_lines":[{"line_number":944,"context_line":"\t\tLOG_DEBUG(\"Processing %s\", CMD_ARGV[i]);"},{"line_number":945,"context_line":""},{"line_number":946,"context_line":"\t\tif (isdigit(*CMD_ARGV[i])) {"},{"line_number":947,"context_line":"\t\t\tunsigned int gpio_num; /* Use a meaningful output parameter for more helpful error messages */"},{"line_number":948,"context_line":"\t\t\tCOMMAND_PARSE_NUMBER(uint, CMD_ARGV[i], gpio_num);"},{"line_number":949,"context_line":"\t\t\tgpio_config-\u003egpio_num \u003d gpio_num;"},{"line_number":950,"context_line":"\t\t\t++i;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"283de057_e2ab87de","line":947,"range":{"start_line":947,"start_character":3,"end_line":947,"end_character":97},"in_reply_to":"0345b69b_dee68d57","updated":"2024-02-10 10:20:15.000000000","message":"Huh, even in the new code like esp32 and xtensa...\n\nI remember I knew about the hidden return in COMMAND_PARSE_... some time ago. I admit it slipped from my mind.\n\nWell, it\u0027s good to refresh the info time to time and keep to be aware. I agree that we should live with this and other crap code for some time...","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"0fa8f5a303d93e0f3ba0d0c157732a0edd15901a","unresolved":true,"context_lines":[{"line_number":944,"context_line":"\t\tLOG_DEBUG(\"Processing %s\", CMD_ARGV[i]);"},{"line_number":945,"context_line":""},{"line_number":946,"context_line":"\t\tif (isdigit(*CMD_ARGV[i])) {"},{"line_number":947,"context_line":"\t\t\tunsigned int gpio_num; /* Use a meaningful output parameter for more helpful error messages */"},{"line_number":948,"context_line":"\t\t\tCOMMAND_PARSE_NUMBER(uint, CMD_ARGV[i], gpio_num);"},{"line_number":949,"context_line":"\t\t\tgpio_config-\u003egpio_num \u003d gpio_num;"},{"line_number":950,"context_line":"\t\t\t++i;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"9159a7c6_7be0e18a","line":947,"range":{"start_line":947,"start_character":3,"end_line":947,"end_character":97},"in_reply_to":"138d7e88_a198da80","updated":"2024-02-08 22:23:14.000000000","message":"The helper COMMAND_PARSE_NUMBER prints an error message when the value in CMD_ARGV[i] does not match the type uint. The error message reports the output variable.\nBefore it was:\n\"gpio_num option value (\u0027%s\u0027) is not valid\"\nnow it\u0027s been changed to:\n\"gpio_config-\u003egpio_num option value (\u0027%s\u0027) is not valid\"\nwhich can be interesting for a SW developer of OpenOCD, but has much less sense for a user.\nShould this part be reverted?","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"8c8618e5dc0ca5aa0d01d43819e7c6ee01cd5e48","unresolved":true,"context_lines":[{"line_number":944,"context_line":"\t\tLOG_DEBUG(\"Processing %s\", CMD_ARGV[i]);"},{"line_number":945,"context_line":""},{"line_number":946,"context_line":"\t\tif (isdigit(*CMD_ARGV[i])) {"},{"line_number":947,"context_line":"\t\t\tunsigned int gpio_num; /* Use a meaningful output parameter for more helpful error messages */"},{"line_number":948,"context_line":"\t\t\tCOMMAND_PARSE_NUMBER(uint, CMD_ARGV[i], gpio_num);"},{"line_number":949,"context_line":"\t\t\tgpio_config-\u003egpio_num \u003d gpio_num;"},{"line_number":950,"context_line":"\t\t\t++i;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"42a23f9c_438f6b68","line":947,"range":{"start_line":947,"start_character":3,"end_line":947,"end_character":97},"in_reply_to":"283de057_e2ab87de","updated":"2024-02-24 23:06:05.000000000","message":"Yep!\nwhat about adding in the coding style that NEW macros should not invoke return?\nThen, considering some action to cleanup the existing code?","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1002139,"name":"Vincent Fazio","email":"vfazio@gmail.com","username":"vfazio"},"change_message_id":"e77c5c34f3b791739dade74d15dccd2311334265","unresolved":true,"context_lines":[{"line_number":944,"context_line":"\t\tLOG_DEBUG(\"Processing %s\", CMD_ARGV[i]);"},{"line_number":945,"context_line":""},{"line_number":946,"context_line":"\t\tif (isdigit(*CMD_ARGV[i])) {"},{"line_number":947,"context_line":"\t\t\tunsigned int gpio_num; /* Use a meaningful output parameter for more helpful error messages */"},{"line_number":948,"context_line":"\t\t\tCOMMAND_PARSE_NUMBER(uint, CMD_ARGV[i], gpio_num);"},{"line_number":949,"context_line":"\t\t\tgpio_config-\u003egpio_num \u003d gpio_num;"},{"line_number":950,"context_line":"\t\t\t++i;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"6e8eca4f_e04324c6","line":947,"range":{"start_line":947,"start_character":3,"end_line":947,"end_character":97},"in_reply_to":"353b8350_9de45727","updated":"2024-02-01 18:28:18.000000000","message":"Fixed","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"a58394901dfa22872282b2de460e7a4e8c70fbca","unresolved":false,"context_lines":[{"line_number":944,"context_line":"\t\tLOG_DEBUG(\"Processing %s\", CMD_ARGV[i]);"},{"line_number":945,"context_line":""},{"line_number":946,"context_line":"\t\tif (isdigit(*CMD_ARGV[i])) {"},{"line_number":947,"context_line":"\t\t\tunsigned int gpio_num; /* Use a meaningful output parameter for more helpful error messages */"},{"line_number":948,"context_line":"\t\t\tCOMMAND_PARSE_NUMBER(uint, CMD_ARGV[i], gpio_num);"},{"line_number":949,"context_line":"\t\t\tgpio_config-\u003egpio_num \u003d gpio_num;"},{"line_number":950,"context_line":"\t\t\t++i;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"138d7e88_a198da80","line":947,"range":{"start_line":947,"start_character":3,"end_line":947,"end_character":97},"in_reply_to":"6e8eca4f_e04324c6","updated":"2024-02-01 18:57:03.000000000","message":"Done","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"95ebfc965a54b92c93995c4dba7f4467be9854ae","unresolved":true,"context_lines":[{"line_number":944,"context_line":"\t\tLOG_DEBUG(\"Processing %s\", CMD_ARGV[i]);"},{"line_number":945,"context_line":""},{"line_number":946,"context_line":"\t\tif (isdigit(*CMD_ARGV[i])) {"},{"line_number":947,"context_line":"\t\t\tunsigned int gpio_num; /* Use a meaningful output parameter for more helpful error messages */"},{"line_number":948,"context_line":"\t\t\tCOMMAND_PARSE_NUMBER(uint, CMD_ARGV[i], gpio_num);"},{"line_number":949,"context_line":"\t\t\tgpio_config-\u003egpio_num \u003d gpio_num;"},{"line_number":950,"context_line":"\t\t\t++i;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"92590fb2_ff82d239","line":947,"range":{"start_line":947,"start_character":3,"end_line":947,"end_character":97},"in_reply_to":"9159a7c6_7be0e18a","updated":"2024-02-09 08:21:33.000000000","message":"Agree. But please improve the comment wording, with emphasize to use of stringify on the variable name in the helper macro - TBH I didn\u0027t grasp what it\u0027s about and thought it\u0027s one of invalid relict comments. I didn\u0027t use blame to find where it came from. My bad, sorry.\n\nBTW COMMAND_PARSE_NUMBER() macro is totally awful at all - the conditional return inside, nice crap...","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"a92a6c20be56aa878732926de281b66aa01718f3","unresolved":true,"context_lines":[{"line_number":944,"context_line":"\t\tLOG_DEBUG(\"Processing %s\", CMD_ARGV[i]);"},{"line_number":945,"context_line":""},{"line_number":946,"context_line":"\t\tif (isdigit(*CMD_ARGV[i])) {"},{"line_number":947,"context_line":"\t\t\tunsigned int gpio_num; /* Use a meaningful output parameter for more helpful error messages */"},{"line_number":948,"context_line":"\t\t\tCOMMAND_PARSE_NUMBER(uint, CMD_ARGV[i], gpio_num);"},{"line_number":949,"context_line":"\t\t\tgpio_config-\u003egpio_num \u003d gpio_num;"},{"line_number":950,"context_line":"\t\t\t++i;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"0345b69b_dee68d57","line":947,"range":{"start_line":947,"start_character":3,"end_line":947,"end_character":97},"in_reply_to":"92590fb2_ff82d239","updated":"2024-02-09 09:37:38.000000000","message":"Agree that COMMAND_PARSE_xxx() macro is crap.\nSince it\u0027s used 530 times in OpenOCD, I have always been reluctant to address it.\nKnowing its behavior, I\u0027m using it carefully in my conversion from .jim_handler to .handler, but it\u0027s not that straightforward.\n\nThere are other macros that return directly.\nWould you agree adding in the coding style that macros should not force a return?\n\nThis is the full list of such macros, plus these are used in other macros.\nIt would take some time to rework all of them, but at least we can prevent adding new ones:\n```\nsrc/flash/nand/s3c24xx.h:40:#define CALL_S3C24XX_DEVICE_COMMAND(d, i)\nsrc/flash/nand/orion.c:28:#define CHECK_HALTED\nsrc/flash/nor/stmsmi.c:33:#define SMI_READ_REG(a)\nsrc/flash/nor/stmsmi.c:44:#define SMI_WRITE_REG(a, v)\nsrc/flash/nor/stmsmi.c:53:#define SMI_POLL_TFF(timeout)\nsrc/helper/command.h:442:#define COMMAND_PARSE_NUMBER(type, in, out)\nsrc/helper/command.h:467:#define COMMAND_PARSE_ADDITIONAL_NUMBER(type, argn, out, name_str)\nsrc/helper/command.h:503:#define COMMAND_PARSE_BOOL(in, out, on, off)\nsrc/helper/command.c:1274:#define DEFINE_PARSE_NUM_TYPE(name, type, func, min, max)\nsrc/helper/command.c:1303:#define DEFINE_PARSE_WRAPPER(name, type, min, max, functype, funcname)\nsrc/target/arc.h:246:#define CHECK_RETVAL(action)\nsrc/target/arm11.h:18:#define CHECK_RETVAL(action)\nsrc/target/arm11.c:1235:#define ARM11_BOOL_WRAPPER(name, print_name)\nsrc/target/espressif/esp32_apptrace.c:506:#define ESP32_APPTRACE_CMD_NUM_ARG_CHECK(_cmd_, _arg_, _start_, _end_)\nsrc/target/espressif/esp_xtensa.c:22:#define ESP_XTENSA_DBGSTUBS_UPDATE_DATA_ENTRY(_e_)\nsrc/target/espressif/esp_xtensa.c:31:#define ESP_XTENSA_DBGSTUBS_UPDATE_CODE_ENTRY(_e_)\nsrc/target/dsp5680xx.c:22:#define err_check(r, c, m)\nsrc/target/dsp5680xx.c:23:#define err_check_propagate(retval)\n```","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"305242575ede51aea517ff99722c1834e22ca94e","unresolved":true,"context_lines":[{"line_number":957,"context_line":"\t\t\t\treturn ERROR_FAIL;"},{"line_number":958,"context_line":"\t\t\t}"},{"line_number":959,"context_line":"\t\t\tLOG_DEBUG(\"-chip arg is %s\", CMD_ARGV[i + 1]);"},{"line_number":960,"context_line":"\t\t\tunsigned int chip_num; /* Use a meaningful output parameter for more helpful error messages */"},{"line_number":961,"context_line":"\t\t\tCOMMAND_PARSE_NUMBER(uint, CMD_ARGV[i + 1], chip_num);"},{"line_number":962,"context_line":"\t\t\tgpio_config-\u003echip_num \u003d chip_num;"},{"line_number":963,"context_line":"\t\t\ti +\u003d 2;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"54d4e55b_c3e06640","line":960,"range":{"start_line":960,"start_character":3,"end_line":960,"end_character":97},"updated":"2024-02-01 06:49:52.000000000","message":"Same as above","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1002139,"name":"Vincent Fazio","email":"vfazio@gmail.com","username":"vfazio"},"change_message_id":"e77c5c34f3b791739dade74d15dccd2311334265","unresolved":true,"context_lines":[{"line_number":957,"context_line":"\t\t\t\treturn ERROR_FAIL;"},{"line_number":958,"context_line":"\t\t\t}"},{"line_number":959,"context_line":"\t\t\tLOG_DEBUG(\"-chip arg is %s\", CMD_ARGV[i + 1]);"},{"line_number":960,"context_line":"\t\t\tunsigned int chip_num; /* Use a meaningful output parameter for more helpful error messages */"},{"line_number":961,"context_line":"\t\t\tCOMMAND_PARSE_NUMBER(uint, CMD_ARGV[i + 1], chip_num);"},{"line_number":962,"context_line":"\t\t\tgpio_config-\u003echip_num \u003d chip_num;"},{"line_number":963,"context_line":"\t\t\ti +\u003d 2;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"cd3f1e78_67d252dd","line":960,"range":{"start_line":960,"start_character":3,"end_line":960,"end_character":97},"in_reply_to":"54d4e55b_c3e06640","updated":"2024-02-01 18:28:18.000000000","message":"Fixed","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"a58394901dfa22872282b2de460e7a4e8c70fbca","unresolved":false,"context_lines":[{"line_number":957,"context_line":"\t\t\t\treturn ERROR_FAIL;"},{"line_number":958,"context_line":"\t\t\t}"},{"line_number":959,"context_line":"\t\t\tLOG_DEBUG(\"-chip arg is %s\", CMD_ARGV[i + 1]);"},{"line_number":960,"context_line":"\t\t\tunsigned int chip_num; /* Use a meaningful output parameter for more helpful error messages */"},{"line_number":961,"context_line":"\t\t\tCOMMAND_PARSE_NUMBER(uint, CMD_ARGV[i + 1], chip_num);"},{"line_number":962,"context_line":"\t\t\tgpio_config-\u003echip_num \u003d chip_num;"},{"line_number":963,"context_line":"\t\t\ti +\u003d 2;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"8aa3c44a_f811c52c","line":960,"range":{"start_line":960,"start_character":3,"end_line":960,"end_character":97},"in_reply_to":"cd3f1e78_67d252dd","updated":"2024-02-01 18:57:03.000000000","message":"Done","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"a58394901dfa22872282b2de460e7a4e8c70fbca","unresolved":true,"context_lines":[{"line_number":907,"context_line":"\t}"},{"line_number":908,"context_line":""},{"line_number":909,"context_line":"\tcommand_print(CMD, \"adapter gpio %s (%s): num %d, chip %d, active-%s%s%s%s\","},{"line_number":910,"context_line":"\t\tgpio_map[gpio_idx].name, dir, gpio_config-\u003egpio_num, gpio_config-\u003echip_num, active_state,"},{"line_number":911,"context_line":"\t\tdrive, pull, init_state);"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"\treturn ERROR_OK;"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"931d596b_ad250e54","line":910,"range":{"start_line":910,"start_character":55,"end_line":910,"end_character":76},"updated":"2024-02-01 18:57:03.000000000","message":"Please cast chip_num to `int` to keep parameter type in sync with the format specifier or some compiler or lint will complain. Do it everywhere where %d is used.\nFor gpio_num here you may revert the specifier back to %u","commit_id":"1ff115e60ed926a2f9fce431cf16d075345e7d2b"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"bb2ec1db8c77deadc9e0ee46a2ece6503aabf9af","unresolved":false,"context_lines":[{"line_number":907,"context_line":"\t}"},{"line_number":908,"context_line":""},{"line_number":909,"context_line":"\tcommand_print(CMD, \"adapter gpio %s (%s): num %d, chip %d, active-%s%s%s%s\","},{"line_number":910,"context_line":"\t\tgpio_map[gpio_idx].name, dir, gpio_config-\u003egpio_num, gpio_config-\u003echip_num, active_state,"},{"line_number":911,"context_line":"\t\tdrive, pull, init_state);"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"\treturn ERROR_OK;"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"a6a90d35_29c7584b","line":910,"range":{"start_line":910,"start_character":55,"end_line":910,"end_character":76},"in_reply_to":"31ec8267_a5c472fa","updated":"2024-02-01 20:47:14.000000000","message":"Done","commit_id":"1ff115e60ed926a2f9fce431cf16d075345e7d2b"},{"author":{"_account_id":1002139,"name":"Vincent Fazio","email":"vfazio@gmail.com","username":"vfazio"},"change_message_id":"4225a029a75cb1fbdbf22c853b594bd43885ad39","unresolved":true,"context_lines":[{"line_number":907,"context_line":"\t}"},{"line_number":908,"context_line":""},{"line_number":909,"context_line":"\tcommand_print(CMD, \"adapter gpio %s (%s): num %d, chip %d, active-%s%s%s%s\","},{"line_number":910,"context_line":"\t\tgpio_map[gpio_idx].name, dir, gpio_config-\u003egpio_num, gpio_config-\u003echip_num, active_state,"},{"line_number":911,"context_line":"\t\tdrive, pull, init_state);"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"\treturn ERROR_OK;"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"31ec8267_a5c472fa","line":910,"range":{"start_line":910,"start_character":55,"end_line":910,"end_character":76},"in_reply_to":"931d596b_ad250e54","updated":"2024-02-01 19:08:41.000000000","message":"Fixed this by casting chip_num to int and using %u as the gpio_num format specifier","commit_id":"1ff115e60ed926a2f9fce431cf16d075345e7d2b"}],"src/jtag/drivers/am335xgpio.c":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"bb2ec1db8c77deadc9e0ee46a2ece6503aabf9af","unresolved":true,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":"\tLOG_DEBUG(\"trst %d gpio: %d %d, srst %d gpio: %d %d\","},{"line_number":251,"context_line":"\t\ttrst,"},{"line_number":252,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_TRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].gpio_num,"},{"line_number":253,"context_line":"\t\tsrst,"},{"line_number":254,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_SRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].gpio_num);"},{"line_number":255,"context_line":"\treturn ERROR_OK;"},{"line_number":256,"context_line":"}"},{"line_number":257,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":3,"id":"d8fc8364_2bfb37bd","line":254,"range":{"start_line":252,"start_character":2,"end_line":254,"end_character":108},"updated":"2024-02-01 20:47:14.000000000","message":"4x (int)","commit_id":"d754a5a96a35d8559f16c5b08399133dd2cd8d2f"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"cbe784bb4c01db162cf1c1fd9354ec289ce7210a","unresolved":false,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":"\tLOG_DEBUG(\"trst %d gpio: %d %d, srst %d gpio: %d %d\","},{"line_number":251,"context_line":"\t\ttrst,"},{"line_number":252,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_TRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].gpio_num,"},{"line_number":253,"context_line":"\t\tsrst,"},{"line_number":254,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_SRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].gpio_num);"},{"line_number":255,"context_line":"\treturn ERROR_OK;"},{"line_number":256,"context_line":"}"},{"line_number":257,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":3,"id":"53d9f115_3f7b8234","line":254,"range":{"start_line":252,"start_character":2,"end_line":254,"end_character":108},"in_reply_to":"2b79d88a_83490e75","updated":"2024-02-01 21:48:12.000000000","message":"Done","commit_id":"d754a5a96a35d8559f16c5b08399133dd2cd8d2f"},{"author":{"_account_id":1002139,"name":"Vincent Fazio","email":"vfazio@gmail.com","username":"vfazio"},"change_message_id":"bde99dd44a21c9cf09d8f92023eac0ac8c762d05","unresolved":true,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":"\tLOG_DEBUG(\"trst %d gpio: %d %d, srst %d gpio: %d %d\","},{"line_number":251,"context_line":"\t\ttrst,"},{"line_number":252,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_TRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].gpio_num,"},{"line_number":253,"context_line":"\t\tsrst,"},{"line_number":254,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_SRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].gpio_num);"},{"line_number":255,"context_line":"\treturn ERROR_OK;"},{"line_number":256,"context_line":"}"},{"line_number":257,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":3,"id":"2b79d88a_83490e75","line":254,"range":{"start_line":252,"start_character":2,"end_line":254,"end_character":108},"in_reply_to":"d8fc8364_2bfb37bd","updated":"2024-02-01 21:06:42.000000000","message":"Good catch, sorry for that.\n\nI had to reformat the lines a bit to fix checkpatch warnings.","commit_id":"d754a5a96a35d8559f16c5b08399133dd2cd8d2f"}],"src/jtag/drivers/bcm2835gpio.c":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"305242575ede51aea517ff99722c1834e22ca94e","unresolved":true,"context_lines":[{"line_number":240,"context_line":"\tif (is_gpio_config_valid(ADAPTER_GPIO_IDX_TRST))"},{"line_number":241,"context_line":"\t\tset_gpio_value(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_TRST], trst);"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"\tLOG_DEBUG(\"BCM2835 GPIO: %s(%d, %d), trst_gpio: %u %u, srst_gpio: %u %u\","},{"line_number":244,"context_line":"\t\t__func__, trst, srst,"},{"line_number":245,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_TRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].gpio_num,"},{"line_number":246,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_SRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].gpio_num);"},{"line_number":247,"context_line":"\treturn ERROR_OK;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"d82cd11d_9a758637","line":244,"range":{"start_line":243,"start_character":12,"end_line":244,"end_character":10},"updated":"2024-02-01 06:49:52.000000000","message":"Both resets are optional so we get 4294967295 for gpio not set. It\u0027s just a debug message, cast to int should be fine.\n\nYou replaced func name by \\__func__ - both are kind of over-verbosity as each message in the debug log contains func name. What about\n`LOG_DEBUG(\"trst %d gpio: %d %d, srst %d gpio: %d %d\", ...)`","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"bb2ec1db8c77deadc9e0ee46a2ece6503aabf9af","unresolved":false,"context_lines":[{"line_number":240,"context_line":"\tif (is_gpio_config_valid(ADAPTER_GPIO_IDX_TRST))"},{"line_number":241,"context_line":"\t\tset_gpio_value(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_TRST], trst);"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"\tLOG_DEBUG(\"BCM2835 GPIO: %s(%d, %d), trst_gpio: %u %u, srst_gpio: %u %u\","},{"line_number":244,"context_line":"\t\t__func__, trst, srst,"},{"line_number":245,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_TRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].gpio_num,"},{"line_number":246,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_SRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].gpio_num);"},{"line_number":247,"context_line":"\treturn ERROR_OK;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c4eeca57_45aa53f7","line":244,"range":{"start_line":243,"start_character":12,"end_line":244,"end_character":10},"in_reply_to":"bdccd0e4_6adf41be","updated":"2024-02-01 20:47:14.000000000","message":"Done","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1002139,"name":"Vincent Fazio","email":"vfazio@gmail.com","username":"vfazio"},"change_message_id":"e77c5c34f3b791739dade74d15dccd2311334265","unresolved":true,"context_lines":[{"line_number":240,"context_line":"\tif (is_gpio_config_valid(ADAPTER_GPIO_IDX_TRST))"},{"line_number":241,"context_line":"\t\tset_gpio_value(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_TRST], trst);"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"\tLOG_DEBUG(\"BCM2835 GPIO: %s(%d, %d), trst_gpio: %u %u, srst_gpio: %u %u\","},{"line_number":244,"context_line":"\t\t__func__, trst, srst,"},{"line_number":245,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_TRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].gpio_num,"},{"line_number":246,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_SRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].gpio_num);"},{"line_number":247,"context_line":"\treturn ERROR_OK;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"bdccd0e4_6adf41be","line":244,"range":{"start_line":243,"start_character":12,"end_line":244,"end_character":10},"in_reply_to":"d82cd11d_9a758637","updated":"2024-02-01 18:28:18.000000000","message":"Used this format and made this consistent between bcm2835 and am335x","commit_id":"206326e2fa6f94396432415976b55f384044df4c"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"bb2ec1db8c77deadc9e0ee46a2ece6503aabf9af","unresolved":true,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"\tLOG_DEBUG(\"trst %d gpio: %d %d, srst %d gpio: %d %d\","},{"line_number":244,"context_line":"\t\ttrst,"},{"line_number":245,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_TRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].gpio_num,"},{"line_number":246,"context_line":"\t\tsrst,"},{"line_number":247,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_SRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].gpio_num);"},{"line_number":248,"context_line":"\treturn ERROR_OK;"},{"line_number":249,"context_line":"}"},{"line_number":250,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":3,"id":"ed7bdb41_2857bf9e","line":247,"range":{"start_line":245,"start_character":2,"end_line":247,"end_character":108},"updated":"2024-02-01 20:47:14.000000000","message":"Not guarded by is_gpio_config_valid, (int) 4 times please","commit_id":"d754a5a96a35d8559f16c5b08399133dd2cd8d2f"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"cbe784bb4c01db162cf1c1fd9354ec289ce7210a","unresolved":false,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"\tLOG_DEBUG(\"trst %d gpio: %d %d, srst %d gpio: %d %d\","},{"line_number":244,"context_line":"\t\ttrst,"},{"line_number":245,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_TRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].gpio_num,"},{"line_number":246,"context_line":"\t\tsrst,"},{"line_number":247,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_SRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].gpio_num);"},{"line_number":248,"context_line":"\treturn ERROR_OK;"},{"line_number":249,"context_line":"}"},{"line_number":250,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":3,"id":"aa3360f8_1b7d25ea","line":247,"range":{"start_line":245,"start_character":2,"end_line":247,"end_character":108},"in_reply_to":"1f86c81b_5da6d58f","updated":"2024-02-01 21:48:12.000000000","message":"Done","commit_id":"d754a5a96a35d8559f16c5b08399133dd2cd8d2f"},{"author":{"_account_id":1002139,"name":"Vincent Fazio","email":"vfazio@gmail.com","username":"vfazio"},"change_message_id":"bde99dd44a21c9cf09d8f92023eac0ac8c762d05","unresolved":true,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"\tLOG_DEBUG(\"trst %d gpio: %d %d, srst %d gpio: %d %d\","},{"line_number":244,"context_line":"\t\ttrst,"},{"line_number":245,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_TRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].gpio_num,"},{"line_number":246,"context_line":"\t\tsrst,"},{"line_number":247,"context_line":"\t\tadapter_gpio_config[ADAPTER_GPIO_IDX_SRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].gpio_num);"},{"line_number":248,"context_line":"\treturn ERROR_OK;"},{"line_number":249,"context_line":"}"},{"line_number":250,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":3,"id":"1f86c81b_5da6d58f","line":247,"range":{"start_line":245,"start_character":2,"end_line":247,"end_character":108},"in_reply_to":"ed7bdb41_2857bf9e","updated":"2024-02-01 21:06:42.000000000","message":"Good catch, sorry for that.\n\nI had to reformat the lines a bit to fix checkpatch warnings.","commit_id":"d754a5a96a35d8559f16c5b08399133dd2cd8d2f"}]}