10 comments:
File src/mainboard/google/volteer/acpi/mipi_camera.asl:
Scope (\_SB.PCI0)
{
Device (IPU0)
{
Name (_ADR, 0x00050000)
Name (_DDN, "Camera and Imaging Subsystem")
}
}
This should not be added to mainboard. It should be in SoC. In general, can you please add mipi_camera.asl in a follow-up CL?
File src/mainboard/google/volteer/bootblock.c:
/*
* This file is part of the coreboot project.
*
* Copyright 2020 Google LLC
*
* 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.
*/
Coreboot is moving towards SPDX headers.
File src/mainboard/google/volteer/dsdt.asl:
Scope (\_SB) {
Device (PEPD)
{
Name (_HID, "INT33A1" /* Intel Power Engine */) // _HID: Hardware ID
Name (_CID, EisaId ("PNP0D80") /* System Power Management Controller */) // _CID: Compatible ID
Name (_UID, One) // _UID: Unique ID
Name (PPD0, Package (0x03)
{
"\\_SB.PC00.SAT0",
Zero,
Package (0x02)
{
Zero,
Package (0x03)
{
0xFF,
Zero,
0x81
}
}
})
}
}
This is not a mainboard device. This should not be added here.
File src/mainboard/google/volteer/mainboard.c:
Patch Set #8, Line 71: mainboard_ec_init();
Any reason why this has to be done as part of chip init? I believe this can be done as part of dev init. Please see hatch as an example.
File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
Patch Set #8, Line 36: register "pch_isclk" = "1"
I am really not sure if this is correct. AFAIU, camera clocks are enabled using the ASL. Any ways, at minimum can you please raise a bug to revisit this later to check if it is really correct for volteer?
Patch Set #8, Line 53: register "sdcard_cd_gpio" = "GPP_E11"
I don't think this should be required. It was useful for native SD controllers. But, volteer seems to be using PCIe-to-SD card which can use in-band wake.
It would be good to have a comment indicating that this is eDP.
Please use tabs instead of spaces here and in the rest of the file.
Patch Set #8, Line 186: .pch_thermal_trip = 75,
This is not even used for TGL. Can you please add it when required?
chip drivers/i2c/generic
register "hid" = ""10EC5682""
register "name" = ""RT58""
register "desc" = ""Realtek RT5682""
register "irq" = "ACPI_IRQ_EDGE_HIGH(GPP_F8_IRQ)"
# Set the jd_src to RT5668_JD1 for jack detection
register "property_count" = "1"
register "property_list[0].type" = "ACPI_DP_TYPE_INTEGER"
register "property_list[0].name" = ""realtek,jd-src""
register "property_list[0].integer" = "1"
device i2c 1a on end
end
Note for future work: All the devices for audio, sar, etc. which are being added here should move to variant overridetree.cb. Not all variants would use the same devices.
To view, visit change 38620. To unsubscribe, or for help writing mail filters, visit settings.