[M] Change in coreboot[master]: mb/purism/librem_cnl: Provide CBFS setting for Mini auto power on

[Jonathon Hall (Code Review)]

Attention is currently required from: Paul Menzel, Angel Pons, Felix Held.

Jonathon Hall has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74363 )

Change subject: mb/purism/librem_cnl: Provide CBFS setting for Mini auto power on
......................................................................

Patch Set 2:

(3 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/74363/comment/7f740027_fed07825 
PS2, Line 9: Control Mini v1/v2 automatic power on by adding a 'board_settings' file
           : to CBFS.  This allows us to use one build each for Mini v1/v2 that is
           : configured differently for different uses.  By default, the EC setting is
           : not configured by coreboot, and the OS could configure it.
           : 
           : Add ITE SuperIO configuration to device tree and configure base address
           : of BRAM1.  (BRAM1 contains the EC firmware setting for automatic power
           : on.)
           : 
           : Test: Build Mini v2, boot with no settings in CBFS.  Add board_settings
           : configured for automatic power-on, flash, boot, confirm EC now powers
           : on automatically when power applied.
> Please reflow for 72 characters per line.

Good catch, did not realize the Gerrit web editor allows 74 chars per line, d'oh

Patchset:

PS2:
> Can the hunk hooking up the Super I/O be separate? (Is there a change in behavior regarding fan cont […]

I'll split that up.  Shouldn't be any changes but I will double check what was enabled by default, was a while back that I put this together.

You could configure this from the OS, I have not tried nvramtool but I have done it manually.  We have customers using these as home servers, and they wanted this baked into the firmware so it could be provisioned easily without an extra step and they did not need board-specific stuff in the OS.  Maybe this is influenced by expectations from AMI firmware that offers this as a firmware setting.

I did not realize CMOS default values could be put in CBFS though, that's a good thought, I'll check that out.  Is there any doc on that by chance?

File src/mainboard/purism/librem_cnl/variants/librem_mini/mainboard.c:

https://review.coreboot.org/c/coreboot/+/74363/comment/21bfb992_a4744a35 
PS2, Line 21: 	uint8_t ec_power_on = 0;
> Any reason to limit the size? Why not `unsigned int` or `bool`? […]

It's reading a 1-byte field from the board settings file with memcpy.  I thought that was simpler than doing the whole dance of casting the data pointer to a struct pointer since there's just the one setting right now, and it is just read once here (no sense memcpying and then extending to an unsigned int).  I can go the struct route if you think that would be clearer though.

(It can't be a bool because it's a three-valued setting, the default is not to touch the EC setting so you could configure from the OS if you prefer.)