Attention is currently required from: Felix Held.
Patch set 4:Code-Review +1
1 comment:
Patchset:
I always wonder if this "sort-of duplicated" code is worth it. Naturally,
all the code that actually ends up duplicated could be common code, and
the sort-of duplicated code needs to be changed anyway, so there shouldn't
be much gain from copy-pasting whole files.https://review.coreboot.org/c/coreboot/+/61084/3/src/soc/amd/sabrina/gpio.c or most of https://review.coreboot.org/c/coreboot/+/61077/2/src/soc/amd/sabrina/chip.c or most of https://review.coreboot.org/c/coreboot/+/61086/3/src/soc/amd/sabrina/uart.c would be examples where I don't see how we could get rid of all remaining sort-of duplicated but not the same code. A lot of the common functions are moved to common and are then called from the SoC-specific code that is slightly different for each SoC; I'd say that that is the best approach right now.
Sorry, I guess you misunderstood. I didn't mean to say we should avoid all
sort-of-duplicated code. We won't ever achieve that. Just the way to get
some new sort-of-duplicated code, should that be copy-first or could we
push the adapted version already. I don't know, so still wondering what
would be better, theoretically, in case we'd at some point have no real /
unwanted duplication left. So let's just hope we get in the position to
ask this question at some point ;) but don't worry about it now.
I'd say that having certain things implemented in each SoC code makes it easier to understand; for example calling mp_init_with_smm somewhere from the cpu cluster's ops is easier to follow than calling it somewhere from some BOOT_STATE_INIT_ENTRY in some soc/vendor/common/block file despite the latter resulting in less code duplication.
Absolutely, yes ;) (I might add that I hate boot-state hooks because people
constantly lose track of what they do when.)
To view, visit change 61077. To unsubscribe, or for help writing mail filters, visit settings.