[L] Change in coreboot[master]: [WIP] mb/prodrive/atlas: Populate smbios table with VPD from ECs EMI
Attention is currently required from: Maximilian Brune, Angel Pons.
David Milosevic has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68137 )
Change subject: [WIP] mb/prodrive/atlas: Populate smbios table with VPD from ECs EMI ......................................................................
Patch Set 8:
(2 comments)
File src/mainboard/prodrive/atlas/emi.h:
https://review.coreboot.org/c/coreboot/+/68137/comment/afe357bd_b873ce44 PS6, Line 40: typedef enum {
just because other people have done it doesn't mean you have to do it to. […]
I totally understand why someone might not like the usage of typedefs. But having so many exceptions to a rule and then still trying to enforce it, just does not make sense to me.
Nevertheless, I will remove the typedefs in the next patchset.
File src/mainboard/prodrive/atlas/emi.c:
https://review.coreboot.org/c/coreboot/+/68137/comment/2cbda9a0_c45803dc PS6, Line 83: {
If the function is not used there is no need to keep it. […]
It does not worsen readability at all. Those are mostly independent functions. You don't have to read the emi_write_region code if it's not important to you. Also if the function is not being used, it wont be compiled and the image stays the same.
Without those functions, this driver would be incomplete and as soon as you require for example a write function, you would have to read and understand the EC-datasheet again in order to implement it. It is easier for me to do that, since I am already working on the read function.
But I see, this discussion is pointless, so the next patchset will remove those functions.
-
David Milosevic (Code Review)