)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"865ce5e7b31a1e54ed85b859a7be9ca63c9f186a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"9ccc9168_ebbec14e","updated":"2021-10-14 22:57:37.000000000","message":"During testing discovered that a romentry 0x00000002 value was getting treated as end-of-table marker.","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001651,"name":"Florian Fainelli","email":"f.fainelli@gmail.com","username":"ffainelli"},"change_message_id":"ca76bf92caa6dec55fdf24e8f6d6d2eff9c8edc6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"377f1e9d_d2a05aea","updated":"2021-10-21 23:04:14.000000000","message":"Fetched the whole series with:\n\ngit fetch https://review.openocd.org/openocd refs/changes/65/6465/1 \u0026\u0026 git checkout -b change-6465 FETCH_HEAD\n\nand it works as expected on my Cortex-A76-based system:\n\n\u003e dap info\nAP ID register 0x24770002\n        Type is MEM-AP APB2 or APB3\nMEM-AP BASE 0x80080003\n        Valid ROM table present\n                Component base address 0x80080000\n                Peripheral ID 0x01000bfa97\n                Designer is 0x0bf, Broadcom\n                Part is 0xa97, Unrecognized\n                Component class is 0x1, ROM table\n                MEMTYPE system memory not present: dedicated debug bus\n        ROMTABLE[0x0] \u003d 0xfff80003\n                Component base address 0x80000000\n                Peripheral ID 0x04007bb4e4\n                Designer is 0x23b, ARM Ltd\n                Part is 0x4e4, Cortex-A76 ROM (ROM Table)\n                Component class is 0x9, CoreSight component\n                Type is 0x00, Miscellaneous, other\n                Dev Arch is 0x47700af7, ARM Ltd \"CoreSight ROM architecture\" r.0\n        [L01] ROMTABLE[0x0] \u003d 0x10003\n                Component base address 0x80010000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x15, Debug Logic, Processor\n                Dev Arch is 0x47708a15, ARM Ltd \"Processor debug architecture (v8.2-A)\" r.0\n        [L01] ROMTABLE[0x4] \u003d 0x20003\n                Component base address 0x80020000\n                Peripheral ID 0x04007bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x14, Debug Control, Trigger Matrix\n                Dev Arch is 0x47701a14, ARM Ltd \"Cross Trigger Interface (CTI) architecture\" r.0\n        [L01] ROMTABLE[0x8] \u003d 0x30003\n                Component base address 0x80030000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x16, Performance Monitor, Processor\n                Dev Arch is 0x47702a16, ARM Ltd \"Processor Performance Monitor (PMU) architecture\" r.0\n        [L01] ROMTABLE[0xc] \u003d 0x40003\n                Component base address 0x80040000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x13, Trace Source, Processor\n                Dev Arch is 0x47724a13, ARM Ltd \"Embedded Trace Macrocell (ETM) architecture\" r.2\n        [L01] ROMTABLE[0x10] \u003d 0xc0002\n                Component not present\n        [L01] ROMTABLE[0x14] \u003d 0xd0006\n                Component not present\n        [L01] ROMTABLE[0x18] \u003d 0xe0006\n                Component not present\n        [L01] ROMTABLE[0x1c] \u003d 0x110003\n                Component base address 0x80110000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x15, Debug Logic, Processor\n                Dev Arch is 0x47708a15, ARM Ltd \"Processor debug architecture (v8.2-A)\" r.0\n        [L01] ROMTABLE[0x20] \u003d 0x120003\n                Component base address 0x80120000\n                Peripheral ID 0x04007bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x14, Debug Control, Trigger Matrix\n                Dev Arch is 0x47701a14, ARM Ltd \"Cross Trigger Interface (CTI) architecture\" r.0\n        [L01] ROMTABLE[0x24] \u003d 0x130003\n                Component base address 0x80130000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x16, Performance Monitor, Processor\n                Dev Arch is 0x47702a16, ARM Ltd \"Processor Performance Monitor (PMU) architecture\" r.0\n        [L01] ROMTABLE[0x28] \u003d 0x140003\n                Component base address 0x80140000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x13, Trace Source, Processor\n                Dev Arch is 0x47724a13, ARM Ltd \"Embedded Trace Macrocell (ETM) architecture\" r.2\n        [L01] ROMTABLE[0x2c] \u003d 0x1c0002\n                Component not present\n        [L01] ROMTABLE[0x30] \u003d 0x210003\n                Component base address 0x80210000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x15, Debug Logic, Processor\n                Dev Arch is 0x47708a15, ARM Ltd \"Processor debug architecture (v8.2-A)\" r.0\n        [L01] ROMTABLE[0x34] \u003d 0x220003\n                Component base address 0x80220000\n                Peripheral ID 0x04007bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x14, Debug Control, Trigger Matrix\n                Dev Arch is 0x47701a14, ARM Ltd \"Cross Trigger Interface (CTI) architecture\" r.0\n        [L01] ROMTABLE[0x38] \u003d 0x230003\n                Component base address 0x80230000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x16, Performance Monitor, Processor\n                Dev Arch is 0x47702a16, ARM Ltd \"Processor Performance Monitor (PMU) architecture\" r.0\n        [L01] ROMTABLE[0x3c] \u003d 0x240003\n                Component base address 0x80240000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x13, Trace Source, Processor\n                Dev Arch is 0x47724a13, ARM Ltd \"Embedded Trace Macrocell (ETM) architecture\" r.2\n        [L01] ROMTABLE[0x40] \u003d 0x2c0002\n                Component not present\n        [L01] ROMTABLE[0x44] \u003d 0x310003\n                Component base address 0x80310000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x15, Debug Logic, Processor\n                Dev Arch is 0x47708a15, ARM Ltd \"Processor debug architecture (v8.2-A)\" r.0\n        [L01] ROMTABLE[0x48] \u003d 0x320003\n                Component base address 0x80320000\n                Peripheral ID 0x04007bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x14, Debug Control, Trigger Matrix\n                Dev Arch is 0x47701a14, ARM Ltd \"Cross Trigger Interface (CTI) architecture\" r.0\n        [L01] ROMTABLE[0x4c] \u003d 0x330003\n                Component base address 0x80330000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x16, Performance Monitor, Processor\n                Dev Arch is 0x47702a16, ARM Ltd \"Processor Performance Monitor (PMU) architecture\" r.0\n        [L01] ROMTABLE[0x50] \u003d 0x340003\n                Component base address 0x80340000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x23b, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x13, Trace Source, Processor\n                Dev Arch is 0x47724a13, ARM Ltd \"Embedded Trace Macrocell (ETM) architecture\" r.2\n        [L01] ROMTABLE[0x54] \u003d 0x3c0002\n                Component not present\n        [L01] ROMTABLE[0x58] \u003d 0x0\n        [L01]   End of ROM table\n        ROMTABLE[0x4] \u003d 0x0\n                End of ROM table\n\n","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"e50b26dbec66fa8321634ba98416851256571808","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4904031d_efb0c8e9","updated":"2021-10-14 19:48:46.000000000","message":"I have a concern with the interpretation of romentry with respect to printing of the \"Component base address\". This was discovered during testing.","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001651,"name":"Florian Fainelli","email":"f.fainelli@gmail.com","username":"ffainelli"},"change_message_id":"f447543f9834700f94e2cc2d14ece477c7755ef4","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"091b034e_b958b6f0","updated":"2021-10-20 22:06:13.000000000","message":"It was a bit hard to chose a specific commit to add the comment to since I have been sort of out of the ADI v6 patch series. On a Cortex-A76 system, using v0.11.0-418-g918811529691, this is what I am getting:\n\n\u003e dap info\nAP ID register 0x24770002\n        Type is MEM-AP APB2 or APB3\nMEM-AP BASE 0x80080003\n        Valid ROM table present\n                Component base address 0x80080000\n                Peripheral ID 0x01000bfa97\n                Designer is 0x0bf, Broadcom\n                Part is 0xa97, Unrecognized\n                Component class is 0x1, ROM table\n                MEMTYPE system memory not present: dedicated debug bus\n        ROMTABLE[0x0] \u003d 0xfff80003\n                Component base address 0x80000000\n                Peripheral ID 0x04007bb4e4\n                Designer is 0x23b, ARM Ltd\n                Part is 0x4e4, Cortex-A76 ROM (ROM Table)\n                Component class is 0x9, CoreSight component\n                Type is 0x00, Miscellaneous, other\n        ROMTABLE[0x4] \u003d 0x0\n                End of ROM table\n\nwhereas I would expect the following:\n\n\u003e dap info\nAP ID register 0x24770002\n        Type is MEM-AP APB\nMEM-AP BASE 0x80080003\n        Valid ROM table present\n                Component base address 0x80080000\n                Peripheral ID 0x01000bfa97\n                Designer is 0x1bf, Broadcom\n                Part is 0xa97, Unrecognized\n                Component class is 0x1, ROM table\n                MEMTYPE system memory not present: dedicated debug bus\n        ROMTABLE[0x0] \u003d 0xfff80003\n                Component base address 0x180000000\n                Peripheral ID 0x04007bb4e4\n                Designer is 0x4bb, ARM Ltd\n                Part is 0x4e4, Cortex-A76 ROM (ROM Table)\n                Component class is 0x9, CoreSight component\n                Type is 0x00, Miscellaneous, other\n        [L01] ROMTABLE[0x0] \u003d 0x10003\n                Component base address 0x180010000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x15, Debug Logic, Processor\n        [L01] ROMTABLE[0x4] \u003d 0x20003\n                Component base address 0x180020000\n                Peripheral ID 0x04007bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x14, Debug Control, Trigger Matrix\n        [L01] ROMTABLE[0x8] \u003d 0x30003\n                Component base address 0x180030000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x16, Performance Monitor, Processor\n        [L01] ROMTABLE[0xc] \u003d 0x40003\n                Component base address 0x180040000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x13, Trace Source, Processor\n        [L01] ROMTABLE[0x10] \u003d 0xc0002\n                Component base address 0x1800c0000\n                Invalid CID 0x00000000\n        [L01] ROMTABLE[0x14] \u003d 0xd0006\n                Component base address 0x1800d0000\n                Invalid CID 0x00000000\n        [L01] ROMTABLE[0x18] \u003d 0xe0006\n                Component base address 0x1800e0000\n                Invalid CID 0x00000000\n        [L01] ROMTABLE[0x1c] \u003d 0x110003\n                Component base address 0x180110000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x15, Debug Logic, Processor\n        [L01] ROMTABLE[0x20] \u003d 0x120003\n                Component base address 0x180120000\n                Peripheral ID 0x04007bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x14, Debug Control, Trigger Matrix\n        [L01] ROMTABLE[0x24] \u003d 0x130003\n                Component base address 0x180130000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x16, Performance Monitor, Processor\n        [L01] ROMTABLE[0x28] \u003d 0x140003\n                Component base address 0x180140000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x13, Trace Source, Processor\n        [L01] ROMTABLE[0x2c] \u003d 0x1c0002\n                Component base address 0x1801c0000\n                Invalid CID 0x00000000\n        [L01] ROMTABLE[0x30] \u003d 0x210003\n                Component base address 0x180210000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x15, Debug Logic, Processor\n        [L01] ROMTABLE[0x34] \u003d 0x220003\n                Component base address 0x180220000\n                Peripheral ID 0x04007bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x14, Debug Control, Trigger Matrix\n        [L01] ROMTABLE[0x38] \u003d 0x230003\n                Component base address 0x180230000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x16, Performance Monitor, Processor\n        [L01] ROMTABLE[0x3c] \u003d 0x240003\n                Component base address 0x180240000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x13, Trace Source, Processor\n        [L01] ROMTABLE[0x40] \u003d 0x2c0002\n                Component base address 0x1802c0000\n                Invalid CID 0x00000000\n        [L01] ROMTABLE[0x44] \u003d 0x310003\n                Component base address 0x180310000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x15, Debug Logic, Processor\n        [L01] ROMTABLE[0x48] \u003d 0x320003\n                Component base address 0x180320000\n                Peripheral ID 0x04007bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x14, Debug Control, Trigger Matrix\n        [L01] ROMTABLE[0x4c] \u003d 0x330003\n                Component base address 0x180330000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x16, Performance Monitor, Processor\n        [L01] ROMTABLE[0x50] \u003d 0x340003\n                Component base address 0x180340000\n                Peripheral ID 0x04005bbd0b\n                Designer is 0x4bb, ARM Ltd\n                Part is 0xd0b, Cortex-A76 Debug (Debug Unit)\n                Component class is 0x9, CoreSight component\n                Type is 0x13, Trace Source, Processor\n        [L01] ROMTABLE[0x54] \u003d 0x3c0002\n                Component base address 0x1803c0000\n                Invalid CID 0x00000000\n        [L01] ROMTABLE[0x58] \u003d 0x0\n        [L01]   End of ROM table\n        ROMTABLE[0x4] \u003d 0x0\n                End of ROM table\n\n\nI don\u0027t think I will have much time for reviewing your changes, but testing would definitively be fine.","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"86a1c9238fdb279ff81df6c54198f3ec5b42139b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8e60bb92_7ddc05c4","updated":"2021-10-08 00:39:48.000000000","message":"Performed code inspection. Some comments...","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"514a89909c07daeec3e8fdd381c55d0e72e07c9a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4316c47c_02b06901","updated":"2021-10-14 20:07:20.000000000","message":"additional comment...","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c09008f406ff94c8e98a6274f45cc530974e7de6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"76edea66_66081f51","updated":"2021-10-13 21:23:54.000000000","message":"thanks, will send a v2","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"45445a54708ef6ae0d8c9be67db1830314878cfd","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ed6ff113_36deaa88","in_reply_to":"091b034e_b958b6f0","updated":"2021-10-20 22:58:31.000000000","message":"Florian, version \"v0.11.0-418-g918811529691\" only includes the \"merged\" commits in the patch relation chain. The ADIv6 specific updates haven\u0027t been approved and merged yet. Thus I wouldn\u0027t expect a \"dap info\" command for an ADIv6 DAP to work with commit hash 918811529. Can you test the commits in the relation chain for this patch series? This patch series contains all ADIv6 needed code updates.\n\nAlso, until V2 of the code is available for the unmerged patches, please note the review comments. They may include code updates needed for your platform.","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001651,"name":"Florian Fainelli","email":"f.fainelli@gmail.com","username":"ffainelli"},"change_message_id":"c4b7ebdb1f8e64fdad08a693fdba3e05e7fff3e5","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f184b438_9c9f9d41","in_reply_to":"ed6ff113_36deaa88","updated":"2021-10-20 23:11:54.000000000","message":"\u003e Florian, version \"v0.11.0-418-g918811529691\" only includes the \"merged\" commits in the patch relation chain.\n\nYes, sorry for not being clear, that was intentional, yet somehow I thought that the merged commits would be enough, that tells you how much I have been out of the review, sorry about that.\n\n\u003e The ADIv6 specific updates haven\u0027t been approved and merged yet. Thus I wouldn\u0027t expect a \"dap info\" command for an ADIv6 DAP to work with commit hash 918811529. Can you test the commits in the relation chain for this patch series? This patch series contains all ADIv6 needed code updates.\n\u003e \n\u003e Also, until V2 of the code is available for the unmerged patches, please note the review comments. They may include code updates needed for your platform.\n\nThanks, let me grab the remainder of the changes and test them before making further comments, otherwise that\u0027s not helpful for anyone.","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001651,"name":"Florian Fainelli","email":"f.fainelli@gmail.com","username":"ffainelli"},"change_message_id":"625f777668617aebfb99ae229891c17945ab30e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ad111db4_4b891f50","updated":"2022-04-27 20:06:11.000000000","message":"Antonio, is there anything blocking the inclusion of this change set?","commit_id":"8f4c5fe905be92af12b496cb1ea154842eae4b5b"}],"src/target/arm_adi_v5.c":[{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"86a1c9238fdb279ff81df6c54198f3ec5b42139b","unresolved":true,"context_lines":[{"line_number":1183,"context_line":"int dap_lookup_cs_component(struct adiv5_ap *ap,"},{"line_number":1184,"context_line":"\t\t\ttarget_addr_t dbgbase, uint8_t type, target_addr_t *addr, int32_t *idx)"},{"line_number":1185,"context_line":"{"},{"line_number":1186,"context_line":"\tuint32_t devid_reg, cidr1, romentry, devtype;"},{"line_number":1187,"context_line":"\tunsigned int entry_offset \u003d 0, max_entry_offset;"},{"line_number":1188,"context_line":"\ttarget_addr_t component_base;"},{"line_number":1189,"context_line":"\tbool rom_entries_64bits;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"6ed5438f_c0468bf4","line":1186,"range":{"start_line":1186,"start_character":28,"end_line":1186,"end_character":36},"updated":"2021-10-08 00:39:48.000000000","message":"\"romentry\" cannot be type uint32_t for 64 bit ROM entries. Since negative offset values are permitted using two\u0027s complement, should this be type int64_t? \"romentry\" is declared in dap_rom_display() as int64_t which to me makes the most sense.","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c09008f406ff94c8e98a6274f45cc530974e7de6","unresolved":false,"context_lines":[{"line_number":1183,"context_line":"int dap_lookup_cs_component(struct adiv5_ap *ap,"},{"line_number":1184,"context_line":"\t\t\ttarget_addr_t dbgbase, uint8_t type, target_addr_t *addr, int32_t *idx)"},{"line_number":1185,"context_line":"{"},{"line_number":1186,"context_line":"\tuint32_t devid_reg, cidr1, romentry, devtype;"},{"line_number":1187,"context_line":"\tunsigned int entry_offset \u003d 0, max_entry_offset;"},{"line_number":1188,"context_line":"\ttarget_addr_t component_base;"},{"line_number":1189,"context_line":"\tbool rom_entries_64bits;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c3ea02d8_f99fce4b","line":1186,"range":{"start_line":1186,"start_character":28,"end_line":1186,"end_character":36},"in_reply_to":"6ed5438f_c0468bf4","updated":"2021-10-13 21:23:54.000000000","message":"Ack","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"86a1c9238fdb279ff81df6c54198f3ec5b42139b","unresolved":true,"context_lines":[{"line_number":1190,"context_line":"\tint retval;"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"\tretval \u003d mem_ap_read_atomic_u32(ap, dbgbase + ARM_CS_C9_DEVID, \u0026devid_reg);"},{"line_number":1193,"context_line":"\tif (retval \u003d\u003d ERROR_OK)"},{"line_number":1194,"context_line":"\t\treturn retval;"},{"line_number":1195,"context_line":"\tretval \u003d mem_ap_read_atomic_u32(ap, dbgbase + ARM_CS_CIDR1, \u0026cidr1);"},{"line_number":1196,"context_line":"\tif (retval !\u003d ERROR_OK)"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"895ca191_9160f09c","line":1193,"range":{"start_line":1193,"start_character":12,"end_line":1193,"end_character":14},"updated":"2021-10-08 00:39:48.000000000","message":"!\u003d if the next line is \"return retval\".","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c09008f406ff94c8e98a6274f45cc530974e7de6","unresolved":false,"context_lines":[{"line_number":1190,"context_line":"\tint retval;"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"\tretval \u003d mem_ap_read_atomic_u32(ap, dbgbase + ARM_CS_C9_DEVID, \u0026devid_reg);"},{"line_number":1193,"context_line":"\tif (retval \u003d\u003d ERROR_OK)"},{"line_number":1194,"context_line":"\t\treturn retval;"},{"line_number":1195,"context_line":"\tretval \u003d mem_ap_read_atomic_u32(ap, dbgbase + ARM_CS_CIDR1, \u0026cidr1);"},{"line_number":1196,"context_line":"\tif (retval !\u003d ERROR_OK)"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"18a724a9_7e5830c6","line":1193,"range":{"start_line":1193,"start_character":12,"end_line":1193,"end_character":14},"in_reply_to":"895ca191_9160f09c","updated":"2021-10-13 21:23:54.000000000","message":"Ack","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"86a1c9238fdb279ff81df6c54198f3ec5b42139b","unresolved":true,"context_lines":[{"line_number":1222,"context_line":"\t\t\t\treturn retval;"},{"line_number":1223,"context_line":"\t\t\tentry_offset +\u003d 4;"},{"line_number":1224,"context_line":""},{"line_number":1225,"context_line":"\t\t\tromentry \u003d (((uint64_t)romentry_high) \u003c\u003c 32) | romentry_low;"},{"line_number":1226,"context_line":"\t\t\tromentry \u0026\u003d ~0x0fffULL;"},{"line_number":1227,"context_line":"\t\t} else {"},{"line_number":1228,"context_line":"\t\t\tromentry \u003d (int32_t)romentry_low;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"1d3d3aee_fc0c145c","line":1225,"range":{"start_line":1225,"start_character":13,"end_line":1225,"end_character":14},"updated":"2021-10-08 00:39:48.000000000","message":"If romentry is declared as a \"int64_t\", add typecast...\nexample: romentry \u003d (int64_t)((((uint64_t)romentry_high) \u003c\u003c 32) | romentry_low);","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"86a1c9238fdb279ff81df6c54198f3ec5b42139b","unresolved":true,"context_lines":[{"line_number":1222,"context_line":"\t\t\t\treturn retval;"},{"line_number":1223,"context_line":"\t\t\tentry_offset +\u003d 4;"},{"line_number":1224,"context_line":""},{"line_number":1225,"context_line":"\t\t\tromentry \u003d (((uint64_t)romentry_high) \u003c\u003c 32) | romentry_low;"},{"line_number":1226,"context_line":"\t\t\tromentry \u0026\u003d ~0x0fffULL;"},{"line_number":1227,"context_line":"\t\t} else {"},{"line_number":1228,"context_line":"\t\t\tromentry \u003d (int32_t)romentry_low;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"ec2a2513_584fa1f3","line":1225,"range":{"start_line":1225,"start_character":3,"end_line":1225,"end_character":11},"updated":"2021-10-08 00:39:48.000000000","message":"romentry is currently declared as a \"uint32_t\".","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"9954f1dbd4eda7aabe86d60e396daab502a036e6","unresolved":false,"context_lines":[{"line_number":1222,"context_line":"\t\t\t\treturn retval;"},{"line_number":1223,"context_line":"\t\t\tentry_offset +\u003d 4;"},{"line_number":1224,"context_line":""},{"line_number":1225,"context_line":"\t\t\tromentry \u003d (((uint64_t)romentry_high) \u003c\u003c 32) | romentry_low;"},{"line_number":1226,"context_line":"\t\t\tromentry \u0026\u003d ~0x0fffULL;"},{"line_number":1227,"context_line":"\t\t} else {"},{"line_number":1228,"context_line":"\t\t\tromentry \u003d (int32_t)romentry_low;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"96b09b81_ecc8beff","line":1225,"range":{"start_line":1225,"start_character":13,"end_line":1225,"end_character":14},"in_reply_to":"109dc8c2_533ee9d2","updated":"2021-10-14 05:31:47.000000000","message":"After thinking about this more, the (int64_t) typecast is redundant since it gets implicitly handled. I think the original code is fine as is.","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c09008f406ff94c8e98a6274f45cc530974e7de6","unresolved":false,"context_lines":[{"line_number":1222,"context_line":"\t\t\t\treturn retval;"},{"line_number":1223,"context_line":"\t\t\tentry_offset +\u003d 4;"},{"line_number":1224,"context_line":""},{"line_number":1225,"context_line":"\t\t\tromentry \u003d (((uint64_t)romentry_high) \u003c\u003c 32) | romentry_low;"},{"line_number":1226,"context_line":"\t\t\tromentry \u0026\u003d ~0x0fffULL;"},{"line_number":1227,"context_line":"\t\t} else {"},{"line_number":1228,"context_line":"\t\t\tromentry \u003d (int32_t)romentry_low;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"109dc8c2_533ee9d2","line":1225,"range":{"start_line":1225,"start_character":13,"end_line":1225,"end_character":14},"in_reply_to":"1d3d3aee_fc0c145c","updated":"2021-10-13 21:23:54.000000000","message":"Ack","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c09008f406ff94c8e98a6274f45cc530974e7de6","unresolved":false,"context_lines":[{"line_number":1222,"context_line":"\t\t\t\treturn retval;"},{"line_number":1223,"context_line":"\t\t\tentry_offset +\u003d 4;"},{"line_number":1224,"context_line":""},{"line_number":1225,"context_line":"\t\t\tromentry \u003d (((uint64_t)romentry_high) \u003c\u003c 32) | romentry_low;"},{"line_number":1226,"context_line":"\t\t\tromentry \u0026\u003d ~0x0fffULL;"},{"line_number":1227,"context_line":"\t\t} else {"},{"line_number":1228,"context_line":"\t\t\tromentry \u003d (int32_t)romentry_low;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c164710b_1efacd5a","line":1225,"range":{"start_line":1225,"start_character":3,"end_line":1225,"end_character":11},"in_reply_to":"ec2a2513_584fa1f3","updated":"2021-10-13 21:23:54.000000000","message":"Ack","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"86a1c9238fdb279ff81df6c54198f3ec5b42139b","unresolved":true,"context_lines":[{"line_number":1225,"context_line":"\t\t\tromentry \u003d (((uint64_t)romentry_high) \u003c\u003c 32) | romentry_low;"},{"line_number":1226,"context_line":"\t\t\tromentry \u0026\u003d ~0x0fffULL;"},{"line_number":1227,"context_line":"\t\t} else {"},{"line_number":1228,"context_line":"\t\t\tromentry \u003d (int32_t)romentry_low;"},{"line_number":1229,"context_line":"\t\t}"},{"line_number":1230,"context_line":""},{"line_number":1231,"context_line":"\t\tcomponent_base \u003d dbgbase + (target_addr_t)(romentry \u0026 0xFFFFF000);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"a90d5730_d2c55874","line":1228,"range":{"start_line":1228,"start_character":3,"end_line":1228,"end_character":36},"updated":"2021-10-08 00:39:48.000000000","message":"If romentry is declared as a \"int64_t\", add typecast...\nexample: romentry \u003d (int64_t)(romentry_low \u0026 0xFFFFF000);","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c09008f406ff94c8e98a6274f45cc530974e7de6","unresolved":false,"context_lines":[{"line_number":1225,"context_line":"\t\t\tromentry \u003d (((uint64_t)romentry_high) \u003c\u003c 32) | romentry_low;"},{"line_number":1226,"context_line":"\t\t\tromentry \u0026\u003d ~0x0fffULL;"},{"line_number":1227,"context_line":"\t\t} else {"},{"line_number":1228,"context_line":"\t\t\tromentry \u003d (int32_t)romentry_low;"},{"line_number":1229,"context_line":"\t\t}"},{"line_number":1230,"context_line":""},{"line_number":1231,"context_line":"\t\tcomponent_base \u003d dbgbase + (target_addr_t)(romentry \u0026 0xFFFFF000);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c408a7e3_4b92c4ed","line":1228,"range":{"start_line":1228,"start_character":3,"end_line":1228,"end_character":36},"in_reply_to":"a90d5730_d2c55874","updated":"2021-10-13 21:23:54.000000000","message":"Ack","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"9954f1dbd4eda7aabe86d60e396daab502a036e6","unresolved":false,"context_lines":[{"line_number":1225,"context_line":"\t\t\tromentry \u003d (((uint64_t)romentry_high) \u003c\u003c 32) | romentry_low;"},{"line_number":1226,"context_line":"\t\t\tromentry \u0026\u003d ~0x0fffULL;"},{"line_number":1227,"context_line":"\t\t} else {"},{"line_number":1228,"context_line":"\t\t\tromentry \u003d (int32_t)romentry_low;"},{"line_number":1229,"context_line":"\t\t}"},{"line_number":1230,"context_line":""},{"line_number":1231,"context_line":"\t\tcomponent_base \u003d dbgbase + (target_addr_t)(romentry \u0026 0xFFFFF000);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"2f468416_bbaea08a","line":1228,"range":{"start_line":1228,"start_character":3,"end_line":1228,"end_character":36},"in_reply_to":"c408a7e3_4b92c4ed","updated":"2021-10-14 05:31:47.000000000","message":"Hum... After thinking about this more, it probably needs to be left as (int32_t) to handle the signed ROMENTRY OFFSET since negative values are permitted using two\u0027s complement.","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"86a1c9238fdb279ff81df6c54198f3ec5b42139b","unresolved":true,"context_lines":[{"line_number":1228,"context_line":"\t\t\tromentry \u003d (int32_t)romentry_low;"},{"line_number":1229,"context_line":"\t\t}"},{"line_number":1230,"context_line":""},{"line_number":1231,"context_line":"\t\tcomponent_base \u003d dbgbase + (target_addr_t)(romentry \u0026 0xFFFFF000);"},{"line_number":1232,"context_line":""},{"line_number":1233,"context_line":"\t\tif (romentry_low \u0026 ARM_CS_ROMENTRY_PRESENT) {"},{"line_number":1234,"context_line":"\t\t\tuint32_t c_cid1;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"888619c5_9745da60","line":1231,"range":{"start_line":1231,"start_character":56,"end_line":1231,"end_character":66},"updated":"2021-10-08 00:39:48.000000000","message":"For 64 bit ROM entries, mask of 0xFFFFF000 truncates the upper 32 bits. I think moving the mask to the above \"else\" part for 32bit ROM entries and removing the mask here makes the most sense.","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c09008f406ff94c8e98a6274f45cc530974e7de6","unresolved":false,"context_lines":[{"line_number":1228,"context_line":"\t\t\tromentry \u003d (int32_t)romentry_low;"},{"line_number":1229,"context_line":"\t\t}"},{"line_number":1230,"context_line":""},{"line_number":1231,"context_line":"\t\tcomponent_base \u003d dbgbase + (target_addr_t)(romentry \u0026 0xFFFFF000);"},{"line_number":1232,"context_line":""},{"line_number":1233,"context_line":"\t\tif (romentry_low \u0026 ARM_CS_ROMENTRY_PRESENT) {"},{"line_number":1234,"context_line":"\t\t\tuint32_t c_cid1;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"9e83310b_cdb15152","line":1231,"range":{"start_line":1231,"start_character":56,"end_line":1231,"end_character":66},"in_reply_to":"888619c5_9745da60","updated":"2021-10-13 21:23:54.000000000","message":"Ack","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"86a1c9238fdb279ff81df6c54198f3ec5b42139b","unresolved":true,"context_lines":[{"line_number":1764,"context_line":"\t\t\tif (retval !\u003d ERROR_OK)"},{"line_number":1765,"context_line":"\t\t\t\treturn retval;"},{"line_number":1766,"context_line":"\t\t\tentry_offset +\u003d 4;"},{"line_number":1767,"context_line":"\t\t\tromentry \u003d (((uint64_t)romentry_high) \u003c\u003c 32) | romentry_low;"},{"line_number":1768,"context_line":"\t\t\tcommand_print(cmd, \"\\t%sROMTABLE[0x%x] \u003d 0x%\" PRIx64, tabs, saved_entry_offset, romentry);"},{"line_number":1769,"context_line":"\t\t\tromentry \u0026\u003d ~0x0fffULL;"},{"line_number":1770,"context_line":"\t\t} else {"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"2b607708_b2cf35c6","line":1767,"range":{"start_line":1767,"start_character":13,"end_line":1767,"end_character":14},"updated":"2021-10-08 00:39:48.000000000","message":"think typecast may be needed here...\nex: romentry \u003d (int64_t)((((uint64_t)romentry_high) \u003c\u003c 32) | romentry_low);","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c09008f406ff94c8e98a6274f45cc530974e7de6","unresolved":false,"context_lines":[{"line_number":1764,"context_line":"\t\t\tif (retval !\u003d ERROR_OK)"},{"line_number":1765,"context_line":"\t\t\t\treturn retval;"},{"line_number":1766,"context_line":"\t\t\tentry_offset +\u003d 4;"},{"line_number":1767,"context_line":"\t\t\tromentry \u003d (((uint64_t)romentry_high) \u003c\u003c 32) | romentry_low;"},{"line_number":1768,"context_line":"\t\t\tcommand_print(cmd, \"\\t%sROMTABLE[0x%x] \u003d 0x%\" PRIx64, tabs, saved_entry_offset, romentry);"},{"line_number":1769,"context_line":"\t\t\tromentry \u0026\u003d ~0x0fffULL;"},{"line_number":1770,"context_line":"\t\t} else {"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"0590203c_79e9dc92","line":1767,"range":{"start_line":1767,"start_character":13,"end_line":1767,"end_character":14},"in_reply_to":"2b607708_b2cf35c6","updated":"2021-10-13 21:23:54.000000000","message":"No need, we are in 64 bit arithmetic.","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"86a1c9238fdb279ff81df6c54198f3ec5b42139b","unresolved":true,"context_lines":[{"line_number":1769,"context_line":"\t\t\tromentry \u0026\u003d ~0x0fffULL;"},{"line_number":1770,"context_line":"\t\t} else {"},{"line_number":1771,"context_line":"\t\t\tcommand_print(cmd, \"\\t%sROMTABLE[0x%x] \u003d 0x%\" PRIx32, tabs, saved_entry_offset, romentry_low);"},{"line_number":1772,"context_line":"\t\t\tromentry \u003d (int32_t)(romentry_low \u0026 0xFFFFF000);"},{"line_number":1773,"context_line":"\t\t}"},{"line_number":1774,"context_line":""},{"line_number":1775,"context_line":"\t\tif (romentry_low \u0026 ARM_CS_ROMENTRY_PRESENT) {"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"20117b63_d7d1cfb7","line":1772,"range":{"start_line":1772,"start_character":15,"end_line":1772,"end_character":22},"updated":"2021-10-08 00:39:48.000000000","message":"int64_t ?","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c09008f406ff94c8e98a6274f45cc530974e7de6","unresolved":false,"context_lines":[{"line_number":1769,"context_line":"\t\t\tromentry \u0026\u003d ~0x0fffULL;"},{"line_number":1770,"context_line":"\t\t} else {"},{"line_number":1771,"context_line":"\t\t\tcommand_print(cmd, \"\\t%sROMTABLE[0x%x] \u003d 0x%\" PRIx32, tabs, saved_entry_offset, romentry_low);"},{"line_number":1772,"context_line":"\t\t\tromentry \u003d (int32_t)(romentry_low \u0026 0xFFFFF000);"},{"line_number":1773,"context_line":"\t\t}"},{"line_number":1774,"context_line":""},{"line_number":1775,"context_line":"\t\tif (romentry_low \u0026 ARM_CS_ROMENTRY_PRESENT) {"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"6ffa3418_95556d21","line":1772,"range":{"start_line":1772,"start_character":15,"end_line":1772,"end_character":22},"in_reply_to":"20117b63_d7d1cfb7","updated":"2021-10-13 21:23:54.000000000","message":"No, int32_t!\nWe have a uint32_t bit in romentry_low. The cast forces using its bit 31 as sign, then C stores the int32_t in a 64 bit container","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"e50b26dbec66fa8321634ba98416851256571808","unresolved":false,"context_lines":[{"line_number":1769,"context_line":"\t\t\tromentry \u0026\u003d ~0x0fffULL;"},{"line_number":1770,"context_line":"\t\t} else {"},{"line_number":1771,"context_line":"\t\t\tcommand_print(cmd, \"\\t%sROMTABLE[0x%x] \u003d 0x%\" PRIx32, tabs, saved_entry_offset, romentry_low);"},{"line_number":1772,"context_line":"\t\t\tromentry \u003d (int32_t)(romentry_low \u0026 0xFFFFF000);"},{"line_number":1773,"context_line":"\t\t}"},{"line_number":1774,"context_line":""},{"line_number":1775,"context_line":"\t\tif (romentry_low \u0026 ARM_CS_ROMENTRY_PRESENT) {"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"38da7266_47a71219","line":1772,"range":{"start_line":1772,"start_character":15,"end_line":1772,"end_character":22},"in_reply_to":"6ffa3418_95556d21","updated":"2021-10-14 19:48:46.000000000","message":"I have a concern with the interpretation of romentry with respect to printing of the \"Component base address\" at the start of this routine. During my testing, with 32-bit APs, in some cases the Component base address gets printed incorrectly.\n\nSorry for the lengthy post and I\u0027ll explain why, but for 32bit APs, I think romentry needs to be masked...\n\nromentry \u003d (int32_t)(romentry_low \u0026 0xFFFFF000);\nif (!is_64bit_ap(ap))\n    romentry \u0026\u003d 0xFFFFFFFFul;\n\nThere are three situations of interest...\n1.\t64b BASE with 64b romentry offset\n2.\t64b BASE with 32b romentry offset\n3.\t32b BASE with 32b romentry offset\n\nThe ARM ADIv6 IHI0074C states \"Negative values are permitted, using two\u0027s complement\". This to me insinuates that the values represented can be signed, but don\u0027t have to be. For scenarios 1 and 3 above, we aren\u0027t concerned about whether a value is signed or unsigned because when adding numbers of the same length in bits, it doesn\u0027t matter.\n\nFor scenario #2, when converting the romentry offset from 32 to 64-bit, the sign extension becomes important. \n\nThe problem happens in the 3rd case with a 32-bit AP and 32 bit romentry offset. In this situation it doesn\u0027t matter if the romentry offset is signed or unsigned because no sign extension needs to occur.\n\nWith that said, I\u0027ve seen chips with a 32-bit AP and 32 bit romentry offset that use unsigned values which appears to be allowed per the ADI spec. In this situation, if the upper bit (bit #31) of the unsigned value is set, this code sign extends the value when storing into \"romentry\". When printing \"Component base address\" at the start of the routine, as a result of the incorrect sign extension, the wrong base address gets printed.\n\nFor example, say the AP is 32b, base_addr is 0x0 and the romentry offset is unsigned 0xA0000000. The intention would be to print the Component base address value as 0xA0000000 and not 0xFFFFFFFFA0000000. To fix this, I think if the AP is 32 bit, the upper romentry 32 bits need to be masked.\n\nIn summary for the three situations of interest...\n1.\t64b BASE with 64b romentry offset (offset can be both signed/unsigned)\n2.\t64b BASE with 32b romentry offset (offset must be signed to support sign extension)\n3.\t32b BASE with 32b romentry offset (offset can be both signed/unsigned)\n\nUse the signed offset calculation for all cases, but for 32b APs, mask the upper 32 bits to handle printing of unsigned values where the offset may be encoded unsigned.","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"514a89909c07daeec3e8fdd381c55d0e72e07c9a","unresolved":true,"context_lines":[{"line_number":1774,"context_line":""},{"line_number":1775,"context_line":"\t\tif (romentry_low \u0026 ARM_CS_ROMENTRY_PRESENT) {"},{"line_number":1776,"context_line":"\t\t\t/* Recurse. \"romentry\" is signed */"},{"line_number":1777,"context_line":"\t\t\tretval \u003d dap_rom_display(cmd, ap, base_addr + romentry, depth + 1);"},{"line_number":1778,"context_line":"\t\t\tif (retval !\u003d ERROR_OK)"},{"line_number":1779,"context_line":"\t\t\t\treturn retval;"},{"line_number":1780,"context_line":"\t\t} else if (romentry !\u003d 0) {"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"f4077846_83ad5247","line":1777,"range":{"start_line":1777,"start_character":49,"end_line":1777,"end_character":57},"updated":"2021-10-14 20:07:20.000000000","message":"In addition to my last comment, \"romentry\" is int64_t thus the equation \"base_addr + romentry\" will get converted to a 64-bit value even if the \"base_addr\" is 32-bit for a 32-bit AP. For 32-bit APs (32-bit base_addr), I think we want to avoid sending a 64-bit address when \"romentry\" is computed from a sign-extended offset where the offset has been encoded unsigned.","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"4964098625aa6aaebc3eb9627dab74ac704bab5c","unresolved":false,"context_lines":[{"line_number":1774,"context_line":""},{"line_number":1775,"context_line":"\t\tif (romentry_low \u0026 ARM_CS_ROMENTRY_PRESENT) {"},{"line_number":1776,"context_line":"\t\t\t/* Recurse. \"romentry\" is signed */"},{"line_number":1777,"context_line":"\t\t\tretval \u003d dap_rom_display(cmd, ap, base_addr + romentry, depth + 1);"},{"line_number":1778,"context_line":"\t\t\tif (retval !\u003d ERROR_OK)"},{"line_number":1779,"context_line":"\t\t\t\treturn retval;"},{"line_number":1780,"context_line":"\t\t} else if (romentry !\u003d 0) {"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"2c83ac6c_f74fd17a","line":1777,"range":{"start_line":1777,"start_character":49,"end_line":1777,"end_character":57},"in_reply_to":"f4077846_83ad5247","updated":"2021-11-09 23:13:30.000000000","message":"Ack","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1001661,"name":"Daniel Goehring","email":"dgoehrin@os.amperecomputing.com","username":"dgoehrin"},"change_message_id":"865ce5e7b31a1e54ed85b859a7be9ca63c9f186a","unresolved":true,"context_lines":[{"line_number":1777,"context_line":"\t\t\tretval \u003d dap_rom_display(cmd, ap, base_addr + romentry, depth + 1);"},{"line_number":1778,"context_line":"\t\t\tif (retval !\u003d ERROR_OK)"},{"line_number":1779,"context_line":"\t\t\t\treturn retval;"},{"line_number":1780,"context_line":"\t\t} else if (romentry !\u003d 0) {"},{"line_number":1781,"context_line":"\t\t\tcommand_print(cmd, \"\\t\\tComponent not present\");"},{"line_number":1782,"context_line":"\t\t} else {"},{"line_number":1783,"context_line":"\t\t\tcommand_print(cmd, \"\\t%s\\tEnd of ROM table\", tabs);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"aae123ec_88b13d04","line":1780,"range":{"start_line":1780,"start_character":13,"end_line":1780,"end_character":21},"updated":"2021-10-14 22:57:37.000000000","message":"\"romentry_low\". A romentry 0x00000002 is valid but misses this check since the lower 12 bits of romentry are masked from above. As a result the routine thinks it\u0027s the end of the table and exits.","commit_id":"54677b9daffd28a75f50c941856029108198574d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"4964098625aa6aaebc3eb9627dab74ac704bab5c","unresolved":false,"context_lines":[{"line_number":1777,"context_line":"\t\t\tretval \u003d dap_rom_display(cmd, ap, base_addr + romentry, depth + 1);"},{"line_number":1778,"context_line":"\t\t\tif (retval !\u003d ERROR_OK)"},{"line_number":1779,"context_line":"\t\t\t\treturn retval;"},{"line_number":1780,"context_line":"\t\t} else if (romentry !\u003d 0) {"},{"line_number":1781,"context_line":"\t\t\tcommand_print(cmd, \"\\t\\tComponent not present\");"},{"line_number":1782,"context_line":"\t\t} else {"},{"line_number":1783,"context_line":"\t\t\tcommand_print(cmd, \"\\t%s\\tEnd of ROM table\", tabs);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"82adb205_00f5c1b8","line":1780,"range":{"start_line":1780,"start_character":13,"end_line":1780,"end_character":21},"in_reply_to":"aae123ec_88b13d04","updated":"2021-11-09 23:13:30.000000000","message":"Ack","commit_id":"54677b9daffd28a75f50c941856029108198574d"}]}
