Attention is currently required from: Angel Pons.
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54944 )
Change subject: Add support for TerribleFire TF530 SPI controller
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/54944/comment/0667ed09_e644963c
PS1, Line 8:
> I'd greatly appreciate if you could elaborate a bit about this programmer. […]
It is programming through a SPI port on the Terrible Fire TF530/534/536 68030 accelerators for the Amiga 500/2000.
Here's some more information
https://github.com/mntmn/tf530https://madexp.com/2018/12/16/tf530-amiga-accellerator/
--
To view, visit https://review.coreboot.org/c/flashrom/+/54944
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I109eec4dec89650bb80d3d1da905d33f65f60f5a
Gerrit-Change-Number: 54944
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Wed, 02 Jun 2021 22:43:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Angel Pons.
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54943 )
Change subject: Add AmigaOS suppport to flashrom
......................................................................
Patch Set 1:
(4 comments)
File Makefile:
https://review.coreboot.org/c/flashrom/+/54943/comment/56cbe169_dd346e01
PS1, Line 141: Bus Pirate, Serprog and PonyProg are not supported under AmigaOS (missing serial support).
> I'd rather not list all the programmers in the comment. Instead, I'd say something like this: […]
This code is copied verbatim from the MSDOS section. If it was good enough for MSDOS, it's good enough for Amiga OS ;)
https://review.coreboot.org/c/flashrom/+/54943/comment/e8c465b2_d348b0c0
PS1, Line 157: # Digilent SPI, Dediprog, Developerbox, USB-Blaster, PICkit2, CH341A and FT2232 are not supported under AmigaOS (missing USB support).
> Same story here: […]
ditto
https://review.coreboot.org/c/flashrom/+/54943/comment/f0663414_6b880498
PS1, Line 213: DOS
> AmigaOS
Caught me! I wish there was a better way of reusing this chunk for both MSDOS and AmigaOS
File flash.h:
https://review.coreboot.org/c/flashrom/+/54943/comment/c1e457f7_989a0623
PS1, Line 56: __AMIGA__
> Why not use `IS_AMIGA` here?
I don't know. What are the rules for using IS_XXX vs the platform dependent defines? It seems (looking at the code below) that this is used inconsistently / interchangeably in the code base?
--
To view, visit https://review.coreboot.org/c/flashrom/+/54943
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3e4a3885f88313aad019454fdbdf4be176b35330
Gerrit-Change-Number: 54943
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Wed, 02 Jun 2021 22:39:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Angel Pons.
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54943 )
Change subject: Add AmigaOS suppport to flashrom
......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/54943/comment/a99d1f46_e5a6478f
PS1, Line 8:
> I'm not sure if it's doable (licensing issues, maybe?), but it would be nice to add a target to util […]
I assume we can get by with compile testing, not runtime testing? That can be done with a cross compiler (albeit there is no prebuilt version for this)
How would I do this? Create a Dockerfile.amiga that contains a crosscompile environment? What's this anita stuff?
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/54943/comment/53553e7b_9c544b15
PS1, Line 162: *argv[]
> I guess this type is what makes AmigaOS unhappy?
Yep.
```
GETOPT(3)
int getopt_long(int argc, char * const argv[],
const char *optstring,
const struct option *longopts, int *longindex);
```
--
To view, visit https://review.coreboot.org/c/flashrom/+/54943
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3e4a3885f88313aad019454fdbdf4be176b35330
Gerrit-Change-Number: 54943
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Wed, 02 Jun 2021 22:34:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: David Bartley, Nico Huber.
Hello build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/54968
to look at the new patch set (#3).
Change subject: flashchips: add support for Winbond W25Q01JV
......................................................................
flashchips: add support for Winbond W25Q01JV
Signed-off-by: David Bartley <andareed(a)gmail.com>
Change-Id: If369409419332070c1fed96ce0e96b7cfe42c169
---
M flashchips.c
M flashchips.h
2 files changed, 45 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/68/54968/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/54968
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If369409419332070c1fed96ce0e96b7cfe42c169
Gerrit-Change-Number: 54968
Gerrit-PatchSet: 3
Gerrit-Owner: David Bartley <andareed(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: David Bartley <andareed(a)gmail.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk.
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54748 )
Change subject: dummyflasher.c: Extract params processing into a separate function
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/54748/comment/4fee50a3_11fcaa29
PS4, Line 834: return 0;
> Yes, returning 0 implying 'ok' case, and this is exactly it - not emulating any flash chip is ok cas […]
Thank you for confirming that it is a 'ok' case.
I'm good with it now.
--
To view, visit https://review.coreboot.org/c/flashrom/+/54748
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I04f55f77bb4703f1d88b2191c45a22be3c97bf87
Gerrit-Change-Number: 54748
Gerrit-PatchSet: 4
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Namyoon Woo <namyoon(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 02 Jun 2021 06:13:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Namyoon Woo <namyoon(a)google.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55107 )
Change subject: nic3com.c: Refactor singleton states into reentrant pattern
......................................................................
Patch Set 2:
(1 comment)
File nic3com.c:
https://review.coreboot.org/c/flashrom/+/55107/comment/b7256eea_5a99fd0c
PS1, Line 151: data->id = id;
> just put the above line below this line and squash into the previous patch.
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/55107
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I9834b82650cd070556cf82207796bc6bd6b31b28
Gerrit-Change-Number: 55107
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
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-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Wed, 02 Jun 2021 05:20:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/55107
to look at the new patch set (#2).
Change subject: nic3com.c: Refactor singleton states into reentrant pattern
......................................................................
nic3com.c: Refactor singleton states into reentrant pattern
Move global singleton states into a struct and store within
the par_master data field for the life-time of the driver.
This is one of the steps on the way to move par_master data
memory management behind the initialisation API, for more
context see other patches under the same topic "register_master_api".
BUG=b:185191942
TEST=builds
Change-Id: I1c3e4836760cc9f4f9a0bd4294e8d2407b150566
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
BUG=b:185191942
TEST=builds
Change-Id: I9834b82650cd070556cf82207796bc6bd6b31b28
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M nic3com.c
1 file changed, 36 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/07/55107/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/55107
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I9834b82650cd070556cf82207796bc6bd6b31b28
Gerrit-Change-Number: 55107
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
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-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Xiang W, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49255 )
Change subject: bitbang-spi.c: support clock polarity and phase
......................................................................
Patch Set 26:
(1 comment)
Patchset:
PS26:
> +Anastasia or +Angel. […]
Yes sure I can rebase if needed.
Does this include hardware testing after the rebase?
--
To view, visit https://review.coreboot.org/c/flashrom/+/49255
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I04c1dfe132d756119229b27c3cd611d1be1abc8d
Gerrit-Change-Number: 49255
Gerrit-PatchSet: 26
Gerrit-Owner: Xiang W <wxjstz(a)126.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org>
Gerrit-Attention: Xiang W <wxjstz(a)126.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Wed, 02 Jun 2021 02:47:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Namyoon Woo, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54748 )
Change subject: dummyflasher.c: Extract params processing into a separate function
......................................................................
Patch Set 4:
(1 comment)
File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/54748/comment/3d00e9ce_4f9c0323
PS4, Line 834: return 0;
> nit: I understand that it will eventually go to dummy_init_out because data->emu_chip == EMULATE_NON […]
Yes, returning 0 implying 'ok' case, and this is exactly it - not emulating any flash chip is ok case. There is also a comment /* Nothing else to do */ which means it is ok case, all done we can return.
The lines below 925-929 are handling an error case, when dummy is asked to emulate a chip, but the chip is not recognised (invalid chip).
I agree it could be done better, I also spend some time reading this code. But I would do further improvements in separate patch.
--
To view, visit https://review.coreboot.org/c/flashrom/+/54748
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I04f55f77bb4703f1d88b2191c45a22be3c97bf87
Gerrit-Change-Number: 54748
Gerrit-PatchSet: 4
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Namyoon Woo <namyoon(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Namyoon Woo <namyoon(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Wed, 02 Jun 2021 02:39:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Namyoon Woo <namyoon(a)google.com>
Gerrit-MessageType: comment