)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"092e176406a41c26d7fa8e413c87cf3d8f1eba3e","unresolved":true,"context_lines":[{"line_number":21,"context_line":"QUESTIONS"},{"line_number":22,"context_line":"* Should all GPIOs have a drive mode? The \"reset_config\" allows it to be"},{"line_number":23,"context_line":"defined for SRST and TRST. How would conflicts between \"reset_config"},{"line_number":24,"context_line":"srst_open_drain\" and \"adapter gpio srst -open-drain\" be resolved?"},{"line_number":25,"context_line":"~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: I7c63c0e4763657ea51790c43fc40d32b7c3580bb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9d89c9fb_3048e4bc","line":24,"updated":"2022-05-18 14:39:28.000000000","message":"The settings with reset_config should be respected, as they are valid for every adapter, even those not GPIO based.\n\nTo avoid replicating the check in every GPIO based driver, better adding in https://review.openocd.org/6967/ the check that TRST and SRST cannot have mode.\nMaybe changing struct gpio_map to pass a set of flags instead or together with \u0027direction\u0027, e.g. GPIO_DRIVE_NO_MODE.\nThen each driver simply ignores the mode and takes the value from reset_config \"at run-time\"; in fact today is possible to change reset_config during the execution. Not sure this really makes sense but that\u0027s current code ...","commit_id":"9ad4a92d1276a912f4533581b34345f89f1e139e"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"88bc12a5719e4795422208f03c8e61add752b1af","unresolved":false,"context_lines":[{"line_number":21,"context_line":"QUESTIONS"},{"line_number":22,"context_line":"* Should all GPIOs have a drive mode? The \"reset_config\" allows it to be"},{"line_number":23,"context_line":"defined for SRST and TRST. How would conflicts between \"reset_config"},{"line_number":24,"context_line":"srst_open_drain\" and \"adapter gpio srst -open-drain\" be resolved?"},{"line_number":25,"context_line":"~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: I7c63c0e4763657ea51790c43fc40d32b7c3580bb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"86d5acbc_c9901e31","line":24,"in_reply_to":"9d89c9fb_3048e4bc","updated":"2022-05-20 12:44:39.000000000","message":"Done","commit_id":"9ad4a92d1276a912f4533581b34345f89f1e139e"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"88bc12a5719e4795422208f03c8e61add752b1af","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"39385fc5_747ff5e3","updated":"2022-05-20 12:44:39.000000000","message":"Thank you for your comments. Revised version to follow shortly.","commit_id":"9ad4a92d1276a912f4533581b34345f89f1e139e"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"092e176406a41c26d7fa8e413c87cf3d8f1eba3e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a10b33d9_d0caa31b","updated":"2022-05-18 14:39:28.000000000","message":"nice!","commit_id":"9ad4a92d1276a912f4533581b34345f89f1e139e"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"e514ba24d63d1f1eed18d9d492e11dbac1485f3b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3471c7e7_d2bc3868","updated":"2022-06-11 14:10:33.000000000","message":"Sorry for taking so long to return on this series","commit_id":"7bc340ed39a568e8b70f999d8db8d01ba0e35542"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"c1af8e5ef7a625186b3a56698c6bb4a79999a066","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d9bc9684_42d22b77","updated":"2022-06-12 15:10:54.000000000","message":"Thanks for reviewing. I hope this answers your question.","commit_id":"7bc340ed39a568e8b70f999d8db8d01ba0e35542"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"0a0de5ebc10fce8745a4294622bfef67cd12b94c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"949630fb_ce1d7ca8","updated":"2022-06-12 22:26:10.000000000","message":"Thanks for your review. Changes as requested, except for a query on user0.","commit_id":"f47ec41036dc58dbed69fcb31fa0f17fddbc4ded"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"cdc6f44922ad6db0302e5512e9096a013c1ebbca","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"92cda3da_35bb7c69","updated":"2022-06-12 22:29:39.000000000","message":"Thanks for your review. I think we need to resolve the meaning of \"active\".","commit_id":"f47ec41036dc58dbed69fcb31fa0f17fddbc4ded"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"ceea93b1e0018812ba8b032e6b23058539946d6a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a7f5d973_93b19aa6","updated":"2022-06-19 20:39:10.000000000","message":"Changes made as requested.\n\nI don\u0027t understand why this is marked as having a merge conflict when it is based on https://review.openocd.org/c/openocd/+/6967 (c32d3fc15c654220404c173838eeb9a43621da5f) which does not show a merge conflict. Is there anything I need to do?","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"9864d22d77e977d65f9d462678d7d7cca8648c38","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ef61c601_14b70652","updated":"2022-06-17 10:08:33.000000000","message":"For the latest patchset I tested SWD transport by identifying an nRF52832, and JTAG transport was tested by identifying an STM32F407. I measured SWCLK at 1MHz so no significant difference there. Before starting OpenOCD I set the srst GPIO into various states (input, output low, output high) and did not observe any glitches.","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"88fc1f09c50d63eb025ec0c9bd8483792d523f15","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"9bfaeec0_c4de9311","updated":"2022-06-17 10:07:07.000000000","message":"Thanks for your comments, I have just pushed an up[dated version that uses a local copy of the GPIO configuration to avoid unnecessary function calls inside the bit-banging functions. ","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"20c3fedd00d94f9524cc6bc3e40f5b73027e7b31","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"2ac1fdd2_80107e63","in_reply_to":"a7f5d973_93b19aa6","updated":"2022-06-20 13:41:06.000000000","message":"No problem, it just show that cannot be merged alone, but depends on the other patch in the series.","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"6323187bbb1b41b758cc563fc9e3b8d1bb394abf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"b7b19133_8d8d2a7f","updated":"2022-06-20 16:10:41.000000000","message":"Thank you for your meticulous reviewing. \n\nThere is one issue I\u0027ve noticed but not resolved here. The file descriptor for /dev/mem is not closed in the quit function. None of the other drivers that use /dev/mem close it in their quit functions either but all close it if they leave init() early due to an error condition. How should I proceed?\n","commit_id":"fde8cec05e5af073d0670a07d1b71f18aa592bae"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"f2912f76d83158f701a9f95ec19d6ea636e8ab98","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"8375622e_d45dc2a8","updated":"2022-06-20 22:13:33.000000000","message":"Thank you for your comments. Changes as requested.","commit_id":"07372e554462f8604d912ac379cee1e77aeed22a"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"0eb1cb20a458c8091fd2574cd0b72be13f4033e4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"b4122c4a_a0fa59a2","updated":"2022-06-20 20:33:36.000000000","message":"very last items, no reason to nit-picky more.\n\nWhat you think could be a next step?\nPort another driver to see if something new pops-out, then merge all? On my side I could only test linuxgpiod.\nHave you had opportunity to verify that the performance is not too much degradated?","commit_id":"07372e554462f8604d912ac379cee1e77aeed22a"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"f51686bb0aefe32f544ca0cb6dd3e9695e8749e0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"93d2ece2_a35a9a88","in_reply_to":"5b028039_2111a079","updated":"2022-06-21 07:27:58.000000000","message":"Yes, I read your comment on closing /dev/mem, then I forgot it. Age is killing neurons!\nIn OpenOCD we are supposed to close all the files and de-allocate any memory before quitting. So, yes, /dev/mem should be closed. But maybe in a separate patch.\n\nAgree, bcm2835gpio and am335xgpio are too similar, I don\u0027t expect any problem to pop-out from this. linuxgpiod or even the old sysfsgpio could be better reference.\nBut if you want to anticipate bcm2835gpio because it\u0027s useful to you, no problem. There could be some minor change/fix, if any, that would be applied there too!","commit_id":"07372e554462f8604d912ac379cee1e77aeed22a"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"f2912f76d83158f701a9f95ec19d6ea636e8ab98","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"5b028039_2111a079","in_reply_to":"b4122c4a_a0fa59a2","updated":"2022-06-20 22:13:33.000000000","message":"Maybe next should be closing the /dev/mem file descriptor in the quit function? Did you see my query about that?\n\nI think that it makes sense that linuxgpiod is the next driver ported. It is a good reference implementation and probably runs on all of the architectures where local GPIO bit-banging is implemented. If you want another driver porting before merging that is better IMO than bcm2835gpio which is too similar to am335xgpio.\n\nI\u0027d also like to see the bcm2835 driver updated as that one is useful to me. That is the time to add the activity LED that I want.\n\nI\u0027ve tested the speed again and I see no noticeable decrease for am335xgpio. That might not hold true for other drivers (eg bcm2835) - I guess it depends on the ratio of CPU clock to IO clock and the capacity to add the extra work for handling the drive mode and active-low additions.","commit_id":"07372e554462f8604d912ac379cee1e77aeed22a"}],"src/jtag/drivers/am335xgpio.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"092e176406a41c26d7fa8e413c87cf3d8f1eba3e","unresolved":true,"context_lines":[{"line_number":116,"context_line":"static int speed_offset \u003d 575;"},{"line_number":117,"context_line":"static unsigned int jtag_delay;"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"static void set_gpio_value_NEW(const struct adapter_gpio_config *gpio_config, int value);"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"static int is_gpio_valid(int gpio_num)"},{"line_number":122,"context_line":"{"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"58edadfd_076135e1","line":119,"updated":"2022-05-18 14:39:28.000000000","message":"reorder the functions so no need for this declaration","commit_id":"9ad4a92d1276a912f4533581b34345f89f1e139e"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"88bc12a5719e4795422208f03c8e61add752b1af","unresolved":false,"context_lines":[{"line_number":116,"context_line":"static int speed_offset \u003d 575;"},{"line_number":117,"context_line":"static unsigned int jtag_delay;"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"static void set_gpio_value_NEW(const struct adapter_gpio_config *gpio_config, int value);"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"static int is_gpio_valid(int gpio_num)"},{"line_number":122,"context_line":"{"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"df7e49ee_a3be59aa","line":119,"in_reply_to":"58edadfd_076135e1","updated":"2022-05-20 12:44:39.000000000","message":"Done","commit_id":"9ad4a92d1276a912f4533581b34345f89f1e139e"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"e514ba24d63d1f1eed18d9d492e11dbac1485f3b","unresolved":true,"context_lines":[{"line_number":183,"context_line":"\t\t\t\u0026\u0026 gpio_config-\u003einit_state !\u003d ADAPTER_GPIO_INIT_STATE_INPUT) {"},{"line_number":184,"context_line":"\t\t/* The GPIO is an output, and needs to be an output. There is a risk"},{"line_number":185,"context_line":"\t\t* that changing the output value might connect two outputs together."},{"line_number":186,"context_line":"\t\t*/"},{"line_number":187,"context_line":"\t\tint current_value \u003d initial_gpio_mode[idx] \u003d\u003d AM335XGPIO_GPIO_MODE_OUTPUT_HIGH ? 1 : 0;"},{"line_number":188,"context_line":"\t\tint new_value \u003d (gpio_config-\u003einit_state \u003d\u003d ADAPTER_GPIO_INIT_STATE_ACTIVE) ^ gpio_config-\u003eactive_low ? 1 : 0;"},{"line_number":189,"context_line":"\t\tif (new_value !\u003d current_value)"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"ea1f706e_1ae35eb6","line":186,"updated":"2022-06-11 14:10:33.000000000","message":"I don\u0027t understand what is the use case, here. Can you please explain better?","commit_id":"7bc340ed39a568e8b70f999d8db8d01ba0e35542"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"88fc1f09c50d63eb025ec0c9bd8483792d523f15","unresolved":false,"context_lines":[{"line_number":183,"context_line":"\t\t\t\u0026\u0026 gpio_config-\u003einit_state !\u003d ADAPTER_GPIO_INIT_STATE_INPUT) {"},{"line_number":184,"context_line":"\t\t/* The GPIO is an output, and needs to be an output. There is a risk"},{"line_number":185,"context_line":"\t\t* that changing the output value might connect two outputs together."},{"line_number":186,"context_line":"\t\t*/"},{"line_number":187,"context_line":"\t\tint current_value \u003d initial_gpio_mode[idx] \u003d\u003d AM335XGPIO_GPIO_MODE_OUTPUT_HIGH ? 1 : 0;"},{"line_number":188,"context_line":"\t\tint new_value \u003d (gpio_config-\u003einit_state \u003d\u003d ADAPTER_GPIO_INIT_STATE_ACTIVE) ^ gpio_config-\u003eactive_low ? 1 : 0;"},{"line_number":189,"context_line":"\t\tif (new_value !\u003d current_value)"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"7d7610e3_c0033473","line":186,"in_reply_to":"0c597c1e_0b753bb8","updated":"2022-06-17 10:07:07.000000000","message":"That\u0027s good to know.","commit_id":"7bc340ed39a568e8b70f999d8db8d01ba0e35542"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"762a1dde40b4a05e6714a5efe128fc43e0dde706","unresolved":true,"context_lines":[{"line_number":183,"context_line":"\t\t\t\u0026\u0026 gpio_config-\u003einit_state !\u003d ADAPTER_GPIO_INIT_STATE_INPUT) {"},{"line_number":184,"context_line":"\t\t/* The GPIO is an output, and needs to be an output. There is a risk"},{"line_number":185,"context_line":"\t\t* that changing the output value might connect two outputs together."},{"line_number":186,"context_line":"\t\t*/"},{"line_number":187,"context_line":"\t\tint current_value \u003d initial_gpio_mode[idx] \u003d\u003d AM335XGPIO_GPIO_MODE_OUTPUT_HIGH ? 1 : 0;"},{"line_number":188,"context_line":"\t\tint new_value \u003d (gpio_config-\u003einit_state \u003d\u003d ADAPTER_GPIO_INIT_STATE_ACTIVE) ^ gpio_config-\u003eactive_low ? 1 : 0;"},{"line_number":189,"context_line":"\t\tif (new_value !\u003d current_value)"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"0c597c1e_0b753bb8","line":186,"in_reply_to":"6f34b7ff_cdad153e","updated":"2022-06-12 18:58:09.000000000","message":"Let me first answer that OpenOCD connects to target without resetting it.\nIf you \"need\" to keep the target in reset during OpenOCD connection, you can use the command \u0027reset_config connect_assert_srst\".\nSome target adds a \"reset\" or a \"reset halt\" in the event gdb-attach, but those are special cases.\n\nThen, let\u0027s return to my question. I understand the purpose, but I don\u0027t see which transition during OpenOCD initialization can create issues.\nBut I see below something strange that is maybe the reason you are setting up this...","commit_id":"7bc340ed39a568e8b70f999d8db8d01ba0e35542"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"c1af8e5ef7a625186b3a56698c6bb4a79999a066","unresolved":false,"context_lines":[{"line_number":183,"context_line":"\t\t\t\u0026\u0026 gpio_config-\u003einit_state !\u003d ADAPTER_GPIO_INIT_STATE_INPUT) {"},{"line_number":184,"context_line":"\t\t/* The GPIO is an output, and needs to be an output. There is a risk"},{"line_number":185,"context_line":"\t\t* that changing the output value might connect two outputs together."},{"line_number":186,"context_line":"\t\t*/"},{"line_number":187,"context_line":"\t\tint current_value \u003d initial_gpio_mode[idx] \u003d\u003d AM335XGPIO_GPIO_MODE_OUTPUT_HIGH ? 1 : 0;"},{"line_number":188,"context_line":"\t\tint new_value \u003d (gpio_config-\u003einit_state \u003d\u003d ADAPTER_GPIO_INIT_STATE_ACTIVE) ^ gpio_config-\u003eactive_low ? 1 : 0;"},{"line_number":189,"context_line":"\t\tif (new_value !\u003d current_value)"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"6f34b7ff_cdad153e","line":186,"in_reply_to":"ea1f706e_1ae35eb6","updated":"2022-06-12 15:10:54.000000000","message":"I would like that when OpenOCD connects to the target it does not temporarily and *unnecessarily* change the state of a pin. This is important now that the initial state can be set in the GPIO config. The use cases I have in mind are targets which are running (or, alternatively, deliberately held in reset) and connected to a bit-banging adapter such as the bcm2835 or am335xgpio. I can see myself using scenarios like this for automated firmware/hardware tests. For such cases the srst pin should not glitch otherwise the target will reset (alternatively, it could run when it was not supposed to which could be bad for other reasons). This doesn\u0027t have to apply to just the srst pin; similar unwanted behaviour could apply to the swdio pin for Nordic Semiconductor nRF51 devices as that pin also functions as a reset pin. This of course only applies when the pin state that OpenOCD found the pin in is compatible with the selected initial state pin state.\n\nIs it possible for OpenOCD to connect to a running target without resetting it? If not then one of the use cases is not possible.","commit_id":"7bc340ed39a568e8b70f999d8db8d01ba0e35542"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"762a1dde40b4a05e6714a5efe128fc43e0dde706","unresolved":true,"context_lines":[{"line_number":197,"context_line":"\t\tbreak;"},{"line_number":198,"context_line":"\tcase ADAPTER_GPIO_INIT_STATE_ACTIVE:"},{"line_number":199,"context_line":"\t\tset_gpio_value(gpio_config, 1);"},{"line_number":200,"context_line":"\t\tAM335XGPIO_SET_OUTPUT(gpio_config);"},{"line_number":201,"context_line":"\t\tbreak;"},{"line_number":202,"context_line":"\tcase ADAPTER_GPIO_INIT_STATE_INPUT:"},{"line_number":203,"context_line":"\t\tAM335XGPIO_SET_INPUT(gpio_config);"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"6641d5d7_076c342f","line":200,"updated":"2022-06-12 18:58:09.000000000","message":"why you put the GPIO in output after the call to set_gpio_value(,1) ???\n\nLet\u0027s restrict to GPIO output active high.\nIf it is push-pull or open-source, set_gpio_value(,1) has put it OUT HIGH. No need to put it output again!\nIf it is open-drain, set_gpio_value(,1) has put it IN. Why you put it in output? It should remain in IN.\n\nI think we have a mismatch on what ADAPTER_GPIO_INIT_STATE_ACTIVE/INACTIVE means in case of open-source and open-drain.\nFor me we should respect the open-source and open-drain properties.\nI mean: open-drain + active high; set active means GPIO in input.\nWhat\u0027s your idea on this?","commit_id":"7bc340ed39a568e8b70f999d8db8d01ba0e35542"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"88fc1f09c50d63eb025ec0c9bd8483792d523f15","unresolved":false,"context_lines":[{"line_number":197,"context_line":"\t\tbreak;"},{"line_number":198,"context_line":"\tcase ADAPTER_GPIO_INIT_STATE_ACTIVE:"},{"line_number":199,"context_line":"\t\tset_gpio_value(gpio_config, 1);"},{"line_number":200,"context_line":"\t\tAM335XGPIO_SET_OUTPUT(gpio_config);"},{"line_number":201,"context_line":"\t\tbreak;"},{"line_number":202,"context_line":"\tcase ADAPTER_GPIO_INIT_STATE_INPUT:"},{"line_number":203,"context_line":"\t\tAM335XGPIO_SET_INPUT(gpio_config);"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"ca42936c_8f75c6a7","line":200,"in_reply_to":"6641d5d7_076c342f","updated":"2022-06-17 10:07:07.000000000","message":"I think we did have differing ideas on what active meant but I think they are aligned now. I\u0027ve removed the incorrect calls to AM335XGPIO_SET_OUTPUT(), and added the missing call to AM335XGPIO_SET_OUTPUT() for push-pull outputs.","commit_id":"7bc340ed39a568e8b70f999d8db8d01ba0e35542"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"d1ed86a5f6bd4e75bab63821f60f41d5cf820cc2","unresolved":true,"context_lines":[{"line_number":1,"context_line":"/* SPDX-License-Identifier: GPL-2.0-or-later */"},{"line_number":2,"context_line":"/***************************************************************************"},{"line_number":3,"context_line":" *   Copyright (C) 2022 by Steve Marple, stevemarple@googlemail.com        *"},{"line_number":4,"context_line":" *                                                                         *"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"57ee734d_a80de87a","line":1,"updated":"2022-06-18 22:04:27.000000000","message":"Please don\u0027t mix SPDX tag with the license text below, otherwise it would be more work to convert all the files to SPDX.\nRemove the SPDX line, for the moment","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"ceea93b1e0018812ba8b032e6b23058539946d6a","unresolved":false,"context_lines":[{"line_number":1,"context_line":"/* SPDX-License-Identifier: GPL-2.0-or-later */"},{"line_number":2,"context_line":"/***************************************************************************"},{"line_number":3,"context_line":" *   Copyright (C) 2022 by Steve Marple, stevemarple@googlemail.com        *"},{"line_number":4,"context_line":" *                                                                         *"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"9171e2d1_4e29444c","line":1,"in_reply_to":"57ee734d_a80de87a","updated":"2022-06-19 20:39:10.000000000","message":"Done","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"d1ed86a5f6bd4e75bab63821f60f41d5cf820cc2","unresolved":true,"context_lines":[{"line_number":108,"context_line":"{"},{"line_number":109,"context_line":"\tunsigned int shift \u003d gpio_config-\u003egpio_num;"},{"line_number":110,"context_line":"\treturn ((AM335XGPIO_READ_REG(gpio_config-\u003echip_num, AM335XGPIO_GPIO_DATAIN_OFFSET) \u003e\u003e shift) \u0026 1)"},{"line_number":111,"context_line":"\t\t\t\t^ gpio_config-\u003eactive_low;"},{"line_number":112,"context_line":"}"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"static void set_gpio_value(const struct adapter_gpio_config *gpio_config, int value)"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"26364d02_1e13e02a","line":111,"updated":"2022-06-18 22:04:27.000000000","message":"active_low is a bool, while the rest is a uint31_t value.\nIn C a bool is also an integer, but this line that mixes them is odd.\nWhat about:\n uint32_t value \u003d AM335XGPIO_READ_REG(gpio_config-\u003echip_num, AM335XGPIO_GPIO_DATAIN_OFFSET);\n value \u003d (value \u003e\u003e shift) \u0026 1;\n return gpio_config-\u003eactive_low ? (value ^ 1) : value;\nor\n return value ^ (gpio_config-\u003eactive_low ? 1 : 0);\nor\n return ((value \u003d\u003d 0) \u003d\u003d gpio_config-\u003eactive_low) ? 1 : 0;","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"ceea93b1e0018812ba8b032e6b23058539946d6a","unresolved":false,"context_lines":[{"line_number":108,"context_line":"{"},{"line_number":109,"context_line":"\tunsigned int shift \u003d gpio_config-\u003egpio_num;"},{"line_number":110,"context_line":"\treturn ((AM335XGPIO_READ_REG(gpio_config-\u003echip_num, AM335XGPIO_GPIO_DATAIN_OFFSET) \u003e\u003e shift) \u0026 1)"},{"line_number":111,"context_line":"\t\t\t\t^ gpio_config-\u003eactive_low;"},{"line_number":112,"context_line":"}"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"static void set_gpio_value(const struct adapter_gpio_config *gpio_config, int value)"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"6e369a3e_747685da","line":111,"in_reply_to":"26364d02_1e13e02a","updated":"2022-06-19 20:39:10.000000000","message":"Done","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"d1ed86a5f6bd4e75bab63821f60f41d5cf820cc2","unresolved":true,"context_lines":[{"line_number":113,"context_line":""},{"line_number":114,"context_line":"static void set_gpio_value(const struct adapter_gpio_config *gpio_config, int value)"},{"line_number":115,"context_line":"{"},{"line_number":116,"context_line":"\tvalue \u003d value ^ gpio_config-\u003eactive_low;"},{"line_number":117,"context_line":"\tswitch (gpio_config-\u003edrive) {"},{"line_number":118,"context_line":"\tcase ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL:"},{"line_number":119,"context_line":"\t\tif (value)"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"89bf1db2_e3ef7a12","line":116,"updated":"2022-06-18 22:04:27.000000000","message":"also here, mix int and bool!\nwould it be easier re-define \u0027active_low\u0027 as int or unsigned int, carrying only value 0 or 1, adding a comment in adapter.h that explains the reason?\nAnd maybe even changing its name to \u0027polarity\u0027?","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"ceea93b1e0018812ba8b032e6b23058539946d6a","unresolved":false,"context_lines":[{"line_number":113,"context_line":""},{"line_number":114,"context_line":"static void set_gpio_value(const struct adapter_gpio_config *gpio_config, int value)"},{"line_number":115,"context_line":"{"},{"line_number":116,"context_line":"\tvalue \u003d value ^ gpio_config-\u003eactive_low;"},{"line_number":117,"context_line":"\tswitch (gpio_config-\u003edrive) {"},{"line_number":118,"context_line":"\tcase ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL:"},{"line_number":119,"context_line":"\t\tif (value)"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"749d996b_12390b30","line":116,"in_reply_to":"89bf1db2_e3ef7a12","updated":"2022-06-19 20:39:10.000000000","message":"Fixed here too.\n\nWhile I can see benefits of changing the data type in adapter.h for this driver other drivers might use different data types for their values and so uint16_t or uint8_t might make more sense for them. In view of this, and that active_low should take one of two values I think adapter.h should stick with bool and I fix this driver.\n\nTo me polarity means the sense of the electrical voltage for the signals (it makes me think of RS-232 which uses real negative voltages in the range -3V to -15V to indicate a \u00271\u0027).","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"d1ed86a5f6bd4e75bab63821f60f41d5cf820cc2","unresolved":true,"context_lines":[{"line_number":191,"context_line":"\t\t* temporarily switch to an an input so that set_gpio_value() can safely"},{"line_number":192,"context_line":"\t\t* set the output value; it will switch back the GPIO to an output if"},{"line_number":193,"context_line":"\t\t* required."},{"line_number":194,"context_line":"\t\t*/"},{"line_number":195,"context_line":"\t\tint current_value \u003d initial_gpio_mode[idx] \u003d\u003d AM335XGPIO_GPIO_MODE_OUTPUT_HIGH ? 1 : 0;"},{"line_number":196,"context_line":"\t\tint new_value \u003d (adapter_gpio_config[idx].init_state \u003d\u003d ADAPTER_GPIO_INIT_STATE_ACTIVE)"},{"line_number":197,"context_line":"\t\t\t\t^ adapter_gpio_config[idx].active_low ? 1 : 0;"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"af65f934_32de13dd","line":194,"updated":"2022-06-18 22:04:27.000000000","message":"I still don\u0027t understand why you have to do all of this.\nLet\u0027s take one example:\n- gpio is currently out high, current_value\u003d1;\n- new state would be open-drain inactive, so would go out low, new_value\u003d0;\nThe big condition above is satisfied.\nSince (new_value !\u003d current_value) you first put the gpio in input then to 0 but not in output. When it will go output 0?\n\nWhat is the benefit of this? Why this is better than directly go from output 1 to output 0?\nYou mention in the comment \u0027There is a risk that changing the output value might connect two outputs together.\u0027 Which are the two outputs? How this prevents the connection?","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"ceea93b1e0018812ba8b032e6b23058539946d6a","unresolved":false,"context_lines":[{"line_number":191,"context_line":"\t\t* temporarily switch to an an input so that set_gpio_value() can safely"},{"line_number":192,"context_line":"\t\t* set the output value; it will switch back the GPIO to an output if"},{"line_number":193,"context_line":"\t\t* required."},{"line_number":194,"context_line":"\t\t*/"},{"line_number":195,"context_line":"\t\tint current_value \u003d initial_gpio_mode[idx] \u003d\u003d AM335XGPIO_GPIO_MODE_OUTPUT_HIGH ? 1 : 0;"},{"line_number":196,"context_line":"\t\tint new_value \u003d (adapter_gpio_config[idx].init_state \u003d\u003d ADAPTER_GPIO_INIT_STATE_ACTIVE)"},{"line_number":197,"context_line":"\t\t\t\t^ adapter_gpio_config[idx].active_low ? 1 : 0;"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"094342c7_a721db8f","line":194,"in_reply_to":"af65f934_32de13dd","updated":"2022-06-19 20:39:10.000000000","message":"You\u0027re right, it isn\u0027t needed. I think it was from an earlier version of set_gpio_value() that worked differently.","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"d1ed86a5f6bd4e75bab63821f60f41d5cf820cc2","unresolved":true,"context_lines":[{"line_number":457,"context_line":"\t\t\t* prevents two outputs connected together. The initial value for"},{"line_number":458,"context_line":"\t\t\t* swdio_dir must be found from the initial value requested for"},{"line_number":459,"context_line":"\t\t\t* swdio."},{"line_number":460,"context_line":"\t\t\t*/"},{"line_number":461,"context_line":"\t\t\tinitial_gpio_mode[ADAPTER_GPIO_IDX_SWDIO_DIR]"},{"line_number":462,"context_line":"\t\t\t\t\u003d get_gpio_mode(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO_DIR]);"},{"line_number":463,"context_line":"\t\t\tif (adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].init_state \u003d\u003d ADAPTER_GPIO_INIT_STATE_INPUT) {"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"f145310b_c7088ae7","line":460,"updated":"2022-06-18 22:04:27.000000000","message":"also here, I don\u0027t understand what you want to do, nor which are the two outputs connected together that this code prevent to get connected.","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"20c3fedd00d94f9524cc6bc3e40f5b73027e7b31","unresolved":false,"context_lines":[{"line_number":457,"context_line":"\t\t\t* prevents two outputs connected together. The initial value for"},{"line_number":458,"context_line":"\t\t\t* swdio_dir must be found from the initial value requested for"},{"line_number":459,"context_line":"\t\t\t* swdio."},{"line_number":460,"context_line":"\t\t\t*/"},{"line_number":461,"context_line":"\t\t\tinitial_gpio_mode[ADAPTER_GPIO_IDX_SWDIO_DIR]"},{"line_number":462,"context_line":"\t\t\t\t\u003d get_gpio_mode(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO_DIR]);"},{"line_number":463,"context_line":"\t\t\tif (adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].init_state \u003d\u003d ADAPTER_GPIO_INIT_STATE_INPUT) {"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"c007318e_53c448ff","line":460,"in_reply_to":"35fe925f_33445789","updated":"2022-06-20 13:41:06.000000000","message":"ok, thanks!","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"ceea93b1e0018812ba8b032e6b23058539946d6a","unresolved":false,"context_lines":[{"line_number":457,"context_line":"\t\t\t* prevents two outputs connected together. The initial value for"},{"line_number":458,"context_line":"\t\t\t* swdio_dir must be found from the initial value requested for"},{"line_number":459,"context_line":"\t\t\t* swdio."},{"line_number":460,"context_line":"\t\t\t*/"},{"line_number":461,"context_line":"\t\t\tinitial_gpio_mode[ADAPTER_GPIO_IDX_SWDIO_DIR]"},{"line_number":462,"context_line":"\t\t\t\t\u003d get_gpio_mode(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO_DIR]);"},{"line_number":463,"context_line":"\t\t\tif (adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].init_state \u003d\u003d ADAPTER_GPIO_INIT_STATE_INPUT) {"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"35fe925f_33445789","line":460,"in_reply_to":"f145310b_c7088ae7","updated":"2022-06-19 20:39:10.000000000","message":"When the swdio GPIO of the AM335x is configured as an output and its external buffer is configured to send the swdio signal from the target to the AM335x two outputs are wired together. \n\nOther drivers (eg bcm2835, linuxgpiod) already take this into account but the initialization is simpler as they do not permit the initial state to be configured by the user.\n\nI\u0027ve improved the comment to clarify this.","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"20c3fedd00d94f9524cc6bc3e40f5b73027e7b31","unresolved":true,"context_lines":[{"line_number":247,"context_line":"\t/* As the \"adapter reset_config\" command keeps the srst and trst gpio drive"},{"line_number":248,"context_line":"\t* mode settings in sync we can use our standard set_gpio_value() function"},{"line_number":249,"context_line":"\t* that honours drive mode and active low."},{"line_number":250,"context_line":"\t*/"},{"line_number":251,"context_line":"\tif (is_gpio_config_valid(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SRST]))"},{"line_number":252,"context_line":"\t\tset_gpio_value(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SRST], srst);"},{"line_number":253,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":6,"id":"716cf340_afb75680","line":250,"updated":"2022-06-20 13:41:06.000000000","message":"align the \u0027*\u0027 chars to the \u0027*\u0027 in first line of the comment\nAlso in other comments below","commit_id":"fde8cec05e5af073d0670a07d1b71f18aa592bae"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"6323187bbb1b41b758cc563fc9e3b8d1bb394abf","unresolved":false,"context_lines":[{"line_number":247,"context_line":"\t/* As the \"adapter reset_config\" command keeps the srst and trst gpio drive"},{"line_number":248,"context_line":"\t* mode settings in sync we can use our standard set_gpio_value() function"},{"line_number":249,"context_line":"\t* that honours drive mode and active low."},{"line_number":250,"context_line":"\t*/"},{"line_number":251,"context_line":"\tif (is_gpio_config_valid(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SRST]))"},{"line_number":252,"context_line":"\t\tset_gpio_value(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SRST], srst);"},{"line_number":253,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":6,"id":"6177351e_9f157df8","line":250,"in_reply_to":"716cf340_afb75680","updated":"2022-06-20 16:10:41.000000000","message":"Done","commit_id":"fde8cec05e5af073d0670a07d1b71f18aa592bae"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"20c3fedd00d94f9524cc6bc3e40f5b73027e7b31","unresolved":true,"context_lines":[{"line_number":264,"context_line":"static void am335xgpio_swdio_drive(bool is_output)"},{"line_number":265,"context_line":"{"},{"line_number":266,"context_line":"\tif (is_output) {"},{"line_number":267,"context_line":"\t\tset_gpio_value(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO_DIR], 1);"},{"line_number":268,"context_line":"\t\tAM335XGPIO_SET_OUTPUT(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO]);"},{"line_number":269,"context_line":"\t} else {"},{"line_number":270,"context_line":"\t\tAM335XGPIO_SET_INPUT(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO]);"}],"source_content_type":"text/x-csrc","patch_set":6,"id":"c490f387_d523c854","line":267,"updated":"2022-06-20 13:41:06.000000000","message":"what happens if ADAPTER_GPIO_IDX_SWDIO_DIR is not valid?","commit_id":"fde8cec05e5af073d0670a07d1b71f18aa592bae"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"6323187bbb1b41b758cc563fc9e3b8d1bb394abf","unresolved":false,"context_lines":[{"line_number":264,"context_line":"static void am335xgpio_swdio_drive(bool is_output)"},{"line_number":265,"context_line":"{"},{"line_number":266,"context_line":"\tif (is_output) {"},{"line_number":267,"context_line":"\t\tset_gpio_value(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO_DIR], 1);"},{"line_number":268,"context_line":"\t\tAM335XGPIO_SET_OUTPUT(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO]);"},{"line_number":269,"context_line":"\t} else {"},{"line_number":270,"context_line":"\t\tAM335XGPIO_SET_INPUT(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO]);"}],"source_content_type":"text/x-csrc","patch_set":6,"id":"66729476_b4dafa86","line":267,"in_reply_to":"c490f387_d523c854","updated":"2022-06-20 16:10:41.000000000","message":"I\u0027m sure you know that bad things will happen! :-)\nOk, those calls are now conditional on the GPIO being valid.","commit_id":"fde8cec05e5af073d0670a07d1b71f18aa592bae"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"20c3fedd00d94f9524cc6bc3e40f5b73027e7b31","unresolved":true,"context_lines":[{"line_number":452,"context_line":"\t\t\t\tset_gpio_value(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO_DIR], 1);"},{"line_number":453,"context_line":"\t\t\t\tAM335XGPIO_SET_OUTPUT(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO_DIR]);"},{"line_number":454,"context_line":"\t\t\t\tinitialize_gpio(ADAPTER_GPIO_IDX_SWDIO);"},{"line_number":455,"context_line":"\t\t\t}"},{"line_number":456,"context_line":"\t\t} else {"},{"line_number":457,"context_line":"\t\t\tinitialize_gpio(ADAPTER_GPIO_IDX_SWDIO);"},{"line_number":458,"context_line":"\t\t}"}],"source_content_type":"text/x-csrc","patch_set":6,"id":"4bc2a685_91316997","line":455,"updated":"2022-06-20 13:41:06.000000000","message":"Somewhere in the adapter init there should be something to force the init value of SWDIO_DIR to match the init direction of SWDIO.\nAfter that, here can be simpler and more readable:\n if (adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].init_state \u003d\u003d ADAPTER_GPIO_INIT_STATE_INPUT) {\n   initialize_gpio(ADAPTER_GPIO_IDX_SWDIO);\n   initialize_gpio(ADAPTER_GPIO_IDX_SWDIO_DIR);\n } else {\n   initialize_gpio(ADAPTER_GPIO_IDX_SWDIO_DIR);\n   initialize_gpio(ADAPTER_GPIO_IDX_SWDIO);\n }","commit_id":"fde8cec05e5af073d0670a07d1b71f18aa592bae"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"6323187bbb1b41b758cc563fc9e3b8d1bb394abf","unresolved":false,"context_lines":[{"line_number":452,"context_line":"\t\t\t\tset_gpio_value(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO_DIR], 1);"},{"line_number":453,"context_line":"\t\t\t\tAM335XGPIO_SET_OUTPUT(\u0026adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO_DIR]);"},{"line_number":454,"context_line":"\t\t\t\tinitialize_gpio(ADAPTER_GPIO_IDX_SWDIO);"},{"line_number":455,"context_line":"\t\t\t}"},{"line_number":456,"context_line":"\t\t} else {"},{"line_number":457,"context_line":"\t\t\tinitialize_gpio(ADAPTER_GPIO_IDX_SWDIO);"},{"line_number":458,"context_line":"\t\t}"}],"source_content_type":"text/x-csrc","patch_set":6,"id":"fdc3a6d5_eeb22388","line":455,"in_reply_to":"4bc2a685_91316997","updated":"2022-06-20 16:10:41.000000000","message":"Done","commit_id":"fde8cec05e5af073d0670a07d1b71f18aa592bae"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"0eb1cb20a458c8091fd2574cd0b72be13f4033e4","unresolved":true,"context_lines":[{"line_number":147,"context_line":"{"},{"line_number":148,"context_line":"\tif (AM335XGPIO_READ_REG(gpio_config-\u003echip_num, AM335XGPIO_GPIO_OE_OFFSET) \u0026 BIT(gpio_config-\u003egpio_num)) {"},{"line_number":149,"context_line":"\t\treturn AM335XGPIO_GPIO_MODE_INPUT;"},{"line_number":150,"context_line":"\t} else {"},{"line_number":151,"context_line":"\t\t/* Return output level too so that pin mode can be fully restored */"},{"line_number":152,"context_line":"\t\tif (AM335XGPIO_READ_REG(gpio_config-\u003echip_num, AM335XGPIO_GPIO_DATAOUT_OFFSET) \u0026 BIT(gpio_config-\u003egpio_num))"},{"line_number":153,"context_line":"\t\t\treturn AM335XGPIO_GPIO_MODE_OUTPUT_HIGH;"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"168bfca8_7487fb54","line":150,"updated":"2022-06-20 20:33:36.000000000","message":"No need for \u0027else\u0027 after a \u0027return\u0027. This function becomes:\n static enum amx335gpio_initial_gpio_mode get_gpio_mode(...)\n {\n   if (...)\n     return AM335XGPIO_GPIO_MODE_INPUT;\n   if (...)\n     return AM335XGPIO_GPIO_MODE_OUTPUT_HIGH;\n   return AM335XGPIO_GPIO_MODE_OUTPUT_LOW;\n }","commit_id":"07372e554462f8604d912ac379cee1e77aeed22a"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"f2912f76d83158f701a9f95ec19d6ea636e8ab98","unresolved":false,"context_lines":[{"line_number":147,"context_line":"{"},{"line_number":148,"context_line":"\tif (AM335XGPIO_READ_REG(gpio_config-\u003echip_num, AM335XGPIO_GPIO_OE_OFFSET) \u0026 BIT(gpio_config-\u003egpio_num)) {"},{"line_number":149,"context_line":"\t\treturn AM335XGPIO_GPIO_MODE_INPUT;"},{"line_number":150,"context_line":"\t} else {"},{"line_number":151,"context_line":"\t\t/* Return output level too so that pin mode can be fully restored */"},{"line_number":152,"context_line":"\t\tif (AM335XGPIO_READ_REG(gpio_config-\u003echip_num, AM335XGPIO_GPIO_DATAOUT_OFFSET) \u0026 BIT(gpio_config-\u003egpio_num))"},{"line_number":153,"context_line":"\t\t\treturn AM335XGPIO_GPIO_MODE_OUTPUT_HIGH;"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"7057bcb7_9d578896","line":150,"in_reply_to":"168bfca8_7487fb54","updated":"2022-06-20 22:13:33.000000000","message":"Done","commit_id":"07372e554462f8604d912ac379cee1e77aeed22a"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"0eb1cb20a458c8091fd2574cd0b72be13f4033e4","unresolved":true,"context_lines":[{"line_number":188,"context_line":"\t\t\tinit_state \u003d ADAPTER_GPIO_INIT_STATE_INACTIVE;"},{"line_number":189,"context_line":"\t\telse"},{"line_number":190,"context_line":"\t\t\tinit_state \u003d ADAPTER_GPIO_INIT_STATE_ACTIVE;"},{"line_number":191,"context_line":"\t}"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"\tswitch (init_state) {"},{"line_number":194,"context_line":"\tcase ADAPTER_GPIO_INIT_STATE_INACTIVE:"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"1791d6d7_1c0fba01","line":191,"updated":"2022-06-20 20:33:36.000000000","message":"this code will be repeated on every driver that use this gpio method.\nIt would be better moving it in previous patch at the end of adapter.c:adapter_gpio_config_handler():\n   /* TODO: move this in \"adapter init\" once available */\n   /* init state of SWDIO forces SWDIO_DIR */\n   if (gpio_idx \u003d\u003d ADAPTER_GPIO_IDX_SWDIO)\n     adapter_config.gpios[ADAPTER_GPIO_IDX_SWDIO_DIR].init_state \u003d\n       (gpio_config-\u003einit_state \u003d\u003d ADAPTER_GPIO_INIT_STATE_INPUT) ?\n       ADAPTER_GPIO_INIT_STATE_INACTIVE :\n       ADAPTER_GPIO_INIT_STATE_ACTIVE;\n   return ERROR_OK;\n }","commit_id":"07372e554462f8604d912ac379cee1e77aeed22a"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"f2912f76d83158f701a9f95ec19d6ea636e8ab98","unresolved":false,"context_lines":[{"line_number":188,"context_line":"\t\t\tinit_state \u003d ADAPTER_GPIO_INIT_STATE_INACTIVE;"},{"line_number":189,"context_line":"\t\telse"},{"line_number":190,"context_line":"\t\t\tinit_state \u003d ADAPTER_GPIO_INIT_STATE_ACTIVE;"},{"line_number":191,"context_line":"\t}"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"\tswitch (init_state) {"},{"line_number":194,"context_line":"\tcase ADAPTER_GPIO_INIT_STATE_INACTIVE:"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"d9d30754_0967c208","line":191,"in_reply_to":"1791d6d7_1c0fba01","updated":"2022-06-20 22:13:33.000000000","message":"Sorry, I misunderstood what you requested previously. Yes, going into adapter.c is much better.","commit_id":"07372e554462f8604d912ac379cee1e77aeed22a"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"0eb1cb20a458c8091fd2574cd0b72be13f4033e4","unresolved":true,"context_lines":[{"line_number":202,"context_line":"\t\tbreak;"},{"line_number":203,"context_line":"\t}"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"\tif (adapter_gpio_config[idx].drive \u003d\u003d ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL)"},{"line_number":206,"context_line":"\t\tAM335XGPIO_SET_OUTPUT(\u0026adapter_gpio_config[idx]);"},{"line_number":207,"context_line":"}"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":7,"id":"7c1d2063_245211a2","line":205,"updated":"2022-06-20 20:33:36.000000000","message":"please add a comment before the if, at every review I hit the head again and again:\n/* direction for non push-pull is already set by set_gpio_value() */","commit_id":"07372e554462f8604d912ac379cee1e77aeed22a"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"f2912f76d83158f701a9f95ec19d6ea636e8ab98","unresolved":false,"context_lines":[{"line_number":202,"context_line":"\t\tbreak;"},{"line_number":203,"context_line":"\t}"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"\tif (adapter_gpio_config[idx].drive \u003d\u003d ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL)"},{"line_number":206,"context_line":"\t\tAM335XGPIO_SET_OUTPUT(\u0026adapter_gpio_config[idx]);"},{"line_number":207,"context_line":"}"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":7,"id":"283b882b_74e92394","line":205,"in_reply_to":"7c1d2063_245211a2","updated":"2022-06-20 22:13:33.000000000","message":"Done","commit_id":"07372e554462f8604d912ac379cee1e77aeed22a"}],"src/jtag/startup.tcl":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"d1ed86a5f6bd4e75bab63821f60f41d5cf820cc2","unresolved":true,"context_lines":[{"line_number":800,"context_line":"\techo \"DEPRECATED! use \u0027adapter serial\u0027 not \u0027xds110 serial\u0027\""},{"line_number":801,"context_line":"\teval adapter serial $args"},{"line_number":802,"context_line":"}"},{"line_number":803,"context_line":""},{"line_number":804,"context_line":"# END MIGRATION AIDS"}],"source_content_type":"text/x-tcl","patch_set":5,"id":"9d10febe_3140c907","line":803,"updated":"2022-06-18 22:04:27.000000000","message":"every command that suddenly disappears, has to be listed here with a backward compatible tcl proc. E.g.\n lappend _telnet_autocomplete_skip \"am335xgpio tdo_num\"\n proc \"am335xgpio tdo_num\" {num} {\n     echo \"DEPRECATED! use \u0027adapter gpio tdo\u0027 not \u0027am335xgpio tdo_num\u0027\"\n     eval adapter gpio tdo [expr {$num % 32}] -chip [expr {$num / 32}]\n }","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"},{"author":{"_account_id":1001975,"name":"Steve Marple","email":"stevemarple@googlemail.com","username":"stevemarple"},"change_message_id":"ceea93b1e0018812ba8b032e6b23058539946d6a","unresolved":false,"context_lines":[{"line_number":800,"context_line":"\techo \"DEPRECATED! use \u0027adapter serial\u0027 not \u0027xds110 serial\u0027\""},{"line_number":801,"context_line":"\teval adapter serial $args"},{"line_number":802,"context_line":"}"},{"line_number":803,"context_line":""},{"line_number":804,"context_line":"# END MIGRATION AIDS"}],"source_content_type":"text/x-tcl","patch_set":5,"id":"d2f0c91d_51d3d127","line":803,"in_reply_to":"9d10febe_3140c907","updated":"2022-06-19 20:39:10.000000000","message":"Nice! I didn\u0027t know that existed.\n\nDone.","commit_id":"ab8b90d40798ec9af18097cfd12b098b9848e06d"}]}