Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32910 )
Change subject: src/arch/x86: Add automatic type41 entry creation ......................................................................
Patch Set 4:
(12 comments)
https://review.coreboot.org/#/c/32910/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32910/2//COMMIT_MSG@9 PS2, Line 9: Emtries
entries
Ack
https://review.coreboot.org/#/c/32910/2//COMMIT_MSG@10 PS2, Line 10:
Please elaborate what type 41 entries are, and what they are needed for.
Ack
https://review.coreboot.org/#/c/32910/2/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/#/c/32910/2/src/arch/x86/smbios.c@55 PS2, Line 55: static const char *smbios_get_name_from_type(u8 device_type)
please use […]
Ack
https://review.coreboot.org/#/c/32910/2/src/arch/x86/smbios.c@77 PS2, Line 77: /* Get the device type from the dev struct */
mention type 41
Ack
https://review.coreboot.org/#/c/32910/2/src/arch/x86/smbios.c@109 PS2, Line 109: break;
Is that `break` really needed?
Ack
https://review.coreboot.org/#/c/32910/2/src/arch/x86/smbios.c@113 PS2, Line 113:
Please remove this blank line.
Ack
https://review.coreboot.org/#/c/32910/2/src/arch/x86/smbios.c@1072 PS2, Line 1072: return 0;
test for dev->on_mainboard
Ack
https://review.coreboot.org/#/c/32910/2/src/arch/x86/smbios.c@1077 PS2, Line 1077: device_type == SMBIOS_DEVICE_TYPE_UNKNOWN)
align with (
Ack
https://review.coreboot.org/#/c/32910/2/src/arch/x86/smbios.c@1086 PS2, Line 1086: dev->bus->secondary, //bus : PCI_SLOT(dev->path.pci.devfn), //device : PCI_FUNC(dev->path.pci.devfn), //func
Please add a space after `//`.
Ack
https://review.coreboot.org/#/c/32910/2/src/include/device/pci_ids.h File src/include/device/pci_ids.h:
https://review.coreboot.org/#/c/32910/2/src/include/device/pci_ids.h@21 PS2, Line 21:
Use tabs for alignment (see above).
Ack
https://review.coreboot.org/#/c/32910/2/src/include/device/pci_ids.h@3085 PS2, Line 3085:
Tab please.
Ack
https://review.coreboot.org/#/c/32910/2/src/include/device/pci_ids.h@3085 PS2, Line 3085: #define PCI_DEVICE_ID_INTEL_KBL_ID_DT_2 0x5918
unrelated
Ack