April 2017 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[staging]: Remove undefined behavior
by build bot (Jenkins) (Code Review) April 27, 2017

April 27, 2017

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/18792 ) Change subject: Remove undefined behavior ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/284/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/292/ : SUCCESS -- To view, visit https://review.coreboot.org/18792 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82b6235e11b425fae45eebbe06b08f81c5bdbb98 Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Remove undefined behavior
by Stefan Reinauer (Code Review) April 27, 2017

April 27, 2017

Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/18792 ) Change subject: Remove undefined behavior ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/18792 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82b6235e11b425fae45eebbe06b08f81c5bdbb98 Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: chipset_enable: Add support for Intel Skylake / Kabylake
by Nico Huber (Code Review) April 27, 2017

April 27, 2017

Nico Huber has posted comments on this change. ( https://review.coreboot.org/18925 ) Change subject: chipset_enable: Add support for Intel Skylake / Kabylake ...................................................................... Patch Set 2: (2 comments) https://review.coreboot.org/#/c/18925/2/chipset_enable.c File chipset_enable.c: Line 852: struct pci_dev *const spi_dev = pci_get_dev(pci_acc, 0, 0, 0x1f, 5); > It just confused me at first, rather than be recognizable, I guess I'd stil It feels pretty common when you've looked too much at coreboot source. Line 858: const int ret_bc = enable_flash_ich_bios_cntl_config_space(spi_dev, pch_generation, 0xdc); > Humm.. could it be by any chance the EC chip ? The librems have a separate The EC flash can be anywhere, even shared SPI chips are possible. But usually, today, you have to ask the EC in a proprietary way to access its flash. One configuration that was common ~10 years ago: a shared SPI flash connected to the EC; the BIOS was then accessed over LPC through the EC ;) that's why we have that scary warning about flashing laptops. The chromiumos patch didn't touch the LPC stuff too, IIRC. -- To view, visit https://review.coreboot.org/18925 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I000819aff25fbe9764f33df85f040093b82cd948 Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: chipset_enable: Add support for Intel Skylake / Kabylake
by Youness Alaoui (Code Review) April 27, 2017

April 27, 2017

Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/18925 ) Change subject: chipset_enable: Add support for Intel Skylake / Kabylake ...................................................................... Patch Set 5: Code-Review+1 (2 comments) https://review.coreboot.org/#/c/18925/2/chipset_enable.c File chipset_enable.c: Line 852: > > Here, I would do : It just confused me at first, rather than be recognizable, I guess I'd still keep the dev->domain, and dev->bus, but you're right about the dev->dev not being used and using 0x1f instead. I'd at least put a comment to say "SPI interface is on D31:F5 according to spec". But maybe it's not clear to me just because I'm not used to this ? Line 858: > Firmware Hub (FWH) is basically a parallel flash chip on the LPC bus. Humm.. could it be by any chance the EC chip ? The librems have a separate flash chip for the EC, but it's SPI too (afaik), and I've seen that chromebooks supports flashing the EC by using the 'internal:bus=lpc' option (which doesn't work with upstream flashrom). I don't know if it would apply to the librems (since librems use a second SPI flash), but it would be interesting for me to check that later. Right now I'm still wondering how I can read/write the EC firmware if I wanted to. Good idea on the mention of SPI-only in the commit message. -- To view, visit https://review.coreboot.org/18925 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I000819aff25fbe9764f33df85f040093b82cd948 Gerrit-PatchSet: 5 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: ichspi: Add support for Intel Skylake
by Youness Alaoui (Code Review) April 27, 2017

April 27, 2017

Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/18962 ) Change subject: ichspi: Add support for Intel Skylake ...................................................................... Patch Set 6: Code-Review+1 -- To view, visit https://review.coreboot.org/18962 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0f4565a3c39f5fe3aec4fc8863605cebed1ad4ee Gerrit-PatchSet: 6 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Convert flashrom to git
by Nico Huber (Code Review) April 27, 2017

April 27, 2017

Nico Huber has posted comments on this change. ( https://review.coreboot.org/19206 ) Change subject: Convert flashrom to git ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/#/c/19206/1/util/git-hooks/pre-push File util/git-hooks/pre-push: Line 60: nonreachable=$(git rev-list $remote_sha ^$local_sha) Maybe add `| head -1` the list could be long. -- To view, visit https://review.coreboot.org/19206 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I64eef21982cac0a0a7419bcd2c8a936672ae9cb2 Gerrit-PatchSet: 1 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: Convert flashrom to git
by Nico Huber (Code Review) April 27, 2017

April 27, 2017

Nico Huber has posted comments on this change. ( https://review.coreboot.org/19206 ) Change subject: Convert flashrom to git ...................................................................... Patch Set 1: (3 comments) https://review.coreboot.org/#/c/19206/1/util/git-hooks/pre-push File util/git-hooks/pre-push: Line 41: else Why put an else here if we exited anyway? PS1, Line 63: pusing *pushing PS1, Line 57: # Make _really_ sure we do not rewrite precious history : for lbranch in "${precious_branches[@]}"; do : if [ "$remote_ref" = "refs/heads/$lbranch" ]; then : nonreachable=$(git rev-list $remote_sha ^$local_sha) : if [ -n "$nonreachable" ]; then : echo "Only fast-forward pushes are allowed on $lbranch." >&2 : echo "$nonreachable is not included in $remote_sha while pusing to $remote_ref" >&2 : exit 1 : fi : fi : done > Isn't this configurable in the upstream repo? The code looks good, I was just worried that we reinvent the wheel. There is the "receive.denyNonFastForwards" option that does the same but for all branches. OTOH, will we have anything but precious bran- ches with creation of new branches being forbidden? -- To view, visit https://review.coreboot.org/19206 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I64eef21982cac0a0a7419bcd2c8a936672ae9cb2 Gerrit-PatchSet: 1 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: trivial change to test jenkins
by build bot (Jenkins) (Code Review) April 27, 2017

April 27, 2017

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/18812 ) Change subject: trivial change to test jenkins ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/283/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/291/ : SUCCESS -- To view, visit https://review.coreboot.org/18812 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49e4cca56938b9f945de2ccd2d0a4ec6acdb0a30 Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: trivial change to test jenkins
by Patrick Georgi (Code Review) April 27, 2017

April 27, 2017

Patrick Georgi has restored this change. ( https://review.coreboot.org/18812 ) Change subject: trivial change to test jenkins ...................................................................... Restored more tests required, it seems? -- To view, visit https://review.coreboot.org/18812 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: restore Gerrit-Change-Id: I49e4cca56938b9f945de2ccd2d0a4ec6acdb0a30 Gerrit-PatchSet: 1 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins)

1 0

Change in flashrom[staging]: Simple, clumsy script to test layout handling
by build bot (Jenkins) (Code Review) April 27, 2017

April 27, 2017

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19352 ) Change subject: Simple, clumsy script to test layout handling ...................................................................... Patch Set 7: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/282/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/290/ : SUCCESS -- To view, visit https://review.coreboot.org/19352 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I37124455071bf3395384a58ad236c998f738c418 Gerrit-PatchSet: 7 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0