Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41943 )
Change subject: sb/intel/lynxpoint: Clean up code ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41943/3/src/southbridge/intel/lynxp... File src/southbridge/intel/lynxpoint/elog.c:
https://review.coreboot.org/c/coreboot/+/41943/3/src/southbridge/intel/lynxp... PS3, Line 143: /*
I left this one like this on purpose, because there's several wake sources (the events listed below)
Ack
https://review.coreboot.org/c/coreboot/+/41943/3/src/southbridge/intel/lynxp... File src/southbridge/intel/lynxpoint/pch.h:
https://review.coreboot.org/c/coreboot/+/41943/3/src/southbridge/intel/lynxp... PS3, Line 403: K 0x800 : #define SIO_REG_PPR_CLOCK_EN (1 << 0) : #define SIO_REG_PPR_RST 0x804 : #define SIO_REG_PPR_RST_ASSERT 0x3 : #define SIO_REG_PPR_GEN 0x808 : #define SIO_REG_PPR_GEN_LTR_MODE_MASK (1 << 2) : #define SIO_REG_PPR_GEN_VOLTAGE_MASK (1 << 3) : #define SIO_REG_PPR_GEN_VOLTAGE(x) ((x & 1) << 3) : #define SIO_REG_AUTO_LTR 0x814 : : #define SIO_REG_SDIO_PPR_GEN 0x1008 : #define SIO_REG_SDIO_PPR_SW_LTR 0x1010 : #define SIO_REG_SDIO_PPR_CMD12 0x3c : #define SIO_REG_SDIO_PPR_CMD12_B30 (1 << 30) :
I don't see any problems. […]
that's what I meant indeed
https://review.coreboot.org/c/coreboot/+/41943/3/src/southbridge/intel/lynxp... PS3, Line 582: 3400 /* 32bit */ : #define HPTC 0x3404 /* 32bit */ : #define GCS 0x3410 /* 32bit */ : #define BUC 0x3414 /* 32bit */ : #define PCH_DISABLE_GBE (1 << 5) : #define FD 0x3418 /* 32bit */ : #define DISPBDF 0x3424 /* 16bit */ : #define FD2 0x3428 /* 32bit */ : #define CG 0x341c /* 32bit */
This is aligned. PCH_DISABLE_GBE is a bit inside a register, so it should stick out.
Ack