Project

General

Profile

Actions

Bug #365

closed

Feature #353: Release v1.3

Variable size in dummy programmer

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

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

100%

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

Description

Feature initially introduced in the patch 44879 (see links). Some issues were found after that, see last comment on the patch. As a follow up 46536 was done. However some issues seem to still be present, specifically

1) Broke building with CONFIG_DUMMY=no
2) probe_variable_size() would have to move to a chip driver. It needs access to programmer internals, though. Maybe it should be an opaque master?
3) Casts objects of all programmers to the dummy one. Only bails out by chance.

Actions #1

Updated by Anastasia Klimchuk almost 2 years ago

  • Assignee set to Anastasia Klimchuk
Actions #2

Updated by Anastasia Klimchuk almost 2 years ago

Adding some more info here.

"Variable size" means that dummy needs to emulates some chip (exact properties don't matter), the only thing that matter is size. So it like "emulate just some generic chip with size S".

Also for testing, commands to run for testing are:

$ flashrom -p dummy:image=${TMP_FILE},size=, \
emulate=VARIABLE_SIZE -w ${IMG_32MB} -V -f

where size can be 8388608, 16777216, 33554432 (that's 3 different test cases)

After running command line, we need to compare files are the same
diff ${TMP_FILE} ${IMG_32MB}

Actions #3

Updated by Anastasia Klimchuk almost 2 years ago

One more thing I just realised, probe_variable_size is modifying flashctx.
However, spi infrastructure, specifically spi_send_command is taking const flashctx.

Actions #4

Updated by Anastasia Klimchuk almost 2 years ago

In general , the "variable size" feature is only used via dummmy programmer and only for emulating. So IMO it should live in dummyflasher.c
However it seems to be wired into spi infra in a wrong way.
I am trying some ideas on how to wire it differently.

Actions #5

Updated by Anastasia Klimchuk almost 2 years ago

I tried several hacks to wire probe_variable_size via spi infra and couldn't make it work.
So I am gradually coming to a conclusion this needs to be wired via opaque infra (that was a good idea mentioned in bug description!)

Actions #6

Updated by Felix Singer almost 2 years ago

Cleanup or bug tracker fit better, I think.

Actions #7

Updated by Anastasia Klimchuk almost 2 years ago

  • Tracker changed from Feature to Bug
  • Status changed from New to Closed

Changed to Bug and marked Closed since https://review.coreboot.org/c/flashrom/+/64488 is merged now.

Actions #8

Updated by Anastasia Klimchuk almost 2 years ago

  • Status changed from Closed to Resolved

Closed was a wrong status, actually I should say Resolved. Corrected status to Resolved.

Actions #9

Updated by Nico Huber almost 2 years ago

  • Status changed from Resolved to Closed

Confirmed building with CONFIG_DUMMY=no works again, testet write and verify for VARIABLE_SIZE chip, code in probe_variable_size() looks quite good now.

Actions #10

Updated by Felix Singer over 1 year ago

  • Status changed from Closed to Resolved
Actions #11

Updated by Felix Singer over 1 year ago

  • % Done changed from 0 to 100
Actions

Also available in: Atom PDF