Attention is currently required from: Nico Huber, Subrata Banik, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62891 )
Change subject: ichspi: Introduce HSFC FCYCLE(cyc)/FCYCLE_MASK(n) macros ......................................................................
Patch Set 3:
(5 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62891/comment/8b4c7ba6_6554ba6e PS3, Line 12: Also, drops unused macros (PCH100_HSFC_FCYCLE_OFF and : PCH100_HSFC_FCYCLE). Can you extract this into a separate patch? Seems like these are two different things (corresponding to 1st and 2nd paragraph in commit message).
https://review.coreboot.org/c/flashrom/+/62891/comment/be76535d_aca982ec PS3, Line 16: brya I see you are using brya to test this chain, if you could add info what chipset brya has, it would be very useful. (and the same for all other patches in the chain ;)
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/62891/comment/83cc1fbc_05b6fd8d PS3, Line 80: /* Changed HSFC Control bits */ Should the comment be removed as well?
https://review.coreboot.org/c/flashrom/+/62891/comment/b1b7f36d_17bef426 PS3, Line 500: reg_val, ", "); No new line needed, this print statement can be on one line https://www.flashrom.org/Development_Guidelines#Coding_style
https://review.coreboot.org/c/flashrom/+/62891/comment/5d89a1b5_cdb247d9 PS3, Line 505: reg_val, ", "); Same as previous
-
Anastasia Klimchuk (Code Review)