Attention is currently required from: Maulik V Vaghela, Mario Scheithauer, Subrata Banik, Lean Sheng Tan, Patrick Rudolph. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55082 )
Change subject: soc/intel/elkhartlake: Update FSP-S storage related configs ......................................................................
Patch Set 8: Code-Review+1
(3 comments)
File src/mainboard/intel/elkhartlake_crb/variants/ehlcrb/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/55082/comment/070770fe_ca3a3e32 PS8, Line 84: # LPSS Serial IO (I2C/UART/GSPI) related UPDs I would expect this line being part of CB:54959
File src/soc/intel/elkhartlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/55082/comment/1ba797f8_6835256f PS8, Line 20: DEF_DITOVAL Would you mind to change it to DEF_DITOVAL_MS to clarify that this value is in ms units?
https://review.coreboot.org/c/coreboot/+/55082/comment/c3d30f71_905df324 PS8, Line 253: params->SataPortsDmVal[i] = config->SataPortsDmVal[i] ? : config->SataPortsDmVal[i] : DEF_DMVAL; Here you can shorten down the line as you are checking for a value which is assigned in the "true"-case to the variable: 'params->SataPortsDmVal[i] = config->SataPortsDmVal[i] ? : DEF_DMVAL;' Same for the next statement.
For details see https://en.wikipedia.org/wiki/%3F:#C