Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38471 )
Change subject: soc/amd/picasso: Add SMMSTORE support ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
Of course I'm on onboard with that idea, however I would really like to get picasso functional in the repo first and then de-duplicate. Then that way we'll have two functioning generations where we can ensure it's done properly. At the moment, I'm doing work in picasso for S3 and discovering subtle differences from stoneyridge; it seems more logical to discover any others first and de-duplicate once.
Last time I checked AMD code (fam10 and fam15 stuff), there were many generations of copied files. Personally, I would not duplicate things in the first place. With some restructuring, I'm pretty sure it's easier to not duplicate in the first place. For example, adding SMBus support for picasso could be as simple as selecting SOC_AMD_COMMON_BLOCK_SMBUS.
I realize we're swerving pretty far off topic now... I'll consider abandoning this patch simply to end the conversation and get back to productive work. My goal was solely to keep picasso in sync with stoneyridge until commonizing can be prioritized. I already require, and have, unrelated local changes to this file, however they won't be affected by a rebase.
Prior to starting picasso, I scrubbed the differences between the onboard FCH for Family 17h and Family 15h. That was the basis for moving so much so much of stoneyridge's functionality into soc/amd/common. Some features were obvious moves, some obviously needed to stay, and others were borderline i.e. I really wanted to get a better feel for their behavior on picasso first. The smi and smbus registers fell into the 3rd category.
...AMD code (fam10 and fam15 stuff), there were many generations of copied files.
I completely agree, and I wouldn't try to defend the development practices from years ago. But common doesn't always mean better; I'll suggest the example where multiple generations relied on a common sb//hudson directory. Hudson was a codename of a specific family of discrete controller hubs from exactly 10 years ago. But then every time a different FCH was used, or AMD released a new APU with an internal hub, the implementation relied on the hudson source -- apparently nobody took the time to study the differences, and subsequently there were bugs that went unnoticed for years.