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)