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:
That copy-paste should not have been approved in the first place.
Well, that's a fair enough argument, but assuming the preferred way to go is to use common code instead, then let's create the common code first before dropping PCO functionality.
With PSP doing raminit, SMBus on amd/picasso may remain unused for a long time. Not sure if your designs have some other SMBus components that use it, and even if they did, existing code only has reference to PCI_DEVICE_ID_AMD_CZ_SMBUS. Is that the correct PCI ID? If not, there is no functionality dropped with the source removal.
I hope that once I have a chance to rip out the hybrid romstage changes, and make the stages look like the more traditional bootblock/<verstage>/romstage, the patches should be much more palatable and approvable quickly. It feels like Google engineers and I are >95% sure we know precisely how we want vboot to work on this system. Once that's settled, I'll be more apt to prioritize making the changes.
Yes, that's the correct device ID - 0x790b. On a separate note, I recall another gerrit entry pointed out that a device ID was added for a duplicate value although the ID was reused across products. (Notwithstanding it's easy to be confused by 0x790b "_CZ" used on PCO...) do you have opinions for how to best name/reuse device IDs? It seems like we typically maintain the ID's name as when it's first introduced.
And no, I don't plan to fix it for you as none of soc/amd/picasso is build-tested on master.
I'm not sure what you mean by "fix". If it's keeping PCO in sync with ST until we have common code, then I have no problem doing it myself.
Fix, as in introduce new SOC_AMD_COMMON_SMBUS kconfig and add/replace necessary PCI IDs and move the ST files.
OK, I'll add it to my list.
As far as "none of soc/amd/picasso is build-tested on master", that's not a valid argument. By that same logic, you could justify removing the entire picasso directory right now. There can't/won't be a mainboard to test until the bootblock/romstage changes are updated and land. I presume you feel the Family 17h development is going too slow, but I'm sorry that you have very limited visibility into the other design, work, and negotiations that are actually happening.
I would need to rebase the entire Mandolin CRB board and the dependencies to get a build-tested result for the common SMBUS approach. It's just less hassle for you to incorporate that on yout patcht-train once/if/where you want it.
I understand completely! I agree it's a real pain right now.