Keith Hui has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41637 )
Change subject: sb/intel/i82371eb: Clean up PM register #defines ......................................................................
sb/intel/i82371eb: Clean up PM register #defines
Remove EIO define. It is unused and means something else, elsewhere in the tree.
Move PMIOSE bit definition next to PMREGMISC, where it actually belongs.
Clarify in comments which PM register defines are in PCI config space are which are in I/O space.
Change-Id: Ic7f2267d013403c0a519c2ee1786bd3c7f5a9708 Signed-off-by: Keith Hui buurin@gmail.com --- M src/southbridge/intel/i82371eb/i82371eb.h 1 file changed, 3 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/41637/1
diff --git a/src/southbridge/intel/i82371eb/i82371eb.h b/src/southbridge/intel/i82371eb/i82371eb.h index a08be0d..449161e 100644 --- a/src/southbridge/intel/i82371eb/i82371eb.h +++ b/src/southbridge/intel/i82371eb/i82371eb.h @@ -43,7 +43,7 @@ #define SMBUS_IO_BASE 0x0f00 #define SMBHSTCFG 0xd2 /* SMBus host configuration */
-/* Power management (ACPI) */ +/* Power management (ACPI) I/O ports, offset from PM_IO_BASE below */ #define PMSTS 0x00 /* Power Management Status */ #define PMEN 0x02 /* Power Management Resume Enable */ #define PWRBTN_EN (1<<8) @@ -90,10 +90,12 @@ #define GPO2 0x36 #define GPO3 0x37
+/* Power management (ACPI) PCI registers */ #define PMBA 0x40 /* Power management base address */ #define DEFAULT_PMBASE 0xe400 #define PM_IO_BASE DEFAULT_PMBASE #define PMREGMISC 0x80 /* Miscellaneous power management */ +#define PMIOSE (1 << 0) /* PM I/O Space Enable */
/* Bit definitions */ #define EXT_BIOS_ENABLE_1MB (1 << 9) /* 1-Meg Extended BIOS Enable */ @@ -109,7 +111,5 @@ #define SSDE0 (1 << 2) /* Secondary Drive 0 UDMA/33 */ #define SSDE1 (1 << 3) /* Secondary Drive 1 UDMA/33 */ #define ISA (1 << 0) /* Select ISA */ -#define EIO (0 << 0) /* Select EIO */ -#define PMIOSE (1 << 0) /* PM I/O Space Enable */
#endif /* SOUTHBRIDGE_INTEL_I82371EB_I82371EB_H */
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41637 )
Change subject: sb/intel/i82371eb: Clean up PM register #defines ......................................................................
Patch Set 1:
(2 comments)
Seems to need a rebuild
https://review.coreboot.org/c/coreboot/+/41637/1/src/southbridge/intel/i8237... File src/southbridge/intel/i82371eb/i82371eb.h:
https://review.coreboot.org/c/coreboot/+/41637/1/src/southbridge/intel/i8237... PS1, Line 57: //#define SUS_TYP_S2>---(2<<10) Hm?
https://review.coreboot.org/c/coreboot/+/41637/1/src/southbridge/intel/i8237... PS1, Line 100: /* Bit definitions */ Which register do these definitions apply to?
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41637
to look at the new patch set (#2).
Change subject: sb/intel/i82371eb: Clean up PM register #defines ......................................................................
sb/intel/i82371eb: Clean up PM register #defines
Remove EIO define. It is unused and means something else, elsewhere in the tree.
Move PMIOSE bit definition next to PMREGMISC, where it actually belongs.
Correct a number of bit defines with glaring errors.
Clarify in comments which PM register defines are in PCI config space are which are in I/O space.
Change-Id: Ic7f2267d013403c0a519c2ee1786bd3c7f5a9708 Signed-off-by: Keith Hui buurin@gmail.com --- M src/southbridge/intel/i82371eb/i82371eb.h 1 file changed, 7 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/41637/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41637 )
Change subject: sb/intel/i82371eb: Clean up PM register #defines ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41637/1/src/southbridge/intel/i8237... File src/southbridge/intel/i82371eb/i82371eb.h:
https://review.coreboot.org/c/coreboot/+/41637/1/src/southbridge/intel/i8237... PS1, Line 57: //#define SUS_TYP_S2>---(2<<10)
Hm?
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41637 )
Change subject: sb/intel/i82371eb: Clean up PM register #defines ......................................................................
Patch Set 2: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41637 )
Change subject: sb/intel/i82371eb: Clean up PM register #defines ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/41637/1/src/southbridge/intel/i8237... File src/southbridge/intel/i82371eb/i82371eb.h:
https://review.coreboot.org/c/coreboot/+/41637/1/src/southbridge/intel/i8237... PS1, Line 100: /* Bit definitions */
Which register do these definitions apply to?
Ack
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41637 )
Change subject: sb/intel/i82371eb: Clean up PM register #defines ......................................................................
sb/intel/i82371eb: Clean up PM register #defines
Remove EIO define. It is unused and means something else, elsewhere in the tree.
Move PMIOSE bit definition next to PMREGMISC, where it actually belongs.
Correct a number of bit defines with glaring errors.
Clarify in comments which PM register defines are in PCI config space are which are in I/O space.
Change-Id: Ic7f2267d013403c0a519c2ee1786bd3c7f5a9708 Signed-off-by: Keith Hui buurin@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41637 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/i82371eb/i82371eb.h 1 file changed, 7 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/southbridge/intel/i82371eb/i82371eb.h b/src/southbridge/intel/i82371eb/i82371eb.h index e20cb5f..405bb74 100644 --- a/src/southbridge/intel/i82371eb/i82371eb.h +++ b/src/southbridge/intel/i82371eb/i82371eb.h @@ -39,7 +39,7 @@ #define SMBUS_IO_BASE 0x0f00 #define SMBHSTCFG 0xd2 /* SMBus host configuration */
-/* Power management (ACPI) */ +/* Power management (ACPI) I/O ports, offset from PM_IO_BASE below */ #define PMSTS 0x00 /* Power Management Status */ #define PMEN 0x02 /* Power Management Resume Enable */ #define PWRBTN_EN (1<<8) @@ -49,8 +49,7 @@ #define SUS_TYP_MSK (7<<10) #define SUS_TYP_S0 (5<<10) #define SUS_TYP_S1 (4<<10) -#define SUS_TYP_S2 (3<<10) -//#define SUS_TYP_S2>---(2<<10) +#define SUS_TYP_S2 (3<<10) /* S2 may also map as 2<<10 */ #define SUS_TYP_S3 (1<<10) #define SUS_TYP_S5 (0<<10) #define SCI_EN (1<<0) @@ -64,11 +63,11 @@ #define EXTSMI_STS (1<<10) #define GSTBY_STS (1<<8) #define GP_STS (1<<7) -#define BM1_STS (1<<6) +#define PM1_STS (1<<6) #define APM_STS (1<<5) #define DEV_STS (1<<4) -#define BIOS_EN (1<<1) /* GBL_RLS write triggers SMI */ -#define LEGACY_USB_EN (1<<0) /* Keyboard controller access triggers SMI */ +#define LEGACY_USB_STS (1<<1) +#define BIOS_STS (1<<0) #define DEVSTS 0x1c /* Device Status */ #define GLBEN 0x20 /* Global Enable */ #define EXTSMI_EN (1<<10) /* EXTSMI# signal triggers SMI */ @@ -86,10 +85,12 @@ #define GPO2 0x36 #define GPO3 0x37
+/* Power management (ACPI) PCI registers */ #define PMBA 0x40 /* Power management base address */ #define DEFAULT_PMBASE 0xe400 #define PM_IO_BASE DEFAULT_PMBASE #define PMREGMISC 0x80 /* Miscellaneous power management */ +#define PMIOSE (1 << 0) /* PM I/O Space Enable */
/* Bit definitions */ #define EXT_BIOS_ENABLE_1MB (1 << 9) /* 1-Meg Extended BIOS Enable */ @@ -105,7 +106,5 @@ #define SSDE0 (1 << 2) /* Secondary Drive 0 UDMA/33 */ #define SSDE1 (1 << 3) /* Secondary Drive 1 UDMA/33 */ #define ISA (1 << 0) /* Select ISA */ -#define EIO (0 << 0) /* Select EIO */ -#define PMIOSE (1 << 0) /* PM I/O Space Enable */
#endif /* SOUTHBRIDGE_INTEL_I82371EB_I82371EB_H */