Project

General

Profile

Actions

Support #356

closed

Feature #353: Release v1.3

Add GD25Q256D from downstream

Added by Anastasia Klimchuk over 2 years ago. Updated almost 2 years ago.

Status:
Resolved
Priority:
Normal
Category:
Flash chip
Target version:
Start date:
04/28/2022
Due date:
% Done:

0%

Estimated time:
Affected versions:
Needs backport to:
Affected hardware:
Affected OS:

Description

Ref commit https://github.com/flashrom/flashrom/commit/86fc9cf7ab221bc54ef6f10252e296fc2d7a22d2
Comments added after patch had been merged. Is there some work left to do for this? Needs investigation.

Actions #1

Updated by Anastasia Klimchuk over 2 years ago

  • Related links updated (diff)
Actions #2

Updated by Edward . over 2 years ago

According to the follow up comment post-merge:

"Are we sure about this? the datasheet explicitly mentions the sequence: CS# goes low -> sending Enter 4-Byte mode command -> CS# goes high."

Nikolai can you validate this commit with a datasheet please?

Actions #3

Updated by Edward . over 2 years ago

  • Assignee set to Anastasia Klimchuk
Actions #4

Updated by Nikolai Artemiev over 2 years ago

IIUC, the comment is about whether enter 4BA command is permitted after a WREN command.

The "CS# goes low -> sending Enter 4-Byte mode command -> CS# goes high." part of the datasheet seems to just be describing the a minimal transaction for issuing an enter 4BA command, I don't think it's saying anything about whether it's permitted after a WREN or not.

We'd probably need to test, but flashrom isn't capable of that right now.

Actions #5

Updated by Nico Huber over 2 years ago

Nikolai Artemiev wrote in #note-4:

IIUC, the comment is about whether enter 4BA command is permitted after a WREN command.

Actually, if it is only permitted after WREN. IOW, if WREN is needed.

The "CS# goes low -> sending Enter 4-Byte mode command -> CS# goes high." part of the datasheet seems to just be describing the a minimal transaction for issuing an enter 4BA command, I don't think it's saying anything about whether it's permitted after a WREN or not.

Ack. I misread that. WREN would have its own CS#low/high sequence. Still if one looks at the WREN description, EN4B is not mentioned.

We'd probably need to test, but flashrom isn't capable of that right now.

You can replace FEATURE_4BA_WREN with FEATURE_4BA_ENTER, if reading >16MiB works, WREN is not needed (requires a programmer that supports 4BA modes).

Actions #6

Updated by Anastasia Klimchuk over 2 years ago

  • Assignee changed from Anastasia Klimchuk to Nikolai Artemiev
Actions #7

Updated by Felix Singer about 2 years ago

  • Tracker changed from Feature to Support
  • Target version changed from 1.3 to main
Actions #8

Updated by Felix Singer about 2 years ago

  • Status changed from New to Feedback
Actions #9

Updated by Anastasia Klimchuk about 2 years ago

  • Status changed from Feedback to New

I have a question, I read the discussion and I am not entirely sure:

Does the chip work right now, with whatever definition is currently in?
Is this a ticket about a bug, or about an improvement of a chip definition?

Actions #10

Updated by Anastasia Klimchuk about 2 years ago

  • Status changed from New to Feedback
Actions #11

Updated by Anastasia Klimchuk about 2 years ago

  • Description updated (diff)
Actions #12

Updated by Liam Flaherty almost 2 years ago

I have tested a GD25Q256D with it's current definition at head using a ft2232_spi programmer - the read, write, erase, and verify functions worked correctly.

Actions #13

Updated by Anastasia Klimchuk almost 2 years ago

  • Category changed from Release prep to Flash chip
  • Status changed from Feedback to In Progress
  • Parent task deleted (#353)

Given that chip works with its current definition, I am unlinking the ticket from 1.3.

However, the ticket stays open, since there is an open conversation in comments.

Actions #14

Updated by Nikolai Artemiev almost 2 years ago

I've tested and found WREN is not needed, so we should change the chip definition for 1.3.

Here is a patch to update the chip definition: https://review.coreboot.org/c/flashrom/+/70342

Actions #15

Updated by Nikolai Artemiev almost 2 years ago

  • Parent task set to #353
Actions #16

Updated by Anastasia Klimchuk almost 2 years ago

  • Status changed from In Progress to Resolved
Actions

Also available in: Atom PDF