Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54989 )
Change subject: bitbang_spi.c: Rename usages of bitbang_spi_master into master
......................................................................
bitbang_spi.c: Rename usages of bitbang_spi_master into master
There were two different things in this file, but both of them were
called "mst" and it was confusing.
Now the variables of type bitbang_spi_master are called "master",
including the member of bitbang_spi_master_data. The variables of
type spi_master are called mst.
BUG=b:185191942
TEST=builds
Change-Id: I2fdbdc2daedde7f6996642cfbb2d34ec06a89621
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/54989
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
---
M bitbang_spi.c
1 file changed, 3 insertions(+), 3 deletions(-)
Approvals:
build bot (Jenkins): Verified
Edward O'Callaghan: Looks good to me, approved
diff --git a/bitbang_spi.c b/bitbang_spi.c
index bb89071..85c867e 100644
--- a/bitbang_spi.c
+++ b/bitbang_spi.c
@@ -95,7 +95,7 @@
}
struct bitbang_spi_master_data {
- const struct bitbang_spi_master *mst;
+ const struct bitbang_spi_master *master;
};
static int bitbang_spi_send_command(const struct flashctx *flash,
@@ -105,7 +105,7 @@
{
unsigned int i;
const struct bitbang_spi_master_data *data = flash->mst->spi.data;
- const struct bitbang_spi_master *master = data->mst;
+ const struct bitbang_spi_master *master = data->master;
/* FIXME: Run bitbang_spi_request_bus here or in programmer init?
* Requesting and releasing the SPI bus is handled in here to allow the
@@ -160,7 +160,7 @@
}
struct bitbang_spi_master_data *data = calloc(1, sizeof(struct bitbang_spi_master_data));
- data->mst = master;
+ data->master = master;
mst.data = data;
register_spi_master(&mst, NULL);
register_shutdown(bitbang_spi_shutdown, data);
--
To view, visit https://review.coreboot.org/c/flashrom/+/54989
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2fdbdc2daedde7f6996642cfbb2d34ec06a89621
Gerrit-Change-Number: 54989
Gerrit-PatchSet: 3
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-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Attention is currently required from: Stefan Reinauer, Paul Menzel.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54943 )
Change subject: Add AmigaOS suppport to flashrom
......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/54943/comment/6135aced_0a26ca71
PS1, Line 8:
> 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)
Yeah, manibuilder only does build testing. A cross compiler would work. The DJGPP target is already using a cross-compiler.
> How would I do this? Create a Dockerfile.amiga that contains a crosscompile environment?
I have no idea how Docker works. I would ask Nico for advice.
> What's this anita stuff?
http://www.gson.org/netbsd/anita/
File Makefile:
https://review.coreboot.org/c/flashrom/+/54943/comment/d5bfc693_03c8edcc
PS1, Line 141: Bus Pirate, Serprog and PonyProg are not supported under AmigaOS (missing serial support).
> This code is copied verbatim from the MSDOS section. […]
I'm actually bothered by the comment, not the code ;)
(or should I say Make-foo instead?)
File flash.h:
https://review.coreboot.org/c/flashrom/+/54943/comment/920eedf8_f6c89262
PS1, Line 56: __AMIGA__
> I don't know. […]
I'm not aware of any rule, it's probably "organic" code growth. It just felt odd that every introduced guard uses `IS_AMIGA`, except for this one.
--
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: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Thu, 03 Jun 2021 02:20:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
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: Paul Menzel, 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:
(7 comments)
Patchset:
PS1:
Thanks for the review!
File tf530_spi.c:
https://review.coreboot.org/c/flashrom/+/54944/comment/4a524533_56212800
PS1, Line 4: * Copyright (C) 2019 Steven J Leary
> This differs from the commit author data.
Steven did the original port and published it on his github.
https://review.coreboot.org/c/flashrom/+/54944/comment/044e3d14_3b06c2cd
PS1, Line 20: #if CONFIG_TF530_SPI == 1
> Is this really necessary? I know other files have these guards, but I don't think they're actually u […]
It is modeled after the LINUX_SPI driver. I did not try to create an improvement over the other drivers but rather keep the consistency with what's there. Is this not recommended?
https://review.coreboot.org/c/flashrom/+/54944/comment/b9ed6081_6ed885c2
PS1, Line 62: static struct ConfigDev *cd = NULL;
> There's an ongoing effort to remove global state from programmers. See changes to `digilent_spi. […]
I'll have a look at this again. I noticed, but since this programmer is on the CPU board itself, there is no chance that this code will ever be called reentrantly.
This is also a global (system wide) handle, e.g. the same value for all tasks in the system.
https://review.coreboot.org/c/flashrom/+/54944/comment/57bb84c4_30602279
PS1, Line 149:
: struct Library *ExpansionBase = NULL;
:
: if ((ExpansionBase = (struct Library *)
: OpenLibrary((unsigned char *)"expansion.library", 0L)) == NULL) {
> Moving the assignment out of the if-block's condition should be easier to read: […]
Yeah they're necessary. :( I'll reorg.
https://review.coreboot.org/c/flashrom/+/54944/comment/13d2311c_e24544ce
PS1, Line 154: 0
> Why return 0?
Good catch
https://review.coreboot.org/c/flashrom/+/54944/comment/4dd773ac_47f8ae7a
PS1, Line 169: register_spi_master(&spi_master_tf53x, NULL);
> This function returns something. Why not propagate the return value? […]
Ack
--
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Wed, 02 Jun 2021 22:51:18 +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: 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