Patch Set 22: Code-Review-1
(19 comments)
Sorry, found some more nits.
ok, np.
19 comments:
FSP
Done
FSP
Done
Patch Set #22, Line 19: This patch also corrects the debug_interface_flag definitions.
Please add a blank line above.
Done
File src/soc/intel/tigerlake/chip.h:
Patch Set #22, Line 207: /* Debug interface flag */
It’s clear from the name itself.
Done
One space.
Done
One space.
Done
File src/soc/intel/tigerlake/fsp_params_jsl.c:
Patch Set #22, Line 52: ARRAY_SIZE(config->SerialIoI2cMode), "copy buffer overflow!");
Can a better error message be printed? […]
I think this communicates well on compilation failure.
unsigned int
Done
Patch Set #22, Line 99: params->PeiGraphicsPeimInit = 0;
Please use the ternary operator.
since too many conditions, I think if else hold good.
Patch Set #22, Line 109: Bytes
lowercase: bytes
Done
Patch Set #22, Line 114: params->Enable8254ClockGatingOnS3 = 1;
In other commits/change-sets this also uses `!CONFIG_USE_LEGACY_8254_TIMER`.
want this to be explicitly set.
Could such things be common code?
can we done, but not in scope of this change. we can have a separate CL for it.
Patch Set #22, Line 150: params->ScsSdCardEnabled = dev->enabled;
Ternary operator: […]
Done
Patch Set #22, Line 166: dev->enabled = 0;
Why doesn’t `dev` need to be checked for being present?
Thanks!!. Done
FSP
Done
File src/soc/intel/tigerlake/romstage/fsp_params_jsl.c:
Patch Set #22, Line 4: * Copyright (C) 2019-2020 Intel Corporation.
Remove the dot/period at the end if you use the full word.
Done
Patch Set #22, Line 34: * is disable in devicetree.cb.
Should fit in one line.
Done
CPU
Done
Patch Set #22, Line 98: m_cfg->PchHdaEnable = dev->enabled;
Please use the ternary operator.
Done
To view, visit change 38461. To unsubscribe, or for help writing mail filters, visit settings.