Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 13: Code-Review-1
(8 comments)
Thank you for the patch, but I am a little upset, that after two people signed off this commit, there are still coding style issues. The reviewers time is also valuable, so they should not have to be distracted with these simple issue.
https://review.coreboot.org/c/coreboot/+/39360/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39360/13//COMMIT_MSG@12 PS13, Line 12: Add ON and OFF logic as Power Rails are same for both sensor Please format it as a list.
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/Kconfig:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 85: Select this option to enable camera ACPI support on the variant Please add a dot/period at the end.
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 4: * Copyright (C) 2020 Intel Corporation. No dot/period at the end.
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 46: STXS(GPP_D15) Please use tabs. Where did you copy this from?
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 54: Sleep(5) What is the unit?
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 4: * Copyright (C) 2020 Intel Corporation. Ditto.
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 4: * Copyright (C) 2020 Intel Corporation. Ditto.
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 18: Name (EP00, Package (0x02) Tabs.