Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39232 )
Change subject: soc/intel/tigerlake: Enable Hybrid storage mode ......................................................................
Patch Set 8:
(8 comments)
https://review.coreboot.org/c/coreboot/+/39232/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39232/6//COMMIT_MSG@7 PS6, Line 7: Hybride
remove the `e` at the end: Hybrid
Ack
https://review.coreboot.org/c/coreboot/+/39232/6//COMMIT_MSG@9 PS6, Line 9: For using
To use
Ack
https://review.coreboot.org/c/coreboot/+/39232/6//COMMIT_MSG@9 PS6, Line 9: storage(optane)
I think `Optane` should be capitalized as well
Ack
https://review.coreboot.org/c/coreboot/+/39232/6//COMMIT_MSG@10 PS6, Line 10: And this can be : configured by fit configuration which is built time.
There are lots of words related to `configure` close together. […]
Ack
https://review.coreboot.org/c/coreboot/+/39232/6//COMMIT_MSG@12 PS6, Line 12: By enabling Hybrid Storage mode in FSP, FSP will detect Hybrid storage
Please add one blank line above to separate paragraphs.
Ack
https://review.coreboot.org/c/coreboot/+/39232/6//COMMIT_MSG@12 PS6, Line 12: FSP
FSP-S
Done
https://review.coreboot.org/c/coreboot/+/39232/6//COMMIT_MSG@13 PS6, Line 13: and save configuration : for next boot up.
Where does it get saved to? Also, to avoid repeating `configure`, I would use: […]
Ack
https://review.coreboot.org/c/coreboot/+/39232/6/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/39232/6/src/soc/intel/tigerlake/chi... PS6, Line 266: /* Hybrid storage mode enable(1)/disable(0) */
It would be inconsistent with the comments above, though... […]
Ack