Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46427 )
Change subject: libpayload/storage/ahci: Use pci_dev struct in ahci_initialize() ......................................................................
libpayload/storage/ahci: Use pci_dev struct in ahci_initialize()
Change-Id: I2d5673c631d978d8ebd0c4a90962ab9cccaf40a2 Signed-off-by: Felix Singer felixsinger@posteo.net --- M payloads/libpayload/drivers/storage/ahci.c M payloads/libpayload/drivers/storage/storage.c M payloads/libpayload/include/storage/ahci.h 3 files changed, 15 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/46427/1
diff --git a/payloads/libpayload/drivers/storage/ahci.c b/payloads/libpayload/drivers/storage/ahci.c index e54bfbf..7c67182 100644 --- a/payloads/libpayload/drivers/storage/ahci.c +++ b/payloads/libpayload/drivers/storage/ahci.c @@ -228,34 +228,33 @@ }; #endif
-void ahci_initialize(pcidev_t dev) +void ahci_initialize(struct pci_dev *dev) { int i;
- const u16 class = pci_read_config16(dev, 0xa); - if (class != 0x0106) + pcidev_t pcidev = PCI_DEV(dev->bus, dev->dev, dev->func); + + if (dev->device_class != PCI_CLASS_STORAGE_AHCI) return; - const u16 vendor = pci_read_config16(dev, 0x00); - const u16 device = pci_read_config16(dev, 0x02);
#if CONFIG(LP_STORAGE_AHCI_ONLY_TESTED) - const u32 vendor_device = pci_read_config32(dev, 0x0); + const u32 vendor_device = dev->vendor_id | dev->device_id >> 16; for (i = 0; i < ARRAY_SIZE(working_controllers); ++i) if (vendor_device == working_controllers[i]) break; if (i == ARRAY_SIZE(working_controllers)) { printf("ahci: Not using untested SATA controller " - "%02x:%02x.%02x (%04x:%04x).\n", PCI_BUS(dev), - PCI_SLOT(dev), PCI_FUNC(dev), vendor, device); + "%02x:%02x.%02x (%04x:%04x).\n", dev->bus, + dev->dev, dev->func, dev->vendor_id, dev->device_id); return; } #endif
printf("ahci: Found SATA controller %02x:%02x.%02x (%04x:%04x).\n", - PCI_BUS(dev), PCI_SLOT(dev), PCI_FUNC(dev), vendor, device); + dev->bus, dev->dev, dev->func, dev->vendor_id, dev->device_id);
hba_ctrl_t *const ctrl = phys_to_virt( - pci_read_config32(dev, 0x24) & ~0x3ff); + pci_read_config32(pcidev, 0x24) & ~0x3ff); hba_port_t *const ports = ctrl->ports;
/* Reset host controller. */ @@ -274,8 +273,8 @@ ctrl->global_ctrl |= HBA_CTRL_AHCI_EN;
/* Enable bus mastering. */ - const u16 command = pci_read_config16(dev, PCI_COMMAND); - pci_write_config16(dev, PCI_COMMAND, command | PCI_COMMAND_MASTER); + const u16 command = pci_read_config16(pcidev, PCI_COMMAND); + pci_write_config16(pcidev, PCI_COMMAND, command | PCI_COMMAND_MASTER);
/* Probe for devices. */ for (i = 0; i < 32; ++i) { diff --git a/payloads/libpayload/drivers/storage/storage.c b/payloads/libpayload/drivers/storage/storage.c index e720a2d..f2bde77 100644 --- a/payloads/libpayload/drivers/storage/storage.c +++ b/payloads/libpayload/drivers/storage/storage.c @@ -112,11 +112,9 @@ struct pci_dev *dev;
for (dev = lib_sysinfo.pacc->devices; dev; dev = dev->next) { - const uint32_t pcidev = PCI_DEV(dev->bus, dev->dev, dev->func); - switch (dev->device_class) { case PCI_CLASS_STORAGE_AHCI: - ahci_initialize(pcidev); + ahci_initialize(dev); break; default: break; diff --git a/payloads/libpayload/include/storage/ahci.h b/payloads/libpayload/include/storage/ahci.h index 2843624..18a0920 100644 --- a/payloads/libpayload/include/storage/ahci.h +++ b/payloads/libpayload/include/storage/ahci.h @@ -26,9 +26,11 @@ * SUCH DAMAGE. */
+#include <pci/pci.h> + #ifndef _STORAGE_AHCI_H #define _STORAGE_AHCI_H
-void ahci_initialize(pcidev_t dev); +void ahci_initialize(struct pci_dev *dev);
#endif
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46427 )
Change subject: libpayload/storage/ahci: Use pci_dev struct in ahci_initialize() ......................................................................
Set Ready For Review
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46427 )
Change subject: libpayload/storage/ahci: Use pci_dev struct in ahci_initialize() ......................................................................
Set Ready For Review
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46427 )
Change subject: libpayload/storage/ahci: Use pci_dev struct in ahci_initialize() ......................................................................
Patch Set 11: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/46427/11/payloads/libpayload/driver... File payloads/libpayload/drivers/storage/ahci.c:
https://review.coreboot.org/c/coreboot/+/46427/11/payloads/libpayload/driver... PS11, Line 235: if (dev->device_class != PCI_CLASS_STORAGE_AHCI) : return; Could be an `assert()` as we already check this before the call.
Hello build bot (Jenkins), Nico Huber, Paul Menzel, Angel Pons, Michael Niewöhner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46427
to look at the new patch set (#12).
Change subject: libpayload/storage/ahci: Use pci_dev struct in ahci_initialize() ......................................................................
libpayload/storage/ahci: Use pci_dev struct in ahci_initialize()
Clean up ahci_initialize() by using pci_dev struct.
Change-Id: I2d5673c631d978d8ebd0c4a90962ab9cccaf40a2 Signed-off-by: Felix Singer felixsinger@posteo.net --- M payloads/libpayload/drivers/storage/ahci.c M payloads/libpayload/drivers/storage/storage.c M payloads/libpayload/include/storage/ahci.h 3 files changed, 12 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/46427/12
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46427 )
Change subject: libpayload/storage/ahci: Use pci_dev struct in ahci_initialize() ......................................................................
Patch Set 14: Code-Review+2
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46427 )
Change subject: libpayload/storage/ahci: Use pci_dev struct in ahci_initialize() ......................................................................
libpayload/storage/ahci: Use pci_dev struct in ahci_initialize()
Clean up ahci_initialize() by using pci_dev struct.
Change-Id: I2d5673c631d978d8ebd0c4a90962ab9cccaf40a2 Signed-off-by: Felix Singer felixsinger@posteo.net Reviewed-on: https://review.coreboot.org/c/coreboot/+/46427 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M payloads/libpayload/drivers/storage/ahci.c M payloads/libpayload/drivers/storage/storage.c M payloads/libpayload/include/storage/ahci.h 3 files changed, 12 insertions(+), 17 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/payloads/libpayload/drivers/storage/ahci.c b/payloads/libpayload/drivers/storage/ahci.c index e54bfbf..6a6fbfd 100644 --- a/payloads/libpayload/drivers/storage/ahci.c +++ b/payloads/libpayload/drivers/storage/ahci.c @@ -228,34 +228,27 @@ }; #endif
-void ahci_initialize(pcidev_t dev) +void ahci_initialize(struct pci_dev *dev) { int i;
- const u16 class = pci_read_config16(dev, 0xa); - if (class != 0x0106) - return; - const u16 vendor = pci_read_config16(dev, 0x00); - const u16 device = pci_read_config16(dev, 0x02); - #if CONFIG(LP_STORAGE_AHCI_ONLY_TESTED) - const u32 vendor_device = pci_read_config32(dev, 0x0); + const u32 vendor_device = dev->vendor_id | dev->device_id << 16; for (i = 0; i < ARRAY_SIZE(working_controllers); ++i) if (vendor_device == working_controllers[i]) break; if (i == ARRAY_SIZE(working_controllers)) { printf("ahci: Not using untested SATA controller " - "%02x:%02x.%02x (%04x:%04x).\n", PCI_BUS(dev), - PCI_SLOT(dev), PCI_FUNC(dev), vendor, device); + "%02x:%02x.%02x (%04x:%04x).\n", dev->bus, + dev->dev, dev->func, dev->vendor_id, dev->device_id); return; } #endif
printf("ahci: Found SATA controller %02x:%02x.%02x (%04x:%04x).\n", - PCI_BUS(dev), PCI_SLOT(dev), PCI_FUNC(dev), vendor, device); + dev->bus, dev->dev, dev->func, dev->vendor_id, dev->device_id);
- hba_ctrl_t *const ctrl = phys_to_virt( - pci_read_config32(dev, 0x24) & ~0x3ff); + hba_ctrl_t *const ctrl = phys_to_virt(pci_read_long(dev, 0x24) & ~0x3ff); hba_port_t *const ports = ctrl->ports;
/* Reset host controller. */ @@ -274,8 +267,8 @@ ctrl->global_ctrl |= HBA_CTRL_AHCI_EN;
/* Enable bus mastering. */ - const u16 command = pci_read_config16(dev, PCI_COMMAND); - pci_write_config16(dev, PCI_COMMAND, command | PCI_COMMAND_MASTER); + const u16 command = pci_read_word(dev, PCI_COMMAND); + pci_write_word(dev, PCI_COMMAND, command | PCI_COMMAND_MASTER);
/* Probe for devices. */ for (i = 0; i < 32; ++i) { diff --git a/payloads/libpayload/drivers/storage/storage.c b/payloads/libpayload/drivers/storage/storage.c index addf8d2..a3b31c5 100644 --- a/payloads/libpayload/drivers/storage/storage.c +++ b/payloads/libpayload/drivers/storage/storage.c @@ -115,7 +115,7 @@ switch (dev->device_class) { #if CONFIG(LP_STORAGE_AHCI) case PCI_CLASS_STORAGE_AHCI: - ahci_initialize(PCI_DEV(dev->bus, dev->dev, dev->func)); + ahci_initialize(dev); break; #endif default: diff --git a/payloads/libpayload/include/storage/ahci.h b/payloads/libpayload/include/storage/ahci.h index 2843624..18a0920 100644 --- a/payloads/libpayload/include/storage/ahci.h +++ b/payloads/libpayload/include/storage/ahci.h @@ -26,9 +26,11 @@ * SUCH DAMAGE. */
+#include <pci/pci.h> + #ifndef _STORAGE_AHCI_H #define _STORAGE_AHCI_H
-void ahci_initialize(pcidev_t dev); +void ahci_initialize(struct pci_dev *dev);
#endif