Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47027 )
Change subject: soc/intel/broadwell/pch/sata.c: Use common ABAR helpers ......................................................................
soc/intel/broadwell/pch/sata.c: Use common ABAR helpers
Change-Id: I2316794628be1ebc524aaa6a460ba6770b700a56 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/pch/sata.c 1 file changed, 41 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/47027/1
diff --git a/src/soc/intel/broadwell/pch/sata.c b/src/soc/intel/broadwell/pch/sata.c index 8b2c513..e157252 100644 --- a/src/soc/intel/broadwell/pch/sata.c +++ b/src/soc/intel/broadwell/pch/sata.c @@ -7,6 +7,7 @@ #include <device/pci.h> #include <device/pci_ids.h> #include <delay.h> +#include <southbridge/intel/common/abar.h> #include <soc/intel/broadwell/pch/chip.h>
#include "iobp.h" @@ -39,14 +40,15 @@ { const struct soc_intel_broadwell_pch_config *config = config_of(dev); u32 reg32; - u8 *abar; - int port;
printk(BIOS_DEBUG, "SATA: Initializing controller in AHCI mode.\n");
/* Enable memory space decoding for ABAR */ pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MEMORY | PCI_COMMAND_IO);
+ const uintptr_t abar = pci_read_config32(dev, PCI_BASE_ADDRESS_5); + printk(BIOS_DEBUG, "ABAR: %p\n", (void *)abar); + /* Set Interrupt Line */ /* Interrupt Pin is set by D31IP.PIP */ pci_write_config8(dev, PCI_INTERRUPT_LINE, 0x0a); @@ -75,38 +77,50 @@ pci_write_config32(dev, 0x94, reg32);
/* Initialize AHCI memory-mapped space */ - abar = (u8 *)(pci_read_config32(dev, PCI_BASE_ADDRESS_5)); - printk(BIOS_DEBUG, "ABAR: %p\n", abar);
- /* CAP (HBA Capabilities) : enable power management */ - reg32 = read32(abar + 0x00); - reg32 |= 0x0c006000; /* set PSC+SSC+SALP+SSS */ - reg32 &= ~0x00020060; /* clear SXS+EMS+PMS */ - reg32 |= (1 << 18); /* SAM: SATA AHCI MODE ONLY */ - write32(abar + 0x00, reg32); + union abar_reg_cap abar_cap = { + .raw = abar_read32(abar, ABAR_REG_CAP), + };
- /* PI (Ports implemented) */ - write32(abar + 0x0c, config->sata_port_map); - (void) read32(abar + 0x0c); /* Read back 1 */ - (void) read32(abar + 0x0c); /* Read back 2 */ + abar_cap.external_sata = 0; /* Should be configurable */ + abar_cap.enclosure_management = 0; + abar_cap.port_multiplier = 0;
- /* CAP2 (HBA Capabilities Extended)*/ + abar_cap.partial_state = 1; + abar_cap.slumber_state = 1; + abar_cap.aggressive_link_pm = 1; + abar_cap.staggered_spinup = 1; + abar_cap.ahci_mode_only = 1; + + abar_write32(abar, ABAR_REG_CAP, abar_cap.raw); + + abar_write_ports_implemented(abar, config->sata_port_map); + + union abar_reg_cap_2 abar_cap_2 = { + .raw = abar_read32(abar, ABAR_REG_CAP_2), + }; + if (config->sata_devslp_disable) { - reg32 = read32(abar + 0x24); - reg32 &= ~(1 << 3); - write32(abar + 0x24, reg32); - } else { - /* Enable DEVSLP */ - reg32 = read32(abar + 0x24); - reg32 |= (1 << 5) | (1 << 4)|(1 << 3)|(1 << 2); - write32(abar + 0x24, reg32); + abar_cap_2.supports_device_sleep = 0;
- for (port = 0; port < 4; port++) { + abar_write32(abar, ABAR_REG_CAP_2, abar_cap_2.raw); + } else { + abar_cap_2.auto_partial_to_slumber = 1; + abar_cap_2.supports_device_sleep = 1; + abar_cap_2.aggressive_devslp_mgmt = 1; + abar_cap_2.devslp_entry_slumber_only = 1; + + abar_write32(abar, ABAR_REG_CAP_2, abar_cap_2.raw); + + for (int port = 0; port < 4; port++) { if (!(config->sata_port_map & (1 << port))) continue; - reg32 = read32(abar + 0x144 + (0x80 * port)); - reg32 |= (1 << 1); /* DEVSLP DSP */ - write32(abar + 0x144 + (0x80 * port), reg32); + + union abar_reg_px_devslp px_devslp = { + .raw = abar_read32(abar, ABAR_REG_PxDEVSLP(port)), + }; + px_devslp.device_sleep_present = 1; + abar_write32(abar, ABAR_REG_PxDEVSLP(port), px_devslp.raw); } }
Hello Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47027
to look at the new patch set (#2).
Change subject: soc/intel/broadwell/pch/sata.c: Use common ABAR helpers ......................................................................
soc/intel/broadwell/pch/sata.c: Use common ABAR helpers
Change-Id: I2316794628be1ebc524aaa6a460ba6770b700a56 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/pch/sata.c 1 file changed, 41 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/47027/2
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47027 )
Change subject: soc/intel/broadwell/pch/sata.c: Use common ABAR helpers ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/47027/2/src/soc/intel/broadwell/pch... File src/soc/intel/broadwell/pch/sata.c:
https://review.coreboot.org/c/coreboot/+/47027/2/src/soc/intel/broadwell/pch... PS2, Line 85: Should be configurable so, why isn't it configurable?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47027 )
Change subject: soc/intel/broadwell/pch/sata.c: Use common ABAR helpers ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47027/2/src/soc/intel/broadwell/pch... File src/soc/intel/broadwell/pch/sata.c:
https://review.coreboot.org/c/coreboot/+/47027/2/src/soc/intel/broadwell/pch... PS2, Line 85: Should be configurable
so, why isn't it configurable?
No one seems to need eSATA support in coreboot. Hotplug works already.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47027 )
Change subject: soc/intel/broadwell/pch/sata.c: Use common ABAR helpers ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47027/2/src/soc/intel/broadwell/pch... File src/soc/intel/broadwell/pch/sata.c:
https://review.coreboot.org/c/coreboot/+/47027/2/src/soc/intel/broadwell/pch... PS2, Line 85: Should be configurable
No one seems to need eSATA support in coreboot. Hotplug works already.
Hotplug works with this set to 0?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47027 )
Change subject: soc/intel/broadwell/pch/sata.c: Use common ABAR helpers ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47027/2/src/soc/intel/broadwell/pch... File src/soc/intel/broadwell/pch/sata.c:
https://review.coreboot.org/c/coreboot/+/47027/2/src/soc/intel/broadwell/pch... PS2, Line 85: Should be configurable
Hotplug works with this set to 0?
Yes, external_sata is more about eSATA than hotplug
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47027 )
Change subject: soc/intel/broadwell/pch/sata.c: Use common ABAR helpers ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47027/2/src/soc/intel/broadwell/pch... File src/soc/intel/broadwell/pch/sata.c:
https://review.coreboot.org/c/coreboot/+/47027/2/src/soc/intel/broadwell/pch... PS2, Line 85: Should be configurable
Yes, external_sata is more about eSATA than hotplug
ah, sorry, I read it like "hotplug for eSATA works" ;)
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47027 )
Change subject: soc/intel/broadwell/pch/sata.c: Use common ABAR helpers ......................................................................
Patch Set 2: Code-Review+2
-
Angel Pons (Code Review) -
Michael Niewöhner (Code Review)