Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40010 )
Change subject: sb/intel/bd82x6x/sata: Fix IDE legacy mode, plus more ......................................................................
sb/intel/bd82x6x/sata: Fix IDE legacy mode, plus more
Assign the correct i/o resources for legacy mode, don't set legacy timing values that don't affect the hardware but set some things missed originally because of formatting issues in the BIOS spec.
Change-Id: I0b078d7790ca801a89066ef6a161d900be5eb778 Signed-off-by: Nico Huber nico.huber@secunet.com --- M src/southbridge/intel/bd82x6x/sata.c 1 file changed, 71 insertions(+), 28 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/40010/1
diff --git a/src/southbridge/intel/bd82x6x/sata.c b/src/southbridge/intel/bd82x6x/sata.c index 0f7a042..9ab7112 100644 --- a/src/southbridge/intel/bd82x6x/sata.c +++ b/src/southbridge/intel/bd82x6x/sata.c @@ -40,6 +40,62 @@ pci_write_config32(dev, SATA_SIRD, value); }
+static void sata_read_resources(struct device *dev) +{ + pci_dev_read_resources(dev); + + u8 sata_mode = 0; + get_option(&sata_mode, "sata_mode"); + if (sata_mode == 2) { + struct resource *res; + + if ((res = find_resource(dev, PCI_BASE_ADDRESS_0))) { + res->base = 0x1f0; + res->size = 8; + res->flags = IORESOURCE_IO | + IORESOURCE_ASSIGNED | IORESOURCE_FIXED; + } + + if ((res = find_resource(dev, PCI_BASE_ADDRESS_1))) { + res->base = 0x3f4; + res->size = 4; + res->flags = IORESOURCE_IO | + IORESOURCE_ASSIGNED | IORESOURCE_FIXED; + } + + if ((res = find_resource(dev, PCI_BASE_ADDRESS_2))) { + res->base = 0x170; + res->size = 8; + res->flags = IORESOURCE_IO | + IORESOURCE_ASSIGNED | IORESOURCE_FIXED; + } + + if ((res = find_resource(dev, PCI_BASE_ADDRESS_3))) { + res->base = 0x374; + res->size = 4; + res->flags = IORESOURCE_IO | + IORESOURCE_ASSIGNED | IORESOURCE_FIXED; + } + } +} + +static void sata_set_resources(struct device *dev) +{ + /* work around bug in pci_dev_set_resources(), it bails out on FIXED */ + + u8 sata_mode = 0; + get_option(&sata_mode, "sata_mode"); + if (sata_mode == 2) { + unsigned int i; + for (i = PCI_BASE_ADDRESS_0; i <= PCI_BASE_ADDRESS_3; i += 4) { + struct resource *res; + if ((res = find_resource(dev, i))) + res->flags &= ~IORESOURCE_FIXED; + } + } + pci_dev_set_resources(dev); +} + static void sata_init(struct device *dev) { u32 reg32; @@ -128,12 +184,8 @@ write32(abar + 0xa0, reg32); } else { /* IDE */ - printk(BIOS_DEBUG, "SATA: Controller in plain mode.\n");
- /* No AHCI: clear AHCI base */ - pci_write_config32(dev, 0x24, 0x00000000); - - /* And without AHCI BAR no memory decoding */ + /* Without AHCI BAR no memory decoding */ reg16 = pci_read_config16(dev, PCI_COMMAND); reg16 &= ~PCI_COMMAND_MEMORY; pci_write_config16(dev, PCI_COMMAND, reg16); @@ -141,35 +193,21 @@ if (sata_mode == 1) { /* Native mode on both primary and secondary. */ pci_or_config8(dev, 0x09, 0x05); + printk(BIOS_DEBUG, "SATA: Controller in IDE compat mode.\n"); } else { /* Legacy mode on both primary and secondary. */ pci_update_config8(dev, 0x09, ~0x05, 0x00); + printk(BIOS_DEBUG, "SATA: Controller in IDE legacy mode.\n"); }
- /* Set Interrupt Line */ - /* Interrupt Pin is set by D31IP.PIP */ - pci_write_config8(dev, INTR_LN, 0xff); + /* Enable i/o decoding */ + pci_write_config16(dev, IDE_TIM_PRI, IDE_DECODE_ENABLE); + pci_write_config16(dev, IDE_TIM_SEC, IDE_DECODE_ENABLE);
- /* 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_SITRE | IDE_ISP_3_CLOCKS | - IDE_RCT_1_CLOCKS | IDE_IE0 | IDE_TIME0); - - /* Sync DMA */ - pci_write_config16(dev, IDE_SDMA_CNT, IDE_SSDE0 | IDE_PSDE0); - pci_write_config16(dev, IDE_SDMA_TIM, 0x0201); - - /* Set IDE I/O Configuration */ - reg32 = SIG_MODE_PRI_NORMAL | FAST_PCB1 | FAST_PCB0 | PCB1 | PCB0; - pci_write_config32(dev, IDE_CONFIG, reg32); - - /* Port enable */ + /* Port enable + OOB retry mode */ reg16 = pci_read_config16(dev, 0x92); reg16 &= ~0x3f; - reg16 |= config->sata_port_map; + reg16 |= config->sata_port_map | 0x8000; pci_write_config16(dev, 0x92, reg16);
/* SATA Initialization register */ @@ -212,6 +250,11 @@
pch_iobp_update(0xea004001, 0x3fffffff, 0xc0000000); pch_iobp_update(0xea00408a, 0xfffffcff, 0x00000100); + + pci_update_config32(dev, 0x98, + ~(1 << 16 | 0x3f << 7 | 3 << 5 | 3 << 3), + 1 << 24 | 1 << 22 | 1 << 20 | 1 << 19 | + 1 << 18 | 1 << 14 | 0x04 << 7 | 1 << 3); }
static void sata_enable(struct device *dev) @@ -255,8 +298,8 @@ };
static struct device_operations sata_ops = { - .read_resources = pci_dev_read_resources, - .set_resources = pci_dev_set_resources, + .read_resources = sata_read_resources, + .set_resources = sata_set_resources, .enable_resources = pci_dev_enable_resources, .acpi_fill_ssdt_generator = sata_fill_ssdt,
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40010 )
Change subject: sb/intel/bd82x6x/sata: Fix IDE legacy mode, plus more ......................................................................
Patch Set 1:
(5 comments)
https://review.coreboot.org/c/coreboot/+/40010/1/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/sata.c:
https://review.coreboot.org/c/coreboot/+/40010/1/src/southbridge/intel/bd82x... PS1, Line 52: if ((res = find_resource(dev, PCI_BASE_ADDRESS_0))) { do not use assignment in if condition
https://review.coreboot.org/c/coreboot/+/40010/1/src/southbridge/intel/bd82x... PS1, Line 59: if ((res = find_resource(dev, PCI_BASE_ADDRESS_1))) { do not use assignment in if condition
https://review.coreboot.org/c/coreboot/+/40010/1/src/southbridge/intel/bd82x... PS1, Line 66: if ((res = find_resource(dev, PCI_BASE_ADDRESS_2))) { do not use assignment in if condition
https://review.coreboot.org/c/coreboot/+/40010/1/src/southbridge/intel/bd82x... PS1, Line 73: if ((res = find_resource(dev, PCI_BASE_ADDRESS_3))) { do not use assignment in if condition
https://review.coreboot.org/c/coreboot/+/40010/1/src/southbridge/intel/bd82x... PS1, Line 92: if ((res = find_resource(dev, i))) do not use assignment in if condition
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40010 )
Change subject: sb/intel/bd82x6x/sata: Fix IDE legacy mode, plus more ......................................................................
Patch Set 1:
I remembered that the was more on the topic when I saw the legacy-option change. We might want to break this down into smaller changes...
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40010 )
Change subject: sb/intel/bd82x6x/sata: Fix IDE legacy mode, plus more ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40010/1/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/sata.c:
https://review.coreboot.org/c/coreboot/+/40010/1/src/southbridge/intel/bd82x... PS1, Line 189: reg16 = pci_read_config16(dev, PCI_COMMAND); : reg16 &= ~PCI_COMMAND_MEMORY; : pci_write_config16(dev, PCI_COMMAND, reg16); I wonder if this is a no-op, why should we have set the MEMORY bit?
Hello Felix Singer, build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40010
to look at the new patch set (#2).
Change subject: sb/intel/bd82x6x/sata: Fix IDE legacy mode, plus more ......................................................................
sb/intel/bd82x6x/sata: Fix IDE legacy mode, plus more
Assign the correct i/o resources for legacy mode, don't set legacy timing values that don't affect the hardware but set some things missed originally because of formatting issues in the BIOS spec.
Change-Id: I0b078d7790ca801a89066ef6a161d900be5eb778 Signed-off-by: Nico Huber nico.huber@secunet.com --- M src/southbridge/intel/bd82x6x/sata.c 1 file changed, 74 insertions(+), 28 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/40010/2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40010 )
Change subject: sb/intel/bd82x6x/sata: Fix IDE legacy mode, plus more ......................................................................
Patch Set 2:
(8 comments)
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/sata.c:
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 131: printk(BIOS_DEBUG, "SATA: Controller in plain mode.\n"); Into CB:39828, please.
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 133: /* No AHCI: clear AHCI base */ : pci_write_config32(dev, 0x24, 0x00000000); : : /* And without AHCI BAR no memory decoding */ Felix, this should be a separate change. For the commit message:
Assignment of PCI resource registers is up to the allocator.
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 149: /* Set Interrupt Line */ : /* Interrupt Pin is set by D31IP.PIP */ : pci_write_config8(dev, INTR_LN, 0xff); Can be a separate change:
Don't try to write read-only register.
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 153: /* 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_SITRE | IDE_ISP_3_CLOCKS | : IDE_RCT_1_CLOCKS | IDE_IE0 | IDE_TIME0); : : /* Sync DMA */ : pci_write_config16(dev, IDE_SDMA_CNT, IDE_SSDE0 | IDE_PSDE0); : pci_write_config16(dev, IDE_SDMA_TIM, 0x0201); : : /* Set IDE I/O Configuration */ : reg32 = SIG_MODE_PRI_NORMAL | FAST_PCB1 | FAST_PCB0 | PCB1 | PCB0; : pci_write_config32(dev, IDE_CONFIG, reg32); These don't affect hardware according to documentation and, IIRC, vendor didn't set them either.
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 43: static void sata_read_resources(struct device *dev) : { : struct resource *res; : : pci_dev_read_resources(dev); : : /* Assign fixed resources for IDE legacy mode */ : : u8 sata_mode = 0; : get_option(&sata_mode, "sata_mode"); : if (sata_mode != 2) : return; : : res = find_resource(dev, PCI_BASE_ADDRESS_0); : if (res) { : res->base = 0x1f0; : res->size = 8; : res->flags = IORESOURCE_IO | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; : } : : res = find_resource(dev, PCI_BASE_ADDRESS_1); : if (res) { : res->base = 0x3f4; : res->size = 4; : res->flags = IORESOURCE_IO | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; : } : : res = find_resource(dev, PCI_BASE_ADDRESS_2); : if (res) { : res->base = 0x170; : res->size = 8; : res->flags = IORESOURCE_IO | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; : } : : res = find_resource(dev, PCI_BASE_ADDRESS_3); : if (res) { : res->base = 0x374; : res->size = 4; : res->flags = IORESOURCE_IO | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; : } : } : : static void sata_set_resources(struct device *dev) : { : /* work around bug in pci_dev_set_resources(), it bails out on FIXED */ : u8 sata_mode = 0; : get_option(&sata_mode, "sata_mode"); : if (sata_mode == 2) { : unsigned int i; : for (i = PCI_BASE_ADDRESS_0; i <= PCI_BASE_ADDRESS_3; i += 4) { : struct resource *const res find_resource(dev, i); : if (res) : res->flags &= ~IORESOURCE_FIXED; : } : } : : pci_dev_set_resources(dev); : } Felix, please squash these into CB:39828.
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 199: printk(BIOS_DEBUG, "SATA: Controller in IDE compat mode.\n"); Into CB:39828, please.
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 203: printk(BIOS_DEBUG, "SATA: Controller in IDE legacy mode.\n"); Into CB:39828, please.
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 260: 1 << 18 | 1 << 14 | 0x04 << 7 | 1 << 3); Separate change, please. For the commit message:
Set some things missed originally because of formatting issues in the BIOS spec. Values were compared with a vendor dump.
(it won't get better than this, the spec also just lists numbers)
Felix Singer has uploaded a new patch set (#3) to the change originally created by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/40010 )
Change subject: sb/intel/bd82x6x/sata: Fix IDE legacy mode, plus more ......................................................................
sb/intel/bd82x6x/sata: Fix IDE legacy mode, plus more
Assign the correct i/o resources for legacy mode, don't set legacy timing values that don't affect the hardware but set some things missed originally because of formatting issues in the BIOS spec.
Change-Id: I0b078d7790ca801a89066ef6a161d900be5eb778 Signed-off-by: Nico Huber nico.huber@secunet.com --- M src/southbridge/intel/bd82x6x/sata.c 1 file changed, 5 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/40010/3
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40010 )
Change subject: sb/intel/bd82x6x/sata: Fix IDE legacy mode, plus more ......................................................................
Patch Set 3:
(7 comments)
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/sata.c:
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 131: printk(BIOS_DEBUG, "SATA: Controller in plain mode.\n");
Into CB:39828, please.
Done
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 133: /* No AHCI: clear AHCI base */ : pci_write_config32(dev, 0x24, 0x00000000); : : /* And without AHCI BAR no memory decoding */
Felix, this should be a separate change. For the commit message: […]
Done
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 149: /* Set Interrupt Line */ : /* Interrupt Pin is set by D31IP.PIP */ : pci_write_config8(dev, INTR_LN, 0xff);
Can be a separate change: […]
Done
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 43: static void sata_read_resources(struct device *dev) : { : struct resource *res; : : pci_dev_read_resources(dev); : : /* Assign fixed resources for IDE legacy mode */ : : u8 sata_mode = 0; : get_option(&sata_mode, "sata_mode"); : if (sata_mode != 2) : return; : : res = find_resource(dev, PCI_BASE_ADDRESS_0); : if (res) { : res->base = 0x1f0; : res->size = 8; : res->flags = IORESOURCE_IO | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; : } : : res = find_resource(dev, PCI_BASE_ADDRESS_1); : if (res) { : res->base = 0x3f4; : res->size = 4; : res->flags = IORESOURCE_IO | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; : } : : res = find_resource(dev, PCI_BASE_ADDRESS_2); : if (res) { : res->base = 0x170; : res->size = 8; : res->flags = IORESOURCE_IO | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; : } : : res = find_resource(dev, PCI_BASE_ADDRESS_3); : if (res) { : res->base = 0x374; : res->size = 4; : res->flags = IORESOURCE_IO | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; : } : } : : static void sata_set_resources(struct device *dev) : { : /* work around bug in pci_dev_set_resources(), it bails out on FIXED */ : u8 sata_mode = 0; : get_option(&sata_mode, "sata_mode"); : if (sata_mode == 2) { : unsigned int i; : for (i = PCI_BASE_ADDRESS_0; i <= PCI_BASE_ADDRESS_3; i += 4) { : struct resource *const res find_resource(dev, i); : if (res) : res->flags &= ~IORESOURCE_FIXED; : } : } : : pci_dev_set_resources(dev); : }
Felix, please squash these into CB:39828.
Done
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 199: printk(BIOS_DEBUG, "SATA: Controller in IDE compat mode.\n");
Into CB:39828, please.
Done
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 203: printk(BIOS_DEBUG, "SATA: Controller in IDE legacy mode.\n");
Into CB:39828, please.
Done
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 260: 1 << 18 | 1 << 14 | 0x04 << 7 | 1 << 3);
Separate change, please. For the commit message: […]
Done
Hello Felix Singer, build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40010
to look at the new patch set (#5).
Change subject: sb/intel/bd82x6x/sata: Clean up IDE modes ......................................................................
sb/intel/bd82x6x/sata: Clean up IDE modes
Don't set legacy timing values that don't affect the hardware but enable the OOB retry mode as already done on the AHCI path.
Change-Id: I0b078d7790ca801a89066ef6a161d900be5eb778 Signed-off-by: Nico Huber nico.huber@secunet.com --- M src/southbridge/intel/bd82x6x/sata.c 1 file changed, 5 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/40010/5
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40010 )
Change subject: sb/intel/bd82x6x/sata: Clean up IDE modes ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40010/1/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/sata.c:
https://review.coreboot.org/c/coreboot/+/40010/1/src/southbridge/intel/bd82x... PS1, Line 189: reg16 = pci_read_config16(dev, PCI_COMMAND); : reg16 &= ~PCI_COMMAND_MEMORY; : pci_write_config16(dev, PCI_COMMAND, reg16);
I wonder if this is a no-op, why should we have set the MEMORY bit?
Ack (wasn't meant as something to be resolved here)
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/sata.c:
https://review.coreboot.org/c/coreboot/+/40010/2/src/southbridge/intel/bd82x... PS2, Line 153: /* 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_SITRE | IDE_ISP_3_CLOCKS | : IDE_RCT_1_CLOCKS | IDE_IE0 | IDE_TIME0); : : /* Sync DMA */ : pci_write_config16(dev, IDE_SDMA_CNT, IDE_SSDE0 | IDE_PSDE0); : pci_write_config16(dev, IDE_SDMA_TIM, 0x0201); : : /* Set IDE I/O Configuration */ : reg32 = SIG_MODE_PRI_NORMAL | FAST_PCB1 | FAST_PCB0 | PCB1 | PCB0; : pci_write_config32(dev, IDE_CONFIG, reg32);
These don't affect hardware according to documentation and, IIRC, vendor […]
Ack (mentioned in the commit message)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40010 )
Change subject: sb/intel/bd82x6x/sata: Clean up IDE modes ......................................................................
Patch Set 5: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/40010/5/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/sata.c:
https://review.coreboot.org/c/coreboot/+/40010/5/src/southbridge/intel/bd82x... PS5, Line 190: i/o I/O
Felix Singer has uploaded a new patch set (#7) to the change originally created by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/40010 )
Change subject: sb/intel/bd82x6x/sata: Clean up IDE modes ......................................................................
sb/intel/bd82x6x/sata: Clean up IDE modes
Don't set legacy timing values that don't affect the hardware but enable the OOB retry mode as already done on the AHCI path.
Change-Id: I0b078d7790ca801a89066ef6a161d900be5eb778 Signed-off-by: Nico Huber nico.huber@secunet.com --- M src/southbridge/intel/bd82x6x/sata.c 1 file changed, 5 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/40010/7
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40010 )
Change subject: sb/intel/bd82x6x/sata: Clean up IDE modes ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40010/5/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/sata.c:
https://review.coreboot.org/c/coreboot/+/40010/5/src/southbridge/intel/bd82x... PS5, Line 190: i/o
I/O
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40010 )
Change subject: sb/intel/bd82x6x/sata: Clean up IDE modes ......................................................................
Patch Set 7: Code-Review+2
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40010 )
Change subject: sb/intel/bd82x6x/sata: Clean up IDE modes ......................................................................
sb/intel/bd82x6x/sata: Clean up IDE modes
Don't set legacy timing values that don't affect the hardware but enable the OOB retry mode as already done on the AHCI path.
Change-Id: I0b078d7790ca801a89066ef6a161d900be5eb778 Signed-off-by: Nico Huber nico.huber@secunet.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/40010 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/bd82x6x/sata.c 1 file changed, 5 insertions(+), 17 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/southbridge/intel/bd82x6x/sata.c b/src/southbridge/intel/bd82x6x/sata.c index 6c6e5be..75a1b1a 100644 --- a/src/southbridge/intel/bd82x6x/sata.c +++ b/src/southbridge/intel/bd82x6x/sata.c @@ -187,26 +187,14 @@ printk(BIOS_DEBUG, "SATA: Controller in IDE legacy mode.\n"); }
- /* 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_SITRE | IDE_ISP_3_CLOCKS | - IDE_RCT_1_CLOCKS | IDE_IE0 | IDE_TIME0); + /* Enable I/O decoding */ + pci_write_config16(dev, IDE_TIM_PRI, IDE_DECODE_ENABLE); + pci_write_config16(dev, IDE_TIM_SEC, IDE_DECODE_ENABLE);
- /* Sync DMA */ - pci_write_config16(dev, IDE_SDMA_CNT, IDE_SSDE0 | IDE_PSDE0); - pci_write_config16(dev, IDE_SDMA_TIM, 0x0201); - - /* Set IDE I/O Configuration */ - reg32 = SIG_MODE_PRI_NORMAL | FAST_PCB1 | FAST_PCB0 | PCB1 | PCB0; - pci_write_config32(dev, IDE_CONFIG, reg32); - - /* Port enable */ + /* Port enable + OOB retry mode */ reg16 = pci_read_config16(dev, 0x92); reg16 &= ~0x3f; - reg16 |= config->sata_port_map; + reg16 |= config->sata_port_map | 0x8000; pci_write_config16(dev, 0x92, reg16);
/* SATA Initialization register */
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40010 )
Change subject: sb/intel/bd82x6x/sata: Clean up IDE modes ......................................................................
Patch Set 8:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/2359 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2358 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2357
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
build bot (Jenkins) (Code Review)
-
Felix Singer (Code Review)
-
Nico Huber (Code Review)