Nico Huber posted comments on this change.

View Change

Patch set 1:

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.

I target to have all information (excluding erase instructions) about
supported instructions in the feature bits.

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.

This is exactly what (I think) I'm doing in the next patch. e.g.
`FEATURE_4BA_READ` means the native 4BA read instruction is sup-
ported and if it's set, we just use that. It only works, though,
is not. I have to admit I'm a bit clueless about the latter,
e.g. are there chips that support an extended address register
and native 4BA instructions? If yes, or if we just want to pre-
pare for it, spi_prepare_address() will need a parameter to
override the default choice, I guess.

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).

My preferred solution is to have more feature bits. One for each sup-
ported instruction if need be. The programmer driver can choose for
itself then what to do (or to bail out).


To view, visit change 22020. To unsubscribe, visit 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 <>
Gerrit-Reviewer: David Hendricks <>
Gerrit-Reviewer: Nico Huber <>
Gerrit-Reviewer: build bot (Jenkins) <>
Gerrit-Comment-Date: Tue, 17 Oct 2017 18:17:13 +0000
Gerrit-HasComments: Yes