Conversation
henopied
left a comment
henopied
left a comment
There was a problem hiding this comment.
Looks good. Just a few suggested tweaks to improve the generated code.
| - name: VAL | ||
| description: Command Byte Enable value. A 1-bit per flash word byte value is placed in this register. | ||
| bit_offset: 0 | ||
| bit_size: 18 |
There was a problem hiding this comment.
I think we may want to further annotate this with the ECC byte and turn the data bytes into a bit field (along with updating the description to clarify that ECC is controlled here.) For the time being, we may also want to reduce this to 9 bit/64 bit mode based on our discussion as there seems to be something fishy going on wrt the SDK's usage in 128-bit mode.
| - name: DISABLE | ||
| description: Disable. | ||
| value: 1 | ||
| enum/STAT: |
There was a problem hiding this comment.
Potentially useless enum.
There was a problem hiding this comment.
This should stay, as it is a status enum where one variant is there was no interrupt.
| - name: BANK | ||
| description: Operate on an entire flash bank. | ||
| value: 5 | ||
| enum/SSERASEDIS: |
There was a problem hiding this comment.
Potentially useless enum.
| description: Command Execute Register. | ||
| fields: | ||
| - name: VAL | ||
| description: Command Execute value Initiates execution of the command specified in the CMDTYPE register. |
There was a problem hiding this comment.
Correcting typos and using the extended description from TRM: "Command Execute value. Initiates execution of the command specified in the CMDTYPE register. This register is blocked for writes after being written to 1 and prior to STATCMD.DONE being set by the flash wrapper hardware. Flash wrapper hardware clears this register after the processing of the command has completed."
| description: Override hardware generation of ECC data for program. Use data written to CMDDATAECC*. | ||
| bit_offset: 17 | ||
| bit_size: 1 | ||
| - name: SSERASEDIS |
There was a problem hiding this comment.
Missing PROGMASKDIS, ERASEMASKDIS.
| bit_offset: 0 | ||
| bit_size: 16 | ||
| enum: SECTORSIZE | ||
| - name: NUMBANKS |
There was a problem hiding this comment.
Add enum for consistency with others that specify min/max (e.g., MAINSIZE).
| access: Read | ||
| fieldset: GBLINFO2 | ||
| - name: BANKINFO0 | ||
| description: Bank Information Register 0 for Bank 0. |
There was a problem hiding this comment.
Maybe too tedious to patch, but these apply not just to bank 0.
| - name: OVERRIDE | ||
| description: Use value from MAXERSPCNTVAL field as maximum erase pulse count. | ||
| value: 1 | ||
| enum/MAXPCNTOVR: |
There was a problem hiding this comment.
Potentially useless enum.
| - name: HARDWARE | ||
| description: The interrupt or event line is in hardware mode. Hardware must clear the RIS. | ||
| value: 2 | ||
| enum/MAXERSPCNTOVR: |
There was a problem hiding this comment.
Potentially useless enum.
| fieldset/STATCMD: | ||
| description: Command Status Register. | ||
| fields: | ||
| - name: CMDDONE |
There was a problem hiding this comment.
CMD prefix is kind of redundant.
2 participants
cc @henopied