Attention is currently required from: Felix Singer, Alex Badea.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73037 )
Change subject: asm106x: add programmer for ASM106x SATA controllers
......................................................................
Patch Set 3:
(7 comments)
Patchset:
PS3:
Hi Alex, thank you for the contribution!
The code looks already quite good. Anyway I have a questions and some minor annotations.
On which device have you tested this driver? Can you mention it in the commit message.
File asm106x.c:
https://review.coreboot.org/c/flashrom/+/73037/comment/ba48e766_d6e120a3
PS3, Line 35: {PCI_VENDOR_ID_ASMEDIA, 0x0612, OK, "ASMedia", "ASM106x"},
This PCI-Id is assigned to the ASM1062 Serial ATA Controller,
the PCI-Id 0x625 is assigned to '106x SATA/RAID Controller'. Beside that there is a PCI-Id 0x0611 for 'ASM1061 SATA IDE Controller'. For which one is this driver?
Or does it work on all of them?
PCI-Ids from pciutils (https://pci-ids.ucw.cz/v2.2/pci.ids)
https://review.coreboot.org/c/flashrom/+/73037/comment/9db02c32_d99bd84a
PS3, Line 51: programmer_delay(flash, 10);
`programmer_delay()` should be used outside of the programmer. It is there to determine if an programmer specific delay function should be called or the default one.
Here you can use `void default_delay(unsigned int usec)`.
With it you can also pass only the `struct pci_dev *pci` to this function.
```
static int asm106x_wait_ready(const struct pci_dev *pci, uint8_t *pval)
```
https://review.coreboot.org/c/flashrom/+/73037/comment/b1ebaf4f_82ac445b
PS3, Line 59: if (pval)
You could move this 'if, assignment, return' where the break is. Then everything below the loop is a timeout.
https://review.coreboot.org/c/flashrom/+/73037/comment/5522f531_3ac722ae
PS3, Line 74: writecnt, readcnt);
This can be moved in the line above.
https://review.coreboot.org/c/flashrom/+/73037/comment/6156e402_58c4eb14
PS3, Line 115: ctrl | ASM106X_CTRL_CSN);
This function call can be in one line
File meson.build:
https://review.coreboot.org/c/flashrom/+/73037/comment/7143d1a9_afdb5109
PS3, Line 152: 'cpu_families' : cpus_port_io,
This line should be removed. The driver uses only PCI register to communicate with the flash, not I/O ports.
--
To view, visit https://review.coreboot.org/c/flashrom/+/73037
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I591b117be911bdb8249247c20530c1cf70f6e70d
Gerrit-Change-Number: 73037
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Badea <vamposdecampos(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Alex Badea <vamposdecampos(a)gmail.com>
Gerrit-Comment-Date: Tue, 14 Feb 2023 10:20:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Thomas Heijligen, Alex Badea.
Hello Felix Singer, build bot (Jenkins), Thomas Heijligen,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/73037
to look at the new patch set (#3).
Change subject: asm106x: add programmer for ASM106x SATA controllers
......................................................................
asm106x: add programmer for ASM106x SATA controllers
The ASMedia ASM106x series is a PCIe-SATA controller chip. It supports
an attached SPI flash chip that can contain configuration and PCI option
ROM. The interface is a simple shifter accessed via PCI config space, up
to 4 bytes at a time. Add a programmer driver for it.
Change-Id: I591b117be911bdb8249247c20530c1cf70f6e70d
Signed-off-by: Alex Badea <vamposdecampos(a)gmail.com>
---
M Makefile
A asm106x.c
M flashrom.8.tmpl
M include/programmer.h
M meson.build
M meson_options.txt
M programmer_table.c
M test_build.sh
8 files changed, 212 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/73037/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/73037
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I591b117be911bdb8249247c20530c1cf70f6e70d
Gerrit-Change-Number: 73037
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Badea <vamposdecampos(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Alex Badea <vamposdecampos(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Thomas Heijligen.
Hello Felix Singer, Thomas Heijligen,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/73037
to look at the new patch set (#2).
Change subject: asm106x: add programmer for ASM106x SATA controllers
......................................................................
asm106x: add programmer for ASM106x SATA controllers
The ASMedia ASM106x series is a PCIe-SATA controller chip. It supports
an attached SPI flash chip that can contain configuration and PCI option
ROM. The interface is a simple shifter accessed via PCI config space,
up to 4 bytes at a time. Add a programmer driver for it.
Change-Id: I591b117be911bdb8249247c20530c1cf70f6e70d
Signed-off-by: Alex Badea <vamposdecampos(a)gmail.com>
---
M Makefile
A asm106x.c
M flashrom.8.tmpl
M include/programmer.h
M meson.build
M programmer_table.c
M test_build.sh
7 files changed, 206 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/73037/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/73037
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I591b117be911bdb8249247c20530c1cf70f6e70d
Gerrit-Change-Number: 73037
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Badea <vamposdecampos(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Thomas Heijligen, Nicholas Chin.
ChrisEric1 CECL has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72057 )
Change subject: Add support for VIA VL805 USB 3 XHCI flashing
......................................................................
Patch Set 11:
(4 comments)
File vl805.c:
https://review.coreboot.org/c/flashrom/+/72057/comment/bd9695ac_8b26fb87
PS3, Line 38: static struct pci_dev *dev = NULL;
> The point I'm trying to make is that the `struct pci_dev *dev` should NOT be a global variable. […]
Here is my broken progress: https://gist.githubusercontent.com/CE1CECL/23ce4bab29bb0382da29ad48c049145c…
dev is defined at the top because vl805_setregval, and vl805_getregval need it, and those are needed before vl805_getregval. So, I don't know, I think I'm missing something.
File vl805.c:
https://review.coreboot.org/c/flashrom/+/72057/comment/a4f41350_58416956
PS10, Line 103: static const struct spi_master spi_master_vl805 = {
> Add `. […]
Done
https://review.coreboot.org/c/flashrom/+/72057/comment/b2dfaa79_7e567b67
PS10, Line 120: static int vl805_shutdown(void *data)
> This has to move above `struct spi_master`
Done
https://review.coreboot.org/c/flashrom/+/72057/comment/ad3afa97_f86ae719
PS10, Line 157: register_shutdown(vl805_shutdown, NULL);
> The `register_shutdown()` can be removed. […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/72057
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38
Gerrit-Change-Number: 72057
Gerrit-PatchSet: 11
Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Mon, 13 Feb 2023 23:49:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: comment
Attention is currently required from: ChrisEric1 CECL, Nicholas Chin.
Hello build bot (Jenkins), Thomas Heijligen,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/72057
to look at the new patch set (#11).
Change subject: Add support for VIA VL805 USB 3 XHCI flashing
......................................................................
Add support for VIA VL805 USB 3 XHCI flashing
It works fine for me on my Raspberry Pi 4 Model B. Was able read read,
erase, write, verify. Only thing is the WP with the W25X10 gives a
untested warning.
Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38
Signed-off-by: Christopher Lentocha <christopherericlentocha(a)gmail.com>
---
M Makefile
M flashrom.8.tmpl
M include/programmer.h
M meson.build
M meson_options.txt
M programmer_table.c
A vl805.c
7 files changed, 216 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/57/72057/11
--
To view, visit https://review.coreboot.org/c/flashrom/+/72057
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38
Gerrit-Change-Number: 72057
Gerrit-PatchSet: 11
Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: ChrisEric1 CECL, Nicholas Chin.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72057 )
Change subject: Add support for VIA VL805 USB XHCI flashing
......................................................................
Patch Set 10:
(4 comments)
File vl805.c:
https://review.coreboot.org/c/flashrom/+/72057/comment/1d31a212_6432a968
PS3, Line 38: static struct pci_dev *dev = NULL;
> Still lost on what you are trying to say, did you mean flashctx instead of flashxtx? […]
The point I'm trying to make is that the `struct pci_dev *dev` should NOT be a global variable. It should be contained in a data structure, allocated in `vl85_init()` , used as second parameter in `register_spi_master()` and passed along to `vl805_spi_send_command` via `flash->mst->data`.
You may want to look into other programmer calling `register_spi_master()` with the second parameter being not `NULL`.
File vl805.c:
https://review.coreboot.org/c/flashrom/+/72057/comment/aff042aa_469774a5
PS10, Line 103: static const struct spi_master spi_master_vl805 = {
Add `.shutdown = vl805_shutdown,`
https://review.coreboot.org/c/flashrom/+/72057/comment/57a6bee0_e9802e09
PS10, Line 120: static int vl805_shutdown(void *data)
This has to move above `struct spi_master`
https://review.coreboot.org/c/flashrom/+/72057/comment/191ab0ba_73ed70bd
PS10, Line 157: register_shutdown(vl805_shutdown, NULL);
The `register_shutdown()` can be removed. It is handled as part of the `struct spi_master`
--
To view, visit https://review.coreboot.org/c/flashrom/+/72057
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38
Gerrit-Change-Number: 72057
Gerrit-PatchSet: 10
Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Mon, 13 Feb 2023 20:41:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Nicholas Chin.
ChrisEric1 CECL has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72057 )
Change subject: Add support for VIA VL805 USB XHCI flashing
......................................................................
Patch Set 10:
(1 comment)
Patchset:
PS10:
Here is the log for with RDID4 and without RDID4 in case you ask later, note, this is one the latest Patchset (10) when writing, and I changed the file and recompiled in another terminal: http://gist.githubusercontent.com/CE1CECL/0d5b1110e95f4a48eca2f823a2d7ca7c/…
--
To view, visit https://review.coreboot.org/c/flashrom/+/72057
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38
Gerrit-Change-Number: 72057
Gerrit-PatchSet: 10
Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Mon, 13 Feb 2023 20:27:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Nicholas Chin.
ChrisEric1 CECL has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72058 )
Change subject: Add support for AMD Ryzen flashing
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
Also if you don't apply this patch, on this PC, it looks like it starts to work, but it just hangs. This fixes the hang for this PC.
--
To view, visit https://review.coreboot.org/c/flashrom/+/72058
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ife51f7dec31b51a7416e417112b0eedb21fae6a0
Gerrit-Change-Number: 72058
Gerrit-PatchSet: 5
Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Mon, 13 Feb 2023 17:58:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Nicholas Chin.
ChrisEric1 CECL has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72058 )
Change subject: Add support for AMD Ryzen flashing
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
flashrom on Linux 6.0.0-2-amd64 (x86_64)
flashrom is free software, get the source code at https://flashrom.org
Using clock_gettime for delay loops (clk_id: 1, resolution: 1ns).
flashrom was built with GCC 12.2.0, little endian
Command line (6 args): /usr/local/sbin/flashrom --verbose --progress --programmer internal --write 08433F20.bin
Initializing internal programmer
/sys/class/mtd/mtd0 does not exist
No coreboot table found.
Using Internal DMI decoder.
DMI string chassis-type: "Desktop"
DMI string system-manufacturer: "HP"
DMI string system-product-name: " "
DMI string system-version: " "
DMI string baseboard-manufacturer: "HP"
DMI string baseboard-product-name: "8433"
DMI string baseboard-version: "11"
W836xx enter config mode worked or we were already in config mode. W836xx leave config mode had no effect.
Active config mode, unknown reg 0x20 ID: 89.
Found chipset "AMD FP4/FP5/AM4" with PCI ID 1022:790e.
Enabling flash write... SPI base address is at 0xfec10000
Ryzen detected.
SpiRomEnable=1, RouteTpm2Sp=0, PrefetchEnSPIFromIMC=1, PrefetchEnSPIFromHost=1
(0x6f0c2105) SpiArbEnable=1, IllegalAccess=0, SpiAccessMacRomEn=0, SpiHostAccessRomEn=0, ArbWaitCount=7, SpiBusy=0
ERROR: State of SpiAccessMacRomEn or SpiHostAccessRomEn prohibits full access.
ROM strap override is not active
PROBLEMS, continuing anyway
The following protocols are supported: LPC, FWH.
Probing for AMIC A49LF040A, 512 kB: probe_jedec_common: id1 0xf8, id2 0x0a, id1 is normal flash content, id2 is normal flash content
...
Probing for Winbond W49V002A, 256 kB: probe_jedec_common: id1 0xff, id2 0xff, id1 parity violation, id1 is normal flash content, id2 is normal flash content
Probing for Winbond W49V002FA, 256 kB: probe_jedec_common: id1 0xff, id2 0xff, id1 parity violation, id1 is normal flash content, id2 is normal flash content
No EEPROM/flash device found.
Note: flashrom can never write if the flash chip isn't found automatically.
-----
Here is the log for latest BIOS on this HP when you run flashrom.
--
To view, visit https://review.coreboot.org/c/flashrom/+/72058
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ife51f7dec31b51a7416e417112b0eedb21fae6a0
Gerrit-Change-Number: 72058
Gerrit-PatchSet: 5
Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Mon, 13 Feb 2023 17:55:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment