Attention is currently required from: Anastasia Klimchuk, Angel Pons, foss(a)volatilesystems.org.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69309?usp=email )
Change subject: flashchips: Add support for XMC XM25QH128A
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
Patchset:
PS3:
> Nikolai, may I ask you to check reg_bits in this chip definition? I checked the rest and replaced re […]
Done, reg_bits matches the datasheet. LGTM overall
--
To view, visit https://review.coreboot.org/c/flashrom/+/69309?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iced40403c6694a55fd648ea2785cdcba21712234
Gerrit-Change-Number: 69309
Gerrit-PatchSet: 3
Gerrit-Owner: foss(a)volatilesystems.org
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: foss(a)volatilesystems.org
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 21 Jun 2023 02:33:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/71872?usp=email )
Change subject: ichspi.c: Bug fix for ich_hwseq_get_flash_id
......................................................................
ichspi.c: Bug fix for ich_hwseq_get_flash_id
In ich_hwseq_get_flash_id, flash_id_to_entry would return
address of a structure present in flashchips array
corresponding to provided manufacture_id and model_id.
If this function returns NULL and if we don't return
after printing the warning using msg_pwarn, we'll be
dereferencing a NULL pointer, hence the return in that
if is provided.
Change-Id: I35c112cd032e3b94e30c347766764392d5bbfe3d
Signed-off-by: Eshan Kelkar <eshangalorithm(a)gmail.com>
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/71872
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M ichspi.c
1 file changed, 1 insertion(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Nikolai Artemiev: Looks good to me, approved
diff --git a/ichspi.c b/ichspi.c
index e9668ef..2e38ff1 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1566,6 +1566,7 @@
if (!entry) {
msg_pwarn("Unable to identify chip, mfg_id: 0x%02"PRIx32", "
"model_id: 0x%02"PRIx32"\n", mfg_id, model_id);
+ return;
}
msg_pdbg("Chip identified: %s\n", entry->name);
--
To view, visit https://review.coreboot.org/c/flashrom/+/71872?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I35c112cd032e3b94e30c347766764392d5bbfe3d
Gerrit-Change-Number: 71872
Gerrit-PatchSet: 3
Gerrit-Owner: eshankelkar(a)galorithm.com
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Edward O'Callaghan, eshankelkar(a)galorithm.com.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71872?usp=email )
Change subject: ichspi.c: Bug fix for ich_hwseq_get_flash_id
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
Eshan thank you for the patch!
--
To view, visit https://review.coreboot.org/c/flashrom/+/71872?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I35c112cd032e3b94e30c347766764392d5bbfe3d
Gerrit-Change-Number: 71872
Gerrit-PatchSet: 2
Gerrit-Owner: eshankelkar(a)galorithm.com
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: eshankelkar(a)galorithm.com
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Wed, 21 Jun 2023 02:01:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, Edward O'Callaghan, eshankelkar(a)galorithm.com.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71872?usp=email )
Change subject: ichspi.c: Bug fix for ich_hwseq_get_flash_id
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/71872?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I35c112cd032e3b94e30c347766764392d5bbfe3d
Gerrit-Change-Number: 71872
Gerrit-PatchSet: 2
Gerrit-Owner: eshankelkar(a)galorithm.com
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: eshankelkar(a)galorithm.com
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 21 Jun 2023 01:38:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/75991?usp=email )
Change subject: flashrom: only unlock for write/erase operations
......................................................................
flashrom: only unlock for write/erase operations
Don't unlock for read/verify operations because most chips don't have
read locks, and the ones that do aren't supported by flashrom's unlock
functions anyway.
Unconditionally unlocking slows down flashrom significantly with some
programmers, particularly linux_mtd due to inefficiency in the current
kernel MTD interface.
BUG=b:283779258
BRANCH=none
TEST=`ninja test`
TEST=`flashrom -{r,w,E,v}` on strongbad
TEST=`flashrom --wp-enable; flashrom -{w,E}` on strongbad
Change-Id: I5dc66474a0b7969b51b86ac9f5daa2c95ae968f1
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M flashrom.c
1 file changed, 15 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/91/75991/1
diff --git a/flashrom.c b/flashrom.c
index b6e5cf8..918bcea 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -2092,17 +2092,23 @@
/* Initialize chip_restore_fn_count before chip unlock calls. */
flash->chip_restore_fn_count = 0;
- /* Given the existence of read locks, we want to unlock for read, erase and write. */
+ /*
+ * Only unlock for erase and write. Read and verify generally do not
+ * require unlocking. Some chips have read locks, but flashrom's chip
+ * drivers don't support disabling them anyway.
+ */
int ret = 1;
- if (flash->chip->decode_range != NO_DECODE_RANGE_FUNC ||
- (flash->mst->buses_supported & BUS_PROG && flash->mst->opaque.wp_write_cfg)) {
- ret = unlock_flash_wp(flash);
- if (ret)
- msg_cerr("Failed to unlock flash status reg with wp support.\n");
+ if (write_it || erase_it) {
+ if (flash->chip->decode_range != NO_DECODE_RANGE_FUNC ||
+ (flash->mst->buses_supported & BUS_PROG && flash->mst->opaque.wp_write_cfg)) {
+ ret = unlock_flash_wp(flash);
+ if (ret)
+ msg_cerr("Failed to unlock flash status reg with wp support.\n");
+ }
+ blockprotect_func_t *bp_func = lookup_blockprotect_func_ptr(flash->chip);
+ if (ret && bp_func)
+ bp_func(flash);
}
- blockprotect_func_t *bp_func = lookup_blockprotect_func_ptr(flash->chip);
- if (ret && bp_func)
- bp_func(flash);
flash->address_high_byte = -1;
flash->in_4ba_mode = false;
--
To view, visit https://review.coreboot.org/c/flashrom/+/75991?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5dc66474a0b7969b51b86ac9f5daa2c95ae968f1
Gerrit-Change-Number: 75991
Gerrit-PatchSet: 1
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: newchange
Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, eshankelkar(a)galorithm.com.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71872?usp=email )
Change subject: ichspi.c: Bug fix for ich_hwseq_get_flash_id
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
Okay I rebased since there was a merge conflict that needed to be resolved manually.
Nikolai could you please approve again? Thank you!
--
To view, visit https://review.coreboot.org/c/flashrom/+/71872?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I35c112cd032e3b94e30c347766764392d5bbfe3d
Gerrit-Change-Number: 71872
Gerrit-PatchSet: 2
Gerrit-Owner: eshankelkar(a)galorithm.com
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: eshankelkar(a)galorithm.com
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Tue, 20 Jun 2023 09:14:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, Edward O'Callaghan, Nikolai Artemiev, eshankelkar(a)galorithm.com.
Anastasia Klimchuk has uploaded a new patch set (#2) to the change originally created by eshankelkar(a)galorithm.com. ( https://review.coreboot.org/c/flashrom/+/71872?usp=email )
The following approvals got outdated and were removed:
Code-Review+2 by Nikolai Artemiev, Verified+1 by build bot (Jenkins)
The change is no longer submittable: Code-Review and Verified are unsatisfied now.
Change subject: ichspi.c: Bug fix for ich_hwseq_get_flash_id
......................................................................
ichspi.c: Bug fix for ich_hwseq_get_flash_id
In ich_hwseq_get_flash_id, flash_id_to_entry would return
address of a structure present in flashchips array
corresponding to provided manufacture_id and model_id.
If this function returns NULL and if we don't return
after printing the warning using msg_pwarn, we'll be
dereferencing a NULL pointer, hence the return in that
if is provided.
Change-Id: I35c112cd032e3b94e30c347766764392d5bbfe3d
Signed-off-by: Eshan Kelkar <eshangalorithm(a)gmail.com>
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M ichspi.c
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/71872/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/71872?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I35c112cd032e3b94e30c347766764392d5bbfe3d
Gerrit-Change-Number: 71872
Gerrit-PatchSet: 2
Gerrit-Owner: eshankelkar(a)galorithm.com
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: eshankelkar(a)galorithm.com
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Anastasia Klimchuk, Edward O'Callaghan, eshankelkar(a)galorithm.com.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71872?usp=email )
Change subject: ichspi.c: Bug fix for ich_hwseq_get_flash_id
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Nikolai, just to check, so is this normal if `ich_hwseq_get_flash_id` doesn't write anything into `f […]
That's right, it is ok for this function to return without copying anything,
--
To view, visit https://review.coreboot.org/c/flashrom/+/71872?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I35c112cd032e3b94e30c347766764392d5bbfe3d
Gerrit-Change-Number: 71872
Gerrit-PatchSet: 1
Gerrit-Owner: eshankelkar(a)galorithm.com
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: eshankelkar(a)galorithm.com
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Tue, 20 Jun 2023 00:19:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, eshankelkar(a)galorithm.com.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71872?usp=email )
Change subject: ichspi.c: Bug fix for ich_hwseq_get_flash_id
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Nikolai, just to check, so is this normal if `ich_hwseq_get_flash_id` doesn't write anything into `flash->chip` and just returns, this is not an error?
--
To view, visit https://review.coreboot.org/c/flashrom/+/71872?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I35c112cd032e3b94e30c347766764392d5bbfe3d
Gerrit-Change-Number: 71872
Gerrit-PatchSet: 1
Gerrit-Owner: eshankelkar(a)galorithm.com
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: eshankelkar(a)galorithm.com
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Mon, 19 Jun 2023 11:35:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67878?usp=email )
Change subject: dirtyjtag: Add DirtyJTAG programmer
......................................................................
Patch Set 17:
(1 comment)
Patchset:
PS17:
> Taken care of in https://review.coreboot. […]
David, do you have more details, maybe logs or error messages that you can give? It's hard to understand the issue otherwise.
The programmer has been tested, but it usually details that matter.
If you could give details, maybe you can post on the mailing list? Or if you have account, maybe you can create issue on https://ticket.coreboot.org/projects/flashrom ? Whatever you prefer.
thank you!
--
To view, visit https://review.coreboot.org/c/flashrom/+/67878?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic43e9a014ed7d04e429e73b30c9dcfdde1a78913
Gerrit-Change-Number: 67878
Gerrit-PatchSet: 17
Gerrit-Owner: Jean THOMAS <virgule(a)jeanthomas.me>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: David Williams
Gerrit-Comment-Date: Mon, 19 Jun 2023 11:29:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: David Williams
Comment-In-Reply-To: Jean THOMAS <virgule(a)jeanthomas.me>
Gerrit-MessageType: comment