)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1001036,"name":"Andreas Bolsch","email":"hyphen0break@gmail.com","username":"abmero"},"change_message_id":"e3a4e84cf2b2f2f910891344b2644b4a10fbdec4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"57c421ce_e6569dac","updated":"2022-12-31 12:49:38.000000000","message":"For the two original points I do agree that not updating retval is certainly an error. For the other two occasions ignoring errors doesn\u0027t look nice, but it\u0027s still no real problem: If jtagspi_cmd fails, there would be no valid ID returned, or setting write enable would have failed, and that\u0027s detected anyway by reading the status register afterwards. ","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"4cd1a21e5e4e784bd2bd1ed0650b672c7234363f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"adeb150b_8e77d032","updated":"2022-12-30 16:10:21.000000000","message":"Thanks for the patch.\nIt really looks like the author of this driver missed that.\n\nIn other drivers we have sometimes like \"ignore returned error because system is not responsive after mass erase\".\nHere the only error returned by jtagspi_cmd() is the error from jtag_execute_queue(); I don\u0027t expect that any call to jtagspi_cmd() is ignoring the error on purpose, as mentioned above.\nWhat about the other two calls I highlighted below?\n\nHow did you identified this? By code analysis or did you fix some real problem?\nDo you think this should be merged in the current RC3 or can we wait for v0.12.0?","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001036,"name":"Andreas Bolsch","email":"hyphen0break@gmail.com","username":"abmero"},"change_message_id":"fa2b9897a33c9b6eedb1d579b97ca04143019c86","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7577174b_1395c9b1","in_reply_to":"187a7127_63072535","updated":"2023-01-02 12:18:08.000000000","message":"Right, this driver is lacking the required hack:\njtag_auto_probe should ignore the return value of jtagspi_probe and always return ERROR_OK. Just tried that with xc6slx16 and w25q256fv: 0xB9 send the flash to power-down, and afterwards re-attach of openocd (without power-on) gives error on flash probe (as expected). Now sending 0xAB wakes up the flash successfully and flash probe is successful then.\n\nThat\u0027s a dirty hack, no doubt. A clean solution would require to introduce separate interface_probed and memory_probed flags instead of a single probed flag, but that touches each and every flash driver ...","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001036,"name":"Andreas Bolsch","email":"hyphen0break@gmail.com","username":"abmero"},"change_message_id":"f3d381488d27135d5e38dbca24f541d8ac145883","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9d4b7e25_3511fc58","in_reply_to":"26477b8f_b201cd98","updated":"2023-01-03 12:51:56.000000000","message":"I think updating this one would be the simplest way, as it deals only with the return values (so same topic), too.","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"d27c7900407d057fdbd32c8c5aa6cd5130b3609f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"26477b8f_b201cd98","in_reply_to":"395c0d5f_52748f6b","updated":"2023-01-03 12:40:41.000000000","message":"Ok ok. At least we (I :)) need your proposed \"fix\" of \u0027jtagspi_auto_probe\u0027.\nShall I prepare a new patch or update this one?\n\nThanks for your help!","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001036,"name":"Andreas Bolsch","email":"hyphen0break@gmail.com","username":"abmero"},"change_message_id":"a90cc721b98413fedb44eb51ac0624c341496af7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"51f29616_3b6345c0","in_reply_to":"431de301_128b158e","updated":"2023-01-01 18:07:48.000000000","message":"There is a special command \"jtagspi cmd\" for configuration, sending the \"release from power-down\" via this command should help.","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"a0ddb7a34cab28b2651ab5e39ffbbc62ba7a3fd2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a679bed9_4de4d9f9","in_reply_to":"51f29616_3b6345c0","updated":"2023-01-01 19:13:41.000000000","message":"Good idea! but \u0027jtagspi cmd\u0027 does a \u0027probe\u0027 first and fails.","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"dfcf38593510c5d2bbeb587acf954e2c068451dd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7b99d283_55d7392f","in_reply_to":"7577174b_1395c9b1","updated":"2023-01-03 09:44:58.000000000","message":"Your solution worked, thanks. I tried on trion T20 with w25q32jv flash. It is as you mentioned - cumbersome. \nSpecially because the flash is not in \"unexpected state\" since the trion does put it in power down after reading the configuration.\nI wonder what is the drawback if we wake the flash up in probe function just before reading the id?","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001036,"name":"Andreas Bolsch","email":"hyphen0break@gmail.com","username":"abmero"},"change_message_id":"1015400ec81fa6dbd575a0d3917ae0a66386043e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"395c0d5f_52748f6b","in_reply_to":"7b99d283_55d7392f","updated":"2023-01-03 10:36:57.000000000","message":"Well, the command sets apart form read status reg. write enable/disable, and plain read are a real mess. The wake-up command seems to be 0xAB for some/most manufacturers, but there might be exceptions. And some (older or EEPROM) devices don\u0027t have a power-down feature at all. In theory unknown/unimplemented commands should be ignored, but that\u0027s not guaranteed. Some datasheets explicitly remark that undocumented commands could cause malfunction and are therefore not allowed.\nI.e. if you want to add an automatic wake-up feature, you must/should add an option to skip this as well ... \n\nApart from that: The flash might be in 3-byte address mode or 4-byte mode, in QPI mode or OPI mode (the latter two don\u0027t apply for this particular driver) or in XIP mode (where no command but only an address is send), 256-byte page or 512-byte page mode, uniform sector size or hybrid sector size mode, ..., either temporarily or via non-volatile setting. And some of these states do affect even the read id command (you won\u0027t believe it, but e.g. ATXP128 doesn\u0027t respond to this command while in OPI mode - just crazy). To deal with all these possible states for all supported devices automatically would be a real nightmare. Hence I\u0027m afraid it\u0027s necessary to deal with an \"unexpected\" case like the present one manually. And yes, this is indeed \"unexpected\", as this applies only to your particular hardware.","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"43846aaee70676002e0ae6755ca098e78b9262a5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"187a7127_63072535","in_reply_to":"86705f2a_4ac8f765","updated":"2023-01-02 01:18:38.000000000","message":"The call to the command handler \u0027flash_command_get_bank\u0027 on line 315 failed and the command is not be executed afterwards.","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"88c3a5baa167db22c41aa13e5e51d486ce446d71","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"af56d5f7_c3777998","in_reply_to":"9b6aeab0_dd985bce","updated":"2023-03-20 08:05:43.000000000","message":"And please split that change from adding return value checks","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"94510f2863a0b98ca254f32d31c65bccfc436d3e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b5409acc_b9ddc819","in_reply_to":"9d4b7e25_3511fc58","updated":"2023-01-03 23:19:35.000000000","message":"Uploaded the new patchset. Thanks again.\n\nDoes it make sense to add an optional parameter to \u0027jtagspi_init\u0027 (in jtagspi.cfg) which when set will send the command.\n\nI\u0027m sorry if this annoys you, but I wasted a lot of time with this problem - at least other don\u0027t have to do it again.","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001036,"name":"Andreas Bolsch","email":"hyphen0break@gmail.com","username":"abmero"},"change_message_id":"42ffde15115b79cc407d47b435148a07510c7638","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"86705f2a_4ac8f765","in_reply_to":"a679bed9_4de4d9f9","updated":"2023-01-01 22:23:35.000000000","message":"Yes, but it should execute anyway, and a manual probe afterward, i.e. \"flash probe \u003cbank_id\u003e\", would succeed then.\nSlightly cumbersome, yes, but that\u0027s what I always face when the flash is in an unexpected state ... \nI\u0027ve thought about a simple resolution several times, but on the other hand auto-probe is in most cases a useful feature.\nThat\u0027s why the \"cmd\" and \"set\" commands deliberately do NOT check info-\u003eprobed, sort of chicken-and-egg problem.","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"3011bd1e63b4c4f0e3ddd033d3cb054be689fdb6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"431de301_128b158e","in_reply_to":"adeb150b_8e77d032","updated":"2023-01-01 16:54:53.000000000","message":"You\u0027re welcome.\n\nI identified them by analyzing the code. I was searching another bug: if the spi flash is in power down mode, the probe function is not able to identify the flash chip. Don\u0027t know how to solve this yet. Maybe probe should always send \u0027release_from_power_down\u0027 before reading the id?\n\nThis fix here can even wait after v0.12.0.","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"3ebecd540926679d751ef77daca0a517a4805063","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f6c27a65_3f70c8bc","in_reply_to":"af56d5f7_c3777998","updated":"2023-03-25 23:55:09.000000000","message":"Changed the \"flash/jtagspi: sending command and setting parameters without probing.\" accordingly. Thanks for your insights.","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001036,"name":"Andreas Bolsch","email":"hyphen0break@gmail.com","username":"abmero"},"change_message_id":"9297d30ef6c12cbf2b4f9132d948aac74b1e5294","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c59847fe_7fdb3be7","in_reply_to":"b5409acc_b9ddc819","updated":"2023-01-04 13:47:00.000000000","message":"No problem ;-) Well, yes, this might be a good idea as long as this doesn\u0027t break anything and its purpose is documented there and in the docs.","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"6654568631e233546ed6cdb0d8b030e26ea74bf7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9b6aeab0_dd985bce","in_reply_to":"c59847fe_7fdb3be7","updated":"2023-03-20 08:03:54.000000000","message":"The autoprobe() hack to always return OK seems me too dirty.\n\nWe have flash_command_get_bank_maybe_probe() in flash/nor/tcl.c, unfortunately declared as static only. I propose to change the name _maybe_ to something better, e.g. _probe_optional and declare it globally in core.h\n\nflash_command_get_bank_probe_optional() can be used in jtagspi_handle_cmd() and jtagspi_handle_set(). Please add comment explaining the reasons.","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"1033ea4c84a7632c0de33a2e6602e994135c1221","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b9cf4899_b6f68c47","in_reply_to":"f6c27a65_3f70c8bc","updated":"2023-03-31 11:38:54.000000000","message":"Done","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"88340ef35c6aa1a35bb4eff7e6f8c7919be5f483","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"b61c663b_b769e7a3","updated":"2023-05-14 19:41:37.000000000","message":"thanks for you efforts, again!","commit_id":"b208ed716cd9650efee0c0353d5f4d5011a1a8a4"}],"src/flash/nor/jtagspi.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"4cd1a21e5e4e784bd2bd1ed0650b672c7234363f","unresolved":true,"context_lines":[{"line_number":387,"context_line":"\t}"},{"line_number":388,"context_line":"\tinfo-\u003etap \u003d bank-\u003etarget-\u003etap;"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"\tjtagspi_cmd(bank, SPIFLASH_READ_ID, NULL, 0, in_buf, -3);"},{"line_number":391,"context_line":"\t/* the table in spi.c has the manufacturer byte (first) as the lsb */"},{"line_number":392,"context_line":"\tid \u003d le_to_h_u24(in_buf);"},{"line_number":393,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"e45aa0d4_8cb149c9","line":390,"updated":"2022-12-30 16:10:21.000000000","message":"here","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"1033ea4c84a7632c0de33a2e6602e994135c1221","unresolved":false,"context_lines":[{"line_number":387,"context_line":"\t}"},{"line_number":388,"context_line":"\tinfo-\u003etap \u003d bank-\u003etarget-\u003etap;"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"\tjtagspi_cmd(bank, SPIFLASH_READ_ID, NULL, 0, in_buf, -3);"},{"line_number":391,"context_line":"\t/* the table in spi.c has the manufacturer byte (first) as the lsb */"},{"line_number":392,"context_line":"\tid \u003d le_to_h_u24(in_buf);"},{"line_number":393,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"2ceb0e73_e177b930","line":390,"in_reply_to":"e45aa0d4_8cb149c9","updated":"2023-03-31 11:38:54.000000000","message":"Done","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"4cd1a21e5e4e784bd2bd1ed0650b672c7234363f","unresolved":true,"context_lines":[{"line_number":491,"context_line":""},{"line_number":492,"context_line":"static int jtagspi_write_enable(struct flash_bank *bank)"},{"line_number":493,"context_line":"{"},{"line_number":494,"context_line":"\tjtagspi_cmd(bank, SPIFLASH_WRITE_ENABLE, NULL, 0, NULL, 0);"},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"\tuint32_t status \u003d (uint32_t)-1;"},{"line_number":497,"context_line":"\tint retval \u003d jtagspi_read_status(bank, \u0026status);"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"ced02583_a3894185","line":494,"updated":"2022-12-30 16:10:21.000000000","message":"here","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"1033ea4c84a7632c0de33a2e6602e994135c1221","unresolved":false,"context_lines":[{"line_number":491,"context_line":""},{"line_number":492,"context_line":"static int jtagspi_write_enable(struct flash_bank *bank)"},{"line_number":493,"context_line":"{"},{"line_number":494,"context_line":"\tjtagspi_cmd(bank, SPIFLASH_WRITE_ENABLE, NULL, 0, NULL, 0);"},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"\tuint32_t status \u003d (uint32_t)-1;"},{"line_number":497,"context_line":"\tint retval \u003d jtagspi_read_status(bank, \u0026status);"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"06532fe0_7c85b9f1","line":494,"in_reply_to":"ced02583_a3894185","updated":"2023-03-31 11:38:54.000000000","message":"Done","commit_id":"c3091a3d51df8bdb662d621409d0694091894198"}]}
