Attention is currently required from: Felix Singer, Edward O'Callaghan, Alex Badea.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73037 )
Change subject: asm106x: add programmer for ASM106x SATA controllers ......................................................................
Patch Set 5: Code-Review+2
(6 comments)
Patchset:
PS5: Thanks for the detailed answers. From my side it looks nearly fine. I've added a few nits.
Beside that I've ordered one of those controllers and will test it in the next days.
Edward, do you see something missing?
File asm106x.c:
https://review.coreboot.org/c/flashrom/+/73037/comment/b11e9d7b_379b19ef PS3, Line 35: {PCI_VENDOR_ID_ASMEDIA, 0x0612, OK, "ASMedia", "ASM106x"},
Ack
Ok, I'm fine with it. If someone has a chip with the other PCI-Id we can clarify it then.
File asm106x.c:
https://review.coreboot.org/c/flashrom/+/73037/comment/ceda25b2_76854e05 PS5, Line 47: x Please use `inttype.h` macros here for fixed length integer. We have problems with those on some compilers and a proper syntax for them comes first with c23. So for uint8_t as hex it gets ``` msg_pdbg2("asm106x status %#02"PRIx8" tries %d\n", val, timeout); ```
https://review.coreboot.org/c/flashrom/+/73037/comment/0bbecbdf_161cd28d PS5, Line 57: msg_pdbg("asm106x timed out, ctrl 0x%02x\n", val); Same here `... ctrl %#02"PRIx8"\n"`
https://review.coreboot.org/c/flashrom/+/73037/comment/e9288783_f380cfe1 PS5, Line 81: msg_pdbg2("asm106x write 0x%08x chunk %u\n", val, chunk); `write %#08"PRIx32" chunk`
https://review.coreboot.org/c/flashrom/+/73037/comment/838c006a_c83343a9 PS5, Line 99: msg_pdbg2("asm106x read 0x%08x chunk %u\n", val, chunk); `read %#08"PRIx32" chunk`