)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1002205,"name":"Matsievskiy S.V.","display_name":"Sergey Matsievskiy","email":"matsievskiysv@gmail.com","username":"matsievskiysv"},"change_message_id":"12eff7a53e5b562eb684d31fea4ffd64d3a5f051","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"667babe0_7108acab","updated":"2023-11-09 12:59:33.000000000","message":"This change is valid and can be easily verified by looking at https://s3-eu-west-1.amazonaws.com/downloads-mips/documents/MD00086-2B-MIPS32BIS-AFP-05.04.pdf#page\u003d282 . Can\u0027t believe that it has not been merged yet.","commit_id":"eec3ceaa5a41a9317464b075ff3ee8f333062979"},{"author":{"_account_id":1000410,"name":"Oleksij Rempel","email":"linux@rempel-privat.de","username":"olerem"},"change_message_id":"cad33ed46827442302b73fc6f9a3021f21caac73","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8a8c0b70_7ab65a9c","in_reply_to":"667babe0_7108acab","updated":"2023-11-09 14:12:34.000000000","message":"Thx, I\u0027ll take a look at this.\n\n\u003e Can\u0027t believe that it has not been merged yet.\n\nWhat is the point of this comment? Should it make me feel bad?","commit_id":"eec3ceaa5a41a9317464b075ff3ee8f333062979"},{"author":{"_account_id":1000410,"name":"Oleksij Rempel","email":"linux@rempel-privat.de","username":"olerem"},"change_message_id":"9bcb3bf55bac8c05b6f686d87f189433c5a0821c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"2c37b8fb_55d03d94","updated":"2023-11-09 20:57:18.000000000","message":"Thank you!","commit_id":"0f08cf0bacd6344e895541a2202ad1104ea3425a"}],"src/flash/nor/cfi.c":[{"author":{"_account_id":1000410,"name":"Oleksij Rempel","email":"linux@rempel-privat.de","username":"olerem"},"change_message_id":"c00ce542368d4ca6a88c55c489769146cea15595","unresolved":true,"context_lines":[{"line_number":1438,"context_line":"\t\tMIPS32_BNE(11, 8, 13),\t\t\t/* bne $t3, $t0, cont\t; if (temp2 !\u003d DQ7mask) goto cont */"},{"line_number":1439,"context_line":"\t\tMIPS32_NOP,\t\t\t\t\t\t/* nop\t\t\t\t\t\t\t\t\t*/"},{"line_number":1440,"context_line":""},{"line_number":1441,"context_line":"\t\tMIPS32_SRA(10, 8, 2),\t\t\t/* srl $t2,$t0,2\t\t; temp1 \u003d DQ7mask \u003e\u003e 2 */"},{"line_number":1442,"context_line":"\t\tMIPS32_AND(11, 10, 11),\t\t\t/* and $t3, $t2, $t3\t; temp2 \u003d temp2 \u0026 temp1\t*/"},{"line_number":1443,"context_line":"\t\tMIPS32_BNE(11, 10, NEG16(8)),\t/* bne $t3, $t2, busy\t; if (temp2 !\u003d temp1) goto busy\t*/"},{"line_number":1444,"context_line":"\t\tMIPS32_NOP,\t\t\t\t\t\t/* nop\t\t\t\t\t\t\t\t\t*/"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"b28d9856_08389e7a","line":1441,"updated":"2023-11-09 14:41:34.000000000","message":"Even by knowing that SRA was actually used. It looks like SRA do not make sense in this context. SRA should preserve signed bit by shifting, but we are working here with mask, which is most probably a bit field, not a signed value.\nDo any one has needed HW to test it?","commit_id":"eec3ceaa5a41a9317464b075ff3ee8f333062979"},{"author":{"_account_id":1000410,"name":"Oleksij Rempel","email":"linux@rempel-privat.de","username":"olerem"},"change_message_id":"52f99aeabefb7c753af4765d78b29315e837b1a1","unresolved":true,"context_lines":[{"line_number":1438,"context_line":"\t\tMIPS32_BNE(11, 8, 13),\t\t\t/* bne $t3, $t0, cont\t; if (temp2 !\u003d DQ7mask) goto cont */"},{"line_number":1439,"context_line":"\t\tMIPS32_NOP,\t\t\t\t\t\t/* nop\t\t\t\t\t\t\t\t\t*/"},{"line_number":1440,"context_line":""},{"line_number":1441,"context_line":"\t\tMIPS32_SRA(10, 8, 2),\t\t\t/* srl $t2,$t0,2\t\t; temp1 \u003d DQ7mask \u003e\u003e 2 */"},{"line_number":1442,"context_line":"\t\tMIPS32_AND(11, 10, 11),\t\t\t/* and $t3, $t2, $t3\t; temp2 \u003d temp2 \u0026 temp1\t*/"},{"line_number":1443,"context_line":"\t\tMIPS32_BNE(11, 10, NEG16(8)),\t/* bne $t3, $t2, busy\t; if (temp2 !\u003d temp1) goto busy\t*/"},{"line_number":1444,"context_line":"\t\tMIPS32_NOP,\t\t\t\t\t\t/* nop\t\t\t\t\t\t\t\t\t*/"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"61657be9_ae561bcc","line":1441,"in_reply_to":"45ace8e0_f9b7fa33","updated":"2023-11-09 16:48:48.000000000","message":"Let me be more precise:\nin case of SRL it will be \"temp1 \u003d DQ7mask \u003e\u003e 2\"\nIn case of SRA it will be something like this:\nunsigned int DQ7mask \u003d ...; // some unsigned value\nunsigned int sign \u003d DQ7mask \u0026 0x80000000; // Check the sign bit (assuming 32-bit unsigned int)\nunsigned int temp1 \u003d DQ7mask \u003e\u003e 2; // Logical right shift by 2 bits\n\nif (sign) {\n    // If the sign bit was set, fill the leftmost bits with 1\u0027s\n    temp1 |\u003d 0xC0000000; // Set the two leftmost bits to 1 (after shifting by 2)\n}\n\nI do not think, SRA should be used in this context. It looks more like SRL should be used here.","commit_id":"eec3ceaa5a41a9317464b075ff3ee8f333062979"},{"author":{"_account_id":1001086,"name":"Tobias Diedrich","email":"tobiasdiedrich@gmail.com","username":"ranma"},"change_message_id":"f9cb7a2bda693ffcc33627a31d14a0b9c720022b","unresolved":true,"context_lines":[{"line_number":1438,"context_line":"\t\tMIPS32_BNE(11, 8, 13),\t\t\t/* bne $t3, $t0, cont\t; if (temp2 !\u003d DQ7mask) goto cont */"},{"line_number":1439,"context_line":"\t\tMIPS32_NOP,\t\t\t\t\t\t/* nop\t\t\t\t\t\t\t\t\t*/"},{"line_number":1440,"context_line":""},{"line_number":1441,"context_line":"\t\tMIPS32_SRA(10, 8, 2),\t\t\t/* srl $t2,$t0,2\t\t; temp1 \u003d DQ7mask \u003e\u003e 2 */"},{"line_number":1442,"context_line":"\t\tMIPS32_AND(11, 10, 11),\t\t\t/* and $t3, $t2, $t3\t; temp2 \u003d temp2 \u0026 temp1\t*/"},{"line_number":1443,"context_line":"\t\tMIPS32_BNE(11, 10, NEG16(8)),\t/* bne $t3, $t2, busy\t; if (temp2 !\u003d temp1) goto busy\t*/"},{"line_number":1444,"context_line":"\t\tMIPS32_NOP,\t\t\t\t\t\t/* nop\t\t\t\t\t\t\t\t\t*/"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"65414ad4_03cd0469","line":1441,"in_reply_to":"576934db_b0705647","updated":"2023-11-09 17:05:54.000000000","message":"That said SRL should be more correct to use here, but given the data constraints SRA should work as well.","commit_id":"eec3ceaa5a41a9317464b075ff3ee8f333062979"},{"author":{"_account_id":1001086,"name":"Tobias Diedrich","email":"tobiasdiedrich@gmail.com","username":"ranma"},"change_message_id":"fca1ce557be423f6e17ac13eb3c97cc868d3c472","unresolved":true,"context_lines":[{"line_number":1438,"context_line":"\t\tMIPS32_BNE(11, 8, 13),\t\t\t/* bne $t3, $t0, cont\t; if (temp2 !\u003d DQ7mask) goto cont */"},{"line_number":1439,"context_line":"\t\tMIPS32_NOP,\t\t\t\t\t\t/* nop\t\t\t\t\t\t\t\t\t*/"},{"line_number":1440,"context_line":""},{"line_number":1441,"context_line":"\t\tMIPS32_SRA(10, 8, 2),\t\t\t/* srl $t2,$t0,2\t\t; temp1 \u003d DQ7mask \u003e\u003e 2 */"},{"line_number":1442,"context_line":"\t\tMIPS32_AND(11, 10, 11),\t\t\t/* and $t3, $t2, $t3\t; temp2 \u003d temp2 \u0026 temp1\t*/"},{"line_number":1443,"context_line":"\t\tMIPS32_BNE(11, 10, NEG16(8)),\t/* bne $t3, $t2, busy\t; if (temp2 !\u003d temp1) goto busy\t*/"},{"line_number":1444,"context_line":"\t\tMIPS32_NOP,\t\t\t\t\t\t/* nop\t\t\t\t\t\t\t\t\t*/"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"576934db_b0705647","line":1441,"in_reply_to":"61657be9_ae561bcc","updated":"2023-11-09 17:02:22.000000000","message":"Since the mask is for DQ7 the mask should be 0x00000080, so the sign bit is unset:\nI think T0 is set in this line:\n\nbuf_set_u32(reg_params[4].value, 0, 32, cfi_command_val(bank, 0x80));\n\nSo in indeed a fixed constant of 0x80 is used, as expected (endian-swapped by cfi_command_val).","commit_id":"eec3ceaa5a41a9317464b075ff3ee8f333062979"},{"author":{"_account_id":1000410,"name":"Oleksij Rempel","email":"linux@rempel-privat.de","username":"olerem"},"change_message_id":"52288b98fb4b589e52f54026d41612640c9d1230","unresolved":true,"context_lines":[{"line_number":1438,"context_line":"\t\tMIPS32_BNE(11, 8, 13),\t\t\t/* bne $t3, $t0, cont\t; if (temp2 !\u003d DQ7mask) goto cont */"},{"line_number":1439,"context_line":"\t\tMIPS32_NOP,\t\t\t\t\t\t/* nop\t\t\t\t\t\t\t\t\t*/"},{"line_number":1440,"context_line":""},{"line_number":1441,"context_line":"\t\tMIPS32_SRA(10, 8, 2),\t\t\t/* srl $t2,$t0,2\t\t; temp1 \u003d DQ7mask \u003e\u003e 2 */"},{"line_number":1442,"context_line":"\t\tMIPS32_AND(11, 10, 11),\t\t\t/* and $t3, $t2, $t3\t; temp2 \u003d temp2 \u0026 temp1\t*/"},{"line_number":1443,"context_line":"\t\tMIPS32_BNE(11, 10, NEG16(8)),\t/* bne $t3, $t2, busy\t; if (temp2 !\u003d temp1) goto busy\t*/"},{"line_number":1444,"context_line":"\t\tMIPS32_NOP,\t\t\t\t\t\t/* nop\t\t\t\t\t\t\t\t\t*/"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"9f9bb728_9f779639","line":1441,"in_reply_to":"65414ad4_03cd0469","updated":"2023-11-09 17:25:29.000000000","message":"Ok, thx. It explains why it worked SRA. I would prefer to keep SRL here, since it is better fit and it reflect state of comment on this line.\n\nCan you please rebase this patch, drop change in cfi driver and add comment in the commit message why SRA worked in the cfi driver and why it is ok to use SRL instead.","commit_id":"eec3ceaa5a41a9317464b075ff3ee8f333062979"},{"author":{"_account_id":1001086,"name":"Tobias Diedrich","email":"tobiasdiedrich@gmail.com","username":"ranma"},"change_message_id":"1e848aca73179b91e975084f200297cae2487f52","unresolved":false,"context_lines":[{"line_number":1438,"context_line":"\t\tMIPS32_BNE(11, 8, 13),\t\t\t/* bne $t3, $t0, cont\t; if (temp2 !\u003d DQ7mask) goto cont */"},{"line_number":1439,"context_line":"\t\tMIPS32_NOP,\t\t\t\t\t\t/* nop\t\t\t\t\t\t\t\t\t*/"},{"line_number":1440,"context_line":""},{"line_number":1441,"context_line":"\t\tMIPS32_SRA(10, 8, 2),\t\t\t/* srl $t2,$t0,2\t\t; temp1 \u003d DQ7mask \u003e\u003e 2 */"},{"line_number":1442,"context_line":"\t\tMIPS32_AND(11, 10, 11),\t\t\t/* and $t3, $t2, $t3\t; temp2 \u003d temp2 \u0026 temp1\t*/"},{"line_number":1443,"context_line":"\t\tMIPS32_BNE(11, 10, NEG16(8)),\t/* bne $t3, $t2, busy\t; if (temp2 !\u003d temp1) goto busy\t*/"},{"line_number":1444,"context_line":"\t\tMIPS32_NOP,\t\t\t\t\t\t/* nop\t\t\t\t\t\t\t\t\t*/"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"df8e79c1_a3bf4c5f","line":1441,"in_reply_to":"9f9bb728_9f779639","updated":"2023-11-09 20:18:04.000000000","message":"Done","commit_id":"eec3ceaa5a41a9317464b075ff3ee8f333062979"},{"author":{"_account_id":1001086,"name":"Tobias Diedrich","email":"tobiasdiedrich@gmail.com","username":"ranma"},"change_message_id":"cd7cfd83b7af1559f8bc846566dd6c5f5373174e","unresolved":true,"context_lines":[{"line_number":1438,"context_line":"\t\tMIPS32_BNE(11, 8, 13),\t\t\t/* bne $t3, $t0, cont\t; if (temp2 !\u003d DQ7mask) goto cont */"},{"line_number":1439,"context_line":"\t\tMIPS32_NOP,\t\t\t\t\t\t/* nop\t\t\t\t\t\t\t\t\t*/"},{"line_number":1440,"context_line":""},{"line_number":1441,"context_line":"\t\tMIPS32_SRA(10, 8, 2),\t\t\t/* srl $t2,$t0,2\t\t; temp1 \u003d DQ7mask \u003e\u003e 2 */"},{"line_number":1442,"context_line":"\t\tMIPS32_AND(11, 10, 11),\t\t\t/* and $t3, $t2, $t3\t; temp2 \u003d temp2 \u0026 temp1\t*/"},{"line_number":1443,"context_line":"\t\tMIPS32_BNE(11, 10, NEG16(8)),\t/* bne $t3, $t2, busy\t; if (temp2 !\u003d temp1) goto busy\t*/"},{"line_number":1444,"context_line":"\t\tMIPS32_NOP,\t\t\t\t\t\t/* nop\t\t\t\t\t\t\t\t\t*/"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"45ace8e0_f9b7fa33","line":1441,"in_reply_to":"b28d9856_08389e7a","updated":"2023-11-09 15:11:45.000000000","message":"From the comment at the top: \"T0 \u003d constant to mask DQ7 bits (also used for Dq5 with shift)\"\n\nThe \"temp1 \u003d DQ7mask \u003e\u003e 2\" in the comment also matches that this is actually a right shift.\n\nDQ7 polling seems to be used to determine flash write in-progress state for these spansion chips:\n\nFrom https://www.farnell.com/datasheets/578094.pdf\n\n\"The device provides several bits to determine the status of a write operation: DQ2, DQ3, DQ5, DQ6, DQ7,\nand RY/BY#. Table 11.1 on page 40 and the following subsections describe the functions of these bits. DQ7,\nRY/BY#, and DQ6 each offer a method for determining whether a program or erase operation is complete or\nin progress. These three bits are discussed first\"\n\n\"When the system detects DQ7 has changed from the complement to true data, it can read valid data at DQ7–DQ0 on the following read cycles\"\n\nThis also has the data polling algorithm diagram in Figure 11.1 on page 37, which uses DQ7 and DQ5, as appears to be implemented here.","commit_id":"eec3ceaa5a41a9317464b075ff3ee8f333062979"}]}