)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"601b11cc763a50666b8801592c01cce88f90a64d","unresolved":true,"context_lines":[{"line_number":10,"context_line":"to the newer v2 API of libgpiod."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"I\u0027ve chosen to add it in parallel to the existing driver so that it"},{"line_number":13,"context_line":"is possible to have releases with support for both."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"I decided to make a dedicated new module to prevent having a lot of"},{"line_number":16,"context_line":"ifdef-ery inside the C file itself - in my eyes, the small amount of"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"dddf40fd_9575f3ca","line":13,"updated":"2024-03-24 22:49:24.000000000","message":"configure.ac asks to pkgconfig the version of the installed ```libgpiod```, similarly to the command line:\n```pkgconf --modversion libgpiod```\nI don\u0027t see any support in this patch for enabling both versions.\n\nBoth Debian and Arch linux provide libgpiod v2 as a replacement of the older version. There is no way to install both, something that is instead possible for new libusb-1.0 and old libusb-compat.","commit_id":"acd26894d3bf2ac36a1776947dd3e3c397b957b5"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"601b11cc763a50666b8801592c01cce88f90a64d","unresolved":true,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"I decided to make a dedicated new module to prevent having a lot of"},{"line_number":16,"context_line":"ifdef-ery inside the C file itself - in my eyes, the small amount of"},{"line_number":17,"context_line":"additionl makefile magic is simpler."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The version of libgpiod is detected during configure time based on"},{"line_number":20,"context_line":"the pkg-config version detection since the library itself does not"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5bf3d0c7_cca293c8","line":17,"updated":"2024-03-24 22:49:24.000000000","message":"I\u0027m also against abusing of ```#if```/```#endif```.\nProbably one option, but still don\u0027t know if the result will look readable, is to convert the code to v2-like functions, and on top of the file having a single ```#if v1```/```#endif``` containing functions that have v2-like prototypes and calls v1 API.\nThis could be done in a two step patches: first converting existing code to v2-like, then adding the ```#if v1```/```#endif```.\nPlease scream if you see something bad in this","commit_id":"acd26894d3bf2ac36a1776947dd3e3c397b957b5"},{"author":{"_account_id":1002250,"name":"Michael Heimpold","email":"mhei@heimpold.de","username":"mhei"},"change_message_id":"33e1cee6eebb197b394245b163886c0024d4f521","unresolved":true,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"I decided to make a dedicated new module to prevent having a lot of"},{"line_number":16,"context_line":"ifdef-ery inside the C file itself - in my eyes, the small amount of"},{"line_number":17,"context_line":"additionl makefile magic is simpler."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The version of libgpiod is detected during configure time based on"},{"line_number":20,"context_line":"the pkg-config version detection since the library itself does not"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7310c356_51fa757b","line":17,"in_reply_to":"5bf3d0c7_cca293c8","updated":"2024-03-29 14:14:13.000000000","message":"I\u0027ll have a 2nd deeper look how it could be possible to support both versions in parallel. The first problem I see is that libgpiod does not provide version defines by itself. So it is not possible without pkg-config magic to know at compile time against which version we are compiling. I\u0027ll think that I\u0027ll open an upstream issue regarding this.\nBTW: my first patches are intended to keep the git history when making a copy of the file - if it is cleanly possible to have support in a single file, then this is obsolete of course.","commit_id":"acd26894d3bf2ac36a1776947dd3e3c397b957b5"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"00952da904b4770d02652be8af1f23ec4c0c0382","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"aba763ca_4c2e27e3","updated":"2024-03-24 22:31:20.000000000","message":"Really, thanks for this patch series.\nI haven\u0027t found time to dig in the differences between libgpiod v1 vs v2 yet.\n\nFrom the second patch in this series I see that checkpatch is not happy with the original code of linuxgpiod. I have pushed a fix for the driver in ```https://review.openocd.org/c/8187``` and a fix for checkpatch in ```https://review.openocd.org/c/8188```\nThere is also a bug in the code, so patch ```https://review.openocd.org/c/8186```\nAnd current build system fails with error if libgpiod v2 is installed, so patch ```https://review.openocd.org/c/8185```\n\nI think you should rebase your series on top of the patches above, maybe after I merge them. If you want review my patches, you are welcome.\n\nYour first and second patch should be squashed together. I mean, you should not rename the file and then resume the initial one. The build with the file renamed fails. You could directly add a copy of the file.\nSaid that, I\u0027m still thinking if your solution is the best one.\nSurely we cannot drop the support for libgpiod v1 for the next years.\nOn one side, having two drivers, one for libgpiod v1 and one for v2 looks easy for further code improvement thanks of v2 new API.\nOn the other side, maintaining two almost identical files it\u0027s a waste of time and resources. Plus, new users will probably only compile for v2, with the risk that soon the code for v1 will get broken and not compile anymore. This let me prefer having in a single file the support for both libgpiod v1 and v2. But I cannot judge what the result would be.\nAlso on this, comments are warmly welcome.","commit_id":"acd26894d3bf2ac36a1776947dd3e3c397b957b5"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"4ce4845d431f810b19031648f4ba53677320dddb","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6bf8bb07_0c4e1c16","in_reply_to":"0498c21a_b7a01047","updated":"2024-04-01 10:12:29.000000000","message":"I appreciate your effort to dig in OpenOCD code.\nI think that my https://review.openocd.org/c/8185 should at least fix the dependency issue for OpenWrt and any other distro, but at the cost of dropping the functionality of the libgpiod v1 driver in OpenOCD. Just a temporarily fix.\nI plan to merge that series by end of next week.\n\nI don\u0027t want to force you to spend more time than needed.\nAfter the merge above, please re-push the series keeping the same structure, but with one single patch that copy the existing linuxgpiod.c as linuxgpiodv2.c (8182 and 8183 squashed) but based on the new updated linuxgpiod.c file, plus a second patch that apply the new API.\nI will eventually consider a rework, later on, when I would get some spare time.\nOr hopefully some other developer will jump on it.","commit_id":"acd26894d3bf2ac36a1776947dd3e3c397b957b5"},{"author":{"_account_id":1002250,"name":"Michael Heimpold","email":"mhei@heimpold.de","username":"mhei"},"change_message_id":"96b64b2bb4f41107b29804bd2ddc293d51f21029","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"942d0cd9_ea17fba2","in_reply_to":"6bf8bb07_0c4e1c16","updated":"2024-04-06 09:09:23.000000000","message":"Spending time for OpenOCD is no wasted time, I can learn a lot :-)\nJust for your information, I\u0027ve worked on a change where both library versions are supported in the existing file. I just wanted to see how the #ifdef stuff would look like and I think the result is not that bad as I expected. Maybe you could have a look on my Github fork and the branch: https://github.com/mhei/openocd/tree/linuxgpiodv2-single-file\nThis is based on the current master, after you merged your series, I\u0027m happy to rework and rebase etc.\nYou can then tell me, which approach you prefer.","commit_id":"acd26894d3bf2ac36a1776947dd3e3c397b957b5"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"6eba1ad346d15992e3a1b80b6b95e3d6a94fa8d4","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"da81c351_654aed0b","in_reply_to":"942d0cd9_ea17fba2","updated":"2024-04-07 19:30:20.000000000","message":"At a first look, the result in your github doesn\u0027t look that bad!\nMaybe you could post it in this gerrit for further review, possibly using a new Change-ID so we can temporarily keep both versions and easily compare them.\n\nI\u0027m not a big supported of #if/#else/#endif, so I prefer minimizing the code inside this conditional compiling. Again, the risk that after few commits one side of the #if/#else/#endif get broken is high.\n\nIn your proposal you use libgpiod API v2 inside wrappers to emulate v1 API used by OpenOCD. I would prefer having the opposite, so one day in the future we could easily drop the v1 code.\nI mean, it would be preferable to first rework the existing code to use API as much as possible compatible with v2, adding the small wrappers to compile them with v1 library. Then, a single #if/#endif to drop the wrappers when v2 library is present.\nAlso for the macro to select v1 vs v2, keep v2 as the normal case and define HAVE_LIBGPIOD_V1 for backward compatibility.\n\nI started writing comments here, then I thought was easier to write it down in the code and I have started coding something, but I cannot complete it and still not test it.\nI have pushed it as WIP. Feel free to use it or to trash all, as you want!\nFrom https://review.openocd.org/c/openocd/+/8201 to 8207\nDamn, the part in #if/#endif is way bigger than I expected.","commit_id":"acd26894d3bf2ac36a1776947dd3e3c397b957b5"},{"author":{"_account_id":1002250,"name":"Michael Heimpold","email":"mhei@heimpold.de","username":"mhei"},"change_message_id":"33e1cee6eebb197b394245b163886c0024d4f521","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0498c21a_b7a01047","in_reply_to":"aba763ca_4c2e27e3","updated":"2024-03-29 14:14:13.000000000","message":"First, thanks for your valueable feedback.\nThis was my first shot to get things compiled against the newer libgpiod. I\u0027m package maintainer of libgpiod on OpenWrt and openocd is one of the dependended packages which currently blocks the update to libgpiod v2.\nSo, to be honest, I don\u0027t have any deeper background with openocd. I\u0027ve just built up a small test environment with a Raspi to make very simple regression tests.\nThat said, I\u0027ll have a look at your patches and your other comments. But since I\u0027m doing this in my free time, it might take some evenings :-)","commit_id":"acd26894d3bf2ac36a1776947dd3e3c397b957b5"},{"author":{"_account_id":1002250,"name":"Michael Heimpold","email":"mhei@heimpold.de","username":"mhei"},"change_message_id":"dcb3c370068cc35fb8b3630fa2ee0794ae5272ac","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3f60503f_68ddbe76","in_reply_to":"da81c351_654aed0b","updated":"2024-04-21 06:25:50.000000000","message":"I tried to follow-up and my current working branch is here: https://github.com/mhei/openocd/tree/linuxgpiodv2-emulate-v2-for-v1\nIt is not tested that much, and I\u0027m traveling the next days, so it might take some evenings until I can push it to gerrit as you suggested. I\u0027ll try my best, but I wanted to keep you updated already.","commit_id":"acd26894d3bf2ac36a1776947dd3e3c397b957b5"}],"src/jtag/drivers/linuxgpiodv2.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"00952da904b4770d02652be8af1f23ec4c0c0382","unresolved":true,"context_lines":[{"line_number":331,"context_line":"\tint l;"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"\tl \u003d snprintf(chip_path, sizeof(chip_path), \"/dev/gpiochip%u\", chip_num);"},{"line_number":334,"context_line":"\tif (l \u003c 0 || l \u003e\u003d (int)sizeof(chip_path))"},{"line_number":335,"context_line":"\t\treturn NULL;"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"\treturn gpiod_chip_open(chip_path);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"1a0e574e_8ddea6b6","line":334,"updated":"2024-03-24 22:31:20.000000000","message":"I think there is no need for the cast to ```int```.\nBoth ```int l``` and ```size_t sizeof()``` are signed. The compiler knows how to handle them","commit_id":"acd26894d3bf2ac36a1776947dd3e3c397b957b5"},{"author":{"_account_id":1002250,"name":"Michael Heimpold","email":"mhei@heimpold.de","username":"mhei"},"change_message_id":"33e1cee6eebb197b394245b163886c0024d4f521","unresolved":true,"context_lines":[{"line_number":331,"context_line":"\tint l;"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"\tl \u003d snprintf(chip_path, sizeof(chip_path), \"/dev/gpiochip%u\", chip_num);"},{"line_number":334,"context_line":"\tif (l \u003c 0 || l \u003e\u003d (int)sizeof(chip_path))"},{"line_number":335,"context_line":"\t\treturn NULL;"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"\treturn gpiod_chip_open(chip_path);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"432473a3_e965c036","line":334,"in_reply_to":"1a0e574e_8ddea6b6","updated":"2024-03-29 14:14:13.000000000","message":"Without the cast, I see:\n\n../src/jtag/drivers/linuxgpiodv2.c:334:24: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror\u003dsign-compare]\n  334 |         if (l \u003c 0 || l \u003e\u003d sizeof(chip_path))\n      |                        ^~\ncc1: all warnings being treated as errors\n\nI could change l to unsigned long int, but then I guess snprintf would complain... since it returns an int.","commit_id":"acd26894d3bf2ac36a1776947dd3e3c397b957b5"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"00952da904b4770d02652be8af1f23ec4c0c0382","unresolved":true,"context_lines":[{"line_number":393,"context_line":""},{"line_number":394,"context_line":"\tswitch (adapter_gpio_config[idx].pull) {"},{"line_number":395,"context_line":"\tcase ADAPTER_GPIO_PULL_NONE:"},{"line_number":396,"context_line":"#ifdef GPIOD_LINE_BIAS_DISABLED"},{"line_number":397,"context_line":"\t\tgpiod_line_settings_set_bias(gpiod_line_settings[idx], GPIOD_LINE_BIAS_DISABLED);"},{"line_number":398,"context_line":"#endif"},{"line_number":399,"context_line":"\t\tbreak;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"fdf4aed3_74a40ce6","line":396,"updated":"2024-03-24 22:31:20.000000000","message":"There is no need for this #ifdef, because in libgpiod v2 this is always present.\nBy the way, you cannot use an enum symbol for #ifdef. I just fixed it for the existing code in https://review.openocd.org/c/openocd/+/8186","commit_id":"acd26894d3bf2ac36a1776947dd3e3c397b957b5"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"00952da904b4770d02652be8af1f23ec4c0c0382","unresolved":true,"context_lines":[{"line_number":398,"context_line":"#endif"},{"line_number":399,"context_line":"\t\tbreak;"},{"line_number":400,"context_line":"\tcase ADAPTER_GPIO_PULL_UP:"},{"line_number":401,"context_line":"#ifdef GPIOD_LINE_BIAS_PULL_UP"},{"line_number":402,"context_line":"\t\tgpiod_line_settings_set_bias(gpiod_line_settings[idx], GPIOD_LINE_BIAS_PULL_UP);"},{"line_number":403,"context_line":"#else"},{"line_number":404,"context_line":"\t\tLOG_WARNING(\"linuxgpiodv2: ignoring request for pull-up on %s: not supported by gpiod v%s\","}],"source_content_type":"text/x-csrc","patch_set":1,"id":"4c727c5e_579e2a1a","line":401,"updated":"2024-03-24 22:31:20.000000000","message":"same here","commit_id":"acd26894d3bf2ac36a1776947dd3e3c397b957b5"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"00952da904b4770d02652be8af1f23ec4c0c0382","unresolved":true,"context_lines":[{"line_number":406,"context_line":"#endif"},{"line_number":407,"context_line":"\t\tbreak;"},{"line_number":408,"context_line":"\tcase ADAPTER_GPIO_PULL_DOWN:"},{"line_number":409,"context_line":"#ifdef GPIOD_LINE_BIAS_PULL_DOWN"},{"line_number":410,"context_line":"\t\tgpiod_line_settings_set_bias(gpiod_line_settings[idx], GPIOD_LINE_BIAS_PULL_DOWN);"},{"line_number":411,"context_line":"#else"},{"line_number":412,"context_line":"\t\tLOG_WARNING(\"linuxgpiodv2: ignoring request for pull-down on %s: not supported by gpiod v%s\","}],"source_content_type":"text/x-csrc","patch_set":1,"id":"149f5e05_2f9c32cf","line":409,"updated":"2024-03-24 22:31:20.000000000","message":"and here","commit_id":"acd26894d3bf2ac36a1776947dd3e3c397b957b5"}]}