Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47013 ) Change subject: mb/google/dedede/var/metaknight: Add device settings ...................................................................... Patch Set 1: (8 comments) https://review.coreboot.org/c/coreboot/+/47013/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/47013/1//COMMIT_MSG@9 PS1, Line 9: Add the configuration in device tree: I would recommend splitting each device configuration into a CL of their own. https://review.coreboot.org/c/coreboot/+/47013/1//COMMIT_MSG@14 PS1, Line 14: 5. Add WiFi configuration Is metaknight using Discrete WiFi or CNVi? https://review.coreboot.org/c/coreboot/+/47013/1//COMMIT_MSG@15 PS1, Line 15: 6. Add DPTF setting Is the DPTF value tuned already. If not, I would recommend you to drop it and use the baseboard DPTF configuration for now. Once you have the tuned value, it can be added. https://review.coreboot.org/c/coreboot/+/47013/1/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/metaknight/overridetree.cb: https://review.coreboot.org/c/coreboot/+/47013/1/src/mainboard/google/dedede... PS1, Line 35: .i2c[3] = { : .speed = I2C_SPEED_FAST, : }, Can be removed. https://review.coreboot.org/c/coreboot/+/47013/1/src/mainboard/google/dedede... PS1, Line 41: }" Disable I2C3 register "SerialIoI2cMode" = "{ [PchSerialIoIndexI2C0] = PchSerialIoPci, [PchSerialIoIndexI2C1] = PchSerialIoPci, [PchSerialIoIndexI2C2] = PchSerialIoPci, [PchSerialIoIndexI2C3] = PchSerialIoDisabled, [PchSerialIoIndexI2C4] = PchSerialIoPci, [PchSerialIoIndexI2C5] = PchSerialIoPci, }" https://review.coreboot.org/c/coreboot/+/47013/1/src/mainboard/google/dedede... PS1, Line 90: Camera User Facing Camera https://review.coreboot.org/c/coreboot/+/47013/1/src/mainboard/google/dedede... PS1, Line 95: Camera World Facing Camera https://review.coreboot.org/c/coreboot/+/47013/1/src/mainboard/google/dedede... PS1, Line 106: "ACPI_IRQ_WAKE_EDGE_LOW(GPP_B3_IRQ)" We recently learnt that HID over I2C uses Level triggered IRQs. So needs to be fixed. Same for all the I2C Human Interface Devices. -- To view, visit https://review.coreboot.org/c/coreboot/+/47013 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie034724ea5c6edff6cdba694605d3238f66be910 Gerrit-Change-Number: 47013 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Chen <Tim-Chen@quantatw.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Henry Sun <henrysun@google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub@google.com> Gerrit-Reviewer: Paul Fagerburg <pfagerburg@chromium.org> Gerrit-Reviewer: Tim Chen <tim-chen@quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Tue, 10 Nov 2020 05:03:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment