Change in coreboot[master]: soc/intel/broadwell/lpc.c: Drop reg-script usage for PCH misc init
Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46351 )
Change subject: soc/intel/broadwell/lpc.c: Drop reg-script usage for PCH misc init ......................................................................
soc/intel/broadwell/lpc.c: Drop reg-script usage for PCH misc init
Change-Id: I4846f9303367452bbb1d21c2d7f4a1fb9f2efe5d Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/lpc.c 1 file changed, 53 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/46351/1
diff --git a/src/soc/intel/broadwell/lpc.c b/src/soc/intel/broadwell/lpc.c index 495f134..7adb926 100644 --- a/src/soc/intel/broadwell/lpc.c +++ b/src/soc/intel/broadwell/lpc.c @@ -172,33 +172,65 @@ enable_alt_smi(config->alt_gp_smi_en); }
-static const struct reg_script pch_misc_init_script[] = { - /* Setup SLP signal assertion, SLP_S4=4s, SLP_S3=50ms */ - REG_PCI_RMW16(GEN_PMCON_3, ~((3 << 4)|(1 << 10)), - (1 << 3)|(1 << 11)|(1 << 12)), +static void pch_misc_init(struct device *dev) +{ + u8 reg8; + u16 reg16; + u32 reg32; + + reg16 = pci_read_config16(dev, GEN_PMCON_3); + + reg16 &= ~(3 << 4); /* SLP_S4# Assertion Stretch 4s */ + reg16 |= (1 << 3); /* SLP_S4# Assertion Stretch Enable */ + + reg16 &= ~(1 << 10); + reg16 |= (1 << 11); /* SLP_S3# Min Assertion Width 50ms */ + + reg16 |= (1 << 12); /* Disable SLP stretch after SUS well */ + + pci_write_config16(dev, GEN_PMCON_3, reg16); + /* Prepare sleep mode */ - REG_IO_RMW32(ACPI_BASE_ADDRESS + PM1_CNT, ~SLP_TYP, SCI_EN), - /* Setup NMI on errors, disable SERR */ - REG_IO_RMW8(0x61, ~0xf0, (1 << 2)), + reg32 = inl(ACPI_BASE_ADDRESS + PM1_CNT); + reg32 &= ~SLP_TYP; + reg32 |= SCI_EN; + outl(reg32, ACPI_BASE_ADDRESS + PM1_CNT); + + /* Set up NMI on errors */ + reg8 = inb(0x61); + reg8 &= ~0xf0; /* Higher nibble must be 0 */ + reg8 |= (1 << 2); /* PCI SERR# disable for now */ + outb(reg8, 0x61); + /* Disable NMI sources */ - REG_IO_OR8(0x70, (1 << 7)), + reg8 = inb(0x70); + reg8 |= (1 << 7); /* Can't mask NMI from PCI-E and NMI_NOW */ + outb(reg8, 0x70); + /* Indicate DRAM init done for MRC */ - REG_PCI_OR8(GEN_PMCON_2, (1 << 7)), + pci_or_config8(dev, GEN_PMCON_2, 1 << 7); + /* Enable BIOS updates outside of SMM */ - REG_PCI_RMW8(0xdc, ~(1 << 5), 0), + pci_and_config8(dev, BIOS_CNTL, ~(1 << 5)); + /* Clear status bits to prevent unexpected wake */ - REG_MMIO_OR32(RCBA_BASE_ADDRESS + 0x3310, 0x0000002f), - REG_MMIO_RMW32(RCBA_BASE_ADDRESS + 0x3f02, ~0x0000000f, 0), + RCBA32_OR(0x3310, 0x2f); + + RCBA32_AND_OR(0x3f02, ~0xf, 0); + /* Enable PCIe Releaxed Order */ - REG_MMIO_OR32(RCBA_BASE_ADDRESS + 0x2314, (1 << 31) | (1 << 7)), - REG_MMIO_OR32(RCBA_BASE_ADDRESS + 0x1114, (1 << 15) | (1 << 14)), + RCBA32_OR(0x2314, (1 << 31) | (1 << 7)), + RCBA32_OR(0x1114, (1 << 15) | (1 << 14)), + /* Setup SERIRQ, enable continuous mode */ - REG_PCI_OR8(SERIRQ_CNTL, (1 << 7) | (1 << 6)), -#if !CONFIG(SERIRQ_CONTINUOUS_MODE) - REG_PCI_RMW8(SERIRQ_CNTL, ~(1 << 6), 0), -#endif - REG_SCRIPT_END -}; + reg8 = pci_read_config8(dev, SERIRQ_CNTL); + reg8 |= 1 << 7; + + if (CONFIG(SERIRQ_CONTINUOUS_MODE)) + reg8 |= 1 << 6; + + pci_write_config8(dev, SERIRQ_CNTL, reg8); +}
/* Magic register settings for power management */ static void pch_pm_init_magic(struct device *dev) @@ -427,7 +459,7 @@ /* Legacy initialization */ isa_dma_init(); sb_rtc_init(); - reg_script_run_on_dev(dev, pch_misc_init_script); + pch_misc_init(dev);
/* Interrupt configuration */ pch_enable_ioapic(dev);
Hello Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46351
to look at the new patch set (#2).
Change subject: soc/intel/broadwell/lpc.c: Drop reg-script usage for PCH misc init ......................................................................
soc/intel/broadwell/lpc.c: Drop reg-script usage for PCH misc init
Change-Id: I4846f9303367452bbb1d21c2d7f4a1fb9f2efe5d Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/lpc.c 1 file changed, 53 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/46351/2
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46351 )
Change subject: soc/intel/broadwell/lpc.c: Drop reg-script usage for PCH misc init ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46351/4/src/soc/intel/broadwell/lpc... File src/soc/intel/broadwell/lpc.c:
https://review.coreboot.org/c/coreboot/+/46351/4/src/soc/intel/broadwell/lpc... PS4, Line 218: RCBA32_AND_OR(0x3f02, ~0xf, 0); This is not a good idea. here GCC will complain for an overflow: - switch to 64 bits - if some one add RCBA16_AND_OR or/and RCBA8_AND_OR , as ~0xf is neither 16 nor 8bits
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46351 )
Change subject: soc/intel/broadwell/lpc.c: Drop reg-script usage for PCH misc init ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46351/4/src/soc/intel/broadwell/lpc... File src/soc/intel/broadwell/lpc.c:
https://review.coreboot.org/c/coreboot/+/46351/4/src/soc/intel/broadwell/lpc... PS4, Line 218: RCBA32_AND_OR(0x3f02, ~0xf, 0);
This is not a good idea. […]
Nak
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46351 )
Change subject: soc/intel/broadwell/lpc.c: Drop reg-script usage for PCH misc init ......................................................................
Patch Set 4: Code-Review+2
(3 comments)
I suppose this is for alignment, so the suggested improvements should happen later.
https://review.coreboot.org/c/coreboot/+/46351/4/src/soc/intel/broadwell/lpc... File src/soc/intel/broadwell/lpc.c:
https://review.coreboot.org/c/coreboot/+/46351/4/src/soc/intel/broadwell/lpc... PS4, Line 185: 1 I automatically stumbled here, would prefer `3 << 10`.
https://review.coreboot.org/c/coreboot/+/46351/4/src/soc/intel/broadwell/lpc... PS4, Line 186: reg16 |= (1 << 11); /* SLP_S3# Min Assertion Width 50ms */ and `2 << 10`
https://review.coreboot.org/c/coreboot/+/46351/4/src/soc/intel/broadwell/lpc... PS4, Line 229: reg8 |= 1 << 6; We trust now that it's 0 by default. I checked the ds, seems fine.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46351 )
Change subject: soc/intel/broadwell/lpc.c: Drop reg-script usage for PCH misc init ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46351/4/src/soc/intel/broadwell/lpc... File src/soc/intel/broadwell/lpc.c:
https://review.coreboot.org/c/coreboot/+/46351/4/src/soc/intel/broadwell/lpc... PS4, Line 185: 1
I automatically stumbled here, would prefer `3 << 10`.
Sounds good.
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46351 )
Change subject: soc/intel/broadwell/lpc.c: Drop reg-script usage for PCH misc init ......................................................................
soc/intel/broadwell/lpc.c: Drop reg-script usage for PCH misc init
Change-Id: I4846f9303367452bbb1d21c2d7f4a1fb9f2efe5d Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/46351 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/soc/intel/broadwell/lpc.c 1 file changed, 53 insertions(+), 22 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/soc/intel/broadwell/lpc.c b/src/soc/intel/broadwell/lpc.c index 495f134..ccd372c 100644 --- a/src/soc/intel/broadwell/lpc.c +++ b/src/soc/intel/broadwell/lpc.c @@ -14,7 +14,6 @@ #include <acpi/acpi_gnvs.h> #include <cpu/x86/smm.h> #include <cbmem.h> -#include <reg_script.h> #include <string.h> #include <soc/gpio.h> #include <soc/iobp.h> @@ -172,33 +171,65 @@ enable_alt_smi(config->alt_gp_smi_en); }
-static const struct reg_script pch_misc_init_script[] = { - /* Setup SLP signal assertion, SLP_S4=4s, SLP_S3=50ms */ - REG_PCI_RMW16(GEN_PMCON_3, ~((3 << 4)|(1 << 10)), - (1 << 3)|(1 << 11)|(1 << 12)), +static void pch_misc_init(struct device *dev) +{ + u8 reg8; + u16 reg16; + u32 reg32; + + reg16 = pci_read_config16(dev, GEN_PMCON_3); + + reg16 &= ~(3 << 4); /* SLP_S4# Assertion Stretch 4s */ + reg16 |= (1 << 3); /* SLP_S4# Assertion Stretch Enable */ + + reg16 &= ~(1 << 10); + reg16 |= (1 << 11); /* SLP_S3# Min Assertion Width 50ms */ + + reg16 |= (1 << 12); /* Disable SLP stretch after SUS well */ + + pci_write_config16(dev, GEN_PMCON_3, reg16); + /* Prepare sleep mode */ - REG_IO_RMW32(ACPI_BASE_ADDRESS + PM1_CNT, ~SLP_TYP, SCI_EN), - /* Setup NMI on errors, disable SERR */ - REG_IO_RMW8(0x61, ~0xf0, (1 << 2)), + reg32 = inl(ACPI_BASE_ADDRESS + PM1_CNT); + reg32 &= ~SLP_TYP; + reg32 |= SCI_EN; + outl(reg32, ACPI_BASE_ADDRESS + PM1_CNT); + + /* Set up NMI on errors */ + reg8 = inb(0x61); + reg8 &= ~0xf0; /* Higher nibble must be 0 */ + reg8 |= (1 << 2); /* PCI SERR# disable for now */ + outb(reg8, 0x61); + /* Disable NMI sources */ - REG_IO_OR8(0x70, (1 << 7)), + reg8 = inb(0x70); + reg8 |= (1 << 7); /* Can't mask NMI from PCI-E and NMI_NOW */ + outb(reg8, 0x70); + /* Indicate DRAM init done for MRC */ - REG_PCI_OR8(GEN_PMCON_2, (1 << 7)), + pci_or_config8(dev, GEN_PMCON_2, 1 << 7); + /* Enable BIOS updates outside of SMM */ - REG_PCI_RMW8(0xdc, ~(1 << 5), 0), + pci_and_config8(dev, BIOS_CNTL, ~(1 << 5)); + /* Clear status bits to prevent unexpected wake */ - REG_MMIO_OR32(RCBA_BASE_ADDRESS + 0x3310, 0x0000002f), - REG_MMIO_RMW32(RCBA_BASE_ADDRESS + 0x3f02, ~0x0000000f, 0), + RCBA32_OR(0x3310, 0x2f); + + RCBA32_AND_OR(0x3f02, ~0xf, 0); + /* Enable PCIe Releaxed Order */ - REG_MMIO_OR32(RCBA_BASE_ADDRESS + 0x2314, (1 << 31) | (1 << 7)), - REG_MMIO_OR32(RCBA_BASE_ADDRESS + 0x1114, (1 << 15) | (1 << 14)), + RCBA32_OR(0x2314, (1 << 31) | (1 << 7)), + RCBA32_OR(0x1114, (1 << 15) | (1 << 14)), + /* Setup SERIRQ, enable continuous mode */ - REG_PCI_OR8(SERIRQ_CNTL, (1 << 7) | (1 << 6)), -#if !CONFIG(SERIRQ_CONTINUOUS_MODE) - REG_PCI_RMW8(SERIRQ_CNTL, ~(1 << 6), 0), -#endif - REG_SCRIPT_END -}; + reg8 = pci_read_config8(dev, SERIRQ_CNTL); + reg8 |= 1 << 7; + + if (CONFIG(SERIRQ_CONTINUOUS_MODE)) + reg8 |= 1 << 6; + + pci_write_config8(dev, SERIRQ_CNTL, reg8); +}
/* Magic register settings for power management */ static void pch_pm_init_magic(struct device *dev) @@ -427,7 +458,7 @@ /* Legacy initialization */ isa_dma_init(); sb_rtc_init(); - reg_script_run_on_dev(dev, pch_misc_init_script); + pch_misc_init(dev);
/* Interrupt configuration */ pch_enable_ioapic(dev);
-
Angel Pons (Code Review) -
HAOUAS Elyes (Code Review)
-
Nico Huber (Code Review)