)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"a2e54671846dca59b77e08c31979db1f38e99e47","unresolved":true,"context_lines":[{"line_number":15,"context_line":"Expand the \"part number\" field to be a full implementor+part number,"},{"line_number":16,"context_line":"excluding the revision/patch fields, to make checking more reliable."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Support Infineon Cortex-M33 from SLx2 MCU."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"The secure microcontroller Infineon SLx2 uses a custom Cortex-M33."},{"line_number":21,"context_line":"The register CPUID reports value 0x490FDB00."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Id81774f829104f57a0c105320d0d2e479fa01522"},{"line_number":24,"context_line":"Signed-off-by: Karl Palsson \u003ckarlp@tweak.au\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"c0fa7ab1_552dc027","line":21,"range":{"start_line":18,"start_character":0,"end_line":21,"end_character":44},"updated":"2023-08-25 09:04:05.000000000","message":"AFAIK not in this patch","commit_id":"a91fe7c06aa0a2920122908e80b534101b054bb5"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"532541a8211d7fae874b88bb894499bc99140ba7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2d55552d_dbce8b1e","updated":"2023-08-03 08:04:16.000000000","message":"Alternatively we can work with unshifted masked CPUID values as linux kernel does:\nhttps://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/include/asm/cputype.h?h\u003dv6.5-rc4","commit_id":"d9d1f598181815d9b6d3028b54dd4c7b22cf96bd"},{"author":{"_account_id":1000859,"name":"Karl Palsson","email":"karlp@tweak.au","username":"karlp"},"change_message_id":"0a8bb754933a1dc957fa306fe734a410a0fefca3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4a45d516_3c579093","updated":"2023-08-04 10:44:26.000000000","message":"Am I right in re-reading the comments that what we\u0027re really trying to decide is\n\na) an \"enum\" is still useful, for... code cleanliness reasons.\n\u003d\u003e implies that it would have to be a single enum with the unshifted, but masked values. My original approach of leaving the enum for arm part numbers only is clearly inferior.\n_or_\nb) Just use defines, stop pretending this is any sort of enumeration.\n\nI can do a) no problems, it seems like the best way forward.","commit_id":"d9d1f598181815d9b6d3028b54dd4c7b22cf96bd"},{"author":{"_account_id":1000859,"name":"Karl Palsson","email":"karlp@tweak.au","username":"karlp"},"change_message_id":"31784a131f809c8d5a3e9d780d3306110889805e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"721d92ca_aa98cd7c","updated":"2023-08-03 15:39:06.000000000","message":"IMO the flash layers shouldn\u0027t be looking at the cortex part number at all, it\u0027s being used as a standin for \"f1 vs f0\" and \"f3 vs gd32\" and the cortex part numbers are a .... poor substitute.  So personally, I think the enum of cortex part numbers should be just dropped entirely, as they\u0027re _only_ used to make the array of known cortex parts, and then the flash layer for f1.  (I already dropped it from efm32, where it didn\u0027t do anything)","commit_id":"d9d1f598181815d9b6d3028b54dd4c7b22cf96bd"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"f493e56fa18d704c66eabf44122656b5bce010d6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"52c7961f_9279768a","updated":"2023-08-03 06:54:17.000000000","message":"Karl, thanks for the change. It looks we really need to track implementor field.\nJust couple of minor comments.","commit_id":"d9d1f598181815d9b6d3028b54dd4c7b22cf96bd"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c45ab714668a517126f04ad349f80d4e6385fcd4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f299aaee_d74d115b","in_reply_to":"2d55552d_dbce8b1e","updated":"2023-08-03 09:03:46.000000000","message":"I missed this comment.\nYes, agree! In line with what I was proposing.\nI\u0027m still checking about using enum with 32 bits unsigned values. The C language should be ok, as it checks the range of the enum values and then decides to use either signed int or unsigned int. Every C compiler should be ok.\nWould it be better using #define in place of enum ?","commit_id":"d9d1f598181815d9b6d3028b54dd4c7b22cf96bd"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"cd531ce475caf6310d4973f000c52c9205de2fea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7b3cbd03_c171d306","in_reply_to":"721d92ca_aa98cd7c","updated":"2023-08-03 15:45:21.000000000","message":"As a general approach, I agree to decouple flash and target subsystems.\n\nBut IIRC, Tarek introduced this test in STM32 flash driver for one bogus device that does not allows reading the part number.\nMaybe he can elaborate more.","commit_id":"d9d1f598181815d9b6d3028b54dd4c7b22cf96bd"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"90aeded96ad673cbb8112277978653491d34f783","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e8d49ebc_4e8189aa","in_reply_to":"7b3cbd03_c171d306","updated":"2023-08-04 05:42:08.000000000","message":"\u003e As a general approach, I agree to decouple flash and target subsystems.\n\nNice theory. but you should had told it to ST hw guys before they designed STM32F0,1,3 series ;-)\n\n\u003e IMO the flash layers shouldn\u0027t be looking at the cortex part number at all,\n\u003e it\u0027s being used as a standin for \"f1 vs f0\" and \"f3 vs gd32\" and the cortex\n\u003e part numbers are a .... poor substitute.\n\nAbsolutely not, Karl.\nstm32f1x.c uses Cortex-M core type mainly to locate the device ID. The ID location is in the system segment for ARMv7M cores and in the peripheral address space for ARMv6M cores. I have no idea why ST designed it this way, but they did so.\nWithout knowing the core type, the driver would need a hint where it should read device ID from a cfg file: of course it\u0027s feasible, looks as a step back in the device autodetection to me.\n\nOther uses:\nstm32f2x.c: detect silicon errata (ST used the same device ID for two devices with different Cortex-M cores)\nstm32h7x.c: distinguish which core from C-M7 C-M4 dual core system accesses the flash because C-M4 cannot read the flash size - a stupid ST design again!","commit_id":"d9d1f598181815d9b6d3028b54dd4c7b22cf96bd"},{"author":{"_account_id":1000859,"name":"Karl Palsson","email":"karlp@tweak.au","username":"karlp"},"change_message_id":"0a8bb754933a1dc957fa306fe734a410a0fefca3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"457ba7d7_f9389be2","in_reply_to":"e8d49ebc_4e8189aa","updated":"2023-08-04 10:44:26.000000000","message":"Those other uses seem _much_ more fine, but the stm32f1x shoudl, in my opinion, be loading in the ST part ids at probe time, not using the cortex part numbers later on.\n\nAs far as part detection going backwards, I think the current design is already backwards :)  It should be _more_ than sufficient to just specify \"stm32\" \n\nmaking a  single enum of the combined IMPL|PART would be a fair compromise though, given that I\u0027ve got zero interest in rewriting all the flash layers :)  I had just initially tried to _drop_ the enum, as it was no longer feasible as just a enum of part numbers.","commit_id":"d9d1f598181815d9b6d3028b54dd4c7b22cf96bd"},{"author":{"_account_id":1000859,"name":"Karl Palsson","email":"karlp@tweak.au","username":"karlp"},"change_message_id":"0a8bb754933a1dc957fa306fe734a410a0fefca3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6cd083fb_5f5a97d3","in_reply_to":"f299aaee_d74d115b","updated":"2023-08-04 10:44:26.000000000","message":"The kernel code can\u0027t make up it\u0027s mind whether to have defines or magic numbers though, so not sure it should be held up as a beacon ;)","commit_id":"d9d1f598181815d9b6d3028b54dd4c7b22cf96bd"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"79df37b316a10939d6a6cc3c9917af778b4fc128","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"2b5dc6dd_def16a43","updated":"2023-08-08 06:45:37.000000000","message":"Karl, besides the Jenkins build failed, I dislike the naming mess:\nCORTEX_xx_PARTNO is assigned to ARM_MAKE_CPUID() macro value,\nthe result of cortex_m_get_cpuid_safe() is compared to CORTEX_xx_PARTNO.\nI\u0027d propose to reserve _cpuid_ for the complete cpuid and refer to field names instead for partial cpuid values (like _impl_partno_) to make clear what is contained.\nOtherwise the code looks good to me.","commit_id":"7a31e85ae7665f5d061812b37685b179b5e6b9db"},{"author":{"_account_id":1000859,"name":"Karl Palsson","email":"karlp@tweak.au","username":"karlp"},"change_message_id":"ed11bb204fec8d7423d5a7e997a12b13c1d1c00f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"91405a5c_4358b2f5","in_reply_to":"2b5dc6dd_def16a43","updated":"2023-08-08 15:48:25.000000000","message":"I agree with this, and have changed most of it, but I\u0027m not sure what to make the list of enum names to, without making them super unwieldy.","commit_id":"7a31e85ae7665f5d061812b37685b179b5e6b9db"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"5fd663517de048ad9f4e12e9510fc13a4681816b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"6ce43ed3_4b46c0d5","updated":"2023-08-08 16:38:30.000000000","message":"Looks ok.\nYang sheng, could you please have a check for the STAR-MC1 part?","commit_id":"531a4237595e922453eaac7908ff8bbb3e918032"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"a0732d4bd22af33e2984055c7ce381cf0146f6e8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"7670a993_9bb88e19","updated":"2023-08-08 17:03:42.000000000","message":"Thanks!","commit_id":"531a4237595e922453eaac7908ff8bbb3e918032"},{"author":{"_account_id":1000859,"name":"Karl Palsson","email":"karlp@tweak.au","username":"karlp"},"change_message_id":"ed11bb204fec8d7423d5a7e997a12b13c1d1c00f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c0b31896_dad8cfff","updated":"2023-08-08 15:48:25.000000000","message":"still not sure how best to unify naming here, but this gets us significantly clsoser hopefully.","commit_id":"531a4237595e922453eaac7908ff8bbb3e918032"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"a2e54671846dca59b77e08c31979db1f38e99e47","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"1ee9748b_32b9ddf3","updated":"2023-08-25 09:04:05.000000000","message":"Ahmed, I assume your changes in patchsets 5, 6, 7 and 8 were submitted unintentionally.\nAnyway we prefer having SLx2 support in a separate patch.\n\nKarl, could you please re-upload your latest patchset 4? Thanks","commit_id":"a91fe7c06aa0a2920122908e80b534101b054bb5"}],"src/target/cortex_m.c":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"f493e56fa18d704c66eabf44122656b5bce010d6","unresolved":true,"context_lines":[{"line_number":2545,"context_line":"\t\t\treturn retval;"},{"line_number":2546,"context_line":""},{"line_number":2547,"context_line":"\t\t/* Get ARCH and CPU types */"},{"line_number":2548,"context_line":"\t\tconst int core_impl \u003d (cpuid \u0026 ARM_CPUID_IMPLEMENTOR_MASK) \u003e\u003e ARM_CPUID_IMPLEMENTOR_POS;"},{"line_number":2549,"context_line":"\t\tconst int core_partno \u003d (cpuid \u0026 ARM_CPUID_PARTNO_MASK) \u003e\u003e ARM_CPUID_PARTNO_POS;"},{"line_number":2550,"context_line":""},{"line_number":2551,"context_line":"\t\tfor (unsigned int n \u003d 0; n \u003c ARRAY_SIZE(cortex_m_parts); n++) {"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"62a48ff3_6a09940e","line":2548,"range":{"start_line":2548,"start_character":2,"end_line":2548,"end_character":8},"updated":"2023-08-03 06:54:17.000000000","message":"I know it\u0027s the unchanged original code, but there is no need to explicitly emphasize \u0027const\u0027. At least the line will be shorter w/o \u0027const\u0027.","commit_id":"d9d1f598181815d9b6d3028b54dd4c7b22cf96bd"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"f493e56fa18d704c66eabf44122656b5bce010d6","unresolved":true,"context_lines":[{"line_number":2546,"context_line":""},{"line_number":2547,"context_line":"\t\t/* Get ARCH and CPU types */"},{"line_number":2548,"context_line":"\t\tconst int core_impl \u003d (cpuid \u0026 ARM_CPUID_IMPLEMENTOR_MASK) \u003e\u003e ARM_CPUID_IMPLEMENTOR_POS;"},{"line_number":2549,"context_line":"\t\tconst int core_partno \u003d (cpuid \u0026 ARM_CPUID_PARTNO_MASK) \u003e\u003e ARM_CPUID_PARTNO_POS;"},{"line_number":2550,"context_line":""},{"line_number":2551,"context_line":"\t\tfor (unsigned int n \u003d 0; n \u003c ARRAY_SIZE(cortex_m_parts); n++) {"},{"line_number":2552,"context_line":"\t\t\tif (core_impl \u003d\u003d cortex_m_parts[n].implementor"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"79fbb2fa_6a25a91e","line":2549,"range":{"start_line":2549,"start_character":2,"end_line":2549,"end_character":7},"updated":"2023-08-03 06:54:17.000000000","message":"similar as above","commit_id":"d9d1f598181815d9b6d3028b54dd4c7b22cf96bd"}],"src/target/cortex_m.h":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"90710443e6940e5dec31c9bc9d3d0f137d04d033","unresolved":true,"context_lines":[{"line_number":43,"context_line":" */"},{"line_number":44,"context_line":"enum cortex_m_partno {"},{"line_number":45,"context_line":"\tCORTEX_M_PARTNO_INVALID,"},{"line_number":46,"context_line":"\tCORTEX_M0_PARTNO   \u003d 0xC20,"},{"line_number":47,"context_line":"\tCORTEX_M1_PARTNO   \u003d 0xC21,"},{"line_number":48,"context_line":"\tCORTEX_M3_PARTNO   \u003d 0xC23,"},{"line_number":49,"context_line":"\tCORTEX_M4_PARTNO   \u003d 0xC24,"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"474fd756_6ce53686","line":46,"updated":"2023-08-03 08:36:19.000000000","message":"We have code in flash subsystem that uses these values. It should also check for the implementor, not only for the partno.\nThere is very low possibility for a clash because the flash driver is already restricting the range of devices, but I see this check mismatch as a bad implementation.\n\nShould we put together partno and implementor? I mean, masking the CPUID register with 0xff00fff0 and changing this as:\nenum cortex_m_id \u003d {\n...\nARM_CORTEX_M23_ID \u003d 0x4100D200,\nREALTEK_M200_ID   \u003d 0x7200D200,\n\nMaybe through a macro\n#define CORTEX_M_ID(implementor, partno) ((implementor)\u003c\u003c24)|((partno)\u003c\u003c4))\nARM_CORTEX_M23_ID \u003d CORTEX_M_ID(0x41, 0xD20),\nREALTEK_M200_ID   \u003d CORTEX_M_ID(0x72, 0xD20),","commit_id":"d9d1f598181815d9b6d3028b54dd4c7b22cf96bd"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"f493e56fa18d704c66eabf44122656b5bce010d6","unresolved":true,"context_lines":[{"line_number":61,"context_line":"#define CORTEX_M_F_TAR_AUTOINCR_BLOCK_4K  BIT(2)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"struct cortex_m_part_info {"},{"line_number":64,"context_line":"\tint implementor;"},{"line_number":65,"context_line":"\tint partno;"},{"line_number":66,"context_line":"\tconst char *name;"},{"line_number":67,"context_line":"\tenum arm_arch arch;"},{"line_number":68,"context_line":"\tuint32_t flags;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"7adee30a_669fde44","line":65,"range":{"start_line":64,"start_character":1,"end_line":65,"end_character":12},"updated":"2023-08-03 06:54:17.000000000","message":"unsigned int?","commit_id":"d9d1f598181815d9b6d3028b54dd4c7b22cf96bd"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"f493e56fa18d704c66eabf44122656b5bce010d6","unresolved":true,"context_lines":[{"line_number":303,"context_line":" * or CORTEX_M_PARTNO_INVALID if the magic number does not match"},{"line_number":304,"context_line":" * or core_info is not initialised."},{"line_number":305,"context_line":" */"},{"line_number":306,"context_line":"static inline int cortex_m_get_partno_safe(struct target *target)"},{"line_number":307,"context_line":"{"},{"line_number":308,"context_line":"\tstruct cortex_m_common *cortex_m \u003d target_to_cortex_m_safe(target);"},{"line_number":309,"context_line":"\tif (!cortex_m)"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"e28ee4ff_fb2b1f0d","line":306,"range":{"start_line":306,"start_character":14,"end_line":306,"end_character":17},"updated":"2023-08-03 06:54:17.000000000","message":"unsigned int?","commit_id":"d9d1f598181815d9b6d3028b54dd4c7b22cf96bd"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"79df37b316a10939d6a6cc3c9917af778b4fc128","unresolved":true,"context_lines":[{"line_number":301,"context_line":"}"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"/**"},{"line_number":304,"context_line":" * @returns cached value of Cortex-M cpuid"},{"line_number":305,"context_line":" * or CORTEX_M_PARTNO_INVALID if the magic number does not match"},{"line_number":306,"context_line":" * or core_info is not initialised."},{"line_number":307,"context_line":" */"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"0fabad09_019b2515","line":304,"range":{"start_line":304,"start_character":37,"end_line":304,"end_character":42},"updated":"2023-08-08 06:45:37.000000000","message":"Very misleading comment!\nThe function does not return the full cpuid, returns just implementor and partno fields but not release and patch numbers!","commit_id":"7a31e85ae7665f5d061812b37685b179b5e6b9db"}]}