Attention is currently required from: Tim Wawrzynczak, Sridhar Siricilla, Nick Vaccaro, Balaji Manigandan, Krishna P Bhat D. Reka Norman has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58679 )
Change subject: util/spd_tools: Add LP5 support for ADL ......................................................................
Patch Set 9:
(6 comments)
Patchset:
PS8:
Reka, +1 to your assessment. […]
Thanks for the comments. I'm glad you agree. It just felt weird trying to describe these ADL-specific things in a README that's meant to be more general.
Patchset:
PS9: Thanks for the reviews!
Should we wait for someone from Intel to review it too before submitting? I _think_ I captured their requirements correctly, but it would be good for someone to check.
File util/spd_tools/src/spd_gen/lp5.go:
https://review.coreboot.org/c/coreboot/+/58679/comment/eab5be53_aaf92cca PS5, Line 18: DensityPerDieGb int
Not for this CL: Does that mean we can remove the RAM_CHANNEL_SELECT GPIO strap? Something to check […]
I'm not familiar with this. I'll look into it.
https://review.coreboot.org/c/coreboot/+/58679/comment/95ec5fcf_857dd150 PS5, Line 456: ok == false
nit: I think the convention is […]
Good catch, that's much cleaner.
https://review.coreboot.org/c/coreboot/+/58679/comment/89b64005_6441ff13 PS5, Line 484: ; ok == false
`; !ok {`
Done
File util/spd_tools/src/spd_gen/lp5.go:
https://review.coreboot.org/c/coreboot/+/58679/comment/6cdaade9_6afe8572 PS8, Line 603: k == false
nit: `!ok`
Done