Change in coreboot[master]: sb/intel/lynxpoint/sata.c: Update as per newer reference code
Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47032 )
Change subject: sb/intel/lynxpoint/sata.c: Update as per newer reference code ......................................................................
sb/intel/lynxpoint/sata.c: Update as per newer reference code
Newer Lynxpoint reference code does some more stuff, which Broadwell already does. Also drop invalid register writes, which were copy-pasted from older southbridges and do not exist anymore on Lynxpoint.
Change-Id: I0907905adfa9a21732f1918f143a1df1954275f8 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/lynxpoint/pch.h M src/southbridge/intel/lynxpoint/sata.c 2 files changed, 69 insertions(+), 89 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/47032/1
diff --git a/src/southbridge/intel/lynxpoint/pch.h b/src/southbridge/intel/lynxpoint/pch.h index 66cd05e..a1d504c 100644 --- a/src/southbridge/intel/lynxpoint/pch.h +++ b/src/southbridge/intel/lynxpoint/pch.h @@ -215,51 +215,10 @@ /* PCI Configuration Space (D31:F2): SATA */ #define PCH_SATA_DEV PCI_DEV(0, 0x1f, 2) #define PCH_SATA2_DEV PCI_DEV(0, 0x1f, 5) -#define INTR_LN 0x3c #define IDE_TIM_PRI 0x40 /* IDE timings, primary */ #define IDE_DECODE_ENABLE (1 << 15) -#define IDE_SITRE (1 << 14) -#define IDE_ISP_5_CLOCKS (0 << 12) -#define IDE_ISP_4_CLOCKS (1 << 12) -#define IDE_ISP_3_CLOCKS (2 << 12) -#define IDE_RCT_4_CLOCKS (0 << 8) -#define IDE_RCT_3_CLOCKS (1 << 8) -#define IDE_RCT_2_CLOCKS (2 << 8) -#define IDE_RCT_1_CLOCKS (3 << 8) -#define IDE_DTE1 (1 << 7) -#define IDE_PPE1 (1 << 6) -#define IDE_IE1 (1 << 5) -#define IDE_TIME1 (1 << 4) -#define IDE_DTE0 (1 << 3) -#define IDE_PPE0 (1 << 2) -#define IDE_IE0 (1 << 1) -#define IDE_TIME0 (1 << 0) #define IDE_TIM_SEC 0x42 /* IDE timings, secondary */
-#define IDE_SDMA_CNT 0x48 /* Synchronous DMA control */ -#define IDE_SSDE1 (1 << 3) -#define IDE_SSDE0 (1 << 2) -#define IDE_PSDE1 (1 << 1) -#define IDE_PSDE0 (1 << 0) - -#define IDE_SDMA_TIM 0x4a - -#define IDE_CONFIG 0x54 /* IDE I/O Configuration Register */ -#define SIG_MODE_SEC_NORMAL (0 << 18) -#define SIG_MODE_SEC_TRISTATE (1 << 18) -#define SIG_MODE_SEC_DRIVELOW (2 << 18) -#define SIG_MODE_PRI_NORMAL (0 << 16) -#define SIG_MODE_PRI_TRISTATE (1 << 16) -#define SIG_MODE_PRI_DRIVELOW (2 << 16) -#define FAST_SCB1 (1 << 15) -#define FAST_SCB0 (1 << 14) -#define FAST_PCB1 (1 << 13) -#define FAST_PCB0 (1 << 12) -#define SCB1 (1 << 3) -#define SCB0 (1 << 2) -#define PCB1 (1 << 1) -#define PCB0 (1 << 0) - #define SATA_SIRI 0xa0 /* SATA Indexed Register Index */ #define SATA_SIRD 0xa4 /* SATA Indexed Register Data */ #define SATA_SP 0xd0 /* Scratchpad */ diff --git a/src/southbridge/intel/lynxpoint/sata.c b/src/southbridge/intel/lynxpoint/sata.c index d8eb2a8..18dbb00 100644 --- a/src/southbridge/intel/lynxpoint/sata.c +++ b/src/southbridge/intel/lynxpoint/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 "chip.h" #include "iobp.h" #include "pch.h" @@ -37,50 +38,35 @@ { u32 reg32;
- u32 *abar; - /* Get the chip configuration */ config_t *config = dev->chip_info;
- printk(BIOS_DEBUG, "SATA: Initializing...\n"); + printk(BIOS_DEBUG, "SATA: Initializing controller in AHCI mode.\n");
if (config == NULL) { printk(BIOS_ERR, "SATA: ERROR: Device not in devicetree.cb!\n"); return; }
- /* SATA configuration */ - /* Enable memory space decoding for ABAR */ pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MEMORY | PCI_COMMAND_IO);
- printk(BIOS_DEBUG, "SATA: Controller in AHCI mode.\n"); + 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, INTR_LN, 0x0a); + pci_write_config8(dev, PCI_INTERRUPT_LINE, 0x0a);
- /* Set timings */ - pci_write_config16(dev, IDE_TIM_PRI, IDE_DECODE_ENABLE | - IDE_ISP_3_CLOCKS | IDE_RCT_1_CLOCKS | - IDE_PPE0 | IDE_IE0 | IDE_TIME0); - pci_write_config16(dev, IDE_TIM_SEC, IDE_DECODE_ENABLE | - IDE_ISP_5_CLOCKS | IDE_RCT_4_CLOCKS); - - /* Sync DMA */ - pci_write_config16(dev, IDE_SDMA_CNT, IDE_PSDE0); - pci_write_config16(dev, IDE_SDMA_TIM, 0x0001); - - /* Set IDE I/O Configuration */ - reg32 = SIG_MODE_PRI_NORMAL | FAST_PCB1 | FAST_PCB0 | PCB1 | PCB0; - pci_write_config32(dev, IDE_CONFIG, reg32); + pci_write_config16(dev, IDE_TIM_PRI, IDE_DECODE_ENABLE); + pci_write_config16(dev, IDE_TIM_SEC, IDE_DECODE_ENABLE);
/* for AHCI, Port Enable is managed in memory mapped space */ pci_update_config16(dev, 0x92, ~0x3f, 0x8000 | config->sata_port_map); udelay(2);
/* Setup register 98h */ - reg32 = pci_read_config16(dev, 0x98); + reg32 = pci_read_config32(dev, 0x98); reg32 |= 1 << 19; /* BWG step 6 */ reg32 |= 1 << 22; /* BWG step 5 */ reg32 &= ~(0x3f << 7); @@ -93,7 +79,7 @@ if (pch_is_lp()) { reg32 &= ~((1 << 31) | (1 << 30)); reg32 |= 1 << 23; - reg32 |= 1 << 24; /* Disable listen mode (hotplug) */ + reg32 |= 1 << 24; /* Enable MPHY Dynamic Power Gating */ } pci_write_config32(dev, 0x98, reg32);
@@ -107,31 +93,60 @@ pci_write_config32(dev, 0x94, reg32);
/* Initialize AHCI memory-mapped space */ - abar = (u32 *)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 + + union abar_reg_cap abar_cap = { + .raw = abar_read32(abar, ABAR_REG_CAP), + }; + + abar_cap.external_sata = 0; /* Should be configurable */ + abar_cap.enclosure_management = 0; + abar_cap.port_multiplier = 0; + abar_cap.interlock_switch = 0; + + abar_cap.partial_state = 1; + abar_cap.slumber_state = 1; + abar_cap.pio_multiple_drq = 1; + abar_cap.cmd_list_override = 1; + abar_cap.interface_speed = 3; + abar_cap.aggressive_link_pm = 1; + abar_cap.staggered_spinup = 1; + if (pch_is_lp()) - reg32 |= (1 << 18); // SAM: SATA AHCI MODE ONLY - write32(abar + 0x00, reg32); - /* PI (Ports implemented) */ - write32(abar + 0x03, config->sata_port_map); - (void)read32(abar + 0x03); /* Read back 1 */ - (void)read32(abar + 0x03); /* Read back 2 */ - /* CAP2 (HBA Capabilities Extended)*/ - reg32 = read32(abar + 0x09); - /* Enable DEVSLP */ + 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 (pch_is_lp()) { - if (config->sata_devslp_disable) - reg32 &= ~(1 << 3); - else - reg32 |= (1 << 5)|(1 << 4)|(1 << 3)|(1 << 2); - } else { - reg32 &= ~0x00000002; + if (config->sata_devslp_disable) { + abar_cap_2.supports_device_sleep = 0; + + 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; + + 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); + } + } } - write32(abar + 0x09, reg32);
/* Set Gen3 Transmitter settings if needed */ if (config->sata_port0_gen3_tx) @@ -167,8 +182,9 @@ << SATA_DTLE_EDGE_SHIFT); }
- /* Additional Programming Requirements */ - /* Power Optimizer */ + /* + * Additional Programming Requirements for Power Optimizer + */
/* Step 1 */ if (pch_is_lp()) @@ -196,9 +212,14 @@ }
reg32 = pci_read_config32(dev, 0x300); - reg32 |= (1 << 17) | (1 << 16); + reg32 |= (1 << 17) | (1 << 16) | (1 << 19); reg32 |= (1 << 31) | (1 << 30) | (1 << 29); pci_write_config32(dev, 0x300, reg32); + + pci_or_config32(dev, 0x98, 1 << 29); + + /* Register Lock */ + pci_or_config32(dev, 0x9c, 1 << 31); }
static void sata_enable(struct device *dev)
Hello Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47032
to look at the new patch set (#2).
Change subject: sb/intel/lynxpoint/sata.c: Update as per newer reference code ......................................................................
sb/intel/lynxpoint/sata.c: Update as per newer reference code
Newer Lynxpoint reference code does some more stuff, which Broadwell already does. Also drop invalid register writes, which were copy-pasted from older southbridges and do not exist anymore on Lynxpoint.
Change-Id: I0907905adfa9a21732f1918f143a1df1954275f8 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/lynxpoint/pch.h M src/southbridge/intel/lynxpoint/sata.c 2 files changed, 69 insertions(+), 89 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/47032/2
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47032
to look at the new patch set (#3).
Change subject: sb/intel/lynxpoint/sata.c: Update as per newer reference code ......................................................................
sb/intel/lynxpoint/sata.c: Update as per newer reference code
Newer Lynxpoint reference code does some more stuff, which Broadwell already does. Also drop invalid register writes, which were copy-pasted from older southbridges and do not exist anymore on Lynxpoint.
Change-Id: I0907905adfa9a21732f1918f143a1df1954275f8 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/lynxpoint/pch.h M src/southbridge/intel/lynxpoint/sata.c 2 files changed, 65 insertions(+), 89 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/47032/3
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47032 )
Change subject: sb/intel/lynxpoint/sata.c: Update as per newer reference code ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/47032/3/src/southbridge/intel/lynxp... File src/southbridge/intel/lynxpoint/sata.c:
https://review.coreboot.org/c/coreboot/+/47032/3/src/southbridge/intel/lynxp... PS3, Line 170: /* Additional Programming Requirements */ actually this looks like the heading for lines 171-201, while "Power Optimizer" and "Clock Gating" where subheadings. Thus, the change on the right doesn't make sense. When these are "additional programming requirements for p.o", where are the "programming requirements"? -> just drop line 170 should be ok
Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/47032 )
Change subject: sb/intel/lynxpoint/sata.c: Update as per newer reference code ......................................................................
Abandoned
-
Angel Pons (Code Review) -
Michael Niewöhner (Code Review)