Hi, I just found this file here : https://github.com/IntelFsp/FSP/blob/Skylake/SkylakeFspBinPkg/Include/GpioConfig.h It defines the reset values for GPIO in this enum : typedef enum { GpioResetDefault = 0x0, ///< Leave value of pad reset unmodified GpioResetPwrGood = 0x1, ///< GPP: RSMRST; GPD: DSW_PWROK; (PadRstCfg = 00b = "Powergood") GpioResetDeep = 0x3, ///< Deep GPIO Reset (PadRstCfg = 01b = "Deep GPIO Reset") GpioResetNormal = 0x5, ///< GPIO Reset (PadRstCfg = 10b = "GPIO Reset" ) GpioResetResume = 0x7 ///< GPP: Reserved; GPD: RSMRST; (PadRstCfg = 11b = "Resume Reset" ) } GPIO_RESET_CONFIG;
We can see it specifically says that 00b is "GPP: RSMRST; GPD: DSW_PWROK" and 11b is "GPP: Reserved; GPD: RSMRST". So I'm leaning towards the datasheet not having a typo, but rather they just have different values for RSMRST depending on whether it's GPP or GPD, so I vote for changing the define in coreboot so it stops being confusing for those who want to use RSMRST. I'll send a patch shortly to do it as I suggested in my previous email, unless someone else has a better idea. Thanks, Youness. On Thu, May 18, 2017 at 2:56 PM, Youness Alaoui <[email protected]> wrote: > Hi, > > I'm working on a skylake port and I've noticed something with the PADRSTCFG > field of the GPIO Pad configuration. If you look at the 100-series datasheet > volume 2 > (https://www-ssl.intel.com/content/www/us/en/chipsets/100-series-chipset-datasheet-vol-2.html). > The Pad configuration DW0 for GPP_A, GPP_B, GPP_C, etc.. (page 1179, 1249) > up to GPP_I (page 1319) has bits 31:30 (PADRSTCFG) defined like this : > 00 = RSMRST# > 01 = Host deep reset. > 10 = PLTRST# > 11 = Reserved > > But the Pad configuration DW0 for GPD pads (page 1296) has it defined like > this : > 00 = DSW_PWROK > 01 = Host deep reset. > 10 = PLTRST# > 11 = RSMRST# > > The problem here is that while the DEEP and PLTRST values are the same, the > RSMRST value is 0 for GPP gpio pads, and 3 for GPD gpio pads. In the skylake > and apollolake gpio_defs.h file, it is defined like this (with different > naming for apollolake but same values) : > #define PADRSTCFG_DSW_PWROK 0 > #define PADRSTCFG_DEEP 1 > #define PADRSTCFG_PLTRST 2 > #define PADRSTCFG_RSMRST 3 > > These defines are only valid for GPD pads, if someone was to use a RSMRST > reset config on a GPP pad, they would be setting the pad to a 'reserved' > configuration, which could have unexpected behavior. > There are two possibilities here, either the datasheet is wrong (and PWROK > should be 3 so the RSMRST value is consistent for both GPP and GPD), or the > datasheet is correct, and in both cases, the value in coreboot is wrong. > > If someone from Intel is reading this list, or if someone here knows someone > inside Intel who could confirm whether or not the datasheet has an error, > that would be great. > In the meantime, I would maybe suggest to change the defines to : > #define PADRSTCFG_GPD_RSMRST 3 > #define PADRSTCFG_GPP_RSMRST 0 > > This would affect both skylake and apollolake, and note that the only > skylake+ boards that use RSMRST seem to be the kblrvp in the rvp7 variant > which sets it for GPP_A12 and GPP_E3 (which means those two gpios have the > wrong setting due to the wrong define). > > What do you think ? > > Thanks, > Youness. -- coreboot mailing list: [email protected] https://mail.coreboot.org/mailman/listinfo/coreboot
