Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31639 )
Change subject: sb/intel/common/firmware: Add an option to use ifdtool ......................................................................
Patch Set 3:
I would pretty much prefer the opposite, i.e. only modify the descriptor if asked to.
In any case, as this implementation only affects the descriptor lock, a Kconfig choice would be better. Would make both Kconfig and Makefile simpler, I guess.
Just so I get it right, you mean that else branch of CONFIG_LOCK_MANAGEMENT_ENGINE in the Makefile doesn’t use ifdtool, right?
Um, no, there are three cases: 1. We want to leave the des- criptor untouched (I think this should be the default). 2. We want to make sure it's locked. 3. We want to make sure it's unlocked. Hence a Kconfig `choice` would reflect that much better.
In the Makefile this could be two separate `if` statements, one for 2. and one for 3. And if none are taken, 1. is implied.