Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43418 )
Change subject: soc/amd/picasso: use FADT devicetree configuration options ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43418/1/src/soc/amd/picasso/acpi.c File src/soc/amd/picasso/acpi.c:
https://review.coreboot.org/c/coreboot/+/43418/1/src/soc/amd/picasso/acpi.c@... PS1, Line 129: fadt->iapc_boot_arch = cfg->fadt_boot_arch; /* legacy free default */
What controls 8042 timer enablement? Maybe hook things up so that this is automatically configured d […]
Hrm, instead of allowing one to arbitrarily override this, maybe use an `enable_8042` option?
https://review.coreboot.org/c/coreboot/+/43418/1/src/soc/amd/picasso/acpi.c@... PS1, Line 132: fadt->flags = cfg->fadt_flags;
it might be a good idea to set all options that are mandatory here and only bitwise or that with the […]
Or even better: have devicetree options to enable such options instead of directly poking FADT. I had an insight recently: the devicetree should be as abstract as possible. That is, hide implementation details and provide high-level settings. Yes, the `register` keyword needs to be killed with fire.