Attention is currently required from: Hsuan-ting Chen, Nikolai Artemiev, Stefan Reinauer.
6 comments:
Commit Message:
Q128E and Q127C/Q128C have compatible main functions, their differences
are
Do you have a link to datasheet? Thank you!
Patch Set #1, Line 31: flashrom_tester with flashrom binary could pass with Q128E
Which specifically flashrom commands did that run? Probe/read/write/erase/write-protect ?
Patchset:
Thanks for the patch! I have some comments, see below.
File flashchips.c:
Patch Set #1, Line 6802: /* FIXME: 128E's OTP: 3072B total and doesn't support QPI */
QPI part of this might be an issue. The chip definition says FEATURE_QPI is supported, so flashrom thinks it is (comments are for humans only). I don't think FEATURE_QPI is actually handled, but it doesn't seem right to say it is supported for all 3 chips.
Maybe you need to split. You can copy existing definition, and put new one in alphabetical order. This will also fix the OTP comments diffs.
.reg_bits =
{
.srp = {STATUS1, 7, RW},
.srl = {STATUS2, 0, RW},
.bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}},
.tb = {STATUS1, 5, RW}, /* Called BP3 in datasheet, acts like TB */
.sec = {STATUS1, 6, RW}, /* Called BP4 in datasheet, acts like SEC */
.cmp = {STATUS2, 6, RW},
}
Are all these registers the same for new chip you are adding, and have you tested write-protect operations? (current chip definition marks write-protect as tested).
If you can check in datasheet that reg_bits are the same, but you haven't tested them, in the new definition keep reg_bits but don't mark write-protect as tested.
If you don't know what are reg_bits from datasheets, don't add them (or you can add them later).
Testing info is stored in `tested` field and it is an enum.
File include/flashchips.h:
Patch Set #1, Line 393: Same as GD25Q128B, GD25B128B, GD25Q127C, and GD25Q128E, can be distinguished by SFDP
With this patch, the same model id GIGADEVICE_GD25Q128 will be used for 5 different chips, used to be 4 now 5.
The macro name is not equal to any of those 5 models, but I see this is a pattern for Gigadevice :\
This patch modifies GD25Q127C, GD25Q128C, GD25Q128E, and previously there was one more entry for GD25B128B, GD25Q128B
From the macro name, seems like GD25Q128B was the first one introduced. So we should say
"Same as GD25B128B, GD25Q127C, GD25Q128C, GD25Q128E <and the rest of comment>"
To view, visit change 78348. To unsubscribe, or for help writing mail filters, visit settings.