Attention is currently required from: Felix Singer, Jason Glenesk, Raul Rangel, Nico Huber, Furquan Shaikh, Marshall Dawson, Tim Wawrzynczak, Angel Pons, Patrick Rudolph, Felix Held. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58118 )
Change subject: acpigen,soc/amd/cezanne,intel/{common,skl}: rework CPPC table passing ......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS8:
I think it makes sense to set it where the table is set. Also, the three-liner repeated in soc/intel/*, I guess that belongs in cpu/intel/common/?
You probably mean `cpu_get_cppc_table`? SKL will use common ACPI soon (CB:44138), so duplication will vanish.
Actually, the code calling cpu_get_cppc_table(). It seems odd to have parts of generic code in soc/ and other parts in cpu/.
Well, much stuff in `soc/intel/block/acpi` is generic code, isn't it? I get what you mean but moving shouldn't be part of that change anyways. If we want to move stuff, let's discuss that in a separate, future change, please. We'd first need to decide what is "generic enough" to be in `src/cpu`.
Out of the many changes, I think what I actually don't like is to remove the struct. Maybe we should keep the acpigen API as is but do all the other clean-up?
Ack
An advantage of keeping struct types is that there's no need to use `memcpy()`, structs can be copied using a regular assignment. CB:56265 is an example.
Well, I have no preference in that regard tbh. One of your arguments there is `assignment avoids a common mistake with sizeof usage`. While that's not wrong, it doesn't apply there bc there was no such mistake. I partly agree to the type checking argument, OTOH I don't see what could go wrong in such simple cases. That might be a bit naive, though.
Don't get me wrong, please, I don't want to bash your change :-) It's just that there are better arguments already like keeping the version together with the entries.