Alan Green has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33997 )
Change subject: flashchips.h: merge definitions from Chromium fork ......................................................................
Patch Set 6:
(4 comments)
https://review.coreboot.org/c/flashrom/+/33997/5/flashchips.h File flashchips.h:
https://review.coreboot.org/c/flashrom/+/33997/5/flashchips.h@204 PS5, Line 204:
Minor: One tab too much?
Done
https://review.coreboot.org/c/flashrom/+/33997/5/flashchips.h@949 PS5, Line 949: W25Q128
I think this also applies to W25Q64 and friends
I'm unfamiliar with these parts. If you'd like me to update the wording to also include W25Q64, please let me know. Otherwise, I will have some time to look up datasheets next week.
https://review.coreboot.org/c/flashrom/+/33997/5/flashchips.h@951 PS5, Line 951: */
This comment doesn't make much sense. All W25Q...W are 1.8V, […]
Do you have a suggestion for alternate wording?
For context, this wording was introduced in downstream change 0715328 - https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/+...
https://review.coreboot.org/c/flashrom/+/33997/5/flashchips.c File flashchips.c:
https://review.coreboot.org/c/flashrom/+/33997/5/flashchips.c@5892 PS5, Line 5892: CD
Out of curiosity, any specific reason for this rename?
The downstream names were more specific and therefore seemed more helpful.
Originally, the part was added with the name GIGADEVICE_GD25LQ128C:
SHA:62cd8106f30a1232464e6818f8736db76c64446e aka https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/+...
Later, this was updated to GIGADEVICE_GD25LQ128CD:
SHA:6c957d745f5d3dcadd1035734a5cf1b804bd0f2f aka https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/+...
I'm happy to retain the upstream name (GIGADEVICE_GD25LQ128) if you'd prefer it.