John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39785 )
Change subject: soc/intel/tigerlake: Configure TCSS xHCI power management ......................................................................
Patch Set 12:
(8 comments)
https://review.coreboot.org/c/coreboot/+/39785/12/src/soc/intel/tigerlake/ac... File src/soc/intel/tigerlake/acpi/tcss.asl:
https://review.coreboot.org/c/coreboot/+/39785/12/src/soc/intel/tigerlake/ac... PS12, Line 6: * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details.
Change to SPDX format?
Removed Copyright (C) 2020 Intel Corp.
https://review.coreboot.org/c/coreboot/+/39785/12/src/soc/intel/tigerlake/ac... PS12, Line 163: */
96 columns
Ack
https://review.coreboot.org/c/coreboot/+/39785/12/src/soc/intel/tigerlake/ac... PS12, Line 203: * 0xFE - Command timeout
One of the calls to DSGS is doing "DSGS() & 0x2" which would return true with these failure return v […]
Ack
https://review.coreboot.org/c/coreboot/+/39785/12/src/soc/intel/tigerlake/ac... PS12, Line 381: (
nit: space before (
Ack
https://review.coreboot.org/c/coreboot/+/39785/12/src/soc/intel/tigerlake/ac... File src/soc/intel/tigerlake/acpi/tcss_xhci.asl:
https://review.coreboot.org/c/coreboot/+/39785/12/src/soc/intel/tigerlake/ac... PS12, Line 6: * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details.
change to SPDX style
Removed Copyright (C) 2020 Intel Corp.
https://review.coreboot.org/c/coreboot/+/39785/12/src/soc/intel/tigerlake/ac... PS12, Line 16: BASE(_ADR), 0x100
Is there any downside to taking the whole 256 byte config space? Could this instead just expose one […]
Ack. Keep D0D3 for debug purpose.
https://review.coreboot.org/c/coreboot/+/39785/12/src/soc/intel/tigerlake/ac... PS12, Line 34: PMEE
Are we sure this is posted write and doesn't need read back? […]
Kernel notifies pci bus driver that xhci D0 entry has commenced. ACPI updates PMCSR by setting PME_en=0. The power flow specification specify "This could be done conditionally based on whether or not the xhci was armed for wake". It does not specifically require read back. It seems ok.
https://review.coreboot.org/c/coreboot/+/39785/12/src/soc/intel/tigerlake/ac... PS12, Line 150: Method (_PS0, 0, Serialized) : { : } : : Method (_PS2, 0, Serialized) : { : } : : Method (_PS3, 0, Serialized) : { : }
do these empty methods need to exist?
Ack