Sorry, found some more nits.
Patch set 22:Code-Review -1
19 comments:
FSP
FSP
Patch Set #22, Line 19: This patch also corrects the debug_interface_flag definitions.
Please add a blank line above.
File src/soc/intel/tigerlake/chip.h:
Patch Set #22, Line 207: /* Debug interface flag */
It’s clear from the name itself.
One space.
One space.
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?
Number of members in params and config differ
unsigned int
Patch Set #22, Line 99: params->PeiGraphicsPeimInit = 0;
Please use the ternary operator.
Patch Set #22, Line 109: Bytes
lowercase: bytes
Patch Set #22, Line 114: params->Enable8254ClockGatingOnS3 = 1;
In other commits/change-sets this also uses `!CONFIG_USE_LEGACY_8254_TIMER`.
Could such things be common code?
Patch Set #22, Line 150: params->ScsSdCardEnabled = dev->enabled;
Ternary operator:
params->ScsSdCardEnabled = pcidev_path_on_root(PCH_DEVFN_SDCARD) ? dev-enabled : 0;
Patch Set #22, Line 166: dev->enabled = 0;
Why doesn’t `dev` need to be checked for being present?
FSP
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.
Patch Set #22, Line 34: * is disable in devicetree.cb.
Should fit in one line.
CPU
Patch Set #22, Line 98: m_cfg->PchHdaEnable = dev->enabled;
Please use the ternary operator.
To view, visit change 38461. To unsubscribe, or for help writing mail filters, visit settings.