David Hendricks has posted comments on this change. ( https://review.coreboot.org/21367 )
Change subject: Move get_layout() from flashrom.c to layout.c
......................................................................
Patch Set 6:
trivial rebase
--
To view, visit https://review.coreboot.org/21367
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic67cf53abddc0aa905674acbcde717d9aed2f66e
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 6
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Philippe Mathieu-Daudé <f4bug(a)amsat.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Tue, 17 Oct 2017 06:49:57 +0000
Gerrit-HasComments: No
David Hendricks has posted comments on this change. ( https://review.coreboot.org/22020 )
Change subject: spi25: Add native 4BA support
......................................................................
Patch Set 1: -Code-Review
(1 comment)
The code here looks fine, I just want to check my understanding of how you're deciding whether to use 4BA native instructions or extended address mode. As you're probably already aware, some chips support one but not the other, so we need to be a little careful.
>From what I've seen, it's safer to use native 4BA instructions when they're available since they should work no matter what mode the chip is in.
Also, we should consider how we might override the default behavior in case the programmer can only operate in a particular mode (I'm thinking Intel chipsets and Dediprog programmers).
https://review.coreboot.org/#/c/22020/1/spi25.c
File spi25.c:
https://review.coreboot.org/#/c/22020/1/spi25.c@363
PS1, Line 363: flash->address_high_byte = addr_high;
The follow-up patch seems to use native 4BA instructions for read and write regardless of what you do here.
--
To view, visit https://review.coreboot.org/22020
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: I644600beaab9a571b97b67f7516abe571d3460c1
Gerrit-Change-Number: 22020
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Tue, 17 Oct 2017 03:40:42 +0000
Gerrit-HasComments: Yes
David Hendricks has posted comments on this change. ( https://review.coreboot.org/22021 )
Change subject: spi25: Enable direct 4BA read and write using feature bits
......................................................................
Patch Set 1: Code-Review-1
(2 comments)
https://review.coreboot.org/#/c/22021/1/spi25.c
File spi25.c:
https://review.coreboot.org/#/c/22021/1/spi25.c@582
PS1, Line 582: JEDEC_BYTE_PROGRAM_4BA
So this will use a native 4BA instruction whether or not you use the extended address register. Maybe I'm missing something?
https://review.coreboot.org/#/c/22021/1/spi25.c@592
PS1, Line 592: JEDEC_READ_4BA
same comment as above
--
To view, visit https://review.coreboot.org/22021
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f6817ca198bf923671a7aa67e956e5477d71848
Gerrit-Change-Number: 22021
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Tue, 17 Oct 2017 03:26:07 +0000
Gerrit-HasComments: Yes
David Hendricks has posted comments on this change. ( https://review.coreboot.org/22018 )
Change subject: spi25: Introduce spi_simple_write_cmd()
......................................................................
Patch Set 1: Code-Review-1
(1 comment)
Just a little bikeshedding, otherwise LGTM.
https://review.coreboot.org/#/c/22018/1/spi25.c
File spi25.c:
https://review.coreboot.org/#/c/22018/1/spi25.c@326
PS1, Line 326: spi_simple_write_cmd
Is there anything other than chip erase commands that this applies to? I think this should be called "spi_erase_cmd" or something more specific to its purpose since it doesn't handle data or addresses.
--
To view, visit https://review.coreboot.org/22018
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib244356fa471e15863b52e6037899d19113cb4a9
Gerrit-Change-Number: 22018
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 16 Oct 2017 22:19:38 +0000
Gerrit-HasComments: Yes