Hi folks,
people often ask me about a potential next release and what is holding
me back. Long story short, I knew about some bad things in our master
branch that make a release impossible and I was very much afraid that
there could be more that I don't know about.
I've been scrolling through the commit log from v1.1 (the last release
I was involved) to today's master. Some of the things I found are simple
like something missing for a flash chip entry. But most of it are things
I'd rather not have on a release branch. And the list turned out to be
much longer than I expected. OTOH, I didn't find issues of the worst
kind I expected.
Not sure how to go on from here. As this is a lot to do, I suggest
that we freeze the master branch for everything that is neither a fix
nor on the list (or a similar case that I missed). I noted sometimes
that we could just drop the code from a release branch. If we'd decide
to do that, we'd also need a plan how to stop the rot on the master
branch.
Below are my raw notes, I don't have the time to write things up nicely.
I noted the commit hash where things began, but tried to take the cur-
rent state into account. For instance, when it's about a new programmer
driver, I was looking at the file, not the initial patch.
Cheers,
Nico
Recommended clean-ups and investigations before v1.3
====================================================
* cb973683 Added partial meson support which confuses package main-
tainers.
Should we remove it from or disable CLI building on release
branches?
* 71b706f5 Adds some libflashrom APIs with questionable design, buggy
implementation and no visible user. Would be easiest to drop
and add back when somebody needs it.
Open comment. Partially fixed up.
* 86fc9cf7 Open comment. Partially fixed up.
* 05c629be Looks incomplete, flash would be fully erased with
programmers that lack native 4BA command support.
* ad08aef6 Undocumented programmer. disable? drop from realease branch?
* 703de983 Added `spireadmode` param to sb600spi. (How) does it work?
* 13a2ef6c Added `lspcon_i2c_spi`. Too generic name: LSPCon is a concept
but driver seems to be about a specific vendor/chip. No pro-
bing just starts to write to hardcoded addresses. WP code
contains magic numbers, board specific? Implements `spi_
master` but contains hard-coded SPI opcodes. Not documented?
disable? drop from release branch?
* d97f87b0 Added `realtek_mst_i2c_spi`. No probing, starts by toggling
a GPIO. Are GPIO settings board specific? Timeout checks seem
to have regressed since original commit. Not documented?
Changes clock settings, is this board specific? Implements
`spi_master` with special cases in an opcode LUT, doesn't
bail out on unknown codes. Falls back on unaligned reads/
writes, were these paths tested? Fixups suggest that the
original code wasn't properly tested, was it by now?
disable? drop from release branch?
* be4682dc Might miss some block erasers.
* f82dd300 Might miss some block erasers.
* a2dc4f7f It seems `clear_spi_id_cache()` is never called. Probably,
should be before probing.
* 3149822c Added variable size in `dummy_programmer`. Didn't look
at it right now, but I remember it caused a lot of trouble.
Probably not fully fixed up yet.
* cb9f3cd0 Possible division by zero.
* ca2e3bce Fixed some libpci usage. IIRC, there are many more similar
cases that might need a fix now that libpci is more strict.
* 0386aa17 sb600spi refactoring. Looks like it dropped an error message
from many error paths.
* f745d0e6, adbae0e2 Added infrastructure for S25F chips. Code style
looks a bit outdated. Probably needs upstream review.
* 8fa792fb Added chips with oddities and w/o reasoning, e.g:
.total_size = 16384, /* This is just half the size.... */
Complete patch needs review.
* 855d6b6f Added Ryzen support to sb600spi. API violations. Stalled
other cleanup work. Soft bricks machines with >16MiB flash.
Patches on Gerrit.
drop from release branch?
* 32f4cb4f Looks like copy-pasta from Winbond chips. Might miss some
block erasers. Missing 4BA_WRITE is probably wrong.
* 45d50a10 Added support code for per-region file arguments. Review was
interrupted, IIRC. Rebase was borked shortly before merge.
Work never finished.
* d4063bf3 Missed to update the manpage. Literal `-` shouldn't be in
`<>`.
* ce983bcc Missed to update the manpage. Filters only spaces when
converting region names to file names.
* 51e1d0e4 Still selects the wrong chipset, IIRC.
* e98b2d11 Review never really started. See comments on Gerrit.
Backport candidates
===================
* 4ca575dc usbdev: Only match requested USB devices
* 8bbb7648 spi95: Check for success before using send_command's returned
data
* b07c53d7 spi25: Debug flashrom crash when Write Protect is ON
Looks like this covers up a general problem with flash-chip
entries. Why would we know how to disable block protection
if we don't know how to report it? Should probably be added
to self checks.
* 51261541 it87spi.c: Prevent use-after-free bug
* da0825f0 chipset_enable.c: check return value from rphysmap() call
* 705212da chipset_enable.c: Validate physmap() return rcrb value
* b76d2810 dediprog: Fix segmentation fault on no device found
* 1a21cc70 helpers.c: Fix undefined behavior in strndup()
* a2f2f3f5 linux_mtd: Disable buffering on the mtd device
* ab623c65 jlink_spi: Reduce transfer size
* Many fixes in dummy_flasher, not sure if they were applicable earlier.
* aedfb7eb flashrom.8: add missing entry for `--flash-contents`
* 89c73b5a linux_mtd: check ioctl() return value properly
Dear fellow flashrom developers,
I've been reflecting lately about written and unwritten rules around our
review process and our related privileges on Gerrit. I couldn't identify
anything that works well, or let's say well enough. So it might be time
we discuss some things and make changes :)
Before the dawn of our Git repository and using Gerrit as a review plat-
form, we had a Subversion repository. With very few people, usually one
or two I think, that had write access. So practically we had gatekeepers
that watched over the code. Eventually, they became a bottleneck so we
widened that a little when starting the work on Gerrit. Today, I often
catch myself falling back into such a gatekeeper mode. Which is futile
anyway because I'm not the only one who can write to the repository. But
I still wonder why this happens and if it's necessary.
I guess first everyone needs to understand how we imagined things with
Gerrit and how it looks like today. As mentioned above we set up a group
"flashrom developers" which has exclusive write access to the reposi-
tory. The idea was to only have people in this group who are long-term
(I was thinking about years) contributors to flashrom, know each other
from real-time communication (IRC) and trust each other. For the review
right "+2", we used the "Reviewers" group from coreboot. IIRC, I pushed
for the latter. I was hoping to grow the communities a little together
that seemed to have gone separate ways in the years before. After all,
a lot of coreboot developers use flashrom in their everyday work.
Combined with the "flashrom developers" group it shouldn't have been
an issue that people with little flashrom knowledge would review.
Since the original setup (about 2017) things went on rather uncon-
trolled. I think the gist of it is that people didn't expect that
flashrom would need/have different rules compared to coreboot. At
some point somebody whom I only knew from their work on coreboot sud-
denly merged patches for flashrom. And I guess people pushing patches
today pretty much expect that there is nothing more to be done once
they see the first +2. No matter if the reviewer knows anything about
flashrom.
When reviewers disagree then, it can become frustrating for everyone.
So currently, I lean towards aligning our process more with that of
coreboot. The big difference seems to be that at coreboot merging
patches, i.e. hitting the "Submit" button, is more or less considered
an administrative task unrelated to one's opinion on the code. While
for us it was supposed to replace the gatekeeping. Changing this would
imply that we need our own group of reviewers. With this, a +2 would
become both rarer and stronger. If this idea meets some consent, I
would propose to copy the "flashrom developers" group for a start.
And when we identify somebody over time who shows in-depth knowledge
about at least a part of flashrom and good self-judgement if they
are experienced enough for a +2, we could add them. Right now, some
candidates come to mind already, Peter, Nikolai, and Thomas.
The "flashrom developers" group also grants the privilege to block
"-2" a change. I have very conflicting feelings about this. On one
hand, this is a tool that is only ever necessary when people in said
group can't trust each other. On the other, that seems to be the
status quo already. Maybe we should have another group on top for
this? This time really only for trusted, established, long-term
contributors? Or maybe for those who maintain the codebase beyond
what they are paid for?
There is one related idea for a rule that crossed my mind: How about
we forbid blocking reverts that fix a regression and apply cleanly
(i.e. without needing to revert anything else in advance). Also, let
them pass after review without waiting 24 hours. At least for myself
this would reduce a lot pressure to fall back to gatekeeping if I knew
I could keep things safe and sound without discussion. For maintainers,
the patches that are already merged matter the most, cause the most
work. This should also be the case for contributors, but alas many
don't see it like this and don't help to fix things after a patch is
merged. Easing reverts would push the hard work back to the people
who break things.
Looks a bit longer than what I intended to write, as usual when one
starts writing ;) If we come to a conclusion wrt. Gerrit privileges,
we could also discuss more rules for each role. While I'm not a fan
of too many rules and would rather keep things to common sense, these
things depend much on the people we add to the respective groups.
I hope it's not too much to digest at once ;)
Cheers,
Nico
Hello there,
I hope you are well today when you receive this email. I am Aarya Chaumal,
a Computer Engineering student at the College of Engineering Pune, India.
While going through organizations for this year's Google's Summer of Code I
came across your organization, Flashrom.
I am a part of onboard computers subsystem at my college's satellite
initiative. Through this, I have closely worked on Atmel SAM E70 XPLAINED
board. Also, I have strong knowledge about C/C++ and assembly language.
From your list of GSoC project ideas, I liked the idea of “Remove global
state from flashrom” and "Optimize Erase-Function Selection", although I am
not quite sure which one is more suitable for me. Can you guide me through
this?
As mentioned in your Contributor commitments and requirements, I started to
do one of the easy projects - Add new flash chip definitions. For this, I
read the relevant datasheets, one from the unlisted chips and another of a
listed one (for reference) but still, I am not getting the information
about some fields for the structure in the datasheet, namely the
feature_bits, probe_timing. Also, do I have to write the probe, read, write
and erase functions for the chip separately? Also, how do I test if my code
is working as I don't have relevant hardware with me? Can you help me with
this? Also what resources should I use to learn more about it?
Thank you for looking into this for me.
Sincerely,
Aarya Chaumal
Good day,
I am a student and very interested in flashrom. I fit the description
of experienced C programmer. Open-source and low-level programming are
my interests. I already have real experience with flashrom (CH341A
programmer + chip "MX25L6436E") and want to become a GSoC 2022
contributor in this project.
I find the "Restructure Shutdown Function" project interestring to me.
But right now it has no mentors. Maybe someone from the mailing can
help me.
I have gone through sources and understand how things are now. The
project description says "have a defined shutdown function in the
programmer API". But we already have it. It's programmer_shutdown().
Or no?
The first and easiest thing that comes to my mind is to refuse using
callbacks and use enums, thusly:
enum shutdown_func {
SERPROG = 1 << 0,
SPI = 1 << 1,
...
};
int programmer_shutdown(enum shutdown_func execfunc)
{
...
if(execfunc & SERPROG)
ret |= serprog_shutdown(...);
if(execfunc & SPI) {
/* the problem is that SPI programmers have own shutdown funcs,
like dlc5_shutdown, byteblaster_shutdown, stlinkv3_spi_shutdown,
so use struct spi_master, that contains shutdown callback */
ret |= spi_master_shutdown(...);
}
...
}
This system has disadvantages, but it'll make easier to track the code
flow. What are the potential problems in this implementation?
What's with struct flashrom_programmer? Is not implemented yet?
I would appreciate any feedback.
--
Joursoir
I'm using a Raspberry Pi Zero and SOIC clip to communicate with a
MX25L12873F chip, using flashrom built from master branch (1b1066e).
Reading the chip is working fine, no matter how many times I do a
read operation, the resulting firmware image is correct. However, I'm
unable to write to the chip:
========================================================================
flashrom -p linux_spi:dev=/dev/spidev0.0,spispeed=1000 -c \
MX25L12833F/MX25L12835F/MX25L12845E/MX25L12865E/MX25L12873F \
-w firmware.bin --ifd -i fd -i bios -i me --noverify-all -VVV -o out.log
Initializing linux_spi programmer
Using device /dev/spidev0.0
Using 1000kHz clock
get_max_kernel_buf_size: Using value from \
/sys/module/spidev/parameters/bufsiz as max buffer size.
linux_spi_init: max_kernel_buf_size: 4096
The following protocols are supported: SPI.
Probing for Macronix \
MX25L12833F/MX25L12835F/MX25L12845E/MX25L12865E/MX25L12873F, \
16384 kB: programmer_map_flash_region: mapping \
MX25L12833F/MX25L12835F/MX25L12845E/MX25L12865E/MX25L12873F from \
0xff000000 to 0x00000000
RDID returned 0xc2 0x20 0x18. compare_id: id1 0xc2, id2 0x2018
Added layout entry 00000000 - 00ffffff named complete flash
Found Macronix flash chip \
"MX25L12833F/MX25L12835F/MX25L12845E/MX25L12865E/MX25L12873F" \
(16384 kB, SPI) on linux_spi.
Chip status register is 0x7c.
Chip status register: Status Register Write Disable (SRWD, SRP, ...) \
is not set
Chip status register: Bit 6 is set
Chip status register: Block Protect 3 (BP3) is set
Chip status register: Block Protect 2 (BP2) is set
Chip status register: Block Protect 1 (BP1) is set
Chip status register: Block Protect 0 (BP0) is set
Chip status register: Write Enable Latch (WEL) is not set
Chip status register: Write In Progress (WIP/BUSY) is not set
programmer_unmap_flash_region: unmapped 0x00000000
This chip may contain one-time programmable memory. flashrom cannot read
and may never be able to write it, hence it may not be able to \
completely
clone the contents of this chip (see man page for details).
programmer_map_flash_region: mapping \
MX25L12833F/MX25L12835F/MX25L12845E/MX25L12865E/MX25L12873F from \
0xff000000 to 0x00000000
Some block protection in effect, disabling... \
Block protection could not be disabled!
Chip status register is 0x7c.
Chip status register: Status Register Write Disable (SRWD, SRP, ...) \
is not set
Chip status register: Bit 6 is set
Chip status register: Block Protect 3 (BP3) is set
Chip status register: Block Protect 2 (BP2) is set
Chip status register: Block Protect 1 (BP1) is set
Chip status register: Block Protect 0 (BP0) is set
Chip status register: Write Enable Latch (WEL) is not set
Chip status register: Write In Progress (WIP/BUSY) is not set
Reading ich descriptor... done.
layout_from_ich_descriptors():1367, returned with value -1.
Couldn't parse the descriptor!
Illegal instruction
========================================================================
On an identical system, I have been using the flashrom internal
programmer to write the same firmware image without any errors parsing
the descriptor.
Any ideas why this happens? I'm a doing something wrong?
Thanks.
Best regards,
Pete Smith
Hi all,
as discussed in the last dev meeting, we can not have one appointment
for EU, AU and US people during the time from 1. April until 30.
September because of the time change. For this time, we have decided to
split the meeting up into one meeting for EU/AU and another one for
EU/US and we will alternate between them, so that all groups have a
chance to participate.
The next meeting will be on 6. April, which will target EU/AU.
We created a poll [1] for voting. For EU/AU, we are proposing the
following dates (UTC+0):
* Monday, Wednesday, Thursday 7:00-8:00
* Monday, Wednesday, Thursday 9:15-10:15
// Felix
[1] https://terminplaner4.dfn.de/dBtjEZxKqmcm721C
Sir/ Ma'am,
I hope you are having a wonderful day,
The GSOC project idea 'Modernize the flashrom website' was present in the
ideas list a week ago but now it is not visible anymore. Please tell if you
have intentionally removed it from the ideas list or there is some bug
resulting in its removal.
Thank you.
Mohit Nehra
(An aspiring student/ contributor to flashrom GSOC 2022)
Sir,
My name is Shivesh Chaturvedi, I am a second year undergraduate student of
the department of Mechanical Engineering enrolled in its btech course at
Indian Institute of Technology, Kharagpur, West Bengal India. I am a
resident of Lucknow Uttar Pradesh, India.
I had worked on the project Idea of Modernising the UI of FlashRom website.
I had now gone through the website nearly 2-3 dozen times and I had found
some issues which I thought rectifying would help a lot of new people like
me. I had worked on basically 5 main points :-
1. Improvement of UI.
2. Having a basic Tutorial on How to use the Website.
3. Some Pages are too long so the idea is to use their pagination view.
4. Having database support for pages like Supported Devices and other pages
also will help to improve the website.
5. Having a community chat box feature will help the users very much.
Currently I am trying to use Flashrom, but finding it very difficult as
there is no proper tutorial available over the internet.
I was attempting to contact using Liber Chat, but there was some
miscommunication on the website. As a result, I'm sending a draft proposal
to this email.
Please Kindly take a look over them and do give me pointers that I am going
in the right direction or not.
Yours Sincerely
Shivesh Chaturvedi
(+91) 8840884608
Hello Team,
Hope you are doing well.
I would like bring this activity in your notice
https://review.coreboot.org/q/topic:PCH_HW_SEQ_Cleanup and seek help to
review the code.
The reason for this refactoring of HW sequencing SPI driver code are:
1. We (Chrome OS team) recently ran into a firmware update issue on
dogfooders (test device utilise by real users and share feedback) systems,
where the attempt to perform SPI flash update operation from host-cpu
(using underlying flashrom utility) is silently failing. Furthermore, we
debug the issue with help of Intel and figure out the problem is related to
multiple master is accessing the SPI flash and operation triggered by
host-cpu side using flashrom (write and erase operation) is getting timed
out (due to given lesser timeout boundary) due to underlying SPI bus is
occupied by other master.
This is the special case, starting from Tiger Lake Chrome Platform, where
we have enable the PVAP (Protected Audio Video Path) which request CSE to
perform some erase and write operation as needed. Now an attempt to perform
firmware update from host-cpu side might coincide with the CSE performing
SPI operations. The recommendation that coming out from Intel is to account
the multiple master SPI operation while we are issuing the timeout from
flashrom side. We are also expecting a doc update from Intel to share the
exact timeout calculation (although I have attempted to document that early
here https://review.coreboot.org/c/flashrom/+/62867/1/ichspi.c#35).
2. Maintain the code symmetry between coreboot and flashrom SPI hardware
sequencing operations.
From the testing side, I have tested this code on Alderlake `Brya`,
Tigerlake `volteer` and Kabylake `eve` devices, and looking for help from
community to check on other systems as well (if possible).
Furthermore, I have attempted to increase the timeout which won't impact
the flashrom performance on the older and/or existing systems.
Looking forward for your help on this request.
Thanks,
Subrata
Dear Flashrom Org,
While I was looking for a GSOC contribution that appeals to my interests and attracts my attention. I noticed your GSOC project ideas for Flashrom, and specifically, the task labeled as "Modernize the flashrom website", which I found quite interesting. Generally, I enjoy web development using CSS, HTML, JavaScript, and PHP, and I'm willing to learn new skills, work with various frameworks, and learn about other helping platforms to enhance my abilities.
However, I would like to gain more details about what this project idea requires in minute details so that I could prepare an appropriate proposal and a good plan if it is suitable to me.
Please Kindly, provide me with additional information about the requirements.
Examples of a bunch of details that I need to know:
- Who will be the mentors? (This isn't clarified for each task separately).
- What are the expected outcomes you hope to accomplish with this task?
- Do you mean specifically that you need to improve the front-end? (Is it specifically a front-end improvement?)
- Must the design be responsive?
- Are there any predetermined languages or frameworks to use?
- Could you clarify what do you mean by saying "However, we have to find out what we need and which framework supports what. Then, various frameworks need to be evaluated and we need a plan on how to migrate to the new site."
I'd appreciate hearing the required details from you.
Kind Regards
Israa Odeh.
Sent from Mail<https://go.microsoft.com/fwlink/?LinkId=550986> for Windows