Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46891 )
Change subject: soc/intel/broadwell/pch: Simplify PCI RMW operations ......................................................................
soc/intel/broadwell/pch: Simplify PCI RMW operations
This reduces the differences between Lynx Point and Broadwell.
Change-Id: Ib53d73e3f89c538ba0f052a98c7aabe815a59472 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/pch/early_pch.c M src/soc/intel/broadwell/pch/pch.c M src/soc/intel/broadwell/pch/pcie.c 3 files changed, 33 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/46891/1
diff --git a/src/soc/intel/broadwell/pch/early_pch.c b/src/soc/intel/broadwell/pch/early_pch.c index 0c4dd7b..b0fd801 100644 --- a/src/soc/intel/broadwell/pch/early_pch.c +++ b/src/soc/intel/broadwell/pch/early_pch.c @@ -73,7 +73,7 @@ enable_smbus();
/* 8.14 Additional PCI Express Programming Steps, step #1 */ - pci_update_config32(_PCH_DEV(PCIE, 0), 0xf4, ~0x60, 0); - pci_update_config32(_PCH_DEV(PCIE, 0), 0xf4, ~0x80, 0x80); - pci_update_config32(_PCH_DEV(PCIE, 0), 0xe2, ~0x30, 0x30); + pci_and_config32(_PCH_DEV(PCIE, 0), 0xf4, ~0x60); + pci_or_config32(_PCH_DEV(PCIE, 0), 0xf4, 0x80); + pci_or_config32(_PCH_DEV(PCIE, 0), 0xe2, 0x30); } diff --git a/src/soc/intel/broadwell/pch/pch.c b/src/soc/intel/broadwell/pch/pch.c index c9debf2..4b8f3a7 100644 --- a/src/soc/intel/broadwell/pch/pch.c +++ b/src/soc/intel/broadwell/pch/pch.c @@ -167,8 +167,6 @@
static void broadwell_pch_enable_dev(struct device *dev) { - u16 reg16; - if (dev->path.type != DEVICE_PATH_PCI) return;
@@ -187,10 +185,8 @@ printk(BIOS_DEBUG, "%s: Disabling device\n", dev_path(dev));
/* Ensure memory, io, and bus master are all disabled */ - reg16 = pci_read_config16(dev, PCI_COMMAND); - reg16 &= ~(PCI_COMMAND_MASTER | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config16(dev, PCI_COMMAND, reg16); + pci_and_config16(dev, PCI_COMMAND, + ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO));
/* Disable this device if possible */ pch_disable_devfn(dev); diff --git a/src/soc/intel/broadwell/pch/pcie.c b/src/soc/intel/broadwell/pch/pcie.c index 7b7b504..f43fa78 100644 --- a/src/soc/intel/broadwell/pch/pcie.c +++ b/src/soc/intel/broadwell/pch/pcie.c @@ -120,7 +120,7 @@ rpc.pin_ownership = pci_read_config32(dev, 0x410); root_port_config_update_gbe_port();
- pci_update_config8(dev, 0xe2, ~(3 << 4), (3 << 4)); + pci_or_config8(dev, 0xe2, 3 << 4); const struct soc_intel_broadwell_pch_config *config = config_of(dev); rpc.coalesce = config->pcie_port_coalesce; } @@ -150,7 +150,7 @@ break; }
- pci_update_config32(dev, 0x418, 0, 0x02000430); + pci_write_config32(dev, 0x418, 0x02000430);
if (root_port_is_first(dev)) { /* @@ -212,23 +212,23 @@ if (!dev->enabled) { /* Configure shared resource clock gating. */ if (rp == 1 || rp == 5 || rp == 6) - pci_update_config8(dev, 0xe1, 0xc3, 0x3c); + pci_or_config8(dev, 0xe1, 0x3c);
- pci_update_config8(dev, 0xe2, ~(3 << 4), (3 << 4)); - pci_update_config32(dev, 0x420, ~(1 << 31), (1 << 31)); + pci_or_config8(dev, 0xe2, 3 << 4); + pci_or_config32(dev, 0x420, 1 << 31);
/* Per-Port CLKREQ# handling. */ if (gpio_is_native(18 + rp - 1)) - pci_update_config32(dev, 0x420, ~0, (3 << 29)); + pci_or_config32(dev, 0x420, 3 << 29);
/* Enable static clock gating. */ if (rp == 1 && !rpc.ports[1]->enabled && !rpc.ports[2]->enabled && !rpc.ports[3]->enabled) { - pci_update_config8(dev, 0xe2, ~1, 1); - pci_update_config8(dev, 0xe1, 0x7f, 0x80); + pci_or_config8(dev, 0xe2, 1); + pci_or_config8(dev, 0xe1, 1 << 7); } else if (rp == 5 || rp == 6) { - pci_update_config8(dev, 0xe2, ~1, 1); - pci_update_config8(dev, 0xe1, 0x7f, 0x80); + pci_or_config8(dev, 0xe2, 1); + pci_or_config8(dev, 0xe1, 1 << 7); } continue; } @@ -236,17 +236,17 @@ enabled_ports++;
/* Enable dynamic clock gating. */ - pci_update_config8(dev, 0xe1, 0xfc, 0x03); - pci_update_config8(dev, 0xe2, ~(1 << 6), (1 << 6)); + pci_or_config8(dev, 0xe1, 0x03); + pci_or_config8(dev, 0xe2, 1 << 6); pci_update_config8(dev, 0xe8, ~(3 << 2), (2 << 2));
/* Update PECR1 register. */ - pci_update_config8(dev, 0xe8, ~0, 3); + pci_or_config8(dev, 0xe8, 3); + if (is_broadwell) { - pci_update_config32(dev, 0x324, ~((1 << 5) | (1 << 14)), - ((1 << 5) | (1 << 14))); + pci_or_config32(dev, 0x324, (1 << 5) | (1 << 14)); } else { - pci_update_config32(dev, 0x324, ~(1 << 5), (1 << 5)); + pci_or_config32(dev, 0x324, 1 << 5); } /* Per-Port CLKREQ# handling. */ if (gpio_is_native(18 + rp - 1)) @@ -254,19 +254,18 @@ * In addition to D28Fx PCICFG 420h[30:29] = 11b, * set 420h[17] = 0b and 420[0] = 1b for L1 SubState. */ - pci_update_config32(dev, 0x420, ~0x20000, - (3 << 29) | 1); + pci_update_config32(dev, 0x420, ~(1 << 17), (3 << 29) | 1);
/* Configure shared resource clock gating. */ if (rp == 1 || rp == 5 || rp == 6) - pci_update_config8(dev, 0xe1, 0xc3, 0x3c); + pci_or_config8(dev, 0xe1, 0x3c);
/* CLKREQ# VR Idle Enable */ RCBA32_OR(0x2b1c, (1 << (16 + i))); }
if (!enabled_ports) - pci_update_config8(rpc.ports[0], 0xe1, ~(1 << 6), (1 << 6)); + pci_or_config8(rpc.ports[0], 0xe1, 1 << 6); }
static void root_port_commit_config(void) @@ -298,7 +297,7 @@ printk(BIOS_DEBUG, "%s: Disabling device\n", dev_path(dev));
/* 8.2 Configuration of PCI Express Root Ports */ - pci_update_config32(dev, 0x338, ~(1 << 26), 1 << 26); + pci_or_config32(dev, 0x338, 1 << 26);
do { reg32 = pci_read_config32(dev, 0x328); @@ -312,7 +311,7 @@ printk(BIOS_DEBUG, "%s: Timeout waiting for 328h\n", dev_path(dev));
- pci_update_config32(dev, 0x408, ~(1 << 27), 1 << 27); + pci_or_config32(dev, 0x408, 1 << 27);
/* Disable this device if possible */ pch_disable_devfn(dev); @@ -536,17 +535,15 @@ pci_update_config32(dev, 0x33c, ~0x00ffffff, 0x854d74);
/* Set Invalid Receive Range Check Enable in MPC register. */ - pci_update_config32(dev, 0xd8, ~0, (1 << 25)); + pci_or_config32(dev, 0xd8, 1 << 25);
- pci_update_config8(dev, 0xf5, 0x0f, 0); + pci_and_config8(dev, 0xf5, 0x0f);
/* Set AER Extended Cap ID to 01h and Next Cap Pointer to 200h. */ if (CONFIG(PCIEXP_AER)) - pci_update_config32(dev, 0x100, ~(1 << 29) & ~0xfffff, - (1 << 29) | 0x10001); + pci_update_config32(dev, 0x100, ~0xfffff, (1 << 29) | 0x10001); else - pci_update_config32(dev, 0x100, ~(1 << 29) & ~0xfffff, - (1 << 29)); + pci_update_config32(dev, 0x100, ~0xfffff, (1 << 29));
/* Set L1 Sub-State Cap ID to 1Eh and Next Cap Pointer to None. */ if (CONFIG(PCIEXP_L1_SUB_STATE)) @@ -554,10 +551,10 @@ else pci_update_config32(dev, 0x200, ~0xfffff, 0);
- pci_update_config32(dev, 0x320, ~(3 << 20) & ~(7 << 6), - (1 << 20) | (3 << 6)); + pci_update_config32(dev, 0x320, ~(3 << 20) & ~(7 << 6), (1 << 20) | (3 << 6)); + /* Enable Relaxed Order from Root Port. */ - pci_update_config32(dev, 0x320, ~(3 << 23), (3 << 23)); + pci_or_config32(dev, 0x320, 3 << 23);
if (rp == 1 || rp == 5 || rp == 6) pci_update_config8(dev, 0xf7, ~0xc, 0);
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46891
to look at the new patch set (#7).
Change subject: soc/intel/broadwell/pch: Simplify PCI RMW operations ......................................................................
soc/intel/broadwell/pch: Simplify PCI RMW operations
This reduces the differences between Lynx Point and Broadwell.
Change-Id: Ib53d73e3f89c538ba0f052a98c7aabe815a59472 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/pch/early_pch.c M src/soc/intel/broadwell/pch/pch.c M src/soc/intel/broadwell/pch/pcie.c 3 files changed, 33 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/46891/7
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46891 )
Change subject: soc/intel/broadwell/pch: Simplify PCI RMW operations ......................................................................
Patch Set 16: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46891 )
Change subject: soc/intel/broadwell/pch: Simplify PCI RMW operations ......................................................................
soc/intel/broadwell/pch: Simplify PCI RMW operations
This reduces the differences between Lynx Point and Broadwell.
Change-Id: Ib53d73e3f89c538ba0f052a98c7aabe815a59472 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/46891 Reviewed-by: Patrick Georgi pgeorgi@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/broadwell/pch/early_pch.c M src/soc/intel/broadwell/pch/pch.c M src/soc/intel/broadwell/pch/pcie.c 3 files changed, 33 insertions(+), 40 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved
diff --git a/src/soc/intel/broadwell/pch/early_pch.c b/src/soc/intel/broadwell/pch/early_pch.c index a18a5ec..5e7ce2a 100644 --- a/src/soc/intel/broadwell/pch/early_pch.c +++ b/src/soc/intel/broadwell/pch/early_pch.c @@ -74,7 +74,7 @@ enable_smbus();
/* 8.14 Additional PCI Express Programming Steps, step #1 */ - pci_update_config32(_PCH_DEV(PCIE, 0), 0xf4, ~0x60, 0); - pci_update_config32(_PCH_DEV(PCIE, 0), 0xf4, ~0x80, 0x80); - pci_update_config32(_PCH_DEV(PCIE, 0), 0xe2, ~0x30, 0x30); + pci_and_config32(_PCH_DEV(PCIE, 0), 0xf4, ~0x60); + pci_or_config32(_PCH_DEV(PCIE, 0), 0xf4, 0x80); + pci_or_config32(_PCH_DEV(PCIE, 0), 0xe2, 0x30); } diff --git a/src/soc/intel/broadwell/pch/pch.c b/src/soc/intel/broadwell/pch/pch.c index e0c5bb0..d4b88f1 100644 --- a/src/soc/intel/broadwell/pch/pch.c +++ b/src/soc/intel/broadwell/pch/pch.c @@ -168,8 +168,6 @@
static void broadwell_pch_enable_dev(struct device *dev) { - u16 reg16; - if (dev->path.type != DEVICE_PATH_PCI) return;
@@ -188,10 +186,8 @@ printk(BIOS_DEBUG, "%s: Disabling device\n", dev_path(dev));
/* Ensure memory, io, and bus master are all disabled */ - reg16 = pci_read_config16(dev, PCI_COMMAND); - reg16 &= ~(PCI_COMMAND_MASTER | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config16(dev, PCI_COMMAND, reg16); + pci_and_config16(dev, PCI_COMMAND, + ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO));
/* Disable this device if possible */ pch_disable_devfn(dev); diff --git a/src/soc/intel/broadwell/pch/pcie.c b/src/soc/intel/broadwell/pch/pcie.c index b098dc2..141fc14 100644 --- a/src/soc/intel/broadwell/pch/pcie.c +++ b/src/soc/intel/broadwell/pch/pcie.c @@ -120,7 +120,7 @@ rpc.pin_ownership = pci_read_config32(dev, 0x410); root_port_config_update_gbe_port();
- pci_update_config8(dev, 0xe2, ~(3 << 4), (3 << 4)); + pci_or_config8(dev, 0xe2, 3 << 4); const struct soc_intel_broadwell_pch_config *config = config_of(dev); rpc.coalesce = config->pcie_port_coalesce; } @@ -150,7 +150,7 @@ break; }
- pci_update_config32(dev, 0x418, 0, 0x02000430); + pci_write_config32(dev, 0x418, 0x02000430);
if (root_port_is_first(dev)) { /* @@ -212,23 +212,23 @@ if (!dev->enabled) { /* Configure shared resource clock gating. */ if (rp == 1 || rp == 5 || rp == 6) - pci_update_config8(dev, 0xe1, 0xc3, 0x3c); + pci_or_config8(dev, 0xe1, 0x3c);
- pci_update_config8(dev, 0xe2, ~(3 << 4), (3 << 4)); - pci_update_config32(dev, 0x420, ~(1 << 31), (1 << 31)); + pci_or_config8(dev, 0xe2, 3 << 4); + pci_or_config32(dev, 0x420, 1 << 31);
/* Per-Port CLKREQ# handling. */ if (gpio_is_native(18 + rp - 1)) - pci_update_config32(dev, 0x420, ~0, (3 << 29)); + pci_or_config32(dev, 0x420, 3 << 29);
/* Enable static clock gating. */ if (rp == 1 && !rpc.ports[1]->enabled && !rpc.ports[2]->enabled && !rpc.ports[3]->enabled) { - pci_update_config8(dev, 0xe2, ~1, 1); - pci_update_config8(dev, 0xe1, 0x7f, 0x80); + pci_or_config8(dev, 0xe2, 1); + pci_or_config8(dev, 0xe1, 1 << 7); } else if (rp == 5 || rp == 6) { - pci_update_config8(dev, 0xe2, ~1, 1); - pci_update_config8(dev, 0xe1, 0x7f, 0x80); + pci_or_config8(dev, 0xe2, 1); + pci_or_config8(dev, 0xe1, 1 << 7); } continue; } @@ -236,17 +236,17 @@ enabled_ports++;
/* Enable dynamic clock gating. */ - pci_update_config8(dev, 0xe1, 0xfc, 0x03); - pci_update_config8(dev, 0xe2, ~(1 << 6), (1 << 6)); + pci_or_config8(dev, 0xe1, 0x03); + pci_or_config8(dev, 0xe2, 1 << 6); pci_update_config8(dev, 0xe8, ~(3 << 2), (2 << 2));
/* Update PECR1 register. */ - pci_update_config8(dev, 0xe8, ~0, 3); + pci_or_config8(dev, 0xe8, 3); + if (is_broadwell) { - pci_update_config32(dev, 0x324, ~((1 << 5) | (1 << 14)), - ((1 << 5) | (1 << 14))); + pci_or_config32(dev, 0x324, (1 << 5) | (1 << 14)); } else { - pci_update_config32(dev, 0x324, ~(1 << 5), (1 << 5)); + pci_or_config32(dev, 0x324, 1 << 5); } /* Per-Port CLKREQ# handling. */ if (gpio_is_native(18 + rp - 1)) @@ -254,19 +254,18 @@ * In addition to D28Fx PCICFG 420h[30:29] = 11b, * set 420h[17] = 0b and 420[0] = 1b for L1 SubState. */ - pci_update_config32(dev, 0x420, ~0x20000, - (3 << 29) | 1); + pci_update_config32(dev, 0x420, ~(1 << 17), (3 << 29) | 1);
/* Configure shared resource clock gating. */ if (rp == 1 || rp == 5 || rp == 6) - pci_update_config8(dev, 0xe1, 0xc3, 0x3c); + pci_or_config8(dev, 0xe1, 0x3c);
/* CLKREQ# VR Idle Enable */ RCBA32_OR(0x2b1c, (1 << (16 + i))); }
if (!enabled_ports) - pci_update_config8(rpc.ports[0], 0xe1, ~(1 << 6), (1 << 6)); + pci_or_config8(rpc.ports[0], 0xe1, 1 << 6); }
static void root_port_commit_config(void) @@ -298,7 +297,7 @@ printk(BIOS_DEBUG, "%s: Disabling device\n", dev_path(dev));
/* 8.2 Configuration of PCI Express Root Ports */ - pci_update_config32(dev, 0x338, ~(1 << 26), 1 << 26); + pci_or_config32(dev, 0x338, 1 << 26);
do { reg32 = pci_read_config32(dev, 0x328); @@ -312,7 +311,7 @@ printk(BIOS_DEBUG, "%s: Timeout waiting for 328h\n", dev_path(dev));
- pci_update_config32(dev, 0x408, ~(1 << 27), 1 << 27); + pci_or_config32(dev, 0x408, 1 << 27);
/* Disable this device if possible */ pch_disable_devfn(dev); @@ -536,17 +535,15 @@ pci_update_config32(dev, 0x33c, ~0x00ffffff, 0x854d74);
/* Set Invalid Receive Range Check Enable in MPC register. */ - pci_update_config32(dev, 0xd8, ~0, (1 << 25)); + pci_or_config32(dev, 0xd8, 1 << 25);
- pci_update_config8(dev, 0xf5, 0x0f, 0); + pci_and_config8(dev, 0xf5, 0x0f);
/* Set AER Extended Cap ID to 01h and Next Cap Pointer to 200h. */ if (CONFIG(PCIEXP_AER)) - pci_update_config32(dev, 0x100, ~(1 << 29) & ~0xfffff, - (1 << 29) | 0x10001); + pci_update_config32(dev, 0x100, ~0xfffff, (1 << 29) | 0x10001); else - pci_update_config32(dev, 0x100, ~(1 << 29) & ~0xfffff, - (1 << 29)); + pci_update_config32(dev, 0x100, ~0xfffff, (1 << 29));
/* Set L1 Sub-State Cap ID to 1Eh and Next Cap Pointer to None. */ if (CONFIG(PCIEXP_L1_SUB_STATE)) @@ -554,10 +551,10 @@ else pci_update_config32(dev, 0x200, ~0xfffff, 0);
- pci_update_config32(dev, 0x320, ~(3 << 20) & ~(7 << 6), - (1 << 20) | (3 << 6)); + pci_update_config32(dev, 0x320, ~(3 << 20) & ~(7 << 6), (1 << 20) | (3 << 6)); + /* Enable Relaxed Order from Root Port. */ - pci_update_config32(dev, 0x320, ~(3 << 23), (3 << 23)); + pci_or_config32(dev, 0x320, 3 << 23);
if (rp == 1 || rp == 5 || rp == 6) pci_update_config8(dev, 0xf7, ~0xc, 0);
-
Angel Pons (Code Review) -
Patrick Georgi (Code Review)