From patchwork Sun Jun 11 20:32:44 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Leonard Frumkin X-Patchwork-Id: 1795081 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=flashrom.org (client-ip=78.46.105.101; helo=coreboot.org; envelope-from=flashrom-bounces@flashrom.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=flashrom.org header.i=@flashrom.org header.a=rsa-sha256 header.s=dkim header.b=rLc4/mDL; dkim=permerror header.d=proton.me header.i=@proton.me header.a=rsa-sha1 header.s=protonmail header.b=HbpQa9LD; dkim-atps=neutral Received: from coreboot.org (coreboot.org [78.46.105.101]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4QhHNL6c8Kz20Vx for ; Thu, 15 Jun 2023 06:39:46 +1000 (AEST) Received: from authenticated-user (PRIMARY_HOSTNAME [PUBLIC_IP]) by coreboot.org (Postfix) with ESMTPA id 5A18020114; Wed, 14 Jun 2023 20:39:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=flashrom.org; s=dkim; t=1686775180; h=from:from:sender:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding:in-reply-to: references:list-id:list-owner:list-unsubscribe:list-subscribe:list-post; bh=QqNaAAZ9h6/XZhWOdXeKFWwUkMvz5pfEoxRGY+dCeH4=; b=rLc4/mDLG1BzvoxVCmGUqe1gwsgFprFjz3GOm3A7tT8HbyjNHtysBYjZGciRE3RJF/wOT+ G7o4kKI2cGzGTjvr5z0qe5Sn8XVqPC9SIwdrIS6bsisyJMvPOMGWmgeX9IkSQD4CYbbjbu Z26To2oeD3DFjmwBtMQ99yzMGAFXxGY= Received: from authenticated-user (PRIMARY_HOSTNAME [PUBLIC_IP]) by coreboot.org (Postfix) with ESMTP id 9608227550 for ; Sun, 11 Jun 2023 20:32:55 +0000 (UTC) Date: Sun, 11 Jun 2023 20:32:44 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=protonmail; t=1686515574; x=1686774774; bh=FC/v+YtLBXLahJhYpcAr2xxGdpLJq7DYRqKxO+pWp+Y=; h=Date:To:From:Subject:Message-ID:Feedback-ID:From:To:Cc:Date: Subject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector; b=HbpQa9LDGe+Ez0xqYAL26+7S9pmeW1Lrb91fu+R+nbGwsjHRSBvuxf6xfQqFVelL6 zsm5qcEPG9vr2dOA7hycE6mgBUdEjFIk7n63kx0jSU8dLxFPw5Rt5VIlJwJbECMXOO 6cRSFzdaNw0pI0u4CbCAeOv8wKxGGaDhN5USch63vHDCkePZDkp+8UpOuAtYhBesNV av1kiCHUAQ2wMFAWmzQMfzoPacP7DKU6hOCVc20nhhxqLQKHpxKnZPhfaRzHAtN0y6 1y+cPiNAeuNwP38QSRhwA2jNhzL6sLxPQ/mZN2GfpyWcUP4iwEKzT7XuegIYx9DdLo STzpO4MKJd7Qw== To: "flashrom@flashrom.org" Message-ID: Feedback-ID: 77832040:user:proton MIME-Version: 1.0 X-MailFrom: LeonardFrumkin3rd@proton.me X-Mailman-Rule-Hits: nonmember-moderation X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-flashrom.flashrom.org-0; header-match-flashrom.flashrom.org-1; header-match-flashrom.flashrom.org-2; header-match-flashrom.flashrom.org-3; header-match-flashrom.flashrom.org-4; header-match-flashrom.flashrom.org-5; header-match-flashrom.flashrom.org-6; header-match-flashrom.flashrom.org-7; header-match-flashrom.flashrom.org-8; header-match-flashrom.flashrom.org-9; header-match-flashrom.flashrom.org-10; header-match-flashrom.flashrom.org-11; header-match-flashrom.flashrom.org-12; header-match-flashrom.flashrom.org-13; header-match-flashrom.flashrom.org-14; header-match-flashrom.flashrom.org-15; header-match-flashrom.flashrom.org-16; header-match-flashrom.flashrom.org-17; header-match-flashrom.flashrom.org-18; header-match-flashrom.flashrom.org-19; header-match-flashrom.flashrom.org-20; header-match-flashrom.flashrom.org-21; header-match-flashrom.flashrom.org- 22; header-match-flashrom.flashrom.org-23; header-match-flashrom.flashrom.org-24; header-match-flashrom.flashrom.org-25; header-match-flashrom.flashrom.org-26; header-match-flashrom.flashrom.org-27; header-match-flashrom.flashrom.org-28; header-match-flashrom.flashrom.org-29; header-match-flashrom.flashrom.org-30; header-match-flashrom.flashrom.org-31; header-match-flashrom.flashrom.org-32; header-match-flashrom.flashrom.org-33; header-match-flashrom.flashrom.org-34; header-match-flashrom.flashrom.org-35; header-match-flashrom.flashrom.org-36; header-match-flashrom.flashrom.org-37; header-match-flashrom.flashrom.org-38; header-match-flashrom.flashrom.org-39; header-match-flashrom.flashrom.org-40; header-match-flashrom.flashrom.org-41; header-match-flashrom.flashrom.org-42; header-match-flashrom.flashrom.org-43; header-match-flashrom.flashrom.org-44; header-match-flashrom.flashrom.org-45; header-match-flashrom.flashrom.org-46; header-match-flashrom.flashrom.org-47; header-match-flash rom.flashrom.org-48; header-match-flashrom.flashrom.org-49; header-match-flashrom.flashrom.org-50; header-match-flashrom.flashrom.org-51; header-match-flashrom.flashrom.org-52; header-match-flashrom.flashrom.org-53; header-match-flashrom.flashrom.org-54; header-match-flashrom.flashrom.org-55; header-match-flashrom.flashrom.org-56; header-match-flashrom.flashrom.org-57; header-match-flashrom.flashrom.org-58; header-match-flashrom.flashrom.org-59; header-match-flashrom.flashrom.org-60; header-match-flashrom.flashrom.org-61; header-match-flashrom.flashrom.org-62; header-match-flashrom.flashrom.org-63; header-match-flashrom.flashrom.org-64; header-match-flashrom.flashrom.org-65; header-match-flashrom.flashrom.org-66; header-match-flashrom.flashrom.org-67; header-match-flashrom.flashrom.org-68; header-match-flashrom.flashrom.org-69; header-match-flashrom.flashrom.org-70; header-match-flashrom.flashrom.org-71; header-match-flashrom.flashrom.org-72; header-match-flashrom.flashrom.org-73; h eader-match-flashrom.flashrom.org-74; header-match-flashrom.flashrom.org-75; header-match-flashrom.flashrom.org-76; header-match-flashrom.flashrom.org-77; header-match-flashrom.flashrom.org-78; header-match-flashrom.flashrom.org-79; header-match-flashrom.flashrom.org-80; header-match-flashrom.flashrom.org-81; header-match-flashrom.flashrom.org-82; header-match-flashrom.flashrom.org-83; header-match-flashrom.flashrom.org-84; header-match-flashrom.flashrom.org-85; header-match-flashrom.flashrom.org-86; header-match-flashrom.flashrom.org-87; header-match-flashrom.flashrom.org-88; header-match-flashrom.flashrom.org-89; header-match-flashrom.flashrom.org-90; header-match-flashrom.flashrom.org-91; header-match-flashrom.flashrom.org-92; header-match-flashrom.flashrom.org-93; header-match-flashrom.flashrom.org-94; header-match-flashrom.flashrom.org-95 Message-ID-Hash: MNMRMA6SHRVVKLIAQ22WKRZK6QX77FBB X-Message-ID-Hash: MNMRMA6SHRVVKLIAQ22WKRZK6QX77FBB X-Mailman-Approved-At: Wed, 14 Jun 2023 20:39:26 +0000 X-Mailman-Version: 3.3.6b1 Precedence: list Subject: [flashrom] Some flashrom issues reported and a couple of patches List-Id: flashrom discussion and development mailing list Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Patchwork-Original-From: Leonard Frumkin via flashrom From: Leonard Frumkin Reply-To: Leonard Frumkin Authentication-Results: coreboot.org; auth=pass smtp.auth=mailman@coreboot.org smtp.mailfrom=flashrom-bounces@flashrom.org X-Spamd-Bar: / Hi! I've encountered some issues in my recent experience with flashrom and wanted to drop you a line, just on the off chance that someone on your side cares. I'm also attaching a couple of patches I've been using. I realize you have a provenance policy in place. This is a pseudonymous account. You're welcome to use this patches for any purpose, under any license/terms of your choosing, including putting your own name on the commits. If that's not good enough - hey, I tried. 1. Certain popular chips (W25Q128.V/GD25Q128, others) support a `fast_read` (0xb command), which simply adds a dummy cycle on the bus before data is returned. This command must be used for reads performed above some threshold speed (e.g. 50Mhz for W25Q128JV). This is mentioned for some in flashchips.c. But there's no support for it (perhaps there was once?). Nicer programmers (like CH347) can read at over 50Mhz (through the last time I checked, the driver had a fixed default frequency which is 15Mhz - slower but safe). It's possible that some users will configure the programmer to a higher speed and violate this constraint. It'd be nicer to use the fast_read command whenever available. Patch attached. 2. The Winbond Chips W25Q16 (same datasheet as W25Q80, 2007), W25Q16V (Datasheet 2009), W25Q16BV (2010),W25Q16JV (2019) all return the same flash ID (0x1540ef), but are different in some "advanced" ways. For one thing, the older W25Q16/W25Q16BV chips have only 2 status registers, while W25Q16JV has 3. The chips are all treated the same in `flashchips.c`. I wasted a day with a salvaged (old) chip, after I used flashrom to ID the chip and ended up looking at the wrong datasheet. The absence of the 3rd status register suggests a way to "sniff" for which chip is present. When status3 is missing, the (invalid) read_SR3 command returns 0xff, which is an invalid value. 3. With W25Q128.V, If SRL=1 in STATUS2, `flashrom --wp-status` reports that the chip is locked until a power cycle. However, the datasheet explains that one can do an OTP write to status register 2, which permanently locks the status register(s?). I had to figure this out with a salvaged chip, when flashrom suggested a power-cycle is all that is required. In fact the chip has its status registers permantently locked. a better message would be nice. 4. When using -c to specify a particular device, the device name param can be onerous for entries which span multiplechips, see for example the device name for `MX25L12833F/etc`. It'd be nice if specifying the name for any one chip covered by an entry would also be accepted. Better for self-documenting scripts too. It's also more future-proof, if it turns out some chips aren't fully compatible and need to be split into separate entries. Any script would still work, and would automatically switch to the new entry. That could be a good or bad thing. Just a thought. Thanks for your work! Flashrom is incredibly useful. Sent with [Proton Mail](https://proton.me/) secure email. From: woot Subject: [PATCH 2/2] Add FEATURE_0B_FAST_READ, and use it in spi_nbyte_read --- flashchips.c | 4 ++-- include/flash.h | 3 +++ spi25.c | 16 ++++++++++++++-- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/flashchips.c b/flashchips.c index 6a5cf495..b0688f04 100644 --- a/flashchips.c +++ b/flashchips.c @@ -17690,7 +17690,7 @@ const struct flashchip flashchips[] = { .page_size = 256, /* supports SFDP */ /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_0B_FAST_READ | FEATURE_WRSR_EXT2 | FEATURE_WRSR2 | FEATURE_WRSR3, .tested = TEST_OK_PREWB, .probe = PROBE_SPI_RDID, @@ -17888,7 +17888,7 @@ const struct flashchip flashchips[] = { .page_size = 256, /* supports SFDP */ /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_0B_FAST_READ | FEATURE_OTP, .tested = TEST_OK_PREW, .probe = PROBE_SPI_RDID, .probe_timing = TIMING_ZERO, diff --git a/include/flash.h b/include/flash.h index 0eace15d..46775745 100644 --- a/include/flash.h +++ b/include/flash.h @@ -164,6 +164,9 @@ enum write_granularity { /* Whether chip has configuration register (RDCR/WRSR_EXT2 commands) */ #define FEATURE_CFGR (1 << 25) +/* Non-4BA Fast Read (0x0b) Support, with 3 byte address followed by dummy byte */ +#define FEATURE_0B_FAST_READ (1 << 26) + #define ERASED_VALUE(flash) (((flash)->chip->feature_bits & FEATURE_ERASED_ZERO) ? 0x00 : 0xff) #define UNERASED_VALUE(flash) (((flash)->chip->feature_bits & FEATURE_ERASED_ZERO) ? 0xff : 0x00) diff --git a/spi25.c b/spi25.c index 6a6ee75d..6a358ad0 100644 --- a/spi25.c +++ b/spi25.c @@ -661,15 +661,27 @@ static int spi_nbyte_program(struct flashctx *flash, unsigned int addr, const ui int spi_nbyte_read(struct flashctx *flash, unsigned int address, uint8_t *bytes, unsigned int len) { + uint8_t cmd[1 + JEDEC_MAX_ADDR_LEN] = { JEDEC_READ, }; + int dummy_len = 0; const bool native_4ba = flash->chip->feature_bits & FEATURE_4BA_READ && spi_master_4ba(flash); - uint8_t cmd[1 + JEDEC_MAX_ADDR_LEN] = { native_4ba ? JEDEC_READ_4BA : JEDEC_READ, }; + const bool fast_read_0b = flash->chip->feature_bits & FEATURE_0B_FAST_READ; + + if (native_4ba) { // prefer 4ba if available, so we always support max flash size + cmd[0] = JEDEC_READ_4BA; + } else if (fast_read_0b) { + cmd[0] = JEDEC_READ_FAST; + dummy_len = 1; + } else { + cmd[0] = JEDEC_READ; + } const int addr_len = spi_prepare_address(flash, cmd, native_4ba, address); if (addr_len < 0) return 1; /* Send Read */ - return spi_send_command(flash, 1 + addr_len, len, cmd, bytes); + int cmd_len = 1 + addr_len + dummy_len; + return spi_send_command(flash, cmd_len, len, cmd, bytes); } /* -- 2.40.1