flashrom

Open questions after discussion :

1) Why isp port and debug port are shifted? It is unusual to shift from left to right () https://github.com/flashrom/flashrom/blob/master/mediatek_i2c_spi.c#L29
2) We need to resolve the comment from original patch https://review.coreboot.org/c/flashrom/+/61288
3) We need to get the board and test the programmer

Summarizing notes from when the driver was introduced:

• Much of the code seems shareable with mstarddc_spi, but that driver doesn't handle write protect. Suggested implementing this driver in the internal programmer and putting WP in board_enable.c and delegating to mstarddc_spi, but I don't think that's appropriate because internal targets main CPU firmware. The systems that use this driver also have accessible flash for the main CPU so we can't support both in the same driver.
• Separate I2C and SMBUs code paths are provided. SMBus doesn't seem needed anymore.

Seems like the best approach is to drop the SMBus paths on the assumption that nobody needs them anymore, and try to have this driver delegate to mstarddc for most operations.

As with several other drivers, there's also the concern that this programmer shouldn't be "standalone", but associated to the "internal" programmer in some way so that it can only be used on supported systems. It seems that the mediatek_i2c_spi programmer is heavily reliant on the firmware running on the scaler chip, so allowing this driver to be used on anything could cause serious problems. This is why the it85spi code was commented out and eventually dropped: https://review.coreboot.org/65378

That being said, there's ideas of an "I want a brick" parameter. While it is a bit of a stopgap measure (hopefully https://knowyourmeme.com/memes/that-sign-cant-stop-me-because-i-cant-read illustrates why), it's definitely better than nothing. Moreover, the "internal" programmer currently lacks a mechanism to choose which "internal" firmware to target, so this would be a long-term idea. If we can't address it along with the other issues, we can create a new bug to keep track of it without blocking this bug [why is it labelled as a feature? eh, it's no big deal].

"allow_brick" parameter already implemented few months ago, here: https://ticket.coreboot.org/issues/395

I manually verified operation of this programmer on a Google Meet Series One Desk 27 by reading, erasing and writing through it:

# flashrom -VVV -p mediatek_i2c_spi:bus=7,allow_brick=yes -c MX25L1605A/MX25L1606E/MX25L1608E -w /tmp/firmware_mod.rom 
flashrom 468b1d9c on Linux 4.19.266-14338-gf7cec04ddb14 (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).
flashrom was built with LLVM Clang 15.0.0 (/var/tmp/portage/sys-devel/llvm-15.0_pre465103_p20220825-r7/work/llvm-15.0_pre465103_p20220825/clang db1978b67431ca3462ad8935bf662c15750b8252), little endian
Command line (7 args): flashrom -VVV -p mediatek_i2c_spi:bus=7,allow_brick=yes -c MX25L1605A/MX25L1606E/MX25L1608E -w /tmp/firmware_mod.rom
Acquiring lock (timeout=180 sec)...
Opened file lock "/run/lock/firmware_utility_lock"
Lock acquired.
Initializing mediatek_i2c_spi programmer
Using I2C bus 7.
Might already be in ISP mode, ignoring
The following protocols are supported: SPI.
Probing for Macronix MX25L1605A/MX25L1606E/MX25L1608E, 2048 kB: master_map_flash_region: mapping MX25L1605A/MX25L1606E/MX25L1608E from 0x00000000ffe00000 to 0x0000000000000000
RDID returned 0xc2 0x20 0x15. compare_id: id1 0xc2, id2 0x2015
Added layout entry 00000000 - 001fffff named complete flash
Found Macronix flash chip "MX25L1605A/MX25L1606E/MX25L1608E" (2048 kB, SPI) on mediatek_i2c_spi.
Chip status register is 0x8c.
Chip status register: Status Register Write Disable (SRWD, SRP, ...) is set
Chip status register: Bit 6 is not set
Chip status register: Block Protect 3 (BP3) is not set
Chip status register: Block Protect 2 (BP2) is not 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
master_unmap_flash_region: unmapped 0x0000000000000000
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).
===
This flash part has status UNTESTED for operations: WP
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@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!
disable_power_management: Disabling power management.
master_map_flash_region: mapping MX25L1605A/MX25L1606E/MX25L1608E from 0x00000000ffe00000 to 0x0000000000000000
Some block protection in effect, disabling... 
    Need to disable the register lock first... Unsetting lock bit(s) failed.
Reading old flash chip contents... read_flash:  region (00000000..0x1fffff) is readable, reading range (00000000..0x1fffff).
done.
fix_erasers_if_needed: kept all erasers
fill_sorted_erasers: found 3 valid erasers
100000..100fff   1000 x 1 eraser 0
Erasing and writing flash chip... 0x100000-0x100fff: E Wwrite_flash:  region (0x100000..0x100fff) is writable, writing range (00000000..0x1fffff).

SUCCESS.
Verifying flash... 000000..0x1fffff verify_range: verifying  region (00000000..0x1fffff)
read_flash:  region (00000000..0x1fffff) is readable, reading range (00000000..0x1fffff).
VERIFIED.
restoring chip status (0x8c)
master_unmap_flash_region: unmapped 0x0000000000000000
restore_power_management: Re-enabling power management.
SUCCESS
Failed to exit serial debug mode (2), ignoring
restore_power_management: Re-enabling power management.