Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33652 )
Change subject: arch/x86/acpi: Add os.asl ......................................................................
Patch Set 1:
(4 comments)
Patch Set 1: Code-Review-1
(4 comments)
I don't like this. Is there a better approach?
NB this -1 is not blocking the change from being merged.
This was originally part of stoneyridge ACPI code, Paul Manzel asked to have it available for everyone.
https://review.coreboot.org/#/c/33652/1/src/arch/x86/acpi/os.asl File src/arch/x86/acpi/os.asl:
https://review.coreboot.org/#/c/33652/1/src/arch/x86/acpi/os.asl@84 PS1, Line 84: Zero
Why not 0x00 instead?
Just because I kind of copied the code from another BIOS (UEFI) using RWeverithing, with some modifications so it won't be called an actual copy. Yes, I could probably use 0x00 and 0x01 (bellow).
https://review.coreboot.org/#/c/33652/1/src/arch/x86/acpi/os.asl@90 PS1, Line 90: If(SCPM(local0, "Microsoft WindowsME: Millennium Edition"))
Do we need to care?
Probably not. Same for Windows NT. I just am not sure if anyone still uses either OS. Even WINXP is no longer supported, though it was still used recently (not sure if still used) on the embedded world.
https://review.coreboot.org/#/c/33652/1/src/arch/x86/acpi/os.asl@94 PS1, Line 94: If(SCPM(local0, "Linux"))
I would not recommend that, see: https://www.kernel.org/doc/Documentation/acpi/osi.txt […]
The work around is in line 24, which verifies if _OSI is implemented. Therefor, _OSI will only be used if implemented.
https://review.coreboot.org/#/c/33652/1/src/arch/x86/acpi/os.asl@115 PS1, Line 115: SCPM
Is this method specific to this file, or could it be reused? […]
My bad, agree. Will change.