[prev in list] [next in list] [prev in thread] [next in thread] 

List:       flashrom
Subject:    [flashrom] Re: Release preparations
From:       Rosen Penev <rosenp () gmail ! com>
Date:       2022-03-13 7:58:24
Message-ID: CAKxU2N9kZ8g+N5ZO2axVNbjJuCr-zQp3FdjeTPYpHw5h3yti7A () mail ! gmail ! com
[Download RAW message or body]

On Sun, Mar 6, 2022 at 3:10 PM Nico Huber <nico.h@gmx.de> wrote:
>
> 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.
I have these applied to the OpenWrt tree:
https://github.com/flashrom/flashrom/pulls/neheb . I recently rebased.
>
> 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
> _______________________________________________
> flashrom mailing list -- flashrom@flashrom.org
> To unsubscribe send an email to flashrom-leave@flashrom.org
_______________________________________________
flashrom mailing list -- flashrom@flashrom.org
To unsubscribe send an email to flashrom-leave@flashrom.org
[prev in list] [next in list] [prev in thread] [next in thread]