)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1002335,"name":"Conor Paxton","display_name":"con-pax","email":"conor.paxton@microchip.com","username":"con-pax","status":"Microchip"},"change_message_id":"3b601b0e2cbbfeb6f4af5ca5372af2a5b615eaae","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"2551c82a_e7797daf","updated":"2025-01-15 16:56:55.000000000","message":"Hi Tomas, thank you very much for taking the time to review the code. You are right, this driver is essentially a stripped version of the ftdi.c. In fact, we would love to \"just\" use the ftdi.c, however our debugger needs to send a signal to one of the channels on the ft4232h before it can work in mpsse mode. Because its such a specific thing to our debug probes, I did not want to pollute the ftdi.c with our command_handler. I could not see any other way of registering command handlers outside of the file, so I thought it would be suitable to implement our own driver. If you would have an alternative approach that we could implement, I would much appreciate it. I can add the original copyright to the file with a note at the end, if that\u0027s suitable?\nThere will be another member of my team taking the work forward so I will pass any review comments onto them.\n\nThanks again,\nConor","commit_id":"3dd7161e5c72b005b7bcaed0cdd2b2eb9ede7725"},{"author":{"_account_id":1002335,"name":"Conor Paxton","display_name":"con-pax","email":"conor.paxton@microchip.com","username":"con-pax","status":"Microchip"},"change_message_id":"654fdc7044db6867d9b01ead7026c50fde33f13f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"28500c55_003c3f34","updated":"2025-01-22 16:24:48.000000000","message":"Thanks Tomas,\nWe are working on a patch to remove the need for a separate driver for the FlashPro 5. its not ready yet, but we thought in the meantime, we should fix the commits that fail in Jenkins.\nCheers!\nConor","commit_id":"726c4e76ebfab6e40a6cc34e638675a6ddf21de1"}],"src/jtag/drivers/mchp_fp5_ftdi.c":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"bab7ed32f9deef00fc831f43760d796bd46b110c","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":"#ifdef HAVE_CONFIG_H"},{"line_number":4,"context_line":"#include \"config.h\""},{"line_number":5,"context_line":"#endif"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"923654bb_63d7a07a","line":2,"updated":"2024-12-18 12:53:15.000000000","message":"This file looks like ftdi.c with removed swd support and added an optional interface D init. We strictly refuse such code duplicity.\n\nMoreover the original copyright was removed, this is at least very unfair and probably GPL violation!","commit_id":"19a1abdc80884a21fd5f7258d84dcc918e9dfe3e"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"7eec3298bd9810918540da4a9ed4192f31039829","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":"#ifdef HAVE_CONFIG_H"},{"line_number":4,"context_line":"#include \"config.h\""},{"line_number":5,"context_line":"#endif"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"528d1d40_912b56df","line":2,"in_reply_to":"0ff88848_6955a5a6","updated":"2025-02-11 13:46:02.000000000","message":"Emma, Conor,\nthe patch quality is better but by far not good. It\u0027s no more totally unacceptable however I would call it quick and dirty hack.\n\n- the patch denies the basic principles of OpenOCD configuration (the device is contacted in configuration phase before the initialization phase starts)\n- an USB device with the first vid/pid pair is opened without care of USB location, serial number, other vid/pid pairs\n- seems me very disputable if setting bitmode and sending only one byte of data justifies the additional dependency of libftdi - could be easily done in mpsse code as well\n- command name `send_bank_signal` is misleading as it resembles ftdi driver `layout_signal`, `set_signal` etc... where signal means something quite different\n\nI\u0027m rather sceptic if we could make an acceptable patch this trial and error way. I\u0027m not going spend lot of time explaining OpenOCD basics. Please consider negotiating a contract with somebody from OpenOCD team to get the work done quickly and in accordance with the project principles.\n\nThanks for understanding","commit_id":"19a1abdc80884a21fd5f7258d84dcc918e9dfe3e"},{"author":{"_account_id":1002360,"name":"DavenportEmmaMCHP","email":"emma.davenport@microchip.com","username":"DavenportEmmaMCHP"},"change_message_id":"e8f1605895101c3ebc33ac32c07a055605b5279a","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":"#ifdef HAVE_CONFIG_H"},{"line_number":4,"context_line":"#include \"config.h\""},{"line_number":5,"context_line":"#endif"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"0ff88848_6955a5a6","line":2,"in_reply_to":"1a8880a3_4ce87396","updated":"2025-02-10 12:58:19.000000000","message":"Hi Tomas,\n\nThe patch has been updated. The new update keeps `send_bank_signal()` in a separate file to avoid the conflict I mentioned in my previous comment. If it is permissible to leave `send_bank_signal()` in a separate file, perfect, if not here is more information on the conflict mentioned previously.\n\nMoving `send_bank_signal()` to `ftdi.c` causes a conflict between the `ftdi_chip_type` enum in `mpsse.h` and `libftdi` outlined below.\n\n`libftdi`\n\n```\nenum ftdi_chip_type\n{\n    TYPE_AM\u003d0,\n    TYPE_BM\u003d1,\n    TYPE_2232C\u003d2,\n    TYPE_R\u003d3,\n    TYPE_2232H\u003d4,\n    TYPE_4232H\u003d5,\n    TYPE_232H\u003d6,\n    TYPE_230X\u003d7,\n};\n```\n\n`mpsse.h`\n\n```\nenum ftdi_chip_type {\n\tTYPE_FT2232C,\n\tTYPE_FT2232H,\n\tTYPE_FT4232H,\n\tTYPE_FT232H,\n\tTYPE_FT2233HP,\n\tTYPE_FT4233HP,\n\tTYPE_FT2232HP,\n\tTYPE_FT4232HP,\n\tTYPE_FT233HP,\n\tTYPE_FT232HP,\n\tTYPE_FT4232HA,\n};\n```\n\nI believe this is a fundamental incompatibility that will prevent `libftdi` from being included into the `mpsse` driver without changes to the driver.\n\nThanks for taking the time to review this","commit_id":"19a1abdc80884a21fd5f7258d84dcc918e9dfe3e"},{"author":{"_account_id":1002360,"name":"DavenportEmmaMCHP","email":"emma.davenport@microchip.com","username":"DavenportEmmaMCHP"},"change_message_id":"eee98e27d8f14e0a421c85b0e0e7b44e551cb50e","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":"#ifdef HAVE_CONFIG_H"},{"line_number":4,"context_line":"#include \"config.h\""},{"line_number":5,"context_line":"#endif"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"1666de55_cb99ece7","line":2,"in_reply_to":"528d1d40_912b56df","updated":"2025-02-12 12:37:41.000000000","message":"Hi Tomas,\n\nThanks for the review, I\u0027m new to developing for OpenOCD so I appreciated the time taken to give feedback. I\u0027ll spend some time implementing your points.\n\nThanks","commit_id":"19a1abdc80884a21fd5f7258d84dcc918e9dfe3e"},{"author":{"_account_id":1002335,"name":"Conor Paxton","display_name":"con-pax","email":"conor.paxton@microchip.com","username":"con-pax","status":"Microchip"},"change_message_id":"3b601b0e2cbbfeb6f4af5ca5372af2a5b615eaae","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":"#ifdef HAVE_CONFIG_H"},{"line_number":4,"context_line":"#include \"config.h\""},{"line_number":5,"context_line":"#endif"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"cf684a43_82d8f510","line":2,"in_reply_to":"923654bb_63d7a07a","updated":"2025-01-15 16:56:55.000000000","message":"Hi Tomas, thank you very much for taking the time to review the code. You are right, this driver is essentially a stripped version of the ftdi.c. In fact, we would love to \"just\" use the ftdi.c, however our debugger needs to send a signal to one of the channels on the ft4232h before it can work in mpsse mode. Because its such a specific thing to our debug probes, I did not want to pollute the ftdi.c with our command_handler. I could not see any other way of registering command handlers outside of the file, so I thought it would be suitable to implement our own driver. If you would have an alternative approach that we could implement, I would much appreciate it. I can add the original copyright to the file with a note at the end, if that\u0027s suitable?\nThere will be another member of my team taking the work forward so I will pass any review comments onto them.\n\nThanks again,\nConor","commit_id":"19a1abdc80884a21fd5f7258d84dcc918e9dfe3e"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"18d3306fca387477144ba74fd486ecf513b35571","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":"#ifdef HAVE_CONFIG_H"},{"line_number":4,"context_line":"#include \"config.h\""},{"line_number":5,"context_line":"#endif"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"1a8880a3_4ce87396","line":2,"in_reply_to":"a74654da_75d77e14","updated":"2025-02-06 16:02:33.000000000","message":"Hi Emma,\nplease upload the updated patch first, I have no idea what conflict you mean.\nGenerally speaking if `enum ftdi_chip_type` misses a FTDI chip then please add it. If you want to abuse the enum for some mchp specific info then please don\u0027t do so.","commit_id":"19a1abdc80884a21fd5f7258d84dcc918e9dfe3e"},{"author":{"_account_id":1002360,"name":"DavenportEmmaMCHP","email":"emma.davenport@microchip.com","username":"DavenportEmmaMCHP"},"change_message_id":"ca697af5819d93e68e4b59cf360751eca45334f0","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":"#ifdef HAVE_CONFIG_H"},{"line_number":4,"context_line":"#include \"config.h\""},{"line_number":5,"context_line":"#endif"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"a74654da_75d77e14","line":2,"in_reply_to":"b436a331_b71e5bad","updated":"2025-02-06 15:48:11.000000000","message":"Hi Tomas, I\u0027m taking over the work on this from Conor Paxton. \n\nI have a question regarding this patchset. In extending `ftdi.c` with the contents of `mchp_ftdi.c` `ftdi.h` will need to be included. The enum `ftdi_chip_type` is defined in the ftdi driver and `mpsse.h, and causes a conflict. Would you recommend the contents of `mchp_ftdi.c` stay separate or modify the mpsse driver?\n\nThanks","commit_id":"19a1abdc80884a21fd5f7258d84dcc918e9dfe3e"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"c7a40b15f7472ded0b115fd9516d18793a9eeaee","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":"#ifdef HAVE_CONFIG_H"},{"line_number":4,"context_line":"#include \"config.h\""},{"line_number":5,"context_line":"#endif"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"b436a331_b71e5bad","line":2,"in_reply_to":"cf684a43_82d8f510","updated":"2025-01-15 20:02:05.000000000","message":"Conor,\nthe OpenOCD is **Open** so please don\u0027t hesitate to propose changes to any part of it, including ftdi.c.\nIMO the cleanest solution would be to extend `ftdi.c` configuration to optionally handle other ft2232 and ft4232 ports (channels, interfaces) and then MCHP FP5 could be configured as any other ftdi based adapter in Tcl file.\nI didn\u0027t study mchp_ftdi.c to the depth. Seems me that extended `layout_init` command for other ports could be enough, couldn\u0027t it?","commit_id":"19a1abdc80884a21fd5f7258d84dcc918e9dfe3e"}],"src/jtag/drivers/mchp_ftdi.c":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"c7a40b15f7472ded0b115fd9516d18793a9eeaee","unresolved":true,"context_lines":[{"line_number":56,"context_line":"\t\terr \u003d ftdi_set_interface(\u0026ftdic, INTERFACE_D);"},{"line_number":57,"context_line":"\t\tif (err \u003c 0) {"},{"line_number":58,"context_line":"\t\t\tLOG_ERROR(\"ftdi_set_interface failed: %d (%s)\", err, ftdi_get_error_string(\u0026ftdic));"},{"line_number":59,"context_line":"\t\t\texit(0);"},{"line_number":60,"context_line":"\t\t}"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"\t\terr \u003d ftdi_usb_open_dev(\u0026ftdic, curdev-\u003edev);"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"1ad62426_ef3b02c2","line":59,"updated":"2025-01-15 20:02:05.000000000","message":"Please no `exit()` on error, just pass the error code to the caller.\n\nI know that two `exit()` are also in ftdi.c\nBut ftdi.c is old code and `exit()` guards an internal problem which never happens. Anyway we should replace it too.","commit_id":"3dd7161e5c72b005b7bcaed0cdd2b2eb9ede7725"}],"tcl/board/microchip_riscv_fp5.cfg":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"c7a40b15f7472ded0b115fd9516d18793a9eeaee","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":"# Microchip RISC-V board"},{"line_number":4,"context_line":"#"}],"source_content_type":"text/x-ttcn-cfg","patch_set":4,"id":"538ab845_3942c18c","line":1,"updated":"2025-01-15 20:02:05.000000000","message":"Same as Antonio commented here:\nhttps://review.openocd.org/c/openocd/+/8561/comment/88065817_4e103211/","commit_id":"3dd7161e5c72b005b7bcaed0cdd2b2eb9ede7725"}]}