Attention is currently required from: Subrata Banik, Simon Chou, TimLiu-SMCI, Jonathan Zhang, Johnny Lin, Paul Menzel, David Hendricks, Christian Walter, Angel Pons, Jian-Ming Wang, Arthur Heymans, Elyes Haouas.
Shuming Chu (Shuming) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71945 )
Change subject: soc/intel/xeon_sp: add ebg (Emmitsburg PCH) directory ......................................................................
Patch Set 11:
(18 comments)
File src/soc/intel/xeon_sp/ebg/include/soc/gpio_soc_defs.h:
https://review.coreboot.org/c/coreboot/+/71945/comment/10f88b53_bc1f57c8 PS8, Line 204: #define GPPC_H8 144 : #define GPPC_H9 145 : #define GPPC_H10 146 : #define GPPC_H11 147 : #define GPPC_H12 148 : #define GPPC_H13 149 : #define GPPC_H14 150
They are not declared as unrouted GPIOs in EDS. […]
I think they are unrouted GPIOs? Refer to #606161 Table 13-11. No matter what, it shall be fine to keep them.
https://review.coreboot.org/c/coreboot/+/71945/comment/0acb1374_8b1b249b PS8, Line 227: #define GPP_J9 165 : #define GPP_J10 166 : #define GPP_J11 167
They are not declared as unrouted GPIOs in EDS. […]
I think they are unrouted GPIOs? Refer to #606161 Table 13-11. No matter what, it shall be fine to keep them.
https://review.coreboot.org/c/coreboot/+/71945/comment/85692b3e_6147dd5d PS8, Line 234: #define GPP_J16 172 : #define GPP_J17 173
GPP_J15 is the last pad configuration in EDS for GPIO community 4. […]
Ack
https://review.coreboot.org/c/coreboot/+/71945/comment/4088c547_b24f10eb PS8, Line 237: GPP_J17
GPP_J15
Like GPP_E23, let's keep using GPP_J17 here due to it does exist.
https://review.coreboot.org/c/coreboot/+/71945/comment/c2f3665d_377ec62e PS8, Line 241: #define GPP_I0 180
Tim, could you check this?
It can be found as unrouted GPIOs in #606161 Table 13-11.
https://review.coreboot.org/c/coreboot/+/71945/comment/d77d9c7c_e9e05337 PS8, Line 244: define GPP_I3 183 : #define GPP_I4 184 : #define GPP_I5 185 : #define GPP_I6 186 : #define GPP_I7 187 : #define GPP_I8 188 : #define GPP_I9 189 : #define GPP_I10 190 : #define GPP_I11 191
They are not declared as unrouted GPIOs in EDS. […]
I think they are unrouted GPIOs? Refer to #606161 Table 13-11. No matter what, it shall be fine to keep them.
https://review.coreboot.org/c/coreboot/+/71945/comment/0fb5abd3_b22352d8 PS8, Line 259: #define GPP_I18 198 : #define GPP_I19 199 : #define GPP_I20 200
They are not declared as unrouted GPIOs in EDS. […]
I think they are unrouted GPIOs? Refer to #606161 Table 13-11. No matter what, it shall be fine to keep them.
https://review.coreboot.org/c/coreboot/+/71945/comment/ceb5a63a_361232e4 PS8, Line 276: #define GPP_L9 213 : #define GPP_L10 214 : #define GPP_L11 215 : #define GPP_L12 216 : #define GPP_L13 217 : #define GPP_L14 218 : #define GPP_L15 219 : #define GPP_L16 220 : #define GPP_L17 221
They are not declared as unrouted GPIOs in EDS. […]
I think they are unrouted GPIOs? Refer to #606161 Table 13-11. No matter what, it shall be fine to keep them.
https://review.coreboot.org/c/coreboot/+/71945/comment/bc312e7a_46b00e96 PS8, Line 296: #define GPP_M9 231 : #define GPP_M10 232
They are not declared as unrouted GPIOs in EDS. […]
I think they are unrouted GPIOs? Refer to #606161 Table 13-11. No matter what, it shall be fine to keep them.
https://review.coreboot.org/c/coreboot/+/71945/comment/43dc6d53_d048cac8 PS8, Line 300: #define GPP_M13 235 : #define GPP_M14 236
They are not declared as unrouted GPIOs in EDS. […]
I think they are unrouted GPIOs? Refer to #606161 Table 13-11. No matter what, it shall be fine to keep them.
https://review.coreboot.org/c/coreboot/+/71945/comment/c81fc921_41d4ff0c PS8, Line 312: #define GPP_N5 245 : #define GPP_N6 246 : #define GPP_N7 247 : #define GPP_N8 248 : #define GPP_N9 249 : #define GPP_N10 250 : #define GPP_N11 251 : #define GPP_N12 252 : #define GPP_N13 253 : #define GPP_N14 254 : #define GPP_N15 255 : #define GPP_N16 256 : #define GPP_N17 257
GPP_N4 is the last pad configuration in EDS for GPIO community 5. […]
Ack
https://review.coreboot.org/c/coreboot/+/71945/comment/d6d09328_d30de115 PS8, Line 326: GPP_N17
GPP_N4
Like GPP_E23, let's keep using GPP_N17 here due to it does exist.
File src/soc/intel/xeon_sp/ebg/include/soc/pcr_ids.h:
https://review.coreboot.org/c/coreboot/+/71945/comment/f7735be9_1a055c94 PS6, Line 6: #define PID_CSME0 0x90
Tim, could you check this? I suppose this is used by HECI?
I check this definition comes from CB:61431 and it may build failed if we don't define this.
File src/soc/intel/xeon_sp/ebg/soc_gpio.c:
https://review.coreboot.org/c/coreboot/+/71945/comment/ef0b3705_cef51706 PS8, Line 24: SPI0_IO_2
GPPC_S0? See comments in gpio_soc_defs.h.
Done
https://review.coreboot.org/c/coreboot/+/71945/comment/429b9654_fb3f9bd6 PS8, Line 38: GPP_J17
GPPC_J15? See comments in gpio_soc_defs.h.
As comments in gpio_soc_defs.h, let's keep GPP_J17 here.
https://review.coreboot.org/c/coreboot/+/71945/comment/22e925d9_c648eb41 PS8, Line 45: GPP_N17
GPPC_N4? See comments in gpio_soc_defs.h.
As comments in gpio_soc_defs.h, let's keep GPP_N17 here.
https://review.coreboot.org/c/coreboot/+/71945/comment/dc672d68_88512d12 PS8, Line 113: GPP_J17
GPP_J15? See comments in gpio_soc_defs.h.
As comments in gpio_soc_defs.h, let's keep GPP_J17 here.
https://review.coreboot.org/c/coreboot/+/71945/comment/10cae6f7_44a90b50 PS8, Line 131: .first_pad = GPP_I0, : .last_pad = GPP_N17,
GPP_I1 to GPP_N4? See comments in gpio_soc_defs.h.
As comments in gpio_soc_defs.h, let's keep GPP_I0/GPP_N17 here.
-
Shuming Chu (Shuming) (Code Review)