Attention is currently required from: Anton Samsonov, Anton Samsonov, Nikolai Artemiev, Stefan Reinauer.
Anastasia Klimchuk has posted comments on this change by Anton Samsonov. ( https://review.coreboot.org/c/flashrom/+/85585?usp=email )
Change subject: flashchips: Add Spansion S25FS512S ......................................................................
Patch Set 2:
(3 comments)
File flashchips/spansion.c:
https://review.coreboot.org/c/flashrom/+/85585/comment/a1a766e3_4533b4b1?usp... : PS2, Line 1043: /* Note on FEATURE_4BA_ENTER: the command set only defines command 0xB7 : to enter 4BA mode, but there is no counterpart command "Exit 4BA mode", : and the code 0xE9 is assigned to another command ("Password unlock"). */ : /* Note on FEATURE_4BA_ENTER_EAR7 (not set): the "Extended address mode" bit : is stored in configuration register CR2V[7], which can only be read / written : by generic commands ("Read any register" / "Write any register"), that itself : expect a 3- or 4-byte address of register in question, which in turn depends on : the same register, that is initially set from non-volatile register CR2NV[7] : that defaults to 0, but can be programmed to 1 for starting in 32-bit mode. */
That is not a "wisdom", but merely observations of trial and error in finding proper combination. […]
You made trials and errors, and documented it: and this comment will be helping lots of other people who are reading it. This makes community stronger, I count this as wisdom! :)
My first thought about commands from datasheet: you spent time implementing Evaluate Erase Status command, and then it turns out it doesn't work as promised. I would be reluctant to spend even more on another very special command: maybe it doesn't work either?
A half of promised Blank Check During Erase functionality is already implemented in flashrom: checking whether bytes already have expected "erase value", and in this case nothing to do.
About your WIP work, I can think of two options. One is: do nothing. Second one: create a WIP patch and make a post on the mailing list about it. Both options are fine, you can do any.
File include/flashchips.h:
https://review.coreboot.org/c/flashrom/+/85585/comment/b0a325c4_35b95f51?usp... : PS2, Line 717: SPANSION_S25FS512S
If you vote for the _UL suffix, then should S25FL512 also be changed to S25FL512S_UL simultaneously, since this patch alters that macro anyway?
Yes I like this idea, sorry I missed the bigger picture. Also because S25FL512 -> S25FL512S_UL now has long ID, it can go together with other log IDs (as the first one alphabetically).
https://review.coreboot.org/c/flashrom/+/85585/comment/8a4da31a_76819d1e?usp... : PS2, Line 717: 0x02200081
03h -> ?? where it is? […]
Thank you so much! Very useful explanation for me.
-
Anastasia Klimchuk (Code Review)