[XL] Change in coreboot[main]: drivers/option: Add forms in cbtables

[Angel Pons (Code Review)]

Attention is currently required from: Benjamin Doron, Lean Sheng Tan, Martin L Roth, Maximilian Brune, Nico Huber, Patrick Rudolph, Sean Rhodes, Werner Zeh.

Angel Pons has posted comments on this change by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/74121?usp=email )

Change subject: drivers/option: Add forms in cbtables
......................................................................

Patch Set 27: Code-Review+1

(9 comments)

File Documentation/drivers/cfr.md:

https://review.coreboot.org/c/coreboot/+/74121/comment/4caca4fa_71293744?usp... :
PS27, Line 9: S
lowercase s

https://review.coreboot.org/c/coreboot/+/74121/comment/b58ae18b_eefcd5be?usp... :
PS27, Line 15: means 
meaning?

https://review.coreboot.org/c/coreboot/+/74121/comment/9c96617a_606d69e0?usp... :
PS27, Line 24: it's
its (there are multiple ocurrences)

File src/commonlib/include/commonlib/cfr.h:

https://review.coreboot.org/c/coreboot/+/74121/comment/1cf92cce_1c046472?usp... :
PS27, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */
We should consider moving this to commonlib/bsd for interoperability. I (wrote the initial implementation) am OK with the license change, but we'd need to ask others first.

https://review.coreboot.org/c/coreboot/+/74121/comment/2372341f_dc34df68?usp... :
PS27, Line 6: #include <stdint.h>
Redundant, types.h already includes it.

https://review.coreboot.org/c/coreboot/+/74121/comment/8c2b493a_b1303600?usp... :
PS27, Line 106: 				 *  CFR_TAG_VARCHAR_UI_HELPTEXT or CFR_TAG_VARCHAR_DEF_VALUE
nit: remove extra space

```suggestion
    			 * CFR_TAG_VARCHAR_UI_HELPTEXT or CFR_TAG_VARCHAR_DEF_VALUE
```

https://review.coreboot.org/c/coreboot/+/74121/comment/df5734b0_98b455ef?usp... :
PS27, Line 143: uint32_t tag;		/* CFR_TAG_OPTION_VARCHAR */
              : 	uint32_t size;
              : 	uint64_t object_id;	/* Uniqueue ID */
              : 	uint64_t dependency_id;	/* Grayout if value of lb_cfr_numeric_option with given ID is 0.
              : 				 * Ignore if field is 0.
              : 				 */
              : 	uint32_t flags;		/* enum cfr_option_flags */
We could have an `option header` structure so that we can factor out filling in the header fields.

File src/drivers/option/cfr.c:

https://review.coreboot.org/c/coreboot/+/74121/comment/ad93cf6a_f0ed2c15?usp... :
PS27, Line 42: 	data = (uint8_t *)(cfr_str + 1);
Right, this is a variable length structure and we don't have a struct member for the data. I would prefer this for clarity:

```suggestion
    char *data = current + sizeof(*cfr_str);
```

https://review.coreboot.org/c/coreboot/+/74121/comment/f4628b3c_5c829b52?usp... :
PS27, Line 48: /* Fill padding bytes */
             : 	padding = cfr_str->size - (sizeof(*cfr_str) + cfr_str->data_length);
             : 	memset(&data[cfr_str->data_length], 0, padding);
This should not be necessary