Attention is currently required from: Angel Pons, Marvin Drees.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55895 )
Change subject: superio/nuvoton/nct5567d: Add NCT5567D support
......................................................................
Patch Set 6:
(2 comments)
Patchset:
PS6:
only had a very brief look; mainly looked at the differences
File src/superio/nuvoton/nct5567d/superio.c:
https://review.coreboot.org/c/coreboot/+/55895/comment/3192fc01_82da2480
PS6, Line 62: 0x0ff8, },
{ NULL, NCT5567D_SP2, PNP_IO0 | PNP_IRQ0, 0x0ff8, }, is missing here
--
To view, visit https://review.coreboot.org/c/coreboot/+/55895
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0a9fab2bb1fb7dc777b14e70a741e04636b967b8
Gerrit-Change-Number: 55895
Gerrit-PatchSet: 6
Gerrit-Owner: Marvin Drees <marvin.drees(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Marvin Drees <marvin.drees(a)9elements.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 13:15:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Krishna Prabhakaran, Furquan Shaikh, Maulik V Vaghela, Selma Bensaid, Angel Pons, Subrata Banik, Patrick Rudolph.
Meera Ravindranath has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52758 )
Change subject: drivers/intel/gma: Support IGD Opregion 2.1
......................................................................
Patch Set 11:
(10 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/52758/comment/f02cab9b_8afb48e2
PS10, Line 12: incase
> in case
Done
File src/drivers/intel/gma/Kconfig:
https://review.coreboot.org/c/coreboot/+/52758/comment/b24ea982_d66f368e
PS6, Line 149: config INTEL_GFX_IGD_OPREGION_2_1
> Please align name with above convention […]
Done
https://review.coreboot.org/c/coreboot/+/52758/comment/ef21e89d_e207e1da
PS6, Line 153: Support IGD Opregion version 2.1
> Platform supports IGD opregion version 2.1 […]
Ack
File src/drivers/intel/gma/Kconfig:
https://review.coreboot.org/c/coreboot/+/52758/comment/b672964b_9bccfd5f
PS10, Line 57: register (0xe0) rather than the SWSCI register (0xe8).
> Place new Kconfig options here.
Done
https://review.coreboot.org/c/coreboot/+/52758/comment/a47d1bd0_dc34f2d1
PS10, Line 149: config INTEL_IGD_OPREGION_VERSION
> This shouldn't be guarded by `if GFX_GMA`. […]
Done
https://review.coreboot.org/c/coreboot/+/52758/comment/093af3ac_f3b895bb
PS10, Line 151: default 0x200
> To handle the different RVDA meaning, as we don't support Opregion versions earlier than 2. […]
Done
File src/drivers/intel/gma/opregion.h:
https://review.coreboot.org/c/coreboot/+/52758/comment/871151e1_4c2e05e5
PS6, Line 32: #define IGD_OPREGION_VERSION 2
> Can we put this macro as IF condition or select as per KCONFIG. […]
Done
File src/drivers/intel/gma/opregion.c:
https://review.coreboot.org/c/coreboot/+/52758/comment/e94b2e59_8e3bd218
PS6, Line 334:
> nit: remove extra line added here
Done
https://review.coreboot.org/c/coreboot/+/52758/comment/9cfc1cbd_27d718c7
PS6, Line 364: if(CONFIG(INTEL_GFX_IGD_OPREGION_2_1))
> We can remove this
Done
File src/drivers/intel/gma/opregion.c:
https://review.coreboot.org/c/coreboot/+/52758/comment/4b651a3c_bc2b2c21
PS10, Line 362: CONFIG_INTEL_IGD_OPREGION_VERSION
> This now has a different value. Looks like it's not intentional? […]
Thanks for this, yes a left shift of 16 is required.
Bits [31:24] - Major Version Number
BCD Integer representing the major version of the OpRegion
Bits [23:16] - Minor Version Number
BCD Integer representing the minor version of the OpRegion
--
To view, visit https://review.coreboot.org/c/coreboot/+/52758
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I95a9f3df185002a4e38faa910f867ace0b97ac2b
Gerrit-Change-Number: 52758
Gerrit-PatchSet: 11
Gerrit-Owner: Meera Ravindranath <meera.ravindranath(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Krishna Prabhakaran <krishna.prabhakaran(a)intel.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Krishna Prabhakaran <krishna.prabhakaran(a)intel.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 07 Jul 2021 10:49:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Werner Zeh.
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56034 )
Change subject: mb/siemens/mc_ehl: Clean up Kconfig
......................................................................
Patch Set 1:
(1 comment)
File src/mainboard/siemens/mc_ehl/Kconfig:
https://review.coreboot.org/c/coreboot/+/56034/comment/fc342ef2_aa85e3c7
PS1, Line 3: select SOC_INTEL_ELKHARTLAKE
: select BOARD_ROMSIZE_KB_32768
: select DRIVERS_I2C_GENERIC
: select HAVE_SPD_IN_CBFS
: select HAVE_ACPI_TABLES
:
> and maybe you can also go directly to 16MB flash size in this patch
sorry...56035: mb/siemens/mc_ehl: Switch to 16 MB ROM and provide a flashmap | https://review.coreboot.org/c/coreboot/+/56035
--
To view, visit https://review.coreboot.org/c/coreboot/+/56034
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If231f37f06c6763d52a821799e87fdb3010af0aa
Gerrit-Change-Number: 56034
Gerrit-PatchSet: 1
Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 10:41:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-MessageType: comment
Attention is currently required from: Werner Zeh.
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56034 )
Change subject: mb/siemens/mc_ehl: Clean up Kconfig
......................................................................
Patch Set 1:
(1 comment)
File src/mainboard/siemens/mc_ehl/Kconfig:
https://review.coreboot.org/c/coreboot/+/56034/comment/47618957_0e6a34fe
PS1, Line 3: select SOC_INTEL_ELKHARTLAKE
: select BOARD_ROMSIZE_KB_32768
: select DRIVERS_I2C_GENERIC
: select HAVE_SPD_IN_CBFS
: select HAVE_ACPI_TABLES
:
> Why don't you sort it alphabetically?
and maybe you can also go directly to 16MB flash size in this patch
--
To view, visit https://review.coreboot.org/c/coreboot/+/56034
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If231f37f06c6763d52a821799e87fdb3010af0aa
Gerrit-Change-Number: 56034
Gerrit-PatchSet: 1
Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 10:39:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-MessageType: comment
Attention is currently required from: Daniel Kurtz, Kevin Chang, Martin Roth.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56100 )
Change subject: grunt/treeya: add Realtek ALC5682 codec support
......................................................................
Patch Set 2:
(3 comments)
File src/mainboard/google/kahlee/variants/treeya/audio.c:
https://review.coreboot.org/c/coreboot/+/56100/comment/c775406c_7d75411b
PS2, Line 22: 0xfedc2000
Can you define a macro for this?
https://review.coreboot.org/c/coreboot/+/56100/comment/9187f9cb_f4a648f9
PS2, Line 24: }
Can one of the checks be moved into the do-while condition?
```
do {
mmio_dev = dev_find_path(mmio_dev, DEVICE_PATH_MMIO);
if (mmio_dev == NULL)
break;
} while (mmio_dev->path.mmio.addr != 0xfedc2000);
```
https://review.coreboot.org/c/coreboot/+/56100/comment/ca9ca20c_25c3d9e8
PS2, Line 27: return;
Can’t you return already above? Maybe also print an error?
--
To view, visit https://review.coreboot.org/c/coreboot/+/56100
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I49c673fd944b2c2a79c4283eee941a16596ba7fa
Gerrit-Change-Number: 56100
Gerrit-PatchSet: 2
Gerrit-Owner: Kevin Chang <kevin.chang(a)lcfc.corp-partner.google.com>
Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Alec Wang <alec.wang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Allen Cheng <allen.cheng(a)lcfc.corp-partner.google.com>
Gerrit-CC: Jerry2 Huang <jerry2.huang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Daniel Kurtz <djkurtz(a)chromium.org>
Gerrit-Attention: Kevin Chang <kevin.chang(a)lcfc.corp-partner.google.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 09:48:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment