)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1001929,"name":"Doug Brunner","email":"doug.a.brunner@gmail.com","username":"doug-a-brunner"},"change_message_id":"939c2dbb07af1d1e83ebffd961e17c51ffe4195d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e2b59eda_e5984076","updated":"2021-11-06 23:10:27.000000000","message":"Updated to handle different device page sizes","commit_id":"e129b28d0f5be66d0df532a515e0da5de8aa8577"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"08cb2b2f0a256055ca249cf2ea10ec1d5363e65e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"a6059572_3c08f0bf","updated":"2021-11-08 16:46:41.000000000","message":"I don\u0027t have a device to test.\nIsn\u0027t it dangerous to make lockbits user writeable? Or is it intention? If it serves for very special purpose only I would prefer to comment out the bank definition.\nWhat happens if somebody issues \u0027flash protect\u0027 on lockbits or user bank? IMO driver should return error.","commit_id":"0a96e4a7065e7b62c803e583a6325ce1938dbc65"},{"author":{"_account_id":1001929,"name":"Doug Brunner","email":"doug.a.brunner@gmail.com","username":"doug-a-brunner"},"change_message_id":"3fd1e37a54a0b1c5b78674f41a8b7a097ca7fa50","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8e401932_843c1f55","in_reply_to":"40653be4_3f693378","updated":"2021-11-08 21:55:55.000000000","message":"FYI, this may take me a couple of days due to workload on other projects; as you might expect, the changes get more extensive to manage the lockbits appropriately for multiple flash banks.","commit_id":"0a96e4a7065e7b62c803e583a6325ce1938dbc65"},{"author":{"_account_id":1001929,"name":"Doug Brunner","email":"doug.a.brunner@gmail.com","username":"doug-a-brunner"},"change_message_id":"77244eba2d412c9f13f9fd213607dbeb2186ad24","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ea7c38ee_47482c13","in_reply_to":"8e401932_843c1f55","updated":"2021-11-12 02:35:11.000000000","message":"Done","commit_id":"0a96e4a7065e7b62c803e583a6325ce1938dbc65"},{"author":{"_account_id":1001929,"name":"Doug Brunner","email":"doug.a.brunner@gmail.com","username":"doug-a-brunner"},"change_message_id":"154948e4680d657b930dc98bea2b432f08b1236b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"40653be4_3f693378","in_reply_to":"a6059572_3c08f0bf","updated":"2021-11-08 21:28:23.000000000","message":"The lockbits page is also used for encryption keys by the SiLabs bootloader/AppLoader system (at least on the EFR32BG1B that we\u0027re using). To enable secure over-the-air updates, you need to be able to write these keys during device programming. I\u0027d guess this is a fairly common use case for the BLE-enabled devices. It does have the potential to be a footgun, though, as you point out; perhaps we prohibit writes to the bottom 512 bytes of the lockbits page (the ones that are actually used for locking other pages), and add appropriate handling to preserve those bytes when erasing the page to rewrite its upper bytes?\n\nGood point about the page protect operation; the existing routine would fail on the user and lockbits pages as implemented (if I\u0027m reading it right, it would end up setting protection on page 0 of the regular flash). I\u0027ll fix that. There are valid use cases for protecting the lockbits and user data pages, but if we support setting that protection, we should also add support for resetting it via the AAP registers (the reset operation clears the whole device). For now, I\u0027ll have it refuse to set that protection.","commit_id":"0a96e4a7065e7b62c803e583a6325ce1938dbc65"},{"author":{"_account_id":1001929,"name":"Doug Brunner","email":"doug.a.brunner@gmail.com","username":"doug-a-brunner"},"change_message_id":"77244eba2d412c9f13f9fd213607dbeb2186ad24","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"842a3785_1aa168cf","updated":"2021-11-12 02:35:11.000000000","message":"Adding Fredrik Hederstierna back to the attention set, since this new patch is a major change from the previous approach.","commit_id":"995e86bf54d3e593a78be606dc7370cf8a9e0810"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"3ba692f7c4f1e7e7b926865b9f02e2f0aec2f4b9","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"37bfd228_71ad958a","updated":"2021-11-30 07:13:16.000000000","message":"Doug,\nthe new change is much bigger than patchset 3. So please check it by clang static analyzer and valgrind as requested in https://openocd.org/doc/doxygen/html/patchguide.html\n\nI\u0027m not quite happy seeing just another linked-list implementation in a flash driver. Your implementation looks clean and correct, so I don\u0027t want to block the change for that reason.\n\nFYI Instead of keeping private list, it\u0027s possible to iterate over all flash banks and pick banks serviced by this driver. Also the efm32x_flash_bank structure is very short and no more than 3 instances can be created. See nrf5 driver for simplified chip/bank_priv allocation.\n\nAs I wrote I don\u0027t have a device to test, so I\u0027m waiting to Fredrik or somebody else who test it on hw.\n","commit_id":"995e86bf54d3e593a78be606dc7370cf8a9e0810"},{"author":{"_account_id":1001929,"name":"Doug Brunner","email":"doug.a.brunner@gmail.com","username":"doug-a-brunner"},"change_message_id":"f541669e2479426ffa6cf53bf80473412b4abcb3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"cd868d05_60636979","updated":"2021-11-29 22:18:52.000000000","message":"Fredrik/Tomas, do you need any more info from me for review?","commit_id":"995e86bf54d3e593a78be606dc7370cf8a9e0810"},{"author":{"_account_id":1001929,"name":"Doug Brunner","email":"doug.a.brunner@gmail.com","username":"doug-a-brunner"},"change_message_id":"520f799ea1925d73f64473c3469a23029e6a0a77","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"7f559d78_f16b7e39","in_reply_to":"37bfd228_71ad958a","updated":"2021-12-09 20:59:35.000000000","message":"I\u0027ll look at the nrf5 to see how that one works, didn\u0027t know it was possible to get all flash banks without keeping a list.\n\nFortunately it looks like USB forwarding over IP from Windows has just become available, so I can now test the build with Valgrind in a Linux VM.","commit_id":"995e86bf54d3e593a78be606dc7370cf8a9e0810"},{"author":{"_account_id":1001929,"name":"Doug Brunner","email":"doug.a.brunner@gmail.com","username":"doug-a-brunner"},"change_message_id":"62ec2b6a32f59cf046e9a1d6405b841112b4e972","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"09167b0a_ce538d35","updated":"2022-01-05 17:44:33.000000000","message":"Adding Tomas to the attention set for merge (apologies if this is the wrong process)","commit_id":"2a91240fde7aca18d040c7b311d1667260947087"},{"author":{"_account_id":1000697,"name":"Fredrik Hederstierna","email":"fredrik.hederstierna@gmail.com"},"change_message_id":"48297b1f8108bef4a34776132366b29e541fb6b4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"6b8416f0_a7bcd2e3","updated":"2021-12-15 17:49:56.000000000","message":"I just tested to erase and flash on my EFR32FG13P and it worked fine!","commit_id":"2a91240fde7aca18d040c7b311d1667260947087"},{"author":{"_account_id":1001929,"name":"Doug Brunner","email":"doug.a.brunner@gmail.com","username":"doug-a-brunner"},"change_message_id":"520f799ea1925d73f64473c3469a23029e6a0a77","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"300cd60f_4c533dde","updated":"2021-12-09 20:59:35.000000000","message":"Reimplemented as discussed.","commit_id":"2a91240fde7aca18d040c7b311d1667260947087"},{"author":{"_account_id":1001929,"name":"Doug Brunner","email":"doug.a.brunner@gmail.com","username":"doug-a-brunner"},"change_message_id":"6aaf5169a32a403bc7a7fbee5da97723dcef8118","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"f3c9489c_c36e2fa2","updated":"2021-12-21 23:45:27.000000000","message":"Sounds like all is in order, thanks for taking a look at it Fredrik. I\u0027ll take myself off the attention set for now since it appears we just need Tomas\u0027s review.","commit_id":"2a91240fde7aca18d040c7b311d1667260947087"},{"author":{"_account_id":1001929,"name":"Doug Brunner","email":"doug.a.brunner@gmail.com","username":"doug-a-brunner"},"change_message_id":"b3234eb800464c287b20d702068e08e37f8db4c3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"3179eecb_1e496b5a","updated":"2021-12-23 18:47:39.000000000","message":"Thanks Tomas, looks like ready to merge. Taking myself off the attention set for now.","commit_id":"2a91240fde7aca18d040c7b311d1667260947087"},{"author":{"_account_id":1000697,"name":"Fredrik Hederstierna","email":"fredrik.hederstierna@gmail.com"},"change_message_id":"f100079e49c0923f0c2f9a0aad34fef98bdebcb7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"373e79a7_2ce93415","updated":"2021-12-15 17:53:57.000000000","message":"The important thing is that this is in line with what I think are the next main upcoming feature requests on the efm32 driver, which I think is support for flashing the sometimes available bootloader back, and also support for the new EFR series-2 devices with Cortex-M33.","commit_id":"2a91240fde7aca18d040c7b311d1667260947087"}],"src/flash/nor/efm32.c":[{"author":{"_account_id":1000697,"name":"Fredrik Hederstierna","email":"fredrik.hederstierna@gmail.com"},"change_message_id":"a28bf619d5ff704453f66ffd206bad81ef9771d3","unresolved":true,"context_lines":[{"line_number":963,"context_line":"\t\tefm32_mcu_info.page_size;"},{"line_number":964,"context_line":""},{"line_number":965,"context_line":"\tif (bank-\u003esize \u003e 0) {"},{"line_number":966,"context_line":"\t\tif (bank-\u003esize % efm32_mcu_info.page_size !\u003d 0) {"},{"line_number":967,"context_line":"\t\t\tLOG_ERROR(\"Flash banks must be a multiple of %d bytes\","},{"line_number":968,"context_line":"\t\t\t\tefm32_mcu_info.page_size);"},{"line_number":969,"context_line":"\t\t\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"b836f31f_b8b58f01","line":966,"updated":"2021-11-06 01:03:25.000000000","message":"I think DEV_INFO also could be added, this bank is only 1kByte, so will give error if requesting 2kByte page size.","commit_id":"0673aab8796d30cfbd73431af9152a37db8f1fd0"},{"author":{"_account_id":1001929,"name":"Doug Brunner","email":"doug.a.brunner@gmail.com","username":"doug-a-brunner"},"change_message_id":"939c2dbb07af1d1e83ebffd961e17c51ffe4195d","unresolved":false,"context_lines":[{"line_number":963,"context_line":"\t\tefm32_mcu_info.page_size;"},{"line_number":964,"context_line":""},{"line_number":965,"context_line":"\tif (bank-\u003esize \u003e 0) {"},{"line_number":966,"context_line":"\t\tif (bank-\u003esize % efm32_mcu_info.page_size !\u003d 0) {"},{"line_number":967,"context_line":"\t\t\tLOG_ERROR(\"Flash banks must be a multiple of %d bytes\","},{"line_number":968,"context_line":"\t\t\t\tefm32_mcu_info.page_size);"},{"line_number":969,"context_line":"\t\t\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"ef156dbf_cec6eff0","line":966,"in_reply_to":"b836f31f_b8b58f01","updated":"2021-11-06 23:10:27.000000000","message":"The device info page seems to be read-only, so we don\u0027t need any support for it in the flash driver, right?\n\nGood catch on the devices with different page sizes, thanks. The user data and lockbits areas seem to be always 1 page, so perhaps we do a round up to page size here?","commit_id":"0673aab8796d30cfbd73431af9152a37db8f1fd0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"3ba692f7c4f1e7e7b926865b9f02e2f0aec2f4b9","unresolved":true,"context_lines":[{"line_number":378,"context_line":"}"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"/**"},{"line_number":381,"context_line":" * Remove flash structure corresponding to this bank, iff it\u0027s"},{"line_number":382,"context_line":" * not used by any others"},{"line_number":383,"context_line":" */"},{"line_number":384,"context_line":"static void efm32x_free_driver_priv(struct flash_bank *bank)"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"9942e71d_f3a0919b","line":381,"range":{"start_line":381,"start_character":54,"end_line":381,"end_character":57},"updated":"2021-11-30 07:13:16.000000000","message":"typo","commit_id":"995e86bf54d3e593a78be606dc7370cf8a9e0810"},{"author":{"_account_id":1001929,"name":"Doug Brunner","email":"doug.a.brunner@gmail.com","username":"doug-a-brunner"},"change_message_id":"520f799ea1925d73f64473c3469a23029e6a0a77","unresolved":false,"context_lines":[{"line_number":378,"context_line":"}"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"/**"},{"line_number":381,"context_line":" * Remove flash structure corresponding to this bank, iff it\u0027s"},{"line_number":382,"context_line":" * not used by any others"},{"line_number":383,"context_line":" */"},{"line_number":384,"context_line":"static void efm32x_free_driver_priv(struct flash_bank *bank)"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"dcc3e741_b701b99c","line":381,"range":{"start_line":381,"start_character":54,"end_line":381,"end_character":57},"in_reply_to":"9942e71d_f3a0919b","updated":"2021-12-09 20:59:35.000000000","message":"Abbreviation for \"if and only if\", but sounds like it is not universally understood. I\u0027ll fully spell it out in future.","commit_id":"995e86bf54d3e593a78be606dc7370cf8a9e0810"}]}