Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52473 )
Change subject: s25f.c: Fix mismatched function definitions
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/52473/comment/b755b2b4_bd022465
PS1, Line 8:
: This was missed because `uint32_t` is `unsigned int` in most cases.
: However, it is not the case for DJGPP 6.1.0 for some reason.
> Understood re the regression :< C + many platforms + bit rot under rugs in Flashrom has been a potent combo to start fires. Generally since it bisect's to Nikolai work we should get him on the hook as he works approx 50% on Flashrom at the moment. I am trying to get more folks on IRC and integrating with upstream so everyone is working together and things are moving in that direction, certainly better than a year ago for example.
Thank you.
> I could make the patch but I think you are better positioned to test it? It is very very hard at Google to have any other environments than the ones that are given, I don't know how to address this easily. I think buildbot is the way forwards here.
>
> For DJGPP it sounds like what we really need to do here is ensure this is integrated into the buildbot somehow? I think that is actually the problem here to be scalable. I'll ping Patrick to see if he has any cycles he could spare to assist in how we can do that.
One word: manibuilder. Integrating manibuilder with Jenkins would be the way to go, IMHO. However, the integration isn't trivial to do.
> For the sanity check I think you have thought about it way more deeply than I. My concern was restricted to `addr`. Is this something we can perhaps get Anastasia across who is keen to get these unit tests to a point of maturity. I wonder if there is anything in this for her?
Something interesting would be to sanity-check the flash chip entries. For instance, in nearly all cases, the block erasers should erase the whole chip. I can't manage to come up with any other ideas right now, but it's something to brainstorm on.
--
To view, visit https://review.coreboot.org/c/flashrom/+/52473
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I656a72b85d4c70b57f6ff9268186a4a60933f8a9
Gerrit-Change-Number: 52473
Gerrit-PatchSet: 3
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Tue, 27 Apr 2021 12:28:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Nicola Corna, Jack Rosenthal, Paul Menzel, Stefan Reinauer, David Hendricks, Daniel Campello, Arthur Heymans, Carl-Daniel Hailfinger.
Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/23021 )
Change subject: layout: Add -i <region>[:<file>] support
......................................................................
Patch Set 36: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/23021
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99
Gerrit-Change-Number: 23021
Gerrit-PatchSet: 36
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Jack Rosenthal <jrosenth(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nicola Corna <nicola(a)corna.info>
Gerrit-Attention: Jack Rosenthal <jrosenth(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>
Gerrit-Comment-Date: Tue, 27 Apr 2021 12:16:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Thomas Walker, Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52310 )
Change subject: flashchips: Add Spansion/Cypress S25FL256L
......................................................................
Patch Set 6:
(1 comment)
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/52310/comment/cdabb3aa_215be900
PS5, Line 16387: FEATURE_4BA
> Forgive me if I'm incorrect, but in the datasheet there is a section: 6.1.1 Extended Address […]
Yes, two ways. Not three. :)
Often there is a third way to access the space above 16MiB. The extended
address register (EAR) works like a page selection. If you leave it at 0,
you can access the first 16MiB with 24-bit address commands, if you set
it to 1, you can access the second 16MiB and so on. Or in other words,
internally the 32-bit address is composed of
<EAR> | <24-bit from command>
Maybe `FEATURE_4BA_EXT_ADDR` is a bit confusing and should be renamed?
Other vendors don't use the term "extended address" unless they refer
to this particular register.
--
To view, visit https://review.coreboot.org/c/flashrom/+/52310
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4e8cba554d0590c94dac92aa91e9ab400efca281
Gerrit-Change-Number: 52310
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Walker <thh.walker(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Walker <thh.walker(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 27 Apr 2021 12:11:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Thomas Walker <thh.walker(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Sam McNally, Nico Huber, Nicola Corna, Jack Rosenthal, Paul Menzel, Stefan Reinauer, David Hendricks, Arthur Heymans, Carl-Daniel Hailfinger.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/23021 )
Change subject: layout: Add -i <region>[:<file>] support
......................................................................
Patch Set 36:
(1 comment)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/23021/comment/1d8c0546_2607b058
PS35, Line 1493: #ifdef __LIBPAYLOAD__
> It feels a bit weird for write_buf_to_include_args() to fail eagerly for __LIBPAYLOAD__ while read_b […]
Agree! Removed!
--
To view, visit https://review.coreboot.org/c/flashrom/+/23021
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99
Gerrit-Change-Number: 23021
Gerrit-PatchSet: 36
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Jack Rosenthal <jrosenth(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nicola Corna <nicola(a)corna.info>
Gerrit-Attention: Jack Rosenthal <jrosenth(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>
Gerrit-Comment-Date: Tue, 27 Apr 2021 12:05:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sam McNally <sammc(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Nicola Corna, Jack Rosenthal, Paul Menzel, Stefan Reinauer, David Hendricks, Daniel Campello, Arthur Heymans, Carl-Daniel Hailfinger.
Daniel Campello has uploaded a new patch set (#36) to the change originally created by David Hendricks. ( https://review.coreboot.org/c/flashrom/+/23021 )
Change subject: layout: Add -i <region>[:<file>] support
......................................................................
layout: Add -i <region>[:<file>] support
Add an optional sub-parameter to the -i parameter to allow building the
image to be written from multiple files. This will also allow regions to
be read from flash and written to separate image files.
This is a rebase of a patch that was ported from chromiumos. A lot of
things have changed, but the idea is the same.
Original patch by Louis Yung-Chieh Lo <yjlou(a)chromium.org>:
Summary: Support -i partition:file feature for both read and write.
Commit: 9c7525f
Review URL: http://codereview.chromium.org/6611015
Ported version by Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at>
and Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>:
Summary: [PATCH 2/6] layout: Add -i <region>[:<file>] support.
Review URL: https://mail.coreboot.org/pipermail/flashrom/2013-October/011729.html
Change-Id: Ic5465659605d8431d931053967b40290195cfd99
Signed-off-by: David Hendricks <dhendrix(a)chromium.org>
Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at>
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
Signed-off-by: Nico Huber <nico.huber(a)secunet.com>
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
Signed-off-by: Daniel Campello <campello(a)chromium.org>
Co-Authored-by: Edward O'Callaghan <quasisec(a)google.com>
Co-Authored-by: Daniel Campello <campello(a)chromium.org>
---
M cli_classic.c
M flash.h
M flashrom.8.tmpl
M flashrom.c
M layout.c
M layout.h
6 files changed, 234 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/23021/36
--
To view, visit https://review.coreboot.org/c/flashrom/+/23021
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99
Gerrit-Change-Number: 23021
Gerrit-PatchSet: 36
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Jack Rosenthal <jrosenth(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nicola Corna <nicola(a)corna.info>
Gerrit-Attention: Jack Rosenthal <jrosenth(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>
Gerrit-MessageType: newpatchset