Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47191 )
Change subject: mb/purism/librem_mini: Set unused GPIO pads to PAD_NC ......................................................................
Patch Set 5:
(12 comments)
https://review.coreboot.org/c/coreboot/+/47191/4/src/mainboard/purism/librem... File src/mainboard/purism/librem_cnl/variants/librem_mini/gpio.c:
https://review.coreboot.org/c/coreboot/+/47191/4/src/mainboard/purism/librem... PS4, Line 83: : /* GPP_B0 - NC/TP */ : PAD_NC(GPP_B0, UP_20K), : : /* GPP_B1 - NC/TP */ : PAD_NC(GPP_B1, UP_20K),
These can be left as NF1 just fine, actually.
technically doesn't really matter, does it?
https://review.coreboot.org/c/coreboot/+/47191/4/src/mainboard/purism/librem... PS4, Line 97: NONE
Needs a pull.
ack
https://review.coreboot.org/c/coreboot/+/47191/4/src/mainboard/purism/librem... PS4, Line 99: /* GPP_B5 - NC */ : PAD_NC(GPP_B5, NONE), : : /* GPP_B6 - NC */ : PAD_NC(GPP_B6, NONE), : : /* GPP_B7 - NC */ : PAD_NC(GPP_B7, NONE), : : /* GPP_B8 - NC */ : PAD_NC(GPP_B8, NONE), : : /* GPP_B9 - NC */ : PAD_NC(GPP_B9, NONE), : : /* GPP_B10 - NC */ : PAD_NC(GPP_B10, NONE),
CLKREQ# seems to be used?
see CB:47220
https://review.coreboot.org/c/coreboot/+/47191/4/src/mainboard/purism/librem... PS4, Line 117: /* GPP_B11 - GPIO */ : PAD_CFG_GPO(GPP_B11, 1, PLTRST),
Not used
yup, nc, none - looks like I missed that one
https://review.coreboot.org/c/coreboot/+/47191/4/src/mainboard/purism/librem... PS4, Line 126: /* GPP_B14 - GPIO */ : PAD_CFG_GPO(GPP_B14, 1, PLTRST),
Should be native
see CB:47220 as commented in PS3 ;)
https://review.coreboot.org/c/coreboot/+/47191/4/src/mainboard/purism/librem... PS4, Line 132: /* GPP_B16 - GSPI0_CLK */ : PAD_CFG_NF(GPP_B16, NONE, PLTRST, NF1),
This one is NC too.
ack. nc, none
https://review.coreboot.org/c/coreboot/+/47191/4/src/mainboard/purism/librem... PS4, Line 205: /* GPP_D7 - NC */ : PAD_NC(GPP_D7, UP_20K),
This is PCH_NVME_RST and needs to be an output
nope; R526 is unstuff/nonpop
https://review.coreboot.org/c/coreboot/+/47191/4/src/mainboard/purism/librem... PS4, Line 395: /* GPP_H21 - GPIO */ : PAD_CFG_GPO(GPP_H21, 0, DEEP),
NC, it's a strap
ack! nc, none
https://review.coreboot.org/c/coreboot/+/47191/4/src/mainboard/purism/librem... PS4, Line 412: /* GPD2 - NC */ : PAD_NC(GPD2, NONE),
I don't think so
but I do - R332 is not populated
https://review.coreboot.org/c/coreboot/+/47191/4/src/mainboard/purism/librem... PS4, Line 462: /* GPP_C6 - NC */ : PAD_NC(GPP_C6, NONE), : : /* GPP_C7 - NC */ : PAD_NC(GPP_C7, NONE),
These two go to the EC
look closer ;) R126 and R130 are not populated
https://review.coreboot.org/c/coreboot/+/47191/4/src/mainboard/purism/librem... PS4, Line 510: /* GPP_C22 - GPIO */ : PAD_CFG_GPO(GPP_C22, 1, PLTRST),
We might want to pull this low to power off USB in sleep states
yep, would need ACPI
https://review.coreboot.org/c/coreboot/+/47191/4/src/mainboard/purism/librem... PS4, Line 562: : /* GPP_E15 - NC */ : PAD_NC(GPP_E15, NONE),
Is it really NC?
yup, because R324 is not populated; can you verify that on the board, please?