11 comments:
FSP
Done
Patch Set #3, Line 311: struct acpi_crat_header
I agree with the argument about not using typedefs (not to mention using name_t). […]
Adding to the list to scrub/convert typedefs to structs for this file in a follow up.
File src/include/acpi/acpi_crat.h:
Patch Set #3, Line 57: #define CRAT_PICASSO_NUM_DRAM_REG 2
This looks like it doesn't belong in a src/include file
Done
I didn't see that this needs to be initialized. If I missed it, then this needs a comment.
Done
File src/include/cpu/amd/msr.h:
This change adds a whole lot of cpuid info into a file called msr.h. […]
Done
Patch Set #3, Line 14: /* EAX Leaf */
The term "leaf" may be more Intel than AMD. […]
Done
File src/soc/amd/picasso/agesa_acpi.c:
Please split the picasso file out of the generic crat patch, and make it follow the other.
Done
Patch Set #3, Line 791: tlb_type
It always feels a bit odd having a variable/argument that's the same name as its type.
Done
It's not clear why we would clear an enable bit. And we just cleared the mem above. […]
I'm glad you mentioned this. I had read in the spec that all the sub categories were required, but i missed the end of the end of the sentence. I removed these entirely.
Patch Set #3, Line 1048: num_nodes++
Was this ever initialized? Or was it initialized to 0 by zeroing the memory in acpi.c. […]
It is zeroed out when the header is created in acpi_create_crat(). Each node is a memory locality.
Meh I'd remove the blank line to make the code similar to below
Done
To view, visit change 47727. To unsubscribe, or for help writing mail filters, visit settings.