Attention is currently required from: Bora Guvendik.
Anil Kumar K has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57560 )
Change subject: chipset_enable.c: Add Ids for recent Intel chipsets
......................................................................
Patch Set 3:
(2 comments)
File chipset_enable.c:
https://review.coreboot.org/c/flashrom/+/57560/comment/b2bdfffe_2ef06019
PS2, Line 2018:
> can you align these please
Ack
https://review.coreboot.org/c/flashrom/+/57560/comment/f58c58ed_7cd580ea
PS2, Line 2020:
> can you align these please
Ack
--
To view, visit https://review.coreboot.org/c/flashrom/+/57560
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iff387076567e639485ac15c25c1bcc8c37a07037
Gerrit-Change-Number: 57560
Gerrit-PatchSet: 3
Gerrit-Owner: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: AndreX Andraos <andrex.andraos(a)intel.com>
Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.corp-partner.google.com>
Gerrit-CC: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com>
Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-CC: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-CC: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-CC: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Comment-Date: Fri, 10 Sep 2021 17:58:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-MessageType: comment
Attention is currently required from: Anil Kumar K.
Bora Guvendik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57560 )
Change subject: chipset_enable.c: Add Ids for recent Intel chipsets
......................................................................
Patch Set 2:
(2 comments)
File chipset_enable.c:
https://review.coreboot.org/c/flashrom/+/57560/comment/8c449314_3269ab32
PS2, Line 2018:
can you align these please
https://review.coreboot.org/c/flashrom/+/57560/comment/373765c1_f0b03083
PS2, Line 2020:
can you align these please
--
To view, visit https://review.coreboot.org/c/flashrom/+/57560
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iff387076567e639485ac15c25c1bcc8c37a07037
Gerrit-Change-Number: 57560
Gerrit-PatchSet: 2
Gerrit-Owner: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: AndreX Andraos <andrex.andraos(a)intel.com>
Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.corp-partner.google.com>
Gerrit-CC: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com>
Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-CC: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-CC: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-CC: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Comment-Date: Fri, 10 Sep 2021 17:45:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Tim Crawford has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57533 )
Change subject: chipset_enable.c: Add TGP-H IDs
......................................................................
Set Ready For Review
--
To view, visit https://review.coreboot.org/c/flashrom/+/57533
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I96f63253d42578151f99dcbb42347afecc03f49d
Gerrit-Change-Number: 57533
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 10 Sep 2021 14:37:52 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jonathan Zhang.
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54965 )
Change subject: Add support for Intel Emmitsburg PCH
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS1:
> Did anybody look into it?
Yes, and IMO we should find a better heuristic or require the user to specify a ICH/PCH generation or PCI ID when the programmer is not "internal".
ICCRIBA is obsolete and is reserved on chipsets for the past several years (6-series was the newest chipset with it AFAICT), so checking `if (content->ICCRIBA == 0x00)` works by accident in most cases it seems. The ISL/PSL field shows inconsistent results between what's documented and what's in BKC images.
Ibex Peak (5-series?) seems obsolete. I can't even find a SPI programming guide for it, though maybe you have a document number handy?
--
To view, visit https://review.coreboot.org/c/flashrom/+/54965
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2a1bb7467e693d1583aa885fa0e277075edd4a3e
Gerrit-Change-Number: 54965
Gerrit-PatchSet: 3
Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: David Hendricks
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Deomid "rojer" Ryabkov <rojer9(a)fb.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Comment-Date: Fri, 10 Sep 2021 06:23:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 )
Change subject: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops
......................................................................
Patch Set 17:
(10 comments)
File ite_ecfw.c:
https://review.coreboot.org/c/flashrom/+/55715/comment/4fe5e38e_bafe7818
PS14, Line 90: wait_for_ext_crystal
the name is confusing; 1 for this bit means `use internal clock until external crystal is ready`, so correct would be `no_wait_for_ext_crystal` or `int_osc_until_crystal_ready`
https://review.coreboot.org/c/flashrom/+/55715/comment/1f76e870_8b33b4ad
PS14, Line 105: constant?
yes
https://review.coreboot.org/c/flashrom/+/55715/comment/632a2032_7558fb3c
PS14, Line 112: enum {
: ESPI_IF = 0xa4,
: LPC_IF,
: };
:
Actually, bit 0 controls LPC vs eSPI, so we could also have 0xa4 static and a bitfield. having the static value in the enum values isn't wrong, though.
struct ite_string {
...
uint8_t _a4_byte :7;
uint8_t espi_lpc :1;
...
}
https://review.coreboot.org/c/flashrom/+/55715/comment/0823cbb4_33c98059
PS14, Line 315: = (blocks_1_2 ? 0x94 : 0x85);
we know the bits / have a struct. let's use it. what about sth like this?
https://review.coreboot.org/c/flashrom/+/55715/comment/530c08d2_4830b917
PS14, Line 324: 0xaa
this means "disable", let's give it a name or comment at least
File ite_ecfw.c:
https://review.coreboot.org/c/flashrom/+/55715/comment/c10f2bd9_9deaeb9c
PS17, Line 142: shutdown
isn't it `force_off` or `wdg_reset`?
https://review.coreboot.org/c/flashrom/+/55715/comment/9f415055_a5d3e18c
PS17, Line 208: 0
maybe add enum or define for the type(s)?
https://review.coreboot.org/c/flashrom/+/55715/comment/b1c1c10b_6ed06605
PS17, Line 782: if (!ec_write_reg(0xf9, 0x20, EC_MAX_STATUS_CHECKS) ||
: !ec_write_reg(0xfa, 0x02, EC_MAX_STATUS_CHECKS) ||
: !ec_write_reg(0xfb, 0x00, EC_MAX_STATUS_CHECKS) ||
: !ec_write_reg(0xf8, 0xb1, EC_MAX_STATUS_CHECKS)) {
: msg_perr("Unable to initialize controller.\n");
: return 1;
: }
:
drop; done in line #208 already
https://review.coreboot.org/c/flashrom/+/55715/comment/45538c55_dec5a924
PS17, Line 813: probe_ite_superio_support(ctx_data);
don't we want to probe the sio before sending any ec commands and exit if it isn't compatible?
https://review.coreboot.org/c/flashrom/+/55715/comment/af59b67b_80b736f1
PS17, Line 835:
: if (register_shutdown(ite_ecfw_shutdown, ctx_data))
: goto ite_ecfw_init_exit;
:
do we really want to reset the device, when flashrom fails to register the shutdown callback?
--
To view, visit https://review.coreboot.org/c/flashrom/+/55715
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031
Gerrit-Change-Number: 55715
Gerrit-PatchSet: 17
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Thu, 09 Sep 2021 15:10:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Michael Niewöhner.
Hello Felix Singer, build bot (Jenkins), Nico Huber, Sergii Dmytruk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/55715
to look at the new patch set (#17).
Change subject: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops
......................................................................
ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops
Add a new programmer supporting ITE Embedded Controllers found on
TUXEDO laptops. Tested on TUXEDO InfinityBook S 14 Gen6 and 15 Gen6
with EC firmware updates from IBV, EC versions 1.07.02TR1 and
1.07.04TR4.
Example standard command to update EC firmware on a TUXEDO laptop:
"flashrom -p ite_ec:romsize=128K,autoload=disable -w eL14MU02.TR1"
Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031
---
M Makefile
M flashrom.8.tmpl
M it87spi.c
A ite_ecfw.c
A ite_ecfw.h
M meson.build
M meson_options.txt
M programmer.h
M programmer_table.c
9 files changed, 1,100 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/15/55715/17
--
To view, visit https://review.coreboot.org/c/flashrom/+/55715
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031
Gerrit-Change-Number: 55715
Gerrit-PatchSet: 17
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: newpatchset