Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32062 )
Change subject: soc/intel/braswell/smbus: Init SMBus ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/#/c/32062/3/src/soc/intel/braswell/Makefile.inc File src/soc/intel/braswell/Makefile.inc:
https://review.coreboot.org/#/c/32062/3/src/soc/intel/braswell/Makefile.inc@... PS3, Line 42: ramstage-y += smbus.c
I don't see how this could work in ramstage. The PCI device is […]
Since I will use it for SPD retrieval only, I may remove it from ramstage
https://review.coreboot.org/#/c/32062/3/src/soc/intel/braswell/smbus.c File src/soc/intel/braswell/smbus.c:
https://review.coreboot.org/#/c/32062/3/src/soc/intel/braswell/smbus.c@18 PS3, Line 18: #include <device/pci_def.h>
is this needed?
Yes, for the PCI_BASE_ADDRESS_4, PCI_COMMAND and PCI_COMMAND_IO values
https://review.coreboot.org/#/c/32062/3/src/soc/intel/braswell/smbus.c@32 PS3, Line 32: /* Disable interrupts */ : REG_IO_WRITE8(SMBUS_BASE_ADDRESS + SMBHSTCTL, 0), : /* Clear errors */ : REG_IO_WRITE8(SMBUS_BASE_ADDRESS + SMBHSTSTAT, 0xff),
The sb/common driver should handle these.
Yes, it clears the errors and interrupts on command setup. I will test without these.