Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42689 )
Change subject: [WIP,RFC] soc/amd/common: Move sb_reset_i2c_slaves() ......................................................................
Patch Set 8:
Patch Set 8:
i don't think that this should be moved to the common gpio code, since it's mostly about i2c. there will also be some gpio-related changes for the i2c pins in renoir/cezanne. moving the code in its current state to the common designware i2c code is probably not the way to go, since the gpio part is very soc-specific. the code apart from the gpio control and set should be quite generic though
The sequence of bit-banging 9 clocks is what makes this i2c related. And true that this is not about the designware i2c either, since this operates in SoC specific register spaces (GPIO and IOMUX) instead of I2C controller register space.
The part I don't like about this in its current form is that we have reconfiguration of pads happening outside the scope of GPIO core. If I read the code correctly, it is already broken from this perspective; program_gpios() gets all the pads that could be configured for i2c, not just the ones that actually are. That's were my motivation of hiding gpio_write32 and iomux_write8 comes from.
-
Kyösti Mälkki (Code Review)