Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42154 )
Change subject: sb/intel/lynxpoint: Use PCI bitwise ops ......................................................................
Patch Set 5:
(3 comments)
Patch Set 4:
(3 comments)
Comments are followup suggestions.
Thanks, will take care of them soon.
https://review.coreboot.org/c/coreboot/+/42154/4/src/southbridge/intel/lynxp... File src/southbridge/intel/lynxpoint/sata.c:
https://review.coreboot.org/c/coreboot/+/42154/4/src/southbridge/intel/lynxp... PS4, Line 50: pci_write_config32(dev, 0x24, 0);
PCI_BASE_ADDRESS_5, maybe clear COMMAND first.
I'll use PCI register names in a follow-up. Reordering things can't be done reproducibly, so it will need to be in a separate commit.
https://review.coreboot.org/c/coreboot/+/42154/4/src/southbridge/intel/lynxp... File src/southbridge/intel/lynxpoint/smihandler.c:
https://review.coreboot.org/c/coreboot/+/42154/4/src/southbridge/intel/lynxp... PS4, Line 58: static void busmaster_disable_on_bus(int bus)
I would say this belongs somewhere in src/device, maybe pci_early.c.
Sure. This function seems to be duplicated across multiple Intel southbridges as well...
https://review.coreboot.org/c/coreboot/+/42154/4/src/southbridge/intel/lynxp... File src/southbridge/intel/lynxpoint/usb_ehci.c:
https://review.coreboot.org/c/coreboot/+/42154/4/src/southbridge/intel/lynxp... PS4, Line 25: pci_write_config32(dev, PCI_BASE_ADDRESS_0, 0);
Not in this patch, but changing BARs would be better done with PCI_COMMAND_MEMORY and _IO cleared.
Ack