Attention is currently required from: Arthur Heymans, Cliff Huang, Felix Held, Felix Singer, Lance Zhao, Naresh Solanki, Patrick Rudolph, Tim Wawrzynczak.
Angel Pons has posted comments on this change by Naresh Solanki. ( https://review.coreboot.org/c/coreboot/+/79404?usp=email )
Change subject: acpi: Add IORT helper functions ......................................................................
Patch Set 5:
(7 comments)
File src/acpi/acpi_iort.c:
https://review.coreboot.org/c/coreboot/+/79404/comment/ac6eadef_3ea3174f?usp... : PS5, Line 9: acpi_iort_its_group_t *its_node_data; Any reason to declare this in advance?
https://review.coreboot.org/c/coreboot/+/79404/comment/8260def4_1243c3c5?usp... : PS5, Line 11: *its = (acpi_iort_node_t *)current; This is an exported function, `*its` may be null. Would be great to assert the function's preconditions
https://review.coreboot.org/c/coreboot/+/79404/comment/5418961d_7e123dda?usp... : PS5, Line 36: *smmu_v3 = (acpi_iort_node_t *)current; Same here
https://review.coreboot.org/c/coreboot/+/79404/comment/24084dc7_4568e83f?usp... : PS5, Line 61: u16 device_name_len = strlen(device_name); ```suggestion const u16 device_name_len = strlen(device_name); ```
https://review.coreboot.org/c/coreboot/+/79404/comment/a474f575_7ea9cd58?usp... : PS5, Line 63: *nc = (acpi_iort_node_t *)current; Same here
https://review.coreboot.org/c/coreboot/+/79404/comment/6f05fae8_b652bbbb?usp... : PS5, Line 98: *rc = (acpi_iort_node_t *)current; Same here
File src/include/acpi/acpi_iort.h:
https://review.coreboot.org/c/coreboot/+/79404/comment/87019951_1101d079?usp... : PS5, Line 113: /* ITS Group */ : unsigned long its_entry(unsigned long current, acpi_iort_t *iort, acpi_iort_node_t **its, : u32 its_count, u32 *identifiers); : : /* SMMUv3 */ : unsigned long smmuv3_entry(unsigned long current, acpi_iort_t *iort, acpi_iort_node_t **smmu_v3, : u64 base, u32 flags); : : /* ID mapping */ : unsigned long id_map_entry(unsigned long current, acpi_iort_node_t *node, u32 input_base, : u32 id_count, u32 output_base, u32 output_reference, u32 flags); : : /* Named Component */ : unsigned long nc_entry(unsigned long current, acpi_iort_t *iort, acpi_iort_node_t **nc, : u32 node_flags, u64 memory_properties, u32 memory_address_limit, : char *device_name); : /* Root Complex */ : unsigned long rc_entry(unsigned long current, acpi_iort_t *node, acpi_iort_node_t **rc, : u64 memory_properties, u32 ats_attribute, u32 pci_segment_number, : u8 memory_address_limit, u16 pasid_capabilities); I would add a namespace-like prefix to these functions, e.g. `its_entry` ---> `acpi_iort_its_entry`.