Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47038 )
Change subject: sb/intel/*/lpc.c: Don't try to write read-only PCICMD bits ......................................................................
sb/intel/*/lpc.c: Don't try to write read-only PCICMD bits
For all these southbridges, the lower nibble of PCICMD is read-only.
Change-Id: Ib3b16b1b9651f7f3bd06ff8bc27dafd8a323e93c Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/bd82x6x/lpc.c M src/southbridge/intel/i82801dx/lpc.c M src/southbridge/intel/i82801gx/lpc.c M src/southbridge/intel/i82801ix/lpc.c M src/southbridge/intel/i82801jx/lpc.c M src/southbridge/intel/ibexpeak/lpc.c M src/southbridge/intel/lynxpoint/lpc.c 7 files changed, 0 insertions(+), 35 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/47038/1
diff --git a/src/southbridge/intel/bd82x6x/lpc.c b/src/southbridge/intel/bd82x6x/lpc.c index df5625a..a498f87 100644 --- a/src/southbridge/intel/bd82x6x/lpc.c +++ b/src/southbridge/intel/bd82x6x/lpc.c @@ -519,11 +519,6 @@ /* Print detected platform */ report_pch_info(dev);
- /* Set the value for PCI command register. */ - pci_write_config16(dev, PCI_COMMAND, - PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - /* IO APIC initialization. */ pch_enable_ioapic(dev);
diff --git a/src/southbridge/intel/i82801dx/lpc.c b/src/southbridge/intel/i82801dx/lpc.c index dec1266..f207c11 100644 --- a/src/southbridge/intel/i82801dx/lpc.c +++ b/src/southbridge/intel/i82801dx/lpc.c @@ -252,11 +252,6 @@
static void lpc_init(struct device *dev) { - /* Set the value for PCI command register. */ - pci_write_config16(dev, PCI_COMMAND, - PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - i82801dx_enable_acpi(dev); /* IO APIC initialization. */ i82801dx_enable_ioapic(dev); diff --git a/src/southbridge/intel/i82801gx/lpc.c b/src/southbridge/intel/i82801gx/lpc.c index eb59eb0..721735c 100644 --- a/src/southbridge/intel/i82801gx/lpc.c +++ b/src/southbridge/intel/i82801gx/lpc.c @@ -341,11 +341,6 @@ { printk(BIOS_DEBUG, "i82801gx: %s\n", __func__);
- /* Set the value for PCI command register. */ - pci_write_config16(dev, PCI_COMMAND, - PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - /* IO APIC initialization. */ i82801gx_enable_ioapic(dev);
diff --git a/src/southbridge/intel/i82801ix/lpc.c b/src/southbridge/intel/i82801ix/lpc.c index ad7141a..652da54 100644 --- a/src/southbridge/intel/i82801ix/lpc.c +++ b/src/southbridge/intel/i82801ix/lpc.c @@ -355,11 +355,6 @@ { printk(BIOS_DEBUG, "i82801ix: %s\n", __func__);
- /* Set the value for PCI command register. */ - pci_write_config16(dev, PCI_COMMAND, - PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - /* IO APIC initialization. */ i82801ix_enable_apic(dev);
diff --git a/src/southbridge/intel/i82801jx/lpc.c b/src/southbridge/intel/i82801jx/lpc.c index 2f7b516..1f4cf29 100644 --- a/src/southbridge/intel/i82801jx/lpc.c +++ b/src/southbridge/intel/i82801jx/lpc.c @@ -347,11 +347,6 @@ { printk(BIOS_DEBUG, "i82801jx: %s\n", __func__);
- /* Set the value for PCI command register. */ - pci_write_config16(dev, PCI_COMMAND, - PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - /* IO APIC initialization. */ i82801jx_enable_apic(dev);
diff --git a/src/southbridge/intel/ibexpeak/lpc.c b/src/southbridge/intel/ibexpeak/lpc.c index 8269dd9..dddecd7 100644 --- a/src/southbridge/intel/ibexpeak/lpc.c +++ b/src/southbridge/intel/ibexpeak/lpc.c @@ -433,11 +433,6 @@ { printk(BIOS_DEBUG, "pch: %s\n", __func__);
- /* Set the value for PCI command register. */ - pci_write_config16(dev, PCI_COMMAND, - PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - /* IO APIC initialization. */ pch_enable_ioapic(dev);
diff --git a/src/southbridge/intel/lynxpoint/lpc.c b/src/southbridge/intel/lynxpoint/lpc.c index a168de0..2e4dcd0 100644 --- a/src/southbridge/intel/lynxpoint/lpc.c +++ b/src/southbridge/intel/lynxpoint/lpc.c @@ -488,11 +488,6 @@ { printk(BIOS_DEBUG, "pch: %s\n", __func__);
- /* Set the value for PCI command register. */ - pci_write_config16(dev, PCI_COMMAND, - PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - /* IO APIC initialization. */ pch_enable_ioapic(dev);
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47038 )
Change subject: sb/intel/*/lpc.c: Don't try to write read-only PCICMD bits ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47038/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47038/2//COMMIT_MSG@9 PS2, Line 9: For all these southbridges, the lower nibble of PCICMD is read-only. And for newer chips, the special bit is even supposed to be 0...
Anyway, was this tested? Sometimes datasheets can be deceiving and read- only actually means read-only to the OS.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47038 )
Change subject: sb/intel/*/lpc.c: Don't try to write read-only PCICMD bits ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47038/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47038/2//COMMIT_MSG@9 PS2, Line 9: For all these southbridges, the lower nibble of PCICMD is read-only.
And for newer chips, the special bit is even supposed to be 0... […]
I've tested this on Lynxpoint-H, the write has no effect.
Hello Felix Singer, build bot (Jenkins), Nico Huber, Arthur Heymans, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47038
to look at the new patch set (#3).
Change subject: sb/intel/*/lpc.c: Don't try to write read-only PCICMD bits ......................................................................
sb/intel/*/lpc.c: Don't try to write read-only PCICMD bits
For all these southbridges, the lower nibble of PCICMD is read-only.
Tested on Asrock B85M Pro4 (Lynxpoint-H), LPC's PCICMD does not change.
Change-Id: Ib3b16b1b9651f7f3bd06ff8bc27dafd8a323e93c Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/bd82x6x/lpc.c M src/southbridge/intel/i82801dx/lpc.c M src/southbridge/intel/i82801gx/lpc.c M src/southbridge/intel/i82801ix/lpc.c M src/southbridge/intel/i82801jx/lpc.c M src/southbridge/intel/ibexpeak/lpc.c M src/southbridge/intel/lynxpoint/lpc.c 7 files changed, 0 insertions(+), 35 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/47038/3
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47038 )
Change subject: sb/intel/*/lpc.c: Don't try to write read-only PCICMD bits ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47038/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47038/2//COMMIT_MSG@9 PS2, Line 9: For all these southbridges, the lower nibble of PCICMD is read-only.
I've tested this on Lynxpoint-H, the write has no effect.
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47038 )
Change subject: sb/intel/*/lpc.c: Don't try to write read-only PCICMD bits ......................................................................
Patch Set 3: Code-Review+2
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47038 )
Change subject: sb/intel/*/lpc.c: Don't try to write read-only PCICMD bits ......................................................................
sb/intel/*/lpc.c: Don't try to write read-only PCICMD bits
For all these southbridges, the lower nibble of PCICMD is read-only.
Tested on Asrock B85M Pro4 (Lynxpoint-H), LPC's PCICMD does not change.
Change-Id: Ib3b16b1b9651f7f3bd06ff8bc27dafd8a323e93c Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47038 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/southbridge/intel/bd82x6x/lpc.c M src/southbridge/intel/i82801dx/lpc.c M src/southbridge/intel/i82801gx/lpc.c M src/southbridge/intel/i82801ix/lpc.c M src/southbridge/intel/i82801jx/lpc.c M src/southbridge/intel/ibexpeak/lpc.c M src/southbridge/intel/lynxpoint/lpc.c 7 files changed, 0 insertions(+), 35 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/southbridge/intel/bd82x6x/lpc.c b/src/southbridge/intel/bd82x6x/lpc.c index df5625a..a498f87 100644 --- a/src/southbridge/intel/bd82x6x/lpc.c +++ b/src/southbridge/intel/bd82x6x/lpc.c @@ -519,11 +519,6 @@ /* Print detected platform */ report_pch_info(dev);
- /* Set the value for PCI command register. */ - pci_write_config16(dev, PCI_COMMAND, - PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - /* IO APIC initialization. */ pch_enable_ioapic(dev);
diff --git a/src/southbridge/intel/i82801dx/lpc.c b/src/southbridge/intel/i82801dx/lpc.c index dec1266..f207c11 100644 --- a/src/southbridge/intel/i82801dx/lpc.c +++ b/src/southbridge/intel/i82801dx/lpc.c @@ -252,11 +252,6 @@
static void lpc_init(struct device *dev) { - /* Set the value for PCI command register. */ - pci_write_config16(dev, PCI_COMMAND, - PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - i82801dx_enable_acpi(dev); /* IO APIC initialization. */ i82801dx_enable_ioapic(dev); diff --git a/src/southbridge/intel/i82801gx/lpc.c b/src/southbridge/intel/i82801gx/lpc.c index eb59eb0..721735c 100644 --- a/src/southbridge/intel/i82801gx/lpc.c +++ b/src/southbridge/intel/i82801gx/lpc.c @@ -341,11 +341,6 @@ { printk(BIOS_DEBUG, "i82801gx: %s\n", __func__);
- /* Set the value for PCI command register. */ - pci_write_config16(dev, PCI_COMMAND, - PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - /* IO APIC initialization. */ i82801gx_enable_ioapic(dev);
diff --git a/src/southbridge/intel/i82801ix/lpc.c b/src/southbridge/intel/i82801ix/lpc.c index ad7141a..652da54 100644 --- a/src/southbridge/intel/i82801ix/lpc.c +++ b/src/southbridge/intel/i82801ix/lpc.c @@ -355,11 +355,6 @@ { printk(BIOS_DEBUG, "i82801ix: %s\n", __func__);
- /* Set the value for PCI command register. */ - pci_write_config16(dev, PCI_COMMAND, - PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - /* IO APIC initialization. */ i82801ix_enable_apic(dev);
diff --git a/src/southbridge/intel/i82801jx/lpc.c b/src/southbridge/intel/i82801jx/lpc.c index 2f7b516..1f4cf29 100644 --- a/src/southbridge/intel/i82801jx/lpc.c +++ b/src/southbridge/intel/i82801jx/lpc.c @@ -347,11 +347,6 @@ { printk(BIOS_DEBUG, "i82801jx: %s\n", __func__);
- /* Set the value for PCI command register. */ - pci_write_config16(dev, PCI_COMMAND, - PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - /* IO APIC initialization. */ i82801jx_enable_apic(dev);
diff --git a/src/southbridge/intel/ibexpeak/lpc.c b/src/southbridge/intel/ibexpeak/lpc.c index 8269dd9..dddecd7 100644 --- a/src/southbridge/intel/ibexpeak/lpc.c +++ b/src/southbridge/intel/ibexpeak/lpc.c @@ -433,11 +433,6 @@ { printk(BIOS_DEBUG, "pch: %s\n", __func__);
- /* Set the value for PCI command register. */ - pci_write_config16(dev, PCI_COMMAND, - PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - /* IO APIC initialization. */ pch_enable_ioapic(dev);
diff --git a/src/southbridge/intel/lynxpoint/lpc.c b/src/southbridge/intel/lynxpoint/lpc.c index 4286e6ca..2872a0b 100644 --- a/src/southbridge/intel/lynxpoint/lpc.c +++ b/src/southbridge/intel/lynxpoint/lpc.c @@ -505,11 +505,6 @@ { printk(BIOS_DEBUG, "pch: %s\n", __func__);
- /* Set the value for PCI command register. */ - pci_write_config16(dev, PCI_COMMAND, - PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - /* IO APIC initialization. */ pch_enable_ioapic(dev);
-
Angel Pons (Code Review) -
Nico Huber (Code Review)