[coreboot-gerrit] Change in coreboot[master]: arch/x86: Init firmware pointer for EC SMSC KBC1098/KBC1126 at build ...

[Bill XIE (Code Review)]

Attention is currently required from: Arthur Heymans, Nico Huber, Patrick Rudolph, Angel Pons, Julius Werner, Michael Niewöhner, Kyösti Mälkki, Aaron Durbin.
Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51671 )

Change subject: arch/x86: Init firmware pointer for EC SMSC KBC1098/KBC1126 at build time
......................................................................

Patch Set 31:

(2 comments)

File src/arch/x86/Kconfig:

https://review.coreboot.org/c/coreboot/+/51671/comment/6e1770f5_1b7c38d3 
PS29, Line 357: config ECFW_PTR_SIZE
              : 	int
              : 	help
              : 	   Size of reserved space for EC firmware pointer
> Exactly that. And ECFW_PTR_ADDR is a Kconfig, so we kind of have to expect that it might change. Which makes me realize the following: If the Kconfig is used in arch/x86/ then it should be declared in arch/x86/Kconfig, along with a help text that describes this constraint (i.e. must be below the reset vector or FIT pointer if present). The EC Kconfig should only set the default number.

As Nico said, ECFW_PTR_ADDR and ECFW_PTR_SIZE had better be declared in arch/x86/Kconfig, and assigned with value in the EC Kconfig.

File src/ec/hp/kbc1126/ecfw_ptr.h:

https://review.coreboot.org/c/coreboot/+/51671/comment/e3e3221a_920ec61f 
PS31, Line 8: struct ecfw_addr {
            : 	/* 8-byte offset of firmware blob in big endian */
            : 	uint16_t off;
            : 	/* bitwise inverse of "off", for error checking */
            : 	uint16_t inv;
            : };
            : 
            : struct ecfw_ptr {
            : 	struct ecfw_addr fw1;
            : 	struct ecfw_addr fw2;
            : };
> These structs should be `__packed` to ensure the compiler doesn't add any padding.

Done