- flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[main]: flashchips: Remove Macronix MX25U25635F from chip list
by DZ (Code Review) 26 Jan '24

26 Jan '24

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. DZ has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79633?usp=email ) Change subject: flashchips: Remove Macronix MX25U25635F from chip list ...................................................................... Patch Set 1: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/79633/comment/08fa2890_0eb4dbda : PS1, Line 10310: .wps = {SECURITY, 7, OTP}, /* This bit is set by WPSEL command */ > Deleting the chip entry isn't a good solution as flashrom should still support the older model. […] Hi, if WPSEL command is issued for MX25U25635F (older chip), it will be ignored. -- To view, visit https://review.coreboot.org/c/flashrom/+/79633?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ief3fd7641bed817066692c4abffff6d3b0df16b9 Gerrit-Change-Number: 79633 Gerrit-PatchSet: 1 Gerrit-Owner: DZ Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 26 Jan 2024 01:59:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: DZ Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: doc: Add docs how to add and update test status of flashchips
by Sergii Dmytruk (Code Review) 25 Jan '24

25 Jan '24

Attention is currently required from: Anastasia Klimchuk, Angel Pons, Nikolai Artemiev, Peter Marheine, Thomas Heijligen. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80205?usp=email ) Change subject: doc: Add docs how to add and update test status of flashchips ...................................................................... Patch Set 3: Code-Review+2 (1 comment) File doc/contrib_howtos/how_to_add_new_chip.rst: https://review.coreboot.org/c/flashrom/+/80205/comment/25178f55_91e1e623 : PS3, Line 86: abc nit: replace with `alphabetic`? -- To view, visit https://review.coreboot.org/c/flashrom/+/80205?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I8dde689f2a0430ae10d3fa68bee727c5a9d70aec Gerrit-Change-Number: 80205 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 25 Jan 2024 17:14:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: doc: Add docs how to add and update test status of flashchips
by Stefan Reinauer (Code Review) 25 Jan '24

25 Jan '24

Attention is currently required from: Anastasia Klimchuk, Angel Pons, Nikolai Artemiev, Peter Marheine, Sergii Dmytruk, Thomas Heijligen. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80205?usp=email ) Change subject: doc: Add docs how to add and update test status of flashchips ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/80205?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I8dde689f2a0430ae10d3fa68bee727c5a9d70aec Gerrit-Change-Number: 80205 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Thu, 25 Jan 2024 16:32:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: doc: Add docs how to add and update test status of flashchips
by Anastasia Klimchuk (Code Review) 25 Jan '24

25 Jan '24

Attention is currently required from: Angel Pons, Nikolai Artemiev, Peter Marheine, Sergii Dmytruk, Stefan Reinauer, Thomas Heijligen. Hello Angel Pons, Nikolai Artemiev, Peter Marheine, Sergii Dmytruk, Stefan Reinauer, Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/80205?usp=email to look at the new patch set (#3). Change subject: doc: Add docs how to add and update test status of flashchips ...................................................................... doc: Add docs how to add and update test status of flashchips For reference, link to doc about adding new chip on the old website: https://wiki.flashrom.org/Development_Guidelines#Adding/reviewing_a_new_fla… Docs on how to update test status of flashchips never existed before. Change-Id: I8dde689f2a0430ae10d3fa68bee727c5a9d70aec Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- A doc/contrib_howtos/how_to_add_new_chip.rst A doc/contrib_howtos/how_to_mark_chip_tested.rst A doc/contrib_howtos/index.rst M doc/index.rst 4 files changed, 268 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/80205/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/80205?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I8dde689f2a0430ae10d3fa68bee727c5a9d70aec Gerrit-Change-Number: 80205 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[main]: doc: Add docs how to add and test flashchips
by Anastasia Klimchuk (Code Review) 25 Jan '24

25 Jan '24

Attention is currently required from: Angel Pons, Nikolai Artemiev, Peter Marheine, Sergii Dmytruk, Stefan Reinauer, Thomas Heijligen. Hello Angel Pons, Nikolai Artemiev, Peter Marheine, Sergii Dmytruk, Stefan Reinauer, Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/80205?usp=email to look at the new patch set (#2). Change subject: doc: Add docs how to add and test flashchips ...................................................................... doc: Add docs how to add and test flashchips For reference, link to doc about adding new chip on the old website: https://wiki.flashrom.org/Development_Guidelines#Adding/reviewing_a_new_fla… Docs on how to test flashchip never existed before. Change-Id: I8dde689f2a0430ae10d3fa68bee727c5a9d70aec Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- A doc/contrib_howtos/how_to_add_new_chip.rst A doc/contrib_howtos/how_to_mark_chip_tested.rst A doc/contrib_howtos/index.rst M doc/index.rst 4 files changed, 268 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/80205/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/80205?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I8dde689f2a0430ae10d3fa68bee727c5a9d70aec Gerrit-Change-Number: 80205 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[main]: doc: Add docs how to add and test flashchips
by Anastasia Klimchuk (Code Review) 25 Jan '24

25 Jan '24

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/80205?usp=email ) Change subject: doc: Add docs how to add and test flashchips ...................................................................... doc: Add docs how to add and test flashchips Change-Id: I8dde689f2a0430ae10d3fa68bee727c5a9d70aec Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- A doc/contrib_howtos/how_to_add_new_chip.rst A doc/contrib_howtos/how_to_mark_chip_tested.rst A doc/contrib_howtos/index.rst M doc/index.rst 4 files changed, 268 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/80205/1 diff --git a/doc/contrib_howtos/how_to_add_new_chip.rst b/doc/contrib_howtos/how_to_add_new_chip.rst new file mode 100644 index 0000000..af619db --- /dev/null +++ b/doc/contrib_howtos/how_to_add_new_chip.rst @@ -0,0 +1,213 @@ +===================== +How to add a new chip +===================== + +To add a new chip definition you need to send a patch to flashrom. This can be your first patch to flashrom. However if it is +your first patch please read carefully :doc:`/dev_guide/development_guide`, and set up dev environment locally. + +The expectation is that you have tested successfully at least some of the operations on the chip (not necessarily all of them). +There is an exception: when datasheet covers more than one model of the same chip, but you only have one and tested that one. +Typically, you would add all the models covered by the datasheet you have, but only mark tested the one you tested. + +To begin with, make sure you have a link to the publicly available datasheet for the chip you want to add. Publicly +available datasheet is a requirement for adding a new chip. + +Open the datasheet, ``flashchips.c`` and ``include/flashchips.h``, and then + +Model ID and vendor ID +====================== + +Find model ID and vendor ID in the datasheet. This information often can be found in the sections like "Device identification", +"Device ID", "Manufacturer ID", "Table of ID definitions" or something similar to that. +These values come in combination with commands used for probing the chip (more on probing below). Most commonly used command for +probing SPI chips is ``0x9f`` which corresponds to ``PROBE_SPI_RDID``. + +At this point you need to double-check if exact same chip is already defined. Search for the ``.name`` and ``.vendor`` of the chip +you have in ``flashchips.c`` and if found, compare values of ``.manufacture_id`` and ``.model_id``. If all 4 values match, it +seems like the chip already defined. In this case you can try using it. Possibly, only the testing status of the chip needs to be +updated, in this case see :doc:`how_to_mark_chip_tested` for instructions. + +Vendor (manufacturer) ID +------------------------ + +Search for vendor ID in ``include/flashchips.h``, IDs are defined as macros. If it exists already, use existing macro in your +chip definition. If it does not exist (which means your chip is the first ever for this vendor), define macro for this ID, +add a comment. Look at existing IDs in ``include/flashchips.h`` as examples. + +Model ID +-------- + +Search for model ID in ``include/flashchips.h``. If there is no macro defined for it, add macro for model ID in the corresponding +section for given vendor. + +Model IDs should follow the pattern. They are all uppercase; first the manufacturer name (used for the manufacturer IDs macros +on top of each paragraph in flashchips.h), followed by an underscore and then the chipname. The latter should in general equal +the ``.name``, with dots (and other disallowed characters) replaced by underscores. Shared chip IDs typically use the macro name +that happened to be added first to flashrom (which is also probably the first one manufactured) and which usually matches +the other chips of that series in ``include/flashchips.h``. + +If you added a new model ID, use it in new chip definition, skip the rest of instructions about Model ID +and continue with the next section "Properties". + +Model ID already exists +^^^^^^^^^^^^^^^^^^^^^^^ + +It is possible that model ID already exists in the header. Then find the corresponding definition in ``flashchips.c`` and inspect +it carefully. Is it the same chip that you have or a different one? + +The question you need to figure out is: whether existing definition can work for your chip or not. You figure that out by +reading the rest of instructions and comparing the values from the datasheet with existing definition. + +Existing definition matches, but it has a different chip name +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +It is possible all the values from the datasheet match existing definition, but it has a different chip name. This is fine, +and in this case you need to add a comment to the macro ID definition which looks like ``Same as ...``. +Look at existing examples in ``include/flashchips.h`` file. +You should change the ``.name`` to reflect the additional chip model (see other chips of naming examples). +You don't need to add new definition. + +Existing definition does not match +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +It is possible that, despite of the same model ID, existing vs new chips have significant different and they +need different definitions. Judging the significance of a difference might be not trivial and might require some understanding +of flashrom behavior, however the examples of significant differences are: different sizes of erase blocks, +or different opcodes for operations (see more on operations below), or different configuration for write-protection (if this +config is defined). + +In this case you need to do both: + +* Add a comment to existing model ID macro, ``Same as ...`` +* Add new chip definition, re-use existing ID macro + +Note that if you need to add new chip definition, it is fine to copy a similar one that already exist and correct the values. + +Make sure to keep the abc ordering of chip names, and place new definition into the section with other chips by the same vendor. + +Properties +========== + +* From the datasheet, get ``.vendor``, ``.name``, ``.bustype``, ``.total_size``. +* ``.voltage`` defines the upper and lower bounds of the supply voltage of the chip. If there are multiple chip models + with different allowed voltage ranges, the `intersection <https://en.wikipedia.org/wiki/Intersection_(set_theory)>`_ + should be used and an appropriate comment added. +* ``.page_size`` is really hard. + Please read this `long explanation <https://mail.coreboot.org/pipermail/flashrom/2013-April/010817.html>`_, + or ignore it for now and set it to 256. +* 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``. +* ``.tested`` is used to indicate if the code was tested to work with real hardware, its possible values are defined + in ``include/flash.h``. Without any tests it should be set to ``TEST_UNTESTED``. + See also another doc :doc:`how_to_mark_chip_tested`. + +Operations +========== + +Each operation is defined as a enum value from a corresponding enum. + +Probe +----- + +``.probe`` indicates which function is called to fetch IDs from the chip and to compare them with the ones in +``.manufacture_id`` and ``.model_id``. For most SPI flash chips ``PROBE_SPI_RDID`` is the most commonly used if the datasheets +mentions **0x9f** as an identification/probing opcode. + +To see the full list of available probing functions, check definition of ``enum probe_func`` in ``include/flash.h``. +You may need to inspect the source code of what a probing function is doing, check the mapping ``lookup_probe_func_ptr`` and +search for the function code. + +``.probe_timing`` is only used for non-SPI chips. It indicates the delay after "enter/exit ID mode" commands in microseconds +(see ``include/flash.h`` for special values). + +Read and write +-------------- + +``.read`` and ``.write`` indicate which functions are used for reading and writing on the chip. Currently flashrom +does only support a single function each. The one that is best supported by existing programmers should be used for now, +but others should be noted in a comment if available. + +To see the full list of available functions, check definitions of ``enum read_func`` and ``enum write_func`` in ``include/flash.h``. +To inspect the source code, check the mappings ``lookup_write_func_ptr`` and ``lookup_read_func_ptr`` and search for +the function code. + +The write granularity can be expressed by the ``.gran`` field. If you think you need something else than the default +``write_gran_256bytes`` then you should definitely ask one of the regular flashrom hackers first. +Possible values can be found in ``include/flash.h``. + +Erase +----- + +``block_erasers`` stores an array of pairs of erase functions (``.block_erase``) with their respective layout (``.eraseblocks``). + +``.block_erase`` is similar to the probing function. You should at least check that the opcode named in the function name +is matching the respective opcode in the datasheet. + +To see the full list of available functions, check definition of ``enum block_erase_func`` in ``include/flash.h``. +To inspect the source code, check the mappings ``lookup_erase_func_ptr`` and search for the function code. + +Two forms of ``.eraseblocks`` can be distinguished: symmetric and asymmetric layouts. +Symmetric means that all blocks that can be erased by an opcode are sized equal. In that case a single range can define +the whole layout (e.g. ``{4 * 1024, 256}`` means 256 blocks of 4 kB each). Asymmetric layouts on the other hand contain +differently sized blocks, ordered by their base addresses (e.g. ``{{8 * 1024, 1}, {4 * 1024, 2}, {16 * 1024, 7}}`` describes +a layout that starts with a single 8 kB block, followed by two 4 kB blocks and 7 16 kB blocks at the end). + +``.eraseblocks`` should be listed in order, from the smallest to the largest size. + +Printlock +--------- + +``.printlock`` is a `misnomer to some extent <https://mail.coreboot.org/pipermail/flashrom/2011-May/006495.html>`_. + +It is used not only to print (write) protected address ranges of the chip, but also to pretty print the values +of the status register(s) - especially true for SPI chips. There are a lot of existing functions for that already +and you should reuse one if possible. Comparing the description of the status register in the datasheet of an already +supported chip with that of your chip can help to determine if you can reuse a printlock function. + +Check for definition of ``enum printlock_func`` and ``lookup_printlock_func_ptr`` for available options and source code. + +Unlock +------ + +``.unlock`` is called before flashrom wants to modify the chip's contents to disable possible write protections. +It is related to the ``.printlock`` function as it tries to change some of the bits displayed by ``.printlock``. + +Check for definition of ``enum blockprotect_func`` and ``lookup_blockprotect_func_ptr`` for available options and source code. + +Write-protection +================ + +Write-protection support is optional, and if you haven't tested it on the chip, don't add it. +If you, however, used and tested it, that would be great to add to chip definition. + +Registers bits +-------------- + +``.reg_bits`` stores information about what configuration bits the chip has and where they are found. + +For example, ``.cmp = {STATUS2, 6, RW}`` indicates that the chip has a complement bit (``CMP``) and it is the 6th bit +of the 2nd status register. See ``struct reg_bit_info`` in ``include/flash.h`` for details on each of the structure's fields. + +Note that some chips have configuration bits that function like ``TB/SEC/CMP`` but are called something else in the datasheet +(e.g. ``BP3/BP4/...``). These bits should be assigned to a field *according their function* and the datasheet name should be +noted in a comment, for example: + +:code:`.sec = {STATUS1, 6, RW}, /* Called BP4 in datasheet, acts like SEC */` + +Decode range +------------ + +``.decode_range`` points to a function that determines what protection range will be selected by particular configuration +bit values. It is required for write-protect operations on the chip. + +Check for definition of ``enum decode_range_func`` and ``lookup_decode_range_func_ptr`` for available options and source code. + +Test your changes +================= + +After making changes in the code, rebuild flashrom, run unit tests, and test the chip. + +Add testing information to commit message. + +When all of the above done, follow :doc:`/dev_guide/development_guide` to push a patch and go through review process. +Dev guide has more details on the process. diff --git a/doc/contrib_howtos/how_to_mark_chip_tested.rst b/doc/contrib_howtos/how_to_mark_chip_tested.rst new file mode 100644 index 0000000..5f02ffd --- /dev/null +++ b/doc/contrib_howtos/how_to_mark_chip_tested.rst @@ -0,0 +1,46 @@ +========================== +How to mark chip as tested +========================== + +flashrom has a massive amount of flashchips definitions, not all of them are fully tested. The reason for this is: +in some situations chip definition can be introduced based on the values that the datasheet claims, +but without testing on hardware. In this case chip definition is marked as not tested, **TEST_UNTESTED**. If later +someone uses the chip for real, and it works, the chip definition can be (and should be) updated, and operations +which run successfully should be marked as tested. + +To mark the chip as tested, someone (one of the maintainers, or one of the contributors) need to send a patch to update +the list of flashchips. It can be you, and *yes it can be your first patch to flashrom!* If this is your first patch, +make sure you read the :doc:`/dev_guide/development_guide` and set up dev environment locally. If this is not possible, you +are welcome to send test results to the mailing list, see :ref:`mailing list`. + +Instructions below assume you went through Development guide, and set up environment and repository locally. + +To begin with, make sure you have full logs from all the operations that you have run successfully and want to +mark as tested. **Providing full logs which indicate successful run is required to mark chip as tested.** + +Information about tested status of the chip is stored in ``flashchips.c``, specifically in the ``.tested`` member +of ``struct flashchip``. + +Relevant definitions and available options are in ``include/flash.h``, specifically see the definition of: + +* ``enum test_state`` +* ``struct tested`` (inside ``struct flashchip``) +* pre-defined macros for ``struct tested``, all of them follow the pattern ``#define TEST_xxx_yyy`` + +Choose the correct value depending on the operations that you have tested successfully. And then: + +#. Open ``flashchips.c`` and find the definition of your chip. +#. Check the tested status of the chip. If you tested *not* on the latest HEAD (perhaps on the latest released version, + or on flashrom built a month ago, etc), it is possible that tested status has been updated already. If the status + has not been updated, +#. Update ``.tested`` value to the one which reflects current test status. Note that not all the operations have to be + tested at once: maybe you tested only probe and read, then mark just that. +#. Make sure flashrom builds successfully, and all the unit tests pass. +#. For commit title, use the string ``flashchips: Mark <chip name> as tested for <operations>``. +#. Provide the logs in commit message, you can use flashrom paste service at `paste.flashrom.org <https://paste.flashrom.org>`_ + or any other paste service. As a plan B, you can post message on the :ref:`mailing list`, attach the logs to the post, + and then add the link to the post to the commit message. +#. Follow :doc:`/dev_guide/development_guide` and send your patch for review. +#. Go through review process until your patch gets approved (see Development guide for more details on this). + +To see the examples of such patches, have a look at commit history of ``flashchips.c``. diff --git a/doc/contrib_howtos/index.rst b/doc/contrib_howtos/index.rst new file mode 100644 index 0000000..0a4fd0e --- /dev/null +++ b/doc/contrib_howtos/index.rst @@ -0,0 +1,8 @@ +Contributors howtos +=================== + +.. toctree:: + :maxdepth: 1 + + how_to_add_new_chip + how_to_mark_chip_tested diff --git a/doc/index.rst b/doc/index.rst index 8217056..2fdf778 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -11,6 +11,7 @@ dev_guide/index user_docs/index + contrib_howtos/index classic_cli_manpage contact release_notes/index -- To view, visit https://review.coreboot.org/c/flashrom/+/80205?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I8dde689f2a0430ae10d3fa68bee727c5a9d70aec Gerrit-Change-Number: 80205 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[main]: flashrom_tester: Fix partial_lock_test on libflashrom
by Anastasia Klimchuk (Code Review) 24 Jan '24

24 Jan '24

Attention is currently required from: Edward O'Callaghan, Evan Benn, Hsuan-ting Chen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79304?usp=email ) Change subject: flashrom_tester: Fix partial_lock_test on libflashrom ...................................................................... Patch Set 4: (1 comment) File util/flashrom_tester/flashrom/src/cmd.rs: https://review.coreboot.org/c/flashrom/+/79304/comment/72322903_544b0229 : PS3, Line 301: // TODO > The flashrom CLI sets its own default flags, and we currently have no need for custom flags, so this […] Okay so just to check my understanding, the reason we need this here (even if empty) is because it is defined in `pub trait Flashrom` and now `cmd.rs` needs an implementation, even if empty? Can this no-op fn be removed from cmd.rs, or is it required? I am not sure whether we get votes from Ed and Evan, maybe not, I will try to finish review by myself, so that's why I am having questions :) -- To view, visit https://review.coreboot.org/c/flashrom/+/79304?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I7a8ac0c0984fef3cd9e73ed8d8097ddf429e54b2 Gerrit-Change-Number: 79304 Gerrit-PatchSet: 4 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)gmail.com> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 24 Jan 2024 10:08:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: Add Idaville platform into chipset enable list and add update IRC reg...
by Kevin Yu (Code Review) 24 Jan '24

24 Jan '24

Attention is currently required from: Anastasia Klimchuk, Angel Pons, David Hendricks. Kevin Yu has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79340?usp=email ) Change subject: Add Idaville platform into chipset enable list and add update IRC region support ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: > Hello Kevin! […] Hi Anastasia Sorry for late, I will do more investigation， and reply you later. KevinYu -- To view, visit https://review.coreboot.org/c/flashrom/+/79340?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Icc526b22a9efc8071e972bb1d8cfc51d6a83651b Gerrit-Change-Number: 79340 Gerrit-PatchSet: 6 Gerrit-Owner: Kevin Yu <ykevin(a)celestica.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 24 Jan 2024 01:20:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: flashchips: Remove Macronix MX25U25635F from chip list
by Nikolai Artemiev (Code Review) 24 Jan '24

24 Jan '24

Attention is currently required from: Anastasia Klimchuk, DZ, Stefan Reinauer. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79633?usp=email ) Change subject: flashchips: Remove Macronix MX25U25635F from chip list ...................................................................... Patch Set 1: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/79633/comment/4b20278f_2c9dc05b : PS1, Line 10310: .wps = {SECURITY, 7, OTP}, /* This bit is set by WPSEL command */ > Thanks for details, now I understand the situation. […] Deleting the chip entry isn't a good solution as flashrom should still support the older model. If it is possible for flashrom to distinguish the chips some how (e.g. custom opcode), then you can use a custom probe function to do that. Otherwise, if the customer builds their own version of flashrom, they can use a local blocklist to ignore certain chips with duplicated IDs. This is what we did for ChromeOS. -- To view, visit https://review.coreboot.org/c/flashrom/+/79633?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ief3fd7641bed817066692c4abffff6d3b0df16b9 Gerrit-Change-Number: 79633 Gerrit-PatchSet: 1 Gerrit-Owner: DZ Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: DZ Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 23 Jan 2024 23:34:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: DZ Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: fmap: Update major/minor version check
by Brian Norris (Code Review) 22 Jan '24

22 Jan '24

Attention is currently required from: Arthur Heymans, Edward O'Callaghan, Hsuan-ting Chen, Paul Menzel. Brian Norris has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79060?usp=email ) Change subject: fmap: Update major/minor version check ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: any chance someone could submit this for me? I don't have permissions -- To view, visit https://review.coreboot.org/c/flashrom/+/79060?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I984835579d3b257a2462906f1f5091b179891bd0 Gerrit-Change-Number: 79060 Gerrit-PatchSet: 2 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Mon, 22 Jan 2024 20:19:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit