Attention is currently required from: ChrisEric1 CECL.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72057 )
Change subject: Add support for VIA VL805 USB controller flashing
......................................................................
Patch Set 23:
(1 comment)
Patchset:
PS23:
> I found the problem with your chip. The code fails when readcnt != 4 […]
I've dug much deeper into this programmer and here is an update:
A lot of the names are misleading.
* Only `0x43`, `0x50`, `0x78` and `0x7c` are real PCI registers.
* All `VL805_REG_*` defines are commands send via the `0x78` register.
* The values for the command are passed via the `0x7c` register.
The values calculated and written as SPI_TRANSACTION value are for the most cases not correct. Only with 4 bytes in and out it is successful. That's the reason why it works with a `RDID4` patched W25x10 flash and SFDP compatible chips.
Random findings:
* The CMD_SPI_CS=0 mussed be send after CMD_SPI_TRANSACTION
```
REG_MCU 0x43 - Reserved register
REG_FWV 0x50 - Firmware Version (This is the only documented one)
REG_CMD 0x78 - XHCI Optional Bits Configuration Address (19:0)
REG_DAT 0x7c - XHCI Optional Bits Configuration Data (31:0)
CMD_MAGIC_30004 0x00030004 - 0x200, unknown magic value
CMD_SPOP_POLLING 0x0004000c - (1|0), currently not sure what's for
CMD_PCI_WB_EN 0x00040020 - 0xffffff01 - not a clue what this is exactly for
CMD_SPI_OUTDATA 0x000400d0 - <spi commands> + <content>
CMD_SPI_INDATA 0x000400e0 - <content>
CMD_SPI_TRANSACTION 0x000400f0 - 0x5__ Here lies the problem!
CMD_CLK_DIVIDER 0x000400f8 - Default can be used, need external measurement
CMD_SPI_CS 0x000400fc - (1|0) voltage on SPI CS Pin, needs confirmation
```
So for now the right values for `CMD_SPI_TRANSACTION` is the missing mystery that needs to be solved.
HELP NEEDED:
If someone has a Raspberry Pi 4, I'm very interested on a strace of their vl805, closed source, update tool https://github.com/raspberrypi/rpi-eeprom
It would be really helpful to get a `strace -x vl805 -d` log of an read, write and verify operation.
--
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: 23
Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
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-Comment-Date: Fri, 03 Mar 2023 14:41:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Stefan Reinauer.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73359 )
Change subject: [WIP] doc: Add build instructions
......................................................................
Patch Set 1:
(5 comments)
File doc/building_from_source.rst:
https://review.coreboot.org/c/flashrom/+/73359/comment/e51260ef_adf4cc06
PS1, Line 32: build
If this is a builddir (I think it is), then let's name it builddir.
The reason is that `build` looks like a command, can be confusing. I would prefer the name which indicates a directory.
And I just discovered, it is called builddir below! :)
https://review.coreboot.org/c/flashrom/+/73359/comment/5413952f_c8b2355a
PS1, Line 174: ross
ross -> cross (typo)
https://review.coreboot.org/c/flashrom/+/73359/comment/9bee4446_2cd5bdca
PS1, Line 192: If you are not happy with the initial configuration, e.g. a programmer is missing,
: or you want to review it
If you want to change your initial configuration for some reason (for example you discovered that a programmer is missing), run::
(In your initial text, I am not sure what did you mean by "you want to review it"? the command changes the config, not just prints it?)
https://review.coreboot.org/c/flashrom/+/73359/comment/cd39e1dd_42a25cc3
PS1, Line 206: To get a code coverage report
For coverage report: let's keep the existing instructions, please! They more detailed, written recently so they are up-to-date. I don't want to lose that piece of wisdom.
File doc/conf.py:
https://review.coreboot.org/c/flashrom/+/73359/comment/9d543edc_6b58441a
PS1, Line 22: 'sphinx.ext.todo'
This is a cool feature! :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/73359
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I96771e98b313a6d26dd2be940ff37998d4124324
Gerrit-Change-Number: 73359
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Fri, 03 Mar 2023 09:03:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Alexander Goncharov.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66343 )
Change subject: cli_classic: refactor arguments parsing into separate func
......................................................................
Patch Set 7: Code-Review+1
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/66343/comment/7d0da03a_beacc1a3
PS7, Line 21: TEST=builds
I would be good to test the patch. Dummyflasher is perfectly fine for the purpose. But unit tests are all running libflashrom, so this layer of cli code is actually not run by tests.
Not necessarily all the paths (and you can't do all the paths with dummyflasher anyway), but even some of the paths, the basic ones would be really good!
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/66343/comment/b2310053_af211dc9
PS7, Line 51: unsigned int wp_start, wp_len;
I think, grouping by purpose would be better than grouping by data type.
For example, wp has a bunch of options, they would be good to keep together (with new line before and after group).
Another group can be about operations that flashrom performs (options between read_it and show_progress).
Then maybe layout group.
Stuff that does not belong anywhere can be at the end in "everything else" group.
My main intention is to make it easier to find the option in the long list.
--
To view, visit https://review.coreboot.org/c/flashrom/+/66343
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id573bc74e3bb46b7dc42f1452fff6394d4f091bc
Gerrit-Change-Number: 66343
Gerrit-PatchSet: 7
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Fri, 03 Mar 2023 08:20:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Alexander Goncharov.
Elyes Haouas has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73394 )
Change subject: meson: fix typo "documtation" -> "documentation"
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/73394
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ia4205b89fd0d7e77ecbcd29392187d8911dd1049
Gerrit-Change-Number: 73394
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Fri, 03 Mar 2023 05:11:19 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Paul Menzel, Edward O'Callaghan, Alexander Goncharov.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73361 )
Change subject: meson: revert to meson version 0.53.0
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/73361/comment/338698f2_2a9ef97d
PS1, Line 10: Pass arguments to `sphinx-doc` via `'-Dkey=value`
> Is the new message better?
yes!
--
To view, visit https://review.coreboot.org/c/flashrom/+/73361
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iff9b8307c741a247a652cf666935c9485fa493fa
Gerrit-Change-Number: 73361
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Fri, 03 Mar 2023 02:36:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Alexander Goncharov.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73394 )
Change subject: meson: fix typo "documtation" -> "documentation"
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/73394
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ia4205b89fd0d7e77ecbcd29392187d8911dd1049
Gerrit-Change-Number: 73394
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Fri, 03 Mar 2023 01:21:50 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Edward O'Callaghan, Anastasia Klimchuk, Alexander Goncharov.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73361 )
Change subject: meson: revert to meson version 0.53.0
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/73361/comment/44d0dc91_a0c8a512
PS1, Line 10: Pass arguments to `sphinx-doc` via `'-Dkey=value`
> Yes, I understand it better now, thanks for the explanation! I think it would be very good to add th […]
Is the new message better?
--
To view, visit https://review.coreboot.org/c/flashrom/+/73361
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iff9b8307c741a247a652cf666935c9485fa493fa
Gerrit-Change-Number: 73361
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Thu, 02 Mar 2023 20:24:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment