Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34166 )
Change subject: soc/intel/{cnl,icl}: Drop open-source-abusive defaults for CAR ......................................................................
Patch Set 3:
Patch Set 3:
Patch Set 3:
Patch Set 2:
My -2 is just for commit msg. i guess we all know what needs to write when we think something been added in past and not required now. Do i really need to be that explicit ?
Most of us are not native English speakers, so misunderstandings can and will happen. Patience is crucial.
In any case, let's be constructive about the commit message: what do you think about this?
soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default
The FSP_CAR option has additional configuration options whose default values result in boot failures. Since default values shall always boot, default to the open-source CAR NEM Enhanced implementation instead.
Additionally, drop the vendor-specific distinction for default values, as the distinction is not justified and does not make any sense.
Much better! I'd rephrase the last paragraph a bit, how about "With the open source implementation the default, we also get rid of a vendor specific special case."?
Sounds good, I was in a rush while writing that last part. I came up with this:
soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default
The FSP_CAR option has additional configuration options whose default values result in boot failures. Since default values shall always boot, default to the open-source CAR NEM Enhanced implementation instead. This also allows us to get rid of an unnecessary vendor-specific special case.