Matthew Garrett has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37991 )
Change subject: Doc/acpi: Add rfkill device specification ......................................................................
Patch Set 4:
(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: wifi
Wi-Fi
Ack
https://review.coreboot.org/c/coreboot/+/37991/3/Documentation/acpi/rfkill.m... PS3, Line 6: bluetooth
Bluetooth
Ack
https://review.coreboot.org/c/coreboot/+/37991/3/Documentation/acpi/rfkill.m... PS3, Line 15:
maybe add a comma
Ack
https://review.coreboot.org/c/coreboot/+/37991/3/Documentation/acpi/rfkill.m... PS3, Line 26: dv
missing space?
Ack
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
Ack
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: […]
Ack
https://review.coreboot.org/c/coreboot/+/37991/3/Documentation/acpi/rfkill.m... PS3, Line 47: is not present
did you mean `is disabled`?
Ack
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. […]
The bit offsets are defined in DEVS, but in HSTA they refer to radio status rather than presence. Reworded a little in an attempt to make this clearer.