Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30414 )
Change subject: mainboard/facebook/fbg1701: Do initial mainboard commit
......................................................................
Patch Set 15:
previous comments still apply
--
To view, visit https://review.coreboot.org/c/coreboot/+/30414
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I28ac78a630ee705b1e546031f024bfe7f952ab39
Gerrit-Change-Number: 30414
Gerrit-PatchSet: 15
Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Thu, 28 Feb 2019 09:26:48 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29394 )
Change subject: src/soc/intel/braswell/hda.c: Configure HDA codecs
......................................................................
Patch Set 8:
> Patch Set 8:
>
> > Patch Set 8:
> >
> > since LPE audio is the norm on braswell devices, might it be better to set a Kconfig for HDA audio and conditionally include hda_verb.c if set? That way you don't need to modify existing boards which don't use HDA audio.
>
> For sure Kconfig can be used. I'm not sure if the PCI configuration of the HDA will changefor boards which have 'HDA disabled' in Kconfig. It will remove the read/set/enable of the HDA controller.
Intel SoC common, Broadwell SoC and many Intel southbridges have HDA support natively in their code base. I do not see any obstacles why Braswell should not follow the same convention. coreboot is a framework and IMO it is better option to support HDA init natively in Braswell and to have hda_verb.c with empty verb tables for each board if they do not support HDA.
--
To view, visit https://review.coreboot.org/c/coreboot/+/29394
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5c23ec311e5b5a6dfd6f031aa19617407fe8ed63
Gerrit-Change-Number: 29394
Gerrit-PatchSet: 8
Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 09:25:14 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29398 )
Change subject: src/soc/intel/braswell/southcluster.c: Correct serial IRQ support
......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/29398/5/src/soc/intel/braswell/Kconfig
File src/soc/intel/braswell/Kconfig:
https://review.coreboot.org/#/c/29398/5/src/soc/intel/braswell/Kconfig@137
PS5, Line 137:
: config ENABLE_SERIRQ
: bool "Enable Serial IRQ"
: default n
: help
: Enable Serial IRQ
:
: config SERIRQ_CONTINUOUS_MODE
: bool "Serial IRQ continuous mode"
: default n
: depends on ENABLE_SERIRQ
: help
: Enable Serial IRQ continuous mode
> Done
Do Chromebooks want to have the SERIRQ disabled as it was in the 'old' code base? Most coreboot boards do have the SERIRQ enabled and the continuous mode is selected in board specific options. SERIRQ is always enabled, but in quiet mode, if continuous mode is not selected. Maybe we should stick to common implementation in coreboot?
--
To view, visit https://review.coreboot.org/c/coreboot/+/29398
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7844cad69dc0563fa6109d779d0afb7c2edd7245
Gerrit-Change-Number: 29398
Gerrit-PatchSet: 5
Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 08:57:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Comment-In-Reply-To: Frans Hendriks <fhendriks(a)eltan.com>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-MessageType: comment
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29415 )
Change subject: src/soc/intel/braswell/southcluster.c: Config ISA DMA controller
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/29415
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib7af3f4ef6d6a29628bb2c27d32071be63ff6af2
Gerrit-Change-Number: 29415
Gerrit-PatchSet: 2
Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 28 Feb 2019 08:42:29 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29419 )
Change subject: soc/intel/braswell: Correct configuration of interrupts
......................................................................
Patch Set 5: Code-Review+2
Tested with Intel Celeron 3160 and 3060 by booting Debian or SeaBIOS. SeaBIOS won't run properly without interrupts and configured PIC.
--
To view, visit https://review.coreboot.org/c/coreboot/+/29419
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I128cb35dd0e348a9cd9fb162651e0aa2b7e4a3ef
Gerrit-Change-Number: 29419
Gerrit-PatchSet: 5
Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 28 Feb 2019 08:41:36 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29290 )
Change subject: src/soc/intel/braswell/cpu.c: Setup local APIC
......................................................................
Patch Set 2: Code-Review+2
Tested by booting Debian on Intel Celeron 3060 and 3160.
--
To view, visit https://review.coreboot.org/c/coreboot/+/29290
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic1da5b1bf235f34b957142e86c70a9dbfa3ded1d
Gerrit-Change-Number: 29290
Gerrit-PatchSet: 2
Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Thu, 28 Feb 2019 08:36:49 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29287 )
Change subject: src/soc/intel/braswell/acpi/irqlinks.asl: Allow IRQ10 and 11 for all LNKx
......................................................................
Patch Set 3: Code-Review+2
Tested by booting Debian on Intel Celeron 3060 and 3160.
--
To view, visit https://review.coreboot.org/c/coreboot/+/29287
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie7a263d7d50f7f85e6195777c1429dcc27a15604
Gerrit-Change-Number: 29287
Gerrit-PatchSet: 3
Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: wvervoorn <wvervoorn(a)eltan.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 08:15:04 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment