flashrom

flashrom@flashrom.org

1 participants
9187 discussions

Code Reviews guidelines discussion
by Anastasia Klimchuk 22 Apr '22

22 Apr '22

Hello People, Few weeks ago at the flashrom meeting we had a discussion about code reviews and the situations that sometimes happen. We decided that it would be good to have some guidelines… and then we decided that I will start writing something. I have been thinking about it since then. I even started writing something, but I wasn’t happy at all: it felt like composing a wall of text that no one would be reading. I decided to think about: what problems are we solving? And then it got much better. I gathered the list of problems that I think are present and repeatedly happen during code reviews. All of the below is RFC and of course, if there is a problem that is missing, please tell. I would really appreciate everyone’s opinions on this! What I suggest is: after agreeing on the list of problems, compose and agree the answers to them (yes easier to say then to do :)), and then it can be like “Code Reviews FAQ” or something like that? Especially if we can encourage people to read the other point of view first. —-------------------------------------- ## Contributor point of view ## —-------------------------------------- #1 Is this comment a conversation or an action item? #2 Is this comment blocking patch from merging or non-blocking? #3 Are we waiting for anyone [else] to have a look at this patch? #4 Is my patch ready to be merged? / Why is my patch not being merged? #5 Who are the best reviewers for this patch? #6 This patch is a real emergency, what is the process for that? —----------------------------------------------------- ## Reviewer / Maintainer point of view ## —----------------------------------------------------- #1 My important comment(s) got ignored and the patch was merged regardless. #2 I have so many code reviews, and everyone wants it fast and complains reviews are too slow. #3 A patch that was merged broke something, but people don't want to revert. #4 I asked the patch owner to fix something, but they are arguing with me instead, and wasting everyone's time. #5 What this patch is doing, and why is it needed? #6 I want to review the patch, but I am busy for the next X weeks. -- Anastasia.

1 0

Unable to read “Boyamicro” SPI chip.
by Frankie LEE (HTX) 22 Apr '22

22 Apr '22

Hi, Unable to read “Boyamicro” SPI chip? Any workaround? Thanks. ============================================================================================== ack@ack:~$ flashrom -p ft2232_spi:type=2232H,port=B,divisor=4 -r firmware.bin flashrom v1.2 on Linux 5.13.0-40-generic (x86_64) flashrom is free software, get the source code at https://flashrom.org Using clock_gettime for delay loops (clk_id: 1, resolution: 1ns). Found Generic flash chip "unknown SPI chip (RDID)" (0 kB, SPI) on ft2232_spi. === This flash part has status NOT WORKING for operations: PROBE READ ERASE WRITE The test status of this chip may have been updated in the latest development version of flashrom. If you are running the latest development version, please email a report to flashrom(a)flashrom.org<mailto:flashrom@flashrom.org> if any of the above operations work correctly for you with this flash chip. Please include the flashrom log file for all operations you tested (see the man page for details), and mention which mainboard or programmer you tested in the subject line. Thanks for your help! Read is not working on this chip. Aborting.

1 0

flashrom dev meeting notes 21.04.22
by Felix Singer 21 Apr '22

21 Apr '22

# 21th April 2022, 6:00 - 7:00 am UTC+0 Attendees: Thomas, David, Edward, Nico, Anastasia, Nikolai ## Decisions Summary * Anastasia will be doing next release. She has never done release before, and needs help from more experienced people. ## Agenda * [FelixS] Redmine is ready to use * If you wonder, new registrations require an unlock by an administrator. This is to prevent spamming and bots. Maybe we can work out something better in the future. * I added additional fields to issues, so that important information is easily visible. For example affected hardware, affected OS, affected version. Please tell me if you find these useful or if you need other fields, trackers, .. * aklm: let’s start using, I will try to create an account and see how it goes * [FelixS] Created a proposal for closing GitHub issues and pull requests automatically * Closes newly created issues and pull requests and adds a comment using GitHub Actions hook. Old issues and pull requests stay untouched. * [flashrom patch](https://review.coreboot.org/c/flashrom/+/63671), based on GitHub Actions hook and [repo- lockdown](https://github.com/dessant/repo-lockdown). * My playground [https://github.com/felixsinger/awesome-repo](https://github.com/felixsinger… * [Nico] ChromeOS fork and attempted convergence * (in case witnesses of that time are present) What led to the fork? * [PatrickG] Velocity requirements: stuff needed to happen to make progress on the products using flashrom and it didn’t happen in time. Not much of a problem when it’s temporary but stuff just piled up over time. * Nico: lots of things went wrong. And it took around 8 months to converge * David: Fork happened when Carl-Daniel was the maintainer, and he was really busy. Tools were worse that time. * We had irreconcilable differences at that time * Edward: Stefan asked me to unfork flashrom. I had to work out how things were done, how to re-write parts of the system. It was just me, on my own, no other resources. I took upstream and tried to make chrome os work with upstream. Except for a few patches: mapping hack inside of sb600spi, I am trying to chase AMD to get documentation , so that I can improve it. * David: Timer stuff I was trying to deal locally. There were a bunch of things that had to be dealt locally. * David: It was uglier downstream than it should have been. Trying to minimize the number of hacks. There are things that can be tr * Edward: Where we are today: I was trying to have a team for flashrom, to spend dedicated time to work on flashrom. We had good results on that, but it takes time. Hard to allocate people. * (in case somebody knows) What is Google’s current strategy to avoid repeating mistakes of the past? * [PatrickG] Generally CrOS is willing to go upstream-first (as with coreboot) after divergence has been resolved. Depends on the upstream, so probably need to work on the questions raised by Edward et al. * [Nico] Upstream-first of a dominant vendor made it harder for others to work upstream on coreboot. It’s not what I had in mind, but this would be repeating a mistake, IMHO. * Nico: There were very good and very bad contributions during last years * Edward: how do you quantify that nothing is happening? * Nico : sb600 is bricking machines , but nothing is happening * Edward: maybe it is a visibility problem, also some things are really hard * Nico: maybe devs in google need to talk to each other? * Edward: original patch came from AMD, not from google * Nico: if it takes a long time to find a solution, maybe just revert? * Edward: we didn’t know it would take so long * David: board bringup. There is 3-4 months time before chromebooks get into users hands, and then bugs get filed. Bugs in the code which is 6 months old, but people are already reassigned to other tasks. * Edward: maybe have a parameter which turns on the behaviour, default is turned off. The problem exists, but it is not trivial. * Nico: The problem with the “upstream-first” idea: when something is not trivial, you push it upstream first, and then try to figure out how to fix it. * Edward: that was exceptional situation, it is better now * Nico: most of the issues in the list are from google. When someone looks at the code, it is clear that code it is not understood. * Edward: we opened issues about non-documented programmer, and added documentation * Nico: one of the drivers is mixing all layers of flashrom. * Nikolai: what are concerns with the code? We will try to remove them. * Edward: The datasheet was in mandarin, and lots of questions were discussed in mandarin with the vendor. Vendors were not giving documentation, despite us asking them many times. * Nico: Who is doing reviews for google? We need reviewers who know all layers of flashrom. * David: Can we simplify the process of adding a new chipset? So that it would be easier to contribute without introducing bugs? * Thomas: It would be nice to have more structure in the code repository. It is hard to understand where the code needs to go. Having a specific place for every component is a good idea * David : flash.h and flashrom.c are dumping grounds * Anastasia: Nico do you think it is possible so that we all work together normally and peacefully? If we resolve issues from the past, and also write guidelines so that new issues won’t appear and development goes normally? >> Yes it is possible. * Let's make a release! * Nico: I spent a lot of time on this already, I can’t spend even more time. Anyone else? * Nikolai: What if the vendor does not give documentation, do you think it’s better to have a driver which is not documented, or no driver at all? * David: for early chromebooks , drivers were not documented. That’s why an option was called “I want brick” * Edward: some of the changes in the chromium tree we are not sending upstream, because there is no documentation * Nikolai: Can we have a flag for situations like sb600spi ? Finer granularity than the whole driver. Feature flags? * Nico: other people work on the sb600spi code, affects everyone who works on the code. * Edward: something similar in kernel hardware quark framework. * Nico: this needs documentation, whatever is needed to understand the quark. Man pages are for the users, but there also should be documentation for developers. In what product is this chip? You can document what you know * [dhendrix] Is this the kind of finger pointing that Edward mentioned above? At this point, CrOS has proved more than happy to move upstream. The Github mirror is probably more of a concern. Perhaps upstream should consider how to get developers to choose upstream rather than the Github mirror or some other fork. * [Nico] Pointing at Google, actually. If one company forks and later wants their code upstream without proper review, something seems wrong. As if they never reflected on the problems. * [aklm] I am not sure we are on the same page on what are “mistakes of the past”. Everyone has their own idea on what the mistakes were. I can start from myself. From my observations, converging the code was considered a purely technical task, and so it was done as a technical task. However, converging the code means converging the people/the teams, converging very different workflows and work cultures. This does not seem to be thought through or ever discussed, and to me this is a mistake. * [quasisec] I am also unclear how CrOS convergence had really much of an impact on upstream considering >99% of the work was adjusting the CrOS fork to realign with upstream. Only an extremely small amount of alignment was in the form of upstream commits, e.g., missing raiden_debug and some unfortunate part of sb600spi. Is there a specific area that stands out? * [Nico] No idea if related to the original reasons for the fork, but that the convergence is stalling new releases for years stands out pretty much. * Anastasia: We currently have more GSOC project proposals than mentors (5 vs 3). If someone wants to be a mentor, it is not too late. Contact Anastasia or Felix.

1 0

[GSoC 2022] Restructure Shutdown Function
by Joursoir 14 Apr '22

14 Apr '22

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

3 15

Future of DirectHW
by Thomas Heijligen 12 Apr '22

12 Apr '22

Hi, is there someone in the flashrom community who has worked with DirectHW and knows if it's still working on supported MacOS systems? I want to figure out how to deal with it in the future of flashrom. -- Thomas

3 5

Couldn't parse the descriptor?
by Pete Smith 12 Apr '22

12 Apr '22

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

2 6

Invitation: Flashrom Dev Meeting (monthly) @ Every 2 weeks from 8am to 9am on Thursday (CEST) (flashrom@flashrom.org)
by felix.singer12＠gmail.com 12 Apr '22

12 Apr '22

You have been invited to the following event. Title: Flashrom Dev Meeting (monthly) Monthly meeting of Flashrom project developers. Meeting notes doc: https://docs.google.com/document/d/18qKvEbfPszjsJJGJhwi8kRVDUG3GZ… When: Every 2 weeks from 8am to 9am on Thursday Central European Time - Berlin Joining info: Join with Google Meet https://meet.google.com/iuf-mtri-zva?hs=224 Join by phone (AU) +61 2 9051 6791 (PIN: 304642402) More phone numbers: https://tel.meet/iuf-mtri-zva?pin=9271993818880&hs=0 Calendar: flashrom(a)flashrom.org Who: * aklm(a)chromium.org- creator * felix.singer12(a)gmail.com * david.hendricks(a)gmail.com * quasisec(a)google.com * nico.h(a)gmx.de * Martin Roth * src(a)posteo.de * martonmiklosqdev(a)gmail.com * piotr.krol(a)3mdeb.com * Maciej Pijanowski * michal.zygowski(a)3mdeb.com * th3fanbus(a)gmail.com * stefan.k.reinauer(a)gmail.com * nartemiev(a)google.com * arthur(a)aheymans.xyz * flashrom(a)flashrom.org Event details: https://calendar.google.com/calendar/event?action=VIEW&eid=MmVscGY2cjNsdTZw… Invitation from Google Calendar: https://calendar.google.com/calendar/ You are receiving this courtesy email at the account flashrom(a)flashrom.org because you are an attendee of this event. To stop receiving future updates for this event, decline this event. Alternatively, you can sign up for a Google Account at https://calendar.google.com/calendar/ and control your notification settings for your entire calendar. Forwarding this invitation could allow any recipient to send a response to the organiser and be added to the guest list, invite others regardless of their own invitation status or to modify your RSVP. Learn more at https://support.google.com/calendar/answer/37135#forwarding

1 0

Board Testing - ASRock TRX40 Creator
by Dario Dorando 09 Apr '22

09 Apr '22

Hey folks, I've tested compatibility of this tool with my mainboard today. Boardname: ASRock TRX40 Creator Vendor website: http://www.asrock.com/mb/AMD/TRX40%20Creator/#Specification Flash-chip: Winbond 25Q128FWSQ - 16MBSuperI/O: nuvoTon NCT6683D-T For the folks interested - A webpage with amazingly detailed photos of that mainboard, including chips and whatnot: https://news.xfastest.com/review/review-03/72941/asrock-trx40-creator-revie… Note that this page is currently down for me, but you can still access it via the wayback machine. Commands ran: sudo flashrom/flashrom -r biosdump.bin -p internal # Note that TRX4CTR1.70 is the rom file that's freely downloadable from ASRock's website. binwalk -W -i TRX4CTR1.70 biosdump.bin > binwalk_diff.txt # Since I'm not really fond of firmware file layouts yet, somebody who's smarter than me might want to verify whether # the differences are mainly due to different bios configs and hardware I've added into my system,# or if some firmware section got omitted on read. more binwalk_diff.txt | grep 0x0 | wc -l # This command showed '17430'. Since binwalk shows 16 bytes per line, there should be about 34kb of differences in binary. # Flashed back the official rom, since I don't really trust the read one. sudo flashrom/flashrom -p internal -w TRX4CTR1.70 # The output from that is in the attached file. sudo flashrom/flashrom -p internal -V lspci -nnvvxxxsuperiotool -deV Flashing the machine with the stock rom did work as well! - Dario

1 0

EU/AU flashrom dev meeting appointment time change
by Felix Singer 08 Apr '22

08 Apr '22

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

2 3

flashrom dev meeting notes 7.04.22
by Felix Singer 08 Apr '22

08 Apr '22

# 7th April 2022, 6:00 - 7:00 am UTC+0 Attendees: Martin, Michal, Felix, Anastasia, Nikolai, David, Thomas, Nico, Arthur, Edward ## Decisions Summary * Write a script to email out a summary every weekend. Martin can work on a script. * Gerrit guidelines document. Anastasia will start something. * Start using the redmine bug tracker. There are still issues with it, but we’re going to start using it in its current state and work on it as we go. Felix will work on this. ## Agenda * [FelixS] Hackathons need a reboot. So I’m thinking about doing one. Please share :) * [Poll and some more details](https://terminplaner4.dfn.de/pBooK8FBDlM2zDwa) * Darmstadt, Germany. 10. - 12 June * [Edward] Not every developer has the resources to spend their whole time in IRC and have a spiritual connection with a select few. * How to ensure each code review pass results in a contributor that knows where they stand? - what's next, what's done, when things are going to be merged. * How to avoid walking on eggshells and guessing how to avoid arguments on every contribution but at the same time make tangible progress? * Looks like this topic is closely related to the next one, let's discuss them together? (All notes are done below). * [Edward] How to avoid constant politics in every contribution and instead find a path to allow contributors to not feel ostracized for daring to make a change even when the change may not be useful to others in the immediate sense. * Nikolai: hard to understand whether people are looking at the change, do they have thoughts. Hard to get to know the current status of patch? How to coordinate things? There are many channels? * Nikolai: hard to know whether it is ok. If there is no response, does it mean everything is okay, or no one had a look? * Edward: If people send patches, they need to know what the next step is? * David: gerrit plugin which pings reviewers every 48h ? we need some automation * Edward: observation. Both sides expecting action, or both sides are not sure whose turn? If something is +2 , can it be merged? How to understand that something is ready to merge? * Nikolai: how to ensure there is a consensus? People who are unfamiliar with the process don’t know how to proceed and then give up. * Silicon vendors are unfamiliar with the process. Can they officially support new chips? They attempt to send a patch, and then they are confused on what are next steps. Sometimes good ideas come up in code reviews, and it is not clear who should do that. * David: If adding a new chip requires some refactoring, maybe someone can take that work and do a refactoring? * Edward: We can create a bug in this situation, and prioritize the bug, so that refactor idea is not dropped. * Martin: We need guidelines, maybe “gerrit guidelines”. Coreboot has [https://doc.coreboot.org/contributing/gerrit_guidelines.html](https://doc.c… * Review comments should make clear what is next step * [Edward] How to make the project more accommodating to silicon partners so we can start getting direct support from manufacturers? * Often non-native speakers, * Often not well practiced in free software workflows, * Often need help and guidance to bring them into the fold. * [Edward] How to avoid commits sitting around for years in some weird state between reviewed and unmerged. * Improve project velocity while maintaining stability (improved testing, capture mistakes in tests) * Nico: Can we cherry-pick from coreboot guidelines? Only the bits that apply? * Nico: we can say “write to the mailing list if you don’t know what to do and patch needs attention” * Felix: Can we share the same guidelines as coreboot? We share processes and concepts * Edward: Let's start with our own. Copy-paste and cherry pick model. * Edward: at the end of week an email with the list “everything that has +2”. Martin: We can write it in bash. * [David] We have a GSOC student who wants to do some of the easy projects. Are those projects supposed to be saved for new contributors to have something easy to do? * [Anastasia] The easy projects page needs to be updated, but the projects do need to be done. Since they haven’t been done in 5 years, let’s let the students do them. * [Felix] Employees at companies aren’t interested in doing the easy projects. * [Edward] Let’s adopt the bug tracker and mark bugs as “easy” for people to work on. * [Anastasia] Students should do a single easy project, then move on to a single full project. * [Edward] How to get the mailing list into a usable medium and the code review tools into being for code review only. Move noise to a bug tracker. * Capture ideas in bug tracker. Ideas come up in code reviews, good ideas that need to be done “in a separate patch” and then this is lost. * Felix: some issues left to be configured in the redmine instance. * Nico: we can start using bug tracker with email feature disabled, and then see how it goes. Setup email feature can be the first ticket? * Let’s start using it? * [Edward] How to move to a culture of searched responsibility over constant finger pointing? * Some kind of policy for breakage. * Understanding that software breaks, it is impossible to catch everything in review. * Culture around writing tests and documentation. * Edward: revert policy? Anastasia: There is a thread on mailing list, revert policy is one of the topics

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

flashrom