Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37991 )
Change subject: Add a doc describing an ACPI device for controlling platform radios. ......................................................................
Patch Set 3:
(8 comments)
https://review.coreboot.org/c/coreboot/+/37991/3/Documentation/acpi/rfkill.m... File Documentation/acpi/rfkill.md:
https://review.coreboot.org/c/coreboot/+/37991/3/Documentation/acpi/rfkill.m... PS3, Line 6: bluetooth Bluetooth
https://review.coreboot.org/c/coreboot/+/37991/3/Documentation/acpi/rfkill.m... PS3, Line 6: wifi Wi-Fi
https://review.coreboot.org/c/coreboot/+/37991/3/Documentation/acpi/rfkill.m... PS3, Line 15: maybe add a comma
https://review.coreboot.org/c/coreboot/+/37991/3/Documentation/acpi/rfkill.m... PS3, Line 26: dv missing space?
https://review.coreboot.org/c/coreboot/+/37991/3/Documentation/acpi/rfkill.m... PS3, Line 34: | Maybe add a space so that the table end isn't next to the contents
https://review.coreboot.org/c/coreboot/+/37991/3/Documentation/acpi/rfkill.m... PS3, Line 47: If a bit is not set in the value returned by the DEVS method The wording of this part confused me. Wouldn't it be simpler to state on the DEVS method:
If a bit is not set, then that type of device is not supported on the platform and the return value of any other method is undefined.
https://review.coreboot.org/c/coreboot/+/37991/3/Documentation/acpi/rfkill.m... PS3, Line 47: is not present did you mean `is disabled`?
https://review.coreboot.org/c/coreboot/+/37991/3/Documentation/acpi/rfkill.m... PS3, Line 55: as defined in : DEVS. If a bit is 1 then the radio is enabled. If a bit is 0 then the radio : is disabled. This seems to be slightly mixed up. I thought DEVS was about supported devices, and not device status?
I guess you mean that the bit meaning is the same as that of SSTA?