Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39115 )
Change subject: mb/google/dedede: Add WLAN configuration ......................................................................
Patch Set 5:
(6 comments)
https://review.coreboot.org/c/coreboot/+/39115/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/39115/5/src/mainboard/google/dedede... PS5, Line 154: PAD_CFG_GPI_SCI_LOW What is this line used for?
https://review.coreboot.org/c/coreboot/+/39115/5/src/mainboard/google/dedede... PS5, Line 156: GPP_D3 Isn't this supposed to be the wake? I don't think PAD_CFG_GPI is the right pad configuration.
https://review.coreboot.org/c/coreboot/+/39115/5/src/mainboard/google/dedede... PS5, Line 188: PAD_CFG_NF(GPP_D19, NONE, DEEP, NF2), : /* D20 : WWAN_WLAN_COEX2 */ : PAD_CFG_NF(GPP_D20, NONE, DEEP, NF2), : /* D21 : WWAN_WLAN_COEX3 */ : PAD_CFG_NF(GPP_D21, NONE, DEEP, NF1), Are these configured for CNVi or PCIe?
https://review.coreboot.org/c/coreboot/+/39115/5/src/mainboard/google/dedede... PS5, Line 330: GPP_H19 Just a note. This will have to be exposed in ACPI tables to allow kernel driver to reset bluetooth if required. See Hatch or Octopus as examples. It can be handled as a follow-up.
https://review.coreboot.org/c/coreboot/+/39115/5/src/mainboard/google/dedede... PS5, Line 385: PAD_CFG_NF(GPD9, NONE, DEEP, NF1), Is this actually connected in hardware? I don't see it going anywhere.
https://review.coreboot.org/c/coreboot/+/39115/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/waddledoo/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/39115/5/src/mainboard/google/dedede... PS5, Line 6: 00.0 Just a note: This device is being set up in the override tree, but its clock source and other PCIe settings are being done in baseboard devicetree?