Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41607 )
Change subject: drivers/intel/mipi_camera: camera SSDT generation
......................................................................
Patch Set 10:
(5 comments)
https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_cam…
File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_cam…
PS6, Line 159: config->ssdb.platform = 9;
: if (!config->ssdb.flash_support)
: config->ssdb.flash_support = 2;
: if (!config->ssdb.privacy_led)
: config->ssdb.privacy_led = 1;
: if (!config->ssdb.mipi_define)
: config->ssdb.mipi_define = 1;
: if (!config->ssdb.mclk_speed)
: config->ssdb.mclk_speed = 19200000;
> Some of these values can be found in Intel's "SkyCam" camera driver SDK (example files Skycam/public […]
Thanks Matt, Sugnan could you add those in?
https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_cam…
PS6, Line 188: /* Method (_DSM, 4, NotSerialized) */
: acpigen_write_method("_DSM", 0x4);
:
: /* ToBuffer (Arg0, Local0) */
: acpigen_write_to_buffer(ARG0_OP, LOCAL0_OP);
:
: /* If (LEqual (Local0, ToUUID(uuid))) */
: acpigen_write_if();
: acpigen_emit_byte(LEQUAL_OP);
: acpigen_emit_byte(LOCAL0_OP);
: acpigen_write_uuid("822ace8f-2814-4174-a56b-5f029fe079ee");
: acpigen_write_return_string(config->sensor_name ? config->sensor_name : "UNKNOWN");
: acpigen_pop_len(); /* If */
:
: /* If (LEqual (Local0, ToUUID(uuid))) */
: acpigen_write_if();
: acpigen_emit_byte(LEQUAL_OP);
: acpigen_emit_byte(LOCAL0_OP);
: acpigen_write_uuid("26257549-9271-4ca4-bb43-c4899d5a4881");
: /* ToInteger (Arg2, Local1) */
: acpigen_write_to_integer(ARG2_OP, LOCAL1_OP);
:
: /* If (LEqual (Local1, 1)) */
: acpigen_write_if_lequal_op_int(LOCAL1_OP, 1);
: acpigen_write_return_integer((config->ssdb.vcm_type ? 1 : 0) +
: (config->ssdb.rom_type ? 1 : 0) + 1);
: acpigen_pop_len(); /* If Arg2=1 */
:
: /* If (LEqual (Local1, 2)) */
: acpigen_write_if_lequal_op_int(LOCAL1_OP, 2);
: acpigen_write_return_integer(((uint32_t)i2c_bus) << 24 |
: ((uint32_t)i2c_addr) << 8 | 0x0); /* type 0 for sensor */
: acpigen_pop_len(); /* If Arg2=2 */
:
: if (config->ssdb.vcm_type) {
: /* If (LEqual (Local1, 3)) */
: acpigen_write_if_lequal_op_int(LOCAL1_OP, 3);
: acpigen_write_return_integer(((uint32_t)i2c_bus) << 24 | /* type 1 for vcm */
: ((uint32_t)config->vcm_address) << 8 | 0x01);
: acpigen_pop_len(); /* If Arg2=3 */
: }
: if (config->ssdb.rom_type) {
: /* If (LEqual (Local1, 3 or 4)) */
: acpigen_write_if_lequal_op_int(LOCAL1_OP, 3 + (config->ssdb.vcm_type ? 1 : 0));
: acpigen_write_return_integer(((uint32_t)i2c_bus) << 24 | /* type 2 for rom */
: ((uint32_t)config->rom_address) << 8 | 0x02);
: acpigen_pop_len(); /* If Arg2=3 or 4 */
: }
:
: acpigen_pop_len(); /* If uuid */
:
: /* Return (Buffer (One) { 0x0 }) */
: acpigen_write_return_singleton_buffer(0x0);
:
: acpigen_pop_len(); /* Method _DSM */
> I think _DSM is mostly used in Intel's design for Windows drivers. […]
You should be able to use acpigen_write_dsm. It takes lists of corresponding UUIDs and callback functions to add what code to run for each `if (arg2 == uuid[0]), etc.`
https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_cam…
PS6, Line 445: if (config->dep) {
: acpigen_write_name("_DEP");
: acpigen_write_package(1);
: acpigen_emit_namestring(config->dep);
: acpigen_pop_len();
: }
> To include a dependency similar to one in the following link. […]
Is the kernel driver using _DEP for something unusual? _DEP is used to "designate device objects that OSPM should assign a higher priority in start ordering due to future operation region accesses."
https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_cam…
File src/drivers/intel/mipi_camera/chip.h:
https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_cam…
PS6, Line 79: uint8_t reserved[13];
> It pads SSDB out so the binary blob in ACPI is the same size as seen on other firmwares. […]
Ack
https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_cam…
PS6, Line 107: const char *pr0;
: const char *pr3; /* _PR3 shouldn't be needed
> Idea was to push @Matt patch mostly as it is and the incremental changes related to power resource i […]
A later patch is OK with me.
--
To view, visit https://review.coreboot.org/c/coreboot/+/41607
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I15979f345fb823df2560db269e902a1ea650b69e
Gerrit-Change-Number: 41607
Gerrit-PatchSet: 10
Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Matt Delco <delco(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com>
Gerrit-Comment-Date: Mon, 01 Jun 2020 17:46:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Matt Delco <delco(a)chromium.org>
Comment-In-Reply-To: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41667 )
Change subject: AGESA f14/f15tn/f16kb: Clean up buildOpts.c files
......................................................................
AGESA f14/f15tn/f16kb: Clean up buildOpts.c files
Until now, the buildOpts.c files were primarily made out of copy-pasted
AGESA options, commented-out definitions and several useless comments;
that is, the materialization of technical debt in GCC-parsable form...
Until now.
It is assumed that the boards in the tree still boot. So, by comparing
their settings, we can extract saner defaults to place into AGESA. Many
of the settings were common across all boards of the same family, so we
promote those values to default settings. In some cases flipping a flag
was required, so the macros to alter that option had to be adapted as
well. Since those AGESA versions are expected to never receive updates,
it should not be a problem to change their files to suit our needs.
As a result, all but two buildOpts.c files now have less than 100 lines.
AGESA f14 boards need less than 50 lines, and f15tn/f16kb just require
about 60 or 70 lines in those files. Hopefully, this will make porting
more mainboards using AGESA f14/f15tn/f16kb a substantially easier task.
TEST=Use abuild --timeless to check that all AGESA f14/f15tn/f16kb
mainboards result in identical coreboot binaries.
Change-Id: Ife1ca5177d85441b9a7b24d64d7fcbabde6e0409
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
M src/mainboard/amd/inagua/buildOpts.c
M src/mainboard/amd/olivehill/buildOpts.c
M src/mainboard/amd/parmer/buildOpts.c
M src/mainboard/amd/persimmon/buildOpts.c
M src/mainboard/amd/south_station/buildOpts.c
M src/mainboard/amd/thatcher/buildOpts.c
M src/mainboard/amd/union_station/buildOpts.c
M src/mainboard/asrock/e350m1/buildOpts.c
M src/mainboard/asrock/imb-a180/buildOpts.c
M src/mainboard/asus/am1i-a/buildOpts.c
M src/mainboard/asus/f2a85-m/buildOpts.c
M src/mainboard/bap/ode_e20XX/buildOpts.c
M src/mainboard/biostar/a68n_5200/buildOpts.c
M src/mainboard/biostar/am1ml/buildOpts.c
M src/mainboard/elmex/pcm205400/buildOpts.c
M src/mainboard/gizmosphere/gizmo/buildOpts.c
M src/mainboard/gizmosphere/gizmo2/buildOpts.c
M src/mainboard/hp/abm/buildOpts.c
M src/mainboard/hp/pavilion_m6_1035dx/buildOpts.c
M src/mainboard/jetway/nf81-t56n-lf/buildOpts.c
M src/mainboard/lenovo/g505s/buildOpts.c
M src/mainboard/lippert/frontrunner-af/buildOpts.c
M src/mainboard/lippert/toucan-af/buildOpts.c
M src/mainboard/msi/ms7721/buildOpts.c
M src/mainboard/pcengines/apu1/buildOpts.c
M src/vendorcode/amd/agesa/f14/Config/PlatformInstall.h
M src/vendorcode/amd/agesa/f15tn/Config/OptionFchInstall.h
M src/vendorcode/amd/agesa/f15tn/Config/PlatformInstall.h
M src/vendorcode/amd/agesa/f15tn/Proc/CPU/cpuLateInit.h
M src/vendorcode/amd/agesa/f16kb/Config/OptionFchInstall.h
M src/vendorcode/amd/agesa/f16kb/Config/OptionGnbInstall.h
M src/vendorcode/amd/agesa/f16kb/Config/PlatformInstall.h
M src/vendorcode/amd/agesa/f16kb/Proc/CPU/cpuLateInit.h
33 files changed, 786 insertions(+), 5,000 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/41667/1
--
To view, visit https://review.coreboot.org/c/coreboot/+/41667
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ife1ca5177d85441b9a7b24d64d7fcbabde6e0409
Gerrit-Change-Number: 41667
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-MessageType: newchange
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41854 )
Change subject: usb/xhci: Fix timeout logic
......................................................................
Patch Set 2:
Was there a bug for this?
--
To view, visit https://review.coreboot.org/c/coreboot/+/41854
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I81a888aa0a1544e55e6a680be8f3b7f6e0d87812
Gerrit-Change-Number: 41854
Gerrit-PatchSet: 2
Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Mon, 01 Jun 2020 17:33:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41621 )
Change subject: mb/asrock/e350m1/buildOpts.c: Fix AGESA_VERSION_STRING
......................................................................
mb/asrock/e350m1/buildOpts.c: Fix AGESA_VERSION_STRING
This is the only board using AGESA f14 with a different version string.
Change it so that it matches the other AGESA f14 boards in the tree.
Change-Id: I384bd96db51457e68a320b99ecdbb2ada0dfbdd5
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
M src/mainboard/asrock/e350m1/buildOpts.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/41621/1
diff --git a/src/mainboard/asrock/e350m1/buildOpts.c b/src/mainboard/asrock/e350m1/buildOpts.c
index 1e644c5..29a20e7 100644
--- a/src/mainboard/asrock/e350m1/buildOpts.c
+++ b/src/mainboard/asrock/e350m1/buildOpts.c
@@ -238,7 +238,7 @@
// This is the release version number of the AGESA component
// This string MUST be exactly 12 characters long
-#define AGESA_VERSION_STRING {'V', '0', '.', '0', '.', '0', '.', '1', ' ', ' ', ' ', ' '}
+#define AGESA_VERSION_STRING {'V', '1', '.', '1', '.', '0', '.', '3', ' ', ' ', ' ', ' '}
/* MEMORY_BUS_SPEED */
#define DDR400_FREQUENCY 200 ///< DDR 400
--
To view, visit https://review.coreboot.org/c/coreboot/+/41621
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I384bd96db51457e68a320b99ecdbb2ada0dfbdd5
Gerrit-Change-Number: 41621
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newchange
Nathaniel L Desimone has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41728 )
Change subject: drivers/intel/fsp2_0: Add FSP 2.2 specific support
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/41728/1/src/drivers/intel/fsp2_0/s…
File src/drivers/intel/fsp2_0/silicon_init.c:
https://review.coreboot.org/c/coreboot/+/41728/1/src/drivers/intel/fsp2_0/s…
PS1, Line 100: multi_phase_params = xmalloc(sizeof(struct fsp_multi_phase_params));
: multi_phase_get_number =
: xmalloc(sizeof(struct fsp_multi_phase_get_number_of_phase_params));
> Do we really need to allocate heap for these? Seems like they could be local variables on the stack.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/41728
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If7177a267f3a9b4cbb60a639f1c737b9a3341913
Gerrit-Change-Number: 41728
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Mon, 01 Jun 2020 16:55:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com>
Gerrit-MessageType: comment
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Maulik V Vaghela, Duncan Laurie, Sridhar Siricilla, Aamir Bohra, Andrey Petrov, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41728
to look at the new patch set (#2).
Change subject: drivers/intel/fsp2_0: Add FSP 2.2 specific support
......................................................................
drivers/intel/fsp2_0: Add FSP 2.2 specific support
• Based on FSP EAS v2.1 – Backward compatibility is retained.
• Added multi-phase silicon initialization to increase the modularity of the
FspSiliconInit() API.
• Added FspMultiPhaseSiInit() API
• FSP_INFO_HEADER changes
o Added FspMultiPhaseSiInitEntryOffset
• Added FSPS_ARCH_UPD
o Added EnableMultiPhaseSiliconInit, bootloaders designed for
FSP 2.0/2.1 can disable the FspMultiPhaseSiInit() API and
continue to use FspSiliconInit() without change.
FSP 2.2 Specification:
https://www.intel.com/content/www/us/en/intelligent-systems/intel-firmware-…
Change-Id: If7177a267f3a9b4cbb60a639f1c737b9a3341913
Signed-off-by: Subrata Banik <subrata.banik(a)intel.com>
---
M src/drivers/intel/fsp2_0/Kconfig
M src/drivers/intel/fsp2_0/include/fsp/api.h
M src/drivers/intel/fsp2_0/include/fsp/info_header.h
M src/drivers/intel/fsp2_0/include/fsp/upd.h
M src/drivers/intel/fsp2_0/include/fsp/util.h
M src/drivers/intel/fsp2_0/silicon_init.c
M src/drivers/intel/fsp2_0/util.c
7 files changed, 130 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/41728/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/41728
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If7177a267f3a9b4cbb60a639f1c737b9a3341913
Gerrit-Change-Number: 41728
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset