Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38221 )
Change subject: soc/amd/picasso: Drop forked copy of SMBus source ......................................................................
Patch Set 4:
What's a pain (i.e. what I completely understand) is if someone wishes to build test changes in the picasso directory, they must checkout the entire work-in-progress stack up through the Mandolin mainboard.
That's not something you can expect of other developers. The mess one creates when submitting code to coreboot's master branch that is not hooked up for build testing, is their own burden.
That's not something you can expect of other developers.
Agreed, and it was never my expectation to place that onus on other developers. It's an unfortunate result of how Picasso (and any Family 17h and later) is so unique in the x86 world; it has taken much longer to land the functionality than originally projected. (I have a "final" revision to the WIP in mind for when I get a moment to work on them.)
Then once it's tested, that change needs to be brought over to a stack to be pushed. The reason for the -2 is that I don't want functioning picasso code ripped out, i.e. assuming one has the mainboard code commit.
Functioning? Can you please point me to the commit that makes use of this copy?
Not one that uses the bus features. However, this is for the first function of the southbridge device, i.e. D14F0. We currently rely on this to run setup_ioapic(). I'm not arguing this is the right vs. wrong location, however it's in use. The WIP mainboard patch is CB:33772.
In my opinion, this patch should not preclude merging CB:38222. My recommended approach would be to submit that, abandon this one, then I or someone else can move the stoneyridge code to common.
Well, that implies that no API changes are done, otherwise this copy would break anyway. I haven't looked to far into Kyösti's queue, though.
CB:38222 only modified a handful of names static to one file. Landing that diverges from this, and leaving the soc//picasso/files as a worse fork -- I assume that's Kyösti's concern. I realize that's not optimal, but unforking this is much lower in priority than my other Picasso priorities at the moment. FWIW, (a) prior to beginning Picasso work, I moved quite a lot of functionality from amd/stoneyridge into amd/common to minimize forking. At this point, I don't recall why I skipped smbus; possibly because coreboot won't require reading SPDs and I didn't yet know what I wanted to do with it. (b) I carefully scrubbed all the remaining soc code to see where the functionality would diverge. (c) For the period of time when I still had many other changes in progress, as soc//stoneyridge was changing, I maintained those modifications in my work. All that to say that... it'll be worth my effort once Picasso lands to do another audit to ensure nothing else was missed.