[S] Change in flashrom[main]: how_to_add_new_chip: Add a section for feature bits and WRSR handling

[Anastasia Klimchuk (Code Review)]

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen, Thomas Heijligen.

Anastasia Klimchuk has posted comments on this change by Hsuan-ting Chen. ( https://review.coreboot.org/c/flashrom/+/82908?usp=email )

Change subject: how_to_add_new_chip: Add a section for feature bits and WRSR handling
......................................................................

Patch Set 1:

(5 comments)

File doc/contrib_howtos/how_to_add_new_chip.rst:

https://review.coreboot.org/c/flashrom/+/82908/comment/bfe99749_cb6f2201?usp... :
PS1, Line 105: Feature Bits
             : -------------
This can be on the same level as "Properties", especially because you have future plans to gradually upgrade it. Which means, change the level of the heading, use `=` instead of `-`, and when you generate the doc check that Feature Bits heading on the same level as Properties as Operations.

https://review.coreboot.org/c/flashrom/+/82908/comment/37d107af_3b345cad?usp... :
PS1, Line 108: The ``.feature_bits`` field in a chip definition encodes various features using a bitmask.
             : Here are some of the selected feature bits that need to be highlighted:
With this, we don't need an item in a list inside Properties. Remove the item from Properties list, and merge the description from there and here, for example like this:
> We encode various features of flash chips in a bitmask named ``.feature_bits``.

Available options can be found in ``include/flash.h``, look for macros defined by the pattern ``#define FEATURE_XXX``.
Some of the feature bits have more detailed docs, see below.

https://review.coreboot.org/c/flashrom/+/82908/comment/d65ef3de_86ef620f?usp... :
PS1, Line 111: Write-Status-Register (WRSR) Handling
             : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This header also goes on level up, use `-`

https://review.coreboot.org/c/flashrom/+/82908/comment/46a3e8b5_fb4220c5?usp... :
PS1, Line 113: 
             : ``FEATURE_WRSR_EWSR``, ``FEATURE_WRSR_WREN``, and ``FEATURE_WRSR_EITHER``. These bits used for **SPI only**.
I think this sentence is not needed. Just to check, is Write-Status-Register used for non-spi?

https://review.coreboot.org/c/flashrom/+/82908/comment/3a563c49_0e203384?usp... :
PS1, Line 110: 
             : Write-Status-Register (WRSR) Handling
             : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
             : 
             : ``FEATURE_WRSR_EWSR``, ``FEATURE_WRSR_WREN``, and ``FEATURE_WRSR_EITHER``. These bits used for **SPI only**.
             : 
             : The Write Status Register (WRSR) is used to configure various settings within the flash chip, including write protection and
             : other features. The way WRSR is accessed varies between SPI flash chips, leading to the need for these feature bits.
             : 
             : * ``FEATURE_WRSR_EWSR``:
             :   This indicates that we need an **Enable-Write-Status-Register** (EWSR) instruction which opens the status register for the
             :   immediately-followed next WRSR instruction. Usually, the opcode is **0x50**.
             : 
             : * ``FEATURE_WRSR_WREN``:
             :   This indicates that we need an **Write-Enable** (WREN) instruction to set the Write Enable Latch (WEL) bit. The WEL bit
             :   must be set priort to every WRSR command. Usually, the opcode is **0x06**.
             : 
             : * ``FEATURE_WRSR_EITHER``:
             :   This indicates that either EWSR or WREN is supported in this chip.
> Hi Anastasia: […]

Thank you for explaining! It's really good that I know your future plans.
The plan is good. I added few more comments, with your plan in mind.

I would say, if a feature bit can be explain in 4 words, it can be inline comment in the flash.h. But in some cases, it's much better to have a more detailed few lines docs, and then it can go here.