5 comments:
File src/soc/intel/tigerlake/chip.c:
Patch Set #3, Line 123: if (dev) {
nit: Kindly use a guard clause ; it will save one level of indentation: […]
Ack
Patch Set #3, Line 124: if (pci_read_config16(dev, PCI_DEVICE_ID) == device_id) {
nit: Same here; a guard clause will save another level of indentation: […]
Ack
reg16 = pci_read_config16(dev, PCI_COMMAND);
/* Check if BME bit is enabled before */
if ((reg16 & PCI_COMMAND_MASTER) == PCI_COMMAND_MASTER) {
reg16 &= ~PCI_COMMAND_MASTER;
pci_write_config16(dev, PCI_COMMAND, reg16);
}
in terms of performance - no need to do read compare write - just write 0 to the bit.
Ack
Patch Set #3, Line 138: if (phase == END_OF_FIRMWARE) {
nit: Guard clause here please: […]
END_OF_FIMWARE is only one of boot stages that could be handled by platform_fsp_notify_status. Note: AFTER_PCI_ENUM(0x20), READY_TO_BOOT(0x40), END_OF_FIRMWARE(0xF0).
"if (phase != END_OF_FRIMWARE) return" would not be considered.
Patch Set #3, Line 149: dev = pcidev_path_on_root(SA_DEVFN_TBT0);
Does |dev} need to be checked for an error here? It seems like it's being done on L142, but I'm not […]
L142 checks for device associated with SA_DEVFN_ROOT. Other SA_DEVFN_TBTx devices are checked in the clear_tbt_pcie_rt_bme function.
To view, visit change 40968. To unsubscribe, or for help writing mail filters, visit settings.